makiolaj/agendula
1
0
Fork 0
Code Issues Pull Requests Actions Releases 7 Activity

docs: fix seven defects in the own-store plan

Browse Source 
Reviewed the plan against the code it describes. Two design holes and
five errors.

Instance identity was the real one. The plan deleted the materialised
instances table without saying what replaces the instance row id, which
TasksRepositoryImpl.updateTask passes to updateInstance and which
ListsScreen keys a lazy list by. Two occurrences of one series can
appear in the same list, so taskId alone is not unique and a hash of
(taskId, start) can collide - as a Compose key that is a visible bug.
Task.id is dropped for occurrenceStart, updateInstance takes
(taskId, occurrenceStart, form), and External mode maps back to a real
instance row with one query. This is the single seam change, and the
plan's "TasksDataSource unchanged" claim was wrong.

Local lists had no account name. TaskList.accountName is non-null,
ListsViewModel groups by it and ListsScreen renders it as a section
header, so a null account_id must still report "Local".

The unique index was wrong: overrides share their master's UID, so
unique (list_id, uid) would reject the rows the recurrence design
depends on. It needs recurrence_id in the key.

Phase 0 broke background reminders. It dropped our authority from
ProviderChangeReceiver's manifest filter while the provider was still
the store, and renamed StorageMode.LOCAL to OWN four phases before OWN
meant Room. Both moved to phase 5.

Parity against the provider was overclaimed: the provider materialises
one occurrence, so multi-occurrence expansion has nothing to compare
against and is tested against RFC 5545 directly.

The phases sum to 6.5-7 weeks, not the 6-6.5 stated, and the difference
from STORAGE-DECISION.md's 4.5-6 is now explained rather than left as a
contradiction.

Gaps closed: WAL vs Auto Backup (checkpoint on ON_STOP, sidecars in the
backup rules, tested in phase 6), cascade rules for master_id and
parent_id, Instant type converters, a rollback path that re-runs the
import from tasks.db.imported, the release note for dropping the
authority and its permissions, and ICalendarWriter.uidFor's synthesis
branch becoming External-only.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Jean-Luc Makiola
2026-08-13 15:30:37 +02:00
parent 13cb27b2ab
commit 8c3cbcf928
3 changed files with 332 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files

108
CLAUDE.md Normal file
View File

@@ -0,0 +1,108 @@
## On-device work (USB-connected phone)
A physical phone is connected over USB. Rules:
- **Install when asked** — if the user says to install/deploy on device, do it
(build + `adb install`). That's the one action you may take on your own.
- **Do nothing else on the device unprompted.** Do not launch the app, take
screenshots, dump/read logcat, poke UI, or otherwise "test" or verify on the
device on your own initiative — even to confirm a change works.
- Read logs, capture screenshots, and inspect on-device behaviour **only when the
user explicitly asks for it, each time.** The user drives when it's time to
look; wait for that instruction.
## UI / design conventions
- Material 3 Expressive throughout. Consult the `material-3` skill before
designing anything new; prefer M3 tokens/components over hardcoded colours.
- **Selection pickers are full-screen** — browse-style "choose one" surfaces
(visibility, reminder, recurrence rule, colour, calendar, add-field, plus the
Settings pickers) use floret-kit's `FullScreenPicker` / `OptionPicker` (a
full-bleed sheet with a pinned title bar and connected grouped rows); a picker
that needs a commit/extra action passes it via the picker's `actions` slot.
The exception is the **recurring scope choosers** — the "this / this & following
/ all" prompts shown when you *save an edit to*, *drag* or *delete* a recurring
event — which stay compact `OptionCard`-in-`AlertDialog` popups (a quick 2–3 option
decision reads better as a popup than a near-empty full screen). `AlertDialog`
is otherwise only for plain confirmations. Radio/text-list dialogs are banned.
## Releases
The committed `versionCode` / `versionName` in `app/build.gradle.kts` **are the
release trigger**: merging a bumped `versionName` into `main` runs
`.gitea/workflows/release.yaml`, which builds, signs, publishes to the
self-hosted F-Droid repo, then mints the `vX.Y.Z` tag + release. `versionCode` is
pinned to `MAJOR*10000 + MINOR*100 + PATCH` (e.g. 2.13.0 → 21300). Full process
in `docs/RELEASING.md`. **Never tag/release UI changes before on-device review
and explicit go-ahead.**
Release builds are kept **F-Droid reproducible** — `vcsInfo`, `dependenciesInfo`,
and the AGP metadata block are deliberately disabled in `build.gradle.kts`; don't
re-enable them. Use the `releaseTest` build type (R8-shrunk twin, debug-signed,
own applicationId suffix) to smoke-test a release candidate on-device.
### Per-version changelogs are written by hand, in every locale
`fastlane/metadata/android/<locale>/changelogs/<versionCode>.txt` is the
"What's New" both F-Droid and Play publish. **There is no auto-translation
layer** — Weblate owns `values-*/strings.xml` only, not the fastlane tree — so
when cutting a release you write these files yourself, one per locale, each a
short summary under **500 characters** (Play's hard cap, applied per locale;
F-Droid truncates in-client).
Write one for **every language the app ships** (`app/src/main/res/values-*`),
using store locale codes: `en-US`, `en-GB`, `de-DE`, `es-ES`, `fr-FR`, `it-IT`,
`pl-PL`, `pt-PT`, `ru-RU`, `zh-CN`, `ar`. Missing locales aren't fatal — both
stores fall back — but see the `en-GB` trap below.
**`en-GB` is the Play Console's default language.** Play's fallback is the
*default locale*, not `en-US`, so a release with only an `en-US` changelog ships
with **no "What's New" at all** — this is why the latest release had none.
`en-GB` must exist.
The rest of the plumbing is already locale-agnostic: `sync_changelog_to_fastlane.sh`
seeds `en-US` only and never overwrites a committed file, while
`fastlane_to_fdroid_localized.sh` and `supply` pick up every locale that has a
`changelogs/` dir. So the files are the whole job.
## Forge / `tea` CLI
**Codeberg is canonical** (`codeberg.org/jlmakiola/calendula`) for git, issues,
PRs, tags and releases — including the `floret-kit` submodule. The self-hosted
Gitea instance is **build infrastructure only**: signing key, F-Droid publishing,
release pipeline.
Use the **`tea` CLI** for forge interaction — not raw API calls. Note the flag is
a *subcommand* flag, not global: `tea pulls list --login codeberg`, never
`tea --login codeberg pulls list`. Two accounts, neither default:
- **Everything → `codeberg`** (user `jlmakiola`). PRs, issues, releases, repo
settings. `tea pulls create --login codeberg ...`
- **Build infra only → `jeanluc`** (`gitea.jeanlucmakiola.de`, user `makiolaj`).
Release-pipeline runs, Actions secrets. `tea ... --login jeanluc`
Workflows are split by directory and this is load-bearing — Forgejo's lookup is
first-match-wins across `.forgejo/` → `.gitea/` → `.github/`:
- `.forgejo/workflows/` runs on **Codeberg** (`ci.yaml`, `translations.yaml`) and
must reference **no secrets** — that's what makes fork PRs safe.
- `.gitea/workflows/` runs on **Gitea** (`release.yaml`, `renovate.yml`) and is
where every secret lives.
Don't add a workflow without deciding which side it belongs on.
## Translations
Community translations are managed on a self-hosted **Weblate**, which owns all
`values-*` files (including German — API only, never hand-edit). Partial
translations are expected (`MissingTranslation` is informational, not fatal);
extra/stale keys stay fatal.
## Git Operations
Use a commit format which references the issues, dont add any Co-Authered by Claude lines, and don't write extensive commit and merge messages, simple human ones suffice, for prs, stuff like testing etc, isnt interesting write what has chnaged, and if deviated from the underling issues pls explain why, add a closes issue line at the end of all prs.
## Comments
Don't add extensive code comments, methode discription, so as a Java Doc is fine, but no extensive explanbanitory conmments

254
docs/OWN-STORE.md
View File

