fix: align pending questions by native occurrence
This commit is contained in:
amosblomqvist
@@ -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);
|
||||
}
|
||||
});
|
||||
|
||||
@@ -0,0 +1,71 @@
|
||||
import React, { act } from "react";
|
||||
(globalThis as Record<string, unknown>).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("<html><body></body></html>");
|
||||
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(<QuestionDock key={`question:${pending.key}`}
|
||||
toolName="quiz" toolCallId={pending.toolCallId} input={pending.input as QuizToolInput}
|
||||
expanded onExpand={() => {}} 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<HTMLButtonElement>('[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(<QuestionDock key={`question:${pending.key}`}
|
||||
toolName="quiz" toolCallId={pending.toolCallId} input={pending.input as QuizToolInput}
|
||||
expanded onExpand={() => {}} onSubmit={() => {}} dictationClaim={false} />));
|
||||
};
|
||||
try {
|
||||
await render([live]);
|
||||
const form = host.querySelector('[data-testid="question-dock"]');
|
||||
await act(async () => { host.querySelector<HTMLButtonElement>('[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(); }
|
||||
});
|
||||
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user