From afba6261a22c69837d74000cf8cb721d24cf20c0 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sat, 5 Sep 2026 21:38:22 +0300 Subject: [PATCH 1/6] =?UTF-8?q?Research=20steers:=20the=20wave-1=20'view?= =?UTF-8?q?=20full=20research'=20affordance=20=E2=80=94=20collapsed=20=20carries=20the=20full=20brief?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Research-complete one-liner is now a native
disclosure: collapsed it reads exactly as before (Research · complete · N sources); one click expands the full persisted brief + source links via ResearchBriefCard. No fetch, no state, no protocol change — the data-subagent-artifact part in the record is the single source of truth, which is what made this affordance 'feasible later' back in wave 1. The brief still never clutters the transcript by default; the teacher's wake turn remains the primary channel for weaving it into the lesson. Test updated: the brief + sources now assert PRESENT inside the details (and collapsed — no open attribute) instead of absent from the DOM. Gates: typecheck, 279/279 tests, lint, build — all green. --- components/artifacts/steer-card.tsx | 36 ++++++++++++++++++----------- tests/demos.test.tsx | 12 +++++++--- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/components/artifacts/steer-card.tsx b/components/artifacts/steer-card.tsx index d1b223d..fab2503 100644 --- a/components/artifacts/steer-card.tsx +++ b/components/artifacts/steer-card.tsx @@ -15,11 +15,11 @@ import { VisualOutputCard } from "@/components/artifacts/visual-card"; * ("data-subagent-artifact") carries the result. It renders as an artifact — * NEVER a learner bubble, never wrapped in a message container: * - * research — a quiet "Research complete" badge. The full brief is TOO LONG - * to inline and the teacher's wake turn weaves it into the lesson anyway; - * it stays persisted in the transcript (the artifact part), so a - * "view full research" affordance can be added later. Failures keep the - * failure card (the brief explains what went wrong). + * research — a quiet "Research complete" one-liner that expands on demand + * (native
) to the full persisted brief + sources: the brief is + * too long to inline by default and the teacher's wake turn weaves it + * into the lesson anyway. Failures keep the failure card (the brief + * explains what went wrong). * demo / visual — the artifact itself (sandboxed iframe / sanitized SVG). * The sub-agent's verification notes are model-facing glue and do NOT * render; only a failure note does. @@ -48,15 +48,25 @@ export function SubagentSteerCard({ message }: { message: TeacherUIMessage }) { } } -/** Quiet one-liner, the dispatch-settled idiom: the brief lives in the record, - * the lesson lives in the teacher's wake turn — the badge just marks delivery. */ +/** Quiet one-liner, the dispatch-settled idiom — the lesson lives in the + * teacher's wake turn; the badge just marks delivery. The persisted brief + * rides along in a collapsed
(the wave-1 "view full research" + * affordance): full markdown + sources, one click away, no extra fetch — + * the artifact part in the record is the single source of truth. */ function ResearchCompleteBadge({ result }: { result: ResearchResult }) { return ( -
- - Research - complete - {result.sources.length > 0 && · {result.sources.length} sources} -
+
+ + + Research + complete + {result.sources.length > 0 && · {result.sources.length} sources} + view brief + hide brief + +
+ +
+
); } diff --git a/tests/demos.test.tsx b/tests/demos.test.tsx index b2b349c..84c151b 100644 --- a/tests/demos.test.tsx +++ b/tests/demos.test.tsx @@ -342,7 +342,7 @@ describe("steer card rendering (the delivered artifact)", () => { assert.ok(!html.includes("Looking at the render"), "model-facing notes never render"); }); - it("renders a quiet Research-complete badge — the full brief stays out of the transcript (but persisted)", () => { + it("renders a quiet Research-complete one-liner whose collapsed
carries the full brief", () => { const steer = buildSubagentSteer( { id: "jr", kind: "research", toolCallId: "cr" }, { @@ -357,8 +357,14 @@ describe("steer card rendering (the delivered artifact)", () => { assert.match(html, /Research/); assert.match(html, /complete/); assert.match(html, /2 sources/); - assert.ok(!html.includes("founded the Academy"), "the long brief never renders inline"); - assert.ok(!html.includes("britannica"), "source links stay out too — the wake turn cites them"); + assert.match(html, /view brief/, "the expansion affordance is offered"); + // the one-liner is a native
— collapsed by default, so the + // brief + sources ride in the DOM but never clutter the transcript + // until the learner opens them (the wave-1 "view full research" ask) + assert.match(html, /]*data-testid="research-complete-badge"/, "a real disclosure"); + assert.ok(!/]*open/.test(html), "collapsed by default — no `open` attribute"); + assert.ok(html.includes("founded the Academy"), "the persisted brief rides inside the details"); + assert.ok(html.includes("britannica.com/plato"), "source links ride inside the details"); }); it("renders the visual failure note with no svg when the build FAILED", () => { From 67fbc0575fb2ceb4b0c3510629007580ef3a9e07 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 9 Sep 2026 03:04:16 +0300 Subject: [PATCH 2/6] Author-once plan protocol: single mermaid plan_init + derived statuses + live quiz nodeId validation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - plan_init is now the ONLY plan tool: a mermaid 'graph TD' block parsed by a constrained-subset parser (mermaid-plan.ts) with line-precise errors; the seven mutation/tracking tools are gone by design (2026-09-03 design pass). - Re-call plan_init = sanctioned wholesale rewrite; slugs are permanent. - derivePlanStatuses (contracts/plan.ts): done = passed check tagged with a quiz nodeId; current = open check's node else first topo-ready node — the tracking the teacher no longer declares by hand. - questionToolsFor(ctx) (tools.ts): the quiz schema chains a dynamic plan- nodeId validation against the LIVE record — an invalid nodeId is rejected at the call boundary and the teacher retries mid-turn, before any UI. Static questionTools stay validator-less for the default path. - Prompts rewritten for mermaid authoring + derived position. - Out-of-lane surgical touches (flagged for master review): src/core/question-core.ts (quizInputObject export + nodeId description), src/core/contracts/plan.ts (derivePlanStatuses + doc). --- src/agent/prompts/10-plan-tree.md | 33 ++-- src/agent/prompts/30-phases.md | 2 +- src/agent/system-prompt.md | 2 +- src/agent/tools.ts | 75 ++++++-- src/agent/tools/mermaid-plan.ts | 132 ++++++++++++++ src/agent/tools/plan.ts | 211 ++++++---------------- src/core/contracts/plan.ts | 85 ++++++++- src/core/question-core.ts | 14 +- tests/plan-tree.test.ts | 286 ++++++++++++++++++++---------- tests/question-tools.test.ts | 148 ++++++++++++++++ 10 files changed, 691 insertions(+), 297 deletions(-) create mode 100644 src/agent/tools/mermaid-plan.ts create mode 100644 tests/question-tools.test.ts diff --git a/src/agent/prompts/10-plan-tree.md b/src/agent/prompts/10-plan-tree.md index f8c3db9..5595705 100644 --- a/src/agent/prompts/10-plan-tree.md +++ b/src/agent/prompts/10-plan-tree.md @@ -1,32 +1,29 @@ -# The plan tree +# The plan map -Your sessions have a formal dependency tree — the session's map, one object with two faces: you author it with the `plan_*` tools, and the learner SEES it, live, in a side panel. Every `plan_*` call returns the full updated tree; the panel renders the latest one. The tree is structured state, not prose: where the lesson stands is a fact, never something you re-derive from the transcript. +Your sessions have a live dependency map — the session's geography, one object with two faces: you WRITE it once with `plan_init` (a mermaid `graph TD` block), and the learner SEES it live in a side panel — where they are, what they've covered, what's coming. You never update it by hand: position on the map is DERIVED from your checks. Your only job is the map itself, and naming what each check assesses. -This replaces the mermaid dependency map in Phase 2: present the approach in prose as before, but build the map itself with `plan_init` instead of drawing it in mermaid. Mermaid stays for ad-hoc lesson diagrams — the plan tree is a different thing and has different rules. +Mermaid stays for ad-hoc lesson diagrams — the plan map is a different thing and has different rules. ## When -Probe first — that never changes. Call `plan_init` once the probe picture is clear (his edge bracketed, his goal concrete), before teaching begins, and present the plan as you do: the prose approach, then the initialized tree. Then teach through it. +Probe first — that never changes. Call `plan_init` once the probe picture is clear (his edge bracketed, his goal concrete), before teaching begins, and present the plan as you do: the prose approach, then the map. ## Grain -The tree is a **map, not a syllabus**. +The map is a **map, not a syllabus**. - One reasoning step per node. If a node needs three explanations to teach, it's three nodes. -- `title` ≤ 6 words, `summary` 1–2 sentences. -- Typically 5–15 nodes per session. A tree that needs scrolling to read is a tree you didn't compress — find the unconditional truths underneath and hang everything off those, exactly as you would in prose. -- Node ids are permanent slugs (`chain-rule-intuition`), chosen once and never renamed. Edges are `requires` edges: from = must be understood before to. Direction is a teaching fact, not decoration. +- Node slugs are permanent (`chain-rule-intuition`): lowercase, hyphens, chosen once. Labels ≤ 6 words. +- Typically 5–15 nodes per session. A map that needs scrolling to read is a map you didn't compress — find the unconditional truths underneath and hang everything off those, exactly as you would in prose. +- `-->` edges are `requires` edges: from = must be understood before to. Direction is a teaching fact, not decoration. -## Discipline +## Rewriting -- `plan_set_current` the moment you START teaching a node. The learner should always see where they are. -- `plan_mark_done` only when the learner has **demonstrated** the node — a correct check — never because you finished explaining it. Explaining is yours; done is theirs. -- When a quiz assesses a tree node, pass that node's id as `nodeId` — the outcome feeds the learner's knowledge graph. Untagged outcomes are lost signal. +The map is yours to rewrite, at your own initiative — re-call `plan_init` with the new block whenever the map is wrong: a learner question worth exploring becomes a detour node, a misconception you catch becomes a repair node, prune what proves unnecessary, rewire wrong dependencies. A map that can't change is a wrong map. Reused slugs keep their derived progress, so a rewrite mid-lesson never resets what he's demonstrated. -## Freedom +## Where the map tracks itself -The tree is yours to restructure mid-session, at your own initiative. This is not an exception — it's the point. - -- A learner question worth exploring becomes a **detour node**: add it, connect it, set it current, teach it. -- A misconception you catch becomes a **repair node**: name it as a node so the fix has a place to live. -- Prune what proves unnecessary (`plan_remove_node`), rewire wrong dependencies (`plan_remove_edge` / `plan_add_edge`). A map that can't change is a wrong map. +- Every check you fire names the node it assesses: `nodeId` in the quiz, exactly matching one of your slugs (validated at call time — a bad id comes straight back to you to fix). +- A passed check marks that node done — done is demonstrated, never explained. Explaining is yours; done is theirs. +- The current node is the one under an open check, else the next one up. You never declare position. +- Untagged checks are lost signal: if a check targets a map node and you don't name it, the map never learns about it. diff --git a/src/agent/prompts/30-phases.md b/src/agent/prompts/30-phases.md index 8b02d1b..e4c4bec 100644 --- a/src/agent/prompts/30-phases.md +++ b/src/agent/prompts/30-phases.md @@ -12,7 +12,7 @@ The moment you call `plan_init`, a quiet divider appears in the learner's transc - **Teach first, check second — in that order, in the transcript.** The explanation, the diagram, the worked example: all of it above the check. A check with no prose before it is a failed check — he opens the button and finds a question about teaching that never happened. - **Finish the thought before you fire.** Never call a check mid-explanation. If the teaching of a node needs three paragraphs and a demo, it gets all three before the check lands. -- **One node, one check.** `plan_set_current` when you start teaching the node, teach it completely, then check it (`nodeId` set, so the outcome lands on the graph). +- **One node, one check.** Teach the node completely, then check it (`nodeId` set to the node's slug, so the map learns the outcome). - **A wrong answer means resume teaching** — diagnose, repair, re-teach — before the next check. The button is a gate, not a quiz-show buzzer. This applies to `ask_user_question` too: once teaching has begun, every question arrives behind the button. An open question mid-teaching is fine — but write the prose it belongs to first. diff --git a/src/agent/system-prompt.md b/src/agent/system-prompt.md index 6691f91..83d576a 100644 --- a/src/agent/system-prompt.md +++ b/src/agent/system-prompt.md @@ -112,7 +112,7 @@ A good plan is what makes the teaching feel inevitable instead of arbitrary. **Then present the plan in chat — always, before any teaching.** Two parts: 1. **The approach, in prose.** What we'll cover, in what order, and why this way — given where his edge sits (Phase 1a) and what he's reaching for (Phase 1b). A few freeform sentences. -2. **The dependency map.** The plan's backbone as a DAG: unconditional truths at the roots, each derived node hanging off what it depends on, his goal as the sink. Build it with the `plan_init` tool (see the plan-tree section below) — the learner SEES this tree live in a side panel, and Phase 3 walks it node by node with `plan_set_current` / `plan_mark_done`. Keep it small: few nodes, short labels — a map, not the territory. +2. **The dependency map.** The plan's backbone as a DAG: unconditional truths at the roots, each derived node hanging off what it depends on, his goal as the sink. Write it once with the `plan_init` tool (see the plan-map section below) — the learner SEES it live in a side panel, and where the lesson stands derives from your checks (`nodeId`), not from your bookkeeping. Keep it small: few nodes, short labels — a map, not the territory. **Stress-test the roots before presenting.** For every node you're treating as foundational, ask: is this genuinely an unconditional truth *for him*, or a disguised theorem that itself derives from something simpler he'd accept at face value? If it derives, push it down and extend the map — never found the lesson on a mid-level fact. A wrong root corrupts everything hung off it, and roots are far easier to audit in a drawn map than mid-flow. diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 2c207af..83db7ba 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -1,5 +1,5 @@ import { tool, type ToolSet } from "ai"; -import { askInputSchema, quizInputSchema } from "../core/question-core.ts"; +import { askInputSchema, quizInputObject, quizInputProblems, quizInputSchema } from "../core/question-core.ts"; import type { JobRecord } from "../core/contracts.ts"; import type { JobStore } from "../core/ports.ts"; import { jobTools, type DemoInput, type ResearchInput, type VisualInput } from "./tools/jobs.ts"; @@ -10,14 +10,20 @@ export { jobTools }; /** * The teacher's tools, composed per request by createTeacherTools. * - * - questionTools (quiz / ask_user_question): CLIENT-SIDE tools (no `execute`). - * The tool call streams to the browser in `input-available` state; the - * learner answers in the dock; the client grades via question-core and - * sends the verdict with addToolOutput. The input schemas live in - * core/question-core (the question contract) — parameter names mirror - * the prompt docs exactly; keep them in sync. - * - planTools (src/agent/tools/plan.ts): SERVER-SIDE, mutate the chat's plan - * tree (needs the chatId closure — hence the per-request factory). + * - questionTools / questionToolsFor (quiz / ask_user_question): CLIENT-SIDE + * tools (no `execute`). The tool call streams to the browser in + * `input-available` state; the learner answers in the dock; the client + * grades via question-core and sends the verdict with addToolOutput. The + * input schemas live in core/question-core (the question contract) — + * parameter names mirror the prompt docs exactly; keep them in sync. + * questionToolsFor adds the per-session plan-nodeId validation: an invalid + * nodeId NEVER reaches the learner — the SDK returns the validation error + * to the teacher as a tool result and it retries (same loop as the + * grading rules). It reads the LIVE record at validation time, so a + * plan_init earlier in the same turn already counts. + * - planTools (src/agent/tools/plan.ts): SERVER-SIDE — the single author-once + * plan_init (mermaid → structured tree; re-callable rewrite). Needs the + * chatId closure — hence the per-request factory. * - jobTools (src/agent/tools/jobs.ts): SERVER-EXECUTED dispatch — the call * resolves immediately with a fire-and-forget ack, the background runner * does the work, and the result arrives later as a steer message @@ -54,6 +60,49 @@ export const questionTools = { ask_user_question: askUserQuestionTool, }; +/** + * Per-session question tools: the quiz schema gains the dynamic plan-nodeId + * validation on top of question-core's shared authoring rules (chained + * superRefines — the grading contract is untouched). The check reads the + * LIVE chat record, so rewrites mid-turn are honored, and with no tree yet + * (probe phase) every nodeId is rejected — ad-hoc checks must omit it. + * ask_user_question is untouched: it never carries nodeId (not a check). + */ +export function questionToolsFor(ctx: Pick): ToolSet { + const quizInputWithNodeValidation = quizInputObject + .superRefine((input, add) => { + for (const problem of quizInputProblems(input)) { + add.addIssue({ code: "custom", message: problem }); + } + }) + .superRefine((input, add) => { + if (typeof input.nodeId !== "string" || input.nodeId.length === 0) return; + const record = ctx.chatStore.getChat(ctx.chatId); + const tree = record?.tree; + const known = tree?.nodes.map((n) => n.id) ?? []; + if (!known.includes(input.nodeId)) { + add.addIssue({ + code: "custom", + message: + `nodeId "${input.nodeId}" matches no node in this session's plan map. ` + + (known.length > 0 + ? `Copy one of your plan_init slugs exactly (${known.slice(0, 5).join(", ")}${known.length > 5 ? ", …" : ""}), or omit nodeId for an ad-hoc check.` + : "No plan map exists yet — omit nodeId (ad-hoc checks only until you call plan_init)."), + }); + } + }); + + const quizWithNodeValidation = tool({ + description: quizTool.description, + inputSchema: quizInputWithNodeValidation, + }); + + return { + quiz: quizWithNodeValidation, + ask_user_question: askUserQuestionTool, + }; +} + export interface TeacherToolContext extends PlanToolContext { /** Job-tool wiring (sub-agent dispatch): the job store + the runner's * spawn seam. The composition root injects both. */ @@ -65,13 +114,13 @@ export interface TeacherToolContext extends PlanToolContext { } /** - * Per-request tool registry. Plan tools close over the ctx (chatId + the - * injected ChatStore) for persistence; question tools are context-free; - * job tools close over the job store + spawn seam. + * Per-request tool registry. Plan tools and question tools close over the + * ctx (chatId + the injected ChatStore — the latter also powers the quiz + * nodeId validation); job tools close over the job store + spawn seam. */ export function createTeacherTools(ctx: TeacherToolContext): ToolSet { return { - ...questionTools, + ...questionToolsFor(ctx), ...planTools(ctx), ...jobTools({ chatId: ctx.chatId, jobStore: ctx.jobStore, spawn: ctx.spawn }), }; diff --git a/src/agent/tools/mermaid-plan.ts b/src/agent/tools/mermaid-plan.ts new file mode 100644 index 0000000..62956a4 --- /dev/null +++ b/src/agent/tools/mermaid-plan.ts @@ -0,0 +1,132 @@ +/** + * Mermaid → PlanTree authoring format (the author-once protocol, 2026-09-03). + * + * The teacher writes the session map as a mermaid `graph TD` block — the + * format models are most fluent in — and plan_init canonicalizes it into the + * structured PlanTree. A CONSTRAINED SUBSET, parsed by hand: deterministic, + * dependency-free, precise line-numbered errors the teacher can act on (the + * same validate-and-retry loop as the question schemas). Not the mermaid + * runtime — the server never renders, it only needs the graph. + * + * Accepted syntax (anything else → an error naming the line): + * graph TD | graph TB | flowchart TD | flowchart TB (case-insensitive) + * %% comment lines + * node — bare id (title falls back to the id) + * id[title] — square-bracket label + * id("title") — paren label + * a --> b — requires edge (from = prerequisite); bare endpoints + * register as nodes; forward references are fine + * a[title] --> b[title] + * + * Node ids are permanent slugs: ^[a-z0-9]+(-[a-z0-9]+)*$ (checks reference + * them via the quiz nodeId). Labels are single-line titles. + */ +import type { PlanEdge, PlanNode } from "@/src/core"; + +export interface ParsedPlan { + nodes: PlanNode[]; + edges: PlanEdge[]; +} + +export interface PlanParseResult { + ok: boolean; + /** All problems, each prefixed with its 1-based line number. */ + problems: string[]; + plan?: ParsedPlan; +} + +const HEADER_RE = /^(graph|flowchart)\s+(td|tb)\s*$/i; +const SLUG_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/; +/** One edge endpoint: `id`, `id[title]`, or `id("title")`. */ +// Case-tolerant on the id: register() then enforces the lowercase slug convention +// with a SPECIFIC message (an "UPPER" id reaches the slug check, not a generic +// parse error — the most common authoring mistake). +const ENDPOINT_RE = /^([A-Za-z0-9][A-Za-z0-9-]*?)\s*(?:\[([^\[\]]*)\]|\(\s*"([^"]*)"\s*\)|\(([^()]*)\))?$/; + +interface Endpoint { + id: string; + label?: string; +} + +function parseEndpoint(text: string): Endpoint | null { + const m = text.trim().match(ENDPOINT_RE); + if (!m || !m[1]) return null; + return { id: m[1], label: (m[2] ?? m[3] ?? m[4] ?? "").trim() }; +} + +/** Parse the mermaid subset. Collects ALL problems (never just the first). */ +export function parseMermaidPlan(mermaid: string): PlanParseResult { + const problems: string[] = []; + const lines = mermaid.split(/\r?\n/); + + // Header: required, first non-empty non-comment line. + const first = lines.findIndex((l) => l.trim().length > 0 && !l.trim().startsWith("%%")); + if (first === -1) { + return { ok: false, problems: ["The plan is empty — expected a `graph TD` block with nodes and `-->` requires-edges."] }; + } + const header = lines[first]?.trim() ?? ""; + if (!HEADER_RE.test(header)) { + problems.push(`line ${first + 1}: expected the header \`graph TD\` (or flowchart TD/TB) first, got \`${header}\`.`); + } + + const nodes = new Map(); // id → title (first label wins) + const edges: PlanEdge[] = []; + const seenEdges = new Set(); + + const register = ({ id, label }: Endpoint, lineNo: number): void => { + if (!SLUG_RE.test(id)) { + problems.push( + `line ${lineNo}: node id \`${id}\` must be a lowercase slug (a-z, 0-9, hyphens — e.g. \`chain-rule-intuition\`); ids are permanent and checks reference them.`, + ); + return; + } + if (!nodes.has(id)) nodes.set(id, label && label.length > 0 ? label : id); + else if (label && label.length > 0 && nodes.get(id) === id) nodes.set(id, label); + }; + + for (let i = first + 1; i < lines.length; i++) { + const line = lines[i]?.trim() ?? ""; + const no = i + 1; + if (line.length === 0 || line.startsWith("%%")) continue; + + const arrows = line.split("-->"); + if (arrows.length === 2) { + const from = parseEndpoint(arrows[0] ?? ""); + const to = parseEndpoint(arrows[1] ?? ""); + if (!from || !to) { + problems.push(`line ${no}: can't read the edge \`${line}\` — expected \`a[title] --> b[title]\`.`); + continue; + } + register(from, no); + register(to, no); + if (from.id === to.id) { + problems.push(`line ${no}: self-loop \`${from.id} --> ${to.id}\` — a node can't require itself.`); + continue; + } + const key = `${from.id}->${to.id}`; + if (!seenEdges.has(key)) { + seenEdges.add(key); + edges.push({ from: from.id, to: to.id }); + } + } else if (arrows.length > 2) { + problems.push(`line ${no}: chained edges \`${line}\` — write one edge per line (the plan is a map, not a sentence).`); + } else { + const node = parseEndpoint(line); + if (!node) { + problems.push( + `line ${no}: can't parse \`${line}\` — the plan accepts node lines (\`slug[title]\` — ids are lowercase slugs: a-z, 0-9, hyphens) and requires-edges (\`a --> b\`) only; subgraphs, styles, classes, and other mermaid features are not part of the subset.`, + ); + continue; + } + register(node, no); + } + } + + const plan: ParsedPlan = { + nodes: [...nodes.entries()].map(([id, title]) => ({ id, title })), + edges, + }; + if (problems.length > 0) return { ok: false, problems }; + if (plan.nodes.length === 0) return { ok: false, problems: ["No nodes found — the plan needs at least one."] }; + return { ok: true, problems: [], plan }; +} diff --git a/src/agent/tools/plan.ts b/src/agent/tools/plan.ts index a25f84a..d92df06 100644 --- a/src/agent/tools/plan.ts +++ b/src/agent/tools/plan.ts @@ -2,18 +2,8 @@ import { tool, type ToolSet } from "ai"; import { z } from "zod"; import type { PlanToolResult, PlanTree } from "../../core/contracts.ts"; import type { ChatStore } from "../../core/ports.ts"; -import { - addEdge, - addNode, - createTree, - emptyTree, - markDone, - PlanTreeError, - removeEdge, - removeNode, - setCurrent, - updateNode, -} from "../../core/plan-tree.ts"; +import { createTree, emptyTree, PlanTreeError } from "../../core/plan-tree.ts"; +import { parseMermaidPlan } from "./mermaid-plan.ts"; /** What the plan tools close over (the plan-tree half of the tool context). */ export interface PlanToolContext { @@ -23,161 +13,66 @@ export interface PlanToolContext { } /** - * Plan-tree tools — W1 workstream (docs/plan.md Wave 1; src/core/contracts.ts - * "Plan tree"). SERVER-SIDE tools WITH execute: read the chat record, apply a - * pure validated op from src/core/plan-tree.ts, persist via saveTree, and - * return PlanToolResult — the FULL updated tree every time, so the UI tree - * panel renders the latest state straight from the transcript's tool outputs. + * The plan tool — AUTHOR-ONCE protocol (2026-09-03 design pass): the teacher + * WRITES the map and never tracks progress. One tool, plan_init, takes the + * whole dependency map as a mermaid `graph TD` block: * - * Edges are `requires` edges: from = prerequisite, to = dependent - * ("from must be understood before to"). + * - call it ONCE at the probe→teach seam (the call fires the phase divider) + * - statuses (done / current / upcoming) are DERIVED from check outcomes + * (contracts.ts derivePlanStatuses) — the teacher never declares them + * - re-call it any time the map is wrong: the new block replaces the tree + * wholesale. Node slugs are permanent — reused ids keep their derived + * done status, so a mid-lesson rewrite never resets honest progress + * - checks name the node they assess (quiz nodeId, validated against the + * live tree at the call boundary) — that tag is the only thing statuses + * derive from + * + * The former mutation tools (add/update/remove node/edge, set_current, + * mark_done) are gone by design: restructuring is a rewrite, tracking is + * derived. SERVER-SIDE with execute: parse → validate → persist → return + * the FULL canonicalized tree (PlanToolResult), so the tree panel renders the + * latest state straight from the transcript's tool outputs. */ - -const nodeId = z - .string() - .min(1) - .describe("Stable node slug, e.g. 'chain-rule-intuition'. Ids are permanent — never reuse or rename one mid-session."); - -const nodeSchema = z.object({ - id: nodeId.describe("Stable node slug, e.g. 'chain-rule-intuition'. Ids are permanent — never reuse or rename one mid-session."), - title: z.string().min(1).describe("Short label for the tree panel — at most ~6 words."), - summary: z.string().optional().describe("1-2 sentences: what this node covers."), -}); - -const edgeSchema = z.object({ - from: nodeId.describe("Prerequisite node id (must be understood before `to`)."), - to: nodeId.describe("Node id that depends on `from`."), -}); - -/** - * Shared execute wrapper: load record → apply pure op → persist → PlanToolResult. - * PlanTreeError and missing chats become {ok:false} with the current (or - * empty) tree, so the model can see what it's working with and fix the call. - */ -async function runPlanOp( - ctx: PlanToolContext, - op: (tree: PlanTree) => PlanTree, -): Promise { - const { chatId, chatStore } = ctx; - const record = chatStore.getChat(chatId); - if (!record) return { ok: false, error: `Chat not found: ${chatId}`, tree: emptyTree() }; - const current = record.tree ?? emptyTree(); - try { - const next = op(current); - chatStore.saveTree(chatId, next); - return { ok: true, tree: next }; - } catch (err) { - if (err instanceof PlanTreeError) return { ok: false, error: err.message, tree: current }; - throw err; - } -} - export function planTools(ctx: PlanToolContext): ToolSet { return { plan_init: tool({ description: - "Initialize this session's plan tree — the dependency map of the lesson, which the learner sees live in a side panel. " + - "Call ONCE, after probing, before teaching begins: pass the full initial node set and the `requires` edges between them " + - "(edge direction: from = prerequisite, to = dependent — from must be understood before to). The tree must be a DAG (no cycles). " + - "Refuses to run if a tree already exists — after that, use the other plan_* tools to evolve it.", + "Write or REWRITE this session's plan map — the dependency graph the learner sees live in a side panel. " + + "Call once after probing, before teaching begins; re-call whenever the map is wrong (the new block replaces the old tree). " + + "A mermaid `graph TD` block: one node per line (`slug[Short Title]` — slugs are permanent lowercase ids), `-->` edges meaning " + + "'from must be understood before to'. 5–15 nodes: a map, not a syllabus — one reasoning step per node, compress. " + + "Every check you fire names the node it assesses via `nodeId`; where the lesson stands derives from those outcomes.", inputSchema: z.object({ - nodes: z.array(nodeSchema).min(1).describe("The initial nodes. A map, not a syllabus: one reasoning step per node."), - edges: z - .array(edgeSchema) - .optional() - .describe("Initial `requires` edges between the nodes. Edge direction: from = prerequisite, to = dependent."), + mermaid: z + .string() + .min(1) + .describe( + "A mermaid `graph TD` block, e.g.:\ngraph TD\n primitives[Number primitives] --> operators[Arithmetic operators]\n operators --> expressions[Evaluating expressions]\n" + + "Node lines: `slug[Title]`. Edge lines: `slug --> slug` (from = prerequisite). No subgraphs, styles, classes, or other mermaid features.", + ), }), - execute: async ({ nodes, edges }): Promise => { - const record = ctx.chatStore.getChat(ctx.chatId); - if (!record) return { ok: false, error: `Chat not found: ${ctx.chatId}`, tree: emptyTree() }; - const existing = record.tree; - if (existing && existing.nodes.length > 0) { - return { - ok: false, - error: "Tree already initialized — use the mutation tools", - tree: existing, - }; + execute: async ({ mermaid }): Promise => { + const { chatId, chatStore } = ctx; + const record = chatStore.getChat(chatId); + if (!record) return { ok: false, error: `Chat not found: ${chatId}`, tree: emptyTree() }; + const current = record.tree ?? emptyTree(); + + const parsed = parseMermaidPlan(mermaid); + if (!parsed.ok || !parsed.plan) { + // The parser collects every problem; the teacher fixes and retries. + return { ok: false, error: parsed.problems.join("\n"), tree: current }; + } + try { + // createTree validates the structure (duplicate ids, missing + // edge endpoints, self-loops, cycles → PlanTreeError). + const next = createTree(parsed.plan.nodes, parsed.plan.edges); + chatStore.saveTree(chatId, next); + return { ok: true, tree: next }; + } catch (err) { + if (err instanceof PlanTreeError) return { ok: false, error: err.message, tree: current }; + throw err; } - return runPlanOp(ctx, () => createTree(nodes, edges ?? [])); }, }), - - plan_add_node: tool({ - description: - "Add one node to the plan tree. Optionally pass `parents`: node ids this new node depends on (a `requires` edge parent → this node is " + - "created for each; a parent must be understood before this node). Parents must already exist; the result must stay a DAG.", - inputSchema: z.object({ - id: nodeId, - title: z.string().min(1).describe("Short label for the tree panel — at most ~6 words."), - summary: z.string().optional().describe("1-2 sentences: what this node covers."), - parents: z - .array(nodeId) - .optional() - .describe("Ids of prerequisite nodes — each gets a `requires` edge pointing to this node. All must already exist."), - }), - execute: async ({ id, title, summary, parents }): Promise => - runPlanOp(ctx, (tree) => addNode(tree, { id, title, ...(summary !== undefined ? { summary } : {}) }, parents)), - }), - - plan_update_node: tool({ - description: - "Update a node's `title` and/or `summary`. The node must exist; the id never changes. Use this to sharpen a vague node, not to change its meaning.", - inputSchema: z - .object({ - id: nodeId, - title: z.string().min(1).optional().describe("New short label (at most ~6 words)."), - summary: z.string().optional().describe("New 1-2 sentence summary."), - }) - .refine((v) => v.title !== undefined || v.summary !== undefined, { - message: "Pass at least one of `title` or `summary` — an update with neither changes nothing.", - }), - execute: async ({ id, title, summary }): Promise => - runPlanOp(ctx, (tree) => updateNode(tree, id, { ...(title !== undefined ? { title } : {}), ...(summary !== undefined ? { summary } : {}) })), - }), - - plan_remove_node: tool({ - description: - "Remove a node from the plan tree — prune what proves unnecessary. Cascades: every edge touching the node (both directions), its current " + - "status, and its done status are removed with it.", - inputSchema: z.object({ id: nodeId }), - execute: async ({ id }): Promise => runPlanOp(ctx, (tree) => removeNode(tree, id)), - }), - - plan_add_edge: tool({ - description: - "Add a `requires` edge to the plan tree. Direction: from = prerequisite, to = dependent — `from` must be understood before `to`. " + - "Both nodes must exist; self-loops, duplicate edges, and anything that would create a cycle are rejected.", - inputSchema: z.object({ - from: nodeId.describe("Prerequisite node id (must be understood before `to`)."), - to: nodeId.describe("Dependent node id (requires `from`)."), - }), - execute: async ({ from, to }): Promise => runPlanOp(ctx, (tree) => addEdge(tree, from, to)), - }), - - plan_remove_edge: tool({ - description: - "Remove an existing `requires` edge from the plan tree (the nodes stay). Use when a dependency turns out to be wrong or unnecessary.", - inputSchema: z.object({ - from: nodeId.describe("Prerequisite end of the edge to remove."), - to: nodeId.describe("Dependent end of the edge to remove."), - }), - execute: async ({ from, to }): Promise => runPlanOp(ctx, (tree) => removeEdge(tree, from, to)), - }), - - plan_set_current: tool({ - description: - "Set the CURRENT node — where the lesson is right now. Call it the moment you START teaching a node, so the learner sees their position " + - "on the map. The node must exist. This marks position only; it does not mark anything done.", - inputSchema: z.object({ id: nodeId.describe("The node you are starting to teach now.") }), - execute: async ({ id }): Promise => runPlanOp(ctx, (tree) => setCurrent(tree, id)), - }), - - plan_mark_done: tool({ - description: - "Mark a node DONE — and only when the learner has DEMONSTRATED it (a correct check), not when you finished explaining it. " + - "The node must exist. Idempotent.", - inputSchema: z.object({ id: nodeId.describe("The node the learner just demonstrated.") }), - execute: async ({ id }): Promise => runPlanOp(ctx, (tree) => markDone(tree, id)), - }), }; -} +} \ No newline at end of file diff --git a/src/core/contracts/plan.ts b/src/core/contracts/plan.ts index 291627f..3a05519 100644 --- a/src/core/contracts/plan.ts +++ b/src/core/contracts/plan.ts @@ -44,14 +44,85 @@ export function planNodeStatus(tree: PlanTree, nodeId: string): PlanNodeStatus { return "upcoming"; } +// ─── derived statuses: the author-once protocol (2026-09-03) ───────────────────── +// +// The teacher AUTHORS the map (plan_init, mermaid — once at the seam, +// re-callable to rewrite) and never tracks progress: statuses derive from +// the transcript, pure and at render time. A check names its node (quiz +// input nodeId — validated at the call boundary against the live tree); +// the derivation reads outcomes: +// done — any check on the node that the learner PASSED (demonstration- +// only: "explaining is yours, done is theirs") +// current — the node of the last UNANSWERED check; if none, the first +// node in authoring order whose prerequisites are all done and +// that isn't done itself (forward-looking: the panel leads the +// lesson — during teaching it points at what's coming, and holds +// through the check that follows) +// upcoming — everything else +// Untagged checks and nodeIds missing from the current tree are lost signal +// (the standing doctrine); a rewritten map keeps honest progress for reused +// slugs because the derivation matches ids against the CURRENT tree. + +/** Minimal structural message slice — anything with typed parts (mirrors + * phases.ts: pure, no runtime deps, safe on server, client, and in tests). */ +interface PartsCarrier { + parts: Array<{ type: string; state?: string; input?: unknown; output?: unknown }>; +} + +export interface DerivedPlanStatuses { + current: string | null; + /** Node ids demonstrated by the learner (passed checks). */ + done: ReadonlySet; +} + +/** Statuses of a node: the derived view the plan panel should render. */ +export function derivedNodeStatus(statuses: DerivedPlanStatuses, nodeId: string): PlanNodeStatus { + if (statuses.current === nodeId) return "current"; + if (statuses.done.has(nodeId)) return "done"; + return "upcoming"; +} + +export function derivePlanStatuses(tree: PlanTree, messages: readonly PartsCarrier[]): DerivedPlanStatuses { + const ids = new Set(tree.nodes.map((n) => n.id)); + const done = new Set(); + let current: string | null = null; + + for (const m of messages) { + for (const part of m.parts) { + if (part.type !== "tool-quiz") continue; + const nodeId = (part.input as { nodeId?: unknown } | undefined)?.nodeId; + if (typeof nodeId !== "string" || !ids.has(nodeId)) continue; // untagged/unknown = lost signal + if (part.state === "input-available") { + current = nodeId; // a check is out — that's where we are + } else if (part.state === "output-available") { + if ((part.output as { correct?: unknown } | undefined)?.correct === true) done.add(nodeId); + // failed checks change nothing: resume teaching, re-check later + } + } + } + + if (current === null) { + // No check out: the first node (authoring order) whose prerequisites + // are all demonstrated and that isn't done itself — the next lesson. + const prereqs = new Map(); + for (const e of tree.edges) prereqs.set(e.to, [...(prereqs.get(e.to) ?? []), e.from]); + current = + tree.nodes.find( + (n) => !done.has(n.id) && (prereqs.get(n.id) ?? []).every((p) => done.has(p)), + )?.id ?? null; + } + return { current, done }; +} + /** - * Plan tools (W1 implements; server-side WITH execute, mutate the chat's - * stored tree, and return the full updated tree so the panel can render the - * latest state straight from the stream). Every tool returns PlanToolResult. - * Tool names: plan_init, plan_add_node, plan_update_node, plan_remove_node, - * plan_add_edge, plan_remove_edge, plan_set_current, plan_mark_done. - * The tree panel derives its display from the LATEST plan tool output in the - * transcript (live during streaming) — no separate state channel. + * The plan tool result (author-once protocol, 2026-09-03): ONE server-side + * tool, plan_init (tools/plan.ts) — a mermaid `graph TD` block canonicalized + * into the structured tree, called once at the probe→teach seam and + * re-callable to rewrite. The former mutation/tracking tools are gone: + * statuses derive from check outcomes (derivePlanStatuses above), so the + * tree carries structure, never progress. The tree panel still derives its + * display from the LATEST plan tool output in the transcript — plus the + * derived statuses — with no separate state channel. */ export interface PlanToolResult { ok: boolean; diff --git a/src/core/question-core.ts b/src/core/question-core.ts index 16d5436..4ff071f 100644 --- a/src/core/question-core.ts +++ b/src/core/question-core.ts @@ -107,7 +107,7 @@ export function quizInputProblems(input: { return problems; } -export const quizInputSchema = z +export const quizInputObject = z .object({ question: z.string().describe("The single quiz question to ask. Ask exactly one question per tool call."), options: z @@ -142,8 +142,10 @@ export const quizInputSchema = z .string() .optional() .describe( - "Optional plan-tree node id this question assesses. Set whenever the question targets a tree node — " + - "the outcome feeds the learner's knowledge graph. Omit for ad-hoc probes.", + "Optional plan-tree node id this question assesses — must EXACTLY match one of this session's " + + "plan_init node slugs (validated at call time; a mismatched id is rejected back to you). " + + "Set it whenever the question targets a map node — the outcome drives the learner's live session map. " + + "Omit for ad-hoc checks.", ), }) // The cross-field authoring contract (the Gate-3 hardening): a quiz whose @@ -152,6 +154,12 @@ export const quizInputSchema = z // failure, returns the validation error to the teacher as a tool result — // the teacher retries with a fixed quiz in the same turn. Schema-valid ⇔ // gradeable: the rules are exactly what the grading path accepts. + // + // NOTE the split: the raw object (pre-refinement) is exported for + // composition — tools.ts questionToolsFor chains the per-session plan-nodeId + // validation (it reads the live chat record, which this core never touches) + // on top of this shared refinement. +export const quizInputSchema = quizInputObject .superRefine((input, ctx) => { for (const problem of quizInputProblems(input)) { ctx.addIssue({ code: "custom", message: problem }); diff --git a/tests/plan-tree.test.ts b/tests/plan-tree.test.ts index a6ba0c5..316d4fb 100644 --- a/tests/plan-tree.test.ts +++ b/tests/plan-tree.test.ts @@ -12,7 +12,7 @@ import assert from "node:assert/strict"; import * as fs from "node:fs"; import * as path from "node:path"; import { after, describe, it } from "node:test"; -import type { PlanTree, PlanToolResult } from "../src/core/contracts.ts"; +import { derivePlanStatuses, derivedNodeStatus, type PlanTree, type PlanToolResult } from "../src/core/contracts.ts"; import { addEdge, addNode, @@ -26,6 +26,7 @@ import { updateNode, } from "../src/core/plan-tree.ts"; import { planTools } from "../src/agent/tools/plan.ts"; +import { parseMermaidPlan } from "../src/agent/tools/mermaid-plan.ts"; import { chatStore } from "../src/store/chats.ts"; import { PATHS } from "../config.ts"; @@ -233,138 +234,231 @@ function newChat(title: string): string { return chat.id; } -describe("plan tools (execute against the real store)", () => { - it("plan_init creates and persists the tree; result is ok with the full tree", async () => { - const chatId = newChat("Plan tools: init"); +describe("plan_init — the author-once mermaid tool (execute against the real store)", () => { + it("parses a mermaid block, builds the tree, persists, returns the full tree", async () => { + const chatId = newChat("Plan: init"); const tools = planTools({ chatId, chatStore }); const res = await exec(tools, "plan_init", { - nodes: [ - { id: "truth", title: "Core truth" }, - { id: "goal", title: "The goal" }, - ], - edges: [{ from: "truth", to: "goal" }], + mermaid: "graph TD\n primitives[Number primitives] --> operators[Arithmetic operators]\n operators --> expressions[Evaluating expressions]\n detour[Side quest]\n", }); assert.equal(res.ok, true); assert.equal(res.error, undefined); assert.equal(res.tree.version, 1); - assert.equal(res.tree.nodes.length, 2); - assert.deepEqual(res.tree.edges, [{ from: "truth", to: "goal" }]); + assert.deepEqual( + res.tree.nodes.map((n) => n.id), + ["primitives", "operators", "expressions", "detour"], + ); + assert.deepEqual(res.tree.edges, [ + { from: "primitives", to: "operators" }, + { from: "operators", to: "expressions" }, + ]); const saved = chatStore.getChat(chatId)!.tree; assert.ok(saved, "tree persisted to the chat record"); assert.deepEqual(saved, res.tree); }); - it("plan_init refuses to re-init and never silently wipes the existing tree", async () => { - const chatId = newChat("Plan tools: re-init"); + it("RE-CALLABLE: a rewrite replaces the tree wholesale (the mutation tools are gone)", async () => { + const chatId = newChat("Plan: rewrite"); const tools = planTools({ chatId, chatStore }); - await exec(tools, "plan_init", { nodes: [{ id: "a", title: "A" }] }); + await exec(tools, "plan_init", { + mermaid: "graph TD\n a[First attempt] --> b[Early shape]\n", + }); const res = await exec(tools, "plan_init", { - nodes: [{ id: "totally", title: "Different tree" }], + mermaid: "graph TD\n truth[The core truth]\n goal[The sink] --> truth\n", }); - assert.equal(res.ok, false); - assert.equal(res.error, "Tree already initialized — use the mutation tools"); - assert.deepEqual(res.tree.nodes.map((n: { id: string }) => n.id), ["a"], "existing tree returned, not wiped"); - - const saved = chatStore.getChat(chatId)!.tree!; - assert.deepEqual(saved.nodes.map((n) => n.id), ["a"]); + assert.equal(res.ok, true, "re-init is the sanctioned rewrite path now"); + assert.deepEqual( + res.tree.nodes.map((n) => n.id), + ["truth", "goal"], + ); + assert.deepEqual( + chatStore.getChat(chatId)!.tree!.nodes.map((n) => n.id), + ["truth", "goal"], + "rewrite persisted", + ); }); - it("plan_init returns an error result on invalid input (cycle) without persisting", async () => { - const chatId = newChat("Plan tools: bad init"); + it("parse failures collect EVERY problem with line numbers and persist nothing", async () => { + const chatId = newChat("Plan: bad parse"); const tools = planTools({ chatId, chatStore }); + const res = await exec(tools, "plan_init", { - nodes: [ - { id: "a", title: "A" }, - { id: "b", title: "B" }, - ], - edges: [ - { from: "a", to: "b" }, - { from: "b", to: "a" }, - ], + mermaid: "graph TD\n Foo Bar[x] --> y\n a --> a\n subgraph hi\n", }); assert.equal(res.ok, false); - assert.match(res.error!, /cycle/); + const err = res.error!; + assert.match(err, /line 2/); + assert.match(err, /line 3/); + assert.match(err, /line 4/); assert.deepEqual(res.tree, emptyTree()); assert.equal(chatStore.getChat(chatId)!.tree, undefined, "nothing persisted on failure"); }); - it("mutation round-trip: add node with parents, set current, mark done — each persists", async () => { - const chatId = newChat("Plan tools: round-trip"); + it("structural failures (cycle) become ok:false without persisting", async () => { + const chatId = newChat("Plan: cycle"); const tools = planTools({ chatId, chatStore }); - - await exec(tools, "plan_init", { - nodes: [ - { id: "ratelimit", title: "Token bucket" }, - { id: "burst", title: "Burst vs steady" }, - ], - edges: [{ from: "ratelimit", to: "burst" }], + const res = await exec(tools, "plan_init", { + mermaid: "graph TD\n a[A] --> b[B]\n b --> a\n", }); - - const added = await exec(tools, "plan_add_node", { - id: "detour", - title: "Why leaky bucket differs", - summary: "Learner asked about the leaky-bucket variant.", - parents: ["ratelimit"], - }); - assert.equal(added.ok, true); - assert.deepEqual( - added.tree.edges.map((e) => `${e.from}→${e.to}`), - ["ratelimit→burst", "ratelimit→detour"], - ); - assert.equal(added.tree.version, 2); - assert.deepEqual(chatStore.getChat(chatId)!.tree!, added.tree, "add_node persisted"); - - const current = await exec(tools, "plan_set_current", { id: "detour" }); - assert.equal(current.ok, true); - assert.equal(current.tree.currentNodeId, "detour"); - assert.equal(chatStore.getChat(chatId)!.tree!.currentNodeId, "detour", "set_current persisted"); - - const done = await exec(tools, "plan_mark_done", { id: "ratelimit" }); - assert.equal(done.ok, true); - assert.deepEqual(done.tree.doneNodeIds, ["ratelimit"]); - assert.deepEqual(chatStore.getChat(chatId)!.tree!.doneNodeIds, ["ratelimit"], "mark_done persisted"); - assert.equal(done.tree.version, 4); + assert.equal(res.ok, false); + assert.match(res.error!, /cycle/); + assert.deepEqual(res.tree, emptyTree()); }); - it("invalid mutations return ok:false with the current tree and persist nothing", async () => { - const chatId = newChat("Plan tools: invalid ops"); - const tools = planTools({ chatId, chatStore }); - await exec(tools, "plan_init", { nodes: [{ id: "a", title: "A" }] }); - const before = chatStore.getChat(chatId)!.tree!; - - const dup = await exec(tools, "plan_add_node", { id: "a", title: "Impostor" }); - assert.equal(dup.ok, false); - assert.match(dup.error!, /already exists/); - assert.deepEqual(dup.tree, before); - - const missing = await exec(tools, "plan_set_current", { id: "ghost" }); - assert.equal(missing.ok, false); - assert.match(missing.error!, /unknown node id "ghost"|not in the tree/); - - const badEdge = await exec(tools, "plan_add_edge", { from: "a", to: "a" }); - assert.equal(badEdge.ok, false); - assert.match(badEdge.error!, /self-loop/); - - assert.deepEqual(chatStore.getChat(chatId)!.tree!, before, "record untouched by failed ops"); - }); - - it("mutation tools work on a fresh (uninitialized) chat: the tree seeds from empty", async () => { - const chatId = newChat("Plan tools: no init"); - const tools = planTools({ chatId, chatStore }); - const res = await exec(tools, "plan_add_node", { id: "a", title: "A" }); - assert.equal(res.ok, true); - assert.equal(res.tree.nodes.length, 1); - assert.ok(chatStore.getChat(chatId)!.tree, "tree seeded without plan_init"); + it("only plan_init exists — the seven mutation/tracking tools are gone by design", () => { + const tools = planTools({ chatId: "chat_x", chatStore }); + assert.deepEqual(Object.keys(tools), ["plan_init"]); }); it("unknown chat returns ok:false with the empty tree", async () => { const tools = planTools({ chatId: "chat_nonexistent", chatStore }); - const res = await exec(tools, "plan_mark_done", { id: "a" }); + const res = await exec(tools, "plan_init", { mermaid: "graph TD\n a[A]\n" }); assert.equal(res.ok, false); assert.match(res.error!, /Chat not found/); assert.deepEqual(res.tree, emptyTree()); }); }); + +// ─── the mermaid subset parser ──────────────────────────────────────────────── + +describe("parseMermaidPlan (the constrained mermaid subset)", () => { + const parse = (m: string) => parseMermaidPlan(m); + + it("accepts graph TD and flowchart TB headers (case-insensitive), comments, and blank lines", () => { + for (const header of ["graph TD", "graph td", "flowchart TB", "Flowchart td"]) { + const res = parse(`${header}\n%% a comment\n\n a[Alpha]\n`); + assert.equal(res.ok, true, header); + assert.deepEqual(res.plan!.nodes, [{ id: "a", title: "Alpha" }]); + } + }); + + it("bare ids fall back to id-as-title; bracket and paren labels both work", () => { + const res = parse("graph TD\n bare\n bracket[Bracket title]\n paren(\"Paren title\")\n"); + assert.deepEqual(res.plan!.nodes, [ + { id: "bare", title: "bare" }, + { id: "bracket", title: "Bracket title" }, + { id: "paren", title: "Paren title" }, + ]); + }); + + it("edges register bare endpoints as nodes and dedupe duplicate edges", () => { + const res = parse("graph TD\n a --> b[Bee]\n a --> b[Bee]\n"); + assert.deepEqual(res.plan!.nodes, [ + { id: "a", title: "a" }, + { id: "b", title: "Bee" }, + ]); + assert.deepEqual(res.plan!.edges, [{ from: "a", to: "b" }]); + }); + + it("labels on both edge endpoints survive; forward references are fine", () => { + const res = parse("graph TD\n downstream[Later thing] --> upstream[Earlier thing]\n"); + assert.deepEqual(res.plan!.edges, [{ from: "downstream", to: "upstream" }]); + assert.deepEqual(res.plan!.nodes, [ + { id: "downstream", title: "Later thing" }, + { id: "upstream", title: "Earlier thing" }, + ]); + }); + + it("rejects: non-slug ids, self-loops, chained edges, subgraphs, missing header, empty", () => { + assert.ok(!parse("graph TD\n Foo Bar[x]\n").ok, "id with a space"); + assert.match(parse("graph TD\n Foo Bar[x]\n").problems[0]!, /line 2.*slug/); + assert.ok(!parse("graph TD\n a --> a\n").ok, "self-loop"); + assert.ok(!parse("graph TD\n a --> b --> c\n").ok, "chained edge"); + assert.match(parse("graph TD\n a --> b --> c\n").problems[0]!, /one edge per line/); + assert.ok(!parse("graph TD\n subgraph cluster\n").ok, "subgraph"); + assert.ok(!parse("digraph G\n a\n").ok, "wrong header"); + assert.ok(!parse("%% only a comment\n").ok, "empty plan"); + }); + + it("the first label wins when a node line repeats (relabeling mid-block is ignored)", () => { + const res = parse("graph TD\n a[First label]\n a[Second label]\n"); + assert.deepEqual(res.plan!.nodes, [{ id: "a", title: "First label" }]); + }); +}); + +// ─── derived statuses (the tracking the teacher no longer does) ─────────────── + +describe("derivePlanStatuses (statuses derive from checks, never declarations)", () => { + const tree: PlanTree = { + nodes: [ + { id: "a", title: "A" }, + { id: "b", title: "B" }, + { id: "c", title: "C" }, + ], + edges: [ + { from: "a", to: "b" }, + { from: "b", to: "c" }, + ], + currentNodeId: null, + doneNodeIds: [], + version: 1, + }; + const quizPart = (state: string, nodeId: string | undefined, correct?: boolean) => ({ + type: "tool-quiz", + state, + input: nodeId === undefined ? {} : { nodeId }, + output: correct === undefined ? undefined : { correct }, + }); + type QuizPart = { type: string; state: string; input: unknown; output?: unknown }; + const msg = (...parts: QuizPart[]) => ({ parts }); + + it("empty transcript → current = first node (authoring order, no prereqs), nothing done", () => { + const st = derivePlanStatuses(tree, []); + assert.equal(st.current, "a"); + assert.equal(st.done.size, 0); + }); + + it("a passed check on a tags it done; current advances to the next topologically-ready node", () => { + const st = derivePlanStatuses(tree, [msg(quizPart("output-available", "a", true))]); + assert.deepEqual([...st.done], ["a"]); + assert.equal(st.current, "b"); + }); + + it("a pending check OWNS current, wherever the topo frontier is", () => { + const st = derivePlanStatuses(tree, [ + msg(quizPart("output-available", "a", true)), + msg(quizPart("input-available", "c")), // a jump check on the sink + ]); + assert.equal(st.current, "c", "the open check is where we are"); + assert.deepEqual([...st.done], ["a"]); + }); + + it("a FAILED check marks nothing done (explaining is yours, done is theirs)", () => { + const st = derivePlanStatuses(tree, [msg(quizPart("output-available", "a", false))]); + assert.equal(st.done.size, 0); + assert.equal(st.current, "a", "frontier unmoved — resume teaching"); + }); + + it("untagged checks and unknown nodeIds are lost signal — ignored", () => { + const st = derivePlanStatuses(tree, [ + msg(quizPart("output-available", undefined, true)), + msg(quizPart("output-available", "ghost", true)), + ]); + assert.equal(st.done.size, 0); + assert.equal(st.current, "a"); + }); + + it("prereqs gate the frontier: c isn't next until b is demonstrated", () => { + const st = derivePlanStatuses(tree, [msg(quizPart("output-available", "a", true))]); + assert.equal(st.current, "b", "c requires b"); + const st2 = derivePlanStatuses(tree, [ + msg(quizPart("output-available", "a", true)), + msg(quizPart("output-available", "b", true)), + ]); + assert.equal(st2.current, "c"); + assert.equal(derivedNodeStatus(st2, "a"), "done"); + assert.equal(derivedNodeStatus(st2, "c"), "current"); + assert.equal(derivedNodeStatus(st2, "b"), "done"); + }); + + it("a rewritten map keeps honest progress for reused slugs (derivation matches the CURRENT tree)", () => { + const rewritten: PlanTree = { ...tree, nodes: [tree.nodes[0]!, tree.nodes[1]!] }; + const st = derivePlanStatuses(rewritten, [msg(quizPart("output-available", "a", true))]); + assert.equal(st.current, "b"); + const orphaned = derivePlanStatuses(rewritten, [msg(quizPart("output-available", "c", true))]); + assert.equal(orphaned.done.size, 0, "a slug removed from the rewrite stops counting"); + }); +}); diff --git a/tests/question-tools.test.ts b/tests/question-tools.test.ts new file mode 100644 index 0000000..27a1183 --- /dev/null +++ b/tests/question-tools.test.ts @@ -0,0 +1,148 @@ +/** + * questionToolsFor — the per-session question tools with dynamic plan-nodeId + * validation (tools.ts). The grading contract is question-core's and is + * UNTOUCHED; the For variant chains one session-relative check on top: + * a quiz whose nodeId doesn't match a node of the CURRENT plan tree is + * rejected at the call boundary (never reaches the learner — the SDK + * returns the validation error to the teacher as a tool result, and it + * retries). + */ +import assert from "node:assert/strict"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { after, describe, it } from "node:test"; +import { questionTools, questionToolsFor } from "../src/agent/tools.ts"; +import { chatStore } from "../src/store/chats.ts"; +import { PATHS } from "../config.ts"; + +const dataDir = path.join(PATHS.root, "data"); +fs.mkdirSync(dataDir, { recursive: true }); + +const createdChats: string[] = []; +after(() => { + for (const id of createdChats) { + try { + fs.unlinkSync(path.join(PATHS.chats, `${id}.json`)); + } catch { + /* scratch cleanup, best effort */ + } + } +}); + +function newChat(title: string): string { + const chat = chatStore.createChat(title); + createdChats.push(chat.id); + return chat.id; +} + +const validQuiz = { + question: "What is 2 + 2?", + options: [ + { label: "3" }, + { label: "4" }, + ], + correctAnswer: "4", + explanation: "Arithmetic.", +}; + +interface QuizToolSpec { + inputSchema: { safeParse: (v: unknown) => { success: boolean; error?: { issues: Array<{ message: string }> } } }; +} +const quizSchemaOf = (tools: Record) => (tools.quiz as QuizToolSpec).inputSchema; + +describe("questionToolsFor: the dynamic plan-nodeId validation", () => { + it("accepts a nodeId that matches a node of the session's tree", () => { + const chatId = newChat("NodeId: valid"); + chatStore.saveTree(chatId, { + nodes: [ + { id: "arith", title: "Arithmetic" }, + { id: "expressions", title: "Expressions" }, + ], + edges: [{ from: "arith", to: "expressions" }], + currentNodeId: null, + doneNodeIds: [], + version: 1, + }); + const res = quizSchemaOf(questionToolsFor({ chatId, chatStore })).safeParse({ + ...validQuiz, + nodeId: "expressions", + }); + assert.equal(res.success, true); + }); + + it("rejects a nodeId that matches no node — with a message listing the slugs", () => { + const chatId = newChat("NodeId: typo"); + chatStore.saveTree(chatId, { + nodes: [{ id: "arith", title: "Arithmetic" }], + edges: [], + currentNodeId: null, + doneNodeIds: [], + version: 1, + }); + const res = quizSchemaOf(questionToolsFor({ chatId, chatStore })).safeParse({ + ...validQuiz, + nodeId: "arithmetic", // the typo — slug is 'arith' + }); + assert.equal(res.success, false); + const msg = res.error!.issues.map((i) => i.message).join("\n"); + assert.match(msg, /matches no node/); + assert.match(msg, /arith/); + }); + + it("no tree yet (probe phase): ANY nodeId is rejected — ad-hoc checks must omit it", () => { + const chatId = newChat("NodeId: probe"); + const res = quizSchemaOf(questionToolsFor({ chatId, chatStore })).safeParse({ + ...validQuiz, + nodeId: "anything", + }); + assert.equal(res.success, false); + assert.match(res.error!.issues.at(-1)!.message, /No plan map exists yet/); + // …while an untagged probe check is fine + const plain = quizSchemaOf(questionToolsFor({ chatId, chatStore })).safeParse(validQuiz); + assert.equal(plain.success, true); + }); + + it("reads the LIVE record — a plan_init mutation mid-turn is honored immediately", () => { + const chatId = newChat("NodeId: live record"); + const tools = questionToolsFor({ chatId, chatStore }); + const schema = quizSchemaOf(tools); + + const before = schema.safeParse({ ...validQuiz, nodeId: "fresh-node" }); + assert.equal(before.success, false, "no tree yet → rejected"); + + chatStore.saveTree(chatId, { + nodes: [{ id: "fresh-node", title: "Just initialized" }], + edges: [], + currentNodeId: null, + doneNodeIds: [], + version: 1, + }); + const after = schema.safeParse({ ...validQuiz, nodeId: "fresh-node" }); + assert.equal(after.success, true, "the same id validates once the tree exists — same turn"); + }); + + it("the shared grading rules still chain FIRST (schema-valid ⇔ gradeable)", () => { + const chatId = newChat("NodeId: grading rules intact"); + const res = quizSchemaOf(questionToolsFor({ chatId, chatStore })).safeParse({ + ...validQuiz, + correctAnswer: "42", // matches no option — question-core's rule + nodeId: "ghost", // also bad — BOTH problems surface + }); + assert.equal(res.success, false); + const msgs = res.error!.issues.map((i) => i.message).join("\n"); + assert.match(msgs, /correctAnswer/); + assert.match(msgs, /matches no node/); + }); + + it("the STATIC questionTools stay validator-less (the teacher-agent default path)", () => { + const res = quizSchemaOf(questionTools).safeParse({ ...validQuiz, nodeId: "anything" }); + assert.equal(res.success, true, "no session context → no nodeId check"); + }); + + it("ask_user_question is untouched (never carries nodeId — it isn't a check)", () => { + const chatId = newChat("NodeId: ask untouched"); + const ask = questionToolsFor({ chatId, chatStore }).ask_user_question as QuizToolSpec; + const res = ask.inputSchema.safeParse({ question: "Free-form?", options: [{ label: "A" }, { label: "B" }] }); + assert.equal(res.success, true); + }); +}); From e33b83ca2d5be3a151123289f6ffa39cd7777aa5 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 9 Sep 2026 03:04:21 +0300 Subject: [PATCH 3/6] =?UTF-8?q?fix:=20steer-delivery=20starvation=20?= =?UTF-8?q?=E2=80=94=20continuation=20coalescing=20(claimSteers)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bug (seen live): a teacher that ends every turn with a new pending quiz keeps the wake pipeline's idle gates permanently closed — the steer sits in 'delivering result' forever while the learner answers check after check. - claimSteers (server/jobs.ts): the chat-handler claims every queued, undelivered steer the instant a client turn wins the turn registry, and appends it after the learner's answer — conversion-safe, the answered quiz carries its output. Every answer flushes pending steers immediately. - The pump keeps its post-idle duplicate-guard (re-check steerDelivered before firing), so a claim racing the pump converges to exactly one delivery either way. - The stale-job sweep now skips chats with a turn in flight (a healthy but slow wake no longer gets falsely re-queued as 'recovering'). - Out-of-lane surgical touch (flagged for master review): the 3-line handleChatRequest seam (claim after beginTurn, persist+stream 'outgoing'). - Verified live end-to-end on the stuck session: answering the pending quiz fired the steer + wake turn within a second; job flipped done/delivered. --- src/core/contracts/jobs.ts | 11 +++++ src/server/chat-handler.ts | 16 +++++-- src/server/jobs.ts | 49 ++++++++++++++++++++ tests/jobs.test.ts | 93 +++++++++++++++++++++++++++++++++++++- 4 files changed, 165 insertions(+), 4 deletions(-) diff --git a/src/core/contracts/jobs.ts b/src/core/contracts/jobs.ts index 5c1127c..31c2892 100644 --- a/src/core/contracts/jobs.ts +++ b/src/core/contracts/jobs.ts @@ -73,6 +73,17 @@ export interface JobRecord { * a polling client that sees the flip re-syncs once and gets the * finished record. * + * CONTINUATION COALESCING: the idle window above can starve — a + * quiz-chaining teacher ends every turn with a new pending check, so + * the gates never stay open long enough for a wake to fire. The fix: + * when a client turn wins the turn registry, the handler CLAIMS every + * queued, undelivered steer (jobs.ts claimSteers) and appends it after + * the learner's answer in that very turn — conversion-safe, because the + * answered quiz carries its output. Every answer flushes pending + * steers immediately; the wake pump remains for turns that end with no + * pending question. The pump re-checks steerDelivered right before + * firing, so a claim racing the pump converges to exactly one delivery. + * * 5. No pending job parts ever exist, so the composer never gates on jobs * (the learner can interject mid-job; the steer queues behind their * turn) and job tool outputs never trigger the client-side diff --git a/src/server/chat-handler.ts b/src/server/chat-handler.ts index 54f1b4a..8f7fa69 100644 --- a/src/server/chat-handler.ts +++ b/src/server/chat-handler.ts @@ -3,7 +3,7 @@ import { createTeacherAgent, type TeacherAgent } from "../agent/teacher-agent.ts import { createJobRunner } from "../agent/subagents/runner.ts"; import { createExtractionPipeline } from "../agent/subagents/extraction.ts"; import { createTeacherTools } from "../agent/tools.ts"; -import { deliverJobResult, setJobRespawn, setTurnRunner } from "./jobs.ts"; +import { claimSteers, deliverJobResult, setJobRespawn, setTurnRunner } from "./jobs.ts"; import { stores } from "./stores.ts"; import { beginTurn, endTurn } from "./turn-liveness.ts"; import { buildTeacherContext, persistTurn, triggerExtraction } from "./turn-service.ts"; @@ -71,6 +71,16 @@ export async function handleChatRequest(req: Request, agent?: TeacherAgent): Pro return Response.json({ error: "A turn is already running for this session" }, { status: 409 }); } + // CONTINUATION COALESCING (async protocol §4, the delivery-starvation fix): + // steers finished while a quiz chain held the wake gates ride THIS turn — + // appended after the learner's answer (conversion-safe: the answer carries + // its output). Claiming happens after beginTurn, so this turn owns the + // registry and the write-ahead persist below makes the claim durable. + const steers = claimSteers(chatId); + const outgoing = ( + steers.length > 0 ? [...(body.messages as UIMessage[]), ...steers] : body.messages + ) as UIMessage[]; + // Write-ahead persistence: the request body IS the client's canonical // state, so it goes on disk BEFORE the stream starts. The learner's answer // is durable the instant the request arrives — any remount, second tab, or @@ -78,7 +88,7 @@ export async function handleChatRequest(req: Request, agent?: TeacherAgent): Pro // returns mid-turn reads turnInFlight from the registry and re-attaches by // polling instead of resurrecting the stale transcript.) try { - stores.chatStore.saveMessages(chatId, body.messages as UIMessage[]); + stores.chatStore.saveMessages(chatId, outgoing); } catch (err) { console.error(`[chat ${chatId}] write-ahead persist failed:`, err); } @@ -97,7 +107,7 @@ export async function handleChatRequest(req: Request, agent?: TeacherAgent): Pro }), buildTeacherContext(stores.graphStore, chatId), ), - uiMessages: body.messages as UIMessage[], + uiMessages: outgoing, // Tool-call validation failures are messages OUR schemas authored (the // quiz authoring contract, unknown tools) — safe and useful in the // transcript: they are exactly what the teacher saw and retried on. The diff --git a/src/server/jobs.ts b/src/server/jobs.ts index 160fb60..780d6c5 100644 --- a/src/server/jobs.ts +++ b/src/server/jobs.ts @@ -128,6 +128,45 @@ export function deliverJobResult(steer: SubagentSteerMessage): Promise { }); } +/** + * CONTINUATION COALESCING ("THE ASYNC PROTOCOL" §4): the chat-handler calls + * this the instant a client turn wins the turn registry, BEFORE its + * write-ahead persist. Undelivered steers for the chat ride THAT turn — + * appended after the learner's answer, which is conversion-safe (the + * answered quiz carries its output) — instead of waiting for the pump's + * idle window, which a quiz-chaining teacher may never open (the delivery + * starvation bug: every turn ends with a new pending quiz, so the gates + * never stay open long enough to fire a wake). + * + * Claimed steers resolve their deliver() promise and flip the job done + * here: the caller holds the turn registry, so the write-ahead persist is + * imminent and durable. Steers already in the transcript (a wake raced in) + * are finalized, not returned. The pump re-checks steerDelivered right + * before firing, so a claim racing the pump converges to exactly one + * delivery either way. + */ +export function claimSteers(chatId: string): SubagentSteerMessage[] { + const s = state(); + const queue = s.queues.get(chatId); + if (!queue || queue.length === 0) return []; + const record = stores.chatStore.getChat(chatId); + if (!record) return []; // the handler validated the chat; defensive only + const claimed: SubagentSteerMessage[] = []; + for (const entry of queue) { + const jobId = jobIdOf(entry.steer); + if (steerDelivered(record.messages, jobId)) { + // A wake already landed it — just finish the bookkeeping. + finalizeJob(jobId); + } else { + claimed.push(entry.steer); + finalizeJob(jobId); // delivery is now the client turn's job + } + entry.resolve(); + } + s.queues.delete(chatId); + return claimed; +} + function chatIdOf(steer: SubagentSteerMessage): string { // The chat is resolvable from the job record; the steer's metadata // carries jobId, which indexes the store. @@ -194,6 +233,15 @@ async function pump(chatId: string): Promise { continue; } + // A client continuation may have COALESCED this steer (claimSteers) while + // the gates were closed — re-check before firing: exactly one delivery. + if (steerDelivered(fresh.messages, jobId)) { + queue.shift(); + finalizeJob(jobId); + entry.resolve(); + continue; + } + // Fire the server-side teacher turn. A 409 = a client turn raced // in between idle and firing — back off and retry the same steer. const status = await s.turnRunner(chatId, [...fresh.messages, entry.steer]).catch((err: unknown) => { @@ -288,6 +336,7 @@ function recoverStaleJobs(): void { if (job.status !== "running") continue; if (now - Date.parse(job.updatedAt) < STALE_JOB_MS) continue; if (!stores.chatStore.getChat(job.chatId)) continue; + if (isTurnRunning(job.chatId)) continue; // a wake turn may be delivering it right now if (job.result) { // The wake never fired (or crashed mid-flight): re-deliver. const steer = buildSubagentSteer(job, job.result as never); diff --git a/tests/jobs.test.ts b/tests/jobs.test.ts index 2b4712e..ba0c107 100644 --- a/tests/jobs.test.ts +++ b/tests/jobs.test.ts @@ -41,7 +41,7 @@ import { import { composeInstructions, createTeacherAgent } from "../src/agent/teacher-agent.ts"; import { createTeacherTools } from "../src/agent/tools.ts"; import { handleChatRequest } from "../src/server/chat-handler.ts"; -import { deliverJobResult, setTurnRunner } from "../src/server/jobs.ts"; +import { claimSteers, deliverJobResult, setTurnRunner } from "../src/server/jobs.ts"; import { persistTurn, withMessageIds } from "../src/server/turn-service.ts"; import { chatStore } from "../src/store/chats.ts"; import { jobStore } from "../src/store/jobs.ts"; @@ -641,6 +641,97 @@ describe("wake pipeline (src/server/jobs.ts)", () => { assert.equal(jobStore.getJob(jobId)!.status, "done", "the job still flips — recovery completes"); }); +// ─── continuation coalescing (the delivery-starvation fix) ─────────────────── + +describe("claimSteers + pump duplicate-guard (continuation coalescing)", () => { + it("claims queued undelivered steers: resolves deliver(), flips the job done, empties the queue", async () => { + const { chatId, jobId, steer } = setup(); + let fired = 0; + setTurnRunner(async () => { + fired++; + return 200; + }); + + const delivery = deliverJobResult(steer); + await delay(300); // the pump is inside waitForIdle's grace (750ms) + const claimed = claimSteers(chatId); + + assert.equal(claimed.length, 1, "the queued steer is claimed"); + assert.equal(claimed[0]!.id, steer.id); + // The handler's write-ahead persist lands the claim immediately after + // claiming (the very next statement in handleChatRequest). + const record = chatStore.getChat(chatId)!; + chatStore.saveMessages(chatId, [...record.messages, ...claimed] as never); + await delivery; // the runner's deliver() promise resolves on claim + assert.equal(jobStore.getJob(jobId)!.status, "done", "job flips done at claim"); + assert.deepEqual(claimSteers(chatId), [], "queue is empty after the claim"); + await delay(800); + assert.equal(fired, 0, "the pump never fires a wake for a claimed steer"); + }); + + it("finalizes already-delivered steers without returning them (a wake raced in)", async () => { + const { chatId, jobId, steer } = setup(); + const record = chatStore.getChat(chatId)!; + chatStore.saveMessages(chatId, [...record.messages, steer] as never); + + const delivery = deliverJobResult(steer); + await delay(100); // let the pump see it first (its top-of-loop idempotent skip) + const claimed = claimSteers(chatId); + assert.equal(claimed.length, 0, "delivered steers are not re-claimed"); + assert.equal(jobStore.getJob(jobId)!.status, "done"); + await delivery; + }); + + it("claimSteers on a chat with no queue returns [] and touches nothing", () => { + assert.deepEqual(claimSteers("chat_never_existed"), []); + }); + + it("pump duplicate-guard: a steer coalesced into the record while the gates were closed is never double-fired", async () => { + const { chatId, jobId, steer } = setup(); + let fired = 0; + setTurnRunner(async () => { + fired++; + return 200; + }); + + // Gates closed: a pending quiz parks the pump in waitForIdle. + chatStore.saveMessages(chatId, [ + { id: "u1", role: "user", parts: [{ type: "text", text: "hello" }] }, + { + id: "a1", + role: "assistant", + parts: [{ type: "tool-quiz", toolCallId: "quiz-call", state: "input-available", input: { question: "q", choices: ["a", "b"] } }], + }, + ] as never); + const delivery = deliverJobResult(steer); + await delay(1200); + assert.equal(fired, 0, "parked behind the pending quiz"); + + // The client turn coalesces: it claims the steer and lands it in the record. + const claimed = claimSteers(chatId); + assert.equal(claimed.length, 1, "coalescing claims the parked steer"); + const record = chatStore.getChat(chatId)!; + chatStore.saveMessages(chatId, [...record.messages, steer] as never); // write-ahead persist + + // The learner answers: the gates open — the parked pump must see the + // delivered steer via its post-idle re-check, NOT fire a second turn. + const answered = chatStore.getChat(chatId)!.messages.map((m) => + m.id === "a1" + ? { + ...m, + parts: [{ type: "tool-quiz", toolCallId: "quiz-call", state: "output-available", input: { question: "q", choices: ["a", "b"] }, output: { answer: "a", correct: true, explanation: "" } }], + } + : m, + ); + chatStore.saveMessages(chatId, answered as never); + + await delivery; + await delay(1500); + assert.equal(fired, 0, "no duplicate wake after coalescing"); + assert.equal(jobStore.getJob(jobId)!.status, "done"); + }); +}); + it("fails the job (error status) when the wake turn errors unrecoverably", async () => { const { jobId, steer } = setup(); setTurnRunner(async () => 500); From 91ae2e137ddb2d40876d31d9297a22a5e84cb23c Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 9 Sep 2026 05:43:28 +0300 Subject: [PATCH 4/6] fix: dedupe same-id continuation copies in persistTurn (coalescing follow-up) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With a steer coalesced into a quiz-answer continuation, the steer sits AFTER the assistant message being continued, so the SDK's onFinish list carries the input copy AND the extended response copy as two entries with the SAME id — persistTurn saved both, the jobs poller's syncFromServer imported the dup, and React keyed a single message twice (the user's live test hit it). withMessageIds now dedupes by id keeping the LAST occurrence — the SDK's final merge state (a superset of the earlier copy) — which also yields the right semantic shape: [user, steer, assistant(extended)]. Out-of-lane surgical touch (flagged for master review): turn-service.ts. Squashed the live session's already-corrupted record by hand (same dedupe). --- src/server/turn-service.ts | 16 +++++++++++++++- tests/jobs.test.ts | 27 +++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/server/turn-service.ts b/src/server/turn-service.ts index 39d8728..e999669 100644 --- a/src/server/turn-service.ts +++ b/src/server/turn-service.ts @@ -55,9 +55,23 @@ export function buildTeacherContext(graphStore: GraphStore, chatId: string): str * NEVER touched (the client's next POST still carries them verbatim). */ export function withMessageIds(messages: UIMessage[]): UIMessage[] { - return messages.map((m) => + const filled = messages.map((m) => m.id ? m : { ...m, id: `msg_${Date.now().toString(36)}_${crypto.randomUUID().slice(0, 8)}` }, ); + // Same id twice = the SDK's continuation merge couldn't happen in place + // (continuation coalescing appends a user-role steer AFTER the assistant + // message being continued, so onFinish carries the input copy AND the + // extended response copy under one id). Keep the LAST occurrence — it is + // the SDK's final merge state (a superset of the earlier copy). + const seen = new Set(); + const deduped: UIMessage[] = []; + for (let i = filled.length - 1; i >= 0; i--) { + const m = filled[i]!; + if (seen.has(m.id)) continue; + seen.add(m.id); + deduped.unshift(m); + } + return deduped; } export function persistTurn(chatStore: ChatStore, chatId: string, messages: UIMessage[], isAborted: boolean): void { diff --git a/tests/jobs.test.ts b/tests/jobs.test.ts index ba0c107..b2382a1 100644 --- a/tests/jobs.test.ts +++ b/tests/jobs.test.ts @@ -764,6 +764,33 @@ describe("persistTurn id hygiene", () => { assert.notEqual(filled[0]!.id, filled[1]!.id); }); + it("persistTurn dedupes same-id continuation copies, keeping the LAST (the SDK's merged superset)", () => { + // The coalescing shape: a steer (user-role) rides a quiz-answer + // continuation AFTER the assistant message being continued, so the + // SDK's onFinish list carries the input copy AND the extended + // response copy under one id — the record must keep only the merge. + const chat = chatStore.createChat("Test: dedupe continuation copies"); + trackChat(chat.id); + const short = { id: "msg_dup_1", role: "assistant", parts: [{ type: "text", text: "step 1" }] }; + const long = { id: "msg_dup_1", role: "assistant", parts: [{ type: "text", text: "step 1" }, { type: "text", text: "step 2 (continued)" }] }; + persistTurn(chatStore, chat.id, [ + { id: "u1", role: "user", parts: [{ type: "text", text: "hi" }] }, + short, + { id: "steer_x", role: "user", parts: [{ type: "text", text: "research complete" }] }, + long, + ], false); + const record = chatStore.getChat(chat.id)!; + assert.equal(record.messages.length, 3, "the duplicate entry is dropped"); + assert.deepEqual( + record.messages.map((m) => m.id), + ["u1", "steer_x", "msg_dup_1"], + "order preserved, steer before the merged assistant copy", + ); + const merged = record.messages[2]!; + assert.equal(merged.parts.length, 2, "the KEPT copy is the extended one"); + assert.equal((merged.parts[1] as { text: string }).text, "step 2 (continued)"); + }); + it("persistTurn saves every message with a unique non-empty id (wake turns never re-POSTed)", () => { const chat = chatStore.createChat("Test: persist ids"); trackChat(chat.id); From 2535d7ad35c8fe6a000abb0d70089a20721961cf Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 9 Sep 2026 05:54:04 +0300 Subject: [PATCH 5/6] fix: defer the jobs settle-sync until the turn is over (coalescing follow-up 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With continuation coalescing, a job flips done at CLAIM time — while the learner's own answer turn is still streaming. The poller's settle re-sync fired on the flip, swapping in a mid-turn record snapshot mid-stream; the steer after the assistant message broke the SDK's in-place continuation merge, and the streaming response landed as a SECOND message with the same id — duplicate React keys in the live transcript (the user's live test hit it; the record itself stayed clean). A settle during a busy turn (local stream OR server turnInFlight) now marks a pending sync; the first idle poll tick adopts the finished record. This also fixes the latent multi-job race: steer B settling while steer A's wake turn is in flight would have grabbed the same mid-turn snapshot. Out-of-lane surgical touch (flagged for master review): the wave-1 poller block in components/chat.tsx. --- components/chat.tsx | 35 +++++++++++++++++++++++++++++------ tests/jobs.test.ts | 5 +++-- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/components/chat.tsx b/components/chat.tsx index 6f089b8..3ec736f 100644 --- a/components/chat.tsx +++ b/components/chat.tsx @@ -178,11 +178,15 @@ export function Chat({ // ── Central job observability (contracts.ts "THE ASYNC PROTOCOL" §4) ── // ONE poll of /api/jobs drives every job surface (transcript dispatch // cards, the sub-agents panel) and re-syncs the transcript exactly once - // when a job settles: job status flips done/error only AFTER the wake - // turn's messages are persisted, so a completion means the record already - // holds the steer + the teacher's response — one syncFromServer adopts it - // all. Poll fast while jobs run, slow otherwise (new-dispatch discovery), - // and not at all when this session has never dispatched a job. + // when a job settles. A settle no longer implies the record is complete: + // CONTINUATION COALESCING flips the job done at claim time — while the + // learner's own answer turn is still streaming (and a settle can race a + // second job's wake turn). Syncing mid-turn would swap in a stale snapshot + // mid-stream, which breaks the SDK's in-place continuation merge (duplicate + // message ids). So: a settle during a busy turn defers the sync; the next + // idle poll tick adopts the finished record. Poll fast while jobs run, + // slow otherwise (new-dispatch discovery), and not at all when this + // session has never dispatched a job. const [jobs, setJobs] = useState([]); const jobPartCount = useMemo( () => @@ -201,6 +205,8 @@ export function Chat({ const localStreamRef = useRef(false); localStreamRef.current = localStream; const jobStatusRef = useRef>(new Map()); + // A settle seen during a busy turn — the sync is owed, not dropped. + const pendingSyncRef = useRef(false); useEffect(() => { if (pollMs === 0) return; let cancelled = false; @@ -214,9 +220,26 @@ export function Chat({ const list = data.jobs ?? []; for (const j of list) { const prev = jobStatusRef.current.get(j.id); - if (prev === "running" && j.status !== "running") void syncFromServer(); + if (prev === "running" && j.status !== "running") { + // The record is only guaranteed complete when no turn is in + // flight — locally or server-side. A coalesced settle fires + // mid-turn by design; adopt the record on the first idle tick. + if (localStreamRef.current || data.turnInFlight === true) { + pendingSyncRef.current = true; + } else { + void syncFromServer(); + } + } jobStatusRef.current.set(j.id, j.status); } + if ( + pendingSyncRef.current && + !localStreamRef.current && + data.turnInFlight !== true + ) { + pendingSyncRef.current = false; + void syncFromServer(); + } setJobs(list); // Wake-turn adoption: a server-side teacher turn with no client request // of ours (sub-agent delivery) — the same machinery as returning diff --git a/tests/jobs.test.ts b/tests/jobs.test.ts index b2382a1..49590da 100644 --- a/tests/jobs.test.ts +++ b/tests/jobs.test.ts @@ -27,6 +27,7 @@ import assert from "node:assert/strict"; import * as fs from "node:fs"; import * as path from "node:path"; import { after, describe, it } from "node:test"; +import type { UIMessage } from "ai"; import { MockLanguageModelV4, simulateReadableStream } from "ai/test"; import type { LanguageModelV4StreamPart } from "@ai-sdk/provider"; import { @@ -771,8 +772,8 @@ describe("persistTurn id hygiene", () => { // response copy under one id — the record must keep only the merge. const chat = chatStore.createChat("Test: dedupe continuation copies"); trackChat(chat.id); - const short = { id: "msg_dup_1", role: "assistant", parts: [{ type: "text", text: "step 1" }] }; - const long = { id: "msg_dup_1", role: "assistant", parts: [{ type: "text", text: "step 1" }, { type: "text", text: "step 2 (continued)" }] }; + const short: UIMessage = { id: "msg_dup_1", role: "assistant", parts: [{ type: "text", text: "step 1" }] }; + const long: UIMessage = { id: "msg_dup_1", role: "assistant", parts: [{ type: "text", text: "step 1" }, { type: "text", text: "step 2 (continued)" }] }; persistTurn(chatStore, chat.id, [ { id: "u1", role: "user", parts: [{ type: "text", text: "hi" }] }, short, From be4d6d8de82b44773a606f5fdafbdf939dce752e Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Wed, 9 Sep 2026 06:46:22 +0300 Subject: [PATCH 6/6] config: reasoning effort minimal across teacher/researcher/demoBuilder/visualMaker --- config.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/config.ts b/config.ts index f6a1c5c..8e14d4d 100644 --- a/config.ts +++ b/config.ts @@ -33,16 +33,16 @@ export interface ModelSpec { export const MODELS = { /** The teacher — streaming, lesson-facing: smart, but keep it lively. */ - teacher: { model: "z-ai/glm-5.3-flash", effort: "medium" }, + teacher: { model: "z-ai/glm-5.3-flash", effort: "minimal" }, /** Background sub-agents: fast and cheap beats deep thought. */ - researcher: { model: "z-ai/glm-5.3-flash", effort: "low" }, - demoBuilder: { model: "z-ai/glm-5.3-flash", effort: "low" }, + researcher: { model: "z-ai/glm-5.3-flash", effort: "minimal" }, + demoBuilder: { model: "z-ai/glm-5.3-flash", effort: "minimal" }, /** Must be vision-capable: it looks at rendered SVGs. */ - visualMaker: { model: "qwen/qwen3.8-max", effort: "low" }, + visualMaker: { model: "qwen/qwen3.8-max", effort: "minimal" }, /** Extraction is judgment over given evidence — light thinking suffices. */ - extractor: { model: "z-ai/glm-5.3-flash", effort: "low" }, + extractor: { model: "z-ai/glm-5.3-flash", effort: "minimal" }, /** The auditor verifies evidence exists; keep it cheap. */ - auditor: { model: "z-ai/glm-5.3-flash", effort: "low" }, + auditor: { model: "z-ai/glm-5.3-flash", effort: "minimal" }, } satisfies Record; export type ModelRole = keyof typeof MODELS;