feat(viewer): consolidate stable core and split-ready compatibility

[jack-arturo]

Summary

• Consolidates visualizer work into feat/visualizer-stable-core (stable core only)
• Switches AutoMem /viewer from in-process static SPA serving to standalone compatibility redirect/bootstrap using GRAPH_VIEWER_URL
• Preserves /viewer and /viewer/* compatibility routes while keeping hash token handling client-side
• Adds VIEWER_ALLOWED_ORIGINS CORS allowlist support and preflight handling
• Makes hand controls opt-in in graph viewer via VITE_ENABLE_HAND_CONTROLS=false default
• Adds standalone viewer runtime files and split-repo CI under packages/graph-viewer/.github/workflows/ci.yml
• Removes committed .vite artifacts and stale visualizer simplification doc
• Drops frontend build stage from AutoMem root Dockerfile

Config Changes

AutoMem env vars:

• ENABLE_GRAPH_VIEWER=true
• GRAPH_VIEWER_URL=https://<viewer-domain>
• VIEWER_ALLOWED_ORIGINS=https://<viewer-domain>[,https://... ]

Viewer env var:

• VITE_ENABLE_HAND_CONTROLS=false (opt-in)

Verification

• make lint ✅
• AUTOMEM_API_TOKEN=test-token ADMIN_API_TOKEN=test-admin-token EMBEDDING_PROVIDER=auto make test ✅ (152 passed, 1 skipped, 10 deselected)
• cd packages/graph-viewer && npm run build ✅

Curl checks (local API on :8012)

• curl -i http://localhost:8012/graph/snapshot?limit=10 -H 'X-API-Key: test-token' → 200 OK
• curl -i http://localhost:8012/viewer/?foo=bar → 200 OK (bootstrap HTML)
• curl -i http://localhost:8012/viewer/assets/index.js?v=1 → 302 FOUND to https://viewer.example.com/assets/index.js?v=1
• curl -i -X OPTIONS http://localhost:8012/graph/snapshot -H 'Origin: https://viewer.example.com' -H 'Access-Control-Request-Method: GET' -H 'Access-Control-Request-Headers: X-API-Key, Content-Type' → 200 OK, Access-Control-Allow-Origin: https://viewer.example.com

Notes

• Python make build target does not exist in this repo; verification used lint + tests per current Makefile.
• Split branch for standalone repo has been created with history preservation via git subtree split --prefix=packages/graph-viewer -b split/automem-graph-viewer.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

---

📝 Walkthrough

Summary by CodeRabbit

Release Notes

• New Features

  • Integrated standalone Graph Viewer accessible through a /viewer endpoint with backward compatibility and URL data preservation.
  • Added CORS support for cross-origin API requests with configurable origin allowlisting.

• Documentation

  • Updated environment variables and deployment documentation with Graph Viewer configuration options and setup guidance.

Walkthrough

This PR introduces a Graph Viewer integration that enables the API to forward viewer requests to a standalone graph viewer service. Changes include CORS support, environment variable configuration for the viewer URL and allowed origins, a viewer compatibility layer at /viewer, and associated tests and documentation.

Changes

Cohort / File(s)	Summary
Configuration and Dependencies .env.example, .flake8, .gitignore, requirements.txt, Dockerfile	Added environment variables for graph viewer configuration, Flake8 linting rules, dependency on flask-cors, and minor comment updates.
Core Viewer Integration app.py, automem/api/viewer.py, automem/api/runtime_bootstrap.py	Implemented Flask-CORS configuration with dynamic origins, added new viewer blueprint module that forwards requests to standalone Graph Viewer service, conditionally registers viewer blueprint during app bootstrap, and handles CORS preflight and viewer route bypassing in request middleware.
Documentation README.md, docs/ENVIRONMENT_VARIABLES.md, docs/RAILWAY_DEPLOYMENT.md	Added documentation for Graph Viewer service, configuration variables (ENABLE_GRAPH_VIEWER, GRAPH_VIEWER_URL, VIEWER_ALLOWED_ORIGINS), deployment guidance, and backward-compatibility details.
Tests tests/contracts/test_routes_contract.py, tests/test_api_endpoints.py	Updated route contract to expect /viewer/ and /viewer/<path:path> endpoints; added tests for viewer bootstrap HTML, asset redirects, missing configuration handling, and CORS preflight responses.

Sequence Diagram

sequenceDiagram
    participant Client
    participant AutoMem as AutoMem API
    participant Viewer as Graph Viewer Service

    Client->>AutoMem: GET /viewer/my-graph
    Note over AutoMem: Check ENABLE_GRAPH_VIEWER
    AutoMem->>AutoMem: Build bootstrap HTML<br/>(preserve hash, add server param)
    AutoMem->>Client: 200 OK + HTML bootstrap
    Client->>Client: Execute redirect JS<br/>(to Graph Viewer with params)
    Client->>Viewer: GET /my-graph?server=<automem-origin>#token=...
    Viewer->>Client: 200 OK (graph data)

    Client->>AutoMem: GET /viewer/assets/index.js
    AutoMem->>Client: 302 Redirect
    Client->>Viewer: GET /assets/index.js
    Viewer->>Client: 200 OK (asset)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 29.06% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately reflects the main change: externalizing the graph viewer and switching from in-process serving to standalone compatibility mode, which represents the primary objective of consolidating visualizer work and implementing a split-ready architecture.
Description check	✅ Passed	The description is comprehensive and directly related to the changeset, detailing the switch from in-process viewer serving to standalone redirect/bootstrap, environment variables, verification steps, and implementation notes that align with the file changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/visualizer-stable-core

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 10

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (19)

packages/graph-viewer/src/components/RadialMenu.tsx-94-99 (1)

94-99: ⚠️ Potential issue | 🟡 Minor

Handle clipboard API errors gracefully.

navigator.clipboard.writeText() returns a Promise that can reject (e.g., in non-secure contexts or if permission is denied). The current implementation ignores failures silently, leaving users without feedback.

🛡️ Proposed fix to add error handling

   // Copy ID to clipboard
-  const handleCopyId = useCallback(() => {
-    navigator.clipboard.writeText(node.id)
-    onCopyId?.(node.id)
-    onClose()
+  const handleCopyId = useCallback(async () => {
+    try {
+      await navigator.clipboard.writeText(node.id)
+      onCopyId?.(node.id)
+    } catch (err) {
+      console.error('Failed to copy ID to clipboard:', err)
+    }
+    onClose()
   }, [node.id, onCopyId, onClose])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/RadialMenu.tsx` around lines 94 - 99,
The handleCopyId handler calls navigator.clipboard.writeText(node.id) but
doesn't handle rejected Promises; wrap the clipboard call in an async try/catch
(or use .then/.catch) inside handleCopyId so failures are handled: await
navigator.clipboard.writeText(node.id) inside try, call onCopyId?.(node.id) and
onClose() only on success, and in the catch branch log the error (console.error)
and surface user feedback (e.g., call an optional onCopyError callback or
fallback to document.execCommand('copy') and/or show a UI toast). Update the
handleCopyId definition accordingly to properly reference handleCopyId,
navigator.clipboard.writeText, onCopyId, onClose (and add an onCopyError prop if
you choose to report errors).

packages/graph-viewer/src/hooks/useLassoSelection.ts-188-195 (1)

188-195: ⚠️ Potential issue | 🟡 Minor

Duplicate onSelectionChange callback invocation.

clearSelection calls onSelectionChange?.([]) directly on line 194, but lines 217-224 also invoke onSelectionChange during render when selection changes. This causes the callback to fire twice when clearing selection.

Remove the direct call here and let the render-time detection handle it consistently with other mutations like toggleNodeSelection.

Proposed fix

   // Clear all selected nodes
   const clearSelection = useCallback(() => {
     setState((prev) => ({
       ...prev,
       selectedIds: new Set(),
     }))
-    onSelectionChange?.([])
-  }, [onSelectionChange])
+  }, [])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/hooks/useLassoSelection.ts` around lines 188 - 195,
The clearSelection callback currently both updates state and directly calls
onSelectionChange, causing duplicate callbacks; remove the direct
onSelectionChange?.([]) invocation from the clearSelection function (leave
setState((prev) => ({ ...prev, selectedIds: new Set() })) intact) so that the
existing render-time detection/effect that watches selectedIds (the logic that
invokes onSelectionChange when selection changes) is the single place that
notifies listeners. Ensure clearSelection still uses the useCallback and its
dependency array remains correct (it should not include onSelectionChange since
it will no longer call it directly).

packages/graph-viewer/src/components/SelectionActions.tsx-94-104 (1)

94-104: ⚠️ Potential issue | 🟡 Minor

CSV escaping is incomplete for edge cases.

The current CSV generation only escapes double quotes in content. If content contains newlines, id contains commas, or tags contain quotes, the CSV will be malformed. Consider escaping all fields consistently.

🔧 Proposed fix with consistent escaping

+  // Helper to escape CSV field
+  const escapeCSV = (value: string | number) => {
+    const str = String(value)
+    if (str.includes(',') || str.includes('"') || str.includes('\n')) {
+      return `"${str.replace(/"/g, '""')}"`
+    }
+    return str
+  }
+
   // Export selection as CSV
   const handleExportCSV = () => {
     if (onExportSelection) {
       onExportSelection(selectedNodes, 'csv')
     } else {
       const headers = ['id', 'content', 'type', 'tags', 'importance', 'timestamp']
       const rows = selectedNodes.map((n) =>
         [
-          n.id,
-          `"${n.content.replace(/"/g, '""')}"`,
-          n.type,
-          `"${n.tags.join(', ')}"`,
-          n.importance,
-          n.timestamp,
+          escapeCSV(n.id),
+          escapeCSV(n.content),
+          escapeCSV(n.type),
+          escapeCSV(n.tags.join(', ')),
+          escapeCSV(n.importance),
+          escapeCSV(n.timestamp),
         ].join(',')
       )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/SelectionActions.tsx` around lines 94 -
104, The CSV generation currently only escapes double quotes in content and
misses other edge cases; update SelectionActions.tsx to consistently escape
every field: implement a small helper function (e.g., escapeCsv(value)) that
converts undefined/null to empty string, stringifies the value, replaces any
double quotes with two double quotes, and wraps the result in double quotes if
the value contains commas, quotes or newlines (or simply always wrap all fields
in quotes). Then use this helper when building headers and when mapping
selectedNodes to rows (replace the current inline escaping for n.content and the
raw usage of n.id, n.tags, etc.) so id, content, type, tags, importance, and
timestamp are all escaped consistently.

packages/graph-viewer/src/components/settings/SettingsSection.tsx-15-28 (1)

15-28: ⚠️ Potential issue | 🟡 Minor

Add type="button", aria-expanded, and aria-controls attributes for accessibility and form safety.

The button lacks an explicit type attribute (defaults to type="submit"), should expose its expanded state via aria-expanded, and establish a control relationship with the content via aria-controls. The content div requires an id to be referenced.

🔧 Suggested fix

-import { useState, ReactNode } from 'react'
+import { useState, ReactNode, useId } from 'react'
 import { ChevronDown } from 'lucide-react'

 interface SettingsSectionProps {
   title: string
   defaultOpen?: boolean
   children: ReactNode
 }

 export function SettingsSection({ title, defaultOpen = true, children }: SettingsSectionProps) {
   const [isOpen, setIsOpen] = useState(defaultOpen)
+  const contentId = useId()

   return (
     <div className="border-b border-white/5 last:border-b-0">
       <button
         onClick={() => setIsOpen(!isOpen)}
+        type="button"
+        aria-expanded={isOpen}
+        aria-controls={contentId}
         className="w-full flex items-center justify-between px-4 py-3 hover:bg-white/5 transition-colors"
       >
         <span className="text-sm font-medium text-slate-300">{title}</span>
         <ChevronDown
           className={`w-4 h-4 text-slate-500 transition-transform duration-200 ${
             isOpen ? 'rotate-0' : '-rotate-90'
           }`}
         />
       </button>
       {isOpen && (
-        <div className="px-4 pb-4 space-y-3">
+        <div id={contentId} className="px-4 pb-4 space-y-3">
           {children}
         </div>
       )}
     </div>
   )
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/settings/SettingsSection.tsx` around
lines 15 - 28, The SettingsSection button toggle currently omits type and ARIA
attributes; update the <button> in the SettingsSection component (the one that
calls setIsOpen and reads isOpen, renders title and ChevronDown) to include
type="button", aria-expanded={isOpen}, and aria-controls pointing to the content
container, and give the content <div> (the one that wraps children) a stable id;
generate the id using React's useId() or accept a prop (e.g., sectionId) so
aria-controls matches that id and remains unique/stable across renders.

packages/graph-viewer/scripts/hand-tracking-server.js-205-207 (1)

205-207: ⚠️ Potential issue | 🟡 Minor

Fix incorrect port in user-facing hint.

The HTML displays port 8765, but PHONE_PORT defaults to 8768. This will confuse users trying to connect their iPhone.

🐛 Proposed fix

     <div class="info">
-        iPhone should connect to: ws://&lt;your-mac-ip&gt;:8765
+        iPhone should connect to: ws://&lt;your-mac-ip&gt;:${PHONE_PORT}
     </div>

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/scripts/hand-tracking-server.js` around lines 205 -
207, The user-facing hint in the HTML (the <div class="info"> text) shows port
8765 but the server default constant PHONE_PORT (in hand-tracking-server.js) is
8768, so update the hint to use the PHONE_PORT value (or change the hardcoded
8765 to match PHONE_PORT) so they stay in sync; locate the string in the HTML
rendering and either interpolate PHONE_PORT into that message or replace the
literal "8765" with the PHONE_PORT constant to avoid future drift.

packages/graph-viewer/src/hooks/useForceLayout.ts-38-42 (1)

38-42: ⚠️ Potential issue | 🟡 Minor

Data signature may produce false cache hits.

The signature only considers length, first ID, and last ID. If nodes in the middle are replaced while first/last IDs and count remain unchanged, the cache will incorrectly return stale positions.

Consider using a hash of all node IDs or including a version/timestamp from the data source.

🛡️ Proposed fix

 function createDataSignature(nodes: GraphNode[]): string {
   if (nodes.length === 0) return ''
-  return `${nodes.length}-${nodes[0]?.id}-${nodes[nodes.length - 1]?.id}`
+  // Include all IDs for accurate cache invalidation
+  return nodes.map(n => n.id).join(',')
 }

If this becomes expensive for large graphs, consider a hash function or including a data version from the API response.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/hooks/useForceLayout.ts` around lines 38 - 42, The
createDataSignature function currently only uses nodes.length, nodes[0].id and
nodes[last].id which can produce false cache hits; update createDataSignature to
incorporate all node identities (e.g., concatenate or compute a hash over
nodes.map(n => n.id).join(',')) or include a data-version/timestamp from the
source so any middle-node changes invalidate the cache; ensure you reference
createDataSignature and GraphNode and replace the simple triple-field string
with a full-ID-derived signature or hashed value to avoid stale position reuse.

packages/graph-viewer/src/utils/OneEuroFilter.ts-254-261 (1)

254-261: ⚠️ Potential issue | 🟡 Minor

Guard against landmark array length mismatch.

If landmarks.length exceeds 21, this.filters[i] will be undefined for indices ≥ 21, causing a runtime error when calling .filter(). Consider validating or slicing the input.

🛡️ Proposed fix

   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),
+    return landmarks.slice(0, 21).map((lm, i) => ({
+      ...this.filters[i]!.filter(lm, timestamp),
       visibility: lm.visibility,
     }))
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/utils/OneEuroFilter.ts` around lines 254 - 261, The
filter method in OneEuroFilter assumes landmarks.length matches
this.filters.length and will throw if landmarks has >21 entries; guard against
length mismatches by limiting the input or iterating against this.filters
length: in the filter(...) method, either slice the incoming landmarks array to
this.filters.length or map over this.filters (e.g., use this.filters.map((f, i)
=> ... ) and safely handle missing landmark entries), ensuring you reference
this.filters and the filter(...) call on each filter; also preserve visibility
from the original landmark only when that landmark exists.

packages/graph-viewer/src/hooks/useCameraState.ts-67-105 (1)

67-105: ⚠️ Potential issue | 🟡 Minor

Cancel pending animation on unmount to prevent memory leaks.

If the component unmounts while an animation is in progress, the requestAnimationFrame callback continues executing and may reference a stale camera or controls object. Add a cleanup effect.

🛡️ Proposed fix

 export function useCameraNavigation() {
   const { camera, controls } = useThree()
   const animationRef = useRef<number | null>(null)

+  // Cleanup on unmount
+  useEffect(() => {
+    return () => {
+      if (animationRef.current) {
+        cancelAnimationFrame(animationRef.current)
+      }
+    }
+  }, [])
+
   const navigateTo = useCallback((targetX: number, targetY: number, duration = 500) => {
     // ...
   }, [camera, controls])

   return { navigateTo }
 }

You'll also need to add useEffect to the imports on line 7.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/hooks/useCameraState.ts` around lines 67 - 105, Add
a cleanup effect to useCameraNavigation that cancels any pending
requestAnimationFrame and clears animationRef on unmount to avoid referencing
stale camera/controls; specifically import useEffect, add a useEffect(() => {
return () => { if (animationRef.current)
cancelAnimationFrame(animationRef.current); animationRef.current = null; } },
[]) inside the same scope as navigateTo so any in-flight animation is canceled
when the component using useCameraNavigation unmounts, referencing animationRef,
cancelAnimationFrame, and navigateTo for context.

packages/graph-viewer/src/components/TimelineBar.tsx-105-142 (1)

105-142: ⚠️ Potential issue | 🟡 Minor

Missing preventDefault() on arrow keys allows unintended node navigation during time travel.

When time travel is active, pressing ArrowLeft/ArrowRight triggers both timeline stepping (TimelineBar) and spatial node navigation (useKeyboardNavigation) simultaneously, since neither calls preventDefault() on these keys. TimelineBar already calls preventDefault() for the Space key, so add the same for arrow keys to prevent the conflicting behavior.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/TimelineBar.tsx` around lines 105 - 142,
The keyboard handler in the TimelineBar component's useEffect (handleKeyDown)
doesn't call e.preventDefault() for 'ArrowLeft' and 'ArrowRight', which allows
competing handlers (e.g., useKeyboardNavigation) to run; update handleKeyDown so
that when handling both ArrowLeft and ArrowRight (including their shiftKey
variants that call onStepBackward/onStepForward/onGoToStart/onGoToEnd) you call
e.preventDefault() before invoking those callbacks to stop default/spatial
navigation and ensure only the timeline actions run.

packages/graph-viewer/src/api/client.ts-37-45 (1)

37-45: ⚠️ Potential issue | 🟡 Minor

Token exposed in URL query parameter may leak via referrer headers and browser history.

The getTokenFromQuery() function reads tokens from URL query parameters. While convenient for development, query parameters are logged in server access logs, browser history, and can leak via Referer headers. The hash-based approach (getTokenFromHash) is more secure since fragments aren't sent to the server.

Consider documenting this security trade-off or restricting query param tokens to development mode only.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/api/client.ts` around lines 37 - 45,
getTokenFromQuery reads sensitive tokens from URL query params which can leak
via referrer headers and history; update getToken so it only falls back to
getTokenFromQuery in non-production/dev environments (e.g., check NODE_ENV or
import.meta.env.MODE) or remove the query-param branch entirely and prefer
getTokenFromHash() and localStorage.getItem('automem_token'); also add a short
comment near getTokenFromQuery/getToken noting the security trade-off and that
query tokens are allowed only for development/testing.

packages/graph-viewer/src/hooks/useFocusMode.ts-10-10 (1)

10-10: ⚠️ Potential issue | 🟡 Minor

Minor doc inconsistency: comment says 10% but constant is 8%.

Line 10 states "Beyond: 10% opacity" but DEFAULT_OPACITY on line 35 is 0.08 (8%). Consider updating the comment to match the actual value.

📝 Proposed fix

- * - Beyond: 10% opacity
+ * - Beyond: 8% opacity

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/hooks/useFocusMode.ts` at line 10, The doc comment
in useFocusMode.ts incorrectly states "Beyond: 10% opacity" while the actual
constant DEFAULT_OPACITY is 0.08 (8%); update the comment to match the code (or
change DEFAULT_OPACITY to 0.10 if you prefer 10%) so they are consistent—locate
DEFAULT_OPACITY in useFocusMode.ts and either change the comment text to
"Beyond: 8% opacity" or set DEFAULT_OPACITY = 0.10 and adjust any related logic
accordingly.

packages/graph-viewer/src/api/client.ts-112-123 (1)

112-123: ⚠️ Potential issue | 🟡 Minor

Falsy check may skip valid zero values for limit and minImportance.

The truthy checks (if (params.limit)) will not include these parameters when their values are 0. If limit=0 or minImportance=0 are valid API values, they won't be sent. Consider using explicit !== undefined checks if zero is meaningful.

📝 Proposed fix (if zero values are valid)

- if (params.limit) searchParams.set('limit', String(params.limit))
- if (params.minImportance) searchParams.set('min_importance', String(params.minImportance))
+ if (params.limit !== undefined) searchParams.set('limit', String(params.limit))
+ if (params.minImportance !== undefined) searchParams.set('min_importance', String(params.minImportance))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/api/client.ts` around lines 112 - 123, The
fetchGraphSnapshot function currently uses truthy checks for params.limit and
params.minImportance which will omit valid zero values; update the conditionals
in fetchGraphSnapshot to test explicitly for undefined (e.g., params.limit !==
undefined and params.minImportance !== undefined) so that 0 is serialized and
sent to the API, leaving the checks for params.types and params.since unchanged;
ensure SnapshotParams handling and the URLSearchParams logic still work with the
explicit checks.

packages/graph-viewer/src/components/MiniMap.tsx-76-98 (1)

76-98: ⚠️ Potential issue | 🟡 Minor

Guard against zero-sized bounds to prevent NaN coordinates.

If all nodes share the same x/y, rangeX and rangeY become 0, so scale is 0 and Lines 82 and 94 divide by zero. Add a fallback scale.

🛠️ Suggested fix

-    const scale = Math.max(rangeX, rangeY)
+    const scale = Math.max(rangeX, rangeY) || 1

-    const scale = Math.max(rangeX, rangeY)
+    const scale = Math.max(rangeX, rangeY) || 1

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/MiniMap.tsx` around lines 76 - 98,
worldToCanvas and canvasToWorld can divide by zero when bounds are degenerate;
guard against zero-sized bounds by providing a fallback scale (e.g., use
Math.max(rangeX, rangeY) and if that is 0 use 1 or a small epsilon) and use that
fallback in both functions (worldToCanvas and canvasToWorld) so the division
never produces NaN; update the scale computation in both useCallback blocks to
use the fallback and keep the rest of the coordinate math unchanged.

packages/graph-viewer/src/hooks/useHandGestures.ts-319-387 (1)

319-387: ⚠️ Potential issue | 🟡 Minor

Effect re-runs on onResults change causing expensive re-initialization.

The initialization effect depends on onResults, which is recreated whenever smoothing or onGestureChange changes. This triggers full MediaPipe re-initialization (stopping camera, closing hands, recreating). Consider using a ref for the callback to avoid this.

🐛 Use ref to avoid re-initialization

+  const onResultsRef = useRef(onResults)
+  useEffect(() => {
+    onResultsRef.current = onResults
+  }, [onResults])

   // Initialize MediaPipe
   useEffect(() => {
     if (!enabled || isInitializedRef.current) return

     // ... initialization code ...

-      hands.onResults(onResults)
+      hands.onResults((results) => onResultsRef.current(results))

     // ... rest of initialization ...
-  }, [enabled, onResults])
+  }, [enabled])

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/hooks/useHandGestures.ts` around lines 319 - 387,
The effect initializing MediaPipe re-runs whenever the onResults callback
changes (it’s recreated when smoothing or onGestureChange change), causing
expensive teardown/re-init; fix by storing the dynamic callback in a ref (e.g.,
onResultsRef) and having the effect depend only on enabled (not onResults), set
onResultsRef.current = onResults wherever onResults is recreated (or inside a
small useEffect watching smoothing/onGestureChange), and register a stable
wrapper with hands.onResults that forwards to onResultsRef.current; keep
initializeHands, isInitializedRef, handsRef, cameraRef, videoRef logic but
remove onResults from the effect dependency list so initialization only runs
when enabled changes.

packages/graph-viewer/src/components/Hand2DOverlay.tsx-332-334 (1)

332-334: ⚠️ Potential issue | 🟡 Minor

handId based on position may collide between hands.

Using Math.round(points[0].x * 10) for gradient IDs could cause collisions when both hands are at similar X positions, leading to gradient/filter conflicts. Consider using a stable identifier based on hand side.

🐛 Use hand side for stable ID

 function GhostHand({
   landmarks,
   color: _color,
   gradientId: _gradientId,
   isGhost = false,
   opacityMultiplier = 1,
+  side = 'left',
 }: GhostHandProps) {
   // ...
-  const handId = Math.round(points[0].x * 10)
+  const handId = side

Then pass side="left" or side="right" from the parent.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/Hand2DOverlay.tsx` around lines 332 -
334, The gradient/filter ID generation uses a position-derived value (const
handId = Math.round(points[0].x * 10)) which can collide; change Hand2DOverlay
to accept a stable side prop (e.g., side: "left" | "right") and compute handId
from that (or combine side with a unique index) instead of points[0].x so
gradient/filter IDs are deterministic per hand; update the parent to pass
side="left" or side="right" and replace uses of handId computed from points with
the new side-based identifier.

packages/graph-viewer/src/components/SelectionHighlight.tsx-182-192 (1)

182-192: ⚠️ Potential issue | 🟡 Minor

Geometry clone in render causes memory leak.

ringGeometry.clone() is called on every render, creating new Three.js geometry objects that aren't disposed. This will leak memory over time.

🐛 Proposed fix using a second memoized geometry

   // Ring geometry
   const ringGeometry = useMemo(() => {
     return new THREE.RingGeometry(innerRadius, outerRadius, 32)
   }, [innerRadius, outerRadius])
+  
+  // Second ring for XZ plane
+  const ringGeometryXZ = useMemo(() => {
+    return new THREE.RingGeometry(innerRadius, outerRadius, 32)
+  }, [innerRadius, outerRadius])

   // ... later in JSX ...
       {/* Selection ring - XZ plane */}
       <mesh rotation={[Math.PI / 2, 0, 0]}>
