Repository navigation
Conversation
The compiled review prompt of PR #861 reached 1,719,252 characters against codex's hard 1,048,576-character cap (input_too_large), so the AI review never ran. The workflow capped tutorial notebook prose but put no budget on the unified diff itself (1.63M here after the notebook exclusion). - tools/ai_review_diff_budget.py: patch -> patch filter. In budget, the output is byte-identical; otherwise 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 - each carrying the `git diff` command the agentic reviewer can run for that file. 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 size. On the PR #861 head this compiles to 981,368 bytes with 9 test-file diffs stubbed and every source, REGISTRY and ledger hunk kept. - tests/test_ai_review_diff_budget.py (36 tests: splitting, tiering, greedy fit, CLI incl. hex-SHA validation, workflow wiring); TestWorkflowDoesNotExecutePRHeadCode allowlists the staged filter. - TODO row: the local codex review has the same gap (hand-trimmed pathspecs today); changelog fragment under Internal.
Overall assessment: ✅ Looks goodNo unmitigated P0 or P1 findings. Executive summary
MethodologyNo findings. Checked Code Quality
PerformanceNo material findings for the intended oversized-diff workload. MaintainabilityNo findings. Base-commit staging, deterministic ordering, and byte-identical passthrough are clear and covered by checks. Tech Debt
SecurityNo findings. The workflow executes the filter staged from Documentation/TestsNo additional findings. The required |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
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).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-0diff --githeader, 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-pathspecgit diffcommand 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 fromBASE_SHAlike the notebook extractor (untrimmed one-shot fallback when absent on base); append the unified diff LAST so its budget isPROMPT_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/test_ai_review_diff_budget.py(splitting, path parsing, tiering, greedy fit, bounded fallbacks, CLI incl. hex-SHA validation, workflow wiring);TestWorkflowDoesNotExecutePRHeadCodeallowlists the staged filter.Note for #861: the bootstrap fallback means #861's own
pull_requestruns still use its head YAML. After this merges, a rebase of #861 (or an/ai-reviewcomment, which runs the default-branch YAML) picks up the budgeted prompt.Methodology references (required if estimator / math changes)
Validation
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.pyall pass (491 passed).Security / privacy
Changelog
changelog.d/20261019-ai-review-diff-budget.md, Internal)