Dashboard app: extract stats orchestration into stats-controller module #76

New issue

Open

opened 2026-05-13 00:05:06 +02:00 by jesse · 0 comments

jesse commented

2026-05-13 00:05:06 +02:00

Owner

Background

app.js owns status polling, stats polling, view show/hide, visibility handling, tab routing, auth bootstrap, and the stats-render token bookkeeping. PR5 added a five-variable mini state machine (statsTimer, statsActiveSessionId, statsPanelVisible, statsRenderToken, pendingStatsToken) plus eight mutation sites and a requestAnimationFrame-aware lifecycle. The module-level mutability was the proximate cause of two recent polling-lifecycle bugs — auth-submit and 401 handler both failed to invalidate the in-flight stats render.

Source: PR5 multi-agent code review (Code quality & maintainability dimension)
File: crates/cognix-server/assets/app.js
Branch: phase-8/dashboard-ui-pr5-stats

Motivation

Encapsulating stats orchestration in a StatsController closure (or class) co-locates the token-cancellation invariant with the state it protects. New polling callers — a future Improve tab, a metrics overlay — would consume the controller instead of bolting more module-level mutables alongside the existing five. The recent lifecycle fixes establish the right behavior; extracting now prevents the next caller from re-introducing the same class of bug.

Why deferred from PR5

The refactor would touch every site PR5 patched for the auth-submit, 401, visibility, pageshow, and session-change handlers. Landing it in the same PR that introduced those fixes is high-blast-radius — the fixes need a release cycle of soak time against the existing structure before the structure changes underneath them.

Proposed work

Add crates/cognix-server/assets/stats-controller.js exporting createStatsController({ getMount, getSessionId, pollMs }) that owns statsTimer, pendingStatsToken, statsRenderToken, in-flight gating, and visibility/pageshow listeners.
Replace the module-level stats state in app.js with a single const statsController = createStatsController(...) and forward the five lifecycle hooks (auth-submit, 401, visibility, pageshow, session-change) to controller methods (invalidate, start, stop).
Update crates/cognix-server/src/transport/ui/tests/assets.rs Node contract test only if it asserts on the old function names; otherwise leave it asserting behavior, not structure.
Register the new file in crates/cognix-server/src/transport/ui/assets.rs.

Acceptance

app.js contains zero let stats* module-level declarations.
All E2E stats tests in e2e/dashboard/stats.spec.ts pass with no test edits.
The Node executable contract test in transport/ui/tests/assets.rs passes.
No change to user-visible polling cadence, auth dialog behavior, or chart rendering.

References

PR5 review (Concurrency §1-2, Code quality §5-6)
Lifecycle fixes in app.js from PR5: auth-submit invalidation, 401 stop+invalidate, pageshow listener, runStatsRender drop-warning

## Background `app.js` owns status polling, stats polling, view show/hide, visibility handling, tab routing, auth bootstrap, and the stats-render token bookkeeping. PR5 added a five-variable mini state machine (`statsTimer`, `statsActiveSessionId`, `statsPanelVisible`, `statsRenderToken`, `pendingStatsToken`) plus eight mutation sites and a `requestAnimationFrame`-aware lifecycle. The module-level mutability was the proximate cause of two recent polling-lifecycle bugs — auth-submit and 401 handler both failed to invalidate the in-flight stats render. **Source:** PR5 multi-agent code review (Code quality & maintainability dimension) **File:** `crates/cognix-server/assets/app.js` **Branch:** `phase-8/dashboard-ui-pr5-stats` ## Motivation Encapsulating stats orchestration in a `StatsController` closure (or class) co-locates the token-cancellation invariant with the state it protects. New polling callers — a future Improve tab, a metrics overlay — would consume the controller instead of bolting more module-level mutables alongside the existing five. The recent lifecycle fixes establish the right behavior; extracting now prevents the next caller from re-introducing the same class of bug. ## Why deferred from PR5 The refactor would touch every site PR5 patched for the auth-submit, 401, visibility, pageshow, and session-change handlers. Landing it in the same PR that introduced those fixes is high-blast-radius — the fixes need a release cycle of soak time against the existing structure before the structure changes underneath them. ## Proposed work - [ ] Add `crates/cognix-server/assets/stats-controller.js` exporting `createStatsController({ getMount, getSessionId, pollMs })` that owns `statsTimer`, `pendingStatsToken`, `statsRenderToken`, in-flight gating, and visibility/pageshow listeners. - [ ] Replace the module-level stats state in `app.js` with a single `const statsController = createStatsController(...)` and forward the five lifecycle hooks (auth-submit, 401, visibility, pageshow, session-change) to controller methods (`invalidate`, `start`, `stop`). - [ ] Update `crates/cognix-server/src/transport/ui/tests/assets.rs` Node contract test only if it asserts on the old function names; otherwise leave it asserting behavior, not structure. - [ ] Register the new file in `crates/cognix-server/src/transport/ui/assets.rs`. ## Acceptance - `app.js` contains zero `let stats*` module-level declarations. - All E2E stats tests in `e2e/dashboard/stats.spec.ts` pass with no test edits. - The Node executable contract test in `transport/ui/tests/assets.rs` passes. - No change to user-visible polling cadence, auth dialog behavior, or chart rendering. ## References - PR5 review (Concurrency §1-2, Code quality §5-6) - Lifecycle fixes in `app.js` from PR5: auth-submit invalidation, 401 stop+invalidate, pageshow listener, `runStatsRender` drop-warning

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

jesse/cognix#76

No description provided.

Rows
Columns