Skip to content

Saved-query / workspace / dashboard: 3 persistence & dirty-tracking bugs (star→no tile, Save no-op after New workspace, view change not dirty) #365

Description

@BorisTyshkevich

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 causesrc/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 against state.savedQueriesnull (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.savedId is 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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinginboxFiled mid-task; not yet triaged into the roadmap

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions