Conversation
Reviewer's GuideIntroduce a Sequence diagram for printing the start banner in code review outputsequenceDiagram
participant CLI
participant run_pr
participant print_start_banner
CLI->>run_pr: Start PR command
run_pr->>print_start_banner: Call print_start_banner()
print_start_banner-->>run_pr: Prints 'code review' banner
run_pr->>run_pr: Print summary and threads
run_pr->>CLI: Output review results
Class diagram for new print_start_banner helperclassDiagram
class Summary {
+print_summary(summary: &[(String, usize)])
+print_start_banner() std::io::Result<()> // newly added
+print_end_banner() std::io::Result<()>
}
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
WalkthroughIntroduce a start banner before summaries and threads, centralise banner error handling via a handle_banner helper, export the start-banner API, update docs and tests to expect banners, and ensure BrokenPipe causes a consistent early exit while other banner errors are logged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as vk (run_pr)
participant GQL as GitHub GraphQL
participant Summary as summary module
User->>CLI: vk pr <url>
CLI->>GQL: Fetch PR threads and reviews
GQL-->>CLI: Threads, reviews
CLI->>Summary: print_start_banner()
alt BrokenPipe when printing start
CLI-->>User: Exit early
else Start banner OK
CLI-->>User: Print summary of files and comment counts
CLI-->>User: Print review threads
CLI->>Summary: print_end_banner()
alt BrokenPipe when printing end
CLI-->>User: Exit early
else End banner OK
CLI-->>User: Completion
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ 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 there - I've reviewed your changes and found some issues that need to be addressed.
- The README's inline code formatting for the start banner wraps awkwardly across lines—consider reformatting it to keep
code reviewin one segment. - Both
print_start_bannerandprint_end_banneruse identical broken-pipe error handling; consider extracting this into a shared helper to avoid duplication. - Currently only empty-state tests cover the new banner—consider adding a non-empty PR test to ensure the start banner also appears in standard review outputs.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The README's inline code formatting for the start banner wraps awkwardly across lines—consider reformatting it to keep `code review` in one segment.
- Both `print_start_banner` and `print_end_banner` use identical broken-pipe error handling; consider extracting this into a shared helper to avoid duplication.
- Currently only empty-state tests cover the new banner—consider adding a non-empty PR test to ensure the start banner also appears in standard review outputs.
## Individual Comments
### Comment 1
<location> `tests/cli.rs:45` </location>
<code_context>
#[case(
Vec::new(),
- "No unresolved comments.\n========== end of code review ==========\n"
+ "========== code review ==========\nNo unresolved comments.\n========== end of code review ==========\n"
)]
#[case(
</code_context>
<issue_to_address>
Consider adding a test case for error conditions when printing the start banner.
Add a test that simulates a failure in `print_start_banner()` (such as a stdout write error) to verify that error handling in `main.rs` functions correctly.
Suggested implementation:
```rust
#[tokio::test]
#[case(
"stdout write error",
"Simulated stdout write error",
)]
```
```rust
)]
// Additional test for error condition in print_start_banner
#[tokio::test]
async fn test_print_start_banner_error() {
use std::io::{self, Write};
use std::sync::{Arc, Mutex};
// Create a writer that always returns an error
struct ErrorWriter;
impl Write for ErrorWriter {
fn write(&mut self, _buf: &[u8]) -> io::Result<usize> {
Err(io::Error::new(io::ErrorKind::Other, "Simulated stdout write error"))
}
fn flush(&mut self) -> io::Result<()> {
Ok(())
}
}
// Assuming print_start_banner takes a writer as argument for testability
let mut writer = ErrorWriter;
let result = crate::print_start_banner(&mut writer);
assert!(result.is_err(), "Expected error when writing to stdout");
assert_eq!(
result.unwrap_err().to_string(),
"Simulated stdout write error"
);
}
```
- If `print_start_banner` does not currently accept a writer argument, you will need to refactor it in your main code to allow dependency injection for testing.
- Ensure that error handling in `main.rs` properly propagates or logs the error from `print_start_banner`.
- You may need to adjust the test to match the actual error handling and function signatures in your codebase.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Passed
See analysis details in CodeScene
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| main.rs | 8.48 → 8.91 | Complex Method, Bumpy Road Ahead |
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.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(1 hunks)docs/vk-design.md(2 hunks)src/main.rs(5 hunks)src/summary.rs(3 hunks)tests/cli.rs(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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:
docs/vk-design.mdREADME.md
**/*.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:
tests/cli.rssrc/summary.rssrc/main.rs
🧬 Code Graph Analysis (2)
tests/cli.rs (2)
tests/utils.rs (2)
shutdown(28-31)start_mitm(47-96)src/api/mod.rs (1)
new(91-98)
src/main.rs (1)
src/summary.rs (5)
print_end_banner(155-157)print_start_banner(121-123)print_summary(79-86)summarize_files(27-39)write_summary(62-76)
🪛 LanguageTool
README.md
[uncategorized] ~33-~33: Possible missing comma found.
Context: ...s. Output begins with a code review banner followed by a summary of files and comm...
(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (8)
docs/vk-design.md (1)
24-26: Document the banner framing: LGTMThe UX bullet clearly explains the opening and closing banners and matches the implemented behaviour.
src/summary.rs (1)
155-157: Delegate to write_end_banner: LGTMReturning the write result directly matches the “no return in one-liners” guidance and keeps behaviour symmetric with the start banner.
src/main.rs (3)
197-199: Start-banner invocation and early-exit on BrokenPipe: LGTMCall placement after fetching threads and early-return semantics are correct.
208-210: End-banner on empty state routed through the common handler: LGTMCentralising BrokenPipe handling keeps behaviour consistent.
238-240: Final end-banner routed through the common handler: LGTMSymmetry with the empty-state path is maintained.
tests/cli.rs (3)
14-15: Import predicates for expressive stdout assertions: LGTMThe predicates-based assertions read clearly and target the new banner-first output.
44-52: Adjust empty-state expectations to include start banner: LGTMThe expected output matches the new framing and preserves the end banner for consumers.
81-137: Handler alias already accepts FnMut; tests compile as is
- tests/utils.rs: line 18 defines
Handler = Arc<Mutex<Box<dyn FnMut(&Request<Incoming>) -> Response<Full<Bytes>> + Send>>>, so the stateful closure works without changes.
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Passed
See analysis details in CodeScene
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| main.rs | 8.48 → 8.91 | Complex Method, Bumpy Road Ahead |
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.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
README.md (1)
33-33: Serial comma after “banner”: LGTMKeep the comma; it improves readability and matches the prior suggestion.
src/summary.rs (1)
12-16: Centralise banner strings via public constants: LGTMAvoid duplication and drift; tests and call sites now share a single source of truth.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (4)
README.md(1 hunks)docs/vk-design.md(3 hunks)src/main.rs(7 hunks)src/summary.rs(5 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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.mddocs/vk-design.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.mddocs/vk-design.md
**/*.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.rssrc/summary.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.rssrc/summary.rs
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 the relevant file(s) in thedocs/directory to reflect the latest state when new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve.
Files:
docs/vk-design.md
🧬 Code Graph Analysis (1)
src/main.rs (1)
src/summary.rs (2)
print_end_banner(161-163)print_start_banner(127-129)
🔍 Remote MCP
Relevant facts discovered via repository inspection:
-
The function that prints the closing banner is implemented in
src/main.rsas:fn print_end_banner() { println!("========== end of code review =========="); }
[::functions.ask_question::]
-
No
src/summary.rsfile exists in the codebase, so theSTART_BANNER/END_BANNERconstants and related APIs must be defined elsewhere or not yet present.
[::functions.ask_question::]
🔇 Additional comments (15)
src/summary.rs (6)
1-1: Module docs clearly explain banners and summary utilitiesThe top-level documentation is concise and aligns with guidelines.
Also applies to: 4-6
94-96: Keepwrite_bannerminimal and scoped: LGTMUsing a single helper with
writeln!is correct and idiomatic.
115-130: Public stdout path for start banner: LGTMSignature and error semantics match write-path; example uses .expect per style guide.
131-148: End banner write-path mirrors start path: LGTMSymmetry helps maintainability.
157-163: Stdout end-banner path delegates to writer: LGTMKeeps a single write path; examples follow house style.
232-270: Error-propagation tests for banner writers: LGTMBoth start and end paths now verified for IO error propagation.
src/main.rs (6)
9-11: Document banner framing in module header: LGTMThe updated docs accurately reflect the start/end banner behaviour.
36-38: Re-exportprint_start_banneralongside summary APIs: LGTMThe public surface accurately exposes both banners and summary helpers.
168-179: Centralise banner error handling viahandle_banner: LGTMReturn value semantics are clear (true = BrokenPipe/early-exit) and logging is scoped.
208-210: Gracefully emit end banner when no threads: LGTMPreserve the end-of-review sentinel for parsers even on empty results.
238-240: Emit end banner after threads: LGTMKeeps the output framing consistent.
459-471: Unit tests forhandle_banner: LGTMBoth BrokenPipe and non-BrokenPipe paths are exercised; behaviour is now locked in.
docs/vk-design.md (3)
36-41: Architecture flow reflects banner-first printing and error handling: LGTMThe paragraph accurately captures sequencing and BrokenPipe handling semantics.
125-127: Class diagram service node: LGTMThe service entry matches the described GraphQL fetch behaviour.
24-26: Validate docs pipelineEnsure the updated Banners description passes Markdown and Mermaid linting:
- Run markdownlint against
docs/vk-design.md:markdownlint docs/vk-design.md- Validate Mermaid diagrams with nixie (or the pipeline’s configured Mermaid linter):
nixieInstall
markdownlint-cliornixieif commands are unavailable, or adjust the docs pipeline configuration accordingly.
Call the start banner helper before network requests so output begins immediately. Handle broken pipes in the empty review case, add a unit test for banner content, and switch README bullets to dashes.
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Failed
(1 hotspot with Complex Method)
(1 file with Complex Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| main.rs | 1 rule in this hotspot | 8.48 → 8.86 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| main.rs | 1 advisory rule | 8.48 → 8.86 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| main.rs | 8.48 → 8.86 | Bumpy Road Ahead |
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.
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Failed
(1 hotspot with Complex Method)
(1 file with Complex Method)
Gates Passed
See analysis details in CodeScene
Reason for failure
| Prevent hotspot decline | Violations | Code Health Impact | |
|---|---|---|---|
| main.rs | 1 rule in this hotspot | 8.48 → 8.86 | Suppress |
| Enforce advisory code health rules | Violations | Code Health Impact | |
|---|---|---|---|
| main.rs | 1 advisory rule | 8.48 → 8.86 | Suppress |
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| main.rs | 8.48 → 8.86 | Bumpy Road Ahead |
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.
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Passed
See analysis details in CodeScene
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| main.rs | 8.48 → 9.24 | Complex Method, Bumpy Road Ahead |
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.
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
src/summary.rs (1)
84-91: Use the project logger instead of eprintln! for consistencyEmit errors via the log crate to keep logging consistent across the binary.
- eprintln!("Failed to write summary: {e}"); + error!("Failed to write summary: {e}");Add this import at the top of the module (outside the selected lines):
use log::error;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
💡 Knowledge Base configuration:
- Jira integration is disabled
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (3)
README.md(1 hunks)src/main.rs(6 hunks)src/summary.rs(5 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/summary.rssrc/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/summary.rssrc/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 (7)
src/summary.rs (5)
print_end_banner(161-163)print_start_banner(127-129)print_summary(85-92)summarize_files(33-45)write_summary(68-82)src/printer/mod.rs (2)
print_reviews(136-145)write_thread(103-118)src/ref_parser.rs (2)
parse_issue_reference(112-117)parse_pr_reference(119-124)src/reviews.rs (2)
fetch_reviews(87-93)latest_reviews(95-115)src/api/mod.rs (1)
new(91-98)src/review_threads/tests.rs (1)
repo(64-69)src/review_threads.rs (1)
filter_threads_by_files(226-240)
🪛 LanguageTool
README.md
[style] ~33-~33: Would you like to use the Oxford spelling “summarizes”? The spelling ‘summarises’ is also correct.
Context: ...t begins with a code review banner, summarises files and comment counts, then prints a...
(OXFORD_SPELLING_Z_NOT_S)
🔇 Additional comments (4)
src/summary.rs (1)
94-147: Banner helpers: cohesive and correctCentralise banner formatting via write_banner and expose start/end writers and printers. Error contracts and doctests look solid.
src/main.rs (3)
9-14: Module docs and public re-exports accurately describe banner framingDocument the start/end banners and re-export the banner + summary helpers. This aligns the public surface with behaviour.
171-182: handle_banner: correct early-exit semantics and loggingReturn true on BrokenPipe and log other errors. Unit tests cover both branches; keep this as the single choke point.
502-513: Unit tests for handle_banner: LGTMExercise both BrokenPipe and non-BrokenPipe paths. This prevents regressions in banner error handling.
| - `pr` — show unresolved pull request comments. It begins with a `code review` | ||
| banner, summarises files and comment counts, then prints an | ||
| `end of code review` banner. Pass file paths after the pull request to | ||
| restrict output to those paths. |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Standardise to en-GB-oxendict (-ize): change “summarises” to “summarizes”
Follow the house style (-ize). Update the wording accordingly.
- banner, summarises files and comment counts, then prints an
+ banner, summarizes files and comment counts, then prints an📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - `pr` — show unresolved pull request comments. It begins with a `code review` | |
| banner, summarises files and comment counts, then prints an | |
| `end of code review` banner. Pass file paths after the pull request to | |
| restrict output to those paths. | |
| - `pr` — show unresolved pull request comments. It begins with a `code review` | |
| banner, summarizes files and comment counts, then prints an | |
| `end of code review` banner. Pass file paths after the pull request to | |
| restrict output to those paths. |
🧰 Tools
🪛 LanguageTool
[style] ~33-~33: Would you like to use the Oxford spelling “summarizes”? The spelling ‘summarises’ is also correct.
Context: ...t begins with a code review banner, summarises files and comment counts, then prints a...
(OXFORD_SPELLING_Z_NOT_S)
🤖 Prompt for AI Agents
In README.md around lines 32 to 35, change the word "summarises" to "summarizes"
to match the en-GB-oxendict (-ize) house style; update the sentence so it reads
"summarizes files and comment counts" while keeping the rest of the wording
unchanged.
| if handle_banner(print_start_banner, "start") { | ||
| return Err(VkError::Io(Box::new(std::io::Error::new( | ||
| ErrorKind::BrokenPipe, | ||
| "broken pipe", | ||
| )))); | ||
| } | ||
| let client = build_graphql_client(&token, global.transcript.as_ref())?; | ||
| let threads = filter_threads_by_files( | ||
| fetch_review_threads(&client, &repo, number).await?, | ||
| &args.files, | ||
| ); | ||
| // Avoid fetching reviews when there are no unresolved threads. | ||
| if threads.is_empty() { | ||
| if args.files.is_empty() { | ||
| println!("No unresolved comments."); | ||
| } else { | ||
| println!("No unresolved comments for the specified files."); | ||
| } | ||
| // Preserve the end-of-review banner for consumers that parse it. | ||
| if let Err(e) = print_end_banner() { | ||
| if is_broken_pipe_io(&e) { | ||
| return Ok(()); | ||
| } | ||
| error!("error printing end banner: {e}"); | ||
| Ok((repo, number, client)) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Avoid modelling BrokenPipe as an error for control flow in setup_pr_output
Return an explicit early-exit signal instead of synthesising a BrokenPipe Io error only to catch-and-ignore it in run_pr. This reduces cognitive load and keeps error paths for actual failures.
- Option A: Change setup_pr_output to return Result<Option<(RepoInfo, u64, GraphQLClient)>, VkError>, where None means early-exit on BrokenPipe.
- Option B: Introduce a small enum: enum PrSetup { Continue(RepoInfo, u64, GraphQLClient), EarlyExit }.
Follow-up if you want a ready patch.
🤖 Prompt for AI Agents
In src/main.rs around lines 198 to 206, setup_pr_output currently synthesizes an
IoError(ErrorKind::BrokenPipe) to signal an early exit; change its API to return
an explicit control-flow signal instead of a fake error. Replace the current
Result<(RepoInfo, u64, GraphQLClient), VkError> with either
Result<Option<(RepoInfo, u64, GraphQLClient)>, VkError> (Option None =
early-exit) or a small enum like enum PrSetup { Continue(RepoInfo, u64,
GraphQLClient), EarlyExit } and return EarlyExit/None where you currently
construct the BrokenPipe error; then update callers (notably run_pr) to match
the new return shape: handle the EarlyExit/None as a normal early-return path
(no error logging) and only treat Err(VkError) as real failures. Ensure you
remove the synthesized BrokenPipe creation and corresponding catch-and-ignore
logic.
| #[allow( | ||
| clippy::unnecessary_wraps, | ||
| reason = "returns Result for interface symmetry" | ||
| )] | ||
| fn handle_empty_threads(files: &[String]) -> Result<(), VkError> { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Replace #[allow(...)] with narrowly scoped #[expect(..., reason = "...")]
House style forbids #[allow]; use #[expect] with a reason instead. This silences the lint in a documented, tightly scoped way.
-#[allow(
- clippy::unnecessary_wraps,
- reason = "returns Result for interface symmetry"
-)]
+#[expect(clippy::unnecessary_wraps, reason = "returns Result for interface symmetry")]
fn handle_empty_threads(files: &[String]) -> Result<(), VkError> {
…
}
-#[allow(clippy::unnecessary_wraps, reason = "future error cases may emerge")]
+#[expect(clippy::unnecessary_wraps, reason = "future error cases may emerge")]
fn generate_pr_output(
threads: Vec<ReviewThread>,
reviews: Vec<PullRequestReview>,
) -> Result<(), VkError> {
…
}Also applies to: 231-233
🤖 Prompt for AI Agents
In src/main.rs around lines 209-213 (and similarly at 231-233), replace the
module-level #[allow(...)] attribute with a narrowly scoped
#[expect(clippy::unnecessary_wraps, reason = "...")] applied immediately above
the specific function(s); specifically, remove the existing #[allow(..., reason
= "...")] and add #[expect(clippy::unnecessary_wraps, reason = "returns Result
for interface symmetry")] directly on the fn handle_empty_threads signature (and
the other function at 231-233), so the lint is silenced with a documented
expectation rather than a broad allow.
| #[test] | ||
| fn write_end_banner_propagates_io_errors() { | ||
| use std::io::{self, Write}; | ||
|
|
||
| struct ErrorWriter; | ||
| impl Write for ErrorWriter { | ||
| fn write(&mut self, _buf: &[u8]) -> io::Result<usize> { | ||
| Err(io::Error::other("Simulated stdout write error")) | ||
| } | ||
|
|
||
| fn flush(&mut self) -> io::Result<()> { | ||
| Ok(()) | ||
| } | ||
| } | ||
|
|
||
| let mut writer = ErrorWriter; | ||
| let err = write_end_banner(&mut writer).expect_err("expect error"); | ||
| assert_eq!(err.to_string(), "Simulated stdout write error"); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add symmetric assertion for end-banner exact text
Lock in the exact END_BANNER output as done for START_BANNER.
Append this test near the other banner tests:
#[test]
fn write_end_banner_outputs_exact_text() {
let mut buf = Vec::new();
write_end_banner(&mut buf).expect("write end banner");
assert_eq!(
String::from_utf8(buf).expect("utf8"),
format!("{END_BANNER}\n"),
);
}🤖 Prompt for AI Agents
In src/summary.rs around lines 262 to 280, add a symmetric unit test that
verifies the exact END_BANNER output text (including the trailing newline)
similar to the existing START_BANNER test; create a test named
write_end_banner_outputs_exact_text that writes the banner into a Vec<u8>,
converts it to a UTF-8 String, and asserts equality with
format!("{END_BANNER}\n") so the exact banner content is locked in.
1 participant
Summary
code reviewbanner before summaries and threadsTesting
make fmtmake lintmake test./target/debug/vk pr https://github.com/leynos/wildside/pull/3 > /tmp/pr_output2.txt && head -n 20 /tmp/pr_output2.txthttps://chatgpt.com/codex/tasks/task_e_689dc44978308322beed3756bf6baa4a
Summary by Sourcery
Add a start banner to code review output and integrate it into the CLI flow
New Features:
prcommand to output acode reviewbannerEnhancements:
Documentation:
Tests: