Stop the AI review failing on a large PR, and point it at the right files

.github/scripts/build_review_diff.py

@@ -0,0 +1,80 @@
+#!/usr/bin/env python3
+"""Build a review diff from the per-file API payload, within a byte budget.
+
+The diff endpoint caps at 20,000 lines (HTTP 406); the files endpoint does not,
+so the diff is reassembled from per-file patches. Selection is file-aware: whole
+patches are taken in priority order until the budget is spent, so the model sees
+complete hunks of the files that matter instead of a byte-sliced prefix of
+whatever sorted first.
+"""
+import json, os, sys
+
+BUDGET = int(os.environ.get("DIFF_BUDGET", "80000"))
+# No single file may take more than this, or two big ones crowd out everything
+# else: on PR #1107, size-ordered with no cap, three files filled the budget.
+PER_FILE = max(6000, BUDGET // 10)
+
+# Paths that are noise in a review: generated, vendored, fixtures, append-only logs.
+# Matched against "/" + filename, so every pattern starts with "/" and therefore
+# matches on a path-segment boundary. The repo has BOTH a root testdata/ and
+# per-package ones; matching the bare word instead would also have caught
+# docs/notes-testdata/, which is not fixtures.
+SKIP = (
+    "/mdl/grammar/parser/",
+    "/.claude/skills/fix-issue/findings/",
+    "/testdata/",
+    "/vscode-mdl/vscode-mdl-",
+)
+SKIP_SUFFIX = (".bson", ".mpr", ".vsix", ".lockb", ".sum", ".png", ".jpg", ".gif", ".pdf")
+
+def rank(f):
+    n = f["filename"]
+    if n.endswith((".go", ".g4")):                       return 0   # the code
+    if n.startswith(".github/") or n == "Makefile":      return 1   # the build
+    if n.endswith((".mdl", ".json", ".yaml", ".yml")):   return 2   # examples, config
+    if n.endswith(".md"):                                return 3   # docs
+    return 4
+
+def noise(n):
+    return n.endswith(SKIP_SUFFIX) or any(s in "/" + n for s in SKIP)
+
+def main():
+    files = json.load(open(sys.argv[1]))
+    out, manifest = [], []
+    included = skipped_noise = omitted_budget = 0
+    used = 0
+
+    for f in files:
+        manifest.append("  %-6s +%-5d -%-5d %s" % (f["status"][:6], f["additions"],
+                                                   f["deletions"], f["filename"]))
+
+    for f in sorted(files, key=lambda f: (rank(f), -(f["additions"] + f["deletions"]), f["filename"])):
+        n, patch = f["filename"], f.get("patch")
+        if patch is None:                       # binary, or too large for a patch
+            continue
+        if noise(n):
+            skipped_noise += 1
+            continue
+        clipped = ""
+        if len(patch) > PER_FILE:
+            patch = patch[:patch.rfind("\n", 0, PER_FILE) + 1]
+            clipped = "... [patch clipped at %d bytes; %d/%d lines changed]\n" % (
+                PER_FILE, f["additions"] + f["deletions"], f["changes"])
+        block = "diff --git a/%s b/%s\n--- a/%s\n+++ b/%s\n%s\n%s" % (n, n, n, n, patch, clipped)
+        if used + len(block) > BUDGET:
+            omitted_budget += 1
+            continue                            # keep trying: a later file may fit
+        out.append(block); used += len(block); included += 1
+
+    with open(os.environ.get("DIFF_OUT", "/tmp/pr.diff"), "w") as fh:
+        fh.write("".join(out))
+    with open(os.environ.get("MANIFEST_OUT", "/tmp/pr-manifest.txt"), "w") as fh:
+        fh.write("\n".join(manifest) + "\n")
+
+    print("files=%d included=%d noise_skipped=%d over_budget=%d bytes=%d"
+          % (len(files), included, skipped_noise, omitted_budget, used))
+    # for the workflow's truncation note
+    with open(os.environ.get("SUMMARY_OUT", "/tmp/diff-summary.txt"), "w") as fh:
+        fh.write("%d %d %d %d" % (len(files), included, skipped_noise, omitted_budget))
+
+main()

.github/workflows/ai-review.yml

@@ -23,20 +23,30 @@ jobs:
         id: diff
         env:
           GH_TOKEN: ${{ github.token }}
+          PR: ${{ github.event.pull_request.number }}
+          REPO: ${{ github.repository }}
+          DIFF_BUDGET: "80000"
         run: |
-          gh pr diff ${{ github.event.pull_request.number }} > /tmp/pr-full.diff
-          FULL_SIZE=$(wc -c < /tmp/pr-full.diff)
-          echo "diff_size=$FULL_SIZE" >> "$GITHUB_OUTPUT"
-
-          # Truncate to ~80k chars to stay within API limits
-          if [ "$FULL_SIZE" -gt 80000 ]; then
-            head -c 80000 /tmp/pr-full.diff > /tmp/pr.diff
-            echo "truncated=true" >> "$GITHUB_OUTPUT"
-          else
-            cp /tmp/pr-full.diff /tmp/pr.diff
-            echo "truncated=false" >> "$GITHUB_OUTPUT"
+          # The diff endpoint caps at 20,000 lines and answers HTTP 406 above it,
+          # which took this job down on #1106 and #1107 (21,366 lines) -- the one
+          # hard-fail path in a workflow whose every other error is a warning.
+          # The per-file endpoint has no such cap, so the review diff is rebuilt
+          # from per-file patches, and a fetch problem downgrades the review
+          # instead of failing the PR.
+          if ! gh api "repos/$REPO/pulls/$PR/files" --paginate > /tmp/pr-files.raw 2>/tmp/pr-files.err; then
+            echo "::warning::could not fetch PR file patches: $(head -c 300 /tmp/pr-files.err)"
+            : > /tmp/pr.diff
+            : > /tmp/pr-manifest.txt
+            echo "status=unavailable" >> "$GITHUB_OUTPUT"
+            exit 0
           fi
 
+          jq -s 'add' /tmp/pr-files.raw > /tmp/pr-files.json
+          STATS=$(python3 .github/scripts/build_review_diff.py /tmp/pr-files.json)
+          echo "$STATS"
+          echo "status=ok" >> "$GITHUB_OUTPUT"
+          echo "stats=$STATS" >> "$GITHUB_OUTPUT"
+
       - name: Get PR info
         env:
           GH_TOKEN: ${{ github.token }}
@@ -68,14 +78,16 @@ jobs:
 
       - name: Build API request
         env:
-          TRUNCATED: ${{ steps.diff.outputs.truncated }}
-          DIFF_SIZE: ${{ steps.diff.outputs.diff_size }}
+          DIFF_STATUS: ${{ steps.diff.outputs.status }}
+          DIFF_STATS: ${{ steps.diff.outputs.stats }}
         run: |
-          TRUNCATION_NOTE=""
-          if [ "$TRUNCATED" = "true" ]; then
-            TRUNCATION_NOTE="NOTE: The diff was truncated to 80k characters. Total size: ${DIFF_SIZE} bytes. Focus on what is visible."
+          if [ ! -s /tmp/pr.diff ]; then
+            echo "::warning::no reviewable diff (status=$DIFF_STATUS) -- skipping review"
+            exit 0
           fi
 
+          TRUNCATION_NOTE="NOTE: the diff below is SELECTED, not complete (${DIFF_STATS}). Whole file patches were taken most-changed-first, capped per file, skipping generated and fixture paths; a clipped patch says so inline. The manifest lists every changed file, so say when a finding needs a file that is not shown rather than assuming it is absent."
+
           # Write the user prompt to a file (avoids shell variable limits)
           cat > /tmp/user-prompt.txt <<PROMPT
           Review this pull request.
@@ -92,6 +104,9 @@ jobs:
           Existing doctype test files:
           $(cat /tmp/doctype-tests.txt)
 
+          Every file changed by this PR (status, +added, -deleted):
+          $(cat /tmp/pr-manifest.txt)
+
           ${TRUNCATION_NOTE}
 
           Review against ALL of the following checklist items:
@@ -181,6 +196,11 @@ jobs:
         env:
           OPENROUTER_API_KEY: ${{ secrets.OPENROUTER_API_KEY }}
         run: |
+          if [ ! -f /tmp/request.json ]; then
+            echo "No request built (no reviewable diff). Nothing to do."
+            exit 0
+          fi
+
           if [ -z "$OPENROUTER_API_KEY" ]; then
             echo "::warning::OPENROUTER_API_KEY secret is not set"
             exit 0