Skip to content

fix(gfql): row-values series carry the table index so NOT IN keeps its rows under the index path - #2121

Merged
lmeyerov merged 10 commits into
masterfrom
fix/gfql-row-mask-index-alignment
Oct 4, 2026
Merged

lmeyerov merged 10 commits into
masterfrom
fix/gfql-row-mask-index-alignment

Conversation

@lmeyerov

@lmeyerov lmeyerov commented Oct 3, 2026 •

Copy link
Copy Markdown
Contributor

Defect

MATCH (a)-[e]->(b) WHERE NOT (b.id IN [...]) RETURN b.id returned 96 rows under index_policy='force' where off/use and plain pandas return 111 (30 nodes / 120 edges, seed 3001, seeds [3, 5, 10]); on 20k nodes / 100k edges, 880 rows (174 whole destination ids) vanished. No extra rows, deterministic, NOT-only, destination-alias-only, index-path-only. Positive IN, the equivalent <> conjunction, and NOT IN on the source alias were all correct.

Cause

The row evaluator mixes two index conventions. Six producers build their result positionally (table_df.reset_index(drop=True).assign(...)[col] → fresh RangeIndex), while most consumers align by label (.loc[mask], assign, and the .where chain inside the tri-valued NOT builder, which broadcasts on the table's own labels). They agree only when the frame has a RangeIndex. The scan hop happens to produce one; the index path's hop returns the nodes frame via take_rows / select_by_ids, which preserve the original row labels — with gaps. Inside NOT, out.where(~true_mask, …) then aligned a 0..27 mask against labels like [0,1,2,3,4,6,…,29]: labels 28/29 had no mask row (→ NaN → dropped) and every label past the gap took its neighbour's value — exactly the dropped ids {9, 28, 29}.

#2020 (c3312ea) fixed this class for the positive-IN pushdown by applying masks by position; NOT goes through a different builder and was never reached.

Fix

One helper, _gfql_on_table_index, gives a positionally built series the table's own index; the shared _gfql_series_from_row_values returns through it (instead of re-resetting to a RangeIndex), and the five inline producers (IN, temporal IN, both list-comparison returns, mask-fill) route through it. Label-aligned and positional consumers now agree on any frame. No .where site changes.

Proof

  • Tiny repro and 20k graph: off/use/force all equal plain-pandas truth (111; 99,754).
  • Reinstate-the-bug: the two detector pins fail against master's code and pass with the fix; the non-RangeIndex Cypher pin is a both-sides control.
  • gfql row + cypher + index suites: 4,889 passed, 420 skipped, 17 xfailed (cuDF deselected: the one cuDF failure is libnvrtc.so.12 missing on this box and reproduces identically on master).
  • The earlier null-handling table is unchanged by this fix (the pandas NaN-as-null IN row and the polars int/float category gate remain separate defects, tracked next).
  • mypy clean; ruff + type-hygiene + comment-encoding guards green (the helper's Any params carry the file's hygiene-ok marker).

Found while verifying the indexing.rst examples with gfql_explain under every index_policy.

Also in this PR: #2116 item 3b — a WHERE that excludes every row raised instead of returning no rows

The colleague's exact text ("AST evaluator unsupported") fires when NOT (alias.col IN [...]) excludes every surviving row: the alias prefilter leaves an empty frame, where_rows evaluates the predicate on it, and IN builds its tri-valued mask from an empty Python list, which pandas types float64 (cuDF: object) and the NOT gate declines. Same root, three pre-existing symptoms on master: pandas NOT-IN-all raised; cuDF alias.col IN [...] matching no row raised "cudf does not support mixed types" (#2125, retitled); pandas [t.id] < ['a'] with no survivor raised "Lengths must match to compare". Fix at the cause: where_rows validates the predicate (_gfql_parse_row_expr) then short-circuits on an empty frame; _gfql_tri_valued_series types the four tri-valued producers boolean even when empty. Pins in test_where_rows_empty_frame_3b.py: 7 shapes × pandas/cuDF → [], controls, bad-predicate-still-rejected, two unit pins; 19 pass here, 9 fail on master. Full compute-lane suites 4907 passed; changed-line coverage 89.1%.

🤖 Generated with Claude Code

https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp

lmeyerov and others added 4 commits October 2, 2026 23:15
…s rows

NOT (b.id IN [...]) on the destination alias returned 96 rows under
index_policy='force' where the scan returns 111; on 20k nodes 880 rows and 174
whole destination ids vanished. Positive IN, the equivalent <> conjunction and
NOT IN on the source alias were correct.

The row evaluator mixed two index conventions: six producers built results
positionally (reset_index(drop=True).assign -> RangeIndex) while most consumers
align by label, including the .where chain inside the tri-valued NOT builder,
which broadcasts on the table's labels. They agree only on a RangeIndex frame.
The index path's hop returns nodes via take_rows / select_by_ids, which keep the
original labels with gaps, so inside NOT a 0..27 mask aligned against labels
[0,1,2,3,4,6..29]: 28 and 29 had no mask row and every label past the gap took
its neighbour's value -- the dropped ids were exactly {9, 28, 29}.

A single helper gives a positionally built series the table's own index; the
shared row-values helper returns through it instead of re-resetting, and the
five inline producers route through it. #2020 fixed the positive-IN pushdown by
applying by position; NOT used a different builder and was never reached.

Pinned: force/use/off parity on the 30-node repro, a non-RangeIndex frame
control, and a unit pin on the helper's index. Detector pins fail on master.
gfql row+cypher+index suites: 4889 passed with cuDF deselected (libnvrtc gap
on this box, identical on master).

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
Temporal comparison, temporal IN and list comparison build a reset work frame and
label-assign evaluator series into it; on the gappy frames the index path returns
the rows shift past the gap. Add _gfql_assign_positional and pin the families
under off/use/force.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
… raising (3b)

NOT (x IN [...]) covering every row raised 'AST evaluator unsupported' on pandas, IN
matching nothing raised 'cudf does not support mixed types', list compares broadcast 0
against 1: the evaluators type masks from the values they see and an empty frame has
none. where_rows validates then short-circuits on an empty frame; tri-valued results
are boolean even when empty.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
@lmeyerov

lmeyerov commented Oct 3, 2026

Copy link
Copy Markdown
Contributor Author

Real-GPU receipt (dgx-spark, graphistry/test-rapids-official:26.02-gfql-polars, cudf 26.02.01 / cupy 13.6.0 / polars 1.35.2) at 642664d: 37 passed (both pin files incl. the cuDF twins + the #2020 alignment pins); CHANGELOG-only commits since (e427960) change no code.

@lmeyerov

lmeyerov commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Read-only review (parallel session) — findings on head 215a297 vs master 88b99cc

Nothing here touches the branch; everything below is reproducible from the listed inputs. Mode: findings, fixes deferred. A patch of the missing tests is attached at the end.

Target: fix/gfql-row-mask-index-alignment @ 215a297 vs master 88b99cc. Mode findings, fixes deferred. Reviewed in a detached worktree; master in a second worktree for before/after. Pandas run; cuDF by reading installed cudf 25.10.00; polars run directly.

What checks out

  • Root cause is real and correctly diagnosed. Reproduced master: 30/120 seed-3001 graph, NOT (b.id IN [3,5,10]) -> off/use 111, force 96; at 100k/500k with 50 seeds master force returns 496,182 of 499,753 rows (3,571 dropped). PR head returns truth under all three policies.
  • PR's tests: 21 pass on head (pandas); against master 11 fail, and the ones that fail are the behavioural detectors (NOT IN force parity, both temporal shapes, three empty-frame shapes). The list =/<>/< and non-RangeIndex pins pass on master too (both-sides controls, as the PR body says).
  • Routes-off replay (GFQL_ROUTES_OFF=index-hop,indexed-kernel) -> 21 pass; no route_engaged marker needed (force degrades to the general path).
  • ./bin/ruff.sh clean; ./bin/typecheck.sh graphistry/compute/gfql/row/pipeline.py clean. _gfql_eval_string_expr -> _gfql_parse_row_expr refactor is behaviour-preserving.
  • Polars: separate engine, no row index; NOT IN -> 111 and all-excluded -> 0 on both trees. Not affected.
  • No new source file -> coverage baseline unchanged (pipeline.py already keyed); no polars in the tests -> no POLARS_TEST_FILES entry. Credentials scan clean.
  • Performance: neutral. set_axis is an O(n) label swap on a fresh series; no sort/reindex/merge. 100k nodes / 500k edges, pandas, median of 5 (time.perf_counter), interleaved master/PR/master/PR: NOT IN b force 427 / 522 / 480 / 418 ms; IN b force 141 / 123 / 147 / 138 ms; _gfql_series_from_row_values on a 500k-row gappy frame 31.9 ms on both trees. Full table in the perf wave notes.
  • CI at head: everything green except RTD; gh pr checks showed test-gfql-core (3.12) pending but gh run view 37185154047 reports it completed successfully.

IMPORTANT

1. The fix closes 6 producers; 10 same-class producers still return a RangeIndex, so NOT/OR still misalign under force

graphistry/compute/gfql/row/pipeline.py:961 (vectorized list compare), :1068 (list literal), :2027 (any/all/none/single), :2111 (list comprehension), :2974/:2980/:2996 (range), :3214/:3284 (subscript), :3402 (slice), :3446/:3505 (list concat) all still end in reset_index(drop=True) / pd.Series(out_values). The NOT builder (_gfql_series_from_truth_masks :2807-2809) and the AND/OR builder (:2850-2861) align by label, so the exact mechanism described in the PR body persists for every one of these.

Failing inputs (same gappy graph as the PR's own pin, PR head, engine="pandas"):

NOT ([b.id] = [9])                        truth 116  off 116  force 103
NOT ([b.id, 1][0] = 9)                    truth 116  off 116  force 103
NOT any(x IN [b.id] WHERE x = 9)          truth 116  off 116  force 103
NOT (size(range(0, b.id)) = 10)           truth 116  off 116  force 103
NOT ([x IN [b.id] | x][0] = 9)            truth 116  off 116  force 103
NOT (([b.id] + [1])[0] = 9)               truth 116  off 116  force 103
[b.id, 2][0..1] = [9]                     truth   4  off   4  force   0   (slice misaligns on its own)
[b.id] = [9] OR b.id = 3                  truth   9  off   9  force   5

Direct evaluator probe on a table with index [0,1,3,4], values [9,3,9,1]: [b.id] = [9] returns index [0,1,2,3]; NOT ([b.id] = [9]) returns [False, True, True, <NA>] (expected [False, True, False, True]). b.id IN [9] / NOT (b.id IN [9]) correctly return index [0,1,3,4].

These are pre-existing on master (identical numbers), so not a regression — but the PR body ("Label-aligned and positional consumers now agree on any frame") and the CHANGELOG ("Masks built inside the evaluator are now aligned to the frame the index path returns") claim the invariant, and it does not hold. Either route the ten returns through _gfql_on_table_index (mechanical), or — better — put one chokepoint in _gfql_eval_expr_ast's return (or align operands positionally inside the truth-mask/boolean builders) so the class is closed for current and future producers; or narrow the claim to IN / temporal IN / list-compare fallback / mask-fill.

2. Both new helpers are identity functions on cuDF

cudf.Series has no set_axis (cudf 25.10.00: python -c "import cudf; print(hasattr(cudf.Series,'set_axis'))" -> False), so _gfql_on_table_index (:2762-2772) returns its input unchanged and _gfql_assign_positional (:2749-2759) falls through to plain assign. On cuDF this PR changes nothing about alignment.

Does the bug reach cuDF? By reading: the index path's cuDF branches preserve labels (index/engine_arrays.py:128 df.iloc[positions], :153 df[df[col].isin(cudf.Series(ids))]); cuDF Series.where is positional (as_column(cond) + length check), so a bare NOT IN is probably fine on cuDF; but cuDF DataFrame.__setitem__ does Series(value)._align_to_index(...) and Series binops align by label, so the temporal work-frame insert at :772 and AND/OR combinations (NOT (b.id IN [3,5]) AND b.id > 2) remain exposed on cuDF. I could not run cuDF kernels here; the PR body says cuDF suites were deselected locally too, so the cuDF side is currently unverified in both directions.

Fix: an engine-neutral relabel — out.index = table_df.index on the freshly built series (cuDF has the index setter; the object is fresh so no aliasing), or construct via type(out)(..., index=table_df.index) — and a dgx run of the detector pins (cd docker && ./test-gpu-local.sh graphistry/tests/compute/gfql/row/test_negated_membership_index_alignment.py). CHANGELOG currently says "pinned ... on pandas and cuDF".

3. where_rows empty-frame short-circuit drops strict-mode schema errors

pipeline.py:4993-4995: _gfql_parse_row_expr validates syntax/capabilities, not property existence. Failing input, strict="strict":

MATCH (a)-[e]->(t) WHERE t.type IN ['robot'] AND t.nosuch = 1 RETURN t.id AS id
master: GFQLSchemaError [column-not-found] Property "t.nosuch" does not exist in row table
PR:     0 rows

Same query with ['company'] (one survivor) still raises on the PR, so whether a typo in a property name is reported now depends on whether the alias prefilter happened to empty the frame. Evaluating t.nosuch = 1 on the empty frame raises the schema error correctly, so resolving property refs on the empty path is cheap; note the short-circuit is still needed for [t.id] < ['a'] on an empty frame (raises "Lengths must match" at :872 without it).

4. Broad except Exception in two new local helpers

pipeline.py:2755-2758 and :2768-2771. The only guarded call is set_axis, which can only raise ValueError on a length mismatch that is already pre-checked — the catch is dead on pandas and only hides the cuDF gap in (2). Policy (review SKILL.md): narrowed, documented exception types at local helpers. Remove, or narrow to (ValueError, TypeError).

5. No cuDF detector for the headline bug

The only cuDF-parametrized membership test, test_not_in_on_a_non_range_indexed_node_frame_matches_the_scalar_form[cudf] (test file L36-45), is a shape whose pandas twin passes on master — a control, not a detector — so it has no power on either engine. The real detectors (L48-59, L85-92) are pandas-only. SKILL.md requires paired cuDF coverage for compute/gfql/row/ changes; see the attached patch for an engine-parametrized detector (importorskip) and a unit pin that documents the set_axis gap.

6. RTD is red on the head and would go red on master with a merge commit

Not a code defect. docs/source/_ext/gfql_bench_data.py enforces max_compute_commit_drift=12 via git rev-list --count <measured>..HEAD -- graphistry/compute against the graphbench receipts (2d64913b5). Master is at 10; the PR head is at 15 (4 branch commits touching compute + the merge). RTD build 34925744 fails with 38 "measured 15 graphistry/compute commits ago" entries. Branch protection has no required status checks, so this does not block the merge button — but a merge commit lands master at 14-15 (docs red); a squash lands 11 (passes, with 1 of headroom). Squash-merge, or re-measure/republish the q1-q9 receipts in pyg-bench first.

SUGGESTION

  • Tests missing for cases the fix does change (master wrong -> PR right), all under force: NOT (b.id IN []) (master 109/120), null rows + NOT IN 3VL (master 5/40), string ids (master 96/111), duplicate-label nodes frame (master 93/111), size(range(0, b.id)) (master raised "Cannot convert non-finite"). Written as functions in the attached patch, plus xfail(strict=True) pins for the residual family in (1), a strict-mode pin for (3), and a values assertion for _gfql_assign_positional's length-mismatch branch (test L101-103 currently asserts only .shape[0]).
  • test_not_in_on_a_non_range_indexed_node_frame_matches_the_scalar_form reads as a detector but passes on master; say so in its docstring (or drop it) so it is not mistaken for coverage of the fix.
  • DRY: _gfql_tri_valued_series (:2741-2747) re-implements _gfql_series_from_row_values's happy path; call it and add the empty-bool typing.
  • CHANGELOG: narrow the two claims per (1)/(2); add the blank line before ### Fixed.

Merge recommendation

Not yet. The diagnosis is right, the pandas IN/NOT IN/temporal/list-fallback shapes are genuinely fixed with no perf cost, and CI is green apart from a docs-drift gate — but the PR's own invariant ("positional and label-aligned consumers agree on any frame") is contradicted by eleven concrete inputs that still return wrong rows under force, the fix is a provable no-op on cuDF while the changelog says cuDF is pinned, and the empty-frame short-circuit silently drops strict schema errors (a real regression against master). Suggested path: add an engine-neutral relabel (.index = table_df.index) at a single chokepoint, resolve property refs on the empty path, add the detector tests from the patch (including a cuDF run on dgx), narrow the changelog, and squash-merge (or republish receipts) so RTD stays green. With those, this is a clean approve.

proposed-tests.patch (not applied; detector + boundary tests described above)
# Proposed additional tests for PR #2121 (review artifact; apply nothing from here automatically).
# Target file: graphistry/tests/compute/gfql/row/test_negated_membership_index_alignment.py
# Measured on the PR head (215a29780) and on master (88b99cc73) with the review scratch scripts;
# "master" numbers below are what the pin would have caught.
--- a/graphistry/tests/compute/gfql/row/test_negated_membership_index_alignment.py
+++ b/graphistry/tests/compute/gfql/row/test_negated_membership_index_alignment.py
@@ -112,0 +113,142 @@
+
+
+def _policies(g, q, engine="pandas"):
+    out = {}
+    for policy in ("off", "use", "force"):
+        nodes = g.gfql(q, engine=engine, index_policy=policy)._nodes
+        out[policy] = len(nodes.to_pandas() if hasattr(nodes, "to_pandas") else nodes)
+    return out
+
+
+def test_not_in_empty_list_keeps_every_row_on_the_index_path():
+    # master: force=109 of 120 (NOT [] is TRUE for every row, null rows included)
+    edges, g = _gappy_graph()
+    assert _policies(g, "MATCH (a)-[e]->(b) WHERE NOT (b.id IN []) RETURN b.id AS id") == {p: len(edges) for p in ("off", "use", "force")}
+    assert _policies(g, "MATCH (a)-[e]->(b) WHERE b.id IN [] RETURN b.id AS id") == {p: 0 for p in ("off", "use", "force")}
+
+
+def test_not_in_three_valued_logic_agrees_across_index_policies():
+    # null rows and null list elements: Cypher 3VL; master force=5 of 40 for the first query
+    rng = np.random.default_rng(3001)
+    n_nodes, n_edges = 30, 120
+    edges = pd.DataFrame({"src": rng.integers(0, n_nodes, n_edges), "dst": rng.integers(0, n_nodes, n_edges)})
+    kind = np.where(np.arange(n_nodes) % 3 == 0, None, np.where(np.arange(n_nodes) % 3 == 1, "x", "y"))
+    nodes = pd.DataFrame({"id": np.arange(n_nodes), "type": kind})
+    g = graphistry.edges(edges, "src", "dst").nodes(nodes, "id").gfql_index_all()
+    t = nodes.set_index("id")["type"].reindex(edges["dst"]).reset_index(drop=True)
+    cases = [
+        ("MATCH (a)-[e]->(b) WHERE NOT (b.type IN ['x']) RETURN b.id AS id", int((t.notna() & (t != "x")).sum())),  # null row -> NULL -> dropped
+        ("MATCH (a)-[e]->(b) WHERE b.type IN ['x', null] RETURN b.id AS id", int((t == "x").sum())),               # only hits are TRUE
+        ("MATCH (a)-[e]->(b) WHERE NOT (b.type IN ['x', null]) RETURN b.id AS id", 0),                              # nothing is FALSE
+        ("MATCH (a)-[e]->(b) WHERE NOT (b.type IN []) RETURN b.id AS id", n_edges),                                # empty list: FALSE for all -> NOT TRUE
+    ]
+    for q, truth in cases:
+        assert _policies(g, q) == {p: truth for p in ("off", "use", "force")}, q
+
+
+def test_not_in_string_ids_agree_across_index_policies():
+    # master: force=96 of 111 (same drop pattern as the integer-id pin)
+    rng = np.random.default_rng(3001)
+    n_nodes, n_edges = 30, 120
+    src, dst = rng.integers(0, n_nodes, n_edges), rng.integers(0, n_nodes, n_edges)
+    edges = pd.DataFrame({"src": [f"n{i}" for i in src], "dst": [f"n{i}" for i in dst]})
+    nodes = pd.DataFrame({"id": [f"n{i}" for i in range(n_nodes)]})
+    g = graphistry.edges(edges, "src", "dst").nodes(nodes, "id").gfql_index_all()
+    truth = int((~edges["dst"].isin(["n3", "n5", "n10"])).sum())
+    assert _policies(g, "MATCH (a)-[e]->(b) WHERE NOT (b.id IN ['n3','n5','n10']) RETURN b.id AS id") == {p: truth for p in ("off", "use", "force")}
+
+
+def test_not_in_on_a_duplicate_labelled_node_frame_agrees_across_index_policies():
+    # master: force=93 of 111; duplicate labels defeat label alignment entirely
+    edges, g = _gappy_graph()
+    nodes = g._nodes.copy()
+    nodes.index = np.arange(len(nodes)) // 2
+    g = graphistry.edges(edges, "src", "dst").nodes(nodes, "id").gfql_index_all()
+    truth = int((~edges["dst"].isin([3, 5, 10])).sum())
+    assert _policies(g, "MATCH (a)-[e]->(b) WHERE NOT (b.id IN [3, 5, 10]) RETURN b.id AS id") == {p: truth for p in ("off", "use", "force")}
+
+
+def test_range_result_inserts_positionally_into_the_reset_work_frame():
+    # master: force raised "Cannot convert non-finite values" (range() start/step series carried gappy labels)
+    edges, g = _gappy_graph()
+    truth = int((edges["dst"] == 9).sum())
+    assert _policies(g, "MATCH (a)-[e]->(b) WHERE size(range(0, b.id)) = 10 RETURN b.id AS id") == {p: truth for p in ("off", "use", "force")}
+
+
+@pytest.mark.xfail(strict=True, reason="#2121 residual: these producers still return a RangeIndex (pipeline.py:961/1068/2027/2111/2974/3284/3402/3446); NOT/OR builders align by label")
+@pytest.mark.parametrize("label,query,truth_mask", [
+    ("NOT list =", "MATCH (a)-[e]->(b) WHERE NOT ([b.id] = [9]) RETURN b.id AS id", lambda d: d != 9),
+    ("NOT list-literal subscript", "MATCH (a)-[e]->(b) WHERE NOT ([b.id, 1][0] = 9) RETURN b.id AS id", lambda d: d != 9),
+    ("NOT any()", "MATCH (a)-[e]->(b) WHERE NOT any(x IN [b.id] WHERE x = 9) RETURN b.id AS id", lambda d: d != 9),
+    ("NOT range()", "MATCH (a)-[e]->(b) WHERE NOT (size(range(0, b.id)) = 10) RETURN b.id AS id", lambda d: d != 9),
+    ("NOT list comprehension", "MATCH (a)-[e]->(b) WHERE NOT ([x IN [b.id] | x][0] = 9) RETURN b.id AS id", lambda d: d != 9),
+    ("NOT list concat", "MATCH (a)-[e]->(b) WHERE NOT (([b.id] + [1])[0] = 9) RETURN b.id AS id", lambda d: d != 9),
+    ("list slice", "MATCH (a)-[e]->(b) WHERE [b.id, 2][0..1] = [9] RETURN b.id AS id", lambda d: d == 9),
+    ("list = OR scalar", "MATCH (a)-[e]->(b) WHERE [b.id] = [9] OR b.id = 3 RETURN b.id AS id", lambda d: (d == 9) | (d == 3)),
+])
+def test_remaining_evaluator_families_agree_across_index_policies(label, query, truth_mask):
+    # Each is correct under 'off'/'use' and wrong under 'force' on the PR head (103/116, 93/116, 0/4, 5/9).
+    edges, g = _gappy_graph()
+    truth = int(truth_mask(edges["dst"]).sum())
+    assert _policies(g, query) == {p: truth for p in ("off", "use", "force")}, label
+
+
+@pytest.mark.parametrize("engine", ["cudf"])
+def test_index_path_parity_detector_runs_on_cudf(engine):
+    # The existing cudf-parametrized test is a both-sides control (passes on master for pandas).
+    # This is the actual detector on cudf: gappy labels from take_rows/select_by_ids + label-aligned assign/binops.
+    cudf = pytest.importorskip("cudf")
+    rng = np.random.default_rng(3001)
+    n_nodes, n_edges = 30, 120
+    edges = pd.DataFrame({"src": rng.integers(0, n_nodes, n_edges), "dst": rng.integers(0, n_nodes, n_edges)})
+    nodes = pd.DataFrame({"id": np.arange(n_nodes),
+                          "ts": pd.to_datetime("2024-01-01") + pd.to_timedelta(np.arange(n_nodes), unit="D")})
+    g = graphistry.edges(cudf.from_pandas(edges), "src", "dst").nodes(cudf.from_pandas(nodes), "id").gfql_index_all()
+    d = edges["dst"]
+    for q, truth in [
+        ("MATCH (a)-[e]->(b) WHERE NOT (b.id IN [3, 5, 10]) RETURN b.id AS id", int((~d.isin([3, 5, 10])).sum())),
+        ("MATCH (a)-[e]->(b) WHERE NOT (b.id IN [3, 5]) AND b.id > 2 RETURN b.id AS id", int((~d.isin([3, 5]) & (d > 2)).sum())),
+        ("MATCH (a)-[e]->(b) WHERE b.ts IN [datetime('2024-01-10T00:00:00')] RETURN b.id AS id", int((d == 9).sum())),
+        ("MATCH (a)-[e]->(b) WHERE NOT (b.ts IN [datetime('2024-01-10T00:00:00')]) RETURN b.id AS id", int((d != 9).sum())),
+    ]:
+        assert _policies(g, q, engine="cudf") == {p: truth for p in ("off", "use", "force")}, q
+
+
+def test_series_from_row_values_is_a_no_op_relabel_on_cudf_today():
+    # Documents the engine gap: cudf.Series has no set_axis, so _gfql_on_table_index returns its input.
+    cudf = pytest.importorskip("cudf")
+    table = cudf.DataFrame({"x": [1, 2, 3]}, index=[0, 2, 7])
+    out = RowPipelineMixin._gfql_series_from_row_values(RowPipelineMixin(), table, [True, False, None], "__t__")
+    assert out.index.to_pandas().tolist() == [0, 2, 7]  # fails today: [0, 1, 2]
+
+
+# Target file: graphistry/tests/compute/gfql/row/test_where_rows_empty_frame_3b.py
+
+def test_strict_absent_property_still_raises_when_no_row_survives():
+    # master: GFQLSchemaError [column-not-found]; PR head: returns 0 rows (the short-circuit validates syntax only)
+    from graphistry.compute.exceptions import GFQLSchemaError
+    with pytest.raises(GFQLSchemaError):
+        _graph().gfql("MATCH (a)-[e]->(t) WHERE t.type IN ['robot'] AND t.nosuch = 1 RETURN t.id AS id", engine="pandas", strict="strict")
+    # control: with a survivor the error is raised on both trees
+    with pytest.raises(GFQLSchemaError):
+        _graph().gfql("MATCH (a)-[e]->(t) WHERE t.type IN ['company'] AND t.nosuch = 1 RETURN t.id AS id", engine="pandas", strict="strict")
+
+
+def test_positional_assign_length_mismatch_keeps_pandas_alignment_values():
+    # tightens the existing shape-only assertion (L101-103): values, not just row count
+    frame = pd.DataFrame({"x": range(5)})
+    short = pd.Series([1, 2], index=[0, 1])
+    out = RowPipelineMixin._gfql_assign_positional(frame, s=short)
+    assert out["s"].tolist()[:2] == [1, 2] and out["s"].isna().tolist()[2:] == [True, True, True]

🤖 Generated with Claude Code

https://claude.ai/code/session_017ropeBMLJUuy6ViYwy15ud

…pty path keeps strict reporting

Review found three gaps in the first cut. The empty-frame short-circuit skipped the absent-property
report, so a typo in a property name went unreported under strict whenever the prefilter happened to
empty the frame; the predicate is now evaluated on the empty frame like any other and a comparison
with a zero-row operand answers a zero-row mask. The alignment helpers relied on set_axis, which
cuDF does not have, so the fix was a no-op there; they now set the index, which both engines support,
and the pins run on cuDF. And the invariant held for six producers only: aligning at the evaluator's
return closes it for every producer, so quantifiers, list comprehensions, range, subscript, slice and
concatenation under NOT and OR now agree across index policies, each pinned.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
@lmeyerov

lmeyerov commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks — this landed on target. All six IMPORTANT items are addressed at 629aa47; replying by number.

1. Ten same-class producers still misalign. Closed as a class, not narrowed. _gfql_eval_expr_ast is now a chokepoint: it calls _gfql_eval_expr_ast_values (the former body, unchanged) and puts every series result on the table's index on the way out, so each producer agrees with the label-aligned NOT/AND/OR builders without touching ten return sites. Your eight inputs, on your gappy graph, off vs force after the change:

NOT ([b.id] = [9])                 116 / 116      (was 116 / 103)
NOT ([b.id, 1][0] = 9)             116 / 116
NOT any(x IN [b.id] WHERE x = 9)   116 / 116
NOT (size(range(0, b.id)) = 10)    116 / 116
NOT ([x IN [b.id] | x][0] = 9)     116 / 116
NOT (([b.id] + [1])[0] = 9)        116 / 116
[b.id, 2][0..1] = [9]                4 / 4        (was 4 / 0)
[b.id] = [9] OR b.id = 3             9 / 9        (was 9 / 5)

Pinned as test_every_producer_agrees_across_index_policies (seven shapes, off/use/force) plus a slice pin. The CHANGELOG sentence now claims the closed class rather than the six shapes.

2. Both helpers are identity functions on cuDF. Confirmed and fixed the way you suggested: _gfql_on_table_index sets .index on a shallow copy, which both engines support, and _gfql_assign_positional routes through it. set_axis is gone, so is the hasattr dispatch. A unit pin asserts the index on a gappy frame is carried on pandas and cuDF, and test_the_evaluator_returns_series_on_the_table_index checks the chokepoint itself on both engines. GPU receipt at the new head to follow.

3. Empty-frame short-circuit drops strict schema errors. Confirmed — this was the real regression. The short-circuit is removed. The reason it existed was [t.id] < ['a'] broadcasting 0 against 1, so instead _gfql_eval_comparison_op answers a zero-row boolean mask when either operand is a zero-row series. The predicate now runs on the empty frame like any other, so t.nosuch raises again under strict, and test_absent_property_on_an_emptied_frame_is_still_reported_under_strict pins both the raise and the lenient zero rows.

4. Broad except Exception in the new helpers. Gone with the rewrite; neither helper catches anything now.

5. No cuDF detector. Added, see 2. The pre-existing control keeps its place with a docstring saying it passes on master too.

6. RTD docs-drift gate. Confirmed and quantified: the gate is git rev-list --count 2d64913b5..HEAD -- graphistry/compute against max_compute_commit_drift=12; master is at 10 and this branch at 15. Across the queue the compute-touching counts are #2121 5, #2127 6, #2128 1, #2131 3, so master lands at about 25 whatever the merge style — squashing does not save it. That makes it a re-measure of the graphbench receipts in pyg-bench, or a budget change, and it is on the owner's decision list rather than something I will quietly raise.

On the suggestions: the five master-wrong/PR-right cases are worth having and I will take the patch's shapes as a follow-up rather than hand-copying them now; DRY between _gfql_tri_valued_series and _gfql_series_from_row_values is unchanged this round; the blank line before ### Fixed is in.

Gates at this head: bin/lint.sh green, mypy green, row+cypher+index 5147 passed, and the full graphistry/tests/compute run has 26 failures that are exactly master's own 26 on the identical subset (cuDF libnvrtc plus umap, both environment).

The GPU receipt found the last empty-frame hole: `[t.type] = ['robot']` on an emptied cuDF frame
reached the mask coercion as an empty object column, and cuDF rejects where(object, False). An
empty mask has no null to fill, so it is cast directly. Master raises on the same input, so this
closes the shape rather than restoring anything.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_012Me1E7ZdDuGqJGu3mMEzhp
@lmeyerov

lmeyerov commented Oct 4, 2026

Copy link
Copy Markdown
Contributor Author

GPU receipt at the current head, and it found one more case that CI cannot see.

dgx-spark, GB10, graphistry/test-rapids-official:26.02-gfql-polars, head 01601b7:

graphistry/tests/compute/gfql/row/test_negated_membership_index_alignment.py
graphistry/tests/compute/gfql/row/test_where_rows_empty_frame_3b.py
graphistry/tests/compute/gfql/row/test_alias_prefilter_alignment_2020.py
  -> 50 passed, 0 failed   (EXIT=0)

The first GPU pass at 629aa47 failed one cell: [t.type] = ['robot'] on an emptied cuDF frame raised cudf does not support mixed types, from the mask coercion doing where(~isna, False) on an empty object column. Master raises on the same input, which I checked directly on a master-tip tree on the same GPU, so it was not a regression, but it is exactly the shape item 3b claims to close. Fixed at 01601b7 by typing an empty mask as boolean before any null fill, which is one guard at the coercion point rather than at the call sites.

Worth noting for the docs lane: this PR's docs/readthedocs status is red and no test is involved. The published benchmark receipts allow 12 graphistry/compute commits of drift and this head is at 17. I am fixing that with a measured waiver in pyg-bench rather than raising the cap, and the queue's four remaining PRs land together once the docs lane is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant