fix(context,storage): exclude retracted claims from kb.context and reindex FTS5 on update#79
Merged
plind-junior merged 2 commits intoMay 28, 2026
Conversation
…index FTS5 on update (vouchdev#78) Two compounding bugs let archived / superseded / redacted claims keep flowing back to agents through kb.context: 1. build_context_pack had no claim.status filter, so any retrieval hit was appended to the pack regardless of subsequent lifecycle mutations. 2. store.update_claim refreshed the embedding cache but never re-indexed the FTS5 row, so claims_fts.status stayed frozen at first-index time. Every lifecycle op (archive, supersede, contradict, confirm) routes through update_claim and was therefore invisible to FTS5 / semantic search. This made ClaimStatus.{ARCHIVED, SUPERSEDED, REDACTED} purely decorative on the read side — agents kept quoting retracted knowledge as if it were live. Fix: - storage.update_claim now also calls index_db.index_claim under a short-lived sqlite connection (mirroring proposals.approve's first-index pattern). FTS5 errors are logged-and-skipped so a lifecycle op never fails because the embedding/FTS5 layer is misconfigured. - context.build_context_pack resolves each claim hit, drops it if the on-disk status is in {ARCHIVED, SUPERSEDED, REDACTED}, and drops it if the claim YAML has been deleted between index time and now. CONTESTED claims keep surfacing so contradictions remain visible. Tests: - test_context_pack_excludes_archived_claims - test_context_pack_excludes_superseded_claims - test_update_claim_refreshes_fts5_status (asserts the FTS5 status column via direct SQL against state.db) No on-disk-layout, schema, or bundle-format change.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR fixes a bug where archived, superseded, and redacted claims were still appearing in ChangesRetracted Claim Filtering and FTS5 Sync
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/vouch/context.py (1)
173-176:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFilter
explainrows the same way asitems.At Line [173],
explainis built from rawhits, so retracted/missing claims can still surface whenexplain=Trueeven though they were excluded fromitems.Suggested fix
def build_context_pack( @@ ) -> ContextPack | dict[str, Any]: hits = _retrieve(store, query, limit) items: list[ContextItem] = [] + explain_rows: list[dict[str, Any]] = [] for kind, hid, summary, score, backend in hits: @@ items.append( ContextItem( id=hid, type=cast(ContextItemKind, kind), summary=summary, score=score, backend=backend, citations=cites, freshness="unknown", ) ) + if explain: + explain_rows.append( + {"kind": kind, "id": hid, "score": score, "backend": backend} + ) @@ result["backend"] = hits[0][4] if hits else "none" if explain: - result["explain"] = [ - {"kind": k, "id": i, "score": sc, "backend": hits[0][4] if hits else "none"} - for k, i, _sn, sc, _be in hits - ] + result["explain"] = explain_rows return result🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/vouch/context.py` around lines 173 - 176, The explain list is currently built directly from raw hits (variable hits) which allows retracted or missing claims to appear even when those were filtered out of items; modify construction of result["explain"] to iterate the same filtered sequence used to build items (e.g., use filtered_hits or the same comprehension/predicate applied when creating items) so that each {"kind": k, "id": i, "score": sc, "backend": ...} entry is only created for hits that passed the items filter; ensure you reference the same hit tuple structure and backend extraction (hits[0][4] or similar) as in the original comprehension.
🧹 Nitpick comments (1)
tests/test_context.py (1)
83-142: ⚡ Quick winAdd a direct REDACTED regression case.
These tests cover ARCHIVED and SUPERSEDED well, but the runtime filter also excludes REDACTED. A dedicated test would prevent regressions on that status path.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_context.py` around lines 83 - 142, Add a test that mirrors the archived/superseded cases to cover the REDACTED path: create a new test (e.g. test_context_pack_excludes_redacted_claims) that uses store.put_source and store.put_claim to insert a claim, calls health.rebuild_index, verifies the claim is returned by context.build_context_pack for a matching query, then call lifecycle.redact(store, claim_id="c1", actor="reviewer") and assert the claim is no longer present in the pack; optionally also check the FTS row (via sqlite3.connect(store.kb_dir / "state.db")) to assert claims_fts.status == "redacted" similar to test_update_claim_refreshes_fts5_status.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/vouch/context.py`:
- Around line 173-176: The explain list is currently built directly from raw
hits (variable hits) which allows retracted or missing claims to appear even
when those were filtered out of items; modify construction of result["explain"]
to iterate the same filtered sequence used to build items (e.g., use
filtered_hits or the same comprehension/predicate applied when creating items)
so that each {"kind": k, "id": i, "score": sc, "backend": ...} entry is only
created for hits that passed the items filter; ensure you reference the same hit
tuple structure and backend extraction (hits[0][4] or similar) as in the
original comprehension.
---
Nitpick comments:
In `@tests/test_context.py`:
- Around line 83-142: Add a test that mirrors the archived/superseded cases to
cover the REDACTED path: create a new test (e.g.
test_context_pack_excludes_redacted_claims) that uses store.put_source and
store.put_claim to insert a claim, calls health.rebuild_index, verifies the
claim is returned by context.build_context_pack for a matching query, then call
lifecycle.redact(store, claim_id="c1", actor="reviewer") and assert the claim is
no longer present in the pack; optionally also check the FTS row (via
sqlite3.connect(store.kb_dir / "state.db")) to assert claims_fts.status ==
"redacted" similar to test_update_claim_refreshes_fts5_status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a04edce3-01dc-4f79-8a3e-7bac3d1bd7ec
📒 Files selected for processing (4)
CHANGELOG.mdsrc/vouch/context.pysrc/vouch/storage.pytests/test_context.py
Member
|
PR should target test branch |
Contributor
Author
Done |
3 tasks
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
2 participants
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
kb.contextreturns claims whose status isARCHIVED,SUPERSEDED,or
REDACTEDas if they were live. Two compounding bugs combine toproduce this:
Bug A —
build_context_packhas no status filter.src/vouch/context.pywalks the hits from_retrieveand appendsevery match to the context pack regardless of
claim.status. Oncea claim is indexed, it stays in retrieval forever.
Bug B —
store.update_claimnever refreshes the FTS5 row. Itcalls
_embed_and_store(semantic cache) but neverindex_db.index_claim(FTS5). Solifecycle.archive,lifecycle.supersede,lifecycle.contradict, andlifecycle.confirm— all of which finish withstore.update_claim(...)— leaveclaims_fts.statusstuck atwhatever value it had at first-index time.
Compound effect: even a future fix that adds a status filter to
context.pywould still leak archived claims, because the FTS5status column it would filter on is stale. Both bugs must be fixed
for retrieval to be correct.
The whole point of
ClaimStatus.{ARCHIVED, SUPERSEDED, REDACTED}is to remove a claim from active circulation while keeping its
history. Today those states are purely decorative on the read side;
agents quote retracted knowledge as if it were live.
End-to-end repro on
mainshows:With this PR applied:
Fix
Two files, both required:
src/vouch/storage.py—update_claimnow also callsindex_db.index_claimunder a short-lived sqlite connection,mirroring
proposals.approve's first-index pattern. FTS5 errorsare logged-and-skipped so a lifecycle op never fails because the
embedding/FTS5 layer is misconfigured.
src/vouch/context.py—build_context_packresolves eachclaim hit, drops it if the on-disk status is in
{ARCHIVED, SUPERSEDED, REDACTED}, and drops it if the claim YAML has beendeleted between index time and now.
CONTESTEDclaims keepsurfacing so contradictions remain visible (consistent with
how
vouch linttreats them as cautions rather than removals).The
_citations_for_claimhelper is folded into the sameget_claimcall since the body is needed for the status checkanyway, avoiding a double lookup.
No on-disk-layout, schema, or bundle-format change. Existing
KBs are unaffected: the FTS5 row gets updated on the next
mutation; archived claims that survive an old index are still
correctly filtered out at context-pack assembly time.
Tests
Three regression tests in
tests/test_context.py:test_context_pack_excludes_archived_claims— file a claim,archive it, assert it's absent from
kb.contextresults.test_context_pack_excludes_superseded_claims— supersedeold by new, assert
newis retrievable andoldis gone.test_update_claim_refreshes_fts5_status— direct SQL queryagainst
state.dbconfirmsclaims_fts.statusmoved from'working'to'archived'afterlifecycle.archive.All 8
tests/test_context.pytests pass on Windows (Python3.12.13). Lint clean (
ruff check src tests). The full suiteshows the same pre-existing Windows-only failures present on
main(O_NOFOLLOWinstorage.py, bundle round-trip CRLF) —both untouched by this PR.
Fixes #78
Summary by CodeRabbit
Bug Fixes
Tests