diff --git a/AGENTS.md b/AGENTS.md index a540b2ef..4b40ccfb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -285,10 +285,14 @@ and [src/runtime/llm-fallback-seam.test.ts](src/runtime/llm-fallback-seam.test.t cap or the context length *and* says it is too large) keeps `shouldAdvance` from falling the chain over to a link that may not be running; on the raised-cap retry the turn fails with the *original* truncation, and elsewhere the provider's own sentence about the limit stays in the message. -6. **A learned window is forgotten the moment the server disproves it.** A successful completion whose - prompt + reply exceed the believed window fires `onContextWindowExceeded`, and bootstrap drops the - observation (catalogue windows are never touched). Learned windows live for the process; a - restart may change the server. +6. **A learned window only moves towards what the server demonstrated.** A successful completion whose + prompt + reply exceed the believed window fires `onContextWindowExceeded`, and bootstrap raises the + learned window to that count (`LearnedContextWindows`; catalogue windows are never touched) — it + used to forget the observation, which sent the next prompt back to the nominal 128k. A request the + provider refuses as too large (`isRequestSizeRejection`, native-tool links) lowers it: the window + the body names, else 0.8 × the prompt estimate, is observed, the conversation is packed to it and + the step is retried once with a notice (`prompt_repacked`). Learned windows live for the process; + a restart may change the server. 7. **The repair pass on `native_tools` runs under the step's cap, not `REPAIR_MAX_TOKENS`.** The 1024-token cap is a grammar-link guard: the prefill strip keeps the think block short there. On the chat transport a reasoning model thinks server-side and 1024 is a guaranteed truncation, so every diff --git a/src/agent/agent-loop.test.ts b/src/agent/agent-loop.test.ts index f463830b..1fc50a68 100644 --- a/src/agent/agent-loop.test.ts +++ b/src/agent/agent-loop.test.ts @@ -9,6 +9,8 @@ import { osFsReadTool } from "../tools/os/fs-read.js"; import { SlotManager } from "../llm/slot-manager.js"; import { TransportError } from "../llm/reliability/llm-failures.js"; import { LlamaServerError } from "../llm/llama-server-client.js"; +import { OpenAiHttpError } from "../llm/provider/openai/openai-http.js"; +import { parseProviderErrorBody } from "../llm/provider/openai/parse-provider-error-body.js"; import { PARSE_RECOVERY_BUDGET } from "./parse-failure-recovery.js"; import { EMPTY_COMPLETION_RECOVERY_BUDGET } from "./empty-completion-recovery.js"; import { createEmptySessionState } from "../session/session-state.js"; @@ -817,7 +819,7 @@ describe("AgentLoop end-to-end with mock LLM", () => { expect(prompts[1]).toContain("cut off after 8192 tokens"); }); - it("forgets a learned window the server just proved too small", async () => { + it("reports a completion that exceeded the learned window, so bootstrap can raise it", async () => { const registry = buildDefaultToolRegistry(); const exceeded: number[] = []; const loop = new AgentLoop({ @@ -961,6 +963,127 @@ describe("AgentLoop end-to-end with mock LLM", () => { expect(calls).toBe(2); }); + it("learns the window a native-tool provider names in its 400, repacks and retries once (F30)", async () => { + const registry = buildDefaultToolRegistry(); + const observed: number[] = []; + const repacks: Array<{ contextWindow: number; source: string }> = []; + const prompts: string[] = []; + let learned: number | null = null; + let calls = 0; + const loop = new AgentLoop({ + registry, + slotManager: new SlotManager(2), + grammar: 'root ::= "ok"', + toolTransport: "native_tools", + toolCallAdapter: null, + llmComplete: async ({ prompt }) => { + calls += 1; + prompts.push(prompt); + if (calls === 1) { + throw new TransportError( + '"vendor" rejected the request (400).', + 400, + "https://x/v1", + { + cause: new OpenAiHttpError( + "openai provider 400: This model's maximum context length is 8192 tokens. However, you requested 9134 tokens (7134 in the messages, 2000 in the completion).", + 400, + "u", + ), + }, + ); + } + return makeNativeCompletion([ + { name: "reply", arguments: JSON.stringify({ text: "fits now" }) }, + ]); + }, + toolDescriptors: TOOLS, + capabilities: CAPS, + skillCatalog: SKILLS, + contextWindow: () => learned, + onContextWindowObserved: (contextWindow) => { + observed.push(contextWindow); + learned = contextWindow; + }, + onEvent: (event) => { + if (event.type === "prompt_repacked") repacks.push(event); + }, + }); + const result = await loop.runTurn( + createEmptySessionState({ id: "s-size-400", workingDir }), + { + userMessage: "keep going", + maxSteps: 5, + taskMaxSteps: 5, + signal: new AbortController().signal, + }, + ); + expect(result.reason).toBe("reply"); + expect(calls).toBe(2); + expect(observed).toEqual([8_192]); + expect(repacks).toEqual([ + expect.objectContaining({ contextWindow: 8_192, source: "provider", stepIndex: 0 }), + ]); + expect(prompts[1]).toContain("trimmed to fit this model's window"); + // The same step, not a new one. + expect(result.session.stepCount).toBe(1); + }); + + it("packs to most of the prompt estimate when the 413 names no window, and fails on a second refusal (F30)", async () => { + const registry = buildDefaultToolRegistry(); + const observed: number[] = []; + const failures: string[] = []; + let promptTokens = 0; + let calls = 0; + const loop = new AgentLoop({ + registry, + slotManager: new SlotManager(2), + grammar: 'root ::= "ok"', + toolTransport: "native_tools", + toolCallAdapter: null, + llmComplete: async () => { + calls += 1; + throw new TransportError( + '"vendor" rejected the request (413).', + 413, + "https://x/v1", + { + cause: new OpenAiHttpError( + "openai provider 413: the request exceeds the available context size", + 413, + "u", + ), + }, + ); + }, + toolDescriptors: TOOLS, + capabilities: CAPS, + skillCatalog: SKILLS, + contextWindow: () => null, + onContextWindowObserved: (contextWindow) => observed.push(contextWindow), + onEvent: (event) => { + if (event.type === "llm_event" && event.event.type === "prompt_built") { + promptTokens = event.event.prompt.tokens.total; + } + if (event.type === "loop_failed") failures.push(event.error.message); + }, + }); + const result = await loop.runTurn( + createEmptySessionState({ id: "s-size-413", workingDir }), + { + userMessage: "keep going", + maxSteps: 5, + taskMaxSteps: 5, + signal: new AbortController().signal, + }, + ); + expect(result.reason).toBe("failed"); + expect(calls).toBe(2); + expect(promptTokens).toBeGreaterThan(0); + expect(observed).toEqual([Math.floor(promptTokens * 0.8)]); + expect(failures[0]).toContain("rejected the request (413)"); + }); + it("fails with the truncation, not the 400, when the provider refuses the raised cap", async () => { const registry = buildDefaultToolRegistry(); const failures: string[] = []; @@ -1093,6 +1216,154 @@ describe("AgentLoop end-to-end with mock LLM", () => { expect(waits).toHaveLength(1); }); + it("stops the turn resumable when the provider's body says the credit is exhausted (F29)", async () => { + // The Codex attempt: a 429 carrying `credit_balance_exhausted` was + // parked and retried as rate limiting, 42 times per worker. + const registry = buildDefaultToolRegistry(); + const events: string[] = []; + let calls = 0; + const body = JSON.stringify({ + error: { + message: "Provider returned error", + code: 429, + metadata: { + raw: '{"error":{"type":"credit_balance_exhausted","message":"Your credit balance is too low"}}', + }, + }, + }); + const loop = new AgentLoop({ + registry, + slotManager: new SlotManager(2), + grammar: 'root ::= "ok"', + llmComplete: async () => { + calls += 1; + throw new TransportError( + '"openrouter" is rate-limiting this key (429).', + 429, + "https://openrouter.ai/api/v1", + { + cause: new OpenAiHttpError( + `openai provider 429: ${body}`, + 429, + "https://openrouter.ai/api/v1/chat/completions", + false, + null, + "openrouter", + undefined, + { body: parseProviderErrorBody(body) }, + ), + }, + ); + }, + toolDescriptors: TOOLS, + capabilities: CAPS, + skillCatalog: SKILLS, + onEvent: (event) => { + if ( + event.type === "provider_waiting" || + event.type === "credit_exhausted" || + event.type === "loop_failed" || + event.type === "loop_completed" + ) { + events.push(event.type); + } + }, + }); + const started = Date.now(); + const result = await loop.runTurn( + createEmptySessionState({ id: "s-credit", workingDir }), + { + userMessage: "keep going", + maxSteps: 5, + taskMaxSteps: 5, + signal: new AbortController().signal, + }, + ); + // One request, no park, no failure: paused where it stood. + expect(calls).toBe(1); + expect(Date.now() - started).toBeLessThan(1_000); + expect(events).toEqual(["credit_exhausted", "loop_completed"]); + expect(result.reason).toBe("max_steps"); + expect(result.stopCause).toBe("credit_exhausted"); + expect(result.session.status).toBe("stalled"); + expect(result.session.lastError).toBe( + 'task_stopped:credit_exhausted: "openrouter" is out of credit after 0 steps', + ); + const last = result.session.turns.at(-1); + expect(last?.kind).toBe("assistant_reply"); + expect((last as { text: string }).text).toContain( + '"openrouter" reports the account is out of credit', + ); + expect((last as { text: string }).text).toContain("say `continue`"); + }); + + it("waits as long as the provider asked, on a 402 the outage wait would otherwise refuse (F29)", async () => { + // OpenRouter's `in_flight_budget_exhausted` with a retry hint ended + // a cloud-only run at 2m19s as final. The hint is honoured instead. + const registry = buildDefaultToolRegistry(); + const waits: Array<{ nextRetryMs: number }> = []; + let calls = 0; + const body = JSON.stringify({ + error: { + code: "in_flight_budget_exhausted", + message: "Too many requests in flight for your balance; retry in 1 s", + }, + }); + const loop = new AgentLoop({ + registry, + slotManager: new SlotManager(2), + grammar: 'root ::= "ok"', + llmComplete: async () => { + calls += 1; + if (calls === 1) { + throw new TransportError( + '"openrouter" refused the request for lack of credit (402).', + 402, + "https://openrouter.ai/api/v1", + { + cause: new OpenAiHttpError( + `openai provider 402: ${body}`, + 402, + "https://openrouter.ai/api/v1/chat/completions", + false, + null, + "openrouter", + undefined, + { body: parseProviderErrorBody(body) }, + ), + }, + ); + } + return makeCompletion( + JSON.stringify({ tool: "reply", args: { text: "budget freed" } }), + ); + }, + toolDescriptors: TOOLS, + capabilities: CAPS, + skillCatalog: SKILLS, + onEvent: (event) => { + if (event.type === "provider_waiting") waits.push(event); + }, + }); + const started = Date.now(); + const result = await loop.runTurn( + createEmptySessionState({ id: "s-inflight", workingDir }), + { + userMessage: "busy balance", + maxSteps: 5, + taskMaxSteps: 5, + signal: new AbortController().signal, + }, + ); + expect(result.reason).toBe("reply"); + expect(calls).toBe(2); + expect(waits).toHaveLength(1); + // The hint (1 s), not the 2 s backoff. + expect(waits[0]!.nextRetryMs).toBe(1_000); + expect(Date.now() - started).toBeGreaterThanOrEqual(950); + expect(Date.now() - started).toBeLessThan(1_900); + }); + it("gives up after the wait budget and fails the turn once", async () => { const registry = buildDefaultToolRegistry(); const waits: number[] = []; diff --git a/src/agent/agent-loop.ts b/src/agent/agent-loop.ts index 03bc33df..177ab9e1 100644 --- a/src/agent/agent-loop.ts +++ b/src/agent/agent-loop.ts @@ -27,6 +27,11 @@ import { classifyFailure, isRequestSizeRejection, } from "../llm/index.js"; +import { readProviderErrorVerdict } from "../llm/reliability/provider-error-verdict.js"; +import { + composeSizeRejectionNotice, + planSizeRejectionRepack, +} from "./size-rejection-recovery.js"; import type { LlmFailureCategory, TruncationCause, @@ -481,14 +486,39 @@ async function abortableSleep(ms: number, signal: AbortSignal): Promise { }); } +/** + * Why a task stopped without the model closing it. The three ceilings + * are the loop's own; `credit_exhausted` is the provider's — the account + * cannot pay for the next request, so the turn parks where it is and + * resumes after a top-up, the same way as after a ceiling. + */ +export type TaskStopCause = + | "step_ceiling" + | "time_ceiling" + | "no_progress" + | "credit_exhausted"; + export function formatTaskStoppedReply(input: { - cause: "step_ceiling" | "time_ceiling" | "no_progress"; + cause: TaskStopCause; stepsTaken: number; stepCeiling: number; elapsedMs: number; + /** For `credit_exhausted`: who said so, and what they said. */ + credit?: { provider: string; detail: string }; }): string { const minutes = Math.max(1, Math.round(input.elapsedMs / 60_000)); const spent = `${input.stepsTaken} steps over ~${minutes} min`; + if (input.cause === "credit_exhausted") { + const who = input.credit?.provider ?? "the provider"; + const said = + input.credit?.detail !== undefined && input.credit.detail.length > 0 + ? ` (${input.credit.detail})` + : ""; + return ( + `(paused: "${who}" reports the account is out of credit${said}, after ${spent}.) ` + + "Here is where I got to — the work so far is kept in this session. Top up the account, then say `continue` to pick up from here." + ); + } const head = input.cause === "time_ceiling" ? `(paused: this task hit its time limit after ${spent}.)` @@ -632,6 +662,35 @@ export type AgentLoopEvent = type: "provider_recovered"; waitedMs: number; } + | { + /** + * The provider's error body says the account cannot pay + * (`credit_balance_exhausted`, `insufficient_credits`, a 402 + * naming credit). The turn stops where it is, resumable after a + * top-up — `loop_completed` follows with `max_steps` and the + * session records `task_stopped:credit_exhausted`. `provider` is + * the link that said so. + */ + type: "credit_exhausted"; + provider: string; + code: string; + message: string; + } + | { + /** + * The provider refused the request for step `stepIndex` as too + * large for its context window; the window was learned + * (`source: "provider"` from the body's own number, `"estimate"` + * from the prompt estimate) and the same step is being retried + * with the conversation packed to it. Fired once per step; a + * second refusal fails the turn with the provider's sentence. + */ + type: "prompt_repacked"; + stepIndex: number; + contextWindow: number; + source: "provider" | "estimate"; + promptTokens: number; + } | { /** * The completion for step `stepIndex` came back cut off, and the @@ -820,7 +879,7 @@ export interface RunTurnResult { * `max_steps` rather than `ok` for a worker that ran out of steps and * said so in its reply. */ - stopCause?: "step_ceiling" | "time_ceiling" | "no_progress"; + stopCause?: TaskStopCause; /** * Steering messages that were pushed but never reached a step — the * turn ended (or was cancelled) before the loop could drain them. @@ -1028,8 +1087,9 @@ export class AgentLoop { * and "made no progress for a whole leg" are different things to * tell someone, and the old single `max_steps` string said neither. */ - let stopCause: "step_ceiling" | "time_ceiling" | "no_progress" = - "step_ceiling"; + let stopCause: TaskStopCause = "step_ceiling"; + /** Set with `stopCause = "credit_exhausted"`: who refused, and what they said. */ + let creditStop: { provider: string; detail: string } | null = null; /** * The model's `reply` / `finish` came on the forced finalization * step, so a ceiling ended the task even though the model closed it. @@ -1069,6 +1129,10 @@ export class AgentLoop { /** The truncation that started the retry, for the message if the retry is refused. */ original: Error; } | null; + /** The step index already retried after a request-size refusal. */ + let sizeRepackRetry: { stepIndex: number } | null = null; + /** The loop's own estimate of the last prompt built, for the repack fallback. */ + let lastPromptTokens = 0; /** * The step index whose leg boundary already ran. A retried step * (outage or truncation) re-enters the loop at the same index; the @@ -1265,6 +1329,8 @@ export class AgentLoop { const outOfTime = Date.now() - taskStartedAt >= durationCeilingMs; if (outOfTime) stopCause = "time_ceiling"; const finalizationStep = i === stepCeiling - 1 || outOfTime; + const effectiveTransport: ToolCallTransport = + pinnedSlice?.toolTransport ?? this.deps.toolTransport ?? "grammar"; const finalizationNotice = "This is the final allowed step. Do not call any non-terminal tool; " + "summarize the completed work with reply, or end the session with finish."; @@ -1358,10 +1424,7 @@ export class AgentLoop { ...(this.deps.liveWorkerSlots ? { liveWorkerSlots: this.deps.liveWorkerSlots } : {}), - toolTransport: - pinnedSlice?.toolTransport ?? - this.deps.toolTransport ?? - "grammar", + toolTransport: effectiveTransport, toolCallAdapter: pinnedSlice?.toolCallAdapter ?? this.deps.toolCallAdapter ?? null, supportsSlotAffinity: @@ -1391,6 +1454,9 @@ export class AgentLoop { : {}), onEvent: (event) => { this.deps.onEvent?.({ type: "llm_event", event }); + if (event.type === "prompt_built") { + lastPromptTokens = event.prompt.tokens.total; + } // Issue #407. Skipped on a fusion worker's throwaway // session: it renders the same store as the orchestrator, // which already warned, and would repeat it per worker. @@ -1940,21 +2006,106 @@ export class AgentLoop { if (repeatedEmptyAfterAnnouncedRetry) { runError = repeatedEmptyCompletionError(err); } + // The provider refused the request for its size. The window it + // named (or, failing that, most of the prompt just estimated) + // becomes the learned window, the conversation is packed to it, + // and the step runs again with a notice. Once per step: a second + // refusal ends the turn with the provider's own sentence. + const repack = cancelled + ? null + : planSizeRejectionRepack({ + error: err, + alreadyRetried: sizeRepackRetry?.stepIndex === i, + raisedCapRefused: + truncationRetry?.stepIndex === i && + truncationRetry.maxTokens !== undefined, + transport: effectiveTransport, + promptTokens: lastPromptTokens, + contextWindow: this.deps.contextWindow?.() ?? null, + canFitWindow: this.deps.onContextWindowObserved !== undefined, + }); + if (repack !== null) { + sizeRepackRetry = { stepIndex: i }; + this.deps.onContextWindowObserved?.(repack.contextWindow); + pendingNotice = composeSizeRejectionNotice(noticeForThisStep); + this.deps.onEvent?.({ + type: "prompt_repacked", + stepIndex: i, + contextWindow: repack.contextWindow, + source: repack.source, + promptTokens: lastPromptTokens, + }); + this.deps.logger?.warn( + "provider refused the request as too large; repacking to its window and retrying the step", + { + sessionId: state.id, + stepIndex: i, + contextWindow: repack.contextWindow, + source: repack.source, + promptTokens: lastPromptTokens, + rejection: runError.message, + }, + ); + runError = null; + i -= 1; + continue; + } + // What the provider's error body says, as opposed to its + // status: exhausted credit is neither an outage to wait out nor + // a request to fall over — nothing changes until someone tops + // up. The turn stops where it is, resumable, and the operator + // is told which provider refused. (A fallback link, when the + // chain has one, has already been tried by the time the error + // reaches here.) + const verdict = cancelled ? null : readProviderErrorVerdict(err); + if (verdict?.kind === "credit_exhausted") { + stopCause = "credit_exhausted"; + creditStop = { provider: verdict.provider, detail: verdict.detail }; + reason = "max_steps"; + this.deps.onEvent?.({ + type: "credit_exhausted", + provider: verdict.provider, + code: verdict.code, + message: verdict.detail, + }); + this.deps.logger?.warn( + "provider reports exhausted credit; pausing the task", + { + sessionId: state.id, + stepIndex: i, + provider: verdict.provider, + code: verdict.code, + error: verdict.detail, + }, + ); + runError = null; + break; + } // The provider is not answering. Park the turn instead of // killing it: nothing of this step has been committed (a // completion failure throws before any tool is dispatched — // tool failures come back as results, not throws), so retrying // the same index replays nothing and duplicates no side effect. + // A provider that asked for a cooldown (`retry-after`, "retry in + // 120 s", OpenRouter's `in_flight_budget_exhausted` — a 402 the + // outage predicate would otherwise refuse) is waited for as + // long as it asked, within the same budget. + const retryHint = + verdict?.kind === "retry_after" ? verdict : null; if ( category === "transport" && !cancelled && providerWaitCfg.enabled && - isWaitableOutage(err) && + (isWaitableOutage(err) || retryHint !== null) && outageWaitedMs < providerWaitCfg.maxWaitMs ) { const nextRetryMs = Math.min( - PROVIDER_WAIT_MAX_BACKOFF_MS, - PROVIDER_WAIT_BASE_MS * 2 ** outageAttempts, + retryHint !== null + ? Math.max(1, retryHint.delayMs) + : Math.min( + PROVIDER_WAIT_MAX_BACKOFF_MS, + PROVIDER_WAIT_BASE_MS * 2 ** outageAttempts, + ), // Never sleep past the budget: the last wait ends exactly at // it, so the operator's configured ceiling is the truth. Math.max(1, providerWaitCfg.maxWaitMs - outageWaitedMs), @@ -2120,6 +2271,7 @@ export class AgentLoop { stepsTaken, stepCeiling, elapsedMs: Date.now() - taskStartedAt, + ...(creditStop !== null ? { credit: creditStop } : {}), }); state = recordTurn(state, assistantReplyTurn(synthetic)); this.deps.onEvent?.({ @@ -2135,7 +2287,10 @@ export class AgentLoop { state = { ...state, status: "stalled", - lastError: `task_stopped:${stopCause}: ${stepsTaken} steps without reply`, + lastError: + creditStop !== null + ? `task_stopped:${stopCause}: "${creditStop.provider}" is out of credit after ${stepsTaken} steps` + : `task_stopped:${stopCause}: ${stepsTaken} steps without reply`, }; } } else if (reason === "reply") { diff --git a/src/agent/batch-executor.test.ts b/src/agent/batch-executor.test.ts index c2f93a7a..89c08400 100644 --- a/src/agent/batch-executor.test.ts +++ b/src/agent/batch-executor.test.ts @@ -320,6 +320,60 @@ describe("executeBatch", () => { expect(out.cancelled).toBe(false); }); + it("appends the received and expected keys to a thrown argument error (F33)", async () => { + const registry = new ToolRegistry(); + registry.register({ + name: "os.fs.read", + description: "r", + readonly: true, + run: async () => { + throw new Error("os.fs.read: `path` must be a non-empty string"); + }, + }); + const inputs = toBatchInputs([ + { tool: "os.fs.read", args: { patth: "secret-value.txt" } }, + ]); + const out = await executeBatch( + inputs, + registry, + ctx(new AbortController().signal), + ); + const result = out.results[0]!.compressed!; + expect(result.status).toBe("error"); + expect(result.summary).toContain( + "os.fs.read: `path` must be a non-empty string — received keys: patth; expected: path, maxBytes, offset, limit, lineNumbers; did you mean `path` instead of `patth`?", + ); + expect(result.summary).not.toContain("secret-value"); + expect(result.details.receivedKeys).toEqual(["patth"]); + expect(result.details.expectedKeys).toEqual([ + "path", + "maxBytes", + "offset", + "limit", + "lineNumbers", + ]); + }); + + it("leaves a thrown runtime error without a key report", async () => { + const registry = new ToolRegistry(); + registry.register({ + name: "os.fs.read", + description: "r", + readonly: true, + run: async () => { + throw new Error("ENOENT: no such file or directory, open 'a'"); + }, + }); + const out = await executeBatch( + toBatchInputs([{ tool: "os.fs.read", args: { path: "a" } }]), + registry, + ctx(new AbortController().signal), + ); + const result = out.results[0]!.compressed!; + expect(result.summary).toBe("ENOENT: no such file or directory, open 'a'"); + expect(result.details.receivedKeys).toBeUndefined(); + }); + it("preserves batch-index order in the returned slots", async () => { const registry = new ToolRegistry(); registry.register({ diff --git a/src/agent/batch-executor.ts b/src/agent/batch-executor.ts index 996ff3b1..e5e00845 100644 --- a/src/agent/batch-executor.ts +++ b/src/agent/batch-executor.ts @@ -11,6 +11,7 @@ import { } from "../compressor/result-compressor.js"; import type { ToolRegistry } from "../tools/tool-registry.js"; import type { ToolRole } from "../tools/tool-roles.js"; +import { describeArgumentError } from "../tools/argument-error-hint.js"; import { CancelledError } from "../llm/index.js"; import { isParallelWithinGroup, @@ -436,11 +437,28 @@ export async function executeBatch( ); } const cause = err instanceof Error ? err : new Error(String(err)); + // An argument error names the key the tool wanted; the model also + // needs the keys it actually sent (`patternes`, `"path"`) and the + // closest accepted one, or it retries the same call blind. Keys + // only — never values. + const hint = describeArgumentError({ + tool: input.call.tool, + args: input.call.args, + message: cause.message, + }); compressed = compressToolResult({ tool: input.call.tool, status: "error", - output: cause.message, - details: { errorName: cause.name }, + output: hint?.message ?? cause.message, + details: { + errorName: cause.name, + ...(hint !== null + ? { + receivedKeys: hint.receivedKeys, + expectedKeys: hint.expectedKeys, + } + : {}), + }, }); } const durationMs = Date.now() - startedAt; diff --git a/src/agent/size-rejection-recovery.test.ts b/src/agent/size-rejection-recovery.test.ts new file mode 100644 index 00000000..89eef601 --- /dev/null +++ b/src/agent/size-rejection-recovery.test.ts @@ -0,0 +1,125 @@ +import { describe, expect, it } from "vitest"; +import { OpenAiHttpError } from "../llm/provider/openai/openai-http.js"; +import { TransportError } from "../llm/reliability/llm-failures.js"; +import { + composeSizeRejectionNotice, + planSizeRejectionRepack, + SIZE_REJECTION_NOTICE, + SIZE_REJECTION_SHRINK, +} from "./size-rejection-recovery.js"; + +/** The error as the loop sees it: the executor's wrapper over the provider's 400. */ +function rejection(body: string, status = 400): TransportError { + return new TransportError( + `"vendor" rejected the request (${status}).`, + status, + "https://x/v1", + { + cause: new OpenAiHttpError(`openai provider ${status}: ${body}`, status, "u"), + }, + ); +} + +const OPENAI_CONTEXT = + "This model's maximum context length is 8192 tokens. However, you requested 9134 tokens (7134 in the messages, 2000 in the completion). Please reduce the length of the messages or completion."; +const OPENROUTER_CONTEXT = + '{"error":{"message":"This endpoint\'s maximum context length is 131072 tokens. However, you requested about 140210 tokens (130210 of text input, 10000 in the output). Please reduce the length of either one, or use the \\"middle-out\\" transform to compress your prompt automatically.","code":400}}'; +const CAP_ONLY = + "max_tokens is too large: 32768. This model supports at most 16384 completion tokens"; + +const base = { + alreadyRetried: false, + raisedCapRefused: false, + transport: "native_tools" as const, + promptTokens: 12_000, + contextWindow: null, + canFitWindow: true, +}; + +describe("planSizeRejectionRepack", () => { + it("learns the window the provider named", () => { + expect( + planSizeRejectionRepack({ ...base, error: rejection(OPENAI_CONTEXT) }), + ).toEqual({ contextWindow: 8_192, source: "provider" }); + expect( + planSizeRejectionRepack({ + ...base, + error: rejection(OPENROUTER_CONTEXT), + contextWindow: 200_000, + }), + ).toEqual({ contextWindow: 131_072, source: "provider" }); + }); + + it("falls back to most of the prompt estimate when the body names no number", () => { + expect( + planSizeRejectionRepack({ + ...base, + error: rejection("the request exceeds the available context size", 413), + }), + ).toEqual({ + contextWindow: Math.floor(12_000 * SIZE_REJECTION_SHRINK), + source: "estimate", + }); + }); + + it("uses the estimate when the named window is not below the believed one", () => { + // The catalogue says 8192 already; the server still refused, so the + // prompt estimate is what is off. + expect( + planSizeRejectionRepack({ + ...base, + error: rejection(OPENAI_CONTEXT), + contextWindow: 8_192, + promptTokens: 8_500, + }), + ).toEqual({ contextWindow: 6_800, source: "estimate" }); + }); + + it("plans nothing for a reply-cap refusal, which a smaller prompt cannot fix", () => { + expect(planSizeRejectionRepack({ ...base, error: rejection(CAP_ONLY) })).toBeNull(); + }); + + it("plans nothing when the refused request was the loop's own raised cap", () => { + expect( + planSizeRejectionRepack({ + ...base, + error: rejection(OPENAI_CONTEXT), + raisedCapRefused: true, + }), + ).toBeNull(); + }); + + it("plans nothing on a grammar link, a second refusal, or with nowhere to learn to", () => { + const error = rejection(OPENAI_CONTEXT); + expect(planSizeRejectionRepack({ ...base, error, transport: "grammar" })).toBeNull(); + expect(planSizeRejectionRepack({ ...base, error, alreadyRetried: true })).toBeNull(); + expect(planSizeRejectionRepack({ ...base, error, canFitWindow: false })).toBeNull(); + }); + + it("plans nothing when the estimate would be implausibly small or not below the belief", () => { + const error = rejection("context size exceeded"); + expect(planSizeRejectionRepack({ ...base, error, promptTokens: 0 })).toBeNull(); + expect(planSizeRejectionRepack({ ...base, error, promptTokens: 900 })).toBeNull(); + expect( + planSizeRejectionRepack({ ...base, error, promptTokens: 12_000, contextWindow: 9_000 }), + ).toBeNull(); + }); + + it("ignores errors that are not size rejections", () => { + expect( + planSizeRejectionRepack({ + ...base, + error: new TransportError("fetch failed", null, ""), + }), + ).toBeNull(); + }); +}); + +describe("composeSizeRejectionNotice", () => { + it("keeps the notice the step already carried, first", () => { + expect(composeSizeRejectionNotice(undefined)).toBe(SIZE_REJECTION_NOTICE); + expect(composeSizeRejectionNotice("loop warning")).toBe( + `loop warning\n\n${SIZE_REJECTION_NOTICE}`, + ); + }); +}); diff --git a/src/agent/size-rejection-recovery.ts b/src/agent/size-rejection-recovery.ts new file mode 100644 index 00000000..4b2146da --- /dev/null +++ b/src/agent/size-rejection-recovery.ts @@ -0,0 +1,99 @@ +import type { ToolCallTransport } from "../llm/provider/completion-types.js"; +import { + isRequestSizeRejection, + readContextLengthFromRejection, + requestSizeRejectionNamesContext, +} from "../llm/reliability/request-size-rejection.js"; + +/** + * How the agent loop recovers from a request the provider refused for + * its size (a 400/413 naming the context length). + * + * A model without a catalog entry is assumed to have 128K; the first + * step whose prompt outgrows the real window used to end the turn with + * the provider's sentence and nothing else. The sentence usually names + * the window ("maximum context length is 8192 tokens … you requested + * 9134") — so the loop learns it, packs the next prompt to it through + * the same learned-window path a mid-reply cut uses, and retries the + * step once with a notice. When the body names no number, the window + * is taken as `SIZE_REJECTION_SHRINK` of the prompt the loop just + * estimated — enough to fit, and the catalogue is corrected the moment + * the server proves the window larger. + * + * Native-tool providers only: a llama-server link reports its window + * through `/props`, and its size 400 means the prompt was mis-sized + * against a window the runtime already knows. + */ +export const SIZE_REJECTION_SHRINK = 0.8; + +/** Below this a "window" is a parsing accident, not a context length. */ +const MIN_PLAUSIBLE_WINDOW = 1_024; + +export interface SizeRejectionRepack { + /** The window to pack the retried prompt to. */ + contextWindow: number; + /** Where the number came from. */ + source: "provider" | "estimate"; +} + +export interface PlanSizeRejectionRepackInput { + /** The failure the step threw. Anything but a size rejection plans nothing. */ + error: unknown; + /** This step index was already retried for size; a second refusal ends the turn. */ + alreadyRetried: boolean; + /** The request was the loop's own raised-cap truncation retry — the cap, not the window, was refused. */ + raisedCapRefused: boolean; + /** The transport of the link that refused. */ + transport: ToolCallTransport; + /** The loop's estimate of the refused prompt, in tokens; `0` when unknown. */ + promptTokens: number; + /** The window the runtime believes in, when it knows one. */ + contextWindow: number | null; + /** Whether a learned window has anywhere to go (`onContextWindowObserved` wired). */ + canFitWindow: boolean; +} + +export function planSizeRejectionRepack( + input: PlanSizeRejectionRepackInput, +): SizeRejectionRepack | null { + if (input.alreadyRetried || input.raisedCapRefused || !input.canFitWindow) { + return null; + } + if (input.transport !== "native_tools") return null; + if (!isRequestSizeRejection(input.error)) return null; + // A refusal that names only the reply cap ("max_tokens is too large + // … supports at most 16384 completion tokens") is not a window + // problem; a smaller prompt changes nothing about it. + if (!requestSizeRejectionNamesContext(input.error)) return null; + const believed = input.contextWindow; + const named = readContextLengthFromRejection(input.error); + if ( + named !== null && + named >= MIN_PLAUSIBLE_WINDOW && + (believed === null || named < believed) + ) { + return { contextWindow: named, source: "provider" }; + } + const estimated = Math.floor(input.promptTokens * SIZE_REJECTION_SHRINK); + if (estimated < MIN_PLAUSIBLE_WINDOW) return null; + if (believed !== null && estimated >= believed) return null; + return { contextWindow: estimated, source: "estimate" }; +} + +/** The `### notice` the retried step carries. */ +export const SIZE_REJECTION_NOTICE = + "The model server rejected the previous request as too large for its context window, so older conversation history was trimmed to fit this model's window. " + + "Continue from the latest tool results, keep your reasoning brief, and emit the tool call now."; + +/** + * Fold the notice into whatever one-shot notice the step already + * carries (loop detector, steering). Existing text first. + */ +export function composeSizeRejectionNotice( + existing: string | undefined, +): string { + if (existing === undefined || existing.length === 0) { + return SIZE_REJECTION_NOTICE; + } + return `${existing}\n\n${SIZE_REJECTION_NOTICE}`; +} diff --git a/src/agent/step-executor.test.ts b/src/agent/step-executor.test.ts index d3b5dac7..1842ed9a 100644 --- a/src/agent/step-executor.test.ts +++ b/src/agent/step-executor.test.ts @@ -26,6 +26,10 @@ import { grammarToolNames, } from "../llm/grammar/build-grammar.js"; import { createEmptySessionState } from "../session/session-state.js"; +import { + PLAIN_INSTRUCT_PROFILE as PLAIN_PROFILE_F31, + QWEN_THINK_PROFILE as QWEN_PROFILE_F31, +} from "../llm/model-profile.js"; import { DEFAULT_TOOL_DESCRIPTORS } from "../prompt/tool-descriptors.js"; import { replyTool } from "../tools/conversation/reply.js"; import { resetConfigCache } from "../config/index.js"; @@ -5064,3 +5068,79 @@ describe("the turn's reasoning effort and output ceiling reach the request (F20) ]); }); }); + +describe("executeStep — server chat template parts (F31)", () => { + const grammarsDir = join(process.cwd(), "grammars"); + + function registryWithReply(): ToolRegistry { + const registry = new ToolRegistry(); + registry.register({ + name: "reply", + description: "reply", + readonly: true, + async run(args: Record) { + return compressToolResult({ + tool: "reply", + status: "ok", + output: String(args.text ?? ""), + }); + }, + }); + return registry; + } + + async function runWith(profile: typeof PLAIN_PROFILE_F31 | typeof QWEN_PROFILE_F31) { + const grammar = await buildGrammar(profile, grammarsDir); + const seen: Array> = []; + await executeStep( + { + session: createEmptySessionState({ id: "s-f31", workingDir: "/w" }), + toolDescriptors: DEFAULT_TOOL_DESCRIPTORS, + capabilities: CAPS, + skillCatalog: SKILLS, + stepIndex: 0, + signal: new AbortController().signal, + userMessage: "hello", + }, + { + registry: registryWithReply(), + slotManager: new SlotManager(2), + toolTransport: "grammar", + llmComplete: async (params) => { + seen.push(params as unknown as Record); + return { + content: JSON.stringify({ tool: "reply", args: { text: "hi" } }), + reasoningContent: "", + stop: true, + truncated: false, + timing: { promptMs: 1, predictedMs: 1, promptTokens: 20, predictedTokens: 5 }, + cacheHitTokens: 0, + slotId: 0, + modelId: "mock", + }; + }, + grammar, + profile, + }, + ); + return seen[0]!; + } + + it("hands a plain-instruct link the prefix and a framing-free tail as chat parts (auto)", async () => { + const params = await runWith(PLAIN_PROFILE_F31); + const chat = params.chat as { system: string; user: string; prefixHash: string }; + expect(chat).toBeDefined(); + expect(chat.system.startsWith("### system")).toBe(true); + expect(chat.user).toContain("### respond"); + expect(chat.prefixHash).toMatch(/^[0-9a-f]+$/); + // The raw text still travels for a link that cannot render. + expect(params.prompt).toBe(`${chat.system}\n${chat.user}`); + expect(params.grammar).toContain("root"); + }); + + it("keeps the hand-built framing and sends no chat parts for a qwen link (auto)", async () => { + const params = await runWith(QWEN_PROFILE_F31); + expect(params.chat).toBeUndefined(); + expect((params.prompt as string).trimEnd().endsWith("")).toBe(true); + }); +}); diff --git a/src/agent/step-executor.ts b/src/agent/step-executor.ts index 64bf29aa..1e299aa8 100644 --- a/src/agent/step-executor.ts +++ b/src/agent/step-executor.ts @@ -102,6 +102,11 @@ import { } from "../session/conversation-turn.js"; import type { ToolRegistry } from "../tools/tool-registry.js"; import { hashPrefix, type SlotManager } from "../llm/slot-manager.js"; +import { + NO_SERVER_TEMPLATE, + resolveServerTemplatePolicy, +} from "../llm/server-template-policy.js"; +import type { ChatPromptParts } from "../llm/provider/completion-types.js"; import { getReasoningTurnFraming, reasoningOpenEmittedByModel, @@ -149,6 +154,13 @@ export interface LlmStreamParams { * implementations memoize. */ grammarPrompt?: () => string; + /** + * The prompt as prefix + tail, for a grammar (llama-server) link that + * renders through the model's own chat template (F31). Set only when + * the primary is a grammar link and the server-template policy is on + * for its profile; the seam forwards it as `CompletionRequest.chat`. + */ + chat?: ChatPromptParts; grammar: string; slotId: number; sessionId: string; @@ -506,10 +518,17 @@ async function executeStepInner( ctx: StepContext, deps: StepDependencies, ): Promise { - const promptCarriesPrefill = promptCarriesReasoningPrefill( - deps.profile, - deps.toolTransport, - ); + // Whether this step's local prompt goes through the model's own chat + // template. The template supplies the turn markers and the reasoning + // prelude, so the prompt is built framing-free, like a chat-transport + // prompt (F31). + const serverTemplate = + deps.toolTransport === "native_tools" + ? NO_SERVER_TEMPLATE + : resolveServerTemplatePolicy(getConfig().localModels, deps.profile); + const promptCarriesPrefill = + !serverTemplate.useServerTemplate && + promptCarriesReasoningPrefill(deps.profile, deps.toolTransport); // The same catalog on every step, the final one included: `### tools` // is stable-prefix bytes, and a catalog narrowed to reply/finish for // the last step re-read the whole prompt on a cold slot. The final @@ -545,8 +564,11 @@ async function executeStepInner( : {}), // Chat providers apply their own template server-side; a literal // reasoning prefill there is at best echoed noise and at worst - // corrupted in transit (Ollama Cloud, ollama/ollama#17248). - suppressReasoningPrefill: deps.toolTransport === "native_tools", + // corrupted in transit (Ollama Cloud, ollama/ollama#17248). The + // same holds for a local link rendering through its own template. + suppressReasoningPrefill: + deps.toolTransport === "native_tools" || + serverTemplate.useServerTemplate, ...(deps.contextWindow !== undefined ? { contextWindow: deps.contextWindow } : {}), @@ -657,6 +679,18 @@ async function executeStepInner( signal: ctx.signal, }), ...(grammarPrompt ? { grammarPrompt } : {}), + ...(serverTemplate.useServerTemplate + ? { + chat: { + system: prompt.stablePrefix, + user: prompt.tail, + prefixHash: slot.prefixHash, + ...(serverTemplate.enableThinking !== undefined + ? { enableThinking: serverTemplate.enableThinking } + : {}), + }, + } + : {}), ...(ctx.maxTokens !== undefined ? { maxTokens: ctx.maxTokens } : {}), // The turn's own settings ride on every completion of the step; the // repair retry spreads `llmParams`, so they inherit without a second diff --git a/src/cli/trace-formatter.ts b/src/cli/trace-formatter.ts index fe62056e..1b221a9f 100644 --- a/src/cli/trace-formatter.ts +++ b/src/cli/trace-formatter.ts @@ -104,6 +104,8 @@ function formatTraceEvent(event: TraceEvent, raw: boolean): string { return `${head} waited=${Math.round(event.waitedMs / 1000)}s`; case "completion_truncated": return `${head} step=${event.stepIndex} cause=${event.cause} reply=${event.completionTokens} prompt=${event.promptTokens} cap=${event.requestedMaxTokens ?? "none"} retry=${event.retry}:${event.retryValue}`; + case "prompt_repacked": + return `${head} step=${event.stepIndex} window=${event.contextWindow} source=${event.source} prompt=${event.promptTokens}`; case "loop_detected": return `${head} step=${event.stepIndex} tool=${event.tool} count=${event.count}${ event.detector !== undefined ? ` detector=${event.detector}` : "" diff --git a/src/config/config-schema.test.ts b/src/config/config-schema.test.ts index 48372c90..2894a55e 100644 --- a/src/config/config-schema.test.ts +++ b/src/config/config-schema.test.ts @@ -2080,3 +2080,33 @@ describe("localModels.completionMaxTokens (config v60)", () => { ).toBe(8192); }); }); + +describe("localModels.useServerTemplate / thinking (F31)", () => { + it("defaults both to auto", () => { + const parsed = parseUserConfigFile({ version: USER_CONFIG_VERSION }); + expect(parsed.localModels.useServerTemplate).toBe("auto"); + expect(parsed.localModels.thinking).toBe("auto"); + expect(USER_CONFIG_DEFAULTS.localModels.useServerTemplate).toBe("auto"); + }); + + it("reads on/off and rejects anything else, naming the field", () => { + const set = parseUserConfigFile({ + version: USER_CONFIG_VERSION, + localModels: { useServerTemplate: "on", thinking: "off" }, + }); + expect(set.localModels.useServerTemplate).toBe("on"); + expect(set.localModels.thinking).toBe("off"); + expect(() => + parseUserConfigFile({ + version: USER_CONFIG_VERSION, + localModels: { thinking: "maybe" }, + }), + ).toThrow(/localModels\.thinking/); + }); + + it("fills an older file that lacks the fields", () => { + const parsed = parseUserConfigFile({ version: 60 }); + expect(parsed.localModels.useServerTemplate).toBe("auto"); + expect(parsed.localModels.thinking).toBe("auto"); + }); +}); diff --git a/src/config/config-schema.ts b/src/config/config-schema.ts index 4c74cbd4..6f91a0f4 100644 --- a/src/config/config-schema.ts +++ b/src/config/config-schema.ts @@ -233,6 +233,10 @@ export interface AtomicAgentConfig { defaultSlotId: number; /** `external` uses `url`; `managed` overrides runtime `url` to localhost + `managed.port`. */ mode: LocalLlmMode; + /** Mirrors `UserConfigFile.localModels.useServerTemplate`. */ + useServerTemplate: LocalTemplateSetting; + /** Mirrors `UserConfigFile.localModels.thinking`. */ + thinking: LocalTemplateSetting; managed: UserManagedLocalLlmConfig; /** * Memory-v2 phase 1B. Second managed daemon for `/embedding`. @@ -1284,6 +1288,24 @@ export type HttpApprovalMode = "never" | "writes" | "always"; export type LocalLlmMode = "external" | "managed"; +/** + * A three-way local-template switch: `auto` lets the runtime decide per + * model family, `on` / `off` force it. Used by + * `localModels.useServerTemplate` and `localModels.thinking`. + */ +export type LocalTemplateSetting = "auto" | "on" | "off"; + +export function parseLocalTemplateSetting( + raw: unknown, + field: string, +): LocalTemplateSetting { + if (raw === "auto" || raw === "on" || raw === "off") return raw; + throw new ConfigValidationError( + field, + `expected auto|on|off, got ${JSON.stringify(raw)}`, + ); +} + export interface UserManagedLocalLlmConfig { modelId: string | null; port: number; @@ -1449,6 +1471,23 @@ export interface UserConfigFile { * this file value (operator override). */ completionMaxTokens: number; + /** + * Render local prompts through the model's own chat template + * (llama-server `POST /apply-template`) instead of atag's hand-built + * framing. `auto` (default) uses the template for every family + * without a hand-built profile — everything but Gemma and Qwen — + * so Llama, GLM, Mistral and other GGUFs get their turn markers. + * `on` forces it for every model, `off` keeps the raw framing. + * The GBNF grammar applies either way. Added in config v66. + */ + useServerTemplate: LocalTemplateSetting; + /** + * The template's thinking switch (`chat_template_kwargs: + * {enable_thinking}`) on server-templated prompts, for families + * whose template reads it. `auto` (default) leaves the template's + * own default; `on` / `off` set it. Added in config v66. + */ + thinking: LocalTemplateSetting; managed: UserManagedLocalLlmConfig; /** * Memory-v2 phase 1B. Optional second managed daemon for @@ -2210,6 +2249,11 @@ export interface UserConfigFile { // `workerMaxOutputTokens` (unset by default: the per-step output cap for // worker completions). An older file parses with all three absent and // behaves as before, except that a cloud fan-out is now bounded at 4. +// v66: `localModels.useServerTemplate` and `localModels.thinking` +// (`auto|on|off`, both default `auto`) — render local prompts through +// the model's own chat template (llama-server `/apply-template`) for +// families without a hand-built profile, and set the template's +// thinking switch. Additive: an older file inherits `auto` for both. export const USER_CONFIG_VERSION = 66; /** @@ -2374,6 +2418,8 @@ export const USER_CONFIG_DEFAULTS: UserConfigFile = { url: "http://127.0.0.1:8080", mode: "external", completionMaxTokens: 8192, + useServerTemplate: "auto", + thinking: "auto", managed: { modelId: null, port: 19091, @@ -4345,6 +4391,15 @@ export function parseUserConfigFile(raw: unknown): UserConfigFile { USER_CONFIG_DEFAULTS.localModels.completionMaxTokens, "localModels.completionMaxTokens", ), + useServerTemplate: parseLocalTemplateSetting( + localModels.useServerTemplate ?? + USER_CONFIG_DEFAULTS.localModels.useServerTemplate, + "localModels.useServerTemplate", + ), + thinking: parseLocalTemplateSetting( + localModels.thinking ?? USER_CONFIG_DEFAULTS.localModels.thinking, + "localModels.thinking", + ), managed, embeddings: embeddingsDaemon, download, diff --git a/src/config/load-config.ts b/src/config/load-config.ts index be60c03b..58d78ac2 100644 --- a/src/config/load-config.ts +++ b/src/config/load-config.ts @@ -198,6 +198,8 @@ export function loadConfig(): AtomicAgentConfig { ENV_DEFAULTS.DEFAULT_SLOT_ID, ), mode: user.localModels.mode, + useServerTemplate: user.localModels.useServerTemplate, + thinking: user.localModels.thinking, managed: { ...user.localModels.managed }, embeddings: { ...user.localModels.embeddings }, download: { ...user.localModels.download }, diff --git a/src/llm/llama-server-client.test.ts b/src/llm/llama-server-client.test.ts index 0dd4e881..0950c1f6 100644 --- a/src/llm/llama-server-client.test.ts +++ b/src/llm/llama-server-client.test.ts @@ -1250,3 +1250,65 @@ describe("extractLlamaErrorDetail", () => { expect(out.endsWith("…")).toBe(true); }); }); + +describe("LlamaServerClient.applyTemplate (F31)", () => { + it("posts the messages and template kwargs to /apply-template and returns the prompt", async () => { + let captured: { url: string; body: Record } | null = null; + const client = new LlamaServerClient({ + baseUrl: "http://127.0.0.1:9999", + fetchImpl: createMockFetch(async (url, init) => { + captured = { + url, + body: JSON.parse(String(init.body)) as Record, + }; + return new Response( + JSON.stringify({ + prompt: "<|im_start|>system\nS<|im_end|>\n<|im_start|>user\nU<|im_end|>\n<|im_start|>assistant\n", + }), + { status: 200, headers: { "content-type": "application/json" } }, + ); + }), + }); + const prompt = await client.applyTemplate( + [ + { role: "system", content: "S" }, + { role: "user", content: "U" }, + ], + { enable_thinking: false }, + ); + expect(prompt).toContain("<|im_start|>assistant\n"); + expect(captured!.url).toBe("http://127.0.0.1:9999/apply-template"); + expect(captured!.body).toEqual({ + messages: [ + { role: "system", content: "S" }, + { role: "user", content: "U" }, + ], + chat_template_kwargs: { enable_thinking: false }, + }); + }); + + it("omits chat_template_kwargs when none are given, and types a missing endpoint", async () => { + let body: Record | null = null; + const ok = new LlamaServerClient({ + baseUrl: "http://127.0.0.1:9999", + fetchImpl: createMockFetch(async (_url, init) => { + body = JSON.parse(String(init.body)) as Record; + return new Response(JSON.stringify({ prompt: "p" }), { status: 200 }); + }), + }); + await ok.applyTemplate([{ role: "user", content: "U" }]); + expect(body).not.toHaveProperty("chat_template_kwargs"); + + const missing = new LlamaServerClient({ + baseUrl: "http://127.0.0.1:9999", + fetchImpl: createMockFetch( + async () => new Response("not found", { status: 404 }), + ), + }); + const err = await missing + .applyTemplate([{ role: "user", content: "U" }]) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(LlamaServerError); + expect((err as LlamaServerError).status).toBe(404); + }); +}); diff --git a/src/llm/llama-server-client.ts b/src/llm/llama-server-client.ts index b300917a..fe3dc621 100644 --- a/src/llm/llama-server-client.ts +++ b/src/llm/llama-server-client.ts @@ -290,6 +290,56 @@ export class LlamaServerClient { } } + /** + * Render `messages` through the server's chat template + * (`POST /apply-template`, present in the bundled build). Returns the + * rendered prompt text, generation prompt included. `chatTemplateKwargs` + * reaches the template as `chat_template_kwargs` (`enable_thinking`). + */ + async applyTemplate( + messages: ReadonlyArray<{ role: string; content: string }>, + chatTemplateKwargs?: Record, + ): Promise { + const config = getConfig(); + const base = this.baseUrlOverride ?? config.localModels.url; + const url = llamaEndpointUrl(base, "/apply-template"); + const controller = new AbortController(); + const timer = setTimeout(() => controller.abort(), this.requestTimeoutMs); + try { + const response = await this.fetchImpl(url, { + method: "POST", + headers: this.buildHeaders(false), + body: JSON.stringify({ + messages, + ...(chatTemplateKwargs !== undefined + ? { chat_template_kwargs: chatTemplateKwargs } + : {}), + }), + signal: controller.signal, + }); + if (!response.ok) { + throw await buildHttpError(response, url); + } + const json = (await response.json()) as { prompt?: unknown }; + if (typeof json.prompt !== "string") { + throw new LlamaServerError( + "apply-template answered without a prompt string", + response.status, + url, + ); + } + return json.prompt; + } catch (err) { + if (err instanceof LlamaServerError) throw err; + const message = err instanceof Error ? err.message : String(err); + throw new LlamaServerError(message, null, url, false, readErrnoCode(err), { + cause: err, + }); + } finally { + clearTimeout(timer); + } + } + async fetchProps(): Promise { const config = getConfig(); const base = this.baseUrlOverride ?? config.localModels.url; diff --git a/src/llm/model-profile.test.ts b/src/llm/model-profile.test.ts index ac70e2b9..5a62189a 100644 --- a/src/llm/model-profile.test.ts +++ b/src/llm/model-profile.test.ts @@ -46,6 +46,19 @@ describe("detectModelProfile", () => { expect(detectModelProfile(LLAMA3_PROPS)).toEqual(PLAIN_INSTRUCT_PROFILE); }); + it("marks a plain template that reads enable_thinking as switchable (F31)", () => { + const glmLike = { + model_alias: "glm-4-9b-chat", + chat_template: + "[gMASK]{% for m in messages %}<|{{ m['role'] }}|>\n{{ m['content'] }}{% endfor %}{% if enable_thinking is defined and not enable_thinking %}<|assistant|>\n{% endif %}", + }; + expect(detectModelProfile(glmLike)).toEqual({ + ...PLAIN_INSTRUCT_PROFILE, + supportsThinkingSwitch: true, + }); + expect(detectModelProfile(LLAMA3_PROPS).supportsThinkingSwitch).toBeUndefined(); + }); + it("detects gemma 4 think profile from channel tags", () => { expect(detectModelProfile(GEMMA4_PROPS)).toEqual(GEMMA4_THINK_PROFILE); }); @@ -71,7 +84,11 @@ describe("detectModelProfile", () => { ...NEMOTRON_PROPS, model_alias: "some-other-chatml-think-model", }), - ).toEqual(PLAIN_INSTRUCT_PROFILE); + ).toEqual({ + // Plain, but the template does read `enable_thinking` (F31). + ...PLAIN_INSTRUCT_PROFILE, + supportsThinkingSwitch: true, + }); }); // Pins branch ordering. This alias satisfies BOTH gates (it contains diff --git a/src/llm/model-profile.ts b/src/llm/model-profile.ts index 8441718d..f9b9d61c 100644 --- a/src/llm/model-profile.ts +++ b/src/llm/model-profile.ts @@ -24,6 +24,13 @@ interface BaseModelProfile { * expose it — prompt-building then relies purely on configured caps. */ contextWindow?: number; + /** + * The chat template reads `enable_thinking` (Qwen, Gemma 4, Nemotron + * and friends), so `chat_template_kwargs: { enable_thinking }` on a + * server-templated request switches reasoning on or off. Absent on + * templates that do not mention it. See `server-template-policy.ts`. + */ + supportsThinkingSwitch?: boolean; /** * Multimodal (vision) capability snapshot derived from `/props`. * @@ -142,6 +149,7 @@ export const QWEN_THINK_PROFILE: TaggedReasoningModelProfile = { reasoningCloseTag: "", requiresPromptThinkPrefix: true, allowThinkPrelude: true, + supportsThinkingSwitch: true, vision: VISION_ABSENT, }; @@ -162,6 +170,7 @@ export const GEMMA4_THINK_PROFILE: TaggedReasoningModelProfile = { turnClose: "\n", assistantOpen: "<|turn>model\n", }, + supportsThinkingSwitch: true, vision: VISION_ABSENT, }; @@ -182,7 +191,13 @@ export function detectModelProfile( ); const contextWindow = readContextWindow(props); const vision = detectVisionSupport(props); - const enriched = { ...base, vision } as ModelProfile; + const enriched = { + ...base, + ...(templateLower.includes("enable_thinking") + ? { supportsThinkingSwitch: true } + : {}), + vision, + } as ModelProfile; if (contextWindow === null) return enriched; return { ...enriched, contextWindow }; } diff --git a/src/llm/provider/completion-types.ts b/src/llm/provider/completion-types.ts index 64565b72..3ee3e0ae 100644 --- a/src/llm/provider/completion-types.ts +++ b/src/llm/provider/completion-types.ts @@ -66,6 +66,21 @@ export interface PromptMessages { tail: string; } +/** + * The prompt as two messages, for a local provider that renders through + * the model's own chat template (`/apply-template`): the stable prefix + * as the system message, the tail as the user message. `prompt` stays + * the raw text for providers and paths that do not render. + */ +export interface ChatPromptParts { + system: string; + user: string; + /** Salted hash of `system`; the rendered prefix is cached by it. */ + prefixHash: string; + /** `chat_template_kwargs.enable_thinking`; absent leaves the template's default. */ + enableThinking?: boolean; +} + export interface CompletionRequest { prompt: string; /** @@ -74,6 +89,8 @@ export interface CompletionRequest { * messages read it, everything else ignores it and sends `prompt`. */ messages?: PromptMessages; + /** See `ChatPromptParts`. Only grammar (llama-server) links receive it. */ + chat?: ChatPromptParts; grammar?: string; slotId?: number; cachePrompt?: boolean; @@ -201,6 +218,12 @@ export interface CompletionResult { * authoritative. */ servedTransport?: ToolCallTransport; + /** + * The provider's generation id (`id` on the response / SSE chunks), + * when it sends one. Recorded in the trace so a billed completion + * can be looked up at the provider. + */ + generationId?: string; } export interface OpenAiToolCall { @@ -236,6 +259,8 @@ export interface StreamFinalResult { finishReason?: string | null; usage?: CompletionUsage; modelId?: string | null; + /** See `CompletionResult.generationId`. */ + generationId?: string; /** * Whether the underlying transport actually delivered a trustworthy * terminal signal — an explicit provider `finish_reason` on any chunk, diff --git a/src/llm/provider/llama-server/llama-server-provider.test.ts b/src/llm/provider/llama-server/llama-server-provider.test.ts index a8938383..1cb29e55 100644 --- a/src/llm/provider/llama-server/llama-server-provider.test.ts +++ b/src/llm/provider/llama-server/llama-server-provider.test.ts @@ -18,8 +18,9 @@ const NO_VISION_PROFILE: ModelProfile = { function fakeClient( complete: LlamaServerClient["complete"], + applyTemplate?: LlamaServerClient["applyTemplate"], ): LlamaServerClient { - return { complete } as unknown as LlamaServerClient; + return { complete, applyTemplate } as unknown as LlamaServerClient; } describe("LlamaServerProvider", () => { @@ -263,3 +264,61 @@ describe("LlamaServerProvider", () => { ).rejects.toThrow(/http 500/); }); }); + +describe("LlamaServerProvider — server chat template (F31)", () => { + const request = { + prompt: "RAW system\nRAW tail", + grammar: 'root ::= "ok"', + slotId: 0, + chat: { system: "SYS", user: "TAIL", prefixHash: "h" }, + }; + + it("renders a request carrying chat parts through /apply-template and keeps the grammar", async () => { + const complete = vi.fn(async (req: { prompt: string }) => ({ + content: req.prompt, + })) as unknown as LlamaServerClient["complete"]; + const applyTemplate = vi.fn( + async (messages: ReadonlyArray<{ role: string; content: string }>) => + `${messages.map((m) => `[${m.role}]${m.content}`).join("")}[assistant]`, + ) as unknown as LlamaServerClient["applyTemplate"]; + const provider = new LlamaServerProvider(fakeClient(complete, applyTemplate), { + getProfile: () => NO_VISION_PROFILE, + getModelId: () => "llama-3.1-8b", + visionEnabledByConfig: false, + visionAutoDetect: false, + maxImageBytes: 1024, + maxImagesPerCall: 1, + }); + await provider.complete(request); + await provider.complete({ ...request, chat: { ...request.chat, user: "TAIL2" } }); + const sent = vi.mocked(complete).mock.calls.map((c) => c[0]); + expect(sent[0]).toMatchObject({ + prompt: "[system]SYS[user]TAIL[assistant]", + grammar: 'root ::= "ok"', + }); + expect(sent[1]!.prompt).toBe("[system]SYS[user]TAIL2[assistant]"); + // One render per prefix, whatever the tail does. + expect(applyTemplate).toHaveBeenCalledTimes(1); + }); + + it("sends the raw prompt when there are no chat parts or the render fails", async () => { + const complete = vi.fn(async () => ({})) as unknown as LlamaServerClient["complete"]; + const applyTemplate = vi.fn(async () => { + throw new Error("boom"); + }) as unknown as LlamaServerClient["applyTemplate"]; + const provider = new LlamaServerProvider(fakeClient(complete, applyTemplate), { + getProfile: () => NO_VISION_PROFILE, + visionEnabledByConfig: false, + visionAutoDetect: false, + maxImageBytes: 1024, + maxImagesPerCall: 1, + }); + const { chat: _chat, ...plain } = request; + await provider.complete(plain); + await provider.complete(request); + const sent = vi.mocked(complete).mock.calls.map((c) => c[0]); + expect(sent[0]!.prompt).toBe("RAW system\nRAW tail"); + expect(sent[1]!.prompt).toBe("RAW system\nRAW tail"); + expect(applyTemplate).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/llm/provider/llama-server/llama-server-provider.ts b/src/llm/provider/llama-server/llama-server-provider.ts index 0b26152d..eb5ef260 100644 --- a/src/llm/provider/llama-server/llama-server-provider.ts +++ b/src/llm/provider/llama-server/llama-server-provider.ts @@ -19,6 +19,8 @@ import { describeImageViaLlamaServer, resolveVisionCapabilities, } from "./llama-server-vision.js"; +import { ServerTemplateRenderer } from "./server-template-renderer.js"; +import type { StructuredLogger } from "../../../tracing/structured-logger.js"; /** * Provider adapter for vision describe-style calls against an external @@ -64,10 +66,18 @@ export class LlamaServerProvider implements LlmProvider { fetchImpl?: typeof fetch; baseUrlOverride?: string; requestTimeoutMs?: number; + /** The model whose template is in force; keys the rendered-prefix cache. */ + getModelId?: () => string | null; + logger?: StructuredLogger; }, ) { this.id = options.id ?? "local-llama"; this.getProfile = options.getProfile; + this.getModelId = options.getModelId; + this.templates = new ServerTemplateRenderer({ + applyTemplate: (messages, kwargs) => client.applyTemplate(messages, kwargs), + ...(options.logger ? { logger: options.logger } : {}), + }); this.visionEnabledByConfig = options.visionEnabledByConfig; this.visionAutoDetect = options.visionAutoDetect; this.maxImageBytes = options.maxImageBytes; @@ -78,6 +88,8 @@ export class LlamaServerProvider implements LlmProvider { } private readonly getProfile: () => ModelProfile; + private readonly getModelId: (() => string | null) | undefined; + private readonly templates: ServerTemplateRenderer; private readonly visionEnabledByConfig: boolean; private readonly visionAutoDetect: boolean; private readonly maxImageBytes: number; @@ -87,13 +99,27 @@ export class LlamaServerProvider implements LlmProvider { private readonly requestTimeoutMs: number; async complete(request: CompletionRequest): Promise { - return this.client.complete(request); + return this.client.complete(await this.rendered(request)); } async *completeStream( request: CompletionRequest, ): AsyncGenerator { - return yield* this.client.completeStream(request); + return yield* this.client.completeStream(await this.rendered(request)); + } + + /** + * A request that carries the prompt as `chat` parts is rendered + * through the model's own template (F31); anything else, and any + * render failure, sends the raw text as before. The grammar rides + * along either way. + */ + private async rendered(request: CompletionRequest): Promise { + if (request.chat === undefined) return request; + const modelKey = `${this.getProfile().id}/${this.getModelId?.() ?? ""}`; + const prompt = await this.templates.render(request.chat, modelKey); + if (prompt === null) return request; + return { ...request, prompt }; } async health(): Promise { diff --git a/src/llm/provider/llama-server/server-template-renderer.test.ts b/src/llm/provider/llama-server/server-template-renderer.test.ts new file mode 100644 index 00000000..8e8adace --- /dev/null +++ b/src/llm/provider/llama-server/server-template-renderer.test.ts @@ -0,0 +1,98 @@ +import { describe, expect, it, vi } from "vitest"; +import { LlamaServerError } from "../../llama-server-client.js"; +import { + ServerTemplateRenderer, + TAIL_SENTINEL, +} from "./server-template-renderer.js"; + +/** A Llama-3-shaped template, as `/apply-template` would render it. */ +function llama3Template( + messages: ReadonlyArray<{ role: string; content: string }>, +): string { + return ( + messages + .map( + (m) => + `<|start_header_id|>${m.role}<|end_header_id|>\n\n${m.content}<|eot_id|>`, + ) + .join("") + "<|start_header_id|>assistant<|end_header_id|>\n\n" + ); +} + +const parts = { + system: "### system\nYou are atomic-agent.", + user: "### conversation\nuser: hi\n### respond\nRespond now.\n", + prefixHash: "h1", +}; + +describe("ServerTemplateRenderer", () => { + it("renders the prefix once per hash and splices each step's tail into it", async () => { + const applyTemplate = vi.fn(async (messages) => llama3Template(messages)); + const renderer = new ServerTemplateRenderer({ applyTemplate }); + const first = await renderer.render(parts, "plain-instruct/llama-3"); + const second = await renderer.render( + { ...parts, user: "### conversation\nuser: hi\nassistant: hello\n" }, + "plain-instruct/llama-3", + ); + expect(first).toBe( + "<|start_header_id|>system<|end_header_id|>\n\n### system\nYou are atomic-agent.<|eot_id|>" + + "<|start_header_id|>user<|end_header_id|>\n\n" + + parts.user + + "<|eot_id|><|start_header_id|>assistant<|end_header_id|>\n\n", + ); + // Same head byte for byte; only the tail moved. + const head = first!.slice(0, first!.indexOf(parts.user)); + expect(second!.startsWith(head)).toBe(true); + expect(second).toContain("assistant: hello"); + expect(applyTemplate).toHaveBeenCalledTimes(1); + expect(applyTemplate.mock.calls[0]![0]).toEqual([ + { role: "system", content: parts.system }, + { role: "user", content: TAIL_SENTINEL }, + ]); + expect(applyTemplate.mock.calls[0]![1]).toBeUndefined(); + }); + + it("re-renders for another prefix, model or thinking setting, and passes the switch through", async () => { + const applyTemplate = vi.fn(async (messages) => llama3Template(messages)); + const renderer = new ServerTemplateRenderer({ applyTemplate }); + await renderer.render(parts, "m1"); + await renderer.render({ ...parts, prefixHash: "h2" }, "m1"); + await renderer.render(parts, "m2"); + await renderer.render({ ...parts, enableThinking: false }, "m1"); + await renderer.render(parts, "m1"); + expect(applyTemplate).toHaveBeenCalledTimes(4); + expect(applyTemplate.mock.calls[3]![1]).toEqual({ enable_thinking: false }); + }); + + it("falls back to the raw prompt when the template loses or duplicates the sentinel", async () => { + const drops = new ServerTemplateRenderer({ + applyTemplate: async () => "[INST] nothing of yours [/INST]", + }); + expect(await drops.render(parts, "m")).toBeNull(); + const doubles = new ServerTemplateRenderer({ + applyTemplate: async () => `${TAIL_SENTINEL} ${TAIL_SENTINEL}`, + }); + expect(await doubles.render(parts, "m")).toBeNull(); + }); + + it("gives up on the endpoint after a 404, and retries other failures next step", async () => { + const missing = vi.fn(async () => { + throw new LlamaServerError("not found", 404, "http://x/apply-template"); + }); + const renderer = new ServerTemplateRenderer({ applyTemplate: missing }); + expect(await renderer.render(parts, "m")).toBeNull(); + expect(await renderer.render(parts, "m")).toBeNull(); + expect(missing).toHaveBeenCalledTimes(1); + + let calls = 0; + const flaky = new ServerTemplateRenderer({ + applyTemplate: async (messages) => { + calls += 1; + if (calls === 1) throw new LlamaServerError("reset", null, "u"); + return llama3Template(messages); + }, + }); + expect(await flaky.render(parts, "m")).toBeNull(); + expect(await flaky.render(parts, "m")).not.toBeNull(); + }); +}); diff --git a/src/llm/provider/llama-server/server-template-renderer.ts b/src/llm/provider/llama-server/server-template-renderer.ts new file mode 100644 index 00000000..4eb6ff9a --- /dev/null +++ b/src/llm/provider/llama-server/server-template-renderer.ts @@ -0,0 +1,136 @@ +import type { StructuredLogger } from "../../../tracing/structured-logger.js"; +import type { ChatPromptParts } from "../completion-types.js"; +import { LlamaServerError } from "../../llama-server-client.js"; + +/** + * Renders a local prompt through the model's own chat template, keeping + * the stable prefix stable. + * + * The prompt reaches the provider as two parts: the stable prefix + * (system, rules, tools — identical from step to step) and the tail + * (conversation, notices, `### respond`). Sending them to + * `/apply-template` as a system and a user message gives the model the + * turn markers its template expects. Doing that on every step would + * cost a round trip each time and, worse, would let the template touch + * the prefix bytes per request. So the prefix is rendered once per + * stable-prefix hash with a sentinel in the user slot; the template's + * output splits at the sentinel into a head (everything up to the user + * content) and a foot (the user turn's close and the assistant opener), + * and each step is `head + tail + foot`. The head is byte-stable, so + * llama-server's prefix cache reuses it exactly as it reused the raw + * prefix. + * + * Failure is never fatal: a server without the endpoint (older builds + * answer 404), a template that loses the sentinel, or a network error + * all fall back to the raw text prompt, with one log line. + */ +export type ApplyTemplate = ( + messages: ReadonlyArray<{ role: string; content: string }>, + chatTemplateKwargs: Record | undefined, +) => Promise; + +export interface ServerTemplateRendererOptions { + applyTemplate: ApplyTemplate; + logger?: StructuredLogger; +} + +/** Rendered prefixes kept per process; a prefix hash changes rarely. */ +const CACHE_MAX_ENTRIES = 8; + +/** Stands in for the tail while the prefix is rendered; never appears in a real prompt. */ +export const TAIL_SENTINEL = "ATAG_TAIL_SENTINEL_7f3a"; + +interface RenderedPrefix { + head: string; + foot: string; +} + +export class ServerTemplateRenderer { + private readonly applyTemplate: ApplyTemplate; + private readonly logger: StructuredLogger | undefined; + private readonly cache = new Map(); + /** Set once the server has shown it has no `/apply-template`. */ + private unsupported = false; + + constructor(options: ServerTemplateRendererOptions) { + this.applyTemplate = options.applyTemplate; + this.logger = options.logger; + } + + /** + * The prompt to send to `/completion`, or `null` when the template + * path is unavailable and the caller should send its raw text. + * `modelKey` names the model whose template is in force, so a hot + * swap to a different GGUF never reuses another template's framing. + */ + async render( + parts: ChatPromptParts, + modelKey: string, + ): Promise { + if (this.unsupported) return null; + const key = `${parts.prefixHash}|${modelKey}|${String(parts.enableThinking)}`; + let rendered = this.cache.get(key); + if (rendered === undefined) { + rendered = (await this.renderPrefix(parts)) ?? undefined; + if (rendered === undefined) return null; + this.remember(key, rendered); + } + return `${rendered.head}${parts.user}${rendered.foot}`; + } + + private async renderPrefix( + parts: ChatPromptParts, + ): Promise { + const kwargs = + parts.enableThinking === undefined + ? undefined + : { enable_thinking: parts.enableThinking }; + let text: string; + try { + text = await this.applyTemplate( + [ + { role: "system", content: parts.system }, + { role: "user", content: TAIL_SENTINEL }, + ], + kwargs, + ); + } catch (err) { + if ( + err instanceof LlamaServerError && + (err.status === 404 || err.status === 405 || err.status === 501) + ) { + this.unsupported = true; + this.logger?.warn( + "llama-server has no /apply-template; local prompts stay in atag's own framing", + { status: err.status }, + ); + return null; + } + this.logger?.warn( + "chat template render failed; sending the raw prompt for this step", + { error: err instanceof Error ? err.message : String(err) }, + ); + return null; + } + const at = text.indexOf(TAIL_SENTINEL); + if (at === -1 || text.lastIndexOf(TAIL_SENTINEL) !== at) { + this.logger?.warn( + "chat template did not carry the user content through; sending the raw prompt", + { occurrences: at === -1 ? 0 : 2 }, + ); + return null; + } + return { + head: text.slice(0, at), + foot: text.slice(at + TAIL_SENTINEL.length), + }; + } + + private remember(key: string, rendered: RenderedPrefix): void { + if (this.cache.size >= CACHE_MAX_ENTRIES) { + const oldest = this.cache.keys().next().value; + if (oldest !== undefined) this.cache.delete(oldest); + } + this.cache.set(key, rendered); + } +} diff --git a/src/llm/provider/openai/generation-id.ts b/src/llm/provider/openai/generation-id.ts new file mode 100644 index 00000000..76a1da72 --- /dev/null +++ b/src/llm/provider/openai/generation-id.ts @@ -0,0 +1,45 @@ +/** + * The provider's generation id (`id` on every SSE chunk and on a unary + * response — `gen-…` on OpenRouter, `chatcmpl-…` on OpenAI), carried on + * completions and on errors thrown after a stream had produced output. + * + * A 504 after 10,528 streamed tokens is still billed. Without the id in + * the trace the cost is invisible; with it the operator can look the + * generation up at the provider and recover the figure. + */ +export function attachGenerationId(err: T, generationId: string | null): T { + if (generationId === null || typeof err !== "object" || err === null) { + return err; + } + const target = err as { generationId?: unknown }; + if (typeof target.generationId !== "string") { + try { + Object.defineProperty(target, "generationId", { + value: generationId, + enumerable: false, + configurable: true, + writable: true, + }); + } catch { + // A frozen error keeps its shape; the id is lost, nothing else. + } + } + return err; +} + +/** Depth cap on the `cause` walk — longer is a cycle. */ +const MAX_CAUSE_DEPTH = 5; + +/** The generation id on an error or any error in its `cause` chain. */ +export function readGenerationId(err: unknown): string | undefined { + let current: unknown = err; + for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth += 1) { + if (typeof current !== "object" || current === null) return undefined; + const id = (current as { generationId?: unknown }).generationId; + if (typeof id === "string" && id.length > 0) return id; + const next = (current as { cause?: unknown }).cause; + if (next === current || next === undefined) return undefined; + current = next; + } + return undefined; +} diff --git a/src/llm/provider/openai/openai-http.test.ts b/src/llm/provider/openai/openai-http.test.ts index 86355718..57301722 100644 --- a/src/llm/provider/openai/openai-http.test.ts +++ b/src/llm/provider/openai/openai-http.test.ts @@ -125,6 +125,37 @@ describe("openAiPostJson", () => { expect(fetchImpl).toHaveBeenCalledTimes(2); }); + it("does not retry a 429 whose body says the credit is exhausted, and keeps the body", async () => { + // Retried 42 times per worker in the field, as if it were throttling. + const body = JSON.stringify({ + error: { + message: "Provider returned error", + code: 429, + metadata: { + raw: '{"error":{"type":"credit_balance_exhausted","message":"Your credit balance is too low"}}', + }, + }, + }); + const fetchImpl = vi + .fn() + .mockResolvedValue(errorResponse(429, body, { "retry-after": "0" })); + const err = await openAiPostJson( + depsWith(fetchImpl as unknown as typeof fetch), + "/x", + {}, + {}, + ).catch((e: unknown) => e); + expect(err).toBeInstanceOf(OpenAiHttpError); + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect((err as OpenAiHttpError).body).toMatchObject({ + message: "Provider returned error", + code: "429", + }); + expect((err as OpenAiHttpError).body?.text).toContain( + "credit_balance_exhausted", + ); + }); + describe("structured RetryInfo metadata", () => { // Gemini's OpenAI-compatible endpoint sends its cooldown only in // the error JSON — google.rpc.RetryInfo with a protobuf Duration diff --git a/src/llm/provider/openai/openai-http.ts b/src/llm/provider/openai/openai-http.ts index 4c8f6c4e..64a92b9d 100644 --- a/src/llm/provider/openai/openai-http.ts +++ b/src/llm/provider/openai/openai-http.ts @@ -7,6 +7,11 @@ import { type CreditLimitLogger, type CreditLimitRetryPlan, } from "./plan-credit-limit-retry.js"; +import { + parseProviderErrorBody, + readProviderErrorReason, + type ProviderErrorBody, +} from "./parse-provider-error-body.js"; export type OpenAiHttpDeps = { baseUrl: string; @@ -64,14 +69,36 @@ export class OpenAiHttpError extends Error { * apart in a postmortem. */ public readonly code: string | undefined = undefined, - options?: { cause?: unknown }, + options?: { + cause?: unknown; + body?: ProviderErrorBody; + generationId?: string; + }, ) { super(message); this.name = "OpenAiHttpError"; if (options?.cause !== undefined) { (this as { cause?: unknown }).cause = options.cause; } + if (options?.body !== undefined) this.body = options.body; + if (options?.generationId !== undefined) { + this.generationId = options.generationId; + } } + + /** + * The error body read for its reason (`parseProviderErrorBody`): + * exhausted credit and cooldown hints live there, not in the status. + * Absent on network failures and on errors built without a body. + */ + readonly body?: ProviderErrorBody; + + /** + * The SSE generation id (`id` on the chunks) of a stream that failed + * after it had started — a 504 after 10,000 streamed tokens is still + * billed, and the id is what recovers the cost from the provider. + */ + readonly generationId?: string; } /** @@ -511,6 +538,8 @@ async function httpErrorFromResponse( false, retryAfterMs, deps.label, + undefined, + { body: parseProviderErrorBody(text) }, ); } @@ -525,9 +554,25 @@ function isRetryableOpenAiError(err: unknown): boolean { if (!(err instanceof OpenAiHttpError)) return false; if (err.timedOut) return false; if (err.status === null) return true; + // A 429 whose body says the account is out of credit is not + // throttling: the same request fails the same way until someone tops + // up, and three fast retries only multiply the refusals (42 per + // worker, once). + if (isCreditExhausted(err)) return false; return err.status >= 500 || err.status === 429 || err.status === 408; } +function isCreditExhausted(err: OpenAiHttpError): boolean { + return ( + readProviderErrorReason({ + status: err.status, + body: err.body, + message: err.message, + retryAfterMs: err.retryAfterMs, + })?.kind === "credit_exhausted" + ); +} + /** * Run `attempt` under the bounded retry policy, spending `budget`. * diff --git a/src/llm/provider/openai/openai-normalise-response.ts b/src/llm/provider/openai/openai-normalise-response.ts index 229c79df..53229938 100644 --- a/src/llm/provider/openai/openai-normalise-response.ts +++ b/src/llm/provider/openai/openai-normalise-response.ts @@ -37,6 +37,9 @@ export function normaliseOpenAiChatResponse( cacheHitTokens: usage.cachedTokens ?? 0, slotId: -1, modelId: typeof json.model === "string" ? json.model : defaultChatModel, + ...(typeof json.id === "string" && json.id.length > 0 + ? { generationId: json.id } + : {}), usage, toolCalls, finishReason: diff --git a/src/llm/provider/openai/openai-provider.ts b/src/llm/provider/openai/openai-provider.ts index 08883545..31eeb0f9 100644 --- a/src/llm/provider/openai/openai-provider.ts +++ b/src/llm/provider/openai/openai-provider.ts @@ -19,7 +19,10 @@ import { openAiToolCallAdapter, withStrictNullArgumentDrop, } from "./openai-tool-call-adapter.js"; -import { createOpenAiStreamConsumer } from "./openai-stream-consumer.js"; +import { + createOpenAiStreamConsumer, + OpenAiSseError, +} from "./openai-stream-consumer.js"; import { buildOpenAiChatBody, resolveMessageShape, @@ -446,6 +449,10 @@ export class OpenAiProvider implements LlmProvider { body = this.buildBody(request, true, shape); continue; } + // An error event inside the stream (OpenRouter's `504 Upstream + // idle timeout` after output) becomes the typed HTTP error the + // loop classifies, carrying the generation id for the trace. + if (err instanceof OpenAiSseError) throw this.httpErrorFromSse(err, path); if (!canReopenStream(err, committed, budget)) throw err; // No `res.body.cancel()` here, on purpose. The only way to reach // this line with a response in hand is `isNetworkError(err)` on @@ -520,6 +527,29 @@ export class OpenAiProvider implements LlmProvider { ); } + /** + * An error the provider reported inside the stream, as the typed HTTP + * failure the rest of the runtime knows: a 5xx parks the turn like a + * 5xx on the open would, and the generation id stays on it. + */ + private httpErrorFromSse(err: OpenAiSseError, path: string): OpenAiHttpError { + return new OpenAiHttpError( + `openai provider ${err.status ?? "stream"}: ${err.message}`, + err.status, + `${this.http.baseUrl}${path}`, + false, + null, + this.http.label, + undefined, + { + cause: err, + ...(err.generationId !== null + ? { generationId: err.generationId } + : {}), + }, + ); + } + async health(): Promise { const start = Date.now(); try { @@ -631,6 +661,9 @@ function completionFromStreamFinal( usage, toolCalls: streamFinal?.toolCalls, finishReason, + ...(streamFinal?.generationId !== undefined + ? { generationId: streamFinal.generationId } + : {}), ...(streamFinal?.earlyStop !== undefined ? { earlyStop: streamFinal.earlyStop } : {}), diff --git a/src/llm/provider/openai/openai-stream-consumer.test.ts b/src/llm/provider/openai/openai-stream-consumer.test.ts index 6d53f074..555adad9 100644 --- a/src/llm/provider/openai/openai-stream-consumer.test.ts +++ b/src/llm/provider/openai/openai-stream-consumer.test.ts @@ -1,6 +1,10 @@ import { describe, expect, it } from "vitest"; -import { createOpenAiStreamConsumer } from "./openai-stream-consumer.js"; +import { + createOpenAiStreamConsumer, + OpenAiSseError, +} from "./openai-stream-consumer.js"; +import { readGenerationId } from "./generation-id.js"; import type { StreamFinalResult } from "../completion-types.js"; function sseFrame(payload: Record): string { @@ -311,3 +315,67 @@ describe("openai stream consumer: text beside a tool-call delta", () => { } }); }); + +describe("openai stream consumer generation id (F29)", () => { + it("carries the chunks' id on the final result", async () => { + const result = await drain( + sseFrame({ + id: "gen-abc123", + model: "test-model", + choices: [{ index: 0, delta: { content: "hi" }, finish_reason: null }], + }) + DONE, + ); + expect(result.generationId).toBe("gen-abc123"); + expect(result.content).toBe("hi"); + }); + + it("throws a typed error on a mid-stream error event, with the id and what streamed before it", async () => { + // OpenRouter, run 14: `504 Upstream idle timeout` after 10,528 tokens. + const body = + sseFrame({ + id: "gen-504", + model: "test-model", + choices: [{ index: 0, delta: { content: "partial" }, finish_reason: null }], + }) + + sseFrame({ + id: "gen-504", + error: { code: 504, message: "Upstream idle timeout" }, + choices: [{ index: 0, delta: {}, finish_reason: "error" }], + }); + const consumer = createOpenAiStreamConsumer("delta_reasoning"); + const iterator = consumer.consume(bodyOf(body), undefined); + const first = await iterator.next(); + expect(first.done).toBe(false); + const err = await iterator.next().catch((e: unknown) => e); + expect(err).toBeInstanceOf(OpenAiSseError); + expect((err as OpenAiSseError).status).toBe(504); + expect((err as OpenAiSseError).message).toBe("Upstream idle timeout"); + expect((err as OpenAiSseError).generationId).toBe("gen-504"); + expect(readGenerationId(err)).toBe("gen-504"); + }); + + it("attaches the id to a body that died after output", async () => { + const chunk = new TextEncoder().encode( + sseFrame({ + id: "gen-dead", + model: "test-model", + choices: [{ index: 0, delta: { content: "some" }, finish_reason: null }], + }), + ); + const dying = new ReadableStream({ + start(controller) { + controller.enqueue(chunk); + }, + pull(controller) { + controller.error(new Error("terminated")); + }, + }); + const consumer = createOpenAiStreamConsumer("delta_reasoning"); + const iterator = consumer.consume(dying, undefined); + await iterator.next(); + const err = await iterator.next().catch((e: unknown) => e); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toBe("terminated"); + expect(readGenerationId(err)).toBe("gen-dead"); + }); +}); diff --git a/src/llm/provider/openai/openai-stream-consumer.ts b/src/llm/provider/openai/openai-stream-consumer.ts index 4eae156a..be2f19af 100644 --- a/src/llm/provider/openai/openai-stream-consumer.ts +++ b/src/llm/provider/openai/openai-stream-consumer.ts @@ -14,6 +14,23 @@ import { type OpenAiToolCallDelta, } from "./parse-sse-chunk.js"; import { extractPartialReplyTextFromToolArguments } from "./tool-arguments-stream-parser.js"; +import { attachGenerationId } from "./generation-id.js"; + +/** + * A provider reported an error inside the SSE stream itself. Thrown by + * the consumer; the provider turns it into an `OpenAiHttpError` with + * the request's url and label, keeping the generation id. + */ +export class OpenAiSseError extends Error { + constructor( + readonly status: number | null, + message: string, + readonly generationId: string | null, + ) { + super(message); + this.name = "OpenAiSseError"; + } +} type MutableToolCall = { /** Position in the final array. See `orderFor`. */ @@ -68,6 +85,7 @@ export function createOpenAiStreamConsumer( let finishReason: string | null = null; let modelId: string | null = null; let usage: CompletionUsage | undefined; + let generationId: string | null = null; // A trustworthy terminal signal: an explicit provider finish_reason // on any chunk, or a parser-recognized terminal event (`[DONE]`). // Some OpenAI-compatible providers send a final finish_reason and @@ -105,6 +123,16 @@ export function createOpenAiStreamConsumer( reasoning, toolArgsBuffer, ); + generationId = chunk.id ?? generationId; + if (chunk.error !== null) { + // Whatever streamed before this is billed under the id; + // the error carries it so the trace can say so. + throw new OpenAiSseError( + chunk.error.status, + chunk.error.message, + generationId, + ); + } content += chunk.delta; reasoningContent += chunk.reasoningDelta; if (chunk.finishReason !== null) terminalObserved = true; @@ -120,6 +148,7 @@ export function createOpenAiStreamConsumer( finishReason, modelId, usage, + generationId, toolCalls, terminalObserved: true, }); @@ -174,6 +203,11 @@ export function createOpenAiStreamConsumer( } if (done) break; } + } catch (err) { + // A body that died after output (`Error: terminated`, a 504 in + // the stream) still cost the tokens it streamed. The id travels + // on the error so the trace row can name the generation. + throw attachGenerationId(err, generationId); } finally { reader.releaseLock(); } @@ -184,6 +218,7 @@ export function createOpenAiStreamConsumer( finishReason, modelId, usage, + generationId, toolCalls, terminalObserved, ...(earlyStop !== undefined ? { earlyStop } : {}), @@ -318,6 +353,7 @@ function buildFinalResult(args: { finishReason: string | null; modelId: string | null; usage?: CompletionUsage; + generationId: string | null; toolCalls: ToolCallAccumulator; terminalObserved: boolean; earlyStop?: CompletionEarlyStop; @@ -342,6 +378,7 @@ function buildFinalResult(args: { modelId: args.modelId, terminalObserved: args.terminalObserved, ...(args.usage ? { usage: args.usage } : {}), + ...(args.generationId !== null ? { generationId: args.generationId } : {}), ...(sortedToolCalls.length > 0 ? { toolCalls: sortedToolCalls } : {}), ...(args.earlyStop !== undefined ? { earlyStop: args.earlyStop } : {}), }; diff --git a/src/llm/provider/openai/parse-provider-error-body.test.ts b/src/llm/provider/openai/parse-provider-error-body.test.ts new file mode 100644 index 00000000..ddd498ec --- /dev/null +++ b/src/llm/provider/openai/parse-provider-error-body.test.ts @@ -0,0 +1,179 @@ +import { describe, expect, it } from "vitest"; +import { + IN_FLIGHT_BUDGET_DEFAULT_WAIT_MS, + parseProviderErrorBody, + readProviderErrorReason, +} from "./parse-provider-error-body.js"; + +describe("parseProviderErrorBody", () => { + it("reads the OpenAI shape", () => { + const body = parseProviderErrorBody( + JSON.stringify({ + error: { + message: "You exceeded your current quota", + type: "insufficient_quota", + code: "insufficient_quota", + }, + }), + ); + expect(body).toMatchObject({ + message: "You exceeded your current quota", + type: "insufficient_quota", + code: "insufficient_quota", + }); + expect(body.retryHintMs).toBeUndefined(); + }); + + it("reads the OpenRouter shape, including the upstream raw body", () => { + const body = parseProviderErrorBody( + JSON.stringify({ + error: { + message: "Provider returned error", + code: 429, + metadata: { + provider_name: "Anthropic", + raw: '{"type":"error","error":{"type":"credit_balance_exhausted","message":"Your credit balance is too low"}}', + }, + }, + }), + ); + expect(body.code).toBe("429"); + expect(body.upstream).toBe("Anthropic"); + expect(body.text).toContain("credit_balance_exhausted"); + }); + + it.each([ + ["retry in 120 s", 120_000], + ["Please retry after 2 minutes.", 120_000], + ["try again in 30 seconds", 30_000], + ["retry in 500ms", 500], + ["Retry the request in 1.5s", 1_500], + ])("reads a textual cooldown: %s", (text, ms) => { + expect( + parseProviderErrorBody(JSON.stringify({ error: { message: text } })) + .retryHintMs, + ).toBe(ms); + }); + + it("copes with a body that is not JSON", () => { + const body = parseProviderErrorBody("Bad gateway"); + expect(body.text).toBe("Bad gateway"); + expect(body.message).toBeUndefined(); + }); +}); + +describe("readProviderErrorReason", () => { + const read = ( + status: number | null, + text: string, + retryAfterMs: number | null = null, + ) => + readProviderErrorReason({ + status, + body: parseProviderErrorBody(text), + message: `openai provider ${status}: ${text.slice(0, 300)}`, + retryAfterMs, + }); + + it("reads exhausted credit off a 429 that OpenRouter relays for Anthropic", () => { + // The Codex attempt: retried 42 times per worker as rate limiting. + expect( + read( + 429, + JSON.stringify({ + error: { + message: "Provider returned error", + code: 429, + metadata: { + raw: '{"error":{"type":"credit_balance_exhausted","message":"Your credit balance is too low"}}', + }, + }, + }), + 30_000, + ), + ).toEqual({ kind: "credit_exhausted", code: "credit_balance_exhausted" }); + }); + + it("reads exhausted credit off OpenAI's insufficient_quota and OpenRouter's insufficient_credits", () => { + expect( + read(429, JSON.stringify({ error: { code: "insufficient_quota" } })), + ).toEqual({ kind: "credit_exhausted", code: "insufficient_quota" }); + expect( + read(402, JSON.stringify({ error: { code: "insufficient_credits" } })), + ).toEqual({ kind: "credit_exhausted", code: "insufficient_credits" }); + }); + + it("treats a 402 that talks about credit as exhausted credit", () => { + expect( + read( + 402, + JSON.stringify({ + error: { + message: + "This request requires more credits, or fewer max_tokens. You requested up to 65536 tokens, but can only afford 900.", + }, + }), + ), + ).toEqual({ kind: "credit_exhausted", code: "402" }); + }); + + it("honours OpenRouter's in-flight budget with its hint, or a default wait", () => { + expect( + read( + 402, + JSON.stringify({ + error: { + code: "in_flight_budget_exhausted", + message: "Too many requests in flight for your balance; retry in 120 s", + }, + }), + ), + ).toEqual({ + kind: "retry_after", + delayMs: 120_000, + code: "in_flight_budget_exhausted", + }); + expect( + read( + 402, + JSON.stringify({ error: { code: "in_flight_budget_exhausted" } }), + ), + ).toEqual({ + kind: "retry_after", + delayMs: IN_FLIGHT_BUDGET_DEFAULT_WAIT_MS, + code: "in_flight_budget_exhausted", + }); + }); + + it("turns a retry-after header on a 429 into a wait, and a plain 429 into nothing", () => { + expect( + read(429, JSON.stringify({ error: { message: "rate limited" } }), 7_000), + ).toEqual({ kind: "retry_after", delayMs: 7_000, code: null }); + expect(read(429, JSON.stringify({ error: { message: "rate limited" } }))).toBeNull(); + }); + + it("reads a textual cooldown off a 503", () => { + expect( + read(503, JSON.stringify({ error: { message: "overloaded, retry in 5 s" } })), + ).toEqual({ kind: "retry_after", delayMs: 5_000, code: null }); + }); + + it("ignores cooldown wording on a status that is not a cooldown", () => { + expect( + read(400, JSON.stringify({ error: { message: "bad request; retry in 5 s" } })), + ).toBeNull(); + expect(read(401, "", 5_000)).toBeNull(); + }); + + it("falls back to the error's own message when no body was kept", () => { + expect( + readProviderErrorReason({ + status: 429, + body: undefined, + message: + 'openai provider 429: {"error":{"type":"credit_balance_exhausted"}}', + retryAfterMs: null, + }), + ).toEqual({ kind: "credit_exhausted", code: "credit_balance_exhausted" }); + }); +}); diff --git a/src/llm/provider/openai/parse-provider-error-body.ts b/src/llm/provider/openai/parse-provider-error-body.ts new file mode 100644 index 00000000..4452d460 --- /dev/null +++ b/src/llm/provider/openai/parse-provider-error-body.ts @@ -0,0 +1,190 @@ +/** + * What an HTTP error body says, read for its reason rather than its + * status. + * + * Two shapes cover the OpenAI-compatible world: + * + * OpenAI `{ "error": { "message", "code", "type", "param" } }` + * OpenRouter `{ "error": { "message", "code", "metadata": { "raw", "provider_name" } } }` + * + * where OpenRouter's `metadata.raw` is often the upstream vendor's own + * body as a string — and that is where `credit_balance_exhausted` + * actually appears. The status alone told the runtime nothing: a 429 + * for exhausted credit was parked and retried as rate limiting (42 + * times per worker, once), and a 402 carrying a "retry in 120 s" hint + * ended a run as final. + * + * Pure: no imports, so the HTTP client can use it on the way out without + * a cycle through the reliability layer. + */ +export interface ProviderErrorBody { + /** `error.message`, when the body parsed. */ + readonly message?: string; + /** `error.code` as a string (a numeric code is kept as its digits). */ + readonly code?: string; + /** `error.type`. */ + readonly type?: string; + /** Upstream vendor name from OpenRouter's `metadata.provider_name`. */ + readonly upstream?: string; + /** + * A cooldown the body's *text* asked for ("retry in 120 s", "try + * again in 2 minutes"). Headers and structured `RetryInfo` are read + * elsewhere; this is the fallback for providers that only say it. + */ + readonly retryHintMs?: number; + /** The whole body, bounded, for wording checks. */ + readonly text: string; +} + +/** Longest body kept for wording checks. */ +const BODY_TEXT_MAX = 2_000; + +/** + * Codes and types that mean the account cannot pay for the request. + * `insufficient_quota` is OpenAI's own; the other two are OpenRouter's + * and Anthropic-via-OpenRouter's. + */ +const CREDIT_CODES = + /\b(?:credit_balance_exhausted|insufficient_credits|insufficient_quota)\b/i; + +/** OpenRouter's "you have too many requests in flight for your balance". */ +const IN_FLIGHT_BUDGET = /\bin_flight_budget_exhausted\b/i; + +/** "retry in 120 s", "retry after 2 minutes", "try again in 30 seconds". */ +const RETRY_HINT = + /\b(?:retry|try again|please wait)(?:\s+\w+){0,2}?\s+(?:in|after)\s+(\d+(?:\.\d+)?)\s*(ms|milliseconds?|s|secs?|seconds?|m|mins?|minutes?)\b/i; + +export function parseProviderErrorBody(text: string): ProviderErrorBody { + const bounded = text.slice(0, BODY_TEXT_MAX); + const error = readErrorObject(bounded); + const raw = error !== null ? readRaw(error.metadata) : null; + const message = readString(error?.message); + const code = readCode(error?.code); + const type = readString(error?.type); + const upstream = readString(readObject(error?.metadata)?.provider_name); + const hint = retryHintMs(`${message ?? ""}\n${raw ?? ""}\n${bounded}`); + return { + ...(message !== undefined ? { message } : {}), + ...(code !== undefined ? { code } : {}), + ...(type !== undefined ? { type } : {}), + ...(upstream !== undefined ? { upstream } : {}), + ...(hint !== null ? { retryHintMs: hint } : {}), + text: raw !== null && !bounded.includes(raw) ? `${bounded}\n${raw}` : bounded, + }; +} + +/** What a body says about the account or a cooldown, before any status logic. */ +export type ProviderErrorReason = + | { kind: "credit_exhausted"; code: string } + | { kind: "retry_after"; delayMs: number | null; code: string | null }; + +/** + * Cooldown a provider is allowed to ask an interactive turn for. Longer + * hints are clipped to this; a provider must not park a run. + */ +export const RETRY_HINT_MAX_MS = 180_000; + +/** Wait for `in_flight_budget_exhausted` when the body names no delay. */ +export const IN_FLIGHT_BUDGET_DEFAULT_WAIT_MS = 30_000; + +/** + * Read the reason out of a parsed body plus the transport facts around + * it. `retryAfterMs` is the header / `RetryInfo` value the HTTP client + * already extracted, when any. + */ +export function readProviderErrorReason(input: { + status: number | null; + body: ProviderErrorBody | undefined; + /** The error's own message: the body's head when `body` is absent. */ + message: string; + retryAfterMs: number | null; +}): ProviderErrorReason | null { + const { status } = input; + const code = input.body?.code ?? input.body?.type ?? ""; + const text = `${code}\n${input.body?.text ?? ""}\n${input.message}`; + const credit = CREDIT_CODES.exec(text); + if (credit !== null) { + return { kind: "credit_exhausted", code: credit[0].toLowerCase() }; + } + if (status === 402 && /\bcredits?\b/i.test(text)) { + return { kind: "credit_exhausted", code: "402" }; + } + if (!isCooldownStatus(status)) return null; + const hinted = input.retryAfterMs ?? input.body?.retryHintMs ?? null; + if (IN_FLIGHT_BUDGET.test(text)) { + return { + kind: "retry_after", + delayMs: hinted ?? IN_FLIGHT_BUDGET_DEFAULT_WAIT_MS, + code: "in_flight_budget_exhausted", + }; + } + if (hinted !== null) { + return { kind: "retry_after", delayMs: hinted, code: code || null }; + } + return null; +} + +/** Statuses whose body may legitimately ask for a cooldown. */ +function isCooldownStatus(status: number | null): boolean { + return status === 402 || status === 408 || status === 429 || (status !== null && status >= 500); +} + +function retryHintMs(text: string): number | null { + const match = RETRY_HINT.exec(text); + if (!match) return null; + const amount = Number(match[1]); + if (!Number.isFinite(amount) || amount < 0) return null; + const unit = match[2]!.toLowerCase(); + const ms = unit.startsWith("ms") || unit.startsWith("milli") + ? amount + : unit.startsWith("m") + ? amount * 60_000 + : amount * 1_000; + return Math.round(ms); +} + +function readErrorObject(text: string): Record | null { + const parsed = tryParseJson(text); + const error = readObject(parsed?.error); + return error; +} + +/** OpenRouter's `metadata.raw`: the upstream body, as text or as an object. */ +function readRaw(metadata: unknown): string | null { + const raw = readObject(metadata)?.raw; + if (typeof raw === "string") return raw.slice(0, BODY_TEXT_MAX); + if (raw !== null && typeof raw === "object") { + try { + return JSON.stringify(raw).slice(0, BODY_TEXT_MAX); + } catch { + return null; + } + } + return null; +} + +function tryParseJson(text: string): Record | null { + const start = text.indexOf("{"); + if (start === -1) return null; + try { + return readObject(JSON.parse(text.slice(start))); + } catch { + return null; + } +} + +function readObject(value: unknown): Record | null { + return value !== null && typeof value === "object" && !Array.isArray(value) + ? (value as Record) + : null; +} + +function readString(value: unknown): string | undefined { + return typeof value === "string" && value.length > 0 ? value : undefined; +} + +function readCode(value: unknown): string | undefined { + if (typeof value === "string" && value.length > 0) return value; + if (typeof value === "number" && Number.isFinite(value)) return String(value); + return undefined; +} diff --git a/src/llm/provider/openai/parse-sse-chunk.ts b/src/llm/provider/openai/parse-sse-chunk.ts index 6082dc21..3e0a5993 100644 --- a/src/llm/provider/openai/parse-sse-chunk.ts +++ b/src/llm/provider/openai/parse-sse-chunk.ts @@ -33,6 +33,14 @@ export function parseOpenAiSseEvent( modelId: string | null; usage: Record | null; toolCallDeltas: OpenAiToolCallDelta[]; + /** The provider's generation id (`id` on the chunk), when it sends one. */ + id: string | null; + /** + * A mid-stream error event (`{"error": {"code": 504, "message": …}}`, + * how OpenRouter reports an upstream that died after output started). + * The consumer throws on it; a chunk carrying one has no delta. + */ + error: { status: number | null; message: string } | null; } { const dataLines: string[] = []; for (const line of rawEvent.split("\n")) { @@ -52,6 +60,8 @@ export function parseOpenAiSseEvent( modelId: null, usage: null, toolCallDeltas: [], + id: null, + error: null, }; } const joined = dataLines.join("\n"); @@ -67,6 +77,8 @@ export function parseOpenAiSseEvent( modelId: null, usage: null, toolCallDeltas: [], + id: null, + error: null, }; } try { @@ -80,6 +92,8 @@ export function parseOpenAiSseEvent( const finishReason = typeof choice?.finish_reason === "string" ? choice.finish_reason : null; const modelId = typeof payload.model === "string" ? payload.model : null; + const id = typeof payload.id === "string" && payload.id.length > 0 ? payload.id : null; + const error = readStreamError(payload.error); const usage = payload.usage && typeof payload.usage === "object" ? (payload.usage as Record) @@ -110,6 +124,8 @@ export function parseOpenAiSseEvent( finishReason, modelId, usage, + id, + error, toolCallDeltas: toolCalls.map((toolCall) => ({ ...(typeof toolCall.index === "number" ? { index: toolCall.index } @@ -143,6 +159,8 @@ export function parseOpenAiSseEvent( finishReason, modelId, usage, + id, + error, toolCallDeltas: [], }; } catch { @@ -157,6 +175,28 @@ export function parseOpenAiSseEvent( modelId: null, usage: null, toolCallDeltas: [], + id: null, + error: null, }; } } + +function readStreamError( + value: unknown, +): { status: number | null; message: string } | null { + if (value === null || typeof value !== "object" || Array.isArray(value)) { + return null; + } + const error = value as { code?: unknown; message?: unknown }; + const code = + typeof error.code === "number" + ? error.code + : typeof error.code === "string" && /^\d{3}$/.test(error.code) + ? Number(error.code) + : null; + const message = + typeof error.message === "string" && error.message.length > 0 + ? error.message + : "stream error"; + return { status: code, message }; +} diff --git a/src/llm/provider/registry/provider-types.ts b/src/llm/provider/registry/provider-types.ts index 3c65e0f6..fa70fad6 100644 --- a/src/llm/provider/registry/provider-types.ts +++ b/src/llm/provider/registry/provider-types.ts @@ -11,6 +11,8 @@ export type ProviderFactoryContext = { entry: LlmProviderConfigEntry; llamaClient?: LlamaServerClient; getProfile?: () => ModelProfile; + /** The local model's id, for the llama-server provider's template cache. */ + getModelId?: () => string | null; logger: StructuredLogger; }; diff --git a/src/llm/provider/registry/register-built-in-providers.ts b/src/llm/provider/registry/register-built-in-providers.ts index 8f4cb912..16792c44 100644 --- a/src/llm/provider/registry/register-built-in-providers.ts +++ b/src/llm/provider/registry/register-built-in-providers.ts @@ -69,6 +69,8 @@ export function registerBuiltInProviderKinds(): void { maxImageBytes: config.vision.maxImageBytes, maxImagesPerCall: config.vision.maxImagesPerCall, baseUrlOverride: ctx.entry.url, + ...(ctx.getModelId ? { getModelId: ctx.getModelId } : {}), + logger: ctx.logger, }); }); diff --git a/src/llm/provider/subscription-cli/subscription-cli-provider.test.ts b/src/llm/provider/subscription-cli/subscription-cli-provider.test.ts index 7abcc3c6..9d2abdc1 100644 --- a/src/llm/provider/subscription-cli/subscription-cli-provider.test.ts +++ b/src/llm/provider/subscription-cli/subscription-cli-provider.test.ts @@ -45,13 +45,13 @@ function buildOptions(overrides: Record = {}) { } describe("SubscriptionCliProvider capabilities", () => { - it("declares the native transport with no vision and no slot affinity", () => { + it("declares the text (JSON-array) transport with no vision and no slot affinity", () => { const provider = makeProvider(); - // native_tools, despite never returning tool_calls: it routes - // step-executor down its guarded recovery ladder instead of the - // repair path, which would cost a second CLI invocation. - expect(provider.capabilities.toolTransport).toBe("native_tools"); - expect(provider.toolCallAdapter).not.toBeNull(); + // The tools are never forwarded to the CLI, so the prompt must ask + // for the JSON-array tool call rather than tell the model never to + // write tool JSON as text (F32). + expect(provider.capabilities.toolTransport).toBe("grammar"); + expect(provider.toolCallAdapter).toBeNull(); expect(provider.streamConsumer).toBeNull(); expect(provider.capabilities.vision).toBe(false); expect(provider.capabilities.supportsSlotAffinity).toBe(false); diff --git a/src/llm/provider/subscription-cli/subscription-cli-provider.ts b/src/llm/provider/subscription-cli/subscription-cli-provider.ts index 924dfeb4..8381d55b 100644 --- a/src/llm/provider/subscription-cli/subscription-cli-provider.ts +++ b/src/llm/provider/subscription-cli/subscription-cli-provider.ts @@ -15,7 +15,6 @@ import type { } from "../llm-provider.js"; import { VisionUnsupportedError } from "../llm-provider.js"; import type { ToolCallAdapter } from "../adapters/tool-call-adapter.js"; -import { openAiToolCallAdapter } from "../openai/openai-tool-call-adapter.js"; import type { CliAdapterDescriptor } from "./cli-adapter-descriptor.js"; import { resolveCliBinary } from "./resolve-cli-binary.js"; import { runCliCommand, type CliRunner } from "./run-cli-completion.js"; @@ -65,16 +64,18 @@ export class SubscriptionCliProvider implements LlmProvider { readonly name: string; readonly capabilities: ProviderCapabilities; /** - * We never return `tool_calls`, yet the transport is `native_tools` - * and the adapter is present on purpose. On the grammar transport a - * format drift throws out of `parseToolCalls` and costs a second full - * CLI invocation on the repair path; on the native transport an empty - * `toolCalls` sends step-executor down its guarded recovery ladder, - * which parses the tool-call JSON out of `content` inside a - * try/catch and otherwise wraps the prose as a `reply`. Same result - * when the model complies, no extra process when it does not. + * The tools are never forwarded to the CLI (no `tools` payload, no + * MCP), so the transport it really serves is the text one: the prompt + * asks for the JSON-array tool call and `parseToolCalls` reads it out + * of `content`. It used to declare `native_tools` — which made the + * persona say "never write tool-call JSON as text" to a model that + * had no other way to call a tool, while Codex's steering text asked + * for the JSON array. The price is that a format drift now takes the + * parser's one-shot retry (a second CLI invocation) instead of the + * native recovery ladder; the gain is a prompt that agrees with the + * wire. See F32. */ - readonly toolCallAdapter: ToolCallAdapter = openAiToolCallAdapter; + readonly toolCallAdapter: ToolCallAdapter | null = null; /** Streaming is owned end to end here; that seam consumes SSE bytes. */ readonly streamConsumer = null; @@ -111,7 +112,7 @@ export class SubscriptionCliProvider implements LlmProvider { this.capabilities = { vision: false, visionSource: "config-disabled", - toolTransport: "native_tools", + toolTransport: "grammar", contextWindow: descriptor.contextWindow, supportsParallelTools: false, // Every completion is a fresh process; there is no slot to pin. diff --git a/src/llm/reliability/index.ts b/src/llm/reliability/index.ts index b6e092f6..dce773d9 100644 --- a/src/llm/reliability/index.ts +++ b/src/llm/reliability/index.ts @@ -16,6 +16,10 @@ export { export type { LlmFailureOptions, ModelErrorOptions } from "./llm-failures.js"; export { classifyFailure } from "./classify-failure.js"; export { isRequestSizeRejection } from "./request-size-rejection.js"; +export { + readProviderErrorVerdict, + type ProviderErrorVerdict, +} from "./provider-error-verdict.js"; // `looksLikeDroppedConnection` is deliberately NOT re-exported: it is the // classifier's own key, used inside `network-error.ts` by `isNetworkError` // and asserted directly by that module's test, with no consumer outside diff --git a/src/llm/reliability/provider-error-verdict.test.ts b/src/llm/reliability/provider-error-verdict.test.ts new file mode 100644 index 00000000..f9c3e52d --- /dev/null +++ b/src/llm/reliability/provider-error-verdict.test.ts @@ -0,0 +1,95 @@ +import { describe, expect, it } from "vitest"; +import { OpenAiHttpError } from "../provider/openai/openai-http.js"; +import { + parseProviderErrorBody, + RETRY_HINT_MAX_MS, +} from "../provider/openai/parse-provider-error-body.js"; +import { TransportError } from "./llm-failures.js"; +import { readProviderErrorVerdict } from "./provider-error-verdict.js"; + +function httpError( + status: number, + text: string, + options: { label?: string; retryAfterMs?: number | null } = {}, +): OpenAiHttpError { + return new OpenAiHttpError( + `openai provider ${status}: ${text.slice(0, 300)}`, + status, + "https://openrouter.ai/api/v1/chat/completions", + false, + options.retryAfterMs ?? null, + options.label ?? "openrouter", + undefined, + { body: parseProviderErrorBody(text) }, + ); +} + +describe("readProviderErrorVerdict", () => { + it("reads through the step executor's TransportError wrapper to the provider's body", () => { + const wrapped = new TransportError( + '"openrouter" is rate-limiting this key (429).', + 429, + "https://openrouter.ai/api/v1", + { + cause: httpError( + 429, + JSON.stringify({ + error: { + message: "Provider returned error", + metadata: { + raw: '{"error":{"type":"credit_balance_exhausted","message":"Your credit balance is too low to access the Anthropic API."}}', + }, + }, + }), + ), + }, + ); + expect(readProviderErrorVerdict(wrapped)).toEqual({ + kind: "credit_exhausted", + provider: "openrouter", + code: "credit_balance_exhausted", + detail: "Provider returned error", + }); + }); + + it("names the host when the provider has no label", () => { + const verdict = readProviderErrorVerdict( + httpError(402, JSON.stringify({ error: { message: "no credits left" } }), { + label: "", + }), + ); + expect(verdict).toMatchObject({ + kind: "credit_exhausted", + provider: "openrouter.ai", + }); + }); + + it("clips a cooldown to the maximum a turn will wait", () => { + const verdict = readProviderErrorVerdict( + httpError( + 402, + JSON.stringify({ + error: { + code: "in_flight_budget_exhausted", + message: "retry in 10 minutes", + }, + }), + ), + ); + expect(verdict).toEqual({ + kind: "retry_after", + provider: "openrouter", + delayMs: RETRY_HINT_MAX_MS, + code: "in_flight_budget_exhausted", + detail: "retry in 10 minutes", + }); + }); + + it("is null for a plain outage, a non-HTTP error, and a deep cause chain", () => { + expect(readProviderErrorVerdict(httpError(503, "upstream down"))).toBeNull(); + expect(readProviderErrorVerdict(new Error("fetch failed"))).toBeNull(); + let deep: Error = httpError(402, JSON.stringify({ error: { code: "insufficient_credits" } })); + for (let i = 0; i < 6; i += 1) deep = new Error(`layer ${i}`, { cause: deep }); + expect(readProviderErrorVerdict(deep)).toBeNull(); + }); +}); diff --git a/src/llm/reliability/provider-error-verdict.ts b/src/llm/reliability/provider-error-verdict.ts new file mode 100644 index 00000000..e1c25a50 --- /dev/null +++ b/src/llm/reliability/provider-error-verdict.ts @@ -0,0 +1,91 @@ +import { OpenAiHttpError } from "../provider/openai/openai-http.js"; +import { + readProviderErrorReason, + RETRY_HINT_MAX_MS, +} from "../provider/openai/parse-provider-error-body.js"; + +/** + * What a thrown provider failure means for the turn, read from the + * error body rather than the status: + * + * - `credit_exhausted`: the account cannot pay. No retry, no fallover + * wait — the turn stops where it is, resumable after a top-up, and + * the operator is told which provider said so. + * - `retry_after`: the provider asked for a cooldown (a `retry-after` + * header, `RetryInfo`, "retry in 120 s" in the text, or OpenRouter's + * `in_flight_budget_exhausted`). The turn waits that long — clipped + * to `RETRY_HINT_MAX_MS` — and retries the same step. + * + * A plain 429 or 5xx with neither reads as `null` and keeps the + * existing outage park. + */ +export type ProviderErrorVerdict = + | { + kind: "credit_exhausted"; + /** Provider label as the user configured it (or the host). */ + provider: string; + code: string; + detail: string; + } + | { + kind: "retry_after"; + provider: string; + /** Already clipped to `RETRY_HINT_MAX_MS`. */ + delayMs: number; + code: string | null; + detail: string; + }; + +/** Depth cap on the `cause` walk — longer is a cycle. */ +const MAX_CAUSE_DEPTH = 5; + +export function readProviderErrorVerdict( + err: unknown, +): ProviderErrorVerdict | null { + const http = findHttpError(err); + if (http === null) return null; + const reason = readProviderErrorReason({ + status: http.status, + body: http.body, + message: http.message, + retryAfterMs: http.retryAfterMs, + }); + if (reason === null) return null; + const provider = http.providerLabel || hostOf(http.url); + const detail = http.body?.message ?? http.message; + if (reason.kind === "credit_exhausted") { + return { kind: "credit_exhausted", provider, code: reason.code, detail }; + } + return { + kind: "retry_after", + provider, + delayMs: Math.min(reason.delayMs ?? RETRY_HINT_MAX_MS, RETRY_HINT_MAX_MS), + code: reason.code, + detail, + }; +} + +/** + * The cloud HTTP error behind whatever wrapper reached the loop: the + * step executor wraps it in a `TransportError` with the original on + * `cause`, and the fallback chain rethrows the last link's error as is. + */ +function findHttpError(err: unknown): OpenAiHttpError | null { + let current: unknown = err; + for (let depth = 0; depth < MAX_CAUSE_DEPTH; depth += 1) { + if (current instanceof OpenAiHttpError) return current; + if (typeof current !== "object" || current === null) return null; + const next = (current as { cause?: unknown }).cause; + if (next === current || next === undefined) return null; + current = next; + } + return null; +} + +function hostOf(url: string): string { + try { + return new URL(url).host; + } catch { + return url; + } +} diff --git a/src/llm/reliability/request-size-rejection.test.ts b/src/llm/reliability/request-size-rejection.test.ts index 94da7bb1..814d8c63 100644 --- a/src/llm/reliability/request-size-rejection.test.ts +++ b/src/llm/reliability/request-size-rejection.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { isRequestSizeRejection } from "./request-size-rejection.js"; +import { + isRequestSizeRejection, + readContextLengthFromRejection, + requestSizeRejectionNamesContext, +} from "./request-size-rejection.js"; import { OpenAiHttpError } from "../provider/openai/openai-http.js"; import { LlamaServerError } from "../llama-server-client.js"; import { TransportError } from "./llm-failures.js"; @@ -90,3 +94,40 @@ describe("isRequestSizeRejection", () => { expect(isRequestSizeRejection(new Error("max_tokens"))).toBe(false); }); }); + +describe("readContextLengthFromRejection / requestSizeRejectionNamesContext (F30)", () => { + const wrapped = (body: string) => + new TransportError('"vendor" rejected the request (400).', 400, "u", { + cause: new OpenAiHttpError(`openai provider 400: ${body}`, 400, "u"), + }); + + it.each([ + [ + "This model's maximum context length is 8192 tokens. However, you requested 9134 tokens", + 8_192, + ], + [ + "This endpoint's maximum context length is 131,072 tokens. However, you requested about 140210 tokens", + 131_072, + ], + ["the model has a context length of only 4096 tokens", 4_096], + ["prompt does not fit the 32768-token context window", 32_768], + ["context window: 200000", 200_000], + ])("reads the window out of %s", (body, expected) => { + expect(readContextLengthFromRejection(wrapped(body))).toBe(expected); + expect(requestSizeRejectionNamesContext(wrapped(body))).toBe(true); + }); + + it("never mistakes the requested count or the reply cap for the window", () => { + expect( + readContextLengthFromRejection( + wrapped("the request exceeds the available context size. you requested 9000 tokens"), + ), + ).toBeNull(); + const cap = wrapped( + "max_tokens is too large: 32768. This model supports at most 16384 completion tokens", + ); + expect(readContextLengthFromRejection(cap)).toBeNull(); + expect(requestSizeRejectionNamesContext(cap)).toBe(false); + }); +}); diff --git a/src/llm/reliability/request-size-rejection.ts b/src/llm/reliability/request-size-rejection.ts index b6b9ee37..dc32ce3d 100644 --- a/src/llm/reliability/request-size-rejection.ts +++ b/src/llm/reliability/request-size-rejection.ts @@ -41,6 +41,45 @@ const SIZE_FIELD_WORDING = const SIZE_VERDICT_WORDING = /too large|too long|too many|exceed|maximum|at most|greater than|limit|reduce/i; +/** + * Whether the rejection talks about the context window at all, as + * opposed to the reply cap alone. Read by the size-rejection repack: + * "max_tokens is too large … supports at most 16384 completion tokens" + * is not fixed by a smaller prompt. + */ +export function requestSizeRejectionNamesContext(error: unknown): boolean { + return CONTEXT_WORDING.test(messageChain(error)); +} + +const CONTEXT_WORDING = /context[ _-]?(?:length|window|size)|\bcontext\b/i; + +/** + * The context window the rejection names, in tokens, or null. + * + * OpenAI: "This model's maximum context length is 8192 tokens. However, + * you requested 9134 tokens". OpenRouter: "This endpoint's maximum + * context length is 131072 tokens. However, you requested about 140000 + * tokens". Others: "context length of only 4096 tokens", "32768-token + * context window". The number after "requested" is never the window. + */ +export function readContextLengthFromRejection(error: unknown): number | null { + const text = messageChain(error); + for (const pattern of CONTEXT_LENGTH_PATTERNS) { + const match = pattern.exec(text); + if (match === null) continue; + const value = Number.parseInt(match[1]!.replace(/[,_]/g, ""), 10); + if (Number.isFinite(value) && value > 0) return value; + } + return null; +} + +const CONTEXT_LENGTH_PATTERNS: readonly RegExp[] = [ + /maximum context length (?:is|of) (\d[\d,_]*) tokens/i, + /context (?:length|window|size) (?:is|of|:)?\s*(?:only\s+)?(\d[\d,_]*)\s*tokens?/i, + /(\d[\d,_]*)[- ]token context/i, + /context (?:length|window|size)[^.\d]{0,40}?(\d[\d,_]*)/i, +]; + function statusOf(error: unknown): number | null | undefined { if (error instanceof OpenAiHttpError) return error.status; if (error instanceof LlamaServerError) return error.status; diff --git a/src/llm/server-template-policy.test.ts b/src/llm/server-template-policy.test.ts new file mode 100644 index 00000000..786a1556 --- /dev/null +++ b/src/llm/server-template-policy.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { + GEMMA4_THINK_PROFILE, + PLAIN_INSTRUCT_PROFILE, + QWEN_THINK_PROFILE, +} from "./model-profile.js"; +import { + NO_SERVER_TEMPLATE, + resolveServerTemplatePolicy, +} from "./server-template-policy.js"; + +describe("resolveServerTemplatePolicy", () => { + it("auto: on for families without a hand-built profile, off for gemma and qwen", () => { + const auto = { useServerTemplate: "auto", thinking: "auto" } as const; + expect(resolveServerTemplatePolicy(auto, PLAIN_INSTRUCT_PROFILE)).toEqual({ + useServerTemplate: true, + enableThinking: undefined, + }); + expect(resolveServerTemplatePolicy(auto, QWEN_THINK_PROFILE)).toBe( + NO_SERVER_TEMPLATE, + ); + expect(resolveServerTemplatePolicy(auto, GEMMA4_THINK_PROFILE)).toBe( + NO_SERVER_TEMPLATE, + ); + }); + + it("on / off override the family rule", () => { + expect( + resolveServerTemplatePolicy( + { useServerTemplate: "on", thinking: "auto" }, + QWEN_THINK_PROFILE, + ).useServerTemplate, + ).toBe(true); + expect( + resolveServerTemplatePolicy( + { useServerTemplate: "off", thinking: "off" }, + PLAIN_INSTRUCT_PROFILE, + ), + ).toBe(NO_SERVER_TEMPLATE); + }); + + it("sets the thinking switch only where the template reads it", () => { + const off = { useServerTemplate: "on", thinking: "off" } as const; + expect(resolveServerTemplatePolicy(off, QWEN_THINK_PROFILE)).toEqual({ + useServerTemplate: true, + enableThinking: false, + }); + expect( + resolveServerTemplatePolicy( + { useServerTemplate: "on", thinking: "on" }, + QWEN_THINK_PROFILE, + ).enableThinking, + ).toBe(true); + // A plain template that never mentions enable_thinking: nothing to set. + expect( + resolveServerTemplatePolicy(off, PLAIN_INSTRUCT_PROFILE).enableThinking, + ).toBeUndefined(); + expect( + resolveServerTemplatePolicy(off, { + ...PLAIN_INSTRUCT_PROFILE, + supportsThinkingSwitch: true, + }).enableThinking, + ).toBe(false); + }); +}); diff --git a/src/llm/server-template-policy.ts b/src/llm/server-template-policy.ts new file mode 100644 index 00000000..0b616329 --- /dev/null +++ b/src/llm/server-template-policy.ts @@ -0,0 +1,55 @@ +import type { ModelProfile } from "./model-profile.js"; + +/** + * Whether a local prompt is rendered through the model's own chat + * template (llama-server `POST /apply-template`) and whether the + * template's thinking switch is set. + * + * Only Gemma and Qwen have hand-built prompt profiles; every other GGUF + * ran as `plain-instruct`, with no turn markers and no way to turn + * thinking off. `auto` therefore means: use the server's template for + * the families the runtime does not know, keep the hand-built framing + * (and its KV layout) for the ones it does. `on` / `off` override that + * either way — `on` for a Gemma/Qwen operator who wants the model's own + * template, `off` for a template that misbehaves. + * + * The thinking switch is a template argument (`chat_template_kwargs: + * {enable_thinking}`), so it only exists on the template path and only + * for templates that read it (`profile.supportsThinkingSwitch`). + */ +export type ServerTemplateSetting = "auto" | "on" | "off"; +export type ThinkingSetting = "auto" | "on" | "off"; + +export interface ServerTemplatePolicy { + readonly useServerTemplate: boolean; + /** `undefined` leaves the template's own default in place. */ + readonly enableThinking: boolean | undefined; +} + +export const NO_SERVER_TEMPLATE: ServerTemplatePolicy = { + useServerTemplate: false, + enableThinking: undefined, +}; + +export function resolveServerTemplatePolicy( + localModels: { + useServerTemplate: ServerTemplateSetting; + thinking: ThinkingSetting; + }, + profile: ModelProfile, +): ServerTemplatePolicy { + const setting = localModels.useServerTemplate; + const useServerTemplate = + setting === "on" + ? true + : setting === "off" + ? false + : profile.id === "plain-instruct"; + if (!useServerTemplate) return NO_SERVER_TEMPLATE; + const thinking = localModels.thinking; + const enableThinking = + thinking === "auto" || profile.supportsThinkingSwitch !== true + ? undefined + : thinking === "on"; + return { useServerTemplate, enableThinking }; +} diff --git a/src/prompt/default-tool-args-schemas.ts b/src/prompt/default-tool-args-schemas.ts index 7806a0bf..b65f3b20 100644 --- a/src/prompt/default-tool-args-schemas.ts +++ b/src/prompt/default-tool-args-schemas.ts @@ -206,6 +206,7 @@ const DEFAULT_TOOL_ARGS_SCHEMAS: ReadonlyMap = new Map< anyOf: [stringSchema, stringArraySchema], }, type: stringSchema, + literal: booleanSchema, caseInsensitive: booleanSchema, multiline: booleanSchema, outputMode: { diff --git a/src/prompt/default-tool-descriptors-a.ts b/src/prompt/default-tool-descriptors-a.ts index 8b91c1a2..52657ec2 100644 --- a/src/prompt/default-tool-descriptors-a.ts +++ b/src/prompt/default-tool-descriptors-a.ts @@ -91,7 +91,7 @@ export const DEFAULT_TOOL_DESCRIPTORS_A: readonly ToolDescriptor[] = [ summary: "Regex ripgrep for text search (content, files_with_matches, count). Best on source/text trees. Avoid tree-wide runs with glob *.pdf (or similar) over huge dirs—slow, binary-heavy, often flaky; prefer os.fs.glob by filename + os.fs.read_document on a small candidate set.", argsSchema: - "{ pattern: string, path?: string, glob?: string | string[], type?: string, caseInsensitive?: boolean, multiline?: boolean, outputMode?: 'content' | 'files_with_matches' | 'count', contextBefore?: number, contextAfter?: number, contextAround?: number, headLimit?: number, offset?: number, showLineNumbers?: boolean, timeoutMs?: number }", + "{ pattern: string, path?: string, glob?: string | string[], type?: string, literal?: boolean, caseInsensitive?: boolean, multiline?: boolean, outputMode?: 'content' | 'files_with_matches' | 'count', contextBefore?: number, contextAfter?: number, contextAround?: number, headLimit?: number, offset?: number, showLineNumbers?: boolean, timeoutMs?: number }", }, { name: "os.fs.edit", diff --git a/src/runtime/bootstrap.ts b/src/runtime/bootstrap.ts index 8b5b68af..211ba610 100644 --- a/src/runtime/bootstrap.ts +++ b/src/runtime/bootstrap.ts @@ -97,6 +97,7 @@ import { modelWantsStrictTools } from "../llm/provider/model-strict-tools.js"; import type { ResolvedModel } from "../llm/provider/model-resolver.js"; import { resolveModelPricingFor } from "./resolve-model-pricing.js"; import type { ReasoningEffort } from "../llm/provider/completion-types.js"; +import { LearnedContextWindows } from "./learned-context-windows.js"; import { ProviderFallbackChain, resolveFallbackChain, @@ -1529,6 +1530,7 @@ export async function createAgentRuntime( : undefined; const getLiveProfile = () => profileManager?.getProfile() ?? profile; + const getLiveModelId = () => profileManager?.getModelId() ?? modelAlias; // Issue #112. The manager above is built either way — construction is // pure field assignment, no I/O — because deleting it on a cloud boot @@ -1570,6 +1572,7 @@ export async function createAgentRuntime( config, llamaClient: llama, getProfile: getLiveProfile, + getModelId: getLiveModelId, logger, }); @@ -1676,32 +1679,25 @@ export async function createAgentRuntime( * mid-session is picked up by the next prompt. */ /** - * Context windows the model server revealed by cutting a reply short - * — `completion_truncated` with cause `context_window`, where prompt + - * reply tokens is the window. Keyed by provider and model, kept for the - * life of the process: the same server keeps the same window, and a + * Context windows the model server revealed — by cutting a reply short + * (`completion_truncated` with cause `context_window`, where prompt + + * reply tokens is the window) or by refusing a request as too large + * (`prompt_repacked`). Keyed by provider and model, kept for the life + * of the process: the same server keeps the same window, and a * restart may well change it (llama.cpp `-c`, Lemonade's auto-sizing). * A demonstrated window overrides the catalogue's nominal 128k default * and clamps a real catalogue entry, since a server can run a model - * with less context than the model supports. + * with less context than the model supports. A window only moves + * towards what the server demonstrated — see `LearnedContextWindows`. */ - const observedContextWindows = new Map(); + const observedContextWindows = new LearnedContextWindows(); const activeModelKey = (): string => `${resolveLlmConfig(getConfig()).activeTextProvider}/${resolveActiveModelName()}`; const observeContextWindow = (contextWindow: number): void => { - if (!Number.isFinite(contextWindow) || contextWindow <= 0) return; - const key = activeModelKey(); - const known = observedContextWindows.get(key); - observedContextWindows.set( - key, - known === undefined ? contextWindow : Math.min(known, contextWindow), - ); + observedContextWindows.observe(activeModelKey(), contextWindow); }; - const forgetContextWindowBelow = (tokens: number): void => { - const key = activeModelKey(); - const known = observedContextWindows.get(key); - if (known !== undefined && tokens > known) - observedContextWindows.delete(key); + const raiseContextWindowTo = (tokens: number): void => { + observedContextWindows.raise(activeModelKey(), tokens); }; const resolveCatalogContextWindow = (): number | null => { const observed = observedContextWindows.get(activeModelKey()); @@ -2383,7 +2379,7 @@ export async function createAgentRuntime( // `### fusion` facts state for an external server. liveWorkerSlots: () => slotManager.observedPoolSize(), onContextWindowObserved: observeContextWindow, - onContextWindowExceeded: forgetContextWindowBelow, + onContextWindowExceeded: raiseContextWindowTo, // A pinned turn (`RunTurnOptions.providerId`, a fusion worker on the // local leg) is built for the pinned link's wire shape, not the // active provider's that the four getters below describe. @@ -2672,6 +2668,7 @@ export async function createAgentRuntime( config: getConfig(), llamaClient: llama, getProfile: getLiveProfile, + getModelId: getLiveModelId, logger, }; @@ -2683,6 +2680,7 @@ export async function createAgentRuntime( config: fresh, llamaClient: llama, getProfile: getLiveProfile, + getModelId: getLiveModelId, logger, }); if (added.length > 0) { @@ -2698,6 +2696,7 @@ export async function createAgentRuntime( config: fresh, llamaClient: llama, getProfile: getLiveProfile, + getModelId: getLiveModelId, logger, }); logger.info("llm: provider refreshed", { id }); diff --git a/src/runtime/learned-context-windows.test.ts b/src/runtime/learned-context-windows.test.ts new file mode 100644 index 00000000..daad351b --- /dev/null +++ b/src/runtime/learned-context-windows.test.ts @@ -0,0 +1,33 @@ +import { describe, expect, it } from "vitest"; +import { LearnedContextWindows } from "./learned-context-windows.js"; + +describe("LearnedContextWindows", () => { + it("keeps the smallest observation per key", () => { + const windows = new LearnedContextWindows(); + windows.observe("openrouter/m", 32_768); + windows.observe("openrouter/m", 16_384); + windows.observe("openrouter/m", 24_000); + expect(windows.get("openrouter/m")).toBe(16_384); + expect(windows.get("other/m")).toBeUndefined(); + }); + + it("grows to what the server held instead of forgetting", () => { + // The old reset dropped the observation here and the next prompt + // was packed to the nominal 128k again. + const windows = new LearnedContextWindows(); + windows.observe("k", 16_384); + windows.raise("k", 20_500); + expect(windows.get("k")).toBe(20_500); + windows.raise("k", 18_000); + expect(windows.get("k")).toBe(20_500); + }); + + it("only raises a window it has learned, and ignores nonsense", () => { + const windows = new LearnedContextWindows(); + windows.raise("k", 20_500); + expect(windows.get("k")).toBeUndefined(); + windows.observe("k", Number.NaN); + windows.observe("k", 0); + expect(windows.get("k")).toBeUndefined(); + }); +}); diff --git a/src/runtime/learned-context-windows.ts b/src/runtime/learned-context-windows.ts new file mode 100644 index 00000000..0b61edc2 --- /dev/null +++ b/src/runtime/learned-context-windows.ts @@ -0,0 +1,41 @@ +/** + * Context windows the model server revealed, keyed by provider and + * model, kept for the life of the process. + * + * Two kinds of evidence move a window, in opposite directions: + * + * - `observe(key, window)`: the server cut a reply short at the window, + * or refused a request as too large for it — the window is at most + * this. A smaller observation replaces a larger one. + * - `raise(key, tokens)`: a completion whose prompt + reply exceeded + * the believed window succeeded — the window is at least this. The + * learned window grows to it. + * + * It used to be forgotten instead of raised, which threw the learning + * away the first time the server served one token more than believed + * and sent the next prompt back to the catalogue's nominal 128k — from + * where the next refusal had to learn it all over again. A window only + * ever moves towards what the server demonstrated; it never resets. + */ +export class LearnedContextWindows { + private readonly windows = new Map(); + + /** The learned window for `key`, when any. */ + get(key: string): number | undefined { + return this.windows.get(key); + } + + /** The server showed the window is at most `window`. */ + observe(key: string, window: number): void { + if (!Number.isFinite(window) || window <= 0) return; + const known = this.windows.get(key); + this.windows.set(key, known === undefined ? window : Math.min(known, window)); + } + + /** The server just held `tokens`: the window is at least that. */ + raise(key: string, tokens: number): void { + if (!Number.isFinite(tokens) || tokens <= 0) return; + const known = this.windows.get(key); + if (known !== undefined && tokens > known) this.windows.set(key, tokens); + } +} diff --git a/src/runtime/llm-link-attempt.ts b/src/runtime/llm-link-attempt.ts index 363b4d18..7406cc0b 100644 --- a/src/runtime/llm-link-attempt.ts +++ b/src/runtime/llm-link-attempt.ts @@ -68,6 +68,16 @@ function grammarRequestFields(params: LlmStreamParams) { grammar: params.grammar, slotId: params.slotId, cachePrompt: params.slotId >= 0, + // A grammar link that can honour a Structured Outputs envelope + // (the subscription CLIs stage it as `--json-schema`) still gets + // it; llama-server ignores it in favour of the grammar. + ...(params.responseFormat + ? { responseFormat: params.responseFormat } + : {}), + // The prefix/tail split for a link that renders through the model's + // own template (F31). Built by the step executor only when the + // primary is a grammar link, so it always matches `params.prompt`. + ...(params.chat ? { chat: params.chat } : {}), }; } diff --git a/src/runtime/resolve-model-pricing.test.ts b/src/runtime/resolve-model-pricing.test.ts index b8d66d12..1e6751fb 100644 --- a/src/runtime/resolve-model-pricing.test.ts +++ b/src/runtime/resolve-model-pricing.test.ts @@ -1,4 +1,5 @@ -import { describe, it, expect } from "vitest"; +import { afterEach, describe, it, expect, vi } from "vitest"; +import { refreshOpenRouterChatCatalogFromApi } from "../llm/provider/openrouter/fetch-openrouter-chat-catalog.js"; import { resolveModelPricingFor } from "./resolve-model-pricing.js"; import type { ResolvedLlmConfig } from "../llm/provider/registry/index.js"; @@ -68,3 +69,43 @@ describe("resolveModelPricingFor", () => { expect(resolveModelPricingFor(resolved, null, "cloud")).toBeUndefined(); }); }); + +describe("resolveModelPricingFor — OpenRouter live catalog (F30)", () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + const resolved: ResolvedLlmConfig = { + activeTextProvider: "or", + activeEmbeddingProvider: "local-llama-embed", + toolTransport: "auto", + providers: [{ id: "or", kind: "openrouter", defaultChatModel: "acme/new-model" }], + }; + + it("takes context_length from the live list for a model neither configured nor bundled", async () => { + vi.stubGlobal( + "fetch", + vi.fn(async () => ({ + ok: true, + json: async () => ({ + data: [ + { + id: "acme/new-model", + name: "New", + context_length: 65_536, + pricing: { prompt: "0.000001", completion: "0.000002" }, + supported_parameters: ["tools"], + }, + ], + }), + })), + ); + expect(await refreshOpenRouterChatCatalogFromApi()).toBe(true); + const model = resolveModelPricingFor(resolved, "acme/new-model"); + expect(model?.source).toBe("live"); + expect(model?.contextWindow).toBe(65_536); + expect(model?.pricing).toEqual({ input: 1, output: 2 }); + // Still unknown: not in the live list either. + expect(resolveModelPricingFor(resolved, "acme/other")?.source).toBe("default"); + }); +}); diff --git a/src/runtime/resolve-model-pricing.ts b/src/runtime/resolve-model-pricing.ts index 28bf9a1b..d4678fc4 100644 --- a/src/runtime/resolve-model-pricing.ts +++ b/src/runtime/resolve-model-pricing.ts @@ -1,9 +1,12 @@ import { catalogForProvider } from "../llm/provider/catalog-for-provider.js"; import { resolveModel, + type ModelCatalogEntry, type ResolvedModel, } from "../llm/provider/model-resolver.js"; +import type { LlmProviderConfigEntry } from "../llm/provider/registry/provider-types.js"; import type { ResolvedLlmConfig } from "../llm/provider/registry/index.js"; +import { getCachedOpenRouterChatPicks } from "../llm/provider/openrouter/fetch-openrouter-chat-catalog.js"; /** * Pricing for a model id on one provider, when any is known. @@ -31,5 +34,21 @@ export function resolveModelPricingFor( const id = providerId ?? resolved.activeTextProvider; const entry = resolved.providers.find((p) => p.id === id); if (!entry) return undefined; - return resolveModel(entry, modelId, catalogForProvider(entry)); + const model = resolveModel(entry, modelId, catalogForProvider(entry)); + if (model.source !== "default") return model; + // Nothing configured and nothing bundled: OpenRouter's live model list + // (fetched for the picker, cached for an hour) still knows the row's + // `context_length` and prices. Better than the nominal 128k default, + // against which every prompt would be mis-sized until the first 400. + const live = liveOpenRouterEntry(entry, modelId); + return live === undefined ? model : { ...live, source: "live" }; +} + +function liveOpenRouterEntry( + entry: LlmProviderConfigEntry, + modelId: string, +): ModelCatalogEntry | undefined { + if (entry.kind !== "openrouter") return undefined; + return getCachedOpenRouterChatPicks()?.find((pick) => pick.id === modelId) + ?.entry; } diff --git a/src/tools/argument-error-hint.test.ts b/src/tools/argument-error-hint.test.ts new file mode 100644 index 00000000..60e088b1 --- /dev/null +++ b/src/tools/argument-error-hint.test.ts @@ -0,0 +1,92 @@ +import { describe, expect, it } from "vitest"; +import { describeArgumentError, nearestKey } from "./argument-error-hint.js"; + +describe("describeArgumentError", () => { + it("appends the received keys, the schema keys and the nearest key", () => { + // A local worker sent `patternes` to os.fs.grep. + const hint = describeArgumentError({ + tool: "os.fs.grep", + args: { patternes: ".add(", path: "js" }, + message: "os.fs.grep: `pattern` must be a non-empty string", + }); + expect(hint).not.toBeNull(); + expect(hint!.receivedKeys).toEqual(["patternes", "path"]); + expect(hint!.expectedKeys[0]).toBe("pattern"); + expect(hint!.expectedKeys).toContain("glob"); + expect(hint!.nearest).toEqual([ + { received: "patternes", expected: "pattern" }, + ]); + expect(hint!.message).toBe( + "os.fs.grep: `pattern` must be a non-empty string — " + + `received keys: patternes, path; expected: ${hint!.expectedKeys.join(", ")}; ` + + "did you mean `pattern` instead of `patternes`?", + ); + }); + + it("echoes keys only, never values", () => { + const hint = describeArgumentError({ + tool: "os.fs.write", + args: { pth: "a.js", content: "SECRET CONTENT" }, + message: "os.fs.write: `path` must be a non-empty string", + }); + expect(hint!.message).not.toContain("SECRET"); + expect(hint!.message).toContain("received keys: pth, content"); + expect(hint!.message).toContain("did you mean `path` instead of `pth`?"); + }); + + it("reports no expected keys for a tool without a schema", () => { + const hint = describeArgumentError({ + tool: "mcp.some.server.tool", + args: { q: 1 }, + message: "`query` is required", + }); + expect(hint!.expectedKeys).toEqual([]); + expect(hint!.nearest).toEqual([]); + expect(hint!.message).toBe("`query` is required — received keys: q"); + }); + + it("says (none) when no keys arrived", () => { + const hint = describeArgumentError({ + tool: "os.fs.read", + args: {}, + message: "os.fs.read: `path` must be a non-empty string", + }); + expect(hint!.message).toContain("received keys: (none)"); + }); + + it("leaves a runtime failure alone", () => { + expect( + describeArgumentError({ + tool: "os.fs.read", + args: { path: "nope.txt" }, + message: "ENOENT: no such file or directory, open 'nope.txt'", + }), + ).toBeNull(); + expect( + describeArgumentError({ + tool: "os.fs.grep", + args: { pattern: "x", path: "/gone" }, + message: "os.fs.grep: path does not exist: /gone", + }), + ).toBeNull(); + }); + + it("does not suggest a key that is further than two edits away", () => { + const hint = describeArgumentError({ + tool: "os.fs.read", + args: { filename: "a" }, + message: "os.fs.read: `path` must be a non-empty string", + }); + expect(hint!.nearest).toEqual([]); + expect(hint!.message).not.toContain("did you mean"); + }); +}); + +describe("nearestKey", () => { + it("matches case-insensitively within two edits, first in schema order on a tie", () => { + expect(nearestKey("Path", ["path", "offset"])).toBe("path"); + expect(nearestKey("limits", ["path", "limit"])).toBe("limit"); + expect(nearestKey("ab", ["abc", "abd"])).toBe("abc"); + expect(nearestKey("content", ["path", "offset"])).toBeNull(); + }); +}); diff --git a/src/tools/argument-error-hint.ts b/src/tools/argument-error-hint.ts new file mode 100644 index 00000000..c8b29ef5 --- /dev/null +++ b/src/tools/argument-error-hint.ts @@ -0,0 +1,121 @@ +import { getDefaultArgsJsonSchema } from "../prompt/default-tool-args-schemas.js"; + +/** + * What a tool's argument error should also say: which keys arrived, + * which the tool accepts, and the closest accepted key for any that it + * does not. + * + * A worker once sent `patternes` to `os.fs.grep`, another sent the keys + * `"\"path\""` and `",limit"` to `os.fs.read`. Each got + * "`path` must be a non-empty string" back — true, and useless: the + * message names the missing key but not the key the model actually + * used, so it retried the same call blind. The keys are echoed, never + * the values: a value can be a whole file. + */ +export interface ArgumentErrorHint { + /** The original message with the key report appended. */ + readonly message: string; + readonly receivedKeys: readonly string[]; + /** Empty when the tool has no registered args schema (MCP tools). */ + readonly expectedKeys: readonly string[]; + /** Received keys the tool does not accept, with their closest accepted key. */ + readonly nearest: ReadonlyArray<{ received: string; expected: string }>; +} + +/** Largest edit distance at which a received key still "means" an expected one. */ +export const NEAREST_KEY_MAX_DISTANCE = 2; + +/** + * The wording tools use when they reject their arguments. A backticked + * identifier is the strongest signal (every built-in names the field + * that way); the phrases cover the few that do not. + */ +const ARGUMENT_ERROR_WORDING = + /`[^`]+`|\bmust be\b|\bis required\b|\brequired\b|\bprovide (?:either|a|an|the|one)\b|\bmissing\b|\bunknown (?:arg|argument|field|key|option)\b|\bnot allowed\b/i; + +/** + * The hint for an error a tool threw on its arguments, or `null` when + * the message does not read as an argument error — a runtime failure + * (ENOENT, a timeout, an approval refusal) gets no key report, which + * would only be noise there. + */ +export function describeArgumentError(input: { + tool: string; + args: Record; + message: string; +}): ArgumentErrorHint | null { + if (!ARGUMENT_ERROR_WORDING.test(input.message)) return null; + const receivedKeys = Object.keys(input.args); + const expectedKeys = expectedKeysFor(input.tool); + const expectedSet = new Set(expectedKeys); + const nearest: Array<{ received: string; expected: string }> = []; + for (const received of receivedKeys) { + if (expectedSet.has(received)) continue; + const match = nearestKey(received, expectedKeys); + if (match !== null) nearest.push({ received, expected: match }); + } + const parts = [ + `received keys: ${receivedKeys.length > 0 ? receivedKeys.join(", ") : "(none)"}`, + ]; + if (expectedKeys.length > 0) parts.push(`expected: ${expectedKeys.join(", ")}`); + for (const { received, expected } of nearest) { + parts.push(`did you mean \`${expected}\` instead of \`${received}\`?`); + } + return { + message: `${input.message} — ${parts.join("; ")}`, + receivedKeys, + expectedKeys, + nearest, + }; +} + +function expectedKeysFor(tool: string): string[] { + const schema = getDefaultArgsJsonSchema(tool); + const properties = schema?.properties; + if ( + properties === null || + typeof properties !== "object" || + Array.isArray(properties) + ) { + return []; + } + return Object.keys(properties as Record); +} + +/** + * The expected key closest to `received` within + * `NEAREST_KEY_MAX_DISTANCE`, comparing case-insensitively; ties go to + * the first in schema order. + */ +export function nearestKey( + received: string, + expected: readonly string[], +): string | null { + let best: { key: string; distance: number } | null = null; + const needle = received.toLowerCase(); + for (const key of expected) { + const distance = editDistance(needle, key.toLowerCase()); + if (distance > NEAREST_KEY_MAX_DISTANCE) continue; + if (best === null || distance < best.distance) best = { key, distance }; + } + return best?.key ?? null; +} + +/** Levenshtein distance; keys are short, so the plain two-row form is enough. */ +function editDistance(a: string, b: string): number { + if (a === b) return 0; + if (a.length === 0) return b.length; + if (b.length === 0) return a.length; + let previous = Array.from({ length: b.length + 1 }, (_, i) => i); + for (let i = 1; i <= a.length; i += 1) { + const current = [i]; + for (let j = 1; j <= b.length; j += 1) { + const substitution = previous[j - 1]! + (a[i - 1] === b[j - 1] ? 0 : 1); + current.push( + Math.min(previous[j]! + 1, current[j - 1]! + 1, substitution), + ); + } + previous = current; + } + return previous[b.length]!; +} diff --git a/src/tools/coerce-tool-args.test.ts b/src/tools/coerce-tool-args.test.ts index 568e82dc..f786a67f 100644 --- a/src/tools/coerce-tool-args.test.ts +++ b/src/tools/coerce-tool-args.test.ts @@ -4,7 +4,7 @@ import { type ToolContext, type ToolDefinition, } from "./tool-registry.js"; -import { coerceToolArgs } from "./coerce-tool-args.js"; +import { coerceToolArgs, normalizeKey } from "./coerce-tool-args.js"; const ctx: ToolContext = { workingDir: "/w", @@ -243,3 +243,58 @@ describe("ToolRegistry.invoke integration", () => { ).rejects.toThrow(/tool not registered/); }); }); + +describe("coerceToolArgs — mangled keys (F33)", () => { + it("unquotes a key wrapped in its own quotes", async () => { + // A cloud worker sent `"path"` (quotes included) to os.fs.read. + const seen = await invokeWith("os.fs.read", { '"path"': "a.txt" }); + expect(seen).toEqual({ path: "a.txt" }); + }); + + it("keeps the real key of a fused label fragment", async () => { + const seen = await invokeWith("os.fs.read", { + path: "a.txt", + ",limit": 20, + }); + expect(seen).toEqual({ path: "a.txt", limit: 20 }); + }); + + it("still coerces the value under a repaired key", async () => { + const seen = await invokeWith("os.fs.read", { + path: "a.txt", + "'limit'": "20", + }); + expect(seen).toEqual({ path: "a.txt", limit: 20 }); + }); + + it("never overwrites a key the model also sent cleanly", async () => { + const args = { path: "a.txt", '"path"': "b.txt" }; + expect(await invokeWith("os.fs.read", args)).toEqual(args); + }); + + it("leaves a key alone when its cleaned form is not in the schema", async () => { + const args = { path: "a.txt", '"nope"': 1 }; + expect(await invokeWith("os.fs.read", args)).toEqual(args); + }); + + it("returns the same object when nothing needed repair", () => { + const args = { path: "a.txt", limit: 3 }; + expect(coerceToolArgs("os.fs.read", args)).toBe(args); + }); +}); + +describe("normalizeKey", () => { + it.each([ + ['"path"', "path"], + ["'path'", "path"], + ["`path`", "path"], + ['"\\"path\\""', "path"], + [",limit", "limit"], + [" offset ", "offset"], + ["path,", null], + ["a b", null], + ["", null], + ])("%j → %j", (raw, expected) => { + expect(normalizeKey(raw)).toBe(expected); + }); +}); diff --git a/src/tools/coerce-tool-args.ts b/src/tools/coerce-tool-args.ts index 6a9abdff..e400140d 100644 --- a/src/tools/coerce-tool-args.ts +++ b/src/tools/coerce-tool-args.ts @@ -28,8 +28,9 @@ export function coerceToolArgs( const properties = argsProperties(name); if (!properties) return args; + const normalised = normalizeArgKeys(args, properties); let coerced: Record | null = null; - for (const [key, value] of Object.entries(args)) { + for (const [key, value] of Object.entries(normalised)) { if (typeof value !== "string") continue; const schema = asSchema(properties[key]); if (!schema) continue; @@ -37,10 +38,55 @@ export function coerceToolArgs( const candidate = tryCoerce(value, schema); if (candidate === undefined) continue; - coerced ??= { ...args }; + coerced ??= { ...normalised }; coerced[key] = candidate; } - return coerced ?? args; + return coerced ?? normalised; +} + +/** + * Repairs argument *keys* that arrived mangled, the way models mangle + * them: a key wrapped in its own quotes (`"\"path\""`), and a key fused + * with a fragment of the prompt's markup (`,limit`) — + * both seen from a cloud worker, both rejected as "`path` must be a + * non-empty string" while the value sat under the mangled key. + * + * Do-no-harm again: a key is renamed only when it is not itself in the + * schema, its cleaned form is, and the model did not also send the + * clean key. Anything else is left for the tool to report. + */ +function normalizeArgKeys( + args: Record, + properties: Schema, +): Record { + let fixed: Record | null = null; + for (const [key, value] of Object.entries(args)) { + if (Object.hasOwn(properties, key)) continue; + const clean = normalizeKey(key); + if (clean === null || clean === key) continue; + if (!Object.hasOwn(properties, clean) || Object.hasOwn(args, clean)) { + continue; + } + fixed ??= { ...args }; + delete fixed[key]; + fixed[clean] = value; + } + return fixed ?? args; +} + +const IDENTIFIER = /^[A-Za-z_$][\w$]*$/; + +/** The identifier a mangled key was meant to be, or null when there is none. */ +export function normalizeKey(key: string): string | null { + let clean = key.trim(); + // A fused fragment ends with the real key after the last comma. + const comma = clean.lastIndexOf(","); + if (comma !== -1) clean = clean.slice(comma + 1).trim(); + // Markup that leaked in from the prompt. + clean = clean.replace(/<[^<>]*>/g, "").trim(); + // Quotes of the model's own JSON, one level or several, escaped or not. + clean = clean.replace(/^(?:\\?["'`])+|(?:\\?["'`])+$/g, "").trim(); + return IDENTIFIER.test(clean) ? clean : null; } /** diff --git a/src/tools/fusion/worker-result.test.ts b/src/tools/fusion/worker-result.test.ts index bfc75e5b..dc4e0dea 100644 --- a/src/tools/fusion/worker-result.test.ts +++ b/src/tools/fusion/worker-result.test.ts @@ -557,6 +557,27 @@ describe("WorkerRunCollector — why a worker stopped", () => { expect(row).not.toHaveProperty("hint"); }); + it("fails a worker whose provider ran out of credit, with the quota hint", () => { + const c = new WorkerRunCollector(); + c.observe({ + type: "credit_exhausted", + provider: "openrouter", + code: "credit_balance_exhausted", + message: "Your credit balance is too low", + }); + const result = c.finish({ + ...base, + reason: "max_steps", + stopCause: "credit_exhausted", + }); + expect(result.status).toBe("failed"); + expect(result.error).toBe( + '"openrouter" is out of credit: Your credit balance is too low', + ); + expect(result.hint).toBe(WORKER_HINT_QUOTA); + expect(result).not.toHaveProperty("notes"); + }); + it("notes that a reply on the forced final step may describe undone work", () => { const c = new WorkerRunCollector(); c.observe( diff --git a/src/tools/fusion/worker-result.ts b/src/tools/fusion/worker-result.ts index ed47f8be..7aeeaa9b 100644 --- a/src/tools/fusion/worker-result.ts +++ b/src/tools/fusion/worker-result.ts @@ -178,6 +178,13 @@ export class WorkerRunCollector { this.lastWaitReason = undefined; return; } + if (event.type === "credit_exhausted") { + // The loop pauses the worker's turn resumable, but a worker is + // never resumed: for the orchestrator this is a failed task with + // the reason, not a stopped one. + this.lastLoopError = `"${event.provider}" is out of credit: ${event.message}`; + return; + } if (event.type !== "llm_event") return; const inner = event.event; if (inner.type === "assistant_reply") { @@ -300,6 +307,9 @@ export class WorkerRunCollector { } function stopCauseNote(cause: WorkerStopCause, stepCount: number): string { + if (cause === "credit_exhausted") { + return "stopped because the provider is out of credit"; + } if (cause === "time_ceiling") { return "stopped at its time limit; any reply was written on the forced final step and may describe work that was not done"; } @@ -337,6 +347,9 @@ export function classifyWorkerStatus( ): WorkerTaskStatus { if (reason === null || reason === "failed") return "failed"; if (reason === "cancelled") return "cancelled"; + // Out of credit is not a ceiling the worker ran into; nothing it + // wrote after that point exists, and re-delegating cannot help. + if (stopCause === "credit_exhausted") return "failed"; if (approvalRefused) return "needs_orchestrator"; if (reason === "max_steps" || stopCause !== undefined) return "max_steps"; return "ok"; diff --git a/src/tools/os/fs-content-check.test.ts b/src/tools/os/fs-content-check.test.ts new file mode 100644 index 00000000..64144c5d --- /dev/null +++ b/src/tools/os/fs-content-check.test.ts @@ -0,0 +1,240 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; +import { mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { ApprovalGate } from "../../approval/approval-gate.js"; +import type { ToolContext } from "../tool-registry.js"; +import { + checkChangedFile, + checkWrittenContent, + DOUBLE_ESCAPE_MIN_LITERALS, + newContentWarnings, +} from "./fs-content-check.js"; +import { buildOsFsEditTool } from "./fs-edit.js"; +import { buildOsFsWriteTool } from "./fs-write.js"; + +const PAGE = [ + "", + "", + "t", + "", + '', + '', + '', + "", + "", + "", +].join("\n"); + +/** The same page with the inline script closed: nothing to warn about. */ +const PAGE_OK = PAGE.replace(" },\n", " },\n };\n"); + +describe("checkWrittenContent — HTML", () => { + it("parses inline classic scripts and reports the line in the file", () => { + const warnings = checkWrittenContent("/w/index.html", PAGE); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toMatchObject({ kind: "html_script", key: "script4" }); + // The block opens on line 8; the unclosed object is reported at its + // end, line 13 in the file — not line 6 inside the block. + expect(warnings[0]!.message).toBe( + "inline