Conversation
4 tasks
Contributor
|
Thanks for this — classic BM25 candidate loading is a real issue we want to keep fixing (classic mode is a long-lived path even though map-nav is now the default). We're landing a combined fix in #252 that keeps your strongest pieces:
It also takes the safer server-side OR tsquery + full-scan fallback from #251, and credits both of you with Closing this PR as superseded by #252 so we don't merge two conflicting approaches for #195. Please keep contributing — follow-ups welcome (classic large-namespace soak tests, metrics, etc.). |
5 tasks
Contributor
EricNGOntos
added a commit
that referenced
this pull request
Aug 11, 2026
Classic path/content channels loaded the full scoped corpus into Python before BM25. Prefetch via simple FTS (server-side OR tsquery), apply section exclusions before the candidate LIMIT, and fall back to a full scan when FTS matches nothing so recall does not regress. Combines the approaches from #244 and #251; closes #195. Co-authored-by: Ray Tien <ray.tien0907@gmail.com> Co-authored-by: nuemaan <anonnumaan@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
EricNGOntos
added a commit
that referenced
this pull request
Aug 11, 2026
* refactor: update retrieval architecture and agentic mode handling - Reorganized the retrieval flowchart to enhance clarity and structure. - Introduced new retrieval modes: classic top-K and map-nav, with clear descriptions for each. - Updated the `use_agentic` parameter to default to map-nav, simplifying user experience. - Removed legacy agentic components and related tests to streamline the codebase. - Adjusted documentation to reflect changes in retrieval modes and internal structures. * perf: bound classic BM25 candidates with Postgres FTS Classic path/content channels loaded the full scoped corpus into Python before BM25. Prefetch via simple FTS (server-side OR tsquery), apply section exclusions before the candidate LIMIT, and fall back to a full scan when FTS matches nothing so recall does not regress. Combines the approaches from #244 and #251; closes #195. Co-authored-by: Ray Tien <ray.tien0907@gmail.com> Co-authored-by: nuemaan <anonnumaan@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com> * fix: green CI for mapnav PR (lint, pyright, demo classic path) Exclude vendored nav/ from pyright, tighten mapnav plan typing, and force demo contract retrieval onto classic so CI does not hit a live LLM. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Ray Tien <ray.tien0907@gmail.com> Co-authored-by: nuemaan <anonnumaan@gmail.com> Co-authored-by: Cursor <cursoragent@cursor.com>
5 tasks
nuemaan
added a commit
to nuemaan/knowhere
that referenced
this pull request
Aug 27, 2026
Review showed the per-lexeme-only budget traded one starvation problem for another. A chunk covering several query terms ranks first under a global ts_rank_cd ordering, and BM25 agrees, but splitting the whole budget per lexeme fills every slice with denser single-term rows and drops it. The same split also truncated corpora that fit inside the budget, where the previous code truncated nothing. The prefilter now unions two pools. The global slice is the previous behaviour unchanged, so nothing it kept can be lost. The per-lexeme floor adds a small number of rows for each lexeme on top, which is what keeps a rare lexeme from being starved. Verified on Postgres 16 against all four corpora from the review thread: rare term under a saturated budget main misses it, union keeps it chunk covering six query terms main keeps it, union keeps it chunk covering two query terms main keeps it, union keeps it 1900 matches under a 2000 cap main 1900 rows, union 1900 rows Adds a contract test for the covering chunk. It fails against the per-lexeme-only version and passes here, so the regression stays closed. The bounding clause is LIMIT :fts_candidate_limit again, so the pre-LIMIT scope filter assertions added in Ontos-AI#244 apply unchanged and that test needed no edit.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Root cause
The content and path channels loaded every active scoped chunk into Python before checking token overlap and building the BM25 corpus. Large namespaces therefore paid corpus-sized memory and CPU costs for every retrieval step.
The SQL prefilter now uses
content_search_tsvorpath_search_tsvwithwebsearch_to_tsquery('simple', ...), preserving any-token behavior. Section exclusions are also evaluated inside the scoped query beforeLIMIT, including a regression guard for sectionless chunks.Validation
uv run pytest packages/shared-python/shared/tests/test_retrieval_search_channels.py apps/api/tests/contract/test_retrieval_contract.py -qmake lintmake typecheckCloses #195