review: Keep incomplete execution out of completed review caches - #36
Conversation
|
@codex review |
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 | ✅ PASS |
| Issue design | ✅ PASS |
| Code & plan conformance | ❌ FAIL |
Scope: aadfc5ba82 · cb2d133738..aadfc5ba82 · full · 1 diff chunk
Usage: 1m 10s · 301,036 tokens · 77.9% cache hit · 7.071 credits
Blockers
plan-conformanceIssue #35 requires incomplete execution to report the concrete inability. The implementation accepts an empty incomplete reason, leaving failed runs without that required diagnostic.code-review.github/scripts/pr-review/run.mjs:207: Reject incomplete responses without a reason
Findings
1 inline finding published.
Summary
The incomplete-execution path accepts an empty reason, so an execution failure can be reported without the concrete cause required by the Issue.
Review metadata
- Commit:
aadfc5ba82 - Range:
cb2d133738..aadfc5ba82 - Mode:
full - Diff chunks:
1 - Model:
gpt-5.6-terra - Reasoning effort:
medium - Session:
repo-1309321116-pr-36-v2 - Generation:
d84756da1e2d884878ae41d184772f2dbe8207506c2db4dd0f171f33df389ba1 - Evidence:
f86e23ea9de4dc587c8a21a7541986e23cacba182dd7f171b3e29389d8ea271c
Totals
- Input:
296,962 - Cached input:
231,415 - Cache write:
65,508 - Output:
4,074 - Reasoning:
1,660 - Total:
301,036 - Estimated credits:
7.071 - 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 |
14s | 29,165 | 18,843 | 64.6% | 481 | 29,646 | 0.943 |
issue |
full |
Issue #35 |
17s | 62,340 | 50,566 | 81.1% | 932 | 63,272 | 1.401 |
code |
full |
chunk 1/1 |
33s | 120,559 | 80,755 | 67.0% | 2,249 | 122,808 | 3.836 |
code |
full |
aggregate |
6s | 84,898 | 81,251 | 95.7% | 412 | 85,310 | 0.890 |
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: f102acbd8e · aadfc5ba82..f102acbd8e · incremental · 1 diff chunk
Usage: 15s · 186,037 tokens · 97.7% cache hit · 1.700 credits
Summary
The prior incomplete-reason finding is demonstrably fixed by the incremental change and its regression tests. No actionable findings remain.
Review metadata
- Commit:
f102acbd8e - Range:
aadfc5ba82..f102acbd8e - Mode:
incremental - Diff chunks:
1 - Model:
gpt-5.6-terra - Reasoning effort:
medium - Session:
repo-1309321116-pr-36-v2 - Generation:
35e763ed12522777e678148363c65df14fb4290c8c398ae1f11230a42c1006fa - Evidence:
22adc5135a8dc56072d7a91d98962411b5752b73823f887d2104d9e0a9d14e98
Totals
- Input:
185,238 - Cached input:
180,926 - Cache write:
4,300 - Output:
799 - Reasoning:
192 - Total:
186,037 - Estimated credits:
1.700 - 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 |
reused |
cached evidence |
0s | 0 | 0 | 0.0% | 0 | 0 | 0.000 |
issue |
reused |
Issue #35 |
0s | 0 | 0 | 0.0% | 0 | 0 | 0.000 |
code |
incremental |
chunk 1/1 |
10s | 90,711 | 87,757 | 96.7% | 580 | 91,291 | 0.951 |
code |
incremental |
aggregate |
5s | 94,527 | 93,169 | 98.6% | 219 | 94,746 | 0.749 |
Summary
A reviewer that could not read its supplied inputs returned policy blockers, and the workflow cached them as a completed review. New requests on the same head then reused the failure instead of retrying (observed in GizClaw/gizos#257).
Require an explicit execution status on every model turn, preflight supplied input paths, and instruct the reviewer to attempt read-only access even when the inputs live outside its working directory. Incomplete or invalid execution fails before the turn is checkpointed. Completed reviews with legitimate blockers remain reusable. Ledger schema v4 invalidates legacy evidence and chunks once; the published review payload and permission restrictions remain unchanged.
Fixes #35
Validation
node .github/scripts/pr-review/test.mjs— failures and same-head recovery for PR, Issue, code chunk, and aggregation; missing status; real blocker reuse; legacy cache invalidation.0 !== 1: it incorrectly returns success) and passes with the fix.node .github/scripts/pr-readiness/test.mjsnode .github/scripts/issue-review/test.mjsnode .github/scripts/review-request/test.mjsgit diff --checkThe tests exercise orchestration with a mocked Codex process. They do not establish live model obedience. Pinned consumers need to adopt the upstream revision after merge before the fix takes effect.