From 1fdd11fa76c6747371b048105a329ffaf1e31847 Mon Sep 17 00:00:00 2001 From: sean-soomgo <261135493+sean-soomgo@users.noreply.github.com> Date: Mon, 10 Aug 2026 12:13:50 +0900 Subject: [PATCH] fix(serve): resolve punctuated and non-ASCII node ids in _find_node (#2467) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_find_node_tiers` builds two normalizations of the query: `term`, which tokenizes on \w+ so punctuation becomes a space, and `norm_query`, which keeps it. The exact tier compared the node id against `term` only, so `term == nid_lower` was false for every id carrying punctuation, and `norm_query` — which already held the right form, and is even one of the two trigram needles — was never compared against the id at all. Comparing `norm_query` to the folded id closes that half. It does not reach ids carrying non-ASCII text. `_node_search_text` indexed the id raw while every query path folds through `_strip_diacritics`, which NFKD-decomposes. Hangul syllables decompose into conjoining jamo, and jamo have combining class 0, so they survive the combining-character filter: the needle's trigrams and the posting's trigrams were disjoint, `_trigram_candidates` returned a candidate list without the node, and it was dropped before any predicate ran. The folded id is now part of the indexed text. Both halves are additive. An id that resolved before resolves to the same node; only ids that previously resolved to nothing can now resolve. The folded field is appended, and only when the fold actually differs, so field positions do not move and an all-ASCII graph indexes byte for byte what it indexed before. Index build, median of 7 runs: graph trigrams postings build 5k all-ASCII before 1723 238876 96ms 5k all-ASCII after 1723 238876 97ms 17k real before 33442 2311784 998ms 17k real after 33490 2312462 990ms 5k half-Hangul before 1735 256381 116ms 5k half-Hangul after 1739 266381 124ms The real graph is the 17269-node one measured below; 354 of its ids are non-ASCII, so the index grows 0.03% and the build stays inside run-to-run noise. The half-Hangul row is a deliberate worst case — every other node id Korean — and even there the cost is paid once per graph load, on a graph where id lookup previously returned nothing at all. On a real 17269-node graph with Korean source filenames, every node id fed back to itself, full population: id class total before after contains punctuation 2326 0 2326 contains Hangul 354 0 354 ASCII, no punctuation 14589 14589 14589 And every query that graph can produce — all 17269 ids plus all 16537 distinct labels — through `_find_node_tiers` on both variants in one process: 31126 identical, 2680 that returned nothing before and resolve now, 0 with a changed first result, 0 lost, 0 with a widened exact tier. Every difference is a query that previously returned nothing. Left alone deliberately: `_score_query` compares the id raw in the same way, so `path` and query seeding still cannot take a punctuated id, and the prefix tier also matches ids against `term` only. Both are behaviour changes beyond this defect rather than part of it. One note for the regression tests: the non-ASCII case needs a graph of at least ~10 nodes. `_trigram_candidates` bails out to a full scan when `min(present) > int(n * 0.10)`, so on a small synthetic graph the index path is never taken and the test passes with the defect still present. --- CHANGELOG.md | 1 + graphify/serve.py | 23 ++++++++++-- tests/test_serve.py | 88 +++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 110 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4b1aca56..863d86c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ Full release notes with details on each version: [GitHub Releases](https://githu - Fix: Swift receiver-type inference now handles `@Environment(Store.self)` properties and factory-initialised bindings (#2561, thanks @fakewaffle). A member call on a receiver typed only through an `@Environment(Type.self)` attribute, or bound to an in-corpus factory whose return type is known (`let x = ServiceFactory.make()`), now resolves. Ambiguous or non-concrete returns (opaque `some P`, arrays, out-of-corpus) stay unresolved rather than guessing. - Fix: the SQL extractor no longer emits a `reads_from` edge to a CTE name (#2577, thanks @wilyan09007). A `WITH cte AS (...)` name is scoped to its query and is no longer treated as a table, so it no longer mints a bare stub that could bind to an unrelated same-named symbol; an outer real table sharing a subquery-CTE's name still resolves. - Fix: a dynamic `await import('…')` inside a nested function or at module scope now produces an edge (#2575, thanks @phudayyy), and `dynamic_import` edges are now included in `affected`. Calls inside a nested named function are also collected now. A dynamic import already captured as a deferred `imports_from` is not double-counted. +- Fix: `explain` resolves node ids that contain punctuation or non-ASCII text (#2467). An id was only ever compared against the `\w+`-tokenized query, so `concept:domain:x`, every `merge-graphs` `::` id, and every Hangul id failed to resolve; the id printed by `explain` could not be fed back into `explain`, and the ambiguity hint "Retry with […] the full node id" named a remedy that could not work. The exact tier now also compares the diacritic-folded id, and the trigram index carries the folded form so a non-ASCII id survives the prefilter. Only ids that previously failed to resolve can now resolve — label queries are unchanged, and an all-ASCII graph indexes byte-identically to before. ## 0.9.37 (2026-08-08) diff --git a/graphify/serve.py b/graphify/serve.py index 3b205d84..e899c32c 100644 --- a/graphify/serve.py +++ b/graphify/serve.py @@ -322,6 +322,14 @@ def _node_search_text(data: dict, nid: str) -> str: - `source_tokens` feeds _find_node's exact source-file path lookup, where a query like "app/api/example/route.ts" tokenizes to "app api example route ts". - `nid` feeds the whole-query `joined == nid_lower` tier. + - a trailing diacritic-folded `nid` feeds _find_node's `norm_query == nid_norm` + tier. Every query path folds through `_strip_diacritics` (NFKD), so a raw-only + id field leaves the needle and the posting under different normal forms and + the node is dropped before any predicate runs (#2467). Hangul is the common + case: NFKD decomposes a syllable into conjoining jamo, which have combining + class 0 and therefore survive the combining-character filter. The field is + appended only when the fold actually differs, so the text an all-ASCII graph + indexes — and every field position the other readers rely on — is unchanged. NUL separators stop a trigram from spanning two fields (a query never contains NUL, so a cross-field trigram can never be a real match). @@ -330,7 +338,13 @@ def _node_search_text(data: dict, nid: str) -> str: label_tokens = " ".join(_search_tokens(data.get("label") or "")) source = (data.get("source_file") or "").lower() source_tokens = " ".join(_search_tokens(data.get("source_file") or "")) - return "\x00".join((norm_label, label_tokens, str(nid).lower(), source, source_tokens)) + nid_text = str(nid).lower() + fields = (norm_label, label_tokens, nid_text, source, source_tokens) + if not nid_text.isascii(): + nid_folded = _strip_diacritics(str(nid)).lower() + if nid_folded != nid_text: + fields += (nid_folded,) + return "\x00".join(fields) def _get_trigram_index(G: nx.Graph) -> dict: @@ -1180,6 +1194,8 @@ def _find_node_tiers( # `term`/`label_tokens` works when the node label tokenizes the same way, but is # fragile if `label` and `norm_label` diverge. `norm_query` matches `norm_label` # symmetrically so an exactly-typed punctuated label always resolves (#1704). + # `nid_norm` below extends that symmetry to node ids, which keep their + # punctuation too and are compared raw against the tokenized `term` (#2467). norm_query = _strip_diacritics(str(label)).lower().strip() source_exact: list[str] = [] exact: list[str] = [] @@ -1198,11 +1214,14 @@ def _find_node_tiers( label_tokens = " ".join(_search_tokens(d.get("label") or "")) source_tokens = " ".join(_search_tokens(d.get("source_file") or "")) nid_lower = nid.lower() + # `_strip_diacritics` is the identity on ASCII, so the NFKD fold is only + # paid for ids that actually carry non-ASCII text. + nid_norm = nid_lower if nid.isascii() else _strip_diacritics(nid).lower() if term == source_tokens: source_exact.append(nid) elif ( term == norm_label or term == bare_label or term == label_tokens or term == nid_lower - or norm_query == norm_label or norm_query == bare_label + or norm_query == norm_label or norm_query == bare_label or norm_query == nid_norm ): exact.append(nid) elif ( diff --git a/tests/test_serve.py b/tests/test_serve.py index 881247c4..ea49c3c7 100644 --- a/tests/test_serve.py +++ b/tests/test_serve.py @@ -1,10 +1,13 @@ """Tests for serve.py - MCP graph query helpers (no mcp package required).""" import json +import unicodedata + import pytest import networkx as nx from networkx.readwrite import json_graph from graphify.serve import ( + _strip_diacritics, _communities_from_graph, _score_nodes, _score_query, @@ -225,6 +228,32 @@ def test_find_node_resolves_when_label_and_norm_label_diverge(): assert _find_node(G, "blockStream.ts") == ["n1"] +def test_find_node_matches_punctuated_node_id_exactly(): + # #2467: the id is only ever compared against `term`, which tokenizes on \w+ + # ("concept:domain:widget" -> "concept domain widget"), so no id carrying + # punctuation could equal it. Only the symmetric `norm_query == nid_norm` + # match resolves an exactly-typed node id. + G = nx.Graph() + G.add_node("concept:domain:widget", label="Widget", norm_label="widget", + source_file="docs/domain.md", source_location="L1") + G.add_node("plain_node_id", label="Plain", norm_label="plain", + source_file="docs/plain.md", source_location="L1") + assert _find_node(G, "concept:domain:widget") == ["concept:domain:widget"] + assert _find_node(G, "plain_node_id") == ["plain_node_id"] # unpunctuated ids as before + assert _find_node(G, "Widget") == ["concept:domain:widget"] # label lookup as before + + +def test_find_node_matches_merge_graphs_namespaced_node_id(): + # #2467: `prefix_graph_for_global` namespaces every id with "::", so on a + # merged graph no node at all resolved by id — including the id that `explain` + # itself had just printed. + G = nx.Graph() + G.add_node("backend::src_server_router_go", label="Router()", + norm_label="router()", source_file="src/server/router.go", + source_location="L12") + assert _find_node(G, "backend::src_server_router_go") == ["backend::src_server_router_go"] + + # --- trigram candidate prefilter (the trigram index that shrinks the O(N) scan) --- @@ -246,6 +275,23 @@ def _make_big_graph(n: int = 150) -> nx.Graph: return G +def _make_non_ascii_id_graph(n: int = 40) -> nx.Graph: + """A graph whose ids carry Hangul, large enough that the prefilter really runs. + + The filler nodes are load-bearing: `_trigram_candidates` bails out to a full + scan when `min(present) > int(n * 0.10)`, so on a two-node graph any present + trigram trips the guard and the index path — where #2467's second defect lives — + is never exercised at all.""" + G = nx.Graph() + for i in range(n): + G.add_node(f"id{i}", label=f"item node {i}", source_file=f"pkg/item_{i}.py") + G.add_node("concept:domain:한글", label="Hangul domain", + source_file="docs/한글.md", source_location="L1") + G.add_node("문서_목록", label="DocumentList", + source_file="src/문서_목록.py", source_location="L1") + return G + + def test_trigrams_basic(): assert _trigrams("foobar") == {"foo", "oob", "oba", "bar"} assert _trigrams("ab") == {"ab"} # <3 chars -> whole string is the key @@ -263,6 +309,19 @@ def test_node_search_text_includes_all_matched_fields(): assert parts[2] == "punct" # nid assert parts[3] == "pkg/foobar.py" # source_file assert parts[4] == "pkg foobar py" # source_file tokens + assert len(parts) == 5 # no folded-id field for an ASCII id (#2467) + + +def test_node_search_text_appends_folded_non_ascii_node_id(): + # #2467: for a Hangul id the raw and folded forms differ — precomposed syllables + # against conjoining jamo. Queries are trigrammed from the folded form, so the + # index has to carry it too, appended so the other field positions do not move. + G = _make_non_ascii_id_graph() + nid = "concept:domain:한글" + parts = _node_search_text(G.nodes[nid], nid).split("\x00") + assert parts[2] == nid + assert parts[5] == _strip_diacritics(nid).lower() + assert parts[5] != parts[2] def test_trigram_candidates_fast_path_fires_for_rare_term(): @@ -310,6 +369,35 @@ def test_find_node_prefilter_is_identical_to_full_scan(monkeypatch): assert fast == full, f"_find_node prefilter diverged (order!) for {label!r}" +def test_find_node_matches_non_ascii_node_id_through_prefilter(): + # #2467: `_node_search_text` indexed the id raw while every query folds through + # `_strip_diacritics`. NFKD decomposes a Hangul syllable into conjoining jamo, + # which have combining class 0 and so survive the combining-character filter — + # the needle's trigrams and the posting's trigrams were disjoint and the node + # was dropped from the candidate list before any predicate could see it. + G = _make_non_ascii_id_graph() + for nid in ("concept:domain:한글", "문서_목록"): + assert unicodedata.normalize("NFKD", nid) != nid # fixture must stay NFKD-sensitive + needles = [" ".join(_search_tokens(nid)), _strip_diacritics(nid).lower()] + candidates = _trigram_candidates(G, needles) + assert candidates is not None # index path, not the full-scan fallback + assert nid in candidates + assert _find_node(G, nid) == [nid] + + +def test_find_node_node_id_prefilter_is_identical_to_full_scan(monkeypatch): + # #2467: an id must resolve the same way whether the candidates came from the + # trigram index or from the full scan. + G = _make_non_ascii_id_graph() + for label in ["concept:domain:한글", "문서_목록", "id7", "item node 7", + "DocumentList", "missing"]: + fast = _find_node(G, label) + _force_full_scan(monkeypatch) + full = _find_node(G, label) + monkeypatch.undo() + assert fast == full, f"_find_node prefilter diverged (order!) for {label!r}" + + def test_find_node_label_tokens_branch_covered_by_index(): # "foo bar baz" matches label "Foo.Bar:Baz" only via the tokenized label_tokens # form (the dotted/colon norm_label never contains the spaced query). The index