Skip to content

Avoid repeating diffs for thread comments#27

Merged
leynos merged 6 commits intomainfrom
codex/refactor-tests-to-eliminate-code-duplication
Jul 21, 2025
Merged

Avoid repeating diffs for thread comments#27
leynos merged 6 commits intomainfrom
codex/refactor-tests-to-eliminate-code-duplication

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Jul 21, 2025

Summary

  • show diff once per review thread instead of repeating for every comment
  • factor comment body rendering into a helper
  • fix markdownlint line length in AGENTS.md

Testing

  • cargo clippy -- -D warnings
  • RUSTFLAGS="-D warnings" cargo test
  • npx markdownlint-cli2 '**/*.md' '#node_modules'

https://chatgpt.com/codex/tasks/task_e_687d8497aa148322ae16e393dd6026a5

Summary by Sourcery

Show diffs only once per review thread by introducing print_comment_body and print_thread helpers, and update print logic to use them; also fix markdownlint line length in AGENTS.md.

New Features:

  • Show diff once per review thread instead of repeating for every comment

Enhancements:

  • Extract a helper for rendering comment bodies and centralize comment/thread printing

Documentation:

  • Wrap a long line in AGENTS.md to satisfy markdownlint line-length rule

@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Jul 21, 2025

Reviewer's Guide

Refactor comment rendering to show diffs only once per review thread, encapsulate body printing in a helper, and correct markdown line lengths in documentation.

Sequence diagram for printing a review thread with non-repeated diffs

sequenceDiagram
    participant Main
    participant MadSkin
    participant ReviewThread
    participant ReviewComment

    Main->>ReviewThread: for each thread
    Main->>MadSkin: print_thread(&skin, &thread)
    MadSkin->>ReviewComment: get first comment
    MadSkin->>MadSkin: print_comment(skin, first)
    MadSkin->>ReviewComment: print_comment_body(skin, first)
    MadSkin->>Main: print first.url
    MadSkin->>ReviewComment: for remaining comments
    MadSkin->>MadSkin: print_comment_body(skin, c)
    MadSkin->>Main: print c.url
Loading

Class diagram for refactored comment and thread rendering

classDiagram
    class MadSkin
    class ReviewComment {
        author: Option<String>
        body: String
        url: String
    }
    class ReviewThread {
        comments: Comments
    }
    class Comments {
        nodes: Vec<ReviewComment>
    }

    MadSkin <.. print_comment_body
    MadSkin <.. print_comment
    MadSkin <.. print_thread
    ReviewThread o-- Comments
    Comments o-- ReviewComment

    class print_comment_body {
        +fn print_comment_body(skin: &MadSkin, comment: &ReviewComment)
    }
    class print_comment {
        +fn print_comment(skin: &MadSkin, comment: &ReviewComment) -> Result<(), Error>
    }
    class print_thread {
        +fn print_thread(skin: &MadSkin, thread: &ReviewThread) -> Result<(), Error>
    }
Loading

File-Level Changes

Change Details Files
Consolidate diff output per review thread
  • Introduce print_thread to sequence comments and control diff display
  • Replace per-comment loop in run_pr with a single call to print_thread
  • Ensure only the first comment prints the diff, subsequent comments render only the body and URL
src/main.rs
Extract comment body rendering into a helper
  • Create print_comment_body to encapsulate author, body, and formatting logic
  • Update print_comment to delegate body rendering to print_comment_body
src/main.rs
Fix markdownlint line length in AGENTS.md
  • Break up an overlong line to satisfy markdownlint rules
AGENTS.md

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jul 21, 2025

Summary by CodeRabbit

  • New Features

    • Introduced a new design document outlining the user experience and architecture of the CLI tool for viewing unresolved GitHub pull request comments.
  • Documentation

    • Improved readability in Rust-specific guidance.
    • Added detailed documentation describing the CLI tool’s workflow and output formatting.
  • Refactor

    • Modularised and unified comment and thread output, ensuring diffs are printed only once per thread and enhancing error handling during output.

Walkthrough

Refactor the comment and thread output logic in the main Rust source file by introducing generic writer-based functions to modularise and unify output generation. Update documentation for Rust-specific guidance in AGENTS.md by splitting a line for clarity. Add a test to verify correct diff output in threads.

