perf(review): exclude pure renames from AI review budget (#386) (#411)

- [ ] 🐛 Bug fix
- [ ] ✨ Feature
- [x] 📝 Documentation
- [ ] 🔧 Refactor
- [x] 🚀 Performance
- [x] ✅ Test
- [ ] 🔒 Security
- [ ] 📦 Dependency update
- [ ] 🏗️ CI/CD

Implements
[#386](#386).

Pure renames (`status=renamed`, `additions+deletions==0`, blank/null
patch) are excluded from `reviewableFiles()` so they never enter
`DiffBudgetPlanner` or line-capped diff sections. Rename+edit (non-empty
patch) stays reviewable. The summary / diff overview discloses a capped
rollup (`N pure renames omitted…`) and pure renames do **not** count
toward truncation / APPROVE hold.

Also maps GitHub’s `previous_filename` onto `FileDiff` for `old → new`
samples in the rollup.

Fixes #386

- [x] Unit tests
- [ ] Integration tests
- [ ] Manual testing

```bash
./mvnw -Dtest=ReviewDiffFormatterTest,DiffBudgetPlannerTest,ReviewContextLoaderTest test
```

118 tests, 0 failures. Updated `shouldHandleFilesWithZeroChanges` for
the new rollup behavior.

- [x] My code follows the project's coding standards
- [x] I have performed a self-review of my own code
- [x] I have added tests that prove my fix is effective or that my
feature works
- [x] New and existing unit tests pass locally with my changes
- [x] I have updated the documentation accordingly
- [x] My changes generate no new warnings or errors

N/A

Targets `release/v0.5.0` (milestone candidate).

Signed-off-by: Thiago Gonzaga <thiago.gonzaga@icloud.com>

Author: devops-thiago

README.md

@@ -514,6 +514,10 @@ This is still an early-stage project; the current constraints are:
   don't fit are disclosed by name instead of silently dropped. The on-demand commands
   (`/describe`, `/changelog`, `/add-docs`) still send the diff in a single call without
   batching.
+- **Pure renames** — files GitHub reports as `renamed` with zero additions/deletions and
+  no patch are omitted from AI review input (they have nothing to review). The summary
+  overview still lists a short rollup (`N pure renames omitted…`). Rename-plus-edit
+  (non-empty patch) stays in the budget.
 - **Single process** — OAuth login sessions, the live WebSocket replay buffer, and
   the per-PR auto-review rate-limit window are in-memory (lost on restart / not shared
   across replicas). Review history and cost totals persist in PostgreSQL.

src/main/java/dev/thiagogonzaga/thrillhousebot/github/GitHubPullRequestClient.java

@@ -118,7 +118,15 @@ record FileDiff(
       int additions,
       int deletions,
       int changes,
-      String patch) {}
+      String patch,
+      @JsonProperty("previous_filename") String previousFilename) {
+
+    /** Convenience overload when the rename source path is unused. */
+    public FileDiff(
+        String filename, String status, int additions, int deletions, int changes, String patch) {
+      this(filename, status, additions, deletions, changes, patch, null);
+    }
+  }
 
   record CompareResponse(@JsonProperty("total_commits") int totalCommits, List<FileDiff> files) {
     public CompareResponse {

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

@@ -545,6 +545,12 @@ private String findingsJson(List<ReviewResponse.Finding> findings) {
   private static String changedFilesOverview(
       ReviewContextLoader.ReviewContext ctx, DiffBudgetPlanner.BudgetPlan plan) {
     var sb = new StringBuilder();
+    // Pure-rename rollup first so clampOverview (keeps a prefix) never drops the disclosure on
+    // large multi-call reviews (#386).
+    var pureRenames = ReviewDiffFormatter.pureRenameFiles(ctx.files());
+    if (!pureRenames.isEmpty()) {
+      sb.append(ReviewDiffFormatter.formatPureRenameRollup(pureRenames));
+    }
     var omitted = Set.copyOf(plan.omittedFiles());
     var clipped = Set.copyOf(plan.clippedFiles());
     for (var file : ctx.reviewableFiles()) {

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

@@ -119,13 +119,94 @@ private static boolean matchesSuffix(PathMatcher suffix, Path path) {
     return false;
   }
 
-  /** Files that are included in AI review scope (non-ignored). */
+  /**
+   * Pure rename: GitHub reports {@code status=renamed} with no content hunks. These burn AI budget
+   * without anything to review — exclude them from model input (see #386). Rename+edit (non-empty
+   * patch and/or non-zero add/del) is still reviewable.
+   */
+  static boolean isPureRename(GitHubPullRequestClient.FileDiff file) {
+    if (file == null || file.status() == null) {
+      return false;
+    }
+    if (!"renamed".equalsIgnoreCase(file.status())) {
+      return false;
+    }
+    if (file.additions() + file.deletions() != 0) {
+      return false;
+    }
+    return file.patch() == null || file.patch().isBlank();
+  }
+
+  /** Pure renames in {@code files}, preserving order. */
+  static List<GitHubPullRequestClient.FileDiff> pureRenameFiles(
+      List<GitHubPullRequestClient.FileDiff> files) {
+    if (files == null || files.isEmpty()) {
+      return List.of();
+    }
+    return files.stream().filter(ReviewDiffFormatter::isPureRename).toList();
+  }
+
+  /**
+   * One-line disclosure for the summary / diff overview. Caps the path sample so bulk package moves
+   * do not dominate the prompt.
+   */
+  static String formatPureRenameRollup(List<GitHubPullRequestClient.FileDiff> pureRenames) {
+    if (pureRenames == null || pureRenames.isEmpty()) {
+      return "";
+    }
+    final int sampleCap = 5;
+    var samples = new ArrayList<String>(Math.min(sampleCap, pureRenames.size()));
+    for (var i = 0; i < pureRenames.size() && samples.size() < sampleCap; i++) {
+      var file = pureRenames.get(i);
+      var prev = file.previousFilename();
+      if (prev != null && !prev.isBlank()) {
+        samples.add(prev + " → " + file.filename());
+      } else {
+        samples.add(file.filename());
+      }
+    }
+    var sb = new StringBuilder();
+    sb.append(pureRenames.size())
+        .append(pureRenames.size() == 1 ? " pure rename" : " pure renames")
+        .append(" omitted from AI review (")
+        .append(String.join(", ", samples));
+    var more = pureRenames.size() - samples.size();
+    if (more > 0) {
+      sb.append(", and ").append(more).append(" more");
+    }
+    sb.append(")\n");
+    return sb.toString();
+  }
+
+  /** Files that are included in AI review scope (non-ignored, non–pure-rename). */
   List<GitHubPullRequestClient.FileDiff> reviewableFiles(
       List<GitHubPullRequestClient.FileDiff> files) {
     if (files == null || files.isEmpty()) {
       return List.of();
     }
-    return files.stream().filter(f -> !isIgnored(f.filename())).toList();
+    return files.stream()
+        .filter(f -> !isIgnored(f.filename()))
+        .filter(f -> !isPureRename(f))
+        .toList();
+  }
+
+  /**
+   * Reviewable files plus pure renames — for prompt context that should still see moved paths
+   * (related-tests list, summary walkthrough counts) without putting empty rename hunks in the
+   * model diff.
+   */
+  static List<GitHubPullRequestClient.FileDiff> withPureRenames(
+      List<GitHubPullRequestClient.FileDiff> reviewable,
+      List<GitHubPullRequestClient.FileDiff> allFiles) {
+    var renames = pureRenameFiles(allFiles);
+    if (renames.isEmpty()) {
+      return reviewable;
+    }
+    var merged =
+        new ArrayList<GitHubPullRequestClient.FileDiff>(reviewable.size() + renames.size());
+    merged.addAll(reviewable);
+    merged.addAll(renames);
+    return merged;
   }
 
   /**
@@ -181,10 +262,19 @@ FormattedDiff buildDiffStringWithStats(
       totalDeletions += file.deletions();
     }
 
+    var pureRenames = pureRenameFiles(files);
     var header =
-        String.format(
-            "## Overview: %d files (+%d -%d)%n%n", files.size(), totalAdditions, totalDeletions);
-    return formatWithLineBudget(header, files, namesOf(reviewableFiles));
+        new StringBuilder(
+            String.format(
+                "## Overview: %d files (+%d -%d)%n%n",
+                files.size(), totalAdditions, totalDeletions));
+    if (!pureRenames.isEmpty()) {
+      header.append(formatPureRenameRollup(pureRenames)).append('\n');
+    }
+    // Pure renames are disclosed in the rollup only — do not emit empty ### headers into the
+    // model input (or count them toward the line-budget truncation / APPROVE hold).
+    var sectionFiles = files.stream().filter(f -> !isPureRename(f)).toList();
+    return formatWithLineBudget(header.toString(), sectionFiles, namesOf(reviewableFiles));
   }
 
   static Set<String> namesOf(List<GitHubPullRequestClient.FileDiff> files) {

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

@@ -58,7 +58,11 @@ AiReviewService.PromptInputs assemble(
     // The diagram request's presence is what gates the model's walkthrough_diagram field.
     String diagramGuidance =
         config.review().diagram().enabled() ? PrReviewPrompts.DIAGRAM_REQUEST : "";
-    String relatedTests = diffFormatter.buildRelatedTests(ctx.reviewableFiles());
+    // Include pure-renamed test files so mock-fidelity / related-tests guidance still sees moves
+    // even though empty rename hunks are excluded from the reviewable diff (#386).
+    String relatedTests =
+        diffFormatter.buildRelatedTests(
+            ReviewDiffFormatter.withPureRenames(ctx.reviewableFiles(), ctx.files()));
     String trailingGuidance =
         combineSections(
             combineSections(

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

@@ -123,15 +123,16 @@ ReviewResult build(
             ? plan.omittedFiles().size() + plan.clippedFiles().size()
             : ctx.omittedFiles();
     // GitHub PR-level totals when available; ignore-glob drops can undercount diff-derived stats.
+    // Pure renames are excluded from reviewableFiles for AI budget (#386) but still belong in the
+    // fallback file count / walkthrough when PR totals could not be fetched.
+    var overviewFiles = overviewFiles(ctx);
     var diffStats =
-        DiffStats.fromFiles(ctx.reviewableFiles(), omitted, truncation)
+        DiffStats.fromFiles(overviewFiles, omitted, truncation)
             .withAuthoritativeTotals(ctx.prTotals());
     var omittedNames = Set.copyOf(truncation.omittedFileNames());
     var changedFiles =
         toChangedFiles(
-            ctx.reviewableFiles().stream()
-                .filter(f -> !omittedNames.contains(f.filename()))
-                .toList());
+            overviewFiles.stream().filter(f -> !omittedNames.contains(f.filename())).toList());
     // A model-reported "unresolved" whose targeted code left the diff (force-push) becomes
     // "superseded" before the gates run, so a vanished finding never holds APPROVE (#336).
     // Skip the DiffLineResolver when there are no statuses — first reviews and empty-status
@@ -296,6 +297,24 @@ static DiffStats fromFiles(
     }
   }
 
+  /**
+   * Reviewable files plus pure renames — the fallback set for Changes Overview counts when GitHub
+   * PR totals are unavailable. Pure renames are omitted from AI input but still part of the PR.
+   */
+  static List<GitHubPullRequestClient.FileDiff> overviewFiles(
+      ReviewContextLoader.ReviewContext ctx) {
+    var pureRenames = ReviewDiffFormatter.pureRenameFiles(ctx.files());
+    if (pureRenames.isEmpty()) {
+      return ctx.reviewableFiles();
+    }
+    var merged =
+        new ArrayList<GitHubPullRequestClient.FileDiff>(
+            ctx.reviewableFiles().size() + pureRenames.size());
+    merged.addAll(ctx.reviewableFiles());
+    merged.addAll(pureRenames);
+    return merged;
+  }
+
   /**
    * Projects the reviewed diff onto the (path, change type) rows the summary walkthrough renders.
    */

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

@@ -324,4 +324,20 @@ void aModelCapNeverReenablesExplicitlyDisabledBudgeting() {
     when(reviewConfig.maxInputTokens()).thenReturn(0);
     assertEquals(Integer.MAX_VALUE, planner.perCallInputBudget());
   }
+
+  @Test
+  void mixedPrPackingPrefersRealDiffsAfterPureRenamesAreFilteredOut() {
+    // Mirrors the orchestrator path: reviewableFiles() drops pure renames before plan().
+    var pure = new FileDiff("moved/B.java", "renamed", 0, 0, 0, null, "moved/A.java");
+    var real = file("src/App.java", 8, patch(8));
+    var reviewable = formatter.reviewableFiles(List.of(pure, real));
+
+    assertEquals(List.of("src/App.java"), reviewable.stream().map(FileDiff::filename).toList());
+
+    when(reviewConfig.maxAiCalls()).thenReturn(3);
+    var plan = planner.plan(reviewable, sectionTokens(real) + 10, 2);
+
+    assertEquals(List.of("src/App.java"), coveredFilenames(plan));
+    assertTrue(plan.omittedFiles().isEmpty(), "pure rename must not appear as budget omission");
+  }
 }

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

@@ -144,12 +144,17 @@ void shouldAccumulateTotalsAcrossMultipleFiles() {
     void shouldHandleFilesWithZeroChanges() {
       var files =
           List.of(
-              new GitHubPullRequestClient.FileDiff("renamed-only.txt", "renamed", 0, 0, 0, null));
+              new GitHubPullRequestClient.FileDiff(
+                  "renamed-only.txt", "renamed", 0, 0, 0, null, "old-name.txt"));
 
       var result = diffFormatter.buildDiffString(files);
 
       assertTrue(result.contains("## Overview: 1 files (+0 -0)"));
-      assertTrue(result.contains("renamed-only.txt (renamed, +0 -0)"));
+      // Pure renames are disclosed in a rollup, not as empty ### sections (#386).
+      assertTrue(
+          result.contains(
+              "1 pure rename omitted from AI review (old-name.txt → renamed-only.txt)"));
+      assertFalse(result.contains("### renamed-only.txt"));
     }
   }
 

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

@@ -680,4 +680,81 @@ void isTestFileShouldRejectNullAndNonTestNames() {
     assertTrue(ReviewDiffFormatter.isTestFile("data_test.csv"));
     assertTrue(ReviewDiffFormatter.isTestFile("native/FooTest.kt"));
   }
+
+  @Nested
+  class PureRenameExclusion {
+
+    @Test
+    void isPureRenameRequiresRenamedStatusZeroDiffAndBlankPatch() {
+      assertTrue(
+          ReviewDiffFormatter.isPureRename(
+              new GitHubPullRequestClient.FileDiff("b.java", "renamed", 0, 0, 0, null, "a.java")));
+      assertTrue(
+          ReviewDiffFormatter.isPureRename(
+              new GitHubPullRequestClient.FileDiff("b.java", "RENAMED", 0, 0, 0, "  ", "a.java")));
+      // Rename + content edit must stay reviewable.
+      assertFalse(
+          ReviewDiffFormatter.isPureRename(
+              new GitHubPullRequestClient.FileDiff(
+                  "b.java", "renamed", 1, 0, 1, "@@ -1 +1,2 @@\n+x", "a.java")));
+      assertFalse(
+          ReviewDiffFormatter.isPureRename(
+              new GitHubPullRequestClient.FileDiff("b.java", "modified", 0, 0, 0, null)));
+    }
+
+    @Test
+    void reviewableFilesSkipsPureRenamesButKeepsRenamePlusEdit() {
+      var formatter = new ReviewDiffFormatter(List.of(), 5000);
+      var pure =
+          new GitHubPullRequestClient.FileDiff(
+              "pkg/B.java", "renamed", 0, 0, 0, null, "pkg/A.java");
+      var edited =
+          new GitHubPullRequestClient.FileDiff(
+              "pkg/D.java", "renamed", 2, 0, 2, "@@ -1 +1,3 @@\n+y\n+z", "pkg/C.java");
+      var modified = file("src/App.java", "modified", 1, 0, "@@ -1 +1,2 @@\n+ok");
+
+      var reviewable = formatter.reviewableFiles(List.of(pure, edited, modified));
+
+      assertEquals(2, reviewable.size());
+      assertEquals("pkg/D.java", reviewable.get(0).filename());
+      assertEquals("src/App.java", reviewable.get(1).filename());
+    }
+
+    @Test
+    void buildDiffStringDisclosesPureRenamesWithoutEmittingEmptySections() {
+      var formatter = new ReviewDiffFormatter(List.of(), 5000);
+      var files =
+          List.of(
+              new GitHubPullRequestClient.FileDiff(
+                  "new/Name.java", "renamed", 0, 0, 0, null, "old/Name.java"),
+              file("src/App.java", "modified", 1, 0, "@@ -1 +1,2 @@\n+ok"));
+
+      var result = formatter.buildDiffStringWithStats(files);
+
+      assertEquals(0, result.omittedFiles(), "pure renames must not count as truncation");
+      assertTrue(
+          result
+              .text()
+              .contains("1 pure rename omitted from AI review (old/Name.java → new/Name.java)"));
+      assertFalse(result.text().contains("### new/Name.java"));
+      assertTrue(result.text().contains("### src/App.java"));
+    }
+
+    @Test
+    void pureRenameRollupCapsSampleAndReportsRemainder() {
+      var renames = new java.util.ArrayList<GitHubPullRequestClient.FileDiff>();
+      for (var i = 0; i < 7; i++) {
+        renames.add(
+            new GitHubPullRequestClient.FileDiff(
+                "n" + i + ".java", "renamed", 0, 0, 0, null, "o" + i + ".java"));
+      }
+
+      var rollup = ReviewDiffFormatter.formatPureRenameRollup(renames);
+
+      assertTrue(rollup.startsWith("7 pure renames omitted from AI review ("));
+      assertTrue(rollup.contains("and 2 more"));
+      assertTrue(rollup.contains("o0.java → n0.java"));
+      assertFalse(rollup.contains("o5.java"));
+    }
+  }
 }