From a5e468829cc0de244bc4d40b139a3498d7ebc67a Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 20 Aug 2026 17:29:00 +0200 Subject: [PATCH] fix(llm-parse): stop discarding an extraction Anthropic sent as a string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the Anthropic provider, an import sometimes reported that no reservations were found even though the document held one and the model had extracted it. Nothing was logged, because from the code's point of view nothing failed: the value simply was not an array, and the check that only accepted arrays returned an empty list. What reached the user was indistinguishable from a document with no booking in it. The tool declares `reservations` as an array and the call forces the tool, so it usually is one. Sometimes the model serialises the same content as a JSON-encoded string instead — either a stringified array, or stringified and wrapped in `{ reservations: ... }` again. Because that is a serialisation choice made per call, the same file imported fine on one attempt and came back empty on the next. Normalising is now one function next to the lenient JSON parser, and the OpenAI-compatible client uses it too — it had grown its own version of the object-unwrap half, and two copies of this rule would drift. It unwraps a string once and no further. A value that is still not a list after that is not a list, and reaching deeper would start turning prose into bookings. Reported with the exact file and lines, and with both observed shapes. Closes #1968 --- .../llm-parse/clients/anthropic.client.ts | 4 +- .../clients/openai-compatible.client.ts | 9 +-- server/src/nest/llm-parse/lenient-json.ts | 36 +++++++++++ .../tests/unit/nest/llm-parse/clients.test.ts | 30 +++++++++ .../unit/nest/llm-parse/lenient-json.test.ts | 61 ++++++++++++++++++- 5 files changed, 130 insertions(+), 10 deletions(-) diff --git a/server/src/nest/llm-parse/clients/anthropic.client.ts b/server/src/nest/llm-parse/clients/anthropic.client.ts index eb18135c0..983b5b650 100644 --- a/server/src/nest/llm-parse/clients/anthropic.client.ts +++ b/server/src/nest/llm-parse/clients/anthropic.client.ts @@ -1,5 +1,6 @@ import type { LlmExtractionClient, LlmExtractionInput } from '../llm-provider.interface'; import { safeFetchLlm } from '../../../utils/ssrfGuard'; +import { toReservationList } from '../lenient-json'; const TIMEOUT_MS = 120_000; const MAX_TOKENS = 8192; @@ -80,8 +81,7 @@ export class AnthropicClient implements LlmExtractionClient { } const toolUse = data.content?.find(b => b.type === 'tool_use' && b.name === TOOL_NAME); - const reservations = toolUse?.input?.reservations; - return Array.isArray(reservations) ? (reservations as Record[]) : []; + return toReservationList(toolUse?.input?.reservations); } } diff --git a/server/src/nest/llm-parse/clients/openai-compatible.client.ts b/server/src/nest/llm-parse/clients/openai-compatible.client.ts index 38ec22fef..39772964e 100644 --- a/server/src/nest/llm-parse/clients/openai-compatible.client.ts +++ b/server/src/nest/llm-parse/clients/openai-compatible.client.ts @@ -1,6 +1,6 @@ import type { LlmExtractionClient, LlmExtractionInput } from '../llm-provider.interface'; import { isNuExtractModel, buildNuExtractUserText, nuExtractToKiReservations } from './nuextract'; -import { parseLenientJson } from '../lenient-json'; +import { parseLenientJson, toReservationList } from '../lenient-json'; import { safeFetchLlm } from '../../../utils/ssrfGuard'; // Generous: a local CPU model (Ollama, no GPU) may cold-load several GB and then @@ -136,10 +136,5 @@ const USER_TEXT = 'Extract every travel reservation from the following document /** Tolerant parse: strip code fences, JSON(5).parse, pull `reservations`. `[]` on failure. */ function parseReservations(content: string | undefined | null): Record[] { - const parsed = parseLenientJson(content); - if (Array.isArray(parsed)) return parsed as Record[]; - if (parsed && typeof parsed === 'object' && Array.isArray((parsed as { reservations?: unknown }).reservations)) { - return (parsed as { reservations: Record[] }).reservations; - } - return []; + return toReservationList(parseLenientJson(content)); } diff --git a/server/src/nest/llm-parse/lenient-json.ts b/server/src/nest/llm-parse/lenient-json.ts index 5a9f930cf..3b7e27dbb 100644 --- a/server/src/nest/llm-parse/lenient-json.ts +++ b/server/src/nest/llm-parse/lenient-json.ts @@ -29,3 +29,39 @@ export function parseLenientJson(content: string | undefined | null): unknown { } } } + +/** + * Whatever a provider handed back, as a list of reservation nodes. + * + * The extraction tool declares `reservations` as an array and the call forces + * that tool, so the answer should already be one. It is not always. Anthropic + * sometimes serialises its tool input as a JSON-encoded string instead (#1968), + * in either of two shapes seen in the wild: + * + * "[{\"@type\":\"LodgingReservation\", …}]" a stringified array + * "{\"reservations\":[{…}]}" stringified and re-wrapped + * + * Whether it happens is up to how the model serialises that particular call, so + * the same document imported fine one minute and came back empty the next — + * indistinguishable, to the person waiting, from a document with no booking in + * it. Nothing was logged, because nothing had failed: the value simply was not + * an array, and the check that only accepted arrays dropped a good extraction. + * + * One unwrap of a string, not a loop: a value that is still not a list after + * that is genuinely not one, and guessing further would start inventing + * bookings out of prose. + */ +export function toReservationList(value: unknown): Record[] { + const list = (v: unknown): Record[] | null => { + if (Array.isArray(v)) return v as Record[]; + if (v && typeof v === 'object' && Array.isArray((v as { reservations?: unknown }).reservations)) { + return (v as { reservations: Record[] }).reservations; + } + return null; + }; + + const direct = list(value); + if (direct) return direct; + if (typeof value === 'string') return list(parseLenientJson(value)) ?? []; + return []; +} diff --git a/server/tests/unit/nest/llm-parse/clients.test.ts b/server/tests/unit/nest/llm-parse/clients.test.ts index 3ea9da66d..2a013b10f 100644 --- a/server/tests/unit/nest/llm-parse/clients.test.ts +++ b/server/tests/unit/nest/llm-parse/clients.test.ts @@ -209,6 +209,36 @@ describe('AnthropicClient', () => { expect(body.tools[0].name).toBe('emit_reservations'); }); + /* + * The model does not always send its tool input as the array the schema + * declares — sometimes it sends the same thing JSON-encoded, in one of two + * shapes. The old check only accepted arrays, so a good extraction was + * discarded and the import reported "no reservations found" with nothing in + * the log: exactly what a document containing no booking produces. Whether it + * happened came down to how the model serialised that particular call, so the + * same file behaved differently between attempts (#1968). + */ + it('reads a tool input the model sent as a stringified array', async () => { + mockFetch(() => + jsonResponse({ stop_reason: 'tool_use', content: [{ type: 'tool_use', name: 'emit_reservations', input: { reservations: JSON.stringify([{ '@type': 'LodgingReservation' }]) } }] }), + ); + expect(await new AnthropicClient().extract(baseInput)).toEqual([{ '@type': 'LodgingReservation' }]); + }); + + it('reads one the model stringified and wrapped again', async () => { + mockFetch(() => + jsonResponse({ stop_reason: 'tool_use', content: [{ type: 'tool_use', name: 'emit_reservations', input: { reservations: JSON.stringify({ reservations: [{ '@type': 'FlightReservation' }] }) } }] }), + ); + expect(await new AnthropicClient().extract(baseInput)).toEqual([{ '@type': 'FlightReservation' }]); + }); + + it('still answers empty when the tool input really is not a list', async () => { + mockFetch(() => + jsonResponse({ stop_reason: 'tool_use', content: [{ type: 'tool_use', name: 'emit_reservations', input: { reservations: 'no bookings in this document' } }] }), + ); + expect(await new AnthropicClient().extract(baseInput)).toEqual([]); + }); + it('throws on a refusal stop_reason', async () => { mockFetch(() => jsonResponse({ stop_reason: 'refusal', content: [] })); await expect(new AnthropicClient().extract(baseInput)).rejects.toThrow(/declined/i); diff --git a/server/tests/unit/nest/llm-parse/lenient-json.test.ts b/server/tests/unit/nest/llm-parse/lenient-json.test.ts index 841f4aa52..33d739681 100644 --- a/server/tests/unit/nest/llm-parse/lenient-json.test.ts +++ b/server/tests/unit/nest/llm-parse/lenient-json.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect } from 'vitest'; -import { parseLenientJson } from '../../../../src/nest/llm-parse/lenient-json'; +import { parseLenientJson, toReservationList } from '../../../../src/nest/llm-parse/lenient-json'; describe('parseLenientJson', () => { it('parses strict JSON', () => { @@ -47,3 +47,62 @@ describe('parseLenientJson', () => { expect(parseLenientJson('this is prose, not json')).toBeNull(); }); }); + +/** + * The shapes a forced tool call actually comes back in (#1968). + * + * The tool declares `reservations` as an array and the call forces the tool, so + * the answer should be an array and usually is. Anthropic sometimes serialises + * its tool input as a JSON-encoded string instead, and the check that only + * accepted arrays dropped those extractions silently: no error, no log, and an + * import that reported "no reservations found" — indistinguishable from a + * document that really had none. Because it depends on how the model serialises + * that particular call, the same file imported fine one minute and empty the + * next. + */ +describe('toReservationList', () => { + const node = { '@type': 'LodgingReservation', reservationNumber: 'ABC123' }; + + it('passes an array straight through, which is the ordinary case', () => { + expect(toReservationList([node])).toEqual([node]); + }); + + it('unwraps the object shape the OpenAI-compatible path produces', () => { + expect(toReservationList({ reservations: [node] })).toEqual([node]); + }); + + it('parses a stringified array', () => { + expect(toReservationList(JSON.stringify([node]))).toEqual([node]); + }); + + it('parses a stringified object that wraps the list again', () => { + expect(toReservationList(JSON.stringify({ reservations: [node] }))).toEqual([node]); + }); + + it('takes the relaxed JSON a model might emit inside that string', () => { + expect(toReservationList("[{'@type': 'FlightReservation', flightNumber: 'LH400',}]")) + .toEqual([{ '@type': 'FlightReservation', flightNumber: 'LH400' }]); + }); + + /* + * The other half of the fix: it must not reach so far that prose starts + * counting as a booking. Anything that is not a list after one unwrap is not + * a list. + */ + it('answers empty for something that is genuinely not a list', () => { + expect(toReservationList(undefined)).toEqual([]); + expect(toReservationList(null)).toEqual([]); + expect(toReservationList('there are no reservations in this document')).toEqual([]); + expect(toReservationList({ hotel: 'Grand' })).toEqual([]); + expect(toReservationList(42)).toEqual([]); + }); + + it('does not unwrap twice, so a doubly-encoded string stays refused', () => { + expect(toReservationList(JSON.stringify(JSON.stringify([node])))).toEqual([]); + }); + + it('keeps an empty list an empty list', () => { + expect(toReservationList([])).toEqual([]); + expect(toReservationList('{"reservations":[]}')).toEqual([]); + }); +});