fix(streaming): settle recovered final answers over stale no-response sidecars

[gabrielalberton]

Thinking Path

• The accepted live-to-final reply contract says no_response means no usable assistant final content was produced.
• PR fix(#5575): preserve completed assistant replies during settle #5592 fixed the basic stale-soft-partial case, but bug(streaming): completed assistant turn can still settle as no_response #5575 documented a recovery variant where durable state already contains the completed answer while the WebUI sidecar still ends with a matching _partial row and the generated generic No response from provider card.
• On current master, that stale sidecar can be reconstructed as prior model context. An assistant-only recovered result is then treated as replay, the current-user boundary is misplaced, and settlement emits another generic error instead of done.
• The safe fix is not “any assistant text means success.” It is a positive, turn-local proof: soft partial only, no explicit/captured terminal error, exact generated error fingerprint, matching final/partial text, no pending tool activity, and authoritative current-turn ownership.

What Changed

• Added one proof-gated recovery helper in api/streaming.py that recognizes only the generated generic no-response sidecar.
• Retires the stale current-turn sidecar from both the visible transcript and context_messages before the existing _settle_result_messages(...) chokepoint runs.
• Defers canonical user reconstruction to the existing _active_turn_identity owner when available, preventing duplicate user rows.
• Preserves real authentication/provider errors, captured HTTP failures, cancellation, compression recovery, tool-limit outcomes, mismatched partial text, and partials carrying tool_calls or _partial_tool_calls.
• Removes only the .json.bak created by this positively identified intentional shrink, after the repaired live sidecar is durably saved, so generic startup recovery cannot resurrect the stale card.
• Added end-to-end and negative regression coverage for both observed persisted row orders and the adjacent error/tool/recovery states.

Why It Matters

A successful long-running turn can otherwise end with a complete answer followed by a misleading rate-limit-style error. Besides confusing the user, the stale visible sidecar can be fed back into model context and repeat the failure on recovery. This change restores one coherent current-turn transcript and keeps no_response reserved for turns that truly have no usable final answer.

Contract Routing

Task type: narrow runtime bug fix
Touched state layers: visible transcript, context_messages, active-turn ownership, settlement terminal state, shrink backup
Relevant public docs:

• AGENTS.md
• CONTRIBUTING.md
• docs/CONTRACTS.md
• docs/rfcs/webui-run-state-consistency-contract.md
• docs/rfcs/live-to-final-assistant-replies.md

Preserved invariants:

• one visible user owner precedes the recovered final answer;
• transcript and model context converge on the same repaired current turn;
• replay/recovery does not duplicate the final answer or generic terminal card;
• real terminal evidence remains authoritative.

This restores the existing accepted contract; it does not change the contract or wire protocol.

Verification

Regression bite on unmodified master (dd7f6ac3)

The exact end-to-end regression failed before the fix:

FAILED test_recovered_final_answer_retires_matching_generic_no_response_sidecar
assert any(event == "done" for event, _ in events)
1 failed

Patched branch

tests/test_issue5121_provider_auth_terminal_error.py
29 passed

tests/test_cancelled_turn_status.py
+ tests/test_auto_compression_terminal_failure.py
+ tests/test_tool_limit_terminal_state.py
+ tests/test_turn_journal_lifecycle.py
+ tests/test_turn_journal_lifecycle_callsite.py
78 passed

session recovery / backup matrix
47 passed

python -m py_compile
passed

git diff --check
passed

python scripts/ruff_lint.py --diff upstream/master
0 findings on added/modified lines

The full repository suite and GitHub CI are expected to provide the broader Python 3.11–3.13 matrix.

Risks / Follow-ups

• The matcher intentionally fails closed. If the generated no-response copy/fingerprint changes, the stale card remains visible rather than risking removal of a real error.
• Historical cards are not bulk-rewritten; the repair applies when the affected recovery shape settles again.
• Partials with tool activity are deliberately out of scope because tool identity requires its own reconciliation.
• No screenshots are included: this is server-side settlement/persistence behavior, and the observable outcome is covered by the end-to-end stream/session regression.
• No documentation files changed because this brings implementation back into the existing accepted contract.

Release-note wording

A recovered completed answer is no longer followed by a misleading No response from provider card when a stale partial/error sidecar survives recovery. Real provider, authentication, cancellation, tool-limit, compression, and captured HTTP failures remain visible.

Model Used

• gpt-5.6-sol via Hermes Agent / LiteLLM
• Tooling: repository inspection, focused pytest matrices, diff-scoped Ruff, and an independent read-only adversarial subagent review

Refs #5575.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 42c0e81a0e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Verify that the matched tail owns the active-turn token

When a user retries the same prompt after an earlier user -> partial -> no_response turn and the new soft-partial result repeats the same assistant text, _split_stale_no_response_tail() matches the old turn by text alone. Every stream has a nonempty identity token, so this check then treats the unrelated old tail as authoritative and removes it, permanently collapsing two turns into one. Require the matched pending_user itself to carry the active-turn token or another exact ownership proof before retiring the tail.

AGENTS.md reference: AGENTS.md:L85-L91

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Delete only the backup created by this repair

When the repaired transcript was assembled from additional state.db rows but the live JSON already has fewer messages than the repaired result, s.save() performs a growth save and therefore does not create or replace .json.bak. This unconditional unlink then deletes any pre-existing backup from an earlier shrink, potentially destroying the only recoverable copy of older history. Record whether this save actually produced the stale-tail backup, or verify that backup's contents before removing it.

AGENTS.md reference: AGENTS.md:L92-L97

Useful? React with 👍 / 👎.

[nesquena-hermes]

Blocking: content equality is not current-turn ownership

The new recovery path can delete an older completed assistant turn when its text happens to match the current recovered answer.

_retire_recovered_final_answer_sidecar() removes prefix[-1] when _is_matching_completed_assistant() sees the same text plus either _db_persisted or finish_reason == "stop". Neither marker ties that row to the active turn. _db_persisted only means that the message was durably flushed, so normal historical assistant rows carry it too. The active-turn token is checked only as a boolean and is never matched to the candidate being removed.

I reproduced the loss at exact head 42c0e81a0e2b with a reviewer-owned sandboxed regression:

1. Start with an older complete exchange whose persisted assistant answer is Done.
2. Append the current user -> _partial assistant("Done") -> generic no-response error tail.
3. Set the recovered partial result to assistant Done and provide an active-turn token.
4. Call the new retirement helper.

Expected: the older [user, assistant] exchange remains intact while only the stale current-turn tail is retired.

Actual: the returned display contains only the older user row. The older completed assistant was popped. The probe failed 1 failed in 3.79s inside warmup-safe-test.sh after a CLEAN threat scan and Layer-3 sandbox entry.

Required fix

• Do not use answer text plus _db_persisted/finish_reason as authority to delete a pre-tail assistant row.
• Require durable provenance tying the candidate to this exact turn/result, preferably a stable state-db message ID or turn ID. get_state_db_session_messages() already selects id, but its projection currently drops it before returning messages.
• Fail closed when provenance is absent. Preserving a possible duplicate is safer than deleting historical transcript data.
• Add a regression with a complete older [user, assistant] exchange whose assistant has the same text as the recovered current answer and an older identity/timestamp. Assert both older rows survive. Keep a separate positive fixture where the duplicate candidate is provably owned by the current turn.

The contributed test file passes (29 passed), as do targeted recovery/retry neighbors (37 passed, 9 deselected), but none covers this same-text multi-turn collision.