refactor: replace text-based edit_diagram with ID-based operations (#267)

* refactor: replace text-based edit_diagram with ID-based operations

- Add applyDiagramOperations() function using DOMParser for ID lookup
- New schema: operations array with type (update/add/delete), cell_id, new_xml
- Update chat-panel.tsx handler for new operations format
- Update OperationsDisplay component to show operation type and cell_id
- Simplify system prompts with new ID-based examples
- Add ID validation for add operations
- Add warning for edges referencing deleted cells

* fix: add ID validation to update operation and remove dead code

- Add ID mismatch validation to update operation (consistency with add)
- Remove orphaned replaceXMLParts function (~300 lines of dead code)
- Update cell_id schema description for clarity
- Add unit tests for applyDiagramOperations (11 tests)
This commit is contained in:
Dayuan Jiang
2025-12-15 14:22:56 +09:00
committed by GitHub
parent 09c556e4c3
commit f175276872
6 changed files with 578 additions and 408 deletions

View File

@@ -88,19 +88,15 @@ Note that:
- NEVER include XML comments (<!-- ... -->) in your generated XML. Draw.io strips comments, which breaks edit_diagram patterns.
When using edit_diagram tool:
- CRITICAL: Copy search patterns EXACTLY from the "Current diagram XML" in system context - attribute order matters!
- Always include the element's id attribute for unique targeting: {"search": "<mxCell id=\\"5\\"", ...}
- Include complete elements (mxCell + mxGeometry) for reliable matching
- Preserve exact whitespace, indentation, and line breaks
- BAD: {"search": "value=\\"Label\\"", ...} - too vague, matches multiple elements
- GOOD: {"search": "<mxCell id=\\"3\\" value=\\"Old\\" style=\\"...\\">", "replace": "<mxCell id=\\"3\\" value=\\"New\\" style=\\"...\\">"}
- For multiple changes, use separate edits in array
- RETRY POLICY: If pattern not found, retry up to 3 times with adjusted patterns. After 3 failures, use display_diagram instead.
- Use operations: update (modify cell by id), add (new cell), delete (remove cell by id)
- 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": "<mxCell id=\\"3\\" value=\\"New Label\\" style=\\"rounded=1;\\" vertex=\\"1\\" parent=\\"1\\">\\n <mxGeometry x=\\"100\\" y=\\"100\\" width=\\"120\\" height=\\"60\\" as=\\"geometry\\"/>\\n</mxCell>"}]}
- Example delete: {"operations": [{"type": "delete", "cell_id": "5"}]}
- Example add: {"operations": [{"type": "add", "cell_id": "new1", "new_xml": "<mxCell id=\\"new1\\" value=\\"New Box\\" style=\\"rounded=1;\\" vertex=\\"1\\" parent=\\"1\\">\\n <mxGeometry x=\\"400\\" y=\\"200\\" width=\\"120\\" height=\\"60\\" as=\\"geometry\\"/>\\n</mxCell>"}]}
⚠️ CRITICAL JSON ESCAPING: When outputting edit_diagram tool calls, you MUST escape ALL double quotes inside string values:
- CORRECT: "y=\\"119\\"" (both quotes escaped)
- WRONG: "y="119\\"" (missing backslash before first quote - causes JSON parse error!)
- Every " inside a JSON string value needs \\" - no exceptions!
⚠️ JSON ESCAPING: Every " inside new_xml MUST be escaped as \\". Example: id=\\"5\\" value=\\"Label\\"
## Draw.io XML Structure Reference
@@ -268,69 +264,43 @@ const EXTENDED_ADDITIONS = `
### edit_diagram Details
**CRITICAL RULES:**
- Copy-paste the EXACT search pattern from the "Current diagram XML" in system context
- Do NOT reorder attributes or reformat - the attribute order in draw.io XML varies and you MUST match it exactly
- Only include the lines that are changing, plus 1-2 surrounding lines for context if needed
- Break large changes into multiple smaller edits
- Each search must contain complete lines (never truncate mid-line)
- First match only - be specific enough to target the right element
edit_diagram uses ID-based operations to modify cells directly by their id attribute.
**Operations:**
- **update**: Replace an existing cell. Provide cell_id and new_xml.
- **add**: Add a new cell. Provide cell_id (new unique id) and new_xml.
- **delete**: Remove a cell. Only cell_id is needed.
**Input Format:**
\`\`\`json
{
"edits": [
{
"search": "EXACT lines copied from current XML (preserve attribute order!)",
"replace": "Replacement lines"
}
"operations": [
{"type": "update", "cell_id": "3", "new_xml": "<mxCell ...complete element...>"},
{"type": "add", "cell_id": "new1", "new_xml": "<mxCell ...new element...>"},
{"type": "delete", "cell_id": "5"}
]
}
\`\`\`
## edit_diagram Best Practices
**Examples:**
### Core Principle: Unique & Precise Patterns
Your search pattern MUST uniquely identify exactly ONE location in the XML. Before writing a search pattern:
1. Review the "Current diagram XML" in the system context
2. Identify the exact element(s) to modify by their unique id attribute
3. Include enough context to ensure uniqueness
### Pattern Construction Rules
**Rule 1: Always include the element's id attribute**
Change label:
\`\`\`json
{"search": "<mxCell id=\\"node5\\"", "replace": "<mxCell id=\\"node5\\" value=\\"New Label\\""}
{"operations": [{"type": "update", "cell_id": "3", "new_xml": "<mxCell id=\\"3\\" value=\\"New Label\\" style=\\"rounded=1;\\" vertex=\\"1\\" parent=\\"1\\">\\n <mxGeometry x=\\"100\\" y=\\"100\\" width=\\"120\\" height=\\"60\\" as=\\"geometry\\"/>\\n</mxCell>"}]}
\`\`\`
**Rule 2: Include complete XML elements when possible**
Add new shape:
\`\`\`json
{
"search": "<mxCell id=\\"3\\" value=\\"Old\\" style=\\"rounded=1;\\" vertex=\\"1\\" parent=\\"1\\">\\n <mxGeometry x=\\"100\\" y=\\"100\\" width=\\"120\\" height=\\"60\\" as=\\"geometry\\"/>\\n</mxCell>",
"replace": "<mxCell id=\\"3\\" value=\\"New\\" style=\\"rounded=1;\\" vertex=\\"1\\" parent=\\"1\\">\\n <mxGeometry x=\\"100\\" y=\\"100\\" width=\\"120\\" height=\\"60\\" as=\\"geometry\\"/>\\n</mxCell>"
}
{"operations": [{"type": "add", "cell_id": "new1", "new_xml": "<mxCell id=\\"new1\\" value=\\"New Box\\" style=\\"rounded=1;fillColor=#dae8fc;\\" vertex=\\"1\\" parent=\\"1\\">\\n <mxGeometry x=\\"400\\" y=\\"200\\" width=\\"120\\" height=\\"60\\" as=\\"geometry\\"/>\\n</mxCell>"}]}
\`\`\`
**Rule 3: Preserve exact whitespace and formatting**
Copy the search pattern EXACTLY from the current XML, including leading spaces, line breaks (\\n), and attribute order.
Delete cell:
\`\`\`json
{"operations": [{"type": "delete", "cell_id": "5"}]}
\`\`\`
### Good vs Bad Patterns
**BAD:** \`{"search": "value=\\"Label\\""}\` - Too vague, matches multiple elements
**BAD:** \`{"search": "<mxCell value=\\"X\\" id=\\"5\\""}\` - Reordered attributes won't match
**GOOD:** \`{"search": "<mxCell id=\\"5\\" parent=\\"1\\" style=\\"...\\" value=\\"Old\\" vertex=\\"1\\">"}\` - Uses unique id with full context
### ⚠️ JSON Escaping (CRITICAL)
Every double quote inside JSON string values MUST be escaped with backslash:
- **CORRECT:** \`"x=\\"100\\" y=\\"200\\""\` - both quotes escaped
- **WRONG:** \`"x=\\"100\\" y="200\\""\` - missing backslash causes JSON parse error!
### Error Recovery
If edit_diagram fails with "pattern not found":
1. **First retry**: Check attribute order - copy EXACTLY from current XML
2. **Second retry**: Expand context - include more surrounding lines
3. **Third retry**: Try matching on just \`<mxCell id="X"\` prefix + full replacement
4. **After 3 failures**: Fall back to display_diagram to regenerate entire diagram
**Error Recovery:**
If cell_id not found, check "Current diagram XML" for correct IDs. Use display_diagram if major restructuring is needed

View File

@@ -377,303 +377,223 @@ export function replaceNodes(currentXML: string, nodes: string): string {
}
}
/**
* Create a character count dictionary from a string
* Used for attribute-order agnostic comparison
*/
function charCountDict(str: string): Map<string, number> {
const dict = new Map<string, number>()
for (const char of str) {
dict.set(char, (dict.get(char) || 0) + 1)
}
return dict
// ============================================================================
// ID-based Diagram Operations
// ============================================================================
export interface DiagramOperation {
type: "update" | "add" | "delete"
cell_id: string
new_xml?: string
}
export interface OperationError {
type: "update" | "add" | "delete"
cellId: string
message: string
}
export interface ApplyOperationsResult {
result: string
errors: OperationError[]
}
/**
* Compare two strings by character frequency (order-agnostic)
* Apply diagram operations (update/add/delete) using ID-based lookup.
* This replaces the text-matching approach with direct DOM manipulation.
*
* @param xmlContent - The full mxfile XML content
* @param operations - Array of operations to apply
* @returns Object with result XML and any errors
*/
function sameCharFrequency(a: string, b: string): boolean {
const trimmedA = a.trim()
const trimmedB = b.trim()
if (trimmedA.length !== trimmedB.length) return false
const dictA = charCountDict(trimmedA)
const dictB = charCountDict(trimmedB)
if (dictA.size !== dictB.size) return false
for (const [char, count] of dictA) {
if (dictB.get(char) !== count) return false
}
return true
}
/**
* Replace specific parts of XML content using search and replace pairs
* @param xmlContent - The original XML string
* @param searchReplacePairs - Array of {search: string, replace: string} objects
* @returns The updated XML string with replacements applied
*/
export function replaceXMLParts(
export function applyDiagramOperations(
xmlContent: string,
searchReplacePairs: Array<{ search: string; replace: string }>,
): string {
// Format the XML first to ensure consistent line breaks
let result = formatXML(xmlContent)
operations: DiagramOperation[],
): ApplyOperationsResult {
const errors: OperationError[] = []
for (const { search, replace } of searchReplacePairs) {
// Also format the search content for consistency
const formattedSearch = formatXML(search)
const searchLines = formattedSearch.split("\n")
// Parse the XML
const parser = new DOMParser()
const doc = parser.parseFromString(xmlContent, "text/xml")
// Split into lines for exact line matching
const resultLines = result.split("\n")
// Remove trailing empty line if exists (from the trailing \n in search content)
if (searchLines[searchLines.length - 1] === "") {
searchLines.pop()
// Check for parse errors
const parseError = doc.querySelector("parsererror")
if (parseError) {
return {
result: xmlContent,
errors: [
{
type: "update",
cellId: "",
message: `XML parse error: ${parseError.textContent}`,
},
],
}
// Always search from the beginning - pairs may not be in document order
const startLineNum = 0
// Try to find match using multiple strategies
let matchFound = false
let matchStartLine = -1
let matchEndLine = -1
// First try: exact match
for (
let i = startLineNum;
i <= resultLines.length - searchLines.length;
i++
) {
let matches = true
for (let j = 0; j < searchLines.length; j++) {
if (resultLines[i + j] !== searchLines[j]) {
matches = false
break
}
}
if (matches) {
matchStartLine = i
matchEndLine = i + searchLines.length
matchFound = true
break
}
}
// Second try: line-trimmed match (fallback)
if (!matchFound) {
for (
let i = startLineNum;
i <= resultLines.length - searchLines.length;
i++
) {
let matches = true
for (let j = 0; j < searchLines.length; j++) {
const originalTrimmed = resultLines[i + j].trim()
const searchTrimmed = searchLines[j].trim()
if (originalTrimmed !== searchTrimmed) {
matches = false
break
}
}
if (matches) {
matchStartLine = i
matchEndLine = i + searchLines.length
matchFound = true
break
}
}
}
// Third try: substring match as last resort (for single-line XML)
if (!matchFound) {
// Try to find as a substring in the entire content
const searchStr = search.trim()
const resultStr = result
const index = resultStr.indexOf(searchStr)
if (index !== -1) {
// Found as substring - replace it
result =
resultStr.substring(0, index) +
replace.trim() +
resultStr.substring(index + searchStr.length)
// Re-format after substring replacement
result = formatXML(result)
continue // Skip the line-based replacement below
}
}
// Fourth try: character frequency match (attribute-order agnostic)
// This handles cases where the model generates XML with different attribute order
if (!matchFound) {
for (
let i = startLineNum;
i <= resultLines.length - searchLines.length;
i++
) {
let matches = true
for (let j = 0; j < searchLines.length; j++) {
if (
!sameCharFrequency(resultLines[i + j], searchLines[j])
) {
matches = false
break
}
}
if (matches) {
matchStartLine = i
matchEndLine = i + searchLines.length
matchFound = true
break
}
}
}
// Fifth try: Match by mxCell id attribute
// Extract id from search pattern and find the element with that id
if (!matchFound) {
const idMatch = search.match(/id="([^"]+)"/)
if (idMatch) {
const searchId = idMatch[1]
// Find lines that contain this id
for (let i = startLineNum; i < resultLines.length; i++) {
if (resultLines[i].includes(`id="${searchId}"`)) {
// Found the element with matching id
// Now find the extent of this element (it might span multiple lines)
let endLine = i + 1
const line = resultLines[i].trim()
// Check if it's a self-closing tag or has children
if (!line.endsWith("/>")) {
// Find the closing tag or the end of the mxCell block
let depth = 1
while (endLine < resultLines.length && depth > 0) {
const currentLine = resultLines[endLine].trim()
if (
currentLine.startsWith("<") &&
!currentLine.startsWith("</") &&
!currentLine.endsWith("/>")
) {
depth++
} else if (currentLine.startsWith("</")) {
depth--
}
endLine++
}
}
matchStartLine = i
matchEndLine = endLine
matchFound = true
break
}
}
}
}
// Sixth try: Match by value attribute (label text)
// Extract value from search pattern and find elements with that value
if (!matchFound) {
const valueMatch = search.match(/value="([^"]*)"/)
if (valueMatch) {
const searchValue = valueMatch[0] // Use full match like value="text"
for (let i = startLineNum; i < resultLines.length; i++) {
if (resultLines[i].includes(searchValue)) {
// Found element with matching value
let endLine = i + 1
const line = resultLines[i].trim()
if (!line.endsWith("/>")) {
let depth = 1
while (endLine < resultLines.length && depth > 0) {
const currentLine = resultLines[endLine].trim()
if (
currentLine.startsWith("<") &&
!currentLine.startsWith("</") &&
!currentLine.endsWith("/>")
) {
depth++
} else if (currentLine.startsWith("</")) {
depth--
}
endLine++
}
}
matchStartLine = i
matchEndLine = endLine
matchFound = true
break
}
}
}
}
// Seventh try: Normalized whitespace match
// Collapse all whitespace and compare
if (!matchFound) {
const normalizeWs = (s: string) => s.replace(/\s+/g, " ").trim()
const normalizedSearch = normalizeWs(search)
for (
let i = startLineNum;
i <= resultLines.length - searchLines.length;
i++
) {
// Build a normalized version of the candidate lines
const candidateLines = resultLines.slice(
i,
i + searchLines.length,
)
const normalizedCandidate = normalizeWs(
candidateLines.join(" "),
)
if (normalizedCandidate === normalizedSearch) {
matchStartLine = i
matchEndLine = i + searchLines.length
matchFound = true
break
}
}
}
if (!matchFound) {
throw new Error(
`Search pattern not found in the diagram. The pattern may not exist in the current structure.`,
)
}
// Replace the matched lines
const replaceLines = replace.split("\n")
// Remove trailing empty line if exists
if (replaceLines[replaceLines.length - 1] === "") {
replaceLines.pop()
}
// Perform the replacement
const newResultLines = [
...resultLines.slice(0, matchStartLine),
...replaceLines,
...resultLines.slice(matchEndLine),
]
result = newResultLines.join("\n")
}
return result
// Find the root element (inside mxGraphModel)
const root = doc.querySelector("root")
if (!root) {
return {
result: xmlContent,
errors: [
{
type: "update",
cellId: "",
message: "Could not find <root> element in XML",
},
],
}
}
// Build a map of cell IDs to elements
const cellMap = new Map<string, Element>()
root.querySelectorAll("mxCell").forEach((cell) => {
const id = cell.getAttribute("id")
if (id) cellMap.set(id, cell)
})
// Process each operation
for (const op of operations) {
if (op.type === "update") {
const existingCell = cellMap.get(op.cell_id)
if (!existingCell) {
errors.push({
type: "update",
cellId: op.cell_id,
message: `Cell with id="${op.cell_id}" not found`,
})
continue
}
if (!op.new_xml) {
errors.push({
type: "update",
cellId: op.cell_id,
message: "new_xml is required for update operation",
})
continue
}
// Parse the new XML
const newDoc = parser.parseFromString(
`<wrapper>${op.new_xml}</wrapper>`,
"text/xml",
)
const newCell = newDoc.querySelector("mxCell")
if (!newCell) {
errors.push({
type: "update",
cellId: op.cell_id,
message: "new_xml must contain an mxCell element",
})
continue
}
// Validate ID matches
const newCellId = newCell.getAttribute("id")
if (newCellId !== op.cell_id) {
errors.push({
type: "update",
cellId: op.cell_id,
message: `ID mismatch: cell_id is "${op.cell_id}" but new_xml has id="${newCellId}"`,
})
continue
}
// Import and replace the node
const importedNode = doc.importNode(newCell, true)
existingCell.parentNode?.replaceChild(importedNode, existingCell)
// Update the map with the new element
cellMap.set(op.cell_id, importedNode)
} else if (op.type === "add") {
// Check if ID already exists
if (cellMap.has(op.cell_id)) {
errors.push({
type: "add",
cellId: op.cell_id,
message: `Cell with id="${op.cell_id}" already exists`,
})
continue
}
if (!op.new_xml) {
errors.push({
type: "add",
cellId: op.cell_id,
message: "new_xml is required for add operation",
})
continue
}
// Parse the new XML
const newDoc = parser.parseFromString(
`<wrapper>${op.new_xml}</wrapper>`,
"text/xml",
)
const newCell = newDoc.querySelector("mxCell")
if (!newCell) {
errors.push({
type: "add",
cellId: op.cell_id,
message: "new_xml must contain an mxCell element",
})
continue
}
// Validate ID matches
const newCellId = newCell.getAttribute("id")
if (newCellId !== op.cell_id) {
errors.push({
type: "add",
cellId: op.cell_id,
message: `ID mismatch: cell_id is "${op.cell_id}" but new_xml has id="${newCellId}"`,
})
continue
}
// Import and append the node
const importedNode = doc.importNode(newCell, true)
root.appendChild(importedNode)
// Add to map
cellMap.set(op.cell_id, importedNode)
} else if (op.type === "delete") {
const existingCell = cellMap.get(op.cell_id)
if (!existingCell) {
errors.push({
type: "delete",
cellId: op.cell_id,
message: `Cell with id="${op.cell_id}" not found`,
})
continue
}
// Check for edges referencing this cell (warning only, still delete)
const referencingEdges = root.querySelectorAll(
`mxCell[source="${op.cell_id}"], mxCell[target="${op.cell_id}"]`,
)
if (referencingEdges.length > 0) {
const edgeIds = Array.from(referencingEdges)
.map((e) => e.getAttribute("id"))
.join(", ")
console.warn(
`[applyDiagramOperations] Deleting cell "${op.cell_id}" which is referenced by edges: ${edgeIds}`,
)
}
// Remove the node
existingCell.parentNode?.removeChild(existingCell)
cellMap.delete(op.cell_id)
}
}
// Serialize back to string
const serializer = new XMLSerializer()
const result = serializer.serializeToString(doc)
return { result, errors }
}
// ============================================================================