From 6aecbdc9b77ec15e9694fe5469ef619a85b7a9d4 Mon Sep 17 00:00:00 2001 From: Jakob Wennberg Date: Mon, 24 Aug 2026 13:19:15 +0200 Subject: [PATCH] fix(agent): surface empty assistant answers instead of silent stops (#1830) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The support chat showed 'Tänker' and then went quiet with no answer and no error. Root cause: the 2026-08-21 RIP-3 cutover moved general.help to the single-call POST /api/agent/ask, whose 1500-token default cap made stop_reason max_tokens routine on tool-loop turns. The empty answer then passed unlogged through the service, the route answered 200 with an empty string, and the console appended an invisible empty bubble. Fixes, single-call path: - ask-service: default maxTokens 1500 -> 5400 (the streaming chat's reply ceiling); an empty final answer now logs model + usage and throws the typed EmptyModelAnswerError instead of passing through. - /api/agent/ask: maxDuration 300, logger, empty answer maps to 502 with 'Assistenten gav inget svar. Försök igen.'; an empty assistant turn is never persisted (the question stays, so retry works). - AskConsole: a 200 with an empty answer shows the error box instead of appending an invisible bubble. - anthropic-family: serialized tool results are bounded at 40000 chars (mirrors run-turn) so one big read cannot eat the output budget; the step-exhausted fallback keeps tools declared with tool_choice none, because replaying tool_use/tool_result without tools is an API 400. Fixes, streaming path (same silent class): - /api/agent/invoke: maxDuration 300 so deep thinking turns are not killed mid-stream at the platform default cap. - run-turn: stop_reason max_tokens with no visible text emits an error event, not a bare turn_complete. - AgentChat: an NDJSON stream that ends without turn_complete or error (and was not aborted) shows 'Anslutningen bröts innan svaret blev klart. Försök igen.' The lib/ai request-shape tests were updated deliberately for the fallback change; general.help stays on the single-call runtime (founder decision, not reverted). Claude-Session: https://claude.ai/code/session_01SyDuePXxUFowaPBKpAv8SF Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com> Co-authored-by: Claude Fable 5 --- DECISIONS.md | 1 + app/api/agent/ask/__tests__/route.test.ts | 27 +++- app/api/agent/ask/route.ts | 32 ++++ app/api/agent/invoke/route.ts | 7 + components/agent/AgentChat.tsx | 14 ++ components/agent/AskConsole.tsx | 8 + lib/agent/ask/__tests__/ask-service.test.ts | 17 ++ lib/agent/ask/ask-service.ts | 28 +++- lib/agent/ask/errors.ts | 18 +++ .../run-turn-empty-max-tokens.test.ts | 153 ++++++++++++++++++ lib/agent/chat/run-turn.ts | 25 +++ lib/ai/__tests__/anthropic-family.test.ts | 57 ++++++- lib/ai/services/anthropic-family.ts | 29 +++- 13 files changed, 409 insertions(+), 7 deletions(-) create mode 100644 lib/agent/ask/errors.ts create mode 100644 lib/agent/chat/__tests__/run-turn-empty-max-tokens.test.ts diff --git a/DECISIONS.md b/DECISIONS.md index 9fd5cc7d..1aa5e83c 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -1171,5 +1171,6 @@ One line per decision: `[YYYY-MM-DD] : `. Appended by agents and [2026-08-22] Per-company invoice sending domains are gated by a manually granted capability (custom_sender_domain), deliberately NOT in PAID_CAPABILITIES: the opt-in must not be trial-seeded or written by the Stripe subscription sync, and non-grantees must see an unchanged invoicing settings page (the section hides on the 403 capability_blocked envelope). The sending-domain module has no Resend orphan-adoption path (a name that already exists is a 409), because the same Resend account holds the platform's own outbound domain. The delivery log was left untouched (no from_address column): adding it would re-open the hardened invoice_deliveries evidence triggers/redaction paths for a nice-to-have, and the log already measures delivered/bounced per send. [2026-08-22] company_sending_domains verification state (domain, status, resend_domain_id, dns_records, verified_at, last_checked_at) is service-role only via a BEFORE trigger keyed on the JWT role claim; tenant JWTs may only open a pending claim and edit sender_local_part/sender_name/enabled. Skeptic refutation: RLS alone let a granted admin insert {domain: platform sender domain, status: verified} through PostgREST and send invoice mail as the platform. The claim/verify helpers therefore take a separate service-role writer for those columns. Second refutation: a domain Resend later flips to failed made every invoice send for that company fail; the Resend adapter now retries once as the platform sender when an explicit company From is rejected (nothing was sent on the rejected attempt, so the retry cannot double-send). [2026-08-22] Sending-domain verification writes bind by (id, company_id, domain, resend_domain_id IS NULL) and verify/webhook compare Resend's domain name with the row before writing verified; resolveInvoiceSender additionally refuses reserved platform domains and non-hostnames at send time. Skeptic re-check: a tenant could delete and re-insert its pending row under the same id with a reserved domain during the claim's Resend round-trip (TOCTOU), and the service-role writer updated by id alone. Defense in depth over a single gate. +[2026-08-24] Assistant empty answer is a typed failure (EmptyModelAnswerError, 502 'Assistenten gav inget svar. Försök igen.'), one manual retry, NO auto-retry: the silent 200 with an empty answer was the 'Tänker then nothing' bug after the RIP-3 cutover (1500-token cap on /api/agent/ask), and auto-retry would double model spend while hiding the regression. Related deliberate request-shape change: anthropic-family's step-exhausted fallback now keeps tools with tool_choice none, because replaying a transcript containing tool_use/tool_result without declaring tools is rejected by the Messages API (the old shape 400ed every step-exhausted answer). general.help stays on the single-call runtime (founder decision); the fix raises headroom to 5400 tokens and surfaces the failure instead. [2026-08-24] Issue #1820 self-billed credit fix: creditConfirmNumber()/originalRef fall back invoice_number -> external_invoice_number (typed 400 INVOICE_CREDIT_NO_NUMBER if both null) instead of relaxing the DB numbering constraint or dropping the type-the-number confirm step; the confirm step stays (dropping it is a founder call). The invoice-date Forval chip surfaces in ALL editor modes, not only self-billed: the silent today-default exists in every mode and the chip line already carries the due date. In self-billed mode fakturadatum + mottagningsdatum render uncollapsed next to the external number (transcription fields, not defaults); the panel rows are hidden there because registering the same RHF field twice desyncs the inputs. The v1 credit route's existing id-slice fallback was left unchanged (public API behavior). [2026-08-24] No-IBAN reconnect pairing (issue #1709) uses only per-currency exactly-one-each-side elimination, deliberately WITHOUT name equality: ASPSPs reformat product names between consents, so requiring it would silently disable the fix for the banks that need it, while the one-per-currency guard already bounds a mis-pair to skipping rows whose account+date+amount+occurrence all collide. upsertFromPsd2 needed no change: its explicit reuse_cash_account_id promote path already covers a same-connection holder, so the fix only names the paired row from the callback. diff --git a/app/api/agent/ask/__tests__/route.test.ts b/app/api/agent/ask/__tests__/route.test.ts index 7494fe52..674a4a42 100644 --- a/app/api/agent/ask/__tests__/route.test.ts +++ b/app/api/agent/ask/__tests__/route.test.ts @@ -28,7 +28,8 @@ vi.mock('@/lib/agent/ask/persist', () => ({ persistAssistantTurn: (...a: unknown[]) => persistAssistant(...a), })) -import { POST } from '../route' +import { POST, maxDuration } from '../route' +import { EmptyModelAnswerError } from '@/lib/agent/ask/errors' const membershipChain = { select: () => membershipChain, eq: () => membershipChain, maybeSingle: async () => ({ data: { user_id: 'user-1' } }) } const supabase = { from: () => membershipChain } @@ -48,6 +49,9 @@ beforeEach(() => { const body = (o: Record = {}) => ({ question: 'Hur gick juli?', ...o }) describe('POST /api/agent/ask', () => { + it('declares a 300s function budget so a tool-loop answer is not killed mid-turn', () => { + expect(maxDuration).toBe(300) + }) it('401 when unauthenticated', async () => { requireAuthMock.mockResolvedValue({ user: null, supabase, error: NextResponse.json({ error: 'x' }, { status: 401 }) }) expect((await POST(createMockRequest('/api/agent/ask', { method: 'POST', body: body() }))).status).toBe(401) @@ -84,6 +88,15 @@ describe('POST /api/agent/ask', () => { expect(answer).not.toHaveBeenCalled() }) + it('502s with a Swedish message when the model answers empty (the silent-stop bug)', async () => { + answer.mockRejectedValue(new EmptyModelAnswerError('claude-sonnet-5')) + const res = await POST(createMockRequest('/x', { method: 'POST', body: body() })) + const { status, body: b } = await parseJsonResponse<{ error: string; code: string }>(res) + expect(status).toBe(502) + expect(b.error).toBe('Assistenten gav inget svar. Försök igen.') + expect(b.code).toBe('empty_model_answer') + }) + it('stateless (no persist): never touches the conversation tables', async () => { await POST(createMockRequest('/x', { method: 'POST', body: body() })) expect(resolveConv).not.toHaveBeenCalled() @@ -154,6 +167,18 @@ describe('POST /api/agent/ask', () => { expect(persistAssistant).not.toHaveBeenCalled() }) + it('502s on an empty answer, keeps the question, never persists a blank assistant turn', async () => { + answer.mockRejectedValue(new EmptyModelAnswerError('claude-sonnet-5')) + const res = await POST(createMockRequest('/x', { method: 'POST', body: body({ persist: true }) })) + const { status, body: b } = await parseJsonResponse<{ error: string }>(res) + expect(status).toBe(502) + expect(b.error).toBe('Assistenten gav inget svar. Försök igen.') + // The user turn is written before the model call (retry keeps the + // question), but no empty assistant turn may ever land in the thread. + expect(persistUser).toHaveBeenCalled() + expect(persistAssistant).not.toHaveBeenCalled() + }) + it('still 503s (no write) when no backend is configured', async () => { aiStatus.mockReturnValue({ configured: false }) const res = await POST(createMockRequest('/x', { method: 'POST', body: body({ persist: true }) })) diff --git a/app/api/agent/ask/route.ts b/app/api/agent/ask/route.ts index ea597159..4a6e6e33 100644 --- a/app/api/agent/ask/route.ts +++ b/app/api/agent/ask/route.ts @@ -8,7 +8,9 @@ import { guardSandbox } from '@/lib/sandbox/guard' import { requireCapability } from '@/lib/entitlements/has-capability' import { CAPABILITY } from '@/lib/entitlements/keys' import { getAiStatus } from '@/lib/ai' +import { createLogger } from '@/lib/logger' import { answerAssistantQuestion } from '@/lib/agent/ask/ask-service' +import { EmptyModelAnswerError } from '@/lib/agent/ask/errors' import { resolveChatConversation, persistUserTurn, @@ -22,6 +24,17 @@ import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-m // so a hosted deploy would silently lose its ledger tools. ensureInitialized() +const log = createLogger('api.agent.ask') + +// A tool-loop answer can take minutes (several model turns, each preceded by +// real report reads). The plan default cap would kill the function mid-answer +// and the client would see a silent empty stop; 300 s matches the other +// long-running model surfaces (app/api/receipt-hunt/run). +export const maxDuration = 300 + +// The Swedish body AskConsole's !res.ok branch renders verbatim. +const EMPTY_ANSWER_MESSAGE = 'Assistenten gav inget svar. Försök igen.' + /** * POST /api/agent/ask: a single-call, provider-agnostic assistant answer over a * bounded read-only tool loop. @@ -108,6 +121,14 @@ export async function POST(request: Request): Promise { }) return NextResponse.json({ data: result }) } catch (err) { + if (err instanceof EmptyModelAnswerError) { + // Already logged with model + usage by ask-service. + return NextResponse.json( + { error: EMPTY_ANSWER_MESSAGE, code: 'empty_model_answer' }, + { status: 502 }, + ) + } + log.error('assistant ask failed', err, { companyId, persist: false }) return NextResponse.json({ error: getUserErrorMessage(err) }, { status: 500 }) } } @@ -143,10 +164,21 @@ export async function POST(request: Request): Promise { tier: parsed.data.tier, }) + // answerAssistantQuestion throws EmptyModelAnswerError on an empty answer, + // so an empty assistant turn is never persisted: the thread keeps the + // question (retryable) but records no blank reply. await persistAssistantTurn(supabase, conversationId, result.answer) return NextResponse.json({ data: { ...result, conversation_id: conversationId } }) } catch (err) { + if (err instanceof EmptyModelAnswerError) { + // Already logged with model + usage by ask-service. + return NextResponse.json( + { error: EMPTY_ANSWER_MESSAGE, code: 'empty_model_answer' }, + { status: 502 }, + ) + } + log.error('assistant ask failed', err, { companyId, persist: true }) return NextResponse.json({ error: getUserErrorMessage(err) }, { status: 500 }) } } diff --git a/app/api/agent/invoke/route.ts b/app/api/agent/invoke/route.ts index fcdbfb05..0941255e 100644 --- a/app/api/agent/invoke/route.ts +++ b/app/api/agent/invoke/route.ts @@ -17,6 +17,13 @@ import { getErrorMessage as getUserErrorMessage } from '@/lib/errors/get-error-m // agent tool registry which is populated by the mcp-server extension at load. ensureInitialized() +// Deep turns (thinking at high/xhigh effort, up to 12 tool iterations, a +// 15-27k-token prompt prefix) can outlive the plan default duration cap; a +// function killed mid-stream closes the NDJSON stream cleanly, which the chat +// used to render as silent success. 300 s matches the other long-running model +// surfaces (app/api/receipt-hunt/run, app/api/agent/ask). +export const maxDuration = 300 + // Hard cap on the per-turn user input. Generous for a chat composer (about // 5k words / 20 pages) but bounds Bedrock token cost if the rate limiter is // ever fail-open and a client floods large payloads. diff --git a/components/agent/AgentChat.tsx b/components/agent/AgentChat.tsx index ba39d095..9b62ff08 100644 --- a/components/agent/AgentChat.tsx +++ b/components/agent/AgentChat.tsx @@ -458,6 +458,11 @@ export default function AgentChat({ const reader = response.body.getReader() const decoder = new TextDecoder() let buffer = '' + // Whether the stream delivered a terminal event (turn_complete or error). + // A serverless function killed at its duration cap closes the stream + // cleanly (done: true, nothing thrown), which used to look identical to + // success: "Tänker" collapsed with no answer and no error. + let sawTerminalEvent = false try { while (true) { const { done, value } = await reader.read() @@ -480,6 +485,9 @@ export default function AgentChat({ // First user-visible event lazily mounts the bubble. `conversation` // is a metadata event with no visible payload so it does not. const ev = parsed as { kind?: string } | null + if (ev && (ev.kind === 'turn_complete' || ev.kind === 'error')) { + sawTerminalEvent = true + } if ( ev && typeof ev.kind === 'string' && @@ -491,6 +499,12 @@ export default function AgentChat({ handleEvent(parsed) } } + // The stream ended without throwing. If no terminal event arrived and + // the user did not abort, the function was cut off mid-turn (duration + // cap, proxy drop): say so instead of presenting silence as success. + if (!sawTerminalEvent && !signal.aborted) { + setErrorMessage('Anslutningen bröts innan svaret blev klart. Försök igen.') + } } catch (err) { if (!signal.aborted) { setErrorMessage(err instanceof Error ? getUserErrorMessage(err) : 'Streamen avbröts.') diff --git a/components/agent/AskConsole.tsx b/components/agent/AskConsole.tsx index bfb95790..e960a0b4 100644 --- a/components/agent/AskConsole.tsx +++ b/components/agent/AskConsole.tsx @@ -185,6 +185,14 @@ export default function AskConsole({ conversationIdRef.current = convId onConversationCreated?.(convId) } + if (answer.trim().length === 0) { + // Belt and braces with the server's 502 guard: a 200 whose answer is + // empty must not append an invisible bubble ("Tänker" collapses and + // nothing appears). Same message the server sends on 502; the thread + // id (if one was created) is kept above so a retry lands in it. + setError('Assistenten gav inget svar. Försök igen.') + return + } setMessages((prev) => [...prev, { role: 'assistant', text: answer }]) } catch { setError('Kunde inte nå assistenten. Kontrollera anslutningen och försök igen.') diff --git a/lib/agent/ask/__tests__/ask-service.test.ts b/lib/agent/ask/__tests__/ask-service.test.ts index c33cd4e8..8e099db6 100644 --- a/lib/agent/ask/__tests__/ask-service.test.ts +++ b/lib/agent/ask/__tests__/ask-service.test.ts @@ -16,6 +16,7 @@ vi.mock('../snapshot', () => ({ })) import { answerAssistantQuestion } from '../ask-service' +import { EmptyModelAnswerError } from '../errors' function supabaseWith(company: { name?: string; entity_type?: string } | null): SupabaseClient { const chain = { @@ -116,6 +117,22 @@ describe('answerAssistantQuestion', () => { expect(call.prompt).toContain('Status: momsregistrerad (momsperiod: quarterly).') }) + it('defaults maxTokens to 5400 (the streaming chat reply ceiling; 1500 caused empty max_tokens answers)', async () => { + await answerAssistantQuestion({ supabase: supabaseWith(null), companyId: 'c1', question: 'Hej?' }) + expect(generateText.mock.calls[0][0].maxTokens).toBe(5400) + }) + + it('rejects with the typed empty-answer error when the model returns no visible text', async () => { + generateText.mockResolvedValue({ text: ' \n', model: 'claude-sonnet-5', usage: {} }) + const promise = answerAssistantQuestion({ + supabase: supabaseWith(null), + companyId: 'c1', + question: 'Hej?', + }) + await expect(promise).rejects.toBeInstanceOf(EmptyModelAnswerError) + await expect(promise).rejects.toMatchObject({ code: 'empty_model_answer' }) + }) + it('honours a custom maxSteps', async () => { buildLedgerTools.mockReturnValue([ { name: 'gnubok_get_vat_report', description: 'd', jsonSchema: {}, execute: vi.fn() }, diff --git a/lib/agent/ask/ask-service.ts b/lib/agent/ask/ask-service.ts index 57a55c3c..d0f57f76 100644 --- a/lib/agent/ask/ask-service.ts +++ b/lib/agent/ask/ask-service.ts @@ -1,8 +1,12 @@ import type { SupabaseClient } from '@supabase/supabase-js' import { getAiService, type AiTier, type AiToolDef } from '@/lib/ai' +import { createLogger } from '@/lib/logger' +import { EmptyModelAnswerError } from './errors' import { buildLedgerTools } from './ledger-tools' import { buildAssistantSnapshot } from './snapshot' +const log = createLogger('agent.ask') + /** * Provider-agnostic assistant answer over a bounded, read-only tool loop. * @@ -57,7 +61,13 @@ export interface AskResult { model: string } -const DEFAULT_MAX_TOKENS = 1500 +// Matches MAX_TOKENS_NO_THINKING in lib/agent/composer/client.ts, the +// streaming chat's reply-sized ceiling on the same model. The original 1500 +// cap was tighter than every other assistant surface and made stop_reason +// max_tokens routine on tool-loop turns: the whole budget could be spent +// before the first visible text block, so the model came back with no text at +// all ("Tänker", then silence). +const DEFAULT_MAX_TOKENS = 5400 const DEFAULT_MAX_STEPS = 5 const MAX_QUESTION_CHARS = 4000 const MAX_CONTEXT_CHARS = 24_000 @@ -137,5 +147,21 @@ export async function answerAssistantQuestion(req: AskRequest): Promise 0 ? { tools, maxSteps: req.maxSteps ?? DEFAULT_MAX_STEPS } : {}), }) + + // An empty answer is a failure, not a result. Passing it through is exactly + // the silent-stop bug: the route would 200 and the console would append an + // invisible bubble. Log loudly (model + usage tell us whether the budget was + // spent on tool churn) and fail typed so the route can answer 502. + if (result.text.trim().length === 0) { + log.error('model returned an empty answer', { + companyId: req.companyId, + model: result.model, + tier: req.tier ?? 'assistant', + usage: result.usage, + toolCount: tools.length, + }) + throw new EmptyModelAnswerError(result.model) + } + return { answer: result.text, model: result.model } } diff --git a/lib/agent/ask/errors.ts b/lib/agent/ask/errors.ts new file mode 100644 index 00000000..e863b020 --- /dev/null +++ b/lib/agent/ask/errors.ts @@ -0,0 +1,18 @@ +/** + * Typed failure for "the model returned no visible text": stop_reason + * max_tokens with the whole budget spent before the first text block, or a + * refusal with an empty body. Before this existed, an empty answer flowed + * through /api/agent/ask as a 200 and rendered as an invisible bubble + * ("Tänker", then silence). + * + * Lives in its own module so the API route and its tests can match on it + * without importing ask-service's full dependency graph. + */ +export class EmptyModelAnswerError extends Error { + readonly code = 'empty_model_answer' + + constructor(model: string) { + super(`Model ${model} returned an empty answer`) + this.name = 'EmptyModelAnswerError' + } +} diff --git a/lib/agent/chat/__tests__/run-turn-empty-max-tokens.test.ts b/lib/agent/chat/__tests__/run-turn-empty-max-tokens.test.ts new file mode 100644 index 00000000..82422be0 --- /dev/null +++ b/lib/agent/chat/__tests__/run-turn-empty-max-tokens.test.ts @@ -0,0 +1,153 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import type { AgentIntent } from '@/lib/agent/intents/types' +import type { StreamEvent } from '../run-turn' + +// Verifies the silent-stop guard: a turn whose last model call ended on +// stop_reason max_tokens WITHOUT any visible text must emit an error event, +// never a bare turn_complete (which the chat renders as an invisible empty +// bubble: "Tänker" collapses and nothing appears). A partial answer that hit +// the ceiling still completes: half an answer on screen is a real answer. +// +// The client mock extends run-turn-thinking.test.ts's: stream().on() records +// handlers and finalMessage() replays the queued response's text blocks +// through the 'text' handler so assistantText accumulates like production. +const messagesCreate = vi.fn() +vi.mock('@/lib/agent/composer/client', () => ({ + getAnthropic: () => ({ + messages: { + stream: (args: unknown) => { + const handlers: Record void> = {} + const stream = { + on: (name: string, fn: (arg: unknown) => void) => { + handlers[name] = fn + return stream + }, + finalMessage: async () => { + const resp = (await messagesCreate(args)) as { + content?: { type: string; text?: string }[] + } + for (const block of resp.content ?? []) { + if (block.type === 'text' && typeof block.text === 'string') { + handlers.text?.(block.text) + } + } + return resp + }, + } + return stream + }, + }, + }), + SONNET_MODEL: 'claude-sonnet-5', + MAX_TOKENS_NO_THINKING: 5400, + MAX_TOKENS_STANDARD: 16000, + MAX_TOKENS_DEEP: 24000, +})) + +vi.mock('../system-prompt', () => ({ + buildSystemPrompt: vi.fn().mockResolvedValue({ + blocks: [], + promptHash: 'sha256:test', + atomsLoaded: [], + }), +})) + +const getManyMock = vi.fn() +vi.mock('@/lib/agent/tools/registry', () => ({ + agentToolRegistry: { + get: () => undefined, + getMany: (...args: unknown[]) => getManyMock(...args), + }, +})) + +import { runChatTurn } from '../run-turn' + +function fakeSupabase() { + const passthrough: Record = {} + const proxy: unknown = new Proxy(passthrough, { + get(_t, prop) { + if (prop === 'then') { + return (resolve: (v: unknown) => void) => resolve({ data: null, error: null }) + } + return () => proxy + }, + }) + return proxy as unknown as Parameters[0]['supabase'] +} + +function baseIntent(): AgentIntent { + return { + id: 'general.help', + buttonLabel: 'x', + sheetTitle: 'x', + atoms: { mode: 'progressive', horizontal: [], includeCompanyVertical: false, includeCompanyModifiers: false }, + tools: [], + model: 'claude-sonnet-4-6', + capture: async () => ({}), + promptTemplate: () => '', + } +} + +async function runAndCollect(): Promise { + const events: StreamEvent[] = [] + getManyMock.mockResolvedValue([]) + await runChatTurn({ + supabase: fakeSupabase(), + userId: 'u', + companyId: 'c', + companyName: 'X', + firstName: 'A', + intent: baseIntent(), + conversationId: 'conv', + userMessage: 'hej', + persist: false, + emit: (event) => { + events.push(event) + return true + }, + }) + return events +} + +beforeEach(() => { + vi.clearAllMocks() +}) + +describe('runChatTurn: max_tokens with no visible text', () => { + it('emits an error event instead of a bare turn_complete', async () => { + messagesCreate.mockResolvedValueOnce({ + // Thinking spent the whole budget: no text block at all. + content: [{ type: 'thinking', thinking: 'long reasoning', signature: 's' }], + stop_reason: 'max_tokens', + }) + const events = await runAndCollect() + expect(events.some((e) => e.kind === 'turn_complete')).toBe(false) + const error = events.find((e) => e.kind === 'error') + expect(error).toBeDefined() + expect((error as { message: string }).message).toBe( + 'Assistenten fick slut på utrymme innan svaret blev klart. Försök igen.', + ) + }) + + it('still completes when max_tokens cut a PARTIAL answer', async () => { + messagesCreate.mockResolvedValueOnce({ + content: [{ type: 'text', text: 'Din moms för juli är' }], + stop_reason: 'max_tokens', + }) + const events = await runAndCollect() + expect(events.some((e) => e.kind === 'error')).toBe(false) + const complete = events.find((e) => e.kind === 'turn_complete') + expect(complete).toBeDefined() + expect((complete as { assistant_text: string }).assistant_text).toBe('Din moms för juli är') + }) + + it('completes normally on end_turn', async () => { + messagesCreate.mockResolvedValueOnce({ + content: [{ type: 'text', text: 'Klart svar.' }], + stop_reason: 'end_turn', + }) + const events = await runAndCollect() + expect(events.some((e) => e.kind === 'error')).toBe(false) + expect(events.some((e) => e.kind === 'turn_complete')).toBe(true) + }) +}) diff --git a/lib/agent/chat/run-turn.ts b/lib/agent/chat/run-turn.ts index 16b9684e..d5172f20 100644 --- a/lib/agent/chat/run-turn.ts +++ b/lib/agent/chat/run-turn.ts @@ -310,6 +310,9 @@ export async function runChatTurn(args: RunTurnArgs): Promise { let assistantText = '' let iterations = 0 + // stop_reason of the last completed model call: lets the post-loop check + // tell "finished" from "ran out of output budget before any visible text". + let lastStopReason: string | null = null // One automatic retry per TURN when the Bedrock stream dies on a transient // error (throttling, 5xx, transport cut, stream corruption). The failed // attempt persisted nothing (persist happens after finalMessage() succeeds), @@ -482,6 +485,7 @@ export async function runChatTurn(args: RunTurnArgs): Promise { } const assistantContent: ContentBlock[] = response.content + lastStopReason = (response as { stop_reason?: string | null }).stop_reason ?? null // Persist the assistant turn (text + tool_use blocks). Thinking blocks are // stripped for storage but kept in `messages` below for the in-turn loop. @@ -654,6 +658,27 @@ export async function runChatTurn(args: RunTurnArgs): Promise { } } + // A turn that hit the output ceiling before producing any visible text + // (thinking or tool churn spent the whole max_tokens budget) is a failure, + // not a completion. A bare turn_complete here is the silent-stop bug: the + // client hides the empty bubble and "Tänker" just disappears. A PARTIAL + // answer that hit the ceiling still completes; only the empty case errors. + if (lastStopReason === 'max_tokens' && assistantText.trim().length === 0) { + log.error('model hit max_tokens with no visible text', { + conversationId, + companyId, + model, + iterations, + thinking: Boolean(intent.thinking), + maxTokens, + }) + emit({ + kind: 'error', + message: 'Assistenten fick slut på utrymme innan svaret blev klart. Försök igen.', + }) + return + } + emit({ kind: 'turn_complete', assistant_text: assistantText }) } diff --git a/lib/ai/__tests__/anthropic-family.test.ts b/lib/ai/__tests__/anthropic-family.test.ts index bb3113d0..f172ad53 100644 --- a/lib/ai/__tests__/anthropic-family.test.ts +++ b/lib/ai/__tests__/anthropic-family.test.ts @@ -262,7 +262,7 @@ describe('generateText read-only tool loop', () => { expect(result.text).toBe('Kunde inte hämta momsrapporten just nu.') }) - it('forces a final tools-off answer when the step budget is exhausted', async () => { + it('forces a final no-more-tools answer when the step budget is exhausted', async () => { const execute = vi.fn().mockResolvedValue({ ok: true }) const svc = createAnthropicFamilyService(readAiConfig()) // Every turn keeps asking for the tool → never terminates on its own. @@ -294,9 +294,60 @@ describe('generateText read-only tool loop', () => { tools: [{ name: 't', description: 'd', jsonSchema: {}, execute }], maxSteps: 2, }) - // 2 loop turns + 1 forced final = 3 calls; the final call omits tools. + // 2 loop turns + 1 forced final = 3 calls. Deliberate request-shape change + // (2026-08-24): the final call KEEPS the tools declared, because the + // transcript it replays holds tool_use/tool_result blocks the API must be + // able to resolve (omitting tools made every step-exhausted answer a 400), + // and forbids further calls via tool_choice none. expect(mockCreate).toHaveBeenCalledTimes(3) - expect(mockCreate.mock.calls[2][0].tools).toBeUndefined() + expect(mockCreate.mock.calls[2][0].tools).toHaveLength(1) + expect(mockCreate.mock.calls[2][0].tools[0].name).toBe('t') + expect(mockCreate.mock.calls[2][0].tool_choice).toEqual({ type: 'none' }) expect(result.text).toBe('Sammanfattning utan fler verktygsanrop.') }) + + it('bounds an oversized tool result before feeding it back to the model', async () => { + const big = 'x'.repeat(120_000) + const execute = vi.fn().mockResolvedValue({ text: big }) + const svc = createAnthropicFamilyService(readAiConfig()) + mockCreate.mockResolvedValueOnce({ + stop_reason: 'tool_use', + content: [{ type: 'tool_use', id: 'tu', name: 'gnubok_get_document_content', input: {} }], + usage: { input_tokens: 1, output_tokens: 1 }, + }) + mockCreate.mockResolvedValueOnce({ + stop_reason: 'end_turn', + content: [{ type: 'text', text: 'Sammanfattat.' }], + usage: { input_tokens: 1, output_tokens: 1 }, + }) + await svc.generateText({ + tier: 'assistant', + prompt: 'x', + maxTokens: 100, + tools: [{ name: 'gnubok_get_document_content', description: 'd', jsonSchema: {}, execute }], + }) + const toolResult = mockCreate.mock.calls[1][0].messages[2].content[0] + // 40k cap plus the short truncation notice; nowhere near the raw 120k. + expect(toolResult.content.length).toBeLessThan(40_400) + expect(toolResult.content).toContain('[avkortat: resultatet var') + }) + + it('returns empty text (for the caller to guard on) when the loop ends on max_tokens with no text block', async () => { + const svc = createAnthropicFamilyService(readAiConfig()) + mockCreate.mockResolvedValueOnce({ + stop_reason: 'max_tokens', + content: [], + usage: { input_tokens: 5, output_tokens: 100 }, + }) + const result = await svc.generateText({ + tier: 'assistant', + prompt: 'x', + maxTokens: 100, + tools: [{ name: 't', description: 'd', jsonSchema: {}, execute: vi.fn() }], + }) + // The service stays a transport: it does not invent text. Callers + // (ask-service) treat '' as a typed failure instead of a silent answer. + expect(result.text).toBe('') + expect(mockCreate).toHaveBeenCalledTimes(1) + }) }) diff --git a/lib/ai/services/anthropic-family.ts b/lib/ai/services/anthropic-family.ts index 389a76ee..0a32aaed 100644 --- a/lib/ai/services/anthropic-family.ts +++ b/lib/ai/services/anthropic-family.ts @@ -18,6 +18,20 @@ import type { const DEFAULT_MAX_STEPS = 4 +// Bound a serialized tool result before it enters the model context. Mirrors +// boundToolResultText in lib/agent/chat/run-turn.ts: same 40k-char ceiling +// (~10k tokens, well under the ~25k-token practical ceiling for a single tool +// return). Unbounded, one large read (full OCR text, a big ledger report) is +// re-sent on every later loop turn and can eat the whole max_tokens budget +// before the model produces any visible text. +const MAX_TOOL_RESULT_CHARS = 40_000 + +function boundToolResult(raw: string): string { + if (raw.length <= MAX_TOOL_RESULT_CHARS) return raw + const head = raw.slice(0, MAX_TOOL_RESULT_CHARS) + return `${head}\n\n[avkortat: resultatet var ${raw.length} tecken, visar de första ${MAX_TOOL_RESULT_CHARS}. Be om en smalare sökning (limit, datumintervall, specifikt id eller fält) för att se mer.]` +} + const EMPTY_USAGE: AiUsage = { inputTokens: 0, outputTokens: 0, @@ -173,16 +187,27 @@ export function createAnthropicFamilyService(cfg: ResolvedAiConfig): AiService { isError = true content = JSON.stringify({ error: err instanceof Error ? err.message : 'Tool failed' }) } - results.push({ type: 'tool_result', tool_use_id: block.id, content, is_error: isError }) + results.push({ + type: 'tool_result', + tool_use_id: block.id, + content: boundToolResult(content), + is_error: isError, + }) } messages.push({ role: 'user', content: results }) } - // Spent the step budget without a final answer: force one with tools off. + // Spent the step budget without a final answer: force one more turn that + // cannot call tools. The tools param MUST stay in the request: the + // transcript above holds tool_use/tool_result blocks, and the Messages + // API rejects a request that replays those without declaring the tools + // they refer to. tool_choice none is the sanctioned "answer in text" knob. const final = await getClient().messages.create({ model, max_tokens: req.maxTokens, ...(req.system ? { system: req.system } : {}), + tools, + tool_choice: { type: 'none' }, messages, }) return { text: textOf(final), model, usage: addUsage(usage, usageOf(final)) }