Dashboard tree management: direct Dashboard and Panel actions #429

Activity

1. BorisTyshkevich

   added

   inboxFiled mid-task; not yet triaged into the roadmapFiled mid-task; not yet triaged into the roadmap

   enhancementNew feature or requestNew feature or request

   and removed

   inboxFiled mid-task; not yet triaged into the roadmapFiled mid-task; not yet triaged into the roadmap

   on Jul 24, 2026
2. BorisTyshkevich commented on Jul 26, 2026

   

   BorisTyshkevich

   on Jul 26, 2026

   CollaboratorAuthor

   More actions

   Re-scoping note from #447 phase 1 (PR #458).

   This issue asks for edit and delete actions for Dashboard panels and curated
   filters. The curated-filter half no longer has a subject:

   • there is no persisted filter to edit or delete. A Dashboard variable is inferred
     from the {name:Type} placeholders in its panels' own SQL, so it appears and
     disappears with those declarations — deleting one from the tree is explicitly a
     non-goal of Replace curated Dashboard filters with inferred Variables and batched option queries #447 ("manually creating, renaming, deleting, or changing the type of
     an active inferred variable from the tree");
   • the one delete that does exist already shipped in phase 1: an orphaned
     variable configuration (stored option SQL whose name no panel declares any more)
     has a trailing trash icon that confirms first and removes only
     variableConfigs[name];
   • editing a variable also already shipped — clicking its tree row opens an SQL
     editor scoped to that variable, and saving blank SQL removes the configuration.

   So what remains here is the panel half: the trash action that deletes a tile
   and the dedicated saved-query copy it owns as one atomic operation. #427's
   CHANGELOG entry hands that over explicitly — removing a panel currently leaves its
   owned copy with no owner, so the copy reappears in the Library beside the original
   it was cloned from, and this issue is where that becomes a single deliberate delete.

   Worth editing the title/body to drop "and curated filters" when this is picked up.