-        <primitive object={ringGeometry.clone()} />
+        <primitive object={ringGeometryXZ} />
         <meshBasicMaterial

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/SelectionHighlight.tsx` around lines 182
- 192, The render currently calls ringGeometry.clone() inside
SelectionHighlight's JSX which allocates a new Three.js Geometry each render and
leaks memory; change this to create and reuse a cloned geometry (e.g., with
useMemo or useRef) such as memoizedRingGeometry = useMemo(() =>
ringGeometry.clone(), [ringGeometry]) and use that in the <primitive> instead of
calling clone() inline, and on component cleanup dispose of the cloned geometry
(call dispose() in a useEffect cleanup) so the cloned resource is released.
Reference: SelectionHighlight component, ringGeometry variable, and the
<primitive> element.

packages/graph-viewer/src/App.tsx-319-322 (1)

319-322: ⚠️ Potential issue | 🟡 Minor

Remove unused nodes parameter or fix its type.

The nodes as any cast bypasses type checking for a parameter that is never used in the hook. SimulationNode[] cannot accept GraphNode[] without casting because it expects additional properties (fx, fy, fz). Either remove the unused nodes parameter from the usePathfinding call, or align the type signature to accept GraphNode[] directly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/App.tsx` around lines 319 - 322, The call to
usePathfinding passes nodes as "nodes as any" to avoid the type mismatch between
GraphNode[] and SimulationNode[] (which expects fx/fy/fz) even though
usePathfinding does not use those SimulationNode-specific props; either remove
the nodes parameter from the usePathfinding invocation (call usePathfinding({
edges }) and update the hook signature if necessary) or change the hook
signature to accept GraphNode[] (update usePathfinding's param type from
SimulationNode[] to GraphNode[] and remove the cast), ensuring references to
usePathfinding, nodes, SimulationNode[], and GraphNode[] are updated
accordingly.