Changes

File(s) Change Summary
AGENTS.md Split a Rust guidance line into two for improved readability; no change to meaning or logic.
src/main.rs Refactor comment/thread printing to use generic writer functions; remove print_comment; add tests.
docs/vk-design.md Add detailed design document describing the vk CLI tool’s user experience, architecture, and testing.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant run_pr
    participant print_thread
    participant write_thread
    participant write_comment_body

    User->>run_pr: Invoke PR review
    run_pr->>print_thread: For each ReviewThread
    print_thread->>write_thread: Write thread to stdout
    write_thread->>write_comment_body: Write first comment (diff + body)
    write_thread->>write_comment_body: Write subsequent comments (body only)
Loading

Estimated code review effort

3 (~45 minutes)

Possibly related PRs

Poem

In lines refactored, logic flows anew,
Threads print cleanly, only once the diff comes through.
Comments now modular, output with grace,
Documentation tidied, a clearer place.
Tests ensure the order, the output, the cheer—
Code and docs in harmony, improvements appear!
🦀✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 157243a and eb14de7.

📒 Files selected for processing (1)
  • src/main.rs (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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.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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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 rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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
🔇 Additional comments (7)
src/main.rs (7)

1-1: Excellent addition of the required module-level documentation.

The module doc comment properly describes the CLI tool's purpose as required by the coding guidelines. This addresses the previous review comment about missing module documentation.


576-594: Well-documented helper function with proper separation of concerns.

The write_comment_body function correctly separates the concern of formatting comment bodies from diff handling. The Rustdoc comment is comprehensive and follows proper formatting conventions.


596-613: Clean implementation with appropriate abstraction.

The write_comment function properly delegates to write_comment_body after handling the diff output. The documentation clearly explains the function's purpose and parameter usage.


615-639: Excellent implementation that achieves the PR objective.

The write_thread function successfully addresses the core requirement of showing diffs only once per thread whilst ensuring all comment bodies are displayed. The logic correctly prints the first comment with its diff using write_comment, then subsequent comments using only write_comment_body.


641-646: Simple and effective stdout wrapper.

The print_thread function provides a clean interface for printing threads to stdout whilst maintaining the generic writer pattern for testability.


712-714: Improved error handling in the refactored printing logic.

The updated run_pr function correctly uses the new print_thread helper and includes proper error handling by logging print failures to stderr. This maintains robustness whilst utilising the new modular design.


1181-1207: Comprehensive behavioural test verifying the core functionality.

The test effectively validates that:

  • Diffs appear exactly once per thread (|-old count assertion)
  • All comment bodies are printed (wrote: count assertion)

The test structure is clean and the assertions are meaningful for verifying the PR's main objective.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/refactor-tests-to-eliminate-code-duplication

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @leynos - I've reviewed your changes and found some issues that need to be addressed.

  • Avoid silently discarding errors by calling .ok() on print_thread in run_pr—either propagate the Result or at least log failures instead of ignoring them.
  • Consider making print_comment_body return a Result (like print_comment) so that both printing helpers have consistent signatures and error handling.
  • You can simplify the default-author logic with comment.author.as_deref().unwrap_or("") instead of as_ref().unwrap_or(&"".into()).
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Avoid silently discarding errors by calling `.ok()` on print_thread in run_pr—either propagate the Result or at least log failures instead of ignoring them.
- Consider making print_comment_body return a Result (like print_comment) so that both printing helpers have consistent signatures and error handling.
- You can simplify the default-author logic with `comment.author.as_deref().unwrap_or("")` instead of `as_ref().unwrap_or(&"".into())`.

## Individual Comments

### Comment 1
<location> `src/main.rs:592` </location>
<code_context>
     println!();
+}
+
+fn print_thread(skin: &MadSkin, thread: &ReviewThread) -> anyhow::Result<()> {
+    let mut iter = thread.comments.nodes.iter();
+    if let Some(first) = iter.next() {
</code_context>

<issue_to_address>
The module does not begin with a `//!` comment as required by the review instructions.

Please add a `//!` module-level comment at the top of this file to describe its purpose, as per the review guidelines.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/main.rs Outdated
println!();
}

fn print_thread(skin: &MadSkin, thread: &ReviewThread) -> anyhow::Result<()> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (review_instructions): The module does not begin with a //! comment as required by the review instructions.

Please add a //! module-level comment at the top of this file to describe its purpose, as per the review guidelines.

Review instructions:

Path patterns: **/*.rs

Instructions:
Every module must begin with a //! comment.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 296f671 and facf159.

📒 Files selected for processing (1)
  • src/main.rs (4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • AGENTS.md

⚙️ CodeRabbit Configuration File

🔇 Additional comments (2)
src/main.rs (2)

1-1: Excellent addition of module documentation.

This properly addresses the previous review comment and follows the coding guidelines requirement for module-level documentation.


687-688: Perfect implementation of the thread-based printing logic.

This change correctly shifts from comment-by-comment iteration to thread-based printing, achieving the PR objective of showing diffs once per thread. The updated error message accurately reflects the new behaviour.

Comment thread src/main.rs
Comment thread src/main.rs
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (5)
src/main.rs (5)

576-586: Add Rustdoc comments to document the helper function.

The write_comment_body function lacks documentation describing its purpose and parameters.


588-597: Add Rustdoc comments to document the helper function.

The write_comment function lacks documentation describing its purpose and parameters.


599-614: Add Rustdoc comments to document the helper function.

The write_thread function lacks documentation describing its purpose and parameters.


616-618: Add Rustdoc comments to document the helper function.

The print_thread function lacks documentation describing its purpose and parameters.


1177-1177: Remove the debug println! statement.

The debug output clutters the test output and should be removed for cleaner test execution.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between facf159 and 384ccd0.

📒 Files selected for processing (2)
  • docs/vk-design.md (1 hunks)
  • src/main.rs (4 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
docs/**/*.md

📄 CodeRabbit Inference Engine (AGENTS.md)

docs/**/*.md: Reference: Use the markdown files within the docs/ directory as a knowledge base and source of truth for project requirements, dependency choices, and architectural decisions.
Update: When new decisions are made, requirements change, libraries are added/removed, or architectural patterns evolve, proactively update the relevant file(s) in the docs/ directory to reflect the latest state.

Files:

  • docs/vk-design.md
**/*.md

📄 CodeRabbit Inference Engine (AGENTS.md)

**/*.md: Validate Markdown files using markdownlint.
Validate Markdown Mermaid diagrams using the nixie CLI.

Files:

  • docs/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
  • Paragraphs and bullets must be wrapped to 80 columns, except where a long URL would prevent this (in which case, silence MD013 for that line)
  • Code blocks should be wrapped to 120 columns.
  • 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.md
**/*.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.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 return in single-line functions.

  • Move conditionals with >2 branches into a predicate function.

  • Avoid unsafe unless 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 rstest fixtures for shared setup and to avoid repetition between tests.

  • Replace duplicated tests with #[rstest(...)] parameterised cases.

  • Prefer mockall for 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
🪛 LanguageTool
docs/vk-design.md

[uncategorized] ~11-~11: Possible missing comma found.
Context: ...g using Termimad. Diffs appear once per thread even when multiple comments reference t...

(AI_HYDRA_LEO_MISSING_COMMA)

🔇 Additional comments (3)
src/main.rs (2)

1-1: Excellent addition of module documentation.

The module-level comment properly describes the CLI tool's purpose as required by the coding guidelines.


684-686: Excellent error handling improvement.

The error handling for print_thread properly logs failures to stderr instead of silently discarding them, which aligns with the design document's error visibility requirements.

docs/vk-design.md (1)

42-58: Excellent architectural documentation with visual aid.

The Mermaid sequence diagram effectively illustrates the interaction flow between components and aligns well with the implemented code structure. The diagram clearly shows how diffs are printed only once per thread whilst comment bodies are printed for each comment.

Comment thread docs/vk-design.md Outdated
@leynos leynos merged commit 5aef18b into main Jul 21, 2025
1 check passed
@coderabbitai coderabbitai Bot mentioned this pull request Aug 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant