From efe17f2cc109537cb32285de5486b8caf30ac053 Mon Sep 17 00:00:00 2001 From: x Date: Mon, 27 Jul 2026 12:06:17 -0700 Subject: [PATCH 1/4] perf(gfql): serve NAMED patterns on the native chain fast path `_try_chain_fast_path` rejected ANY op carrying an alias, so a named `g.gfql([n(name='x'), e_forward(name='r'), n(name='y')])` fell to the full two-pass BFS purely because the ops were named. Naming is a PROJECTION concern, not a traversal one; it should not decide which engine path runs. RE-MEASURED on a 200-node / 800-edge pandas graph (data work ~0, so this is the per-query plan floor). Five ALTERNATING PAIRED runs vs origin/master 233b64c8, each value the median of 200 reps after 20 warmups (range over the five runs in parentheses -- the run-to-run spread is real): named 3-op traversal 25.2 ms (24.2-27.0) -> 2.3 ms (2.1-2.7) ~11x the same + rows(binding_ops) 40.4 (38.1-43.6) -> 17.4 (16.8-19.8) ~2.3x The rows() output is byte-identical between arms; the traversal output is value-identical (dtypes deliberately changed, below). SCOPE, AND THE EARLIER CLAIM WAS WRONG. The previous description said "every Cypher multi-alias MATCH...RETURN names its ops, so the entire Cypher surface is locked out". An instrumented run of the graph-benchmark q1-q9 lane at 20k, both engines, both arms, falsifies that three independent ways: every consult on that lane is ops=1 and this change only edits the len(ops)==3 branch; g.gfql() for those shapes is served earlier by gfql_fast_paths.py (_execute_single_hop_grouped_aggregate_fast_path, _execute_two_hop_count_fast_path, _execute_seeded_typed_hop_fast_path) and never reaches here; and this path is pandas/cuDF only while the losing column is polars. Zero consults on q1/q2/q3/q4/ q8/q9 pandas and on all of q1-q9 polars, identical on both arms. NO BENCHMARK CELL MOVES. A positive control confirms the lever is live rather than inert (arm A DECLINE at the removed gate, arm B SERVE, same native 3-op named chain). Under the pyg-bench-proof-plus-lock-in rule this therefore carries NO perf claim on the board; the lane is specified as follow-up in the PR description. HOW THE TAGS ARE DERIVED: from the KEPT EDGES the fast path returns. `combine_steps` tags a node with an alias iff it still participates in a surviving edge, and those edges are exactly the surviving ones, so `isin` over their endpoint columns is the same predicate without the join. A seed whose edges all fail the filter yields an empty edge frame and is tagged False -- the dead-end case, and why the tag keys on edges rather than on the node filter. DECLINES KEPT DELIBERATELY: undirected + named (alias identity is not derivable from endpoint columns there); duplicate alias names (E201 lives in `combine_steps`, so serving them would BYPASS the check and silently succeed); and a resident index that would actually serve the shape (`_resident_seed_indexes` asks about index VALIDITY -- a registry-presence check declines on every query and makes the whole thing a measured no-op). CODE LOCATION: `_tag_fast_path_aliases` lives in `chain_fast_paths.py`, not `chain.py`. That is the pre-existing direction, confirmed not recalled: the module was created by 289c6abe "extract seeded fast-path specializations to dedicated modules (#1755)" and its docstring states the one-way import rule. The docstring now records it explicitly so the next helper does not land in chain.py again. DTYPES: ADOPT the conformant ones. The pandas full path upcasts non-id int64 -> float64 and bool -> object through its rows-pivot merges. Verified rather than assumed that this is an ARTIFACT: openCypher TCK clauses/return/Return2.feature [2] expects `1` for an integer node property (its value grammar distinguishes Integer from Float); Neo4j treats INTEGER and FLOAT as distinct property types and reports valueType()="INTEGER NOT NULL"; and this library's own polars and cuDF canonical paths already return int64/bool for the same query, so only the pandas merge upcasts. The fast path keeps int64/bool. User-visible where a named pandas pattern is served: 30 rather than 30.0, and a bool alias flag rather than object. Values unchanged. NOT aligned yet, deliberately out of scope: the pandas full path and the Cypher-layer projection still emit the artifact. TESTS: 4 deliberate rewrites, each commented with the rule it now encodes. test_fast_path_gating_returns_none_for_ineligible and the _FAST_SHAPES/_BYPASS_SHAPES differential move named shapes from decline to serve and add the new declines; test_pandas_datetime_property_declines and test_engine_mismatch_declines keep asserting the Cypher-layer decline and identical values, with the dtype rule now asserted explicitly instead of incidentally. NEW coverage for the new capability: test_fast_path_named_alias_columns_match_full_path (9 shapes x 2 engines) compares the alias flag COLUMNS and per-row values fast-vs-full, which the id-set differential cannot see, and is serve-asserted. CHANGELOG added (Performance + Changed). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB --- CHANGELOG.md | 2 + graphistry/compute/chain.py | 59 +++++++++--- graphistry/compute/chain_fast_paths.py | 51 +++++++++++ .../gfql/index/test_indexed_bindings.py | 30 ++++++- .../gfql/test_seeded_typed_hop_fastpath.py | 64 ++++++++++++- graphistry/tests/compute/test_chain.py | 90 ++++++++++++++++++- 6 files changed, 280 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac88700713..9b2f51496b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,8 +22,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL indexed fixed-hop bindings now dispatch BEFORE the canonical traversal (pandas / cuDF / native Polars)**: the resident-index fixed-hop path (`rows(binding_ops=...)`, the Cypher multi-alias `MATCH … RETURN` shape) was consulted only *inside* row materialization — after the engine had already run the full canonical traversal — so its compact path bag was computed on top of the graph-sized work it exists to avoid. Both the pandas/cuDF chain boundary and the native Polars chain now ask the same shared, structural gate first and, only when it can serve the WHOLE middle exactly, skip the canonical traversal and hand the compact state to the unchanged row materializer. Engagement stays operator/index/dtype/cost based — no query, schema, or hop-count recognition — and every unsupported, seeded, prefiltered, policy-bearing, shortest-path, or cost-gated shape still declines to canonical execution with identical results. On a standard LDBC SNB SF1 interactive-short profile this removed the redundant two-hop typed-mask and 16-merge buckets and cut the profiled call ~11.9× (1263 ms → 106 ms), exact 19-row oracle preserved. - **Indexed seed lookup now covers a unique node id PLUS extra scalar constraints**: `{node_id: v, other: w}` previously fell back to a full node-table scan even though the unique node-id index can gather at most one row; it now gathers that one row and applies the remaining constraints to it. Missing ids and non-matching extra constraints return the same empty result as before, and duplicate-id graphs (which cannot build the index) are unaffected. - **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. +- **The pandas/cuDF chain fast path serves NAMED patterns**: `_try_chain_fast_path` rejected any op carrying an alias, so `g.gfql([n(name='x'), e_forward(name='r'), n(name='y')])` fell to the full two-pass BFS purely because the ops were named — naming is a PROJECTION concern, not a traversal one, and it should not decide which engine path runs. The alias flag columns `combine_steps` would have merged in are now reconstructed directly from the edges the fast path returns (`_tag_fast_path_aliases`): `combine_steps` tags a node with an alias iff it still participates in a surviving edge, and those edges are exactly the ones the fast path already produced, so `isin` over their endpoint columns is the same predicate computed without the join. A seed whose edges all fail the filter yields an empty edge frame and is tagged `False`, which is the dead-end case. Deliberate declines kept: **undirected + named** (an undirected edge makes a node reachable as either endpoint, so alias identity is not derivable from the endpoint columns), **duplicate alias names** (E201 is raised by `combine_steps`, so serving here would bypass the check and let alias reuse silently succeed), and **a resident index that would actually serve the shape** (checked with `_resident_seed_indexes`, i.e. index VALIDITY for these exact frames — a registry-presence check instead declines on every query and silently turns the whole optimization into a measured no-op). **Measured** on a 200-node / 800-edge pandas graph, where data-proportional work is ~0 so this is the per-query plan floor. Five alternating paired runs against `origin/master`, each the median of 200 reps after 20 warmups: the named 3-op traversal `25.2 ms (24.2–27.0) → 2.3 ms (2.1–2.7)`, **~11×**, and the same pattern followed by `rows(binding_ops=...)` `40.4 ms (38.1–43.6) → 17.4 ms (16.8–19.8)`, **~2.3×**. The `rows()` output is byte-identical between the two arms and the traversal output is value-identical (dtypes differ deliberately, see *Changed*). **Scope, stated plainly**: this is a NATIVE `g.gfql([...named...])` / `g.chain([...named...])` improvement only. It is pandas/cuDF-only, and an instrumented run of the graph-benchmark q1–q9 lane showed the chain fast path is **never consulted** for those Cypher queries — they are served earlier by the Cypher-layer fast paths in `gfql_fast_paths.py`, or hit a separate single-op gate — so **no benchmark cell moves** and no lane currently exercises this. A benchmark lane over the native named-chain surface is required follow-up before any perf claim is made on the board. ### Changed +- **A served chain fast path now returns Cypher-conformant int/bool dtypes on named patterns too**: the pandas full path's rows-pivot upcasts non-id `int64 → float64` and `bool → object` through its merges. That is a pandas merge artifact, not a Cypher semantic — the openCypher TCK (`clauses/return/Return2.feature`, "Returning a node property value": `CREATE ({num: 1})` / `RETURN a.num` expects `1`, and the TCK value grammar writes an Integer as bare decimal digits and a Float with its decimals) and Neo4j (INTEGER and FLOAT are distinct property types; `valueType()` on an integer property reports `INTEGER NOT NULL`) both keep the integer, and this library's own **polars** and **cuDF** canonical paths already return `int64`/`bool` for the same query, so only the pandas merge upcasts. The chain fast path already preserved the conformant dtypes for UNNAMED patterns (documented in its docstring); serving named patterns extends that to the named surface rather than creating a new divergence, and the conformant dtype is deliberately kept rather than cast back to reproduce the defect. **User-visible** where a named pandas pattern is served by the fast path: an integer node property comes back `int64` instead of `float64` (`30` rather than `30.0`, including in JSON), and an alias flag column comes back `bool` instead of `object`. Values are unchanged. **Not aligned yet** (deliberately out of scope, follow-up): the pandas full path itself, and the Cypher-layer seeded projection, still emit the artifact — so pandas can still report `float64` for the same logical query depending on which internal path serves it. - **`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. - **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. diff --git a/graphistry/compute/chain.py b/graphistry/compute/chain.py index 32fe1020f0..cfc3fd64fc 100644 --- a/graphistry/compute/chain.py +++ b/graphistry/compute/chain.py @@ -11,7 +11,7 @@ from .ast import ASTObject, ASTNode, ASTEdge, ASTCall, Direction, from_json as ASTObject_from_json, serialize_binding_ops from .typing import DataFrameT, SeriesT from .util import generate_safe_column_name -from .chain_fast_paths import _seeded_typed_hop_pandas_cudf +from .chain_fast_paths import _seeded_typed_hop_pandas_cudf, _tag_fast_path_aliases from graphistry.compute.validate.validate_schema import validate_chain_schema from graphistry.compute.gfql.same_path_types import ( WhereComparison, @@ -822,9 +822,11 @@ def _try_chain_fast_path( Same node/edge sets + VALUES as the full machinery (trackA_golden + hop/chain suites); the 1-hop additionally preserves int node dtypes (the full path upcasts - int→float via merge). Gated to unnamed/unqueried nodes + a plain single-hop edge; - filtered-undirected and seeded chains fall through. polars/dask/spark also fall - through (own fast path / lazy semantics).""" + int→float via merge — the merge is the artifact, int is the Cypher-conformant type). + Gated to unqueried nodes + a plain single-hop edge; NAMED ops are served (the alias + flags are reconstructed by `_tag_fast_path_aliases`) except when undirected or when + the same alias is reused. filtered-undirected and seeded chains fall through. + polars/dask/spark also fall through (own fast path / lazy semantics).""" from graphistry.compute.filter_by_dict import filter_by_dict if engine_concrete not in (Engine.PANDAS, Engine.CUDF): @@ -852,13 +854,28 @@ def _materialize_fast_path_graph() -> Plottable: if len(ops) != 3: return None n0, e1, n2 = ops - if not (isinstance(n0, ASTNode) and n0._name is None and n0.query is None): + # Aliases are a PROJECTION concern, not a traversal one: capture them, serve the + # traversal on the fast path, and tag the result (_tag_fast_path_aliases). Rejecting + # them here sent a NAMED `g.gfql([n(name=..), e(..), n(name=..)])` to the full + # two-pass BFS purely because the ops carried names — measured ~25.2 ms before vs + # ~2.3 ms after (medians of 5 paired runs), on a 200-node graph where data work is ~0. + # SCOPE, measured: this does NOT reach the Cypher `MATCH ... RETURN` surface for the + # benchmark shapes. Those are served earlier by `gfql_fast_paths.py` and never consult + # this function at all, so do not attribute a Cypher-surface win to this gate. + alias_n0, alias_e1, alias_n2 = n0._name, e1._name, n2._name + _named = [a for a in (alias_n0, alias_e1, alias_n2) if a is not None] + if len(_named) != len(set(_named)): + # Duplicate alias reuse is an E201 error, and `combine_steps` is what raises it. + # Serving these here would BYPASS that check and silently succeed — decline so the + # full path still errors. (Caught by test_polars_duplicate_alias_declines_like_pandas.) return None - if not (isinstance(n2, ASTNode) and n2._name is None and n2.query is None): + if not (isinstance(n0, ASTNode) and n0.query is None): + return None + if not (isinstance(n2, ASTNode) and n2.query is None): return None if not (isinstance(e1, ASTEdge) and e1.is_simple_single_hop() and e1.source_node_match is None - and e1.destination_node_match is None and e1._name is None + and e1.destination_node_match is None and e1.source_node_query is None and e1.destination_node_query is None and e1.edge_query is None and not e1.include_zero_hop_seed and not e1.prune_to_endpoints): # prune keeps only the arrival side -> full path @@ -868,6 +885,11 @@ def _materialize_fast_path_graph() -> Plottable: # below rather than falling through to the full two-pass machinery. source/dest # node match + edge_query (richer predicates) still bail above. direction = e1.direction + if direction == "undirected" and (alias_n0 is not None or alias_n2 is not None): + # An undirected edge makes a node reachable as EITHER endpoint, so "which alias + # does this node carry" is not derivable from the endpoint columns the way it is + # for a directed hop. Decline to the full path rather than guess. + return None unconstrained = not n0.filter_dict and not n2.filter_dict if not unconstrained and direction == "undirected": return None # filtered-undirected (OR of both directions) -> full path @@ -877,6 +899,20 @@ def _materialize_fast_path_graph() -> Plottable: src, dst, node = g._source, g._destination, g._node if src is None or dst is None or node is None: return None # no edge/node bindings -> can't fast-path; full path handles it + if (alias_n0 is not None or alias_e1 is not None or alias_n2 is not None) \ + and direction != "undirected" and g._nodes is not None and g._edges is not None: + # DEFER TO THE INDEX when one would ACTUALLY serve. A resident index is + # policy-controlled and can beat this scan-based path, and it is what serves named + # patterns today, so taking them here would SHADOW it. + # `_resident_seed_indexes` is the right question: it returns None unless BOTH + # indexes are VALID for these exact frames (fingerprint + identity via get_valid). + # A registry-presence / policy check is NOT equivalent — `get_registry` returns a + # non-None registry even when nothing is indexed, so that spelling declines on every + # query and silently turns this whole optimization into a no-op (measured: the win + # went to exactly 0). Re-measure the win, not just the suite, after touching this. + from .chain_fast_paths import _resident_seed_indexes + if _resident_seed_indexes(g, g._nodes, g._edges, node, src, dst, direction) is not None: + return None concat = df_concat(engine_concrete) if unconstrained: # No node filter to reduce by: validate BOTH endpoints against the full @@ -902,7 +938,8 @@ def _materialize_fast_path_graph() -> Plottable: if engine_concrete in (Engine.PANDAS, Engine.CUDF): _fast_res = _seeded_typed_hop_pandas_cudf(g, n0, n2, e1, src, dst, node, direction) if _fast_res is not None: - return _fast_res + return _tag_fast_path_aliases( + _fast_res, alias_n0, alias_e1, alias_n2, src, dst, node, direction) from_col, to_col = (src, dst) if direction == "forward" else (dst, src) edges = g._edges if n0.filter_dict: @@ -935,7 +972,8 @@ def _materialize_fast_path_graph() -> Plottable: edges[[dst]].rename(columns={dst: node}), ]).drop_duplicates() nodes = cand[cand[node].isin(final[node])] - return g.nodes(nodes).edges(edges) + return _tag_fast_path_aliases( + g.nodes(nodes).edges(edges), alias_n0, alias_e1, alias_n2, src, dst, node, direction) endpoints = concat([ edges[[src]].rename(columns={src: node}), edges[[dst]].rename(columns={dst: node}), @@ -943,7 +981,8 @@ def _materialize_fast_path_graph() -> Plottable: nodes = g._nodes[g._nodes[node].isin(endpoints[node])] # match the full path's merge, which collapses duplicate node-id rows nodes = nodes.drop_duplicates(subset=[node]) - return g.nodes(nodes).edges(edges) + return _tag_fast_path_aliases( + g.nodes(nodes).edges(edges), alias_n0, alias_e1, alias_n2, src, dst, node, direction) @otel_traced("gfql.chain", attrs_fn=_chain_otel_attrs) diff --git a/graphistry/compute/chain_fast_paths.py b/graphistry/compute/chain_fast_paths.py index 45dd638b64..c4ef0e7869 100644 --- a/graphistry/compute/chain_fast_paths.py +++ b/graphistry/compute/chain_fast_paths.py @@ -4,6 +4,10 @@ typed 1-hop pandas/cuDF reduction and the seeded typed RETURN-destination pandas/cuDF and polars reductions. Pure code move, no behavior change. chain.py imports from here (one direction); this module imports only leaf modules (no back-edge into chain.py). + +New fast-path helpers belong HERE, not in chain.py — that is the direction this module was +created to establish and it is why `_tag_fast_path_aliases` lives alongside the seeded +reductions rather than next to `_try_chain_fast_path`. """ # ruff: noqa: E501 @@ -18,6 +22,53 @@ from graphistry.compute.gfql.index.registry import AdjacencyIndex, NodeIdIndex +def _tag_fast_path_aliases( + res: Plottable, + alias_n0: Optional[str], alias_e1: Optional[str], alias_n2: Optional[str], + src: str, dst: str, node: str, direction: Direction, +) -> Plottable: + """Attach the alias flag columns the full path's ``combine_steps`` would have merged in. + + The chain fast path's gate used to reject ANY named op, so a named + `g.gfql([n(name=..), e(..), n(name=..)])` fell to the full two-pass machinery purely + because the ops carried names — measured ~25.2 -> ~2.3 ms (medians of 5 paired runs) on + a 200-node graph where data-proportional work is ~0. Naming is a PROJECTION concern, + not a traversal one, so it should not change which engine path runs. NOTE the scope: this is the NATIVE chain + surface. The Cypher `MATCH ... RETURN` shapes on the graph benchmark are served + earlier by `gfql_fast_paths.py` and never reach here (measured, both engines). + + Why deriving the tags from the RETURNED EDGES matches the full path: `combine_steps` + tags a node with an alias iff it matched that step in the BACKWARD-PRUNED frame, i.e. + iff it still participates in a surviving edge. The edges this function receives are + exactly the surviving ones — the fast path has already applied the node filters, the + edge_match and the endpoint validation — so ``isin`` over their endpoint columns is the + same predicate, computed without the join. + + A seed whose edges all fail the type filter yields an empty edge frame, so it is tagged + False rather than True: that is the dead-end case, and it is why the tag keys on the + edges rather than on the node filter. + """ + if alias_n0 is None and alias_e1 is None and alias_n2 is None: + return res + nodes: Optional[DataFrameT] = res._nodes + edges: Optional[DataFrameT] = res._edges + if nodes is None or edges is None: + return res + from_col, to_col = (src, dst) if direction == "forward" else (dst, src) + node_flags: Dict[str, SeriesT] = {} + if alias_n0 is not None: + node_flags[alias_n0] = nodes[node].isin(edges[from_col]) + if alias_n2 is not None: + node_flags[alias_n2] = nodes[node].isin(edges[to_col]) + if node_flags: + nodes = nodes.assign(**node_flags) + if alias_e1 is not None: + edge_flags: Dict[str, bool] = {alias_e1: True} + edges = edges.assign(**edge_flags) + + return res.nodes(nodes).edges(edges) + + def _seeded_scalar_filters(fd: Optional[Dict[str, Any]], df: DataFrameT) -> Optional[Dict[str, Any]]: """Resolve a filter dict to plain scalar column==value pairs, or None to bail to the general path. Mirrors filter_by_dict.resolve_filter_column exactly for diff --git a/graphistry/tests/compute/gfql/index/test_indexed_bindings.py b/graphistry/tests/compute/gfql/index/test_indexed_bindings.py index 2c44c25ce5..40e2745527 100644 --- a/graphistry/tests/compute/gfql/index/test_indexed_bindings.py +++ b/graphistry/tests/compute/gfql/index/test_indexed_bindings.py @@ -6,7 +6,7 @@ """ from __future__ import annotations -from typing import Any, Dict, List, Sequence, Tuple, Type +from typing import Any, Dict, List, Optional, Sequence, Tuple, Type import numpy as np import pandas as pd @@ -14,7 +14,8 @@ import graphistry from graphistry.Engine import Engine -from graphistry.compute.ast import ASTEdge, e_forward, e_undirected, n +from graphistry.Plottable import Plottable +from graphistry.compute.ast import ASTEdge, ASTObject, e_forward, e_undirected, n ENGINES = ["pandas", "polars", "cudf"] @@ -148,6 +149,7 @@ def _run( if generic: import graphistry.compute.gfql.index.bindings as indexed_bindings import graphistry.compute.gfql_unified as gfql_unified + import graphistry.compute.chain as chain_mod monkeypatch.setattr( indexed_bindings, @@ -159,6 +161,30 @@ def _run( "_execute_seeded_typed_hop_fast_path", lambda *args, **kwargs: None, ) + # `generic` enumerates the acceleration seams to disable. The chain fast path now + # serves NAMED patterns (it previously rejected any alias, which is why it was + # absent here), so it has to be listed — otherwise the comparison stops being + # generic-vs-accelerated and becomes accelerated-vs-a-DIFFERENT-accelerated. + # + # But disable ONLY the NEW capability, not the seam. This path has always served + # UNNAMED middles, and `test_unnamed_middle_rows_call_matches_canonical` compares + # exactly that shape — blanket-disabling turns that case from fast-vs-fast into + # general-vs-fast and it fails for a reason that has nothing to do with what it + # tests. Verified: master's product code plus a blanket patch reproduces that + # failure on its own. + _real_fast_path = chain_mod._try_chain_fast_path + + def _fast_path_without_named( + g_in: Plottable, + ops: List[ASTObject], + engine_concrete: Engine, + start_nodes: Optional[pd.DataFrame] = None, + ) -> Optional[Plottable]: + if any(op._name is not None for op in ops): + return None + return _real_fast_path(g_in, ops, engine_concrete, start_nodes) + + monkeypatch.setattr(chain_mod, "_try_chain_fast_path", _fast_path_without_named) return g.gfql( query, engine=engine, diff --git a/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py b/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py index b57908b597..abccc0491f 100644 --- a/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py +++ b/graphistry/tests/compute/gfql/test_seeded_typed_hop_fastpath.py @@ -13,11 +13,14 @@ including full-path side-channels (policy hooks, same-path WHERE, OPTIONAL null rows, WITH..MATCH carried seeds, list-`labels` columns, null ids). """ +from typing import Dict, Tuple + import numpy as np import pandas as pd import pytest import graphistry +from graphistry.Plottable import Plottable from graphistry.compute.ast import n, e_forward, e_reverse import graphistry.compute.chain as chain_mod import graphistry.compute.gfql_unified as gfql_unified @@ -726,6 +729,49 @@ def _pl_graph(self): return graphistry.nodes(pl.from_pandas(g._nodes), "id").edges( pl.from_pandas(g._edges), "src", "dst") + # ------------------------------------------------------------------ + # THE PANDAS FULL PATH'S int64->float64 / bool->object UPCAST IS AN ARTIFACT. + # + # It comes from the rows-pivot's merges, not from Cypher. Evidence, three independent + # sources (this is a DELIBERATE rule, not a test "fix"): + # 1. openCypher TCK `tck/features/clauses/return/Return2.feature` scenario [2] + # "Returning a node property value": `CREATE ({num: 1})` / `MATCH (a) RETURN a.num` + # expects `1`. The TCK value grammar (`tck/README.adoc`) writes an Integer as a + # bare string of decimal digits and a Float "in decimal form with all present + # decimals", so `1.0` would be a DIFFERENT expected value. + # 2. Neo4j (reference engine) Cypher manual: INTEGER and FLOAT are distinct property + # types and an INTEGER property read back is INTEGER + # (`valueType(n.prop)` -> "INTEGER NOT NULL"). + # 3. This repo's own POLARS full path already returns int64/bool for this exact + # query — only the pandas merge upcasts. So the upcast is engine-local, which is + # what an artifact looks like and what a semantic does not. + # => int64/bool is the CONFORMANT result. Where a fast path serves, it keeps the + # conformant dtype rather than casting back to reproduce the defect (casting back was + # also measured to be per-column and unprincipled: a blanket cast took the suite from + # 20 to 45 failures). + # NOT YET ALIGNED (deliberately out of scope, tracked as follow-up): the pandas full + # path itself, and the Cypher-layer projection pinned by + # `test_pandas_int_bool_dtype_parity` below, still emit the artifact. + _FULL_PATH_PANDAS_UPCASTS: Dict[str, Tuple[str, str]] = { + "a": ("int64", "float64"), "f": ("bool", "object")} + + def _assert_values_equal_conformant_dtypes(self, fast: Plottable, full: Plottable) -> None: + """Values identical; where the pandas full path upcast, the served path keeps the + Cypher-conformant dtype and we assert BOTH sides explicitly.""" + f, u = _canon_nodes(fast), _canon_nodes(full) + assert list(f.columns) == list(u.columns) + for col in f.columns: + rule = self._FULL_PATH_PANDAS_UPCASTS.get(col) + if rule is not None and str(u[col].dtype) == rule[1]: + conformant, artifact = rule + assert str(f[col].dtype) == conformant, ( + f"{col}: served path must keep the conformant dtype " + f"{conformant}, got {f[col].dtype}") + pd.testing.assert_series_equal( + f[col].astype(artifact), u[col], check_names=False) + else: + pd.testing.assert_series_equal(f[col], u[col], check_names=False) + def _fast_and_full(self, g, engine, q, expect_engage=True): hits = {"n": 0} real = gfql_unified._execute_seeded_typed_hop_fast_path @@ -756,9 +802,16 @@ def test_polars_int_bool_dtype_parity(self): pd.testing.assert_frame_equal(_canon_nodes(fast), _canon_nodes(full)) def test_pandas_datetime_property_declines(self): + """M2/dtype pin: the CYPHER seeded projection still DECLINES a datetime property. + DELIBERATE CHANGE: the assertion used to be a plain frame_equal against the full + path, which incidentally locked the pandas merge upcast (`a` as float64). That was + never the guarantee this test exists for — the guarantee is "the Cypher fast path + declines, and the answer is still right". Since the native chain fast path now + serves the declined shape, `a` comes back int64, which is the conformant type + (see _FULL_PATH_PANDAS_UPCASTS above). Values are still asserted identical.""" q = self.Q.replace("p.flag AS f", "p.ts AS t") fast, full = self._fast_and_full(self._typed_graph(), "pandas", q, expect_engage=False) - pd.testing.assert_frame_equal(_canon_nodes(fast), _canon_nodes(full)) + self._assert_values_equal_conformant_dtypes(fast, full) @pytest.mark.parametrize("engine", ["pandas", "polars"]) def test_edges_empty_frame_not_none(self, engine): @@ -772,11 +825,18 @@ def test_edges_empty_frame_not_none(self, engine): assert full._edges.shape[0] == 0 def test_engine_mismatch_declines(self): + """M2 pin: a requested-vs-actual engine mismatch DECLINES the Cypher seeded + projection. DELIBERATE CHANGE, same reason as test_pandas_datetime_property_ + declines: the first arm's frame_equal incidentally locked the pandas merge upcast. + The declined shape is now served by the native chain fast path, so `a`/`f` come + back int64/bool — the conformant types. Note arm 2 needs no change at all: the + POLARS full path already returns int64/bool, which is the third piece of evidence + that the pandas float64 is engine-local artifact rather than Cypher semantics.""" pytest.importorskip("polars") # polars frames + engine='pandas': full converts to pandas; fast must decline fast, full = self._fast_and_full(self._pl_graph(), "pandas", self.Q, expect_engage=False) assert type(fast._nodes).__module__.startswith("pandas") - pd.testing.assert_frame_equal(_canon_nodes(fast), _canon_nodes(full)) + self._assert_values_equal_conformant_dtypes(fast, full) # pandas frames + engine='polars' (reentry direction): also declines fast2, full2 = self._fast_and_full(self._typed_graph(), "polars", self.Q, expect_engage=False) assert type(fast2._nodes).__module__ == type(full2._nodes).__module__ diff --git a/graphistry/tests/compute/test_chain.py b/graphistry/tests/compute/test_chain.py index 1f084583c5..6757d28676 100644 --- a/graphistry/tests/compute/test_chain.py +++ b/graphistry/tests/compute/test_chain.py @@ -1,9 +1,12 @@ import os +from typing import Sequence + import pandas as pd import pytest from graphistry.compute.ast import ASTEdgeUndirected, ASTNode, ASTEdge, n, e, e_undirected, e_forward, e_reverse from graphistry.compute.chain import Chain, _try_chain_fast_path +from graphistry.compute.typing import DataFrameT from graphistry.compute.predicates.is_in import IsIn, is_in from graphistry.compute.predicates.numeric import gt from graphistry.tests.test_compute import CGFull @@ -680,13 +683,27 @@ def topd(df): ("edge_match_unconstrained", lambda: [n(), e_forward(hops=1, edge_match={'w': 5}), n()]), ("edge_match_seeded", lambda: [n({'attr': 10}), e_forward(hops=1, edge_match={'w': 5}), n()]), ("edge_match_dst_filter", lambda: [n(), e_forward(hops=1, edge_match={'w': 5}), n({'attr': 30})]), + # DELIBERATE RULE CHANGE: naming an op is a PROJECTION concern, not a traversal one, + # so it no longer decides which engine path runs. `_tag_fast_path_aliases` reconstructs + # the alias flag columns `combine_steps` would have merged in, so these are now FAST + # shapes. (Was `("named_node", ...)` under _BYPASS_SHAPES: that entry encoded the old + # "any alias -> decline" gate, not a semantic guarantee.) + ("named_src", lambda: [n(name='x'), e_forward(hops=1), n()]), + ("named_dst", lambda: [n(), e_forward(hops=1), n(name='y')]), + ("named_edge", lambda: [n(), e_forward(hops=1, name='r'), n()]), + ("named_all_fwd", lambda: [n(name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("named_all_rev", lambda: [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')]), + ("named_filtered", lambda: [n({'attr': 10}, name='x'), e_forward(hops=1), n(name='y')]), ] # shapes that BYPASS the fast path (still must be correct via the full path) _BYPASS_SHAPES = [ ("hops_2", lambda: [n(), e_forward(hops=2), n()]), ("filtered_undirected", lambda: [n({'attr': 10}), e_undirected(hops=1), n({'attr': 30})]), - ("named_node", lambda: [n(name='x'), e_forward(hops=1), n()]), + # Named + undirected STAYS a bypass: an undirected edge makes a node reachable as + # EITHER endpoint, so "which alias does this node carry" is not derivable from the + # endpoint columns the way it is for a directed hop. + ("named_undirected", lambda: [n(name='x'), e_undirected(hops=1), n(name='y')]), # prune_to_endpoints: fast path returns both endpoints; full path keeps only the # arrival side. Must bypass the fast path (regression guard for the prune gate). ("prune_endpoints_fwd", lambda: [n(), e_forward(hops=1, prune_to_endpoints=True), n()]), @@ -718,6 +735,58 @@ def test_fast_path_differential_parity_vs_full_path(engine, label, build): f"{label}: bypass shape must decline the fast path" +# Named shapes whose ALIAS FLAG COLUMNS (not merely node/edge sets) must match the full +# path. `_setsig` above compares ids only, so it cannot see a wrong alias tag. +_NAMED_ALIAS_SHAPES = [ + ("src_only", lambda: [n(name='x'), e_forward(hops=1), n()]), + ("dst_only", lambda: [n(), e_forward(hops=1), n(name='y')]), + ("edge_only", lambda: [n(), e_forward(hops=1, name='r'), n()]), + ("all_forward", lambda: [n(name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("all_reverse", lambda: [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')]), + ("src_filtered", lambda: [n({'attr': 10}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("dst_filtered", lambda: [n(name='x'), e_forward(hops=1, name='r'), n({'attr': 30}, name='y')]), + ("edge_match", lambda: [n(name='x'), e_forward(hops=1, edge_match={'w': 5}, name='r'), n(name='y')]), + # DEAD END: attr==50 is node 4, which has no outgoing edge. The tag keys on the + # SURVIVING EDGES, so the alias must come back False/empty rather than True. + ("dead_end_seed", lambda: [n({'attr': 50}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), +] + + +@pytest.mark.parametrize("engine", ["pandas", "cudf"]) +@pytest.mark.parametrize("label,build", _NAMED_ALIAS_SHAPES, + ids=[s[0] for s in _NAMED_ALIAS_SHAPES]) +def test_fast_path_named_alias_columns_match_full_path(engine, label, build): + """The capability this fast-path extension actually adds: when the traversal is + served without the BFS, the alias flag columns `combine_steps` would have merged in + are RECONSTRUCTED from the surviving edges, and must be identical to the full path's + — column set, per-row values and all. Serve-asserted so a gate regression that + declines everything fails here instead of passing vacuously.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + g = _fast_graph(engine) + if engine == "pandas": + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is not None, \ + f"{label}: named shape must be SERVED by the fast path" + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + + def flags(df: DataFrameT, key_cols: Sequence[str]) -> pd.DataFrame: + # cuDF frames satisfy DataFrameT structurally but only they carry .to_pandas() + pdf = pd.DataFrame(df.to_pandas() if "cudf" in type(df).__module__ else df) # type: ignore[attr-defined] + alias_cols = sorted(set(pdf.columns) & {'x', 'y', 'r'}) + cols = list(key_cols) + alias_cols + out = pdf[cols].sort_values(cols).reset_index(drop=True) + # bool vs object is the same merge artifact as int64 vs float64 (see + # test_fast_path_preserves_int_node_dtypes); this test pins the alias VALUES, + # and the dtype rule is pinned separately. + for c in alias_cols: + out[c] = out[c].astype(bool) + return out + + pd.testing.assert_frame_equal(flags(fast._nodes, ['v']), flags(full._nodes, ['v'])) + pd.testing.assert_frame_equal(flags(fast._edges, ['s', 'd']), flags(full._edges, ['s', 'd'])) + + @pytest.mark.parametrize("engine", ["pandas", "cudf"]) def test_fast_path_preserves_int_node_dtypes(engine): """Documented behavior change: the 1-hop fast path PRESERVES node-attribute @@ -758,6 +827,16 @@ def test_fast_path_gating_returns_none_for_ineligible(): # #1755 lever-3: typed edges are now accepted (edge filter on the frontier) [n(), e_forward(hops=1, edge_match={'w': 5}), n()], [n({'attr': 10}), e_forward(hops=1, edge_match={'w': 5}), n()], + # DELIBERATE RULE CHANGE (was asserted INELIGIBLE as "named_node"): an alias is a + # PROJECTION concern, so it no longer gates the traversal path. The old assertion + # encoded the gate itself, not a semantic the fast path could not meet — the alias + # flag columns are reconstructible from the surviving edges + # (`_tag_fast_path_aliases`), which is exactly what `combine_steps` computes. + [n(name='x'), e_forward(hops=1), n()], + [n(), e_forward(hops=1), n(name='y')], + [n(), e_forward(hops=1, name='r'), n()], + [n(name='x'), e_forward(hops=1, name='r'), n(name='y')], + [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')], ] for ops in eligible: assert _try_chain_fast_path(g, ops, Engine.PANDAS, None) is not None, f"should accept {ops}" @@ -765,7 +844,14 @@ def test_fast_path_gating_returns_none_for_ineligible(): ineligible = [ ("hops_2", [n(), e_forward(hops=2), n()], None, Engine.PANDAS), ("filtered_undirected", [n({'attr': 10}), e_undirected(hops=1), n({'attr': 30})], None, Engine.PANDAS), - ("named_node", [n(name='x'), e_forward(hops=1), n()], None, Engine.PANDAS), + # NEW declines that come with serving aliases. Undirected: a node is reachable as + # EITHER endpoint, so alias identity is not derivable from the endpoint columns. + ("named_undirected", [n(name='x'), e_undirected(hops=1), n(name='y')], None, Engine.PANDAS), + ("named_undirected_edge", [n(), e_undirected(hops=1, name='r'), n(name='y')], None, Engine.PANDAS), + # Duplicate alias reuse is E201, and `combine_steps` is what raises it; serving + # here would BYPASS the check and let alias reuse silently succeed. + ("duplicate_alias_nodes", [n(name='x'), e_forward(hops=1), n(name='x')], None, Engine.PANDAS), + ("duplicate_alias_edge", [n(name='r'), e_forward(hops=1, name='r'), n()], None, Engine.PANDAS), ("node_query", [n(query='attr > 5'), e_forward(hops=1), n()], None, Engine.PANDAS), ("prune_endpoints", [n(), e_forward(hops=1, prune_to_endpoints=True), n()], None, Engine.PANDAS), ("seeded", [n()], seed, Engine.PANDAS), From cd91f19e1c45f4f953140d9223f74d0b1943e471 Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sat, 1 Aug 2026 18:06:41 -0700 Subject: [PATCH 2/4] test(gfql): harden positive+negative coverage of the named chain fast path POSITIVE (served-asserted + value parity vs the policy-forced full path): - full-frame VALUE parity on EVERY column (ids, data cols, alias flags) for named fwd/rev/seeded/edge_match shapes - previously only id sets and flag columns were compared, so 'attr'/'w' values were never checked on a named served result; also pins the served-lane dtype promises (int64 data cols, real bool alias flags) - empty-result named patterns (zero-match seed / dst filter / edge_match) stay served and return the full path's empty SHAPE with alias columns present, no throw - zero-edge-table graph: named pattern served, both lanes agree - datetime64 + categorical node columns ride through the served lane with identical values and preserved dtypes; a categorical seed filter stays served NEGATIVE (must decline, fallback value-identical): - e2e duplicate NODE alias still raises E201 on BOTH routes (the gate decline is what keeps combine_steps in charge of that error; only a gate-level check existed before) - cross-type node/edge alias share pinned as NOT-E201 (combine_steps checks per frame): the gate declines it conservatively and both routes agree on values - named patterns DEFER to a VALID covering resident index, and only then: unnamed is not deferred, a reverse hop with only edge_out_adj built is not deferred, and the deferred answer is value-identical to the full path - mixed supported+unsupported gate declines: named + hops=2 / node query / edge_query / source_node_match / prune_to_endpoints / seeded start_nodes - alias shadowing an existing data or TO-side binding column: parity pins (both lanes do the same pre-existing overwrite); cross-frame same-names pinned as no-ops BUGS found while testing, pinned as STRICT xfails rather than papered over: - node alias == the node id binding ('v'): the fast path SERVES and silently overwrites the id column with the bool flag while the full path raises ValueError - edge alias == the hop's FROM-side binding (forward+name='s', reverse+name='d'): both lanes serve but return DIFFERENT node sets (fast [0..4] vs full [1..4]) Also annotates the six shape tables (List[Tuple[str, Callable[[], List[ASTObject]]]]), fixing the pre-existing mypy operator error at the _FAST_SHAPES + _BYPASS_SHAPES parametrize; file is now mypy-clean and ruff-clean. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr --- graphistry/tests/compute/test_chain.py | 323 ++++++++++++++++++++++++- 1 file changed, 318 insertions(+), 5 deletions(-) diff --git a/graphistry/tests/compute/test_chain.py b/graphistry/tests/compute/test_chain.py index 6757d28676..27b2c65e86 100644 --- a/graphistry/tests/compute/test_chain.py +++ b/graphistry/tests/compute/test_chain.py @@ -1,10 +1,10 @@ import os -from typing import Sequence +from typing import Callable, List, Sequence, Tuple import pandas as pd import pytest -from graphistry.compute.ast import ASTEdgeUndirected, ASTNode, ASTEdge, n, e, e_undirected, e_forward, e_reverse +from graphistry.compute.ast import ASTEdgeUndirected, ASTNode, ASTEdge, ASTObject, n, e, e_undirected, e_forward, e_reverse from graphistry.compute.chain import Chain, _try_chain_fast_path from graphistry.compute.typing import DataFrameT from graphistry.compute.predicates.is_in import IsIn, is_in @@ -667,7 +667,7 @@ def topd(df): # shapes that ARE accelerated by the fast path -_FAST_SHAPES = [ +_FAST_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ ("node_only", lambda: [n()]), ("node_filter", lambda: [n({'attr': 20})]), ("node_pred", lambda: [n({'attr': is_in([10, 30])})]), @@ -697,7 +697,7 @@ def topd(df): ] # shapes that BYPASS the fast path (still must be correct via the full path) -_BYPASS_SHAPES = [ +_BYPASS_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ ("hops_2", lambda: [n(), e_forward(hops=2), n()]), ("filtered_undirected", lambda: [n({'attr': 10}), e_undirected(hops=1), n({'attr': 30})]), # Named + undirected STAYS a bypass: an undirected edge makes a node reachable as @@ -737,7 +737,7 @@ def test_fast_path_differential_parity_vs_full_path(engine, label, build): # Named shapes whose ALIAS FLAG COLUMNS (not merely node/edge sets) must match the full # path. `_setsig` above compares ids only, so it cannot see a wrong alias tag. -_NAMED_ALIAS_SHAPES = [ +_NAMED_ALIAS_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ ("src_only", lambda: [n(name='x'), e_forward(hops=1), n()]), ("dst_only", lambda: [n(), e_forward(hops=1), n(name='y')]), ("edge_only", lambda: [n(), e_forward(hops=1, name='r'), n()]), @@ -787,6 +787,310 @@ def flags(df: DataFrameT, key_cols: Sequence[str]) -> pd.DataFrame: pd.testing.assert_frame_equal(flags(fast._edges, ['s', 'd']), flags(full._edges, ['s', 'd'])) +def _norm_all_cols(df: DataFrameT, key_cols: Sequence[str]) -> pd.DataFrame: + """Full-frame canonicalization: ALL columns, sorted column order, key-sorted rows.""" + # cuDF frames satisfy DataFrameT structurally but only they carry .to_pandas() + pdf = pd.DataFrame(df.to_pandas() if "cudf" in type(df).__module__ else df) # type: ignore[attr-defined] + cols = sorted(pdf.columns) + return pdf[cols].sort_values(list(key_cols)).reset_index(drop=True) + + +def _assert_full_frame_value_parity(fast: DataFrameT, full: DataFrameT, + key_cols: Sequence[str]) -> None: + """Same columns, same per-row VALUES on every column. Dtype is deliberately NOT + compared: the served lane keeps the Cypher-conformant int64/bool where the full + path's merges upcast to float64/object (pinned separately).""" + f, u = _norm_all_cols(fast, key_cols), _norm_all_cols(full, key_cols) + assert list(f.columns) == list(u.columns), f"column sets differ: {list(f.columns)} vs {list(u.columns)}" + assert len(f) == len(u), f"row counts differ: {len(f)} vs {len(u)}" + for c in f.columns: + pd.testing.assert_series_equal( + f[c], u[c], check_names=False, check_dtype=False, check_categorical=False) + + +# Named served shapes for FULL-FRAME parity. `_setsig` compares id sets and the flags +# test compares alias columns, so before this NO test compared the carried DATA columns +# ('attr', 'w') of a named served result against the full path. +_NAMED_VALUE_PARITY_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ + ("all_forward", lambda: [n(name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("all_reverse", lambda: [n(name='x'), e_reverse(hops=1, name='r'), n(name='y')]), + ("seed_filtered", lambda: [n({'attr': 10}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("edge_match", lambda: [n(name='x'), e_forward(hops=1, edge_match={'w': 5}, name='r'), n(name='y')]), +] + + +@pytest.mark.parametrize("engine", ["pandas", "cudf"]) +@pytest.mark.parametrize("label,build", _NAMED_VALUE_PARITY_SHAPES, + ids=[s[0] for s in _NAMED_VALUE_PARITY_SHAPES]) +def test_fast_path_named_full_frame_value_parity(engine, label, build): + """POSITIVE, whole-frame: a named served result must carry the same VALUES as the + full path on EVERY column — ids, data columns, and alias flags — not just the id + sets and flag columns the other tests pin. Also pins the served lane's documented + dtype promises: data ints stay int64 and alias flags are real bools.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + g = _fast_graph(engine) + if engine == "pandas": + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is not None, \ + f"{label}: named shape must be SERVED by the fast path" + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['s', 'd']) + # served-lane dtype promises (the full path upcasts via merge; pinned in + # test_fast_path_preserves_int_node_dtypes and the CHANGELOG dtype note) + fn = _norm_all_cols(fast._nodes, ['v']) + assert fn['attr'].dtype.kind == 'i', "served lane must keep int node attrs int" + for c in set(fn.columns) & {'x', 'y'}: + assert fn[c].dtype == bool, f"served alias flag {c} must be bool, got {fn[c].dtype}" + fe = _norm_all_cols(fast._edges, ['s', 'd']) + for c in set(fe.columns) & {'r'}: + assert fe[c].dtype == bool, f"served alias flag {c} must be bool, got {fe[c].dtype}" + + +# Named shapes whose CORRECT answer is EMPTY. The serve gate cannot see result +# cardinality, so these all engage the fast path — and an empty answer must come back +# as the right empty SHAPE (alias columns present, zero rows), not a throw and not a +# missing-column frame. +_NAMED_EMPTY_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ + # seed filter matches no node at all (distinct from dead_end_seed, which matches + # a node that has no surviving edge) + ("zero_seed", lambda: [n({'attr': 999}, name='x'), e_forward(hops=1, name='r'), n(name='y')]), + ("zero_dst", lambda: [n(name='x'), e_forward(hops=1, name='r'), n({'attr': 999}, name='y')]), + ("zero_edge_match", lambda: [n(name='x'), e_forward(hops=1, edge_match={'w': 999}, name='r'), n(name='y')]), +] + + +@pytest.mark.parametrize("engine", ["pandas", "cudf"]) +@pytest.mark.parametrize("label,build", _NAMED_EMPTY_SHAPES, + ids=[s[0] for s in _NAMED_EMPTY_SHAPES]) +def test_fast_path_named_empty_result_matches_full_path(engine, label, build): + """POSITIVE boundary: named patterns matching ZERO rows are still served, and the + empty result must be shape-identical to the full path — same columns INCLUDING the + alias flag columns, zero rows, no exception.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + g = _fast_graph(engine) + if engine == "pandas": + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is not None, \ + f"{label}: empty-result named shape must still be SERVED" + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + fn, un = _norm_all_cols(fast._nodes, ['v']), _norm_all_cols(full._nodes, ['v']) + fe, ue = _norm_all_cols(fast._edges, ['s', 'd']), _norm_all_cols(full._edges, ['s', 'd']) + assert len(fn) == 0 and len(fe) == 0, f"{label}: expected empty result" + assert list(fn.columns) == list(un.columns), "empty nodes must keep alias columns" + assert list(fe.columns) == list(ue.columns), "empty edges must keep alias columns" + assert {'x', 'y'} <= set(fn.columns) and 'r' in fe.columns + assert len(un) == 0 and len(ue) == 0 + + +def test_fast_path_named_zero_edge_graph_matches_full_path(): + """POSITIVE boundary: a graph with an EMPTY edge table. The named pattern is served, + and both lanes must agree on the all-empty answer with alias columns present.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + nodes = pd.DataFrame({'v': [0, 1], 'attr': [10, 20]}) + edges = pd.DataFrame({'s': pd.Series([], dtype='int64'), + 'd': pd.Series([], dtype='int64'), + 'w': pd.Series([], dtype='int64')}) + g = CGFull().nodes(nodes, 'v').edges(edges, 's', 'd') + ops = [n(name='x'), e_forward(hops=1, name='r'), n(name='y')] + assert _try_chain_fast_path(g, ops, Engine.PANDAS, None) is not None + fast = g.gfql(ops) + full = g.gfql(ops, policy=_FAST_NOOP_POLICY) + for res in (fast, full): + assert res._nodes.shape[0] == 0 and res._edges.shape[0] == 0 + assert {'x', 'y'} <= set(fast._nodes.columns) and 'r' in fast._edges.columns + assert list(sorted(fast._nodes.columns)) == list(sorted(full._nodes.columns)) + assert list(sorted(fast._edges.columns)) == list(sorted(full._edges.columns)) + + +@pytest.mark.parametrize("route", ["fast_default", "full_policy"]) +def test_fast_path_duplicate_alias_still_raises_e201(route): + """NEGATIVE, end-to-end: duplicate NODE alias reuse must still RAISE E201 through + the public API. The gate-level decline (asserted in + test_fast_path_gating_returns_none_for_ineligible) exists precisely so + `combine_steps` stays in charge of this error; if the decline ever regressed, alias + reuse would silently SUCCEED on the served lane — this pins the user-visible raise + on both routes so that regression cannot land quietly.""" + from graphistry.compute.exceptions import ErrorCode, GFQLValidationError + g = _fast_graph("pandas") + kwargs = {} if route == "fast_default" else {"policy": _FAST_NOOP_POLICY} + with pytest.raises(GFQLValidationError) as exc_info: + g.gfql([n(name='x'), e_forward(hops=1), n(name='x')], **kwargs) + assert exc_info.value.code == ErrorCode.E201 + assert "'x'" in str(exc_info.value) + + +def test_fast_path_cross_type_alias_share_declines_and_matches(): + """NEGATIVE boundary pinning a subtlety found while testing: a NODE alias and an + EDGE alias sharing one name is NOT an E201 — `combine_steps` checks duplicates + per frame (node pass vs edge pass), and the two flag columns land on different + frames. The gate still declines it conservatively (duplicate_alias_edge in the + ineligible list), which is safe exactly because the full path serves it — so pin + that the two routes AGREE: no raise, same values, the flag on both frames.""" + g = _fast_graph("pandas") + ops = lambda: [n(name='r'), e_forward(hops=1, name='r'), n()] # noqa: E731 + default_route = g.gfql(ops()) + policy_route = g.gfql(ops(), policy=_FAST_NOOP_POLICY) + for res in (default_route, policy_route): + assert 'r' in res._nodes.columns and 'r' in res._edges.columns + _assert_full_frame_value_parity(default_route._nodes, policy_route._nodes, ['v']) + _assert_full_frame_value_parity(default_route._edges, policy_route._edges, ['s', 'd']) + + +def test_fast_path_named_defers_to_valid_resident_index(): + """NEGATIVE (deliberate decline): when BOTH resident indexes validly cover the + directed hop, a NAMED pattern must DECLINE the chain fast path so the index path + (which the gate would shadow) serves it — and the answer must not change. The gate + keys on index VALIDITY, so an index that does NOT cover the shape (reverse hop with + only the out-adjacency built) must NOT cause a decline, and unnamed patterns are + not deferred at all.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + from graphistry.compute.gfql.index.api import create_index + g = _fast_graph("pandas") + gi = create_index(create_index(g, 'edge_out_adj'), 'node_id') + named = [n(name='x'), e_forward(hops=1), n(name='y')] + assert _try_chain_fast_path(gi, named, Engine.PANDAS, None) is None, \ + "named + valid covering index must defer (decline) to the index path" + assert _try_chain_fast_path(gi, [n(), e_forward(hops=1), n()], Engine.PANDAS, None) is not None, \ + "unnamed is not deferred: the index deferral is scoped to named patterns" + assert _try_chain_fast_path(gi, [n(name='x'), e_reverse(hops=1), n(name='y')], Engine.PANDAS, None) is not None, \ + "reverse hop is NOT covered by edge_out_adj alone, so no deferral" + # deferral must be a routing decision only — values identical to the full path + deferred = gi.gfql(named) + full = g.gfql(named, policy=_FAST_NOOP_POLICY) + assert _setsig(deferred) == _setsig(full) + _assert_full_frame_value_parity(deferred._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(deferred._edges, full._edges, ['s', 'd']) + + +def test_fast_path_named_datetime_categorical_columns_ride_along(): + """POSITIVE dtype edge: datetime64 and categorical NODE columns must ride through + the served named lane unchanged — same values as the full path, dtypes preserved + (the served lane never merges, so it must not degrade either dtype) — and a seed + FILTER over the categorical column must both stay served and agree on values.""" + from graphistry.compute.chain import _try_chain_fast_path + from graphistry.Engine import Engine + nodes = pd.DataFrame({ + 'v': [0, 1, 2, 3, 4], + 'ts': pd.to_datetime(['2024-01-01', '2024-01-02', '2024-01-03', '2024-01-04', '2024-01-05']), + 'cat': pd.Categorical(['a', 'b', 'a', 'c', 'b']), + }) + edges = pd.DataFrame({'s': [0, 1, 2, 3, 0], 'd': [1, 2, 3, 4, 2], 'w': [5, 6, 7, 8, 9]}) + g = CGFull().nodes(nodes, 'v').edges(edges, 's', 'd') + ops = [n(name='x'), e_forward(hops=1, name='r'), n(name='y')] + assert _try_chain_fast_path(g, ops, Engine.PANDAS, None) is not None + fast = g.gfql(ops) + full = g.gfql(ops, policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['s', 'd']) + assert str(fast._nodes['ts'].dtype) == 'datetime64[ns]' + assert str(fast._nodes['cat'].dtype) == 'category' + # categorical seed filter: still served, same answer + ops2 = [n({'cat': 'a'}, name='x'), e_forward(hops=1), n(name='y')] + assert _try_chain_fast_path(g, ops2, Engine.PANDAS, None) is not None + f2 = g.gfql(ops2) + u2 = g.gfql(ops2, policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(f2._nodes, u2._nodes, ['v']) + _assert_full_frame_value_parity(f2._edges, u2._edges, ['s', 'd']) + + +# Alias names that SHADOW a real column WITHOUT breaking parity. Today BOTH lanes +# overwrite the shadowed column with the flag, identically — that (pre-existing, +# full-path) contract is what these pin, so a lane can't drift to a different +# overwrite/raise behavior alone. The FROM-side binding columns are excluded: those +# DIVERGE today and are pinned as strict xfails below. +_ALIAS_SHADOW_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ + ("node_alias_shadows_node_data_col", lambda: [n(name='attr'), e_forward(hops=1), n()]), + ("edge_alias_shadows_edge_data_col", lambda: [n(), e_forward(hops=1, name='w'), n()]), + # TO-side binding columns keep parity (the from-side ones do not, see xfail below) + ("edge_alias_shadows_dst_binding_fwd", lambda: [n(), e_forward(hops=1, name='d'), n()]), + ("edge_alias_shadows_src_binding_rev", lambda: [n(), e_reverse(hops=1, name='s'), n()]), + # cross-frame names are NOT collisions: nodes have no 'w', edges have no 'v' + ("node_alias_named_like_edge_col", lambda: [n(name='w'), e_forward(hops=1), n()]), + ("edge_alias_named_like_node_id", lambda: [n(), e_forward(hops=1, name='v'), n()]), +] + + +@pytest.mark.parametrize("label,build", _ALIAS_SHADOW_SHAPES, + ids=[s[0] for s in _ALIAS_SHADOW_SHAPES]) +def test_fast_path_alias_shadowing_column_matches_full_path(label, build): + """NEGATIVE-ish boundary: alias names that collide with existing data/binding + columns must behave IDENTICALLY on both lanes (today: same silent overwrite the + full path has always done; cross-frame same-name cases are no-ops). Edge key + columns may themselves be overwritten here, so edges are keyed on 'w' (unique).""" + g = _fast_graph("pandas") + fast = g.gfql(build()) + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['w']) + + +@pytest.mark.xfail(strict=True, reason=( + "BUG (wrong-serve): an EDGE alias that collides with the hop's FROM-side binding " + "column (forward + name=src, reverse + name=dst) is SERVED by the fast path but " + "the two lanes return DIFFERENT node sets (e.g. fast [0..4] vs full [1..4] on the " + "5-node fixture): the full path's overwrite of the from-column corrupts its own " + "node reduction, the fast path tags after reducing. TO-side collisions keep " + "parity (pinned above). The lanes must agree — the gate should decline an edge " + "alias equal to the active from-binding (or both lanes should raise).")) +@pytest.mark.parametrize("build", [ + lambda: [n(), e_forward(hops=1, name='s'), n()], + lambda: [n(), e_reverse(hops=1, name='d'), n()], +], ids=["fwd_alias_is_src_binding", "rev_alias_is_dst_binding"]) +def test_fast_path_edge_alias_colliding_with_from_binding_matches_full_path(build): + """The two lanes must be observationally equivalent: both raise, or both return + the same frames.""" + g = _fast_graph("pandas") + try: + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + full_raised = None + except Exception as ex: # noqa: BLE001 — parity contract, not error-type contract + full, full_raised = None, type(ex) + if full_raised is not None: + with pytest.raises(full_raised): + g.gfql(build()) + else: + fast = g.gfql(build()) + assert full is not None + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['w']) + + +@pytest.mark.xfail(strict=True, reason=( + "BUG (wrong-serve): a node alias that collides with the NODE ID BINDING column " + "('v') is SERVED by the fast path, which silently OVERWRITES the id column with " + "the bool alias flag — the node ids are destroyed. The full path raises for the " + "same query (pandas: \"The column label 'v' is not unique\"). The two lanes must " + "agree: the gate should decline (or both should raise). Fix: decline when a node " + "alias equals g._node (and audit src/dst equivalents on the edge side).")) +@pytest.mark.parametrize("build", [ + lambda: [n(name='v'), e_forward(hops=1), n()], + lambda: [n(), e_forward(hops=1), n(name='v')], +], ids=["n0_alias_is_node_binding", "n2_alias_is_node_binding"]) +def test_fast_path_alias_colliding_with_node_id_binding_matches_full_path(build): + """The two lanes must be observationally equivalent: both raise, or both return + the same frames. Today the served lane returns frames whose 'v' column holds the + alias FLAGS instead of the node ids, while the full path raises ValueError.""" + g = _fast_graph("pandas") + try: + full = g.gfql(build(), policy=_FAST_NOOP_POLICY) + full_raised = None + except Exception as ex: # noqa: BLE001 — parity contract, not error-type contract + full, full_raised = None, type(ex) + if full_raised is not None: + with pytest.raises(full_raised): + g.gfql(build()) + else: + fast = g.gfql(build()) + assert full is not None + _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) + _assert_full_frame_value_parity(fast._edges, full._edges, ['s', 'd']) + + @pytest.mark.parametrize("engine", ["pandas", "cudf"]) def test_fast_path_preserves_int_node_dtypes(engine): """Documented behavior change: the 1-hop fast path PRESERVES node-attribute @@ -855,6 +1159,15 @@ def test_fast_path_gating_returns_none_for_ineligible(): ("node_query", [n(query='attr > 5'), e_forward(hops=1), n()], None, Engine.PANDAS), ("prune_endpoints", [n(), e_forward(hops=1, prune_to_endpoints=True), n()], None, Engine.PANDAS), ("seeded", [n()], seed, Engine.PANDAS), + # MIXED supported + unsupported: an alias is now a served concern, but it must + # never FLIP an otherwise-ineligible shape to served — the unsupported piece + # (multi-hop, queries, richer predicates, prune, seeds) still declines the whole op list. + ("named_hops_2", [n(name='x'), e_forward(hops=2), n(name='y')], None, Engine.PANDAS), + ("named_node_query", [n(query='attr > 5', name='x'), e_forward(hops=1), n(name='y')], None, Engine.PANDAS), + ("named_edge_query", [n(name='x'), e_forward(hops=1, edge_query='w > 5'), n(name='y')], None, Engine.PANDAS), + ("named_source_node_match", [n(name='x'), e_forward(hops=1, source_node_match={'attr': 10}), n(name='y')], None, Engine.PANDAS), + ("named_prune_endpoints", [n(name='x'), e_forward(hops=1, prune_to_endpoints=True), n(name='y')], None, Engine.PANDAS), + ("named_seeded", [n(name='x'), e_forward(hops=1), n(name='y')], seed, Engine.PANDAS), ("non_eager_engine", [n()], None, Engine.DASK), ("two_ops", [n(), e_forward(hops=1)], None, Engine.PANDAS), ] From d62783c141046978d5dd6a20dd8b1867efbb09bd Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sat, 1 Aug 2026 18:11:11 -0700 Subject: [PATCH 3/4] fix(gfql): decline the chain fast path on alias/binding-column collisions Adversarial parity testing (previous commit) found two WRONG-SERVES in the named fast path, pinned there as strict xfails. This gates both to DECLINE - never serve - and flips the four xfail params to hard passes: - a NODE alias equal to the node-id binding (g._node): the tagger would OVERWRITE the id column with the bool flag (destroying the ids) while the full path raises "The column label '' is not unique" on the same query. Now declined; the test asserts the gate declines AND both routes raise the same error class. - an EDGE alias equal to the hop's FROM-side binding (forward+src / reverse+dst): the two lanes returned DIFFERENT node sets (full path's flag overwrite corrupts its own node reduction; the fast path tags after reducing). Now declined; the test asserts the gate declines AND full-frame value parity. Scope checked before gating: TO-side binding collisions (forward+dst, reverse+src) and ALL undirected edge-alias collisions (name = src/dst/data col/node id) keep lane parity - probed explicitly - so they stay served and are pinned as parity tests, not gated. The new declines are also pinned structurally in the gate's ineligible list (n0/n2 alias == node binding, edge alias == src fwd / dst rev). CHANGELOG: the PR entry's "deliberate declines kept" list now includes the binding-collision class. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr --- CHANGELOG.md | 2 +- graphistry/compute/chain.py | 13 +++++++ graphistry/tests/compute/test_chain.py | 52 +++++++++++++++----------- 3 files changed, 44 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67b0d0e501..615b1b1cc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL indexed fixed-hop bindings now dispatch BEFORE the canonical traversal (pandas / cuDF / native Polars)**: the resident-index fixed-hop path (`rows(binding_ops=...)`, the Cypher multi-alias `MATCH … RETURN` shape) was consulted only *inside* row materialization — after the engine had already run the full canonical traversal — so its compact path bag was computed on top of the graph-sized work it exists to avoid. Both the pandas/cuDF chain boundary and the native Polars chain now ask the same shared, structural gate first and, only when it can serve the WHOLE middle exactly, skip the canonical traversal and hand the compact state to the unchanged row materializer. Engagement stays operator/index/dtype/cost based — no query, schema, or hop-count recognition — and every unsupported, seeded, prefiltered, policy-bearing, shortest-path, or cost-gated shape still declines to canonical execution with identical results. On a standard LDBC SNB SF1 interactive-short profile this removed the redundant two-hop typed-mask and 16-merge buckets and cut the profiled call ~11.9× (1263 ms → 106 ms), exact 19-row oracle preserved. - **Indexed seed lookup now covers a unique node id PLUS extra scalar constraints**: `{node_id: v, other: w}` previously fell back to a full node-table scan even though the unique node-id index can gather at most one row; it now gathers that one row and applies the remaining constraints to it. Missing ids and non-matching extra constraints return the same empty result as before, and duplicate-id graphs (which cannot build the index) are unaffected. - **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. -- **The pandas/cuDF chain fast path serves NAMED patterns**: `_try_chain_fast_path` rejected any op carrying an alias, so `g.gfql([n(name='x'), e_forward(name='r'), n(name='y')])` fell to the full two-pass BFS purely because the ops were named — naming is a PROJECTION concern, not a traversal one, and it should not decide which engine path runs. The alias flag columns `combine_steps` would have merged in are now reconstructed directly from the edges the fast path returns (`_tag_fast_path_aliases`): `combine_steps` tags a node with an alias iff it still participates in a surviving edge, and those edges are exactly the ones the fast path already produced, so `isin` over their endpoint columns is the same predicate computed without the join. A seed whose edges all fail the filter yields an empty edge frame and is tagged `False`, which is the dead-end case. Deliberate declines kept: **undirected + named** (an undirected edge makes a node reachable as either endpoint, so alias identity is not derivable from the endpoint columns), **duplicate alias names** (E201 is raised by `combine_steps`, so serving here would bypass the check and let alias reuse silently succeed), and **a resident index that would actually serve the shape** (checked with `_resident_seed_indexes`, i.e. index VALIDITY for these exact frames — a registry-presence check instead declines on every query and silently turns the whole optimization into a measured no-op). **Measured** on a 200-node / 800-edge pandas graph, where data-proportional work is ~0 so this is the per-query plan floor. Five alternating paired runs against `origin/master`, each the median of 200 reps after 20 warmups: the named 3-op traversal `25.2 ms (24.2–27.0) → 2.3 ms (2.1–2.7)`, **~11×**, and the same pattern followed by `rows(binding_ops=...)` `40.4 ms (38.1–43.6) → 17.4 ms (16.8–19.8)`, **~2.3×**. The `rows()` output is byte-identical between the two arms and the traversal output is value-identical (dtypes differ deliberately, see *Changed*). **Scope, stated plainly**: this is a NATIVE `g.gfql([...named...])` / `g.chain([...named...])` improvement only. It is pandas/cuDF-only, and an instrumented run of the graph-benchmark q1–q9 lane showed the chain fast path is **never consulted** for those Cypher queries — they are served earlier by the Cypher-layer fast paths in `gfql_fast_paths.py`, or hit a separate single-op gate — so **no benchmark cell moves** and no lane currently exercises this. A benchmark lane over the native named-chain surface is required follow-up before any perf claim is made on the board. +- **The pandas/cuDF chain fast path serves NAMED patterns**: `_try_chain_fast_path` rejected any op carrying an alias, so `g.gfql([n(name='x'), e_forward(name='r'), n(name='y')])` fell to the full two-pass BFS purely because the ops were named — naming is a PROJECTION concern, not a traversal one, and it should not decide which engine path runs. The alias flag columns `combine_steps` would have merged in are now reconstructed directly from the edges the fast path returns (`_tag_fast_path_aliases`): `combine_steps` tags a node with an alias iff it still participates in a surviving edge, and those edges are exactly the ones the fast path already produced, so `isin` over their endpoint columns is the same predicate computed without the join. A seed whose edges all fail the filter yields an empty edge frame and is tagged `False`, which is the dead-end case. Deliberate declines kept: **alias names that collide with a binding column the tagger would overwrite** (a node alias equal to the node-id binding wrong-served — the flag OVERWROTE the id column while the full path raises — and an edge alias equal to the hop's FROM-side binding returned a DIFFERENT node set per lane; both found by adversarial parity testing and now declined, with decline + raise/parity pins; TO-side collisions keep parity and stay served), **undirected + named** (an undirected edge makes a node reachable as either endpoint, so alias identity is not derivable from the endpoint columns), **duplicate alias names** (E201 is raised by `combine_steps`, so serving here would bypass the check and let alias reuse silently succeed), and **a resident index that would actually serve the shape** (checked with `_resident_seed_indexes`, i.e. index VALIDITY for these exact frames — a registry-presence check instead declines on every query and silently turns the whole optimization into a measured no-op). **Measured** on a 200-node / 800-edge pandas graph, where data-proportional work is ~0 so this is the per-query plan floor. Five alternating paired runs against `origin/master`, each the median of 200 reps after 20 warmups: the named 3-op traversal `25.2 ms (24.2–27.0) → 2.3 ms (2.1–2.7)`, **~11×**, and the same pattern followed by `rows(binding_ops=...)` `40.4 ms (38.1–43.6) → 17.4 ms (16.8–19.8)`, **~2.3×**. The `rows()` output is byte-identical between the two arms and the traversal output is value-identical (dtypes differ deliberately, see *Changed*). **Scope, stated plainly**: this is a NATIVE `g.gfql([...named...])` / `g.chain([...named...])` improvement only. It is pandas/cuDF-only, and an instrumented run of the graph-benchmark q1–q9 lane showed the chain fast path is **never consulted** for those Cypher queries — they are served earlier by the Cypher-layer fast paths in `gfql_fast_paths.py`, or hit a separate single-op gate — so **no benchmark cell moves** and no lane currently exercises this. A benchmark lane over the native named-chain surface is required follow-up before any perf claim is made on the board. ### Changed - **A served chain fast path now returns Cypher-conformant int/bool dtypes on named patterns too**: the pandas full path's rows-pivot upcasts non-id `int64 → float64` and `bool → object` through its merges. That is a pandas merge artifact, not a Cypher semantic — the openCypher TCK (`clauses/return/Return2.feature`, "Returning a node property value": `CREATE ({num: 1})` / `RETURN a.num` expects `1`, and the TCK value grammar writes an Integer as bare decimal digits and a Float with its decimals) and Neo4j (INTEGER and FLOAT are distinct property types; `valueType()` on an integer property reports `INTEGER NOT NULL`) both keep the integer, and this library's own **polars** and **cuDF** canonical paths already return `int64`/`bool` for the same query, so only the pandas merge upcasts. The chain fast path already preserved the conformant dtypes for UNNAMED patterns (documented in its docstring); serving named patterns extends that to the named surface rather than creating a new divergence, and the conformant dtype is deliberately kept rather than cast back to reproduce the defect. **User-visible** where a named pandas pattern is served by the fast path: an integer node property comes back `int64` instead of `float64` (`30` rather than `30.0`, including in JSON), and an alias flag column comes back `bool` instead of `object`. Values are unchanged. **Not aligned yet** (deliberately out of scope, follow-up): the pandas full path itself, and the Cypher-layer seeded projection, still emit the artifact — so pandas can still report `float64` for the same logical query depending on which internal path serves it. diff --git a/graphistry/compute/chain.py b/graphistry/compute/chain.py index cfc3fd64fc..bcb2093d41 100644 --- a/graphistry/compute/chain.py +++ b/graphistry/compute/chain.py @@ -899,6 +899,19 @@ def _materialize_fast_path_graph() -> Plottable: src, dst, node = g._source, g._destination, g._node if src is None or dst is None or node is None: return None # no edge/node bindings -> can't fast-path; full path handles it + if alias_n0 == node or alias_n2 == node: + # A node alias EQUAL TO THE NODE-ID BINDING would make `_tag_fast_path_aliases` + # overwrite the id column with the bool flag (destroying the ids) while the full + # path raises on the same query ("The column label '' is not unique") — + # a wrong-serve found by adversarial parity testing. Decline; never serve. + return None + if alias_e1 is not None and direction in ("forward", "reverse") \ + and alias_e1 == (src if direction == "forward" else dst): + # An edge alias EQUAL TO THE HOP'S FROM-SIDE BINDING (forward+src / reverse+dst) + # made the two lanes return DIFFERENT node sets: the full path's flag overwrite + # corrupts its own node reduction, the fast path tags after reducing. TO-side + # collisions keep parity (pinned in tests) and stay served. Decline; never serve. + return None if (alias_n0 is not None or alias_e1 is not None or alias_n2 is not None) \ and direction != "undirected" and g._nodes is not None and g._edges is not None: # DEFER TO THE INDEX when one would ACTUALLY serve. A resident index is diff --git a/graphistry/tests/compute/test_chain.py b/graphistry/tests/compute/test_chain.py index 27b2c65e86..766e0d8ae3 100644 --- a/graphistry/tests/compute/test_chain.py +++ b/graphistry/tests/compute/test_chain.py @@ -1001,8 +1001,9 @@ def test_fast_path_named_datetime_categorical_columns_ride_along(): # Alias names that SHADOW a real column WITHOUT breaking parity. Today BOTH lanes # overwrite the shadowed column with the flag, identically — that (pre-existing, # full-path) contract is what these pin, so a lane can't drift to a different -# overwrite/raise behavior alone. The FROM-side binding columns are excluded: those -# DIVERGE today and are pinned as strict xfails below. +# overwrite/raise behavior alone. The FROM-side binding columns and the node-id +# binding are excluded here: those wrong-served (diverged) before, are now GATED to +# decline, and are pinned by the two regression tests below. _ALIAS_SHADOW_SHAPES: List[Tuple[str, Callable[[], List[ASTObject]]]] = [ ("node_alias_shadows_node_data_col", lambda: [n(name='attr'), e_forward(hops=1), n()]), ("edge_alias_shadows_edge_data_col", lambda: [n(), e_forward(hops=1, name='w'), n()]), @@ -1029,22 +1030,22 @@ def test_fast_path_alias_shadowing_column_matches_full_path(label, build): _assert_full_frame_value_parity(fast._edges, full._edges, ['w']) -@pytest.mark.xfail(strict=True, reason=( - "BUG (wrong-serve): an EDGE alias that collides with the hop's FROM-side binding " - "column (forward + name=src, reverse + name=dst) is SERVED by the fast path but " - "the two lanes return DIFFERENT node sets (e.g. fast [0..4] vs full [1..4] on the " - "5-node fixture): the full path's overwrite of the from-column corrupts its own " - "node reduction, the fast path tags after reducing. TO-side collisions keep " - "parity (pinned above). The lanes must agree — the gate should decline an edge " - "alias equal to the active from-binding (or both lanes should raise).")) @pytest.mark.parametrize("build", [ lambda: [n(), e_forward(hops=1, name='s'), n()], lambda: [n(), e_reverse(hops=1, name='d'), n()], ], ids=["fwd_alias_is_src_binding", "rev_alias_is_dst_binding"]) def test_fast_path_edge_alias_colliding_with_from_binding_matches_full_path(build): - """The two lanes must be observationally equivalent: both raise, or both return - the same frames.""" + """REGRESSION (wrong-serve, found by adversarial parity testing, now GATED): an + edge alias equal to the hop's FROM-side binding column (forward+name=src, + reverse+name=dst) used to be SERVED with a DIFFERENT node set than the full path + (fast [0..4] vs full [1..4] on this fixture — the full path's flag overwrite + corrupts its own node reduction). The gate now DECLINES it; TO-side collisions + keep parity and stay served (pinned above). Both routes must agree: both raise, + or both return the same frames.""" + from graphistry.Engine import Engine g = _fast_graph("pandas") + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is None, \ + "edge alias == FROM-side binding must DECLINE the fast path" try: full = g.gfql(build(), policy=_FAST_NOOP_POLICY) full_raised = None @@ -1060,22 +1061,20 @@ def test_fast_path_edge_alias_colliding_with_from_binding_matches_full_path(buil _assert_full_frame_value_parity(fast._edges, full._edges, ['w']) -@pytest.mark.xfail(strict=True, reason=( - "BUG (wrong-serve): a node alias that collides with the NODE ID BINDING column " - "('v') is SERVED by the fast path, which silently OVERWRITES the id column with " - "the bool alias flag — the node ids are destroyed. The full path raises for the " - "same query (pandas: \"The column label 'v' is not unique\"). The two lanes must " - "agree: the gate should decline (or both should raise). Fix: decline when a node " - "alias equals g._node (and audit src/dst equivalents on the edge side).")) @pytest.mark.parametrize("build", [ lambda: [n(name='v'), e_forward(hops=1), n()], lambda: [n(), e_forward(hops=1), n(name='v')], ], ids=["n0_alias_is_node_binding", "n2_alias_is_node_binding"]) def test_fast_path_alias_colliding_with_node_id_binding_matches_full_path(build): - """The two lanes must be observationally equivalent: both raise, or both return - the same frames. Today the served lane returns frames whose 'v' column holds the - alias FLAGS instead of the node ids, while the full path raises ValueError.""" + """REGRESSION (wrong-serve, found by adversarial parity testing, now GATED): a + node alias equal to the NODE ID BINDING column ('v') used to be SERVED, silently + overwriting the id column with the bool alias flag while the full path raised. + The gate now DECLINES it, so the two lanes are observationally equivalent: both + raise, or both return the same frames (today: both raise pandas' ValueError).""" + from graphistry.Engine import Engine g = _fast_graph("pandas") + assert _try_chain_fast_path(g, build(), Engine.PANDAS, None) is None, \ + "node alias == node-id binding must DECLINE the fast path" try: full = g.gfql(build(), policy=_FAST_NOOP_POLICY) full_raised = None @@ -1168,6 +1167,15 @@ def test_fast_path_gating_returns_none_for_ineligible(): ("named_source_node_match", [n(name='x'), e_forward(hops=1, source_node_match={'attr': 10}), n(name='y')], None, Engine.PANDAS), ("named_prune_endpoints", [n(name='x'), e_forward(hops=1, prune_to_endpoints=True), n(name='y')], None, Engine.PANDAS), ("named_seeded", [n(name='x'), e_forward(hops=1), n(name='y')], seed, Engine.PANDAS), + # BINDING-COLLISION declines (wrong-serves found by adversarial parity testing): + # a node alias equal to the node-id binding would overwrite the id column where + # the full path raises; an edge alias equal to the hop's FROM-side binding made + # the two lanes return different node sets. TO-side collisions keep parity and + # remain served (see test_fast_path_alias_shadowing_column_matches_full_path). + ("n0_alias_is_node_binding", [n(name='v'), e_forward(hops=1), n()], None, Engine.PANDAS), + ("n2_alias_is_node_binding", [n(), e_forward(hops=1), n(name='v')], None, Engine.PANDAS), + ("edge_alias_is_src_binding_fwd", [n(), e_forward(hops=1, name='s'), n()], None, Engine.PANDAS), + ("edge_alias_is_dst_binding_rev", [n(), e_reverse(hops=1, name='d'), n()], None, Engine.PANDAS), ("non_eager_engine", [n()], None, Engine.DASK), ("two_ops", [n(), e_forward(hops=1)], None, Engine.PANDAS), ] From e7152a3a5276b262f2731b6eb2d92e626471c68f Mon Sep 17 00:00:00 2001 From: Leo Meyerovich Date: Sun, 2 Aug 2026 09:08:28 -0700 Subject: [PATCH 4/4] test(gfql): assert dtype preservation against the input, not a hardcoded ns unit The new named-fast-path dtype test hardcoded 'datetime64[ns]', but pandas 3.x (pandas==3.0.5 in the failing CI lanes) infers datetime64[us] from pd.to_datetime on date strings, so the INPUT itself is [us] and both the served lane and the policy-forced full path preserve it exactly. Assert what the test actually means: the served lane's dtype equals the input's and the full path's (and is a datetime64), pandas-version-agnostic. Verified green under both pandas 3.0.5 and pandas 2.2.3. Fixes red test-pandas-compat-gfql (latest, py3.14), test-core-python (3.13), and test-gfql-core (3.14) on the merge result with master (#1836/#1837). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr --- graphistry/tests/compute/test_chain.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/graphistry/tests/compute/test_chain.py b/graphistry/tests/compute/test_chain.py index 766e0d8ae3..033df0acdd 100644 --- a/graphistry/tests/compute/test_chain.py +++ b/graphistry/tests/compute/test_chain.py @@ -987,8 +987,13 @@ def test_fast_path_named_datetime_categorical_columns_ride_along(): full = g.gfql(ops, policy=_FAST_NOOP_POLICY) _assert_full_frame_value_parity(fast._nodes, full._nodes, ['v']) _assert_full_frame_value_parity(fast._edges, full._edges, ['s', 'd']) - assert str(fast._nodes['ts'].dtype) == 'datetime64[ns]' + # dtype preservation is asserted against the INPUT (and the full path), not a + # hardcoded unit: pandas 2.x infers datetime64[ns] here, pandas 3.x datetime64[us] + assert fast._nodes['ts'].dtype == nodes['ts'].dtype + assert fast._nodes['ts'].dtype == full._nodes['ts'].dtype + assert str(fast._nodes['ts'].dtype).startswith('datetime64[') assert str(fast._nodes['cat'].dtype) == 'category' + assert fast._nodes['cat'].dtype == full._nodes['cat'].dtype # categorical seed filter: still served, same answer ops2 = [n({'cat': 'a'}, name='x'), e_forward(hops=1), n(name='y')] assert _try_chain_fast_path(g, ops2, Engine.PANDAS, None) is not None