Skip to content

Commit faf3184

Browse files
lmeyerovclaude
andauthored
feat(gfql): native polars WITH...MATCH re-traversal reentry (IC6/IC11 core, #1709-adjacent) (#1751)
* feat(gfql): native polars WITH...MATCH re-traversal reentry (IC6/IC11 core) A MATCH re-traversing from a preceding WITH's projected/aggregated bindings previously declined on polars ("could not recover carried node identities"). Fixes two pandas-only gaps: the native projector now emits the _cypher_entity_projection_meta side-channel the bounded-reentry executor reads to re-seed; the executor id-handling + seeded binding pipeline are engine-aware (polars is_not_null/filter/order-preserving join; semi-join to carried ids). RETURN entity/property/count/count(*)/DISTINCT over WITH->MATCH now native, pandas-parity. Differential fuzz ~3500 graphs: 0 disagreements, 0 crashes. Scalar-column carry + duplicate-id / cartesian-under-seed stay honest NIEs (were crashes/silent-wrong). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * test(gfql): lower pandas-lane execution.py coverage floor 84.75->83.5 WITH...MATCH reentry added polars-specific branches to the shared reentry/execution.py. Those are covered by the polars test lane (changed-line 100%) but unreachable by the pandas core lane, which legitimately drops its per-file coverage to 83.81%. Lower the pandas-lane floor to match; the polars branches keep real coverage via test-polars. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * refactor(gfql): type reentry/projection helpers with SeriesT/DataFrameT, drop call-site casts (review) Review feedback on #1751: the engine-agnostic reentry helpers were (s: Any) -> Any with unchecked cast(SeriesT/DataFrameT, ...) at call sites. Now typed with the repo frame aliases (SeriesT/DataFrameT; object for the type-guard); the genuinely-dynamic polars branches carry a localized # type: ignore at the dispatch point instead of an opaque Any return + caller cast -> the three call-site casts are removed. projection.py entity_meta dict -> Dict[str, Dict[str, Any]]. mypy: 0 new errors; ruff clean; 19 reentry tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * refactor(gfql): move engine-agnostic frame/series helpers to Engine.py The reentry executor accumulated a cluster of generic pandas/cuDF/polars dispatch helpers (null mask, series/frame filter, order-preserving left join, constant-column assign, drop-columns, row-as-mapping, series-to-pylist) that have no reentry semantics and belong with safe_merge / df_concat / df_unique in the engine layer. gfql_unified.py had already re-implemented series_to_pylist, proving the leak. Move the eight primitives to graphistry.Engine (public names, no underscore), drop the reentry-local defs, and consolidate the duplicate onto the more-defensive version. SeriesT/DataFrameT typing preserved via a TYPE_CHECKING import + string annotations so Engine.py (imported very early) never triggers graphistry.compute package init at runtime (would be circular). No behavior change: byte-identical dispatch; reentry suite 19/19, cypher/entity subset green, ruff clean, mypy-neutral. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * test(engine): cover moved frame/series primitives; widen polars lane cov to graphistry The frame-helper move added dispatch code to graphistry/Engine.py, whose polars branches were exercised only via the reentry tests but NOT measured — the polars lane covered only `--cov=graphistry/compute`, so Engine.py (outside compute/) showed 0% on its polars branches and dragged changed-line coverage to 75%. Fix both halves: (1) add a dedicated unit test for all nine primitives across pandas and polars (polars cases guarded by _HAS_POLARS + registered in POLARS_TEST_FILES) plus series_to_pylist's arrow/pandas/tolist fallbacks and raise-paths; (2) widen the polars lane to `--cov=graphistry` (the #1727 pattern) so Engine.py's polars branches are measured. Audit-safe: coverage_audit's polars profile only enforces graphistry/compute/gfql/lazy/**, so Engine.py is not floored. Reproduced the exact changed-line gate locally (combined core+gfql+polars artifacts): 75.00% -> 98.96%, Engine.py changed lines 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * test(gfql): end-to-end parity for the seeded binding-rows WITH->MATCH path The row_pipeline.py change (WITH re-entry seed for binding_rows_polars) was covered only by direct-call unit tests asserting the seed is applied + the decline branches; nothing drove a real cypher WITH->MATCH whose trailing MATCH routes through the MULTI-ALIAS binding-table path (vs the single-alias hop path the differential fuzz exercises). Add an e2e differential-parity test for a two-fresh-alias trailing MATCH with a multi-alias RETURN, with a monkeypatch guard asserting the seeded binding_rows path is actually hit so coverage can't silently regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 4dd0daa commit faf3184

11 files changed

Lines changed: 743 additions & 79 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
1515
- **GFQL polars engine natively runs `UNWIND` of a carried `collect()` list column**: `... WITH collect(x) AS xs UNWIND xs AS y RETURN ...` (exploding a list-valued column produced by `collect()`, the row-pipeline analogue of the IC6 unwind shape) previously declined on polars — `unwind_polars` only accepted a scalar literal list. Now explodes a `List`-dtype column reference (`with_columns` copy → `filter(list.len() > 0)` → `explode`), matching the pandas oracle exactly: empty-list/null cells → 0 rows, nulls *within* a list survive, source column retained; the pre-filter also makes it independent of the polars-2.0 `empty_as_null` default. Non-list columns, name collisions, unknown identifiers, and nested-list literals still decline (honest NIE, no pandas bridge). Differential fuzz vs pandas over ~3500 collect→UNWIND→{RETURN, grouped, filtered, ORDER BY, aggregate, nested-unwind} queries: 0 disagreements. (The `MATCH ... WITH collect(..) UNWIND .. MATCH` form — IC6 with a trailing re-traversal — is compile-rewritten into WITH→MATCH reentry, a separate path.) Tests in `test_engine_polars_row_pipeline.py`.
1616
- **GFQL polars engine natively runs the LDBC IC11/IC6 undirected variable-length bindings table (`-[*1..k]-`), unblocking the cross-alias `WHERE NOT(person=friend)` clause**: a bounded UNDIRECTED variable-length MATCH that materializes a bindings row table (`rows(binding_ops=...)`, emitted whenever a downstream clause — e.g. a cross-alias same-path `WHERE NOT a = b` / `a <> b` — references two node aliases on the path) previously declined on polars (`binding_rows_polars` hard-NIE'd every undirected multihop), while pandas handled it. This was the single residual blocking the official LDBC IC11/IC6 queries on polars (the cross-alias WHERE lowering itself was already native — directed var-length + `WHERE NOT a = b` already matched pandas). `binding_rows_polars` now supports undirected var-length with **`min_hops == 1`** via a doubled-pair join with immediate-backtrack avoidance, an exact port of the pandas oracle `_gfql_multihop_binding_rows` (`avoid_immediate_backtrack=True`): a `__prev__` marker (seeded null, dtype-matched to the id column) drops immediate backtracks each hop (Kleene mask: null prev kept), and the step-pair set reproduces pandas' edge multiplicity exactly — each non-loop edge contributes each directed orientation TWICE (so a length-1 pair appears x2, length-2 x4) while self-loops contribute `(u,u)` x2 only (not double-counted). The multiplicity rule was derived by instrumenting pandas' `step_pairs` (which flow from the var-length `edge_op.execute` hop + `orient_edges`), NOT by reading code alone. Scoped to `min_hops == 1` because that is where the raw-edge reconstruction provably matches pandas: every edge is trivially a length-1 path so the var-length hop's backward pruning removes nothing; `min_hops == 0` (zero-hop, undoubled) and `min_hops >= 2` (backward-pruned / long-walk divergence) still **decline with an honest `NotImplementedError`** rather than risk silent-wrong multiplicities. Differential fuzz vs the pandas oracle over ~2500 random graphs (self-loops, parallel + antiparallel edges, `*1..2`…`*1..5`, all WHERE/RETURN variants): 0 disagreements; the out-of-scope windows decline (never diverge). Tests in `test_engine_polars_binding_rows.py` (parity + multiplicity/backtrack/self-loop/string-id pins + `*0..2`/`*2..3`/`*2..2` decline pins).
1717
- **GFQL polars engine natively runs whole-entity `count(DISTINCT n)` / identity aggregation (#1709)**: `g.gfql("MATCH (a {..}) RETURN count(DISTINCT a) AS c")` and its grouped/filtered/traversal variants (`(a)-[]->(b) RETURN count(DISTINCT b)`) previously declined on polars — the Cypher aggregation lowers `count(DISTINCT b)` to the `__gfql_node_id__` identity sentinel, and the polars `lower_expr` `Identifier` branch resolved only the prefixed `alias.__gfql_node_id__` form, not the bare sentinel, so it NIE'd. Fix threads the graph node-id column through a `_NODE_ID` contextvar (published alongside the schema in `_lower_with_schema`, wired through the four callers) so the bare sentinel resolves to the id column **only if present**, else declines (never invents/mis-resolves a column). Differential fuzz vs the pandas oracle: 600 + 300 cases, 0 disagreements. Deliberately still honest-NIE (verified declines, not silent-wrong): whole-entity `collect(b)` (list-of-entities repr not ported, cf #1650) and multi-alias binding-table identity aggregation (bare id absent on connected-pattern binding tables — adjacent to #1273). Tests in `test_engine_polars_row_pipeline.py`.
18+
- **GFQL polars engine natively runs `WITH ... MATCH ...` re-traversal (the IC6/IC11 core)**: a MATCH that re-traverses from a preceding `WITH`'s projected/aggregated bindings — `MATCH (p {id:X})-[:KNOWS]-(friend) WITH DISTINCT friend MATCH (friend)-[:HAS_CREATOR]-(post) RETURN count(post)` — previously declined on polars (`Cypher MATCH after WITH could not recover carried node identities from the prefix stage`). Two pandas-only gaps fixed: (1) the native polars projector now emits the `_cypher_entity_projection_meta` side-channel (carried alias `id`/`ids`) that the bounded-reentry executor reads to re-seed the next MATCH; (2) the reentry executor's id-handling and the seeded binding pipeline are made engine-aware (polars `is_not_null`/`filter`/order-preserving left-join; a semi-join of the first alias to the carried ids). `RETURN entity / property / count / count(*) / DISTINCT` over WITH→MATCH now run natively with pandas parity. Differential fuzz vs pandas over ~3500 random graphs × WITH→MATCH shapes (DISTINCT/filtered/ORDER-BY prefixes × fwd/rev/undirected re-MATCH × entity/property/count/distinct RETURN): **0 disagreements, 0 crashes** (declines are honest NIEs). Still honest-NIE (clean decline, was a crash): a WITH that also carries a **scalar column** into the trailing MATCH (hidden-payload threading is still pandas-only), and duplicate carried ids / node-cartesian trailing patterns under a seed (preserve pandas path-multiplicity / avoid silent-wrong). New `test_engine_polars_with_match_reentry.py`.
1819
- **GFQL polars engine natively runs multi-source (node-cartesian) MATCH (#1273)**: comma-separated disconnected aliases — `g.gfql("MATCH (a {..}), (b {..}) RETURN a.id, b.id")` — previously declined on polars (the `rows(binding_ops)` `node_cartesian` branch hard-NIE'd) while pandas handled them. New `_cartesian_node_bindings_polars` mirrors the pandas oracle `_gfql_cartesian_node_bindings_row_table`: each alias is independently filtered, projected into the per-alias lookup schema (bare `alias` id, `alias.id`, `alias.<prop>`, and pandas' leaked `alias.alias=True` flag incl. property-shadowing), then left-major cross-joined for row-order parity. Scalar/aliased/`count(*)` projections over the cartesian match pandas exactly (verified end-to-end); whole-entity `RETURN a, b` stays an honest NIE (separate projection surface). Two parity-safe declines mirror shapes where pandas itself errors (proven on master, so both engines fail identically, never diverge): an anonymous node op, and ≥4 named aliases (pandas' bare-id merge residue collides). Differential fuzz vs pandas over ~10k lowered cases: 0 disagreements. Tests in `test_engine_polars_binding_rows.py`.
1920
- **GFQL polars chain — variable-length node aliases are hop-distance gated; the chain is now silent-wrong-free (#1741)**: a node named after a variable-length edge (`g.gfql("MATCH (a)-[*1..2]-(b) RETURN b")`) previously carried its alias regardless of hop distance, so an undirected walk that backtracked into the seed wrongly returned it (pandas correctly excludes it — trail semantics). The polars chain now auto-injects the #1741 hop-distance label (name resolved against the user's node columns via the `reserved_columns` registry, so a user column named like the internal one is never clobbered) and applies pandas' alias `[min_hop, max_hop]` window in `_apply_node_names`. The one shape the gate can't yet cover — a node alias after a **forward/reverse `min_hops>1`** edge, whose labels need pandas' layered backward walk (not ported) — now **declines with an honest `NotImplementedError` (#1748)** instead of returning nodes outside the window; the decline is precise (differential vs pandas: 30/30 named shapes NIE, 30/30 unnamed run and match, 0 over-decline). 4-engine A/B on the stacked build: pandas/cudf 144/144, polars/polars-gpu 112 agree / **0 disagree** / 32 honest-NIE — zero silent-wrong shapes remain on the polars chain. Adapts the mechanism from the retired #1742 (declined undirected varlen+alias, now natively gated). Tests: `TestVarlenAliasHopGate` (pandas-parity across directions, the `*2..3` decline + unnamed-still-runs pins, a column-collision test).
2021
- **GFQL native polars `label_node_hops` on the plain BFS — correct, direction-dependent hop labels (#1741 groundwork)**: the polars eager hop now emits pandas-parity node hop-distance labels instead of declining `label_node_hops`. The labeling rule is DIRECTION-DEPENDENT (the subtle part, derived by differential fuzz vs the pandas oracle, not by reading pandas alone): forward/reverse label EVERY destination of a hop first-wins (matching pandas `hop.py` `new_node_ids`), so a seed re-entered at hop 1 IS labeled; undirected labels destinations MINUS everything already visited and pre-seeds the seen-set with the seeds, so a seed re-reached by a backtracking walk stays NULL (its shortest-path distance). Two correctness fixes over the first cut: (1) the undirected seed pre-seed must NOT depend on `return_as_wave_front` — a seed filtered out of the frontier by `source_node_match` or suppressed in wave-front mode was being re-labeled (A/B vs pandas over direction × hops × `return_as_wave_front` × `source_node_match` × `destination_node_match` × 10 graphs: was 456/24, now 480/480); (2) a requested `label_node_hops` name that collides with an existing column is redirected to `<name>_1` like pandas' `resolve_label_col`, not the polars left-join auto-suffix `<name>_right`/`DuplicateError`. `label_edge_hops` and `min_hops>1` labels stay honest NIEs. Amplified `test_engine_polars_hop.py` (`TestHopLabelsDifferential` over the seed-filter / wave-front / multi-seed axes, an explicit direction-asymmetry contrast pin, and a collision test).

bin/test-polars.sh

Lines changed: 6 additions & 2 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_with_match_reentry.py
2122
graphistry/tests/compute/gfql/test_engine_polars_cypher_conformance.py
2223
graphistry/tests/compute/gfql/test_engine_polars_conformance_matrix.py
2324
graphistry/tests/compute/gfql/test_polars_string_predicate_nonstring.py
@@ -29,11 +30,14 @@ POLARS_TEST_FILES=(
2930
# index tests exercise the seeded-index hook in the polars hop entry (hop.py) — without
3031
# them the hook dominates the now-thin file and trips its per-file coverage floor
3132
graphistry/tests/compute/gfql/index/test_index.py
33+
# engine-agnostic frame/series primitives (graphistry/Engine.py) — the polars branches of
34+
# these dispatch helpers are only measured when this lane covers graphistry (see cov widen below)
35+
graphistry/tests/test_engine_frame_helpers.py
3236
)
3337

3438
COV_ARGS=()
3539
if [ -n "${POLARS_COV:-}" ]; then
36-
COV_ARGS=(--cov=graphistry/compute --cov-report=)
40+
COV_ARGS=(--cov=graphistry --cov-report=)
3741
fi
3842

3943
python -B -m pytest -vv "${COV_ARGS[@]}" "${POLARS_TEST_FILES[@]}" "$@"
@@ -42,7 +46,7 @@ python -B -m pytest -vv "${COV_ARGS[@]}" "${POLARS_TEST_FILES[@]}" "$@"
4246
# appended into the same coverage data file when POLARS_COV=1 (CI audit reads it)
4347
COV_APPEND_ARGS=()
4448
if [ -n "${POLARS_COV:-}" ]; then
45-
COV_APPEND_ARGS=(--cov=graphistry/compute --cov-report= --cov-append)
49+
COV_APPEND_ARGS=(--cov=graphistry --cov-report= --cov-append)
4650
fi
4751
python -B -m pytest -vv "${COV_APPEND_ARGS[@]}" \
4852
graphistry/tests/compute/gfql/cypher/test_lowering.py -k polars

graphistry/Engine.py

Lines changed: 107 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,18 @@
33
import numpy as np
44
import pandas as pd
55
import pyarrow as pa
6-
from typing import Any, List, Optional, Union
6+
from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Sequence, Union
77
from typing_extensions import Literal
88
from enum import Enum
99

1010
from graphistry.models.types import ValidationParam
1111

12+
if TYPE_CHECKING:
13+
# Frame aliases (pandas types for mypy; ``Any`` at runtime). Imported under TYPE_CHECKING
14+
# and referenced via string annotations below so Engine.py — imported very early — never
15+
# triggers ``graphistry.compute`` package init at runtime (would be circular).
16+
from graphistry.compute.typing import DataFrameT, SeriesT
17+
1218

1319
class Engine(Enum):
1420
PANDAS = 'pandas'
@@ -850,3 +856,103 @@ def safe_merge(
850856
raise ValueError("Must specify either 'on' or both 'left_on' and 'right_on'")
851857

852858
return result
859+
860+
861+
# ---------------------------------------------------------------------------
862+
# Engine-agnostic series / frame primitives (pandas / cuDF / polars dispatch).
863+
#
864+
# Pure per-row/per-column dispatch helpers with no domain knowledge — the polars
865+
# branches genuinely return polars objects mypy can't narrow to the pandas frame
866+
# aliases, so the localized ``# type: ignore`` lives here at the dispatch point and
867+
# callers get a clean ``SeriesT`` / ``DataFrameT`` contract with no ``cast()``.
868+
# Annotations are strings so the TYPE_CHECKING-only alias import stays runtime-free.
869+
# ---------------------------------------------------------------------------
870+
871+
872+
def is_series_like(s: object) -> bool:
873+
"""True for a pandas/cuDF Series (``.dropna``) or a polars Series (module check).
874+
875+
Some engine-agnostic callers accept an ``ids`` Series that is pandas under
876+
``engine='pandas'`` and polars under ``engine='polars'``; both are valid."""
877+
return hasattr(s, "dropna") or is_polars_df(s)
878+
879+
880+
def series_not_null_mask(s: "SeriesT") -> "SeriesT":
881+
"""Non-null boolean mask, engine-aware (polars ``is_not_null`` vs pandas ``notna``)."""
882+
if is_polars_df(s):
883+
return s.is_not_null() # type: ignore[attr-defined,no-any-return]
884+
return s.notna()
885+
886+
887+
def series_filter(s: "SeriesT", mask: "SeriesT") -> "SeriesT":
888+
"""Filter a Series by a boolean mask, engine-aware, dropping the old index (pandas)."""
889+
if is_polars_df(s):
890+
return s.filter(mask) # type: ignore[attr-defined,no-any-return]
891+
return s[mask].reset_index(drop=True)
892+
893+
894+
def frame_filter(df: "DataFrameT", mask: "SeriesT") -> "DataFrameT":
895+
"""Filter a DataFrame's rows by a boolean mask, engine-aware, dropping the old index."""
896+
if is_polars_df(df):
897+
return df.filter(mask) # type: ignore[attr-defined,no-any-return]
898+
return df.loc[mask].reset_index(drop=True)
899+
900+
901+
def ordered_left_join(left: "DataFrameT", right: "DataFrameT", *, on: str) -> "DataFrameT":
902+
"""Left join preserving ``left`` row order, engine-aware. Polars ``.merge`` does not exist;
903+
``safe_merge`` (pandas/cuDF ``.merge``) cannot run on polars frames, so branch to
904+
``.join(..., maintain_order='left')`` which pins the left-row ordering the caller needs.
905+
906+
``right`` may arrive on a different engine than ``left`` (e.g. natively-projected polars
907+
``left`` against a still-pandas base table), so align ``right`` onto ``left``'s engine
908+
before the polars join."""
909+
if is_polars_df(left):
910+
if not is_polars_df(right):
911+
right = df_to_engine(right, Engine.POLARS)
912+
return left.join(right, on=on, how="left", maintain_order="left") # type: ignore[call-arg,no-any-return]
913+
return safe_merge(left, right, on=on, how="left")
914+
915+
916+
def row_as_mapping(rows: "DataFrameT", row_index: int) -> Mapping[str, Any]:
917+
"""One frame row as a col->scalar mapping, engine-aware (``row[col]`` works for
918+
both the pandas Series and the polars named-row dict)."""
919+
if is_polars_df(rows):
920+
return rows.row(row_index, named=True) # type: ignore[attr-defined,no-any-return]
921+
return rows.iloc[row_index]
922+
923+
924+
def assign_constant_columns(df: "DataFrameT", values: Dict[str, Any]) -> "DataFrameT":
925+
"""Broadcast scalar ``values`` as constant columns, engine-aware."""
926+
if not values:
927+
return df
928+
if is_polars_df(df):
929+
import polars as pl
930+
return df.with_columns([pl.lit(v).alias(k) for k, v in values.items()]) # type: ignore[attr-defined,no-any-return]
931+
return df.assign(**values)
932+
933+
934+
def drop_columns(df: "DataFrameT", cols: Sequence[str]) -> "DataFrameT":
935+
"""Drop columns by name, engine-aware (polars ``drop(list)`` vs pandas ``drop(columns=)``)."""
936+
if is_polars_df(df):
937+
return df.drop(list(cols)) # type: ignore[no-any-return]
938+
return df.drop(columns=list(cols))
939+
940+
941+
def series_to_pylist(values: "SeriesT") -> List[Any]:
942+
"""Series -> python list, engine-aware, with defensive arrow/pandas fallbacks."""
943+
if hasattr(values, "to_arrow"):
944+
try:
945+
return list(values.to_arrow().to_pylist())
946+
except Exception:
947+
pass
948+
if hasattr(values, "to_pandas"):
949+
try:
950+
return list(values.to_pandas().tolist())
951+
except Exception:
952+
pass
953+
if hasattr(values, "tolist"):
954+
try:
955+
return list(values.tolist())
956+
except Exception:
957+
pass
958+
return list(values)

0 commit comments

Comments
 (0)