Codex review on PR #114: when the RAF poll window expires before
highlighted nodes have been measured, the timeout fallback was setting
`fittedKeyRef.current = targetKey`, marking the step as fitted even
though the proper highlight fit never ran. If Stage 2 layout landed
after the 4s cap, the effect early-returned on the next nodes update
because the target key already matched, so the camera stayed pinned to
the fallback layer fit instead of zooming onto the actual highlights.
Fix:
- Subscribe to React Flow's user-node array via `useNodes()` so the
effect re-fires when Stage 2 finally produces the highlighted ids
after the per-step RAF poll has already given up.
- On timeout, pan into the layer for usability but do NOT set
`fittedKeyRef`. The next nodes update gets another shot at the
highlight fit, and on success `fittedKeyRef` records the proper fit.
- Use a separate `fallbackKeyRef` to ensure the fallback `fitView`
fires at most once per step — without this, every subsequent nodes
update during the unready window would trigger a viewport jump.
- Reset both refs when `tourHighlightedNodeIds` clears (stop tour).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codex review on PR #114 flagged that `layerResetIfChanged` cleared
`containerLayoutCache` and `expandedContainers` but left
`pendingFocusContainer` intact. Because container ids collide across
layers (the very reason the cache reset exists), a manual expand in
layer A that hadn't yet hit its 1.2s clear timer could leak its id
into layer B's namespace and recenter the viewport on an unrelated
container right after navigation.
The same hazard applies to every other reset path that drops the
container caches. Add `pendingFocusContainer: null` to all of them:
- layerResetIfChanged (tour cross-layer reset, the originally flagged
site)
- drillIntoLayer
- navigateToOverview
- setFocusNode
- setPersona
- setGraph
- toggleNodeTypeFilter
- clearContainerLayouts
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Four related issues that surfaced while walking the Learn-mode tour
through a multi-layer project (microservices-demo):
1. Tour auto-expand never released. The tour effect that expands
highlighted nodes' containers had no corresponding collapse when the
step changed, so containers accumulated open as the user advanced.
Track the set of containers we expanded and release any not needed
by the current step; user-toggled containers are never tracked here,
so they're never auto-collapsed.
2. Manual container toggle yanked off-screen. Stage 2 reflow shifted
the just-clicked container away from the cursor. `toggleContainer`
now records `pendingFocusContainer` on expand; GraphView locks the
viewport onto that container's centre with the current zoom so it
appears to expand in place.
3. Tour fitView fired before highlighted children existed. A single
RAF after `tourHighlightedNodeIds` change wasn't enough — child
nodes only appear once Stage 2 layout writes
`containerLayoutCache`, and React Flow only knows their absolute
position after a measure pass. `useNodes()` doesn't fire on
measure completion, so we poll `getInternalNode().measured` each
frame (up to ~4s) and call `fitView({ nodes })` once every
highlight is measured, with `maxZoom: 1.2 / minZoom: 0.4`. While
waiting, a new `tourFitPending` flag drives a "Locating tour
highlight…" overlay so the user knows the layout is still settling.
4. Cross-layer tour transitions reused stale Stage 2 cache. Container
ids derive from per-layer state (folder names in folder strategy,
`container:cluster-N` in community strategy) and collide across
layers — API Contracts and Load Testing both produce
`container:cluster-0`. `setTourStep` / `nextTourStep` /
`prevTourStep` / `startTour` didn't reset the container caches the
way `drillIntoLayer` does, so when tour crossed a layer the new
layer's expanded containers hit the previous layer's cache, Stage 2
skipped its rerun, and children never showed. Extracted
`layerResetIfChanged` and applied it in all four tour actions.
Verified end-to-end against microservices-demo via headless Chrome:
all 15 tour steps now expand the right container(s), zoom onto the
referenced files, and collapse the previous step's auto-expansions.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reviewer @Lum1104 (PR #112) caught a silent semantic regression: the new
`filterNodes` reads layer membership through `nodeIdToLayerId.get(node.id)`,
which is first-wins. The pre-#112 path was any-layer-wins —
`layers.some(layer => filters.layerIds.has(layer.id) && layer.nodeIds.includes(node.id))`.
For a node X listed in both L1 and L2 with only L2 selected, the old code
kept X; the new code dropped it. The schema permits multi-layer membership,
so this was a behavior change, not a bug fix.
Fix: keep two distinct indexes in the store. Both are rebuilt once on
`setGraph`, so the O(1)-per-node performance win from #112 is preserved.
- `nodeIdToLayerId: Map<string, string>` — first-matching-layer wins.
Drives navigation (drillIntoLayer / tour step → layer / sidebar
history) where one canonical layer is the right answer. Unchanged.
- `nodeIdToLayerIds: Map<string, Set<string>>` — every layer the node
belongs to. Drives `filterNodes` membership checks. Restores
any-layer-wins exactly.
`filterNodes` now iterates the (small) layer-id set per node looking for
intersection with `filters.layerIds`. ExportMenu reads
`nodeIdToLayerIds` from the store.
Verified locally:
- Added `filters.test.ts` regression: node in (L1, L2) with only L2
selected must pass. Failed against the first-wins implementation;
passes now.
- `pnpm --filter @understand-anything/dashboard test` — 42 / 42 pass
(was 41; +1 multi-layer regression test; perf-guard at 100 layers ×
100 nodes still <50 ms).
- `pnpm --filter @understand-anything/dashboard exec tsc --noEmit` — clean.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three hot paths in the dashboard ran `layer.nodeIds.includes(node.id)`,
which is O(K) per check. Combined with their enclosing loops they
collectively spent quadratic time per render of the overview / per
filter recompute / per node-selection event. On the 4.8 MB knowledge
graph reported in #102, the overview render alone took ~470 ms of
synchronous main-thread work before ELK / React Flow ran — long
enough for the page to register as unresponsive.
Fix: precompute two indexes once when a graph is loaded.
- `nodesById: Map<string, GraphNode>`
- `nodeIdToLayerId: Map<string, string>` (first layer wins, matching
prior `findNodeLayer` semantics)
Both live in `useDashboardStore` and are rebuilt by `setGraph`. The
three call sites:
1. `useOverviewGraph` (GraphView.tsx) — per-layer complexity aggregation
moved into a new `computeLayerStats(layer, nodesById)` helper that
walks `layer.nodeIds` instead of filtering all `graph.nodes`. Search
match counts now read straight from `nodeIdToLayerId` instead of
rebuilding a layer index on every searchResults change.
2. `filterNodes` (utils/filters.ts) — takes `nodeIdToLayerId` instead of
`Layer[]`; the layer-membership check is one Map.get() per node.
Updated `ExportMenu.tsx` caller to pass the store-level index.
3. `findNodeLayer` (store.ts) — replaced with `nodeIdToLayerId.get()` at
the four call sites. `navigateTourToLayer` helper updated to take the
index rather than the whole graph.
Behavior is preserved exactly:
- "First layer wins" semantics for nodes that appear in multiple
layers (#102 schema doesn't forbid this).
- 30 % aggregate-complexity threshold pinned by tests.
- Layer filter that excludes layer-less orphans, but ungated when
no layers are selected.
Verified locally:
Bench (`scripts/benchmark-aggregations.mjs`, node 22):
100 layers × 200 nodes (#102 shape): 475 ms → 2 ms (232× faster)
50 layers × 200 nodes: 116 ms → 0.6 ms (190× faster)
30 layers × 100 nodes: 12 ms → 0.2 ms (63× faster)
Tests: `pnpm --filter @understand-anything/dashboard test`
24 → 41 pass (+17 new tests across `layerStats.test.ts` and
`filters.test.ts`, including a #102 perf-regression guard at
100 layers × 100 nodes < 50 ms).
`pnpm --filter @understand-anything/core test` — 654 / 654 pass.
`pnpm --filter @understand-anything/dashboard exec tsc -b` — clean.
`pnpm --filter @understand-anything/dashboard build` — clean.
Pre-existing on master and not from this branch: `pnpm lint` errors
out with "eslint: command not found" — `eslint` isn't installed by any
package and the root `lint` script is bare `eslint .`. Out of scope here.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
P1: Anchor edges at the collapsed-side container atom when only one
side of an aggregated edge is expanded. The previous pass always
emitted file→file endpoints, but a collapsed container's children
aren't rendered as nodes — React Flow silently dropped those edges.
Now the collapsed endpoint stays as the container id, and the
expanded endpoint becomes the real file id; multiple files
collapsing onto the same (container → file) pair are deduped.
P2: Pending-fit ref + post-render trigger replaces the fixed 50ms
timer for layer-transition fitView. ELK takes ~125ms+ on medium
layers, so the timer was firing before positions arrived and the
viewport stayed on the old layer. Now we mark a pending fit on
navigation change and execute it once `nodes` populate, then
requestAnimationFrame so React Flow has placed the nodes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Stage 1 stashes intra-folder file→file edges on `topo.intraContainer`
but `expandedEdges` only walked `topo.edges` (aggregated cross-
container only). When a user expanded a container the children
appeared but their internal wiring was invisible, even though the
NodeInfo sidebar listed the connections.
Now after the inflated cross-container pass, also push every
intra-container edge whose owning container is expanded.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
setFocusNode was missed in the C1 cache-reset pass. Focus mode narrows
filteredGraphNodes to focus + 1-hop neighbors, so a container that
survives still has a subset of its children — the cache must drop
or it'll keep returning positions for filtered-out ids.
Mark applyDagreLayout @deprecated. The structural views all use ELK
now; the helper is retained for one release as a quick fallback path
and removed in the next.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
C1 (cache leak across topology changes):
Container ids derive from folder names (`container:auth`, etc.) and
collide across layers, personas, and node-type filters. Without a
reset, navigating from Layer A to Layer B left Stage 2's cache and
size memory keyed by Layer A children, so Stage 2 short-circuited
and Layer B's children silently disappeared.
Reset containerLayoutCache, containerSizeMemory, and
expandedContainers in:
- drillIntoLayer (most visible — layer drill)
- navigateToOverview (drilling out)
- toggleNodeTypeFilter (filter change shifts container.nodeIds)
- setPersona (persona filters node types)
setGraph already resets all three on full graph reload — unchanged.
C2 (bundle regression):
Add elkjs and graphology rules to vite manualChunks. The main index
chunk drops from 525KB gzipped back to 62KB, with ELK split into a
parallel-loadable 439KB chunk. Restores baseline first-paint cost.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task 14 stale TODO:
- Delete the TODO(Task 14) comment that this very task already
resolved (selection→container neighbor highlighting now flows
through isFocusedViaChild).
Task 15 visible container size:
- mergeElkPositions now propagates width/height from the ELK output
back onto the React Flow node, not just position. After a
stage1Tick-driven re-layout, the container atom resizes to match
the actual Stage 2 footprint instead of staying clipped at the
pre-expansion estimate. ELK echoes back the same width/height we
pass in for non-container nodes, so this is a no-op for them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task 13 onMove auto-expand:
- Skip when event === null so React Flow's programmatic moves
(fitView at layer entry, zoomTo from search) don't cascade-expand
every container the moment the layer paints.
- Track previous zoom; only fire when zoom actually increased. Pans
and zoom-outs are now no-ops, so a user who manually collapses a
container while zoomed in can keep it that way.
Task 12 inflated edge ids:
- Append index suffix so parallel edges with identical
(source, target, type) — which the schema doesn't disallow — don't
collide in React Flow's edge map.
Other Task 12 concerns (per-container Stage 2 cancellation, O(K·E)
edge bucket pre-pass, extent:"parent" clipping pre-Task-15) are
known trade-offs documented in the plan; deferring to follow-ups.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Stable handleContainerToggle via useCallback so ContainerNode's
memo() actually short-circuits. Reading toggleContainer through
getState() keeps Stage 1 from accidentally subscribing to
expandedContainers.
- Cap pre-expansion container size at 800x600 so layers with many
files don't render as huge empty boxes during Stage 1. Stage 2
measurement and Task 15's reflow handle the real size.
- Update stale "dagre relayout" comment now that ELK is the layout
engine; flag the Task 14 follow-up for selection→container
neighbor highlighting via inline TODO.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cross-cutting fixes for the dagre→ELK migration pattern:
- nodesToElkInput accepts an optional layoutOptionsOverride so views
can override defaults (e.g. direction) without forking the helper.
- DomainGraphView restores its original LR layout by passing
{ "elk.direction": "RIGHT" } — the previous commit silently shifted
it to TB because the helper hardcoded DOWN.
- Both overview and DomainGraphView now .catch the ELK promise so
strict-mode (DEV) failures don't surface as unhandled rejections.
- Both also console.warn returned issues until Task 16 wires the
WarningBanner funnel; auto-corrected/dropped issues no longer
silently disappear in production.
- Minor: DRY the duplicate `as unknown as Node[]` cast in GraphView.
Ranksep label-aware spacing in DomainGraphView is intentionally
deferred — see Task 14 polish or follow-up.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ContainerNode (Task 8 quality review):
- Add accessibility: role="button", tabIndex, aria-expanded,
aria-label with item count + state, focus-ring on focus, Enter/Space
keyboard activation. Containers are the primary lazy-load
interaction so keyboard support shouldn't be a regression.
- Use var(--color-diff-changed) and the font-serif Tailwind class
instead of literals so theme switching still works.
- Replace the 🔍 emoji with a typographic "N hits" pill matching the
badge pattern used by LayerClusterNode.
- Add a displayName so React DevTools shows "ContainerNode" rather
than "Memo(Anonymous)".
elk-layout.test.ts (Task 6 quality review):
- Drop the (c as { x?: number }).x casts now that ElkChild declares
x?/y? directly. Tests now exercise the typed contract.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Earlier review fix-up dropped the `as GraphEdge` cast but didn't add
the `direction` and `weight` fields, breaking `tsc -b`. Tests still
passed because vitest doesn't typecheck. Add the missing fields so
the dashboard build is green again.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Re-export NODE_WIDTH/NODE_HEIGHT from utils/layout.ts as the
fallback dimensions in elk-layout.ts. Coupling them with the dagre/
force defaults prevents silent drift during the migration.
- Declare optional x/y on ElkChild so downstream consumers (Tasks 9-12)
see the position contract in the type. Comment notes ELK sets them
on output but they're absent on input.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Length-prefix the inter-container bucket key to eliminate collision
risk when container ids contain the separator (e.g. folder names with
spaces). Adds a regression test.
- Rename AggregatedContainerEdge.types -> edgeTypes for symmetry with
the existing LayerEdgeAggregation.edgeTypes field.
- Drop the as-cast in the test helper by giving it the real EdgeType
literal type and a complete object shape.
- Reword the JSDoc to match what the function actually does (silently
drop unmapped endpoints; callers needing strict can pre-filter).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- louvain.ts: rewrite the misleading "disconnected nodes get -1" comment
(the library actually assigns unique ids; the reassignment loop is a
defensive net for future API drift).
- containers.test.ts: add tags:[] to the test node helper for shape
fidelity; replace the vacuously-true "every is community" assertion
with one that actually fails if the community fallback regresses.
- containers.ts: collapse the two-pass groupByFolder into a single
branching loop; guard Cluster A-Z naming so it doesn't wrap into
punctuation past 26 communities.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Task 2 implementation surfaced inconsistencies between the spec/plan
and the actual codebase:
- The real type exports are GraphNode / GraphEdge, not KnowledgeGraph*
- Folder grouping uses bucket count (folders + rooted) not just folder
count, with threshold < 2 instead of < 3
- Concentration threshold is > 70% (was > 60%) so 2/3 distributions
stay in folder mode
- Single-child suppression is gated by layer total ≥ 3 to avoid
emptying tiny layers
- Removed the "layer < 8 → flat" rule; the algorithm is now uniform
and small layers fall through naturally via single-child suppression
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Code review flagged that without it as a direct dep, TypeScript would
fail to resolve types like Attributes/NodeKey from transitive
graphology-types when later tasks import them.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Design doc for replacing dagre with ELK across structural views, plus
folder/community-based containers and lazy two-stage layout for the
layer-detail view to fix horizontal sprawl at 50+ nodes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR #108 ships a new user-visible feature (in-dashboard source code viewer
with Files tab + double-click navigation), so it warrants a minor bump
across all 5 version-tracked files.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Double-clicking a file in the Files tab previously only opened the source
viewer; the graph stayed wherever it was. Now also calls navigateToNode
first (drills into the layer + selects the node), then re-opens the code
viewer so the source panel stays visible.
Order matters: navigateToNodeInLayer resets codeViewerOpen, so the
openCodeViewer call has to come after.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two issues from review of #108:
1. Sidebar exclusivity regression — replacing the entire sidebar with the
CodeViewer when codeViewerOpen hid NodeInfo / LearnPanel / ProjectOverview
while reading source. Same pattern that was rejected in #50. Restore the
slide-up bottom overlay (per CLAUDE.md), keep sidebar tabs always visible,
and revert the <aside> overflow-hidden change so long content can scroll.
2. Cross-graph mismatch (Codex P1) — FileExplorer always builds its tree from
the structural graph and emits structural node IDs, but CodeViewer resolved
IDs against the domain graph in domain mode, yielding "No file selected".
Fall back to the structural graph when the active graph misses, so the
Files tab works regardless of viewMode.
Also update CLAUDE.md to describe the Files tab + modal expand behavior so
future Claude sessions don't re-introduce the slide-up regression.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>