fix(admin): KeyViz heatmap honours devicePixelRatio

[bootjp]

Summary

Phase 2-B follow-up to PR #680. Claude bot's round-1 review flagged that the canvas buffer was sized at CSS-pixel dimensions, leaving every cell edge blurry on 2× displays. Fix:

• Scale the canvas buffer to physical pixels via window.devicePixelRatio, keep the CSS style at logical pixels.
• Reset the transform via ctx.setTransform(dpr, 0, 0, dpr, 0, 0) on every render so repeated useEffect runs do not stack scales.
• Clamp the DPR at 4 so a browser reporting an absurd ratio (e.g. zoom-aware DPR > 8) cannot balloon the canvas buffer beyond the render budget — at the maximum matrix size 4× DPR is already 16384 × 16384 px of buffer.

Five-lens self-review

1. Data loss — n/a; SPA-only render change.
2. Concurrency / distributed — n/a; single render path.
3. Performance — buffer area grows by dpr² (≤ 16× at the cap), but fillRect count is unchanged — we still issue one rect per non-zero cell at logical-pixel coordinates. Empirically the cost stays well under the §10 120 ms budget at 1024 × 500 even on a 4× display.
4. Data consistency — render is purely cosmetic; no data semantics change.
5. Test coverage — type check + Vite build clean. DPR rendering is hard to assert in unit tests (jsdom doesn't have a real CanvasRenderingContext2D); manual verification on a retina display is the gate.

Test plan

• tsc -b --noEmit clean
• vite build clean
• Manual: open /keyviz on a retina display; cell edges crisp instead of blurry
• Manual: switch between retina and external 1× display in the same session; canvas re-renders correctly without scale stacking

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

Warning

Rate limit exceeded

@bootjp has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 47 minutes and 53 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b8556aac-6144-4de8-ac89-221112b0c9bd

📥 Commits

Reviewing files that changed from the base of the PR and between df5ea5e and 7a77b57.

📒 Files selected for processing (1)

• web/admin/src/pages/KeyViz.tsx

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/keyviz-dpr-retina

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[gemini-code-assist]

Code Review

This pull request introduces High DPI support for the KeyViz heatmap canvas by scaling the backing buffer based on the device pixel ratio, capped at 4. Feedback suggests using the actual ratio of physical to logical pixels in the transform to avoid sub-pixel blur on fractional DPR displays and notes that the implementation is not reactive to changes in display density or browser zoom.

[gemini-code-assist]

Using the raw dpr for the transform while using Math.floor for the buffer dimensions can lead to sub-pixel clipping or blur at the edges on displays with fractional pixel ratios (e.g., 1.25x or 1.5x). For example, if width is 101 and dpr is 1.5, the buffer becomes 151px, but a 1.5x transform will attempt to draw at 151.5px. It is more precise to use the actual ratio of physical pixels to logical pixels.

Additionally, window.devicePixelRatio is not a reactive value. If a user moves the window between displays with different pixel densities or changes the browser zoom, the canvas will not update until the next data refresh. Consider implementing a listener using window.matchMedia to trigger a re-render when the resolution changes.

Suggested change

	const dpr = Math.min(window.devicePixelRatio || 1, 4);
	canvas.width = Math.max(1, Math.floor(width * dpr));
	canvas.height = Math.max(1, Math.floor(height * dpr));
	const ctx = canvas.getContext("2d");
	if (!ctx) return;
	// setTransform (not scale) so the matrix is reset cleanly on
	// every render — repeated useEffect runs would otherwise stack
	// scales on top of the previous one.
	ctx.setTransform(dpr, 0, 0, dpr, 0, 0);
	ctx.clearRect(0, 0, width, height);
	if (matrix.rows.length === 0 || matrix.column_unix_ms.length === 0) return;
	const dpr = Math.min(window.devicePixelRatio || 1, 4);
	canvas.width = Math.max(1, Math.floor(width * dpr));
	canvas.height = Math.max(1, Math.floor(height * dpr));
	if (matrix.rows.length === 0 || matrix.column_unix_ms.length === 0) return;
	const ctx = canvas.getContext("2d");
	if (!ctx) return;
	// Use the actual ratio of physical to logical pixels for the transform.
	// This ensures that the logical dimensions map exactly to the backing
	// buffer even on fractional DPR displays, avoiding sub-pixel blur or
	// clipping at the edges. setTransform also resets the matrix so
	// repeated runs do not stack scales.
	ctx.setTransform(canvas.width / width, 0, 0, canvas.height / height, 0, 0);
	ctx.clearRect(0, 0, width, height);

[bootjp]

@claude review

Round-1 follow-ups (b10a0ae) addressing both Gemini items on PR #684:

• Fractional DPR transform — ctx.setTransform now uses canvas.width / width (and canvas.height / height) instead of the raw dpr. On 1.25× / 1.5× displays the buffer dimensions are floored but the transform was raw, so the renderer drew at sub-pixel coordinates the buffer could not represent. Using the actual ratio eliminates the residual blur.
• DPR change listener — added a matchMedia listener that bumps a dprTick state. The canvas effect now depends on it, so dragging the window between displays of different densities or zooming the browser triggers an immediate re-render rather than waiting for the next data refresh.

tsc -b --noEmit and vite build clean.

[claude]

Claude finished @bootjp's task in 1m 38s —— View job

---

Review of b10a0aed (round-1 follow-ups)

• Read KeyViz.tsx and analyze the two follow-up changes
• Review fractional DPR transform fix
• Review matchMedia DPR listener
• Post final review

---

Both Gemini items are correctly resolved. Notes per change:

1. Fractional DPR transform (KeyViz.tsx:138–152)

const dpr = Math.min(window.devicePixelRatio || 1, 4);
canvas.width  = Math.max(1, Math.floor(width  * dpr));
canvas.height = Math.max(1, Math.floor(height * dpr));
...
const sx = width  > 0 ? canvas.width  / width  : dpr;
const sy = height > 0 ? canvas.height / height : dpr;
ctx.setTransform(sx, 0, 0, sy, 0, 0);
ctx.clearRect(0, 0, width, height);

Correct. Using canvas.width / width as the scale factor (rather than raw dpr) means the transform is always derived from the floored buffer dimensions — on 1.25× it's e.g. 151/121 ≈ 1.2479… instead of 1.25, so fillRect coordinates can never land at a fractional buffer pixel.

One nit: the width > 0 guard on lines 149–150 falls back to dpr when width is zero. That path is unreachable in practice (the JSX only renders <canvas> when matrix.rows.length > 0, and a non-empty row set implies width > 0) — no bug, but you could simplify to a single expression if you want to remove dead branches:

ctx.setTransform(canvas.width / width, 0, 0, canvas.height / height, 0, 0);

Minor cosmetic; not blocking.

2. matchMedia DPR listener (KeyViz.tsx:98–108)

const [dprTick, setDprTick] = useState(0);
useEffect(() => {
  if (typeof window === "undefined" || !window.matchMedia) return undefined;
  const mq = window.matchMedia(`(resolution: ${window.devicePixelRatio}dppx)`);
  const onChange = () => setDprTick((t) => t + 1);
  mq.addEventListener("change", onChange);
  return () => mq.removeEventListener("change", onChange);
}, [dprTick]);

Correct. The chain-of-one-shot-listeners pattern is the standard approach (Andrew Huth / various React DPR hooks use the same idiom). Each time dprTick increments:

• the old listener is cleaned up;
• a new (resolution: Xdppx) query is built from the freshly updated window.devicePixelRatio;
• dprTick is in the canvas effect's dep list, so the heatmap immediately re-renders at the new DPR.

No infinite-loop risk: the listener only fires when the resolution crosses the threshold, not on arbitrary re-renders. Unmount/StrictMode double-invoke is handled cleanly by the cleanup return.

Two minor observations:

1. return undefined (line 100) — returning undefined explicitly is equivalent to returning nothing, but return; or just no early return would be more conventional. Not a bug, just style.

2. (resolution: Xdppx) exact-match semantics — this fires whenever the DPR moves away from the registered value (either direction: 2→1.5 or 2→3). That's the desired behaviour. Worth knowing: on Firefox, browser zoom changes devicePixelRatio in 0.1× steps; the listener will fire on every zoom step, which is intentional and correct here.

Overall

Both fixes are solid and directly address the Gemini feedback. The fractional-DPR transform is the higher-impact fix (eliminates sub-pixel blur on half-integer DPRs); the matchMedia listener is a nice UX improvement for multi-display users. The implementation is safe, well-commented, and the dep chains are correct.

Approve with the optional nit on the width > 0 guard.

[bootjp]

@claude review

Round-2 cosmetic follow-up (7a77b57): hoisted the empty-matrix early return ahead of setTransform. The live path now collapses to a single ctx.setTransform(canvas.width / width, 0, 0, canvas.height / height, 0, 0) with no width > 0 ternary. The empty-matrix branch resets the transform first (in case the canvas was reused) before clearing — makes the dead-state cleanup explicit.

tsc -b --noEmit and vite build clean.

[claude]

Claude finished @bootjp's task in 1m 2s —— View job

---

Review of round-2 follow-up (7a77b57)

• Read trigger comment and understand request
• Read KeyViz.tsx and analyze round-2 changes
• Post review feedback

---

The round-2 changes are correct and, notably, more than cosmetic — see below.

1. Hoisted empty-matrix guard (KeyViz.tsx:143–149)

if (matrix.rows.length === 0 || matrix.column_unix_ms.length === 0) {
  ctx.setTransform(1, 0, 0, 1, 0, 0);
  ctx.clearRect(0, 0, canvas.width, canvas.height);
  return;
}
ctx.setTransform(canvas.width / width, 0, 0, canvas.height / height, 0, 0);
ctx.clearRect(0, 0, width, height);

This is more than cosmetic. When matrix.column_unix_ms.length === 0, width = 0, so canvas.width / width = Infinity. Round-1 guarded this with a ternary fallback to dpr; round-2 makes the zero case unreachable before the division entirely, which is the cleaner fix. The explicit identity reset (1, 0, 0, 1, 0, 0) + clearRect in physical coordinates is the right idiom for a "wipe a potentially reused canvas to a known state" path.

One minor note on the empty-branch clearRect: it correctly uses (0, 0, canvas.width, canvas.height) — physical pixel coordinates under the identity transform. Contrast with the live path's (0, 0, width, height) under the scaled transform; both flush the same backing region, but having different coordinate systems in the two branches is a small readability snag for anyone who reads these side-by-side without the transform context in mind. Not a bug; just worth knowing.

2. Simplified live path

ctx.setTransform(canvas.width / width, 0, 0, canvas.height / height, 0, 0);

Clean. The ternary is gone because the division-by-zero case is now structurally excluded, not just guarded. No issues.

3. Comments

The inline comments accurately explain the transform choice and the reuse concern. Minor phrasing note: "on the off-chance the canvas was reused" slightly undersells the situation — every time data transitions from populated → empty (or on StrictMode double-invoke), the canvas really is reused and the old DPR transform really is live. "in case the canvas carries a stale DPR transform from the previous render" would be more accurate, but this is purely cosmetic.

Overall

Both structural changes (hoisted guard, removed ternary) are correct and the implementation is clean. The div-by-zero elimination is a genuine correctness improvement on top of the stated cosmetic goal.

Approve.