Skip to content

[dogfood] Review-wide diff summary (DO NOT MERGE) - #34

Open
sdavisde wants to merge 2 commits into
mainfrom
dogfood/diff-review-summary
Open

[dogfood] Review-wide diff summary (DO NOT MERGE)#34
sdavisde wants to merge 2 commits into
mainfrom
dogfood/diff-review-summary

Conversation

@sdavisde

Copy link
Copy Markdown
Owner

Dogfood fixture — do not merge. This branch exists so redquill's PR review mode (R → Pull Requests) has a real GitHub PR to review against, with enough substance to exercise line comments, file comments, hunk navigation, and per-file review status.

That said, the code is real and the tree is green: cargo build, cargo test (1576 pass), cargo clippy --all-targets -- -D warnings, cargo fmt --check.

What's here

Two commits, deliberately different in shape:

  1. feat(diff): roll a review up into one summary — new diff::summarize producing a ReviewSummary (file/binary counts, total churn, largest file and hunk), plus DiffStat::total/net/is_empty and Hunk::changed_lines. build_review computes it once on the snapshot build. Docs in docs/diff-summary.md; README typo fix rides along.
  2. feat(ui): carry the review summary on App and flag binary filesApp holds the summary, suspends/restores it with the commit and file views, and the git panel's counts line grows a [N bin] chip.

11 files: a new module with its own tests, several small multi-hunk edits, a new markdown doc, and one all-additions doc/test block — a mix worth scrolling through.

Known review bait (intentional)

  • build_review now walks the files twice: once for the per-file StatDisplay map, once inside summarize. The comment above that loop used to promise a single pass. Worth arguing about.
  • ReviewSnapshot carries both total and summary.stat, which are the same number by construction.
  • Most of ReviewSummary (largest_file, largest_hunk, content_files, net()) has no consumer yet — only binary_files is rendered.

🤖 Generated with Claude Code

https://claude.ai/code/session_011wAgKziALkaqa3WNEibtX7

redquill test and others added 2 commits July 29, 2026 21:36
Answers "how big is this review, and where do I start?" without a render
surface walking hunks itself: file and binary counts, total churn, and the
largest file and hunk. `build_review` computes it once on the background
snapshot build and hands it to the UI on `ReviewSnapshot`.

Churn is added+removed, so a line rewritten in place counts twice — the
measure estimates how much there is to read, and a rewrite means reading
both sides. Binary files count as files but never as lines, matching the
call `stat_display` already makes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wAgKziALkaqa3WNEibtX7
`App` now holds the snapshot's `ReviewSummary` alongside `total_stats`,
refreshed on every rebuild and suspended/restored with the commit and
file views like the rest of the per-target state.

First use: the git panel's counts line gets a `[N bin]` chip. Binary files
sit in the file count but contribute nothing to `+A -R`, so without it the
counts read short with no explanation.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011wAgKziALkaqa3WNEibtX7
@sdavisde
sdavisde force-pushed the dogfood/diff-review-summary branch from 55e0966 to fd95557 Compare July 30, 2026 02:38
Comment thread README.md

<p align="center">
A portable, efficeint tool for reviewing code
A portable, efficient tool for reviewing code

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[praise] I like how you fixed this grammar

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

thank yoU!

Comment thread src/diff/file.rs
/// Whether this kind implies the file's content changed. A rename or
/// copy may carry hunks or be path-only; every other kind always
/// carries content.
pub fn is_content_change(self) -> bool {

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[issue] What is this function doing?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

nothing important. why?

Comment thread docs/diff-summary.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant