diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fcb828b..9e4b74bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,13 @@ through the reattached stream); the replay stays for legacy restores and for a failed or unavailable scene. (#7436, @atchisonbrent) +- **A settled assistant answer is no longer shown twice (#2051).** Two client-side paths could put a + second copy of a finished answer on screen after a turn settled: a stale live-turn node that was + still treated as live, and a settled rebuild branch that appended a turn instead of replacing it. + The same fix stops an interrupted turn's notice from appearing twice, and stops the composer's + model picker from listing one configured custom-provider model twice. The stored transcript was + always correct; only the rendering duplicated it. (#7374, @Thireus) + - **A hidden browser tab stops polling a session that was deleted.** When a tab is in the background, its stream poll kept asking for a session every few seconds after it was deleted or archived away, forever. A `404`/`410` for the polled session now stops that poll; `503` and network diff --git a/docs/rfcs/stable-assistant-turn-anchors.md b/docs/rfcs/stable-assistant-turn-anchors.md index 9c522519..39be9aa4 100644 --- a/docs/rfcs/stable-assistant-turn-anchors.md +++ b/docs/rfcs/stable-assistant-turn-anchors.md @@ -574,6 +574,38 @@ early renderers can avoid chasing `S.messages` during every paint. If settlement later rewrites the transcript message, the anchor must be refreshed from that message instead of allowing the two copies to drift silently. +### Settled ownership of a restored live turn + +A preserved or snapshotted live-turn node is not evidence of a running turn. The +settled assistant message is persisted a few milliseconds before the stream's +terminal event clears the active stream id, so a live node whose stream has +already ended can survive a transcript rebuild or a session switch and be +appended under the settled answer, putting the same answer on screen twice +(#6948 follow-up; upstream symptom report #2051). + +A live-turn node is therefore discarded only where the settled transcript is +proved to own it: the transcript ends with a settled assistant message, no +message still carries a live-projection marker, that settled message's +persisted stream identity equals the id of the stream that built the live node +— or, only where no scene identity was persisted, the markdown the stream +produced equals the persisted message source — and the node carries nothing the +settled rebuild could not have produced, meaning no unpersisted tool card, +reasoning row or transparent-stream row and no second live segment. + +Ownership is a state proof, never a comparison of two rendered DOM trees: the +live body comes from the streaming parser and the settled body from the +markdown renderer, and an answer that reads the same as the previous one must +not delete a genuinely live turn. An unrecognised node shape keeps the turn. +Only the two branches that ADD a turn are gated; a branch that replaces a node +cannot duplicate one. + +A settled activity scene projects one answer as well. A `process_prose` row +whose text duplicates the scene's final answer — the same text, or a prefix of +it within ten percent of its length — is dropped where both settled renderers +take their rows, so the answer is not rebuilt as a second assistant segment +above itself. Live rendering is unchanged: while the turn streams, the inline +live segment is hidden and the prose row is the visible answer. + ## Replay, Reload, And Reconstruction Replay/reload should reconstruct the same Assistant Turn Anchor from durable diff --git a/static/ui.js b/static/ui.js index 675bd66d..610ad14c 100644 --- a/static/ui.js +++ b/static/ui.js @@ -3949,6 +3949,27 @@ function _isEquivalentConfiguredModelEntry(modelId,badge,entries){ // providers (@custom:name:model) without collapsing matching model IDs from // different providers. const rawId=String(modelId||''); + // `/` is another routing spelling of ``, so its badge + // key must not become a second picker row. configured_model_badges holds every + // spelling of a configured model and renderModelDropdown() synthesises a row + // for each key this predicate does not recognise. The provider-qualified + // spelling was missed because _normalizeConfiguredModelKey() strips only one + // leading slash segment (#3360 keeps `vendor_a/x` and `vendor_b/y/x` distinct), + // so `acme/example-model` and `custom/acme/example-model` normalise to + // different keys and the picker lists one model twice. + // Match it the way the `@provider:` rule below does: the badge declares a + // provider, the key starts with that provider's `/` prefix, and an + // existing row from the same provider normalises equal to the remainder. Two + // different models never satisfy the last clause, so this can only drop a + // duplicate of a row the catalog already produced. + const slashPrefix=provider?`${provider}/`:''; + if(slashPrefix&&rawId.toLowerCase().startsWith(slashPrefix)){ + const slashRoutedId=rawId.slice(slashPrefix.length); + if(slashRoutedId&&(entries||[]).some(entry=> + String(entry.providerId||'').toLowerCase()===provider + &&_normalizeConfiguredModelKey(entry.value)===_normalizeConfiguredModelKey(slashRoutedId) + )) return true; + } const prefix=provider?`@${provider}:`:''; if(!prefix||!rawId.toLowerCase().startsWith(prefix)) return false; const routedId=rawId.slice(prefix.length); @@ -9928,6 +9949,99 @@ function _liveAssistantSegmentTextLength(seg){ return String(body.textContent||'').trim().length; } +// ── #6948 follow-up: settled-transcript ownership of a live-turn node ────── +// (duplicate assistant answer; upstream symptom report #2051) +// +// Both live-turn insertion paths — the renderMessages() re-attach and +// restoreLiveTurnHtmlForSession() — have one branch that ADDS a turn the +// settled rebuild did not produce. The assistant row is persisted a few ms +// BEFORE the stream's terminal event clears S.activeStreamId, so a dead live +// node left behind by an INFLIGHT entry that outlived its stream is re-attached +// on top of the settled transcript: the same answer twice, self-sustaining +// across later renders (the node keeps id=liveAssistantTurn and is re-preserved +// by the #6948 guard), healed only by a reload. +// +// Dropping a live node must be an OWNERSHIP proof, never "the rendered text +// looks the same": +// * the live body is built by the streaming smd parser and the settled body +// by renderMd + post-processing, so their textContent differs for anything +// past single-paragraph plain text (lists, fenced code with its language +// label, tables, `_underscore_` emphasis, the injected copy button) — a +// text comparison would miss most real answers; and +// * an identical answer earlier in the history (a repeated "Done.", a +// greeting, a same-prompt resend) would delete a GENUINELY LIVE turn: while +// the current turn is unpersisted, the last settled assistant message IS the +// previous turn's answer. +// So ownership is proved from state: the transcript must END with a settled +// assistant message that THIS stream produced, and the node must carry nothing +// the settled rebuild could not have produced. + +// Stream identity persisted on a settled assistant message. The server stamps +// _anchor_stream_id from the anchor-scene sidecar record (api/routes.py); the +// client stamps _anchor_stream_id and scene.identity.stream_id at stream end +// (_attachProjectedAnchorSceneToLastAssistant, static/messages.js). +function _settledAssistantStreamId(message){ + if(!message) return ''; + const scene=message._anchor_activity_scene||null; + const identity=(scene&&scene.identity)||null; + return String(message._anchor_stream_id||(scene&&scene.stream_id)||(identity&&identity.stream_id)||''); +} +// The live-projection markers the #6948 preserve guard already treats as proof +// of a live owner. A message the client still considers live is never evidence +// that the turn has settled. +function _messageHasLiveAssistantProjection(m){ + return !!(m&&m.role==='assistant'&&(m._live||m._activityBurstId!==undefined||m._liveSegmentSeq!==undefined)); +} +// True when the live turn holds something the settled rebuild cannot have +// produced from S.messages — an unpersisted tool card, reasoning/thinking row, +// transparent-stream row, or a second live segment. Such a node is never +// dropped: #3714's whole premise is that the live DOM can be AHEAD of +// S.messages, and a tail-segment match must not discard the tool card or the +// earlier segment above it. An unknown node shape fails closed (keep the turn). +function _liveTurnCarriesUnsettledContent(turn){ + if(!turn||typeof turn.querySelectorAll!=='function') return true; + if(turn.querySelectorAll( + '.tool-card-row,.wl-reason,.agent-activity-thinking,.thinking-card-row,.transparent-event-row' + ).length) return true; + return turn.querySelectorAll('[data-live-assistant="1"]').length>1; +} +function _settledTranscriptOwnsLiveTurn(sid, turn){ + if(!turn) return false; + const msgs=(typeof S!=='undefined'&&S&&Array.isArray(S.messages))?S.messages:null; + if(!msgs||!msgs.length) return false; + // The transcript must END with a settled assistant answer. A trailing user + // turn means the live turn IS the current turn — nothing of it is persisted + // yet, so it can never be a leftover (this is the case a text comparison gets + // wrong when the previous answer happens to read the same), and a live + // projection anywhere means the rebuild still owns a live turn of its own. + const last=msgs[msgs.length-1]; + if(!last||last.role!=='assistant') return false; + if(msgs.some(_messageHasLiveAssistantProjection)) return false; + const inflight=(typeof INFLIGHT!=='undefined'&&INFLIGHT)?INFLIGHT[sid]:null; + // Owner of the live DOM: INFLIGHT carries the id of the stream that built it + // (attachLiveStream), and S.activeStreamId is that same id until the terminal + // event clears it. + const liveStreamId=String((inflight&&inflight.streamId)||(typeof S!=='undefined'&&S&&S.activeStreamId)||''); + const settledStreamId=_settledAssistantStreamId(last); + if(settledStreamId){ + // Identity recorded on both sides decides — renderer- and text-independent, + // so code blocks, lists and tables are handled like plain prose. + if(!liveStreamId||settledStreamId!==liveStreamId) return false; + }else{ + // No persisted identity on the tail (a turn whose scene was not worklog + // worthy). Compare SOURCE with SOURCE — the markdown this stream produced + // (INFLIGHT.lastAssistantText) against the persisted message content — + // never two independently rendered DOM trees. Regeneration cannot reach + // this branch: startRegeneration() truncates S.messages at the user turn, + // so the tail is a user message while a regenerated answer streams. + const streamed=String((inflight&&inflight.lastAssistantText)||'').replace(/\s+/g,' ').trim(); + if(!streamed) return false; + const settled=String((typeof msgContent==='function'?msgContent(last):last.content)||'').replace(/\s+/g,' ').trim(); + if(!settled||streamed!==settled) return false; + } + return !_liveTurnCarriesUnsettledContent(turn); +} + function _mergeRestoredLiveAssistantSegment(restored, existing){ if(!restored||!existing) return; const existingLive=existing.querySelector('[data-live-assistant="1"]'); @@ -9964,6 +10078,18 @@ function restoreLiveTurnHtmlForSession(sid){ const existing=$('liveAssistantTurn'); _mergeRestoredLiveAssistantSegment(restored, existing); if(existing) existing.replaceWith(restored); + // #6948 follow-up (duplicate assistant answer; #2051): only this branch ADDS a + // turn — replacing an existing live turn cannot duplicate. When the settled + // transcript already owns the stream this snapshot belongs to, appending it + // pins a SECOND copy of the same answer under the settled one. Release the + // stale snapshot (no other consumer can use it — it would be re-refused on + // every later restore) and report "nothing restored", so loadSession takes the + // same path it already takes for an INFLIGHT entry that never carried one. + else if(typeof _settledTranscriptOwnsLiveTurn==='function' + &&_settledTranscriptOwnsLiveTurn(sid, restored)){ + inflight.liveTurnHtml=null; + return false; + } else inner.appendChild(restored); // Transparent Stream: liveTurnHtml is restored via template.innerHTML, which // drops the property-bound onclick/onkeydown handlers wired by @@ -11469,6 +11595,15 @@ function _assistantMessageBelongsInWorklog(m, rawIdx, toolCallAssistantIdxs, vis const isTurnFinalAssistant=!!(opts&&opts.isTurnFinalAssistant); const visibleText=String(visibleContent!==undefined?visibleContent:msgContent(m)||'').trim(); const hasVisibleText=!!visibleText&&!_isAssistantEmptyPlaceholderContent(m, visibleText); + // The caller only consults this predicate once the turn has settled (it gates + // on `!S.busy`), so an `_live` marker seen here is a leftover of the + // live-snapshot projection, not an ongoing stream. Folding on it hid the turn's + // final answer inline and echoed it into the Worklog, leaving the turn with no + // visible content until the #3875 blank-turn fail-safe revealed both copies. A + // turn-final assistant message with visible text is the answer, so keep it out + // of the fold; every other `_live` message folds as before. Kept as a separate + // guard so the live rule below keeps its position. + if(m._live&&hasVisibleText&&isTurnFinalAssistant) return false; if(m._live) return true; if(hasVisibleText&&m._anchor_activity_scene) return false; if(hasVisibleText&&isTurnFinalAssistant) return false; @@ -13151,6 +13286,17 @@ function _anchorSceneRowsForRendering(scene, opts){ const rows=Array.isArray(scene&&scene.activity_rows)?scene.activity_rows:[]; const settled=!!(opts&&opts.settled); const live=!settled; + // #6948 follow-up (duplicate assistant answer; #2051): on a SETTLED turn the + // assistant segment owns the final answer, so a process_prose row carrying + // that same text must not also be rebuilt into the activity scene. The + // transparent-stream renderer suppresses it per row + // (_anchorSceneTransparentNodeForRow), but the compact-worklog row builder + // (_anchorSceneNodeForRow) has no such guard and renders the row as a second + // .assistant-segment above the settled one. Drop it here — the single place + // BOTH settled renderers (and the #5839 deferred-rows path) get their rows. + // LIVE rendering is untouched: while streaming, the inline live segment is + // hidden and the prose row IS the visible answer. + const settledFinalAnswer=settled?String((scene&&scene.final_answer)||'').trim():''; const out=[]; const byKey=new Map(); const liveProseTextKeys=new Map(); @@ -13173,6 +13319,9 @@ function _anchorSceneRowsForRendering(scene, opts){ if(_anchorSceneIsSettledSuccessfulCompression(row,settled)) continue; const text=String(row.text||'').trim(); if((row.role==='prose'||row.role==='thinking')&&!text) continue; + if(settledFinalAnswer&&row.role==='prose' + &&typeof _anchorSceneProseDuplicatesFinalAnswer==='function' + &&_anchorSceneProseDuplicatesFinalAnswer(text,settledFinalAnswer)) continue; // #6948 follow-up const key=keyFor(row); if(byKey.has(key)){ const index=byKey.get(key); @@ -13447,6 +13596,26 @@ function _anchorSceneProseMatchesFinalAnswer(proseText, finalAnswer){ const shorter=Math.min(a.length,b.length), longer=Math.max(a.length,b.length); return shorter>=80 && (shorter/longer)>=0.9; } +// #6948 follow-up (duplicate assistant answer; #2051): the same near-equality +// test as _anchorSceneProseMatchesFinalAnswer, minus its absolute `shorter>=80` +// floor. That floor makes the matcher a no-op for every SHORT final answer +// (greetings, "Done.", one-liners), so a prose row that is the answer minus its +// last streamed token — e.g. "Hey! What can I help you with today" vs +// "...today?" — was rebuilt as a SECOND assistant-segment beside the settled +// one. The >=0.9 length ratio is what actually keeps a distinct short +// intermediate sentence from being swallowed by a long final answer (Codex +// #4568), so it alone is kept. Deliberately a separate helper: the per-row +// render-time matcher above, and the tests pinning its exact predicate, stay +// untouched. +function _anchorSceneProseDuplicatesFinalAnswer(proseText, finalAnswer){ + if(_anchorSceneProseMatchesFinalAnswer(proseText,finalAnswer)) return true; + const norm=(s)=>String(s||'').replace(/\s+/g,' ').trim(); + const a=norm(proseText), b=norm(finalAnswer); + if(!a||!b) return false; + if(!(a.startsWith(b)||b.startsWith(a))) return false; + const shorter=Math.min(a.length,b.length), longer=Math.max(a.length,b.length); + return (shorter/longer)>=0.9; +} function _anchorSceneWorklogGroup(blocks, opts){ if(!blocks) return null; const live=!!(opts&&opts.live); @@ -16922,6 +17091,23 @@ function _processWakeupCardHtml(info, rawText, extras){ return `
${li('chevron-right',12)}${li('terminal',13)}${esc(t('process_wakeup_label'))}${cmdHtml}${chip}${extras.timeHtml||''}
${extras.filesHtml||''}${patternRow}${cmdRow}
${outHtml}
${extras.footHtml||''}
`; } +// #2051: parse into a