fix(context): drop archived pages from context packs - #595
Conversation
vouchdev#581 taught kb.search to drop archived pages, but build_context_pack was never given the same filter — it excludes retracted claims and passes pages straight through. archiving a page therefore hid it from search while it kept being injected into every context pack, which is the surface that actually seeds agent context. _append_graph_neighbors had the identical gap on the expansion path. the page half of the predicate moves into _page_is_live and all three call sites use it. the claim half stays inlined because those callers need the fetched claim anyway for citations and origin tags — pages are only ever tested, so one shared helper is enough to stop the two recall surfaces drifting again, which is exactly how this survived vouchdev#581. covered by a regression asserting an archived page is absent from a context pack while a live one survives, plus a direct contract test for the predicate over archived and missing pages.
|
Warning Review limit reached
Next review available in: 59 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
#581 taught
kb.searchto drop archived pages.build_context_packwas never given the same filter — it excludes retracted claims and passes pages straight through.so archiving a page hides it from
kb.searchwhile it keeps being injected into every context pack, which is the surface that actually seeds agent context._append_graph_neighborscarried the identical gap on the expansion path.reproducing
test_search_kb_excludes_retracted_claimsalready assertsp-archis absent fromkb.searchfor the same fixture, so the two recall surfaces disagree today.the change
the page half of the predicate moves into
_page_is_live; all three call sites (_filter_live_hits, the context-pack item loop,_append_graph_neighbors) use it.the claim half stays inlined at each site because those callers need the fetched claim anyway for citations and origin tags — re-fetching it through a boolean predicate would double the reads on the hot path. pages are only ever tested, so one shared helper is enough to stop the two recall surfaces drifting again, which is exactly how this survived #581.
tests
test_context_pack_excludes_archived_pages— the regression above; fails on the previous code withassert 'p-arch' not in {'p-arch', 'p-live'}test_page_is_live_rejects_archived_and_missing— pins the predicate's contract over archived and missing pages, which is what the graph path now routes throughverification
note that
tests/test_context.pyskips entirely on the base[dev,web]install (its autouse fixtureimportorskips numpy), so these run in the embeddings job rather than the main matrix. i verified them with numpy present.unrelated, but worth flagging while i was in there: with numpy installed but not the full
[embeddings]extra, five tests fail on a cleantestcheckout —test_cli.py::test_fsck_clean_kb_prints_clean_and_exits_zero,test_cli.py::test_search_fts5_backend_label,test_delete.py::test_deindex_removes_fts_and_prov, and bothtest_hot_memory.py::test_covered_methods_attach_sidebar_when_kb_has_recent_claimsparams. nothing to do with this pr — ci never hits it because the base job has no numpy — but a contributor who installs numpy for one thing lands in a confusing half-configured state. happy to file it separately.