Remove backwards-compat shims guarding private symbols (#163)

docs/developers-guide.md

@@ -553,6 +553,61 @@ handle both validation and publish-phase failures through one `except` clause,
 or catch `PublishError` first when publish-phase failures require distinct
 handling.
 
+### Internal APIs carry no compatibility aliases
+
+Private (underscore-prefixed) functions and modules carry no stability
+contract: `lading`'s own application code is the only consumer of its
+internals. When a helper moves to a new canonical module, update every call
+site and test patch target in the same change — do not leave a module-level
+alias or thin wrapper behind to keep old `monkeypatch.setattr` targets
+resolving. Tests must patch or invoke the module that defines the helper
+(for example, `publish_preflight._run_preflight_checks` rather than a
+re-export on `publish`). If an internal seam churns often enough that many
+call sites keep breaking, introduce an explicit port (a protocol the call
+sites depend on, as with `CommandRunner` in `lading.runtime`) rather than
+accreting ad hoc backwards-compatibility shims.
+
+This rule applies to private, underscore-prefixed symbols only. A public
+exception type raised by a still-public entry point must remain re-exported
+on the module that exposes that entry point: for example, `publish` re-exports
+`PublishPlanError as PublishPlanError` so that callers catching
+`except publish.PublishPlanError` after a call to `publish.plan_publication()`
+continue to work.
+
+#### Shim inventory
+
+The issue #163 sweep removed the following shims; each row records the
+canonical replacement callers and tests now use directly:
+
+| Removed shim | Location | Canonical replacement |
+| --- | --- | --- |
+| Eleven `publish_preflight` private aliases (`_preflight_argument_sets`, `_CargoPreflightOptions`, `_apply_compiletest_externs`, `_build_preflight_environment`, `_build_test_arguments`, `_compose_preflight_arguments`, `_normalise_test_excludes`, `_run_aux_build_commands`, `_run_cargo_preflight`, `_validate_lockfile_freshness`, `_verify_clean_working_tree`) | `publish.py` | `lading.commands.publish_preflight` (patch/call the defining module directly) |
+| `_run_preflight_checks` thin wrapper | `publish.py` | `publish_preflight._run_preflight_checks` (called directly by `run()`) |
+| Re-exports `_append_section`, `_format_plan` | `publish.py` | `publish_plan.append_section`, `publish_plan.format_plan` |
+| Re-export `metadata_module` | `publish.py` | `lading.workspace.metadata` |
+| Re-export `StripPatchesSetting` | `publish.py` | `lading.config.StripPatchesSetting` |
+| Six `bump_toml` re-exports (`_parse_manifest`, `_select_table`, `_assign_version`, `_value_matches`, `_update_dependency_sections`, `_update_dependency_table`) | `bump.py` | `lading.commands.bump_toml` (`parse_manifest`, `select_table`, `assign_version`, `value_matches`, `update_dependency_sections`, `update_dependency_table`) |
+| `_log = LOGGER` alias | `bump.py` | the module-level `LOGGER` |
+| Private `_append_section` / `_format_plan` | `publish_plan.py` | renamed to public `append_section` / `format_plan` |
+| `split_command` / `_split_command` wrapper | `publish_execution.py` | `lading.runtime.subprocess_runner.split_command` |
+
+#### Retained boundaries
+
+Not every module-level indirection is a compatibility shim. The following
+boundaries were deliberately kept because they serve a purpose beyond masking
+a rename:
+
+- `publish._invoke` — the dependency-injection seam: it is the default
+  `CommandRunner` used by `run()`, and tests stub it to intercept subprocess
+  execution. Not a compatibility alias.
+- `publish.PublishPlanError` (re-exported as `PublishPlanError as
+  PublishPlanError`) — a public exception raised by the still-public
+  `publish.plan_publication()`; retained so `except publish.PublishPlanError`
+  keeps working. It is a public-API boundary, not a private shim.
+- The `CommandRunner` protocol in `lading.runtime` — the stable port for
+  execution concerns; the sanctioned alternative to ad hoc shims when a seam
+  churns.
+
 ### Extracted publish modules
 
 `publish_plan.py` owns publication planning and plan rendering. Its
@@ -874,13 +929,9 @@ reintroduces a second invocation log at any level is pinned by the tests in
 
 `lading.commands.publish_preflight` performs workspace validation before any
 crate is packaged or published. It is the canonical (and only) home of
-`_run_preflight_checks` and `_preflight_argument_sets`. `publish.py`
-re-exports `_preflight_argument_sets` as a bare module-level alias for
-backwards compatibility with existing test patches, and must not re-declare
-it. `_run_preflight_checks`, however, is exposed through a thin wrapper in
-`publish.py` that preserves the historical optional-`configuration` contract
-(resolving configuration via `_ensure_configuration` when the caller omits
-it) before delegating to the canonical implementation. The public entry
+`_run_preflight_checks` and its helpers. `publish.py` calls the module
+directly and holds no aliases or wrappers for its names; tests that patch or
+invoke pre-flight helpers must target `publish_preflight` itself. The entry
 point is:
 
 ```python

docs/lading-design.md

@@ -207,11 +207,9 @@ graph TD
     H --> I["Updated staged manifest used for publish"]
 
     B --> J["Module: lading.commands.publish_execution (publish_execution.py)"]
-    J --> K["split_command"]
-    J --> L["should_use_cmd_mox_stub"]
-    J --> M["normalise_cmd_mox_command"]
+    J --> K["_invoke: subprocess execution with error adaptation"]
 
-    B --> N["Use re-exported publish helpers"]
+    B --> N["Module: lading.commands.publish_preflight (publish_preflight.py)"]
     N --> O["Compose final publish plan and execute commands"]
 ```
 

lading/commands/bump.py

@@ -24,7 +24,6 @@
 - :mod:`lading.commands.bump_docs` — documentation version rewrites.
 - :mod:`lading.commands.bump_lockfiles` — lockfile regeneration.
 - :mod:`lading.commands.bump_readme` — workspace README transposition.
-- :mod:`lading.commands.bump_toml` — low-level TOML manipulation.
 """
 
 from __future__ import annotations
@@ -42,7 +41,6 @@
     bump_lockfiles,
     bump_manifests,
     bump_readme,
-    bump_toml,
 )
 from lading.commands.bump_manifests import (
     _WORKSPACE_SELECTORS,
@@ -64,7 +62,6 @@
     from lading.workspace import WorkspaceCrate, WorkspaceGraph
 
 LOGGER = logging.getLogger(__name__)
-_log = LOGGER
 
 
 @dc.dataclass(frozen=True, slots=True)
@@ -134,7 +131,7 @@ def run(
 ) -> str:
     """Update workspace and crate manifest versions to ``target_version``."""
     context = _initialize_bump_context(workspace_root, options)
-    _log.debug(
+    LOGGER.debug(
         "Bump context initialised: %d excluded crate(s), %d to update",
         len(context.excluded),
         len(context.updated_crate_names),
@@ -182,7 +179,7 @@ def _initialize_bump_context(
         if resolved_options.rebuild_lockfiles is None
         else resolved_options.rebuild_lockfiles
     )
-    _log.debug(
+    LOGGER.debug(
         "rebuild_lockfiles resolution: raw_flag=%r, configured_default=%r, resolved=%r",
         resolved_options.rebuild_lockfiles,
         configuration.bump.rebuild_lockfiles,
@@ -253,7 +250,7 @@ def _process_crate_manifests(
                 updated_count += 1
             case _CrateManifestOutcome.UNCHANGED:
                 pass
-    _log.debug(
+    LOGGER.debug(
         "Crate manifest processing complete: %d processed, %d skipped, %d updated",
         processed_count,
         skipped_count,
@@ -279,7 +276,7 @@ def _process_documentation_files(
 
 def _process_readme_transposition(context: _BumpContext, *, dry_run: bool) -> set[Path]:
     """Transpose workspace README files into opted-in member crates."""
-    _log.debug("Starting workspace README transposition")
+    LOGGER.debug("Starting workspace README transposition")
     changed_readmes: set[Path] = set()
     transposed_entry_count = 0
     source_readme_path = context.root_path / "README.md"
@@ -300,11 +297,11 @@ def _process_readme_transposition(context: _BumpContext, *, dry_run: bool) -> se
                 _source_text=cached_text,
             )
         except bump_readme.ReadmeTranspositionError:
-            _log.error("README transposition failed for crate %r", crate.name)
+            LOGGER.exception("README transposition failed for crate %r", crate.name)
             raise
         if changed_path is not None:
             changed_readmes.add(changed_path)
-    _log.debug(
+    LOGGER.debug(
         "README transposition complete: %d entries, %d file(s) changed",
         transposed_entry_count,
         len(changed_readmes),
@@ -379,7 +376,7 @@ def _apply_crate_manifest_update(
     )
 
     if _should_skip_crate_update(selectors, dependency_sections):
-        _log.debug("Skipping crate manifest update for excluded crate %r", crate.name)
+        LOGGER.debug("Skipping crate manifest update for excluded crate %r", crate.name)
         return _CrateManifestOutcome.SKIPPED
 
     crate_options = dc.replace(
@@ -393,19 +390,19 @@ def _apply_crate_manifest_update(
         crate_options,
     )
     if was_updated:
-        _log.debug(
+        LOGGER.debug(
             "Updated crate manifest for crate %r: manifest=%s",
             crate.name,
             crate.manifest_path,
         )
         if not crate_options.dry_run:
-            _log.debug(
+            LOGGER.debug(
                 "Wrote crate manifest for crate %r: manifest=%s",
                 crate.name,
                 crate.manifest_path,
             )
     else:
-        _log.debug(
+        LOGGER.debug(
             "Crate manifest already up to date for crate %r: manifest=%s",
             crate.name,
             crate.manifest_path,
@@ -415,12 +412,3 @@ def _apply_crate_manifest_update(
         if was_updated
         else _CrateManifestOutcome.UNCHANGED
     )
-
-
-# Re-export low-level TOML helpers used by tests for backward compatibility.
-_parse_manifest = bump_toml.parse_manifest
-_select_table = bump_toml.select_table
-_assign_version = bump_toml.assign_version
-_value_matches = bump_toml.value_matches
-_update_dependency_sections = bump_toml.update_dependency_sections
-_update_dependency_table = bump_toml.update_dependency_table

lading/commands/publish.py

@@ -36,12 +36,10 @@
 **Related modules**
 
 * :mod:`lading.commands.publish_plan` — plan construction and formatting
-* :mod:`lading.commands.publish_preflight` — canonical home of
-  ``_run_preflight_checks`` and ``_preflight_argument_sets`` (``cargo check``
-  / ``cargo test`` / git-status guards). This module re-exports
-  ``_preflight_argument_sets`` as a bare alias and ``_run_preflight_checks``
-  as a thin configuration-resolving wrapper, both for backwards
-  compatibility with existing test patches.
+* :mod:`lading.commands.publish_preflight` — canonical home of the publish
+  pre-flight checks (``cargo check`` / ``cargo test`` / git-status guards).
+  Callers and tests use that module directly; this module holds no
+  compatibility aliases for its helpers.
 * :mod:`lading.commands.publish_errors` — :class:`PublishPreflightError` and
   :class:`PublishError`
 * :mod:`lading.commands.publish_execution` — subprocess invocation and cmd-mox
@@ -59,7 +57,7 @@
 from pathlib import Path
 
 from lading import config as config_module
-from lading.commands import publish_preflight as _publish_preflight
+from lading.commands import publish_preflight
 from lading.commands.cargo_output_adapter import (
     CargoIndexLookupFailure,
     CargoSubprocessResult,
@@ -82,38 +80,13 @@
 )
 from lading.commands.publish_plan import (
     PublishPlan,
-    append_section,
     format_plan,
     plan_publication,
 )
 from lading.commands.publish_plan import (
-    PublishPlanError as _PublishPlanError,
+    PublishPlanError as PublishPlanError,  # public re-export for plan_publication
 )
 from lading.utils.path import normalise_workspace_root
-from lading.workspace import metadata as _metadata_module
-
-StripPatchesSetting = config_module.StripPatchesSetting
-metadata_module = _metadata_module
-PublishPlanError = _PublishPlanError
-_append_section = append_section
-_format_plan = format_plan
-# Backwards-compatible aliases (issue #96): publish_preflight owns the
-# canonical implementations; existing tests patch and call them through
-# this module, so the names must keep resolving here. Plain assignments
-# (not imports) keep the re-export intent visible to linters.
-# (``_run_preflight_checks`` is the exception: it is a thin wrapper defined
-# below that preserves the historical optional-``configuration`` contract.)
-_preflight_argument_sets = _publish_preflight._preflight_argument_sets
-_CargoPreflightOptions = _publish_preflight._CargoPreflightOptions
-_apply_compiletest_externs = _publish_preflight._apply_compiletest_externs
-_build_preflight_environment = _publish_preflight._build_preflight_environment
-_build_test_arguments = _publish_preflight._build_test_arguments
-_compose_preflight_arguments = _publish_preflight._compose_preflight_arguments
-_normalise_test_excludes = _publish_preflight._normalise_test_excludes
-_run_aux_build_commands = _publish_preflight._run_aux_build_commands
-_run_cargo_preflight = _publish_preflight._run_cargo_preflight
-_validate_lockfile_freshness = _publish_preflight._validate_lockfile_freshness
-_verify_clean_working_tree = _publish_preflight._verify_clean_working_tree
 
 LOGGER = logging.getLogger(__name__)
 
@@ -592,28 +565,6 @@ def _ensure_configuration(
         return config_module.load_configuration(workspace_root)
 
 
-def _run_preflight_checks(
-    workspace_root: Path,
-    *,
-    allow_dirty: bool,
-    configuration: LadingConfig | None = None,
-    runner: CommandRunner | None = None,
-) -> None:
-    """Run publish pre-flight checks, resolving configuration when absent.
-
-    Thin wrapper over :func:`publish_preflight._run_preflight_checks` that
-    preserves the historical optional-``configuration`` contract: callers may
-    omit ``configuration`` and have it loaded from the active context or the
-    workspace before the canonical implementation runs.
-    """
-    _publish_preflight._run_preflight_checks(
-        workspace_root,
-        allow_dirty=allow_dirty,
-        configuration=_ensure_configuration(configuration, workspace_root),
-        runner=runner,
-    )
-
-
 def _ensure_workspace(
     workspace: WorkspaceGraph | None, workspace_root: Path
 ) -> WorkspaceGraph:
@@ -705,7 +656,7 @@ def run(
     active_configuration = _ensure_configuration(configuration_override, root_path)
     active_workspace = _ensure_workspace(workspace_override, root_path)
 
-    _run_preflight_checks(
+    publish_preflight._run_preflight_checks(
         root_path,
         allow_dirty=effective_options.allow_dirty,
         configuration=active_configuration,

lading/commands/publish_execution.py

@@ -7,7 +7,6 @@
 
 from lading.commands.publish_errors import PublishPreflightError
 from lading.runtime import CommandSpawnError
-from lading.runtime.subprocess_runner import split_command as _runtime_split_command
 from lading.runtime.subprocess_runner import (
     subprocess_runner as _default_subprocess_runner,
 )
@@ -36,17 +35,6 @@ def _invoke(
         raise _publish_error(str(exc)) from exc
 
 
-def _split_command(command: cabc.Sequence[str]) -> tuple[str, tuple[str, ...]]:
-    """Return the program and argument tuple for ``command``."""
-    try:
-        return _runtime_split_command(command)
-    except ValueError as exc:
-        raise _publish_error(str(exc)) from exc
-
-
-split_command = _split_command
-
 __all__ = [
     "_invoke",
-    "split_command",
 ]