Repository navigation
Binder cross-kind alias rebind guard (#1357 partial) - #1372
Merged
Merged
Conversation
Centralize entity_kind alias-scope enforcement in FrontendBinder so a MATCH pattern that re-uses an existing scalar/path/edge alias as a node variable (or vice versa) is rejected at the binder layer with a structured GFQLValidationError carrying existing_kind / new_kind / new_role context, instead of being silently merged into a wrong-kind BoundVariable. Previously a downstream re-entry compile-time check at reentry/compiletime.py:274 caught the scalar→node case for one specific WITH-prefix shape with the message string "Cypher MATCH after WITH scalar-only prefix aliases cannot be reused as node variables". The new binder-level guard fires earlier and covers all four cross-kind transitions (node↔edge, edge↔node, path↔node, path↔edge) uniformly across MATCH and re-entry MATCH paths. The downstream guard is preserved as defense-in-depth. Updates two test_cycle_policy.py assertions and one test_lowering.py assertion to match the new error surface (binder-level instead of re-entry compile-time). The intent is unchanged — reject scalar→node rebind across WITH — only the rejection site and message moved. Surfaced by PR #1351 round-001 agent-03 binder-parity audit. This commit ships the safe portion of #1357 (cross-kind dimension); full strict_name_resolution flip at lowering.py:8400 is deferred — discovery in plans/1357-binder-strict-name-resolution/research/ classified 75 failures across multiple binder coverage gaps (namespaced functions, quantifiers, list comprehensions, CALL/YIELD scope, post-WITH UNWIND traversal) that need their own fixes first. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ypoint (#1357) 14 tests pinning the four cross-kind transitions (node↔edge, edge↔node, path↔node, path↔edge) plus same-kind admit cases. Exercises both FrontendBinder().bind() directly and compile_cypher_query so the guard's reach is locked at both layers. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ion rollout (#1357) 13 tests pinning current loose-mode admits at the binder + compile entry points for the five binder-coverage gaps the discovery flip surfaced (namespaced builtins, quantifier predicates, post-WITH UNWIND traversal, CALL/YIELD scope, and unresolved-alias rejections currently routed through downstream guards). When a follow-up PR closes a gap and is ready to flip strict mode, the corresponding test here flips from "admits" to "raises". The parameterized cases serve as the gap-by-gap readiness ledger. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
6 tests pinning where gfql_validate(strict=True) and compile_cypher_query agree (cross-kind rebind — parity ships in this PR) vs diverge (quantifier predicates, post-WITH UNWIND — known binder gaps blocking the post-normalize strict flip). Each divergence case becomes a parity case once the corresponding binder gap is closed. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
- test_binder_cross_kind_rebind.py: extend coverage with scalar-origin matrix (WITH expression projection, WITH literal projection — both routed through reentry MATCH path), dedicated reentry-MATCH cross-kind test (MATCH...WITH a MATCH ()-[a]->()), and a schema-vs-cross-kind precedence test pinning E301 (label miss) raises before E204 when both apply on the same MATCH. - test_binder_strict_compile_baseline.py: tighten exception assertions from broad Exception+substring to GFQLValidationError + context["value"] / structured str(...) — survives a future strict-mode move from E108 to E204 without going vacuous. - CHANGELOG.md: clarify wording — the guard covers all three cross-kind pairs (node↔edge, node↔scalar, edge↔scalar), where path aliases bind as entity_kind="scalar" alongside other scalar carriers (UNWIND, WITH-expr projection, CALL/YIELD). NOTE for follow-ups: UNWIND-then-MATCH cases are NOT reliably rejected today — _bind_graph_sequence binds ast.matches before ast.unwinds, so MATCH (x) binds x as a fresh node before UNWIND silently overwrites entity_kind. Tracked in #1371 P1. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This was referenced May 9, 2026
Closed
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
Centralize one dimension of #1357 (cypher binder strict-name-resolution) at the binder layer: cross-kind alias rebind. Defers the full
strict_name_resolution=Trueflip atlowering.py:8400to follow-up issues tracked in #1371 (binder coverage gaps)._bind_node_pattern/_bind_relationship_pattern/_bind_path_aliasnow reject MATCH patterns that re-use an existing alias as a different entity kind (node↔edge, edge↔node, path↔node, path↔edge) withGFQLValidationError(E204)carrying structuredexisting_kind/new_kind/new_rolecontext, instead of silently merging into a wrong-kindBoundVariable.graphistry/compute/gfql/cypher/reentry/compiletime.py:274caught the scalar→node case for one specific WITH-prefix shape with the message"Cypher MATCH after WITH scalar-only prefix aliases cannot be reused as node variables". The new binder-level guard fires earlier and covers all four cross-kind transitions uniformly across MATCH and re-entry MATCH paths. Downstream guard preserved as defense-in-depth.test_cycle_policy.pyassertions + 1test_lowering.pyassertion to match the new error surface (intent unchanged — reject scalar→node rebind across WITH).Why partial?
A discovery flip of
FrontendBinder().bind(query, PlanContext(), strict_name_resolution=True)atlowering.py:8400exposed 75 test failures ingraphistry/tests/compute/gfql/cypher/. Triage showed most failures are pre-existing binder coverage gaps surfacing as false positives in strict mode, not real-bugs-strict-catches:_bind_graph_sequenceWITH collect(b1) AS bees UNWIND bees AS b2 ...—beesevaluated against pre-WITH scope.WHERE all(x IN n.labels WHERE x = 'A')—xnot modeled as comprehension-local.CALL graphistry.degree() YIELD nodeId RETURN nodeId— YIELD aliases dropped before re-bind.RETURN duration.inSeconds(...)— parsed asalias.property.Each gap is a self-contained binder fix tracked in #1371 (meta-issue). When all four close, the post-normalize strict flip at
lowering.py:8400becomes safe.This PR ships the safe portion of #1357 (cross-kind dimension) plus regression-pin tests + parity baselines so the gap-by-gap readiness state is observable.
What's tested
test_binder_cross_kind_rebind.py— 14 tests covering all four cross-kind transitions plus same-kind admit cases. Exercises bothFrontendBinder().bind()directly andcompile_cypher_queryso the guard's reach is locked at both layers.test_binder_strict_compile_baseline.py— 13 regression-pin tests asserting current loose-mode admits at the binder + compile entry points for the binder-coverage gaps. Each parametrized case will flip from "admits" → "raises" as the corresponding gap closes ([META] Cypher binder coverage gaps blocking #1357 strict_name_resolution rollout #1371).test_validator_runtime_strict_parity.py— 6 tests pinning wheregfql_validate(strict=True)andcompile_cypher_queryagree (cross-kind rebind — parity ships in this PR) vs diverge (quantifier predicates, post-WITH UNWIND — known gaps blocking the post-normalize flip).Files
graphistry/compute/gfql/frontends/cypher/binder.py_cross_kind_rebind_errorhelper + entity_kind guards in_bind_node_pattern,_bind_relationship_pattern,_bind_path_alias.graphistry/tests/compute/gfql/cypher/test_binder_cross_kind_rebind.pygraphistry/tests/compute/gfql/cypher/test_binder_strict_compile_baseline.pygraphistry/tests/compute/gfql/cypher/test_validator_runtime_strict_parity.pygraphistry/tests/compute/gfql/cypher/test_cycle_policy.pygraphistry/tests/compute/gfql/cypher/test_lowering.pyCHANGELOG.mdTest plan
pytest graphistry/tests/compute/gfql/cypher/— 1264 passed, 94 skipped, 15 xfailed.pytest graphistry/tests/compute/— 2872+ passed (broader compute suite, no Cypher regressions)../bin/ruff.sh+./bin/mypy.sh— clean on touched files.python bin/ci_cypher_surface_guard.py— pass.Closes part of #1357 (cross-kind dimension); meta #1371 tracks the remaining strict-rollout gaps.
🤖 Generated with Claude Code