diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index df2bd05..8d15f48 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -89,6 +89,8 @@ Two obligations, and the second is the one that gets skipped: 1. **Stamp it.** A result file records the model ids and dimensions it ran with, alongside the `git_sha` it already carries. A number you cannot attribute to a stack is not evidence. + `tools/eval_common.append_baseline` does this for every row it writes (ADR-055): the embedder id and + dimension, the reranker id and whether it was on, and the summarizer id, from the run's own config. 2. **Invalidate it, in the same PR as the swap.** Do not leave a superseded baseline reading as current. Add the caveat where the *verdict* lives — the ADR — not only in the results file, since the ADR is what people quote. diff --git a/docs/adr-backlog.md b/docs/adr-backlog.md index 768c81a..bb01fad 100644 --- a/docs/adr-backlog.md +++ b/docs/adr-backlog.md @@ -148,8 +148,8 @@ retrieval playground. Honor the "no web server / no build pipeline" constraint ( community recompute (or house in ADR-010). Sources: modernization P5; design-doc A1/A2. - **Amend ADR-005 (Versioning/Self-Healing):** ADR-010's content-hash drift layer is the natural companion; standardize the MD5/SHA-256 split onto one hash (XXH3) while there. -- **Amend ADR-004 (Tiers):** fix the "150+" → "66 (claimed)" competitor figure; A3 graded confidence is the - evolution of the `candidate` boolean. +- **Amend ADR-017 (Tiers, formerly draft ADR-004):** A3 graded confidence is the evolution of the `candidate` + boolean. The "150+" → "66 (claimed)" competitor-figure fix is done: ADR-017 says 66, and ADR-008 states it. --- @@ -175,7 +175,7 @@ MRR@10/NDCG@10/Recall@k + tokens/tool-calls; baseline checked in. **Wave 1 — ROI + moat (parallelizable).** `ADR-009` (modernization) and `ADR-008`+`ADR-011` (conformance + the resolver that earns the precision). *Why here:* 009 is the cheapest quality win; 008/011 build the differentiation. -Also fold in **amend-ADR-004** (the "150+"→"66" fix + graded confidence) since 008/011 touch `Edge`. **Gate:** +Also fold in **amend-ADR-017** (graded confidence; the "150+"→"66" fix is done) since 008/011 touch `Edge`. **Gate:** CoIR shows measurable lift over the Wave-0 baseline **and** a per-language precision/recall table is published. **Wave 2 — Robustness.** `ADR-010` (drift). Fold in **amend-ADR-005** (hash standardization). **Gate:** cold-start diff --git a/docs/adr/ADR-055-one-result-per-text.md b/docs/adr/ADR-055-one-result-per-text.md new file mode 100644 index 0000000..ea036f0 --- /dev/null +++ b/docs/adr/ADR-055-one-result-per-text.md @@ -0,0 +1,225 @@ +# ADR-055: One Result per Text, and Eval Rows That Name Their Stack + +**Status:** accepted +**Date:** 2026-10-20 +**Branch:** `feature/adr-055-dedupe-results` +**Reviewer:** @edb +**Backlog:** [B-010](../backlog.md#b-010) (read-time dedup only), [B-009](../backlog.md#b-009), [B-005](../backlog.md#b-005) +**Depends on:** none +**Depended on by:** none yet. [B-011](../backlog.md#b-011) (reinforcement) and B-010's ingestion-time gate build on the dedup key chosen here. + +## Context + +**B-010: the same text comes back twice.** +- When a file is smaller than the tier-2 window, its tier-2 and tier-3 windows are both the whole + file. Both are embedded and stored under different FAISS ids (the tier name is part of + `stable_id`), so both can be retrieved. +- On 2026-07-27, about a fifth of the returned slots were such a twin: `src/stable_id.py`'s 3,794 + characters at ranks 2 and 3 of a top-5. +- With summaries on (ADR-030), `_fuse_summaries` already keeps one chunk per `(file, scope without + _part_N)`, which catches the twins whose scope labels match. Without summaries, and in + `_rerank`'s cut, nothing collapses them. + +**B-009: an eval row can't say what produced it.** +- `benchmarks/real_repo/*.jsonl` rows carry `git_sha` but no model ids, and `git_sha` is + `"unknown"` on the authoritative n=148 cloud run. +- So every reranker number was found, two hours of archaeology later, to predate the + `bge-code-v1` swap. `CONTRIBUTING.md` §4.2 now asks for the stack by convention; this makes the + harness write it. + +**B-005:** two research-doc pointers still describe a closed question as open. + +## Decision + +### 1. Collapse identical text in one file, before the cut (B-010) + +`hybrid_retriever._dedupe_same_text()` runs over the ranked list before every top-k cut: the three +exits of `_rerank` and the final cut in `_fuse_summaries`. +- **Key:** `(file, text)`. The dict hashes the text, so this is a content hash with no collision + risk; the strings are already in memory. Byte-identical only, no whitespace normalization. +- **Survivor:** the hit from the most specific tier (tier 1, then 2, then 3). It takes the best + score and the best position in its group, so collapsing never demotes the text. +- **Refill:** the cut comes after, so the freed slot goes to the next distinct candidate. A caller + asking for 5 gets 5 different texts when the pool has them. +- **Order:** every other chunk keeps its relative order. +- **Empty text is never a match.** A chunk with no text says nothing about its content (the + existing tests build chunks with `text=""`), so it is kept as it is. + +### 2. Identical text in different files is kept, both copies (B-010) + +The backlog wants cross-file copies caught too. **This branch does not collapse them.** +- `find_similar_code` exists to find copies. Collapsing the second file would delete the answer. +- `detect_pattern_violations`, `trace_data_flow`, `find_dead_code` and `find_test_coverage` + report per file. A copy-pasted violating file would vanish from the report. +- An "also in: …" note on the survivor would keep the path in the retriever, but nothing would + show it: every tool renders `RetrievedChunk`s itself, in `MCPServer.py`, which this branch does + not edit. The path would be dropped in practice. + +So the dedup key includes the file. Cross-file collapse is an open question for @edb (below). + +### 3. Every eval row names its stack (B-009) + +`tools/eval_common.append_baseline` stamps each row with `embedder_model_id`, `embedder_dim`, +`reranker_model_id`, `reranker_enabled` and `summarizer_model_id` (`stack_provenance()`), and, at +@edb's request (open question 3), `summarization_enabled` and the `[retrieval]` fusion settings: +`fusion_mode`, `dense_weight`, `sparse_weight`, `summary_weight` and `file_chunk_weight`. They +change results as much as a model id does. +- The values come from `load_indexer_config()`, the same call (from the same working directory) + that `core.py` and `HybridRetriever` make during the run. A caller may pass `config=` instead. +- A missing key falls back to the code default the run itself would use (`core._DEFAULT_*`, + `hybrid_retriever._DEFAULT_RERANKER_*` and `_DEFAULT_*` fusion constants, + `config.DEFAULT_SUMMARIZER_MODEL_ID`, `config.DEFAULT_SUMMARIZATION_ENABLED`), not to a third + copy of the defaults. +- **A field the record already has wins.** The reranker is switched per arm, not by config: the + real-repo arm C turns it on whatever `[reranker].enabled` says. So the two harnesses set + `reranker_enabled` themselves (one line each, see the log), and the config value is only the + fallback. The real-repo harness does the same for `fusion_mode`, which arm D sets to `convex`. + It records the mode the arm asked for: if `rank_bm25` is missing, `HybridRetriever` falls back + to `rrf` and prints so, and the row does not see that. +- Existing `benchmarks/` files are not rewritten: an old row cannot be stamped honestly after the + fact. New rows carry the fields from the next run on. + +### 4. Two stale pointers (B-005) + +- `study-codebase-memory-mcp.md`: the ⚠️ fact-check flag becomes a note that it is discharged. +- `adr-backlog.md`: the amendment is against ADR-017, the tier model's home, and the "150+" fix is + marked done. ADR-017 already says 66, and ADR-008 states the correction. + +### 5. The iterative retriever uses the same identity (added at @edb's request, open question 2) + +`iterative_retriever.py` drops chunks it has already seen from later rounds. It keyed that on +`scope` alone. A tier-2/3 scope is a positional label (`Full File_part_1`) that **every file +shares**, so once one file's whole-file chunk had been seen, every other file's matching part was +dropped from rounds 2 and 3. A round made only of such chunks came back empty after the filter, +and the loop stopped as "no new chunks", as if the relevant code were exhausted. Only +`investigate_architecture(deep=True)` uses this retriever. +- **Key:** `(file, text)`, the identity §1 dedups on, so a search and its rounds agree. Empty text + falls back to `faiss_id`, as §1 never matches empty text. +- **Final pool:** `_dedupe_same_text()` over the score-sorted pool, replacing the `faiss_id` + dedup, so the one rule runs through the whole retriever. +- `RetrievalSession.explored_fqns` is renamed `explored`. It hasn't held only FQNs since tier 2/3 + existed, and nothing outside the module reads it. +- **Cost:** a deep investigation may now run a round it used to skip. It is a correctness fix, + not a measured gain: there is no eval for the iterative path. + +## Consequences + +**Better:** +- A search returns as many distinct texts as it was asked for, and the 4,000-token packing budget + of `semantic_code_search` stops paying for the same bytes twice. Existing indexes benefit with no + rebuild. +- A gold chunk's rank can only stay or rise with the dedup on the summary-free path: everything + removed is a copy of something ranked above it. +- A baseline row states its own stack, so a stale number is visible in the row. + +**Worse:** +- Vendored or copy-pasted files still take one slot per copy in `semantic_code_search`. +- A dedup pass over the pool on every call: a dict over the pool, at most `max(35, top_n)` chunks + (`top_n` is 80 for `trace_data_flow`). Negligible next to an embedding. +- With summaries on, removing a twin from the code ranking moves every chunk below it up a rank, so + their code-side RRF share grows a little against summary-only chunks. The fused order can change + in ways other than "one fewer duplicate". Not measured: there is no GPU or model here. + +**Neutral (the side effects checked):** +- **`iterative_retriever.py`.** Each round gets `top_n` distinct texts. Twins had equal scores, so + they could make the 5th-best-to-top ratio look like a plateau and stop the loop early; + confidence is now a little lower when a round had twins, which can mean an extra round. Its own + final dedup and its explored-chunk key now use the same `(file, text)` identity (Decision §5). +- **Graph expansion** runs on the semantic hits before the dedup, so its seeds and budget are + unchanged. Seeds are tier-1 FQNs, and same-file tier-1 twins are practically nonexistent. +- **`tier_projection`** is a CoIR harness setting (`tools/coir_eval.py`). CoIR runs its own FAISS + over the CoIR corpus and never calls `HybridRetriever`, so it is untouched. The real-repo harness + grades `retrieve()`'s output, and per the rank argument above its MRR can only stay or rise on + the summary-free arms. +- **`core.pack_context_safely`** has no caller in `src/` or `tools/` today. If one appears, it + receives distinct texts. `MCPServer._pack_results` (the live packer) gets the same benefit. +- The index stays homogeneous: every file still has its tier-3 chunk. That only changes with the + ingestion-time gate, which is out of scope. + +## Alternatives Considered + +| Option | Why rejected | +|--------|-------------| +| Collapse identical text across files, keep an "also in: …" list on the survivor | Breaks `find_similar_code` and the per-file scan tools, and no tool would render the list without an `MCPServer.py` change. Open question below. | +| Key on `text` alone, drop cross-file copies silently | Same breakage, with the path gone too. | +| Keep the best-scoring twin instead of the most specific tier | Twins have the same embedding, so their scores differ only by tier rank noise. The tier-2 chunk is the narrower claim about what the text is. | +| Dedupe inside `_semantic_search` | Before expansion and the category boost: the boost and the reranker could reorder afterwards, and structural hits would be missed. The cut is the place. | +| Dedupe in `MCPServer._search` | Only MCP callers would get it; the eval harness and the iterative retriever would not. The prompt and the backlog both put it in the retriever. | +| Hash with MD5, like `chunk_summaries` | A dict on the text is the same operation without a hash collision to reason about. The MD5 precedent matters if the key is ever persisted, which this one is not. | +| Ingestion-time gate (skip a tier that equals the lower one) | Out of scope: it doesn't fix existing indexes and it makes them heterogeneous. Waits on a GPU eval with B-011. | +| B-009: stamp `reranker_enabled` from config only | Wrong for arm C, which turns the reranker on by constructor argument. A row would claim the reranker was off on the run that measured it. | +| B-009: rewrite the existing `benchmarks/` rows | The stack can't be recovered honestly for those runs; that is the whole problem. | + +## Open questions for @edb + +1. **Cross-file copies in `semantic_code_search`.** Should the read-only search tool collapse + byte-identical text across files, and show "also in: a.py, b.py" on the survivor? It would need a + `retrieve(..., collapse_copies=True)` opt-in, an `also_in` field on `RetrievedChunk`, and a line + in `_pack_results`. It must stay off for `find_similar_code` and the per-file scan tools. +2. ~~**`iterative_retriever`'s scope-only `explored_fqns` key.**~~ Answered 2026-10-20: fix it + here. Decision §5. +3. ~~**B-009: should the row also carry `summarization_enabled`** and the fusion knobs?~~ Answered + 2026-10-20: yes. Decision §3. + +## Implementation Log + +- [x] ADR (this file), first commit on the branch +- [x] B-010: `_dedupe_same_text` before the three cuts in `_rerank` and the final cut in + `_fuse_summaries`. In `_fuse_summaries` the fused list is now built in full (≤ ~60 chunks) + before the cut, so the dedup runs ahead of the existing ADR-030 part filter. +- [x] B-010 tests, `tests/test_dedupe_results.py` (9): + - a tier-2/tier-3 twin pair becomes one tier-2 hit at the pair's best score, and the slot refills; + - the tier-2 hit wins when it ranks first too; a tier-1 chunk equal to its whole file stands for all three; + - identical text in two files keeps both copies; + - distinct texts keep their order, and a collapse leaves every other hit in order; + - empty text never matches; + - the reranked path collapses too; + - a twin that arrives only through its summary, under another scope label, doesn't take a second slot. +- [x] B-009: `stack_provenance()` and the `append_baseline` stamp; tests in `tests/test_eval_common.py` + (3): a temp `indexer.toml` with non-default ids reaches the row through the working directory; a + field the record sets wins; a missing key reports the code default. +- [x] B-009, outside this ADR's file list (smallest change, both files are owned by no session): + - `tools/real_repo_eval.py`: per-arm rows record `ARMS[arm]["reranker_enabled"]`; lift rows + record it when both arms agree and `None` when they differ (the reranker lift); + - `tools/coir_eval.py`: rows record `embedder_model_id` and `reranker_model_id` from the + harness's own `load_config()` (it loads its models from `_ROOT/indexer.toml` with its own + fallbacks, not through `core.py`), and `reranker_enabled = reranker is not None`. +- [x] `CONTRIBUTING.md` §4.2: one line saying the harness stamps each row. +- [x] B-005: the two doc pointers, in their own commit. `adr-backlog.md`'s lines had moved to 151 and + 178 since the backlog item was written. +- [x] Gates, on CPU, Python 3.12.3, no model and no network to huggingface.co (`HF_HUB_OFFLINE=1`): + - flake8 on `src/`: clean; + - `pytest --co`: 716 collected (704 on `871b55c`), no errors; + - `pytest`: 666 passed, 2 skipped, 48 failed. `871b55c` gives 654 passed, 2 skipped and the + **same 48 failures**, all from the HuggingFace tokenizer the chunker needs, which isn't + cached here (31 direct `OSError`/`LocalEntryNotFoundError`, the rest are files that then + failed to index); + - `conformance_eval.py --check-baseline`, `resolution_eval.py --check-baseline`, + `l5x_fixture_teeth.py --check`: all exit 0. +- [x] MCP Inspector: not run. No tool, signature or docstring changed; the dedup is inside + `retrieve()`. +- [ ] **Not measured:** the duplicate rate before and after on a real index. The tier-2/3 windows + are cut by the embedder's tokenizer, which isn't available here, so not even a model-free count + of twin files was possible. The 2026-07-27 figure (about a fifth of the slots; `BAAI/bge-code-v1`, + dim 1536, reranker off, `fusion_mode = "rrf"`, 98-file index of this repo) is the only number. + Worth re-running those four queries on the next GPU session. + +- [x] Follow-up after @edb answered open questions 2 and 3 (2026-10-20, local, not the cloud + session): + - `stack_provenance()` adds `summarization_enabled`, `fusion_mode`, `dense_weight`, + `sparse_weight`, `summary_weight` and `file_chunk_weight`; `tools/real_repo_eval.py` records + each arm's `fusion_mode` (a lift row gets `None` when the pair differs). Tests extended in + `tests/test_eval_common.py`. + - `iterative_retriever.py` keyed on `(file, text)` (Decision §5). + `tests/test_iterative_explored.py` (5): 4 fail on `10e89fd` and pass with the fix; the fifth + (symbol scopes unaffected) passes on both. + - Gates on Windows, Python 3.14.4, CPU, tokenizer cached (`HF_HUB_OFFLINE=1`): `pytest` 720 + passed, 1 skipped, **0 failed**, so the 48 cloud failures above were the missing tokenizer + only; flake8 on `src/` clean; `conformance_eval.py` and `resolution_eval.py` + `--check-baseline` exit 0. + +**Notes:** +- 2026-10-20: chose `(file, text)` over `text` as the key once it was clear `find_similar_code` + and `detect_pattern_violations` read `retrieve()`'s output directly. See Decision §2. +- 2026-10-20: `pip install torch --index-url https://download.pytorch.org/whl/cpu` was blocked + here; `pip install -e ".[dev]"` pulled torch from PyPI instead. diff --git a/docs/study-codebase-memory-mcp.md b/docs/study-codebase-memory-mcp.md index d3703a3..a773543 100644 --- a/docs/study-codebase-memory-mcp.md +++ b/docs/study-codebase-memory-mcp.md @@ -338,9 +338,8 @@ tool calls across 31 languages; plus an MCP supply-chain verification pipeline; - **Convergence on the honesty principle.** Their per-edge confidence ≈ ADR-004's candidate contract ≈ ADR-006's EXTRACTED-only provenance. Three independent expressions of the same "don't assert structure you can't prove" rule. -- **⚠️ Fact-check flag:** the paper claims **66 languages**, but ADR-004 attributes **"150+"** to - `codebase-memory-mcp`. Either the ADR cited marketing/an older number, or conflated it with another tool. - Worth correcting the ADR to "66 (claimed)" so the comparison is defensible. +- **Fact-check (discharged):** the paper claims **66 languages**; an early draft cited "150+", the figure in + their README. The tier ADR (renumbered ADR-004 → ADR-017) says 66, and ADR-008 states the correction. ### 9.2 ADR-005 (Chunk Versioning + Self-Healing) — two re-index axes; ours is the one they lack diff --git a/src/hybrid_retriever.py b/src/hybrid_retriever.py index 085f721..38a19da 100644 --- a/src/hybrid_retriever.py +++ b/src/hybrid_retriever.py @@ -79,6 +79,8 @@ _TIER1_NAME = "tier1_surgical" _TIER2_NAME = "tier2_component" _TIER3_NAME = "tier3_architectural" +# ADR-055: when two hits in one file have the same text, the more specific tier stands. +_TIER_ORDER = {_TIER1_NAME: 1, _TIER2_NAME: 2, _TIER3_NAME: 3} # --------------------------------------------------------------------------- # Config defaults (ADR-026 §8) — these MUST equal the values shipped in @@ -129,6 +131,35 @@ def _is_fqn(scope: str) -> bool: return "::" in scope +def _dedupe_same_text(ranked: list[RetrievedChunk]) -> list[RetrievedChunk]: + """One hit per text in a file, best first (ADR-055, B-010). Run before a top-k cut. + + A file smaller than the tier-2 window has identical tier-2 and tier-3 chunks, and + both were returned: about a fifth of the slots on 2026-07-27. The hit from the most + specific tier stands, at its group's best position and score, so the text is never + demoted and the cut refills from the next distinct hit. The file is part of the key: + find_similar_code and the per-file scan tools need every copy. Empty text says + nothing about content, so it never matches. + """ + out: list[RetrievedChunk] = [] + slot: dict[tuple[str, str], int] = {} # (file, text) → index in out + for chunk in ranked: + if not chunk.text: + out.append(chunk) + continue + key = (chunk.file, chunk.text) + i = slot.get(key) + if i is None: + slot[key] = len(out) + out.append(chunk) + continue + kept = out[i] + if _TIER_ORDER.get(chunk.tier, 9) < _TIER_ORDER.get(kept.tier, 9): + chunk.score = kept.score + out[i] = chunk + return out + + # --------------------------------------------------------------------------- # HybridRetriever # --------------------------------------------------------------------------- @@ -350,8 +381,7 @@ def _fuse_summaries(self, query: str, ranked: list[RetrievedChunk], score[fid] = score.get(fid, 0.0) + self._summary_weight / (_RRF_K + rank + 1) by_id = {c.faiss_id: c for c in ranked} - out: list[RetrievedChunk] = [] - seen: set[tuple[str, str]] = set() + fused: list[RetrievedChunk] = [] for fid in sorted(score, key=score.get, reverse=True): chunk = by_id.get(fid) if chunk is None: @@ -364,6 +394,14 @@ def _fuse_summaries(self, query: str, ranked: list[RetrievedChunk], text=meta.get("text", ""), source="summary", tags=meta.get("tags") or [], corroborated=True, ) + chunk.score = score[fid] + fused.append(chunk) + + out: list[RetrievedChunk] = [] + seen: set[tuple[str, str]] = set() + # ADR-055: a chunk found only through its summary can be the twin of one + # already ranked, so identical text collapses again before the cut. + for chunk in _dedupe_same_text(fused): # One chunk per split parent. The parts of a big class or file sit in # both lists, so without this they filled the top N as near-duplicates: # p-queue's pq-concurrency returned 10 chunks covering 2 scopes, and lost @@ -374,7 +412,6 @@ def _fuse_summaries(self, query: str, ranked: list[RetrievedChunk], if key in seen: continue seen.add(key) - chunk.score = score[fid] out.append(chunk) if len(out) == top_n: break @@ -615,7 +652,8 @@ def _rerank(self, query: str, pool: list[RetrievedChunk], """Score every candidate in `pool` against `query` via the cross-encoder. Falls back to RRF score ordering if the cross-encoder is unavailable. - Returns at most ``top_n`` chunks, highest score first. + Returns at most ``top_n`` chunks, highest score first, one per text in a + file (ADR-055). """ if not pool: return [] @@ -630,7 +668,7 @@ def _rerank(self, query: str, pool: list[RetrievedChunk], if query_categories & set(chunk.tags): chunk.score += _CATEGORY_BOOST pool.sort(key=lambda c: c.score, reverse=True) - return pool[:top_n] + return _dedupe_same_text(pool)[:top_n] pairs = [(query, c.text) for c in pool] @@ -649,7 +687,7 @@ def _rerank(self, query: str, pool: list[RetrievedChunk], exc, ) pool.sort(key=lambda c: c.score, reverse=True) - return pool[:top_n] + return _dedupe_same_text(pool)[:top_n] # Composite scoring name_mentions: dict[str, int] = {} @@ -678,4 +716,4 @@ def _rerank(self, query: str, pool: list[RetrievedChunk], chunk.score = ce_score + density_bonus + locality_bonus + category_bonus pool.sort(key=lambda c: c.score, reverse=True) - return pool[:top_n] + return _dedupe_same_text(pool)[:top_n] diff --git a/src/iterative_retriever.py b/src/iterative_retriever.py index 49534f8..79e186c 100644 --- a/src/iterative_retriever.py +++ b/src/iterative_retriever.py @@ -7,17 +7,18 @@ Each round: 1. Enriches the query with top-3 FQNs from prior evidence (context injection) - 2. Excludes already-explored FQNs so each round finds new code + 2. Excludes chunks already explored, by (file, text), so each round finds new code 3. Accumulates the new chunks into the evidence pool 4. Checks for a score plateau (confidence stopping condition) After all rounds, the full accumulated pool is deduplicated and sorted by score. +A chunk's identity is its (file, text), the key HybridRetriever dedups on (ADR-055). """ from __future__ import annotations from dataclasses import dataclass, field -from hybrid_retriever import HybridRetriever, RetrievedChunk +from hybrid_retriever import HybridRetriever, RetrievedChunk, _dedupe_same_text from db import CodeDB @@ -25,12 +26,23 @@ class RetrievalSession: """State accumulated across iteration rounds.""" query: str - explored_fqns: set[str] = field(default_factory=set) + explored: set = field(default_factory=set) # _identity() of every chunk seen evidence_pool: list[RetrievedChunk] = field(default_factory=list) iteration: int = 0 confidence: float = 0.0 # 0.0–1.0; ratio of 5th-best to top score +def _identity(chunk: RetrievedChunk): + """Which chunks count as the same evidence: (file, text), as in ADR-055. + + Not the scope: a tier-2/3 scope is a positional label (`Full File_part_1`) that + every file shares, so keying on it excluded every other file's whole-file chunks + once one had been seen, and could end the loop as "no new chunks". Empty text + says nothing about content, so it falls back to the chunk's own id. + """ + return (chunk.file, chunk.text) if chunk.text else chunk.faiss_id + + class IterativeRetriever: """ Multi-round retrieval with explored-node memory and confidence-based stopping. @@ -84,13 +96,13 @@ def retrieve( new_chunks = self._retriever.retrieve(enriched_query) new_chunks = [ c for c in new_chunks - if c.scope not in session.explored_fqns + if _identity(c) not in session.explored ] if not new_chunks: break - session.explored_fqns.update(c.scope for c in new_chunks) + session.explored.update(_identity(c) for c in new_chunks) session.evidence_pool.extend(new_chunks) # Confidence = ratio of 5th-best score to top score (plateau detection). @@ -109,13 +121,10 @@ def retrieve( if session.confidence >= confidence_threshold: break - # Final deduplicated, sorted pool - seen: set[int] = set() - deduped: list[RetrievedChunk] = [] - for c in sorted(session.evidence_pool, key=lambda c: c.score, reverse=True): - if c.faiss_id not in seen: - seen.add(c.faiss_id) - deduped.append(c) + # Final pool, best first. The explored check already keeps a text from coming + # back in a later round; this applies the retriever's own rule to the whole pool. + deduped = _dedupe_same_text( + sorted(session.evidence_pool, key=lambda c: c.score, reverse=True)) return deduped[:top_n], session diff --git a/tests/test_dedupe_results.py b/tests/test_dedupe_results.py new file mode 100644 index 0000000..fb57d3c --- /dev/null +++ b/tests/test_dedupe_results.py @@ -0,0 +1,116 @@ +"""ADR-055 (B-010): one result per text in a file, collapsed before the top-k cut. + +A file smaller than the tier-2 window has identical tier-2 and tier-3 chunks, and both +were returned. These pin: the twins become one hit from the most specific tier, the +freed slot refills from the next candidate, copies in other files are kept, and every +other hit keeps its order. No model: the retriever is built bare, as in +test_summary_index.py. +""" +import faiss +import numpy as np + +import hybrid_retriever as hr + +T1, T2, T3 = hr._TIER1_NAME, hr._TIER2_NAME, hr._TIER3_NAME +WHOLE_FILE = "def f():\n return 1\n" + + +def mk(fid, score, text, tier=T1, file="f.py", scope=None): + return hr.RetrievedChunk(faiss_id=fid, score=score, file=file, + scope=scope or f"{file}::s{fid}", tier=tier, text=text, + source="semantic") + + +def _bare(monkeypatch, reranker=None): + r = hr.HybridRetriever.__new__(hr.HybridRetriever) + r._reranker_enabled = reranker is not None + r._reranker = reranker + r._reranker_failed = False + monkeypatch.setattr(hr, "classify_query", lambda q: []) + return r + + +def test_tier2_and_tier3_twins_become_one_tier2_hit_and_the_slot_refills(monkeypatch): + r = _bare(monkeypatch) + pool = [mk(1, 0.9, WHOLE_FILE, T3, scope="Full File_part_1"), + mk(2, 0.8, WHOLE_FILE, T2, scope="Full File_part_1"), + mk(3, 0.7, "c"), mk(4, 0.6, "d")] + out = r._rerank("q", pool, top_n=3) + assert [c.faiss_id for c in out] == [2, 3, 4] # 4 refills the freed slot + assert out[0].tier == T2 + assert out[0].score == 0.9 # the text keeps its best rank + + +def test_the_more_specific_tier_wins_when_it_ranks_first_too(): + out = hr._dedupe_same_text([mk(2, 0.9, WHOLE_FILE, T2), mk(1, 0.8, WHOLE_FILE, T3)]) + assert [(c.faiss_id, c.score) for c in out] == [(2, 0.9)] + + +def test_a_tier1_chunk_equal_to_its_whole_file_stands_for_it(): + out = hr._dedupe_same_text([mk(3, 0.9, WHOLE_FILE, T3), mk(2, 0.8, WHOLE_FILE, T2), + mk(1, 0.7, WHOLE_FILE, T1)]) + assert [(c.faiss_id, c.tier, c.score) for c in out] == [(1, T1, 0.9)] + + +def test_identical_text_in_two_files_keeps_both_copies(monkeypatch): + # find_similar_code exists to find copies, and the scan tools report per file. + r = _bare(monkeypatch) + pool = [mk(1, 0.9, WHOLE_FILE, T2, file="a.py"), + mk(2, 0.8, WHOLE_FILE, T2, file="vendor/a.py"), + mk(3, 0.7, "c")] + out = r._rerank("q", pool, top_n=3) + assert [(c.faiss_id, c.file) for c in out] == [(1, "a.py"), (2, "vendor/a.py"), (3, "f.py")] + + +def test_distinct_texts_keep_their_order(): + ranked = [mk(i, 1.0 - i / 10, f"text {i}", tier) for i, tier in + enumerate([T1, T3, T2, T1, T2, T3, T1])] + assert hr._dedupe_same_text(list(ranked)) == ranked + + +def test_a_collapse_leaves_every_other_hit_in_order(): + ranked = [mk(1, 0.9, "a"), mk(2, 0.8, WHOLE_FILE, T3), mk(3, 0.7, "b"), + mk(4, 0.6, WHOLE_FILE, T2), mk(5, 0.5, "c")] + assert [c.faiss_id for c in hr._dedupe_same_text(ranked)] == [1, 4, 3, 5] + + +def test_empty_text_is_never_a_match(): + ranked = [mk(1, 0.9, "", T2), mk(2, 0.8, "", T3)] + assert [c.faiss_id for c in hr._dedupe_same_text(ranked)] == [1, 2] + + +def test_the_reranked_path_collapses_too(monkeypatch): + class Scorer: + def predict(self, pairs, batch_size): + return [1.0 if text == WHOLE_FILE else 0.5 for _, text in pairs] + + r = _bare(monkeypatch, reranker=Scorer()) + pool = [mk(1, 0.9, WHOLE_FILE, T3, scope="Full File_part_1"), + mk(2, 0.8, WHOLE_FILE, T2, scope="Full File_part_1"), + mk(3, 0.7, "c"), mk(4, 0.6, "d")] + out = r._rerank("q", pool, top_n=2) + assert len(out) == 2 and out[0].faiss_id == 2 + assert len({c.text for c in out}) == 2 + + +def test_a_summary_only_twin_does_not_take_a_second_slot(monkeypatch): + # Chunk 2 (tier 2) is ranked; its tier-3 twin 1 arrives only through its summary, + # and at a different scope label, so the ADR-030 part filter alone would keep both. + dim = 4 + q = np.zeros(dim, dtype=np.float32) + q[0] = 1.0 + r = hr.HybridRetriever.__new__(hr.HybridRetriever) + r._summary = faiss.IndexIDMap(faiss.IndexFlatIP(dim)) + r._summary.add_with_ids(np.stack([q]), np.array([1], dtype=np.int64)) + r._summary_weight = 1.0 + r._file_chunk_weight = 1.0 + + class Store: + def get(self, fid): + return {"file": "f.py", "scope": "Full File", "tier": T3, "text": WHOLE_FILE} + + r._doc_store = Store() + monkeypatch.setattr(hr, "embed", lambda text: q) + ranked = [mk(2, 1.0, WHOLE_FILE, T2, scope="Full File_part_1"), mk(3, 0.9, "c")] + out = r._fuse_summaries("q", ranked, 2) + assert [c.faiss_id for c in out] == [2, 3] diff --git a/tests/test_eval_common.py b/tests/test_eval_common.py index 8804964..a4f3970 100644 --- a/tests/test_eval_common.py +++ b/tests/test_eval_common.py @@ -82,3 +82,80 @@ def test_append_baseline_custom_key_fields(tmp_path): assert len(rows) == 2 # (B) replaced, (C) appended b = next(r for r in rows if r["arm"] == "B") assert b["mrr"] == 0.9 + + +# ── ADR-055 (B-009): every row names its stack ────────────────────────────── + +_TOML = """ +[embeddings] +model_id = "example/fake-embedder" +dimension = 384 + +[reranker] +model_id = "example/fake-reranker" +enabled = true + +[summarization] +model_id = "example/fake-summarizer" +enabled = false + +[retrieval] +fusion_mode = "convex" +dense_weight = 0.6 +sparse_weight = 0.4 +summary_weight = 0.25 +file_chunk_weight = 0.75 +""" + + +def test_append_baseline_stamps_the_stack_the_run_loaded(tmp_path, monkeypatch): + # The run's config is found from the working directory, as core.py and + # HybridRetriever find it; non-default ids prove the values come from it. + (tmp_path / "indexer.toml").write_text(_TOML, encoding="utf-8") + monkeypatch.chdir(tmp_path) + path = str(tmp_path / "out" / "b.jsonl") + ec.append_baseline({"subtask": "s1", "config": "dense", "v": 1}, path) + row = next(ec.read_jsonl(path)) + assert row["embedder_model_id"] == "example/fake-embedder" + assert row["embedder_dim"] == 384 + assert row["reranker_model_id"] == "example/fake-reranker" + assert row["reranker_enabled"] is True + assert row["summarizer_model_id"] == "example/fake-summarizer" + assert row["summarization_enabled"] is False + assert row["fusion_mode"] == "convex" + assert row["dense_weight"] == 0.6 + assert row["sparse_weight"] == 0.4 + assert row["summary_weight"] == 0.25 + assert row["file_chunk_weight"] == 0.75 + + +def test_a_field_the_record_sets_wins_over_the_config(tmp_path, monkeypatch): + # The real-repo eval turns the reranker on per arm, whatever the config says. + (tmp_path / "indexer.toml").write_text(_TOML, encoding="utf-8") + monkeypatch.chdir(tmp_path) + path = str(tmp_path / "b.jsonl") + ec.append_baseline({"repo": "click", "arm": "B", "reranker_enabled": False}, path, + ("repo", "arm")) + row = next(ec.read_jsonl(path)) + assert row["reranker_enabled"] is False + assert row["reranker_model_id"] == "example/fake-reranker" + + +def test_a_key_the_config_leaves_out_reports_the_code_default(tmp_path): + import config + import core + import hybrid_retriever as hr + stack = ec.stack_provenance({"embeddings": {"model_id": "example/fake-embedder"}}) + assert stack == { + "embedder_model_id": "example/fake-embedder", + "embedder_dim": core._DEFAULT_DIMENSION, + "reranker_model_id": hr._DEFAULT_RERANKER_MODEL_ID, + "reranker_enabled": hr._DEFAULT_RERANKER_ENABLED, + "summarizer_model_id": config.DEFAULT_SUMMARIZER_MODEL_ID, + "summarization_enabled": config.DEFAULT_SUMMARIZATION_ENABLED, + "fusion_mode": hr._DEFAULT_FUSION_MODE, + "dense_weight": hr._DEFAULT_DENSE_WEIGHT, + "sparse_weight": hr._DEFAULT_SPARSE_WEIGHT, + "summary_weight": hr._DEFAULT_SUMMARY_WEIGHT, + "file_chunk_weight": hr._DEFAULT_FILE_CHUNK_WEIGHT, + } diff --git a/tests/test_iterative_explored.py b/tests/test_iterative_explored.py new file mode 100644 index 0000000..df11e48 --- /dev/null +++ b/tests/test_iterative_explored.py @@ -0,0 +1,74 @@ +"""ADR-055: the iterative retriever knows a chunk by its (file, text), not its scope. + +A tier-2/3 scope is a positional label (`Full File_part_1`) that every file shares. +Keyed on it, one file's whole-file chunk hid every other file's from later rounds, and +a round of only such chunks ended the loop as "no new chunks". These pin: other files' +whole-file chunks survive into later rounds, a round of them doesn't stop the loop, and +a text seen in one round is not added again. No model: the base retriever is a stub +that returns a fixed list per round. +""" +import hybrid_retriever as hr +from iterative_retriever import IterativeRetriever + +T1, T2, T3 = hr._TIER1_NAME, hr._TIER2_NAME, hr._TIER3_NAME + + +def mk(fid, score, text, tier=T2, file="a.py", scope="Full File_part_1"): + return hr.RetrievedChunk(faiss_id=fid, score=score, file=file, scope=scope, + tier=tier, text=text, source="semantic") + + +class _Rounds: + """Returns the next round's hits on each call, then nothing.""" + + def __init__(self, *rounds): + self._rounds = list(rounds) + self.calls = 0 + + def retrieve(self, query): + self.calls += 1 + return self._rounds.pop(0) if self._rounds else [] + + +def _run(*rounds, **kw): + base = _Rounds(*rounds) + kw.setdefault("confidence_threshold", 2.0) # never plateau: run every round + chunks, session = IterativeRetriever(base, db=None).retrieve("q", **kw) + return chunks, session, base + + +def test_other_files_whole_file_chunks_survive_into_later_rounds(): + chunks, _, _ = _run( + [mk(1, 0.9, "a body", file="a.py")], + [mk(2, 0.8, "b body", file="b.py"), mk(3, 0.7, "c body", file="c.py")]) + assert [c.file for c in chunks] == ["a.py", "b.py", "c.py"] + + +def test_a_round_of_only_other_files_whole_file_chunks_does_not_stop_the_loop(): + _, session, base = _run( + [mk(1, 0.9, "a body", file="a.py")], + [mk(2, 0.8, "b body", file="b.py")], + [mk(3, 0.7, "c body", file="c.py", tier=T3)]) + assert base.calls == 3 and session.iteration == 3 + + +def test_a_text_seen_in_one_round_is_not_added_again(): + chunks, _, _ = _run( + [mk(1, 0.9, "same body", tier=T2)], + [mk(2, 0.95, "same body", tier=T3), mk(3, 0.5, "other", file="b.py")]) + assert [(c.faiss_id, c.file) for c in chunks] == [(1, "a.py"), (3, "b.py")] + + +def test_symbols_with_their_own_scopes_are_unaffected(): + chunks, _, _ = _run( + [mk(1, 0.9, "def f(): ...", tier=T1, scope="a.py::f")], + [mk(1, 0.9, "def f(): ...", tier=T1, scope="a.py::f"), + mk(2, 0.8, "def g(): ...", tier=T1, scope="a.py::g")]) + assert [c.scope for c in chunks] == ["a.py::f", "a.py::g"] + + +def test_empty_text_falls_back_to_the_chunk_id(): + chunks, _, _ = _run( + [mk(1, 0.9, "", file="a.py")], + [mk(2, 0.8, "", file="a.py"), mk(1, 0.7, "", file="a.py")]) + assert sorted(c.faiss_id for c in chunks) == [1, 2] diff --git a/tools/coir_eval.py b/tools/coir_eval.py index a9729d5..1d08aac 100644 --- a/tools/coir_eval.py +++ b/tools/coir_eval.py @@ -327,6 +327,10 @@ def run_subtask(task, model, reranker, cfg, limit_queries, seed, config_name="de "tier_projection": cfg["tier_projection"], "model_id": cfg["model_id"], "reranker_id": cfg["reranker_id"] if reranker is not None else None, + # ADR-055: this harness loads its own models; append_baseline stamps the rest + "embedder_model_id": cfg["model_id"], + "reranker_model_id": cfg["reranker_id"], + "reranker_enabled": reranker is not None, "fusion_weights": ({"dense": dense_w, "sparse": sparse_w} if use_sparse else None), "rerank_depth": rerank_depth if paired else None, "git_sha": _git_sha(_ROOT), diff --git a/tools/eval_common.py b/tools/eval_common.py index 6d10922..db75348 100644 --- a/tools/eval_common.py +++ b/tools/eval_common.py @@ -40,6 +40,44 @@ def git_sha(cwd=None): return "unknown" +def stack_provenance(config=None): + """The stack a run used, for stamping every baseline row (ADR-055, B-009): its + models, whether summaries were on, and the fusion settings. + + Read from ``load_indexer_config()``, the call ``core.py`` and ``HybridRetriever`` + make during the run, so the stamp cannot drift from what ran. A key the config + leaves out reports the code default the run fell back to. ``reranker_enabled`` is + the config's switch; a harness that turns the reranker on per arm records its own, + and the same goes for ``fusion_mode``. + """ + from config import (DEFAULT_SUMMARIZATION_ENABLED, DEFAULT_SUMMARIZER_MODEL_ID, + load_indexer_config) + from core import _DEFAULT_DIMENSION, _DEFAULT_MODEL_ID + from hybrid_retriever import (_DEFAULT_DENSE_WEIGHT, _DEFAULT_FILE_CHUNK_WEIGHT, + _DEFAULT_FUSION_MODE, _DEFAULT_RERANKER_ENABLED, + _DEFAULT_RERANKER_MODEL_ID, _DEFAULT_SPARSE_WEIGHT, + _DEFAULT_SUMMARY_WEIGHT) + + cfg = load_indexer_config() if config is None else config + emb = cfg.get("embeddings", {}) + rer = cfg.get("reranker", {}) + summ = cfg.get("summarization", {}) + ret = cfg.get("retrieval", {}) + return { + "embedder_model_id": emb.get("model_id", _DEFAULT_MODEL_ID), + "embedder_dim": int(emb.get("dimension", _DEFAULT_DIMENSION)), + "reranker_model_id": rer.get("model_id", _DEFAULT_RERANKER_MODEL_ID), + "reranker_enabled": bool(rer.get("enabled", _DEFAULT_RERANKER_ENABLED)), + "summarizer_model_id": str(summ.get("model_id", DEFAULT_SUMMARIZER_MODEL_ID)), + "summarization_enabled": bool(summ.get("enabled", DEFAULT_SUMMARIZATION_ENABLED)), + "fusion_mode": str(ret.get("fusion_mode", _DEFAULT_FUSION_MODE)), + "dense_weight": float(ret.get("dense_weight", _DEFAULT_DENSE_WEIGHT)), + "sparse_weight": float(ret.get("sparse_weight", _DEFAULT_SPARSE_WEIGHT)), + "summary_weight": float(ret.get("summary_weight", _DEFAULT_SUMMARY_WEIGHT)), + "file_chunk_weight": float(ret.get("file_chunk_weight", _DEFAULT_FILE_CHUNK_WEIGHT)), + } + + def dcg(gains): return sum(g / np.log2(i + 2) for i, g in enumerate(gains)) @@ -94,13 +132,17 @@ def ci95(values): return 1.96 * float(arr.std(ddof=1)) / (arr.size ** 0.5) -def append_baseline(record, path, key_fields=("subtask", "config")): +def append_baseline(record, path, key_fields=("subtask", "config"), config=None): """Append ``record`` to a JSONL baseline, replacing any row with the same key. ``key_fields`` are the record keys that identify a unique run (CoIR: subtask×config; real-repo: repo×arm×language). The file is rewritten sorted by - the key so diffs stay stable and git-friendly (§6 comparability). Returns ``path``. + the key so diffs stay stable and git-friendly (§6 comparability). The record is + stamped with ``stack_provenance(config)`` first; a field it already has wins. + Returns ``path``. """ + for k, v in stack_provenance(config).items(): + record.setdefault(k, v) existing = [] if os.path.exists(path): existing = list(read_jsonl(path)) diff --git a/tools/real_repo_eval.py b/tools/real_repo_eval.py index fb50b63..d427f12 100644 --- a/tools/real_repo_eval.py +++ b/tools/real_repo_eval.py @@ -251,7 +251,8 @@ def evaluate(repo, arms, limit=0, verbose=False): for arm in arms: rows = arm_rows[arm] rec = {"repo": name, "language": lang, "arm": arm, "n": len(rows), - "git_sha": _git_sha(_ROOT)} + "git_sha": _git_sha(_ROOT), "reranker_enabled": ARMS[arm]["reranker_enabled"], + "fusion_mode": ARMS[arm]["fusion_mode"]} for metric in ("mrr@10", "ndcg@10", "map", "recall@1", "recall@5", "recall@10", "success@1", "success@5", "success@10"): mean, ci = _mean_ci(rows, metric) @@ -268,7 +269,12 @@ def evaluate(repo, arms, limit=0, verbose=False): if x not in per_query or y not in per_query: continue rec = {"repo": name, "language": lang, "arm": f"lift:{label}", - "pair": f"{x}-{y}", "git_sha": _git_sha(_ROOT)} + "pair": f"{x}-{y}", "git_sha": _git_sha(_ROOT), + # ADR-055: None when the pair differs (the reranker lift is on − off) + "reranker_enabled": (ARMS[x]["reranker_enabled"] + if ARMS[x]["reranker_enabled"] == ARMS[y]["reranker_enabled"] else None), + "fusion_mode": (ARMS[x]["fusion_mode"] + if ARMS[x]["fusion_mode"] == ARMS[y]["fusion_mode"] else None)} for metric in LIFT_METRICS: mean, ci, n = paired_lift(per_query[x], per_query[y], metric) rec[metric] = round(mean, 4)