Derive bump crate sets once in the bump context (#97) - #121
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:
This PR centralises bump-time crate-set derivation in It refactors manifest bumping around a closed per-crate outcome enum ( Documentation was updated: Test coverage was expanded and reorganised: Hypothesis property tests validate crate-set derivation and manifest-selection parity against a naive reference model, with existing bump output snapshots remaining unchanged. Additional focused unit tests cover bump context properties, manifest writing, selector logic, TOML/version helper utilities, and bump output formatting; the prior monolithic bump internals test module was removed in favour of these smaller responsibility-focused suites. WalkthroughCentralise crate-set derivation in ChangesCentralised crate-set threading
Sequence Diagram(s)sequenceDiagram
participant Init as _initialize_bump_context
participant Process as _process_crate_manifests
participant Apply as _apply_crate_manifest_update
participant Write as _update_manifest
Init->>Process: Provide _BumpContext
loop For each workspace crate
Process->>Apply: Pass context and crate name
Apply->>Apply: Derive selectors and dependency targets from context
Apply->>Write: Rewrite manifest when needed
Apply-->>Process: Return _CrateManifestOutcome
end
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 20✅ Passed checks (20 passed)
📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
![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.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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.
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
5cb7c2d to
cd8932a
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.
ddb1f24 to
508c189
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
Fold `params.test_id` into the three assertion failure messages of the parametrised `test_update_crate_manifest` so a failure identifies the offending case without cross-referencing the parametrize table. Assertion conditions are unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![codescene-access[bot]](https://avatars.githubusercontent.com/in/3755875?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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.py`:
- Around line 128-133: _CrateManifestUpdate currently allows an invalid state by
combining was_skipped and was_updated booleans; replace it with a single enum
(or Literal) representing skipped, updated, and unchanged so the outcome is
closed and type-safe. Update _update_crate_manifest to return the enum value and
refactor _process_crate_manifests to use match/case on that enum instead of
checking .was_skipped/.was_updated, and adjust any other call sites that rely on
the old boolean fields.
In `@tests/unit/test_bump_context_properties.py`:
- Around line 126-129: Add failure messages to the bare asserts in the affected
tests so Hypothesis failures identify which crate/example and expected values
broke. Update the checks in test_bump_context_properties and the
_assert_crate_manifest_update assertions to use assert with a descriptive
message including the crate name, example name, and expected vs actual
outcome/version values, referencing context.excluded,
context.updated_crate_names, and _assert_crate_manifest_update to locate the
assertions.
- Around line 286-353: The Hypothesis test for manifest selection is unbounded,
which can make this file-I/O-heavy suite slow and unpredictable in CI. Update
the `@settings` on test_manifest_selection_matches_naive_reference to include
max_examples=20 while keeping the existing suppress_health_check for
HealthCheck.function_scoped_fixture. Locate the change in
test_manifest_selection_matches_naive_reference and keep the rest of the test
logic unchanged.
In `@tests/unit/test_bump_manifest_writing.py`:
- Around line 21-22: The unit tests use bare asserts in multiple cases, which
makes failures hard to identify across parametrized branches. Update the
assertions in the test module to include a clear failure message for each check,
especially in the test cases around the version load and manifest write
scenarios, and apply the same pattern consistently in the related test functions
so the failing condition is obvious when an assert trips. Use the existing test
function names and assertion sites as the anchors for the edits.
In `@tests/unit/test_bump_result_formatting.py`:
- Around line 1-7: The test module is over the logical-line limit and currently
mixes result-message formatting checks with _update_crate_manifest coverage.
Move the change-description, grammar, and snapshot rendering tests from
test_bump_result_formatting.py into a new dedicated
test_bump_output_formatting.py, keeping the existing file focused on
lading.commands.bump._update_crate_manifest and its workspace-manifest fixtures.
Preserve the existing test names and assertions while relocating the
Result-message grammar and rendering block so the module stays under the
400-line cap.
- Around line 410-424: Add failure messages to the newly introduced asserts in
the bump result formatting tests so they follow the existing pattern and are
easier to debug. Update the bare assertions in the message formatting checks
around _build_expected_body, _build_expected_categories, and the header
validation in test_bump_result_formatting to use assert ... , "message" instead
of plain asserts, matching the earlier test_update_crate_manifest block.
In `@tests/unit/test_bump_selectors.py`:
- Around line 8-24: The unit tests for _determine_package_selectors and
_should_skip_crate_update use bare asserts, so add explicit failure messages to
each assertion. Update the asserts in
test_determine_package_selectors_respects_exclusions,
test_determine_package_selectors_includes_package_for_active_crates, and
test_should_skip_crate_update_requires_selectors_or_dependencies to use the
assert-with-message form, keeping the existing expectations but attaching a
short message that explains the expected selector or skip behavior.
In `@tests/unit/test_bump_toml_helpers.py`:
- Line 14: The unit tests in the bump TOML helpers file use bare asserts, so add
failure messages to each assertion for clearer debugging. Update the existing
checks in the nested-table selection and related test cases referenced by the
assertions in the test module, using the relevant test function names (such as
the ones covering absent-table handling, version assignment, value matching, and
out-of-order proxies) to locate them and convert each bare assert to an assert
with a descriptive message.
🪄 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: 62c87c23-5a99-4595-9f11-90d3eefc8f58
📒 Files selected for processing (8)
docs/developers-guide.mdlading/commands/bump.pytests/unit/__snapshots__/test_bump_result_formatting.ambrtests/unit/test_bump_context_properties.pytests/unit/test_bump_manifest_writing.pytests/unit/test_bump_result_formatting.pytests/unit/test_bump_selectors.pytests/unit/test_bump_toml_helpers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cmd-mox(auto-detected)leynos/shared-actions(auto-detected)
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 8
🤖 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.py`:
- Around line 128-133: _CrateManifestUpdate currently allows an invalid state by
combining was_skipped and was_updated booleans; replace it with a single enum
(or Literal) representing skipped, updated, and unchanged so the outcome is
closed and type-safe. Update _update_crate_manifest to return the enum value and
refactor _process_crate_manifests to use match/case on that enum instead of
checking .was_skipped/.was_updated, and adjust any other call sites that rely on
the old boolean fields.
In `@tests/unit/test_bump_context_properties.py`:
- Around line 126-129: Add failure messages to the bare asserts in the affected
tests so Hypothesis failures identify which crate/example and expected values
broke. Update the checks in test_bump_context_properties and the
_assert_crate_manifest_update assertions to use assert with a descriptive
message including the crate name, example name, and expected vs actual
outcome/version values, referencing context.excluded,
context.updated_crate_names, and _assert_crate_manifest_update to locate the
assertions.
- Around line 286-353: The Hypothesis test for manifest selection is unbounded,
which can make this file-I/O-heavy suite slow and unpredictable in CI. Update
the `@settings` on test_manifest_selection_matches_naive_reference to include
max_examples=20 while keeping the existing suppress_health_check for
HealthCheck.function_scoped_fixture. Locate the change in
test_manifest_selection_matches_naive_reference and keep the rest of the test
logic unchanged.
In `@tests/unit/test_bump_manifest_writing.py`:
- Around line 21-22: The unit tests use bare asserts in multiple cases, which
makes failures hard to identify across parametrized branches. Update the
assertions in the test module to include a clear failure message for each check,
especially in the test cases around the version load and manifest write
scenarios, and apply the same pattern consistently in the related test functions
so the failing condition is obvious when an assert trips. Use the existing test
function names and assertion sites as the anchors for the edits.
In `@tests/unit/test_bump_result_formatting.py`:
- Around line 1-7: The test module is over the logical-line limit and currently
mixes result-message formatting checks with _update_crate_manifest coverage.
Move the change-description, grammar, and snapshot rendering tests from
test_bump_result_formatting.py into a new dedicated
test_bump_output_formatting.py, keeping the existing file focused on
lading.commands.bump._update_crate_manifest and its workspace-manifest fixtures.
Preserve the existing test names and assertions while relocating the
Result-message grammar and rendering block so the module stays under the
400-line cap.
- Around line 410-424: Add failure messages to the newly introduced asserts in
the bump result formatting tests so they follow the existing pattern and are
easier to debug. Update the bare assertions in the message formatting checks
around _build_expected_body, _build_expected_categories, and the header
validation in test_bump_result_formatting to use assert ... , "message" instead
of plain asserts, matching the earlier test_update_crate_manifest block.
In `@tests/unit/test_bump_selectors.py`:
- Around line 8-24: The unit tests for _determine_package_selectors and
_should_skip_crate_update use bare asserts, so add explicit failure messages to
each assertion. Update the asserts in
test_determine_package_selectors_respects_exclusions,
test_determine_package_selectors_includes_package_for_active_crates, and
test_should_skip_crate_update_requires_selectors_or_dependencies to use the
assert-with-message form, keeping the existing expectations but attaching a
short message that explains the expected selector or skip behavior.
In `@tests/unit/test_bump_toml_helpers.py`:
- Line 14: The unit tests in the bump TOML helpers file use bare asserts, so add
failure messages to each assertion for clearer debugging. Update the existing
checks in the nested-table selection and related test cases referenced by the
assertions in the test module, using the relevant test function names (such as
the ones covering absent-table handling, version assignment, value matching, and
out-of-order proxies) to locate them and convert each bare assert to an assert
with a descriptive message.
🪄 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: 62c87c23-5a99-4595-9f11-90d3eefc8f58
📒 Files selected for processing (8)
docs/developers-guide.mdlading/commands/bump.pytests/unit/__snapshots__/test_bump_result_formatting.ambrtests/unit/test_bump_context_properties.pytests/unit/test_bump_manifest_writing.pytests/unit/test_bump_result_formatting.pytests/unit/test_bump_selectors.pytests/unit/test_bump_toml_helpers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cmd-mox(auto-detected)leynos/shared-actions(auto-detected)
🛑 Comments failed to post (8)
lading/commands/bump.py (1)
128-133: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Replace the two-boolean outcome with an enum to close off the invalid state.
_CrateManifestUpdatemodels three mutually exclusive outcomes (skipped, updated, unchanged) with two independent booleans, which permits a fourth, meaningless combination (was_skipped=True, was_updated=True) that nothing prevents at the type level. Collapse this into a singleenum.Enum(orLiteral) so illegal states are unrepresentable and call sites can pattern-match instead of chainingifs.♻️ Proposed refactor
-@dc.dataclass(frozen=True, slots=True) -class _CrateManifestUpdate: - """Outcome from processing a single crate manifest.""" - - was_skipped: bool - was_updated: bool +class _ManifestUpdateOutcome(enum.Enum): + """Result of processing a single crate manifest.""" + + SKIPPED = enum.auto() + UPDATED = enum.auto() + UNCHANGED = enum.auto()- return _CrateManifestUpdate(was_skipped=True, was_updated=False) + return _ManifestUpdateOutcome.SKIPPED- return _CrateManifestUpdate(was_skipped=False, was_updated=was_updated) + return ( + _ManifestUpdateOutcome.UPDATED + if was_updated + else _ManifestUpdateOutcome.UNCHANGED + )Update
_process_crate_manifestsand_update_crate_manifesttomatchon the enum instead of reading.was_skipped/.was_updated.As per coding guidelines: "Use
Enumfor fixed sets of related constants" and "Use structural pattern matching (match/case) for branching over structured data, especially for enums."Also applies to: 399-399, 429-429
🤖 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/bump.py` around lines 128 - 133, _CrateManifestUpdate currently allows an invalid state by combining was_skipped and was_updated booleans; replace it with a single enum (or Literal) representing skipped, updated, and unchanged so the outcome is closed and type-safe. Update _update_crate_manifest to return the enum value and refactor _process_crate_manifests to use match/case on that enum instead of checking .was_skipped/.was_updated, and adjust any other call sites that rely on the old boolean fields.tests/unit/test_bump_context_properties.py (2)
126-129: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add failure messages to the bare asserts.
context.excluded/context.updated_crate_nameschecks (126-129) and the_assert_crate_manifest_updateoutcome/version checks (271-283) fail silently on which crate/example broke, since Hypothesis shrinks across many generated names. Attach the crate name and expected values to each assert.🐛 Proposed fix (representative examples)
- assert context.excluded == frozenset(exclude) - assert context.updated_crate_names == frozenset( - crate.name for crate in workspace.crates if crate.name not in set(exclude) - ) + assert context.excluded == frozenset(exclude), f"excluded mismatch for {exclude=}" + assert context.updated_crate_names == frozenset( + crate.name for crate in workspace.crates if crate.name not in set(exclude) + ), f"updated_crate_names mismatch for {exclude=}"- assert outcome.was_skipped is expect_skipped - assert outcome.was_updated is (not expect_skipped) + assert outcome.was_skipped is expect_skipped, f"skip mismatch for {crate.name!r}" + assert outcome.was_updated is (not expect_skipped), f"update mismatch for {crate.name!r}"Apply the same treatment to the remaining bare asserts at lines 275-277 and 283.
As per path instructions, "Useassert …, "message"over bare asserts".Also applies to: 271-283
🤖 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/test_bump_context_properties.py` around lines 126 - 129, Add failure messages to the bare asserts in the affected tests so Hypothesis failures identify which crate/example and expected values broke. Update the checks in test_bump_context_properties and the _assert_crate_manifest_update assertions to use assert with a descriptive message including the crate name, example name, and expected vs actual outcome/version values, referencing context.excluded, context.updated_crate_names, and _assert_crate_manifest_update to locate the assertions.Source: Path instructions
286-353: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
Bound Hypothesis examples for the file-I/O-heavy manifest test.
This test writes real manifests to disk for every generated example with no
max_examplescap, unlike the documented pattern for I/O-heavy property suites. Bound it to keep CI time predictable.`@given`(data=st.data()) -@settings(suppress_health_check=[HealthCheck.function_scoped_fixture]) +@settings( + max_examples=20, + suppress_health_check=[HealthCheck.function_scoped_fixture], +) def test_manifest_selection_matches_naive_reference(As per coding guidelines,
"Property-based tests in the publish suite use@settings(max_examples=20, suppress_health_check=[HealthCheck.function_scoped_fixture]) to keep continuous integration (CI) fast".📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.`@given`(data=st.data()) `@settings`( max_examples=20, suppress_health_check=[HealthCheck.function_scoped_fixture], ) def test_manifest_selection_matches_naive_reference( tmp_path: Path, data: st.DataObject, ) -> None: """``_apply_crate_manifest_update`` honours the context crate sets. Reference semantics (issue `#97`): a non-excluded crate's package version is rewritten to the target; an excluded crate's package version is left untouched; a dependency edge is rewritten only when the crate it targets is itself being updated, and only in the manifest section its ``kind`` selects. The manifest pass must consume ``context.excluded`` and ``context.updated_crate_names`` — derived once in ``_initialize_bump_context`` — rather than re-deriving them per crate. ``tmp_path`` is a function-scoped fixture reused across Hypothesis examples; each example rewrites every manifest to its initial state before processing, so the shared directory cannot leak state between examples. """ names = sorted( data.draw(st.sets(_crate_name, min_size=2, max_size=6), label="names") ) # Force at least one updated (non-excluded) crate and at least one excluded # crate so every example exercises both branches of the skip rule. updated_pivot = data.draw(st.sampled_from(names), label="updated_pivot") other_names = [name for name in names if name != updated_pivot] excluded_pivot = data.draw(st.sampled_from(other_names), label="excluded_pivot") extra_excluded = data.draw( st.sets(st.sampled_from(other_names), max_size=len(other_names)), label="extra_excluded", ) exclude_set = {excluded_pivot} | extra_excluded exclude = tuple(sorted(exclude_set)) edges = _draw_dependency_edges(data, names, updated_pivot) dependencies: _DependencyEdges = { name: tuple(sorted(targets.items())) for name, targets in edges.items() } updated_names = {name for name in names if name not in exclude_set} assert exclude_set, "example must exclude at least one crate" assert any( target in updated_names for targets in edges.values() for target in targets ), "example must contain a dependency on an updated crate" _write_synthetic_manifests(tmp_path, names, dependencies) workspace = _synthetic_workspace(names, root=tmp_path, dependencies=dependencies) context = bump._initialize_bump_context( tmp_path, bump.BumpOptions( configuration=_make_config(exclude=exclude), workspace=workspace, ), ) crates_by_name = {crate.name: crate for crate in workspace.crates} for name in names: _assert_crate_manifest_update( crates_by_name[name], edges[name], context, _ExpectedManifestOutcome( updated_names=updated_names, excluded=name in exclude_set, ), )🤖 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/test_bump_context_properties.py` around lines 286 - 353, The Hypothesis test for manifest selection is unbounded, which can make this file-I/O-heavy suite slow and unpredictable in CI. Update the `@settings` on test_manifest_selection_matches_naive_reference to include max_examples=20 while keeping the existing suppress_health_check for HealthCheck.function_scoped_fixture. Locate the change in test_manifest_selection_matches_naive_reference and keep the rest of the test logic unchanged.Source: Coding guidelines
tests/unit/test_bump_manifest_writing.py (1)
21-22: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add failure messages to the bare asserts.
None of the asserts in this file carry a message, making a Hypothesis-free but multi-branch failure harder to diagnose (e.g. which parametrised case failed at lines 74-78).
🐛 Proposed fix (representative example)
- assert changed is True - assert document["dependencies"]["alpha"].value == "1.0.0" - assert ( - document["workspace"]["dependencies"]["alpha"].value == expected_workspace_alpha - ) + assert changed is True, "dependency-section update should report a change" + assert document["dependencies"]["alpha"].value == "1.0.0", "top-level alpha not updated" + assert ( + document["workspace"]["dependencies"]["alpha"].value == expected_workspace_alpha + ), f"workspace alpha mismatch for include_workspace_sections={include_workspace_sections}"Apply the same treatment to the remaining bare asserts (lines 21-22, 34-38, 49-50, 90-91, 106-109).
As per path instructions, "Useassert …, "message"over bare asserts".Also applies to: 34-38, 49-50, 74-78, 90-91, 106-109
🤖 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/test_bump_manifest_writing.py` around lines 21 - 22, The unit tests use bare asserts in multiple cases, which makes failures hard to identify across parametrized branches. Update the assertions in the test module to include a clear failure message for each check, especially in the test cases around the version load and manifest write scenarios, and apply the same pattern consistently in the related test functions so the failing condition is obvious when an assert trips. Use the existing test function names and assertion sites as the anchors for the edits.Source: Path instructions
tests/unit/test_bump_result_formatting.py (2)
1-7: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
File exceeds the 400-logical-line cap.
The module reaches 461 lines. Split the change-description/grammar/snapshot formatting tests (roughly the
# Result-message grammar and renderingblock) into a dedicatedtest_bump_output_formatting.py, keeping this file focused on_update_crate_manifestcoverage.As per path instructions, "Files must not exceed 400 logical lines: Decompose large modules into subpackages".
🤖 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/test_bump_result_formatting.py` around lines 1 - 7, The test module is over the logical-line limit and currently mixes result-message formatting checks with _update_crate_manifest coverage. Move the change-description, grammar, and snapshot rendering tests from test_bump_result_formatting.py into a new dedicated test_bump_output_formatting.py, keeping the existing file focused on lading.commands.bump._update_crate_manifest and its workspace-manifest fixtures. Preserve the existing test names and assertions while relocating the Result-message grammar and rendering block so the module stays under the 400-line cap.Source: Path instructions
410-424: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add failure messages to the newly-added asserts.
The earlier
test_update_crate_manifestblock (328-335) already carries assert messages per the prior fix, but this later block reintroduces bare asserts.🐛 Proposed fix (representative example)
if not any((manifests, documents, readmes, lockfiles)): - assert message == "No manifest changes required; all versions already 1.2.3." + assert message == "No manifest changes required; all versions already 1.2.3.", ( + "empty change set should report no-changes message" + ) return lines = message.splitlines() - assert lines[1:] == _build_expected_body(root, changes) + assert lines[1:] == _build_expected_body(root, changes), "body lines mismatch"Apply the same treatment to lines 421-424, 446-447, and 458-461.
As per path instructions, "Useassert …, "message"over bare asserts".Also applies to: 446-447, 458-461
🤖 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/test_bump_result_formatting.py` around lines 410 - 424, Add failure messages to the newly introduced asserts in the bump result formatting tests so they follow the existing pattern and are easier to debug. Update the bare assertions in the message formatting checks around _build_expected_body, _build_expected_categories, and the header validation in test_bump_result_formatting to use assert ... , "message" instead of plain asserts, matching the earlier test_update_crate_manifest block.Source: Path instructions
tests/unit/test_bump_selectors.py (1)
8-24: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add failure messages to the bare asserts.
Logic coverage is correct, but none of the four asserts carry a message.
🐛 Proposed fix
- assert bump._determine_package_selectors("beta", {"beta"}) == () + assert bump._determine_package_selectors("beta", {"beta"}) == (), ( + "excluded crate should yield no selectors" + ) ... - assert bump._determine_package_selectors("beta", set()) == (("package",),) + assert bump._determine_package_selectors("beta", set()) == (("package",),), ( + "active crate should yield the package selector" + ) ... - assert bump._should_skip_crate_update((), {}) is True + assert bump._should_skip_crate_update((), {}) is True, ( + "empty selectors and dependencies should skip" + ) assert ( bump._should_skip_crate_update((("package",),), {"dependencies": ("alpha",)}) is False - ) + ), "non-empty selectors should not skip"As per path instructions, "Use
assert …, "message"over bare asserts".📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.def test_determine_package_selectors_respects_exclusions() -> None: """Excluded crates produce no package selectors.""" assert bump._determine_package_selectors("beta", {"beta"}) == (), ( "excluded crate should yield no selectors" ) def test_determine_package_selectors_includes_package_for_active_crates() -> None: """Active crates receive the package selector tuple.""" assert bump._determine_package_selectors("beta", set()) == (("package",),), ( "active crate should yield the package selector" ) def test_should_skip_crate_update_requires_selectors_or_dependencies() -> None: """Skipping occurs only when both selectors and dependency sections are empty.""" assert bump._should_skip_crate_update((), {}) is True, ( "empty selectors and dependencies should skip" ) assert ( bump._should_skip_crate_update((("package",),), {"dependencies": ("alpha",)}) is False ), "non-empty selectors should not skip"🤖 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/test_bump_selectors.py` around lines 8 - 24, The unit tests for _determine_package_selectors and _should_skip_crate_update use bare asserts, so add explicit failure messages to each assertion. Update the asserts in test_determine_package_selectors_respects_exclusions, test_determine_package_selectors_includes_package_for_active_crates, and test_should_skip_crate_update_requires_selectors_or_dependencies to use the assert-with-message form, keeping the existing expectations but attaching a short message that explains the expected selector or skip behavior.Source: Path instructions
tests/unit/test_bump_toml_helpers.py (1)
14-14: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add failure messages to the bare asserts.
All assertions in this file (nested-table selection, absent-table handling, version assignment, value matching, out-of-order proxies) are bare.
🐛 Proposed fix (representative example)
- assert table is document["workspace"]["package"] + assert table is document["workspace"]["package"], "nested table selector failed" ... - assert table is None + assert table is None, "non-table selector path should return None" ... - assert bump._assign_version(None, "1.0.0") is False + assert bump._assign_version(None, "1.0.0") is False, "absent table should be a no-op"Apply the same treatment to the remaining bare asserts at lines 32-33, 39, 44-45, 52-53, 68-69, and 82-83.
As per path instructions, "Useassert …, "message"over bare asserts".Also applies to: 21-21, 26-26, 32-33, 39-39, 44-45, 52-53, 68-69, 82-83
🤖 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/test_bump_toml_helpers.py` at line 14, The unit tests in the bump TOML helpers file use bare asserts, so add failure messages to each assertion for clearer debugging. Update the existing checks in the nested-table selection and related test cases referenced by the assertions in the test module, using the relevant test function names (such as the ones covering absent-table handling, version assignment, value matching, and out-of-order proxies) to locate them and convert each bare assert to an assert with a descriptive message.Source: Path instructions
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
…ssages (#97) Address a batch of review findings on the bump command and its tests. - bump.py: replace the two-boolean `_CrateManifestUpdate` dataclass with a closed `_CrateManifestOutcome` enum (SKIPPED / UPDATED / UNCHANGED) so the both-true state is unrepresentable; `_process_crate_manifests` selects on it with match/case. Remove the redundant `_update_crate_manifest` wrapper (a pass-through once the outcome is an enum) and point its sole caller at `_apply_crate_manifest_update`. - Split test_bump_result_formatting.py: the change-description, grammar, and snapshot result-message tests move to a new test_bump_output_formatting.py (with the snapshot file renamed accordingly), leaving the original focused on `_apply_crate_manifest_update` and its fixtures. Both modules are now well under the 400-line cap. - Cap the I/O-heavy manifest-selection Hypothesis test at max_examples=20, matching the repo's established pattern. - Add descriptive failure messages to the bare asserts across test_bump_selectors, test_bump_toml_helpers, test_bump_manifest_writing, test_bump_context_properties, and test_bump_output_formatting; the manifest property helper now names the crate and reports expected vs actual. No behavioural change to the bump command; the enum carries the same skipped/updated/unchanged information the boolean pair did. 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.
|
@coderabbitai Have the following failed checks now been resolved? If further work is required, please provide an AI agent prompt for the remaining work to be done to address these failures. Do not treat warnings as optional or aspirational. Where a change is out of scope for this PR, propose a GitHub issue unless one exists already. (Treat documentation and validation coverage as in scope). ❌ Failed checks (2 warnings)
|
|
@coderabbitai Have the following now been resolved? Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity. Inline comments:
In
In
In
In
In
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== settings/max_examples on property tests ==="
rg -n "max_examples|`@settings`|HealthCheck" tests/unit/test_bump_context_properties.py
echo ""
echo "=== file line counts ==="
wc -l tests/unit/test_bump_context_properties.py tests/unit/test_bump_result_formatting.py tests/unit/test_bump_manifest_writing.py tests/unit/test_bump_selectors.py tests/unit/test_bump_toml_helpers.py 2>/dev/null
echo ""
echo "=== does test_bump_command_internals.py still exist? ==="
fd test_bump_command_internals.py
echo ""
echo "=== _CrateManifestUpdate / outcome dataclass in bump.py ==="
rg -n "_CrateManifestUpdate|was_skipped|was_updated" lading/commands/bump.py
echo ""
echo "=== docs mention of max_examples pattern ==="
rg -n "max_examples" docs/developers-guide.mdLength of output: 1767 🏁 Script executed: #!/bin/bash
echo "=== _apply_crate_manifest_update and outcome type ==="
rg -n "_apply_crate_manifest_update|_process_crate_manifests|_should_skip_crate_update" lading/commands/bump.py
echo ""
sed -n '380,435p' lading/commands/bump.pyLength of output: 2637
Performance And Resource Use — ✅ resolved. `@settings`(
max_examples=20,
suppress_health_check=[HealthCheck.function_scoped_fixture],
)directly above the manifest-selection property test, matching the 20-example pattern documented in Architectural Complexity And Maintainability — ✅ resolved. Both remediation items landed:
No further work is required for either check. ( •̀ᴗ•́) nine for nine |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== _CrateManifestUpdate ==="
rg -n "_CrateManifestUpdate|was_skipped|was_updated" lading/commands/bump.py
echo ""
sed -n '100,145p' lading/commands/bump.py
echo "=== test_bump_context_properties.py asserts ==="
sed -n '100,140p' tests/unit/test_bump_context_properties.py
echo "---settings---"
rg -n "`@settings`|max_examples|suppress_health_check" tests/unit/test_bump_context_properties.py
sed -n '280,360p' tests/unit/test_bump_context_properties.py
echo "=== test_bump_manifest_writing.py asserts ==="
grep -n "assert " tests/unit/test_bump_manifest_writing.py | head -40
echo "=== test_bump_result_formatting.py length & asserts ==="
wc -l tests/unit/test_bump_result_formatting.py
grep -n "^def \|^class " tests/unit/test_bump_result_formatting.py
grep -n "assert " tests/unit/test_bump_result_formatting.py | head -40
echo "=== test_bump_selectors.py asserts ==="
cat -n tests/unit/test_bump_selectors.py
echo "=== test_bump_toml_helpers.py asserts ==="
grep -n "assert " tests/unit/test_bump_toml_helpers.py | head -40Length of output: 11848
class _CrateManifestOutcome(enum.Enum):
"""Closed outcome of processing a single crate manifest.
Replaces a two-boolean record so the skipped/updated/unchanged states are
mutually exclusive and the ``both true`` state is unrepresentable.
"""
SKIPPED = enum.auto()
UPDATED = enum.auto()
UNCHANGED = enum.auto()
`@settings`(
max_examples=20,
suppress_health_check=[HealthCheck.function_scoped_fixture],
)is applied directly above
Nice work clearing the full batch. (ゝ∀・)b |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lading/commands/bump.py (1)
434-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTighten
excludedtocabc.Set[str]for the same reasonupdated_crateswas tightened.
_determine_package_selectorsstill acceptsexcluded: cabc.Collection[str], so a caller could hand it a list and silently degrade thecrate_name in excludedcheck to O(n). The sibling helpers (_workspace_dependency_sections,_dependency_sections_for_crate) were already tightened tocabc.Set[str]for exactly this reason in this PR — extend the same contract here for consistency, since the only caller passescontext.excluded, which is already afrozenset[str].♻️ Proposed fix
def _determine_package_selectors( crate_name: str, - excluded: cabc.Collection[str], + excluded: cabc.Set[str], ) -> tuple[tuple[str, ...], ...]:🤖 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/bump.py` around lines 434 - 449, Tighten `_determine_package_selectors` to accept `excluded: cabc.Set[str]` instead of `cabc.Collection[str]`, matching the existing set-based contract used by `_workspace_dependency_sections` and `_dependency_sections_for_crate`. Update the helper signature and any related type hints in `bump.py` so the membership check stays O(1) and remains consistent with the `context.excluded` caller, which already provides a `frozenset[str]`.
🤖 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 `@tests/unit/test_bump_context_properties.py`:
- Around line 230-262: The forced “depends on updated crate” edge is currently
attached to an arbitrary crate in `_draw_dependency_edges`, so the
excluded-crate rewrite path is not guaranteed to be exercised. Update the helper
and its call site in `test_bump_context_properties` so the forced edge is pinned
to `excluded_pivot` instead of a random non-updated crate, and pass that
excluded pivot through to `_draw_dependency_edges` alongside `updated_pivot`.
In `@tests/unit/test_bump_output_formatting.py`:
- Around line 46-89: Add assertion messages to every bare equality check in the
affected bump output formatting tests so failures explain what went wrong.
Update test_format_manifest_path_relative,
test_format_manifest_path_outside_workspace, and the no-op asserts around the
later sections in test_bump_output_formatting.py to use assert ... , "message"
consistently, following the existing style already used in the sibling
formatting tests.
- Line 25: The test module is using two different references for the same class,
so update all `BumpChanges` usages in this file to consistently use
`bump_output.BumpChanges` instead of `bump.BumpChanges`. Keep the existing
`bump_output` import as the single source of truth and replace the remaining
`bump` references at the relevant `BumpChanges` construction sites and
assertions.
In `@tests/unit/test_bump_result_formatting.py`:
- Around line 32-141: The private helper docstrings in
_make_test_crate_with_dependency, _make_workspace_with_alpha_dependency, and
_parse_manifest_versions use mixed Google/numpy style and are too verbose for
underscore-prefixed functions. Simplify each private helper to a single-line
summary only, or if structured docs must remain, convert the section headers to
numpy style with Parameters/Returns and remove the Google-style Args:. Apply the
same docstring cleanup consistently across all three helpers so the test module
follows the numpy docstring convention.
In `@tests/unit/test_bump_selectors.py`:
- Around line 22-30: The test for bump._should_skip_crate_update only covers the
both-empty and both-non-empty extremes, so add explicit branch coverage for the
empty-selectors/non-empty-dependencies case and the
non-empty-selectors/empty-dependencies case. Update
test_should_skip_crate_update_requires_selectors_or_dependencies to use a
parametrized pytest test (and add the pytest import) so each OR-branch is
asserted directly, while still keeping the existing crate-update skip behavior
checks.
---
Outside diff comments:
In `@lading/commands/bump.py`:
- Around line 434-449: Tighten `_determine_package_selectors` to accept
`excluded: cabc.Set[str]` instead of `cabc.Collection[str]`, matching the
existing set-based contract used by `_workspace_dependency_sections` and
`_dependency_sections_for_crate`. Update the helper signature and any related
type hints in `bump.py` so the membership check stays O(1) and remains
consistent with the `context.excluded` caller, which already provides a
`frozenset[str]`.
🪄 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: c67e240b-24f7-4cd8-9641-8d34af81f4c5
📒 Files selected for processing (10)
docs/developers-guide.mdlading/commands/bump.pytests/unit/__snapshots__/test_bump_output_formatting.ambrtests/unit/test_bump_command_internals.pytests/unit/test_bump_context_properties.pytests/unit/test_bump_manifest_writing.pytests/unit/test_bump_output_formatting.pytests/unit/test_bump_result_formatting.pytests/unit/test_bump_selectors.pytests/unit/test_bump_toml_helpers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cmd-mox(auto-detected)leynos/shared-actions(auto-detected)
💤 Files with no reviewable changes (1)
- tests/unit/test_bump_command_internals.py
Address a batch of review findings across the bump tests and one typing tweak. - bump.py: tighten `_determine_package_selectors` from `cabc.Collection[str]` to `cabc.Set[str]`, matching the sibling section helpers and the frozenset `context.excluded` caller so membership stays O(1). - test_bump_context_properties.py: pin the forced dependency edge to `excluded_pivot` (not a random crate) so the excluded-crate dependency rewrite path is exercised on every example. - test_bump_output_formatting.py: reference `bump_output.BumpChanges` / `bump_output._format_result_message` consistently (drop the `bump` alias) and add failure messages to the remaining bare equality asserts. - test_bump_result_formatting.py: reduce three private helper docstrings to single-line summaries. - test_bump_selectors.py: parametrise the skip-behaviour test across all four selector/dependency OR-branches. 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/bump.py is usually changed with: lading/tests/unit/test_cli.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.
|
@coderabbitai review |
✅ Action performedReview finished.
|
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/bump.py (1)
434-449: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFix the mixed docstring style on
_determine_package_selectors.The docstring pairs a Google-style
Args:block with a numpy-styleReturns\n-------section._determine_package_selectorsis a private helper, so per path instructions it should carry only a single-line summary — full structured docs are reserved for public interfaces.🩹 Proposed fix
def _determine_package_selectors( crate_name: str, excluded: cabc.Set[str], ) -> tuple[tuple[str, ...], ...]: - """Return package selectors for the crate, respecting exclusion rules. - - Args: - crate_name: Name of the crate to check. - excluded: Set of excluded crate names. - - Returns - ------- - Package selectors tuple, or empty tuple if crate is excluded. - - """ + """Return package selectors for the crate, respecting exclusion rules.""" return () if crate_name in excluded else (("package",),)As per path instructions, "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/bump.py` around lines 434 - 449, The docstring on _determine_package_selectors mixes Google and numpy styles and is too verbose for a private helper; replace it with a single-line summary only. Keep the function name and existing behavior unchanged, and remove the Args/Returns sections so the docstring matches the path instruction for private functions and methods.Source: Path instructions
🤖 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/bump.py`:
- Around line 434-449: The docstring on _determine_package_selectors mixes
Google and numpy styles and is too verbose for a private helper; replace it with
a single-line summary only. Keep the function name and existing behavior
unchanged, and remove the Args/Returns sections so the docstring matches the
path instruction for private functions and methods.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b0638458-c4c0-4e8b-b11d-08379ed55029
📒 Files selected for processing (5)
lading/commands/bump.pytests/unit/test_bump_context_properties.pytests/unit/test_bump_output_formatting.pytests/unit/test_bump_result_formatting.pytests/unit/test_bump_selectors.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/cmd-mox(auto-detected)leynos/shared-actions(auto-detected)
|
@coderabbitai please create a single new GitHub issue for the following concern and all other instances of this issue across the codebase: lading/commands/bump.py (1)
🩹 Proposed fixdef _determine_package_selectors(
crate_name: str,
excluded: cabc.Set[str],
) -> tuple[tuple[str, ...], ...]:
- """Return package selectors for the crate, respecting exclusion rules.
-
- Args:
- crate_name: Name of the crate to check.
- excluded: Set of excluded crate names.
-
- Returns
- -------
- Package selectors tuple, or empty tuple if crate is excluded.
-
- """
+ """Return package selectors for the crate, respecting exclusion rules."""
return () if crate_name in excluded else (("package",),)As per path instructions, "Docstrings must follow the 🤖 Detailed instructionsVerify each finding against current code. Fix only still-valid issues, skip the In Source: Path instructions |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
echo "=== search for mixed docstring patterns (Args: followed by Returns\n----) ==="
rg -n -B2 "Args:" lading -A15 --type py | grep -B15 "Returns$" | head -100
echo ""
echo "=== more targeted: find functions with 'Args:' block in private helpers ==="
rg -n "Args:" --type py lading
echo ""
echo "=== find 'Returns' followed by dashes (numpy style) near Args: (google style) in same docstring ==="
rg -nP "Args:" --type py -A 20 lading | rg -B20 "^\d+-\s*-------"Length of output: 5173 🏁 Script executed: #!/bin/bash
echo "=== bump_toml.py function signatures ==="
sed -n '110,175p' lading/commands/bump_toml.py
echo ""
echo "=== bump.py functions at 480-500 ==="
sed -n '475,500p' lading/commands/bump.py
echo ""
echo "=== config.py function at 225-250 ==="
sed -n '220,250p' lading/config.pyLength of output: 4340 🏁 Script executed: #!/bin/bash
set -euo pipefail
body=$(cat <<'EOF'
## Summary
Several docstrings in the codebase mix Google-style `Args:` blocks with numpy-style `Returns`/`Raises` sections. Per the repository's path instructions, docstrings must follow the numpy style guide throughout: private functions/methods should carry only a single-line summary, and public interfaces should use full structured numpy-style docs (`Parameters`, `Returns`, `Raises` — not `Args:`).
Originating from review discussion on `#121` (comment: https://github.com/leynos/lading/pull/121#issuecomment-placeholder), raised by `@leynos`.
## Affected locations
**Private helpers (should be truncated to a single-line summary):**
- `lading/commands/bump.py` — `_determine_package_selectors` (Lines 434-449)
- `lading/commands/bump.py` — `_update_manifest` (Lines 481-500)
- `lading/config.py` — `_validate_mapping_keys` (Lines 225-244)
**Public functions (should be converted to full numpy-style docs, i.e. `Args:` → `Parameters\n----------`):**
- `lading/commands/bump_toml.py` — `update_section` (Lines 128-144)
- `lading/commands/bump_toml.py` — `update_dependency_sections` (Lines 156-171)
## Required changes
- For each private helper listed above, replace the docstring with a single-line summary only, removing the `Args:`/`Returns`/`Raises` sections.
- For each public function listed above, rewrite the docstring to full numpy style: replace `Args:` with `Parameters\n----------` (with parameter types), and keep `Returns`/`Raises` sections in numpy format (already numpy-style in these cases).
- Audit the rest of the codebase for the same `Args:` + numpy-section mixed pattern to catch any remaining instances beyond those listed here.
## Acceptance criteria
- No docstring mixes Google-style `Args:` with numpy-style `Returns`/`Raises` sections.
- Private functions/methods carry single-line-summary docstrings only.
- Public functions/methods carry full numpy-style structured docstrings.
## References
- PR: https://github.com/leynos/lading/pull/121
- Requested by: `@leynos`
EOF
)
gh issue create \
--repo leynos/lading \
--title "Fix mixed Google/numpy docstring style across bump.py, bump_toml.py, config.py" \
--body "$body" \
--assignee leynosLength of output: 2445
Instances included: Private helpers (fix: truncate to single-line summary, per path instructions):
Public functions (fix: convert fully to numpy style, i.e.
I assigned it to you. Let me know if you'd like the fix for these applied in this PR instead of deferred to the new issue. |
2 participants
Summary
Closes #97
_update_crate_manifestnow consumes theexcludedandupdated_crate_namessets from_BumpContextinstead of re-deriving both from options for every crate, removing the O(n²) workspace-member iteration._BumpContext.excludedfield is now used._validate_bump_optionsis removed — its only caller performed the redundant re-extraction.docs/developers-guide.md, and thebump.pymodule docstring now orients contributors onrunas the orchestrator, theBumpOptions/BumpChangesrecords, the single-source-of-truth crate-set rule, and delegation tobump_docs,bump_lockfiles,bump_readme, andbump_toml.Testing
test_manifest_selection_matches_naive_reference, drives_apply_crate_manifest_updateagainst real temporary per-crate manifests: it assertswas_skipped/was_updatedand the on-disk package and dependency versions against a naive reference. Every drawn example forces at least one excluded crate and at least one dependency on an updated crate, so the test fails if the manifest pass stops consumingcontext.excludedorcontext.updated_crate_names(verified against both mutations).make check-fmt,make lint,make typecheck, andmake test(675 passed) all green.References
🤖 Generated with Claude Code