Skip to content

fix(harness): bounded retry + honest turn end for empty final steps - #324

Merged
itsdestin merged 12 commits into
masterfrom
fix/empty-final-step-recovery
Aug 22, 2026
Merged

fix(harness): bounded retry + honest turn end for empty final steps#324
itsdestin merged 12 commits into
masterfrom
fix/empty-final-step-recovery

Conversation

@itsdestin

Copy link
Copy Markdown
Owner

What

Native turns were silently ending mid-task when the model's final step came back with zero content (no text, no tool calls) under a valid stop finish — experienced as ~3 minutes of nothing after a tool result, then an unexplained stop requiring a manual "continue" (observed 3x live on stealth/ox-alpha, 2026-08-20/21; also reproduced on qwen3.8-max).

Now:

  • An empty step with an orderly finish reason gets one silent re-run (history untouched — the empty step never pushed anything, so the request is repeatable; both attempts bill into turn usage).
  • A second consecutive empty step ends the turn honestly with a new stopReason: 'empty_response' and a visible footer: "The model returned an empty response. Retrying may help."
  • finishReason-gated: 'length'/'content-filter' empties keep today's exact max_tokens/refusal behavior — the retry never masks a real stop reason.
  • Fully-silent turns render the footer too (spec decision 4): the reducer creates the turn on abnormal-stopReason turn-complete, and the bubble renders a footer-only row — previously this worst-case shape had nothing to attach the explanation to.

What it deliberately does NOT touch

Stall clocks, the park guard, mapStopReason, the frozen emit surface (no new event types or IPC — the state rides the existing turn-complete payload), TRANSCRIPT_INTERRUPT, Kotlin (native stopReason is opaque passthrough — verified).

Docs

  • Spec: youcoded-dev/docs/active/specs/2026-08-21-empty-final-step-turn-recovery-design.md (reviewed + ratified; includes prior-art comparison with opencode v1.18.21's sibling unknown-finish fix)
  • Plan: youcoded-dev/docs/active/plans/2026-08-21-empty-final-step-turn-recovery-plan.md (independently reviewed; verdict approve-with-fixes, findings folded in)

Testing

TDD throughout — 15 new tests written failing-first: the retry ladder (cases 1–4), reasoning-only steps classified empty (case 5, new reasoningChunks helper), the finishReason gate (case 6), interrupt-wins (case 7), specialist-child settle (8a/8b), footer copy (case 9), suite-stability (case 10), and the fully-silent-turn reducer/bubble pair (case 11). Downstream eval-runner/specialist suites that staged "model simply stopped" with one empty step doubled it to keep their premise under the new contract. harness-stall-watchdog.test.ts untouched and green; verify.sh green (tsc, vitest, knip, eslint, ast-grep); full desktop suite 5,995 passed.

Known open item: the footer-only row (decision 4) has not been visually inspected — it renders only in the rare all-empty failure state; styling can be tweaked post-merge if needed.

🤖 Generated with Claude Code

itsdestin and others added 12 commits August 21, 2026 14:55
…ec cases 1-4, 6, 7)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ning-only-step test (spec case 5)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ty_response' turn end

A step with no text and no tool calls yet an orderly finishReason used to end
the turn as a silent end_turn — experienced as ~3min of nothing after a tool
result (spec 2026-08-21-empty-final-step-turn-recovery-design.md). Now: one
silent re-run (history untouched, both attempts billed), then an honest
'empty_response' stop. finishReason-gated so 'length'/'content-filter' keep
today's exact behavior. No new events or IPC; one console.error for diagnosis.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ec case 9)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ive empty steps

The empty-step retry changes the harness contract: one scripted {} step no
longer ends a turn (it gets silently re-run and consumes the next script).
Eval-runner and specialist-nudge tests that staged 'model simply stopped'
double their empty step to keep their premise; the specialist test now also
pins the layered recovery (step-level retry first, turn-level nudge second).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…treamed nothing (spec decision 4)

Review-driven addition: content-creating actions are what mint assistant turns,
so a fully-contentless empty_response turn had nothing to attach its honest
footer to — the worst-case shape of the bug stayed silent. Reducer now creates
the turn on abnormal-stopReason turn-complete; the bubble renders a footer-only
row for it. end_turn/interrupt behavior unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…list beside mapStopReason (+tool-calls), structured log

Three review findings on the retry ladder:
- Whitespace-only steps ('\n\n') were pushed to history (truthiness gate) yet
  classified empty (trim gate) — the retry then re-sent a conversation ending
  in a dangling whitespace assistant message. One shared predicate now feeds
  both gates.
- The orderly-finish list lived 1,480 lines from mapStopReason (drift risk)
  and excluded 'tool-calls' — the empty shape a stream takes when every
  announced call is dropped as malformed, likeliest on small local models;
  it previously ended the turn with the raw passthrough reason 'tool-calls'.
  Now ORDERLY_EMPTY_FINISHES, defined next to mapStopReason.
- The retry's only diagnostic was console.error, which reaches nobody in a
  packaged build; now the structured log() that writes ~/.claude/desktop.log.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lh1iH7j6qQDcHRFWMS1dhL
…d; one stamp site; event-time timestamp

Review findings: the mint branch broke TRANSCRIPT_TURN_COMPLETE's absorb
contract — the watcher re-emits turn-complete and re-dock replay re-delivers
it, while content actions ARE uuid-deduped, so every replay arrived with
currentTurnId null and appended a fresh ghost turn + timeline row, unbounded.
Abnormal completions now record their uuid in seenUuids (both the stamp and
the mint path — a live max_tokens turn must not re-mint as a ghost on replay)
and the mint checks it. The duplicated metadata-stamp literal (which had
already diverged: 'model: action.model' vs '?? turn.model') is collapsed to
one stamp site, and a minted turn takes the EVENT's timestamp instead of
Date.now() so a replayed footer row doesn't display the re-dock time.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lh1iH7j6qQDcHRFWMS1dhL
…tView/BubbleFeed gates for abnormal segment-less turns

The PR's user-visible fix never rendered: both timeline render sites drop
segment-less turns before AssistantTurnBubble mounts, and every shipped test
mounted the bubble directly — so all tests passed while the real app still
showed the exact unexplained silence the PR exists to fix. One shared
abnormalStopReason() predicate (exported from AssistantTurnBubble) now
drives the bubble's two footer gates AND both timeline gates; a new test
crosses the ChatView boundary (state in → footer out) so this can't silently
regress. The footer-only row also gains the timestamp trailer the bubble
path already had, and the copy states the verified fact: 'The model returned
an empty response twice. Retrying may help.'

Visible side effect (flagged for Destin): a turn interrupted while still
'preparing' a tool call — segment-less after preparing-card reaping — now
renders its 'Interrupted.' footer instead of vanishing entirely.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lh1iH7j6qQDcHRFWMS1dhL
…egment-less turns exist

The doc enumerated stopReason as a closed set and documented turns as minted
only by content actions — the invariant the empty-step-recovery PR inverts.
Records the new mint rule, its uuid idempotency, and the shared
abnormalStopReason() gate predicate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lh1iH7j6qQDcHRFWMS1dhL
…pty-step retry; one predicate for mint AND render gates

Independent review of the first fix round found:
- The empty 'tool-calls' shape (announced call, dropped as malformed) almost
  always leaves a 'Preparing…' card on screen, and the empty-step retry —
  unlike its manual-Retry and stall-retry siblings — never withdrew it: the
  step re-runs inside the same turn, so endTurn's reaping never fires and the
  orphan spins beside the retry's own cards until turn end. StepResult now
  carries pendingPreparing (started-but-never-completed call ids) out of the
  stream, and the retry withdraws them before re-running. Pinned by test.
- abnormalStopReason was hand-inlined in the reducer (drift risk between the
  mint gate and the render gates). It now lives in chat-types.ts — shared by
  the reducer, the bubble's footer gates, and, via shouldRenderAssistantTurn,
  the ChatView/BubbleFeed timeline gates, which are now mirrored by
  construction instead of by comment.
- The interrupt path's partial-text push was the last emptiness site still on
  truthiness; it now trims like the others (a whitespace-only partial is no
  partial at all).

