feat(gfql): fast-path engagement in gfql_explain + assert_fast_path - #1868
Merged
Conversation
lmeyerov
force-pushed
the
feat/gfql-trace-fastpaths
branch
from
August 11, 2026 02:47
7f5890c to
e707a87
Compare
Extends the #1860 col-stats trace to the fast paths themselves, which is what the engagement audit needs. Fast paths are contracted 'same answer, faster', so a DEAD one is invisible: the fallback returns the right result and every value test passes. Ratio in test_lowering.py: 665 value assertions vs 42 engagement. Recorded at the CALL SITE in gfql_unified, not via shims or in-body hooks: it is one place, it is where the decision is consumed, and it cannot be bypassed the way patching a directly-imported name is -- gfql_unified imports these names directly, which already produced one false negative during an audit. assert_fast_path(g, query, path, served=) makes engagement a one-liner against a PUBLIC surface. Pinned: both paths engage on their shapes; a shape neither serves declines both with a correct answer; SHORT-CIRCUIT is distinguishable from DECLINE (when grouped-agg serves, two_hop_count is absent from the map rather than present-and-False -- conflating those lets a dead path read as 'declined for a good reason'); and the helper FAILS when the path did not fire. Registered in POLARS_TEST_FILES. Local: 0 non-cudf failures. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
Reviewer: insufficiently typed -- Any instead of Plottable, bare str instead of Literal. All three flagged cases were real. - FastPathName is now a Literal in index/types.py, next to the other index vocabularies. Closed BY CONSTRUCTION: adding a path means adding it there, which is what stops a typo silently recording a decision nobody can assert on. - ColStatsOutcomeName likewise, and api.py's local copy now ALIASES it rather than duplicating the four strings in two files. - g: Any -> Plottable, engine: str -> EngineAbstractType, and the outcomes map is keyed by the outcome Literal across all four engagement helpers -- including the two that landed in #1861 with the same weakness. - The two remaining Any are on trace-step dicts, annotated hygiene-ok: a trace step is a heterogeneous TypedDict by contract, same rationale filter_mask_by_dict already carries. Added a runtime pin for the Literal's failure mode: a mistyped path name must fail LOUDLY, because 'never consulted' otherwise reads exactly like a correctly declining path. mypy clean on all changed modules; hygiene clean; lint 2/2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
Reviewer: cross-platform testing. Real gap -- the engagement pins were pandas-only, and engagement is per-ENGINE: a fast path that serves on pandas can silently DECLINE on polars or cuDF and no value test would notice, because both engines still answer correctly via the fallback. That is the exact blind spot these pins exist for, so pandas-only made them half a test. All three shapes now run on all three engines (12 cases). Local: 11 pass, the one cudf arm fails on this box's missing libnvrtc -- GPU-verified separately. Note #1865 does NOT need the same treatment: _semi is a polars-only function and polars does not mix eagerness, so eager/lazy IS its cross-platform axis, and polars-gpu shares the CPU polars path. Already parametrized that way. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
lmeyerov
force-pushed
the
feat/gfql-trace-fastpaths
branch
from
August 11, 2026 03:51
e7cbb3f to
13f9a21
Compare
pull Bot
pushed a commit
to admariner/pygraphistry
that referenced
this pull request
Aug 11, 2026
…agement Closes the coverage gap the audit itself exposed: graphistry#1868 traced two of the three fast paths, so a clean audit only meant 'the two instrumented ones fire'. seeded_typed_hop is the third and last at this seam. Its pin doubles as evidence about the other two: because it is consulted LAST, seeing all three in one map proves the earlier paths DECLINED rather than short-circuited. The negative control caught this change, which is the point: it asserts an EXACT dict, so adding a path forces it to be revisited. A control that silently ignores a new path stops being a control -- the two failures on pandas and polars were correct behaviour, not breakage. 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
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.
Extends the #1860 col-stats trace to the fast paths themselves — the surface the engagement audit needs.
Fast paths are contracted same answer, faster, so a dead one is invisible: the fallback returns the right result and every value test passes. Ratio in
test_lowering.py: 665 value assertions vs 42 engagement.Recorded at the call site, not via shims
gfql_unifiedimports these names directly, so patching the definition site never intercepts — that already produced one false negative during an audit here. Recording where the decision is consumed is one place, covers every return path, and cannot be bypassed.Pins
two_hop_countis absent rather than present-and-False; conflating them lets a genuinely dead path read as "declined for a good reason"assert_fast_pathfails when the path did not fire — an engagement pin that cannot fail is worse than noneRegistered in
POLARS_TEST_FILES; 0 non-cuDF failures locally.🤖 Generated with Claude Code
https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi