mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
fix(booking-import): say why a recognised booking could not be imported
A document the model read correctly could still come back as "No reservations could be extracted". Every type mapper can answer null when the node carries no reservationFor, or one with no name, and both call sites pushed the item only if there was one — with no else. The unknown-@type branch next to them does warn, so a node TREK did not recognise was reported while a node it recognised but could not map vanished: no item, no warning, nothing in the log. Because the file did produce a node, the "no reservations found" fallback was skipped too, and the preview arrived empty and silent. Every recognised type could disappear this way, on both the kitinerary and the AI path. Each of them now warns, naming the file, the index, the type and the reason. Three smaller silences went with it. The debug line labelled "Raw LLM Response" printed the already-parsed array, so its inspect formatting — single quotes, bare keys — read as a provider emitting broken JSON and sent this report at the JSON parser, which has handled that shape since 4.0.0. A single reservation answered on its own, with no array and no wrapper, was dropped by the list unwrapper although the schema downstream would have taken it. And a response nobody could read returned an empty list, which is the same answer as a voucher with no booking in it; it now raises, on the Anthropic client too, where a reply cut off at the token limit produced exactly this report's symptom. The plugin AI surface keeps its old empty answer, since a value is what a plugin contracts for. The retry ladder gained a rung that sends no response_format at all, for providers that support neither json_schema nor json_object. Closes #2375
This commit is contained in:
@@ -65,9 +65,56 @@ describe('OpenAiCompatibleClient', () => {
|
||||
expect(out).toEqual([{ '@type': 'LodgingReservation', checkinTime: '2026-08-28T00:00:00', price: 146.25 }]);
|
||||
});
|
||||
|
||||
it('returns [] on malformed content', async () => {
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: 'not json' } }] }));
|
||||
expect(await new OpenAiCompatibleClient().extract(baseInput)).toEqual([]);
|
||||
/*
|
||||
* Prose and an honest empty list both came back as `[]`, so a provider that
|
||||
* answered neither JSON nor a reservation looked exactly like a document
|
||||
* holding no booking: no item, no warning, nothing in the log (#2375). Only
|
||||
* what nothing could read throws — a parsed answer that simply holds no
|
||||
* reservation keeps the old empty path.
|
||||
*/
|
||||
it('throws on content the lenient parser cannot read (#2375)', async () => {
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: 'I could not find a booking in this document.' } }] }));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/did not answer with JSON/);
|
||||
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: '42' } }] }));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/did not answer with JSON/);
|
||||
});
|
||||
|
||||
it('throws when the response carries no content at all (#2375)', async () => {
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: '' } }] }));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/empty response/);
|
||||
|
||||
// A content filter or a zero-token completion leaves the field off entirely.
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: {} }] }));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/empty response/);
|
||||
});
|
||||
|
||||
it('still answers empty for a document that genuinely holds no booking', async () => {
|
||||
for (const content of ['[]', '{"reservations":[]}', '{}', '{"reservations":null}', JSON.stringify('{"reservations":[]}')]) {
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content } }] }));
|
||||
expect(await new OpenAiCompatibleClient().extract(baseInput)).toEqual([]);
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps the unreadable response out of the message on a managed instance', async () => {
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: 'Booking for Jane Doe, 12 Rue de Rivoli' } }] }));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/Jane Doe/);
|
||||
|
||||
// The response is the booking, and on a managed install the operator's log
|
||||
// is not the place for it — same split as the extracted text.
|
||||
vi.stubEnv('TREK_MANAGED', 'true');
|
||||
try {
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/did not answer with JSON$/);
|
||||
} finally {
|
||||
vi.unstubAllEnvs();
|
||||
}
|
||||
});
|
||||
|
||||
it('reads a single reservation the model answered on its own', async () => {
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: '{"@type":"LodgingReservation","reservationNumber":"733"}' } }] }));
|
||||
expect(await new OpenAiCompatibleClient().extract(baseInput)).toEqual([
|
||||
{ '@type': 'LodgingReservation', reservationNumber: '733' },
|
||||
]);
|
||||
});
|
||||
|
||||
it('throws on non-2xx', async () => {
|
||||
@@ -187,12 +234,37 @@ describe('OpenAiCompatibleClient', () => {
|
||||
expect(second.response_format.type).toBe('json_object');
|
||||
});
|
||||
|
||||
it('throws when the json_object retry also fails (400 twice)', async () => {
|
||||
/*
|
||||
* A provider behind a proxy that supports neither grammar rejected both rungs
|
||||
* and the import failed hard, which is what `drop_params` on the proxy side was
|
||||
* being used to work around (#2375). The last attempt sends the request the
|
||||
* proxy would have produced.
|
||||
*/
|
||||
it('drops response_format entirely once json_schema and json_object are both refused (#2375)', async () => {
|
||||
safeFetchLlmMock
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'no json_schema' }, false, 400))
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'no json_object either' }, false, 400));
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'no json_object either' }, false, 400))
|
||||
.mockResolvedValueOnce(jsonResponse({ choices: [{ message: { content: '{"reservations":[{"@type":"FlightReservation"}]}' } }] }));
|
||||
const out = await new OpenAiCompatibleClient().extract(baseInput);
|
||||
expect(out).toEqual([{ '@type': 'FlightReservation' }]);
|
||||
expect(safeFetchLlmMock).toHaveBeenCalledTimes(3);
|
||||
|
||||
const bodies = safeFetchLlmMock.mock.calls.map(c => JSON.parse((c[1] as RequestInit).body as string));
|
||||
expect(bodies[0].response_format.type).toBe('json_schema');
|
||||
expect(bodies[1].response_format).toEqual({ type: 'json_object' });
|
||||
expect('response_format' in bodies[2]).toBe(false);
|
||||
// Only the grammar goes; the prompt, the model and the token cap are untouched.
|
||||
expect(bodies[2].messages).toEqual(bodies[0].messages);
|
||||
expect(bodies[2].max_tokens).toBe(4096);
|
||||
});
|
||||
|
||||
it('throws when the attempt without response_format also fails (400 three times)', async () => {
|
||||
safeFetchLlmMock
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'no json_schema' }, false, 400))
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'no json_object either' }, false, 400))
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'nothing this model accepts' }, false, 400));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/400/);
|
||||
expect(safeFetchLlmMock).toHaveBeenCalledTimes(2);
|
||||
expect(safeFetchLlmMock).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it('does not retry the NuExtract path on 400', async () => {
|
||||
@@ -255,6 +327,17 @@ describe('OpenAiCompatibleClient — NuExtract path', () => {
|
||||
expect(body.response_format).toBeUndefined();
|
||||
});
|
||||
|
||||
it('throws when NuExtract answered something nothing could read (#2375)', async () => {
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: 'I am NuExtract, a template filling model.' } }] }));
|
||||
await expect(
|
||||
new OpenAiCompatibleClient().extract({ ...baseInput, model: 'nuextract', text: 'Hotel doc' }),
|
||||
).rejects.toThrow(/did not answer with JSON/);
|
||||
|
||||
// A template that came back filled with nothing is still an answer.
|
||||
mockFetch(() => jsonResponse({ choices: [{ message: { content: '{"type":null,"name":null}' } }] }));
|
||||
expect(await new OpenAiCompatibleClient().extract({ ...baseInput, model: 'nuextract', text: 'Hotel doc' })).toEqual([]);
|
||||
});
|
||||
|
||||
it('keeps the system prompt and response_format for non-NuExtract models', async () => {
|
||||
const fetchFn = mockFetch(() => jsonResponse({ choices: [{ message: { content: '{"reservations":[]}' } }] }));
|
||||
await new OpenAiCompatibleClient().extract({ ...baseInput, model: 'qwen2.5:7b' });
|
||||
@@ -306,6 +389,38 @@ describe('AnthropicClient', () => {
|
||||
expect(await new AnthropicClient().extract(baseInput)).toEqual([]);
|
||||
});
|
||||
|
||||
/*
|
||||
* A forced tool that was never called, and a run that stopped at the cap with a
|
||||
* fragment of one, both came back as [] — so a long voucher reached the person
|
||||
* as "no reservations found", with an empty warnings array and nothing in the
|
||||
* log. That is the #2375 symptom on the provider the dropdown offers first, so
|
||||
* the same rule applies here: only what the client could read is an answer.
|
||||
*/
|
||||
it('throws when the answer was cut off at the token cap (#2375)', async () => {
|
||||
mockFetch(() =>
|
||||
jsonResponse({ stop_reason: 'max_tokens', content: [{ type: 'tool_use', name: 'emit_reservations', input: {} }] }),
|
||||
);
|
||||
await expect(new AnthropicClient().extract(baseInput)).rejects.toThrow(/cut off at the 8192-token limit/);
|
||||
});
|
||||
|
||||
it('throws when the forced tool was never called (#2375)', async () => {
|
||||
mockFetch(() =>
|
||||
jsonResponse({ stop_reason: 'end_turn', content: [{ type: 'text', text: 'I could not find a booking in this document.' }] }),
|
||||
);
|
||||
await expect(new AnthropicClient().extract(baseInput)).rejects.toThrow(/without calling emit_reservations/);
|
||||
|
||||
// A response carrying no content block at all lands on the same message.
|
||||
mockFetch(() => jsonResponse({ stop_reason: 'end_turn' }));
|
||||
await expect(new AnthropicClient().extract(baseInput)).rejects.toThrow(/without calling emit_reservations/);
|
||||
});
|
||||
|
||||
it('still answers empty for a tool call that carries an empty list', async () => {
|
||||
mockFetch(() =>
|
||||
jsonResponse({ stop_reason: 'tool_use', content: [{ type: 'tool_use', name: 'emit_reservations', input: { reservations: [] } }] }),
|
||||
);
|
||||
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);
|
||||
|
||||
@@ -79,6 +79,17 @@ describe('toReservationList', () => {
|
||||
expect(toReservationList(JSON.stringify({ reservations: [node] }))).toEqual([node]);
|
||||
});
|
||||
|
||||
/*
|
||||
* A single booking answered on its own, with no array and no wrapper. The
|
||||
* check only knew the two list shapes, so a perfectly good extraction was
|
||||
* dropped without a word — the same silent empty preview as #1968, from a
|
||||
* different direction (#2375).
|
||||
*/
|
||||
it('takes a single reservation answered on its own', () => {
|
||||
expect(toReservationList(node)).toEqual([node]);
|
||||
expect(toReservationList(JSON.stringify(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' }]);
|
||||
|
||||
@@ -61,6 +61,7 @@ import { DbRpc } from '../../../src/nest/plugins/host/rpc/db.rpc';
|
||||
import type { PluginUserSettingsService } from '../../../src/nest/plugins/plugin-user-settings.service';
|
||||
import { MetaRpc } from '../../../src/nest/plugins/host/rpc/meta.rpc';
|
||||
import { HostSurfaceRpc } from '../../../src/nest/plugins/host/rpc/host-surface.rpc';
|
||||
import { UnreadableLlmResponse } from '../../../src/nest/llm-parse/clients/openai-compatible.client';
|
||||
import { getPluginDataDb, closePluginDataDb } from '../../../src/nest/plugins/host/plugin-host-state';
|
||||
import { db as mockDb } from '../../../src/db/database';
|
||||
import { DatabaseService } from '../../../src/nest/database/database.service';
|
||||
@@ -472,6 +473,27 @@ describe('HostSurfaceRpc — users, broadcasts, notify, ai, oauth, scheduler', (
|
||||
expect((await call(makeHost('surface', 'ai:invoke'), 'ai.complete', { prompt: 'p' }, 5)).result).toEqual({ text: '' });
|
||||
});
|
||||
|
||||
/*
|
||||
* The clients raise on a response nothing could read so the booking import can
|
||||
* warn per file (#2375). This surface answered an empty value for a model that
|
||||
* talks prose long before that, and it is versioned on its own, so the plugin
|
||||
* keeps the empty value — while a request that genuinely failed still has to
|
||||
* reach it as an error.
|
||||
*/
|
||||
it('HOSTRPC-026a an unreadable answer stays empty here, a failed request does not (#2375)', async () => {
|
||||
const host = makeHost('surface', 'ai:invoke');
|
||||
llmExtract.mockRejectedValueOnce(new UnreadableLlmResponse('the model did not answer with JSON: Sure, happy to help.'));
|
||||
expect((await call(host, 'ai.complete', { prompt: 'p' }, 5)).result).toEqual({ text: '' });
|
||||
|
||||
llmExtract.mockRejectedValueOnce(new UnreadableLlmResponse('the model returned an empty response'));
|
||||
expect((await call(host, 'ai.extract', { text: 't', jsonSchema: { type: 'object' } }, 5)).result).toEqual({ results: [] });
|
||||
|
||||
llmExtract.mockRejectedValueOnce(new Error('LLM request failed (503): upstream is down'));
|
||||
const failed = await call(host, 'ai.complete', { prompt: 'p' }, 5);
|
||||
expect(failed.error?.code).toBe('HOST_ERROR');
|
||||
expect(failed.error?.message).toMatch(/503/);
|
||||
});
|
||||
|
||||
it('HOSTRPC-027 oauth.getToken returns only the acting user\'s access token', async () => {
|
||||
const host = makeHost('surface', 'oauth:client');
|
||||
expect((await call(host, 'oauth.getToken', {}, 5)).result).toEqual({ accessToken: 'tok-5' });
|
||||
|
||||
@@ -107,3 +107,62 @@ describe('kitinerary mapper — printed 12-hour clocks (#2094)', () => {
|
||||
expect(items).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A recognised @type whose mapper cannot build an item used to leave nothing
|
||||
* behind: no item, no warning, no log. With a single node in the file the
|
||||
* service's own "no reservations found" never fired either, so a hotel voucher
|
||||
* the model had read correctly but namelessly came back as an empty preview,
|
||||
* indistinguishable from a document holding no booking at all (#2375).
|
||||
*/
|
||||
describe('kitinerary mapper — a recognised type that cannot be mapped (#2375)', () => {
|
||||
it('warns instead of dropping a lodging whose reservationFor carries no name', () => {
|
||||
const { items, warnings } = mapReservations([
|
||||
{ '@type': 'LodgingReservation', reservationNumber: 'HMTRSX', reservationFor: {} },
|
||||
] as any, 'airbnb.pdf');
|
||||
|
||||
expect(items).toHaveLength(0);
|
||||
expect(warnings).toEqual([
|
||||
'Incomplete LodgingReservation in airbnb.pdf[0] (no name in reservationFor) — skipped',
|
||||
]);
|
||||
});
|
||||
|
||||
it('warns for a flight that carries no reservationFor at all', () => {
|
||||
const { items, warnings } = mapReservations([
|
||||
{ '@type': 'FlightReservation', reservationNumber: 'ABC123' },
|
||||
] as any, 'ticket.eml');
|
||||
|
||||
expect(items).toHaveLength(0);
|
||||
expect(warnings).toEqual([
|
||||
'Incomplete FlightReservation in ticket.eml[0] (no reservationFor) — skipped',
|
||||
]);
|
||||
});
|
||||
|
||||
it('names the type it could not map, TouristAttractionVisit included', () => {
|
||||
const { warnings } = mapReservations([
|
||||
{ '@type': 'TouristAttractionVisit', reservationFor: {} },
|
||||
] as any, 'museum.pdf');
|
||||
|
||||
expect(warnings[0]).toBe('Incomplete TouristAttractionVisit in museum.pdf[0] (no name in reservationFor) — skipped');
|
||||
});
|
||||
|
||||
it('keeps the bookings it could map and warns only about the weak one', () => {
|
||||
const { items, warnings } = mapReservations([
|
||||
flight('MIX1', FRA, BER, '2026-06-11T10:00:00', '2026-06-11T12:00:00', 'LH 800'),
|
||||
{ '@type': 'LodgingReservation', reservationFor: {} },
|
||||
] as any, 'trip.eml');
|
||||
|
||||
expect(items).toHaveLength(1);
|
||||
expect(warnings).toHaveLength(1);
|
||||
expect(warnings[0]).toContain('trip.eml[1]');
|
||||
});
|
||||
|
||||
it('still warns exactly once for an unknown type', () => {
|
||||
const { items, warnings } = mapReservations([
|
||||
{ '@type': 'RocketLaunchReservation', reservationFor: { name: 'Starbase' } },
|
||||
] as any, 'mars.eml');
|
||||
|
||||
expect(items).toHaveLength(0);
|
||||
expect(warnings).toEqual(['Unknown type "RocketLaunchReservation" in mars.eml[0] — skipped']);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user