feat(gfql): native polars unbounded directed var-length rows(binding_ops) — unblocks LDBC IS6 on polars (#1709) - #1781
Merged
Conversation
…(LDBC IS6, #1709) The Cypher multi-alias bindings table (`rows(binding_ops=...)`) lowered natively for fixed-length and BOUNDED variable-length segments, but an unbounded fixed-point segment (`-[*]->` / `-[*0..]->`) declined with "polars engine does not yet natively support cypher row op 'rows'". That was the last shape blocking engine='polars' on LDBC SNB interactive-short-6, the one interactive-short query polars could not answer. Unbounded DIRECTED fixed point now lowers natively. Termination is data-dependent, so instead of pandas' expand-paths-until-empty (which materializes every partial path at every hop) the lowering runs a dedup-by-node frontier walk to find the exhaustion depth D, then reuses the SAME lazy bounded pair-join loop the `-[*1..k]->` arm uses with max_hops=D. Deduping by endpoint changes no walk's EXISTENCE, only its multiplicity, which the bounded loop then reproduces in full -- so the emitted rows are pandas-identical while the exponential path expansion happens once, lazily. A cycle reachable from the seed means infinitely many paths: that raises the same E108 "require terminating variable-length segments" error pandas raises, detected by a pigeonhole bound on the distinct nodes rather than after the blow-up. Also fixes a latent silent-wrong found by the differential fuzz: the polars builder rebuilt bindings from the CHAIN OUTPUT while pandas rebuilds from the pre-chain base graph. The traversal prunes to what IT considers matched, which is not the bindings builder's match set -- a zero-hop var-length segment binds a seed with no matching outgoing edge, and the traversal drops that node. So `-[*0..k]->` could silently return fewer rows than pandas. The builder now sources its node/edge tables from the same base graph pandas uses (and that the indexed builder was already handed). Honest declines unchanged in spirit (NIE, never a silent answer): undirected unbounded, aliased var-length relationships, and unbounded segments without to_fixed_point (pandas truncates at a bound this lowering cannot reconstruct). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxxTFpKiqA4FQa9Pjy8yR1
…bag lazily - extract `_directed_varlen_reachable_polars` so the unbounded fixed-point arm and the bounded `-[*1..k]->` arm run literally the same loop instead of two copies kept in sync by a comment. - pin the generic bindings builder's path bag as a LazyFrame. It always was one; mypy inferred eager because `filter_by_dict_polars` declared the eager type. - make `filter_by_dict_polars` frame-polymorphic via a constrained TypeVar (DataFrame | LazyFrame) rather than declaring one flavour and being called with both -- the eager viz lane and the lazy chain lane take the same `.filter(expr)` path, and the TypeVar keeps the caller's flavour on the way out. Removes the mypy noise this file was carrying, so the new code lands at delta 0. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxxTFpKiqA4FQa9Pjy8yR1
Widening filter_by_dict_polars to a constrained TypeVar changes the diagnostic at its two engine-neutral DataFrameT call sites from arg-type to type-var, so the existing localized ignore no longer applied. Retarget it and add the twin in gfql_fast_paths. Both sites are already gated on a polars engine; the cast is what the surrounding code has always asserted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NxxTFpKiqA4FQa9Pjy8yR1
lmeyerov
commented
Jul 27, 2026
| <!-- Do Not Erase This Section - Used for tracking unreleased changes --> | ||
|
|
||
| ### Added | ||
| - **Native polars `rows(binding_ops=...)` for UNBOUNDED directed variable-length patterns (`-[*]->` / `-[*0..]->`) (#1709)**: the Cypher multi-alias bindings table already lowered natively for fixed-length and *bounded* variable-length segments, but an unbounded fixed-point segment declined with `NotImplementedError: polars engine does not yet natively support cypher row op 'rows'`. This was the last shape blocking `engine='polars'` on LDBC SNB interactive-short-6 (`MATCH (m:Message)-[:REPLY_OF*0..]->(p:Post)<-[:CONTAINER_OF]-(f:Forum)-[:HAS_MODERATOR]->(mod)`), the one interactive-short query polars could not answer. It now runs natively: a dedup-by-node frontier walk finds the exhaustion depth, then the SAME lazy bounded pair-join loop the `-[*1..k]->` arm uses materializes one row per distinct edge SEQUENCE (Cypher path multiplicity, parallel edges included) — no pandas bridge, no `to_pandas()` round trip. A cycle reachable from the seed means infinitely many paths; that raises the same E108 "require terminating variable-length segments" error pandas raises, and is detected before the path expansion blows up rather than after. Still declining honestly (NIE, never a silent answer): UNDIRECTED unbounded (`-[*]-`, needs the min_hops == 1 multiplicity reconstruction plus backtrack-aware termination — pandas rejects it outright), aliased variable-length relationships (pandas rejects those too), and unbounded segments WITHOUT `to_fixed_point` (pandas silently truncates at a bound this lowering cannot reconstruct). Cross-engine parity is the gate: differential fuzz vs the pandas oracle over random DAGs and cyclic graphs with self-loops and parallel edges, plus pinned IS6/zero-hop/multiplicity/cycle tests. |
Contributor
Author
There was a problem hiding this comment.
need pyg-bench perf expectation too now that unblocks some task(s)
once happy with postive/negative tests on either side of opt boundary, and ci still green, can land both sides
…peline.py Pure code move, no behaviour change. row_pipeline.py 1638 -> 1528 lines; the unbounded/variable-length arms live in polars/varlen_rows.py with a docstring explaining the exhaustion-depth walk and why deduping by endpoint is sound. Motivation is rebase surface: #1757, #1714 and the seeded-chain work all touch row_pipeline.py, and ~270 lines of var-length specialization sitting in the core flow made every one of those a conflict. RowPipelineMixin stays imported FUNCTION-LOCALLY exactly as at the original site — hoisting it to module scope reintroduces an import cycle (row.pipeline -> lazy engine -> back), so the move preserves it and says why. 2244 polars tests pass, ruff clean.
Contributor
Author
|
Assessed against the landing bar.
Two things I found on the branchIt was carrying three commits that were never pushed (origin was at Kept — Moved off this branch — They are parked on |
Contributor
Author
|
pyg-bench #103 (merged): The baseline that makes this a clean pinMeasured 2026-07-27 at SF1 on both master
Nothing already in the stack moves it — so this is your cell specifically, and the gate A missing probe result is a failure, not a skip: a query quietly dropped from a suite What remains on this PRRun the SNB polars lane against this branch and confirm the gate flips to exit 0. |
…eline
Adversarial review of this branch found a silent-wrong regression and a red CI.
BLOCKER — `-[*k..]->` with k >= 2 returned WRONG COUNTS, no error.
The unbounded-directed arm of the `rows` gate had no `min_hops` guard. Cypher
`-[*k..]->` lowers to `{hops: null, min_hops: k, to_fixed_point: true,
direction: forward}`, so every k >= 2 was SERVED. Pandas' `step_pairs` come
from the var-length `edge_op.execute` hop, which empties when its
`max_reached_hop < min_hops` and otherwise drops edges labelled below
min_hops; `max_reached_hop` is a dedup-by-node BFS eccentricity, not a
longest-walk length, so the raw-edge reconstruction here expands a different
edge multiset. On a 7-node acyclic graph, both engines answering:
MATCH (a)-[*3..]->(b) RETURN count(*) pandas 0 polars 30
MATCH (a {id: 0})-[*2..]->(b) RETURN count(*) pandas 24 polars 41
85 of 1200 acyclic fuzz cases diverged, with zero declines. Master declines all
of them. The second gate this PR's description relies on (`_is_native_multihop`
in `_chain_traversal_polars`) never runs here: `RETURN count(*)` lowers to a
pure-CALL chain, so the `rows` gate is the only gate. The existing decline test
missed it because it pins `to_fixed_point=False` — a shape Cypher never emits.
Now the unbounded arm requires resolved `min_hops <= 1`, mirroring the
undirected arm; 0 and 1 (`-[*]->`, `-[*0..]->`, the IS6 walk) stay served.
BLOCKER — CI was red and the coverage gate never ran. `varlen_rows.py` was
missing from `coverage_baselines/ci-polars-py3.12.json`, whose own note requires
a complete list, so `test-polars (3.12)` exited 3 and `changed-line-coverage`
(which needs it) was SKIPPED — this branch's changed lines were never gated.
Also fixed:
* the cycle path raised a different exception CLASS and `.code` per engine
(pandas GFQLTypeError/E303 via execute_call's wrapper, polars a raw
GFQLValidationError/E108 because the native kernel runs before that
wrapper). Repo control flow keys on `.code`. Now wrapped identically.
* cycle detection was bounded by the GLOBAL node count with an eager collect
per hop, so a two-node cycle reachable from one seed cost O(graph) collects
(measured linear in global N) before raising. Now bounded by the REACHABLE
set: a walk of h edges visits h+1 nodes, all within `seen`, so h >= |seen|
IS a repeat. Exact in both directions, and the O(E) `node_cap` scan it
replaces is gone entirely (`pairs_df.height == 0` is the same test).
* the "hoisting reintroduces an import cycle" note was FALSE — disproven by
hoisting. The sibling import moves to the import block and the mid-file
`# noqa: E402` goes; `RowPipelineMixin` stays function-local, matching the
engine convention. Unused typing imports and a stale line count removed.
* `to_fixed_point` WITH an explicit bound was newly served and undocumented.
It is parity-correct (pandas ignores the flag once max_hops is set) but was
pinned by nothing; now tested and in the changelog.
Tests, all mutation-checked (reverting each fix fails the matching test):
* gate-level, k in 0..3, either side of the boundary
* end-to-end through Cypher on the repro graph — the one that actually
catches the blocker, since both engines answer and only values differ
* the cycle test now asserts class and `.code` are ENGINE-INDEPENDENT rather
than pinning polars' own code (the old form passed while they disagreed)
* a long acyclic walk must not be mistaken for a cycle, and a small reachable
cycle behind a large unreachable remainder must still raise — the two
failure modes of the new bound
Verified in-container on dgx-spark: 86 passed in the binding-rows file, 276
with the row-pipeline file, and `graphistry/tests/compute` with `--gpus all`
gives an IDENTICAL 9-failure set to this branch's base (pre-existing dask/cuDF
coercion), 6821 passed vs the base's 6804. mypy differential: 166 errors on
both trees, so these changes add none. ruff clean under the repo config.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
The re-review of the previous commit found that the gate rewrite which closed
the unbounded `-[*k..]->` hole left a narrower one open, and that the test I
added to pin it could not have caught it.
Master declined on `bool(op.to_fixed_point)` ALONE. Restructuring the gate to
key on the RESOLVED MAX incidentally let `to_fixed_point=True` combined with an
explicit bound fall through to the bounded arm — where it hits the same
reconstruction gap as the unbounded case, silently, for min_hops >= 3:
fwd min=3 max=4 tfp=True pandas 0 polars 30 (master: NIE)
fwd min=3 max=5 tfp=True pandas 0 polars 30
fwd min=4 max=5 tfp=True pandas 0 polars 6
fwd hops=3 tfp=True pandas 0 polars 24
reverse spellings: identical
Not reachable from Cypher — `cypher/parser.py` sets to_fixed_point False for
`*k` and `*i..k`, and only leaves it True for `*` and `*k..`, which always have
max_hops None. So this is the AST / `rows(binding_ops=...)` wire surface only.
Hard to reach is not correct, and the previous commit additionally claimed in
the CHANGELOG that the shape "matches pandas", which is false.
Declining it restores master's behaviour exactly, so it cannot regress anything
that previously worked.
WHY THE OLD TEST WAS BLIND, since the same mistake is easy to repeat: it
compared flagged-vs-unflagged WITHIN each engine and its parametrization
stopped at min_hops=2. A within-engine comparison never consults the pandas
oracle, so it cannot see a divergence where BOTH engines answer; and min_hops
<= 2 happens to agree on that fixture. It now asserts the DECLINE, runs to
min_hops=4, and is joined by a value test on the divergence graph that compares
ROW COUNTS against the oracle (engine-neutral: a bare `rows()` call emits
engine-specific scaffolding columns on every shape, pre-existing and unrelated).
Mutation-checked: removing the guard fails 9 tests, including the value test —
the property the previous version lacked.
Verified in-container on dgx-spark: binding-rows file 90 passed;
`graphistry/tests/compute` with `--gpus all` gives 6825 passed and a failure set
byte-identical to this branch's base (the 9 pre-existing dask/cuDF coercion
failures).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
lmeyerov
added a commit
that referenced
this pull request
Jul 27, 2026
…surfaces (#1788) The named-middle rewrite turns `[...named ops..., rows()]` into `rows(binding_ops=<middle>)` so a Cypher multi-alias RETURN lowers to a bindings table. It excluded calls already carrying `binding_ops`, `source` or `alias_endpoints` — but not a non-default `table`. So merely NAMING an op in the middle changed which table came back: * odd-length named middle -> the rewrite fired and the caller silently got the BINDINGS table instead of the edges table. No error; `table=` ignored. * even-length named middle -> a path ending on an EDGE, so the rewritten op list is not an alternating node/edge path and it hard-errored with "require ... a single connected alternating node/edge path". * UNNAMED middle -> correct. Which is what made this look shape-specific rather than naming-specific. Found while chasing why LDBC IS3 is the only interactive-short cell with a competitor number and no GFQL score: its adapter runs a two-pass workaround whose edge half is exactly `(person)-[r:KNOWS]-` + `rows(table="edges")`. Both surfaces need the guard — `compute/chain.py` and the native polars `gfql/lazy/engine/polars/chain.py` carry the rewrite independently, and the mutation checks below show fixing one leaves the other wrong. THE GUARD KEYS ON A NON-DEFAULT TABLE, NOT ON `is None`. `rows()` declares `table: str = "nodes"` and always emits it, so `params.get("table") is None` is never true — my first attempt used that and disabled the rewrite outright, breaking the IS6 bindings path (5 regressions). Those were caught only by re-baselining against current master rather than against the stale baseline I had from an earlier branch, which no longer matched after #1781/#1785 landed. The residual cost is that an EXPLICIT `rows(table="nodes")` is byte-identical to a bare `rows()` at the params level and still rewrites; that limitation is now pinned by a test rather than left to be rediscovered. Distinguishing them would need `rows()` to default `table=None`, a wire-format change and out of scope. Tests: 10 cases across both engines — the even-length (IS3) case, the SILENT odd-length case, named-vs-unnamed equivalence, the documented `table="nodes"` limitation, and the NEGATIVE side: a bare `rows()` after a named middle must STILL get the bindings table, without which "never rewrite" would pass and break every Cypher multi-alias RETURN. Mutation-checked PER SURFACE: removing the pandas guard fails 3 pandas cases with polars green; removing the polars guard fails 3 polars cases with pandas green. Verified in-container on dgx-spark: `graphistry/tests/compute` with `--gpus all` gives 7030 passed against master 3f2128e's 7020 (+10, the new file) and an IDENTICAL 9-failure set (pre-existing dask/cuDF coercion). Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lmeyerov
added a commit
that referenced
this pull request
Jul 27, 2026
…pes (#1787) Pandas is the oracle and the contract is parity-or-NotImplementedError. Three BOUNDED var-length shapes in the native polars `rows(binding_ops=...)` path returned a DIFFERENT count with no error. Same root-cause family as the unbounded case #1781 declined: pandas' step_pairs come from the var-length `edge_op.execute` hop, whose hop-window pruning -- and, when seeded, its per-seed BFS -- changes the edge multiplicity in a way a rebuild from the raw matching edge table does not reproduce. Declined (diverging graphs out of 60 random ones per shape, differential fuzz against the pandas oracle): * directed `-[*k..m]->` with min_hops >= 3 (53/60; pandas 0 vs polars 35), and min_hops >= 2 off a FILTERED seed (27-30/60). Plain min_hops <= 2 is fuzz-clean, including as a non-first segment -- that is the graph-bench q3 `-[*1..k]->` shape, still served. * undirected `-[*1..1]-` / `-[*1]-`, the degenerate window: 60/60, halves the count (pandas 36 vs polars 18). `-[*1..2]-` and wider agree, which is why the existing tests missed it. * undirected `-[*1..k]-` that does not start from the full node set: filtered seed 51/60, non-first segment 36/60. Every directed equivalent agrees, so this is specific to the undirected doubled-pair expansion. The gate keys on an EXPLICIT var-length window rather than on `EdgeSemantics.is_multihop`, because `-[*1..1]-` resolves to min == max == 1 and so is NOT multihop -- yet pandas still routes it through the var-length hop (`-[]-` gives 18 on a graph where `-[*1..1]-` gives 36). Plain `-[]-` is untouched. Tests: explicit decline + still-served neighbours for each of the three, plus a seeded differential fuzz over random small graphs (cyclic, parallel edges, self-loops) that asserts parity-or-raise AND that enough shapes are still served for the check to mean anything. Bounded windows only: undirected unbounded shapes through the pandas oracle can exhaust the box. The new test file is REGISTERED in bin/test-polars.sh. That list is an explicit allowlist, and the polars lane is the only CI lane with polars installed -- so the only lane that can execute this file at all. Without the registration the three `return None` decline statements were the only changed lines no lane ever ran (10/13 = 76.92%, exactly the changed-line-coverage gate failure); with it the block is 13/13. Mutation-checked: dropping the min_hops clause fails 4 tests, the degenerate undirected window 3, the undirected-seed clause 3, the whole gate 8. Full graphistry/tests/compute is unchanged by the declines (9 pre-existing failures, same set). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
lmeyerov
added a commit
that referenced
this pull request
Jul 28, 2026
…pes (#1787) Pandas is the oracle and the contract is parity-or-NotImplementedError. Three BOUNDED var-length shapes in the native polars `rows(binding_ops=...)` path returned a DIFFERENT count with no error. Same root-cause family as the unbounded case #1781 declined: pandas' step_pairs come from the var-length `edge_op.execute` hop, whose hop-window pruning -- and, when seeded, its per-seed BFS -- changes the edge multiplicity in a way a rebuild from the raw matching edge table does not reproduce. Declined (diverging graphs out of 60 random ones per shape, differential fuzz against the pandas oracle): * directed `-[*k..m]->` with min_hops >= 3 (53/60; pandas 0 vs polars 35), and min_hops >= 2 off a FILTERED seed (27-30/60). Plain min_hops <= 2 is fuzz-clean, including as a non-first segment -- that is the graph-bench q3 `-[*1..k]->` shape, still served. * undirected `-[*1..1]-` / `-[*1]-`, the degenerate window: 60/60, halves the count (pandas 36 vs polars 18). `-[*1..2]-` and wider agree, which is why the existing tests missed it. * undirected `-[*1..k]-` that does not start from the full node set: filtered seed 51/60, non-first segment 36/60. Every directed equivalent agrees, so this is specific to the undirected doubled-pair expansion. The gate keys on an EXPLICIT var-length window rather than on `EdgeSemantics.is_multihop`, because `-[*1..1]-` resolves to min == max == 1 and so is NOT multihop -- yet pandas still routes it through the var-length hop (`-[]-` gives 18 on a graph where `-[*1..1]-` gives 36). Plain `-[]-` is untouched. Tests: explicit decline + still-served neighbours for each of the three, plus a seeded differential fuzz over random small graphs (cyclic, parallel edges, self-loops) that asserts parity-or-raise AND that enough shapes are still served for the check to mean anything. Bounded windows only: undirected unbounded shapes through the pandas oracle can exhaust the box. The new test file is REGISTERED in bin/test-polars.sh. That list is an explicit allowlist, and the polars lane is the only CI lane with polars installed -- so the only lane that can execute this file at all. Without the registration the three `return None` decline statements were the only changed lines no lane ever ran (10/13 = 76.92%, exactly the changed-line-coverage gate failure); with it the block is 13/13. Mutation-checked: dropping the min_hops clause fails 4 tests, the degenerate undirected window 3, the undirected-seed clause 3, the whole gate 8. Full graphistry/tests/compute is unchanged by the declines (9 pre-existing failures, same set). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
lmeyerov
added a commit
that referenced
this pull request
Jul 28, 2026
…pes (#1787) (#1794) * fix(gfql): decline three silently-wrong bounded var-length polars shapes (#1787) Pandas is the oracle and the contract is parity-or-NotImplementedError. Three BOUNDED var-length shapes in the native polars `rows(binding_ops=...)` path returned a DIFFERENT count with no error. Same root-cause family as the unbounded case #1781 declined: pandas' step_pairs come from the var-length `edge_op.execute` hop, whose hop-window pruning -- and, when seeded, its per-seed BFS -- changes the edge multiplicity in a way a rebuild from the raw matching edge table does not reproduce. Declined (diverging graphs out of 60 random ones per shape, differential fuzz against the pandas oracle): * directed `-[*k..m]->` with min_hops >= 3 (53/60; pandas 0 vs polars 35), and min_hops >= 2 off a FILTERED seed (27-30/60). Plain min_hops <= 2 is fuzz-clean, including as a non-first segment -- that is the graph-bench q3 `-[*1..k]->` shape, still served. * undirected `-[*1..1]-` / `-[*1]-`, the degenerate window: 60/60, halves the count (pandas 36 vs polars 18). `-[*1..2]-` and wider agree, which is why the existing tests missed it. * undirected `-[*1..k]-` that does not start from the full node set: filtered seed 51/60, non-first segment 36/60. Every directed equivalent agrees, so this is specific to the undirected doubled-pair expansion. The gate keys on an EXPLICIT var-length window rather than on `EdgeSemantics.is_multihop`, because `-[*1..1]-` resolves to min == max == 1 and so is NOT multihop -- yet pandas still routes it through the var-length hop (`-[]-` gives 18 on a graph where `-[*1..1]-` gives 36). Plain `-[]-` is untouched. Tests: explicit decline + still-served neighbours for each of the three, plus a seeded differential fuzz over random small graphs (cyclic, parallel edges, self-loops) that asserts parity-or-raise AND that enough shapes are still served for the check to mean anything. Bounded windows only: undirected unbounded shapes through the pandas oracle can exhaust the box. The new test file is REGISTERED in bin/test-polars.sh. That list is an explicit allowlist, and the polars lane is the only CI lane with polars installed -- so the only lane that can execute this file at all. Without the registration the three `return None` decline statements were the only changed lines no lane ever ran (10/13 = 76.92%, exactly the changed-line-coverage gate failure); with it the block is 13/13. Mutation-checked: dropping the min_hops clause fails 4 tests, the degenerate undirected window 3, the undirected-seed clause 3, the whole gate 8. Full graphistry/tests/compute is unchanged by the declines (9 pre-existing failures, same set). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB * test(gfql): engine-parametrize the #1787 varlen contract (pandas/polars/cuDF/polars-gpu) Review follow-ups on #1794. Comments -> tests: the gate in row_pipeline.py carried per-shape prose asserting which shapes diverge and by how much. A comment asserting behaviour is unverified and rots. Every numeric claim is now a named test; the comment keeps only the WHY of the deliberate divergence from master's serving decision plus a pointer to the tests. 29 -> 18 comment lines, executable logic byte-identical (the only non-comment hunks are two trailing comments on existing lines). Engine-parametrized: renamed to test_varlen_bounded_engine_parity_1787.py and parametrized over pandas / polars / cuDF / polars-gpu. It encodes the INTENDED per-engine behaviour rather than assuming identity -- the polars engines must decline exactly the shapes the pandas-API engines must answer -- with oracle counts pinned as literals so a regression that makes every engine equally wrong still fails. GPU params gate on a runtime probe and report a reasoned SKIP, never a silent pass; the coverage boundary (no CI lane runs cuDF or polars-gpu) is stated in the module docstring. Dropping the module-level polars importorskip also means the pandas params now execute in test-gfql-core, where the old file was skipped in its entirety (re: #1795). Found while doing so: cuDF silently returns 1 where pandas returns 9 on seeded undirected degenerate `(a {p:v})-[*1..1]-(b)` (23/40 random graphs; drop any one of seeded/undirected/max==1 and it agrees). Separate defect, filed as #1798 and pinned here with xfail(strict=True). Mutation-checked both directions: reverting the gate to 233b64c fails 13 tests; widening it to decline all bounded var-length fails 12. Verified: dgx-spark GB10 / cuDF 26.02.01 / polars 1.35.2 via graphistry/test-rapids-official:26.02-gfql-polars with --gpus all -- 80 passed, 1 xfailed, 0 skipped. CI-equivalent local polars lane (cuDF import-blocked) 2395 passed, 54 skipped, 0 failed. bin/lint.sh and bin/typecheck.sh clean. Note: the accompanying CI fix for the test-polars py3.12 timeout (#1797) touches .github/workflows/ci.yml and could not be pushed with the available token, which lacks the `workflow` OAuth scope. The patch is attached to the PR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes part of #1709.
The gap
rows(binding_ops=...)— the Cypher multi-alias bindings table — already lowered natively on polars for fixed-length hops, bounded directed-[*i..k]->, bounded undirected-[*1..k]-, and node-cartesian patterns. An unbounded variable-length segment still declined:That was the last shape blocking
engine='polars'on LDBC SNB interactive-short-6 (MATCH (m:Message)-[:REPLY_OF*0..]->(p:Post)<-[:CONTAINER_OF]-(f:Forum)-[:HAS_MODERATOR]->(mod)) — the one interactive-short query polars could not answer at all.Now native
Unbounded DIRECTED fixed point:
-[*]->,-[*0..]->,<-[*]-,-[:TYPE*]->, with or without a fixed-hop tail, seeded or unseeded.Pandas finds the traversal depth by expanding the path frontier until it is empty, which materializes every partial path at every hop. This lowering splits that in two:
D(each hop is O(N), not O(paths));-[*1..k]->arm runs, withmax_hops = D.Step 1 is exact, not an approximation: the path frontier at hop
his non-empty iff some walk of lengthhleaves a seed, and deduping by endpoint node changes no walk's existence — only its multiplicity, which step 2 then reproduces in full (pairsis never deduped, so parallel edges multiply per hop exactly as the pandas merge does). The emitted rows are pandas-identical while the exponential path expansion happens once, lazily. No pandas bridge, noto_pandas()round trip.Non-termination: a cycle reachable from a seed means infinitely many paths. Pandas raises E108 "Cypher multi-alias row bindings currently require terminating variable-length segments"; so do we, via the same shared helper, with the same message. Detection is by pigeonhole on the distinct nodes touched by the matched edges (a walk of
Nedges visitsN+1nodes, so a non-empty frontier at hopNis a reachable cycle) — i.e. before the blow-up rather than after it, which is the one place we deliberately beat pandas' cost while landing on the identical outcome.The bounded
-[*1..k]->arm and the new arm now literally share one loop (_directed_varlen_reachable_polars) rather than two copies kept in sync by a comment.Also fixed: a latent silent-wrong on
-[*0..k]->Found by the differential fuzz while adding the above, pre-existing on master:
The polars bindings builder rebuilt its table from the chain output, while the pandas oracle rebuilds from the pre-chain base graph. The traversal prunes to what it considers matched, and that is not the bindings builder's match set — a zero-hop variable-length segment binds a seed that has no matching outgoing edge, and the traversal drops that node entirely. So
MATCH (a)-[*0..2]->(b)could silently return fewer rows than pandas: no error, just missing rows.The builder now sources its node/edge tables from the same base graph pandas uses — which the indexed builder in the same function was already being handed. Perf impact measured below: none.
Still declines (honest NIE, never a silent answer)
-[*]-(undirected unbounded)min_hops == 1multiplicity reconstruction and backtrack-aware termination; pandas rejects it outright (GFQLTypeError)-[r*..]->(aliased var-length)to_fixed_point(min_hops=2, no max)len(step_pairs) + 1iterations rather than erroring, and itsstep_pairsrow count is not reconstructible here — so the truncation depth, hence the answer, is not reproduciblemin_hops != 1Separately unchanged:
min_hops > 1withto_fixed_pointstill declines one level up, in_chain_traversal_polars(_is_native_multihop) — a polars hop gap, not arowsgap, and out of scope here.AUTO engine routing is untouched (#1743 stays deferred).
Verification
DGX Spark, RAPIDS image
sha256:74544b1b…, candidate7efc2083vs master84be35fbrun identically.Correctness
84be35fbtests/computeCPUtest_engine_coercion.py::TestChainCoercion::test_chain_dask_edgestests/compute -k cudf--config-file mypy.ini graphistry)mypy delta computed by bucketing both runs on
(file, error-code)and diffing (a raw count is not a delta): no bucket is worse; the reductions arepolars/row_pipeline.pyarg-type 4→0 and assignment 2→0,polars/chain.pyarg-type 1→0, andgfql_fast_paths.pyarg-type 47→32 / assignment 13→3 / attr-defined 5→0 / dict-item 2→0 / union-attr 1→0. The reduction comes from typingfilter_by_dict_polarswith a constrainedDataFrame | LazyFrameTypeVar — it was declared eager and called with both flavours, which was poisoning inference downstream.Differential fuzz vs the pandas oracle — 10 edge shapes (
*0../*1../*2..fixed point, forward + reverse, typed + untyped,to_fixed_pointwith and withoutmax_hops, bounded controls) × 2 pattern tails × random graphs (DAGs and cyclic, with self-loops, parallel and antiparallel edges, integer and string node ids), comparing the projected result table:Before the base-graph fix the same harness reported 171/800 mismatches, all of them the dropped zero-hop rows.
Perf (dgx-spark, exclusive perf lock held for one run at a time, synthetic 440k-node / 420k-edge reply-forest + forum/moderator graph, warm median of 7, all rows verified non-empty):
REPLY_OF*0..walk, polarsNotImplementedErrorREPLY_OF*0..walk, pandasThe last three are the shapes the base-graph rebuild could have regressed; they land within run-to-run noise. This is a synthetic IS6-shaped probe, not an LDBC SF1 measurement — the SF1 harness number should be re-taken by the benchmark lane now that the query runs.
Tests
test_engine_polars_binding_rows.py: 12 new parity queries inSUPPORTED(unbounded forward/reverse/typed/filtered/with-tail, plus the-[*0..2]->zero-hop cases that were silently wrong);-[*]-moved intoDEFERRED; 7 new focused tests — the IS6 pattern end-to-end at both a multi-hop seed and a zero-hop seed, the zero-hop-seed-with-no-outgoing-edge regression pin, path multiplicity over a diamond with parallel edges, cycle and self-loop both raising the E108 diagnosis on both engines, the three decline shapes, and the depth-0 boundary.test_engine_polars_row_pipeline.py: the unbounded-defers test now asserts the directed case is served and the undirected/aliased cases still raise.🤖 Generated with Claude Code
https://claude.ai/code/session_01NxxTFpKiqA4FQa9Pjy8yR1