diff --git a/GIT_STATUS_SNAPSHOT.md b/GIT_STATUS_SNAPSHOT.md new file mode 100644 index 00000000000..2ce2a903060 --- /dev/null +++ b/GIT_STATUS_SNAPSHOT.md @@ -0,0 +1,8 @@ +# Git Status Snapshot + +## Initial State + +- **Detached HEAD at**: dc25669b6 (release: v1.1.1) +- **Original Target Commit**: 0e33a3011f6bef4316b6faed8269a2fd0e6cfb88 +- **Branch containing commit**: fix/ramarivera_glm4.7_interleaved_thinking_fix +- **Untracked files**: packages/opencode/repro_schema.ts diff --git a/bun.lock b/bun.lock index 8c57d8630fc..7744654f10e 100644 --- a/bun.lock +++ b/bun.lock @@ -1,6 +1,5 @@ { "lockfileVersion": 1, - "configVersion": 1, "workspaces": { "": { "name": "opencode", diff --git a/packages/opencode/repro_schema.ts b/packages/opencode/repro_schema.ts new file mode 100644 index 00000000000..1ac71573f13 --- /dev/null +++ b/packages/opencode/repro_schema.ts @@ -0,0 +1,65 @@ +import z from "zod" + +const PermissionAction = z.enum(["ask", "allow", "deny"]) +const PermissionObject = z.record(z.string(), PermissionAction) +const PermissionRule = z.union([PermissionAction, PermissionObject]) + +const Permission = z + .object({ + read: PermissionRule.optional(), + edit: PermissionRule.optional(), + bash: PermissionRule.optional(), + }) + .catchall(PermissionRule) + .or(PermissionAction) + +const Agent = z.object({ + permission: Permission.optional(), +}) + +const Config = z.object({ + agent: z.record(z.string(), Agent).optional(), +}) + +// Test case 1: Valid object permission +const case1 = { + agent: { + plan: { + permission: { + edit: { "*.md": "allow" }, + }, + }, + }, +} + +// Test case 2: Invalid action in object +const case2 = { + agent: { + plan: { + permission: { + edit: { "*.md": "invalid_action" }, + }, + }, + }, +} + +// Test case 4: Object passed to Enum-only schema +const EnumOnly = z.object({ + edit: PermissionAction, +}) +const case4 = { edit: { "*.md": "allow" } } + +console.log("--- Testing Case 1 (Valid Object) ---") +const result1 = Config.safeParse(case1) +if (result1.success) console.log("Success!") +else console.log(JSON.stringify(result1.error.issues, null, 2)) + +console.log("\n--- Testing Case 2 (Invalid Action in Object) ---") +const result2 = Config.safeParse(case2) +if (result2.success) console.log("Success!") +else console.log(JSON.stringify(result2.error.issues, null, 2)) + +console.log("\n--- Testing Case 4 (Object passed to Enum-only) ---") +const result4 = EnumOnly.safeParse(case4) +if (result4.success) console.log("Success!") +else console.log(JSON.stringify(result4.error.issues, null, 2)) diff --git a/packages/opencode/src/provider/transform.ts b/packages/opencode/src/provider/transform.ts index 48011c81a93..674fa7855bd 100644 --- a/packages/opencode/src/provider/transform.ts +++ b/packages/opencode/src/provider/transform.ts @@ -75,6 +75,98 @@ export namespace ProviderTransform { return result } + if ( + model.providerID === "z.ai" || + model.providerID === "zai-coding-plan" || + model.api.id.toLowerCase().includes("glm-4.7") || + model.api.id.toLowerCase().includes("glm-4.6") + ) { + return msgs.map((msg) => { + if (msg.role === "assistant" && Array.isArray(msg.content)) { + const reasoningParts = (msg.content as Array<{ type: string; text?: string }>).filter( + (part) => part.type === "reasoning", + ) as Array<{ type: "reasoning"; text: string }> + const reasoningText = reasoningParts.map((part) => part.text).join("") + + const invokePattern = /([\s\S]*?)<\/invoke>/g + const hasMalformedTools = invokePattern.test(reasoningText) + invokePattern.lastIndex = 0 + + if (hasMalformedTools) { + const toolCalls: Array<{ + type: "tool-call" + toolCallId: string + toolName: string + input: Record + }> = [] + let match + + while ((match = invokePattern.exec(reasoningText)) !== null) { + const toolName = match[1] + const argsText = match[2] + + const argKeyPattern = + /([^<]+)<\/arg_key>\s*([^<]*(?:(?!)[^<]+)*)<\/arg_value>/g + const args: Record = {} + let argMatch = argKeyPattern.exec(argsText) + + if (argMatch) { + argKeyPattern.lastIndex = 0 + while ((argMatch = argKeyPattern.exec(argsText)) !== null) { + const key = argMatch[1].trim() + const value = argMatch[2].trim() + args[key] = value + } + } else { + const directTagPattern = /<(\w+)>([^<]+)<\/\1>/g + let directMatch + while ((directMatch = directTagPattern.exec(argsText)) !== null) { + const key = directMatch[1] + const value = directMatch[2].trim() + if (key !== "invoke" && !key.startsWith("/")) { + args[key] = value + } + } + } + + toolCalls.push({ + type: "tool-call", + toolCallId: `tc_${Date.now()}_${Math.random().toString(36).slice(2, 9)}`, + toolName, + input: args, + }) + } + + const cleanText = reasoningText + .replace(invokePattern, "") + .replace(/[\s\S]*?<\/arg_key>/g, "") + .replace(/[\s\S]*?<\/arg_value>/g, "") + .replace(/[\s\S]*?<\/description>/g, "") + .replace(/<(\w+)>[^<]+<\/\1>/g, "") + .replace(/\s+/g, " ") + .trim() + + const nonReasoningContent = (msg.content as Array).filter((part) => part.type !== "reasoning") + const newContent = [...nonReasoningContent, ...toolCalls] as Array + + if (cleanText.length > 0) { + newContent.push({ + type: "reasoning", + text: cleanText, + }) + } + + return { + ...msg, + content: newContent, + } + } + } + + return msg + }) + } + if ( model.capabilities.interleaved && typeof model.capabilities.interleaved === "object" && diff --git a/packages/opencode/test/provider/test_glm47_thinking_fix.test.ts b/packages/opencode/test/provider/test_glm47_thinking_fix.test.ts new file mode 100644 index 00000000000..84ec3277aef --- /dev/null +++ b/packages/opencode/test/provider/test_glm47_thinking_fix.test.ts @@ -0,0 +1,211 @@ +import { describe, expect, test } from "bun:test" +import type { ModelMessage } from "ai" +import { ProviderTransform } from "../../src/provider/transform" +import type { Provider } from "../../src/provider/provider" + +const createModel = (): Provider.Model => ({ + id: "z.ai/glm-4.7", + providerID: "zai-coding-plan", + api: { + id: "glm-4.7", + url: "https://api.z.ai/api/anthropic", + npm: "@ai-sdk/openai-compatible", + }, + name: "GLM-4.7", + capabilities: { + temperature: true, + reasoning: true, + attachment: false, + toolcall: true, + input: { text: true, audio: false, image: false, video: false, pdf: false }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: { + field: "reasoning_content", + }, + }, + cost: { + input: 0.001, + output: 0.002, + cache: { read: 0.0001, write: 0.0002 }, + }, + limit: { + context: 128000, + output: 8192, + }, + status: "active", + options: {}, + headers: {}, + release_date: "2024-01-01", +}) + +describe("ProviderTransform.message - GLM-4.7 malformed thinking block", () => { + test("GLM-4.7 with tool call XML in reasoning_content should extract malformed tool calls", () => { + const malformedReasoning = `Let me think about what to do here. + + bun test packages/portal/scripts/generate/workflow/workflow.test.ts 2>&1 + Run workflow inference unit tests + +After running the tests, I'll analyze the results.` + + const msgs: ModelMessage[] = [ + { + role: "assistant", + content: [{ type: "reasoning", text: malformedReasoning }], + }, + ] + + const result = ProviderTransform.message(msgs, createModel()) + + // The fix should: + // 1. Extract the tool call from reasoning_content + // 2. Add it as a proper tool-call part + // 3. Clean the reasoning text to remove the XML + + expect(result).toHaveLength(1) + + if (typeof result[0].content === "string") { + throw new Error("Expected content to be an array, not a string") + } + + // Check that tool call was extracted + const toolCalls = result[0].content.filter((part) => part.type === "tool-call") + expect(toolCalls.length).toBe(1) + expect(toolCalls[0].toolName).toBe("bash") + expect(toolCalls[0].input).toEqual({ + command: "bun test packages/portal/scripts/generate/workflow/workflow.test.ts 2>&1", + description: "Run workflow inference unit tests", + }) + + // Check that reasoning was cleaned + const reasoningParts = result[0].content.filter((part) => part.type === "reasoning") + expect(reasoningParts.length).toBe(1) + expect(reasoningParts[0].text).not.toContain(" { + const thinkingBlock = ` + Reviewing Phase 1.8 implementation + 1 + 4 + true +` + + const msgs: ModelMessage[] = [ + { + role: "assistant", + content: [{ type: "reasoning", text: thinkingBlock }], + }, + ] + + const result = ProviderTransform.message(msgs, createModel()) + + // For thinking-specific tools like pal_thinkdeep, we may want to keep them in reasoning + // or extract them - the fix should handle this case appropriately + expect(result).toHaveLength(1) + + if (typeof result[0].content === "string") { + throw new Error("Expected content to be an array, not a string") + } + + const toolCalls = result[0].content.filter((part) => part.type === "tool-call") + expect(toolCalls.length).toBe(1) + expect(toolCalls[0].toolName).toBe("pal_thinkdeep") + expect(toolCalls[0].input).toEqual({ + step: "Reviewing Phase 1.8 implementation", + step_number: "1", + total_steps: "4", + next_step_required: "true", + }) + }) + + test("GLM-4.7 with multiple tool calls in reasoning should extract all", () => { + const malformedReasoning = `I'll need to run several commands: + + ls -la + List files + + + bun install + Install dependencies + + + bun test + Run tests +` + + const msgs: ModelMessage[] = [ + { + role: "assistant", + content: [{ type: "reasoning", text: malformedReasoning }], + }, + ] + + const result = ProviderTransform.message(msgs, createModel()) + + expect(result).toHaveLength(1) + + if (typeof result[0].content === "string") { + throw new Error("Expected content to be an array, not a string") + } + + const toolCalls = result[0].content.filter((part) => part.type === "tool-call") + expect(toolCalls.length).toBe(3) + + expect(toolCalls[0].toolName).toBe("bash") + expect(toolCalls[0].input).toEqual({ + command: "ls -la", + description: "List files", + }) + + expect(toolCalls[1].toolName).toBe("bash") + expect(toolCalls[1].input).toEqual({ + command: "bun install", + description: "Install dependencies", + }) + + expect(toolCalls[2].toolName).toBe("bash") + expect(toolCalls[2].input).toEqual({ + command: "bun test", + description: "Run tests", + }) + }) + + test("Properly formatted GLM-4.7 response should not be affected", () => { + const msgs: ModelMessage[] = [ + { + role: "assistant", + content: [ + { type: "reasoning", text: "Let me check the tests first." }, + { + type: "tool-call", + toolCallId: "test-1", + toolName: "bash", + input: { command: "bun test" }, + }, + { type: "text", text: "Tests passed!" }, + ], + }, + ] + + const result = ProviderTransform.message(msgs, createModel()) + + expect(result).toHaveLength(1) + // Should preserve existing structure + expect(result[0].content).toHaveLength(3) + + if (typeof result[0].content === "string") { + throw new Error("Expected content to be an array, not a string") + } + + const reasoning = result[0].content.filter((part) => part.type === "reasoning") + expect(reasoning.length).toBe(1) + expect(reasoning[0].text).toBe("Let me check the tests first.") + + const toolCalls = result[0].content.filter((part) => part.type === "tool-call") + expect(toolCalls.length).toBe(1) + expect(toolCalls[0].input).toEqual({ command: "bun test" }) + }) +}) diff --git a/ramarivera_glm4.7_interleaved_thinking_fix/session_logs//msg_example.json b/ramarivera_glm4.7_interleaved_thinking_fix/session_logs//msg_example.json new file mode 100644 index 00000000000..5720a7aa170 --- /dev/null +++ b/ramarivera_glm4.7_interleaved_thinking_fix/session_logs//msg_example.json @@ -0,0 +1,29 @@ +{ + "id": "msg_b4c2bb883001Yd3E3BGGMOn1tK", + "sessionID": "", + "role": "assistant", + "time": { + "created": 1766509492355, + "completed": 1766509558418 + }, + "parentID": "", + "modelID": "glm-4.7", + "providerID": "zai-coding-plan", + "mode": "build", + "agent": "build", + "path": { + "cwd": "", + "root": "" + }, + "cost": 0, + "tokens": { + "input": 32, + "output": 506, + "reasoning": 0, + "cache": { + "read": 163403, + "write": 0 + } + }, + "finish": "stop" +} \ No newline at end of file diff --git a/ramarivera_glm4.7_interleaved_thinking_fix/session_logs//parts/msg__1/part_example.json b/ramarivera_glm4.7_interleaved_thinking_fix/session_logs//parts/msg__1/part_example.json new file mode 100644 index 00000000000..b4a264da4cf --- /dev/null +++ b/ramarivera_glm4.7_interleaved_thinking_fix/session_logs//parts/msg__1/part_example.json @@ -0,0 +1,11 @@ +{ + "id": "", + "sessionID": "", + "messageID": "", + "type": "reasoning", + "text": "pal_thinkdeepstepReviewing Phase 1.8: Workflow Inference implementation for architecture and code quality assessment.\n\nKey implementation files:\n- /packages/portal/scripts/generate/workflow/inference.ts\n- /packages/portal/scripts/generate/workflow/manifest.ts\n- /packages/portal/scripts/generate/workflow/embeddings.ts\n- /packages/portal/scripts/generate/workflow/orchestrator.ts\n- /packages/portal/scripts/generate/workflow/types.ts\n- /packages/portal/scripts/generate/workflow/workflow.test.ts\n\nOpenSpec context: Phase 1.8 is defined in openspec/changes/add-grimoire-portal/tasks.md\n\nCurrent implementation focus: LLM-driven workflow inference with manifest persistence and embeddings integration.\n\nKey concerns to assess:\n1. Service architecture and separation of concerns\n2. Mocking strategy and test quality\n3. Type safety and TypeScript practices\n4. Integration readiness with shared-embeddings packagestep_number1total_steps4next_step_requiredtruefindingsI am reviewing the Phase 1.8: Workflow Inference implementation across multiple service files. The workflow inference service (inference.ts) appears to be the core orchestrator that calls LLM services, reads volume content, and produces workflow manifests. The manifest service (manifest.ts) handles JSON file persistence. The embeddings service (embeddings.ts) uses LanceEmbeddingStore from @grimoire/shared-embeddings to update workflow relationships. The orchestrator (orchestrator.ts) coordinates the full pipeline flow. Types are defined in workflow/types.ts and re-exported in services/types.ts.\n\nThe implementation seems conceptually sound - LLM inference → manifest write → embeddings update. However I need to assess:\n- Are the services properly decoupled?\n- Is there clear separation of concerns?\n- Is the logging appropriate?\n- Are error cases handled?\n- Are type definitions correct?modelgpt-5.2-highthinking_modemediumconfidencemedium", + "time": { + "start": 1766509536091, + "end": 1766509558307 + } +} \ No newline at end of file diff --git a/ramarivera_glm4.7_interleaved_thinking_fix/test_glm47_thinking_fix.test.ts b/ramarivera_glm4.7_interleaved_thinking_fix/test_glm47_thinking_fix.test.ts new file mode 100644 index 00000000000..9240164a3dc --- /dev/null +++ b/ramarivera_glm4.7_interleaved_thinking_fix/test_glm47_thinking_fix.test.ts @@ -0,0 +1,283 @@ +import { describe, expect, test } from "bun:test" +import { ProviderTransform } from "../../src/provider/transform" + +describe("ProviderTransform.message - GLM-4.7 malformed thinking block", () => { + test("GLM-4.7 with tool call XML in reasoning_content should extract malformed tool calls", () => { + const malformedReasoning = `Let me think about what to do here. + + bun test packages/portal/scripts/generate/workflow/workflow.test.ts 2>&1 + Run workflow inference unit tests + +After running the tests, I'll analyze the results.` + + const msgs = [ + { + role: "assistant", + content: [{ type: "reasoning", text: malformedReasoning }], + }, + ] as any[] + + const result = ProviderTransform.message(msgs, { + id: "z.ai/glm-4.7", + providerID: "zai-coding-plan", + api: { + id: "glm-4.7", + url: "https://api.z.ai/api/anthropic", + npm: "@ai-sdk/openai-compatible", + }, + name: "GLM-4.7", + capabilities: { + temperature: true, + reasoning: true, + attachment: false, + toolcall: true, + input: { text: true, audio: false, image: false, video: false, pdf: false }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: { + field: "reasoning_content", + }, + }, + cost: { + input: 0.001, + output: 0.002, + cache: { read: 0.0001, write: 0.0002 }, + }, + limit: { + context: 128000, + output: 8192, + }, + status: "active", + options: {}, + headers: {}, + release_date: "2024-01-01", + } as any) + + // The fix should: + // 1. Extract the tool call from reasoning_content + // 2. Add it as a proper tool-call part + // 3. Clean the reasoning text to remove the XML + + expect(result).toHaveLength(1) + + // Check that tool call was extracted + const toolCalls = result[0].content.filter((part: any) => part.type === "tool-call") + expect(toolCalls.length).toBe(1) + expect(toolCalls[0].toolName).toBe("bash") + expect(toolCalls[0].input).toEqual({ + command: "bun test packages/portal/scripts/generate/workflow/workflow.test.ts 2>&1", + }) + + // Check that reasoning was cleaned + const reasoningParts = result[0].content.filter((part: any) => part.type === "reasoning") + expect(reasoningParts.length).toBe(1) + expect(reasoningParts[0].text).not.toContain(" { + const thinkingBlock = ` + Reviewing Phase 1.8 implementation + 1 + 4 + true +` + + const msgs = [ + { + role: "assistant", + content: [{ type: "reasoning", text: thinkingBlock }], + }, + ] as any[] + + const result = ProviderTransform.message(msgs, { + id: "z.ai/glm-4.7", + providerID: "zai-coding-plan", + api: { + id: "glm-4.7", + url: "https://api.z.ai/api/anthropic", + npm: "@ai-sdk/openai-compatible", + }, + name: "GLM-4.7", + capabilities: { + temperature: true, + reasoning: true, + attachment: false, + toolcall: true, + input: { text: true, audio: false, image: false, video: false, pdf: false }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: { + field: "reasoning_content", + }, + }, + cost: { + input: 0.001, + output: 0.002, + cache: { read: 0.0001, write: 0.0002 }, + }, + limit: { + context: 128000, + output: 8192, + }, + status: "active", + options: {}, + headers: {}, + release_date: "2024-01-01", + } as any) + + // For thinking-specific tools like pal_thinkdeep, we may want to keep them in reasoning + // or extract them - the fix should handle this case appropriately + expect(result).toHaveLength(1) + + // Either the thinking block is cleaned OR the tool call is extracted + const reasoningText = result[0].content + .filter((part: any) => part.type === "reasoning") + .map((part: any) => part.text) + .join("") + + // The behavior depends on implementation - either: + // 1. Extract pal_thinkdeep as a tool-call + // 2. Or keep it in reasoning (since it's a thinking tool, not execution) + // 3. Or clean it from reasoning + expect(true).toBe(true) // Placeholder - actual behavior depends on fix + }) + + test("GLM-4.7 with multiple tool calls in reasoning should extract all", () => { + const malformedReasoning = `I'll need to run several commands: + + ls -la + List files + + + bun install + Install dependencies + + + bun test + Run tests +` + + const msgs = [ + { + role: "assistant", + content: [{ type: "reasoning", text: malformedReasoning }], + }, + ] as any[] + + const result = ProviderTransform.message(msgs, { + id: "z.ai/glm-4.7", + providerID: "zai-coding-plan", + api: { + id: "glm-4.7", + url: "https://api.z.ai/api/anthropic", + npm: "@ai-sdk/openai-compatible", + }, + name: "GLM-4.7", + capabilities: { + temperature: true, + reasoning: true, + attachment: false, + toolcall: true, + input: { text: true, audio: false, image: false, video: false, pdf: false }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: { + field: "reasoning_content", + }, + }, + cost: { + input: 0.001, + output: 0.002, + cache: { read: 0.0001, write: 0.0002 }, + }, + limit: { + context: 128000, + output: 8192, + }, + status: "active", + options: {}, + headers: {}, + release_date: "2024-01-01", + } as any) + + expect(result).toHaveLength(1) + + // All three tool calls should be extracted + const toolCalls = result[0].content.filter((part: any) => part.type === "tool-call") + expect(toolCalls.length).toBe(3) + + expect(toolCalls[0].toolName).toBe("bash") + expect(toolCalls[0].input).toEqual({ command: "ls -la" }) + + expect(toolCalls[1].toolName).toBe("bash") + expect(toolCalls[1].input).toEqual({ command: "bun install" }) + + expect(toolCalls[2].toolName).toBe("bash") + expect(toolCalls[2].input).toEqual({ command: "bun test" }) + }) + + test("Properly formatted GLM-4.7 response should not be affected", () => { + const msgs = [ + { + role: "assistant", + content: [ + { type: "reasoning", text: "Let me check the tests first." }, + { + type: "tool-call", + toolCallId: "test-1", + toolName: "bash", + input: { command: "bun test" }, + }, + { type: "text", text: "Tests passed!" }, + ], + }, + ] as any[] + + const result = ProviderTransform.message(msgs, { + id: "z.ai/glm-4.7", + providerID: "zai-coding-plan", + api: { + id: "glm-4.7", + url: "https://api.z.ai/api/anthropic", + npm: "@ai-sdk/openai-compatible", + }, + name: "GLM-4.7", + capabilities: { + temperature: true, + reasoning: true, + attachment: false, + toolcall: true, + input: { text: true, audio: false, image: false, video: false, pdf: false }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: { + field: "reasoning_content", + }, + }, + cost: { + input: 0.001, + output: 0.002, + cache: { read: 0.0001, write: 0.0002 }, + }, + limit: { + context: 128000, + output: 8192, + }, + status: "active", + options: {}, + headers: {}, + release_date: "2024-01-01", + } as any) + + // Should preserve existing structure + expect(result).toHaveLength(1) + expect(result[0].content).toHaveLength(3) + + const reasoning = result[0].content.filter((part: any) => part.type === "reasoning") + expect(reasoning.length).toBe(1) + expect(reasoning[0].text).toBe("Let me check the tests first.") + + const toolCalls = result[0].content.filter((part: any) => part.type === "tool-call") + expect(toolCalls.length).toBe(1) + expect(toolCalls[0].input).toEqual({ command: "bun test" }) + }) +})