Dashboard sessions: extract pure renderers from `initSessions` closure to module scope #71

New issue

Open

opened 2026-05-10 23:59:29 +02:00 by jesse · 0 comments

jesse commented

2026-05-10 23:59:29 +02:00

Owner

Background

assets/sessions.js defines initSessions(hooks) as a single ~327-line closure that wraps state (per-session sequence numbers, in-flight controllers, active session id) together with the entire render pipeline. The closure violates the project's function-size rule and makes per-helper unit testing impractical.

Source: code review of branch phase-8/dashboard-ui-pr3-sessions
File: crates/cognix-server/assets/sessions.js:44-370
Branch: phase-8/dashboard-ui-pr3-sessions

Motivation

Project rule: functions < 50 lines, files < 400 lines.
The closure contains many helpers that depend only on DOM nodes passed in — buildDetailHeader, buildDetailTabs, renderFallback, renderThoughtCards, renderPaletteItems, applySearchFilter, isTypingTarget — and not on closure state. They can be lifted to module scope without behavior change.
Module-scope helpers in the same file (splitThoughtsText, parseThoughtCard, clearChildren, isTypingTarget) already demonstrate the pattern.
Smaller helpers shrink the closure to the state-bound concerns (sequenceById, inflightById, activeSessionId, listRefreshSequence, sessionFetch, selectSession, loadSessionList, palette open/close, key handler) and open the door to direct unit tests for the renderers.

Why deferred from `phase-8/dashboard-ui-pr3-sessions`

The refactor is structural and risks churn against the PR3 review surface. Performed inside PR3 it would have overlapped with the correctness fixes for parseThoughtCard, the loadSessionList race guard, and the sessionFetch cleanup, making review noisier.
The closure's behavior is locked in by the existing static and e2e tests; the refactor is safer once those pass.

Proposed work

Move buildDetailHeader, buildDetailTabs, renderFallback, renderThoughtCards, renderPaletteItems to module scope; pass schemaFallbackText and any required DOM nodes as parameters
Move applySearchFilter, isTypingTarget, setActiveRow if their parameter lists stay reasonable (≤ 3 args)
Keep sessionFetch, selectSession, loadSessionList, palette open/close, and the global keydown handler inside initSessions because they reference closure state
Add at least one unit-level test (or DOM-level test) per extracted renderer covering the happy path and one schema-fallback edge
Confirm crates/cognix-server/src/transport/ui/assets.rs static contract tests still pass (test_session_bound_rpc_uses_session_fetch, test_dashboard_tool_contracts_for_session_calls, test_schema_check_fallback_for_session_renderers)

Acceptance

No function in assets/sessions.js exceeds 50 lines
initSessions closure body shrinks to state-bound logic only
All existing Playwright tests pass without modification
All static asset contract tests pass without modification
At least one new direct test exists for an extracted renderer

References

Branch: phase-8/dashboard-ui-pr3-sessions
Code review finding: MEDIUM — initSessions closure size and helper extraction
Project rule: .claude/rules/coding-style.md — functions < 50 lines

## Background `assets/sessions.js` defines `initSessions(hooks)` as a single ~327-line closure that wraps state (per-session sequence numbers, in-flight controllers, active session id) together with the entire render pipeline. The closure violates the project's function-size rule and makes per-helper unit testing impractical. **Source:** code review of branch `phase-8/dashboard-ui-pr3-sessions` **File:** `crates/cognix-server/assets/sessions.js:44-370` **Branch:** `phase-8/dashboard-ui-pr3-sessions` ## Motivation - Project rule: functions < 50 lines, files < 400 lines. - The closure contains many helpers that depend only on DOM nodes passed in — `buildDetailHeader`, `buildDetailTabs`, `renderFallback`, `renderThoughtCards`, `renderPaletteItems`, `applySearchFilter`, `isTypingTarget` — and not on closure state. They can be lifted to module scope without behavior change. - Module-scope helpers in the same file (`splitThoughtsText`, `parseThoughtCard`, `clearChildren`, `isTypingTarget`) already demonstrate the pattern. - Smaller helpers shrink the closure to the state-bound concerns (`sequenceById`, `inflightById`, `activeSessionId`, `listRefreshSequence`, `sessionFetch`, `selectSession`, `loadSessionList`, palette open/close, key handler) and open the door to direct unit tests for the renderers. ## Why deferred from `phase-8/dashboard-ui-pr3-sessions` - The refactor is structural and risks churn against the PR3 review surface. Performed inside PR3 it would have overlapped with the correctness fixes for `parseThoughtCard`, the `loadSessionList` race guard, and the `sessionFetch` cleanup, making review noisier. - The closure's behavior is locked in by the existing static and e2e tests; the refactor is safer once those pass. ## Proposed work - [ ] Move `buildDetailHeader`, `buildDetailTabs`, `renderFallback`, `renderThoughtCards`, `renderPaletteItems` to module scope; pass `schemaFallbackText` and any required DOM nodes as parameters - [ ] Move `applySearchFilter`, `isTypingTarget`, `setActiveRow` if their parameter lists stay reasonable (≤ 3 args) - [ ] Keep `sessionFetch`, `selectSession`, `loadSessionList`, palette open/close, and the global keydown handler inside `initSessions` because they reference closure state - [ ] Add at least one unit-level test (or DOM-level test) per extracted renderer covering the happy path and one schema-fallback edge - [ ] Confirm `crates/cognix-server/src/transport/ui/assets.rs` static contract tests still pass (`test_session_bound_rpc_uses_session_fetch`, `test_dashboard_tool_contracts_for_session_calls`, `test_schema_check_fallback_for_session_renderers`) ## Acceptance - No function in `assets/sessions.js` exceeds 50 lines - `initSessions` closure body shrinks to state-bound logic only - All existing Playwright tests pass without modification - All static asset contract tests pass without modification - At least one new direct test exists for an extracted renderer ## References - Branch: `phase-8/dashboard-ui-pr3-sessions` - Code review finding: MEDIUM — `initSessions` closure size and helper extraction - Project rule: `.claude/rules/coding-style.md` — functions < 50 lines

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#71

No description provided.

Rows
Columns

Dashboard sessions: extract pure renderers from initSessions closure to module scope #71