refactor(ipuaro): simplify LLM integration with pure XML tool format

Refactor OllamaClient to use pure XML format for tool calls as
designed in CONCEPT.md. Removes dual system (Ollama native tools +
XML parser) in favor of single source of truth (ResponseParser).

Changes:
- Remove tools parameter from ILLMClient.chat() interface
- Remove convertTools(), convertParameters(), extractToolCalls()
- Add XML format instructions to system prompt with examples
- Add CDATA support in ResponseParser for multiline content
- Add tool name validation with helpful error messages
- Move ToolDef/ToolParameter to shared/types/tool-definitions.ts

Benefits:
- Simplified architecture (single source of truth)
- CONCEPT.md compliance (pure XML as designed)
- Better validation (early detection of invalid tools)
- Reduced complexity (fewer format conversions)

Tests: 1444 passed (+4 new tests)
Coverage: 97.83% lines, 91.98% branches, 99.16% functions
This commit is contained in:
imfozilbek
2025-12-01 21:03:55 +05:00
parent 902d1db831
commit 0433ef102c
13 changed files with 290 additions and 212 deletions

View File

@@ -198,12 +198,12 @@ describe("HandleMessage", () => {
expect(toolMessages.length).toBeGreaterThan(0)
})
it("should return error for unknown tools", async () => {
it("should return error for unregistered tools", async () => {
vi.mocked(mockTools.get).mockReturnValue(undefined)
vi.mocked(mockLLM.chat)
.mockResolvedValueOnce(
createMockLLMResponse(
'<tool_call name="unknown_tool"><param>value</param></tool_call>',
'<tool_call name="get_complexity"><path>src</path></tool_call>',
true,
),
)

View File

@@ -95,53 +95,37 @@ describe("OllamaClient", () => {
)
})
it("should pass tools when provided", async () => {
it("should not pass tools parameter (tools are in system prompt)", async () => {
const client = new OllamaClient(defaultConfig)
const messages = [createUserMessage("Read file")]
const tools = [
{
name: "get_lines",
description: "Get lines from file",
parameters: [
{
name: "path",
type: "string" as const,
description: "File path",
required: true,
},
],
},
]
await client.chat(messages, tools)
await client.chat(messages)
expect(mockOllamaInstance.chat).toHaveBeenCalledWith(
expect.objectContaining({
tools: expect.arrayContaining([
model: "qwen2.5-coder:7b-instruct",
messages: expect.arrayContaining([
expect.objectContaining({
type: "function",
function: expect.objectContaining({
name: "get_lines",
}),
role: "user",
content: "Read file",
}),
]),
}),
)
expect(mockOllamaInstance.chat).toHaveBeenCalledWith(
expect.not.objectContaining({
tools: expect.anything(),
}),
)
})
it("should extract tool calls from response", async () => {
it("should extract tool calls from XML in response content", async () => {
mockOllamaInstance.chat.mockResolvedValue({
message: {
role: "assistant",
content: "",
tool_calls: [
{
function: {
name: "get_lines",
arguments: { path: "src/index.ts" },
},
},
],
content:
'<tool_call name="get_lines"><path>src/index.ts</path></tool_call>',
tool_calls: undefined,
},
eval_count: 30,
})
@@ -424,47 +408,6 @@ describe("OllamaClient", () => {
})
})
describe("tool parameter conversion", () => {
it("should include enum values when present", async () => {
const client = new OllamaClient(defaultConfig)
const messages = [createUserMessage("Get status")]
const tools = [
{
name: "get_status",
description: "Get status",
parameters: [
{
name: "type",
type: "string" as const,
description: "Status type",
required: true,
enum: ["active", "inactive", "pending"],
},
],
},
]
await client.chat(messages, tools)
expect(mockOllamaInstance.chat).toHaveBeenCalledWith(
expect.objectContaining({
tools: expect.arrayContaining([
expect.objectContaining({
function: expect.objectContaining({
parameters: expect.objectContaining({
properties: expect.objectContaining({
type: expect.objectContaining({
enum: ["active", "inactive", "pending"],
}),
}),
}),
}),
}),
]),
}),
)
})
})
describe("error handling", () => {
it("should handle ECONNREFUSED errors", async () => {

View File

@@ -72,7 +72,7 @@ describe("ResponseParser", () => {
})
it("should parse null values", () => {
const response = `<tool_call name="test">
const response = `<tool_call name="get_lines">
<value>null</value>
</tool_call>`
@@ -92,7 +92,7 @@ describe("ResponseParser", () => {
})
it("should parse JSON objects", () => {
const response = `<tool_call name="test">
const response = `<tool_call name="get_lines">
<config>{"key": "value"}</config>
</tool_call>`
@@ -123,6 +123,59 @@ describe("ResponseParser", () => {
start: 5,
})
})
it("should reject unknown tool names", () => {
const response = `<tool_call name="unknown_tool"><path>test.ts</path></tool_call>`
const result = parseToolCalls(response)
expect(result.toolCalls).toHaveLength(0)
expect(result.hasParseErrors).toBe(true)
expect(result.parseErrors[0]).toContain("Unknown tool")
expect(result.parseErrors[0]).toContain("unknown_tool")
})
it("should support CDATA for multiline content", () => {
const response = `<tool_call name="edit_lines">
<path>src/index.ts</path>
<content><![CDATA[const x = 1;
const y = 2;]]></content>
</tool_call>`
const result = parseToolCalls(response)
expect(result.toolCalls[0].params.content).toBe("const x = 1;\nconst y = 2;")
})
it("should handle multiple tool calls with mixed content", () => {
const response = `Some text
<tool_call name="get_lines"><path>a.ts</path></tool_call>
More text
<tool_call name="get_function"><path>b.ts</path><name>foo</name></tool_call>`
const result = parseToolCalls(response)
expect(result.toolCalls).toHaveLength(2)
expect(result.toolCalls[0].name).toBe("get_lines")
expect(result.toolCalls[1].name).toBe("get_function")
expect(result.content).toContain("Some text")
expect(result.content).toContain("More text")
})
it("should handle parse errors gracefully and continue", () => {
const response = `<tool_call name="unknown_tool1"><path>test.ts</path></tool_call>
<tool_call name="get_lines"><path>valid.ts</path></tool_call>
<tool_call name="unknown_tool2"><path>test2.ts</path></tool_call>`
const result = parseToolCalls(response)
expect(result.toolCalls).toHaveLength(1)
expect(result.toolCalls[0].name).toBe("get_lines")
expect(result.hasParseErrors).toBe(true)
expect(result.parseErrors).toHaveLength(2)
expect(result.parseErrors[0]).toContain("unknown_tool1")
expect(result.parseErrors[1]).toContain("unknown_tool2")
})
})
describe("formatToolCallsAsXml", () => {