mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-09 13:01:00 +00:00
fix(import): round Health Connect values through importValue
The Health Connect import stored its unit factors' binary noise as is (grams to kilograms, millis to minutes, the day sums) and rounded the pulse means and nutrients privately. Every measurement value now takes importValue at the write boundary, and importValue takes a precision for the two places that need another one. The rounding guard now covers the import directory and the scaled Math.round form.
This commit is contained in:
1 parent
29c5b5b83b
commit
0fe9d5d224
5 files changed
+97
-22
No files matched your search
@@ -1,11 +1,14 @@
|
||||
/**
|
||||
* Structural guard: a provider client rounds an imported value through
|
||||
* `importValue` (`src/lib/integrations/import-value.ts`) and nothing else.
|
||||
* Structural guard: a provider client or a file importer rounds an imported
|
||||
* value through `importValue` (`src/lib/integrations/import-value.ts`) and
|
||||
* nothing else.
|
||||
*
|
||||
* Each client used to keep its own `parseFloat(n.toFixed(2))`. Five copies
|
||||
* of one rule drift apart, and `toFixed` mis-rounds halves on the double
|
||||
* below the decimal. A `toFixed` or a private `round2` in a provider
|
||||
* directory is the class coming back.
|
||||
* below the decimal. The Health Connect importer had the other spelling,
|
||||
* `Math.round(x * 1000) / 1000`, in three places and stored its unit
|
||||
* factors' noise unrounded everywhere else. A `toFixed`, a private `round2`
|
||||
* or a scaled `Math.round` in these directories is the class coming back.
|
||||
*/
|
||||
import { readFileSync } from "node:fs";
|
||||
import { join } from "node:path";
|
||||
@@ -25,8 +28,18 @@ const PROVIDER_DIRS = [
|
||||
"strava",
|
||||
"whoop",
|
||||
"withings",
|
||||
"import",
|
||||
];
|
||||
const ROUNDING = /\.toFixed\s*\(|\b(?:function|const|let)\s+round2\b/;
|
||||
const ROUNDING =
|
||||
/\.toFixed\s*\(|\b(?:function|const|let)\s+round2\b|Math\.round\s*\([^;]*\*\s*(10+)\s*\)\s*\/\s*\1\b/;
|
||||
|
||||
/** Files that round for display, not on the way in, and say so. */
|
||||
const ALLOWED: Record<string, string> = {
|
||||
"dawarich/movement.ts":
|
||||
"Formats a stored distance to one decimal in the account's unit for the day view; nothing is stored.",
|
||||
"import/unzip-export-xml.ts":
|
||||
"Prints a compression ratio into an error message.",
|
||||
};
|
||||
|
||||
function strip(text: string): string {
|
||||
return text
|
||||
@@ -47,8 +60,10 @@ describe("provider clients round through importValue", () => {
|
||||
});
|
||||
|
||||
it("finds no rounding of their own", () => {
|
||||
const offenders = files.filter((f) =>
|
||||
ROUNDING.test(strip(readFileSync(join(LIB, f), "utf8"))),
|
||||
const offenders = files.filter(
|
||||
(f) =>
|
||||
!(f in ALLOWED) &&
|
||||
ROUNDING.test(strip(readFileSync(join(LIB, f), "utf8"))),
|
||||
);
|
||||
expect(offenders).toEqual([]);
|
||||
});
|
||||
@@ -57,6 +72,12 @@ describe("provider clients round through importValue", () => {
|
||||
expect(ROUNDING.test("const v = parseFloat(n.toFixed(2));")).toBe(true);
|
||||
expect(ROUNDING.test("function round2(n: number) {}")).toBe(true);
|
||||
expect(ROUNDING.test("const round2 = (n: number) => n;")).toBe(true);
|
||||
expect(ROUNDING.test("const a = Math.round(e.amount * 1000) / 1000;")).toBe(
|
||||
true,
|
||||
);
|
||||
expect(ROUNDING.test("value: Math.round(mean * 10) / 10,")).toBe(true);
|
||||
expect(ROUNDING.test("value: importValue(n)")).toBe(false);
|
||||
expect(ROUNDING.test("Math.round((end - start) / 1000)")).toBe(false);
|
||||
expect(ROUNDING.test("Math.round(mmol * MGDL_PER_MMOL)")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,7 @@
|
||||
* Privacy: the result carries counts only, per type and per app. No value,
|
||||
* no timestamp and nothing a person typed (notes, titles) leaves this module.
|
||||
*/
|
||||
import { importValue } from "@/lib/integrations/import-value";
|
||||
import type {
|
||||
FlowLevel,
|
||||
GlucoseContext,
|
||||
@@ -547,7 +548,7 @@ class ImportRun {
|
||||
this.stat("PULSE").read += 1;
|
||||
await sink.push({
|
||||
type: "PULSE",
|
||||
value: Math.round(mean * 10) / 10,
|
||||
value: importValue(mean, 1),
|
||||
valueMin: hcNumber(best.lo),
|
||||
valueMax: hcNumber(best.hi),
|
||||
// The middle of the local hour, in the record's own zone.
|
||||
@@ -889,7 +890,10 @@ class ImportRun {
|
||||
const days: Array<{ dayKey: string; value: number }> = [];
|
||||
for (const day of this.dailyGroups(sql, HC_CATEGORY.ACTIVITY)) {
|
||||
this.stat(spec.type).read += 1;
|
||||
days.push({ dayKey: day.dayKey, value: day.total * spec.factor });
|
||||
days.push({
|
||||
dayKey: day.dayKey,
|
||||
value: importValue(day.total * spec.factor),
|
||||
});
|
||||
if (days.length >= WRITE_BATCH) {
|
||||
await this.writeDayTotals(spec.type, days.splice(0));
|
||||
}
|
||||
@@ -1136,7 +1140,7 @@ class ImportRun {
|
||||
): Promise<void> {
|
||||
if (entries.length === 0) return;
|
||||
const valid = entries.filter((e) => {
|
||||
const amount = Math.round(e.amount * 1000) / 1000;
|
||||
const amount = importValue(e.amount, 3);
|
||||
if (amount > NUTRIENT_CATALOG[e.code].plausibleDailyMax) {
|
||||
this.nutrients.outOfRange += 1;
|
||||
this.skip("out_of_range", 1, `nutrient:${e.code}`);
|
||||
@@ -1160,7 +1164,7 @@ class ImportRun {
|
||||
);
|
||||
const fresh: Prisma.NutrientIntakeDayCreateManyInput[] = [];
|
||||
for (const e of valid) {
|
||||
const amount = Math.round(e.amount * 1000) / 1000;
|
||||
const amount = importValue(e.amount, 3);
|
||||
const prior = existing.get(`${e.day}|${e.code}`);
|
||||
if (prior === undefined) {
|
||||
fresh.push({
|
||||
@@ -1597,8 +1601,19 @@ class MeasurementSink {
|
||||
private readonly hourlyPulse = false,
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Every measurement value this import writes passes here (or through
|
||||
* `writeDayTotals`), so this is where it takes the one import rounding:
|
||||
* a mapper's unit factor (grams to kilograms, millis to minutes) leaves
|
||||
* binary noise that would otherwise be stored and compared as is.
|
||||
*/
|
||||
async push(row: PendingRow): Promise<void> {
|
||||
this.rows.push(row);
|
||||
this.rows.push({
|
||||
...row,
|
||||
value: importValue(row.value),
|
||||
valueMin: row.valueMin == null ? row.valueMin : importValue(row.valueMin),
|
||||
valueMax: row.valueMax == null ? row.valueMax : importValue(row.valueMax),
|
||||
});
|
||||
if (this.rows.length >= WRITE_BATCH) await this.flush();
|
||||
}
|
||||
|
||||
|
||||
@@ -26,4 +26,12 @@ describe("importValue", () => {
|
||||
expect(importValue(1e-7)).toBe(0);
|
||||
expect(importValue(1e22)).toBe(1e22);
|
||||
});
|
||||
|
||||
it("rounds to another precision on request, halves away from zero", () => {
|
||||
expect(importValue(72.34999, 1)).toBe(72.3);
|
||||
expect(importValue(1.0005, 3)).toBe(1.001);
|
||||
expect(importValue(-2.5, 0)).toBe(-3);
|
||||
expect(importValue(0.30000000000000004, 3)).toBe(0.3);
|
||||
expect(() => importValue(1, 7)).toThrow(RangeError);
|
||||
});
|
||||
});
|
||||
@@ -9,20 +9,27 @@
|
||||
* also mis-rounds on halves (`(1.005).toFixed(2)` is `"1.00"`, because the
|
||||
* double below 1.005 is what gets rounded).
|
||||
*
|
||||
* Two decimals, rounded half away from zero on the decimal the value prints
|
||||
* as, so the result is the double nearest to a two-place decimal and prints
|
||||
* cleanly. Two places is the precision every provider client already stored;
|
||||
* this keeps it. `src/__tests__/provider-rounding-guard.test.ts` keeps
|
||||
* `toFixed` out of the provider directories.
|
||||
* Two decimals by default, rounded half away from zero on the decimal the
|
||||
* value prints as, so the result is the double nearest to a short decimal
|
||||
* and prints cleanly. Two places is the precision every provider client
|
||||
* already stored; this keeps it. A source whose values need another
|
||||
* precision (a nutrient in grams to the milligram) passes `places` rather
|
||||
* than rounding privately. `src/__tests__/provider-rounding-guard.test.ts`
|
||||
* keeps `toFixed` and hand-rolled `Math.round(x * 10^n) / 10^n` out of the
|
||||
* provider and import directories.
|
||||
*/
|
||||
export function importValue(n: number): number {
|
||||
export function importValue(n: number, places = 2): number {
|
||||
if (!Number.isFinite(n)) return n;
|
||||
if (!Number.isInteger(places) || places < 0 || places > 6) {
|
||||
throw new RangeError(`importValue: places must be 0..6, got ${places}`);
|
||||
}
|
||||
const magnitude = Math.abs(n);
|
||||
const text = String(magnitude);
|
||||
const scale = 10 ** places;
|
||||
// A value printed in exponent form is below a millionth or beyond 1e21;
|
||||
// shifting its text would not parse, and plain arithmetic is exact enough.
|
||||
const hundredths = text.includes("e")
|
||||
? Math.round(magnitude * 100)
|
||||
: Math.round(Number(`${text}e2`));
|
||||
return (Math.sign(n) * hundredths) / 100 || 0;
|
||||
const units = text.includes("e")
|
||||
? Math.round(magnitude * scale)
|
||||
: Math.round(Number(`${text}e${places}`));
|
||||
return (Math.sign(n) * units) / scale || 0;
|
||||
}
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
rmSync,
|
||||
writeFileSync,
|
||||
} from "node:fs";
|
||||
import { DatabaseSync } from "node:sqlite";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
|
||||
@@ -233,6 +234,29 @@ describe("Health Connect import, mapping", () => {
|
||||
expect(JSON.stringify(result)).not.toContain("Evening run");
|
||||
});
|
||||
|
||||
it("stores a scaled value with the import rounding, not its unit factor's noise", async () => {
|
||||
const prisma = getPrismaClient();
|
||||
const user = await createUser("hc-round");
|
||||
const { path } = fixtureDb("round", { days: 1, endUtc: today });
|
||||
// Grams to kilograms: 80 345.678 g is 80.345678 kg before rounding.
|
||||
const db = new DatabaseSync(path);
|
||||
db.exec("UPDATE weight_record_table SET weight = 80345.678");
|
||||
db.close();
|
||||
await importHealthConnectExport({
|
||||
prisma,
|
||||
dbPath: path,
|
||||
userId: user.id,
|
||||
userTimezone: "Europe/Berlin",
|
||||
connectedIntegrations: [],
|
||||
});
|
||||
const weights = await prisma.measurement.findMany({
|
||||
where: { userId: user.id, type: "WEIGHT", source: "HEALTH_CONNECT" },
|
||||
select: { value: true },
|
||||
});
|
||||
expect(weights.length).toBeGreaterThan(0);
|
||||
for (const w of weights) expect(w.value).toBe(80.35);
|
||||
});
|
||||
|
||||
it("writes nothing the second time the same export comes in", async () => {
|
||||
const prisma = getPrismaClient();
|
||||
const user = await createUser("hc-twice");
|
||||
|
||||
Reference in new issue
Block a user