@@ -30,8 +30,8 @@ BEFORE AFTER
│ │
TasksRepository TasksRepository ← unchanged
│ │
TasksDataSource (interface) TasksDataSource ← unchanged
│ ╱ ╲
TasksDataSource (interface) TasksDataSource ← one method
│ ╱ ╲ changes
AndroidTasksDataSource RoomTasksDataSource AndroidTasksDataSource
│ │ │
ContentResolver Room / SQLite ContentResolver
@@ -42,10 +42,18 @@ BEFORE AFTER
(deleted) tasks.org (external, unchanged)
```
**Unchanged above the data layer.** `TasksRepository`, every ViewModel, every
screen. Verified: exactly one file outside `data/tasks` references
`TasksContract` (`domain/Models.kt`, for five constants), and it stops doing so
in phase 0.
**Unchanged above the data layer.** Every ViewModel, every screen. Verified:
exactly one file outside `data/tasks` references `TasksContract`
(`domain/Models.kt`, for five constants), and it stops doing so in phase 0.
Navigation addresses tasks by `taskId` throughout (`Destinations.kt:59`,
`TaskDetailScreen.kt:202,384`) — never by the instance id — so the change below
does not reach the UI.
**One seam method changes.** `TasksDataSource.updateInstance(instanceId, form)`
becomes `updateInstance(taskId, occurrenceStart, form)`, and `Task` gains
`occurrenceStart: Instant?`. See *Instance identity* — this is the one place the
"nothing above the data layer changes" claim needed qualifying, and
`TasksRepositoryImpl.updateTask` is the only caller.
**Deleted.** The `:provider` Gradle module, its manifest `<provider>`, its two
custom permissions, its 84 Java files, its 13 translated string resources, and
@@ -73,6 +81,12 @@ enum class StorageMode {
and the `TaskProvider(isOwn = true)` case go with it: in `OWN` mode there is no
authority, no ContentResolver and no permission to grant.
> **This rename happens in phase 5, not phase 0.** Between phases 1 and 4 both
> stores exist, so the enum carries `LOCAL` (dmfs), `OWN` (Room) and `EXTERNAL`
> simultaneously. Renaming `LOCAL` → `OWN` up front would make `OWN` mean *dmfs*
> for four phases and *Room* afterwards, which is exactly the kind of thing that
> gets misread six weeks later.
`ProviderResolver` narrows to what it was always really for — **discovering
external providers** — and `ProviderStatus.READY` becomes unconditional in `OWN`
mode.
@@ -85,10 +99,30 @@ mode.
ContentProvider. Users who need interop pick External mode, or wait for a
possible read-only facade (explicitly out of scope — see *Deliberately not
doing*).
- **Local lists still report an account name.** `TaskList.accountName` is a
non-null String that `ListsViewModel.kt:98` groups by and
`ListsScreen.kt:222` renders as a section header. `RoomTasksDataSource` maps
`account_id IS NULL` to `accountName = "Local"`, `accountType =
"local"`, so the existing grouping and `TaskList.isLocal` keep working with no
UI change. (`isLocal` moves off `TasksContract.LOCAL_ACCOUNT_TYPE` in phase 0
and compares against a `domain` constant instead.)
Knock-on, benign: `TaskEditViewModel.kt:101` picks the first *non*-local list
as the edit form's default. In `OWN` mode with no account configured every
list is local, so it falls through to `firstOrNull()`. Same practical result,
worth knowing before someone reports it as a bug.
- **Auto Backup gets simpler and safer.** One Room file we control, with a
documented restore path, instead of a provider database whose `cleanUpLists`
routine could delete restored lists whose accounts no longer exist.
⚠️ With one caveat that has to be handled, not assumed away: **Room enables
write-ahead logging by default**, and Auto Backup copies files without
checkpointing. A `-wal` sidecar can hold writes the backed-up `.db` does not.
We checkpoint (`PRAGMA wal_checkpoint(TRUNCATE)`) on `ON_STOP` and include
`.db`, `-wal` and `-shm` together in the backup rules, so a restore is
consistent either way. Phase 6 tests this, because "our backup is safer" is
the kind of claim that is worth exactly as much as its test.
---
## The schema
@@ -121,8 +155,14 @@ Four tables. Designed from Agendula's actual reads and writes plus RFC 5545's
### `tasks`
Master rows *and* recurrence overrides live here; an override is a row with
`recurrence_id` set and `parent_task_id` pointing at its master.
Master rows *and* recurrence overrides live here. An override is a row with
`recurrence_id` set and `master_id` pointing at its series master.
> `master_id` and `parent_id` are different things and must not be conflated.
> **`parent_id`** is task hierarchy — a subtask's parent, the thing
> `RELATED-TO;RELTYPE=PARENT` carries. **`master_id`** is recurrence — which
> series an override belongs to. A row can have both: a subtask can itself
> recur.
| Group | Columns |
|---|---|
@@ -150,8 +190,26 @@ own. This is what makes an honest round-trip possible, and it replaces the
provider's `data0`–`data15` bag with something that cannot silently lose a field
it has no column for.
Indices: `(list_id, is_deleted)`, `(parent_id)`, `(uid)` unique per list,
`(master_id, recurrence_id)`, `(is_dirty)`.
Indices: `(list_id, is_deleted)`, `(parent_id)`, `(master_id, recurrence_id)`,
`(is_dirty)`, and **unique on `(list_id, uid, recurrence_id)`**.
> The unique index deliberately includes `recurrence_id`. An override **shares
> its master's UID** — that is what makes it an override rather than a separate
> task — so a unique index on `(list_id, uid)` alone would reject the very rows
> the recurrence design depends on. With `recurrence_id` NULL on the master and
> set on each override, the constraint says the right thing: one master and at
> most one override per occurrence, per UID, per list.
**Cascades.** `master_id` is `ON DELETE CASCADE` — deleting a series deletes its
overrides, which would otherwise become unreachable rows that still sync.
`parent_id` is `ON DELETE SET NULL`: deleting a parent promotes its subtasks to
top level rather than destroying work the user did not ask to lose. `task_id` on
`task_alarms` cascades.
**Type converters.** Every time column is `kotlin.time.Instant` in the entity
and INTEGER epoch-millis in SQLite, via one `@TypeConverter` pair. `status` and
`priority` convert through the existing `domain` enums, so `statusFromInt` /
`toInt()` keep their single home.
### `task_alarms`
@@ -206,6 +264,43 @@ materialised table's entire class of staleness bugs never exists.
- Client-side expansion is required for CalDAV regardless: server-side
`CALDAV:expand` on `VTODO` is broken on every server `SYNC.md` targets.
### Instance identity
Deleting the materialised `instances` table deletes the instance **row id**, and
two things use it today:
- `TasksRepositoryImpl.updateTask` → `dataSource.updateInstance(current.id, …)`
- `ListsScreen.kt:422` → `items(results, key = { it.id })`
The Compose key is the constraint that decides the design. Two occurrences of
one series can appear in the same list, so `taskId` alone is not unique, and a
hash of `(taskId, start)` folded into a `Long` can collide — which as a Compose
key is a visible bug, not a theoretical one.
So we address occurrences by what they actually are:
```kotlin
data class Task(
val taskId: Long, // the master row — unchanged, what navigation uses
val occurrenceStart: Instant?, // null for a non-recurring task
…
)
fun updateInstance(taskId: Long, occurrenceStart: Instant, form: TaskForm)
```
`Task.id` is dropped; the Compose key becomes `"$taskId@${occurrenceStart}"`,
which is unique by construction and stable across reloads.
**External mode absorbs this without loss.** `AndroidTasksDataSource` maps
`(taskId, occurrenceStart)` back to a real instance row with one query —
`WHERE task_id = ? AND instance_start = ?` — before writing through the
instances URI. One extra query on an operation the user performs by hand, in
exchange for a seam that does not depend on a foreign table's row ids.
This is the **only** change to `TasksDataSource`, and
`TasksRepositoryImpl.updateTask` is its only caller.
### Completing one occurrence of a recurring task
The provider's `Detaching.java` implemented **model (d): detach the occurrence
@@ -272,8 +367,25 @@ Rules that make this safe:
local lists, with their `uid` preserved. Rare, but preserving the UID is what
lets them be re-attached to an account later.
`tasks.db.imported` is excluded from Auto Backup; the new Room database is
included, which is the whole point of owning it.
`tasks.db.imported` is excluded from Auto Backup; the new Room database (with
its `-wal` and `-shm` sidecars) is included, which is the whole point of owning
it.
### If the import goes wrong in production
The rename is not just tidiness — it is the rollback. `tasks.db.imported` is a
complete, untouched dmfs database, so recovery does not need `:provider` to
still exist:
1. `OneShotImport` can be re-run against `tasks.db.imported` as well as
`tasks.db`; the DataStore flag is clearable by a targeted fix release.
2. Re-import truncates the Room tables first and re-runs in one transaction, so
a second attempt is not a merge and cannot duplicate.
3. Only after a release with no import defects reported does a subsequent
version delete `tasks.db.imported`.
This is the reason phase 5 (deleting `:provider`) ships *after* phase 4 rather
than with it — and the reason the deletion is its own release.
---
@@ -308,15 +420,28 @@ every per-server quirk in the server-reality table.
### Phase 0 — Untangle (0.5 wk)
- `domain/Models.kt` stops importing `TasksContract`; the status and priority
constants move into `domain`. This is the last contract reference above the
data layer.
- `StorageMode`: `LOCAL` → `OWN`; `ProviderResolver` loses `own` / `isOwn`.
- `ProviderChangeReceiver`'s manifest filter drops our own authority.
- Add Room + KSP to the version catalog (KSP is already applied to `:app`).
- `domain/Models.kt` stops importing `TasksContract`; the status, priority and
local-account constants move into `domain`. This is the last contract
reference above the data layer.
- `Task.id` → `Task.occurrenceStart`; `updateInstance(taskId, occurrenceStart,
form)`. `AndroidTasksDataSource` gains the lookup query, so the *existing*
provider path exercises the new signature before Room ever does.
- Add `StorageMode.OWN` as a **third** value alongside `LOCAL` and `EXTERNAL`.
- Add Room + `room.schemaLocation` to the version catalog (KSP is already
applied to `:app` for Hilt).
**Done when:** the app still builds and behaves identically, with the provider
still present and still default.
Deliberately **not** here — both were in an earlier draft and both were wrong:
- *Renaming `LOCAL` → `OWN`.* The provider is still the store until phase 4;
renaming now makes `OWN` mean dmfs for four phases and Room afterwards.
Phase 5.
- *Dropping our authority from `ProviderChangeReceiver`'s manifest filter.* That
receiver is what re-syncs reminders while the app is backgrounded
(`ProviderChangeReceiver.kt:47`). Removing the filter while the provider is
still live would silently stop background reminder updates. Phase 5.
**Done when:** the app builds and behaves identically, provider still present,
still default, and the seam change is proven on the provider path.
### Phase 1 — Schema and DAOs (1 wk)
@@ -341,19 +466,38 @@ The hard phase. Budget accordingly.
a series with an override, a series with an exception, and an unbounded rule
hitting the window ceiling.
**Done when:** `updateInstance` and recurring reads pass parity tests written
against the current provider's observed behaviour, *except* where model (a)
deliberately differs from model (d) — those differences enumerated as tests.
**Done when:** the expansion suite is green and single-occurrence editing forks
correctly.
⚠️ **Parity against the provider is only partly available, and the earlier draft
of this plan overclaimed it.** The provider materialises exactly one upcoming
occurrence, so there is no multi-occurrence behaviour to compare against. The
split:
| Behaviour | Reference |
|---|---|
| Multi-occurrence expansion | RFC 5545 §3.8.5 and `lib-recur` directly — **no provider parity exists** |
| The single next occurrence | provider parity, while it is still in-tree |
| Editing one occurrence (forking) | provider parity — *except* model (a) vs (d), enumerated as explicit difference tests |
| All-day and DST handling | provider parity |
That partial availability is still the reason deletion is phase 5 rather than
phase 0. It is just not the blanket safety net it was described as.
### Phase 3 — Semantics parity (1 wk)
- Completion coherence: `status` ↔ `percent_complete` ↔ `completed_at` ↔ closed,
replacing `AutoCompleting.java` and — importantly — the reopen asymmetry that
`TaskWriteMapper` currently works around in the app.
- Parent/child integrity, orphan handling on delete.
- Parent/child integrity: `parent_id` is `ON DELETE SET NULL`, so deleting a
parent promotes its subtasks rather than destroying them.
- Validation: `DUE` xor `DURATION`, `due >= dtstart`, all-day pinned to UTC
midnight, list must exist.
- Delete semantics: hard delete when `account_id IS NULL`, tombstone when set.
- Delete semantics: hard delete when `account_id IS NULL`, tombstone when set;
`master_id` cascades so a deleted series takes its overrides with it.
- `ICalendarWriter.uidFor`'s synthesis branch becomes dead on the Room path
(`uid` is NOT NULL). It stays for External, where UIDs really can be absent —
the KDoc gets updated to say which path each branch now serves.
**Done when:** `TaskWriteMapper`'s provider-quirk workarounds are demonstrably
unnecessary on the Room path (they stay for External).
@@ -363,8 +507,9 @@ unnecessary on the Room path (they stay for External).
- `OneShotImport` per the rules above, with tests over a fixture `tasks.db`
captured from a real v0.3.x install.
- `OWN` becomes the default for new installs and for upgraders after import.
- Backup rules updated: include the Room database, exclude `tasks.db.imported`
and the Keystore blob.
- Backup rules updated: include the Room database **and its `-wal`/`-shm`
sidecars**, exclude `tasks.db.imported` and the Keystore blob. WAL checkpoint
on `ON_STOP`.
**Done when:** an upgrade from a v0.3.2 APK with seeded data lands every task,
list and reminder in Room, verified by count and by content.
@@ -374,9 +519,22 @@ list and reminder in Room, verified by count and by content.
- Remove the module, its `settings.gradle.kts` include, its `:app` dependency,
the three dmfs deps it pulled in, `provider/PROVENANCE.md`.
- Add `lib-recur` (and `rfc5545-datetime`) directly to `:app`.
- `StorageMode`: `LOCAL` is deleted, `OWN` is what remains beside `EXTERNAL`.
`ProviderResolver` loses `own` / `isOwn`.
- `ProviderChangeReceiver`'s manifest filter drops our own authority — safe
*now*, because nothing of ours broadcasts `ACTION_PROVIDER_CHANGED` any more.
In `OWN` mode the in-app `InvalidationTracker` observer covers foreground
changes, and until sync exists nothing outside the app can change our data.
**When `SYNC.md` phase 3 lands, the sync worker must call
`ReminderScheduler.sync()` itself** — that is the replacement for the
broadcast, and it belongs in the sync work, not here.
- Attribution screen: dmfs code is gone, but `lib-recur` stays and is
Apache-2.0. `PROVENANCE.md` is replaced by a short note in
`STORAGE-DECISION.md` recording that the fork existed and why it ended.
- **Release note, user-facing:** dropping the `<provider>` also drops the
`de.jeanlucmakiola.agendula.tasks` authority and both custom permissions.
Anyone who pointed DAVx5 or another app at that authority loses it silently —
it has to be called out in the release, with External mode as the answer.
**Done when:** `./gradlew build` is green with `:provider` absent, and the APK
declares no ContentProvider and no custom permissions.
@@ -385,11 +543,32 @@ declares no ContentProvider and no custom permissions.
- Room migration test infrastructure (`MigrationTestHelper`) wired up, so v1 →
v2 is cheap when sync adds columns.
- Restore-path test: Auto Backup restore into a fresh install.
- Restore-path test: Auto Backup restore into a fresh install, **including the
WAL case** — write, background, restore, verify the last write survived.
- Performance check at 5,000 tasks with 20 recurring series.
**Total: 6–6.5 weeks** to a shipping app with its own store, before any CalDAV
work begins. `SYNC.md`'s own estimate drops by 2.5–4 weeks in exchange.
### Total
| Phase | | |
|---|---|---:|
| 0 | Untangle | 0.5 |
| 1 | Schema and DAOs | 1 |
| 2 | Recurrence | 1.5–2 |
| 3 | Semantics parity | 1 |
| 4 | Import and cutover | 1 |
| 5 | Delete `:provider` | 0.5 |
| 6 | Harden | 1 |
| | | **6.5–7 wk** |
Against which `SYNC.md`'s own estimate drops by 2.5–4 weeks, so the net cost of
owning the store is roughly **+2.5 to +4.5 weeks** — before counting the bugs
that stop being unfixable.
> This does not contradict `STORAGE-DECISION.md`'s 4.5–6 week figure; it
> supersedes it. That estimate costed only the new store (schema, expansion,
> semantics, import, tests) on the assumption `:provider` would be *kept*
> alongside it. This plan deletes the provider, which adds phase 0's untangling
> and phase 5's removal — work the earlier figure never had to include.
---
@@ -405,8 +584,10 @@ work begins. `SYNC.md`'s own estimate drops by 2.5–4 weeks in exchange.
The 93 existing app tests must stay green throughout. The 56 provider tests
leave with the module in phase 5 — replaced, not abandoned: phases 2 and 3 owe
equivalent coverage of the behaviour those tests protected, and phase 2's
parity suite is written against them.
equivalent coverage of the behaviour those tests protected. Note the limit
recorded in phase 2: parity covers the single next occurrence, forking and
all-day/DST handling. Multi-occurrence expansion has no provider behaviour to
compare against and is tested against RFC 5545 and `lib-recur` directly.
---
@@ -414,10 +595,11 @@ parity suite is written against them.
| Risk | Mitigation |
|---|---|
| **Recurrence is subtler than estimated** | Phase 2 is isolated and pure-JVM; it can overrun without blocking phases 3–4. The provider stays in-tree until phase 5, so we can always compare against it. |
| **Import loses a user's data** | Read-only source, single transaction, count verification, source file renamed not deleted, fixture-based tests. |
| **Regression in a behaviour nobody documented** | Phase 2's parity tests are written *against the provider while it is still present*. That is why deletion is phase 5, not phase 0. |
| **Losing third-party interop** | External mode covers users who need it. A read-only facade stays possible later; nothing in this design forecloses it. |
| **Recurrence is subtler than estimated** | The likeliest overrun, and the least mitigated — provider parity does not cover multi-occurrence expansion, so the reference is the RFC. Phase 2 is isolated and pure-JVM, so it can overrun without blocking phases 3–4. |
| **Import loses a user's data** | Read-only source, single transaction, count verification, source renamed not deleted, re-runnable from `tasks.db.imported`, fixture-based tests. |
| **Regression in a behaviour nobody documented** | Partial: phase 2 and 3 parity tests run against the provider while it is still present, for the behaviours where parity exists at all. That is why deletion is phase 5. |
| **A restore silently loses recent writes** | WAL checkpoint on `ON_STOP`, sidecars included in the backup rules, and a phase 6 test that exercises exactly this. |
| **Losing third-party interop** | External mode covers users who need it. Called out in the phase 5 release note. A read-only facade stays possible later; nothing here forecloses it. |
| **Room + KSP build cost** | KSP is already in the build for Hilt; Room adds one processor. |
---

6
docs/STORAGE-DECISION.md
View File

@@ -143,6 +143,12 @@ Two things make instance expansion less frightening than its line count:
| Tests to parity with the current 93 + 56 | 1 wk |
| **Net additional** | **4.5–6 wk** |
> ⚠️ Superseded by [`OWN-STORE.md`](OWN-STORE.md)'s **6.5–7 wk**. The figure
> above costed the new store only, on the assumption `:provider` would be kept
> beside it. The decision taken was to delete the provider, which adds the
> untangling (phase 0) and the removal (phase 5) that this estimate never had to
> include. The costing logic stands; the total does not.
### What it removes from the sync plan
Roughly sixteen of the phase-1 audit's storage findings are **provider-imposed**