Skip to content

feat(gfql): polars whole-entity aggregation Cypher — HAS_label disambiguation + identity-key + key_prefixes (LDBC IC4 native)#1737

Merged
lmeyerov merged 9 commits into
masterfrom
feat/gfql-polars-rows-entity-groupby
Jul 20, 2026
Merged

feat(gfql): polars whole-entity aggregation Cypher — HAS_label disambiguation + identity-key + key_prefixes (LDBC IC4 native)#1737
lmeyerov merged 9 commits into
masterfrom
feat/gfql-polars-rows-entity-groupby

Conversation

@lmeyerov

Copy link
Copy Markdown
Contributor

What

Stack PR #5 (base: #1736#1734#1735 → master). Unlocks the polars rows/aggregation NIE class found by the 2026-07-19 LDBC cypher baseline: the official IC4 (new-topics) query now runs natively on polars, parity-exact — comma-MATCH, WITH DISTINCT, CASE projections, whole-entity sum aggregation, HAVING-style WHERE.

Three pandas-parity ports (all previously honest NIEs)

  1. HAS_ destination disambiguation (binding_rows_polars): instead of declining every labeled-destination pattern, apply pandas' _gfql_disambiguate_has_edge_destination_nodes rule — an unlabeled next-node op after a HAS_<Label>-typed edge narrows to that label ONLY when candidate node ids collide across labels (unique ids keep the unnarrowed domain, exactly like pandas).
  2. Whole-entity identity key: alias.__gfql_node_id__ (GFQL: avoid spurious entity-text stringification of returned entities (return structured/Arrow frames) #1650 grouping key) resolves to the bare alias id column — the polars bindings table intentionally omits pandas' join-residue columns; the bare alias column IS the identity key.
  3. group_by(key_prefixes=...): expand every <prefix>* row-table column into the key set exactly like pandas (functionally dependent on the identity key → group sizes unchanged).

Validation (dgx, 26.02-gfql-polars)

  • Harness (pyg-bench snb-interactive, SF0.1, official query texts): new-topics cypher on polars = ok, 127.8 ms, expected-rows parity (was NIE). Polars cypher coverage 7/20 → 8/20.
  • test_runtime_physical_cutover + polars conformance + logical planner: 184 passed.
  • New test_polars_rows_entity_groupby.py: pandas oracle + polars parity on a discriminating fixture (tag invalidated by a pre-window post). 2 passed.

Not in scope (named follow-ups)

Undirected bounded var-length -[:KNOWS*1..2]- (IC6/IC11 — immediate-backtrack avoidance port), zero-hop unbounded varlen (IS6, feat/issue-983 branch), shortestPath scalar backend (IC13).

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL

@lmeyerov

Copy link
Copy Markdown
Contributor Author

Self-review checkpoint (per .agents/skills/review discipline) caught two parity divergences in the initial HAS_label port vs pandas' actual call site (row/pipeline.py:3670): (1) pandas probes id duplicates on the reached candidate domain (candidate_source ∩ wavefront) — the initial port probed the full node table, which would over-narrow when ids collide globally but not among reached nodes; (2) pandas gates the narrowing to forward single-hop edges — the initial port applied it to all directions. Fixed: the polars twin now runs after the hop's state semi-join with identical gates, and re-semi-joins the state when it narrows. Two new engine-parametrized regression tests pin both branches against the pandas oracle (skip-narrowing on unique-reached; narrow on reached-collision). 186 suite + 6 file tests pass on dgx; IC4 harness parity re-confirmed.

@lmeyerov
lmeyerov force-pushed the fix/gfql-optional-match-polars-frames branch from da41fb1 to 5e6d307 Compare July 19, 2026 20:29
@lmeyerov
lmeyerov force-pushed the feat/gfql-polars-rows-entity-groupby branch from e3260ad to 3e9840a Compare July 19, 2026 20:29
@lmeyerov
lmeyerov marked this pull request as ready for review July 19, 2026 20:32
@lmeyerov
lmeyerov force-pushed the fix/gfql-optional-match-polars-frames branch from 5e6d307 to e513b8c Compare July 19, 2026 21:14
lmeyerov added a commit that referenced this pull request Jul 19, 2026
…-row groupby test (review)

Review feedback on #1737:
- op.params is wire-format Dict[str, Any] keyed by the runtime op.function, so
  precise typing = per-function TypedDicts narrowed at dispatch; adds
  _GroupByParams for the group_by site this PR touches (full tagged-union
  ASTCall params is a larger AST refactor, out of scope here).
- test_try_native_row_op_declines_whole_row_group_prefixes asserted the
  pre-#1737 decline behavior; whole-row (prefixed-key) grouping now lowers
  natively, so the test asserts the native result against the hand-derived
  oracle instead (was the test-polars CI failure).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
@lmeyerov
lmeyerov force-pushed the feat/gfql-polars-rows-entity-groupby branch from 3e9840a to 509d564 Compare July 19, 2026 21:15
@lmeyerov
lmeyerov force-pushed the fix/gfql-optional-match-polars-frames branch from e513b8c to 2da6ba6 Compare July 19, 2026 21:33
lmeyerov added a commit that referenced this pull request Jul 19, 2026
…-row groupby test (review)

Review feedback on #1737:
- op.params is wire-format Dict[str, Any] keyed by the runtime op.function, so
  precise typing = per-function TypedDicts narrowed at dispatch; adds
  _GroupByParams for the group_by site this PR touches (full tagged-union
  ASTCall params is a larger AST refactor, out of scope here).
- test_try_native_row_op_declines_whole_row_group_prefixes asserted the
  pre-#1737 decline behavior; whole-row (prefixed-key) grouping now lowers
  natively, so the test asserts the native result against the hand-derived
  oracle instead (was the test-polars CI failure).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
@lmeyerov
lmeyerov force-pushed the feat/gfql-polars-rows-entity-groupby branch from 509d564 to 48a03d5 Compare July 19, 2026 21:33
Base automatically changed from fix/gfql-optional-match-polars-frames to master July 19, 2026 23:25
lmeyerov and others added 6 commits July 19, 2026 16:49
…guation + identity-key + key_prefixes)

