feat(gfql): surface column-stat fact decisions in gfql_explain - #1860
Merged
Conversation
lmeyerov
added a commit
that referenced
this pull request
Aug 9, 2026
…ic surface Perf work is contracted 'same answer, faster', so a DEAD optimization is invisible: the scan fallback returns the right result and every value test stays green. Measured in test_lowering.py: 665 value assertions vs 42 engagement ones, and three bugs in this stack lived in that gap. The usual way to write engagement pins -- patch a private callee and count calls -- FAILS OPEN. If another module did a direct 'from .gfql_fast_paths import X', it holds its own reference and patching the definition site never intercepts, so the pin cannot fail and the silence reads like a dead code path. I produced that false negative while auditing. These helpers read the public gfql_explain trace instead, so they observe the decision where it is made. assert_col_stats asserts served/not-served and per-(role, column) outcomes, and refuses to pass vacuously when no consult was reached. The existing trace pins now dogfood it, and a self-test asserts the helper FAILS when the optimization did not fire -- an engagement pin that cannot fail is worse than none. Stacked on #1860. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
Facts are a pure accelerator, so a dead one is invisible: the query still returns the right answer via the scan, every value test stays green, and you pay the build for nothing. That is the failure mode behind three bugs in this stack -- the boolean label form never engaging, the shared-pass fingerprint making every fact look stale, and col_stats_by_type silently no-oping. Each consult now records op=col_stats with an outcome whose cases need different fixes: absent / stale / insufficient / served. 'insufficient' is what a whole-frame fact always is against a typed pattern -- the structural limitation that took a full investigation to diagnose is now one trace line. Gated by the same _trace_active() check the adjacency decisions use, so it costs nothing outside index_trace()/gfql_explain; pinned both ways. It also makes engagement testable against a PUBLIC surface rather than by patching private callees -- which matters: patching a name that another module imported directly does not intercept, and produces a false negative that reads like a dead optimization. Stacked on #1859. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
CI mypy (3.12/3.13) rejected the new trace records: IndexTraceStep is a TypedDict with a closed key set and Literal-constrained path/decision_code, so the added role/column/type_column/type_value keys and the col_stats_* codes were type errors. Local mypy passed only because I checked a file subset. Declared properly rather than cast around: the keys join IndexTraceStep, 'facts' joins IndexPath, the four col_stats_* codes join IndexDecisionCode, and outcome -> code is an explicit checked mapping so adding one side without the other is a type error instead of a string that silently never matches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
lmeyerov
force-pushed
the
feat/gfql-colstats-trace
branch
from
August 9, 2026 22:39
547590f to
00c57e0
Compare
lmeyerov
added a commit
that referenced
this pull request
Aug 9, 2026
…ic surface Perf work is contracted 'same answer, faster', so a DEAD optimization is invisible: the scan fallback returns the right result and every value test stays green. Measured in test_lowering.py: 665 value assertions vs 42 engagement ones, and three bugs in this stack lived in that gap. The usual way to write engagement pins -- patch a private callee and count calls -- FAILS OPEN. If another module did a direct 'from .gfql_fast_paths import X', it holds its own reference and patching the definition site never intercepts, so the pin cannot fail and the silence reads like a dead code path. I produced that false negative while auditing. These helpers read the public gfql_explain trace instead, so they observe the decision where it is made. assert_col_stats asserts served/not-served and per-(role, column) outcomes, and refuses to pass vacuously when no consult was reached. The existing trace pins now dogfood it, and a self-test asserts the helper FAILS when the optimization did not fire -- an engagement pin that cannot fail is worse than none. Stacked on #1860. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
lmeyerov
added a commit
that referenced
this pull request
Aug 9, 2026
…ic surface Perf work is contracted 'same answer, faster', so a DEAD optimization is invisible: the scan fallback returns the right result and every value test stays green. Measured in test_lowering.py: 665 value assertions vs 42 engagement ones, and three bugs in this stack lived in that gap. The usual way to write engagement pins -- patch a private callee and count calls -- FAILS OPEN. If another module did a direct 'from .gfql_fast_paths import X', it holds its own reference and patching the definition site never intercepts, so the pin cannot fail and the silence reads like a dead code path. I produced that false negative while auditing. These helpers read the public gfql_explain trace instead, so they observe the decision where it is made. assert_col_stats asserts served/not-served and per-(role, column) outcomes, and refuses to pass vacuously when no consult was reached. The existing trace pins now dogfood it, and a self-test asserts the helper FAILS when the optimization did not fire -- an engagement pin that cannot fail is worse than none. Stacked on #1860. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
lmeyerov
added a commit
that referenced
this pull request
Aug 11, 2026
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
lmeyerov
added a commit
that referenced
this pull request
Aug 11, 2026
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
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.
Stacked on #1859 (base is
perf/gfql-colstats-bytype-flag, not master).Closes the gap behind three bugs in this stack. Facts are a pure accelerator, so a dead one is invisible: the query still returns the right answer via the scan, every value test stays green, and you pay the build for nothing. That is exactly how the boolean-label rejection, the shared-pass fingerprint regression, and the
col_stats_by_typeno-op all survived a green suite.Each consult now records
op: col_statswith an outcome whose cases need different fixes:absentstaleinsufficientservedImmediately diagnostic — this is the structural limitation that took a full investigation to find, now one line:
Gated by the same
_trace_active()check the adjacency decisions use, so it costs nothing outsideindex_trace()/gfql_explain; pinned both ways.It also makes engagement testable against a public surface instead of monkeypatching private callees. That matters concretely: patching a name another module imported directly does not intercept the call, and I produced exactly that false negative while auditing — it read like a dead fast path.
🤖 Generated with Claude Code
https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi