fix(F-15): address Opus review findings (CRITICAL + WARN)
CRITICAL: - cli.py: invert cite-boost formula: divide distance by (1 + alpha*pr) instead of multiply — high-PageRank papers now correctly rank higher - cli.py: wire graph_min_corpus_size config into graph report; warning emitted to stderr so JSON stdout stays parseable WARN: - graph.py: document that damping is ignored in small-graph uniform branch - cli.py: sort dangling citations before slicing for stable output - cli.py: replace raise typer.Exit(0) with return in empty-graph path - tests: fix *extra_args helper signatures; add cite-boost ordering test and small-corpus warning test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -47,7 +47,7 @@ def _sample_graph() -> nx.DiGraph:
|
||||
|
||||
|
||||
class TestGraphReport:
|
||||
def _run(self, graph=None, known_ids=("A", "B", "C"), *extra_args):
|
||||
def _run(self, graph=None, known_ids=("A", "B", "C"), extra_args=()):
|
||||
if graph is None:
|
||||
graph = _sample_graph()
|
||||
conn = _make_conn_with_paper_ids(*known_ids)
|
||||
@@ -56,7 +56,7 @@ class TestGraphReport:
|
||||
patch("codex.db.get_conn", side_effect=_make_conn_cm(conn)),
|
||||
patch("codex.graph.build_citation_graph", return_value=graph),
|
||||
):
|
||||
return runner.invoke(app, ["graph", "report"])
|
||||
return runner.invoke(app, ["graph", "report", *extra_args])
|
||||
|
||||
def test_exit_zero(self):
|
||||
assert self._run().exit_code == 0
|
||||
@@ -111,6 +111,12 @@ class TestGraphReport:
|
||||
result = runner.invoke(app, ["graph", "report", "--top-n", "3"])
|
||||
assert result.exit_code == 0
|
||||
|
||||
def test_small_corpus_warning_shown(self):
|
||||
# known_ids has only 3 papers (< graph_min_corpus_size default 15)
|
||||
result = self._run(known_ids=("A", "B", "C"))
|
||||
assert result.exit_code == 0
|
||||
assert "Warning" in result.output or "warning" in result.output
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# codex graph related
|
||||
@@ -118,7 +124,7 @@ class TestGraphReport:
|
||||
|
||||
|
||||
class TestGraphRelated:
|
||||
def _run(self, paper_id: str, graph=None, *extra_args):
|
||||
def _run(self, paper_id: str, graph=None, extra_args=()):
|
||||
if graph is None:
|
||||
graph = _sample_graph()
|
||||
conn = MagicMock()
|
||||
@@ -137,12 +143,12 @@ class TestGraphRelated:
|
||||
assert "No papers" in result.output
|
||||
|
||||
def test_no_results_message_when_min_shared_high(self):
|
||||
result = self._run("A", _sample_graph(), "--min-shared", "99")
|
||||
result = self._run("A", _sample_graph(), extra_args=("--min-shared", "99"))
|
||||
assert "No papers" in result.output
|
||||
|
||||
def test_related_papers_shown(self):
|
||||
# A cites {B,C}; B also cites C → shared=1 with A
|
||||
result = self._run("A", _sample_graph(), "--min-shared", "1")
|
||||
result = self._run("A", _sample_graph(), extra_args=("--min-shared", "1"))
|
||||
assert result.exit_code == 0
|
||||
# B shares C with A; should appear
|
||||
assert "B" in result.output
|
||||
@@ -154,15 +160,22 @@ class TestGraphRelated:
|
||||
|
||||
|
||||
class TestSearchCiteBoost:
|
||||
def _make_two_paper_conn(self, pr_winner: str) -> MagicMock:
|
||||
"""Two papers A and B at equal distance; pr_winner has a high PageRank score."""
|
||||
conn = MagicMock()
|
||||
conn.execute.return_value.fetchall.return_value = [
|
||||
{"id": "A", "title": "Paper A", "year": 2020, "distance": 0.5},
|
||||
{"id": "B", "title": "Paper B", "year": 2021, "distance": 0.5},
|
||||
]
|
||||
return conn
|
||||
|
||||
def test_cite_boost_exits_zero(self):
|
||||
conn = MagicMock()
|
||||
paper_rows = [
|
||||
conn.execute.return_value.fetchall.return_value = [
|
||||
{"id": "A", "title": "Paper A", "year": 2020, "distance": 0.2},
|
||||
]
|
||||
graph = _sample_graph()
|
||||
|
||||
conn.execute.return_value.fetchall.return_value = paper_rows
|
||||
|
||||
def _fake_encode(texts):
|
||||
return np.zeros((len(texts), 1024), dtype="float32")
|
||||
|
||||
@@ -174,3 +187,38 @@ class TestSearchCiteBoost:
|
||||
mock_emb.return_value.encode_dense.side_effect = _fake_encode
|
||||
result = runner.invoke(app, ["search", "paper", "hodge star", "--cite-boost"])
|
||||
assert result.exit_code == 0
|
||||
|
||||
def test_cite_boost_promotes_high_pr_paper(self):
|
||||
"""High-PR paper should appear first (lower boosted distance) via divide formula."""
|
||||
import networkx as nx
|
||||
|
||||
# Build a graph where A has high PageRank (many papers cite A)
|
||||
g = nx.DiGraph()
|
||||
for i in range(10):
|
||||
g.add_edge(f"P{i}", "A") # A is heavily cited → high PR
|
||||
g.add_edge("P0", "B") # B is barely cited → low PR
|
||||
|
||||
conn = MagicMock()
|
||||
conn.execute.return_value.fetchall.return_value = [
|
||||
{"id": "A", "title": "Hub", "year": 2020, "distance": 0.5},
|
||||
{"id": "B", "title": "Niche", "year": 2021, "distance": 0.5},
|
||||
]
|
||||
|
||||
def _fake_encode(texts):
|
||||
return np.zeros((len(texts), 1024), dtype="float32")
|
||||
|
||||
with (
|
||||
patch("codex.db.get_conn", side_effect=_make_conn_cm(conn)),
|
||||
patch("codex.embed.get_embedder") as mock_emb,
|
||||
patch("codex.graph.build_citation_graph", return_value=g),
|
||||
):
|
||||
mock_emb.return_value.encode_dense.side_effect = _fake_encode
|
||||
result = runner.invoke(app, ["search", "paper", "query", "--cite-boost"])
|
||||
|
||||
assert result.exit_code == 0
|
||||
lines = [ln for ln in result.output.strip().splitlines() if "Hub" in ln or "Niche" in ln]
|
||||
assert len(lines) == 2
|
||||
# A (Hub) should appear before B (Niche) — lower boosted distance
|
||||
hub_line = next(line for line in lines if "Hub" in line)
|
||||
niche_line = next(line for line in lines if "Niche" in line)
|
||||
assert lines.index(hub_line) < lines.index(niche_line)
|
||||
|
||||
Reference in New Issue
Block a user