Skip to content

Commit 370a4c3

Browse files
lmeyerovclaude
andcommitted
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
1 parent 233b64c commit 370a4c3

3 files changed

Lines changed: 192 additions & 0 deletions

File tree

bin/test-polars.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ POLARS_TEST_FILES=(
1818
graphistry/tests/compute/gfql/test_engine_polars_chain.py
1919
graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py
2020
graphistry/tests/compute/gfql/test_engine_polars_binding_rows.py
21+
graphistry/tests/compute/gfql/test_engine_polars_varlen_divergence_1787.py
2122
graphistry/tests/compute/gfql/test_engine_polars_with_match_reentry.py
2223
graphistry/tests/compute/gfql/test_engine_polars_cypher_conformance.py
2324
graphistry/tests/compute/gfql/test_engine_polars_conformance_matrix.py

graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1312,6 +1312,52 @@ def _names(lf: pl.LazyFrame) -> List[str]:
13121312
)
13131313
if _resolved_min != 1:
13141314
return None
1315+
# #1787: three more BOUNDED var-length shapes this raw-edge reconstruction gets
1316+
# SILENTLY wrong, all found by differential fuzzing against the pandas oracle
1317+
# over random small graphs (the `n/60` counts below are diverging graphs out of
1318+
# 60 random ones per shape). Same root-cause family as the unbounded case #1781
1319+
# declined: pandas' step_pairs come from the var-length `edge_op.execute` hop,
1320+
# whose hop-window pruning -- and, when seeded, its per-seed BFS -- changes the
1321+
# edge multiplicity in a way a rebuild from the raw matching edge table does not
1322+
# reproduce. Pandas is the oracle and the contract is parity-or-NotImplementedError,
1323+
# so decline until the multiplicity is reconstructible.
1324+
#
1325+
# Gated on an EXPLICIT var-length window, not on `sem.is_multihop`: the degenerate
1326+
# `-[*1..1]-` resolves to min == max == 1 and so is NOT multihop, yet pandas still
1327+
# routes it through the var-length hop (`-[]-` gives 18 where `-[*1..1]-` gives 36
1328+
# on the same graph). That is exactly why the existing tests miss it.
1329+
if op.min_hops is not None or op.max_hops is not None:
1330+
_vl_max = op.max_hops if op.max_hops is not None else op.hops
1331+
_vl_min = op.min_hops if op.min_hops is not None else (
1332+
op.hops if op.hops is not None else 1
1333+
)
1334+
# A seed is anything that starts the segment from less than the whole node
1335+
# set: a filtered start alias, or a re-entry / `WITH` seed frame.
1336+
_prev_op = ops[idx - 1] if idx >= 1 else None
1337+
_seeded_start = start_nodes is not None or (
1338+
isinstance(_prev_op, ASTNode) and bool(_prev_op.filter_dict)
1339+
)
1340+
if _vl_max is not None:
1341+
if op.direction == "undirected":
1342+
# - `-[*1..1]-` / `-[*1]-`: the DEGENERATE window halves the count
1343+
# (60/60; pandas 36 vs polars 18). `-[*1..2]-` and wider agree.
1344+
if _vl_max == 1:
1345+
return None
1346+
# - undirected `-[*1..k]-` OFF THE FULL NODE SET: the doubled-pair
1347+
# expansion over-counts when the segment does not start from every
1348+
# node -- filtered seed 51/60, non-first segment 36/60. Every
1349+
# DIRECTED equivalent agrees, so this is specific to the doubling.
1350+
if _seeded_start or idx > 1:
1351+
return None
1352+
# - directed `-[*k..m]->` with min_hops >= 3 (53/60; pandas 0 vs polars
1353+
# 35), and min_hops >= 2 WITH a filtered seed (27-30/60).
1354+
# `max_reached_hop` in compute/hop.py is a dedup-by-node BFS
1355+
# eccentricity, not a longest-walk length, so pandas returns empty (or
1356+
# prunes) where this rebuild expands a different edge multiset.
1357+
# min_hops <= 2 off the full node set is fuzz-clean (0/60, including as
1358+
# a non-first segment) -- that is the graph-bench q3 `-[*1..k]->` shape.
1359+
elif _vl_min >= 3 or (_vl_min >= 2 and _seeded_start):
1360+
return None
13151361
if op.direction not in ("forward", "reverse", "undirected"):
13161362
return None
13171363
if any(
Lines changed: 145 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
"""Native polars var-length binding rows: parity with the pandas oracle, or decline (#1787).
2+
3+
Three bounded shapes returned a DIFFERENT count from pandas with no error -- the contract is
4+
parity-or-``NotImplementedError``, so they are now declined, mirroring what #1781 did for the
5+
unbounded case:
6+
7+
1. directed ``-[*k..m]->`` with ``min_hops >= 3`` (and ``min_hops >= 2`` off a filtered seed);
8+
2. undirected ``-[*1..1]-`` / ``-[*1]-``, the degenerate window, which HALVED the count;
9+
3. undirected ``-[*1..k]-`` that does not start from the full node set (filtered seed, or a
10+
non-first segment), which OVER-counted.
11+
12+
The differential fuzz at the bottom is what found all three (and what proves the gate is not
13+
merely declining everything): it asserts served shapes still equal pandas AND that the
14+
neighbouring shapes are still served, so a lazily over-broad gate fails it.
15+
"""
16+
import random
17+
18+
import pandas as pd
19+
import pytest
20+
21+
import graphistry
22+
23+
pl = pytest.importorskip("polars")
24+
25+
26+
def _pair(nodes, edges):
27+
return (
28+
graphistry.nodes(nodes, "id").edges(edges, "s", "d"),
29+
graphistry.nodes(pl.from_pandas(nodes), "id").edges(pl.from_pandas(edges), "s", "d"),
30+
)
31+
32+
33+
def _count(g, query, engine):
34+
return int(g.gfql(query, engine=engine)._nodes["c"].to_list()[0])
35+
36+
37+
# The fixtures from the issue, one per divergence.
38+
BOUNDED_NODES = pd.DataFrame({"id": list(range(7))})
39+
BOUNDED_EDGES = pd.DataFrame(
40+
[(4, 6), (0, 4), (1, 2), (4, 6), (5, 6), (4, 5), (4, 6)], columns=["s", "d"]
41+
)
42+
UNDIR_NODES = pd.DataFrame({"id": list(range(5))})
43+
UNDIR_EDGES = pd.DataFrame(
44+
[(1, 2), (1, 2), (0, 2), (1, 0), (4, 2), (3, 2), (3, 4), (0, 3), (1, 2)], columns=["s", "d"]
45+
)
46+
SEEDED_NODES = pd.DataFrame({"id": list(range(7)), "kind": ["a", "b"] * 3 + ["a"]})
47+
SEEDED_EDGES = pd.DataFrame(
48+
[(0, 3), (6, 3), (4, 2), (1, 3), (6, 3), (3, 1), (3, 5), (4, 1), (3, 3), (0, 3)],
49+
columns=["s", "d"],
50+
)
51+
52+
DECLINED = [
53+
# (nodes, edges, query) -- each returned a wrong count before the gate.
54+
(BOUNDED_NODES, BOUNDED_EDGES, "MATCH (a)-[*3..3]->(b) RETURN count(*) AS c"),
55+
(BOUNDED_NODES, BOUNDED_EDGES, "MATCH (a)-[*3..4]->(b) RETURN count(*) AS c"),
56+
(SEEDED_NODES, SEEDED_EDGES, "MATCH (a {kind:'a'})-[*2..3]->(b) RETURN count(*) AS c"),
57+
(UNDIR_NODES, UNDIR_EDGES, "MATCH (a)-[*1..1]-(b) RETURN count(*) AS c"),
58+
(UNDIR_NODES, UNDIR_EDGES, "MATCH (a)-[*1]-(b) RETURN count(*) AS c"),
59+
(SEEDED_NODES, SEEDED_EDGES, "MATCH (a {kind:'a'})-[*1..2]-(b) RETURN count(*) AS c"),
60+
(SEEDED_NODES, SEEDED_EDGES, "MATCH (a)-[]->(b)-[*1..2]-(c) RETURN count(*) AS c"),
61+
]
62+
63+
STILL_SERVED = [
64+
# Neighbours of each declined shape: the gate must not swallow these.
65+
(BOUNDED_NODES, BOUNDED_EDGES, "MATCH (a)-[*1..2]->(b) RETURN count(*) AS c"),
66+
(BOUNDED_NODES, BOUNDED_EDGES, "MATCH (a)-[*2..3]->(b) RETURN count(*) AS c"),
67+
(UNDIR_NODES, UNDIR_EDGES, "MATCH (a)-[]-(b) RETURN count(*) AS c"),
68+
(UNDIR_NODES, UNDIR_EDGES, "MATCH (a)-[*1..2]-(b) RETURN count(*) AS c"),
69+
(UNDIR_NODES, UNDIR_EDGES, "MATCH (a)-[*1..3]-(b) RETURN count(*) AS c"),
70+
(SEEDED_NODES, SEEDED_EDGES, "MATCH (a {kind:'a'})-[*1..2]->(b) RETURN count(*) AS c"),
71+
(SEEDED_NODES, SEEDED_EDGES, "MATCH (a)-[]->(b)-[*2..3]->(c) RETURN count(*) AS c"),
72+
]
73+
74+
75+
@pytest.mark.parametrize("nodes,edges,query", DECLINED)
76+
def test_divergent_varlen_shapes_decline(nodes, edges, query):
77+
"""A shape that cannot be reproduced must RAISE, never answer differently."""
78+
_, g_pl = _pair(nodes, edges)
79+
with pytest.raises(NotImplementedError):
80+
g_pl.gfql(query, engine="polars")
81+
82+
83+
@pytest.mark.parametrize("nodes,edges,query", STILL_SERVED)
84+
def test_neighbouring_varlen_shapes_still_served_and_match_pandas(nodes, edges, query):
85+
"""The gate is a scalpel: every neighbouring shape still runs natively AND matches pandas."""
86+
g_pd, g_pl = _pair(nodes, edges)
87+
assert _count(g_pl, query, "polars") == _count(g_pd, query, "pandas")
88+
89+
90+
@pytest.mark.parametrize("nodes,edges,query", DECLINED)
91+
def test_declined_shapes_still_answerable_on_pandas(nodes, edges, query):
92+
"""Declining is an ENGINE limit, not a query rejection: pandas still answers all of them."""
93+
g_pd, _ = _pair(nodes, edges)
94+
assert _count(g_pd, query, "pandas") >= 0
95+
96+
97+
FUZZ_SHAPES = [
98+
"MATCH (a)-[*1..2]->(b) RETURN count(*) AS c",
99+
"MATCH (a)-[*2..2]->(b) RETURN count(*) AS c",
100+
"MATCH (a)-[*2..3]->(b) RETURN count(*) AS c",
101+
"MATCH (a)-[*3..3]->(b) RETURN count(*) AS c",
102+
"MATCH (a)-[*1..1]-(b) RETURN count(*) AS c",
103+
"MATCH (a)-[*1..2]-(b) RETURN count(*) AS c",
104+
"MATCH (a)-[*1..3]-(b) RETURN count(*) AS c",
105+
"MATCH (a {kind:'a'})-[*1..2]-(b) RETURN count(*) AS c",
106+
"MATCH (a {kind:'a'})-[*1..2]->(b) RETURN count(*) AS c",
107+
"MATCH (a {kind:'a'})-[*2..3]->(b) RETURN count(*) AS c",
108+
"MATCH (a)-[]->(b)-[*1..2]-(c) RETURN count(*) AS c",
109+
"MATCH (a)-[]->(b)-[*2..3]->(c) RETURN count(*) AS c",
110+
]
111+
112+
113+
def test_bounded_varlen_differential_fuzz_against_pandas_oracle():
114+
"""Random small graphs (cyclic, parallel edges, self-loops): match pandas or raise.
115+
116+
Bounded windows only -- undirected UNBOUNDED shapes through the pandas oracle can blow the
117+
box up (75 GiB RSS observed), and they are already declined elsewhere. Seeded so a failure
118+
is reproducible; the shape list spans both sides of every gate boundary, so a gate that
119+
declines too much shows up as the served-count assertion below going to zero.
120+
"""
121+
rnd = random.Random(1787)
122+
served = 0
123+
for _ in range(25):
124+
n_nodes = rnd.randint(3, 7)
125+
nodes = pd.DataFrame({
126+
"id": list(range(n_nodes)),
127+
"kind": [rnd.choice("ab") for _ in range(n_nodes)],
128+
})
129+
edges = pd.DataFrame(
130+
[(rnd.randrange(n_nodes), rnd.randrange(n_nodes)) for _ in range(rnd.randint(3, 10))],
131+
columns=["s", "d"],
132+
)
133+
g_pd, g_pl = _pair(nodes, edges)
134+
for query in FUZZ_SHAPES:
135+
expected = _count(g_pd, query, "pandas")
136+
try:
137+
got = _count(g_pl, query, "polars")
138+
except NotImplementedError:
139+
continue # an honest decline is allowed; a different answer is not
140+
served += 1
141+
assert got == expected, (
142+
f"polars diverged from the pandas oracle on {query!r}: "
143+
f"{got} != {expected}\nnodes={nodes.to_dict('list')}\nedges={edges.values.tolist()}"
144+
)
145+
assert served > 100, f"gate declines too much to be a useful parity check ({served})"

0 commit comments

Comments
 (0)