mirror of
https://github.com/safishamsi/graphify.git
synced 2026-10-13 07:29:33 +00:00
fix(cluster,analyze,report): isolate AMBIGUOUS edges from top-N and modularity
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 ff52a29266)
This commit is contained in:
1 parent
4cf3dfba06
commit
97f0dc197e
6 files changed
+266
-4
No files matched your search
+6
-1
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
+15
-2
@@ -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)
|
||||
@@ -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():
|
||||
|
||||
@@ -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
|
||||
@@ -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
|
||||
Reference in new issue
Block a user