Repository navigation
fix(think): extend the interrupted assistant message on recovery continuations (#1876) - #2329
Conversation
🦋 Changeset detectedLatest commit: f9077fd The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
⚪ agents import sizesMeasured 343 runtime imports as minified bundles. The primary size is gzip; raw minified size is included for diagnosis. An existing import growing by more than 10% is marked red. This report is informational.
Compared No import sizes changed. All 343 current runtime imports
Reported by agent-think[bot]. |
agents
@cloudflare/ai-chat
@cloudflare/codemode
hono-agents
@cloudflare/shell
@cloudflare/think
@cloudflare/voice
@cloudflare/worker-bundler
commit: |
169134d to
5d34717
Compare
5d34717 to
c29ac32
Compare
| const leaf = this.messages.at(-1); | ||
| const continuationAssistant = | ||
| continuation && options?.extendLeafAssistant && leaf?.role === "assistant" | ||
| ? leaf | ||
| : undefined; | ||
| const leafMetadata = continuationAssistant?.metadata; | ||
| const accumulator = new StreamAccumulator({ | ||
| messageId: crypto.randomUUID() | ||
| messageId: continuationAssistant?.id ?? crypto.randomUUID(), | ||
| continuation: continuationAssistant !== undefined, | ||
| existingParts: continuationAssistant?.parts.map(settleInterruptedPart), | ||
| existingMetadata: | ||
| leafMetadata !== null && typeof leafMetadata === "object" | ||
| ? (leafMetadata as Record<string, unknown>) | ||
| : undefined |
There was a problem hiding this comment.
🟡 Failed hydration splits recovered answers
After hydration fails, continuationAssistant misses the durable leaf because startup hydration empties this.messages. Recovery then allocates a new ID and persists a second assistant message.
Learn more
Recovery starts after a Durable Object wake, when transcript hydration can fail because storage cannot materialize even the bounded window. Startup then intentionally leaves this.messages empty while preserving the durable session. continueLastTurn() still proceeds because it validates the leaf through session.getLatestLeaf(), but _streamResult() cannot find that leaf in the cache. It creates a random ID, so the upsert appends instead of updating the interrupted assistant.
Example: A deploy interrupts assistant a-1. On restart, transcript hydration hits SQLITE_NOMEM, but session.getLatestLeaf() still returns a-1. Recovery generates the continuation, seeds no existing parts, and stores a new assistant ID beside a-1.
Recommended fix: Resolve the assistant being extended from durable session state, or carry the validated lastLeaf from continueLastTurn() into _streamResult(). Keep the target-ID validation so recovery cannot extend a different assistant if the conversation changes.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
I looked into this and it cannot split the answer. With an empty in-memory view, _assembleModelMessages returns nothing and _prepareInferenceInvocation throws "No messages to send to the model" before _streamResult runs, so no new assistant id is ever allocated. A windowed (budgeted) hydration still holds the latest leaf. I confirmed this with a harness that empties the cache before the scheduled continuation: nothing streams and the durable transcript stays [user, assistant]. Continuing a turn while hydration is broken is a separate degraded-mode question, so I left this PR unchanged.
c29ac32 to
732ac00
Compare
…inuations (#1876) `_streamResult` always created its `StreamAccumulator` with a fresh id, so a recovery continuation (eviction, deploy, or stream stall) persisted a second assistant message instead of finishing the interrupted one. Recovery continuations now seed the accumulator from the assistant leaf (id, parts, metadata), so the answer stays a single message. Direct `continueLastTurn()` calls and tool auto-continuations keep their documented behavior of writing a separate assistant message. Fixes #1876 Co-authored-by: Sunil Pai <18808+threepointone@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
A recovery continuation opens fresh text and reasoning parts, and an end chunk closes only the newest part of its type, so the part the interruption cut off stayed in state streaming after the recovered answer finished. Settle those parts when seeding the accumulator. Co-authored-by: Cursor <cursoragent@cursor.com>
732ac00 to
f9077fd
Compare
Problem
_streamResultalways created itsStreamAccumulatorwith a fresh id. So a recovery continuation after an eviction, deploy, or stream stall persisted a second assistant message instead of finishing the interrupted one. The UI showed two half-answers.Fix
Recovery continuations (
trigger: "recovery-continue") seed the accumulator from the assistant leaf: its id, parts, and metadata. The answer stays a single message.A first version did this for every continuation. That broke the documented
continueLastTurn()contract and the approval and tool auto-continuation flows (#1627), which intentionally write a separate assistant message. The change is therefore scoped to recovery only, and those behaviours are unchanged.docs/think/sub-agents.md,docs/think/index.md, and thecontinueLastTurn()JSDoc describe both behaviours.Validation:
run-turn-recovery.test.ts, plus the Think schedules _chatRecoveryContinue for pre-first-chunk stalls, causing recovery to skip silently #1941 stall tests from fix(think): retry pre-first-chunk stalls and call onChatRecovery for stalls (#1941, #2042) #2328, now assert a single assistant message.runTurncontinuation, andcontinueLastTurn()tests still pass unchanged.pnpm run checkpassed.Fixes #1876
Authored by @advaitpaliwal, rebased and scoped to recovery continuations.
Stacked PR 8 of 10. Base: #2328.