refactor: drop the tests and comment lines that were not earning their place

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-21 00:02:30 +00:00 · 2026-09-17 11:23:08 +02:00
parent 549c6d7907
commit e8b2dd0048
7 changed files with 35 additions and 92 deletions
@@ -17,11 +17,10 @@
 		/** Identifies the run, so a reused viewer lands on the new one's output. */
 		runKey?: string
 		/**
-		 * How to render an output that is not text. An `output_schema` makes `output`
-		 * an object, and the right rendering for it is whatever the result viewer
-		 * would do with that object on its own — a table for rows, the file viewer
-		 * for an S3 object. Passed in rather than imported so this component does not
-		 * have to reach back into the viewer that renders it.
+		 * How to render an output that is not text: whatever the result viewer would do
+		 * with that object on its own, a table for rows or the file viewer for an S3
+		 * object. Passed in rather than imported so this does not reach back into the
+		 * viewer rendering it.
 		 */
 		structuredOutput: Snippet<[unknown]>
 	}
@@ -1,14 +1,8 @@
 /**
- * The pane an agent run is rendered in: the nearest ancestor that is a real
- * scroll container.
- *
- * Selection is on `overflow-y` alone, deliberately *not* on whether the element
- * currently overflows. A pane that happens to fit its content is still the pane,
- * and skipping it walks straight past into page chrome — `#content` and `<body>`
- * report `scrollHeight > clientHeight` merely because the document scrolls, so an
- * overflow test picks them and scrolling one drops a whole run page to its
- * footer. Their `overflow-y` is `visible`, which is what actually distinguishes
- * them, and the walk stops at the page's content root either way.
+ * The pane an agent run is rendered in. Selected on `overflow-y` alone, never on
+ * whether the element currently overflows: a pane that fits its content is still
+ * the pane, and an overflow test walks past it into `#content`, which overflows
+ * merely because the document scrolls — scrolling that drops the whole page.
 */
 export function runPane(node: HTMLElement | undefined | null): HTMLElement | undefined {
 	let current = node?.parentElement
@@ -175,16 +175,8 @@ describe('the max-iterations path', () => {
 				result: { messages }
 			}
 		})
-		expect(partial).toBeDefined()
-		expect(buildAgentTrace(partial!)).toEqual([
-			{
-				kind: 'tool',
-				name: 'query_metrics',
-				args: '{"w":"30m"}',
-				result: '{"eu-central-1":0.184}',
-				jobId: '0199-job'
-			},
-			{ kind: 'assistant', content: 'eu-central-1 is down.', sources: undefined }
-		])
+		// The trace itself is `buildAgentTrace`'s, pinned above; what this path can
+		// lose is the tags it reads, and an untagged conversation traces to nothing.
+		expect(buildAgentTrace(partial ?? [])).toHaveLength(2)
 	})
 })
@@ -2,14 +2,10 @@ import type { WebSearchSource } from './copilot/chat/shared'
 import type { AgentMessage } from './aiAgentResult'

 /**
- * One entry in the trace of an agent run. Built from the envelope alone: the tool
- * arguments come from the assistant message that asked for the call, and the
- * result from the `tool` message that answered it, so the trace renders without
- * waiting on any request. A tool's child job is enrichment (logs, duration,
- * whether it succeeded), not what makes the row.
- *
- * The prompt and the user's question are deliberately absent. They are inputs to
- * the step and are shown as inputs; the trace is what the agent did with them.
+ * One entry in the trace of an agent run, built from the envelope alone so it
+ * renders without waiting on any request; a tool's child job is enrichment. The
+ * prompt and the question are deliberately absent: they are the step's inputs and
+ * are shown as inputs, while the trace is what the agent did with them.
 */
 export type AgentTraceEntry =
 	| { kind: 'assistant'; content: string; sources?: WebSearchSource[] }
