From cee9b7d473fea789b5097fcdc1db62fe364d40bd Mon Sep 17 00:00:00 2001 From: Joaquin Hui Gomez Date: Tue, 17 Mar 2026 02:25:17 +0000 Subject: [PATCH 1/6] fix: Ollama MCP tool calling for Mistral and Gemma3 models (#9249) Fix two issues with Ollama tool calling: 1. Mistral/Ministral: reorder messages so system messages never follow tool messages (causes "Unexpected role 'system' after role 'tool'") 2. Gemma3: strip unsupported fields (index, id, type) from tool_calls when converting assistant messages to Ollama format (causes "unknown variant 'index'") Also forward assistant toolCalls to Ollama's tool_calls format for proper multi-turn tool calling conversations. Co-Authored-By: Claude Opus 4.6 --- core/llm/llms/Ollama.test.ts | 240 +++++++++++++++++++++++++++++++++++ core/llm/llms/Ollama.ts | 61 ++++++++- 2 files changed, 300 insertions(+), 1 deletion(-) create mode 100644 core/llm/llms/Ollama.test.ts diff --git a/core/llm/llms/Ollama.test.ts b/core/llm/llms/Ollama.test.ts new file mode 100644 index 00000000000..38d6f6efab9 --- /dev/null +++ b/core/llm/llms/Ollama.test.ts @@ -0,0 +1,240 @@ +jest.mock("@continuedev/fetch", () => ({ + streamResponse: jest.fn(), +})); + +jest.mock("../../util/ollamaHelper.js", () => ({ + getRemoteModelInfo: jest.fn(), +})); + +jest.mock("../../util/url.js", () => ({ + extractBase64FromDataUrl: jest.fn(), +})); + +import { ChatMessage } from "../../index.js"; +import Ollama from "./Ollama.js"; + +function createOllama(): Ollama { + // Create instance without triggering constructor's fetch call + const instance = Object.create(Ollama.prototype); + instance.model = "test-model"; + instance.completionOptions = {}; + instance.fetch = jest.fn(); + return instance; +} + +describe("Ollama", () => { + describe("_convertToOllamaMessage", () => { + let ollama: Ollama; + + beforeEach(() => { + ollama = createOllama(); + }); + + it("should convert a basic user message", () => { + const msg: ChatMessage = { role: "user", content: "hello" }; + const result = (ollama as any)._convertToOllamaMessage(msg); + expect(result).toEqual({ role: "user", content: "hello" }); + }); + + it("should convert assistant message with toolCalls, stripping index and other unsupported fields", () => { + const msg: ChatMessage = { + role: "assistant", + content: "", + toolCalls: [ + { + id: "tc_123", + type: "function", + index: 0, // This field causes errors on Gemma3 + function: { + name: "get_weather", + arguments: '{"city":"London"}', + }, + } as any, + ], + }; + const result = (ollama as any)._convertToOllamaMessage(msg); + + expect(result.tool_calls).toBeDefined(); + expect(result.tool_calls).toHaveLength(1); + expect(result.tool_calls[0]).toEqual({ + function: { + name: "get_weather", + arguments: { city: "London" }, + }, + }); + // Verify no index, id, or type fields leaked through + expect(result.tool_calls[0]).not.toHaveProperty("index"); + expect(result.tool_calls[0]).not.toHaveProperty("id"); + expect(result.tool_calls[0]).not.toHaveProperty("type"); + }); + + it("should handle toolCalls with object arguments (not string)", () => { + const msg: ChatMessage = { + role: "assistant", + content: "", + toolCalls: [ + { + id: "tc_456", + type: "function", + function: { + name: "search", + arguments: { query: "test" } as any, + }, + } as any, + ], + }; + const result = (ollama as any)._convertToOllamaMessage(msg); + expect(result.tool_calls[0].function.arguments).toEqual({ + query: "test", + }); + }); + + it("should not add tool_calls for assistant messages without them", () => { + const msg: ChatMessage = { role: "assistant", content: "Sure!" }; + const result = (ollama as any)._convertToOllamaMessage(msg); + expect(result.tool_calls).toBeUndefined(); + }); + + it("should convert tool result messages", () => { + const msg: ChatMessage = { + role: "tool", + content: '{"temp": 20}', + toolCallId: "tc_123", + }; + const result = (ollama as any)._convertToOllamaMessage(msg); + expect(result.role).toBe("tool"); + expect(result.content).toBe('{"temp": 20}'); + }); + + it("should filter out toolCalls without a function name", () => { + const msg: ChatMessage = { + role: "assistant", + content: "", + toolCalls: [ + { + id: "tc_1", + type: "function", + function: { + name: "valid_tool", + arguments: "{}", + }, + }, + { + id: "tc_2", + type: "function", + function: { + name: undefined as any, + arguments: "{}", + }, + }, + ], + }; + const result = (ollama as any)._convertToOllamaMessage(msg); + expect(result.tool_calls).toHaveLength(1); + expect(result.tool_calls[0].function.name).toBe("valid_tool"); + }); + }); + + describe("_reorderMessagesForToolCompat", () => { + let ollama: Ollama; + + beforeEach(() => { + ollama = createOllama(); + }); + + it("should move system message from after tool to before assistant+tool block", () => { + const messages = [ + { role: "system" as const, content: "You are helpful" }, + { role: "user" as const, content: "What's the weather?" }, + { + role: "assistant" as const, + content: "", + tool_calls: [ + { function: { name: "get_weather", arguments: {} } }, + ], + }, + { role: "tool" as const, content: '{"temp": 20}' }, + { role: "system" as const, content: "Use metric units" }, + { role: "user" as const, content: "Thanks" }, + ]; + + const result = (ollama as any)._reorderMessagesForToolCompat(messages); + + // No system message should follow a tool message + for (let i = 1; i < result.length; i++) { + if (result[i].role === "system") { + expect(result[i - 1].role).not.toBe("tool"); + } + } + + // The moved system message should appear before the assistant + const sysIdx = result.findIndex( + (m: any) => m.role === "system" && m.content === "Use metric units", + ); + const assistantIdx = result.findIndex( + (m: any) => m.role === "assistant", + ); + expect(sysIdx).toBeLessThan(assistantIdx); + }); + + it("should not modify messages when no system follows tool", () => { + const messages = [ + { role: "system" as const, content: "You are helpful" }, + { role: "user" as const, content: "Hello" }, + { role: "assistant" as const, content: "Hi there" }, + ]; + + const result = (ollama as any)._reorderMessagesForToolCompat(messages); + expect(result).toEqual(messages); + }); + + it("should handle multiple tool results before a system message", () => { + const messages = [ + { role: "user" as const, content: "Do two things" }, + { + role: "assistant" as const, + content: "", + tool_calls: [ + { function: { name: "tool1", arguments: {} } }, + { function: { name: "tool2", arguments: {} } }, + ], + }, + { role: "tool" as const, content: "result1" }, + { role: "tool" as const, content: "result2" }, + { role: "system" as const, content: "extra instructions" }, + ]; + + const result = (ollama as any)._reorderMessagesForToolCompat(messages); + + // System should come before the assistant message + const sysIdx = result.findIndex( + (m: any) => m.content === "extra instructions", + ); + const assistantIdx = result.findIndex( + (m: any) => m.role === "assistant", + ); + expect(sysIdx).toBeLessThan(assistantIdx); + + // No system message should follow a tool message + for (let i = 1; i < result.length; i++) { + if (result[i].role === "system") { + expect(result[i - 1].role).not.toBe("tool"); + } + } + }); + + it("should handle system message after tool when no preceding assistant", () => { + // Edge case: tool messages without a preceding assistant + const messages = [ + { role: "tool" as const, content: "result" }, + { role: "system" as const, content: "instructions" }, + ]; + + const result = (ollama as any)._reorderMessagesForToolCompat(messages); + + // System should be moved before tool + expect(result[0].role).toBe("system"); + expect(result[1].role).toBe("tool"); + }); + }); +}); diff --git a/core/llm/llms/Ollama.ts b/core/llm/llms/Ollama.ts index d2d8660b644..5934297dce7 100644 --- a/core/llm/llms/Ollama.ts +++ b/core/llm/llms/Ollama.ts @@ -313,6 +313,27 @@ class Ollama extends BaseLLM implements ModelInstaller { }; ollamaMessage.content = renderChatMessage(message); + + // Convert assistant tool calls to Ollama format, stripping unsupported + // fields like `index` (which causes errors on Gemma3 models). + if ( + message.role === "assistant" && + "toolCalls" in message && + message.toolCalls?.length + ) { + ollamaMessage.tool_calls = message.toolCalls + .filter((tc) => tc.function?.name) + .map((tc) => ({ + function: { + name: tc.function!.name!, + arguments: + typeof tc.function!.arguments === "string" + ? JSON.parse(tc.function!.arguments!) + : tc.function!.arguments ?? {}, + }, + })); + } + if (Array.isArray(message.content)) { const images: string[] = []; message.content.forEach((part) => { @@ -409,12 +430,50 @@ class Ollama extends BaseLLM implements ModelInstaller { } } + /** + * Reorder messages so that system messages never appear directly after tool + * messages. Some Ollama models (Mistral, Ministral) reject the sequence + * `tool → system` with "Unexpected role 'system' after role 'tool'". + * This moves such system messages to just before the preceding + * assistant+tool block. + */ + private _reorderMessagesForToolCompat( + messages: OllamaChatMessage[], + ): OllamaChatMessage[] { + const result: OllamaChatMessage[] = [...messages]; + + for (let i = 1; i < result.length; i++) { + if ( + result[i].role === "system" && + result[i - 1].role === "tool" + ) { + // Find the start of the tool block (assistant tool_call + tool results) + let insertIdx = i - 1; + while (insertIdx > 0 && result[insertIdx - 1].role === "tool") { + insertIdx--; + } + // Also skip past the assistant message that triggered the tool calls + if (insertIdx > 0 && result[insertIdx - 1].role === "assistant") { + insertIdx--; + } + + const [sysMsg] = result.splice(i, 1); + result.splice(insertIdx, 0, sysMsg); + // Don't increment i — re-check current position after splice + } + } + + return result; + } + protected async *_streamChat( messages: ChatMessage[], signal: AbortSignal, options: CompletionOptions, ): AsyncGenerator { - const ollamaMessages = messages.map(this._convertToOllamaMessage); + const ollamaMessages = this._reorderMessagesForToolCompat( + messages.map(this._convertToOllamaMessage), + ); const chatOptions: OllamaChatOptions = { model: this._getModel(), messages: ollamaMessages, From 0d5b55eb6eb9cb8b063f24f9bb0ab76ef3d3abba Mon Sep 17 00:00:00 2001 From: Joaquin Hui Gomez Date: Tue, 17 Mar 2026 11:48:56 +0000 Subject: [PATCH 2/6] fix: resolve CI failures in prettier and jest module resolution Remove .js extensions from jest.mock paths so the moduleNameMapper resolves ollamaHelper and url modules correctly. Apply prettier formatting to both changed files. Co-Authored-By: Claude Opus 4.6 --- core/llm/llms/Ollama.test.ts | 16 +++++----------- core/llm/llms/Ollama.ts | 7 ++----- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/core/llm/llms/Ollama.test.ts b/core/llm/llms/Ollama.test.ts index 38d6f6efab9..d2cf269d229 100644 --- a/core/llm/llms/Ollama.test.ts +++ b/core/llm/llms/Ollama.test.ts @@ -2,11 +2,11 @@ jest.mock("@continuedev/fetch", () => ({ streamResponse: jest.fn(), })); -jest.mock("../../util/ollamaHelper.js", () => ({ +jest.mock("../../util/ollamaHelper", () => ({ getRemoteModelInfo: jest.fn(), })); -jest.mock("../../util/url.js", () => ({ +jest.mock("../../util/url", () => ({ extractBase64FromDataUrl: jest.fn(), })); @@ -149,9 +149,7 @@ describe("Ollama", () => { { role: "assistant" as const, content: "", - tool_calls: [ - { function: { name: "get_weather", arguments: {} } }, - ], + tool_calls: [{ function: { name: "get_weather", arguments: {} } }], }, { role: "tool" as const, content: '{"temp": 20}' }, { role: "system" as const, content: "Use metric units" }, @@ -171,9 +169,7 @@ describe("Ollama", () => { const sysIdx = result.findIndex( (m: any) => m.role === "system" && m.content === "Use metric units", ); - const assistantIdx = result.findIndex( - (m: any) => m.role === "assistant", - ); + const assistantIdx = result.findIndex((m: any) => m.role === "assistant"); expect(sysIdx).toBeLessThan(assistantIdx); }); @@ -210,9 +206,7 @@ describe("Ollama", () => { const sysIdx = result.findIndex( (m: any) => m.content === "extra instructions", ); - const assistantIdx = result.findIndex( - (m: any) => m.role === "assistant", - ); + const assistantIdx = result.findIndex((m: any) => m.role === "assistant"); expect(sysIdx).toBeLessThan(assistantIdx); // No system message should follow a tool message diff --git a/core/llm/llms/Ollama.ts b/core/llm/llms/Ollama.ts index 5934297dce7..3bdcfdd1fe7 100644 --- a/core/llm/llms/Ollama.ts +++ b/core/llm/llms/Ollama.ts @@ -329,7 +329,7 @@ class Ollama extends BaseLLM implements ModelInstaller { arguments: typeof tc.function!.arguments === "string" ? JSON.parse(tc.function!.arguments!) - : tc.function!.arguments ?? {}, + : (tc.function!.arguments ?? {}), }, })); } @@ -443,10 +443,7 @@ class Ollama extends BaseLLM implements ModelInstaller { const result: OllamaChatMessage[] = [...messages]; for (let i = 1; i < result.length; i++) { - if ( - result[i].role === "system" && - result[i - 1].role === "tool" - ) { + if (result[i].role === "system" && result[i - 1].role === "tool") { // Find the start of the tool block (assistant tool_call + tool results) let insertIdx = i - 1; while (insertIdx > 0 && result[insertIdx - 1].role === "tool") { From 7f1172de92cc206c63338025106058e51233b6a8 Mon Sep 17 00:00:00 2001 From: Joaquin Hui Gomez Date: Tue, 17 Mar 2026 18:12:39 +0000 Subject: [PATCH 3/6] fix: remove relative-path jest.mock calls that break ESM resolution The jest ESM config resolves relative mock paths from the setup file context, not the test file, causing 'Cannot find module' errors. These mocks are unnecessary since the test bypasses the constructor via Object.create() and only tests pure transformation methods. Co-Authored-By: Claude Opus 4.6 --- core/llm/llms/Ollama.test.ts | 8 -------- 1 file changed, 8 deletions(-) diff --git a/core/llm/llms/Ollama.test.ts b/core/llm/llms/Ollama.test.ts index d2cf269d229..78d69dd157a 100644 --- a/core/llm/llms/Ollama.test.ts +++ b/core/llm/llms/Ollama.test.ts @@ -2,14 +2,6 @@ jest.mock("@continuedev/fetch", () => ({ streamResponse: jest.fn(), })); -jest.mock("../../util/ollamaHelper", () => ({ - getRemoteModelInfo: jest.fn(), -})); - -jest.mock("../../util/url", () => ({ - extractBase64FromDataUrl: jest.fn(), -})); - import { ChatMessage } from "../../index.js"; import Ollama from "./Ollama.js"; From 6383fa58058dfb65a7714027d4eaf614a3029c55 Mon Sep 17 00:00:00 2001 From: Joaquin Hui Gomez Date: Wed, 18 Mar 2026 11:29:30 +0000 Subject: [PATCH 4/6] fix: guard unprotected JSON.parse calls in Ollama.ts Wrap JSON.parse of tool-call arguments in _convertToOllamaMessage and pull-progress lines in installModel with try/catch to prevent unhandled exceptions from aborting the request flow. Addresses Cubic review feedback on PR #11523. Co-Authored-By: Claude Opus 4.6 Signed-off-by: Joaquin Hui Gomez --- core/llm/llms/Ollama.ts | 38 +++++++++++++++++++++++++++----------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/core/llm/llms/Ollama.ts b/core/llm/llms/Ollama.ts index 3bdcfdd1fe7..9719ca89d99 100644 --- a/core/llm/llms/Ollama.ts +++ b/core/llm/llms/Ollama.ts @@ -323,15 +323,27 @@ class Ollama extends BaseLLM implements ModelInstaller { ) { ollamaMessage.tool_calls = message.toolCalls .filter((tc) => tc.function?.name) - .map((tc) => ({ - function: { - name: tc.function!.name!, - arguments: - typeof tc.function!.arguments === "string" - ? JSON.parse(tc.function!.arguments!) - : (tc.function!.arguments ?? {}), - }, - })); + .map((tc) => { + let args: JSONSchema7Object; + if (typeof tc.function!.arguments === "string") { + try { + args = JSON.parse(tc.function!.arguments!); + } catch (e) { + console.warn( + `Failed to parse tool call arguments for "${tc.function!.name}": ${e}`, + ); + args = {}; + } + } else { + args = (tc.function!.arguments as JSONSchema7Object) ?? {}; + } + return { + function: { + name: tc.function!.name!, + arguments: args, + }, + }; + }); } if (Array.isArray(message.content)) { @@ -771,8 +783,12 @@ class Ollama extends BaseLLM implements ModelInstaller { const chunk = new TextDecoder().decode(value); const lines = chunk.split("\n").filter(Boolean); for (const line of lines) { - const data = JSON.parse(line); - progressReporter?.(data.status, data.completed, data.total); + try { + const data = JSON.parse(line); + progressReporter?.(data.status, data.completed, data.total); + } catch (e) { + console.warn(`Error parsing Ollama pull response: ${e}`); + } } } } finally { From 97b012bea2e97e23b7e938b59b87794ee46a1401 Mon Sep 17 00:00:00 2001 From: Joaquin Hui Gomez Date: Wed, 18 Mar 2026 12:04:06 +0000 Subject: [PATCH 5/6] fix: add missing type cast through unknown for ToolCall arguments TypeScript rejects a direct cast from string to JSONSchema7Object. Cast through unknown first to satisfy the type checker. Signed-off-by: Joaquin Co-Authored-By: Claude Opus 4.6 --- core/llm/llms/Ollama.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/core/llm/llms/Ollama.ts b/core/llm/llms/Ollama.ts index 9719ca89d99..99993e247da 100644 --- a/core/llm/llms/Ollama.ts +++ b/core/llm/llms/Ollama.ts @@ -335,7 +335,8 @@ class Ollama extends BaseLLM implements ModelInstaller { args = {}; } } else { - args = (tc.function!.arguments as JSONSchema7Object) ?? {}; + args = + (tc.function!.arguments as unknown as JSONSchema7Object) ?? {}; } return { function: { From a631e6c40510a7ee130809cf61d2d1a1babec40a Mon Sep 17 00:00:00 2001 From: Joaquin Hui Gomez Date: Wed, 18 Mar 2026 15:58:21 +0000 Subject: [PATCH 6/6] fix: guard undefined arguments before JSONSchema7Object cast --- core/llm/llms/Ollama.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/core/llm/llms/Ollama.ts b/core/llm/llms/Ollama.ts index 99993e247da..f72e41185b0 100644 --- a/core/llm/llms/Ollama.ts +++ b/core/llm/llms/Ollama.ts @@ -335,8 +335,9 @@ class Ollama extends BaseLLM implements ModelInstaller { args = {}; } } else { - args = - (tc.function!.arguments as unknown as JSONSchema7Object) ?? {}; + args = tc.function!.arguments + ? (tc.function!.arguments as unknown as JSONSchema7Object) + : {}; } return { function: {