From aff0e405c77a712f1d9b2d10bb76c345d93f05e2 Mon Sep 17 00:00:00 2001 From: Sinity Date: Mon, 3 Aug 2026 10:42:42 +0200 Subject: [PATCH] fix(storage): exclude byte-identical duplicates from revision baseline tie-break Problem PR #3574 collapsed byte-identical full-revision captures into one representative plus "duplicate" decisions, and 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) picks the unique baseline by finding the single candidate with the newest acquisition_generation among FULL, byte-proven candidates; 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 and 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 pre-existing (#3406) 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. Solution The guard itself 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. 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. plan_revision_replay now excludes that signature from the baseline candidate pool; excluded duplicates fall through to the existing trailing loop and are marked DEFERRED against whatever head the genuine evidence accepts. 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). - 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. - devtools test tests/unit/storage/test_revision_replay.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 (full 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: new tests confirmed failing against the unpatched revision_replay.py (git stash), then passing after the fix. - devtools verify --quick running in background; result to be added before PR/merge. Ref polylogue-qhk8z --- polylogue/archive/revision_replay.py | 42 +++++++++- tests/unit/storage/test_revision_replay.py | 91 ++++++++++++++++++++++ 2 files changed, 132 insertions(+), 1 deletion(-) diff --git a/polylogue/archive/revision_replay.py b/polylogue/archive/revision_replay.py index 8be9f8c99b..2705deb974 100644 --- a/polylogue/archive/revision_replay.py +++ b/polylogue/archive/revision_replay.py @@ -52,11 +52,49 @@ def accepted_raw_ids(self) -> tuple[str, ...]: return self.accepted_chain +def _is_full_duplicate_signature(candidate: RevisionCandidate) -> bool: + """Detect a byte-identical "duplicate" decision from its stored columns alone. + + ``revision_governance.classify_raw_revision_cohort``'s classifier + (``archive/revision_authority.py``'s prefix-DAG proof) guarantees at most + one node per cohort is ever the genuine chain root (``relation= + "baseline"``, ``predecessor_raw_id=None``); every other proven chain + member carries a ``predecessor_raw_id``. A byte-identical "duplicate" + decision deliberately also gets ``predecessor_raw_id=None`` (it is not a + chain-continuing child of anything -- see + ``HistoricalRevisionDecision.duplicate_of_raw_id``), so a FULL, + BYTE_PROVEN candidate with no predecessor that is NOT itself the cohort's + ``baseline_raw_id`` can only be such a duplicate. Its + ``acquisition_generation`` mirrors its representative's real chain + position purely for display/ordering (polylogue-5unky); treating it as an + independent competing "newest full baseline" candidate here made it tie + with its own representative whenever it duplicated the accepted head, + producing a false "multiple byte-proven full baselines share the newest + generation" ambiguity for a cohort that in fact has one unambiguous + baseline -- which then routed the whole cohort into the "no accepted + chain" membership-census fallback and tripped + ``ActiveByteRevisionChainError`` on retirement, even though the + duplicate's own baseline/predecessor links were never unsafe to retire + past (polylogue-qhk8z). + """ + return ( + candidate.kind is RawRevisionKind.FULL + and candidate.authority is RawRevisionAuthority.BYTE_PROVEN + and candidate.predecessor_raw_id is None + and candidate.baseline_raw_id is not None + and candidate.baseline_raw_id != candidate.raw_id + ) + + def plan_revision_replay(candidates: list[RevisionCandidate]) -> RevisionReplayPlan: """Choose a unique proven full baseline and its exact append chain. No enumeration order, timestamp, raw-id ordering, or provider timestamp can promote evidence. A tie or branch stops replay at the last unique head. + A byte-identical duplicate of an already-accepted node (see + ``_is_full_duplicate_signature``) never competes for baseline selection; + it is left for the trailing loop below to mark ``DEFERRED`` against + whatever head the genuine evidence accepts. """ if not candidates: raise ValueError("revision replay requires at least one candidate") @@ -71,7 +109,9 @@ def plan_revision_replay(candidates: list[RevisionCandidate]) -> RevisionReplayP proven_full = [ candidate for candidate in candidates - if candidate.kind is RawRevisionKind.FULL and candidate.authority is RawRevisionAuthority.BYTE_PROVEN + if candidate.kind is RawRevisionKind.FULL + and candidate.authority is RawRevisionAuthority.BYTE_PROVEN + and not _is_full_duplicate_signature(candidate) ] applications: dict[str, RevisionApplication] = {} if not proven_full: diff --git a/tests/unit/storage/test_revision_replay.py b/tests/unit/storage/test_revision_replay.py index 98a86e6ff4..0dd93fc2ae 100644 --- a/tests/unit/storage/test_revision_replay.py +++ b/tests/unit/storage/test_revision_replay.py @@ -361,6 +361,40 @@ def test_replay_requires_byte_proven_full_baseline() -> None: assert _decisions(candidates) == {"asserted": ApplicationDecision.DEFERRED} +def test_replay_does_not_treat_a_duplicate_of_the_accepted_baseline_as_a_competing_head() -> None: + """polylogue-qhk8z: a byte-identical duplicate of the accepted baseline must + not create a false "multiple byte-proven full baselines" tie. + + ``revision_governance.classify_raw_revision_cohort`` writes a duplicate + decision's ``baseline_raw_id`` to the SAME chain root as the real + baseline row (``predecessor_raw_id=None`` on both, per + ``HistoricalRevisionDecision.duplicate_of_raw_id``'s contract), and + mirrors the baseline's own ``acquisition_generation`` onto it + (polylogue-5unky's fix). Before this fix, ``plan_revision_replay``'s + "unique newest generation" tie-break saw two FULL BYTE_PROVEN candidates + sharing generation 0 and misclassified this as an ambiguous multi- + baseline fork -- even though one of the two candidates literally IS the + baseline (``baseline_raw_id == raw_id``) and the other is only its own + duplicate. That false ambiguity emptied ``accepted_raw_ids``, which + routed backfill/rebuild callers into the "no accepted chain" membership- + census fallback for a cohort that in fact has one unambiguous baseline, + which then tripped ``ActiveByteRevisionChainError`` on retirement + (reproduced end-to-end in + ``test_duplicate_of_accepted_baseline_does_not_trip_membership_census_guard``). + """ + candidates = [ + _candidate("raw-a-baseline", RawRevisionKind.FULL, 0, baseline="raw-a-baseline"), + _candidate("raw-b-duplicate", RawRevisionKind.FULL, 0, baseline="raw-a-baseline"), + ] + plan = plan_revision_replay(candidates) + assert plan.accepted_raw_ids == ("raw-a-baseline",) + decisions = {item.raw_id: item.decision for item in plan.applications} + assert decisions == { + "raw-a-baseline": ApplicationDecision.SELECTED_BASELINE, + "raw-b-duplicate": ApplicationDecision.DEFERRED, + } + + def test_cohort_classification_promotes_late_baseline_and_deferred_append(tmp_path: Path) -> None: initialize_active_archive_root(tmp_path) with ArchiveStore.open_existing(tmp_path, read_only=False) as archive: @@ -506,6 +540,63 @@ def test_duplicate_generation_copy_does_not_drop_the_chain_continuing_representa assert _acquisition_generation(archive, mid_duplicate) == 1 +def test_duplicate_of_accepted_baseline_does_not_trip_membership_census_guard(tmp_path: Path) -> None: + """polylogue-qhk8z end-to-end reproduction: PR #3574's byte-identical- + duplicate collapse (I4) plus polylogue-5unky's generation-mirroring fix + together made a duplicate of the accepted baseline share that baseline's + ``acquisition_generation``. Before the ``plan_revision_replay`` fix + (``test_replay_does_not_treat_a_duplicate_of_the_accepted_baseline_as_a_ + competing_head``), that shared generation made ``plan_revision_replay`` + see two competing "newest" full baselines and reject the cohort as + ambiguous, emptying ``accepted_raw_ids`` even though the cohort has one + genuine, unambiguous baseline. Callers (``sources/revision_backfill.py``, + ``sources/live/batch.py``) treat an empty ``accepted_raw_ids`` as "no + accepted chain" and fall back to folding every full-only raw for this + identity into membership governance via + ``replace_raw_membership_census(..., retire_full_revision_governance= + True)`` -- which raised ``ActiveByteRevisionChainError`` the moment it + tried to retire the baseline raw_id, because the duplicate's own + ``baseline_raw_id`` column still durably points at it. This reproduces + that exact interaction against a real archive (mirroring the two-page + re-export shape ``test_revision_backfill.py`` and + ``test_rebuild_paging_content_order.py`` construct) and proves the + cohort is now accepted outright, so the membership-census fallback path + is never even reached -- while confirming the guard itself still fails + closed for a raw a duplicate genuinely still depends on. + """ + initialize_active_archive_root(tmp_path) + with ArchiveStore.open_existing(tmp_path, read_only=False) as archive: + baseline = _write_full_raw(archive, raw_id="raw-a-baseline", payload=b"hello world", acquired_at_ms=1) + duplicate = _write_full_raw(archive, raw_id="raw-b-duplicate", payload=b"hello world", acquired_at_ms=2) + + plan = archive.classify_raw_revision_cohort("codex:session") + + # The cohort has a unique byte-proven baseline -- the duplicate no + # longer manufactures a false "multiple newest baselines" ambiguity. + # Backfill/rebuild callers (sources/revision_backfill.py) gate the + # membership-census fallback on exactly this ``not + # plan.accepted_raw_ids`` check, so a non-empty result here means + # that fallback -- and the guard inside it -- is never invoked for + # this cohort in production. + assert plan.accepted_raw_ids == (baseline,) + assert _acquisition_generation(archive, duplicate) == 0 + + # The membership-census guard itself must remain intact: retiring + # the baseline directly still fails closed, because the duplicate's + # baseline_raw_id column durably points at it -- a real dependent, + # not a false one. + with pytest.raises(archive_revision_governance.ActiveByteRevisionChainError): + archive.replace_raw_membership_census( + baseline, + [], + parser_fingerprint=RAW_AUTHORITY_PARSER_FINGERPRINT, + censused_at_ms=0, + detail="test-duplicate-guard", + retire_full_revision_governance=True, + ) + archive.rollback() + + def test_real_append_chain_folds_segmentation_distinct_full_snapshot(tmp_path: Path) -> None: initialize_active_archive_root(tmp_path)