fix(storage): dedupe judgment-request generation across census cycles - #3290
Merged
Merged
Conversation
Problem: a census cycle re-encountering the same still-unjudged raw authority conflict (same raw_id/logical_source_key) minted a brand new judgment candidate assertion each time, because the assertion_id was derived from item.plan_id, which is itself derived from a fresh evidence digest every cycle. Prior cycles' still-pending candidates were never deduped against. Found live 2026-07-27 while manually triaging a batch of judgment candidates: 24 candidates in `judge --list` for what was actually 6 real conversations in conflict, ~4x the review burden the operator actually needed to do. What changed: _record_judgment_candidate now looks up an existing still-`candidate` judgment assertion for the same (raw_id, logical_source_key) pair before minting a new one, and refreshes it in place (new plan_id/evidence_digest) instead of creating a duplicate. This only dedupes pending-vs-pending: an assertion the operator already accepted/rejected/deferred is untouched, so a fresh judgment request can still be raised if the same conflict resurfaces after a prior disposition (the reason for a rejection may no longer apply once evidence changes). Verification: tests/unit/storage/test_browser_capture_origin_repair.py (14 tests, 1 new - test_repeat_census_of_same_pending_conflict_reuses_ one_judgment_candidate, directly exercises _record_judgment_candidate called twice with the same conflict identity and different plan_ids; confirmed it fails without the fix - asserts on the specific stale assertion_id mismatch). mypy --strict polylogue/storage/raw_reconciler.py clean. ruff check + format clean. Ref polylogue-rjtv
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughPending judgment candidates are now deduplicated by raw conflict identity across census cycles. The existing candidate identifier is reused when available, while newly encountered conflicts retain the prior identifier derivation. A unit test verifies reuse and latest-cycle data storage. ChangesRaw-authority candidate deduplication
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@polylogue/storage/raw_reconciler.py`:
- Around line 676-680: Update the identity lookup query in the raw
reconciliation flow to use SQLite’s null-safe IS comparison for
logical_source_key, allowing None to match stored JSON null values while
preserving non-null matching. Add a regression test covering repeated conflicts
where RawAuthorityFrontierItem.logical_source_key is None and verify no
duplicate is created.
- Around line 672-686: Make the lookup and creation in
_record_judgment_candidate atomic by moving the pending-row SELECT and
subsequent upsert_assertion call into the same immediate write transaction.
Preserve the existing lookup criteria and deterministic assertion_id fallback,
and ensure concurrent census cycles resolve through the transaction/conflict
path without duplicate candidate rows.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: f41e4c4a-2637-432f-8e07-d73c16c93470
📒 Files selected for processing (2)
polylogue/storage/raw_reconciler.pytests/unit/storage/test_browser_capture_origin_repair.py
Comment on lines
+672
to
+686
| pending_row = conn.execute( | ||
| """ | ||
| SELECT assertion_id FROM assertions | ||
| WHERE kind = 'judgment' AND status = 'candidate' | ||
| AND json_extract(value_json, '$.raw_id') = ? | ||
| AND json_extract(value_json, '$.logical_source_key') = ? | ||
| LIMIT 1 | ||
| """, | ||
| (item.raw_id, item.logical_source_key), | ||
| ).fetchone() | ||
| assertion_id = ( | ||
| str(pending_row[0]) | ||
| if pending_row is not None | ||
| else f"judgment:{_digest(['raw-authority-frontier', item.plan_id])}" | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C 6 \
'def _immediate_user_write_transaction|CREATE (UNIQUE )?INDEX|CREATE TABLE assertions' \
polylogue/storage/sqlite
rg -n -C 8 \
'pending_row = conn\.execute|def _record_judgment_candidate|def upsert_assertion' \
polylogue/storage/raw_reconciler.py \
polylogue/storage/sqlite/archive_tiers/user_write.pyRepository: Sinity/polylogue
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== Files matching raw_reconciler.py/user_write.py/assertions DDL ==\n'
fd -a 'raw_reconciler.py|user_write.py' polylogue/storage | sed 's#^\./##'
rg -n -C 4 'CREATE TABLE assertions|CREATE UNIQUE INDEX.*assertions|CREATE INDEX.*assertions' \
polylogue/storage/sqlite/migrations polylogue/storage/sqlite/archive_tiers \
--max-count 80
printf '\n== raw_reconciler relevant sections ==\n'
wc -l polylogue/storage/raw_reconciler.py
sed -n '620,710p' polylogue/storage/raw_reconciler.py
printf '\n== upsert_assertion usages and definitions ==\n'
rg -n -C 12 'def upsert_assertion|upsert_assertion|_record_judgment_candidate|pending_row = conn\.execute' \
polylogue/storage/raw_reconciler.py polylogue/storage/sqlite/archive_tiers/user_write.py \
--max-count 100Repository: Sinity/polylogue
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '== user_write immediate transaction and upsert_assertion implementation ==\n'
sed -n '86,130p' polylogue/storage/sqlite/archive_tiers/user_write.py
sed -n '1099,1220p' polylogue/storage/sqlite/archive_tiers/user_write.py
printf '\n== assertions table DDL and all assertion indexes/migrations ==\n'
python3 - <<'PY'
from pathlib import Path
import re
root = Path('polylogue/storage/sqlite')
for p in sorted(root.glob('**/*.sql')) + sorted(root.glob('archive_tiers/*.py')):
text = p.read_text()
if 'CREATE TABLE assertions' in text or 'idx_assertions' in text:
print(f'--- {p} ---')
for line in re.finditer(r'(?ms)CREATE TABLE assertions[\s\S]+?(?=\n\n|\Z)', text):
end = line.end()
start=max(0,end-800); print(text[start:end][:1200])
for l in re.finditer(r'(?m)CREATE .* INDEX .* assertions|idx_assertions', text):
start=max(0,l.start()-150); end=min(len(text),l.end()+150)
print(text[start:end])
PY
printf '\n== call sites around _record_judgment_candidate and transaction context ==\n'
rg -n -C 6 '_record_judgment_candidate|with closing|conn:|BEGIN IMMEDIATE|_immediate_user_write_transaction' \
polylogue/storage/raw_reconciler.py polylogue/storage -g '*.py' \
--max-count 120Repository: Sinity/polylogue
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
import re
for p in [Path('polylogue/storage/sqlite/archive_tiers/user.py'), Path('polylogue/storage/sqlite/archive_tiers/user_write.py')]:
text = p.read_text()
for name in ['CREATE TABLE assertions', 'CREATE UNIQUE INDEX', 'idx_assertions']:
if name in text:
print(f'--- assertions occurrences in {p} ---')
lines = text.splitlines()
for i,l in enumerate(lines,1):
if name in l:
lo=max(1,i-3); hi=min(len(lines),i+20 if 'idx_assertions' in l else i+8)
print(f'[{p}:{lo}-{hi}]')
for j in range(lo,hi+1):
print(f'{j:4}: {lines[j-1]}')
print()
print('--- focused assertions index/statements by regex ---')
for p in [Path('polylogue/storage/sqlite/archive_tiers/user.py'), Path('polylogue/storage/sqlite/archive_tiers/user_write.py')]:
text = p.read_text()
for m in re.finditer(r'(?ms)CREATE (UNIQUE )?INDEX\s+\Qidx_assertions\E[\s\S]{0,220}', text):
print('---', p, '---')
print(m.group(0))
PY
printf '\n===== raw_reconciler _record_judgment_candidate complete =====\n'
sed -n '640,705p' polylogue/storage/raw_reconciler.py
printf '\n===== user_write immediate transaction and upsert_assertion =====\n'
sed -n '86,120p' polylogue/storage/sqlite/archive_tiers/user_write.py
sed -n '1099,1195p' polylogue/storage/sqlite/archive_tiers/user_write.py
printf '\n===== all candidate creation paths that derive assertion IDs outside immediate_user_write_transaction =====\n'
sed -n '1215,1295p' polylogue/storage/sqlite/archive_tiers/user_write.py
sed -n '1315,1630p' polylogue/storage/sqlite/archive_tiers/user_write.pyRepository: Sinity/polylogue
Length of output: 6326
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '===== raw_reconciler _record_judgment_candidate complete =====\n'
sed -n '640,705p' polylogue/storage/raw_reconciler.py
printf '\n===== user_write immediate transaction and upsert_assertion =====\n'
sed -n '86,120p' polylogue/storage/sqlite/archive_tiers/user_write.py
sed -n '1099,1195p' polylogue/storage/sqlite/archive_tiers/user_write.py
printf '\n===== assertions table DDL and indexes from user.py =====\n'
sed -n '20,62p' polylogue/storage/sqlite/archive_tiers/user.py
printf '\n===== candidate creation paths that derive assertion IDs outside immediate_user_write_transaction =====\n'
sed -n '1215,1290p' polylogue/storage/sqlite/archive_tiers/user_write.py
sed -n '1315,1630p' polylogue/storage/sqlite/archive_tiers/user_write.pyRepository: Sinity/polylogue
Length of output: 29642
Make raw-authority candidate creation atomic.
_record_judgment_candidate selects an existing candidate row outside the upsert_assertion immediate transaction, so two concurrent census cycles can both miss, derive the same item.plan_id assertion id, and both insert. Wrap the lookup-and-upsert in one write transaction, or add a durable conflict-identity invariant for the pending raw-authority frontier assertion and handle its conflict path.
🤖 Prompt for 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.
In `@polylogue/storage/raw_reconciler.py` around lines 672 - 686, Make the lookup
and creation in _record_judgment_candidate atomic by moving the pending-row
SELECT and subsequent upsert_assertion call into the same immediate write
transaction. Preserve the existing lookup criteria and deterministic
assertion_id fallback, and ensure concurrent census cycles resolve through the
transaction/conflict path without duplicate candidate rows.
Comment on lines
+676
to
+680
| AND json_extract(value_json, '$.raw_id') = ? | ||
| AND json_extract(value_json, '$.logical_source_key') = ? | ||
| LIMIT 1 | ||
| """, | ||
| (item.raw_id, item.logical_source_key), |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Handle logical_source_key=None in the identity lookup.
RawAuthorityFrontierItem.logical_source_key is nullable, but json_extract(...) = ? with None evaluates to SQL NULL, never true. Repeated conflicts without a logical-source key will still create duplicates. Use SQLite null-safe comparison (IS ?) and add a nullable-key regression case.
🤖 Prompt for 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.
In `@polylogue/storage/raw_reconciler.py` around lines 676 - 680, Update the
identity lookup query in the raw reconciliation flow to use SQLite’s null-safe
IS comparison for logical_source_key, allowing None to match stored JSON null
values while preserving non-null matching. Add a regression test covering
repeated conflicts where RawAuthorityFrontierItem.logical_source_key is None and
verify no duplicate is created.
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
1 participant
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.
Summary
_record_judgment_candidatenow looks up an existing still-candidatejudgment assertion for the same(raw_id, logical_source_key)conflict identity before minting a new one, refreshing it in place instead of creating a duplicate.Problem
A census cycle re-encountering the same still-unjudged raw authority conflict minted a brand new judgment candidate each time, because the assertion_id was derived from
item.plan_id, itself derived from a fresh evidence digest every cycle. Found live 2026-07-27 while manually triaging judgment candidates: 24 candidates injudge --listfor what was actually 6 real conversations in conflict (~4x the review burden actually needed).Solution
Dedupe is scoped to pending-vs-pending only: an assertion already accepted/rejected/deferred by the operator is untouched, so a fresh judgment request can still be raised if the same conflict resurfaces after a prior disposition (the reason for a rejection may no longer apply once evidence changes).
Verification
tests/unit/storage/test_browser_capture_origin_repair.py: 14 tests, 1 new (test_repeat_census_of_same_pending_conflict_reuses_one_judgment_candidate) — directly exercises_record_judgment_candidatecalled twice with the same conflict identity and different plan_ids; confirmed it fails without the fix.mypy --strict polylogue/storage/raw_reconciler.py: cleanruff check+format: cleanRef polylogue-rjtv
Summary by CodeRabbit
Bug Fixes
Tests