mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-21 22:46:32 +00:00
fix(extract): retry with json_object and surface AI import failures (#1546)
OpenAI-compatible providers that only support json_object (DeepSeek, Mistral, some vLLM/llama.cpp) reject the json_schema response_format with a 400, and the resulting error was swallowed silently: not logged server-side and never rendered by the background-task widget, leaving only a generic "no reservations" message. - Retry the chat/completions request once with response_format json_object when the json_schema attempt returns 400 (non-NuExtract only); the system prompt already dictates the output shape - Log swallowed llm-parse errors with an [llm-parse] tag so failures show up in server logs - Render task warnings under the empty-preview note in the background tasks widget so the actual provider error reaches the user
This commit is contained in:
@@ -0,0 +1,45 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { screen } from '@testing-library/react'
|
||||
import { render } from '../../../tests/helpers/render'
|
||||
import { useBackgroundTasksStore, type BackgroundImportTask } from '../../store/backgroundTasksStore'
|
||||
import BackgroundTasksWidget from './BackgroundTasksWidget'
|
||||
|
||||
vi.mock('../../api/websocket', () => ({ addListener: vi.fn(), removeListener: vi.fn() }))
|
||||
vi.mock('../../api/client', () => ({
|
||||
// Keep the rehydrate/poll backstops pending so the seeded state is what renders.
|
||||
reservationsApi: { importJobStatus: vi.fn(() => new Promise(() => {})) },
|
||||
}))
|
||||
|
||||
const task = (overrides: Partial<BackgroundImportTask> = {}): BackgroundImportTask => ({
|
||||
id: 'j1',
|
||||
tripId: 't1',
|
||||
label: 'voucher.pdf',
|
||||
status: 'done',
|
||||
done: 0,
|
||||
total: 1,
|
||||
items: [],
|
||||
warnings: [],
|
||||
...overrides,
|
||||
})
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
useBackgroundTasksStore.setState({ tasks: [] })
|
||||
})
|
||||
|
||||
describe('BackgroundTasksWidget', () => {
|
||||
it('shows the warnings when a finished job produced no items', () => {
|
||||
const warning = 'voucher.pdf: AI parsing failed — LLM request failed (400): response_format unsupported'
|
||||
useBackgroundTasksStore.setState({ tasks: [task({ warnings: [warning] })] })
|
||||
render(<BackgroundTasksWidget />)
|
||||
expect(screen.getByText('No reservations could be extracted from the uploaded files.')).toBeInTheDocument()
|
||||
expect(screen.getByText(warning)).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it('shows only the empty-preview note when there are no warnings', () => {
|
||||
useBackgroundTasksStore.setState({ tasks: [task()] })
|
||||
render(<BackgroundTasksWidget />)
|
||||
expect(screen.getByText('No reservations could be extracted from the uploaded files.')).toBeInTheDocument()
|
||||
expect(screen.queryByText(/AI parsing failed/)).not.toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
@@ -136,7 +136,14 @@ export default function BackgroundTasksWidget() {
|
||||
{t('common.import')}
|
||||
</button>
|
||||
) : (
|
||||
<div style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', color: 'var(--text-faint)', marginTop: 1 }}>{t('reservations.import.previewEmpty')}</div>
|
||||
<div style={{ fontSize: 'calc(11px * var(--fs-scale-caption, 1))', color: 'var(--text-faint)', marginTop: 1 }}>
|
||||
{t('reservations.import.previewEmpty')}
|
||||
{(task.warnings?.length ?? 0) > 0 && (
|
||||
<div style={{ color: '#b45309', marginTop: 3, whiteSpace: 'pre-wrap', wordBreak: 'break-word', maxHeight: 96, overflowY: 'auto' }}>
|
||||
{task.warnings!.join('\n')}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)
|
||||
)}
|
||||
|
||||
|
||||
@@ -18,6 +18,10 @@ const MAX_TOKENS = 4096;
|
||||
* template inlined in a single user message, no system prompt and no
|
||||
* `response_format` (see ./nuextract.ts) — that's how the fine-tune expects to
|
||||
* be driven; the generic instruct path applies to every other model.
|
||||
*
|
||||
* Structured output is requested as `json_schema` first; servers that only
|
||||
* support `json_object` (DeepSeek, Mistral, some vLLM/llama.cpp) reject that
|
||||
* with a 400, so the request is retried once in `json_object` mode.
|
||||
*/
|
||||
export class OpenAiCompatibleClient implements LlmExtractionClient {
|
||||
async extract(input: LlmExtractionInput): Promise<Record<string, unknown>[]> {
|
||||
@@ -39,7 +43,7 @@ export class OpenAiCompatibleClient implements LlmExtractionClient {
|
||||
});
|
||||
}
|
||||
|
||||
const body = {
|
||||
const baseBody = {
|
||||
model: input.model,
|
||||
max_tokens: MAX_TOKENS,
|
||||
// Extraction is a deterministic task — Ollama defaults to 0.7, which makes
|
||||
@@ -53,33 +57,24 @@ export class OpenAiCompatibleClient implements LlmExtractionClient {
|
||||
{ role: 'system', content: input.prompt },
|
||||
{ role: 'user', content: userContent },
|
||||
],
|
||||
...(nuextract
|
||||
? {}
|
||||
: {
|
||||
response_format: {
|
||||
type: 'json_schema' as const,
|
||||
json_schema: { name: 'reservations', schema: input.jsonSchema, strict: false },
|
||||
},
|
||||
}),
|
||||
};
|
||||
const body = nuextract
|
||||
? baseBody
|
||||
: {
|
||||
...baseBody,
|
||||
response_format: {
|
||||
type: 'json_schema' as const,
|
||||
json_schema: { name: 'reservations', schema: input.jsonSchema, strict: false },
|
||||
},
|
||||
};
|
||||
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), TIMEOUT_MS);
|
||||
let res: Response;
|
||||
try {
|
||||
// baseUrl is user-configurable — guard it against pointing at the cloud
|
||||
// metadata endpoint, while still allowing a local/LAN Ollama.
|
||||
res = await safeFetchLlm(url, {
|
||||
method: 'POST',
|
||||
signal: controller.signal,
|
||||
headers: {
|
||||
'content-type': 'application/json',
|
||||
...(input.apiKey ? { authorization: `Bearer ${input.apiKey}` } : {}),
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
let res = await this.send(url, body, input.apiKey);
|
||||
// Servers that only support `json_object` (DeepSeek, Mistral, some
|
||||
// vLLM/llama.cpp) reject `json_schema` with a 400 — retry once in
|
||||
// `json_object` mode. The system prompt already dictates the exact output
|
||||
// shape (and mentions JSON, which json_object mode requires).
|
||||
if (!res.ok && res.status === 400 && !nuextract) {
|
||||
res = await this.send(url, { ...baseBody, response_format: { type: 'json_object' as const } }, input.apiKey);
|
||||
}
|
||||
|
||||
if (!res.ok) {
|
||||
@@ -93,6 +88,26 @@ export class OpenAiCompatibleClient implements LlmExtractionClient {
|
||||
const content = data.choices?.[0]?.message?.content;
|
||||
return nuextract ? parseNuExtract(content) : parseReservations(content);
|
||||
}
|
||||
|
||||
private async send(url: string, body: unknown, apiKey?: string): Promise<Response> {
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), TIMEOUT_MS);
|
||||
try {
|
||||
// baseUrl is user-configurable — guard it against pointing at the cloud
|
||||
// metadata endpoint, while still allowing a local/LAN Ollama.
|
||||
return await safeFetchLlm(url, {
|
||||
method: 'POST',
|
||||
signal: controller.signal,
|
||||
headers: {
|
||||
'content-type': 'application/json',
|
||||
...(apiKey ? { authorization: `Bearer ${apiKey}` } : {}),
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
} finally {
|
||||
clearTimeout(timer);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Strip code fences and JSON.parse; `null` on failure. */
|
||||
|
||||
@@ -71,6 +71,7 @@ export class LlmParseService {
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
console.error(`[llm-parse] Could not read "${file.originalName}":`, err instanceof Error ? err.message : err);
|
||||
return {
|
||||
kiItems: [],
|
||||
warnings: [`${file.originalName}: could not read file — ${err instanceof Error ? err.message : String(err)}`],
|
||||
@@ -90,6 +91,7 @@ export class LlmParseService {
|
||||
});
|
||||
return { kiItems: routed.kiItems, warnings: [...warnings, ...routed.warnings] };
|
||||
} catch (err) {
|
||||
console.error(`[llm-parse] AI parsing failed for "${file.originalName}" (provider=${config.provider}):`, err instanceof Error ? err.message : err);
|
||||
return {
|
||||
kiItems: [],
|
||||
warnings: [`${file.originalName}: AI parsing failed — ${err instanceof Error ? err.message : String(err)}`],
|
||||
@@ -102,6 +104,7 @@ export class LlmParseService {
|
||||
raw = await createLlmClient(config).extract(input);
|
||||
console.debug('[DEBUG] Raw LLM Response: ', raw);
|
||||
} catch (err) {
|
||||
console.error(`[llm-parse] AI parsing failed for "${file.originalName}" (provider=${config.provider}):`, err instanceof Error ? err.message : err);
|
||||
return {
|
||||
kiItems: [],
|
||||
warnings: [`${file.originalName}: AI parsing failed — ${err instanceof Error ? err.message : String(err)}`],
|
||||
|
||||
@@ -55,6 +55,41 @@ describe('OpenAiCompatibleClient', () => {
|
||||
it('throws on non-2xx', async () => {
|
||||
mockFetch(() => jsonResponse({ error: 'bad' }, false, 401));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/401/);
|
||||
expect(safeFetchLlmMock).toHaveBeenCalledTimes(1); // no retry on non-400
|
||||
});
|
||||
|
||||
it('retries once with json_object when the server rejects json_schema (400)', async () => {
|
||||
safeFetchLlmMock
|
||||
.mockResolvedValueOnce(jsonResponse({ error: { message: 'response_format type is unavailable' } }, 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(2);
|
||||
|
||||
const first = JSON.parse((safeFetchLlmMock.mock.calls[0][1] as RequestInit).body as string);
|
||||
const second = JSON.parse((safeFetchLlmMock.mock.calls[1][1] as RequestInit).body as string);
|
||||
expect(first.response_format.type).toBe('json_schema');
|
||||
expect(second.response_format).toEqual({ type: 'json_object' });
|
||||
expect(second.messages).toEqual(first.messages);
|
||||
expect(second.model).toBe(first.model);
|
||||
});
|
||||
|
||||
it('throws when the json_object retry also fails (400 twice)', async () => {
|
||||
safeFetchLlmMock
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'no json_schema' }, false, 400))
|
||||
.mockResolvedValueOnce(jsonResponse({ error: 'no json_object either' }, false, 400));
|
||||
await expect(new OpenAiCompatibleClient().extract(baseInput)).rejects.toThrow(/400/);
|
||||
expect(safeFetchLlmMock).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it('does not retry the NuExtract path on 400', async () => {
|
||||
safeFetchLlmMock.mockResolvedValueOnce(jsonResponse({ error: 'bad' }, false, 400));
|
||||
await expect(
|
||||
new OpenAiCompatibleClient().extract({ ...baseInput, model: 'hf.co/numind/NuExtract-2.0-2B-GGUF:latest' }),
|
||||
).rejects.toThrow(/400/);
|
||||
expect(safeFetchLlmMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('sends an image natively as image_url but never a file/pdf part', async () => {
|
||||
|
||||
@@ -122,6 +122,14 @@ describe('LlmParseService', () => {
|
||||
expect(res.warnings[0]).toMatch(/AI parsing failed/i);
|
||||
});
|
||||
|
||||
it('logs the swallowed client error to console.error', async () => {
|
||||
const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
extract.mockRejectedValue(new Error('boom'));
|
||||
await svc().parse(file('a.txt'), 1);
|
||||
expect(spy).toHaveBeenCalledWith(expect.stringContaining('[llm-parse]'), 'boom');
|
||||
spy.mockRestore();
|
||||
});
|
||||
|
||||
it('routes the local provider through the extraction router instead of the single-shot client', async () => {
|
||||
resolveLlmConfig.mockReturnValue(cfg({ provider: 'local', baseUrl: 'http://ollama:11434/v1', apiKey: 'k' }));
|
||||
extractText.mockResolvedValue('Hotel booking');
|
||||
@@ -159,6 +167,15 @@ describe('LlmParseService', () => {
|
||||
expect(res.warnings[0]).toMatch(/AI parsing failed/i);
|
||||
});
|
||||
|
||||
it('logs the swallowed router error to console.error', async () => {
|
||||
const spy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
resolveLlmConfig.mockReturnValue(cfg({ provider: 'local' }));
|
||||
routeExtraction.mockRejectedValue(new Error('ollama down'));
|
||||
await svc().parse(file('a.txt'), 1);
|
||||
expect(spy).toHaveBeenCalledWith(expect.stringContaining('[llm-parse]'), 'ollama down');
|
||||
spy.mockRestore();
|
||||
});
|
||||
|
||||
it('warns when the file cannot be read (text extraction throws)', async () => {
|
||||
extractText.mockRejectedValue(new Error('corrupt pdf'));
|
||||
const res = await svc().parse(file('a.pdf', '%PDF'), 1);
|
||||
|
||||
Reference in New Issue
Block a user