Conversation
…nt script-tool mapping and boundaries; bump version to 0.0.9
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds version 0.0.9 metadata, introduces two CLI options ( Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (CLI)
participant Script as post_process.py
participant MCP as MCP Tools
participant FS as Filesystem
User->>Script: invoke with flags (--dir-rule/--base-dir/--outdir)
Script->>Script: validate flags (pairing & mutual exclusion)
alt invalid flags
Script->>User: exit with error (invalid combination)
else valid flags
Script->>MCP: prepare params (dir_rule or converted from outdir)
MCP->>FS: request/write output paths per dir_rule
MCP->>User: return status/results
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip You can get early access to new features in CodeRabbit.Enable the |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/jmcomic_ai/skills/jmcomic/SKILL.md (1)
422-451:⚠️ Potential issue | 🟡 MinorDocument the full output-path flag contract here.
This adds the happy-path
--dir-ruleexample, but it still omits the actual CLI contract:--dir-ruleand--base-dirmust be passed together, both are mutually exclusive with--outdir, and--outdiris the shorthand for{"rule": "Bd", "base_dir": ...}. Without that, agents will keep generating commands the script rejects. As per coding guidelines,Every new tool or parameter change must be reflected in src/jmcomic_ai/skills/jmcomic/SKILL.md - this file is the agent manual documenting all capabilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jmcomic_ai/skills/jmcomic/SKILL.md` around lines 422 - 451, The SKILL.md post_process section omits the precise CLI contract for output path flags; update the post_process.py documentation in src/jmcomic_ai/skills/jmcomic/SKILL.md to state that --dir-rule and --base-dir must always be provided together, they are mutually exclusive with --outdir, and that --outdir is a shorthand equivalent to providing params {"rule": "Bd", "base_dir": "<path>"}; include a concise example for each case (happy-path with --dir-rule + --base-dir, shorthand with --outdir), and mention the script validation behavior (rejects mixed usage and missing pair) so agents generate only accepted flag combinations.
🧹 Nitpick comments (1)
src/jmcomic_ai/skills/jmcomic/scripts/post_process.py (1)
39-42: Giveparamsan explicit type before growing this branch.
paramsnow carriesstr,bool, and nested dict values, butmainremains untyped, so mypy still will not meaningfully check this path. Please annotatemain() -> None, typeparams(dict[str, Any]or a smallTypedDict), and run mypy on this file so the newdir_rulepayload is actually covered. As per coding guidelines,src/**/*.py: Use static type hints and verify withmypy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/jmcomic_ai/skills/jmcomic/scripts/post_process.py` around lines 39 - 42, Annotate main() with a return type (main() -> None) and give params an explicit type (for example declare params: dict[str, Any] or create a small TypedDict for the expected keys) before the branch that assigns dir_rule; update imports to include typing.Any or TypedDict as needed, and ensure the dir_rule value conforms to that typed shape (refer to the params variable and the main function in post_process.py and the dir_rule payload assigned in the if/elif block); finally run mypy on this file and fix any resulting type errors so the new dir_rule path is statically checked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/jmcomic_ai/skills/jmcomic/scripts/post_process.py`:
- Around line 30-37: The flag-combination checks run after constructing
JmcomicService so invalid CLI invocations can initialize the service before
failing; move or duplicate the parser.error checks to run before the
JmcomicService(args.option) instantiation (i.e., validate args.outdir,
args.dir_rule and args.base_dir immediately after parsing) so parser.error is
invoked prior to creating JmcomicService; ensure you reference the same
conditions (args.outdir && (args.dir_rule || args.base_dir), args.dir_rule &&
!args.base_dir, args.base_dir && !args.dir_rule) and keep the existing
parser.error messages.
---
Outside diff comments:
In `@src/jmcomic_ai/skills/jmcomic/SKILL.md`:
- Around line 422-451: The SKILL.md post_process section omits the precise CLI
contract for output path flags; update the post_process.py documentation in
src/jmcomic_ai/skills/jmcomic/SKILL.md to state that --dir-rule and --base-dir
must always be provided together, they are mutually exclusive with --outdir, and
that --outdir is a shorthand equivalent to providing params {"rule": "Bd",
"base_dir": "<path>"}; include a concise example for each case (happy-path with
--dir-rule + --base-dir, shorthand with --outdir), and mention the script
validation behavior (rejects mixed usage and missing pair) so agents generate
only accepted flag combinations.
---
Nitpick comments:
In `@src/jmcomic_ai/skills/jmcomic/scripts/post_process.py`:
- Around line 39-42: Annotate main() with a return type (main() -> None) and
give params an explicit type (for example declare params: dict[str, Any] or
create a small TypedDict for the expected keys) before the branch that assigns
dir_rule; update imports to include typing.Any or TypedDict as needed, and
ensure the dir_rule value conforms to that typed shape (refer to the params
variable and the main function in post_process.py and the dir_rule payload
assigned in the if/elif block); finally run mypy on this file and fix any
resulting type errors so the new dir_rule path is statically checked.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98b17d7e-7a22-4b5f-bdc2-82e87b71077e
📒 Files selected for processing (5)
CHANGELOG.mdpyproject.tomlsrc/jmcomic_ai/__init__.pysrc/jmcomic_ai/skills/jmcomic/SKILL.mdsrc/jmcomic_ai/skills/jmcomic/scripts/post_process.py
…nt script-tool mapping and boundaries; bump version to 0.0.9
描述
相关 Issue
Closes #
更改类型
测试
检查清单
截图(如适用)
其他信息
Summary by CodeRabbit
New Features
--dir-ruleand--base-dirparameters for precise post-processing output path control via a dir_rule DSL.Documentation
Chores