Polish treemap view in vortex explorer

[robert3005]

Polish treemap view in vortex explorer

[codspeed-hq]

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚡ 4 improved benchmarks
❌ 2 regressed benchmarks
✅ 1589 untouched benchmarks
⏩ 4 skipped benchmarks¹

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
❌	Simulation	chunked_varbinview_into_canonical[(1000, 10)]	168.9 µs	205.5 µs	-17.81%
❌	Simulation	slice_empty_vortex	339.4 ns	397.8 ns	-14.66%
⚡	Simulation	chunked_bool_canonical_into[(1000, 10)]	26.9 µs	16.7 µs	+60.92%
⚡	Simulation	chunked_varbinview_canonical_into[(100, 100)]	259.5 µs	224.4 µs	+15.64%
⚡	Simulation	chunked_varbinview_into_canonical[(100, 100)]	306.8 µs	271.5 µs	+13.01%
⚡	Simulation	bitwise_not_vortex_buffer_mut[128]	273.6 ns	244.4 ns	+11.93%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing claude/stoic-brown-5pzm5s (28f77da) with develop (00c39e8)}

Footnotes

1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

[0ax1]

No preview deploy?

[0ax1]

codex:

• Findings

  - Medium: vortex-web/src/components/explorer/BlockTreemap.tsx:158 leaves selfSelected.current stale after a treemap-originated selection. After clicking tile A, selecting another node externally, then selecting A externally again, the effect treats
    A as self-originated and refuses to re-root the treemap. The marker set at vortex-web/src/components/explorer/BlockTreemap.tsx:268 should be one-shot: clear it when it matches, and also clear it before handling a non-matching external selection.

[0ax1]

@claude in depth review

[github-actions]

Claude finished @0ax1's task in 2m 25s —— View job

---

In-depth review — Polish treemap view

Reviewed the full diff (9 files, +673/−350). This is a clean refactor: the old detail/TreemapPane.{tsx,stories} are replaced by explorer/BlockTreemap.tsx + a dedicated physicalStats.ts, with new Storybook stories and a theme decorator. No dangling references to the deleted files remain, and expandArrayTree is properly memoized (App.tsx:79), so the re-root effect won't fire spuriously. Nice work overall — below are the findings.

---

🔴 Correctness — confirming the codex finding (the one real bug)

selfSelected.current is never cleared, so it goes stale — BlockTreemap.tsx:158-165, marker set at :268, :283, :297.

The ref is meant to distinguish "the map selected this" from "something external selected this," so only external selections re-root the zoom. But it's a write-once latch that's never reset. Trace:

1. Single-click tile A → selfSelected.current = 'A', onSelectNode('A'). Effect sees 'A' === selfSelected.current → returns early (correct, no re-root).
2. Select B from the tree panel → 'B' !== 'A' → setDrillId('B') (correct). But selfSelected.current is still 'A'.
3. Select A again from the tree panel → 'A' === selfSelected.current → returns early. Bug: this was an external selection and should re-root to A, but it's treated as self-originated.

The marker needs to be one-shot: clear it the moment it's consumed. The minimal fix is in the effect:

useEffect(() => {
  if (!selectedNodeId) return;
  if (selectedNodeId === selfSelected.current) {
    selfSelected.current = null; // consume the one-shot marker
    return;
  }
  selfSelected.current = null;   // external selection: don't let a stale marker linger
  setDrillId(selectedNodeId);
  const node = findNodeById(root, selectedNodeId);
  if (node && isFlatLayout(node) && !node.children.some((c) => c.isArrayNode)) {
    onExpand?.(node.id);
  }
}, [selectedNodeId, root, onExpand]);

I checked the interaction with the async expandArrayTree re-root path: clearing on match is safe because expandIfFlat/the effect's onExpand call is already guarded by !node.children.some(c => c.isArrayNode), so a redundant effect run after root changes won't double-expand. Fix this →

---

🟡 Minor / nits (non-blocking)

1. Duplicated arraySubtreeBytes — defined identically in BlockTreemap.tsx:54-60 and physicalStats.ts:32-38. Worth exporting one from physicalStats.ts and importing it, so the two can't drift.

2. Tooltip rebuilds the segment map on every mouse move — TileTooltip calls nodePhysicalStats(node, segments, fileSize) on each render (BlockTreemap.tsx:397), and nodePhysicalStats does new Map(segments.map(...)) plus collectSubtreeSegments (physicalStats.ts:76-81). Since handleMouseMove updates tooltip.{x,y} on every move, this recomputes for every pixel of motion. BlockTreemap already has a memoized segmentMap; consider passing it in (or memoizing the stats by node.id) to avoid the per-move O(segments) rebuild. Likely negligible for small files, but the HeavyChunks story (500 chunks) is exactly the case where it adds up.

3. % of file includes metadata, block area does not — block sizing uses data-only subtreeBytes (:63-69), while the tooltip's fractionOfFile uses totalBytes = dataBytes + metadataBytes (physicalStats.ts:86-93). A tile's rendered area and its % of file figure can therefore disagree slightly. Probably intentional, but a one-line comment noting the distinction would save a future reader the confusion.

4. Same stale-marker pattern at the other two write sites — the double-click (:283) and zoom-out button (:297) also set selfSelected.current without ever clearing it. The one-shot fix above resolves all three, but worth confirming the zoom-out case behaves as intended once the marker is cleared on consume.

✅ Things that look good

• Header-band layout trick (paddingTop after paddingOuter, then the y -= HEADER shift) is well commented and explains the non-obvious ordering requirement.
• hitTest correctly prefers the deepest tile and lets container header bands remain clickable.
• Story coverage (Root, Field, HeavyChunks) plus the theme decorator in .storybook/preview.ts is a good addition.
• git status clean, no leftover imports, deleted files fully removed.

Re: "No preview deploy?"

That's outside the diff — this PR doesn't touch CI/preview-deploy config, so I can't tell from the changed files whether a preview should have been produced. If the preview workflow is expected for vortex-web changes, that's a CI/config question for the repo's deploy setup rather than something in this PR.

I did not run npm/Storybook build or lint locally (this is a read-only review). Recommend running npm run lint / tsc and a quick Storybook smoke-check of the three stories before merge.
• branch claude/stoic-brown-5pzm5s

[robert3005]

Addressed the review in 5e5d5f9:

• 🔴 stale selfSelected latch — it's now a one-shot marker, cleared the moment it's consumed (both on the self-match branch and before handling an external selection). Re-selecting a previously map-selected node from the tree panel now re-roots the zoom instead of being suppressed.
• Duplicated arraySubtreeBytes — exported from physicalStats.ts and imported in BlockTreemap; the copy is gone.
• Per-mousemove stats recompute — nodePhysicalStats now takes the prebuilt segmentMap, and TileTooltip memoizes its stats by node, so they aren't recomputed on every pointer move (matters for the HeavyChunks story).
• Area vs % of file — added a comment on subtreeBytes noting tile area is data-only bytes while the tooltip's % of file includes metadata, so the two can differ slightly.

Ran eslint + tsc (clean) and a Storybook build of the three stories.

---

Generated by Claude Code