Add DeepWiki README badge and fix optional cmd-mox typing for ty checker - #64
Conversation
`ty` rejects assigning `None` to a name typed as the imported `command_runner` module under `ModuleNotFoundError`. Give `cmd_runner_module` an explicit `types.ModuleType | None` annotation and load the module in try/else so both branches remain well typed. Keep `types` under `TYPE_CHECKING` to satisfy ruff import rules and group the block with other typing-only imports. Co-authored-by: Cursor <cursoragent@cursor.com>
Link the repository overview on DeepWiki for discoverability from the main readme. Co-authored-by: Cursor <cursoragent@cursor.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
ChangesThis PR adds a DeepWiki documentation badge to the README and makes typing/import changes in lading/commands/publish_execution.py so the optional cmd-mox dependency is accepted as absent by the type-checker and avoids runtime NameError when annotations are evaluated. README.md
lading/commands/publish_execution.py
Issues and follow-ups
ValidationAll checks passed:
WalkthroughSummarise two unrelated edits: a DeepWiki badge was added to the README header; separately, ChangesDocumentation Badge
Publish Execution: Import and Typing Adjustments
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 inconclusive)
✅ Passed checks (17 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a DeepWiki documentation badge to the README and refactors the optional cmd-mox import in publish_execution.py to be explicitly typed and type-checker-friendly when the dependency is absent. Flow diagram for optional cmd_mox import and typing in publish_executionflowchart TD
A[Module_imports_publish_execution] --> B[Initialize_cmd_runner_module_annotation]
B --> C{Import_cmd_mox_command_runner}
C -->|Success| D[Assign__cmd_runner_module_to_cmd_runner_module]
C -->|ModuleNotFoundError| E[Set_cmd_runner_module_to_None]
D --> F[Runtime_functions_use_cmd_runner_module_if_not_None]
E --> F
subgraph Type_checking
G[TYPE_CHECKING_block_imports_types_and_PublishPreflightError]
H[cmd_runner_module_annotated_as_ModuleType_or_None]
end
A --> G
G --> H
H --> C
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
@coderabbitai please add a GitHub issue for configurable lockfile rebuild mechanism following bump that allows the rebuild of lockfiles using, for example: Add a cargo generate-lockfile --manifest-path crates/rstest-bdd/tests/ui_lints/Cargo.tomlAn example configuration might read: lockfile_manifests = [
"crates/rstest-bdd/tests/ui_lints/Cargo.toml",
]Cargo.toml itself need not be specified as it is assumed. A separate |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 43
It covers all four proposed changes:
|
|
@coderabbitai please create a new GitHub issue for the following: When using the workspace README adoption feature, relative links in the Markdown file should be updated to reflect the correct location. README.md transposition should be done at bump time rather than publish time to enable this. Because this README lives under |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 43
It covers both required changes:
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The
cmd_runner_module: types.ModuleType | Noneannotation referencestypesat runtime whiletypesis only imported underTYPE_CHECKING, which can raise aNameErrorunlessfrom __future__ import annotationsis guaranteed for this module; consider either importingtypesunconditionally or using a string annotation /typingalias to keep it purely type-checker-only.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `cmd_runner_module: types.ModuleType | None` annotation references `types` at runtime while `types` is only imported under `TYPE_CHECKING`, which can raise a `NameError` unless `from __future__ import annotations` is guaranteed for this module; consider either importing `types` unconditionally or using a string annotation / `typing` alias to keep it purely type-checker-only.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Review feedback noted that `types.ModuleType` annotations can fail with `NameError` if `types` is only loaded under `TYPE_CHECKING` and deferral does not apply everywhere. Keep stdlib `types` material at module scope and document the rationale next to the guarded ruff `TC003` suppression. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@coderabbitai Have the following now been resolved?
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 1551
The module docstring records the reasoning explicitly, so future maintainers know why |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lading/commands/publish_execution.py`:
- Around line 1-7: Replace the current module docstring with a purpose-first
docstring that explains this module provides command-execution helpers for
publish operations (brief description, main utilities, and key exports such as
split_command, should_use_cmd_mox_stub, normalise_cmd_mox_command and when to
use them), and move the implementation-detail about keeping the stdlib types
import at module scope into an inline comment adjacent to the types import
(leave the import as-is and add a short comment: why module-scope types is
required so cmd_runner_module: types.ModuleType | None annotations won’t raise
NameError).
🪄 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: b95066ff-c032-4f48-994d-026f780601cf
📒 Files selected for processing (1)
lading/commands/publish_execution.py
Describe ``_invoke``, the cmd-mox routing story and the exported helper aliases. Keep the stdlib ``types`` rationale next to the import so the import block stays isort-clean. Co-authored-by: Cursor <cursoragent@cursor.com>
Summary
This branch adds the DeepWiki documentation badge next to the readme title so
that readers can open the DeepWiki-hosted repository overview. It also
adjusts the optional
cmd-moxloader in publish execution sotyacceptsNonewhen the dependency is absent, restoring a cleanmake typecheckgatealongside existing ruff rules.
There is no associated roadmap task, execplan or issue.
Review walkthrough
lading/commands/publish_execution.pyfor the annotated optional import,TYPE_CHECKING-scopedtypesimport and try/else loading pattern.Validation
make buildmake check-fmtmake lintmake typecheckmake test: 432 passedmarkdownlint README.mdRepository-wide
make markdownlintstill reports pre-existing MD013 line-lengthfindings elsewhere; only
README.mdwas covered for markdownlint for this change.Notes
https://github.com/leynos/lading/pull/new/docs/readme-deepwiki-badgeas the PR creation shortcut.Summary by Sourcery
Add a DeepWiki documentation badge to the README and tighten typing around the optional cmd-mox dependency in publish execution.
New Features:
Bug Fixes:
Enhancements: