Move cargo output parsing to an adapter (#69) - #88
Merged
Conversation
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRefactor cargo stdout/stderr parsing into cargo_output_adapter producing CargoIndexLookupFailure; wire publish and publish_index_check to consume the structured failure, migrate tests to construct/pass failures, and update developer documentation. ChangesCargo output adapter refactoring
Sequence DiagramsequenceDiagram
participant Cargo as Cargo subprocess
participant Adapter as cargo_output_adapter.parse_index_lookup_failure
participant Publish as lading.commands.publish
participant IndexCheck as publish_index_check._handle_index_missing_version
Cargo->>Publish: run cargo package/publish
Publish->>Adapter: CargoSubprocessResult + crate_name + subcommand
Adapter-->>Publish: CargoIndexLookupFailure | None
Publish->>IndexCheck: decision on fatal vs downgrade based on failure and plan
Possibly related PRs
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Reviewer's GuideExtracts Cargo index-lookup parsing into a dedicated cargo_output_adapter, introduces a structured CargoIndexLookupFailure value object, and updates publish/index handling, tests, and docs to use the new adapter and structured failures while also normalizing some markdown formatting. Sequence diagram for adapted cargo index lookup handlingsequenceDiagram
participant Cargo as cargo_package
participant PublishPkg as publish._package_crate
participant Adapter as cargo_output_adapter.parse_index_lookup_failure
participant PubHandle as publish._handle_index_missing_version
participant IndexHandle as publish_index_check._handle_index_missing_version
Cargo->>PublishPkg: run_command("cargo", ("package", ...))
PublishPkg-->>PublishPkg: exit_code, stdout, stderr
PublishPkg->>Adapter: parse_index_lookup_failure(crate_name, "package", exit_code, stdout, stderr)
Adapter-->>PublishPkg: CargoIndexLookupFailure | None
alt CargoIndexLookupFailure
PublishPkg->>PubHandle: _handle_index_missing_version(failure, plan, options)
PubHandle->>IndexHandle: _handle_index_missing_version(failure, plan, options, error_cls)
IndexHandle-->>PubHandle: (downgrade or raise)
PubHandle-->>PublishPkg: (propagate result)
else None
PublishPkg-->>PublishPkg: _format_cargo_failure_message(...)
PublishPkg-->>PublishPkg: raise PublishPreflightError
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
lodyai
Bot
force-pushed
the
issue-69-move-cargo-output-parsing-to-an-adapter
branch
from
June 8, 2026 22:41
746cfbc to
1d8a1e5
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@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. Excess Number of Function Argumentslading/commands/cargo_output_adapter.py: parse_index_lookup_failure What lead to degradation?parse_index_lookup_failure has 5 arguments, max arguments = 4 Why does this problem occur?Functions with many arguments indicate either a) low cohesion where the function has too many responsibilities, or b) a missing abstraction that encapsulates those arguments. How to fix it?Start by investigating the responsibilities of the function. Make sure it doesn't do too many things, in which case it should be split into smaller and more cohesive functions. Consider the refactoring INTRODUCE PARAMETER OBJECT to encapsulate arguments that refer to the same logical concept. Helpful refactoring examplesThis code health issue has been solved before in this project. Here are some examples for inspiration: undefined# conftest.py (114b752)
def publish_fixtures(
- tmp_path: Path,
- make_crate: typ.Callable[[Path, str, _CrateSpec | None], WorkspaceCrate],
- make_workspace: typ.Callable[[Path, WorkspaceCrate], WorkspaceGraph],
- make_config: typ.Callable[..., config_module.LadingConfig],
- make_dependency: typ.Callable[[str], WorkspaceDependency],
- publish_options: publish.PublishOptions,
+ request: pytest.FixtureRequest, publish_options: publish.PublishOptions
) -> PublishFixtures:
"""Return the composite publish fixtures used across unit suites."""
+ tmp_path: Path = request.getfixturevalue("tmp_path")
+ make_crate = request.getfixturevalue("make_crate")
+ make_workspace = request.getfixturevalue("make_workspace")
+ make_config = request.getfixturevalue("make_config")
+ make_dependency = request.getfixturevalue("make_dependency")
return PublishFixtures( |
This comment was marked as resolved.
This comment was marked as resolved.
lodyai
Bot
force-pushed
the
issue-69-move-cargo-output-parsing-to-an-adapter
branch
from
June 9, 2026 18:44
1d8a1e5 to
96bb243
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
leynos
marked this pull request as ready for review
June 9, 2026 18:57
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Introduce a cargo output adapter that converts raw package and publish diagnostics into structured index lookup failures before the publish domain decides whether to downgrade or raise them.
Keep the rebased publish command aligned with the command-runner changes from main by removing stale cmd-mox aliases and restoring the lockfile freshness preflight alias. Also tidy the developers guide spacing left by the conflict resolution.
Add a dedicated CargoSubprocessResult value object for parser inputs. Move the new parameter object ahead of existing failure types and wire the parser to use it, preserving the existing CargoIndexLookupFailure surface.
Three review fixes for the cargo-output-parsing-to-adapter work: - developers-guide.md: correct the publish_manifest.py ownership sentence. README adoption is performed by `lading bump`, not publish_manifest.py, which only handles staging-time manifest mutations, workspace copying, and publish.strip_patches. - cargo_output_adapter.py: convert the parse_index_lookup_failure Examples block to an interactive numpy-style doctest that constructs a CargoSubprocessResult and asserts the extracted missing_dependency_name. The docstring becomes raw to satisfy D301. - publish_index_check.py: route the two fatal-path helpers (_raise_name_extraction_failure, _raise_out_of_plan_dependency) through the injected publish logger via the shared failure context instead of the module-level LOGGER, matching their siblings, and drop the now-unused LOGGER. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Three remaining issues from the CI quality check:
- publish_index_check.py: remove the module-level mutable
_INDEX_MISSING_VERSION_DOWNGRADE_COUNTER and its
_record_index_missing_version_downgrade helper. The counter was
never read outside the module, accumulated state with no
observable effect, was not synchronised for concurrent access,
and was not reset between tests, violating test isolation. The
downgrade is already recorded via the logger.warning call that
follows.
- developers-guide.md: update three stale API signatures in the
index-lookup handling section:
- parse_index_lookup_failure now takes keyword-only
(*, crate_name, subcommand, result: CargoSubprocessResult);
document CargoSubprocessResult and CargoIndexLookupFailure
as the input/output types.
- _IndexMissingVersionFailure fields: replace stale 'invocation'
with 'failure: CargoIndexLookupFailure' and add 'failure_message'.
- _log_missing_dependency_failure second parameter renamed from
'invocation' to 'lookup_failure'.
- test_cargo_output_adapter.py: add four Hypothesis property tests
per AGENTS.md line 140 (property tests required when a change
introduces parsing over varied input):
- exit_code=0 always returns None regardless of output content
- both markers + nonzero exit code always produces a failure
- absence of the crates.io index marker always returns None
- extracted missing_dependency_name matches the valid crate-name
pattern when present
Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
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`:
- Line 368: Update the documented signature for _handle_publish_result to match
the implementation: replace the parameters (crate, exit_code, stdout, stderr,
plan, options) with (crate, output, *, plan, options) and add a short clarifying
sentence that output is the tuple (exit_code, stdout, stderr) which the function
unpacks at its boundary; reference the actual function in
lading/commands/publish.py (_handle_publish_result) when making the change so
the docs remain consistent with the code.
In `@tests/unit/publish/test_cargo_output_adapter.py`:
- Around line 183-195: Remove the runtime guard that early-returns when the
index marker appears and instead constrain the Hypothesis strategies for stdout
and stderr so neither can contain _MARKER_INDEX; update the test function
test_parse_index_lookup_failure_missing_index_marker_returns_none to generate
stdout and stderr that exclude _MARKER_INDEX (use .filter or a combined
strategy) and then assert directly that _parse_index_lookup_failure(exit_code,
stdout, stderr) is None, referencing _MARKER_INDEX, stdout, stderr and the
helper _parse_index_lookup_failure to locate the change.
🪄 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: 79dc8d8c-8689-4122-b9ad-f2ebadcc9ee4
📒 Files selected for processing (9)
docs/developers-guide.mdlading/commands/cargo_output_adapter.pylading/commands/publish.pylading/commands/publish_index_check.pytests/unit/publish/test_cargo_output_adapter.pytests/unit/publish/test_index_detection.pytests/unit/publish/test_index_order_properties.pytests/unit/publish/test_phase_dispatch.pytests/unit/publish/test_snapshot_messages.py
💤 Files with no reviewable changes (1)
- tests/unit/publish/test_index_detection.py
lodyai
Bot
force-pushed
the
issue-69-move-cargo-output-parsing-to-an-adapter
branch
from
June 10, 2026 00:19
5ffd11d to
2697419
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The repo follows en-GB Oxford spelling (en-GB-oxendict: -ize/-ization, -yse, -our), so "normalizes"/"normalization" is correct. The rebase resolution had adopted the -ise forms; restore the -ize forms in the crate-name canonicalization section. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/unit/publish/test_cargo_output_adapter.py (1)
193-205:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConstrain stdout to eliminate the runtime guard.
The past review comment at this location is still unresolved. Currently,
stdoutis unconstrained (st.text()), so Hypothesis can generate examples wherestdoutcontains the index marker. The runtime guard at lines 202-204 discards those examples, reducing property-test signal.Constrain
stdoutto exclude the marker, matching the existingstderrconstraint:In `tests/unit/publish/test_cargo_output_adapter.py`, in the test `test_parse_index_lookup_failure_missing_index_marker_returns_none`: 1. Change the `stdout` strategy from `st.text()` to `st.text().filter(lambda s: _MARKER_INDEX.lower() not in s.lower())` so that it mirrors the already-constrained `stderr` strategy. 2. Remove the three-line runtime guard: combined = f"{stdout}\n{stderr}" if _MARKER_INDEX.lower() in combined.lower(): return These lines are no longer needed once both strategies exclude the marker. 3. Replace them with a direct assertion: assert _parse_index_lookup_failure(exit_code, stdout, stderr) is None The result should be: `@given`( stdout=st.text().filter(lambda s: _MARKER_INDEX.lower() not in s.lower()), stderr=st.text().filter(lambda s: _MARKER_INDEX.lower() not in s.lower()), exit_code=st.integers(min_value=1, max_value=255), ) `@settings`(max_examples=80, deadline=None) def test_parse_index_lookup_failure_missing_index_marker_returns_none( stdout: str, stderr: str, exit_code: int ) -> None: """Absence of the crates.io index marker always produces None.""" assert _parse_index_lookup_failure(exit_code, stdout, stderr) is None Validate with `make test` and `make typecheck`.🤖 Prompt for 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. In `@tests/unit/publish/test_cargo_output_adapter.py` around lines 193 - 205, Update the test_test_parse_index_lookup_failure_missing_index_marker_returns_none by constraining stdout like stderr: change stdout from st.text() to st.text().filter(lambda s: _MARKER_INDEX.lower() not in s.lower()), remove the runtime guard that builds combined and returns when the marker is present, and directly assert that _parse_index_lookup_failure(exit_code, stdout, stderr) is None; keep the existing stderr constraint and settings/@given decorator and run tests/typecheck after the change.
🤖 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 494-496: Update the docs text around `_canonical_crate_name(name)`
to use en-GB-oxendict spelling: replace "normalizes" with "normalises" and
"normalization" with "normalisation" in the description that explains
hyphen-to-underscore conversion and the canonical publish-order index lookup
(also apply the same replacement at the other occurrence referenced). Ensure the
function name `_canonical_crate_name(name)` remains unchanged and only the
American spellings are updated to British variants.
---
Duplicate comments:
In `@tests/unit/publish/test_cargo_output_adapter.py`:
- Around line 193-205: Update the
test_test_parse_index_lookup_failure_missing_index_marker_returns_none by
constraining stdout like stderr: change stdout from st.text() to
st.text().filter(lambda s: _MARKER_INDEX.lower() not in s.lower()), remove the
runtime guard that builds combined and returns when the marker is present, and
directly assert that _parse_index_lookup_failure(exit_code, stdout, stderr) is
None; keep the existing stderr constraint and settings/@given decorator and run
tests/typecheck after the change.
🪄 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: 1b6e0606-8e9f-4675-8ecc-16f0f06bfe98
📒 Files selected for processing (3)
docs/developers-guide.mdlading/commands/cargo_output_adapter.pytests/unit/publish/test_cargo_output_adapter.py
Mirror the stderr strategy's filter on the stdout strategy so both exclude the crates.io index marker, then drop the now-redundant runtime guard in favour of a direct assertion. The marker contains no newline, so excluding it from each stream individually guarantees its absence from the newline-joined haystack. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as resolved.
This comment was marked as resolved.
…tests Address two automated review warnings. Testing (Property / Proof): _canonical_crate_name was only covered by example-based behaviour tests in test_phase_dispatch.py. Add test_canonicalisation_properties.py with three Hypothesis properties: - canonicalisation drops every hyphen, preserves length, and is idempotent; - hyphen- and underscore-joined spellings of the same segments canonicalise equal (the unification that prevents false out-of-plan errors); - a publish-order index keyed by canonical name resolves a crate looked up with any separator styling, mirroring _validate_dependency_placement. Testing (Unit And Behavioural): add a parametrised golden test feeding verbatim, authentic full cargo index-miss transcripts (cargo package's "failed to prepare local package for uploading" form and cargo publish's "failed to verify package tarball" form, both with the candidate-versions and required-by context lines) through the public parse_index_lookup_failure API, asserting the structured failure and extracted dependency name. This guards the adapter's markers and name regex against drift from real cargo output and covers the package-phase path at the adapter level. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Owner
Author
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
parse_index_lookup_failure exceeded CodeScene's 70-line method threshold. Extract the two distinct concerns in its body via Extract Function: - _output_matches_index_markers(haystack): "does this output indicate a crates.io index miss?" — the marker-presence check. - _build_index_lookup_failure(...): "construct the structured value object" — name extraction, classification debug log, and CargoIndexLookupFailure assembly. The top-level function now reads as a clear decision flow (success exit, non-matching output, matched failure) and falls below the threshold. No behavioural change; existing adapter and integration tests cover both helpers through the public API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lading/commands/cargo_output_adapter.py (1)
57-76: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd full structured docstrings to public dataclasses.
Both
CargoSubprocessResultandCargoIndexLookupFailureare public API entities consumed bypublish.pyandpublish_index_check.py, yet they carry only single-line docstrings. Guidelines require full NumPy-style structured documentation for all public interfaces, including anAttributessection documenting each field.📝 Proposed fix to add Attributes sections
`@dc.dataclass`(frozen=True, slots=True) class CargoSubprocessResult: - """Raw process output from a single cargo invocation.""" + """Raw process output from a single cargo invocation. + + Attributes + ---------- + exit_code : int + Process exit code returned by the cargo subprocess. + stdout : str + Standard output stream captured from the cargo subprocess. + stderr : str + Standard error stream captured from the cargo subprocess. + """ exit_code: int stdout: str stderr: str `@dc.dataclass`(frozen=True, slots=True) class CargoIndexLookupFailure: - """Represents a cargo failure where the index could not resolve a dependency.""" + """Represents a cargo failure where the index could not resolve a dependency. + + Attributes + ---------- + crate_name : str + Name of the crate whose cargo invocation failed. + subcommand : Literal["package", "publish"] + Cargo subcommand that produced the failure. + exit_code : int + Non-zero process exit code from the failed cargo subprocess. + stdout : str + Standard output stream captured from the failed subprocess. + stderr : str + Standard error stream captured from the failed subprocess. + missing_dependency_name : str | None + Name of the dependency crate that could not be resolved from the + crates.io index, or :data:`None` if name extraction failed. + """ crate_name: str subcommand: typ.Literal["package", "publish"] exit_code: int stdout: str stderr: str missing_dependency_name: str | NoneAs per coding guidelines: "Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."🤖 Prompt for 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. In `@lading/commands/cargo_output_adapter.py` around lines 57 - 76, The dataclasses CargoSubprocessResult and CargoIndexLookupFailure currently have only single-line docstrings; update each to a full NumPy-style docstring including a short summary plus an "Attributes" section documenting every field (for CargoSubprocessResult: exit_code, stdout, stderr; for CargoIndexLookupFailure: crate_name, subcommand, exit_code, stdout, stderr, missing_dependency_name), using the numpy style wording and types as shown in the suggested diff so the public API documentation meets the project guideline.Source: Coding guidelines
🤖 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.
Outside diff comments:
In `@lading/commands/cargo_output_adapter.py`:
- Around line 57-76: The dataclasses CargoSubprocessResult and
CargoIndexLookupFailure currently have only single-line docstrings; update each
to a full NumPy-style docstring including a short summary plus an "Attributes"
section documenting every field (for CargoSubprocessResult: exit_code, stdout,
stderr; for CargoIndexLookupFailure: crate_name, subcommand, exit_code, stdout,
stderr, missing_dependency_name), using the numpy style wording and types as
shown in the suggested diff so the public API documentation meets the project
guideline.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1222d0fb-e60b-4bac-bc71-36fbcaf88369
📒 Files selected for processing (1)
lading/commands/cargo_output_adapter.py
CargoSubprocessResult and CargoIndexLookupFailure are public API consumed by publish.py and publish_index_check.py but carried only single-line docstrings. Expand both to full NumPy-style docstrings with an Attributes section documenting every field, per the project's public-interface documentation guideline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
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.
Owner
Author
|
@coderabbitai Have the following now been resolved? lading/commands/cargo_output_adapter.py (1)
📝 Proposed fix to add Attributes sections`@dc.dataclass`(frozen=True, slots=True)
class CargoSubprocessResult:
- """Raw process output from a single cargo invocation."""
+ """Raw process output from a single cargo invocation.
+
+ Attributes
+ ----------
+ exit_code : int
+ Process exit code returned by the cargo subprocess.
+ stdout : str
+ Standard output stream captured from the cargo subprocess.
+ stderr : str
+ Standard error stream captured from the cargo subprocess.
+ """
exit_code: int
stdout: str
stderr: str
`@dc.dataclass`(frozen=True, slots=True)
class CargoIndexLookupFailure:
- """Represents a cargo failure where the index could not resolve a dependency."""
+ """Represents a cargo failure where the index could not resolve a dependency.
+
+ Attributes
+ ----------
+ crate_name : str
+ Name of the crate whose cargo invocation failed.
+ subcommand : Literal["package", "publish"]
+ Cargo subcommand that produced the failure.
+ exit_code : int
+ Non-zero process exit code from the failed cargo subprocess.
+ stdout : str
+ Standard output stream captured from the failed subprocess.
+ stderr : str
+ Standard error stream captured from the failed subprocess.
+ missing_dependency_name : str | None
+ Name of the dependency crate that could not be resolved from the
+ crates.io index, or :data:`None` if name extraction failed.
+ """
crate_name: str
subcommand: typ.Literal["package", "publish"]
exit_code: int
stdout: str
stderr: str
missing_dependency_name: str | NoneAs per coding guidelines: "Docstrings must follow the 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Coding guidelines |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
cat lading/commands/cargo_output_adapter.pyLength of output: 6987
|
lodyai Bot
pushed a commit
that referenced
this pull request
Jun 10, 2026
The branch was rebased onto origin/main, which had landed four changes that overlap the file extractions on this branch. Because the branch relocated the affected code, those improvements had to be re-applied at the new homes and one merge-only reconciliation re-instated: - #103 (centralise Cargo dependency-section names): bump moved `_DEPENDENCY_SECTION_BY_KIND` and the section helpers into `bump_manifests`. Derive the mapping and `_workspace_dependency_sections` from `bump_toml.DEPENDENCY_SECTIONS` there, matching what #103 did in `bump.py` before the extraction. (`bump_docs`/`bump_toml` carried the change through the merge unchanged.) - build_workspace_graph refactor (from #109): the builders moved to `workspace/graph_build`. Carry over the `_collect_workspace_crates` extraction and the explicit `allow_none=False` sequence checks, and point the #109 helper test at the relocated `graph_build` symbols. - interrogate 100% docstring gate (from #109): document the four `@typ.overload` stubs in `toml_coerce` and `models` (interrogate does not recognise the `typ.overload` alias), matching the documented ellipsis-stub form already used on main. - publish_pipeline split vs #88 cargo-output adapter: the per-crate pipeline split was reconciled against #88 in the branch's main-merge, which the linear rebase dropped. Restore the reconciled `publish_pipeline` that imports the index-lookup helpers from `cargo_output_adapter`/`publish_index_check` rather than the removed pre-#88 `_CargoInvocation`/`_is_index_missing_version_error` names. #98 (dead IPC-timeout constructors) and #102 (shared command-failure detail helpers) merged into their unmoved homes without adjustment. make check-fmt, make lint (ruff + interrogate 100% + pylint 10.00/10), make typecheck, and make test (628 passed) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lodyai Bot
pushed a commit
that referenced
this pull request
Jun 12, 2026
The branch was rebased onto origin/main, which had landed four changes that overlap the file extractions on this branch. Because the branch relocated the affected code, those improvements had to be re-applied at the new homes and one merge-only reconciliation re-instated: - #103 (centralise Cargo dependency-section names): bump moved `_DEPENDENCY_SECTION_BY_KIND` and the section helpers into `bump_manifests`. Derive the mapping and `_workspace_dependency_sections` from `bump_toml.DEPENDENCY_SECTIONS` there, matching what #103 did in `bump.py` before the extraction. (`bump_docs`/`bump_toml` carried the change through the merge unchanged.) - build_workspace_graph refactor (from #109): the builders moved to `workspace/graph_build`. Carry over the `_collect_workspace_crates` extraction and the explicit `allow_none=False` sequence checks, and point the #109 helper test at the relocated `graph_build` symbols. - interrogate 100% docstring gate (from #109): document the four `@typ.overload` stubs in `toml_coerce` and `models` (interrogate does not recognise the `typ.overload` alias), matching the documented ellipsis-stub form already used on main. - publish_pipeline split vs #88 cargo-output adapter: the per-crate pipeline split was reconciled against #88 in the branch's main-merge, which the linear rebase dropped. Restore the reconciled `publish_pipeline` that imports the index-lookup helpers from `cargo_output_adapter`/`publish_index_check` rather than the removed pre-#88 `_CargoInvocation`/`_is_index_missing_version_error` names. #98 (dead IPC-timeout constructors) and #102 (shared command-failure detail helpers) merged into their unmoved homes without adjustment. make check-fmt, make lint (ruff + interrogate 100% + pylint 10.00/10), make typecheck, and make test (628 passed) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lodyai Bot
pushed a commit
that referenced
this pull request
Jun 16, 2026
The branch was rebased onto origin/main, which had landed four changes that overlap the file extractions on this branch. Because the branch relocated the affected code, those improvements had to be re-applied at the new homes and one merge-only reconciliation re-instated: - #103 (centralise Cargo dependency-section names): bump moved `_DEPENDENCY_SECTION_BY_KIND` and the section helpers into `bump_manifests`. Derive the mapping and `_workspace_dependency_sections` from `bump_toml.DEPENDENCY_SECTIONS` there, matching what #103 did in `bump.py` before the extraction. (`bump_docs`/`bump_toml` carried the change through the merge unchanged.) - build_workspace_graph refactor (from #109): the builders moved to `workspace/graph_build`. Carry over the `_collect_workspace_crates` extraction and the explicit `allow_none=False` sequence checks, and point the #109 helper test at the relocated `graph_build` symbols. - interrogate 100% docstring gate (from #109): document the four `@typ.overload` stubs in `toml_coerce` and `models` (interrogate does not recognise the `typ.overload` alias), matching the documented ellipsis-stub form already used on main. - publish_pipeline split vs #88 cargo-output adapter: the per-crate pipeline split was reconciled against #88 in the branch's main-merge, which the linear rebase dropped. Restore the reconciled `publish_pipeline` that imports the index-lookup helpers from `cargo_output_adapter`/`publish_index_check` rather than the removed pre-#88 `_CargoInvocation`/`_is_index_missing_version_error` names. #98 (dead IPC-timeout constructors) and #102 (shared command-failure detail helpers) merged into their unmoved homes without adjustment. make check-fmt, make lint (ruff + interrogate 100% + pylint 10.00/10), make typecheck, and make test (628 passed) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
lodyai Bot
pushed a commit
that referenced
this pull request
Jul 7, 2026
The branch was rebased onto origin/main, which had landed four changes that overlap the file extractions on this branch. Because the branch relocated the affected code, those improvements had to be re-applied at the new homes and one merge-only reconciliation re-instated: - #103 (centralise Cargo dependency-section names): bump moved `_DEPENDENCY_SECTION_BY_KIND` and the section helpers into `bump_manifests`. Derive the mapping and `_workspace_dependency_sections` from `bump_toml.DEPENDENCY_SECTIONS` there, matching what #103 did in `bump.py` before the extraction. (`bump_docs`/`bump_toml` carried the change through the merge unchanged.) - build_workspace_graph refactor (from #109): the builders moved to `workspace/graph_build`. Carry over the `_collect_workspace_crates` extraction and the explicit `allow_none=False` sequence checks, and point the #109 helper test at the relocated `graph_build` symbols. - interrogate 100% docstring gate (from #109): document the four `@typ.overload` stubs in `toml_coerce` and `models` (interrogate does not recognise the `typ.overload` alias), matching the documented ellipsis-stub form already used on main. - publish_pipeline split vs #88 cargo-output adapter: the per-crate pipeline split was reconciled against #88 in the branch's main-merge, which the linear rebase dropped. Restore the reconciled `publish_pipeline` that imports the index-lookup helpers from `cargo_output_adapter`/`publish_index_check` rather than the removed pre-#88 `_CargoInvocation`/`_is_index_missing_version_error` names. #98 (dead IPC-timeout constructors) and #102 (shared command-failure detail helpers) merged into their unmoved homes without adjustment. make check-fmt, make lint (ruff + interrogate 100% + pylint 10.00/10), make typecheck, and make test (628 passed) all green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This branch moves Cargo index lookup diagnostic parsing out of the publish-domain decision helper and into a command-boundary adapter. It introduces a structured
CargoIndexLookupFailurevalue object so publish logic receives typed failure data instead of reading Cargo stderr/stdout markers directly.Closes #69.
Review walkthrough
Validation
make check-fmt: passed.make lint: passed.make test: passed, 474 tests.make typecheck: passed.make markdownlint: passed.make nixie: passed.coderabbit review --agent: passed with 0 findings after follow-up fixes.Notes
make fmtalso normalised Markdown wrapping in nearby documentation files while updating docs/developers-guide.md.Summary by Sourcery
Move cargo index-lookup error parsing into a dedicated adapter and consume a structured failure object in publish workflows.
Enhancements:
Tests: