diff --git a/.github/scripts/build_review_diff.py b/.github/scripts/build_review_diff.py new file mode 100755 index 000000000..0ab1c67c2 --- /dev/null +++ b/.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() diff --git a/.github/workflows/ai-review.yml b/.github/workflows/ai-review.yml index df3e7461a..5142047b8 100644 --- a/.github/workflows/ai-review.yml +++ b/.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 <