From b84ab1cdc07c791a0cd382278955e13c9544aa45 Mon Sep 17 00:00:00 2001 From: amosblomqvist Date: Sat, 26 Sep 2026 05:46:33 +0300 Subject: [PATCH] fix: reasoning always renders above its message; dock drops delivered subagents at delivery TWO owner-reported bugs from the live red-circle session (chat_muhpyoll_27305e23), both investigated to root cause first: 1. THINKING BLOCKS UNDER THE MESSAGE. GLM emits trailing reasoning tokens, so the persisted content lands [text, thinking] (4 of the 6 thinking-bearing messages from [21] on in that session) - and the view canon presented arrival order verbatim, rendering the reasoning UNDER its own message. Fix: projectView stable-partitions each assistant message - reasoning parts first (their own order kept), then text/tool parts. Owner ruling 2026-09-26: the reasoning belongs above, always. Covers live partials and settled records alike (one projection canon). NOT addressed (owner ruling): the plan_tools "missing" claim in the same session - wire-verified 11 tools incl. plan_init; GLM confabulation, no mitigation wanted. 2. DOCK CARDS LINGERING AFTER DELIVERY. /api/activity lists jobs with status "running", and the status flipped only in settleSteers at the DELIVERING TURN'S onFinish - so a subagent whose card was already visible in the transcript (marker injected mid-run) stayed in the background dock until the teacher finished teaching. Fix: flip the job at the moment the result reaches the learner - noteSteerVisible (jobs.ts) called from BOTH visibility seams: the marker's message_end wire event (live steer injection, ui-stream) and the write-ahead persist (the wake's input appends, chat-handler). finalizeJob is idempotent; settleSteers/runner-200 re-run it as a no-op, and the registry's delivery promise still settles exactly as before. Tests: 1001/1001 - view-canon hoist pin (trailing [text, think, call] projects reasoning-first; think-first unchanged), noteSteerVisible unit pin (flip at the seam, idempotent, handoff no-op), and the mid-run steering e2e now holds the turn open past the injection and pins the job flipping done at the marker's message_end WHILE the turn still streams. Live-verified read-only on the owner's server: the original session reloads with every reasoning trigger above its message. --- src/core/view.ts | 18 ++++++++--- src/server/chat-handler.ts | 18 +++++++++-- src/server/jobs.ts | 14 +++++++++ src/server/pi/ui-stream.ts | 7 +++++ tests/chat-flow.test.ts | 61 +++++++++++++++++++++++++++++++++++++- tests/jobs.test.ts | 19 +++++++++++- 6 files changed, 129 insertions(+), 8 deletions(-) diff --git a/src/core/view.ts b/src/core/view.ts index e439b06..2372672 100644 --- a/src/core/view.ts +++ b/src/core/view.ts @@ -156,18 +156,28 @@ export function projectView(messages: readonly LhMessage[], live?: LiveTurnState if (role === "assistant") { const assistant = m as AssistantMessage; - const parts: LhViewPart[] = []; + // OWNER RULING (2026-09-26): reasoning ALWAYS renders above the + // message it belongs to. Providers emit trailing reasoning tokens + // (GLM fires a final reasoning burst after the answer), which + // lands the thinking block AFTER the text in the persisted + // content — presenting arrival order verbatim put the reasoning + // UNDER the message. The view canon owns presentation order: + // stable-partition each assistant message, reasoning parts first + // (their own order kept), then everything else. + const rest: LhViewPart[] = []; + const reasoning: LhViewPart[] = []; let textN = 0; let reasoningN = 0; for (const block of assistant.content) { if (block.type === "text") { - parts.push({ type: "text", id: `${m.id}-t${textN++}`, text: block.text, state: "done" }); + rest.push({ type: "text", id: `${m.id}-t${textN++}`, text: block.text, state: "done" }); } else if (block.type === "thinking") { - parts.push({ type: "reasoning", id: `${m.id}-r${reasoningN++}`, text: block.thinking, state: "done" }); + reasoning.push({ type: "reasoning", id: `${m.id}-r${reasoningN++}`, text: block.thinking, state: "done" }); } else if (block.type === "toolCall") { - parts.push(toolPartOf(block, live)); + rest.push(toolPartOf(block, live)); } } + const parts: LhViewPart[] = [...reasoning, ...rest]; if (parts.length === 0) continue; // presentation-only (nothing renderable) const group = lastAssistant(); if (group) { diff --git a/src/server/chat-handler.ts b/src/server/chat-handler.ts index d4b4498..814ff8b 100644 --- a/src/server/chat-handler.ts +++ b/src/server/chat-handler.ts @@ -4,7 +4,15 @@ import { piEngineForSpec, type PiEngine } from "../agent/models.ts"; import { createJobRunner } from "../agent/subagents/runner.ts"; import { createExtractionPipeline } from "../agent/subagents/extraction.ts"; import type { ModelRole } from "../../config.ts"; -import { deliverJobResult, registerLiveAgent, setJobRespawn, setTurnRunner, settleSteers, unregisterLiveAgent } from "./jobs.ts"; +import { + deliverJobResult, + noteSteerVisible, + registerLiveAgent, + setJobRespawn, + setTurnRunner, + settleSteers, + unregisterLiveAgent, +} from "./jobs.ts"; import { piTurnResponse, runPiTurn } from "./pi/ui-stream.ts"; import { effectiveSettings, stores } from "./stores.ts"; import { searchAvailable, searchProvider } from "./search.ts"; @@ -28,7 +36,7 @@ import { type TurnInput, } from "../core/input-events.ts"; import { steerDelivered } from "../core/contracts.ts"; -import type { CallUsage } from "../core/contracts.ts"; +import type {CallUsage, SteerMessage} from "../core/contracts.ts"; /** * One chat turn over the server-owned native record. Framework-free (plain @@ -219,6 +227,12 @@ export async function handleChatInput( const merged = [...record.messages, ...appends]; try { stores.chatStore.saveMessages(chatId, merged); + // Dock fix (owner ruling 2026-09-26): persisted markers are visible + // in the record — flip their jobs now (the wake's steers ride this + // write-ahead; the delivering turn may run for minutes). + for (const appended of appends) { + if ((appended as { role?: string }).role === "custom") noteSteerVisible(appended as SteerMessage); + } } catch (err) { console.error(`[chat ${chatId}] write-ahead persist failed:`, err); } diff --git a/src/server/jobs.ts b/src/server/jobs.ts index 35435a2..2c26e2e 100644 --- a/src/server/jobs.ts +++ b/src/server/jobs.ts @@ -156,6 +156,20 @@ function jobIdOf(steer: SteerMessage): string | null { return typeof details?.jobId === "string" ? details.jobId : null; } +/** Dock bookkeeping: the marker reached the learner's view — flip its job + * NOW instead of waiting for the delivering turn to settle. Called from + * BOTH visibility seams: the marker's message_end wire event (live steer + * injection — the card is on screen while the teacher keeps teaching) and + * the write-ahead persist in handleChatInput (the wake's input appends — + * the card is durable in the record). The registry's delivery promise + * still settles via settleSteers / the runner-200 path; finalizeJob is + * idempotent, so the later flip is a no-op. Owner ruling (2026-09-26): a + * delivered subagent leaves the background dock the moment its result is + * visible, not when the teacher's turn ends. */ +export function noteSteerVisible(steer: SteerMessage): void { + finalizeSteer(steer); +} + /** Finalize a delivered steer's job bookkeeping — job steers only. */ function finalizeSteer(steer: SteerMessage): void { const jobId = jobIdOf(steer); diff --git a/src/server/pi/ui-stream.ts b/src/server/pi/ui-stream.ts index 0e89302..eed1461 100644 --- a/src/server/pi/ui-stream.ts +++ b/src/server/pi/ui-stream.ts @@ -1,4 +1,6 @@ import type { Agent, AgentEvent, AgentMessage } from "@earendil-works/pi-agent-core"; +import type { SteerMessage } from "../../core/contracts.ts"; +import { noteSteerVisible } from "../jobs.ts"; /** * THE NATIVE WIRE (Wave D, owner-approved 2026-09-26): one agent turn, streamed @@ -123,6 +125,11 @@ export function runPiTurn(spec: PiTurnSpec): PiTurnHandle { if (event.type === "message_end") { const m = event.message as { role?: string; stopReason?: string } | undefined; if (m?.role === "assistant" && m.stopReason === "aborted") sawAbort = true; + // Dock fix (owner ruling 2026-09-26): an injected steer marker's + // message_end = the card is visible to the learner — flip the + // job's bookkeeping now (the turn may keep teaching for minutes; + // settleSteers re-runs the flip as a no-op idempotently). + if (m?.role === "custom") noteSteerVisible(event.message as SteerMessage); } if (event.type === "agent_end") { closeAll(); diff --git a/tests/chat-flow.test.ts b/tests/chat-flow.test.ts index 050da3e..8751645 100644 --- a/tests/chat-flow.test.ts +++ b/tests/chat-flow.test.ts @@ -404,6 +404,10 @@ describe("teacher chat flow (scripted model)", () => { // the reaction AFTER the boundary injection (records its prompt). let release!: () => void; const gate = new Promise((r) => (release = r)); + let release2!: () => void; + const gate2 = new Promise((r) => (release2 = r)); + let jobIdMid = ""; + let jobFlippedMidTurn: string | undefined; const seen: string[] = []; const faux = createFauxCore({}); faux.setResponses([ @@ -414,8 +418,14 @@ describe("teacher chat flow (scripted model)", () => { { stopReason: "toolUse" }, ); }, - (context) => { + async (context) => { seen.push(context.messages.map((m) => JSON.stringify((m as { content: unknown }).content)).join("\n")); + // HOLD the turn open past the injection: the marker's + // message_end already fired, so the job must be flipped + // done NOW (dock fix, owner 2026-09-26) — not at this + // turn's settle. + jobFlippedMidTurn = jobStore.getJob(jobIdMid)?.status; + await gate2; return fauxAssistantMessage([fauxText("Folded the marker in.")]); }, ]); @@ -437,14 +447,21 @@ describe("teacher chat flow (scripted model)", () => { // steering queue (vehicle 1) — the queued marker waits for the // in-flight turn to complete so the injection lands at its boundary. const job = jobStore.createJob({ kind: "research", chatId: chat.id, toolCallId: "call-mid" }); + jobIdMid = job.id; + // The runner stores the result BEFORE delivery (finalizeJob reads it). + jobStore.updateJob(job.id, { result: { brief: "Mid-run brief.", sources: [] } }); const steer = buildSubagentSteer(job, { brief: "Mid-run brief.", sources: [] }); const delivery = deliverJobResult(steer); await new Promise((r) => setTimeout(r, 300)); // vehicle 1 hands off assert.equal(wakes, 0, "no wake — the live run owns the delivery"); release(); // step 1 completes → tool result → TURN BOUNDARY → inject → step 2 + // Step 2 has pended briefly — give the wire seam a beat, then release. + await new Promise((r) => setTimeout(r, 150)); + release2(); await drain(await res); await delivery; // resolves at settleSteers (the run persisted the marker) + assert.equal(jobFlippedMidTurn, "done", "the job flipped at the marker's message_end — WHILE the turn still streamed"); // Record shape: the marker sits BETWEEN completed turn units — after // the toolResult it must never split, adjacent to it. @@ -467,6 +484,48 @@ describe("teacher chat flow (scripted model)", () => { assert.equal(wakes, 0, "delivery rode the live run end to end"); }); + it("the view canon HOISTS reasoning above the message text (owner ruling 2026-09-26)", () => { + // GLM emits trailing reasoning tokens — the persisted content lands + // [text, thinking] (and [text, thinking, toolCall]), which arrival-order + // presentation rendered as reasoning UNDER its message. The projection + // stable-partitions: reasoning parts first (their own order), then the + // rest — live partials and settled records alike. + const baseMessage = { + role: "assistant", + api: "openai-completions", + provider: "openrouter", + model: "test", + usage: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + timestamp: 1, + }; + const trailingThink: LhMessage = { + ...baseMessage, + id: "a_trail", + content: [ + { type: "text", text: "The answer text." }, + { type: "thinking", thinking: "Checked the map afterwards." }, + { type: "toolCall", id: "call-tr", name: "quiz", arguments: { question: "q", options: [{ label: "a" }], correctAnswer: "a", explanation: "" } }, + ], + stopReason: "toolUse", + } as never; + const view = projectView([{ id: "u0", role: "user", content: "hi", timestamp: 1 } as never, trailingThink]); + assert.equal(view.length, 2); + const parts = view[1]!.parts.filter((p) => p.type !== "step-start"); + const order = parts.map((p) => p.type); + assert.equal(order[0], "reasoning", "reasoning renders FIRST regardless of content order"); + assert.ok(order.indexOf("text") > order.indexOf("reasoning"), "text follows the reasoning"); + assert.equal(order[order.length - 1], "tool-quiz", "the tool call stays last"); + // The canonical think-first shape is unchanged. + const leadingThink: LhMessage = { + ...baseMessage, + id: "a_lead", + stopReason: "stop", + content: [{ type: "thinking", thinking: "First." }, { type: "text", text: "The answer text." }], + } as never; + const view2 = projectView([{ id: "u1", role: "user", content: "hi", timestamp: 1 } as never, leadingThink]); + assert.deepEqual(view2[1]!.parts.filter((p) => p.type !== "step-start").map((p) => p.type), ["reasoning", "text"], "think-first messages project identically"); + }); + it("a cancelled view NEVER aborts the turn — it runs to completion (owner ruling 2026-09-25)", async () => { let release!: () => void; const gate = new Promise((r) => (release = r)); diff --git a/tests/jobs.test.ts b/tests/jobs.test.ts index b68ad42..f57a126 100644 --- a/tests/jobs.test.ts +++ b/tests/jobs.test.ts @@ -50,8 +50,9 @@ import { handleChatRequest } from "../src/server/chat-handler.ts"; import { abortJob, deliverJobResult, - registerLiveAgent, + noteSteerVisible, recoverStaleJobs, + registerLiveAgent, setJobRespawn, setTurnRunner, settleSteers, @@ -991,6 +992,22 @@ describe("Wave C: live steering handoff + settlement reconciliation", () => { assert.equal(jobStore.getJob(jobId)!.status, "done"); }); + it("noteSteerVisible flips the job at the visibility seam — before any settle (owner ruling 2026-09-26)", async () => { + // The dock fix: the marker reaching the learner's view (wire + // message_end / write-ahead persist) flips the job NOW — the + // delivering turn may keep teaching for minutes. Idempotent with + // the settle-path finalize. + const { chatId, jobId, steer } = setup(); + assert.equal(jobStore.getJob(jobId)!.status, "running"); + noteSteerVisible(steer); + assert.equal(jobStore.getJob(jobId)!.status, "done", "flipped at the seam, no turn settled"); + noteSteerVisible(steer); // the settle-path flip re-runs — no-op + assert.equal(jobStore.getJob(jobId)!.status, "done"); + // A probe-handoff (no jobId) is a harmless no-op. + const handoff = buildProbeHandoffSteer(chatId); + noteSteerVisible(handoff); + }); + it("the idle wake carries EVERY pending marker in ONE turn", async () => { const { chatId, steer } = setup(); const job2 = jobStore.createJob({ kind: "research", chatId, toolCallId: "call-2" });