fix: quiet Retrying lines, quiz option descriptions nuked, dividers at the markers

Three owner rulings from the gradient-descent session (chat_muhskq6p_e8c3600b — 7 failed quiz calls, answers leaked in option descriptions, planning divider floating at the transcript top): 1. RETRYING LINES (general): any tool call that errors back to the agent (validation failure, malformed args — the retry loop) renders one quiet "Retrying…" SystemLine instead of silence. Question tools previously rendered null, so a flailing quiz loop read as a stall. Aborts are terminal ("Operation aborted") and stay silent. Verified live: exactly 7 lines render at the 7 failure sites of the session. 2. QUIZ OPTION DESCRIPTIONS: NUKED. The model used them to telegraph correctness in every quiz of the session ("that moves uphill — it climbs f instead of descending it") — answerable by elimination. The field is gone from BOTH quiz schemas (probe + teach); ask keeps it (ungraded — nothing to leak). The dock additionally suppresses descriptions for quiz kind, so pre-fix records stop leaking too. Verified live: the session's pending quiz renders bare labels. Plus the diagnosed failure-mode fixes: GLM sometimes emits `options` as a JSON-encoded string (6 of 7 failures). prepareArguments hooks on all three question tools coerce it BEFORE pi's validation (the call lands instead of bouncing); the client coerces again in normalizeOptions (pi persists raw call args). Schema descriptions hardened ("REQUIRED ... never a JSON-encoded string"). 3. DIVIDERS AT THE MARKERS: the probe-handoff SystemLine ("Preparing your plan…") IS NOW the Planning divider; the accepted verdict IS NOW the Teaching divider ("✓ Plan accepted" line gone) — one signal per seam, not two. The fold-based divider block is DELETED: Wave D grouping merged the whole probe into one giant assistant group, so the inclusive-fold comparison floated the divider to the transcript top (reproduced against the real record: 6 view messages, phase flipped at the giant group). The markers render in the right spot by record construction. A DECLINED verdict is not a seam — it keeps the "↻ Plan revision requested" line. Verified live: plan divider after the probe concluded, teach divider after the plan presentation, nothing at the top. Tests: 1004/1004 — string-coercion pins (normalizeOptions + the prepareArguments hooks on all three tools, identity fast path), quiz-schema-has-no-description pin (ask keeps it).
2026-09-26 19:08:39 +03:00
parent b84ab1cdc0
commit 9853075331
6 changed files with 186 additions and 44 deletions
@@ -34,7 +34,7 @@ import { QuestionDock } from "@/components/question-dock";
 import { SidebarToggle } from "@/components/sidebar";
 import type { ArtifactPartProps, JobRecord, Phase3, QuestionAnswerData, TurnUsage } from "@/src/core";
 import type { LhViewMessage } from "@/src/core";
-import { PhaseChip, PhaseDivider, planNodeCount } from "@/components/phase-divider";
+import { PhaseChip, PhaseDivider } from "@/components/phase-divider";
 import { CoverageRing } from "@/components/extraction-coverage";
 import { findPendingToolCall, phaseAt3, phaseTimeline, streamingToolActivity } from "@/src/core";
 import { slotsConfigured, type ModelsSettings } from "@/src/core";
