Skip to content

Commit 595a607

Browse files
committed
fix(arigraph): RRF gives each leg one vote per id at its best rank
Codex P2 review on #724: the accumulation loop credited every occurrence of an id within a single ranked list with another 1/(k+rank). A leg that surfaces the same entity more than once (several relations to one node, before caller-side dedup) would double-count and swamp consensus across the other fused legs. Fix: per-list dedup — each list votes once per id at its best (first) rank. Depth still folds the shallowest across all occurrences. +2 regression tests (duplicate_id_in_one_leg_votes_once, duplicate_id_still_folds_shallowest_depth). 9/9 rrf lib tests + doctest green; fmt + clippy clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016b33swuXE23hKtqxsHu9p1
1 parent 2c87c04 commit 595a607

2 files changed

Lines changed: 43 additions & 5 deletions

File tree

‎.claude/board/AGENT_LOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
- **Task:** the inventory (#723) named RRF fusion as the D-GR-2 retrieval keystone — every ranked leg exists (`Bm25Index::rank`, `PersonalizedPageRank::ranked`, CAM-PQ) but nothing fused them. Landed the pure fusion primitive (the SAP "Practical GraphRAG" gap), ahead of G0 like `Bm25Index`/`PersonalizedPageRank`/`Communities`.
44
- **Change:** `arigraph/rrf.rs` — `reciprocal_rank_fusion(ranked_lists: &[&[ScoredId]], k) -> Vec<ScoredId>` (Cormack 2009; `Σ 1/(k+rank)`, 1-based, k=60 `DEFAULT_RRF_K`). Fuses by RANK so the per-list scores need not be commensurable (the reason it combines BM25 f64 / PPR probability / CAM-PQ i8). Deterministic (BTreeMap id-asc + stable score-desc sort); shallowest `depth` wins; returns the contract `ScoredId`. Re-exported in `arigraph/mod.rs`.
55
- **Pure/reversible:** computes a fused ranking, reads no carrier state. The WIRING into `OsintRetriever::retrieve` stays GATED on the G0 verdict (per plan §5 + STATUS_BOARD D-GR-2).
6-
- **Commit / Tests / Outcome:** commit `<pending>`; `cargo test -p lance-graph --lib -- graph::arigraph::rrf` 7/7 + doctest 1/1 green; clippy scoped `-p lance-graph --lib` clean on the addition (the 8 warnings are pre-existing `blasgraph/ndarray_bridge.rs` SIMD dead-code). Branch `claude/happy-hamilton-0azlw4` (restarted from post-#723 main).
6+
- **Commit / Tests / Outcome:** feature `1306bf6`, fmt `2c87c04`, Codex-flagged per-leg dedup fix follow-up; `cargo test -p lance-graph --lib -- graph::arigraph::rrf` 9/9 + doctest 1/1 green; clippy scoped `-p lance-graph --lib` clean on the addition (the 8 warnings are pre-existing `blasgraph/ndarray_bridge.rs` SIMD dead-code). Codex P2 (RRF must give each leg one vote per id at its best rank — a duplicated entity in one leg was double-counting) FIXED + 2 regression tests. Branch `claude/happy-hamilton-0azlw4`; PR #724.
77

88
## 2026-07-18 — GraphRAG representations inventory — 7 papers × V3 substrate matrix (6 Opus paper-readers + 1 Opus v3-harvest; main-thread synthesis)
99

‎crates/lance-graph/src/graph/arigraph/rrf.rs‎

Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
//! module only lands the algorithm, ahead of that gate, exactly as
2525
//! `Bm25Index`/`PersonalizedPageRank`/`Communities` landed as pure capabilities.
2626
27-
use std::collections::BTreeMap;
27+
use std::collections::{BTreeMap, BTreeSet};
2828

2929
use lance_graph_contract::doc_graph::ScoredId;
3030

@@ -67,11 +67,18 @@ pub fn reciprocal_rank_fusion(ranked_lists: &[&[ScoredId]], k: f64) -> Vec<Score
6767
// preserves within equal fused scores.
6868
let mut acc: BTreeMap<&str, (f64, u8)> = BTreeMap::new();
6969
for list in ranked_lists {
70+
// RRF gives each list AT MOST ONE vote per id, at its best (first) rank.
71+
// A leg that surfaces the same entity more than once (e.g. several
72+
// relations to one node, before caller-side dedup) must not stack
73+
// `1/(k+rank)` contributions and swamp consensus across the other legs.
74+
let mut voted: BTreeSet<&str> = BTreeSet::new();
7075
for (pos, item) in list.iter().enumerate() {
71-
let rank = pos as f64 + 1.0; // 1-based
7276
let entry = acc.entry(item.id.as_str()).or_insert((0.0, u8::MAX));
73-
entry.0 += 1.0 / (k + rank);
74-
entry.1 = entry.1.min(item.depth);
77+
entry.1 = entry.1.min(item.depth); // shallowest depth across all occurrences
78+
if voted.insert(item.id.as_str()) {
79+
// best-first order ⇒ the first occurrence is the best rank
80+
entry.0 += 1.0 / (k + pos as f64 + 1.0);
81+
}
7582
}
7683
}
7784
let mut fused: Vec<ScoredId> = acc
@@ -178,4 +185,35 @@ mod tests {
178185
y.iter().map(|s| s.score).collect::<Vec<_>>()
179186
);
180187
}
188+
189+
#[test]
190+
fn duplicate_id_in_one_leg_votes_once() {
191+
// A leg surfaces "a" twice (rank 1 and rank 3) plus "b" at rank 2 — as a
192+
// node with several relations would before caller-side dedup. RRF must
193+
// credit "a" ONCE at its best rank (1), not 1/(k+1)+1/(k+3).
194+
let leg = [
195+
ScoredId::new("a", 1.0, 0),
196+
ScoredId::new("b", 1.0, 0),
197+
ScoredId::new("a", 1.0, 0),
198+
];
199+
let fused = reciprocal_rank_fusion(&[&leg], DEFAULT_RRF_K);
200+
assert_eq!(fused.len(), 2);
201+
let a = fused.iter().find(|s| s.id == "a").unwrap().score;
202+
let b = fused.iter().find(|s| s.id == "b").unwrap().score;
203+
let expected_a = (1.0f64 / (DEFAULT_RRF_K + 1.0)) as f32; // best rank only
204+
assert!(
205+
(a - expected_a).abs() < 1e-7,
206+
"a double-counted: {a} vs {expected_a}"
207+
);
208+
assert!(a > b); // single best-rank vote still beats b's rank-2 vote
209+
}
210+
211+
#[test]
212+
fn duplicate_id_still_folds_shallowest_depth() {
213+
// Duplicate occurrences don't re-vote, but depth still takes the min.
214+
let leg = [ScoredId::new("a", 1.0, 5), ScoredId::new("a", 1.0, 2)];
215+
let fused = reciprocal_rank_fusion(&[&leg], DEFAULT_RRF_K);
216+
assert_eq!(fused.len(), 1);
217+
assert_eq!(fused[0].depth, 2); // min(5, 2), even though the 2nd didn't vote
218+
}
181219
}

0 commit comments

Comments
 (0)