Address memory graph review feedback

packages/memory-graph/src/__tests__/version-chain.test.ts

@@ -90,6 +90,31 @@ describe("VersionChainIndex", () => {
 		expect(chain?.map((e) => e.version)).toEqual([1, 2])
 	})
 
+	it("only infers broken version entries and preserves valid backend versions", () => {
+		const idx = new VersionChainIndex()
+		const doc = makeDoc("d1", [
+			makeMem({ id: "m1", version: 1 }),
+			makeMem({
+				id: "m2",
+				parentMemoryId: "m1",
+				rootMemoryId: "m1",
+				version: 5,
+			}),
+			makeMem({
+				id: "m3",
+				parentMemoryId: "m2",
+				rootMemoryId: "m1",
+				version: 5,
+			}),
+		])
+		idx.rebuild([doc])
+
+		const chain = idx.getChain("m3")
+		expect(chain).not.toBeNull()
+		expect(chain?.map((e) => e.id)).toEqual(["m1", "m2", "m3"])
+		expect(chain?.map((e) => e.version)).toEqual([1, 5, 6])
+	})
+
 	it("getChain from middle element returns full chain (backward + forward)", () => {
 		const idx = new VersionChainIndex()
 		const doc = makeDoc("d1", [

packages/memory-graph/src/canvas/renderer.ts

@@ -7,6 +7,7 @@ import type {
 } from "../types"
 import type { ViewportState } from "./viewport"
 import { drawDocIcon, roundRect } from "./document-icons"
+import { hashString } from "../utils/hash"
 
 export interface RenderState {
 	selectedNodeId: string | null
@@ -157,14 +158,6 @@ function applyRelationLevelOfDetail(
 	}
 }
 
-function hashString(value: string): number {
-	let hash = 0
-	for (let i = 0; i < value.length; i++) {
-		hash = (Math.imul(31, hash) + value.charCodeAt(i)) | 0
-	}
-	return hash >>> 0
-}
-
 function clampNumber(value: number, min: number, max: number): number {
 	return value < min ? min : value > max ? max : value
 }

packages/memory-graph/src/canvas/version-chain.ts

@@ -80,19 +80,22 @@ export class VersionChainIndex {
 		// A single-entry chain (standalone v1 with no children) is not useful
 		if (all.length <= 1) return null
 
-		const shouldInferVersions = all.some((m, index) => {
-			if (!Number.isFinite(m.version) || m.version < 1) return true
-			const previous = all[index - 1]
-			return previous != null && m.version <= previous.version
-		})
+		let lastVersion = 0
+		const chain: ChainEntry[] = all.map((m) => {
+			const version =
+				Number.isFinite(m.version) && m.version > lastVersion
+					? m.version
+					: lastVersion + 1
+			lastVersion = version
 
-		const chain: ChainEntry[] = all.map((m, index) => ({
-			id: m.id,
-			version: shouldInferVersions ? index + 1 : m.version,
-			memory: m.memory,
-			isForgotten: m.isForgotten,
-			isLatest: m.isLatest,
-		}))
+			return {
+				id: m.id,
+				version,
+				memory: m.memory,
+				isForgotten: m.isForgotten,
+				isLatest: m.isLatest,
+			}
+		})
 
 		for (const entry of chain) {
 			this.cache.set(entry.id, chain)

packages/memory-graph/src/hooks/use-graph-data.ts

@@ -1,4 +1,4 @@
-import { useMemo, useRef } from "react"
+import { useEffect, useMemo, useRef } from "react"
 import type {
 	DocumentNodeData,
 	GraphApiDocument,
@@ -8,6 +8,7 @@ import type {
 	GraphThemeColors,
 	MemoryNodeData,
 } from "../types"
+import { hashString } from "../utils/hash"
 
 const SEVEN_DAYS_MS = 7 * 24 * 60 * 60 * 1000
 const ONE_DAY_MS = 24 * 60 * 60 * 1000
@@ -18,6 +19,7 @@ const APPEND_CLUSTER_RADIUS = MEMORY_ORBIT_BASE + 180
 const APPEND_AREA_GAP = 160
 const APPEND_CANDIDATES_PER_RING = 18
 const APPEND_MAX_RINGS = 8
+const APPEND_SPATIAL_CELL_SIZE = APPEND_CLUSTER_RADIUS + APPEND_AREA_GAP + 120
 const GOLDEN_ANGLE = Math.PI * (3 - Math.sqrt(5))
 const CLUSTER_COLORS = [
 	"#58C7E8",
@@ -255,25 +257,13 @@ function connect(map: Map<string, Set<string>>, a: string, b: string) {
 	map.get(b)?.add(a)
 }
 
-function hashString(value: string): number {
-	let hash = 0
-	for (let i = 0; i < value.length; i++) {
-		hash = (Math.imul(31, hash) + value.charCodeAt(i)) | 0
-	}
-	return hash >>> 0
-}
-
 /**
  * Simple deterministic hash of a string to a number in [0, 1).
  * Used for initial node placement so the force simulation has a
  * deterministic starting layout.
  */
 function hashToUnit(str: string): number {
-	let h = 0
-	for (let i = 0; i < str.length; i++) {
-		h = (Math.imul(31, h) + str.charCodeAt(i)) | 0
-	}
-	return ((h >>> 0) % 10000) / 10000
+	return (hashString(str) % 10000) / 10000
 }
 
 export function getNodeBounds(nodes: GraphNode[]) {
@@ -313,6 +303,7 @@ export function getAppendPosition(
 		return { x: canvasWidth / 2, y: canvasHeight / 2 }
 	}
 
+	const spatialGrid = buildAppendSpatialGrid(existingNodes)
 	const boundsWidth = bounds.maxX - bounds.minX
 	const boundsHeight = bounds.maxY - bounds.minY
 	const baseRadiusX = boundsWidth / 2 + APPEND_CLUSTER_RADIUS + APPEND_AREA_GAP
@@ -332,7 +323,7 @@ export function getAppendPosition(
 				y: bounds.centerY + Math.sin(angle) * radiusY,
 			}
 
-			if (isAppendCandidateOpen(candidate, existingNodes)) {
+			if (isAppendCandidateOpen(candidate, spatialGrid)) {
 				return candidate
 			}
 		}
@@ -349,20 +340,54 @@ export function getAppendPosition(
 
 function isAppendCandidateOpen(
 	candidate: { x: number; y: number },
-	existingNodes: GraphNode[],
+	spatialGrid: Map<string, GraphNode[]>,
 ) {
-	for (const node of existingNodes) {
-		const minDistance = APPEND_CLUSTER_RADIUS + node.size / 2 + APPEND_AREA_GAP
-		const dx = candidate.x - node.x
-		if (Math.abs(dx) > minDistance) continue
-		const dy = candidate.y - node.y
-		if (Math.abs(dy) > minDistance) continue
-		if (dx * dx + dy * dy < minDistance * minDistance) return false
+	const cellX = getAppendSpatialCell(candidate.x)
+	const cellY = getAppendSpatialCell(candidate.y)
+	for (let x = cellX - 1; x <= cellX + 1; x++) {
+		for (let y = cellY - 1; y <= cellY + 1; y++) {
+			const nodes = spatialGrid.get(getAppendSpatialKey(x, y))
+			if (!nodes) continue
+			for (const node of nodes) {
+				const minDistance =
+					APPEND_CLUSTER_RADIUS + node.size / 2 + APPEND_AREA_GAP
+				const dx = candidate.x - node.x
+				if (Math.abs(dx) > minDistance) continue
+				const dy = candidate.y - node.y
+				if (Math.abs(dy) > minDistance) continue
+				if (dx * dx + dy * dy < minDistance * minDistance) return false
+			}
+		}
 	}
 
 	return true
 }
 
+function buildAppendSpatialGrid(nodes: GraphNode[]): Map<string, GraphNode[]> {
+	const grid = new Map<string, GraphNode[]>()
+	for (const node of nodes) {
+		const key = getAppendSpatialKey(
+			getAppendSpatialCell(node.x),
+			getAppendSpatialCell(node.y),
+		)
+		const bucket = grid.get(key)
+		if (bucket) {
+			bucket.push(node)
+		} else {
+			grid.set(key, [node])
+		}
+	}
+	return grid
+}
+
+function getAppendSpatialCell(value: number): number {
+	return Math.floor(value / APPEND_SPATIAL_CELL_SIZE)
+}
+
+function getAppendSpatialKey(x: number, y: number): string {
+	return `${x}:${y}`
+}
+
 /**
  * Pure function that computes graph edges from documents.
  * Extracted from the hook for testability.
@@ -428,10 +453,12 @@ export function useGraphData(
 ) {
 	const nodeCache = useRef<Map<string, GraphNode>>(new Map())
 
-	const nodes = useMemo(() => {
+	const graphData = useMemo<{
+		nodes: GraphNode[]
+		cache: Map<string, GraphNode>
+	}>(() => {
 		if (!documents || documents.length === 0) {
-			nodeCache.current.clear()
-			return []
+			return { nodes: [], cache: new Map<string, GraphNode>() }
 		}
 
 		const currentIds = new Set<string>()
@@ -440,11 +467,11 @@ export function useGraphData(
 			for (const mem of doc.memories) currentIds.add(mem.id)
 		}
 
-		for (const [id] of nodeCache.current.entries()) {
-			if (!currentIds.has(id)) nodeCache.current.delete(id)
-		}
-
-		const appendPlacementNodes = Array.from(nodeCache.current.values())
+		const previousCache = nodeCache.current
+		const nextCache = new Map<string, GraphNode>()
+		const appendPlacementNodes = Array.from(previousCache.values()).filter(
+			(node) => currentIds.has(node.id),
+		)
 		let appendIndex = 0
 		const clusterAssignments = computeClusterAssignments(documents)
 
@@ -469,7 +496,7 @@ export function useGraphData(
 			const initialX = cx + Math.cos(angle) * radius
 			const initialY = cy + Math.sin(angle) * radius
 
-			let docNode = nodeCache.current.get(doc.id)
+			const previousDocNode = previousCache.get(doc.id)
 			const docData: DocumentNodeData = {
 				id: doc.id,
 				title: doc.title,
@@ -480,12 +507,16 @@ export function useGraphData(
 				memories: doc.memories,
 			}
 
-			if (docNode) {
-				docNode.data = docData
-				docNode.borderColor = docCluster.color
-				docNode.clusterKey = docCluster.key
-				docNode.clusterColor = docCluster.color
-				docNode.isDragging = draggingNodeId === doc.id
+			let docNode: GraphNode
+			if (previousDocNode) {
+				docNode = {
+					...previousDocNode,
+					data: docData,
+					borderColor: docCluster.color,
+					clusterKey: docCluster.key,
+					clusterColor: docCluster.color,
+					isDragging: draggingNodeId === doc.id,
+				}
 			} else {
 				const appendPosition =
 					appendPlacementNodes.length > 0
@@ -510,36 +541,35 @@ export function useGraphData(
 					isHovered: false,
 					isDragging: false,
 				}
-				nodeCache.current.set(doc.id, docNode)
 				appendPlacementNodes.push(docNode)
 			}
+			nextCache.set(doc.id, docNode)
 			result.push(docNode)
 
 			const memCount = doc.memories.length
 			for (let i = 0; i < memCount; i++) {
 				const mem = doc.memories[i]
 				if (!mem) continue
-				let memNode = nodeCache.current.get(mem.id)
+				const previousMemNode = previousCache.get(mem.id)
 				const memData: MemoryNodeData = {
 					...mem,
 					documentId: doc.id,
 					content: mem.memory,
 				}
+				const cluster = clusterAssignments.get(mem.id)
 
-				if (memNode) {
-					memNode.data = memData
-					const cluster = clusterAssignments.get(mem.id)
-					memNode.borderColor = getMemoryNodeBorderColor(
-						mem,
-						colors,
-						cluster?.color,
-					)
-					memNode.clusterKey = cluster?.key ?? null
-					memNode.clusterColor = cluster?.color ?? null
-					memNode.isDragging = draggingNodeId === mem.id
+				let memNode: GraphNode
+				if (previousMemNode) {
+					memNode = {
+						...previousMemNode,
+						data: memData,
+						borderColor: getMemoryNodeBorderColor(mem, colors, cluster?.color),
+						clusterKey: cluster?.key ?? null,
+						clusterColor: cluster?.color ?? null,
+						isDragging: draggingNodeId === mem.id,
+					}
 				} else {
 					const memOffset = getMemoryOrbitOffset(i, memCount, mem.id)
-					const cluster = clusterAssignments.get(mem.id)
 					memNode = {
 						id: mem.id,
 						type: "memory",
@@ -553,17 +583,21 @@ export function useGraphData(
 						isHovered: false,
 						isDragging: false,
 					}
-					nodeCache.current.set(mem.id, memNode)
 					appendPlacementNodes.push(memNode)
 				}
+				nextCache.set(mem.id, memNode)
 				result.push(memNode)
 			}
 		}
 
-		return result
+		return { nodes: result, cache: nextCache }
 	}, [documents, canvasWidth, canvasHeight, draggingNodeId, colors])
 
+	useEffect(() => {
+		nodeCache.current = graphData.cache
+	}, [graphData.cache])
+
 	const edges = useMemo(() => computeEdges(documents), [documents])
 
-	return { nodes, edges }
+	return { nodes: graphData.nodes, edges }
 }

packages/memory-graph/src/utils/hash.ts

@@ -0,0 +1,7 @@
+export function hashString(value: string): number {
+	let hash = 0
+	for (let i = 0; i < value.length; i++) {
+		hash = (Math.imul(31, hash) + value.charCodeAt(i)) | 0
+	}
+	return hash >>> 0
+}