From d6d7c09e2e69ddf144fa28fb92876522118135e5 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sun, 27 Sep 2026 02:05:51 +0300 Subject: [PATCH] =?UTF-8?q?quiz=20nodeId=20required=20(owner=20ruling=2020?= =?UTF-8?q?26-09-26):=20no=20untagged=20teach=20quizzes=20=E2=80=94=20boun?= =?UTF-8?q?dary=20rejects,=20model=20retries=20tagged;=20off-map=20checks?= =?UTF-8?q?=20route=20to=20ask=5Fuser=5Fquestion;=20anti-rush=20doctrine?= =?UTF-8?q?=20in=20schema+prompt;=20docs+comments=20updated?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/probe-teach-split.md | 9 ++++--- src/agent/prompts/teacher/10-planning.md | 2 +- src/agent/tools.ts | 19 ++++++++------- src/core/contracts/plan.ts | 6 ++--- src/core/phases.ts | 5 ++-- src/core/question-core.ts | 19 +++++++-------- tests/chat-flow.test.ts | 15 ++++++++++++ tests/question-core.test.ts | 1 + tests/question-tools.test.ts | 31 +++++++++++++++++++----- 9 files changed, 72 insertions(+), 35 deletions(-) diff --git a/docs/probe-teach-split.md b/docs/probe-teach-split.md index 35fd287..d1b1c96 100644 --- a/docs/probe-teach-split.md +++ b/docs/probe-teach-split.md @@ -79,10 +79,11 @@ That is the wrong signal and is **abolished**. The new contract: regardless of individual check outcomes (a failed check followed by a re-teach and a move-on is still a visited node). - **Failed checks move nothing.** Results no longer feed derivation at all. -- **Untagged checks are legal ad-hoc checks** — they don't move the map - (quick comprehension checks don't need nodes). Not every check needs a - node. The prompt stresses tagging when teaching a map node: an untagged - stream leaves the map frozen (lost signal). +- **Untagged checks no longer exist** (owner ruling 2026-09-26): `nodeId` + is required on the teach quiz schema — an untagged call is rejected at the + boundary and the model retries tagged. Off-map checks (quick comprehension + checks that don't fit a node) route through ask_user_question. The map can + never freeze from a lost tagging signal again. - **No fallback current.** Before the first tagged check nothing is current. No derived-from-prereqs guess. - Present-but-invalid ids are still rejected at the call boundary (existing diff --git a/src/agent/prompts/teacher/10-planning.md b/src/agent/prompts/teacher/10-planning.md index adfb19e..8d5fa40 100644 --- a/src/agent/prompts/teacher/10-planning.md +++ b/src/agent/prompts/teacher/10-planning.md @@ -26,7 +26,7 @@ You write the map with `plan_init` (a mermaid `graph TD` block). It is a **map, The map is yours to rewrite at your own initiative: re-call `plan_init` whenever it's wrong — a detour node for a question worth exploring, a repair node for a caught misconception, prune what proves unnecessary. A map that can't change is a wrong map. -**Tag a quiz with `nodeId` whenever it's about a map node** — that's what moves the map: its live position is simply the latest tagged node, so asking about the next node IS how the lesson gets there. A tag is a position, not a verdict — nodes are never marked finished, a node takes as many questions as it deserves, and you move on by asking about the next one. Untagged questions are ad-hoc checks — legal, but they move nothing. +**Every quiz carries `nodeId`** (required — a quiz without it is rejected): that's what moves the map. Its live position is simply the latest tagged node, so asking about the next node IS how the lesson gets there. A tag is a position, not a verdict — nodes are never marked finished. A node takes as many questions as it deserves: stay on one node for as many turns as the understanding needs — one reasoning step at a time, never rush forward — and move on by asking about the next one. Off-map checks (no plan node fits) use ask_user_question. ## Presenting the plan — always, before any teaching diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 34c3f28..bd20fd0 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -201,8 +201,8 @@ function nodeIdProblem(ctx: Pick, nodeI return ( `nodeId "${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).") + ? `Copy one of your plan_init slugs exactly (${known.slice(0, 5).join(", ")}${known.length > 5 ? ", …" : ""}).` + : "No plan map exists yet — call plan_init before posing quizzes.") ); } return null; @@ -212,7 +212,9 @@ function nodeIdProblem(ctx: Pick, nodeI * + the per-session plan-nodeId validation (both run in execute — the * grading contract is untouched). Reads the LIVE chat record, so rewrites * mid-turn are honored, and with no tree yet every nodeId is rejected. - * Omitting nodeId is legal — an ad-hoc check that moves nothing. */ + * nodeId is REQUIRED (owner ruling 2026-09-26): the untagged ad-hoc quiz is + * gone — off-map checks route through ask_user_question, and the teach + * seam (first tagged quiz) is structurally guaranteed. */ export function teachQuizToolFor(ctx: Pick): AgentTool { return { name: "quiz", @@ -222,8 +224,9 @@ export function teachQuizToolFor(ctx: Pick { const problems = [...quizInputProblems(input)]; - if (typeof input.nodeId === "string" && input.nodeId.length > 0) { - const nodeIdProblemText = nodeIdProblem(ctx, input.nodeId); - if (nodeIdProblemText) problems.push(nodeIdProblemText); - } + const nodeIdProblemText = nodeIdProblem(ctx, input.nodeId); + if (nodeIdProblemText) problems.push(nodeIdProblemText); if (problems.length > 0) throw new ToolInputError("quiz", problems); return questionResult(); }, diff --git a/src/core/contracts/plan.ts b/src/core/contracts/plan.ts index 84c0e74..334d2e0 100644 --- a/src/core/contracts/plan.ts +++ b/src/core/contracts/plan.ts @@ -55,9 +55,9 @@ export type PlanNodeStatus = "current" | "visited" | "upcoming"; // node is the teacher's judgment the previous one is finished, regardless of // check outcomes (a failed check then a move-on is still visited); results // feed nothing; there is NO fallback current (before the first tagged check -// nothing is current). Untagged checks are legal ad-hoc checks — they move -// nothing. NodeIds missing from the current tree are ignored (a rewrite -// may have dropped the node). +// nothing is current). Untagged checks no longer exist (nodeId is required +// on the teach quiz, owner ruling 2026-09-26). NodeIds missing from the +// current tree are ignored (a rewrite may have dropped the node). /** Minimal structural message slice — anything with typed parts (mirrors * phases.ts: pure, no runtime deps, safe on server, client, and in tests). */ diff --git a/src/core/phases.ts b/src/core/phases.ts index 4c563cd..a4128cb 100644 --- a/src/core/phases.ts +++ b/src/core/phases.ts @@ -17,8 +17,9 @@ * turn, and the learner's next message is a natural reply (a change * request, or the go-ahead). The live indicator LAGS by design (owner * accepted): "teach" shows only once the first node's question is posed. - * A teacher that never tags a quiz would sit in "plan" visually — - * off-doctrine (tagging is prompt-mandated and nodeId is call-validated). + * A teacher that never tags a quiz can no longer exist — nodeId is + * schema-required at the teach quiz boundary (owner ruling 2026-09-26), + * so the seam signal is structural, not prompt-mandated. * * The former derived-occupancy seam (phase-of-tree via `currentNodeId`) * is GONE, as is the verdict-marker fold: no backwards compatibility diff --git a/src/core/question-core.ts b/src/core/question-core.ts index a72418c..ba74424 100644 --- a/src/core/question-core.ts +++ b/src/core/question-core.ts @@ -200,16 +200,15 @@ export const quizInputBase = Type.Object({ // never touches). export const quizInputObject = Type.Object({ ...quizInputBase.properties, - nodeId: Type.Optional( - Type.String({ - description: - "Optional plan-tree node id this question is about — must EXACTLY match one of this session's " + - "plan_init node slugs (validated at call time; a mismatched id is rejected back to you). " + - "Tag every question that targets a map node: the lesson's position on the map derives from these tags " + - "(the node the latest tagged question is about is where the map stands; earlier ones count as visited). " + - "Omit for quick ad-hoc checks that don't target a node.", - }), - ), + nodeId: Type.String({ + description: + "REQUIRED plan-tree node id this question is about — must EXACTLY match one of this session's " + + "plan_init node slugs (validated at call time; a mismatched id is rejected back to you). " + + "The lesson's position on the map derives from these tags (the node the latest tagged question is about is " + + "where the map stands; earlier ones count as visited). A node takes as many questions as it deserves: stay on " + + "one node for as many turns as the understanding needs — one reasoning step at a time, never rush forward. " + + "For off-map checks use ask_user_question.", + }), }); /** The PROBE-side quiz declaration: the shared authoring contract, no diff --git a/tests/chat-flow.test.ts b/tests/chat-flow.test.ts index 8751645..1e70979 100644 --- a/tests/chat-flow.test.ts +++ b/tests/chat-flow.test.ts @@ -55,6 +55,7 @@ const QUIZ_INPUT = { ], correctAnswer: "Mercury", explanation: "Mercury is the innermost planet.", + nodeId: "planet-order", }; /** Turn 1: a bit of reasoning, then the quiz tool call. Turn 2: plain text. @@ -187,6 +188,18 @@ function post(chatId: string, input: unknown): Request { function createConfiguredChat(title: string) { const chat = chatStore.createChat(title); chatStore.saveSettings(chat.id, { model: { provider: "openrouter", id: "z-ai/glm-5.3-flash" } }); + // nodeId is required on teach quizzes (owner ruling 2026-09-26) — the + // scripted lessons quiz against this seeded tree. + chatStore.saveTree(chat.id, { + nodes: [ + { id: "planet-order", title: "Planet order" }, + { id: "genealogy", title: "Genealogy of religion" }, + { id: "indexicals", title: "Indexical 'actual'" }, + { id: "modal-realism", title: "Modal realism" }, + ], + edges: [], + version: 1, + }); return chat; } @@ -560,6 +573,7 @@ describe("teacher chat flow (scripted model)", () => { // same turn. This pins that whole mechanism (the live Nietzsche bug). const BAD_QUIZ_INPUT = { question: "What is Nietzsche's genealogical claim about religion?", + nodeId: "genealogy", options: [{ label: "A cultural diagnosis" }, { label: "Atheism as a conclusion" }], correctAnswer: "cultural-diagnosis", // the exact live failure shape: a slug, no match explanation: "Genealogy diagnoses, it does not convert.", @@ -635,6 +649,7 @@ describe("teacher chat flow (scripted model)", () => { // question, and the model's retry must become the pending quiz. const STRINGY_QUIZ_INPUT = { question: "Lewis's indexical analysis of 'actual' — which is it?", + nodeId: "indexicals", options: JSON.stringify([ { label: "Like 'I'", value: "indexical" }, { label: "Like a title", value: "unique" }, diff --git a/tests/question-core.test.ts b/tests/question-core.test.ts index e4a92b1..c158f9e 100644 --- a/tests/question-core.test.ts +++ b/tests/question-core.test.ts @@ -73,6 +73,7 @@ describe("quizInputSchema / quizInputProblems (the authoring contract)", () => { options: [{ label: "Venus" }, { label: "Mercury" }, { label: "Mars" }], correctAnswer: "Mercury", explanation: "Mercury is the innermost planet.", + nodeId: "planet-order", }; it("accepts a valid single-select quiz (label as correctAnswer)", () => { diff --git a/tests/question-tools.test.ts b/tests/question-tools.test.ts index f3c407b..d33fe93 100644 --- a/tests/question-tools.test.ts +++ b/tests/question-tools.test.ts @@ -25,7 +25,7 @@ import { } from "../src/agent/tools.ts"; import { TOGGLEABLE_TOOLS } from "../src/core/pending.ts"; import { jobStore } from "../src/store/jobs.ts"; -import { probeQuizInputSchema } from "../src/core/question-core.ts"; +import { probeQuizInputSchema, quizInputObject } from "../src/core/question-core.ts"; import { chatStore } from "../src/store/chats.ts"; import { PATHS } from "../config.ts"; @@ -57,6 +57,7 @@ const validQuiz = { ], correctAnswer: "4", explanation: "Arithmetic.", + nodeId: "arith", }; import type { AgentTool } from "@earendil-works/pi-agent-core"; @@ -119,15 +120,33 @@ describe("teachQuizToolFor: the dynamic plan-nodeId validation (position signal) assert.match(msg, /arith/); }); - it("no tree yet: ANY nodeId is rejected — omit it for ad-hoc checks", async () => { + it("no tree yet: ANY nodeId is rejected — quizzes wait for plan_init", async () => { const chatId = newChat("NodeId: no tree"); const tool = teachQuizToolFor({ chatId, chatStore }); const res = await boundaryParse(tool, { ...validQuiz, nodeId: "anything" }); assert.equal(res.success, false); - assert.match(problemLines(res.error).at(-1)!, /No plan map exists yet/); - // …while an untagged ad-hoc check is fine - const plain = await boundaryParse(tool, validQuiz); - assert.equal(plain.success, true); + assert.match(problemLines(res.error).at(-1)!, /No plan map exists yet — call plan_init/); + }); + + it("missing nodeId is rejected at the boundary (owner ruling 2026-09-26: no untagged quizzes)", async () => { + const chatId = newChat("NodeId: required"); + chatStore.saveTree(chatId, { + nodes: [{ id: "arith", title: "Arithmetic" }], + edges: [], + version: 1, + }); + const tool = teachQuizToolFor({ chatId, chatStore }); + const { nodeId: _dropped, ...untagged } = validQuiz; + const res = await boundaryParse(tool, untagged); + assert.equal(res.success, false, "an untagged quiz must be rejected back to the model"); + // The structural pin: the SCHEMA itself names the missing field (pi's + // boundary formats Value.Errors as the tool-input-error the model sees). + const schemaErrors = [...Value.Errors(quizInputObject, untagged)].map((e) => e.message); + assert.match( + schemaErrors.join("\n"), + /nodeId/, + "the schema rejection names nodeId so the model can retry tagged", + ); }); it("reads the LIVE record — a plan_init mutation mid-turn is honored immediately", async () => {