feat(gfql): instrument the seeded typed hop, completing fast-path engagement - #1869
Merged
Conversation
…agement Closes the coverage gap the audit itself exposed: #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
CI caught a real regression, not a stale test: test_..._1767_cliff asserts every SERVED step carries engine=='polars', and my fast_path records omitted engine entirely. The right fix is the record, not the test -- engagement is a PER-ENGINE property (a path can serve on one engine and decline on another; that is the whole reason the pins are parametrized), so the engine belongs in it exactly as it already does for adjacency decisions. Both enum spellings reach the recorder, and str(Engine.POLARS) is 'Engine.POLARS' not 'polars', so it normalizes to the wire value every other step records. CHANGELOG entry added under Added. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
…to str CI (mypy 3.12-3.14): the executor holds 'EngineAbstract | str' at this seam -- post-parse but NOT yet narrowed to a Literal -- so EngineAbstractType was too narrow for the call sites. Named it TraceEngine in index/types.py rather than the two alternatives I would otherwise have reached for: widening the parameter back to a bare str (which is what the reviewer flagged in the first place) or casting at three call sites (which hides the widening and trips the hygiene ratchet). One definition, with a comment on WHY the type is wide there; the recorder still normalizes to the wire value every other trace step uses. mypy clean on all three modules; hygiene clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi
Review pass on this PR's own additions, same rule applied to everything else: - The enum-normalization prose appeared TWICE (both recorders) for a one-line expression that test_..._1767_cliff already pins by asserting engine=='polars'. Removed both. - The per-engine rationale kept, at one line instead of three: WHY the field is recorded is a design reason, not something a passing test conveys. - TraceEngine's note trimmed to why the type is wide, which is the only part the type system cannot state itself. 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.
Closes the coverage gap the engagement audit itself exposed. #1868 traced two of the three fast paths, so a clean audit only meant "the two instrumented ones fire" —
seeded_typed_hopis the third and last at this seam.The audit result, now complete
Ten shape × engine combinations across pandas and polars: no silently-dead fast paths. Every shape engages the path it should, and the untyped shape correctly declines all three.
Its pin doubles as evidence about the others
seeded_typed_hopis consulted last, so seeing all three in one decision map proves the earlier two declined rather than short-circuited — which is the distinction that separates "working correctly" from "never reached".The negative control did its job
Adding the third path broke
test_a_shape_neither_path_serves_declines_bothon pandas and polars. That was correct behaviour, not breakage: it asserts an exact dict, so instrumenting a new path forces it to be revisited. A negative control that silently ignores a new path stops being a control.🤖 Generated with Claude Code
https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi