feat(memory-graph): configurable labels + popover z-index#1201
Conversation
Document-facing copy and hover popover stacking were hardcoded, forcing downstream apps to patch dist/ output when product vocabulary (sources vs documents) or app-shell layering differ. - add prop (MemoryGraphLabels): legend group, edge copy, popover type fallback, ID label, nav actions, memory/loading count templates; defaults preserve existing strings exactly - add prop; default stays 100 - export DEFAULT_LABELS, DEFAULT_HOVER_POPOVER_Z_INDEX + new types
![vorflux[bot]](https://avatars.githubusercontent.com/in/2483280?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
Reviewed — found 1 issue(s). I reviewed the configurable memory graph labels and popover layering diff for API correctness and runtime regressions. The new shallow label merge can remove required default label functions when callers pass optional properties as undefined.
Findings
packages/memory-graph/src/components/memory-graph.tsx
labelOverridescan overwrite required defaults withundefined, which can later crash label consumers such asNodeHoverPopoverorLoadingIndicator.
Verdict
undefined so partial or conditional label objects cannot remove required labels.
| layering, | ||
| }: MemoryGraphProps) { | ||
| const resolvedLabels = useMemo<ResolvedMemoryGraphLabels>( | ||
| () => ({ ...DEFAULT_LABELS, ...labelOverrides }), |
There was a problem hiding this comment.
Because MemoryGraphLabels properties are optional, callers can pass values such as { memoryCount: undefined }. This spread treats that undefined as an override and removes the default function, so later calls like labels.memoryCount(...) can crash. Please merge only defined override values, or nullish-coalesce each resolved label back to DEFAULT_LABELS.
TestingBrowser testing for the changed memory graph UI passed in the playground, including default labels, expanded legend rendering, and hover popover layering. However, the Commands run: git fetch origin pull/1201/head:refs/heads/pr-1201-test
git checkout pr-1201-test
bun install --frozen-lockfile
cd packages/memory-graph && bun run test
cd apps/memory-graph-playground && bun run dev:app --hostname 0.0.0.0
session update-preview-url --url localhost:3004
port expose --port 3004
curl -sS -L https://11nc2ikdv3ui.preview.us1.vorflux.com
npx agent-browser ...Result: Browser evidence: VerdictAttached Images and Videos |
1 participant
Document-facing copy and hover popover stacking were hardcoded, forcing downstream apps to patch dist/ output when product vocabulary (sources vs documents) or app-shell layering differ.
Problem
@supermemory/memory-graph hardcodes document-facing copy (legend group, edge labels, popover actions/type/ID label) and the hover popover z-index (100). Downstream apps with different vocabulary ("sources") or shell layering must patch dist/ output on every upgrade.
Solution
labelsprop (MemoryGraphLabels) — all document-facing strings configurable, incl.memoryCount(count)andloadingMoreDocuments(count)templates. Partial overrides merge withDEFAULT_LABELS, which preserve current strings exactly.layeringprop (MemoryGraphLayering) —hoverPopoverZIndex, default 100 unchanged.DEFAULT_LABELS,DEFAULT_HOVER_POPOVER_Z_INDEX,MemoryGraphLabels,MemoryGraphLayering,ResolvedMemoryGraphLabels.No props = identical output (verified by mounted render test). Backward compatible.
Testing
tsc --noEmit, biome,vite buildall cleanCloses #1200
Session Details
(aside)to your comment to have me ignore it.