Skip to content

[Sync] Ride history backup - #276

Draft
KacperKozak wants to merge 31 commits into
devfrom
feat/ride-history-backup
Draft

[Sync] Ride history backup#276
KacperKozak wants to merge 31 commits into
devfrom
feat/ride-history-backup

Conversation

@KacperKozak

@KacperKozak KacperKozak commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

This PR implements the app side of Ride History backup, paired with KacperKozak/vescape-server#12. A signed-in Rider's data syncs to the server continuously and automatically, from native, including mid-ride with the app backgrounded and the screen off — so losing a phone no longer loses months of riding.\n\n> [!WARNING]\n> Risk: High — changes native background sync, authentication, deletion semantics, and recovery behavior across both platforms.\n> Complexity: High — coordinates ordered batches, cursors, retries, Account lifecycle, cross-platform parity, and server contracts.\n> DB: Schema + data — adds tombstones, Board-keyed telemetry, sync cursors/sequences, and deletion actions through multiple Room and GRDB migrations.\n\n## Description\n\nThe backup implementation slices land on this branch and are tested once on device at the end. Native authentication is extracted into #283 and KacperKozak/vescape-server#16 so other server-backed capabilities can ship without waiting for backup.

What this lands

  • Backup that runs itself. Native scans each table forward from its Sync Cursor, sends small Sync Batches, and advances only what the server accepted. Being offline is a pause, not a failure — the next batch repairs it.
  • Reusable native authentication. Backup consumes the Device Token capability from [Auth] 1 - Authenticate native callers #283 and KacperKozak/vescape-server#16; it does not own authentication. Clerk provisions the credential through JS once, then native can call Account-data endpoints without a live JS runtime.
  • Boards are tombstoned, never deleted, on both sides, so Ride History outlives the Board that produced it (ADR-0027). This also closes the foreign-key wedge where an orphaned Tune Profile refused a whole batch.
  • Telemetry keyed on board_id, dropping the mutable BLE identifier and the denormalized Board name (ADR-0028, closes [Telemetry] Reconsider device_name denormalization on telemetry tables #274). Orphaned rows get a minted tombstoned Board so nothing becomes unuploadable.
  • Deletions travel as Sync Actions on an append-only typed log, with a target enum that makes a retention sweep structurally incapable of deleting the rides the backup exists to preserve.
  • One setting and one status line. "Back up over Wi-Fi only", defaulting off, offered once with the pending volume shown. Backup state renders in the account widget on the social sheet; failures become Diagnostic Events.

Tasks

#284 carries the correctness risk now that #279 and #280 have shipped on dev. #283 is intentionally outside the backup sequence and depends only on its standalone server peer, KacperKozak/vescape-server#16.

Already on the branch

Board tombstones and telemetry keyed on the Board id (#279, #280, ADR-0027, ADR-0028, closes #274) shipped on dev as #437 and came back in through the merge. They were written and tested on this branch; dev carries the later, broader version and this PR now consumes it rather than defining it.

  • The dev version also keys telemetry_markers, diagnostic_events and metric_exclusion_ranges on the Board, so the boardId -> bleId translation this branch still needed is gone entirely, along with the session-boundary bug it caused.
  • The bucket rebuild moved to schema 41 -> 42 / v42_telemetry_board_id, which is below every sync migration here. On this branch the rebuild ran after updated_at and sync_seq landed and had to carry them across explicitly; in the merged order those columns do not exist yet, so the rebuild is simply dev's and the carry-across is gone.
  • ADR-0027 on dev is deliberately app-scoped and defers the server half to this PR. The merge folds that half back in: the Board-owned foreign key, the Sync Batch a missing parent would refuse whole, and the explicit configuration cascade that replaces the server's ON DELETE CASCADE. Server half is KacperKozak/vescape-server#27.
  • What survives here and not on dev: the tombstone moves updated_at and sync_seq through the same stamping path as any other edit, BoardInput is Omit<Board, 'updatedAt' | 'deletedAt'> so JS cannot fabricate either, and the tombstone emits a Board Sync Action alongside the upsert.

The Sync Action log (#282). An append-only local log of semantic removals, sync_actions, keyed on its own AUTOINCREMENT cursor — SQLite guarantees that key monotonic and never reused, so the log needs no sync_seq. Room 45 -> 46 and GRDB v46_sync_actions, additive and no-op on re-run. The server half is KacperKozak/vescape-server#12.

  • Two shapes, deliberately different. A Board syncs its deleted_at as an ordinary upsert and emits one action: the row says the Board is deleted, the action says its configuration is gone. Its board settings, Board Warnings and Alert Rules are raw deletes covered by that one action, so an upsert never quietly deletes rows in three other tables. Everything else — Alert Rules, Tune Profiles, Privacy Zones, Favorites, app settings, board settings, Board Warnings — leaves no row behind, so the action is the only signal there is.
  • Seven Rider-facing write points, plus the automatic ones that mean the same thing: reset-to-default app settings, legalPolicy clearing, the corrupt-setting cleanup (deliberately semantic, so a restore cannot resurrect a value this phone already rejected), deleted Board-setting keys on a Board edit, preset-rule regeneration through the ordinary Alert Rule delete, and a Board Warning cleared by a clean detector evaluation.
  • Parent-covered cascades emit nothing. Deleting a Favorite emits one Favorite action and no Favorite Media actions; deleting a Tune Profile emits one and no Tune History actions — matching the server's own cascade.
  • Maintenance is silent by construction. Cursor-gated retention, migrations and the wipe behind a database restore write no actions, and DeleteTarget has no case for a pruned table, so a sweep is structurally incapable of naming one. Asserted on both platforms, mirroring the server's DELETE_ACTION_TARGETS test. No database trigger writes the log — intent cannot be inferred from SQL alone.
  • Stamped from the row being removed, max(now, row.updated_at) (last_detected_at for Board Warnings, its own change clock). A plain now on a rewound clock produces an action the server reads as a no-op and cannot self-heal from, because the row it would re-send is gone. A Board tombstone and its action share the newly ratcheted Board timestamp.
  • Cursor before pruning. The accepted-action cursor commits first and pruning reads it back rather than trusting a caller, so a crash between the two re-sends an action — a no-op — instead of losing one. The upload loop itself is [Sync] 6 - Upload Sync Batches #284.
  • Classification is a test, not a convention. Every delete against a syncable table is classified semantic, parent cascade or maintenance: Android scans the DAO source (Room keeps its SQL out of reach of a JVM test), iOS asserts the behaviour against a real database and refuses a raw delete written outside a delete-owning store.

Change Timestamps and Sync Cursors on boards, alerts and telemetry_minute_buckets — the first part of #281's mechanism, landed ahead of it.

Change Timestamps. boards and alerts carried created_at only and telemetry_minute_buckets carried nothing, so a board rename, an alert toggle, or a minute bucket still filling was invisible to an "everything changed since T" query. Room 42 -> 43 and GRDB v43_sync_cursors, both additive, both backfilling existing rows rather than leaving them at DEFAULT 0:

table backfill source
boards created_at
alerts created_at
telemetry_minute_buckets last_sample_at_ms

Native stamps the timestamp from its own clock on every write, never trusting the bridge value — including setAlertRuleEnabled on both platforms, a targeted UPDATE rather than a row rewrite and the specific regression that change exists to prevent. On the TS side Board and AlertRule gain updatedAt: number, and upsertBoard/upsertAlertRule take BoardInput/AlertRuleInput (Omit<…, 'updatedAt'>) so no call site can fabricate one.

The Sync Cursor split off the Change Timestamp (#275). One wall clock cannot do both jobs, because a device clock that steps backwards breaks each in a different direction:

  • the upload scan misses the write entirely — it lands below a cursor position the phone has already passed, and nothing on the server can notice, because the cursor is client-held
  • the server drops the write even when the scan does send it — every mutable table upserts under WHERE stored.updated_at < EXCLUDED.updated_at, so the older stored copy wins and the phone advances its cursor regardless. Single device, no conflict, edit lost.

So they are now two columns. sync_seq is a device-local counter, bumped on every write, and the scan runs on it; it never crosses the wire, so there is no schema, protocol or server change. updated_at keeps its wall-clock meaning and is ratcheted to max(previous + 1, now) per row, which is bounded by the size of the rewind and self-corrects once the clock passes it again. Room 43 -> 44 and GRDB v44_sync_seq. Recorded server-side as vescape-server ADR-0007.

The counter lives in its own sync_sequences table rather than being derived as MAX(sync_seq) + 1 per table: deleting the highest row would hand the same number out twice, which is the original bug reintroduced by the fix.

Sync Cursors on the six remaining mutable tables (#281). app_settings, board_settings, board_warnings, privacy_zones, tune_profiles and favorites now carry sync_seq and a ratcheted Change Timestamp, stamped by the same nextSyncSeq / ratchetUpdatedAt primitives the earlier tables use. Room 44 -> 45 and GRDB v45_sync_seq_remaining, backfilling existing rows from rowid and seeding each counter past the highest position handed out.

  • Append-only tables get nothing. Telemetry frames, markers, diagnostic events, metric exclusion ranges and tune history entries all declare INTEGER PRIMARY KEY AUTOINCREMENT, which SQLite guarantees monotonic and never reused — their key already is their cursor. Asserted, so a future column add has to argue with a test.
  • board_warnings also gains updated_at, the one table of the six that never had a wall clock at all, backfilled from its newest detection. It is distinct from last_detected_at: a severity or payload change rewrites the row without being a fresh detection.
  • Every targeted UPDATE moves both columns, the setAlertRuleEnabled shape that motivated the original work: the privacy-zone toggle and the three tune-profile updates (rename, save, rollback) now ratchet and renumber in their own SQL rather than round-tripping an entity.
  • Migration lists are frozen per version. SYNC_SEQ_TABLES_V43 and SYNC_SEQ_TABLES_V44 are separate, so growing the set never retroactively changes what an older migration step does.

The minute-bucket ratchet correction (#281). The bucket merge clamped its Change Timestamp with max(existing, incoming) on the stated premise that the server upserts that table unconditionally. It does not — it guards it with WHERE stored.updated_at < EXCLUDED.updated_at exactly like every other mutable table, so on a backwards clock step the row was scanned, sent, and silently dropped. Buckets now use the same max(previous + 1, now) ratchet as boards and alerts, and the comment asserting the exception is gone.

The uploader (#284). Native scans each table forward from its Sync Cursor, sends a small Sync Batch, and advances only what the server accepted. It runs inside the window the app already keeps alive — the foreground service during a ride on Android, the existing background modes on iOS — at 30 s while samples are being produced and 5 min when nothing is pending, with immediate kicks on connectivity regained, ride end and sign-in, and an immediate re-send while a 200 still leaves rows pending.

  • Two pure modules carry the interesting behaviour, per ADR-0010. SyncBatchBuilder walks the server's own table order — parents before children, Delete Actions last — and stops at 1000 rows or 1 MiB of actual compact UTF-8 bytes, measured on the encoded body rather than estimated. It never orders by backlog size, which would build a batch the server refuses whole. SyncPolicy turns ride/network/Wi-Fi-only/gate/pending/backoff state into send, wait or paused. Neither touches a database, a clock or the network.
  • Cursors commit after the response, in their own transaction. A cursor advanced past rows the server did not take is unrecoverable; a cursor left behind is a re-send the server upserts idempotently, so every failure path falls toward re-sending. A 200 advances only after the accepted map validates exactly — a missing table, an extra table or a mismatched count is a protocol failure, not a success.
  • Strict wire encoding. Native builds the JSON itself, field by field, enforcing the server's own bounds (key lengths, int32/int64 ranges, finite numbers, explicit nulls for nullable columns) before transport. A row that cannot encode pauses the engine with the row retained — never skipped, never quarantined. Same for 413 on a single row.
  • Transport policy. 400/409/422/unknown 4xx/malformed 2xx pause permanently with no cursor movement; 401 pauses for sign-in; 429 honours Retry-After; 5xx, network and timeout back off 30 s doubling to 15 min, reset on success. A permanent pause writes one coalesced, metadata-only Diagnostic Event — failure class, table, cursor, app version, never row contents, coordinates or the token.
  • Retention now asks the cursor. A never-bound database keeps the existing age-only cleanup. A bound one deletes only where the age cutoff and id <= accepted cursor hold, or sync_seq <= accepted cursor for minute buckets — so a bucket rewritten after its earlier version uploaded survives until the new position is accepted. A missing cursor is 0, which protects every row. The sweep reads the cursor and deletes in one transaction, and emits no Sync Actions.
  • Account change is a replace, not a cursor reset. The Device Token exchange returns a stable Account id; the first Account claims the database through a new sync_binding row (Room 46 -> 47, GRDB v47_sync_binding). The same Account keeps everything. A different one makes native refuse to bind and report accountChangeRequiresReset without storing the credential; JS shows a destructive warning saying cloud restore does not exist in this version, and only confirmation runs the ordered transition — stop, invalidate, replace the database, clear cursors and actions, bind, install the token, start. A generation captured per request is re-read before the commit, so a response from the previous Account is a no-op. Cancelling leaves the old database and binding untouched.
  • Signing out stops the uploader but keeps the binding, so data recorded while signed out stays retention-protected for the same Account.

A cross-agent review (Codex, read-only) went over this slice; eight findings landed as fixes:

  • The uploader now survives a cold launch. Only provisioning started the loop, so a signed-in phone that restarted never uploaded again. The module's OnCreate calls resumeIfBound, which re-binds the stored Account and starts the loop.
  • Scan, send and commit are serialized against an Account reset — a Kotlin Mutex, a Swift task chain — and the generation is captured before the scan rather than after it. Previously a reset landing mid-pass could commit the previous Account's cursors onto the fresh database. stop() now cancels in-flight kicks too, and the reset no longer starts the loop itself: the caller installs the new Device Token first.
  • The wipe is verified. A failed database delete or a refused re-bind now throws instead of quietly reopening the previous Account's database under a new Account's token.
  • A batch stops at a table the byte cap truncated. It used to carry on into later tables, which could put a Board's alerts in a batch whose Board did not fit — a dependency conflict the server refuses whole, and a permanent pause.
  • 413 at the smallest byte target pauses instead of resending the same bytes forever, and a shrink no longer reports itself as an upload (it returns a distinct retry outcome, and an exhausted drain yields instead of spinning at zero delay).
  • A failed cursor commit backs off rather than claiming success: iOS was swallowing the write error, which would have re-sent an accepted batch immediately and forever.
  • iOS sends boards.transport. It is the one platform that stores the Board Transport on the Board, and the server declares the field for exactly that; sending null lost the Board Link's transport on restore.
  • iOS key-length validation counts UTF-16 code units, matching the server's compiled value.length <= 128 and Kotlin's String.length. Swift's count counts grapheme clusters, so an emoji-heavy key could pass locally and wedge the batch server-side.

Two deliberate deviations from the issue, both about rows that can never be uploaded rather than rows waiting to be:

  • Unowned telemetry is not offered to the scan and the cursor moves over it. The server keys frames and buckets on the Board (ADR-0028) and has nowhere to put a sample that names none, so a frame with a null board_id, or a bucket on the unknown-Board sentinel, can never become uploadable. Pausing on one would wedge backup permanently on a row nobody can fix. The consequence is that a later owned row carries the cursor past a skipped one, so retention prunes unowned telemetry on age alone — exactly as it did before the Account binding existed.
  • Retry-After reads a response header, which meant widening the shared ApiResponse to carry lowercased headers and adding a VescapeApi.exchange that returns the raw status. The uploader needs 409, 413 and 429 kept apart, and ApiResult collapses them.

Verification: bun run test:android and bun run test:ios both green, including new pure-module, wire-boundary, engine-against-a-fake-transport and cursor-gated-retention suites on both platforms. bun run ts, lint, format:check, knip and bun test clean. Not run on device.

The Rider-facing half (#285). One setting, one choice, one status line.

  • A master switch, off by default. syncEnabled gates the uploader outright: switched off there is no scan, no request, no backoff, no pause notification — the loop is stopped, not left deciding to do nothing every five minutes. It is checked ahead of everything in both decide and describe, including a pause, because switched off is not a broken uploader waiting to be resumed. Phone-local and deliberately not synced: a kill switch must not travel through the mechanism it kills, or a restored snapshot could switch backup back on. Lives on its own Settings → Sync page with the status line and the Wi-Fi row; the Database page keeps only local backup/restore/rebuild.
  • "Back up over Wi-Fi only", a synced App Setting defaulting to off. With it on, nothing uploads on a metered connection, mid-ride included — no row classes, no backlog thresholds, no partial exceptions. Native reads the setting itself (AppDataRepository pushes every write to the uploader, and the uploader restores it before its first pass on a cold launch), so the JS setSyncWifiOnly call is gone: a restored backup or the one-time choice reaches the uploader the same way the settings row does.
  • The one-time choice, offered once when backup is on, with the pending volume shown, from the same per-table pending count the uploader already computes. Its "asked and answered" flag is deliberately phone-local — the expensive first upload belongs to the phone holding the backlog, so a restore onto a second phone asks that Rider rather than deciding for them.
  • The status line. SyncPolicy.describe derives the Rider-facing state from the same SyncState the send/wait decision reads, so the line can never disagree with what the uploader is doing: signed out, up to date, syncing, waiting for Wi-Fi, offline, paused. Signed out outranks the pause it produces (a phone with no credential is not a broken backup), and a batch waiting on backoff still reads as syncing.
  • Native emits, JS renders. New onSyncStatus event, replayed on subscribe and re-pulled on foreground, mirroring onAppStatus. It lands in the account widget on the social sheet — in its signed-in branch's identity block, not a fourth branch — and in a new Backup section on the Database settings screen next to the Wi-Fi switch. The signed-out hint now names backup as the reason to sign in.
  • Every paused reason also raises a notification, on both platforms, distinguishing sign in again / update required / backup error. A pause does not clear through ordinary retry, and it is cleared again the moment the pause lifts. Failures were already coalesced Diagnostic Events from [Sync] 6 - Upload Sync Batches #284.
  • BackupStatusLine takes an optional status prop, so every state has a live showcase preview under Settings → Components → Widgets.

Which app settings are per-Account and which are per-phone — the #277 question that gated this slice, now answered. Rider identity (riderId, riderName, riderColor), this phone's session state (selectedBoardId, the last GPS and direction-point coordinates), connection and companion behaviour (autoConnect, companion presence and its cooldown, connection sounds, auto-close and its delay) and Wear pairing (wearMirrorIntervalMs, wearAutoLaunchOnConnect) stay on the phone. Everything else syncs.

The split is a native constant list (NOT_SYNCED_SETTING_KEYS / notSyncedSettingKeys) rather than a per-key column, and it is enforced by never handing those rows a cursor position: sync_seq stays 0, which is below every Sync Cursor, so no scan can see them. The migration's rowid backfill is undone for exactly those keys, or an uploader would ship whatever the phone happened to hold at upgrade time, once. Rider Name and Rider Color live in app_settings by design so Group Ride keeps working signed-out — that placement is precisely what makes them phone-local rather than Account-scoped.

Schema parity with the server

A drift audit against the server's wire contract found backup was silently not restoring what it was given, in three directions at once. The server half is vescape-app/vescape-server#45; this branch is what puts the rows on the wire.

Tables the app had that backup never carried. VESC Fault Occurrences and their Captures (ADR-0037) are the one category of Rider-visible history that is permanent on the phone — exempt from every retention sweep, kept after the Board is gone — and none of it was synced, so a lost phone lost the whole fault record. Schema 48 gives an Occurrence the ratcheted updated_at it has to be judged on and both tables a sync_seq. A dismissal moves the stamp, which is the entire reason the column exists. Capture samples keep their local autoincrement id at home and are identified by occurrence and capture time, the way Tune History already is.

Columns the app grew that the wire never learned. Alert Rules now send thresholdKind, configFieldId and the two offsets, plus repeatEverySeconds and beepCount. A config-relative rule was restoring as a bare fixed rule at a stale number — worse than losing it, because it looks configured. Board Warnings now send updatedAt, so the server stops judging them on a detection time that does not move when a severity is re-triaged.

Columns the app dropped that the wire still demanded. faultCode and faultCount are gone from the frame and bucket encoders; the server dropped both columns and sending them now refuses the whole batch.

Two wedges

Both would have paused backup permanently on a retained row that no retry can clear.

  • Metric Exclusion Ranges can carry an empty board_id — the sanitizers write it for samples that match no Board — and unlike the frame and bucket scans, this one had no filter. The server's composite foreign key rejects it and 409s the entire batch.
  • Fault Capture samples are the first firmware-sourced floats ever put on the wire, and a non-finite one threw a permanent protocol error. They now encode as null through a new reading writer: these columns are nullable precisely because a field the firmware did not send is absent, so an unusable one is absent too. number still refuses non-finite for values a Rider authored, where a NaN is a bug worth stopping for.

Also

  • syncWifiOnly joins the settings that never travel. It answers what this phone's connection costs, not what the Rider prefers, so a restore onto a cellular-only phone was inheriting the other phone's answer.
  • Motor Config Values are dropped with their Board on both platforms. iOS had been keeping them on the link-mismatch path too, which was a silent parity divergence.
  • The SyncTable @parity tag pointed at a TypeScript type that never existed. Removed rather than invented — the wire table names do not cross the bridge.

Favorite Media stays out deliberately: its manifest describes bytes that live outside SQLite (ADR-0030) and nothing carries them yet, so a restore would list media it cannot open and that ADR's own reconciliation would then delete the rows again.

The dev merge

dev moved a long way under this branch, so the merge is a rewrite of the migration ladder rather than a textual reconciliation. Schema is 47 on both platforms.

This branch had taken schema 32 (Room 31 -> 32, GRDB v32) for its first sync migration and pushed the already-shipped alert-repeat migration to the tail as 38 -> 39 / v39_alert_repeat. That was safe while nothing above 32 existed; dev has since spent 32 through 42. So the slot goes back to alert-repeat, this branch's tombstone and board-id migrations are dropped as duplicates of dev's 40 -> 41 and 41 -> 42, and every sync migration moves above dev's:

this branch merged migration
31 -> 32 / v32_sync_cursors 42 -> 43 / v43_sync_cursors Change Timestamps
32 -> 33 / v33_sync_seq 43 -> 44 / v44_sync_seq sync_sequences and sync_seq
33 -> 34 / v34_board_deleted_at dropped, is dev's 40 -> 41
34 -> 35 / v35_telemetry_board_id dropped, is dev's 41 -> 42
35 -> 36 / v36_sync_seq_remaining 44 -> 45 / v45_sync_seq_remaining six remaining tables
36 -> 37 / v37_sync_actions 45 -> 46 / v46_sync_actions the Sync Action log
37 -> 38 / v38_sync_binding 46 -> 47 / v47_sync_binding the Account binding
38 -> 39 / v39_alert_repeat dropped, back to dev's 31 -> 32

Judgment calls worth reviewing:

  • Favorites resolve names through the tombstone-aware lookup. dev reads them from getBoards(), which filters tombstones, so a Favorite on a deleted Board lost its name — the exact case ADR-0027 exists to prevent. Both platforms now use getBoardNames() / boardNamesById().
  • faultCode and faultCount go out as explicit null. dev dropped both columns when VESC faults became Board-owned evidence in their own tables (ADR-0037), and those tables are not in SyncTable yet. The server still declares the fields, so null is the honest value rather than a stale zero.
  • The Board-owned decode caches are maintenance, not a cascade. deleteBoardConfigValues, deleteMotorConfigValues and deleteBoardConfigChangeNotice are absent from SyncTable, so nothing on the server needs removing and the Board's own action already says its configuration is gone.

A pre-existing dev bug fell out of the merge. upsertBucket in modules/vescape-core/ios/telemetry/TelemetryDao.swift binds 28 values against 27 columns — removing fault_count left the literal 0 one slot right, on max_gps_speed_centi_mps instead of gps_distance_cm. Every minute-bucket write on iOS fails with SQLite error 1: 28 values for 27 columns. Nothing on dev exercises it; this branch's SyncCursorMigrationTests do. Fixed here, but it is live on dev and wants its own fix there.

Docs

Decisions that are hard to reverse are recorded rather than left in the diff:

  • docs/adr/0027-boards-are-tombstoned-never-deleted.md, with its server peer as vescape-server ADR-0008
  • docs/adr/0028-telemetry-is-keyed-on-board-id.md, closing [Telemetry] Reconsider device_name denormalization on telemetry tables #274
  • docs/adr/0005-… gains a Scope section — its "no reconstruction on read" rule is about replaying Telemetry Samples, not about bounded configuration lookups, which is what ADR-0028 relies on
  • CONTEXT.md in both repos: Device Token, and the rule that Ride History is owned by the Vescape Account and only labelled by a Board. The server's glossary also renames Delete Action to Sync Action with a type, so the log generalises past deletion.

Verification

After the dev merge and the schema-parity work:

bun run test:android: 811 tests, 0 failures. Room KSP codegen accepts the merged entities and DAO, and schema 48 is exercised for columns, indices, the updated_at backfill, counter seeding and re-run safety.

bun run test:ios: green, 665 tests. #278 landed the runnable Swift test target, so the GRDB migrations are executed rather than mirrored on faith, and the iOS Sync Cursor tests run the real migrator against an in-memory database.

bun run ts, bun test (949 pass), bun run lint, bun run format:check and bun knip are clean. Not run on device.

The server side is green independently: bun run check on vescape-app/vescape-server#45 is 609 pass, with the DB-backed suites running against a real Postgres with the migration chain applied from scratch.

Open questions

  • The iOS upsertBucket arity bug is fixed here but still live on dev. It wants a standalone fix on dev rather than waiting for this branch.
  • This branch strands any existing branch-build install. The dev merge renumbered the sync migrations, so a phone sitting on the old schema 39 has no path forward: Android wipes silently through fallbackToDestructiveMigration, and iOS throws on a replayed migration and will not open the database at all. Acceptable for an unreleased branch, but delete and reinstall before taking this build.
  • The server database must be dropped and re-migrated. vescape-app/vescape-server#45 corrects the migrations that were wrong in place rather than layering fixups, so schema_migrations already records the old versions and the edits will not re-apply on their own.

Deliberately out of scope

  • Restore as a Rider-facing flow. The server can serve the data back; nothing here downloads or applies it.
  • Scheduled background upload outside a ride. No WorkManager, no BGTaskScheduler. A ride that ends offline on a phone that is never reopened waits for the next open or the next ride.
  • Per-ride backup badges in Ride History. The single status line is the whole surface in this version.
  • Multi-device live sync. Two phones on one Account is last-write-wins by Change Timestamp, with no merge UI.
  • Group Ride changes. It stays unauthenticated on Redis, untouched.

Tickets

@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds native-stamped updatedAt cursors for boards, alert rules, and telemetry buckets across Android, iOS, TypeScript, and E2E storage. Database migrations backfill and index cursors, while write paths, optimistic state, and tests validate propagation and monotonic bucket updates.

Changes

Incremental sync cursor contracts and schema

Layer / File(s) Summary
Cursor contracts and schema
modules/vescape-core/android/.../telemetry/*, modules/vescape-core/ios/..., modules/vescape-core/src/index.ts, src/modules/alerts/lib/customAlertRules.ts
Adds updatedAt fields and input types, database columns and indexes, version 28 migrations, and zero-valued cursors for in-memory legal-mode overlays.

Native persistence

Layer / File(s) Summary
Native cursor stamping and propagation
modules/vescape-core/android/.../telemetry/*, modules/vescape-core/ios/.../telemetry/*
Stamps board and alert writes, serializes cursors, updates alert toggles, and preserves the maximum cursor when merging telemetry buckets.

JavaScript state and E2E persistence

Layer / File(s) Summary
JavaScript write shapes and optimistic state
modules/vescape-core/src/index.ts, modules/vescape-core/src/e2eFake.ts, src/modules/alerts/store/*, src/modules/board/store/*
Uses cursor-free input shapes for native writes, stamps optimistic local rules and boards, and updates E2E board storage and seeded timestamps.

Validation

Layer / File(s) Summary
Migration and cursor validation
modules/vescape-core/android/src/test/..., modules/vescape-core/ios/.../SyncCursorMigrationTests.swift, src/modules/*/*.test.ts
Tests schema migration and backfill behavior, cursor propagation, alert toggles, bucket merges, monotonicity, and updated entity fixtures.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant JavaScriptStore
  participant NativeRepository
  participant TelemetryDatabase
  JavaScriptStore->>NativeRepository: Submit BoardInput or AlertRuleInput
  NativeRepository->>TelemetryDatabase: Stamp and persist updated_at
  TelemetryDatabase-->>NativeRepository: Return persisted entity cursor
  NativeRepository-->>JavaScriptStore: Provide entity with updatedAt
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR updates updated_at handling, but the linked issue requires a separate monotonic sync_seq cursor for scans, which is not shown here. Add the device-local sync_seq table/column, migrate existing rows, and switch client sync scans to sync_seq instead of updated_at.
Docstring Coverage ⚠️ Warning Docstring coverage is 36.17% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Out of Scope Changes check ✅ Passed The changes appear focused on sync-cursor timestamps, migrations, and test coverage; no clearly unrelated code paths stand out.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the pull request's main purpose: preparing ride history data for synchronization.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/ride-history-backup

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.

@KacperKozak
KacperKozak marked this pull request as draft July 27, 2026 01:46

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🧹 Nitpick comments (1)
modules/vescape-core/ios/telemetry/AppDataRepository.swift (1)

28-60: 🩺 Stability & Availability | 🔵 Trivial

Silent write/read no-op when writer is nil.

write and read swallow both a nil writer and any thrown error with no logging/diagnostics. Given this PR is specifically about not missing rider writes (#275), a hot-swap window where TelemetryDatabase.pool is nil means board/alert-rule upserts silently vanish while notifyDataChanged still fires as if the write succeeded. This mirrors an existing pattern (TuneProfileStore/BoardWarningStore), so it's not new, but worth hardening now that the sync-reliability bar is being raised: consider capturing a diagnostic event (similar to DiagnosticReporter usage on Android) when a write is dropped or throws.

🤖 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 `@modules/vescape-core/ios/telemetry/AppDataRepository.swift` around lines 28 -
60, Harden AppDataRepository’s read and write helpers so nil writers and
database errors emit diagnostic events instead of failing silently. Update
read(_:_:) and write(_:) to report whether the writer is unavailable or the
database operation throws, while preserving the existing fallback behavior for
reads and no-op behavior for writes; use the repository’s established
diagnostics mechanism.
🤖 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 `@modules/vescape-core/src/index.ts`:
- Around line 173-178: Replace the `updatedAt` documentation at
modules/vescape-core/src/index.ts:173-178 and :272-277 to identify it as
wall-clock last-write-wins metadata, not the sync cursor. In
modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt:475-505,
add the 28→29 migration with atomic strictly increasing `sync_sequences`,
per-row `sync_seq` for every syncable write, and scan indexes. Add the
equivalent GRDB migration and schema in
modules/vescape-core/ios/telemetry/TelemetryDatabase.swift:421-444; retain
`updatedAt` for last-write-wins while scans use `sync_seq`.

In `@src/modules/alerts/store/alertsStore.ts`:
- Around line 15-20: Update the optimistic mutation paths in update() and
setEnabled() to wrap the locally written alert rule with withLocalCursor before
storing it. Ensure both paths assign a fresh local updatedAt while leaving the
persisted/native write behavior unchanged.

---

Nitpick comments:
In `@modules/vescape-core/ios/telemetry/AppDataRepository.swift`:
- Around line 28-60: Harden AppDataRepository’s read and write helpers so nil
writers and database errors emit diagnostic events instead of failing silently.
Update read(_:_:) and write(_:) to report whether the writer is unavailable or
the database operation throws, while preserving the existing fallback behavior
for reads and no-op behavior for writes; use the repository’s established
diagnostics mechanism.
🪄 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: 3c2fa431-314d-4b15-a8b1-1a21855f995e

📥 Commits

Reviewing files that changed from the base of the PR and between fe0b1a9 and 7836646.

📒 Files selected for processing (22)
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/alerts/AlertEngine.kt
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/AppDataRepository.kt
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryBucketBuilder.kt
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDao.kt
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryEntities.kt
  • modules/vescape-core/android/src/test/java/expo/modules/vescapecore/alerts/AlertEngineTest.kt
  • modules/vescape-core/android/src/test/java/expo/modules/vescapecore/telemetry/ProfileStatsRepositoryTest.kt
  • modules/vescape-core/android/src/test/java/expo/modules/vescapecore/telemetry/SyncCursorMigrationTest.kt
  • modules/vescape-core/ios/alerts/AlertEngine.swift
  • modules/vescape-core/ios/alerts/AlertEngineTests.swift
  • modules/vescape-core/ios/telemetry/AppDataRepository.swift
  • modules/vescape-core/ios/telemetry/SyncCursorMigrationTests.swift
  • modules/vescape-core/ios/telemetry/TelemetryDao.swift
  • modules/vescape-core/ios/telemetry/TelemetryDatabase.swift
  • modules/vescape-core/src/e2eFake.ts
  • modules/vescape-core/src/index.ts
  • src/modules/alerts/lib/customAlertRules.ts
  • src/modules/alerts/store/alertPresetStore.test.ts
  • src/modules/alerts/store/alertsStore.ts
  • src/modules/board/store/boardStore.test.ts
  • src/modules/board/store/boardStore.ts

Comment on lines +173 to +178
/**
* Incremental-sync cursor: epoch ms of the last write to this board, from the same clock as
* {@link createdAt}. Native stamps it on every upsert (including partial edits), so a value sent
* from JS is ignored — read it, do not author it.
*/
updatedAt: number

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Add the required monotonic sync_seq; do not scan by updatedAt.

The PR objective requires sync_seq specifically because wall clocks can move backwards. A write stamped at 900 after a prior scan cursor of 1000 is omitted by an updated_at > 1000 scan; same-millisecond writes have the same problem. Retain updated_at for last-write-wins, but allocate and persist a strictly increasing sequence atomically for every syncable write.

  • modules/vescape-core/src/index.ts#L173-L178: describe updatedAt as wall-clock LWW metadata, not the scan cursor.
  • modules/vescape-core/src/index.ts#L272-L277: apply the same contract correction for alert rules.
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt#L475-L505: add the 28→29 migration, sync_sequences, row-level sync_seq, and scan indexes.
  • modules/vescape-core/ios/telemetry/TelemetryDatabase.swift#L421-L444: add the equivalent GRDB migration and schema.
📍 Affects 3 files
  • modules/vescape-core/src/index.ts#L173-L178 (this comment)
  • modules/vescape-core/src/index.ts#L272-L277
  • modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt#L475-L505
  • modules/vescape-core/ios/telemetry/TelemetryDatabase.swift#L421-L444
🤖 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 `@modules/vescape-core/src/index.ts` around lines 173 - 178, Replace the
`updatedAt` documentation at modules/vescape-core/src/index.ts:173-178 and
:272-277 to identify it as wall-clock last-write-wins metadata, not the sync
cursor. In
modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt:475-505,
add the 28→29 migration with atomic strictly increasing `sync_sequences`,
per-row `sync_seq` for every syncable write, and scan indexes. Add the
equivalent GRDB migration and schema in
modules/vescape-core/ios/telemetry/TelemetryDatabase.swift:421-444; retain
`updatedAt` for last-write-wins while scans use `sync_seq`.

Comment on lines +15 to +20
/**
* Native owns `updatedAt` (the incremental-sync cursor) and stamps it from its own clock. Rules
* mirrored into local state before that write lands carry this optimistic value until the next
* `load()` replaces them with the persisted rows.
*/
const withLocalCursor = (rule: AlertRuleInput): AlertRule => ({ ...rule, updatedAt: Date.now() })

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Stamp every optimistic rule mutation.

update() and setEnabled() still retain the prior updatedAt locally. Apply withLocalCursor in both paths so the optimistic entity reflects the pending native write.

🤖 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 `@src/modules/alerts/store/alertsStore.ts` around lines 15 - 20, Update the
optimistic mutation paths in update() and setEnabled() to wrap the locally
written alert rule with withLocalCursor before storing it. Ensure both paths
assign a fresh local updatedAt while leaving the persisted/native write behavior
unchanged.

@KacperKozak KacperKozak mentioned this pull request Jul 28, 2026
# Conflicts:
#	docs/agents/issue-tracker.md
#	modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt
#	modules/vescape-core/ios/telemetry/TelemetryDatabase.swift
# Conflicts:
#	modules/vescape-core/android/src/main/java/expo/modules/vescapecore/telemetry/TelemetryDatabase.kt
#	modules/vescape-core/ios/telemetry/TelemetryDatabase.swift
@KacperKozak KacperKozak mentioned this pull request Aug 1, 2026
16 tasks
@KacperKozak KacperKozak mentioned this pull request Aug 1, 2026
37 tasks
@KacperKozak KacperKozak mentioned this pull request Aug 1, 2026
10 tasks
@KacperKozak KacperKozak mentioned this pull request Aug 1, 2026
16 tasks
@KacperKozak KacperKozak mentioned this pull request Aug 7, 2026
KacperKozak added a commit that referenced this pull request Aug 7, 2026
…ize hook

- useBackupSlot reports unavailable for every Rider until #276 lands; offering
  "sign in to back up" promised a capability this build does not have.
- fmtCompactCount picks its suffix after rounding, so 999_999 reads 1.0M not 1000k.
- New useDatabaseSize hook for surfaces that only want the number; the drawer no
  longer mounts backup/restore/rebuild state and its progress listener.
Wire the Settings Drawer to the real uploader: `useBackupSlot` now projects
`useSyncStatusStore` instead of Clerk, `BackupSlot` covers every activity
native reports (off / signedOut / idle / syncing / blocked), and the strip
cell opens Sync settings. A paused backup badges the gear in error red.

Conflicts: iOS `Events` list keeps both `onReplayPhoneHeading` and
`onSyncStatus`; `AppDataRepository` keeps the `writer` rename with dev's
failure logging; `AccountWidget` stays deleted — `AccountPill` owns identity
and the drawer's backup cell replaces its `BackupStatusLine`.
@KacperKozak KacperKozak added area:tech Internal refactor, tech upgrades, no user-visible behavior change area:ios iOS-only work: platform port and iOS-specific native code. Not cross-platform work that touches iOS area:board Board profiles, board table, and board settings area:history Ride history, sessions, buckets, graphs area:telemetry Live telemetry ingest and display area:native Touches native side (modules/vesc-ble, Swift/Kotlin) area:db Touches database / persistent storage area:sync Backup sync — native uploader, Sync Cursors, Sync Actions, Device Token area:auth Clerk sessions, native credentials, and endpoint caller policy area:core App shell, storage, lifecycle, infra area:design Design system, theme, and color tokens complexity:high Critical paths, subtle correctness, native pipelines. Use opus. labels Aug 19, 2026
KacperKozak added a commit that referenced this pull request Aug 31, 2026
* Tombstone deleted Boards, never remove the row

Deleting a Board now stamps boards.deleted_at instead of removing the row, so
Ride History keeps a resolvable Board identity (ADR 0027). Board-owned
configuration is still hard-deleted; telemetry and Tune Profiles are not.

getBoards() filters tombstones, getBoard(id) deliberately resolves them, and
connect paths refuse a tombstoned Board. An ordinary upsert carries an existing
tombstone forward, so deletion is terminal.

Ported from #276 (#279) and the closed #435 (#428), renumbered onto schema 40 -> 41.

* Key telemetry on the Board id, not the BLE identifier

Every telemetry table — frames, minute buckets, markers, diagnostic events and
metric exclusion ranges — now keys on board_id and drops both device_id (the
mutable BLE identifier) and device_name (the Board name denormalized at capture
time). Ride History resolves Board names by lookup, so a rename relabels history
(ADR 0028, closes #274).

Migration 41 -> 42 rebuilds all five tables, resolving each BLE identifier to a
Board exactly once through a shared scratch map so no two tables can pick
different claimants of a duplicated identifier. Rows that resolve to no Board
mint a tombstoned Board named from their historical device_name, so orphaned
history keeps a label and stays joinable.

The boardId -> bleId translation used by markers, events and ranges is deleted.
RideHistoryRepository (added on dev after #276 branched) is rekeyed the same way.

Ported from #276 (#280) with the follow-up dedup and marker/event/range rekey.

* Align the iOS schema stamp and the docs with board-id keying

The iOS backup stamp still claimed schema 41, so a current backup left the
board-id migration unstamped and replayed it on restore — caught by
TelemetryMigrationTests. Also rekeys the native API and history docs, adds both
ADRs to the docs index, and records the Board Tombstone rules in CONTEXT.md.

* Record Board-id keying as a whole-schema rule, not a telemetry one

Every durable Board-owned table now keys on board_id; boards.ble_id is the only
BLE identifier left and is a Board Link attribute, not a join key.

* Fix nested GRDB read crash in Ride History page

boardNamesById() opens its own pool read; calling it inside getPage's read
tripped GRDBPrecondition and killed the reader queue. Hoist it out, like
every other call site already does.
#279 and #280 landed on dev as #437, so the branch's own tombstone and
board-id migrations are dropped and dev's ladder is the one that ships.
The sync migrations move to the tail of it: 42→43 Change Timestamps,
43→44 sync_seq, 44→45 the six remaining tables, 45→46 sync_actions,
46→47 sync_binding. Schema 47 on both platforms.

The branch had taken schema 32 (Room 31→32, GRDB v32) for its first sync
migration and pushed the shipped alert-repeat migration to the tail. Dev
has since spent 32 through 42, so the slot goes back to alert-repeat and
every sync migration is renumbered above dev's.

Rebuilding the buckets on board_id now happens before the sync columns
exist, so the rebuild no longer has to carry them and the assertions that
it does are gone.
faultCount goes out as 0, not null. The server declares it non-nullable
inside a strict schema it validates whole, so a null refused the entire
Sync Batch rather than the field — backup would have wedged on the first
minute bucket. faultCode stays null; that one the server declares
nullable.

SYNC_SEQ_TABLES_V43/V44 renamed to V44/V45 to match the convention that
the constant names the schema version its migration produces.

Comments that had come to describe the opposite of the code: Favorites
have a server table now and the uploader does not drop the case, the
Kotlin cursor test header still named schema 31 and 32, and the decode
caches are maintenance because they are rebuilt on the next read, not
because the Board's action covers them.
The server side of this landed in vescape-server#45; this is the half that
actually puts the rows on the wire.

VESC Fault Occurrences and their Captures now sync. They are the one category of
Rider-visible history that is permanent on the phone — exempt from every
retention sweep, kept after the Board is gone — and backup carried none of it, so
a lost phone lost the entire fault record. Schema 48 gives an Occurrence the
ratcheted `updated_at` it needs to be judged on and both tables a `sync_seq`; a
dismissal moves the stamp, which is the whole reason the column exists. Capture
samples keep their local autoincrement id at home and are identified by
occurrence and capture time, as Tune History already is.

Alert Rules send the four columns that say how their thresholds are read, plus
the repeat cadence and beep count. A config-relative rule was restoring as a bare
fixed rule at a stale number — worse than losing it, because it looks configured.

Board Warnings send `updatedAt`, so the server stops judging them on a detection
time that does not move when a severity is re-triaged.

`faultCode` and `faultCount` are gone from the frame and bucket encoders. The
server dropped both columns, and sending them now refuses the whole batch.

Two wedges, both of which would have stopped backup permanently on a retained row
that no retry can clear:

Metric Exclusion Ranges can carry an empty `board_id` — the sanitizers write it
for samples that match no Board — and unlike the frame and bucket scans this one
had no filter. The server's composite foreign key rejects it and 409s the batch.

Fault Capture samples are the first firmware-sourced floats ever put on the wire,
and a non-finite one would have thrown a permanent protocol error. They now
encode as null through a new `reading` writer: these columns are nullable
precisely because a field the firmware did not send is absent, so an unusable one
is absent too. `number` still refuses non-finite for values a Rider authored.

Also: `syncWifiOnly` joins the settings that never travel, so a restore onto a
cellular-only phone stops inheriting the other phone's data-plan answer; Motor
Config Values are dropped with their Board on both platforms, where iOS had been
keeping them on the mismatch path too; and the `SyncTable` parity tag pointing at
a TypeScript type that never existed is removed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:auth Clerk sessions, native credentials, and endpoint caller policy area:board Board profiles, board table, and board settings area:core App shell, storage, lifecycle, infra area:db Touches database / persistent storage area:design Design system, theme, and color tokens area:history Ride history, sessions, buckets, graphs area:ios iOS-only work: platform port and iOS-specific native code. Not cross-platform work that touches iOS area:native Touches native side (modules/vesc-ble, Swift/Kotlin) area:sync Backup sync — native uploader, Sync Cursors, Sync Actions, Device Token area:tech Internal refactor, tech upgrades, no user-visible behavior change area:telemetry Live telemetry ingest and display complexity:high Critical paths, subtle correctness, native pipelines. Use opus.

Projects

None yet

1 participant