Three pandas-parity ports unlock the LDBC IC4 shape natively on polars:
binding_rows_polars applies the HAS_<Label> candidate-domain narrowing (only
under duplicated candidate ids, mirroring pandas exactly) instead of a blanket
decline; alias.__gfql_node_id__ resolves to the bare alias id column; and
group_by expands key_prefixes like pandas. Official IC4 runs parity-exact
(harness ok @127.8ms SF0.1, was NIE). 184 suite tests + 2 new pass on dgx.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…ain dup probe, forward single-hop gate)

Self-review of the initial port found two parity divergences vs the pandas
call site (row/pipeline.py:3670): pandas probes id duplicates on the REACHED
candidate domain (candidate_source ∩ wavefront), not the full node table, and
applies the narrowing only for forward single-hop edges. The polars twin now
runs after the hop's state semi-join with the same gates; two new
engine-parametrized tests pin both branches (global-dup-but-unique-reached
must NOT narrow; reached-collision must narrow). 186+2 tests pass on dgx.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…-row groupby test (review)

Review feedback on #1737:
- op.params is wire-format Dict[str, Any] keyed by the runtime op.function, so
  precise typing = per-function TypedDicts narrowed at dispatch; adds
  _GroupByParams for the group_by site this PR touches (full tagged-union
  ASTCall params is a larger AST refactor, out of scope here).
- test_try_native_row_op_declines_whole_row_group_prefixes asserted the
  pre-#1737 decline behavior; whole-row (prefixed-key) grouping now lowers
  natively, so the test asserts the native result against the hand-derived
  oracle instead (was the test-polars CI failure).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…s (review)

