From 09ad0d829e280aa9b8c99444417b3c315e0b5c25 Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 10 Jun 2026 00:53:16 +0200 Subject: [PATCH 1/8] Tidy minor structural overlaps in plan/CLI/publish paths 13a: collapse _format_crates_section into the single section renderer. The bespoke function duplicated _append_section plus an empty message and a fixed formatter. The renderer is now _render_section, a query returning the section lines (which also keeps the argument count within the lint ceiling); _format_plan composes it for every section. The historical public append_section (listed in __all__) is retained as a thin backwards-compatible shim that extends the passed-in list by delegating to _render_section, so existing importers keep working while new code uses render_section. 13b: catch PublishPreflightError alone in the live pipeline, with a comment noting that PublishError subclasses it, so the redundant tuple is not reintroduced by accident. 13c: deduplicate the load-workspace-and-run block in cli._run_with_context. Configuration scope is resolved first (a nullcontext when already loaded), then a single block runs the command. 13d: document that utils.commands.LADING_CATALOGUE is staged but not yet wired into the execution path, in both the module docstring and the developers' guide, with a pointer to the roadmap Phase 5 steps that will wire it. Tests: property test pins the render_section header/item/empty-message invariants, unit tests pin the append_section shim (list mutation, empty no-op, parity with render_section), syrupy snapshots lock the fully rendered publish plan with and without publishable crates, and a behavioural test pins identical downstream behaviour for both _run_with_context branches. Closes #107 --- docs/developers-guide.md | 6 + lading/cli.py | 14 +- lading/commands/publish.py | 4 +- lading/commands/publish_plan.py | 102 +++++++++----- lading/utils/commands.py | 6 + .../test_formatting_helpers.ambr | 23 ++++ tests/unit/publish/test_formatting_helpers.py | 128 ++++++++++++++++-- tests/unit/test_cli.py | 41 ++++++ tests/unit/test_publish_formatting.py | 38 ------ 9 files changed, 271 insertions(+), 91 deletions(-) create mode 100644 tests/unit/publish/__snapshots__/test_formatting_helpers.ambr diff --git a/docs/developers-guide.md b/docs/developers-guide.md index e13cc649..c609efa5 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -913,6 +913,12 @@ values are pinned by a syrupy snapshot. See the [cmd-mox usage guide](./cmd-mox-usage-guide.md#environment-variables) for the operator-facing description of the variable and its failure modes. +`lading.utils.commands.LADING_CATALOGUE` is the staged cuprum programme +catalogue (cargo, git). It is intentionally not yet wired into the execution +path — the subprocess runner still spawns processes directly — and becomes live +with the Phase 5 runner-migration steps in `docs/roadmap.md`. Treat it as a +registration point, not as active allowlist enforcement. + #### Subprocess invocation logging `subprocess_runner` emits **exactly one** log record per external command diff --git a/lading/cli.py b/lading/cli.py index a1017258..2577feff 100644 --- a/lading/cli.py +++ b/lading/cli.py @@ -24,7 +24,7 @@ import os import sys import typing as typ -from contextlib import contextmanager +from contextlib import AbstractContextManager, contextmanager, nullcontext from pathlib import Path from cyclopts import App @@ -303,17 +303,15 @@ def _run_with_context( ) -> str: """Execute ``runner`` with configuration and workspace data.""" active_runner = command_runner or _select_runner() + configuration_scope: AbstractContextManager[object] = nullcontext() try: configuration = config.current_configuration() except config.ConfigurationNotLoadedError: + # Freshly loaded configuration must be installed for the duration of + # the command; an already-active configuration needs no new scope. configuration = config.load_configuration(workspace_root) - with ( - config.use_configuration(configuration), - metadata_module.use_command_runner(active_runner), - ): - workspace_model = load_workspace(workspace_root) - return runner(workspace_root, configuration, workspace_model, active_runner) - with metadata_module.use_command_runner(active_runner): + configuration_scope = config.use_configuration(configuration) + with configuration_scope, metadata_module.use_command_runner(active_runner): workspace_model = load_workspace(workspace_root) return runner(workspace_root, configuration, workspace_model, active_runner) diff --git a/lading/commands/publish.py b/lading/commands/publish.py index 7b0aaea2..9eda7ab8 100644 --- a/lading/commands/publish.py +++ b/lading/commands/publish.py @@ -539,7 +539,9 @@ def _execute_live_publication_pipeline( ", ".join(completed) if completed else "none", ) raise PublishPreflightError(str(exc)) from exc - except (PublishPreflightError, PublishError): + # PublishError subclasses PublishPreflightError, so the base class + # alone covers both pre-flight and publish failures here. + except PublishPreflightError: LOGGER.exception( "Live pipeline: aborted on crate %s — %d/%d crates completed (%s)", crate.name, diff --git a/lading/commands/publish_plan.py b/lading/commands/publish_plan.py index a7bcc603..bff3fc22 100644 --- a/lading/commands/publish_plan.py +++ b/lading/commands/publish_plan.py @@ -197,19 +197,49 @@ def plan_publication( ) -def _format_crates_section( - lines: list[str], - crates: tuple[WorkspaceCrate, ...], +def render_section[T]( + items: cabc.Sequence[T], *, header: str, + formatter: cabc.Callable[[T], str] = str, empty_message: str | None = None, -) -> None: - """Append publishable crate details to ``lines``.""" - if crates: - lines.append(header) - lines.extend(f"- {crate.name} @ {crate.version}" for crate in crates) - elif empty_message is not None: - lines.append(empty_message) +) -> list[str]: + """Return the rendered lines for one plan section. + + This is the single section renderer for publish-plan output (issue #107): + a query returning the formatted lines rather than mutating an accumulator, + so callers compose sections with ``list.extend``. + + Parameters + ---------- + items: + Entries rendered beneath ``header``. An empty sequence renders no + header and no bullets unless ``empty_message`` is supplied. + header: + Section heading emitted before the formatted entries. + formatter: + Callable mapping each item to its display string. Defaults to ``str``. + empty_message: + Rendered in place of the section when ``items`` is empty; omit it to + skip empty sections entirely. + + Returns + ------- + list[str] + The rendered lines for the section. + + Examples + -------- + >>> render_section(["alpha", "beta"], header="Members:") + ['Members:', '- alpha', '- beta'] + >>> render_section([], header="Members:") + [] + >>> render_section([], header="Members:", empty_message="Members: none") + ['Members: none'] + """ + if items: + return [header, *(f"- {formatter(item)}" for item in items)] + return [] if empty_message is None else [empty_message] def append_section[T]( @@ -221,6 +251,10 @@ def append_section[T]( ) -> None: """Append a formatted section to ``lines`` when ``items`` is non-empty. + Thin command wrapper over :func:`render_section` (issue #107), retained as + the historical public helper. New code may prefer ``render_section``, which + returns the lines as a query rather than mutating ``lines``. + Parameters ---------- lines: @@ -248,9 +282,7 @@ def append_section[T]( >>> lines ['Header:', 'Members:', '- alpha', '- beta'] """ - if items: - lines.append(header) - lines.extend(f"- {formatter(item)}" for item in items) + lines.extend(render_section(items, header=header, formatter=formatter)) def format_plan(plan: PublishPlan, *, strip_patches: StripPatchesSetting) -> str: @@ -288,28 +320,33 @@ def format_plan(plan: PublishPlan, *, strip_patches: StripPatchesSetting) -> str f"Strip patch strategy: {strip_patches}", ] - _format_crates_section( - lines, - plan.publishable, - header=f"Crates to publish ({len(plan.publishable)}):", - empty_message="Crates to publish: none", + lines.extend( + render_section( + plan.publishable, + header=f"Crates to publish ({len(plan.publishable)}):", + formatter=lambda crate: f"{crate.name} @ {crate.version}", + empty_message="Crates to publish: none", + ) ) - append_section( - lines, - plan.skipped_manifest, - header="Skipped (publish = false):", - formatter=lambda crate: crate.name, + lines.extend( + render_section( + plan.skipped_manifest, + header="Skipped (publish = false):", + formatter=lambda crate: crate.name, + ) ) - append_section( - lines, - plan.skipped_configuration, - header="Skipped via publish.exclude:", - formatter=lambda crate: crate.name, + lines.extend( + render_section( + plan.skipped_configuration, + header="Skipped via publish.exclude:", + formatter=lambda crate: crate.name, + ) ) - append_section( - lines, - plan.missing_configuration_exclusions, - header="Configured exclusions not found in workspace:", + lines.extend( + render_section( + plan.missing_configuration_exclusions, + header="Configured exclusions not found in workspace:", + ) ) return "\n".join(lines) @@ -321,4 +358,5 @@ def format_plan(plan: PublishPlan, *, strip_patches: StripPatchesSetting) -> str "append_section", "format_plan", "plan_publication", + "render_section", ] diff --git a/lading/utils/commands.py b/lading/utils/commands.py index 8ceca55c..d38f3af5 100644 --- a/lading/utils/commands.py +++ b/lading/utils/commands.py @@ -8,6 +8,12 @@ Migration context: This is Step 5.1 of the Cuprum migration. Subsequent steps will migrate existing plumbum and subprocess code to use this catalogue via ``scoped(allowlist=LADING_CATALOGUE.allowlist)``. + +The catalogue is staged but intentionally not yet wired into the execution +path: the subprocess runner still spawns processes directly. The wiring +lands with the runner migration steps in ``docs/roadmap.md`` (Phase 5, +"Migrate `lading/runtime/subprocess_runner.py`" onwards); do not mistake +this module for live enforcement in the meantime. """ from __future__ import annotations diff --git a/tests/unit/publish/__snapshots__/test_formatting_helpers.ambr b/tests/unit/publish/__snapshots__/test_formatting_helpers.ambr new file mode 100644 index 00000000..bf77a28c --- /dev/null +++ b/tests/unit/publish/__snapshots__/test_formatting_helpers.ambr @@ -0,0 +1,23 @@ +# serializer version: 1 +# name: test_format_plan_snapshot_with_publishable + ''' + Publish plan for + Strip patch strategy: all + Crates to publish (2): + - alpha @ 0.1.0 + - beta @ 0.1.0 + Skipped (publish = false): + - gamma + Skipped via publish.exclude: + - delta + Configured exclusions not found in workspace: + - missing + ''' +# --- +# name: test_format_plan_snapshot_without_publishable + ''' + Publish plan for + Strip patch strategy: per-crate + Crates to publish: none + ''' +# --- diff --git a/tests/unit/publish/test_formatting_helpers.py b/tests/unit/publish/test_formatting_helpers.py index da9e1657..f796e82a 100644 --- a/tests/unit/publish/test_formatting_helpers.py +++ b/tests/unit/publish/test_formatting_helpers.py @@ -3,27 +3,29 @@ from __future__ import annotations import typing as typ +from pathlib import Path + +import hypothesis.strategies as st +from hypothesis import given from lading.commands import publish_plan from .conftest import make_crate if typ.TYPE_CHECKING: - from pathlib import Path + from syrupy.assertion import SnapshotAssertion -def test_append_section_appends_formatted_items() -> None: +def test_render_section_renders_formatted_items() -> None: """Generic section helper applies the provided formatter.""" class Dummy: def __init__(self, value: str) -> None: self.value = value - lines = [] items = (Dummy("alpha"), Dummy("beta")) - publish_plan.append_section( - lines, + lines = publish_plan.render_section( items, header="Header:", formatter=lambda item: item.value.upper(), @@ -32,22 +34,50 @@ def __init__(self, value: str) -> None: assert lines == ["Header:", "- ALPHA", "- BETA"] -def test_append_section_defaults_to_string_conversion() -> None: +def test_render_section_defaults_to_string_conversion() -> None: """Default formatter handles simple string values without boilerplate.""" - lines: list[str] = [] + lines = publish_plan.render_section(("alpha", "beta"), header="Header:") + + assert lines == ["Header:", "- alpha", "- beta"] + + +def test_render_section_omits_header_for_empty_sequences() -> None: + """Helper returns no lines when there is nothing to report.""" + assert publish_plan.render_section((), header="Header:") == [] + + +def test_append_section_extends_list_in_place() -> None: + """The backwards-compatible shim mutates ``lines`` like the old helper.""" + lines = ["preamble"] publish_plan.append_section(lines, ("alpha", "beta"), header="Header:") - assert lines == ["Header:", "- alpha", "- beta"] + assert lines == ["preamble", "Header:", "- alpha", "- beta"] -def test_append_section_omits_header_for_empty_sequences() -> None: - """Helper leaves ``lines`` unchanged when there is nothing to report.""" - lines = ["prefix"] +def test_append_section_appends_nothing_when_empty() -> None: + """An empty section leaves ``lines`` untouched, matching prior behaviour.""" + lines = ["preamble"] publish_plan.append_section(lines, (), header="Header:") - assert lines == ["prefix"] + assert lines == ["preamble"] + + +def test_append_section_matches_render_section() -> None: + """The shim delegates to ``render_section`` so both stay in lock-step.""" + lines: list[str] = [] + + publish_plan.append_section( + lines, + ("alpha", "beta"), + header="Header:", + formatter=str.upper, + ) + + assert lines == publish_plan.render_section( + ("alpha", "beta"), header="Header:", formatter=str.upper + ) def test_format_plan_formats_skipped_sections(tmp_path: Path) -> None: @@ -73,3 +103,77 @@ def test_format_plan_formats_skipped_sections(tmp_path: Path) -> None: assert lines[manifest_index + 1] == "- beta" assert lines[configuration_index + 1] == "- gamma" assert lines[missing_index + 1] == "- missing" + + +def test_render_section_renders_empty_message_when_empty() -> None: + """The optional ``empty_message`` replaces an absent section.""" + lines = publish_plan.render_section( + (), header="Header:", empty_message="Nothing to report" + ) + + assert lines == ["Nothing to report"] + + +@given( + items=st.lists(st.text(min_size=1, max_size=8), max_size=6), + use_empty_message=st.booleans(), +) +def test_render_section_invariants( + items: list[str], + use_empty_message: bool, # noqa: FBT001 - hypothesis-driven keyword +) -> None: + """Header appears iff items exist; each item renders exactly once.""" + empty_message = "Nothing to report" if use_empty_message else None + + lines = publish_plan.render_section( + items, header="Header:", empty_message=empty_message + ) + + if items: + assert lines[0] == "Header:" + assert lines[1:] == [f"- {item}" for item in items] + elif empty_message is not None: + assert lines == [empty_message] + else: + assert lines == [] + + +def _normalise_plan_message(message: str, root: Path) -> str: + """Replace the absolute workspace root so snapshots stay deterministic.""" + return message.replace(str(root), "") + + +def test_format_plan_snapshot_with_publishable( + tmp_path: Path, snapshot: SnapshotAssertion +) -> None: + """The full rendered plan with publishable and skipped crates is stable.""" + root = tmp_path.resolve() + plan = publish_plan.PublishPlan( + workspace_root=root, + publishable=(make_crate(root, "alpha"), make_crate(root, "beta")), + skipped_manifest=(make_crate(root, "gamma", publish_flag=False),), + skipped_configuration=(make_crate(root, "delta"),), + missing_configuration_exclusions=("missing",), + ) + + message = publish_plan.format_plan(plan, strip_patches="all") + + assert snapshot == _normalise_plan_message(message, root) + + +def test_format_plan_snapshot_without_publishable( + tmp_path: Path, snapshot: SnapshotAssertion +) -> None: + """An empty publish set renders the empty message instead of a header.""" + root = tmp_path.resolve() + plan = publish_plan.PublishPlan( + workspace_root=root, + publishable=(), + skipped_manifest=(), + skipped_configuration=(), + missing_configuration_exclusions=(), + ) + + message = publish_plan.format_plan(plan, strip_patches="per-crate") + + assert snapshot == _normalise_plan_message(message, root) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index ad5e427d..666aed01 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -710,3 +710,44 @@ def test_workspace_env_sets_and_restores( with cli._workspace_env(tmp_path): assert os.environ[cli.WORKSPACE_ROOT_ENV_VAR] == str(tmp_path) assert cli.WORKSPACE_ROOT_ENV_VAR not in os.environ + + +def test_run_with_context_branches_behave_identically( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + write_config: cabc.Callable[[str], Path], +) -> None: + """Pre-loaded and freshly-loaded configuration take the same path. + + Issue #107 (13c): `_run_with_context` previously duplicated the + load-workspace-and-run block across both branches; this pins identical + downstream behaviour for each. + """ + write_config("") + workspace_graph = _make_workspace(tmp_path.resolve()) + monkeypatch.setattr(cli, "load_workspace", lambda _: workspace_graph) + calls: list[tuple[object, ...]] = [] + + def runner( + root: Path, + configuration: config_module.LadingConfig, + workspace: WorkspaceGraph, + command_runner: object, + ) -> str: + calls.append((root, configuration, workspace, command_runner)) + return "ran" + + fresh_result = cli._run_with_context(tmp_path.resolve(), runner) + + configuration = config_module.load_configuration(tmp_path) + with config_module.use_configuration(configuration): + preloaded_result = cli._run_with_context(tmp_path.resolve(), runner) + + assert fresh_result == preloaded_result == "ran" + assert len(calls) == 2 + fresh_call, preloaded_call = calls + assert fresh_call[0] == preloaded_call[0] == tmp_path.resolve() + assert isinstance(fresh_call[1], config_module.LadingConfig) + assert preloaded_call[1] is configuration + assert fresh_call[2] is workspace_graph + assert preloaded_call[2] is workspace_graph diff --git a/tests/unit/test_publish_formatting.py b/tests/unit/test_publish_formatting.py index 0b839c4f..ef454fbb 100644 --- a/tests/unit/test_publish_formatting.py +++ b/tests/unit/test_publish_formatting.py @@ -14,44 +14,6 @@ from lading.workspace import WorkspaceCrate -def test_append_section_appends_formatted_items() -> None: - """Generic section helper applies the provided formatter.""" - - class Dummy: - def __init__(self, value: str) -> None: - self.value = value - - lines: list[str] = [] - items = (Dummy("alpha"), Dummy("beta")) - - publish_plan.append_section( - lines, - items, - header="Header:", - formatter=lambda item: item.value.upper(), - ) - - assert lines == ["Header:", "- ALPHA", "- BETA"] - - -def test_append_section_defaults_to_string_conversion() -> None: - """Default formatter handles simple string values without boilerplate.""" - lines: list[str] = [] - - publish_plan.append_section(lines, ("alpha", "beta"), header="Header:") - - assert lines == ["Header:", "- alpha", "- beta"] - - -def test_append_section_omits_header_for_empty_sequences() -> None: - """Helper leaves ``lines`` unchanged when there is nothing to report.""" - lines = ["prefix"] - - publish_plan.append_section(lines, (), header="Header:") - - assert lines == ["prefix"] - - def test_format_plan_formats_skipped_sections( tmp_path: Path, make_crate: cabc.Callable[[Path, str, _CrateSpec | None], WorkspaceCrate], From d5cd4c516b8baad1cd5bf8d5763e80dcd4114373 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 19 Jul 2026 23:03:16 +0200 Subject: [PATCH 2/8] Document publish-plan output and test the CLI config path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address two review warnings on the publish-plan/CLI work. Users' guide: document the publish-plan summary that `lading publish` prints on completion — the crates-to-publish list, the skipped-crate sections, and the `Crates to publish: none` empty state — which was previously covered only by tests and the developers' guide. CLI-boundary test: add `test_publish_via_app_matches_across_config_branches`, which drives `cli.app` (the public command boundary) for both `_run_with_context` configuration branches. `cli.main` always installs configuration before dispatch, so the disk-loaded branch is reachable publicly only through `cli.app` without an active scope. The test asserts identical downstream behaviour (same workspace, equal configuration) while pinning the provenance difference: the disk branch reloads a fresh config object; the pre-loaded branch reuses the active one. The white-box `test_run_with_context_branches_behave_identically` is retained. --- docs/users-guide.md | 26 +++++++++++++++++++ tests/unit/test_cli.py | 57 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/docs/users-guide.md b/docs/users-guide.md index dc1b4636..9eb50af5 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -155,6 +155,32 @@ crate fails, crates already uploaded to crates.io are not rolled back. Reruns skip versions that are already present on crates.io and continue with the remaining crates. +At the end of the run, `lading publish` prints a summary of the publish plan +it computed, listing the crates to publish and any crates skipped because +they are marked `publish = false`, excluded via `publish.exclude`, or named +in `publish.exclude` but absent from the workspace. When no crates are +publishable, the summary reports `Crates to publish: none` instead of a list: + +```text +Publish plan for +Strip patch strategy: all +Crates to publish (2): +- alpha @ 0.1.0 +- beta @ 0.1.0 +Skipped (publish = false): +- gamma +Skipped via publish.exclude: +- delta +Configured exclusions not found in workspace: +- missing +``` + +```text +Publish plan for +Strip patch strategy: per-crate +Crates to publish: none +``` + `bump` adopts the workspace `README.md` for any member crate that sets `readme.workspace = true`. The adopted README is written into the crate directory and relative Markdown links are rewritten so they still resolve from diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 666aed01..1eb90035 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -751,3 +751,60 @@ def runner( assert preloaded_call[1] is configuration assert fresh_call[2] is workspace_graph assert preloaded_call[2] is workspace_graph + + +def test_publish_via_app_matches_across_config_branches( + monkeypatch: pytest.MonkeyPatch, + tmp_path: Path, + minimal_config: Path, +) -> None: + """Public ``lading publish`` behaves the same for both config branches. + + Issue #107 (13c): this exercises both ``_run_with_context`` branches + through the public Cyclopts command boundary (``cli.app``), complementing + the white-box ``test_run_with_context_branches_behave_identically``. + ``cli.main`` always installs configuration before dispatch, so the + disk-loaded branch is only reachable publicly via ``cli.app`` invoked + without an active configuration scope. + """ + assert minimal_config.exists() + workspace_graph = _make_workspace(tmp_path.resolve()) + monkeypatch.setattr(cli, "load_workspace", lambda _: workspace_graph) + calls: list[tuple[Path, config_module.LadingConfig, WorkspaceGraph]] = [] + + def fake_run( + root: Path, + configuration: config_module.LadingConfig, + workspace: WorkspaceGraph, + *, + options: publish_command.PublishOptions, + ) -> str: + assert isinstance(options, publish_command.PublishOptions) + calls.append((root, configuration, workspace)) + return "published" + + monkeypatch.setattr(publish_command, "run", fake_run) + args = ["publish", "--workspace-root", str(tmp_path)] + + # Disk-loaded branch: no configuration is active, so `_run_with_context` + # loads `lading.toml` from disk. + disk_result = cli.app(args) + + # Pre-loaded branch: an active configuration scope makes the same call + # reuse that configuration under a nullcontext instead of reloading. + preloaded = config_module.load_configuration(tmp_path) + with config_module.use_configuration(preloaded): + preloaded_result = cli.app(args) + + assert disk_result == preloaded_result == "published" + assert len(calls) == 2 + disk_call, preloaded_call = calls + # Identical downstream behaviour: same workspace root, same injected + # workspace graph, and equal configuration content for both branches. + assert disk_call[0] == preloaded_call[0] == tmp_path.resolve() + assert disk_call[2] is preloaded_call[2] is workspace_graph + assert disk_call[1] == preloaded_call[1] + # The branches differ only in provenance: the disk branch reloads a fresh + # config object; the pre-loaded branch reuses the active one. + assert preloaded_call[1] is preloaded + assert disk_call[1] is not preloaded From 99bc53151692313ad1c4ce62c89208965f298c4a Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 22 Jul 2026 19:22:28 +0200 Subject: [PATCH 3/8] Add assertion messages and link the roadmap reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review comments on the publish-plan/CLI test and docs. Tests: give the previously bare assertions in the render_section unit tests focused diagnostic messages — the default-formatting, empty-section, empty-message, the four property invariants, and both snapshot assertions — plus the `minimal_config.exists()` setup check in the CLI config-branch test. Each message states the contract being verified so a failure explains itself. Docs: turn the code-formatted `docs/roadmap.md` reference in the LADING_CATALOGUE / Phase 5 text into a relative Markdown link, `[Phase 5 runner-migration steps](./roadmap.md)`, matching the guide's existing relative-link style. --- docs/developers-guide.md | 2 +- tests/unit/publish/test_formatting_helpers.py | 32 +++++++++++++------ tests/unit/test_cli.py | 4 ++- 3 files changed, 27 insertions(+), 11 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index c609efa5..6731ae6b 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -916,7 +916,7 @@ operator-facing description of the variable and its failure modes. `lading.utils.commands.LADING_CATALOGUE` is the staged cuprum programme catalogue (cargo, git). It is intentionally not yet wired into the execution path — the subprocess runner still spawns processes directly — and becomes live -with the Phase 5 runner-migration steps in `docs/roadmap.md`. Treat it as a +with the [Phase 5 runner-migration steps](./roadmap.md). Treat it as a registration point, not as active allowlist enforcement. #### Subprocess invocation logging diff --git a/tests/unit/publish/test_formatting_helpers.py b/tests/unit/publish/test_formatting_helpers.py index f796e82a..e0adc8ab 100644 --- a/tests/unit/publish/test_formatting_helpers.py +++ b/tests/unit/publish/test_formatting_helpers.py @@ -38,12 +38,16 @@ def test_render_section_defaults_to_string_conversion() -> None: """Default formatter handles simple string values without boilerplate.""" lines = publish_plan.render_section(("alpha", "beta"), header="Header:") - assert lines == ["Header:", "- alpha", "- beta"] + assert lines == ["Header:", "- alpha", "- beta"], ( + "default formatter should stringify each item under the header" + ) def test_render_section_omits_header_for_empty_sequences() -> None: """Helper returns no lines when there is nothing to report.""" - assert publish_plan.render_section((), header="Header:") == [] + assert publish_plan.render_section((), header="Header:") == [], ( + "an empty sequence renders no lines when no empty_message is given" + ) def test_append_section_extends_list_in_place() -> None: @@ -111,7 +115,7 @@ def test_render_section_renders_empty_message_when_empty() -> None: (), header="Header:", empty_message="Nothing to report" ) - assert lines == ["Nothing to report"] + assert lines == ["Nothing to report"], "empty_message replaces an absent section" @given( @@ -130,12 +134,18 @@ def test_render_section_invariants( ) if items: - assert lines[0] == "Header:" - assert lines[1:] == [f"- {item}" for item in items] + assert lines[0] == "Header:", "the header appears first when items exist" + assert lines[1:] == [f"- {item}" for item in items], ( + "each item renders exactly once as a bullet" + ) elif empty_message is not None: - assert lines == [empty_message] + assert lines == [empty_message], ( + "only the empty_message renders for an empty section" + ) else: - assert lines == [] + assert lines == [], ( + "no lines render for an empty section without an empty_message" + ) def _normalise_plan_message(message: str, root: Path) -> str: @@ -158,7 +168,9 @@ def test_format_plan_snapshot_with_publishable( message = publish_plan.format_plan(plan, strip_patches="all") - assert snapshot == _normalise_plan_message(message, root) + assert snapshot == _normalise_plan_message(message, root), ( + "rendered plan with publishable and skipped crates matches the snapshot" + ) def test_format_plan_snapshot_without_publishable( @@ -176,4 +188,6 @@ def test_format_plan_snapshot_without_publishable( message = publish_plan.format_plan(plan, strip_patches="per-crate") - assert snapshot == _normalise_plan_message(message, root) + assert snapshot == _normalise_plan_message(message, root), ( + "empty publish set renders the empty-state plan snapshot" + ) diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 1eb90035..75ced822 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -767,7 +767,9 @@ def test_publish_via_app_matches_across_config_branches( disk-loaded branch is only reachable publicly via ``cli.app`` invoked without an active configuration scope. """ - assert minimal_config.exists() + assert minimal_config.exists(), ( + "the minimal_config fixture must write lading.toml for the disk-loaded path" + ) workspace_graph = _make_workspace(tmp_path.resolve()) monkeypatch.setattr(cli, "load_workspace", lambda _: workspace_graph) calls: list[tuple[Path, config_module.LadingConfig, WorkspaceGraph]] = [] From 1f58ca96b2ac3616d7464891cdfc0330004c3520 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 26 Jul 2026 03:15:14 +0200 Subject: [PATCH 4/8] Add assertion messages and document plan/CLI helpers Address review comments on the publish-plan/CLI work. Tests: give the config-branch tests (`test_run_with_context_branches_ behave_identically`, `test_publish_via_app_matches_across_config_branches`) and the remaining section-helper tests diagnostic assertion messages, so a failure names the invariant it checks (result, call count, workspace root, configuration object, or workspace graph). No test logic changes. Users' guide: move the `Crates to publish: none` empty-state sentence so it introduces the empty-case example rather than the populated one. Developers' guide: document the `publish_plan` section helpers (`render_section`, `append_section`, `format_plan`) under the extracted publish modules, and add a "CLI context loading" subsection describing `_run_with_context` and its single-branch configuration resolution. --- docs/developers-guide.md | 47 ++++++++++++++++ docs/users-guide.md | 6 +- tests/unit/publish/test_formatting_helpers.py | 14 +++-- tests/unit/test_cli.py | 56 ++++++++++++++----- 4 files changed, 102 insertions(+), 21 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 6731ae6b..bf788353 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -489,6 +489,37 @@ invocations resolve to `False`, and explicit values are honoured. This keeps the command dataclass free of adapter-only state while preserving the dry-run default expected by operators. + +### CLI context loading (`_run_with_context`) + +```python +_run_with_context( + workspace_root: Path, + runner: Callable[[Path, LadingConfig, WorkspaceGraph, CommandRunner], str], + *, + command_runner: CommandRunner | None = None, +) -> str +``` + +`_run_with_context` is the single shared entry point that both the `bump` and +`publish` CLI commands call to run a command with configuration and workspace +data. It resolves configuration once: it first tries +`config.current_configuration()`, the already-installed context-var +configuration that `main()` sets up. If that raises +`config.ConfigurationNotLoadedError` — for example, when a command is invoked +programmatically via `cli.app` without a preloaded scope — it loads +configuration from disk with `config.load_configuration(workspace_root)` and +installs it under a fresh `config.use_configuration(...)` scope. When +configuration is already active, a `nullcontext()` stands in for that scope, +so the single `with` block is uniform: this was the issue #107 refactor that +removed the duplicated load-workspace-and-run block from the two branches. + +Under that scope, alongside +`metadata_module.use_command_runner(active_runner)`, `_run_with_context` loads +the workspace graph via `load_workspace(workspace_root)` and calls the +caller-supplied `runner(workspace_root, configuration, workspace_model, +active_runner)`, returning its string result. + ### Exception hierarchy (`lading.exceptions`) `lading.exceptions.LadingError` is the package-level base class for domain @@ -622,6 +653,22 @@ did not match a workspace crate. `plan_publication()` builds that object by filtering non-publishable crates, applying `publish.exclude`, validating `publish.order` when present, or deriving a deterministic dependency order. +`publish_plan.py` also owns section rendering for plan output. +`render_section(items, *, header, formatter=str, empty_message=None) -> +list[str]` is the single section renderer (issue #107): a query that returns +the formatted lines rather than mutating an accumulator. Non-empty items +render `[header, "- ", ...]`; an empty sequence renders `[]` +(the section is omitted) unless `empty_message` is supplied, in which case it +renders `[empty_message]`. `append_section(lines, items, *, header, +formatter=str) -> None` is a thin, backwards-compatible command wrapper that +delegates to `render_section` and extends `lines` in place; it has no +`empty_message` parameter and is retained as the historical public helper, +though new code may prefer `render_section`. `format_plan(plan, *, +strip_patches) -> str` composes the `lading publish` plan summary by calling +`render_section` for the publishable crates (with `empty_message="Crates to +publish: none"`) and for each skipped-crate group, then joins the resulting +lines. All three helpers are public exports of `publish_plan`. + `publish_manifest.py` owns staging-time manifest mutations. It contains workspace preparation types and helpers that copy the workspace tree and apply the `publish.strip_patches` strategy to the staged `Cargo.toml`. These diff --git a/docs/users-guide.md b/docs/users-guide.md index 9eb50af5..edaf4a22 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -158,8 +158,7 @@ remaining crates. At the end of the run, `lading publish` prints a summary of the publish plan it computed, listing the crates to publish and any crates skipped because they are marked `publish = false`, excluded via `publish.exclude`, or named -in `publish.exclude` but absent from the workspace. When no crates are -publishable, the summary reports `Crates to publish: none` instead of a list: +in `publish.exclude` but absent from the workspace: ```text Publish plan for @@ -175,6 +174,9 @@ Configured exclusions not found in workspace: - missing ``` +When no crates are publishable, the summary reports `Crates to publish: none` +instead of a list: + ```text Publish plan for Strip patch strategy: per-crate diff --git a/tests/unit/publish/test_formatting_helpers.py b/tests/unit/publish/test_formatting_helpers.py index e0adc8ab..c6bca89a 100644 --- a/tests/unit/publish/test_formatting_helpers.py +++ b/tests/unit/publish/test_formatting_helpers.py @@ -31,7 +31,9 @@ def __init__(self, value: str) -> None: formatter=lambda item: item.value.upper(), ) - assert lines == ["Header:", "- ALPHA", "- BETA"] + assert lines == ["Header:", "- ALPHA", "- BETA"], ( + "the formatter should be applied to each item under the header" + ) def test_render_section_defaults_to_string_conversion() -> None: @@ -56,7 +58,9 @@ def test_append_section_extends_list_in_place() -> None: publish_plan.append_section(lines, ("alpha", "beta"), header="Header:") - assert lines == ["preamble", "Header:", "- alpha", "- beta"] + assert lines == ["preamble", "Header:", "- alpha", "- beta"], ( + "append_section should append the header and formatted items in place" + ) def test_append_section_appends_nothing_when_empty() -> None: @@ -65,7 +69,9 @@ def test_append_section_appends_nothing_when_empty() -> None: publish_plan.append_section(lines, (), header="Header:") - assert lines == ["preamble"] + assert lines == ["preamble"], ( + "append_section should leave lines unchanged for an empty section" + ) def test_append_section_matches_render_section() -> None: @@ -81,7 +87,7 @@ def test_append_section_matches_render_section() -> None: assert lines == publish_plan.render_section( ("alpha", "beta"), header="Header:", formatter=str.upper - ) + ), "append_section should produce the same lines as render_section" def test_format_plan_formats_skipped_sections(tmp_path: Path) -> None: diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 75ced822..ff08640f 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -743,14 +743,26 @@ def runner( with config_module.use_configuration(configuration): preloaded_result = cli._run_with_context(tmp_path.resolve(), runner) - assert fresh_result == preloaded_result == "ran" - assert len(calls) == 2 + assert fresh_result == preloaded_result == "ran", ( + "both branches must return the runner's result" + ) + assert len(calls) == 2, "the runner should execute exactly once per branch" fresh_call, preloaded_call = calls - assert fresh_call[0] == preloaded_call[0] == tmp_path.resolve() - assert isinstance(fresh_call[1], config_module.LadingConfig) - assert preloaded_call[1] is configuration - assert fresh_call[2] is workspace_graph - assert preloaded_call[2] is workspace_graph + assert fresh_call[0] == preloaded_call[0] == tmp_path.resolve(), ( + "both branches must pass the resolved workspace root" + ) + assert isinstance(fresh_call[1], config_module.LadingConfig), ( + "the fresh branch must load a LadingConfig from disk" + ) + assert preloaded_call[1] is configuration, ( + "the preloaded branch must reuse the active configuration object" + ) + assert fresh_call[2] is workspace_graph, ( + "the fresh branch must pass the loaded workspace graph" + ) + assert preloaded_call[2] is workspace_graph, ( + "the preloaded branch must pass the loaded workspace graph" + ) def test_publish_via_app_matches_across_config_branches( @@ -781,7 +793,9 @@ def fake_run( *, options: publish_command.PublishOptions, ) -> str: - assert isinstance(options, publish_command.PublishOptions) + assert isinstance(options, publish_command.PublishOptions), ( + "publish.run must receive a PublishOptions instance" + ) calls.append((root, configuration, workspace)) return "published" @@ -798,15 +812,27 @@ def fake_run( with config_module.use_configuration(preloaded): preloaded_result = cli.app(args) - assert disk_result == preloaded_result == "published" - assert len(calls) == 2 + assert disk_result == preloaded_result == "published", ( + "both config branches must return the same command result" + ) + assert len(calls) == 2, "publish.run should execute exactly once per branch" disk_call, preloaded_call = calls # Identical downstream behaviour: same workspace root, same injected # workspace graph, and equal configuration content for both branches. - assert disk_call[0] == preloaded_call[0] == tmp_path.resolve() - assert disk_call[2] is preloaded_call[2] is workspace_graph - assert disk_call[1] == preloaded_call[1] + assert disk_call[0] == preloaded_call[0] == tmp_path.resolve(), ( + "both branches must pass the resolved workspace root" + ) + assert disk_call[2] is preloaded_call[2] is workspace_graph, ( + "both branches must pass the injected workspace graph" + ) + assert disk_call[1] == preloaded_call[1], ( + "both branches must resolve equal configuration content" + ) # The branches differ only in provenance: the disk branch reloads a fresh # config object; the pre-loaded branch reuses the active one. - assert preloaded_call[1] is preloaded - assert disk_call[1] is not preloaded + assert preloaded_call[1] is preloaded, ( + "the preloaded branch must reuse the active configuration object" + ) + assert disk_call[1] is not preloaded, ( + "the disk branch must reload a fresh configuration object" + ) From bf456eb668fafdfc36fe4f95c5ac0e624cdf4c72 Mon Sep 17 00:00:00 2001 From: leynos Date: Sun, 26 Jul 2026 18:19:44 +0200 Subject: [PATCH 5/8] Adopt plaintext fences and tidy blank line Post-rebase reconciliation with main's #176 (plaintext prose fences): convert the two publish-plan example fences in the users' guide from `text` to `plaintext` to match the repository convention, and drop a duplicate blank line that the entity merge left before the new "CLI context loading" subsection in the developers' guide (MD012). --- docs/developers-guide.md | 1 - docs/users-guide.md | 4 ++-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index bf788353..45308f77 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -489,7 +489,6 @@ invocations resolve to `False`, and explicit values are honoured. This keeps the command dataclass free of adapter-only state while preserving the dry-run default expected by operators. - ### CLI context loading (`_run_with_context`) ```python diff --git a/docs/users-guide.md b/docs/users-guide.md index edaf4a22..29bb322c 100644 --- a/docs/users-guide.md +++ b/docs/users-guide.md @@ -160,7 +160,7 @@ it computed, listing the crates to publish and any crates skipped because they are marked `publish = false`, excluded via `publish.exclude`, or named in `publish.exclude` but absent from the workspace: -```text +```plaintext Publish plan for Strip patch strategy: all Crates to publish (2): @@ -177,7 +177,7 @@ Configured exclusions not found in workspace: When no crates are publishable, the summary reports `Crates to publish: none` instead of a list: -```text +```plaintext Publish plan for Strip patch strategy: per-crate Crates to publish: none From 5fc09165eb6fedd97e73b099e414388c3f39ad6b Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Jul 2026 16:35:43 +0200 Subject: [PATCH 6/8] Drop duplicate format_plan skipped-sections test `test_format_plan_formats_skipped_sections` in test_publish_formatting.py was a behavioural duplicate of the canonical test in tests/unit/publish/test_formatting_helpers.py (identical assertions on `format_plan` skipped-section rendering; only the crate-construction plumbing differed). Remove it and the imports it alone used (`collections.abc`, `publish_plan`, `_CrateSpec`, `WorkspaceCrate`); the file keeps its unique `test_format_preparation_summary` coverage. --- tests/unit/test_publish_formatting.py | 34 +-------------------------- 1 file changed, 1 insertion(+), 33 deletions(-) diff --git a/tests/unit/test_publish_formatting.py b/tests/unit/test_publish_formatting.py index ef454fbb..a40e891e 100644 --- a/tests/unit/test_publish_formatting.py +++ b/tests/unit/test_publish_formatting.py @@ -2,45 +2,13 @@ from __future__ import annotations -import collections.abc as cabc import typing as typ -from lading.commands import publish, publish_plan -from tests.unit.conftest import _CrateSpec +from lading.commands import publish if typ.TYPE_CHECKING: from pathlib import Path - from lading.workspace import WorkspaceCrate - - -def test_format_plan_formats_skipped_sections( - tmp_path: Path, - make_crate: cabc.Callable[[Path, str, _CrateSpec | None], WorkspaceCrate], -) -> None: - """``format_plan`` renders skipped crates using their names only.""" - root = tmp_path.resolve() - manifest_skipped = make_crate(root, "beta", _CrateSpec(publish=False)) - config_skipped = make_crate(root, "gamma") - plan = publish_plan.PublishPlan( - workspace_root=root, - publishable=(), - skipped_manifest=(manifest_skipped,), - skipped_configuration=(config_skipped,), - missing_configuration_exclusions=("missing",), - ) - - message = publish_plan.format_plan(plan, strip_patches="all") - - lines = message.splitlines() - manifest_index = lines.index("Skipped (publish = false):") - configuration_index = lines.index("Skipped via publish.exclude:") - missing_index = lines.index("Configured exclusions not found in workspace:") - - assert lines[manifest_index + 1] == "- beta" - assert lines[configuration_index + 1] == "- gamma" - assert lines[missing_index + 1] == "- missing" - def test_format_preparation_summary_reports_bump_readme_handling( tmp_path: Path, From ede85f94c435a19a855d8e29aac2c40fab1a81e2 Mon Sep 17 00:00:00 2001 From: leynos Date: Mon, 27 Jul 2026 16:44:02 +0200 Subject: [PATCH 7/8] Correct LADING_CATALOGUE migration reference to Phase 5.2 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catalogue-wiring migration target was misrouted to the low-level `subprocess_runner.py` (and cited a roadmap step that does not exist). The catalogue's `scoped(allowlist=…)` enforcement is wired in by roadmap Phase 5.2 "Publish Execution Migration", which rewires `publish_execution.py`'s command execution (the roadmap's `_invoke_via_subprocess()` step; `_invoke` today) onto cuprum. Point the `utils/commands.py` module docstring and the developers' guide note at that step, keeping the "staged, not yet wired" caveat. --- docs/developers-guide.md | 10 +++++++--- lading/utils/commands.py | 10 ++++++---- 2 files changed, 13 insertions(+), 7 deletions(-) diff --git a/docs/developers-guide.md b/docs/developers-guide.md index 45308f77..e09eab12 100644 --- a/docs/developers-guide.md +++ b/docs/developers-guide.md @@ -961,9 +961,13 @@ operator-facing description of the variable and its failure modes. `lading.utils.commands.LADING_CATALOGUE` is the staged cuprum programme catalogue (cargo, git). It is intentionally not yet wired into the execution -path — the subprocess runner still spawns processes directly — and becomes live -with the [Phase 5 runner-migration steps](./roadmap.md). Treat it as a -registration point, not as active allowlist enforcement. +path — `publish_execution._invoke` still delegates to the subprocess runner, +which spawns processes directly. It becomes live with the [Phase 5.2 +publish-execution migration](./roadmap.md), which rewires +`publish_execution.py` command execution (the roadmap's +`_invoke_via_subprocess()` step) onto the catalogue's +`scoped(allowlist=…)` model. Treat it as a registration point, not as +active allowlist enforcement. #### Subprocess invocation logging diff --git a/lading/utils/commands.py b/lading/utils/commands.py index d38f3af5..b725b262 100644 --- a/lading/utils/commands.py +++ b/lading/utils/commands.py @@ -10,10 +10,12 @@ catalogue via ``scoped(allowlist=LADING_CATALOGUE.allowlist)``. The catalogue is staged but intentionally not yet wired into the execution -path: the subprocess runner still spawns processes directly. The wiring -lands with the runner migration steps in ``docs/roadmap.md`` (Phase 5, -"Migrate `lading/runtime/subprocess_runner.py`" onwards); do not mistake -this module for live enforcement in the meantime. +path: ``publish_execution._invoke`` still delegates to the subprocess runner, +which spawns processes directly. The wiring lands with the Phase 5.2 +publish-execution migration in ``docs/roadmap.md`` -- the +``_invoke_via_subprocess()`` step that rewires ``publish_execution.py`` +command execution onto ``scoped(allowlist=LADING_CATALOGUE.allowlist)``; do +not mistake this module for live enforcement in the meantime. """ from __future__ import annotations From f341c58780004f17a2cb2e436f415493e11f469f Mon Sep 17 00:00:00 2001 From: leynos Date: Wed, 29 Jul 2026 13:16:26 +0200 Subject: [PATCH 8/8] Expand the publish_plan module docstring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The module summarized itself in one line, leaving its purpose and its place in the publish flow implicit. Describe the two responsibilities it backs — resolving a `PublishPlan` and rendering that plan for operators — and record the call sites: `publish.run` builds the plan before staging and formats it at the end, and `lading.cli` prints that return value verbatim (so the text is user-facing output, though `cli.py` never imports this module). Note that syrupy snapshots pin the rendered summary, so output changes must update them and the users' guide. --- lading/commands/publish_plan.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/lading/commands/publish_plan.py b/lading/commands/publish_plan.py index bff3fc22..c5a6ad3f 100644 --- a/lading/commands/publish_plan.py +++ b/lading/commands/publish_plan.py @@ -1,4 +1,32 @@ -"""Publication planning and formatting helpers.""" +"""Publication planning and formatting helpers. + +Summary +------- +Backs ``lading publish`` with two responsibilities: deciding *what* to publish +and rendering *that decision* for operators. :func:`plan_publication` resolves +a workspace graph and the ``publish`` configuration into an immutable +:class:`PublishPlan` — the publication order plus each skipped-crate group — +raising :class:`PublishPlanError` when no valid plan exists. +:func:`format_plan` renders that plan as the plain-text summary operators +read, composing every section through :func:`render_section`. + +Functions / Call sites +---------------------- +* :func:`plan_publication` — called by ``publish.run`` before staging, to fix + the publication order and the skipped-crate groups. +* :func:`format_plan` — called at the end of ``publish.run``; ``lading.cli`` + prints that return value verbatim, so this text is user-facing output. + ``cli.py`` never imports this module directly. +* :func:`render_section` / :func:`append_section` — the single section + renderer (a query returning lines) and its in-place wrapper. +* :class:`PublishPlan` — a typing-only dependency of ``publish_manifest`` + and ``publish_index_check``. + +The rendered summary is pinned by syrupy snapshots in +``tests/unit/publish/__snapshots__/test_formatting_helpers.ambr``, so changing +a header, a bullet, or the ``Crates to publish: none`` empty state updates +those snapshots and the publish-plan section of the users' guide. +""" from __future__ import annotations