From 984b6af0cc95facb41dac428723b37170a346146 Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 20:21:12 +0000 Subject: [PATCH 01/11] docs: plan for portable event identity (restore to new device) Adds a development plan for the first half of #273: making a restored app database re-associate with the right calendars and events on a new phone. Event rows currently store only device-local autoincrement IDs (cid, id), so an auto-backup restore leaves every event orphaned. The plan captures durable identity at write time -- the calendar account tuple plus the event's iCalendar UID_2445 -- in the unused reserved column s2, avoiding any schema migration, and re-resolves both IDs on a detected restore with retry plus a manual trigger. Also notes an existing bug in ApplicationController.restoreToActive(), which looks up the old calendar ID against the new device's provider and so cannot work cross-device. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- docs/dev_todo/portable_event_identity.md | 218 +++++++++++++++++++++++ 1 file changed, 218 insertions(+) create mode 100644 docs/dev_todo/portable_event_identity.md diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md new file mode 100644 index 00000000..1dcc540d --- /dev/null +++ b/docs/dev_todo/portable_event_identity.md @@ -0,0 +1,218 @@ +# Feature: Portable Event Identity (Restore-to-New-Device Calendar Re-association) + +**GitHub Issue:** [#273](https://github.com/williscool/CalendarNotification/issues/273) — "Data Sync 2.0" + +Scope: only the first half of #273, which the issue author split out explicitly: + +> "One is is it possible to find out how to rearrange or store some new data so that you can restore a backup of the application (maybe just database) on a new phone and it will properly associate with the calendars. If we can do that let's do that." + +The login/multi-device bidirectional sync half is deliberately **not** in this plan. + +## Context + +Android auto-backup already backs up every app database verbatim (`res/xml/backup_rules.xml` includes `domain="database" path="."`). So the event rows *do* arrive on a new phone. The problem is that what they store is **device-local numeric IDs that mean nothing on the new device**: + +- `eventsV9.cid` — the calendar's `CalendarContract.Calendars._ID` +- `eventsV9.id` — the event's `CalendarContract.Events._ID` + +Both are autoincrement row IDs assigned by the new device's Calendar Provider when it re-syncs from Google. They will not match. The result on a restored phone: snoozed/active events survive in the list but are orphaned — tapping one opens the wrong event or falls back to a time view, calendar filter pills don't match, and per-calendar "handled" settings silently revert to their `true` default. + +The building blocks already exist but are wired into the wrong places: + +| Building block | Where it lives | Currently used for | +|---|---|---| +| `CalendarBackupInfo` (account/type/owner/displayName/name) | `calendar/CalendarBackupInfo.kt` | settings JSON export only | +| `getCalendarBackupInfo()` / `findMatchingCalendarId()` (3-tier fallback match) | `calendar/CalendarProvider.kt:1765-1851` | settings import + single-event un-dismiss | +| `CalendarSettingBackup` durable tuple | `backup/BackupData.kt:29-37` | settings JSON export only | +| `CalendarRecord` (already carries owner/accountName/accountType/name) | `calendar/CalendarRecord.kt` | **never persisted** — only `calendarId` is | + +Note `ApplicationController.restoreToActive()` (`app/ApplicationController.kt:1321-1325`) already *tries* to do the right thing, but it is structurally broken for the cross-device case: + +```kotlin +val calendarBackupInfo = calendarProvider.getCalendarBackupInfo(context, event.calendarId) +``` + +It looks up the **old** calendar ID against the **new** device's provider — which returns `null` — then falls back to the stale ID. The identity must be captured at write time on the old device, not recovered at restore time on the new one. + +## Goal + +Store durable, provider-independent identity alongside every stored event so that a database restored onto a new phone can re-resolve itself to the correct local calendar and event rows. After this, restoring a backup (or just letting Android auto-backup do its thing) yields a working event list: correct calendar attribution, working filter pills, working "open in calendar", and correct per-calendar handled settings. + +## Non-Goals + +- **Login / accounts (Google, Zitadel)** — the other half of #273; tracked separately. +- **Bidirectional multi-device sync of snooze/mute state** — the second half of #273. This plan makes the *data* portable; it does not make it live-shared. +- **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. +- **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. +- **Schema migration** — explicitly avoided; see Key Decisions. +- **Re-resolving `MonitorStorage` alerts** — `MonitorAlertEntity.toAlertEntry()` already drops `calendarId` entirely and the monitor table is short-lived scan state, rebuilt from the provider. Not worth carrying identity there. + +## Key Decisions Summary + +| Decision | Choice | Rationale | +|---|---|---| +| Where to store identity | Existing **unused reserved columns** (`eventsV9.s2`, `dismissedEventsV2.s2`) as a small JSON blob | Zero schema migration, zero Room version bump, zero risk to the cr-sqlite/PowerSync column contract. These columns are written as `""` today and read by nothing. | +| Calendar identity | Reuse `CalendarBackupInfo` (account name/type, owner, displayName, name) | Already exists, already has a tested 3-tier fallback matcher, already the proven format in settings backup. | +| Event identity | `Events.UID_2445`, falling back to `Events._SYNC_ID` | `UID_2445` is the iCalendar UID — globally stable and identical across devices for the same Google/CalDAV event. Available since API 17; minSdk is 24. | +| When identity is captured | On every event write (add/update), best-effort | Cheap, keeps identity fresh, and means any future backup is restorable without a migration pass. | +| When re-resolution runs | Lazily, on a detected restore, **retrying** until resolved — plus a manual trigger | Calendars often sync onto the phone *after* our first launch, so a one-shot pass would match nothing. | +| Manual trigger UX | Mirror the existing pull-to-refresh + overflow "Refresh" in `prefs/CalendarsActivity.kt` | The user explicitly asked for "a way to manually invoke also like the refresh behavior in the handled calendars ui". That screen already does `ContentResolver.requestSync` then reloads — same shape. | +| Unmatched events | Leave the row intact with its stale ID, keep the identity blob, mark unresolved | Matches the existing fail-soft convention (`reloadCalendarEventAlertFromEvent` returns `NoChange` rather than deleting). Never destroy user data because a match failed. | +| Restore detection | Compare a stored install fingerprint against the current one | Cheap and reliable. Auto-backup deliberately excludes `events_storage_state.xml`, so a prefs-based marker is a proven pattern here. | + +## Design Decisions + +### Why reserved columns rather than a schema migration + +`EventAlertEntity` (`eventsstorage/EventAlertEntity.kt`) declares `i2`–`i8` and `s2` as reserved, and `EventsStorageImplV9` writes them as `0`/`""`. Nothing reads them. Using `s2` means: + +- No Room version bump, no new `Migration`, no new legacy `EventsStorageImplV10`. +- The Supabase table (`supabase/migrations/20250301213237_events.sql`) already has an `s2` column, so the sync payload keeps working unchanged. +- `installCrsqliteOnTable` in `src/lib/cr-sqlite/install.ts` rewrites the PK but doesn't enumerate columns — unaffected. + +The tradeoff is that `s2` becomes semantically meaningful, so it needs a named constant and a comment at the entity declaration rather than staying "reserved". That's a documentation cost, not a correctness one. + +### Why `UID_2445` and not just re-matching on title+time + +Title+time heuristics produce false positives on recurring and duplicated events, and break when the user edits a title. `UID_2445` is the iCalendar UID that Google/CalDAV sync assigns; it is the same string on every device that syncs that event. It's the only genuinely stable event identifier Android exposes. + +Caveat worth stating up front: **locally-created events that have never synced to an account may have a null/empty `UID_2445`.** Those events are also the ones least likely to exist on the new phone at all (a local-only calendar isn't restored by Google). They degrade to unresolved and keep their stale ID — no crash, no data loss. + +### Resolution strategy + +A restored event needs two lookups, in order: + +1. **Calendar**: `findMatchingCalendarId(context, storedBackupInfo)` → new `cid`. Reuses the existing 3-tier matcher untouched. +2. **Event**: query `Events.CONTENT_URI` for `UID_2445 = ? AND CALENDAR_ID = ?` (scoped to the just-matched calendar to avoid cross-calendar collisions) → new `id`. + +Because `(id, istart)` is the primary key of `eventsV9`, changing `id` is a **delete + re-insert**, not an update. That is the single riskiest operation in this plan, so it must be transactional per-event and must not run while the row is being mutated elsewhere. `instanceStartTime` is derived from the event's actual start time and is stable across devices for the same instance, so it carries over unchanged. + +A PK collision is possible if the new `(id, istart)` already exists (e.g. the new device independently re-added the same event). In that case, keep the existing row and drop the restored duplicate — the live row is the more trustworthy one. + +## Current Architecture + +Identity flow today, and where it breaks on restore: + +| Layer | Event identity | Calendar identity | Survives restore? | +|---|---|---|---| +| `eventsV9` / `dismissedEventsV2` | PK `(id, istart)` | payload `cid`, unindexed, `-1` = unknown | ❌ both stale | +| Per-calendar prefs | n/a | SharedPrefs key `calendar_handled_.` | ❌ orphaned, defaults to `true` | +| Open in calendar app | `ContentUris.withAppendedId(Events.CONTENT_URI, eventId)` (`calendar/CalendarIntents.kt:37,45`) | not used | ❌ wrong/missing event | +| Reload/refresh | `getEvent(eventId)`, `getAlertByEventIdAndTime(eventId, alertTime)` (`app/CalendarReloadManager.kt`) | not used | ❌ degrades to `NoChange` | +| Settings JSON export | n/a | `CalendarSettingBackup` account tuple | ✅ already durable | + +The key insight: the calendar half of this problem was already solved once for settings. This plan applies the same idea one layer down, to the event rows themselves, and adds the event half. + +## Implementation Plan + +### Phase 0: Capture identity at write time + +**0a — Identity model.** New `calendar/PortableEventIdentity.kt`: a small serializable type holding the `CalendarBackupInfo` fields plus `eventUid` and the originating `calendarId`/`eventId`. Include a schema `version` field for forward compatibility. Serialize with kotlinx.serialization (already a dependency, used by `backup/BackupData.kt`) to a compact JSON string. + +**0b — Read the UID from the provider.** Add `Events.UID_2445` (with `_SYNC_ID` fallback) to the projection in `CalendarProvider.getEvent()` (`calendar/CalendarProvider.kt:418-439`) and expose it on `EventRecord`. Keep it nullable — not every event has one. + +**0c — Persist it.** Map the blob into `EventAlertEntity.s2` / `DismissedEventEntity.s2` in `fromRecord()`/`toRecord()`, and mirror it in `EventsStorageImplV9` and `DismissedEventsStorageImplV2` so the legacy fallback path doesn't silently drop it. Populate on add/update in `ApplicationController` where the record is first built from the provider. + +**Checkpoint:** new events written on this device carry a populated `s2`. Existing rows still have `""` — that's expected and handled in Phase 2. + +### Phase 1: Resolution engine + +New `calendar/EventIdentityResolver.kt` — pure orchestration, no UI, constructor-injected `CalendarProviderInterface` and `CNPlusClockInterface` so it's Robolectric-testable (per `docs/testing/dependency_injection_patterns.md`). + +Responsibilities: +- Given a stored record + its identity blob, resolve `(newCalendarId, newEventId)`. +- Report a typed outcome: resolved / unresolved-calendar / unresolved-event / no-identity-stored / already-current. +- Apply the resolution to storage, handling the delete+reinsert for a changed PK and the collision case from Design Decisions. + +This class is the whole substance of the feature; keep it small and free of Android UI dependencies. + +### Phase 2: Backfill for pre-existing rows + +Rows written before Phase 0 have an empty `s2`. While the app is still on the *original* device, those rows can be backfilled by reading the identity from the live provider (the stale IDs are still valid here). Run this opportunistically on app start when unbackfilled rows exist. + +This is what makes the feature useful to the current user rather than only to new installs — without it, today's data is still unrestorable. + +### Phase 3: Restore detection + retry + +Store an install fingerprint in its own SharedPreferences file, and **exclude that file from `backup_rules.xml`** so it does not survive a restore — the same trick `EventsStorageState` already relies on. Absent/mismatched fingerprint on launch ⇒ treat as a restore and mark all events pending re-resolution. + +Retry semantics (per the user's answer): keep pending events marked until each resolves, re-attempting on app start and after calendar rescans, rather than burning the attempt once. Cap attempts with a backoff so a permanently-unmatchable event doesn't re-query forever. + +### Phase 4: Manual trigger + +Mirror `prefs/CalendarsActivity.kt:196-243`: a "Re-link events to calendars" action that requests a calendar sync, waits, then runs the resolver and reports counts (`resolved / unresolved`), reusing the `ImportStats`-style feedback shape from `backup/SettingsBackupManager.kt`. Placement next to the existing export/import entries in `prefs/MiscSettingsFragmentX.kt` is the natural home. + +### Phase 5: Per-calendar settings repair + +On a detected restore, rewrite orphaned `calendar_handled_.` keys to their new IDs using the same matcher. `SettingsBackupManager.importCalendarSettings()` (`backup/SettingsBackupManager.kt:388-435`) already does exactly this from a JSON file — the logic should be extracted and shared rather than duplicated. + +## Files to Modify/Create + +### New Files + +| File | Purpose | +|---|---| +| `calendar/PortableEventIdentity.kt` | Serializable identity blob + JSON encode/decode | +| `calendar/EventIdentityResolver.kt` | Resolution engine and typed outcomes | +| `test/.../calendar/EventIdentityResolverRobolectricTest.kt` | Core resolution logic tests | +| `test/.../calendar/PortableEventIdentityTest.kt` | Pure serialization round-trip tests | +| `androidTest/.../calendar/EventIdentityRestoreTest.kt` | Real-provider end-to-end | + +### Modified Files + +| File | Changes | +|---|---| +| `calendar/CalendarProvider.kt` | Add `UID_2445`/`_SYNC_ID` to `getEvent()` projection; add a lookup-by-UID query | +| `calendar/CalendarProviderInterface.kt` | Declare the new lookup | +| `calendar/EventRecord.kt` | Carry nullable `eventUid` | +| `eventsstorage/EventAlertEntity.kt` | Name `s2` as the identity column; map in `fromRecord`/`toRecord` | +| `eventsstorage/EventsStorageImplV9.kt` | Mirror the mapping on the legacy path | +| `dismissedeventsstorage/DismissedEventEntity.kt`, `DismissedEventsStorageImplV2.kt` | Same, for dismissed events | +| `app/ApplicationController.kt` | Populate identity on write; **fix `restoreToActive()` (line 1321) to use the stored blob instead of re-querying the stale ID** | +| `backup/SettingsBackupManager.kt` | Extract calendar-remap logic for reuse in Phase 5 | +| `prefs/MiscSettingsFragmentX.kt` | Manual re-link action | +| `res/xml/backup_rules.xml` | Exclude the new fingerprint prefs file | +| `res/values/strings.xml` | Strings for the action + result dialog | + +## Testing Plan + +Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCalendarProvider.kt:204-224`) already stubs `getCalendarBackupInfo` and `findMatchingCalendarId`, so the Robolectric path is mostly already scaffolded; it needs a UID-lookup stub added. + +### Unit / Robolectric + +- **Serialization**: round-trip; unknown future `version` decodes without throwing; malformed/empty `s2` yields null rather than an exception (no broad `catch (Exception)` — catch `SerializationException` specifically). +- **Resolver happy path**: calendar and event both match → new IDs applied. +- **Partial match**: calendar matches, UID does not → calendar updated, event left stale, marked unresolved. +- **No match**: neither matches → row untouched, still marked pending (proves the retry path and the no-data-loss guarantee). +- **No identity stored**: legacy row with empty `s2` → skipped cleanly. +- **Already current**: IDs unchanged → no write (guards against pointless delete+reinsert churn). +- **PK collision**: target `(id, istart)` already occupied → existing row kept, duplicate dropped. +- **Backfill**: pre-existing row + live provider → `s2` populated. +- **Settings repair**: orphaned `calendar_handled_.N` keys remapped; unmatched ones reported. +- **Restore detection**: fingerprint absent/mismatched ⇒ restore; matching ⇒ no-op. + +Follow the existing pattern in `test/.../calendar/CalendarBackupRestoreRobolectricTest.kt` (injected storage, no native SQLite). + +### Instrumentation + +Only for what needs the real Calendar Provider and real cr-sqlite: + +- Create a calendar + event via the real provider, capture identity, simulate a restore by deleting and recreating the calendar/event under new IDs, and assert the resolver re-links correctly. +- Assert the delete+reinsert PK change survives a real `RoomEventsStorage` round-trip. +- Use the unique-suffix isolation technique from `docs/dev_completed/calendar_backup_restore_test_isolation.md` — that doc exists precisely because calendar IDs drift between runs. + +Per `docs/build/wsl_unison_environment.md`, instrumentation runs from Windows (`C:\dev\CN`), filtered with `-Pandroid.testInstrumentationRunnerArguments.class=...`, **not** `--tests`. + +## Verification + +1. **Unit/Robolectric** (from WSL): + `cd android && ./gradlew -PBUILD_ARCH="x86_64" -PreactNativeArchitectures="x86_64" :app:testX8664DebugUnitTest` +2. **Instrumentation** (from Windows, after the user runs Unison — *never* run it unprompted): + `.\gradlew.bat :app:connectedX8664DebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.github.quarck.calnotify.calendar.EventIdentityRestoreTest` +3. **Real end-to-end restore** using the existing harness `scripts/test_cloud_backup.sh com.github.quarck.calnotify` — it drives `bmgr` through a real backup/uninstall/reinstall cycle. Before: restored events are orphaned. After: they re-link. This is the actual acceptance test for the issue. +4. **Manual sanity**: with events snoozed, trigger the manual re-link action and confirm the reported resolved/unresolved counts, filter pills, and "open in calendar" all behave. + +## Open Questions + +- Should a restored-but-unresolved event be visually marked in the list (e.g. the existing `calendarId = -1` "calendar not found" treatment via `createCalendarNotFoundCal`), or stay silent until it resolves? Leaning silent, since the retry usually resolves it within a sync cycle or two. +- `DismissedEventsStorage` carries the identity blob for symmetry, but dismissed events are historical. Worth confirming whether re-resolving them is wanted at all, or whether Phase 0's capture is enough there. From 43f1d71df09a386d4be2da00779913f187b83003 Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 20:41:58 +0000 Subject: [PATCH 02/11] docs: explain UID_2445 vs the account email, add diagrams Clarifies why the calendar's account email alone can't solve the restore problem. There are two stale IDs: cid (which calendar) and id (which event in it). The account tuple resolves the first; every event in a calendar shares that same email, so it can't resolve the second. UID_2445 is the RFC 5545 iCalendar UID -- server-assigned, so it's identical on every device that syncs the event, unlike _ID which is a local SQLite row number. Adds three mermaid diagrams (matching the existing docs/architecture convention): the two-halves split, why local _ID diverges across devices, and the full resolution flow with its failure paths. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- docs/dev_todo/portable_event_identity.md | 77 +++++++++++++++++++++++- 1 file changed, 74 insertions(+), 3 deletions(-) diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index 1dcc540d..7dea732b 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -72,11 +72,59 @@ Store durable, provider-independent identity alongside every stored event so tha The tradeoff is that `s2` becomes semantically meaningful, so it needs a named constant and a comment at the entity declaration rather than staying "reserved". That's a documentation cost, not a correctness one. -### Why `UID_2445` and not just re-matching on title+time +### The two halves: why the email is necessary but not sufficient -Title+time heuristics produce false positives on recurring and duplicated events, and break when the user edits a title. `UID_2445` is the iCalendar UID that Google/CalDAV sync assigns; it is the same string on every device that syncs that event. It's the only genuinely stable event identifier Android exposes. +There are **two** broken IDs, and the account email only fixes one of them. -Caveat worth stating up front: **locally-created events that have never synced to an account may have a null/empty `UID_2445`.** Those events are also the ones least likely to exist on the new phone at all (a local-only calendar isn't restored by Google). They degrade to unresolved and keep their stale ID — no crash, no data loss. +| What's stale | Example | What identifies it durably | +|---|---|---| +| `cid` — *which calendar* | `3` | the account tuple (email + type + owner) | +| `id` — *which event in it* | `91427` | the event's iCalendar UID | + +The email answers *"which calendar is this?"* — that's `findMatchingCalendarId()`, already written and already used by settings backup. It does **not** answer *"which of the 800 events in that calendar is this row?"* Every event in your work calendar shares the same email, so the email narrows 800 events down to 800 events. + +```mermaid +flowchart TD + A["Restored row
cid=3, id=91427"] --> B{"Which calendar?"} + B -->|"account tuple
(the email)"| C["cid = 12 ✅"] + C --> D{"Which event
inside it?"} + D -->|"the email again"| E["800 candidates ❌
they all share it"] + D -->|"UID_2445"| F["id = 55310 ✅
exactly one"] +``` + +So the plan uses **both**: the email tuple to find the calendar, then the UID to find the event within it. + +### What `UID_2445` actually is + +It's the [RFC 5545](https://datatracker.ietf.org/doc/html/rfc5545#section-3.8.4.7) iCalendar `UID` property — the identifier the calendar *format* uses, as opposed to `_ID`, which is just a row number in the local SQLite database. The `2445` is a fossil: RFC 2445 was the original iCalendar spec, obsoleted by 5545, but Android kept the constant name. It's `CalendarContract.Events.UID_2445`, present since API 17 (your minSdk is 24), and holds a string like: + +``` +040000008200E00074C5B7101A82E00800000000B0F2C8B5A1D9DA01000000000000000 +``` + +or, on Google Calendar, typically something closer to `abc123def456@google.com`. + +The key property: **the server assigns it, so it's the same string on every device that syncs that event.** `_ID` is assigned locally by whichever device happened to insert the row first, which is exactly why it doesn't survive a restore. + +```mermaid +flowchart LR + G["Google Calendar
UID abc123@google.com"] --> P1["Old phone
_ID 91427"] + G --> P2["New phone
_ID 55310"] + P1 -.->|"backup restores
_ID 91427"| P2 + P2 --> X["91427 doesn't exist here ❌
UID abc123 does ✅"] +``` + +### Why not title + time instead + +Title+time heuristics produce false positives on recurring and duplicated events (a weekly standup is many rows with identical titles), and break the moment the user edits a title. The UID survives renames, reschedules, and recurrence. + +### Caveats + +**Locally-created events that never synced to an account may have a null/empty `UID_2445`.** Those events are also the ones least likely to exist on the new phone at all — a local-only calendar isn't restored by Google. They degrade to unresolved and keep their stale ID: no crash, no data loss. + +`_SYNC_ID` is the fallback when `UID_2445` is empty. It's also server-assigned and stable, but it's the sync adapter's own key rather than the portable iCalendar one, so it's second choice. + +**The email tuple is not perfectly unique either** — this is why `findMatchingCalendarId()` already has three tiers. One account can expose several calendars (primary, birthdays, a shared team calendar), all with the same `ACCOUNT_NAME`. That's why the match uses account name + type + owner, and only falls back to display name. ### Resolution strategy @@ -89,6 +137,29 @@ Because `(id, istart)` is the primary key of `eventsV9`, changing `id` is a **de A PK collision is possible if the new `(id, istart)` already exists (e.g. the new device independently re-added the same event). In that case, keep the existing row and drop the restored duplicate — the live row is the more trustworthy one. +Every failure path below leaves the row intact and retryable — nothing is ever deleted because a match failed: + +```mermaid +flowchart TD + A["Stored event row"] --> B{"Identity blob
in s2?"} + B -->|"no (pre-Phase 0)"| Z["Skip — backfill handles it"] + B -->|yes| C["findMatchingCalendarId
(account tuple)"] + + C --> D{"Calendar
matched?"} + D -->|no| Y["Unresolved — keep row,
retry next launch"] + D -->|yes| E["Query Events for
UID_2445 in that calendar"] + + E --> F{"Event
matched?"} + F -->|no| X["Update cid only,
mark unresolved, retry"] + F -->|yes| G{"IDs already
current?"} + + G -->|yes| W["No write — done"] + G -->|no| H{"Target (id, istart)
already taken?"} + + H -->|yes| V["Keep live row,
drop restored duplicate"] + H -->|no| U["Delete + re-insert
under new id ✅"] +``` + ## Current Architecture Identity flow today, and where it breaks on restore: From 0a67471df68dd75ee0366d36a55284c7e3e8757b Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 21:10:55 +0000 Subject: [PATCH 03/11] docs: schema reference; clarify the risky write and its blast radius Resolution strategy was vague about which write is actually dangerous. Separates them: cid is a plain column (safe in-place UPDATE), while id is half the primary key, so changing it forces delete + re-insert. Sequences the safe write first and commits it, so a failed event lookup still leaves the row strictly improved. Found on review: MonitorStorage and DismissedEventsStorage are keyed on eventId in separate databases, so an id change orphans their rows -- which would break restoreToUpcoming, since clearWasHandled aborts when the alert is missing. Splits Phase 1 into 1a (cid only) and 1b (id re-key across all three DBs with manual rollback). Corrects the non-goal that wrongly excluded the monitor DB entirely. Documents the calendar sync window: Google syncs ~12 months back, and there is no API to widen it (requestSync takes no date range). Bounded impact, since MAX_SCAN_BACKWARD_DAYS is 31 -- a limit on the tail, not the feature. Adds database_schema_reference.md covering all three databases, since this is the first deliberate claim on a reserved column. Sets the rule that claiming one requires renaming and documenting it, and records the existing i1=flags / s1=description precedents. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- .../architecture/database_schema_reference.md | 114 ++++++++++++++++++ docs/dev_todo/portable_event_identity.md | 85 +++++++++++-- 2 files changed, 190 insertions(+), 9 deletions(-) create mode 100644 docs/architecture/database_schema_reference.md diff --git a/docs/architecture/database_schema_reference.md b/docs/architecture/database_schema_reference.md new file mode 100644 index 00000000..22ee98a3 --- /dev/null +++ b/docs/architecture/database_schema_reference.md @@ -0,0 +1,114 @@ +# Database Schema Reference + +Column-by-column reference for the app's three SQLite databases. Column names are abbreviated for historical reasons (the 2016 schema used short names to save space), so this table is the map between the on-disk name and what it actually means. + +Authoritative definitions live in the Room entities; this doc is the human-readable index: + +- `eventsstorage/EventAlertEntity.kt` +- `dismissedeventsstorage/DismissedEventEntity.kt` +- `monitorstorage/MonitorAlertEntity.kt` + +## Databases + +| Database | Room file | Legacy file | Table | Primary key | +|---|---|---|---|---| +| Events (active/snoozed) | `RoomEvents` | `Events` | `eventsV9` | `(id, istart)` | +| Dismissed events | `RoomDismissedEvents` | `DismissedEvents` | `dismissedEventsV2` | `(eventId, instanceStart)` | +| Calendar monitor | `RoomCalendarMonitor` | `CalendarMonitor` | `manualAlertsV1` | `(eventId, alertTime, instanceStart)` | + +All three are **separate database files**. There are no foreign keys between them and cross-database transactions are not possible — code that must stay consistent across two of them does manual rollback (see `ApplicationController.unsnoozeToUpcoming`). + +## `eventsV9` — active and snoozed events + +The short column names here are the ones most likely to confuse. Note especially `attsts`/`oattsts`, which are easy to read backwards. + +| Column | Field | Type | Notes | +|---|---|---|---| +| `cid` | `calendarId` | Long | CalendarContract `Calendars._ID`. **Device-local.** `-1` = unknown, treated as handled (fail-open) | +| `id` | `eventId` | Long | CalendarContract `Events._ID`. **Device-local.** PK part 1 | +| `istart` | `instanceStartTime` | Long | PK part 2. Distinguishes occurrences of a recurring event | +| `iend` | `instanceEndTime` | Long | | +| `estart` | `startTime` | Long | The *event's* start, vs the *instance's* start above | +| `eend` | `endTime` | Long | | +| `altm` | `alertTime` | Long | When the reminder fires | +| `nid` | `notificationId` | Int | Android notification ID. Independent of `id` | +| `ttl` | `title` | String | | +| `s1` | `description` | String | A string column that is *not* reserved despite the name | +| `loc` | `location` | String | | +| `snz` | `snoozedUntil` | Long | `0` = not snoozed | +| `ls` | `lastStatusChangeTime` | Long | | +| `dsts` | `displayStatus` | Int | `EventDisplayStatus` enum | +| `clr` | `color` | Int | | +| `rep` | `isRepeating` | Int | 0/1 | +| `alld` | `isAllDay` | Int | 0/1 | +| `ogn` | `origin` | Int | `EventOrigin` enum | +| `fsn` | `timeFirstSeen` | Long | | +| `attsts` | **`eventStatus`** | Int | `EventStatus` enum — confirmed/tentative/canceled | +| `oattsts` | **`attendanceStatus`** | Int | `AttendanceStatus` enum — the user's RSVP | +| `i1` | `flags` | Long | Bitfield: `IS_MUTED=1`, `IS_TASK=2`, `IS_ALARM=4`, `IS_PINNED=8` | +| `i2`–`i8` | *reserved* | Long | Unused; written as `0` | +| `s2` | *reserved* | String | Unused; written as `""`. **Claimed by the portable-identity work** — see below | + +## `dismissedEventsV2` — dismissal history + +Mirrors `eventsV9` but uses **long column names**, so don't copy-paste column lists between the two. + +Same-meaning columns with different names: `eventStart`/`eventEnd` (vs `estart`/`eend`), `instanceStart`/`instanceEnd` (vs `istart`/`iend`), `snoozeUntil` (vs `snz`), `displayStatus` (vs `dsts`), `title`/`location`/`color`/`isRepeating`/`allDay` spelled out. `s1` is again `description`. + +Additional columns: + +| Column | Field | Notes | +|---|---|---| +| `dismissTime` | `dismissTime` | | +| `dismissType` | `dismissType` | `EventDismissType` enum | +| `lastSeen` | `lastStatusChangeTime` | Same field as `ls` in `eventsV9`, different name | + +Reserved: `i2`–`i9`, `s2`, `s3`. Does **not** carry `eventStatus`/`attendanceStatus`/`timeFirstSeen`/`origin`. + +## `manualAlertsV1` — calendar monitor scan state + +Short-lived bookkeeping for alerts the app discovered by scanning, rebuilt from the provider. + +| Column | Field | Notes | +|---|---|---| +| `calendarId` | `calendarId` | **Written but never read** — `toAlertEntry()` drops it, and `MonitorEventAlertEntry` has no such field | +| `eventId` | `eventId` | PK part 1 | +| `alertTime` | `alertTime` | PK part 2 | +| `instanceStart` | `instanceStartTime` | PK part 3 | +| `instanceEnd` | `instanceEndTime` | | +| `allDay` | `isAllDay` | | +| `alertCreatedByUs` | `alertCreatedByUs` | Distinguishes our synthetic alerts from provider ones | +| `wasHandled` | `wasHandled` | Whether we've already fired for this alert | +| `i1`, `i2` | *reserved* | | + +## On the reserved columns + +Every table carries spare `iN`/`sN` columns from the original 2016 schema. They are written as `0`/`""` and read by nothing. + +They exist so a field can be added **without a schema migration** — no Room version bump, no new legacy `EventsStorageImplV10`, and no change to the Supabase table (`supabase/migrations/20250301213237_events.sql` already mirrors every column, reserved ones included), which keeps the PowerSync payload working untouched. + +The cost is that the name stops describing the content: a column called `s2` holding calendar identity is opaque to anyone reading a raw DB dump. So the rule is: + +> **When you claim a reserved column, rename the constant to describe its meaning, document it in this file, and leave a comment at the entity declaration. Never leave a live column named "reserved".** + +Note the precedent: `i1` was already claimed for `flags`, and `s1` for `description`. Both are documented above rather than left as mysteries. + +### Currently claimed + +| Table | Column | Claimed by | Status | +|---|---|---|---| +| `eventsV9` | `i1` | `flags` bitfield | in use | +| `eventsV9` | `s1` | `description` | in use | +| `eventsV9` | `s2` | portable event identity | planned — see [portable_event_identity.md](../dev_todo/portable_event_identity.md) | +| `dismissedEventsV2` | `i1` | `flags` bitfield | in use | +| `dismissedEventsV2` | `s1` | `description` | in use | + +## Identity caveat + +`cid` and `id` are both **device-local autoincrement row IDs** assigned by that device's Calendar Provider. They are meaningless on any other device, which is why a restored database cannot re-associate with calendars without extra stored identity. See [portable_event_identity.md](../dev_todo/portable_event_identity.md). + +## Related + +- [domain_model.md](./domain_model.md) — the domain types these rows map to +- [storage_lifecycle.md](./storage_lifecycle.md) — Room vs legacy, and the `.use {}` pattern +- [../dev_completed/room_database_migration.md](../dev_completed/room_database_migration.md) — the copy-based migration strategy diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index 7dea732b..0835cde6 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -45,7 +45,7 @@ Store durable, provider-independent identity alongside every stored event so tha - **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. - **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. - **Schema migration** — explicitly avoided; see Key Decisions. -- **Re-resolving `MonitorStorage` alerts** — `MonitorAlertEntity.toAlertEntry()` already drops `calendarId` entirely and the monitor table is short-lived scan state, rebuilt from the provider. Not worth carrying identity there. +- **Storing identity blobs in `MonitorStorage`** — `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so it doesn't need its own identity. **But its rows are keyed on `eventId` and must still be re-keyed when an `id` changes** — see the cross-database note in Design Decisions. (Corrected on review; the original plan wrongly treated the monitor DB as entirely out of scope.) ## Key Decisions Summary @@ -72,6 +72,8 @@ Store durable, provider-independent identity alongside every stored event so tha The tradeoff is that `s2` becomes semantically meaningful, so it needs a named constant and a comment at the entity declaration rather than staying "reserved". That's a documentation cost, not a correctness one. +There's precedent: `i1` was already claimed for the `flags` bitfield and `s1` for `description`, so this is the third such claim rather than a new practice. To stop the cost compounding, [database_schema_reference.md](../architecture/database_schema_reference.md) now documents every column in all three databases and sets the rule — **when you claim a reserved column, rename the constant to describe its meaning and document it there; never leave a live column named "reserved."** + ### The two halves: why the email is necessary but not sufficient There are **two** broken IDs, and the account email only fixes one of them. @@ -133,9 +135,40 @@ A restored event needs two lookups, in order: 1. **Calendar**: `findMatchingCalendarId(context, storedBackupInfo)` → new `cid`. Reuses the existing 3-tier matcher untouched. 2. **Event**: query `Events.CONTENT_URI` for `UID_2445 = ? AND CALENDAR_ID = ?` (scoped to the just-matched calendar to avoid cross-calendar collisions) → new `id`. -Because `(id, istart)` is the primary key of `eventsV9`, changing `id` is a **delete + re-insert**, not an update. That is the single riskiest operation in this plan, so it must be transactional per-event and must not run while the row is being mutated elsewhere. `instanceStartTime` is derived from the event's actual start time and is stable across devices for the same instance, so it carries over unchanged. +#### Which write is actually dangerous + +Worth separating, because the two updates carry very different risk: + +- **`cid` is a plain column.** Changing it is an ordinary in-place `UPDATE` — not destructive, not a PK change. Nothing is deleted. +- **`id` is half the primary key.** Room's `@Update` matches *on* the PK, so it structurally cannot change it. Changing `id` means **delete + re-insert**, and that's the one genuinely dangerous operation here. + +So the sequencing rule is: **update `cid` first and commit it.** Even if the event lookup then fails, the row has strictly improved — the calendar is now correct, filter pills work, and per-calendar settings apply. Nothing is risked to gain that. + +#### Why we can afford to be bold about it + +Your instinct that a restored row is "already broken" is the right frame, and it's what makes this tractable. A row whose `id` points at a nonexistent event is already inert: it can't be opened, it can't be reloaded, `reloadCalendarEventAlertFromEvent` degrades it to `NoChange` forever. **Rewriting it can only move it from broken toward working.** + +But "already broken" is only true when the ID is genuinely stale. It is *not* true if we're wrong about that — and a false positive would delete a live, working row. So the delete+re-insert is gated on having positively identified the replacement first: + +1. Resolve the new `id` **before touching storage**. No match → no write at all; the row stays exactly as it is. +2. Only if a new `id` is found, and it differs from the current one, perform delete+insert **inside a single transaction** (`RoomEventsStorage` already uses `runInTransaction`/`beginTransaction` throughout). +3. Never delete without a successful insert in the same transaction. A crash mid-way rolls back to the original row. + +`instanceStartTime` carries over unchanged — it's derived from the event's actual start time and is stable across devices for the same instance. + +#### The collision case + +If the new `(id, istart)` already exists, the new device independently re-added the same event. Keep the existing row and drop the restored duplicate: the live row is the one the app has actually been maintaining. This is a genuine merge decision, not an error. + +#### Cross-database fallout (found while reviewing this) + +`MonitorStorage` lives in a **separate database** and is keyed `(eventId, alertTime, instanceStart)`. Changing `id` in `eventsV9` therefore orphans the matching monitor alert, and there are no cross-database transactions to lean on. + +This matters concretely: `restoreToUpcoming` (`ApplicationController.kt:1293-1300`) aborts when `clearWasHandled` can't find the alert, specifically to prevent data loss. An orphaned monitor row would make un-dismissing such an event fail. -A PK collision is possible if the new `(id, istart)` already exists (e.g. the new device independently re-added the same event). In that case, keep the existing row and drop the restored duplicate — the live row is the more trustworthy one. +The same applies to `dismissedEventsV2`, which is keyed on `eventId` in its own database. + +So the resolver must re-key **all three** databases for a given event, in a defined order, with manual rollback on partial failure — the pattern `unsnoozeToUpcoming` already establishes. This is the main thing that grew in scope on review, and it argues for doing `eventsV9` first and treating the other two as follow-on steps with their own tests. Every failure path below leaves the row intact and retryable — nothing is ever deleted because a match failed: @@ -147,19 +180,40 @@ flowchart TD C --> D{"Calendar
matched?"} D -->|no| Y["Unresolved — keep row,
retry next launch"] - D -->|yes| E["Query Events for
UID_2445 in that calendar"] + D -->|yes| S["UPDATE cid — safe,
commit now ✅"] + S --> E["Query Events for
UID_2445 in that calendar"] E --> F{"Event
matched?"} - F -->|no| X["Update cid only,
mark unresolved, retry"] - F -->|yes| G{"IDs already
current?"} + F -->|no| X["Stop — cid gain kept,
mark unresolved, retry"] + F -->|yes| G{"id already
current?"} G -->|yes| W["No write — done"] G -->|no| H{"Target (id, istart)
already taken?"} H -->|yes| V["Keep live row,
drop restored duplicate"] - H -->|no| U["Delete + re-insert
under new id ✅"] + H -->|no| U["Transaction:
delete + re-insert,
re-key monitor + dismissed ✅"] ``` +### How far back does the new device's calendar actually go? + +A real constraint on how much this feature can ever recover, and worth stating plainly because it bounds expectations. + +**Google Calendar syncs roughly the past 12 months and the next 12 months** to the device's Calendar Provider. Older events exist on the server but are simply not present locally — they're reachable only via calendar.google.com. ([Google Calendar Help](https://support.google.com/calendar/answer/6261951?hl=en&co=GENIE.Platform%3DAndroid), [aCalendar's writeup](https://acalendar.tapirapps.de/en/support/solutions/articles/36000013393-past-future-events-are-missing-in-google-calendars)) + +**Can we widen that window?** No. `ContentResolver.requestSync()` takes extras like `SYNC_EXTRAS_MANUAL` / `SYNC_EXTRAS_EXPEDITED` — which is exactly what `CalendarsActivity.requestCalendarSyncAndRefresh()` already does — but there is **no API to request a date range**. The window is the sync adapter's own policy; a third-party app cannot parameterize or extend it. There's no service to call to backfill older events into the provider. + +**How much does this actually cost us?** Very little in practice, because of what this app stores: + +- `MAX_SCAN_BACKWARD_DAYS = 31` (`Consts.kt:177`) — the app itself only looks back a month. +- Active and snoozed events are, by their nature, recent or upcoming. An event snoozed from 14 months ago is not a realistic case. +- Dismissed-event history is the only store that reaches far back, and it's historical: it doesn't need to reopen in the calendar app. + +So the 12-month floor sits well outside the range this feature actually operates in. The honest framing is that **this is a limit on the tail, not on the feature.** + +**What happens to an event outside the window:** exactly the unresolved path already specified — the row is kept, marked unresolved, and retried. It never resolves, which is correct: the event genuinely isn't on this device. The app already renders this gracefully via `createCalendarNotFoundCal` (`CalendarProvider.kt:1384`). No crash, no data loss, no special-casing needed. + +This does mean the retry cap from Phase 3 matters — without a backoff, permanently-unmatchable old events would re-query the provider forever. + ## Current Architecture Identity flow today, and where it breaks on restore: @@ -191,9 +245,15 @@ The key insight: the calendar half of this problem was already solved once for s New `calendar/EventIdentityResolver.kt` — pure orchestration, no UI, constructor-injected `CalendarProviderInterface` and `CNPlusClockInterface` so it's Robolectric-testable (per `docs/testing/dependency_injection_patterns.md`). Responsibilities: -- Given a stored record + its identity blob, resolve `(newCalendarId, newEventId)`. +- Given a stored record + its identity blob, resolve `(newCalendarId, newEventId)` — **lookup only, no writes**. - Report a typed outcome: resolved / unresolved-calendar / unresolved-event / no-identity-stored / already-current. -- Apply the resolution to storage, handling the delete+reinsert for a changed PK and the collision case from Design Decisions. +- Apply the resolution, in the order established in Design Decisions: commit the safe `cid` update first, then attempt the `id` re-key only when a replacement was positively identified. + +Split into two sub-phases, because the risk profile is very different: + +**1a — `cid` only.** Plain column update, no PK change, no cross-database fallout. This alone fixes calendar attribution, filter pills, and per-calendar settings. Independently shippable and independently testable. + +**1b — `id` re-key.** The delete+re-insert, transactional per event. Must also re-key the matching rows in `manualAlertsV1` and `dismissedEventsV2`, which live in separate databases with no shared transaction — follow the manual-rollback pattern in `ApplicationController.unsnoozeToUpcoming`. Land this only once 1a is solid. This class is the whole substance of the feature; keep it small and free of Android UI dependencies. @@ -244,6 +304,9 @@ On a detected restore, rewrite orphaned `calendar_handled_.` keys to thei | `prefs/MiscSettingsFragmentX.kt` | Manual re-link action | | `res/xml/backup_rules.xml` | Exclude the new fingerprint prefs file | | `res/values/strings.xml` | Strings for the action + result dialog | +| `eventsstorage/EventAlertDao.kt`, `RoomEventsStorage.kt` | Transactional re-key (delete + insert) for a changed `id` | +| `monitorstorage/` + `dismissedeventsstorage/` storages | Re-key rows on an `id` change (Phase 1b), with manual rollback across DBs | +| `docs/architecture/database_schema_reference.md` | Mark `s2` as claimed once implemented | ## Testing Plan @@ -258,6 +321,10 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa - **No identity stored**: legacy row with empty `s2` → skipped cleanly. - **Already current**: IDs unchanged → no write (guards against pointless delete+reinsert churn). - **PK collision**: target `(id, istart)` already occupied → existing row kept, duplicate dropped. +- **`cid` committed independently**: calendar resolves but event does not → the `cid` update is still persisted (proves the safe-write-first ordering, and that a partial resolution is an improvement rather than a rollback). +- **Transactional re-key**: insert fails mid-re-key → original row still present, nothing lost. +- **Cross-database re-key**: after an `id` change, the matching `manualAlertsV1` and `dismissedEventsV2` rows are re-keyed too; a failure on either leaves all three consistent via manual rollback. +- **Orphaned monitor alert regression guard**: re-keyed event can still `restoreToUpcoming` — i.e. `clearWasHandled` finds its alert. This is the concrete failure the cross-DB work exists to prevent. - **Backfill**: pre-existing row + live provider → `s2` populated. - **Settings repair**: orphaned `calendar_handled_.N` keys remapped; unmatched ones reported. - **Restore detection**: fingerprint absent/mismatched ⇒ restore; matching ⇒ no-op. From 33af2f22bf4b6be24f392ea434ac9f12f31c3e3f Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 21:29:21 +0000 Subject: [PATCH 04/11] docs: scope to Room only, specify the s2 payload, drop conversational voice Three corrections to the portable event identity plan. Room only. Phase 0c previously added identity mapping to EventsStorageImplV9 and DismissedEventsStorageImplV2, which is new code on a path already scheduled for deletion (deprecated_features.md item 5). Removes those from scope and states the consequence: on the legacy fallback, s2 stays empty and the resolver skips those rows -- an already-degraded mode, left no worse than today. Added as a Key Decision and noted in the schema reference. Specifies the s2 payload. The plan described the blob abstractly but never showed it. Adds a concrete JSON example with a field table, explains why origCid/origId are stored despite duplicating the row's columns (they are the staleness check that distinguishes "never resolved" from "already resolved" across retries), gives the per-row size, and names who writes and reads it -- only EventIdentityResolver; nothing in normal runtime touches s2. Also states what it is not: not a content cache, since duplicating title/times would create a second source of truth. Removes conversational references so the plan reads standalone. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- .../architecture/database_schema_reference.md | 2 + docs/dev_todo/portable_event_identity.md | 63 ++++++++++++++++--- 2 files changed, 55 insertions(+), 10 deletions(-) diff --git a/docs/architecture/database_schema_reference.md b/docs/architecture/database_schema_reference.md index 22ee98a3..b616151e 100644 --- a/docs/architecture/database_schema_reference.md +++ b/docs/architecture/database_schema_reference.md @@ -85,6 +85,8 @@ Short-lived bookkeeping for alerts the app discovered by scanning, rebuilt from Every table carries spare `iN`/`sN` columns from the original 2016 schema. They are written as `0`/`""` and read by nothing. +New claims apply to the **Room entities only** — the legacy `*Impl*` classes are deprecated and scheduled for removal (`../dev_todo/deprecated_features.md`, item 5), so they should not gain new field handling. + They exist so a field can be added **without a schema migration** — no Room version bump, no new legacy `EventsStorageImplV10`, and no change to the Supabase table (`supabase/migrations/20250301213237_events.sql` already mirrors every column, reserved ones included), which keeps the PowerSync payload working untouched. The cost is that the name stops describing the content: a column called `s2` holding calendar identity is opaque to anyone reading a raw DB dump. So the rule is: diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index 0835cde6..ede8b1a0 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -45,7 +45,7 @@ Store durable, provider-independent identity alongside every stored event so tha - **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. - **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. - **Schema migration** — explicitly avoided; see Key Decisions. -- **Storing identity blobs in `MonitorStorage`** — `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so it doesn't need its own identity. **But its rows are keyed on `eventId` and must still be re-keyed when an `id` changes** — see the cross-database note in Design Decisions. (Corrected on review; the original plan wrongly treated the monitor DB as entirely out of scope.) +- **Storing identity blobs in `MonitorStorage`** — `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so it doesn't need its own identity. **But its rows are keyed on `eventId` and must still be re-keyed when an `id` changes** — see the cross-database note in Design Decisions. ## Key Decisions Summary @@ -56,8 +56,9 @@ Store durable, provider-independent identity alongside every stored event so tha | Event identity | `Events.UID_2445`, falling back to `Events._SYNC_ID` | `UID_2445` is the iCalendar UID — globally stable and identical across devices for the same Google/CalDAV event. Available since API 17; minSdk is 24. | | When identity is captured | On every event write (add/update), best-effort | Cheap, keeps identity fresh, and means any future backup is restorable without a migration pass. | | When re-resolution runs | Lazily, on a detected restore, **retrying** until resolved — plus a manual trigger | Calendars often sync onto the phone *after* our first launch, so a one-shot pass would match nothing. | -| Manual trigger UX | Mirror the existing pull-to-refresh + overflow "Refresh" in `prefs/CalendarsActivity.kt` | The user explicitly asked for "a way to manually invoke also like the refresh behavior in the handled calendars ui". That screen already does `ContentResolver.requestSync` then reloads — same shape. | +| Manual trigger UX | Mirror the existing pull-to-refresh + overflow "Refresh" in `prefs/CalendarsActivity.kt` | That screen already requests a calendar sync then reloads, which is exactly the shape needed here. Reusing a familiar interaction beats inventing a new one. | | Unmatched events | Leave the row intact with its stale ID, keep the identity blob, mark unresolved | Matches the existing fail-soft convention (`reloadCalendarEventAlertFromEvent` returns `NoChange` rather than deleting). Never destroy user data because a match failed. | +| Storage scope | **Room implementations only** | Legacy storage is deprecated and scheduled for removal (`deprecated_features.md` item 5). It's a migration-failure fallback; new code there would be written to be deleted. | | Restore detection | Compare a stored install fingerprint against the current one | Cheap and reliable. Auto-backup deliberately excludes `events_storage_state.xml`, so a prefs-based marker is a proven pattern here. | ## Design Decisions @@ -128,9 +129,48 @@ Title+time heuristics produce false positives on recurring and duplicated events **The email tuple is not perfectly unique either** — this is why `findMatchingCalendarId()` already has three tiers. One account can expose several calendars (primary, birthdays, a shared team calendar), all with the same `ACCOUNT_NAME`. That's why the match uses account name + type + owner, and only falls back to display name. +### What actually goes in the `s2` column + +`s2` holds one JSON object per event row: a snapshot of **how to find this event again from scratch**, written using only identifiers that mean something on a different device. Concretely: + +```json +{ + "v": 1, + "cal": { + "acct": "will@example.com", + "type": "com.google", + "owner": "will@example.com", + "disp": "Work", + "name": "will@example.com" + }, + "uid": "abc123def456@google.com", + "origCid": 3, + "origId": 91427 +} +``` + +Field by field: + +| Field | Source | Why it's there | +|---|---|---| +| `v` | constant | Schema version, so a future field can be added without breaking old rows | +| `cal` | `getCalendarBackupInfo(calendarId)` | The five fields `findMatchingCalendarId()` already matches on — exactly the `CalendarBackupInfo` shape | +| `uid` | `Events.UID_2445` (fallback `_SYNC_ID`) | Identifies the specific event within that calendar | +| `origCid` / `origId` | the row's current `cid` / `id` | The IDs in force when the snapshot was taken | + +**Why store `origCid`/`origId` when they're already in the row?** They're the staleness check. If `origId` still equals the row's `id`, the row hasn't been re-keyed yet; if they differ, resolution has already run. Without them, there's no way to tell "never resolved" from "already resolved" — which matters because the retry loop re-runs on every launch and must not redo completed work. + +**Size:** roughly 150–250 bytes per row. For a typical few-hundred-row database that's well under 100 KB, which is why storing it per-row rather than in a shared side table is acceptable. + +**Who writes it:** Phase 0 on every event add/update (fresh rows), Phase 2 backfill (pre-existing rows). + +**Who reads it:** only `EventIdentityResolver`. Nothing in the normal app runtime reads `s2` — the app keeps using `cid`/`id` exactly as it does today. The blob is dormant until a restore is detected or the manual re-link action runs, at which point it's the sole input to the resolution below. + +**What it is not:** it is not a cache of event content. Title, times, and location are already stored in their own columns and are refreshed from the provider by the normal reload path. Duplicating them here would create a second source of truth that could drift. + ### Resolution strategy -A restored event needs two lookups, in order: +Resolution consumes the `s2` blob described above. A restored event needs two lookups, in order: 1. **Calendar**: `findMatchingCalendarId(context, storedBackupInfo)` → new `cid`. Reuses the existing 3-tier matcher untouched. 2. **Event**: query `Events.CONTENT_URI` for `UID_2445 = ? AND CALENDAR_ID = ?` (scoped to the just-matched calendar to avoid cross-calendar collisions) → new `id`. @@ -146,7 +186,7 @@ So the sequencing rule is: **update `cid` first and commit it.** Even if the eve #### Why we can afford to be bold about it -Your instinct that a restored row is "already broken" is the right frame, and it's what makes this tractable. A row whose `id` points at a nonexistent event is already inert: it can't be opened, it can't be reloaded, `reloadCalendarEventAlertFromEvent` degrades it to `NoChange` forever. **Rewriting it can only move it from broken toward working.** +A restored row is already broken, and that is what makes rewriting it tractable. A row whose `id` points at a nonexistent event is inert: it cannot be opened, it cannot be reloaded, and `reloadCalendarEventAlertFromEvent` degrades it to `NoChange` indefinitely. **Rewriting it can only move it from broken toward working.** But "already broken" is only true when the ID is genuinely stale. It is *not* true if we're wrong about that — and a false positive would delete a live, working row. So the delete+re-insert is gated on having positively identified the replacement first: @@ -168,7 +208,7 @@ This matters concretely: `restoreToUpcoming` (`ApplicationController.kt:1293-130 The same applies to `dismissedEventsV2`, which is keyed on `eventId` in its own database. -So the resolver must re-key **all three** databases for a given event, in a defined order, with manual rollback on partial failure — the pattern `unsnoozeToUpcoming` already establishes. This is the main thing that grew in scope on review, and it argues for doing `eventsV9` first and treating the other two as follow-on steps with their own tests. +So the resolver must re-key **all three** databases for a given event, in a defined order, with manual rollback on partial failure — the pattern `unsnoozeToUpcoming` already establishes. This is why the `id` re-key is split into its own sub-phase: `eventsV9` lands first, with the other two databases as follow-on steps carrying their own tests. Every failure path below leaves the row intact and retryable — nothing is ever deleted because a match failed: @@ -232,11 +272,15 @@ The key insight: the calendar half of this problem was already solved once for s ### Phase 0: Capture identity at write time -**0a — Identity model.** New `calendar/PortableEventIdentity.kt`: a small serializable type holding the `CalendarBackupInfo` fields plus `eventUid` and the originating `calendarId`/`eventId`. Include a schema `version` field for forward compatibility. Serialize with kotlinx.serialization (already a dependency, used by `backup/BackupData.kt`) to a compact JSON string. +**0a — Identity model.** New `calendar/PortableEventIdentity.kt` implementing the JSON shape specified in "What actually goes in the `s2` column" above. Serialize with kotlinx.serialization (already a dependency, used by `backup/BackupData.kt`), using short `@SerialName`s to keep the per-row cost down. Decoding must return null rather than throw on malformed or empty input, catching `SerializationException` specifically (never broad `Exception`, per `AGENTS.md`). **0b — Read the UID from the provider.** Add `Events.UID_2445` (with `_SYNC_ID` fallback) to the projection in `CalendarProvider.getEvent()` (`calendar/CalendarProvider.kt:418-439`) and expose it on `EventRecord`. Keep it nullable — not every event has one. -**0c — Persist it.** Map the blob into `EventAlertEntity.s2` / `DismissedEventEntity.s2` in `fromRecord()`/`toRecord()`, and mirror it in `EventsStorageImplV9` and `DismissedEventsStorageImplV2` so the legacy fallback path doesn't silently drop it. Populate on add/update in `ApplicationController` where the record is first built from the provider. +**0c — Persist it.** Map the blob into `EventAlertEntity.s2` / `DismissedEventEntity.s2` in `fromRecord()`/`toRecord()`. Populate on add/update in `ApplicationController` where the record is first built from the provider. + +**Room only — do not touch the legacy storage implementations.** Legacy storage (`EventsStorageImplV9`, `DismissedEventsStorageImplV2`, `LegacyEventsStorage`) is deprecated and scheduled for removal (`docs/dev_todo/deprecated_features.md`, item 5). It exists solely as a fallback if Room migration throws. Adding identity handling there would mean writing new code on a path slated for deletion. + +The consequence is acceptable: on the legacy fallback path, `s2` stays `""`, every event reports "no identity stored", and the resolver skips it. That path is already a degraded mode — the user is running without Room because migration failed — and it leaves the data no worse than it is today. **Checkpoint:** new events written on this device carry a populated `s2`. Existing rows still have `""` — that's expected and handled in Phase 2. @@ -267,7 +311,7 @@ This is what makes the feature useful to the current user rather than only to ne Store an install fingerprint in its own SharedPreferences file, and **exclude that file from `backup_rules.xml`** so it does not survive a restore — the same trick `EventsStorageState` already relies on. Absent/mismatched fingerprint on launch ⇒ treat as a restore and mark all events pending re-resolution. -Retry semantics (per the user's answer): keep pending events marked until each resolves, re-attempting on app start and after calendar rescans, rather than burning the attempt once. Cap attempts with a backoff so a permanently-unmatchable event doesn't re-query forever. +Retry semantics: keep pending events marked until each resolves, re-attempting on app start and after calendar rescans, rather than burning the attempt once. Cap attempts with a backoff so a permanently-unmatchable event doesn't re-query forever. ### Phase 4: Manual trigger @@ -297,8 +341,7 @@ On a detected restore, rewrite orphaned `calendar_handled_.` keys to thei | `calendar/CalendarProviderInterface.kt` | Declare the new lookup | | `calendar/EventRecord.kt` | Carry nullable `eventUid` | | `eventsstorage/EventAlertEntity.kt` | Name `s2` as the identity column; map in `fromRecord`/`toRecord` | -| `eventsstorage/EventsStorageImplV9.kt` | Mirror the mapping on the legacy path | -| `dismissedeventsstorage/DismissedEventEntity.kt`, `DismissedEventsStorageImplV2.kt` | Same, for dismissed events | +| `dismissedeventsstorage/DismissedEventEntity.kt` | Same, for dismissed events (Room entity only) | | `app/ApplicationController.kt` | Populate identity on write; **fix `restoreToActive()` (line 1321) to use the stored blob instead of re-querying the stale ID** | | `backup/SettingsBackupManager.kt` | Extract calendar-remap logic for reuse in Phase 5 | | `prefs/MiscSettingsFragmentX.kt` | Manual re-link action | From b8cf922e5b645d521d5864a51300861fa5b191a3 Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 21:37:45 +0000 Subject: [PATCH 05/11] docs: store portable identity in its own Room DB, not the s2 column Reverses the earlier decision to put identity in eventsV9.s2. The reserved text columns are a finite, effectively irreversible resource and should be spent only on data that must live in the event row -- read on the hot path, needed in the same query, or required to travel through the PowerSync pipeline. Portable identity is none of those: it's read only by the resolver on a restore, joins by key, and should NOT reach Supabase, since it contains account emails. That last point inverts the original rationale, which counted the Supabase table already mirroring s2 as a benefit. A new database is cheap here: no legacy predecessor and no copy migration (unlike the existing three), backup_rules.xml already covers it via domain="database", and PowerSync targets eventsV9 by name so a new table is invisible to sync. Switches the payload from a JSON blob to typed Room columns, which removes the serialization layer entirely and makes resolveAttempts a plain UPDATE rather than decode/mutate/re-encode. Notes the real cost: a fourth store with no cross-database transactions, so an id re-key must now move four rows with manual rollback. Identity rows are derived metadata, so orphans are harmless. Schema reference gains the new database and guidance on when a reserved column is and isn't the right home. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- .../architecture/database_schema_reference.md | 16 +- docs/dev_todo/portable_event_identity.md | 146 ++++++++++-------- 2 files changed, 92 insertions(+), 70 deletions(-) diff --git a/docs/architecture/database_schema_reference.md b/docs/architecture/database_schema_reference.md index b616151e..11ed7a24 100644 --- a/docs/architecture/database_schema_reference.md +++ b/docs/architecture/database_schema_reference.md @@ -1,6 +1,6 @@ # Database Schema Reference -Column-by-column reference for the app's three SQLite databases. Column names are abbreviated for historical reasons (the 2016 schema used short names to save space), so this table is the map between the on-disk name and what it actually means. +Column-by-column reference for the app's SQLite databases. Column names are abbreviated for historical reasons (the 2016 schema used short names to save space), so this table is the map between the on-disk name and what it actually means. Authoritative definitions live in the Room entities; this doc is the human-readable index: @@ -13,10 +13,13 @@ Authoritative definitions live in the Room entities; this doc is the human-reada | Database | Room file | Legacy file | Table | Primary key | |---|---|---|---|---| | Events (active/snoozed) | `RoomEvents` | `Events` | `eventsV9` | `(id, istart)` | +| Portable event identity *(planned)* | `RoomEventIdentity` | — (new) | `eventIdentityV1` | `(eventId, instanceStart)` | | Dismissed events | `RoomDismissedEvents` | `DismissedEvents` | `dismissedEventsV2` | `(eventId, instanceStart)` | | Calendar monitor | `RoomCalendarMonitor` | `CalendarMonitor` | `manualAlertsV1` | `(eventId, alertTime, instanceStart)` | -All three are **separate database files**. There are no foreign keys between them and cross-database transactions are not possible — code that must stay consistent across two of them does manual rollback (see `ApplicationController.unsnoozeToUpcoming`). +These are **separate database files**. There are no foreign keys between them and cross-database transactions are not possible — code that must stay consistent across two of them does manual rollback (see `ApplicationController.unsnoozeToUpcoming`). + +Note what the shared `(eventId, instanceStart)` key implies: changing an event's `eventId` requires re-keying its rows in *every* one of these databases, with no transaction spanning them. ## `eventsV9` — active and snoozed events @@ -47,7 +50,7 @@ The short column names here are the ones most likely to confuse. Note especially | `oattsts` | **`attendanceStatus`** | Int | `AttendanceStatus` enum — the user's RSVP | | `i1` | `flags` | Long | Bitfield: `IS_MUTED=1`, `IS_TASK=2`, `IS_ALARM=4`, `IS_PINNED=8` | | `i2`–`i8` | *reserved* | Long | Unused; written as `0` | -| `s2` | *reserved* | String | Unused; written as `""`. **Claimed by the portable-identity work** — see below | +| `s2` | *reserved* | String | Unused; written as `""` — the only spare text column in this table | ## `dismissedEventsV2` — dismissal history @@ -87,9 +90,13 @@ Every table carries spare `iN`/`sN` columns from the original 2016 schema. They New claims apply to the **Room entities only** — the legacy `*Impl*` classes are deprecated and scheduled for removal (`../dev_todo/deprecated_features.md`, item 5), so they should not gain new field handling. +**Spend them sparingly.** There is exactly one spare text column per table and claiming one is effectively irreversible once rows are written. Reserve them for data that genuinely *must* live in the row: read on the hot path, needed in the same query as the event, or required to travel with the row through the PowerSync/Supabase pipeline. Anything that is merely *associated* with an event — derived metadata, bookkeeping, anything read only on rare paths — belongs in its own table or database, joined by `(eventId, instanceStart)`. + +A worked example of choosing the latter: [portable_event_identity.md](../dev_todo/portable_event_identity.md) initially planned to use `eventsV9.s2` and deliberately moved to a separate Room database instead. + They exist so a field can be added **without a schema migration** — no Room version bump, no new legacy `EventsStorageImplV10`, and no change to the Supabase table (`supabase/migrations/20250301213237_events.sql` already mirrors every column, reserved ones included), which keeps the PowerSync payload working untouched. -The cost is that the name stops describing the content: a column called `s2` holding calendar identity is opaque to anyone reading a raw DB dump. So the rule is: +The cost is that the name stops describing the content: a column called `i1` holding a flags bitfield is opaque to anyone reading a raw DB dump. So the rule is: > **When you claim a reserved column, rename the constant to describe its meaning, document it in this file, and leave a comment at the entity declaration. Never leave a live column named "reserved".** @@ -101,7 +108,6 @@ Note the precedent: `i1` was already claimed for `flags`, and `s1` for `descript |---|---|---|---| | `eventsV9` | `i1` | `flags` bitfield | in use | | `eventsV9` | `s1` | `description` | in use | -| `eventsV9` | `s2` | portable event identity | planned — see [portable_event_identity.md](../dev_todo/portable_event_identity.md) | | `dismissedEventsV2` | `i1` | `flags` bitfield | in use | | `dismissedEventsV2` | `s1` | `description` | in use | diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index ede8b1a0..9cc27e1c 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -44,36 +44,67 @@ Store durable, provider-independent identity alongside every stored event so tha - **Bidirectional multi-device sync of snooze/mute state** — the second half of #273. This plan makes the *data* portable; it does not make it live-shared. - **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. - **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. -- **Schema migration** — explicitly avoided; see Key Decisions. -- **Storing identity blobs in `MonitorStorage`** — `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so it doesn't need its own identity. **But its rows are keyed on `eventId` and must still be re-keyed when an `id` changes** — see the cross-database note in Design Decisions. +- **Migrating the existing databases** — no schema change to `eventsV9`, `dismissedEventsV2`, or `manualAlertsV1`. The identity database is new and starts at version 1. +- **Storing identity for `MonitorStorage` rows** — `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so it doesn't need its own identity. **But its rows are keyed on `eventId` and must still be re-keyed when an `id` changes** — see the cross-database note in Design Decisions. ## Key Decisions Summary | Decision | Choice | Rationale | |---|---|---| -| Where to store identity | Existing **unused reserved columns** (`eventsV9.s2`, `dismissedEventsV2.s2`) as a small JSON blob | Zero schema migration, zero Room version bump, zero risk to the cr-sqlite/PowerSync column contract. These columns are written as `""` today and read by nothing. | +| Where to store identity | A **new, dedicated Room database** with typed columns | Keeps the scarce reserved text columns free for data that must live in the event row. Identity is off the hot path, joinable by key, and stays out of the Supabase sync payload by construction. | | Calendar identity | Reuse `CalendarBackupInfo` (account name/type, owner, displayName, name) | Already exists, already has a tested 3-tier fallback matcher, already the proven format in settings backup. | | Event identity | `Events.UID_2445`, falling back to `Events._SYNC_ID` | `UID_2445` is the iCalendar UID — globally stable and identical across devices for the same Google/CalDAV event. Available since API 17; minSdk is 24. | | When identity is captured | On every event write (add/update), best-effort | Cheap, keeps identity fresh, and means any future backup is restorable without a migration pass. | | When re-resolution runs | Lazily, on a detected restore, **retrying** until resolved — plus a manual trigger | Calendars often sync onto the phone *after* our first launch, so a one-shot pass would match nothing. | | Manual trigger UX | Mirror the existing pull-to-refresh + overflow "Refresh" in `prefs/CalendarsActivity.kt` | That screen already requests a calendar sync then reloads, which is exactly the shape needed here. Reusing a familiar interaction beats inventing a new one. | -| Unmatched events | Leave the row intact with its stale ID, keep the identity blob, mark unresolved | Matches the existing fail-soft convention (`reloadCalendarEventAlertFromEvent` returns `NoChange` rather than deleting). Never destroy user data because a match failed. | +| Unmatched events | Leave the event row intact with its stale ID, keep the identity row, mark unresolved | Matches the existing fail-soft convention (`reloadCalendarEventAlertFromEvent` returns `NoChange` rather than deleting). Never destroy user data because a match failed. | | Storage scope | **Room implementations only** | Legacy storage is deprecated and scheduled for removal (`deprecated_features.md` item 5). It's a migration-failure fallback; new code there would be written to be deleted. | | Restore detection | Compare a stored install fingerprint against the current one | Cheap and reliable. Auto-backup deliberately excludes `events_storage_state.xml`, so a prefs-based marker is a proven pattern here. | ## Design Decisions -### Why reserved columns rather than a schema migration +### Why a separate Room database rather than a reserved column -`EventAlertEntity` (`eventsstorage/EventAlertEntity.kt`) declares `i2`–`i8` and `s2` as reserved, and `EventsStorageImplV9` writes them as `0`/`""`. Nothing reads them. Using `s2` means: +An earlier revision of this plan stored the identity blob in the unused `eventsV9.s2` reserved column. That is rejected in favour of a **new, dedicated Room database**. -- No Room version bump, no new `Migration`, no new legacy `EventsStorageImplV10`. -- The Supabase table (`supabase/migrations/20250301213237_events.sql`) already has an `s2` column, so the sync payload keeps working unchanged. -- `installCrsqliteOnTable` in `src/lib/cr-sqlite/install.ts` rewrites the PK but doesn't enumerate columns — unaffected. +The reserved columns (`i2`–`i8`, `s2`) are a small, finite, one-time resource — there is exactly one spare text column per table, and claiming it is effectively irreversible once rows are written. It should be spent on data that **must** live inside the event row: something read on the app's hot path, needed in the same query as the event, or required to travel with the row through the PowerSync/Supabase pipeline. -The tradeoff is that `s2` becomes semantically meaningful, so it needs a named constant and a comment at the entity declaration rather than staying "reserved". That's a documentation cost, not a correctness one. +Portable identity is none of those: -There's precedent: `i1` was already claimed for the `flags` bitfield and `s1` for `description`, so this is the third such claim rather than a new practice. To stop the cost compounding, [database_schema_reference.md](../architecture/database_schema_reference.md) now documents every column in all three databases and sets the rule — **when you claim a reserved column, rename the constant to describe its meaning and document it there; never leave a live column named "reserved."** +- **It is not read on the hot path.** Nothing in normal app runtime touches it. Only `EventIdentityResolver` reads it, and only on a detected restore or a manual re-link. +- **It does not need to be in the same row.** It is keyed by `(eventId, instanceStartTime)` and can be joined when needed. +- **It should not travel to Supabase.** The sync layer targets the `eventsV9` table by name, so keeping identity out of that table keeps it out of the sync payload by construction — which is what we want, since the blob contains account emails. + +That last point inverts an argument from the earlier revision. Using `s2` was originally justified partly because the Supabase table already mirrors it, so the payload "keeps working unchanged" — but on reflection, silently shipping account identifiers to the remote database is a drawback, not a benefit. + +#### What the separate database costs + +Less than it might appear, because the infrastructure is already in place: + +- **No schema migration.** A brand-new database starts at `version = 1` with no legacy predecessor and no copy-migration step — strictly simpler than the existing three, which all carry legacy baggage. `monitorstorage/MonitorDatabase.kt` is the closest template: single entity, `version = 1`, `exportSchema = false`. +- **Backed up automatically.** `res/xml/backup_rules.xml` includes ``, so any new database file is covered with no config change. This is essential — the identity DB is useless unless it restores alongside the events it describes. +- **No sync impact.** PowerSync/cr-sqlite is wired to `eventsV9` explicitly (`src/lib/features/SetupSync.tsx`, `src/lib/powersync/Schema.tsx`); a new table is invisible to it. + +The real cost is the one inherent to this codebase: **a fourth separate database file means a fourth store with no cross-database transactions.** Identity rows can drift from the events they describe — e.g. an event is deleted but its identity row lingers. This is tolerable because identity rows are pure derived metadata: an orphaned one is harmless, and the resolver ignores identity with no matching event. A periodic cleanup pass can prune orphans opportunistically; correctness never depends on it. + +#### Schema + +One table, keyed to match `eventsV9`'s primary key so rows join cleanly: + +| Column | Type | Notes | +|---|---|---| +| `eventId` | Long | PK part 1 — matches `eventsV9.id` | +| `instanceStart` | Long | PK part 2 — matches `eventsV9.istart` | +| `acctName` / `acctType` / `owner` / `dispName` / `calName` | String | The `CalendarBackupInfo` tuple, as real columns | +| `eventUid` | String? | `Events.UID_2445`, fallback `_SYNC_ID`. Nullable — not every event has one | +| `origCalendarId` | Long | The `cid` in force when captured | +| `origEventId` | Long | The `id` in force when captured | +| `capturedAt` | Long | Via `CNPlusClockInterface`, never `System.currentTimeMillis()` | +| `resolveAttempts` | Int | Backs the Phase 3 retry cap | + +**Real typed columns, not a JSON blob.** Once the constraint of squeezing into one text column is gone, there is no reason to serialize. Typed columns are queryable (e.g. "all rows with `resolveAttempts > N`"), enforced by Room at compile time, and need no `SerializationException` handling. The `resolveAttempts` counter in particular is a plain `UPDATE` rather than a decode/mutate/re-encode cycle. + +It also means no serialization layer at all: the Room entity *is* the model, with no encode/decode step to test or to fail. ### The two halves: why the email is necessary but not sufficient @@ -129,48 +160,30 @@ Title+time heuristics produce false positives on recurring and duplicated events **The email tuple is not perfectly unique either** — this is why `findMatchingCalendarId()` already has three tiers. One account can expose several calendars (primary, birthdays, a shared team calendar), all with the same `ACCOUNT_NAME`. That's why the match uses account name + type + owner, and only falls back to display name. -### What actually goes in the `s2` column - -`s2` holds one JSON object per event row: a snapshot of **how to find this event again from scratch**, written using only identifiers that mean something on a different device. Concretely: - -```json -{ - "v": 1, - "cal": { - "acct": "will@example.com", - "type": "com.google", - "owner": "will@example.com", - "disp": "Work", - "name": "will@example.com" - }, - "uid": "abc123def456@google.com", - "origCid": 3, - "origId": 91427 -} -``` - -Field by field: +### What gets stored, and who uses it -| Field | Source | Why it's there | -|---|---|---| -| `v` | constant | Schema version, so a future field can be added without breaking old rows | -| `cal` | `getCalendarBackupInfo(calendarId)` | The five fields `findMatchingCalendarId()` already matches on — exactly the `CalendarBackupInfo` shape | -| `uid` | `Events.UID_2445` (fallback `_SYNC_ID`) | Identifies the specific event within that calendar | -| `origCid` / `origId` | the row's current `cid` / `id` | The IDs in force when the snapshot was taken | +One identity row per stored event, holding **everything needed to find that event again from scratch** using only identifiers that are meaningful on a different device. For an event in a Google work calendar: -**Why store `origCid`/`origId` when they're already in the row?** They're the staleness check. If `origId` still equals the row's `id`, the row hasn't been re-keyed yet; if they differ, resolution has already run. Without them, there's no way to tell "never resolved" from "already resolved" — which matters because the retry loop re-runs on every launch and must not redo completed work. +| Column | Example value | +|---|---| +| `eventId` / `instanceStart` | `91427` / `1764547200000` — the join key back to `eventsV9` | +| `acctName` / `acctType` | `will@example.com` / `com.google` | +| `owner` | `will@example.com` | +| `dispName` / `calName` | `Work` / `will@example.com` | +| `eventUid` | `abc123def456@google.com` | +| `origCalendarId` / `origEventId` | `3` / `91427` | -**Size:** roughly 150–250 bytes per row. For a typical few-hundred-row database that's well under 100 KB, which is why storing it per-row rather than in a shared side table is acceptable. +**Why store `origCalendarId`/`origEventId` when they duplicate the event row?** They are the staleness check. If `origEventId` still equals the event's current `id`, resolution has not run for this row; if they differ, it already has. Without them there is no way to distinguish "never resolved" from "already resolved" — which matters because the retry loop re-runs on every launch and must not redo completed work. **Who writes it:** Phase 0 on every event add/update (fresh rows), Phase 2 backfill (pre-existing rows). -**Who reads it:** only `EventIdentityResolver`. Nothing in the normal app runtime reads `s2` — the app keeps using `cid`/`id` exactly as it does today. The blob is dormant until a restore is detected or the manual re-link action runs, at which point it's the sole input to the resolution below. +**Who reads it:** only `EventIdentityResolver`. Nothing in normal app runtime reads this database. The app keeps using `cid`/`id` exactly as it does today; these rows lie dormant until a restore is detected or the manual re-link action runs, at which point they are the sole input to the resolution below. -**What it is not:** it is not a cache of event content. Title, times, and location are already stored in their own columns and are refreshed from the provider by the normal reload path. Duplicating them here would create a second source of truth that could drift. +**What it is not:** not a cache of event content. Title, times, and location live in their own columns in `eventsV9` and are refreshed from the provider by the normal reload path. Duplicating them here would create a second source of truth that could drift. ### Resolution strategy -Resolution consumes the `s2` blob described above. A restored event needs two lookups, in order: +Resolution consumes the identity rows described above. A restored event needs two lookups, in order: 1. **Calendar**: `findMatchingCalendarId(context, storedBackupInfo)` → new `cid`. Reuses the existing 3-tier matcher untouched. 2. **Event**: query `Events.CONTENT_URI` for `UID_2445 = ? AND CALENDAR_ID = ?` (scoped to the just-matched calendar to avoid cross-calendar collisions) → new `id`. @@ -200,7 +213,7 @@ But "already broken" is only true when the ID is genuinely stale. It is *not* tr If the new `(id, istart)` already exists, the new device independently re-added the same event. Keep the existing row and drop the restored duplicate: the live row is the one the app has actually been maintaining. This is a genuine merge decision, not an error. -#### Cross-database fallout (found while reviewing this) +#### Cross-database fallout `MonitorStorage` lives in a **separate database** and is keyed `(eventId, alertTime, instanceStart)`. Changing `id` in `eventsV9` therefore orphans the matching monitor alert, and there are no cross-database transactions to lean on. @@ -208,13 +221,15 @@ This matters concretely: `restoreToUpcoming` (`ApplicationController.kt:1293-130 The same applies to `dismissedEventsV2`, which is keyed on `eventId` in its own database. -So the resolver must re-key **all three** databases for a given event, in a defined order, with manual rollback on partial failure — the pattern `unsnoozeToUpcoming` already establishes. This is why the `id` re-key is split into its own sub-phase: `eventsV9` lands first, with the other two databases as follow-on steps carrying their own tests. +The identity database itself is keyed the same way, so it is a fourth store needing the same treatment — its row must move to the new `(eventId, instanceStart)` alongside the event, or the next retry would not find it. + +So the resolver must re-key **four** databases for a given event, in a defined order, with manual rollback on partial failure — the pattern `unsnoozeToUpcoming` already establishes. This is why the `id` re-key is split into its own sub-phase: `eventsV9` plus its identity row land first, with the monitor and dismissed databases as follow-on steps carrying their own tests. Every failure path below leaves the row intact and retryable — nothing is ever deleted because a match failed: ```mermaid flowchart TD - A["Stored event row"] --> B{"Identity blob
in s2?"} + A["Stored event row"] --> B{"Identity row
exists?"} B -->|"no (pre-Phase 0)"| Z["Skip — backfill handles it"] B -->|yes| C["findMatchingCalendarId
(account tuple)"] @@ -231,7 +246,7 @@ flowchart TD G -->|no| H{"Target (id, istart)
already taken?"} H -->|yes| V["Keep live row,
drop restored duplicate"] - H -->|no| U["Transaction:
delete + re-insert,
re-key monitor + dismissed ✅"] + H -->|no| U["Transaction: delete + re-insert,
re-key identity, monitor,
dismissed ✅"] ``` ### How far back does the new device's calendar actually go? @@ -272,24 +287,24 @@ The key insight: the calendar half of this problem was already solved once for s ### Phase 0: Capture identity at write time -**0a — Identity model.** New `calendar/PortableEventIdentity.kt` implementing the JSON shape specified in "What actually goes in the `s2` column" above. Serialize with kotlinx.serialization (already a dependency, used by `backup/BackupData.kt`), using short `@SerialName`s to keep the per-row cost down. Decoding must return null rather than throw on malformed or empty input, catching `SerializationException` specifically (never broad `Exception`, per `AGENTS.md`). +**0a — Identity storage.** New `identitystorage/` package following the shape of `monitorstorage/`: `EventIdentityEntity` (the schema in Design Decisions), `EventIdentityDao`, and `EventIdentityDatabase` at `version = 1`, name `RoomEventIdentity`. No legacy predecessor and no copy-migration — this is a fresh database. Use `CrSqliteRoomFactory` for consistency with the existing three. **0b — Read the UID from the provider.** Add `Events.UID_2445` (with `_SYNC_ID` fallback) to the projection in `CalendarProvider.getEvent()` (`calendar/CalendarProvider.kt:418-439`) and expose it on `EventRecord`. Keep it nullable — not every event has one. -**0c — Persist it.** Map the blob into `EventAlertEntity.s2` / `DismissedEventEntity.s2` in `fromRecord()`/`toRecord()`. Populate on add/update in `ApplicationController` where the record is first built from the provider. +**0c — Persist it.** Write an identity row whenever an event is added or updated in `ApplicationController`, keyed `(eventId, instanceStartTime)`. Best-effort: a failure to capture identity must never fail the event write itself. **Room only — do not touch the legacy storage implementations.** Legacy storage (`EventsStorageImplV9`, `DismissedEventsStorageImplV2`, `LegacyEventsStorage`) is deprecated and scheduled for removal (`docs/dev_todo/deprecated_features.md`, item 5). It exists solely as a fallback if Room migration throws. Adding identity handling there would mean writing new code on a path slated for deletion. -The consequence is acceptable: on the legacy fallback path, `s2` stays `""`, every event reports "no identity stored", and the resolver skips it. That path is already a degraded mode — the user is running without Room because migration failed — and it leaves the data no worse than it is today. +The consequence is acceptable: on the legacy fallback path no identity rows are written, every event reports "no identity stored", and the resolver skips it. That path is already a degraded mode — the user is running without Room because migration failed — and it leaves the data no worse than it is today. -**Checkpoint:** new events written on this device carry a populated `s2`. Existing rows still have `""` — that's expected and handled in Phase 2. +**Checkpoint:** new events written on this device get an identity row. Pre-existing events have none — expected, and handled in Phase 2. ### Phase 1: Resolution engine New `calendar/EventIdentityResolver.kt` — pure orchestration, no UI, constructor-injected `CalendarProviderInterface` and `CNPlusClockInterface` so it's Robolectric-testable (per `docs/testing/dependency_injection_patterns.md`). Responsibilities: -- Given a stored record + its identity blob, resolve `(newCalendarId, newEventId)` — **lookup only, no writes**. +- Given a stored record + its identity row, resolve `(newCalendarId, newEventId)` — **lookup only, no writes**. - Report a typed outcome: resolved / unresolved-calendar / unresolved-event / no-identity-stored / already-current. - Apply the resolution, in the order established in Design Decisions: commit the safe `cid` update first, then attempt the `id` re-key only when a replacement was positively identified. @@ -303,7 +318,7 @@ This class is the whole substance of the feature; keep it small and free of Andr ### Phase 2: Backfill for pre-existing rows -Rows written before Phase 0 have an empty `s2`. While the app is still on the *original* device, those rows can be backfilled by reading the identity from the live provider (the stale IDs are still valid here). Run this opportunistically on app start when unbackfilled rows exist. +Events stored before Phase 0 have no identity row. While the app is still on the *original* device, they can be backfilled by reading identity from the live provider (the stale IDs are still valid there). Run this opportunistically on app start while any event lacks an identity row. This is what makes the feature useful to the current user rather than only to new installs — without it, today's data is still unrestorable. @@ -327,10 +342,13 @@ On a detected restore, rewrite orphaned `calendar_handled_.` keys to thei | File | Purpose | |---|---| -| `calendar/PortableEventIdentity.kt` | Serializable identity blob + JSON encode/decode | +| `identitystorage/EventIdentityEntity.kt` | Room entity — the identity schema | +| `identitystorage/EventIdentityDao.kt` | Queries: get/put by key, find events lacking identity, bump attempts | +| `identitystorage/EventIdentityDatabase.kt` | Room DB `RoomEventIdentity`, version 1 | +| `identitystorage/EventIdentityStorage.kt` | Storage facade matching existing conventions | | `calendar/EventIdentityResolver.kt` | Resolution engine and typed outcomes | | `test/.../calendar/EventIdentityResolverRobolectricTest.kt` | Core resolution logic tests | -| `test/.../calendar/PortableEventIdentityTest.kt` | Pure serialization round-trip tests | +| `androidTest/.../identitystorage/EventIdentityStorageTest.kt` | Real-SQLite storage round-trip | | `androidTest/.../calendar/EventIdentityRestoreTest.kt` | Real-provider end-to-end | ### Modified Files @@ -340,16 +358,14 @@ On a detected restore, rewrite orphaned `calendar_handled_.` keys to thei | `calendar/CalendarProvider.kt` | Add `UID_2445`/`_SYNC_ID` to `getEvent()` projection; add a lookup-by-UID query | | `calendar/CalendarProviderInterface.kt` | Declare the new lookup | | `calendar/EventRecord.kt` | Carry nullable `eventUid` | -| `eventsstorage/EventAlertEntity.kt` | Name `s2` as the identity column; map in `fromRecord`/`toRecord` | -| `dismissedeventsstorage/DismissedEventEntity.kt` | Same, for dismissed events (Room entity only) | -| `app/ApplicationController.kt` | Populate identity on write; **fix `restoreToActive()` (line 1321) to use the stored blob instead of re-querying the stale ID** | +| `app/ApplicationController.kt` | Write identity rows on event add/update; **fix `restoreToActive()` (line 1321) to use the stored identity instead of re-querying the stale ID** | | `backup/SettingsBackupManager.kt` | Extract calendar-remap logic for reuse in Phase 5 | | `prefs/MiscSettingsFragmentX.kt` | Manual re-link action | -| `res/xml/backup_rules.xml` | Exclude the new fingerprint prefs file | +| `res/xml/backup_rules.xml` | Exclude the new fingerprint prefs file (the identity DB is already covered by `domain="database"`) | | `res/values/strings.xml` | Strings for the action + result dialog | | `eventsstorage/EventAlertDao.kt`, `RoomEventsStorage.kt` | Transactional re-key (delete + insert) for a changed `id` | | `monitorstorage/` + `dismissedeventsstorage/` storages | Re-key rows on an `id` change (Phase 1b), with manual rollback across DBs | -| `docs/architecture/database_schema_reference.md` | Mark `s2` as claimed once implemented | +| `docs/architecture/database_schema_reference.md` | Document the new identity database | ## Testing Plan @@ -357,18 +373,18 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa ### Unit / Robolectric -- **Serialization**: round-trip; unknown future `version` decodes without throwing; malformed/empty `s2` yields null rather than an exception (no broad `catch (Exception)` — catch `SerializationException` specifically). +- **Identity storage**: write/read round-trip by `(eventId, instanceStart)`; absent row returns null cleanly. - **Resolver happy path**: calendar and event both match → new IDs applied. - **Partial match**: calendar matches, UID does not → calendar updated, event left stale, marked unresolved. - **No match**: neither matches → row untouched, still marked pending (proves the retry path and the no-data-loss guarantee). -- **No identity stored**: legacy row with empty `s2` → skipped cleanly. +- **No identity stored**: event with no identity row → skipped cleanly. - **Already current**: IDs unchanged → no write (guards against pointless delete+reinsert churn). - **PK collision**: target `(id, istart)` already occupied → existing row kept, duplicate dropped. - **`cid` committed independently**: calendar resolves but event does not → the `cid` update is still persisted (proves the safe-write-first ordering, and that a partial resolution is an improvement rather than a rollback). - **Transactional re-key**: insert fails mid-re-key → original row still present, nothing lost. -- **Cross-database re-key**: after an `id` change, the matching `manualAlertsV1` and `dismissedEventsV2` rows are re-keyed too; a failure on either leaves all three consistent via manual rollback. +- **Cross-database re-key**: after an `id` change, the identity row plus the matching `manualAlertsV1` and `dismissedEventsV2` rows are re-keyed too; a failure on any leaves all four consistent via manual rollback. - **Orphaned monitor alert regression guard**: re-keyed event can still `restoreToUpcoming` — i.e. `clearWasHandled` finds its alert. This is the concrete failure the cross-DB work exists to prevent. -- **Backfill**: pre-existing row + live provider → `s2` populated. +- **Backfill**: pre-existing event + live provider → identity row created. - **Settings repair**: orphaned `calendar_handled_.N` keys remapped; unmatched ones reported. - **Restore detection**: fingerprint absent/mismatched ⇒ restore; matching ⇒ no-op. @@ -396,4 +412,4 @@ Per `docs/build/wsl_unison_environment.md`, instrumentation runs from Windows (` ## Open Questions - Should a restored-but-unresolved event be visually marked in the list (e.g. the existing `calendarId = -1` "calendar not found" treatment via `createCalendarNotFoundCal`), or stay silent until it resolves? Leaning silent, since the retry usually resolves it within a sync cycle or two. -- `DismissedEventsStorage` carries the identity blob for symmetry, but dismissed events are historical. Worth confirming whether re-resolving them is wanted at all, or whether Phase 0's capture is enough there. +- Dismissed events get identity rows for symmetry, but they are historical. Worth confirming whether re-resolving them is wanted at all, or whether capture alone is enough there. From 43455e4d7cb9d39f6b986613dd051dc24c1717a2 Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 21:50:49 +0000 Subject: [PATCH 06/11] docs: descriptive column names for the identity DB, unstack table rows Two readability fixes. Column names are spelled out rather than abbreviated. The new table had been drafted in the eventsV9 house style (acctName, dispName, origEventId), but that style is a 2016 inheritance kept only because changing it now is risky -- there is no reason for a brand-new table to adopt it. The five calendar columns now map one-to-one onto the CalendarContract.Calendars columns they come from (calendarAccountName, calendarOwnerAccount, ...), and resolveAttempts becomes resolutionAttemptCount. Column names cost schema space, not per-row space. Unstacks the slash-combined table rows. Cramming "acctName / acctType" into one row against "will@example.com / com.google" made the reader match up values positionally for no benefit -- the table had no width pressure. One column per row now. Adds a column-naming section to the schema reference so the rule outlives this plan, using attsts/oattsts as the cautionary case. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- .../architecture/database_schema_reference.md | 10 +++- docs/dev_todo/portable_event_identity.md | 49 ++++++++++++------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/docs/architecture/database_schema_reference.md b/docs/architecture/database_schema_reference.md index 11ed7a24..4f971ea8 100644 --- a/docs/architecture/database_schema_reference.md +++ b/docs/architecture/database_schema_reference.md @@ -13,13 +13,19 @@ Authoritative definitions live in the Room entities; this doc is the human-reada | Database | Room file | Legacy file | Table | Primary key | |---|---|---|---|---| | Events (active/snoozed) | `RoomEvents` | `Events` | `eventsV9` | `(id, istart)` | -| Portable event identity *(planned)* | `RoomEventIdentity` | — (new) | `eventIdentityV1` | `(eventId, instanceStart)` | +| Portable event identity *(planned)* | `RoomEventIdentity` | — (new) | `eventIdentityV1` | `(eventId, instanceStartTime)` | | Dismissed events | `RoomDismissedEvents` | `DismissedEvents` | `dismissedEventsV2` | `(eventId, instanceStart)` | | Calendar monitor | `RoomCalendarMonitor` | `CalendarMonitor` | `manualAlertsV1` | `(eventId, alertTime, instanceStart)` | These are **separate database files**. There are no foreign keys between them and cross-database transactions are not possible — code that must stay consistent across two of them does manual rollback (see `ApplicationController.unsnoozeToUpcoming`). -Note what the shared `(eventId, instanceStart)` key implies: changing an event's `eventId` requires re-keying its rows in *every* one of these databases, with no transaction spanning them. +Note what the shared event key implies: changing an event's `eventId` requires re-keying its rows in *every* one of these databases, with no transaction spanning them. + +## A note on column naming + +The three original tables use abbreviated column names (`cid`, `istart`, `dsts`, `attsts`) inherited from the 2016 schema. They are now costly to change — the names are baked into the legacy `*Impl*` classes, the Supabase mirror table, and the PowerSync payload — so they stay as they are, and this document exists largely to decode them. + +**New tables should not copy that style.** Column names are stored once in the schema, not per row, so abbreviating them buys nothing and costs readability. `attsts` vs `oattsts` is the cautionary case: two adjacent columns whose abbreviated names give no hint that one is the event's status and the other is the user's RSVP. Spell new columns out, and name them after the source they come from where one exists (e.g. a column holding `Calendars.ACCOUNT_NAME` should read `calendarAccountName`). ## `eventsV9` — active and snoozed events diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index 9cc27e1c..83719f19 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -89,20 +89,27 @@ The real cost is the one inherent to this codebase: **a fourth separate database #### Schema -One table, keyed to match `eventsV9`'s primary key so rows join cleanly: +One table, keyed to match `eventsV9`'s primary key so rows join cleanly. One column per row — no slash-combined entries, no abbreviations. | Column | Type | Notes | |---|---|---| -| `eventId` | Long | PK part 1 — matches `eventsV9.id` | -| `instanceStart` | Long | PK part 2 — matches `eventsV9.istart` | -| `acctName` / `acctType` / `owner` / `dispName` / `calName` | String | The `CalendarBackupInfo` tuple, as real columns | +| `eventId` | Long | PK part 1. Joins to `eventsV9.id` | +| `instanceStartTime` | Long | PK part 2. Joins to `eventsV9.istart` | +| `calendarAccountName` | String | `Calendars.ACCOUNT_NAME` — usually the account email | +| `calendarAccountType` | String | `Calendars.ACCOUNT_TYPE` — e.g. `com.google` | +| `calendarOwnerAccount` | String | `Calendars.OWNER_ACCOUNT` | +| `calendarDisplayName` | String | `Calendars.CALENDAR_DISPLAY_NAME` — e.g. "Work" | +| `calendarName` | String | `Calendars.NAME` | | `eventUid` | String? | `Events.UID_2445`, fallback `_SYNC_ID`. Nullable — not every event has one | -| `origCalendarId` | Long | The `cid` in force when captured | -| `origEventId` | Long | The `id` in force when captured | -| `capturedAt` | Long | Via `CNPlusClockInterface`, never `System.currentTimeMillis()` | -| `resolveAttempts` | Int | Backs the Phase 3 retry cap | +| `originalCalendarId` | Long | The `cid` in force when this row was captured | +| `originalEventId` | Long | The `id` in force when this row was captured | +| `capturedAtTime` | Long | Via `CNPlusClockInterface`, never `System.currentTimeMillis()` | +| `resolutionAttemptCount` | Int | Backs the Phase 3 retry cap | +| `lastResolutionAttemptTime` | Long | Backs the retry backoff | -**Real typed columns, not a JSON blob.** Once the constraint of squeezing into one text column is gone, there is no reason to serialize. Typed columns are queryable (e.g. "all rows with `resolveAttempts > N`"), enforced by Room at compile time, and need no `SerializationException` handling. The `resolveAttempts` counter in particular is a plain `UPDATE` rather than a decode/mutate/re-encode cycle. +**Spell the names out.** The `eventsV9` abbreviations (`cid`, `istart`, `dsts`, `attsts`) are a 2016 inheritance that is now costly to change and easy to misread — `attsts` vs `oattsts` being the worst case. This table is new, so it carries no such constraint: the five calendar columns map one-to-one onto the `CalendarContract.Calendars` columns they come from and are named to make that obvious. The storage cost of long column names is per-schema, not per-row. + +**Real typed columns, not a JSON blob.** Once the constraint of squeezing into one text column is gone, there is no reason to serialize. Typed columns are queryable (e.g. "rows where `resolutionAttemptCount` exceeds the cap"), enforced by Room at compile time, and need no `SerializationException` handling. `resolutionAttemptCount` in particular becomes a plain `UPDATE` rather than a decode/mutate/re-encode cycle. It also means no serialization layer at all: the Room entity *is* the model, with no encode/decode step to test or to fail. @@ -166,14 +173,20 @@ One identity row per stored event, holding **everything needed to find that even | Column | Example value | |---|---| -| `eventId` / `instanceStart` | `91427` / `1764547200000` — the join key back to `eventsV9` | -| `acctName` / `acctType` | `will@example.com` / `com.google` | -| `owner` | `will@example.com` | -| `dispName` / `calName` | `Work` / `will@example.com` | +| `eventId` | `91427` | +| `instanceStartTime` | `1764547200000` | +| `calendarAccountName` | `will@example.com` | +| `calendarAccountType` | `com.google` | +| `calendarOwnerAccount` | `will@example.com` | +| `calendarDisplayName` | `Work` | +| `calendarName` | `will@example.com` | | `eventUid` | `abc123def456@google.com` | -| `origCalendarId` / `origEventId` | `3` / `91427` | +| `originalCalendarId` | `3` | +| `originalEventId` | `91427` | + +`eventId` + `instanceStartTime` are the join key back to `eventsV9`. -**Why store `origCalendarId`/`origEventId` when they duplicate the event row?** They are the staleness check. If `origEventId` still equals the event's current `id`, resolution has not run for this row; if they differ, it already has. Without them there is no way to distinguish "never resolved" from "already resolved" — which matters because the retry loop re-runs on every launch and must not redo completed work. +**Why store `originalCalendarId`/`originalEventId` when they duplicate the event row?** They are the staleness check. If `originalEventId` still equals the event's current `id`, resolution has not run for this row; if they differ, it already has. Without them there is no way to distinguish "never resolved" from "already resolved" — which matters because the retry loop re-runs on every launch and must not redo completed work. **Who writes it:** Phase 0 on every event add/update (fresh rows), Phase 2 backfill (pre-existing rows). @@ -221,7 +234,7 @@ This matters concretely: `restoreToUpcoming` (`ApplicationController.kt:1293-130 The same applies to `dismissedEventsV2`, which is keyed on `eventId` in its own database. -The identity database itself is keyed the same way, so it is a fourth store needing the same treatment — its row must move to the new `(eventId, instanceStart)` alongside the event, or the next retry would not find it. +The identity database itself is keyed the same way, so it is a fourth store needing the same treatment — its row must move to the new `(eventId, instanceStartTime)` alongside the event, or the next retry would not find it. So the resolver must re-key **four** databases for a given event, in a defined order, with manual rollback on partial failure — the pattern `unsnoozeToUpcoming` already establishes. This is why the `id` re-key is split into its own sub-phase: `eventsV9` plus its identity row land first, with the monitor and dismissed databases as follow-on steps carrying their own tests. @@ -312,7 +325,7 @@ Split into two sub-phases, because the risk profile is very different: **1a — `cid` only.** Plain column update, no PK change, no cross-database fallout. This alone fixes calendar attribution, filter pills, and per-calendar settings. Independently shippable and independently testable. -**1b — `id` re-key.** The delete+re-insert, transactional per event. Must also re-key the matching rows in `manualAlertsV1` and `dismissedEventsV2`, which live in separate databases with no shared transaction — follow the manual-rollback pattern in `ApplicationController.unsnoozeToUpcoming`. Land this only once 1a is solid. +**1b — `id` re-key.** The delete+re-insert, transactional per event. Must also re-key the event's own identity row plus the matching rows in `manualAlertsV1` and `dismissedEventsV2` — four databases, no shared transaction, so follow the manual-rollback pattern in `ApplicationController.unsnoozeToUpcoming`. Land this only once 1a is solid. This class is the whole substance of the feature; keep it small and free of Android UI dependencies. @@ -373,7 +386,7 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa ### Unit / Robolectric -- **Identity storage**: write/read round-trip by `(eventId, instanceStart)`; absent row returns null cleanly. +- **Identity storage**: write/read round-trip by `(eventId, instanceStartTime)`; absent row returns null cleanly. - **Resolver happy path**: calendar and event both match → new IDs applied. - **Partial match**: calendar matches, UID does not → calendar updated, event left stale, marked unresolved. - **No match**: neither matches → row untouched, still marked pending (proves the retry path and the no-data-loss guarantee). From 7364e195d583d4b99434306b6fc2b9774527c2d5 Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 22:14:06 +0000 Subject: [PATCH 07/11] docs: guard the -1L sentinel, gate backfill on the fingerprint Addresses three PR review threads. findMatchingCalendarId() returns -1L, not null, when all three fallback tiers miss. Since cid = -1 means "unknown, treated as handled" (fail-open), writing that result unguarded would turn a stale-but-plausible calendar ID into one the app silently treats as handled -- making the row quieter rather than better, which inverts the premise the safe-write-first ordering rests on. Requires an explicit != -1L check before any write. The same trap exists in shipping code: restoreToActive()'s elvis catches a null from getCalendarBackupInfo() but passes -1L straight through, so the guard belongs there too. Phase 2 and Phase 3 assume opposite devices, and "any event lacks an identity row" is exactly the condition holding right after a restore. Unguarded, backfill on a restored device would query the provider with a meaningless id and write an identity row from whatever came back -- worse than nothing, since the resolver would then treat the event as covered instead of skipping it. Backfill now runs only when the fingerprint matches, making Phase 3 a prerequisite for Phase 2. Also states what backfill can recover: it inoculates data not yet restored; already-orphaned data is past saving either way. Adds test cases for both, and rephrases the MonitorStorage non-goal so it no longer leads with an exemption it walks back a clause later. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- docs/dev_todo/portable_event_identity.md | 39 +++++++++++++++++++++--- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index 83719f19..84c6e8ec 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -45,7 +45,7 @@ Store durable, provider-independent identity alongside every stored event so tha - **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. - **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. - **Migrating the existing databases** — no schema change to `eventsV9`, `dismissedEventsV2`, or `manualAlertsV1`. The identity database is new and starts at version 1. -- **Storing identity for `MonitorStorage` rows** — `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so it doesn't need its own identity. **But its rows are keyed on `eventId` and must still be re-keyed when an `id` changes** — see the cross-database note in Design Decisions. +- **Storing identity for `MonitorStorage` rows** — no identity row of its own, but its rows still need re-keying when an `id` changes. `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so there is nothing to capture; the re-key is covered in the cross-database note in Design Decisions. ## Key Decisions Summary @@ -201,6 +201,24 @@ Resolution consumes the identity rows described above. A restored event needs tw 1. **Calendar**: `findMatchingCalendarId(context, storedBackupInfo)` → new `cid`. Reuses the existing 3-tier matcher untouched. 2. **Event**: query `Events.CONTENT_URI` for `UID_2445 = ? AND CALENDAR_ID = ?` (scoped to the just-matched calendar to avoid cross-calendar collisions) → new `id`. +#### The `-1L` sentinel must be guarded explicitly + +`findMatchingCalendarId()` signals "no match" by returning **`-1L`, not `null`** (`CalendarProvider.kt:1851`), after all three fallback tiers miss. + +That is not a neutral "unknown" in this schema. As the schema reference records, `cid = -1` means *unknown, treated as handled* — it is fail-open. So writing an unguarded matcher result would convert a stale-but-plausible calendar ID into `-1`, which the app then reads as handled. The row would get quieter rather than better, inverting the premise that rewriting a broken row can only move it toward working. + +**Every use of the matcher's result must check `!= -1L` before writing it.** A no-match is the unresolved path: leave `cid` untouched and retry later. + +The same trap already exists in shipping code. `restoreToActive()` (`ApplicationController.kt:1324-1326`) uses an elvis operator that catches a `null` from `getCalendarBackupInfo()` but passes a `-1L` from the matcher straight through into the event copy: + +```kotlin +val newCalendarId = calendarBackupInfo?.let { backupInfo -> + calendarProvider.findMatchingCalendarId(context, backupInfo) +} ?: event.calendarId // catches null, but not -1L +``` + +Since "Files to Modify" already commits to fixing that line, the guard belongs in both places. + #### Which write is actually dangerous Worth separating, because the two updates carry very different risk: @@ -246,8 +264,8 @@ flowchart TD B -->|"no (pre-Phase 0)"| Z["Skip — backfill handles it"] B -->|yes| C["findMatchingCalendarId
(account tuple)"] - C --> D{"Calendar
matched?"} - D -->|no| Y["Unresolved — keep row,
retry next launch"] + C --> D{"Calendar matched?
(result != -1L)"} + D -->|"no / -1L"| Y["Unresolved — keep row,
cid untouched, retry"] D -->|yes| S["UPDATE cid — safe,
commit now ✅"] S --> E["Query Events for
UID_2445 in that calendar"] @@ -331,12 +349,20 @@ This class is the whole substance of the feature; keep it small and free of Andr ### Phase 2: Backfill for pre-existing rows -Events stored before Phase 0 have no identity row. While the app is still on the *original* device, they can be backfilled by reading identity from the live provider (the stale IDs are still valid there). Run this opportunistically on app start while any event lacks an identity row. +Events stored before Phase 0 have no identity row. On the **original** device they can be backfilled by reading identity from the live provider — this works precisely because nothing is broken yet: the stored IDs are not stale, they are live. + +**Backfill must run only when the Phase 3 fingerprint matches.** This is a hard precondition, not an optimization. Phase 2 and Phase 3 make opposite assumptions about which device you are on, and "any event lacks an identity row" is exactly the condition that holds right after a restore. -This is what makes the feature useful to the current user rather than only to new installs — without it, today's data is still unrestorable. +Running backfill on a restored device would take an `id` that now points at nothing — or worse, at an unrelated event the new device happened to assign that number — query the provider with it, and write an identity row from whatever came back. That is worse than doing nothing: the event would then *have* an identity row, so the resolver treats it as covered instead of skipping it. Best case it never resolves; worst case it points at an unrelated event and the 1b re-key moves the row onto it. + +So the ordering is: detect the install fingerprint first (Phase 3), and only backfill when it matches. Consequently Phase 3's detection logic is a prerequisite for Phase 2 even though it is numbered after it. + +**What backfill can and cannot recover.** It is inoculation for data that has not been restored yet, not a rescue for data already orphaned. Anyone who has already restored onto a new device is past saving regardless of what ships here — the provider IDs that identity would have been derived from are gone. What backfill buys is that the *current* device's data becomes restorable from this point forward, which is what makes the feature useful to existing users rather than only to new installs. ### Phase 3: Restore detection + retry +The fingerprint check here also gates Phase 2, so build it first even though it is numbered later. + Store an install fingerprint in its own SharedPreferences file, and **exclude that file from `backup_rules.xml`** so it does not survive a restore — the same trick `EventsStorageState` already relies on. Absent/mismatched fingerprint on launch ⇒ treat as a restore and mark all events pending re-resolution. Retry semantics: keep pending events marked until each resolves, re-attempting on app start and after calendar rescans, rather than burning the attempt once. Cap attempts with a backoff so a permanently-unmatchable event doesn't re-query forever. @@ -394,12 +420,15 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa - **Already current**: IDs unchanged → no write (guards against pointless delete+reinsert churn). - **PK collision**: target `(id, istart)` already occupied → existing row kept, duplicate dropped. - **`cid` committed independently**: calendar resolves but event does not → the `cid` update is still persisted (proves the safe-write-first ordering, and that a partial resolution is an improvement rather than a rollback). +- **No calendar match writes nothing**: matcher returns `-1L` → `cid` left untouched, *not* set to `-1`. Guards the fail-open sentinel described in Design Decisions. +- **`restoreToActive` sentinel guard**: matcher returns `-1L` during an un-dismiss → the event keeps its original `calendarId` rather than being written to `-1`. - **Transactional re-key**: insert fails mid-re-key → original row still present, nothing lost. - **Cross-database re-key**: after an `id` change, the identity row plus the matching `manualAlertsV1` and `dismissedEventsV2` rows are re-keyed too; a failure on any leaves all four consistent via manual rollback. - **Orphaned monitor alert regression guard**: re-keyed event can still `restoreToUpcoming` — i.e. `clearWasHandled` finds its alert. This is the concrete failure the cross-DB work exists to prevent. - **Backfill**: pre-existing event + live provider → identity row created. - **Settings repair**: orphaned `calendar_handled_.N` keys remapped; unmatched ones reported. - **Restore detection**: fingerprint absent/mismatched ⇒ restore; matching ⇒ no-op. +- **Backfill is gated on the fingerprint**: on a mismatched (restored) fingerprint, backfill writes no identity rows at all — the case that would otherwise manufacture identity from meaningless IDs. Follow the existing pattern in `test/.../calendar/CalendarBackupRestoreRobolectricTest.kt` (injected storage, no native SQLite). From fa42407b1c7bbfc8130e45f02cf7cf4e6be51e5a Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 22:22:03 +0000 Subject: [PATCH 08/11] docs: add heuristic recovery path for already-orphaned data (Phase 6) The plan asserted that anyone who already restored onto a new device is "past saving regardless of what ships here." That was stated without checking, and it is wrong. An orphaned row is not empty: it still holds title, startTime, instanceStartTime, location and isAllDay. instanceStartTime is UTC epoch millis derived from the event's real start, so unlike id and cid it is NOT device-assigned -- it is the same value on the new device for the same event. That makes it a usable matching key. Adds Phase 6: query CalendarContract.Instances over a window bracketing the stored instanceStartTime (same API the existing instance scan already uses at CalendarProvider.kt:1594), filter candidates by title/isAllDay/location, and accept only when exactly one survives. A match yields both a real eventId and calendarId, and lets an identity row be written so the event is protected against the next restore. Marked optional and manual-only, with its limits stated rather than glossed: recurring events with identical titles are the weak case, renamed or moved events will not match, and ambiguity is declined outright. Acceptable because the failure mode is "still unresolved" -- exactly where the row already sits -- so it can only improve matters. The sync-window section notes it bounds Phase 6 too. Also notes the hit rate should be measured with the existing test_cloud_backup.sh harness rather than assumed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- docs/dev_todo/portable_event_identity.md | 37 +++++++++++++++++++++++- 1 file changed, 36 insertions(+), 1 deletion(-) diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index 84c6e8ec..c4735384 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -44,6 +44,7 @@ Store durable, provider-independent identity alongside every stored event so tha - **Bidirectional multi-device sync of snooze/mute state** — the second half of #273. This plan makes the *data* portable; it does not make it live-shared. - **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. - **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. +- **Guaranteed recovery of already-orphaned data** — Phase 6 offers a best-effort heuristic pass for devices restored before this feature shipped, but it is explicitly partial: recurring, renamed, and duplicate-titled events will not match. Not a guarantee, and gated behind a manual action. - **Migrating the existing databases** — no schema change to `eventsV9`, `dismissedEventsV2`, or `manualAlertsV1`. The identity database is new and starts at version 1. - **Storing identity for `MonitorStorage` rows** — no identity row of its own, but its rows still need re-keying when an `id` changes. `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so there is nothing to capture; the re-key is covered in the cross-database note in Design Decisions. @@ -296,6 +297,8 @@ A real constraint on how much this feature can ever recover, and worth stating p So the 12-month floor sits well outside the range this feature actually operates in. The honest framing is that **this is a limit on the tail, not on the feature.** +Note this bounds Phase 6 as well: heuristic matching queries the same provider, so an event outside the sync window has no candidates to match against regardless of how good the heuristic is. + **What happens to an event outside the window:** exactly the unresolved path already specified — the row is kept, marked unresolved, and retried. It never resolves, which is correct: the event genuinely isn't on this device. The app already renders this gracefully via `createCalendarNotFoundCal` (`CalendarProvider.kt:1384`). No crash, no data loss, no special-casing needed. This does mean the retry cap from Phase 3 matters — without a backoff, permanently-unmatchable old events would re-query the provider forever. @@ -357,7 +360,9 @@ Running backfill on a restored device would take an `id` that now points at noth So the ordering is: detect the install fingerprint first (Phase 3), and only backfill when it matches. Consequently Phase 3's detection logic is a prerequisite for Phase 2 even though it is numbered after it. -**What backfill can and cannot recover.** It is inoculation for data that has not been restored yet, not a rescue for data already orphaned. Anyone who has already restored onto a new device is past saving regardless of what ships here — the provider IDs that identity would have been derived from are gone. What backfill buys is that the *current* device's data becomes restorable from this point forward, which is what makes the feature useful to existing users rather than only to new installs. +**What backfill can and cannot recover.** Backfill itself is inoculation for data that has not been restored yet, not a rescue for data already orphaned: it derives identity from the live provider using the stored `id`, which only works while that `id` is still valid. On an already-restored device it is unusable — hence the fingerprint gate above. + +That does **not** mean already-restored data is unrecoverable, only that backfill is the wrong tool for it. A separate heuristic pass can recover a useful fraction of it, because an orphaned row is not empty — it still holds `title`, `startTime`, `instanceStartTime`, `location`, and `isAllDay`. See Phase 6. ### Phase 3: Restore detection + retry @@ -375,6 +380,32 @@ Mirror `prefs/CalendarsActivity.kt:196-243`: a "Re-link events to calendars" act On a detected restore, rewrite orphaned `calendar_handled_.` keys to their new IDs using the same matcher. `SettingsBackupManager.importCalendarSettings()` (`backup/SettingsBackupManager.kt:388-435`) already does exactly this from a JSON file — the logic should be extracted and shared rather than duplicated. +### Phase 6: Heuristic recovery for already-orphaned data (optional) + +Everything above only helps devices that captured identity *before* the restore. This phase is the one path that helps data already sitting orphaned on a restored device — including the maintainer's own, which is the main reason it is worth building. + +**Why it is possible at all.** An orphaned row's IDs are meaningless, but the row is not empty. It still carries `title`, `startTime`, `instanceStartTime`, `location`, and `isAllDay`. Crucially, **`instanceStartTime` is UTC epoch millis derived from the event's real start time**, so it is the *same value* on the new device for the same event — it is not device-assigned the way `id` and `cid` are. + +**The matching pass.** For each unresolved row with no identity: + +1. Query `CalendarContract.Instances.query()` over a narrow window bracketing the stored `instanceStartTime` (the existing instance-scan code at `CalendarProvider.kt:1594` already uses this API, so the query shape is proven). +2. Filter candidates by exact `title` match, then `isAllDay`, then `location` where present. +3. Accept **only when exactly one candidate survives.** Two or more ⇒ ambiguous ⇒ leave unresolved. + +Once matched, the row yields both a real `eventId` and its `calendarId`, and can be re-keyed through the same Phase 1b machinery — and an identity row can be written for it, so it is protected against the *next* restore. + +**Why this is Phase 6 and marked optional.** It is a genuine heuristic, unlike the UID match, which is exact: + +- **Recurring events are the weak case.** A weekly standup has many instances with identical titles; only the instance start time separates them, so a slightly shifted series produces either no match or an ambiguous one. Correct behaviour there is to decline. +- **Renamed or moved events will not match**, since both signals are content-based. +- **Requires a one-to-one survivor.** Duplicate-titled events at the same time are declined outright. + +Those limits are acceptable *because the alternative is nothing*. The failure mode is "still unresolved", exactly where the row already is — this pass can only improve matters, never worsen them, provided the single-candidate rule is strict. + +**Safety.** Same rules as everywhere else: resolve before writing, never write on ambiguity, and reuse the transactional re-key from Phase 1b. Given it is heuristic, it should be **opt-in via the manual re-link action rather than automatic**, so a mis-match is a user-initiated action with a visible report (`matched / ambiguous / unmatched`) rather than a silent background rewrite. + +**Verification.** `scripts/test_cloud_backup.sh` already drives a real backup/uninstall/reinstall cycle, so the honest measurement is available: restore, count how many rows this pass resolves, and report the rate. That number decides whether Phase 6 is worth keeping, and it should be measured rather than assumed. + ## Files to Modify/Create ### New Files @@ -386,6 +417,7 @@ On a detected restore, rewrite orphaned `calendar_handled_.` keys to thei | `identitystorage/EventIdentityDatabase.kt` | Room DB `RoomEventIdentity`, version 1 | | `identitystorage/EventIdentityStorage.kt` | Storage facade matching existing conventions | | `calendar/EventIdentityResolver.kt` | Resolution engine and typed outcomes | +| `calendar/HeuristicEventMatcher.kt` | Phase 6 content-based matching (optional phase) | | `test/.../calendar/EventIdentityResolverRobolectricTest.kt` | Core resolution logic tests | | `androidTest/.../identitystorage/EventIdentityStorageTest.kt` | Real-SQLite storage round-trip | | `androidTest/.../calendar/EventIdentityRestoreTest.kt` | Real-provider end-to-end | @@ -429,6 +461,9 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa - **Settings repair**: orphaned `calendar_handled_.N` keys remapped; unmatched ones reported. - **Restore detection**: fingerprint absent/mismatched ⇒ restore; matching ⇒ no-op. - **Backfill is gated on the fingerprint**: on a mismatched (restored) fingerprint, backfill writes no identity rows at all — the case that would otherwise manufacture identity from meaningless IDs. +- **Heuristic match, single candidate** (Phase 6): one instance at the stored time with a matching title → resolved, and an identity row written for future restores. +- **Heuristic match, ambiguous** (Phase 6): two same-titled instances at the same time → declined, row left unresolved. The rule that keeps a heuristic safe. +- **Heuristic match, renamed event** (Phase 6): title differs → no match, row untouched. Follow the existing pattern in `test/.../calendar/CalendarBackupRestoreRobolectricTest.kt` (injected storage, no native SQLite). From d032b3637ccfa0be4c9e374032b7f84b857e97ff Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 22:31:49 +0000 Subject: [PATCH 09/11] docs: state the capture-before-move precondition up front The plan never said plainly that UID_2445 has to be captured on the OLD device while its IDs are still valid. Read without that, "use the UID to find the event" sounds like a lookup that works from nothing -- it doesn't. The app cannot ask a new device for the UID of event 91427, because 91427 means nothing there. Adds a precondition section to the Goal: an ASCII diagram of the snapshot-then-search flow, the blunt practical consequence (this works properly only if you still have the old device, or a backup taken from it after this ships), and a two-tier table separating the exact path (Phases 0-5, requires that backup) from the best-effort one (Phase 6, requires nothing). Also flags the misreading directly in the UID section. Per review, Phase 6 now skips recurring events outright rather than attempting and usually declining them -- they were the known weak case and are not worth the risk for the fraction they would recover. Reframes Phase 6 as explicitly best-effort rather than partial-but- trying, with a matching test case. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- docs/dev_todo/portable_event_identity.md | 54 ++++++++++++++++++++---- 1 file changed, 45 insertions(+), 9 deletions(-) diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index c4735384..e40791f7 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -38,13 +38,45 @@ It looks up the **old** calendar ID against the **new** device's provider — wh Store durable, provider-independent identity alongside every stored event so that a database restored onto a new phone can re-resolve itself to the correct local calendar and event rows. After this, restoring a backup (or just letting Android auto-backup do its thing) yields a working event list: correct calendar attribution, working filter pills, working "open in calendar", and correct per-calendar handled settings. +### The precondition: identity must be captured *before* the move + +This is the single most important thing to understand about the design, and it is easy to misread. + +`UID_2445` is **not** something that can be looked up from an orphaned row. The app has no way to ask the new device "what is the UID of event `91427`?", because `91427` is a row number that means nothing there. The UID has to be read from the provider on the **old** device, while the IDs are still valid, and stored. + +So the mechanism is a snapshot, not a lookup: + +``` +OLD DEVICE NEW DEVICE + event id=91427 ──read UID from──► provider + │ provider + ▼ + identity row (restore) + uid=abc123@google.com ────────────────────► search provider for + uid=abc123@google.com + │ + ▼ + found: id=55310 ✅ +``` + +**The practical consequence:** this feature works properly only if you still have the old device, or a backup taken from it **after this feature ships**. A backup made before then contains no identity rows, and no amount of cleverness on the new device recovers them. + +### Two tiers, deliberately + +| | Requires | Accuracy | Covers | +|---|---|---|---| +| **Exact** (Phases 0–5) | A backup taken after this ships | Exact — UID match, no guessing | The intended path | +| **Best-effort** (Phase 6) | Nothing; works on any orphaned row | Heuristic — may decline | Older backups, already-restored devices | + +Phase 6 exists precisely because the precondition above will not always hold — including for data already sitting orphaned today. It matches on content that survives a restore (title + instance start time) rather than on identity, so it is partial by nature and declines rather than guesses. It is a safety net under the main mechanism, not a substitute for it. + ## Non-Goals - **Login / accounts (Google, Zitadel)** — the other half of #273; tracked separately. - **Bidirectional multi-device sync of snooze/mute state** — the second half of #273. This plan makes the *data* portable; it does not make it live-shared. - **Changing the PowerSync/Supabase payload** — `cid` still ships raw to Supabase. Worth fixing later (it has no account context), but it's sync-side and out of scope here. See `docs/dev_todo/data_sync_improvements.md`. - **A user-facing events export/import file** — this plan rides on the existing Android auto-backup of the DB files. A manual events export is a separate feature. -- **Guaranteed recovery of already-orphaned data** — Phase 6 offers a best-effort heuristic pass for devices restored before this feature shipped, but it is explicitly partial: recurring, renamed, and duplicate-titled events will not match. Not a guarantee, and gated behind a manual action. +- **Guaranteed recovery when identity was never captured** — Phase 6 is a best-effort heuristic for older backups and already-restored devices. It is explicitly partial: recurring events are skipped by design, and renamed or duplicate-titled events will not match. Not a guarantee, and gated behind a manual action. - **Migrating the existing databases** — no schema change to `eventsV9`, `dismissedEventsV2`, or `manualAlertsV1`. The identity database is new and starts at version 1. - **Storing identity for `MonitorStorage` rows** — no identity row of its own, but its rows still need re-keying when an `id` changes. `MonitorAlertEntity.toAlertEntry()` drops `calendarId` entirely and the table is short-lived scan state rebuilt from the provider, so there is nothing to capture; the re-key is covered in the cross-database note in Design Decisions. @@ -148,6 +180,8 @@ or, on Google Calendar, typically something closer to `abc123def456@google.com`. The key property: **the server assigns it, so it's the same string on every device that syncs that event.** `_ID` is assigned locally by whichever device happened to insert the row first, which is exactly why it doesn't survive a restore. +To be explicit, since this is the easy misreading: the UID is useful only because we **stored it on the old device**. It is a value we carry with us, not one the new device can derive from an orphaned row — see the precondition in the Goal. + ```mermaid flowchart LR G["Google Calendar
UID abc123@google.com"] --> P1["Old phone
_ID 91427"] @@ -380,27 +414,28 @@ Mirror `prefs/CalendarsActivity.kt:196-243`: a "Re-link events to calendars" act On a detected restore, rewrite orphaned `calendar_handled_.` keys to their new IDs using the same matcher. `SettingsBackupManager.importCalendarSettings()` (`backup/SettingsBackupManager.kt:388-435`) already does exactly this from a JSON file — the logic should be extracted and shared rather than duplicated. -### Phase 6: Heuristic recovery for already-orphaned data (optional) +### Phase 6: Best-effort recovery when there is no identity (optional) -Everything above only helps devices that captured identity *before* the restore. This phase is the one path that helps data already sitting orphaned on a restored device — including the maintainer's own, which is the main reason it is worth building. +Phases 0–5 all depend on the precondition in the Goal: identity was captured on the old device. Phase 6 is the fallback for when it was not — an older backup, or data already sitting orphaned on a restored device today. That last case is the main reason it is worth building. **Why it is possible at all.** An orphaned row's IDs are meaningless, but the row is not empty. It still carries `title`, `startTime`, `instanceStartTime`, `location`, and `isAllDay`. Crucially, **`instanceStartTime` is UTC epoch millis derived from the event's real start time**, so it is the *same value* on the new device for the same event — it is not device-assigned the way `id` and `cid` are. **The matching pass.** For each unresolved row with no identity: -1. Query `CalendarContract.Instances.query()` over a narrow window bracketing the stored `instanceStartTime` (the existing instance-scan code at `CalendarProvider.kt:1594` already uses this API, so the query shape is proven). -2. Filter candidates by exact `title` match, then `isAllDay`, then `location` where present. -3. Accept **only when exactly one candidate survives.** Two or more ⇒ ambiguous ⇒ leave unresolved. +1. **Skip recurring events outright** (`isRepeating`). They are the known weak case — a weekly standup has many identically-titled instances, so the signals cannot separate them reliably. Not worth the risk for the fraction it would recover; decline and move on. +2. Query `CalendarContract.Instances.query()` over a narrow window bracketing the stored `instanceStartTime` (the existing instance-scan code at `CalendarProvider.kt:1594` already uses this API, so the query shape is proven). +3. Filter candidates by exact `title` match, then `isAllDay`, then `location` where present. +4. Accept **only when exactly one candidate survives.** Two or more ⇒ ambiguous ⇒ leave unresolved. Once matched, the row yields both a real `eventId` and its `calendarId`, and can be re-keyed through the same Phase 1b machinery — and an identity row can be written for it, so it is protected against the *next* restore. -**Why this is Phase 6 and marked optional.** It is a genuine heuristic, unlike the UID match, which is exact: +**Why this is optional and best-effort.** Unlike the UID match, which is exact, this is a genuine heuristic with known gaps: -- **Recurring events are the weak case.** A weekly standup has many instances with identical titles; only the instance start time separates them, so a slightly shifted series produces either no match or an ambiguous one. Correct behaviour there is to decline. +- **Recurring events are skipped by design** (step 1 above). - **Renamed or moved events will not match**, since both signals are content-based. - **Requires a one-to-one survivor.** Duplicate-titled events at the same time are declined outright. -Those limits are acceptable *because the alternative is nothing*. The failure mode is "still unresolved", exactly where the row already is — this pass can only improve matters, never worsen them, provided the single-candidate rule is strict. +Those limits are acceptable *because the alternative is nothing*. The failure mode is "still unresolved" — exactly where the row already sits — so this pass can only improve matters, never worsen them, provided the skip and single-candidate rules stay strict. Best-effort is the goal here, not completeness. **Safety.** Same rules as everywhere else: resolve before writing, never write on ambiguity, and reuse the transactional re-key from Phase 1b. Given it is heuristic, it should be **opt-in via the manual re-link action rather than automatic**, so a mis-match is a user-initiated action with a visible report (`matched / ambiguous / unmatched`) rather than a silent background rewrite. @@ -463,6 +498,7 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa - **Backfill is gated on the fingerprint**: on a mismatched (restored) fingerprint, backfill writes no identity rows at all — the case that would otherwise manufacture identity from meaningless IDs. - **Heuristic match, single candidate** (Phase 6): one instance at the stored time with a matching title → resolved, and an identity row written for future restores. - **Heuristic match, ambiguous** (Phase 6): two same-titled instances at the same time → declined, row left unresolved. The rule that keeps a heuristic safe. +- **Heuristic skips recurring** (Phase 6): `isRepeating` row → not attempted at all, regardless of how good the candidate looks. - **Heuristic match, renamed event** (Phase 6): title differs → no match, row untouched. Follow the existing pattern in `test/.../calendar/CalendarBackupRestoreRobolectricTest.kt` (injected storage, no native SQLite). From 25098e434101dcdbe1a1270add86d513444c6559 Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 22:53:20 +0000 Subject: [PATCH 10/11] docs: an old backup can gain identity if restored where its IDs work "A backup made before then has no identity rows" was true but framed as terminal, which hid a real case. Backfill does not require the old device as such -- it requires the stored id to still resolve against whatever provider is present. That holds on the original device, and equally for an older backup restored back ONTO that device. So there are three cases, not two, and the middle one is recoverable: restore the old backup onto the original device, let backfill run, and the data becomes identity-bearing and therefore portable from then on. That is a genuine migration path for pre-feature data: restore old -> backfill -> re-backup -> move. This also exposes a bug in the Phase 2 gate. Gating purely on the install fingerprint is too blunt, because restoring an old backup onto the original device ALSO clears the fingerprint -- so the gate would have skipped backfill in exactly the case we want it to run. Replaces it with a cheap validation sample: look up a handful of stored ids and compare returned title/start against the stored rows. Agreement means the ids are live and backfill is safe; disagreement or empty results mean a new provider, so skip to Phase 6. The fingerprint stays as a fast path rather than the sole gate. Adds tests for both directions, including the same-device restore that must not be misread as a new-device restore. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- docs/dev_todo/portable_event_identity.md | 30 +++++++++++++++++------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index e40791f7..105d7340 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -59,7 +59,17 @@ OLD DEVICE NEW DEVICE found: id=55310 ✅ ``` -**The practical consequence:** this feature works properly only if you still have the old device, or a backup taken from it **after this feature ships**. A backup made before then contains no identity rows, and no amount of cleverness on the new device recovers them. +**The practical consequence** depends on where the backup is restored, and it is worth being precise because there are three cases, not two: + +| Case | Identity rows present? | Can they be created? | +|---|---|---| +| Backup taken **after** this ships | Yes — captured at write time | n/a, already there | +| Older backup, restored onto the **same** device (or one whose provider still holds those IDs) | No | **Yes** — Phase 2 backfill manufactures them from the live provider, because the stored IDs are still valid there | +| Older backup, restored onto a **new** device | No | **No** — the IDs resolve to nothing or to unrelated events, so Phase 6's content heuristic is the only option | + +The middle row is the one worth noticing: **an old backup is not automatically a lost cause.** Restore it onto the original device, let backfill run, and the data becomes identity-bearing — and therefore portable from then on. That is a genuine migration path for data captured before this feature existed: restore old → backfill → re-backup → move. + +The case that truly cannot be fixed is the last one: identity was never captured, and the provider that could have supplied it is gone. That is exactly what Phase 6 exists for, and why it is best-effort rather than exact. ### Two tiers, deliberately @@ -388,19 +398,21 @@ This class is the whole substance of the feature; keep it small and free of Andr Events stored before Phase 0 have no identity row. On the **original** device they can be backfilled by reading identity from the live provider — this works precisely because nothing is broken yet: the stored IDs are not stale, they are live. -**Backfill must run only when the Phase 3 fingerprint matches.** This is a hard precondition, not an optimization. Phase 2 and Phase 3 make opposite assumptions about which device you are on, and "any event lacks an identity row" is exactly the condition that holds right after a restore. +**What backfill actually requires.** Not "the original device" as such — it requires the stored `id` to still resolve correctly against whatever provider is present. That holds on the original device, and it also holds for an **older backup restored back onto that same device**, which is why that migration path works (see the three cases in the Goal). + +The danger is running it when the IDs *don't* resolve. On a genuinely new device, backfill would take an `id` that now points at nothing — or worse, at an unrelated event the new device happened to assign that number — query the provider with it, and write an identity row from whatever came back. That is worse than doing nothing: the event would then *have* an identity row, so the resolver treats it as covered instead of skipping it. Best case it never resolves; worst case it points at an unrelated event and the 1b re-key moves the row onto it. -Running backfill on a restored device would take an `id` that now points at nothing — or worse, at an unrelated event the new device happened to assign that number — query the provider with it, and write an identity row from whatever came back. That is worse than doing nothing: the event would then *have* an identity row, so the resolver treats it as covered instead of skipping it. Best case it never resolves; worst case it points at an unrelated event and the 1b re-key moves the row onto it. +**The gate must therefore distinguish "same provider" from "new provider", not merely "restored or not".** A plain fingerprint-missing check is too blunt: restoring an old backup onto the original device also clears the fingerprint, and that is precisely the case we want backfill to run in. -So the ordering is: detect the install fingerprint first (Phase 3), and only backfill when it matches. Consequently Phase 3's detection logic is a prerequisite for Phase 2 even though it is numbered after it. +So the check should be a **cheap validation sample** rather than a pure fingerprint test: take a handful of stored events, look up each `id` in the provider, and compare the returned title and start time against the stored row. If they agree, the IDs are live and backfill is safe. If they disagree or come back empty, treat it as a new provider and skip to the Phase 6 path. The fingerprint remains useful as a fast path — matching fingerprint means definitely same install, no sampling needed — but a mismatch should trigger validation rather than an outright skip. -**What backfill can and cannot recover.** Backfill itself is inoculation for data that has not been restored yet, not a rescue for data already orphaned: it derives identity from the live provider using the stored `id`, which only works while that `id` is still valid. On an already-restored device it is unusable — hence the fingerprint gate above. +Consequently Phase 3's detection logic is still a prerequisite for Phase 2, but as an optimization rather than the sole gate. -That does **not** mean already-restored data is unrecoverable, only that backfill is the wrong tool for it. A separate heuristic pass can recover a useful fraction of it, because an orphaned row is not empty — it still holds `title`, `startTime`, `instanceStartTime`, `location`, and `isAllDay`. See Phase 6. +**What backfill can and cannot recover.** It creates identity for data whose IDs are still valid, whether that data was written here originally or restored from an older backup onto the same device. It cannot help once the IDs are meaningless — that is Phase 6's job, matching on content (`title`, `startTime`, `instanceStartTime`, `location`, `isAllDay`) rather than identity. ### Phase 3: Restore detection + retry -The fingerprint check here also gates Phase 2, so build it first even though it is numbered later. +The fingerprint check here feeds Phase 2's gate as a fast path, so build it first even though it is numbered later. Note Phase 2 does not rely on it alone — see the validation-sample check there, which is what allows an old backup restored onto the same device to still backfill. Store an install fingerprint in its own SharedPreferences file, and **exclude that file from `backup_rules.xml`** so it does not survive a restore — the same trick `EventsStorageState` already relies on. Absent/mismatched fingerprint on launch ⇒ treat as a restore and mark all events pending re-resolution. @@ -495,7 +507,9 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa - **Backfill**: pre-existing event + live provider → identity row created. - **Settings repair**: orphaned `calendar_handled_.N` keys remapped; unmatched ones reported. - **Restore detection**: fingerprint absent/mismatched ⇒ restore; matching ⇒ no-op. -- **Backfill is gated on the fingerprint**: on a mismatched (restored) fingerprint, backfill writes no identity rows at all — the case that would otherwise manufacture identity from meaningless IDs. +- **Backfill declines on a new provider**: validation sample disagrees (stored title/start do not match what the `id` returns) → backfill writes no identity rows. The case that would otherwise manufacture identity from meaningless IDs. +- **Backfill proceeds on the same provider**: fingerprint cleared by a restore, but the validation sample agrees → backfill still runs. Covers restoring an older backup onto the original device, which must not be treated as a new-device restore. +- **Validation sample on an empty provider**: lookups return nothing → treated as a new provider, not as agreement. - **Heuristic match, single candidate** (Phase 6): one instance at the stored time with a matching title → resolved, and an identity row written for future restores. - **Heuristic match, ambiguous** (Phase 6): two same-titled instances at the same time → declined, row left unresolved. The rule that keeps a heuristic safe. - **Heuristic skips recurring** (Phase 6): `isRepeating` row → not attempted at all, regardless of how good the candidate looks. From fd53116f7f8bf71b67104494ebcb419da50ccbc8 Mon Sep 17 00:00:00 2001 From: William Harris Date: Sun, 20 Sep 2026 23:15:38 +0000 Subject: [PATCH 11/11] docs: fix backup rules gap that would break restore detection silently Addresses three review threads, one of them blocking. backup_rules.xml is not read on Android 12+. The manifest declares only fullBackupContent (API 30 and below) while targetSdkVersion is 36; API 31+ reads android:dataExtractionRules, which is not declared, so the platform falls back to backing up everything. That has been harmless because backup_rules.xml is almost all -- but Phase 3 is the first thing here needing an . As written, the fingerprint would be backed up, always match on launch, and a restore would never be detected, leaving Phases 1/2/5 waiting on a signal that never fires. Adds data_extraction_rules.xml as Phase 3's first step, excluding the fingerprint from both and -- the latter matters separately since Android 12 split cable transfer from cloud backup with independent rules. Corrects the "backed up with no config change" claim: true today only because of the platform default, not because the is read. Once the rules file exists the include must be repeated in both sections or the identity DB silently stops being backed up. _SYNC_ID does not stack with UID_2445 the way the text implied -- AOSP notes _sync_id is null for never-synced events, the same case UID_2445 misses. Adds a de-risking step before Phase 0: verify UID_2445 is actually populated on real calendars, given a long-standing unresolved Android issue claiming it is always null. If sparse, Phase 6's heuristic becomes primary rather than a fallback. Notes EventsStorageState's stale exclusion comment as a separate pre-existing bug, and adds D2D transport testing to verification. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01N34fouw76cNkoFVg6V3j4Z --- docs/dev_todo/portable_event_identity.md | 70 ++++++++++++++++++++++-- 1 file changed, 66 insertions(+), 4 deletions(-) diff --git a/docs/dev_todo/portable_event_identity.md b/docs/dev_todo/portable_event_identity.md index 105d7340..cea3b7be 100644 --- a/docs/dev_todo/portable_event_identity.md +++ b/docs/dev_todo/portable_event_identity.md @@ -125,7 +125,7 @@ That last point inverts an argument from the earlier revision. Using `s2` was or Less than it might appear, because the infrastructure is already in place: - **No schema migration.** A brand-new database starts at `version = 1` with no legacy predecessor and no copy-migration step — strictly simpler than the existing three, which all carry legacy baggage. `monitorstorage/MonitorDatabase.kt` is the closest template: single entity, `version = 1`, `exportSchema = false`. -- **Backed up automatically.** `res/xml/backup_rules.xml` includes ``, so any new database file is covered with no config change. This is essential — the identity DB is useless unless it restores alongside the events it describes. +- **Backed up by default today, but this needs care.** Any new database file is currently covered — though on Android 12+ that is because the platform backs up everything when `dataExtractionRules` is absent, *not* because `backup_rules.xml`'s `` is read (it isn't; see Phase 3). Once Phase 3 adds a `data-extraction-rules` file, that include must be repeated in **both** the `` and `` sections, or the identity DB silently stops being backed up on one path. This is essential — the identity DB is useless unless it restores alongside the events it describes. - **No sync impact.** PowerSync/cr-sqlite is wired to `eventsV9` explicitly (`src/lib/features/SetupSync.tsx`, `src/lib/powersync/Schema.tsx`); a new table is invisible to it. The real cost is the one inherent to this codebase: **a fourth separate database file means a fourth store with no cross-database transactions.** Identity rows can drift from the events they describe — e.g. an event is deleted but its identity row lingers. This is tolerable because identity rows are pure derived metadata: an orphaned one is harmless, and the resolver ignores identity with no matching event. A periodic cleanup pass can prune orphans opportunistically; correctness never depends on it. @@ -208,7 +208,18 @@ Title+time heuristics produce false positives on recurring and duplicated events **Locally-created events that never synced to an account may have a null/empty `UID_2445`.** Those events are also the ones least likely to exist on the new phone at all — a local-only calendar isn't restored by Google. They degrade to unresolved and keep their stale ID: no crash, no data loss. -`_SYNC_ID` is the fallback when `UID_2445` is empty. It's also server-assigned and stable, but it's the sync adapter's own key rather than the portable iCalendar one, so it's second choice. +`_SYNC_ID` is the fallback when `UID_2445` is empty, but it does **not** stack the way one might assume. AOSP's `CalendarProvider2` notes that *"if this event hasn't been sync'ed with the server yet, the `_sync_id` field will be null"* — so `_SYNC_ID` is null in exactly the never-synced case described above. The fallback covers only the narrower situation where an event *did* sync but the provider populated `_SYNC_ID` without `UID_2445`. Worth having, but the never-synced case degrades to unresolved regardless of it. + +#### De-risk before building: confirm `UID_2445` is actually populated + +The entire exact-match path assumes this column has values. There is a long-standing Android issue titled ["CalendarContract.Events.UID_2445 column is always null"](https://issuetracker.google.com/issues/37053160) whose resolution is not publicly readable, so the assumption should be tested rather than trusted. + +**Do this first, before any of Phase 0.** One throwaway query across a few real calendars — count non-null `UID_2445` against total events, broken down by account type — settles it in minutes. + +The result changes the plan materially: + +- **Well populated** ⇒ proceed as written; Phase 6 stays an optional safety net. +- **Sparse or null on common providers** ⇒ the exact path is not viable, and **Phase 6's heuristic becomes the primary mechanism** rather than the fallback. That is a different plan, and better to discover now than after Phases 0–5 are built on it. **The email tuple is not perfectly unique either** — this is why `findMatchingCalendarId()` already has three tiers. One account can expose several calendars (primary, birthdays, a shared team calendar), all with the same `ACCOUNT_NAME`. That's why the match uses account name + type + owner, and only falls back to display name. @@ -365,6 +376,8 @@ The key insight: the calendar half of this problem was already solved once for s ### Phase 0: Capture identity at write time +**Before anything else:** confirm `UID_2445` is actually populated on real calendars (see Design Decisions). If it is sparse, the exact-match path is not viable and Phase 6 becomes primary — that determination should happen before writing any of the code below. + **0a — Identity storage.** New `identitystorage/` package following the shape of `monitorstorage/`: `EventIdentityEntity` (the schema in Design Decisions), `EventIdentityDao`, and `EventIdentityDatabase` at `version = 1`, name `RoomEventIdentity`. No legacy predecessor and no copy-migration — this is a fresh database. Use `CrSqliteRoomFactory` for consistency with the existing three. **0b — Read the UID from the provider.** Add `Events.UID_2445` (with `_SYNC_ID` fallback) to the projection in `CalendarProvider.getEvent()` (`calendar/CalendarProvider.kt:418-439`) and expose it on `EventRecord`. Keep it nullable — not every event has one. @@ -414,7 +427,47 @@ Consequently Phase 3's detection logic is still a prerequisite for Phase 2, but The fingerprint check here feeds Phase 2's gate as a fast path, so build it first even though it is numbered later. Note Phase 2 does not rely on it alone — see the validation-sample check there, which is what allows an old backup restored onto the same device to still backfill. -Store an install fingerprint in its own SharedPreferences file, and **exclude that file from `backup_rules.xml`** so it does not survive a restore — the same trick `EventsStorageState` already relies on. Absent/mismatched fingerprint on launch ⇒ treat as a restore and mark all events pending re-resolution. +Store an install fingerprint in its own SharedPreferences file that must **not** survive a restore. Absent/mismatched fingerprint on launch ⇒ treat as a restore and mark all events pending re-resolution. + +#### Prerequisite: `backup_rules.xml` is not read on Android 12+ + +This must be fixed **before** Phase 3 works at all, and it is easy to miss because the current setup only appears to work. + +The manifest declares the legacy attribute only: + +```xml +android:allowBackup="true" +android:fullBackupContent="@xml/backup_rules" +``` + +but `targetSdkVersion = 36`. `fullBackupContent` applies to **API 30 and below**; API 31+ reads `android:dataExtractionRules`, which is not declared. With it absent the platform falls back to its default — back up everything except no-backup and cache dirs. + +That has been harmless so far because `backup_rules.xml` is almost entirely ``, and "include everything" is a superset of that. It stops being harmless here: **Phase 3 is the first thing in this codebase that needs an ``.** On any Android 12+ device the fingerprint would be backed up with everything else, always match on launch, and a restore would never be detected — leaving Phases 1, 2 and 5 waiting for a signal that never fires. The feature would fail silently, which is the worst way for it to fail. + +The fix, as Phase 3's first step: + +1. Add `res/xml/data_extraction_rules.xml` and declare `android:dataExtractionRules` alongside the existing `fullBackupContent` — keep both, since API 24–30 devices still read the old one. +2. Exclude the fingerprint prefs from ``. +3. Exclude it from `` as well. + +```xml + + + + + + + + + + +``` + +**`` matters on its own.** Android 12 split direct phone-to-phone transfer (the setup-wizard cable flow) from cloud backup, with independent rules. Excluding only from `` would leave the cable path undetected — and that path is a very common way to reach exactly the scenario this feature exists for. + +**Carry the `` into both sections.** Once a `data-extraction-rules` file exists, the platform default no longer applies, so the database include must be repeated in both blocks. Omitting it from either one would silently stop backing up the identity database on that path, which breaks the feature quietly — the identity DB is useless unless it restores alongside the events it describes. + +**Note on the `EventsStorageState` precedent.** Its doc comment claims *"This prefs file is NOT in backup_rules.xml, so it won't be backed up"* — that is no longer true on API 31+, for exactly the reason above. It is a pre-existing bug and out of scope here, but it means the pattern should not be cited as proven. Worth filing separately. Retry semantics: keep pending events marked until each resolves, re-attempting on app start and after calendar rescans, rather than burning the attempt once. Cap attempts with a backoff so a permanently-unmatchable event doesn't re-query forever. @@ -479,7 +532,9 @@ Those limits are acceptable *because the alternative is nothing*. The failure mo | `app/ApplicationController.kt` | Write identity rows on event add/update; **fix `restoreToActive()` (line 1321) to use the stored identity instead of re-querying the stale ID** | | `backup/SettingsBackupManager.kt` | Extract calendar-remap logic for reuse in Phase 5 | | `prefs/MiscSettingsFragmentX.kt` | Manual re-link action | -| `res/xml/backup_rules.xml` | Exclude the new fingerprint prefs file (the identity DB is already covered by `domain="database"`) | +| `res/xml/backup_rules.xml` | Exclude the fingerprint prefs (API 30 and below) | +| `res/xml/data_extraction_rules.xml` | **New** — API 31+ backup rules; include the databases and exclude the fingerprint in *both* `` and `` | +| `android/app/src/main/AndroidManifest.xml` | Declare `android:dataExtractionRules` alongside the existing `fullBackupContent` | | `res/values/strings.xml` | Strings for the action + result dialog | | `eventsstorage/EventAlertDao.kt`, `RoomEventsStorage.kt` | Transactional re-key (delete + insert) for a changed `id` | | `monitorstorage/` + `dismissedeventsstorage/` storages | Re-key rows on an `id` change (Phase 1b), with manual rollback across DBs | @@ -507,6 +562,7 @@ Tests first, per `AGENTS.md`. `MockCalendarProvider` (`test/.../testutils/MockCa - **Backfill**: pre-existing event + live provider → identity row created. - **Settings repair**: orphaned `calendar_handled_.N` keys remapped; unmatched ones reported. - **Restore detection**: fingerprint absent/mismatched ⇒ restore; matching ⇒ no-op. +- **Fingerprint is genuinely excluded from backup**: verify on an API 31+ device that a backup/restore cycle does *not* carry the fingerprint across — the failure this guards against is silent, so it needs an explicit check rather than an assumption. - **Backfill declines on a new provider**: validation sample disagrees (stored title/start do not match what the `id` returns) → backfill writes no identity rows. The case that would otherwise manufacture identity from meaningless IDs. - **Backfill proceeds on the same provider**: fingerprint cleared by a restore, but the validation sample agrees → backfill still runs. Covers restoring an older backup onto the original device, which must not be treated as a new-device restore. - **Validation sample on an empty provider**: lookups return nothing → treated as a new provider, not as agreement. @@ -534,8 +590,14 @@ Per `docs/build/wsl_unison_environment.md`, instrumentation runs from Windows (` 2. **Instrumentation** (from Windows, after the user runs Unison — *never* run it unprompted): `.\gradlew.bat :app:connectedX8664DebugAndroidTest -Pandroid.testInstrumentationRunnerArguments.class=com.github.quarck.calnotify.calendar.EventIdentityRestoreTest` 3. **Real end-to-end restore** using the existing harness `scripts/test_cloud_backup.sh com.github.quarck.calnotify` — it drives `bmgr` through a real backup/uninstall/reinstall cycle. Before: restored events are orphaned. After: they re-link. This is the actual acceptance test for the issue. + - Run it on an **API 31+** target specifically, since that is where the `dataExtractionRules` gap bites. + - It only exercises the **cloud** transport. For device-to-device, list transports with `adb shell bmgr list transports` and switch to `com.google.android.gms/.backup.migrate.service.D2dTransport`. Note you [cannot restore *from* D2D via `bmgr`](https://lucid.co/techblog/2022/11/14/testing-android-device-to-device-transfer), so that half stays manual. 4. **Manual sanity**: with events snoozed, trigger the manual re-link action and confirm the reported resolved/unresolved counts, filter pills, and "open in calendar" all behave. +## Follow-ups outside this plan + +- **`EventsStorageState`'s backup exclusion is broken on API 31+.** Its doc comment says the prefs file is not backed up because it is absent from `backup_rules.xml`, but that file is not read on API 31+, so the value likely *is* being backed up and restored. Pre-existing and out of scope here, but worth filing — the Phase 3 `data_extraction_rules.xml` work is the natural place to fix it. + ## Open Questions - Should a restored-but-unresolved event be visually marked in the list (e.g. the existing `calendarId = -1` "calendar not found" treatment via `createCalendarNotFoundCal`), or stay silent until it resolves? Leaning silent, since the retry usually resolves it within a sync cycle or two.