fix(gfql): gate polars varlen node aliases by hop distance (refs #1741)#1747
Conversation
9fec80f to
c2add5d
Compare
Cross-engine validation on dgx-spark — 0 regressions, 70/98 mismatches fixed4-engine differential sweep against pandas as oracle, 144 cases (4 hand graphs + 12 random graphs × 9 varlen/fixed patterns), run in-container on dgx-spark under
Set-diff of the two mismatch sets: 0 regressions, 70 fixed. polars and polars-gpu behave identically throughout, and cudf/pandas are untouched — confirming the change is confined to the polars lane. The 28 residual mismatches (14 per polars engine) are all one shape, forward Raw JSON + the sweep script are committed in the campaign evidence set. |
…+ label-col collision (#1741/#1746) Two correctness fixes to the native polars label_node_hops path, moved down from the stacked chain PR so #1746 is right standalone: 1. Undirected seed pre-seed (was in #1747). Without pre-seeding label_seen_nodes with the seeds, an undirected walk in wave-front mode (visited starts empty) re-labels a seed it backtracks into, and a seed filtered out of the frontier by source_node_match also gets re-labeled. pandas excludes all original seeds from undirected labeling. Differential vs pandas over direction x hops x return_as_wave_front x source_node_match x destination_node_match x 10 graphs: was 456/24, now 480/480. 2. Label column-name collision parity. A requested label_node_hops name that already exists on the node table is now redirected to `<name>_1` (matching pandas' resolve_label_col) instead of the polars left-join auto-suffix `<name>_right` / DuplicateError. Amplifies TestHopLabelsDifferential with the seed-filter / wave-front / multi-seed axes that were previously unvaried (exactly the diverging cases), plus a collision test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
c2add5d to
3f0fa21
Compare
Rebased to chain-only + collision-hardenedTwo changes since the review:
Full polars chain suite: 1000 passed, 2 xfailed, ruff clean. Cross-engine A/B (all 4 engines, dgx-spark) re-run below. |
…ong (#1748) Adapts the retired #1742 decline pattern to the one shape #1747's hop-gate can't yet cover: a node named after a forward/reverse variable-length edge with min_hops>1. Those labels come from pandas' layered backward walk (not ported), so the alias runs ungated and tags nodes outside the [min_hop, max_hop] window — silently wrong rows. Now an honest NotImplementedError pointing at #1748. The decline is precise (differential vs pandas, 15 graphs x fwd/rev x named/unnamed): 30/30 named shapes NIE, 30/30 unnamed shapes run and match pandas, 0 silent-wrong, 0 over-decline. With this the whole #1741 stack leaves ZERO silent-wrong shape on the polars chain. xfail pins flipped to positive assert-raises tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
Now leaves ZERO silent-wrong shapes (adapted the closed #1742's mechanism for #1748)Rather than discard the closed #1742 (which declined the undirected-varlen-alias shape), its decline pattern is adapted here to the one shape this PR's hop-gate can't yet cover: a node named after a forward/reverse Precise, not blunt — differential vs pandas (15 graphs × fwd/rev × named/unnamed):
4-engine cross-engine sweep on dgx-spark, before → after this commit:
The xfail pins flipped to positive assert-raises tests. Net: the whole #1741 stack is now silent-wrong-free on the polars chain — every unsupported shape is an honest error, every supported shape matches pandas. Raw: |
| # generate_safe_column_name (see _chain_traversal_polars), so a user column literally named | ||
| # `__gfql_chain_node_hop__` can't be clobbered (would otherwise crash on the int/str compare in | ||
| # the gate). This base string is only the seed + the fallback for callers that don't resolve. | ||
| _AUTO_NODE_HOP = "__gfql_chain_node_hop__" |
There was a problem hiding this comment.
extern to a symbols.py?
A node named after a variable-length edge (`MATCH (a)-[*1..2]-(b)`) must carry its alias only when its hop distance falls inside the edge's [min_hop, max_hop] window (pandas chain.py:456-501). The polars chain had no distance to gate on, so it aliased a backtracked-to seed pandas leaves unaliased — silently wrong RETURN rows. Auto-injects the hop-distance label (name resolved against the user's node columns via generate_safe_column_name, so a user column literally named __gfql_chain_node_hop__ is never clobbered — a clash would crash the int/str compare) and applies pandas' alias window in _apply_node_names. The underlying hop-label correctness (undirected seed pre-seed) now lives in the base PR #1746. Retitled closes->refs #1741: forward/reverse min_hops>1 aliases still run ungated (a whole family of silently-wrong shapes) tracked as #1748 and pinned here with strict xfail tests so the gap is visible in CI and flips to failure the moment it lands. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…ong (#1748) Adapts the retired #1742 decline pattern to the one shape #1747's hop-gate can't yet cover: a node named after a forward/reverse variable-length edge with min_hops>1. Those labels come from pandas' layered backward walk (not ported), so the alias runs ungated and tags nodes outside the [min_hop, max_hop] window — silently wrong rows. Now an honest NotImplementedError pointing at #1748. The decline is precise (differential vs pandas, 15 graphs x fwd/rev x named/unnamed): 30/30 named shapes NIE, 30/30 unnamed shapes run and match pandas, 0 silent-wrong, 0 over-decline. With this the whole #1741 stack leaves ZERO silent-wrong shape on the polars chain. xfail pins flipped to positive assert-raises tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…ecise types (review c2,c3) Establishes graphistry/compute/gfql/lazy/engine/polars/reserved_columns.py — a per-engine registry of the __gfql_*__ internal column-name bases, referencing the repo-wide reserved prefix/suffix from identifiers.py. _AUTO_NODE_HOP now sources CHAIN_NODE_HOP from it. Types made precise: _auto_node_hop_col(op: ASTObject), _alias_hop_bounds(op: ASTEdge), _exec wavefront params Optional[Any]. (Follow-up: migrate remaining inline literals.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
…e + #1748 decline) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
196f69d to
7605dbc
Compare
…baseline New file was 100% covered but absent from ci-polars-py3.12.json, which the per-file floor audit requires (fails un-baselined targets). Floor 90 (constants, always 100%). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6dQEcjdazEnzuvuwf73ZL
Stacked on #1746 — review that one first; this PR's diff is only the chain half.
What this fixes
#1741: on the polars engine,
MATCH (a {id: 'p0'})-[*1..2]-(b) RETURN breturned{p0, p1, p2}where pandas returns{p1, p2}. Silently wrong rows, not an error. After this PR both engines return{p1, p2}.Root cause (corrected — the original write-up was wrong)
The first diagnosis said "polars lacks pandas' backtrack avoidance". Direct A/B refutes that: both engines return identical node sets and identical final
_nodes; only the alias flag diverges.The actual pandas rule is a hop-distance gate (
compute/chain.py~456-501): a node named after an edge op is aliased only whenmin_hop <= hop <= max_hop. The seed's hop label is null under an undirected walk, so it fails the gate and loses the alias. polars had no hop labels at all, so it could not run the gate — #1746 adds the labels, this PR adds the gate.Implementation
label_node_hopswhenever an edge op is variable-length or output-sliced, mirroringast.py:621-625needs_auto_labels.[min_hop, max_hop]window in_apply_node_names.return_as_wave_front— the mode the chain always hops in.visited_nodesstarts empty there, but pandas labels a backtracked-to seed null in both modes, so the undirected label-seen set is now pre-seeded with the seeds unconditionally.Verification
graphistry/tests/compute/gfql+test_compute_hops.py+test_compute_chain.py.origin/master's on the same environment (temporal / cudf / viz) — I ran the same selection on a detached master worktree and diffed the failure lists: zero new failures.Residual gap (kept on #1741)
min_hops > 1chains get no gate — requesting labels there would turn a currently-native chain into an NIE, since those labels come from the layered backward walk (hop.py:676-778), which is not yet ported.Follow-up
Once this lands, #1742 closes unmerged — it was the interim honest-NIE decline for exactly this shape, and a raise is the wrong end state (per review feedback). Its test flips from "raises" to "matches the pandas oracle".
🤖 Generated with Claude Code