Python context layer: lazy BM25 retrieval and prewarm - #241
Open
jat255 wants to merge 4 commits into
Open
Conversation
jat255
changed the title
jat255
marked this pull request as draft
September 2, 2026 00:04
jat255
force-pushed
the
jat255/context-layer-retrieval
branch
from
September 4, 2026 17:27
411c5d5 to
77713c6
Compare
jat255
marked this pull request as ready for review
September 4, 2026 17:52
jat255
force-pushed
the
jat255/context-layer-retrieval
branch
from
September 4, 2026 18:03
77713c6 to
47a78c1
Compare
jat255
force-pushed
the
jat255/context-layer-retrieval
branch
from
September 6, 2026 01:39
47a78c1 to
7be4e17
Compare
The index is built on first search rather than at construction: it is the most expensive part of building an agent, and many conversations never search. Two constraints of the retrieval library shape this. retrieve_bm25 pads its result up to top_k with unscored rows rather than dropping them, so a query matching nothing still returns a full result set. Without the metric filter, search() would hand the agent arbitrary context and present it as relevant. ingest() upserts on a document origin it also requires to be non-empty, so each document gets a distinct synthetic origin. Sharing one would make two files with identical text collapse into a single chunk.
Builds the index ahead of the first search, for callers that know a search is coming and would rather not pay for it mid-conversation. Kept synchronous. The milestone scope floated putting the build behind asyncio.to_thread, but the thread hop belongs at the tool boundary, which knows whether it is on an event loop. Inside the layer it would force every caller async for a call that is a no-op on a warm store.
- Build the store under a lock so concurrent first searches on a shared layer cannot each build (and all but one discard) a store. - search() validates top_k >= 1 instead of leaking a duckdb BinderException, and its docstring states the return shape, ordering, empty-result behavior, and the top_k bound; prewarm() says when it is worth calling. - Correct the ingest() comment: a shared origin fails the ingest with a duplicate-origin error; it does not collapse chunks. - Note that search() is public ahead of the R counterpart's internal context_search(n = ...), and leave a TODO for the commons_context_store_build span once pkg-py has tracing. - Tests: top_k validation, prewarm/search consistency, and a threaded test that a cold layer builds exactly one store (fails without the lock).
jat255
force-pushed
the
jat255/context-layer-retrieval
branch
from
September 6, 2026 23:52
7be4e17 to
9c22826
Compare
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
1 participant
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.
Second of two PRs for M4, the Python context layer (kata
g1bfandhmh8). Stacked on #240, which adds the class this builds on. Python only.ContextLayer.search(query, top_k=3)retrieves over a raghildaDuckDBStorewithembed=Noneand a BM25 index. The index is built on first search rather than at construction, because store setup is the most expensive part of building an agent and many conversations never search.prewarm()builds it ahead of time for callers that know a search is coming.Two constraints of the retrieval library drove the implementation, and both are covered by tests that fail without the corresponding code.
retrieve_bm25pads its result up totop_kwith unscored rows instead of dropping them, so a query matching nothing still returns a full result set.search()keeps only rows with a non-null bm25 metric. Without that filter the agent receives arbitrary chunks presented as relevant.ragnar_retrieve_bm25returns zero rows in the same situation, so this is a difference between the engines rather than between the packages.ingest()upserts on a document origin that it also requires to be non-empty, so each document gets a distinct synthetic origin. Sharing one collapses two files with identical text into a single chunk.search()andprewarm()are synchronous. The milestone scope floated putting the build behindasyncio.to_thread; the thread hop belongs at the tool boundary in M5, which knows whether it is on an event loop, rather than inside the layer where it would force every caller async for a call that is a no-op on a warm store.Nothing here is added to
tests/shared/. Retrieval ranking is engine-specific and is not a cross-language contract.