diff --git a/CHANGELOG.md b/CHANGELOG.md index 2aa4e21..8db5013 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ and public product updates. ## Unreleased +### Plan-first turn for multi-part questions + +- Asked for revenue *and* ticket counts per customer, the agent wrote one query joining both + children of the same parent and silently multiplied the rows, returning 3,405 for a customer + whose revenue is 1,702.50. A warning in the grounding context did not prevent it - the agent + wrote one confident query in a single step - so the fix is a turn with **no tools offered**, + forcing it to commit to an approach in writing first +- The trigger is deliberately narrow: a false negative costs nothing, a false positive spends a + turn on every question. Data-modification questions are excluded outright, because planning + them measured *worse* - it turned a wrong number into a blank refusal. Safety stays the + verification pass's job +- **Measured**: 1/8 runs without planning, 8/8 with, on the one case in either suite that + triggers it. The hard A/B moved +30.6 to +33.3 - close to one noise unit, and necessarily so, + since planning fires on 1 of 12 cases and could move the aggregate by at most 8.3 points. The + case-level result is strong; the suite-level result is not, and both are reported +- `--no-plan` disables it, and a test asserts a non-triggering question takes a byte-identical + path, so the feature is provably free when it does not apply + ### Measure names are unique, and the fan-out case is gradeable (bug fix) - Two tables with the same numeric column produced the same measure name, and the metric compiler diff --git a/ROADMAP.md b/ROADMAP.md index 65dcaa2..610683d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -374,7 +374,31 @@ see the measure-grain defect in step 6.2. Build one step at a time: **Re-measured after the fix**: the hard A/B is unchanged at **+30.6** (grounded 31/36, raw-sql 20/36) across two runs at different code states, which is a useful reproducibility signal for the harness itself. -7. Short planning/decomposition for multi-part questions (bounded). +7. **Short planning/decomposition for multi-part questions** - ✅ Built (`PLANNING_PROMPT`, + `buildPlanningTurn`, `src/core/agent/loop.ts`; `--no-plan` to disable). + Before acting on a question that asks for more than one thing, the agent writes a plan in a + turn with **no tools offered**, so it has to commit to an approach in writing first. That + forcing function is the point: the grain warning from 6.2 was already in the context for the + failing entity and did not prevent the double count, because the agent wrote one confident + query in a single step. + **Deliberately narrow trigger.** A false negative costs nothing; a false positive spends a + turn on every question. A bare "and" would have fired on "give the customer name and the + amount" - one quantity in two columns, already answered correctly - so the trigger keys on a + genuine second quantity or instruction. Write-verb questions are excluded outright: measured, + planning them turned a wrong number into a blank refusal, the over-refusal failure the + verification pass exists to prevent. Their failure was never decomposition, so safety stays + verification's job. + **Measured** on `hard-fanout-revenue-and-cases`, the only case in either suite that triggers + it: **1/8 runs without planning, 8/8 with** (a repeat-5 run plus a full-suite repeat-3 run + each way). The 2x fan-out inflation it was returning is gone. + **Suite-level, stated honestly**: the hard A/B moved **+30.6 to +33.3** (grounded 31/36 to + 33/36). That is close to one noise unit, and it has to be: planning fires on 1 of 12 cases, so + the most it could ever move the aggregate is 8.3 points. The case-level result is strong, the + suite-level result is weak, and those are different claims. + **Two implementation bugs the measurement caught**, neither visible to the scripted tests: + the transcript ended on an assistant turn (which the API rejects outright), and the agent + finalized on the plan without running anything in 2 of 5 runs until the handoff message said + plainly that the plan is not the answer. 8. **Native desktop app** (decided 2026-07-25) — the flagship product surface. A native macOS app (Swift + AppKit) embeds a libghostty terminal pane running Claude Code / Codex under the user's own subscription (no BYOK). diff --git a/src/adapters/cli/ask.ts b/src/adapters/cli/ask.ts index a6ae53c..e5a4056 100644 --- a/src/adapters/cli/ask.ts +++ b/src/adapters/cli/ask.ts @@ -52,6 +52,8 @@ export interface RunAskOptions { maxSteps?: number; /** Run a self-critique pass before answering (default true). */ verify?: boolean; + /** Plan first on multi-part questions (default true). */ + plan?: boolean; /** Print each agent tool step. */ verbose?: boolean; /** Injected in tests; built from env credentials otherwise. */ @@ -160,6 +162,7 @@ export async function runAsk(options: RunAskOptions): Promise { complete: ai.agentComplete, maxSteps: options.maxSteps, verify: options.verify ?? true, + plan: options.plan ?? true, onStep: options.verbose ? (step) => log(formatStep(step)) : undefined, }); diff --git a/src/adapters/cli/eval.ts b/src/adapters/cli/eval.ts index 8daa8f2..0881c80 100644 --- a/src/adapters/cli/eval.ts +++ b/src/adapters/cli/eval.ts @@ -16,6 +16,8 @@ export interface RunEvalOptions { verify?: boolean; /** Agent suite: which configuration to measure (default "grounded"). */ arm?: string; + /** Agent suite: plan first on multi-part questions (default true). */ + plan?: boolean; /** Agent suite: run both arms interleaved and print the comparison. */ ab?: boolean; /** Agent suite: the agent's turn budget. */ @@ -55,6 +57,7 @@ export async function runEval(options: RunEvalOptions): Promise { repeat: options.repeat, provider: options.provider, verify: options.verify, + plan: options.plan, maxSteps: options.steps, onProgress: options.json ? undefined : (line: string) => log(line), }; diff --git a/src/adapters/cli/index.ts b/src/adapters/cli/index.ts index 56de615..ed29647 100644 --- a/src/adapters/cli/index.ts +++ b/src/adapters/cli/index.ts @@ -32,8 +32,8 @@ Usage: (needs a key) against evals/cases/. --json for raw output; agent accepts --cases , --repeat , --steps , --no-verify (baseline with the self-critique pass off), - --arm , and --ab (run both arms - interleaved and print the grounding comparison). + --arm , --no-plan, and --ab (run + both arms interleaved and print the comparison). Both suites accept --dataset , --cases-file and --glossary to score a different dataset. querypad help Show this help @@ -56,6 +56,8 @@ Options for ask: --no-verify Skip the self-critique pass (on by default: the agent re-checks projection/ranking/safety before answering) + --no-plan Skip the plan-first turn (on by default, and only + fires for questions asking for more than one thing) --verbose Print each agent tool step Environment: ANTHROPIC_API_KEY or OPENAI_API_KEY for the chosen provider. @@ -142,6 +144,7 @@ async function main(argv: string[]): Promise { showSql: flags["show-sql"] === true, maxSteps: steps && Number.isFinite(steps) ? steps : undefined, verify: flags["no-verify"] !== true, + plan: flags["no-plan"] !== true, verbose: flags.verbose === true, }); return 0; @@ -193,6 +196,7 @@ async function main(argv: string[]): Promise { repeat: repeat && Number.isFinite(repeat) ? repeat : undefined, provider: stringFlag(flags, "provider"), verify: flags["no-verify"] !== true, + plan: flags["no-plan"] !== true, arm: stringFlag(flags, "arm"), ab: flags.ab === true, dataset: stringFlag(flags, "dataset"), diff --git a/src/core/agent/loop.ts b/src/core/agent/loop.ts index edfcd96..b109cfb 100644 --- a/src/core/agent/loop.ts +++ b/src/core/agent/loop.ts @@ -25,6 +25,48 @@ Guidance: /** Write verbs that mean the user asked to modify data — refuse those, answer the rest. */ const WRITE_VERB = /\b(delete|drop|truncate|update|insert|alter|remove)\b/i; +/** + * Questions that ask for a second quantity or a second instruction. Deliberately + * narrow: it is the trigger for spending an extra model turn, so a false negative + * costs nothing (the agent proceeds as before) while a false positive costs a turn + * on every question. Derived from observed failures, not from intuition - a bare + * "and" would fire on "give the customer name and the amount", which is one + * quantity with two columns and already answered correctly. + */ +const MULTI_PART = + /\bthen\b|\band how many\b|\band their\b|\band the (number|count)\b|\balong with\b|\bas well as\b/i; + +/** + * Plan-first turn for a question that asks for more than one thing. The failure it + * targets is specific and measured: asked for revenue *and* ticket counts per + * customer, the agent writes a single query joining both children of the same + * parent and silently multiplies the rows - returning 3405 for a customer whose + * revenue is 1702.50, because that customer has two tickets. + * + * Note the last line. Our grader compares one result set, and more importantly the + * question genuinely asks for one table, so a plan that splits the work into two + * separate answers would be wrong for the user as well as unscoreable. + */ +export const PLANNING_PROMPT = `Before running anything, write a short plan (2 lines at most): +1. List each quantity the question asks for, and which table each one comes from. +2. If two quantities come from different tables that both hang off the same parent, joining them in one query repeats rows and inflates the totals. Compute each at its own grain - a subquery or CTE per quantity - then combine. +Then carry out the plan and return a single result set that answers the whole question.`; + +/** + * The planning turn for a question, or null when the extra turn would buy nothing. + * Pure — unit-tested without the network. + * + * Data-modification questions are deliberately excluded even though they read as + * multi-part. Measured: planning them made things worse, turning a wrong number into + * a blank refusal — the over-refusal failure the verification pass exists to prevent. + * Their failure was never decomposition (the agent already separates the parts), so + * safety stays verification's job and planning stays decomposition's. + */ +export function buildPlanningTurn(question: string): string | null { + if (WRITE_VERB.test(question)) return null; + return MULTI_PART.test(question) ? PLANNING_PROMPT : null; +} + /** * Self-critique checklist injected once before the agent finalizes. Targets the * mistakes a naive agent makes: over-projecting columns, ignoring ranking words, @@ -102,6 +144,11 @@ export interface RunAgentQueryOptions { maxSteps?: number; /** Run one self-critique turn before accepting the final answer (default false). */ verify?: boolean; + /** + * Write a plan first, but only for questions that ask for more than one thing + * (default false). Costs one model turn when it fires and nothing when it does not. + */ + plan?: boolean; /** Restrict the agent to these tool names (default: the whole toolkit). */ tools?: string[]; /** @@ -120,6 +167,17 @@ function textFrom(content: ContentBlock[]): string { .trim(); } +/** + * Trim text blocks and drop empties. The API rejects an assistant message whose final + * text block ends in whitespace, which a model writing a bulleted plan does routinely. + */ +function trimTextBlocks(content: ContentBlock[]): ContentBlock[] { + const isText = (b: ContentBlock): b is { type: "text"; text: string } => b.type === "text"; + return content + .map((block) => (isText(block) ? { ...block, text: block.text.trim() } : block)) + .filter((block) => !isText(block) || block.text.length > 0); +} + function toolUsesFrom(content: ContentBlock[]): ToolUseBlock[] { return content.filter((block): block is ToolUseBlock => block.type === "tool_use"); } @@ -153,6 +211,29 @@ export async function runAgentQuery(options: RunAgentQueryOptions): Promise void; } @@ -178,6 +181,7 @@ async function runCase( runner: deps.runner, maxSteps: deps.maxSteps, verify: deps.verify, + plan: deps.plan, tools: arm.tools, systemPrompt: arm.systemPrompt, complete: deps.complete, @@ -258,6 +262,7 @@ export async function runAgentSuite(options: AgentSuiteOptions = {}): Promise t.name), @@ -324,6 +330,7 @@ export async function runAbSuite( repeat, maxSteps: options.maxSteps ?? AB_MAX_STEPS, verify: options.verify ?? true, + plan: options.plan ?? true, // Accuracy-only for both arms: identical rubric is the whole point. behavioral: false, onProgress: options.onProgress, diff --git a/src/evals/types.ts b/src/evals/types.ts index 8575d3d..3b20893 100644 --- a/src/evals/types.ts +++ b/src/evals/types.ts @@ -86,6 +86,7 @@ export interface SuiteConfig { arm: string; repeat: number; verify: boolean; + plan: boolean; behavioral: boolean; maxSteps: number; tools: string[]; diff --git a/test/ask.test.ts b/test/ask.test.ts index becd558..6c0400b 100644 --- a/test/ask.test.ts +++ b/test/ask.test.ts @@ -5,6 +5,7 @@ import { buildAskContext } from "../src/core/agent/ask-context"; import { parseFollowups, runAsk, type AskAi } from "../src/adapters/cli/ask"; import { resolveSource } from "../src/adapters/cli/source"; import { + buildPlanningTurn, buildVerificationTurn, VERIFICATION_PROMPT, type AgentComplete, @@ -358,3 +359,119 @@ test("buildVerificationTurn adds a safety note only for data-modification questi assert.ok(plain.includes("Projection")); assert.ok(plain.includes("Ranking")); }); + +// ---- Planning pass (multi-part questions) -------------------------------------- + +test("buildPlanningTurn fires only for questions asking for more than one thing", () => { + // The two shapes measured as failing: a second quantity, and a second instruction. + assert.ok( + buildPlanningTurn("show the customer name, their net revenue, and how many cases they opened") + ); + // A write instruction reads as multi-part but is excluded on purpose: planning it + // measured worse, turning a wrong number into a blank refusal. Safety is the + // verification pass's job. + assert.equal(buildPlanningTurn("Delete every void invoice, then tell me how many remain."), null); + assert.ok(buildPlanningTurn("Count the open invoices, then show revenue per region.")); + + // Single-quantity questions must not pay for a turn. The third is the important + // one: "and the amount" is one quantity split over two columns, and it already + // answers correctly - a bare "and" trigger would have fired on it. + assert.equal(buildPlanningTurn("How many customers are there?"), null); + assert.equal(buildPlanningTurn("Break down net revenue by product category name."), null); + assert.equal( + buildPlanningTurn("Which single customer has the highest net revenue? Give the name and the amount."), + null + ); +}); + +test("planning adds a tool-free turn before acting, only when it fires", async () => { + const seen: { tools: number; last: string }[] = []; + const spy = (script: ToolCompletion[]): AgentComplete => { + let i = 0; + return async ({ messages, tools }) => { + const last = messages[messages.length - 1]; + seen.push({ + tools: tools.length, + last: typeof last.content === "string" ? last.content : "[blocks]", + }); + return script[Math.min(i++, script.length - 1)]; + }; + }; + + const multiPart = "show each customer's revenue and how many cases they opened"; + await runAsk({ + question: multiPart, + source: resolveSource({ folder: "fixtures/data" }), + ai: agentAi([], { agentComplete: spy([textReply("plan"), textReply("done")]) }), + log: () => {}, + }); + + // The first call is the planning turn: no tools offered, so the model must answer + // in prose, and the prompt it sees is the planning instruction. + assert.equal(seen[0].tools, 0, "planning turn must offer no tools"); + assert.match(seen[0].last, /write a short plan/i); + // The turns that follow are the normal loop, with the toolkit back. + assert.ok(seen[1].tools > 0, "the loop must run with tools after planning"); + + seen.length = 0; + await runAsk({ + question: "how many users are there", + source: resolveSource({ folder: "fixtures/data" }), + ai: agentAi([], { agentComplete: spy([textReply("done")]) }), + log: () => {}, + }); + assert.ok(seen[0].tools > 0, "a single-part question must not spend a planning turn"); +}); + +test("planning can be turned off for a question that would otherwise fire", async () => { + const toolCounts: number[] = []; + const complete: AgentComplete = async ({ tools }) => { + toolCounts.push(tools.length); + return textReply("done"); + }; + + await runAsk({ + question: "show revenue and how many cases each customer opened", + source: resolveSource({ folder: "fixtures/data" }), + plan: false, + ai: agentAi([], { agentComplete: complete }), + log: () => {}, + }); + assert.ok(toolCounts[0] > 0, "with --no-plan the first turn is the normal loop"); +}); + +test("a question that does not trigger planning takes a byte-identical path", async () => { + // Planning must be free when it does not fire. This is what lets a suite score be + // reasoned about case by case: any case whose question does not match the trigger + // cannot have been affected by the feature. + const record = (): { complete: AgentComplete; calls: unknown[] } => { + const calls: unknown[] = []; + return { + calls, + complete: async ({ system, messages, tools }) => { + calls.push({ system, messages: JSON.parse(JSON.stringify(messages)), tools: tools.length }); + return textReply("done"); + }, + }; + }; + + const on = record(); + await runAsk({ + question: "Break down net revenue by product category name.", + source: resolveSource({ folder: "fixtures/data" }), + plan: true, + ai: agentAi([], { agentComplete: on.complete }), + log: () => {}, + }); + + const off = record(); + await runAsk({ + question: "Break down net revenue by product category name.", + source: resolveSource({ folder: "fixtures/data" }), + plan: false, + ai: agentAi([], { agentComplete: off.complete }), + log: () => {}, + }); + + assert.deepEqual(on.calls, off.calls); +});