From 985307533181d491bf48eb6fd57b78f6b7b2f6e8 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sat, 26 Sep 2026 19:08:39 +0300 Subject: [PATCH] fix: quiet Retrying lines, quiz option descriptions nuked, dividers at the markers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- components/chat.tsx | 74 ++++++++++++++++++++---------------- components/question-dock.tsx | 7 +++- src/agent/tools.ts | 24 ++++++++++++ src/core/question-core.ts | 51 +++++++++++++++++++++---- tests/question-core.test.ts | 29 ++++++++++++++ tests/question-tools.test.ts | 45 +++++++++++++++++++++- 6 files changed, 186 insertions(+), 44 deletions(-) diff --git a/components/chat.tsx b/components/chat.tsx index 821ef1c..4b34b6b 100644 --- a/components/chat.tsx +++ b/components/chat.tsx @@ -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. ) : 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. - - Preparing your plan… - + // 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. + ) : 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). - 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"} - + // 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) + ? + : ↻ Plan revision requested ) : ( - {mi > 0 && phases[mi] !== phases[mi - 1] && ( - - )} {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 ( + + Retrying… + + ); + } switch (part.type) { case "text": return {part.text}; diff --git a/components/question-dock.tsx b/components/question-dock.tsx index feb1bf4..ece98dd 100644 --- a/components/question-dock.tsx +++ b/components/question-dock.tsx @@ -457,9 +457,12 @@ function QuestionDockForm({ {opt.label} - {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. {opt.description} )} diff --git a/src/agent/tools.ts b/src/agent/tools.ts index 1f73f27..34c3f28 100644 --- a/src/agent/tools.ts +++ b/src/agent/tools.ts @@ -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(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 = { name: "ask_user_question", label: "ask_user_question", @@ -60,6 +81,7 @@ export const askUserQuestionTool: AgentTool = { "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 = { "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 { const problems = [...quizInputProblems(input)]; diff --git a/src/core/question-core.ts b/src/core/question-core.ts index 627874e..a72418c 100644 --- a/src/core/question-core.ts +++ b/src/core/question-core.ts @@ -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(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(); - 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(), diff --git a/tests/question-core.test.ts b/tests/question-core.test.ts index 5bcec0e..e4a92b1 100644 --- a/tests/question-core.test.ts +++ b/tests/question-core.test.ts @@ -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 } }) + .items!.properties!; + assert.ok(!("description" in quizOption), "quiz option schema has no description"); + const askOption = (askInputSchema.properties.options as { items?: { properties?: Record } }) + .items!.properties!; + assert.ok("description" in askOption, "ask option schema keeps description"); }); it("handles undefined input", () => { diff --git a/tests/question-tools.test.ts b/tests/question-tools.test.ts index e0697aa..f3c407b 100644 --- a/tests/question-tools.test.ts +++ b/tests/question-tools.test.ts @@ -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"); + }); +});