Skip to content

Schema-graph drawer: fix stale-write race, add cancellation, progressive draw #124

Description

@BorisTyshkevich

Problem

The inline schema-lineage graph (drag a db/table onto the results pane → showSchemaGraph, src/ui/app.js) has two related issues:

  1. Stale-write race (same bug class as inbox: openNodeDetail/openDetailPane has no stale-response guard #97). showSchemaGraph captures tab up front, sets tab.result = newResult('Table') with a loading placeholder, awaits ch.loadSchemaLineage, then writes tab.result.schemaGraph = {...} when it resolves — with no check that tab.result is still its result object. If the user runs a query (or Explain) in the same tab while the lineage fetch is in flight, run()/runScript() replace tab.result with a fresh object; when the stale lineage fetch resolves afterward it writes .schemaGraph onto that new object, and the results-pane priority chain (src/ui/results.js) silently renders the stale graph instead of the query's actual output. The same last-resolved-wins problem hits two drags in a row. inbox: openNodeDetail/openDetailPane has no stale-response guard #97 fixed this exact class for the fullscreen node-detail pane via a request-identity guard; that fix was never extended here.
  2. No cancellation. There's no AbortController on the lineage fetch, and no way to cancel it manually. Combined with Schema lineage: cap the EXPLAIN AST fan-out on large schemas #42 (unbounded EXPLAIN AST fan-out per view/MV), a slow fetch on a large schema just sits there with a generic spinner until it finishes or errors.
  3. Inline drawer is blank longer than it needs to be. buildSchemaGraph edges come from two very different costs: structured dependencies_*/loading_dependencies_* fields, MV-target parsing, and engine-arg parsing are all free (computed client-side from the first system.tables query) — only a View/MV's source tables require a per-object EXPLAIN AST round trip. Today the graph isn't drawn at all until every EXPLAIN AST settles, even though a meaningfully-connected graph is often available immediately from the free edges.
  4. UX based only on mouse drag is unclear for users. Drag-to-drawer is not a discoverable gesture; there's no visible affordance hinting it exists.

Proposal

0. New trigger: click a closed database row to also draw its schema.
Today a single click on a database row in the left tree only toggles expanded (src/ui/schema.js:120-125) — no network, no drawer. Add: on the collapsed → expanded transition only, also call showSchemaGraph({ db }) to draw it in the bottom drawer.

  • Deliberately scoped to that one transition, not every click: clicking to collapse an already-open db must not re-draw, re-fetch, or auto-close the drawer — otherwise a click made to tidy up the tree also triggers a network fetch and a redraw, which is surprising and wasteful.
  • Re-clicking an already-expanded db (no state change) does nothing new either.
  • Drag-to-drawer stays as an additional, explicit way to draw a specific table's/db's graph (unaffected by this).

1. Request-identity guard (correctness fix, mirrors #97). Stash a request token (or the specific result object) when showSchemaGraph starts; before the final write, check it's still current and no-op otherwise. This is the actual fix for the race — cancellation below is an optimization on top of it, not a substitute.

2. Cancellation.

  • Add app.state.schemaGraphAbortController — a third context alongside the existing app.state.abortController (run/script) and exportAbort (export), same established pattern.
  • Add a cancelSchemaGraph({ clearResult = false }) helper: aborts the controller if set, clears it, and optionally clears tab.result.schemaGraph (used when a new operation is taking over the drawer vs. just cancelling to retry).
  • Manual cancel: extend the loading placeholder with a Cancel affordance, mirroring the existing .exp-cancel button in showExportProgress (src/ui/results.js:1309-1316) rather than inventing new UI.
  • Cancel result state, explicitly: if Phase A has already drawn (nodes + free edges visible), cancelling Phase B keeps that graph on screen — clear schemaGraph.loading/stop the spinner and Cancel button, and mark it visibly partial (e.g. a title/badge noting view/MV source edges may be incomplete) so it isn't mistaken for the fully-resolved graph. If cancelled before Phase A has anything to show, clear the result (tab.result = null) and fall back to the existing generic empty state (results.js:154-157, "Press ⌘↵ to run query") — no new "cancelled" placeholder needed. Either way, clicking Expand afterward is unaffected: expandSchemaGraph always re-fetches full lineage from scratch (app.js — "keeps the inline path's shape frozen"), so a partial/cancelled inline graph never leaks into the fullscreen view.
  • Automatic cancel: call cancelSchemaGraph() from the top of both run() (app.js:503 — covers Run and Explain, same function) and runScript() (app.js:631 — a separate function with its own tab.result write, easy to miss), and from showSchemaGraph's own entry (a new click/drag replaces the old one). The identity guard from (1) makes this belt-and-suspenders, not load-bearing.
  • Signal threading, and a swallow-all trap to fix on the way: queryJson(ctx, sql) doesn't accept a signal today; add one and pass it through to authedFetch's existing signal param (ch-client.js:32, already wired for fetch). Then in loadSchemaLineage, pass the signal to every queryJson/EXPLAIN AST call, including the one behind tryQueryData (ch-client.js:294-301, the system.dictionaries best-effort read). tryQueryData currently does catch { return null } unconditionally — if that's left as-is, an abort mid-flight on the dictionaries read gets swallowed and loadSchemaLineage continues as though the read had merely failed (a permissions/missing-table case), instead of propagating the cancellation. tryQueryData needs to rethrow when e.name === 'AbortError' and only swallow genuine query errors.

3. Two-phase progressive draw.

  • Phase A (immediate): build+layout the graph from the free edges only (dependencies/target/engine-arg/dictionary edges) as soon as the first system.tables/system.dictionaries queries resolve. Draw it right away instead of showing a blank/generic spinner.
  • Phase B (background): resolve EXPLAIN AST per view/MV (already fire independently via Promise.all; keep that, just don't block the first paint on it). Once all have settled (success or error), do one final layout+redraw with the merged edge set.
  • No core changes needed in src/core/schema-graph.js for this — buildSchemaGraph already treats astTables as optional (schema-graph.js:173, guarded by Array.isArray(t.astTables)), so Phase A and Phase B are just two calls to the existing function, before and after AST population.
  • Explicitly not a re-layout per resolved edge — dagre re-ranks the whole graph on its topology, so re-running it on every single edge would visibly jitter/reposition nodes while the user is looking at it. One clean second pass avoids that.
  • While phase B is in flight, reflect progress in the spinner/placeholder text (e.g. "resolving 3/12 view sources…") instead of a static message — cheap, no layout thrash, keeps the user informed it isn't stuck.

Related / explicitly not duplicating

Acceptance criteria

  • Clicking a closed database row expands it and draws its schema graph in the bottom drawer; clicking it again to collapse does not re-fetch, re-draw, or force the drawer closed.
  • Running or Explaining a query (single or script) in a tab while that tab's schema-graph lineage fetch is still in flight does not corrupt the query result (no stray schemaGraph property leaking onto the new result) — covered by a regression test reproducing the current race for both run() and runScript().
  • Dragging/clicking a second db/table before the first lineage fetch resolves shows the second graph, never the first (last-triggered wins, not last-resolved).
  • An abort during a best-effort sub-query (e.g. system.dictionaries) propagates as a cancellation of the whole lineage fetch, not a silent "no dictionaries, continue" — covered by a tryQueryData unit test asserting AbortError is rethrown, not swallowed.
  • The loading placeholder has a working Cancel action; cancelling aborts the in-flight fetch and leaves the pane in a clean (non-error, non-stale) state: the Phase A graph stays visible (marked partial) if it had already drawn, otherwise the pane falls back to the normal empty-results placeholder.
  • Starting Run or Explain (statement or script) while a lineage fetch is pending cancels it automatically (no wasted completion write, no console error from an unhandled rejection).
  • The graph draws immediately with whatever edges are free/known, before any EXPLAIN AST results are in; a single second layout pass merges in view/MV source edges once they've all settled.
  • npm test stays green at the existing per-file coverage gates (CLAUDE.md hard rule 1) — new/changed logic in src/net/ch-client.js, src/ui/app.js, src/ui/schema.js, src/ui/placeholder.js, src/ui/results.js needs matching test coverage in tests/unit/.

Likely touched files

  • src/ui/app.jsshowSchemaGraph (identity guard, abort wiring, two-phase build), cancelSchemaGraph helper, hooks in run() and runScript()
  • src/ui/schema.js — click handler: draw on the collapsed→expanded transition
  • src/net/ch-client.jsqueryJson (accept signal), tryQueryData (rethrow AbortError), loadSchemaLineage (thread signal through every call, report incremental progress instead of a single opaque Promise.all)
  • src/ui/placeholder.js — cancellable placeholder variant (or extend loadingPlaceholder with an optional onCancel)
  • src/ui/results.js — wire the Cancel button, progress text
  • src/core/schema-graph.js — no change expected (already handles missing astTables); confirm with a test rather than modify
  • tests/unit/ch-client.test.js, schema.test.js, app.test.js (or equivalent), placeholder.test.js, results.test.js

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingenhancementNew feature or request

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions