From 0877d44b83620b93d20cbf703e497fd79a906fe0 Mon Sep 17 00:00:00 2001 From: Kiyeon Jeon Date: Sun, 26 Jul 2026 02:42:37 +0900 Subject: [PATCH] feat: 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 3405 for a customer whose revenue is 1702.50. The grain warning added in 6.2 was already in the context for that exact entity and did not prevent it, because 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 before it can act. The trigger is deliberately narrow. A false negative costs nothing; a false positive spends a turn on every question. A bare 'and' would fire on 'give the customer name and the amount' - one quantity in two columns, already answered correctly - so it 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, which is the over-refusal failure the verification pass exists to prevent. Their failure was never decomposition, so safety stays verification's job and planning stays decomposition's. Measured on the one 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). 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 necessarily so - 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. A test asserts a non-triggering question takes a byte-identical path, so the feature is provably free when it does not apply. --- CHANGELOG.md | 18 ++++++ ROADMAP.md | 26 ++++++++- src/adapters/cli/ask.ts | 3 + src/adapters/cli/eval.ts | 3 + src/adapters/cli/index.ts | 8 ++- src/core/agent/loop.ts | 81 ++++++++++++++++++++++++++ src/evals/run-agent.ts | 7 +++ src/evals/types.ts | 1 + test/ask.test.ts | 117 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 261 insertions(+), 3 deletions(-) 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); +});