fix(explain): address code-review findings

- run(): a typed EXPLAIN now honors exactly what was typed — a plain EXPLAIN
  always opens the verbatim Explain view instead of inheriting a stale rich view
  from a previous run/tab. Drop the unused app.state.explainView. (review #1)
- dot.parseDot: blank quoted label strings before edge scanning and only keep
  edges between declared processors, so a `->` inside a label (e.g. a lambda
  `x -> x + 1`) no longer fabricates phantom nodes/edges. (review #2)
- panzoom.zoomBox: guard a degenerate zero-size viewBox (defensive).
- overlay: Esc stops propagation so it doesn't also reach the app key handler.
- remove now-dead isExplain from core/format.js (superseded by parseExplain).
- document the ontime DOT fixture's capture provenance.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QGBS74oUsXarGkCRQKEFLu

Author: BorisTyshkevich

src/core/dot.js

@@ -33,23 +33,24 @@ export function parseDot(text) {
 
   const nodes = [];
   const seen = new Set();
-  const add = (id, label) => {
-    if (seen.has(id) || DOT_KEYWORDS.has(id)) return;
-    seen.add(id);
-    nodes.push({ id, label });
-  };
-
   const nodeRe = /([A-Za-z_][\w]*)\s*\[\s*label\s*=\s*"((?:[^"\\]|\\.)*)"/g;
   let m;
-  while ((m = nodeRe.exec(body))) add(m[1], unescapeDot(m[2]));
+  while ((m = nodeRe.exec(body))) {
+    const id = m[1];
+    if (seen.has(id) || DOT_KEYWORDS.has(id)) continue;
+    seen.add(id);
+    nodes.push({ id, label: unescapeDot(m[2]) });
+  }
 
+  // Scan edges with quoted labels blanked out, so a `->` (or an id) inside a
+  // processor label — e.g. a lambda `x -> x + 1` — can't be mistaken for a
+  // data-flow edge. Only edges between already-declared processors are kept
+  // (ClickHouse always declares its nodes), which also rules out phantom nodes.
   const edges = [];
+  const edgeBody = body.replace(/"(?:[^"\\]|\\.)*"/g, '""');
   const edgeRe = /([A-Za-z_][\w]*)\s*->\s*([A-Za-z_][\w]*)/g;
-  while ((m = edgeRe.exec(body))) {
-    if (DOT_KEYWORDS.has(m[1]) || DOT_KEYWORDS.has(m[2])) continue;
-    edges.push({ from: m[1], to: m[2] });
-    add(m[1], m[1]);
-    add(m[2], m[2]);
+  while ((m = edgeRe.exec(edgeBody))) {
+    if (seen.has(m[1]) && seen.has(m[2])) edges.push({ from: m[1], to: m[2] });
   }
   return { nodes, edges };
 }

src/core/format.js

@@ -74,12 +74,6 @@ export function detectSqlFormat(sql) {
   return m ? m[1] : null;
 }
 
