feat: move the day-processing outbox to a device-local table (ADR 0044)#3564
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 29 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe day-processing outbox moves from filesystem-backed job files to a device-local Drift database. The change adds transactional persistence, atomic SQL claiming, indexed queries, startup migration with cutover sentinels, terminal-row retention, updated runtime wiring, and corresponding test coverage. ChangesDay-processing outbox storage
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant AppStartup
participant LegacyFileStore
participant OutboxMigration
participant DayProcessingDb
participant OutboxRepository
AppStartup->>OutboxMigration: initialize outbox
OutboxMigration->>LegacyFileStore: read and verify legacy jobs
OutboxMigration->>DayProcessingDb: import missing jobs and write sentinel
AppStartup->>OutboxRepository: create repository with database
OutboxRepository->>DayProcessingDb: query, mutate, and claim jobs transactionally
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
The file-per-job outbox re-read, JSON-decoded and checksummed every job file on each claim, and terminal jobs are deliberately retained as the Activity ledger — so per-action work grew with install age. Claiming is now an indexed statement over pending rows only. - DayProcessingDb: one row per job, with partial indexes that keep the retained ledger off the drain path, the review-fence sweep and the runtime's schedule probe, plus a (day_id, kind, created_at) index for Activity. - Claiming is a single atomic UPDATE ... WHERE id = (SELECT ... LIMIT 1) RETURNING, so no window exists in which a second claimer can take a row between it being chosen and owned. _serialize, _recoverPartials, _quarantine and the SHA-256 envelope are deleted. - The three whole-store readers become bounded queries and getAll() is removed rather than left as a trap for the next reader. - Retention deletes terminal rows past 90 days on the existing once-per-start repair pass; non-terminal rows never age out, since a parked job is outstanding user intent that retryNow can resurrect. - Migration imports the legacy directory during startup, before the runtime and any enqueue path exist, verifies by identity rather than row count, and commits the sentinel in the same transaction as the confirming scan. Job files stay one release as a rollback. Amends ADR 0044: the specified select-then-conditional-UPDATE guarded a race that a drift transaction already prevents, making its retry branch unreachable. The single-statement form removes the question instead.
The integration_test root builds its own getIt, so it constructs the outbox repository directly and was missed by an analyze over lib/test.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3564 +/- ##
==========================================
- Coverage 99.22% 99.22% -0.01%
==========================================
Files 1774 1779 +5
Lines 130106 130184 +78
==========================================
+ Hits 129093 129170 +77
- Misses 1013 1014 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
matthiasn
force-pushed
the
feat/day-processing-outbox-table
branch
from
July 24, 2026 22:28
f713fec to
5c7bd3b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@docs/adr/0044-day-processing-outbox-storage.md`:
- Around line 176-178: Replace the future July 25, 2026 amendment date with the
actual non-future date consistently in
docs/adr/0044-day-processing-outbox-storage.md: update the supersession
reference at lines 176-178 and the amendment heading and retrospective text at
lines 398-400 to use the same date.
In `@lib/features/daily_os_next/README.md`:
- Around line 261-264: Update the ledger-retention description near “Terminal
jobs are retained deliberately” to state that terminal rows are pruned after 90
days and the ledger therefore has bounded retention. Preserve the explanation
that partial indexes exclude retained terminal rows from hot paths, and ensure
the README matches the implemented pruning behavior.
In `@lib/features/daily_os_next/services/day_processing_legacy_file_store.dart`:
- Around line 89-96: Update the destination/partial conflict handling in the
day-processing migration flow to decode and compare both files before deleting
the partial. Retain and promote a valid partial when it represents newer state
using the existing updatedAt or generation fields, preserve the published
destination when it is newer, and delete the partial only after verifying it is
stale or invalid.
In `@lib/features/daily_os_next/services/day_processing_outbox_migration.dart`:
- Around line 70-73: Update the post-migration path returning `imported` so an
unstable final verification cannot complete normally and start the table-only
outbox. In the migration flow used by `initializeDayProcessingOutbox`, throw to
abort startup when a legacy file is detected after the final snapshot, or retain
the legacy fallback repository until verification succeeds; do not leave the
filesystem job invisible for the current session.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 40bb4dd3-778a-4f18-a1e2-be9baa05f02c
📒 Files selected for processing (31)
docs/adr/0044-day-processing-outbox-storage.mdlib/features/daily_os_next/README.mdlib/features/daily_os_next/database/day_processing_db.dartlib/features/daily_os_next/database/day_processing_db.driftlib/features/daily_os_next/database/day_processing_db.g.dartlib/features/daily_os_next/database/day_processing_job_row.dartlib/features/daily_os_next/services/day_activity_repository.dartlib/features/daily_os_next/services/day_audio_review_fence.dartlib/features/daily_os_next/services/day_processing_legacy_file_store.dartlib/features/daily_os_next/services/day_processing_outbox_migration.dartlib/features/daily_os_next/services/day_processing_outbox_repository.dartlib/features/daily_os_next/services/day_processing_runtime.dartlib/features/daily_os_next/services/day_processing_startup.dartlib/features/daily_os_next/state/day_processing_runtime_provider.dartlib/get_it.darttest/features/daily_os_next/integration/day_agent_durable_jobs_smoke_test.darttest/features/daily_os_next/integration/day_agent_pipeline_harness.darttest/features/daily_os_next/logic/real_day_agent_test.darttest/features/daily_os_next/services/day_activity_repository_test.darttest/features/daily_os_next/services/day_audio_review_fence_test.darttest/features/daily_os_next/services/day_processing_legacy_file_store_test.darttest/features/daily_os_next/services/day_processing_outbox_migration_test.darttest/features/daily_os_next/services/day_processing_outbox_processor_test.darttest/features/daily_os_next/services/day_processing_outbox_repair_test.darttest/features/daily_os_next/services/day_processing_outbox_repository_test.darttest/features/daily_os_next/services/day_processing_runtime_test.darttest/features/daily_os_next/services/day_processing_startup_test.darttest/features/daily_os_next/services/day_processing_test_db.darttest/features/daily_os_next/state/capture_controller_test.darttest/features/daily_os_next/state/day_activity_provider_test.darttest/features/daily_os_next/state/day_processing_runtime_provider_test.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/features/daily_os_next/state/capture_controller_test.dart (1)
79-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the obsolete filesystem fixture.
After this DB-based wiring,
outboxRootis only created at Line 77, stored at Line 120, and deleted at Line 182. Remove it from_Benchand teardown.As per coding guidelines, “Avoid retaining unused code.”
🤖 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 `@test/features/daily_os_next/state/capture_controller_test.dart` around lines 79 - 81, Remove the obsolete outboxRoot filesystem fixture from the _Bench setup and teardown, including its creation, storage, and deletion, while preserving the DayProcessingOutboxRepository database wiring.Source: Coding guidelines
🤖 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 `@integration_test/tutorial/tutorial_harness.dart`:
- Line 478: Add the harness-created dayProcessingDb to _closeables wherever it
is registered, including the corresponding setup at the other referenced
location, and update TutorialAppHarness.dispose() to recognize DayProcessingDb
and await db.close(). Preserve existing closeable teardown behavior for other
resource types.
---
Nitpick comments:
In `@test/features/daily_os_next/state/capture_controller_test.dart`:
- Around line 79-81: Remove the obsolete outboxRoot filesystem fixture from the
_Bench setup and teardown, including its creation, storage, and deletion, while
preserving the DayProcessingOutboxRepository database wiring.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: f961669c-bc6e-4dc7-abc0-2782d47a08d1
📒 Files selected for processing (32)
docs/adr/0044-day-processing-outbox-storage.mdintegration_test/tutorial/tutorial_harness.dartlib/features/daily_os_next/README.mdlib/features/daily_os_next/database/day_processing_db.dartlib/features/daily_os_next/database/day_processing_db.driftlib/features/daily_os_next/database/day_processing_db.g.dartlib/features/daily_os_next/database/day_processing_job_row.dartlib/features/daily_os_next/services/day_activity_repository.dartlib/features/daily_os_next/services/day_audio_review_fence.dartlib/features/daily_os_next/services/day_processing_legacy_file_store.dartlib/features/daily_os_next/services/day_processing_outbox_migration.dartlib/features/daily_os_next/services/day_processing_outbox_repository.dartlib/features/daily_os_next/services/day_processing_runtime.dartlib/features/daily_os_next/services/day_processing_startup.dartlib/features/daily_os_next/state/day_processing_runtime_provider.dartlib/get_it.darttest/features/daily_os_next/integration/day_agent_durable_jobs_smoke_test.darttest/features/daily_os_next/integration/day_agent_pipeline_harness.darttest/features/daily_os_next/logic/real_day_agent_test.darttest/features/daily_os_next/services/day_activity_repository_test.darttest/features/daily_os_next/services/day_audio_review_fence_test.darttest/features/daily_os_next/services/day_processing_legacy_file_store_test.darttest/features/daily_os_next/services/day_processing_outbox_migration_test.darttest/features/daily_os_next/services/day_processing_outbox_processor_test.darttest/features/daily_os_next/services/day_processing_outbox_repair_test.darttest/features/daily_os_next/services/day_processing_outbox_repository_test.darttest/features/daily_os_next/services/day_processing_runtime_test.darttest/features/daily_os_next/services/day_processing_startup_test.darttest/features/daily_os_next/services/day_processing_test_db.darttest/features/daily_os_next/state/capture_controller_test.darttest/features/daily_os_next/state/day_activity_provider_test.darttest/features/daily_os_next/state/day_processing_runtime_provider_test.dart
🚧 Files skipped from review as they are similar to previous changes (27)
- lib/features/daily_os_next/services/day_processing_startup.dart
- test/features/daily_os_next/services/day_processing_outbox_processor_test.dart
- lib/features/daily_os_next/state/day_processing_runtime_provider.dart
- lib/features/daily_os_next/database/day_processing_db.dart
- test/features/daily_os_next/services/day_processing_test_db.dart
- test/features/daily_os_next/state/day_activity_provider_test.dart
- test/features/daily_os_next/integration/day_agent_pipeline_harness.dart
- lib/features/daily_os_next/database/day_processing_db.drift
- test/features/daily_os_next/integration/day_agent_durable_jobs_smoke_test.dart
- lib/features/daily_os_next/services/day_processing_runtime.dart
- test/features/daily_os_next/state/day_processing_runtime_provider_test.dart
- lib/features/daily_os_next/services/day_activity_repository.dart
- lib/features/daily_os_next/services/day_audio_review_fence.dart
- lib/features/daily_os_next/database/day_processing_job_row.dart
- test/features/daily_os_next/services/day_audio_review_fence_test.dart
- test/features/daily_os_next/services/day_processing_runtime_test.dart
- test/features/daily_os_next/services/day_processing_legacy_file_store_test.dart
- test/features/daily_os_next/services/day_processing_outbox_repair_test.dart
- lib/features/daily_os_next/README.md
- lib/features/daily_os_next/services/day_processing_legacy_file_store.dart
- test/features/daily_os_next/services/day_processing_outbox_migration_test.dart
- lib/features/daily_os_next/services/day_processing_outbox_migration.dart
- test/features/daily_os_next/logic/real_day_agent_test.dart
- lib/get_it.dart
- lib/features/daily_os_next/services/day_processing_outbox_repository.dart
- test/features/daily_os_next/services/day_processing_outbox_repository_test.dart
- lib/features/daily_os_next/database/day_processing_db.g.dart
- Legacy import keeps the newest readable state per job instead of trusting one filename. atomicWriteBytes writes '<path>.tmp.<micros>.<pid>.media', not '.json.part', so a crash in its write/rename window left a file the old recovery ignored entirely — and on a job's first write that scratch file is the only copy. All encodings are now decoded and compared on generation, then updatedAt. - README no longer claims the ledger grows for the life of the install; retention bounds how far back it reaches, partial indexes keep even retained rows off hot paths. - Correct the unstable-migration comment: the filesystem is not authoritative once the repository reads the table, so say plainly that un-imported work is invisible for that session and recovered on the next start. Log it rather than failing startup.
This was referenced Jul 24, 2026
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.
Implements the ADR 0044 decision: the Daily OS processing outbox moves from
one JSON file per job to a device-local Drift table.
Why
_readAllUnsafe()did alistSync()over the job directory and, for everyfile, read it, JSON-decoded the envelope, verified its SHA-256 and decoded the
payload. That backed
claimNext,getAllandsignalConnectivityRestored,and
DayProcessingRuntime.drainAndScheduleperformed several of those scansper nudge — on every outbox mutation, connectivity restore, startup and retry
wakeup.
Terminal jobs are retained on purpose (
"Jobs remain after success as the local processing ledger consumed by Activity and startup repair"), so thescanned set grew for the life of the install. Every recording adds at least
two job files. Per-action work on the main isolate therefore grew linearly
with app age, and the part that grew was the part retained deliberately.
The same failure mode was already hit, measured and fixed in this repo's sync
queue —
inbound_event_queuekeeps itsappliedledger out of the drain pathwith partial indexes, after full scans cost "73s of DB time per hour" on a
desktop mid-drain. A directory of JSON files has no equivalent.
What changed
Schema (
database/day_processing_db.drift) — one row per job; envelopecolumns are real columns, the kind payload and run keys stay JSON. Three
indexes: a partial over non-terminal rows keyed
(created_at, id)for theclaim/fence/schedule paths,
(day_id, kind, created_at)for Activity, and apartial over terminal rows for retention. Timestamps are epoch milliseconds,
matching
inbound_event_queueand preserving precision Drift's defaultDateTime mapping would truncate.
Claiming is one atomic statement —
UPDATE … WHERE id = (SELECT … ORDER BY created_at, id LIMIT 1) RETURNING …._serialize,_recoverPartials,_quarantineand the SHA-256 envelope are deleted; transactions and WAL coverwhat they were for. Fencing on claimed mutations is unchanged.
Readers — all three whole-store scans become bounded queries
(
getForDay,getPendingByKind,getSchedulable), andgetAll()is removedrather than left as a trap for the next reader.
Retention — terminal rows past 90 days are deleted on the existing
once-per-start repair pass. Non-terminal rows never age out regardless: a job
parked in
failed/waitingForUser/waitingForNetworkis outstanding userintent that
retryNowor a re-enqueue can still resurrect.Migration —
initializeDayProcessingOutboxruns during startup, beforethe runtime and before any enqueue path is wired (the write barrier), imports
every job file, verifies by identity with a re-scan until stable, and
commits the sentinel in the same transaction as the confirming scan.
DayProcessingLegacyFileStorekeeps the partial-recovery and quarantine logicread-only for the import; job files stay on disk one release as a rollback.
ADR amendment
The ADR specified select-then-conditional-
UPDATEwith a retry when a claimloses a race — my answer to a review comment on #3562. Implementing it showed
both statements run inside one drift transaction on a single connection, so
that race cannot occur, the generation guard could never fail, and the retry
branch was unreachable dead code. The single-statement form removes the
question rather than answering it. Recorded as an amendment on ADR 0044.
Testing
daily_os_nextand the affected agents suite.four modified files is covered.
quarantine), migration (idempotency, interrupted import, never rewinding a
row the live repository advanced, keeping a row whose file was quarantined),
startup wiring, bounded queries, retention.
fvm dart analyze lib testclean; formatter clean.No CHANGELOG entry — this is an internal storage change with no runtime
behavior a user would notice.
Note for review
Drift silently drops a column named
key— it generated the migrations tablewithout it rather than erroring, which would have shipped a sentinel that
could never be written and a migration that re-ran forever. Renamed to
migration_key; only the migration tests caught it.Summary by CodeRabbit