Skip to content

refactor(#276): extract WorkbenchSession — phase 3a#281

Merged
BorisTyshkevich merged 1 commit into
mainfrom
refactor/workbench-session-276
Jul 17, 2026
Merged

refactor(#276): extract WorkbenchSession — phase 3a#281
BorisTyshkevich merged 1 commit into
mainfrom
refactor/workbench-session-276

Conversation

@BorisTyshkevich

Copy link
Copy Markdown
Collaborator

What & why

Part of #276 — Phase 3a of the app.ts → services refactor (PRs #278 Phase 0–1, #279 Phase 2 merged). The issue's Phase 3 covers both route sessions; plan review split it into 3a (WorkbenchSession, this PR) and 3b (DashboardSession, next) — the two domains are mutually non-importing by requirement and independently revertable.

WorkbenchSession (src/ui/workbench/workbench-session.ts, 52-test spec at 100/98.7/100/100) is the route-scoped owner of the workbench running operation: run/runScript/runEntry/cancel moved verbatim (F6 wave-clock ordering, batch() discipline, finally teardown order, result.source-before-running-flip all preserved with their comments). Per issue rule 5, cancellation state now belongs to its owner: runT0/runQueryId/runTick and the in-flight AbortController are private session fields — the RunState cast and AppState.abortController are deleted.

  • The session is the sole production writer of running; shared signals stay on AppState via a narrow WorkbenchStateSlice (structurally satisfied by AppState; never the App object — rule 1).
  • DOM/render stays in the shell via injected hooks (renderResults, varGateBlocked (toast-owning), recordHistory, tickElapsed, onAuthFailed → chCtx.onSignedOut()); pure helpers are direct imports, not hooks (no god-bag). savedForTab/recordScriptHistory narrowed to Picks in state.ts (type-only).
  • The three run-coupled renderApp effects now register via session.attachShell(...) with captured disposers — idempotent on re-render, which incidentally fixes a latent double-registration on the connect → renderApp path.
  • destroy() disposes effects, clears the elapsed ticker, aborts + KILLs an in-flight operation — unit-proven per the acceptance criteria ("Both route sessions expose destroy() and tests prove cleanup"); no production caller until Phase 5's route shells (documented in the module header).
  • build/check-boundaries.mjs generalized to a rule list: src/application ⇒ no ui/editor (Phase 0 rule); ui/workbenchui/dashboard must not import each other; ui/dashboard must not import the editor (Phase 3 rules, active as the directories appear).

Call-site inventory (from plan review): actions.run/cancel plus the three additional run() callers (explainQuery/setExplainView/setResultRowLimit) delegate to the session; app.elapsedMs delegates; 4 app.test.ts tests rewritten from private-state pokes to observable behavior (fetch counts; the live query_id asserted on the KILL QUERY wire). app.ts sheds ~430 lines. dashboard.ts/results.ts/shortcuts.ts untouched.

Review: 4-angle pass — removed-behavior and cross-file angles clean (the latter re-ran the full gate itself); fixes applied: stale module header, WorkbenchResultView now derived from AppState['resultView']['value'] (no drifting hand-copy), two unnecessary test double-casts removed.

Verification: full gate green (105 files / 3,052 tests + tsc + check:arch), build green.

Acceptance criteria progress: Workbench route-scoped session with owned cancellation state ✅; destroy() with proven cleanup ✅ (workbench half); route-session import boundary enforced ✅.

Checklist

  • npm test passes (the per-file coverage gate is non-negotiable)
  • Tests added/updated in the same change as the code
  • npm run build succeeds (single-file dist/sql.html)
  • Layers kept honest: pure logic in src/core/, network in src/net/ (injected fetch), DOM in src/ui/
  • No new runtime dependency (or it's a deliberate, justified addition — see CONTRIBUTING)
  • README / CHANGELOG.md ([Unreleased]) updated if behavior or the deployed surface changed
  • Reconciled affected tracked work (roadmap Roadmap to 1.0.0 #68, the issue body, ADR/CHANGELOG) if this change reshaped it

🤖 Generated with Claude Code

src/ui/workbench/workbench-session.ts is the route-scoped owner of the
workbench running operation: run/runScript/runEntry/cancel orchestration
moved verbatim (F6 wave-clock ordering, batch() discipline, finally
teardown order, result.source-before-running-flip). The run bookkeeping
fields (runT0/runQueryId/runTick) and the in-flight AbortController are
PRIVATE session state — the RunState cast and AppState.abortController
are deleted (issue rule 5: cancellation state belongs to its owner).
The session is the sole production writer of `running`; shared signals
(resultView/resultSort/forceExplain/...) stay on AppState via a narrow
WorkbenchStateSlice (structurally satisfied by AppState — never the App
object). DOM/render stays in the shell through injected hooks
(renderResults/toast-owning varGate/recordHistory/tickElapsed/
onAuthFailed → chCtx.onSignedOut); pure helpers (tabPanel/savedForTab/
mergedSourceArgs/panelExecution/...) are direct imports, not hooks.
savedForTab/recordScriptHistory in state.ts narrowed to Picks (type-only).

The three run-coupled renderApp effects now register through
session.attachShell(...) with captured disposers — idempotent on
re-render, which also fixes a latent double-registration on the
connect → renderApp path. destroy() disposes effects, clears the
elapsed ticker, aborts + KILLs an in-flight operation; unit-proven
(no production caller until Phase 5's route shells — documented).

Split from the issue's Phase 3 by plan review: 3a = workbench (this),
3b = DashboardSession (next PR). build/check-boundaries.mjs generalized
to a RULES list and now also enforces: ui/workbench ↔ ui/dashboard must
not import each other; ui/dashboard must not import the editor.

actions.run/cancel plus explainQuery/setExplainView/setResultRowLimit
delegate to the session; app.elapsedMs delegates; 4 app.test.ts tests
rewritten from private-state pokes to observable behavior (fetch counts,
live query_id on the KILL wire). app.ts sheds ~430 lines.

Gate: 105 files / 3052 tests green; build green; check:arch green
(3 files / 2 active rules). New session spec: 52 tests, file at
100/98.7/100/100.

Part of #276 (phase 3a; roadmap #68 updated).

Claude-Session: ad39bf19-1690-43c7-abb4-f0084fca00a2

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@BorisTyshkevich
BorisTyshkevich merged commit 3379516 into main Jul 17, 2026
9 checks passed
BorisTyshkevich added a commit that referenced this pull request Jul 17, 2026
src/ui/dashboard/dashboard-session.ts is the route-scoped owner of the
dashboard execution runtime: the 6-way tile pool, wave generations
(reserved at wave CREATION, not worker start — the race-closing
invariant), per-slot AbortControllers, runFavoriteSource's
gate/stream/classify core, filter waves + curated-field merge +
persistence ports, runAffected's cascade, and retryFilter.
Constructible without App (issue rule 1); input is an explicit
DashboardRuntimeInput built by the shell from the favorites list, so a
stored DashboardDocument can replace that source later without touching
the session. Every DOM write stays in the shell behind injected hooks;
slots keep their route's own DOM nodes for now (session-state/shell-DOM
split deferred to Phase 5, documented). Tile supersede remains
client-abort only — never a server KILL (intentional, preserved).

renderDashboard(app) keeps its signature as the integration entry —
tests/unit/dashboard.test.ts passed UNMODIFIED, the strongest
behavior-preservation signal this extraction has.

destroy() aborts all in-flight tile/KPI/filter work, tears down chart
instances, disposes the filter bar, and flips a destroyed flag that
turns runAll/runAffected/retryFilter into no-ops (re-checked after the
token-preflight await too) — so an orphaned filter-bar debounce timer
firing post-teardown can never issue a request or trigger a sign-out.
buildFilterBar now returns { el, dispose() }; the dashboard shell and
the detached Data view both dispose the previous bar on re-render,
closing today's orphan-timer gap. Review round also added a
stale-generation guard on streamed progress: a superseded request's
last buffered chunk can no longer write into the newer wave's live
loading label (the pre-refactor closure-captured label was immune;
the guard restores that).

Gate: 106 files / 3090 tests green (dashboard suite untouched), build
green, check:arch green (ui/dashboard rules active: no ui/workbench,
no editor imports). e2e chromium+webkit fully green; firefox 40/40
serialized (parallel-run flake is the documented environment issue).
New session spec: 36 tests; dashboard-session.ts 100/94.9/100/100,
dashboard.ts 100/97.9/97.4/100, filter-bar.ts 100/100/100/100.

Part of #276 (phase 3b — completes Phase 3 with #281; roadmap #68
updated).

Claude-Session: ad39bf19-1690-43c7-abb4-f0084fca00a2

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant