Skip to content

feat(gq): undirected edge traversal — $a <edge> $b#336

Merged
aaltshuler merged 5 commits into
mainfrom
feat/undirected-traversal
Jul 5, 2026
Merged

feat(gq): undirected edge traversal — $a <edge> $b#336
aaltshuler merged 5 commits into
mainfrom
feat/undirected-traversal

Conversation

@aaltshuler

@aaltshuler aaltshuler commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator

Implements iss-gq-undirected-traversal via the cheap Expand-internal design — no dependency on iss-744/579's Union machinery (direction is handled inside the operator; alternation will compose later by carrying the flag per branch).

What ships

$a <edgeName> $b matches the edge in either direction, with set semantics (both-ways pairs and self-loops appear once). Composes with hop bounds (<knows>{1,3}) and not { } ("no edge in either direction"). Only valid on same-endpoint-type edges — an asymmetric edge (e.g. WorksAt: Person -> Company) is rejected at typecheck with the new T22 (it's well-typed in at most one orientation; the error points at the directional form).

Why it's cheap

The machinery already existed: the IR's Direction enum gains Both; the topology index always builds both CSR and CSC; the indexed arm already had the reverse probe (("dst","src")); the per-source dedup gates already guard emission on both arms. The angle-bracket token is collision-free (comparisons live in the structurally separate filter production).

A real bug found red-first

The undirected anti-join test failed against the naive implementation and exposed a lowering gap: negation inners are typechecked into a discarded context clone, so the direction lookup silently fell back to Out inside not { }. Fix: undirectedness travels on the AST node itself — the syntax is the source of truth, immune to context-propagation gaps.

Verification

  • Compiler: parse (bare/bounded/inside-not), typecheck (Both on Knows, T22 on WorksAt), 254 tests green.
  • Engine: traversal.rs (directional-miss demonstration + out ∪ in, both-ways dedup, var-length from an incoming-only node, undirected vs directional anti-join), traversal_indexed.rs CSR==indexed, and proptest_equivalence.rs widened with <knows>{1,3} — CSR == indexed == auto over generated graphs.
  • cargo test --workspace --locked green (68 suites).

Motivating case: the dev graph's issue_related dashboards silently missed incoming IssueRelated edges; with this, one undirected pattern replaces the two-query workaround.


Open in Devin Review

Greptile Summary

Adds $a <edge> $b undirected edge traversal syntax to GQ, matching a same-endpoint-type edge in either direction with set semantics (both-ways pairs and self-loops appear once). The implementation extends the existing Direction enum with a Both variant and threads it through parser → typecheck → IR lowering → CSR/indexed execution paths, guarded by a new T22 typecheck error for asymmetric edges.

  • Parser/typecheck/IR: undirected_edge = { "<" ~ edge_ident ~ ">" } is parsed into Traversal::undirected: bool; typecheck enforces T22 (same-endpoint-type only) then overrides the inferred direction to Both; lowering carries direction from the AST node directly (bypassing the ResolvedTraversal context lookup that was discarded for negation inners — the root-cause fix described in the PR).
  • Engine (CSR): execute_expand_csr additionally walks the CSC (adj_rev) per source node, chained with the CSR scan; the existing visited/seen_dst_dense gates provide correct dedup for bidirectional pairs and self-loops.
  • Engine (indexed): execute_expand_indexed iterates endpoint_probes(Both) = [("src","dst"),("dst","src")], merging both BTREE scans into a single neighbor_map per hop; coverage is priced conservatively as the worst of both probed columns via the new worse_coverage helper; the anti-join mask checks both CSR and CSC adjacency for "no edge in either direction".

Confidence Score: 5/5

Safe to merge. The change adds a new syntax form with a correctness guard (T22) at typecheck, extends existing well-tested BFS machinery, and includes comprehensive tests across parser, typecheck, lowering, CSR, indexed, and property-based paths.

All three execution arms (CSR, indexed, auto) are covered by tests, including the dedup edge cases (bidirectional pairs, self-loops) and the negation-inner regression that motivated carrying direction on the AST node. The T22 guard prevents asymmetric-edge misuse at compile time. No existing behavior is altered for directional traversals.

No files require special attention.

Important Files Changed

Filename Overview
crates/omnigraph/src/exec/query.rs Core execution changes: adds endpoint_probes, worse_coverage, and adj_rev/CSC walk in both the CSR and indexed expand paths; anti-join mask updated for both-direction check. Direction grouping in both expand arms is consistent.
crates/omnigraph-compiler/src/query/typecheck.rs T22 guard added before binding analysis; direction is overridden to Both after inference (no-op for same-type edges); direction correctly declared mut. Clean design.
crates/omnigraph-compiler/src/ir/lower.rs Direction resolved from AST traversal.undirected flag instead of the context lookup (the root-cause fix for the negation-inner regression); dst_type and src_type for Both correctly use from_type per T22 guarantee.
crates/omnigraph-compiler/src/query/query.pest New undirected_edge rule; grammar comment correctly notes angle-bracket is collision-free with comparison operators in the structurally separate filter production.
crates/omnigraph/tests/traversal.rs Three new tests cover: out-union-in union + dedup, variable-length hops from incoming-only nodes, undirected vs directional anti-join.
crates/omnigraph/tests/traversal_indexed.rs One-hop undirected CSR==indexed test plus a degraded-coverage test that appends an unindexed fragment to exercise the pessimistic coverage path.
crates/omnigraph/tests/proptest_equivalence.rs Adds related query with {1,3} to the property-test suite, widening the CSR==indexed==auto invariant to cover undirected multi-hop over generated graphs.
crates/omnigraph-compiler/src/types.rs Adds Direction::Both variant with docstring; no serialization or wire-format exposure.
docs/user/queries/index.md Undirected traversal syntax documented inline with T22, hop-bounds, and not{} composition. Cost note is accurate user-facing guidance.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["a edge b undirected syntax"] --> B["Parser: Traversal undirected=true"]
    B --> C{Typecheck}
    C -->|"from_type != to_type"| D["T22 error asymmetric edge rejected"]
    C -->|"from_type == to_type"| E["direction = Both ResolvedTraversal pushed"]
    E --> F["IR Lowering reads traversal.undirected not context lookup"]
    F --> G["IROp::Expand direction=Both"]
    G --> H{"execute_expand coverage = worse of src_col and dst_col"}
    H -->|indexed path| J["execute_expand_indexed probes src+dst BTREE per hop neighbor_map merges both seen_dst dedup"]
    H -->|CSR path| K["execute_expand_csr chain adj CSR + adj_rev CSC per frontier node visited + seen_dst dedup"]
    G --> L["IROp::AntiJoin not block"]
    L --> M["try_bulk_anti_join_mask checks both adj and adj_rev has_neighbors"]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["a edge b undirected syntax"] --> B["Parser: Traversal undirected=true"]
    B --> C{Typecheck}
    C -->|"from_type != to_type"| D["T22 error asymmetric edge rejected"]
    C -->|"from_type == to_type"| E["direction = Both ResolvedTraversal pushed"]
    E --> F["IR Lowering reads traversal.undirected not context lookup"]
    F --> G["IROp::Expand direction=Both"]
    G --> H{"execute_expand coverage = worse of src_col and dst_col"}
    H -->|indexed path| J["execute_expand_indexed probes src+dst BTREE per hop neighbor_map merges both seen_dst dedup"]
    H -->|CSR path| K["execute_expand_csr chain adj CSR + adj_rev CSC per frontier node visited + seen_dst dedup"]
    G --> L["IROp::AntiJoin not block"]
    L --> M["try_bulk_anti_join_mask checks both adj and adj_rev has_neighbors"]
Loading

Comments Outside Diff (1)

  1. crates/omnigraph/src/exec/query.rs, line 1050-1066 (link)

    P2 Cost model underestimates Direction::Both work by ~2×

    gather_cost_inputs returns a single edge_count and a src_node_count derived from from_type for Direction::Both, but the indexed path actually fires two BTREE scans per hop — once keyed by src and once by dst (see endpoint_probes). The planner's ~hops × frontier × fanout cost estimate is therefore half the real indexed work for undirected traversals. This can tip the planner toward the indexed path when CSR would be cheaper for large frontiers or densely connected undirected edges.

    The user-facing doc update already notes "its cost is roughly twice the directional equivalent," but the cost model feeding the decision doesn't reflect this. A simple correction is to double edge_count (or the derived effective_max_hops-weighted product) when direction == Direction::Both.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

    Fix in Claude Code

Reviews (3): Last reviewed commit: "test: lowering-boundary pins for Both + ..." | Re-trigger Greptile

iss-gq-undirected-traversal, the Expand-internal design (no plan-level
Union; unblocked from iss-744/579): the grammar gains an undirected_edge
alternative in the traversal rule (angle brackets are collision-free —
comparisons live in the structurally separate filter production), the
AST Traversal carries `undirected`, Direction gains `Both`, and
typecheck resolves undirected patterns to Both after the new T22 rule:
undirected requires a same-endpoint-type edge (an asymmetric edge is
well-typed in at most one orientation, so the form is rejected with
guidance to use the directional pattern). Lowering passes Both through;
the reverse-expand orientation flip is a no-op for a symmetric
traversal. Bounds ({min,max}) and not{} compose unchanged.

Direction has no serde derives and IR never crosses a wire — no
compatibility surface. Parser/typecheck tests cover the bare, bounded,
inside-not forms, Both resolution on Knows (Person->Person), and the
T22 rejection on WorksAt (Person->Company).
CSR arm: an undirected hop chains csr(edge).neighbors with
csc(edge).neighbors under the existing visited/seen_dst gates, so
both-direction pairs and self-loops dedup for free (set semantics),
including per-hop in variable-length BFS. Indexed arm: endpoint_probes
returns both (src,dst) and (dst,src) orientations for Both; the per-hop
scan runs once per orientation into one neighbor_map, deduped by the
existing per-source seen_dst sets. Bulk anti-join: "no edge in EITHER
direction" (csr || csc has_neighbors).

Also fixes a lowering gap the anti-join test caught red: negation
inners are typechecked into a discarded context clone, so the
ResolvedTraversal direction lookup silently fell back to Out inside
not{} — undirectedness now travels on the AST node itself (the syntax
is the source of truth), immune to context-propagation gaps.

Tests: traversal.rs (out ∪ in vs the directional miss, both-ways dedup,
var-length from an incoming-only node, undirected anti-join vs
directional), traversal_indexed.rs both_modes equivalence, and the
proptest arm-equivalence property widened with <knows>{1,3}.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

Open in Devin Review

Comment thread crates/omnigraph/src/exec/query.rs
Comment thread crates/omnigraph/src/exec/query.rs
…nti-join arm grouping

Devin: the cost model's index-coverage probe checked only the primary
endpoint column, so an undirected traversal with a degraded dst index
was priced as fully indexed; coverage is now the pessimistic combination
over every column endpoint_probes will scan. Greptile: the bulk
anti-join grouped Both with In for src_type_name but with Out for the
adjacency — safe only under T22's from==to guarantee; Both now groups
with Out in both arms so a future T22 relaxation cannot split them
silently.
…erage

Closes two coverage gaps from review of my own test surface: (1) the
lowering boundary — undirected lowers to Expand{direction: Both} both
top-level and inside not{} (the discarded-context-clone regression is
now pinned at the layer it lives at, not only end-to-end); (2) the
review-fix path — undirected both_modes equivalence with a
mutation-appended unindexed fragment degrading BTREE coverage, so
results stay correct and mode-equivalent whichever path the pessimistic
coverage pricing picks.
@aaltshuler aaltshuler merged commit 2faa8df into main Jul 5, 2026
7 checks passed
@aaltshuler aaltshuler deleted the feat/undirected-traversal branch July 5, 2026 21:59
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.

1 participant