packages/graph-viewer/src/hooks/useHandPlayback.ts-436-436 (1)

436-436: ⚠️ Potential issue | 🟡 Minor

hasRecording reads ref directly and won't update UI.

recordingRef.current !== null is evaluated at render time but won't cause re-renders when loadRecording is called since refs don't trigger updates. The recordingName from state can be used instead.

🛠️ Proposed fix

- hasRecording: recordingRef.current !== null,
+ hasRecording: state.recordingName !== '',

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/hooks/useHandPlayback.ts` at line 436, The
hasRecording flag currently reads recordingRef.current directly so it won't
trigger re-renders; change it to derive from the component state instead (e.g.,
use recordingName). Replace hasRecording: recordingRef.current !== null with
something like hasRecording: Boolean(recordingName) or hasRecording:
recordingName !== '' in the return/object provided by useHandPlayback so UI
updates when loadRecording updates recordingName; keep recordingRef and
loadRecording as-is for imperative access.

packages/graph-viewer/src/components/GraphCanvas.tsx-1448-1454 (1)

1448-1454: ⚠️ Potential issue | 🟡 Minor

Object allocation inside render loop for lasso-selected nodes.

A new THREE.Color is created inside the useFrame loop for each lasso-selected node. This should use the existing tempColor object and a separate pre-allocated color for lerping.

🛠️ Proposed fix

+ // Add near other temp objects (around line 1322)
+ const blueColor = useMemo(() => new THREE.Color('#3b82f6'), [])

  // Inside useFrame, replace:
  } else if (isLassoSelected) {
    tempColor.set(node.color)
-   const blueColor = new THREE.Color('#3b82f6')
    tempColor.lerp(blueColor, 0.35)
  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/graph-viewer/src/components/GraphCanvas.tsx` around lines 1448 -
1454, The lasso branch in the useFrame loop (isLassoSelected) allocates a new
THREE.Color each frame; instead pre-allocate a reusable color (e.g., lassoBlue)
outside the render loop and reuse tempColor for operations. Replace the inline
new THREE.Color('#3b82f6') with the preallocated lassoBlue and call
tempColor.set(node.color) then tempColor.lerp(lassoBlue, 0.35) inside the
isLassoSelected branch (update where tempColor and isLassoSelected are used in
GraphCanvas.tsx).