Remove dead lockfile.refresh_lockfile; fix module docstring (#99)

docs/developers-guide.md

@@ -225,16 +225,12 @@ this helper for consistent path handling, including `lading.cli`,
 Private helpers `_handle_git_ls_files_failure` and `_lockfiles_with_manifests`
 perform the error-handling and path-filtering passes respectively.
 
-`refresh_lockfile(manifest_path, runner)` runs
-`cargo generate-lockfile --manifest-path` for the supplied manifest and raises
-`LockfileRefreshError` on non-zero exit. `_refresh_lockfiles` in `bump.py`
-calls it after manifest rewrites. The refresh loop is intentionally not
-transactional: if a later lockfile refresh fails, previously rewritten
-manifests and refreshed lockfiles remain on disk, and the operator should fix
-the Cargo error, then run
-`cargo generate-lockfile --manifest-path <path>/Cargo.toml` for each affected
-crate manifest. Rerunning `lading bump` will not refresh lockfiles once the
-manifests are already rewritten.
+Lockfile regeneration after `lading bump` is owned by
+`lading.commands.bump_lockfiles.regenerate_lockfiles`, which runs
+`cargo update --workspace` per configured manifest. The two cargo strategies
+differ deliberately: bump refreshes existing pinned versions in place after
+manifest rewrites, while publish only probes freshness read-only via
+`cargo metadata --locked` and never regenerates.
 
 `validate_lockfile_freshness(manifest_path, runner)` runs
 `cargo metadata --locked --manifest-path ... --format-version=1`. It returns a
@@ -243,8 +239,8 @@ Cargo says need updating under `--locked`, and unrelated Cargo failures.
 `_validate_lockfile_freshness` in `publish_preflight.py` calls it before the
 cargo check/test pre-flight.
 
-`LockfileDiscoveryError` and `LockfileRefreshError` inherit `LadingError`;
-their messages include git or Cargo details respectively.
+`LockfileDiscoveryError` inherits `LadingError`; its messages include the git
+failure detail.
 
 ### `load_cargo_metadata`
 
@@ -349,7 +345,6 @@ area so existing handling remains precise:
 | `CargoMetadataError`        | `lading.workspace.metadata`        | Base for cargo metadata execution and parsing failures.                                         |
 | `CommandSpawnError`         | `lading.runtime.subprocess_runner` | Raised when the subprocess runner cannot spawn an external command.                             |
 | `LockfileDiscoveryError`    | `lading.commands.lockfile`         | Raised when git cannot list tracked lockfiles.                                                  |
-| `LockfileRefreshError`      | `lading.commands.lockfile`         | Raised when `cargo generate-lockfile` fails.                                                    |
 | `LockfileRegenerationError` | `lading.commands.bump_lockfiles`   | Raised when configured bump lockfile manifests are invalid or `cargo update --workspace` fails. |
 | `PublishPlanError`          | `lading.commands.publish_plan`     | Raised when a publish plan cannot be constructed.                                               |
 | `ReadmeTranspositionError`  | `lading.commands.bump_readme`      | Raised when the workspace README cannot be transposed into a crate during `lading bump`.        |
@@ -380,7 +375,7 @@ Usage guidance:
 - CLI and integration boundaries may catch `LadingError` to render expected
   lading failures as user-facing diagnostics.
 - Feature code should catch the narrowest local exception it can handle, such
-  as `PublishPreflightError` or `LockfileRefreshError`, and let unrelated
+  as `PublishPreflightError` or `LockfileRegenerationError`, and let unrelated
   `LadingError` subclasses propagate.
 - Tests should assert the specific exception type for the behaviour under test,
   then use `LadingError` only when verifying common boundary handling.
@@ -637,8 +632,6 @@ Defined metrics:
 | -------------------------------- | ----------------------------- | --------------------------------------------------------------------------------------------------------------------- |
 | `publish.index_lookup_downgrade` | `subcommand`, `missing_crate` | `_handle_index_missing_version` downgrades a crates.io index-lookup failure to a warning (in-plan, override enabled). |
 | `lockfile.discovered`            | (none)                        | Incremented by the number of tracked lockfiles each `discover_tracked_lockfiles` call returns.                        |
-| `lockfile.refresh`               | `outcome`                     | One increment per `refresh_lockfile` invocation; `outcome` is `success` or `failure`.                                 |
-| `lockfile.refresh.duration`      | (none)                        | Duration observation around each `cargo generate-lockfile` invocation.                                                |
 | `lockfile.validate`              | `outcome`                     | One increment per `validate_lockfile_freshness` call; `outcome` is `fresh`, `stale`, or `failed`.                     |
 | `lockfile.validate.duration`     | (none)                        | Duration observation around each `cargo metadata --locked` probe.                                                     |
 

lading/commands/lockfile.py

@@ -1,27 +1,29 @@
-"""Cargo lockfile discovery, refresh, and freshness validation helpers.
+"""Cargo lockfile discovery and freshness validation helpers.
 
 This module centralises the Cargo lockfile operations shared by release
-workflows. It discovers lockfiles that belong to the source workspace,
-regenerates them after manifest rewrites, and validates that Cargo can read
-them under ``--locked`` before expensive publish pre-flight commands run.
+workflows. It discovers lockfiles that belong to the source workspace and
+validates that Cargo can read them under ``--locked`` before expensive
+publish pre-flight commands run.
 
 Discovery is intentionally conservative. :func:`discover_tracked_lockfiles`
 queries the git index for tracked ``Cargo.lock`` files, then narrows the
 result to paths that are not under a ``target`` directory and have an adjacent
 ``Cargo.toml`` manifest.
 
-``lading bump`` calls :func:`discover_tracked_lockfiles` followed by
-:func:`refresh_lockfile` after it updates manifest versions. ``lading publish``
-uses :func:`discover_tracked_lockfiles` and
-:func:`validate_lockfile_freshness` before the cargo check/test pre-flight, so
-stale lockfiles fail early with an actionable repair command.
+Call graph: ``lading publish`` uses :func:`discover_tracked_lockfiles` and
+:func:`validate_lockfile_freshness` before the cargo check/test pre-flight,
+so stale lockfiles fail early with an actionable repair command.
+``lading bump`` regenerates lockfiles via
+:func:`lading.commands.bump_lockfiles.regenerate_lockfiles` instead, which
+runs ``cargo update --workspace``: bump wants existing pinned versions
+refreshed in place after manifest rewrites, whereas validation here uses
+``cargo metadata --locked`` purely as a read-only freshness probe.
 
-Typical local or CI usage follows the same sequence:
+Typical publish-side usage:
 
 ```python
 lockfiles = discover_tracked_lockfiles(workspace_root, runner)
 for lockfile_path in lockfiles:
-    refresh_lockfile(lockfile_path.parent / "Cargo.toml", runner)
     validate_lockfile_freshness(lockfile_path.parent / "Cargo.toml", runner)
 ```
 """
@@ -37,7 +39,7 @@
 
 from lading.exceptions import LadingError
 from lading.utils import metrics
-from lading.utils.process import append_detail, command_detail, with_detail
+from lading.utils.process import append_detail, command_detail
 
 if typ.TYPE_CHECKING:
     from lading.runtime import CommandRunner
@@ -47,16 +49,10 @@
 
 # Metric names (issue #91); documented in docs/developers-guide.md.
 DISCOVERED_LOCKFILES_METRIC = "lockfile.discovered"
-REFRESH_METRIC = "lockfile.refresh"
-REFRESH_DURATION_METRIC = "lockfile.refresh.duration"
 VALIDATE_METRIC = "lockfile.validate"
 VALIDATE_DURATION_METRIC = "lockfile.validate.duration"
 
 
-class LockfileRefreshError(LadingError):
-    """Raised when Cargo cannot regenerate a lockfile."""
-
-
 class LockfileDiscoveryError(LadingError):
     """Raised when git cannot list tracked lockfiles."""
 
@@ -174,49 +170,6 @@ def discover_tracked_lockfiles(
     return lockfiles
 
 
-def refresh_lockfile(
-    manifest_path: Path,
-    runner: CommandRunner,
-) -> Path:
-    """Regenerate the lockfile for ``manifest_path`` and return its path.
-
-    Parameters
-    ----------
-    manifest_path
-        Path to the ``Cargo.toml`` manifest to generate a lockfile for.
-    runner
-        Callable used to run external commands. It receives a command sequence
-        and returns ``(exit_code, stdout, stderr)``.
-
-    Returns
-    -------
-    Path
-        Path to the generated ``Cargo.lock`` in ``manifest_path.parent``.
-
-    Raises
-    ------
-    LockfileRefreshError
-        Raised when ``cargo generate-lockfile`` returns a non-zero exit code.
-        The error message includes Cargo's stderr, or stdout when stderr is
-        empty, so callers can report why regeneration failed.
-    """
-    lockfile_path = manifest_path.parent / "Cargo.lock"
-    LOGGER.info("Refreshing %s", lockfile_path)
-    started_at = time.perf_counter()
-    exit_code, stdout, stderr = runner(
-        ("cargo", "generate-lockfile", "--manifest-path", str(manifest_path)),
-        cwd=manifest_path.parent,
-    )
-    metrics.observe_duration(REFRESH_DURATION_METRIC, time.perf_counter() - started_at)
-    if exit_code != 0:
-        metrics.increment_counter(REFRESH_METRIC, outcome="failure")
-        message = with_detail(f"Failed to refresh {lockfile_path}", stdout, stderr)
-        raise LockfileRefreshError(message)
-    metrics.increment_counter(REFRESH_METRIC, outcome="success")
-    LOGGER.info("Refreshed %s", lockfile_path)
-    return lockfile_path
-
-
 def validate_lockfile_freshness(
     manifest_path: Path,
     runner: CommandRunner,

lading/exceptions.py

@@ -8,7 +8,7 @@
 
 Examples include `ConfigurationError`, `PublishPreflightError`,
 `CargoMetadataError`, `WorkspaceModelError`, `LockfileDiscoveryError`, and
-`LockfileRefreshError`. Feature-specific subclasses should continue to inherit
+`LockfileRegenerationError`. Feature-specific subclasses should continue to inherit
 from their local root exception, preserving precise handling within each
 component while keeping a consistent package-wide exception hierarchy.
 """

tests/unit/__snapshots__/test_command_failure_messages.ambr

@@ -8,9 +8,6 @@
 # name: test_cargo_metadata_invocation_message[stdout_fallback]
   'out'
 # ---
-# name: test_lockfile_refresh_failure_message
-  'Failed to refresh /ws/crates/alpha/Cargo.lock: error: registry offline'
-# ---
 # name: test_lockfile_regeneration_failure_message
   'Cargo lockfile regeneration failed for /ws/Cargo.toml with exit code 101: error: dependency conflict'
 # ---

tests/unit/test_command_failure_messages.py

@@ -24,7 +24,7 @@
 
 import pytest
 
-from lading.commands import bump_lockfiles, lockfile
+from lading.commands import bump_lockfiles
 from lading.workspace import metadata
 
 if typ.TYPE_CHECKING:
@@ -51,17 +51,6 @@ def runner(
     return runner
 
 
-def test_lockfile_refresh_failure_message(snapshot: SnapshotAssertion) -> None:
-    """``refresh_lockfile`` renders the canonical detail suffix."""
-    manifest = Path("/ws/crates/alpha/Cargo.toml")
-    with pytest.raises(lockfile.LockfileRefreshError) as excinfo:
-        lockfile.refresh_lockfile(
-            manifest, _failing_runner("", "error: registry offline\n")
-        )
-
-    assert snapshot == str(excinfo.value)
-
-
 def test_lockfile_regeneration_failure_message(snapshot: SnapshotAssertion) -> None:
     """``regenerate_lockfiles`` renders the canonical detail suffix."""
     with pytest.raises(bump_lockfiles.LockfileRegenerationError) as excinfo:

tests/unit/test_lockfile.py

@@ -175,49 +175,6 @@ def runner(
         lockfile.discover_tracked_lockfiles(tmp_path, runner)
 
 
-def test_refresh_lockfile_returns_lockfile_path(tmp_path: Path) -> None:
-    """Successful lockfile refresh returns the expected Cargo.lock path."""
-    manifest = tmp_path / "Cargo.toml"
-
-    def runner(
-        command: cabc.Sequence[str],
-        *,
-        cwd: Path | None = None,
-        env: cabc.Mapping[str, str] | None = None,
-    ) -> tuple[int, str, str]:
-        assert command == (
-            "cargo",
-            "generate-lockfile",
-            "--manifest-path",
-            str(manifest),
-        )
-        assert cwd == manifest.parent
-        return 0, "", ""
-
-    expected = tmp_path / "Cargo.lock"
-    result = lockfile.refresh_lockfile(manifest, runner)
-    assert result == expected, (
-        "refresh helper returned unexpected lockfile path; "
-        f"expected {expected!r}, got {result!r}"
-    )
-
-
-def test_refresh_lockfile_raises_on_failure(tmp_path: Path) -> None:
-    """Refresh failures include cargo stderr in the raised error."""
-    manifest = tmp_path / "Cargo.toml"
-
-    def runner(
-        command: cabc.Sequence[str],
-        *,
-        cwd: Path | None = None,
-        env: cabc.Mapping[str, str] | None = None,
-    ) -> tuple[int, str, str]:
-        return 101, "", "failed to resolve"
-
-    with pytest.raises(lockfile.LockfileRefreshError, match="failed to resolve"):
-        lockfile.refresh_lockfile(manifest, runner)
-
-
 def _validate_lockfile_freshness_for_result(
     tmp_path: Path, exit_code: int, stderr: str
 ) -> lockfile.LockfileFreshness:
@@ -380,28 +337,6 @@ def test_discovery_records_lockfile_count(tmp_path: Path) -> None:
     assert metrics.counter_value(lockfile.DISCOVERED_LOCKFILES_METRIC) == 1
 
 
-@pytest.mark.usefixtures("_metrics_registry")
-def test_refresh_records_success_outcome_and_duration(tmp_path: Path) -> None:
-    """A successful refresh counts a success and observes a duration."""
-    lockfile.refresh_lockfile(tmp_path / "Cargo.toml", _static_runner(0, "", ""))
-
-    assert metrics.counter_value(lockfile.REFRESH_METRIC, outcome="success") == 1
-    assert metrics.counter_value(lockfile.REFRESH_METRIC, outcome="failure") == 0
-    assert metrics.duration_stats(lockfile.REFRESH_DURATION_METRIC).count == 1
-
-
-@pytest.mark.usefixtures("_metrics_registry")
-def test_refresh_records_failure_outcome(tmp_path: Path) -> None:
-    """A failed refresh counts a failure and still observes a duration."""
-    with pytest.raises(lockfile.LockfileRefreshError):
-        lockfile.refresh_lockfile(
-            tmp_path / "Cargo.toml", _static_runner(101, "", "boom")
-        )
-
-    assert metrics.counter_value(lockfile.REFRESH_METRIC, outcome="failure") == 1
-    assert metrics.duration_stats(lockfile.REFRESH_DURATION_METRIC).count == 1
-
-
 @pytest.mark.usefixtures("_metrics_registry")
 @pytest.mark.parametrize(
     ("exit_code", "stderr", "expected_state"),