refactor(ui): batch CommandDeckAdapter.render() into a requestAnimationFrame pass #56

New issue

Open

opened 2026-09-22 16:31:13 +00:00 by gabogg · 0 comments

gabogg commented

2026-09-22 16:31:13 +00:00

Owner

Spun out of #14 Candidate 3 during the grilling session of 2026-09-22. Candidate 3 is otherwise delivered — this is the single item from it that did not ship.

Problem

CommandDeckAdapter.render(snapshot) (app/static/js/src/ui/command_deck_adapter.js:215) writes innerHTML synchronously across 17 private renderers (renderHud :235, renderPortalMatrix :504, renderFluxStream :1310, …). #14's Candidate 3 specified coordinating them "in an atomic requestAnimationFrame batch"; the deep mount/render interface shipped, the rAF batching did not.

Why it was deferred

The test cost is substantial and the benefit is currently theoretical — no jitter has been reported.

All 22 tests in tests/frontend/test_command_deck_adapter.test.js (1197 lines) call the private renderers directly — adapter.renderHud(snapshot), renderPortalMatrix, renderOpenLongest, renderLiveActivityStream, renderStatusStrip, renderMeasurementsBar, renderCountingCameras, renderFluxStream — so those methods are de facto public test API. They assert on raw innerHTML substrings (e.g. :57, html.includes('WS: LIVE')) against a hand-rolled createMockElement() stub returning {innerHTML, textContent, children} (:12-18). There is no DOM library.

Any move to rAF batching, or to DOM-node construction instead of innerHTML strings, breaks essentially every assertion.

Collision risk is also high: command_deck_adapter.js took +198 lines in 7ba208d (#37/#24/#25) and further changes in e7bf76b (#42), bbc435a (#44) and 3368f4f, all dated 2026-09-21.

Prerequisite

Decide the test strategy first — a real DOM shim, snapshot fixtures, or keeping innerHTML so the current assertions survive. That decision, not the rAF change itself, is the bulk of the work.

Acceptance criteria

Test strategy decided and recorded before implementation.
render() coalesces its writes into a single rAF pass; no partial frame is observable.
Frontend suite green (currently 59 node --test tests, 22 of them in the deck suite).

🤖 Generated with Claude Code

Triage resolution — 2026-09-23

Close as wontfix for now. The issue reports no observed rendering jitter;
the prior architectural intention alone does not justify this change.

Reopen when a reproducible rendering performance problem is accompanied by
measurements. At that point, choose the smallest effective change and test
strategy. Scheduling the public render() method does not inherently require
replacing every private-renderer innerHTML assertion or adopting a DOM library.

No rendering implementation or test-harness migration is requested by this
resolution.

Spun out of #14 Candidate 3 during the grilling session of 2026-09-22. Candidate 3 is otherwise **delivered** — this is the single item from it that did not ship. ## Problem `CommandDeckAdapter.render(snapshot)` (`app/static/js/src/ui/command_deck_adapter.js:215`) writes `innerHTML` **synchronously** across 17 private renderers (`renderHud` `:235`, `renderPortalMatrix` `:504`, `renderFluxStream` `:1310`, …). #14's Candidate 3 specified coordinating them "in an atomic `requestAnimationFrame` batch"; the deep `mount`/`render` interface shipped, the rAF batching did not. ## Why it was deferred The test cost is substantial and the benefit is currently theoretical — no jitter has been reported. All **22** tests in `tests/frontend/test_command_deck_adapter.test.js` (1197 lines) call the private renderers **directly** — `adapter.renderHud(snapshot)`, `renderPortalMatrix`, `renderOpenLongest`, `renderLiveActivityStream`, `renderStatusStrip`, `renderMeasurementsBar`, `renderCountingCameras`, `renderFluxStream` — so those methods are de facto public test API. They assert on **raw `innerHTML` substrings** (e.g. `:57`, `html.includes('WS: LIVE')`) against a hand-rolled `createMockElement()` stub returning `{innerHTML, textContent, children}` (`:12-18`). There is no DOM library. Any move to rAF batching, or to DOM-node construction instead of `innerHTML` strings, breaks essentially every assertion. Collision risk is also high: `command_deck_adapter.js` took +198 lines in `7ba208d` (#37/#24/#25) and further changes in `e7bf76b` (#42), `bbc435a` (#44) and `3368f4f`, all dated 2026-09-21. ## Prerequisite Decide the test strategy **first** — a real DOM shim, snapshot fixtures, or keeping `innerHTML` so the current assertions survive. That decision, not the rAF change itself, is the bulk of the work. ## Acceptance criteria - [ ] Test strategy decided and recorded before implementation. - [ ] `render()` coalesces its writes into a single rAF pass; no partial frame is observable. - [ ] Frontend suite green (currently 59 `node --test` tests, 22 of them in the deck suite). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- ### Triage resolution — 2026-09-23 Close as **wontfix for now**. The issue reports no observed rendering jitter; the prior architectural intention alone does not justify this change. Reopen when a reproducible rendering performance problem is accompanied by measurements. At that point, choose the smallest effective change and test strategy. Scheduling the public `render()` method does not inherently require replacing every private-renderer `innerHTML` assertion or adopting a DOM library. No rendering implementation or test-harness migration is requested by this resolution.

gabogg added the

needs-triage

label

2026-09-22 16:32:04 +00:00

gabogg

2026-09-23 14:30:17 +00:00