The guard read `* 1000` anywhere in the two mapping files as a
re-implemented kilometre factor. Both files also turn seconds into
milliseconds, so an unrelated conversion would have failed it for the
wrong reason and the next person would have deleted the rule.
A rescale now counts only in a length context: a distance identifier on
the same line or within the five above it, which covers the body of a
distance-named helper. The offending line is named in the failure. The
matcher's limit is written next to it — a length conversion in a helper
named nothing like a distance, more than five lines below its signature,
still slips through.
Checked both ways against the real file: a millisecond conversion passes,
a `km * 1000` helper fails.
Review finding L4.
The rollups and the cached status insights are recomputed after the
transaction, so a throw there left the rows repaired and every chart on
the pre-repair numbers — and no later run would notice, because the
audit row makes the account skip from then on.
The tail is wrapped now. The audit row is written with
`rollupsRefreshed: false` and only flipped once the recompute returned,
so a re-run can tell "repaired, rollups pending" from "done" and says so.
A run that could not finish the tail names the account, prints the
backfill to re-run — `scripts/backfill-rollups.ts --user <userId>` — and
exits non-zero.
Review finding M2.
The provenance criterion proves where a row came from, not which build
imported it. An account re-imported on the fixed build carries the same
`EXPORT_XML_SOURCE_MAX` stamp and the same `stats:` external id with the
right numbers and no audit row, so it is selected exactly like a broken
one — and its rest days, under 200 m because the phone stayed at home,
survive the plausibility check after another thousandfold multiply.
So one candidate row that would leave the range now refuses the whole
account: nothing written, no audit row, the offending rows and a printed
reason instead. The idempotency guard only ever held against the
script's own prior run; this holds against the case it could not see.
The runbook says the same in words — a re-imported account must not be
repaired, the re-import already did it — and says what the dry run is
for: it is where the decision is made, not a formality in front of one
already taken.
Integration coverage for the re-imported account: correct rows, an
`--apply` run, nothing changes and no audit row is left behind.
Review finding H1.
The record-unit conversion is now opt-in per caller and only the
`export.xml` archive path opts in. There the attribute is authoritative:
Apple stamps every quantity `<Record>` with the account's own display
unit, so `km` means kilometres and the reading is meaningless without it.
`POST /api/measurements/batch` keeps its documented contract — `unit` is
captured for audit and never read. Nothing in the tree pins the native
client's unit strings for the seventeen convertible identifiers, and a
client that reads a sample in metres while stamping the person's display
unit into the field would have had every walking distance multiplied by a
thousand on ingest. Reading the field there is a wire change and needs
the client's strings pinned first.
The mapping-table audit and the shared conversion module stay. The batch
path's identity behaviour is written down where it is decided and pinned
twice: every convertible identifier round-trips a sibling unit unchanged
by default, and a walking-distance entry sent as `km` is stored as it
arrived. The OpenAPI description says what the server actually does.
Review finding H2.
The defect was invisible from the outside because every behavioural test in
the suite seeded a tagged reading. Two checks close that:
A structural guard that reads the dashboard surfaces and fails when a tile
derives presence from a list of enum buckets with no null / unknown arm and
no written reason, following one hop through a named constant. It carries
its own positive and negative controls, so a refactor that silently stops
matching anything fails here rather than going quiet.
An integration test that seeds twenty readings with a genuinely NULL context
against a real Postgres and reads the tile back through the same resolver
the page renders with: eligible, latest value intact, and a tagged reading
still filed under its own context.
Refs #943
Both report paths dropped them: the raw path iterated the named contexts,
and the dense path skipped any row without a context outright, so a report
built for a meter that records no meal time carried an empty glucose panel
while the readings sat in the database. The FHIR export dropped them for
the third time, at the LOINC lookup — its own comment already said an
unspecified reading codes as 2339-0, but the table had no entry to hit.
The insights context breakdown names the bucket, and the Coach's glucose
block now resolves through the shared helper rather than its own copy.
Refs #943
`buildGlucoseTargets` had the same gap as the dashboard tile: it iterated
the four named contexts, so an account whose readings carry no meal time
got no glucose target at all. Route the loop through the shared grouper.
The untagged card is judged against the RANDOM band and is not editable —
one threshold, one editor row — so the cog renders disabled the way it does
for a derived card. The insights reference panel lists the new target type,
otherwise such an account sees no band on the blood-glucose sub-page.
Refs #943
Tile eligibility filtered the four named contexts against the per-context
summaries and asked whether any of them held readings. An account whose
source records no meal time matched none of them, so the tile was omitted
with the module on, the layout toggle on, and a reading from today. Nothing
warned; the strip was simply short one card.
Move the gate out of the page into `resolveGlucoseTiles`, which walks the
shared bucket list — untagged included — and hand the caller the label key
with the summary. Both summary producers group through the shared resolver,
so `UNSPECIFIED` reaches the wire with its own count, latest value and
trend, and a tagged account still gets its per-context breakdown.
Refs #943
`glucose_context` is nullable, but every surface that fanned out over the
four enum members treated the named list as exhaustive. A meter synced
through Apple Health writes no HealthKit meal-time metadata at all, so an
account can hold nothing but rows the list cannot match.
Add one shared notion of the untagged bucket — the name the Coach's glucose
block already used — with a resolver that files NULL, blank and unrecognised
values under it, and a grouper that walks the named contexts first and the
untagged bucket last. The untagged bucket borrows the RANDOM threshold: an
untagged reading is a spot reading, and a fifth threshold would give the user
a second editor row judging the same readings by a different number.
Labels in all seven locales.
Refs #943
The two metric sub-page cases waited for `.recharts-wrapper`, a class the
charting library writes and every chart on the page carries, so the gate was
satisfied by whichever chart rendered first rather than by the one the route
exists to show. On `/insights/mood` that is exactly what happened: the mock
carried a single mood entry on a fixed date, the chart withholds its line below
three distinct days, and the sparse placeholder painted while some other chart
answered the gate. The mood line was never waited for and may never have been
scanned.
Both charts now carry `data-slot="chart-plot"` on the wrapper their data branch
renders — an empty window still paints the empty state and the scan still waits
— and the mood fixture is anchored on now with ten daily scores, the same way
the measurement rows above it are.
`totalEnergyBurned` rides the same account-display-unit contract as the
distance beside it: a Health app set to kilojoules writes `kJ`, and the
column is kilocalories. Convert it through the shared table.
Refs #944
The unit fix only helps the next import. An account that imported an
archive before it carries a decade of days understated 1000x, and the
authoritative repair — re-importing the archive on the fixed build —
needs an archive the account may no longer have.
`scripts/repair-apple-health-distance.ts` is dry-run by default and
selects rows only by the stamp the archive fold itself writes: the
EXPORT_XML_SOURCE_MAX provenance plus the `stats:` external id minted
for that fold. Never by "the value looks small", so a row a native sync
has since overwritten is left alone. `--apply` multiplies one account
per transaction and writes the audit row in the same transaction, which
is what makes a second run a no-op. The archive's own unit is named by
the operator (`--unit=mi`) because the discarded attribute is the defect
itself, and a row whose repaired value would leave the plausible range
is reported rather than written.
Refs #944
Apple stamps every quantity `<Record>` of an export archive with the
account's own display unit, not the unit the mapping table assumes: a
metric archive reports walking distance in km, an imperial one in mi,
body mass in lb, energy in kJ, temperature in degF. The mapper accepted
`input.unit` and never read it, so a 2.484 km day was folded into the
day's total as 2.484 m and a decade of imported history read as a couple
of metres walked.
`hk-units.ts` is now the one place a HealthKit unit is converted, shared
by the record path and the workout path (which carried its own km/mi
branch — the duplicate is what let the record path stay wrong). It
converts only inside a family and only for units Apple can emit, and
returns the reading untouched for anything it cannot place, so an
archive that already speaks the table's unit is unchanged.
Every mapping-table entry whose unit cannot be converted — a
dimensionless count, a pinned event, a logarithmic dB level, a compound
rate, the percent units whose value rides as a 0..1 fraction — now
carries a written reason, and a structural guard fails on an entry that
offers neither a convertible unit nor a reason.
Refs #944
The Apple export writes DistanceWalkingRunning in the account's own
length unit; the record mapping assumes metres and never reads the
attribute, so a 2.484 km day lands as 2.484 m.
Refs #944
useRecordCapabilities answers per section now. canWriteDomain and
canManageDomain read the two lists the account payload publishes; in
one's own record they are always true, and under a refused, pending or
unproven context always false. canManage becomes the coarse switch,
true when any section is manageable, and is what the banner and the
navigation read.
Every add, edit and delete control names its own section: labs, mind,
profile, illness, cycle, medications, measurements, documents. Creates
that a WRITE route answers (visits, practitioners) ask canWriteDomain.
The generic delete button, the selection bar, the capture picker, the
dashboard quick entry and the bottom nav take the section from their
callers. Controls whose routes resolve the caller rather than the
record (settings shortcuts, chart overlay preferences, custom metrics,
the Today rail, OCR, efficacy targets, credential tabs) ask for the
caller's own record instead, and the customise-shortcut guard now
requires that shape.
The vault has no delegated write route, so its controls stay withheld
under every grant, and the documents page says so in one sentence
where the upload control would be, in all seven locales.
Every accountAccess entry on GET /api/auth/me carries writableDomains
and manageableDomains: the sections the grant opens, intersected with
the sections whose routes accept a delegated write at that level. A
READ grant gets two empty lists, a WRITE grant gets writable sections
only, a MANAGE grant gets both, and the vault appears in neither.
The client parser checks the structural invariants (no repeats, within
the grant's sections, manageable within writable, nothing below the
level) and requires the two published views of the active record to
agree on the lists. Regenerate the OpenAPI document and extend the
payload consumer guard with the active-record readers.
Add the one table of which record sections have a route declaring
requireRecordAuth("write") or ("manage"), derived from the route files,
and a structural guard that re-derives it from src/app/api on every run
and fails on a disagreement in either direction. The matcher tolerates
the line break a formatter puts after the opening parenthesis and
asserts a non-zero match count.
delegatedDomains() turns a grant's level and scope into the sections it
can add to and the sections it can change, keeping write within manage.
"Sign out everywhere" revoked every other refresh token and nothing else.
A native login is two rows, and the Bearer check consults only the
paired access token, so the other phones kept working until that token
expired, up to a day on the default policy. The route also handed
destroyOtherSessions a session-kind credential for a Bearer caller, whose
session.id is the ApiToken row id; the exception matched nothing and the
calling phone revoked its own refresh token. destroyOtherSessions now
finds the paired access tokens through the refresh rows and revokes them
inside the same transaction, reporting the count, and the route names a
Bearer caller by its access-token hash. Programmatic tokens are untouched.
rotateRefreshToken marked the consumed row under usedAt: null alone, so a
revocation landing between the read and the write was overtaken and a
pair minted after the family ended stayed live. The guard now includes
revokedAt: null; on a lost race the minted pair is retired and the reason
distinguishes a revoked family from a double consume.
withIdempotency consulted the grant only at replay time, and only for its
existence; a narrow-scoped token was checked only on the delegated arm.
Both move into the cell key: a delegated cell carries the grant's id,
level and scope fingerprint, a narrow token's cells carry the token, and
a request with no live grant neither reads nor claims a cell. Cookie and
wildcard own-record cells are keyed byte-for-byte as before.
Tests: integration for the paired access tokens (cookie and Bearer
caller), a deterministic revoke-mid-rotation race against Postgres, unit
coverage for the new cell keys, and the structural guard moved to the
new invariant.
Rotation revoked the access token paired with the consumed refresh token
the instant the rotation committed. A native client with requests already
on the wire got 401 revoked on those, read that as "re-authenticate",
rotated again with the refresh token it had just spent, and tripped reuse
detection: the device's whole token family died because the client was
busy at the boundary.
Pull the paired token's expiry in to fifteen seconds instead. It is long
enough for an in-flight request on a slow mobile link to finish, including
a retry, and short enough that the token gains nothing meaningful over the
validity it already had. bearer.ts answers a passed expiry with reason
expired, which a client treats as "refresh", not as "sign in again".
LEAST(expires_at, $1) takes the minimum inside the single UPDATE, so the
write can only ever shorten: a token already inside its last seconds keeps
its own expiry, and there is no read-then-write window. The bound value is
an ISO-8601 UTC string cast in SQL so the comparison does not shift with
the host's timezone.
Reuse detection is untouched. A replayed refresh token, or one presented
under a spoofed device id, still revokes the family and its access tokens
on the spot, and so does the logout path.
Nothing calls it any more: the weekly pass and the off-host job both
walk the payload through the streaming writer. Keeping an exported
function that builds the entire record as one string invites the next
caller to reintroduce the memory profile this branch removes.
The off-host job was said to benefit from the streaming work the weekly
in-database backup got, and it did not. Only the payload builder was
shared: `runOffhostBackup` called `buildFullBackupJson` for the whole
document, `gzipSync` over that whole string, a whole-buffer
`createCipheriv` pass over the result, and handed the finished buffer to
a single `PutObject`. Four full copies of the record alive at once, on
the same arithmetic the weekly pass had already been fixed for.
Configuring an S3 target on a long-lived record therefore restarted the
container. On an account of 445 000 measurements the JSON alone is
242 MB, and against a 1 GB container's 524 MB heap the run died of heap
exhaustion seventeen seconds in — taking every signed-in session with
it, because the job shares the app process.
The write path is a stream end to end now: `streamFullBackupJson`
produces the document a page at a time, gzip and an incremental
AES-256-GCM writer consume it as it arrives, and `@aws-sdk/lib-storage`
puts it up in two 8 MB parts. What the process holds is fixed by the
pipeline's shape rather than by the record. Measured on that same
445 000-measurement account under `--max-old-space-size=450`, with the
heap pre-loaded to what a warm server holds: the old path dies with
`Ineffective mark-compacts near heap limit`, the new one finishes with a
peak 79 MB above its baseline and writes a 9.1 MB object.
The stored envelope gains version 3, which moves the GCM tag from in
front of the ciphertext to the end. That is the only change the stream
required — the tag exists only once the last block is in, so a leading
one means the whole object must exist before its first byte can be sent.
Nothing about the authentication moves: the tag still covers every
ciphertext byte and is verified before a byte of plaintext comes back.
Versions 1 and 2 still read, so every object already in a bucket
restores through the same reader with no flag to tell them apart.
The incremental writer is the crypto module's, split into its byte-level
half so the base64 form the in-database blob uses and the raw form an
object needs are one implementation under two framings. The off-host key
handling is unchanged: a separate `BACKUP_ENCRYPTION_KEY`, 64 hex or
32-byte base64, passed in rather than looked up.
A run that uploaded nothing for anybody is a failed job now, not
`ok: true` with an empty bucket behind it. Wrong credentials, a missing
bucket and an unreachable endpoint all fail every account rather than
one, and the target's own sentence rides out as the cause. A run where
some account got a copy still succeeds; retrying the whole cohort over
one object would re-upload everybody's.
The one ceiling left is structural rather than a memory bound, and it is
counted rather than discovered: a multipart upload carries 10 000 parts,
so an object past 80 GB is refused for that account with a clear message
instead of failing halfway through with an SDK error about part numbers.
`tests/integration/offhost-backup-streaming-memory.test.ts` pins the
budget from both sides — what the streaming uploader holds, and that the
materialising path does not fit the same budget on the same fixture in
the same process, so the number cannot pass by measuring nothing.
A gateway can answer a failed generation with HTTP 200 and put the reason
in the body: a top-level `error` object and no `choices`, or
`choices[0].error` beside `finish_reason: "error"` when a generation dies
mid-flight. Both fell through to the empty-content throw, so a dead credit
balance, a data-policy block, a rate limit and a model that is down all
recorded the same hop: status 0, reason "returned empty content", no
excerpt. Nothing on the provider health card told them apart.
Raise the shape a non-2xx reply raises instead, with the gateway's own
message leading and its `error.code` as the status when that code is an
HTTP error status. The chain classifier, the health ledger and the card
then see 402 / 403 / 429 / 503 and treat each of them the way they treat
the same status on the wire. Message and excerpt are bounded and
key-redacted by the same helper the non-2xx path uses, so remote text is
no more trusted here than it is there.
Gated on the gateway tag: api.openai.com signals every failure with a
status, so the pinned tags parse exactly as before, and a genuine empty
reply stays the empty-content throw it was. The local-endpoint provider
had the same hole on its buffered path, its buffered-reply fallback and
its stream, and gets the same treatment. The body classifier learned the
two account-side refusals a gateway words in its own vocabulary rather
than the vendor's.
The per-user date order reached the entry form and nothing else. A
reporter switched away from MM/DD/YYYY, the date picker followed, and
the dashboard chart axes and the measurements list kept the old order.
`makeFormatters` took `timeFormat` and `dateFormat` as optional
parameters defaulting to "AUTO", so six of the seven constructions in
the tree silently rendered the locale default and nothing anywhere
said the setting had been dropped. Drop both defaults. The compiler
then names every caller: the chart bucket labels, the legacy
`@/lib/format` helpers the measurements list renders through, the
medication compliance and efficacy charts, the doctor-report PDF and
the clinician share view.
Thread the preference:
- `makeBucketLabelFormatters` takes the order; the health and mood
charts read `useDateFormatPreference()`. The UTC pin decides which
day the label names, the preference decides how it is spelled, and
the two are separate questions.
- `format.ts` reads `readStoredDateFormat()` beside the timezone and
hour-cycle mirrors it already read.
- The PDF takes `dateFormat` as a required render option, alongside
`timeFormat`, which was optional for the same reason.
- The clinician share view spells the OWNER's dates the owner's way.
It also picked up the owner's hour cycle, which it had been ignoring
while the PDF route beside it already honoured it. Both resolve
through the new `resolveUserFormatPreferences`, the counterpart to
`resolveUserTimezone`.
Textual months stay on the UI locale. The preference pins numeric
field order by rendering through de-DE / en-US / en-CA, and a textual
month through those would change the month name's language.
Add `date-order-explicit-formatter-guard`: a raw numeric date renderer
must take its locale from the preference or carry a written reason,
and a formatter construction may not hard-code "AUTO" for the order.
Its allowlist holds thirteen calendar-day key builders, each with the
reason it has no order to get wrong. The sibling timezone guard passes
every one of them, because naming a calendar says nothing about
spelling a date.
Refs #922
crypto.randomUUID() is [SecureContext] in the Web Cryptography IDL, so a
page served over plain HTTP never gets it. Measured in Chromium against
http://<lan-ip>: isSecureContext false, randomUUID and subtle undefined,
getRandomValues still a function.
The document vault minted its queue id and Idempotency-Key that way, inside
the file-input change handler. On a LAN self-host the call threw before a
queue row existed, so picking a file produced no upload, no error and no
trace — the picker simply looked inert, and the only way through was to
reach the app over HTTPS. The client transport has minted a default
Idempotency-Key the same way since v1.37.18, which put every apiPost and
apiPatch on the same footing: on such a host nothing wrote at all.
Add randomId(), which prefers the platform call and otherwise assembles a
v4 UUID from getRandomValues — the same CSPRNG, the same 122 bits, no
secure context required — and route every browser-side call site through
it. Nothing in an upload needs one: the bytes travel over multipart XHR and
every cryptographic operation happens on the server. Passkeys, web push and
home-screen install still need HTTPS; browsers withhold those outright.
A structural guard fails a direct crypto.randomUUID() anywhere outside the
two server-side files that may keep it, so the class cannot return one call
site at a time.
Also point the CSV and JSON import cards straight at the import guide. They
resolved through a redirect under the integrations tree, which is the wrong
place for a guide that is not an integration runbook. The suite that
covered those links pinned the old spelling rather than the guarantee; it
now asserts an absolute docs-site URL, no internal path, and new-tab
hygiene.
The tile now calls t() with a key looked up from HRV_LABEL_KEY rather
than a literal, and the call-site coverage guard only reads literal
arguments. So the label left the guard's field of view exactly when it
became per-series: dropping either key from a bundle would ship a raw
key into the dashboard of whichever locale lost it, with nothing in the
gate to say so.
The keys are read from the map, so a third HRV series is covered by
construction rather than by remembering. The count is anchored, because
an empty map would satisfy the loop by matching nothing. A second case
asserts the RMSSD label actually names the measure in every locale,
which is the whole reason the second key exists.
Verified by deleting the key from one bundle: three cases fail and name
the locale and the key.
Two notes from review, neither a behaviour change.
`counts only live tokens carrying this scope` mints successfully and then
inspects the `where` handed to the count — it was sitting in the refusal
block and belongs beside the other what-was-the-helper-given cases. Moved
verbatim.
`still mints on the last free slot` stays where it is, next to the 409 it
is the other half of: the pair is what pins `>=` against `>`, and reading
either alone tells you less than reading both together.
The ceiling is count-then-create and so not atomic — two mints racing at
nine both pass and the account lands eleven. Now said out loud, along with
the two things a reader cannot see from this file: the overshoot is bounded
because the rate limit above is a single atomic upsert, and the house
pattern for making it exact is an advisory lock, named with call sites. Left
unserialised on purpose — this catches a runaway loop rather than metering a
quota, and a lock on a credential mint buys nothing for what it costs.
The mint's rate limit bounds a burst and does nothing about accumulation:
a stuck automation retrying politely inside its 10-per-minute allowance
still leaves several hundred live credentials behind in an afternoon,
each one to be revoked by hand.
Count live rows before the create and refuse at ten.
Live, not minted-ever, so revoking frees a slot and an expired token
stops occupying one — the ceiling bounds what currently exists rather
than what the account has ever done. Scoped to measurements:write for a
reason worth stating: signing in mints a wildcard access token per
session and those accumulate on their own, so counting every row would
let ordinary browser use exhaust the budget and then refuse a mint for a
reason invisible to the person hitting it.
409 rather than 429. This is a conflict with the account's current
state, not a rate: it does not clear by waiting, it clears by revoking,
and a 429 would tell the caller the opposite of what is true.
Placed after validation, so a malformed request hears about the
malformation rather than the ceiling, and before the create, so hitting
the limit costs no credential.
Ten is set to catch a loop rather than to ration — a household running
Home Assistant, a scale bridge and a couple of scripts sits at three or
four, and it is not a security boundary since the holder can revoke and
mint freely.
Tests pin the boundary, the query shape and the ordering, and were
mutation-checked: turning >= into > fails the ceiling case, and dropping
the scope filter fails the query-shape case. The integration leg seeds
a wildcard and an expired row as decoys, then revokes one live token and
asserts exactly one slot opens — the leg that fails if the count is of
mints rather than of tokens alive.
Only CHANGELOG.md conflicted: v1.38.7 released while this branch was open,
so the new release heading landed where the branch's [Unreleased] section
sits. The HRV entries stay under [Unreleased] above 1.38.7; everything from
1.38.7 down is upstream's file verbatim.
docs/api/openapi.yaml auto-merged. Upstream's only change there was the
`info.version` bump to 1.38.7, which the merge took while keeping this
branch's DashboardLayoutReadResult, and package.json now reads 1.38.7 to
match — so the merged file should equal a fresh generate. It is a generated
file, though, so confirm with `pnpm openapi:generate` rather than trusting
the three-way merge.
The widgets GET grew a response field and the spec was never told. Nothing
would have caught it: `openapi:check` compares the generated yaml against
the Zod registry, and an undeclared field changes neither — so CI stays
green while the published contract quietly stops describing what the route
returns. iOS decodes this payload.
`dashboardLayoutResult` is shared by the GET and the PUT echo, but the
field is resolved per request from the caller's modules and their data, so
it exists only on the read. A separate `dashboardLayoutReadResult` extends
the shared schema for the GET rather than widening both verbs and
describing a field the PUT never sends.
docs/api/openapi.yaml is NOT regenerated in this commit: `pnpm
openapi:generate` cannot run here (the Prisma engine download is refused,
so `@/generated/prisma/client` is missing and the registry will not load).
The tree is therefore in drift on purpose and `pnpm openapi:check` will
fail until `pnpm openapi:generate` is run and the yaml committed.
The writer added in v1.38.6 compared the process's live heap usage against
80 % of V8's heap limit and aborted above it. That reading is not the
backup's footprint; it is the whole process's, garbage included. A
long-running app sits at 400 MB of largely collectable heap, so on a 1 GB
container (a 419 MB budget) the weekly pass aborted the first flush of every
account: four out of four in seven seconds, the smallest of them a demo
record whose entire stored copy is 1.2 MB, each told its record was too large
and its operator to buy memory.
Drop the gauge. What the writer holds is bounded by its shape and needs no
reading to prove: the three unbounded tables are pulled a page at a time,
every other section is released as soon as its JSON is in the sink, and at
most one flush of text is pending. The one copy that does grow with the
record is the stored blob, so the bound moves there — counted in the
ciphertext bytes actually produced, against a fifth of this process's heap
limit, which is what two copies of the answer plus a live set have to fit
inside. The message names both numbers.
A pass that wrote nothing for anybody now fails its job instead of reporting
`ok: true` with `backed: 0`, so the failing-queue panel and the backups page
read the same thing an operator would. A pass where some accounts were
written keeps passing with the failures as counts.
Measured under --max-old-space-size=450 from a heap deliberately dirtied
first: 445 000 measurements peak at 28 MB held by the backup, a one-row
record at under 1 MB.
Both routes 500'd in their unit tests because the change gave them
dependencies the mocks did not know about: the widgets GET reads a module
flag and probes for an SDNN row, and the analytics slim slice resolves the
module map. Neither test mocks `@/lib/modules/gate`, and the widgets test's
prisma double has no `measurement` model — so both calls threw and the
handler answered 500. Real breakage from this branch, not stale
expectations.
The widgets GET no longer resolves the whole module map for one answer.
`hrv` is the only conditionally-dead widget and `recovery` the only module
that decides it, so it reads that one flag through `isModuleEnabled`, the
way this route's neighbours read a single flag.
Both gate mocks spread the real module so only the one call is stubbed;
everything else that imports it keeps live behaviour. The widgets
`beforeEach` states the modules-on default explicitly, because
`resetAllMocks` strips a factory's implementation and every test below
assumes it.
Added coverage for the behaviour itself rather than only unbreaking the
suite: recovery off with no SDNN marks `hrv` unavailable, an account WITH
SDNN keeps the toggle, and recovery on does not run the probe at all — the
one added read on a hot cached route must not touch the common account.
Five points from the review, in the order they were raised.
1. The RMSSD marker did not survive. The tile label paints truncated,
uppercase and on one line, so an appended `· RMSSD` was the first thing
dropped in the longer locales — the part a reader needs to tell a 40 ms
RMSSD night from a collapsed SDNN one. `measurements.typeHrvRmssd` already
exists in all seven bundles, so each series now carries its own key.
`pickHrvSummary` returns that key beside the series it chose, which keeps
one mapping from series to label rather than a conditional at the call site.
2. The module gate bit on one path only. `gateSummariesByModules` was
private to the snapshot builder, and with the snapshot flag off the tiles
are fed by `/api/analytics`, which filtered nothing — so recovery-off plus
RMSSD-only still painted the tile, on the exact path the helper's docblock
promised it could not. The filter moves next to the map it reads, and both
analytics slices apply it. Gated after the cache read, so the cached cell
stays module-independent and toggling a module takes effect on the next
request instead of waiting out the TTL.
3. The same class one step on: recovery off plus RMSSD only left a switch
that was on, a tile that could never paint, and no explanation. The repo
already calls that a dead switch and hides the row; HRV is only dead
sometimes, so the answer needs one fact the Settings screen does not have.
The server resolves it — `unavailableWidgetIds` on the widgets payload —
and Settings drops those rows. An account with SDNN keeps the toggle: its
tile still works with recovery off, because SDNN is a vital no module owns.
The SDNN probe is an index-only existence check that runs only when
recovery is off, outside the layout cache so the row goes the moment the
module does.
4. The brittle test sliced from the helper to the end of the file, so any
function added below it would have gone red pointing at HRV. Bounded to the
function's own body, with a guard on the bound itself. The three wiring
assertions matched formatted source text — they would have passed just as
well if the tile never rendered — and are replaced by a test that renders
the tile through a real TrendCard and asserts the reading is on screen.
One negative source check is kept, naming the expression the bug lived on
rather than the formatting of its replacement.
5. Rebased onto v1.38.6; the changelog entry sits under [Unreleased].
Not verified here: `pnpm db:generate` cannot run in this container (the
Prisma engine download is refused), so snapshot.test.ts, the widgets route
test and the analytics route test could not load, and typecheck is
incomplete. Lint, format and the HRV suite (17 tests) pass.
The "neither series" test walks three inputs in one array literal. Inferred
from the literals alone, the bare `{}` widens to `{ HRV_RMSSD?: undefined }`,
and TypeScript rejects that against `pickHrvSummary`'s
`Record<string, DataSummary> | undefined` — an optional-undefined property is
not assignable to the index signature. Vitest never typechecks, so the suite
ran green while `tsc --noEmit` failed.
Annotating the array pins the parameter type at the declaration and leaves all
three cases exactly as they were.
HRV lives under two measurement types and they are not the same measure:
Apple Health / Fitbit / Google Health write SDNN as
`HEART_RATE_VARIABILITY`, while Oura / Polar / WHOOP — and a bridge
pushing a watch that reports it that way — write nightly RMSSD as
`HRV_RMSSD`. Every HRV surface unions the two: `/insights/hrv` through
`fallbackMeasurementType`, the Coach through
`COACH_SOURCE_MEASUREMENT_TYPES.hrv`, the MCP reads through
`withHrvFallback`. `sub-page-metric.ts` even says listing both is what
keeps "the tab, pill, and dashboard tile lit whichever source the user
has".
The dashboard tile did not. It read SDNN alone, so an RMSSD-only account
got a Settings → Dashboard toggle that turned on and rendered nothing,
with no explanation, while the insights page charted the same readings —
which is why it reads as a broken layout rather than a missing series.
`pickHrvSummary` picks the series with the same rule `withHrvFallback`
uses: the primary wins whenever it has any rows, the fallback only when it
has none. Never a merge — the two share `ms` but measure different things,
so blending them would invent a number no device reported. The caller gets
the winning type back, because the freshness caption has to read the
series on show and the label has to name the measure when it is not SDNN
(`· RMSSD`, as the insights chart title already does).
Second, less visible half: `HRV_RMSSD` is recovery-owned in
`METRIC_STATUS_MODULE_OWNERS`, but the dashboard snapshot gates off
`SUMMARY_TYPE_MODULE`, a different map, and that one did not list it. The
omission was inert while the tile read SDNN alone; adding the fallback
without it would hand a recovery-off account a tile built from
recovery-owned rows — the exact hazard the owner map documents. The entry
goes in beside the four sibling recovery types, so the gate stays server
side and the client keeps no competing copy.
`DataBackup.data` holds every weekly disaster-recovery snapshot and every
uploaded pack, AES-256-GCM at rest like everything else, under a column
called `data`. The rotation registry was keyed on the `*Encrypted` naming
convention, so the script never read a backup, reported zero rows
remaining, and the runbook read that zero as permission to retire the
previous key. Following the documented procedure destroyed every stored
backup — the copy an operator reaches for precisely when nothing else
works.
Register it, and register the idempotent-replay response cache the same
sweep turned up. The backup column walks in bounded batches (one row is a
whole compressed account) and re-seals the ciphertext without reading the
plaintext, so both stored envelopes come back byte-identical and so will
any later one. The cache is marked disposable: a row it cannot read is
dropped rather than counted as an error, because keeping it only
guarantees the next replay gets a body it cannot parse.
Running the script against a seeded database rather than reading it turned
up two more. It died on the mood day-context note, whose table is keyed on
the entry rather than on an `id`, and abandoned every column after it —
the backups included. And the document index's verbatim text rotated
without ever being named in the summary.
Make the naming assumption stop carrying weight. A new guard traces the
encryption helpers through their wrappers, walks every Prisma write
payload, and fails when a column receives ciphertext without being
registered. It asserts an anchored, non-zero match count so a matcher that
stops matching fails instead of passing. A second check pins each model's
primary key against the schema.
The script now ends by naming the registered columns it did NOT walk and
exiting non-zero when any is missing: a zero that means "nothing left" and
a zero that means "never looked" must not print the same. The runbook's
retire-the-key step was rewritten around that signal, and tells an
operator who already rotated on an older release how to get their backups
back.
Three assertions repeated 18.016 and the product it gave. They were the
reason the two factors could sit in the tree at once looking correct. They
take the exported constant now, and the parser test asserts the round trip
a reader on mmol/L experiences rather than a magic product.
The list renders a reading through the metric/imperial transform registry,
and glucose is not in it, so the list kept saying 95 mg/dL while the
dashboard tile said 5.3 mmol/L for the same reading — correct on both
counts and disagreeing anyway.
The edit sheet is the half that could have done damage. Its label follows
the same resolution as the row, so on mmol/L the number in the input is in
mmol/L and has to come back the other way before the PATCH. Row, edit seed
and inversion move together; converting the display alone would rewrite a
95 mg/dL reading as 5.3 the first time somebody corrected a typo.
An mg/dL account edits in the unit the column already holds and stays on
the identity path, deliberately: the converter rounds, and sending an
mg/dL edit through it would quantise a stored 95.48 the moment somebody
opened the sheet to fix a note. A row whose stored unit is not the type's
canonical one still shows verbatim, as before.
`User.glucoseUnit` has existed since v1.2 and around thirty surfaces read
it — the dashboard tiles, the glucose page, the targets panel, the CSV and
FHIR exports, the doctor report, the Coach snapshot, the low-reading push
copy. Nothing could ever write it. No route, no schema, no field on any
form, so every account resolved to the mg/dL default and the mmol/L half
of the app was unreachable code. Anyone who reads glucose in mmol/L
converted in their head on every screen.
This is the missing end: a `GET`/`PATCH /api/auth/me/glucose-unit` scalar
on the pattern the unit-preference endpoint next door established, and a
dropdown in the profile form beside it. Deliberately its own control
rather than a branch of metric/imperial: metric countries are split on
which unit they read glucose in, so one dropdown would get half of them
wrong.
Storage is untouched and stays untouched. Every reading is written in
mg/dL by every ingest path and the unit only selects a display branch, so
nothing already recorded is reinterpreted. The entry form moved with the
preference, which is the half that could have hurt: it asks in the chosen
unit and inverts through `toCanonicalMgdl` on submit, so a 5.3 typed by an
mmol/L reader is filed as 95 mg/dL rather than as 5.3 — a value the
plausibility band accepts in silence and every surface reads back as a
severe hypo.
The contract gains the endpoint and nothing else; the account payload
already carried the field.
A value imported in mmol/L was multiplied by 18.016; one displayed in
mmol/L was divided by 18.0182. The two sat in different files, both
correct-looking, and a reading that went in as 5.3 did not come back out
as 5.3. Latent while nobody could select mmol/L, live the moment they can.
One exported constant now, on the guideline figure the display path
already used, and the glucose insight page takes it from there rather than
keeping a third copy. The test names the constant instead of repeating the
digits, which is how the two came apart in the first place, and it asserts
the round trip a reader on mmol/L actually experiences.
Marc-André Bombeck