From cd6ba0214f862219e40c785bf85b6dfb239eddcd Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 30 Jun 2026 13:20:11 +0000 Subject: [PATCH] fix(ai): hardened ollama malformed tool-call handling - Added a fetchWithRetry response-body gate so deterministic 5xx provider failures can skip retry delays. - Stopped retrying llama.cpp malformed tool-call JSON responses from local Ollama and surfaced recovery guidance. - Covered the Ollama parse-failure path and generic retry gate behavior. Fixes #3899 --- packages/ai/CHANGELOG.md | 4 ++ packages/ai/src/error/format.ts | 13 ++++- packages/ai/src/providers/ollama.ts | 9 ++++ .../ollama-tool-call-json-parse-error.test.ts | 54 +++++++++++++++++++ packages/utils/CHANGELOG.md | 4 ++ packages/utils/src/fetch-retry.ts | 13 ++++- packages/utils/test/fetch-retry.test.ts | 19 +++++++ 7 files changed, 113 insertions(+), 3 deletions(-) create mode 100644 packages/ai/test/ollama-tool-call-json-parse-error.test.ts diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index cc98662b5..cfee2e274 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed local Ollama/llama.cpp malformed tool-call JSON failures being retried as generic 500 errors, surfacing a clearer recovery message instead. ([#3899](https://github.com/can1357/oh-my-pi/issues/3899)) + ## [16.2.7] - 2026-06-30 ### Added diff --git a/packages/ai/src/error/format.ts b/packages/ai/src/error/format.ts index 7e9765c8c..1b55e95cb 100644 --- a/packages/ai/src/error/format.ts +++ b/packages/ai/src/error/format.ts @@ -6,13 +6,21 @@ import { } from "../utils/http-inspector"; import { formatErrorMessageWithRetryAfter } from "../utils/retry-after"; +const OLLAMA_TOOL_CALL_ARGUMENTS_PARSE_PATTERN = + /failed to parse tool call arguments as json|\[json\.exception\.parse_error\.101\]/i; + +function rewriteOllamaToolCallJsonError(message: string): string { + if (!OLLAMA_TOOL_CALL_ARGUMENTS_PARSE_PATTERN.test(message)) return message; + return `Local Ollama model emitted malformed tool-call JSON and llama.cpp rejected it (HTTP 500). This is usually a deterministic model-output failure after context degradation, not a transient server outage; reload the model or reduce context, then retry.\n${message}`; +} + /** Inputs that steer {@link formatMessage}'s formatter selection. */ export interface FormatMessageOptions { /** When present, the raw request is dumped into the message for 400-class failures. */ rawRequestDump?: RawHttpRequestDump; /** Captured non-2xx response body, appended to the message when available. */ capturedErrorResponse?: CapturedHttpErrorResponse; - /** Provider id; `"github-copilot"` triggers the copilot message rewrite. */ + /** Provider id; gates provider-specific user-facing rewrites. */ provider?: string; } @@ -32,5 +40,8 @@ export async function formatMessage(error: unknown, opts: FormatMessageOptions = if (opts.provider === "github-copilot") { message = rewriteCopilotError(message, error, opts.provider); } + if (opts.provider === "ollama") { + message = rewriteOllamaToolCallJsonError(message); + } return message; } diff --git a/packages/ai/src/providers/ollama.ts b/packages/ai/src/providers/ollama.ts index d46a50453..fdc31ad7f 100644 --- a/packages/ai/src/providers/ollama.ts +++ b/packages/ai/src/providers/ollama.ts @@ -323,6 +323,13 @@ function createChatBody(model: Model<"ollama-chat">, context: Context, options: }; } +const OLLAMA_TOOL_CALL_ARGUMENTS_PARSE_PATTERN = + /failed to parse tool call arguments as json|\[json\.exception\.parse_error\.101\]/i; + +function shouldRetryOllamaResponse(response: Response, bodyText: string): boolean { + return response.status < 500 || !OLLAMA_TOOL_CALL_ARGUMENTS_PARSE_PATTERN.test(bodyText); +} + async function captureHttpErrorResponse(response: Response): Promise { let bodyText: string | undefined; let bodyJson: unknown; @@ -598,6 +605,7 @@ export const streamOllama: StreamFunction<"ollama-chat"> = ( body: JSON.stringify(body), signal: watchdog.signal, defaultDelayMs: OLLAMA_RETRY_DELAYS_MS, + shouldRetryResponse: shouldRetryOllamaResponse, fetch: options.fetch, timeout: false, }); @@ -747,6 +755,7 @@ export const streamOllama: StreamFunction<"ollama-chat"> = ( } const result = await AIError.finalize(error, { api: model.api, + provider: model.provider, signal: options.signal, rawRequestDump, capturedErrorResponse, diff --git a/packages/ai/test/ollama-tool-call-json-parse-error.test.ts b/packages/ai/test/ollama-tool-call-json-parse-error.test.ts new file mode 100644 index 000000000..b1438b6b6 --- /dev/null +++ b/packages/ai/test/ollama-tool-call-json-parse-error.test.ts @@ -0,0 +1,54 @@ +import { afterEach, describe, expect, it, vi } from "bun:test"; +import { scheduler } from "node:timers/promises"; +import { streamOllama } from "@oh-my-pi/pi-ai/providers/ollama"; +import type { Context, Model } from "@oh-my-pi/pi-ai/types"; +import { buildModel } from "@oh-my-pi/pi-catalog/build"; + +const model: Model<"ollama-chat"> = buildModel({ + id: "qwen3.6-coder:27b", + name: "Qwen 3.6 Coder 27B", + api: "ollama-chat", + provider: "ollama", + baseUrl: "http://127.0.0.1:11434", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 131_072, + maxTokens: 8192, +}); + +const context: Context = { + messages: [{ role: "user", content: "Use bash to run a Python command.", timestamp: 0 }], +}; + +const llamaToolParseFailure = JSON.stringify({ + error: { + code: 500, + message: + "Failed to parse tool call arguments as JSON: [json.exception.parse_error.101] parse error at line 1, column 557: syntax error while parsing value - invalid string: missing closing quote; last read: '\"uv run python -c \\\"\\nimport jax.numpy as jnp\\nimport'", + }, +}); + +describe("Ollama malformed tool-call JSON errors", () => { + afterEach(() => vi.restoreAllMocks()); + + it("does not retry deterministic llama.cpp tool argument parse failures", async () => { + vi.spyOn(scheduler, "wait").mockResolvedValue(undefined); + let calls = 0; + const fetchMock = async () => { + calls += 1; + return new Response(llamaToolParseFailure, { status: 500 }); + }; + + const result = await streamOllama(model, context, { + apiKey: "ollama", + fetch: fetchMock, + }).result(); + + expect(calls).toBe(1); + expect(result.stopReason).toBe("error"); + expect(result.errorStatus).toBe(500); + expect(result.errorMessage).toContain("Local Ollama model emitted malformed tool-call JSON"); + expect(result.errorMessage).toContain("reload the model"); + }); +}); diff --git a/packages/utils/CHANGELOG.md b/packages/utils/CHANGELOG.md index 30553986d..3d13c99f8 100644 --- a/packages/utils/CHANGELOG.md +++ b/packages/utils/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- Added a response-body retry gate to `fetchWithRetry()` for deterministic provider failures that return retryable HTTP statuses. + ## [16.2.7] - 2026-06-30 ### Added diff --git a/packages/utils/src/fetch-retry.ts b/packages/utils/src/fetch-retry.ts index 19f69ccc0..c2c8e0c7f 100644 --- a/packages/utils/src/fetch-retry.ts +++ b/packages/utils/src/fetch-retry.ts @@ -129,6 +129,12 @@ export interface FetchWithRetryOptions extends RequestInit { * mock during tests. */ fetch?: (input: string | URL | Request, init?: RequestInit) => Promise; + /** + * Optional retry gate for HTTP responses whose status is retryable. Receives a + * cloned body string so callers can fail fast on deterministic provider + * failures that happen to use a 5xx status. + */ + shouldRetryResponse?: (response: Response, bodyText: string, attempt: number) => boolean | Promise; /** * Bun extension forwarded verbatim to the underlying `fetch` call. `false` * disables Bun's native ~300s pre-response timeout (callers that own a @@ -160,6 +166,7 @@ export async function fetchWithRetry( maxDelayMs = DEFAULT_MAX_DELAY_MS, defaultDelayMs, prepareInit, + shouldRetryResponse, fetch: fetchImpl = fetch, timeout = false, ...baseInit @@ -196,8 +203,10 @@ export async function fetchWithRetry( if (!isRetryableStatus(response.status)) return response; if (attempt + 1 >= maxAttempts) return response; - const hint = extractRetryHint(response, await response.clone().text()); - if (hint !== undefined && hint > maxDelayMs) return response; + const retryBody = await response.clone().text(); + if (shouldRetryResponse && !(await shouldRetryResponse(response, retryBody, attempt))) return response; + + const hint = extractRetryHint(response, retryBody); const delayMs = Math.min(hint ?? resolveDefaultDelay(defaultDelayMs, attempt, maxDelayMs), maxDelayMs); await scheduler.wait(delayMs, { signal }); diff --git a/packages/utils/test/fetch-retry.test.ts b/packages/utils/test/fetch-retry.test.ts index a47bf38ee..cb5491cc9 100644 --- a/packages/utils/test/fetch-retry.test.ts +++ b/packages/utils/test/fetch-retry.test.ts @@ -40,4 +40,23 @@ describe("fetchWithRetry", () => { expect(await response.text()).toBe("done"); expect(attempt).toBe(2); }); + + it("lets callers stop retries for deterministic response bodies", async () => { + let attempt = 0; + const customFetch = async () => { + attempt += 1; + return new Response("deterministic provider failure", { status: 500 }); + }; + + const response = await fetchWithRetry("https://example.invalid/z", { + fetch: customFetch, + defaultDelayMs: 1, + maxAttempts: 3, + shouldRetryResponse: (_response, bodyText) => !bodyText.includes("deterministic"), + }); + + expect(response.status).toBe(500); + expect(await response.text()).toBe("deterministic provider failure"); + expect(attempt).toBe(1); + }); });