From 2cd0c6e8637e784953a4b8e7f7984862d7cc8b62 Mon Sep 17 00:00:00 2001 From: RubenGlez Date: Fri, 14 Aug 2026 16:17:24 +0200 Subject: [PATCH] fix: recover specs when a model drops a closing bracket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Smaller models reliably under-close deeply nested response schemas and then stop with finish_reason "stop" — they believe they finished, so all three generation attempts reproduced the same truncation and the endpoint was documented as nothing at all. Measured on deepseek-chat, this was the cause of *every* failing accuracy fixture: 4 of 14 endpoints produced no spec, while every reply that did parse scored perfectly. The eval gate had been reporting this as an accuracy problem because it only prints a mean. The reply is now re-read with the missing brackets restored. Crucially the bracket is not simply appended: in the captured failures the model under-closed just before a trailing top-level key, so closing at the end parses but nests `security` inside `responses` — valid JSON, wrong document. Each candidate reading is validated against OperationSchema, which is the only reliable arbiter of where the bracket belonged. Also: - Retry guidance names the likely cause instead of echoing the parser position, which models ignored. - The gate prints why each imperfect fixture lost points, so a hard zero is distinguishable from a slightly-wrong spec. deepseek-chat on the eval suite: 0.714-0.929 (straddling the 0.85 gate, 4-5 generation failures) → 0.951-0.964 with zero failures, over three consecutive gate runs. Claude-Session: https://claude.ai/code/session_01GrfumUQ4vFUAgsFYwzDyK8 --- CHANGELOG.md | 26 +++ apps/evals/matrix.ts | 9 +- .../core/src/__tests__/builder-json.test.ts | 137 ++++++++++++++++ packages/core/src/spec/builder.ts | 152 ++++++++++++++++-- 4 files changed, 313 insertions(+), 11 deletions(-) create mode 100644 packages/core/src/__tests__/builder-json.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 7024290..653c9e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,32 @@ All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed +- **Recover specs from models that drop a closing bracket.** Smaller models + reliably under-close deeply nested response schemas and then stop with + `finish_reason: "stop"`, so all three generation attempts reproduced the same + truncation and the endpoint was documented as nothing at all. Measured on + `deepseek-chat`, this was the cause of *every* failing accuracy fixture: 4 of + 14 endpoints produced no spec, while every reply that did parse scored + perfectly. The reply is now re-read with the missing brackets restored, and + the candidate readings are validated against the Operation schema so the + bracket is placed where it actually belonged — closing at the end also parses, + but silently nests `security` inside `responses`. + + Accuracy for `deepseek-chat` on the eval suite goes from 0.714–0.929 + (straddling the 0.85 gate) to 0.951–0.964, with zero generation failures. + +### Changed +- Retry guidance after a malformed reply names the likely cause instead of + echoing the parser ("Expected ',' or '}' ... at position 557"), which models + ignored — in practice all three attempts repeated the identical mistake. +- The CI accuracy gate now prints why each imperfect fixture lost points. It + previously reported only a mean, which cannot distinguish "the spec was + slightly wrong" from "generation threw and scored a hard zero" — they need + opposite fixes, and the second hid this provider bug for weeks. + ## [0.10.0] - 2026-08-14 ### Fixed diff --git a/apps/evals/matrix.ts b/apps/evals/matrix.ts index bf1758c..e1e5c3e 100644 --- a/apps/evals/matrix.ts +++ b/apps/evals/matrix.ts @@ -144,11 +144,18 @@ if (GATE) { continue } process.stdout.write(`gating ${label(c)} (${fixtures.length} fixtures)...\n`) - const { mean, worst } = await runModel(c) + const { mean, worst, results } = await runModel(c) tested.push({ c, mean }) console.log( ` ${label(c).padEnd(40)} mean=${mean.toFixed(3)} worst ${worst.rel.replace('fixtures/', '')} (${worst.score.toFixed(2)})` ) + // Print why each imperfect fixture lost points. Without this the gate + // reports a bare mean, which cannot distinguish "the spec was slightly + // wrong" from "generation threw and scored a hard 0" — they need opposite + // fixes, and the second one hid a real provider bug for weeks. + for (const r of results.filter((r) => r.score < 1)) { + console.log(` ${r.rel.replace('fixtures/', '').padEnd(34)} ${r.score.toFixed(2)} ${r.reason}`) + } } if (tested.length === 0) { diff --git a/packages/core/src/__tests__/builder-json.test.ts b/packages/core/src/__tests__/builder-json.test.ts new file mode 100644 index 0000000..c66d1c1 --- /dev/null +++ b/packages/core/src/__tests__/builder-json.test.ts @@ -0,0 +1,137 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' + +const { replies } = vi.hoisted(() => ({ replies: { queue: [] as string[], prompts: [] as string[] } })) + +vi.mock('ai', () => ({ + generateText: vi.fn(async ({ prompt }: { prompt: string }) => { + replies.prompts.push(prompt) + return { text: replies.queue.shift() ?? '{}' } + }), +})) + +import { buildOperation } from '../spec/builder.js' +import type { CaptureEvent } from '../types.js' + +const event: CaptureEvent = { + method: 'GET', + path: '/me', + query: {}, + params: {}, + body: null, + response: { user: { id: '1' }, orders: 2 }, + status: 200, + requestHeaders: {}, + responseHeaders: {}, + durationMs: 5, +} + +const ai = { provider: 'ollama' as const } + +beforeEach(() => { + replies.queue = [] + replies.prompts = [] +}) + +// Verbatim output captured from deepseek-chat on the get-me-bearer fixture: +// 16 opening braces, 15 closing, finish_reason "stop" — the model believed it +// was done. This produced a hard 0.00 on 4 of 14 accuracy fixtures. +const DEEPSEEK_TRUNCATED = + '{"summary":"Get current user info","description":"Retrieves the authenticated user\'s profile.",' + + '"operationId":"getMe","parameters":[],"responses":{"200":{"description":"Current user profile",' + + '"content":{"application/json":{"schema":{"type":"object","properties":{"user":{"type":"object",' + + '"properties":{"id":{"type":"string"},"name":{"type":"string"}}},"orders":{"type":"integer"}}}}}},' + + '"security":[{"bearerAuth":[]}]}' + +describe('recovering truncated model output', () => { + it('is genuinely unparseable as-is', () => { + expect(() => JSON.parse(DEEPSEEK_TRUNCATED)).toThrow() + }) + + it('recovers a real deepseek reply missing one closing brace', async () => { + replies.queue = [DEEPSEEK_TRUNCATED] + const op = await buildOperation(event, null, ai) + + expect(op.summary).toBe('Get current user info') + expect(op.operationId).toBe('getMe') + // The nested schema must survive the repair, not just the outer object. + const schema = op.responses?.['200']?.content?.['application/json']?.schema as Record + expect(Object.keys(schema.properties)).toEqual(['user', 'orders']) + expect(Object.keys(schema.properties.user.properties)).toEqual(['id', 'name']) + // The dropped brace belonged before `,"security"`, not at the end. Closing + // at the end also parses, but buries security inside responses — so this + // asserts the repair picked the right insertion point, not merely a valid one. + expect(op.security).toEqual([{ bearerAuth: [] }]) + expect((op.responses as Record).security).toBeUndefined() + }) + + it('recovers on the first attempt, without burning retries', async () => { + replies.queue = [DEEPSEEK_TRUNCATED] + await buildOperation(event, null, ai) + expect(replies.prompts).toHaveLength(1) + }) + + it('closes several missing brackets, including arrays', async () => { + replies.queue = [ + '{"summary":"List","responses":{"200":{"description":"OK"}},"parameters":[{"name":"page","in":"query"', + ] + const op = await buildOperation(event, null, ai) + expect(op.summary).toBe('List') + expect(op.parameters?.[0]?.name).toBe('page') + }) + + it('ignores braces inside string values when balancing', async () => { + replies.queue = [ + '{"summary":"Uses {curly} and [square] in prose","responses":{"200":{"description":"OK"}}', + ] + const op = await buildOperation(event, null, ai) + expect(op.summary).toBe('Uses {curly} and [square] in prose') + }) + + it('leaves well-formed output untouched', async () => { + replies.queue = ['{"summary":"Fine","responses":{"200":{"description":"OK"}}}'] + const op = await buildOperation(event, null, ai) + expect(op.summary).toBe('Fine') + }) + + it('still handles markdown fences', async () => { + replies.queue = ['```json\n{"summary":"Fenced","responses":{"200":{"description":"OK"}}}\n```'] + const op = await buildOperation(event, null, ai) + expect(op.summary).toBe('Fenced') + }) + + it('does not paper over output that is broken some other way', async () => { + // Balanced brackets, but a missing comma — repair must not mask this. + const broken = '{"summary":"A" "responses":{"200":{"description":"OK"}}}' + replies.queue = [broken, broken, broken] + await expect(buildOperation(event, null, ai)).rejects.toThrow(/after 3 attempts/) + }) +}) + +describe('retry guidance', () => { + it('names the likely cause after a syntax failure instead of echoing the parser', async () => { + // Unrepairable syntax error, so it retries. + const broken = '{"summary":"A" "x":}' + replies.queue = [broken, '{"summary":"Recovered","responses":{"200":{"description":"OK"}}}'] + + const op = await buildOperation(event, null, ai) + expect(op.summary).toBe('Recovered') + + const retryPrompt = replies.prompts[1] + expect(retryPrompt).toContain('missing closing') + expect(retryPrompt).toContain('every bracket you') + }) + + it('reports the schema mismatch when the JSON parsed but the shape was wrong', async () => { + // Valid JSON, but `in` is not one of the allowed parameter locations. + replies.queue = [ + '{"summary":"Bad param","parameters":[{"name":"page","in":"nowhere"}]}', + '{"summary":"Recovered","responses":{"200":{"description":"OK"}}}', + ] + + await buildOperation(event, null, ai) + + const retryPrompt = replies.prompts[1] + expect(retryPrompt).toContain('did not match the required shape') + expect(retryPrompt).not.toContain('missing closing') + }) +}) diff --git a/packages/core/src/spec/builder.ts b/packages/core/src/spec/builder.ts index 67f5e44..90680a2 100644 --- a/packages/core/src/spec/builder.ts +++ b/packages/core/src/spec/builder.ts @@ -51,8 +51,85 @@ function describeModelError(err: unknown, model: string): string | null { ) } -/** Pull a JSON object out of a model's text reply, tolerating markdown fences and prose. */ -function extractJson(text: string): unknown { +/** + * The brackets still open at the end of `text`, outermost first. Only + * structural brackets count; anything inside a string literal is skipped, so a + * `}` in a description can't unbalance the count. + */ +function unclosedBrackets(text: string): string[] { + const open: string[] = [] + let inString = false + let escaped = false + + for (const ch of text) { + if (escaped) { + escaped = false + continue + } + if (ch === '\\') { + escaped = true + continue + } + if (ch === '"') { + inString = !inString + continue + } + if (inString) continue + if (ch === '{' || ch === '[') open.push(ch) + else if (ch === '}' || ch === ']') open.pop() + } + return open +} + +const closerFor = (c: string) => (c === '{' ? '}' : ']') + +/** Append whatever `text` left open, so the result is bracket-balanced. */ +function closeUnbalanced(text: string): string { + const open = unclosedBrackets(text) + if (open.length === 0) return text + return text + open.reverse().map(closerFor).join('') +} + +/** + * Offsets of a `,` that directly follows a closing bracket — the boundaries + * between sibling members, and the only places a dropped `}` can plausibly + * belong. Latest first: the deepest truncation is the most likely one. + */ +function memberBoundaries(text: string): number[] { + const out: number[] = [] + for (let i = 1; i < text.length; i++) { + if (text[i] !== ',') continue + const prev = text[i - 1] + if (prev === '}' || prev === ']') out.push(i) + } + return out.reverse() +} + +const tryParse = (s: string): { ok: boolean; value?: unknown } => { + try { + return { ok: true, value: JSON.parse(s) } + } catch { + return { ok: false } + } +} + +/** + * Every plausible reading of a model reply, cheapest first. + * + * Smaller models reliably drop a `}` on deeply nested response schemas and then + * stop with `finish_reason: "stop"` — they believe they finished, so retrying + * reproduces the same mistake. Measured on deepseek-chat this accounted for + * *every* failed fixture in the accuracy suite: 4 of 14 endpoints produced + * nothing at all, while every reply that did parse scored perfectly. + * + * The dropped bracket is usually not missing from the end. In the captured + * failures the model under-closed just before a trailing top-level key, so + * naively appending `}` parses but nests `security` inside `responses` — valid + * JSON, wrong document. Rather than guess, this yields each reading and lets + * the caller pick the first that satisfies OperationSchema; the schema is the + * only reliable arbiter of where the bracket belonged. + */ +export function* jsonCandidates(text: string): Generator { let t = text.trim() const fence = t.match(/```(?:json)?\s*([\s\S]*?)```/i) if (fence) t = fence[1].trim() @@ -61,7 +138,51 @@ function extractJson(text: string): unknown { if (start === -1 || end === -1 || end < start) { throw new Error('no JSON object found in model output') } - return JSON.parse(t.slice(start, end + 1)) + + // Bounded at the last `}` so trailing prose is ignored. + const exact = tryParse(t.slice(start, end + 1)) + if (exact.ok) { + yield exact.value + return + } + + // From here on, work to the end of the reply rather than the last `}`: when a + // model stops mid-structure that `}` is an *inner* one, and the bounded slice + // would silently amputate everything after it. + const body = t.slice(start) + const open = unclosedBrackets(body) + if (open.length === 0) throw new Error(`unparseable model output: ${exact.value ?? 'invalid JSON'}`) + + const closers = [...open].reverse().map(closerFor).join('') + + // (a) the model simply stopped early — close at the end. + const atEnd = tryParse(closeUnbalanced(body)) + if (atEnd.ok) yield atEnd.value + + // (b) it under-closed before a later sibling — close at that boundary instead. + // Capped so a pathological reply can't cost unbounded parse attempts. + for (const idx of memberBoundaries(body).slice(0, 8)) { + const patched = closeUnbalanced(body.slice(0, idx) + closers + body.slice(idx)) + const attempt = tryParse(patched) + if (attempt.ok) yield attempt.value + } +} + +/** + * What to tell the model after a rejected attempt. A raw parser message + * ("Expected ',' or '}' ... at position 557") is not actionable — in practice + * the model made the identical mistake on all three attempts. Name the likely + * cause instead. + */ +function retryGuidance(lastError: string, wasSyntax: boolean): string { + if (wasSyntax) { + return ( + 'Your previous reply was not valid JSON. The usual cause is a missing closing "}" or "]" ' + + 'on a deeply nested schema. Re-emit the entire object and check that every bracket you ' + + `open is also closed. Output only the JSON object. Parser said: ${lastError}` + ) + } + return `Your previous response did not match the required shape: ${lastError}` } const MAX_ATTEMPTS = 3 @@ -152,11 +273,12 @@ export async function buildOperation( .join('\n') let lastError = '' + let lastErrorWasSyntax = false for (let attempt = 1; attempt <= MAX_ATTEMPTS; attempt++) { const prompt = attempt === 1 ? basePrompt - : `${basePrompt}\n\nYour previous response was invalid: ${lastError}\nReturn ONLY a corrected JSON object.` + : `${basePrompt}\n\n${retryGuidance(lastError, lastErrorWasSyntax)}\nReturn ONLY a corrected JSON object.` let text: string try { @@ -175,14 +297,24 @@ export async function buildOperation( } try { - const parsed = OperationSchema.safeParse(extractJson(text)) - if (parsed.success) return finalize(parsed.data, event) - lastError = parsed.error.issues - .map((i) => `${i.path.join('.')}: ${i.message}`) - .join('; ') - .slice(0, 300) + // Take the first reading of the reply that satisfies the schema. For a + // well-formed reply that is the one and only candidate; for a reply with + // a dropped bracket the schema decides where it belonged. + let schemaError = '' + for (const candidate of jsonCandidates(text)) { + const parsed = OperationSchema.safeParse(candidate) + if (parsed.success) return finalize(parsed.data, event) + schemaError ||= parsed.error.issues + .map((i) => `${i.path.join('.')}: ${i.message}`) + .join('; ') + .slice(0, 300) + } + lastError = schemaError || 'no candidate parse matched the Operation schema' + lastErrorWasSyntax = false } catch (err) { + // Nothing in the reply could be read as JSON at all. lastError = (err instanceof Error ? err.message : String(err)).slice(0, 300) + lastErrorWasSyntax = true } }