diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx index c8ad05d7b..7c3928794 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.extra.test.tsx @@ -1,4 +1,4 @@ -// FE-W4BGT-001 to FE-W4BGT-022 +// FE-W4BGT-001 to FE-W4BGT-024 import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import { screen, act, waitFor } from '@testing-library/react' import { render, fireEvent } from '../../../tests/helpers/render' @@ -276,3 +276,30 @@ describe('BackgroundTasksWidget — AI retry', () => { expect(await screen.findByText('No model configured')).toBeInTheDocument() }) }) + +/** + * The reporter's phone showed a green tick beside "No reservations could be + * extracted" and two warnings (#2477). A parse that found nothing is not a + * success, so it wears the warning mark; the tick stays for a parse that did. + */ +describe('BackgroundTasksWidget: the mark on a finished job (#2477)', () => { + it('FE-W4BGT-023: a job that found nothing shows the warning mark, not the success tick', () => { + useBackgroundTasksStore.setState({ + tasks: [task({ items: [], warnings: ['Incomplete LodgingReservation in booking.pdf[0] (no reservationFor)'] })], + }) + const { baseElement } = render() + + expect(screen.getByText('No reservations could be extracted from the uploaded files.')).toBeInTheDocument() + expect(baseElement.querySelector('svg.lucide-alert-triangle')).not.toBeNull() + expect(baseElement.querySelector('svg.lucide-check-circle2')).toBeNull() + expect(baseElement.querySelector('.animate-spin')).toBeNull() + }) + + it('FE-W4BGT-024: a job with items keeps the success tick, even with warnings', () => { + useBackgroundTasksStore.setState({ tasks: [task({ items: [{ id: 1 }] as never, warnings: ['could not locate "X"'] })] }) + const { baseElement } = render() + + expect(baseElement.querySelector('svg.lucide-check-circle2')).not.toBeNull() + expect(baseElement.querySelector('svg.lucide-alert-triangle')).toBeNull() + }) +}) diff --git a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx index 55ec57249..6db1ea821 100644 --- a/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx +++ b/client/src/components/BackgroundTasks/BackgroundTasksWidget.tsx @@ -1,7 +1,7 @@ import { createPortal } from 'react-dom' import { useEffect, useRef, useState } from 'react' import { useNavigate } from 'react-router' -import { Loader2, CheckCircle2, AlertCircle, X } from 'lucide-react' +import { Loader2, CheckCircle2, AlertCircle, AlertTriangle, X } from 'lucide-react' import { useTranslation } from '../../i18n' import { addListener, removeListener } from '../../api/websocket' import { reservationsApi, healthApi } from '../../api/client' @@ -141,7 +141,11 @@ export default function BackgroundTasksWidget() { >
{(task.status === 'running' || (task.status === 'done' && task.items === undefined)) && } - {task.status === 'done' && task.items !== undefined && } + {task.status === 'done' && task.items !== undefined && task.items.length > 0 && } + {/* A parse that found nothing is not a success, whatever the job + status says: the same card used to show a green tick next to + "no reservations could be extracted" (#2477). */} + {task.status === 'done' && task.items?.length === 0 && } {task.status === 'error' && }
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 5a9094a9c..f00ebec27 100644 --- a/server/src/nest/llm-parse/clients/openai-compatible.client.ts +++ b/server/src/nest/llm-parse/clients/openai-compatible.client.ts @@ -44,6 +44,12 @@ function rejectsTemperature(detail: string): boolean { && /unsupported|not supported|does not support|only the default/i.test(detail); } +/** The `response_format` the given attempt carried, by its type. */ +function sentFormat(shape: RequestShape, nuextract: boolean): string { + if (nuextract || shape.noResponseFormat) return 'none'; + return shape.jsonObject ? 'json_object' : 'json_schema'; +} + /** * OpenAI-compatible chat-completions client. Covers both the "openai" cloud * provider and the "local" provider (Ollama / vLLM / llama.cpp / LM Studio), @@ -170,6 +176,10 @@ export class OpenAiCompatibleClient implements LlmExtractionClient { if (!res.ok) { throw new Error(`LLM request failed (${res.status}): ${detail.slice(0, 300)}`); } + // Which rung answered. A report could not tell whether the schema reached the + // model at all or the ladder had fallen back to the prompt alone (#2477). + // Metadata only, so it is the same on a managed install. + console.debug(`[DEBUG] LLM answered with response_format=${sentFormat(shape, nuextract)}`); const data = (await res.json()) as { choices?: { message?: { content?: string } }[]; diff --git a/server/src/nest/llm-parse/currency-code.ts b/server/src/nest/llm-parse/currency-code.ts new file mode 100644 index 000000000..28e012808 --- /dev/null +++ b/server/src/nest/llm-parse/currency-code.ts @@ -0,0 +1,40 @@ +/** + * The ISO 4217 codes this runtime knows. Node ships full ICU, so this is the + * complete list; a build without `Intl.supportedValuesOf` leaves it empty and + * `toIsoCurrency` then settles for the shape of a code. + */ +const ISO_CURRENCIES: ReadonlySet = new Set( + typeof Intl.supportedValuesOf === 'function' ? Intl.supportedValuesOf('currency') : [], +); + +/** A symbol standing on its own. `$` alone is read as USD, as the extractor does. */ +const SYMBOLS: ReadonlyMap = new Map([ + ['€', 'EUR'], + ['£', 'GBP'], + ['¥', 'JPY'], + ['$', 'USD'], +]); + +/** + * The currency an AI answer named, as an ISO 4217 code, or undefined when it + * names none. + * + * A model's `priceCurrency` is whatever it made of the document: `EUR`, `eur`, + * `€`, `Euros`, and once `EURials` (#2477). That value used to reach the cost + * form untouched. It is read as an exact code in any case first, then as the + * code it starts with when those three letters are a real code (`EURials` and + * `Euros` are EUR, `LEK` is nothing, since the lek is ALL), then as a lone + * currency symbol. Anything else is undefined, so the caller can fall back to + * the document or drop the field instead of passing on a currency nobody can + * convert. + */ +export function toIsoCurrency(raw: unknown, known: ReadonlySet = ISO_CURRENCIES): string | undefined { + if (typeof raw !== 'string') return undefined; + const value = raw.trim().toUpperCase(); + if (!value) return undefined; + const isCode = (code: string) => (known.size > 0 ? known.has(code) : /^[A-Z]{3}$/.test(code)); + if (isCode(value)) return value; + const prefix = /^[A-Z]{3}/.exec(value)?.[0]; + if (prefix && isCode(prefix)) return prefix; + return SYMBOLS.get(value); +} diff --git a/server/src/nest/llm-parse/llm-parse.service.ts b/server/src/nest/llm-parse/llm-parse.service.ts index ae559aa88..b83c817ae 100644 --- a/server/src/nest/llm-parse/llm-parse.service.ts +++ b/server/src/nest/llm-parse/llm-parse.service.ts @@ -4,7 +4,8 @@ import { LlmConfigResolver } from './llm-config.resolver'; import { buildSystemPrompt, KI_RESERVATION_JSON_SCHEMA } from './llm-prompt'; import type { LlmExtractionInput } from './llm-provider.interface'; import { isPdf, extractText } from './text-extract'; -import { routeExtraction, detectFlightNumbers } from './router/extraction-router'; +import { routeExtraction, detectFlightNumbers, extractTotalPrice } from './router/extraction-router'; +import { toIsoCurrency } from './currency-code'; import { Injectable } from '@nestjs/common'; import { kiReservationSchema } from '@trek/shared'; import { RuntimeEnvService } from '../app-config/runtime-env.service'; @@ -130,10 +131,21 @@ export class LlmParseService { } const kiItems: KiReservation[] = []; + // A model can list one stay twice, and did (#1638, #2477): two identical + // nodes are one booking, not two hotels. Only exact copies, and only on this + // path; the kitinerary extractor reports what the document holds. + const seen = new Set(); for (const node of raw) { const result = kiReservationSchema.safeParse(node); - if (result.success) kiItems.push(normalizeNode(result.data) as unknown as KiReservation); - else warnings.push(`${file.originalName}: skipped an unrecognized AI result`); + if (!result.success) { + warnings.push(`${file.originalName}: skipped an unrecognized AI result`); + continue; + } + const normalized = normalizeCurrency(normalizeNode(result.data), input.text); + const key = stableKey(normalized); + if (seen.has(key)) continue; + seen.add(key); + kiItems.push(normalized as unknown as KiReservation); } return { kiItems, warnings }; @@ -182,3 +194,31 @@ function normalizeNode(node: Record): Record { out.reservationFor = reservationFor; return out; } + +/** + * The node with its `priceCurrency` as an ISO 4217 code, or without one. + * + * What the model wrote wins when it reads as a code. Otherwise, and only when + * the node carries a price, the currency comes from the document's own total + * (a Booking.com print shows `€ 89,35`). When neither yields a code the field + * is dropped: an empty currency lets the cost form pick one, a made up one + * (`EURials`, #2477) is saved and cannot be converted. + */ +function normalizeCurrency(node: Record, documentText: string | undefined): Record { + const hasPrice = node.price != null && node.price !== ''; + if (node.priceCurrency == null && !hasPrice) return node; + const code = + toIsoCurrency(node.priceCurrency) ?? + (hasPrice && documentText ? toIsoCurrency(extractTotalPrice(documentText)?.currency) : undefined); + const { priceCurrency: _dropped, ...rest } = node; + return code ? { ...rest, priceCurrency: code } : rest; +} + +/** JSON with every object's keys sorted, so two equal nodes give one string. */ +function stableKey(value: unknown): string { + return JSON.stringify(value, (_key, v: unknown) => + v && typeof v === 'object' && !Array.isArray(v) + ? Object.fromEntries(Object.entries(v as Record).sort(([a], [b]) => a.localeCompare(b))) + : v, + ); +} diff --git a/server/src/nest/reservation-import/reservation-import.controller.ts b/server/src/nest/reservation-import/reservation-import.controller.ts index af77fb9d7..25a1c81e7 100644 --- a/server/src/nest/reservation-import/reservation-import.controller.ts +++ b/server/src/nest/reservation-import/reservation-import.controller.ts @@ -35,6 +35,11 @@ const MAX_FILES = 5; const UPLOAD = { storage: memoryStorage(), limits: { fileSize: MAX_FILE_BYTES, files: MAX_FILES }, + // Like every other upload route (storage-upload.factory.ts). Multer reads the + // name as latin1 otherwise, so "Bestätigung.pdf" came back as "Bestätigung.pdf" + // in the warnings, and the review could not find the file to attach it to the + // booking, because the client matches on the name it sent (#2477). + defParamCharset: 'utf8' as const, }; /** diff --git a/server/tests/e2e/booking-import.e2e.test.ts b/server/tests/e2e/booking-import.e2e.test.ts new file mode 100644 index 000000000..3cc2c1a7e --- /dev/null +++ b/server/tests/e2e/booking-import.e2e.test.ts @@ -0,0 +1,225 @@ +/** + * Booking import e2e (#2477): a Booking.com print read by Gemini through its + * OpenAI-compatible endpoint, driven through the real upload route. + * + * Real: the JwtAuthGuard and TripAccessGuard, multer on the import route, the + * text extraction of a PDF, LlmParseService, the OpenAI-compatible client with + * its request body, and the kitinerary mapper. Replaced: the kitinerary binary + * (not installed, as on the reporter's instance, so the AI path runs) and the + * one outbound call, `safeFetchLlm`, which answers the way Gemini did. + * + * The PDF is built here from synthetic text; the reporter's document is never + * part of the repository. + */ +import { describe, it, expect, beforeAll, afterAll, beforeEach, vi } from 'vitest'; +import request from 'supertest'; +import cookieParser from 'cookie-parser'; +import type { Server } from 'http'; +import { Test } from '@nestjs/testing'; +import { sessionCookie } from './harness'; + +const { canAccessTrip, safeFetchLlm } = vi.hoisted(() => ({ canAccessTrip: vi.fn(), safeFetchLlm: vi.fn() })); +// The temp db is born inside the factory, which runs before anything imports it, +// and read back below through the mocked module itself. +vi.mock('../../src/db/database', async () => { + const { default: Database } = await import('better-sqlite3'); + const tmp = new Database(':memory:'); + tmp.exec('PRAGMA journal_mode = WAL'); + tmp.exec('PRAGMA foreign_keys = ON'); + return { + db: tmp, + canAccessTrip, + isOwner: vi.fn(() => true), + getPlaceWithTags: vi.fn(), + closeDb: () => {}, + reinitialize: () => {}, + }; +}); +vi.mock('../../src/websocket', () => ({ broadcast: vi.fn(), broadcastToUser: vi.fn() })); +vi.mock('../../src/utils/ssrfGuard', async (orig) => ({ ...(await orig>()), safeFetchLlm })); + +import { db } from '../../src/db/database'; +import { createTables } from '../../src/db/schema'; +import { runMigrations } from '../../src/db/migrations'; +import { DatabaseModule } from '../../src/nest/database/database.module'; +import { RealtimeModule } from '../../src/nest/realtime/realtime.module'; +import { ReservationImportModule } from '../../src/nest/reservation-import/reservation-import.module'; +import { KitineraryExtractorService } from '../../src/nest/booking-import/kitinerary-extractor.service'; +import { NotificationsService } from '../../src/nest/notifications/notifications.service'; +import { PermissionsService } from '../../src/nest/permissions/permissions.service'; +import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter'; +import { ZodValidationPipe } from '../../src/nest/common/zod-validation.pipe'; + +/** A one-page PDF whose text layer holds `lines`, in the standard Helvetica. */ +function pdfWithText(lines: string[]): Buffer { + // PDF string literal: escape \ ( ), and write the euro sign as its WinAnsi byte. + const esc = (s: string) => s.replace(/[\\()]/g, (c) => '\\' + c).replace(/€/g, '\x80'); + const content = ['BT', '/F1 10 Tf', '12 TL', '20 780 Td', ...lines.map((l) => `(${esc(l)}) '`), 'ET'].join('\n'); + const objects = [ + '<< /Type /Catalog /Pages 2 0 R >>', + '<< /Type /Pages /Kids [3 0 R] /Count 1 >>', + '<< /Type /Page /Parent 2 0 R /MediaBox [0 0 595 842] /Resources << /Font << /F1 5 0 R >> >> /Contents 4 0 R >>', + `<< /Length ${Buffer.byteLength(content, 'latin1')} >>\nstream\n${content}\nendstream`, + '<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica /Encoding /WinAnsiEncoding >>', + ]; + let body = '%PDF-1.4\n'; + const offsets: number[] = []; + objects.forEach((o, i) => { + offsets.push(Buffer.byteLength(body, 'latin1')); + body += `${i + 1} 0 obj\n${o}\nendobj\n`; + }); + const xref = Buffer.byteLength(body, 'latin1'); + const rows = offsets.map((n) => `${String(n).padStart(10, '0')} 00000 n \n`).join(''); + body += `xref\n0 ${objects.length + 1}\n0000000000 65535 f \n${rows}`; + body += `trailer\n<< /Size ${objects.length + 1} /Root 1 0 R >>\nstartxref\n${xref}\n%%EOF\n`; + return Buffer.from(body, 'latin1'); +} + +const PRINT = pdfWithText([ + 'Harbour View Inn', + 'Address: Example Road 1, 1000 Sample Town', + 'Preis (für 2 Gäste)', + '€ 89,35', + 'ANREISE 6 SEPTEMBER 13:00 ABREISE 7 SEPTEMBER 11:00', +]); + +/** Gemini's chat-completions answer carrying `reservations` as its content. */ +function geminiAnswers(reservations: unknown[]) { + safeFetchLlm.mockResolvedValue({ + ok: true, + status: 200, + json: async () => ({ choices: [{ message: { content: JSON.stringify({ reservations }) } }] }), + text: async () => '', + } as unknown as Response); +} + +/** The node Gemini returned in the report: every declared root field, no venue. */ +const bareStay = { + '@type': 'LodgingReservation', + checkinTime: '2026-09-06T13:00:00', + checkoutTime: '2026-09-07T11:00:00', + price: 89.35, + priceCurrency: 'EURials', +}; + +describe('Booking import e2e (#2477): a schema-bound provider on the upload route', () => { + let server: Server; + let app: Awaited>; + let tripId: number; + + async function build() { + const moduleRef = await Test.createTestingModule({ imports: [DatabaseModule, RealtimeModule, ReservationImportModule] }) + .overrideProvider(KitineraryExtractorService) + .useValue({ onModuleInit: () => {}, isAvailable: () => false, extract: vi.fn(), describe: () => ({ available: false }) }) + .overrideProvider(NotificationsService) + .useValue({ send: vi.fn().mockResolvedValue(undefined) }) + .compile(); + const nest = moduleRef.createNestApplication(); + nest.use(cookieParser()); + nest.useGlobalFilters(new TrekExceptionFilter()); + nest.useGlobalPipes(new ZodValidationPipe()); + await nest.init(); + return nest; + } + + const upload = (fileName: string, mode: string) => + request(server) + .post(`/api/trips/${tripId}/reservations/import/booking`) + .set('Cookie', sessionCookie(1)) + .field('mode', mode) + .attach('files', PRINT, { filename: fileName, contentType: 'application/pdf' }); + + const sentSchema = () => { + const body = JSON.parse((safeFetchLlm.mock.calls[0][1] as RequestInit).body as string); + return body.response_format.json_schema.schema.properties.reservations.items; + }; + + beforeAll(async () => { + createTables(db); + runMigrations(db); + db.prepare( + "INSERT INTO users (id, username, email, password_hash, role, password_version) VALUES (1, 'e2e-user', 'e2e@example.test', 'x', 'user', 0)", + ).run(); + tripId = Number(db.prepare("INSERT INTO trips (user_id, title) VALUES (1, 'Albania')").run().lastInsertRowid); + // The instance config the reporter ran: Gemini behind the "openai" provider. + db.prepare( + `INSERT INTO addons (id, name, type, enabled, config) VALUES ('llm_parsing', 'AI Parsing', 'integration', 1, ?) + ON CONFLICT(id) DO UPDATE SET enabled = 1, config = excluded.config`, + ).run(JSON.stringify({ provider: 'openai', model: 'gemini-3.5-flash', baseUrl: 'https://generativelanguage.googleapis.com/v1beta/openai' })); + app = await build(); + vi.spyOn(app.get(PermissionsService), 'checkPermission').mockReturnValue(true); + server = app.getHttpServer(); + }); + + beforeEach(() => { + canAccessTrip.mockImplementation((id: unknown) => db.prepare('SELECT * FROM trips WHERE id = ?').get(id)); + safeFetchLlm.mockReset(); + }); + + afterAll(async () => { + await app.close(); + }); + + it('401 without a cookie', async () => { + const res = await request(server).post(`/api/trips/${tripId}/reservations/import/booking`); + expect(res.status).toBe(401); + expect(safeFetchLlm).not.toHaveBeenCalled(); + }); + + it('sends Gemini a reservationFor with declared venue fields, and requires it', async () => { + geminiAnswers([]); + await upload('booking.pdf', 'force-ai'); + + const item = sentSchema(); + expect(item.required).toEqual(['@type', 'reservationFor']); + expect(item.properties.reservationFor.properties).toMatchObject({ + name: { type: 'string' }, + address: { type: 'string' }, + telephone: { type: 'string' }, + }); + // The PDF went as text, and its euro amount survived the extraction. + const user = JSON.parse((safeFetchLlm.mock.calls[0][1] as RequestInit).body as string).messages[1].content[0].text; + expect(user).toContain('Harbour View Inn'); + expect(user).toContain('€ 89,35'); + }); + + it("the reported answer: two venue-less nodes give one warning, with the file's own name", async () => { + geminiAnswers([bareStay, { ...bareStay }]); + const res = await upload('Bestätigung_1234567890.PDF', 'force-ai'); + + expect(res.status).toBe(201); + expect(res.body.items).toEqual([]); + // One warning, not two: the copy is gone before the mapper sees it. + expect(res.body.warnings).toHaveLength(1); + expect(res.body.warnings[0]).toMatch(/^Incomplete LodgingReservation in Bestätigung_1234567890\.PDF\[0\] \(no reservationFor\)/); + expect(res.body.files).toEqual([{ fileName: 'Bestätigung_1234567890.PDF', aiAvailable: true, aiUsed: true }]); + }); + + it('once Gemini fills the venue: one hotel in EUR, attached to the uploaded file', async () => { + const stay = { ...bareStay, reservationFor: { name: 'Harbour View Inn', address: 'Example Road 1, 1000 Sample Town' } }; + geminiAnswers([stay, { ...stay }]); + const res = await upload('Bestätigung_1.pdf', 'force-ai'); + + expect(res.status).toBe(201); + expect(res.body.warnings).toEqual([]); + expect(res.body.items).toHaveLength(1); + expect(res.body.items[0]).toMatchObject({ + type: 'hotel', + title: 'Harbour View Inn', + needs_review: true, + _accommodation: { check_in: '2026-09-06T13:00', check_out: '2026-09-07T11:00' }, + metadata: { price: 89.35, priceCurrency: 'EUR' }, + source: { fileName: 'Bestätigung_1.pdf', index: 0 }, + }); + }); + + it("the upload dialog's mode reaches the same path, and a currency the model made up comes from the document", async () => { + geminiAnswers([{ ...bareStay, priceCurrency: 'ZZZ', reservationFor: { name: 'Harbour View Inn' } }]); + const res = await upload('Bestätigung_2.pdf', 'fallback-on-empty'); + + expect(res.status).toBe(201); + expect(res.body.items).toHaveLength(1); + expect(res.body.items[0].metadata.priceCurrency).toBe('EUR'); + expect(res.body.items[0].source.fileName).toBe('Bestätigung_2.pdf'); + }); +}); diff --git a/server/tests/unit/nest/booking-import/reservation-import.controller.test.ts b/server/tests/unit/nest/booking-import/reservation-import.controller.test.ts index b1891715a..cf51572ce 100644 --- a/server/tests/unit/nest/booking-import/reservation-import.controller.test.ts +++ b/server/tests/unit/nest/booking-import/reservation-import.controller.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { HttpException } from '@nestjs/common'; +import { HttpException, type Type } from '@nestjs/common'; +import { INTERCEPTORS_METADATA } from '@nestjs/common/constants'; import { ReservationImportController } from '../../../../src/nest/reservation-import/reservation-import.controller'; import type { BookingImportService } from '../../../../src/nest/booking-import/booking-import.service'; import type { User } from '../../../../src/types'; @@ -60,3 +61,22 @@ describe('ReservationImportController.preview', () => { expect(svc.preview).toHaveBeenCalledWith([expect.anything()], 'no-ai', 1); }); }); + +/** + * Multer decodes a multipart filename as latin1 unless told otherwise, and the + * booking import did not tell it: "Bestätigung.pdf" reached the warnings as + * "Bestätigung.pdf", and the review could not attach the file it no longer + * recognised by name (#2477). Read off the interceptor both upload routes carry. + */ +describe('ReservationImportController upload options', () => { + const multerOf = (handler: keyof ReservationImportController) => { + const [Interceptor] = Reflect.getMetadata(INTERCEPTORS_METADATA, ReservationImportController.prototype[handler]) as Type<{ multer: { defParamCharset: string; limits: unknown } }>[]; + return new Interceptor().multer; + }; + + it.each(['preview', 'previewAsync'] as const)('%s decodes non-ASCII filenames as UTF-8', (handler) => { + const multer = multerOf(handler); + expect(multer.defParamCharset).toBe('utf8'); + expect(multer.limits).toEqual({ fileSize: 10 * 1024 * 1024, files: 5 }); + }); +}); diff --git a/server/tests/unit/nest/llm-parse/clients.test.ts b/server/tests/unit/nest/llm-parse/clients.test.ts index 638b86f95..9a151dbeb 100644 --- a/server/tests/unit/nest/llm-parse/clients.test.ts +++ b/server/tests/unit/nest/llm-parse/clients.test.ts @@ -287,6 +287,38 @@ describe('OpenAiCompatibleClient', () => { parts = JSON.parse((fetchFn.mock.calls[1][1] as RequestInit).body as string).messages[1].content; expect(parts.every((p: any) => p.type !== 'file' && p.type !== 'image_url')).toBe(true); }); + + /** + * A report could not say whether the schema ever reached the model or the + * ladder had fallen back to the prompt alone (#2477). The debug line names + * the rung that answered, and nothing of the booking. + */ + it('logs which response_format the answering attempt carried (#2477)', async () => { + const debug = vi.spyOn(console, 'debug').mockImplementation(() => {}); + const ok = () => jsonResponse({ choices: [{ message: { content: '{"reservations":[]}' } }] }); + const refused = () => jsonResponse({ error: 'no' }, false, 400); + const lines = () => debug.mock.calls.map(c => String(c[0])).filter(l => l.includes('response_format=')); + try { + safeFetchLlmMock.mockResolvedValueOnce(ok()); + await new OpenAiCompatibleClient().extract(baseInput); + safeFetchLlmMock.mockResolvedValueOnce(refused()).mockResolvedValueOnce(ok()); + await new OpenAiCompatibleClient().extract(baseInput); + safeFetchLlmMock.mockResolvedValueOnce(refused()).mockResolvedValueOnce(refused()).mockResolvedValueOnce(ok()); + await new OpenAiCompatibleClient().extract(baseInput); + safeFetchLlmMock.mockResolvedValueOnce(jsonResponse({ choices: [{ message: { content: '{}' } }] })); + await new OpenAiCompatibleClient().extract({ ...baseInput, model: 'hf.co/numind/NuExtract-2.0-2B-GGUF:latest' }); + + expect(lines()).toEqual([ + '[DEBUG] LLM answered with response_format=json_schema', + '[DEBUG] LLM answered with response_format=json_object', + '[DEBUG] LLM answered with response_format=none', + '[DEBUG] LLM answered with response_format=none', + ]); + expect(debug.mock.calls.flat().join(' ')).not.toContain('Flight AB123'); + } finally { + debug.mockRestore(); + } + }); }); describe('OpenAiCompatibleClient — NuExtract path', () => { diff --git a/server/tests/unit/nest/llm-parse/currency-code.test.ts b/server/tests/unit/nest/llm-parse/currency-code.test.ts new file mode 100644 index 000000000..3174efaff --- /dev/null +++ b/server/tests/unit/nest/llm-parse/currency-code.test.ts @@ -0,0 +1,52 @@ +import { describe, it, expect } from 'vitest'; +import { toIsoCurrency } from '../../../../src/nest/llm-parse/currency-code'; + +describe('toIsoCurrency', () => { + it('keeps an ISO 4217 code in any case and spacing', () => { + expect(toIsoCurrency('EUR')).toBe('EUR'); + expect(toIsoCurrency('eur')).toBe('EUR'); + expect(toIsoCurrency(' chf ')).toBe('CHF'); + expect(toIsoCurrency('ALL')).toBe('ALL'); + }); + + it('reads a value that starts with a real code as that code (#2477)', () => { + expect(toIsoCurrency('EURials')).toBe('EUR'); + expect(toIsoCurrency('Euros')).toBe('EUR'); + expect(toIsoCurrency('EUR 89,35')).toBe('EUR'); + }); + + it('maps a currency symbol that stands alone', () => { + expect(toIsoCurrency('€')).toBe('EUR'); + expect(toIsoCurrency('£')).toBe('GBP'); + expect(toIsoCurrency('¥')).toBe('JPY'); + expect(toIsoCurrency(' $ ')).toBe('USD'); + }); + + it('answers undefined for anything that is not a currency it can name', () => { + // The lek is ALL; LEK is no code, and its prefix is none either. + expect(toIsoCurrency('LEK')).toBeUndefined(); + expect(toIsoCurrency('XYZ')).toBeUndefined(); + // A dollar with a country in front is not guessed as USD. + expect(toIsoCurrency('CA$')).toBeUndefined(); + expect(toIsoCurrency('US$')).toBeUndefined(); + expect(toIsoCurrency('€€')).toBeUndefined(); + expect(toIsoCurrency('')).toBeUndefined(); + expect(toIsoCurrency(' ')).toBeUndefined(); + }); + + it('answers undefined for a value that is not a string', () => { + expect(toIsoCurrency(null)).toBeUndefined(); + expect(toIsoCurrency(undefined)).toBeUndefined(); + expect(toIsoCurrency(978)).toBeUndefined(); + expect(toIsoCurrency({ code: 'EUR' })).toBeUndefined(); + }); + + it('settles for the shape of a code when the runtime lists no currencies', () => { + const none = new Set(); + expect(toIsoCurrency('eur', none)).toBe('EUR'); + expect(toIsoCurrency('EURials', none)).toBe('EUR'); + expect(toIsoCurrency('XYZ', none)).toBe('XYZ'); + expect(toIsoCurrency('€', none)).toBe('EUR'); + expect(toIsoCurrency('CA$', none)).toBeUndefined(); + }); +}); diff --git a/server/tests/unit/nest/llm-parse/llm-parse.service.test.ts b/server/tests/unit/nest/llm-parse/llm-parse.service.test.ts index c7eefd84b..d5ac611e9 100644 --- a/server/tests/unit/nest/llm-parse/llm-parse.service.test.ts +++ b/server/tests/unit/nest/llm-parse/llm-parse.service.test.ts @@ -20,7 +20,12 @@ const { routeExtraction, detectFlightNumbers } = vi.hoisted(() => ({ routeExtraction: vi.fn(), detectFlightNumbers: vi.fn(() => [] as string[]), })); -vi.mock('../../../../src/nest/llm-parse/router/extraction-router', () => ({ routeExtraction, detectFlightNumbers })); +// The router's pure helpers stay real: the currency fallback reads the document's +// total through extractTotalPrice. +vi.mock('../../../../src/nest/llm-parse/router/extraction-router', async (orig) => { + const actual = await orig() as Record; + return { ...actual, routeExtraction, detectFlightNumbers }; +}); import { LlmParseService } from '../../../../src/nest/llm-parse/llm-parse.service'; import type { LlmConfigResolver } from '../../../../src/nest/llm-parse/llm-config.resolver'; @@ -188,3 +193,94 @@ describe('LlmParseService', () => { expect(res.warnings[0]).toContain('corrupt pdf'); }); }); + +/** + * What a schema-bound provider answered for one Booking.com stay (#2477): the + * same node twice, and a currency the model made up. Synthetic values in the + * reporter's shape. + */ +describe('LlmParseService: cleaning up an AI answer (#2477)', () => { + const stay = (over: Record = {}) => ({ + '@type': 'LodgingReservation', + checkinTime: '2026-09-06T13:00:00', + checkoutTime: '2026-09-07T11:00:00', + price: 89.35, + priceCurrency: 'EURials', + reservationFor: { name: 'Harbour View Inn', address: 'Example Road 1' }, + ...over, + }); + const PRINT = ['Harbour View Inn', 'PREIS', '1 Zimmer € 74,46', 'Preis', '(für 2 Gäste)', '€ 89,35'].join('\n'); + + it('collapses exact duplicate nodes from one AI answer', async () => { + extractText.mockResolvedValue(PRINT); + extract.mockResolvedValue([stay(), stay()]); + const res = await svc().parse(file('Bestätigung_1.pdf', '%PDF'), 1); + expect(res.kiItems).toHaveLength(1); + expect(res.warnings).toEqual([]); + }); + + it("collapses the reporter's two nodes without a venue into one as well", async () => { + extractText.mockResolvedValue(PRINT); + const bare = { '@type': 'LodgingReservation', checkinTime: '2026-09-06T13:00:00', checkoutTime: '2026-09-07T11:00:00', price: 89.35, priceCurrency: 'EURials' }; + extract.mockResolvedValue([bare, { ...bare }]); + const res = await svc().parse(file('Bestätigung_1.pdf', '%PDF'), 1); + expect(res.kiItems).toEqual([{ ...bare, priceCurrency: 'EUR' }]); + }); + + it('treats the same fields in another key order as the same node', async () => { + const a = stay(); + const b = { reservationFor: { address: 'Example Road 1', name: 'Harbour View Inn' }, priceCurrency: 'EURials', price: 89.35, checkoutTime: a.checkoutTime, checkinTime: a.checkinTime, '@type': a['@type'] }; + extract.mockResolvedValue([a, b]); + const res = await svc().parse(file('a.txt'), 1); + expect(res.kiItems).toHaveLength(1); + }); + + it('keeps distinct nodes that only look alike', async () => { + extract.mockResolvedValue([stay(), stay({ checkinTime: '2026-09-07T13:00:00', checkoutTime: '2026-09-08T11:00:00' })]); + const res = await svc().parse(file('a.txt'), 1); + expect(res.kiItems).toHaveLength(2); + }); + + it('turns a garbled priceCurrency into the code it starts with', async () => { + extract.mockResolvedValue([stay()]); + const res = await svc().parse(file('a.txt'), 1); + expect(res.kiItems[0].priceCurrency).toBe('EUR'); + }); + + it("falls back to the document's currency symbol when the model named no currency", async () => { + extractText.mockResolvedValue(PRINT); + extract.mockResolvedValue([stay({ priceCurrency: 'ZZZ' }), stay({ priceCurrency: undefined, checkinTime: '2026-10-01T15:00:00' })]); + const res = await svc().parse(file('b.pdf', '%PDF'), 1); + expect(res.kiItems.map((n) => n.priceCurrency)).toEqual(['EUR', 'EUR']); + }); + + it('drops the currency when neither the model nor the document names one', async () => { + extractText.mockResolvedValue('Harbour View Inn, 89.35 total'); + extract.mockResolvedValue([stay({ priceCurrency: 'ZZZ' })]); + const res = await svc().parse(file('b.txt'), 1); + expect(res.kiItems[0]).not.toHaveProperty('priceCurrency'); + expect(res.kiItems[0].price).toBe(89.35); + }); + + it('asks the document only when the node has a price', async () => { + extractText.mockResolvedValue(PRINT); + extract.mockResolvedValue([stay({ price: undefined, priceCurrency: 'Pesos' })]); + const res = await svc().parse(file('b.pdf', '%PDF'), 1); + expect(res.kiItems[0]).not.toHaveProperty('priceCurrency'); + }); + + it('has no document to ask on the native PDF path', async () => { + resolveLlmConfig.mockReturnValue(cfg({ provider: 'anthropic' })); + extract.mockResolvedValue([stay({ priceCurrency: 'ZZZ' })]); + const res = await svc().parse(file('b.pdf', '%PDF'), 1); + expect(extractText).not.toHaveBeenCalled(); + expect(res.kiItems[0]).not.toHaveProperty('priceCurrency'); + }); + + it('leaves a node without price and currency as it came', async () => { + const plain = { '@type': 'FlightReservation', reservationFor: { flightNumber: 'X1' } }; + extract.mockResolvedValue([plain]); + const res = await svc().parse(file('a.txt'), 1); + expect(res.kiItems).toEqual([plain]); + }); +}); diff --git a/server/tests/unit/nest/llm-parse/llm-prompt.test.ts b/server/tests/unit/nest/llm-parse/llm-prompt.test.ts index 2fa178dfb..5984ff79f 100644 --- a/server/tests/unit/nest/llm-parse/llm-prompt.test.ts +++ b/server/tests/unit/nest/llm-parse/llm-prompt.test.ts @@ -2,6 +2,18 @@ import { describe, it, expect } from 'vitest'; import { buildSystemPrompt, KI_RESERVATION_JSON_SCHEMA } from '../../../../src/nest/llm-parse/llm-prompt'; import { KI_RESERVATION_TYPES } from '@trek/shared'; +/** The slice of JSON Schema these assertions read. */ +interface SchemaNode { + type?: string | readonly string[]; + properties?: Record; + items?: SchemaNode; + required?: readonly string[]; + additionalProperties?: boolean; +} + +const root = KI_RESERVATION_JSON_SCHEMA as SchemaNode; +const item = root.properties.reservations.items; + describe('llm-prompt', () => { it('names every recognized @type the mapper supports', () => { const prompt = buildSystemPrompt(); @@ -23,4 +35,54 @@ describe('llm-prompt', () => { expect(item.properties['@type'].enum).toEqual([...KI_RESERVATION_TYPES]); expect(item.required).toContain('@type'); }); + + /** + * Gemini decodes against the schema and fills declared properties only. An + * open `reservationFor` without any came back missing, and every hotel lost + * its name (#1638, #2477). + */ + it('declares the venue fields on reservationFor so a schema-constrained provider can fill them (#2477)', () => { + const rf = item.properties.reservationFor; + + expect(item.required).toEqual(['@type', 'reservationFor']); + expect(rf.type).toBe('object'); + expect(rf.additionalProperties).toBe(true); + for (const field of ['name', 'address', 'telephone', 'url']) expect(rf.properties[field]).toEqual({ type: 'string' }); + expect(rf.properties.geo.properties).toEqual({ latitude: { type: 'number' }, longitude: { type: 'number' } }); + }); + + it('declares every nested object the prompt names, per type (#2477)', () => { + const rf = item.properties.reservationFor.properties; + const keys = (node: SchemaNode) => Object.keys(node.properties ?? {}); + + expect(keys(rf.departureAirport)).toEqual(['iataCode', 'name', 'geo']); + expect(keys(rf.airline)).toEqual(['name', 'iataCode']); + for (const stop of ['departureStation', 'arrivalBusStop', 'arrivalBoatTerminal']) { + expect(keys(rf[stop])).toEqual(['name', 'geo']); + } + expect(keys(rf.location)).toEqual(['name', 'address', 'geo', 'telephone', 'url']); + expect(keys(rf.rentalCompany)).toEqual(['name']); + // The car desks sit at the root, as the prompt says, with the same venue fields. + expect(item.properties.pickupLocation).toEqual(rf.location); + expect(item.properties.dropoffLocation).toEqual(rf.location); + }); + + it('nests no object deeper than two levels below reservationFor (#2477)', () => { + const depth = (node: SchemaNode): number => + node.type === 'object' && node.properties + ? 1 + Math.max(0, ...Object.values(node.properties).map(depth)) + : 0; + // reservationFor itself, then e.g. departureAirport, then its geo. + expect(depth(item.properties.reservationFor)).toBe(3); + }); + + it('every object in the schema but the root stays open to extra fields', () => { + const walk = (node: SchemaNode | undefined, path: string): string[] => { + if (!node) return []; + const own = node.type === 'object' && path !== '' && node.additionalProperties !== true ? [path] : []; + const children = Object.entries(node.properties ?? {}).flatMap(([k, v]) => walk(v, `${path}.${k}`)); + return [...own, ...children, ...walk(node.items, `${path}[]`)]; + }; + expect(walk(root, '')).toEqual([]); + }); }); diff --git a/server/tests/unit/services/kitineraryMapper.test.ts b/server/tests/unit/services/kitineraryMapper.test.ts index 580735fe3..b90516abc 100644 --- a/server/tests/unit/services/kitineraryMapper.test.ts +++ b/server/tests/unit/services/kitineraryMapper.test.ts @@ -166,3 +166,39 @@ describe('kitinerary mapper — a recognised type that cannot be mapped (#2375)' expect(warnings).toEqual(['Unknown type "RocketLaunchReservation" in mars.eml[0] — skipped']); }); }); + +/** + * The node a schema-bound provider writes once `reservationFor` has declared + * fields: the venue inside it, times and price at the root, as the prompt asks. + * Synthetic data in the shape of a Booking.com print (#2477). + */ +describe('kitinerary mapper: a prompt-shaped AI lodging (#2477)', () => { + it('maps a LodgingReservation with reservationFor.name to one hotel with its stay and price', () => { + const { items, warnings } = mapReservations([ + { + '@type': 'LodgingReservation', + checkinTime: '2026-09-06T13:00:00', + checkoutTime: '2026-09-07T11:00:00', + price: 89.35, + priceCurrency: 'EUR', + reservationFor: { + name: 'Harbour View Inn', + address: 'Example Road 1, 1000 Sample Town', + telephone: '+00 000 000 000', + }, + }, + ] as Parameters[0], 'Bestätigung_1.pdf'); + + expect(warnings).toEqual([]); + expect(items).toHaveLength(1); + expect(items[0]).toMatchObject({ + type: 'hotel', + title: 'Harbour View Inn', + location: 'Example Road 1, 1000 Sample Town', + _venue: { name: 'Harbour View Inn', phone: '+00 000 000 000' }, + _accommodation: { check_in: '2026-09-06T13:00', check_out: '2026-09-07T11:00' }, + metadata: { check_in_time: '13:00', check_out_time: '11:00', price: 89.35, priceCurrency: 'EUR' }, + source: { fileName: 'Bestätigung_1.pdf', index: 0 }, + }); + }); +}); diff --git a/shared/src/reservation/ki-reservation.schema.ts b/shared/src/reservation/ki-reservation.schema.ts index da0ec5ceb..35cd6754e 100644 --- a/shared/src/reservation/ki-reservation.schema.ts +++ b/shared/src/reservation/ki-reservation.schema.ts @@ -11,9 +11,9 @@ import { z } from 'zod'; * missing fields, so validation only guarantees each node is an object with a * string `@type`; everything else passes through untouched. * - `KI_RESERVATION_JSON_SCHEMA` — the JSON Schema handed to the LLM providers - * (OpenAI-compatible `response_format`, Anthropic tool `input_schema`). It is - * descriptive guidance, kept deliberately permissive on `reservationFor` so the - * model can include type-specific fields; the prompt names the exact fields. + * (OpenAI-compatible `response_format`, Anthropic tool `input_schema`). It + * declares every field the prompt names and still lets anything else through, + * so a model is steered but never refused for adding a field of its own. */ /** The `@type` values `mapReservations()` recognises (its switch + flight grouping). */ @@ -39,11 +39,97 @@ export const kiReservationArraySchema = z.object({ }); export type KiReservationArrayDto = z.infer; +/** Coordinates as schema.org `GeoCoordinates`. */ +const GEO_JSON_SCHEMA = { + type: 'object', + additionalProperties: true, + properties: { latitude: { type: 'number' }, longitude: { type: 'number' } }, +} as const; + +/** A station, a bus stop or a boat terminal. */ +const STOP_JSON_SCHEMA = { + type: 'object', + additionalProperties: true, + properties: { name: { type: 'string' }, geo: GEO_JSON_SCHEMA }, +} as const; + +const AIRPORT_JSON_SCHEMA = { + type: 'object', + additionalProperties: true, + properties: { iataCode: { type: 'string' }, name: { type: 'string' }, geo: GEO_JSON_SCHEMA }, +} as const; + +/** A place somebody goes to: an event location, a car pickup or return desk. */ +const VENUE_JSON_SCHEMA = { + type: 'object', + additionalProperties: true, + properties: { + name: { type: 'string' }, + address: { type: 'string' }, + geo: GEO_JSON_SCHEMA, + telephone: { type: 'string' }, + url: { type: 'string' }, + }, +} as const; + +/** + * The type-specific payload, one union of what the prompt asks for per `@type`. + * + * It used to be an open object with no properties at all, and a provider that + * decodes against the schema (Gemini through its OpenAI-compatible endpoint) + * fills declared properties only. It answered with every root field it could + * see and no `reservationFor`, so a hotel arrived without its name and the + * mapper had to drop it (#1638, #2477). Each field is optional, since a flight + * has no venue name and a hotel has no flight number, and nothing is nested + * deeper than two objects below this one. + */ +const RESERVATION_FOR_JSON_SCHEMA = { + type: 'object', + additionalProperties: true, + properties: { + // The venue itself: lodging, restaurant, event, boat, rental car. + name: { type: 'string' }, + address: { type: 'string' }, + telephone: { type: 'string' }, + url: { type: 'string' }, + geo: GEO_JSON_SCHEMA, + // Flight. + flightNumber: { type: 'string' }, + airline: { + type: 'object', + additionalProperties: true, + properties: { name: { type: 'string' }, iataCode: { type: 'string' } }, + }, + departureAirport: AIRPORT_JSON_SCHEMA, + arrivalAirport: AIRPORT_JSON_SCHEMA, + departureTime: { type: 'string' }, + arrivalTime: { type: 'string' }, + // Train, bus and boat. + trainNumber: { type: 'string' }, + trainName: { type: 'string' }, + departureStation: STOP_JSON_SCHEMA, + arrivalStation: STOP_JSON_SCHEMA, + busNumber: { type: 'string' }, + busName: { type: 'string' }, + departureBusStop: STOP_JSON_SCHEMA, + arrivalBusStop: STOP_JSON_SCHEMA, + departureBoatTerminal: STOP_JSON_SCHEMA, + arrivalBoatTerminal: STOP_JSON_SCHEMA, + // Event and rental car. + startDate: { type: 'string' }, + endDate: { type: 'string' }, + location: VENUE_JSON_SCHEMA, + model: { type: 'string' }, + make: { type: 'string' }, + rentalCompany: { type: 'object', additionalProperties: true, properties: { name: { type: 'string' } } }, + }, +} as const; + /** * JSON Schema for the providers' structured-output entry points. Object root * (both OpenAI and Anthropic want an object, not a bare array). `reservationFor` - * is intentionally open (`additionalProperties: true`) so the model can fill the - * type-specific sub-object; the prompt enumerates the field names per `@type`. + * is required and spells out its fields (see above); `additionalProperties` + * stays open on every object, so a field the prompt does not name still passes. */ export const KI_RESERVATION_JSON_SCHEMA = { type: 'object', @@ -65,17 +151,16 @@ export const KI_RESERVATION_JSON_SCHEMA = { dropoffTime: { type: 'string' }, startTime: { type: 'string' }, endTime: { type: 'string' }, - // Type-specific payload (Flight/Train/Lodging/…): open object. - reservationFor: { type: 'object', additionalProperties: true }, - pickupLocation: { type: 'object', additionalProperties: true }, - dropoffLocation: { type: 'object', additionalProperties: true }, + reservationFor: RESERVATION_FOR_JSON_SCHEMA, + pickupLocation: VENUE_JSON_SCHEMA, + dropoffLocation: VENUE_JSON_SCHEMA, seat: { type: 'string' }, class: { type: 'string' }, platform: { type: 'string' }, price: { type: ['number', 'string'] }, priceCurrency: { type: 'string' }, }, - required: ['@type'], + required: ['@type', 'reservationFor'], }, }, },