From 797cd632ff7009a63f9eff9592c739155b322a55 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Sun, 28 Jun 2026 21:27:08 +0200 Subject: [PATCH] refactor(schema): address PR #62/#63 review follow-ups MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Code-review follow-ups for the zoom-bridge (#62) and schema-graph (#63) PRs. Correctness / behavior: - ch-client: the transitive-lineage nodeCap now counts only *linked* nodes, not every standalone table — so a single large DB of mostly-unrelated tables no longer truncates on round 1 and skips its cross-DB lineage walk (#63 review). - schema-graph: guard the db-prefix label strip with `&& n.name` so a node with an empty name can't be rewritten to a blank title (#63 review). Altitude / cleanup: - dot-layout: split the schema-specific "lineage first, singles gridded below" policy out of the generic dagre wrapper into a dedicated `schemaLayout()`; `dagreLayout` reverts to its plain generic form (no opts). Drops the dead `usedCols` (== cols) term and the always-built `connected` set on the pipeline path (#62/#63 review). - dom: extract the fixed-popover anchor recipe (divide client coords by the zoom) into a pure, 100%-covered `fixedAnchor()` helper; the File menu and the Save/user popover now share it instead of hand-transcribing the math — which also moves the previously-untested anchoredPopover arithmetic under test (#62). - app: simplify `dragCtx.scale` to report the page zoom without the redundant per-axis ternary (dragValue already ignores scale for the % axes) (#62). - dom: note in zoomScale that the measured element is immaterial (page-global) (#62). Tests added for schemaLayout (grid extent + no-lineage + no-singles + empty), fixedAnchor (both corners + floors), the ·inner label-skip, and the linked-only nodeCap. All 1020 tests pass; dot-layout/dom back to 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_019kE9qbgBNBrfNgwg9fRsMJ --- src/core/dot-layout.js | 101 ++++++++++++++++++-------------- src/core/schema-graph.js | 2 +- src/net/ch-client.js | 8 ++- src/ui/app.js | 19 +++--- src/ui/dom.js | 18 ++++++ src/ui/explain-graph.js | 6 +- src/ui/file-menu.js | 12 ++-- tests/unit/ch-client.test.js | 19 ++++++ tests/unit/dom.test.js | 22 ++++++- tests/unit/dot-layout.test.js | 76 ++++++++++++++++-------- tests/unit/schema-graph.test.js | 12 ++++ 11 files changed, 202 insertions(+), 93 deletions(-) diff --git a/src/core/dot-layout.js b/src/core/dot-layout.js index 76b2ac1..7885fbd 100644 --- a/src/core/dot-layout.js +++ b/src/core/dot-layout.js @@ -20,11 +20,23 @@ export function nodeWidth(label) { return Math.max(MIN_W, String(label).length * CHAR_W + PAD_X); } +// Box size for a node: honor an explicit w/h when it carries one (the rich schema +// cards pre-compute w/h from their content via cardSize); otherwise fall back to +// the label-based width + fixed height (pipeline + inline schema boxes). +const sizeOf = (n) => ({ width: n.w != null ? n.w : nodeWidth(n.label), height: n.h != null ? n.h : NODE_H }); +// `kind`/`db`/`name`/`external` (node) and `label` (edge) pass through for the +// schema graph's colouring, external-dimming + click-to-SHOW-CREATE (so the UI +// need not re-split the id or keep a side-channel for these). +const carry = (n) => ({ id: n.id, label: n.label, kind: n.kind, db: n.db, name: n.name, external: n.external }); + /** + * Lay out a graph with dagre. Generic (pipeline + schema lineage): every node is + * ranked top→bottom and edges routed. Returns `{ nodes, edges, width, height }` + * with node x/y as top-left. * @param dagre the injected dagre module (`{ graphlib, layout }`) * @param graph parsed `{ nodes:[{id,label}], edges:[{from,to}] }` */ -export function dagreLayout(dagre, graph, opts = {}) { +export function dagreLayout(dagre, graph) { const nodes = graph.nodes || []; if (!nodes.length) return { nodes: [], edges: [], width: 0, height: 0 }; const ids = new Set(nodes.map((n) => n.id)); @@ -32,33 +44,14 @@ export function dagreLayout(dagre, graph, opts = {}) { // would just loop onto its own box). const edges = (graph.edges || []).filter((e) => ids.has(e.from) && ids.has(e.to) && e.from !== e.to); - // Schema views opt into `isolatedLast`: lay out only the connected nodes with - // dagre and pack the edge-less "single" tables into a grid *below* the lineage, - // so a whole-DB graph reads as "relationships first, loose tables after" rather - // than dagre ranking the orphans across the top. Other callers (the pipeline - // graph) keep every node in the dagre pass. - const connected = new Set(); - for (const e of edges) { connected.add(e.from); connected.add(e.to); } - const singles = opts.isolatedLast ? nodes.filter((n) => !connected.has(n.id)) : []; - const ranked = opts.isolatedLast ? nodes.filter((n) => connected.has(n.id)) : nodes; - - // Honor a node's explicit size when it carries one (the rich schema cards - // pre-compute w/h from their content via cardSize); otherwise fall back to the - // label-based width + fixed height (pipeline + inline schema boxes). - const sizeOf = (n) => ({ width: n.w != null ? n.w : nodeWidth(n.label), height: n.h != null ? n.h : NODE_H }); - // `kind`/`db`/`name`/`external` (node) and `label` (edge) pass through for the - // schema graph's colouring, external-dimming + click-to-SHOW-CREATE (so the UI - // need not re-split the id or keep a side-channel for these). - const carry = (n) => ({ id: n.id, label: n.label, kind: n.kind, db: n.db, name: n.name, external: n.external }); - const g = new dagre.graphlib.Graph(); g.setGraph({ rankdir: 'TB', nodesep: NODESEP, ranksep: RANKSEP, marginx: MARGIN, marginy: MARGIN }); g.setDefaultEdgeLabel(() => ({})); - for (const n of ranked) g.setNode(n.id, sizeOf(n)); + for (const n of nodes) g.setNode(n.id, sizeOf(n)); for (const e of edges) g.setEdge(e.from, e.to); - if (ranked.length) dagre.layout(g); + dagre.layout(g); - const outNodes = ranked.map((n) => { + const outNodes = nodes.map((n) => { const dn = g.node(n.id); return { ...carry(n), x: dn.x - dn.width / 2, y: dn.y - dn.height / 2, w: dn.width, h: dn.height }; }); @@ -67,26 +60,46 @@ export function dagreLayout(dagre, graph, opts = {}) { points: g.edge(e.from, e.to).points.map((p) => ({ x: p.x, y: p.y })), })); const gg = g.graph(); - let width = ranked.length ? gg.width : 0; - let height = ranked.length ? gg.height : 0; + return { nodes: outNodes, edges: outEdges, width: gg.width, height: gg.height }; +} + +/** + * Schema-graph layout: dagre the connected lineage, then grid-pack the edge-less + * "single" tables *below* it — so a whole-DB graph reads as "relationships first, + * loose tables after" rather than dagre ranking the orphans across the top. The + * grid is a roughly-square block of uniform cells (widest/tallest single), + * left-aligned at the margin, one ranksep below the lineage (or at the top when + * there is no lineage at all). Same `{ nodes, edges, width, height }` shape. + */ +export function schemaLayout(dagre, graph) { + const nodes = graph.nodes || []; + if (!nodes.length) return { nodes: [], edges: [], width: 0, height: 0 }; + const ids = new Set(nodes.map((n) => n.id)); + const edges = (graph.edges || []).filter((e) => ids.has(e.from) && ids.has(e.to) && e.from !== e.to); + const connected = new Set(); + for (const e of edges) { connected.add(e.from); connected.add(e.to); } + const singles = nodes.filter((n) => !connected.has(n.id)); + if (!singles.length) return dagreLayout(dagre, graph); // no orphans → plain dagre - // Grid-pack the singles beneath the connected layout: a roughly-square block of - // uniform cells (widest/tallest single), left-aligned at the margin, sitting one - // ranksep below the lineage (or at the top when there's no lineage at all). - if (singles.length) { - const cells = singles.map(sizeOf); - const colW = Math.max(...cells.map((c) => c.width)); - const rowH = Math.max(...cells.map((c) => c.height)); - const cols = Math.max(1, Math.ceil(Math.sqrt(singles.length))); - const top = height ? height + RANKSEP : MARGIN; - singles.forEach((n, i) => { - const col = i % cols, row = Math.floor(i / cols); - outNodes.push({ ...carry(n), x: MARGIN + col * (colW + NODESEP), y: top + row * (rowH + NODESEP), w: cells[i].width, h: cells[i].height }); - }); - const usedCols = Math.min(cols, singles.length); - const rows = Math.ceil(singles.length / cols); - width = Math.max(width, MARGIN * 2 + usedCols * colW + (usedCols - 1) * NODESEP); - height = top + rows * rowH + (rows - 1) * NODESEP + MARGIN; - } - return { nodes: outNodes, edges: outEdges, width, height }; + // Lay the lineage out with dagre (connected nodes only, so the orphans don't + // reserve a rank-0 row across the top), then append the grid beneath it. + const base = dagreLayout(dagre, { nodes: nodes.filter((n) => connected.has(n.id)), edges }); + const cells = singles.map(sizeOf); + const colW = Math.max(...cells.map((c) => c.width)); + const rowH = Math.max(...cells.map((c) => c.height)); + const cols = Math.max(1, Math.ceil(Math.sqrt(singles.length))); + const top = base.height ? base.height + RANKSEP : MARGIN; + const gridded = singles.map((n, i) => ({ + ...carry(n), + x: MARGIN + (i % cols) * (colW + NODESEP), + y: top + Math.floor(i / cols) * (rowH + NODESEP), + w: cells[i].width, h: cells[i].height, + })); + const rows = Math.ceil(singles.length / cols); + return { + nodes: [...base.nodes, ...gridded], + edges: base.edges, + width: Math.max(base.width, MARGIN * 2 + cols * colW + (cols - 1) * NODESEP), + height: top + rows * rowH + (rows - 1) * NODESEP + MARGIN, + }; } diff --git a/src/core/schema-graph.js b/src/core/schema-graph.js index 487eb45..3467120 100644 --- a/src/core/schema-graph.js +++ b/src/core/schema-graph.js @@ -241,7 +241,7 @@ export function buildSchemaGraph(rows, focus) { // friendly ·inner and external-source labels are left alone — and ids/edges // (which key everything, incl. click-to-SHOW-CREATE) are unaffected. const curDb = focus && focus.db; - if (curDb) for (const n of outNodes) { if (n.db === curDb && n.label === n.id) n.label = n.name; } + if (curDb) for (const n of outNodes) { if (n.db === curDb && n.name && n.label === n.id) n.label = n.name; } return { nodes: outNodes, edges: outEdges }; } diff --git a/src/net/ch-client.js b/src/net/ch-client.js index 00a9c3d..3ec2ba7 100644 --- a/src/net/ch-client.js +++ b/src/net/ch-client.js @@ -237,7 +237,13 @@ export async function loadLineageTransitive(ctx, focus, opts = {}) { dictionaries = dictionaries.concat(part.dictionaries); } const graph = buildSchemaGraph({ tables, dictionaries }); - if (graph.nodes.length >= nodeCap) { truncated = true; break; } + // Cap on the *lineage* size — count only nodes that participate in an edge. + // Standalone tables are cheap to render and never drive cross-DB expansion, so + // they must not trip the cap (a single big DB of mostly-unrelated tables would + // otherwise truncate on the first round, before its few links are followed). + const linked = new Set(); + for (const e of graph.edges) { linked.add(e.from); linked.add(e.to); } + if (linked.size >= nodeCap) { truncated = true; break; } frontier = externalDbs(graph, loaded); } return { rows: { tables, dictionaries }, truncated }; diff --git a/src/ui/app.js b/src/ui/app.js index 9375dab..1a6507a 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -4,7 +4,7 @@ // window, location, fetch, crypto, sessionStorage) is injected so the whole // controller is testable under happy-dom with stubs. -import { h, zoomScale } from './dom.js'; +import { h, zoomScale, fixedAnchor } from './dom.js'; import { Icon } from './icons.js'; import { createState, activeTab, KEYS, recordHistory, saveQuery, savedForTab, tabChart, @@ -730,13 +730,11 @@ export function createApp(env = {}) { }; app.dom[refKey] = node; const r = anchorEl.getBoundingClientRect(); - // Bridge the shipped html{zoom}: getBoundingClientRect is post-zoom px, but a - // fixed element's top/right are re-scaled by zoom on paint — divide by scale so - // the popover anchors under the button (same as the File menu / editor popovers). - const scale = zoomScale(anchorEl); + // Right-align under the button, bridging html{zoom} (see fixedAnchor / zoomScale). + const a = fixedAnchor(r, zoomScale(anchorEl), { viewportW: win.innerWidth || 0 }); node.style.position = 'fixed'; - node.style.top = (r.bottom / scale + 6) + 'px'; - node.style.right = Math.max(8, ((win.innerWidth || 0) - r.right) / scale) + 'px'; + node.style.top = a.top + 'px'; + node.style.right = a.right + 'px'; doc.body.appendChild(node); doc.addEventListener('keydown', onKey, true); doc.addEventListener('mousedown', onOutside, true); @@ -890,9 +888,10 @@ export function renderApp(app, helpers) { const dragCtx = { state, rectFor, - // Only the px-based 'col' axis needs the html{zoom} bridge (the '%' axes use a - // zoom-cancelling ratio); measure the sidebar, which lives in the zoomed tree. - scale: (axis) => (axis === 'col' ? zoomScale(sidebar) : 1), + // The px-based 'col' axis divides clientX by the page zoom; the '%' axes use a + // zoom-cancelling ratio and ignore `scale` (dragValue's default), so we needn't + // special-case the axis here — just report the page zoom from the sidebar. + scale: () => zoomScale(sidebar), apply: (axis, value) => { if (axis === 'col') sidebar.style.width = value + 'px'; else if (axis === 'sideRow') sidebar.firstElementChild.style.height = value + '%'; diff --git a/src/ui/dom.js b/src/ui/dom.js index 0c8b7b8..1053c53 100644 --- a/src/ui/dom.js +++ b/src/ui/dom.js @@ -60,6 +60,8 @@ export function s(tag, props, ...children) { // post-zoom px while layout (offsetWidth) is pre-zoom CSS px, so their ratio is // the zoom. The single source of truth for bridging `html{zoom}` when mapping // between client coords and CSS px (editor popovers, results column-resize). +// `zoom` is a page-global html{} property, so the element measured is immaterial +// — pass any laid-out element near the work; the ratio is the same everywhere. // Falls back to 1 for any non-positive/non-finite ratio — an unlaid-out element // gives 0/0 → NaN, and offsetWidth 0 with a non-zero rect gives Infinity; both // (and a degenerate 0-width) must read as "no zoom", not blow up a divisor. @@ -67,3 +69,19 @@ export function zoomScale(el) { const s = el.getBoundingClientRect().width / el.offsetWidth; return Number.isFinite(s) && s > 0 ? s : 1; } + +// Place a fixed-position popover anchored under a button, bridging `html{zoom}`: +// getBoundingClientRect coords are post-zoom px but a fixed element's top/left/right +// are re-scaled by zoom on paint, so divide by `scale` (from zoomScale). Returns +// `{ top, left }`, or `{ top, right }` when `viewportW` is given (right-align to +// the anchor's right edge). `gap` is the px below the anchor; `min` floors the +// side inset. Pure arithmetic on a DOMRect-like — the single recipe for the File +// menu, the Save popover and the user menu. +export function fixedAnchor(rect, scale, opts = {}) { + const gap = opts.gap != null ? opts.gap : 6; + const min = opts.min != null ? opts.min : 8; + const top = rect.bottom / scale + gap; + return opts.viewportW != null + ? { top, right: Math.max(min, (opts.viewportW - rect.right) / scale) } + : { top, left: Math.max(min, rect.left / scale) }; +} diff --git a/src/ui/explain-graph.js b/src/ui/explain-graph.js index abd4d6e..9584454 100644 --- a/src/ui/explain-graph.js +++ b/src/ui/explain-graph.js @@ -8,7 +8,7 @@ import { h, s, withDocument } from './dom.js'; import { Icon } from './icons.js'; import { parseDot } from '../core/dot.js'; -import { dagreLayout } from '../core/dot-layout.js'; +import { dagreLayout, schemaLayout } from '../core/dot-layout.js'; import { buildCardModel, cardSize, CARD } from '../core/schema-cards.js'; import { qualifyIdent } from '../core/format.js'; import { fitBox, fitWidthBox, zoomBox, panBox, viewBoxStr } from '../core/panzoom.js'; @@ -180,7 +180,7 @@ export function buildPipelineSvg(rawText, dagre) { /** Build the schema-lineage SVG from a `{nodes,edges}` graph (kind-coloured). */ export function buildSchemaSvg(graph, dagre, onNode) { - return renderGraphSvg(dagreLayout(dagre, graph || { nodes: [], edges: [] }, { isolatedLast: true }), { + return renderGraphSvg(schemaLayout(dagre, graph || { nodes: [], edges: [] }), { nodeClass: (n) => 'eg-node eg-node--' + (n.kind || 'table'), edgeClass: (e) => 'eg-edge eg-edge--' + (e.kind || 'feeds'), edgeLabel: (e) => e.kind, @@ -248,7 +248,7 @@ export function buildRichSchemaSvg(graph, dagre, onNode) { }); // `external` rides through dagreLayout (like kind/db/name), so the node class can // read it off the laid node — no side-channel needed. - const laid = dagreLayout(dagre, { nodes: sized, edges: g.edges || [] }, { isolatedLast: true }); + const laid = schemaLayout(dagre, { nodes: sized, edges: g.edges || [] }); // Overlay any manually-moved positions remembered for this result, then // straighten the edges touching a moved node so they still connect on first draw. const positions = g.savedPositions; diff --git a/src/ui/file-menu.js b/src/ui/file-menu.js index cba77da..e946e45 100644 --- a/src/ui/file-menu.js +++ b/src/ui/file-menu.js @@ -5,7 +5,7 @@ // effect goes through an injected seam (app.saveJSON / app.saveStr / // app.downloadFile / app.FileReader / app.document), so it is fully testable. -import { h, zoomScale } from './dom.js'; +import { h, zoomScale, fixedAnchor } from './dom.js'; import { Icon } from './icons.js'; import { flashToast } from './toast.js'; import { renderSavedHistory } from './saved-history.js'; @@ -104,13 +104,11 @@ export function openFileMenu(app) { app.dom.fileMenu = menu; doc.body.appendChild(overlay); const r = app.dom.fileBtn.getBoundingClientRect(); - // Bridge the shipped html{zoom}: getBoundingClientRect is post-zoom px, but a - // fixed element's top/left are re-scaled by zoom on paint — divide by scale so - // the menu anchors under the button (same as the editor popovers via zoomScale). - const scale = zoomScale(app.dom.fileBtn); + // Anchor under the button, bridging html{zoom} (see fixedAnchor / zoomScale). + const a = fixedAnchor(r, zoomScale(app.dom.fileBtn)); menu.style.position = 'fixed'; - menu.style.top = (r.bottom / scale + 6) + 'px'; - menu.style.left = Math.max(8, r.left / scale) + 'px'; + menu.style.top = a.top + 'px'; + menu.style.left = a.left + 'px'; doc.body.appendChild(menu); doc.addEventListener('keydown', onKey, true); } diff --git a/tests/unit/ch-client.test.js b/tests/unit/ch-client.test.js index d3ec742..72af692 100644 --- a/tests/unit/ch-client.test.js +++ b/tests/unit/ch-client.test.js @@ -461,6 +461,25 @@ describe('loadLineageTransitive', () => { expect(out.truncated).toBe(true); }); + it('counts only linked nodes toward the cap — standalone tables do not truncate the cross-DB walk', async () => { + // 'a' has one cross-DB link (a.t → b.mv) plus 4 standalone tables: 6 total nodes + // after round 1, but only 2 linked. nodeCap 3 must NOT truncate, and 'b' must load. + const ctx = ctxWith((url, init) => { + const sql = init.body; + if (/EXPLAIN AST/.test(sql) || /system\.dictionaries/.test(sql)) return jsonResp({ data: [] }); + if (/database = 'a'/.test(sql)) return jsonResp({ data: [ + tbl('a', 't', 'MergeTree', { dependencies_database: ['b'], dependencies_table: ['mv'] }), + tbl('a', 's1', 'MergeTree'), tbl('a', 's2', 'MergeTree'), + tbl('a', 's3', 'MergeTree'), tbl('a', 's4', 'MergeTree'), + ] }); + if (/database = 'b'/.test(sql)) return jsonResp({ data: [tbl('b', 'mv', 'MaterializedView')] }); + return jsonResp({ data: [] }); + }); + const out = await loadLineageTransitive(ctx, { db: 'a' }, { nodeCap: 3 }); + expect(out.truncated).toBe(false); // 2 linked < cap 3 + expect(new Set(out.rows.tables.map((t) => t.database)).has('b')).toBe(true); // cross-DB walk reached b + }); + it('loads a multi-database frontier concurrently in one round', async () => { const ctx = ctxWith((url, init) => { const sql = init.body; diff --git a/tests/unit/dom.test.js b/tests/unit/dom.test.js index f8a5987..b7211c6 100644 --- a/tests/unit/dom.test.js +++ b/tests/unit/dom.test.js @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { h, s, withDocument, zoomScale } from '../../src/ui/dom.js'; +import { h, s, withDocument, zoomScale, fixedAnchor } from '../../src/ui/dom.js'; const SVG_NS = 'http://www.w3.org/2000/svg'; @@ -56,6 +56,26 @@ describe('zoomScale', () => { }); }); +describe('fixedAnchor', () => { + it('left-aligns under the anchor, dividing client coords by the zoom scale', () => { + const a = fixedAnchor({ bottom: 40, left: 100 }, 1.2); + expect(a.top).toBeCloseTo(40 / 1.2 + 6); // r.bottom/scale + default 6px gap + expect(a.left).toBeCloseTo(100 / 1.2); + expect(a.right).toBeUndefined(); + }); + it('right-aligns to the anchor when viewportW is given', () => { + const a = fixedAnchor({ bottom: 40, right: 1180 }, 1.2, { viewportW: 1200 }); + expect(a.top).toBeCloseTo(40 / 1.2 + 6); + expect(a.right).toBeCloseTo((1200 - 1180) / 1.2); + expect(a.left).toBeUndefined(); + }); + it('floors the side inset at `min` (default 8) and honors a custom gap', () => { + expect(fixedAnchor({ bottom: 0, left: 0 }, 1)).toMatchObject({ top: 6, left: 8 }); + expect(fixedAnchor({ bottom: 0, right: 1200 }, 1, { viewportW: 1200 })).toMatchObject({ right: 8 }); + expect(fixedAnchor({ bottom: 10, left: 50 }, 1, { gap: 0, min: 0 })).toMatchObject({ top: 10, left: 50 }); + }); +}); + describe('s (SVG namespace)', () => { it('creates elements in the SVG namespace with attrs, style, events, and children', () => { const onclick = vi.fn(); diff --git a/tests/unit/dot-layout.test.js b/tests/unit/dot-layout.test.js index 4c194a7..1576d89 100644 --- a/tests/unit/dot-layout.test.js +++ b/tests/unit/dot-layout.test.js @@ -1,7 +1,7 @@ import { describe, it, expect } from 'vitest'; import dagre from '@dagrejs/dagre'; import { parseDot } from '../../src/core/dot.js'; -import { dagreLayout, nodeWidth } from '../../src/core/dot-layout.js'; +import { dagreLayout, schemaLayout, nodeWidth } from '../../src/core/dot-layout.js'; // dagre is pure (no DOM), so the tests drive it directly — the same library the // app injects at runtime via the app.Dagre seam. @@ -74,33 +74,57 @@ describe('dagreLayout', () => { expect(g.edges).toEqual([{ from: 'a', to: 'b', points: expect.any(Array) }]); }); - describe('isolatedLast (schema views)', () => { - it('packs edge-less nodes into a grid below the connected lineage', () => { - const g = dagreLayout(dagre, { - nodes: [ - { id: 'a', label: 'A' }, { id: 'b', label: 'B' }, // a → b lineage - { id: 's1', label: 'S1' }, { id: 's2', label: 'S2' }, { id: 's3', label: 'S3' }, // singles - ], - edges: [{ from: 'a', to: 'b' }], - }, { isolatedLast: true }); - const by = Object.fromEntries(g.nodes.map((n) => [n.id, n])); - expect(g.nodes).toHaveLength(5); // every node kept - expect(g.edges).toHaveLength(1); // lineage edge preserved - const lineageBottom = Math.max(by.a.y + by.a.h, by.b.y + by.b.h); - for (const id of ['s1', 's2', 's3']) expect(by[id].y).toBeGreaterThanOrEqual(lineageBottom); // all below - expect(by.a.y).toBeLessThan(by.b.y); // lineage still laid out top→bottom +}); + +describe('schemaLayout (lineage first, singles gridded below)', () => { + it('packs edge-less nodes into a grid below the connected lineage', () => { + const g = schemaLayout(dagre, { + nodes: [ + { id: 'a', label: 'A' }, { id: 'b', label: 'B' }, // a → b lineage + { id: 's1', label: 'S1' }, { id: 's2', label: 'S2' }, { id: 's3', label: 'S3' }, // singles + ], + edges: [{ from: 'a', to: 'b' }], }); + const by = Object.fromEntries(g.nodes.map((n) => [n.id, n])); + expect(g.nodes).toHaveLength(5); // every node kept + expect(g.edges).toHaveLength(1); // lineage edge preserved + const lineageBottom = Math.max(by.a.y + by.a.h, by.b.y + by.b.h); + for (const id of ['s1', 's2', 's3']) expect(by[id].y).toBeGreaterThanOrEqual(lineageBottom); // all below + expect(by.a.y).toBeLessThan(by.b.y); // lineage still laid out top→bottom + }); + + it('grids singles from the top with a known extent when there is no lineage', () => { + // 3 unlabeled-width singles: cols = ceil(sqrt 3) = 2, rows = 2; colW = MIN_W 64, + // rowH = NODE_H 30; MARGIN 12, NODESEP 26. + const g = schemaLayout(dagre, { + nodes: [{ id: 'x', label: 'X' }, { id: 'y', label: 'Y' }, { id: 'z', label: 'Z' }], + edges: [], + }); + expect(g.nodes).toHaveLength(3); + expect(g.edges).toEqual([]); + expect(g.width).toBe(12 * 2 + 2 * 64 + 1 * 26); // 178 — two columns + expect(g.height).toBe(12 + 2 * 30 + 1 * 26 + 12); // 110 — two rows, top at MARGIN + const by = Object.fromEntries(g.nodes.map((n) => [n.id, n])); + expect(by.x).toMatchObject({ x: 12, y: 12 }); // row 0, col 0 + expect(by.y).toMatchObject({ x: 12 + 64 + 26, y: 12 }); // row 0, col 1 + expect(by.z).toMatchObject({ x: 12, y: 12 + 30 + 26 }); // row 1, col 0 + }); - it('grids all nodes from the top when none are connected (no lineage)', () => { - const g = dagreLayout(dagre, { - nodes: [{ id: 'x', label: 'X' }, { id: 'y', label: 'Y' }, { id: 'z', label: 'Z' }], - edges: [], - }, { isolatedLast: true }); - expect(g.nodes).toHaveLength(3); - expect(g.edges).toEqual([]); - expect(g.width).toBeGreaterThan(0); - expect(g.height).toBeGreaterThan(0); - expect(Math.min(...g.nodes.map((n) => n.y))).toBeLessThanOrEqual(12); // top row at the margin + it('falls back to plain dagre when there are no singles', () => { + const g = schemaLayout(dagre, { + nodes: [{ id: 'a', label: 'A' }, { id: 'b', label: 'B' }], + edges: [{ from: 'a', to: 'b' }], }); + expect(g.nodes).toHaveLength(2); + expect(g.edges).toHaveLength(1); + expect(Object.fromEntries(g.nodes.map((n) => [n.id, n.y])).a) + .toBeLessThan(Object.fromEntries(g.nodes.map((n) => [n.id, n.y])).b); + }); + + it('returns an empty layout for no nodes and tolerates missing nodes/edges keys', () => { + expect(schemaLayout(dagre, {})).toEqual({ nodes: [], edges: [], width: 0, height: 0 }); // no `nodes` key + const g = schemaLayout(dagre, { nodes: [{ id: 'solo', label: 'Solo' }] }); // no `edges` key + expect(g.nodes).toHaveLength(1); + expect(g.edges).toEqual([]); }); }); diff --git a/tests/unit/schema-graph.test.js b/tests/unit/schema-graph.test.js index 8d42c7f..a5bdc86 100644 --- a/tests/unit/schema-graph.test.js +++ b/tests/unit/schema-graph.test.js @@ -208,6 +208,18 @@ describe('buildSchemaGraph', () => { expect(byId['other.remote_tbl'].label).toBe('other.remote_tbl'); // another db → stays qualified }); + it('keeps the friendly ·inner label for a same-db implicit-MV storage node (not stripped to bare name)', () => { + const UUID = 'abcd'; + const rows = { tables: [ + T('lin', 'events', 'MergeTree'), + T('lin', 'mv', 'MaterializedView', { uuid: UUID, create_table_query: 'CREATE MATERIALIZED VIEW lin.mv (x UInt8) ENGINE = MergeTree AS SELECT 1 FROM lin.events' }), + T('lin', '.inner_id.' + UUID, 'MergeTree'), + ], dictionaries: [] }; + const byId = Object.fromEntries(buildSchemaGraph(rows, { kind: 'db', db: 'lin' }).nodes.map((n) => [n.id, n])); + expect(byId['lin..inner_id.' + UUID].label).toBe('·inner'); // label !== id → left alone, not rewritten + expect(byId['lin.events'].label).toBe('events'); // ordinary same-db node still stripped + }); + it('keeps every table as a standalone node when a whole-DB graph has no relationships', () => { // A DB of unrelated tables (e.g. all URL engine) still renders its tables — // showing the objects beats an empty "no relationships" screen, even though