-/** True if the statement is an EXPLAIN (leading keyword). EXPLAIN output is plan
- *  text, so the caller renders it raw unless the user gave an explicit FORMAT. Pure. */
-export function isExplain(sql) {
-  return /^\s*EXPLAIN\b/i.test(String(sql || ''));
-}
-
 /**
  * Derive a short display name for a saved query: "Query · <table>" when a
  * FROM clause is present, else the first 48 chars of the collapsed SQL.

src/core/panzoom.js

@@ -18,6 +18,7 @@ export function fitBox(gw, gh, pad = 0.04) {
  * aspect is preserved.
  */
 export function zoomBox(vb, factor, cx, cy, minW, maxW) {
+  if (!vb.w || !vb.h) return vb; // nothing to zoom (degenerate box)
   const want = vb.w / factor;
   const w = Math.max(minW, Math.min(maxW, want));
   const k = w / vb.w; // actual applied scale after clamping

src/state.js

@@ -58,11 +58,9 @@ export function createState(read = { loadJSON, loadStr }) {
     running: false,
     abortController: null,
     resultView: 'table',
-    // The active EXPLAIN view (Explain/Indexes/Projections/Pipeline/Estimate),
-    // kept across re-runs like resultView. `forceExplain` is set by the Explain
-    // button to put an ordinary query into EXPLAIN-view mode; a normal Run clears
-    // it. Both session-only.
-    explainView: 'explain',
+    // `forceExplain` is set by the Explain button to put an ordinary query into
+    // EXPLAIN-view mode; a normal Run clears it (session-only). The active view is
+    // derived per-run from the typed statement / clicked tab, not stored here.
     forceExplain: false,
     resultSort: { col: null, dir: 'asc' },
     sidePanel: read.loadStr(KEYS.sidePanel, 'saved'),

src/ui/app.js

@@ -395,13 +395,14 @@ export function createApp(env = {}) {
     let fmt;
     let explainView = null;
     if (explainMode) {
-      // The Explain view runs the statement verbatim (honoring arbitrary params);
-      // a rich view is auto-selected only on an exact canonical match, else we keep
-      // the last-used view. Rich views derive a query from the inner statement.
+      // View precedence: an explicit tab click wins; otherwise a *typed* EXPLAIN
+      // is honored exactly (canonical match → its rich view, else the verbatim
+      // Explain view); the button-forced path falls through to Explain. We never
+      // inherit a stale view from a previous run/tab — typing a plain EXPLAIN must
+      // show the plan, not whatever view was last open.
       explainView = (opts && opts.explainView)
         || (parsed && detectExplainView(parsed))
-        || app.state.explainView || 'explain';
-      app.state.explainView = explainView;
+        || 'explain';
       fmt = (EXPLAIN_VIEWS.find((v) => v.id === explainView) || EXPLAIN_VIEWS[0]).chFormat;
       const inner = parsed ? parsed.inner : tab.sql;
       runSql = explainView === 'explain'

src/ui/explain-graph.js

@@ -63,7 +63,7 @@ export function openPipelineFullscreen(app, rawText) {
   const doc = (app && app.document) || document;
   const built = buildPipelineSvg(rawText || '');
 
-  const onKey = (e) => { if (e.key === 'Escape') close(); };
+  const onKey = (e) => { if (e.key === 'Escape') { e.stopPropagation(); close(); } };
   let backdrop;
   // `close` only fires from listeners attached after `backdrop` is assigned.
   function close() {

tests/e2e/fixtures/ontime-pipeline-graph.dot

@@ -1,3 +1,9 @@
+// Captured from the antalya demo cluster (ontime dataset) — the real output of
+// `EXPLAIN PIPELINE graph = 1` for the fact/dim-join query documented in
+// tests/e2e/pipeline-graph.spec.js. To re-capture if ClickHouse's pipeline DOT
+// format changes, run that query with `EXPLAIN PIPELINE graph = 1 <query> FORMAT
+// TabSeparatedRaw` against ontime.fact_ontime ⋈ ontime.dim_airports and paste the
+// digraph below (drop this comment header — the parser starts at `digraph`).
 digraph
 {
   rankdir="LR";

tests/unit/app.test.js

@@ -373,6 +373,15 @@ describe('query run', () => {
     await app.actions.run();
     expect(app.activeTab().result.explainView).toBe('indexes');
   });
+  it('does not leak a previous rich view onto a freshly-typed plain EXPLAIN', async () => {
+    const { app } = appForRun([[(u, sql) => /EXPLAIN/.test(sql), resp({ text: 'digraph{}' })]]);
+    app.activeTab().sql = 'EXPLAIN PIPELINE graph = 1 SELECT 1';
+    await app.actions.run();
+    expect(app.activeTab().result.explainView).toBe('pipeline');
+    app.activeTab().sql = 'EXPLAIN SELECT 2'; // plain → must show the plan, not pipeline
+    await app.actions.run();
+    expect(app.activeTab().result.explainView).toBe('explain');
+  });
   it('setExplainView re-runs a derived query and never edits the SQL', async () => {
     const { app, e } = appForRun([[(u, sql) => /EXPLAIN/.test(sql), resp({ text: 'digraph{}' })]]);
     app.activeTab().sql = 'EXPLAIN SELECT 1';

tests/unit/dot.test.js

@@ -20,15 +20,21 @@ digraph
     expect(g.nodes.map((n) => n.id)).toEqual(['a', 'b']);
     expect(g.edges).toEqual([{ from: 'a', to: 'b' }]);
   });
-  it('de-duplicates node ids and skips DOT keywords', () => {
+  it('de-duplicates node ids, skips DOT keywords, and drops edges to undeclared ids', () => {
     const g = parseDot('node [label="default"]; n1 [label="A"]; n1 [label="A again"]; n1 -> n2;');
-    // `node` keyword skipped; n1 only once; n2 added from the edge.
-    expect(g.nodes).toEqual([{ id: 'n1', label: 'A' }, { id: 'n2', label: 'n2' }]);
+    // `node` keyword skipped; n1 only once; n2 never declared → no phantom node…
+    expect(g.nodes).toEqual([{ id: 'n1', label: 'A' }]);
+    expect(g.edges).toEqual([]); // …and its edge is dropped
   });
-  it('skips edges whose endpoints are DOT keywords', () => {
+  it('skips edges whose endpoints are not declared nodes', () => {
     const g = parseDot('digraph { n1 [label="A"]; node -> n1; }');
     expect(g.edges).toEqual([]);
   });
+  it('ignores -> and ids that appear inside label strings (no phantom nodes/edges)', () => {
+    const g = parseDot('digraph { n0 [label="Join a -> b"]; n1 [label="Scan"]; n0 -> n1; }');
+    expect(g.nodes.map((n) => n.id)).toEqual(['n0', 'n1']); // no phantom a / b
+    expect(g.edges).toEqual([{ from: 'n0', to: 'n1' }]);
+  });
   it('unescapes quotes and collapses \\n in labels', () => {
     const g = parseDot('digraph { n1 [label="line1\\nline2"]; n2 [label="say \\"hi\\""]; }');
     expect(g.nodes[0].label).toBe('line1 line2');

tests/unit/format.test.js

@@ -1,6 +1,6 @@
 import { describe, it, expect } from 'vitest';
 import {
-  clamp, formatRows, formatBytes, timeAgo, sqlString, inferQueryName, isNumericType, shortVersion, userShortName, withStatementBreak, detectSqlFormat, isExplain,
+  clamp, formatRows, formatBytes, timeAgo, sqlString, inferQueryName, isNumericType, shortVersion, userShortName, withStatementBreak, detectSqlFormat,
 } from '../../src/core/format.js';
 
 describe('clamp', () => {
@@ -101,20 +101,6 @@ describe('detectSqlFormat', () => {
   });
 });
 
-describe('isExplain', () => {
-  it('detects a leading EXPLAIN (any variant), ignoring leading whitespace/case', () => {
-    expect(isExplain('EXPLAIN SELECT 1')).toBe(true);
-    expect(isExplain('  explain pipeline SELECT 1')).toBe(true);
-    expect(isExplain('EXPLAIN AST SELECT 1')).toBe(true);
-  });
-  it('is false for non-EXPLAIN statements', () => {
-    expect(isExplain('SELECT 1')).toBe(false);
-    expect(isExplain('SELECT explain FROM t')).toBe(false); // EXPLAIN not the leading keyword
-    expect(isExplain('')).toBe(false);
-    expect(isExplain(null)).toBe(false);
-  });
-});
-
 describe('inferQueryName', () => {
   it('uses FROM table when present', () => {
     expect(inferQueryName('SELECT * FROM system.tables')).toBe('Query · system.tables');