From 3993122a4e72f06771d45a69e507c6d20d4d1096 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 28 Oct 2026 23:41:24 +0200 Subject: [PATCH] fix: keep live question placeholders pending client-side --- src/core/native-messages.ts | 14 ++++++- tests/pending-authority.test.ts | 20 +++++++++ tests/pending-question-form.test.tsx | 61 ++++++++++++++++++++++++++++ tests/session-core.test.ts | 22 ++++++++++ 4 files changed, 116 insertions(+), 1 deletion(-) diff --git a/src/core/native-messages.ts b/src/core/native-messages.ts index 5ac8193..5ec3a72 100644 --- a/src/core/native-messages.ts +++ b/src/core/native-messages.ts @@ -682,7 +682,10 @@ export function danglingQuestionOf( * 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); + // turn_end includes the temporary questionResult acknowledgement. The + // view ignores it too: it is NOT the learner's answer. Exclude it only + // from client authority, leaving persisted/server result policy intact. + const question = questionOccurrenceOf(messages.filter(message => !isLiveQuestionPlaceholder(message))); if (!question) return null; let occurrences = 0; for (const message of messages) { @@ -700,6 +703,15 @@ export function pendingQuestionOf(messages: readonly LhMessage[]): PendingQuesti }; } +/** Only the transient c_ result with the questionResult contract qualifies. + * Real answers have details; errors/aborts remain resolving results. A saved + * null result is not reinterpreted by this client-only compatibility seam. */ +function isLiveQuestionPlaceholder(message: LhMessage): boolean { + return message.role === "toolResult" && message.id.startsWith("c_") && + !message.isError && message.details === null && + (QUESTION_TOOLS as readonly string[]).includes(message.toolName); +} + /** 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--) { diff --git a/tests/pending-authority.test.ts b/tests/pending-authority.test.ts index 948b29f..bec5f06 100644 --- a/tests/pending-authority.test.ts +++ b/tests/pending-authority.test.ts @@ -93,6 +93,26 @@ test("unique question keys survive live-to-persisted message stamps without rese assert.notEqual(pending([live])!.messageId, pending([persisted])!.messageId); }); +test("only temporary successful question placeholders are excluded from client pending authority", () => { + const message = assistant("new", "q"); + const placeholder = { ...newAnswerResult("q", "quiz", {} as never), id: "c_placeholder", details: null }; + for (const tool of ["quiz", "probe", "ask_user_question", "explain", "recall"]) { + const own = { ...placeholder, toolName: tool }; + assert.equal(pendingQuestionOf([message, own])?.toolCallId, "q"); + assert.equal(danglingQuestionOf([message, own]), null, "server/persisted policy does not change"); + } + for (const result of [ + { ...placeholder, id: "persisted-null-result" }, + { ...placeholder, details: undefined }, + { ...placeholder, isError: true }, + { ...placeholder, isError: true, content: [{ type: "text" as const, text: "Operation aborted" }] }, + { ...placeholder, toolName: "visual" }, + { ...newAnswerResult("q", "quiz", { kind: "quiz", answers: [], dontKnow: true } as never), id: "c_answer" }, + ]) { + assert.equal(pendingQuestionOf([message, result]), null, "real answers, persisted results and failures still resolve the call"); + } +}); + 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"); diff --git a/tests/pending-question-form.test.tsx b/tests/pending-question-form.test.tsx index 363d6ec..9ddc8bd 100644 --- a/tests/pending-question-form.test.tsx +++ b/tests/pending-question-form.test.tsx @@ -5,6 +5,8 @@ import { test } from "node:test"; import { createRoot } from "react-dom/client"; import { parseHTML } from "linkedom"; import { QuestionDock } from "../components/question-dock.tsx"; +import { SessionModel } from "../components/session-core.ts"; +import { findPendingToolCall } from "../src/core/pending.ts"; import { pendingQuestionOf, type LhMessage } from "../src/core/native-messages.ts"; import type { QuizToolInput } from "../src/core/question-core.ts"; @@ -47,6 +49,65 @@ for (const reusedId of [false, true]) { }); } +test("live question validation → turn_end placeholder → canonical sync keeps the same form and typed reasoning", async () => { + const host = document.createElement("div"); document.body.append(host); + const root = createRoot(host); + const encoder = new TextEncoder(); + let wire!: ReadableStreamDefaultController; + const body = new ReadableStream({ start(controller) { wire = controller; } }); + const model = new SessionModel({ chatId: "fixture", initialMessages: [], fetch: async () => new Response(body) }); + let submitted: { thinking?: string } | undefined; + const render = () => { + const state = model.getState(); + const pending = findPendingToolCall(state.messages, pendingQuestionOf(state.nativeMessages)); + root.render(pending ? {}} onSubmit={answer => { submitted = answer; }} dictationClaim={false} /> : null); + }; + model.onChange = render; + const event = async (value: unknown) => { + await act(async () => { wire.enqueue(encoder.encode(`data: ${JSON.stringify(value)}\n\n`)); }); + }; + let consume: Promise | undefined; + try { + await act(async () => { consume = model.resumeStream(false); }); + const saved = question("saved", "q1", "Streamed question"); + const { id: _savedId, ...message } = saved; + await event({ type: "message_start", message }); + await event({ type: "message_end", message }); + assert.equal(host.querySelector('[data-testid="question-dock"]'), null, "validation still gates the form"); + await event({ type: "tool_execution_end", toolCallId: "q1", toolName: "quiz", isError: false, + result: { content: [{ type: "text", text: "Question posed" }], details: null } }); + const form = host.querySelector('[data-testid="question-dock"]'); + assert.ok(form); + const occurrence = pendingQuestionOf(model.getState().nativeMessages)!; + await act(async () => { + host.querySelector('[role="radio"]')!.click(); + const editor = host.querySelector('[data-testid="thinking-field"] [data-file-path-input]')!; + editor.textContent = "Keep my typed reasoning"; + editor.dispatchEvent(new window.Event("input", { bubbles: true })); + }); + await event({ type: "turn_end", message, toolResults: [{ role: "toolResult", toolCallId: "q1", toolName: "quiz", + content: [{ type: "text", text: "Question posed" }], details: null, isError: false, timestamp: 1 }] }); + assert.ok(host.querySelector('[data-testid="question-dock"]') === form, "placeholder must never unmount the form"); + assert.deepEqual(pendingQuestionOf(model.getState().nativeMessages), occurrence, "exact native occurrence stays pending"); + await act(async () => { model.setMessages([saved]); }); + assert.ok(host.querySelector('[data-testid="question-dock"]') === form, "canonical save must not reset the form either"); + assert.ok(host.querySelector('[role="radio"][aria-checked="true"]')); + assert.equal(host.querySelector('[data-testid="thinking-field"] [data-file-path-input]')!.textContent, "Keep my typed reasoning"); + await act(async () => { host.querySelector('form')!.dispatchEvent(new window.Event("submit", { bubbles: true, cancelable: true })); }); + assert.equal(submitted?.thinking, "Keep my typed reasoning", "the React form state, not only DOM text, survived"); + } finally { + await act(async () => { + wire.enqueue(encoder.encode("data: [DONE]\n\n")); wire.close(); + await consume; + root.unmount(); + }); + model.onChange = undefined; + 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); diff --git a/tests/session-core.test.ts b/tests/session-core.test.ts index efc669c..20209e1 100644 --- a/tests/session-core.test.ts +++ b/tests/session-core.test.ts @@ -1020,6 +1020,28 @@ describe("answer submission recovery", () => { }); } + it("accepts immediate/recovery submissions and dismissals while a live question placeholder is in the tail", async () => { + for (const dismiss of [false, true]) { + let posts = 0; + const m = model({ initialMessages: [question], fetch: async (url) => { + if (String(url) === "/api/chat" || String(url).endsWith("/dismiss")) { + posts++; + return new Response("refused", { status: 500 }); + } + return Response.json({ messages: [question], turnInFlight: false }); + } }); + m.applyWireEvent({ type: "turn_end", message: question, toolResults: [{ role: "toolResult", toolCallId: "q1", toolName: "quiz", + content: [{ type: "text", text: "Question posed" }], details: null, isError: false, timestamp: 1 }] }); + const key = pendingQuestionOf([question])!.key; + if (dismiss) await m.dismissQuestion("q1", answer, key); + else { + await m.submitAnswer("quiz", "q1", answer, key); + await m.submitAnswer("quiz", "q1", answer, key); + } + assert.equal(posts, dismiss ? 1 : 2, "placeholder must not reject the render's question identity"); + } + }); + 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;