Dedupe DataStore reads by porting SD Maid SE's DataStoreValue DSL

[d4rken]

What changed

The charging card on the dashboard no longer flickers. While plugged in with charge recording on, it used to visibly collapse and re-expand for a fraction of a second roughly every 20 seconds — losing its chart and elapsed time each time.

Under the hood this was one missing guard, so the fix also closes the whole class of bug: every stored setting now deduplicates itself, and a few settings that are only meaningful together (a charge session and who owns it, the restore Amply still owes you, the alarm config, theme and shortcut state) are stored as a single record instead of scattered keys, so they can never be read half-written.

Existing app data is dropped. Settings reset to defaults on first launch after installing this. Test users only, so no migration was written — but wipe app data before installing rather than updating in place, and don't update while a one-time full charge is running: that session's record won't carry over, and the protective limit wouldn't be restored automatically.

Technical Context

Root cause. Amply keeps one shared Preferences DataStore. Preferences DataStore emits the entire snapshot to every collector on any write, and none of the 14 preference flows had distinctUntilChanged(). ChargeStatsRecorder stamps lastCaptureWallMillis on every recorded sample (StatsCadence.CHARGING_MIN_INTERVAL_MILLIS = 20_000, plus every 1% change) — always a new timestamp, so a genuinely new snapshot. That re-emitted captureEnabled unchanged, restarting the flatMapLatest in statsDashboardStates and replaying its onStart loading marker.

Why the DSL rather than 14 distinctUntilChanged() calls. The guard is a property of the shared-store design, not of any one facade, so it belongs in the primitive. Ported from SD Maid SE's DataStoreValue. It dedupes on the raw stored value, before the reader runs — so the comparison never depends on a domain type's equals, and no decoding happens for a duplicate emission.

What the record consolidation does and does not buy. It buys shorter facades and clean update {} read-modify-write. It does not buy atomicity or fewer emissions — the old code already wrote each record's keys in a single edit {}, and DataStore emits the whole snapshot regardless of how many keys changed. Worth stating explicitly since it's the intuitive-but-wrong justification.

Corruption semantics are per record, deliberately. Records whose decode was already all-or-nothing (session, recovery, interruption) keep a whole-record fallback. ChargingPreferences does not: its four facts degrade independently today, so a wholesale fallback would swap a valid Adaptive/90% protective baseline for the 80% default — and the next write would persist that downgrade. It parses field by field from a JsonObject, so a wrong-typed lastRequestedAt can't take a valid protective down with it.

For the same reason every field except the charge policy itself is optional. Provenance is diagnostic metadata; making pid required meant a record missing it decoded to null — silently discarding a restore Amply still owed the user. There's a test for exactly that.

Also fixed: a pre-existing torn read in InterruptionAssessor.captureRecoveryPickup(), which read recovery provenance, work id and session through four separate suspend calls and could pair fields from either side of a concurrent adopt or clear. Now one read each.

Review guidance. The risk is concentrated in FullChargeStore and ChargingPreferences — that's the persistence behind restoring the user's charge limit. common/datastore/DataStoreValue.kt is the primitive everything else rests on. The rest is mechanical.

Verification

• 706 tests pass on both flavors; lintVital clean on all four beta/release variants; R8 release builds clean with no serialization stripping (new kotlinx-serialization-json dependency).
• Every new guard was checked against a negative control — reverting the fix must make its test fail. Removing the dedupe: 6 emissions instead of 2, and stats providers recreated 4× instead of 1×. Reverting per-field parsing, the metadata defaults and the alarm normalization dedupe: 7 targeted failures.
• One test in DataStoreValueTest is explicitly documented as not a guard — re-writing an equal value is suppressed by DataStore itself, so a regression test built that way would pass against the very bug it targets.

Still outstanding

Device pass not yet done. Unit tests can't prove the HAL side. Before merge this needs: a full-charge session end-to-end, a force-stop mid-session (restore still happens + interruption card appears), and a reboot recovery (restore happens, no card). Plus confirming the card no longer blips while plugged in for >60s.