Skip to content

fix(review): PR summary "Changes Overview" counts diverge from GitHub's file/line totals #298

Description

@devops-thiago

The PR summary "Changes Overview" reports file/line counts derived from the bot's own reviewable-file list, not GitHub's authoritative PR totals, so it undercounts whenever files are removed by the ignore-glob. See #39 (walkthrough / file-by-file origin) and #234 (truncation disclosure). Not #239 (that is per-model tokenizer for token counts).

Problem

PrSummaryGenerator.generate renders "Changes Overview" straight from its filesChanged/additions/deletions int params (PrSummaryGenerator.java:102105):

sb.append("### Changes Overview\n");
sb.append("- **Files changed:** ").append(filesChanged).append("\n");
sb.append("- **Lines added:** ").append(signed('+', additions)).append("\n");
sb.append("- **Lines removed:** ").append(signed('-', deletions)).append("\n\n");

Those params come from DiffStats.fromFiles(ctx.reviewableFiles(), ctx.omittedFiles()) (VerdictBuilder.java:60), passed into summaryGenerator.generate(...) at VerdictBuilder.java:229232. DiffStats.fromFiles sums per-file additions()/deletions() over reviewableFiles and uses files.size() for the file count (VerdictBuilder.java:165172); the omittedFiles argument is stored only as the 4th record field to drive the truncated() flag and does not reduce the counts.

reviewableFiles is the ignore-glob-filtered subset of the PR's files (ReviewContextLoader.java:120ReviewDiffFormatter.reviewableFiles, which filters out isIgnored(...) paths). So any file removed by the ignore-glob contributes nothing to the count. The result is a "Changes Overview" derived from the filtered file list rather than GitHub's authoritative changedFiles/additions/deletions, and it silently undercounts.

Note: line-budget truncation (omittedFiles, ReviewContextLoader.java:127) does not currently undercount this section — DiffStats.fromFiles still sums over the full reviewableFiles list regardless of how many files the rendered diff dropped. It is the ignore-glob that causes the divergence today. (The changedFiles.size() used for the walkthrough rollup — PrSummaryGenerator.java:276 — is the same ignore-glob-filtered source.)

Evidence

Dogfood on devops-thiago/MongOCOM#46. Posted summary "Changes Overview":

### Changes Overview
- Files changed: 26
- Lines added: +958
- Lines removed: -186

GitHub's own totals (gh pr view 46, verified live: changedFiles=27 additions=975 deletions=196):

changedFiles  27
additions     975
deletions     196

The 26 matches the walkthrough table exactly — 20 rows + "…and 6 more file(s)" = 26 — confirming the count is the ignore-glob-filtered file list, not GitHub's total (27). The line counts are short by the same reasoning (+958/-186 vs +975/-196), i.e. one file was dropped by the ignore-glob.

Secondary (minor, optional): "Key Findings" cited the HIGH finding at ci.yml:4 while the inline comment anchored ci.yml:5 — a one-line drift, likely a separate anchor issue.

Where

  • PrSummaryGenerator.generate — the "Changes Overview" rendering (PrSummaryGenerator.java:88105).
  • VerdictBuilder.DiffStats.fromFiles — the counts fed in, computed from reviewableFiles (VerdictBuilder.java:155172).
  • VerdictBuilder.build — the DiffStats.fromFiles(ctx.reviewableFiles(), ctx.omittedFiles()) call site (VerdictBuilder.java:60); the summaryGenerator.generate(...) call is in VerdictBuilder.buildResult (VerdictBuilder.java:229).
  • ReviewContextLoader.ReviewContext — already carries the full unfiltered files list (ReviewContextLoader.java:82), but no PR-level totals.
  • GitHubPullRequestClient.PullRequestDetails — currently only models title, body, head, base (GitHubPullRequestClient.java:98); it does not carry changed_files/additions/deletions, so the fix must add those fields.

Proposed fix

  1. Surface GitHub's authoritative PR totals — changed_files, additions, deletions — by adding them to PullRequestDetails (the same fields gh pr view reads), plumbed through ReviewContext alongside the existing file list.
  2. Populate the "Changes Overview" filesChanged/additions/deletions from those totals rather than from DiffStats.fromFiles(reviewableFiles).
  3. Keep the 20-row walkthrough cap (MAX_FILE_ROWS) and the "…and N more file(s)" note as-is — but derive the "N more" from the authoritative file total, not from the reviewable-list size, so the rollup count matches too.
  4. On a truncated PR, "Changes Overview" should reflect the full GitHub totals while the walkthrough table and truncation banner disclose the omission (ties to fix(review): disclose diff truncation and don't auto-APPROVE a truncated review #234). Interim: if PR-level totals cannot be fetched, fall back to the current diff-derived counts rather than failing the summary.

Acceptance criteria

  • "Changes Overview" Files changed / Lines added / Lines removed equal GitHub's changed_files / additions / deletions for the PR.
  • On MongOCOM#46's shape (27 files, +975/-196, 20-row cap), the overview reads 27 / +975 / −196 and the walkthrough still shows 20 rows + "…and 7 more file(s)".
  • Non-truncated PR with no ignore-globbed files: overview counts unchanged.
  • Ignore-globbed PR: overview shows full GitHub totals; the ignored files are still excluded from the walkthrough table.
  • Truncated PR: overview shows full GitHub totals while the walkthrough rollup + truncation banner disclose the omission.
  • PrSummaryGeneratorTest covers a case where reviewable-file counts diverge from the passed-in authoritative totals.

Related

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions