fix(storage): stream safe raw authority replay#2966
Conversation
Problem: Raw replay classified and parsed retained JSONL through read_all(), so a single oversized but otherwise safe authority component could never reach the bounded executor. What changed: Add chunked full-snapshot prefix proof and retained-raw stream access. Stream-record providers now parse directly from blob handles, and admission permits an oversized component only when every member is on that route. Full/append fold verification and metadata-only application steps no longer materialize raw blobs. Other providers remain fail-closed behind the existing resource envelope. Verification: devtools test tests/unit/storage/test_raw_revision_authority.py \ tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_repair.py 79 passed. devtools verify --quick: ruff, mypy, generated surfaces, layering, schema roundtrip, manifests, and CI workflow checks passed. Ref polylogue-hjpx.2.
|
Warning Review limit reached
Next review available in: 46 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 (8)
📝 WalkthroughWalkthroughThe change introduces streamed raw revision descriptors and classification, adds incremental archive digest and segment verification, and updates backfill and repair resource gates to recognize stream-safe oversized payloads. Tests validate streamed classification, non-eager backfill, and repair behavior. ChangesStreaming raw revision processing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Backfill
participant ArchiveStore
participant ArchiveBlobPublisher
participant StreamParser
participant Repair
Backfill->>ArchiveStore: request raw_revision_descriptor
ArchiveStore->>ArchiveBlobPublisher: open_raw_revision_material
ArchiveBlobPublisher-->>StreamParser: provide BinaryIO payload
StreamParser-->>Backfill: return retained kind and payload size
Repair->>ArchiveStore: check raw stream safety
Repair-->>Repair: allow or block oversized component
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![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: 6424723e75
ℹ️ 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".
| stream_safe = all(_retained_raw_is_stream_safe(archive, raw_id) for raw_id in payload_sizes) | ||
| if (oversized or total_payload_bytes > max_payload_bytes) and not stream_safe: |
There was a problem hiding this comment.
When an authority component consists entirely of Codex/Claude JSONL raws, this condition now admits it regardless of the 1 GiB component limit, but only the raw-byte reader is streaming: parse_stream_payload still builds complete ParsedSession message trees, _ParsedSessionSpill.add() serializes each tree, and for_logical_key() uses fetchall() before unpickling the entire cohort. A multi-GiB JSONL session or aggregate cohort can therefore consume multiple GiB of RAM and OOM the daemon; stream-safe admission needs a bound on the materialized parser/spill representation rather than bypassing the component limit.
Useful? React with 👍 / 👎.
| for parent in revisions | ||
| if parent.raw_id != child.raw_id | ||
| and _stream_is_prefix( | ||
| parent, | ||
| child, |
There was a problem hiding this comment.
For a logical source with multiple increasing full snapshots, these nested loops invoke _stream_is_prefix for every smaller/larger pair, and each invocation reopens both blobs and rereads the complete parent prefix. Thus a cohort of n large revisions changes from one disk read per retained blob to quadratic blob I/O—for example, 20 roughly 1 GiB snapshots require about 190 GiB of prefix reads in addition to the initial size scans—making the newly supported large-revision replay liable to stall for hours. The prefix chain should be established without repeatedly streaming each blob for all pair combinations.
Useful? React with 👍 / 👎.
Problem: Streaming retained bytes does not bound the parser's complete ParsedSession construction. Admitting arbitrary multi-GiB record streams could still exceed the daemon resource envelope before the spill cache is reached. What changed: Retain the existing component-size admission gate while preserving streamed parsing for admitted JSONL revisions. Expand streamed/eager authority tests across all ambiguous prefix cohorts. Verification: devtools test tests/unit/storage/test_raw_revision_authority.py \ tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_repair.py 82 passed. Ref polylogue-hjpx.2.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 188-208: Update classify_historical_full_revision_streams to avoid
comparing every ordered revision pair: sort revisions by actual size, reject
duplicate sizes according to the existing decision/error behavior, and evaluate
only adjacent sorted revisions with _stream_is_prefix. Preserve the unique
proper-prefix chain classification while ensuring each adjacent pair matches,
rather than building prefix_pairs from all revision combinations.
In `@polylogue/sources/revision_backfill.py`:
- Around line 183-190: Update the revision backfill budget logic around
_retained_raw_is_stream_safe so total_payload_bytes and oversized include only
non-stream-safe raw members, while preserving blocking when those eager bytes
exceed the limit; apply the same aggregate filtering to the component-member
budget check in polylogue/sources/revision_backfill.py lines 183-190 and
polylogue/storage/repair.py lines 5623-5629.
🪄 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
Run ID: 2764934d-3d64-47af-9f3f-670e7a211e08
📒 Files selected for processing (7)
polylogue/archive/revision_authority.pypolylogue/sources/revision_backfill.pypolylogue/storage/repair.pypolylogue/storage/sqlite/archive_tiers/archive.pytests/unit/sources/test_revision_backfill.pytests/unit/storage/test_raw_revision_authority.pytests/unit/storage/test_repair.py
| payload_sizes = archive.raw_payload_sizes([raw_id for raw_id, _index in rows]) | ||
| total_payload_bytes = sum(payload_sizes.values()) | ||
| oversized = [raw_id for raw_id, size in payload_sizes.items() if size > max_payload_bytes] | ||
| if oversized or total_payload_bytes > max_payload_bytes: | ||
| stream_safe = all(_retained_raw_is_stream_safe(archive, raw_id) for raw_id in payload_sizes) | ||
| if (oversized or total_payload_bytes > max_payload_bytes) and not stream_safe: | ||
| blocked_ids = oversized or list(payload_sizes) | ||
| raise RawRevisionReplayResourceBlockedError( | ||
| sorted(blocked_ids), max_payload_bytes, total_payload_bytes |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Exclude streamed bytes from mixed-component eager budgets.
Both gates treat stream safety as all-or-nothing, so one eager member causes every streamed member’s bytes to count against the materialization limit.
polylogue/sources/revision_backfill.py#L183-L190: calculatetotal_payload_bytesandoversizedfrom non-stream-safe raws only.polylogue/storage/repair.py#L5623-L5629: block only when the aggregate size of non-stream-safe component members exceeds the limit.
📍 Affects 2 files
polylogue/sources/revision_backfill.py#L183-L190(this comment)polylogue/storage/repair.py#L5623-L5629
🤖 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/sources/revision_backfill.py` around lines 183 - 190, Update the
revision backfill budget logic around _retained_raw_is_stream_safe so
total_payload_bytes and oversized include only non-stream-safe raw members,
while preserving blocking when those eager bytes exceed the limit; apply the
same aggregate filtering to the component-member budget check in
polylogue/sources/revision_backfill.py lines 183-190 and
polylogue/storage/repair.py lines 5623-5629.
Problem: live append and full replay still reopened retained JSONL raw blobs through the eager materialization helper, bypassing the streaming authority path. The streamed classifier also compared every pair of historical revisions. What changed: centralize retained-raw parsing in the source layer; JSONL records now use the blob stream in historical, append, and full replay. Reduce the prefix-chain proof to sorted adjacent comparisons, which is equivalent for a unique strict-prefix chain. Verification: devtools test tests/unit/storage/test_raw_revision_authority.py tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_repair.py tests/unit/sources/test_live_batch_support.py (150 passed); devtools verify --quick.
|
Review triage after the follow-up commits:
|
1 participant
Summary
Streams admitted retained JSONL revisions through raw-authority census and replay while preserving byte-prefix authority without loading full blobs into memory.
Problem
The replay path used
read_all()for record streams despite blob/parser streaming support. It inflated peak RSS beneath the existing component resource envelope. Ref polylogue-hjpx.2.Solution
Verification
devtools test tests/unit/storage/test_raw_revision_authority.py tests/unit/sources/test_revision_backfill.py tests/unit/storage/test_repair.py— 82 passed.devtools verify --quick— all 15 static/generated gates passed.Adversarial review notes
Iteration 1 found a real gap: the first revision admitted arbitrary oversized streams even though parser output is still materialized. Commit
8be68f9dfrestores the hard admission limit; production-shape parser checkpointing remains explicitly owned by polylogue-hjpx.2.What I decided not to change
Non-stream exports/bundles and oversized record streams remain resource-blocked. They need parser-output checkpointing plus a measured production-shape envelope, not a weaker admission rule. Authority components remain atomic because partial execution would invalidate immutable plan and conservation receipts.