Fix relationship-row aggregate multiplicity for Cypher collect/count/sum (#1343) - #1348

Rebased onto master after #1348 (#1343 aggregate fix) landed; the
narrow-shape flatten wiring at compile_cypher_query adds 12 lines to
lowering.py, so 8688 → 8700.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

Rebased onto master after #1348 (#1343 aggregate fix) landed; the
narrow-shape flatten wiring at compile_cypher_query adds 12 lines to
lowering.py, so 8688 → 8700.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

…1341) (#1351)

* feat(cypher): admit carried-endpoint rebind via single-MATCH flatten (#1341)

Flatten the narrow IC1 reentry shape — single prefix MATCH, single pure
bare-alias WITH, single trailing MATCH whose patterns reference only
carried aliases AND add structural edges — into a comma-separated
single-MATCH clause that the existing two-endpoint shortestPath /
shared-alias paths handle directly. The blanket reject at
``lowering.py`` is preserved for residual non-admitted shapes.

Repurposed the prior failfast test to a positive admit test plus a
no-back-edge empty-result test, and added a dedicated IC1
``shortestPath((p)-[:KNOWS*1..3]-(friend))`` regression with cuDF parity.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(cypher): cover flatten disqualification branches + harden OPTIONAL prefix (#1341)

Address review wave 1:

- Disqualify ``OPTIONAL MATCH`` prefix in ``flatten_carried_endpoint_rebind``
  to keep the merged single MATCH from over-broadening optional scope onto
  trailing-derived patterns.
- Comment the ``parser.py`` invariant that mirrors top-level WHERE into
  ``match_clauses[-1].where`` so the inline-WHERE check covers both shapes.
- Add ``test_flatten_carried_endpoint_rebind.py`` — 13 direct unit tests
  anchoring each early-return branch (DISTINCT, alias rename, ORDER BY,
  LIMIT, fresh trailing alias, OPTIONAL prefix/trailing, multi-WITH,
  reentry WHERE, no-edge trailing, no-reentry baseline) plus the IC1 +
  simple rebind admit cases.
- Note the ``MatchClause.pattern_alias_kinds`` default-tuple invariant in
  ``_normalized_kinds`` so a future ``PathPatternKind`` rebase doesn't
  silently mis-fill.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(cypher): flatten requires carried set == prefix-bound aliases (#1341)

Wave-2 review surfaced a scope-leak: when WITH dropped a prefix-bound
alias and post-WITH RETURN/ORDER BY referenced it, the existing reentry
path emitted a clean "Unknown Cypher alias" scope error, but flatten
silently re-admitted the merged single MATCH where the dropped alias
remained in scope. Tighten the flatten check from ``carried.issubset``
to ``carried == prefix_aliases`` so partial-carry shapes fall through to
the reentry path's correct rejection.

Also clarify the defensive ``trailing_match.where`` branch (parser routes
post-WITH WHEREs to ``reentry_wheres``; the check guards AST-built
inputs) and tighten the ``reentry_where_present`` test docstring.

Adds direct regression ``test_flatten_disqualifies_partial_carry_drops_prefix_alias``.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* chore(ci): bump cypher surface-guard baseline for #1341 wiring

Rebased onto master after #1348 (#1343 aggregate fix) landed; the
narrow-shape flatten wiring at compile_cypher_query adds 12 lines to
lowering.py, so 8688 → 8700.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs+test(cypher): note flatten recursion termination + lock prefix-WHERE preservation (#1341)

Wave 3 cleanup:
- Comment recursion termination invariant at ``compile_cypher_query``
  (flatten output has empty ``reentry_matches``; the recursion cannot
  re-enter the flatten branch).
- Add ``test_flatten_preserves_prefix_match_where_on_merged_match``: asserts
  the prefix MatchClause's WhereClause object is preserved by-reference on
  the merged single MATCH (also covers the parser's top-level mirror onto
  ``match_clauses[-1].where``).
- Tighten ``_parse`` typing in the unit suite so the AST narrowing is
  explicit.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(cypher): include relationship-variable aliases in flatten equality (#1341)

Wave-4 review surfaced a relationship-variable scope leak: when prefix
bound a named relationship variable (``[r:R]``) and WITH dropped it,
post-WITH RETURN/ORDER BY references to ``r.<prop>`` would land in scope
under the merged single MATCH but produce a clean scope error on the
existing reentry path. Direct repro:

    MATCH (a:A)-[r:R]->(b:B) WITH a, b MATCH (b)-[:S]->(a) RETURN r.weight

Flatten silently admitted and returned ``r.weight``; the existing reentry
path correctly rejects.

Fix: extend ``_node_aliases`` with ``_all_pattern_aliases`` (nodes +
relationships) and key the equality check off the union, so dropping a
named relationship variable forces fall-through to the reentry-path
rejection.

Adds two regression tests:
- ``test_flatten_disqualifies_drops_relationship_variable``
- ``test_flatten_admits_when_relationship_variable_is_carried``

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* fix(cypher): include prefix path aliases in flatten equality (#1341)

Wave-5 review surfaced a defense-in-depth gap: ``MatchClause.pattern_aliases``
(path aliases like ``MATCH path = (a)-->(b)``) were not part of the
prefix-alias set, so a query like

    MATCH path = (a:A)-[:R]->(b:B) WITH a, b MATCH (b)-[:S]->(a) RETURN length(path)

passed the carry-set equality even though WITH dropped ``path``. No
user-visible leak today (the row pipeline rejects ``length(path)`` /
``nodes(path)`` etc.), but a future row-pipeline path-function expansion
would silently re-admit ``path`` into post-WITH scope through the merged
single MATCH.

Tighten the equality check to union path aliases from ``pattern_aliases``
into the prefix-alias set. Adds ``test_flatten_disqualifies_drops_prefix_path_alias``.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(cypher): round-001 amplification — lock-in matrix + admit-side oracle (#1341)

Test-amplification skill round-001. Five wave reviews on PR #1351 found 4
bugs all of one taxonomy class: carry-equality boundary scope-leaks via
missing alias kinds and missing semantic disqualifications. Round-001
locks in the equivalence class so future drift is caught at test time
rather than via another review wave.

New tests:
- ``test_flatten_alias_kind_enumeration_locks_in_with_ast`` —
  introspection lock-in: enumerates ``typing.get_args(PatternElement)`` and
  ``dataclasses.fields(MatchClause)``; asserts ``_all_pattern_aliases``
  collects each variable-bearing pattern subtype's ``variable``, and that
  the ``MatchClause.pattern_aliases`` field is observed in the prefix-
  alias union via a parsed admit/disqualify probe.
- ``test_flatten_disqualifies_partial_carry_for_each_alias_kind`` —
  parametrized partial-carry sweep over (node, rel, path) alias kinds.
- ``test_flatten_disqualifies_optional_match_combinations`` —
  parametrized OPTIONAL matrix asserting only (F,F) admits.
- ``test_flatten_disqualifies_both_inline_wheres_via_built_ast`` —
  hand-built AST input that reaches the otherwise-unreachable defensive
  ``prefix.where AND trailing.where`` branch.
- ``test_string_cypher_flatten_admit_matches_hand_flattened_oracle_ic1`` /
  ``..._simple_rebind`` — admit-side correctness oracle: result equality
  between WITH-form (flatten path) and the manually-flattened single-
  MATCH form.
- ``test_string_cypher_executes_with_match_reentry_relationship_variable_carried_on_cudf`` —
  cuDF parity for the rel-var-carried admit case (mirrors the existing
  IC1 cuDF parity test).

Total: 6 new test functions / ~9 parametrized cases; no source changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(cypher): cuDF-resilient assertions for round-001 amplification (#1341)

Validation on dgx-spark via rapids 25.02 + 26.02 (docker/test-rapids-
official-matrix.sh) surfaced two pre-existing cuDF row-pipeline issues
that round-001 lock-in tests exposed:

- ``length(path)`` projection on shortestPath returns object/string dtype
  on cuDF instead of numeric (pandas baseline = float64). Both flatten-on
  and hand-flattened single-MATCH forms produce identical (broken) cuDF
  output → not introduced by the new flatten module. Filed as #1354.
  Workaround: ``test_string_cypher_executes_ic1_shortest_path_with_carried_
  endpoints_rebound_on_cudf`` now extracts friend IDs and int-coerces dist
  values for comparison instead of asserting full dict equality.

- Shared-alias multi-pattern with relationship-property projection
  segfaults (exit 139) on cuDF — affects both the WITH-form (flatten path)
  and the hand-flattened single-MATCH form. C-level crash; out of scope
  for this PR. Filed as #1355. ``test_string_cypher_executes_with_match_
  reentry_relationship_variable_carried_on_cudf`` is ``@pytest.mark.skip``'d
  with the issue reference; the pandas equivalent (``test_flatten_admits_
  when_relationship_variable_is_carried``) still anchors the wave-4 fix.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(cypher): wave-7 amplification follow-ups (#1341)

Wave-7 review found two real test-quality gaps in the round-001
amplification additions:

- ``test_flatten_alias_kind_enumeration_locks_in_with_ast`` was vacuous
  on existing ``PatternElement`` subtypes. Both ``NodePattern`` and
  ``RelationshipPattern`` have required fields without defaults
  (``labels``, ``properties``, ``direction``, ``types``); the prior
  ``kwargs={}; break; continue`` skip caused the assertion to never
  execute against either subtype. Replace with a ``typing.get_origin``-
  driven filler that supplies empty tuples for ``Tuple[...]`` fields and
  the first allowed value for ``Literal`` fields, plus a final
  ``asserted_subtype`` guard so the test fails loudly if no
  variable-bearing subtype is exercised. Verified via sabotage probe:
  monkey-patching ``_all_pattern_aliases`` to ignore RelationshipPattern
  now fails the test with ``must collect variable from
  RelationshipPattern (got set())``.

- ``test_string_cypher_executes_ic1_shortest_path_with_carried_
  endpoints_rebound_on_cudf`` int-coerced ``r["dist"]`` directly, which
  would ``ValueError`` on ``'1.0'`` strings or ``TypeError`` on
  ``None``/``NaN`` if cuDF behavior evolves. Replace with
  ``int(float(r["dist"]))`` two-step coercion plus an explicit
  ``len(rows) == 3`` shape check. Cite #1354 by issue number in the
  comment.

Polish: link #1355 by issue number in the rel-var-rel-prop cuDF skip
reason (was generic "filed as a separate cuDF parity issue"), and note
why ``skip`` rather than ``xfail`` is the correct mechanism for a
SIGSEGV failure mode.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* test(cypher): wave-8 lock-in defensive — require both PatternElement subtypes (#1341)

Wave-8 review SUGGESTION: the lock-in test's ``"variable" not in
type_hints: continue`` skip would let the test pass vacuously if a
future refactor moves ``variable`` off both ``NodePattern`` and
``RelationshipPattern`` (e.g. into a base mixin). Add an explicit
``{"NodePattern", "RelationshipPattern"}.issubset(asserted_subtype_names)``
final guard so the lock-in always anchors against today's concrete
subtypes by name.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

* docs+test(cypher): wave-9 follow-ups — CHANGELOG, docstring drift, SKIP/non-bare tests (#1341)

Wave-9 review surfaced:

- IMPORTANT: missing CHANGELOG entry under ``## [Development] / ###
  Changed`` (PR converts a previously-rejected query shape into an
  admitted, executable shape; recent ``## [Development]`` peers all
  have entries). Added.

- Documentation drift in ``flatten.py`` module docstring claiming
  "fresh trailing aliases ... disqualify the pattern". Only fresh
  trailing *node* aliases disqualify; fresh trailing relationship and
  path aliases admit (they are legitimately in scope post-WITH).
  Tightened the docstring.

- Two parser-reachable disqualification branches lacked direct tests:
  ``WITH a, b SKIP 5`` and ``WITH a, b.id``. Added
  ``test_flatten_disqualifies_with_skip`` and
  ``test_flatten_disqualifies_non_bare_with_item``.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>