Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 49 additions & 12 deletions .github/workflows/ai_pr_review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,16 @@ jobs:
echo "Base SHA does not contain tools/notebook_md_extract.py; tutorial prose extraction will be skipped (one-shot bootstrap)."
fi

# Stage the diff budget filter from BASE_SHA for the same reason.
# Bootstrap-safe: absent on base, the unified diff is appended
# untrimmed (the pre-filter behaviour).
if git show "${BASE_SHA}":tools/ai_review_diff_budget.py > /tmp/ai_review_diff_budget.py 2>/dev/null; then
echo "Diff budget filter staged from base SHA ${BASE_SHA}."
else
rm -f /tmp/ai_review_diff_budget.py
echo "Base SHA does not contain tools/ai_review_diff_budget.py; the unified diff will be appended untrimmed (one-shot bootstrap)."
fi

# Sanitize untrusted text so hostile content can't close the
# wrapper tags and inject instructions to the reviewer.
# Case- and whitespace-tolerant; PR_TITLE / PR_BODY / PREV_REVIEW
Expand Down Expand Up @@ -338,19 +348,10 @@ jobs:
echo ""
echo "Changed files:"
git --no-pager diff --name-status "$BASE_SHA" "$HEAD_SHA"
echo ""
echo "Unified diff (context=5):"
# Exclude large generated/data files from the full diff to stay
# within the model's input limit. The --name-status above still
# lists them. Narrowed to real-data assets, notebook outputs, and
# the qte golden fixture (generated single-line JSON, ~700KB - a
# regeneration blew the input limit at PR #688; the reviewable
# artifact is benchmarks/R/generate_qte_golden.R).
git --no-pager diff --unified=5 "$BASE_SHA" "$HEAD_SHA" \
-- . ':!benchmarks/data/real/*.json' ':!benchmarks/data/real/*.csv' \
':!benchmarks/data/qte_golden.json' \
':!docs/tutorials/*.ipynb'
} >> "$PROMPT"
# The unified diff is appended LAST (after the notebook prose
# below) so its budget can be computed from the final size of
# everything else in the prompt.

# Tutorial notebook prose extraction: substitute a markdown view
# for the .ipynb JSON that the unified diff excludes. The extractor
Expand Down Expand Up @@ -512,6 +513,42 @@ jobs:
} >> "$PROMPT"
fi

# Unified diff, fitted to the remaining prompt budget.
# Exclude large generated/data files from the full diff to stay
# within the model's input limit. The --name-status above still
# lists them. Narrowed to real-data assets, notebook outputs, and
# the qte golden fixture (generated single-line JSON, ~700KB - a
# regeneration blew the input limit at PR #688; the reviewable
# artifact is benchmarks/R/generate_qte_golden.R).
git --no-pager diff --unified=5 "$BASE_SHA" "$HEAD_SHA" \
-- . ':!benchmarks/data/real/*.json' ':!benchmarks/data/real/*.csv' \
':!benchmarks/data/qte_golden.json' \
':!docs/tutorials/*.ipynb' > /tmp/ai-review-diff.patch
# Codex rejects a prompt above 1,048,576 characters outright
# (input_too_large; PR #861 compiled to 1.72M). PROMPT_BUDGET is the
# whole-prompt target in BYTES (a bound on characters for UTF-8),
# leaving ~48K headroom under the hard cap; the diff gets whatever
# the rules, PR text, previous review, file list and notebook prose
# left. The filter (staged from BASE_SHA above) passes an in-budget
# patch through byte-for-byte and otherwise replaces whole per-file
# blocks with one-line stubs, largest and lowest-value paths first,
# each carrying the git command that shows that file's hunks.
PROMPT_BUDGET=1000000
{
echo ""
echo "Unified diff (context=5):"
} >> "$PROMPT"
if [ -f /tmp/ai_review_diff_budget.py ]; then
DIFF_BUDGET=$(( PROMPT_BUDGET - $(wc -c < "$PROMPT") ))
if [ "$DIFF_BUDGET" -lt 0 ]; then DIFF_BUDGET=0; fi
python3 /tmp/ai_review_diff_budget.py --diff /tmp/ai-review-diff.patch \
--budget "$DIFF_BUDGET" --base-sha "$BASE_SHA" --head-sha "$HEAD_SHA" >> "$PROMPT"
else
cat /tmp/ai-review-diff.patch >> "$PROMPT"
fi
rm -f /tmp/ai-review-diff.patch
echo "Compiled prompt size: $(wc -c < "$PROMPT") bytes (target ${PROMPT_BUDGET})."

