fix(storage): let a raw rewrite its own accepted revision head - #3527
Conversation
Problem
polylogue-buq8/i415/lkos each independently flagged codex-session rows
with message_count=0 despite 996 KB-3.3 MB of real event_msg/
response_item content in the raw bytes. Direct reproduction against the
live archive's flagship sample (native_id
0199fada-d8bd-7fc0-997b-d23d3a6849c7) falsifies both prior hypotheses:
running the current parse_stream_payload directly against the exact
live raw bytes extracts 1,496 real messages correctly, so the content
loss is not a codex parser defect in polylogue/sources/ (i415's "old
rollout envelope" theory) and is not gated by raw_sessions.
revision_authority='quarantined' either -- 1,742 of 1,757 quarantined
codex-session rows in the live archive already have non-zero message
counts, so that column is orthogonal to whether materialization ran
(falsifying buq8/lkos's "materialization never runs" framing).
The actual mechanism, confirmed via source.db/index.db inspection: each
affected session's raw_revision_heads.accepted_raw_id equals its own
sessions.raw_id, with decided_at_ms (the governance decision) months
after sessions.updated_at_ms (the original, defective write). A
bookkeeping-only backfill later recorded the raw as authoritative
without re-running message extraction against it.
revision_authority_refuses_write's "governed" check then refused every
subsequent write for that session_id unconditionally -- including the
very raw the backfill had just declared authoritative -- because it
only checked whether any raw_revision_heads row existed for the
session_id, never which raw it named. Whatever originally produced the
zero-message content (a long-since-fixed parser bug, an interrupted
write, or another historical defect this checkout can no longer
observe) was permanently frozen: no future re-ingest tick could ever
pass the gate to correct it.
Solution
storage/sqlite/archive_tiers/ingest_precedence.py:
revision_authority_refuses_write now compares the incoming raw_id
against raw_revision_heads.accepted_raw_id and refuses only when they
differ (a losing/competing raw trying to overwrite the winner). A
write for the accepted raw itself -- the winner re-asserting its own
content -- is no longer refused, closing the corrective-rewrite gap
without reopening it to arbitrary last-writer-wins (a different raw_id
is still refused, pinned by a new regression test).
tests/unit/pipeline/test_ingest_batch.py adds two regression tests:
test_write_session_allows_rewrite_of_its_own_accepted_revision_head
reproduces the exact defect shape (governed session, zero existing
messages, same raw_id, real incoming content) and asserts the write
now succeeds; test_write_session_still_refuses_a_different_raw_than_
the_accepted_head pins that the existing invariant (a different raw
for a governed session_id stays refused) is untouched.
Verification
devtools test tests/unit/pipeline/test_ingest_batch.py
tests/unit/storage/test_revision_replay.py
tests/unit/storage/test_revision_application.py
tests/unit/storage/test_raw_revision_authority.py
tests/unit/sources/test_live_batch_support.py
tests/unit/sources/test_revision_backfill.py
tests/unit/storage/test_repair.py tests/unit/storage/test_raw_retention.py
tests/unit/storage/test_raw_authority_ledger.py
tests/unit/storage/test_quarantined_accepted_raw_repair.py
tests/unit/storage/test_duplicate_raw_identity_repair.py
tests/unit/storage/test_incremental_rebuild_equivalence.py
tests/unit/daemon/test_raw_materialization_parse_stage_equivalence.py
tests/unit/storage/test_browser_capture_origin_repair.py
tests/unit/cli/test_status.py tests/unit/daemon/test_daemon_status.py
tests/unit/maintenance/test_raw_authority_reset.py
tests/integration/test_append_cohort_memory.py
tests/unit/sources/test_dispatch_payloads.py
tests/unit/sources/test_dispatch_ordering.py
tests/unit/sources/test_source_laws.py
-> 2 new tests pass; every other failure (24 in the raw-authority/
repair/append-cohort cluster, 1 in test_dispatch_payloads.py's
unrelated repeated-session_meta message-count assertion) is
reproduced identically on the unmodified baseline (verified via
git stash), so this change adds zero regressions.
New regression test verified to fail without the fix (reverted the
production diff, reran, confirmed `assert False is True`, then
reapplied).
devtools verify --quick -> exit 0 (ruff format/check, mypy --strict,
render all --check, topology/layering/closure-matrix/schema-policy
lab checks all pass)
Residual / follow-up
This fix unblocks *future* re-ingest of the affected sessions but does
not retroactively repair already-materialized rows: the live archive's
zero-message sessions were written before this fix existed, so they
still need a session-scoped reparse (a normal daemon reprocess tick
touching those raw_ids, or `polylogue ops reset --index &&
polylogued run` for a full rebuild) to actually pick up the corrected
behavior. This is not a schema change (no index.db structural delta,
so no lifecycle.py SEMANTIC_REPARSE declaration applies) -- it is an
application-logic fix in the write-precedence layer, and the repair
step is an ordinary reprocess of the specific affected raw_ids, not an
index generation bump.
polylogue-buq8 and polylogue-lkos's framing ("materialization never
runs" / "message-extraction defect distinct from the gate") does not
match what the live data shows; i415's "old rollout envelope" parser
theory is also not supported -- direct reproduction shows the current
parser handles the flagship sample correctly. All three describe the
same underlying write-gate defect fixed here, not three independent
problems, and not the systemic raw-authority absorbing-state issue
tracked separately (and still open) in polylogue-u19l.
Ref polylogue-buq8, polylogue-i415, polylogue-lkos
Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 31 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: ASSERTIVE 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 |
Left implementation-trail comments on all three beads: the root cause is one shared write-precedence gate defect (fixed in this branch), not three independent problems, and none of the beads' original root-cause framing (quarantine blocking materialization / old-format parser gap / distinct extraction defect) matches what direct reproduction against the live raw bytes shows. Co-Authored-By: Claude <noreply@anthropic.com>
Comments left; fix is in PR #3527 (unmerged) and the live archive still needs an ordinary reparse of the affected raw_ids after merge -- leaving these open rather than closing prematurely. Co-Authored-By: Claude <noreply@anthropic.com>
![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: 0b66d48498
ℹ️ 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".
| if governed_row is not None and str(governed_row[0]) != raw_id: | ||
| return True |
There was a problem hiding this comment.
When the accepted raw is reparsed to a different content hash—the parser-fix scenario this carve-out targets—the two callers proceed through their ordinary session-write paths, which update sessions.content_hash but never reissue record_revision_application_sync. Consequently, raw_revision_heads.accepted_content_hash and its application receipts retain the old hash; validate_raw_replay_application_receipt explicitly rejects this mismatch, leaving corrected sessions inconsistent with the raw-authority ledger. Route this case through the authoritative replay/receipt path or update the head and application receipts atomically with the session.
Useful? React with 👍 / 👎.
| governed_row = conn.execute( | ||
| "SELECT accepted_raw_id FROM raw_revision_heads WHERE session_id = ? LIMIT 1", | ||
| (session_id,), | ||
| ).fetchone() | ||
| if governed is not None: | ||
| if governed_row is not None and str(governed_row[0]) != raw_id: |
There was a problem hiding this comment.
If historical drift or an interrupted repair leaves multiple raw_revision_heads rows for the same session_id—a supported shape handled as parallel_session_heads in storage/repair.py—this LIMIT 1 examines an arbitrary head. An incoming raw matching the selected row is therefore allowed to overwrite the session even when another head accepts a different raw, while the same write may be refused under a different row order. Check for the existence of any head whose accepted_raw_id differs from the incoming raw instead of selecting one row.
Useful? React with 👍 / 👎.
Problem: revision_authority_refuses_write's governed-head check used `SELECT accepted_raw_id FROM raw_revision_heads WHERE session_id = ? LIMIT 1`. Historical drift or an interrupted repair can leave more than one raw_revision_heads row for a session (storage/repair.py's parallel_session_heads shape), so an incoming raw matching whichever row LIMIT 1 happened to select was allowed through even when a different parallel head accepted another raw -- and the same write could be refused or allowed depending on row insertion/scan order alone. Flagged by the CodeRabbit-equivalent bot review on PR #3527 as a P2 finding. Solution: check for the existence of any head whose accepted_raw_id differs from the incoming raw, instead of comparing against one arbitrarily selected row. Order-independent by construction. Verification: new regression test builds two parallel heads (one accepting the incoming raw, one accepting a different raw) and asserts the write is still refused; confirmed to fail on the unmodified code (git-stash-verified) and pass with the fix. devtools test tests/unit/pipeline/test_ingest_batch.py tests/unit/storage/test_raw_revision_authority.py tests/unit/storage/test_revision_application.py tests/unit/archive/test_session_revision_membership.py (134 passed); devtools verify --quick (exit 0). The review's other (P1) finding -- accepted-raw rewrites don't reissue raw_revision_applications receipts, so a later formal replay-plan validation could see a stale accepted_content_hash -- is a real gap in the authoritative-replay ledger's write path, not this narrow comparison bug. It touches both ordinary write call sites (revision_governance.py, ingest_batch/_core.py) and the receipt synthesis contract in revision_application.py; fixing it correctly needs its own scoped design rather than a same-lane addendum. Tracked as a follow-up (see PR comment). Co-Authored-By: Claude <noreply@anthropic.com>
|
P2 (arbitrary-row selection under P1 (stale |
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
1 participant
Summary
Fixes a genuine, confirmed write-precedence defect that silently freezes zero-message
codex-sessionrows in the archive, closing polylogue-buq8/i415/lkos. All three beads described the same live-archive symptom (real, substantial raw content butmessage_count=0) with three different, mutually-inconsistent root-cause theories; direct reproduction against the live raw bytes falsifies all three theories and locates the actual defect one layer downstream, inrevision_authority_refuses_write.Problem
buq8 theorized
revision_authority='quarantined'blocks materialization entirely. i415 theorized an old-format rollout envelope the codex parser can't handle. lkos theorized a distinct "message-extraction defect" residual from PR #3497. None of these hold up:codex-sessionrows withrevision_authority='quarantined', only 15 havemessage_count=0— 1,742 have real content. Quarantine status is orthogonal to whether materialization happened.parse_stream_payloadagainst the exact live raw bytes of buq8/i415's flagship sample (native_id=0199fada-d8bd-7fc0-997b-d23d3a6849c7, 3.3 MB) extracts 1,496 real messages. The current codex parser is not defective for this shape.raw_revision_heads.accepted_raw_idequals its ownsessions.raw_id.raw_revision_heads.decided_at_msis months aftersessions.updated_at_ms— a bookkeeping-only backfill retroactively recorded this raw as authoritative long after the original (defective, for reasons this checkout can no longer observe) write happened, without re-running message extraction.The actual bug:
revision_authority_refuses_write's "governed" check refused every future write for a session_id once anyraw_revision_headsrow existed for it, without ever comparing which raw the row names as accepted. This meant the very raw a backfill had just declared authoritative could never pass its own gate to correct itself — a session whose content was defectively written before governance existed stays permanently frozen, because no future re-ingest tick can ever get past this check.Solution
polylogue/storage/sqlite/archive_tiers/ingest_precedence.py:revision_authority_refuses_writenow compares the incomingraw_idagainstraw_revision_heads.accepted_raw_idand refuses only when they differ (a losing/competing raw trying to overwrite the winner). A write for the accepted raw itself — the winner re-asserting its own content — is no longer refused. This is the single shared gate used by both write paths (ingest_batch/_core.py's_write_sessionandrevision_governance.py's_write_parsed_precedence_result), so the fix applies to both without duplication.tests/unit/pipeline/test_ingest_batch.pyadds two regression tests:test_write_session_allows_rewrite_of_its_own_accepted_revision_headreproduces the exact defect shape (governed session, zero existing messages, same raw_id, real incoming content) and asserts the write now succeeds. Verified to fail (assert False is True) against the unmodified production code, then pass after the fix.test_write_session_still_refuses_a_different_raw_than_the_accepted_headpins that the existing invariant (a genuinely different, losing raw for a governed session_id) stays refused — the fix doesn't reopen last-writer-wins.Verification
2 new tests pass. Every other failure observed (24 across the raw-authority-ledger/repair/append-cohort-memory cluster, plus 1 pre-existing count-drift in
test_dispatch_payloads.py's repeated-session_metafixture) reproduces identically against the unmodified baseline (git stash+ rerun), confirmed test-by-test — this change adds zero regressions.-> exit 0 (ruff format/check, mypy --strict, render all --check, topology/layering/closure-matrix/schema-policy lab checks all pass).
Residual / follow-ups
This fix unblocks future re-ingest of affected sessions but does not retroactively repair already-materialized rows — the live archive's existing zero-message sessions were written before this fix existed and need an ordinary session-scoped reparse (a normal daemon reprocess tick touching those raw_ids, or
polylogue ops reset --index && polylogued runfor a full rebuild) to pick up corrected content. This is not a schema/index-structure change, so nolifecycle.pySEMANTIC_REPARSEdeclaration applies — it's an application-logic fix in the write-precedence layer, and the repair is an ordinary reprocess, not an index generation bump.buq8's "materialization never runs" and lkos's "message-extraction defect distinct from the gate" framings do not match what the live data shows; i415's "old rollout envelope" parser theory is also not supported. All three describe the same underlying write-gate defect fixed here, not three independent problems — and this is explicitly not the systemic raw-authority absorbing-state issue tracked separately (and still open, unrelated scope) in polylogue-u19l.
Ref polylogue-buq8, polylogue-i415, polylogue-lkos
🤖 Generated with Claude Code