From 6f10c56f4ed4f21474cb1950858899d15d9e9092 Mon Sep 17 00:00:00 2001 From: PatrickDev-it Date: Fri, 7 Aug 2026 18:37:36 +0200 Subject: [PATCH] fix(web): parse each pass's own fields, salvage truncated output MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The second pass rendered nothing: no typewriter, and the sections after Known Requirements disappeared instead of filling. Two bugs, both from the previous commit, reproduced deterministically against the real parsers. 1. The parsers walked the global SPEC_KEYS order and gave up when the key they expected first was absent. The ground pass emits directive/task/ context/known_requirements — starting at index 0, so it worked. The expand pass starts at inferred_requirements, so parsePartialSpec returned {} for every token and parseStreamingSpec reported no active field. Nothing streamed and nothing rendered. parsePartialSpec, parseStreamingSpec and parseCompiledSpec now take the field list the caller is actually expecting; index.ts passes pass.fields. 2. parseCompiledSpec required a balanced {...} via extractJsonObject. A generation that hits its token cap stops mid-object, so there was no closing brace and the entire payload was discarded — which is why the cards vanished rather than filling. It now salvages via the streaming parser, keeping every completed field plus the completed items of the list that was mid-write. The truncation itself was under-allocation: 480 tokens was carried over from when this pass owned two fields, and it now owns six lists. Raised to 900 — ~150 per field, still well under the 320-per-field ceiling strategies.py measured as where a small model starts padding. The existing tests passed throughout because every fixture was a full envelope starting at "directive" — the first pass's shape, never the second's. New fixtures use the payload the expand pass really emits, and one asserts the un-scoped call still parses nothing, so the bug cannot return silently. The budget test now models scalar and list fields separately instead of assuming they cost the same. Also: show one skeleton at a time rather than the whole pass. Four bars pulsing at once read as a stalled page, not as progress. Co-Authored-By: Claude Opus 5 (1M context) --- src/web/index.ts | 12 +++-- src/web/lib/prompt.test.ts | 101 ++++++++++++++++++++++++++++++++++--- src/web/lib/prompt.ts | 59 ++++++++++++++++++---- 3 files changed, 152 insertions(+), 20 deletions(-) diff --git a/src/web/index.ts b/src/web/index.ts index 75c4200..1f9e740 100644 --- a/src/web/index.ts +++ b/src/web/index.ts @@ -177,7 +177,7 @@ async function compile(): Promise { rail.set('busy', `${pass.label} · ${index + 1}/${COMPILE_PASSES.length}`); // Draw the pass's sections as skeletons immediately, so the wait before the first token // reads as work in progress rather than a finished page. - specView.setStreaming(spec, null, pass.fields); + specView.setStreaming(spec, null, pass.fields.slice(0, 1)); updateExportState(); let raw = ''; @@ -190,16 +190,18 @@ async function compile(): Promise { raw += token; // Includes the field mid-write, so text appears character by character rather than a // card at a time, plus skeletons for what this pass has not reached yet. - const streaming = parseStreamingSpec(raw); + const streaming = parseStreamingSpec(raw, pass.fields); const merged = { ...spec, ...streaming.complete }; - const pending = pass.fields.filter((field) => !(field in merged) && field !== streaming.active?.field); - specView.setStreaming(merged, streaming.active, pending); + // Only the next section, not every remaining one — four bars pulsing at once read as + // a stalled page rather than as progress. + const next = pass.fields.find((field) => !(field in merged) && field !== streaming.active?.field); + specView.setStreaming(merged, streaming.active, next ? [next] : []); updateExportState(); }, }); // Each pass returns only its own keys; merge rather than replace. - const passSpec = parseCompiledSpec(text || raw, userInput); + const passSpec = parseCompiledSpec(text || raw, userInput, pass.fields); for (const field of pass.fields) { const value = passSpec[field]; if (Array.isArray(value) ? value.length > 0 : String(value ?? '').trim().length > 0) { diff --git a/src/web/lib/prompt.test.ts b/src/web/lib/prompt.test.ts index 8884dec..7d3be08 100644 --- a/src/web/lib/prompt.test.ts +++ b/src/web/lib/prompt.test.ts @@ -150,12 +150,6 @@ describe('COMPILE_PASSES', () => { expect(covered).toContain('directive'); }); - test('keeps per-pass budgets tight', () => { - // strategies.py records that raising a small model's per-unit budget produced repetitive - // filler, not richer output. Guard against "fixing" sparse output by raising these. - for (const pass of COMPILE_PASSES) expect(pass.maxTokens).toBeLessThanOrEqual(512); - }); - test('separates extraction from inference, which is the split worth its latency', () => { expect(COMPILE_PASSES[0]?.fields).toContain('known_requirements'); expect(COMPILE_PASSES[0]?.fields).not.toContain('inferred_requirements'); @@ -283,3 +277,98 @@ describe('parseStreamingSpec', () => { expect(active?.partialText ?? '').not.toContain('":"'); }); }); + +/** + * Regressions from the live bug where the second pass produced no visible output. + * + * Every earlier fixture in this file was a full envelope starting at `directive`, which is what + * the FIRST pass emits. The second pass starts at `inferred_requirements`, and the parsers walked + * the global key order and gave up when `directive` was absent — so the suite stayed green while + * the app silently rendered nothing. These fixtures use the payload shape the second pass really + * produces. + */ +describe('pass-scoped parsing (the second pass does not start at "directive")', () => { + const PASS2 = COMPILE_PASSES[1]!.fields; + const MID_STREAM = + '{"inferred_requirements":["Define the recipient segment","Add a subject line"],"implementation_strategy":["Draft the body'; + + test('parsePartialSpec reads a payload that starts mid-envelope', () => { + expect(parsePartialSpec(MID_STREAM, PASS2)).toEqual({ + inferred_requirements: ['Define the recipient segment', 'Add a subject line'], + }); + }); + + test('without the field list it parses nothing — the exact shape of the bug', () => { + expect(parsePartialSpec(MID_STREAM)).toEqual({}); + }); + + test('parseStreamingSpec exposes the in-flight field, so the typewriter runs', () => { + const { active } = parseStreamingSpec(MID_STREAM, PASS2); + expect(active?.field).toBe('implementation_strategy'); + expect(active?.partialText).toBe('Draft the body'); + }); + + test('every pass can parse its own first field', () => { + for (const pass of COMPILE_PASSES) { + const first = pass.fields[0]!; + const raw = `{"${first}":${first.includes('_') && first !== 'known_requirements' ? '["x"]' : '"x"'}`; + // Not asserting the value, only that the parser engages instead of bailing on key order. + expect(() => parseStreamingSpec(raw, pass.fields)).not.toThrow(); + } + }); +}); + +describe('truncation salvage', () => { + const PASS2 = COMPILE_PASSES[1]!.fields; + + test('keeps completed fields when the generation stopped before the closing brace', () => { + // Hitting the token cap leaves no balanced object; discarding the payload for a missing `}` + // is what made the later sections disappear instead of filling. + const truncated = + '{"inferred_requirements":["Segment the recipients","Write a subject line","State one call to action"],"implementation_strategy":["Draft the'; + const spec = parseCompiledSpec(truncated, 'fallback', PASS2); + expect(spec.inferred_requirements).toHaveLength(3); + }); + + test('keeps the completed items of the list that was mid-write', () => { + const truncated = '{"inferred_requirements":["first","second","thi'; + const spec = parseCompiledSpec(truncated, 'fallback', PASS2); + expect(spec.inferred_requirements).toEqual(['first', 'second']); + }); + + test('a well-formed payload still takes the fast path unchanged', () => { + const whole = + '{"inferred_requirements":["a"],"implementation_strategy":["b"],"constraints":[],"quality_expectations":[],"validation_checklist":[],"output_requirements":[]}'; + const spec = parseCompiledSpec(whole, 'fallback', PASS2); + expect(spec.inferred_requirements).toEqual(['a']); + expect(spec.implementation_strategy).toEqual(['b']); + }); + + test('still falls back to raw text when there is nothing structured at all', () => { + const spec = parseCompiledSpec('the model just wrote prose', 'the request'); + expect(spec.task).toBe('the request'); + expect(spec.context).toBe('the model just wrote prose'); + }); +}); + +describe('pass budgets scale with the number of fields', () => { + test('every pass can fit the output it asks for', () => { + // A flat per-field floor is the wrong model: three short strings cost far less than six lists + // of 3-6 items. 480 tokens across SIX lists was the under-allocation that truncated pass 2 + // mid-object. Budget roughly 40 tokens for a scalar field and 130 for a list. + const LIST_FIELDS = new Set(COMPILER_SECTIONS.filter((s) => s.isList).map((s) => s.field)); + for (const pass of COMPILE_PASSES) { + const lists = pass.fields.filter((f) => LIST_FIELDS.has(f)).length; + const scalars = pass.fields.length - lists; + expect(pass.maxTokens).toBeGreaterThanOrEqual(scalars * 40 + lists * 130); + } + }); + + test('and stays under the per-field ceiling the server measured', () => { + // strategies.py: 960 tokens for a single field produced repetitive filler; 320/field is the + // point that was kept. This encodes the lesson per field rather than per call. + for (const pass of COMPILE_PASSES) { + expect(pass.maxTokens / pass.fields.length).toBeLessThanOrEqual(320); + } + }); +}); diff --git a/src/web/lib/prompt.ts b/src/web/lib/prompt.ts index 35bfde6..6a8d8ad 100644 --- a/src/web/lib/prompt.ts +++ b/src/web/lib/prompt.ts @@ -110,7 +110,12 @@ export const COMPILE_PASSES: CompilePass[] = [ 'validation_checklist', 'output_requirements', ], - maxTokens: 480, + // Six lists at 3-6 items each do not fit in 480 tokens. The previous value was carried over + // from when this pass owned two fields, and the generation hit the cap mid-object every time — + // which, before the salvage path existed, discarded the whole pass. ~150 tokens per field is + // still well under the 320-per-field ceiling the server measured as the point where a small + // model starts padding rather than stopping. + maxTokens: 900, }, ]; @@ -274,9 +279,12 @@ function findKeyStart(raw: string, key: string, from = 0): number { * that has started but not yet closed is left unset rather than guessed at. This is what lets the * UI render completed sections as they arrive instead of the raw JSON buffer while generation is * still in progress. */ -export function parsePartialSpec(raw: string): Partial { +export function parsePartialSpec( + raw: string, + fields: ReadonlyArray = SPEC_KEYS +): Partial { const result: Partial = {}; - for (const [i, key] of SPEC_KEYS.entries()) { + for (const [i, key] of fields.entries()) { const keyStart = findKeyStart(raw, key); if (keyStart === -1) break; @@ -284,7 +292,7 @@ export function parsePartialSpec(raw: string): Partial { if (colonIndex === -1) break; const valueStart = colonIndex + 1; - const nextKey = SPEC_KEYS[i + 1]; + const nextKey = fields[i + 1]; let segment: string | null; if (nextKey) { const nextKeyStart = findKeyStart(raw, nextKey, valueStart); @@ -352,10 +360,10 @@ function readOpenString(buffer: string, from: number): string | null { * cards at once. This additionally exposes the field currently being written — including a * half-finished sentence — so text appears character by character as the model produces it. */ -export function parseStreamingSpec(raw: string): StreamingSpec { - const complete = parsePartialSpec(raw); +export function parseStreamingSpec(raw: string, fields: ReadonlyArray = SPEC_KEYS): StreamingSpec { + const complete = parsePartialSpec(raw, fields); const completedCount = Object.keys(complete).length; - const key = SPEC_KEYS[completedCount]; + const key = fields[completedCount]; if (!key) return { complete, active: null }; const keyStart = findKeyStart(raw, key); @@ -412,7 +420,11 @@ export function parseStreamingSpec(raw: string): StreamingSpec { /** Best-effort parse: a valid JSON object wins outright; anything else still renders instead of * failing the whole request, matching the project's existing safe-fallback philosophy — the raw * text becomes the "task" field so nothing the model produced is silently discarded. */ -export function parseCompiledSpec(raw: string, fallbackTask: string): CompiledSpec { +export function parseCompiledSpec( + raw: string, + fallbackTask: string, + fields: ReadonlyArray = SPEC_KEYS +): CompiledSpec { const jsonText = extractJsonObject(raw); if (jsonText) { try { @@ -430,9 +442,38 @@ export function parseCompiledSpec(raw: string, fallbackTask: string): CompiledSp output_requirements: asStringArray(parsed.output_requirements), }; } catch { - // fall through to the raw-text fallback below + // fall through to the salvage path below } } + + // Salvage. A generation that hits its token cap stops mid-object, so there is no balanced + // `{...}` for `extractJsonObject` to find — and discarding the whole payload for a missing + // closing brace threw away every field that HAD completed. That is what made the later sections + // vanish instead of filling. Re-use the streaming parser, which reads field by field and does + // not need the object to be closed. + const { complete, active } = parseStreamingSpec(raw, fields); + const salvaged: Partial = { ...complete }; + // The field that was mid-write when the cap hit still has whole items behind it; keeping four + // of five beats discarding the section. + if (active?.isList && active.completedItems.length > 0) { + (salvaged as Record)[active.field] = active.completedItems; + } + + if (Object.keys(salvaged).length > 0) { + return { + directive: salvaged.directive ?? '', + task: salvaged.task?.trim() ? salvaged.task : fallbackTask, + context: salvaged.context ?? '', + known_requirements: salvaged.known_requirements ?? [], + inferred_requirements: salvaged.inferred_requirements ?? [], + implementation_strategy: salvaged.implementation_strategy ?? [], + constraints: salvaged.constraints ?? [], + quality_expectations: salvaged.quality_expectations ?? [], + validation_checklist: salvaged.validation_checklist ?? [], + output_requirements: salvaged.output_requirements ?? [], + }; + } + return { directive: '', task: fallbackTask,