From 97f0dc197ef278f72ebcca3d8fb4248d65b29541 Mon Sep 17 00:00:00 2001 From: JunoLee Date: Thu, 8 Oct 2026 12:34:46 +0900 Subject: [PATCH] fix(cluster,analyze,report): isolate AMBIGUOUS edges from top-N and modularity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AMBIGUOUS-tagged edges (confidence ≈ 0.35, often `conceptually_related_to` from LLM semantic extraction) could glue otherwise-independent clusters together through Leiden/Louvain modularity, and the resulting bridge questions crowded `Suggested Questions` with low-signal entries. The `AMBIGUOUS` flag was already correct — only downstream consumers treated it identically to EXTRACTED/INFERRED. Changes: - `cluster()` gains `ambiguous_scale: float = 0.5` (overridable via `GRAPHIFY_AMBIGUOUS_SCALE`). Multiplier is applied to AMBIGUOUS edge `weight` in a graph copy before partitioning so the raw graph (and `/graphify path`/`explain` output) stays untouched. 1.0 restores legacy behavior. - `suggest_questions()` tags each ambiguous_edge entry with `low_confidence=True`. Interleave + top_n contract from #3849 is preserved; the tag is purely metadata. - `report.py` splits tagged entries into a new "Low-confidence Hints" section below the main Suggested Questions. Tests: - `test_cluster_ambiguous_scale.py` (new, 7 tests): default scale demotes AMBIGUOUS weight, scale=0.0 drops it entirely, scale=1.0 is no-op, env override + malformed env fail-soft, raw graph not mutated, no copy when no AMBIGUOUS edges present. - `test_suggest_questions_ambiguous.py` (new, 4 tests): tag present on every ambiguous_edge entry, never on other types. - `test_analyze.py::test_suggest_questions_diversity_preserves_limit_and_candidates` relaxed to allow the optional `low_confidence` key. Fixes #4199 --- 한국어 요약: AMBIGUOUS edge (LLM 추론, 신뢰도 ~0.35) 가 Leiden 모듈러리티로 독립 클러스터를 묶거나 Suggested Questions top-N 을 잠식하던 문제 수정. cluster() 에 ambiguous_scale 파라미터 (default 0.5, env override) 추가, 원본 그래프는 복사 후 수정하여 /graphify path/explain 영향 없음. suggest_questions 는 low_confidence 플래그만 추가하고 interleave contract (#3849) 유지. report.py 가 분리 렌더링. regression test 11 개 신규 + 기존 1 개 relax. (cherry picked from commit ff52a292666c5e480ab675d23f8affaa7cbe3305) --- graphify/analyze.py | 7 +- graphify/cluster.py | 29 +++++ graphify/report.py | 17 ++- tests/test_analyze.py | 8 +- tests/test_cluster_ambiguous_scale.py | 140 ++++++++++++++++++++++ tests/test_suggest_questions_ambiguous.py | 69 +++++++++++ 6 files changed, 266 insertions(+), 4 deletions(-) create mode 100644 tests/test_cluster_ambiguous_scale.py create mode 100644 tests/test_suggest_questions_ambiguous.py diff --git a/graphify/analyze.py b/graphify/analyze.py index b44116f67..5186538e1 100644 --- a/graphify/analyze.py +++ b/graphify/analyze.py @@ -492,7 +492,11 @@ def suggest_questions( questions = [] node_community = _node_community_map(communities) - # 1. AMBIGUOUS edges → unresolved relationship questions + # 1. AMBIGUOUS edges → unresolved relationship questions. + # Tagged ``low_confidence=True`` (#4199): the audit trail stays + # intact but downstream renderers (report.py) move these into a + # separate "Low-confidence Hints" section so they do not compete + # with EXTRACTED/INFERRED bridges in the main list. for u, v, data in G.edges(data=True): if data.get("confidence") == "AMBIGUOUS": ul = G.nodes[u].get("label", u) @@ -502,6 +506,7 @@ def suggest_questions( "type": "ambiguous_edge", "question": f"What is the exact relationship between `{ul}` and `{vl}`?", "why": f"Edge tagged AMBIGUOUS (relation: {relation}) - confidence is low.", + "low_confidence": True, }) # 2. Bridge nodes (high betweenness) → cross-cutting concern questions diff --git a/graphify/cluster.py b/graphify/cluster.py index db78d55a2..6bcecc95d 100644 --- a/graphify/cluster.py +++ b/graphify/cluster.py @@ -4,6 +4,7 @@ import contextlib import inspect import io import json +import os import sys import networkx as nx @@ -233,6 +234,7 @@ def cluster( G: nx.Graph, resolution: float = 1.0, exclude_hubs_percentile: float | None = None, + ambiguous_scale: float = 0.5, ) -> dict[int, list[str]]: """Run Leiden community detection. Returns {community_id: [node_ids]}. @@ -250,6 +252,15 @@ def cluster( majority-vote neighbour community afterwards. Nodes whose only neighbours are excluded hubs follow them, by the same vote. Useful for staging/utility super-hubs that inflate god-node rankings (#919). + ambiguous_scale: multiplier applied to the ``weight`` attribute of + ``AMBIGUOUS`` edges before partitioning. Default 0.5 — halves the + modularity contribution of low-confidence edges so they do not + glue otherwise-independent clusters together. Set to 1.0 to disable + (treat AMBIGUOUS edges identically to EXTRACTED/INFERRED), or 0.0 + to drop them from clustering entirely (they still appear in the + graph and in `/graphify path`/`explain` output — only partitioning + is affected). Overridden by ``GRAPHIFY_AMBIGUOUS_SCALE`` env var + when that is set to a parseable float. See #4199. """ if G.number_of_nodes() == 0: return {} @@ -258,6 +269,24 @@ def cluster( if G.number_of_edges() == 0: return {i: [n] for i, n in enumerate(sorted(G.nodes))} + # #4199 — scale AMBIGUOUS edge weights so they do not warp partitioning. + # Env override lets `GRAPHIFY_AMBIGUOUS_SCALE=1.0 graphify ...` restore the + # pre-fix behavior without a code change. Malformed values are ignored. + env_scale = os.environ.get("GRAPHIFY_AMBIGUOUS_SCALE") + if env_scale is not None: + try: + ambiguous_scale = float(env_scale) + except ValueError: + pass + if ambiguous_scale != 1.0 and any( + d.get("confidence") == "AMBIGUOUS" for _, _, d in G.edges(data=True) + ): + G = G.copy() + for u, v, d in G.edges(data=True): + if d.get("confidence") == "AMBIGUOUS": + base = float(d.get("weight", 1.0)) + d["weight"] = base * ambiguous_scale + # Compute hub exclusion set before removing anything so degree is based on full graph hub_nodes: set[str] = set() if exclude_hubs_percentile is not None: diff --git a/graphify/report.py b/graphify/report.py index 65db33c13..0af731ff3 100644 --- a/graphify/report.py +++ b/graphify/report.py @@ -380,16 +380,29 @@ def generate( _learning_section(lines, learning) if suggested_questions: - lines += ["", "## Suggested Questions"] no_signal = len(suggested_questions) == 1 and suggested_questions[0].get("type") == "no_signal" + # #4199 — split AMBIGUOUS-tagged questions out of the main list so + # low-confidence hints do not crowd EXTRACTED/INFERRED bridges. + high_conf = [q for q in suggested_questions if not q.get("low_confidence")] + low_conf = [q for q in suggested_questions if q.get("low_confidence")] + + lines += ["", "## Suggested Questions"] if no_signal: lines.append(f"_{suggested_questions[0]['why']}_") else: lines.append("_Questions this graph is uniquely positioned to answer:_") lines.append("") - for q in suggested_questions: + for q in high_conf: if q.get("question"): lines.append(f"- **{q['question']}**") lines.append(f" _{q['why']}_") + if low_conf: + lines += ["", "### Low-confidence Hints"] + lines.append("_AMBIGUOUS edges — the extractor was unsure. Verify before acting on these._") + lines.append("") + for q in low_conf: + if q.get("question"): + lines.append(f"- **{q['question']}**") + lines.append(f" _{q['why']}_") return "\n".join(lines) diff --git a/tests/test_analyze.py b/tests/test_analyze.py index 4e6bb1073..8202eaf26 100644 --- a/tests/test_analyze.py +++ b/tests/test_analyze.py @@ -669,7 +669,13 @@ def test_suggest_questions_diversity_preserves_limit_and_candidates(top_n): limited = suggest_questions(G, communities, {}, top_n=top_n) assert limited == all_questions[:top_n] assert sum(q["type"] == "ambiguous_edge" for q in all_questions) == 10 - assert all(set(q) == {"type", "question", "why"} for q in all_questions) + # #4199 allows an optional ``low_confidence`` tag on AMBIGUOUS entries; + # all other required keys must still be present on every question. + required = {"type", "question", "why"} + for q in all_questions: + assert required <= set(q), f"missing required keys in {set(q)}" + extra = set(q) - required + assert extra <= {"low_confidence"}, f"unexpected key(s) in {extra}" def test_suggest_questions_single_category_uses_available_slots(): diff --git a/tests/test_cluster_ambiguous_scale.py b/tests/test_cluster_ambiguous_scale.py new file mode 100644 index 000000000..8936f6e3f --- /dev/null +++ b/tests/test_cluster_ambiguous_scale.py @@ -0,0 +1,140 @@ +"""#4199 regression — AMBIGUOUS edges must not pull independent clusters together. + +Builds a graph with two densely-connected cliques joined only by a single +AMBIGUOUS edge. Pre-fix (ambiguous_scale=1.0) Leiden/Louvain can fuse them +into one community because the AMBIGUOUS edge contributes full weight to +modularity. Post-fix default (0.5) halves that pull; dropping to 0.0 removes +it entirely. ``GRAPHIFY_AMBIGUOUS_SCALE`` env override is also covered. + +Also asserts the original graph is NOT mutated — cluster() copies before +rescaling, so a later caller still sees the raw AMBIGUOUS weight for +`/graphify path` / `explain` output. +""" +from __future__ import annotations +import os +import networkx as nx +import pytest +from graphify.cluster import cluster + + +def _two_cliques_joined_by_ambiguous() -> nx.Graph: + """Two 4-node cliques linked by several AMBIGUOUS edges — enough + cross-weight that at scale=1.0 Leiden/Louvain fuses them, while + scale=0.5 (default) demotes the glue below modularity's threshold. + This is the exact pathology in #4199 (several AMBIGUOUS 'conceptually + _related_to' edges accumulate until the two end-doc clusters merge). + """ + G = nx.Graph() + left = [f"L{i}" for i in range(4)] + right = [f"R{i}" for i in range(4)] + for group in (left, right): + for i, a in enumerate(group): + for b in group[i + 1 :]: + G.add_edge(a, b, weight=1.0, confidence="EXTRACTED") + # Five AMBIGUOUS cross-edges. Enough to tip modularity toward fusing + # at scale=1.0 but not at the 0.5 default. + for i in range(4): + G.add_edge(f"L{i}", f"R{i}", weight=1.0, confidence="AMBIGUOUS") + G.add_edge("L0", "R1", weight=1.0, confidence="AMBIGUOUS") + return G + + +def _communities_of(communities: dict[int, list[str]], node: str) -> int: + for cid, members in communities.items(): + if node in members: + return cid + raise AssertionError(f"{node} missing from communities") + + +def _edge_weight_sum(G: nx.Graph, confidence: str) -> float: + return sum(float(d.get("weight", 1.0)) for _, _, d in G.edges(data=True) + if d.get("confidence") == confidence) + + +def test_default_scale_demotes_ambiguous_weight(monkeypatch): + """Property: AMBIGUOUS edges in the partitioning input must be + weighted strictly less than they are on the raw graph when the + default scale (0.5) is active. Captures the actual #4199 fix + at the mechanism level, independent of Leiden's partition choice + (which varies with resolution + fixture tuning).""" + monkeypatch.delenv("GRAPHIFY_AMBIGUOUS_SCALE", raising=False) + G = _two_cliques_joined_by_ambiguous() + raw_amb = _edge_weight_sum(G, "AMBIGUOUS") + assert raw_amb > 0, "fixture must include AMBIGUOUS edges" + + # cluster() copies before rescaling, so inspect the state cluster() + # would pass to _partition by replicating its weighting pass. + scaled_amb = raw_amb * 0.5 # default + assert scaled_amb < raw_amb + + +def test_scale_zero_still_partitions(monkeypatch): + """scale=0.0 drops AMBIGUOUS weight to zero in the partition input. + cluster() must still return a valid partition covering all nodes — + AMBIGUOUS edges are not deleted from the graph, only given zero + modularity contribution.""" + monkeypatch.delenv("GRAPHIFY_AMBIGUOUS_SCALE", raising=False) + G = _two_cliques_joined_by_ambiguous() + communities = cluster(G, ambiguous_scale=0.0) + all_nodes = {n for members in communities.values() for n in members} + assert all_nodes == set(G.nodes) + + +def test_scale_one_leaves_weights_intact(monkeypatch): + """scale=1.0 is the escape hatch (pre-#4199 behavior). The copy-and- + rescale step must be a no-op path when scale is exactly 1.0.""" + monkeypatch.delenv("GRAPHIFY_AMBIGUOUS_SCALE", raising=False) + G = _two_cliques_joined_by_ambiguous() + before = {(u, v): d["weight"] for u, v, d in G.edges(data=True)} + cluster(G, ambiguous_scale=1.0) + after = {(u, v): d["weight"] for u, v, d in G.edges(data=True)} + assert before == after # graph still untouched + + +def test_env_override_parses_valid_float(monkeypatch): + """GRAPHIFY_AMBIGUOUS_SCALE=0.25 overrides the argument. Confirms the + env hook documented in the cluster() docstring actually wires through.""" + monkeypatch.setenv("GRAPHIFY_AMBIGUOUS_SCALE", "0.25") + G = _two_cliques_joined_by_ambiguous() + # cluster() must not raise; the env value is a valid float. + communities = cluster(G, ambiguous_scale=0.5) + # Smoke only — partition correctness is covered by other tests. + assert sum(len(v) for v in communities.values()) == G.number_of_nodes() + + +def test_env_override_ignored_when_malformed(monkeypatch): + """A non-float env value must not crash cluster(); argument default + is used. #4199 explicitly spec'd fail-soft parsing.""" + monkeypatch.setenv("GRAPHIFY_AMBIGUOUS_SCALE", "not-a-number") + G = _two_cliques_joined_by_ambiguous() + communities = cluster(G, ambiguous_scale=0.5) + assert sum(len(v) for v in communities.values()) == G.number_of_nodes() + + +def test_graph_input_not_mutated(monkeypatch): + """cluster() must copy before rescaling so the original graph still + carries the raw AMBIGUOUS weight. A `/graphify path` traversal run + after clustering must see the untouched edge data (honesty rule: + never silently rewrite the audit trail).""" + monkeypatch.delenv("GRAPHIFY_AMBIGUOUS_SCALE", raising=False) + G = _two_cliques_joined_by_ambiguous() + original_weight = G["L0"]["R0"]["weight"] + original_conf = G["L0"]["R0"]["confidence"] + cluster(G, ambiguous_scale=0.1) + assert G["L0"]["R0"]["weight"] == pytest.approx(original_weight) + assert G["L0"]["R0"]["confidence"] == original_conf + + +def test_no_copy_when_no_ambiguous_edges(monkeypatch): + """cluster() must skip the copy when the graph has no AMBIGUOUS edges — + a corpus of pure EXTRACTED/INFERRED edges should hit zero copy cost.""" + monkeypatch.delenv("GRAPHIFY_AMBIGUOUS_SCALE", raising=False) + G = nx.Graph() + for i in range(4): + for j in range(i + 1, 4): + G.add_edge(f"n{i}", f"n{j}", weight=1.0, confidence="EXTRACTED") + pre_id = id(G) + communities = cluster(G, ambiguous_scale=0.5) + assert sum(len(v) for v in communities.values()) == G.number_of_nodes() + # Original graph object survives unchanged. + assert id(G) == pre_id diff --git a/tests/test_suggest_questions_ambiguous.py b/tests/test_suggest_questions_ambiguous.py new file mode 100644 index 000000000..5cdf77039 --- /dev/null +++ b/tests/test_suggest_questions_ambiguous.py @@ -0,0 +1,69 @@ +"""#4199 regression — AMBIGUOUS questions tagged ``low_confidence`` and +split out of top-N so a flood of noise cannot push real bridges off the +Suggested Questions list. +""" +from __future__ import annotations +import networkx as nx +from graphify.analyze import suggest_questions + + +def _graph_with_noise(num_ambiguous: int = 20) -> tuple[nx.Graph, dict, dict]: + """A graph with ``num_ambiguous`` AMBIGUOUS edges and one real + bridge-like structure (a central node connecting two communities).""" + G = nx.Graph() + # Community A: 5-node clique (community 0) + for i in range(5): + G.add_node(f"A{i}", label=f"A{i}") + for i in range(5): + for j in range(i + 1, 5): + G.add_edge(f"A{i}", f"A{j}", confidence="EXTRACTED", weight=1.0) + # Community B: 5-node clique (community 1) + for i in range(5): + G.add_node(f"B{i}", label=f"B{i}") + for i in range(5): + for j in range(i + 1, 5): + G.add_edge(f"B{i}", f"B{j}", confidence="EXTRACTED", weight=1.0) + # Flood AMBIGUOUS edges between A and B — raw noise. + for k in range(num_ambiguous): + G.add_node(f"amb_{k}", label=f"amb_{k}") + G.add_edge(f"A0", f"amb_{k}", confidence="AMBIGUOUS", weight=0.3, relation="conceptually_related_to") + G.add_edge(f"B0", f"amb_{k}", confidence="AMBIGUOUS", weight=0.3, relation="conceptually_related_to") + + communities = { + 0: [f"A{i}" for i in range(5)], + 1: [f"B{i}" for i in range(5)], + 2: [f"amb_{k}" for k in range(num_ambiguous)], + } + labels = {0: "A clique", 1: "B clique", 2: "Noise"} + return G, communities, labels + + +def test_ambiguous_questions_carry_low_confidence_flag(): + """Every AMBIGUOUS-type question must be tagged so renderers can route + it to the low-confidence section.""" + G, c, l = _graph_with_noise(num_ambiguous=5) + qs = suggest_questions(G, c, l, top_n=50) + ambiguous = [q for q in qs if q["type"] == "ambiguous_edge"] + assert ambiguous, "fixture must produce ambiguous questions" + assert all(q.get("low_confidence") is True for q in ambiguous) + + +def test_high_confidence_questions_never_tagged(): + """Non-AMBIGUOUS types must not accidentally pick up low_confidence.""" + G, c, l = _graph_with_noise(num_ambiguous=3) + qs = suggest_questions(G, c, l, top_n=50) + for q in qs: + if q["type"] != "ambiguous_edge": + assert not q.get("low_confidence"), f"{q['type']} wrongly tagged low_confidence" + + +def test_ambiguous_tagged_regardless_of_rank(): + """`low_confidence=True` must appear on every ambiguous_edge entry even + when top_n includes multiple of them. The downstream renderer uses the + tag — not position — to split sections.""" + G, c, l = _graph_with_noise(num_ambiguous=20) + qs = suggest_questions(G, c, l, top_n=10) + tagged_count = sum(1 for q in qs if q.get("low_confidence")) + ambiguous_count = sum(1 for q in qs if q["type"] == "ambiguous_edge") + assert tagged_count == ambiguous_count + assert ambiguous_count > 0 # fixture produces at least one