@@ -111,16 +107,10 @@ export function buildAgentTrace(messages: AgentMessage[]): AgentTraceEntry[] {
 }

 /**
- * Separates the turn that produced the output from the rest of the trace, so a
- * view can render the answer once, under its own heading, with the citations
- * that belong to it.
- *
- * The turn is found by content, and from the end rather than at it. Every turn
- * that produced text is an entry while only one of them became the output, so a
- * run whose last turn returned a tool call and no text leaves its answer sitting
- * mid-trace — and anything that only inspects the final entry renders that answer
- * twice. Searching from the end is what makes two turns of identical text resolve
- * to the later one.
+ * Separates the turn that produced the output from the rest of the trace, so the
+ * answer is rendered once with the citations that belong to it. Found by content
+ * and searched from the end: a run whose last turn returned a tool call leaves its
+ * answer mid-trace, where inspecting only the final entry prints it twice.
 */
 export function splitFinalAnswer(
 	entries: AgentTraceEntry[],
@@ -37,8 +37,6 @@ describe('parseAgentResult', () => {
 		['messages that are not a list', { output: 'a', messages: { role: 'user' } }],
 		['no messages at all', { output: 'a', messages: [] }],
 		['a message without a role', { output: 'a', messages: [{ content: 'ask' }] }],
-		['an array', [envelope]],
-		['a string', 'output'],
 		['null', null]
 	])('rejects %s', (_label, value) => {
 		expect(parseAgentResult(value)).toBeUndefined()
@@ -91,15 +89,6 @@ describe('agent stream', () => {
 		expect(isAgentStream('{"type":"tool_call","function_name":"q"}\n')).toBe(false)
 	})

-	it('folds the token deltas into the answer so far', () => {
-		const { stream } = advanceAgentStream(events, emptyAgentStreamProgress())
-		expect(stream.current).toBe('eu-central-1 is down')
-		expect(stream.reasoning).toBe('checking')
-		expect(stream.entries).toEqual([
-			{ kind: 'tool', callId: 'c1', name: 'query_metrics', running: false, success: true }
-		])
-	})
-
 	// The stream only grows, so each poll must fold in the new lines and re-read
 	// none of the old ones — the reason this is incremental at all.
 	it('resumes where the previous poll stopped', () => {
@@ -51,13 +51,10 @@ function hasRole(message: unknown): boolean {
 }

 /**
- * A job result is whatever its script returned, so a message that passed the
- * shape check still has arbitrary anything underneath. Everything downstream
- * reads these as `AgentMessage`, and a value of the wrong type there throws in
- * the middle of rendering, taking the whole result viewer with it.
- *
- * So the coercion happens once, here: past this point the declared type is the
- * real one, and no reader needs a guard of its own.
+ * A job result is whatever its script returned, so a message that passed the shape
+ * check still has anything underneath. Coerced once here rather than guarded at
+ * each reader: a wrong type reaching one throws mid-render and takes the whole
+ * result viewer down, including the plain error it usually rides on.
 */
 function toAgentMessage(raw: Record<string, unknown>): AgentMessage {
 	const toolCalls = Array.isArray(raw.tool_calls)
@@ -98,22 +95,10 @@ function toAgentMessages(raw: unknown[]): AgentMessage[] {
 }

 /**
- * Recognise the envelope by its shape rather than by a marker key the worker
- * would have to add: sniffing works on runs that already completed, and the
- * envelope is also what a nested agent hands back, where an added key would
- * travel into the parent's conversation.
- *
- * The signature is deliberately closed — no key outside `ENVELOPE_KEYS`, and
- * every message carrying a `role` — so an ordinary result that happens to have
- * an `output` field cannot claim it.
- *
- * It stops short of also requiring a recognised `agent_action`, which would rule
- * out a hand-written script returning this same shape. Not every completed run
- * is guaranteed to tag a message (a run whose provider returns its answer
- * through a structured-output tool leaves the final assistant message untagged),
- * and the two failures are not symmetric: claiming a lookalike costs a viewer
- * one click on the JSON toggle, while rejecting a real agent hides its answer
- * with nothing on screen to say why.
+ * Recognised by shape, not a marker key: sniffing works on completed runs, and an
+ * added key would travel into a parent agent's conversation. Deliberately not also
+ * requiring a tagged `agent_action` — a run answering through a structured-output
+ * tool tags nothing, and hiding a real answer costs more than claiming a lookalike.
 */
 export function parseAgentResult(result: unknown): AgentResult | undefined {
 	if (!isRecord(result)) {
@@ -276,12 +261,10 @@ export function isAgentStream(raw: string): boolean {
 }

 /**
- * Fold the events that arrived since `previous` into the answer so far.
- *
- * Incremental rather than a parse of the whole buffer: the stream only ever
- * grows, a poll can arrive every 50ms, and a `tool_result` event carries the
- * tool's entire output — so re-reading everything each time is quadratic in the
- * number of events with a large constant.
+ * Fold the events that arrived since `previous` into the answer so far. Incremental
+ * rather than a parse of the whole buffer: the stream only grows, a poll can arrive
+ * every 50ms, and a `tool_result` carries the tool's entire output, so re-reading it
+ * all each tick is quadratic with a large constant.
 */
 export function advanceAgentStream(
 	raw: string,
@@ -1,12 +1,8 @@
 /**
- * The mechanics of keeping a growing pane pinned to its end, shared by the AI
- * chat transcript and the agent run viewer.
- *
- * Both need the same non-obvious guard, which is why this is not two copies: a
- * programmatic scroll dispatches its own `scroll` event asynchronously, and if
- * content lands between the call and the event the gap can exceed the threshold
- * for one tick — read naively, that looks like the reader scrolling away and
- * disengages the follow mid-stream.
+ * Keeps a growing pane pinned to its end, for the chat transcript and the agent run
+ * viewer. Shared for one non-obvious guard: a programmatic scroll dispatches its
+ * `scroll` event asynchronously, so content landing in between widens the gap for a
+ * tick, which reads as the reader scrolling away and disengages the follow.
 */

 /**