diff --git a/components/chat.tsx b/components/chat.tsx index e928386..69b0f38 100644 --- a/components/chat.tsx +++ b/components/chat.tsx @@ -233,37 +233,50 @@ export function Chat({ void syncTurns(); }, [syncTurns]); // After the learner's own turn ends the log has a fresh entry — and the - // persisted transcript may have grown server-authored parts the live - // stream can't carry (plan blocks attach at persist). Adopt the record - // the moment the server agrees the turn is done: endTurn flips ONLY - // after persist, so a not-in-flight record is a complete one — the same - // invariant the wake-adoption poll below rides. Poll briefly (the flip - // trails the client's stream-end by milliseconds), then adopt once. + // Post-turn adoption + the HANDOFF WATCH. Two things can trail our own + // stream's end: (1) the server's persist/endTurn — milliseconds, and the + // record is only complete once not-in-flight (plan blocks attach at + // persist); (2) a WAKE TURN — the probe→teacher handoff fires + // ~0.75–1.2s later (the pump's grace), and NO jobs poll is running to + // notice it: probe-phase sessions have zero job traffic, so the jobs + // poll is switched off and the open client never learns the teacher's + // turn exists (the freeze the user hit 2026-09-11 — the sidebar saw + // busy→idle while the session sat frozen). So: adopt at the first idle + // read as before, then keep watching briefly — if turnInFlight flips + // true, hand off to the wake-adoption poll below (busy shimmer, adopt + // at finish). const prevStatusRef = useRef(status); useEffect(() => { if (prevStatusRef.current !== "ready" && status === "ready") { // immediate: turns-only refetch — the stats bar updates even while // the server's persist/endTurn lags the client by milliseconds void syncTurns(); - // deferred: adopt the full record once the server agrees the turn - // is done (plan blocks and any other persist-authored parts) let cancelled = false; let timer: ReturnType | undefined; + let adopted = false; + const WATCH_ATTEMPTS = 8; // 8 × 500ms: persist lag + the ~1.2s handoff const adopt = async (attempt: number) => { if (cancelled) return; try { const res = await fetch(`/api/chats/${chatId}`); if (res.ok) { const record = (await res.json()) as { turnInFlight?: boolean }; - if (record.turnInFlight !== true) { - void syncFromServer(); + if (!adopted) { + // phase 1: wait out our own turn's persist lag, then adopt once + if (record.turnInFlight !== true) { + adopted = true; + void syncFromServer(); + } + } else if (record.turnInFlight === true) { + // phase 2 (the watch): a wake turn started — the handoff. + setTurnInFlight(true); // → the wake-adoption poll takes over return; } } } catch { /* transient — keep polling */ } - if (attempt < 8 && !cancelled) timer = setTimeout(() => void adopt(attempt + 1), 500); + if (attempt < WATCH_ATTEMPTS && !cancelled) timer = setTimeout(() => void adopt(attempt + 1), 500); }; void adopt(0); return () => { diff --git a/src/agent/prompts/teacher/10-planning.md b/src/agent/prompts/teacher/10-planning.md index 20539c1..75d9b98 100644 --- a/src/agent/prompts/teacher/10-planning.md +++ b/src/agent/prompts/teacher/10-planning.md @@ -27,7 +27,7 @@ Mermaid stays for ad-hoc lesson diagrams — the plan map is a different thing a 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. -- Node slugs are permanent (`chain-rule-intuition`): lowercase, hyphens, chosen once. Labels ≤ 6 words. +- Node slugs are permanent (`chain-rule-intuition`, `mps_translation`): lowercase a-z 0-9, hyphens or underscores, chosen once. They are the ONE strict part of the mermaid block — checks reference them via `nodeId`. Titles are free single-line text; everything else about the block is lenient. - 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. diff --git a/src/agent/tools/mermaid-plan.ts b/src/agent/tools/mermaid-plan.ts index 62956a4..5b1f884 100644 --- a/src/agent/tools/mermaid-plan.ts +++ b/src/agent/tools/mermaid-plan.ts @@ -1,25 +1,31 @@ /** - * Mermaid → PlanTree authoring format (the author-once protocol, 2026-09-03). + * Mermaid → PlanTree authoring format (the author-once protocol, 2026-09-03; + * free-form pass 2026-09-11 — the muscle-protein session post-mortem: four + * failed plan_init calls over ids the parser silently hated). * - * 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. + * The teacher writes the session map as a mermaid block — the format models + * are most fluent in — and plan_init canonicalizes it into the structured + * PlanTree. Hand-parsed subset: 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) + * THE ONE STRICT RULE: node ids are permanent slugs the teacher references + * via quiz nodeId — lowercase words (a-z, 0-9) joined by hyphens OR + * underscores (`chain-rule-intuition`, `mps_translation`). Everything else + * is as free-form as mermaid allows: + * graph|flowchart TD|TB|LR|RL|BT (direction is cosmetic — we layout + * ourselves; case-insensitive) * %% comment lines * node — bare id (title falls back to the id) - * id[title] — square-bracket label + * id[title] — square-bracket label; any single-line title, a + * surrounding "…" pair is stripped * 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. + * a --> b — requires edge (from = prerequisite); ==>, -.-> are + * synonyms; chains a --> b --> c expand to pairs + * a[title] --> b[title] — inline labels anywhere an endpoint appears + * Bare endpoints register as nodes; forward references are fine; duplicate + * edges dedupe; the first label a node gets wins. */ import type { PlanEdge, PlanNode } from "@/src/core"; @@ -35,13 +41,18 @@ export interface PlanParseResult { plan?: ParsedPlan; } -const HEADER_RE = /^(graph|flowchart)\s+(td|tb)\s*$/i; -const SLUG_RE = /^[a-z0-9]+(-[a-z0-9]+)*$/; +const HEADER_RE = /^(graph|flowchart)\s+(td|tb|lr|rl|bt)\s*$/i; +const SLUG_RE = /^[a-z0-9]+([_-][a-z0-9]+)*$/; +/** Any arrow spelling counts as a requires-edge; chains split on it. */ +const ARROW_RE = /-->|==>|-\.->/; /** 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*\)|\(([^()]*)\))?$/; +// The id CANDIDATE class is deliberately broad — anything but whitespace, +// brackets, parens — so EVERY bad id (UPPER, dots, underscores…) reaches +// register()'s SPECIFIC slug message instead of dying at the regex layer +// with a generic parse error. The muscle-protein post-mortem: underscores +// failed HERE, the actionable message never fired, and the teacher spent +// four attempts hunting punctuation the parser didn't care about. +const ENDPOINT_RE = /^([^\s()\[\]]+)\s*(?:\[([^\[\]]*)\]|\(\s*"([^"]*)"\s*\)|\(([^()]*)\))?$/; interface Endpoint { id: string; @@ -51,7 +62,15 @@ interface Endpoint { 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() }; + let label = (m[2] ?? m[3] ?? m[4] ?? "").trim(); + // `id["title"]` is idiomatic mermaid — don't keep the quote pair in the title. + if ( + label.length >= 2 && + ((label.startsWith('"') && label.endsWith('"')) || (label.startsWith("'") && label.endsWith("'"))) + ) { + label = label.slice(1, -1).trim(); + } + return { id: m[1], label }; } /** Parse the mermaid subset. Collects ALL problems (never just the first). */ @@ -76,7 +95,7 @@ export function parseMermaidPlan(mermaid: string): PlanParseResult { 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.`, + `line ${lineNo}: node id \`${id}\` must be a lowercase slug (a-z, 0-9, joined by hyphens or underscores — e.g. \`chain-rule-intuition\`); ids are permanent and checks reference them.`, ); return; } @@ -89,32 +108,44 @@ export function parseMermaidPlan(mermaid: string): PlanParseResult { 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; + // Edges (incl. chains): split on any arrow spelling; every segment is + // an endpoint, consecutive pairs become requires-edges. + const segments = line.split(ARROW_RE); + if (segments.length >= 2) { + const endpoints: Endpoint[] = []; + let unreadable = false; + for (const seg of segments) { + const ep = parseEndpoint(seg); + if (!ep) { + problems.push( + `line ${no}: can't read the edge \`${line}\` — expected \`a[title] --> b[title]\` (chains like \`a --> b --> c\` are fine).`, + ); + unreadable = true; + break; + } + endpoints.push(ep); } - 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; + if (!unreadable) { + for (const ep of endpoints) register(ep, no); + for (let k = 0; k + 1 < endpoints.length; k++) { + const from = endpoints[k]!; + const to = endpoints[k + 1]!; + 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 }); + } + } } - 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.`, + `line ${no}: can't parse \`${line}\` — ids are the strict part (lowercase slugs: a-z, 0-9, hyphens, underscores); node lines (\`slug[title]\`) and requires-edges (\`a --> b\`) are the accepted forms; subgraphs, styles, and classes are not part of the subset.`, ); continue; } diff --git a/src/agent/tools/plan.ts b/src/agent/tools/plan.ts index d92df06..36b7efd 100644 --- a/src/agent/tools/plan.ts +++ b/src/agent/tools/plan.ts @@ -17,12 +17,15 @@ export interface PlanToolContext { * WRITES the map and never tracks progress. One tool, plan_init, takes the * whole dependency map as a mermaid `graph TD` block: * - * - 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 + * - call it ONCE right after the probe concludes, to present the map (the + * LEARNER'S ACCEPTANCE of the presented plan, not this call, ends + * planning — docs/probe-teach-split.md §4) + * - statuses (current / visited / upcoming) are DERIVED from the quiz + * nodeId tags the teacher fires (contracts.ts derivePlanStatuses) — the + * teacher never declares them, and there is no "done" bookkeeping * - 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 + * wholesale. Node slugs are permanent — the map tracks nothing between + * rewrites * - 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 @@ -31,24 +34,26 @@ export interface PlanToolContext { * 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. + * latest state straight from the transcript's tool outputs (the plan + * window and the plan blocks). */ export function planTools(ctx: PlanToolContext): ToolSet { return { plan_init: tool({ description: - "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.", + "Write or REWRITE this session's plan map — the dependency graph the learner follows live in the plan window. " + + "Call once right after the probe concludes, to present the map; re-call whenever the map is wrong (the new block replaces the old tree). " + + "A mermaid graph block (any direction): one node per line (`slug[Short Title]` — slugs are permanent lowercase ids: a-z, 0-9, hyphens/underscores), " + + "`-->` edges meaning 'from must be understood before to' (chains fine). 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 tags.", inputSchema: z.object({ 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.", + "A mermaid graph block (any direction), e.g.:\ngraph TD\n primitives[Number primitives] --> operators[Arithmetic operators] --> expressions[Evaluating expressions]\n" + + "The only strict rule: ids are permanent lowercase slugs (a-z, 0-9, hyphens/underscores) — checks reference them via nodeId. " + + "Titles are free single-line text; `-->` means 'from = prerequisite' (chains fine); no subgraphs, styles, or classes.", ), }), execute: async ({ mermaid }): Promise => { diff --git a/tests/plan-tree.test.ts b/tests/plan-tree.test.ts index ad3827c..737c924 100644 --- a/tests/plan-tree.test.ts +++ b/tests/plan-tree.test.ts @@ -238,17 +238,64 @@ describe("parseMermaidPlan (the constrained mermaid subset)", () => { ]); }); - it("rejects: non-slug ids, self-loops, chained edges, subgraphs, missing header, empty", () => { + it("rejects: non-slug ids (with the SPECIFIC message), self-loops, subgraphs, missing header, empty", () => { + // The muscle-protein post-mortem (2026-09-11): underscore ids died at + // the regex layer with a generic message, and the teacher spent four + // plan_init calls hunting punctuation the parser didn't care about. + // Now EVERY bad id reaches the slug check — the message names the id. 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.match(parse("graph TD\n mps.translation[x]\n").problems[0]!, /node id `mps.translation` must be a lowercase slug/); + assert.match(parse("graph TD\n MPS[x]\n").problems[0]!, /node id `MPS` must be a lowercase slug/); + assert.match( + parse("graph TD\n a[Alpha] --> mps.translation[Beta]\n").problems[0]!, + /node id `mps.translation` must be a lowercase slug/, + "bad id inside an edge also gets the specific message", + ); 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("free-form pass (2026-09-11): underscores, chains, arrow synonyms, any direction, quoted titles", () => { + // Underscores are first-class slug chars — kept verbatim, referenced + // verbatim by quiz nodeId. + const us = parse("graph TD\n mps_translation[Ribosomes build protein] --> net_balance[Net balance]\n"); + assert.ok(us.ok, "underscore ids"); + assert.deepEqual(us.plan!.nodes, [ + { id: "mps_translation", title: "Ribosomes build protein" }, + { id: "net_balance", title: "Net balance" }, + ]); + // The exact shape the muscle-protein teacher wrote (attempt 1): + // quoted titles with colons, commas, equals signs — must just work. + const quoted = parse( + 'graph TD\n mps_translation["MPS = ribosomes building protein"] --> per_meal["Per-meal dose: ~20-40 g saturates, excess oxidized"]\n', + ); + assert.ok(quoted.ok, "quoted punctuation-rich titles"); + assert.deepEqual(quoted.plan!.nodes, [ + { id: "mps_translation", title: "MPS = ribosomes building protein" }, + { id: "per_meal", title: "Per-meal dose: ~20-40 g saturates, excess oxidized" }, + ]); + // Chains expand to consecutive pairs. + const chain = parse("graph TD\n a[Alpha] --> b[Bee] --> c[See]\n"); + assert.ok(chain.ok, "chained edges"); + assert.deepEqual(chain.plan!.edges, [ + { from: "a", to: "b" }, + { from: "b", to: "c" }, + ]); + // Arrow synonyms ==>, -.-> mean the same as -->. + for (const arrow of ["==>", "-.->"]) { + const res = parse(`graph TD\n a ${arrow} b\n`); + assert.ok(res.ok, arrow); + assert.deepEqual(res.plan!.edges, [{ from: "a", to: "b" }]); + } + // Any direction header (we layout ourselves). + for (const header of ["graph LR", "flowchart RL", "graph BT"]) { + assert.ok(parse(`${header}\n a --> b\n`).ok, header); + } + }); + 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" }]);