fix(search): cap per-file hits so a small limit spans more files - #34
Conversation
RRF fusion in HybridRanker ranks purely by position and CodeIndexService then took a flat Take(limit) off the front of that order, with no notion of which file a candidate came from. When a topic has several sibling declarations in the same file that all score highly, they can consume most of a small limit before a different, still-relevant file's chunk is ever reached, even when it ranked only a few places lower. Add ResultDiversifier: a cap-then-backfill pass that walks the fused ranking once, keeping at most 2 (default) chunks per file during the first pass and appending deferred candidates afterwards to fill any remaining slots. This never returns fewer results than a plain Take(limit) would have, and never reorders within a file - it only lets other files' candidates in ahead of a dominant file's overflow. Wire it into CodeIndexService.SearchWithStatusAsync: fuse to branchDepth (not limit) so the diversifier has headroom to draw from, then diversify down to limit.
Uniform per-file capping regressed two SearchQualityTests golden
queries ("TrustSet", "LedgerEntry"): each legitimately matches three
sibling declarations in one file via SymbolMatcher's exact/prefix/
substring bands, and capping bumped the query's own named class out
in favor of an unrelated chunk from a different, merely-still-under-
cap file - a strictly worse result, not a diversified one.
SymbolMatcher only ever matches a query that is, as a whole, a
literal substring of a chunk's symbol/signature/directory, so it
essentially never matches a natural-language sentence - the shape of
every query in the real reproduction. Exempting symbol-branch hits
from the cap therefore leaves precision identifier lookups exactly as
clustered as they earned the right to be, while still catching the
vector-only crowding the real defect is made of.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
…last Backfill runs after the capped pass, so appending a deferred candidate directly placed it behind every hit selected while it sat deferred. With three chunks from one file and one from another at limit 4, the output came back as ranks [0, 1, 3, 2]: a rank-3 hit presented below the rank-4 hit that had displaced it, while its own `score` field still said it was stronger. Selection is now recorded as flags over the input's positions and emitted in input order, so membership changes and ordering does not -- which is the only thing diversification was ever meant to do. The new test asserts both the literal expected order and the invariant behind it (scores never increase down the list), since the invariant is what a caller actually relies on.
|
Дописал одно и завёл отдельный issue на второе. Порядок выдачи. Backfill дописывал отложенного кандидата в конец, поэтому список переставал быть монотонным по релевантности: при трёх чанках одного файла и одном чужом с Кейс auto-lock — не про разнообразие, и это важное уточнение к моей же постановке задачи. Вы это показали убедительно: Отдельно отмечу правильный ход с golden-запросами: когда первая версия сломала Тесты: 419 (360 Core + 59 Server), было 408. |
1 participant
Summary
Semantic search under-retrieves peripheral files when a small
limitgets crowded out by sibling chunks from one or two central files.HybridRankerfuses purely by rank position andCodeIndexServicetook a flatTake(limit)off the front of that order, with no notion of which file a candidate came from.Measured against the real
walletproject's index (same methodology as the earlier cost/accuracy benchmark): a natural-language query about the network-unavailable UI flow put three separate members ofNetworkUnavailableModal.razor.csinto 3 of 5 result slots atlimit=5, leaving no room forXrplSharpClientService.ExecuteIfConnected— a distinct, alternate failure path (the connectivity check that throwsNotConnectedException) that ranked 7th overall but 1st among files not already represented.ResultDiversifier: a cap-then-backfill pass (default cap 2 per file) that walks the fused ranking once, keeping non-exempt candidates under the cap and backfilling deferred ones afterward. Never returns fewer results than a plainTake(limit)would have.SearchQualityTestsgolden queries ("TrustSet", "LedgerEntry") where several sibling declarations in one file are the legitimately correct multi-part answer.SymbolMatcheronly ever matches a query that is, as a whole, a literal substring of a symbol/signature/directory, so it essentially never matches a natural-language sentence — the shape of every query in the real reproduction — making this a clean, principled split between "precision identifier lookup" and "vector-only crowding."CodeIndexService.SearchWithStatusAsync: fuse tobranchDepth(notlimit) so the diversifier has headroom to draw from, then diversify down tolimit.What this does not fix
A second reproduction case (auto-lock lifecycle: the Windows-only wiring in
Platforms/Windows/App.xaml.csvs. the cross-platformApp.xaml.cs/AuthStateProvider.cs) is not a crowding defect — confirmed by restricting search to just that file viapath_filter, which still returns very few/zero hits for the exact phrasing that missed it. That file competes against more thanmaxPerFileother genuinely distinct, relevant files (iOS background handling, the settings page, the web lifecycle bridge) for the same smalllimit, so no per-file cap recovers it; it is a separate ranking/relevance-floor question, not addressed here.Test plan
ResultDiversifierTests.cscovering: crowding across multiple peripheral files, symbol-branch exemption (the exact TrustSetFlags-shaped regression), exempt candidates not consuming their file's non-exempt quota, backfill guarantees, edge cases (empty/negative limit, non-positive cap).CodeIndexServiceTests.csreproducing the file-crowding shape end-to-end throughSearchWithStatusAsync.CodeIndex.Core.Tests(including allSearchQualityTests) + 59/59CodeIndex.Server.Testspass.walletindex (907 files, 6956 chunks, real Ollama embeddings) with a before/after harness built against this commit vs. the parent commit:ExecuteIfConnected/NotConnectedExceptionnow appears in the top 5 for both network-failure reproduction queries where it previously did not.