Feature/graph visualization clustering improvements #42
Conversation
- Add server URL input field with default Railway URL - Use setServerConfig to persist both URL and token - Test connection before saving credentials - Improve error display with styled alert box 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add flask-cors dependency - Enable CORS on all routes for cross-origin browser access 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Backend: - Add viewer blueprint to serve React SPA from /viewer/ - Support hash-based token auth (#token=xxx) for embedded mode - Enable CORS for cross-origin browser access - Optional via ENABLE_GRAPH_VIEWER env (default: true) Frontend (packages/graph-viewer/): - 3D WebGL graph visualization with React Three Fiber - Force-directed layout using d3-force-3d - Search, filter, and community isolation features - Inspector panel for memory details - Configurable server URL and token Infrastructure: - Multi-stage Dockerfile (Node.js build + Python runtime) - Frontend built during Docker build, not committed to repo Access at: https://your-server/viewer/#token=YOUR_API_TOKEN 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The viewer serves static files that don't need API auth. Authentication is handled client-side via URL hash fragment (#token=xxx). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Assets were loading from root (/) instead of /viewer/static/, causing 401 errors since those paths require authentication. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Implements Meta Quest-style hand gesture controls using MediaPipe Hands: - Two-hand spread/pinch for zoom in/out - Two-hand rotation for orbiting camera - Two-hand pan for moving view - Hand skeleton wireframe overlay (cyan left, magenta right) - Toggle button in header to enable/disable gesture control Requires camera permission when enabled. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Add GestureDebugOverlay component showing all hand tracking data: - FPS counter, tracking status, hands detected - Two-hand gesture values (distance, rotation, zoom/rotate/pan deltas) - Single-hand gestures (pinch strength, grab strength with progress bars) - Pinch ray data (origin, direction, strength) for both hands - Key landmark positions for each detected hand - Implement Meta Quest-style pinch ray (laser pointer): - Ray origin at midpoint between thumb and index tips - Direction from wrist toward pinch point - Visual intensity based on pinch strength - Glow sphere and ring at origin when pinch active - Dashed line when not fully pinched, solid when engaged - Integrate debug overlay into App with toggle button: - Debug button appears when gestures are enabled - Real-time gesture state updates via callback 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Create Hand2DOverlay component using SVG for crisp rendering - Hand appears as screen overlay (not inside 3D scene) - Scale based on hand depth: closer to camera = larger hand - Include pinch-to-laser with glow effects - Mirror X axis to match natural hand movement - Disable 3D HandSkeletonOverlay in favor of 2D version The hand now appears life-size against the screen, matching the user's actual hand position relative to the camera. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Remove X-axis mirroring so hand points same direction as user's - Invert depth scaling: closer to camera = smaller (going into screen) - Laser now points toward center of graph (the nexus) - Hand shrinks as it "reaches into" the screen toward the memory graph 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… and two-hand support - Add position interpolation with SMOOTHING_FACTOR for fluid hand tracking - Implement ghost effect with configurable fade when hand disappears - Track both left (cyan) and right (magenta) hands - Lasers always point toward center nexus - Grip indicator: laser turns white/bright when pinched - Connection line between hands when both gripping - Impact 'warm spot' at center where laser hits - SVG glow filters for visual effects 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
When both hands are gripping (pinch valid), users can: - SPREAD hands apart = zoom in (move camera closer) - PINCH hands together = zoom out - ROTATE hands around each other = orbit camera - MOVE both hands together = pan view - PULL both hands toward camera = dolly in (pull graph closer) - PUSH both hands away = dolly out Falls back to wrist-based gestures when hands are detected but not gripping. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Performance optimizations implemented: - Instanced mesh rendering for nodes (1 draw call for all nodes) - Batched LineSegments for edges (1 draw call for all edges) - Reduced sphere geometry from 32x32 to 12x12 segments - LOD labels: only show labels for nearby/selected nodes (max 10) - Performance mode toggle: disables Bloom/Vignette post-processing - Single useFrame callback instead of per-node callbacks - Reusable temp objects to avoid GC pressure - Frustum culling enabled on instanced mesh Before: ~100 draw calls, 200 useFrame callbacks, 100k vertices After: ~3 draw calls, 3 useFrame callbacks, shared geometry 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Disable auto-rotate by default (start still, less disorienting) - Add BUILD_VERSION indicator in header to verify deployments - Current version: 2024-12-11-perf-v2 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ost hand UI improvements for hand gesture controls: - Add gesture smoothing with lerp, deadzone, and max speed clamps - Implement gentle recentering when not actively manipulating - Make lasers default toward center (70%) with hand-based deviation (30%) - Replace skeleton splines with translucent ghost hand effect - Ghost hand features palm fill, finger paths, gradients, and glow effects 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Un-mirror hand X coordinates (webcam is mirrored, flip back for natural feel) - Change gesture from camera manipulation to cloud translation - Pinch + pull back = pull memory cloud closer toward you - Spread + push forward = push memory cloud away - More physically intuitive interaction model 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Gesture improvements: - Single hand: pinch + pull/push now translates cloud in Z - Two hands: compound forces create 3D rotation (X and Y axes) - Left hand X movement rotates Y, right hand X movement rotates opposite - Creates torque effect at laser intersection point - Both hands contribute to Z translation additively - Smooth recentering for both position and rotation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Visual improvements: - Inverted depth scaling: hand closer to camera = smaller (farther in 3D) - Puffy white glove appearance like Mario/cartoon hands - Semi-transparent with soft edges and volumetric feel - Rounded fingertips with highlight dots - Knuckle bumps and wrist cuff details - Layered strokes create puffy tube effect on fingers 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Reimplemented hand rendering with Smash Bros Master Hand aesthetic: - Filled volumetric finger shapes (capsule/sausage geometry) - Proper palm shape connecting finger bases - Radial gradient from white to soft lavender - Ambient occlusion shadows in finger creases - Knuckle definition shadows - Rim lighting effect on edges - Specular highlight dots on fingertips - Soft drop shadow for depth - Wrist cuff detail 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Introduces stable pointer ray with arm model and One Euro Filter for accurate hand-based node selection and manipulation. Adds ExpandedNodeView for animated node expansion, LaserPointer for 3D laser visualization, and useHandInteraction hook for gesture-driven graph interaction. Updates GraphCanvas and Hand2DOverlay to support pinch-to-select, Z manipulation, two-hand rotation/zoom, and visual hit indicators.
Introduces a new WebSocket server for iPhone hand tracking with web visualization in packages/graph-viewer/scripts. Refines gesture and pointer logic in GraphCanvas, useHandInteraction, and related components for more accurate hand-based interactions. Updates AGENTS.md with a task completion checklist. Removes unused code and types for clarity.
Introduces a new hand lock and grab state machine for intentional gesture-based cloud manipulation. Adds the useHandLockAndGrab hook, HandControlOverlay UI, and integrates these into App and GraphCanvas. Updates useHandInteraction to allow disabling pinch-to-select when using grab controls, and enhances iPhone hand tracking to compute grab strength. This enables more robust and user-friendly hand gesture interactions for zooming and rotating the graph.
Updated Vite configuration to allow dynamic API target via environment variable, improving flexibility for local and production environments. Adjusted Hand2DOverlay to include hand lock state for visual feedback during interactions. Enhanced GraphCanvas with new aiming and pinch-click features, refining user experience for gesture-based interactions. Added support for URL parameters to override server settings for local development, streamlining testing against remote backends.
Adds a dev:all launcher for starting the iPhone bridge + Vite together, and upgrades the UI/bridge status reporting (browser↔bridge, phone↔bridge, LiDAR availability, ports/IPs) for easier debugging.
Plan to improve memory display and organization while keeping hand gesture features. Key additions: - Obsidian-style settings panel (filters, display, forces, clustering) - Multi-layer relationship visualization with styled edges - Smart clustering (by type, tags, or semantic similarity) - Enhanced selection focus mode with context highlighting - Keyboard shortcuts supplementing gesture navigation Research sources included for graph visualization best practices.
Sprint 1 implementation: - Add SettingsPanel with collapsible sections (Filters, Relationships, Display, Clustering, Forces) - Add SliderControl and ToggleControl UI components - Add ForceConfig, DisplayConfig, ClusterConfig types - Wire force configuration to useForceLayout hook - Add reheat button to restart force simulation - Settings panel docked to right side with toggle button This provides real-time control over graph visualization parameters including node size, link thickness, force strengths, and filtering.
Sprint 2 implementation: - Add edge styles with distinct colors per relationship type - Causal (blue): LEADS_TO, EVOLVED_INTO, DERIVED_FROM - Temporal (orange/gray): OCCURRED_BEFORE, INVALIDATED_BY - Associative (green): EXEMPLIFIES, REINFORCES, RELATES_TO - Conflict (red): CONTRADICTS - Hierarchical (violet/slate): PREFERS_OVER, PART_OF - Filter edges by relationship type visibility - Apply link thickness and opacity from display settings - Apply node size scale from display settings - Toggle label visibility and configure fade distance Display settings now control: - Edge colors based on relationship type - Edge visibility per relationship type - Link thickness and opacity - Node size scaling - Label visibility and fade distance
- Add useClusterDetection hook for detecting clusters by type, tags, or semantic similarity - Add ClusterBoundaries component rendering dotted spheres around clusters - Integrate clustering config through GraphCanvas to Scene - Support three clustering modes: type (memory type), tags (first tag), semantic (connected components) - Boundaries gently rotate for visual interest - Fibonacci sphere point distribution for even dotted effect
- Add SelectionHighlight component with glowing ring around selected node - Add ConnectedPathsHighlight with flowing particles along edges - Pulsing animation on selection ring for visual feedback - Three-ring display (XY and XZ planes) for 3D depth perception - Particles flow from selected node to connected nodes - Track selected node layout position for accurate highlight placement
- Add useKeyboardNavigation hook for Obsidian-style keyboard shortcuts - Arrow keys navigate between nodes spatially (up/down/left/right) - Shift+Arrow up/down for Z-axis navigation (forward/backward) - Tab/Shift+Tab cycles through nodes sequentially - Escape to deselect, R to reheat, L to toggle labels - Comma to toggle settings panel - Question mark shows help in console - Ignores input when focus is in text fields
Added the 'ws' package to dependencies and updated npm scripts to use 'npx' for Vite commands in package.json for improved compatibility.
Introduces a UI toggle to switch between MediaPipe (webcam) and iPhone hand tracking sources, with initial source determined by URL parameters. Refactors tracking source state management, updates related props, and improves MediaPipe hand gesture cleanup to prevent errors during component unmount.
Revised iPhone hand landmark keys to match Vision framework abbreviations and updated all related calculations to use new keys. Added debug logging for tracking source changes, incoming iPhone messages, and landmark keys. Improved LiDAR depth normalization for MediaPipe compatibility and enhanced error handling and message logging in the WebSocket connection.
Refactors hand gesture controls to use displacement-based panning and depth for more intuitive graph manipulation, replacing velocity-based movement. Adds a 'Reset View' button to center the graph and reset rotation, with support for this callback throughout the app. Enhances hand overlay visuals with activation flash, improved opacity logic, and more accurate laser/pointing detection. Updates hand lock and grab logic for more robust pose detection and longer lock persistence.
Introduces several new interactive features to the graph viewer, including a bookmarks panel for saving and restoring camera positions, lasso selection for bulk node actions, animated edge particles, a pathfinding overlay, a timeline bar for time travel, a radial menu for node actions, and a tag cloud for filtering. Updates App.tsx and GraphCanvas.tsx to integrate these features and their state management, and adds supporting hooks and components. Also improves keyboard navigation and sound effects integration.
Introduces a new useHandCursor hook for simplified hand cursor tracking, replacing complex pointing and pinch logic in GraphCanvas. Refactors force layout logic in useForceLayout to use a module-level cache for improved stability and performance, and updates related components for more stable data references and improved event handling. Also improves lasso overlay usability and hand overlay depth feedback.
Simplifies hand gesture controls to only support fist grab for panning the graph. Removes hand cursor, laser pointer, expanded node selection, and related overlays and hooks. Updates overlays to provide only basic visual hand feedback without lasers or node selection.
Enhances hand tracking by refining hand lock acquisition, adding depth-aware pointing and pinch detection, and improving overlay visualization for both MediaPipe and iPhone LiDAR sources. The commit introduces more intentional hand lock gating, depth-based pointing heuristics, and visual feedback for hand state and depth. It also fixes candidate hand tracking, improves inertial panning, and updates the debug overlay to show world Z in meters.
Introduces hooks for recording and replaying hand gesture data, enabling automated testing and playback of hand interactions. Updates gesture state to include pinchPoint for direct pinch selection, refactors selection logic to use pinchPoint, and adds visual feedback for pinch pre-select. UI now displays recording and playback indicators, and exposes global automation APIs in test mode.
Introduces two-hand pinch gesture for pan/zoom/rotate in the graph viewer, with visual feedback and bimanual state tracking. Updates gesture handling hooks to support bimanual pinch, selection clearing via open palm, and consistent coordinate mirroring for both MediaPipe and iPhone hand tracking. Also updates UI overlays and node dimming for improved clarity. Adds ESLint and Flake8 config files for code quality.
Refines cluster detection to prioritize entity tags and group nodes by shared tags, with improved logic for tag selection. Lowers the semantic clustering edge strength threshold for tighter clusters. Updates force layout to cluster nodes by primary tag with deterministic positioning, and adjusts default force simulation parameters for denser, more cohesive clusters.
Introduces a new /graph/projected API endpoint in the backend that uses UMAP to project memory embeddings into 3D space, enabling semantic clustering of nodes. Updates the frontend to support toggling UMAP-based layouts, fetches projected positions, and bypasses force simulation when server-provided positions are available. Adds related UI controls, types, and caching logic. Also documents the implementation plan and adds umap-learn to requirements.
Adds server-side UMAP dimensionality reduction to position nodes by embedding similarity rather than just tags/edges. Semantically related memories now cluster together regardless of explicit relationships. Backend: - Add /graph/projected endpoint with UMAP 3D projection - Lazy-load umap-learn to avoid slow startup - Scale projections to 100-unit visualization range - Add umap-learn>=0.5.5 to requirements.txt Frontend: - Add UMAP Layout toggle in Clustering settings - New useProjectedGraph hook with 5-min cache - useForceLayout skips simulation when server positions provided - Show info banner when UMAP mode active 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Backend and frontend now support up to 5000 nodes. Instanced rendering handles this fine - bottleneck is UMAP computation time (~15-30s for 5k). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds WebXR/Quest VR support to the graph viewer using @react-three/xr, including a VR entry button and XR store. Refactors force layout seeding to support clustering by tags or type, and improves deterministic node placement. Updates API client to use a new default server URL and switches to API key in query string for requests. Also adds a 'Reset View' button to the UI and enhances CORS handling in the backend to support custom headers and preflight requests.
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a new Graph Viewer frontend (React + TypeScript + Three.js) with many UI components, hooks, and hand-gesture tooling; extends the Flask backend with CORS, a /viewer blueprint, and a new /graph/projected UMAP projection endpoint; adds Docker multi-stage frontend build and lint/build configs. Changes
Sequence Diagram(s)mermaid Viewer->>API: GET /graph/projected?limit=&n_neighbors=&min_dist=&spread= Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Fix all issues with AI Agents 🤖
In @.gitignore:
- Around line 27-29: The .gitignore update excludes Graph Viewer build artifacts
(packages/graph-viewer/dist/, packages/graph-viewer/node_modules/,
automem/static/viewer/) but omits Vite's cache directory; add a Vite cache
ignore pattern such as .vite/ (or the broader **/.vite/) to the .gitignore so
packages/graph-viewer/.vite/deps/ and any other Vite cache folders are not
committed.
In @automem/api/graph.py:
- Around line 322-330: The n_neighbors value passed into UMAP lacks a
lower-bound check so a user-provided 1 can slip through and cause UMAP
validation errors; locate where n_neighbors is read/processed (the variable used
when constructing reducer = umap_mod.UMAP(...) and in the min(n_neighbors,
len(embeddings) - 1) expression) and enforce a minimum of 2 when parsing the
request (e.g., replace the current parsing with one that casts to int, clamps to
50, and then applies max(2, ...)); ensure the downstream min(...,
len(embeddings) - 1) still uses this validated n_neighbors value before passing
it into UMAP.
- Around line 314-316: Remove the unused variable by deleting the assignment to
memory_ids in the embeddings extraction block: the code that creates memory_ids
= [p.id for p in filtered_points] is unused and should be removed from the
function where embeddings = np.array([p.vector for p in filtered_points],
dtype=np.float32) is computed (refer to the local variable names embeddings and
filtered_points); ensure there are no other references to memory_ids elsewhere
in the module before removing it.
In @packages/graph-viewer/.vite/deps/package.json:
- Around line 1-3: Remove the committed Vite build artifacts: delete the
auto-generated Vite pre-bundling cache directory inside the graph-viewer package
(and its package.json) from the repo, add a rule to .gitignore to ignore Vite
cache/pre-bundle outputs (so these files are not committed again), and run git
rm -r on the committed artifact directory followed by a commit to remove them
from version control.
In @packages/graph-viewer/index.html:
- Line 5: The HTML references a non-existent favicon via the link tag
(href="/vite.svg"); fix by either adding the actual favicon file to a
public/static location and updating the href to that committed path, or change
the href to point at an existing committed SVG/ICO in the repo; update the <link
rel="icon" ...> element in packages/graph-viewer/index.html accordingly so it
points to a valid asset.
In @packages/graph-viewer/package.json:
- Line 36: Move the "ws" entry out of the package.json "dependencies" section
and add it under "devDependencies" (the package name is "ws" with version
"^8.18.3"); this keeps it available for the development-only script
packages/graph-viewer/scripts/hand-tracking-server.js while preventing it from
being bundled for the frontend. After editing package.json, update the lockfile
by running your package manager install (npm install / pnpm install / yarn
install) to reflect the change. Ensure no other production code imports "ws" and
that only the hand-tracking-server.js script requires it.
In @packages/graph-viewer/scripts/hand-tracking-server.js:
- Around line 371-372: The empty catch block after attempting to send the
initial bridge status to newly connected web clients silently swallows errors;
update the catch on the try that wraps the initial status send so it logs the
caught error (e.g., console.error or the existing logger) with a clear message
like "Failed to send initial bridge status to new client" and include the error
object to preserve stack/diagnostics; keep the rest of the error handling
unchanged.
- Around line 205-207: The info text shows the wrong port (8765) while the
actual default PHONE_PORT is 8768; update the HTML string inside the info div to
show 8768 or, better, interpolate/insert the PHONE_PORT constant so the
displayed port always matches the actual PHONE_PORT value (replace the hardcoded
"8765" in the div with PHONE_PORT or its value).
In @packages/graph-viewer/src/api/client.ts:
- Around line 53-60: The addTokenToUrl function exposes the API token in the URL
(via api_key query) — stop appending the token to URLs and instead send it in an
Authorization header (e.g., Bearer <token>) when making HTTP requests; update
callers of addTokenToUrl (or replace its usage) so they obtain the token with
getToken and set the Authorization header on the request object, leaving URL
composition unchanged; keep query-token only for unavoidable cases (initial
health check or CORS preflight) and ensure getToken still throws if missing so
callers can handle that error path.
In @packages/graph-viewer/src/App.tsx:
- Around line 841-854: The error display assumes error is an Error instance;
change the rendering in the block that references error and refetch to extract
the message defensively (e.g. compute a safeMessage from typeof error ===
'string' ? error : (error && (error as Error).message) ?? String(error) or
similar) and render safeMessage instead of (error as Error).message so non-Error
values and null/undefined are handled without crashing; keep the Retry button
wired to refetch.
In @packages/graph-viewer/src/components/EdgeParticles.tsx:
- Around line 143-159: The update loop can use a different per-edge count than
was used when creating geometry; set and reuse the actualParticlesPerEdge
computed during geometry creation (store it in a ref like
actualParticlesPerEdgeRef) and replace usages of particlesPerEdge in the
EdgeParticles update logic with actualParticlesPerEdgeRef.current, and also
ensure the inner loop bounds check uses edgeDataRef.current.length to prevent
edgeIndex from ever exceeding the buffer (e.g., for loop condition: i <
actualParticlesPerEdgeRef.current && edgeIndex < edgeDataRef.current.length).
In @packages/graph-viewer/src/components/ExpandedNodeView.tsx:
- Around line 273-275: In ExpandedNodeView, avoid unconditionally slicing
connNode.content to 25 chars; instead check its length before truncating and
appending "..." so short content is not modified. Locate the JSX that renders
{connNode.content.slice(0, 25)}... (inside the Text element) and replace it with
logic that returns connNode.content as-is when connNode.content.length <= 25,
otherwise returns connNode.content.slice(0, 25) + "..." (also handle
null/undefined content defensively if needed).
- Around line 72-85: The animation start time is taken from performance.now()
but elapsed is computed from Three.js's state.clock.elapsedTime, causing drift;
stop setting animationStartRef.current in useEffect using performance.now(), and
instead initialize animationStartRef.current from state.clock.elapsedTime inside
the first call to useFrame when isExpanding becomes true (set
isAnimatingRef.current = true and setAnimationProgress(0) there), leaving the
existing elapsed / EXPAND_DURATION logic unchanged; update/remove the
performance.now() assignment in useEffect and rely on the state.clock-based
start time to keep timing consistent for groupRef, isAnimatingRef,
animationStartRef, and setAnimationProgress.
In @packages/graph-viewer/src/components/GraphCanvas.tsx:
- Around line 868-888: Inside the per-frame node loop you allocate a new
THREE.Vector3 for every node; instead preallocate two temp vectors (e.g.,
tempWorldPos and tempProjected) outside the loop (or in enclosing scope used by
useFrame), then inside the loop call tempWorldPos.set(n.x ?? 0, n.y ?? 0, n.z ??
0), run group.localToWorld(tempWorldPos), copy to tempProjected
(tempProjected.copy(tempWorldPos)) and call tempProjected.project(camera) (since
project mutates the vector), compute screenX/screenY from tempProjected, and use
those to compute dx/dy/dist and update nearestDist/nearestNode; this avoids
per-iteration allocations while preserving existing logic and symbols
(layoutNodes, group.localToWorld, project, camera, PINCH_SELECT_RADIUS,
nearestDist, nearestNode).
- Around line 293-306: The VR button onClick handler contains debug console.log
calls; remove the console.log('VR button clicked') and console.log('immersive-vr
supported:', supported') statements inside the onClick async function (leaving
the navigator.xr checks, alerts, and xrStore.enterVR() behavior intact) so the
handler no longer emits development console output in production.
- Around line 1509-1514: The code creates a new THREE.Color('#3b82f6') inside
the per-frame/per-node branch (the useFrame loop) for lasso-selected nodes which
allocates on every frame; hoist that color creation out of the loop (e.g.,
define a const blueColor = new THREE.Color('#3b82f6') at module scope or in the
GraphCanvas component body) and reuse it when calling tempColor.lerp(blueColor,
0.35) in the isLassoSelected branch so only tempColor and baseNodeColor are
mutated per-frame.
In @packages/graph-viewer/src/components/Hand2DOverlay.tsx:
- Around line 76-78: The closure reads leftSmoothed/rightSmoothed directly which
can be stale; replace the external conditional and state-read with a functional
updater that inspects the current state inside setLeftSmoothed and
setRightSmoothed (e.g., call setLeftSmoothed(prev => { if (prev &&
!prev.isGhost) return { ...prev, isGhost: true, lastSeen: now }; return prev; })
and do the analogous change for setRightSmoothed) so the ghost transition uses
the latest state atomically.
In @packages/graph-viewer/src/components/HandControlOverlay.tsx:
- Line 41: The ternary assigning m is redundant—`const m = lock.mode === 'idle'
? lock.metrics : lock.metrics` always yields lock.metrics; replace it with a
direct assignment `const m = lock.metrics` or, if the original intent was to
choose different metrics based on lock.mode, update the alternate branch to the
correct property (e.g., `lock.idleMetrics` vs `lock.activeMetrics`) so the
conditional uses the intended values; locate the assignment to m in
HandControlOverlay (the `m` variable and `lock.mode` / `lock.metrics`
references) and apply the change.
In @packages/graph-viewer/src/components/MiniMap.tsx:
- Around line 77-98: The worldToCanvas and canvasToWorld callbacks can divide by
zero when bounds.maxX==bounds.minX and bounds.maxY==bounds.minY (scale === 0);
modify both functions to guard against scale being 0 by computing scale =
Math.max(rangeX, rangeY) and if scale === 0 set a safe fallback (e.g., 1 or a
small non-zero value) or return a centered coordinate, then proceed with the
existing calculations using that non-zero scale so conversions never perform
division by zero; update references in worldToCanvas and canvasToWorld to use
this guarded scale.
In @packages/graph-viewer/src/components/SearchBar.tsx:
- Around line 14-20: The debounce in the SearchBar useEffect resets whenever the
parent passes a new onChange reference; stabilize the callback by creating a ref
(e.g., onChangeRef) and assign onChange to onChangeRef.current whenever onChange
changes, then change the debouncing effect to depend only on localValue and call
onChangeRef.current(localValue) inside the timer; this preserves debouncing even
if the parent recreates the onChange function while still reacting to the latest
onChange implementation.
In @packages/graph-viewer/src/components/settings/SettingsPanel.tsx:
- Around line 17-29: RELATIONSHIP_INFO duplicates data from EDGE_STYLES; replace
the hardcoded map by importing EDGE_STYLES from lib/edgeStyles.ts and derive
label and color directly and compute style by mapping each edgeStyle.dashPattern
(e.g., [] -> 'solid', [number,...] -> 'dashed' or 'dotted' depending on pattern
length or values) so SettingsPanel uses EDGE_STYLES as the single source of
truth; update usages to reference the transformed object (keep the
RELATIONSHIP_INFO name if convenient) and remove the duplicated literals.
In @packages/graph-viewer/src/components/settings/ToggleControl.tsx:
- Around line 8-38: The label element in ToggleControl is not programmatically
associated with the switch button, so update ToggleControl to associate them:
either wrap the clickable row (including the existing label text and the button
with role="switch") inside a single <label> element (so clicks on the text
toggle the switch and screen readers announce it), or assign the button an id
and replace the <label> with a proper <label htmlFor="{thatId}"> to link the
text to the control; ensure the button keeps role="switch" and
aria-checked={checked} so accessibility semantics remain intact.
In @packages/graph-viewer/src/components/TagCloud.tsx:
- Around line 17-32: The TagCloudProps interface declares a tags prop that is
never used by the TagCloud component; remove the unused tags property from
TagCloudProps (or, if tags was intended to be used, update the TagCloud
component to consume tags instead of filteredTags where appropriate). Locate the
interface named TagCloudProps and either delete the line "tags: TagData[]" or
modify the TagCloud component to reference the tags prop (and adjust logic in
functions like onToggleTag, filtering, and rendering) so the prop is actually
used; ensure all usages and tests are updated accordingly.
In @packages/graph-viewer/src/components/TokenPrompt.tsx:
- Around line 22-34: The code stores the serverUrl/token after only calling
checkHealth, which doesn't validate the API token; update the submit flow in
TokenPrompt so that after await checkHealth(serverUrl) you perform an
authenticated lightweight request (e.g., GET /graph/stats or another existing
auth-guard endpoint) using the provided token and only call
setServerConfig(serverUrl, token) and onSubmit(token) if that authenticated
check succeeds; on failure setError with the auth error message and avoid
storing credentials; keep the existing error/finally handling and use the same
err-to-message logic as currently used.
In @packages/graph-viewer/src/hooks/useAuth.ts:
- Around line 46-47: The current hook computes isAuthenticated once via
checkAuth() on mount so it doesn't update when token or serverUrl change; update
the hook to compute isAuthenticated reactively (e.g., replace the one-time call
with a derived value using useMemo(() => checkAuth(), [token, serverUrl]) or
maintain isAuthenticated in state and run an effect that calls checkAuth()
whenever token or serverUrl change), and ensure calls to setToken() and
clearAuth() update those dependencies so the UI reflects the new authentication
state.
In @packages/graph-viewer/src/hooks/useCameraState.ts:
- Around line 67-106: The hook useCameraNavigation starts requestAnimationFrame
loops and stores the id in animationRef but never cancels them on unmount; add a
cleanup effect that cancels any pending frame by calling
cancelAnimationFrame(animationRef.current) and setting animationRef.current =
null in a useEffect return cleanup. Locate animationRef and navigateTo inside
useCameraNavigation and implement useEffect(() => () => { if
(animationRef.current) cancelAnimationFrame(animationRef.current);
animationRef.current = null; }, []) so any running animation is cancelled when
the component unmounts.
In @packages/graph-viewer/src/hooks/useClusterDetection.ts:
- Around line 93-96: Update the mismatched comment in useClusterDetection: the
loop building the adjacency list (variable adj iterating over edges) uses a
threshold of 0.25, so change the comment that currently says "strength >= 0.3"
to "strength >= 0.25" (or alternatively change the literal to 0.3 if the
intended threshold is 0.3) so the comment and the check on edge.strength remain
consistent.
In @packages/graph-viewer/src/hooks/useForceLayout.ts:
- Around line 298-304: reheat currently calls setIsSimulating(true) but never
flips it back when the d3 simulation stops; update reheat to attach an "end"
listener to layoutCache.simulation that calls setIsSimulating(false) (e.g.,
layoutCache.simulation.on('end', () => setIsSimulating(false))) and ensure you
remove any previous listener before adding a new one to avoid leaks (or add the
listener once when the simulation is created); reference the reheat function,
layoutCache.simulation, and setIsSimulating, and if using useCallback adjust
dependencies or add a cleanup to detach the "end" handler when appropriate.
- Around line 43-46: The current createDataSignature only uses length and
first/last IDs and can miss changes to middle nodes; update createDataSignature
to produce a signature that reflects all node identities (e.g., combine
nodes.map(n => n.id) into a deterministic string or compute a compact hash of
that array) so any ID change or reorder invalidates the cache; for very large
graphs implement a sampling + hash strategy or use a fast non-cryptographic hash
(e.g., FNV or xxhash) over the joined IDs to keep the signature size small.
In @packages/graph-viewer/src/hooks/useHandGestures.ts:
- Around line 126-130: The normalizeAngle function can infinite-loop for
NaN/Infinity; first guard that case by checking Number.isFinite(angle) and
return a safe default (e.g., 0) or throw a clear error if not finite, then
replace the while-loops with a robust modulo-based normalization using 2*Math.PI
(i.e., reduce angle via remainder/modulo into the [-PI, PI] range) so very large
inputs don't loop; update the function normalizeAngle to perform the finite
check and then the modulo normalization.
- Around line 389-398: There are two useEffect cleanup blocks duplicating
teardown for cameraRef, handsRef, and videoRef; remove the redundant effect (the
one returning stop()/close()/remove() without the isCleaningUpRef guard) so only
the existing cleanup that uses isCleaningUpRef runs, or consolidate them into a
single effect that checks isCleaningUpRef before calling
cameraRef.current.stop(), handsRef.current.close(), and
videoRef.current.remove(); ensure only one teardown path references cameraRef,
handsRef, videoRef and uses isCleaningUpRef to avoid double-cleanup.
In @packages/graph-viewer/src/hooks/useHandLockAndGrab.ts:
- Line 273: In useHandLockAndGrab the const nowMs = performance.now() is
included in the useMemo dependency which forces recomputation every render;
either remove useMemo and compute the value inline (replace the useMemo(...)
that produces next with an IIFE so next is recalculated each frame), or
implement a time-throttling ref strategy: add lastComputeRef and prevResultRef,
call performance.now() locally, check if now - lastComputeRef.current <
desiredInterval (e.g., 16ms) then return prevResultRef.current, otherwise
perform the heavy computation, update lastComputeRef.current and
prevResultRef.current and return the new result; ensure the useMemo dependency
array only contains stable flags (like enabled) if you keep memoization.
In @packages/graph-viewer/src/hooks/useHandPlayback.ts:
- Around line 374-382: The useEffect that starts/cancels requestAnimationFrame
can schedule a duplicate loop because play() and tick() already manage
scheduling; remove this effect (the useEffect block that checks state.isPlaying
&& !state.isPaused and calls requestAnimationFrame(tick) /
cancelAnimationFrame(animationFrameRef.current) ) or change it to only cancel on
unmount (so it doesn't start frames), ensuring play(), tick(), and
animationFrameRef are the single source of truth for starting/stopping the
animation loop and avoiding double requests.
In @packages/graph-viewer/src/hooks/useHandRecording.ts:
- Line 92: The id generation in useHandRecording.ts uses the deprecated
String.prototype.substr; update the id line that defines const id =
`rec_${Date.now()}_${Math.random().toString(36).substr(2, 9)}` to use a
non-deprecated method—replace .substr(2, 9) with either .substring(2, 11) or
.slice(2, 11) so the resulting alphanumeric segment remains 9 characters; keep
the rest of the template literal unchanged and ensure the symbol id remains used
the same way elsewhere.
- Line 115: The avgFps calculation can divide by zero when duration is 0; update
the avgFps computation in useHandRecording (the line computing avgFps from
frames.length and duration) to guard against duration === 0 and return 0 (or a
sensible default) instead of performing the division; locate the avgFps
expression and change it to a conditional that only divides when duration > 0
(e.g., avgFps = duration > 0 ? (frames.length / duration) * 1000 : 0) so
stopRecording()/startRecording() back-to-back won’t produce Infinity.
In @packages/graph-viewer/src/hooks/useIPhoneHandTracking.ts:
- Around line 382-397: The bug is that the same OneEuroFilter instances
(filters.landmarks[idx]) are used to filter both screen and world landmark
streams in the same frame, causing state corruption; change the world-landmark
path in the mapping that produces worldLandmarks so it uses a separate filter
collection (e.g., filters.worldLandmarks[idx]) instead of
filters.landmarks[idx], and if filters.worldLandmarks does not exist ensure you
initialize a parallel array of filters (same length/structure as
filters.landmarks) when filters are created so each landmark stream has its own
stateful filters.
In @packages/graph-viewer/src/hooks/useLassoSelection.ts:
- Around line 216-224: The selection change callback is being invoked during
render; move the logic that compares prevSelectionRef.current and
state.selectedIds and calls onSelectionChange(selectedNodes) into a useEffect so
it runs after render. Specifically, create a useEffect that depends on
[state.isDrawing, state.selectedIds, onSelectionChange, selectedNodes], perform
the areSetsEqual(prevSelectionRef.current, state.selectedIds) check inside it,
update prevSelectionRef.current = new Set(state.selectedIds) when they differ,
and call onSelectionChange(selectedNodes) only from within that effect to avoid
render-side effects.
In @packages/graph-viewer/src/lib/sounds.ts:
- Around line 78-81: The getContext logic currently calls
this.audioContext.resume() without awaiting the Promise, so playback calls that
follow may run before the context is resumed; update getContext (in
packages/graph-viewer/src/lib/sounds.ts) to await this.audioContext.resume()
when state === 'suspended' and propagate the async nature (make getContext
return a Promise and update callers), or alternatively make the play methods
async and await getContext/resume before scheduling/starting sounds (adjust
methods like playSound / any play* functions to await getContext()). Ensure
resume errors are caught/logged if you choose fire-and-forget.
In @packages/graph-viewer/src/main.tsx:
- Line 16: ReactDOM.createRoot(document.getElementById('root')!) uses a non-null
assertion which will throw if the 'root' element is missing; change it to first
retrieve const rootEl = document.getElementById('root') and add a runtime check:
if rootEl is null, log a clear error (or render a fallback UI) and return,
otherwise call ReactDOM.createRoot(rootEl). Ensure you reference the existing
ReactDOM.createRoot call and the 'root' id when making this change.
In @packages/graph-viewer/src/utils/OneEuroFilter.ts:
- Around line 254-262: The filter method in HandLandmarkFilter assumes
this.filters has an entry for every landmark, causing an out-of-bounds error
when landmarks.length !== this.filters.length; update the filter(landmarks,
timestamp) implementation to guard accesses to this.filters[i] (or to compare
landmarks.length to this.filters.length) and handle missing filters by either
truncating/ignoring extra landmarks or returning the original landmark unchanged
for indices where this.filters[i] is undefined; modify the loop in filter
(and/or initialize/resize this.filters elsewhere) so references to
this.filters[i].filter(...) are only invoked when this.filters[i] exists.
♻️ Duplicate comments (1)
packages/graph-viewer/.vite/deps/_metadata.json (1)
1-8: Build artifacts should not be committed to version control.This metadata file is part of Vite's build cache (same issue as
package.jsonin the same directory). Both files should be excluded from version control.
🟡 Minor comments (23)
packages/graph-viewer/src/components/Hand2DOverlay.tsx-76-78 (1)
76-78: Potential stale closure when checking ghost transition.The condition reads
leftSmootheddirectly from the closure, but this state isn't in the dependency array. IfgestureStatechanges rapidly, the closure may capture a stale reference. Consider using the functional updater to access current state:🔎 Suggested approach using functional updater
- } else if (leftSmoothed && !leftSmoothed.isGhost) { - // Hand disappeared - start ghost mode - setLeftSmoothed(prev => prev ? { ...prev, isGhost: true, lastSeen: now } : null) - } + } else { + // Hand disappeared - start ghost mode if applicable + setLeftSmoothed(prev => { + if (prev && !prev.isGhost) { + return { ...prev, isGhost: true, lastSeen: now } + } + return prev + }) + }Apply the same pattern to
rightSmoothedon lines 100-102.packages/graph-viewer/src/lib/sounds.ts-78-81 (1)
78-81:audioContext.resume()returns a Promise that isn't awaited.When the audio context is suspended,
resume()is called but not awaited. Sounds triggered immediately after may fail to play. Consider returning a Promise fromgetContextor handling this asynchronously.🔎 Possible fix
- private getContext(): AudioContext | null { + private async ensureContext(): Promise<AudioContext | null> { if (!this.audioContext) { try { this.audioContext = new (window.AudioContext || (window as unknown as { webkitAudioContext: typeof AudioContext }).webkitAudioContext)() } catch { console.warn('Web Audio API not supported') return null } } // Resume if suspended (browser autoplay policy) if (this.audioContext.state === 'suspended') { - this.audioContext.resume() + await this.audioContext.resume() } return this.audioContext }Note: This would require making play methods async or using a fire-and-forget pattern. Given the sound feedback use case, the current approach may be acceptable as resume typically completes quickly.
Committable suggestion skipped: line range outside the PR's diff.
automem/api/graph.py-314-316 (1)
314-316: Remove unused variablememory_ids.The variable
memory_idsis assigned but never used. It appears to be left over from refactoring.🔎 Proposed fix
# Extract embeddings matrix embeddings = np.array([p.vector for p in filtered_points], dtype=np.float32) - memory_ids = [p.id for p in filtered_points]packages/graph-viewer/src/hooks/useHandRecording.ts-92-92 (1)
92-92: Replace deprecatedsubstrwithsubstringorslice.
String.prototype.substris deprecated. Usesubstringorsliceinstead.🔎 Proposed fix
- const id = `rec_${Date.now()}_${Math.random().toString(36).substr(2, 9)}` + const id = `rec_${Date.now()}_${Math.random().toString(36).slice(2, 11)}`packages/graph-viewer/src/hooks/useHandRecording.ts-115-115 (1)
115-115: Division by zero possible if recording stopped immediately.If
stopRecording()is called immediately afterstartRecording(),durationcould be 0, leading to division by zero (Infinity FPS).🔎 Proposed fix
- const avgFps = frames.length > 0 ? (frames.length / duration) * 1000 : 0 + const avgFps = frames.length > 0 && duration > 0 ? (frames.length / duration) * 1000 : 0packages/graph-viewer/src/components/ExpandedNodeView.tsx-273-275 (1)
273-275: Content truncation without length check.The content is unconditionally sliced to 25 characters and appended with "...", even if the content is shorter than 25 characters.
🔎 Proposed fix
<Text position={[0, connNode.radius + 1.5, 0]} fontSize={1.8} color="#f1f5f9" anchorX="center" anchorY="bottom" outlineWidth={0.08} outlineColor="#000000" fillOpacity={satProgress} > - {connNode.content.slice(0, 25)}... + {connNode.content.length > 25 + ? `${connNode.content.slice(0, 25)}...` + : connNode.content} </Text>packages/graph-viewer/src/components/ExpandedNodeView.tsx-72-85 (1)
72-85: Potential timing drift between performance.now() and Three.js clock.
animationStartRef.currentis set usingperformance.now() / 1000(line 74), but the elapsed time is calculated usingstate.clock.elapsedTime(line 84). These clocks may not be synchronized, especially if the Three.js clock was started at a different time, causing animation timing issues.🔎 Proposed fix - use consistent timing source
useEffect(() => { if (isExpanding) { - animationStartRef.current = performance.now() / 1000 + animationStartRef.current = -1 // Signal to capture on first frame isAnimatingRef.current = true setAnimationProgress(0) } }, [isExpanding]) useFrame((state) => { if (!isAnimatingRef.current || !groupRef.current) return - const elapsed = state.clock.elapsedTime - animationStartRef.current + if (animationStartRef.current < 0) { + animationStartRef.current = state.clock.elapsedTime + } + const elapsed = state.clock.elapsedTime - animationStartRef.currentpackages/graph-viewer/src/hooks/useCameraState.ts-67-106 (1)
67-106: Missing cleanup for animation frame on unmount.The
useCameraNavigationhook stores an animation frame ID but doesn't cancel it on unmount, which could cause state updates on an unmounted component.🔎 Proposed fix
+import { useRef, useCallback, useEffect } from 'react' + export function useCameraNavigation() { const { camera, controls } = useThree() const animationRef = useRef<number | null>(null) + useEffect(() => { + return () => { + if (animationRef.current) { + cancelAnimationFrame(animationRef.current) + } + } + }, []) + const navigateTo = useCallback((targetX: number, targetY: number, duration = 500) => { // ... existing code }, [camera, controls]) return { navigateTo } }packages/graph-viewer/scripts/hand-tracking-server.js-371-372 (1)
371-372: Empty catch block silently swallows errors.The empty
catch {}block on line 372 hides potential issues when sending the initial bridge status to newly connected web clients. At minimum, log the error.Proposed fix
try { ws.send(JSON.stringify({ type: 'bridge_status', phonePort: PHONE_PORT, webPort: WEB_PORT, phoneConnected: phoneClients.size > 0, ips: getLocalIps(), lastHandFrameAt: latestHandData?.frameTimestamp || null, })); - } catch {} + } catch (err) { + console.error('Failed to send bridge status:', err.message); + }packages/graph-viewer/src/components/MiniMap.tsx-77-98 (1)
77-98: Potential division by zero in coordinate conversion.If all nodes have the same position,
rangeXandrangeYwould both be 0, makingscale = 0. This would cause division by zero in the conversion calculations.Proposed fix
const worldToCanvas = useCallback((x: number, y: number) => { const rangeX = bounds.maxX - bounds.minX const rangeY = bounds.maxY - bounds.minY - const scale = Math.max(rangeX, rangeY) + const scale = Math.max(rangeX, rangeY) || 1 // Prevent division by zero const canvasX = ((x - bounds.minX) / scale) * size const canvasY = ((bounds.maxY - y) / scale) * size // Flip Y axis return { x: canvasX, y: canvasY } }, [bounds, size]) const canvasToWorld = useCallback((canvasX: number, canvasY: number) => { const rangeX = bounds.maxX - bounds.minX const rangeY = bounds.maxY - bounds.minY - const scale = Math.max(rangeX, rangeY) + const scale = Math.max(rangeX, rangeY) || 1 // Prevent division by zero const x = (canvasX / size) * scale + bounds.minX const y = bounds.maxY - (canvasY / size) * scale // Flip Y axis return { x, y } }, [bounds, size])packages/graph-viewer/scripts/hand-tracking-server.js-205-207 (1)
205-207: Incorrect port in HTML documentation.The HTML info section displays
8765but the actual defaultPHONE_PORTis8768. This could confuse users trying to connect their iPhone.Proposed fix
<div class="info"> - iPhone should connect to: ws://<your-mac-ip>:8765 + iPhone should connect to: ws://<your-mac-ip>:${PHONE_PORT} </div>packages/graph-viewer/src/components/HandControlOverlay.tsx-41-41 (1)
41-41: Redundant ternary expression.The expression
lock.mode === 'idle' ? lock.metrics : lock.metricsalways evaluates tolock.metrics. This appears to be incomplete or a copy-paste error.🔎 Proposed fix
- const m = lock.mode === 'idle' ? lock.metrics : lock.metrics + const m = lock.metricspackages/graph-viewer/src/components/TagCloud.tsx-17-32 (1)
17-32: Unusedtagsprop in interface.The
tagsprop is declared inTagCloudPropsbut never used in the component body. The component only usesfilteredTags. Either remove the unused prop or document its intended purpose.🔎 Proposed fix
interface TagCloudProps { - tags: TagData[] filteredTags: TagData[] selectedTags: Set<string> // ... rest of props }packages/graph-viewer/src/components/TokenPrompt.tsx-22-34 (1)
22-34: Token is not validated before being stored.The
checkHealthcall only verifies server reachability—it doesn't validate the API token. Users may successfully "connect" but then encounter auth failures on subsequent API calls. Consider adding a lightweight authenticated endpoint check (e.g.,/graph/stats) to validate the token before storing.🔎 Suggested approach
try { // Test connection to the server await checkHealth(serverUrl) + + // Store config temporarily to test authentication + setServerConfig(serverUrl, token) + + // Verify token works with an authenticated endpoint + await fetchGraphStats() - // Store config and notify parent - setServerConfig(serverUrl, token) onSubmit(token) } catch (err) { + // Clear invalid config on failure + localStorage.removeItem('automem_server') + localStorage.removeItem('automem_token') setError((err as Error).message || 'Connection failed') } finally {Committable suggestion skipped: line range outside the PR's diff.
packages/graph-viewer/src/hooks/useClusterDetection.ts-93-96 (1)
93-96: Comment-code mismatch on strength threshold.The comment on line 93 states "strength >= 0.3" but the actual threshold on line 96 is
0.25. This inconsistency could confuse future maintainers.🔎 Proposed fix
- // Build adjacency list from edges with strength >= 0.3 (lowered for tighter clusters) + // Build adjacency list from edges with strength >= 0.25 (lowered for tighter clusters) const adj = new Map<string, string[]>() for (const edge of edges) { if (edge.strength >= 0.25) {packages/graph-viewer/src/utils/OneEuroFilter.ts-254-262 (1)
254-262: Potential index out of bounds if landmarks array length !== 21.
HandLandmarkFilterassumes exactly 21 landmarks. If the input array has more than 21 elements,this.filters[i]will beundefinedfori >= 21, causing a runtime error.🔎 Add bounds check
filter( landmarks: Array<{ x: number; y: number; z: number; visibility?: number }>, timestamp?: number ): Array<{ x: number; y: number; z: number; visibility?: number }> { - return landmarks.map((lm, i) => ({ - ...this.filters[i].filter(lm, timestamp), - visibility: lm.visibility, - })) + return landmarks.slice(0, 21).map((lm, i) => ({ + ...this.filters[i].filter(lm, timestamp), + visibility: lm.visibility, + })) }packages/graph-viewer/src/hooks/useForceLayout.ts-298-304 (1)
298-304: Reheat doesn't updateisSimulatingto false when simulation stops.
reheatsetsisSimulating(true)but there's no mechanism to set it back tofalsewhen the simulation completes. The simulation runs detached from React's lifecycle after reheat.🔎 Consider tracking simulation end
const reheat = useCallback(() => { if (layoutCache.simulation) { - layoutCache.simulation.alpha(0.5).restart() + layoutCache.simulation + .alpha(0.5) + .on('end', () => setIsSimulating(false)) + .restart() setIsSimulating(true) } }, [])packages/graph-viewer/src/hooks/useForceLayout.ts-43-46 (1)
43-46: Weak data signature may miss node changes.
createDataSignatureonly considers array length and first/last node IDs. Changes to middle nodes (additions/removals that maintain length, or ID changes) won't invalidate the cache, potentially returning stale layouts.🔎 Consider a more robust signature
function createDataSignature(nodes: GraphNode[]): string { if (nodes.length === 0) return '' - return `${nodes.length}-${nodes[0]?.id}-${nodes[nodes.length - 1]?.id}` + // Include a hash of all node IDs for better change detection + const idHash = nodes.map(n => n.id).join(',') + return `${nodes.length}-${idHash}` }For very large graphs, consider a sampling approach or hash function to avoid long strings.
packages/graph-viewer/src/components/GraphCanvas.tsx-1287-1288 (1)
1287-1288: Remove debug console.log statements before production.Multiple
console.logstatements for debugging mouse interactions should be removed or gated behind a debug flag for production builds.🔎 Lines containing debug logs
- Line 1287:
console.log('🖱️ DOM PointerUp - distance:', ...)- Line 1302:
console.log('🖱️ Raycast - NDC:', ...)- Line 1306:
console.log('🖱️ Raycast hit instanceId:', ...)- Line 1309:
console.log('🖱️ Found node:', ...)- Line 1613:
console.log('🖱️ R3F onClick triggered')- Line 1618:
console.log('🖱️ R3F onClick - intersects:', ...)- Line 1622:
console.log('🖱️ R3F onClick hit instanceId:', ...)- Line 1625:
console.log('🖱️ R3F onClick found node:', ...)Also applies to: 1302-1303, 1306-1307, 1309-1310, 1613-1614, 1618-1619, 1622-1623, 1625-1626
packages/graph-viewer/src/components/GraphCanvas.tsx-293-306 (1)
293-306: Console logging in VR button click handler.Debug logging in the VR button handler should be removed for production.
🔎 Proposed fix
onClick={async () => { - console.log('VR button clicked') if (!navigator.xr) { alert('WebXR not supported. Need HTTPS?') return } const supported = await navigator.xr.isSessionSupported('immersive-vr') - console.log('immersive-vr supported:', supported) if (!supported) {packages/graph-viewer/src/hooks/useHandGestures.ts-389-398 (1)
389-398: Duplicate cleanup logic may cause issues.There are two
useEffecthooks with cleanup logic for the same resources (lines 375-386 and 389-398). The second effect (lines 389-398) runs on every render due to the empty dependency array, potentially callingstop()/close()on already-cleaned resources.The first cleanup (lines 375-386) already handles everything properly with the
isCleaningUpRefguard.🔎 Proposed fix - remove duplicate cleanup
- // Cleanup on unmount - useEffect(() => { - return () => { - cameraRef.current?.stop() - handsRef.current?.close() - if (videoRef.current) { - videoRef.current.remove() - } - } - }, []) - return {packages/graph-viewer/src/components/GraphCanvas.tsx-1509-1514 (1)
1509-1514: Creating newTHREE.Colorinside useFrame loop.A new
THREE.Color('#3b82f6')is created for each lasso-selected node on every frame. This should be hoisted outside the loop.🔎 Proposed fix
+ // Constant colors (defined outside useFrame to avoid recreation) + const LASSO_BLUE = useMemo(() => new THREE.Color('#3b82f6'), []) // Inside useFrame: } else if (isLassoSelected) { tempColor.set(baseNodeColor) - const blueColor = new THREE.Color('#3b82f6') - tempColor.lerp(blueColor, 0.35) + tempColor.lerp(LASSO_BLUE, 0.35) }Committable suggestion skipped: line range outside the PR's diff.
packages/graph-viewer/src/App.tsx-841-854 (1)
841-854: Handle error type more defensively.The error from the query hook may not always be an
Errorinstance. Consider safer error message extraction.🔎 Proposed fix
{error && ( <div className="absolute inset-0 flex items-center justify-center bg-black/50 z-10"> <div className="glass p-6 rounded-xl max-w-md text-center"> <div className="text-red-400 text-lg mb-2">Connection Error</div> - <div className="text-slate-400 text-sm mb-4">{(error as Error).message}</div> + <div className="text-slate-400 text-sm mb-4"> + {error instanceof Error ? error.message : 'An unexpected error occurred'} + </div> <button onClick={() => refetch()} className="px-4 py-2 bg-blue-600 hover:bg-blue-500 rounded-lg transition-colors" > Retry </button> </div> </div> )}
| packages/graph-viewer/dist/ | ||
| packages/graph-viewer/node_modules/ | ||
| automem/static/viewer/ |
There was a problem hiding this comment.
Add .vite/ pattern to prevent committing Vite build cache.
The three new patterns correctly exclude Graph Viewer build artifacts and dependencies. However, Vite's build cache directory (.vite/) should also be ignored, as evidenced by the build artifacts in packages/graph-viewer/.vite/deps/ that were committed in this PR.
🔎 Recommended addition
packages/graph-viewer/dist/
packages/graph-viewer/node_modules/
+packages/graph-viewer/.vite/
automem/static/viewer/Or more broadly to catch all Vite caches:
packages/graph-viewer/dist/
packages/graph-viewer/node_modules/
+**/.vite/
automem/static/viewer/📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| packages/graph-viewer/dist/ | |
| packages/graph-viewer/node_modules/ | |
| automem/static/viewer/ | |
| packages/graph-viewer/dist/ | |
| packages/graph-viewer/node_modules/ | |
| packages/graph-viewer/.vite/ | |
| automem/static/viewer/ |
🤖 Prompt for AI Agents
In @.gitignore around lines 27-29, The .gitignore update excludes Graph Viewer
build artifacts (packages/graph-viewer/dist/,
packages/graph-viewer/node_modules/, automem/static/viewer/) but omits Vite's
cache directory; add a Vite cache ignore pattern such as .vite/ (or the broader
**/.vite/) to the .gitignore so packages/graph-viewer/.vite/deps/ and any other
Vite cache folders are not committed.
| reducer = umap_mod.UMAP( | ||
| n_components=3, | ||
| n_neighbors=min(n_neighbors, len(embeddings) - 1), | ||
| min_dist=min_dist, | ||
| spread=spread, | ||
| metric="cosine", | ||
| random_state=42, # Reproducible projections | ||
| ) | ||
| projected_coords = reducer.fit_transform(embeddings) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
UMAP umap-learn minimum n_neighbors parameter constraint version 0.5.5
💡 Result:
Short answer: in umap-learn 0.5.x (including 0.5.5) n_neighbors must be >= 2 (the code raises "n_neighbors must be greater than 1"); also n_neighbors should be chosen ≤ number of samples (and precomputed_knn, if used, must contain at least n_neighbors). See the UMAP source/docs for the checks and recommendations. [1][2]
Sources:
- UMAP implementation (parameter validation: raises if n_neighbors < 2). [1]
- UMAP parameter docs (recommended range and usage notes). [2]
🏁 Script executed:
# Let's examine the actual code context in the repository
find . -name "graph.py" -path "*/automem/api/*" -type fRepository: verygoodplugins/automem
Length of output: 89
🏁 Script executed:
# Check for UMAP usage and context around lines 322-330
find . -name "graph.py" -path "*/automem/api/*" -type f -exec cat -n {} \;Repository: verygoodplugins/automem
Length of output: 32905
Add explicit validation for UMAP n_neighbors minimum constraint.
The n_neighbors parameter lacks a lower bound check. Line 249 defaults to 15 and caps at 50, but doesn't enforce the UMAP requirement of n_neighbors >= 2. If a user passes n_neighbors=1, the clamping at line 324 (min(n_neighbors, len(embeddings) - 1)) would still yield 1, causing UMAP to fail with a validation error.
Add a lower bound: n_neighbors = max(2, min(int(request.args.get("n_neighbors", 15)), 50))
🤖 Prompt for AI Agents
In @automem/api/graph.py around lines 322-330, The n_neighbors value passed into
UMAP lacks a lower-bound check so a user-provided 1 can slip through and cause
UMAP validation errors; locate where n_neighbors is read/processed (the variable
used when constructing reducer = umap_mod.UMAP(...) and in the min(n_neighbors,
len(embeddings) - 1) expression) and enforce a minimum of 2 when parsing the
request (e.g., replace the current parsing with one that casts to int, clamps to
50, and then applies max(2, ...)); ensure the downstream min(...,
len(embeddings) - 1) still uses this validated n_neighbors value before passing
it into UMAP.
| { | ||
| "type": "module" | ||
| } |
There was a problem hiding this comment.
Build artifacts should not be committed to version control.
The .vite/deps/ directory contains Vite's dependency pre-bundling cache and build metadata, which are auto-generated during development and change frequently. Committing these files creates unnecessary version control noise and potential merge conflicts.
🔎 Recommended fix
Remove this file from the repository and update .gitignore:
+# Vite build cache
+**/.vite/Then remove the committed artifacts:
git rm -r packages/graph-viewer/.vite/Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @packages/graph-viewer/.vite/deps/package.json around lines 1-3, Remove the
committed Vite build artifacts: delete the auto-generated Vite pre-bundling
cache directory inside the graph-viewer package (and its package.json) from the
repo, add a rule to .gitignore to ignore Vite cache/pre-bundle outputs (so these
files are not committed again), and run git rm -r on the committed artifact
directory followed by a commit to remove them from version control.
| <html lang="en" class="dark"> | ||
| <head> | ||
| <meta charset="UTF-8" /> | ||
| <link rel="icon" type="image/svg+xml" href="/vite.svg" /> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for vite.svg existence in graph-viewer package
fd -t f "vite.svg" packages/graph-viewer/Repository: verygoodplugins/automem
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check for vite.svg in common asset locations
find packages/graph-viewer -type f -name "vite.svg" 2>/dev/nullRepository: verygoodplugins/automem
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check the public directory structure in graph-viewer
find packages/graph-viewer -type d -name "public" 2>/dev/null
ls -la packages/graph-viewer/public/ 2>/dev/null || echo "No public directory found"Repository: verygoodplugins/automem
Length of output: 92
🏁 Script executed:
#!/bin/bash
# Check all SVG files in graph-viewer
fd -t f "\.svg$" packages/graph-viewer/Repository: verygoodplugins/automem
Length of output: 49
🏁 Script executed:
#!/bin/bash
# Check if graph-viewer has a vite config that might handle favicon
find packages/graph-viewer -type f \( -name "vite.config.*" -o -name "index.html" \) -exec cat {} \;Repository: verygoodplugins/automem
Length of output: 2169
Remove or replace the favicon reference.
The vite.svg file does not exist in the repository, and there is no public/ directory to serve it from. Either create a public/ directory with the actual favicon asset or update the href attribute to reference a committed SVG file.
🤖 Prompt for AI Agents
In @packages/graph-viewer/index.html around line 5, The HTML references a
non-existent favicon via the link tag (href="/vite.svg"); fix by either adding
the actual favicon file to a public/static location and updating the href to
that committed path, or change the href to point at an existing committed
SVG/ICO in the repo; update the <link rel="icon" ...> element in
packages/graph-viewer/index.html accordingly so it points to a valid asset.
| "react-resizable-panels": "^2.1.7", | ||
| "tailwind-merge": "^2.5.5", | ||
| "three": "^0.170.0", | ||
| "ws": "^8.18.3" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/graph-viewer/package.jsonRepository: verygoodplugins/automem
Length of output: 190
🏁 Script executed:
git ls-files packages/graph-viewer/ | head -20Repository: verygoodplugins/automem
Length of output: 995
🏁 Script executed:
cat -n packages/graph-viewer/package.jsonRepository: verygoodplugins/automem
Length of output: 2145
🏁 Script executed:
rg "import.*\bws\b" packages/graph-viewer/src/Repository: verygoodplugins/automem
Length of output: 49
🏁 Script executed:
rg "ws\b" packages/graph-viewer/src/ --type ts --type tsxRepository: verygoodplugins/automem
Length of output: 94
🏁 Script executed:
rg "ws\b" packages/graph-viewer/src/ -A 2 -B 2Repository: verygoodplugins/automem
Length of output: 11257
🏁 Script executed:
grep -n "\"ws\"" packages/graph-viewer/package-lock.json | head -5Repository: verygoodplugins/automem
Length of output: 93
🏁 Script executed:
cat packages/graph-viewer/package-lock.json | grep -A 10 '"ws":' | head -20Repository: verygoodplugins/automem
Length of output: 412
🏁 Script executed:
python3 << 'EOF'
import json
with open('packages/graph-viewer/package-lock.json', 'r') as f:
lock_data = json.load(f)
# Check if ws is in direct dependencies
if 'ws' in lock_data.get('dependencies', {}):
ws_entry = lock_data['dependencies']['ws']
print("ws is a DIRECT dependency:")
print(f" Version: {ws_entry.get('version', 'N/A')}")
print(f" Resolved: {ws_entry.get('resolved', 'N/A')}")
print()
# Check what packages depend on ws
print("Packages that depend on ws:")
for pkg_name, pkg_info in lock_data.get('dependencies', {}).items():
if 'dependencies' in pkg_info and 'ws' in pkg_info['dependencies']:
print(f" - {pkg_name}")
EOFRepository: verygoodplugins/automem
Length of output: 153
🏁 Script executed:
rg '"ws"' packages/graph-viewer/package-lock.json | head -10Repository: verygoodplugins/automem
Length of output: 90
🏁 Script executed:
rg "require.*ws|import.*from.*['\"]ws['\"]" packages/graph-viewer/ --type js --type jsonRepository: verygoodplugins/automem
Length of output: 174
🏁 Script executed:
cat -n packages/graph-viewer/scripts/hand-tracking-server.js | head -50Repository: verygoodplugins/automem
Length of output: 1983
🏁 Script executed:
cat packages/graph-viewer/package.json | grep -A 10 '"scripts"'Repository: verygoodplugins/automem
Length of output: 417
🏁 Script executed:
cat -n packages/graph-viewer/scripts/dev-all.mjsRepository: verygoodplugins/automem
Length of output: 2749
Move ws from dependencies to devDependencies.
The ws package is used exclusively in packages/graph-viewer/scripts/hand-tracking-server.js, which is a Node.js development utility for the iPhone hand-tracking bridge. The browser React app correctly uses the native WebSocket API. Since ws is only needed for development tooling and not for the frontend bundle, it should be in devDependencies instead of main dependencies.
🤖 Prompt for AI Agents
In @packages/graph-viewer/package.json around line 36, Move the "ws" entry out
of the package.json "dependencies" section and add it under "devDependencies"
(the package name is "ws" with version "^8.18.3"); this keeps it available for
the development-only script
packages/graph-viewer/scripts/hand-tracking-server.js while preventing it from
being bundled for the frontend. After editing package.json, update the lockfile
by running your package manager install (npm install / pnpm install / yarn
install) to reflect the change. Ensure no other production code imports "ws" and
that only the hand-tracking-server.js script requires it.
| // Effect to continue playback loop | ||
| useEffect(() => { | ||
| if (state.isPlaying && !state.isPaused) { | ||
| animationFrameRef.current = requestAnimationFrame(tick) | ||
| } | ||
| return () => { | ||
| cancelAnimationFrame(animationFrameRef.current) | ||
| } | ||
| }, [state.isPlaying, state.isPaused, tick]) |
There was a problem hiding this comment.
Potential double animation frame scheduling.
This effect starts a new animation frame when isPlaying becomes true, but tick() (line 241) also schedules the next frame via requestAnimationFrame(tick). When play() is called (line 267), it also schedules a frame. This can lead to multiple concurrent animation loops.
🔎 Proposed fix - remove the redundant effect
- // Effect to continue playback loop
- useEffect(() => {
- if (state.isPlaying && !state.isPaused) {
- animationFrameRef.current = requestAnimationFrame(tick)
- }
- return () => {
- cancelAnimationFrame(animationFrameRef.current)
- }
- }, [state.isPlaying, state.isPaused, tick])The play() function already handles starting the animation loop. This effect appears redundant and could cause timing issues.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Effect to continue playback loop | |
| useEffect(() => { | |
| if (state.isPlaying && !state.isPaused) { | |
| animationFrameRef.current = requestAnimationFrame(tick) | |
| } | |
| return () => { | |
| cancelAnimationFrame(animationFrameRef.current) | |
| } | |
| }, [state.isPlaying, state.isPaused, tick]) | |
| // Previous code... |
🤖 Prompt for AI Agents
In @packages/graph-viewer/src/hooks/useHandPlayback.ts around lines 374-382, The
useEffect that starts/cancels requestAnimationFrame can schedule a duplicate
loop because play() and tick() already manage scheduling; remove this effect
(the useEffect block that checks state.isPlaying && !state.isPaused and calls
requestAnimationFrame(tick) / cancelAnimationFrame(animationFrameRef.current) )
or change it to only cancel on unmount (so it doesn't start frames), ensuring
play(), tick(), and animationFrameRef are the single source of truth for
starting/stopping the animation loop and avoiding double requests.
| const landmarks: NormalizedLandmarkList = rawLandmarks.map((lm, idx) => { | ||
| const filtered = filters.landmarks[idx].filter( | ||
| { x: lm.x, y: lm.y, z: lm.z ?? 0 }, | ||
| now | ||
| ) | ||
| return { ...filtered, visibility: lm.visibility } | ||
| }) | ||
|
|
||
| const worldLandmarks: NormalizedLandmarkList = rawWorldLandmarks.map((lm, idx) => { | ||
| // World landmarks use same filter as screen landmarks for consistency | ||
| const filtered = filters.landmarks[idx].filter( | ||
| { x: lm.x, y: lm.y, z: lm.z ?? 0 }, | ||
| now | ||
| ) | ||
| return { ...filtered, visibility: lm.visibility } | ||
| }) |
There was a problem hiding this comment.
Critical: Same filter instances used twice per frame corrupts filter state.
rawLandmarks and rawWorldLandmarks are both filtered through filters.landmarks[idx] in the same frame. Since OneEuroFilter is stateful (each filter() call updates internal state based on the previous value), the second call on line 392-393 overwrites the state from the first call on line 383-384.
This causes worldLandmarks to receive incorrect smoothing because the filter sees the already-filtered screen coordinates before processing world coordinates.
🔎 Suggested fix: Use separate filter sets for world landmarks
interface HandFilters {
landmarks: OneEuroFilter3D[] // 21 landmarks
+ worldLandmarks: OneEuroFilter3D[] // 21 world landmarks (separate set)
pinch: OneEuroFilter
grab: OneEuroFilter
pointerDirection: OneEuroFilter3D
}
function createHandFilters(): HandFilters {
return {
landmarks: Array.from({ length: 21 }, () => new OneEuroFilter3D(ONE_EURO_PRESETS.landmark)),
+ worldLandmarks: Array.from({ length: 21 }, () => new OneEuroFilter3D(ONE_EURO_PRESETS.landmark)),
pinch: new OneEuroFilter(ONE_EURO_PRESETS.gesture),
grab: new OneEuroFilter(ONE_EURO_PRESETS.gesture),
pointerDirection: new OneEuroFilter3D(ONE_EURO_PRESETS.pointer),
}
}Then use filters.worldLandmarks[idx] for filtering rawWorldLandmarks.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @packages/graph-viewer/src/hooks/useIPhoneHandTracking.ts around lines
382-397, The bug is that the same OneEuroFilter instances
(filters.landmarks[idx]) are used to filter both screen and world landmark
streams in the same frame, causing state corruption; change the world-landmark
path in the mapping that produces worldLandmarks so it uses a separate filter
collection (e.g., filters.worldLandmarks[idx]) instead of
filters.landmarks[idx], and if filters.worldLandmarks does not exist ensure you
initialize a parallel array of filters (same length/structure as
filters.landmarks) when filters are created so each landmark stream has its own
stateful filters.
| const prevSelectionRef = useRef<Set<string>>(new Set()) | ||
| if ( | ||
| onSelectionChange && | ||
| !state.isDrawing && | ||
| !areSetsEqual(prevSelectionRef.current, state.selectedIds) | ||
| ) { | ||
| prevSelectionRef.current = new Set(state.selectedIds) | ||
| onSelectionChange(selectedNodes) | ||
| } |
There was a problem hiding this comment.
Side effect during render - move to useEffect.
The selection change callback is executed during render, which can cause issues with React's concurrent mode and may lead to unexpected behavior. This should be moved to a useEffect.
🔎 Proposed fix using useEffect
- // Notify parent when selection changes
- const prevSelectionRef = useRef<Set<string>>(new Set())
- if (
- onSelectionChange &&
- !state.isDrawing &&
- !areSetsEqual(prevSelectionRef.current, state.selectedIds)
- ) {
- prevSelectionRef.current = new Set(state.selectedIds)
- onSelectionChange(selectedNodes)
- }
+ // Notify parent when selection changes
+ const prevSelectionRef = useRef<Set<string>>(new Set())
+ useEffect(() => {
+ if (
+ onSelectionChange &&
+ !state.isDrawing &&
+ !areSetsEqual(prevSelectionRef.current, state.selectedIds)
+ ) {
+ prevSelectionRef.current = new Set(state.selectedIds)
+ onSelectionChange(selectedNodes)
+ }
+ }, [state.isDrawing, state.selectedIds, selectedNodes, onSelectionChange])Also update the import:
-import { useState, useCallback, useRef, useMemo } from 'react'
+import { useState, useCallback, useRef, useMemo, useEffect } from 'react'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const prevSelectionRef = useRef<Set<string>>(new Set()) | |
| if ( | |
| onSelectionChange && | |
| !state.isDrawing && | |
| !areSetsEqual(prevSelectionRef.current, state.selectedIds) | |
| ) { | |
| prevSelectionRef.current = new Set(state.selectedIds) | |
| onSelectionChange(selectedNodes) | |
| } | |
| const prevSelectionRef = useRef<Set<string>>(new Set()) | |
| useEffect(() => { | |
| if ( | |
| onSelectionChange && | |
| !state.isDrawing && | |
| !areSetsEqual(prevSelectionRef.current, state.selectedIds) | |
| ) { | |
| prevSelectionRef.current = new Set(state.selectedIds) | |
| onSelectionChange(selectedNodes) | |
| } | |
| }, [state.isDrawing, state.selectedIds, selectedNodes, onSelectionChange]) |
| const prevSelectionRef = useRef<Set<string>>(new Set()) | |
| if ( | |
| onSelectionChange && | |
| !state.isDrawing && | |
| !areSetsEqual(prevSelectionRef.current, state.selectedIds) | |
| ) { | |
| prevSelectionRef.current = new Set(state.selectedIds) | |
| onSelectionChange(selectedNodes) | |
| } | |
| import { useState, useCallback, useRef, useMemo, useEffect } from 'react' |
🤖 Prompt for AI Agents
In @packages/graph-viewer/src/hooks/useLassoSelection.ts around lines 216-224,
The selection change callback is being invoked during render; move the logic
that compares prevSelectionRef.current and state.selectedIds and calls
onSelectionChange(selectedNodes) into a useEffect so it runs after render.
Specifically, create a useEffect that depends on [state.isDrawing,
state.selectedIds, onSelectionChange, selectedNodes], perform the
areSetsEqual(prevSelectionRef.current, state.selectedIds) check inside it,
update prevSelectionRef.current = new Set(state.selectedIds) when they differ,
and call onSelectionChange(selectedNodes) only from within that effect to avoid
render-side effects.
| }, | ||
| }) | ||
|
|
||
| ReactDOM.createRoot(document.getElementById('root')!).render( |
There was a problem hiding this comment.
Add runtime check for root element.
The non-null assertion operator (!) assumes the 'root' element exists in the DOM. If the element is missing (e.g., due to malformed HTML or build issues), this will throw an uncaught error at runtime rather than providing a graceful fallback.
🔎 Proposed fix with runtime check
-ReactDOM.createRoot(document.getElementById('root')!).render(
+const rootElement = document.getElementById('root')
+if (!rootElement) {
+ throw new Error('Failed to find the root element')
+}
+ReactDOM.createRoot(rootElement).render(
<React.StrictMode>
<QueryClientProvider client={queryClient}>
<App />
</QueryClientProvider>
</React.StrictMode>,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ReactDOM.createRoot(document.getElementById('root')!).render( | |
| const rootElement = document.getElementById('root') | |
| if (!rootElement) { | |
| throw new Error('Failed to find the root element') | |
| } | |
| ReactDOM.createRoot(rootElement).render( | |
| <React.StrictMode> | |
| <QueryClientProvider client={queryClient}> | |
| <App /> | |
| </QueryClientProvider> | |
| </React.StrictMode>, | |
| ) |
🤖 Prompt for AI Agents
In @packages/graph-viewer/src/main.tsx around line 16,
ReactDOM.createRoot(document.getElementById('root')!) uses a non-null assertion
which will throw if the 'root' element is missing; change it to first retrieve
const rootEl = document.getElementById('root') and add a runtime check: if
rootEl is null, log a clear error (or render a fallback UI) and return,
otherwise call ReactDOM.createRoot(rootEl). Ensure you reference the existing
ReactDOM.createRoot call and the 'root' id when making this change.
Introduces support for the VITE_API_TARGET environment variable to configure the API base URL in both the API client and TokenPrompt component. This allows deployments to specify the API endpoint via environment configuration, improving flexibility.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @packages/graph-viewer/src/components/TokenPrompt.tsx:
- Line 11: VITE_API_TARGET is a Vite build-time env var (not a runtime deploy
var); update the frontend development/build docs to state that VITE_API_TARGET
must be set before running dev/build, reference the symbol VITE_API_TARGET and
its use in vite.config.ts and TokenPrompt.tsx, show the default fallback URL
(https://automem.up.railway.app), document how to set it in local .env,
.env.local or CI pipeline env settings, and note that changing it requires
re-running the Vite dev server or rebuilding the app so the new value is baked
into the bundle.
🧹 Nitpick comments (4)
packages/graph-viewer/src/api/client.ts (2)
55-58: Token priority favors less secure query parameter.The priority order places URL query parameter first, which is the least secure method (sent to server, logged). Hash-based tokens are more secure (client-side only). Consider reordering to: hash > query > localStorage, or document why query param must have highest priority.
🔎 Proposed reordering for better security
function getToken(): string | null { - // Priority: URL query param > URL hash > localStorage - return getTokenFromQuery() || getTokenFromHash() || localStorage.getItem('automem_token') + // Priority: URL hash > URL query param > localStorage + // Hash is more secure (client-side only), query param kept for backward compatibility + return getTokenFromHash() || getTokenFromQuery() || localStorage.getItem('automem_token') }
47-47: Document the hardcoded fallback URL.Line 47 falls back to a hardcoded Railway deployment URL. Consider documenting this choice or making it configurable via environment variable to avoid surprises in different deployment contexts.
packages/graph-viewer/src/components/TokenPrompt.tsx (2)
10-12: Code duplication withgetEnvApiTarget().The environment variable trimming logic here duplicates the
getEnvApiTarget()function inclient.ts(lines 11-17). Consider reusing that utility to reduce duplication.🔎 Proposed refactor to reuse existing utility
+import { checkHealth, setServerConfig, getEnvApiTarget } from '../api/client' export function TokenPrompt({ onSubmit }: TokenPromptProps) { - const defaultServerUrl = - (import.meta.env.VITE_API_TARGET && import.meta.env.VITE_API_TARGET.trim()) || - 'https://automem.up.railway.app' + const defaultServerUrl = getEnvApiTarget() || 'https://automem.up.railway.app' const [serverUrl, setServerUrl] = useState(defaultServerUrl)Note: This requires exporting
getEnvApiTargetfromclient.ts.
66-73: Consider movingautoFocusto the first input.Line 94 sets
autoFocuson the API Token field, but users might need to modify the Server URL first. Consider placingautoFocuson the Server URL input (line 66) for better UX flow, or omit it entirely since both fields are visible.Also applies to: 88-96
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/graph-viewer/src/api/client.tspackages/graph-viewer/src/components/TokenPrompt.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
packages/graph-viewer/src/components/TokenPrompt.tsx (1)
packages/graph-viewer/src/api/client.ts (2)
checkHealth(228-232)setServerConfig(75-78)
packages/graph-viewer/src/api/client.ts (1)
packages/graph-viewer/src/lib/types.ts (3)
GraphSnapshot(57-73)GraphNeighbors(75-84)GraphStats(86-107)
🔇 Additional comments (5)
packages/graph-viewer/src/api/client.ts (3)
122-136: Well-structured parameter interfaces.The parameter interfaces (
SnapshotParams,ProjectedParams,NeighborsParams) are clearly defined with optional fields and appropriate types. The camelCase-to-snake_case parameter mapping in the API functions is correct.Also applies to: 177-181
228-232: Health check correctly unauthenticated.The
checkHealthfunction appropriately doesn't require authentication, making it suitable for connection validation before credentials are configured.
66-73: Token still exposed in URL query parameters (unresolved from previous review).The
addTokenToUrlfunction continues to append the API token as a query parameter (api_key=...) across all API calls. Query parameters appear in browser history, server logs, and proxy logs, creating a security risk.As noted in the previous review, switching to
Authorization: Bearer <token>headers (now thatgetAuthHeadersexists) would be more secure. This requires confirming the backend accepts Bearer token authentication.Verify backend support for Bearer token authentication:
#!/bin/bash # Check if the backend API endpoints support Authorization header with Bearer tokens rg -n -C3 "Authorization|Bearer|api_key" automem/api/Also applies to: 146-146, 172-172, 195-195, 201-201, 210-210, 220-220
⛔ Skipped due to learnings
Learnt from: CR Repo: verygoodplugins/automem PR: 0 File: CLAUDE.md:0-0 Timestamp: 2025-12-09T02:15:22.291Z Learning: Applies to automem/app.py : Support authentication via Bearer token, X-API-Key header, or query parameter for API endpointspackages/graph-viewer/src/components/TokenPrompt.tsx (2)
18-37: Form submission logic is well-structured.The handler properly validates inputs, tests connectivity via
checkHealth()before storing credentials, and provides clear error feedback. The flow is secure and user-friendly.
39-144: UI implementation is polished and accessible.The form provides clear labels, visual feedback (loading spinner, error messages), proper input types (
passwordfor token,urlfor server), and helpful privacy/documentation notes. The gradient styling and responsive layout are well-executed.
|
|
||
| export function TokenPrompt({ onSubmit }: TokenPromptProps) { | ||
| const defaultServerUrl = | ||
| (import.meta.env.VITE_API_TARGET && import.meta.env.VITE_API_TARGET.trim()) || |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Comprehensive search for VITE_API_TARGET documentation and configuration
echo "=== Searching for VITE_API_TARGET references ==="
rg "VITE_API_TARGET" -t js -t ts -t json -t md --no-heading -i
echo ""
echo "=== Looking for .env files and templates ==="
fd -t f "\.(env|example|template)"
echo ""
echo "=== Searching for Docker configuration ==="
fd -t f "Dockerfile|docker-compose"
echo ""
echo "=== Looking for build/deployment documentation ==="
fd -t f "README|DEPLOY|BUILD|CONTRIBUTING" -iRepository: verygoodplugins/automem
Length of output: 1216
🏁 Script executed:
#!/bin/bash
# Check documentation and configuration files for VITE_API_TARGET
echo "=== Contents of vite.config.ts (VITE_API_TARGET sections) ==="
rg -A 2 -B 2 "VITE_API_TARGET" packages/graph-viewer/vite.config.ts
echo ""
echo "=== Check Dockerfile for environment variable setup ==="
cat Dockerfile
echo ""
echo "=== Check docker-compose.yml ==="
cat docker-compose.yml
echo ""
echo "=== Check RAILWAY_DEPLOYMENT.md ==="
cat docs/RAILWAY_DEPLOYMENT.md
echo ""
echo "=== Check main README.md for build/deployment sections ==="
head -100 README.mdRepository: verygoodplugins/automem
Length of output: 22232
🏁 Script executed:
#!/bin/bash
# Search more specifically for VITE_API_TARGET in deployment docs and env templates
echo "=== Check if VITE_API_TARGET mentioned in RAILWAY_DEPLOYMENT.md ==="
rg "VITE_API_TARGET" docs/RAILWAY_DEPLOYMENT.md
echo ""
echo "=== Search for all .env* files anywhere ==="
fd -t f "\.env"
echo ""
echo "=== Check if there's frontend build docs ==="
fd -t f "README" packages/graph-viewer/
echo ""
echo "=== Full vite.config.ts to see complete documentation ==="
head -50 packages/graph-viewer/vite.config.ts
echo ""
echo "=== Check api/client.ts usage context ==="
rg -B 3 -A 3 "import.meta.env.VITE_API_TARGET" packages/graph-viewer/src/api/client.tsRepository: verygoodplugins/automem
Length of output: 2026
🏁 Script executed:
#!/bin/bash
# Final check: verify the complete picture of VITE_API_TARGET documentation
echo "=== Check all files mentioning VITE_API_TARGET ==="
rg "VITE_API_TARGET" -l
echo ""
echo "=== Check Dockerfile for any env var documentation ==="
grep -i "ENV\|ARG" Dockerfile | head -20
echo ""
echo "=== Check if docker-compose mentions VITE_API_TARGET ==="
rg "VITE_API_TARGET" docker-compose.yml
echo ""
echo "=== Check frontend package.json for build script ==="
cat packages/graph-viewer/package.json | grep -A 5 '"scripts"'Repository: verygoodplugins/automem
Length of output: 629
Clarify VITE_API_TARGET is a build-time variable and document it in development guides.
VITE_API_TARGET at line 11 is a build-time environment variable (used by Vite during development and build), not a runtime deployment variable. It currently has only an inline comment in vite.config.ts. Document it in frontend development/build documentation so developers know to set it when building locally or in CI/CD pipelines to override the default API endpoint (https://automem.up.railway.app).
🤖 Prompt for AI Agents
In @packages/graph-viewer/src/components/TokenPrompt.tsx at line 11,
VITE_API_TARGET is a Vite build-time env var (not a runtime deploy var); update
the frontend development/build docs to state that VITE_API_TARGET must be set
before running dev/build, reference the symbol VITE_API_TARGET and its use in
vite.config.ts and TokenPrompt.tsx, show the default fallback URL
(https://automem.up.railway.app), document how to set it in local .env,
.env.local or CI pipeline env settings, and note that changing it requires
re-running the Vite dev server or rebuilding the app so the new value is baked
into the bundle.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app.py (1)
161-164: Refine path matching to prevent unintended bypasses.The current implementation uses
request.path.startswith("/viewer")which could match unintended paths like/viewer-admin,/viewerdata, or/viewer2. This could inadvertently bypass authentication for routes that weren't intended to be public.🔧 Recommended fix: Use precise path matching
- # Allow unauthenticated access to the graph viewer (static files) - # Token is passed via URL hash fragment (client-side only) - if request.path.startswith("/viewer"): - return + # Allow unauthenticated access to the graph viewer (static files) + # Token is passed via URL hash fragment (client-side only) + if request.path == "/viewer" or request.path.startswith("/viewer/"): + returnThis ensures only
/viewerand/viewer/*paths bypass authentication, preventing false matches.
🤖 Fix all issues with AI agents
In @app.py:
- Around line 115-120: The CORS setup currently uses a wildcard origin ("*")
which is unsafe for production; update the CORS initialization (where CORS(app,
resources=... ) is called) to read allowed origins from an environment variable
(e.g., CORS_ORIGINS) and pass those specific domains instead of "*", parsing a
comma-separated list into the origins value and falling back to a safe default
(like localhost-only) for development; ensure the updated logic only enables "*"
in non-production environments and uses explicit origins in production.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignoreAGENTS.mdapp.py
🚧 Files skipped from review as they are similar to previous changes (1)
- AGENTS.md
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.py: Run 'black .' to format Python code before committing
Run 'flake8' to lint Python code for quality issues
**/*.py: Python files must use type hints
Indent Python code with 4 spaces; maintain line length of 100 characters (enforced by Black)
Use snake_case for module and function names
Use PascalCase for class names
Use UPPER_SNAKE_CASE for constants
Use Black for code formatting
Use Isort with profile=black for import sorting
Use Flake8 for linting Python code
Files:
app.py
🧠 Learnings (1)
📚 Learning: 2025-12-09T02:15:22.291Z
Learnt from: CR
Repo: verygoodplugins/automem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-09T02:15:22.291Z
Learning: Applies to automem/app.py : Support authentication via Bearer token, X-API-Key header, or query parameter for API endpoints
Applied to files:
app.py
🧬 Code graph analysis (1)
app.py (1)
automem/api/viewer.py (2)
create_viewer_blueprint(21-65)is_viewer_enabled(68-70)
🔇 Additional comments (6)
.gitignore (1)
30-32: LGTM!The new entries correctly ignore the Graph Viewer frontend build artifacts (
dist/,node_modules/) and the served static assets directory (automem/static/viewer/), aligning with the Dockerfile's multi-stage build that outputs frontend assets toautomem/static/viewer/. While the specific graph-viewer patterns are technically covered by existing global patterns (lines 23 and 27), being explicit improves clarity and makes the .gitignore self-documenting for the new frontend subsystem.app.py (5)
31-31: LGTM: CORS import added.The import is correctly placed with other Flask imports and necessary for the CORS configuration below.
152-154: LGTM: CORS preflight bypass implemented correctly.Allowing OPTIONS requests to bypass authentication is required by the CORS specification for preflight requests. This is standard practice and necessary for cross-origin requests to work properly.
3633-3633: LGTM: Viewer blueprint imports added correctly.The imports for
create_viewer_blueprintandis_viewer_enabledfromautomem.api.viewerare appropriately placed with other blueprint imports.
3746-3750: LGTM: Viewer blueprint registration implemented correctly.The conditional registration of the viewer blueprint follows the established pattern and includes helpful logging. The feature flag approach allows the Graph Viewer to be optionally enabled via the
ENABLE_GRAPH_VIEWERenvironment variable.
3147-3148: Add return type hint for coding guidelines compliance.The
require_api_tokenfunction is missing a return type hint. According to the coding guidelines, Python files must use type hints.📝 Proposed fix
@app.before_request -def require_api_token() -> None: +def require_api_token() -> None: if not API_TOKEN: returnWait, checking the current code... Line 3148 shows:
def require_api_token() -> None:Actually, the return type hint is already present in the provided code. Let me re-check...
Looking at line 3148 in the annotated code:
3147 @app.before_request 3148 def require_api_token() -> None:The type hint
-> Noneis already there. This is correct.Actually, upon closer inspection, the return type hint is already present at line 3148. No changes needed.
| CORS( | ||
| app, | ||
| resources={ | ||
| r"/*": {"origins": "*", "allow_headers": ["Content-Type", "X-API-Key", "Authorization"]} | ||
| }, | ||
| ) # Enable CORS for all routes |
There was a problem hiding this comment.
Restrict CORS origins to known domains in production.
The wildcard origin "*" allows any website to make cross-origin requests to this API, potentially exposing sensitive memory data to malicious sites. While convenient for development, this poses a significant security risk in production.
🔒 Recommended fix: Use environment-based origin configuration
+# Parse allowed origins from environment or use secure defaults
+CORS_ORIGINS = os.getenv("CORS_ORIGINS", "http://localhost:3000,http://localhost:5173").split(",")
+
CORS(
app,
resources={
- r"/*": {"origins": "*", "allow_headers": ["Content-Type", "X-API-Key", "Authorization"]}
+ r"/*": {
+ "origins": CORS_ORIGINS,
+ "allow_headers": ["Content-Type", "X-API-Key", "Authorization"],
+ }
},
-) # Enable CORS for all routes
+)For production deployments, set CORS_ORIGINS to your frontend domain(s):
CORS_ORIGINS="https://yourdomain.com,https://app.yourdomain.com"🤖 Prompt for AI Agents
In @app.py around lines 115 - 120, The CORS setup currently uses a wildcard
origin ("*") which is unsafe for production; update the CORS initialization
(where CORS(app, resources=... ) is called) to read allowed origins from an
environment variable (e.g., CORS_ORIGINS) and pass those specific domains
instead of "*", parsing a comma-separated list into the origins value and
falling back to a safe default (like localhost-only) for development; ensure the
updated logic only enables "*" in non-production environments and uses explicit
origins in production.
Disabled the focus/spotlight mode UI and related logic throughout the Graph Viewer app, including keyboard shortcuts and radial menu options. The 'Enter XR' button now only appears when WebXR is supported in a secure context. Improved bimanual pinch activation and smoothing for pan/zoom/rotate gestures, and updated hand overlays and control badges for clearer feedback. Also fixed the reset view callback wiring so Reset View works again.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/graph-viewer/src/components/HandControlOverlay.tsx`:
- Line 41: In HandControlOverlay replace the no-op ternary that sets const m =
lock.mode === 'idle' ? lock.metrics : lock.metrics with a direct assignment to
const m = lock.metrics; locate the initialization of m in that component (the
variable/constant named m and the lock object) and simplify it to the single
direct assignment, or if different behavior was intended for 'idle' vs other
modes, implement the correct branch logic instead of the duplicate ternary.
In `@packages/graph-viewer/src/components/RadialMenu.tsx`:
- Around line 89-94: The handleCopyId callback currently calls
navigator.clipboard.writeText(node.id) without handling rejections; update
handleCopyId (the useCallback function) to handle the returned Promise by making
it async or chaining .then/.catch so clipboard failures are caught and do not
cause unhandled rejections. On success, continue to call onCopyId?.(node.id) and
onClose(); on failure, suppress or log the error (e.g., call process/log
function or noop) and still call onClose() if appropriate so the UI remains
responsive. Ensure references to node.id, onCopyId, onClose, and
navigator.clipboard.writeText are used to locate and modify the code.
In `@packages/graph-viewer/src/hooks/useKeyboardNavigation.ts`:
- Around line 169-213: The arrow-key handlers in the shortcuts map (ArrowUp,
ArrowDown, ArrowLeft, ArrowRight inside useKeyboardNavigation.ts) should call
event.preventDefault() at the start of each action to stop the browser from
scrolling while navigating nodes; update the action functions for ArrowUp,
ArrowDown, ArrowLeft and ArrowRight to call event.preventDefault() before
computing findNodeInDirection or calling onNodeSelect so navigation keys only
affect node selection.
♻️ Duplicate comments (1)
packages/graph-viewer/src/components/GraphCanvas.tsx (1)
863-870: Avoid per-frameVector3allocations in the pinch-selection loop.
This is the same per-node allocation hot-spot as before and can add heavy GC pressure at 5k nodes.🔧 Suggested reuse pattern
+ // Reusable vectors for pinch selection (avoid per-node allocations) + const tempWorldPos = useMemo(() => new THREE.Vector3(), []) + const tempProjected = useMemo(() => new THREE.Vector3(), []) ... for (const n of layoutNodes) { - const worldPos = new THREE.Vector3(n.x ?? 0, n.y ?? 0, n.z ?? 0) - group.localToWorld(worldPos) - const projected = worldPos.project(camera) + tempWorldPos.set(n.x ?? 0, n.y ?? 0, n.z ?? 0) + group.localToWorld(tempWorldPos) + tempProjected.copy(tempWorldPos).project(camera) - const screenX = ((projected.x + 1) / 2) * rect.width - const screenY = ((-projected.y + 1) / 2) * rect.height + const screenX = ((tempProjected.x + 1) / 2) * rect.width + const screenY = ((-tempProjected.y + 1) / 2) * rect.height
🧹 Nitpick comments (2)
packages/graph-viewer/src/components/RadialMenu.tsx (1)
197-245: Add aria labels for icon-only buttons.
The close button and radial items are icon-only; screen readers will miss intent. Consideraria-label(andaria-disabled) on buttons.packages/graph-viewer/src/components/GraphCanvas.tsx (1)
1282-1305: Consider gating/removing debug console logs.
The pointer handlers log on every click path, which can flood production consoles. A debug flag or dev-only guard would keep signal without noise.Also applies to: 1608-1621
| ? { text: `ACQUIRING (${lock.frames})`, color: 'bg-yellow-500/20 text-yellow-200 border-yellow-400/30' } | ||
| : { text: 'IDLE', color: 'bg-slate-500/20 text-slate-200 border-slate-400/30' } | ||
|
|
||
| const m = lock.mode === 'idle' ? lock.metrics : lock.metrics |
There was a problem hiding this comment.
No-op ternary expression — both branches return the same value.
This conditional serves no purpose as lock.metrics is returned regardless of the mode. Simplify to a direct assignment:
Suggested fix
- const m = lock.mode === 'idle' ? lock.metrics : lock.metrics
+ const m = lock.metrics📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const m = lock.mode === 'idle' ? lock.metrics : lock.metrics | |
| const m = lock.metrics |
🤖 Prompt for AI Agents
In `@packages/graph-viewer/src/components/HandControlOverlay.tsx` at line 41, In
HandControlOverlay replace the no-op ternary that sets const m = lock.mode ===
'idle' ? lock.metrics : lock.metrics with a direct assignment to const m =
lock.metrics; locate the initialization of m in that component (the
variable/constant named m and the lock object) and simplify it to the single
direct assignment, or if different behavior was intended for 'idle' vs other
modes, implement the correct branch logic instead of the duplicate ternary.
| // Copy ID to clipboard | ||
| const handleCopyId = useCallback(() => { | ||
| navigator.clipboard.writeText(node.id) | ||
| onCopyId?.(node.id) | ||
| onClose() | ||
| }, [node.id, onCopyId, onClose]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/graph-viewer && find . -name "RadialMenu.tsx" -type fRepository: verygoodplugins/automem
Length of output: 98
🏁 Script executed:
cd packages/graph-viewer && head -150 src/components/RadialMenu.tsx | tail -100Repository: verygoodplugins/automem
Length of output: 2521
🏁 Script executed:
cd packages/graph-viewer && cat -n src/components/RadialMenu.tsx | sed -n '80,110p'Repository: verygoodplugins/automem
Length of output: 1051
🏁 Script executed:
cd packages/graph-viewer && rg "clipboard" --type ts --type tsx -i -B 2 -A 2Repository: verygoodplugins/automem
Length of output: 94
🏁 Script executed:
cd packages/graph-viewer && cat -n src/components/RadialMenu.tsx | sed -n '1,50p'Repository: verygoodplugins/automem
Length of output: 1495
🏁 Script executed:
cd packages/graph-viewer && grep -n "aria-label\|title=" src/components/RadialMenu.tsx | head -20Repository: verygoodplugins/automem
Length of output: 103
🏁 Script executed:
cd packages/graph-viewer && cat -n src/components/RadialMenu.tsx | sed -n '200,280p'Repository: verygoodplugins/automem
Length of output: 3213
🏁 Script executed:
cd packages/graph-viewer && wc -l src/components/RadialMenu.tsxRepository: verygoodplugins/automem
Length of output: 100
🏁 Script executed:
cd packages/graph-viewer && cat -n src/components/RadialMenu.tsx | sed -n '230,260p'Repository: verygoodplugins/automem
Length of output: 1409
🏁 Script executed:
cd packages/graph-viewer && rg "clipboard" -t ts -B 2 -A 2Repository: verygoodplugins/automem
Length of output: 431
🏁 Script executed:
cd packages/graph-viewer && cat -n src/components/RadialMenu.tsx | sed -n '195,250p'Repository: verygoodplugins/automem
Length of output: 2569
🏁 Script executed:
find . -name "package.json" -type f | head -5 | xargs grep -l "automem\|graph-viewer"Repository: verygoodplugins/automem
Length of output: 133
🏁 Script executed:
cd packages/graph-viewer && cat -n src/components/RadialMenu.tsx | sed -n '89,95p'Repository: verygoodplugins/automem
Length of output: 301
🏁 Script executed:
cd packages && grep -r "catch\|\.then" --include="*.tsx" --include="*.ts" | grep -i "clipboard\|writeText" | head -5Repository: verygoodplugins/automem
Length of output: 49
🏁 Script executed:
cd packages/graph-viewer && cat -n src/components/RadialMenu.tsx | sed -n '195,212p'Repository: verygoodplugins/automem
Length of output: 813
Handle clipboard API rejections to prevent unhandled promise rejections.
navigator.clipboard.writeText() returns a Promise that can reject due to permissions, insecure contexts, or unavailable clipboard. The current code doesn't handle these failures, risking unhandled promise rejections. Add error handling to catch and suppress or gracefully handle failures.
🤖 Prompt for AI Agents
In `@packages/graph-viewer/src/components/RadialMenu.tsx` around lines 89 - 94,
The handleCopyId callback currently calls navigator.clipboard.writeText(node.id)
without handling rejections; update handleCopyId (the useCallback function) to
handle the returned Promise by making it async or chaining .then/.catch so
clipboard failures are caught and do not cause unhandled rejections. On success,
continue to call onCopyId?.(node.id) and onClose(); on failure, suppress or log
the error (e.g., call process/log function or noop) and still call onClose() if
appropriate so the UI remains responsive. Ensure references to node.id,
onCopyId, onClose, and navigator.clipboard.writeText are used to locate and
modify the code.
| const shortcuts: KeyboardShortcuts = { | ||
| // Navigation | ||
| ArrowUp: { | ||
| description: 'Navigate up', | ||
| action: () => { | ||
| const node = event.shiftKey | ||
| ? findNodeInDirection('backward') | ||
| : findNodeInDirection('up') | ||
| if (node) onNodeSelect(node) | ||
| }, | ||
| }, | ||
| ArrowDown: { | ||
| description: 'Navigate down', | ||
| action: () => { | ||
| const node = event.shiftKey | ||
| ? findNodeInDirection('forward') | ||
| : findNodeInDirection('down') | ||
| if (node) onNodeSelect(node) | ||
| }, | ||
| }, | ||
| ArrowLeft: { | ||
| description: 'Navigate left', | ||
| action: () => { | ||
| const node = findNodeInDirection('left') | ||
| if (node) onNodeSelect(node) | ||
| }, | ||
| }, | ||
| ArrowRight: { | ||
| description: 'Navigate right', | ||
| action: () => { | ||
| const node = findNodeInDirection('right') | ||
| if (node) onNodeSelect(node) | ||
| }, | ||
| }, | ||
| Tab: { | ||
| description: 'Next/previous node', | ||
| action: () => { | ||
| event.preventDefault() | ||
| const node = event.shiftKey | ||
| ? navigateSequential('previous') | ||
| : navigateSequential('next') | ||
| if (node) onNodeSelect(node) | ||
| }, | ||
| }, | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "useKeyboardNavigation.ts" --type fRepository: verygoodplugins/automem
Length of output: 123
🏁 Script executed:
cat -n packages/graph-viewer/src/hooks/useKeyboardNavigation.ts | head -250Repository: verygoodplugins/automem
Length of output: 8556
🏁 Script executed:
cat -n packages/graph-viewer/src/hooks/useKeyboardNavigation.ts | sed -n '250,300p'Repository: verygoodplugins/automem
Length of output: 2090
🏁 Script executed:
cat -n packages/graph-viewer/src/hooks/useKeyboardNavigation.ts | sed -n '290,310p'Repository: verygoodplugins/automem
Length of output: 857
Prevent default browser scrolling for arrow-key navigation.
Arrow keys can scroll the page while you're navigating nodes. Add event.preventDefault() to the arrow key handlers to prevent this interference.
🔧 Suggested fix
ArrowUp: {
description: 'Navigate up',
action: () => {
+ event.preventDefault()
const node = event.shiftKey
? findNodeInDirection('backward')
: findNodeInDirection('up')
if (node) onNodeSelect(node)
},
},
ArrowDown: {
description: 'Navigate down',
action: () => {
+ event.preventDefault()
const node = event.shiftKey
? findNodeInDirection('forward')
: findNodeInDirection('down')
if (node) onNodeSelect(node)
},
},
ArrowLeft: {
description: 'Navigate left',
action: () => {
+ event.preventDefault()
const node = findNodeInDirection('left')
if (node) onNodeSelect(node)
},
},
ArrowRight: {
description: 'Navigate right',
action: () => {
+ event.preventDefault()
const node = findNodeInDirection('right')
if (node) onNodeSelect(node)
},
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const shortcuts: KeyboardShortcuts = { | |
| // Navigation | |
| ArrowUp: { | |
| description: 'Navigate up', | |
| action: () => { | |
| const node = event.shiftKey | |
| ? findNodeInDirection('backward') | |
| : findNodeInDirection('up') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| ArrowDown: { | |
| description: 'Navigate down', | |
| action: () => { | |
| const node = event.shiftKey | |
| ? findNodeInDirection('forward') | |
| : findNodeInDirection('down') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| ArrowLeft: { | |
| description: 'Navigate left', | |
| action: () => { | |
| const node = findNodeInDirection('left') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| ArrowRight: { | |
| description: 'Navigate right', | |
| action: () => { | |
| const node = findNodeInDirection('right') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| Tab: { | |
| description: 'Next/previous node', | |
| action: () => { | |
| event.preventDefault() | |
| const node = event.shiftKey | |
| ? navigateSequential('previous') | |
| : navigateSequential('next') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| const shortcuts: KeyboardShortcuts = { | |
| // Navigation | |
| ArrowUp: { | |
| description: 'Navigate up', | |
| action: () => { | |
| event.preventDefault() | |
| const node = event.shiftKey | |
| ? findNodeInDirection('backward') | |
| : findNodeInDirection('up') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| ArrowDown: { | |
| description: 'Navigate down', | |
| action: () => { | |
| event.preventDefault() | |
| const node = event.shiftKey | |
| ? findNodeInDirection('forward') | |
| : findNodeInDirection('down') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| ArrowLeft: { | |
| description: 'Navigate left', | |
| action: () => { | |
| event.preventDefault() | |
| const node = findNodeInDirection('left') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| ArrowRight: { | |
| description: 'Navigate right', | |
| action: () => { | |
| event.preventDefault() | |
| const node = findNodeInDirection('right') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, | |
| Tab: { | |
| description: 'Next/previous node', | |
| action: () => { | |
| event.preventDefault() | |
| const node = event.shiftKey | |
| ? navigateSequential('previous') | |
| : navigateSequential('next') | |
| if (node) onNodeSelect(node) | |
| }, | |
| }, |
🤖 Prompt for AI Agents
In `@packages/graph-viewer/src/hooks/useKeyboardNavigation.ts` around lines 169 -
213, The arrow-key handlers in the shortcuts map (ArrowUp, ArrowDown, ArrowLeft,
ArrowRight inside useKeyboardNavigation.ts) should call event.preventDefault()
at the start of each action to stop the browser from scrolling while navigating
nodes; update the action functions for ArrowUp, ArrowDown, ArrowLeft and
ArrowRight to call event.preventDefault() before computing findNodeInDirection
or calling onNodeSelect so navigation keys only affect node selection.
2 participants
This pull request introduces several improvements to the Automem project, most notably adding support for an optional interactive Graph Viewer frontend, improving cross-origin API accessibility, and enhancing the graph API with a new UMAP-based projection endpoint. It also includes build process updates and developer workflow documentation enhancements.
Graph Viewer Integration and API Improvements:
viewerblueprint (automem/api/viewer.py) to serve the optional Graph Viewer React SPA at/viewer/, controlled by theENABLE_GRAPH_VIEWERenvironment variable. The Dockerfile and application entrypoint (app.py) were updated to build and serve the frontend when enabled. [1] [2] [3] [4]/graph/projectedAPI endpoint that returns a 3D UMAP-projected layout of memory nodes based on their vector embeddings, supporting semantic visualization of memories. This includes lazy-loading of UMAP for performance and filtering options via query parameters. [1] [2]/graph/snapshotand/graph/projectedendpoints from 2000 to 5000 to support larger graphs.API Accessibility and Security:
Developer Experience and Build Process:
.flake8configuration file to standardize Python linting rules and exclude unnecessary directories.AGENTS.mdwith a clear task completion checklist for running builds, lints, and tests before declaring work complete.Frontend Build Artifacts:
packages/graph-viewer/.vite/deps/_metadata.json,packages/graph-viewer/.vite/deps/package.json) [1] [2]