diff --git a/admin/app/services/chat_service.ts b/admin/app/services/chat_service.ts index 619cadb4..7a44f68d 100644 --- a/admin/app/services/chat_service.ts +++ b/admin/app/services/chat_service.ts @@ -8,6 +8,13 @@ import { OllamaService } from './ollama_service.js' import { SYSTEM_PROMPTS } from '../../constants/ollama.js' import { toTitleCase } from '../utils/misc.js' import { resolveTasksModel } from '../utils/tasks_model.js' +import { + SUGGESTIONS_SCHEMA, + TITLE_SCHEMA, + parseStructured, + pickSuggestions, + pickTitle, +} from '../utils/structured_output.js' @inject() export class ChatService { @@ -90,11 +97,25 @@ export class ChatService { stream: false, think: false, thinkingCapable, + // Grammar-constrained on the native transport, so the response is three + // strings in an array rather than whatever prose the model felt like. + format: SUGGESTIONS_SCHEMA, + // The default of 0.8 is actively hostile to format stability, and there is + // nothing creative about picking three canned opening questions. + temperature: 0, }) if (response && response.message && response.message.content) { const content = response.message.content.trim() - + + const structured = parseStructured(content, pickSuggestions) + if (structured) { + return structured.map((s) => toTitleCase(s)) + } + logger.warn( + `[ChatService] Model "${model}" returned unparseable suggestion JSON; falling back to text parsing` + ) + // Handle both comma-separated and newline-separated formats let suggestions: string[] = [] @@ -273,8 +294,6 @@ export class ChatService { async generateTitle(sessionId: number, userMessage: string, assistantMessage: string, model: string) { try { - let title: string - // Titles are aesthetic work; route them to the tasks model when one is // configured rather than the chat model that just answered. const titleModel = (await this.resolveTasksModel(model)) ?? model @@ -291,9 +310,17 @@ export class ChatService { ], think: false, thinkingCapable, + format: TITLE_SCHEMA, + // See the note on suggestions: naming a chat is not a creative task, and + // the backend default of 0.8 makes the format wobble. + temperature: 0, }) - title = response?.message?.content?.trim() + const content = response?.message?.content?.trim() ?? '' + // The schema path and the text path share the same truncation: "under 50 + // characters" is a request the model can ignore either way. + let title = parseStructured(content, pickTitle) ?? content + title = title.slice(0, 57) + (title.length > 57 ? '...' : '') if (!title) { // Nothing left once reasoning was split out — fall back to the user's own words. logger.warn( diff --git a/admin/app/services/ollama_service.ts b/admin/app/services/ollama_service.ts index 30442c28..512ae4a3 100644 --- a/admin/app/services/ollama_service.ts +++ b/admin/app/services/ollama_service.ts @@ -112,6 +112,12 @@ type ChatInput = { // (batching and GPU non-determinism still move outputs, hence --repeats). temperature?: number seed?: number + // JSON Schema (or the string 'json') constraining the decoder, for the structured + // ancillary calls — title, suggestion chips, query rewrite. Native-only: it is a + // top-level field on /api/chat, not an `options` member, and the OpenAI-compat + // endpoint has no equivalent we can rely on across backends. Callers must keep a + // string-parsing fallback for exactly that reason. + format?: string | object // Aborts the upstream request when the client disconnects, so an abandoned generation // doesn't keep decoding server-side and block Ollama's single parallel slot (#1065). signal?: AbortSignal @@ -516,6 +522,9 @@ export class OllamaService { // nothing, so non-thinking models and other backends are unaffected. ...(chatRequest.think !== undefined ? { think: chatRequest.think } : {}), ...(chatRequest.keepAlive !== undefined ? { keep_alive: chatRequest.keepAlive } : {}), + // Sibling of `options`, not a member of it — putting it in the options bag is + // silently ignored and the call decodes unconstrained. + ...(chatRequest.format !== undefined ? { format: chatRequest.format } : {}), options: this._nativeOptions(chatRequest), }) diff --git a/admin/app/services/rag_pipeline_service.ts b/admin/app/services/rag_pipeline_service.ts index 93c82631..264ff767 100644 --- a/admin/app/services/rag_pipeline_service.ts +++ b/admin/app/services/rag_pipeline_service.ts @@ -19,6 +19,7 @@ import { planPrompt } from '../utils/context_budget.js' import { estimateMessagesTokens } from '../utils/token_estimate.js' import { resolveTasksModel } from '../utils/tasks_model.js' import { buildContextBlock, getContextLimitsForModel } from '../utils/rag_prompt.js' +import { QUERIES_SCHEMA, parseStructured, pickQueries } from '../utils/structured_output.js' /** * Everything that happens between "a user sent a message" and "a payload goes @@ -278,9 +279,16 @@ export class RagPipelineService { temperature: 0, think: false, thinkingCapable, + // Grammar-constrained on the native transport. An array rather than a bare + // string because multi-query fusion then costs a prompt change and nothing + // else; only the first entry is used today. + format: QUERIES_SCHEMA, }) - const rewrittenQuery = response.message.content.trim() + const raw = response.message.content.trim() + // Falls back to the raw text on any parse failure — on a non-native backend the + // grammar was never applied, so the old bare-string behaviour is still correct. + const rewrittenQuery = parseStructured(raw, pickQueries)?.[0] ?? raw // Empty means the response was reasoning and nothing else (or was truncated // mid-thought). Embedding an empty string would search the corpus for nothing // and quietly poison retrieval for the rest of the conversation, so fall back diff --git a/admin/app/utils/structured_output.ts b/admin/app/utils/structured_output.ts new file mode 100644 index 00000000..e129dbed --- /dev/null +++ b/admin/app/utils/structured_output.ts @@ -0,0 +1,121 @@ +/** + * JSON Schemas and a tolerant parser for the structured ancillary calls — + * chat titles, suggestion chips, and the RAG query rewrite. + * + * The prompts used to *ask* for a format and the callers recovered the answer by + * string surgery: split on comma, else on newline, strip `1.`, strip bullets, + * strip quotes. That is a request, not a constraint, and the failure mode was + * silent — an empty chip list, or a paragraph as the sidebar title. + * + * Ollama's `format` parameter compiles a JSON Schema to a GBNF grammar, so the + * decoder cannot emit anything else. It only exists on the native /api/chat + * transport, so every caller keeps its old parser as the fallback for other + * backends; `parseStructured` returning null is what selects that path. + * + * Pure functions, no framework imports — see `think_stream.ts` / `rag_prompt.ts`. + */ + +/** Sidebar chat title. */ +export const TITLE_SCHEMA = { + type: 'object', + properties: { + title: { type: 'string' }, + }, + required: ['title'], + additionalProperties: false, +} as const + +/** The three conversation-starter chips on an empty chat. */ +export const SUGGESTIONS_SCHEMA = { + type: 'object', + properties: { + suggestions: { + type: 'array', + items: { type: 'string' }, + minItems: 3, + maxItems: 3, + }, + }, + required: ['suggestions'], + additionalProperties: false, +} as const + +/** + * History-aware query rewrite. An array rather than a single string even though + * only the first entry is used today: multi-query / RAG-Fusion (gap analysis 3.2) + * then needs a prompt change and nothing else. + */ +export const QUERIES_SCHEMA = { + type: 'object', + properties: { + queries: { + type: 'array', + items: { type: 'string' }, + minItems: 1, + }, + }, + required: ['queries'], + additionalProperties: false, +} as const + +/** + * Pull a JSON object out of a model response and narrow it with `pick`. + * + * Total by construction — every failure returns null so the caller falls through + * to its string parser. This runs on the chat critical path via the rewrite, so + * it must never throw. + * + * Tolerates the things an unconstrained model does anyway: a ```json fence, a + * sentence of preamble, trailing commentary. Grammar-constrained output needs + * none of that, but the same helper is used on backends where the grammar was + * never applied. + */ +export function parseStructured(raw: string, pick: (value: any) => T | null): T | null { + if (!raw) return null + + // Outermost braces: the first `{` to the last `}`. Preamble and trailing prose + // both fall outside it, and a nested object stays inside. + const start = raw.indexOf('{') + const end = raw.lastIndexOf('}') + if (start === -1 || end <= start) return null + + let parsed: any + try { + parsed = JSON.parse(raw.slice(start, end + 1)) + } catch { + return null + } + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return null + + try { + return pick(parsed) + } catch { + return null + } +} + +/** Non-empty trimmed string, or null. */ +function cleanString(value: unknown): string | null { + if (typeof value !== 'string') return null + const trimmed = value.trim() + return trimmed.length > 0 ? trimmed : null +} + +/** Trimmed, non-empty entries of a string array. Null when nothing survives. */ +function cleanStringArray(value: unknown): string[] | null { + if (!Array.isArray(value)) return null + const cleaned = value.map(cleanString).filter((s): s is string => s !== null) + return cleaned.length > 0 ? cleaned : null +} + +/** `{title: string}` -> the title. */ +export const pickTitle = (value: any): string | null => cleanString(value?.title) + +/** `{suggestions: string[]}` -> up to three non-empty suggestions. */ +export const pickSuggestions = (value: any): string[] | null => { + const suggestions = cleanStringArray(value?.suggestions) + return suggestions ? suggestions.slice(0, 3) : null +} + +/** `{queries: string[]}` -> every non-empty query, best first. */ +export const pickQueries = (value: any): string[] | null => cleanStringArray(value?.queries) diff --git a/admin/constants/ollama.ts b/admin/constants/ollama.ts index 43f5abd1..e655e91c 100644 --- a/admin/constants/ollama.ts +++ b/admin/constants/ollama.ts @@ -138,9 +138,11 @@ export const RAG_PLACEMENT: 'tail' | 'front' = 'tail' * Token cap on the query-rewrite call. A rewrite is one short sentence; the * prompt already asks for under 150 words. Without a cap a small model can * ramble, and every one of those tokens is latency the user waits through - * before retrieval even starts. + * before retrieval even starts. Sized to leave room for the JSON envelope the + * rewrite is now grammar-constrained to emit — a truncated object parses to + * nothing and costs the turn its rewrite entirely. */ -export const QUERY_REWRITE_MAX_TOKENS = 120 +export const QUERY_REWRITE_MAX_TOKENS = 160 /** * How long Ollama keeps a chat model — and its KV cache — resident after a @@ -206,13 +208,13 @@ Do NOT use: - Statements that are not suggestions themselves, such as praise for asking the question - Direct questions or commands to the user -Return ONLY the 3 suggestions as a comma-separated list with no additional text, formatting, numbering, or quotation marks. The suggestions should be in title case. -Ensure that your suggestions are comma-separated with no conjunctions like "and" or "or". -Do not use line breaks, new lines, or extra spacing to separate the suggestions. -Format: suggestion1, suggestion2, suggestion3 + +Respond with JSON: {"suggestions": ["...", "...", "..."]} `, - title_generation: `You are a title generator. Given the start of a conversation, generate a concise, descriptive title under 50 characters. Return ONLY the title text with no quotes, punctuation wrapping, or extra formatting.`, + title_generation: `You are a title generator. Given the start of a conversation, generate a concise, descriptive title under 50 characters. + +Respond with JSON: {"title": "..."}`, query_rewrite: ` You are a query rewriting assistant. Your task is to reformulate the user's latest question to include relevant context from the conversation history. @@ -223,7 +225,7 @@ Rules: 2. Include key entities, topics, and context from previous messages 3. Make it a clear, searchable query 4. Do NOT answer the question - only rewrite the user's query to be more effective for retrieval -5. Output ONLY the rewritten query, nothing else +5. Respond with JSON: {"queries": ["..."]} — a single rewritten query in the array Examples: @@ -232,7 +234,7 @@ User: "How do I install Gentoo?" Assistant: [detailed installation guide] User: "Is an internet connection required to install?" -Rewritten Query: "Is an internet connection required to install Gentoo Linux?" +Rewritten Query: {"queries": ["Is an internet connection required to install Gentoo Linux?"]} --- @@ -241,6 +243,6 @@ User: "What's the best way to preserve meat?" Assistant: [preservation methods] User: "How long does it last?" -Rewritten Query: "How long does preserved meat last using curing or smoking methods?" +Rewritten Query: {"queries": ["How long does preserved meat last using curing or smoking methods?"]} `, } diff --git a/admin/tests/unit/structured_output.spec.ts b/admin/tests/unit/structured_output.spec.ts new file mode 100644 index 00000000..392476ed --- /dev/null +++ b/admin/tests/unit/structured_output.spec.ts @@ -0,0 +1,165 @@ +/** + * Tolerant JSON extraction for the constrained ancillary calls. + * + * The grammar makes the happy path boring; what these lock in is the *fallback* + * behaviour, because `parseStructured` runs on every backend, including the ones + * where `format` was never applied. Returning null is what selects a caller's + * old string parser, so a wrong null and a wrong non-null are both bugs. + * + * Pure functions only — no MySQL, Redis, Qdrant, or Ollama needed: + * npm run test:unit + */ +import * as assert from 'node:assert/strict' +import { test } from 'node:test' + +import { + QUERIES_SCHEMA, + SUGGESTIONS_SCHEMA, + TITLE_SCHEMA, + parseStructured, + pickQueries, + pickSuggestions, + pickTitle, +} from '../../app/utils/structured_output.js' + +// --- parseStructured: extraction --------------------------------------------- + +test('parses a clean object', () => { + assert.equal(parseStructured('{"title": "Water Purification"}', pickTitle), 'Water Purification') +}) + +test('parses through a ```json fence', () => { + const raw = '```json\n{"title": "Gentoo Install"}\n```' + assert.equal(parseStructured(raw, pickTitle), 'Gentoo Install') +}) + +test('parses past leading prose and trailing commentary', () => { + const raw = 'Sure, here you go:\n{"title": "Meat Curing"}\nHope that helps!' + assert.equal(parseStructured(raw, pickTitle), 'Meat Curing') +}) + +test('nested objects survive the outermost-brace span', () => { + const raw = '{"title": "A", "meta": {"b": 1}}' + assert.equal(parseStructured(raw, pickTitle), 'A') +}) + +test('malformed JSON returns null', () => { + assert.equal(parseStructured('{"title": "unterminated', pickTitle), null) +}) + +test('no braces at all returns null', () => { + assert.equal(parseStructured('Water Purification Basics', pickTitle), null) +}) + +test('empty input returns null', () => { + assert.equal(parseStructured('', pickTitle), null) +}) + +test('a bare JSON array is not an object', () => { + // `[{"title": "x"}]` — the brace span parses, but the top level must be an object. + assert.equal(parseStructured('["a", "b"]', pickTitle), null) +}) + +test('a throwing picker is caught, not propagated', () => { + // This is on the chat critical path via the rewrite; it must never throw. + assert.equal( + parseStructured('{"title": "x"}', () => { + throw new Error('boom') + }), + null + ) +}) + +// --- pickTitle --------------------------------------------------------------- + +test('title: wrong type is rejected rather than stringified', () => { + assert.equal(parseStructured('{"title": 42}', pickTitle), null) +}) + +test('title: whitespace-only is rejected', () => { + assert.equal(parseStructured('{"title": " "}', pickTitle), null) +}) + +test('title: missing key is rejected', () => { + assert.equal(parseStructured('{"name": "x"}', pickTitle), null) +}) + +test('title: extra keys are tolerated', () => { + assert.equal(parseStructured('{"title": "x", "confidence": 0.9}', pickTitle), 'x') +}) + +// --- pickSuggestions --------------------------------------------------------- + +test('suggestions: three strings come back trimmed', () => { + const raw = '{"suggestions": [" How Do I Purify Water? ", "B", "C"]}' + assert.deepEqual(parseStructured(raw, pickSuggestions), ['How Do I Purify Water?', 'B', 'C']) +}) + +test('suggestions: an over-long array is sliced to three', () => { + const raw = '{"suggestions": ["A", "B", "C", "D", "E"]}' + assert.deepEqual(parseStructured(raw, pickSuggestions), ['A', 'B', 'C']) +}) + +test('suggestions: non-string entries are dropped, not coerced', () => { + assert.deepEqual(parseStructured('{"suggestions": ["A", 7, null, "B"]}', pickSuggestions), [ + 'A', + 'B', + ]) +}) + +test('suggestions: an empty array is null so the text parser runs', () => { + assert.equal(parseStructured('{"suggestions": []}', pickSuggestions), null) +}) + +test('suggestions: an array of only blanks is null', () => { + assert.equal(parseStructured('{"suggestions": ["", " "]}', pickSuggestions), null) +}) + +test('suggestions: a string instead of an array is null', () => { + assert.equal(parseStructured('{"suggestions": "A, B, C"}', pickSuggestions), null) +}) + +// --- pickQueries ------------------------------------------------------------- + +test('queries: the first entry is the one retrieval uses', () => { + const raw = '{"queries": ["Is internet required to install Gentoo Linux?"]}' + assert.deepEqual(parseStructured(raw, pickQueries), [ + 'Is internet required to install Gentoo Linux?', + ]) +}) + +test('queries: extra paraphrases are kept for future multi-query fusion', () => { + const raw = '{"queries": ["a", "b", "c"]}' + assert.deepEqual(parseStructured(raw, pickQueries), ['a', 'b', 'c']) +}) + +test('queries: a leading blank is dropped so [0] is never empty', () => { + // An empty query would be embedded and quietly poison retrieval for the turn. + assert.deepEqual(parseStructured('{"queries": [" ", "real query"]}', pickQueries), [ + 'real query', + ]) +}) + +test('queries: an empty array is null so the raw message is used', () => { + assert.equal(parseStructured('{"queries": []}', pickQueries), null) +}) + +// --- schemas ----------------------------------------------------------------- + +test('every schema marks its key required and forbids extra properties', () => { + // Without `required`, GBNF happily lets a small model emit `{}`. + for (const [schema, key] of [ + [TITLE_SCHEMA, 'title'], + [SUGGESTIONS_SCHEMA, 'suggestions'], + [QUERIES_SCHEMA, 'queries'], + ] as const) { + assert.deepEqual(schema.required, [key]) + assert.equal(schema.additionalProperties, false) + assert.equal(schema.type, 'object') + } +}) + +test('the suggestions schema pins the count at exactly three', () => { + assert.equal(SUGGESTIONS_SCHEMA.properties.suggestions.minItems, 3) + assert.equal(SUGGESTIONS_SCHEMA.properties.suggestions.maxItems, 3) +})