[needs careful review] feat(sync): default fetch-all history + device-only retention (#12, #13) #46

Merged
JMR-dev merged 15 commits from feat-fetch-all-retention into main 2026-07-02 04:03:50 +00:00
JMR-dev commented 2026-07-01 05:27:44 +00:00 (Migrated from github.com)

Closes #12. Closes #13.

Replaces the fixed 50-message-per-folder header cap with a background,
resumable full-history backfill, and adds a user-configurable device-only
retention
limit that prunes local mail beyond it — never deleting from the
server.

⚠ Architecture-critical — please review these surfaces closely

  1. Backfill resumability / cancellation — MailBackfiller.backfillFolder
    pages a folder newest→oldest via ImapClient.fetchOlderThan. It derives the
    next boundary from the lowest currently-cached UID each batch (not a
    stored cursor) and persists progress in the new backfill_progress table
    after every NonCancellable batch write. A run stopped by process death /
    network loss / WorkManager cancellation resumes from the cached rows. It runs
    off MailSyncer's sync mutex so pull-to-refresh stays responsive. Please
    sanity-check the resume invariant and that headers are always persisted
    before the boundary advances.

  2. Backfill ↔ prune precedence — the rule is: backfill pauses (does not
    mark the folder complete) once the account's retention floor is reached, and
    the pruner only deletes below that same floor — disjoint working sets.
    They also share a process-wide MailMaintenanceGate mutex so they never run
    at once (defence in depth). Because backfill pauses (not completes) at the
    floor, loosening retention later resumes filling automatically. Please review
    reachedRetentionFloor + the "pause vs complete" distinction.

  3. Server-load batching / backoff — fetchOlderThan bounds both memory
    (one BACKFILL_BATCH_SIZE=50 page materialized) and network (boundary found
    by binary search over message numbers = O(log n) tiny UID FETCHs, not a
    full lower-range scan). Each WorkManager run does ≤DEFAULT_MAX_BATCHES=20
    pages with a 250 ms inter-page delay; periodic cadence is 30 min, battery-
    not-low + network constrained. Is the batch/backoff shape acceptable for
    large mailboxes? Note: body prefetch during backfill honours FetchPolicy
    (default ALWAYS) and can be heavy — flagging for a tuning opinion.

  4. Room schema migration (9 → 10) — MIGRATION_9_10 adds
    messages.uid (backfilled from the id's numeric tail via
    rtrim(id,'0123456789')), nullable account_settings.retentionCount/Months,
    and the backfill_progress table. Exported 10.json is committed and a
    MigrationTestHelper test (Migration9To10Test, instrumented) validates it.

What changed

  • #12: messages.uid materialized column powers a windowed deletion
    reconcile in MailSyncer (deleteSyncedInWindowNotIn) so foreground sync no
    longer wipes everything outside the recent 50 — backfilled history survives.
    Backfill kicked on account-add and on a periodic schedule.
  • #13: per-account count/age retention overrides + global default
    (RetentionPolicy, 0 = keep everything). MailPruner deletes local rows
    beyond the limit (attachment rows cascade; on-disk cache removed; deletes
    chunked under SQLite's 999-param limit) and never issues a server delete.
    Settings UI for global + per-account with "device only, not the server" copy.

Self-review found & fixed

  • Foreground↔prune re-download fight when a count limit is below the 50-msg
    window → foreground fetch window is now capped by the retention count.
  • deleteByIds could exceed SQLite's 999-parameter limit on a first large prune
    → chunked.
  • Two instrumented ViewModel tests broken by the new SyncScheduler DI param →
    updated.

Testing

Fast CI gate green: assembleDebug + testDebugUnitTest + lintDebug +
ktlintCheck + detekt. New JVM tests use GreenMail to prove the paged
backfill caches >50 messages and resumes after an interruption
(ImapClientBackfillTest, MailBackfillerTest), plus count/age pruning that
never asks the server to delete (MailPrunerTest) and RetentionPolicyTest.

Left to CI / manual (needs an emulator): the MigrationTestHelper migration
test and any Compose UI verification of the new retention settings + a backfill
progress indicator (no progress UI is included in this PR).

🤖 Generated with Claude Code

Closes #12. Closes #13. Replaces the fixed 50-message-per-folder header cap with a background, resumable **full-history backfill**, and adds a user-configurable **device-only retention** limit that prunes local mail beyond it — never deleting from the server. ## ⚠ Architecture-critical — please review these surfaces closely 1. **Backfill resumability / cancellation** — `MailBackfiller.backfillFolder` pages a folder newest→oldest via `ImapClient.fetchOlderThan`. It derives the next boundary from the **lowest currently-cached UID** each batch (not a stored cursor) and persists progress in the new `backfill_progress` table after every `NonCancellable` batch write. A run stopped by process death / network loss / WorkManager cancellation resumes from the cached rows. It runs **off** `MailSyncer`'s sync mutex so pull-to-refresh stays responsive. Please sanity-check the resume invariant and that headers are always persisted before the boundary advances. 2. **Backfill ↔ prune precedence** — the rule is: backfill **pauses** (does not mark the folder complete) once the account's retention floor is reached, and the pruner only deletes **below** that same floor — disjoint working sets. They also share a process-wide `MailMaintenanceGate` mutex so they never run at once (defence in depth). Because backfill pauses (not completes) at the floor, loosening retention later resumes filling automatically. Please review `reachedRetentionFloor` + the "pause vs complete" distinction. 3. **Server-load batching / backoff** — `fetchOlderThan` bounds both memory (one `BACKFILL_BATCH_SIZE`=50 page materialized) and network (boundary found by binary search over message numbers = O(log n) tiny `UID FETCH`s, not a full lower-range scan). Each WorkManager run does ≤`DEFAULT_MAX_BATCHES`=20 pages with a 250 ms inter-page delay; periodic cadence is 30 min, battery- not-low + network constrained. Is the batch/backoff shape acceptable for large mailboxes? Note: body prefetch during backfill honours `FetchPolicy` (default `ALWAYS`) and can be heavy — flagging for a tuning opinion. 4. **Room schema migration (9 → 10)** — `MIGRATION_9_10` adds `messages.uid` (backfilled from the id's numeric tail via `rtrim(id,'0123456789')`), nullable `account_settings.retentionCount/Months`, and the `backfill_progress` table. Exported `10.json` is committed and a `MigrationTestHelper` test (`Migration9To10Test`, instrumented) validates it. ## What changed - **#12:** `messages.uid` materialized column powers a **windowed** deletion reconcile in `MailSyncer` (`deleteSyncedInWindowNotIn`) so foreground sync no longer wipes everything outside the recent 50 — backfilled history survives. Backfill kicked on account-add and on a periodic schedule. - **#13:** per-account count/age retention overrides + global default (`RetentionPolicy`, 0 = keep everything). `MailPruner` deletes local rows beyond the limit (attachment rows cascade; on-disk cache removed; deletes chunked under SQLite's 999-param limit) and **never** issues a server delete. Settings UI for global + per-account with "device only, not the server" copy. ## Self-review found & fixed - Foreground↔prune re-download fight when a count limit is **below** the 50-msg window → foreground fetch window is now capped by the retention count. - `deleteByIds` could exceed SQLite's 999-parameter limit on a first large prune → chunked. - Two instrumented ViewModel tests broken by the new `SyncScheduler` DI param → updated. ## Testing Fast CI gate green: `assembleDebug` + `testDebugUnitTest` + `lintDebug` + `ktlintCheck` + `detekt`. New JVM tests use **GreenMail** to prove the paged backfill caches **>50** messages and **resumes** after an interruption (`ImapClientBackfillTest`, `MailBackfillerTest`), plus count/age pruning that never asks the server to delete (`MailPrunerTest`) and `RetentionPolicyTest`. **Left to CI / manual (needs an emulator):** the `MigrationTestHelper` migration test and any Compose UI verification of the new retention settings + a backfill progress indicator (no progress UI is included in this PR). 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Sign in to join this conversation.