perf(sources): parse byte-identical raw rows once per (blob_hash, source_path)#3151
Merged
Conversation
…rce_path) Problem: 17% of the live archive's newest-only bytes (8.7GB of 52.1GB) are byte-identical duplicate blobs stored under multiple raw rows — e.g. one 442MB codex rollout acquired 8 times within 2.3 seconds during an acquisition stampede (8 raw rows, same blob_hash, same source_path) — and census parsed every row independently, paying the full decode for identical bytes each time. The blob store already dedups storage by hash; the parse layer did not. What changed: _parse_retained_raws groups its batch by (blob_hash, source_path), parses one representative per group through the existing sequential/pool machinery (_parse_unique_retained_raws), and fans the outcome out to duplicate rows with each row's own revision_kind re-attached from its descriptor. source_path stays in the dedup key deliberately: some parsers derive identity from the path (e.g. beads workspace-scoped native ids), so cross-path duplicates of the same bytes still parse separately. Exceptions fan out to all rows of the failed group. Verification: devtools test tests/unit/sources/test_revision_backfill.py (29 passed). New tests: dedup grouping with per-row revision_kind fan-out + cross-path isolation, and exception fan-out. Anti-vacuity via patch-revert (not stash — refs/stash is shared across worktrees): both fail on pre-fix code. devtools verify --quick exit 0. Current write_raw_payload is idempotent for identical (payload, path) — verified by probe — so new stampedes cannot recreate the shape; this covers the historical rows. Ref polylogue-869u Co-Authored-By: Claude <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 13 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 Run ID: 📒 Files selected for processing (2)
✨ Finishing Touches🧪 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 |
Sinity
added a commit
that referenced
this pull request
Jul 21, 2026
… by blob_hash (#3234) ## Summary Two fixes in the revision-authority census area, sharing the raw-materialization footprint: - **polylogue-52l2**: stop an isolated later-discovered raw from silently becoming the permanently accepted content for a logical identity that already has known ambiguous/quarantined siblings. - **polylogue-869u**: dedup census parse by `blob_hash` across `source_path` for providers whose parse identity is byte-content-only, avoiding redundant full parses of byte-identical duplicate blobs. ## Problem ### polylogue-52l2 `classify_raw_revision_cohort` classifies a byte-prefix chain from whichever `raw_sessions` rows currently carry `revision_kind='full'` for a `logical_source_key` — not against the complete sibling population for that identity. Retiring an ambiguous sibling to membership governance (`replace_raw_membership_census(..., retire_full_revision_governance=True)`, what the backfill/live-watcher callers do once a cohort is decided ambiguous) nulls that raw's `raw_sessions.logical_source_key`, so it disappears from the query. A THIRD raw for the same identity, discovered afterward (e.g. a re-acquired browser-capture snapshot), is then evaluated completely alone: `classify_historical_full_revision_streams` unconditionally accepts a one-member stream as a byte-proven baseline (no sibling to prove a byte prefix against), so the isolated raw permanently becomes the accepted session content — an outcome that depends on incremental discovery order, not on which content is actually correct. Reproduced directly against `ArchiveStore` (`tests/unit/storage/test_revision_replay.py`), mirroring the live incremental watcher's own call sequence (`sources/live/batch.py`: `bind_raw_revision` then `classify_raw_revision_cohort`, no census-phase re-derivation in between). The equivalent two-call scenario through `backfill_historical_revision_evidence` does NOT reproduce: its census phase unconditionally re-parses every still-unindexed raw and its connected-component selection expansion reunites retired siblings by shared membership evidence before classification runs — protections the live incremental path lacks. This class of bug is closely related to (but distinct from) the head-collision fixes in #3204/#3205/#3211 that landed in the last 48h; those addressed head-handoff precedence and cohort absorption for an *already-established* head, not this isolated-singleton acceptance path. ### polylogue-869u Live evidence (2026-07-19, `source.db`): the newest-revision-per-`logical_source_key` population is 87,177 rows / 52.1 GiB, but only 85,066 DISTINCT `blob_hash` values / 43.4 GiB — the same bytes (e.g. one 442MB codex computer-use rollout) recur under up to 8 different `logical_source_key`s / source paths, from re-acquisition or re-export. The existing dedup (#3151, `_parse_retained_raws`) only collapses rows sharing BOTH `blob_hash` AND `source_path`, so this whole cross-path duplicate population — 8.7GB / 17% of newest-only bytes — still paid a full parse per row. ## Solution ### polylogue-52l2 (`polylogue/storage/sqlite/archive_tiers/archive.py`, `polylogue/sources/revision_backfill.py`, `polylogue/sources/live/batch.py`, `polylogue/archive/revision_authority.py`) - New `ArchiveStore.raw_membership_retired_full_revision_siblings()`: finds `raw_session_memberships` rows for a `logical_source_key` whose `raw_membership_census.detail` matches the (now shared) `HISTORICAL_NON_PREFIX_GOVERNANCE_DETAIL` marker written at retirement — survives the `raw_sessions.logical_source_key` NULL-out. - `classify_raw_revision_cohort` refuses the byte-chain path entirely whenever this identity has retired sibling evidence, so the caller's existing "no accepted chain" fallback (`convertible_full_revision_raw_ids`) folds the newly-discovered raw into membership governance instead, where the real prefix-based classifier weighs every known sibling together, rather than silently establishing a wrong permanent head. - Unified the two `retire_full_revision_governance` call sites' previously-divergent detail strings onto one shared constant so the guard recognizes retirement from either path. - **Deferred** (named, not hidden): once an identity's retired siblings are recognized, nothing in the LIVE incremental watcher path (unlike offline backfill) currently re-unites them with the new raw for a real membership decision — it fails closed instead (logs a warning, surfaces as failed), correctly never establishing wrong content, but requiring a later offline backfill/repair pass to resolve the identity fully. Also out of scope: PR #3204 already tracks a known, separate limitation (a content-ahead capture yielding to a byte-kind chain head) as polylogue-nfl5. ### polylogue-869u (`polylogue/sources/revision_backfill.py`) `_parse_retained_raws`' grouping key now drops `source_path` for a new `_PATH_INDEPENDENT_PARSE_PROVIDERS` allowlist (ChatGPT, Claude web/Code, Codex, Gemini/Gemini CLI, Grok, Drive) — those parsers derive session identity purely from payload bytes. Providers whose parse DOES depend on `source_path` keep the original `(blob_hash, source_path)` key: Beads derives workspace-scoped native ids from `source_path`, Antigravity's brain-metadata mode and Hermes' ATOF/ATIF/verification-evidence modes derive `profile_root` from `source_path`. `Provider.UNKNOWN` stays path-scoped out of caution. ## Verification - `devtools test tests/unit/storage/test_revision_replay.py tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_raw_retention.py tests/property/test_sql_injection_boundary.py` → 160 passed. - New regression `test_isolated_later_raw_does_not_override_known_ambiguous_cohort` fails on unmodified master (asserts `accepted_raw_ids == ()` but gets the isolated raw's id) and passes after the 52l2 fix. - `mypy --strict` clean on all four touched modules. - `devtools verify --quick` → exit 0. - **Dedup receipt (869u)**: synthetic corpus of 40 distinct ~300KB Codex payloads, each duplicated across 5 different source paths (200 raw rows, mirroring the live re-acquisition-stampede shape), measured via real `_parse_retained_raw` invocation counts: - before (`blob_hash`+`source_path` key): 200 parse calls, 0.516s wall clock - after (`blob_hash` key, safe providers): 40 parse calls, 0.104s wall clock - 160 avoided parse calls (80% reduction on this corpus). Note: the task brief also asked to run `tests/unit/storage/test_no_string_interpolated_sql.py` explicitly after any `archive.py` edit. That file does not exist in this repo (grepped for "interpolated" and "hardcoded.*line" repo-wide, no match) — ran `tests/property/test_sql_injection_boundary.py` as the closest existing analog instead (40 passed). ## AC matrix **polylogue-52l2** - Isolated-singleton acceptance over a known-ambiguous cohort: **satisfied** — guarded in `classify_raw_revision_cohort`, proven by a failing-then-passing regression test. - Cross-tick reunification of retired siblings with a new raw in the LIVE incremental path: **deferred** — currently fails closed (no wrong content, but no automatic resolution either); needs a follow-up to fold retired-but-known siblings back into a real membership decision at the live-watcher call site, analogous to what offline backfill already does via `convertible_full_revision_raw_ids`. - Direct-seed order-reversal for `source_outage_interval_events`/`capture_gap_events` (bead's "related, separately-confirmed finding"): **not addressed** — out of this PR's footprint (`_write_parsed_precedence_result` in the excluded `archive_tiers/write.py`); still needs its own investigation pass as the bead notes. **polylogue-869u** - Byte-identical blobs parsed at most once per parser fingerprint, per identity-relevant path scope: **satisfied** for the `_PATH_INDEPENDENT_PARSE_PROVIDERS` allowlist; path-dependent providers (Beads/Antigravity/Hermes/Unknown) intentionally keep path-scoped dedup. - Measured corpus receipt with parse-call counts and wall-clock before/after: **satisfied** — see Verification above. - No identity regression for source_path-dependent parsers, test covers the Beads workspace case: **satisfied** — `test_parse_retained_raws_preserves_path_scoped_dedup_for_path_dependent_providers`. Ref polylogue-52l2, polylogue-869u Co-Authored-By: Claude <noreply@anthropic.com> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Improved handling of historical revision governance during archive classification and replay. * Enhanced retained-data processing to safely deduplicate identical content across paths where supported. * **Bug Fixes** * Prevented later isolated revisions from overriding previously identified ambiguous revision groups. * Preserved path-specific handling for providers whose results depend on source paths. * **Tests** * Added coverage for cross-path deduplication and ambiguous revision classification scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- 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
_parse_retained_rawsnow groups its batch by(blob_hash, source_path), parses one representative per group, and fans the outcome out to duplicate rows (with each row's ownrevision_kindre-attached). Byte-identical raw rows — 17% of the live archive's newest-only bytes — stop paying repeated full parses.Problem
Live evidence (2026-07-19, source.db): newest-revision-per-logical-key = 87,177 rows / 52.1GB, but only 85,066 distinct
blob_hashes / 43.4GB — 8.7GB of parse work is byte-identical duplicates. Worst case: one 442MB codex rollout acquired 8 times within 2.3 seconds (acquisition stampede; 8 raw rows, same blob, same path), each row fully parsed by census. The blob store dedups storage by hash; the parse layer did not.Solution
_parse_retained_rawsbecomes a dedup wrapper: group by(blob_hash, source_path), delegate representatives to_parse_unique_retained_raws(the previous body, unchanged sequential/pool logic including the perf(sources): sequential parse below aggregate pool floor, stop worker churn #3149 amortization floor), fan out outcomes.source_pathstays in the key deliberately — some parsers derive identity from the path (beads workspace-scoped native ids), so cross-path duplicates of the same bytes still parse separately. Exceptions fan out to every row of the failed group.write_raw_payloadis idempotent for identical (payload, path) — verified by probe — so new stampedes can't recreate the shape; this covers the historical rows and any future writer regression.Verification
devtools test tests/unit/sources/test_revision_backfill.py— 29 passed.test_parse_retained_raws_dedupes_identical_blob_and_path(parse-call sequence is exactly one per distinct (blob, path); duplicate rows get their ownrevision_kind; different path with same bytes still parses) andtest_parse_retained_raws_fans_out_exceptions_to_duplicate_rows. Anti-vacuity via patch-revert of the implementation file (notgit stash—refs/stashis shared across worktrees): both fail on pre-fix code (2 failed).devtools verify --quick— exit 0.Ref polylogue-869u
🤖 Generated with Claude Code
https://claude.ai/code/session_01QUsH3Rhq6oAZpYPWcsZqnZ