Skip to content

fix(opencode): resample turns with an unmapped finish reason - #156

Open
Alezander9 wants to merge 1 commit into
mainfrom
finish-reason-resample
Open

fix(opencode): resample turns with an unmapped finish reason#156
Alezander9 wants to merge 1 commit into
mainfrom
finish-reason-resample

Conversation

@Alezander9

@Alezander9 Alezander9 commented Aug 22, 2026

Copy link
Copy Markdown
Member

Summary

A provider stream that opens, delivers a first chunk, then dies without sending a finish chunk is reported by the AI SDK as finishReason: "other" with null usage. "other" is not in our FinishReason literal set (packages/llm/src/schema/ids.ts:39), so session/llm/ai-sdk.ts:22 maps it to "unknown".

The two loop guards in session/prompt.ts then disagreed:

line excludes result on "unknown"
error check 1295 tool-calls, unknown not finished, so no error recorded
loop-exit check 1113 tool-calls finished, so the loop breaks

The turn exited cleanly mid-task with no error, no timeout and no errored span. The run was indistinguishable from a model that gave up.

This changes the loop-exit check to exclude "unknown" as well, so the turn resamples instead of ending the run, and adds a warning log so the event stays visible when the resample succeeds.

Why "other" means "the stream died"

stepFinishReason is initialised to "other" in the AI SDK (node_modules/ai/dist/index.mjs:7246) and is only overwritten when a real finish chunk arrives. The AI SDK still emits finish-step when the socket closes, so "other" reaches us with the start value intact.

NORMAL STREAM                      DEAD STREAM (this bug)
  t=0s    open                       t=0s     open
  t=1s    first chunk                t=2s     first chunk
  t=2-40s text, tool call            t=2-500s ........ silence ........
  t=40s   finish chunk               t=500s   socket closes, no finish chunk
          -> "tool-calls"                     -> "other" -> "unknown"
          -> real tokens, real cost           -> 0 in / 0 out tokens, cost 0

Impact

Measured on the Odysseys benchmark with gpt-5.6-luna xhigh, bcode v0.1.20:

  • 0.054% of LLM calls (24 / 44,618), which at ~102 calls/task is ~5.3% of tasks.
  • 11/200 on one full run, 12/200 on another, 1/50 on a partial run.
  • 100% of them were the last span in the trace. All were scored as completed attempts.
  • Not time-clustered, so not a provider incident.
  • Tell: final_result on the affected tasks is the agent's opening plan ("I'll work through the requested storefronts in sequence..."), never a completion.

Removing the affected tasks moves that arm from 93.7 rubric / 86.0 perfect to 98.1 / 91.5 — the entire measured gap to the reference harness.

opus-5 appears unaffected on a different provider path (0 / 10,550 spans; expected 5.7 at this rate, P(0)=0.003).

Why this is safe

  1. "unknown" never denotes a normal completion. It is the fallback branch in every mapper: openai-chat.ts:383, openai-responses.ts:528, anthropic-messages.ts:563, gemini.ts:376, bedrock-converse.ts:436. The fix therefore also covers the native runtime, not just the AI SDK bridge.
  2. The repeat request is identical. The dead turn writes only a step-start part. toModelMessagesEffect produces an assistant message with no content blocks, and the AI SDK drops empty blocks (processBlock() returns early on block.length === 0), so no empty assistant message is sent to the provider.
  3. It makes the two guards agree. The error check already held the position that "unknown" is not a completion; this applies the same rule at the second site rather than inventing a new one.

Alternative considered

Routing this through SessionRetry instead (new error class, maxTotalAttempts, backoff). Rejected for now:

  • It needs a new entry in AssistantErrorSchema, which is a public wire contract change and drags regenerated SDK output into the diff.
  • Backoff buys nothing when the drop itself takes 286-701s.
  • SessionRetry resamples the same assistant message, so it would have to wipe already-streamed parts (as the OutputLengthError path does via resetOutputLimit()); loop continuation preserves any partial work instead.
  • Unbounded resampling matches existing behaviour: retryable() already returns no maxTotalAttempts for transient API errors (session/retry.ts:140), and retry.ts:28 documents that as intentional.

Verification

  • bun typecheck in packages/opencode: clean.
  • Pre-push filtered typecheck: 16/16 successful.
  • Added loop resamples when the provider closes a stream without a finish reason to test/session/prompt.test.ts, mirroring the existing loop continues when finish is tool-calls test. It queues a reply with no finish chunk, then a normal reply, and asserts two calls and a "stop" finish.

Note: test/session/prompt.test.ts does not run on macOS — 31/58 tests fail on a clean tree, because the fixture's temp-dir opencode.json is not picked up (project root resolves to /) and model resolution falls through to a live opencode/big-pickle. CI has to validate the new test.


Summary by cubic

Resamples turns that end with an unmapped finish reason ("unknown") and logs a warning. Previously, dead streams mapped to "unknown" broke the loop without recording an error; now the loop continues and retries instead of silently truncating the run.

  • Review notes: updates loop-exit logic in packages/opencode/src/session/prompt.ts to exclude "unknown" (like "tool-calls") and adds a warning log with session/message IDs; adds a test in packages/opencode/test/session/prompt.test.ts for streams that close without a finish chunk; no migration required and normal completions are unchanged.

Written for commit 1fdb393. Summary will update on new commits.

Review in cubic

A provider stream that opens, delivers a first chunk, then dies without
sending a finish chunk is reported by the AI SDK as finishReason "other"
with null usage. "other" is not in the FinishReason literal set, so
session/llm/ai-sdk.ts maps it to "unknown".

The two loop guards then disagreed. The error check at prompt.ts:1295
excludes "unknown", so no error was recorded. The loop-exit check did
not, so the loop broke. The turn exited cleanly mid-task with no error,
no timeout and no errored span, and the run was indistinguishable from a
model that gave up.

Measured on the Odysseys benchmark: 0.054% of LLM calls, ~5.3% of tasks
(11/200 and 12/200 on two full runs), and 100% of them terminal.

"unknown" is the fallback in every mapper (openai-chat, openai-responses,
anthropic-messages, gemini, bedrock-converse) and never denotes a normal
completion, so exclude it from the loop-exit check as well and let the
turn resample. The dead turn contributes no model messages, so the repeat
request is identical to the one that was dropped. A warning log keeps the
event visible when the resample succeeds.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 2 files

Re-trigger cubic

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