fix(salience): exclude retired claims from the reflex#519
Conversation
the entity-salience reflex counted every claim referencing a matched entity, skipping the retired-claim exclusion that context, graph, digest, health and experts all apply. a superseded, archived or redacted claim therefore inflated an entity's claim_count and could be handed back as its top_claim_id, pointing an agent at knowledge the reviewer had already retracted. vouchdev#78 closed this leak on the context path and the same exclusion was carried to the other read surfaces; the salience sidebar was missed. compute_salience now skips retired claims, so _meta.vouch_salience reports live evidence only. the regression test covers all three retired statuses. it fails on the previous code: the retired claim sorts ahead of the live one, so it both inflates claim_count and takes top_claim_id.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughEntity salience excludes superseded, archived, and redacted claims from ChangesEntity salience filtering
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@src/vouch/salience.py`:
- Around line 38-41: Update the comment near the live-evidence claim filtering
logic to begin with lowercase prose by changing its opening “A” to lowercase;
leave the remaining explanation and behavior unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e331abf-9c82-4076-b5ac-8d8c9bfc59ab
📒 Files selected for processing (3)
CHANGELOG.mdsrc/vouch/salience.pytests/test_salience.py
house style is lowercase prose in comments; the new frozenset comment opened with a capital. no behaviour change.
What changed
compute_saliencenow skips claims whose status issuperseded,archivedorredactedwhen it builds the per-entity claim list, so_meta.vouch_saliencecounts live evidence only. the exclusion set is amodule-level frozenset, matching how
graph,digest,healthandexpertseach declare it.Why
the salience reflex was the one read surface that never got the
retired-claim filter. every other one applies it —
context.py,graph.py,digest.py,health.py,compile.py, andexperts.py,which spells the invariant out: "a superseded / archived / redacted
claim is not live evidence and must never inflate an entity ranking".
compute_saliencewalkedstore.list_claims()with no status check atall.
two things went wrong as a result. an entity's
claim_countcountedretracted claims, so the number disagreed with every other surface. and
top_claim_idis just the lowest-sorting claim id, so a retired claimcould take the top slot and hand an agent a pointer to knowledge the
reviewer had already pulled — including a
redactedclaim, the onestatus that exists precisely because the text was masked.
this is #78 on the surface it missed: that issue closed the
archived/superseded leak in
kb.context, and the same exclusion wascarried to the other read paths, but the salience sidebar was skipped.
What might break
nothing on disk: no file moves, no field changes shape, and no
kb.*method changes signature — this is a surface change to values, not to
shape. for a kb that has retired claims, an entity's
claim_countin_meta.vouch_saliencewill drop and itstop_claim_idmay move to alive claim. that is the fix. a kb with no retired claims is byte-identical.
VEP
not a surface change — no method, model, on-disk layout, bundle or
audit-log shape is touched.
Tests
CHANGELOG.mdupdated under## [Unreleased]make checkpasses locally (lint + mypy + pytest) — see belowtests/test_salience.py::test_retired_claims_are_not_salientisparametrized over all three retired statuses and fails on the previous
code. the fixture's live claim is
c1, the retired onec0, so on theold code the retired claim both inflates
claim_count(2 != 1) and winstop_claim_id.what i ran, and honestly what i could not:
ruff check src tests— clean.mypy src— clean apart from two pre-existing errors this branchdoesn't touch (
sandbox.py:112,os.getuid/os.getgiddon't existon windows; they resolve on linux ci).
pytest tests/test_salience.pyplus every module that consumessalience —
hooks,trust,openclaw_context_engine,experts,sessions,themes,triage— all pass.i'm on windows and the full suite wedges locally on an unrelated test, so
i leaned on ci for the whole matrix rather than claim a green
make checki didn't see.
Summary by CodeRabbit
claim_countandtop_claim_id, preventing recommendations to retired content.