Skip to content

Three store APIs have no production caller, so three branches below them are unreachable (raw-outbox Policy 1, locked day-owner, ouraRaw day-keyed reads) #981

Open

Description

@vishk23

Three store APIs exist, are unit-tested, and have no production caller. Each one gates a branch further down that therefore cannot execute in a shipping build. Found while auditing what NOOP computes or stores that never reaches anything, checked against a real device DB.

None of these are urgent. They're the kind of thing that reads as working until you check.

1. markRawBatchSynced is never called, so the raw-outbox prune only ever evicts

Packages/WhoopStore/Sources/WhoopStore/RawOutbox.swift:168. Call sites: PruneTests.swift:21,22,53,66 and RawOutboxTests.swift:38. Nothing in Strand/ or android/app/src/main/.

So syncedAt is NULL for every row that ever exists, and pruneRaw Policy 1 — delete synced batches older than the window, RawOutbox.swift:202 — can never fire. Only Policy 2's size cap (:210-229) bounds the table. The outbox is never drained, only evicted, which is the failure mode the comment at :189 is describing when it cites the 19 GB growth.

Note this is currently masked: rawBatch is empty on my device and, I'd guess, on everyone's, because raw capture is off by default (Strand/Collect/Collector.swift:191, Strand/Collect/Backfiller.swift:623, both reading enableRawCapture which defaults false). So the dead branch only bites whoever turns raw capture on — precisely the person who least wants unbounded growth.

2. setDayOwner is never called, so the locked day-owner override is unreachable

Packages/WhoopStore/Sources/WhoopStore/DeviceRegistryStore.swift:204. Call sites: DeviceRegistryStoreTests.swift:150,155 and DayOwnerReadIntegrationTests.swift:81. No production caller.

The reader is production — Strand/Data/IntelligenceEngine.swift:1573 — so the resolver runs on every pass and always falls through with lockedOwner: nil (:1607). dayOwnership has 0 rows on my device across its whole history, in the on-device DB and the cloud mirror alike.

So the "user pinned this day to this device" path is a complete, tested feature with no way to reach it from the UI. Either it lost its call site or it never got one.

3. ouraRaw: 944 archived payloads, a day column that is NULL on every row, and no production reader

Packages/WhoopStore/Sources/WhoopStore/OuraRawStore.swift. Three separate things, worth taking together:

  • day is NULL on all 944 rows on my device. Both producers hardcode it: Strand/Oura/OuraSyncCoordinator.swift:53 and :146 both pass day: nil. The struct comment at OuraRawStore.swift:16 says "YYYY-MM-DD when the document is day-keyed", and I think that's the actual explanation rather than an oversight — what gets archived is a page, which can span many documents and many days, so there's no single day to record. The schema was designed for documents and the producers write pages.
  • Consequently the index idx_ouraRaw_device_endpoint_day (Database.swift:501) has a permanently-NULL third column, and ouraRaw(deviceId:endpoint:)'s ORDER BY day ASC (OuraRawStore.swift:52) is a no-op — it returns rows in arbitrary order while looking deterministic.
  • Both readers have no production caller: ouraRaw(deviceId:endpoint:) (:47) and ouraRawCount(deviceId:endpoint:) (:60) are referenced only by OuraRawStoreTests.swift.

The archive itself is deliberate and I'd keep it — the header calls it "the 'as much as we can get' backstop: the verbatim payload is kept so a future metric can be derived without re-hitting the Oura API", which is a good reason. The problem is that when that future derivation happens, whoever writes it will reach for WHERE day = ? or trust the ORDER BY, and both silently do nothing useful.

Cheapest honest fix: drop day and the 3-column index, keep (deviceId, endpoint, documentId) + fetchedAt, and order by fetchedAt. Alternatively archive per-document so day becomes fillable. Either is fine; the current state is the one that misleads.

Suggested resolution

  1. Call markRawBatchSynced from the upload path, or delete it and Policy 1 and say the outbox is size-capped only.
  2. Wire setDayOwner to a UI affordance, or delete the locked-owner branch on both platforms.
  3. Pick a day semantic for ouraRaw and make schema, producers and reader agree.

