From 3047d1923857a8b0e839781619228b32c027094c Mon Sep 17 00:00:00 2001 From: Dayuan Jiang <34411969+DayuanJiang@users.noreply.github.com> Date: Thu, 25 Dec 2025 13:19:04 +0900 Subject: [PATCH] fix: rename edit_diagram type field to operation for better model compatibility (#402) Fixes #374 - Models were confused by the `type` field name and sent `operation` instead. This change: - Renames DiagramOperation.type to DiagramOperation.operation across all files (MCP server, web app, hooks, components, system prompts) - Adds JSON examples in tool descriptions to show correct format - Updates all test data to use the new field name Affected files: - lib/utils.ts - app/api/chat/route.ts - hooks/use-diagram-tool-handlers.ts - components/chat-message-display.tsx - lib/system-prompts.ts - packages/mcp-server/src/diagram-operations.ts - packages/mcp-server/src/index.ts - scripts/test-diagram-operations.mjs MCP server version bumped to 0.1.6 --- app/api/chat/route.ts | 14 +++-- components/chat-message-display.tsx | 16 +++--- hooks/use-diagram-tool-handlers.ts | 2 +- lib/system-prompts.ts | 18 +++---- lib/utils.ts | 8 +-- packages/mcp-server/package.json | 2 +- packages/mcp-server/src/diagram-operations.ts | 8 +-- packages/mcp-server/src/index.ts | 18 +++++-- scripts/test-diagram-operations.mjs | 54 +++++++++---------- 9 files changed, 78 insertions(+), 62 deletions(-) diff --git a/app/api/chat/route.ts b/app/api/chat/route.ts index 351f328..1d1ad1e 100644 --- a/app/api/chat/route.ts +++ b/app/api/chat/route.ts @@ -609,14 +609,22 @@ Operations: For update/add, new_xml must be a complete mxCell element including mxGeometry. -⚠️ JSON ESCAPING: Every " inside new_xml MUST be escaped as \\". Example: id=\\"5\\" value=\\"Label\\"`, +⚠️ JSON ESCAPING: Every " inside new_xml MUST be escaped as \\". Example: id=\\"5\\" value=\\"Label\\" + +Example - Add a rectangle: +{"operations": [{"operation": "add", "cell_id": "rect-1", "new_xml": ""}]} + +Example - Delete a cell: +{"operations": [{"operation": "delete", "cell_id": "rect-1"}]}`, inputSchema: z.object({ operations: z .array( z.object({ - type: z + operation: z .enum(["update", "add", "delete"]) - .describe("Operation type"), + .describe( + "Operation to perform: add, update, or delete", + ), cell_id: z .string() .describe( diff --git a/components/chat-message-display.tsx b/components/chat-message-display.tsx index b094f8e..0a14597 100644 --- a/components/chat-message-display.tsx +++ b/components/chat-message-display.tsx @@ -40,7 +40,7 @@ import ExamplePanel from "./chat-example-panel" import { CodeBlock } from "./code-block" interface DiagramOperation { - type: "update" | "add" | "delete" + operation: "update" | "add" | "delete" cell_id: string new_xml?: string } @@ -53,12 +53,12 @@ function getCompleteOperations( return operations.filter( (op) => op && - typeof op.type === "string" && - ["update", "add", "delete"].includes(op.type) && + typeof op.operation === "string" && + ["update", "add", "delete"].includes(op.operation) && typeof op.cell_id === "string" && op.cell_id.length > 0 && // delete doesn't need new_xml, update/add do - (op.type === "delete" || typeof op.new_xml === "string"), + (op.operation === "delete" || typeof op.new_xml === "string"), ) } @@ -79,20 +79,20 @@ function OperationsDisplay({ operations }: { operations: DiagramOperation[] }) {
{operations.map((op, index) => (
- {op.type} + {op.operation} cell_id: {op.cell_id} diff --git a/hooks/use-diagram-tool-handlers.ts b/hooks/use-diagram-tool-handlers.ts index f0df05d..0e4c7bb 100644 --- a/hooks/use-diagram-tool-handlers.ts +++ b/hooks/use-diagram-tool-handlers.ts @@ -30,7 +30,7 @@ type AddToolOutputParams = AddToolOutputSuccess | AddToolOutputError type AddToolOutputFn = (params: AddToolOutputParams) => void interface DiagramOperation { - type: "update" | "add" | "delete" + operation: "update" | "add" | "delete" cell_id: string new_xml?: string } diff --git a/lib/system-prompts.ts b/lib/system-prompts.ts index e2afa20..5016ae6 100644 --- a/lib/system-prompts.ts +++ b/lib/system-prompts.ts @@ -99,9 +99,9 @@ When using edit_diagram tool: - For update/add: provide cell_id and complete new_xml (full mxCell element including mxGeometry) - For delete: only cell_id is needed - Find the cell_id from "Current diagram XML" in system context -- Example update: {"operations": [{"type": "update", "cell_id": "3", "new_xml": "\\n \\n"}]} -- Example delete: {"operations": [{"type": "delete", "cell_id": "5"}]} -- Example add: {"operations": [{"type": "add", "cell_id": "new1", "new_xml": "\\n \\n"}]} +- Example update: {"operations": [{"operation": "update", "cell_id": "3", "new_xml": "\\n \\n"}]} +- Example delete: {"operations": [{"operation": "delete", "cell_id": "5"}]} +- Example add: {"operations": [{"operation": "add", "cell_id": "new1", "new_xml": "\\n \\n"}]} ⚠️ JSON ESCAPING: Every " inside new_xml MUST be escaped as \\". Example: id=\\"5\\" value=\\"Label\\" @@ -282,9 +282,9 @@ edit_diagram uses ID-based operations to modify cells directly by their id attri \`\`\`json { "operations": [ - {"type": "update", "cell_id": "3", "new_xml": ""}, - {"type": "add", "cell_id": "new1", "new_xml": ""}, - {"type": "delete", "cell_id": "5"} + {"operation": "update", "cell_id": "3", "new_xml": ""}, + {"operation": "add", "cell_id": "new1", "new_xml": ""}, + {"operation": "delete", "cell_id": "5"} ] } \`\`\` @@ -293,17 +293,17 @@ edit_diagram uses ID-based operations to modify cells directly by their id attri Change label: \`\`\`json -{"operations": [{"type": "update", "cell_id": "3", "new_xml": "\\n \\n"}]} +{"operations": [{"operation": "update", "cell_id": "3", "new_xml": "\\n \\n"}]} \`\`\` Add new shape: \`\`\`json -{"operations": [{"type": "add", "cell_id": "new1", "new_xml": "\\n \\n"}]} +{"operations": [{"operation": "add", "cell_id": "new1", "new_xml": "\\n \\n"}]} \`\`\` Delete cell: \`\`\`json -{"operations": [{"type": "delete", "cell_id": "5"}]} +{"operations": [{"operation": "delete", "cell_id": "5"}]} \`\`\` **Error Recovery:** diff --git a/lib/utils.ts b/lib/utils.ts index 6d36865..74e7038 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -455,7 +455,7 @@ export function replaceNodes(currentXML: string, nodes: string): string { // ============================================================================ export interface DiagramOperation { - type: "update" | "add" | "delete" + operation: "update" | "add" | "delete" cell_id: string new_xml?: string } @@ -528,7 +528,7 @@ export function applyDiagramOperations( // Process each operation for (const op of operations) { - if (op.type === "update") { + if (op.operation === "update") { const existingCell = cellMap.get(op.cell_id) if (!existingCell) { errors.push({ @@ -580,7 +580,7 @@ export function applyDiagramOperations( // Update the map with the new element cellMap.set(op.cell_id, importedNode) - } else if (op.type === "add") { + } else if (op.operation === "add") { // Check if ID already exists if (cellMap.has(op.cell_id)) { errors.push({ @@ -632,7 +632,7 @@ export function applyDiagramOperations( // Add to map cellMap.set(op.cell_id, importedNode) - } else if (op.type === "delete") { + } else if (op.operation === "delete") { const existingCell = cellMap.get(op.cell_id) if (!existingCell) { errors.push({ diff --git a/packages/mcp-server/package.json b/packages/mcp-server/package.json index 9825de3..6823615 100644 --- a/packages/mcp-server/package.json +++ b/packages/mcp-server/package.json @@ -1,6 +1,6 @@ { "name": "@next-ai-drawio/mcp-server", - "version": "0.1.5", + "version": "0.1.6", "description": "MCP server for Next AI Draw.io - AI-powered diagram generation with real-time browser preview", "type": "module", "main": "dist/index.js", diff --git a/packages/mcp-server/src/diagram-operations.ts b/packages/mcp-server/src/diagram-operations.ts index 824e309..5976e87 100644 --- a/packages/mcp-server/src/diagram-operations.ts +++ b/packages/mcp-server/src/diagram-operations.ts @@ -4,7 +4,7 @@ */ export interface DiagramOperation { - type: "update" | "add" | "delete" + operation: "update" | "add" | "delete" cell_id: string new_xml?: string } @@ -77,7 +77,7 @@ export function applyDiagramOperations( // Process each operation for (const op of operations) { - if (op.type === "update") { + if (op.operation === "update") { const existingCell = cellMap.get(op.cell_id) if (!existingCell) { errors.push({ @@ -129,7 +129,7 @@ export function applyDiagramOperations( // Update the map with the new element cellMap.set(op.cell_id, importedNode) - } else if (op.type === "add") { + } else if (op.operation === "add") { // Check if ID already exists if (cellMap.has(op.cell_id)) { errors.push({ @@ -181,7 +181,7 @@ export function applyDiagramOperations( // Add to map cellMap.set(op.cell_id, importedNode) - } else if (op.type === "delete") { + } else if (op.operation === "delete") { const existingCell = cellMap.get(op.cell_id) if (!existingCell) { errors.push({ diff --git a/packages/mcp-server/src/index.ts b/packages/mcp-server/src/index.ts index e2af673..a41788b 100644 --- a/packages/mcp-server/src/index.ts +++ b/packages/mcp-server/src/index.ts @@ -265,14 +265,22 @@ server.registerTool( "- add: Add a new cell. Provide cell_id (new unique id) and new_xml.\n" + "- update: Replace an existing cell by its id. Provide cell_id and complete new_xml.\n" + "- delete: Remove a cell by its id. Only cell_id is needed.\n\n" + - "For add/update, new_xml must be a complete mxCell element including mxGeometry.", + "For add/update, new_xml must be a complete mxCell element including mxGeometry.\n\n" + + "Example - Add a rectangle:\n" + + '{"operations": [{"operation": "add", "cell_id": "rect-1", "new_xml": ""}]}\n\n' + + "Example - Update a cell:\n" + + '{"operations": [{"operation": "update", "cell_id": "3", "new_xml": ""}]}\n\n' + + "Example - Delete a cell:\n" + + '{"operations": [{"operation": "delete", "cell_id": "rect-1"}]}', inputSchema: { operations: z .array( z.object({ - type: z + operation: z .enum(["update", "add", "delete"]) - .describe("Operation type"), + .describe( + "Operation to perform: add, update, or delete", + ), cell_id: z.string().describe("The id of the mxCell"), new_xml: z .string() @@ -356,13 +364,13 @@ server.registerTool( ) if (fixed) { log.info( - `Operation ${op.type} ${op.cell_id}: XML auto-fixed: ${fixes.join(", ")}`, + `Operation ${op.operation} ${op.cell_id}: XML auto-fixed: ${fixes.join(", ")}`, ) return { ...op, new_xml: fixed } } if (!valid && error) { log.warn( - `Operation ${op.type} ${op.cell_id}: XML validation failed: ${error}`, + `Operation ${op.operation} ${op.cell_id}: XML validation failed: ${error}`, ) } } diff --git a/scripts/test-diagram-operations.mjs b/scripts/test-diagram-operations.mjs index f5e9113..b58ad8d 100644 --- a/scripts/test-diagram-operations.mjs +++ b/scripts/test-diagram-operations.mjs @@ -22,7 +22,7 @@ function applyDiagramOperations(xmlContent, operations) { result: xmlContent, errors: [ { - type: "update", + operation: "update", cellId: "", message: `XML parse error: ${parseError.textContent}`, }, @@ -36,7 +36,7 @@ function applyDiagramOperations(xmlContent, operations) { result: xmlContent, errors: [ { - type: "update", + operation: "update", cellId: "", message: "Could not find element in XML", }, @@ -51,11 +51,11 @@ function applyDiagramOperations(xmlContent, operations) { }) for (const op of operations) { - if (op.type === "update") { + if (op.operation === "update") { const existingCell = cellMap.get(op.cell_id) if (!existingCell) { errors.push({ - type: "update", + operation: "update", cellId: op.cell_id, message: `Cell with id="${op.cell_id}" not found`, }) @@ -63,7 +63,7 @@ function applyDiagramOperations(xmlContent, operations) { } if (!op.new_xml) { errors.push({ - type: "update", + operation: "update", cellId: op.cell_id, message: "new_xml is required for update operation", }) @@ -76,7 +76,7 @@ function applyDiagramOperations(xmlContent, operations) { const newCell = newDoc.querySelector("mxCell") if (!newCell) { errors.push({ - type: "update", + operation: "update", cellId: op.cell_id, message: "new_xml must contain an mxCell element", }) @@ -85,7 +85,7 @@ function applyDiagramOperations(xmlContent, operations) { const newCellId = newCell.getAttribute("id") if (newCellId !== op.cell_id) { errors.push({ - type: "update", + operation: "update", cellId: op.cell_id, message: `ID mismatch: cell_id is "${op.cell_id}" but new_xml has id="${newCellId}"`, }) @@ -94,10 +94,10 @@ function applyDiagramOperations(xmlContent, operations) { const importedNode = doc.importNode(newCell, true) existingCell.parentNode?.replaceChild(importedNode, existingCell) cellMap.set(op.cell_id, importedNode) - } else if (op.type === "add") { + } else if (op.operation === "add") { if (cellMap.has(op.cell_id)) { errors.push({ - type: "add", + operation: "add", cellId: op.cell_id, message: `Cell with id="${op.cell_id}" already exists`, }) @@ -105,7 +105,7 @@ function applyDiagramOperations(xmlContent, operations) { } if (!op.new_xml) { errors.push({ - type: "add", + operation: "add", cellId: op.cell_id, message: "new_xml is required for add operation", }) @@ -118,7 +118,7 @@ function applyDiagramOperations(xmlContent, operations) { const newCell = newDoc.querySelector("mxCell") if (!newCell) { errors.push({ - type: "add", + operation: "add", cellId: op.cell_id, message: "new_xml must contain an mxCell element", }) @@ -127,7 +127,7 @@ function applyDiagramOperations(xmlContent, operations) { const newCellId = newCell.getAttribute("id") if (newCellId !== op.cell_id) { errors.push({ - type: "add", + operation: "add", cellId: op.cell_id, message: `ID mismatch: cell_id is "${op.cell_id}" but new_xml has id="${newCellId}"`, }) @@ -136,11 +136,11 @@ function applyDiagramOperations(xmlContent, operations) { const importedNode = doc.importNode(newCell, true) root.appendChild(importedNode) cellMap.set(op.cell_id, importedNode) - } else if (op.type === "delete") { + } else if (op.operation === "delete") { const existingCell = cellMap.get(op.cell_id) if (!existingCell) { errors.push({ - type: "delete", + operation: "delete", cellId: op.cell_id, message: `Cell with id="${op.cell_id}" not found`, }) @@ -201,7 +201,7 @@ function assert(condition, message) { test("Update operation changes cell value", () => { const { result, errors } = applyDiagramOperations(sampleXml, [ { - type: "update", + operation: "update", cell_id: "2", new_xml: '', @@ -224,7 +224,7 @@ test("Update operation changes cell value", () => { test("Update operation fails for non-existent cell", () => { const { errors } = applyDiagramOperations(sampleXml, [ { - type: "update", + operation: "update", cell_id: "999", new_xml: '', }, @@ -239,7 +239,7 @@ test("Update operation fails for non-existent cell", () => { test("Update operation fails on ID mismatch", () => { const { errors } = applyDiagramOperations(sampleXml, [ { - type: "update", + operation: "update", cell_id: "2", new_xml: '', }, @@ -254,7 +254,7 @@ test("Update operation fails on ID mismatch", () => { test("Add operation creates new cell", () => { const { result, errors } = applyDiagramOperations(sampleXml, [ { - type: "add", + operation: "add", cell_id: "new1", new_xml: '', @@ -274,7 +274,7 @@ test("Add operation creates new cell", () => { test("Add operation fails for duplicate ID", () => { const { errors } = applyDiagramOperations(sampleXml, [ { - type: "add", + operation: "add", cell_id: "2", new_xml: '', }, @@ -289,7 +289,7 @@ test("Add operation fails for duplicate ID", () => { test("Add operation fails on ID mismatch", () => { const { errors } = applyDiagramOperations(sampleXml, [ { - type: "add", + operation: "add", cell_id: "new1", new_xml: '', }, @@ -303,7 +303,7 @@ test("Add operation fails on ID mismatch", () => { test("Delete operation removes cell", () => { const { result, errors } = applyDiagramOperations(sampleXml, [ - { type: "delete", cell_id: "3" }, + { operation: "delete", cell_id: "3" }, ]) assert( errors.length === 0, @@ -315,7 +315,7 @@ test("Delete operation removes cell", () => { test("Delete operation fails for non-existent cell", () => { const { errors } = applyDiagramOperations(sampleXml, [ - { type: "delete", cell_id: "999" }, + { operation: "delete", cell_id: "999" }, ]) assert(errors.length === 1, "Should have one error") assert( @@ -327,18 +327,18 @@ test("Delete operation fails for non-existent cell", () => { test("Multiple operations in sequence", () => { const { result, errors } = applyDiagramOperations(sampleXml, [ { - type: "update", + operation: "update", cell_id: "2", new_xml: '', }, { - type: "add", + operation: "add", cell_id: "new1", new_xml: '', }, - { type: "delete", cell_id: "3" }, + { operation: "delete", cell_id: "3" }, ]) assert( errors.length === 0, @@ -354,14 +354,14 @@ test("Multiple operations in sequence", () => { test("Invalid XML returns parse error", () => { const { errors } = applyDiagramOperations(" { const { errors } = applyDiagramOperations("", [ - { type: "delete", cell_id: "1" }, + { operation: "delete", cell_id: "1" }, ]) assert(errors.length === 1, "Should have one error") assert(