Skip to content

fix(gfql): decline three silently-wrong bounded var-length polars shapes (#1787) - #1794

Merged
lmeyerov merged 2 commits into
masterfrom
fix/gfql-1787-varlen-polars-declines
Jul 28, 2026
Merged

fix(gfql): decline three silently-wrong bounded var-length polars shapes (#1787)#1794
lmeyerov merged 2 commits into
masterfrom
fix/gfql-1787-varlen-polars-declines

Conversation

@lmeyerov

@lmeyerov lmeyerov commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Fixes #1787. Independent of #1793 (touches only the polars var-length gate); either can land first.

Pandas is the oracle and the contract is parity-or-NotImplementedError. Three bounded var-length shapes in the native polars rows(binding_ops=...) path returned a different count with no error. Same root-cause family as the unbounded case #1781 declined: pandas' step_pairs come from the var-length edge_op.execute hop, whose hop-window pruning -- and, when seeded, its per-seed BFS -- changes the edge multiplicity in a way a rebuild from the raw matching edge table does not reproduce.

Declining is a deliberate divergence from master, which served these: a silent wrong answer becomes a loud, actionable error, and engine='pandas' still answers all of them.

What is declined

Counts are diverging graphs out of 60 random ones per shape, differential fuzz against the pandas oracle (all measured on master c76f23a3):

shape diverged example
directed -[*k..m]->, min_hops >= 3 53/60 pandas 0, polars 35
directed -[*k..m]->, min_hops >= 2 off a filtered seed 27-30/60 pandas 3, polars 4
undirected -[*1..1]- / -[*1]- (degenerate window) 60/60 pandas 36, polars 18 -- halved
undirected -[*1..k]- not starting from the full node set (filtered seed, or non-first segment) 51/60, 36/60 pandas 38, polars 122

The middle two rows are the same clause; the filtered-seed directed case is not in the issue -- the fuzz turned it up while establishing where the min_hops boundary actually is.

What is still served

-[*1..2]->, -[*2..2]->, -[*2..3]-> (including as a non-first segment -- the graph-bench q3 shape), -[*1..k]-> off a filtered seed, undirected -[*1..2]- / -[*1..3]- off the full node set, the DIRECTED degenerate window -[*1..1]->, and plain -[]-. All fuzz-clean at 0/60 and asserted in the tests, so an over-broad gate fails.

Why the gate keys on an explicit var-length window

Not on EdgeSemantics.is_multihop: -[*1..1]- resolves to min == max == 1 and is therefore not multihop, yet pandas still routes it through the var-length hop -- on the same graph -[]- gives 18 where -[*1..1]- gives 36. That is exactly why the existing tests missed this one. Plain -[]- (no min_hops/max_hops in the AST) is untouched.

Tests: engine-parametrized, and the prose moved into them

graphistry/tests/compute/gfql/test_varlen_bounded_engine_parity_1787.py (renamed from test_engine_polars_varlen_divergence_1787.py), 81 cases over pandas / polars / cuDF / polars-gpu.

Two review points drove the rewrite.

1. Comments that should have been tests. row_pipeline.py carried per-shape prose asserting which shapes diverge and by how much. A comment asserting behaviour is unverified and rots. Every numeric claim is now a named test, and the comment keeps only what a test cannot state -- the WHY of the deliberate divergence from master's serving decision, plus a pointer to the test file. The gate's comment block went from 48 lines to 19.

former comment claim now
-[]- gives 18 where -[*1..1]- gives 36, so the gate cannot key on is_multihop test_degenerate_window_is_not_the_same_query_as_a_plain_edge
max_reached_hop is a BFS eccentricity, so the oracle returns 0 at min_hops >= 3 test_directed_min_hops_3_collapses_to_empty_on_the_oracle
min_hops == 2 is clean unseeded and diverges only under a seed test_directed_min_hops_2_declines_only_when_the_segment_is_seeded
the over-count is specific to the undirected doubling; every directed twin agrees test_undirected_seed_declines_while_its_directed_twin_is_served
a non-first segment starts from less than the full node set, like a seed test_undirected_non_first_segment_declines_while_its_directed_twin_is_served
the max == 1 decline is undirected-only test_degenerate_window_decline_is_undirected_only

2. Engine-parametrized, not polars-only. "Which shapes are answerable, and with what value" is engine-agnostic semantics. The file now encodes the intended per-engine behaviour rather than assuming identity: the polars engines must DECLINE exactly the shapes the pandas-API engines must ANSWER, and every neighbouring shape must return the same pinned oracle count on all four. Oracle counts are pinned as literals, so a regression that makes every engine equally wrong still fails.

It paid for itself immediately -- see the cuDF defect below.

A cuDF defect this cross-engine parametrization found: #1798

MATCH (a {prop: v})-[*1..1]-(b) -- seeded, undirected, degenerate window -- returns cuDF 1 vs pandas 9, silently. 23 of 40 random graphs diverge on that shape; drop any one of {seeded, undirected, max == 1} and cuDF agrees (0/40 for each of the unseeded, directed, no-window and wider-window variants). It is a separate defect from #1787 and is not fixed here -- the polars engines decline this shape, so the polars fix cannot mask it. Filed as #1798 and pinned by test_seeded_undirected_degenerate_window_agrees_with_the_oracle with xfail(strict=True), so the fix cannot land unnoticed.

CI: a real defect this PR tipped over, filed as #1797patch attached, NOT pushed

Action needed from a maintainer. The fix edits .github/workflows/ci.yml, and the token available to me lacks the workflow OAuth scope (refusing to allow an OAuth App to create or update workflow .github/workflows/ci.yml). The patch is posted as a comment on this PR and is ready to git apply.

This PR is now fully green (77/77), including test-polars (3.12) — but that green is luck, not a fix, and should not be read as closing #1797. Job durations from the API, same job, workload identical to within +1.1s (measured):

run test-polars (3.12) outcome
30304440110 attempt 2 615s cancelled (timeout at 600s)
30304440110 attempt 3 611s cancelled (timeout at 600s)
30312253135 (this one) 431s success

The siblings move the same way across those runs (3.11: 249s → 361s), so this is runner variance around a budget the lane sits right up against — the same commit can pass or time out depending on which runner it draws.

test-polars (3.12) timed out twice at timeout-minutes: 10 -- and GitHub reports a timed-out job as cancelled, which is why it initially read as unrelated fail-fast collateral. The log shows 2377 passed, 13 skipped ... in 572.43s (0:09:32) followed by The operation was canceled. -- zero test failures, pure budget exhaustion. py3.12 is the only cell running Polars tests with coverage plus the per-file coverage audit, roughly double the work; siblings finish in 4m09s-5m51s. Appending one file to POLARS_TEST_FILES was enough to tip it, and changed-line-coverage then reported SKIPPED downstream of the missing artifact. A rerun does not clear it (0-for-2).

The attached patch fixes it by splitting, not by bumping the number:

  • new job test-polars-coverage (py3.12, timeout-minutes: 20) carries the coverage pass, the audit and the artifact upload;
  • 3.12 leaves the test-polars matrix, so every remaining cell does identical work and the 10-minute budget is a real hang detector again;
  • no python version loses a run -- py3.12 was already running only the coverage step, so this is the previous step layout re-homed, not a coverage reduction;
  • changed-line-coverage needs: now includes the new job, since it consumes that job's artifact.

Raising the number was the one-line alternative: it would have handed five cells a 2-3x-slack budget to hide a hang in, and deferred the same failure to the next test addition. Verified via the branches API that master has no required_status_checks configuration, so introducing a new check name does not block merges (if required checks are added later, test-polars-coverage belongs in the list).

#1797 records the structural limit that remains rather than letting the bump hide it: the polars lane is serial (bin/test-polars.sh has no -n auto, unlike test-gfql-core) and its allowlist grows monotonically, so the ceiling will be reached again.

Where these tests actually run (re: #1795)

The old file opened with a module-level pytest.importorskip("polars"), so in the pandas-only test-gfql-core lane it was skipped in its entirety. The new file has no module-level import guard:

lane result
test-gfql-core (py3.12, 3.14; whole tests/compute/gfql dir, no polars) 18 passed, 63 visibly skipped with per-engine reasons (verified locally with polars and cudf import-blocked)
test-polars / test-polars-coverage (allowlist updated to the new filename) 58 passed, 22 skipped, 1 xfailed
dgx-spark GPU, all four engines 80 passed, 1 xfailed, 0 skipped

The GPU parameters gate on a runtime probe, so where they cannot run they report SKIPPED with a reason that says so explicitly rather than reading as coverage. The boundary is stated in the module docstring: no CI lane runs cuDF or polars-gpu (ci-gpu.yml is hard-disabled and does not install the polars extra), so a green CI run is evidence for pandas + polars only.

Mutation checks

Both directions, since a gate can be wrong by being too narrow or too broad:

mutation result
whole #1787 gate reverted to pre-fix 233b64c8 13 failed -- every declined shape plus every boundary test
gate widened to decline all bounded var-length 12 failed -- every still-served neighbour plus the fuzz's served-count floor

Verification receipts

  • dgx-spark, graphistry/test-rapids-official:26.02-gfql-polars, docker run --gpus all, GB10 / cuDF 26.02.01 / polars 1.35.2: test_varlen_bounded_engine_parity_1787.py = 80 passed, 1 xfailed, 0 skipped in 48.00s.
  • Local (polars 1.42, cuDF 25.10, RTX 3080 Ti): same file 58 passed, 22 skipped (polars-gpu), 1 xfailed; full bin/test-polars.sh lane green.
  • bin/lint.sh clean.
  • Earlier full-suite receipt, unchanged by this round: graphistry/tests/compute on master c76f23a3 = 9 failed / 7050 passed / 90 skipped / 16 xfailed; on this branch the failure set is identical.

CHANGELOG entry added under ### Fixed.

🤖 Generated with Claude Code

https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB

@lmeyerov
lmeyerov force-pushed the fix/gfql-1787-varlen-polars-declines branch from c26f656 to 370a4c3 Compare July 27, 2026 20:52
@lmeyerov

Copy link
Copy Markdown
Contributor Author

Rebased onto master 233b64c8 (#1793 merged) and fixed the changed-line coverage gate. Force-pushed: c26f6567 -> 370a4c3e.

Root cause of the 76.92%

The three uncovered lines were 1345, 1351, 1360 — the three return None decline statements, i.e. exactly the new behaviour. Nothing else in the block was uncovered: the surrounding condition lines are executed by the pre-existing polars tests on shapes that pass the gate.

They were uncovered because bin/test-polars.sh is an explicit test-file allowlist and the new test file was not in it, and the polars lane is the only one of the three combined lanes with polars installed — so the only lane that can execute row_pipeline.py at all. The tests existed and asserted the right thing; no lane ever ran them.

Fix is one line: register graphistry/tests/compute/gfql/test_engine_polars_varlen_divergence_1787.py in POLARS_TEST_FILES, alongside every other test_engine_polars_*.py. No pragmas, no threshold change, nothing deleted, and no line was written off as unreachable — all 13 are reachable and now executed.

Verified with the real gate script, not by inference

Ran bin/changed_line_coverage.py --min-percent 80 against a coverage file produced by the polars lane alone (a strict subset of CI's three-lane union, so CI can only be better):

coverage data result
polars lane without the registration 10/13 = 76.92%, missing 1345, 1351, 1360, exit 3 — exactly reproduces the CI failure
polars lane with the registration 13/13 = 100.00%, missing none, exit 0

test_engine_polars_varlen_divergence_1787.py itself is in the report's "Skipped Changed Python Files" (tests are excluded from the gate), so row_pipeline.py is the whole denominator.

Everything else re-verified on the rebased tree

  • Mutation check re-run at the new head: dropping the min_hops clause fails 4 tests, the degenerate undirected window 3, the undirected-seed clause 3, the whole gate 8; unmutated 22 pass. Each decline statement is still independently load-bearing.
  • Full graphistry/tests/compute with --gpus all: master 233b64c8 = 9 failed, 7068 passed, 90 skipped, 16 xfailed; this branch = 9 failed, 7090 passed, 90 skipped, 16 xfailed. Identical failure set, +22 = the new tests.
  • mypy: 161 errors on 233b64c8, 161 here — zero added. ruff clean.
  • ./bin/test-polars.sh passes as a whole with the new file in the list (that is how the coverage above was produced).

The per-file floors in coverage_audit.py --profile gfql-polars are floors with a below-only tolerance and the change only raises row_pipeline.py (70 -> 67 missing statements, 91% either way), and it adds no new source file to the resolved target set, so no baseline needs regenerating.

🤖 Generated with Claude Code

https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB

lmeyerov added a commit that referenced this pull request Jul 27, 2026
…rs/cuDF/polars-gpu)

Review follow-ups on #1794.

Comments -> tests: the gate in row_pipeline.py carried per-shape prose asserting which
shapes diverge and by how much. A comment asserting behaviour is unverified and rots.
Every numeric claim is now a named test; the comment keeps only the WHY of the
deliberate divergence from master's serving decision plus a pointer to the tests.
29 -> 18 comment lines, executable logic byte-identical (the only non-comment hunks are
two trailing comments on existing lines).

Engine-parametrized: renamed to test_varlen_bounded_engine_parity_1787.py and
parametrized over pandas / polars / cuDF / polars-gpu. It encodes the INTENDED
per-engine behaviour rather than assuming identity -- the polars engines must decline
exactly the shapes the pandas-API engines must answer -- with oracle counts pinned as
literals so a regression that makes every engine equally wrong still fails. GPU params
gate on a runtime probe and report a reasoned SKIP, never a silent pass; the coverage
boundary (no CI lane runs cuDF or polars-gpu) is stated in the module docstring.
Dropping the module-level polars importorskip also means the pandas params now execute
in test-gfql-core, where the old file was skipped in its entirety (re: #1795).

Found while doing so: cuDF silently returns 1 where pandas returns 9 on seeded
undirected degenerate `(a {p:v})-[*1..1]-(b)` (23/40 random graphs; drop any one of
seeded/undirected/max==1 and it agrees). Separate defect, filed as #1798 and pinned here
with xfail(strict=True).

Mutation-checked both directions: reverting the gate to 233b64c fails 13 tests;
widening it to decline all bounded var-length fails 12.

Verified: dgx-spark GB10 / cuDF 26.02.01 / polars 1.35.2 via
graphistry/test-rapids-official:26.02-gfql-polars with --gpus all -- 80 passed,
1 xfailed, 0 skipped. CI-equivalent local polars lane (cuDF import-blocked) 2395 passed,
54 skipped, 0 failed. bin/lint.sh and bin/typecheck.sh clean.

Note: the accompanying CI fix for the test-polars py3.12 timeout (#1797) touches
.github/workflows/ci.yml and could not be pushed with the available token, which lacks
the `workflow` OAuth scope. The patch is attached to the PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
@lmeyerov

Copy link
Copy Markdown
Contributor Author

The CI fix for #1797 could not be pushed: the token available to the agent lacks the workflow OAuth scope, so any commit touching .github/workflows/ci.yml is rejected. The patch below is the complete change, verified to parse as valid workflow YAML with no dangling needs: references.

Apply with:

git apply <<'PATCH'
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 43a5e955..8891ab68 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -1236,7 +1236,9 @@ jobs:
     # PR hygiene only: combine coverage collected by existing CPU test jobs and
     # gate executable package lines touched by this PR. Historical uncovered
     # lines outside the diff are intentionally ignored.
-    needs: [changes, test-core-python, test-gfql-core, test-polars, generate-lockfiles]
+    # test-polars-coverage, not test-polars: the polars-coverage-py3.12 artifact this job
+    # combines is produced by the coverage job (#1797 split it out of the matrix).
+    needs: [changes, test-core-python, test-gfql-core, test-polars, test-polars-coverage, generate-lockfiles]
     if: ${{ github.event_name == 'pull_request' && ((needs.changes.outputs.python == 'true' && needs.changes.outputs.narrow_python_only != 'true') || needs.changes.outputs.gfql == 'true' || needs.changes.outputs.schema_lane == 'true' || needs.changes.outputs.pandas_compat == 'true' || needs.changes.outputs.core == 'true' || needs.changes.outputs.infra == 'true') && !(needs.changes.outputs.docs_only_latest == 'true') }}
     runs-on: ubuntu-latest
     timeout-minutes: 5
@@ -1572,6 +1574,10 @@ jobs:
         source pygraphistry/bin/activate
         ./bin/test-graphviz.sh
 
+  # Plain polars lane. Every cell does IDENTICAL work, so the timeout below is a real hang
+  # detector rather than slack absorbing one outlier cell. py3.12 is covered by
+  # test-polars-coverage (same tests, under coverage) and is deliberately absent here, so no
+  # version loses its run -- this is exactly the previous step layout, re-homed.
   test-polars:
     needs: [changes, test-minimal-python, test-gfql-core, generate-lockfiles]
     if: ${{ ((needs.changes.outputs.python == 'true' && needs.changes.outputs.narrow_python_only != 'true') || needs.changes.outputs.gfql == 'true' || needs.changes.outputs.pandas_compat == 'true' || needs.changes.outputs.core == 'true' || needs.changes.outputs.infra == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule') && !(needs.changes.outputs.docs_only_latest == 'true' && (github.event_name == 'push' || github.event_name == 'pull_request')) }}
@@ -1580,7 +1586,7 @@ jobs:
 
     strategy:
       matrix:
-        python-version: [3.9, '3.10', 3.11, 3.12, '3.13', '3.14']
+        python-version: [3.9, '3.10', 3.11, '3.13', '3.14']
 
     steps:
 
@@ -1610,20 +1616,56 @@ jobs:
         uv pip install -e . --no-deps
 
     - name: Polars tests
-      if: ${{ matrix.python-version != '3.12' }}
       run: |
         source pygraphistry/bin/activate
         ./bin/test-polars.sh
 
+  # Split out of the test-polars matrix (#1797): coverage instrumentation plus the per-file
+  # audit make this pass ~2x the plain lane, and while it shared the matrix job's
+  # `timeout-minutes` it consumed nearly all of it -- adding ONE test file to
+  # bin/test-polars.sh tipped it into a TIMEOUT (which GitHub reports as `cancelled`, with
+  # zero test failures in the log). Its own job means its budget is explicit and growth here
+  # cannot cancel the five plain cells.
+  test-polars-coverage:
+    needs: [changes, test-minimal-python, test-gfql-core, generate-lockfiles]
+    if: ${{ ((needs.changes.outputs.python == 'true' && needs.changes.outputs.narrow_python_only != 'true') || needs.changes.outputs.gfql == 'true' || needs.changes.outputs.pandas_compat == 'true' || needs.changes.outputs.core == 'true' || needs.changes.outputs.infra == 'true' || github.event_name == 'workflow_dispatch' || github.event_name == 'schedule') && !(needs.changes.outputs.docs_only_latest == 'true' && (github.event_name == 'push' || github.event_name == 'pull_request')) }}
+    runs-on: ubuntu-latest
+    timeout-minutes: 20
+
+    steps:
+
+    - name: Checkout repo
+      uses: actions/checkout@v4
+      with:
+        lfs: true
+        persist-credentials: false
+
+    - name: Set up Python 3.12
+      uses: actions/setup-python@v5
+      with:
+        python-version: '3.12'
+
+    - name: Download lockfiles
+      uses: actions/download-artifact@v4
+      with:
+        name: lockfiles
+        path: requirements
+
+    - name: Install Python dependencies
+      run: |
+        python -m venv pygraphistry
+        source pygraphistry/bin/activate
+        python -m pip install --upgrade pip uv
+        uv pip install --require-hashes -r requirements/test-polars-py3.12.lock
+        uv pip install -e . --no-deps
+
     - name: Polars tests with coverage
-      if: ${{ matrix.python-version == '3.12' }}
       run: |
         source pygraphistry/bin/activate
         mkdir -p build/polars-coverage
         COVERAGE_FILE=build/polars-coverage/coverage.polars-py3.12 POLARS_COV=1 ./bin/test-polars.sh
 
     - name: Polars GFQL coverage audit (per-file floors)
-      if: ${{ matrix.python-version == '3.12' }}
       run: |
         source pygraphistry/bin/activate
         python bin/coverage_audit.py \
@@ -1636,7 +1678,6 @@ jobs:
         cat build/polars-coverage-audit/gfql-polars-coverage-audit.md | tee -a "$GITHUB_STEP_SUMMARY"
 
     - name: Upload polars coverage
-      if: ${{ matrix.python-version == '3.12' }}
       uses: actions/upload-artifact@v4
       with:
         name: polars-coverage-py3.12
PATCH

What it does: moves the py3.12 coverage pass + per-file audit + artifact upload into a new test-polars-coverage job (timeout-minutes: 20), drops 3.12 from the test-polars matrix so every remaining cell does identical work under an honest 10-minute budget, and adds the new job to changed-line-coverage's needs: since it consumes that job's artifact. py3.12 was already running only the coverage step, so no version loses a run. Rationale and the remaining structural limit are in #1797.

lmeyerov added a commit that referenced this pull request Jul 28, 2026
…erywhere (#1805)

* test(gfql): make the polars lane see the 280 tests it was skipping everywhere

polars is installed in exactly ONE CI lane (test-polars, driven by bin/test-polars.sh),
and that lane runs a hand-maintained file list. Every other lane collection-skips a
module-level `pytest.importorskip("polars")` and every polars/polars-gpu parameter. A
polars-gated test the list omits therefore runs NOWHERE while both lanes report green.

Measured on run 30311675717 (PR #1799): 280 tests executed locally with polars installed
and appear in ZERO job logs of that run, including six whole modules — four of them named
`test_engine_polars_*`. Two real defects were sitting in the blind spot:

  - `test_explicit_polars_gpu_declines_indexed_helper_and_falls_back` executes
    engine='polars-gpu' but only gates on `importorskip("polars")`, so it raises
    ImportError wherever polars is present without the RAPIDS cudf_polars stack. It has
    never once run: 9 SKIPPED, 0 executed, across the whole run.
  - `test_case_regex_unicode_trick_matrix[toupper-eq-ss-fold]` and
    `[tolower-turkish-dotted-i]` catch a genuine silent cross-engine divergence: under
    pandas>=3 the pandas engine answers [9] where polars answers [8, 9] for
    `toUpper(n.name) = 'STRASSE'`. Filed as #1802 with the repro; both rows are marked
    xfail(strict=True) keyed on the pandas accessor's own behaviour, so the fix XPASSes
    and retires the marks.

Changes:
  - bin/test-polars.sh gains the six invisible modules plus the four whose polars
    parameters were invisible.
  - New `test_polars_lane_completeness.py` parses POLARS_TEST_FILES and fails when a
    polars-mentioning test module is neither in the lane nor in a documented exemption,
    when a module-level polars gate sits outside the lane, when a listed path is gone, or
    when a polars-gated test in the `-k polars` phase is unselectable by that filter.
    Mutation-checked: dropping one entry turns two of its assertions red.
  - Split the polars-gpu test so the CPU half (helper must decline) runs wherever polars
    does, and the execution half skips honestly without cudf_polars.
  - Renamed the two test_lowering.py polars tests `-k polars` could not select.

Test-only; no product code touched, so no CHANGELOG entry.

Widened lane, py3.11 + polars 1.43.1 + pandas 3.0.5: 2726 passed, 142 skipped, 3 xfailed,
0 failed (was 1636 passed on the same box). ORDERING NOTE: #1797 records that the py3.12
coverage cell is already at its 10-minute ceiling and that adding a single file tipped it
into a timeout; the pending ci.yml split (comment on PR #1794) must land BEFORE this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB

* test(gfql): granular AST types in the polars-lane completeness guard

Replace the getattr/ast.AST pairing in the guard's two helpers with the concrete node
types they are actually called with: _node_source takes ast.stmt | ast.expr and reads
.lineno/.end_lineno directly (end_lineno is Optional[int], so the None case is handled
rather than defaulted through getattr), and _selected_by_k_polars takes a FuncDef alias
so .name and .decorator_list are checked attributes instead of getattr lookups with
invented fallbacks. No behaviour change; guard still 8 passed, ruff clean, mypy clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
lmeyerov and others added 2 commits July 27, 2026 19:59
…pes (#1787)

Pandas is the oracle and the contract is parity-or-NotImplementedError. Three
BOUNDED var-length shapes in the native polars `rows(binding_ops=...)` path
returned a DIFFERENT count with no error. Same root-cause family as the
unbounded case #1781 declined: pandas' step_pairs come from the var-length
`edge_op.execute` hop, whose hop-window pruning -- and, when seeded, its
per-seed BFS -- changes the edge multiplicity in a way a rebuild from the raw
matching edge table does not reproduce.

Declined (diverging graphs out of 60 random ones per shape, differential fuzz
against the pandas oracle):

  * directed `-[*k..m]->` with min_hops >= 3 (53/60; pandas 0 vs polars 35),
    and min_hops >= 2 off a FILTERED seed (27-30/60). Plain min_hops <= 2 is
    fuzz-clean, including as a non-first segment -- that is the graph-bench q3
    `-[*1..k]->` shape, still served.
  * undirected `-[*1..1]-` / `-[*1]-`, the degenerate window: 60/60, halves the
    count (pandas 36 vs polars 18). `-[*1..2]-` and wider agree, which is why
    the existing tests missed it.
  * undirected `-[*1..k]-` that does not start from the full node set: filtered
    seed 51/60, non-first segment 36/60. Every directed equivalent agrees, so
    this is specific to the undirected doubled-pair expansion.

The gate keys on an EXPLICIT var-length window rather than on
`EdgeSemantics.is_multihop`, because `-[*1..1]-` resolves to min == max == 1 and
so is NOT multihop -- yet pandas still routes it through the var-length hop
(`-[]-` gives 18 on a graph where `-[*1..1]-` gives 36). Plain `-[]-` is
untouched.

Tests: explicit decline + still-served neighbours for each of the three, plus a
seeded differential fuzz over random small graphs (cyclic, parallel edges,
self-loops) that asserts parity-or-raise AND that enough shapes are still served
for the check to mean anything. Bounded windows only: undirected unbounded
shapes through the pandas oracle can exhaust the box.

The new test file is REGISTERED in bin/test-polars.sh. That list is an explicit
allowlist, and the polars lane is the only CI lane with polars installed -- so
the only lane that can execute this file at all. Without the registration the
three `return None` decline statements were the only changed lines no lane ever
ran (10/13 = 76.92%, exactly the changed-line-coverage gate failure); with it
the block is 13/13.

Mutation-checked: dropping the min_hops clause fails 4 tests, the degenerate
undirected window 3, the undirected-seed clause 3, the whole gate 8. Full
graphistry/tests/compute is unchanged by the declines (9 pre-existing failures,
same set).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
…rs/cuDF/polars-gpu)

Review follow-ups on #1794.

Comments -> tests: the gate in row_pipeline.py carried per-shape prose asserting which
shapes diverge and by how much. A comment asserting behaviour is unverified and rots.
Every numeric claim is now a named test; the comment keeps only the WHY of the
deliberate divergence from master's serving decision plus a pointer to the tests.
29 -> 18 comment lines, executable logic byte-identical (the only non-comment hunks are
two trailing comments on existing lines).

Engine-parametrized: renamed to test_varlen_bounded_engine_parity_1787.py and
parametrized over pandas / polars / cuDF / polars-gpu. It encodes the INTENDED
per-engine behaviour rather than assuming identity -- the polars engines must decline
exactly the shapes the pandas-API engines must answer -- with oracle counts pinned as
literals so a regression that makes every engine equally wrong still fails. GPU params
gate on a runtime probe and report a reasoned SKIP, never a silent pass; the coverage
boundary (no CI lane runs cuDF or polars-gpu) is stated in the module docstring.
Dropping the module-level polars importorskip also means the pandas params now execute
in test-gfql-core, where the old file was skipped in its entirety (re: #1795).

Found while doing so: cuDF silently returns 1 where pandas returns 9 on seeded
undirected degenerate `(a {p:v})-[*1..1]-(b)` (23/40 random graphs; drop any one of
seeded/undirected/max==1 and it agrees). Separate defect, filed as #1798 and pinned here
with xfail(strict=True).

Mutation-checked both directions: reverting the gate to 233b64c fails 13 tests;
widening it to decline all bounded var-length fails 12.

Verified: dgx-spark GB10 / cuDF 26.02.01 / polars 1.35.2 via
graphistry/test-rapids-official:26.02-gfql-polars with --gpus all -- 80 passed,
1 xfailed, 0 skipped. CI-equivalent local polars lane (cuDF import-blocked) 2395 passed,
54 skipped, 0 failed. bin/lint.sh and bin/typecheck.sh clean.

Note: the accompanying CI fix for the test-polars py3.12 timeout (#1797) touches
.github/workflows/ci.yml and could not be pushed with the available token, which lacks
the `workflow` OAuth scope. The patch is attached to the PR.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015YsqAZQLbqjSDrYSFz2GoB
@lmeyerov
lmeyerov force-pushed the fix/gfql-1787-varlen-polars-declines branch from f8c3a28 to a41b2c3 Compare July 28, 2026 03:01
@lmeyerov
lmeyerov merged commit d08720e into master Jul 28, 2026
77 checks passed
@lmeyerov
lmeyerov deleted the fix/gfql-1787-varlen-polars-declines branch July 28, 2026 08:54
lmeyerov added a commit that referenced this pull request Jul 28, 2026
Conflicting files: CHANGELOG.md (both sides prepended to '### Changed'; took master's
file and re-inserted this branch's entry) and
graphistry/compute/gfql/lazy/engine/polars/row_pipeline.py (master's #1794 bounded
var-length decline gate + this branch's is_polars_df TypeGuard retype; disjoint
hunks, both kept).

No substance change to the branch. Verified on the merged tree: ruff clean repo-wide,
mypy clean repo-wide with polars absent (the config the python-lint-types lane runs),
and with polars stubs visible 4 -> 2 errors vs master (both remaining are pre-existing
in degrees.py, untouched here).
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.

GFQL polars: variable-length binding rows diverge from pandas (bounded min_hops>=3; undirected *1..1 halves; undirected filtered-seed over-counts)

1 participant