ci(ai-review): fit the unified diff into the codex input budget

[igerber]

Summary

• The AI PR review of feat(v4): PR 3 of the 4.0 cut - remove the fit-time aggregate=/balance_e= keywords #861 never ran: the compiled prompt reached 1,719,252 characters against codex's hard 1,048,576-character cap (input_too_large). The workflow capped tutorial notebook prose but put no budget on the unified diff itself (1.63M on that PR after the notebook exclusion).
• New tools/ai_review_diff_budget.py: a patch-to-patch filter. In budget, the output is byte-identical to the input. Over budget, whole per-file blocks (split on the column-0 diff --git header, which body lines can never start) are replaced by one-line stubs - tests/ / benchmarks/ / tools/ / .claude/ first, diff_diff/ / rust/ only when unavoidable, largest first, never a stub that is not smaller than the hunks it replaces - each carrying the literal-pathspec git diff command the agentic reviewer can run for that file. Output never exceeds the budget. Paths come from git's own headers (rename/copy, +++/---, C-quoted, binary, mode-only). Sizes are bytes, an upper bound on characters for UTF-8. stdlib only; no shell.
• .github/workflows/ai_pr_review.yml: stage the filter from BASE_SHA like the notebook extractor (untrimmed one-shot fallback when absent on base); append the unified diff LAST so its budget is PROMPT_BUDGET (1,000,000 bytes) minus the compiled size of everything else; log the final prompt size. Simulated on the feat(v4): PR 3 of the 4.0 cut - remove the fit-time aggregate=/balance_e= keywords #861 head: 981,368 bytes with 9 test-file diffs stubbed and every source, REGISTRY and ledger hunk kept.
• Tests: tests/test_ai_review_diff_budget.py (splitting, path parsing, tiering, greedy fit, bounded fallbacks, CLI incl. hex-SHA validation, workflow wiring); TestWorkflowDoesNotExecutePRHeadCode allowlists the staged filter.
• TODO row: the local codex review backend has the same gap (hand-trimmed pathspecs today).

Note for #861: the bootstrap fallback means #861's own pull_request runs still use its head YAML. After this merges, a rebase of #861 (or an /ai-review comment, which runs the default-branch YAML) picks up the budgeted prompt.

Methodology references (required if estimator / math changes)

• Method name(s): N/A - no methodology changes
• Paper / source link(s): N/A
• Any intentional deviations from the source (and why): None

Validation

• Tests added/updated: tests/test_ai_review_diff_budget.py (new, 43 tests), tests/test_openai_review.py (allowlist entry); tests/test_openai_review.py, tests/test_tracking_files.py, tests/test_changelog_fragments.py, tests/test_notebook_md_extract.py all pass (491 passed).
• Backtest / simulation / notebook evidence (if applicable): the edited build step was executed locally on the feat(v4): PR 3 of the 4.0 cut - remove the fit-time aggregate=/balance_e= keywords #861 head with the real PR title/body; compiled prompt 981,368 bytes, 9 stubs.

Security / privacy

• Confirm no secrets/PII in this PR: Yes

Changelog

• changelog.d/ fragment added (or N/A - no user-visible change): Yes (changelog.d/20261019-ai-review-diff-budget.md, Internal)

[github-actions]

Overall assessment: ✅ Looks good

No unmitigated P0 or P1 findings.

Executive summary

• No estimator, identification, weighting, or inference changes; no methodology discrepancy found.
• Budget checks passed for passthrough, tier ordering, empty input, and small-budget fallbacks.
• One P2 edge case affects recovery commands for non-UTF-8 filenames.
• Validation: 35 file-free unit cases, three workflow security checks, and CLI probes passed through a standard-library harness. Pytest was unavailable.

Methodology

No findings. Checked docs/methodology/REGISTRY.md; this diff affects review infrastructure only, so no estimator-specific paper cross-check is required.

Code Quality

• P2 — Recovery commands lose non-UTF-8 filename bytes.
  Location: tools/ai_review_diff_budget.py:L198-L199, L214-L221.
  Impact: Git permits filenames containing non-UTF-8 bytes. _decode(..., errors="replace") changes those bytes before constructing the literal pathspec. For example, Git’s quoted tests/bad\377.py becomes tests/bad�.py, so the generated command cannot retrieve the omitted file.
  Concrete fix: Retain the original path bytes and render a lossless, shell-escaped pathspec separately from the display name. Add a regression case containing \377.

Performance

No material findings for the intended oversized-diff workload.

Maintainability

No findings. Base-commit staging, deterministic ordering, and byte-identical passthrough are clear and covered by checks.

Tech Debt

• P3 — Local-backend budgeting is tracked.
  Location: TODO.md:L89.
  Impact: Local Codex reviews still require manual trimming for oversized patches.
  Concrete fix: Reuse the filter in the local backend as recorded in the new backlog row. Informational; no action required for this PR.

Security

No findings. The workflow executes the filter staged from BASE_SHA; SHA validation and path quoting preserve the existing execution boundary.

Documentation/Tests

No additional findings. The required changelog.d/ fragment is present. Tests cover structural splitting, path parsing, prioritization, bounded fallbacks, CLI validation, and workflow wiring.

[igerber]

Closing: we will split #861 into smaller PRs rather than trim what the reviewer sees.