Skip to content

feat(dag-viewer): lane-based Compare/Union DAG-merge view - #249

Merged
zzylol merged 1 commit into
mainfrom
feat/dag-viewer-compare-union-186
Aug 23, 2026
Merged

zzylol merged 1 commit into
mainfrom
feat/dag-viewer-compare-union-186

Conversation

@zzylol

@zzylol zzylol commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Implements #186: two new view modes for tools/dag-viewer, reachable via a
Single / Compare / Union toggle next to the drop zone. Single stays the
default and is behaviorally unchanged; Compare and Union only activate once
two or more loaded queries are checked.

The issue text predates a couple of renames — corrected as I went:
dag_export.rs lives at crates/types/src/dag_export.rs, not
crates/ir/..., and asap_plan::cse::dedupe_subtrees was deleted in #192.
The real CSE pass today, asap_types::pre_asap::cse::share_common_subtrees
(landed via #235), still isn't wired into any caller that would give this
viewer real CSE output to render, so both new modes build on the same
hash-based structural-match proxy the existing single-view ring
highlighting already used — not real Rc identity. #244 (structural-hash
unification, not yet merged as of this PR) wasn't rebased in; this targets
main as-is.

Interaction model

  • Compare — every checked query gets its own lane: a dashed compound
    box titled with the query's name, using cytoscape.js's built-in
    compound-node grouping. A dashed line connects a node in one lane to the
    nearest earlier lane containing a node with the same structural hash, so
    "what recurs across these queries" is a line you trace instead of a ring
    you hunt for node by node. The link edges are added to the graph after
    the per-lane dagre layout call returns — they're purely decorative and
    never influence dagre's ranking, so lanes stay exactly one-per-query no
    matter how much cross-lane sharing there is.

  • Union — the checked queries merge into one graph. Any node whose hash
    is shared by ≥ 2 of them collapses into a single node (double border in
    the graph, and the side panel lists which queries it's present in, plus
    which of them it's the root of — a node can be the root of more than one
    query if two queries are fully identical). Edges from every query that
    reaches a shared node converge onto it instead of each query drawing a
    disconnected copy of the same subtree.

  • A small header note (""Shared" = same structural hash across independent exports, not live Rc identity"), shown only while in Compare/Union,
    plus an expanded README section, keep the UI honest about what "shared"
    means here — see the "Shared-subtree highlighting is a proxy, not real
    CSE" section for the two concrete reasons a hash match isn't a real-CSE
    guarantee (no PartialEq/legality re-check, no Rc identity crossing
    the tool's process boundary).

Handling branching DAGs

QueryExpr graphs branch at arbitrary depth (Merge/Concat with N
children, Join/SetOp/BinaryOp with two, LetBinding with two
structurally different children) — Union mode's merge is exactly the case
where this matters: a merged node can end up with several parents at
once
(e.g. two queries whose Aggregate differs but whose Scan is
identical both point at that one merged Scan). dagre lays out
multi-parent DAGs natively — it's a general layered-DAG algorithm, not a
tree layout — so no special-cased layout logic was needed for this beyond
building the merged node/edge set correctly (dedupe nodes and edges by a
merge key, keep every distinct incoming edge). Compare mode's lanes don't
need this since each lane is internally still a plain per-query tree/DAG,
identical in shape to what single-view already renders.

What didn't need to change

WorkloadGraph / DagGraph / DagNode in
crates/types/src/dag_export.rs already carry everything both modes
need — each node's hash, children, and each query's root — confirmed
by reading the exporter before writing any JS. No Rust changes.

Validation

No browser is available in this environment, so I went further than
eyeballing: the real inline script from index.html (plus node-style.js,
completely unmodified) was loaded into an actual V8 context via
py_mini_racer, with minimal DOM stubs for the handful of document/
window APIs it touches, and exercised against real dag_export output:

  • A freshly cargo run -p asap-devtools --bin dag_export-generated
    multi-query file (4 queries, deliberately overlapping: two SQL queries
    differing only in aggregate function, an exact duplicate of one of them,
    and a fully disjoint PromQL query) plus the committed dag.example.json.
  • Captured the exact elements arrays that would be handed to
    cytoscape/dagre (via a capturing stub in place of the real
    cytoscape() call) and asserted on them: no duplicate node/edge ids,
    every edge references a node that exists, no Compare-mode structural
    edge ever crosses a lane boundary, Union-mode's merge groups exactly
    match hand-computed expected sharing — including the merged Scan node
    ending up with two structurally different parents (the multi-parent
    branching case) and a node correctly tracked as root of more than one
    merged query.
  • Exercised the <2-selected path (must show the hint and never call
    cytoscape()), the mode-toggle's default-select-all-on-first-switch
    behavior and that the checkbox selection survives switching back to
    Single, and confirmed single-view's generated element shape is
    byte-for-byte unchanged from before the refactor (regression check).
  • Separately, loaded dagre.min.js standalone (no cytoscape, no DOM
    needed — it's a pure graph-layout library) and fed it a small
    compound/lane graph directly, confirming dagre's compound-node support
    does lay disconnected lanes out side by side without overlap, in
    insertion order — the load-bearing assumption behind Compare mode's lane
    layout.
  • JS syntax additionally checked with esprima.

Two small pre-existing doc bugs fixed in passing: the empty-state
instructions referenced the old asap-lower crate name (now
asap-devtools, matching the rest of the docs), and a JS comment pointed
at the old crates/ir/src/dag_export.rs path.

Docs

tools/dag-viewer/README.md's "Future work" section (which pointed at
this issue) is replaced with a real "Compare and Union mode" section
describing how to use both modes and what "shared" means, matching the
rest of the README's style. RUNNING.md gets a short pointer to it.

Closes #186

🤖 Generated with Claude Code

@zzylol zzylol changed the title dag-viewer: lane-based Compare/Union DAG-merge view feat(dag-viewer): lane-based Compare/Union DAG-merge view Aug 22, 2026
Implements #186: two new view modes for tools/dag-viewer, reachable via a
Single/Compare/Union toggle next to the drop zone (Single stays the
default, unchanged behavior).

- Compare mode lays every checked query out in its own compound-node lane
  (cytoscape.js's built-in lane grouping), side by side. A dashed link,
  added *after* the per-lane dagre layout runs, connects nodes across
  lanes whenever their structural hash matches -- so sharing is a line you
  can trace instead of a ring you hunt for one node at a time. Because the
  link edges are added post-layout, they never influence dagre's ranking
  or pull nodes out of their lane.

- Union mode merges the checked queries into a single graph: any node
  whose hash is shared by >= 2 of them collapses into one node (double
  border), with edges from every query that reaches it converging onto
  it instead of each drawing a disconnected copy. QueryExpr graphs branch
  at arbitrary depth, so a merged node can end up with several parents at
  once -- dagre lays out multi-parent DAGs natively, so no special-cased
  layout was needed beyond building the merged node/edge set correctly.

- The issue's stale references are corrected along the way: the exporter
  lives at crates/types/src/dag_export.rs (not crates/ir/...), and the
  hash-based "shared" signal both modes build on is the same proxy the
  existing single-view ring highlighting already used, now described
  accurately in both the UI copy (a header note, shown only in
  Compare/Union) and README.md -- explicitly not real Rc identity and not
  asap_plan::cse::dedupe_subtrees (deleted in #192; the real pass today,
  asap_types::pre_asap::cse::share_common_subtrees, still isn't wired into
  a caller that would give this viewer real CSE output to render).

- Two small pre-existing doc bugs fixed in passing: the empty-state
  instructions referenced the old asap-lower crate name (now
  asap-devtools, matching the rest of the docs), and the "shared subtree"
  JS comment pointed at crates/ir/src/dag_export.rs.

No Rust changes: WorkloadGraph/DagGraph/DagNode already carry everything
needed (per-node hash, children, each query's root) -- confirmed by
reading crates/types/src/dag_export.rs before writing any JS.

Validation (no browser available in this environment): the JS syntax was
checked with esprima, and the real inline script from index.html (plus
node-style.js, unmodified) was loaded into a genuine V8 context via
py_mini_racer with minimal DOM stubs, then exercised against real
dag_export output -- a freshly cargo-run multi-query export with
overlapping and disjoint queries, plus the committed dag.example.json --
capturing the exact elements arrays index.html would hand to
cytoscape+dagre and asserting on them: no duplicate node/edge ids, every
edge references a real node, no Compare-mode structural edge ever crosses
a lane boundary, Union-mode merge groups match hand-computed expected
sharing (including a merged Scan node with two structurally different
parents converging on it -- the multi-parent branching case), multi-root
tracking for a node that's the root of more than one merged query, the
<2-selected hint path never calls cytoscape(), and single-view's element
shape is byte-for-byte unchanged from before the refactor. Separately,
dagre.min.js itself was loaded standalone and given a compound/lane graph
directly, confirming dagre's compound-node support does lay disconnected
lanes out side by side without overlap, in insertion order -- the
assumption Compare mode's lane layout relies on.

Closes #186

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@zzylol
zzylol force-pushed the feat/dag-viewer-compare-union-186 branch from abff1b6 to 814316d Compare August 23, 2026 00:40
@zzylol
zzylol merged commit 4752359 into main Aug 23, 2026
1 check passed
@zzylol
zzylol deleted the feat/dag-viewer-compare-union-186 branch August 23, 2026 04:12
zzylol added a commit that referenced this pull request Aug 23, 2026
Adds a headless/no-browser path for tools/dag-viewer, on top of #249's
Compare/Union modes:

- `render.py` bakes a dag_export WorkloadGraph JSON (or several, merged
  the same way index.html's multi-file drop does) into one self-contained
  HTML file — every vendored script and the query data inlined, so it
  opens directly with no server and no drag-and-drop. index.html's own
  fetch('dag.json') convenience is blocked as cross-origin by most
  browsers over file://; embedding sidesteps that too, not just the
  no-browser case. `--mode compare|union` opens the page straight into
  that view with every loaded query pre-selected.

- Extracted index.html's inline <script> into `viewer.js`, unchanged
  apart from the additions below, so index.html and render.py's output
  share one copy of the interaction logic instead of forking it.

- Mode switches, the shared-subtree ring, and initial layout now animate
  (dagre's `animate: true`, a `transition-property` on node/edge style so
  class toggles ease instead of snap, and a fade-in on newly built
  elements) rather than hard-cutting to a new static layout on every
  render() call.

- `test_render.py` unit-tests render.py's merge/inline/escape logic
  (plain unittest.TestCase, matching tools/clickhouse's convention; not
  wired into CI, same as that tool).

Query cost isn't rendered — dag_export doesn't emit one yet (no cost
estimator wired into pre-ASAP IR). The side panel dumps a clicked node's
`detail` verbatim, so a future `cost` field there needs no viewer change.

Stacked on #249 (targets that branch, not main, until it merges) — see
issue #186 and PR #249's description for the Compare/Union groundwork
this builds on.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
zzylol added a commit that referenced this pull request Aug 24, 2026
* feat(dag-viewer): standalone Python render + animated transitions

Adds a headless/no-browser path for tools/dag-viewer, on top of #249's
Compare/Union modes:

- `render.py` bakes a dag_export WorkloadGraph JSON (or several, merged
  the same way index.html's multi-file drop does) into one self-contained
  HTML file — every vendored script and the query data inlined, so it
  opens directly with no server and no drag-and-drop. index.html's own
  fetch('dag.json') convenience is blocked as cross-origin by most
  browsers over file://; embedding sidesteps that too, not just the
  no-browser case. `--mode compare|union` opens the page straight into
  that view with every loaded query pre-selected.

- Extracted index.html's inline <script> into `viewer.js`, unchanged
  apart from the additions below, so index.html and render.py's output
  share one copy of the interaction logic instead of forking it.

- Mode switches, the shared-subtree ring, and initial layout now animate
  (dagre's `animate: true`, a `transition-property` on node/edge style so
  class toggles ease instead of snap, and a fade-in on newly built
  elements) rather than hard-cutting to a new static layout on every
  render() call.

- `test_render.py` unit-tests render.py's merge/inline/escape logic
  (plain unittest.TestCase, matching tools/clickhouse's convention; not
  wired into CI, same as that tool).

Query cost isn't rendered — dag_export doesn't emit one yet (no cost
estimator wired into pre-ASAP IR). The side panel dumps a clicked node's
`detail` verbatim, so a future `cost` field there needs no viewer change.

Stacked on #249 (targets that branch, not main, until it merges) — see
issue #186 and PR #249's description for the Compare/Union groundwork
this builds on.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(dag-viewer): correct test_render.py's run instructions

python3 -m unittest tools/dag-viewer/test_render.py doesn't actually work
run from the repo root -- unittest resolves the file-path form to a bare
'test_render' module but never adds tools/dag-viewer/ to sys.path, so the
file's own 'from render import ...' fails with ModuleNotFoundError. Same
latent bug exists in tools/clickhouse/test_extract_functions.py's docs
(copied from there), left alone since it's pre-existing and out of scope
here.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(dag-viewer): clean error messages for render.py's bad-input paths

A nonexistent input file or malformed JSON crashed with a raw Python
traceback (FileNotFoundError / json.JSONDecodeError propagating straight
out of main()) instead of a one-line stderr message + exit 1, matching
the existing 'no queries in input' / '--mode needs 2+ queries' style.
Found by actually running every command tools/dag-viewer's docs mention,
end to end, including through a real cargo-built dag_export.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

* fix(dag-viewer): reverse edge arrow direction to data-flow order

Structural edges pointed from a node to its children (parent -> child,
i.e. output -> its input), which is backwards for a data-flow diagram:
an arrowhead should point at the consumer, not the input. Swap
source/target for the edges built in all three view modes
(single/compare/union) so arrows now run input -> consumer, matching
the top-to-bottom dagre layout (scans at top, query root at bottom).

Addresses review feedback on #255.

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

dag-viewer: lane-based Compare/Union DAG-merge view

1 participant