Skip to content

Fix mixed Google and NumPy docstrings (#162) - #199

Merged
leynos merged 31 commits into
mainfrom
issue-162-fix-mixed-google-numpy-docstring-style-across-bump-py-bump-toml-py-config-py
Aug 2, 2026
Merged

Fix mixed Google and NumPy docstrings (#162)#199
leynos merged 31 commits into
mainfrom
issue-162-fix-mixed-google-numpy-docstring-style-across-bump-py-bump-toml-py-config-py

Conversation

@lodyai

@lodyai lodyai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This branch standardizes the affected docstrings on the repository's NumPy
conventions so that private helpers retain concise summaries and public
interfaces provide structured parameter and return documentation.

It also removes the remaining mixed Google/NumPy docstring found by the
whole-tree audit while preserving the deliberately pure-Google helper outwith
this issue's scope.

Closes #162.

Review walkthrough

Validation

  • make check-fmt: passed after every milestone
  • make typecheck: passed after every milestone
  • make lint: passed after every milestone
  • make test: 739 passed after every milestone
  • Whole-tree Args: audit: no mixed Google/NumPy docstrings remain
  • git diff --check origin/main...HEAD: passed
  • coderabbit review --agent: zero findings after each of three milestones

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @LodyAI[bot], you have reached your weekly rate limit of 500000 diff characters.

Please try again later or upgrade to continue using Sourcery

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Summary

  • Standardize repository docstrings on NumPy conventions.
  • Use concise one-line summaries for private helpers.
  • Document public interfaces with Parameters, Returns, Raises, and Examples sections.
  • Audit the repository and remove the remaining mixed Google/NumPy docstring.
  • Enable Ruff DOC checks and configure targeted pydoclint suppressions.
  • Add _BumpCliRun typing and centralize BDD bump command execution.
  • Remove redundant publish fixture aliases and update affected tests.
  • Extract fixture crate creation from create_nontrivial_workspace.
  • Document counter_value behaviour for unset counters returning 0.
  • Clarify lockfile discovery and regeneration error handling.

Validation

  • Pass formatting, type checking, linting, 739 tests, whole-tree Args: audit, diff checks, and CodeRabbit review.
  • Address issue #162.

Walkthrough

The pull request standardises NumPy-style documentation, enables documentation linting, updates selected type annotations, clarifies lockfile errors, and consolidates selected BDD, E2E, and unit-test helpers.

Changes

Documentation alignment

Layer / File(s) Summary
Production documentation and contracts
lading/cli.py, lading/commands/*, lading/config.py, lading/runtime/*, lading/toml_coerce/*, lading/utils/*, lading/workspace/*
Public interfaces now use structured NumPy-style documentation. Private helpers use concise summaries. Examples, return values, errors, and edge cases are documented.
Documentation lint configuration
pyproject.toml
Ruff DOC rules and pydoclint settings are enabled. Targeted suppressions cover delegated and factory-produced errors.
Test support and fixture contracts
tests/bdd/*, tests/e2e/*, tests/helpers/*, tests/conftest.py, tests/e2e/conftest.py, tests/unit/publish/conftest.py
Fixtures and helpers receive structured documentation. Shared bump, publish, workspace, and invocation helpers are introduced or reused.
Unit-test fixture consolidation
tests/unit/conftest.py, tests/unit/test_publish_planning.py, tests/unit/test_publish_staging.py
Planning and preparation fixture aliases are removed. Publish tests use PublishFixtures.
Behavioural documentation updates
lading/commands/bump_lockfile_regeneration.py, docs/developers-guide.md, docs/users-guide.md
Lockfile failure documentation now distinguishes one attempted manifest from multiple attempted manifests.
Minor runtime and helper changes
lading/runtime/subprocess_runner.py, lading/utils/metrics.py, tests/e2e/helpers/workspace_builder.py, tests/bdd/steps/test_publish_helpers.py
Environment mappings accept object values, unset counters return 0, and selected test helpers centralise command and fixture handling.

Possibly related PRs

Suggested labels: Issue

Suggested reviewers: leynos, codescene-access

Poem

Docstrings align,
Ruff checks each contract in line,
Fixtures share one frame,
Lockfile errors name the same,
Clear APIs remain.


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (3 warnings, 2 inconclusive)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR includes unrelated behaviour and test changes, including counter defaults, environment type widening, and fixture/helper refactors. Remove unrelated functional and test refactors, or link separate issues that define their scope.
Developer Documentation ⚠️ Warning The PR changes counter_value and subprocess environment types, and adds Ruff DOC/pydoclint policy, but the developer guide documents none of these; its only change covers lockfile wording. Document the changed metrics and subprocess contracts, plus the NumPy/DOC/pydoclint policy and exceptions, in docs/developers-guide.md.
Testing (Property / Proof) ⚠️ Warning counter_value now guarantees 0 for every absent metric key, but the unchanged Hypothesis suite does not test this range-based invariant. Add a Hypothesis property that generates arbitrary metric names and label mappings, queries absent keys, and asserts a zero result without mutating snapshot().
Security And Privacy ❓ Inconclusive Investigation is still in progress. Continue reviewing the changed source and test files for security or privacy impact.
Concurrency And State ❓ Inconclusive Investigation is still in progress; no verdict submitted yet. Gather the remaining evidence before deciding.
✅ Passed checks (15 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the docstring-standardisation changes and links issue #162.
Description check ✅ Passed The description explains the docstring standardisation, audit, affected scope, and validation results.
Linked Issues check ✅ Passed The changes satisfy issue #162 by standardising private and public docstrings and auditing for mixed Google/NumPy style.
Docstring Coverage ✅ Passed Docstring coverage is 97.15% which is sufficient. The required threshold is 80.00%.
Testing (Overall) ✅ Passed Existing tests directly assert unset counters return 0 and zero increments remain absent; environment stringification is also asserted. Other changes are documentation or equivalent test-helper ref...
User-Facing Documentation ✅ Passed Pass: the patch makes no CLI workflow change; the only runtime change is internal metrics test-seam behaviour, and docs/users-guide.md documents the lockfile failure wording.
Module-Level Documentation ✅ Passed Token audit found module docstrings in all 157 Python modules; production and test headers state each module’s purpose and relevant relationship to CLI, workflows, helpers, or tests.
Testing (Unit And Behavioural) ✅ Passed Pass this check: existing unit tests cover unset counters and non-string environment values, while BDD/E2E scenarios invoke the CLI through subprocess boundaries; no new workflow needs tests.
Testing (Compile-Time / Ui) ✅ Passed No Rust/TypeScript compile-time change or UI output change is present; Python runtime edits have unit coverage, and no snapshot asset changed.
Unit Architecture ✅ Passed Diff review found no new hidden dependencies or side-effects: production changes are docstrings, env typing, and read-only counter default; test helpers delegate through explicit runners and record...
Domain Architecture ✅ Passed Keep this change: production code adds no new domain-to-infrastructure dependency; subprocess values are translated at the runtime boundary, while other executable changes are equivalent loading an...
Observability ✅ Passed The patch adds documentation and type annotations only; the Counter access remains 0 for missing keys, environment stringification is unchanged, and no new operational path needs logging, metrics,...
Performance And Resource Use ✅ Passed Pass the check: diff inspection found documentation changes only, plus O(1) metric lookup, type widening, and equivalent helper refactors; no new unbounded loops, I/O, blocking work, or caches.
Architectural Complexity And Maintainability ✅ Passed The change is documentation-led. New local test helpers consolidate repeated CLI, invocation, and fixture setup; no production layers, dependencies, registries, or architectural boundaries were added.
Rust Compiler Lint Integrity ✅ Passed The PR changes only Python, Markdown, and pyproject.toml files; no Rust source or Cargo metadata changed, and no Rust lint suppression or clone was introduced.
📋 Issue Planner

Let us write the prompt for your AI agent so you can ship faster (with fewer bugs).

View plan for ticket: #162

✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issue-162-fix-mixed-google-numpy-docstring-style-across-bump-py-bump-toml-py-config-py

Comment @coderabbitai help to get the list of available commands.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review July 22, 2026 13:20
@coderabbitai coderabbitai Bot added the Issue label Jul 22, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lading/commands/bump_toml.py`:
- Around line 133-148: Add concise, runnable Examples sections to the public
docstrings for update_section in lading/commands/bump_toml.py (lines 133-148),
update_dependency_sections in lading/commands/bump_toml.py (lines 165-180), and
make_preflight_config in tests/unit/publish/conftest.py (lines 80-89). Each
example must show representative inputs and the resulting bool or configuration,
including a dependency override for make_preflight_config.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 15432a03-d19a-4cf1-92a0-4ad44de9b68f

📥 Commits

Reviewing files that changed from the base of the PR and between 5e5be63 and f47357d.

📒 Files selected for processing (4)
  • lading/commands/bump_manifests.py
  • lading/commands/bump_toml.py
  • lading/config.py
  • tests/unit/publish/conftest.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/cmd-mox (auto-detected)
  • leynos/cuprum (auto-detected)
  • leynos/shared-actions (auto-detected)

Comment thread lading/commands/bump_toml.py
codescene-access[bot]

This comment was marked as outdated.

@lodyai
lodyai Bot force-pushed the issue-162-fix-mixed-google-numpy-docstring-style-across-bump-py-bump-toml-py-config-py branch from 46e0660 to bbf56ae Compare July 24, 2026 02:06
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@lodyai
lodyai Bot force-pushed the issue-162-fix-mixed-google-numpy-docstring-style-across-bump-py-bump-toml-py-config-py branch 2 times, most recently from bb7d284 to 01a3bca Compare July 26, 2026 16:20
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

Code Duplication

lading/commands/bump_lockfiles.py:

What lead to degradation?

The module contains 2 functions with similar structure: CargoLockfileRepository.regenerate_lockfiles,CargoLockfileRepository.resolve_lockfile_paths

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

Code Duplication

tests/bdd/steps/test_bump_steps.py:

What lead to degradation?

The module contains 2 functions with similar structure: when_invoke_lading_bump,when_invoke_lading_bump_dry_run

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@coderabbitai

This comment was marked as resolved.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

Code Duplication

tests/bdd/steps/test_publish_helpers.py:

What lead to degradation?

The module contains 4 functions with similar structure: _get_package_invocations,_get_publish_invocations,_get_test_invocation_envs,_get_test_invocations

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@coderabbitai

This comment was marked as resolved.

@leynos

leynos commented Jul 26, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

Code Duplication

tests/e2e/steps/test_e2e_steps.py:

What lead to degradation?

The module contains 3 functions with similar structure: when_run_lading_bump,when_run_lading_publish,when_run_lading_publish_allow_dirty

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

coderabbitai[bot]

This comment was marked as resolved.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- target module outline ---'
ast-grep outline lading/workspace/metadata.py --items all --view expanded

printf '%s\n' '--- error factories and parser context ---'
rg -n -A150 -B18 \
  '^class CargoMetadataParseError\b|^\s+def (invalid_json|non_object_payload)\b|^def (_parse_cargo_metadata|load_cargo_metadata)\b' \
  lading/workspace/metadata.py

printf '%s\n' '--- references and assertions ---'
rg -n -C4 \
  '\bCargoMetadataParseError\.(invalid_json|non_object_payload)\s*\(|cargo metadata produced invalid JSON output|cargo metadata produced a non-object JSON payload' \
  lading tests -g '*.py' || true

printf '%s\n' '--- focused test files ---'
fd -t f -e py . tests | rg 'metadata|workspace' | sort

printf '%s\n' '--- current target-file diff ---'
git diff --no-ext-diff --unified=12 -- lading/workspace/metadata.py

Length of output: 980


@leynos lading/workspace/metadata.pyCargoMetadataParseError.invalid_json, CargoMetadataParseError.non_object_payload

Do not apply a fix or suppress the CodeScene diagnostic yet. Current-codegraph verification failed because repository setup failed before the inspection commands ran. I could not verify the current implementations, parser call sites, tests, or PR diff.

Retry the request after repository access is available. Keep this finding unresolved until that inspection completes.

You are interacting with an AI system.

leynos and others added 2 commits August 2, 2026 09:54
_raise_aggregated_failure branches on len(manifests) == 1, so describing the
re-raise path in terms of how many lockfiles were regenerated is too narrow --
and doubly wrong, since a failed manifest produces no regenerated lockfile at
all. Every manifest is attempted, so the distinction is the attempt count.

The regenerate_lockfiles docstring was corrected in 5af1852; this applies the
same correction to the three surviving prose copies in the user and developer
guides.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
9fe366b widened SubprocessContext.env and its consumers to Mapping[str,
object], but left subprocess_runner's own env parameter as Mapping[str, str].
That made the public entry point the narrowest link in the chain: a caller
could not pass a Path or int through subprocess_runner even though the context
field, normalise_environment and the redacting logger all accept one.

Widening a parameter is contravariance-safe, so subprocess_runner still
satisfies the CommandRunner protocol; ty confirms this. The protocol itself is
left alone -- narrowing that port would cascade to every implementation.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

@leynos

leynos commented Aug 2, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Have the following now been resolved?

lading/testing/cmd_mox_runner.py (1)

147-147: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use single-line docstrings for private helpers.
The changed private helpers use structured documentation blocks. Replace those blocks with concise summaries.

  • lading/testing/cmd_mox_runner.py#L147-L147: Shorten _resolve_cmd_mox_timeout.
  • lading/testing/cmd_mox_runner.py#L186-L186: Shorten _process_cmd_mox_response.
  • lading/testing/cmd_mox_runner.py#L235-L235: Shorten _should_namespace_cargo_command.
  • lading/testing/cmd_mox_runner.py#L298-L298: Shorten _build_cmd_mox_passthrough_env.
  • lading/testing/cmd_mox_runner.py#L315-L315: Shorten _merge_cmd_mox_path_entries.
  • lading/testing/cmd_mox_runner.py#L341-L341: Shorten _cmd_mox_shim_directory.
  • tests/bdd/steps/test_publish_infrastructure.py#L255-L268: Shorten _normalise_preflight_responses while retaining its precedence and flag-normalisation rules.
  • tests/bdd/steps/test_publish_when_steps.py#L27-L42: Shorten _invoke_publish_with_options.
    As per coding guidelines, private helpers retain concise summaries. As per path instructions, private functions require single-line docstrings.
🤖 Detailed instructions

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @lading/testing/cmd_mox_runner.py at line 147, Replace the structured
docstrings on private helpers with concise single-line summaries: update
_resolve_cmd_mox_timeout, _process_cmd_mox_response,
_should_namespace_cargo_command, _build_cmd_mox_passthrough_env, and
_merge_cmd_mox_path_entries in lading/testing/cmd_mox_runner.py (lines 147, 186,
235, 298, and 315), plus _cmd_mox_shim_directory at line 341; shorten
_normalise_preflight_responses in tests/bdd/steps/test_publish_infrastructure.py
lines 255-268 while preserving its precedence and flag-normalisation rules, and
shorten _invoke_publish_with_options in
tests/bdd/steps/test_publish_when_steps.py lines 27-42.

Sources: Coding guidelines, Path instructions

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- declaration outlines ---'
ast-grep outline lading/testing/cmd_mox_runner.py --items all --view expanded
ast-grep outline tests/bdd/steps/test_publish_infrastructure.py --items all --view expanded
ast-grep outline tests/bdd/steps/test_publish_when_steps.py --items all --view expanded

printf '%s\n' '--- target declarations and context ---'
rg -n -A55 -B10 \
  '^def (_resolve_cmd_mox_timeout|_process_cmd_mox_response|_should_namespace_cargo_command|_build_cmd_mox_passthrough_env|_merge_cmd_mox_path_entries|_cmd_mox_shim_directory|_normalise_preflight_responses|_invoke_publish_with_options)\b' \
  lading/testing/cmd_mox_runner.py \
  tests/bdd/steps/test_publish_infrastructure.py \
  tests/bdd/steps/test_publish_when_steps.py

printf '%s\n' '--- target docstring shape summary ---'
python3 - <<'PY'
import ast
from pathlib import Path

targets = {
    Path("lading/testing/cmd_mox_runner.py"): {
        "_resolve_cmd_mox_timeout",
        "_process_cmd_mox_response",
        "_should_namespace_cargo_command",
        "_build_cmd_mox_passthrough_env",
        "_merge_cmd_mox_path_entries",
        "_cmd_mox_shim_directory",
    },
    Path("tests/bdd/steps/test_publish_infrastructure.py"): {
        "_normalise_preflight_responses",
    },
    Path("tests/bdd/steps/test_publish_when_steps.py"): {
        "_invoke_publish_with_options",
    },
}

for path, names in targets.items():
    print(path)
    tree = ast.parse(path.read_text())
    found = set()
    for node in ast.walk(tree):
        if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name in names:
            found.add(node.name)
            doc = ast.get_docstring(node, clean=False)
            lines = [] if doc is None else doc.splitlines()
            print(f"  {node.name}: docstring_lines={len(lines)}; first_line={lines[0] if lines else None!r}")
    missing = names - found
    if missing:
        raise SystemExit(f"{path}: missing declarations: {sorted(missing)}")
PY

printf '%s\n' '--- current diff for target files ---'
git diff --no-ext-diff --unified=8 -- \
  lading/testing/cmd_mox_runner.py \
  tests/bdd/steps/test_publish_infrastructure.py \
  tests/bdd/steps/test_publish_when_steps.py

Length of output: 34882


@leynos Mark this finding as resolved.

File and symbol Status Current-source verification
lading/testing/cmd_mox_runner.py_resolve_cmd_mox_timeout, _process_cmd_mox_response, _should_namespace_cargo_command, _build_cmd_mox_passthrough_env, _merge_cmd_mox_path_entries, _cmd_mox_shim_directory Resolved Each helper has a concise one-line docstring.
tests/bdd/steps/test_publish_infrastructure.py_normalise_preflight_responses Resolved The helper has a one-line docstring: “First publish override wins; package/publish --allow-dirty follows config.”
tests/bdd/steps/test_publish_when_steps.py_invoke_publish_with_options Resolved with stale location This symbol does not exist in this file. The current codegraph locates it in tests/bdd/steps/test_publish_infrastructure.py at Line 402. It has a one-line docstring: “Register preflight doubles, enable stubs, and run the CLI.”

Treat this as static current-source verification. Do not claim runtime validation.

You are interacting with an AI system.

@leynos

leynos commented Aug 2, 2026

Copy link
Copy Markdown
Owner

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

Code Duplication

lading/workspace/metadata.py:

What lead to degradation?

The module contains 2 functions with similar structure: CargoMetadataParseError.invalid_json,CargoMetadataParseError.non_object_payload

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- target module outline ---'
ast-grep outline lading/workspace/metadata.py --items all --view expanded

printf '%s\n' '--- error factories and parsing context ---'
rg -n -A130 -B16 \
  '^class CargoMetadataParseError\b|^\s+def (invalid_json|non_object_payload)\b|^def (_parse_cargo_metadata|load_cargo_metadata)\b' \
  lading/workspace/metadata.py

printf '%s\n' '--- direct factory call sites and message assertions ---'
rg -n -C4 \
  '\bCargoMetadataParseError\.(invalid_json|non_object_payload)\s*\(|cargo metadata produced invalid JSON output|cargo metadata produced a non-object JSON payload' \
  lading tests -g '*.py' || true

printf '%s\n' '--- focused metadata test candidates ---'
fd -t f -e py . tests | rg 'metadata|workspace' | sort

printf '%s\n' '--- current PR diff for the target file ---'
git diff --no-ext-diff --unified=12 -- lading/workspace/metadata.py

Length of output: 1029


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- target declarations and parser context ---'
rg -n -A120 -B14 \
  '^class CargoMetadataParseError\b|^\s+def (invalid_json|non_object_payload)\b|^def (_parse_cargo_metadata|load_cargo_metadata)\b' \
  lading/workspace/metadata.py

printf '%s\n' '--- references and assertions ---'
rg -n -C4 \
  '\bCargoMetadataParseError\.(invalid_json|non_object_payload)\s*\(|cargo metadata produced invalid JSON output|cargo metadata produced a non-object JSON payload' \
  lading tests -g '*.py' || true

printf '%s\n' '--- target-file diff ---'
git diff --no-ext-diff --unified=12 -- lading/workspace/metadata.py

Length of output: 756


@leynos lading/workspace/metadata.pyCargoMetadataParseError.invalid_json, CargoMetadataParseError.non_object_payload

Keep this finding unresolved. The second current-codegraph inspection also failed during repository setup. No inspection command ran. I cannot verify the current implementations, their distinct parser branches, call sites, tests, or PR diff.

Retry after repository access is available. Do not apply a refactor or suppress the CodeScene diagnostic until that verification completes.

You are interacting with an AI system.

@leynos

leynos commented Aug 2, 2026

Copy link
Copy Markdown
Owner

@coderabbitai resume

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown
✅ Action performed

Reviews resumed.

_expect_sequence: the discarded paragraph explained why the helper is a typed
wrapper rather than functools.partial. That is design rationale rather than
caller-facing behaviour, and the return-narrowing it describes is already
documented on the two overload stubs, which are the caller-visible contract.
The overloads and their docstrings are untouched.

_coerce_publish_setting: its prose documented a coercion rule that is not
recoverable from the (value: object, package_id: str) -> bool signature --
None and non-empty registry lists mean publishable, false and an empty list do
not. AGENTS.md requires non-obvious behaviour to survive, so the replacement
one-liner carries the rule itself rather than restating the function name.

Both are now exactly one physical line, so ignore-one-line-docstrings exempts
them from DOC201/DOC501 despite dropping Returns and Raises.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/developers-guide.md`:
- Around line 276-278: Update the flowchart decision in the documented
error-handling flow to test whether one manifest was attempted, replacing the
workspace-root-specific question. Relabel the branches to distinguish “Yes, one
manifest attempted” from “No, multiple manifests attempted,” ensuring a single
nested-manifest failure follows the original Cargo error path and multiple
attempts follow the aggregate path.

In `@lading/runtime/subprocess_runner.py`:
- Line 51: Update the public SubprocessContext docstring to include a
NumPy-style env parameter section, documenting that env accepts non-string
values and that normalise_environment converts each value with str() before
subprocess execution.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2f2409a1-aae0-42ac-a052-7719e198e567

📥 Commits

Reviewing files that changed from the base of the PR and between 5af1852 and a6cb3b0.

📒 Files selected for processing (6)
  • docs/developers-guide.md
  • docs/users-guide.md
  • lading/config.py
  • lading/runtime/subprocess_runner.py
  • tests/bdd/steps/test_publish_infrastructure.py
  • tests/bdd/steps/test_publish_when_steps.py
🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

  • leynos/cmd-mox (auto-detected)
  • leynos/cuprum (auto-detected)
  • leynos/shared-actions (auto-detected)

Comment thread docs/developers-guide.md
Comment thread lading/runtime/subprocess_runner.py
Introduce CliRunResult, a TypedDict with the four keys _run_cli actually
produces, and use it wherever a publish step passes that dict around.

The contract lives in its own module rather than in test_common_steps, which
owns _run_cli. test_common_steps imports the bump and publish step modules at
the bottom of the file so their steps register with pytest-bdd, which makes it
a hub: test_common_steps -> test_publish_when_steps ->
test_publish_infrastructure -> test_common_steps would close a real cycle.
That is why _run_cli is reached through TYPE_CHECKING guards and deferred
function-body imports today. cli_run_types imports no sibling step module, so
it cannot join that cycle. pytest collects only test_*.py, so the module is
not mistaken for a test, and the tests/bdd/steps/*.py per-file ignores still
cover it.

test_bump_steps already declared _BumpCliRun with these exact four fields, so
the shared contract replaces it rather than sitting beside it. Typing _run_cli
also makes the typ.cast in _invoke_lading_bump redundant, so it becomes a
direct return.

Every import of the contract sits in a TYPE_CHECKING block: the annotations
are deferred by __future__.annotations and the type has no runtime use, so
ruff's TC001 requires it there.

The e2e run_cli helper is left alone -- it returns workspace_root plus an
extra command key, so it does not fit this contract. The shared then_* steps
in test_common_steps and test_bump_steps keep dict[str, typ.Any]; they are not
publish-specific, and pytest-bdd injects them as fixtures, so no call edge
forces the change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

run_cli builds one dict literal with five unconditional keys, so declare
_CliRunResult alongside it and return that instead of dict[str, typ.Any].
The shape stays exactly as it was: command is a list[str] and workspace_root
is the Path passed through unconverted.

This is a separate contract from the BDD tree's CliRunResult rather than a
shared one: the e2e result carries an extra command key and names the path
workspace_root, so the two cannot be unified without changing one of the
dictionaries.

Retype the consumers that actually hold a CLI result -- the private
_run_lading_in_e2e_workspace helper, the three when_run_lading_* steps that
target the cli_run fixture, and the two then steps that read it -- plus the
Returns sections that named the old type. The e2e_state and publish_spies
mappings keep dict[str, typ.Any]; they are unrelated bags, and e2e_state in
particular gains a regenerated_marker key at runtime that a closed TypedDict
would reject.

_CliRunResult is imported under TYPE_CHECKING because test_e2e_steps only
names it in annotations; ruff's TC001 requires that placement.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
codescene-access[bot]

This comment was marked as outdated.

codescene-access[bot]

This comment was marked as outdated.

CodeScene flagged invalid_json and non_object_payload as duplicated structure.
The duplication was real but the shared part was scaffolding, not behaviour:
each body was one return cls("<literal>"), and each was called exactly once,
on adjacent lines of _parse_cargo_metadata. Extracting a shared abstraction
would only have produced cls(message) -- the constructor that already exists.

Inlining removes 30 lines and makes the module consistent with itself: every
other raise here already constructs the error directly, so the factories were
the outlier rather than the pattern. The class is not re-exported and nothing
outside the two call sites referenced either factory.

This also retires a file-wide lint suppression. The DOC501/DOC502 ignore for
metadata.py existed because pydoclint keys those rules on the syntactic
raise-callable and could not resolve a factory classmethod to its exception
type. With the raises direct, only load_cargo_metadata still needs DOC502 --
for exceptions it genuinely propagates -- so a justified per-function noqa
replaces the file-wide entry, and the pyproject comment no longer describes
factories that have been deleted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No quality gates enabled for this code.

@codescene-access codescene-access Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Our agent can fix these. Install it.

Gates Passed
6 Quality Gates Passed

Absence of Expected Change Pattern

  • lading/lading/commands/publish.py is usually changed with: lading/tests/bdd/steps/test_publish_steps.py

Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.

@leynos
leynos merged commit 12ba215 into main Aug 2, 2026
7 checks passed
@leynos
leynos deleted the issue-162-fix-mixed-google-numpy-docstring-style-across-bump-py-bump-toml-py-config-py branch August 2, 2026 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix mixed Google/numpy docstring style across bump.py, bump_toml.py, config.py

3 participants