Repository navigation
feat(cypher): ReentryPlan + multi-whole-row prefix WITH (#989 slices 4.1+4.3a) - #1248
Merged
Merged
Conversation
…#989 slice 1) Slice 4.1 of the #989 row-carrier cleanup: add an explicit ReentryPlan + CarriedAlias contract for the bounded MATCH-after-WITH path, populated alongside the existing scalar_reentry_alias / scalar_reentry_columns fields on CompiledCypherExecutionExtras. This commit is additive and a no-behavior-change refactor: * New module graphistry/compute/gfql/cypher/reentry_plan.py * New field reentry_plan: Optional[ReentryPlan] on CompiledCypherExecutionExtras * New property accessor on CompiledCypherQuery * Population at the bounded-reentry construction site; propagation through _map_terminal_reentry_query and _attach_graph_context Read sites still consume scalar_reentry_alias / scalar_reentry_columns and _compiled_query_reentry_contract; later slices switch readers to the plan and lift the single-whole-row constraint. Tests: graphistry/tests/compute/gfql/cypher/ — 1091 passed, 66 skipped, 15 xfailed (no regressions). Refs #987 #989 #999. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…es unused (#989 slice 4.3a) Lift the single-whole-row gate at lowering.py:7207 (compile-time) and gfql_unified.py:1220 (runtime contract). Multi-whole-row prefix WITH (e.g. WITH a, x, y) now compiles when only the trailing-MATCH source alias is referenced downstream. For the case IC3 actually needs — non-source whole-row aliases referenced in trailing WHERE / RETURN / WITH — emit an actionable failfast pointing to #989. The full row-carrier rewrite (slice 4.3b) lands the carry of non-source alias properties as hidden columns; that is tracked under #989 as the architectural exit gate. Implementation: * `_bounded_reentry_carry_columns`: returns `(reentry_alias, carried_scalars, non_source_alias_names)` instead of raising on N>1 whole-rows. Picks `reentry_alias` from the trailing-MATCH first node alias when it is a carried whole-row, else falls back to the first. * `_collect_non_source_alias_references`: scans trailing WHERE (predicates + tree path), trailing WITH stages, RETURN items, ORDER BY items, and reentry UNWINDs for references to non-source aliases. * `_iter_property_refs`: walks structured `WhereClause.predicates` for `PropertyRef` nodes (the alias-keyed reference shape). * `_compiled_query_reentry_contract` (gfql_unified): consults the ReentryPlan field added in the prior slice and picks the recorded reentry_alias_name when present. * `ReentryPlan.aliases` now includes a `CarriedAlias` entry per non-source whole-row alias for future slices to populate `carried_properties`. Tests: graphistry/tests/compute/gfql/cypher/ — 1093 passed (+2 new focused cases for multi-whole-row admit + non-source-alias failfast). Refs #987 #989 #999. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…urce-alias scanner (#989 slice 4.3a follow-up) Review of PR #1248 surfaced a latent AttributeError in _collect_non_source_alias_references: stage.where on ProjectionStage is Optional[ExpressionText] (raw text node), not Optional[WhereClause]. The prior code passed it to _scan_where(...) which crashes on .expr_tree on any multi-whole-row prefix followed by a downstream WITH ... WHERE stage. Fix: scan stage.where.text directly (symmetric to the unwind / return / order_by branches in the same loop). Add regression test test_string_cypher_admits_multi_whole_row_prefix_with_downstream_stage_where exercising the previously-broken code path on a green-execution shape. Tests: 1094 passed (+1 new regression), no regressions. Refs #989 #999. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…ields, add plan structure test, comment cleanup (#989) Wave 2 review of PR #1248 (parallel per-dimension subagents) surfaced two IMPORTANT findings that survived the adversarial pass: * Spec + Quality consensus: `CarriedAlias.id_column` (Optional[str] = None) and `carried_properties` (Tuple[str, ...] = ()) were declared but never set, violating CLAUDE.md "Don't design for hypothetical future requirements." Removed; will land in slice 4.3b at first set. * Tests: no test asserted `compiled_query.reentry_plan` structure, leaving silent-regression risk for the plan field in slice 4.3b. Added `test_compile_cypher_records_reentry_plan_for_multi_whole_row_prefix` covering reentry_alias_name, scalar_only, alias output_names, and is_reentry_alias flag distribution. Stacking related Quality SUGGESTIONs: * Replaced "Slice 4.1+4.3" inline planning context with #989-anchored comment per the skill's "ephemeral reference leakage" rule. * Hoisted `dataclasses.fields` to module-top import, dropped the misleading "to avoid top-level shadowing" comment. * Narrowed `_iter_property_refs(node: Any)` to `node: object` and rewrote the docstring to enumerate the actual leaf types it cannot recurse into. Adversarial pass REJECTED a wave-2 BLOCKER claim of infinite recursion in `_iter_property_refs` — frozen dataclasses with PropertyRef leaves are acyclic by construction; the trailing fall-through is correct. Tests: 1095 passed (+1 new) on graphistry/tests/compute/gfql/cypher/. Refs #987 #989 #999. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…lice 4.3b) When a prefix `WITH a, x, y` projects multiple whole-row aliases, rewrite the prefix at compile time to project the non-source aliases as scalar property carries: `WITH a, x.id AS x__id, ...`. Trailing `<non_source>.<prop>` references rewrite to a property access on the reentry-alias's hidden column (`<reentry_alias>.<__cypher_reentry_<alias>__<prop>__>`). The existing scalar carry plumbing forwards the values onto the reentry-alias's row table; no runtime changes needed. Closes the previously-locked #1026 regression (multi-alias WITH + OPTIONAL MATCH) which now executes correctly instead of raising. Slice 4.3a's failfast tightens to bare-reference cases only — bare `<non_source>` carries (and IC3's multi-stage `WITH a, x, y` chain that re-projects through stages) remain out of scope for this slice and now point users at the still-pending follow-up work under #989. Implementation: * `_reentry_property_carry_name(alias, prop)` helper for the unwrapped carry alias used in the prefix WITH; the existing `_reentry_hidden_column_name` wraps it at runtime to the final hidden column name. * `_rewrite_multi_whole_row_prefix(prefix_stage, ...)` rewrites the prefix items: drops bare non-source aliases that have property refs downstream, inserts scalar projection per (alias, property) pair. * `_collect_non_source_alias_property_refs` returns (props_by_alias, bare_referenced) — the dict drives the prefix rewrite, bare_referenced gates the failfast. * `_rewrite_reentry_expr_to_hidden_properties` extended with optional `non_source_carried_props` arg; AST-level rewrite of `PropertyAccessExpr(Identifier(non_source), prop)` runs BEFORE bare-identifier substitution to avoid double-mangling. * `ReentryPlan.aliases` now records `CarriedAlias.carried_properties` for the rewritten non-source aliases (field re-introduced from slice 4.1, now used). Tests: 1096 passed (+1 net new positive — `_admits_non_source_alias_property_carry_through_reentry`, `_failfast_..._bare_referenced` adapted from prior failfast). The `test_issue_1026_multi_alias_with_optional_match_rejects_cleanly` flipped to `_carries_non_source_property` asserting correct execution. Refs #987 #989 #999 #1026. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…terminism comment, ORDER BY failfast test (#989) Wave 3 review of slice 4.3b (commit c6a9852) flagged two SUGGESTION-level items resolvable in a small follow-up: * Carry name `<alias>__<prop>` could collide with a user-projected scalar in the same prefix WITH (hypothetical, low-likelihood). Tightened to `__carry_<alias>__<prop>__` so the namespace can't overlap with any unquoted user identifier. * Sort-determinism comment on the property-tuple ordering inside `_rewrite_multi_whole_row_prefix` so future readers know the order is cosmetic and that downstream AST rewrites match by name. Plus a missing failfast-scope test: * `test_string_cypher_failfast_rejects_multi_whole_row_prefix_when_non_source_alias_is_bare_referenced_in_order_by` exercises the `query.order_by` branch of `_collect_non_source_alias_property_refs`. Adversarial pass on wave 3 confirmed the remaining flagged items (double-rewrite safety, bare-vs-property filter logic, end-to-end carry correctness, flipped #1026 test) are all CORRECT. Tests: 1097 passed (+1 new failfast scope test), no regressions. Refs #989. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
…am WITH stages (#989 slice 4.3c) Slice 4.3c fold-up: when a downstream WITH stage forwards carried whole-row aliases as bare items (`WITH a, x, y, collect(...)`), drop them at compile time. Their properties are already carried onto the reentry-alias's row table as hidden columns; the bare items are pure forwarding noise. Without this, the slice 4.3a/4.3b bare-reference failfast fires on IC3-shaped multi-stage chains where each `WITH` re-projects the carried aliases for downstream use. With this drop, those queries pass the failfast and proceed to the property-access rewrite layer. Note: this slice does NOT close the full IC3 lane. Multi-stage chains that span more than one reentry MATCH (`MATCH (a)-[:KNOWS]-(friend) ... MATCH (friend)<-[:HAS_CREATOR]-(message)`) still fail at a deeper runtime boundary — the hidden columns carrying `x.id`/`y.id` live on `a`'s row table and don't follow forward to `friend`'s row table after the first reentry's join. That cross-reentry carry-forward is the remaining slice 4.3d work tracked under #989. Implementation: * `_drop_bare_alias_items_from_stage(stage, aliases, *, identifier_re)`: pure helper that drops bare-identifier projection items whose name is in the carried set. Items with explicit aliases (`x AS xref`) are preserved — those are uses, not forwards. * `_rewrite_multi_whole_row_prefix(...)` extended to also return the rewritten downstream WITH stages tail. The bare-ref scanner now runs on the cleaned tail so forwarding patterns don't false-positive. * Caller threads the rewritten tail back into `query.with_stages` before the prefix compile + suffix construction. Tests: 1097 passed. The bare-ref failfast test updated to use `WHERE x = b` (a true USE) since `WITH b, x` is now correctly admitted as a forwarding pattern. Refs #989. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
lmeyerov
marked this pull request as ready for review
May 3, 2026 05:19
lmeyerov
commented
May 3, 2026
| if [[ -n "${tck_sha_override}" ]]; then | ||
| tck_ref="${tck_ref}+sha-override" | ||
| tck_sha="${tck_sha_override}" | ||
| fi |
…arry slices Adds an Internal entry under [Development] covering PR #1248's contributions beyond the #1071 lift already on master: - ReentryPlan + CarriedAlias dataclass IR (#987 step 1 / #989 slice 4.1) - Multi-whole-row admit when non-source aliases unused (slice 4.3a) - Compile-time property carry rewrite via __carry_<alias>__<prop>__ hidden columns, closing the multi-alias case of the #1026 regression-lock (slice 4.3b) - Bare-item drop in downstream forwarding WITH stages (slice 4.3c) Cross-reentry-boundary carry forwarding (IC3 / #999 slice 4.3d) is called out as remaining follow-up. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
This was referenced May 3, 2026
Closed
Closed
Closed
Cypher/GFQL: support multi-MATCH-source row projection in RETURN/WITH (IC5 residual from #999)
#1257
Closed
This was referenced May 4, 2026
lmeyerov
added a commit
that referenced
this pull request
May 4, 2026
Lifts the failfast at the trailing-MATCH-first-alias check in ``_compile_bounded_reentry_query`` for the free-form case (LDBC SNB IC3 endpoint): when the trailing MATCH binds aliases none of which is in the prefix WITH's carried whole-row set, treat every carried alias as non-source and use the trailing MATCH's first alias as the carrier label for downstream rewrites. The existing ``_collect_non_source_alias_property_refs`` / ``_rewrite_reentry_expr_to_hidden_properties`` machinery (slice 4.3a/b from #1248) materializes carried-alias property references as hidden columns; with this change the same machinery covers free-form cases by passing all whole-row aliases as ``non_source_alias_names``. The ``ReentryPlan`` constructed for free-form sets ``free_form=True`` (no ``CarriedAlias`` entry has ``is_reentry_alias=True``) so the runtime can branch into a broadcast path. The runtime change lands in a follow-up commit; until then, free-form queries compile but return wrong rows because the existing ``_compiled_query_reentry_state`` still seeds from the wrong alias's node ids — the regression is gated by the failfast tests (which fail intentionally at this commit and are retargeted alongside the runtime commit). Also tightens the surviving guard from ``first_alias is None or first_alias != reentry_alias`` to ``first_alias is None`` since the admit branch now sets ``reentry_alias = first_alias`` for the previously-rejected case. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
lmeyerov
added a commit
that referenced
this pull request
May 4, 2026
#1263) Lands the runtime half of the #1263 conservative admit: * New ``_compiled_query_freeform_reentry_state`` in ``gfql_unified.py`` branches on ``ReentryPlan.free_form``. For a single-row prefix WITH it broadcasts every ``__cypher_reentry_*`` hidden column from the prefix row onto every base node and returns ``start_nodes=None``, so the trailing MATCH runs as a regular MATCH and inherits the carried values via the row pipeline. Multi-row prefix raises a clear failfast pointing at the multi-row free-form follow-up slice. * Compile gate added in ``_compile_bounded_reentry_query``: when ``free_form`` is True AND the trailing scope references any carried alias property (e.g. ``WHERE country.id IN [x.id, y.id]`` in literal IC3), raise a scoped ``#1263`` failfast pointing at the rewrite-order refactor follow-up. The double-rewrite that emerges from composing ``_demote_secondary_whole_row_aliases`` (#1071) with ``_rewrite_reentry_expr_to_hidden_properties`` (#1248) under free-form is deferred to its own focused slice. End-to-end: * Simple free-form (``MATCH (a) WITH a MATCH (c)-[:T]->(d) RETURN ...``) compiles + executes vectorized on pandas. * IC3-shape with carried-property WHERE rejects with a scoped #1263 failfast (clear error pointing at the follow-up). Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
lmeyerov
added a commit
that referenced
this pull request
May 4, 2026
Lifts the failfast at the trailing-MATCH-first-alias check in ``_compile_bounded_reentry_query`` for the free-form case (LDBC SNB IC3 endpoint): when the trailing MATCH binds aliases none of which is in the prefix WITH's carried whole-row set, treat every carried alias as non-source and use the trailing MATCH's first alias as the carrier label for downstream rewrites. The existing ``_collect_non_source_alias_property_refs`` / ``_rewrite_reentry_expr_to_hidden_properties`` machinery (slice 4.3a/b from #1248) materializes carried-alias property references as hidden columns; with this change the same machinery covers free-form cases by passing all whole-row aliases as ``non_source_alias_names``. The ``ReentryPlan`` constructed for free-form sets ``free_form=True`` (no ``CarriedAlias`` entry has ``is_reentry_alias=True``) so the runtime can branch into a broadcast path. The runtime change lands in a follow-up commit; until then, free-form queries compile but return wrong rows because the existing ``_compiled_query_reentry_state`` still seeds from the wrong alias's node ids — the regression is gated by the failfast tests (which fail intentionally at this commit and are retargeted alongside the runtime commit). Also tightens the surviving guard from ``first_alias is None or first_alias != reentry_alias`` to ``first_alias is None`` since the admit branch now sets ``reentry_alias = first_alias`` for the previously-rejected case. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
lmeyerov
added a commit
that referenced
this pull request
May 4, 2026
#1263) Lands the runtime half of the #1263 conservative admit: * New ``_compiled_query_freeform_reentry_state`` in ``gfql_unified.py`` branches on ``ReentryPlan.free_form``. For a single-row prefix WITH it broadcasts every ``__cypher_reentry_*`` hidden column from the prefix row onto every base node and returns ``start_nodes=None``, so the trailing MATCH runs as a regular MATCH and inherits the carried values via the row pipeline. Multi-row prefix raises a clear failfast pointing at the multi-row free-form follow-up slice. * Compile gate added in ``_compile_bounded_reentry_query``: when ``free_form`` is True AND the trailing scope references any carried alias property (e.g. ``WHERE country.id IN [x.id, y.id]`` in literal IC3), raise a scoped ``#1263`` failfast pointing at the rewrite-order refactor follow-up. The double-rewrite that emerges from composing ``_demote_secondary_whole_row_aliases`` (#1071) with ``_rewrite_reentry_expr_to_hidden_properties`` (#1248) under free-form is deferred to its own focused slice. End-to-end: * Simple free-form (``MATCH (a) WITH a MATCH (c)-[:T]->(d) RETURN ...``) compiles + executes vectorized on pandas. * IC3-shape with carried-property WHERE rejects with a scoped #1263 failfast (clear error pointing at the follow-up). Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
lmeyerov
added a commit
that referenced
this pull request
May 4, 2026
* feat(cypher/ir): add ReentryPlan.free_form marker for intermediate MATCH (#1263) Adds a `free_form: bool = False` field to `ReentryPlan` so the runtime can distinguish the LDBC SNB IC3 free-form intermediate MATCH shape from the existing whole-row and scalar-only prefix shapes. When the trailing MATCH after a prefix `WITH` introduces aliases none of which is in the carried set, no carried alias anchors the seed pattern. The runtime needs to broadcast carried columns onto the base node table (single-prefix-row) or fall back to per-row union (multi-prefix-row) so the trailing MATCH cross-joins implicitly via the row pipeline. Default value preserves existing whole-row and scalar-only behavior. Compile + runtime branches that consume this marker land in subsequent commits. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(cypher): admit free-form intermediate MATCH at compile (#1263) Lifts the failfast at the trailing-MATCH-first-alias check in ``_compile_bounded_reentry_query`` for the free-form case (LDBC SNB IC3 endpoint): when the trailing MATCH binds aliases none of which is in the prefix WITH's carried whole-row set, treat every carried alias as non-source and use the trailing MATCH's first alias as the carrier label for downstream rewrites. The existing ``_collect_non_source_alias_property_refs`` / ``_rewrite_reentry_expr_to_hidden_properties`` machinery (slice 4.3a/b from #1248) materializes carried-alias property references as hidden columns; with this change the same machinery covers free-form cases by passing all whole-row aliases as ``non_source_alias_names``. The ``ReentryPlan`` constructed for free-form sets ``free_form=True`` (no ``CarriedAlias`` entry has ``is_reentry_alias=True``) so the runtime can branch into a broadcast path. The runtime change lands in a follow-up commit; until then, free-form queries compile but return wrong rows because the existing ``_compiled_query_reentry_state`` still seeds from the wrong alias's node ids — the regression is gated by the failfast tests (which fail intentionally at this commit and are retargeted alongside the runtime commit). Also tightens the surviving guard from ``first_alias is None or first_alias != reentry_alias`` to ``first_alias is None`` since the admit branch now sets ``reentry_alias = first_alias`` for the previously-rejected case. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(cypher): runtime broadcast for simple free-form + scoped failfast (#1263) Lands the runtime half of the #1263 conservative admit: * New ``_compiled_query_freeform_reentry_state`` in ``gfql_unified.py`` branches on ``ReentryPlan.free_form``. For a single-row prefix WITH it broadcasts every ``__cypher_reentry_*`` hidden column from the prefix row onto every base node and returns ``start_nodes=None``, so the trailing MATCH runs as a regular MATCH and inherits the carried values via the row pipeline. Multi-row prefix raises a clear failfast pointing at the multi-row free-form follow-up slice. * Compile gate added in ``_compile_bounded_reentry_query``: when ``free_form`` is True AND the trailing scope references any carried alias property (e.g. ``WHERE country.id IN [x.id, y.id]`` in literal IC3), raise a scoped ``#1263`` failfast pointing at the rewrite-order refactor follow-up. The double-rewrite that emerges from composing ``_demote_secondary_whole_row_aliases`` (#1071) with ``_rewrite_reentry_expr_to_hidden_properties`` (#1248) under free-form is deferred to its own focused slice. End-to-end: * Simple free-form (``MATCH (a) WITH a MATCH (c)-[:T]->(d) RETURN ...``) compiles + executes vectorized on pandas. * IC3-shape with carried-property WHERE rejects with a scoped #1263 failfast (clear error pointing at the follow-up). Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): retarget free-form failfast tests for #1263 conservative admit Two #1263 regression-locks added in #1270 expected the carried-alias gate to fire for free-form intermediate MATCH. The conservative admit landed in the preceding commits flips the simple shape from rejection to positive execution and tightens the rejection scope to the carried-property variant: * ``test_string_cypher_failfast_rejects_intermediate_reentry_match_with_no_carried_source`` → renamed to ``..._with_carried_property_in_trailing_where`` and pinned to the new scoped #1263 failfast (carried-alias property in trailing scope). * ``test_string_cypher_failfast_rejects_simple_freeform_intermediate_reentry_match`` → renamed to ``test_string_cypher_executes_simple_freeform_intermediate_reentry_match`` and converted to positive: asserts the trailing MATCH binds correctly against the broadcast carried row table. * New ``..._on_cudf_when_available`` mirror with ``pytest.importorskip("cudf")`` for engine parity per coordinator def-of-done. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): add multi-prefix-row free-form failfast regression (#1263) Wave 1 review (single CONFIRMED IMPORTANT): the runtime failfast at gfql_unified.py for multi-prefix-row free-form admit was untested. Adds a positive regression-lock that builds a 2-row prefix and asserts the scoped ``single-row prefix WITH only`` failfast wording. Refs #1263 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * docs(changelog): #1263 simple free-form intermediate MATCH admit Records the conservative #1263 close: simple free-form admit + scoped failfast for the carried-property-in-trailing-WHERE variant + runtime broadcast helper + retargeted regression tests + three TCK admits. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): Wave 2 amplification — multi-carried + empty prefix free-form (#1263) Wave 2 review per agents/skills/review/SKILL.md confirmed convergence (two consecutive non-significant-advance waves) and surfaced two amplification SUGGESTIONs that the reviewers verified produce correct behavior. Locking the contract: * test_string_cypher_executes_freeform_intermediate_reentry_match_with_multi_carried_aliases — covers `WITH a, b MATCH (c)-[:T]->(d)` shape (multi-carried-aliases admit). * test_string_cypher_executes_freeform_intermediate_reentry_match_with_empty_prefix — covers `_compiled_query_freeform_reentry_state`'s empty-prefix early-return path. Refs #1263 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
lmeyerov
added a commit
that referenced
this pull request
May 4, 2026
* feat(cypher/ir): add ReentryPlan.free_form marker for intermediate MATCH (#1263) Adds a `free_form: bool = False` field to `ReentryPlan` so the runtime can distinguish the LDBC SNB IC3 free-form intermediate MATCH shape from the existing whole-row and scalar-only prefix shapes. When the trailing MATCH after a prefix `WITH` introduces aliases none of which is in the carried set, no carried alias anchors the seed pattern. The runtime needs to broadcast carried columns onto the base node table (single-prefix-row) or fall back to per-row union (multi-prefix-row) so the trailing MATCH cross-joins implicitly via the row pipeline. Default value preserves existing whole-row and scalar-only behavior. Compile + runtime branches that consume this marker land in subsequent commits. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(cypher): admit free-form intermediate MATCH at compile (#1263) Lifts the failfast at the trailing-MATCH-first-alias check in ``_compile_bounded_reentry_query`` for the free-form case (LDBC SNB IC3 endpoint): when the trailing MATCH binds aliases none of which is in the prefix WITH's carried whole-row set, treat every carried alias as non-source and use the trailing MATCH's first alias as the carrier label for downstream rewrites. The existing ``_collect_non_source_alias_property_refs`` / ``_rewrite_reentry_expr_to_hidden_properties`` machinery (slice 4.3a/b from #1248) materializes carried-alias property references as hidden columns; with this change the same machinery covers free-form cases by passing all whole-row aliases as ``non_source_alias_names``. The ``ReentryPlan`` constructed for free-form sets ``free_form=True`` (no ``CarriedAlias`` entry has ``is_reentry_alias=True``) so the runtime can branch into a broadcast path. The runtime change lands in a follow-up commit; until then, free-form queries compile but return wrong rows because the existing ``_compiled_query_reentry_state`` still seeds from the wrong alias's node ids — the regression is gated by the failfast tests (which fail intentionally at this commit and are retargeted alongside the runtime commit). Also tightens the surviving guard from ``first_alias is None or first_alias != reentry_alias`` to ``first_alias is None`` since the admit branch now sets ``reentry_alias = first_alias`` for the previously-rejected case. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(cypher): runtime broadcast for simple free-form + scoped failfast (#1263) Lands the runtime half of the #1263 conservative admit: * New ``_compiled_query_freeform_reentry_state`` in ``gfql_unified.py`` branches on ``ReentryPlan.free_form``. For a single-row prefix WITH it broadcasts every ``__cypher_reentry_*`` hidden column from the prefix row onto every base node and returns ``start_nodes=None``, so the trailing MATCH runs as a regular MATCH and inherits the carried values via the row pipeline. Multi-row prefix raises a clear failfast pointing at the multi-row free-form follow-up slice. * Compile gate added in ``_compile_bounded_reentry_query``: when ``free_form`` is True AND the trailing scope references any carried alias property (e.g. ``WHERE country.id IN [x.id, y.id]`` in literal IC3), raise a scoped ``#1263`` failfast pointing at the rewrite-order refactor follow-up. The double-rewrite that emerges from composing ``_demote_secondary_whole_row_aliases`` (#1071) with ``_rewrite_reentry_expr_to_hidden_properties`` (#1248) under free-form is deferred to its own focused slice. End-to-end: * Simple free-form (``MATCH (a) WITH a MATCH (c)-[:T]->(d) RETURN ...``) compiles + executes vectorized on pandas. * IC3-shape with carried-property WHERE rejects with a scoped #1263 failfast (clear error pointing at the follow-up). Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): retarget free-form failfast tests for #1263 conservative admit Two #1263 regression-locks added in #1270 expected the carried-alias gate to fire for free-form intermediate MATCH. The conservative admit landed in the preceding commits flips the simple shape from rejection to positive execution and tightens the rejection scope to the carried-property variant: * ``test_string_cypher_failfast_rejects_intermediate_reentry_match_with_no_carried_source`` → renamed to ``..._with_carried_property_in_trailing_where`` and pinned to the new scoped #1263 failfast (carried-alias property in trailing scope). * ``test_string_cypher_failfast_rejects_simple_freeform_intermediate_reentry_match`` → renamed to ``test_string_cypher_executes_simple_freeform_intermediate_reentry_match`` and converted to positive: asserts the trailing MATCH binds correctly against the broadcast carried row table. * New ``..._on_cudf_when_available`` mirror with ``pytest.importorskip("cudf")`` for engine parity per coordinator def-of-done. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): add multi-prefix-row free-form failfast regression (#1263) Wave 1 review (single CONFIRMED IMPORTANT): the runtime failfast at gfql_unified.py for multi-prefix-row free-form admit was untested. Adds a positive regression-lock that builds a 2-row prefix and asserts the scoped ``single-row prefix WITH only`` failfast wording. Refs #1263 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * docs(changelog): #1263 simple free-form intermediate MATCH admit Records the conservative #1263 close: simple free-form admit + scoped failfast for the carried-property-in-trailing-WHERE variant + runtime broadcast helper + retargeted regression tests + three TCK admits. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): Wave 2 amplification — multi-carried + empty prefix free-form (#1263) Wave 2 review per agents/skills/review/SKILL.md confirmed convergence (two consecutive non-significant-advance waves) and surfaced two amplification SUGGESTIONs that the reviewers verified produce correct behavior. Locking the contract: * test_string_cypher_executes_freeform_intermediate_reentry_match_with_multi_carried_aliases — covers `WITH a, b MATCH (c)-[:T]->(d)` shape (multi-carried-aliases admit). * test_string_cypher_executes_freeform_intermediate_reentry_match_with_empty_prefix — covers `_compiled_query_freeform_reentry_state`'s empty-prefix early-return path. Refs #1263 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
lmeyerov
added a commit
that referenced
this pull request
May 4, 2026
…is defaults (#1280) * phase1: validators, deep ring axis checks, and persist axis defaults * fix: satisfy mypy for dataset_id URL refresh helper * fix: make validate_settings typing compatible with python 3.8 * refactor: centralize axis field constants and typed dict contracts * docs(changelog): record settings/axis validation + persist defaults work * refactor(validate): centralize ring axis payload validators * types(validate): add named url params/react settings aliases * types(url_params): propagate named URLParamsDict across APIs * feat(cypher): admit simple free-form intermediate MATCH (#1263) (#1279) * feat(cypher/ir): add ReentryPlan.free_form marker for intermediate MATCH (#1263) Adds a `free_form: bool = False` field to `ReentryPlan` so the runtime can distinguish the LDBC SNB IC3 free-form intermediate MATCH shape from the existing whole-row and scalar-only prefix shapes. When the trailing MATCH after a prefix `WITH` introduces aliases none of which is in the carried set, no carried alias anchors the seed pattern. The runtime needs to broadcast carried columns onto the base node table (single-prefix-row) or fall back to per-row union (multi-prefix-row) so the trailing MATCH cross-joins implicitly via the row pipeline. Default value preserves existing whole-row and scalar-only behavior. Compile + runtime branches that consume this marker land in subsequent commits. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(cypher): admit free-form intermediate MATCH at compile (#1263) Lifts the failfast at the trailing-MATCH-first-alias check in ``_compile_bounded_reentry_query`` for the free-form case (LDBC SNB IC3 endpoint): when the trailing MATCH binds aliases none of which is in the prefix WITH's carried whole-row set, treat every carried alias as non-source and use the trailing MATCH's first alias as the carrier label for downstream rewrites. The existing ``_collect_non_source_alias_property_refs`` / ``_rewrite_reentry_expr_to_hidden_properties`` machinery (slice 4.3a/b from #1248) materializes carried-alias property references as hidden columns; with this change the same machinery covers free-form cases by passing all whole-row aliases as ``non_source_alias_names``. The ``ReentryPlan`` constructed for free-form sets ``free_form=True`` (no ``CarriedAlias`` entry has ``is_reentry_alias=True``) so the runtime can branch into a broadcast path. The runtime change lands in a follow-up commit; until then, free-form queries compile but return wrong rows because the existing ``_compiled_query_reentry_state`` still seeds from the wrong alias's node ids — the regression is gated by the failfast tests (which fail intentionally at this commit and are retargeted alongside the runtime commit). Also tightens the surviving guard from ``first_alias is None or first_alias != reentry_alias`` to ``first_alias is None`` since the admit branch now sets ``reentry_alias = first_alias`` for the previously-rejected case. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * feat(cypher): runtime broadcast for simple free-form + scoped failfast (#1263) Lands the runtime half of the #1263 conservative admit: * New ``_compiled_query_freeform_reentry_state`` in ``gfql_unified.py`` branches on ``ReentryPlan.free_form``. For a single-row prefix WITH it broadcasts every ``__cypher_reentry_*`` hidden column from the prefix row onto every base node and returns ``start_nodes=None``, so the trailing MATCH runs as a regular MATCH and inherits the carried values via the row pipeline. Multi-row prefix raises a clear failfast pointing at the multi-row free-form follow-up slice. * Compile gate added in ``_compile_bounded_reentry_query``: when ``free_form`` is True AND the trailing scope references any carried alias property (e.g. ``WHERE country.id IN [x.id, y.id]`` in literal IC3), raise a scoped ``#1263`` failfast pointing at the rewrite-order refactor follow-up. The double-rewrite that emerges from composing ``_demote_secondary_whole_row_aliases`` (#1071) with ``_rewrite_reentry_expr_to_hidden_properties`` (#1248) under free-form is deferred to its own focused slice. End-to-end: * Simple free-form (``MATCH (a) WITH a MATCH (c)-[:T]->(d) RETURN ...``) compiles + executes vectorized on pandas. * IC3-shape with carried-property WHERE rejects with a scoped #1263 failfast (clear error pointing at the follow-up). Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): retarget free-form failfast tests for #1263 conservative admit Two #1263 regression-locks added in #1270 expected the carried-alias gate to fire for free-form intermediate MATCH. The conservative admit landed in the preceding commits flips the simple shape from rejection to positive execution and tightens the rejection scope to the carried-property variant: * ``test_string_cypher_failfast_rejects_intermediate_reentry_match_with_no_carried_source`` → renamed to ``..._with_carried_property_in_trailing_where`` and pinned to the new scoped #1263 failfast (carried-alias property in trailing scope). * ``test_string_cypher_failfast_rejects_simple_freeform_intermediate_reentry_match`` → renamed to ``test_string_cypher_executes_simple_freeform_intermediate_reentry_match`` and converted to positive: asserts the trailing MATCH binds correctly against the broadcast carried row table. * New ``..._on_cudf_when_available`` mirror with ``pytest.importorskip("cudf")`` for engine parity per coordinator def-of-done. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): add multi-prefix-row free-form failfast regression (#1263) Wave 1 review (single CONFIRMED IMPORTANT): the runtime failfast at gfql_unified.py for multi-prefix-row free-form admit was untested. Adds a positive regression-lock that builds a 2-row prefix and asserts the scoped ``single-row prefix WITH only`` failfast wording. Refs #1263 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * docs(changelog): #1263 simple free-form intermediate MATCH admit Records the conservative #1263 close: simple free-form admit + scoped failfast for the carried-property-in-trailing-WHERE variant + runtime broadcast helper + retargeted regression tests + three TCK admits. Refs #1263 #999 #989 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> * test(cypher): Wave 2 amplification — multi-carried + empty prefix free-form (#1263) Wave 2 review per agents/skills/review/SKILL.md confirmed convergence (two consecutive non-significant-advance waves) and surfaced two amplification SUGGESTIONs that the reviewers verified produce correct behavior. Locking the contract: * test_string_cypher_executes_freeform_intermediate_reentry_match_with_multi_carried_aliases — covers `WITH a, b MATCH (c)-[:T]->(d)` shape (multi-carried-aliases admit). * test_string_cypher_executes_freeform_intermediate_reentry_match_with_empty_prefix — covers `_compiled_query_freeform_reentry_state`'s empty-prefix early-return path. Refs #1263 Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
5 of 6 tasks
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.
Summary
Foundation work toward
#989row-carrier IR. Two slices on top ofmaster@6891d39e4:Slice 4.1 — Introduce
ReentryPlan+CarriedAliasdataclasses (graphistry/compute/gfql/cypher/reentry_plan.py). Addreentry_plan: Optional[ReentryPlan]toCompiledCypherExecutionExtrasand propagate through_execution_extras_with/_map_terminal_reentry_query/_attach_graph_context. No behavior change — read sites still consumescalar_reentry_alias/scalar_reentry_columns.Slice 4.3a — Lift the
len(whole_row_columns) != 1gate atlowering.py:7207and the matching runtime gate atgfql_unified.py:1220. Multi-whole-row prefixWITH(WITH a, x, y) now compiles when only the trailing-MATCH source alias is referenced downstream. For the IC3 case (non-source whole-row alias referenced in trailingWHERE/RETURN), emit an actionable failfast pointing to#989for the slice 4.3b follow-up that lands the actual property carry.What this PR does and does not close
#987ReentryPlan handshake cleanup — partially addressed (Step 1 dataclass + scalar/whole-row migration to plan field). Steps 2–5 (replace hidden-column protocol, move runtime stitching, split lowering.py) remain follow-up.#989row-carrier IR — partial. The plan dataclass and the multi-whole-row compile path are in place; the property carry for non-source aliases (slice 4.3b) is the remaining architectural exit gate.#999multi-stage MATCH/WITH/MATCH — not yet closed. IC3 still hits the new failfast (correctly: it referencesx.id/y.idin trailing). IC5 still hits the multi-MATCH-source row projection gate atlowering.py:1914(Slice 2 follow-up).The win in this PR: actionable diagnostics and a reusable plan contract for the rest of the row-carrier work.
Scope rationale
See
plans/989-reentryplan-multi-alias-carry/{plan.md,design/current-shape.md,design/reentry-plan.md}(gitignored) for the full design. Short version: the parser-surface "Path A" referenced in#999was empty — both literal IC3 and IC5 fall through to row-lowering invariants. Slice 4.3a is the smallest meaningful step that establishes the plan contract without writing throwaway hidden-column code.Test plan
pytest -q graphistry/tests/compute/gfql/cypher/— 1093 passed (+2 new), 66 skipped, 15 xfailedtest_string_cypher_admits_multi_whole_row_prefix_when_non_source_aliases_are_unusedtest_string_cypher_failfast_rejects_multi_whole_row_prefix_when_non_source_alias_is_referencedFollow-up plan
_reentry_hidden_property_column(alias, prop), scanner that returnsDict[alias, Set[prop]], AST-level rewriter forPropertyAccessExpr(<non_source>, prop)→PropertyAccessExpr(<reentry_alias>, hidden_name), runtime extension of_compiled_query_reentry_stateto merge hidden columns from each non-source alias's meta entry. Closes IC3 lane of#999.lowering.py:1914). Larger scope; separate PR under#989.#987Steps 2–5 (extract reentry runtime toreentry_runtime.py, remove the legacyscalar_reentry_*fields after consumers migrate tocompiled_query.reentry_plan).Refs
#987#989#999.