fix(query): dedup seeds by label to stop generic-symbol flooding (#1766)
Many nodes sharing one generic label (framework route handlers all labelled GET/POST, a repeated handler) consumed every BFS seed slot, so query traversal explored near-identical neighborhoods and buried the actual target. Seed selection now dedups by normalized label (GET/Get/get collapse together), keeping one representative per label, and the per-term guarantee loop honors the same cap so it can't reintroduce a dupe. Adopts @devcool20's seed-dedup from #1832 but drops that PR's second mechanism — a per-label multiplicity penalty applied inside the shared _score_nodes. That scorer also resolves shortest_path/explain endpoints, so dividing scores there silently reweighted path/explain (out of scope for #1766 and able to flip endpoint selection); the dedup alone bounds the flood. Also normalizes the dedup key (the PR keyed on the raw label). Adds the tests the PR was missing: dedup of homonymous labels, case/diacritic normalization, per-term-guarantee cap, and a guard that identical-label nodes still score equally in _score_nodes. Co-Authored-By: devcool20 <devcool20@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
devcool20
Claude Opus 4.8
parent
da9616d99e
commit
6ca8604142
@@ -8,6 +8,8 @@ Full release notes with details on each version: [GitHub Releases](https://githu
|
||||
|
||||
- Fix: the CLI no longer crashes with exit code 255 when a downstream reader closes the pipe early (#1807 / #1811, thanks @varuntej07). Truncating output with `head`, PowerShell's `Select-Object -First N`, or `sed q` disconnected the reader mid-write, graphify hit an unhandled `BrokenPipeError` (or `OSError(EINVAL)` on Windows) and exited 255 — so CI wrappers and agent harnesses that both trim output and check the exit code read a successful query as a command failure. An early-closing reader is now treated as success: stdout is flushed inside the guard (piped stdout is block-buffered, so a small output would otherwise only flush at interpreter shutdown, where the error escapes as a noisy "Exception ignored" and a nonzero exit), then redirected to devnull so the shutdown flush can't raise again, and the process exits 0.
|
||||
|
||||
- Fix: `graphify query` no longer floods results with homonymous generic symbols (#1766 / #1832, thanks @devcool20). When dozens of nodes share one generic label — framework route handlers all labelled `GET`/`POST`, a repeated `handler` — they used to consume every BFS seed slot, so the traversal explored many near-identical neighborhoods and drowned out the query's actual target. Seed selection now deduplicates by normalized label (`GET`/`Get`/`get` collapse together), keeping at most one representative per label while still guaranteeing a seed for each distinct query term. The per-label cap is scoping-only — the shared node scorer (also used by `shortest_path`/`explain` endpoint resolution) is left untouched, so those are unaffected.
|
||||
|
||||
- Fix: semantic cache writes are now scoped to the files actually dispatched in each extraction batch (#1757 / #1835, thanks @TPAteeq). A model can attribute a node's `source_file` to another corpus file; `save_semantic_cache` would then replace (or, mid-run, pollute) that other file's complete cache entry with a stray fragment — silently, with no shrink guard. Writes now honor an `allowed_source_files` allowlist: the final CLI write is scoped to the uncached file set, and the per-chunk incremental checkpoint (`extract_corpus_parallel`, the default path for `extract`/`update`) is scoped to that chunk's own files, so an out-of-scope attribution is skipped with a warning instead of clobbering a legitimate entry.
|
||||
|
||||
- Fix: `graphify export graphml` no longer crashes on dict- or list-valued attributes (#1831 / #1830, thanks @hofmockel). `nx.write_graphml` only accepts scalar values, so a per-node `metadata` dict or the graph-level `hyperedges` list raised `GraphML does not support type <class 'dict'/'list'>` and failed the entire export — on a real ~2,300-node graph, every attempt. `to_graphml` now coerces `None -> ""` and JSON-serializes non-scalars across graph/node/edge scopes (GraphML-native int/float/bool/str pass through unchanged), and writes atomically via a temp file so a failed export no longer leaves a 0-byte `.graphml` that downstream tooling mistakes for a completed one.
|
||||
|
||||
+29
-3
@@ -435,11 +435,33 @@ def _pick_seeds(
|
||||
"""
|
||||
if not scored:
|
||||
return []
|
||||
|
||||
# Deduplicate seeds by (normalized) label so a generic, homonymous symbol —
|
||||
# e.g. dozens of route handlers all labelled `GET`/`POST`, or a `handler`
|
||||
# repeated across a framework — contributes at most one seed instead of
|
||||
# consuming every slot and flooding the BFS with near-identical neighborhoods
|
||||
# (#1766). The key mirrors _score_nodes' normalization so `GET`/`Get`/`get`
|
||||
# collapse together. When G is absent we can't read labels, so fall back to
|
||||
# the (unique) node id, which is a no-op — preserving the old behavior.
|
||||
def _seed_label_key(nid: str) -> str:
|
||||
if G is None:
|
||||
return nid
|
||||
data = G.nodes[nid]
|
||||
return (data.get("norm_label")
|
||||
or _strip_diacritics(data.get("label") or "").lower()) or nid
|
||||
|
||||
top_score = scored[0][0]
|
||||
seeds = []
|
||||
for score, nid in scored[:max_k]:
|
||||
seeds: list[str] = []
|
||||
seen_labels: set[str] = set()
|
||||
for score, nid in scored:
|
||||
if len(seeds) >= max_k:
|
||||
break
|
||||
if seeds and score < top_score * gap_ratio:
|
||||
break
|
||||
key = _seed_label_key(nid)
|
||||
if key in seen_labels:
|
||||
continue
|
||||
seen_labels.add(key)
|
||||
seeds.append(nid)
|
||||
|
||||
if G is not None and terms:
|
||||
@@ -451,7 +473,11 @@ def _pick_seeds(
|
||||
best_score = term_scored[0][0]
|
||||
tied = [nid for s, nid in term_scored if s == best_score]
|
||||
best_nid = max(tied, key=lambda n: G.degree(n)) if len(tied) > 1 else term_scored[0][1]
|
||||
if best_nid not in seeds:
|
||||
# Honor the same per-label cap so the per-term guarantee can't
|
||||
# reintroduce a second copy of an already-seeded generic label.
|
||||
key = _seed_label_key(best_nid)
|
||||
if best_nid not in seeds and key not in seen_labels:
|
||||
seen_labels.add(key)
|
||||
seeds.append(best_nid)
|
||||
return seeds
|
||||
|
||||
|
||||
@@ -769,6 +769,65 @@ def test_pick_seeds_diversity_recovers_starved_term(monkeypatch):
|
||||
assert "target" in seeds_after
|
||||
|
||||
|
||||
# --- generic-symbol seed flooding (#1766) ---
|
||||
|
||||
def test_pick_seeds_dedups_homonymous_generic_labels():
|
||||
"""Many nodes sharing one generic label (e.g. framework `GET` handlers)
|
||||
must contribute at most ONE seed, not consume every slot (#1766). A
|
||||
distinct, relevant label still gets its own seed."""
|
||||
G = nx.DiGraph()
|
||||
for i in range(5):
|
||||
G.add_node(f"get{i}", label="GET", source_file=f"routes/r{i}.py")
|
||||
G.add_node("um", label="users_model", source_file="models/users.py")
|
||||
# Score all the GET nodes above users_model so, pre-fix, they'd take every slot.
|
||||
scored = [(1000.0, f"get{i}") for i in range(5)] + [(900.0, "um")]
|
||||
seeds = _pick_seeds(scored, G=G)
|
||||
get_seeds = [s for s in seeds if s.startswith("get")]
|
||||
assert len(get_seeds) == 1, f"expected one GET representative, got {get_seeds}"
|
||||
# A different, well-within-gap label is not starved out by the GET flood.
|
||||
assert "um" in seeds
|
||||
|
||||
|
||||
def test_pick_seeds_dedup_key_is_case_and_diacritic_normalized():
|
||||
"""`GET`/`Get`/`get` are the same generic label and must dedup together."""
|
||||
G = nx.DiGraph()
|
||||
G.add_node("a", label="GET", source_file="a.py")
|
||||
G.add_node("b", label="Get", source_file="b.py")
|
||||
G.add_node("c", label="get", source_file="c.py")
|
||||
scored = [(1000.0, "a"), (990.0, "b"), (980.0, "c")]
|
||||
seeds = _pick_seeds(scored, G=G)
|
||||
assert len(seeds) == 1, f"case-variant duplicates not collapsed: {seeds}"
|
||||
|
||||
|
||||
def test_pick_seeds_per_term_guarantee_does_not_reintroduce_generic_dupe(monkeypatch):
|
||||
"""The per-term guarantee loop must honor the same per-label cap, so it can't
|
||||
add a second `GET` after dedup already seeded one (#1766)."""
|
||||
G = nx.DiGraph()
|
||||
for i in range(3):
|
||||
G.add_node(f"get{i}", label="GET", source_file=f"r{i}.py")
|
||||
G.add_node("um", label="users_model", source_file="users.py")
|
||||
G.add_edge("um", "get0")
|
||||
scored = _score_nodes(G, ["get", "users"])
|
||||
seeds = _pick_seeds(scored, G=G, terms=["get", "users"])
|
||||
get_seeds = [s for s in seeds if s.startswith("get")]
|
||||
assert len(get_seeds) == 1, f"per-term guarantee reintroduced a GET dupe: {seeds}"
|
||||
|
||||
|
||||
def test_score_nodes_scores_identical_labels_equally():
|
||||
"""Guard against a per-label multiplicity penalty leaking into _score_nodes
|
||||
(shared by shortest_path / explain endpoint resolution): two nodes with the
|
||||
SAME label must receive the SAME score for a query, i.e. the fix lives in
|
||||
seed selection, not in the shared scorer (#1766 followup)."""
|
||||
G = nx.DiGraph()
|
||||
G.add_node("g1", label="GET", source_file="a.py")
|
||||
G.add_node("g2", label="GET", source_file="b.py")
|
||||
G.add_node("g3", label="GET", source_file="c.py")
|
||||
by_id = {nid: s for s, nid in _score_nodes(G, ["get"])}
|
||||
assert by_id["g1"] == by_id["g2"] == by_id["g3"], (
|
||||
f"identical-label nodes scored differently: {by_id}"
|
||||
)
|
||||
|
||||
|
||||
# --- actionable truncation hint (#897) ---
|
||||
|
||||
def test_subgraph_to_text_truncation_hint_is_actionable():
|
||||
|
||||
Reference in New Issue
Block a user