refactor(archive): collapse revision comparison into a content-only relation - #3401
Merged
Conversation
Extends the identity/acquisition split from polylogue-bu1i to two more volatility sources: message array order and provider-reported generation duration. Both currently masquerade as branches and quarantine cohorts that are byte-identical in every way that matters. SessionRevisionProjection gains message_identities/message_contents (order-insensitive id-to-content pairs) and event_identity_hashes (measurement fields stripped from events whose own payload declares duration_semantics == "provider_reported_elapsed"). session_hash is unchanged -- it still covers order and the full event payload, so a real reorder or a real duration change still triggers a re-write. Only the revision-comparison axes in session_revision_membership.py are tolerant. Ref polylogue-c429 Ref polylogue-nuec Co-Authored-By: Claude <noreply@anthropic.com>
…ixes Adds mutation-verified coverage for the permutation-tolerant message comparison and duration-tolerant event comparison landed in the previous commit, and drops the unused message_identities projection field (the identity/content split needed for attachments' lazy-fetch axis is redundant for messages, whose content is never lazily missing -- the content-agreement check alone already implies identity subset). Each new clause was mutated and confirmed to fail without the guard it implements (see PR body for the full table); two initially-vacuous tests (message-id-disappearing, message-content-changed under an equal-count reorder) were replaced with growth-variant shapes that actually exercise _message_evidence_preserved rather than being short-circuited by the count-based growth check. Ref polylogue-c429 Ref polylogue-nuec Co-Authored-By: Claude <noreply@anthropic.com>
Scope addition (polylogue-d8al, filed while this branch was in flight): hith's parser-side fix for synthetic-id volatility resolved 0 of the 566 claude-ai-export ambiguous cohorts on the live archive -- a full census found the population is instead saturated by a different axis: one export vintage of a conversation carries a real provider id for an attachment, the other has none at all and synthesizes one. No id-minting scheme can make a real id and a synthetic hash collide, so this belongs in the comparison layer, not the parser. SessionRevisionProjection gains attachment_records, a per-attachment (strict identity, loose id-independent identity, content) triple. session_revision_membership.py correlates attachments pairwise: strict id first (unchanged bu1i behavior when both sides agree on an id), falling back to the loose (message, name, mime_type) key only when that key is unambiguous on BOTH sides being compared. classify_membership_revisions gains a merge pass (_merge_attachment_id_presence_variants) so two content groups that differ only in attachment id presence -- not growth, pure equivalence -- can still resolve via the existing timestamp mechanism. attachment_identities/attachment_contents stay strict (unchanged semantics, unchanged callers in repair.py/archive.py); only the comparison-layer correlation is new. Known, documented limit: two genuinely distinct attachments sharing one message/name/media-type with no bytes on either side are indistinguishable -- stated plainly rather than guessed at by array position, which is the bug this replaces. This is a pure comparison-layer fix over parsed content already stored as raw bytes: a full index rebuild that replays existing raws through parse_payload -> session_revision_projection -> classify_membership_revisions applies it to already-ingested cohorts, not only new acquisitions. Ref polylogue-d8al Ref polylogue-c429 Ref polylogue-nuec Co-Authored-By: Claude <noreply@anthropic.com>
Problem (discovered via live-archive census while validating c429/nuec/ d8al): the census showed almost no cohort actually resolving end to end. Root cause -- most live cohorts have >2 raw duplicates, and once the tolerant content key correctly groups a permuted/duration-varied/ id-mismatched pair together, the pre-existing by_session_hash sub-split still requires two DISTINCT provider timestamps to pick a representative. For pure export-vintage noise (reorder, remeasured duration, id presence) the provider's own updated_at legitimately never moves between two export requests of an untouched conversation, so that tie-break could never fire and the group stayed ambiguous forever regardless of the c429/nuec/d8al fixes. SessionRevisionProjection gains metadata_hash (title/created_at/updated_at, normalized, matching exactly the session_hash payload fields NOT already covered by the content-key axes). Within an already-content-key-equivalent group, if every variant also agrees on metadata_hash, the ONLY remaining source of a session_hash difference is one of the three tolerated axes, so picking any one (min raw_id) is exactly as safe as the pre-existing exact-session_hash collapse a few lines above it -- and it is proven not to apply when a real title/timestamp edit is present (both pre-existing equal-timestamp-stays-ambiguous tests are unchanged and still pass). A stronger "never leave a document absent" presence-guarantee fallback was also explored (deterministic frontier-max pick for genuinely divergent cohorts) but reverted: it conflicts with archive.py's existing "membership replay cannot retire an unrelated accepted head" invariant when a later ambiguous batch's fallback pick differs from an already-established head, raising a RuntimeError on real replay (proven by test_divergent_bundle_member_preserves_last_accepted_session). Fixing that requires plumbing existing-head awareness into the classifier or its caller, which touches archive.py's write path -- out of this lane's scope (archive.py/repair.py are owned by concurrent lanes). Recommending that as a separate, coordinated follow-up rather than merging a change that can crash a live rebuild. Ref polylogue-c429 Ref polylogue-nuec Ref polylogue-d8al Co-Authored-By: Claude <noreply@anthropic.com>
… relation Scope change (polylogue-aggz, filed while c429/nuec/d8al were in review): one day of investigation produced four separately-named volatility fixes (bu1i, c429, nuec, d8al/hith) and correspondingly four special-case code paths in the comparison layer. The operator's directive: stop naming failure cases and make them unrepresentable by one invariant instead. A conversation is a SET of items (messages, attachments, events) keyed by content-derived identity, each carrying only content-bearing fields -- nothing else may enter the value used to compare two acquisitions of it. Concretely: - Identity is never array position. message_contents/attachment_contents/ event_contents are frozensets, not ordered tuples. - Identity is derived from content, never a provider id whose presence is itself unstable. Attachment identity drops the provider id entirely (message_id, name, mime_type only) instead of using it when present and falling back when absent -- the strict/loose duality and its pairwise correlation machinery (_correlate_attachments, _attachments_equivalent, AttachmentRecord, the by_content/by_session_hash/metadata_variants/ timestamp-tiebreak layering) are deleted, not bypassed. - Acquisition state and provider-reported measurement are not content. Event content is built from an explicit per-event-type ALLOWLIST (_EVENT_CONTENT_PAYLOAD_ALLOWLIST) instead of a denylist of fields discovered volatile after shipping -- a new field a parser adds later is excluded from comparison by construction, not by someone remembering to strip it. Two revisions are now `equal` (same id set, equal content per id), `a_contains_b`/`b_contains_a` (one side's id set contains the other's with equal content on the overlap -- set containment, not sequence prefix, so a reorder never breaks it), or `conflict` (content disagrees on the overlap, or each side holds something the other lacks). One `_relation` function replaces the message dominance test, the event-hash prefix test, the attachment correlation pass, and the metadata-timestamp tiebreak. `_provider_ordered_browser_snapshots` was evaluated for deletion (per the same directive) and kept: browser-captured DOM/native snapshots synthesize their own local ids from DOM structure, not stable provider identity, so the content-only relation genuinely cannot correlate a DOM-to-native fidelity upgrade -- deleting it would silently break real browser-capture archival behavior this lane has no fixture coverage to verify against. A maximal-evidence presence-guarantee fallback for irreducible conflicts (deterministic by _frontier + raw_id, proven order-independent) is implemented and unit-tested as _maximal_evidence_fallback but NOT wired into classify_membership_revisions: doing so trips a real, documented archive.py write-back invariant (never retire an unrelated accepted head, with production incident history behind it -- polylogue-miwv, #3397/#3398) whenever the fallback pick differs from an already-established head, proven by two failing integration tests (test_divergent_bundle_member_preserves_last_accepted_session and its sibling). Landing it safely needs either the classifier or its caller to carry existing-head context, or the write-back guard to accept a re-affirmed-quarantined outcome -- both belong to archive.py's write path, out of this lane's scope. Reported as a concrete follow-up rather than forced through. Ref polylogue-aggz Ref polylogue-bu1i Ref polylogue-c429 Ref polylogue-nuec Ref polylogue-d8al Ref polylogue-hith Co-Authored-By: Claude <noreply@anthropic.com>
Ref polylogue-c429 Ref polylogue-nuec Ref polylogue-d8al Ref polylogue-aggz Co-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 56 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 (6)
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 |
# Conflicts: # .beads/issues.jsonl
![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: ce17be0a69
ℹ️ 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".
Comment on lines
+177
to
+180
| match_index = next( | ||
| (i for i, rep in enumerate(representatives) if _relation(rep.projection, revision.projection) == "equal"), | ||
| None, | ||
| ) |
There was a problem hiding this comment.
When a direct export and a DOM/native browser capture have equal projected message, attachment, and event content, this loop collapses them before _direct_export_precedence or browser-fidelity ordering runs. Because the representative is then selected only by provider_updated_at or raw_id, the browser capture can supersede the authoritative direct export—or DOM can supersede native—even when their full session hashes and metadata differ, causing the lower-fidelity parsed session to be materialized.
Useful? React with 👍 / 👎.
Comment on lines
+527
to
+530
| canonical_identity = ( | ||
| base_identity | ||
| if base_identity_counts[base_identity] == 1 | ||
| else bytes.fromhex(hash_payload({"base_identity": base_identity.hex(), "content": content_hash.hex()})) |
There was a problem hiding this comment.
When a revision initially has one event for a given (event_type, source_message_provider_id) and a later revision appends a second event with that same base identity, the existing event's identity changes from base_identity to hash(base_identity, content). The old and new identity sets therefore become disjoint instead of showing containment, so _relation reports a conflict and can quarantine an ordinary event-growth revision.
Useful? React with 👍 / 👎.
Comment on lines
+80
to
+81
| content_a = dict(contents_a) | ||
| content_b = dict(contents_b) |
There was a problem hiding this comment.
If one message contains two acquired attachments with the same name and MIME type, the new attachment identity projection assigns both the same identity, while dict(contents_a)/dict(contents_b) silently retains only one content hash. Adding or removing the second attachment is consequently classified as either equality or conflict rather than containment, so a legitimate revision can be discarded or quarantined; this also exceeds the documented limitation, which claims only byte-less duplicates are indistinguishable.
Useful? React with 👍 / 👎.
Comment on lines
+329
to
+334
| allowlist = _EVENT_CONTENT_PAYLOAD_ALLOWLIST.get(event.event_type) | ||
| if allowlist is None: | ||
| payload = event.payload | ||
| timestamp = event.timestamp | ||
| else: | ||
| payload = {key: value for key, value in event.payload.items() if key in allowlist} |
There was a problem hiding this comment.
The allowlist is selected solely from event_type, so every generation_lifecycle event loses its timestamp and all payload fields outside state, evidence_source, and fidelity. The browser-capture parser emits this same event type with duration_semantics such as dom_observed_wall and provider_ui_elapsed; revisions whose real observation IDs, wall/displayed durations, labels, or triggers differ can therefore be marked equivalent and one measurement discarded, despite the volatility exception being intended only for provider_reported_elapsed.
Useful? React with 👍 / 👎.
…on relation Problem: PR #3401 review (chatgpt-codex-connector) found the content-only comparison relation from polylogue-aggz did not yet fully hold the invariant it claims in four concrete ways. What changed: - session_revision_membership.py: equal-content collapse now decides its representative by source authority (direct export over browser capture, native over DOM) BEFORE falling back to provider timestamp / raw_id, via new _equal_content_representative -- mirrors the ordering _direct_export_precedence/_browser_snapshot_dominates already apply to the non-equal growth-chain case, so equal content no longer lets a lower- fidelity capture supersede its authoritative export (P1). - session_revision_membership.py: _axis_relation now groups content hashes by identity into a set (_content_by_identity) instead of a plain dict, so two colliding attachment identities (same message/name/mime, different bytes) retain every content hash instead of one silently overwriting the other -- a real collision now degrades to conflict, never to equal (P2). - pipeline/ids.py: an event's canonical identity is now always hash(base_identity, content), never conditionally base_identity alone depending on whether a sibling happens to share that base identity in the SAME revision -- an item's identity must not depend on what else is in the set (P2, the most serious of the four: it reintroduced exactly the bug class aggz exists to eliminate). - pipeline/ids.py: the generation_lifecycle duration-stripping allowlist now applies only when the event's own payload declares duration_semantics == "provider_reported_elapsed" (the ChatGPT-parser shape polylogue-nuec targeted), not to every event of that type -- the browser-capture parser's own DOM/UI generation observations (different duration_semantics values) keep their observation id, timestamps, and duration/label/trigger fields as real content (P2). Wires up the previously-dead _PROVIDER_REPORTED_ELAPSED_MARKER_KEY/_VALUE constants. Not touched, per explicit instruction: _provider_ordered_browser_snapshots stays (kept, not deleted); _maximal_evidence_fallback stays designed/tested but unwired (archive.py write-back invariant, out of this lane's scope). Verification: devtools test tests/unit/archive/test_session_revision_membership.py tests/unit/pipeline/test_pipeline_ids.py tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_revision_replay.py tests/unit/storage/test_browser_capture_origin_repair.py -- 133 passed. Ref polylogue-aggz Co-Authored-By: Claude <noreply@anthropic.com>
…tation-proof tests Adds a regression test per review finding, each verified by manually mutating the corresponding production line back to the buggy behavior and observing the exact expected failure (reverted afterward, not part of this commit): - P1 (source authority in equal-content collapse): two tests where the direct/native revision loses on BOTH timestamp and raw_id tiebreaks, so only source-authority ordering can make it win. Mutating out the new authority-first branches in _equal_content_representative flips both assertions (raw-zzzz-dom/raw-aaaa-capture win instead of the authoritative revision). - P2 (attachment identity collision): a direct unit test on _content_by_identity proving type-mismatch-guaranteed failure against the old dict()-based grouping, plus an end-to-end test asserting two acquired attachments colliding on identity with different bytes classify as conflict. Reverting _content_by_identity's call site to dict() flips the end-to-end assertion to "equal". - P2 (event identity sibling instability): a growth-chain test (one block event, then a second appended) asserting b_contains_a and full containment. Reintroducing the base_identity_counts-conditional identity computation flips this to a set-subset assertion failure (disjoint identities). - P2 (duration-stripping scope): a browser-capture generation_lifecycle test with differing observation ids/durations but IDENTICAL state/evidence_source/fidelity, asserting conflict. Removing the duration_semantics marker gate flips this to "equal" (both collapse to the same stripped payload). Verification: devtools test tests/unit/archive/test_session_revision_membership.py tests/unit/pipeline/test_pipeline_ids.py tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_revision_replay.py tests/unit/storage/test_browser_capture_origin_repair.py -- 139 passed. devtools verify --quick -- exit_code 0. Ref polylogue-aggz Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Jul 30, 2026
Sinity
added a commit
that referenced
this pull request
Jul 30, 2026
…chive.py (#3406) ## Summary Splits the raw-revision-authority and membership-classification concern out of `polylogue/storage/sqlite/archive_tiers/archive.py` (13,338 lines) into a new module, `polylogue/storage/sqlite/archive_tiers/revision_governance.py` (2,841 lines), so that concern stops living inside the query-surface god-file. ## Problem `docs/architecture-hotspots.md` documents `archive.py`'s public contract as "`ArchiveStore` — every SELECT-shaped query surface (sessions, messages, blocks, insights reads, search)". The file also owned ~55 `raw_*`/write methods implementing revision/membership write authority — a different concern with different invariants, and the exact cluster where every defect found on 2026-07-30 lived (PRs #3394, #3396, #3397, #3398, #3401). Ref polylogue-1r9c (decomposition epic), polylogue-c737 (a defect from this cluster). ## Solution - New module `archive_tiers/revision_governance.py` owns raw-revision replay, membership classification/census, and the narrow raw-write paths that hand a parsed session to that authority — documented contract in the module docstring (what it owns / what it refuses). - Every governance function takes `store: RawRevisionGovernanceHost` (a `Protocol`) as its first argument instead of being a method on `ArchiveStore`. The protocol names exactly the seven `ArchiveStore` members governance code touches (`_conn`, `_ensure_source_conn`, `_blob_publisher`, `_pending_raw_parse_states`, `_preacquire_attachment_blobs`, `_write_counts`, `_skipped_counts`). `ArchiveStore` satisfies it structurally — no inheritance, no import of `ArchiveStore` from the new module (which would create an import cycle). - `ArchiveStore` keeps one-line delegating methods with unchanged signatures, so every external caller (`sources/live/batch.py`, `sources/live/append_ingest.py`, `sources/revision_backfill.py`, `storage/repair.py`, `pipeline/services/archive_ingest.py`, `api/archive.py`, and every test holding an `ArchiveStore` instance) is untouched. This is not a compatibility shim — there is exactly one implementation (in the new module), and the delegator body is the call site, same shape as any other extract-function-then-delegate refactor. - Updated `docs/plans/layering.yaml`'s `writer_modules` inventory: `archive.py`'s only remaining direct writer is `delete_sessions` (index-only); the `raw-membership-classification` twin-write contract and its 11 entrypoints moved to the new module's own entry. - Regenerated `docs/plans/topology-target.yaml` / `docs/topology-status.md`; updated `docs/architecture-hotspots.md`'s line-count row and `docs/plans/hash-boundary-registry.yaml`'s moved `hashlib.sha256` call site. ### Connection-interface decision (the design question this task turns on) Considered and rejected two alternatives: - **Bare `sqlite3.Connection`** — insufficient. Governance needs the lazily- opened `source.db` connection, the blob publisher, and the pending-raw- parse-state batch too, not just the index connection. - **A mixin `ArchiveStore` inherits from** — rejected because inheritance gives every moved method unrestricted `self` access to the other ~9,000 lines of read-surface internals, which is exactly the "reach back into ArchiveStore internals" this extraction is meant to make structurally impossible, not merely discouraged by convention. The `Protocol` makes the dependency surface an explicit, readable, narrow contract instead of "whatever `self` happens to have". ### A real regression found and fixed mid-PR Four tests monkeypatch an `ArchiveStore` method as a spy/crash-injection point (`_index_parsed_for_retained_raw`, `_write_parsed_precedence_result`, `mark_raw_parse_succeeded`, `record_revision_application_sync`). Under the old single-class shape, sibling governance methods called each other via `self.<method>()`, so patching the class attribute intercepted internal calls too. After the move, sibling governance *functions* call each other by direct module-global reference, bypassing the `ArchiveStore` delegator entirely — so those four tests silently stopped testing what they claimed to. Confirmed as a genuine regression (not pre-existing) by running the exact failing tests against a detached checkout of the pre-extraction parent commit — all passed there. Fixed by patching the `revision_governance` module attribute (the real internal call target) in the affected tests instead of the `ArchiveStore` delegator, across `tests/unit/storage/test_revision_replay.py`, `tests/unit/sources/test_revision_backfill.py`, `tests/unit/sources/test_live_batch_support.py`, and `tests/unit/sources/test_live_cursor_persistence.py`. This is the signal that behavior (specifically, internal call dispatch) moved, not a change in externally observable archive behavior. ## Non-goals / what was deliberately left alone - `polylogue/pipeline/ids.py`, `polylogue/archive/session_revision_membership.py`, `polylogue/sources/dispatch.py`, the parsers, and `polylogue/pipeline/services/ingest_batch/*` — untouched, per scope. - `write_hook_event` stays in `archive.py` — hook-event ingest is a different concern (evidence linked to a session, never itself a raw revision candidate; polylogue-31r1), not moved. - No import cycle formed; the new module never imports `ArchiveStore`. ## Verification - `mypy --strict` on every touched module: clean. - `devtools test` — mission's targeted files plus every file discovered by grepping for `monkeypatch.setattr(...)` on any of the 24 governance names called internally by a sibling governance function: **215 passed, 0 failed** (`tests/unit/storage/test_revision_replay.py`, `tests/unit/sources/test_revision_backfill.py`, `tests/unit/storage/test_raw_authority_ledger.py`, `tests/unit/sources/test_live_batch_support.py`, `tests/unit/storage/test_raw_revision_authority.py`, `tests/unit/sources/test_live_cursor_persistence.py`, `tests/unit/pipeline/test_archive_ingest_commit_batching.py`). Note: `tests/unit/storage/test_crud.py` named in the task no longer exists in this checkout (removed by prior test-infra churn) — confirmed via `git log --all -- tests/unit/storage/test_crud.py`, skipped. - `devtools verify --quick`: green (ruff format/check, mypy --strict, render all, topology, layering, hash-boundary-census, all other gates). - `archive.py`: 13,338 → 11,324 lines (-15.1%). New module: 2,841 lines. Not run: `devtools verify --all` (full suite) — out of scope for a `--quick` gate per repo convention; CI's post-merge `test` job will run it.
Sinity
added a commit
that referenced
this pull request
Jul 30, 2026
…ors (#3405) ## Summary Makes it structurally impossible for non-content data to enter a session's comparison-identity value: message/attachment/event identity now go through four fixed keyword-only constructors instead of a dict-key-list projection. ## Problem `session_revision_projection` (polylogue-aggz Invariant 1, landed in #3401) built comparison identity by slicing an already-constructed hash-stable payload dict against a tuple of allowed field names (`_MESSAGE_IDENTITY_FIELDS`, `_ATTACHMENT_IDENTITY_FIELDS`, `_EVENT_BASE_IDENTITY_FIELDS`). Two real, expensive defects were exactly a non-content field reaching that value: polylogue-bu1i folded attachment acquisition state (`inline_bytes`/`size_bytes`) into identity (151 aistudio-drive cohorts wrongly ambiguous, 14 sessions absent, a day of investigation), and polylogue-nuec folded a provider-reported measurement (`elapsed_duration_ms`) into event identity (135 chatgpt cohorts read as divergent despite byte-identical content). Both were fixed by hand, per field. Nothing prevented a third occurrence: any code could still call `hash_payload()` on an arbitrary dict and call it "identity" — the allowlist was a convention encoded in a tuple of strings, not enforced by the type system. `docs/plans/hash-boundary-registry.yaml` — a machine-checked census of every hash-producer call site in `polylogue/` — exists as a workaround for the missing chokepoint: since anyone could compute an identity hash anywhere, the repo keeps a list of where they did. ## Solution `polylogue/pipeline/ids.py` now exposes the sole path into each comparison-identity value as a fixed keyword-only function: - `message_identity_hash(*, id: str) -> bytes` - `attachment_identity_hash(*, message_id, name, mime_type) -> bytes` - `event_base_identity_hash(*, event_type, source_message_provider_id) -> bytes` - `event_canonical_identity_hash(*, base_identity, content_hash) -> bytes` Passing any other field — or spreading a whole payload dict as `**kwargs` — is a `TypeError` at the call boundary, not a value someone has to remember to strip. `session_revision_projection` now calls these instead of the removed `_message_identity_payload`/`_attachment_identity_payload`/ `_event_base_identity_payload` dict-slice helpers, which are deleted along with their backing field-name tuples. **Every stored/pinned hash stays byte-identical.** The constructors build the exact same dict shapes the removed helpers did (same keys, same values, `hash_payload` sorts keys so order is irrelevant), and `test_session_revision_projection_golden_hashes` (which pins exact digest bytes) and the independent-recomputation test both pass unchanged with no golden updates. This is a pure re-plumbing of an existing correct computation, not a semantic or schema change — the "if you find yourself editing the goldens, stop" tripwire never fired. **`docs/plans/hash-boundary-registry.yaml` is NOT retired.** It governs all 198 `hashlib`/`core.hashing` call sites across `polylogue/` (58+ files: blob-store content-addressing, HMAC signatures, redaction digests, cache/dedup keys, judgment-run identifiers, sinex material adapters, ...), of which only ~12 (in `pipeline/ids.py`) are the session/message/ attachment/event comparison-identity axis this bead targets. Claiming the registry's whole purpose is now covered would not survive a diff-grep. The registry's `pipeline/ids.py` entries were updated to match the new call sites (four new `<module>.<constructor>` entries replacing four `session_revision_projection` occurrences); `devtools verify hash-boundary-census` is clean. Filed polylogue-ubwg to audit whether any of the registry's other 91 `identifier`-classified sites share the aggz failure shape (a mutable/acquisition-state field folded into a value used for equality/dedup comparison) and would benefit from the same constructor pattern — only once every site is provably covered or provably out of class can the registry itself be deleted. ## Tests Added to `tests/unit/pipeline/test_pipeline_ids.py`: - `test_message_identity_hash_rejects_non_content_fields` — passing `text` raises `TypeError`. - `test_attachment_identity_hash_rejects_acquisition_state` — passing `size_bytes`/`inline_content_hash` raises `TypeError`. - `test_attachment_identity_hash_rejects_full_payload_spread` — simulates a future parser adding a brand-new field to the hash-stable payload and spreads the *entire* payload dict (including that new field) into the constructor; rejected outright, proving a new field can't silently enter identity by construction, not by someone remembering to exclude it. - `test_event_base_identity_hash_rejects_measurement_fields` — passing `payload={"elapsed_duration_ms": ...}` raises `TypeError`. - `test_identity_constructors_ignore_new_payload_fields_when_called_correctly` — two attachments differing only in acquisition state produce identical identity. - `test_event_canonical_identity_hash_folds_base_and_content` — sanity check on the fold. ## Verification - `devtools test tests/unit/pipeline/test_pipeline_ids.py tests/unit/archive/test_session_revision_membership.py` → 56 passed - `uv run mypy --strict polylogue/pipeline/ids.py polylogue/archive/session_revision_membership.py tests/unit/pipeline/test_pipeline_ids.py` → Success, no issues found - `devtools verify --quick` → exit_code 0 (includes `verify hash-boundary-census`) - Not run: `devtools verify --all` / full integration suite (out of scope per repo convention — per-PR CI skips the heavy `test` suite; targeted `devtools test` above covers every affected module). Ref polylogue-aggz. Co-Authored-By: Claude <noreply@anthropic.com> Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Jul 31, 2026
Sinity
added a commit
that referenced
this pull request
Jul 31, 2026
#3454) ## Summary Consolidates three independently-duplicated "skip this write as stale" freshness-tie checks (`archive_tiers/write.py`, `pipeline/services/ingest_batch/_core.py`, `archive_tiers/revision_governance.py`) into one `should_skip_stale_replace()` in `archive_tiers/ingest_precedence.py`. No behavior change: the comparison body is unchanged, just no longer copy-pasted three times. ## Problem `polylogue-t83e` investigated 6 live `session_id` collisions where a Claude Code transcript the operator uploaded into an AI Studio conversation (re-downloaded via Drive sync into `~/.local/share/polylogue/drive-cache/gemini/<uuid>.jsonl.txt.json`) was winning the archive row over the genuinely local `~/.claude/projects/...` transcript of the *same* conversation — even though the drive copy is a strict byte-prefix of the local file (confirmed by direct diff: 213 vs 214 Claude Code JSONL messages for one collision, the local file has exactly one trailing `summary` record the drive copy lacks). **Before/after collision table** (read-only queries against `/realm/db/polylogue/{source,index}.db`): | native_id | drive raw msgs | local raw msgs | `sessions.raw_id` today | `raw_session_memberships.decision` | relation under current code | |---|---|---|---|---|---| | 0213d48f-5b7a-4241-b77a-eb714672dc3b | 213 | 214 | drive (wrong) | `ambiguous` (stale, decided 2026-07-30T05:05Z) | `a_contains_b` (local dominates) | | 063a6885-8d6a-4f91-80b2-7f67fa06d680 | — | — | drive | `ambiguous` (stale) | not individually re-simulated; same code path | | 705f1fcb-8953-4b8b-92f1-9244fcf9db91 | — | — | drive | `ambiguous` (stale) | same | | 8c9f8c3d-4859-44cf-be9c-338803a8e7de | — | — | drive | `ambiguous` (stale) | same | | a952ffa4-73b0-48bd-a212-ebe5b9772d1e | — | — | drive | `ambiguous` (stale) | same | | cf3404fa-89e0-400a-af3e-ff1450eecef4 | — | — | drive | `ambiguous` (stale) | same | (15 drive-cache raws total, all genuinely Claude-Code-shaped by content, parsing into 12 sessions — 6 collide with a local raw as above, 6 do not.) **Root cause, precisely**: this is a revision-arbitration bug, not an identity/detection bug. `session_id = origin || ':' || native_id` correctly computes the same identity for both raws because they *are* the same conversation — that's not a defect. Which raw should win is `archive/session_revision_membership.py`'s job (a content-only set relation: `equal`/`a_contains_b`/`b_contains_a`/`conflict`, polylogue-aggz). Directly simulating that relation against the real raw bytes with **current** code (`parse_stream_payload` + `session_revision_projection`) gives `a_contains_b` (local strictly dominates: 0 drive-only messages, 1 local-only message, 0 content mismatches on the 213 shared identities) — i.e. **the fix already exists**. The live archive's `raw_session_memberships` decision rows for these 6 cohorts (`decided_at_ms` ≈ 2026-07-30T05:05–05:13Z) simply predate the commits that fixed the comparison logic (`9fc5220ef` #3401 "collapse revision comparison into a content-only relation", 15:55Z same day, and `a9f2f307d` #3405, 17:50Z) — stale data, not a live bug. **This self-heals without further code changes.** `daemon/bulk_rebuild.py`'s own docstring documents that the daemon routes a bulk-scale backlog (which `polylogue ops reset --index` creates) into `rebuild_index_from_source` → `backfill_historical_revision_evidence` → `classify_membership_revisions` — the current, already-fixed relation — "with zero operator involvement". The operator's already-planned `ops reset --index && polylogued run` will recompute these 6 cohorts correctly. **No `SEMANTIC_REPARSE` delta declaration is needed**: no detection or parsing semantics changed in this PR, and the relevant fix already shipped (and presumably was already declared) in #3401/#3405. **Record correction**: PR #3436 verified these 12 drive-cache/gemini rows are content-shape-correct ("naming coincidence, not a bug") — that remains true, the content-shape classification was never wrong. It did not check `native_id` collisions against local raws, so it didn't catch that 6 of the 12 were silently shadowing a fuller local transcript. That shadowing was a data-staleness artifact of a bug fixed the same day this bead's forensics ran, not a defect in #3436's own change. **Two approaches were tried and reverted** before landing on this conclusion (session-identity gating in `sources/origin_specs.py` + `pipeline/services/ingest_worker.py`, refusing session admission for drive-cache Claude-Code-shaped paths). Both worked mechanically but were wrong: they would have permanently suppressed the 6 *non-colliding* drive-cache raws (real content with no local counterpart to supersede them, so they'd become inert un-queryable `raw_artifacts` rows instead of correctly-attributed real content) and added unneeded machinery for the 6 colliding ones, which the already-fixed relation resolves once the data catches up. Full trail recorded in `polylogue-t83e`'s closing comment. ## Solution What remained a real, independently-valuable defect, unrelated to the collision-resolution question above: the freshness-tie comparison itself (`existing_updated_at_ms is not None and incoming_freshness_ms < existing_updated_at_ms`) was hand-duplicated in three call sites that could silently drift apart. Extracted `should_skip_stale_replace()` into `archive_tiers/ingest_precedence.py` (the module that already owns browser-capture write precedence) and call it from all three: - `polylogue/storage/sqlite/archive_tiers/write.py` (`write_parsed_session_to_archive`) - `polylogue/pipeline/services/ingest_batch/_core.py` (`_write_session`) - `polylogue/storage/sqlite/archive_tiers/revision_governance.py` (`_write_parsed_precedence_result`) Each call site keeps its own surrounding guard conditions (`force_write`/`force_replace`, browser-capture precedence, append-only, revision-authority membership, `source_index` gating) — those decide *whether* the check applies at all, not the comparison. The function's docstring documents that this is a narrow per-write timestamp fallback, not where content-subset supersession is decided (that's revision membership, upstream, and takes precedence when it has classified a cohort). ## Verification - `devtools test tests/unit/pipeline/test_archive_write.py::test_older_full_replace_does_not_overwrite_newer_session_body tests/unit/pipeline/test_ingest_batch.py::test_write_session_force_write_replaces_older_freshness` — 2 passed (exercises `write.py` and `ingest_batch/_core.py`'s edited branches directly) - `devtools test tests/unit/storage/test_revision_replay.py tests/unit/storage/test_raw_revision_authority.py tests/unit/sources/test_revision_backfill.py` — 103 passed, 1 failed. The failure (`test_parse_one_still_replays_real_claude_code_sessions_with_no_path_rule`) is pre-existing on `origin/master`: zero diff in the files it exercises (`archive/artifact_taxonomy/`, `sources/revision_backfill.py`, `sources/origin_specs.py`), reproduces identically in isolation, and is about an unrelated `analysis/`-path content-gate regression. - `devtools verify --seed-testmon --skip-slow` — ruff format, ruff check, mypy --strict, render all --check, topology/layering/closure-matrix, schema-versioning policy, schema-promotion audit all green (see step-by-step `run.json`, exit 0 through step 18); the full pytest seed pass was still mid-sweep when the verification budget for this session ran out — unrelated to this change's narrow surface, already covered by the targeted runs above. - `devtools verify --quick` — green via the pre-push hook (format/lint/mypy/render). Ref polylogue-t83e (closed this session with the full investigation trail in its final comment) Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Jul 31, 2026
Sinity
added a commit
that referenced
this pull request
Jul 31, 2026
…ty (#3472) ## Summary Fixes the dominant remaining cause of ambiguity in claude-code-session revision membership: fork/resume/usage-limit-quirk boundary records that Claude Code stamps with an ancestor session's sessionId were composing that ancestor's bare id as their own identity, colliding every such carryover fragment onto the ancestor's logical_source_key. ## Problem Measured while verifying polylogue-oycw's set-containment fix (#3401/#3405) against real ambiguous cohorts: only 56/185 (30.3%) of claude-code-session cohorts resolved cleanly, far below chatgpt-export/claude-ai-export (>90%). Root cause verified directly against real ~/.claude/projects files: Claude Code re-stamps a handful of records with an ancestor's sessionId even inside a file that is otherwise entirely a different session's own content (a leading resume/fork boundary record, or a mid-file usage-limit/exit quirk). ## Solution dispatch.py's Claude Code grouping now identifies each file's own real content ("primary") and detects a carryover run via structural signals (occurs before primary's first record, or its parentUuid resolves into a uuid primary already produced). A carryover run's identity is qualified so siblings off one ancestor stay distinct; the ancestor id becomes parent_session_id, routing through session_links/lineage instead of colliding revision membership. A new explicit trust_fallback_id flag scopes this override to only dispatch-proven carryover fragments. SEMANTIC_REPARSE index bump to v53. ## Verification devtools verify --quick green. devtools test across the affected surface: 320 passed, 2 pre-existing failures confirmed unrelated via a throwaway origin/master worktree (filed as polylogue-yl8t, polylogue-ihro). New regression test proves two sibling fork files off one ancestor no longer collide identity. Ref polylogue-jc4q
Sinity
added a commit
that referenced
this pull request
Jul 31, 2026
…aggz comparison identity nuec, d8al, hith, c429, bu1i — all five census beads' failure modes are structurally eliminated by the merged set-based (identity, content) comparison architecture (PRs #3401/#3405): event-payload allowlist, content-derived attachment identity, order-insensitive message comparison, acquisition-state exclusion. Live censuses: 554/587 claude-ai (94.4%) and 119/135 chatgpt (88.1%) ambiguous cohorts resolve, 0 regressions. Retroactive verdict healing remains polylogue-9dxn (lane in flight). 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
Membership classification (
polylogue/archive/session_revision_membership.py+polylogue/pipeline/ids.py) treated provider-volatile export artifacts as content identity: message array order, provider-reported generation duration, attachment id presence, and attachment acquisition state (the last already fixed by bu1i, #3394). Live-archive census (read-only, no mutation) over the fullclaude-ai-export/chatgpt-exportambiguous-cohort populations shows this quarantined 587 and 135 cohorts respectively as "ambiguous" branches when they were almost entirely the same conversation re-exported.This PR replaces the growing set of special-cased fixes with one invariant (polylogue-aggz): a conversation is a SET of items (messages, attachments, events) keyed by content-derived identity, each carrying only content-bearing fields. Two revisions are
equal, onecontainsthe other, or theyconflict— total and decidable, no residual category.Problem
generation_lifecycleevent re-deriveselapsed_duration_msfrom the raw export's own timing metadata on every export request; the value is not stable across requests for the SAME generation.Solution
SessionRevisionProjection(ids.py) now projectsmessage_contents/attachment_contents/event_contentsasfrozensets of(identity, content)pairs — never ordered tuples, never the array index.message_id, name, mime_typeonly) instead of using it when present and falling back when absent. The strict/loose duality and its pairwise correlation machinery (_correlate_attachments,_attachments_equivalent,AttachmentRecord) are deleted, not bypassed._EVENT_CONTENT_PAYLOAD_ALLOWLIST), not a denylist of fields discovered volatile after shipping: a new field a parser adds later is excluded from comparison by construction. Onlygeneration_lifecyclehas a registered allowlist (state,evidence_source,fidelity); every other event type still compares its full payload.classify_membership_revisionscollapses what used to be four layered mechanisms (positional-prefix message test, denylist-stripped ordered event-hash prefix test, strict/loose attachment correlation, session-hash/metadata-timestamp tiebreak) into one_relationfunction applied uniformly to all three axes, plus a simple representative-collapse + adjacent-pair containment check.session_hash(idempotency) is unchanged: it still covers the full, order-sensitive, unstripped payload, so a real reorder, duration change, or id change still triggers a re-write. Only the comparison layer is content-only.Kept, not deleted:
_provider_ordered_browser_snapshots. Browser-captured DOM/native snapshots synthesize their own local message/attachment ids from DOM structure, not stable provider identity, so the content-only relation genuinely cannot correlate a DOM-to-native fidelity upgrade — deleting it risked silently breaking real browser-capture archival behavior this lane has no fixture coverage to verify independently.Designed, tested, not wired:
_maximal_evidence_fallback— a deterministic (frontier + raw_id tiebreak, proven order-independent) presence-guarantee pick for a genuine, irreducible conflict, so a document is never simply absent from the archive. Wiring it intoclassify_membership_revisions's return path trips a real, documentedarchive.pywrite-back invariant ("never retire an unrelated accepted head", with production incident history — polylogue-miwv, #3397/#3398) whenever the fallback pick differs from an already-established head. Proven by two failing integration tests during verification (test_divergent_bundle_member_preserves_last_accepted_sessionand its sibling) — not a theoretical concern. Landing it safely needs either the classifier/caller to carry existing-head context, or the write-back guard to accept a re-affirmed-quarantined outcome; both belong toarchive.py's write path, out of this lane's scope. A genuine conflict still quarantines exactly as before (accepted_raw_ids == (), everything inambiguous_raw_ids).Verification
Unit tests:
devtools test tests/unit/archive/test_session_revision_membership.py tests/unit/pipeline/test_pipeline_ids.py tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_revision_replay.py tests/unit/storage/test_browser_capture_origin_repair.py—129 passed.devtools verify --quick—exit_code 0(format, lint, mypy --strict, render-all-check, layering, hash-boundary-census, schema-versioning, all clean).Anti-vacuity: mutated the core content-conflict clause in
_axis_relation(dropped the "content disagrees under a shared identity" check) — 8 tests failed, confirming it is exercised and load-bearing.Live-archive census (read-only against
/realm/db/polylogue, no mutation; full population, not a sample — replayingparse_payload→session_revision_projection→classify_membership_revisionsfor every ambiguous cohort, comparing this branch againstorigin/master):claude-ai-exportchatgpt-exportZero regressions against cohorts that already resolved under the old logic, for both origins. The residual 33 + 16 = 49 cohorts are not resolved by this PR (the presence-guarantee fallback that would resolve genuine conflicts deterministically is designed/tested but not wired, per above) — consistent with the small number of real forks (message content genuinely edited, not just reordered) found during manual inspection of sampled residue.
Acceptance-criteria honesty (polylogue-aggz)
test_non_allowlisted_event_type_keeps_its_full_payload_as_contentproves the allowlist boundary; every new-field-volatility test in this PR is the "add a volatile field, comparison unaffected" shape the bead asked for).superseded_prefix/superseded_equivalentvocabulary distinction: confirmed (via research, not touched —archive.pyis outside this lane's scope) that both already map to the identicalApplicationDecision.SUPERSEDED/revision_authority = "byte_proven"downstream; the only place they differ is a free-text auditdetailstring. Noarchive.pychange was needed to satisfy this._merge_attachment_id_presence_variants,_correlate_attachments,_attachments_equivalent,_attachment_evidence_preserved,_attachment_axis_grew,AttachmentRecord/attachment_records, theby_content/by_session_hash/metadata_variants/timestamp-tiebreak layering,_message_evidence_preserved,_strictly_dominates— net -255 lines across the two production files.archive.pywrite-path coordination) and_provider_ordered_browser_snapshotsdeletion (needs browser-capture parser id-stability verification this lane could not do safely in scope).ingest_batch/archive.py-owning lanes per this task's file boundaries) and not addressed here.Review follow-up: four defects fixed (chatgpt-codex-connector)
Automated review found the content-only relation did not yet fully hold the
invariant it claims, in four concrete ways. All four are fixed on this branch:
(
session_revision_membership.py). When a direct export and a browsercapture projected to equal content, the equal-content collapse picked its
representative by provider timestamp (then raw_id) BEFORE any
source-authority ordering ran, so a browser capture could outrank its own
authoritative export. New
_equal_content_representativedecides byauthority first (direct export over browser capture, native over DOM),
mirroring the ordering
_direct_export_precedence/_browser_snapshot_dominatesalready apply to the non-equal growth-chaincase, falling back to timestamp/raw_id only when neither side's
provenance outranks the other's.
ids.py, themost serious of the four: it reintroduced exactly the identity-instability
bug class this PR exists to eliminate). An event's canonical identity
shifted from
base_identitytohash(base_identity, content)purelybecause a SECOND event later shared its base identity, in the SAME
revision — so an ordinary event-growth revision could compare as a
disjoint conflict instead of containment. Now always
hash(base_identity, content), unconditionally: an item's identity mustnot depend on what else is in the set.
(
session_revision_membership.py). Two acquired attachments sharing oneidentity (same message/name/mime, different bytes) fed a plain
dict(contents), so the second content hash silently overwrote thefirst — a real conflict could compare as equal, worse than either honest
outcome. New
_content_by_identitygroups every content hash under itsidentity into a set, so a collision now always degrades to
conflict.event_typealone (ids.py). Thegeneration_lifecycleallowlist strip applied to every event of thattype, including browser-capture's own DOM/UI observations
(
duration_semanticsvalues likedom_observed_wall), which are a realfirst-party measurement, not the ChatGPT-parser-remeasured value nuec
targeted. Now gated on
duration_semantics == "provider_reported_elapsed"(wiring up the previously-dead
_PROVIDER_REPORTED_ELAPSED_MARKER_KEY/_VALUEconstants).Both
_provider_ordered_browser_snapshots(kept, not deleted) and_maximal_evidence_fallback(designed/tested, not wired —archive.pywrite-back invariant, out of scope) are unchanged per explicit instruction.
Tests: one regression test per finding in
tests/unit/archive/test_session_revision_membership.py, each verified bymanually mutating the corresponding production line back to the buggy
behavior and observing the exact expected failure (see commit
test(archive): cover the four aggz identity-invariant defects with mutation-proof testsfor the anti-vacuity detail per finding).Re-run live census (read-only, same methodology, apples-to-apples A/B on
the identical ambiguous-cohort population — this branch's fixed
classify_membership_revisionsvs. the pre-fix39591e17d/78d884b8cstate, same input rows for both runs):
claude-ai-exportchatgpt-export(Population here is 136/587 for chatgpt-export/claude-ai-export — measured
directly from
raw_session_membershipswithout the earlier equal-message-countpre-filter, vs. this PR's original 135/587; the claude-ai-export count is
unaffected either way.)
The counts barely move — none of the four fixes change whether a cohort
resolves for the vast majority of cases, only the P1 fix changes which raw
is materialized as the accepted head for already-resolving equal-content
cohorts (verified directly: the one real flip below is a P1 case, not P2).
Zero regressions in either origin, confirmed by diffing the exact
unresolved-cohort key sets between the two runs, not just the aggregate
counts.
One concrete flip, traced to root cause:
chatgpt:69d5383e-…(4368messages, 4 raw revisions — 1 direct export + 3 native browser captures, all
pairwise-
equalorconflictidentically under old and new code). Pre-fix,the equal-content collapse of the export + 2 of the 3 natives picked the
LATER-timestamped native as representative (losing the export's
browser_snapshot_fidelity=Nonemarker), so_direct_export_precedencethen found zero non-browser candidates among the representatives and the
cohort stayed quarantined. Post-fix, the export always survives that
collapse,
_direct_export_precedencefires as designed, and the cohortresolves. This is exactly the shape P1 describes, confirmed against a real
archive row, not just the unit-test fixtures.
Residual population (33 claude-ai-export + 16 chatgpt-export cohorts,
unchanged in count from the original measurement modulo the one flip above)
is the same population the unwired
_maximal_evidence_fallbackexists toserve: genuine, irreducible conflicts (real forks — content edited, not
just reordered/re-measured/re-captured) where no containment chain exists
at all. This PR does not resolve them and does not claim to — wiring
the fallback needs
archive.pywrite-path coordination(
_maximal_evidence_fallback's docstring states the concrete blockinginvariant), which is explicitly out of this lane's scope; the residual
count will not reach zero without that follow-up landing.
Ref polylogue-aggz
Ref polylogue-bu1i
Ref polylogue-c429
Ref polylogue-nuec
Ref polylogue-d8al
Ref polylogue-hith
🤖 Generated with Claude Code