fix(storage): scope quarantine-refinement inspection by logical source key - #3371
Merged
Merged
Conversation
…e key ## Problem Discovered live 2026-07-28 while investigating polylogue-zaiz (the residual "3 fan-out sessions stuck" finding from the ihc8/ewfp incident chain): `_inspect_quarantined_accepted_raw` looked up the accepted-head row by `accepted_raw_id` alone, requiring exactly one match system-wide -- the same architectural gap `_inspect_duplicate_raw_identity` had before polylogue-ihc8's fix. Forked/subagent/resumed sessions can physically replay the identical parent evidence, so the exact same raw_id can legitimately be the accepted head of several logical source keys/sessions at once. For a fan-out raw, this unconditionally raised "expected one accepted head, found N" for EVERY sibling, regardless of the underlying quarantine reason -- permanently non-actionable, not a crash this time (the census-level `unresolved_provenance` fallback in `_classify_frontier` degrades gracefully), but structurally blocking any of them from ever being refined. Investigation also found a much larger, separate fact while scoping this down: 15,798 of ~41,334 `raw_sessions` rows (38% of the archive) carry `revision_kind='unknown'` -- a pre-existing, already-known backlog predating this session, unrelated to the scoping bug and out of scope for a single fix. Confirmed the two are independent: the untyped/quarantined envelope path already handles `revision_kind='unknown'` correctly: the sole blocker for genuine fan-out cases is the head lookup's scoping. ## Solution - `_inspect_quarantined_accepted_raw` now requires a `logical_source_key` parameter, scoping the `heads`/`session_rows`/`applications` queries to it (mirroring `_inspect_duplicate_raw_identity`'s fix). - `inspect_quarantined_accepted_raws` now takes `(raw_id, logical_source_key)` pairs instead of bare raw_ids (blob-budget partitioning still dedupes by raw_id alone; the per-session identity proof runs once per pair). - `_strategy_overrides` builds per-session override keys (`_quarantine_override_key`) instead of bare-raw_id keys for the quarantine path specifically; the frontier-item lookup tries the session-scoped key first, falling back to the bare key unchanged for browser-origin/conflict overrides (which are properties of the raw itself, not per-session, and don't have this fan-out shape). - `_apply_strategy`'s REFINE_QUARANTINE branch threads the same `logical_source_key` through, and gains the identical graceful "ineligible" no-op handling already shipped for FOLD_DUPLICATE_ALIAS (polylogue-ewfp, #3369) -- a fan-out raw's refinement mutates the SHARED `raw_sessions` row itself (not a distinct canonical twin), so only one sibling can ever win; the others must resolve as a permanent, non-crashing ineligibility if selected in the same apply batch. ## Verification - `devtools test tests/unit/storage/test_quarantined_accepted_raw_repair.py tests/unit/storage/test_duplicate_raw_identity_repair.py tests/unit/storage/test_raw_authority_ledger.py` -> 52 passed (new regression tests reproduce the exact fan-out shape; anti-vacuity confirmed by reverting the scoping fix and observing the exact pre-fix failure) - `devtools test tests/unit/storage/ -k "raw_reconciler or raw_authority or duplicate or quarantine or repair"` -> 228 passed, 1 pre-existing unrelated failure (confirmed via git stash on master) - `mypy polylogue/storage/raw_reconciler.py polylogue/storage/repair.py` -> clean - `devtools verify --quick` -> exit 0 Ref polylogue-zaiz, polylogue-ihc8 Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughQuarantine inspection and refinement now scope proofs, overrides, and apply-time validation by ChangesQuarantine fan-out refinement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RawAuthorityActuator
participant FrontierClassification
participant ScopedQuarantineInspection
participant QuarantineRefinement
RawAuthorityActuator->>FrontierClassification: classify logical source key
FrontierClassification->>ScopedQuarantineInspection: request scoped quarantine proof
ScopedQuarantineInspection-->>FrontierClassification: eligible or ineligible result
RawAuthorityActuator->>ScopedQuarantineInspection: re-inspect locked raw
ScopedQuarantineInspection-->>RawAuthorityActuator: scoped witness
RawAuthorityActuator->>QuarantineRefinement: refine eligible quarantine
QuarantineRefinement-->>RawAuthorityActuator: refinement result
Possibly related PRs
Suggested labels: 🚥 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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bb6b7ed0c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| quarantine_locked = _inspect_quarantined_accepted_raw( | ||
| root, item.raw_id, conn=source_conn, logical_source_key=logical_source_key | ||
| ) | ||
| if quarantine_locked.status == "ineligible": |
There was a problem hiding this comment.
When a plan was eligible during authorization but the locked inspection becomes ineligible—for example because its head, application receipt, session, or blob evidence changed—this branch commits and returns normally, so apply_raw_authority_frontier records the plan as EXECUTED, leaves retryable_plan_count at zero, and reports success despite performing no repair or proving a terminal state. The claimed fan-out exception cannot be distinguished here, and two different logical keys cannot both pass the inspector's singleton parsed-identity equality in the first place; keep this proof-loss path fail-closed or recognize only a narrowly verified terminal sibling outcome.
AGENTS.md reference: AGENTS.md:L157-L159
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 @.beads/issues.jsonl:
- Line 400: Update the issue metadata for polylogue-zaiz to match the
investigation and implemented scope: close the narrow fan-out-session task with
a resolution noting the broader revision_kind='unknown' backfill is out of
scope, or retitle and re-scope it explicitly to that unresolved backfill instead
of leaving the original broad task open.
In `@polylogue/storage/raw_reconciler.py`:
- Around line 1307-1329: The ineligible branch in the quarantine reconciliation
flow incorrectly treats transient read and blob failures as permanently
resolved. Update the handling around quarantine_locked.status and
_inspect_quarantined_accepted_raw so only genuinely terminal
envelope/content-mismatch causes return the benign repaired response; propagate
authority-tier read errors and missing or hash-invalid retained blobs through
the existing retryable error path.
In `@polylogue/storage/repair.py`:
- Around line 755-760: Update the rejection reason in the len(session_rows)
check within the repair flow to describe that the accepted head does not resolve
to exactly one indexed session for the specified raw and session scope, removing
the inaccurate “unique indexed session” claim while preserving the existing
predicate and quarantine behavior.
In `@tests/unit/storage/test_quarantined_accepted_raw_repair.py`:
- Around line 370-401: Merge the consecutive sqlite3.connect blocks in the test
setup into one connection context, keeping both the
raw_session_memberships/raw_membership_census inserts and the raw_sessions
update in the same transaction before the existing commit.
- Around line 444-487: Add coverage for the quarantine_locked.status ==
"ineligible" early return in _apply_strategy by first applying session_a’s valid
plan, then attempting to apply a stale-but-selected plan for session_b. Assert
the sibling attempt is a no-op with the expected non-success/retry behavior,
while preserving session_b’s accepted-head row and avoiding unintended changes.
🪄 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: 1b8aa0ee-81fd-470d-8aa4-786b9afc8ee3
📒 Files selected for processing (4)
.beads/issues.jsonlpolylogue/storage/raw_reconciler.pypolylogue/storage/repair.pytests/unit/storage/test_quarantined_accepted_raw_repair.py
| {"_type":"issue","id":"polylogue-sru.3","title":"Benign-recovery vs consequential-silence split by handler kind","description":"Read failures are ~94% silent but 'tried another path' is usually benign; Bash/test failures are the consequential class. Scope the headline to consequential handler kinds or add an explicit split — credibility depends on not inflating with trivial recoveries.","design":"Handler kind is already available on the paired failure row (actions lane exposes handler/tool). Define the consequential set explicitly in code (Bash/test/build/write-class handlers) and the benign-recovery set (Read/Glob/Grep-class 'tried another path'), emit split headline rows: silent-proceed among consequential vs among all. Keep the mapping a named constant with a rationale comment so reviewers can argue with it. Report both; never let the headline mix classes silently. Same regen/tests as the other methodology children.","status":"closed","priority":1,"issue_type":"task","assignee":"Sinity","owner":"ezo.dev@gmail.com","created_at":"2026-07-03T04:31:28Z","created_by":"Sinity","updated_at":"2026-07-03T07:58:08Z","started_at":"2026-07-03T07:55:37Z","closed_at":"2026-07-03T07:58:08Z","close_reason":"Completed: claim-vs-evidence now reports a first-class handler-class split separating consequential shell/edit/write-class tool failures from benign read/search/path-discovery failures and other tools. The regenerated active-archive artifact shows consequential=4,177 failures with 921 silent-proceed (22.0% lower bound), benign_recovery=633 with 166 silent-proceed (26.2%), and other=190 with 92 silent-proceed (48.4%). Focused tests and demo shelf checks passed.","labels":["area:substrate","campaign"],"dependencies":[{"issue_id":"polylogue-sru.3","depends_on_id":"polylogue-sru","type":"parent-child","created_at":"2026-07-03T06:31:28Z","created_by":"Sinity","metadata":"{}"}],"dependency_count":0,"dependent_count":1,"comment_count":0} | ||
| {"_type":"issue","id":"polylogue-sru.1","title":"Expose action-unit outcome fields + followup_class as product capability","description":"Capabilities-may-not-be-silos gate for the campaign: the facts the report needs must become composable query capability. After this, the whole report is `actions where is_error:true | group by session.origin, followup_class | count` and every future cut (model/tool/repo/time) is free.","design":"1) is_error/exit_code are normalized at parse time (sources/parsers/base_models.py:74-75) but ActionQueryRowPayload (surfaces/payloads.py:~1298) carries neither — add as filterable/groupable action-unit fields. 2) Add derived followup_class (acknowledged|silent_proceed|wordless_continuation|ambiguous) + followup_message_ref computed in the source-derived lowering (no cache tables). 3) Reduce devtools workspace claim-vs-evidence to a render preset over these query strings, or retire it. Touchpoint chain: stage parser -\u003e AST to_payload -\u003e executor -\u003e metadata.py aggregate_group_fields -\u003e shell_completion_values.py -\u003e devtools render openapi + cli-output-schemas + cli-reference. Line refs pre-07-03; re-locate.","acceptance_criteria":"Fixture session with known unacknowledged failure fires via pure query strings; report README numbers reproducible from the printed queries.","notes":"Completed: action-unit outcome follow-up classification is now shared query capability. is_error/exit_code were already wired; this slice added source-derived followup_class and followup_message_ref over existing actions/messages/blocks, exposed followup_class as filterable/groupable action metadata, added action row payload fields, routed root CLI terminal-unit aggregate expressions before session-selector compilation, and moved the report classifier from scripts into polylogue.archive.actions.followup. Reproduction/query forms are now printed in .agent/demos/claim-vs-evidence/PUBLIC_REPRODUCTION.md: actions where is_error:true | group by followup_class | count; actions where followup_class:silent_proceed. Verification: focused DSL/report/CLI tests passed; active demo packet regenerated over archive root /home/sinity/.local/share/polylogue schema v23 with 41,886 structured failures and 5,000 inspected; devtools verify --quick passed run 20260703T092510Z-quick-718233-46e8b587.","status":"closed","priority":1,"issue_type":"feature","assignee":"Sinity","owner":"ezo.dev@gmail.com","created_at":"2026-07-03T04:31:27Z","created_by":"Sinity","updated_at":"2026-07-03T09:25:36Z","started_at":"2026-07-03T09:05:37Z","closed_at":"2026-07-03T09:25:36Z","close_reason":"Completed","labels":["area:query","area:substrate","campaign"],"dependencies":[{"issue_id":"polylogue-sru.1","depends_on_id":"polylogue-sru","type":"parent-child","created_at":"2026-07-03T06:31:26Z","created_by":"Sinity","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} | ||
| {"_type":"issue","id":"polylogue-zaiz","title":"Resolve quarantined fan-out sessions via refine_quarantined_raw actuator","status":"open","priority":2,"issue_type":"task","owner":"ezo.dev@gmail.com","created_at":"2026-07-28T11:10:11Z","created_by":"Sinity","updated_at":"2026-07-28T11:10:11Z","dependencies":[{"issue_id":"polylogue-zaiz","depends_on_id":"polylogue-ihc8","type":"relates-to","created_at":"2026-07-28T13:10:35Z","created_by":"Sinity","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} | ||
| {"_type":"issue","id":"polylogue-zaiz","title":"Resolve quarantined fan-out sessions via refine_quarantined_raw actuator","notes":"2026-07-28 investigation (proper, careful treatment as requested): read\n_inspect_quarantined_accepted_raw (polylogue/storage/repair.py:698) in\nfull before touching anything.\n\nFound TWO layered facts, not one isolated bug:\n\n1. Same architectural gap class as ihc8/dmvo/ewfp: this function's `heads`\n lookup (`SELECT ... FROM raw_revision_heads WHERE accepted_raw_id = ?`)\n is unscoped by logical_source_key and requires `len(heads) == 1`,\n exactly like ihc8's original bug in _inspect_duplicate_raw_identity.\n For this fan-out shape (3 sessions sharing one stale raw_id), this\n ALWAYS returns \"expected one accepted head, found 3\" -- structurally\n ineligible for refinement regardless of anything else. This part alone\n would need the same per-session scoping fix ihc8 got.\n\n2. MORE IMPORTANTLY -- a much bigger, pre-existing finding that changes\n the scope entirely. Direct read-only query:\n\n SELECT revision_kind, revision_authority, COUNT(*) FROM raw_sessions\n GROUP BY revision_kind, revision_authority;\n\n unknown|quarantined|15798\n full|byte_proven|13467\n full|quarantined|7773\n append|byte_proven|2396\n append|quarantined|1900\n\n 15,798 of ~41,334 raw_sessions rows (38% of the ENTIRE archive) carry\n revision_kind='unknown' -- the schema's bare default, meaning these rows\n predate whatever backfill/migration established byte-proven revision\n classification and were simply never classified. This is NOT specific\n to the 3 stuck fan-out sessions; it is the exact same large-scale\n pre-existing gap already surfaced this session as \"2214 active index\n raw seeds with a broken predecessor chain\" in `polylogue status --full`.\n\n Critically: `_inspect_quarantined_accepted_raw`'s eligibility proof\n compares the raw's actual envelope against an EXPECTED envelope that\n hardcodes `RawRevisionKind.FULL.value`\n (polylogue/storage/repair.py, expected_envelope construction). A raw\n whose OWN stored revision_kind is 'unknown' can never match this\n envelope -- meaning the refine_quarantined_raw actuator, AS DESIGNED,\n cannot resolve ANY of the 15,798 unknown-kind rows, not just these 3.\n Refining them requires first backfilling their revision_kind\n classification post-hoc (determining whether each was originally a full\n snapshot or an append delta from other evidence), which is an entirely\n separate, much larger undertaking than fixing a code bug -- likely the\n real substance behind the \"archive convergence\" work already assessed\n this session (z9gh's dependency tree, b5l durable-tier transition,\n 1xc scale-hardening) as multi-week, not something to force here.\n\nConclusion: these 3 sessions are not a small scoped follow-up like\nihc8/ewfp were. They are the visible tip of the archive's large,\nalready-known 38%-of-rows revision-classification backfill gap. Scoping\nthis bead down to \"fix these 3 sessions\" would be misleading -- either\n(a) fix only the head-scoping bug (matching ihc8's pattern), which would\nstill not let these 3 refine successfully since their revision_kind is\n'unknown', not FULL, or (b) undertake the real fix (revision_kind\nbackfill for 15,798 rows), which is out of scope for a single bead and\nbelongs with the already-tracked large-scale convergence program.\n\nRecommend: close this bead's narrow framing as \"investigated, correctly\nre-scoped\" rather than continuing to treat it as an isolated fix. The\nhead-scoping bug (item 1 above) is real and independently worth fixing\n(it affects the refine_quarantined_raw actuator for EVERY future fan-out\ncase, not just these 3 sessions) -- filing that narrowly-scoped slice\nseparately, since it's genuinely small and safe regardless of the larger\nbackfill question.\n","status":"open","priority":2,"issue_type":"task","owner":"ezo.dev@gmail.com","created_at":"2026-07-28T11:10:11Z","created_by":"Sinity","updated_at":"2026-07-28T11:22:47Z","dependencies":[{"issue_id":"polylogue-zaiz","depends_on_id":"polylogue-ihc8","type":"relates-to","created_at":"2026-07-28T13:10:35Z","created_by":"Sinity","metadata":"{}"}],"dependency_count":0,"dependent_count":0,"comment_count":0} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Synchronize the issue metadata with the investigation outcome.
The notes recommend closing the narrow bead framing, and this PR fixes the independently identified head-scoping bug, but the entry remains status: "open" under the original broad title. Either close it with the scoped resolution or retitle/re-scope it explicitly to the unresolved revision_kind='unknown' backfill.
🤖 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 @.beads/issues.jsonl at line 400, Update the issue metadata for
polylogue-zaiz to match the investigation and implemented scope: close the
narrow fan-out-session task with a resolution noting the broader
revision_kind='unknown' backfill is out of scope, or retitle and re-scope it
explicitly to that unresolved backfill instead of leaving the original broad
task open.
Comment on lines
+1307
to
+1329
| if quarantine_locked.status == "ineligible": | ||
| # polylogue-zaiz: mirrors the fold_duplicate_alias | ||
| # batch-race fix (polylogue-ewfp, #3369). A fan-out raw | ||
| # shared by several sessions can have more than one | ||
| # sibling selected together in the same apply batch | ||
| # (all looked eligible at the pre-apply census | ||
| # snapshot); once one sibling's refinement commits, | ||
| # every other sibling's own re-inspection legitimately | ||
| # finds "ineligible" -- permanently, not transiently. | ||
| # The witness comparison below is skipped deliberately: | ||
| # it exists to catch drift that would invalidate an | ||
| # ELIGIBLE refinement between census and apply, which | ||
| # does not apply once no refinement is being attempted | ||
| # at all. | ||
| source_conn.commit() | ||
| return json_document( | ||
| { | ||
| "strategy": item.actuator.value, | ||
| "repaired_count": 0, | ||
| "already_repaired_count": 0, | ||
| "ineligible_reason": quarantine_locked.reason, | ||
| } | ||
| ) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
ineligible is treated as benign for every cause, including transient read failures.
_inspect_quarantined_accepted_raw returns the same ineligible status for genuinely terminal fan-out races and for transient/environmental faults — e.g. authority tiers are unreadable: {exc} on sqlite3.Error (polylogue/storage/repair.py Line 842-844) and retained raw blob is missing or fails its content hash. Those now record EXECUTED (permanently resolved) instead of RETRYABLE, so the operator receipt claims a resolved plan for what is actually an I/O fault. The post-apply census will still re-surface the row, but the outcome ledger is misleading.
Consider gating the benign path on the causes that are genuinely terminal (e.g. envelope/content-mismatch reasons) and letting read/blob faults raise as before.
🤖 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 1307 - 1329, The ineligible
branch in the quarantine reconciliation flow incorrectly treats transient read
and blob failures as permanently resolved. Update the handling around
quarantine_locked.status and _inspect_quarantined_accepted_raw so only genuinely
terminal envelope/content-mismatch causes return the benign repaired response;
propagate authority-tier read errors and missing or hash-invalid retained blobs
through the existing retryable error path.
Comment on lines
755
to
760
| session_rows = conn.execute( | ||
| "SELECT session_id, raw_id, content_hash FROM index_tier.sessions WHERE raw_id = ?", | ||
| (raw_id,), | ||
| "SELECT session_id, raw_id, content_hash FROM index_tier.sessions WHERE raw_id = ? AND session_id = ?", | ||
| (raw_id, session_id), | ||
| ).fetchall() | ||
| if len(session_rows) != 1 or str(session_rows[0]["session_id"]) != session_id: | ||
| if len(session_rows) != 1: | ||
| return _quarantined_raw_item(raw_id, "accepted head is not the raw's unique indexed session") |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Scoping is right; the ineligible reason is now stale.
The predicate no longer proves the session is the raw's unique indexed session (by design — fan-out siblings legitimately share the raw), so the message misdescribes what failed. Reword to reflect the scoped check.
✏️ Proposed wording fix
- return _quarantined_raw_item(raw_id, "accepted head is not the raw's unique indexed session")
+ return _quarantined_raw_item(
+ raw_id, f"accepted head session {session_id} has no unique indexed session row on this raw"
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| session_rows = conn.execute( | |
| "SELECT session_id, raw_id, content_hash FROM index_tier.sessions WHERE raw_id = ?", | |
| (raw_id,), | |
| "SELECT session_id, raw_id, content_hash FROM index_tier.sessions WHERE raw_id = ? AND session_id = ?", | |
| (raw_id, session_id), | |
| ).fetchall() | |
| if len(session_rows) != 1 or str(session_rows[0]["session_id"]) != session_id: | |
| if len(session_rows) != 1: | |
| return _quarantined_raw_item(raw_id, "accepted head is not the raw's unique indexed session") | |
| session_rows = conn.execute( | |
| "SELECT session_id, raw_id, content_hash FROM index_tier.sessions WHERE raw_id = ? AND session_id = ?", | |
| (raw_id, session_id), | |
| ).fetchall() | |
| if len(session_rows) != 1: | |
| return _quarantined_raw_item( | |
| raw_id, f"accepted head session {session_id} has no unique indexed session row on this raw" | |
| ) |
🤖 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/repair.py` around lines 755 - 760, Update the rejection
reason in the len(session_rows) check within the repair flow to describe that
the accepted head does not resolve to exactly one indexed session for the
specified raw and session scope, removing the inaccurate “unique indexed
session” claim while preserving the existing predicate and quarantine behavior.
Comment on lines
+370
to
+401
| with sqlite3.connect(root / "source.db") as source: | ||
| source.execute( | ||
| """ | ||
| INSERT INTO raw_session_memberships ( | ||
| raw_id, logical_source_key, provider_session_id, source_revision, | ||
| normalized_content_hash, message_count, acquisition_generation, | ||
| revision_authority | ||
| ) VALUES (?, ?, ?, ?, ?, ?, 0, 'quarantined') | ||
| """, | ||
| (raw_id, key_a, "fanout-quarantine", content_hash_a.hex(), content_hash_a, len(session_a.messages)), | ||
| ) | ||
| source.execute( | ||
| """ | ||
| INSERT INTO raw_membership_census ( | ||
| raw_id, parser_fingerprint, status, member_count, censused_at_ms | ||
| ) VALUES (?, 'revision-membership-v1', 'complete', 1, 0) | ||
| """, | ||
| (raw_id,), | ||
| ) | ||
| source.commit() | ||
| with sqlite3.connect(root / "source.db") as source: | ||
| source.execute( | ||
| """ | ||
| UPDATE raw_sessions | ||
| SET logical_source_key = NULL, revision_kind = 'unknown', source_revision = NULL, | ||
| baseline_raw_id = NULL, acquisition_generation = NULL, | ||
| revision_authority = 'quarantined' | ||
| WHERE raw_id = ? | ||
| """, | ||
| (raw_id,), | ||
| ) | ||
| source.commit() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
Merge the two consecutive source.db connections.
Lines 370-389 and 390-401 open and commit source.db twice back-to-back with no intervening reader; a single block is equivalent and cheaper.
🤖 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 `@tests/unit/storage/test_quarantined_accepted_raw_repair.py` around lines 370
- 401, Merge the consecutive sqlite3.connect blocks in the test setup into one
connection context, keeping both the
raw_session_memberships/raw_membership_census inserts and the raw_sessions
update in the same transaction before the existing commit.
Comment on lines
+444
to
+487
| def test_quarantine_refinement_applies_for_the_matching_sibling_only(tmp_path: Path) -> None: | ||
| """polylogue-zaiz regression: the genuinely-matching sibling refines cleanly. | ||
|
|
||
| Unlike duplicate-alias fan-out (where eligibility is a race -- any | ||
| sibling looks eligible until a canonical twin is claimed), quarantine- | ||
| refinement eligibility is a fixed content-match fact: only | ||
| ``session_a`` (whose accepted content genuinely matches the raw) is | ||
| ever ``SAFELY_REKEYABLE``. Applying its plan must succeed end-to-end | ||
| without disturbing ``session_b``'s own (permanently ineligible, | ||
| unaffected) accepted-head row. | ||
| """ | ||
| raw_id, heads = _seed_quarantined_raw_fanout(tmp_path) | ||
| (session_a, key_a), (session_b, key_b) = heads | ||
|
|
||
| preview = inspect_raw_authority_frontier(_config(tmp_path)) | ||
| selected = next( | ||
| item | ||
| for item in preview.items | ||
| if item.raw_id == raw_id | ||
| and item.logical_source_key == key_a | ||
| and item.state is RawAuthorityFrontierState.SAFELY_REKEYABLE | ||
| ) | ||
|
|
||
| report = apply_raw_authority_frontier( | ||
| _config(tmp_path), | ||
| preview_census_id=preview.census_id, | ||
| selected_plan_ids=(selected.plan_id,), | ||
| ) | ||
|
|
||
| assert report.executed_plan_count == 1 | ||
| assert report.retryable_plan_count == 0 | ||
| assert report.success | ||
|
|
||
| with sqlite3.connect(tmp_path / "source.db") as source: | ||
| assert source.execute( | ||
| "SELECT revision_authority, baseline_raw_id FROM raw_sessions WHERE raw_id = ?", | ||
| (raw_id,), | ||
| ).fetchone() == ("byte_proven", raw_id) | ||
| with sqlite3.connect(tmp_path / "index.db") as index: | ||
| # session_b's own head still points at the shared raw_id -- the | ||
| # refinement is scoped to session_a only, not a global side effect. | ||
| assert index.execute( | ||
| "SELECT accepted_raw_id FROM raw_revision_heads WHERE session_id = ?", (session_b,) | ||
| ).fetchone() == (raw_id,) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
No coverage for the new ineligible batch-race no-op path.
session_b is never executable in this fixture, so _apply_strategy's new quarantine_locked.status == "ineligible" early return (polylogue/storage/raw_reconciler.py Lines 1307-1329) is never exercised — the very branch this PR adds for sibling races. A test that applies key_a's plan and then forces an apply against a stale-but-selected sibling plan would pin that contract.
I can draft that test if useful.
🤖 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 `@tests/unit/storage/test_quarantined_accepted_raw_repair.py` around lines 444
- 487, Add coverage for the quarantine_locked.status == "ineligible" early
return in _apply_strategy by first applying session_a’s valid plan, then
attempting to apply a stale-but-selected plan for session_b. Assert the sibling
attempt is a no-op with the expected non-success/retry behavior, while
preserving session_b’s accepted-head row and avoiding unintended changes.
Sinity
added a commit
that referenced
this pull request
Jul 28, 2026
…eceipt (#3372) ## Summary Fix the next layer found while completing polylogue-zaiz's investigation: after PR #3371's fan-out scoping fix, the 3 real stuck sessions still failed inspection with "competing raw-revision application authority exists". ## Problem raw_revision_applications is an append-only decision history, not a single-current-state table. Any session with more than one historical revision-authority decision accumulates multiple rows for the same logical_source_key. The lookup matched raw_id = ? OR accepted_raw_id = ?, additionally pulling in rows from OTHER raw_ids' own receipts that cite this raw as their superseded predecessor. ## Solution Scope the lookup to raw_id = ? AND logical_source_key = ? alone. ## Definitive finding Applying this fix against the live archive (read-only) advances all 3 stuck sessions to their genuine final classification: accepted_frontier_kind='semantic', not 'byte'. These sessions were originally accepted under semantic equivalence, not a byte-proof -- refine_quarantined_raw requires byte-frontier authority by design and cannot resolve them. This is not a bug; it's a real architectural boundary. Recorded on polylogue-zaiz as the definitive answer. ## Verification - devtools test (3 files) -> 53 passed - mypy clean - devtools verify --quick -> exit 0 - Confirmed against the live archive (read-only) Ref polylogue-zaiz, polylogue-ihc8 Co-Authored-By: Claude <noreply@anthropic.com>
Sinity
added a commit
that referenced
this pull request
Jul 28, 2026
…ile sg80 polylogue-zaiz's own scope (fan-out scoping bugs) is fixed and confirmed live via PRs #3371 and #3372. The 3 remaining stuck sessions have a definitive, non-bug explanation: accepted_frontier_kind='semantic', which refine_quarantined_raw cannot resolve by design. Filed polylogue-sg80 for the genuinely separate semantic-frontier-aware actuator design question. Co-Authored-By: Claude <noreply@anthropic.com>
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
Fix the architectural gap behind
polylogue-zaiz(the residual "3 fan-out sessions stuck" finding from theihc8/ewfpincident chain):_inspect_quarantined_accepted_rawhad the same unscoped-lookup bug class_inspect_duplicate_raw_identityhad beforeihc8's fix.Problem
Forked/subagent/resumed sessions can physically replay the identical parent evidence, so the same raw_id can legitimately be the accepted head of several logical source keys/sessions at once.
_inspect_quarantined_accepted_rawlooked up the accepted-head row byaccepted_raw_idalone, requiring exactly one match system-wide — for a fan-out raw, this unconditionally raised "expected one accepted head, found N" for every sibling, permanently blocking any of them from ever being refined.Investigation also surfaced a much larger, separate, pre-existing fact: 15,798 of ~41,334
raw_sessionsrows (38% of the archive) carryrevision_kind='unknown'— a known backlog predating this session and unrelated to this scoping bug (confirmed the untyped/quarantined envelope path already handles it correctly; the sole blocker for genuine fan-out cases is the head lookup's scoping).Solution
_inspect_quarantined_accepted_rawrequires alogical_source_keyparameter, scoping its lookups (mirroring_inspect_duplicate_raw_identity's fix).inspect_quarantined_accepted_rawstakes(raw_id, logical_source_key)pairs instead of bare raw_ids._strategy_overridesbuilds per-session override keys for the quarantine path specifically, falling back to bare-raw_id keys unchanged for browser-origin/conflict overrides (not per-session shaped)._apply_strategy's REFINE_QUARANTINE branch threads the same key through and gains the identical graceful "ineligible" no-op handling already shipped for FOLD_DUPLICATE_ALIAS (ewfp, fix(storage): resolve duplicate-alias batch-race as a permanent no-op #3369) — a fan-out raw's refinement mutates the shared row itself, so only one sibling can ever win.Verification
devtools test tests/unit/storage/test_quarantined_accepted_raw_repair.py tests/unit/storage/test_duplicate_raw_identity_repair.py tests/unit/storage/test_raw_authority_ledger.py→ 52 passed (new regression tests reproduce the exact fan-out shape; anti-vacuity confirmed by reverting)devtools test tests/unit/storage/ -k "raw_reconciler or raw_authority or duplicate or quarantine or repair"→ 228 passed, 1 pre-existing unrelated failure (confirmed via git stash on master)mypy polylogue/storage/raw_reconciler.py polylogue/storage/repair.py→ cleandevtools verify --quick→ exit 0Ref polylogue-zaiz, polylogue-ihc8
Co-Authored-By: Claude noreply@anthropic.com
Summary by CodeRabbit
Bug Fixes
Tests