refactor(#478): caller-neutral VariableBar activation port - #555
Merged
Conversation
…tral names `VariableBarApp` is shared by Dashboard and detached Data, but its activation port was named after Workbench persistence (state.filterActive, params.saveFilterActive), leaking one caller's storage model into a shared UI contract. Rename the shared port to state.activeByName / params.saveActive; every persisted name and storage key (AppState.filterActive, WorkbenchParameterSession.saveFilterActive, effectiveFilterActive, asb:filterActive) is unchanged, with no storage migration. Both callers now build an explicit adapter instead of a same-shape cast: Dashboard aliases its local in-memory draft map with a no-op saveActive; detached Data aliases the real AppState.filterActive object and routes saveActive to app.params.saveFilterActive(). A new results.test.ts case proves the alias by identity (app.state.filterActive === activeRef after a bar-driven edit), not just behavior. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GiubaoqEuBzAyo5C4P8Vqr
Review nit: the comment still pointed at the `as VariableBarApp` cast this change removed. Describe the explicit adapter that replaced it instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GiubaoqEuBzAyo5C4P8Vqr
7 tasks
lesandie
pushed a commit
to lesandie/altinity-sql-browser
that referenced
this pull request
Aug 2, 2026
… read, not a snapshot The VariableBarApp adapters built by results.ts (detached Data, Altinity#555) and dashboard.ts (pre-Altinity#555) copied varRecent into state as a plain data property, captured once at adapter-construction time. Unlike activeByName, varRecent is replaced wholesale on every record/clear (workbench-parameter-session.ts), so the copy went stale immediately: Clear recent persisted but an open dropdown kept listing cleared values, and a re-run's newly recorded value never showed until the pane reopened. VariableBarApp now exposes getVarRecent(): RecentMap, a live-read callback both adapters implement over their own varRecent reference, read at call time. Updated the single consumer in variable-bar.ts, both production adapters, the port's doc comment, and every other port construction (variable-bar.test.ts's asBarApp, tests/e2e/time-range.html). Added behavioural regression tests (wholesale-replacement-then-reopen, and Clear-recent through a production-like clearVarRecent) in variable-bar.test.ts, results.test.ts, and dashboard.test.ts — each verified to fail against the reverted snapshot form before being restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GiubaoqEuBzAyo5C4P8Vqr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Closes #478.
VariableBarAppis shared by Dashboard and detached Data, but its activation port was named after Workbench persistence (state.filterActive,params.saveFilterActive) — leaking one caller's storage model into a shared UI contract. Dashboard uses those members for an in-memory draft map with no persistence at all.Renames only the shared port to caller-neutral names —
state.activeByName,params.saveActive— and gives both callers explicit adapters:src/ui/dashboard.ts) aliases its localdraftActivemap;saveActiveis a no-op because the draft is never persisted.src/ui/results.ts) builds an explicit adapter aliasingapp.state.filterActiveand routingsaveActivetoapp.params.saveFilterActive(), replacing the previousapp as VariableBarAppsame-shape cast.Every persisted name and storage key is unchanged —
AppState.filterActive,WorkbenchParameterSession.saveFilterActive,effectiveFilterActive,asb:filterActive. No storage migration. Behaviour is identical; this is a naming/seam refactor.The aliasing invariant is now falsifiable
The variable bar mutates the activation map in place, so an adapter that copied instead of aliased would break activation silently.
tests/unit/results.test.tsgains a test that captures the object reference beforeexpandDataPanebuilds the bar, edits a field, then assertsapp.state.filterActive === activeRef(same identity — a copy would fail), that the write landed on that real object, and thatsaveFilterActive()was called.tests/e2e/time-range.html's hand-builtvariableBarAppfixture was updated to match the renamed port — an App-shape change has to reach the e2e fixtures, not just the specs.Checklist
npm testpasses (the per-file coverage gate is non-negotiable)npm run buildsucceeds (single-filedist/sql.html)src/core/, network insrc/net/(injected fetch), DOM insrc/ui/CHANGELOG.md([Unreleased]) updated if behavior or the deployed surface changed🤖 Generated with Claude Code
https://claude.ai/code/session_01GiubaoqEuBzAyo5C4P8Vqr