Deduplicate publish plan helpers

[leynos]

Summary

• add a reusable helper for formatting publish plan sections
• consolidate skipped and missing item formatting on the new helper

Testing

• make check-fmt
• make typecheck
• make lint
• make test

---

https://chatgpt.com/codex/tasks/task_e_68fd82856a2c83228b1884d1ab19178c

Summary by Sourcery

Introduce a generic section formatter for publish plans and refactor existing helpers to use it, reducing duplication.

New Features:

• Add a reusable generic helper (_format_items_section) for formatting publish plan sections

Enhancements:

• Remove specialized skipped and names formatting functions and consolidate logic into the new helper
• Update publish plan output to use the generic helper with item-specific formatters

Summary by CodeRabbit

• Refactor

  • Consolidated duplicate formatting helpers into a single generic formatter while preserving publish output and visible behavior.

• Tests

  • Added unit and BDD tests to verify formatting behavior and that the publish output omits the "Configured exclusions not found in workspace:" section when empty.

• Chores

  • Added linter configuration updates.

[coderabbitai]

Caution

Review failed

The pull request is closed.

Walkthrough

Replaces two specialized formatting helpers with a single generic _append_section[T] that formats sequences via an optional formatter; updates _format_plan to use it and adjusts tests and BDD expectations to reflect the unified behavior.

Changes

Cohort / File(s)	Summary
Publish command refactor lading/commands/publish.py	Adds generic helper _append_section[T](lines: list[str], items: typ.Sequence[T], *, header: str, formatter: typ.Callable[[T], str] = str) -> None and removes _format_skipped_section and _format_names_section. Updates _format_plan to call _append_section for skipped_manifest, skipped_configuration, and missing_configuration_exclusions, supplying appropriate formatter callables (e.g., lambda crate: crate.name).
BDD feature update tests/bdd/features/cli.feature	Updates scenario expectation to assert the publish output omits the header Configured exclusions not found in workspace: for the relevant case.
BDD step addition tests/bdd/steps/test_publish_steps.py	Adds then_publish_omits_section(cli_run: dict[str, typ.Any], header: str) which parses CLI output and asserts the specified header is absent from the publish plan.
Unit tests for helper tests/unit/test_publish_command.py	Adds tests for _append_section: verifies custom formatter usage, default str conversion, omission for empty sequences, and integration checks ensuring _format_plan lists skipped manifests/configurations and missing exclusions correctly.
Linter config pyproject.toml	Adds ruff lint configuration: mccabe.max-complexity = 9 and flake8-import-conventions banned/alias mappings.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review attention:
  • lading/commands/publish.py: confirm generic type signature, default formatter, and that header lines are emitted only when items is non-empty.
  • _format_plan call sites: ensure supplied formatter lambdas produce identical visible output (e.g., crate.name) and spacing/newline behavior matches previous helpers.
  • Tests/BDD: validate parsing logic in then_publish_omits_section and unit tests cover empty and non-empty cases.

Poem

🐰 I hopped through lists both short and long,
Folded helpers into one neat song.
Headers appear only when things exist,
Names align neatly — I gave a twitchy twist.
I munch on changes, then thump with glee.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The PR title "Deduplicate publish plan helpers" directly reflects the primary change in the changeset. The raw summary shows that two specialized formatting helpers (_format_skipped_section and _format_names_section) are replaced with a single generic _append_section[T] helper, consolidating duplicated formatting logic. The title clearly and concisely communicates this deduplication objective, which aligns with the PR's stated goal. The title is specific enough that a teammate reviewing the history would immediately understand the core change without being vague or overly broad.
Docstring Coverage	✅ Passed	Docstring coverage is 90.91% which is sufficient. The required threshold is 80.00%.

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 403e62d and cae4ece.

📒 Files selected for processing (2)

• pyproject.toml (1 hunks)
• tests/unit/test_publish_command.py (1 hunks)

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3a10cf6 and 077b65e.

📒 Files selected for processing (1)