Replace Sequence[Any] with SelectItem/OrderKey/AggSpec aliases across
select/order_by/group_by lowering; group_by_polars now takes
Sequence[str] keys + Sequence[AggSpec] + Optional[Sequence[str]]
key_prefixes (matching the pandas oracle sig); _GroupByParams.aggregations
narrowed to List[AggSpec]; _agg_expr schema typed as Mapping[str, pl.DataType].

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…a (deferred #1735 review)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
@lmeyerov
lmeyerov force-pushed the feat/gfql-polars-rows-entity-groupby branch from 48a03d5 to 74fe4a9 Compare July 19, 2026 23:49
…_rows_polars (review F1)

The disambiguation gate probed the chain-combine node table, which is already
deduplicated by id — the duplicate probe could never fire, and with the old
decline removed, duplicate-id graphs returned silently row-order-dependent
first-occurrence rows where pandas narrows to the HAS_<Label> label row
(reproduced end-to-end). The label row pandas narrows to may already be gone
from the deduped table, so a faithful native answer is impossible here: probe
the pre-chain base graph (_gfql_rows_base_graph) instead — unique-id graphs
stay native (pandas would not narrow either; IC4 keeps its win), duplicate-id
graphs decline to the honest NIE.

Also from review: binding-rows-path regression tests that actually reach the
gate (dup-id NIE both row orders + unique-id parity), key_prefixes genuine
expansion/overlap test, malformed-AggSpec decline test, nan_clean soundness
comment corrected (polars does have in-place mutation APIs; state the real
no-mutation contract), AggSpec import made runtime (introspectable TypedDict),
lower_select_items/lower_order_by_keys return List[pl.Expr].

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
@lmeyerov

Copy link
Copy Markdown
Contributor Author

Review-skill pass (3 dimensions + adversarial seam re-review) — ready for human review

BLOCKER found & fixed (2cdc9c33): the HAS_<Label> disambiguation gate probed the chain-combine node table, which is already id-deduplicated — the duplicate probe could never fire (coverage-verified dead code), and with the old NIE decline removed, duplicate-id graphs silently returned row-order-dependent first-occurrence rows where pandas narrows to the label row (reproduced end-to-end both row orders). Since chain-combine has already dropped the label rows pandas narrows to, a faithful native answer is impossible from the deduped table. Fix: probe the pre-chain base graph (_gfql_rows_base_graph) — unique-id graphs stay native (pandas would not narrow there either, structurally + 90-comparison differential fuzz; IC4 keeps its native win), duplicate-id/unknown-base graphs decline to the honest NIE. New tests reach the actual gate (coverage-confirmed), incl. dup-id NIE both row orders + unique-id parity.

Also from review:

  • nan_clean soundness comment corrected — polars does have in-place mutation (extend, replace_column, insert_column, hstack(in_place=True)); the real contract is "don't mutate frames after handing them to GFQL" (same as the pandas-input paths).
  • AggSpec import made runtime (TypedDict now get_type_hints-introspectable; safe without polars — verified under an import blocker).
  • lower_select_items/lower_order_by_keys return List[pl.Expr] (consistency with the PR's own typing sweep).
  • key_prefixes genuine-expansion + key-overlap test (previous unit test's prefix matched zero columns); malformed-AggSpec decline tests.
  • Old "narrowing" tests kept but relabeled honestly: their single-MATCH shape routes through the plain chain executor, not binding_rows_polars.

Filed #1739 (pre-existing on master, A/B-verified): the single-MATCH aggregating HAS_<Label> shape on duplicate-id graphs silently diverges on polars via a different path — out of this PR's scope.

Seam re-review verdict: APPROVE (6/6 probes pass; full local gfql lane 4443 passed, failures = known environmental set only). CI green at 2cdc9c33; #1738 re-rebased on top (551cc63e), also green.

Non-blocking follow-ups noted: exercise the two defensive decline lines (base-graph-absent / id-col-absent), and a pre-existing LazyFrame-input crash at chain.py:720 (before this gate) deserves its own issue.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL

OrderKey = Union[Tuple[str, str], List[str]]
"""Sort key: ``(expr, 'asc'|'desc')``."""
AggSpec = Union[Tuple[str, str], Tuple[str, str, Optional[str]], List[Optional[str]]]
"""Aggregation spec: ``(alias, func[, expr])``; ``expr`` None/omitted only for count(*)."""

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

factor out?

lmeyerov and others added 2 commits July 19, 2026 19:38
…all/support (review)

The wire-format payload types describe ASTCall.params for every engine, not
just polars: move them next to their runtime validators in
gfql/call/support.py (single source of truth, kept in lockstep) and adopt
them in the engine-neutral consumers — pandas RowPipelineMixin.group_by,
the ast.group_by builder, and the cypher lowering's aggregation lists
(TYPE_CHECKING imports where the call package __init__ would cycle).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
Pure typing growth in cypher/lowering.py from the engine-neutral
SelectItem/OrderKey/AggSpec hoist (TYPE_CHECKING import block +
List["AggSpec"] annotations at 3 call sites). No new compiled-surface
fields or properties.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
@lmeyerov
lmeyerov merged commit c355660 into master Jul 20, 2026
77 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant