Repository navigation
Fix OCR review routing for large PRs - #2442
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
OpenCodeReview first-pass review
🟠 2 finding(s) (1 medium / 1 low) — see inline comments.
✅ Posted 2 inline comment(s).
OCR pilot metrics & packet coverage
OCR pilot metrics
- Routing: config-docs (router-v10)
- Changed files: 3 supported / 3 total; Lean 0, trust docs 0, workflow/scripts 3, contracts 0, docs 0
- Changed lines: 70 supported; thresholds large Lean >=3 files or >800 lines
- OCR: status success; comments 2; files 3; tokens 286746; tool calls 30; warnings 0; duration 317s
- Largest changed files: .github/scripts/test-ocr-routing.js (+39/-0), .github/scripts/ocr-router.js (+4/-17), .github/ocr/README.md (+10/-0)
Pilot mode: advisory only. Codex Review remains the merge gate.
| ### Large-diff routing (September 2026) | ||
|
|
||
| The router accepts Git output up to 32 MiB (including context) instead of Node's | ||
| 1 MiB default. PR-wide file/line counts no longer disable packet reviews: |
There was a problem hiding this comment.
OpenCodeReview [documentation · low]
The new section accurately describes the router changes (32 MiB maxBuffer at the git() wrapper in ocr-router.js, removal of the packetMaxFiles/packetMaxChangedLines early-skip branch in decideRoute, and the retained ≤8 packet / ≤4 group bounds), and it correctly keeps the partial-coverage and trusted-default-branch caveats. One consistency gap to fix: line 29 of this same file (the large-lean-hotspots routing-mode bullet) still says "If the diff exceeds the bounded packet budget, more than 12 Lean files or more than 2500 supported changed lines ... posts a concrete coverage report and checklist instead of a generic skip." Since this PR removes that file/line-count disable, the earlier bullet is now stale and contradicts the new section — update it so the two don't disagree (the task description even calls out updating stale blocker wording). Note also that packetResidualRisk in ocr-router.js still emits "Diff exceeded packet budget" when counts exceed the old thresholds even though packets were produced, which is consistent with the stale line-29 wording; that is code-side and outside this file, but the README should at least not assert the no-packet behavior anymore.
| warnings: decision.mode === 'large-lean-hotspots' | ||
| ? [{ type: 'routing', message: 'Diff exceeded bounded packet review capability; full OCR not attempted.' }] | ||
| ? [{ type: 'routing', message: 'Large diff receives bounded packet review only; full-diff OCR is not attempted.' }] | ||
| : [], |
There was a problem hiding this comment.
OpenCodeReview [bug · medium]
This warning message is inaccurate for the only branch that can reach it. Line 1434 already routes every large-lean-hotspots decision that has packets to buildPacketizedResult, so this fallback object is only built when decision.packets is empty (no safe packets could be produced). In that case writePacketReviewPlan returns '' and packet_review.enabled is false — no bounded packet review happens at all. Claiming "Large diff receives bounded packet review only" here overstates coverage precisely in the zero-coverage case, which conflicts with the stated goal that partial coverage stays explicit (and with the README's "must not be read as full review coverage" contract). Reword it to describe what actually occurred, e.g. that no safe review packets could be produced and the diff receives no semantic packet review.
Suggested change:
warnings: decision.mode === 'large-lean-hotspots'
? [{ type: 'routing', message: 'No safe review packets could be produced; this diff received no semantic packet review. Use the top-changed-files checklist for manual review.' }]
: [],
1 participant
OCR crashes on PR #2438 because Git's diff exceeds Node's default 1 MiB output buffer. Earlier heads also skipped all semantic review when the PR exceeded 12 Lean files or 2,500 changed lines.
Allow up to 32 MiB of Git output and retain the existing bounded packet selection for large diffs (eight packets, four file groups). Partial coverage stays explicit; trusted tooling still comes from the default branch. Update the stale blocker wording now that the packet runner exists.
Validation: routing suite passes, including a real Git fixture with 33 Lean files and over 1 MiB of diff; Lean LSP setup tests pass. Routing the actual #2438 head now generates a semantic review plan for its 45 Lean files / 16,298 supported changed lines.