fix(kb): born-managed must not provision over an established legacy agent - #1109
Merged
Merged
Conversation
…gent Born-managed treated the absence of a KB_Record as 'brand-new agent' and provisioned a managed KB on the first upload. But legacy KBs are not first-class: they share one S3-Vectors index and never write a KB_Record, so an established legacy agent looks identical to a new one. Its NEXT upload was therefore mistaken for a first upload, flipping retrieval to an empty managed KB and stranding the existing corpus on the legacy index. Guard the record-is-None branch on an existing-documents check (assistant_has_documents: cheap COUNT, Limit=1, no ownership check). Only provision when the agent has zero documents; fail toward legacy on any probe error. Adds mutation-guard + helper unit tests (31 pass).
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.
Problem
Born-managed (
MANAGED_KB_NEW_DEFAULT) is meant to provision a managed KB on a brand-new agent's first document. It decided "is this new?" by checking for aKB_Recordrow and treating its absence as new.But legacy KBs are not first-class: they share one S3-Vectors index (
assistants-index) and never write aKB_Record. So an established legacy agent looks identical to a brand-new one. Its next upload was mistaken for a first upload → born-managed stampedretrievalEngine=managed, flipping retrieval to the empty, still-provisioning managed KB and stranding the agent's existing corpus on the legacy index (only the new upload gets ingested into managed).Reproduced in dev on
ast-43c0869ddd07: an agent with an existing legacy doc kicked off "Provisioning knowledge base…" on the next upload.Fix
Guard the
record is Nonebranch ofbegin_born_managedon an existing-documents check. Only provision when the agent has zero documents; an agent that has already ingested anything is an established legacy KB and stays legacy.assistant_has_documents()indocument_service.py: cheapSelect='COUNT',Limit=1probe. No ownership check (the caller already authorised the upload — it answers a question about data, not access).True) on any error / missing table config: wrongly provisioning over a legacy agent loses its corpus from retrieval, whereas wrongly staying legacy is benign (Upgrade still works)._joinpath (record exists) is unchanged and was already correct.Only
MANAGED_KB_NEW_DEFAULT-on environments were affected (dev; prod flag is off).Tests
tests/lambdas/test_kb_born_managed.py31 pass,tests/routes/test_documents.py7 pass, ruff clean.