Give ingested measurements their own source value - #892
Open
Antiheld86 wants to merge 6 commits into
Open
Conversation
Owner
|
Read this one carefully, because it is the more consequential of the two. The shape is right: the source is resolved server-side from the credential rather than taken from the body, the migration number does not collide with what landed on main today, and you rebased far enough that the label catalogue that arrived this morning already carries Three things before I can merge it. One I have to decide, not you. One behaviour change that is not in the changelog. Canonical-row picking prefers a ranked source and falls back to keeping everything when nothing ranks. Today a manual row and an ingested row are both Two verifications. Run Worth saying plainly: widening the 422 from "names Apple Health" to "names any source" is the right call, and the dedup namespace note in your description is the kind of consequence I would rather read from the author than discover myself. |
Rows pushed in through a `measurements:write` Bearer token are attributed `MANUAL` today, so afterwards nobody can tell which readings came off the scale and which were typed by hand. Give that write surface its own `MeasurementSource` value, the way 0189 did for TELEGRAM and 0207 did for MCP. Enum extension only: purely additive, no row touched, `IF NOT EXISTS` makes a rerun safe, and the value is not USED in the migration so Postgres' same-transaction restriction does not bite. Declared last in the datamodel so the Prisma member order and the physical order stay identical. No backfill, and none is possible — `measurements` carries no actor or token column, so a bridge-written MANUAL row is byte-identical to a typed one. Existing rows stay MANUAL.
Adds the `EXTERNAL_SOURCE` leaf beside `scopes.ts` and mirrors the value in `measurementSourceEnum` so the read shapes and the sync decoder see it. It stays out of `WRITABLE_MEASUREMENT_SOURCES`: a source a client could assert would prove nothing about where a reading came from, and that constant is published to clients as `ingest.writeAllowlist`. The value-edit gate in `PUT /api/measurements/[id]` needs the other question answered, though — whether the number is the user's own to fix — so it now reads a separate `USER_CORRECTABLE_MEASUREMENT_SOURCES`. The 409 that protects a Withings row exists because the value is the provider's reading; the hardware behind an ingest token is the user's own scale, and locking those rows would contradict what the settings card and the Home Assistant guide promise. That is the only site reading the wider set.
Under a `measurements:write` credential the row's source is now decided by the server on both routes, and the body may not name one at all. The 422 widens from "names APPLE_HEALTH" to "names any source". It was narrow only because the forced value was MANUAL, so a body saying MANUAL was never actually overridden. Now it would be — and overriding a client's explicit assertion quietly is the thing both routes' comments rule out, while honouring it would defeat the change: a bridge that kept sending MANUAL would stay indistinguishable from a typed reading. Status and errorCode are unchanged, and the documented Home Assistant payload sends no source at all. The single-POST predicate keeps reading the raw body: the create schema defaults `source` to MANUAL, so after parsing "said MANUAL" and "said nothing" are the same value. Both of that route's create sites resolve the source, the array arm included — that is the combined BP + Pulse path a bridge uses, and missing it would leave exactly those rows unbadged. Two consequences worth naming. Bridge rows no longer join the MANUAL/APPLE_HEALTH cross-source merge, so they can't be swallowed by a rule written for the iOS adopt-on-pair case. And a bridge posting at the same instant as a hand entry now lands both rows instead of a 409, which is honest and does not double-count: the rollup collapse is per (type, day, source) and MANUAL outranks the unlisted EXTERNAL.
The write-smoke suite is `satisfies Record<MeasurementSource, SourceCase>`, so growing the enum is a compile error until a case exists; the new one goes through the ordinary single-entry POST, which is the production path. It carries the trap this suite sets for you written down: `beforeEach` seeds a session cookie and the caller resolver is cookie-first, so arming only the authorization header resolves as that session and quietly writes MANUAL — the case would then pass without testing anything. Both hardcoded totals move 14 → 15. In the bearer-scope suite the two rows that asserted MANUAL now assert EXTERNAL, and three cases join them: a body naming MANUAL is refused on each route (the widening from "names APPLE_HEALTH" to "names anything", which nothing else covered), and a bridge-written row is still value- editable by its owner through PUT. That last one guards the split between the write allowlist and the correctable set — folding the two constants back together value-locks every bridge row behind a 409, and this is what notices.
The measurements list already badges any row whose source is not MANUAL and already builds its source filter from the shared enum, so both pick the new value up on their own. What it needs is a name: `EXTERNAL` reads as a leaked enum constant, and it is the only new value a user is expected to operate, since it is an option in the filter and a badge on every bridged row. Both label catalogues gain an entry. They are keyed by the enum, so a new source without a label is a type error and the coverage guard fails the suite — which is exactly what happened here, and is the reason this carries a settings label too even though EXTERNAL sits on no priority ladder and so never renders there. The settings card's scope note and the Home Assistant guide both claimed readings arrive "as if you had entered them by hand", which is no longer true. Both now say the rows are marked as coming from an external device and are still yours to correct, and the guide spells out that the payload must not name a source at all. The changelog entries go under Unreleased as Changed: v1.38.1 shipped the token, so these describe a change to something people already have. They say what that means in practice — rows written before this keep the old label and cannot be sorted out afterwards, a batch re-push can duplicate because the dedup key now carries the source, and an automation sending an explicit `source` field has to drop it.
Review caught a behaviour change this branch causes but never wrote down. `pickCanonicalSourceRows` takes the first ladder source present in a day's bucket and filters the day to that source. Before, a typed reading and a bridged one were both MANUAL, so both survived. Now MANUAL ranks, EXTERNAL does not, and the bridged row leaves the canonical set on any day that also has a typed one. That reaches every surface needing one source per day — the daily totals behind the charts, the health score, the doctor report, correlations, mood crosstabs and personal-record detection. A day with only bridged readings is untouched: nothing ranked is present, so the picker's pass-through keeps everything. The changelog says so in the user's terms, including the part that is a fix rather than a change: two indistinguishable MANUAL rows were both counted, which inflated a running total on a day somebody did both. Four cases pin it, because it is emergent from a rank ladder and stated in no single place: the contested day, the bridged-only day, the both-MANUAL shape the change moves away from, and a stored ladder that names EXTERNAL being honoured. Ranking EXTERNAL turns the first two red, which is the review anybody adding it to a ladder should have to pass.
Antiheld86
force-pushed
the
feat/measurement-source-external
branch
from
September 2, 2026 19:57
5ff8fa2 to
1fc7944
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Rows written through the
measurements:writeingest token are attributedMANUAL— the same label a hand-typed reading gets — so afterwards nobody can tell which readings came off the scale. This gives that write surface its ownMeasurementSourcevalue,EXTERNAL, exactly as0189did forTELEGRAMand0207did forMCP.The measurements list already badges any row whose source is not
MANUAL, and its source filter is already built from the shared enum, so both pick the new value up on their own. "Which readings came from the scale?" is answered by a control that existed and merely lacked a value.Follows up the review note on #881. Note that the token shipped in v1.38.1, so unlike when this was drafted, the two consequences below are changes to live behaviour rather than to something nobody had yet.
Rebased onto current
main, so it carries the migration renumbered past0335_insights_cached_localeand the label work reconciled with #889 — see "Labels" below.The source becomes entirely server-decided, and the 422 widens
Both routes already state the principle — refused loudly rather than rewritten quietly. Today
scoped ? "MANUAL"happens to equal the only permitted value, so nothing is ever actually overridden. The moment the forced value becomesEXTERNAL, an explicit"source":"MANUAL"would be silently overridden, which is the thing that comment forbids; honouring it instead would defeat the change, since a bridge that kept sendingMANUALwould stay indistinguishable.So the refusal widens from "names
APPLE_HEALTH" to "names anything". Status anderrorCodeare unchanged. This is the one thing here that could affect a working setup: an automation that explicitly sends asourceofMANUALgot a 201 on v1.38.1 and gets a 422 after this. The worked example indocs/integrations/home-assistant.mdhas never sent asourcefield, so anything copied from the guide is unaffected — but the release notes call the required change out explicitly, since it cannot be detected server-side.Bridge rows stay correctable, through a new constant rather than a widened one
PUT /api/measurements/[id]refuses a value edit when the row's source is outsideWRITABLE_MEASUREMENT_SOURCES, and underEXTERNALthose rows would become value-locked — contradicting the Home Assistant guide, the settings card in all seven locales, and the v1.38.1 notes.But that constant is published as
ingest.writeAllowliston/api/meta/capabilities, so addingEXTERNALto it would tell every client it may name that source and let one forge bridge provenance. The two questions it conflates are genuinely different — may a client assert this source? versus is this number the user's own to fix? — so the edit gate now reads a separateUSER_CORRECTABLE_MEASUREMENT_SOURCES, at exactly one call site.Labels — reconciled with #889
This branch originally carried its own
formatMeasurementSourcebranch and a singlemeasurements.sourceExternalkey, written before #889 landed. That branch is gone: #889's catalogue insrc/lib/i18n/source-labels.tssupersedes it, and the map handles the new value once it has an entry.Both catalogues gain one. They are typed
Recordkeyed by the enum andmeasurement-source-label-coverage.test.tsreads the members straight out ofprisma/schema.prisma, so adding a source without labelling it is a type error and a red suite — which is exactly what happened here on the first rebase attempt. That is also why there is asettings.sections.sources.sourceLabels.EXTERNALkey even thoughEXTERNALsits on no priority ladder and so never renders on that page: the map is exhaustive over the enum, not over what is currently reachable. Both keys are filled in all seven locales, and the guard checks each one resolves in every bundle.Not added to any priority ladder
Following
TELEGRAMandMCP, for two independent reasons.metricSourceLadderis.max(8)and thestepsandspo2defaults are already at 8, so a ninth entry would come back fromGET /api/auth/me/source-priority, be PUT back whole by the settings page, and 422 — breaking ladder saves for every user. And rank 90 is the behaviour we want anyway:MANUALis last on every ladder, so a hand correction wins a contested day while anEXTERNAL-only day still surfaces.Type of change
Test plan
measurement-source-write-smoke.test.ts, whose case tablesatisfiesa Record keyed byMeasurementSource, and a red suite inmeasurement-source-label-coverage.test.tsuntil both catalogues carry the new member.bearer-scope-enforcement.test.ts: the two rows that assertedMANUALnow assertEXTERNAL; a body namingMANUALis refused on each route with nothing written (the widening above, which nothing else covered); and a bridge-written row is still value-editable by its owner throughPUT, which is what notices if the two source constants are ever folded back together.pnpm openapi:check— in syncsourceand confirm the row storesEXTERNAL, badges, and appears under the filter; push one namingMANUALand confirm 422 with nothing written; edit that row's value in the UI and confirm it succeeds while a Withings row still 409s; on a day carrying both a bridge reading and a hand entry, confirm the tile shows theMANUALone. Not yet done.Screenshots (UI changes only)
Not captured. The visible change is the existing source badge appearing on bridged rows and a new
External deviceoption in the existing source filter — no new component or layout.Checklist
mainbranch (trunk-based — see CONTRIBUTING.md)pnpm typecheckpasses locallypnpm lintpasses locally — 0 errorspnpm testpasses locally — 22511 passed | 16 skipped | no failurepnpm format:checkpasses locallypnpm buildpasses locallyt("key")with bothmessages/en.jsonandmessages/de.jsonupdated — two new keys in all seven locales:measurements.sourceExternalfor the list badge and filter, andsettings.sections.sources.sourceLabels.EXTERNALfor fix(i18n): name every measurement source instead of showing its enum #889's settings catalogue.settings.measurementsToken.scopeNoteis also reworded in all seven, since it claimed readings arrive "as if you had entered them by hand", which is no longer true.CHANGELOG.mdif user-visible — under the current## [Unreleased]→ Changed. The released sections are untouched.docs/audit/ordocs.healthlog.devif behavior or self-hosting docs change —docs/integrations/home-assistant.mdrewritten (its "Readings arrive as manually entered" section became wrong) anddocs/api/openapi.yamlregeneratedRollout
No backfill, and none is possible in principle.
Measurementcarries no actor or token column andAuditLognever records the credential, so a bridge-writtenMANUALrow is byte-identical to a hand-typed one — that indistinguishability is the bug being fixed, and any heuristic would relabel real hand entries as machine ones. Any rows written since v1.38.1 stayMANUAL: still editable, correctly not badged. The clean split starts from this change forward, which the release notes say plainly.One thing worth naming: the dedup namespace moves. A bridge that already pushed batch entries as
MANUALand re-pushes the sameexternalIds will insert second rows underEXTERNALrather than deduping. With v1.38.1 only a day old this affects few installs if any, but it is invisible when it happens — the guide and the release notes both tell anyone hit by it to delete the older pair.Release gate — the iOS decoder
schema.prismaandvalidations/measurement.tseach repeat, ten times over, that "the iOS DTO mirrors this exact spelling so the/api/sync/changesdelta feed decodes the rows." That decoder is a closed enum keyed on spelling, there is no documented unknown-value fallback anywhere in this repo, and/api/sync/changesdoes no server-side source filtering — it selectssourceraw and echoes it verbatim.So a row carrying
EXTERNALreaching a native client may fail to decode. The iOS decoder needs to learnEXTERNALbefore any such row ships. That makes this a coordinated server + client release, and the sequencing is a maintainer call — it is the one thing here that cannot be settled inside this repository.Linked issues
Refs #881, #878