Extend bump to update workspace dependencies (refactor) - #55
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughSummarise the bump refactor: delegate TOML and documentation processing to new modules ( Changes
Sequence DiagramsequenceDiagram
participant CLI as Bump command (CLI)
participant TOML as bump_toml
participant DOCS as bump_docs
participant FS as Filesystem
CLI->>TOML: parse_manifest(manifest_path)
TOML->>FS: read manifest file
FS-->>TOML: TOML content
TOML-->>CLI: parsed TOMLDocument
CLI->>TOML: update_dependency_sections(doc, sections, target_version, include_workspace_sections)
TOML->>TOML: select_table → update_dependency_table → assign_version
TOML-->>CLI: changed? (bool)
rect rgba(120,180,160,0.12)
note over CLI,DOCS: Documentation TOML‑fence update flow
CLI->>DOCS: resolve_documentation_targets(workspace_root, config)
DOCS->>FS: glob/read documentation files
FS-->>DOCS: file paths / contents
CLI->>DOCS: update_documentation_files(paths, target_version, crates, dry_run)
DOCS->>DOCS: parse markdown tokens → locate TOML fences
DOCS->>TOML: update_toml_snippet_versions / prepare_version_replacement
TOML-->>DOCS: updated snippet + change flag
DOCS->>FS: write_atomic_text(updated_markdown)
end
CLI->>TOML: write_atomic_text(updated_manifest)
TOML->>FS: atomic write manifest
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
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 (1)**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
⚙️ CodeRabbit configuration file
Files:
🧬 Code graph analysis (1)lading/commands/bump_docs.py (2)
🔍 Remote MCP DeepwikiSummary of Additional ContextBump Command ArchitectureThe Workspace Dependencies ContextWorkspace dependencies are a specific type of dependency within a Rust workspace that refer to other crates also part of the same workspace, distinct from regular dependencies which typically refer to external crates from BumpOptions Configuration
Documentation Processing PipelineThe bump command uses glob patterns defined in the This context validates that the PR's refactoring into ⏰ 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)
🔇 Additional comments (11)
Comment |
Reviewer's GuideRefactors the bump command to delegate TOML and documentation handling to new bump_toml and bump_docs modules while extending dependency bumping to workspace.* sections, and adds tests to cover the new behavior and structure. Sequence diagram for bump workflow with workspace dependency updates and docs processingsequenceDiagram
actor Developer
participant CLI as bump_command
participant TOML as bump_toml
participant DOCS as bump_docs
Developer->>CLI: invoke bump(root_path, target_version, options)
CLI->>CLI: build BumpOptions(include_workspace_sections)
rect rgb(235, 245, 255)
CLI->>TOML: parse_manifest(manifest_path)
TOML-->>CLI: document
loop selectors to update package and workspace.package
CLI->>TOML: select_table(document, selector)
TOML-->>CLI: table
CLI->>TOML: assign_version(table, target_version)
TOML-->>CLI: changed?
end
alt dependency_sections configured
CLI->>TOML: update_dependency_sections(document, dependency_sections, target_version, include_workspace_sections)
note right of TOML: updates [dependencies], [dev-dependencies], [build-dependencies]
alt include_workspace_sections is True
note right of TOML: also updates [workspace.dependencies], [workspace.dev-dependencies], [workspace.build-dependencies]
end
end
alt manifest_changed and not options.dry_run
CLI->>TOML: write_atomic_text(manifest_path, document.as_string())
end
end
rect rgb(245, 255, 235)
CLI->>DOCS: resolve_documentation_targets(root_path, documentation_config)
DOCS-->>CLI: documentation_paths
CLI->>DOCS: update_documentation_files(documentation_paths, target_version, updated_crate_names, dry_run)
loop each documentation file
DOCS->>DOCS: rewrite_markdown_toml_fences(text, dependency_targets, target_version)
DOCS->>DOCS: update_toml_snippet_versions(snippet, dependency_targets, target_version)
DOCS->>TOML: assign_version(select_table(document, path), target_version)
DOCS->>TOML: update_dependency_table(table, dependency_targets, target_version)
alt snippet_changed and not dry_run
DOCS->>TOML: write_atomic_text(doc_path, updated_text)
end
end
end
CLI-->>Developer: report updated manifests and docs
Class diagram for refactored bump, bump_toml, and bump_docs modulesclassDiagram
class bump {
+_update_manifest(manifest_path, selectors, target_version, options)
+_process_documentation_files(context, target_version)
+_parse_manifest(manifest_path)
+_select_table(document, keys)
+_assign_version(table, target_version)
+_value_matches(value, expected)
+_update_dependency_sections(document, dependency_sections, target_version, include_workspace_sections)
+_update_dependency_table(table, dependency_names, target_version)
}
class bump_toml {
+value_as_string(value)
+compose_requirement(existing, target_version)
+prepare_version_replacement(value, target_version)
+assign_dependency_version_field(container, target_version)
+update_dependency_entry(container, key, entry, target_version)
+update_dependency_table(table, dependency_names, target_version)
+update_section(document, path, names, target_version)
+update_dependency_sections(document, dependency_sections, target_version, include_workspace_sections)
+parse_manifest(manifest_path)
+select_table(document, keys)
+assign_version(table, target_version)
+value_matches(value, expected)
+write_atomic_text(file_path, content)
}
class bump_docs {
+resolve_documentation_targets(workspace_root, documentation)
+update_documentation_files(documentation_paths, target_version, updated_crates, dry_run)
+rewrite_markdown_toml_fences(markdown_text, dependency_targets, target_version)
+replace_markdown_fences(markdown_text, language, transform)
+token_matches_language(token, language)
+render_fence(token, lines, language, transform)
+extract_fence_indent(line, fence_marker)
+update_toml_snippet_dependencies(document, dependency_targets, target_version)
+update_toml_snippet_versions(snippet, dependency_targets, target_version)
}
bump ..> bump_toml : uses
bump ..> bump_docs : uses
bump_docs ..> bump_toml : uses
class BumpOptions {
+dry_run: bool
+dependency_sections: Mapping
+include_workspace_sections: bool
}
bump ..> BumpOptions : reads
File-Level Changes
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.
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: Comment on lines +433 to +434 *,
include_workspace_sections: bool = False,❌ New issue: Bumpy Road Ahead |
|
@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix: Comment on file def _update_manifest(
def _update_manifest( # noqa: PLR0913❌ New issue: Excess Number of Function Arguments |
|
@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 +353 to +377 def test_run_updates_workspace_build_dependencies_section(
tmp_path: pathlib.Path,
) -> None:
"""Workspace dependency entries in [workspace.build-dependencies] are updated."""
workspace = _make_workspace(tmp_path)
manifest_path = tmp_path / "Cargo.toml"
manifest_path.write_text(
"[workspace]\n"
'members = ["crates/alpha", "crates/beta"]\n\n'
"[workspace.package]\n"
'version = "0.1.0"\n\n'
"[workspace.build-dependencies]\n"
'alpha = { version = "0.1.0" }\n',
encoding="utf-8",
)
configuration = _make_config()
bump.run(
tmp_path,
"3.0.0",
options=bump.BumpOptions(configuration=configuration, workspace=workspace),
)
document = parse_toml(manifest_path.read_text(encoding="utf-8"))
build_deps = document["workspace"]["build-dependencies"]
assert build_deps["alpha"]["version"].value == "3.0.0"❌ New issue: Code Duplication |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
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: Comment on file dependency_sections: typ.Mapping[str, typ.Collection[str]] = dc.field(
default_factory=lambda: types.MappingProxyType({})
)
include_workspace_sections: bool = False❌ New issue: Lines of Code in a Single File |
|
@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: str,
version_spec: str,
target_version: str,
expected_version: str,
) -> None:
"""Workspace dependency entries in [workspace.<section>] are updated."""
workspace = _make_workspace(tmp_path)
manifest_path = tmp_path / "Cargo.toml"
manifest_path.write_text(
"[workspace]\n"
'members = ["crates/alpha", "crates/beta"]\n\n'
"[workspace.package]\n"
'version = "0.1.0"\n\n'
f"[workspace.{section}]\n"
f"alpha = {version_spec}\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]["alpha"]
# Handle both string format ("0.1.0") and table format ({ version = "0.1.0" })
actual_version = entry["version"].value if hasattr(entry, "get") else entry.value
assert actual_version == expected_version❌ New issue: Excess Number of Function Arguments |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
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: Comment on file """Documentation processing utilities for version bumping."""❌ New issue: Overall Code Complexity |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Add support for updating version entries in [workspace.dependencies], [workspace.dev-dependencies], and [workspace.build-dependencies] sections when running lading bump. This ensures workspaces that centralize dependency versions have all references kept consistent. The implementation adds an include_workspace_sections parameter to _update_dependency_sections() and _update_manifest(), which is enabled when processing the workspace manifest but not for individual crate manifests. Closes #53 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Move include_workspace_sections into BumpOptions dataclass for better cohesion with other behavioural flags like dry_run. This reduces the parameter count of _update_manifest from 5 to 4. Extract _update_section helper function to eliminate duplicated table lookup and update logic, reducing nesting depth from 3 to 2. Consolidate three separate workspace dependency section tests into a single parametrized test, reducing test code from ~75 lines to ~40 lines while maintaining the same coverage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
- Extracted TOML manipulation utilities for version bumping into a new `bump_toml.py` module. - Extracted documentation processing utilities into a new `bump_docs.py` module. - Updated `bump.py` to re-export necessary functions for backward compatibility and to delegate functionality to the new modules. - Improved code organization and separation of concerns for the bump command's internal implementation. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
Use absolute feature paths derived from __file__ for BDD steps. Normalize markdown tables and wrap text for linting.
Extract helpers for version assignment and dependency sections so snippet rewrites are simpler and easier to follow.
965d1dd to
6e97cd3
Compare
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 found 2 issues, and left some high level feedback:
- In
bump_docs.update_toml_snippet_versions, combining the three update calls withorchanges the behavior by short‑circuiting after the firstTrue, so later updates (e.g., dependency sections) may be skipped ifpackageorworkspace.packagealready changes; consider using separate calls with an accumulatedchangedflag as in the original implementation. - The new
_FEATURES_DIRbase used in the BDD step files points toPath(__file__).resolve().parent.parent / 'features', which fortests/bdd/stepsresolves totests/featuresinstead of the previous../features/...undertests/bdd; please double‑check the directory layout to ensure the scenarios are still loaded from the intendedfeaturesdirectory.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `bump_docs.update_toml_snippet_versions`, combining the three update calls with `or` changes the behavior by short‑circuiting after the first `True`, so later updates (e.g., dependency sections) may be skipped if `package` or `workspace.package` already changes; consider using separate calls with an accumulated `changed` flag as in the original implementation.
- The new `_FEATURES_DIR` base used in the BDD step files points to `Path(__file__).resolve().parent.parent / 'features'`, which for `tests/bdd/steps` resolves to `tests/features` instead of the previous `../features/...` under `tests/bdd`; please double‑check the directory layout to ensure the scenarios are still loaded from the intended `features` directory.
## Individual Comments
### Comment 1
<location> `lading/commands/bump_docs.py:181-182` </location>
<code_context>
+ if not dependency_targets:
+ return False
+
+ sections = ("dependencies", "dev-dependencies", "build-dependencies")
+ return any(
+ _update_single_dependency_section(
+ document, section, dependency_targets, target_version
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `any(...)` short-circuits and prevents later dependency sections from being updated
Previously, all dependency sections (`dependencies`, `dev-dependencies`, `build-dependencies`) were updated, even if an earlier one changed. With `return any(_update_single_dependency_section(...))`, `any` short-circuits on the first `True`, so later sections are never visited and may retain stale versions. Please switch back to an explicit loop that always processes all sections while accumulating a `changed` flag.
</issue_to_address>
### Comment 2
<location> `lading/commands/bump_docs.py:201-206` </location>
<code_context>
+ except TOMLKitError:
+ return snippet, False
+
+ changed = (
+ _try_assign_version_at_path(document, ("package",), target_version)
+ or _try_assign_version_at_path(
+ document, ("workspace", "package"), target_version
+ )
+ or update_toml_snippet_dependencies(
+ document, dependency_targets, target_version
+ )
</code_context>
<issue_to_address>
**issue (bug_risk):** Short-circuiting `or` chain can prevent some updates from running in TOML snippets
The previous logic always attempted all three updates (package, workspace.package, and dependencies) while separately tracking whether anything changed. With `changed = (a or b or c)`, the calls now short‑circuit: if `_try_assign_version_at_path(document, ("package",), ...)` returns `True`, the workspace and dependency updates are skipped, so some versions may never be updated. To retain the original behavior, use a non–short-circuiting pattern (e.g., `changed = False` plus separate `if`/assignment statements for each update).
</issue_to_address>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
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (11)
docs/lading-design.mddocs/scripting-standards.mddocs/users-guide.mdlading/commands/bump.pylading/commands/bump_docs.pylading/commands/bump_toml.pytests/bdd/steps/test_commands_catalogue_steps.pytests/bdd/steps/test_common_steps.pytests/bdd/steps/test_workspace_metadata_steps.pytests/unit/test_bump_command_integration.pytests/unit/test_bump_command_internals.py
🧰 Additional context used
📓 Path-based instructions (6)
**/*.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/bdd/steps/test_commands_catalogue_steps.pytests/unit/test_bump_command_integration.pytests/bdd/steps/test_common_steps.pytests/unit/test_bump_command_internals.pylading/commands/bump_docs.pylading/commands/bump.pylading/commands/bump_toml.pytests/bdd/steps/test_workspace_metadata_steps.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/bdd/steps/test_commands_catalogue_steps.pytests/unit/test_bump_command_integration.pytests/bdd/steps/test_common_steps.pytests/unit/test_bump_command_internals.pylading/commands/bump_docs.pylading/commands/bump.pylading/commands/bump_toml.pytests/bdd/steps/test_workspace_metadata_steps.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/bdd/steps/test_commands_catalogue_steps.pytests/unit/test_bump_command_integration.pytests/bdd/steps/test_common_steps.pytests/unit/test_bump_command_internals.pytests/bdd/steps/test_workspace_metadata_steps.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/bdd/steps/test_commands_catalogue_steps.pytests/unit/test_bump_command_integration.pytests/bdd/steps/test_common_steps.pytests/unit/test_bump_command_internals.pytests/bdd/steps/test_workspace_metadata_steps.py
docs/**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
docs/**/*.md: Use the markdown files within thedocs/directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Proactively update relevant files in thedocs/directory when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve to keep documentation accurate and current.
docs/**/*.md: Use British English based on the Oxford English Dictionary (en-GB-oxendict) with suffixes: -ize in words like 'realize' and 'organization', -lyse in words like 'analyse' and 'paralyse', -our in words like 'colour' and 'behaviour', -re in words like 'centre' and 'calibre', double 'l' in words like 'cancelled' and 'counsellor', maintain 'e' in words like 'likeable', -ogue in words like 'catalogue'
Use the Oxford comma in documentation: 'ships, planes, and hovercraft' where it aids comprehension
Treat company names as collective nouns in documentation, for example 'Lille Industries are expanding'
Write headings in sentence case
Use Markdown headings (#, ##, ###, and so on) in order without skipping levels
Follow markdownlint recommendations for Markdown formatting
Always provide a language identifier for fenced code blocks; use 'plaintext' for non-code text
Use '-' as the first level bullet and renumber lists when items change in Markdown
Prefer inline links using text or angle brackets around the URL in Markdown
Ensure blank lines before and after bulleted lists and fenced blocks in Markdown
Ensure tables have a delimiter line below the header row in Markdown
Expand any uncommon acronym on first use, for example, Continuous Integration (CI)
Wrap paragraphs at 80 columns in documentation
Wrap code at 120 columns in documentation
Do not wrap tables in documentation
Use footnotes referenced with [^label] in Markdown documentation
Include Mermaid diagrams in documentation where they add clarity
When embedding figures in documentation, use 
**/*.md: For Markdown files (.md only), ensure changes pass lint checks viamake markdownlint.
For Markdown files (.md only), ensure Mermaid diagrams pass validation using nixie viamake nixie.
Files:
docs/lading-design.mddocs/scripting-standards.mddocs/users-guide.md
⚙️ CodeRabbit configuration file
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -yse / -our) spelling and grammar
- Headings must not be wrapped.
- Documents must start with a level 1 heading
- Headings must correctly increase or decrease by no more than one level at a time
- Use GitHub-flavoured Markdown style for footnotes and endnotes.
- Numbered footnotes must be numbered by order of appearance in the document.
Files:
docs/lading-design.mddocs/scripting-standards.mddocs/users-guide.md
docs/scripting-standards.md
📄 CodeRabbit inference engine (docs/roadmap.md)
Document cuprum as the standard for command execution in scripting standards documentation
Files:
docs/scripting-standards.md
🧬 Code graph analysis (4)
tests/unit/test_bump_command_integration.py (4)
tests/unit/test_cli.py (1)
_make_workspace(163-176)tests/helpers/workspace_builders.py (2)
_make_workspace(100-106)_make_config(123-135)lading/cli.py (1)
bump(266-286)lading/commands/bump.py (2)
run(95-113)BumpOptions(35-44)
tests/unit/test_bump_command_internals.py (1)
lading/cli.py (1)
bump(266-286)
lading/commands/bump_docs.py (2)
lading/config.py (1)
DocumentationConfig(55-72)lading/commands/bump_toml.py (3)
write_atomic_text(219-239)select_table(176-192)update_dependency_table(97-110)
tests/bdd/steps/test_workspace_metadata_steps.py (3)
lading/workspace/metadata.py (1)
load_cargo_metadata(104-123)lading/workspace/models.py (2)
load_workspace(176-183)WorkspaceGraph(72-173)tests/helpers/workspace_helpers.py (1)
install_cargo_stub(15-38)
🔍 Remote MCP
Let me try searching with web search to gather relevant context about workspace dependencies and TOML manipulation patterns.## Summary of Additional Context Found
Based on my research, I have gathered relevant background context about workspace dependencies in Rust/Cargo that provides important context for reviewing this PR:
Workspace Dependencies Context
The [workspace] section in Cargo.toml can now have a dependencies section which works the same way as the [dependencies] section in Cargo.toml. Each workspace member can then reference this section in the workspace with a new dependency directive by setting { workspace = true }, which indicates that the dependency should be looked up from workspace.dependencies in the workspace root.
Workspace inheritance can include things like shared version numbers, repository URLs, or rust-version, which also helps keep these values in sync between crates when updating them.
Multiple Dependency Section Types
The PR extends bump to handle three types of workspace dependency sections, which mirrors standard Cargo structure. The affected sections are analogous to their top-level counterparts:
[workspace.dependencies]- regular dependencies[workspace.dev-dependencies]- development dependencies[workspace.build-dependencies]- build script dependencies
Code Refactoring Pattern
The PR implements a sensible architectural pattern by extracting TOML manipulation logic into dedicated modules:
- bump_toml.py - Handles low-level TOML parsing, table navigation, and dependency updates (utilizing the
tomlkitlibrary for TOML operations) - bump_docs.py - Processes Markdown documentation files with embedded TOML code fences for updating dependency snippets
- bump.py - Delegates to these modules while maintaining backward compatibility through re-exports
This modularization addresses code maintainability by separating concerns: manifest manipulation, documentation processing, and command orchestration.
Key Review Considerations
The refactoring touches on several important aspects:
- TOML structure preservation: The code uses
tomlkitto maintain formatting and comments when updating versions - Atomic writes: Files are written atomically to prevent corruption during failures
- Dry-run support: Documentation updates respect a dry-run flag
- Backward compatibility: Tests continue to work through re-exported functions via aliases
[::web_search::], with reference to Cargo workspace documentation standards and workspace dependencies feature behavior
⏰ 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 (43)
docs/scripting-standards.md (1)
36-39: LGTM!The reformatting maintains readability whilst adhering to documentation wrapping standards. The cuprum description remains clear and accurate.
docs/lading-design.md (1)
713-714: LGTM!The reformatting improves consistency in the section whilst preserving the technical content. The description of BDD scenarios remains clear.
tests/bdd/steps/test_common_steps.py (3)
9-9: LGTM!The runtime import of
Pathis necessary for the dynamic feature directory resolution. The change aligns with coding guidelines mandating pathlib for cross-platform path manipulation.
16-16: LGTM!The dynamic feature directory resolution is correctly implemented using pathlib operations. The leading underscore appropriately indicates module-private scope.
18-18: LGTM!The dynamic path resolution for the scenarios file improves maintainability and robustness. The pattern is consistent across the test suite.
docs/users-guide.md (1)
141-145: LGTM!The table formatting adjustment improves clarity by explicitly listing the valid values for
strip_patchesin a "one of" format. The documentation remains accurate and well-structured.lading/commands/bump_toml.py (14)
1-29: LGTM!The module header, imports, and type definitions are well-structured. The TYPE_CHECKING pattern correctly avoids runtime imports whilst maintaining type safety. The
_TableLiketype alias and corresponding runtime tuple provide clean abstraction for tomlkit's table types.
31-37: LGTM!The
value_as_stringhelper cleanly extracts string values from tomlkit Items or plain strings. The implementation correctly handles both cases with appropriate type narrowing.
39-48: LGTM!The
compose_requirementfunction correctly preserves version operators (^, ~, etc.) from the existing requirement string. The regex-based prefix extraction handles edge cases appropriately.
50-66: LGTM!The
prepare_version_replacementfunction correctly handles version updates whilst preserving TOML formatting trivia. The defensive suppression ofAttributeErrorduring trivia copying is appropriate given tomlkit's internals.
68-79: LGTM!The
assign_dependency_version_fieldfunction correctly updates version fields in dependency tables. The boolean return value clearly indicates whether a change occurred.
81-95: LGTM!The
update_dependency_entryfunction correctly handles both string and table-based dependency specifications. The comment on line 93 helpfully clarifies OutOfOrderTableProxy's item assignment support.
97-111: LGTM!The
update_dependency_tablefunction correctly iterates through dependency names and applies updates. The accumulation of changes via thechangedflag is appropriate for reporting whether any updates occurred.
113-136: LGTM!The
update_sectionfunction provides a clean abstraction for updating dependencies at a specific table path. The comprehensive docstring follows NumPy format as required by coding guidelines.
138-168: LGTM!The
update_dependency_sectionsfunction correctly implements workspace section updates. Theinclude_workspace_sectionsflag provides explicit control over whether[workspace.dependencies],[workspace.dev-dependencies], and[workspace.build-dependencies]sections are updated alongside their top-level counterparts. The default ofFalsemaintains backward compatibility.
170-174: LGTM!The
parse_manifestfunction provides a clean wrapper for loading TOML manifests with proper UTF-8 encoding.
176-193: LGTM!The
select_tablefunction correctly navigates nested table structures using duck typing for the.getmethod. The defensive approach with type checking at each step prevents runtime errors from malformed documents.
195-210: LGTM!The
assign_versionfunction correctly updates version fields whilst preserving TOML formatting trivia. The fallback to plain assignment for non-Item values ensures broad compatibility.
212-217: LGTM!The
value_matchesfunction provides a clean abstraction for comparing values that may be tomlkit Items or plain Python values.
219-239: LGTM!The
write_atomic_textfunction correctly implements atomic file writes using the temp-file-and-replace pattern. The best-effort permission preservation (line 233) appropriately handles Windows compatibility viasuppress(AttributeError). The cleanup in the finally block (lines 237-239) is defensive against edge cases where the replace might fail.tests/bdd/steps/test_commands_catalogue_steps.py (3)
30-30: LGTM!The runtime import of
Pathis consistent with the pattern established in other BDD test modules and necessary for dynamic feature directory resolution.
40-40: LGTM!The
_FEATURES_DIRcomputation follows the same pattern as other BDD test modules, providing consistent feature file resolution across the test suite.
42-42: LGTM!The dynamic path resolution for the scenarios file is consistent with the pattern across the BDD test suite.
tests/bdd/steps/test_workspace_metadata_steps.py (3)
8-8: LGTM!The runtime import of
Pathmaintains consistency with the pattern established across the BDD test suite.
21-21: LGTM!The
_FEATURES_DIRcomputation is consistent with the pattern across the BDD test suite.
23-23: LGTM!The dynamic path resolution for the scenarios file maintains consistency across the test suite.
tests/unit/test_bump_command_internals.py (4)
443-457: LGTM!The test correctly validates that
include_workspace_sections=Trueupdates both top-level and workspace dependency sections. The assertions verify that version operators are preserved (^1.0.0) whilst applying the target version.
460-475: LGTM!The test correctly validates that
include_workspace_sections=Falsepreserves workspace dependency sections unchanged. This is crucial for backward compatibility. The inline comment on line 474 helpfully clarifies the expected behaviour.
478-488: LGTM!The test correctly validates the edge case where only workspace dependency sections exist. This ensures the implementation handles workspace-centric manifests correctly.
491-506: LGTM!The test comprehensively validates workspace dev-dependencies and build-dependencies updates. The coverage of both string (
~0.1.0) and table ({ version = "0.1.0" }) dependency formats ensures robustness across different Cargo manifest styles.tests/unit/test_bump_command_integration.py (2)
301-342: Well-structured parametrized test.The consolidation of workspace dependency section tests into a single parametrized function with tuple unpacking is clean. The handling of both string and table formats via
isinstancecheck is appropriate.One minor observation: the inline comment at line 337 is helpful for maintainers understanding the dual format handling.
345-370: Good coverage for prefix and extra field preservation.This test validates an important edge case: ensuring version prefixes (e.g.,
^,~) and additional fields (e.g.,path) are preserved when updating workspace dependencies. The assertions are specific and cover both simple string and table entry formats.lading/commands/bump.py (4)
10-10: Clean module delegation.The import of
bump_docsandbump_tomlestablishes a clear separation of concerns: TOML manipulation in one module, documentation processing in another.
191-199: Delegation to bump_docs is straightforward.The calls to
bump_docs.resolve_documentation_targetsandbump_docs.update_documentation_filescleanly pass through the required parameters without unnecessary transformation.
359-372: Manifest update delegation is correct.The delegation to
bump_tomlfunctions (parse_manifest,select_table,assign_version,update_dependency_sections,write_atomic_text) maintains the same logic flow whilst offloading implementation details to the dedicated module.
414-420: Re-exports preserve backward compatibility.Re-exporting internal functions from
bump_tomlmaintains compatibility with existing tests that access them viabump._function_name(). Tests intests/unit/test_bump_command_internals.pydirectly referencebump._select_table(),bump._assign_version(),bump._value_matches(), andbump._update_dependency_sections(). These re-exports must remain in place to prevent test failures. Document that these aliases exist solely for test compatibility and may be removed once tests are refactored to import directly frombump_toml.lading/commands/bump_docs.py (7)
1-1: Module docstring is adequate.The single-line docstring suffices for a focused utility module. Per coding guidelines, consider expanding if the module grows in complexity.
26-39: Glob resolution is correct.The function correctly handles empty patterns, deduplicates via
set, and filters to files only. The return type astuple[Path, ...]ensures immutability.
42-62: File update loop is sound.The dry-run check correctly prevents writes, and the atomic write via
bump_toml.write_atomic_textensures file integrity.
65-83: Nonlocal mutation for change tracking is acceptable.The nested
_applyfunction withnonlocal changedis a pragmatic approach for tracking whether any transformation occurred. This pattern avoids more complex state threading.
86-105: Fence replacement logic is correct.The function correctly preserves content outside matched fences by tracking
last_indexand appending remaining lines at the end. The use ofsplitlines(keepends=True)preserves line endings.
117-139: Fence rendering preserves structure correctly.The function handles indentation, fence markers, info strings, and trailing newlines appropriately. The regex for suffix matching is robust.
22-23: Runtime placeholders are acceptable but consider importing at runtime.The pattern of assigning
typ.Anyat runtime for type-only imports is functional. An alternative is conditional import within function bodies, but this approach is cleaner for module-level type annotations.
Replace `any()` and `or` chains with explicit loops to ensure all sections are always processed. The short-circuit evaluation would stop after the first True result, preventing later dependency sections from being updated. 🤖 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: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lading/commands/bump_docs.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
lading/commands/bump_docs.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:
lading/commands/bump_docs.py
🧬 Code graph analysis (1)
lading/commands/bump_docs.py (2)
lading/config.py (1)
DocumentationConfig(55-72)lading/commands/bump_toml.py (3)
assign_version(195-209)select_table(176-192)update_dependency_table(97-110)
🔍 Remote MCP Deepwiki
Summary of additional repository facts relevant to reviewing this PR
-
bump now updates workspace-level dependency sections ([workspace.dependencies], [workspace.dev-dependencies], [workspace.build-dependencies]) in addition to top-level sections; the planner/updates use the WorkspaceGraph metadata (cargo metadata → WorkspaceGraph) and map dependency.kind → manifest section names.
-
TOML and Markdown processing were extracted into two new modules used by bump:
- bump_toml: tomlkit-based parsing/updates, functions for assign_version, update_dependency_table/sections, write_atomic_text (atomic file writes that preserve permissions).
- bump_docs: markdown-it-py token-based fence rewriting for TOML code fences; updates package/workspace.package versions and dependency tables inside fences; supports dry-run and preserves fence indentation/trailing newlines.
-
Files are written atomically (temporary file + Path.replace) and tomlkit trivia is preserved where possible; I/O functions suppress certain errors but return explicit change indicators.
-
Tests added/modified:
- Unit tests for include_workspace_sections behavior and dependency-section updates.
- Integration tests expanded to cover workspace.* sections and workspace dependency prefixes.
- BDD tests and test fixtures use cmd-mox stubs for cargo/git invocations; tests set LADING_USE_CMD_MOX_STUB and CMOX_IPC_SOCKET to stub external commands.
-
Configuration/behavioral notes:
- bump respects bump.exclude (crate version not updated) but still updates dependency requirements referencing bumped crates.
- bump.documentation.globs resolves globs relative to workspace root and only updates files that are regular files.
-
Known reviewer concerns / code-quality signals recorded in PR comments:
- Nested conditional complexity in _update_dependency_sections and cyclomatic complexity in bump_docs; concrete refactor sketches were proposed (extract helpers like _update_section, move flags into options dataclass, parametrize duplicated tests). These are already noted as suggested refactors in the PR description.
Sources used
- Repository documentation and command/module summaries (generated wiki):
⏰ 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 (8)
lading/commands/bump_docs.py (8)
1-24: LGTM: Module structure and imports are correct.The module docstring, future annotations import, and TYPE_CHECKING block follow best practices. Runtime placeholders prevent import-time side effects.
42-62: LGTM: File update logic is correct.The function correctly processes documentation files, rewrites TOML fences, and writes atomically when not in dry-run mode. The defensive check on line 51 (
if name) is harmless.
65-83: LGTM: Fence rewriting logic is sound.The closure pattern correctly tracks changes across fence transformations. The delegation to
replace_markdown_fenceswith a transform callback is clean.
86-105: LGTM: Markdown fence replacement logic is correct.The token-based reconstruction preserves non-fence content and selectively transforms matching language fences. The line index arithmetic for slicing and reassembly is sound.
108-146: LGTM: Fence rendering and helper utilities are correct.The functions correctly handle case-insensitive language matching, preserve indentation and trailing newlines, and reconstruct fence blocks accurately.
148-169: LGTM: Private helper functions are well-factored.These thin wrappers over
bump_tomlutilities are appropriately scoped. Single-line docstrings are acceptable for private functions per guidelines.
172-187: Approved: Short-circuit bug fixed.The explicit loop now processes all three dependency sections (
dependencies,dev-dependencies,build-dependencies) without short-circuiting. This correctly addresses the critical issue flagged in previous reviews whereany()would stop after the firstTrueresult.
190-211: Approved: Short-circuit bug fixed.The separate
ifstatements (lines 201-207) ensure all three update operations execute and accumulate changes correctly. This addresses the critical issue from previous reviews where anorchain would short-circuit and skip later updates.
- Add NumPy-style docstrings with Parameters, Returns, and Raises sections to all public functions in bump_docs.py - Simplify redundant ternary expression in update_toml_snippet_versions 🤖 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.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
lading/commands/bump_docs.py
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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:
lading/commands/bump_docs.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:
lading/commands/bump_docs.py
🧬 Code graph analysis (1)
lading/commands/bump_docs.py (2)
lading/config.py (1)
DocumentationConfig(55-72)lading/commands/bump_toml.py (4)
write_atomic_text(219-239)assign_version(195-209)select_table(176-192)update_dependency_table(97-110)
⏰ 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 (10)
lading/commands/bump_docs.py (10)
26-53: LGTM!The function has a complete NumPy-style docstring, correctly deduplicates paths using a set, and returns an immutable tuple. The implementation is clean and follows best practices.
97-132: LGTM!The closure-based approach for tracking changes across transform calls is appropriate. The function delegates correctly to
replace_markdown_fencesandupdate_toml_snippet_versions.
135-170: LGTM!The token-based fence replacement logic correctly preserves non-fence content and handles the reconstruction via line slicing. The approach is sound.
173-193: LGTM!The function correctly handles edge cases (empty info, case differences) and follows a clear, defensive pattern.
196-241: LGTM!The function correctly preserves indentation, trailing newlines, and fence markers. The explicit
ValueErrorfor missing map data is appropriate defensive programming.
244-261: LGTM!Simple and correct implementation for extracting leading whitespace.
264-272: LGTM!Clean composition of
select_tableandassign_versionfrombump_toml.
275-285: LGTM!Correct early return pattern and clean delegation to
bump_toml.update_dependency_table.
288-319: LGTM!The explicit loop with
changedflag correctly addresses the previous short-circuiting bug. All three dependency sections are now guaranteed to be processed.
322-364: LGTM!The function correctly addresses previous review feedback:
- Explicit
ifstatements replace the short-circuitingorchain- All update paths are guaranteed to execute
- Trailing newline preservation is handled correctly
- The return statement has been simplified
…ation_files Add error handling for OSErrors when reading documentation files, logging a warning and skipping unreadable files instead of failing. This improves robustness when processing multiple documentation files during version bumps. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.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 Functionality
Tests
API/Usage
Test plan
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/dfecc461-6419-4fcf-be45-ad183a16da57
Summary by Sourcery
Refactor the bump command to delegate TOML and documentation version-updating logic to dedicated modules while extending dependency bumping to workspace-level sections.
Enhancements:
Tests: