Fix batch rewind archival in watcher#574
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Your trial has ended. Reactivate Greptile to resume code reviews.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_f39eac7e-d8f5-4847-97c8-63944b625819) |
|
Warning Review limit reached
Next review available in: 48 seconds 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: Organization UI 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 |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| flush_interval_ms=rewind_archive_interval_ms, | ||
| ) | ||
|
|
||
| def on_rewind(filepath: str, session_id: str, old_offset: int, new_offset: int): |
There was a problem hiding this comment.
🟠 High cli/__init__.py:3361
Buffering rewinds by session_id causes the delayed batch flush in _RewindArchiveBatcher.flush() to archive all rows with that conversation_id and archived_at IS NULL. Because JSONLWatcher.poll_once() continues processing new_lines from the restored timeline immediately after calling on_rewind, any replacement chunks inserted before the batch flush will also be soft-archived, wiping out the new timeline instead of only the reverted pre-rewind content. Consider recording a cutoff (e.g. old_offset or timestamp) at rewind time and limiting the archival SQL to chunks created before that cutoff, rather than archiving by conversation_id alone.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/brainlayer/cli/__init__.py around line 3361:
Buffering rewinds by `session_id` causes the delayed batch flush in `_RewindArchiveBatcher.flush()` to archive *all* rows with that `conversation_id` and `archived_at IS NULL`. Because `JSONLWatcher.poll_once()` continues processing `new_lines` from the restored timeline immediately after calling `on_rewind`, any replacement chunks inserted before the batch flush will also be soft-archived, wiping out the new timeline instead of only the reverted pre-rewind content. Consider recording a cutoff (e.g. `old_offset` or timestamp) at rewind time and limiting the archival SQL to chunks created before that cutoff, rather than archiving by `conversation_id` alone.
| self._last_flush_at = time.perf_counter() | ||
| self.archived_total = 0 | ||
|
|
||
| def _get_vector_store(self): |
There was a problem hiding this comment.
🟠 High cli/__init__.py:3221
_RewindArchiveBatcher._get_vector_store() instantiates a bare VectorStore, which acquires the global writer pidfile. The watch command sets BRAINLAYER_ARBITRATED=1 and advertises that the drain process owns DB writes, so when the drain daemon is running the first flush() call raises WriterInUseError instead of archiving reverted chunks. Every rewind archival attempt therefore fails and reverted chunks stay live. Consider routing archival through the same arbitrated write path the rest of the watcher uses, or document why direct writer acquisition is intentional here.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/brainlayer/cli/__init__.py around line 3221:
`_RewindArchiveBatcher._get_vector_store()` instantiates a bare `VectorStore`, which acquires the global writer pidfile. The `watch` command sets `BRAINLAYER_ARBITRATED=1` and advertises that the drain process owns DB writes, so when the drain daemon is running the first `flush()` call raises `WriterInUseError` instead of archiving reverted chunks. Every rewind archival attempt therefore fails and reverted chunks stay live. Consider routing archival through the same arbitrated write path the rest of the watcher uses, or document why direct writer acquisition is intentional here.
| self._last_flush_at = time.perf_counter() | ||
| return affected | ||
|
|
||
| def maybe_flush(self, reason: str) -> int: |
There was a problem hiding this comment.
🟡 Medium cli/__init__.py:3269
maybe_flush() is only invoked from the rewind callback, so when a single rewind is followed by normal watcher traffic with no further rewinds, the session id stays in _pending_session_ids indefinitely. The reverted realtime_watcher chunks remain unarchived until shutdown or a later rewind, so search keeps returning stale rolled-back content for the rest of the run. Consider calling maybe_flush() on a periodic tick (e.g., the watcher poll loop) so the flush_interval_ms timer is actually enforced.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @src/brainlayer/cli/__init__.py around line 3269:
`maybe_flush()` is only invoked from the rewind callback, so when a single rewind is followed by normal watcher traffic with no further rewinds, the session id stays in `_pending_session_ids` indefinitely. The reverted `realtime_watcher` chunks remain unarchived until shutdown or a later rewind, so search keeps returning stale rolled-back content for the rest of the run. Consider calling `maybe_flush()` on a periodic tick (e.g., the watcher poll loop) so the `flush_interval_ms` timer is actually enforced.
1 participant
Summary
Note
Medium Risk
Changes how the live watcher writes to the chunk store on rewind; behavior should match prior soft-archive rules, but delayed flushes mean archival is no longer immediate per rewind until batch/interval/shutdown triggers.
Overview
Rewind archival in
brainlayer watchis batched instead of one DB round-trip per session.When a JSONL checkpoint rewind is detected, the watcher no longer opens
VectorStoreand runs a per-sessionUPDATEimmediately. It queues session IDs in_RewindArchiveBatcher, which issues a single batchedUPDATE(withconversation_id IN (...)) forrealtime_watcherchunks that are not yet archived, reusing one store connection until shutdown.Flushes run when the pending session count hits a threshold (default 50, overridable via
BRAINLAYER_REWIND_ARCHIVE_BATCH), after a time interval (defaultflush_interval * 3, overridable viaBRAINLAYER_REWIND_ARCHIVE_INTERVAL_MS), and always on watcher exit in afinallyblock. Thearchived_at IS NULLguard keeps repeat rewinds idempotent.New tests in
test_rewind_batch_archival.pycover multi-session batching, threshold and interval flushes, shutdown flush, and idempotency.Reviewed by Cursor Bugbot for commit 7cf7fcc. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Fix batch rewind archival in the watcher by coalescing session updates into a single DB write
_RewindArchiveBatcherinbrainlayer/cli/__init__.pyto buffer session IDs and flush them in a singleUPDATEagainst the chunks table, rather than one DB write per rewind event.try/finally.BRAINLAYER_REWIND_ARCHIVE_BATCHandBRAINLAYER_REWIND_ARCHIVE_INTERVAL_MSenvironment variables, with fallback defaults and warnings on invalid values.tests/test_rewind_batch_archival.pycovering threshold flush, interval flush, idempotency, and shutdown flush.on_rewindhandler no longer writes to the DB immediately; archival is deferred until a flush condition is met.📊 Macroscope summarized 7cf7fcc. 1 file reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted
🗂️ Filtered Issues
No issues evaluated.