3. BorisTyshkevich commented on Jul 27, 2026

   

   BorisTyshkevich

   on Jul 27, 2026

   Last edited by BorisTyshkevich

   CollaboratorAuthor

   More actions

   Ship log — #429

   Branch model: one PR per phase off main. Every phase in the issue's plan lands
   green and self-consistent on its own (phase 1 is two primitive corrections; 2–5 are
   additive row chrome; 6 is a decision), so no integration branch is needed. Each
   phase gets its own PR and its own human merge gate.

   Phase	Status	PR
   1 — Resolve-before-navigate and blank-title trim (#443 + #476)	shipped	#490
   2 — Dashboard-row gesture split: chevron / name / trailing (#472)	shipped	#492
   3 — Dashboard-document metadata: one create path + rename pencil (#481)	shipped	#495
   4 — Direct row actions, ⋯ removed: panel pencil + panel/Dashboard trash (#494)	shipped	#505
   5 — The Open-in-Dashboard focus button (#494's remainder)	blocked on #438	—
   6 — Remove obsolete repair planner; preserve fail-closed ownership safety (#500)	in review	#514

   Phases 4 and 5 were re-cut by #494, which supersedes the old "panel edit" /
   "panel delete" split and folds the ⋯-menu removal in with them: the row chrome,
   both commands and the focus navigation had to land as one vertical slice or the
   tree would carry two competing interaction models. What is left of #494 after
   phase 4 is the Open-in-Dashboard button alone, which #438 gates.

   The sequencing constraint that blocked phases 2–5 is cleared: PR #484 (#428,
   Library-drag assignment) merged on 2026-07-27 as deb552a, before phase 1 started.
   Phase 2 is unblocked.

   Phase 1 — handoff

   Landed (2 commits on fix/resolve-before-navigate-429p1):

   • src/ui/app.ts — openSavedQuery resolves the id against state.savedQueries
     before showQuerySurface(). A miss raises one flashToast and changes no
     surface, no route, no tab.
   • src/dashboard/application/dashboard-viewer-session.ts:561 — the authored
     tile.title is trimmed before the fallback chain, in the one place a tile's
     display title is resolved.
   • Doc comments corrected in src/ui/app.types.ts (the miss contract) and
     src/ui/dashboard.ts:2185 (ts.title is "never blank", not "never empty").
   • Tests in tests/unit/app.test.ts, tests/unit/dashboard-viewer-session.test.ts,
     tests/unit/dashboard.test.ts; CHANGELOG.md [Unreleased] → Fixed ×2.

   Decisions taken (not in the issue):

   • Miss wording is That query is no longer part of this workspace. — copied
     verbatim in shape from openDashboard's existing missing-Dashboard toast
     (app.ts:2366) rather than inventing a fourth phrasing next to it and
     file-menu.ts's two. Reuse this wording for the phase-5 delete diagnostics.
   • The caller audit the issue demands is done and recorded in the PR. Three
     production callers, none dependent on the unconditional switch:
     dashboard-tree.ts:289 (row open-query), dashboard-tree.ts:350
     (post-assignment reveal — safe because mutateWorkspace projects synchronously
     inside the awaited chain and returns ok:false if the route moved), and
     dashboard.ts:1855 (Open in Workbench — the button is null unless
     queryById.has(ts.queryId)). Phases 2, 4 and 5 add callers to this same
     primitive; they now inherit the report-and-stay-put behaviour for free.
   • app.ts:1895 had its rationale reconciled, not just its code. The Add a Databases/Dashboards switcher and dashboard hierarchy tree #426
     comment justified cancelling deferred tree clicks partly on "a deleted panel's
     deferred open would reach openSavedQuery with a dead id" — that hazard now
     lives at the callee, so the comment says so. If a later phase revisits click
     cancellation, that half of the argument is gone.
   • The "reports exactly once" test needed real machinery. flashToast reuses
     ONE .share-toast element per document (src/ui/toast.ts:35-41), so
     querySelectorAll('.share-toast').length can never exceed 1 and any
     count-the-elements assertion is unfalsifiable. The test materialises the element
     and spies on classList.add, counting how many times a report raises it —
     verified to fail against a deliberate double-report. Every later phase's
     "shows exactly one diagnostic" test must do the same; the obvious assertion
     silently passes.

   Deviations from the spec: none. Both requirement lists implemented verbatim,
   including the issue's own suggested authored || … expression.

   Deferred acceptance criteria — phase 1 claims exactly two bullets:
   openSavedQuery resolves before it navigates and reports a miss (#443); a
   whitespace-only title never reaches a heading or an accessible name (#476).
   Everything else → phases 2–6.

   The issue's Blank-titles test list also carries "the new pencils cannot author a
   blank-but-truthy title". No pencil exists yet, so that bullet is carried to
   the phases that build them: phase 3 (Dashboard title) and phase 4 (panel query
   name). Trim on write there; the read side is already settled.

   #438 / layout engines: phase 1 adds no focus-tile gesture, so nothing needed
   verifying on the flow engine. The obligation stands for phases 2, 4 and 5.

   Verification: npm test 5816 passed / 177 files, per-file gate held;
   tsc --noEmit clean; npm run build clean; npm run test:e2e 281 passed on
   chromium + webkit. Sabotage-checked — with both production changes reverted,
   exactly the four new tests fail and nothing else, so no existing test was
   depending on the old behaviour.

   Follow-ups filed: #489 (inbox) — tile.description has the identical
   untrimmed-fallback shape one line below the title
   (dashboard-viewer-session.ts:562-563); a whitespace-only description still beats
   the query description and renders an empty .dash-tile-desc. Out of #476's scope
   (it corrupts no accessible name), so not folded into this PR.

   For phase 3, not filed as an issue because phase 3 already owns it:
   src/ui/file-menu.ts reads dashboard.title untrimmed at :630, :648, :1010
   and :1044 — the last one produces a whitespace export filename — while
   src/application/dashboard-tree-model.ts:382 trims it. Phase 3 is already required
   to "apply the same rule to any title this issue's dialogs write", so fix these four
   when the rename pencil lands rather than leaving the File menu disagreeing with the
   tree.

   Phase 2 — handoff

   Landed (2 commits on feat/dashboard-row-gestures-429p2, PR #492):

   • src/application/dashboard-tree-model.ts — the Dashboard row now emits
     single: open-dashboard(view), double: null, unchanged shift, and a
     one-item menu. double === null IS the view's arbitration switch — no new
     field was added, and every later phase inherits it.
   • src/ui/dashboard-tree.ts — pressRow arbitrates only when row.double !== null
     (i.e. panel rows only); buildChevron renders a real <button>; toggleFromChevron
     moves the keyboard owner and restores focus to the chevron; syncRovingTabindex
     syncs chevrons with their row.
   • src/styles.css — .dash-tree-chev button reset + a focus ring in a channel the
     row does not use.
   • Tests: tests/unit/dashboard-tree.test.ts (new describe block for the disclosure
     control), tests/unit/dashboard-tree-model.test.ts, tests/unit/app.test.ts,
     tests/e2e/dashboard-tree.spec.js, tests/e2e/tile-open-workbench.spec.js
     (its openDashboard helper used dblclick); CHANGELOG.md → Changed.

   Decisions taken (the issue delegated these to this phase):

   • The redundant-menu-item reconcile is a SPLIT: Open in View dropped,
     Open in Edit kept. View is the row's primary press now (pointer and Enter),
     so nothing about it is hidden — which was the item's only justification. Edit
     exists only as Shift-click / Shift+Enter, both hidden modifiers, so the menu is
     where it is discoverable at all. Consequence for phase 3: the Dashboard row
     still carries exactly ONE trailing control, so the "third icon on a narrow sidebar
     row" worry is not yet triggered — phase 3 may add a pencil icon or a second row
     in this menu. The non-goal "moving or redesigning the trailing menu as a control"
     was respected: only its contents changed.
   • The chevron is roving-tabindex'd, not unconditionally tabbable. A native
     <button> per row would have added one Tab stop per chevron for the whole
     collection, breaking the ARIA tree's single-composite-tab-stop contract. It now
     carries the same tabindex as its row, so Tab walks the FOCUSED row's three
     targets and then leaves. syncRovingTabindex had to learn about it too.
   • Chevron keyboard activation re-focuses the chevron explicitly. commitUi
     repaints through replaceChildren, and the generic restore lands on the row —
     so without this, one Space expanded and the next Enter navigated. The
     chevron toggle therefore also moves the keyboard owner to its own row (as the row's
     own click already did), which is what keeps that button in the Tab order.
   • The double-click guarantee is enforced at the callee, not in the tree. The row
     dispatches the same idempotent open-dashboard(view) twice; app.openDashboard
     already collapses a repeated same-id/same-mode open into a no-op. That is now
     pinned by a REPEATED identical open pushes no second history entry in
     tests/unit/app.test.ts (sabotage-checked) instead of being assumed. The tree
     deliberately does not duplicate the guard — it cannot see the route. Phases 4/5
     add more callers of the same primitives and inherit this.
   • Three guards the split made unreachable were DELETED, not left as dead code
     (and the coverage gate would have flagged two of them anyway):
     the chevron's cancelFor, the Add Library drag assignment to Dashboard panels and variables #428 drop handler's cancelFor, and toggleRow's
     Dashboard arm (now toggleGroupRow, asserting row.group!). The reasoning, which
     a later phase must not silently invalidate: only a row with a double is ever
     arbitrated, and that now means a PANEL row — which has no chevron and accepts no
     drop. If any later phase gives a Dashboard row a double-click action, all three
     guards come back.

   Deviations from the spec: none. One reading worth recording: the global
   ## Tests → ## Regressions bullet "row click/double-click/Shift-click contract
   remains unchanged" cannot apply to Dashboard rows, since ### Dashboard row (revised)
   exists precisely to change them; it is read as scoping to the rows this issue does not
   revise, and panel/variable/group rows are genuinely untouched (their tests are
   unmodified).

   The bug that only a real browser could show — worth internalising before phases
   3–5 add more row chrome:

   Hit-testing and getBoundingClientRect both use the transformed box. The
   chevron button is 10×24 (stretched to the row height so a 10px glyph is worth
   aiming at), so the collapsed row's rotate(-90deg) turned its clickable band into
   24 wide × 10 tall — spilling 7px each side and swallowing clicks meant for the
   row icon, which then expanded instead of opening. The schema tree's chevron gets
   away with rotating its own box only because it is a 10×10 square. The rotation
   now lives on the glyph inside the control. happy-dom has no layout, so the unit
   suite was fully green throughout; an e2e case now asserts the box stays in its own
   slot and never reaches the icon, verified to fail with the rotation put back.

   Deferred acceptance criteria — phase 2 claims exactly two bullets: the three
   independent targets (#472), and action buttons do not interfere with tree navigation
   gestures for the controls that exist today (chevron, ⋯, the #447 orphan trash;
   phases 4/5 re-assert it for the pencil and panel trash). The Dashboard pencil
   itself is phase 3 — the ### Dashboard row (revised) table's fourth row and the
   Rendering and event isolation bullets about pencils are deliberately not delivered
   here.

   #438 / layout engines: phase 2 adds no tile-focus gesture (a Dashboard row
   navigates to a Dashboard, never to a member; member rows are untouched), so nothing
   needed verifying on the flow engine. The obligation stands for phases 4 and 5.

   Verification: npm test 5827 passed / 177 files, per-file gate held;
   tsc --noEmit clean; npm run build clean; npm run test:e2e 442 passed, 0
   failed on chromium + webkit + firefox. Sabotage-checked six ways (Enter/Space
   bubbling past the chevron, no focus restore, no keyboard-owner move, every chevron
   tabbable, the model split reverted, Open in View restored) — each caught by 1–6
   tests and nothing else. Also driven in the real served app against live
   ClickHouse: chevron BUTTON labelled Expand Sales revenue, Tab order row →
   chevron → ⋯, three distinct focus treatments, chevron click expands without
   touching the URL, name click routes to ?surface=dashboard&mode=view, no page errors.

   Environment note for the next phase: the dependabot Playwright bump left all
   three browser binaries stale, which presents as ~435 e2e failures at 1ms each with
   Executable doesn't exist. Run npx playwright install chromium webkit firefox.
   Firefox now launches in this sandbox — the old unshare CLONE_NEWPID EPERM is
   gone, so all three browsers are a local signal again.

   An accessibility trap phases 3–5 will hit again. A treeitem takes its
   accessible name from its CONTENTS, so every labelled control you add to a row gets
   folded into that row's own announcement. Measured with Chromium's accname engine
   after the chevron landed:

   collapsed   "Expand Sales revenue Sales revenue 2"
   expanded    "Collapse Sales revenue Sales revenue 2 Actions for Sales revenue"
   

   Fixed by giving the row an explicit aria-label from rowAccessibleName()
   (ui/dashboard-tree.ts), composed from the MODEL's strings in paint order — name,
   count, status word, meta, marker label. Phase 3's pencil and phases 4/5's
   pencil+trash need NO change to this, because the row's name no longer depends on
   its children at all; but if you add row CONTENT (not a control) that should be
   announced, add it to rowAccessibleName or it becomes invisible to a screen reader.
   Unit tests assert the exact composed name for a dashboard, group, orphan-variable and
   broken-panel row; the e2e assertion goes through getByRole, which is the only
   cross-engine way to test this (happy-dom cannot compute an accessible name).

   Review pass. An independent read-only audit against this phase's contract came
   back clean on under-delivery, scope creep, non-goal breaches and rule-softening. Its
   substantive finding was the accessible name above. Two smaller ones were taken:
   #472's test list asks for coverage across collapsed/expanded × active/inactive and the
   expanded-AND-current corner was missing, and a real dblclick (not two synthesized
   clicks) is now asserted to dispatch one repeated command.

   Follow-ups filed: none. No out-of-scope defect surfaced that phases 3–6 do not
   already own.

   Phase 3 — handoff

   Landed (1 commit on feat/dashboard-metadata-429p3, PR #495):

   • src/dashboard/application/empty-dashboard.ts — createEmptyDashboard gains
     an optional title param (default DEFAULT_DASHBOARD_TITLE, now exported).
   • src/ui/dialog-shell.ts (new) — openDialogShell/openNameDialog promoted
     out of file-menu.ts, generalized with an explicit returnFocusTo and an
     onClose hook; closeOpenDialogShell() replaces the dialogClosers
     WeakMap + app.dom.fileDialog bookkeeping with a single module-local
     "current open dialog" slot (with its own stale-vs-newer-dialog guard).
   • src/ui/file-menu.ts — repointed onto dialog-shell.ts
     (openFileDialogShell supplies returnFocusTo: app.dom.fileBtn ?? null,
     byte-identical to the old hardcoded restore); doNewDashboard uses the
     widened createEmptyDashboard signature.
   • src/ui/dashboard.ts — the empty-workspace placeholder's Create button now
     opens openNameDialog and commits through appendDashboard, dropping the
     dead resolveCompatibilityDashboard(latest).dashboard guard (the !latest
     half stays — type-required, provably dead at runtime).
   • src/workspace/workspace-dashboards.ts — renameDashboard (pure planner).
   • src/application/dashboard-title.ts (new) — commitDashboardRename, the
     mutateWorkspace wrapper, modeled 1:1 on commitVariableConfig.
   • src/application/dashboard-tree-model.ts — DashboardTreeRow.renamable
     (true for the Dashboard row only).
   • src/ui/dashboard-tree.ts — buildRenameButton + openDashboardMetadataDialog
     (the two-field title/description dialog, built directly on openDialogShell
     rather than folded into it — one consumer today); DashboardTreeApp gained
     acquireKeyboardOwner.
   • src/styles.css — .dash-tree-rename-btn (shares .dash-tree-menu-btn's
     reveal rule + its own [aria-expanded="true"] override), .fm-dialog-textarea.
   • src/ui/app.types.ts — removed the now-dead AppDom.fileDialog field.
   • Tests: tests/unit/dialog-shell.test.ts (new), dashboard-title.test.ts
     (new), workspace-dashboards.test.ts, dashboard-tree-model.test.ts,
     dashboard-tree.test.ts, dashboard.test.ts, app.test.ts,
     typography-contract.test.js (added dialog-shell.ts to the scanned
     sources); tests/e2e/dashboard-tree.spec.js; CHANGELOG.md → Added + Changed.

   Decisions taken (the issue delegated these to this phase):

   • The two-field metadata dialog is its own function, not folded into the
     promoted openNameDialog. openNameDialog stays single-field (the shape
     every create-flow caller needs); the rename dialog is built directly on the
     lower-level openDialogShell primitive and colocated in dashboard-tree.ts
     — hard rule 5's "extract a shared primitive only when a SECOND consumer
     appears" argues against forcing one shape to cover both today.
   • The pencil is a dedicated, hover/:focus-within-revealed icon button
     before the ⋯, not a new ⋯ menu item — matching the issue's own
     "Action button rendering" pencil/trash icon convention (pencil icon,
     revealed on hover/focus, Edit dashboard <name> label) even though that
     section is formally phase 5's. This keeps the Dashboard row at TWO trailing
     controls (pencil + ⋯), not three, so the "third icon on a narrow row"
     concern the issue flags never triggers here. The existing one-item ⋯ menu
     (Open in Edit) is untouched.
   • renameDashboard refuses (commits nothing) on a blank-after-trim title,
     matching renameSaved's precedent and unreachable through the dialog itself
     (whose own confirm stays disabled) — defense in depth, not a UX path, since
     the Dashboard title schema field carries no minLength floor.
   • description: undefined (the argument omitted) leaves the stored value
     untouched; an explicit string trims and clears on empty — the one
     production caller (the pencil dialog) always passes an explicit string, so
     today only the "clears on empty" branch is reachable; the omitted-leaves-
     untouched branch is unit-tested at the pure-planner layer for a future
     title-only caller (there is none yet).
   • Generalizing the dialog shell's teardown from a WeakMap keyed by
     app.dom.fileDialog to a single module-local slot is deliberate, not
     incidental: only one modal dialog is ever meaningfully open at a time, and
     the single slot lets a Dashboard-tree-opened dialog participate in the same
     "force-close on a surface transition" teardown File-menu's own dialogs
     already had — a real gap, since the tree (unlike File-menu's overlays)
     persists across Query/Dashboard surface switches and had no dialog of its
     own before this phase.

   The bug that only a real browser could show — worth internalising before
   phase 4/5's own pencil/trash controls hit the same trap:

   The pencil is display: none except on hover/:focus-within, like the ⋯
   it sits beside. Close its dialog via Escape/Cancel/Save and the pointer has
   typically moved onto the dialog's own controls by then, so neither
   :hover nor :focus-within still holds on the row — and focus() on an
   already-display: none element is a silent no-op in a real browser.
   happy-dom enforces no CSS layout at all, so the equivalent unit test passed
   regardless; only e2e caught it. Fixed the way buildMenuButton already
   solves it: the trigger's own aria-expanded stays "true" for the dialog's
   whole lifetime (set on open, kept through openDialogShell's
   returnFocusTo.focus() call, and only reset to "false" in the new
   onClose hook — by which point :focus-within covers the now-focused
   trigger independently). Any phase-4/5 control that opens a dialog from a
   hover-revealed button needs the same aria-expanded treatment, pinned by
   both a unit test (the attribute toggle) and a dedicated e2e test (the actual
   focus-restore, real-browser only).

   Deviations from the spec: none. The "Blank means blank" write-side bullet
   (carried here from phase 1's handoff, "the new pencils cannot author a
   blank-but-truthy title") is satisfied — renameDashboard trims and refuses
   blank, exactly like phase 1's read-side trim.

   Deferred acceptance criteria — phase 3 claims exactly two bullets:
   Dashboard rows can edit title/description through a pencil action; exactly
   one command creates a Dashboard, both entry points prompt and append (#481).
   Everything else → phases 4–6.

   #438 / layout engines: phase 3 adds no tile-focus gesture (the pencil
   edits Dashboard-level metadata, never a member), so nothing needed verifying
   on the flow engine. The obligation stands for phases 4 and 5.

   Verification: npm test 5877 passed / 179 files, per-file gate held;
   tsc --noEmit clean; npm run build clean; npm run test:e2e full
   dashboard-tree.spec.js green on chromium + webkit + firefox, full suite
   152/152 on chromium (webkit clean; a handful of unrelated Firefox specs
   flaked under full parallel load and passed individually on retry — known
   sandbox flakiness, none touch this phase's files). An independent read-only
   review pass against the diff found no defects across correctness, the
   dialog-shell staleness guard, coverage-gate honesty, boundary rules, and
   scope. Driven in the real served app against live ClickHouse (local
   ch-26.6): created a Dashboard via File ▸ New dashboard…, renamed it via the
   pencil (title + description), tree label updated, toast fired, no console
   errors — including moving the mouse away right after Save, to exercise the
   focus-restore fix above.

   Follow-ups filed: none. No out-of-scope defect surfaced that phases 4–6
   do not already own.

   Phase 4 — handoff

   Landed (4 commits on feat/dashboard-tree-direct-actions-494, PR #505). Two
   distinct bodies of work, deliberately in one PR because the second builds three
   more buttons and two more dialogs on the first:

   The four #495 post-merge review findings (c778cba):

   • ui/dashboard-tree.ts — isolateActivationKeys on every nested control, plus
     a guard in handleTreeKeydown that ignores an Enter originating on a
     button. Two independent layers; each is separately tested, including against a
     deliberately naive button that installs no handler of its own.
   • ui/dialog-shell.ts — openMetadataDialog (the two-field dialog, promoted out
     of dashboard-tree.ts now that the panel pencil is a second consumer) AWAITS
     its commit and keeps the card open with the typed values on every unsuccessful
     outcome; role="dialog"/aria-modal/aria-labelledby on the shell.
   • application/dashboard-create.ts (new) — the ONE creation command + a pure
     dashboardCreateMessage. Both entry points call it.

   #494 itself (1d7d1e7, a4c0ff5, f058150): the model's actions, the pure
   dashboard/application/dashboard-removal.ts, the commit wrappers
   application/dashboard-delete.ts and application/dashboard-panel-metadata.ts,
   the view's one buildActionButton, and the CSS cluster. Full file list in the PR.

   Decisions taken (not in the issue):

   • The Open-in-Dashboard focus button is NOT built. Owner decision at the start
     of the phase, recorded in Dashboard tree: replace overflow menus with direct Panel focus, edit and delete actions (#429 phase 4) #494's body: it dispatches
     focus: { kind: 'tile' }, which Fix Dashboard focus and scrolling for flow-layout KPI tile groups #438 proves is a no-op on flow-layout KPI
     tiles, and Dashboard tree: replace overflow menus with direct Panel focus, edit and delete actions (#429 phase 4) #494 forbids shipping a known no-op focus action. This is the whole
     of phase 5 now. It is not a regression — the capability shipped inside the
     ⋯ menu, and a panel row's double-click / Shift-click still do it.
   • menu, deletable and renamable collapsed into one actions list
     rather than a fourth boolean being added. Availability is answered once, in the
     model, through buildQueryOwnershipIndex — OwnershipWorkspace.queries was
     narrowed to { id } (and libraryQueries made generic) so the tree's loosened
     TreeQuery can be indexed by the SAME ownership rule instead of a private copy
     of it. Any later phase adding a row control adds a DashboardTreeAction, not
     a boolean.
   • The model composes the destructive CONFIRMATION sentence (action.confirm),
     because it names resources by their resolved display titles — the panel's AND
     its owning Dashboard's — and the view holds only the row it is painting. Same
     reason diagnostic is composed there.
   • Replace curated Dashboard filters with inferred Variables and batched option queries #447's orphan-variable trash keeps its always-visible treatment
     (.dash-tree-act-static) while the four new controls reveal on
     hover/:focus-within. Dashboard tree: replace overflow menus with direct Panel focus, edit and delete actions (#429 phase 4) #494 says "preserve" that control, and its visibility IS
     the affordance that says an orphaned configuration can be dropped.
   • A panel-metadata write reuses renameSaved rather than growing a second
     metadata writer — that is what keeps a linked Query tab reconciling (clean tabs
     adopt, dirty tabs conflict-flag). What it needed was proof of ownership at
     DEQUEUE time, so patchSavedSpec gained an optional guard(base). It is
     evaluated BEFORE the existing index < 0 test, which makes deletedExternally
     unreachable for a guarded caller — documented at the call site rather than
     handled.
   • removeDashboardDocument keeps a query that a second Dashboard also
     references (the invalid multi-owner state). Deleting it would trade one
     integrity problem for a worse one.

   The bugs only a real browser could show — internalise these before phase 5:

   A successful commit repaints the tree BEFORE the dialog closes. #495's
   review-2 fix (await the write, then close) means the trigger button captured
   when the dialog opened is detached by the time returnFocusTo is used — and
   focus() on a detached node is a silent no-op, so the keyboard lands on
   <body>. renderDashboardTree's own restore does not help: it is conditional
   on the tree ALREADY holding focus, and focus was inside the body-mounted
   dialog. returnFocusTo now accepts a RESOLVER, called at close time. Any
   phase-5 dialog opened from a row control needs the same.

   A confirmation menu autofocuses its first row. With the destructive item
   first, an Enter pressed out of momentum — the reflex right after opening any
   other menu — deletes a Dashboard. openMenu took an initialFocus option;
   confirmations pass 'last' (Cancel). Any new destructive confirmation must
   do the same.

   A custom focus ring on the action cluster collided with the chevron's. #472
   requires the row's targets to be visibly distinguishable, and the existing
   Tab-walk e2e test caught it. The controls keep the user agent's own ring.

   Deviations from the spec: one, above (the focus button). Everything else in
   #494 is implemented; its acceptance criteria are ticked in the issue body with
   the two focus bullets left open and the reason recorded there.

   Deferred acceptance criteria: the two Open-in-Dashboard bullets → phase 5,
   after #438.

   Review pass. Two independent read-only reviews (correctness, then
   accessibility/interaction). Both came back clean on scope, layer rules and
   under-delivery. Their substantive findings — the two focus bugs and the
   confirmation default above, an unavailable delete that lost its destructive
   presentation because destructive was derived from act.confirm rather than
   from the KIND, and four assertions that could not fail (the same-row cancelFor,
   the pencil/trash glyph mapping, two not.toBe('') checks that also pass on a
   missing attribute, and an e2e post-delete "focus" check that asserted row TEXT) —
   are all fixed and sabotage-checked in f058150.

   Verification: npm test 6035 passed / 184 files, per-file gate held;
   tsc --noEmit clean; npm run build clean; npm run test:e2e 484 passed, 0
   failed on chromium + webkit + firefox (--workers=1). Sabotage-checked: the
   focus-resolver fix, and the narrow-pane layout test (against a label pinned to
   flex: 0 0 auto — note that flex-shrink: 0 alone changes NOTHING there,
   because .tree-row .label is flex: 1, i.e. basis 0). Driven in the real
   served app against local ClickHouse 26.6: File-menu create → toast; Library
   drag → panel; open the Dashboard; tree pencil rename → the rendered tile heading
   AND the tree row both followed; tree trash → the tile left the rendered
   Dashboard, focus fell back to the Panels group, the Library source query was
   untouched, console clean.

   Follow-ups filed: none. The one gap either review could name that this PR
   does not close — no test on the new path for "the tile was the current Dashboard
   member" / "its query was open in the Query surface" reconciliation — holds today
   through existing machinery (main-surface.ts's retainMember,
   reconcileTabsWithSavedQueries) and was verified live, but is not pinned by a
   test of its own. Phase 5 should pin it while it is in the same code.

   Phase 6 — handoff

   In review (commit 22d27fa on
   refactor/remove-repair-planner-429p6, PR #514):

   • Deleted the unused
     src/dashboard/application/saved-query-mutation.ts planner and its dedicated
     478-line unit suite. No production caller existed.
   • Removed stale live references from the import, layout, state, tree-model and
     test comments. The historical release note that records the planner's original
     introduction remains intact.
   • Added an [Unreleased] removal note and a source/test architecture guard that
     prevents the retired API and repair vocabulary from returning unnoticed.
   • Strengthened production-path coverage in query-ownership.test.ts,
     saved-history.test.ts and state.test.ts: ownership only on
     dashboards[1] hides the query and its Library trash; zero-owner Library
     deletion preserves both Dashboards and reconciles its linked tab; deleting or
     invalidating the remotely-owned query is rejected with committed/state/tab
     truth unchanged.
   • Reconciled Dashboard tree management: direct Dashboard and Panel actions #429's phase section, tests and acceptance bullet to record the
     final remove-planner decision and retire the stale repair-dialog/navigation
     requirement.

   Decision taken: follow #500's delete, do not wire branch. #494's direct
   Panel/Dashboard trash commands are the sole cascading delete UX. Library derives
   zero-owner queries across the complete Dashboard collection, so #431's old
   "delete an owned query from Library while another Dashboard references it" path
   is no longer reachable. deleteSaved and commitSavedQuery still run strict
   whole-workspace validation and fail atomically for malformed or invalidating
   data.

   Why the old planner is not a fallback: its one repair object applied across
   all Dashboards while tile ids are Dashboard-local; matching ids could rewrite an
   unrelated Dashboard. Its generic tile-scoped suggestions were not specific to
   the requested mutation, and wiring it would duplicate #494 with a broader,
   unsafe ownership model.

   Deviations from the phase-6 decision in #500: none. The original #429 text
   still offered wire-or-delete and required navigation or a repair chooser; that
   was the stale requirement #500 explicitly superseded, and the issue body now
   records the settled decision.

   Acceptance claimed: only the phase-6 planner disposition. The
   Open-in-Dashboard focus button remains phase 5, blocked on #438; this PR uses
   Part of #429, not Closes #429.

   Review pass: two independent read-only audits. One was clean; the other
   found that the first replacement mutation tests used only the compatibility
   Dashboard and therefore would not catch a stale-current narrowing regression.
   All three state-path tests now load committed truth with an empty first
   Dashboard and ownership only on the second. Sabotage-checking
   baselineWorkspace to ignore committed latest makes exactly those three tests
   fail.

   Verification: npm test 6089 passed / 183 files, TypeScript clean and the
   per-file coverage gate held (100% statements/lines); npm run build clean;
   Chromium + WebKit e2e 341 passed / 1 skipped with --workers=1; repository
   search finds no retired planner API or repair vocabulary in source/tests.

   Follow-ups filed: none. No out-of-scope defect surfaced.
4. BorisTyshkevich

   added 3 commits that reference this issue on Jul 27, 2026

   Merge pull request #490 from Altinity/fix/resolve-before-navigate-429p1

   ...

   b556280

   fix(#429): give the tree row its own accessible name, so its buttons …

   ...

   a407e20

   Merge pull request #492 from Altinity/feat/dashboard-row-gestures-429p2

   ...

   f82680b
5. BorisTyshkevich commented on Jul 27, 2026

   

   BorisTyshkevich

   on Jul 27, 2026 – with ChatGPT Codex Connector

   Last edited by BorisTyshkevich

   CollaboratorAuthor

   More actions

   The issue body now supersedes this amendment and links #494 as the authoritative Phase 4 implementation contract.
6. BorisTyshkevich

   changed the title [-]Add edit and delete actions for dashboard panels and curated filters[/-] [+]Dashboard tree management: direct Dashboard and Panel actions[/+] on Jul 27, 2026
7. BorisTyshkevich

   added a commit that references this issue on Jul 27, 2026

   Merge pull request #495 from Altinity/feat/dashboard-metadata-429p3

   ...

   434643e
8. BorisTyshkevich commented on Jul 27, 2026

   

   BorisTyshkevich

   on Jul 27, 2026 – with ChatGPT Codex Connector

   CollaboratorAuthor

   More actions

   Phase 6 has been re-reviewed against the current ownership model and revised direct-action work in #494.

   Decision: remove the unused repair planner; do not build a repair chooser. Tracked in #500.

   Why:

   • the Library projection now excludes every query referenced by any Dashboard, across the complete collection;
   • owned-query deletion belongs to Dashboard tree: replace overflow menus with direct Panel focus, edit and delete actions (#429 phase 4) #494’s Panel/Dashboard trash commands;
   • the exact A Dashboard the UI cannot reach can make a saved query permanently undeletable #431 Library-delete path is no longer reachable through the current UI;
   • planSavedQueryMutation still has no production caller and documents an unsafe cross-Dashboard tile-ID collision in its global switch-variant repair model;
   • strict aggregate validation remains the correct fail-closed behaviour for invalidating query edits.

   #500 is the authoritative implementation issue for deleting the planner/tests, preserving validation, strengthening ownership-projection coverage, and retiring the stale repair-UI requirement from #429.
9. BorisTyshkevich

   added a commit that references this issue on Jul 28, 2026

   Merge pull request #514 from Altinity/refactor/remove-repair-planner-…

   ...

   ce89ef2
10. BorisTyshkevich

    closed this as completedon Jul 28, 2026
11. lesandie

    added 7 commits that reference this issue on Aug 2, 2026

    fix(Altinity#429): resolve a saved query before navigating, trim blan…

    ...

    9813cf3

    fix(Altinity#429): make the single-diagnostic claim falsifiable, alig…

    ...

    634c8f2

    feat(Altinity#429): split the Dashboard tree row into chevron, name a…

    ...

    9115126

    fix(Altinity#429): rotate the chevron GLYPH, not the disclosure button

    ...

    96dd842

    feat(Altinity#429): unify Dashboard creation and add the metadata ren…

    ...

    eabdbca

    fix(Altinity#429): handle rename outcomes, isolate button keys, unify…

    ...

    c778cba

    refactor(Altinity#429): remove obsolete repair planner

    ...

    22d27fa