Summary
ai_review_inconclusive and ai_review_provider_unparseable_exhausted errors have been recurring (Sentry: LOOPOVER-1P/2B/29). One observed case shows the model returning a clean, well-formed response with "assessment": "Cannot review — the diff appears out of sync with the PR head.", "blockers": [], "nits": [], "suggestions": [] — the model's own deliberate INCOHERENT_DIFF_ASSESSMENT bail (its prompt explicitly instructs it to emit this exact text rather than rubber-stamp a diff it can't map to the PR). parseModelReview (src/services/ai-review.ts) correctly treats this as unusable (null, same as a genuine parse failure), but the retry loop (runWorkersOpinion) doesn't distinguish the two: it burns the full 3-attempt retry budget per model (6 attempts across primary + fallback) re-asking the same model the same question, before finally giving up.
Area
MCP
Expected behavior
When a model's response is specifically the deliberate INCOHERENT_DIFF_ASSESSMENT bail, the retry loop should stop retrying that model immediately — same as the existing short-circuit for a CLI timeout, a 429, or a structural config error — since the model already gave its deliberate, confident answer about this diff and a same-model retry won't change it.
Actual behavior
The retry loop treats an INCOHERENT_DIFF_ASSESSMENT bail identically to a generic unparseable response and retries the full budget.
Validation
Confirmed by reading parseModelReview's handling (assessment === INCOHERENT_DIFF_ASSESSMENT returning null, same as a genuine parse failure) and the retry loop's fallthrough after a null parse, which has no short-circuit unlike its catch block's existing ones.
Summary
ai_review_inconclusiveandai_review_provider_unparseable_exhaustederrors have been recurring (Sentry: LOOPOVER-1P/2B/29). One observed case shows the model returning a clean, well-formed response with"assessment": "Cannot review — the diff appears out of sync with the PR head.", "blockers": [], "nits": [], "suggestions": []— the model's own deliberateINCOHERENT_DIFF_ASSESSMENTbail (its prompt explicitly instructs it to emit this exact text rather than rubber-stamp a diff it can't map to the PR).parseModelReview(src/services/ai-review.ts) correctly treats this as unusable (null, same as a genuine parse failure), but the retry loop (runWorkersOpinion) doesn't distinguish the two: it burns the full 3-attempt retry budget per model (6 attempts across primary + fallback) re-asking the same model the same question, before finally giving up.Area
MCP
Expected behavior
When a model's response is specifically the deliberate
INCOHERENT_DIFF_ASSESSMENTbail, the retry loop should stop retrying that model immediately — same as the existing short-circuit for a CLI timeout, a 429, or a structural config error — since the model already gave its deliberate, confident answer about this diff and a same-model retry won't change it.Actual behavior
The retry loop treats an
INCOHERENT_DIFF_ASSESSMENTbail identically to a generic unparseable response and retries the full budget.Validation
Confirmed by reading
parseModelReview's handling (assessment === INCOHERENT_DIFF_ASSESSMENTreturningnull, same as a genuine parse failure) and the retry loop's fallthrough after anullparse, which has no short-circuit unlike itscatchblock's existing ones.