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([]); + }); +});