Conversation
Reviewer's GuideAdds support for fetching and displaying the most recent review banner from each reviewer in the pr command by extending the GraphQL model, implementing review fetch and pagination, integrating review selection and output, and updating documentation. Sequence diagram for fetching and displaying latest review banners in pr commandsequenceDiagram
participant User as actor User
participant CLI as pr command
participant GraphQLClient
participant GitHubAPI as GitHub GraphQL API
participant Output as Terminal Output
User->>CLI: Run pr command
CLI->>GraphQLClient: fetch_review_threads()
GraphQLClient->>GitHubAPI: COMMENT_QUERY
GitHubAPI-->>GraphQLClient: Comment data
GraphQLClient-->>CLI: Review threads
CLI->>GraphQLClient: fetch_reviews()
GraphQLClient->>GitHubAPI: REVIEWS_QUERY (with pagination)
GitHubAPI-->>GraphQLClient: Review data
GraphQLClient-->>CLI: List of PullRequestReview
CLI->>CLI: latest_reviews() (select most recent per reviewer)
CLI->>Output: print_reviews() (display review banners)
CLI->>Output: print_summary() and print_thread() (existing behavior)
Class diagram for new and updated types for review fetching and outputclassDiagram
class User {
+String login
}
class PullRequestReview {
+String body
+String submitted_at
+String state
+Option<User> author
}
class ReviewConnection {
+Vec<PullRequestReview> nodes
+PageInfo page_info
}
class ReviewPr {
+ReviewConnection reviews
}
class ReviewRepo {
+ReviewPr pull_request
}
class ReviewData {
+ReviewRepo repository
}
User <.. PullRequestReview : author
PullRequestReview <|-- ReviewConnection : nodes
ReviewConnection <|-- ReviewPr : reviews
ReviewPr <|-- ReviewRepo : pull_request
ReviewRepo <|-- ReviewData : repository
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Summary by CodeRabbit
WalkthroughUpdate the documentation and main logic to support fetching, processing, and displaying the latest pull request reviews per reviewer, in addition to unresolved threads. Introduce new data structures, GraphQL queries, and output formatting functions. Integrate the review display into the pull request summary flow and clarify documentation accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI
participant GitHubAPI
User->>CLI: Run pull request command
CLI->>GitHubAPI: Fetch PR threads
CLI->>GitHubAPI: Fetch PR reviews (paginated)
GitHubAPI-->>CLI: Return threads and reviews
CLI->>CLI: Filter unresolved threads
CLI->>CLI: Extract latest review per reviewer
CLI->>User: Print summary, reviews, then threads
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (1)Cargo.toml📄 CodeRabbit Inference Engine (AGENTS.md)
Files:
🔇 Additional comments (1)
✨ 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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
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 found some issues that need to be addressed.
- latest_reviews uses a BTreeMap keyed by login which results in alphabetical ordering; if you need chronological or insertion order, consider using a HashMap and then sorting the Vec by timestamp or reviewer activity.
- Comparing ‘submitted_at’ timestamps as raw strings leans on ISO 8601 lexicographical order; to avoid subtle ordering bugs and enable richer date handling, parse these into a proper DateTime type.
- There’s a lot of new review-related structs and GraphQL logic in main.rs—consider extracting them into a dedicated reviews module for better separation of concerns and maintainability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- latest_reviews uses a BTreeMap keyed by login which results in alphabetical ordering; if you need chronological or insertion order, consider using a HashMap and then sorting the Vec by timestamp or reviewer activity.
- Comparing ‘submitted_at’ timestamps as raw strings leans on ISO 8601 lexicographical order; to avoid subtle ordering bugs and enable richer date handling, parse these into a proper DateTime type.
- There’s a lot of new review-related structs and GraphQL logic in main.rs—consider extracting them into a dedicated reviews module for better separation of concerns and maintainability.
## Individual Comments
### Comment 1
<location> `src/main.rs:720` </location>
<code_context>
+ skin: &MadSkin,
+ review: &PullRequestReview,
+) -> anyhow::Result<()> {
+ let author = review.author.as_ref().map_or("", |u| u.login.as_str());
+ writeln!(out, "\u{1f4dd} \x1b[1m{author}\x1b[0m {}:", review.state)?;
+ let _ = skin.write_text_on(&mut out, &review.body);
</code_context>
<issue_to_address>
Empty string for missing author may be ambiguous.
Consider using a placeholder like "(unknown)" or "(bot)" instead of an empty string to indicate a missing author.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
let author = review.author.as_ref().map_or("", |u| u.login.as_str());
=======
let author = review.author.as_ref().map_or("(unknown)", |u| u.login.as_str());
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/main.rs:722` </location>
<code_context>
+) -> anyhow::Result<()> {
+ let author = review.author.as_ref().map_or("", |u| u.login.as_str());
+ writeln!(out, "\u{1f4dd} \x1b[1m{author}\x1b[0m {}:", review.state)?;
+ let _ = skin.write_text_on(&mut out, &review.body);
+ writeln!(out)?;
+ Ok(())
</code_context>
<issue_to_address>
Ignoring the result of write_text_on may hide errors.
Handle or log errors from skin.write_text_on to aid debugging, as done in print_reviews.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
let _ = skin.write_text_on(&mut out, &review.body);
=======
if let Err(e) = skin.write_text_on(&mut out, &review.body) {
eprintln!("error writing review body: {e}");
}
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/main.rs:283` </location>
<code_context>
+}
+
+#[derive(Deserialize)]
+struct ReviewData {
+ repository: ReviewRepo,
+}
</code_context>
<issue_to_address>
Consider collapsing redundant wrapper structs and simplifying the latest review selection logic for improved clarity and conciseness.
Here are two small focused refactorings that trim out a couple of the one-field wrappers and simplify the “latest per author” logic without changing any behavior:
1) Collapse `ReviewRepo` & `ReviewPr` into a single helper by using `#[serde(rename_all = "camelCase")]`
Before you had:
```rust
#[derive(Deserialize)]
struct ReviewData {
repository: ReviewRepo,
}
#[derive(Deserialize)]
struct ReviewRepo {
#[serde(rename = "pullRequest")]
pull_request: ReviewPr,
}
#[derive(Deserialize)]
struct ReviewPr {
reviews: ReviewConnection,
}
```
You can replace all of that with:
```rust
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
struct ReviewData {
repository: RepositoryReviews,
}
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
struct RepositoryReviews {
pull_request: ReviewConnection,
}
#[derive(Debug, Deserialize)]
#[serde(rename_all = "camelCase")]
struct ReviewConnection {
nodes: Vec<PullRequestReview>,
page_info: PageInfo,
}
```
Now you only need *three* types instead of five, and `pageInfo` → `page_info` is handled by the `rename_all`.
2) Replace your manual `BTreeMap` entry logic in `latest_reviews` with a simple grouping + `max_by_key` (via `itertools`) or a tiny `HashMap`‐based fold. For example, with [`itertools`](https://docs.rs/itertools):
```rust
use itertools::Itertools;
fn latest_reviews(reviews: Vec<PullRequestReview>) -> Vec<PullRequestReview> {
reviews
.into_iter()
.into_group_map_by(|r| {
r.author
.as_ref()
.map(|u| u.login.clone())
.unwrap_or_default()
})
.into_iter()
.filter_map(|(_, group)| group.into_iter().max_by_key(|r| r.submitted_at))
.collect()
}
```
Or, without any extra crate:
```rust
use std::collections::HashMap;
fn latest_reviews(reviews: Vec<PullRequestReview>) -> Vec<PullRequestReview> {
let mut latest: HashMap<String, PullRequestReview> = HashMap::new();
for r in reviews {
let key = r
.author
.as_ref()
.map(|u| u.login.clone())
.unwrap_or_default();
let entry = latest.entry(key).or_insert_with(|| r.clone());
if r.submitted_at > entry.submitted_at {
*entry = r;
}
}
latest.into_values().collect()
}
```
These two tweaks retain all functionality, remove several tiny wrapper structs, and make the grouping logic more declarative.
</issue_to_address>
### Comment 4
<location> `src/main.rs:251` </location>
<code_context>
}
-#[derive(Debug, Deserialize, Default)]
+#[derive(Debug, Deserialize, Default, Clone)]
struct User {
login: String,
</code_context>
<issue_to_address>
The module is missing a `//!` comment at the top as required by the review instructions.
Please add a `//!` module-level comment at the very top of this file to describe its purpose, as per the project guidelines.
</issue_to_address>
### Comment 5
<location> `docs/vk-design.md:32` </location>
<code_context>
-`run_pr` fetches all unresolved threads and calls `print_thread`. Errors from
-`print_thread` are surfaced via logging.
+`run_pr` fetches the latest review banner from each reviewer and all unresolved
+threads. The reviews are printed after the summary and before individual
+threads. Errors from `print_thread` are surfaced via logging.
</code_context>
<issue_to_address>
This paragraph line exceeds 80 columns and should be wrapped accordingly.
Please ensure all paragraph lines are wrapped to 80 columns for readability and consistency.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@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: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.toml(1 hunks)src/main.rs(5 hunks)src/reviews.rs(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
Cargo.toml
📄 CodeRabbit Inference Engine (AGENTS.md)
Use explicit version ranges in Cargo.toml and keep dependencies up-to-date.
Files:
Cargo.toml
**/*.rs
📄 CodeRabbit Inference Engine (AGENTS.md)
**/*.rs: Run cargo fmt, cargo clippy -- -D warnings, and RUSTFLAGS="-D warnings" cargo test before committing.
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 using Arc to reduce the amount of data returned.
Write unit and behavioural tests for new functionality. Run both before and after making any change.
Document public APIs using Rustdoc comments (///) so documentation can be generated with cargo doc.
Prefer immutable data and avoid unnecessary mut bindings.
Handle errors with the Result type instead of panicking where feasible.
Avoid unsafe code unless absolutely necessary and document any usage clearly.
Files:
src/main.rssrc/reviews.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.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/reviews.rs
🔇 Additional comments (14)
src/main.rs (6)
7-7: LGTM!The module declaration is correctly placed and follows standard Rust conventions.
9-9: LGTM!The import statement correctly brings in the required functions from the reviews module.
253-253: LGTM!Adding Clone to the User struct is necessary for the review functionality and follows standard Rust practices.
333-333: LGTM!Changing the visibility to pub(crate) appropriately allows reuse of the pagination functionality in the reviews module whilst maintaining encapsulation.
657-657: LGTM!The review fetching is correctly integrated with proper error handling using the ? operator.
667-669: LGTM!The review processing and printing logic is well-integrated, with appropriate spacing for output formatting.
src/reviews.rs (8)
1-1: LGTM!The module documentation properly follows the required //! format and clearly describes the module's purpose.
3-10: LGTM!The imports are well-organised and include only the necessary dependencies for the module's functionality.
11-38: LGTM!The struct definitions are well-designed with appropriate visibility, proper serde attributes, and sensible field types including proper timestamp handling.
40-56: LGTM!The GraphQL query is well-structured with proper pagination support and includes all necessary fields for review functionality.
58-85: LGTM!The fetch functions are well-implemented with proper async/await patterns, consistent error handling, and good reuse of existing pagination logic.
87-107: LGTM!The latest_reviews function correctly implements the logic to find the most recent review per author using HashMap's Entry API and proper timestamp comparison.
109-124: LGTM!The write_review function properly addresses previous feedback by using "(unknown)" for missing authors and includes appropriate error handling for the markdown rendering with stderr logging.
126-132: LGTM!The print_reviews function is well-implemented with proper error handling and efficient use of stdout locking.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary
vk-design.mdTesting
cargo clippy -- -D warningsRUSTFLAGS="-D warnings" cargo testmarkdownlint docs/vk-design.mdnixie docs/vk-design.md(fails: too many arguments)https://chatgpt.com/codex/tasks/task_e_688291e48c9c8322ba9c5d6466702bee
Summary by Sourcery
Add support for fetching pull request reviews and printing each reviewer’s latest review banner in the pr command
New Features:
Documentation: