feat(rag): add a real reranker to RAG retrieval by OchnikBartek · Pull Request #911 · vstorm-co/agenticos

OchnikBartek · 2026-08-18T13:51:16Z

What this adds

A real reranker for RAG retrieval (#142): a second pass that reorders search
candidates by a model's judgement rather than by embedding distance. Off by
default, configured per knowledge base, and billed to the organization's own
key.

BaseReranker + CohereReranker (Cohere Rerank 3.5) behind an interface —
a second provider is a second implementation, not a rewrite. Retrieval
overfetches (4× the limit), reranks, then truncates, so a good answer sitting
well below the top by distance can surface.
Per-collection resolution (rerank_resolution.py) mirrors embedding
resolution: a collection reranks only when it names both a model and a usable
org secret. Every other case — no key, a key that vanished, the wrong kind —
resolves to no reranker, so retrieval is byte-for-byte its pre-feature self;
the three misconfiguration cases are logged, the normal off state stays silent.
Metering. A rerank is priced per search, not per token, and genai-prices
does not know rerank models — so a new book_ambient_spend(SpendEntry) books a
pre-priced entry to whatever ledger is metering the search. The Cohere
per-search price is a dated constant checked against cohere.com/pricing. This
also wraps POST /rag/search in a metering block, closing a Meter the web chat surface so knowledge searches are billed #16-class gap where
even its embeddings went unbilled.
Both retrieval paths rerank. The /rag/search route and the agent-run
knowledge tool build their RetrievalService through one shared
build_reranker, so an agent's knowledge search reranks exactly as the route
does — the agent-run half of "spend recorded on both paths".
UI. Reranking is set in the create-KB dialog, and — new here — changed or
turned off after creation from a Reranking panel + edit dialog on the KB
detail page, matching what the backend and docs already promised
("unlike the embedding model this can be changed later"). An unreachable
feature is worse than none (RAG search: fix the page, then improve the search #61).
Docs (docs/file-processing.md), a kb-rerank onboarding stop, and
no-secret-escapes coverage all updated.

Notes from review

Three honesty fixes made along the way, all called out in the commits:

retrieve_multi resolves the reranker from the first collection and runs it
once over the union — unambiguous on the agent-run path (bound collections share
one org and one config), but /rag/search can pass a mixed set, so the
first-collection rule is now stated plainly in the docstring and the docs.
min_score gates recall on vector distance and is not re-applied after
reranking (the reranked score is a different scale) — noted so a caller does
not threshold on it.
The Cohere purpose was reused, not duplicated: cohere already exists as a
model-provider purpose, so it appears under "Model provider" in the vault rather
than a new category.

Verification

Backend: make test green with the 100% platform-layer gate; new modules
(rerank_resolution.py, knowledge_search.py) added to both the coverage and
ty include lists. make lint-backend, make db-check, migration round-trip on
real pgvector all green.
Frontend: make lint-frontend and the coverage gate green; new components and
the updateRerank hook branch fully covered.
Live: verified end to end in an isolated Docker stack — KB create with a
reranking key, upload → ingest (real embeddings) → search reordered vs. an
identical collection with reranking off, and the search-path spend row landing
in ingestion_spend.

Nothing is knowingly red.

Closes #142

record_ambient_usage() prices every call through genai-prices, which knows chat and embedding models and nothing else. A reranker call routed through it would book cost_usd=0, priced=False, so its spend would be invisible to the monthly budgets and reported as a floor. book_ambient_spend(entry) is the sibling for spend that is not token-priced: the caller computes the cost from a published per-search price and hands the finished SpendEntry over, so it lands priced=True with a real number. Like its sibling it is a no-op when nothing is metering - a search outside any run has no ledger open and must still run rather than refuse. Groundwork for the RAG reranker (#142); no caller yet. Refs #142

Two nullable columns mirroring the embedding pair: rerank_model (the reranker's name) and rerank_secret_id (the org vault key that pays for it, FK to organization_secrets, SET NULL on delete). Reranking is on for a collection only when both are set; either NULL leaves retrieval exactly as it was, so existing rows and unconfigured deployments are unchanged. Unlike the embedding key there is no deployment fallback - a reranker with no key is simply off - so nothing goes into RAGSettings. Verified against a real pgvector database: alembic upgrade head, alembic check reports no drift (model matches the migration), and downgrade -1 -> upgrade round-trips. tests/test_migrations.py green. Refs #142

reranker_for_collection() is the sibling of embeddings_for_collection(): it asks per collection whether a reranker is configured and returns its model and the organization key it runs on, or None. The one deliberate difference from embedding resolution is the whole design of the feature being off by default. Embeddings fall back to the deployment key when a collection's chosen one is gone; reranking has no deployment key, so every path but a usable organization secret resolves to None and retrieval is byte-for-byte its pre-feature self. The three degraded reasons (secret missing / unusable / wrong kind) are still told apart from the normal off state and logged, exactly as EmbeddingKeySource names its own - a chosen key that vanished is an operator's problem, a collection that chose nothing is not. Adds the `cohere` secret purpose (category other, api_key) with RERANK_KEY_PURPOSES as its consumer. rerank_resolution.py joins the coverage + ty gates beside embedding_resolution.py (100%, verified). Refs #142

BaseReranker is the interface retrieval depends on; CohereReranker is the first and only implementation. rerank() reorders the candidate SearchResults by Cohere's relevance score and re-scores them with it, so a caller ordering or thresholding on score reads the reranker's judgement rather than the vector distance the candidates arrived with. Cost is booked through book_ambient_spend, not record_ambient_usage: a rerank call is priced per search unit (one query, up to 100 documents), not per token, and genai-prices does not know rerank models. The per-search price is the one number in the metering path that lives in this repository - $0.002/unit, confirmed against cohere.com/pricing on 2026-08-18, with a comment telling the next maintainer to re-check the page and the constant together. Booked only after the call returns, because Cohere does not bill a failed request; a raise propagates so retrieval can degrade to the un-reranked order rather than failing the search. The client is built lazily and injectable, so the tests drive it with no network and no key. Refs #142

RetrievalService takes an optional reranker resolver. When a collection resolves one, retrieve() overfetches a wider candidate net (4x rather than 2x), reranks, and truncates to the limit; retrieve_multi() gathers each collection's candidates and reranks the union once, because an agent's bound collections share one organization and so one reranker. With no resolver - or none configured for the collection - every path is byte-for-byte the previous by-distance one, down to each collection contributing its top `limit` before a multi-collection merge. Recall is split out of retrieve() into _recall() so the multi path can fuse before ranking rather than rank-then-fuse. The collection stamp moves onto every candidate so it survives reranking, which builds fresh results. A reranker failure at query time degrades to the distance order with a log line rather than failing the search - reranking is an improvement on a working retrieval, not a dependency of it; the misconfiguration cases never reach here, resolution having already turned those into no reranker. deps wires reranker_for_collection through a composition-root adapter that binds the resolved credential to a CohereReranker - the one place a second provider would branch. Refs #142

The search route embedded the query - and, once a collection is configured, reranks - inside no metered_by block and against no ledger, so neither cost reached the organization's monthly bill (#16 class). Reranking is what made that worth fixing; metering the embeddings too is the beneficial side effect. KnowledgeSearchService owns it: it resolves collection access, opens a ledger scoped to the caller's organization, runs the search inside metered_by so the ambient embedding and rerank calls book to it, and persists what they spent to ingestion_spend with a null document id - the same sink a worker's ingestion spend lands in. The route drops to plumbing, which also keeps the metering out of a route handler the layering forbids logic in. Access is resolved before the ledger opens, so a cross-tenant collection still refuses the whole search before any vectors are read - the tenant-isolation route tests in test_platform_flows.py stay green. knowledge_search.py joins the coverage + ty gates (100%). Refs #142, #16

The rerank model and key are now on KnowledgeBaseCreate/Update/Read and threaded through the service and repository, so a collection can actually be configured to rerank. Two rules the service enforces: - A model and a key together, or neither. A lone half reads as configured and does nothing (resolution requires both), so it is refused where the person setting it can see why rather than silently ignored at search time. - The key must be a Cohere-purpose secret the organization holds, checked at write time - the mirror of the embedding-key check, and for the same reason: resolution degrades a bad key to no reranking, so this is the one moment a wrong choice is visible. Unlike the embedding model, reranking can be changed after creation. Update sends the pair only when the caller actually included it (read from model_fields_set), so an update about something else leaves reranking alone and sending both as null is how it is turned off - a distinction the repo's None-means-skip convention cannot make, hence the explicit set_rerank flag. The Read schema exposes the model and the secret id; the id names a vault row, never its value. Refs #142

The OpenAPI no-secret-escapes sweep found KnowledgeBaseRead.rerank_secret_id as a new credential-shaped field; it is an id naming a revocable vault reference, never the key, exactly like embedding_secret_id, so it joins the allowlist with that reason rather than being removed from the response. Adds a resolution test that the rerank key is looked up scoped to the collection's own organization - the tenant boundary at the resolution layer, on top of the vault's own cross-tenant refusal (test_vault.py) and the org-scoped secret query. Refs #142

Adds a "Reranking — a second pass, off unless configured" subsection under the embeddings heading in file-processing.md: what a reranker is and where it wires into retrieval, the per-KB Cohere configuration mirroring embeddings, the off-by-default design and its degradation to no-rerank, the runtime fallback to distance order, and the metered-spend divergence from genai-prices plus the /rag/search metering gap it closes. Refs #142

Mirrors the embedding-key picker: a "Reranking" disclosure in CreateKBDialog with an Off/key Select over the organization's cohere-purpose vault keys (plus inline add-a-key), placed after Embeddings. There is one reranker and no endpoint listing them, so the model is a frontend constant (rerank-v3.5) and choosing a key is choosing to rerank - the submit sends the key and the model together or neither, matching the backend's both-or-neither rule. Create-time only, exactly as embeddings are: the KB detail page has no edit control for these config fields, and the backend accepts the pair at create. rerank_model / rerank_secret_id added to CreateKnowledgeBaseInput. A guided-tour stop (flow-kb-field-rerank) mirrors the embeddings one, gated on collections:edit and anchored on the disclosure. Tests cover both branches: Off posts neither field, a chosen key posts rerank_secret_id and rerank_model=rerank-v3.5. Refs #142

Standing the stack up surfaced it: `cohere` is already a model-provider purpose (pydantic-ai supports Cohere chat models, and the provider list mints a purpose for every provider), so adding a `cohere` service entry produced two purposes with the same id - all_purposes() returned it as both model_provider and other, which the vault picker would render twice. A Cohere API key reranks and chats alike, so the right fix is to reuse the existing purpose rather than mint a second: drop the services.json entry and let RERANK_KEY_PURPOSES point at the model-provider one. This corrects the earlier choice of category "other", which was made before it was known that the id was already taken. No functional change to the rerank flow: a key stored under the cohere purpose still matches RERANK_KEY_PURPOSES, and the builder's rerank picker still filters on purpose == "cohere". Refs #142

The knowledge capability built its own RetrievalService with no reranker resolver, so an agent searching its bound collections never reranked - only the /rag/search route did. That contradicts the contract: rerank spend is meant to be recorded on both the agent-run path and the route (#142 done-when), and the whole "a rerank during a knowledge search books automatically" framing assumed the run's open ledger would see it. Both paths now build their RetrievalService with one shared composition point, build_reranker() in reranker.py, which resolves a collection's credential and binds it to the CohereReranker. deps.get_retrieval_service and the knowledge tool's get_retrieval_service both pass it, so reranking is wired identically and a second provider is a branch in one place. The agent-run path's rerank cost books to the run's ledger, which was already open. Found auditing the branch against the issue. Tests assert both paths wire build_reranker and that it degrades to None when unconfigured. Refs #142

…cale Two honesty fixes surfaced in review, both docstring/doc only - no behaviour change. retrieve_multi resolves the reranker from collection_names[0] and runs it once over the union. The docstring justified that with "an agent's bound collections share one organization and so one reranker", which is true on the agent-run path but silent about /rag/search, where a caller may pass any readable set of one organization and the first collection's setting governs the whole union - a set led by a plain collection stays in distance order even if a later one reranks. Said plainly now, in the docstring and in docs/file-processing.md. min_score gates recall on the vector-distance score; after reranking, score carries the reranker's relevance judgement on a different scale and min_score is not re-applied. A caller thresholding on a reranked result's score as if it were the recall score would be wrong. Noted on _recall.

The backend already let a knowledge base's reranker be changed - the update schema reads rerank_model and rerank_secret_id as a pair, sets them when both are sent and turns reranking off when both are null - and the Read schema and docs both promise "unlike the embedding model this can be changed later". Only the UI disagreed: reranking could be set at creation and never touched again, so the whole change/turn-off path was reachable only through the API. An unreachable feature is worse than none (#61), so the console now matches what the backend and the docs already say. The detail page gains a Reranking panel beside the ingestion one - its own section, because reranking is a retrieval-time setting that changes on a different day than how the documents were read - stating whether searches are reranked and with which key, resolved from the vault and falling back to a neutral label for a reader who cannot list secrets. Its Edit opens a dialog that mirrors the ingestion one: a key picker whose "off" is one of its options, an inline "add a Cohere key", and a Save that sends the pair the backend reads together (a key with the one model, or two nulls). `updateRerank` on useKBDetail is the mutation, patching the same /kb/{id}. Also: a kb-rerank tour stop so the new section is not invisible to the walkthrough; the three rerank constants moved to src/lib/rerank-config.ts so the create and edit dialogs cannot drift; rerank_model and rerank_secret_id added to the KnowledgeBase read type (and the nine fixtures that build one). Verified: new components and the hook branch fully covered; make lint-frontend and the coverage gate green (the one miss is the pre-existing resume.ts:45 artifact, covered on CI).

chatgpt-codex-connector · 2026-08-18T13:51:23Z

You have reached your Codex usage limits for security reviews. Please try again later.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d97c2bb08a

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Binding a rerank key is lending it: reranking spends it for everyone who can search the collection. `_check_rerank_secret` only looked the key up scoped to the organization, so a `collections:edit` holder who supplied the UUID of another member's private Cohere secret bound a key that `secrets:view` would refuse them - the picker never offers it, but the API takes an id and an id is guessable. Now it runs `resolve_access(..., SECRETS_VIEW)` on the row exactly as agent secret bindings do, and refuses a key the caller cannot reach as "not in this organization's vault" - the same answer as a genuine miss, so a refusal cannot be told apart and used to enumerate the vault. The mirror `_check_embedding_secret` has the identical gap; it predates this branch and is filed separately rather than folded in.

The rerank pair is written together, but deleting the chosen Cohere secret nulls rerank_secret_id through the foreign key while leaving rerank_model set. `_resolve_reranker` classified that half state as the normal, silent NOT_CONFIGURED case, so reranking stopped with no signal at all - an operator had no way to see a key deletion had quietly turned it off. Now only the genuine null/null state is silent; a model with no key resolves to SECRET_MISSING and logs the same warning the other degraded cases do.

The query embedding is booked before the vector query it pays for, so a search that fails mid-flight has already spent - and recording that on the request session was pointless, because the failed request rolls the session back and takes the spend row with it. Failed searches therefore underreported provider cost and monthly budget usage. The failure path now books through a session of its own that commits, so the cost lands whether or not the answer did - the platform records spend even when the run fails. The success path is unchanged.

An app-scoped collection carries no organization_id, so it can hold no vault key and the backend refuses one ("Only an organization collection can carry a vault key"). The detail page offered its Edit anyway, so the control could only ever fail. It now shows the Reranking panel as a read-only fact and drops Edit for app scope. The create dialog needs no change: its scope picker offers only personal and org, never app.

The detail page now carries a Reranking section with its own Edit button, so a page-wide getByRole("button", {name: "Edit"}) matches two elements and Playwright's strict mode fails the ingestion spec. Scoped to the "How documents are read" region, where the spec means to click.

chatgpt-codex-connector · 2026-08-21T20:41:28Z

You have reached your Codex usage limits for security reviews. Please try again later.

chatgpt-codex-connector · 2026-08-21T20:45:06Z

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: a5f3a7c61b

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Scoping reranker resolution to the acting tenant made `retrieve`'s `organization_id` a required keyword-only argument. The `rag-search` CLI command still called `retrieve()` without it, so `project cmd rag-search` raised `TypeError: retrieve() missing 1 required keyword-only argument: 'organization_id'` before any search ran. The unit suite did not catch it - app/commands/rag.py is not in the coverage gate and had no test for this path. The CLI is a tenantless operator path with no acting organization, so it passes `organization_id=None` explicitly, the same as the other tenantless vector-store calls. Reranker resolution with no organization behaves as it did before tenant scoping. Added a regression test asserting search_async scopes retrieval to no organization; it fails (KeyError) against the un-fixed call. Found by codex review of this PR.

OchnikBartek · 2026-08-21T20:48:31Z

@codex review

chatgpt-codex-connector · 2026-08-21T20:48:45Z

You have reached your Codex usage limits for security reviews. Please try again later.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d3b2a6da2f

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Rerank spend was derived as ceil(candidate_count / 100), one search unit per 100 candidates. Cohere splits a document past its token threshold into several billable documents, so a request with fewer than 100 candidates but large chunks (ingestion allows chunks up to 8192 chars) consumes several search units while this recorded one - the dashboard and the monthly budget ledger drifting below the provider bill. Read the billed figure from the response instead: meta.billed_units.search_units is what Cohere charged. Every level of that chain is optional, so when it is absent fall back to the old candidate-count estimate. The float is rounded up to whole units and cost stays a Decimal. Tests cover the billed-units path (a small candidate set billed several units), a fractional unit rounding up, and the fallback when the response omits the figure. reranker.py stays at 100% coverage. Found by codex review of this PR.

A secret's usage listing ("used_by" / what breaks if I delete this) was built solely from agents_using(), which searches agent draft specs. A Cohere key used only as a knowledge base's rerank credential - or an OpenAI key used only as a KB embedding credential - therefore read as "not used yet", inviting an admin to delete a key a collection was actively resolving. Both references are SET NULL on delete, so the deletion does not error; it silently stops reranking or embedding for every bound collection. Add knowledge_bases_using(organization_id, secret_id) covering both embedding_secret_id and rerank_secret_id, and fold its rows into used_by as kind="knowledge_base" beside the agents. The embedding gap predates this branch - KB secret references were never surfaced to the vault - so the one query closes both rather than only the rerank case this PR added. - schemas/secret.py: SecretUsage.kind widens to "agent" | "knowledge_base"; the shape is unchanged, so the frontend contract is additive (the table already renders usage names without branching on kind). - frontend types mirror the widened union. - docs/secrets.md documents KB embedding/rerank bindings and that the usage listing reports them. Tests: a key bound only by a KB is reported (unit), and knowledge_bases_using finds both embedding and rerank bindings scoped to the organization (integration). Found by codex review of this PR.

Scoping resolution to the acting organization (get_for_collection, own-org wins) narrowed the shared-collection-name exposure but did not close it. The access check and the resolution used different tie-breaks on one name: CollectionAccessService returns the first *readable* row, while get_for_collection returns the caller's own-org row unconditionally. When an app-scoped collection and a restricted org collection of the caller's own organization share a collection_name, a member who may read the app collection but holds no grant on the restricted org one was authorized against the app row, yet resolution returned the org row - unsealing and spending its embedding and rerank keys. An intra-organization access-control bypass reaching a key the caller was never granted. Root cause: knowledge_search computed the authorized rows (readable_all) and then kept only their collection_name strings; retrieval and the resolvers re-looked-up by (collection_name, organization_id). Thread the authorized knowledge base id from the search path down to the resolvers, which read that exact row (get_by_id) instead of re-selecting by name. The id comes from the same readable_all that granted access, so resolution can never land on a row access did not. The parameter is optional and defaults to the previous behaviour, so ingestion and the CLI - which choose the row themselves and have no distinct authorized identity - keep the organization-scoped get_for_collection lookup. The agent-run tool identifies its bound collections by name from the spec and is unchanged: it has no per-user access check to diverge from. Threaded through retrieve / retrieve_multi / _recall / _bm25_search / _reranker_for, PgVectorStore.search / _for_collection, build_reranker, and both embeddings_for_collection / reranker_for_collection; the resolver callable types gain the third argument. Cover the refusal: an integration test creates an app collection and a restricted org collection on one name and asserts that resolving by the authorized app row returns the app config and never the org key, where a name+organization lookup returns the org row. Unit tests assert the id reaches the resolvers (retrieval) and that a given id reads that row and skips the name lookup (both resolvers, at 100% on the gated modules). Refs #913. Found by codex review of this PR.

Threading the authorized knowledge base id gave the embedding resolver a third argument, and `PgVectorStore._for_collection` now calls the resolver with three. This integration test's stub resolver still took two, so it raised `TypeError: _no_collection_of_its_own() takes from 1 to 2 positional arguments but 3 were given` - a failure invisible locally (no database) and caught only by the CI test job. Widen the stub to accept the ignored `knowledge_base_id`. Verified against a real pgvector: the whole tests/integration/ suite passes (584), including this file and the app-vs-restricted-org cover-the-refusal test.

Main took the triggers and sandbox stacks since the last re-parent, so its chain now runs to 0055_sandbox_operations and the rerank pair - kept at 0046/0047 from the previous renumbering - forked it at 0045 again, this time also colliding with main's own 0046-0047 by number. Same fix as 55bce96 and a5f3a7c, one merge later: 0046_knowledge_base_rerank becomes 0056 (down 0055_sandbox_operations) and 0045->0047 ingestion_spend_source becomes 0057. Verified against a real pgvector 16: alembic heads reports one head, tests/test_migrations.py passes whole (upgrade, downgrade, cycle, current-matches-head), tests/test_migration_chain.py passes, the rerank suites (51 tests) and the KB-dialog/onboarding frontend suites pass on the merged tree. Refs #911

OchnikBartek · 2026-08-25T22:51:50Z

@codex review

chatgpt-codex-connector · 2026-08-25T22:52:03Z

You have reached your Codex usage limits for security reviews. Please try again later.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2c72d655df

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

OchnikBartek · 2026-08-26T00:17:17Z

@codex review

chatgpt-codex-connector · 2026-08-26T00:17:26Z

You have reached your Codex usage limits for security reviews. Please try again later.

chatgpt-codex-connector · 2026-08-26T00:21:49Z

Codex Review: Didn't find any major issues. Swish!

Reviewed commit: 2c72d655df

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

The direct-search fix (#911) threaded the authorized knowledge base id into resolution, but the agent-run path still transported collection names alone. AgentRunnerService._collection_names() resolved spec.collection_ids to names and dropped the ids, and search_knowledge_base supplied only organization_id - so when an agent binds a collection whose collection_name is shared by another row in the same organization, get_for_collection() could select the other row and apply its embedding/rerank configuration, unsealing and billing a key the agent publisher was never granted. The same intra-organization exposure the search path closed, reachable through a bound agent instead of a direct search. Carry the bound id beside the name the whole way: - _collection_names becomes _bound_collections, returning (name, id) pairs; the runner puts both into resources as kb_collection_names / kb_collection_ids. - AgentDeps carries kb_collection_ids aligned with kb_collection_names; the factory reads it from resources. - The delegation path carries it too: ResolvedSubagent.collection_ids, and the clone the library hands a delegate has both put back. - The knowledge toolset passes ctx.deps.kb_collection_ids, and search_knowledge_base forwards knowledge_base_id(s) to retrieve/retrieve_multi. Ids are used only when their length matches the resolved names; the nameless _active_kb_collections fallback (which nothing sets) therefore resolves by organization, exactly as before. Tests: the bound id reaches retrieve on the single and multi paths, and a length mismatch drops the ids rather than pinning the wrong one. Verified the whole backend suite plus the 100% gate against a real pgvector. Refs #913. Found by codex review of this PR.

main advanced with the embedding-provider feature, the worker's per-flow engine (#948) and the RAG lookup-index backfill; this reconciles it with the branch's reranker, tenant-scoped resolution (#913) and agent-path bound-KB-id threading (#911). The two feature sets are fused rather than either chosen: - embedding_resolution / vectorstore / rag_tasks: main's provider catalog and per-flow engine kept, and organization_id + knowledge_base_id re-threaded through the refactored store (search, _for_collection, insert_document, _ensure_collection, get_collection_info, create_collection) so resolution still reads the authorized row rather than a same-named one (#913). - knowledge_base repo/schema/service: main's embedding_provider update path and the branch's rerank pair sit side by side. - frontend: the KB dialog and detail page carry both the embedding-provider and the rerank controls; use-knowledge-bases exposes updateEmbeddings and updateRerank. - migrations: main renumbered backfill_rag_lookup_indexes to 0058; the branch's two migrations are reparented onto it as 0059 (rerank) and 0060 (spend source), leaving a single linear head. Also adds the AsyncSession import main's vectorstore.py uses but does not declare - the file NameErrors on import without it. Verified against a real pgvector: full backend suite green at 100% coverage, make lint-backend and lint-frontend clean, alembic check reports no drift, and the tenant-isolation and agent-path integration tests pass.

…roken

Main merged 0059_invite_fk_ondelete off 0058 while this branch's rerank chain also sat on 0058, so the PR's merge-with-main check saw two 0059 heads. Renumber the branch's two migrations onto main's new head: 0060_knowledge_base_rerank and 0061_ingestion_spend_source. Single linear head verified against a real pgvector (test_migration_chain, test_migrations).

…on (#1425) ## What this fixes `knowledge_bases.collection_name` is indexed but **not unique** — two organizations can name a collection the same string and share one vector table. `embeddings_for_collection` resolved the knowledge base by name alone (`get_by_collection_name(...).first()`), returning an arbitrary tenant's row. So an embedding call for org A could resolve **org B's** knowledge base: unsealing B's vault key under B's scope, billing B, and running A's text through B's credential (#913). It affects ingestion and search alike, and predates the rerank work the reviewer raised it against. ## The fix — resolve within the caller's organization - **`knowledge_base_repo.get_for_collection(db, name, organization_id)`** — the org-scoped lookup: the caller's own row first, then an app-scoped (deployment-wide) one, and **never a third tenant's**. The worker's `_knowledge_base_for` held the same two-pass logic and now delegates to it, so the tenant-narrowing rule lives in one place. - **`embeddings_for_collection(name, organization_id=None)`** takes the org. The resolver interface is now `(str, UUID | None)` and the store threads it: `search`/`get_collection_info` → `_for_collection` → resolver. The agent search passes `deps.organization_id`; the route search and the `…/info` route pass `ctx.organization_id`; ingestion binds the flow's own org in the resolver closure (the store passes none on the insert path). `None` keeps the old first-match for a caller with no organization in hand — a local-directory sync. The adversarial self-review (the automated reviewer is off, #311) flagged `get_collection_info` as the one metadata path still resolving by name alone — it unsealed a foreign tenant's key (for `dim` only; no billing, nothing returned). It is threaded here too. The remaining org-`None` path is the app-admin-only local-directory sync, unchanged from before. ## Verification - **Two-tenant integration test** (the crux): org A and org B share a `collection_name`; each resolves its own knowledge base, a third org gets `None` rather than a foreign row, an app-scoped row is the fallback, and the embedding resolver **never unseals another tenant's vault key**. - The resolver's unit tests re-pointed at the org-scoped lookup; `embedding_resolution.py` stays at 100%. `make lint-backend` green. ## Scope — what this does not close - The **physical vector table is still keyed by `collection_name`**, so two tenants sharing a name still share a table — the store-level isolation question #913 flags separately. Not closed here. - The **rerank resolver** #913 also names (`rerank_resolution.py`) **is not in the tree yet** — it is part of unmerged PR #911, where this was first raised. Its identical fix belongs there, on the same `get_for_collection`. Closes #913 --------- Co-authored-by: DEENUU1 <111304236+DEENUU1@users.noreply.github.com>

OchnikBartek added 14 commits August 18, 2026 10:24

OchnikBartek added the enhancement New feature or request label Aug 18, 2026

OchnikBartek self-assigned this Aug 18, 2026

OchnikBartek requested a review from DEENUU1 August 18, 2026 13:51

github-code-quality Bot found potential problems Aug 18, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 18, 2026

View reviewed changes

OchnikBartek added 5 commits August 18, 2026 20:47

This was referenced Aug 18, 2026

rag: binding an embedding secret skips the caller's secret-view check #912

Closed

rag: per-collection resolution picks a KB by non-unique collection_name #913

Closed

Issue map: how the open issues relate, and what blocks what #168

Open

Merge branch 'main' into feat/rag-reranker

345bdfd

OchnikBartek mentioned this pull request Aug 19, 2026

Stitch the parallel W3 features into one product: triggers dashboard widget, triggers onboarding, unified naming #594

Closed

7 tasks

chatgpt-codex-connector Bot reviewed Aug 21, 2026

View reviewed changes

Comment thread backend/app/repositories/knowledge_base.py

Comment thread backend/app/services/rag/reranker.py Outdated

Comment thread backend/app/db/models/knowledge_base.py

OchnikBartek added 6 commits August 21, 2026 23:05

Merge remote-tracking branch 'origin/main' into feat/rag-reranker

b869b89

github-code-quality Bot found potential problems Aug 23, 2026

View reviewed changes

chatgpt-codex-connector Bot reviewed Aug 25, 2026

View reviewed changes

Comment thread backend/app/agents/capabilities/knowledge/_search.py

OchnikBartek added 2 commits August 26, 2026 19:08

github-code-quality Bot found potential problems Aug 26, 2026

View reviewed changes

OchnikBartek added 3 commits August 26, 2026 23:42

style(rag): wrap the reserved-names test resolver ruff-format wants b…

2472ff3

…roken

Merge remote-tracking branch 'origin/main' into feat/rag-reranker

2dcbd86

github-code-quality Bot found potential problems Aug 26, 2026

View reviewed changes

DEENUU1 mentioned this pull request Aug 27, 2026

fix(api): read last-seen from every session, count only usable ones #1279

Merged

OchnikBartek mentioned this pull request Sep 4, 2026

fix(rag): scope per-collection embedding resolution to the organization #1425

Merged

DEENUU1 marked this pull request as draft September 5, 2026 11:52

Uh oh!

Conversation

OchnikBartek commented Aug 18, 2026

What this adds

Notes from review

Verification

Uh oh!

chatgpt-codex-connector Bot commented Aug 18, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chatgpt-codex-connector Bot commented Aug 21, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 21, 2026

Uh oh!

OchnikBartek commented Aug 21, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 21, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

OchnikBartek commented Aug 25, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 25, 2026

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

OchnikBartek commented Aug 26, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 26, 2026

Uh oh!

chatgpt-codex-connector Bot commented Aug 26, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees