fix(store): a re-asserted retired belief revives only for a person (#1215) - #1216
Conversation
`get_belief_by_content_hash` had no `valid_to` filter, so re-asserting a statement that had been retired resolved to the tombstone: nothing was inserted, a corroboration row was written against the retired belief, and nothing became visible. `aelf lock` on a retired statement printed success while `aelf locked` stayed empty. The lookup now excludes retired rows by default, matching `get_belief` (#1210). The two callers that use it as a UNIQUE-constraint guard rather than to read content opt in — `content_hash` is `NOT NULL UNIQUE` (#219), so a tombstone still owns its hash. Having seen the tombstone, `insert_or_corroborate` applies the ratified policy, tiered by who is asserting: an explicit user assertion revives the belief at the posterior it was retired at, with an audit row; background capture leaves it retired and records nothing, so an agent re-observing text cannot silently undo the user's curation.
Asserts the invariant rather than the call sites: which tier revives, and that the other tier writes nothing. Each tier carries a negative control on live content, because "capture did not revive" and "capture did nothing at all" are satisfied by the same assertions on a store where corroboration is broken outright. Also covers the second UNIQUE-constraint guard: a GC'd phantom is not regenerated by the next wonder pass.
There was a problem hiding this comment.
Sorry @robotrocketscience, you have reached your weekly rate limit of 500000 diff characters.
Please try again later or upgrade to continue using Sourcery
|
Warning Review limit reached
Next review available in: 30 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 (5)
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 |
Reviewer's GuideImplements tiered behavior for re-asserting retired beliefs and introduces lifecycle‑aware content-hash lookups, ensuring explicit user actions revive beliefs while background capture respects tombstones and avoids UNIQUE-constraint violations. Sequence diagram for tiered re-assertion of a retired beliefsequenceDiagram
actor User
participant MemoryStore
User->>MemoryStore: insert_or_corroborate(b, source_type in CORROBORATION_SOURCES_USER_EXPLICIT)
MemoryStore->>MemoryStore: get_belief_by_content_hash(b.content_hash, include_retired=True)
alt retired belief found
MemoryStore->>MemoryStore: restore_belief(existing.id)
MemoryStore->>MemoryStore: insert_feedback_event(belief_id=existing.id, source=FEEDBACK_SOURCE_REASSERT_REVIVE)
MemoryStore->>MemoryStore: record_corroboration(existing.id, source_type)
MemoryStore-->>User: (existing.id, False)
else no belief or live belief
MemoryStore->>MemoryStore: insert_belief(b)
MemoryStore-->>User: (b.id, True)
end
Sequence diagram for background capture respecting retired tombstonessequenceDiagram
participant BackgroundCapture
participant MemoryStore
BackgroundCapture->>MemoryStore: insert_or_corroborate(b, source_type not in CORROBORATION_SOURCES_USER_EXPLICIT)
MemoryStore->>MemoryStore: get_belief_by_content_hash(b.content_hash, include_retired=True)
alt retired belief found
MemoryStore-->>BackgroundCapture: (existing.id, False)
else no belief or live belief
MemoryStore->>MemoryStore: insert_belief(b)
MemoryStore->>MemoryStore: record_corroboration(b.id, source_type)
MemoryStore-->>BackgroundCapture: (b.id, True)
end
Sequence diagram for wonder_ingest UNIQUE-constraint guardsequenceDiagram
participant WonderLifecycle
participant MemoryStore
WonderLifecycle->>WonderLifecycle: wonder_ingest(phantoms)
WonderLifecycle->>WonderLifecycle: _constituent_key(phantom.constituent_belief_ids, phantom.generator)
WonderLifecycle->>MemoryStore: get_belief_by_content_hash(key, include_retired=True)
alt belief (including retired) exists
WonderLifecycle-->>WonderLifecycle: skipped += 1
else no belief
WonderLifecycle->>MemoryStore: insert_belief(phantom_belief)
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
PR-size soft capThis PR is over the advisory size threshold:
Bigger PRs collide with more open work, which under the parallel-session workflow tends to produce repeated This is advisory only — nothing is blocked. If the size is intentional (large refactor, module removal, generated code), apply the |
|
[claim:review:Toug:2026-07-30T21:11:01Z] |
Review — LGTM, verified independentlyPolicy is the ratified one (person revives, capture never does), so I did not The default flip is safeChanging Behaviour matches the table, across every sourceRebuilt the scenario independently — insert,
The specific defect is gone: capture no longer accrues evidence against a Re-asserting three times as a person yields 1 row and exactly 1
The tests are distinguishing, not decorativeReverted each half and confirmed the suite notices:
Note on the first: one of those two failures is Full suite on the branch: 6454 passed, 69 skipped, 71 xfailed. CI green. One observation, no action neededFor capture sources the skip path returns Adding |
|
[release:review:Toug:2026-07-30T21:21:49Z] |
|
merge-train: merged b90cab7 → |
Closes #1215.
get_belief_by_content_hashhad novalid_tofilter. #1210 gaveget_beliefone; this is the same hole on the lookup every ingest path actually resolves
through.
The symptom, reproduced end to end
After that second lock:
The store holds exactly one row and nothing can see it. The most explicit act
a user has for asserting ground truth is a no-op on every retrieval surface,
and the CLI says it worked.
This is the residual case #1164 did not cover. That fix correctly moved the
lock upgrade onto the resolved id rather than the minted lock id — which is
why
lock_level='user'now lands at all. It did not consider that theresolved row may be a tombstone, so the upgrade lands somewhere unreadable.
The plain-ingest case is the same mechanism without the lock: capture
re-observes a sentence the user retired, writes a corroboration row against
the tombstone, and drops the content.
Why this is a policy decision, not a filter
content_hashisNOT NULL UNIQUE(#219). So "insert a fresh row alongsidethe tombstone" is not available — the constraint forbids it. That leaves
revive-or-refuse, and the two differ in what they do to a user's curation.
Ratified: tier it by who is asserting.
cli_remember,mcp_remember—aelf lock,aelf remember, MCP twinsvalid_tocleared, FTS row restored, back in searchRe-typing a sentence yourself is a deliberate act. A transcript scan finding
it again is not, and under the old behaviour that scan was quietly accruing
evidence on a belief the user had removed.
Revival deliberately does not move the posterior — the belief comes back
exactly where it left. The re-assertion is recorded as a
belief_corroborationsrow, which is where that signal belongs, plus areassert:reviveaudit row so the transition is not silent in eitherdirection (
retirealready writesuser_retired_force).Verified across every source:
The two opt-ins are UNIQUE-constraint guards, not reads
The lookup now excludes retired rows by default. Two callers opt back in, and
both do so for the same reason
insert_belief's id-collision guard did in#1210 — a tombstone still owns its key:
insert_or_corroborate, which must see the tombstone in order to apply thepolicy above rather than blindly INSERT into a UNIQUE column.
wonder_ingest, whose dedupe key is a synthetic content hash.The second is load-bearing rather than defensive, and pinned. Reverting it:
It is also right on its own terms — a phantom the lifecycle GC'd should not be
regenerated by the next wonder pass.
Tests assert the invariant per tier, with controls
tests/test_reasserted_retired_belief_1215.py, 23 tests. Each tier carries anegative control on live content, because "capture did not revive" and
"capture did nothing at all" are satisfied by the same assertions on a store
where corroboration is broken outright.
Verified distinguishing rather than assumed — three independent reverts:
valid_towonder_ingestloses its opt-inIntegrityErrorAcceptance criteria
not silently dropped.
aelf lockon a retired statement surfaces the belief rather thanprinting success while leaving it invisible.
resolved to it by content hash.
re-assertion of live content still corroborates normally.
Verification
changing — the default flip is contained by the two opt-ins.
Noted, not fixed
Found while reviewing #1214, alongside
aelf lockon a retired statementleaving it retired-but-locked. That second one is fixed here as a
consequence rather than separately: the lock path is
cli_remember, so itnow revives and the belief is genuinely locked and visible. No separate
issue needed.
Summary by Sourcery
Define a tiered policy for re-asserting retired beliefs so that explicit user assertions revive them while background capture leaves them retired and invisible.
New Features:
Bug Fixes:
aelf lockon a retired belief so that it genuinely revives and locks the belief instead of reporting success on an unreadable row.content_hashUNIQUE constraint.Enhancements:
Documentation:
Tests: