Repository navigation
Saved-query / workspace / dashboard: 3 persistence & dirty-tracking bugs (star→no tile, Save no-op after New workspace, view change not dirty) #365
Description
Three related defects in the saved-query / workspace / dashboard persistence + dirty-tracking model, found during manual testing of the deployed build (github.demo, v0.6.1 (b37feb2) — the PR #364 preview). None are caused by PR #364 — all three are pre-existing and shipped in v0.6.x. Grouping them because they share the same subsystem (linked-tab spec draft, StoredWorkspaceV1 aggregate, star→tile).
Code references below are at commit b37feb2.
1. Starring a query adds no dashboard tile ("No tiles yet")
Repro: New workspace → save a query → click the star (Favorite) in the Queries panel → open Dashboard.
Actual: spec.favorite flips to true, but the dashboard shows "No tiles yet — star a query in the Queries panel to add it to the dashboard." (0 tiles), even after Refresh. The committed aggregate's dashboard stays null.
Root cause — src/dashboard/application/tile-membership.ts:68:
if (!dashboard) return null;dashboard.tiles[] is the canonical membership; starring a panel-role query is supposed to add a tile in the same atomic commit — but only if a dashboard already exists. When the committed dashboard is null, favoriting can never create one; it silently re-commits null. The design assumes boot-time legacy migration always mints a dashboard, but migrateLegacyWorkspaceIfNeeded skips migration whenever any aggregate record already exists — including one a save wrote with dashboard: null before boot migration ran (the race is acknowledged in the comment at src/state.ts:558-567). So a dashboard: null aggregate is never repaired, and the empty-state CTA is unreachable-by-design in that state.
Path: star (src/ui/saved-history.ts:136) → toggleFavorite (src/state.ts:872) → patchSavedSpec (src/state.ts:822) → toggleTileMembership (tile-membership.ts:68) → viewer builds tiles from dashboard.tiles → empty state at src/ui/dashboard.ts:733.
Introduced by: 1636237 — "fix: workspace/dashboard persistence follow-ups — star→tile (#299) … (#304)" → PR #304 (issue #299). In releases v0.6.0, v0.6.1.
Fix direction: bootstrap/mint a dashboard when favoriting against null (like buildLegacyMigrationCandidate does), or have migration/save repair a dashboard: null aggregate instead of keying the migration marker solely on record existence.
2. "Save" silently no-ops on an editor tab after "New workspace"
Repro: Have a saved query with an open editor tab bound to it → File → New workspace (clears saved queries; "open editor tabs are unaffected") → on that leftover tab click Save → fill name → Save.
Actual: the popover's Save handler fires (verified: mousedown+click both land on the enabled button) but nothing persists, the popover doesn't close, and there is no error/toast. Cancel works. A brand-new tab saves fine.
Root cause — a dangling tab.savedId read inconsistently by two paths:
savedForTab(src/state.ts:675) resolves the id againststate.savedQueries→null(query is gone) → routes to the create path (openSavePopover,src/ui/app.ts:1296), so the button reads "Save".createSavedQuery(src/state.ts:693) guards on raw truthiness:if (!tab || tab.savedId) return { ok: false, entry: null };
tab.savedIdis still set (pointing at the deleted query) → treated as already-linked → returns{ ok:false }with no diagnostics.
Popover stays open because the commit closure (src/ui/app.ts:1317) only calls close() on success, and the no-diagnostics !result.ok path returns silently:
if (!result.ok) {
if (result.diagnostics?.length) flashToast('Save failed: ...');
return; // no diagnostics → silent; close() never runs
}Upstream cause: applyCommittedWorkspace (src/ui/app.ts:1478) replaces state.savedQueries wholesale on New workspace without scrubbing open tabs' savedId — unlike deleteSaved (src/state.ts:929), which nulls savedId/resets editorMode for affected tabs. (Open-workspace / Import-replace share applyCommittedWorkspace, so they likely have the same gap.)
Introduced by: 367a817 — "feat(#287): Dashboard v1 Phase 5 — … workspace aggregate source of truth (#301)" → PR #301 (issue #287), the commit that first wired file-menu "New workspace" through commitWorkspace/applyCommittedWorkspace. (The truthiness guard itself is older — #218 — but harmless until #301 made it reachable.) In releases v0.6.0, v0.6.1.
Fix direction: reconcile open tabs in applyCommittedWorkspace the way deleteSaved does (null out savedId / reset editorMode for any tab whose savedId is no longer in workspace.queries). Optionally also make createSavedQuery's guard resolve the id (mirror savedForTab) so a dangling-linked tab still falls through to a real create, and give the silent { ok:false } a diagnostic.
3. Changing presentation chart → Table does not dirty a saved query (can't save the change)
Repro: Create a query → pick a chart (e.g. Pie) → Save → switch the result view back to Table.
Actual: the toolbar Save button stays "Saved" (inactive) — the view change can't be persisted. Asymmetric: Table→chart / chart→chart does dirty (goes through the picker), but chart→Table / chart→JSON does not.
Root cause — the Table/JSON view-switch buttons only mutate the transient resultView signal, never mark dirty — src/ui/results.ts:638:
tabs = viewSwitcherTabs(app.state.resultView.value, (id) => { app.state.resultView.value = id as 'table' | 'json' | 'panel' | 'filter'; }, false);(no tab.dirtySpec, no app.updateSaveBtn()). Contrast the "Result presentation" picker, which sets { dirty: true } + markDirty() (src/ui/panels.ts:546-559). table is deliberately excluded from that picker (src/core/result-choice.ts), so the only way to reach Table is via these non-dirtying view buttons.
The dirty predicate never considers the view anyway — src/ui/app.ts:1215:
const clean = !!entry && !tab.dirtySql && !tab.dirtySpec;Aggravating: even if a view change did set dirty, the re-save path commitSavedQuery (src/state.ts:742) / commitLinkedQuery (src/ui/app.ts:1261) only commits the spec draft and never re-reads state.resultView into spec.view (unlike createSavedQuery at src/state.ts:706). So spec.view is captured only at first save; the shown view is effectively never re-savable.
Introduced by: f415931 — "feat(saved): remember the result view and auto-run on restore" (2026-06-23, direct commit to main, no PR). This first made the result view a persisted saved-query property (spec.view) without wiring view changes into the dirty computation. Present in every release, v0.1.0 → v0.6.1.
Fix direction: in the Table/JSON view-switch handler (src/ui/results.ts:638), for a linked tab write the chosen view into the spec draft (spec.view) via patchSpecDraft(..., { dirty: true }) + app.updateSaveBtn(), mirroring the picker; and in commitSavedQuery/commitLinkedQuery capture state.resultView into spec.view the way createSavedQuery does. (Alternatively compute dirty by comparing the tab's effective spec.view against the saved entry's.)
Notes
- Verified via read-only investigation (code +
git blame/git show/git tag --contains) plus a live repro of harden(deploy): add CSP + security headers, drop external Google Fonts #1 and fix(results): vertical scroll + reliable ⌘A in the raw output pane #2 on github.demo. - harden(deploy): add CSP + security headers, drop external Google Fonts #1 and fix(results): vertical scroll + reliable ⌘A in the raw output pane #2 both stem from the
StoredWorkspaceV1aggregate can legitimately holddashboard: null/ dangling tab links; a shared "reconcile tabs + ensure a dashboard exists after any committed-workspace swap" pass would address both.
Activity
fix(#365): reconcile saved query persistence state (#367)