Skip to content

Commit a606df1

Browse files
committed
keyviz spa: round-3 follow-ups (PR #680)
Addresses Claude bot round-2 + round-3: - KeyViz.tsx: TimeAxis moved inside the overflow-auto scroll container so its absolutely-positioned labels track the canvas when the heatmap overflows horizontally. Round-2 raised this and the round-2 commit missed it; round-3 re-flagged. - KeyViz.tsx: drop the dead `maxValue === 0 ? 0 :` ternary in the render loop — the v === 0 short-circuit above guarantees we never reach that path with maxValue at zero. Add a one-line comment explaining the invariant. - Design doc 6 lens 3: replace the stale "single putImageData" claim with the fillRect-per-non-zero-cell description so the five-lens summary matches 4.3. - Design doc 4.2: drop the route-axis-label paragraph that described an alternative the implementation never adopted; note the row-detail flyout supersedes it.
1 parent ae956a2 commit a606df1

2 files changed

Lines changed: 18 additions & 6 deletions

File tree

docs/design/2026_04_27_proposed_keyviz_spa_integration.md

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -167,8 +167,11 @@ ceil(56 px / cellW))` so adjacent labels never overlap at small cell
167167
widths — at `cellW = 2 px` a naive every-tenth stride would pack
168168
~54 px of monospace label into 2 px of horizontal space.
169169

170-
Route axis labels: `bucket_id` truncated to 12 chars with a tooltip
171-
on hover. The full row data is available in the row-detail flyout.
170+
No inline labels are drawn on the route (Y) axis. At `cellH = 2 px`
171+
text would not fit, and at `cellH = 4 px` it would crowd into the
172+
heatmap. Instead, hovering over a row reveals the full `bucket_id`,
173+
key range, route count, and route IDs in a row-detail flyout below
174+
the canvas — the flyout supersedes the inline label idea.
172175

173176
### 4.3 Performance budget
174177

@@ -223,8 +226,9 @@ Per `CLAUDE.md`, recorded for completeness even on a frontend change:
223226
2. **Concurrency / distributed failures** — n/a; a single browser tab
224227
polls a single handler instance. The handler itself is already
225228
tested for concurrent observers.
226-
3. **Performance** — Phase 2 §10 budget honoured by canvas + single
227-
`putImageData`. No new dependency. Polling defaults to off.
229+
3. **Performance** — Phase 2 §10 budget honoured by canvas +
230+
`fillRect` per non-zero cell (see §4.3 for why we deliberately
231+
avoid `putImageData`). No new dependency. Polling defaults to off.
228232
4. **Data consistency** — The SPA renders whatever the handler
229233
returns; consistency guarantees come from the existing sampler
230234
(in-memory, leader-issued counters per Phase 2 design §5.1).

web/admin/src/pages/KeyViz.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,12 +122,15 @@ function Heatmap({ matrix }: HeatmapProps) {
122122
// 1024 × 500: the colour ramp runs once per cell rather than per
123123
// pixel, and zero-value cells are skipped so the only work on a
124124
// quiet matrix is the initial clearRect.
125+
// The `v === 0` short-circuit guarantees `maxValue > 0` by the
126+
// time we reach the divide, so an explicit zero-divide guard is
127+
// unreachable: every row that would trip it has already continued.
125128
for (let i = 0; i < matrix.rows.length; i++) {
126129
const row = matrix.rows[i];
127130
for (let j = 0; j < row.values.length; j++) {
128131
const v = row.values[j];
129132
if (v === 0) continue;
130-
const t = maxValue === 0 ? 0 : v / maxValue;
133+
const t = v / maxValue;
131134
const [r, g, b, a] = ramp(t);
132135
ctx.fillStyle = `rgba(${r}, ${g}, ${b}, ${a / 255})`;
133136
ctx.fillRect(j * cellW, i * cellH, cellW, cellH);
@@ -163,16 +166,21 @@ function Heatmap({ matrix }: HeatmapProps) {
163166
No tracked routes — drive some traffic and refresh.
164167
</div>
165168
) : (
169+
// TimeAxis lives inside the scroll container so its labels —
170+
// which are absolutely positioned at `idx * cellW` — track the
171+
// canvas as the user scrolls horizontally. Putting it outside
172+
// would freeze the labels under the left edge whenever the
173+
// canvas overflows.
166174
<div className="overflow-auto border border-border rounded">
167175
<canvas
168176
ref={canvasRef}
169177
onMouseMove={onMove}
170178
onMouseLeave={onLeave}
171179
style={{ display: "block", width, height }}
172180
/>
181+
<TimeAxis columnUnixMs={matrix.column_unix_ms} cellW={cellW} />
173182
</div>
174183
)}
175-
<TimeAxis columnUnixMs={matrix.column_unix_ms} cellW={cellW} />
176184
{hoverRow !== null && matrix.rows[hoverRow] && (
177185
<RowDetail row={matrix.rows[hoverRow]} index={hoverRow} />
178186
)}

0 commit comments

Comments
 (0)