perf(gfql): index AUTO preserves polars frames (rework of #1767, stacked on #1743) - #1843
Merged
lmeyerov merged 5 commits intoAug 12, 2026
Conversation
lmeyerov
marked this pull request as ready for review
August 12, 2026 06:52
lmeyerov
force-pushed
the
perf/gfql-index-auto-polars-rework
branch
from
August 12, 2026 06:58
dee75f6 to
3423ff9
Compare
lmeyerov
added a commit
that referenced
this pull request
Aug 12, 2026
Rebasing #1843 onto current master surfaced a real interaction: the col-stats entry point (added by the typed-facts campaign AFTER this PR was written) used plain resolve_engine, and gfql_index_all now ENDS by calling it -- so under AUTO it coerced the preserved polars frames back to pandas at the last step, breaking all four inversion pins. Routed through resolve_index_engine like the other index entry points. 262 index tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
, stacked on #1743) resolve_engine(AUTO) maps polars input frames to PANDAS (legacy input-format policy), so gfql_index_all()/create_index() with the default engine coerce-and-REPLACED a user's polars frames with pandas copies: every later polars-engine query then paid a full-frame pandas->polars re-conversion (O(E) per call) and the resident pandas index could never fingerprint-match. New resolve_index_engine(): when the caller said AUTO and BOTH resident frames are eager polars, the index layer resolves POLARS and indexes the frames in place (the layer is already engine-polymorphic: numpy sidecar arrays + polars row-gather). Wired into create_index, _is_resident_index_valid, show_indexes, and the DDL wire's NODE_PROP reuse check. Explicit engines are unchanged. Why this is safe NOW and was not in 2026-07: the retracted #1767 shipped this without query-side AUTO routing, so an AUTO-built polars index met an AUTO(pandas) query -> every hop declined to the scan floor, a measured 125-534x DEFAULT-path regression. This branch is STACKED ON #1743 (fix/gfql-auto-engine-polars-native), whose gfql() guard routes AUTO on all-polars-frame graphs to the native polars engine -- so the cliff inverts into the win: AUTO builds the polars index, AUTO queries route polars, index serves. Pinned by test_inversion_auto_index_auto_gfql_serves_polars_index (index_trace() must show path=index engine=polars with NO engine argument anywhere, plus value parity vs both explicit-engine spellings). show_indexes() under AUTO now reports the routed truth through the #1841 usable/reason columns: the previously not-usable "polars index + AUTO query" combination flips to usable=True (test renamed to test_polars_index_auto_query_usable_the_1841_flip); explicit mismatched-engine previews keep the #1838 decline wording. Deliberately narrower than #1743's query gate, self-consistent either way because create_index coerces the frames it indexes: LazyFrame graphs keep the legacy pandas build (an index cannot gather from a lazy plan), and edges-only graphs keep it too (materialize_nodes() does not yet produce polars nodes from polars edges -- pre-existing gap, also reachable via explicit engine='polars'). Pandas/cuDF graphs, mixed frames, and nodes-only graphs are unchanged, each pinned. Verification (local CPU lane, cudf/gpu params env-excluded on this box): index suite 229 passed; polars cypher conformance + cache coverage lock 177 passed / 3 skipped; lint.sh clean; scoped mypy (api.py, wire.py) clean; type-hygiene guard OK (no growth). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr
Rebasing #1843 onto current master surfaced a real interaction: the col-stats entry point (added by the typed-facts campaign AFTER this PR was written) used plain resolve_engine, and gfql_index_all now ENDS by calling it -- so under AUTO it coerced the preserved polars frames back to pandas at the last step, breaking all four inversion pins. Routed through resolve_index_engine like the other index entry points. 262 index tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
…te agreement Review: 'why does resolve_index_engine reason about pandas on a polars path?' Because it POST-CORRECTED resolve_engine's legacy polars->pandas mapping instead of deriving from the frames -- a third independent AUTO gate alongside the legacy resolver and the #1849 query router, and third reimplementations drift. The gate now reads directly: explicit engines win; AUTO + both frames eager polars -> POLARS; everything else keeps the base resolution (never touches cudf/dask). The reviewer's deeper suspicion -- cross-engine pos/neg coverage -- was right. New test_auto_engine_agreement.py pins the property that actually matters: the INDEX gate and the QUERY gate must land together per frame combination, because their DISAGREEMENT is the #1767 failure class (index built for one engine meets a query routed to another; every hop declines to the scan floor; answers stay correct so nothing fails). Matrix: pandas/polars/lazy/mixed both structurally and END TO END (AUTO build then AUTO query, result frames must match the built engine). Explicit-engine-wins pinned. cudf pinned as source-native with the polars-gpu question left VISIBLE rather than silently decided either way. Also: _is_eager_polars typed Optional[DataFrameT] and built on is_polars_df (which admits LazyFrames -- the one-line reason it exists); the 32-line resolve_index_engine docstring cut to the two underivable facts (the #1767 cliff history and the do-not-widen constraint), gates pointed at their pins. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
… invariants On the stack, resolve_engine's base is POLARS for lazy/mixed too, so resolve_index_engine inverts from escalation to NARROWING: polars-resolved but not (both frames present and eager) -> pandas, because an index cannot gather rows from a lazy plan. The self-heal keeps the gates in AGREEMENT (create_index coerces the frames it indexes, so later AUTO queries route with post-build frames) -- pinned in test_auto_engine_agreement. Last two doctrine pins flipped per the never-pin-accidents rule: - 'hop AUTO still bridges to pandas' -- the bridge WAS the 1767-era accident; modern AUTO serves hop natively on polars frames, values pinned. - 'expected an engine_mismatch decline' -- under agreed gates that scenario CANNOT occur (both sides resolve polars); the invariant is an honest cost/coverage decline on the AGREED engine, never a mismatch. Full stack: 0 non-cudf failures across index + conformance + lowering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
lmeyerov
force-pushed
the
perf/gfql-index-auto-polars-rework
branch
from
August 12, 2026 21:23
cfba971 to
3377041
Compare
lmeyerov
changed the base branch from
master
to
feat/gfql-engine-resolve-modern
August 12, 2026 21:23
The polars-first dtype classifiers let the WHERE pushdown render string predicates and bare float comparisons natively, so two pins of the broken classifier's routing became false: the query-raises pin (the gap is closed, parity with the pandas oracle is the contract) and the float-residual spy (a bare comparison no longer reaches the residual lane; abs() keeps the NaN-guard test on the lane it exercises). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
Contributor
Author
|
Folded into #1873 and closing: the two halves are atomically coupled — this PR's index-gate agreement (and its flipped pins) need modern AUTO to exist, and #1873's resolver change leaves the old index gate red without this rework. Neither is green standalone at any merge point, so they land as one reviewable unit. All review items from this thread are addressed in the folded commits (715575b, c8c1325, e05d3cd, 3377041): typed |
lmeyerov
added a commit
that referenced
this pull request
Aug 13, 2026
Rebasing #1843 onto current master surfaced a real interaction: the col-stats entry point (added by the typed-facts campaign AFTER this PR was written) used plain resolve_engine, and gfql_index_all now ENDS by calling it -- so under AUTO it coerced the preserved polars frames back to pandas at the last step, breaking all four inversion pins. Routed through resolve_index_engine like the other index entry points. 262 index tests pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
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
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.
DO NOT MERGE BEFORE #1743
Stacked on #1743 (
fix/gfql-auto-engine-polars-native) and contains its commits until it lands (plus a sync merge of current master to pick up #1841'susable/reasoncolumns). Draft against master so CI runs; once #1743 merges, this rebases/retargets to a clean index-only diff. Do not land this first — the whole point of the rework is the ordering.What this is: the retracted #1767, made safe by sequencing
The retracted #1767 made
gfql_index_all()/create_index()underengine='auto'preserve resident polars frames (index in place, no pandas coerce-and-replace). It was retracted because query-side AUTO still routed polars-frame graphs to pandas, so the AUTO-built polars index met an AUTO(pandas) query and every hop declined to the scan floor — a measured 125–534x DEFAULT-path regression (250K/1M/4M edges, dgx-spark, receipts in the #1767 thread).#1743 removes the mismatch at its root:
g.gfql(...)with no engine on an all-polars-frame graph now routes to the native polars engine. With that underneath, the index-side preservation inverts from the cliff into the win:The inversion pin
test_inversion_auto_index_auto_gfql_serves_polars_index:gfql_index_all()+g.gfql(<index-served query>)with no engine argument anywhere — the exact two-default-spellings scenario that regressed — must showpath=index,engine=polarsinindex_trace(), answer in polars frames, and value-match BOTH explicit-engine spellings (polars/polars and pandas/pandas oracles).The #1841 flip
show_indexes()under AUTO now reports the routed truth: the "polars index + AUTO query" combination that #1841 was built to call out asusable=Falseflips tousable=True(test_polars_index_auto_query_usable_the_1841_flip, plus the all-default spelling intest_auto_built_index_reports_usable_under_auto). Explicit mismatched-engine previews (show_indexes(engine='pandas')against a polars index) keep #1838's decline wording, pinned per kind.Change surface (index-only; no routing — that is #1743's job, underneath)
graphistry/compute/gfql/index/api.py: newresolve_index_engine()— AUTO + both frames eager polars →Engine.POLARS; wired intocreate_index,_is_resident_index_valid,show_indexes. Explicit engines untouched.graphistry/compute/gfql/index/wire.py: the DDLCREATE INDEXNODE_PROP reuse check uses the same resolution (otherwise IF-valid reuse would mismatch the build it just did).Ported from old #1767: the
_resolve_index_enginemechanism (now also coveringshow_indexes/wire, which the old diff predated — #1841/#1838 didn't exist then), the frame-identity preservation contract, and the whole negative-pin test suite (LazyFrame, mixed-frame, nodes-only, edges-only, pandas-stays-pandas, cudf-stays-cudf, explicit-engine-still-coerces, index-service spy with pandas-oracle parity).Dropped from old #1767: its 276x standalone-perf framing (the thread showed the explicit-engine fast path was already reachable on master; the honest claim here is default-path consistency), and its CHANGELOG entry (rewritten as behavior-only with the landing-order dependency stated).
Gate deliberately narrower than #1743's query gate, self-consistent because
create_indexcoerces the frames it indexes (later AUTO queries route with the post-build frames): LazyFrame graphs keep the legacy pandas build (an index cannot gather rows from a lazy plan); edges-only graphs too —materialize_nodes()does not yet produce polars nodes from polars edges, a pre-existing gap also reachable via explicitengine='polars'on such a graph (probed empirically; guessing polars there would crash the build).Known interaction left to #1743's review: on cudf-frame graphs, #1743's polars-gpu arm can route an AUTO query away from a resident cudf index (GPU lanes only); index resolution here deliberately leaves cuDF source-native and takes no position on that arm.
Tests (CPU lane, this box: cudf/gpu params env-excluded — no GPU libs)
graphistry/tests/compute/gfql/index/: 229 passed (83 deselected cudf/gpu)test_index.pyalone: 142 passed; new/changed: inversion pin, fix(gfql): show_indexes reports engine usability, not just fingerprint validity #1841 flip x2, DDLSHOW GFQL INDEXESflip, per-kind reason under explicit mismatch, + the portedTestIndexAutoPreservesPolarsFrames(11 tests)bin/lint.shclean · scoped mypy (api.py,wire.py) clean · type-hygiene guard OK (no growth)libnvrtcOSError(master's owntest_cudf_index_pandas_query_not_usablefails identically here); they are GPU-lane tests.🤖 Generated with Claude Code
https://claude.ai/code/session_011AB4RZpph3uSFUpzKnZJcr