Skip to content

Commit 5e5d5f9

Browse files
committed
fix(vortex-web): address treemap review — one-shot re-zoom marker + nits
- selfSelected is now a one-shot marker: cleared the moment it's consumed, so re-selecting a previously map-selected node from the tree panel re-roots the zoom instead of being suppressed by a stale latch. - Deduplicate arraySubtreeBytes (export it from physicalStats, drop the copy in BlockTreemap). - nodePhysicalStats now takes the prebuilt segmentMap, and the tooltip memoizes its stats by node, so they're not recomputed on every mouse move. - Document that tile area is data-only bytes while the tooltip's "% of file" includes metadata, so the two can differ slightly. Signed-off-by: Robert <robert@spiraldb.com>
1 parent 68ca1f7 commit 5e5d5f9

2 files changed

Lines changed: 31 additions & 20 deletions

File tree

vortex-web/src/components/explorer/BlockTreemap.tsx

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,12 @@ import {
1717
DTYPE_COLORS,
1818
} from '../swimlane/utils';
1919
import { useTheme } from '../../contexts/ThemeContext';
20-
import { nodePhysicalStats, formatPercent, formatBytesPerRow } from './physicalStats';
20+
import {
21+
nodePhysicalStats,
22+
arraySubtreeBytes,
23+
formatPercent,
24+
formatBytesPerRow,
25+
} from './physicalStats';
2126

2227
interface BlockTreemapProps {
2328
/** The full layout tree the map can explore. */
@@ -51,15 +56,11 @@ type RectNode = HierarchyRectangularNode<TreeNode>;
5156
const HEADER = 16;
5257
const PAD = 2;
5358

54-
function arraySubtreeBytes(node: LayoutTreeNode): number {
55-
const own = (node.bufferLengths ?? []).reduce((s, b) => s + b, 0);
56-
const childBytes = node.children
57-
.filter((c) => c.isArrayNode)
58-
.reduce((s, c) => s + arraySubtreeBytes(c), 0);
59-
return own + childBytes;
60-
}
61-
62-
/** Total encoded bytes for a node's whole subtree. */
59+
/**
60+
* Total encoded *data* bytes for a node's whole subtree (no metadata) — this is
61+
* what tile areas are proportional to. Note the tooltip's "% of file" uses
62+
* total bytes (data + metadata), so a tile's area and its % can differ slightly.
63+
*/
6364
function subtreeBytes(node: LayoutTreeNode, segmentMap: Map<number, SegmentMapEntry>): number {
6465
if (node.isArrayNode) return arraySubtreeBytes(node);
6566
return collectSubtreeSegments(node).reduce(
@@ -154,9 +155,16 @@ export function BlockTreemap({
154155
useEffect(() => setDrillId(root.id), [root.id]);
155156

156157
// An external selection (e.g. clicking the tree panel) drives the zoom: re-root
157-
// the map at the selected node. Selections the map made itself are ignored.
158+
// the map at the selected node. Selections the map made itself are ignored via
159+
// a one-shot marker that is cleared as soon as it is consumed — otherwise a
160+
// stale marker would suppress a later external re-selection of the same node.
158161
useEffect(() => {
159-
if (!selectedNodeId || selectedNodeId === selfSelected.current) return;
162+
if (!selectedNodeId) return;
163+
if (selectedNodeId === selfSelected.current) {
164+
selfSelected.current = null;
165+
return;
166+
}
167+
selfSelected.current = null;
160168
setDrillId(selectedNodeId);
161169
const node = findNodeById(root, selectedNodeId);
162170
if (node && isFlatLayout(node) && !node.children.some((c) => c.isArrayNode)) {
@@ -374,7 +382,7 @@ export function BlockTreemap({
374382
</svg>
375383
)}
376384

377-
{tooltip && <TileTooltip tooltip={tooltip} segments={segments} fileSize={fileSize} />}
385+
{tooltip && <TileTooltip tooltip={tooltip} segmentMap={segmentMap} fileSize={fileSize} />}
378386
</div>
379387
);
380388
}
@@ -386,15 +394,19 @@ function truncate(s: string, maxChars: number): string {
386394

387395
function TileTooltip({
388396
tooltip,
389-
segments,
397+
segmentMap,
390398
fileSize,
391399
}: {
392400
tooltip: Tooltip;
393-
segments: SegmentMapEntry[];
401+
segmentMap: Map<number, SegmentMapEntry>;
394402
fileSize: number;
395403
}) {
396404
const { node, x, y } = tooltip;
397-
const stats = nodePhysicalStats(node, segments, fileSize);
405+
// Keyed on the node so it isn't recomputed for every pixel of mouse movement.
406+
const stats = useMemo(
407+
() => nodePhysicalStats(node, segmentMap, fileSize),
408+
[node, segmentMap, fileSize],
409+
);
398410
const dtypeCat = getDtypeCategory(node.dtype);
399411
const dtypeColor = DTYPE_COLORS[dtypeCat];
400412
return (

vortex-web/src/components/explorer/physicalStats.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ export interface PhysicalStats {
2929
}
3030

3131
/** Total buffer bytes for an array-encoding node subtree. */
32-
function arraySubtreeBytes(node: LayoutTreeNode): number {
32+
export function arraySubtreeBytes(node: LayoutTreeNode): number {
3333
const own = (node.bufferLengths ?? []).reduce((sum, b) => sum + b, 0);
3434
const childBytes = node.children
3535
.filter((c) => c.isArrayNode)
@@ -54,12 +54,12 @@ function arraySubtreeBufferCount(node: LayoutTreeNode): number {
5454
* Compute the physical statistics for a node, aggregated over its subtree.
5555
*
5656
* @param node the layout or array node to describe
57-
* @param segments the file's segment map (used to resolve layout byte sizes)
57+
* @param segmentMap segment index → entry, used to resolve layout byte sizes
5858
* @param fileSize total file size in bytes, used for the percentage-of-file metric
5959
*/
6060
export function nodePhysicalStats(
6161
node: LayoutTreeNode,
62-
segments: SegmentMapEntry[],
62+
segmentMap: Map<number, SegmentMapEntry>,
6363
fileSize: number,
6464
): PhysicalStats {
6565
const isArray = node.isArrayNode ?? false;
@@ -73,7 +73,6 @@ export function nodePhysicalStats(
7373
bufferCount = arraySubtreeBufferCount(node);
7474
segmentCount = 0;
7575
} else {
76-
const segmentMap = new Map(segments.map((s) => [s.index, s]));
7776
const ids = new Set(collectSubtreeSegments(node));
7877
dataBytes = 0;
7978
for (const id of ids) {

0 commit comments

Comments
 (0)