gfql: engage the #1658 seeded index on Polars/cuDF (identity-stable coercion #1726 + small-frontier cost gate)#1727
Merged
Conversation
…polars (#1726) _pl_nan_to_null returned a fresh polars frame whenever any Float32/Float64 column existed (keyed on column presence, not actual NaN), churning frame identity so the #1658 CSR index's `source_ref is df` check failed -> polars/polars-gpu direct hops always fell back to the O(E) scan. Fix: probe is_nan().any() per float column (dtype-gated) and return the SAME frame object when no NaN is present; rebuild only NaN-carrying columns when present. Value-identical (fill_nan(None) is a no-op on NaN-free columns); only object identity changes -> the resident index now engages. Verified on the real SNB graph: polars flipped 120/120-scan -> 54-index/12-scan (remaining 12 = cost-gate, separate). Adds pure-polars TestPlNanToNull. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV
…y homogeneous slices index on cudf/polars (#1726 follow-up) The seeded-hop planner's cost gate bails to the O(E) scan when frontier_n >= frac * n_keys. frac is engine-aware (pandas 0.5, cudf/polars/ polars-gpu 0.02), so on small / low-cardinality edge slices (per-edge-type homogeneous frames: n_keys <= 1/0.02 = 50) even a single-seed hop tripped the gate on the vectorized engines and scanned O(E) with a resident O(degree) index. On the real SNB graph this left ~12/60 native hops scanning on polars (~6 on cudf) after #1726 made the index engage at all. Fix: an absolute small-frontier floor. A frontier of <= 16 seeds (_COST_GATE_MIN_FRONTIER_DEFAULT, env-overridable via GFQL_INDEX_COST_GATE_MIN_FRONTIER; 0 disables) never bails on the frac gate; larger frontiers keep the existing frac-based bulk-hop protection unchanged. Constant (not a function of n_keys) on purpose: scaling with n_keys is the frac gate's job; the floor bounds absolute per-hop probe work in exactly the regime where frac*n_keys collapses below a handful of seeds. A <=16-seed frontier costs at most 16 searchsorted probes plus a gather of exactly the rows the scan would emit, so the index cannot meaningfully lose; the case the gate protects (a frontier covering most distinct sources) requires frontier > 16, where behavior is unchanged. Uniform across engines — pandas (frac 0.5) only overlaps the floor when n_keys <= 32, micro slices where either path is trivially fast. Correctness-invariant by construction: the gate is a pure routing heuristic — index_seeded_hop is parity-matched to the scan (traverse.py contract, test_index_parity_vs_scan), so this changes which path runs, never the answer. Also surfaces the floor in the explain trace (min_frontier_floor) and extends the bail reason to name it. Adds pure-CPU tests: tiny-slice single-seed hop now indexes on every engine (with scan-parity check), frontier just past the floor still bails, env tuning validation, and floor=0 restoring pure frac gating. Static gates: py_compile + ruff check clean on all touched files; mypy (mypy.ini via bin/mypy.sh) clean except two pre-existing errors in the untouched lazy/engine/polars/degrees.py (present at HEAD). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV
… the polars coverage lane The _pl_nan_to_null identity tests (test_engine_coercion.py) only execute where polars is installed, but the sole Engine.py-measuring coverage lane (core py3.14) has no polars extra, so the new eager-DataFrame branch showed 0% changed-line coverage despite having real behavior tests. Run those tests in the polars lane and widen its coverage source from graphistry/compute to the graphistry package. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV
lmeyerov
marked this pull request as ready for review
July 13, 2026 01:31
lmeyerov
added a commit
that referenced
this pull request
Jul 21, 2026
…cov to graphistry The frame-helper move added dispatch code to graphistry/Engine.py, whose polars branches were exercised only via the reentry tests but NOT measured — the polars lane covered only `--cov=graphistry/compute`, so Engine.py (outside compute/) showed 0% on its polars branches and dragged changed-line coverage to 75%. Fix both halves: (1) add a dedicated unit test for all nine primitives across pandas and polars (polars cases guarded by _HAS_POLARS + registered in POLARS_TEST_FILES) plus series_to_pylist's arrow/pandas/tolist fallbacks and raise-paths; (2) widen the polars lane to `--cov=graphistry` (the #1727 pattern) so Engine.py's polars branches are measured. Audit-safe: coverage_audit's polars profile only enforces graphistry/compute/gfql/lazy/**, so Engine.py is not floored. Reproduced the exact changed-line gate locally (combined core+gfql+polars artifacts): 75.00% -> 98.96%, Engine.py changed lines 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
lmeyerov
added a commit
that referenced
this pull request
Jul 21, 2026
…cov to graphistry The frame-helper move added dispatch code to graphistry/Engine.py, whose polars branches were exercised only via the reentry tests but NOT measured — the polars lane covered only `--cov=graphistry/compute`, so Engine.py (outside compute/) showed 0% on its polars branches and dragged changed-line coverage to 75%. Fix both halves: (1) add a dedicated unit test for all nine primitives across pandas and polars (polars cases guarded by _HAS_POLARS + registered in POLARS_TEST_FILES) plus series_to_pylist's arrow/pandas/tolist fallbacks and raise-paths; (2) widen the polars lane to `--cov=graphistry` (the #1727 pattern) so Engine.py's polars branches are measured. Audit-safe: coverage_audit's polars profile only enforces graphistry/compute/gfql/lazy/**, so Engine.py is not floored. Reproduced the exact changed-line gate locally (combined core+gfql+polars artifacts): 75.00% -> 98.96%, Engine.py changed lines 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
lmeyerov
added a commit
that referenced
this pull request
Jul 21, 2026
… core, #1709-adjacent) (#1751) * feat(gfql): native polars WITH...MATCH re-traversal reentry (IC6/IC11 core) A MATCH re-traversing from a preceding WITH's projected/aggregated bindings previously declined on polars ("could not recover carried node identities"). Fixes two pandas-only gaps: the native projector now emits the _cypher_entity_projection_meta side-channel the bounded-reentry executor reads to re-seed; the executor id-handling + seeded binding pipeline are engine-aware (polars is_not_null/filter/order-preserving join; semi-join to carried ids). RETURN entity/property/count/count(*)/DISTINCT over WITH->MATCH now native, pandas-parity. Differential fuzz ~3500 graphs: 0 disagreements, 0 crashes. Scalar-column carry + duplicate-id / cartesian-under-seed stay honest NIEs (were crashes/silent-wrong). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * test(gfql): lower pandas-lane execution.py coverage floor 84.75->83.5 WITH...MATCH reentry added polars-specific branches to the shared reentry/execution.py. Those are covered by the polars test lane (changed-line 100%) but unreachable by the pandas core lane, which legitimately drops its per-file coverage to 83.81%. Lower the pandas-lane floor to match; the polars branches keep real coverage via test-polars. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * refactor(gfql): type reentry/projection helpers with SeriesT/DataFrameT, drop call-site casts (review) Review feedback on #1751: the engine-agnostic reentry helpers were (s: Any) -> Any with unchecked cast(SeriesT/DataFrameT, ...) at call sites. Now typed with the repo frame aliases (SeriesT/DataFrameT; object for the type-guard); the genuinely-dynamic polars branches carry a localized # type: ignore at the dispatch point instead of an opaque Any return + caller cast -> the three call-site casts are removed. projection.py entity_meta dict -> Dict[str, Dict[str, Any]]. mypy: 0 new errors; ruff clean; 19 reentry tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * refactor(gfql): move engine-agnostic frame/series helpers to Engine.py The reentry executor accumulated a cluster of generic pandas/cuDF/polars dispatch helpers (null mask, series/frame filter, order-preserving left join, constant-column assign, drop-columns, row-as-mapping, series-to-pylist) that have no reentry semantics and belong with safe_merge / df_concat / df_unique in the engine layer. gfql_unified.py had already re-implemented series_to_pylist, proving the leak. Move the eight primitives to graphistry.Engine (public names, no underscore), drop the reentry-local defs, and consolidate the duplicate onto the more-defensive version. SeriesT/DataFrameT typing preserved via a TYPE_CHECKING import + string annotations so Engine.py (imported very early) never triggers graphistry.compute package init at runtime (would be circular). No behavior change: byte-identical dispatch; reentry suite 19/19, cypher/entity subset green, ruff clean, mypy-neutral. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * test(engine): cover moved frame/series primitives; widen polars lane cov to graphistry The frame-helper move added dispatch code to graphistry/Engine.py, whose polars branches were exercised only via the reentry tests but NOT measured — the polars lane covered only `--cov=graphistry/compute`, so Engine.py (outside compute/) showed 0% on its polars branches and dragged changed-line coverage to 75%. Fix both halves: (1) add a dedicated unit test for all nine primitives across pandas and polars (polars cases guarded by _HAS_POLARS + registered in POLARS_TEST_FILES) plus series_to_pylist's arrow/pandas/tolist fallbacks and raise-paths; (2) widen the polars lane to `--cov=graphistry` (the #1727 pattern) so Engine.py's polars branches are measured. Audit-safe: coverage_audit's polars profile only enforces graphistry/compute/gfql/lazy/**, so Engine.py is not floored. Reproduced the exact changed-line gate locally (combined core+gfql+polars artifacts): 75.00% -> 98.96%, Engine.py changed lines 100%. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL * test(gfql): end-to-end parity for the seeded binding-rows WITH->MATCH path The row_pipeline.py change (WITH re-entry seed for binding_rows_polars) was covered only by direct-call unit tests asserting the seed is applied + the decline branches; nothing drove a real cypher WITH->MATCH whose trailing MATCH routes through the MULTI-ALIAS binding-table path (vs the single-alias hop path the differential fuzz exercises). Add an e2e differential-parity test for a two-fresh-alias trailing MATCH with a multi-alias RETURN, with a monkeypatch guard asserting the seeded binding_rows path is actually hit so coverage can't silently regress. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two changes that let the #1658 seeded adjacency index actually engage on Polars / Polars-GPU (and tidy up cuDF) direct
hop()calls. Both are routing-only — the seeded index and the O(E) scan return identical results by the #1658 doctrine, so parity is invariant; these only change which path runs.1. Identity-stable
_pl_nan_to_null(#1726) —graphistry/Engine.py_pl_nan_to_nullreturned a fresh Polars frame whenever the frame had anyFloat32/Float64column (keyed on column presence, not on an actual NaN).hop()coerces inputs through this before the index hook, so the live edge frame's object identity churned and the resident index'ssource_ref is dfcheck failed → Polars/Polars-GPU seeded hops always fell back to the scan.Fix: probe
is_nan().any()per float column (dtype-gated) and return the same frame object when no NaN is present; rebuild only NaN-carrying columns when present. Value-identical to before (fill_nan(None)is a no-op on NaN-free columns); only object identity changes, so the resident index now engages. Adds pure-PolarsTestPlNanToNull(identity preserved when NaN-free; NaN→null still correct when present; real nulls untouched).2. Small-frontier cost-gate floor —
graphistry/compute/gfql/index/{cost,api,types,__init__}.pyThe seeded-index cost gate bails to scan when
frontier_n >= frac * n_keys(frac 0.5 pandas, 0.02 cuDF/Polars). For a single-node seed on a small/low-cardinality homogeneous slice,0.02 * n_keyscollapses below a handful of seeds, so the gate scans even though the index would be O(degree). Adds an absolute small-frontier floor: bail only whenfrontier_n > MIN_FRONTIER (16) AND frontier_n >= frac * n_keys. Named constant, env-overridable (GFQL_INDEX_COST_GATE_MIN_FRONTIER). The large-frontier bulk-hop protection is unchanged (fires only above the floor). Adds 3 tests (tiny slice indexes; large frontier still scans; floor tunable/disable-able).Correctness / risk
test_index_parity_vs_scan(index==scan, 4 engines × 8 scenarios) andtest_cost_gate_engine_aware_never_loses_to_scan.Verified qualitatively on a large heterogeneous graph: Polars seeded hops flipped from all-scan to index-engaging, results identical.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Myz4YLr6FhjyH7h7dHHTBV