Conversation
Reviewer's GuideEnhances PR comment listing with optional file-based filtering by introducing a new helper, extending the CLI to accept file paths, integrating the filter into the PR command flow, and updating documentation to describe the feature. Class diagram for updated PrArgs and review thread filteringclassDiagram
class PrArgs {
Option<String> reference
Vec<String> files
}
class ReviewThread {
CommentConnection comments
}
class CommentConnection {
Vec<ReviewComment> nodes
}
class ReviewComment {
String path
...
}
PrArgs --> ReviewThread : used for filtering
ReviewThread --> CommentConnection
CommentConnection --> ReviewComment
class filter_threads_by_files {
+filter_threads_by_files(threads: Vec<ReviewThread>, files: &[String]) -> Vec<ReviewThread>
}
filter_threads_by_files ..> ReviewThread : filters
filter_threads_by_files ..> PrArgs : uses files
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
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
WalkthroughAdd optional file-path filtering to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant Main
participant ReviewThreads
User->>CLI: Run `vk pr <url|number> [file1 file2 ...]`
CLI->>Main: Parse args into PrArgs (includes files)
Main->>ReviewThreads: fetch_review_threads()
Main->>ReviewThreads: filter_threads_by_files(threads, files)
ReviewThreads-->>Main: Return filtered threads
Main->>CLI: Render comments for filtered threads
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @leynos - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/review_threads.rs:218` </location>
<code_context>
+ .filter(|t| {
+ t.comments
+ .nodes
+ .first()
+ .is_some_and(|c| set.contains(c.path.as_str()))
+ })
+ .collect()
</code_context>
<issue_to_address>
Only the first comment's path is considered; clarify if this matches intended behavior.
If you intend to match any comment's path in a thread, iterate over all nodes instead of just the first. Otherwise, this approach is correct.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Greptile Summary
This PR extends the vk pr command to support file-based filtering of pull request review comments. The change allows users to specify one or more file paths as arguments to restrict the output to only show comments for those specific files, which is particularly useful for developers or AI agents who want to focus on comments related to specific files they're working on.
The implementation adds a new files field to the PrArgs struct in cli_args.rs that accepts zero or more file path arguments via clap's command-line parsing. The core filtering logic is implemented through a new filter_threads_by_files function in review_threads.rs that uses a HashSet for efficient path lookup and filters threads based on the file path of the first comment in each thread. The filtering is applied in main.rs after fetching review threads but before processing them for display.
The feature maintains backward compatibility by making file arguments optional - when no files are specified, all review threads are returned unchanged. The implementation integrates cleanly with the existing codebase architecture, following established patterns for CLI argument handling and data processing. Comprehensive documentation updates ensure users understand the new functionality through updated README examples, usage syntax, and design documentation.
Important Files Changed
File Changes
| Filename | Score | Overview |
|---|---|---|
| src/cli_args.rs | 5/5 | Added files field to PrArgs struct with proper clap and serde attributes for CLI argument parsing |
| src/review_threads.rs | 5/5 | Implemented filter_threads_by_files function with HashSet-based filtering logic and comprehensive tests |
| src/main.rs | 4/5 | Integrated file filtering into pr command execution flow with proper imports and test coverage |
| README.md | 5/5 | Updated documentation to show file filtering syntax, usage examples, and feature description |
| docs/vk-design.md | 5/5 | Added targeted review feature documentation and updated PrArgs description to reflect file filtering |
Confidence score: 5/5
- This PR is safe to merge with minimal risk as it implements a straightforward additive feature with proper backward compatibility
- Score reflects well-structured implementation following established patterns, comprehensive testing, and thorough documentation updates
- No files require special attention as all changes follow good practices and include appropriate test coverage
5 files reviewed, no comments
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| #[test] | ||
| fn filter_threads_by_files_retains_matches() { | ||
| let threads = vec![ | ||
| ReviewThread { | ||
| comments: CommentConnection { | ||
| nodes: vec![ReviewComment { | ||
| path: "src/lib.rs".into(), | ||
| ..Default::default() | ||
| }], | ||
| ..Default::default() | ||
| }, | ||
| ..Default::default() | ||
| }, | ||
| ReviewThread { | ||
| comments: CommentConnection { | ||
| nodes: vec![ReviewComment { | ||
| path: "README.md".into(), | ||
| ..Default::default() | ||
| }], | ||
| ..Default::default() | ||
| }, | ||
| ..Default::default() | ||
| }, | ||
| ]; | ||
| let files = vec![String::from("README.md")]; | ||
| let filtered = filter_threads_by_files(threads, &files); | ||
| assert_eq!(filtered.len(), 1); | ||
| let path = filtered | ||
| .first() | ||
| .and_then(|t| t.comments.nodes.first()) | ||
| .map(|c| c.path.as_str()); | ||
| assert_eq!(path, Some("README.md")); | ||
| } |
There was a problem hiding this comment.
style: Consider adding a test case for empty files list to verify the early return behavior explicitly.
| #[test] | |
| fn filter_threads_by_files_retains_matches() { | |
| let threads = vec![ | |
| ReviewThread { | |
| comments: CommentConnection { | |
| nodes: vec![ReviewComment { | |
| path: "src/lib.rs".into(), | |
| ..Default::default() | |
| }], | |
| ..Default::default() | |
| }, | |
| ..Default::default() | |
| }, | |
| ReviewThread { | |
| comments: CommentConnection { | |
| nodes: vec![ReviewComment { | |
| path: "README.md".into(), | |
| ..Default::default() | |
| }], | |
| ..Default::default() | |
| }, | |
| ..Default::default() | |
| }, | |
| ]; | |
| let files = vec![String::from("README.md")]; | |
| let filtered = filter_threads_by_files(threads, &files); | |
| assert_eq!(filtered.len(), 1); | |
| let path = filtered | |
| .first() | |
| .and_then(|t| t.comments.nodes.first()) | |
| .map(|c| c.path.as_str()); | |
| assert_eq!(path, Some("README.md")); | |
| } | |
| #[test] | |
| fn filter_threads_by_files_retains_matches() { | |
| let threads = vec![ | |
| ReviewThread { | |
| comments: CommentConnection { | |
| nodes: vec![ReviewComment { | |
| path: "src/lib.rs".into(), | |
| ..Default::default() | |
| }], | |
| ..Default::default() | |
| }, | |
| ..Default::default() | |
| }, | |
| ReviewThread { | |
| comments: CommentConnection { | |
| nodes: vec![ReviewComment { | |
| path: "README.md".into(), | |
| ..Default::default() | |
| }], | |
| ..Default::default() | |
| }, | |
| ..Default::default() | |
| }, | |
| ]; | |
| let files = vec![String::from("README.md")]; | |
| let filtered = filter_threads_by_files(threads, &files); | |
| assert_eq!(filtered.len(), 1); | |
| let path = filtered | |
| .first() | |
| .and_then(|t| t.comments.nodes.first()) | |
| .map(|c| c.path.as_str()); | |
| assert_eq!(path, Some("README.md")); | |
| } | |
| #[test] | |
| fn filter_threads_by_files_returns_all_when_files_empty() { | |
| let threads = vec![ | |
| ReviewThread { | |
| comments: CommentConnection { | |
| nodes: vec![ReviewComment { | |
| path: "src/lib.rs".into(), | |
| ..Default::default() | |
| }], | |
| ..Default::default() | |
| }, | |
| ..Default::default() | |
| }, | |
| ReviewThread { | |
| comments: CommentConnection { | |
| nodes: vec![ReviewComment { | |
| path: "README.md".into(), | |
| ..Default::default() | |
| }], | |
| ..Default::default() | |
| }, | |
| ..Default::default() | |
| }, | |
| ]; | |
| let filtered = filter_threads_by_files(threads.clone(), &[]); | |
| assert_eq!(filtered.len(), threads.len()); | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
README.md(3 hunks)src/main.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Every module must begin with a module level (//!) comment explaining the module's purpose and utility.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Place function attributes after doc comments.
Do not usereturnin single-line functions.
Use predicate functions for conditional criteria with more than two branches.
Prefer immutable data and avoid unnecessarymutbindings.
Handle errors with theResulttype instead of panicking where feasible.
Prefer semantic error enums: Derivestd::error::Error(via thethiserrorcrate) for any condition the caller might inspect, retry, or map to an HTTP status.
Use an opaque error only at the app boundary: Useeyre::Reportfor human-readable logs; these should not be exposed in public APIs.
Never export the opaque type from a library: Convert to domain enums at API boundaries, and toeyreonly in the mainmain()entrypoint or top-level async task.
Clippy warnings MUST be disallowed.
Fix any warnings emitted during tests in the code itself rather than silencing them.
Where a function is too long, extract meaningfully named helper functions adhering to separation of concerns and CQRS.
Where a function has too many parameters, group related parameters in meaningfully named structs.
Where a function is returning a large error consider usingArcto reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Avoidunsafecode unless absolutely necessary and document any usage clearly.
Lints must not be silenced except as a last resort.
Lint rule suppressions must be tightly scoped and include a clear reason.
Preferexpectoverallow.
Prefer.expect()over.unwrap().
Useconcat!()to combine long string literals rather than escaping newlines with a backslash.
Files:
src/main.rs
⚙️ CodeRabbit Configuration File
**/*.rs: * Seek to keep the cyclomatic complexity of functions no more than 12.
Adhere to single responsibility and CQRS
Place function attributes after doc comments.
Do not use
returnin single-line functions.Move conditionals with >2 branches into a predicate function.
Avoid
unsafeunless absolutely necessary.Every module must begin with a
//!doc comment that explains the module's purpose and utility.Comments and docs must follow en-GB-oxendict (-ize / -our) spelling and grammar
Lints must not be silenced except as a last resort.
#[allow]is forbidden.- Only narrowly scoped
#[expect(lint, reason = "...")]is allowed.- No lint groups, no blanket or file-wide suppression.
- Include
FIXME:with link if a fix is expected.Where code is only used by specific features, it must be conditionally compiled or a conditional expectation for unused_code applied.
Use
rstestfixtures for shared setup and to avoid repetition between tests.Replace duplicated tests with
#[rstest(...)]parameterised cases.Prefer
mockallfor mocks/stubs.Prefer
.expect()over.unwrap()Ensure that any API or behavioural changes are reflected in the documentation in
docs/Ensure that any completed roadmap steps are recorded in the appropriate roadmap in
docs/Files must not exceed 400 lines in length
- Large modules must be decomposed
- Long match statements or dispatch tables should be decomposed by domain and collocated with targets
- Large blocks of inline data (e.g., test fixtures, constants or templates) must be moved to external files and inlined at compile-time or loaded at run-time.
Files:
src/main.rs
**/*.md
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.md: Documentation must use en-GB-oxendict spelling and grammar, except for the naming of the "LICENSE" file.
Validate Markdown files usingmake markdownlint.
Runmake fmtafter any documentation changes to format all Markdown files and fix table markup.
Validate Mermaid diagrams in Markdown files by runningmake nixie.
Markdown paragraphs and bullet points must be wrapped at 80 columns.
Code blocks in Markdown must be wrapped at 120 columns.
Tables and headings in Markdown must not be wrapped.
Use dashes (-) for list bullets in Markdown.
Use GitHub-flavoured Markdown footnotes ([^1]) for references and footnotes.
Files:
README.md
⚙️ CodeRabbit Configuration File
**/*.md: * Avoid 2nd person or 1st person pronouns ("I", "you", "we")
- Use en-GB-oxendict (-ize / -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:
README.md
🧬 Code Graph Analysis (1)
src/main.rs (1)
src/review_threads.rs (2)
filter_threads_by_files(208-222)fetch_review_threads(138-172)
🔍 Additional research (Deepwiki)
Summary of relevant context found:
-
ReviewThread is defined/used in src/main.rs with fields: id (String), is_resolved (bool), comments (CommentConnection). Review-thread fetching/processing occurs via fetch_review_threads, summarize_files, and print_thread in src/main.rs.,
-
The PR adds a new filter helper (filter_threads_by_files) in src/review_threads.rs and integrates it into run_pr: PrArgs gains pub files: Vec, run_pr applies filter_threads_by_files to fetched threads before rendering. Tests were added for the helper and CLI parsing. (These changes align with the README/docs updates describing file-path filtering.)
⏰ 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: build-test
🔇 Additional comments (6)
README.md (3)
15-16: Usage syntax with optional file arguments — LGTMDocument the variadic FILE args clearly. Matches the implemented CLI.
18-19: Add concise guidance for file filtering — LGTMExplain the behaviour succinctly and in line with the feature.
62-63: Provide example demonstrating file-path filtering — LGTMShow a concrete invocation alongside the baseline example.
src/main.rs (3)
33-34: Re-exportfilter_threads_by_files— LGTMAlign with existing re-exports from
review_threads; keeps call sites tidy.
326-333: CLI parsing without files — LGTMAssert the empty
filesvector correctly. Test name and expectations are clear.
334-345: CLI parsing with files — LGTMCapture and order of file args are verified. Matches the README usage.
| banner. Pass file paths after the pull request to restrict output to those | ||
| paths. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Switch list bullets from * to - to meet repository Markdown guidelines
Use dashes for list bullets per the coding guidelines. Update both list items.
Below shows the corrected list section (outside the selected lines):
- `pr` — show unresolved pull request comments. A summary of files and comment
counts is printed first. When finished, `vk` prints an `end of code review`
banner. Pass file paths after the pull request to restrict output to those
paths.
- `issue` — read a GitHub issue (**to do**)🤖 Prompt for AI Agents
In README.md around lines 34-35, the list bullets use '*' but the repository
Markdown guideline requires '-' dashes; update both list items in that section
to use '-' instead of '*', preserving indentation and line breaks and the
existing content/formatting of the two list entries.
| let threads = filter_threads_by_files( | ||
| fetch_review_threads(&client, &repo, number).await?, | ||
| &args.files, | ||
| ); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Refine the empty-state message when a file filter is applied
Improve UX by indicating that no comments matched the provided file paths.
Apply this change outside the selected lines:
// around lines 165-168
if threads.is_empty() {
if args.files.is_empty() {
println!("No unresolved comments.");
} else {
println!("No unresolved comments for the specified files.");
}
return Ok(());
}🤖 Prompt for AI Agents
In src/main.rs around lines 160-163, the code filters review threads by files
but the empty-state message doesn't reflect whether a file filter was used; add
a conditional after the filtering (around lines 165-168) that checks if
threads.is_empty() and prints "No unresolved comments." when args.files is
empty, otherwise print "No unresolved comments for the specified files.", then
return Ok(()) to exit early.
1 participant
Summary
vk prto accept file paths to restrict outputTesting
make fmtmake lintmake testmake markdownlintmake nixie(fails: libXfixes.so.3: cannot open shared object file)https://chatgpt.com/codex/tasks/task_e_6898970e8a2c8322b07b90b04160b8f6
Summary by Sourcery
Enable filtering of pull request review comments by file paths by extending the CLI, implementing a filtering helper, and updating documentation and tests.
New Features:
Enhancements:
Documentation:
Tests: