diff --git a/src/lib/utils/json/coerce.ts b/src/lib/utils/json/coerce.ts index 5f261d62c..bf787dc6a 100644 --- a/src/lib/utils/json/coerce.ts +++ b/src/lib/utils/json/coerce.ts @@ -56,6 +56,78 @@ function parseOrRepair( } } +/** Bounds the recursive nested-string unwrap against pathological inputs. */ +const MAX_NESTED_UNWRAP_DEPTH = 6; + +/** + * Recursively replace any string-valued field whose content is itself a JSON + * object/array with the parsed value. Models sometimes double-encode a NESTED + * field — e.g. `{ "attachment": "{\"k\":1}" }` instead of + * `{ "attachment": { "k": 1 } }` — which fails schema validation even though the + * intended object is right there. (`coerceJsonToSchema` already unwraps a + * stringified TOP-LEVEL object; this handles the nested case.) + * + * A parsed string is NOT re-descended into: its own string fields (e.g. an + * attachment's `content`) are the model's intended values and must be left + * alone. Recursion only walks already-structural objects/arrays to find + * stringified fields anywhere in the tree. Returns a NEW value (never mutates + * the input) plus whether anything changed, so the caller can skip a redundant + * re-validation when nothing was unwrapped. Callers MUST re-validate the result + * against the schema — that gate is what keeps an over-eager unwrap (a field + * that should stay a string) from being accepted. + */ +function deepUnwrapJsonStrings( + value: unknown, + depth = 0, +): { value: unknown; changed: boolean } { + if (depth > MAX_NESTED_UNWRAP_DEPTH) { + return { value, changed: false }; + } + if (typeof value === "string") { + const s = value.trim(); + const looksJson = + (s.startsWith("{") && s.endsWith("}")) || + (s.startsWith("[") && s.endsWith("]")); + if (looksJson) { + try { + const parsed: unknown = JSON.parse(s); + if (parsed !== null && typeof parsed === "object") { + // Parsed one stringified layer. Do NOT descend into `parsed` — its + // own string fields are intended values, not double-encodings. + return { value: parsed, changed: true }; + } + } catch { + // not JSON — leave the string as-is + } + } + return { value, changed: false }; + } + if (Array.isArray(value)) { + let changed = false; + const out = value.map((item) => { + const r = deepUnwrapJsonStrings(item, depth + 1); + if (r.changed) { + changed = true; + } + return r.value; + }); + return { value: changed ? out : value, changed }; + } + if (value !== null && typeof value === "object") { + let changed = false; + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) { + const r = deepUnwrapJsonStrings(v, depth + 1); + if (r.changed) { + changed = true; + } + out[k] = r.value; + } + return { value: changed ? out : value, changed }; + } + return { value, changed: false }; +} + /** * Try to produce canonical JSON from `text`. Returns null when no JSON object * could be recovered (caller should then keep the raw text). @@ -170,6 +242,25 @@ export function coerceJsonToSchema( }; if (safeParseable.safeParse(outcome.value).success) { schemaValid.push(record); + } else { + // The model may have double-encoded a NESTED field as a JSON string + // (e.g. `{"attachment":"{...}"}` instead of `{"attachment":{...}}`), + // which fails validation even though the intended object is present. + // Unwrap stringified object/array fields and re-validate before giving + // up — the safeParse gate rejects any over-eager unwrap. + const unwrapped = deepUnwrapJsonStrings(outcome.value); + if ( + unwrapped.changed && + unwrapped.value !== null && + typeof unwrapped.value === "object" && + safeParseable.safeParse(unwrapped.value).success + ) { + schemaValid.push({ + value: unwrapped.value, + repaired: true, + truncated: candidate.truncated, + }); + } } } diff --git a/test/continuous-test-suite-coerce-nested-unwrap.ts b/test/continuous-test-suite-coerce-nested-unwrap.ts new file mode 100644 index 000000000..0957ea555 --- /dev/null +++ b/test/continuous-test-suite-coerce-nested-unwrap.ts @@ -0,0 +1,145 @@ +#!/usr/bin/env tsx +/** + * Continuous Test Suite: coerce deep-unwrap of nested stringified JSON (no API). + * + * Models sometimes double-encode a NESTED field as a JSON *string* — + * `{ "attachment": "{...}" }` instead of `{ "attachment": { ... } }` — which + * fails schema validation even though the intended object is right there. This + * was observed in production (Tara's `{summary, attachment}` envelope): the + * `attachment` came back as a JSON string, so the declared `pdf` was lost and + * the content degraded to a markdown/text file. + * + * coerceJsonToSchema already unwraps a stringified TOP-LEVEL object; this suite + * locks in the NESTED-field unwrap: when no candidate passes the schema, string + * fields that are themselves JSON objects/arrays are parsed and the result is + * re-validated. Crucially, the re-validation gate prevents over-unwrapping a + * field that should stay a string, and a parsed object is not re-descended into + * (so an attachment's own `content` is preserved verbatim). + * + * Run: npx tsx test/continuous-test-suite-coerce-nested-unwrap.ts + */ +import { z } from "zod"; +import { defineSuite, assertEqual } from "./helpers/harness.js"; +import { coerceJsonToSchema } from "../src/lib/utils/json/coerce.js"; + +const { test, runSuite } = defineSuite( + "Coerce nested stringified-object unwrap", +); + +// Mirrors curator's StructuredAgentResponseSchema: attachment must be a nested +// object (or null) with string fields. +const schema = z.object({ + summary: z.string().min(1), + attachment: z + .object({ + filename: z.string(), + extension: z.string(), + mimetype: z.string(), + content: z.string(), + }) + .nullable(), +}); + +const obj = ( + r: ReturnType, +): Record => + (r?.structuredData ?? {}) as Record; +const att = ( + r: ReturnType, +): Record => + (obj(r).attachment ?? {}) as Record; + +await test("unwraps a stringified nested attachment object (the production bug)", () => { + const attachment = { + filename: "tara_structural_response_test", + extension: "pdf", + mimetype: "application/pdf", + content: "# Report\n\nbody", + }; + const text = JSON.stringify({ + summary: "done", + attachment: JSON.stringify(attachment), + }); + const r = coerceJsonToSchema(text, schema); + assertEqual( + typeof att(r).filename, + "string", + "attachment became a real object", + ); + assertEqual(att(r).extension, "pdf", "declared extension recovered"); + assertEqual(att(r).content, "# Report\n\nbody", "content preserved"); +}); + +await test("does not re-descend into the parsed object: JSON-looking content stays a string", () => { + const attachment = { + filename: "data", + extension: "json", + mimetype: "application/json", + content: '{"x":1}', // legitimately a JSON string as the file's content + }; + const text = JSON.stringify({ + summary: "done", + attachment: JSON.stringify(attachment), + }); + const r = coerceJsonToSchema(text, schema); + assertEqual( + typeof att(r).filename, + "string", + "attachment unwrapped to object", + ); + assertEqual( + att(r).content, + '{"x":1}', + "content kept as a string, not re-parsed", + ); +}); + +await test("a clean nested object passes through unchanged (no unwrap needed)", () => { + const clean = { + summary: "ok", + attachment: { + filename: "f", + extension: "md", + mimetype: "text/markdown", + content: "x", + }, + }; + const r = coerceJsonToSchema(JSON.stringify(clean), schema); + assertEqual(att(r).filename, "f", "clean object preserved"); + assertEqual(att(r).extension, "md", "clean extension preserved"); +}); + +await test("null attachment is preserved", () => { + const r = coerceJsonToSchema( + JSON.stringify({ summary: "hi", attachment: null }), + schema, + ); + assertEqual(obj(r).attachment, null, "null attachment preserved"); +}); + +await test("does NOT unwrap a JSON-looking value that is validly a string", () => { + // summary is a string that happens to look like JSON; the schema wants a + // string, so the direct parse already validates and deep-unwrap never runs. + const summaryStr = '{"not":"a real summary object"}'; + const r = coerceJsonToSchema( + JSON.stringify({ summary: summaryStr, attachment: null }), + schema, + ); + assertEqual( + obj(r).summary, + summaryStr, + "JSON-looking summary kept as a string", + ); +}); + +await test("no schema → nested string is left untouched (first parseable wins)", () => { + const text = JSON.stringify({ summary: "x", attachment: '{"a":1}' }); + const r = coerceJsonToSchema(text); // no schema to gate an unwrap + assertEqual( + typeof obj(r).attachment, + "string", + "without a schema, the nested string is not unwrapped", + ); +}); + +await runSuite();