From c985ecf9feeb74b6bb07bf4ea6c9c5c45afc0724 Mon Sep 17 00:00:00 2001
From: Rohit Ghumare <ghumare64@gmail.com>
Date: Wed, 27 May 2026 18:11:09 +0100
Subject: [PATCH] fix(graph): tolerate reordered XML attrs on entity +
 relationship (closes #635)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previous parser hard-coded attribute order: type-before-name on <entity>, type/source/target/weight on <relationship>. Codex CLI's LLM frequently emits attrs in a different order (name first, source/target before type), and any deviation caused the whole tag to silently drop — graph extraction returned 0 nodes for an entire run.

Replaced both regexes with an order-independent two-pass approach:
- Tiny parseAttrs() reads any key="value" pair in any order
- entitySelfClose + entityWithBody patterns separate the two tag shapes (preserving the #494 fix where greedy attr-matching swallowed trailing / and merged adjacent entities)
- Relationship matcher now reads source/target/type/weight from parsed attrs, falls back to weight=0.5 on missing or non-finite, requires type+source+target present to emit the edge

Regression test: <entity name=... type=...> + <relationship source=... target=... type=...> shape that previously dropped to zero nodes/edges. 7/7 graph tests pass.
---
 src/functions/graph.ts | 89 ++++++++++++++++++++++++++----------------
 test/graph.test.ts     | 30 ++++++++++++++
 2 files changed, 85 insertions(+), 34 deletions(-)

diff --git a/src/functions/graph.ts b/src/functions/graph.ts
index f24e52dfd..09c23af06 100644
--- a/src/functions/graph.ts
+++ b/src/functions/graph.ts
@@ -15,6 +15,22 @@ import {
 import { recordAudit } from "./audit.js";
 import { logger } from "../logger.js";
 
+// Parse all key="value" pairs from a tag's attribute string, in any
+// order. The previous parser hard-coded attribute order
+// (type before name on <entity>, type/source/target/weight on
+// <relationship>) and silently dropped nodes/edges when the upstream
+// LLM emitted attributes in a different order — Codex in particular
+// likes to lead with `name=` (#635).
+function parseAttrs(raw: string): Record<string, string> {
+  const attrs: Record<string, string> = {};
+  const attrRegex = /([A-Za-z_][\w:-]*)="([^"]*)"/g;
+  let m;
+  while ((m = attrRegex.exec(raw)) !== null) {
+    attrs[m[1]] = m[2];
+  }
+  return attrs;
+}
+
 function parseGraphXml(
   xml: string,
   observationIds: string[],
@@ -26,27 +42,25 @@ function parseGraphXml(
   const edges: GraphEdge[] = [];
   const now = new Date().toISOString();
 
-  // Lazy `[^>]*?` so the self-closing alternation gets a chance before
-  // greedy attribute matching consumes the trailing `/` and the regex
-  // falls through to the explicit-close branch, which then runs ahead to
-  // the *next* entity's `</entity>` and silently drops a node (#494
-  // follow-up: greedy `[^>]*` was eating the `/` and merging two entity
-  // declarations into one match).
-  const entityRegex =
-    /<entity\s+type="([^"]+)"\s+name="([^"]+)"[^>]*?(?:\/>|>([\s\S]*?)<\/entity>)/g;
-  let match;
-  while ((match = entityRegex.exec(xml)) !== null) {
-    const type = match[1] as GraphNode["type"];
-    const name = match[2];
-    const propsBlock = match[3] ?? "";
-    const properties: Record<string, string> = {};
+  // Two passes because <entity> can be self-closing or have a body
+  // (<property> children). The self-closing form needs `[^>]*[^/]` on
+  // the attr group so the trailing `/` isn't swallowed into the match
+  // (root cause of #494). The explicit-close form picks up the
+  // property block.
+  const entitySelfClose = /<entity\b([^>]*?)\/>/g;
+  const entityWithBody = /<entity\b([^>]*[^/])>([\s\S]*?)<\/entity>/g;
 
+  const addEntity = (rawAttrs: string, propsBlock = ""): void => {
+    const attrs = parseAttrs(rawAttrs);
+    const type = attrs["type"] as GraphNode["type"] | undefined;
+    const name = attrs["name"];
+    if (!type || !name) return;
+    const properties: Record<string, string> = {};
     const propRegex = /<property\s+key="([^"]+)">([^<]*)<\/property>/g;
     let propMatch;
     while ((propMatch = propRegex.exec(propsBlock)) !== null) {
       properties[propMatch[1]] = propMatch[2];
     }
-
     nodes.push({
       id: generateId("gn"),
       type,
@@ -55,31 +69,38 @@ function parseGraphXml(
       sourceObservationIds: observationIds,
       createdAt: now,
     });
+  };
+
+  let match;
+  while ((match = entitySelfClose.exec(xml)) !== null) {
+    addEntity(match[1]);
+  }
+  while ((match = entityWithBody.exec(xml)) !== null) {
+    addEntity(match[1], match[2]);
   }
 
-  const relRegex =
-    /<relationship\s+type="([^"]+)"\s+source="([^"]+)"\s+target="([^"]+)"\s+weight="([^"]+)"\s*\/>/g;
+  const relRegex = /<relationship\b([^>]*?)\/>/g;
   while ((match = relRegex.exec(xml)) !== null) {
-    const type = match[1] as GraphEdge["type"];
-    const sourceName = match[2];
-    const targetName = match[3];
-    const parsedWeight = parseFloat(match[4]);
-    const weight = Number.isNaN(parsedWeight) ? 0.5 : parsedWeight;
+    const attrs = parseAttrs(match[1]);
+    const type = attrs["type"] as GraphEdge["type"] | undefined;
+    const sourceName = attrs["source"];
+    const targetName = attrs["target"];
+    if (!type || !sourceName || !targetName) continue;
+    const parsedWeight = parseFloat(attrs["weight"] ?? "");
+    const weight = Number.isFinite(parsedWeight) ? parsedWeight : 0.5;
 
     const sourceNode = nodes.find((n) => n.name === sourceName);
     const targetNode = nodes.find((n) => n.name === targetName);
-
-    if (sourceNode && targetNode) {
-      edges.push({
-        id: generateId("ge"),
-        type,
-        sourceNodeId: sourceNode.id,
-        targetNodeId: targetNode.id,
-        weight: Math.max(0, Math.min(1, weight)),
-        sourceObservationIds: observationIds,
-        createdAt: now,
-      });
-    }
+    if (!sourceNode || !targetNode) continue;
+    edges.push({
+      id: generateId("ge"),
+      type,
+      sourceNodeId: sourceNode.id,
+      targetNodeId: targetNode.id,
+      weight: Math.max(0, Math.min(1, weight)),
+      sourceObservationIds: observationIds,
+      createdAt: now,
+    });
   }
 
   return { nodes, edges };
diff --git a/test/graph.test.ts b/test/graph.test.ts
index ec08ed8c3..ca50264ce 100644
--- a/test/graph.test.ts
+++ b/test/graph.test.ts
@@ -132,6 +132,36 @@ describe("Graph Functions", () => {
     expect(edges[0].type).toBe("uses");
   });
 
+  it("graph-extract tolerates reordered attributes (#635)", async () => {
+    // Codex CLI's LLM tends to emit attribute order name→type and
+    // source→target→type rather than the hard-coded type-first /
+    // type/source/target/weight sequence the old parser required.
+    mockProvider.compress.mockResolvedValueOnce(`<entities>
+<entity name="src/index.ts" type="file"/>
+<entity name="main" type="function"><property key="lang">typescript</property></entity>
+</entities>
+<relationships>
+<relationship source="src/index.ts" target="main" type="uses" weight="0.9"/>
+</relationships>`);
+
+    const result = (await sdk.trigger("mem::graph-extract", {
+      observations: [testObs],
+    })) as { success: boolean; nodesAdded: number; edgesAdded: number };
+
+    expect(result.success).toBe(true);
+    expect(result.nodesAdded).toBe(2);
+    expect(result.edgesAdded).toBe(1);
+
+    const nodes = await kv.list<GraphNode>("mem:graph:nodes");
+    expect(nodes.find((n) => n.name === "src/index.ts")?.type).toBe("file");
+    expect(nodes.find((n) => n.name === "main")?.type).toBe("function");
+
+    const edges = await kv.list<GraphEdge>("mem:graph:edges");
+    expect(edges).toHaveLength(1);
+    expect(edges[0].type).toBe("uses");
+    expect(edges[0].weight).toBeCloseTo(0.9, 5);
+  });
+
   it("graph-query with search returns matching nodes", async () => {
     await sdk.trigger("mem::graph-extract", { observations: [testObs] });