Skip to content

fix: recover from malformed AI review findings - #42

Open
SwartzMss wants to merge 19 commits into
mainfrom
fix/ai-review-malformed-json-retry
Open

SwartzMss wants to merge 19 commits into
mainfrom
fix/ai-review-malformed-json-retry

Conversation

@SwartzMss

@SwartzMss SwartzMss commented Jul 23, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • capture and log OpenAI-compatible finish_reason and optional token usage metadata
  • retry a present-but-malformed final findings payload exactly once without replaying the invalid tool call
  • require top-level findings and an explicit supported severity: "error", rejecting structurally or semantically invalid findings instead of producing false clean reviews
  • keep missing-payload and candidate-overflow protocol errors outside malformed recovery
  • parse every bounded submit/content candidate through one pipeline, compare normalized findings without depending on order, and reject conflicting final payloads
  • scan assistant content independently from each {, so malformed braces or quotes cannot hide a later valid JSON payload
  • share one eight-candidate budget across tool calls and content objects, and cap AI HTTP response bodies at 4 MiB
  • preserve JSON-content and diff-only constraints across malformed and timeout recovery
  • send malformed-recovery instructions as trusted system messages
  • preserve compacted tool evidence across repeated timeout finalization
  • keep structured timeout classification so recognized HTTP timeouts enter the existing diff-only fallback

Root cause

The runner retried transport and HTTP failures, but treated an HTTP 200 response containing truncated submit_review_findings arguments as a terminal JSON parse failure. The response DTO also discarded completion metadata, so logs could not distinguish an output-length stop from other provider-side generation failures.

AiFindingsResponse.findings and finding severity were permissive, allowing missing fields or unsupported values to become successful error findings or false clean reviews. Candidate parsing conflated missing payloads with malformed JSON, returned early inside assistant content, compared findings in provider order, and used global brace/string state that let a malformed prefix hide later valid JSON.

Candidate enumeration and HTTP body reads were also unbounded. Repeated timeout finalization could discard compacted evidence, replace the active diff-only constraint, or downgrade malformed recovery to an untrusted user instruction. The last HTTP attempt used narrower duplicated error mapping, turning recognized timeout responses into AiRequestFailed and preventing the existing diff-only timeout fallback.

Impact

Malformed final output gets one bounded recovery attempt, while missing-payload and overflow protocol errors fail immediately without an extra API request. Missing findings, missing/unsupported severity, and semantically invalid non-empty findings can no longer produce false clean reviews.

All submit tool payloads and all bounded JSON objects found in assistant content are independently parsed and validated. Multiple valid candidates are accepted only when their canonical findings agree, regardless of order; conflicting empty/non-empty results return AiResponseParseFailed. A malformed prefix no longer prevents recovery from a later valid object.

Tool and content candidates share an eight-candidate limit, response bodies are limited to 4 MiB, malformed recovery remains a trusted system instruction, and repeated timeouts retain acquired evidence plus active diff-only constraints.

Validation

  • cargo fmt --check
  • git diff --check
  • cargo clippy --all-targets --all-features -- -D warnings
  • cargo test — 198 library tests, 5 binary tests, and 21 end-to-end tests passed

@SwartzMss
SwartzMss marked this pull request as ready for review July 23, 2026 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant