Rewrite @supermemory/memory-graph with perf optimizations + consolidate consumers#809
Rewrite @supermemory/memory-graph with perf optimizations + consolidate consumers#809vorflux[bot] wants to merge 18 commits intomainfrom
Conversation
…date consumers Package changes (packages/memory-graph): - Spatial grid nearest-neighbor: O(n^2) -> O(n*k) for doc-doc edge rendering - Zoom-based edge culling: skip low-similarity edges at low zoom levels - Modular canvas engine from mono repo (renderer, simulation, viewport, hit-test) - Remove vanilla-extract CSS, use inline styles only - Add motion/react for slide-in animations - Export mock data generator for stress testing - Add performance and integration tests (15/15 pass) Web app changes (apps/web): - Remove local graph implementation (~4400 lines deleted) - Add thin wrapper (memory-graph-wrapper.tsx) using useGraphApi hook - Keep app-specific graph-card.tsx and use-graph-api.ts Playground changes (apps/memory-graph-playground): - Add stress test buttons (50/100/200/500 docs) - Add mock data generation via package - Convert DocumentWithMemories to GraphApiDocument format - Show FPS counter during stress tests All TypeScript compiles clean. Performance tests: 15/15 pass.
…xNodes - 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
79 tests across 6 test files covering: - SpatialIndex: grid rebuild, queryPoint hit-testing, boundary cells, hash detection - ViewportState: worldToScreen/screenToWorld roundtrip, pan, zoomImmediate, zoomTo animation, fitToNodes, centerOn, inertia decay - ForceSimulation: init/destroy lifecycle, update hot-swap, reheat/coolDown - VersionChainIndex: chain building via parentMemoryId, caching, rebuild - Graph data utils: normalizeDocCoordinates, getMemoryBorderColor, getEdgeVisualProps, screenToBackendCoords, calculateBackendViewport - Mock data: deterministic seeded output, correct counts, valid edge references Also exports pure utility functions from use-graph-data.ts for testability.
|
Automatic Review Skipped Too many files for automatic review. If you would still like a review, you can trigger one manually by commenting: |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
supermemory-app | 8dced3d | Commit Preview URL Branch Preview URL |
Mar 27 2026, 06:54 AM |
….POSITIVE_INFINITY, line length formatting
…for npm publish - Package exports now point to source files (./src/index.tsx) for workspace consumers, allowing Next.js/Turbopack to resolve without pre-building - Added publishConfig swap script that switches exports to dist/ during npm publish (prepublishOnly) and back to source after (postpublish) - Added @supermemory/memory-graph to transpilePackages in web app next.config.ts so Next.js transpiles the source files correctly
… swap prepack/postpack fire for both 'pack' and 'publish', and postpack always runs after packing regardless of publish success, preventing the working tree from being left in dist-exports state on failure.
Memory nodes with isLatest=false are now visually dimmed: - Full-size nodes: 35% opacity with dashed border stroke - Zoomed-out dots: 35% opacity with thinner stroke - Selected/hovered superseded nodes render at full opacity so users can still inspect them
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| @@ -33,7 +33,6 @@ export class VersionChainIndex { | |||
| const mem = this.memoryMap.get(memoryId) | |||
| if (!mem || mem.version <= 1) return null | |||
|
|
|||
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Added childrenMap for forward traversal from parent to children - getChain now walks backward to root AND forward to latest descendant - Clicking v1 root node now shows the complete version chain UI - Standalone v1 nodes (no children) still return null (no chain to show) - Updated tests to reflect bidirectional behavior + added v1 root test
- 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
| )} | ||
|
|
||
| {/* Graph container */} | ||
| <div className={styles.graphContainer} ref={containerRef}> | ||
| <div style={canvasContainerStyle} ref={containerRef}> | ||
| {containerSize.width > 0 && containerSize.height > 0 && ( |
There was a problem hiding this comment.
Bug: GraphCanvas receives an undefined simulation prop on initial render because mutating simulationRef doesn't trigger a re-render, causing the graph to appear frozen.
Severity: HIGH
Suggested Fix
To ensure GraphCanvas receives the simulation object immediately after it's created, trigger a re-render. This can be achieved by moving the simulation object into React state using useState instead of useRef. For example: const [simulation, setSimulation] = useState<ForceSimulation | null>(null); and then call setSimulation(newSimulation); inside the effect.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/memory-graph/src/components/memory-graph.tsx#L548-L551
Potential issue: The `simulationRef` is initialized to `null` and populated within a
`useEffect` hook. However, mutating a ref does not trigger a component re-render.
Consequently, on its initial render, the `GraphCanvas` component receives
`simulation={undefined}` because `simulationRef.current` is still `null`. The render
loop inside `GraphCanvas` checks `simulation?.isActive()` to determine if it should draw
a new frame. Since the simulation object is initially undefined, this check fails, and
the canvas does not update, making the graph appear frozen while the physics simulation
runs in the background.
There was a problem hiding this comment.
Good catch. This was a real bug -- mutating simulationRef doesn't trigger a re-render, so GraphCanvas received undefined for the simulation prop on initial render. The render loop's cb.current.simulation?.isActive() ?? false would always be false, causing frames to be skipped even while the simulation was running and mutating node positions.
Fixed in 13eb5e9 by adding a useState for the simulation object alongside the ref. The state triggers a re-render when the simulation is first created, ensuring GraphCanvas receives it. The ref is kept for imperative access in callbacks (reheat/coolDown).
- Use useState for simulation so GraphCanvas receives it after creation (ref mutation alone didn't trigger re-render, causing frozen graph) - Keep simulationRef for imperative access (reheat/coolDown in callbacks) - Add edge case tests: orphaned non-root memory, cross-document chains, circular references with membership assertion, cold-cache middle node
MaheshtheDev
left a comment
MaheshtheDev
left a comment
There was a problem hiding this comment.
this looks good to me but have to test in prod as well with heavy load
…rgotten X icon, version edge glow - Drop shadows on selected/hovered nodes (document + memory) for depth - Subtle gradient fill on document nodes - Improved dimming contrast for superseded memories (0.3 alpha + strikethrough) - Hover glow ring with dashed outline for interactivity feedback - X icon overlay on forgotten memory nodes (size > 14px) - Glow pass behind version edges for visual emphasis - lightenColor utility with cache + guard for non-6-digit hex - Use memData.isForgotten instead of fragile color comparison - Optimize version edge batch: skip filter, use first.isVersion flag - Add 10 unit tests for lightenColor (hex parsing, clamping, cache, edge cases)
| if (!activeNodeData || !viewportRef.current) return null | ||
| const vp = viewportRef.current | ||
| const screen = vp.worldToScreen(activeNodeData.x, activeNodeData.y) | ||
| return { | ||
| screenX: screen.x, | ||
| screenY: screen.y, | ||
| nodeRadius: (activeNodeData.size * vp.zoom) / 2, | ||
| } | ||
| // zoomDisplay triggers re-computation on viewport changes (pan/zoom) | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [activeNodeData, zoomDisplay]) |
There was a problem hiding this comment.
Bug: The node popover's position does not update during pan operations because its recalculation is incorrectly tied only to zoom changes, causing it to appear detached from the node.
Severity: MEDIUM
Suggested Fix
Modify the onViewportChange callback in graph-canvas.tsx to trigger on pan events in addition to zoom events. This will ensure that any viewport movement causes a state update, forcing the activePopoverPosition useMemo hook to recalculate the popover's position correctly during both pan and zoom actions.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/memory-graph/src/components/memory-graph.tsx#L458-L469
Potential issue: The `activePopoverPosition` `useMemo` hook, which calculates a node's
popover screen coordinates, only re-evaluates when the `zoomDisplay` state changes. The
`onViewportChange` callback in `graph-canvas.tsx` is responsible for updating this
state, but it is only triggered on zoom events, not on pan events. Consequently, when a
user pans the graph without zooming, the popover's position is not recalculated. This
causes the popover to remain fixed at its old screen coordinates while the associated
node moves, making it appear disconnected from the node it is supposed to be annotating.
- Change bun:test import to vitest for CI compatibility - Add viewportVersion counter for popover recalculation during pan - Add hex color guard in lightenColor for non-6-digit formats - Optimize version edge filtering with batch property - Simplify stroke style conditions in drawDocumentNode
| // Module-level reusable batch map – cleared each frame instead of reallocating | ||
| const edgeBatches = new Map<string, PreparedEdge[]>() |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Fixed. viewportVersion is now in the activePopoverPosition dependency array (replacing zoomDisplay), and the onViewportChange callback now fires based on actual viewport state changes (panX/panY/zoom comparison) rather than only during animated transitions (vpMoving). This covers mouse-drag panning, momentum, spring zoom, and simulation-driven node movement.
- Use viewportVersion (not zoomDisplay) as activePopoverPosition dependency - Fire onViewportChange based on actual viewport state diff (panX/panY/zoom) instead of only during animated transitions (vpMoving) - Also fire during simulation so popover tracks settling nodes - Remove dead lastReportedZoom variable - Remove no-op node ID tracking effect and prevIdsRef
| for (let i = 0; i < docs.length; i++) { | ||
| const d = docs[i]! | ||
| const cx = Math.floor(d.x / CELL) | ||
| const cy = Math.floor(d.y / CELL) | ||
| let best1 = -1 | ||
| let best2 = -1 | ||
| let dist1 = Number.POSITIVE_INFINITY | ||
| let dist2 = Number.POSITIVE_INFINITY | ||
|
|
||
| for (let j = 0; j < docs.length; j++) { | ||
| if (j === i) continue | ||
| const dx = docs[j]!.x - d.x | ||
| const dy = docs[j]!.y - d.y | ||
| const dist = dx * dx + dy * dy | ||
| if (dist < dist1) { | ||
| best2 = best1 | ||
| dist2 = dist1 | ||
| best1 = j | ||
| dist1 = dist | ||
| } else if (dist < dist2) { | ||
| best2 = j | ||
| dist2 = dist | ||
| for (let dx = -1; dx <= 1; dx++) { | ||
| for (let dy = -1; dy <= 1; dy++) { |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
This is by design. The coordinates are in world space, not screen space -- the force simulation constrains document positions within a bounded area where 200px cells with a 3x3 search (600px effective radius) covers the vast majority of nearest-neighbor pairs. For typical graphs (50-500 docs), documents are always within this radius of their nearest neighbors.
The only edge case would be 2-3 documents spread extremely far apart, where the missing dashed cosmetic line has negligible visual impact. The O(n*k) performance benefit (vs O(n^2)) is significant for larger graphs -- at 500 docs, this avoids 250,000 distance comparisons per frame.
If this becomes a visual issue in practice, we can add a fallback that does a brute-force scan only when best1 === -1 (no neighbors found at all), which would be rare and wouldn't affect the common-case performance.
| nodes.length, | ||
| containerSize.width, |
There was a problem hiding this comment.
Bug: The slideshow useEffect hook includes containerSize in its dependencies, causing the slideshow interval to reset and jump to a new node on any container resize.
Severity: MEDIUM
Suggested Fix
Create a ref for containerSize, similar to how nodesRef is used. Update this ref with the latest containerSize on each render. Inside the pick function, read the dimensions from containerSizeRef.current. Finally, remove containerSize.width and containerSize.height from the useEffect dependency array to prevent the interval from being reset on resize.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: packages/memory-graph/src/components/memory-graph.tsx#L443-L444
Potential issue: The `useEffect` hook that manages the slideshow feature incorrectly
includes `containerSize.width` and `containerSize.height` in its dependency array. A
`ResizeObserver` updates these values on any container resize, causing the effect to
re-run. The effect's cleanup function clears the 3.5-second slideshow interval, and the
effect body immediately picks a new node. This interrupts the slideshow's intended
cadence, causing it to abruptly jump to a new node whenever the window is resized,
creating a jarring user experience.
There was a problem hiding this comment.
Good catch. Fixed in 8dced3d5 -- moved containerSize and onSlideshowNodeChange to refs (containerSizeRef, onSlideshowNodeChangeRef) so the slideshow useEffect reads them via .current instead of closing over the values. The dependency array is now just [isSlideshowActive, nodes.length], so the interval only resets when the slideshow is toggled or the node count changes, not on container resize.
Use refs for containerSize and onSlideshowNodeChange inside the slideshow useEffect to avoid resetting the 3.5s interval when the container is resized. The interval now only resets when isSlideshowActive or nodes.length changes.
1 participant
Summary
Rewrites the
@supermemory/memory-graphpackage with a high-performance canvas engine and consolidates duplicate graph implementations fromapps/webinto the shared package. Net result: ~10,900 lines deleted, ~3,400 lines added.Package changes (
packages/memory-graph)Performance optimizations:
Architecture:
renderer.ts,simulation.ts,viewport.ts,hit-test.ts,input-handler.ts,version-chain.tsmotion/reactfor slide-in panel animations@supermemory/memory-graph/mock-data) for stress testingBug fixes (from review):
useEffectinfinite re-mount (usednodesRefinstead ofnodesin deps)edgesuseMemo cascading re-computation (computeallNodeIdsfromnormalizedDocsdirectly)zoomDisplayto deps)maxNodesprop (was declared but unused)SpatialIndexhash collisions (include node IDs, 10x position granularity)simulationRefmutation didn't cause re-render, soGraphCanvasreceivedundefinedon initial render, causing the graph to appear frozen. Fixed by addinguseStatealongside the ref.getChain()returnednullfor v1 root nodes, hiding the version chain UI. Added bidirectional traversal (backward to root + forward to latest descendant) with achildrenMapindex.isLatest === falsememory nodes.Workspace resolution (source exports):
./src/index.tsx) for workspace consumers, so Next.js/Turbopack can resolve without pre-building the packageprepack/postpackscript pair (scripts/swap-exports.ts) swaps exports todist/during npm publish and restores source exports afterward@supermemory/memory-graphadded totranspilePackagesinapps/web/next.config.tsdist/files don't existWeb app changes (
apps/web)memory-graph-wrapper.tsx) usinguseGraphApihookgraph-card.tsxanduse-graph-api.tsPlayground changes (
apps/memory-graph-playground)Testing
Unit Tests (86/86 pass)
spatial-index.test.ts(11 tests): grid rebuild, queryPoint hit-testing, boundary cells, hash detectionviewport.test.ts(21 tests): worldToScreen/screenToWorld roundtrip, pan, zoom, fitToNodes, centerOn, inertiasimulation.test.ts(8 tests): init/destroy lifecycle, update hot-swap, reheat/coolDownversion-chain.test.ts(17 tests): chain building via parentMemoryId, bidirectional traversal, caching, rebuild, edge cases (orphaned nodes, cross-document chains, circular references, branching children)graph-data-utils.test.ts(20 tests): normalizeDocCoordinates, getMemoryBorderColor, getEdgeVisualProps, coordinate transformsmock-data.test.ts(9 tests): deterministic seeded output, correct counts, valid edge referencesTypeScript Compilation
packages/memory-graph:npx tsc --noEmit-- 0 errorsapps/web: 0 graph-related errors (pre-existing errors in unrelated files only)apps/memory-graph-playground:npx tsc --noEmit-- 0 errorsPackage Build
UI Stress Test (Playground)
Tested via browser automation at
localhost:3000:Screenshots:
Video recording:
CI Status
Dependency Ordering
This PR must merge and the package must be published to npm as v0.2.0 before the mono PR #1311 can merge.
Merge order:
cd packages/memory-graph && bun publish(prepack script handles export swap automatically)Companion PR
apps/console-v2Attached Images and Videos
View in Vorflux Session
Session Details
(aside)to your comment to have me ignore it.