Fix review bugs: slideshow re-mount, edges cascade, stale popover, maxNodes

- Fix slideshow useEffect: use nodesRef instead of nodes in deps to prevent
  interval teardown/recreation on every render
- Fix edges useMemo: compute allNodeIds from normalizedDocs directly instead
  of depending on nodes (which changes identity every render)
- Fix popover position: add zoomDisplay to deps so position updates on pan/zoom
- Implement maxNodes prop: limit documents before passing to useGraphData
- Fix SpatialIndex hash: include node IDs and use 10x position granularity
  to prevent false cache hits during physics simulation
This commit is contained in:
Vorflux AI 2026-03-26 18:04:25 +00:00
parent 1c5ef7e5ed
commit 4dcf3b2a62
3 changed files with 41 additions and 14 deletions

View file

@ -58,8 +58,16 @@ export class SpatialIndex {
private computeHash(nodes: GraphNode[]): number {
let hash = nodes.length
for (const n of nodes) {
hash = (hash * 31 + (Math.round(n.x) | 0)) | 0
hash = (hash * 31 + (Math.round(n.y) | 0)) | 0
// Use finer granularity (10x) to detect sub-pixel movements
// and incorporate a simple string hash of the ID to avoid
// false matches when nodes swap positions
let idHash = 0
for (let i = 0; i < n.id.length; i++) {
idHash = ((idHash << 5) - idHash + n.id.charCodeAt(i)) | 0
}
hash = (hash * 31 + idHash) | 0
hash = (hash * 31 + (Math.round(n.x * 10) | 0)) | 0
hash = (hash * 31 + (Math.round(n.y * 10) | 0)) | 0
}
return hash
}

View file

@ -27,6 +27,7 @@ export function MemoryGraph({
highlightDocumentIds = [],
highlightsVisible = true,
showFps = false,
maxNodes,
isSlideshowActive = false,
onSlideshowNodeChange,
onSlideshowStop: _onSlideshowStop,
@ -51,8 +52,14 @@ export function MemoryGraph({
const [selectedNode, setSelectedNode] = useState<string | null>(null)
const [zoomDisplay, setZoomDisplay] = useState(50)
// Limit documents if maxNodes is set
const limitedDocuments = useMemo(() => {
if (!maxNodes || documents.length <= maxNodes) return documents
return documents.slice(0, maxNodes)
}, [documents, maxNodes])
const { nodes, edges } = useGraphData(
documents,
limitedDocuments,
apiEdges,
null,
containerSize.width,
@ -62,8 +69,8 @@ export function MemoryGraph({
// Rebuild version chain index when documents change
useEffect(() => {
chainIndex.current.rebuild(documents)
}, [documents])
chainIndex.current.rebuild(limitedDocuments)
}, [limitedDocuments])
// Smart simulation re-init: track node ID set, only init() when IDs change
const prevSimIdsRef = useRef<string>("")
@ -386,7 +393,10 @@ export function MemoryGraph({
return () => window.removeEventListener("keydown", handler)
}, [navigateUp, navigateDown, navigateNext, navigatePrev])
// Slideshow
// Slideshow — use a ref for nodes to avoid re-creating the interval on every render
const nodesRef = useRef(nodes)
nodesRef.current = nodes
useEffect(() => {
if (!isSlideshowActive || nodes.length === 0) {
if (!isSlideshowActive) {
@ -398,17 +408,18 @@ export function MemoryGraph({
let lastIdx = -1
const pick = () => {
if (nodes.length === 0) return
const currentNodes = nodesRef.current
if (currentNodes.length === 0) return
let idx: number
if (nodes.length > 1) {
if (currentNodes.length > 1) {
do {
idx = Math.floor(Math.random() * nodes.length)
idx = Math.floor(Math.random() * currentNodes.length)
} while (idx === lastIdx)
} else {
idx = 0
}
lastIdx = idx
const n = nodes[idx]!
const n = currentNodes[idx]!
setSelectedNode(n.id)
viewportRef.current?.centerOn(
n.x,
@ -426,7 +437,7 @@ export function MemoryGraph({
return () => clearInterval(interval)
}, [
isSlideshowActive,
nodes,
nodes.length,
containerSize.width,
containerSize.height,
onSlideshowNodeChange,
@ -448,7 +459,9 @@ export function MemoryGraph({
screenY: screen.y,
nodeRadius: (activeNodeData.size * vp.zoom) / 2,
}
}, [activeNodeData])
// zoomDisplay triggers re-computation on viewport changes (pan/zoom)
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [activeNodeData, zoomDisplay])
const activeVersionChain = useMemo(() => {
if (!activeNodeData || activeNodeData.type !== "memory") return null

View file

@ -183,7 +183,13 @@ export function useGraphData(
if (!normalizedDocs || normalizedDocs.length === 0) return []
const result: GraphEdge[] = []
const allNodeIds = new Set(nodes.map((n) => n.id))
// Build allNodeIds from normalizedDocs directly to avoid depending on `nodes`
// (which changes identity on every render due to draggingNodeId/colors deps)
const allNodeIds = new Set<string>()
for (const doc of normalizedDocs) {
allNodeIds.add(doc.id)
for (const mem of doc.memories) allNodeIds.add(mem.id)
}
for (const doc of normalizedDocs) {
for (const mem of doc.memories) {
@ -229,7 +235,7 @@ export function useGraphData(
}
return result
}, [normalizedDocs, apiEdges, nodes])
}, [normalizedDocs, apiEdges])
return { nodes, edges, scale, offsetX, offsetY }
}