fix(hop)!: filter-domain invariant + to_fixed_point==saturated (#1892)

[lmeyerov]

First umbrella of the fix cycle (owner: iterate after fixes). Fixes #1892's two HIGH classes with the round-003 RED pins as the acceptance gate — all 8 flipped, 42/42 green both engines; gates stash-verified against pre-existing env failures. Stacked on #1883. Behavior changes CHANGELOG'd: id-only seeds now work at hops=1; stale seed columns can't flip values; undirected tfp no longer leaks unencountered seeds and equals the saturated bounded arm.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MF7uRZLKZaD6Q9FGWSmyXi

[lmeyerov]

Hold this PR — its headline invariant is still false in an arm the pin does not cover. Found by a round-011 re-probe of the direct hop() surface (109 hand-oracled cases, oracles written before execution): full detail in #1918.

test_hop_undirected_tfp_wavefront_matches_saturated_bounded parameterizes only over {source_node_match}/{destination_node_match} with seeds [0,1]. Run its exact fixture and kwargs unfiltered with a single seed:

pandas seeds=[0] filt=none : bounded(hops=3)=[0,1,2]  tfp=[1,2]   VIOLATED
pandas seeds=[1] filt=none : bounded(hops=3)=[0,1,2]  tfp=[0,2]   VIOLATED
pandas seeds=[0,1] filt=none: bounded=[0,1,2] tfp=[0,1,2] OK  <- the arm the pin happened to pick
polars: correct in all six arms

The F-02 patch in this PR intersected the undirected keep-set with matches_nodes, which removed the leak it was aimed at. It did not address the upstream heuristic — _undirected_component_seed_keep_ids keeps a seed only when its component holds >1 seed, and _undirected_cycle_nodes only when it is on a cycle — dropping seeds the bounded arm keeps. Any acyclic single-seed component reproduces (2-node path, 3-node path with the seed in the middle, star); a triangle does not, which is why the cycle-shaped fixtures pass.

The same probe found three further pandas silent-wrongs on this surface (edges-only graphs returning the entire node table; hop-tracking flags destroying seed attributes and upcasting int64→float64; undirected edges doubled under tracking) plus two cross-engine gaps. All in #1918, sequenced F4-first so this PR can go green on its own claim.

Nothing here says the fix is wrong — it is incomplete, and the pin's parameterization hid the incomplete part. That is exactly what a re-probe is for.

[lmeyerov]

Hold lifted on the correctness side. F4 — this PR's headline to_fixed_point == saturated bounded invariant being false in the unfiltered single-seed arm — is fixed in #1920, and its pin here is widened to cover exactly the three cells the old parameterization omitted (they fail at base, so the pin is not vacuous).

Root cause worth recording: the tfp+undirected wavefront strip decided seed retention by topology heuristics (does the component hold more than one seed, is the seed on a cycle) while the bounded arm asks "was the seed re-encountered". This PR's F-02 patch fixed the resulting leak but never the drop. #1920 replaces both with the single rule the bounded arm already used, deleting 72 lines of heuristics — and undirected tfp got 63% faster as a side effect (34.7 → 12.8 ms) from the itertuples walks that went with them.

Still open before I can recommend sign-off: the whole-board A/B found a pandas q1/q2 regression (+11.3%/+10.0% @20k, +6.9%/+6.7% @100k, pandas-only, Bonferroni-significant) somewhere in this twelve-PR stack. A bisect over the PR heads is running to attribute it; hop is a plausible suspect since q1/q2 are degree-shaped, so I want that answer before signing this one.

[lmeyerov]

asymptotics belong more in pyg-bench than these comments afaict, audit & fix, incl root cause on why authoring & review rules let through

[lmeyerov]

clear correctness belong in clear code, tests, tck-gfql tests, etc, not comments

[lmeyerov]

changes seemingly look good, likely pyg-bench etc obligations for the removals

[lmeyerov]

Both comments on hop.py:1154 are addressed in 009c52dc6 ("name the source-filter domain rule instead of narrating its cost").

The two comments you objected to are deleted, not moved. git show 009c52dc6:graphistry/compute/hop.py | grep -n "itertuples walks\|F-02\|topology-only heuristics" returns nothing.

On "correctness belongs in clear code and tests, not comments." The # #1892 F-02: the topology-only heuristics ignore source/dest filter pruning; keep only seeds the traversal re-encountered so tfp == saturated bounded comment was narrating a rule that the code did not state. The three-statement inline block is now one named call:

keep_seed_ids = _undirected_rediscovered_seed_ids(
    final_edges, wavefront_seed_ids_df, matches_nodes)

The re-encounter intersection that the comment described is now inside that helper (& _reached_node_ids(reached_nodes, node_col)), so it cannot be read as optional, and the predicate itself is named _reached_node_ids rather than described.

The correctness claim is pinned by tests, not prose — in test_hop_boundary_matrix.py:

• test_undirected_tfp_equals_saturated_bounded
• test_undirected_tfp_equals_saturated_bounded_filtered
• test_undirected_tfp_wavefront_rediscovered_seeds
• test_undirected_tfp_wavefront_filtered_rediscovered_seeds

That matrix is 395 hand-oracled cells, 134 of which fail at this PR's base.

On "asymptotics belong in pyg-bench." Agreed — the # NOTE: both helpers are O(E) python itertuples walks; vectorize if this arm gets hot line is gone and no cost claim replaced it.

On "root cause on why authoring & review rules let through" — that part is not fixed by this PR, and I filed it as two:

• docs(review): encode the rejection categories the GFQL stack kept hitting #1924 adds the missing rules to agents/skills/review/SKILL.md: a verdict table that sends multi-line narration to an extracted named helper and a "why this fix was made" comment to the pin's test name, with perf/asymptotics claims classified as a BLOCKER that belongs in pyg-bench; plus a self-review gate that greps <base>..HEAD before pushing, and a rule that a justified keep is still a keep — the bar is "can a test or a name carry this instead?", not "is there a reason?".
• ci: enforce comment-encoding rules with a guard #1926 makes it mechanical rather than a matter of reviewer attention: bin/ci_comment_density_guard.py fails on a perf-claim comment (this exact O(E) line is the pattern it matches), on 2+ adjacent narration lines, and on issue-number rationale comments. Ratcheted per-file so master's existing debt is frozen rather than reopened. I checked it against this PR and the other two under review: the guard would have failed all three.

The honest root cause is that the review skill had no rule for either category, so both were authored and reviewed as normal — the guard exists so it stops depending on whether the rule gets remembered.