• lading/commands/publish.py (3 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.py

📄 CodeRabbit inference engine (.rules/python-00.md)

**/*.py: Python files must use snake_case filenames (e.g., http_client.py, task_queue.py)
Classes must use PascalCase
Variables and functions must use snake_case
Module-level constants must use UPPER_SNAKE_CASE
Prefix non-exported helpers or internal APIs with a single leading underscore
Use typing everywhere and maintain full static type coverage
Use TypedDict or @DataClass for structured data; prefer @DataClass(slots=True) for internal-only usage
Avoid Any; prefer precise types (TypeVar, Protocol, Literal, Union); use typing.cast only when necessary with justification; use object for unknown opaque values
Be explicit with return types (e.g., -> None, -> str) for all public functions and methods
Favor immutability: prefer tuples to lists and MappingProxyType for read-only mappings; document third-party frozendict if used
Use # pyright: ignore sparingly and include an explanation comment when used
Avoid side effects at import time (no global state mutation or actions on import)
Use docstrings (NumPy format) for public functions, classes, and modules
Explain tricky code with inline comments

**/*.py: Prefer context managers to encapsulate setup/teardown for resources (files, locks, connections) instead of manual try/finally blocks
Use contextlib.contextmanager (@contextmanager) to implement simple, linear setup/teardown context managers
Implement a class with enter and exit for context managers that require internal state or more complex lifecycle handling
For file I/O, prefer with open(...) as ... over open()/try/finally/close patterns
Choose @contextmanager when control flow is linear and no persistent state is needed
Choose a class-based context manager when there is internal state, lifecycle methods, re-entry, or advanced context needs

**/*.py: Exception classes must end with the suffix 'Error' (N818)
Prefer specific built-ins (e.g., TypeError, ValueError) or domain exceptions over raising Exception directly (TRY003/TRY004)
Preserve causal chains...

Files:

• lading/commands/publish.py

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Sourcery review

🔇 Additional comments (2)

lading/commands/publish.py (2)

129-152: LGTM! Clean generic implementation.

The implementation correctly generalizes the formatting logic and achieves the PR objective of reducing code duplication. The function signature is well-typed, the docstring is complete, and the logic is straightforward.

---

170-187: LGTM! Consistent formatter usage.

All three calls to _format_items_section correctly apply the generic helper with appropriate formatters. The identity lambda on line 186 maintains consistency with the formatter interface pattern.

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 077b65e and 403e62d.

📒 Files selected for processing (4)

• lading/commands/publish.py (2 hunks)
• tests/bdd/features/cli.feature (1 hunks)
• tests/bdd/steps/test_publish_steps.py (1 hunks)
• tests/unit/test_publish_command.py (1 hunks)

✅ Files skipped from review due to trivial changes (1)

• tests/bdd/features/cli.feature

🚧 Files skipped from review as they are similar to previous changes (1)

• lading/commands/publish.py

🧰 Additional context used

📓 Path-based instructions (3)

**/*.py

📄 CodeRabbit inference engine (.rules/python-00.md)

**/*.py: Python files must use snake_case filenames (e.g., http_client.py, task_queue.py)
Classes must use PascalCase
Variables and functions must use snake_case
Module-level constants must use UPPER_SNAKE_CASE
Prefix non-exported helpers or internal APIs with a single leading underscore
Use typing everywhere and maintain full static type coverage
Use TypedDict or @DataClass for structured data; prefer @DataClass(slots=True) for internal-only usage
Avoid Any; prefer precise types (TypeVar, Protocol, Literal, Union); use typing.cast only when necessary with justification; use object for unknown opaque values
Be explicit with return types (e.g., -> None, -> str) for all public functions and methods
Favor immutability: prefer tuples to lists and MappingProxyType for read-only mappings; document third-party frozendict if used
Use # pyright: ignore sparingly and include an explanation comment when used
Avoid side effects at import time (no global state mutation or actions on import)
Use docstrings (NumPy format) for public functions, classes, and modules
Explain tricky code with inline comments

**/*.py: Prefer context managers to encapsulate setup/teardown for resources (files, locks, connections) instead of manual try/finally blocks
Use contextlib.contextmanager (@contextmanager) to implement simple, linear setup/teardown context managers
Implement a class with enter and exit for context managers that require internal state or more complex lifecycle handling
For file I/O, prefer with open(...) as ... over open()/try/finally/close patterns
Choose @contextmanager when control flow is linear and no persistent state is needed
Choose a class-based context manager when there is internal state, lifecycle methods, re-entry, or advanced context needs

**/*.py: Exception classes must end with the suffix 'Error' (N818)
Prefer specific built-ins (e.g., TypeError, ValueError) or domain exceptions over raising Exception directly (TRY003/TRY004)
Preserve causal chains...

Files:

• tests/bdd/steps/test_publish_steps.py
• tests/unit/test_publish_command.py

{**/unittests/test_*.py,tests/**/*.py}

