fix(capture): recover replaced browser snapshots#2930
Conversation
Problem: browser capture paths remained excluded forever after five transient failures, even when the extension atomically replaced them with complete files. Provider-native snapshot progress was also quarantined when stable messages reordered or attachments were acquired later.\n\nClear exclusion only when the observed file identity changes. Classify mutable browser snapshots with fidelity, provider freshness, stable message and attachment identities, and acquisition order for attachment-only enrichment while retaining ambiguity for lost identities and divergent branches.\n\nRef polylogue-3v1.\n\nCo-Authored-By: Claude <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 40 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 (5)
📝 WalkthroughWalkthroughThe change adds browser snapshot fidelity and provider identity metadata to membership revisions, orders compatible snapshots by provider authority, enriches live ingestion records, exposes raw acquisition timestamps, and revives excluded cursors when files are replaced. ChangesBrowser snapshot membership
Watcher exclusion recovery
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant LiveBatchProcessor
participant classify_membership_revisions
participant ArchiveStore
LiveBatchProcessor->>classify_membership_revisions: submit enriched membership revisions
classify_membership_revisions->>classify_membership_revisions: order compatible browser snapshots
classify_membership_revisions-->>LiveBatchProcessor: return strict chain and equivalents
LiveBatchProcessor->>ArchiveStore: apply raw session membership decisions
ArchiveStore-->>LiveBatchProcessor: return stored membership state
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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5f0b7e11c
ℹ️ 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".
| if ( | ||
| cursor.byte_size, | ||
| cursor.st_dev, | ||
| cursor.st_ino, | ||
| cursor.mtime_ns, | ||
| ) == (size, stat.st_dev, stat.st_ino, stat.st_mtime_ns): |
There was a problem hiding this comment.
Bind quarantine to the failed file observation
When a previously successful source is replaced by malformed content, _record_failed_cursor deliberately preserves the last accepted cursor observation while incrementing failures. After the fifth failure sets excluded, this comparison therefore sees the still-malformed file as a replacement on the next watcher scan, clears the exclusion and resets failure_count to zero. The same poison file then enters another retry cycle instead of remaining quarantined; store the failed stat observation separately or update it when exclusion is reached.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 3340459. Quarantine now records the exact failed stat observation only when the retry budget is exhausted; ordinary backoff still preserves the last accepted cursor. Added a regression covering an accepted file replaced by unchanged malformed content.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/session_revision_membership.py`:
- Around line 117-144: Update the DOM-to-native branch in
_browser_snapshot_dominates to compare the newer and older frontiers
elementwise, requiring every frontier dimension to be greater than or equal. Do
not use tuple lexicographic comparison, so event or attachment regressions
cannot pass.
🪄 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: c1742cb6-cbc0-4e9a-b3b1-d58a5b305b1c
📒 Files selected for processing (8)
polylogue/archive/session_revision_membership.pypolylogue/sources/live/batch.pypolylogue/sources/live/cursor.pypolylogue/sources/live/watcher.pypolylogue/storage/sqlite/archive_tiers/archive.pytests/unit/archive/test_session_revision_membership.pytests/unit/sources/test_live_batch_support.pytests/unit/sources/test_live_watcher.py
A replaced malformed source inherited the prior successful cursor stat, so exclusion appeared stale immediately and the watcher revived unchanged poison. Record the failed file observation when the retry budget is exhausted while preserving accepted cursor authority during ordinary backoff. Co-Authored-By: Claude <noreply@anthropic.com>
A lexicographic DOM-to-native frontier comparison could accept a snapshot that gained messages while losing events or attachments. Require every frontier dimension to remain monotonic and cover the shrinking-attachment case. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
Recover browser captures after transient parse quarantine and accept later provider-authoritative browser snapshots without weakening divergent-branch protection.
Problem
The 27-session Sol Pro production census found current extension files parsing to 2,551 messages while the index exposed 205. Twenty-two cursor paths were excluded after five transient failures and never reconsidered after atomic replacement. Separately, mutable provider-native snapshots were quarantined because stable context/tool messages can reorder and output attachments can become available without serialized strict-prefix growth.
Solution
Ref polylogue-3v1.
Verification
devtools test tests/unit/archive/test_session_revision_membership.py tests/unit/sources/test_live_batch_support.py::test_browser_capture_replacement_advances_membership_head_and_acquires_attachment tests/unit/sources/test_live_batch_support.py::test_browser_capture_provider_timestamp_advances_reordered_native_snapshot tests/unit/sources/test_live_watcher.py::test_cursor_mark_failed_quarantines_repeated_failures tests/unit/sources/test_live_watcher.py::test_replaced_excluded_file_is_revived_without_retrying_unchanged_poisondevtools verify --quick20260716T090428Z-quick-53371-e8ba2806devtools verify --quick20260716T090710Z-quick-56296-18a04001Summary by CodeRabbit
New Features
Bug Fixes