fix(ollama): address code review findings for native API (#11828)

- Handle SDK "toolResult" role (camelCase) in message conversion
- Replace module-level mutable counter with crypto.randomUUID()
- Extract and pass tools from context to Ollama request body
- Unify duplicate OLLAMA_BASE_URL constants
- Remove unused SimpleStreamOptions import
- Add warning logs for malformed NDJSON lines
- Fix tool call test assertions (empty content, UUID format)
This commit is contained in:
BrokenFinger98 2026-02-08 21:36:52 +09:00 committed by Peter Steinberger
parent 9b4419488a
commit b6d7bd9923
3 changed files with 57 additions and 14 deletions

View File

@ -22,6 +22,7 @@ import {
SYNTHETIC_BASE_URL,
SYNTHETIC_MODEL_CATALOG,
} from "./synthetic-models.js";
import { OLLAMA_NATIVE_BASE_URL } from "./ollama-stream.js";
import {
TOGETHER_BASE_URL,
TOGETHER_MODEL_CATALOG,
@ -79,7 +80,7 @@ const QWEN_PORTAL_DEFAULT_COST = {
cacheWrite: 0,
};
const OLLAMA_BASE_URL = "http://127.0.0.1:11434";
const OLLAMA_BASE_URL = OLLAMA_NATIVE_BASE_URL;
const OLLAMA_API_BASE_URL = OLLAMA_BASE_URL;
const OLLAMA_DEFAULT_CONTEXT_WINDOW = 128000;
const OLLAMA_DEFAULT_MAX_TOKENS = 8192;

View File

@ -49,12 +49,18 @@ describe("convertToOllamaMessages", () => {
]);
});
it("converts tool result messages", () => {
it("converts tool result messages with 'tool' role", () => {
const messages = [{ role: "tool", content: "file1.txt\nfile2.txt" }];
const result = convertToOllamaMessages(messages);
expect(result).toEqual([{ role: "tool", content: "file1.txt\nfile2.txt" }]);
});
it("converts SDK 'toolResult' role to Ollama 'tool' role", () => {
const messages = [{ role: "toolResult", content: "command output here" }];
const result = convertToOllamaMessages(messages);
expect(result).toEqual([{ role: "tool", content: "command output here" }]);
});
it("handles empty messages array", () => {
const result = convertToOllamaMessages([]);
expect(result).toEqual([]);
@ -99,11 +105,12 @@ describe("buildAssistantMessage", () => {
};
const result = buildAssistantMessage(response, modelInfo);
expect(result.stopReason).toBe("end_turn");
expect(result.content.length).toBe(2); // empty text + tool_use
expect(result.content[1].type).toBe("tool_use");
const toolUse = result.content[1] as { type: "tool_use"; name: string; input: Record<string, unknown> };
expect(result.content.length).toBe(1); // tool_use only (empty content is skipped)
expect(result.content[0].type).toBe("tool_use");
const toolUse = result.content[0] as { type: "tool_use"; id: string; name: string; input: Record<string, unknown> };
expect(toolUse.name).toBe("bash");
expect(toolUse.input).toEqual({ command: "ls -la" });
expect(toolUse.id).toMatch(/^ollama_call_[0-9a-f-]{36}$/);
});
it("sets all costs to zero for local models", () => {

View File

@ -1,5 +1,4 @@
import type { StreamFn } from "@mariozechner/pi-agent-core";
import type { SimpleStreamOptions } from "@mariozechner/pi-ai";
import { AssistantMessageEventStream } from "@mariozechner/pi-ai";
// ── Ollama /api/chat request types ──────────────────────────────────────────
@ -137,7 +136,9 @@ export function convertToOllamaMessages(
content: text,
...(toolCalls.length > 0 ? { tool_calls: toolCalls } : {}),
});
} else if (role === "tool") {
} else if (role === "tool" || role === "toolResult") {
// SDK uses "toolResult" (camelCase) for tool result messages.
// Ollama API expects "tool" role.
const text = extractTextContent(msg.content);
result.push({ role: "tool", content: text });
}
@ -146,6 +147,39 @@ export function convertToOllamaMessages(
return result;
}
// ── Tool extraction ─────────────────────────────────────────────────────────
type SdkToolDef = {
name?: string;
description?: string;
parameters?: Record<string, unknown>;
inputSchema?: Record<string, unknown>;
[key: string]: unknown;
};
function extractOllamaTools(tools: unknown[] | undefined): OllamaTool[] {
if (!tools || !Array.isArray(tools)) {
return [];
}
const result: OllamaTool[] = [];
for (const tool of tools) {
const t = tool as SdkToolDef;
const name = t.name;
if (typeof name !== "string" || !name) {
continue;
}
result.push({
type: "function",
function: {
name,
description: typeof t.description === "string" ? t.description : "",
parameters: t.inputSchema ?? t.parameters ?? {},
},
});
}
return result;
}
// ── Response conversion ─────────────────────────────────────────────────────
interface AssistantMessageLike {
@ -172,8 +206,6 @@ interface AssistantMessageLike {
timestamp: number;
}
let toolCallIdCounter = 0;
export function buildAssistantMessage(
response: OllamaChatResponse,
modelInfo: { api: string; provider: string; id: string },
@ -187,10 +219,9 @@ export function buildAssistantMessage(
const toolCalls = response.message.tool_calls;
if (toolCalls && toolCalls.length > 0) {
for (const tc of toolCalls) {
toolCallIdCounter += 1;
content.push({
type: "tool_use",
id: `ollama_call_${toolCallIdCounter}_${Date.now()}`,
id: `ollama_call_${crypto.randomUUID()}`,
name: tc.function.name,
input: tc.function.arguments,
});
@ -244,7 +275,7 @@ export async function* parseNdjsonStream(
try {
yield JSON.parse(trimmed) as OllamaChatResponse;
} catch {
// Skip malformed lines
console.warn("[ollama-stream] Skipping malformed NDJSON line:", trimmed.slice(0, 120));
}
}
}
@ -253,7 +284,7 @@ export async function* parseNdjsonStream(
try {
yield JSON.parse(buffer.trim()) as OllamaChatResponse;
} catch {
// Skip malformed trailing data
console.warn("[ollama-stream] Skipping malformed trailing data:", buffer.trim().slice(0, 120));
}
}
}
@ -268,13 +299,17 @@ export function createOllamaStreamFn(baseUrl: string): StreamFn {
const run = async () => {
try {
const ctx = context as { messages?: AgentMessage[]; system?: string };
const ctx = context as { messages?: AgentMessage[]; system?: string; tools?: unknown[] };
const ollamaMessages = convertToOllamaMessages(ctx.messages ?? [], ctx.system as string);
// Extract tools from context if available and convert to Ollama format.
const ollamaTools = extractOllamaTools(ctx.tools);
const body: OllamaChatRequest = {
model: model.id,
messages: ollamaMessages,
stream: true,
...(ollamaTools.length > 0 ? { tools: ollamaTools } : {}),
...(typeof options?.temperature === "number"
? { options: { temperature: options.temperature } }
: {}),