feat(lock-consistency): annotation slots and lock-conflict mapping (#1175) - #1244
Conversation
…1175) Pure half of the build-first item: which retrieved beliefs slot-conflict with a user lock, with the noise suppression that makes the annotation readable. Suppression was measured, not asserted. Unsuppressed, 6.12% of retrieved unlocked beliefs conflict and 63.7% of those trace to one version-and-date lock. Two candidate rules were tried and rejected first: a per-retrieval share cap (refuted -- that lock flags a p50 of 2.9% of any one pack, so its noise is across queries not within one), and dropping multi-valued keys alone (helps to 4.21% but concentration got worse, 72.4% on the next version lock). Adding version-shaped keys and calendar-shaped values takes it to 1.38% with the top lock at 27.2%, spread across five locks. The filter lives here rather than in value_compare.extract_values: narrowing the shipped extractor would change what counts as a numeric slot for every consumer, which is the blast radius #1228 declined to take. Not yet wired into retrieval.
…de too (#1175) Passing the candidate's raw slots through instead of annotation_slots() passed all 23 tests -- every other case is suppressed on the lock side first, so the asymmetry was invisible to them. An ambiguous key is exactly as meaningless on the candidate side, and now a test says so.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
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: 22 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 (3)
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 |
robotrocketscience
added
the
attn:review
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 |
Reviewer's GuideImplements the library-side lock-consistency annotation for belief slots, including suppression rules for noisy numeric keys and a conflict-mapping helper, and pins the behavior with focused tests plus a changelog entry. Retrieval wiring and hook rendering are explicitly deferred. Sequence diagram for lock_conflict_annotations flowsequenceDiagram
participant Caller
participant lock_consistency
participant value_compare
Caller->>lock_consistency: lock_conflict_annotations(candidates, locked_pairs)
alt locked_pairs empty
lock_consistency-->>Caller: {}
else locked_pairs non_empty
lock_consistency->>lock_consistency: annotation_slots(slots) on each locked_pairs item
loop candidates
lock_consistency->>lock_consistency: annotation_slots(slots)
alt candidate_slots non_empty
loop filtered_locks
lock_consistency->>value_compare: find_conflicts(candidate_slots, lock_slots)
alt conflict_found
lock_consistency->>lock_consistency: out[belief_id] = lock.id
lock_consistency->>lock_consistency: break
end
end
end
end
lock_consistency-->>Caller: out
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
[claim:review:Setr:2026-07-31T17:22:32Z] |
|
[claim:review:Garsecg:2026-07-31T17:22:59Z] |
|
[claim:review:Toug:2026-07-31T17:23:01Z] |
|
[release:review:Garsecg:2026-07-31T17:23:04Z] |
|
[release:review:Toug:2026-07-31T17:23:06Z] |
|
Reviewed. The design is right and the reasoning is unusually well evidenced — refuting two candidate suppression rules with data before shipping the third is exactly the right order, and putting the filter here rather than narrowing Mutation-tested all six load-bearing behaviours. Five of six fail a named test; one does not.
1. Lock-side filtering is load-bearing but untestedReplacing It is not dead symmetry. The two suppressions keyed on vocabulary are symmetric enough that candidate-side filtering alone masks them, but the multi-valued rule is per-belief, so a key can be multi-valued in the lock and single-valued in the candidate: lock = ValueSlots(numeric=(NumericSlot("timeout", 30.0), NumericSlot("timeout", 60.0)), enum=())
cand = ValueSlots(numeric=(NumericSlot("timeout", 45.0),), enum=())
find_conflicts(annotation_slots(cand), annotation_slots(lock)) # False <- shipped
find_conflicts(annotation_slots(cand), lock) # True <- spuriousThat is precisely the annotation the multi-valued rule exists to prevent: a lock naming two different timeouts cannot disagree with anything. Worth a test — note 2.
|
robotrocketscience
added
the
ready-to-merge
|
[release:review:Setr:2026-07-31T17:27:36Z] |
github-actions
Bot
removed
the
ready-to-merge
|
merge-train: merged 00b5eae → |
robotrocketscience
removed
the
attn:review
1 participant
Builds the library half of #1175's build-first item, greenlit by the operator
today after its stated blocker (#1227) closed via #1228. ANNOTATE only —
DROP is not being built. Retrieval wiring and the
hook.pyrender landseparately, matching how
clustering.pyshipped its pure half first.Why this item
The L0 locked tier is materialised and injected ahead of everything else, and
nothing checks the L1/L2.5 tiers against it. So the model reads the user's most
explicit epistemic act and then reads a contradiction of it in the same prompt.
The suppression is the design, and two attempts at it were wrong
This issue said to design the noise suppression up front because 45% of
conflicts traced to one version-string lock. Measured across 189 real
prompts replayed against the live 44,584-belief store, counting conflicts on
retrieved unlocked beliefs — the population that matters, per the earlier
R&D round, not the whole store.
Baseline 553 / 9,036 = 6.12%, with
6d849282("aelfrice v3.6.0 shipped2026-06-19 …") alone at 63.7%.
Attempt 1 — cap the share of one pack a single lock may flag. Refuted. That
lock flags a p50 of 2.9% of any one pack (p90 = 10.2%). Its noise is spread
across queries, not concentrated within one, so a per-retrieval threshold
is structurally blind to it.
Attempt 2 — drop keys that are multi-valued within one belief. Helps, then
backfires. The dominant lock carries key
vwith four distinct values in onesentence (3.6, 3.5, 1.0, 3.7), which cannot be a functional dependency — a
principled rule needing no vocabulary. It takes 6.12% → 4.21% and evicts that
lock from the top five. But concentration got worse: the next
version-and-date lock took over at 72.4%. Multi-valuedness was a symptom.
What the extractor is actually doing, from that lock's slots:
Shipped rule — all three sources:
The residual spreads 27.2 / 16.8 / 14.4 / 12.8 / 11.2% across five locks —
roughly one annotated belief per 72 injected. That is the difference between an
annotation a user reads and one they learn to ignore.
Design decisions worth flagging
The filter lives here, not in
value_compare.extract_values. Narrowing theshipped extractor changes what counts as a numeric slot for every consumer —
the wider blast radius #1228 explicitly declined to take. The annotation gets
its own filtered view; the extractor is untouched.
Suppression is symmetric. An ambiguous key is exactly as meaningless on the
candidate side as on the lock side.
Suppression is slot-scoped, not belief-scoped. A belief carrying both a
version literal and a genuine disagreement is still annotated on the genuine
one.
Verification
Six mutations, all caught: removing each of the three rules independently,
widening the calendar band to every integer, and skipping candidate-side
suppression.
That last one escaped the first pass — it passed all 23 tests, because
every other case suppresses on the lock side first and so cannot see the
asymmetry. Found by running the mutation, not by reading the tests; there is
now a test for it, and the mutation fails that test and nothing else.
The controls are deliberate: a genuine conflict is annotated (without it,
over-broad suppression would satisfy every suppression test), a repeated
identical value is not treated as multi-valued, and values outside the
calendar band survive.
Full suite: 6701 passed, 69 skipped, 71 xfailed.
Not in this PR
Wiring into
retrieve_with_tiersand the<belief … conflicts-with="…">render at
hook.py, behind a default-off flag. Threading the annotation mapout of the tier walk touches the result shape, so it is its own reviewable
change — and this module can be read and benched without a hot-path edit.
Summary by Sourcery
Introduce a library module to compute lock-consistency annotations between locked and unlocked beliefs, without changing retrieval wiring or drop behaviour.
New Features:
lock_consistencymodule that identifies slot-level conflicts between user locks and candidate beliefs for annotation purposes only.Enhancements:
Documentation:
Tests: