Measured on the harness at @v0 (v0.26.0), zh-cn, during the injected-defect certification run for QuantEcon/project-translation#28: 2 of 83 review runs died with no verdict posted at all.
Both failed identically:
evaluateDiff: retryable error on attempt 1/3: Expected property name or '}' in JSON at position 1 (line 1 column 2) — then attempt 2/3, then evaluateDiff: Failed to parse response after 3 attempts, then Action failed.
Runs: 32337319824 (PR #756), 32337521485 (PR #777).
Two things worth separating.
The retry budget is spent on a retry that cannot work. callWithRetry treats a parse failure as retryable and re-sends the same prompt with the same parameters. Both runs failed three times with a byte-identical error at the same offset, which is the signature of a deterministic response to that input rather than a transient one. Three attempts cost three full evaluateDiff prompts — the expensive half of a review — and reach the same place. A parse failure should either change something on retry (lower temperature is not available, but asking for a repair of the returned text is) or fail fast and stop paying.
The blast radius is the whole review, not the diff half. evaluateDiff runs after evaluateTranslation, so a failure here discards a completed translation evaluation that was already paid for, and the PR ends with no verdict block at all. Downstream that is fail-closed and therefore safe — a consumer with no block must route to editor — but it is silent: the PR simply has no review comment, which is indistinguishable at a glance from a review that has not run yet. Under Stage 5 it is also the one outcome that produces no record of why a PR was not auto-merged.
Rate observed here is 2/83 (2.4%). At the estate's current volume that is roughly one lost review a fortnight; it is much more visible in any batch workload.
Suggested shape, in preference order: (1) make the failed-parse path degrade rather than abort — emit the verdict with diffChecks unresolved and a diff-check finding recording the failure, which gates to editor while preserving the translation evaluation and the block itself; (2) do not burn three identical attempts on a deterministic parse failure.
Found during the M0 reviewer certification (QuantEcon/project-translation#28). Not fixed there deliberately — the reviewer is frozen while it is being measured.
Measured on the harness at
@v0(v0.26.0), zh-cn, during the injected-defect certification run for QuantEcon/project-translation#28: 2 of 83 review runs died with no verdict posted at all.Both failed identically:
evaluateDiff: retryable error on attempt 1/3: Expected property name or '}' in JSON at position 1 (line 1 column 2)— then attempt 2/3, thenevaluateDiff: Failed to parse response after 3 attempts, thenAction failed.Runs: 32337319824 (PR #756), 32337521485 (PR #777).
Two things worth separating.
The retry budget is spent on a retry that cannot work.
callWithRetrytreats a parse failure as retryable and re-sends the same prompt with the same parameters. Both runs failed three times with a byte-identical error at the same offset, which is the signature of a deterministic response to that input rather than a transient one. Three attempts cost three fullevaluateDiffprompts — the expensive half of a review — and reach the same place. A parse failure should either change something on retry (lower temperature is not available, but asking for a repair of the returned text is) or fail fast and stop paying.The blast radius is the whole review, not the diff half.
evaluateDiffruns afterevaluateTranslation, so a failure here discards a completed translation evaluation that was already paid for, and the PR ends with no verdict block at all. Downstream that is fail-closed and therefore safe — a consumer with no block must route toeditor— but it is silent: the PR simply has no review comment, which is indistinguishable at a glance from a review that has not run yet. Under Stage 5 it is also the one outcome that produces no record of why a PR was not auto-merged.Rate observed here is 2/83 (2.4%). At the estate's current volume that is roughly one lost review a fortnight; it is much more visible in any batch workload.
Suggested shape, in preference order: (1) make the failed-parse path degrade rather than abort — emit the verdict with
diffChecksunresolved and adiff-checkfinding recording the failure, which gates toeditorwhile preserving the translation evaluation and the block itself; (2) do not burn three identical attempts on a deterministic parse failure.Found during the M0 reviewer certification (QuantEcon/project-translation#28). Not fixed there deliberately — the reviewer is frozen while it is being measured.