@@ -1067,41 +1067,31 @@ export function Chat({
 									// can still hold empty ids — the index keeps keys unique for them.
 									<SubagentSteerCard key={m.id || `m-${mi}`} message={m} />
 								) : m.role === "user" && m.parts.some((p) => p.type === "data-probe-handoff") ? (
-								// The probe→teach handoff wake (custom-message scheme): quiet
-								// system line, never a user bubble — the wake is scaffolding,
-								// the "Planning" divider marks the seam, and the model-facing
-								// instruction lives in the adapter.
-								<SystemLine key={m.id || `m-${mi}`} testId="probe-handoff">
-									Preparing your plan…
-								</SystemLine>
+								// The probe→teach handoff wake (custom-message scheme): the
+								// PLANNING DIVIDER IS the seam (owner ruling 2026-09-26 — it
+								// replaced the "Preparing your plan…" system line; the marker
+								// renders in the right spot by record construction, so the old
+								// fold-based divider — which floated to the transcript top once
+								// Wave D grouping merged the whole probe into one giant
+								// assistant group — is gone). Never a user bubble: the wake is
+								// scaffolding; the model-facing instruction lives in the adapter.
+								<PhaseDivider key={m.id || `m-${mi}`} phase="plan" />
 							) : m.role === "user" && m.parts.some((p) => p.type === "data-verdict") ? (
-								// The learner's plan verdict (custom-message scheme): its own
-								// message, the quiet marker — the accept is the phase seam, a
-								// decline asks for revision. Never a user bubble: the model-
-								// facing wording is the adapter's, and this is the learner's
-								// ACT, not their words (a typed note, if any, rendered above
-								// as its own real message).
-								<SystemLine
-									key={m.id || `m-${mi}`}
-									testId={
-										(m.parts.find((p) => p.type === "data-verdict") as
-											| { data?: { accepted?: boolean } }
-											| undefined)?.data?.accepted
-											? "verdict-accepted"
-											: "verdict-declined"
-									}
-								>
-									{(m.parts.find((p) => p.type === "data-verdict") as
-										| { data?: { accepted?: boolean } }
-										| undefined)?.data?.accepted
-										? "✓ Plan accepted"
-										: "↻ Plan revision requested"}
-								</SystemLine>
+								// The learner's plan verdict (custom-message scheme): the
+								// accept IS the phase seam — it renders the TEACHING DIVIDER
+								// (owner ruling 2026-09-26: replaced the "✓ Plan accepted"
+								// line; same spot, one signal, not two). A DECLINE is not a
+								// seam — it stays the quiet revision line. Never a user
+								// bubble: the model-facing wording is the adapter's, and this
+								// is the learner's ACT, not their words (a typed note, if
+								// any, renders above as its own real message).
+								((m.parts.find((p) => p.type === "data-verdict") as
+									| { data?: { accepted?: boolean } }
+									| undefined)?.data?.accepted)
+									? <PhaseDivider key={m.id || `m-${mi}`} phase="teach" />
+									: <SystemLine key={m.id || `m-${mi}`} testId="verdict-declined">↻ Plan revision requested</SystemLine>
 							) : (
 									<Fragment key={m.id || `m-${mi}`}>
-									{mi > 0 && phases[mi] !== phases[mi - 1] && (
-										<PhaseDivider phase={phases[mi]!} nodeCount={planNodeCount(m)} />
-									)}
 									<Message from={m.role} className={m.role === "assistant" ? "max-w-full" : undefined}>
 									<MessageContent className={m.role === "assistant" ? "w-full" : undefined}>
 										{m.role === "user" ? (
@@ -1113,6 +1103,24 @@ export function Chat({
 										) : (
 											m.parts.map((part, pi) => {
 												const isLastPart = mi === messages.length - 1 && pi === m.parts.length - 1;
+												// A tool call that errored BACK TO THE AGENT (validation
+												// failure, malformed args) is invisible work — the agent
+												// reads the error and retries. Owner ruling 2026-09-26:
+												// surface the retry loop as one quiet line, any tool,
+												// instead of silence (question tools used to render null,
+												// so a flailing quiz loop read as a stall). Aborts are
+												// terminal — nothing retries — so they stay silent.
+												if (
+													part.state === "output-error" &&
+													part.type.startsWith("tool-") &&
+													part.errorText !== "Operation aborted"
+												) {
+													return (
+														<SystemLine key={`retry-${pi}`} testId="tool-retry">
+															Retrying…
+														</SystemLine>
+													);
+												}
 												switch (part.type) {
 													case "text":
 														return <MessageResponse key={pi}>{part.text}</MessageResponse>;
@@ -457,9 +457,12 @@ function QuestionDockForm({
 									</span>
 									<span>
 										<MessageResponse>{opt.label}</MessageResponse>
-										{opt.description && (
+										{kind !== "quiz" && opt.description && (
 											// Option subtext renders in Source Sans 3 (owner's mockup);
-											// the "I don't know" copy stays JetBrains Mono.
+											// the "I don't know" copy stays JetBrains Mono. QUIZ options render
+											// NO description (owner ruling 2026-09-26): the model used them to
+											// telegraph correctness — the schema dropped the field and old
+											// records must not leak either.
 											<span className="lh-prose block text-xs text-muted-foreground">{opt.description}</span>
 										)}
 									</span>
@@ -4,6 +4,7 @@ import { questionResult, serverExecute, ToolInputError } from "./tool-execute.ts
 import {
 	askInputSchema,
 	askOptionProblems,
+	parseOptionsIfString,
 	probeQuizInputSchema,
 	quizInputObject,
 	quizInputProblems,
@@ -51,6 +52,26 @@ export type { ProbeDoneContext } from "./tools/probe.ts";
 *   only.
 */

+/** GLM wobble guard (owner-approved, 2026-09-26): models sometimes emit
+ *  `options` as a JSON-encoded string instead of an array; pi runs this
+ *  BEFORE schema validation, so a double-encoded call lands instead of
+ *  bouncing back as a validation error + wasted retry turn. NOTE: pi
+ *  persists the tool call with its RAW arguments — the client coerces
+ *  again in normalizeOptions (question-core), the same parseOptionsIfString.
+ *  Anything unparseable passes through untouched: validation then reports
+ *  honestly and the agent retries with a real error to learn from. */
+function prepareQuestionArguments<T extends object>(args: unknown): T {
+	if (args && typeof args === "object" && !Array.isArray(args)) {
+		const o = args as { options?: unknown };
+		if (typeof o.options === "string") {
+			const parsed = parseOptionsIfString(o.options);
+			if (parsed !== o.options) return { ...o, options: parsed } as T;
+		}
+		return o as T;
+	}
+	return args as T;
+}
+
 export const askUserQuestionTool: AgentTool<typeof askInputSchema> = {
 	name: "ask_user_question",
 	label: "ask_user_question",
@@ -60,6 +81,7 @@ export const askUserQuestionTool: AgentTool<typeof askInputSchema> = {
 		"or you need an open-ended response. ONE question per turn: call a single ask_user_question, then end your " +
 		"turn — his answer determines the next.",
 	parameters: askInputSchema,
+	prepareArguments: prepareQuestionArguments,
 	executionMode: "sequential",
 	execute: async (_toolCallId, input) => {
 		const problems = askOptionProblems(input.options ?? []);
@@ -85,6 +107,7 @@ export const probeQuizTool: AgentTool<typeof probeQuizInputSchema> = {
 		"questions use ask_user_question instead. ONE question per turn: call a single quiz, then end your turn — " +
 		"his answer determines the next question to ask.",
 	parameters: probeQuizInputSchema,
+	prepareArguments: prepareQuestionArguments,
 	executionMode: "sequential",
 	execute: async (_toolCallId, input) => {
 		const problems = quizInputProblems(input);
@@ -206,6 +229,7 @@ export function teachQuizToolFor(ctx: Pick<PlanToolContext, "chatId" | "chatStor
 			"only when non-empty. No free-text answers — for non-graded questions use ask_user_question instead. " +
 			"ONE question per turn: call a single quiz, then end your turn — his answer determines the next.",
 		parameters: quizInputObject,
+		prepareArguments: prepareQuestionArguments,
 		executionMode: "sequential",
 		execute: async (_toolCallId, input) => {
 			const problems = [...quizInputProblems(input)];
@@ -30,19 +30,46 @@ export interface QuizOption {
 // ─── tool input schemas (the question contract) ─────────────────────────────
 // Parameter names mirror the prompt docs exactly — keep them in sync.

-/** One answer option — shared by quiz + ask. */
-const optionProps = {
-	label: Type.String({ description: "Display label for the answer option" }),
+/** One quiz answer option. Owner ruling (2026-09-26): NO `description`
+ *  field — the model reliably used it to leak correctness ("that's not the
+ *  gradient", "close but…"), making quizzes answerable by elimination.
+ *  Neutral elaboration belongs in the question text or the post-answer
+ *  explanation; the option is a bare claim. */
+const quizOptionProps = {
+	label: Type.String({ description: "Display label for the answer option — a bare claim, no reasoning" }),
 	value: Type.Optional(
 		Type.String({
 			description: "Optional machine-readable value returned for the option. Defaults to the label.",
 		}),
 	),
+};
+
+/** One ask answer option — keeps `description`: ask is ungraded preference
+ *  collection, there is no correctness to leak. */
+const askOptionProps = {
+	...quizOptionProps,
 	description: Type.Optional(
 		Type.String({ description: "Optional extra detail shown below the option" }),
 	),
 };

+/** GLM wobble guard (owner-approved, 2026-09-26): models sometimes emit
+ *  `options` as a JSON-ENCODED STRING instead of an array. Parse it before
+ *  use. Shared by the tools' prepareArguments (server, pre-validation) and
+ *  normalizeOptions (client — the persisted call keeps the raw string, so
+ *  the dock must coerce again). Returns the input untouched when it isn't
+ *  a parseable array — validation then reports honestly. */
+export function parseOptionsIfString<T>(options: string | T[] | undefined): unknown {
+	if (typeof options !== "string") return options;
+	try {
+		const parsed: unknown = JSON.parse(options);
+		if (Array.isArray(parsed)) return parsed;
+	} catch {
+		/* not JSON — fall through untouched */
+	}
+	return options;
+}
+
 // ─── cross-field authoring validation (tool-schema refinements) ──────────
 // Enforced at the tool-call boundary (see the superRefines below): an invalid
 // question NEVER reaches the learner — the SDK returns the validation error
@@ -130,10 +157,11 @@ export const quizInputBase = Type.Object({
 		description:
 			"The single quiz question to ask. ONE question per turn: ask, then end your turn — the answer determines the next question.",
 	}),
-	options: Type.Array(Type.Object(optionProps), {
+	options: Type.Array(Type.Object(quizOptionProps), {
 		minItems: 2,
 		description:
-			"The answer options (2 or more). Options only — there is no free-text mode. " +
+			"REQUIRED. The answer options (2 or more) as a JSON ARRAY of option objects — never a " +
+			"JSON-encoded string. Options only — there is no free-text mode. " +
 			"Give each option a stable `value`; you reference the correct one by that value in correctAnswer. " +
 			"Never add your own 'I don't know' option — that choice is added for you.",
 	}),
@@ -205,7 +233,7 @@ export const askInputSchema = Type.Object({
 		Type.String({ description: "Optional extra context or instructions shown under the question." }),
 	),
 	options: Type.Optional(
-		Type.Array(Type.Object(optionProps), {
+		Type.Array(Type.Object(askOptionProps), {
 			minItems: 2,
 			description:
 				"Optional multiple-choice options. Omit for free-form text input. " +
@@ -330,10 +358,17 @@ export const QUESTION_ANSWERED_ENTRY = "lh_question_answered";
 // ─── options ────────────────────────────────────────────────────────────────

 export function normalizeOptions(
-	options: Array<{ label: string; value?: string; description?: string }> | undefined,
+	options: string | Array<{ label: string; value?: string; description?: string }> | undefined,
 ): QuizOption[] {
 	const seen = new Set<string>();
-	return (options || [])
+	// The persisted tool call can carry GLM's string-encoded options —
+	// parseOptionsIfString is the same coercion the server's
+	// prepareArguments runs pre-validation (see parseOptionsIfString).
+	const coerced = parseOptionsIfString(options);
+	const list = Array.isArray(coerced)
+		? (coerced as Array<{ label: string; value?: string; description?: string }>)
+		: [];
+	return list
 		.map((option) => ({
 			label: option.label.trim(),
 			value: option.value?.trim() || option.label.trim(),
@@ -8,6 +8,7 @@ import { Value } from "typebox/value";
 import type { TSchema } from "typebox";
 import {
 	askInputSchema,
+	quizInputBase,
 	askOptionProblems,
 	buildAnswer,
 	coerceCorrectAnswer,
@@ -184,6 +185,34 @@ describe("normalizeOptions", () => {

 	it("throws on duplicate option values", () => {
 		assert.throws(() => normalizeOptions([{ label: "a", value: "x" }, { label: "b", value: "x" }]), /duplicate/);
+
+		// GLM wobble guard (2026-09-26 gradient-descent session): the model
+		// sometimes emits `options` as a JSON-encoded string — 6 of 7 quiz
+		// validation failures in that session. The dock (and grading) coerce
+		// via the same parseOptionsIfString the server's prepareArguments
+		// runs pre-validation.
+		const asString = JSON.stringify([{ label: "Slope", value: "slope" }, { label: "Value", value: "value" }]);
+		const coerced = normalizeOptions(asString);
+		assert.deepEqual(coerced, [
+			{ label: "Slope", value: "slope", description: undefined },
+			{ label: "Value", value: "value", description: undefined },
+		], "a JSON-encoded string coerces to the same normalized options");
+		// Non-JSON strings pass through and behave like no options (the
+		// server's schema validation reports the honest error instead).
+		assert.deepEqual(normalizeOptions("not json"), []);
+	});
+
+it("quiz options carry NO description field; ask keeps it (owner nuke, 2026-09-26)", () => {
+	// The model used quiz option descriptions to telegraph correctness
+	// ("that's not the gradient") in every quiz of the gradient-descent
+	// session — the field is gone from BOTH quiz schemas; ask (ungraded,
+	// nothing to leak) keeps it.
+	const quizOption = (quizInputBase.properties.options as { items?: { properties?: Record<string, unknown> } })
+		.items!.properties!;
+	assert.ok(!("description" in quizOption), "quiz option schema has no description");
+	const askOption = (askInputSchema.properties.options as { items?: { properties?: Record<string, unknown> } })
+		.items!.properties!;
+	assert.ok("description" in askOption, "ask option schema keeps description");
 	});

 	it("handles undefined input", () => {
@@ -15,7 +15,14 @@ import * as fs from "node:fs";
 import * as path from "node:path";
 import { after, describe, it } from "node:test";
 import { Value } from "typebox/value";
-import { createProberTools, createTeacherTools, teachQuizToolFor, type TeacherToolContext } from "../src/agent/tools.ts";
+import {
+	askUserQuestionTool,
+	createProberTools,
+	createTeacherTools,
+	probeQuizTool,
+	teachQuizToolFor,
+	type TeacherToolContext,
+} 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";
@@ -231,3 +238,39 @@ describe("tool toggles (core/pending.ts + factories)", () => {
 		);
 	});
 });
+
+
+// ─── prepareArguments: the GLM string-encoded options guard (2026-09-26) ────
+
+describe("prepareQuestionArguments (the string-encoded options wobble)", () => {
+	// The gradient-descent session (chat_muhskq6p): 6 of 7 quiz validation
+	// failures were `options` arriving as a JSON-encoded STRING. pi runs
+	// prepareArguments BEFORE schema validation — the coerced call lands
+	// instead of bouncing back as a wasted retry turn.
+	const goodOptions = [
+		{ label: "The slope of the tangent line", value: "slope" },
+		{ label: "The value of the curve", value: "value" },
+	];
+	const asString = JSON.stringify(goodOptions);
+
+	it("coerces string-encoded options on ALL THREE question tools", () => {
+		for (const tool of [askUserQuestionTool, probeQuizTool, teachQuizToolFor({ chatId: "chat_x", chatStore: undefined as never })]) {
+			const prepare = (tool as unknown as { prepareArguments?: (a: unknown) => unknown }).prepareArguments;
+			assert.ok(prepare, `${tool.name} has the hook`);
+			const out = prepare({ question: "q", options: asString, correctAnswer: "slope", explanation: "e" });
+			assert.deepEqual(
+				(out as { options: unknown }).options,
+				goodOptions,
+				`${tool.name} coerces the string to an array`,
+			);
+		}
+	});
+
+	it("passes everything else through UNTOUCHED (identity — pi's fast path)", () => {
+		const prepare = (probeQuizTool as unknown as { prepareArguments: (a: unknown) => unknown }).prepareArguments;
+		const arrayArgs = { question: "q", options: goodOptions, correctAnswer: "slope", explanation: "e" };
+		assert.equal(prepare(arrayArgs), arrayArgs, "array args return the SAME object (no defensive copies)");
+		const unparseable = { question: "q", options: "not json", correctAnswer: "slope", explanation: "e" };
+		assert.equal(prepare(unparseable), unparseable, "unparseable strings pass through — validation reports honestly");
+	});
+});