fix: multi-select Other coexistence + the malformed-assessment probe-done crash
Two live bugs from the modal-realism stress runs: 1. Multi-select ask: picking the custom 'Other…' row cleared every already-picked option, and picking any option cleared a typed custom answer — single-select displacement logic applied to both modes. The dock's selection is now a pure state machine in question-core (toggleOption/toggleOther over DockSelection, unit-tested): single displaces (unchanged), multi coexists — neither channel clears the other; the typed text always survives. 2. ProbeDoneCard crashed on a REJECTED probe_done call: the model stringified 'assessment' (the same double-encoding habit as the options bug), zod rejected the call, the model retried — but the card's input-preview fallback trusted the errored part's unvalidated input shape and died on assessment.strands.map. The preview now reads input only while the call is pending or streaming (guarded with Array.isArray); an output-error part renders a quiet malformed-and-retried note instead of the garbage. Pinned: question-core.test.ts (3 dock-selection machine cases), render-cards.test.tsx (4 probe-done card cases incl. the exact live stringified shape, plus the multi-select ask markup pin). Gates: tsc, lint, build, 315/315.
This commit is contained in:
@@ -12,13 +12,27 @@ import type { ProbeDoneResult, ProbeDoneInput } from "@/src/core";
|
||||
* the prober retried on (rendered quiet and small — it is model-facing
|
||||
* bookkeeping, kept only for transcript honesty).
|
||||
*
|
||||
* The input preview is only trustworthy while the call is pending — an
|
||||
* input-available input cleared schema validation at the boundary, and a
|
||||
* mid-stream partial is guarded below. An output-error part's input was
|
||||
* REJECTED by validation and can be any shape (live crash: the model
|
||||
* stringified `assessment`); never render it.
|
||||
*
|
||||
* Deliberately plain v1: the proper assessment viewer design is deferred to
|
||||
* the UI wave; this card just guarantees the data is never invisible.
|
||||
*/
|
||||
export function ProbeDoneCard({ part }: { part: { input?: unknown; output?: unknown } }) {
|
||||
export function ProbeDoneCard({ part }: { part: { input?: unknown; output?: unknown; state?: unknown } }) {
|
||||
const input = part.input as ProbeDoneInput | undefined;
|
||||
const output = part.output as ProbeDoneResult | undefined;
|
||||
const assessment = output?.ok ? output.assessment : (input?.assessment ?? null);
|
||||
const state = typeof part.state === "string" ? part.state : "";
|
||||
|
||||
const assessment =
|
||||
output?.ok
|
||||
? output.assessment
|
||||
: state === "output-error"
|
||||
? null // the rejected call — its input is unvalidated garbage
|
||||
: (input?.assessment ?? null);
|
||||
const strands = assessment && Array.isArray(assessment.strands) ? assessment.strands : null;
|
||||
|
||||
return (
|
||||
<div data-testid="probe-done-card" className="my-3 rounded-xl border bg-card/60 p-3">
|
||||
@@ -28,15 +42,19 @@ export function ProbeDoneCard({ part }: { part: { input?: unknown; output?: unkn
|
||||
</div>
|
||||
{output && !output.ok ? (
|
||||
<p className="text-sm text-muted-foreground">The prober retried: {output.error}</p>
|
||||
) : assessment ? (
|
||||
) : state === "output-error" ? (
|
||||
<p className="text-sm text-muted-foreground">
|
||||
The assessment was malformed — the prober retried with a corrected one.
|
||||
</p>
|
||||
) : strands ? (
|
||||
<div className="space-y-2">
|
||||
{assessment.strands.map((s, i) => (
|
||||
{strands.map((s, i) => (
|
||||
<div key={i} className="rounded-lg bg-background/60 px-3 py-2">
|
||||
<div className="text-sm font-medium">{s.strand}</div>
|
||||
<div className="mt-0.5 text-sm text-muted-foreground">{s.assessment}</div>
|
||||
</div>
|
||||
))}
|
||||
{assessment.summary && (
|
||||
{assessment?.summary && (
|
||||
<p className="px-1 text-sm text-muted-foreground">{assessment.summary}</p>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -8,16 +8,21 @@ import { cn } from "@/lib/utils";
|
||||
import {
|
||||
buildAnswer,
|
||||
coerceCorrectAnswer,
|
||||
emptyDockSelection,
|
||||
normalizeOptions,
|
||||
resolveCorrect,
|
||||
seededRandom,
|
||||
shuffleOptions,
|
||||
toggleOption,
|
||||
toggleOther,
|
||||
type AnswerRequest,
|
||||
type AskToolInput,
|
||||
type DockSelection,
|
||||
type QuestionAnswerData,
|
||||
type QuestionData,
|
||||
type QuestionMode,
|
||||
type QuizToolInput,
|
||||
type SelectMode,
|
||||
type Phase3,
|
||||
} from "@/src/core";
|
||||
|
||||
@@ -211,42 +216,30 @@ function QuestionDockForm({
|
||||
return q;
|
||||
}, [toolCallId, kind, input]);
|
||||
|
||||
const [selected, setSelected] = useState<Set<number>>(new Set()); // 0-based indices
|
||||
const [dontKnow, setDontKnow] = useState(false);
|
||||
// "Other" is selected by typing into its inline input (or clicking its
|
||||
// circle). Picking a sibling option deselects it, but the typed text
|
||||
// survives — the learner's words are never thrown away.
|
||||
const [otherSelected, setOtherSelected] = useState(false);
|
||||
// The selection state machine lives in question-core (pure, tested):
|
||||
// single-select displaces, multi-select coexists — picking the ask
|
||||
// "Other…" row in a multi-select must never clear already-picked
|
||||
// options, and picking an option must never clear a typed custom answer.
|
||||
const [sel, setSel] = useState<DockSelection>(emptyDockSelection());
|
||||
// The learner's transfer channel — thinking, never "a note".
|
||||
const [thinking, setThinking] = useState("");
|
||||
const [other, setOther] = useState("");
|
||||
const [text, setText] = useState("");
|
||||
const selectMode: SelectMode = qd.mode === "multi-select" ? "multi-select" : "single-select";
|
||||
|
||||
const toggle = (idx: number) => {
|
||||
setDontKnow(false);
|
||||
setOtherSelected(false);
|
||||
setSelected((prev) => {
|
||||
if (qd.mode === "multi-select") {
|
||||
const next = new Set(prev);
|
||||
if (next.has(idx)) next.delete(idx);
|
||||
else next.add(idx);
|
||||
return next;
|
||||
}
|
||||
return new Set([idx]);
|
||||
});
|
||||
};
|
||||
const toggle = (idx: number) => setSel(toggleOption(selectMode, sel, idx));
|
||||
|
||||
const canSubmit =
|
||||
qd.mode === "text"
|
||||
? text.trim().length > 0
|
||||
: dontKnow || (otherSelected && other.trim().length > 0) || selected.size > 0;
|
||||
: sel.dontKnow || (sel.otherSelected && other.trim().length > 0) || sel.selected.size > 0;
|
||||
|
||||
const submit = () => {
|
||||
if (!canSubmit) return;
|
||||
const req: AnswerRequest = {
|
||||
dontKnow,
|
||||
values: [...selected].map((i) => qd.options[i]!.value),
|
||||
other: otherSelected ? other : undefined,
|
||||
dontKnow: sel.dontKnow,
|
||||
values: [...sel.selected].map((i) => qd.options[i]!.value),
|
||||
other: sel.otherSelected ? other : undefined,
|
||||
text: qd.mode === "text" ? text : undefined,
|
||||
note: qd.mode !== "text" ? thinking.trim() || undefined : undefined,
|
||||
};
|
||||
@@ -255,11 +248,7 @@ function QuestionDockForm({
|
||||
|
||||
// Selecting Other never clears the thinking field — two channels, the
|
||||
// answer and the reasoning behind it, side by side.
|
||||
const selectOther = () => {
|
||||
setDontKnow(false);
|
||||
setSelected(new Set());
|
||||
setOtherSelected(true);
|
||||
};
|
||||
const selectOther = () => setSel(toggleOther(selectMode, sel));
|
||||
|
||||
return (
|
||||
<div data-testid="question-dock">
|
||||
@@ -281,7 +270,7 @@ function QuestionDockForm({
|
||||
{qd.mode !== "text" && (
|
||||
<div className="flex flex-col gap-2 pb-1" role={qd.mode === "multi-select" ? "group" : "radiogroup"}>
|
||||
{qd.options.map((opt, i) => {
|
||||
const isSel = selected.has(i);
|
||||
const isSel = sel.selected.has(i);
|
||||
return (
|
||||
<button
|
||||
key={opt.value}
|
||||
@@ -318,15 +307,13 @@ function QuestionDockForm({
|
||||
{kind === "quiz" && (
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
setSelected(new Set());
|
||||
setOtherSelected(false);
|
||||
setDontKnow((v) => !v);
|
||||
}}
|
||||
aria-pressed={dontKnow}
|
||||
onClick={() =>
|
||||
setSel((prev) => ({ ...prev, selected: new Set(), otherSelected: false, dontKnow: !prev.dontKnow }))
|
||||
}
|
||||
aria-pressed={sel.dontKnow}
|
||||
className={cn(
|
||||
"rounded-lg border border-dashed px-3 py-2 text-left text-sm text-muted-foreground transition-colors",
|
||||
dontKnow && "border-primary text-foreground",
|
||||
sel.dontKnow && "border-primary text-foreground",
|
||||
)}
|
||||
>
|
||||
I don't know
|
||||
@@ -336,24 +323,24 @@ function QuestionDockForm({
|
||||
{kind === "ask" && (
|
||||
<div
|
||||
role={qd.mode === "multi-select" ? "checkbox" : "radio"}
|
||||
aria-checked={otherSelected}
|
||||
aria-checked={sel.otherSelected}
|
||||
data-testid="other-option"
|
||||
onClick={selectOther}
|
||||
className={cn(
|
||||
"flex items-start gap-3 rounded-lg border px-3 py-2 text-left text-sm transition-colors",
|
||||
otherSelected ? "border-primary bg-accent" : "border-dashed hover:bg-accent/50",
|
||||
sel.otherSelected ? "border-primary bg-accent" : "border-dashed hover:bg-accent/50",
|
||||
)}
|
||||
>
|
||||
<span
|
||||
className={cn(
|
||||
"mt-0.5 flex size-4 shrink-0 items-center justify-center border",
|
||||
qd.mode === "multi-select" ? "rounded-sm" : "rounded-full",
|
||||
otherSelected
|
||||
sel.otherSelected
|
||||
? "border-primary bg-primary text-primary-foreground"
|
||||
: "border-muted-foreground/40",
|
||||
)}
|
||||
>
|
||||
{otherSelected && (
|
||||
{sel.otherSelected && (
|
||||
<span className={cn(qd.mode === "multi-select" ? "text-[10px]" : "size-1.5 rounded-full bg-current")}>
|
||||
{qd.mode === "multi-select" ? "✓" : ""}
|
||||
</span>
|
||||
@@ -363,7 +350,9 @@ function QuestionDockForm({
|
||||
value={other}
|
||||
onChange={(e) => {
|
||||
setOther(e.target.value);
|
||||
if (!otherSelected) setOtherSelected(true);
|
||||
// Typing selects the row — displacing in
|
||||
// single-select, alongside the siblings in multi.
|
||||
setSel((prev) => (prev.otherSelected ? prev : toggleOther(selectMode, prev)));
|
||||
}}
|
||||
placeholder="Type your own answer…"
|
||||
className="w-full min-w-0 flex-1 bg-transparent text-sm outline-none placeholder:text-muted-foreground"
|
||||
|
||||
@@ -278,6 +278,44 @@ export interface AnswerRequest {
|
||||
note?: string;
|
||||
}
|
||||
|
||||
// ─── dock selection (the client state machine, pure for tests) ─────────────
|
||||
|
||||
/** The question dock's selection state (0-based indices in display order). */
|
||||
export interface DockSelection {
|
||||
selected: Set<number>;
|
||||
otherSelected: boolean;
|
||||
dontKnow: boolean;
|
||||
}
|
||||
|
||||
export const emptyDockSelection = (): DockSelection => ({ selected: new Set(), otherSelected: false, dontKnow: false });
|
||||
|
||||
/**
|
||||
* Toggling a listed option. Single-select: the option becomes THE answer and
|
||||
* displaces "Other". Multi-select: the option toggles alongside its siblings
|
||||
* AND "Other" — selections are independent (picking options must never clear
|
||||
* a custom answer, and picking the custom row must never clear them).
|
||||
*/
|
||||
export function toggleOption(mode: SelectMode, sel: DockSelection, idx: number): DockSelection {
|
||||
if (mode === "multi-select") {
|
||||
const selected = new Set(sel.selected);
|
||||
if (selected.has(idx)) selected.delete(idx);
|
||||
else selected.add(idx);
|
||||
return { selected, otherSelected: sel.otherSelected, dontKnow: false };
|
||||
}
|
||||
return { selected: new Set([idx]), otherSelected: false, dontKnow: false };
|
||||
}
|
||||
|
||||
/**
|
||||
* Toggling the ask "Other…" row. Single-select: picking it makes it THE
|
||||
* answer, displacing the listed options (the typed text always survives).
|
||||
* Multi-select: it toggles alongside the listed selections and clears
|
||||
* nothing.
|
||||
*/
|
||||
export function toggleOther(mode: SelectMode, sel: DockSelection): DockSelection {
|
||||
if (mode === "multi-select") return { ...sel, otherSelected: !sel.otherSelected, dontKnow: false };
|
||||
return { selected: new Set(), otherSelected: true, dontKnow: false };
|
||||
}
|
||||
|
||||
export const QUESTION_ENTRY = "lh_question";
|
||||
export const QUESTION_ANSWERED_ENTRY = "lh_question_answered";
|
||||
|
||||
|
||||
@@ -19,6 +19,9 @@ import {
|
||||
quizInputProblems,
|
||||
quizInputSchema,
|
||||
resolveCorrect,
|
||||
toggleOption,
|
||||
toggleOther,
|
||||
emptyDockSelection,
|
||||
shuffleOptions,
|
||||
type QuestionData,
|
||||
toClientQuestion,
|
||||
@@ -434,3 +437,50 @@ describe("makeId", () => {
|
||||
assert.equal(ids.size, 200);
|
||||
});
|
||||
});
|
||||
|
||||
// ─── the dock selection state machine (multi-select Other coexistence) ──
|
||||
// The live bug: in a multi-select ask, picking the custom "Other…" row
|
||||
// cleared every already-picked option (single-select displacement logic
|
||||
// applied to both modes). The machine lives here, pure and pinned.
|
||||
describe("dock selection (toggleOption / toggleOther)", () => {
|
||||
it("multi-select: options and Other coexist — neither clears the other", () => {
|
||||
let sel = emptyDockSelection();
|
||||
sel = toggleOption("multi-select", sel, 0);
|
||||
sel = toggleOption("multi-select", sel, 1);
|
||||
assert.deepEqual([...sel.selected], [0, 1]);
|
||||
// Picking Other keeps the sibling selections.
|
||||
sel = toggleOther("multi-select", sel);
|
||||
assert.deepEqual([...sel.selected], [0, 1], "Other never clears picked options");
|
||||
assert.equal(sel.otherSelected, true);
|
||||
// Picking another option keeps Other selected.
|
||||
sel = toggleOption("multi-select", sel, 2);
|
||||
assert.equal(sel.otherSelected, true, "options never clear a typed custom answer");
|
||||
assert.deepEqual([...sel.selected], [0, 1, 2]);
|
||||
// Deselecting an option toggles it alone.
|
||||
sel = toggleOption("multi-select", sel, 0);
|
||||
assert.deepEqual([...sel.selected], [1, 2]);
|
||||
// Other toggles off alone; the siblings survive.
|
||||
sel = toggleOther("multi-select", sel);
|
||||
assert.equal(sel.otherSelected, false);
|
||||
assert.deepEqual([...sel.selected], [1, 2]);
|
||||
});
|
||||
|
||||
it("multi-select: any selection clears dont-know", () => {
|
||||
let sel = { selected: new Set<number>(), otherSelected: false, dontKnow: true };
|
||||
sel = toggleOption("multi-select", sel, 0);
|
||||
assert.equal(sel.dontKnow, false);
|
||||
sel = toggleOther("multi-select", sel);
|
||||
assert.equal(sel.dontKnow, false);
|
||||
});
|
||||
|
||||
it("single-select: displacement semantics are unchanged", () => {
|
||||
let sel = emptyDockSelection();
|
||||
sel = toggleOption("single-select", sel, 0);
|
||||
sel = toggleOther("single-select", sel);
|
||||
assert.deepEqual([...sel.selected], [], "Other displaces the picked option");
|
||||
assert.equal(sel.otherSelected, true);
|
||||
sel = toggleOption("single-select", sel, 1);
|
||||
assert.equal(sel.otherSelected, false, "an option displaces Other");
|
||||
assert.deepEqual([...sel.selected], [1]);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -17,6 +17,7 @@ import { describe, it } from "node:test";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
import { MessageResponse } from "../components/ai-elements/message.tsx";
|
||||
import { QuestionDock } from "../components/question-dock.tsx";
|
||||
import { ProbeDoneCard } from "../components/artifacts/probe-done-card.tsx";
|
||||
import { AnsweredCard } from "../components/answered-card.tsx";
|
||||
import { buildAnswer, coerceCorrectAnswer, normalizeOptions, resolveCorrect, type QuestionData } from "../src/core/question-core.ts";
|
||||
|
||||
@@ -103,6 +104,22 @@ describe("render smoke tests", () => {
|
||||
assert.match(html, /Your thinking/, "ask select-mode also gets the learner's voice");
|
||||
});
|
||||
|
||||
it("multi-select ask renders checkbox rows and the select-all hint", () => {
|
||||
const html = renderDock({
|
||||
toolName: "ask_user_question",
|
||||
toolCallId: "m1",
|
||||
input: {
|
||||
question: "Which topics interest you?",
|
||||
multiSelect: true,
|
||||
options: [{ label: "Physics" }, { label: "Philosophy" }],
|
||||
} as never,
|
||||
});
|
||||
assert.match(html, /select all that apply/);
|
||||
assert.ok(html.includes('role="group"'), "checkbox group");
|
||||
assert.ok((html.match(/role="checkbox"/g) ?? []).length >= 3, "options AND the Other row are checkboxes");
|
||||
assert.ok(html.includes('data-testid="other-option"'), "Other is still an option row in multi-select");
|
||||
});
|
||||
|
||||
it("phase scales the thinking field: teaching invites depth, probing stays quick", () => {
|
||||
const teach = renderToStaticMarkup(
|
||||
<QuestionDock
|
||||
@@ -302,3 +319,60 @@ describe("teach-check reveal (W5 phase separation)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ─── probe-done card (the malformed-assessment crash) ─────────────────────
|
||||
// The live bug: the model stringified `assessment` (a rejected input), and
|
||||
// the card's input-preview fallback trusted the shape — strands died. The
|
||||
// preview now reads only pending inputs; an errored part renders the quiet
|
||||
// retry note.
|
||||
describe("probe-done card", () => {
|
||||
const validAssessment = {
|
||||
strands: [
|
||||
{ strand: "modal logic basics", assessment: "Floor: solid intuition; ceiling: notation." },
|
||||
{ strand: "indexical 'actual'", assessment: "Genuine floor — prior exposure likely." },
|
||||
],
|
||||
summary: "Bracketed: teach the symbols on the intuition he has.",
|
||||
};
|
||||
|
||||
it("renders the strand-by-strand assessment from the seam ack", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<ProbeDoneCard
|
||||
part={{ state: "output-available", output: { ok: true, assessment: validAssessment } } as never}
|
||||
/>,
|
||||
);
|
||||
assert.ok(html.includes('data-testid="probe-done-card"'));
|
||||
assert.match(html, /modal logic basics/);
|
||||
assert.match(html, /indexical 'actual'|indexical 'actual'/);
|
||||
assert.match(html, /Bracketed/);
|
||||
});
|
||||
|
||||
it("previews the assessment from a PENDING (validated) input", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<ProbeDoneCard part={{ state: "input-available", input: { assessment: validAssessment } } as never} />,
|
||||
);
|
||||
assert.match(html, /modal logic basics/);
|
||||
});
|
||||
|
||||
it("a REJECTED assessment (the stringified live shape) renders the retry note, never the garbage", () => {
|
||||
// The exact live crash: assessment arrived as a string, zod rejected
|
||||
// the call, the model retried — this dead part must not explode.
|
||||
const html = renderToStaticMarkup(
|
||||
<ProbeDoneCard
|
||||
part={{
|
||||
state: "output-error",
|
||||
errorText: "Validation failed",
|
||||
input: { assessment: "strands: [{\"strand\": ...stringified garbage" },
|
||||
} as never}
|
||||
/>,
|
||||
);
|
||||
assert.match(html, /malformed/);
|
||||
assert.match(html, /retried/);
|
||||
assert.ok(!html.includes("strands:"), "the rejected input never renders");
|
||||
});
|
||||
|
||||
it("a mid-stream partial without strands renders the writing note, not a crash", () => {
|
||||
const html = renderToStaticMarkup(
|
||||
<ProbeDoneCard part={{ state: "input-streaming", input: { assessment: { summary: "…" } } } as never} />,
|
||||
);
|
||||
assert.match(html, /Writing the assessment/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user