fix(storage): collapse byte-equal duplicates before revision-chain proof - #3574
Merged
Merged
Conversation
Problem: classify_historical_full_revision_streams (and its eager sibling) treated any two full-revision captures that tied on size as an unprovable fork, quarantining the *entire* cohort even when the tie was two byte-identical copies of the same capture -- the ordinary result of re-acquiring an unchanged file. A live audit (polylogue-lb39z) measured 13,671 quarantined raw_sessions rows / 50.2GB (79% of quarantined bytes) caused by exactly this: duplicate captures misread as ambiguity, with the verdict written cohort-wide instead of localized to any real divergence. A stuck baseline from this bug also let the live watcher pump unbounded quarantined rows for one growing file (one Codex rollout produced 799 quarantined snapshots / 6.28GB of overlapping tails). What changed: both classifiers now (1) collapse byte-identical revisions onto one representative before any chain-order proof runs (I4) -- content hashed directly for in-memory payloads, streamed via one read pass for the streaming variant, so no extra I/O beyond what already ran; every non-representative duplicate mirrors its representative's verdict with a new `relation="duplicate"`; and (2) localize any residual divergence to only the fork tip instead of the whole cohort (I5): a shared ancestor with two mutually-incomparable children now classifies the ancestor BYTE_PROVEN and only the fork's children QUARANTINED, rather than quarantining all three. Two disconnected/incomparable roots (no shared ancestor at all) still quarantine everything -- there is no anchor to localize from, matching prior behavior for that genuinely irreducible case. classify_untyped_full_revision_groups' "whole cohort must be one provable chain" contract is preserved by checking every decision instead of only the first. Alternatives rejected: promoting the fork's larger/newer-looking child as the accepted head was considered and rejected -- it would make an arbitrary sort-order or size tie-break silently pick a winner among genuinely competing evidence, which is exactly the kind of silent-wrong-guess this subsystem has zero tolerance for (see polylogue-yla8's write-gate history). Leaving the fork tip quarantined and letting the (separate, still-unwired) judgment-assertion flow adjudicate it is the safe default. Verification: devtools test tests/unit/storage/test_raw_revision_authority.py (24 passed, including new anti-vacuity tests proving the pre-fix classifier quarantined a byte-equal duplicate pair and a shared-root fork wholesale, and the post-fix classifier does not); devtools test -k "raw_revision or revision_governance or raw_authority" (115 passed, 6 failed -- all 6 reproduce identically with this change reverted via `git stash`, confirmed pre-existing/unrelated: 5 are devtools/raw_authority_scale_proof.py synthetic-corpus repair-pass assertions and 1 is an integration daemon-health probe timeout); mypy --strict on both touched modules (no issues); devtools lab policy schema-versioning (intact); ruff check + format (clean). Ref polylogue-lb39z (Phase 1, item 1 of 5). Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 39 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 (2)
📝 WalkthroughWalkthroughHistorical revision classification now supports duplicate relations, deduplicates identical captures, and localizes quarantine to divergent or ambiguous descendants. Governance requires every cohort decision to be byte-proven. ChangesRevision authority classification
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RevisionInputs
participant HistoricalRevisionClassifier
participant StreamOpener
participant RevisionGovernance
RevisionInputs->>HistoricalRevisionClassifier: provide revision payloads or streams
HistoricalRevisionClassifier->>StreamOpener: calculate size and SHA-256
StreamOpener-->>HistoricalRevisionClassifier: return stream metadata
HistoricalRevisionClassifier->>HistoricalRevisionClassifier: deduplicate and prove prefix chains
HistoricalRevisionClassifier-->>RevisionGovernance: return revision decisions
RevisionGovernance->>RevisionGovernance: require BYTE_PROVEN authority for every decision
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: 3
🤖 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/archive/revision_authority.py`:
- Around line 264-269: Update _expand_duplicate_decisions so each duplicate
HistoricalRevisionDecision preserves the representative decision’s
predecessor_raw_id instead of setting it to None, allowing duplicate full
revisions to retain their chain generation through revision_governance.
- Around line 154-170: In the parent-selection loop for the revision
classification logic, replace the nested `maximal` filter with selection of the
single largest candidate by size, avoiding any additional is_prefix calls.
Preserve the existing parents and children updates, and remove the unreachable
ambiguous-parent handling if it depends on multiple parents for non-root nodes.
In `@tests/unit/storage/test_raw_revision_authority.py`:
- Around line 91-98: Replace the vacuous lower-bound assertion on the opened
stream counts with an upper-bound assertion matching the expected three-member
chain behavior: 3 hashing opens, 6 pair-comparison opens, and 2 maximal-filter
opens, for a total of at most 11. Keep the existing assertion that all three
stream IDs were opened.
🪄 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: 1f654880-3432-44ba-b011-abfaf590d429
📒 Files selected for processing (3)
polylogue/archive/revision_authority.pypolylogue/storage/sqlite/archive_tiers/revision_governance.pytests/unit/storage/test_raw_revision_authority.py
Comment on lines
+264
to
+269
| decision_by_raw_id[member_id] = HistoricalRevisionDecision( | ||
| raw_id=member_id, | ||
| authority=rep_decision.authority, | ||
| relation="duplicate", | ||
| predecessor_raw_id=None, | ||
| ) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find every consumer that reads predecessor_raw_id from a HistoricalRevisionDecision.
set -euo pipefail
rg -n -C6 '\bpredecessor_raw_id\b' --glob '*.py'Repository: Sinity/polylogue
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Tracked Python files containing HistoricalRevisionDecision or predecessor_raw_id:\n'
git ls-files '*.py' | xargs rg -n -C6 '\b(HistoricalRevisionDecision|predecessor_raw_id)\b' || true
printf '\nTarget file outline around relevant methods:\n'
wc -l polyilogue/archive/revision_authority.py 2>/dev/null || true
sed -n '1,340p' polylogue/archive/revision_authority.pyRepository: Sinity/polylogue
Length of output: 50373
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'Define HistoricalRevisionDecision models/usages in source storage files only:\n'
git ls-files 'polylogue/**/*.py' | xargs rg -n -C4 '\bclass HistoricalRevisionDecision\b|\bpredecessor_raw_id\b' || true
printf '\nRelevant classifier tests around duplicate behavior:\n'
sed -n '138,250p' tests/unit/storage/test_raw_revision_authority.py
printf '\nRelevant source around classifier expansion if present:\n'
sed -n '220,285p' polylogue/archive/revision_authority.py 2>/dev/null || trueRepository: Sinity/polylogue
Length of output: 50372
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf 'class and method definitions in revision_authority.py:\n'
ast-grep outline polylogue/archive/revision_authority.py --view expanded 2>/dev/null || true
printf '\nHistoricalRevisionDecision definition and duplicate expansion:\n'
rg -n -C20 'class HistoricalRevisionDecision|HistoricalRevisionDecision|_expand_duplicate_decisions|classify_historical_full_revision_streams' polylogue/archive/revision_authority.py
printf '\nGovernance chain builder section:\n'
sed -n '880,935p' polylogue/storage/sqlite/archive_tiers/revision_governance.pyRepository: Sinity/polylogue
Length of output: 16485
Preserve the chain parent on duplicate full revisions.
HistoricalRevisionDecision carries predecessor_raw_id, and revision_governance uses it to compute acquisition_generation with generation_by_raw_id. When a BYTE_PROVEN mid-chain representative has relation="predecessor", _expand_duplicate_decisions creates duplicates with relation="duplicate" and predecessor_raw_id=None, so its generation drops to baseline/zero instead of continuing the chain.
Mirror the representative’s predecessor_raw_id for duplicates, or state in the docstring that duplicate proven ancestors are treated as orphans.
🤖 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/archive/revision_authority.py` around lines 264 - 269, Update
_expand_duplicate_decisions so each duplicate HistoricalRevisionDecision
preserves the representative decision’s predecessor_raw_id instead of setting it
to None, allowing duplicate full revisions to retain their chain generation
through revision_governance.
Comment on lines
+91
to
+98
| # polylogue-lb39z (I4/I5): the classifier now hashes every stream once | ||
| # (for byte-equal dedup) and compares every smaller/larger pair (not just | ||
| # size-adjacent ones) so it can localize a fork instead of nuking the | ||
| # whole cohort -- more opens than the old adjacent-only walk, but still | ||
| # bounded and never re-reading a stream's full payload more than once per | ||
| # comparison it actually participates in. | ||
| assert opened.count("oldest") + opened.count("middle") + opened.count("newest") >= 3 | ||
| assert set(opened) == {"oldest", "middle", "newest"} |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
The open-count assertion is vacuous; assert an upper bound instead.
Line 98 asserts that set(opened) contains all three ids. That already implies each id appears at least once, so the >= 3 check at Line 97 adds no constraint. The comment states the read count is "bounded", but no assertion enforces a bound.
An upper bound matters here. _classify_deduped_nodes calls is_prefix once per ordered pair and then again inside the maximal filter, so the streamed open count grows faster than linearly with cohort size. An upper-bound assertion pins the read amplification and fails if it regresses.
For this 3-member linear chain: 3 opens for hashing, plus 3 smaller/larger pairs, each opening 2 streams, plus the maximal re-comparison for the largest node.
♻️ Proposed replacement asserting a real bound
assert set(opened) == {"oldest", "middle", "newest"}
- assert opened.count("oldest") + opened.count("middle") + opened.count("newest") >= 3
+ # Pin the read amplification: 3 hashing opens plus a bounded number of
+ # pairwise prefix comparisons. Tighten this number if the comparison
+ # strategy changes; do not relax it silently.
+ assert len(opened) <= 12🤖 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_raw_revision_authority.py` around lines 91 - 98,
Replace the vacuous lower-bound assertion on the opened stream counts with an
upper-bound assertion matching the expected three-member chain behavior: 3
hashing opens, 6 pair-comparison opens, and 2 maximal-filter opens, for a total
of at most 11. Keep the existing assertion that all three stream IDs were
opened.
…parent filter Address CodeRabbit finding on PR #3574: the "maximal" candidate filter recomputed is_prefix for every candidate pair, re-opening and re-reading parent blobs through _stream_is_prefix. Byte-prefix is transitive over a totally-ordered-by-size candidate set (if parent1 and parent2 are both prefixes of the same child and sizes[parent1] < sizes[parent2], parent1 is necessarily a prefix of parent2), so the unique maximal candidate is simply the largest one -- no further is_prefix comparisons needed. This turns what was O(n^3) streamed reads on a linear revision chain back into the O(n^2) the classifier's docstring already claims. Also strengthens a vacuous test assertion (opened.count(...) >= 3, trivially implied by set membership already checked on the next line) into a real upper-bound pin on read amplification (9 opens for the 3-member fixture, verified empirically against the fixed classifier). Deliberately NOT applying CodeRabbit's second suggestion (mirror a duplicate's predecessor_raw_id onto the duplicate decision) -- verified it would introduce a dict-key collision in revision_governance.py's generation walk (children = {predecessor_raw_id: raw_id, ...} keyed by predecessor; giving a duplicate the same predecessor_raw_id as its representative makes both compete for the same dict key, risking silently dropping the real downstream chain from generation numbering). That finding is real but needs a properly tested fix, not a one-line change under merge-train time pressure -- left as a follow-up. Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
Applied CodeRabbit's finding 1 (quadratic streamed-blob-read amplification in the Finding 2 (mirror a duplicate's |
Sinity
added a commit
that referenced
this pull request
Aug 2, 2026
Sinity
added a commit
that referenced
this pull request
Aug 2, 2026
Sinity
added a commit
that referenced
this pull request
Aug 2, 2026
…ntative Problem: _expand_duplicate_decisions (archive/revision_authority.py) gives every duplicate member predecessor_raw_id=None, so classify_raw_revision_cohort's generation walk (a dict comprehension keyed by predecessor_raw_id) never sees duplicates and they fall back to acquisition_generation=0 regardless of true chain position. Flagged by CodeRabbit review on PR #3574 (polylogue-5unky); the obvious fix (mirror the representative's predecessor_raw_id onto the duplicate) was rejected because it makes the duplicate and its representative compete for the same predecessor-keyed dict slot, risking silently dropping the real chain-continuing representative from the walk. What changed: added HistoricalRevisionDecision.duplicate_of_raw_id, populated only for relation="duplicate" entries, deliberately kept separate from predecessor_raw_id so a duplicate never enters the predecessor-keyed walk. classify_raw_revision_cohort now runs a separate post-pass after the real chain walk that copies each duplicate's generation directly from its already-computed representative. Verification: devtools test tests/unit/storage/test_revision_replay.py -k duplicate (2 passed, both proven to fail against the pre-fix code); devtools test -k raw_authority (90 passed, 6 pre-existing/unrelated scale-proof timeouts confirmed by rerunning that file with this change stashed out, same failure set); devtools verify --quick (exit_code 0, 19/19 steps green). Ref polylogue-5unky Co-Authored-By: Claude <noreply@anthropic.com>
Sinity
added a commit
that referenced
this pull request
Aug 2, 2026
…ntative (#3580) ## Summary Duplicate revision decisions (byte-identical to another raw in the same cohort) now record their true `acquisition_generation` (copied from their representative) instead of always falling back to 0. ## Problem `_expand_duplicate_decisions` (`polylogue/archive/revision_authority.py`) gives every duplicate member `predecessor_raw_id=None`. `revision_governance .py`'s generation walk in `classify_raw_revision_cohort` builds a predecessor-keyed dict (`{decision.predecessor_raw_id: decision.raw_id for decision in decisions if decision.predecessor_raw_id is not None}`) and walks it from the baseline. Because duplicates never carry a `predecessor_raw_id`, they never appear in that dict and never get visited by the walk, so they fall back to `acquisition_generation=0` regardless of their representative's true chain position. Flagged by CodeRabbit review on PR #3574 (polylogue-5unky). The obvious fix — mirror the representative's `predecessor_raw_id` onto the duplicate — was considered and rejected: it makes the duplicate and its representative compete for the same key in the plain-dict comprehension above. Whichever entry the comprehension writes last silently wins that slot, so the duplicate can overwrite the real chain-continuing representative and break generation numbering for everything downstream of it. That would be a worse regression than the bug being fixed. Severity is low: `acquisition_generation` on a duplicate row is a display/ordering field, not something authority/replay-plan logic depends on for duplicates. Still worth fixing correctly since this is a safety-adjacent subsystem. ## Solution Implemented approach (b) from the two options the tracking bead named: a separate post-pass that copies the representative's already-computed generation, rather than folding duplicates into the predecessor-keyed walk at all. - Added `HistoricalRevisionDecision.duplicate_of_raw_id: str | None` (`polylogue/archive/revision_authority.py`), populated only for `relation="duplicate"` entries in `_expand_duplicate_decisions`, pointing at the representative raw_id. Deliberately kept separate from `predecessor_raw_id` -- a duplicate is not a chain-continuing child of anything, it's a second copy of its representative's own bytes, so it must never compete for a `children` dict slot. - `classify_raw_revision_cohort` (`polylogue/storage/sqlite/archive_tiers/revision_governance.py`) now runs a post-pass after the real chain walk: for every `relation="duplicate"` decision, `generation_by_raw_id[dup.raw_id] = generation_by_raw_id.get(dup.duplicate_of_raw_id, 0)`. Duplicates never participate in the predecessor-keyed walk. Approach (a) (children-as-list, picking the chain-continuing child specifically) was considered but doesn't fit the semantics as cleanly: a duplicate isn't an additional *child* of the representative's predecessor, it's the *same* generation as the representative itself. ## Verification - `devtools test tests/unit/storage/test_revision_replay.py -k duplicate` -- 2 passed. Both new tests were confirmed to fail against the pre-fix code (`assert 0 == 1`) by stashing the fix and rerunning, proving they exercise the real bug, then re-verified passing with the fix restored. - `test_duplicate_decision_mid_chain_gets_representative_generation_not_zero`: builds a 3-link byte chain (base -> mid -> head) plus a byte-identical duplicate of the *middle* link, proves the duplicate gets generation 1 (mid's real position), not the 0 fallback. - `test_duplicate_generation_copy_does_not_drop_the_chain_continuing_representative`: constructs the exact collision shape the rejected fix would have hit -- a duplicate of `mid` that is same-size and lexicographically *later* than `mid` (the ordering the rejected fix's dict comprehension would need to let the duplicate clobber `mid`'s real predecessor-keyed slot) -- and proves `head`, two links downstream of `mid`, still gets its correct real generation (2), i.e. the true chain-continuing representative was never displaced from the walk. - `devtools test -k raw_authority` -- 90 passed, 6 failed. All 6 failures are in `test_raw_authority_scale_proof.py` / `test_raw_authority_daemon_health_proof.py` (timeout-driven scale/daemon tests). Confirmed pre-existing and unrelated: reran `test_raw_authority_scale_proof.py` alone with this change stashed out and got the identical failure set. - `devtools verify --quick` -- `exit_code: 0`, all 19 steps green (ruff format/check, mypy, render all, topology, layering, closure-matrix, schema roundtrip, manifests, ci-workflows, doc-commands, docs-coverage, test-infra-currency, pytest-timeout-overrides, degrade-loudly, hash-boundary-census, schema-versioning policy, classifier-fingerprints policy, schema promotion audit). Also ran again automatically via the pre-push hook -- green. Ref polylogue-5unky
Sinity
added a commit
that referenced
this pull request
Aug 2, 2026
…y lint (#3588) ## Summary Lands items 3 and 4 of polylogue-lb39z's five-item raw-authority Phase 1 program (items 1-2 already merged via PR #3574 and #3577). Both items are low-risk (read-only classifier + dry-run actuator; static AST lint) and share the same investigation session, so they land in one PR with two focused commits rather than two separate PRs. ## Problem **Item 3**: 2,712 `raw_sessions` rows are `revision_kind='append'`, `revision_authority='quarantined'`, and have zero `raw_session_memberships` rows at all -- a genuine fixed point. The only mechanism that ever promotes an append raw (`_promote_contiguous_append_evidence`) requires its byte-contiguous predecessor to already be `byte_proven`; when the predecessor is itself stuck quarantined, no amount of re-running that cascade reaches the child. **Item 4**: polylogue-w32w found `RawAuthorityFrontierState.UNRESOLVED_PROVENANCE` paired with the dispatched `RawAuthorityActuator.REFINE_QUARANTINE` -- an actuator no path (daemon or operator) could ever select. 4,174 blockers accumulated behind this for weeks undetected. PR #3466 added a runtime constructor guard (`RawAuthorityFrontierItem.__post_init__`) for the exact shape, but a constructor guard only fires when something actually constructs the bad pairing -- w32w's own close note explicitly left this bead's broader ask (a static lab-policy check) open. ## Solution **Item 3** (`polylogue/storage/raw_append_chain_backfill.py` + `polylogue/maintenance/raw_append_chain_backfill_apply.py`): a new read-only classifier proves each membershipless append row's own claimed `[append_start_offset:append_end_offset)` byte range directly against its live source file's current bytes, reusing the exact byte-window comparison polylogue-u19l's `live_source_reconciliation` module already validated -- a proof independent of any ancestor's authority. The actuator follows the identical dry-run-default / verified-backup-required-to-apply / immutable-receipt pattern as the merged u19l and lb39z-item-2 actuators, promoting exact matches to `byte_proven` (reusing the existing `live_source_verification_v1` evidence value -- the mechanism is identical, only the target population differs). It deliberately never touches `predecessor_raw_id`/`baseline_raw_id`/`acquisition_generation`; once a row is proven, the existing cascade picks it up for free on the next convergence pass. Adds migration 020 (source schema v19->v20, new `raw_append_chain_backfill_receipts` table) and `devtools workspace raw-append-chain-backfill-apply`. **Item 4** (`devtools/verify_raw_authority_frontier_executability.py`): statically parses `polylogue/storage/raw_reconciler.py` and enumerates every literal `(state, actuator)` pair constructible via `_item(...)` and `_StrategyOverride(...)` call sites (17 pairs on current source; 1 dynamic forwarding site correctly reported informational-only, since its literal source is separately covered). Each pair is re-checked against the real `_EXECUTABLE_STATES`/`_APPLY_DISPATCHED_ACTUATORS` imported directly from `raw_reconciler.py`, so this lint can never drift out of sync with the actual gate. Wired as `devtools lab policy raw-authority-frontier-executability`. ## Not in scope / explicitly deferred Item 5 of polylogue-lb39z (re-classify the ambiguous cohort + wire `_maximal_evidence_fallback`) is intentionally NOT in this PR -- flagged in the bead as the highest-risk item, directly touching the never-retire-an-accepted-head invariant. See the bead notes for a detailed account of what's needed and why it needs its own dedicated, unhurried session. No live archive mutation was performed or attempted. Both actuators are proven only against synthetic fixtures; live application is a separate, later, operator-supervised step. ## Verification - `devtools test tests/unit/storage/test_raw_append_chain_backfill.py tests/unit/maintenance/test_raw_append_chain_backfill_apply.py` -- 5 passed - `devtools test tests/unit/storage/test_durable_migrations.py` -- 40 passed (hardcoded v19->v20 migration-chain assertions updated) - `devtools test tests/unit/devtools/test_verify_raw_authority_frontier_executability.py` -- 6 passed, including an anti-vacuity test reproducing the exact pre-#3466 defect shape in a synthetic fixture and proving the lint catches it - `devtools test -k raw_authority` -- 90 passed, 6 failed (confirmed pre-existing/unrelated: `test_raw_authority_scale_proof.py` / `test_raw_authority_daemon_health_proof.py`, identical to the prior lb39z session's findings) - `devtools test -k raw_materialization` -- 118 passed - `devtools verify --quick` -- clean (format, lint, mypy --strict, render all --check, topology projection regenerated) Ref polylogue-lb39z (Phase 1, items 3 and 4) / polylogue-w32w Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
Sinity
added a commit
that referenced
this pull request
Aug 3, 2026
…e tie-break (#3616) ## Summary Fixes a cross-feature interaction between PR #3574 (byte-identical duplicate collapse) and the pre-existing (#3406) membership-census guard: a duplicate of the accepted revision baseline could make `plan_revision_replay` misclassify a cohort with one unambiguous baseline as ambiguous, which then routed backfill/rebuild into a fallback path that tripped `ActiveByteRevisionChainError`. ## Problem PR #3574 collapses byte-identical full-revision captures into one representative plus "duplicate" decisions. A follow-up fix (#3580) made a duplicate's `acquisition_generation` mirror its representative's generation for correct display ordering. Together these made a duplicate of the accepted baseline/head share that node's generation number. `plan_revision_replay` (`polylogue/archive/revision_replay.py`) selects the unique baseline by finding the single FULL, byte-proven candidate with the newest `acquisition_generation`. A duplicate sharing that generation now looked like a second competing baseline, so the tie-break declared the cohort ambiguous and returned an empty `accepted_raw_ids` for a cohort that in fact has one unambiguous baseline. Backfill/rebuild callers (`sources/revision_backfill.py`) treat an empty `accepted_raw_ids` as "no accepted chain" and fold every full-only raw for the identity into membership governance via `replace_raw_membership_census(..., retire_full_revision_governance=True)`. That path's guard then raised `ActiveByteRevisionChainError` the moment it tried to retire the baseline raw, because the duplicate's own `baseline_raw_id` column still durably points at it. Confirmed via `git log -S "ActiveByteRevisionChainError"` that the guard itself is unchanged since #3406; the trigger is #3574/#3580's newly-created generation-sharing shape, not the guard. ## Solution The guard is correct and intentionally strict: a raw genuinely still pointed at by another raw's `predecessor_raw_id`/`baseline_raw_id` must not be retired out of byte governance (a real dependent chain shouldn't lose its baseline out from under it). The defect is upstream, in `plan_revision_replay`'s inability to distinguish a harmless duplicate from a genuinely competing baseline. A FULL, byte-proven candidate with no `predecessor_raw_id` that is NOT itself the cohort's `baseline_raw_id` can only be such a duplicate -- the classifier that writes these columns (`archive/revision_authority.py`) guarantees at most one true root per cohort (`relation="baseline"`). `plan_revision_replay` now excludes that signature from the baseline candidate pool via `_is_full_duplicate_signature`; excluded duplicates still fall through to the existing trailing loop and are marked `DEFERRED` against whatever head the genuine evidence accepts. No schema change; this is a pure classification/replay-planning fix in `polylogue/archive/revision_replay.py`, shared correctly by both the backfill/rebuild path (`revision_governance.classify_raw_revision_cohort`) and the live watcher (`sources/live/batch.py`), which both build `RevisionCandidate` lists from the same `raw_sessions` columns. ## Verification - `devtools test tests/unit/sources/test_revision_backfill.py::test_backfill_content_cache_across_pages_reduces_parses_and_matches_uncached_archive tests/unit/storage/test_rebuild_paging_content_order.py::test_rebuild_content_order_paging_dedups_first_time_classification_via_content_cache` -- 2 passed (previously failing with `ActiveByteRevisionChainError`; this is the reproduction the tracking item names). - `devtools test tests/unit/storage/test_revision_replay.py tests/unit/storage/test_raw_revision_authority.py tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_rebuild_paging_content_order.py tests/unit/sources/test_live_batch_support.py -k "revision or membership or duplicate or backfill or paging"` -- 127 passed. - Full-file runs of the same five files -- 169 passed, 3 pre-existing failures in `test_live_batch_support.py` confirmed identical with this change reverted (`git stash`); unrelated to this fix. - Red-first: the two new tests in `test_revision_replay.py` were confirmed failing against the unpatched `revision_replay.py` (`git stash`), then passing after the fix. - `devtools verify --quick` -- exit 0 (format, lint, mypy --strict, render all --check, layering, closure-matrix, schema roundtrip, schema-versioning, classifier-fingerprints, schema-promotion-audit, etc., all passed). Anti-vacuity: the failing production path exercised is `sources/revision_backfill.py`'s `if not plan.accepted_raw_ids:` fallback into `archive.replace_raw_membership_census(..., retire_full_revision_governance=True)`; the mutation that makes the new tests fail is reverting the `_is_full_duplicate_signature` exclusion in `plan_revision_replay` (confirmed via `git stash` on `polylogue/archive/revision_replay.py` alone). The guard invariant itself (a real dependent chain must block retirement) is separately re-asserted inside the new archive-level test by calling `replace_raw_membership_census` directly and expecting `ActiveByteRevisionChainError`, and is unaffected by this change (pre-existing `test_membership_sweep_defers_sibling_retirement_instead_of_quarantining_current_raw` in `test_live_batch_support.py` still passes unmodified). Ref polylogue-qhk8z ## Scope not addressed None -- this closes the full interaction described in the tracking item (root-cause fix in the replay planner, not a guard loosening), with a regression test reproducing the exact reported failure plus a unit-level test pinning the underlying tie-break defect.
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
Fixes the classifier defect identified in the 2026-08-02 authority-dataflow audit (invariants I4/I5): byte-identical duplicate captures of one source were treated as an unprovable fork, quarantining the entire revision cohort even when only a duplicate observation was involved. This is item 1 of 5 in the raw-authority-redesign Phase 1 plan (polylogue-lb39z).
Problem
classify_historical_full_revision_streams(and its eager siblingclassify_historical_full_revisions,polylogue/archive/revision_authority.py) quarantined an entire full-revision cohort the moment any two members tied on size, without ever comparing their bytes. Two byte-identical captures of the same file — the ordinary result of re-acquiring an unchanged source — tie on size and triggered this path. A live, read-only audit measured 13,671 quarantinedraw_sessionsrows (50.2GB, 79% of quarantined bytes) caused by exactly this, and traced a second-order effect: a stuck baseline from this bug lets the live watcher pump unbounded quarantined rows for a single growing file (one Codex rollout produced 799 quarantined snapshots / 6.28GB of overlapping tails, since the append path can't chain onto a quarantined baseline).Separately, the classifier's ambiguity verdict was cohort-atomic: one genuinely divergent pair anywhere in a cohort quarantined every member, including a proven prefix chain that led up to the fork.
Solution
Both classifiers now:
relation="duplicate".BYTE_PROVENif it sits on a single, unbranched path back to the cohort's one true root (no ambiguous parent set, no sibling fork anywhere on the path). A shared ancestor with two mutually-incomparable children now classifies the ancestorBYTE_PROVENand only the fork's childrenQUARANTINED, instead of quarantining all three. Two genuinely disconnected/incomparable roots (no shared ancestor at all) still quarantine everything — there is no anchor to localize from, matching prior behavior for that irreducible case.classify_untyped_full_revision_groups(storage/sqlite/archive_tiers/revision_governance.py) is updated to check that every decision in a cohort isBYTE_PROVEN(not just the first), preserving its existing "whole cohort must be one provable chain" contract now that partial-cohort verdicts are possible.classify_raw_revision_cohort's baseline/generation-number derivation required no changes: duplicate and ambiguous decisions both leavepredecessor_raw_id=None(matching the pre-existing convention for ambiguous rows), so the existingchildrendict walk used to number generations never collides.Alternatives rejected
Promoting the fork's larger/newer-looking child as the accepted head was considered and rejected: it would make an arbitrary size/sort-order tie-break silently pick a winner among genuinely competing evidence, exactly the kind of silent-wrong-guess this subsystem has zero tolerance for (see the
polylogue-yla8incident history). Leaving the fork tip quarantined for the (separate, still-unwired) judgment-assertion flow to adjudicate is the safe default and matches the report's own recommendation.Verification
devtools test tests/unit/storage/test_raw_revision_authority.py— 24 passed, including new anti-vacuity tests that assert the pre-fix classifier quarantined a byte-equal duplicate pair and a shared-root fork wholesale (both parametrized/dedicated tests reproduce the pre-fix bug, then prove the fix resolves it).devtools test -k "raw_revision or revision_governance or raw_authority"— 115 passed, 6 failed. All 6 failures reproduce identically with this branch's changes reverted (git stash), confirming they are pre-existing and unrelated: 5 aretests/unit/devtools/test_raw_authority_scale_proof.pysynthetic-corpus repair-pass assertions, 1 istests/integration/test_raw_authority_daemon_health_proof.py, a real-daemon-subprocess probe that timed out under host contention.mypy --stricton both touched modules plus the test file — no issues.devtools lab policy schema-versioning— intact (no schema touched by this PR).ruff check+ruff format --check— clean.devtools verify --quickgate — passed (ruff format/check, mypy, render all, topology, layering, closure-matrix, schema roundtrip, manifests, CI workflows, doc-commands, degrade-loudly, hash-boundary-census, classifier-fingerprints, schema-promotion-audit).Ref polylogue-lb39z
Summary by CodeRabbit
New Features
Bug Fixes