Skip to content

fix: make watcher rewind archival APSW-safe#595

Open
EtanHey wants to merge 1 commit into
mainfrom
fix/brainbar-truth-rewind
Open

fix: make watcher rewind archival APSW-safe#595
EtanHey wants to merge 1 commit into
mainfrom
fix/brainbar-truth-rewind

Conversation

@EtanHey

@EtanHey EtanHey commented Jul 14, 2026

Copy link
Copy Markdown
Owner

Summary

  • make watcher rewind archival own an explicit APSW BEGIN IMMEDIATE / COMMIT transaction with guarded rollback
  • add real APSW durability, rollback, telemetry, pending-session, and retry regression coverage
  • lock BrainBar's existing naive-local timestamp normalization with a deterministic non-UTC fixture

Truth investigation

  • the ad-hoc created_at >= datetime('now', '-10 minutes') count was not valid for mixed ISO timestamp formats because it compared timestamp strings lexically
  • current BrainBar source already normalizes timestamp epochs and uses watcher_liveness_events for watcher activity; the installed BrainBar 1.4.2 build at a2fd98d1 predates those source truth fixes
  • current source distinguishes actual store-flush queue depth from fallback replay debt, so replay debt is not presented as an active queued store lane

Verification

  • pytest: 3,622 passed, 60 skipped, 5 xfailed
  • swift test: 751 passed, 2 skipped
  • pre-push clean-environment gate: 3,530 unit tests passed, plus MCP registration, 40 isolated routing/eval tests, Bun fixture, and FTS5 regression shell
  • focused rewind/telemetry suite: 9 passed; Ruff and git diff --check clean
  • local CodeRabbit CLI was rate-limited; independent red-team and blue-team fallback reviews passed after all findings were fixed

Deploy / integration QA

  • restart com.brainlayer.watch after deploying the Python rewind fix
  • rebuild/install and restart BrainBar so the currently installed stale 1.4.2 app picks up the source truth fixes
  • visual verification was not performed; orchestrator local integration QA remains required

This PR must remain OPEN. The orchestrator owns integration and merge timing.


Note

Medium Risk
The change mutates realtime_watcher chunk archival on rewind in production, but the diff is small, transaction-scoped, and backed by new APSW durability and rollback tests; BrainBar changes are test-only.

Overview
Watcher rewind archival no longer calls Connection.commit() on APSW. _RewindArchiveBatcher.flush now runs an explicit BEGIN IMMEDIATE → UPDATE → COMMIT, with ROLLBACK if anything fails after the transaction starts, while keeping telemetry and pending-session batching behavior.

Regression coverage adds real-VectorStore/APSW tests: archived rows survive after close, failed flushes roll back with pending sessions left for retry and error telemetry, plus the existing batching mocks still pass.

BrainBar gains a deterministic XCTest (Asia/Jerusalem TZ, naive local created_at string) asserting agent pipeline window buckets treat that timestamp as local wall clock—locking already-shipped normalization, not changing dashboard SQL in this diff.

An implementation plan doc captures truth-investigation notes and verification steps.

Reviewed by Cursor Bugbot for commit c3be151. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Fix watcher rewind archival to use explicit SQL transactions instead of APSW connection commit

  • Wraps the UPDATE in _RewindArchiveBatcher.flush in an explicit BEGIN IMMEDIATE/COMMIT transaction via SQL statements, replacing a direct call to vector_store.conn.commit() which does not exist on APSW connections.
  • On failure, attempts a ROLLBACK before re-raising, ensuring partial updates do not persist.
  • Adds tests covering both the happy path (real APSW connection archives rows) and rollback path (trigger-forced abort leaves pending rows intact and telemetry records an error outcome).
  • Adds a test for naive local wall-clock timestamp handling in BrainDatabaseWindowedBucketsTests to verify correct bucket bucketing across timezones.
📊 Macroscope summarized c3be151. 2 files reviewed, 0 issues evaluated, 0 issues filtered, 0 comments posted

🗂️ Filtered Issues

No issues evaluated.

Summary by CodeRabbit

  • Bug Fixes

    • Improved rewind archival reliability by ensuring database updates commit atomically and roll back safely after failures.
    • Preserved pending archival work for successful retry after a transient error.
    • Corrected local wall-clock timestamp handling for activity window bucketing.
  • Tests

    • Added coverage for archival success, rollback, retry behavior, and local-time bucket calculations.
  • Documentation

    • Added an implementation plan covering regression testing, archival fixes, and quality checks.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your trial has ended. Reactivate Greptile to resume code reviews.

@cursor

cursor Bot commented Jul 14, 2026

Copy link
Copy Markdown

Bugbot couldn't run - usage limit reached

Bugbot 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_20b109d9-39a9-43b3-b758-432dae62250d)

@EtanHey

EtanHey commented Jul 14, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@EtanHey

EtanHey commented Jul 14, 2026

Copy link
Copy Markdown
Owner Author

@codex review

@EtanHey

EtanHey commented Jul 14, 2026

Copy link
Copy Markdown
Owner Author

@cursor @BugBot review

@cursor

cursor Bot commented Jul 14, 2026

Copy link
Copy Markdown

You need to increase your spend limit or enable usage-based billing to run background agents. Go to Cursor

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 14, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

No new commits to review since the last review.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1d9ead3c-c1d9-4d41-8f40-8b60900e1e80

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

The changes add BrainBar coverage for naive local timestamps, make rewind archival use explicit APSW transaction boundaries, add commit and rollback regression tests, and document the implementation and validation plan.

Changes

Truth and rewind behavior

Layer / File(s) Summary
Local wall-clock bucket regression
brain-bar/Tests/BrainBarTests/BrainDatabaseWindowedBucketsTests.swift
Adds local timestamp insertion support and verifies agent window buckets under the Asia/Jerusalem timezone.
Rewind archival transaction handling
src/brainlayer/cli/__init__.py, tests/test_rewind_batch_archival.py
Uses explicit BEGIN IMMEDIATE, COMMIT, and rollback handling, with tests covering successful archival, failed updates, telemetry, and retry behavior.
Implementation plan
docs/plans/2026-07-14-brainbar-truth-rewind.md
Documents the regression work, transaction changes, validation commands, and operational restart notes.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Poem

A bunny watched the timestamps hop,
While archive rolls now cleanly stop.
Commit, rollback, buckets bright—
Tests keep every trail in sight.
“Nibble-safe!” the rabbit sings.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: making watcher rewind archival APSW-safe with explicit transaction handling.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/brainbar-truth-rewind

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: c3be151608

ℹ️ 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".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant