feat(retrieval): semantic contradiction detector v3 — typed-slot value-comparison gate (#422) - #431
Merged
robotrocketscience merged 5 commits intoMay 5, 2026
Conversation
Reviewer's GuideAdds a deterministic typed-slot value-comparison contradiction detector (v3) behind a feature flag and integrates it into the relationship detector, with a new stdlib-only value_compare module, tests, docs, and a bench gate to enforce recall/precision targets. Sequence diagram for analyze with value-comparison gate enabledsequenceDiagram
actor Caller
participant RelationshipDetector as relationship_detector
participant ValueCompare as value_compare
Caller->>RelationshipDetector: analyze(text_a, text_b, use_value_comparison=true)
activate RelationshipDetector
alt use_value_comparison is true
RelationshipDetector->>ValueCompare: extract_values(text_a)
activate ValueCompare
ValueCompare-->>RelationshipDetector: ValueSlots slots_a
deactivate ValueCompare
RelationshipDetector->>ValueCompare: extract_values(text_b)
activate ValueCompare
ValueCompare-->>RelationshipDetector: ValueSlots slots_b
deactivate ValueCompare
RelationshipDetector->>ValueCompare: find_conflicts(slots_a, slots_b, numeric_rel_tol)
activate ValueCompare
ValueCompare-->>RelationshipDetector: tuple SlotConflict conflicts
deactivate ValueCompare
alt conflicts not empty
RelationshipDetector-->>Caller: RelationshipVerdict(label=contradicts, score=1.0, residual_overlap=0.0, rationale="value_comparison:...")
else no conflicts
RelationshipDetector->>RelationshipDetector: extract_signals, _residual_jaccard
RelationshipDetector-->>Caller: RelationshipVerdict (v1 path)
end
else use_value_comparison is false
RelationshipDetector->>RelationshipDetector: extract_signals, _residual_jaccard
RelationshipDetector-->>Caller: RelationshipVerdict (v1 path)
end
deactivate RelationshipDetector
Class diagram for value_compare typed-slot gate and relationship_detector integrationclassDiagram
class relationship_detector {
+RelationshipVerdict analyze(text_a: str, text_b: str, residual_overlap_min: float, use_value_comparison: bool)
+str classify(text_a: str, text_b: str, use_value_comparison: bool)
}
class RelationshipVerdict {
+str label
+float score
+float residual_overlap
+str rationale
}
class NumericSlot {
+str key
+float value
}
class EnumSlot {
+str category
+str group_id
+str member
}
class ValueSlots {
+tuple~NumericSlot~ numeric
+tuple~EnumSlot~ enum
}
class SlotConflict {
+str kind
+str key
+str value_a
+str value_b
}
class value_compare {
+ValueSlots extract_values(text: str)
+tuple~SlotConflict~ find_conflicts(slots_a: ValueSlots, slots_b: ValueSlots, numeric_rel_tol: float)
}
relationship_detector --> RelationshipVerdict : returns
relationship_detector ..> value_compare : imports
value_compare *-- ValueSlots : creates
ValueSlots *-- NumericSlot : numeric
ValueSlots *-- EnumSlot : enum
value_compare --> SlotConflict : returns
relationship_detector ..> SlotConflict : uses
relationship_detector ..> ValueSlots : uses
value_compare ..> NumericSlot : constructs
value_compare ..> EnumSlot : constructs
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✨ 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The rationale construction in
relationship_detector.analyzezips independently sortedkindsandkeys, which can mispair conflict kinds with unrelated keys; consider building the rationale directly from theconflictslist (e.g., using(c.kind, c.key)tuples) to keep kind/key pairs aligned. - Several docstrings/comments in
value_compare(e.g., references to units and(key, unit)comparisons in_NUMERIC_REandfind_conflicts) no longer match the implementation, which only tracks(key, value); tighten these descriptions to avoid confusion about unit handling and the actual comparison surface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The rationale construction in `relationship_detector.analyze` zips independently sorted `kinds` and `keys`, which can mispair conflict kinds with unrelated keys; consider building the rationale directly from the `conflicts` list (e.g., using `(c.kind, c.key)` tuples) to keep kind/key pairs aligned.
- Several docstrings/comments in `value_compare` (e.g., references to units and `(key, unit)` comparisons in `_NUMERIC_RE` and `find_conflicts`) no longer match the implementation, which only tracks `(key, value)`; tighten these descriptions to avoid confusion about unit handling and the actual comparison surface.
## Individual Comments
### Comment 1
<location path="src/aelfrice/relationship_detector.py" line_range="265-267" />
<code_context>
+ # this gate emits) so the auto-emit policy in #422
+ # acceptance #3 can route on (conflicts present AND
+ # score >= floor).
+ kinds = sorted({c.kind for c in conflicts})
+ keys = sorted({c.key for c in conflicts})
+ rationale = "value_comparison:" + ",".join(
+ f"{kind}={key}" for kind, key in zip(kinds, keys)
+ ) if len(kinds) == len(keys) else (
</code_context>
<issue_to_address>
**issue (bug_risk):** Rationale formatting can mis-associate conflict kinds and keys.
`kinds` and `keys` are independently deduped, sorted, and then zipped. This breaks the original `(kind, key)` associations, so a given `kind` may be paired with the wrong `key`, especially when mixing numeric and enum conflicts, producing a misleading rationale. Instead, build the rationale from the original `conflicts` sequence, preserving `(c.kind, c.key)` pairs (e.g., by iterating conflicts directly or sorting a collection of `(kind, key)` tuples).
</issue_to_address>
### Comment 2
<location path="src/aelfrice/value_compare.py" line_range="273-280" />
<code_context>
+) -> tuple[SlotConflict, ...]:
+ """Return all mutual-exclusion conflicts between two beliefs' slots.
+
+ Numeric conflict: same ``(key, unit)`` with values outside the
+ relative-tolerance band. Unit-mismatch is silent — different
+ units mean different scales and the comparator cannot adjudicate
+ without a unit-conversion table (out of scope).
</code_context>
<issue_to_address>
**suggestion:** Docstring describes key+unit semantics that the implementation no longer models.
The implementation now treats numeric conflicts as keyed only by `key`, while the docstring still describes `(key, unit)` semantics and the extractor no longer handles units. This inconsistency could mislead future readers into assuming unit-aware behavior. Please either update the docstring to match the current behavior or restore explicit unit handling if that’s still required.
```suggestion
"""Return all mutual-exclusion conflicts between two beliefs' slots.
Numeric conflict: same ``key`` with values outside the
relative-tolerance band (``numeric_rel_tol``). Units are not
interpreted or compared — any unit information present on slots
is ignored by this function.
Enum conflict: same ``category`` with different ``member`` values.
```
</issue_to_address>
### Comment 3
<location path="docs/value_compare.md" line_range="120" />
<code_context>
+Skip-on-no-corpus on public CI. Below-floor blocks the v3 default-on
+flip; the operator decides whether to widen `ENUM_VOCAB`, tune
+`DEFAULT_NUMERIC_REL_TOL`, or accept the precision/recall trade-off
+under audit-only surface (`aelf resolve` style human-in-loop).
+
+## Maintenance
</code_context>
<issue_to_address>
**suggestion (typo):** Consider changing "human-in-loop" to the more standard "human-in-the-loop".
"Human-in-loop" reads like a typo and is uncommon; using the standard "human-in-the-loop" would improve clarity without changing the meaning.
```suggestion
under audit-only surface (`aelf resolve` style human-in-the-loop).
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+265
to
+267
| kinds = sorted({c.kind for c in conflicts}) | ||
| keys = sorted({c.key for c in conflicts}) | ||
| rationale = "value_comparison:" + ",".join( |
There was a problem hiding this comment.
issue (bug_risk): Rationale formatting can mis-associate conflict kinds and keys.
kinds and keys are independently deduped, sorted, and then zipped. This breaks the original (kind, key) associations, so a given kind may be paired with the wrong key, especially when mixing numeric and enum conflicts, producing a misleading rationale. Instead, build the rationale from the original conflicts sequence, preserving (c.kind, c.key) pairs (e.g., by iterating conflicts directly or sorting a collection of (kind, key) tuples).
Comment on lines
+273
to
+280
| """Return all mutual-exclusion conflicts between two beliefs' slots. | ||
|
|
||
| Numeric conflict: same ``(key, unit)`` with values outside the | ||
| relative-tolerance band. Unit-mismatch is silent — different | ||
| units mean different scales and the comparator cannot adjudicate | ||
| without a unit-conversion table (out of scope). | ||
|
|
||
| Enum conflict: same ``category`` with different ``member`` values. |
There was a problem hiding this comment.
suggestion: Docstring describes key+unit semantics that the implementation no longer models.
The implementation now treats numeric conflicts as keyed only by key, while the docstring still describes (key, unit) semantics and the extractor no longer handles units. This inconsistency could mislead future readers into assuming unit-aware behavior. Please either update the docstring to match the current behavior or restore explicit unit handling if that’s still required.
Suggested change
| """Return all mutual-exclusion conflicts between two beliefs' slots. | |
| Numeric conflict: same ``(key, unit)`` with values outside the | |
| relative-tolerance band. Unit-mismatch is silent — different | |
| units mean different scales and the comparator cannot adjudicate | |
| without a unit-conversion table (out of scope). | |
| Enum conflict: same ``category`` with different ``member`` values. | |
| """Return all mutual-exclusion conflicts between two beliefs' slots. | |
| Numeric conflict: same ``key`` with values outside the | |
| relative-tolerance band (``numeric_rel_tol``). Units are not | |
| interpreted or compared — any unit information present on slots | |
| is ignored by this function. | |
| Enum conflict: same ``category`` with different ``member`` values. |
| Skip-on-no-corpus on public CI. Below-floor blocks the v3 default-on | ||
| flip; the operator decides whether to widen `ENUM_VOCAB`, tune | ||
| `DEFAULT_NUMERIC_REL_TOL`, or accept the precision/recall trade-off | ||
| under audit-only surface (`aelf resolve` style human-in-loop). |
There was a problem hiding this comment.
suggestion (typo): Consider changing "human-in-loop" to the more standard "human-in-the-loop".
"Human-in-loop" reads like a typo and is uncommon; using the standard "human-in-the-loop" would improve clarity without changing the meaning.
Suggested change
| under audit-only surface (`aelf resolve` style human-in-loop). | |
| under audit-only surface (`aelf resolve` style human-in-the-loop). |
|
This PR is now behind Auto-rebase was removed because the bot has no signing key; rebasing as the bot strips author signatures and the |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import pytest |
| """Unit tests for `aelfrice.value_compare` (#422).""" | ||
| from __future__ import annotations | ||
|
|
||
| import pytest |
robotrocketscience
force-pushed
the
feat/issue-422-value-comparison-detector
branch
from
May 5, 2026 16:40
224d1d5 to
cfc5a2e
Compare
…or (#422) Stdlib-only successor to #201's residual-overlap relatedness gate. Extracts numeric (key, value) pairs and curated enum (category, group_id, member) triples from belief prose; compares two beliefs' slots and emits SlotConflict on mutual exclusion. Numeric conflict: same key, different value (outside relative tolerance). Enum conflict: same category, different group_id — aliases within a group (sync ≡ synchronous, readonly ≡ read-only) do not conflict. Initial taxonomy covers 9 categories tuned for engineering / spec contradiction patterns; the dict is the single source of truth and grows on bench evidence. Deterministic, regex-only. No embeddings, no learned classifiers. Wiring into relationship_detector + tests follow.
30 tests cover: - numeric extraction (assignment, separator words, filler-key drop, negative/decimal/exponent, dedup, multi-kv) - enum extraction (simple match, alias group_id collapse, hyphenated members, word-boundary integrity) - comparator semantics (no-conflict on empty, numeric mismatch fires, within-tolerance silent, custom-tol override, enum group-disjoint conflict, alias pair no-conflict, default-state / completeness / access-mode categories, mixed numeric+enum) - vocab integrity (groups pairwise disjoint within a category) - determinism (byte-identical repeat, dataclass hash + round-trip)
…lue_comparison flag (#422) analyze() and classify() gain a use_value_comparison kwarg. Default False — preserves v1 behaviour byte-for-byte. When True, the typed- slot gate runs before the residual-overlap floor; if any mutual-exclusion conflict is found, verdict is contradicts at score 1.0 with rationale 'value_comparison:<kind>=<key>'. The slot match is the relatedness signal — token overlap is bypassed by design (that's the entire point of #422 vs #201's R2 floor). 9 integration tests pin: v1 backward compat (paraphrase pair still unrelated), v3 catches numeric/enum paraphrase contradictions, v3 respects alias collapse + within-tolerance numerics, classify() passes flag through, score pinned at 1.0 for slot-fire verdicts.
) Re-runs the labeled adversarial corpus through the v3 flag-on path (use_value_comparison=True). Computes confusion-matrix tp/fp/fn/tn on the contradicts-vs-not axis and asserts both recall and precision floors. Per #422 acceptance #2, calibrated against #201's R2 numbers (recall 0.033 / precision 0.667). Skip-on-no-corpus + row floor of 30 contradicts-labeled rows for stable recall measurement. Failure message surfaces both dimensions so the operator can see which floor is gating without re-running.
Documents extractor + comparator surface (numeric slots, enum vocab + alias groups, conflict semantics), integration via relationship_detector use_value_comparison flag, determinism guarantee, bench-gate location, and maintenance recipe (adding a category, tuning numeric tolerance). Acceptance #5 of the issue.
robotrocketscience
force-pushed
the
feat/issue-422-value-comparison-detector
branch
from
May 5, 2026 16:44
cfc5a2e to
ddffc9c
Compare
4 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
3 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.
Closes #422 (value-comparison route). Implements route (b) from the spec — stdlib-only, deterministic, no embeddings, no learned classifiers. The embedding route (route (a)) stays a separately filed v2.x decision per operator preference.
Why route (b) only
Operator preference: aelfrice prefers deterministic stdlib gates over embedding/ML in the runtime path. Embeddings introduce supply-chain + lifecycle costs and break replay-equality (#262 / #403) and bench-gate reproducibility. The value-comparison route hits the same failure mode (paraphrase contradictions that miss the residual-overlap floor) without those trade-offs.
What ships
aelfrice.value_compare(new module)Typed-slot extractor + mutual-exclusion comparator. Extracts
NumericSlot(key, value)andEnumSlot(category, group_id, member)from belief prose;find_conflicts()returnsSlotConflicttuples on key/category mismatches. Initial enum taxonomy: 9 categories (execution_mode,default_state,storage_mode,completeness,strictness,necessity,visibility,access_mode,determinism) with alias groups sosync≡synchronous.relationship_detector.analyze(..., use_value_comparison=True)The v3 gate runs before the residual-overlap floor — slot match is the relatedness signal, no token overlap required. Default
Falsepreserves v1 behaviour byte-for-byte. Slot-fire verdicts pin score=1.0 with rationalevalue_comparison:<kind>=<key>.Bench gate (
tests/bench_gate/test_contradiction_v3.py)Re-runs the labeled adversarial corpus through the flag-on path. Asserts recall ≥ 0.5 AND precision ≥ 0.7 per #422 acceptance #2 (vs #201 R2 numbers: recall 0.033, precision 0.667). Skip-on-no-corpus.
Acceptance map
use_value_comparison).POTENTIALLY_STALEemission is NOT wired here — that's feat(retrieval): edge-type-keyed rerank consumer — prerequisite for #387 POTENTIALLY_STALE demotion #421's substrate, separate PR.docs/value_compare.mdcovering taxonomy, integration, determinism, maintenance.Out of scope
aelf doctor/aelf resolveUX wiring — separate concern.Falseby default; flip is operator decision after lab-side bench.Test plan
uv run pytest tests/test_value_compare.py -v— 30 passeduv run pytest tests/test_relationship_detector_v3.py -v— 9 passeduv run pytest tests/test_relationship_detector.py -q— passes (v1 unchanged)uv run pytest --ignore=tests/bench_gate -q— 2495 passed, 23 skipped, no regressionsuv run pytest tests/bench_gate/test_contradiction_v3.py— skips cleanly without corpusAELFRICE_CORPUS_ROOT=... uv run pytest tests/bench_gate/test_contradiction_v3.py— runs once corpus is mounted; gates the v3 default-on flipStack note
This PR will stack behind #425 / #429's soak-gate timer (same
consecutive-green ≥ 7dReplay Soak Gate blocker).Summary by Sourcery
Introduce a deterministic typed-slot value-comparison contradiction gate and integrate it as an optional v3 path in the relationship detector, with supporting tests, benchmarks, and documentation.
New Features:
Enhancements:
Tests: