Repository navigation
fix: centralize Codex review model and diff limit - #45
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ OpenAI PR Review: FAIL
Conclusion: Not ready. 1 readiness blocker must be resolved before merge.
Review checks
| Check | Result |
|---|---|
| PR format | ❌ FAIL |
| Issue design | ✅ PASS |
| Code & plan conformance | ✅ PASS |
Scope: 5ca7325702 · cb6b80fa75..5ca7325702 · full · 1 diff chunk
Usage: 38s · 129,272 tokens · 72.0% cache hit · 3.611 credits
Blockers
pr-linkagePull request must natively close at least one same-repository Issue.
Summary
All completed code chunks were reviewed with no actionable findings or plan-conformance blockers.
Review metadata
- Commit:
5ca7325702 - Range:
cb6b80fa75..5ca7325702 - Mode:
full - Diff chunks:
1 - Model:
gpt-5.6-terra - Reasoning effort:
medium - Session:
repo-1309321116-pr-45-v2 - Generation:
95b47f5c3f1df6eb41f9737f253d45b1d07c8576c39577ff595e132fbea14b72 - Evidence:
077a29eb1e46fa693e4daf3d06e3599d0323fd9cfe2ea327c747858c86b73236
Totals
- Input:
127,095 - Cached input:
91,538 - Cache write:
35,533 - Output:
2,177 - Reasoning:
862 - Total:
129,272 - Estimated credits:
3.611 - Credit rate per 1M tokens:
62.5input /6.25cached /375output
Token and cache details
| Stage | Mode | Target | Time | Input | Cached | Hit | Output | Total | Credits |
|---|---|---|---|---|---|---|---|---|---|
pr |
deterministic |
format rules |
0s | 0 | 0 | 0.0% | 0 | 0 | 0.000 |
pr |
full |
pr |
12s | 34,239 | 20,109 | 58.7% | 679 | 34,918 | 1.263 |
code |
full |
chunk 1/1 |
18s | 50,919 | 31,649 | 62.2% | 1,072 | 51,991 | 1.804 |
code |
full |
aggregate |
8s | 41,937 | 39,780 | 94.9% | 426 | 42,363 | 0.543 |
There was a problem hiding this comment.
✅ OpenAI PR Review: PASS
Conclusion: Ready from the OpenAI review perspective. PR format, linked Issue design, and code/plan conformance passed with no actionable findings.
Review checks
| Check | Result |
|---|---|
| PR format | ✅ PASS |
| Issue design | ✅ PASS |
| Code & plan conformance | ✅ PASS |
Scope: 5ca7325702 · 5ca7325702..5ca7325702 · incremental · 1 diff chunk
Usage: 26s · 193,701 tokens · 65.9% cache hit · 5.445 credits
Summary
No code changes or unresolved findings; the current implementation remains conformant with Issue #46.
Review metadata
- Commit:
5ca7325702 - Range:
5ca7325702..5ca7325702 - Mode:
incremental - Diff chunks:
1 - Model:
gpt-5.6-terra - Reasoning effort:
medium - Session:
repo-1309321116-pr-45-v2 - Generation:
1eb2f92a52604ab3bf471340fa724984295878e953cff5c80f30c1e91ce42f66 - Evidence:
71ea48011a676d3510570b62b185705df2302d88cb6d3320eaea78bff3756dc3
Totals
- Input:
192,219 - Cached input:
126,648 - Cache write:
65,550 - Output:
1,482 - Reasoning:
263 - Total:
193,701 - Estimated credits:
5.445 - Credit rate per 1M tokens:
62.5input /6.25cached /375output
Token and cache details
| Stage | Mode | Target | Time | Input | Cached | Hit | Output | Total | Credits |
|---|---|---|---|---|---|---|---|---|---|
pr |
deterministic |
format rules |
0s | 0 | 0 | 0.0% | 0 | 0 | 0.000 |
pr |
incremental |
pr |
7s | 44,990 | 21,871 | 48.6% | 350 | 45,340 | 1.713 |
issue |
full |
Issue #46 |
11s | 80,716 | 72,902 | 90.3% | 726 | 81,442 | 1.216 |
code |
reused |
chunk 1 |
0s | 0 | 0 | 0.0% | 0 | 0 | 0.000 |
code |
incremental |
aggregate |
8s | 66,513 | 31,875 | 47.9% | 406 | 66,919 | 2.516 |
There was a problem hiding this comment.
❌ OpenAI PR Review: FAIL
Conclusion: Not ready. 3 readiness blockers must be resolved before merge.
Review checks
| Check | Result |
|---|---|
| PR format | ❌ FAIL |
| Issue design | ❌ FAIL |
| Code & plan conformance | ❌ FAIL |
Scope: dcf9195d34 · 5ca7325702..dcf9195d34 · incremental · 1 diff chunk
Usage: 54s · 422,005 tokens · 86.6% cache hit · 7.023 credits
Blockers
pr-formatThe updated summary says the delivered limit is 100,000,000 bytes, while linked Issue #46 defines a fixed 5,000,000-byte limit. Update the PR body to match the approved scope, or update and re-review the Issue if the policy changed.issue-designThis workflow automatically reviews external-fork PRs and processes oversized diffs in sequential chunks. Raising the fixed cap from 5 MB to 100 MB can permit hundreds of model turns from one untrusted PR, but the Issue does not specify a budget, chunk-count/runtime cap, approval gate, or other mitigation. Define the intended resource-abuse safeguard and acceptance criteria.plan-conformanceThe linked Issue requires a safety and cost-control decision for this increase. This automatic external-fork review path can process hundreds of sequential chunks for a 100 MB untrusted diff, but neither the plan nor this implementation supplies a budget, chunk-count/runtime cap, approval gate, or other mitigation.
Summary
The 100 MB limit is implemented consistently, but the linked Issue remains blocked on the required resource-abuse safeguard.
Review metadata
- Commit:
dcf9195d34 - Range:
5ca7325702..dcf9195d34 - Mode:
incremental - Diff chunks:
1 - Model:
gpt-5.6-terra - Reasoning effort:
medium - Session:
repo-1309321116-pr-45-v2 - Generation:
8c874583de7c3041bdc2c6c019004dae1d90a2cb306b29c2d5e6abda212665ff - Evidence:
bc65f2166217a9f1e3495b67b2cb6c064924a367a39b6d51a97e9e7a02180ad9
Totals
- Input:
418,689 - Cached input:
362,467 - Cache write:
56,192 - Output:
3,316 - Reasoning:
1,332 - Total:
422,005 - Estimated credits:
7.023 - Credit rate per 1M tokens:
62.5input /6.25cached /375output
Token and cache details
| Stage | Mode | Target | Time | Input | Cached | Hit | Output | Total | Credits |
|---|---|---|---|---|---|---|---|---|---|
pr |
deterministic |
format rules |
0s | 0 | 0 | 0.0% | 0 | 0 | 0.000 |
pr |
incremental |
pr |
11s | 108,807 | 85,797 | 78.9% | 690 | 109,497 | 2.233 |
issue |
incremental |
Issue #46 |
17s | 78,583 | 75,352 | 95.9% | 1,021 | 79,604 | 1.056 |
code |
incremental |
chunk 1/1 |
18s | 132,775 | 108,222 | 81.5% | 1,102 | 133,877 | 2.624 |
code |
incremental |
aggregate |
8s | 98,524 | 93,096 | 94.5% | 503 | 99,027 | 1.110 |
There was a problem hiding this comment.
❌ OpenAI PR Review: FAIL
Conclusion: Not ready. 2 readiness blockers and 1 actionable finding must be resolved before merge.
Review checks
| Check | Result |
|---|---|
| PR format | ❌ FAIL |
| Issue design | ✅ PASS |
| Code & plan conformance | ❌ FAIL |
Scope: dcf9195d34 · 5ca7325702..dcf9195d34 · incremental · 1 diff chunk
Usage: 1m 0s · 380,267 tokens · 93.3% cache hit · 5.117 credits
Blockers
pr-formatThe Summary now says the shared reviewer enforces a 100,000,000-byte limit. The linked implementation plan and delivered workflow policy specify 5,000,000 bytes, so the PR body no longer clearly describes the delivered scope.code-review.github/workflows/codex-openai-review.yml:322: Keep the automatic-review diff cap within a bounded cost envelope
Findings
1 inline finding published.
Summary
The fixed 100 MB cap introduces an actionable resource-exhaustion regression.
Review metadata
- Commit:
dcf9195d34 - Range:
5ca7325702..dcf9195d34 - Mode:
incremental - Diff chunks:
1 - Model:
gpt-5.6-terra - Reasoning effort:
medium - Session:
repo-1309321116-pr-45-v2 - Generation:
8c874583de7c3041bdc2c6c019004dae1d90a2cb306b29c2d5e6abda212665ff - Evidence:
bc65f2166217a9f1e3495b67b2cb6c064924a367a39b6d51a97e9e7a02180ad9
Totals
- Input:
376,667 - Cached input:
351,545 - Cache write:
25,095 - Output:
3,600 - Reasoning:
1,794 - Total:
380,267 - Estimated credits:
5.117 - Credit rate per 1M tokens:
62.5input /6.25cached /375output
Token and cache details
| Stage | Mode | Target | Time | Input | Cached | Hit | Output | Total | Credits |
|---|---|---|---|---|---|---|---|---|---|
pr |
deterministic |
format rules |
0s | 0 | 0 | 0.0% | 0 | 0 | 0.000 |
pr |
incremental |
pr |
17s | 71,350 | 66,029 | 92.5% | 1,025 | 72,375 | 1.130 |
issue |
incremental |
Issue #46 |
9s | 78,022 | 74,145 | 95.0% | 520 | 78,542 | 0.901 |
code |
incremental |
chunk 1/1 |
20s | 130,309 | 119,865 | 92.0% | 1,194 | 131,503 | 1.850 |
code |
incremental |
aggregate |
14s | 96,986 | 91,506 | 94.3% | 861 | 97,847 | 1.237 |
| env: | ||
| REVIEW_MODEL: gpt-6-sol | ||
| REVIEW_EFFORT: low | ||
| REVIEW_MAX_DIFF_BYTES: '100000000' |
There was a problem hiding this comment.
This reusable workflow is triggered automatically for external PRs, yet this raises the always-enforced complete-diff limit to 100 MB while the default chunk target remains 180 KB. A contributor can therefore force roughly hundreds of sequential Codex review chunks in one automatic run, creating a practical API-credit/runner-consumption denial of service for every caller. The former caller-specific cap let each repository choose its cost envelope; use a substantially bounded shared limit or retain a trusted caller-configurable cap.
Useful? React with 👍 / 👎.
1 participant
Closes #46
Summary
gpt-6-sol,lowreasoning effort, and a 100,000,000-byte total diff limit regardless of caller-providedmodel,effort, ormax-diff-bytes.The requested "light" setting maps to Codex's supported
lowreasoning effort;lightis not a valid effort value. Existing callers pinned to an older workflow SHA must update that reference before this policy takes effect, then may remove the three legacy inputs.Validation
node .github/scripts/pr-review/test.mjsnode .github/scripts/pr-readiness/test.mjsnode .github/scripts/issue-review/test.mjsactionlint -ignore 'property "workflow_(repository|sha)" is not defined' .github/workflows/*.ymlgit diff --checkUnfiltered
actionlintstill reports only its pre-existing unsupportedjob.workflow_shaandjob.workflow_repositorycontext diagnostics.