Refactor module#7
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe CLI was split into argument parsing, help text, and version lookup modules. Source-file discovery now has explicit and recursive paths. Language duplicate-mitigation data and classification moved into language submodules, and crate-level wiring now uses the new discovery and report entry points. ChangesDuplicate-report pipeline refactor
Sequence Diagram(s)sequenceDiagram
participant run
participant discovery as discovery::changed_files_against_origin
participant report as report::detect_duplicate_blocks
run->>discovery: current_dir
discovery-->>run: changed files
run->>report: processed_files
report-->>run: duplicate blocks
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/cli/args.rs (1)
158-168: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow the declared
--codem8-*long options through normalization.Line 167 currently turns the long flags defined on Lines 14-30 into
unknown argumenterrors, so standard invocations like--codem8-verbose,--codem8-git-branch,--codem8-file-extension rs, and--codem8-files pathnever reach Clap.Suggested fix
fn normalized_clap_arg(arg: String) -> Result<String> { if arg == "-verbose" { Ok("--codem8-verbose".to_owned()) } else if arg == "-git-branch" { Ok("--codem8-git-branch".to_owned()) + } else if matches!( + arg.as_str(), + "--report-duplicate" + | "--codem8-verbose" + | "--codem8-git-branch" + | "--codem8-file-extension" + | "--codem8-files" + ) || arg.starts_with("--codem8-file-extension=") + || arg.starts_with("--codem8-files=") + { + Ok(arg) } else if let Some(value) = arg.strip_prefix("-file-extension=") { Ok(format!("--codem8-file-extension={value}")) } else if let Some(value) = arg.strip_prefix("-files=") { Ok(format!("--codem8-files={value}")) } else if arg.starts_with("--") && arg != "--report-duplicate" { Err(CodeM8Error::new(format!("unknown argument: {arg}"))) } else { Ok(arg) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/args.rs` around lines 158 - 168, The normalized_clap_arg helper is rejecting valid declared long options by treating every "--" argument except "--report-duplicate" as unknown. Update normalized_clap_arg so the existing Clap-defined "--codem8-*" flags are allowed through unchanged (or normalized consistently), including the options handled by Args/Clap such as verbose, git-branch, file-extension, and files, while still rejecting truly unknown long arguments.
🧹 Nitpick comments (1)
src/cli/version.rs (1)
1-13: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse Cargo package metadata as the version source.
help_text()renders this helper directly, so the CLI banner now depends onCargo.lockstaying in sync withCargo.toml. That adds a second source of truth for release metadata and can show the wrong version after a future bump.env!("CARGO_PKG_VERSION")is tied to the compiled crate and removes the lockfile parser entirely.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/cli/version.rs` around lines 1 - 13, The version helper in codem8_version_from_cargo_lock should stop reading from Cargo.lock and use Cargo package metadata instead. Replace the lockfile-based lookup used by help_text() with the compiled crate version source so the CLI banner always matches the built package, and remove the CargoLockPackage/cargo_lock_packages parsing path if it is no longer needed.
🤖 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 `@src/cli/help.rs`:
- Around line 6-12: Update the help documentation in the help output so the
supported `-h` entrypoint is advertised alongside `help`. In `parse_command` and
the help text emitted from `src/cli/help.rs`, make sure the usage and command
list clearly show both invocations, keeping the existing `help` entry and adding
`-h` as an equivalent supported command so it is discoverable.
In `@src/discovery/explicit.rs`:
- Around line 56-60: The `display_path` logic in `explicit.rs` is relativizing
`canonical_path` against an unnormalized `current_dir`, so `strip_prefix` can
fail for symlinked or non-canonical workspaces and leak absolute paths.
Canonicalize or otherwise normalize `current_dir` before the
`canonical_path.strip_prefix(...)` check inside the `display_path` computation,
so `normalize_display_path` consistently returns stable relative paths for
`src/line.rs` and similar inputs.
In `@src/language/patterns.rs`:
- Around line 141-143: The Rust mitigation regex in the pattern set is too broad
because the optional separator in the expression also matches struct-literal
field labels like field: value,. Tighten the pattern used in the Rust rule so it
only matches path or enum-fragment forms with :: and does not allow a lone
colon; update the regex in the Rust patterns entry that is consumed by
classification::register_rust_rules so the comment about path/enum fragments
remains accurate.
---
Outside diff comments:
In `@src/cli/args.rs`:
- Around line 158-168: The normalized_clap_arg helper is rejecting valid
declared long options by treating every "--" argument except
"--report-duplicate" as unknown. Update normalized_clap_arg so the existing
Clap-defined "--codem8-*" flags are allowed through unchanged (or normalized
consistently), including the options handled by Args/Clap such as verbose,
git-branch, file-extension, and files, while still rejecting truly unknown long
arguments.
---
Nitpick comments:
In `@src/cli/version.rs`:
- Around line 1-13: The version helper in codem8_version_from_cargo_lock should
stop reading from Cargo.lock and use Cargo package metadata instead. Replace the
lockfile-based lookup used by help_text() with the compiled crate version source
so the CLI banner always matches the built package, and remove the
CargoLockPackage/cargo_lock_packages parsing path if it is no longer needed.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 822da5ec-b765-4594-a949-09da04fe84fa
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
AGENTS.mdCargo.tomlsrc/cli/args.rssrc/cli/help.rssrc/cli/mod.rssrc/cli/version.rssrc/discovery.rssrc/discovery/explicit.rssrc/discovery/git.rssrc/discovery/mod.rssrc/discovery/recursive.rssrc/language.rssrc/language/classification.rssrc/language/mod.rssrc/language/patterns.rssrc/language/registry.rssrc/lib.rssrc/report/duplicate_detection.rssrc/report/duplicate_renderer.rssrc/report/mod.rs
💤 Files with no reviewable changes (2)
- src/discovery.rs
- src/language.rs
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: b4prog/CodeM8
Timestamp: 2026-06-26T14:42:45.825Z
Learning: Before finishing a change, run `cargo fmt --all -- --check` from the workspace root and fix any formatting issues before handing work back.
Learnt from: CR
Repo: b4prog/CodeM8
Timestamp: 2026-06-26T14:42:45.825Z
Learning: Before finishing a change, run `cargo test` from the workspace root and fix any test failures before handing work back.
Learnt from: CR
Repo: b4prog/CodeM8
Timestamp: 2026-06-26T14:42:45.825Z
Learning: Before finishing a change, run `cargo clippy --workspace --all-targets --all-features -- -D warnings -W clippy::too_many_lines -W clippy::too_many_arguments -W clippy::type_complexity -W clippy::excessive_nesting -W clippy::cognitive_complexity -W clippy::pedantic -W clippy::nursery -W clippy::cargo` from the workspace root and fix any Clippy issues before handing work back.
Learnt from: CR
Repo: b4prog/CodeM8
Timestamp: 2026-06-26T14:42:45.825Z
Learning: Before finishing a change, run `cargo build --locked --all-targets` from the workspace root and fix any build issues before handing work back.
Learnt from: CR
Repo: b4prog/CodeM8
Timestamp: 2026-06-26T14:42:45.825Z
Learning: Treat Clippy warnings as errors for generated or edited code.
Learnt from: CR
Repo: b4prog/CodeM8
Timestamp: 2026-06-26T14:42:45.825Z
Learning: Prefer changes that satisfy the repository `clippy.toml` configuration without adding `#[allow(...)]` attributes unless a maintainer explicitly asks for them.
Learnt from: CR
Repo: b4prog/CodeM8
Timestamp: 2026-06-26T14:42:45.825Z
Learning: If a required command cannot be run in the current environment, call that out clearly in the handoff.
🔇 Additional comments (5)
src/cli/help.rs (1)
50-89: LGTM!src/discovery/mod.rs (1)
19-41: LGTM!src/discovery/recursive.rs (1)
25-161: LGTM!src/report/mod.rs (1)
1-5: LGTM!src/lib.rs (1)
35-50: LGTM!
Owner
Author
|
@CodeRabbit resume |
✅ Action performedReviews resumed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
0.7.0.