- name: Run Codex
id: run_codex
if: steps.pr.outputs.is_fork == 'false'
Expand Down
1 change: 1 addition & 0 deletions TODO.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ generic sparse-FE, QR+SVD rank-detection redundancy, `check_finite` bypass — m

| Issue | Location | Origin | Effort | Priority |
|-------|----------|--------|--------|----------|
| Local codex review (`openai_review.py --backend codex`) has no diff budget - an oversized PR needs hand-trimmed `:(exclude)` pathspecs (#861 compiled to 1.35 MB locally); reuse `tools/ai_review_diff_budget.py` on the codex-backend patch the way the CI workflow does | `.claude/scripts/openai_review.py` | #861 | Quick | Low |
| Re-validate the plan-review dual engine on `gpt-6.1-sol` with `tools/plan-review-eval`: re-pin every arm's composite `cli_version` to the live Claude Code + codex CLI pair, add a gpt-6.1-sol arm, run the harness against the gpt-5.6-sol baseline (the 2026-10 swap carried the model over from the code-review campaign 2 verdict without a plan-review campaign). | `tools/plan-review-eval/config/configs.json` | reviewer-eval gpt61sol PR-2 | Heavy | Low |
| TWFE-weights tutorial section: `attgt_weights` (all three `type=` values side by side) + `decompose_twfe_weights` + `plot_twfe_weights` on `mpdta`, either as a section of `02_staggered_did.ipynb` or a new notebook registered in `docs/tutorials/index.rst`; follow-on PR by decision, the feature ships without it | `docs/tutorials/02_staggered_did.ipynb` | #812 | Mid | Medium |
| Geo-experiments tutorial: the SDiD fits on the `generate_factor_data(...)` panel (conversions ~1,500, sigma-hat ~310) hit the Frank-Wolfe iteration cap - the placebo fit warns once and the bootstrap cross-check warns on 100/100 draws - yet the narrative presents the bootstrap SE as a paper-faithful robustness check. Pre-existing on `main` under BOTH backends (surfaced when the notebook outputs were refreshed for the shape-only fit diagnostic); the committed April outputs predate the aggregated non-convergence warning. Either rescale / re-tune the DGP (or raise `max_iter` / loosen `min_decrease`) so the refits converge and re-execute, or label the bootstrap result unreliable and drop it from the "Validity evidence" claim. | `docs/tutorials/18_geo_experiments.ipynb` | local codex review of the SDiD shape-only fit-RMSE PR | Mid | Low |
Expand Down
13 changes: 13 additions & 0 deletions changelog.d/20261019-ai-review-diff-budget.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
### Internal
- **CI AI review fits oversized diffs into the codex input cap.** The compiled review
prompt of PR #861 reached 1.72M characters against codex's hard 1,048,576-character
limit (`input_too_large`), so the review never ran; the workflow capped notebook prose
but never the unified diff. `.github/workflows/ai_pr_review.yml` now appends the diff
last, under a whole-prompt byte budget (1,000,000) derived from everything else already
in the prompt, through the new `tools/ai_review_diff_budget.py` (staged from `BASE_SHA`
like the notebook extractor; untrimmed one-shot fallback when absent on base). An
in-budget patch passes through byte-for-byte; an oversized one has whole per-file blocks
replaced by one-line stubs - `tests/` / `benchmarks/` / `tools/` first, `diff_diff/` /
`rust/` only when unavoidable, largest first - each naming the `git diff` command the
reviewer can run for that file. Pinned by `tests/test_ai_review_diff_budget.py`; the
staging/ordering contract joins `TestWorkflowDoesNotExecutePRHeadCode`.
Loading
Loading