From f6746eb91c16efdfd47c6a616110ed1342313bae Mon Sep 17 00:00:00 2001 From: Fini Date: Wed, 29 Apr 2026 09:49:45 +0800 Subject: [PATCH] fix(ab-corpus): forbid mixing batch_design with element tools in T MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The earlier T instruction split tool selection into PRIMARY / COMPOSITE (element tools) and FALLBACK (batch_design). FALLBACK was scoped "when no element tool fits a given component shape" — implying you could mix per-component. But output-parser.ts silently drops every batch_design tag whenever any element call is also present (filter ELEMENT_TOOL_NAME_RE then return). A mixed response {"name":"add_section_header_v0",...} {"name":"batch_design", ...scaffolding...} would lose the batch_design half and only run the element call — the brief is half-applied without anyone noticing. Codex stop-time review caught the mismatch. Reframes T as a binary choice: - STRATEGY A: every component fits an add_*_v0 tool — emit one tag per component - STRATEGY B: at least one component needs batch_design — emit a SINGLE batch_design covering the whole brief Plus an explicit "Do not mix Strategy A and Strategy B" guard line. Tests assert both strategy markers are present in every T variant and the dropped per-component-fallback phrase is gone. The elements.md cookbook still teaches batch_design + element tool composition for real MCP multi-round usage; the corpus T arm is single-shot so the trailing T_TOOL_CALL_INSTRUCTIONS overrides via "last instruction wins". --- .../ab-corpus/__tests__/build-prompt.test.ts | 32 +++++++++++++++---- scripts/ab-corpus/build-prompt.ts | 13 +++----- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/scripts/ab-corpus/__tests__/build-prompt.test.ts b/scripts/ab-corpus/__tests__/build-prompt.test.ts index e05e7eb90..74f7aa484 100644 --- a/scripts/ab-corpus/__tests__/build-prompt.test.ts +++ b/scripts/ab-corpus/__tests__/build-prompt.test.ts @@ -9,16 +9,21 @@ import { buildSystemPrompt } from '../build-prompt'; // the diet silently no-ops. const T_MULTI_TOOL_MARKER = 'Respond with one or more `` tags'; -const T_COMPOSITE_MARKER = 'COMPOSITE (multi-component briefs'; +const T_STRATEGY_A_MARKER = 'STRATEGY A — element tools'; +const T_STRATEGY_B_MARKER = 'STRATEGY B — batch_design fallback'; +const T_NO_MIX_MARKER = 'Do not mix Strategy A and Strategy B'; const B_BATCH_MARKER = '{"name": "batch_design", "arguments": {"operations": ""}}'; // Earlier versions of the T instructions explicitly forbade multi-tool // output ("Respond with one tag" / "Do not combine multiple tags"); // that wording undercut the multi-tool teaching baked into elements.md. -// Codex stop-time review caught the contradiction — guard against -// regression by asserting the forbidding text is GONE. +// A subsequent revision flipped to allow chaining but quietly invited +// mixed batch_design + element tags per-component, which the corpus +// parser silently drops. Codex stop-time review caught both. Guard +// against re-introducing either failure mode. const FORBIDDEN_SINGLE_TAG_PHRASE = 'one `` tag, nothing else'; const FORBIDDEN_NO_COMBINE_PHRASE = 'Do not combine multiple tags'; +const FORBIDDEN_PER_COMPONENT_FALLBACK_PHRASE = 'when no element tool fits a given component shape'; describe('buildSystemPrompt', () => { it('T + composite returns the largest prompt (cookbook present)', () => { @@ -64,11 +69,12 @@ describe('buildSystemPrompt', () => { expect(distinct.size).toBe(1); }); - it('every T variant carries the multi-tool output-format marker', () => { + it('every T variant carries the multi-tool + Strategy A/B output-format markers', () => { for (const difficulty of ['obvious', 'optional', 'composite'] as const) { const built = buildSystemPrompt('T', { difficulty }); expect(built.system).toContain(T_MULTI_TOOL_MARKER); - expect(built.system).toContain(T_COMPOSITE_MARKER); + expect(built.system).toContain(T_STRATEGY_A_MARKER); + expect(built.system).toContain(T_STRATEGY_B_MARKER); } }); @@ -80,9 +86,21 @@ describe('buildSystemPrompt', () => { } }); - it('B variant carries the batch_design marker but not the multi-tool split', () => { + it('T variant explicitly forbids mixing Strategy A and Strategy B (parser drops mixed)', () => { + for (const difficulty of ['obvious', 'optional', 'composite'] as const) { + const built = buildSystemPrompt('T', { difficulty }); + expect(built.system).toContain(T_NO_MIX_MARKER); + // Earlier wording invited per-component fallback ("when no element + // tool fits a given component shape") which the parser silently + // drops when element calls are also present. Guard the rephrase. + expect(built.system).not.toContain(FORBIDDEN_PER_COMPONENT_FALLBACK_PHRASE); + } + }); + + it('B variant carries the batch_design marker but not the Strategy A/B split', () => { const built = buildSystemPrompt('B'); expect(built.system).toContain(B_BATCH_MARKER); - expect(built.system).not.toContain(T_COMPOSITE_MARKER); + expect(built.system).not.toContain(T_STRATEGY_A_MARKER); + expect(built.system).not.toContain(T_STRATEGY_B_MARKER); }); }); diff --git a/scripts/ab-corpus/build-prompt.ts b/scripts/ab-corpus/build-prompt.ts index dfa826f0d..cf48dfacd 100644 --- a/scripts/ab-corpus/build-prompt.ts +++ b/scripts/ab-corpus/build-prompt.ts @@ -45,21 +45,18 @@ const T_TOOL_CALL_INSTRUCTIONS = [ '', 'OUTPUT FORMAT — EMIT AS TOOL CALL(S):', '', - 'Respond with one or more `` tags, nothing else. The harness reads every `` tag in your output, so chain as many as the brief implies. Choose tools based on intent:', + 'Respond with one or more `` tags, nothing else. The harness reads every `` tag in your output, so chain as many as the brief implies. Pick ONE strategy for the whole response — do NOT mix element tools with batch_design in the same output:', '', - 'PRIMARY (single component): emit one tag for the add_*_v0 element tool that fits.', - ' {"name": "add_X_v0", "arguments": {...}}', - '', - 'COMPOSITE (multi-component briefs — settings panel with N rows, team list with N members, audit feed with N entries, onboarding screen with N steps): emit ONE `` per component under the same parent. Tags appear in render order (top-to-bottom for vertical, left-to-right for horizontal).', + 'STRATEGY A — element tools (preferred): when every component in the brief fits an `add_*_v0` element tool, emit one tag per component, in render order (top-to-bottom for vertical layouts, left-to-right for horizontal). Single-component briefs produce exactly one tag; multi-component briefs (settings panel with N rows, team list with N members, audit feed with N entries, onboarding screen with N step cards) produce N+1 or more.', ' {"name": "add_section_header_v0", "arguments": {"title": "Notifications"}}', ' {"name": "add_setting_row_v0", "arguments": {...}}', ' {"name": "add_setting_row_v0", "arguments": {...}}', '', - 'FALLBACK: when no element tool fits a given component shape, emit a single batch_design call.', + 'STRATEGY B — batch_design fallback: when the brief includes any component shape that NO element tool covers (heterogeneous custom layout, post-hoc styling, bespoke scaffolding), emit a SINGLE batch_design call covering the WHOLE response. Multiple `` tags carrying batch_design or any mix of batch_design + element tools is not supported — the harness drops the batch_design half and only the element calls run.', ' {"name": "batch_design", "arguments": {"operations": ""}}', - 'The `operations` value is a single string containing the batch_design DSL.', + 'The `operations` value is a single string containing the batch_design DSL covering every component in the brief.', '', - 'Do not add prose before, after, or between tags.', + 'Do not add prose before, after, or between tags. Do not mix Strategy A and Strategy B in the same output — choose element tools for every component or batch_design for every component.', '', ].join('\n');