Known residual (deliberate, documented in the PR): rebuildHistory coalesces a
whitespace-only step's persisted deltas into the retry step's text on resume —
a cosmetic live-vs-rebuilt divergence, smaller than before this branch, left
for a follow-up rather than touching the parity arbiter here.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lh1iH7j6qQDcHRFWMS1dhL
@itsdestin

Copy link
Copy Markdown
Owner Author

Deep review + fixes (2026-08-21, third session)

Two independent adversarial review rounds ran against this branch; every finding was verified against the code before fixing, and each fix round was itself independently re-reviewed before pushing. Five new commits (3270a00a..fe514664):

The big one

The PR's user-visible fix was dead code. Both timeline render sites (ChatView.tsx, buddy BubbleFeed.tsx) drop segment-less turns before AssistantTurnBubble ever mounts — and every shipped test mounted the bubble directly or asserted on reducer state, so all tests passed while the real app still showed the exact unexplained silence this PR exists to fix. Fixed with one shared predicate pair in chat-types.ts (abnormalStopReason / shouldRenderAssistantTurn) used by the reducer's mint gate, the bubble's footer gates, and both timeline gates — pinned by a new test that mounts ChatView itself (state in → footer out).

Also fixed

  • Whitespace-only steps were pushed to history (truthiness gate) yet classified empty (trim gate) — the retry then re-sent a conversation ending in a dangling whitespace assistant message (400 on Anthropic-shaped endpoints). One shared stepHasText predicate now feeds both gates; the interrupt path trims too.
  • Ghost turns on replay: the segment-less mint broke turn-complete's absorb contract — every re-dock replay appended a fresh ghost turn. Abnormal completions now record their uuid in seenUuids; the mint checks it. Minted turns take the event's timestamp; the two metadata-stamp sites merged into one (they had already diverged on model).
  • Empty 'tool-calls' finish (announced call dropped as malformed — the likeliest shape on small local models) now gets the retry instead of surfacing the raw passthrough reason; the orderly-finish list is now ORDERLY_EMPTY_FINISHES, one const next to mapStopReason.
  • Orphaned 'Preparing…' card during the retry: withdrawn explicitly (same rule as the manual/stall retry paths), via a new StepResult.pendingPreparing field. Pinned by test.
  • Diagnosability: console.error → structured log() (~/.claude/desktop.log).
  • Copy: 'The model returned an empty response twice. Retrying may help.' — states the verified fact; the footer-only row also gained the timestamp every other message has.
  • docs/chat-reducer.md no longer contradicts the code (stopReason open set; segment-less turns exist).

Visible side effects to be aware of (deliberate, flagged)

  • Interrupting while a tool call is still 'preparing' now leaves a small 'Interrupted.' footer where the turn previously vanished entirely.
  • A thinking-only CC turn ending max_tokens/pause_turn now renders its footer where there was silence. pause_turn is the one to watch (a standing row while CC continues in a new turn) — easy to narrow later if it annoys.

Known residuals (documented in workspace ROADMAP, deliberately not fixed here)

  • Whitespace-only steps' persisted deltas still rebuild into resumed history (cosmetic; smaller than pre-fix; proper fix touches the history-rebuild parity arbiter).
  • The stamp path (not the mint) is still not uuid-idempotent when a replay lands mid-live-turn — pre-existing.
  • A non-empty step with one completed + one dropped tool call still strands a preparing card until turn end — pre-existing, now has a cheap fix shape via pendingPreparing.

Verification: bash scripts/verify.sh green (tsc, related vitest, knip, eslint, ast-grep); 156 tests across the four touched suites.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Lh1iH7j6qQDcHRFWMS1dhL

@itsdestin
itsdestin merged commit 748480d into master Aug 22, 2026
3 of 4 checks passed
@itsdestin
itsdestin deleted the fix/empty-final-step-recovery branch August 22, 2026 20:51
itsdestin added a commit that referenced this pull request Aug 22, 2026
…ep-recovery references

The workspace archived the spec when PR #324 merged; comment-only change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Lh1iH7j6qQDcHRFWMS1dhL
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