Skip to content

Commit f913f1a

Browse files
lmeyerovclaude
andcommitted
test(gfql): run the #1788/#1790 suites on all four engines, in a lane that exists (CB3 a+b)
Both suites tested the semantics of WHICH TABLE a query returns — engine-agnostic semantics — on pandas + polars only, and neither file was in `bin/test-polars.sh`. `test_rows_table_named_middle.py` also carried a module-level `pytest.importorskip("polars")`, and the `test-gfql-core` lane installs no polars, so that file ran in NO CI LANE AT ALL. `test_rewrite_param_discard.py` named cuDF but skipped polars-gpu outright. Now: fixed engine list (pandas / cuDF / polars / polars-gpu), classified availability check, both files in the polars lane, module-level importorskip dropped (5 pandas cases become live in test-gfql-core). TWO PROBE TRAPS, BOTH HIT BY TRYING RATHER THAN REASONED ABOUT: * A probe that runs THE SHAPE UNDER TEST disarms its own file. Reverting the #1788 `table` guard made the probe raise, and all 20 parameters reported SKIPPED instead of failing. The smoke query is now a plain traversal with no `rows()` call in it. * A probe that SWALLOWS every exception is worse than none. A transient `MemoryError: ... cudaErrorMemoryAllocation` in a fresh GPU container silently dropped cuDF from a run that otherwise passed — and a skipped GPU parameter reads as evidence of passing. `engine_skip_reason` now CLASSIFIES: a missing module skips, a recognisable GPU-stack error skips with its text quoted, and ANY other failure propagates. It is deliberately not `available_nonpandas_engines()`, which builds its list by importability, so a missing engine vanishes from the report rather than showing as SKIPPED. GPU RECEIPTS — dgx GB10, `graphistry/test-rapids-official:26.02-gfql-polars`, `docker run --gpus all`, cuDF 26.02.01 / polars 1.35.2: 45 passed, 3 xfailed, 0 skipped (repeated; all four engines running) MUTATION-CHECKED on the same box by reverting the #1788 and #1790 `table` guards: 24 failed / 0 skipped, spread EVENLY over the four engines (6 each) so the added parameters are not decorative. TWO REAL GAPS SURFACED BY THE NEW PARAMETERS, pinned strict-xfail, not papered over: * #1803 — the indexed bindings bypass gate admits only (PANDAS, CUDF, POLARS), while the native polars chain hands it `Engine.POLARS_GPU` whenever the GPU target is active. So polars-gpu ALWAYS reports `served: False, reason: 'unsupported_engine'` and silently runs the canonical scan. Values are unaffected; the optimization is lost with no signal. * #1804 — the native polars bindings builder never receives `alias_prefilters` (5 rows on pandas/cuDF vs 12 on polars). Already xfailed before this change, but with no issue number, so there was nothing to close and nothing to find. RUNTIME DELTA: zero. No production line changed — the diff is two test files, a shared test helper, `bin/test-polars.sh` and the CHANGELOG — so no pyg-bench lane run is required (CB5), and that claim is checkable from the diff. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
1 parent 233b64c commit f913f1a

5 files changed

Lines changed: 232 additions & 20 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
2424
- **Seeded typed-hop property projection keeps its fast path through DISTINCT / ORDER BY / SKIP / LIMIT**: those trailing row ops are plain frame operations, so the fast path now delegates them to the canonical chain instead of declining the whole query.
2525

