Fix version bump with out-of-order TOML tables and workspace sections - #54
Conversation
When a Cargo.toml has [package.metadata.docs.rs] appearing after other top-level tables like [dependencies], tomlkit returns an OutOfOrderTableProxy instead of a Table. The _select_table function previously only accepted Table instances, causing version bumps to be silently skipped for such crates. This change: - Imports OutOfOrderTableProxy from tomlkit.container - Adds _TableLike type alias (Table | OutOfOrderTableProxy) - Updates _select_table to accept both table types - Updates _assign_version and dependency functions accordingly - Adds unit tests for out-of-order table handling Closes #52 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reviewer's GuideExtend the bump command’s TOML handling to treat tomlkit OutOfOrderTableProxy objects as table-like in selection, dependency updating, and version assignment, and add regression tests for out-of-order [package] tables. Class diagram for updated TOML table handling in bump commandclassDiagram
class Table
class OutOfOrderTableProxy
class TOMLDocument
class _TableLike {
<<interface>>
}
class BumpInternals {
+_select_table(document: TOMLDocument | _TableLike, keys: tuple_str) _TableLike
+_assign_version(table: _TableLike | None, target_version: str) bool
+_update_dependency_table(table: _TableLike, dependency_names: Collection_str, target_version: str) bool
+_update_dependency_entry(container: _TableLike, key: str, entry: object, target_version: str) bool
}
Table ..|> _TableLike
OutOfOrderTableProxy ..|> _TableLike
BumpInternals ..> Table
BumpInternals ..> OutOfOrderTableProxy
BumpInternals ..> TOMLDocument
BumpInternals ..> _TableLike
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughBroaden TOML handling by introducing a _TableLike union (Table | OutOfOrderTableProxy), update multiple internal helpers and signatures to accept it, and add a BumpOptions flag to include workspace-level dependency sections. Changes
Sequence Diagram(s)(Skip; changes do not introduce a new multi-component control flow requiring a sequence diagram.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
**/test_*.py📄 CodeRabbit inference engine (.rules/python-00.md)
Files:
**/*test*.py📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
Files:
🧬 Code graph analysis (1)tests/unit/test_bump_command_integration.py (5)
🔍 Remote MCP DeepwikiSummaryBased on the comprehensive exploration of the PR context and supporting documentation, here is the relevant information for reviewing PR #54: Key Problem Being SolvedThe PR fixes version bump handling when TOML contains out-of-order tables (tomlkit.OutOfOrderTableProxy), which occurs when TOML sections appear in a non-standard order. The recent tomlkit library includes changes to "ensure unique table indices when adding items to out-of-order tables", making it essential for lading to handle these structures correctly. Core ChangesThe PR implements two main features:
Test CoverageThe PR adds three new tests:
Note on Static Analysis Feedback: The CodeScene analysis flagged the test function with 5 parameters. As documented in the PR, removing the redundant Architecture ContextThe bump command uses ⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (3)
Comment |
![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.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Instead of sprinkling
# type: ignore[index]around_TableLikeusages, consider defining a smallProtocolwith__getitem__/__setitem__andgetso mypy/pyright understand the interface and you don’t need to suppress type checking at each call site. - _TABLE_LIKE_TYPES is currently untyped (
typ.Final = (Table, OutOfOrderTableProxy)); giving it an explicit type such astyp.Final[tuple[type[Table], type[OutOfOrderTableProxy]]]would improve type safety and editor support when used inisinstancechecks.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Instead of sprinkling `# type: ignore[index]` around `_TableLike` usages, consider defining a small `Protocol` with `__getitem__`/`__setitem__` and `get` so mypy/pyright understand the interface and you don’t need to suppress type checking at each call site.
- _TABLE_LIKE_TYPES is currently untyped (`typ.Final = (Table, OutOfOrderTableProxy)`); giving it an explicit type such as `typ.Final[tuple[type[Table], type[OutOfOrderTableProxy]]]` would improve type safety and editor support when used in `isinstance` checks.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
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)
447-460: Add justification fortype: ignorecomment on line 459.Same rationale as above: the ignore directive needs explanation for maintainability.
🔎 Proposed fix
- container[key] = replacement # type: ignore[index] + container[key] = replacement # type: ignore[index] # OutOfOrderTableProxy supports item assignment
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
lading/commands/bump.pytests/unit/test_bump_command_internals.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: For Python files, ensure changes pass all relevant unit and behavioral tests according to testing guidelines.
For Python files, ensure changes pass lint checks viamake lint.
For Python files, ensure changes adhere to formatting standards viamake check-fmt.
For Python files, ensure changes pass type checking viamake typecheck.
For Python development, refer to detailed guidelines in the.rules/directory including Python Code Style Guidelines, Context Managers, Exception Design/Raising/Handling/Logging, Generators, Project Configuration, Return Patterns, and Typing best practices.
**/*.py: Use snake_case.py for Python file names, naming them for their contents
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private or internal helpers with a single underscore (_)
Enable and maintain full static type coverage using Pyright; use typing everywhere
Use TypedDict or @DataClass(slots=True) for structured data
Avoid Any; use precise types (TypeVar, Protocol, Literal, Union) and typing.cast only when necessary with justification
Be explicit with return type annotations using -> None, -> str, etc., for all public functions and class methods
Prefer immutability: use tuples over lists and types.MappingProxyType for read-only mappings
Enforce strict mode in Pyright; treat all Pyright warnings as CI errors and use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Document public functions, classes, and modules using NumPy-format docstrings
Use inline comments to explain tricky code and non-obvious logic
**/*.py: Use context managers (viacontextlib.contextmanageror__enter__/__exit__methods) to encapsulate setup and teardown logic for resource management (files, locks, connections, etc.)
Use@contextlib.contextmanagerdecorator for straightforward procedural setup...
Files:
tests/unit/test_bump_command_internals.pylading/commands/bump.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep C90 / mccabe complexity ≤ 9
- Follow single responsibility and CQRS (command/query segregation)
- Prefer structural pattern matching to
- Prefer structural pattern matching over
isinstance()or imperative decomposition.- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...
Files:
tests/unit/test_bump_command_internals.pylading/commands/bump.py
**/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior, not internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors
Files:
tests/unit/test_bump_command_internals.py
**/*test*.py
📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
In tests, use narrow exception assertions with
pytest.raises()specifying the expected type and optionally constraining the message via regex (B017)
Files:
tests/unit/test_bump_command_internals.py
🧬 Code graph analysis (1)
tests/unit/test_bump_command_internals.py (1)
lading/commands/bump.py (2)
_select_table(691-707)_assign_version(710-724)
🔍 Remote MCP Deepwiki
Based on my search through the repository and documentation, I've gathered comprehensive context about this PR. Let me provide a summary of the relevant findings for your pull request review:
Summary of Additional Context
What This PR Does
This pull request fixes version bump handling when tomlkit returns OutOfOrderTableProxy for out-of-order TOML tables. The fix is necessary because when TOML tables appear in non-standard order (e.g., [package], [dependencies], [package.metadata.docs.rs]), tomlkit returns a special proxy object instead of a regular Table, which caused the bump logic to fail when trying to update versions.
Key Technical Changes The PR introduces:
-
New Type Union (
_TableLike): A union type that accepts bothTableandOutOfOrderTableProxyto generalize table-like structures across the bump command. -
Function Signature Updates: Core functions like
_update_dependency_table,_update_dependency_entry,_select_table, and_assign_versionnow accept the broader_TableLiketype instead of justTable. -
Type Check Expansion: The code now uses an
isinstance()check against a_TABLE_LIKE_TYPEStuple containing both table types, ensuring nested table-like structures are handled consistently. -
New Import:
OutOfOrderTableProxyis imported fromtomlkit.containerto support the type union.
Architectural Context The _select_table function is central to this fix. It navigates TOML documents to retrieve nested tables by key sequences. The PR updates the type checking logic so that both Table and OutOfOrderTableProxy instances are recognized as valid table-like objects during traversal.
Testing Coverage Two new unit tests verify the fix:
test_select_table_handles_out_of_order_package: Ensures selecting a package table returns a usable table-like object when an out-of-order[package.metadata.docs.rs]table appears after other sectionstest_assign_version_works_with_out_of_order_table: Verifies version assignment works correctly with the proxy
Framework Context The project uses tomlkit specifically to preserve TOML formatting and comments during manifest rewrites. This is critical because the bump command must maintain user-defined document structure and formatting. The introduction of OutOfOrderTableProxy handling extends this formatting preservation to workspaces with non-standard TOML table ordering.
Related Architecture The bump command workflow involves:
- Parsing manifests with
tomlkitto preserve structure - Selecting appropriate tables for version updates
- Assigning new versions while preserving trivia (comments, whitespace)
- Writing changes atomically back to disk
The PR ensures all these steps work correctly when tables are out-of-order.
Code Quality Considerations for Review
Key areas to validate:
- Type consistency: Verify that all code paths treating the union type handle both
TableandOutOfOrderTableProxyidentically in terms of indexing and attribute access - Test comprehensiveness: The two new tests should cover the most common out-of-order scenarios
- Breaking changes: Confirm that the signature changes to internal helpers don't affect any external APIs or other modules
- Type ignore markers: Review the necessity of any
type: ignorecomments added to suppress type checking warnings
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Sourcery review
🔇 Additional comments (5)
tests/unit/test_bump_command_internals.py (2)
413-426: LGTM!Test correctly verifies that
_select_tablehandlesOutOfOrderTableProxywhen[package.metadata.docs.rs]appears after other sections, causing tomlkit to return a proxy object. The assertion chain is appropriate.
429-440: LGTM!Test correctly verifies version assignment works with the proxy object. The assertions validate both the return value and the side effect on the table.
lading/commands/bump.py (3)
17-17: LGTM!Import of
OutOfOrderTableProxyfromtomlkit.containeris correctly placed with other tomlkit imports.
691-707: LGTM!The
_select_tablefunction correctly handles bothTableandOutOfOrderTableProxyvia the_TABLE_LIKE_TYPEStuple. Theisinstancechecks properly narrow the type, and the return type annotation is accurate.
710-724: LGTM!The
_assign_versionfunction signature correctly accepts_TableLike | None, enabling version updates on bothTableandOutOfOrderTableProxyinstances. The implementation logic remains sound.
Address PR review comments: - Add explicit type annotation to _TABLE_LIKE_TYPES for improved type safety and editor support - Add justification comments explaining why type: ignore[index] is needed (OutOfOrderTableProxy supports indexing/assignment at runtime but type stubs don't reflect this) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Move the workspace sections behavior flag into BumpOptions dataclass where it belongs semantically alongside other flags like dry_run. Changes: - Add include_workspace_sections field to BumpOptions dataclass - Update _update_manifest to check options.include_workspace_sections - Add _update_workspace_dependency_sections helper function - Update _process_workspace_manifest to set include_workspace_sections=True This refactoring improves code organization by grouping related behavioral flags together in the options object. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reduce code duplication and nesting in dependency section handling: - Add _update_section helper to consolidate table selection and update - Merge _update_workspace_dependency_sections into _update_dependency_sections using include_workspace_sections parameter - Reduce maximum nesting depth from 3 to 2 Add parametrized test for workspace dependency sections: - Test [workspace.dependencies], [workspace.dev-dependencies], and [workspace.build-dependencies] sections - Handle both simple string format and table format with version key - Use clear test IDs for pytest output 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
leynos
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: tests/unit/test_bump_command_integration.py Comment on lines +309 to +339 def test_run_updates_workspace_dependency_sections(
tmp_path: pathlib.Path,
section_name: str,
initial_version: str,
target_version: str,
expected_version: str,
) -> None:
"""Workspace-level dependency sections are updated with the new version."""
workspace = _make_workspace(tmp_path)
manifest_path = tmp_path / "Cargo.toml"
manifest_path.write_text(
"[workspace]\n"
f'members = ["crates/alpha", "crates/beta"]\n\n'
"[workspace.package]\n"
'version = "0.1.0"\n\n'
f"[workspace.{section_name}]\n"
f"alpha = {initial_version}\n",
encoding="utf-8",
)
configuration = _make_config()
bump.run(
tmp_path,
target_version,
options=bump.BumpOptions(configuration=configuration, workspace=workspace),
)
document = parse_toml(manifest_path.read_text(encoding="utf-8"))
entry = document["workspace"][section_name]["alpha"]
# Handle both simple string format and table format with version key
version = entry["version"].value if hasattr(entry, "get") else entry.value
assert version == expected_version❌ New issue: Excess Number of Function Arguments |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 11668 @leynos, the issue is that the test function has 5 parameters when the maximum allowed is 4. Examining the test cases, I noticed that Suggested FixRemove the @pytest.mark.parametrize(
("section_name", "initial_version", "target_version"),
[
("dependencies", '"0.1.0"', "1.2.3"),
("dev-dependencies", '{ version = "0.1.0" }', "2.0.0"),
("build-dependencies", '"0.1.0"', "3.0.0"),
],
ids=["dependencies", "dev-dependencies", "build-dependencies"],
)
def test_run_updates_workspace_dependency_sections(
tmp_path: pathlib.Path,
section_name: str,
initial_version: str,
target_version: str,
) -> None:
"""Workspace-level dependency sections are updated with the new version."""
workspace = _make_workspace(tmp_path)
manifest_path = tmp_path / "Cargo.toml"
manifest_path.write_text(
"[workspace]\n"
f'members = ["crates/alpha", "crates/beta"]\n\n'
"[workspace.package]\n"
'version = "0.1.0"\n\n'
f"[workspace.{section_name}]\n"
f"alpha = {initial_version}\n",
encoding="utf-8",
)
configuration = _make_config()
bump.run(
tmp_path,
target_version,
options=bump.BumpOptions(configuration=configuration, workspace=workspace),
)
document = parse_toml(manifest_path.read_text(encoding="utf-8"))
entry = document["workspace"][section_name]["alpha"]
# Handle both simple string format and table format with version key
version = entry["version"].value if hasattr(entry, "get") else entry.value
assert version == target_versionAI Coding Agent Prompt |
The expected_version parameter always equals target_version in all test cases, making it redundant. This simplifies the test by reducing the parameter count from 5 to 4. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <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
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
lading/commands/bump.pytests/unit/test_bump_command_integration.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: For Python files, ensure changes pass all relevant unit and behavioral tests according to testing guidelines.
For Python files, ensure changes pass lint checks viamake lint.
For Python files, ensure changes adhere to formatting standards viamake check-fmt.
For Python files, ensure changes pass type checking viamake typecheck.
For Python development, refer to detailed guidelines in the.rules/directory including Python Code Style Guidelines, Context Managers, Exception Design/Raising/Handling/Logging, Generators, Project Configuration, Return Patterns, and Typing best practices.
**/*.py: Use snake_case.py for Python file names, naming them for their contents
Use PascalCase for class names
Use snake_case for variable and function names
Use UPPER_SNAKE_CASE for module-level constants
Prefix private or internal helpers with a single underscore (_)
Enable and maintain full static type coverage using Pyright; use typing everywhere
Use TypedDict or @DataClass(slots=True) for structured data
Avoid Any; use precise types (TypeVar, Protocol, Literal, Union) and typing.cast only when necessary with justification
Be explicit with return type annotations using -> None, -> str, etc., for all public functions and class methods
Prefer immutability: use tuples over lists and types.MappingProxyType for read-only mappings
Enforce strict mode in Pyright; treat all Pyright warnings as CI errors and use # pyright: ignore sparingly with explanation
Avoid side effects at import time; modules should not modify global state or perform actions on import
Document public functions, classes, and modules using NumPy-format docstrings
Use inline comments to explain tricky code and non-obvious logic
**/*.py: Use context managers (viacontextlib.contextmanageror__enter__/__exit__methods) to encapsulate setup and teardown logic for resource management (files, locks, connections, etc.)
Use@contextlib.contextmanagerdecorator for straightforward procedural setup...
Files:
tests/unit/test_bump_command_integration.pylading/commands/bump.py
⚙️ CodeRabbit configuration file
**/*.py: - Keep C90 / mccabe complexity ≤ 9
- Follow single responsibility and CQRS (command/query segregation)
- Prefer structural pattern matching to
- Prefer structural pattern matching over
isinstance()or imperative decomposition.- Docstrings must follow the
numpystyle guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces.- Move conditionals with >2 branches to predicate/helper functions
- Avoid
eval,exec,pickle, monkey-patching,ctypes, unsafe shell- Every module must begin with a triple-quoted docstring explaining its purpose, utility, and usage, including example calls if appropriate.
- Comments and docs must follow en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Lint suppressions:
- Blanket
# noqa, file-level skips, and categories are forbidden- Only narrow in-line disables (
# noqa: XYZ) are permitted, and must be accompanied byFIXME:or a ticket link, and used only as a last resort.- Use
pytestfixtures for shared setup (conftest.pyorfixtures/)- Replace duplicate tests with
@pytest.mark.parametrize- Prefer
pytest-mockorunittest.mockfor stubs/mocks- Use
assert …, "message"over bare asserts- Reflect all API/behaviour changes in
docs/and update roadmap on completion- Files must not exceed 400 logical lines:
- Decompose large modules into subpackages
- Split large
match/caseor dispatch tables by domain and collocate with targets if appropriate- Move bulky data (fixtures, templates) to external files for parsing at runtime
- Mutable defaults and shadowed built-ins are forbidden
- All code must have clear type hints using modern style (
A | B,list[str],class Foo[A]:,type Bar = int, etc.), with ABC imports drawn from the correct stdlib module.- All path manipulation must be performed using pathlib for cross platform safety. Do not use string manipulation for extraction of path components or filename ele...
Files:
tests/unit/test_bump_command_integration.pylading/commands/bump.py
**/test_*.py
📄 CodeRabbit inference engine (.rules/python-00.md)
**/test_*.py: Use pytest idioms: prefer fixtures over setup/teardown, parametrize broadly, avoid unnecessary mocks
Group related tests using class with method names prefixed by test_
Write tests from a user's perspective; test public behavior, not internals
Avoid mocking too much; prefer test doubles only for external services or non-deterministic behaviors
Files:
tests/unit/test_bump_command_integration.py
**/*test*.py
📄 CodeRabbit inference engine (.rules/python-exception-design-raising-handling-and-logging.md)
In tests, use narrow exception assertions with
pytest.raises()specifying the expected type and optionally constraining the message via regex (B017)
Files:
tests/unit/test_bump_command_integration.py
🧬 Code graph analysis (1)
tests/unit/test_bump_command_integration.py (5)
tests/unit/conftest.py (1)
_make_workspace(128-131)tests/unit/test_cli.py (1)
_make_workspace(163-176)tests/helpers/workspace_builders.py (1)
_make_workspace(100-106)lading/cli.py (1)
bump(266-286)lading/commands/bump.py (2)
run(115-133)BumpOptions(55-64)
⏰ Context from checks skipped due to timeout of 120000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Sourcery review
- GitHub Check: Sourcery review
🔇 Additional comments (10)
lading/commands/bump.py (10)
17-17: LGTM!Import correctly added for
OutOfOrderTableProxyto support the new_TableLikeunion.
33-38: LGTM!Type alias and constant are well-defined with explicit annotations. The use of PEP 695
typekeyword andtyp.Finalfor the tuple aligns with the coding guidelines.
64-64: LGTM!New
include_workspace_sectionsfield with a sensible default maintains backward compatibility.
181-185: LGTM!Correctly enables
include_workspace_sectionsspecifically for the workspace manifest via immutabledc.replace.
366-382: LGTM!Docstring clearly documents the
include_workspace_sectionsbehaviour, and the option is correctly propagated to_update_dependency_sections.
426-447: LGTM!Clean refactor with the new
_update_sectionhelper. The conditional workspace section handling is clear and maintains backward compatibility via the default value.
450-464: LGTM!Well-extracted helper with single responsibility. The docstring clearly explains the
pathtuple convention.
467-496: LGTM!Type signatures correctly updated to
_TableLike. Thetype: ignorecomments now include justifications explainingOutOfOrderTableProxyruntime behaviour, as required by coding guidelines.
727-743: LGTM!The
_select_tablefunction correctly handles bothTableandOutOfOrderTableProxyusing consistentisinstancechecks against_TABLE_LIKE_TYPES. The logic properly returnsNonewhen navigation fails or when an emptykeystuple is passed with a non-table-like document.
746-760: LGTM!Type signature correctly broadened to
_TableLike | None, enabling version assignment onOutOfOrderTableProxyinstances without any logic changes.
The expected_version parameter always equals target_version in all test cases, making it redundant. This simplifies the test by reducing the parameter count from 5 to 4. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <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
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.
1 participant
Summary
Changes
Core logic
Tests
Test plan
📎 Task: https://www.terragonlabs.com/task/5d8fd7af-a60e-4bda-a4fa-ee140503046d