Skip to content

Fix stack 04: consistent finish_reason for structured/empty/truncated results#28

Merged
ebarti merged 0 commit into
review-fixes/03-eventsfrom
review-fixes/04-finish-reason
Jul 2, 2026
Merged

Fix stack 04: consistent finish_reason for structured/empty/truncated results#28
ebarti merged 0 commit into
review-fixes/03-eventsfrom
review-fixes/04-finish-reason

Conversation

@ebarti

@ebarti ebarti commented Jul 1, 2026

Copy link
Copy Markdown
Owner

Stacked review-fix PR 4/N (base: review-fixes/03-events).

The same vendor condition produced opposite verdicts across adapters (REVIEW.md §1.1, §1.2, §1.3). Unified:

  • Unsatisfied output_schemafailed everywhere. Claude previously fell through to finish_reason="done" with parsed_output=None, reporting a schema failure as success. Shared message via _common.structured_output_unsatisfied_error.
  • Empty completion → failed everywhere (no output AND no tool calls AND no structured output), via _common.empty_completion_error. Codex previously failed on any empty output even after real tool work; now only when nothing usable came back. Claude/Antigravity previously succeeded on empty; now fail.
  • Antigravity extracts a finish reason and maps a non-natural stop (MAX_TOKENSmax_tokens, SAFETY/RECITATIONfailed) instead of reporting truncated/blocked responses as success. (ASSUMES VENDOR BEHAVIOR: probes finish_reason/stop_reason on the response and candidates.)
  • Antigravity calls structured_output() only when a schema was requested, instead of unconditionally fabricating parsed_output (or crashing if the SDK errors when unconfigured).

Tests for each behavior across all three adapters. Full suite 158 passed / 12 skipped; ruff + mypy strict clean.


Re-review amendment. The shared structured-output failure message now claims exactly what is checked — JSON-parseability, not schema conformance (schema validation stays with the vendor SDK and the caller).

Also: added a test pinning that a SAFETY (or any unrecognized) Antigravity stop reason surfaces as finish_reason="failed" with the reason in the error, never as silent success — the unknown-reason branch was previously untested.


Round 2 (re-review follow-up)

  • Codex turns ending in the SDK's non-terminal inProgress status — or any future unknown status — now fail closed as finish_reason="failed" with a surfaced error, instead of reading as success with partial output. Tests added for both cases.

@ebarti ebarti force-pushed the review-fixes/03-events branch from 12424d5 to 1168cb4 Compare July 2, 2026 10:00
@ebarti ebarti force-pushed the review-fixes/04-finish-reason branch from f47fda4 to f088d0f Compare July 2, 2026 10:00
@ebarti ebarti merged commit cb1c5be into review-fixes/03-events Jul 2, 2026
@ebarti ebarti force-pushed the review-fixes/03-events branch from 1168cb4 to cb1c5be Compare July 2, 2026 15:13
@ebarti ebarti force-pushed the review-fixes/04-finish-reason branch from b7e38ae to cb1c5be Compare July 2, 2026 15:13
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