2626
### Changed
27+
- **`rows(table=...)` and rewrite-param regression suites now run on all four engines, and their polars params run in a lane at all** (tests only; zero runtime delta — no production line changed). The #1788 and #1790 suites were parametrized over pandas + polars only, and neither file was in `bin/test-polars.sh`, so `test_rows_table_named_middle.py` — which carried a module-level `pytest.importorskip("polars")` — ran in **no CI lane at all** (the `test-gfql-core` lane installs no polars, so the whole file skipped there). Both files are now parametrized over pandas / cuDF / polars / polars-gpu from a **fixed** engine list plus a classified availability check, and both are in the polars lane; dropping the module-level `importorskip` also makes 5 pandas cases live in `test-gfql-core`. The availability check is deliberately not `available_nonpandas_engines()`, which builds its list by importability so a missing engine vanishes from the report instead of showing as SKIPPED, and it is deliberately not a blanket `except Exception` either: a missing module skips, a recognisable GPU-stack error skips **with its text quoted**, and any other failure propagates — because a skipped GPU parameter reads as evidence of passing. Both traps were hit by trying rather than reasoned about: a probe that ran the shape under test turned a reverted production guard into 20 SKIPS instead of 20 failures, and a blanket probe silently dropped cuDF from an otherwise-green GPU run on a transient `cudaErrorMemoryAllocation`. GPU receipts on dgx GB10 / cuDF 26.02.01 / polars 1.35.2 / `docker run --gpus all`: **45 passed, 3 xfailed, 0 skipped**; mutation-checked by reverting the #1788 and #1790 `table` guards — **24 failures spread evenly over all four engines (6 each)**, so the added parameters are not decorative. Two gaps the new parameters surfaced are pinned as strict xfails rather than papered over: #1803 (the indexed bindings bypass excludes `Engine.POLARS_GPU`, so polars-gpu silently takes the scan path) and #1804 (the native polars bindings builder never receives `alias_prefilters` — previously an unnumbered xfail).
2728
- **`is_lazy` is a type predicate, so the polars engine's eager/lazy split is checked instead of asserted**: `dtypes.is_lazy` decides WHICH member of the two-member `PolarsFrame` union a frame is, but it was declared `-> bool`, so that answer was discarded at the call boundary — an eager-only attribute (`.height`, `.columns`, `.schema`) reached after a lazy guard was unprovable, and the native polars chain closed the gap with `cast("pl.DataFrame", frame)`. A cast is not a type: it re-asserts, unchecked, the exact fact the predicate had just established, once per call site, and a wrong one surfaces as a production `AttributeError` rather than a CI error. `is_lazy` is now declared `-> TypeIs["pl.LazyFrame"]` (PEP 742), which narrows the *negative* branch as well as the positive — the eager side is the one that needed it, so `TypeGuard` would not have done — and the cast is gone. Four previously untyped chain-combine helpers (`_semi`, `_combine_edges`, `_apply_node_names`, and the `_known_empty` guard) now carry real signatures: `_semi`'s two frames share the `PolarsT` TypeVar because polars joins do not mix eagerness, and the two combine helpers take the `_LazyShim` collect-once duck-type they are actually called with. Typing `_apply_node_names` also retired a `getattr(next_step, "edges_empty", None)` — the shim declares that tri-state in `__slots__`, so a typo is now a checker error instead of a silent `None` that would have re-armed the very cardinality gate the guard disarms. Internal only: no runtime behaviour, no public API, and the `TypeIs` import is `TYPE_CHECKING`-only, so no new runtime dependency floor.
2829
- **GFQL execution context is declared rather than attached at runtime**: the private `_gfql_*` per-execution fields (index policy and registry, row-pipeline base graph, carried seed nodes, edge aliases, shortest-path backend, and the indexed-bindings handoff) are now declared on `Plottable` with defaults on `PlotterBase`, instead of being set with `setattr` and read back with `getattr(..., default)`. No public API or behaviour change; internal call sites are typed, and hand-rolled `Plottable` stand-ins must now construct the full context.
2930

