feat(kb): dead-letter document reconciler (task 16.5, §5.37) - #1007
Open
DerrickF wants to merge 1 commit into
Open
feat(kb): dead-letter document reconciler (task 16.5, §5.37)#1007DerrickF wants to merge 1 commit into
DerrickF wants to merge 1 commit into
Conversation
The ingestion consumer is the only writer of DOC# status. When its event dead-letters (Lambda async retry capped at 2), a document Bedrock already indexed is left parked non-terminal forever, and the retrieval filter serves only 'complete' -- so its content sits in the KB fully retrievable and invisible to every query. Two such docs occurred in dev; both needed manual repair. Add document_reconciler.py: the missing second writer. Daily, it finds DOC# rows stuck non-terminal (uploading/chunking/embedding) past a 60-minute grace gate, probes Bedrock per document, and drives a stranded-but-retrievable doc to 'complete' (the §5.37 case). It reuses the consumer's own probes -- document_status, the equals-on-document_id retrievability search, its status-set constants, and set_document_terminal -- so §5.37/§5.38/§5.39 live in one place. FAILED -> failed; NOT_FOUND -> re-ingest from S3 (the scheduled form of task 14.4's one-click retry). Modelled on reconciler.py: ships DISARMED (MANAGED_KB_DOC_RECONCILER_ARMED, empty reads as off); per-run action limit applies in both modes so the report is trustworthy; grace gate is a pure function of the row's own updatedAt and fails closed; terminal/deleting rows are never candidates. Guards in tests/lambdas/test_kb_document_reconciler.py (61 tests), mutation-verified. Scheduling + IAM wiring + arming are a deploy-gated follow-up; the flag is exempted in the env-contract test's OPTIONAL_OVERRIDES until that lands.
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.
What
Adds
kb_migration/document_reconciler.py— the missing second writer ofDOC#document status — closing HANDOFF §5.37 / task 16.5.Why
On the managed path a document's
DOC#status is written by exactly one writer: the ingestion consumer, which polls Bedrock until the doc is genuinely retrievable and only then writescomplete. But it runs in a Lambda whose async retry is capped at 2 attempts (a hard service limit). When an event exhausts those retries and dead-letters, the row is left non-terminal (uploading/chunking/embedding) with nothing left to revisit it — even though Bedrock often finished indexing seconds later, so the content is in the KB fully retrievable.That is invisible and permanent:
rag_service._filter_vectors_by_document_statusserves onlycompletedocs, so a stranded row's chunks are dropped from every query. The user was told their upload worked; the assistant never cites it. Two such docs occurred in dev and both needed a manual DynamoDB edit.How
Daily, the reconciler finds
DOC#rows stuck non-terminal past a 60-minute grace gate, asks Bedrock the ground truth per document, and:complete(the §5.37 fix)failedIt reuses the consumer's own probes —
document_status(GetKnowledgeBaseDocuments), theequals-on-document_idretrievability search, its status-set constants, andset_document_terminal— so §5.37 (poll budget), §5.38 (unfiltered search finds the wrong doc) and §5.39 (undeclared statuses) live in one place, not four. The one thing it does not reuse is the consumer's polling: it takes a single retrievability reading per doc, because it sweeps a fleet rather than shepherding one upload.Safety (modelled on
reconciler.py)MANAGED_KB_DOC_RECONCILER_ARMED, empty string reads as off. Report-only logs exactly what it would do and writes nothing.updatedAt, never discovery time, and fails closed when it cannot be read — so an in-flight upload is never marked from under the consumer.complete/failed/deletingrows are never candidates — a soft-deleted doc must not be resurrected.Tests
tests/lambdas/test_kb_document_reconciler.py— 61 tests, moto DynamoDB + a stub backend modelling Bedrock's document view (same style astest_kb_ingestion_consumer). Mutation-verified: neutering the retrievability gate failstest_indexed_but_not_retrievable_is_left_short_of_complete; wideningNON_TERMINAL_STATUSESfailstest_terminal_and_deleting_rows_are_never_candidates. Green: new suite +test_kb_reconciler+test_kb_backend_boundary+test_lambda_image_imports+test_kb_migration_env_contract(145 total). ruff clean; mypy profile identical to the siblingreconciler.py.Deploy-gated follow-up (NOT in this PR)
The reconciler's own Lambda + EventBridge schedule + IAM in
kb-migration-construct.ts, then setting/flipping the arming flag. The flag is exempted in the env-contract test'sOPTIONAL_OVERRIDESuntil that infra lands. No infrastructure changed and nothing is deployed by this PR.