From 8f40e4b984dbe4dbcf91d291e7a8546eee09b4e0 Mon Sep 17 00:00:00 2001 From: Juan Garcia Longaron <1871337+juanyque@users.noreply.github.com> Date: Wed, 22 Jul 2026 21:50:02 +0200 Subject: [PATCH] fix: bound per-term query seed recovery Co-authored-by: Codex (GPT-5) --- graphify/serve.py | 60 +++++++++++++++++++++-------------- tests/test_serve.py | 76 +++++++++++++++++++++++++++++---------------- 2 files changed, 85 insertions(+), 51 deletions(-) diff --git a/graphify/serve.py b/graphify/serve.py index f32a91673..be762a892 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -567,16 +567,19 @@ def _pick_seeds( seeds, so the BFS traversal only ever explores the neighborhood of the one unrelated exact match — see #1445. - When `G` and `best_seed_by_term` are supplied, this guarantees at least one - seed per distinct query term that has any match at all, so one term's - incidental collision cannot starve out the others. The per-token winners - in `best_seed_by_term` are precomputed by `_score_query` (during the same - traversal that produced `scored`) so this function no longer rescores the - graph per term — see #1445 and the `_score_query` docstring. + When `G` and `best_seed_by_term` are supplied, the remaining slots recover + singleton winners for terms that the combined ranking would otherwise + starve out. Recovery is bounded by `max_k`: long natural-language queries + must not turn the seed cap into a soft suggestion and flood traversal with + one generic seed per token. When several terms share a winner, that node is + preferred because it covers more of the query with one slot. The per-token + winners in `best_seed_by_term` are precomputed by `_score_query` (during the + same traversal that produced `scored`) so this function no longer rescores + the graph per term — see #1445 and the `_score_query` docstring. Coverage scaling in _score_nodes (#1602) now dampens a lone collision's exact tier on multi-term queries, which brings label-matching relevant - nodes back inside the gap window; this per-term guarantee remains + nodes back inside the gap window; this per-term recovery remains load-bearing for relevant nodes matched only via substrings, whose flat scores a dampened collision can still exceed. """ @@ -611,24 +614,33 @@ def _seed_label_key(nid: str) -> str: seen_labels.add(key) seeds.append(nid) - if G is not None and best_seed_by_term: - # Guarantee one seed per distinct query term that has any match at all, - # so an incidental exact match on one term cannot starve matches on - # other terms (#1445). Iterate tokens in a deterministic sorted order - # so seeds added by this loop have a stable order independent of dict - # iteration — preserving the legacy `_pick_seeds(terms=...)` behavior - # which iterated `sorted({tok ...})`. Per-token winners arrive - # precomputed in `best_seed_by_term` from `_score_query`'s single - # traversal, so `_pick_seeds` no longer rescoring the graph per term. - # The per-label dedup cap also gates these additions, so the guarantee - # cannot reintroduce a second copy of an already-seeded generic label - # (#1766). - for term in sorted(best_seed_by_term): - best_nid = best_seed_by_term[term] - # Honor the same per-label cap so the per-term guarantee can't - # reintroduce a second copy of an already-seeded generic label. + if G is not None and best_seed_by_term and len(seeds) < max_k: + # Recover per-term winners without exceeding max_k. Grouping by node + # lets one candidate represent several otherwise-starved terms with a + # single slot; combined score then gives deterministic relevance order + # among equally broad candidates. This preserves #1445's recovery while + # preventing a long query from appending one generic seed per token. + terms_by_nid: dict[str, set[str]] = {} + for term, best_nid in best_seed_by_term.items(): + if best_nid not in seeds: + terms_by_nid.setdefault(best_nid, set()).add(term) + score_by_nid = {nid: score for score, nid in scored} + candidates = sorted( + terms_by_nid, + key=lambda nid: ( + -len(terms_by_nid[nid]), + -score_by_nid.get(nid, 0.0), + len(G.nodes[nid].get("label") or nid), + nid, + ), + ) + for best_nid in candidates: + if len(seeds) >= max_k: + break + # Honor the same per-label cap so recovery cannot reintroduce a + # second copy of an already-seeded generic label (#1766). key = _seed_label_key(best_nid) - if best_nid not in seeds and key not in seen_labels: + if key not in seen_labels: seen_labels.add(key) seeds.append(best_nid) return seeds diff --git a/tests/test_serve.py b/tests/test_serve.py index fdaa2753c..b5a615c6f 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -812,7 +812,7 @@ def test_pick_seeds_diversity_recovers_starved_term(monkeypatch): as an unrelated field/identifier) outscores every SUBSTRING match on the query's other, actually-relevant terms by ~1000x. Without G/best_seed_by_term, the 20%-gap cutoff discards the relevant candidate - entirely; with them, it is recovered as a guaranteed per-term seed. + entirely; with them, it is recovered while seed capacity remains. """ G = nx.DiGraph() # "unrelated" is an exact label match for the query term "unrelated" and @@ -826,7 +826,7 @@ def test_pick_seeds_diversity_recovers_starved_term(monkeypatch): terms = ["unrelated", "widget"] # `_score_query` does the combined scoring and the per-term singleton # winner tracking in one traversal; `_pick_seeds` consumes its - # `best_seed_by_term` to satisfy the per-term guarantee without rescoring. + # `best_seed_by_term` to recover starved terms without rescoring. qs = _score_query(G, terms, collect_per_term_seeds=True) scored = qs.ranked @@ -839,6 +839,44 @@ def test_pick_seeds_diversity_recovers_starved_term(monkeypatch): assert "target" in seeds_after +def test_pick_seeds_per_term_recovery_respects_max_k_and_prefers_coverage(): + """Per-term recovery must not turn ``max_k`` into a soft suggestion. + + A long natural-language query can have many distinct terms whose singleton + winners are all different nodes. The recovery path should stay bounded and + spend its remaining slots on nodes that account for the most query terms. + """ + G = nx.DiGraph() + G.add_node("anchor", label="CheckoutMatchingResult", source_file="result.py") + G.add_node("bridge", label="rpc_response_adapter", source_file="adapter.py") + G.add_node("service", label="Processor", source_file="processor.py") + G.add_node("model", label="Models", source_file="models.py") + + scored = [ + (1000.0, "anchor"), + (10.0, "bridge"), + (9.0, "service"), + (8.0, "model"), + ] + best_seed_by_term = { + "rpc": "bridge", + "response": "bridge", + "adapter": "bridge", + "processor": "service", + "models": "model", + } + + seeds = _pick_seeds( + scored, + max_k=3, + G=G, + best_seed_by_term=best_seed_by_term, + ) + + assert seeds == ["anchor", "bridge", "service"] + assert len(seeds) <= 3 + + # --- generic-symbol seed flooding (#1766) --- def test_pick_seeds_dedups_homonymous_generic_labels(): @@ -1134,43 +1172,27 @@ def test_score_query_best_seed_by_term_matches_legacy_singleton_scoring(terms): @pytest.mark.parametrize("terms", SYLLABLE_QUERIES) -def test_pick_seeds_with_optimized_best_seed_matches_legacy_semantics(terms): +def test_pick_seeds_with_optimized_best_seed_matches_bounded_reference(terms): """The seeds produced by `_pick_seeds(qs.ranked, G=G, best_seed_by_term= - qs.best_seed_by_term)` exactly match what the legacy `_pick_seeds(terms=...)` - loop would have produced (recreated via the reference oracle).""" + qs.best_seed_by_term)` exactly match seeds produced from the legacy + per-token winner oracle, while the shared bounded selector owns ordering.""" G = _make_random_scoring_graph(80, seed=7) qs = _score_query(G, terms, collect_per_term_seeds=True) ref_best = _reference_best_seed_by_term(G, terms) - # Legacy `_pick_seeds(terms=...)` ran `_score_nodes(G, [term])` per token - # to build ref_best, then deduped by label key. The new `_pick_seeds( - # best_seed_by_term=...)` only swaps the source of the per-token winners, - # so it must produce the same seeds given equivalent inputs. + # Legacy `_pick_seeds(terms=...)` ran `_score_nodes(G, [term])` per token to + # build ref_best. The optimized scorer only swaps the source of those + # winners, so the bounded selector must produce the same result from + # equivalent inputs. opt_seeds = _pick_seeds(qs.ranked, G=G, best_seed_by_term=qs.best_seed_by_term) ref_seeds = _pick_seeds(qs.ranked, G=G, best_seed_by_term=ref_best) assert opt_seeds == ref_seeds, f"terms={terms}: ref={ref_seeds} opt={opt_seeds}" - # Per-term guarantee: every legacy winner with a non-empty seed slot is - # accounted for — either it appears in the seed list or another node with - # the same normalized label already claimed the slot (#1766 label dedup). - ref_seed_set = set(ref_seeds) - for term, nid in ref_best.items(): - if nid in ref_seed_set: - continue - nid_label = (G.nodes[nid].get("norm_label") - or G.nodes[nid].get("label") - or nid) - seeded_with_same_label = any( - (G.nodes[s].get("norm_label") or G.nodes[s].get("label") or s) == nid_label - for s in ref_seeds - ) - assert seeded_with_same_label, ( - f"term {term!r} winner {nid!r} dropped without label-dedup reason" - ) + assert len(ref_seeds) <= 3 def test_score_query_matches_legacy_across_random_deterministic_graphs(): """Across many deterministic random graphs and many random multi-term queries, the single-pass scorer's combined ranking, per-token winners, - and resulting seed list all match the legacy semantics. Exercises label + and bounded seed list all match the reference winners. Exercises label collisions, ties, broad terms, missing terms, and graph size variance.""" import random