From 5a4d610ed999e8453e5b6592e02f9fe84fda102d Mon Sep 17 00:00:00 2001
From: Vorflux AI <noreply@vorflux.com>
Date: Fri, 27 Mar 2026 02:21:03 +0000
Subject: [PATCH] Address review: improve comments, naming, add edge case tests

- Rename tip -> cursor for consistency with backward walk
- Fix misleading test descriptions to reflect bidirectional behavior
- Add test for circular parent references (cycle protection)
- Add test for branching children (first-child-wins behavior)
- Document linear chain assumption in forward walk comment
---
 .../src/__tests__/version-chain.test.ts       | 46 +++++++++++++++++--
 .../memory-graph/src/canvas/version-chain.ts  | 13 ++++--
 2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/packages/memory-graph/src/__tests__/version-chain.test.ts b/packages/memory-graph/src/__tests__/version-chain.test.ts
index 64139aa4..d5b75978 100644
--- a/packages/memory-graph/src/__tests__/version-chain.test.ts
+++ b/packages/memory-graph/src/__tests__/version-chain.test.ts
@@ -37,15 +37,15 @@ function makeDoc(id: string, memories: GraphApiMemory[]): GraphApiDocument {
 }
 
 describe("VersionChainIndex", () => {
-	it("getChain returns null for version 1 memories (no chain)", () => {
+	it("getChain returns null for standalone memory (no parent, no children)", () => {
 		const idx = new VersionChainIndex()
 		const doc = makeDoc("d1", [makeMem({ id: "m1", version: 1 })])
 		idx.rebuild([doc])
-		// version <= 1 returns null per implementation
+		// Single memory with no parent and no children — not a chain
 		expect(idx.getChain("m1")).toBeNull()
 	})
 
-	it("builds chain by walking parentMemoryId backwards then reversing", () => {
+	it("getChain from latest node returns full chain in version order", () => {
 		const idx = new VersionChainIndex()
 		const doc = makeDoc("d1", [
 			makeMem({ id: "m1", version: 1 }),
@@ -230,6 +230,46 @@ describe("VersionChainIndex", () => {
 		expect(chain2!.map((e) => e.id)).toEqual(["m3", "m4"])
 	})
 
+	it("handles circular parent references without infinite loop", () => {
+		const idx = new VersionChainIndex()
+		const doc = makeDoc("d1", [
+			makeMem({ id: "m1", version: 1, parentMemoryId: "m2" }),
+			makeMem({ id: "m2", version: 2, parentMemoryId: "m1" }),
+		])
+		idx.rebuild([doc])
+
+		// Cycle: m1->m2->m1. The visited set prevents infinite loops.
+		const chain = idx.getChain("m1")
+		expect(chain).not.toBeNull()
+		expect(chain!.length).toBe(2)
+	})
+
+	it("branching children: follows first child by document order", () => {
+		const idx = new VersionChainIndex()
+		const doc = makeDoc("d1", [
+			makeMem({ id: "m1", version: 1 }),
+			makeMem({
+				id: "m2a",
+				parentMemoryId: "m1",
+				rootMemoryId: "m1",
+				version: 2,
+			}),
+			makeMem({
+				id: "m2b",
+				parentMemoryId: "m1",
+				rootMemoryId: "m1",
+				version: 2,
+			}),
+		])
+		idx.rebuild([doc])
+
+		// m1 has two children; forward walk picks the first (m2a)
+		const chain = idx.getChain("m1")
+		expect(chain).not.toBeNull()
+		expect(chain!.length).toBe(2)
+		expect(chain!.map((e) => e.id)).toEqual(["m1", "m2a"])
+	})
+
 	it("chain entries have correct fields", () => {
 		const idx = new VersionChainIndex()
 		const doc = makeDoc("d1", [
diff --git a/packages/memory-graph/src/canvas/version-chain.ts b/packages/memory-graph/src/canvas/version-chain.ts
index a17bd97a..e6a9cbb3 100644
--- a/packages/memory-graph/src/canvas/version-chain.ts
+++ b/packages/memory-graph/src/canvas/version-chain.ts
@@ -56,17 +56,20 @@ export class VersionChainIndex {
 		}
 		backward.reverse()
 
-		// Walk forward from the selected node to find descendants
+		// Walk forward from the selected node to find descendants.
+		// Version chains are linear (each memory has one parent), so we
+		// follow the first child at each step. If branching occurs, only
+		// the first branch (by document order) is included.
 		const forward: GraphApiMemory[] = []
-		let tip: GraphApiMemory | undefined = mem
-		while (tip) {
-			const children = this.childrenMap.get(tip.id)
+		let cursor: GraphApiMemory | undefined = mem
+		while (cursor) {
+			const children = this.childrenMap.get(cursor.id)
 			if (!children || children.length === 0) break
 			const child = this.memoryMap.get(children[0])
 			if (!child || visited.has(child.id)) break
 			visited.add(child.id)
 			forward.push(child)
-			tip = child
+			cursor = child
 		}
 
 		// Combine: backward (root..selected) + forward (selected+1..latest)