Happy to send a PR for any of these — (3) especially, it's small. Raising first because each has a "delete it" option that's arguably better than wiring it, and that's your call.

Environment: WHOOP 5/MG + Oura API import, iOS. Row counts from my own device DB (782 MB) and cloud mirror, read-only. File:line against a fork of main at af44563c.

Activity

  1. ryanbr commented on Jul 31, 2026

    @ryanbr
    Owner

    Checked all three against main. All three hold. One of the resolutions you offer is not actually available, which settles that item; the other two are genuinely open.

    1 — verified, and it is not an either/or

    markRawBatchSynced has five call sites, all in PruneTests / RawOutboxTests. Nothing in Strand/, StrandiOS/ or android/. So syncedAt is NULL for every row and Policy 1's DELETE FROM rawBatch WHERE syncedAt IS NOT NULL AND syncedAt < ? cannot match. Confirmed dead branch.

    But "call it from the upload path" is not an option here — there is no upload path and there cannot be one. CLAUDE.md:13: "There is no server, no account, no cloud sync, no telemetry", and that is a hard scope limit rather than a gap. The only production consumers of rawBatch are Collector.storageStats (a count) and TestCentreReport (a byte figure); nothing drains it, because there is nowhere to drain it to.

    WhoopStore is adapted from johnmiddleton12/my-whoop (see ATTRIBUTION.md), so this reads as inherited from a design that did sync.

    Correcting myself here: I first wrote that delete was the only resolution consistent with the project's constraints. Re-reading the file, that was too strong on both counts. The 19 GB note belongs to Policy 2, which is credited with the growth bound in its own paragraph — so the docs never actually claim Policy 1 bounds anything, and there is no false statement to correct. And an on-device export lane would be a perfectly server-free way to drain the outbox, at which point marking exported batches synced and letting Policy 1 reap them is exactly the shape this code already has. Dormant-but-correct code is not a constraint violation.

    So the real defect is narrower: a reader sees two policies and reasonably assumes both run. #994 states the dormancy at both sites, with the condition under which it would wake up, and keeps the code.

    Worth adding: this is Swift-only. syncedAt and markRawBatchSynced do not exist in android/, so it is a one-platform change.

    2 — verified, and it is on BOTH platforms

    Swift: declaration plus three test call sites. Android: DeviceRegistry.kt:175 and DeviceRegistryDao.kt:143 exist, and nothing calls either. So the feature is symmetrically unreachable.

    DayOwnerResolver.resolve short-circuits on if let locked = lockedOwner { return locked }, so with the table permanently empty that line has never executed in a shipping build.

    This one is a genuine product question rather than a cleanup: is "pin this day to this device" a feature worth having? If yes it needs a UI affordance on both platforms; if no, the branch and both APIs come out on both. The resolver lives in StrandAnalytics, so whichever way it goes the parity contract applies.

    3 — verified in every part

    Both producers pass day: nil (OuraSyncCoordinator.swift:54 and :146), ORDER BY day ASC is at OuraRawStore.swift:52, the index is at Database.swift:500, and both readers are referenced only by OuraRawStoreTests. Your page-vs-document explanation is almost certainly right — the schema was designed for documents and the producers archive pages.

    One refinement on the fix, because your version bundles two things with different risk:

    • Stop misleading — change ORDER BY day ASC to ORDER BY fetchedAt ASC and correct the struct comment. No migration, a couple of lines, and it removes the trap you are describing (someone trusting a deterministic order that is arbitrary).
    • Clean the schema — dropping day and the 3-column index is a migration, and CLAUDE.md requires a versioned one with a test and forbids mutating an existing migration. Worth doing, but separately, and it is the part that can go wrong.

    Splitting those means the misleading behaviour is gone immediately and the schema change can be reviewed on its own merits.

    Suggested order

    1. (3) first — send it as you offered, ideally split as above. Lowest risk, and it is the one that will otherwise bite whoever writes the future derivation.
    2. (1) — delete, per the constraint above. Small and Swift-only.
    3. (2) — needs the product call before any code moves.

    Thanks for checking these against a real device DB rather than the schema; the "0 rows in dayOwnership across its whole history" is the kind of evidence that turns a suspicion into a finding.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions