diff --git a/components/chat.tsx b/components/chat.tsx index 657676b..82bb4bd 100644 --- a/components/chat.tsx +++ b/components/chat.tsx @@ -72,7 +72,7 @@ import { CoverageRing } from "@/components/extraction-coverage"; import { Button } from "@/components/ui/button"; import { activeProblemOf, - danglingQuestionOf, + pendingQuestionOf, dismissedAnswer, endsInFailedTurn, findPendingToolCall, @@ -416,6 +416,7 @@ export function Chat({ void dismissQuestion( p.toolCallId, reading ? { ...answer, readingThinking: reading } : answer, + p.key, ); if (busyRef.current) abortInFlightTurn(); return; @@ -447,7 +448,7 @@ export function Chat({ // mount-only by design (turnInFlight read at initial value) }, []); - const pending = findPendingToolCall(messages, danglingQuestionOf(nativeMessages)?.toolCallId ?? null); + const pending = findPendingToolCall(messages, pendingQuestionOf(nativeMessages)); // The abort handler reads the pending question from a ref (the window // listener is mount-scoped; the value it needs is render-scoped). const pendingRef = useRef(pending); @@ -536,7 +537,7 @@ export function Chat({ readingThinkingRef.current = ""; setReadingThinking(""); }, []); - const pendingQuestionId = pending?.toolCallId ?? null; + const pendingQuestionId = pending?.key ?? null; useEffect(() => { // Different calls never share a draft; a failed submission of THIS // call restores the reading channel alongside the form's snapshot. @@ -1678,8 +1679,8 @@ export function Chat({ fold) and NEVER overlaps the composer. */} { if (pending) saveQuestionDraft(pending.toolCallId, { form }); }} + initialQuestionDraft={pending ? questionDraft(pending.key)?.form : undefined} + onSaveQuestionDraft={(form) => { if (pending) saveQuestionDraft(pending.key, { form }); }} practice={activeProblem} compactionRunning={compacting} onSubmitThinking={submitThinking} @@ -1749,11 +1750,11 @@ export function Chat({ // ride the same callback (QuestionAnswerData.dismissed). The // reading-thinking draft rides along (owner 2026-10-03) — the // check answer is its only carrier. - saveQuestionDraft(pending.toolCallId, { readingThinking: readingThinkingRef.current }); + saveQuestionDraft(pending.key, { readingThinking: readingThinkingRef.current }); const reading = readingThinkingRef.current.trim() || undefined; // The pending-id effect clears this only when the question // actually leaves. An offline preflight keeps the draft visible. - submitAnswer(pending.tool, pending.toolCallId, reading ? { ...output, readingThinking: reading } : output); + submitAnswer(pending.tool, pending.toolCallId, reading ? { ...output, readingThinking: reading } : output, pending.key); }} /> @@ -1934,7 +1935,7 @@ function DockOverlay({ setupCta, queuedSteers, }: { - pending: ReturnType; + pending: ReturnType; initialQuestionDraft?: QuestionFormDraft; onSaveQuestionDraft: (draft: QuestionFormDraft) => void; /** Compaction running for this chat (round 14) — the docks' submit @@ -2001,7 +2002,7 @@ function DockOverlay({ const [revealed, setRevealed] = useState(false); const revealPending = requiresReveal && !revealed; - const pendingId = pending?.toolCallId ?? null; + const pendingId = pending?.key ?? null; const initialPendingRef = useRef(pendingId); // Teach-question ARRIVAL state, applied during render — BEFORE the @@ -2064,7 +2065,7 @@ function DockOverlay({ const shellRef = useRef(null); const contentRef = useRef(null); const mode = pending - ? `question:${pending.toolCallId}` + ? `question:${pending.key}` : practice ? `practice:${practice.toolCallId}` : "chat"; diff --git a/components/session-core.ts b/components/session-core.ts index f02aef2..e859a4b 100644 --- a/components/session-core.ts +++ b/components/session-core.ts @@ -17,8 +17,7 @@ import type { AgentEvent } from "@earendil-works/pi-agent-core"; import type { ToolResultMessage } from "@earendil-works/pi-ai"; import { needsContinuation, - danglingQuestionOf, - answeredCallIds, + pendingQuestionOf, failedResponseAtEnd, measuredContextTokens, newAnswerResult, @@ -119,12 +118,13 @@ async function* sseEvents(reader: ReadableStreamDefaultReader): Asyn /** The dangling question call's tool name (the optimistic answer result's * toolName — the server stamps the same from its own record scan). */ export function pendingCallTool(messages: readonly LhMessage[], toolCallId: string): string | null { - for (const m of messages) { - if ((m as { role?: string }).role !== "assistant") continue; - const content = (m as { content?: unknown }).content; - if (!Array.isArray(content)) continue; - for (const block of content as Array<{ type?: string; id?: string; name?: string }>) { - if (block.type === "toolCall" && block.id === toolCallId) return block.name ?? null; + // Match projection's newest-occurrence pairing for pre-uniquifier records. + for (let mi = messages.length - 1; mi >= 0; mi--) { + const m = messages[mi]!; + if (m.role !== "assistant") continue; + for (let bi = m.content.length - 1; bi >= 0; bi--) { + const block = m.content[bi]!; + if (block.type === "toolCall" && block.id === toolCallId) return block.name; } } return null; @@ -644,9 +644,15 @@ export class SessionModel { * POST + consume. Ready-gated: one turn per client at a time (the * server's 409 + identity retry covers the wake-race window). */ - async send(input: ChatInput, onAccepted?: () => void): Promise { + async send(input: ChatInput, onAccepted?: () => void, questionKey?: string): Promise { const recovering = this.statusValue === "error"; if (!this.readyRef && !recovering) return false; + const subject = input.type === "answer" ? pendingQuestionOf([...this.base, ...this.tail]) : null; + // A render-scoped answer must not move to another occurrence sharing + // its id. This identity is local only; the server's wire is unchanged. + if (input.type === "answer" && questionKey !== undefined && + (subject?.key !== questionKey || subject.toolCallId !== input.toolCallId)) return false; + const expectedKey = questionKey ?? (input.type === "answer" && subject?.toolCallId === input.toolCallId ? subject.key : undefined); this.consumeAbort?.abort(); const controller = new AbortController(); this.consumeAbort = controller; @@ -668,7 +674,8 @@ export class SessionModel { await this.resumeStream(false); return false; } - if (input.type === "answer" && (answeredCallIds(record.messages).has(input.toolCallId) || danglingQuestionOf(record.messages)?.toolCallId !== input.toolCallId)) { + const current = pendingQuestionOf(record.messages); + if (current?.toolCallId !== input.toolCallId || (expectedKey !== undefined && current.key !== expectedKey)) { // The call already resolved, or no longer owns the dock. Adopt // truth, never POST that answer a second time. this.setStatus("ready"); @@ -700,7 +707,7 @@ export class SessionModel { const tool = pendingCallTool([...this.base, ...this.tail], input.toolCallId) ?? "quiz"; const result = { ...newAnswerResult(input.toolCallId, tool, input.answer), id: localId() }; optimistic.push(result); - this.unconfirmedAnswers.set(result.id, input.toolCallId); + this.unconfirmedAnswers.set(result.id, expectedKey ?? input.toolCallId); } if (optimistic.length > 0) this.mutate(() => { this.tail = [...this.tail, ...optimistic]; @@ -828,9 +835,9 @@ export class SessionModel { /** The answer path (the dock): the answer IS the question call's tool * result (fork 1), riding as its input event. */ - submitAnswer(tool: string, toolCallId: string, output: QuestionAnswerData): Promise { + submitAnswer(tool: string, toolCallId: string, output: QuestionAnswerData, questionKey?: string): Promise { void tool; - return this.send({ type: "answer", toolCallId, answer: output }).then(() => undefined); + return this.send({ type: "answer", toolCallId, answer: output }, undefined, questionKey).then(() => undefined); } /** The dismissal path (owner design 2026-10-03, "A2"): the skipped @@ -839,8 +846,10 @@ export class SessionModel { * user's choice; the next plain send is the wake. On failure the * durable record wins: the error surfaces, the record-changed sync * (or any later refetch) replaces the optimistic append with truth. */ - async dismissQuestion(toolCallId: string, answer: QuestionAnswerData): Promise { + async dismissQuestion(toolCallId: string, answer: QuestionAnswerData, questionKey?: string): Promise { if (!this.readyRef) return; + const subject = pendingQuestionOf([...this.base, ...this.tail]); + if (questionKey !== undefined && (subject?.key !== questionKey || subject.toolCallId !== toolCallId)) return; const tool = pendingCallTool([...this.base, ...this.tail], toolCallId) ?? "quiz"; this.mutate(() => { this.tail = [...this.tail, { ...newAnswerResult(toolCallId, tool, answer), id: localId() }]; @@ -890,19 +899,21 @@ export class SessionModel { } /** Submission snapshots outlive the dock, not the session mount. */ - questionDraft(toolCallId: string): QuestionDraft | undefined { - return this.questionDrafts.get(toolCallId); + questionDraft(questionKey: string): QuestionDraft | undefined { + return this.questionDrafts.get(questionKey); } - saveQuestionDraft(toolCallId: string, draft: Partial): void { - this.questionDrafts.set(toolCallId, { ...this.questionDrafts.get(toolCallId), ...draft }); + saveQuestionDraft(questionKey: string, draft: Partial): void { + this.questionDrafts.set(questionKey, { ...this.questionDrafts.get(questionKey), ...draft }); } private forgetResolvedDrafts(record: readonly LhMessage[]): void { - const answered = answeredCallIds(record); - for (const id of answered) this.questionDrafts.delete(id); - for (const [id, callId] of this.unconfirmedAnswers) { - if (answered.has(callId)) this.unconfirmedAnswers.delete(id); + const pendingKey = pendingQuestionOf(record)?.key; + for (const key of this.questionDrafts.keys()) { + if (key !== pendingKey) this.questionDrafts.delete(key); + } + for (const [id, key] of this.unconfirmedAnswers) { + if (key !== pendingKey) this.unconfirmedAnswers.delete(id); } } diff --git a/components/session.ts b/components/session.ts index 4b1cbb5..294a64b 100644 --- a/components/session.ts +++ b/components/session.ts @@ -106,20 +106,20 @@ export function useChatSession({ chatId, initialMessages, initialError, initiall const continueIfOwed = useCallback((record: readonly LhMessage[]) => void model.continueIfOwed(record), [model]); const steer = useCallback((text: string) => void model.steer(text), [model]); const submitAnswer = useCallback( - (tool: string, toolCallId: string, output: QuestionAnswerData) => - void model.submitAnswer(tool, toolCallId, output), + (tool: string, toolCallId: string, output: QuestionAnswerData, questionKey?: string) => + void model.submitAnswer(tool, toolCallId, output, questionKey), [model], ); const dismissQuestion = useCallback( - (toolCallId: string, answer: QuestionAnswerData) => void model.dismissQuestion(toolCallId, answer), + (toolCallId: string, answer: QuestionAnswerData, questionKey?: string) => void model.dismissQuestion(toolCallId, answer, questionKey), [model], ); const dismissProblem = useCallback( (problemCallId: string) => void model.dismissProblem(problemCallId), [model], ); - const questionDraft = useCallback((toolCallId: string) => model.questionDraft(toolCallId), [model]); - const saveQuestionDraft = useCallback((toolCallId: string, draft: Parameters[1]) => model.saveQuestionDraft(toolCallId, draft), [model]); + const questionDraft = useCallback((questionKey: string) => model.questionDraft(questionKey), [model]); + const saveQuestionDraft = useCallback((questionKey: string, draft: Parameters[1]) => model.saveQuestionDraft(questionKey, draft), [model]); const setMessages = useCallback((record: LhMessage[], turnError?: string) => model.setMessages(record, turnError), [model]); const adoptRecord = useCallback((record: LhMessage[]) => model.adoptRecord(record), [model]); const stop = useCallback(() => model.stop(), [model]); diff --git a/src/core/native-messages.ts b/src/core/native-messages.ts index 520fa86..5ac8193 100644 --- a/src/core/native-messages.ts +++ b/src/core/native-messages.ts @@ -35,7 +35,7 @@ import type { AgentMessage } from "@earendil-works/pi-agent-core"; import type { AssistantMessage, Message, ToolResultMessage, UserMessage } from "@earendil-works/pi-ai"; -import { QUESTION_TOOLS } from "./pending.ts"; +import { QUESTION_TOOLS, type PendingQuestion } from "./pending.ts"; import { CLAUDE_CODE_PROVIDER } from "./contracts/claude-code.ts"; import { CONCLUDE_ACK_TEXT, @@ -673,16 +673,48 @@ export function toLlmMessages(messages: readonly AgentMessage[]): Message[] { export function danglingQuestionOf( messages: readonly LhMessage[], ): { toolCallId: string; tool: string; input: unknown } | null { + const question = questionOccurrenceOf(messages); + return question ? { toolCallId: question.toolCallId, tool: question.tool, input: question.input } : null; +} + +/** The same authority, retaining native boundaries for the client dock. + * Unique call ids are stable even when a live message's local id is replaced + * by its persisted stamp. Only colliding ids need an occurrence-qualified + * draft/reset key; this never changes stored ids or the answer wire. */ +export function pendingQuestionOf(messages: readonly LhMessage[]): PendingQuestion | null { + const question = questionOccurrenceOf(messages); + if (!question) return null; + let occurrences = 0; + for (const message of messages) { + // A streamed c_ assistant can temporarily coexist with its saved + // copy during append-only adoption. New live calls are already + // uniquified; only settled history establishes reuse. + if (message.role !== "assistant" || message.id.startsWith("c_")) continue; + for (const call of message.content) { + if (call.type === "toolCall" && call.id === question.toolCallId) occurrences++; + } + } + return { + ...question, + key: occurrences > 1 ? JSON.stringify([question.messageId, question.contentIndex, question.toolCallId]) : question.toolCallId, + }; +} + +/** Shared native authority: newest assistant only, siblings in call order. */ +function questionOccurrenceOf(messages: readonly LhMessage[]): Omit | null { for (let mi = messages.length - 1; mi >= 0; mi--) { const m = messages[mi]!; if ((m as { role?: string }).role !== "assistant") continue; const assistant = m as AssistantMessage & { id: string }; const answered = resultIdsFrom(messages.slice(mi + 1)); - for (const block of assistant.content) { + for (const [contentIndex, block] of assistant.content.entries()) { if (block.type !== "toolCall") continue; if (!(QUESTION_TOOLS as readonly string[]).includes(block.name)) continue; if (answered.has(block.id)) continue; - return { toolCallId: block.id, tool: block.name, input: block.arguments }; + return { + toolCallId: block.id, tool: block.name, input: block.arguments, + messageId: assistant.id, contentIndex, + }; } return null; // the newest assistant message's questions are all answered } diff --git a/src/core/pending.ts b/src/core/pending.ts index aa9134a..923e1d6 100644 --- a/src/core/pending.ts +++ b/src/core/pending.ts @@ -38,10 +38,18 @@ export interface PendingToolCall { input: unknown; } +/** Client-only identity: projection may group several native assistants. + * Never persisted or sent as part of an answer event. */ +export interface PendingQuestion extends PendingToolCall { + messageId: string; + contentIndex: number; + /** Unique call ids stay stable across live → persisted message stamps. + * Reused ids include their native occurrence instead. */ + key: string; +} + /** Tools that render in the question dock and gate the composer. The - * probe-phase names (probe/recall, owner 2026-10-09) join their teach - * twins — old records keep the teach names; new probe-phase calls carry - * the phase names. */ + * probe-phase names join their teach twins; old records keep teach names. */ export const QUESTION_TOOLS = [ "quiz", "probe", @@ -105,14 +113,20 @@ export function filterEnabledTools(tools: Record, enabled?: Enable return out; } -function findPending(messages: UIMessage[], names: readonly string[], toolCallId?: string): PendingToolCall | null { +function findPending(messages: UIMessage[], names: readonly string[], authority?: string | PendingQuestion): PendingToolCall | null { for (let m = messages.length - 1; m >= 0; m--) { const msg = messages[m]!; for (let p = 0; p < msg.parts.length; p++) { const part = msg.parts[p]!; if (isToolUIPart(part) && part.state === "input-available") { const tool = part.type.slice("tool-".length); - if (names.includes(tool) && (toolCallId === undefined || part.toolCallId === toolCallId)) { + if (!names.includes(tool)) continue; + if (typeof authority === "object") { + if (part.toolCallId !== authority.toolCallId || tool !== authority.tool || + part.sourceMessageId !== authority.messageId || part.sourceContentIndex !== authority.contentIndex) continue; + return authority; + } + if (authority === undefined || part.toolCallId === authority) { return { toolCallId: part.toolCallId as string, tool, input: part.input }; } } @@ -121,12 +135,16 @@ function findPending(messages: UIMessage[], names: readonly string[], toolCallId return null; } -/** Grouped view messages lose native assistant boundaries. When the native - * record supplies an authority, expose only that call after view validation. - * null means no native question is pending; undefined keeps view-only callers. */ -export function findPendingToolCall(messages: UIMessage[], toolCallId?: string | null): PendingToolCall | null { - if (toolCallId === null) return null; - return findPending(messages, QUESTION_TOOLS, toolCallId); +/** Grouped view messages lose native assistant boundaries. Native callers + * supply the exact occurrence, not just an id: its OWN part must have cleared + * validation. Never fall back to an older available question. + * null means no native pending question; undefined keeps view-only callers. + * The string overload is retained for view-only/id-filter consumers. */ +export function findPendingToolCall(messages: UIMessage[], authority: PendingQuestion | null): PendingQuestion | null; +export function findPendingToolCall(messages: UIMessage[], authority?: string | null): PendingToolCall | null; +export function findPendingToolCall(messages: UIMessage[], authority?: string | PendingQuestion | null): PendingToolCall | null { + if (authority === null) return null; + return findPending(messages, QUESTION_TOOLS, authority); } // ─── Turn activity label (2026-09-19, the “Writing a question…” fix) ────── diff --git a/src/core/view.ts b/src/core/view.ts index 166d8d5..74e4c8d 100644 --- a/src/core/view.ts +++ b/src/core/view.ts @@ -52,6 +52,9 @@ export interface LhViewPart extends Record { text?: string; state?: string; toolCallId?: string; + /** Native location, retained across display grouping (client-only). */ + sourceMessageId?: string; + sourceContentIndex?: number; input?: unknown; output?: unknown; rawInput?: unknown; @@ -219,13 +222,17 @@ export function projectView(messages: readonly LhMessage[], live?: LiveTurnState // 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) { + for (const [contentIndex, block] of assistant.content.entries()) { 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") { const part = toolPartOf(block, live, streamingMessage); + if ((QUESTION_TOOLS as readonly string[]).includes(block.name)) { + part.sourceMessageId = m.id; + part.sourceContentIndex = contentIndex; + } if (block.name === SEND_TO_USER_TOOL) { // Failed/aborted generations never execute their calls; // the provider error/abort UI already owns that outcome. diff --git a/tests/pending-authority.test.ts b/tests/pending-authority.test.ts index 633f651..948b29f 100644 --- a/tests/pending-authority.test.ts +++ b/tests/pending-authority.test.ts @@ -1,16 +1,20 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { findPendingToolCall } from "../src/core/pending.ts"; -import { danglingQuestionOf, newAnswerResult, type LhMessage } from "../src/core/native-messages.ts"; +import { danglingQuestionOf, pendingQuestionOf, newAnswerResult, newUserMessage, type LhMessage } from "../src/core/native-messages.ts"; import { inputAppends } from "../src/core/input-events.ts"; import { projectView } from "../src/core/view.ts"; function assistant(id: string, ...calls: string[]): LhMessage { - return { id, role: "assistant", content: calls.map(id => ({ type: "toolCall", id, name: "quiz", arguments: { question: id } })), stopReason: "toolUse", timestamp: 1 } as LhMessage; + return { + id, role: "assistant", content: calls.map(id => ({ type: "toolCall", id, name: "quiz", arguments: { question: id } })), + stopReason: "toolUse", timestamp: 1, api: "openai-completions", provider: "fixture", model: "fixture", + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, totalTokens: 0, cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 } }, + }; } function pending(messages: LhMessage[], view = projectView(messages)) { - return findPendingToolCall(view, danglingQuestionOf(messages)?.toolCallId ?? null); + return findPendingToolCall(view, pendingQuestionOf(messages)); } test("a later assistant question supersedes an unanswered question in the same projected group", () => { @@ -38,3 +42,66 @@ test("the authoritative question still waits for projected input validation", () } assert.equal(pending(messages, view), null); }); + +// Synthetic compatibility cases, not claims about the affected user's history. +test("reused ids select the newest native question's input, not an older available part", () => { + const old = assistant("old", "call_0"); + const newer = assistant("new", "call_0"); + if (newer.role === "assistant") newer.content.unshift({ type: "text", text: "new teaching" }); + if (newer.role === "assistant" && newer.content[1]?.type === "toolCall") newer.content[1].arguments = { question: "NEW" }; + const messages = [old, newer]; + const question = pending(messages)!; + assert.equal(question.messageId, "new"); + assert.equal(question.contentIndex, 1); + assert.deepEqual(question.input, { question: "NEW" }); + assert.notEqual(question.key, pending([old])!.key); + assert.deepEqual(danglingQuestionOf(messages), { toolCallId: "call_0", tool: "quiz", input: { question: "NEW" } }, "server authority/shape unchanged"); + const answered = inputAppends({ messages }, { type: "answer", toolCallId: question.toolCallId, answer: { kind: "quiz", answers: [] } as never }); + const parts = projectView([...messages, ...answered]).flatMap(m => m.parts).filter(p => p.type === "tool-quiz"); + assert.equal(parts[0]!.state, "input-available", "old transcript card is untouched"); + assert.equal(parts[1]!.state, "output-available", "result still pairs newest-first"); + assert.equal(pending([...messages, ...answered]), null); +}); + +for (const state of ["input-streaming", "output-error", "output-available"]) { + test(`an older available duplicate never bypasses the authoritative part's ${state} state`, () => { + const messages = [assistant("old", "call_0"), assistant("new", "call_0")]; + const view = projectView(messages); + const ownPart = view.flatMap(m => m.parts).find(p => p.sourceMessageId === "new" && p.type === "tool-quiz")!; + ownPart.state = state; + assert.equal(pending(messages, view), null); + }); +} + +test("a missing authoritative part cannot fall back to another occurrence", () => { + const messages = [assistant("old", "call_0"), assistant("new", "call_0")]; + assert.equal(pending(messages, projectView(messages.slice(0, 1))), null); +}); + +test("older questions never revive across display groups or after a text-only newest assistant", () => { + const messages = [assistant("old", "old-q"), newUserMessage("another turn"), assistant("new", "new-q")]; + assert.equal(pending(messages)?.toolCallId, "new-q"); + assert.equal(pending([...messages, newAnswerResult("new-q", "quiz", {} as never)]), null); + const textOnly = { ...assistant("text"), content: [{ type: "text", text: "done" }] } as LhMessage; + assert.equal(pending([...messages, textOnly]), null); +}); + +test("unique question keys survive live-to-persisted message stamps without resetting drafts", () => { + const live = assistant("c_live", "unique-q"); + const persisted = { ...live, id: "persisted" }; + assert.equal(pending([live])!.key, pending([persisted])!.key); + assert.notEqual(pending([live])!.messageId, pending([persisted])!.messageId); +}); + +test("native occurrence checks preserve every question tool and its own live validation gate", () => { + for (const tool of ["quiz", "probe", "ask_user_question", "explain", "recall"]) { + const message = assistant("live", "q"); + if (message.role === "assistant" && message.content[0]?.type === "toolCall") message.content[0].name = tool; + const live = { executions: new Map(), inFlight: true, streamingIds: new Set(["live"]) }; + assert.equal(pending([message], projectView([message], live)), null); + live.executions.set("q", { isError: false }); + assert.equal(pending([message], projectView([message], live))?.tool, tool); + live.executions.set("q", { isError: true }); + assert.equal(pending([message], projectView([message], live)), null); + } +}); diff --git a/tests/pending-question-form.test.tsx b/tests/pending-question-form.test.tsx new file mode 100644 index 0000000..363d6ec --- /dev/null +++ b/tests/pending-question-form.test.tsx @@ -0,0 +1,71 @@ +import React, { act } from "react"; +(globalThis as Record).React = React; +import assert from "node:assert/strict"; +import { test } from "node:test"; +import { createRoot } from "react-dom/client"; +import { parseHTML } from "linkedom"; +import { QuestionDock } from "../components/question-dock.tsx"; +import { pendingQuestionOf, type LhMessage } from "../src/core/native-messages.ts"; +import type { QuizToolInput } from "../src/core/question-core.ts"; + +const dom = parseHTML(""); +Object.assign(globalThis, { + window: dom.window, document: dom.document, HTMLElement: dom.window.HTMLElement, + IS_REACT_ACT_ENVIRONMENT: true, +}); + +function question(id: string, callId: string, text: string): LhMessage { + return { id, role: "assistant", content: [{ type: "toolCall", id: callId, name: "quiz", arguments: { + question: text, options: [{ label: "Yes", value: "yes" }, { label: "No", value: "no" }], correctAnswer: "yes", explanation: "Yes is correct.", + } }], stopReason: "toolUse", timestamp: 1 } as unknown as LhMessage; +} + +// The same keyed-content boundary as Chat's DockOverlay. This exercises the +// actual form state under synthetic DOM, not CSS layout or a live provider. +for (const reusedId of [false, true]) { + test(`question form resets on a newer question (${reusedId ? "reused" : "unique"} call id)`, async () => { + const host = document.createElement("div"); document.body.append(host); + const root = createRoot(host); + const render = async (messages: LhMessage[]) => { + const pending = pendingQuestionOf(messages)!; + await act(async () => root.render( {}} onSubmit={() => {}} dictationClaim={false} />)); + }; + try { + const old = question("old", "q1", "OLD question"); + await render([old]); + const oldForm = host.querySelector('[data-testid="question-dock"]'); + await act(async () => { host.querySelector('[role="radio"]')!.click(); }); + assert.ok(host.querySelector('[role="radio"][aria-checked="true"]')); + await render([old, question("new", reusedId ? "q1" : "q2", "NEW question")]); + assert.notEqual(host.querySelector('[data-testid="question-dock"]'), oldForm); + assert.equal(host.querySelector('[role="radio"][aria-checked="true"]'), null); + assert.match(host.textContent!, /NEW question/); + assert.doesNotMatch(host.textContent!, /OLD question/); + } finally { await act(async () => root.unmount()); host.remove(); } + }); +} + +test("saving a unique live question preserves its mounted form and selection", async () => { + const host = document.createElement("div"); document.body.append(host); + const root = createRoot(host); + const live = question("c_live", "unique-q", "Same question"); + const saved = { ...live, id: "saved" }; + const render = async (messages: LhMessage[]) => { + const pending = pendingQuestionOf(messages)!; + await act(async () => root.render( {}} onSubmit={() => {}} dictationClaim={false} />)); + }; + try { + await render([live]); + const form = host.querySelector('[data-testid="question-dock"]'); + await act(async () => { host.querySelector('[role="radio"]')!.click(); }); + // Append-only adoption can briefly retain the live copy beside the saved one. + await render([saved, live]); + await render([saved]); + assert.equal(host.querySelector('[data-testid="question-dock"]'), form); + assert.ok(host.querySelector('[role="radio"][aria-checked="true"]')); + } finally { await act(async () => root.unmount()); host.remove(); } +}); diff --git a/tests/session-core.test.ts b/tests/session-core.test.ts index 0e98e80..efc669c 100644 --- a/tests/session-core.test.ts +++ b/tests/session-core.test.ts @@ -24,7 +24,7 @@ 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, newThinkingMessage, needsContinuation, endsInDismissal } from "../src/core/native-messages.ts"; +import { newUserMessage, newAnswerResult, newThinkingMessage, needsContinuation, endsInDismissal, pendingQuestionOf } from "../src/core/native-messages.ts"; import { endsInFailedTurn, newCompactionMarker, newPlanBlockMessage, projectView } from "../src/core/index.ts"; import { dismissedAnswer } from "../src/core/question-core.ts"; import { buildAnswer, type QuestionData } from "../src/core/question-core.ts"; @@ -1020,6 +1020,86 @@ describe("answer submission recovery", () => { }); } + it("does not mistake an old same-id answer for resolution of the current question during recovery", async () => { + let posts = 0; + const newer = { ...question, id: "new-question" } as LhMessage; + const record = [question, newAnswerResult("q1", "quiz", answer), newer]; + const m = model({ initialMessages: record, fetch: async (url) => { + if (String(url) === "/api/chat") { posts++; return new Response("refused", { status: 500 }); } + return Response.json({ messages: record, turnInFlight: false }); + } }); + const key = pendingQuestionOf(record)!.key; + m.saveQuestionDraft(key, { readingThinking: "NEW reasoning" }); + await m.submitAnswer("quiz", "q1", answer, key); + await m.submitAnswer("quiz", "q1", answer, key); + assert.equal(posts, 2, "an older occurrence's result does not forbid retrying this one"); + assert.equal(m.questionDraft(key)?.readingThinking, "NEW reasoning"); + m.setMessages([...record, newAnswerResult("q1", "quiz", answer)]); + assert.equal(m.questionDraft(key), undefined); + }); + + it("adopts a newer same-id occurrence instead of reposting an older question's answer", async () => { + let posts = 0; + const newer = { ...question, id: "new-question" } as LhMessage; + const m = model({ initialMessages: [question], fetch: async (url) => { + if (String(url) === "/api/chat") { posts++; throw new TypeError("offline"); } + return Response.json({ messages: [question, newer], turnInFlight: false }); + } }); + const key = pendingQuestionOf([question])!.key; + await m.submitAnswer("quiz", "q1", answer, key); + await m.submitAnswer("quiz", "q1", answer, key); + assert.equal(posts, 1); + assert.equal(m.status, "ready"); + assert.equal(pendingQuestionOf(m.getState().nativeMessages)?.messageId, "new-question"); + }); + + it("rejects stale render callbacks locally, including same-id silent dismissals", async () => { + let requests = 0; + const m = model({ initialMessages: [question], fetch: async () => { requests++; return Response.json({ ok: true }); } }); + const oldKey = pendingQuestionOf([question])!.key; + const newer = { ...question, id: "new-question" } as LhMessage; + m.setMessages([question, newer]); + await m.submitAnswer("quiz", "q1", answer, oldKey); + await m.dismissQuestion("q1", answer, oldKey); + assert.equal(requests, 0); + assert.equal(m.getState().nativeMessages.length, 2, "no optimistic answer targets the newer question"); + }); + + it("uses the newest occurrence's tool for optimistic answers and dismissals", async () => { + const older = { ...question, id: "old-ask", content: [{ type: "toolCall", id: "q1", name: "ask_user_question", arguments: {} }] } as LhMessage; + for (const dismiss of [false, true]) { + const m = model({ initialMessages: [older, question], fetch: async () => { throw new TypeError("offline"); } }); + const key = pendingQuestionOf([older, question])!.key; + let optimisticTool: string | undefined; + m.onChange = () => { + const result = m.getState().nativeMessages.find(m => m.role === "toolResult"); + if (result?.role === "toolResult") optimisticTool = result.toolName; + }; + if (dismiss) await m.dismissQuestion("q1", answer, key); + else await m.submitAnswer("quiz", "q1", answer, key); + assert.equal(optimisticTool, "quiz"); + } + }); + + it("does not leak drafts between same-id questions, and preserves unique-id live handover", () => { + const m = model({ initialMessages: [question] }); + const oldKey = pendingQuestionOf([question])!.key; + m.saveQuestionDraft(oldKey, { readingThinking: "old reasoning" }); + const newer = { ...question, id: "new-question" } as LhMessage; + m.setMessages([question, newer]); + const newKey = pendingQuestionOf([question, newer])!.key; + assert.notEqual(oldKey, newKey); + assert.equal(m.questionDraft(newKey), undefined); + assert.equal(m.questionDraft(oldKey), undefined); + + const live = { ...question, id: "c_live" }; + m.setMessages([live]); + const liveKey = pendingQuestionOf([live])!.key; + m.saveQuestionDraft(liveKey, { readingThinking: "live reasoning" }); + m.setMessages([question]); + assert.equal(m.questionDraft(pendingQuestionOf([question])!.key)?.readingThinking, "live reasoning"); + }); + it("adopts a changed pending question instead of sending the stale answer", async () => { let posts = 0; const newer = { ...question, id: "new-question", content: [{ type: "toolCall", id: "q2", name: "quiz", arguments: {} }] } as LhMessage;