fix(review): ground the PR summary in the whole change set (#335)

src/main/java/dev/thiagogonzaga/thrillhousebot/review/FindingPipeline.java

@@ -19,6 +19,7 @@
 import com.fasterxml.jackson.databind.ObjectMapper;
 import dev.thiagogonzaga.thrillhousebot.config.BotIdentity;
 import dev.thiagogonzaga.thrillhousebot.dashboard.ReviewSession;
+import dev.thiagogonzaga.thrillhousebot.github.GitHubPullRequestClient;
 import dev.thiagogonzaga.thrillhousebot.github.GitHubReviewClient;
 import dev.thiagogonzaga.thrillhousebot.review.ai.AiReviewService;
 import dev.thiagogonzaga.thrillhousebot.review.ai.FindingVerificationService;
@@ -50,6 +51,9 @@
 @ApplicationScoped
 public class FindingPipeline {
 
+  /** Directory rows listed in the scope header before the remainder is rolled up by count. */
+  private static final int MAX_SCOPE_DIRECTORIES = 10;
+
   private record BatchOutcome(
       int index,
       List<ReviewResponse.Finding> findings,
@@ -551,6 +555,8 @@ private static String changedFilesOverview(
     if (!pureRenames.isEmpty()) {
       sb.append(ReviewDiffFormatter.formatPureRenameRollup(pureRenames));
     }
+    // Scope totals next, ahead of the per-file rows, for the same reason: clamping drops the tail.
+    sb.append(changeScopeSummary(ctx));
     var omitted = Set.copyOf(plan.omittedFiles());
     var clipped = Set.copyOf(plan.clippedFiles());
     for (var file : ctx.reviewableFiles()) {
@@ -576,6 +582,93 @@ private static String changedFilesOverview(
     return sb.toString();
   }
 
+  /**
+   * PR-level scope header for the summary call: the authoritative file/line totals (GitHub's, the
+   * same numbers the rendered Changes Overview reports — #298) plus how the change is spread across
+   * directories. The summary call never sees the diff, so without these the only cue for how big
+   * the change is is a file list the input budget may have clamped — which is how a multi-file
+   * decompose got described as one extracted class (#335). Rendered as data, ahead of the per-file
+   * rows, so clamping can only take the tail.
+   */
+  private static String changeScopeSummary(ReviewContextLoader.ReviewContext ctx) {
+    var files = VerdictBuilder.overviewFiles(ctx);
+    var additions = 0;
+    var deletions = 0;
+    for (var file : files) {
+      additions += file.additions();
+      deletions += file.deletions();
+    }
+    var totals = ctx.prTotals();
+    // A non-positive file count means the totals carry nothing usable: fall back to the
+    // diff-derived counts rather than announcing a zero-file PR over a non-empty file list.
+    var authoritative = totals != null && totals.filesChanged() > 0;
+    var filesChanged = authoritative ? totals.filesChanged() : files.size();
+    if (authoritative) {
+      additions = totals.additions();
+      deletions = totals.deletions();
+    }
+    if (filesChanged <= 0) {
+      return "";
+    }
+    var sb = new StringBuilder();
+    sb.append("PR scope (whole pull request): ")
+        .append(filesChanged)
+        .append(filesChanged == 1 ? " file changed, +" : " files changed, +")
+        .append(additions)
+        .append(" -")
+        .append(deletions)
+        .append("\n");
+    appendDirectoryBreakdown(sb, files);
+    return sb.toString();
+  }
+
+  /**
+   * "directory: N files (+a -d)" rows, most files first, so a change spread over several packages
+   * cannot read as a single-file edit. Bounded so a wide PR cannot crowd out the file list.
+   */
+  private static void appendDirectoryBreakdown(
+      StringBuilder sb, List<GitHubPullRequestClient.FileDiff> files) {
+    if (files.isEmpty()) {
+      return;
+    }
+    var byDirectory = new LinkedHashMap<String, int[]>();
+    for (var file : files) {
+      var stats = byDirectory.computeIfAbsent(directoryOf(file.filename()), key -> new int[3]);
+      stats[0]++;
+      stats[1] += file.additions();
+      stats[2] += file.deletions();
+    }
+    var rows = new ArrayList<>(byDirectory.entrySet());
+    rows.sort(
+        Comparator.<Map.Entry<String, int[]>>comparingInt(e -> -e.getValue()[0])
+            .thenComparing(Map.Entry::getKey));
+    sb.append("Directories touched: ").append(byDirectory.size()).append("\n");
+    for (var row : rows.subList(0, Math.min(rows.size(), MAX_SCOPE_DIRECTORIES))) {
+      sb.append("- ")
+          .append(row.getKey())
+          .append(": ")
+          .append(row.getValue()[0])
+          .append(row.getValue()[0] == 1 ? " file (+" : " files (+")
+          .append(row.getValue()[1])
+          .append(" -")
+          .append(row.getValue()[2])
+          .append(")\n");
+    }
+    if (rows.size() > MAX_SCOPE_DIRECTORIES) {
+      sb.append("- (+").append(rows.size() - MAX_SCOPE_DIRECTORIES).append(" more directories)\n");
+    }
+  }
+
+  /**
+   * The file's parent directory, or a stable label for a path carrying no directory component —
+   * which also covers a blank one. A null path is not guarded here: the per-file loop above already
+   * dereferences the same name against an immutable set, so it could never reach this point.
+   */
+  private static String directoryOf(String path) {
+    var slash = path.lastIndexOf('/');
+    return slash <= 0 ? "(repository root)" : path.substring(0, slash);
+  }
+
   /**
    * Runs the raw model response through the full post-AI chain and persists it. The {@code
    * lineResolver} is shared with the caller's verdict backstop, so it is passed in rather than

src/main/java/dev/thiagogonzaga/thrillhousebot/review/ai/PrReviewPrompts.java

@@ -357,16 +357,25 @@ mismatches between what the author claims and what the code does (claimed change
             - Base summary.total_findings and the per-severity counts on the findings provided
               below — unless a "(+N more findings not shown …)" note follows the array; use that
               note's stated true totals then, since the list was truncated to fit your input.
-            - overall_assessment and pr_purpose must be consistent with those findings and the
-              changed-files list; do not contradict them.
+            - overall_assessment and pr_purpose must be consistent with those findings, with the PR
+              scope totals, and with the changed-files list; do not contradict them. A summary whose
+              scope is narrower than the stated PR scope is wrong: few or no findings means the
+              reviewed code looked fine, never that the change was small or touched one file.
 
             The "summary" object must include:
             - total_findings, critical, high, medium, low: counts of the findings provided below
             - overall_assessment: one-sentence verdict on the change
-            - pr_purpose: 1-3 sentences on what this change does, derived from the changed files and
-              findings — describe behavior, not file names
+            - pr_purpose: 1-3 sentences on what the WHOLE change set does. You do not see the diff,
+              so ground it in the PR title and description (the author's stated intent) together
+              with the PR scope totals and the changed-file list below, which are computed from the
+              diff and are authoritative. Cover the change set as a whole: when it spans many files
+              or several directories, say so and name the main areas it touches. Never present one
+              extracted class, one file, or the one component that happens to carry findings as if
+              it were the whole pull request. Describe behavior, not a file listing.
             - description_gaps: when a PR description is provided, an array of concrete mismatches
-              between what the author claims and what the change does. Empty array otherwise.
+              between what the author claims and what the change does — including a description
+              whose scope is narrower than the change itself (it covers one component, or far fewer
+              files than the PR scope totals report). Empty array otherwise.
             - file_summaries: an array of { path, summary } objects giving a file-by-file
               walkthrough. "path" must match a changed-file path exactly; "summary" is a single line
               (max ~100 chars) on what changed in that file. Most impactful first, cap at 15.
@@ -396,7 +405,9 @@ mismatches between what the author claims and what the code does (claimed change
             ## Findings already computed for this PR (final — summarize, do not change)
             {{findings}}
 
-            ## Changed files
+            ## PR scope and changed files (computed from the diff — authoritative)
+            Everything listed here belongs to this pull request; the purpose you write must
+            account for all of it, not just the entries with findings.
             {{changedFiles}}
 
             {{#if previousFindings}}

src/test/java/dev/thiagogonzaga/thrillhousebot/review/FindingPipelineTest.java

@@ -42,6 +42,7 @@
 import dev.thiagogonzaga.thrillhousebot.review.ai.PrReviewPrompts;
 import dev.thiagogonzaga.thrillhousebot.review.ai.ReviewResponse;
 import dev.thiagogonzaga.thrillhousebot.review.ai.TokenCounter;
+import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.CompletionException;
@@ -113,6 +114,17 @@ private static ReviewContextLoader.ReviewContext reviewContext() {
 
   /** Same context with a caller-supplied changed-file list, so rename disclosure can be driven. */
   private static ReviewContextLoader.ReviewContext reviewContext(List<FileDiff> files) {
+    return reviewContext(
+        files,
+        List.of(
+            new FileDiff("a.java", "modified", 3, 0, 3, ""),
+            new FileDiff("b.java", "modified", 2, 0, 2, "")),
+        null);
+  }
+
+  /** Same context with an explicit reviewable-file set and GitHub's PR totals (may be null). */
+  private static ReviewContextLoader.ReviewContext reviewContext(
+      List<FileDiff> files, List<FileDiff> reviewableFiles, ReviewContextLoader.PrTotals prTotals) {
     return new ReviewContextLoader.ReviewContext(
         files,
         "raw legacy diff",
@@ -130,11 +142,9 @@ private static ReviewContextLoader.ReviewContext reviewContext(List<FileDiff> fi
         List.of(),
         "",
         "",
-        List.of(
-            new FileDiff("a.java", "modified", 3, 0, 3, ""),
-            new FileDiff("b.java", "modified", 2, 0, 2, "")),
+        reviewableFiles,
         () -> new DiffLineResolver(Map.of()),
-        null);
+        prTotals);
   }
 
   private static DiffBudgetPlanner.BudgetPlan multiBatchPlan() {
@@ -275,6 +285,150 @@ void summaryOverviewLeadsWithThePureRenameRollup() {
     assertTrue(captor.getValue().changedFiles().contains("pkg/A.java → pkg/B.java"));
   }
 
+  /** Runs the multi-call path and returns the changed-files section the summary call received. */
+  private String captureSummaryChangedFiles(
+      ReviewSession session, ReviewContextLoader.ReviewContext ctx) {
+    var template = new AiReviewService.PromptInputs("d", "ctx", "base", "stack", "tests", "", "");
+    when(aiReviewService.reviewBatch(eq(session), any(), anyInt(), anyInt()))
+        .thenReturn(new ReviewResponse(List.of(), List.of(), null));
+    var captor = ArgumentCaptor.forClass(AiReviewService.SummaryInputs.class);
+    when(aiReviewService.summarize(eq(session), captor.capture()))
+        .thenReturn(new ReviewResponse(List.of(), List.of(), null));
+
+    pipeline.run(session, template, ctx, multiBatchPlan(), new DiffLineResolver(Map.of()));
+
+    return captor.getValue().changedFiles();
+  }
+
+  @Test
+  void summaryOverviewStatesTheWholePrScopeForAMultiFileRefactor() {
+    // The #335 fixture: a decompose whose title/body announce the full scope, but whose summary
+    // call sees no diff — without these totals nothing tells it the change is more than one class.
+    var session = ReviewSession.create("owner/repo", 1, "Decompose the orchestrator", "sha");
+    var files =
+        List.of(
+            new FileDiff(
+                "src/main/java/app/review/Orchestrator.java", "modified", 40, 900, 940, ""),
+            new FileDiff(
+                "src/main/java/app/review/CiStatusEvaluator.java", "added", 120, 0, 120, ""),
+            new FileDiff("src/main/java/app/review/FindingPipeline.java", "added", 300, 0, 300, ""),
+            new FileDiff("src/test/java/app/review/PipelineTest.java", "added", 200, 0, 200, ""),
+            new FileDiff("README.md", "modified", 4, 2, 6, ""));
+    var overview = captureSummaryChangedFiles(session, reviewContext(files, files, null));
+
+    assertTrue(
+        overview.contains("PR scope (whole pull request): 5 files changed, +664 -902"), overview);
+    assertTrue(overview.contains("Directories touched: 3"), overview);
+    assertTrue(overview.contains("- src/main/java/app/review: 3 files (+460 -900)"), overview);
+    assertTrue(overview.contains("- src/test/java/app/review: 1 file (+200 -0)"), overview);
+    assertTrue(overview.contains("- (repository root): 1 file (+4 -2)"), overview);
+    // Ahead of the per-file rows, so clamping a long overview can only drop the tail.
+    assertTrue(
+        overview.indexOf("PR scope (whole pull request)") < overview.indexOf("README.md (modified"),
+        overview);
+  }
+
+  @Test
+  void summaryOverviewScopeUsesGitHubsAuthoritativeTotalsWhenAvailable() {
+    // Same totals the rendered Changes Overview reports (#298), so the prose cannot contradict it.
+    var session = ReviewSession.create("owner/repo", 1, "Big PR", "sha");
+    var reviewable =
+        List.of(
+            new FileDiff("a.java", "modified", 3, 0, 3, ""),
+            new FileDiff("b.java", "modified", 2, 0, 2, ""));
+    var ctx = reviewContext(List.of(), reviewable, new ReviewContextLoader.PrTotals(23, 1612, 240));
+
+    var overview = captureSummaryChangedFiles(session, ctx);
+
+    assertTrue(
+        overview.contains("PR scope (whole pull request): 23 files changed, +1612 -240"), overview);
+  }
+
+  @Test
+  void summaryOverviewScopeStaysSingularForASingleFilePr() {
+    // No regression on small single-purpose PRs: no multi-file or multi-directory language.
+    var session = ReviewSession.create("owner/repo", 1, "Fix a typo", "sha");
+    var files = List.of(new FileDiff("src/main/java/app/Tiny.java", "modified", 3, 1, 4, ""));
+    var overview = captureSummaryChangedFiles(session, reviewContext(files, files, null));
+
+    assertTrue(overview.contains("PR scope (whole pull request): 1 file changed, +3 -1"), overview);
+    assertTrue(overview.contains("Directories touched: 1"), overview);
+    assertTrue(overview.contains("- src/main/java/app: 1 file (+3 -1)"), overview);
+    assertFalse(overview.contains("files changed"), overview);
+    assertFalse(overview.contains("more directories"), overview);
+  }
+
+  @Test
+  void summaryOverviewScopeFallsBackToDiffCountsWhenPrTotalsAreEmpty() {
+    // A zero file count means the totals carry nothing usable; announcing a zero-file PR over a
+    // non-empty file list would contradict the very list printed below it.
+    var session = ReviewSession.create("owner/repo", 1, "Totals unavailable", "sha");
+    var reviewable =
+        List.of(
+            new FileDiff("a.java", "modified", 3, 0, 3, ""),
+            new FileDiff("b.java", "modified", 2, 0, 2, ""));
+    var ctx = reviewContext(List.of(), reviewable, new ReviewContextLoader.PrTotals(0, 0, 0));
+
+    var overview = captureSummaryChangedFiles(session, ctx);
+
+    assertTrue(
+        overview.contains("PR scope (whole pull request): 2 files changed, +5 -0"), overview);
+  }
+
+  @Test
+  void summaryOverviewOmitsTheScopeBlockWhenNothingIsInTheChangeSet() {
+    // Nothing reviewable and no PR totals: emit no scope header at all rather than "0 files".
+    var session = ReviewSession.create("owner/repo", 1, "Everything ignored", "sha");
+
+    var overview = captureSummaryChangedFiles(session, reviewContext(List.of(), List.of(), null));
+
+    assertEquals("", overview);
+  }
+
+  @Test
+  void summaryOverviewScopeKeepsTotalsWhenNoFileSurvivesTheIgnoreGlob() {
+    // GitHub still reports the PR's real size when the ignore-glob drops every changed file, so
+    // the header stands on its own — with no directory breakdown, which would have to be empty.
+    var session = ReviewSession.create("owner/repo", 1, "All ignored", "sha");
+    var ctx = reviewContext(List.of(), List.of(), new ReviewContextLoader.PrTotals(23, 1612, 240));
+
+    var overview = captureSummaryChangedFiles(session, ctx);
+
+    assertEquals("PR scope (whole pull request): 23 files changed, +1612 -240\n", overview);
+  }
+
+  @Test
+  void summaryOverviewScopeBucketsAPathWithNoDirectoryAtTheRoot() {
+    // A name carrying no directory component — a blank one included — buckets at the root instead
+    // of opening a directory row named after it.
+    var session = ReviewSession.create("owner/repo", 1, "Odd payload", "sha");
+    var files =
+        List.of(
+            new FileDiff("  ", "modified", 2, 0, 2, ""),
+            new FileDiff("src/app/A.java", "modified", 4, 1, 5, ""));
+
+    var overview = captureSummaryChangedFiles(session, reviewContext(files, files, null));
+
+    assertTrue(
+        overview.contains("PR scope (whole pull request): 2 files changed, +6 -1"), overview);
+    assertTrue(overview.contains("Directories touched: 2"), overview);
+    assertTrue(overview.contains("- (repository root): 1 file (+2 -0)"), overview);
+    assertTrue(overview.contains("- src/app: 1 file (+4 -1)"), overview);
+  }
+
+  @Test
+  void summaryOverviewRollsUpDirectoriesBeyondTheCap() {
+    var session = ReviewSession.create("owner/repo", 1, "Wide PR", "sha");
+    var files = new ArrayList<FileDiff>();
+    for (var i = 0; i < 12; i++) {
+      files.add(new FileDiff("pkg" + i + "/File.java", "modified", 1, 0, 1, ""));
+    }
+    var overview = captureSummaryChangedFiles(session, reviewContext(files, files, null));
+
+    assertTrue(overview.contains("Directories touched: 12"), overview);
+    assertTrue(overview.contains("- (+2 more directories)"), overview);
+  }
+
   @Test
   void budgetedSingleBatchSendsThePlannedTextNotTheRawDiff() {
     var session = ReviewSession.create("owner/repo", 1, "One big file", "sha");
@@ -500,13 +654,26 @@ void summaryFindingsJsonIsClampedToThePerCallBudget() throws Exception {
     var session = ReviewSession.create("owner/repo", 1, "Big PR", "sha");
     var ctx = reviewContext();
     var template = new AiReviewService.PromptInputs("d", "d", "", "", "", "", "");
-    var high = new ReviewResponse.Finding("high", "high", "a.java", 1, "H", "d", "o", "n");
-    var medium = new ReviewResponse.Finding("medium", "high", "a.java", 3, "M", "d", "o", "n");
-    var nullRisk = new ReviewResponse.Finding(null, "high", "a.java", 2, "N", "d", "o", "n");
-    var critical = new ReviewResponse.Finding("critical", "high", "b.java", 1, "C", "d", "o", "n");
+    // Descriptions long enough that one finding outweighs the changed-files overview: the budget
+    // below leaves room for exactly one, and the overview keeps its own (larger) share unclamped.
+    var desc =
+        "this description exists to make one serialized finding the dominant cost ".repeat(3);
+    var high = new ReviewResponse.Finding("high", "high", "a.java", 1, "H", desc, "o", "n");
+    var medium = new ReviewResponse.Finding("medium", "high", "a.java", 3, "M", desc, "o", "n");
+    var nullRisk = new ReviewResponse.Finding(null, "high", "a.java", 2, "N", desc, "o", "n");
+    var critical = new ReviewResponse.Finding("critical", "high", "b.java", 1, "C", desc, "o", "n");
 
     var tokenCounter = new TokenCounter();
-    var overview = "a.java (modified, +3 -0)\nb.java (modified, +2 -0)\n";
+    // The overview the pipeline will build (scope header + per-file rows), so the budget below is
+    // calibrated against the same fixed sections the production code measures.
+    var overview =
+        """
+        PR scope (whole pull request): 2 files changed, +5 -0
+        Directories touched: 1
+        - (repository root): 2 files (+5 -0)
+        a.java (modified, +3 -0)
+        b.java (modified, +2 -0)
+        """;
     var fixedSections =
         PrReviewPrompts.SUMMARY_SYSTEM + PrReviewPrompts.SUMMARY_USER + "d" + overview;
     var criticalJson = new ObjectMapper().writeValueAsString(List.of(critical));