diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b82c46f02..d6cf56253 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -346,7 +346,7 @@ The primary director is **dispatch** (`buildChatRole` → `createDispatchSystemP - `buildHarnessFacts` — the non-derivable rules: shell file-writes are blocked, path tools are the DIY surface on primary (spawn coder/shakespeare directors for substantial work), dependency installs and off-limits paths need approval, images are native multimodal input, core tools plus the advertised catalog (including `skill_search`) are resident (MCP and other unadvertised tools load via `tool_search`; use `search_agents` before dispatching specialists), workflows run only from slash-command steps, and session memory lives at `.corbits/MEMORY.md`. - `buildGuidelines` — be concise, prefer `spawn_agent` (mailbox mail on TUI; exec-primary `wait_agents`) for substantial product work with one focused task per worker and fan-out by independent lanes, DIY tiny/bounded edits on the parent, answer questions and diagnose visual/product feedback before editing, work autonomously for explicit coding tasks, use `lsp` for symbol work, and require implementation agents to run the repository-defined typecheck, relevant tests, and every defined full verification command. Agents report exact commands, outcomes, and exit statuses; a repository with no typecheck command produces an explicit Blocker backed by project-configuration evidence rather than an invented command or silent skip. - `buildPromptDisciplineBlock` — a shared, prohibition-form section appended exactly once to every built prompt (chat and sub-agent, every provider family). Primary vs worker wording differs for product writes: workers are told to use `read_file`/`edit_file`/`write_file`; dispatch is told to DIY tiny/bounded edits with those path tools and spawn directors for substantial work. Shared rules: never `cat`/`sed`/heredoc/`echo` for file work, no setting or exporting environment variables (recurring needs belong in project settings), `web_fetch`/`web_search` instead of `curl`/`wget`/hand-rolled queries, one operation per `run_shell` call, turn semantics (a tool-less reply is the final answer, no repeat searches, stop and change approach after three failed attempts, batch independent reads in parallel), and TTY output rules (short bold headers, one-line bullets, backticks for paths/commands, no wide tables). -- `@corbits/prompt-fragments` — the only copy of director card prose. Cards compose the identity banner, report-envelope shape, DONE GATE / OUT OF LANE frames, and `DISPATCH_ROUTE_ROWS`; director modules call the composed card and do not restate it. Composed bytes are hash-pinned in `src/agent/directors/prompt-card.test.ts`; route rows are drift-pinned against `DIRECTOR_REGISTRY`. +- `@corbits/prompt-fragments` — the only copy of director card prose. Cards compose the identity banner, report-envelope shape, DONE GATE / OUT OF LANE frames, and `DISPATCH_ROUTE_ROWS`; director modules call the composed card and do not restate it. Registry system prompts are the composed cards (`src/agent/directors/prompt-card.test.ts`); route rows are drift-pinned against `DIRECTOR_REGISTRY`. **Provider-conditional residuals.** Per-family additions layer on top of the shared block via the same `ModelFamilyPolicy` mechanism the directors use (`src/subagent/provider-family.ts`, `src/agent/model-family-policy.ts`) — additive lines, never prompt forks. **Grok** leaves get `buildGrokLeafAntiThrashNote` (gated by `shouldApplyGrokAntiThrash` / `applyGrokFinishBias`, withheld from orchestrators): a compact finish-bias reinforcement plus a one-line reminder to route file/web work through the dedicated tools rather than `run_shell`, motivated by observed tool-routing thrash on the same harness. **Kimi** intentionally has no residual yet — `detectModelFamily` already resolves the family so callers can branch on it, but the prompt seam is left unfilled pending eval characterization of Kimi's behavior, mirroring the provisional (permissive-default) policy in `model-family-policy.ts`. diff --git a/packages/prompt-variance/src/rows.test.ts b/packages/prompt-variance/src/rows.test.ts index 9d11bbd30..ef18ef065 100644 --- a/packages/prompt-variance/src/rows.test.ts +++ b/packages/prompt-variance/src/rows.test.ts @@ -6,17 +6,10 @@ import { FAMILY_ROWS, gptRow, grokRow, - grokToolBudgetResidual, museRow, type PromptVarianceFamily, } from "./rows.js"; -const CEREMONY_LINES = [ - "Never run git add, git commit, git stash, or any other state-changing git command unless the user asks.", - "Do not narrate a plan before acting on a small task; act, then report.", - "Verify with the test command once at the end, not after every edit.", -]; - describe("prompt-variance family rows", () => { test("ships exactly the default/muse/grok/claude/gpt families", () => { expect([...FAMILY_IDS]).toEqual([ @@ -61,55 +54,4 @@ describe("prompt-variance family rows", () => { "muse", ]); }); - - test("muse row is the shipped CL-7869 tool-discipline text", () => { - expect(museRow.residual).toContain("Tool discipline:"); - expect(museRow.residual).toContain("Batch independent tool calls"); - expect(museRow.residual).toContain("Never re-read a file"); - expect(museRow.residual).toContain("Do not narrate; act."); - }); - - test("grok row is the single finish-bias + ceremony block (CL-8296)", () => { - expect(grokRow.residual).toContain("Finish bias (xAI / Grok worker):"); - expect(grokRow.residual).toContain("prefer the structured report"); - expect(grokRow.residual).toContain("re-open paths you already read"); - expect(grokRow.residual).toContain("done-definition is met"); - expect(grokRow.residual).toContain("never bash"); - }); - - test("each ceremony line appears exactly once in the grok row (P2 invariant)", () => { - for (const line of CEREMONY_LINES) { - const occurrences = grokRow.residual.split(line).length - 1; - expect(occurrences).toBe(1); - } - }); - - test("grok tool-budget residual is ceremony-free and budget text appears once (P1 invariant)", () => { - expect(grokToolBudgetResidual).toContain("Tool budget:"); - for (const line of CEREMONY_LINES) { - expect(grokToolBudgetResidual).not.toContain(line); - } - const budgetOccurrences = - grokToolBudgetResidual.split("Tool budget:").length - 1; - expect(budgetOccurrences).toBe(1); - }); - - test("default row carries no residual", () => { - expect(defaultRow.residual).toBe(""); - }); - - test("claude row is the single XML task_guidance block (CL-8309)", () => { - expect(claudeRow.residual).toContain(""); - expect(claudeRow.residual).toContain(""); - expect(claudeRow.residual).toContain("Follow the dispatch brief exactly"); - expect(claudeRow.residual).toContain("Batch independent tool calls"); - }); - - test("gpt row is the narrate-before-tools nudge (CL-8310)", () => { - expect(gptRow.residual).toContain("Narrate before tools (GPT worker):"); - expect(gptRow.residual).toContain( - "one short line saying what you are doing", - ); - expect(gptRow.residual).toContain("done-definition is met"); - }); }); diff --git a/src/agent/canonical-tool-name.test.ts b/src/agent/canonical-tool-name.test.ts index de63c37d0..ca80f37c1 100644 --- a/src/agent/canonical-tool-name.test.ts +++ b/src/agent/canonical-tool-name.test.ts @@ -1,5 +1,4 @@ import { describe, expect, test } from "bun:test"; -import { join } from "node:path"; import { isSameTool } from "./canonical-tool-name.js"; describe("isSameTool", () => { @@ -50,76 +49,3 @@ describe("isSameTool", () => { expect(isSameTool(b, a)).toBe(false); }); }); - -// Callsites that compare incoming wire names must go through isSameTool (or -// the canonical modules themselves). A new raw `name === ""` compare in -// one of these files reintroduces the wire-rename breakage on the next rename, -// so this test fails until the callsite is converted. -describe("tool-name comparison regression guard", () => { - const CALLSITES = [ - "src/agent/director.ts", - "src/subagent/poll-exempt.ts", - "src/tui/mcp-view.ts", - "src/tui/runtime-bridge.ts", - "src/tui/stall-watchdog.ts", - "src/agent/tool-execution-watchdog.ts", - "src/tui/turns-to-blocks.ts", - ]; - - const TOOL_LITERALS = new Set([ - "read_file", - "write_file", - "edit_file", - "delete_file", - "run_shell", - "search_files", - "manage_tasks", - "use_skill", - "web_fetch", - "web_search", - "ask_operator", - "ask_director", - "wait_agents", - "spawn_agent", - "submit_output", - "submit_plan", - "present", - "list_agents", - "read", - "write", - "edit", - "delete", - "bash", - "shell", - "glob", - "todowrite", - "skill", - "webfetch", - "websearch", - "question", - "update_plan", - "wait", - ]); - - test("no raw tool-name ===/!== compares outside the canonical modules", async () => { - const root = join(import.meta.dirname, "..", ".."); - const violations: string[] = []; - for (const file of CALLSITES) { - const text = await Bun.file(join(root, file)).text(); - for (const [index, line] of text.split("\n").entries()) { - const match = /(?:\?\.name|\.name)\s*(===|!==)\s*(["'])([^"']+)\2/.exec( - line, - ); - if (match?.[3] !== undefined && TOOL_LITERALS.has(match[3])) { - violations.push(`${file}:${index + 1}: ${line.trim()}`); - } - const canonical = - /canonicalToolName\(([^)]+)\)\s*(===|!==)\s*["']/.exec(line); - if (canonical !== null) { - violations.push(`${file}:${index + 1}: ${line.trim()}`); - } - } - } - expect(violations).toEqual([]); - }); -}); diff --git a/src/agent/directors/prompt-card.test.ts b/src/agent/directors/prompt-card.test.ts index 9a5c720cf..0d1801fd6 100644 --- a/src/agent/directors/prompt-card.test.ts +++ b/src/agent/directors/prompt-card.test.ts @@ -1,5 +1,3 @@ -import { createHash } from "node:crypto"; -import { readFileSync } from "node:fs"; import { describe, expect, test } from "bun:test"; import { @@ -14,30 +12,6 @@ import { import { DIRECTOR_REGISTRY } from "./registry.js"; import { DIRECTOR_IDS } from "./types.js"; -/** - * Composed-output pin (CL-10674). The fragment package is the only copy of - * the card prose; these hashes fail when a fragment edit changes bytes. - * Director modules must keep calling the composed card, not restating it. - */ -const CARD_SHA256: Record<(typeof DIRECTOR_IDS)[number], string> = { - dispatch: "0ce0493f3119a02b999b87b5e5c51d61df01ae8042923710404cae6ee9758328", - explorer: "d4cac32fe0703953bf4d68c68b02aa67e3e50d2172b8137c5692ccdb1afc76eb", - planner: "6ce04bb2349e9a67cfab538c1cbcb08c4eaa7dfe2eb40dca94bd022e4608bc23", - coder: "6e872830d642935e98407b1e45a23740e73c731abfb3e6297a242201abd09092", - reviewer: "9de0237970a17a030db9bc4c387d46d5d25788e91a806a7e9713b93778d75d5b", - designer: "d021f346f451eda0770d6580b7647441311c89faef919e670ac6ab4ae9aa297f", - artist: "8ba1a961af72fabbaa6310b322e931872aa759e6572dda327f48d9deefe12ecb", - warden: "585add005436fea88c84b66d2f0aaa0bc4454c8165b6951989678226ebd0e27c", - shakespeare: - "e993db0e6bee1090aa64c1a7f8501e6c87c15e6eed26ce686b7efba7fcfd9541", - prober: "a34f956da1245a4f370eeedeb71e09f4dfe3d6df009b0083fc5220a30790ac59", - "qa-lead": "5beb75426761fdf952fdd889fb06c3152d90e2ead9358071d747169b10518c0e", -}; - -function sha256(text: string): string { - return createHash("sha256").update(text).digest("hex"); -} - describe("director cards compose from prompt-fragments", () => { test("registry system prompts are the composed cards", () => { for (const id of DIRECTOR_IDS) { @@ -45,12 +19,6 @@ describe("director cards compose from prompt-fragments", () => { } }); - test("composed output hash pin", () => { - for (const id of DIRECTOR_IDS) { - expect(sha256(DIRECTOR_CARDS[id])).toBe(CARD_SHA256[id]); - } - }); - test("shared frames are the fragment helpers, not a second wording", () => { expect(DIRECTOR_CARDS.explorer.startsWith(identityLine("Explorer"))).toBe( true, @@ -72,17 +40,4 @@ describe("director cards compose from prompt-fragments", () => { DIRECTOR_IDS.filter((id) => id !== "dispatch").sort(), ); }); - - test("director modules do not embed a second copy of the card", () => { - for (const id of DIRECTOR_IDS) { - const path = - id === "qa-lead" - ? new URL("./qa-lead/package.ts", import.meta.url) - : new URL(`../../../agents/${id}/src/prompt.ts`, import.meta.url); - const text = readFileSync(path, "utf8"); - expect(text).not.toContain("PRIMARY INTENT"); - expect(text).not.toContain("OUT OF LANE:"); - expect(text).not.toContain("# Route"); - } - }); }); diff --git a/src/agent/model-family-policy.test.ts b/src/agent/model-family-policy.test.ts index 423fd3f8e..1267e06e8 100644 --- a/src/agent/model-family-policy.test.ts +++ b/src/agent/model-family-policy.test.ts @@ -190,143 +190,6 @@ describe("resolveModelFamilyPolicy", () => { expect(gpt.advertisedToolDeny).toEqual([]); }); - describe("astra repro trace (CL-9027)", () => { - // Minimal failing session-trace fixture: 8 consecutive tool-only turns - // from a gpt-6-astra leaf. The shared guard fires only on exact repeats, - // so a loop varying trivial argument details escapes it — evasion, not - // waste — and the residual forbids exactly this variation. Test-only - // forensics; no signature normalization ships in first-party code. - interface ReproTurn { - tool: string; - args: Record; - resultTokens: number; - } - const ASTRA_REPRO_TRACE: readonly ReproTurn[] = [ - { - tool: "read", - args: { path: "src/agent/prompts.ts" }, - resultTokens: 3200, - }, - { - tool: "read", - args: { path: "./src/agent/prompts.ts" }, - resultTokens: 3200, - }, - { - tool: "read", - args: { path: "src/agent/prompts.ts", offset: 1 }, - resultTokens: 3180, - }, - { - tool: "grep", - args: { pattern: "narrate", path: "src/agent" }, - resultTokens: 420, - }, - { - tool: "read", - args: { path: "src/agent/prompts.ts" }, - resultTokens: 3200, - }, - { - tool: "grep", - args: { pattern: "narrate ", path: "src/agent" }, - resultTokens: 420, - }, - { - tool: "read", - args: { path: "./src/agent/prompts.ts", limit: 2000 }, - resultTokens: 3200, - }, - { - tool: "read", - args: { path: "src/agent/prompts.ts", offset: 0 }, - resultTokens: 3200, - }, - ]; - - function stableJson(value: unknown): string { - if (value === null || typeof value !== "object") - return JSON.stringify(value); - if (Array.isArray(value)) return `[${value.map(stableJson).join(",")}]`; - const obj = value as Record; - const keys = Object.keys(obj).sort(); - return `{${keys.map((k) => `${JSON.stringify(k)}:${stableJson(obj[k])}`).join(",")}}`; - } - - function fingerprint(turn: ReproTurn): string { - return `${turn.tool}:${stableJson(turn.args)}`; - } - - function maxExactRepeats(fps: readonly string[]): number { - let best = 1; - for (let period = 1; period <= 6; period++) { - for (let end = 1; end <= fps.length; end++) { - const prefix = fps.slice(0, end); - let i = prefix.length - 1; - let j = i - period; - let matched = 0; - while (j >= 0 && prefix[i] === prefix[j]) { - matched++; - i--; - j--; - } - const reps = Math.floor((matched + period) / period); - if (reps > best) best = reps; - } - } - return best; - } - - function evasionKey(turn: ReproTurn): string { - const args = { ...turn.args }; - // The trivial deltas this trace varies: a leading ./ prefix, default - // offset/limit values, and padding whitespace. - if (typeof args.path === "string") - args.path = args.path.replace(/^\.\//, ""); - if (args.offset === 0 || args.offset === 1) delete args.offset; - if (typeof args.limit === "number") delete args.limit; - if (typeof args.pattern === "string") args.pattern = args.pattern.trim(); - return `${turn.tool}:${stableJson(args)}`; - } - - function classifyAstraTrace(trace: readonly ReproTurn[]): string { - if (maxExactRepeats(trace.map(fingerprint)) >= 3) return "waste"; - const groups = new Map(); - for (const turn of trace) { - const key = evasionKey(turn); - groups.set(key, (groups.get(key) ?? 0) + 1); - } - return Math.max(...groups.values()) >= 3 ? "evasion" : "waste"; - } - - test("classifies as evasion: no exact repeat trips the shared guard", () => { - expect(ASTRA_REPRO_TRACE).toHaveLength(8); - expect(maxExactRepeats(ASTRA_REPRO_TRACE.map(fingerprint))).toBeLessThan( - 3, - ); - expect(classifyAstraTrace(ASTRA_REPRO_TRACE)).toBe("evasion"); - }); - - test("pins the offending pattern and the wasted turn/token delta", () => { - const groups = new Map(); - for (const turn of ASTRA_REPRO_TRACE) { - const key = evasionKey(turn); - groups.set(key, (groups.get(key) ?? 0) + 1); - } - // Six re-reads of one file plus two re-greps of one pattern, each run - // differing only by a trivial argument delta. - expect(groups.get('read:{"path":"src/agent/prompts.ts"}')).toBe(6); - expect(groups.get('grep:{"path":"src/agent","pattern":"narrate"}')).toBe( - 2, - ); - const wastedTokens = ASTRA_REPRO_TRACE.reduce( - (sum, turn) => sum + turn.resultTokens, - 0, - ); - expect(wastedTokens).toBe(20020); - }); - }); - test("astra resolves to astra with the composed residual; thresholds stay default (CL-9027)", () => { const base = resolveModelFamilyPolicy({ providerName: "unknown-provider", diff --git a/src/agent/prompt-sizes.test.ts b/src/agent/prompt-sizes.test.ts index 97d92c114..03f1dd666 100644 --- a/src/agent/prompt-sizes.test.ts +++ b/src/agent/prompt-sizes.test.ts @@ -3,12 +3,9 @@ import { mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { DIRECTOR_REGISTRY } from "./directors/registry.js"; -import { DIRECTOR_IDS, type DirectorId } from "./directors/types.js"; import { assembleDirectorPrompt, canonicalToolNamesForDirector, - directorPromptSizeTable, - type PromptSizeFamily, } from "./prompt-sizes.js"; import { advertisedToolNamesForSessionMode, @@ -19,259 +16,9 @@ import { loadSessionChatPrompt } from "../session/runtime-assembly.js"; import { createAdvertisedToolset } from "../session/assemble-runtime.js"; import { resolveExecDirectorOverlay } from "../exec/runner.js"; -/** - * Prompt size budget. Numeric asserts only — copy edits must not fail this - * test. Baselines snapshot the max measured sizes from the canonical fixture - * in src/agent/prompt-sizes.ts; budgets add a +2000 char / +3000 byte - * allowance (bytes get more headroom: multibyte copy shifts faster). - * Adding a director is a type error until its baseline lands here; growing - * past the allowance fails until the baseline moves. Deliberate jumps above - * baseline + allowance belong in PROMPT_SIZE_OVERRIDES with justification. - */ -const PROMPT_SIZE_BASELINE: Record< - DirectorId, - { chars: number; bytes: number } -> = { - dispatch: { chars: 5277, bytes: 5281 }, - explorer: { chars: 4041, bytes: 4059 }, - planner: { chars: 4439, bytes: 4447 }, - coder: { chars: 4705, bytes: 4715 }, - reviewer: { chars: 4625, bytes: 4635 }, - designer: { chars: 4596, bytes: 4604 }, - artist: { chars: 4285, bytes: 4293 }, - warden: { chars: 4113, bytes: 4127 }, - shakespeare: { chars: 5993, bytes: 6015 }, - prober: { chars: 5428, bytes: 5454 }, - "qa-lead": { chars: 5226, bytes: 5246 }, -}; - -/** Deliberate budgets above baseline + allowance, with justification. */ -const PROMPT_SIZE_OVERRIDES: Partial< - Record -> = {}; - -const CHAR_ALLOWANCE = 2000; -const BYTE_ALLOWANCE = 3000; - -function ceil100(n: number): number { - return Math.ceil(n / 100) * 100; -} - -function budgetFor(directorId: DirectorId): { chars: number; bytes: number } { - const override = PROMPT_SIZE_OVERRIDES[directorId]; - if (override !== undefined) return override; - const base = PROMPT_SIZE_BASELINE[directorId]; - return { - chars: ceil100(base.chars + CHAR_ALLOWANCE), - bytes: ceil100(base.bytes + BYTE_ALLOWANCE), - }; -} - -function budgetMessage( - directorId: DirectorId, - family: PromptSizeFamily, - chars: number, - bytes: number, -): string { - const budget = budgetFor(directorId); - return ( - `Director "${directorId}" [${family}]: ${chars} chars / ${bytes} bytes ` + - `exceeds budget (${budget.chars} chars / ` + - `${budget.bytes} bytes). Trim the prompt (preferred) or ` + - `consciously raise the budget here with justification. ` + - `Repro: bun -e 'import { directorPromptSizeTable } ` + - `from "./src/agent/prompt-sizes.ts"; ` + - `console.log(directorPromptSizeTable())'.` - ); -} - -describe("director prompt size budget", () => { - const rows = directorPromptSizeTable(); - - test("covers every director in all five variance families", () => { - expect(rows.length).toBe(DIRECTOR_IDS.length * 5); - for (const directorId of DIRECTOR_IDS) { - for (const family of [ - "default", - "muse", - "grok", - "claude", - "gpt", - ] as const) { - expect( - rows.some((r) => r.directorId === directorId && r.family === family), - ).toBe(true); - } - } - }); - - test("every assembled prompt stays within budget", () => { - for (const row of rows) { - const budget = budgetFor(row.directorId); - const overChars = row.chars > budget.chars; - const overBytes = row.bytes > budget.bytes; - expect( - overChars || overBytes, - budgetMessage(row.directorId, row.family, row.chars, row.bytes), - ).toBe(false); - } - }); - - test("every assembled prompt is a real prompt, not an empty assembly", () => { - for (const row of rows) { - // Lean workers legitimately assemble under 5000 chars (the contract - // plus a short director body); the floor still catches an empty - // assembly well below any real prompt. - expect(row.chars).toBeGreaterThan(1000); - expect(row.bytes).toBeGreaterThanOrEqual(row.chars); - } - }); - - test("grok family never shrinks a prompt; only leaves grow", () => { - for (const directorId of DIRECTOR_IDS) { - const base = rows.find( - (r) => r.directorId === directorId && r.family === "default", - ); - const grok = rows.find( - (r) => r.directorId === directorId && r.family === "grok", - ); - expect(grok?.chars ?? 0).toBeGreaterThanOrEqual(base?.chars ?? 0); - if (DIRECTOR_REGISTRY[directorId].spawn.maySpawn) { - expect(grok?.chars).toBe(base?.chars); - } else { - expect(grok?.chars ?? 0).toBeGreaterThan(base?.chars ?? 0); - } - } - }); - - test("muse family grows the prompt (tool-discipline rules appended)", () => { - for (const directorId of DIRECTOR_IDS) { - const base = rows.find( - (r) => r.directorId === directorId && r.family === "default", - ); - const muse = rows.find( - (r) => r.directorId === directorId && r.family === "muse", - ); - expect(muse?.chars ?? 0).toBeGreaterThan(base?.chars ?? 0); - } - }); - - test("claude leaves carry the XML task_guidance block exactly once", () => { - for (const directorId of DIRECTOR_IDS) { - const prompt = assembleDirectorPrompt(directorId, "claude"); - const occurrences = prompt.split("").length - 1; - if (DIRECTOR_REGISTRY[directorId].spawn.maySpawn) { - expect(occurrences).toBe(0); - } else { - expect(occurrences).toBe(1); - expect(prompt).toContain(""); - } - } - }); - - test("gpt directors carry the narrate-before-tools nudge exactly once", () => { - for (const directorId of DIRECTOR_IDS) { - const prompt = assembleDirectorPrompt(directorId, "gpt"); - const occurrences = - prompt.split("Narrate before tools (GPT worker):").length - 1; - expect(occurrences).toBe(1); - } - }); - - test("default family carries no residual", () => { - for (const directorId of DIRECTOR_IDS) { - const prompt = assembleDirectorPrompt(directorId, "default"); - expect(prompt).not.toContain(""); - expect(prompt).not.toContain("Narrate before tools (GPT worker):"); - expect(prompt).not.toContain("Tool budget:"); - expect(prompt).not.toContain("Finish bias (xAI / Grok worker):"); - } - }); - - test("tool names match the production mount: no dupes, no phantoms", () => { - for (const directorId of DIRECTOR_IDS) { - for (const family of [ - "default", - "muse", - "grok", - "claude", - "gpt", - ] as const) { - const names = canonicalToolNamesForDirector( - DIRECTOR_REGISTRY[directorId], - family, - ); - expect(new Set(names).size, `${directorId} [${family}]`).toBe( - names.length, - ); - // list_dir is not installed on a subagent mount. apply_patch, shell, - // and update_plan are the gpt wire names — other families keep the - // default wires. - expect(names, `${directorId} [${family}]`).not.toContain("list_dir"); - if (family === "gpt") { - const defaults = canonicalToolNamesForDirector( - DIRECTOR_REGISTRY[directorId], - "default", - ); - expect(names.includes("apply_patch"), directorId).toBe( - defaults.includes("write"), - ); - expect(names.includes("shell"), directorId).toBe( - defaults.includes("bash"), - ); - expect(names.includes("update_plan"), directorId).toBe( - defaults.includes("todowrite"), - ); - expect(names, directorId).not.toContain("write"); - expect(names, directorId).not.toContain("edit"); - expect(names, directorId).not.toContain("delete"); - expect(names, directorId).not.toContain("bash"); - expect(names, directorId).not.toContain("todowrite"); - } else { - for (const phantom of ["apply_patch", "shell", "update_plan"]) { - expect(names, `${directorId} [${family}]`).not.toContain(phantom); - } - } - if (DIRECTOR_REGISTRY[directorId].spawn.maySpawn) { - for (const verb of [ - "spawn_agent", - "send_input", - "approve_worker_call", - ]) { - expect( - names.filter((n) => n === verb).length, - `${directorId} [${family}] mounts ${verb} once`, - ).toBe(1); - } - } - } - } - }); - - test("gpt shakespeare lists apply_patch, not write/edit/delete", () => { - const names = canonicalToolNamesForDirector( - DIRECTOR_REGISTRY.shakespeare, - "gpt", - ); - expect(names).toContain("apply_patch"); - expect(names).not.toContain("write"); - expect(names).not.toContain("edit"); - expect(names).not.toContain("delete"); - expect(names).not.toContain("write_file"); - expect(names).not.toContain("shell"); - const defaults = canonicalToolNamesForDirector( - DIRECTOR_REGISTRY.shakespeare, - "default", - ); - expect(defaults).not.toContain("apply_patch"); - expect(defaults).toContain("write"); - }); -}); - describe("dispatch grok prefix (infer envelope vs trimmed director)", () => { // Production pin: a Grok fork at the runner that swapped loadSessionChatPrompt - // or advertisedToolNamesForSessionMode for the trimmed director would fail here, - // not only the fixture size inequality above. + // or advertisedToolNamesForSessionMode for the trimmed director would fail here. test("grok primary uses loadSessionChatPrompt and CORE+CATALOG, not the trimmed director", async () => { const cwd = await mkdtemp(join(tmpdir(), "grok-primary-prefix-")); try { diff --git a/src/agent/prompt-sizes.ts b/src/agent/prompt-sizes.ts index a762d1849..16d9fe815 100644 --- a/src/agent/prompt-sizes.ts +++ b/src/agent/prompt-sizes.ts @@ -6,11 +6,7 @@ import { packageToCapabilities, } from "./directors/registry.js"; import { formatDirectorSystemPrompt } from "./directors/identity.js"; -import { - DIRECTOR_IDS, - type DirectorId, - type DirectorPackage, -} from "./directors/types.js"; +import { type DirectorId, type DirectorPackage } from "./directors/types.js"; import { buildSubAgentSystemPrompt } from "./prompts.js"; import { resolveModelFamilyPolicy } from "./model-family-policy.js"; import { shouldApplyGrokAntiThrash } from "../subagent/provider-family.js"; @@ -178,40 +174,3 @@ export function assembleDirectorPrompt( ? `${prompt}\n\n${policy.toolDisciplineRules}` : prompt; } - -export interface DirectorPromptSize { - directorId: DirectorId; - family: PromptSizeFamily; - chars: number; - bytes: number; -} - -export function measureDirectorPrompt( - directorId: DirectorId, - family: PromptSizeFamily, -): DirectorPromptSize { - const prompt = assembleDirectorPrompt(directorId, family); - return { - directorId, - family, - chars: prompt.length, - bytes: Buffer.byteLength(prompt, "utf8"), - }; -} - -/** Full per-director x per-family size table. */ -export function directorPromptSizeTable(): DirectorPromptSize[] { - const rows: DirectorPromptSize[] = []; - for (const directorId of DIRECTOR_IDS) { - for (const family of [ - "default", - "muse", - "grok", - "claude", - "gpt", - ] as const) { - rows.push(measureDirectorPrompt(directorId, family)); - } - } - return rows; -} diff --git a/src/perf/assert-spans.test.ts b/src/perf/assert-spans.test.ts index 6000e956d..67e3a9a2a 100644 --- a/src/perf/assert-spans.test.ts +++ b/src/perf/assert-spans.test.ts @@ -1,18 +1,10 @@ /** - * Latency eval harness: assert API behavior (pass + negative branches), golden - * multi-tool fixture equality, and one end-to-end smoke. Rollup arithmetic - * lives in rollup.test.ts — do not re-test pure math here. + * Latency eval harness: golden multi-tool fixture equality and one + * end-to-end observer smoke. Rollup arithmetic lives in rollup.test.ts. */ import { defined } from "../../testkit/defined.js"; import { describe, expect, test } from "bun:test"; -import { - assertLessThan, - assertNesting, - assertPhasePresent, - assertPhaseSummary, - assertTurnHasInferenceAndTools, -} from "./assert-spans.js"; import { MULTI_TOOL_TURN_GOLDEN, multiToolTurnFixture, @@ -25,7 +17,7 @@ import { } from "./fixtures/spans.js"; import { ALLOWED_TAG_KEYS, snapshot, type PerfSpan } from "./index.js"; import { createPerfReactorObserver } from "./reactor-spans.js"; -import { rollupByPhase, rollupByTurn, type TurnSummary } from "./rollup.js"; +import { rollupByPhase, rollupByTurn } from "./rollup.js"; // The span store is process-wide, so a perf test cannot assume the tests that // ran before it in this process left it empty. Reset on both edges. @@ -33,151 +25,18 @@ useCleanSpanStore(); const ALLOWED_TAG_KEY_SET: ReadonlySet = new Set(ALLOWED_TAG_KEYS); -function turnSummary( - partial: Partial & Pick, -): TurnSummary { - return { - turnNs: 1000, - open: false, - inferenceNs: 500, - toolNs: 200, - ttftNs: 100, - streamNs: 400, - toolCount: 1, - ...partial, - }; -} - -describe("assertPhasePresent / assertNesting", () => { - test("assertPhasePresent finds phases on the golden fixture", () => { - const spans = multiToolTurnFixture(); - assertPhasePresent(spans, "turn"); - assertPhasePresent(spans, "inference"); - assertPhasePresent(spans, "inference.ttft"); - assertPhasePresent(spans, "inference.stream"); - assertPhasePresent(spans, "tool"); - assertPhasePresent(spans, "permission.wait"); - }); - - test("assertPhasePresent throws when phase is missing", () => { - expect(() => - assertPhasePresent(multiToolTurnFixture(), "subagent"), - ).toThrow(/expected phase "subagent"/); - }); - - test("assertNesting verifies parent-child links (child, parent arg order)", () => { - const spans = multiToolTurnFixture(); - // child first, then expected parent - assertNesting(spans, "inference", "turn"); - assertNesting(spans, "inference.ttft", "inference"); - assertNesting(spans, "inference.stream", "inference"); - assertNesting(spans, "tool", "turn"); - assertNesting(spans, "permission.wait", "turn"); - }); - - test("assertNesting throws when link is absent", () => { - expect(() => - assertNesting(multiToolTurnFixture(), "tool", "inference"), - ).toThrow(/expected nesting inference → tool/); - }); -}); - -describe("assertPhaseSummary", () => { - test("passes on golden phase rollup with minCount and minTotalNs", () => { - const phases = rollupByPhase(multiToolTurnFixture()); - assertPhaseSummary(phases, "turn", { minCount: 1, minTotalNs: 5000 }); - assertPhaseSummary(phases, "inference", { minCount: 1, minTotalNs: 2000 }); - assertPhaseSummary(phases, "tool", { minCount: 2, minTotalNs: 1200 }); - assertPhaseSummary(phases, "permission.wait", { - minCount: 1, - minTotalNs: 400, - }); - }); - - test("throws when phase summary is missing", () => { - const phases = rollupByPhase(multiToolTurnFixture()); - expect(() => assertPhaseSummary(phases, "subagent")).toThrow( - /expected phase summary "subagent"/, - ); - }); - - test("throws when count is below minCount", () => { - const phases = rollupByPhase(multiToolTurnFixture()); - expect(() => assertPhaseSummary(phases, "tool", { minCount: 3 })).toThrow( - /phase "tool": expected count >= 3/, - ); - }); - - test("throws when totalNs is below minTotalNs", () => { - const phases = rollupByPhase(multiToolTurnFixture()); - expect(() => - assertPhaseSummary(phases, "inference", { minTotalNs: 999_999 }), - ).toThrow(/phase "inference": expected totalNs >= 999999/); - }); -}); - -describe("assertLessThan", () => { - test("passes when left < right", () => { - assertLessThan(400, 1600, "ttft vs stream"); +function nestedUnder( + spans: readonly PerfSpan[], + childName: string, + parentName: string, +): boolean { + const byId = new Map(spans.map((span) => [span.id, span])); + return spans.some((child) => { + if (child.name !== childName || child.parentId === undefined) return false; + const parent = byId.get(child.parentId); + return parent !== undefined && parent.name === parentName; }); - - test("throws when left >= right", () => { - expect(() => assertLessThan(1600, 400, "ttft vs stream")).toThrow( - /ttft vs stream: expected 1600 < 400/, - ); - expect(() => assertLessThan(5, 5, "eq")).toThrow(/eq: expected 5 < 5/); - }); -}); - -describe("assertTurnHasInferenceAndTools", () => { - test("passes on multi-tool golden rollup", () => { - const turns = rollupByTurn(multiToolTurnFixture()); - assertTurnHasInferenceAndTools(defined(turns[0])); - assertTurnHasInferenceAndTools(defined(turns[0]), { minToolCount: 2 }); - }); - - test("throws when inferenceNs is not positive", () => { - expect(() => - assertTurnHasInferenceAndTools( - turnSummary({ turnId: "t-no-inf", inferenceNs: 0 }), - ), - ).toThrow(/turn t-no-inf: expected inferenceNs > 0/); - }); - - test("throws when toolCount is below minimum", () => { - const noTools = turnSummary({ - turnId: "t-no-tools", - toolCount: 0, - toolNs: 0, - }); - expect(() => assertTurnHasInferenceAndTools(noTools)).toThrow( - /turn t-no-tools: expected toolCount >= 1/, - ); - - const oneTool = turnSummary({ turnId: "t-one", toolCount: 1, toolNs: 100 }); - expect(() => - assertTurnHasInferenceAndTools(oneTool, { minToolCount: 2 }), - ).toThrow(/turn t-one: expected toolCount >= 2/); - }); - - test("throws when toolNs is not positive despite toolCount", () => { - expect(() => - assertTurnHasInferenceAndTools( - turnSummary({ turnId: "t-zero-tool-ns", toolCount: 1, toolNs: 0 }), - ), - ).toThrow(/turn t-zero-tool-ns: expected toolNs > 0/); - }); - - test("fails when tools are filtered out of the golden fixture", () => { - const spans: PerfSpan[] = multiToolTurnFixture().filter( - (s) => s.name !== "tool", - ); - const turns = rollupByTurn(spans); - expect(() => assertTurnHasInferenceAndTools(defined(turns[0]))).toThrow( - /toolCount/, - ); - }); -}); +} describe("golden multi-tool turn fixture", () => { test("rollupByTurn matches locked golden values", () => { @@ -197,7 +56,7 @@ describe("golden multi-tool turn fixture", () => { test("TTFT is strictly less than stream on the golden fixture", () => { const turn = defined(rollupByTurn(multiToolTurnFixture())[0]); - assertLessThan(turn.ttftNs, turn.streamNs, "ttft vs stream"); + expect(turn.ttftNs).toBeLessThan(turn.streamNs); }); }); @@ -234,21 +93,27 @@ describe("observer pipeline → snapshot → rollup → assertions", () => { const spans = completed(snapshot()); - assertPhasePresent(spans, "turn"); - assertPhasePresent(spans, "inference"); - assertPhasePresent(spans, "inference.ttft"); - assertPhasePresent(spans, "inference.stream"); - assertPhasePresent(spans, "tool"); + for (const phase of [ + "turn", + "inference", + "inference.ttft", + "inference.stream", + "tool", + ]) { + expect(spans.some((span) => span.name === phase)).toBe(true); + } - assertNesting(spans, "inference", "turn"); - assertNesting(spans, "inference.ttft", "inference"); - assertNesting(spans, "inference.stream", "inference"); - assertNesting(spans, "tool", "turn"); + expect(nestedUnder(spans, "inference", "turn")).toBe(true); + expect(nestedUnder(spans, "inference.ttft", "inference")).toBe(true); + expect(nestedUnder(spans, "inference.stream", "inference")).toBe(true); + expect(nestedUnder(spans, "tool", "turn")).toBe(true); const turns = rollupByTurn(spans); expect(turns).toHaveLength(1); - assertTurnHasInferenceAndTools(defined(turns[0]), { minToolCount: 2 }); - expect(defined(turns[0]).toolCount).toBe(2); + const turn = defined(turns[0]); + expect(turn.inferenceNs).toBeGreaterThan(0); + expect(turn.toolCount).toBe(2); + expect(turn.toolNs).toBeGreaterThan(0); // Live clock: duration magnitudes are non-deterministic under sync hrtime // (ttftNs can exceed streamNs). Assert wall ordering instead of a no-op @@ -265,7 +130,11 @@ describe("observer pipeline → snapshot → rollup → assertions", () => { } const phases = rollupByPhase(spans); - assertPhaseSummary(phases, "tool", { minCount: 2 }); - assertPhaseSummary(phases, "inference", { minCount: 1 }); + expect( + phases.find((phase) => phase.name === "tool")?.count, + ).toBeGreaterThanOrEqual(2); + expect( + phases.find((phase) => phase.name === "inference")?.count, + ).toBeGreaterThanOrEqual(1); }); }); diff --git a/src/perf/assert-spans.ts b/src/perf/assert-spans.ts deleted file mode 100644 index 463fe46ee..000000000 --- a/src/perf/assert-spans.ts +++ /dev/null @@ -1,113 +0,0 @@ -/** - * Eval/test harness assertions over PerfTrace snapshots and rollups. Pure - * helpers that throw Error with a clear message on failure (no bun:test - * import). - */ - -import type { PerfSpan, SpanName } from "./index.js"; -import type { PhaseSummary, TurnSummary } from "./rollup.js"; - -/** Verify at least one span with the given phase name exists. */ -export function assertPhasePresent( - spans: readonly PerfSpan[], - phaseName: SpanName | string, -): void { - const found = spans.some((s) => s.name === phaseName); - if (!found) { - const names = [...new Set(spans.map((s) => s.name))].sort().join(", "); - throw new Error( - `expected phase "${phaseName}" in snapshot; present phases: [${names || "none"}]`, - ); - } -} - -/** - * Verify a span named `childName` has parentId pointing at a span named - * `parentName`. Arg order: (spans, child, parent). - */ -export function assertNesting( - spans: readonly PerfSpan[], - childName: SpanName | string, - parentName: SpanName | string, -): void { - const byId = new Map(spans.map((s) => [s.id, s])); - const ok = spans.some((child) => { - if (child.name !== childName || child.parentId === undefined) return false; - const parent = byId.get(child.parentId); - return parent !== undefined && parent.name === parentName; - }); - if (!ok) { - throw new Error( - `expected nesting ${parentName} → ${childName}; no matching parentId link found`, - ); - } -} - -export interface TurnInferenceToolsOpts { - /** Minimum tool invocations required (default 1). */ - minToolCount?: number; -} - -/** A turn that ran tools must report positive inference and tool cost. */ -export function assertTurnHasInferenceAndTools( - turn: TurnSummary, - opts?: TurnInferenceToolsOpts, -): void { - const minToolCount = opts?.minToolCount ?? 1; - if (turn.inferenceNs <= 0) { - throw new Error( - `turn ${turn.turnId}: expected inferenceNs > 0, got ${turn.inferenceNs}`, - ); - } - if (turn.toolCount < minToolCount) { - throw new Error( - `turn ${turn.turnId}: expected toolCount >= ${minToolCount} when tools ran, got ${turn.toolCount}`, - ); - } - if (turn.toolNs <= 0) { - throw new Error( - `turn ${turn.turnId}: expected toolNs > 0 when tools ran, got ${turn.toolNs}`, - ); - } -} - -/** Assert a < b for magnitude checks (e.g. TTFT < stream wall). */ -export function assertLessThan( - left: number, - right: number, - label = "magnitude", -): void { - if (!(left < right)) { - throw new Error(`${label}: expected ${left} < ${right}`); - } -} - -/** - * Assert a phase summary exists with count >= minCount (and totalNs when - * given). - */ -export function assertPhaseSummary( - phases: readonly PhaseSummary[], - phaseName: SpanName | string, - opts?: { minCount?: number; minTotalNs?: number }, -): PhaseSummary { - const phase = phases.find((p) => p.name === phaseName); - if (phase === undefined) { - const names = phases.map((p) => p.name).join(", "); - throw new Error( - `expected phase summary "${phaseName}"; present: [${names || "none"}]`, - ); - } - const minCount = opts?.minCount ?? 1; - if (phase.count < minCount) { - throw new Error( - `phase "${phaseName}": expected count >= ${minCount}, got ${phase.count}`, - ); - } - if (opts?.minTotalNs !== undefined && phase.totalNs < opts.minTotalNs) { - throw new Error( - `phase "${phaseName}": expected totalNs >= ${opts.minTotalNs}, got ${phase.totalNs}`, - ); - } - return phase; -}