bin/test-polars.sh

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ POLARS_TEST_FILES=(
1919
graphistry/tests/compute/gfql/test_engine_polars_row_pipeline.py
2020
graphistry/tests/compute/gfql/test_engine_polars_binding_rows.py
2121
graphistry/tests/compute/gfql/test_engine_polars_with_match_reentry.py
22+
# #1788 / #1790 regression suites: engine-parametrized over pandas/cuDF/polars/polars-gpu,
23+
# so their POLARS params only ever run here (test-gfql-core installs no polars)
24+
graphistry/tests/compute/gfql/test_rows_table_named_middle.py
25+
graphistry/tests/compute/gfql/test_rewrite_param_discard.py
2226
graphistry/tests/compute/gfql/test_engine_polars_cypher_conformance.py
2327
graphistry/tests/compute/gfql/test_engine_polars_conformance_matrix.py
2428
graphistry/tests/compute/gfql/test_polars_string_predicate_nonstring.py

graphistry/tests/compute/gfql/polars_test_utils.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,3 +181,58 @@ def assert_surfaces_agree(res_a, res_b, label):
181181
assert res_a[0] == res_b[0], f"{label}: surface divergence {res_a[0]} != {res_b[0]} (silent-bridge class)"
182182
if res_a[0] == "ok":
183183
assert res_a[1] == res_b[1], f"{label}: surfaces both ok but signatures differ"
184+
185+
186+
# --- fixed-engine-list plumbing (#1795 / CB3). ONE definition, because the alternative is the
187+
# trap this replaces: `available_nonpandas_engines()` above BUILDS its list by importability, so
188+
# an engine that is missing simply vanishes from the report instead of showing as SKIPPED. ---
189+
190+
#: Substrings that identify a broken/absent GPU STACK rather than a product defect. Matched
191+
#: against `type(ex).__name__: str(ex)`, lowercased. Deliberately narrow: everything not listed
192+
#: here is treated as a REAL failure, because the cost of guessing wrong in that direction is a
193+
#: silently green GPU lane.
194+
_GPU_ENVIRONMENT_MARKERS = (
195+
"libnvrtc", "libcuda", "libcudart", "nvrtc", "cuinit",
196+
"no cuda-capable device", "cuda driver", "cuda runtime",
197+
"cuda_error", "cudaerror", "driver version",
198+
"gpu engine requested", # polars' own message when cudf_polars is absent
199+
)
200+
201+
202+
def gpu_environment_reason(ex: BaseException) -> "str | None":
203+
"""Return a skip reason iff ``ex`` is an ENVIRONMENT fact, else None (= a real failure).
204+
205+
A probe that swallows every exception is worse than no probe: it converts a genuine
206+
regression into a SKIP, and a skipped GPU parameter reads as evidence of passing. Measured
207+
on the dgx box — a probe that ran the shape under test turned a reverted production guard
208+
into 20 skips instead of 20 failures, and an intermittent cold-start error in a fresh
209+
container silently dropped cuDF from a run that otherwise passed.
210+
"""
211+
if isinstance(ex, ImportError):
212+
return f"{type(ex).__name__}: {ex}"
213+
text = f"{type(ex).__name__}: {ex}".lower()
214+
if any(marker in text for marker in _GPU_ENVIRONMENT_MARKERS):
215+
return f"{type(ex).__name__}: {ex}"
216+
return None
217+
218+
219+
def engine_skip_reason(engine, smoke):
220+
"""``None`` => this engine MUST run here. A string => a stated, checkable skip reason.
221+
222+
``smoke`` is a zero-argument callable running a query that is NOT the shape under test —
223+
running the asserted shape would let a regression disarm its own test.
224+
"""
225+
import importlib.util
226+
227+
for module in {"polars": ("polars",), "polars-gpu": ("polars", "cudf_polars"),
228+
"cudf": ("cudf",)}.get(engine, ()):
229+
if importlib.util.find_spec(module) is None:
230+
return f"{module} is not installed"
231+
try:
232+
smoke()
233+
except BaseException as ex: # noqa: BLE001 — classified below, never blanket-swallowed
234+
reason = gpu_environment_reason(ex)
235+
if reason is not None:
236+
return reason
237+
raise
238+
return None

graphistry/tests/compute/gfql/test_rewrite_param_discard.py

Lines changed: 90 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,36 @@
2424
``attach_prop_aliases`` — the #1711 projection pushdown — that is observable: naming the
2525
middle instead of spelling ``binding_ops`` by hand silently attaches EVERY alias's
2626
properties. Same query, same bindings, different output schema.
27+
28+
ENGINE LIST IS FIXED, NOT PROBED-INTO-EXISTENCE. ``polars-gpu`` joins the three engines this
29+
file already carried: it is the GPU collect target of the SAME native polars chain, so it
30+
runs the same rewrite and the same indexed bypass, and it must therefore land on the same
31+
side of every gate polars does — including the strict xfail at the bottom. An engine that
32+
cannot run in the current environment reports SKIPPED with a reason; it never vanishes from
33+
the report (the trap in ``polars_test_utils.available_nonpandas_engines()``, which silently
34+
shrinks). The probe RUNS a query rather than importing a module, because a box where
35+
``cudf``/``cudf_polars`` import against an incomplete CUDA runtime passes every import check
36+
and then dies inside the first real kernel — which reads as a product failure rather than a
37+
missing environment.
38+
39+
COVERAGE BOUNDARY, stated rather than hidden behind a skip: no CI lane runs cuDF or
40+
polars-gpu (``ci-gpu.yml`` is hard-disabled and does not install the ``polars`` extra), so
41+
those two parameters report SKIPPED on CI. They are exercised out of band on the dgx GPU box
42+
against ``graphistry/test-rapids-official:26.02-gfql-polars`` (``docker run --gpus all`` —
43+
omitting ``--gpus all`` FABRICATES failures rather than skipping). Treat a green CI run as
44+
evidence for pandas + polars only.
2745
"""
2846
from __future__ import annotations
2947

30-
from typing import Any, List
48+
from functools import lru_cache
49+
from typing import Any, List, Optional, Tuple
3150

3251
import numpy as np
3352
import pandas as pd
3453
import pytest
3554

3655
import graphistry
56+
from graphistry.tests.compute.gfql.polars_test_utils import engine_skip_reason
3757
from graphistry.compute.ast import (
3858
ASTObject, e_forward, e_undirected, n, rows, select, serialize_binding_ops,
3959
)
@@ -53,11 +73,13 @@
5373
# comes back — so the tests below pin the SMALL number, not merely "not empty".
5474
ALL_EDGES = len(EDGES)
5575

56-
ENGINES = ["pandas", "polars", "cudf"]
76+
PANDAS_API_ENGINES: Tuple[str, ...] = ("pandas", "cudf")
77+
POLARS_API_ENGINES: Tuple[str, ...] = ("polars", "polars-gpu")
78+
ENGINES: Tuple[str, ...] = PANDAS_API_ENGINES + POLARS_API_ENGINES
5779

5880

5981
def _frames(engine: str) -> Any:
60-
if engine == "polars":
82+
if engine in POLARS_API_ENGINES:
6183
pl = pytest.importorskip("polars")
6284
return pl.from_pandas(NODES), pl.from_pandas(EDGES)
6385
if engine == "cudf":
@@ -72,6 +94,38 @@ def _graph(engine: str, *, indexed: bool) -> Any:
7294
return g.gfql_index_all(engine=engine) if indexed else g
7395

7496

97+
@lru_cache(maxsize=None)
98+
def _engine_skip_reason(engine: str) -> Optional[str]:
99+
"""``None`` => this engine MUST run here; a string => a stated, checkable skip reason.
100+
101+
Runs BOTH halves this file needs, because the INDEXED path is the one that reaches cupy
102+
kernels: a scan-only smoke test lets an incomplete CUDA install through and reports its
103+
kernel error as a test failure (measured on a box with cudf 25.10 importable and
104+
`libnvrtc.so.12` missing — 4 fabricated failures).
105+
106+
Never the shape under test: a probe that runs it disarms the file — reverting the
107+
indexed-bypass `table` guard turned every parameter into a SKIP instead of a failure. And
108+
never a blanket `except Exception`: a missing module skips, a recognisable GPU-stack error
109+
skips with its text quoted, and ANY other failure propagates, because a skipped GPU
110+
parameter reads as evidence of passing.
111+
"""
112+
def smoke() -> None:
113+
ops = [n({"id": 1}), e_forward(), n()]
114+
_graph(engine, indexed=False).gfql(list(ops), engine=engine)
115+
_graph(engine, indexed=True).gfql(list(ops), engine=engine, index_policy="force")
116+
117+
return engine_skip_reason(engine, smoke)
118+
119+
120+
def _require(engine: str) -> None:
121+
reason = _engine_skip_reason(engine)
122+
if reason is not None:
123+
pytest.skip(
124+
f"engine {engine!r} unavailable here ({reason}) — NOT evidence that it passes; "
125+
"see the COVERAGE BOUNDARY note in this module's docstring"
126+
)
127+
128+
75129
def _run(engine: str, ops: List[ASTObject], *, indexed: bool) -> pd.DataFrame:
76130
kwargs: dict = {"engine": engine}
77131
if indexed:
@@ -100,6 +154,7 @@ def test_indexed_bypass_honours_rows_table_edges(engine: str) -> None:
100154
Indexed and unindexed must agree, and both must be the narrowed set. The absolute
101155
count is pinned because the failure mode is exactly `ALL_EDGES`.
102156
"""
157+
_require(engine)
103158
ops = [n({"id": 1}, name="a"), e_forward(name="r"), n(name="b"), rows(table="edges")]
104159
scan = _run(engine, ops, indexed=False)
105160
indexed = _run(engine, ops, indexed=True)
@@ -114,6 +169,7 @@ def test_indexed_bypass_honours_rows_table_edges(engine: str) -> None:
114169
@pytest.mark.parametrize("engine", ENGINES)
115170
def test_indexed_bypass_honours_rows_table_edges_undirected(engine: str) -> None:
116171
"""Undirected middle: node 1 has four incident edges, not twelve."""
172+
_require(engine)
117173
ops = [n({"id": 1}, name="a"), e_undirected(name="r"), n(name="b"), rows(table="edges")]
118174
scan = _run(engine, ops, indexed=False)
119175
indexed = _run(engine, ops, indexed=True)
@@ -133,6 +189,7 @@ def test_indexed_bypass_table_edges_survives_a_projection(engine: str) -> None:
133189
values can distinguish them — and the indexed path returned every edge type in the
134190
graph rather than the three on node 1's outgoing edges.
135191
"""
192+
_require(engine)
136193
ops = [n({"id": 1}, name="a"), e_forward(name="r"), n(name="b"),
137194
rows(table="edges"), select([("type", "type")])]
138195
scan = _run(engine, ops, indexed=False)
@@ -143,14 +200,33 @@ def test_indexed_bypass_table_edges_survives_a_projection(engine: str) -> None:
143200
)
144201

145202

146-
@pytest.mark.parametrize("engine", ENGINES)
203+
@pytest.mark.parametrize("engine", [
204+
"pandas",
205+
"cudf",
206+
"polars",
207+
pytest.param("polars-gpu", marks=pytest.mark.xfail(
208+
strict=True,
209+
reason="#1803: the indexed bindings bypass excludes Engine.POLARS_GPU "
210+
"(index/bindings.py gate), so polars-gpu always takes the scan path",
211+
)),
212+
])
147213
def test_indexed_bypass_still_serves_a_bare_rows(engine: str) -> None:
148214
"""THE NEGATIVE SIDE: declining on a non-default `table` must not decline everything.
149215
150216
Without this, "never take the bypass" would pass every test above while silently
151217
retiring the indexed bindings path. Asserted on the trace, not on the answer — the
152218
answer is identical either way, which is precisely why the regression would be quiet.
219+
220+
``polars-gpu`` STRICT-xfails, and that is the point of adding the parameter (#1802). The
221+
native polars chain hands `Engine.POLARS_GPU` to `try_indexed_connected_bindings_state`
222+
whenever the GPU target is active, but that function's first gate admits only
223+
`(PANDAS, CUDF, POLARS)` — so the bypass reports `served: False,
224+
reason: 'unsupported_engine'` and polars-gpu silently runs the canonical scan. Values are
225+
unaffected (every other case in this file passes on polars-gpu); what is lost is the
226+
optimization, with no signal. A strict xfail rather than a skip so that whoever widens the
227+
gate is told to delete the marker instead of leaving a stale "known gap" behind.
153228
"""
229+
_require(engine)
154230
from graphistry.compute.gfql.index import index_trace
155231

156232
ops = [n({"id": 1}, name="a"), e_forward(name="r"), n(name="b"), rows()]
@@ -184,6 +260,7 @@ def test_named_middle_rewrite_keeps_attach_prop_aliases(engine: str) -> None:
184260
against the explicit spelling rather than a hardcoded column list, so the assertion
185261
tracks whatever the bindings builder emits.
186262
"""
263+
_require(engine)
187264
binding_ops = serialize_binding_ops(MIDDLE)
188265
rewritten = _run(engine, list(MIDDLE) + [rows(attach_prop_aliases=["a"])], indexed=False)
189266
explicit = _run(engine, [rows(binding_ops=binding_ops, attach_prop_aliases=["a"])],
@@ -206,6 +283,7 @@ def test_named_middle_rewrite_keeps_attach_prop_aliases(engine: str) -> None:
206283
@pytest.mark.parametrize("engine", ENGINES)
207284
def test_named_middle_rewrite_keeps_attach_prop_aliases_values(engine: str) -> None:
208285
"""The pushdown must narrow the SCHEMA without changing the bindings themselves."""
286+
_require(engine)
209287
binding_ops = serialize_binding_ops(MIDDLE)
210288
rewritten = _run(engine, list(MIDDLE) + [rows(attach_prop_aliases=["a"])], indexed=False)
211289
explicit = _run(engine, [rows(binding_ops=binding_ops, attach_prop_aliases=["a"])],
@@ -229,11 +307,15 @@ def test_named_middle_rewrite_keeps_attach_prop_aliases_values(engine: str) -> N
229307

230308
@pytest.mark.parametrize("engine", [
231309
"pandas",
310+
"cudf",
232311
pytest.param("polars", marks=pytest.mark.xfail(
233312
strict=True,
234-
reason="native polars bindings builder never receives alias_prefilters",
313+
reason="#1804: native polars bindings builder never receives alias_prefilters",
314+
)),
315+
pytest.param("polars-gpu", marks=pytest.mark.xfail(
316+
strict=True,
317+
reason="#1804: polars-gpu is the same native builder as polars, same missing param",
235318
)),
236-
"cudf",
237319
])
238320
def test_alias_prefilters_are_honoured_by_the_bindings_builder(engine: str) -> None:
239321
"""`alias_prefilters` narrows the bindings on pandas/cuDF — and is IGNORED on polars.
@@ -242,7 +324,7 @@ def test_alias_prefilters_are_honoured_by_the_bindings_builder(engine: str) -> N
242324
`attach_prop_aliases` only, so the param never reaches it; the generic path threads it
243325
into `_gfql_binding_ops_row_table`. Same query, 5 rows vs 12.
244326
245-
Not fixed here: the hint is documented as ADVISORY (the cypher lowering that emits it
327+
Filed as #1804. Not fixed here: the hint is documented as ADVISORY (the cypher lowering that emits it
246328
always keeps the equivalent post-join filter, so Cypher answers agree across engines
247329
and only the pushdown is lost), and honouring it natively means porting the whole
248330
prefilter evaluator — a feature, not this fix. Hand-written GFQL that passes
@@ -252,6 +334,7 @@ def test_alias_prefilters_are_honoured_by_the_bindings_builder(engine: str) -> N
252334
polars learns the param this turns into a failure, so whoever fixes it is told to
253335
delete the marker instead of leaving a stale "known gap" behind.
254336
"""
337+
_require(engine)
255338
binding_ops = serialize_binding_ops(OPEN_MIDDLE)
256339
unfiltered = _run(engine, [rows(binding_ops=binding_ops)], indexed=False)
257340
filtered = _run(engine, [rows(binding_ops=binding_ops, alias_prefilters=ALIAS_PREFILTER)],

0 commit comments

Comments
 (0)