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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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<CapturedHttpErrorResponse> {
|
||||
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,
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
|
||||
@@ -129,6 +129,12 @@ export interface FetchWithRetryOptions extends RequestInit {
|
||||
* mock during tests.
|
||||
*/
|
||||
fetch?: (input: string | URL | Request, init?: RequestInit) => Promise<Response>;
|
||||
/**
|
||||
* 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<boolean>;
|
||||
/**
|
||||
* 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 });
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user