fix(booking-import): stop writing an unreadable type down as a hotel (#2076)

When neither the keyword table nor the model produced a type, the router wrote
"hotel" and cast whatever string it got to FlatType without checking it. A hotel
is a booking type, so a ferry voucher imported from the Transport tab opened the
booking form, which offers six chips and not one transport among them, and
"other" was the only honest pick left. An unrecognised string got through the
cast too and then vanished silently in nuExtractToKiReservations.

The type is validated against FLAT_TYPES now. When it cannot be read the item is
still extracted with the lodging shape so it survives mapping, but it says so,
and the tie-breaker #2080 added finally has something to fire on: isUnplaceableItem
only ever matched a type outside both form's vocabularies, and the server can
only emit its own eight, so on a real document it never matched at all.

The tab travels with the job instead of sitting in component state. The parse
deliberately outlives navigation and reload and the review is triggered by the
global widget on any page, so by the time it ran the state had remounted back to
its default and the context was gone.
This commit is contained in:
Maurice
2026-08-28 13:23:23 +02:00
committed by Maurice
parent 016fdeec64
commit 130272cfb0
12 changed files with 175 additions and 28 deletions
@@ -10,6 +10,9 @@ interface BookingImportModalProps {
isOpen: boolean
onClose: () => void
tripId: number
/** The tab this import was started from, carried into the job so the review
* can pick the right form for an item whose type had to be guessed (#2076). */
kind?: 'transports' | 'bookings'
}
const ACCEPTED_EXTS = ['.eml', '.pdf', '.pkpass', '.html', '.htm', '.txt']
@@ -22,7 +25,7 @@ const MAX_FILES = 5
* (progress over the WebSocket). When it finishes, the trip page opens the per-item
* review flow — so the user can navigate and keep editing while it works.
*/
export default function BookingImportModal({ isOpen, onClose, tripId }: BookingImportModalProps) {
export default function BookingImportModal({ isOpen, onClose, tripId, kind }: BookingImportModalProps) {
const { t } = useTranslation()
const addTask = useBackgroundTasksStore((s) => s.addTask)
const fileInputRef = useRef<HTMLInputElement>(null)
@@ -100,7 +103,7 @@ export default function BookingImportModal({ isOpen, onClose, tripId }: BookingI
// Keep the uploaded files so the review can attach each source document to its booking —
// in memory for the immediate path, and in IndexedDB so it survives a reload mid-parse.
await saveImportFiles(jobId, files)
addTask({ id: jobId, tripId: String(tripId), label: files.map((f) => f.name).join(', '), total: files.length, files, mode })
addTask({ id: jobId, tripId: String(tripId), label: files.map((f) => f.name).join(', '), total: files.length, files, mode, kind })
handleClose()
} catch (err: any) {
setError(err?.response?.data?.error ?? t('reservations.import.error'))
@@ -52,9 +52,16 @@ export function isTransportItem(item: BookingImportPreviewItem): boolean {
}
/**
* Neither form can express this type, so neither is obviously right. The tab the
* user started the import from breaks the tie (#2076).
* Neither form is obviously right, so the tab the user started the import from
* breaks the tie (#2076).
*
* Two cases. A type neither form can express, and a type the extractor filled in
* rather than read. The second is the one that actually occurs: the server can
* only ever emit its eight known types, so the first branch never fires on a real
* document, and a ferry voucher the model could not classify arrived as a
* placeholder 'hotel' with no way to tell it from a real one.
*/
export function isUnplaceableItem(item: BookingImportPreviewItem): boolean {
if (item.type_guessed === true) return true
return !TRANSPORT_TYPES.has(item.type) && !BOOKING_TYPES.has(item.type)
}
@@ -126,7 +126,7 @@ export default function MTripSheets({ planner, shell }: MTripSheetsProps) {
/>
)}
<BookingImportModal isOpen={planner.showBookingImport} onClose={() => planner.setShowBookingImport(false)} tripId={tripId} />
<BookingImportModal isOpen={planner.showBookingImport} onClose={() => planner.setShowBookingImport(false)} tripId={tripId} kind={planner.bookingImportKind} />
<AirTrailImportModal isOpen={planner.showAirTrailImport} onClose={() => planner.setShowAirTrailImport(false)} tripId={tripId} pushUndo={planner.pushUndo} />
{/* Trip edit + share/members, opened from the Mehr sheet. */}
+2 -2
View File
@@ -252,7 +252,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
prefillCoords, setPrefillCoords, editingAssignmentId, setEditingAssignmentId,
showTripForm, setShowTripForm, showMembersModal, setShowMembersModal,
showReservationModal, setShowReservationModal, editingReservation, setEditingReservation,
showBookingImport, setShowBookingImport, setBookingImportKind, bookingImportAvailable,
showBookingImport, setShowBookingImport, bookingImportKind, setBookingImportKind, bookingImportAvailable,
airTrailAvailable, showAirTrailImport, setShowAirTrailImport,
bookingForAssignmentId, setBookingForAssignmentId,
showTransportModal, setShowTransportModal, editingTransport, setEditingTransport,
@@ -877,7 +877,7 @@ function TripPlannerPageDesktop(): React.ReactElement | null {
</Suspense>
</ErrorBoundary>
)}
<BookingImportModal isOpen={showBookingImport} onClose={() => setShowBookingImport(false)} tripId={tripId} />
<BookingImportModal isOpen={showBookingImport} onClose={() => setShowBookingImport(false)} tripId={tripId} kind={bookingImportKind} />
<AirTrailImportModal isOpen={showAirTrailImport} onClose={() => setShowAirTrailImport(false)} tripId={tripId} pushUndo={pushUndo} />
<ConfirmDialog
isOpen={!!deletePlaceId}
@@ -1642,8 +1642,10 @@ describe('useTripPlanner — booking import review', () => {
const { result } = await renderPlanner()
const odd = { type: 'shuttle-voucher', title: 'Airport transfer' }
act(() => { result.current.setBookingImportKind('transports') })
act(() => { result.current.startImportReview([odd] as never) })
// The tab now travels with the review rather than living in component state:
// the parse outlives navigation and reload, and the review is triggered by the
// global widget, so by then the state has remounted back to its default.
act(() => { result.current.startImportReview([odd] as never, [], 'transports') })
expect(result.current.showTransportModal).toBe(true)
expect(result.current.showReservationModal).toBe(false)
@@ -1654,8 +1656,33 @@ describe('useTripPlanner — booking import review', () => {
const { result } = await renderPlanner()
const odd = { type: 'shuttle-voucher', title: 'Airport transfer' }
act(() => { result.current.setBookingImportKind('bookings') })
act(() => { result.current.startImportReview([odd] as never) })
act(() => { result.current.startImportReview([odd] as never, [], 'bookings') })
expect(result.current.showReservationModal).toBe(true)
expect(result.current.showTransportModal).toBe(false)
})
// The case that actually occurs. The server only ever emits its eight known
// types, so 'shuttle-voucher' above never reaches a real import: a document the
// model could not classify arrived as a placeholder 'hotel', which IS a booking
// type, so the tie-breaker never ran and the tab lost every time (#2076).
it('FE-TP-HOOK-112b: a guessed hotel opens the transport form when the import began there', async () => {
seedTrip()
const { result } = await renderPlanner()
const guessed = { type: 'hotel', title: 'Airport transfer', type_guessed: true }
act(() => { result.current.startImportReview([guessed] as never, [], 'transports') })
expect(result.current.showTransportModal).toBe(true)
expect(result.current.showReservationModal).toBe(false)
})
it('FE-TP-HOOK-112c: a real hotel from the same tab still opens the booking form', async () => {
seedTrip()
const { result } = await renderPlanner()
const real = { type: 'hotel', title: 'Ryokan' }
act(() => { result.current.startImportReview([real] as never, [], 'transports') })
expect(result.current.showReservationModal).toBe(true)
expect(result.current.showTransportModal).toBe(false)
+14 -3
View File
@@ -274,6 +274,11 @@ export function useTripPlanner() {
const importQueueRef = useRef<BookingImportPreviewItem[]>([])
// The files this import was parsed from, so each reviewed booking can attach its source doc.
const importSourceFilesRef = useRef<File[]>([])
// The tab the items under review came from. A ref, not the bookingImportKind
// state: the parse outlives navigation and reload, and the review is triggered
// by the global widget, so by then the state has remounted back to its default.
// The value comes off the persisted job (#2076).
const importKindRef = useRef<'transports' | 'bookings'>('bookings')
// Manual route planning: off by default, toggled from the day-plan footer. Mode
// is per-session and selects which travel time the connectors show — either a
// built-in OSRM profile or a plugin route profile ('plugin:<id>/<profile>').
@@ -933,7 +938,7 @@ export function useTripPlanner() {
const srcName = item.source?.fileName
const srcFile = srcName ? importSourceFilesRef.current.find(f => f.name === srcName) : undefined
if (srcFile) draft._sourceFiles = [srcFile]
if (isTransportItem(item) || (isUnplaceableItem(item) && bookingImportKind === 'transports')) {
if (isTransportItem(item) || (isUnplaceableItem(item) && importKindRef.current === 'transports')) {
setShowReservationModal(false); setEditingReservation(null); setReservationPrefill(null)
setEditingTransport(null); setTransportModalDayId(null)
setTransportPrefill(draft); setShowTransportModal(true)
@@ -944,9 +949,14 @@ export function useTripPlanner() {
}
}
const startImportReview = (items: BookingImportPreviewItem[], sourceFiles: File[] = []) => {
const startImportReview = (
items: BookingImportPreviewItem[],
sourceFiles: File[] = [],
kind: 'transports' | 'bookings' = 'bookings',
) => {
if (!items.length) return
importSourceFilesRef.current = sourceFiles
importKindRef.current = kind
importQueueRef.current = items.slice(1)
setImportReviewActive(true)
openImportItem(items[0])
@@ -967,12 +977,13 @@ export function useTripPlanner() {
const items = task.items
const jobId = task.id
const inMemory = task.sourceFiles
const kind = task.kind ?? 'bookings'
dismissBgTask(jobId)
// Prefer the in-memory files (immediate path); after a reload they live in IndexedDB.
void (async () => {
const files = inMemory && inMemory.length ? inMemory : await getImportFiles(jobId)
deleteImportFiles(jobId)
startImportReview(items, files)
startImportReview(items, files, kind)
})()
}
}, [bgTasks, tripId, startImportReview, dismissBgTask])
+13 -3
View File
@@ -32,11 +32,19 @@ export interface BackgroundImportTask {
sourceFiles?: File[]
/** Mode this run used — the widget only offers an AI retry if it wasn't already one. Not persisted. */
mode?: BookingImportMode
/**
* Which tab the import was started from. It decides which form an item opens
* when its type is guessed (#2076), and unlike the other transient flags it IS
* persisted: the parse deliberately outlives navigation and reload, and the
* review is triggered by the global widget on any page, so a component-local
* value is gone by the time it is needed.
*/
kind?: 'transports' | 'bookings'
}
interface BackgroundTasksState {
tasks: BackgroundImportTask[]
addTask: (task: { id: string; tripId: string; label: string; total: number; files?: File[]; mode?: BookingImportMode }) => void
addTask: (task: { id: string; tripId: string; label: string; total: number; files?: File[]; mode?: BookingImportMode; kind?: 'transports' | 'bookings' }) => void
setProgress: (id: string, tripId: string, done: number, total: number) => void
setDone: (id: string, tripId: string, items: BookingImportPreviewItem[], warnings: string[]) => void
setError: (id: string, tripId: string, error: string) => void
@@ -63,7 +71,7 @@ export const useBackgroundTasksStore = create<BackgroundTasksState>()(
return {
tasks: [],
addTask: ({ id, tripId, label, total, files, mode }) => upsert(id, tripId, { label, total, status: 'running', done: 0, sourceFiles: files, mode }),
addTask: ({ id, tripId, label, total, files, mode, kind }) => upsert(id, tripId, { label, total, status: 'running', done: 0, sourceFiles: files, mode, kind }),
setProgress: (id, tripId, done, total) => upsert(id, tripId, { done, total, status: 'running' }),
setDone: (id, tripId, items, warnings) => upsert(id, tripId, { status: 'done', items, warnings, done: items?.length ?? 0 }),
setError: (id, tripId, error) => upsert(id, tripId, { status: 'error', error }),
@@ -81,7 +89,9 @@ export const useBackgroundTasksStore = create<BackgroundTasksState>()(
partialize: (state) => ({
tasks: state.tasks
.filter((t) => !t.consumed && t.status !== 'error')
.map((t) => ({ id: t.id, tripId: t.tripId, label: t.label, status: t.status, done: t.done, total: t.total })),
// `kind` rides along: it is not a review flag but the context the review
// needs, and without it a reload silently falls back to 'bookings'.
.map((t) => ({ id: t.id, tripId: t.tripId, label: t.label, status: t.status, done: t.done, total: t.total, kind: t.kind })),
}),
},
),
@@ -127,6 +127,11 @@ export interface KiEvent {
/** A single output node from kitinerary-extractor's JSON array */
export interface KiReservation {
'@type': string;
/**
* TREK's own marker, not schema.org: the extractor could not read a type and
* fell back to the lodging shape so the item would survive mapping (#2076).
*/
trekTypeGuessed?: boolean;
reservationNumber?: string;
checkinTime?: KiDateTimeish;
checkoutTime?: KiDateTimeish;
@@ -182,6 +187,8 @@ export interface ParsedAccommodation {
*/
export interface ParsedBookingItem {
type: string;
/** The type was filled in, not read from the document (#2076). */
type_guessed?: boolean;
title: string;
reservation_time?: string | null;
reservation_end_time?: string | null;
@@ -170,6 +170,9 @@ function buildNode(x: Record<string, unknown>): Record<string, unknown> | undefi
const node: Record<string, unknown> = {
'@type': atType,
// Not a schema.org field: it says the router had to guess the type, so the
// importer can offer the form the user was importing into (#2076).
...(x.type_guessed === true ? { trekTypeGuessed: true } : {}),
reservationNumber: x.booking_reference,
seat: x.seat,
class: x.travel_class,
@@ -18,9 +18,10 @@
* existing `nuExtractToKiReservations` mapper, so nothing downstream changes.
*/
import { normalizeLocalDateTime } from '@trek/shared';
import type { KiReservation } from '../../booking-import/kitinerary.types';
import { nuExtractToKiReservations } from '../clients/nuextract';
import { FLAT_SCHEMA_BY_TYPE, FLIGHTS_ARRAY_SCHEMA, UNION_SINGLE_SCHEMA, type FlatType, type FlatLike } from './flat-schemas';
import { FLAT_SCHEMA_BY_TYPE, FLAT_TYPES, FLIGHTS_ARRAY_SCHEMA, UNION_SINGLE_SCHEMA, type FlatType, type FlatLike } from './flat-schemas';
import { extractEnforced } from './ollama-format.client';
export interface RouterContext {
@@ -38,9 +39,9 @@ const TYPE_HINT: Record<FlatType, string> = {
train: 'train. from_name/to_name = stations, vehicle_number = train number, times = full ISO, price/currency = total fare.',
bus: 'bus. from_name/to_name = stops, times = full ISO, price/currency = total fare.',
ferry: 'ferry/cruise. from_name/to_name = terminals/ports, times = full ISO, price/currency = total fare.',
car: 'rental car. operator = the rental company, from_name = pick-up location, to_name = return location (may differ), departure_time = pick-up, arrival_time = return, price/currency = total rental cost.',
car: 'rental car. operator = the rental company, from_name = pick-up location, to_name = return location (may differ), departure_time = pick-up, arrival_time = return, times = full ISO (24-hour), price/currency = total rental cost.',
hotel: 'hotel stay. name = hotel name, address = the hotel street address, checkin_time/checkout_time = full ISO date-time, price/currency = total paid.',
restaurant: 'restaurant booking. name = the restaurant, address = its street address, start_time = the reservation date-time, price/currency = total if shown.',
restaurant: 'restaurant booking. name = the restaurant, address = its street address, start_time = the reservation date-time as full ISO (24-hour), price/currency = total if shown.',
event: 'event/attraction. name = the event/ticket, address = the venue, start_time/end_time = full ISO, price/currency = ticket price.',
};
@@ -150,8 +151,14 @@ export function extractTotalPrice(text: string): { price: string; currency?: str
*/
export function fixArrivalDate(flat: FlatLike): FlatLike {
if (!TRANSPORT_TYPES.includes(flat.type)) return flat;
const dep = /(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2})/.exec(String(flat.departure_time ?? ''));
const arr = /(\d{2}:\d{2})/.exec(String(flat.arrival_time ?? ''));
// Both sides go through the 12-hour normalizer first. Reading the arrival
// with a bare /(\d{2}:\d{2})/ used to throw a trailing "PM" away, and the
// comparison below is lexicographic: "04:00" < "12:40" rolled a 13:11
// arrival onto the next day and turned a short hop into an overnight (#2094).
const depSource = normalizeLocalDateTime(String(flat.departure_time ?? ''));
const arrSource = normalizeLocalDateTime(String(flat.arrival_time ?? ''));
const dep = /(\d{4}-\d{2}-\d{2})T(\d{2}:\d{2})/.exec(depSource);
const arr = /(\d{2}:\d{2})/.exec(arrSource.slice(10));
if (!dep || !arr) return flat;
const [, depDate, depTime] = dep;
const arrTime = arr[1];
@@ -167,16 +174,25 @@ const DATE_FIELDS = ['departure_time', 'arrival_time', 'checkin_time', 'checkout
* Coerce a date value to ISO 8601. Models occasionally ignore the format instruction and
* emit a natural-language date ("Aug 23 2025 13:30"), which the downstream `splitIso` then
* slices into garbage ("Aug 23 202"). Keep already-ISO values untouched; otherwise parse and
* reformat. (The server runs in UTC, so the components line up.)
* reformat.
*
* These fields are naive local wall-clock strings everywhere else in TREK, so the components
* are read back locally. Reading with Date.parse and printing with getUTC* used to shift every
* such value by the container's UTC offset, which docker-compose sets from TZ and documents
* with Europe/Berlin. That is why a rental car came back off by the offset rather than by
* twelve hours: only `car` and `restaurant` lack the full-ISO instruction in TYPE_HINT, so
* only they reach this parse at all (#2094).
*/
function toIso(value: unknown): unknown {
if (typeof value !== 'string' || !value.trim()) return value;
if (/^\d{4}-\d{2}-\d{2}/.test(value)) return value;
// An ISO date with a printed meridiem behind it ("2026-06-11T02:30 PM") is not
// already ISO, it only looks like it. Resolve the clock, keep everything else.
if (/^\d{4}-\d{2}-\d{2}/.test(value)) return normalizeLocalDateTime(value);
const t = Date.parse(value);
if (Number.isNaN(t)) return value;
const d = new Date(t);
const p = (n: number) => String(n).padStart(2, '0');
return `${d.getUTCFullYear()}-${p(d.getUTCMonth() + 1)}-${p(d.getUTCDate())}T${p(d.getUTCHours())}:${p(d.getUTCMinutes())}:00`;
return `${d.getFullYear()}-${p(d.getMonth() + 1)}-${p(d.getDate())}T${p(d.getHours())}:${p(d.getMinutes())}:00`;
}
/** Normalize every date-ish field on a flat reservation to ISO before mapping. */
@@ -213,8 +229,21 @@ async function extractSingle(text: string, ctx: RouterContext): Promise<FlatLike
return fixArrivalDate(normalizeDates({ ...out, type: known }));
}
const out = (await call(UNION_SINGLE_SCHEMA, 'Pick the correct "type".')) ?? {};
const type = (typeof out.type === 'string' ? out.type : 'hotel') as FlatType;
return fixArrivalDate(normalizeDates({ ...out, type }));
// "I could not tell" used to be written down as "hotel", and a hotel is a
// booking, so a ferry voucher imported from the Transport tab opened the
// booking form with no transport type to pick and stayed 'other' (#2076).
// An unvalidated cast also let any string through, and the item then vanished
// silently in nuExtractToKiReservations, which drops an unmapped type.
const raw = typeof out.type === 'string' ? out.type.toLowerCase().trim() : '';
const picked = (FLAT_TYPES as string[]).includes(raw) ? (raw as FlatType) : null;
// Still extract with the hotel shape so the item survives the mapping, but say
// out loud that the type was guessed. Only here: a valid pick by the model is
// a real answer, and flagging it would push AI-parsed hotels into the
// transport form whenever the document never says the word "hotel".
const flat: FlatLike = picked
? { ...out, type: picked }
: { ...out, type: 'hotel' as FlatType, type_guessed: true };
return fixArrivalDate(normalizeDates(flat));
}
/**
@@ -23,6 +23,13 @@ export const FLAT_TYPES: FlatType[] = ['flight', 'train', 'bus', 'ferry', 'car',
* are the ones the router reads directly; the index signature carries the rest unchanged. */
export interface FlatLike {
type: FlatType;
/**
* The type was not read from the document, it was filled in because neither the
* keyword table nor the model produced a valid one. The extractor still uses the
* hotel shape so the item survives mapping, but the flag rides along so the
* importer opens the form the user was actually importing into (#2076).
*/
type_guessed?: boolean;
booking_reference?: string;
vehicle_number?: string;
from_code?: string;
@@ -114,7 +114,10 @@ describe('routeExtraction', () => {
expect(res.kiItems).toEqual([{ '@type': 'Mock' }]);
const flats = mapToKi.mock.calls[0][0];
expect(flats).toHaveLength(2);
expect(flats[0].departure_time).toMatch(/^2025-08-23T\d{2}:\d{2}:00$/); // natural-language → ISO
// Exact now, not a pattern: toIso reads and prints local components, so a naive
// string is the same wall clock on every machine. It used to parse locally and
// print in UTC, which shifted the value by the container's offset (#2094).
expect(flats[0].departure_time).toBe('2025-08-23T10:00:00');
expect(flats[1].arrival_time).toBe('2025-08-31T07:00:00'); // overnight roll (TZ-safe: derived from the ISO departure date)
});
@@ -214,3 +217,43 @@ describe('routeExtraction', () => {
});
});
describe('printed 12-hour clocks and unreadable types (#2094, #2076)', () => {
it('fixArrivalDate resolves both meridiems before deciding the day rolled over', () => {
// '01:11 pm' used to be read as '01:11', which sorts before the 09:51
// departure, so a three-hour hop was rolled onto the next day.
const out = fixArrivalDate({ type: 'flight', departure_time: '2026-06-11T09:51 am', arrival_time: '2026-06-11T01:11 pm' });
expect(out.arrival_time).toBe('2026-06-11T13:11:00');
});
it('fixArrivalDate still rolls a genuine overnight', () => {
const out = fixArrivalDate({ type: 'flight', departure_time: '2026-06-11T10:00 pm', arrival_time: '2026-06-12T06:00 am' });
expect(out.arrival_time).toBe('2026-06-12T06:00:00');
});
it('resolves a meridiem that sits behind an ISO date', async () => {
extractEnforced.mockResolvedValue({ name: 'B&B Hotel', address: 'Str 1', checkin_time: '2026-05-01T03:00 pm', checkout_time: '2026-05-02T11:00 am' });
await routeExtraction('Hotel booking — check-in 1 May', CTX);
const flats = mapToKi.mock.calls[0][0];
expect(flats[0].checkin_time).toBe('2026-05-01T15:00:00');
expect(flats[0].checkout_time).toBe('2026-05-02T11:00:00');
});
it('marks a type it could not read instead of writing it down as a hotel', async () => {
extractEnforced.mockResolvedValue({ type: 'shuttle voucher', name: 'Airport Transfer' });
await routeExtraction('A document with no obvious type keywords', CTX);
const flats = mapToKi.mock.calls[0][0];
// Still the hotel shape, so the item survives mapping, but the guess is
// declared so the importer can offer the form the user was importing into.
expect(flats[0].type).toBe('hotel');
expect(flats[0].type_guessed).toBe(true);
});
it('does not mark a type the model picked legitimately', async () => {
extractEnforced.mockResolvedValue({ type: 'event', name: 'Concert' });
await routeExtraction('A document with no obvious type keywords', CTX);
const flats = mapToKi.mock.calls[0][0];
expect(flats[0].type).toBe('event');
expect(flats[0].type_guessed).toBeUndefined();
});
});