📄 CodeRabbit inference engine (.rules/python-00.md)

{**/unittests/test_*.py,tests/**/*.py}: Use pytest idioms: prefer fixtures over setup/teardown, parametrize broadly, avoid unnecessary mocks
Group related tests using classes with method names prefixed by test_
Write tests from a user's perspective: test public behaviour, not internals

Files:

• tests/bdd/steps/test_publish_steps.py
• tests/unit/test_publish_command.py

tests/**/*.py

📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)

In tests, assert specific exception types (and optional message via regex) rather than using broad Exception (B017)

Files:

• tests/bdd/steps/test_publish_steps.py
• tests/unit/test_publish_command.py

🧬 Code graph analysis (1)

tests/unit/test_publish_command.py (1)

lading/commands/publish.py (3)

• _append_section (127-151)
• PublishPlan (19-31)
• _format_plan (154-187)

🪛 GitHub Actions: CI

tests/unit/test_publish_command.py

[error] 1-1: Ruff formatting check failed. 1 file would be reformatted by 'ruff format --check'.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Sourcery review

🔇 Additional comments (5)

tests/bdd/steps/test_publish_steps.py (1)

112-116: LGTM! Clean test step implementation.

The new BDD step correctly validates that a section header is absent from the publish plan output. It properly reuses the existing _publish_plan_lines helper and follows pytest-bdd conventions.

tests/unit/test_publish_command.py (4)

351-367: LGTM! Well-structured test with custom formatter.

This test correctly validates that _append_section applies the provided formatter to each item. The inner Dummy class is appropriate for testing purposes, and the assertion comprehensively checks both the header and formatted item output.

---

370-376: LGTM! Clean test of default behavior.

This test correctly validates that _append_section defaults to string conversion when no formatter is provided, ensuring ergonomic usage with simple string sequences.

---

379-385: LGTM! Good edge case coverage.

This test correctly validates that _append_section leaves the lines buffer unchanged when given an empty sequence, ensuring clean output without unnecessary headers.

---

388-410: LGTM! Comprehensive integration test.

This test thoroughly validates the _format_plan output by verifying that all skipped sections (manifest, configuration, and missing exclusions) are correctly formatted with appropriate headers and name-only formatting for crate items.

[leynos]

@coderabbitai Have the following now been resolved?

• Consider using a more generic type for items (e.g. Sequence[T]) instead of tuple[T, ...] so the helper accepts any iterable.
• You could give formatter a default of str to avoid passing lambda x: x for simple string cases.
• Perhaps rename _format_items_section to something like _format_section or _append_section to better reflect its generic role.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the implementation of _append_section in the PR
ast-grep --pattern $'def _append_section($$$):
  $$$'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Read the publish.py file to check the _append_section implementation
cat lading/commands/publish.py | grep -A 10 "def _append_section"

Length of output: 259

---

@leynos Yes, all three suggestions have been resolved! ✅

1. Generic type for items: Now uses typ.Sequence[T] instead of tuple[T, ...] ✓
2. Default formatter: The formatter parameter now has a default value of str, eliminating the need for lambda x: x in simple cases ✓
3. Function naming: The function has been renamed to _append_section ✓

The final signature is:

def _append_section[T](
    lines: list[str],
    items: typ.Sequence[T],
    *,
    header: str,
    formatter: typ.Callable[[T], str] = str,
) -> None:

This is a nice, clean, and flexible implementation that addresses all the feedback perfectly!