From 9ccb33ea3b16eb815822a71273489f6e68927c61 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Mon, 5 Oct 2026 01:02:43 +0300 Subject: [PATCH] =?UTF-8?q?fix:=20the=20practice=20live-view=20law=20?= =?UTF-8?q?=E2=80=94=20live=20is=20positive=20evidence;=20absorb=20custom?= =?UTF-8?q?=20markers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thinking-submission incident (owner report, 2026-10-05): two spinners sandwiching the submitted thinking card, and the card doubling once tokens flowed. Records on disk were clean — both bugs were live-view-only. - view.ts LIVE IS POSITIVE EVIDENCE (owner ruling): a tool call renders live (input-streaming) ONLY when its own message is streaming THIS turn (streamingIds) — session busyness can never resurrect a settled call. The pose turn's problem call kept flipping to a mid-transcript spinner because the fallback keyed on live.inFlight. applyResultTo's placeholder fork now also MARKS the settled ack (input-available) instead of leaving the guessed state standing. Live cards now sit at the bottom by construction, never mid-transcript. - session-core.ts absorbed(): the custom role gets its identity key (customType + details deep-equal) — the optimistic thinking marker is dropped when the write-ahead's server copy lands, so the +400ms adoption window can no longer double the card. Total over roles; types that never appear optimistically never reach the branch. Pinned: settled problem never live mid-turn; this turn's streaming call stays shimmer; dangling-settled is input-available; absorbed by customType+details (different details = different submission); model-level adoption keeps exactly one marker. --- components/session-core.ts | 22 ++++++++ src/core/view.ts | 37 ++++++++++--- tests/session-core.test.ts | 106 ++++++++++++++++++++++++++++++++++++- 3 files changed, 157 insertions(+), 8 deletions(-) diff --git a/components/session-core.ts b/components/session-core.ts index b49b25a..547821b 100644 --- a/components/session-core.ts +++ b/components/session-core.ts @@ -23,6 +23,7 @@ import { newUserMessage, projectView, type ChatInput, + type LhMarkerMessage, type LhMessage, type LhViewMessage, type LiveExecution, @@ -134,9 +135,30 @@ export function absorbed(m: LhMessage, record: readonly LhMessage[]): boolean { (r as ToolResultMessage).toolCallId === toolCallId, ); } + if (role === "custom") { + // Markers (the thinking submission, 2026-10-05 — the doubled-card + // bug: the optimistic practice-thinking card was never absorbed, so + // the adoption window rendered it beside the write-ahead's server + // copy for the whole turn). TOTAL over roles by identity key — a + // marker is itself when its customType AND structured details match; + // types that never appear optimistically simply never reach here. + const marker = m as LhMarkerMessage; + return record.some( + (r) => + (r as { role?: string }).role === "custom" && + (r as LhMarkerMessage).customType === marker.customType && + sameDetails((r as LhMarkerMessage).details, marker.details), + ); + } return false; } +/** Structural deep-equal for marker details (JSON-stable payloads — the + * constructors mint plain data). */ +function sameDetails(a: unknown, b: unknown): boolean { + return JSON.stringify(a) === JSON.stringify(b); +} + export interface SessionState { messages: LhViewMessage[]; nativeMessages: LhMessage[]; diff --git a/src/core/view.ts b/src/core/view.ts index 81bb7ac..4284abf 100644 --- a/src/core/view.ts +++ b/src/core/view.ts @@ -181,13 +181,20 @@ export function projectView(messages: readonly LhMessage[], live?: LiveTurnState const reasoning: LhViewPart[] = []; let textN = 0; let reasoningN = 0; + // LIVE IS POSITIVE EVIDENCE (owner, 2026-10-05): a call renders + // live ONLY when its own message is streaming THIS turn — session + // busyness alone can never resurrect a settled call (the practice + // bug: a thinking submission flipped the POSE turn's problem call + // back to a spinner mid-transcript; live cards belong at the + // bottom, and this gate is what puts them there by construction). + const streamingMessage = live?.streamingIds.has(m.id) === true; for (const block of assistant.content) { if (block.type === "text") { rest.push({ type: "text", id: `${m.id}-t${textN++}`, text: block.text, state: "done" }); } else if (block.type === "thinking") { reasoning.push({ type: "reasoning", id: `${m.id}-r${reasoningN++}`, text: block.thinking, state: "done" }); } else if (block.type === "toolCall") { - rest.push(toolPartOf(block, live)); + rest.push(toolPartOf(block, live, streamingMessage)); } } const parts: LhViewPart[] = [...reasoning, ...rest]; @@ -232,10 +239,15 @@ function isPlaceholderResultTool(name: string): boolean { return (QUESTION_TOOLS as readonly string[]).includes(name) || name === "problem"; } -/** A tool call block → its view part, with the live execution layer. */ +/** A tool call block → its view part, with the live execution layer. + * `streamingMessage` — POSITIVE EVIDENCE (owner, 2026-10-05): this call's + * own message is streaming THIS turn. The input-streaming shimmer keys on + * it, never on session busyness: a call from a settled message can never + * render live, whatever else is running. */ function toolPartOf( block: { id: string; name: string; arguments?: unknown }, live: LiveTurnState | undefined, + streamingMessage: boolean, ): LhViewPart { const part: LhViewPart = { type: `tool-${block.name}`, @@ -262,10 +274,11 @@ function toolPartOf( part.output = exec.output; } } else { - // No execution yet: mid-turn it is input-streaming (the shimmer - // activity); in a settled record a dangling call is the dock's - // pending question (input-available). - part.state = live?.inFlight ? "input-streaming" : "input-available"; + // No execution yet: live ONLY on positive evidence — the call's own + // message is streaming this turn (its arguments may literally be + // mid-write). Anything else is a dangling call from a settled + // message — the dock's pending state, never a shimmer. + part.state = streamingMessage ? "input-streaming" : "input-available"; } return part; } @@ -280,7 +293,17 @@ function applyResultTo(part: LhViewPart | undefined, result: ToolResultMessage): // AnsweredCard a null and crash the render (the live crash, 2026-09-26). const tool = part.type.startsWith("tool-") ? part.type.slice("tool-".length) : part.type; const isQuestion = isPlaceholderResultTool(tool); - if (isQuestion && !result.isError && result.details == null) return; + if (isQuestion && !result.isError && result.details == null) { + // The placeholder fork (fork 2) stands: the clean pose/ask ack carries + // no card data — NEVER output-available. But the call IS settled + // (the practice bug, 2026-10-05: this early-return left the part on + // whatever toolPartOf guessed, which mid-turn resurrected a settled + // problem as a live spinner). input-available is the settled truth: + // the ProblemCard reads the call's input, the dock keys on the + // native fold — neither wants the ack's (null) payload. + part.state = "input-available"; + return; + } applyResult(part, result); } // A result with no visible part (e.g. a call whose assistant message diff --git a/tests/session-core.test.ts b/tests/session-core.test.ts index f4f46a9..04ae9bd 100644 --- a/tests/session-core.test.ts +++ b/tests/session-core.test.ts @@ -24,7 +24,8 @@ import { afterEach, describe, it } from "node:test"; import { absorbed, pendingCallTool, SessionModel } from "../components/session-core.ts"; import type { ChatInput } from "../src/core/input-events.ts"; import type { LhMessage } from "../src/core/native-messages.ts"; -import { newUserMessage, newAnswerResult, needsContinuation, endsInDismissal } from "../src/core/native-messages.ts"; +import { newUserMessage, newAnswerResult, newThinkingMessage, needsContinuation, endsInDismissal } from "../src/core/native-messages.ts"; +import { projectView } from "../src/core/index.ts"; import { dismissedAnswer } from "../src/core/question-core.ts"; import { buildAnswer, type QuestionData } from "../src/core/question-core.ts"; @@ -937,3 +938,106 @@ describe("newAnswerResult two audiences", () => { assert.equal((msg.details as { questionText?: string }).questionText, "Which planet is closest to the Sun?"); }); }); + +// ─── the practice live-view law (owner, 2026-10-05) ───────────────────────── +// +// The two-spinners + doubled-card incident: submitting thinking flipped the +// POSE turn's problem call back to a live spinner mid-transcript (session +// busyness leaked into per-call state), and the optimistic thinking marker +// was never absorbed (role "custom" was missing from absorbed()), so the +// adoption window rendered the submission twice for the whole turn. + +/** The post-settle pose-turn record of a practice session. */ +function poseRecord(): LhMessage[] { + return [ + { id: "m1", role: "user", content: "db practice" }, + { + id: "m3", + role: "assistant", + content: [{ type: "toolCall", id: "call-p1", name: "problem", arguments: { statement: "Q?" } }], + } as unknown as LhMessage, + { + id: "m4", + role: "toolResult", + toolCallId: "call-p1", + toolName: "problem", + content: [{ type: "text", text: "Problem posed — the turn ends here." }], + details: null, + } as unknown as LhMessage, + ] as LhMessage[]; +} + +describe("the live-view law — live is positive evidence, not session busyness", () => { + it("a settled problem call NEVER renders live during another turn (the two-spinner bug)", () => { + // The thinking turn's live state: inFlight, but NOTHING streams that + // touches the pose call, and the executions map (per-turn) is empty. + const live = { executions: new Map(), inFlight: true, streamingIds: new Set() }; + const view = projectView(poseRecord(), live); + const problem = view.flatMap((m) => m.parts).find((p) => p.type === "tool-problem"); + assert.ok(problem, "the problem part projects"); + assert.equal(problem!.state, "input-available", "settled = the ProblemCard, never a shimmer"); + }); + + it("a call in THIS turn's streaming message stays input-streaming (the shimmer's true home)", () => { + const live = { executions: new Map(), inFlight: true, streamingIds: new Set(["c_as1"]) }; + const messages = [ + ...poseRecord(), + { + id: "c_as1", + role: "assistant", + content: [{ type: "toolCall", id: "call-c1", name: "conclude", arguments: { outcome: "solved" } }], + } as unknown as LhMessage, + ]; + const view = projectView(messages, live); + const conclude = view.flatMap((m) => m.parts).find((p) => p.type === "tool-conclude"); + assert.ok(conclude, "the conclude part projects"); + assert.equal(conclude!.state, "input-streaming", "the live card renders at the bottom, by construction"); + }); + + it("a DANGLING call in a settled message is the dock's pending — not a mid-turn shimmer", () => { + const live = { executions: new Map(), inFlight: true, streamingIds: new Set() }; + const messages = [ + { id: "m1", role: "user", content: "go" }, + { + id: "m2", + role: "assistant", + content: [{ type: "toolCall", id: "call-p9", name: "problem", arguments: { statement: "Q?" } }], + } as unknown as LhMessage, + ] as LhMessage[]; + const view = projectView(messages, live); + const part = view.flatMap((m) => m.parts).find((p) => p.type === "tool-problem"); + assert.equal(part!.state, "input-available", "no evidence, no shimmer — even mid-turn"); + }); +}); + +describe("absorbed — the custom role (the doubled thinking card)", () => { + it("absorbs the optimistic thinking marker by customType + details", () => { + const optimistic = { ...newThinkingMessage("call-p1", false, "my trace"), id: "c_opt1" } as LhMessage; + const serverCopy = { ...newThinkingMessage("call-p1", false, "my trace"), id: "srv_1" } as LhMessage; + assert.ok(absorbed(optimistic, [...poseRecord(), serverCopy]), "same submission → the local copy drops"); + // A DIFFERENT submission (commit flag, text) is its own marker. + const other = { ...newThinkingMessage("call-p1", true, "my trace"), id: "srv_2" } as LhMessage; + assert.ok(!absorbed(optimistic, [...poseRecord(), other]), "different details → not the same submission"); + // The c_ gate: server stamps are never absorbed, whatever the role. + assert.ok(!absorbed(serverCopy, [optimistic]), "server ids never drop"); + }); + + it("the adoption window cannot double the card (model-level end-to-end)", () => { + // The exact client sequence of the incident: the optimistic marker in + // the tail (client id), then the +400ms adoption fetch swaps in the + // write-ahead record (server id). ONE card must remain. + const m = model({ chatId: "chat_x", initialMessages: poseRecord() }); + const optimistic = { ...newThinkingMessage("call-p1", false, "my trace"), id: "c_opt1" } as LhMessage; + m.applyWireEvent({ type: "message_start", message: optimistic }); + assert.equal( + m.getState().nativeMessages.filter((x) => (x as { role?: string }).role === "custom").length, + 1, + "the optimistic marker is in the tail", + ); + const serverCopy = { ...newThinkingMessage("call-p1", false, "my trace"), id: "srv_1" } as LhMessage; + m.adoptRecord([...poseRecord(), serverCopy]); + const customs = m.getState().nativeMessages.filter((x) => (x as { role?: string }).role === "custom"); + assert.equal(customs.length, 1, "the server copy replaced the optimistic one — no doubling"); + assert.equal((customs[0] as { id?: string }).id, "srv_1"); + }); +});