mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-14 14:40:52 +00:00
Account for every dropped Apple Health sample the result used to hide
Three silent gaps in the import accounting, one commit: The reproductive samples of an export land in recordsRead but were discarded without a counter when cycle tracking is off — the result now names them (samplesSkippedModuleDisabled). A cycle flush that failed partway zeroed its stats AFTER day-by-day writes had landed; the flush is now fault-isolated per day and reports daysFailed plus the first failure reason instead of claiming nothing happened. And the import card hardcoded skipped: 0 while the worker has computed, persisted and served per-class skip counters since v1.15 — it now renders them, splitting deliberate exclusions (unsupported and deferred types, a disabled cycle module) from genuine refusals (unparseable, out of range, failed writes), with a per-type breakdown. Only refusals tip the outcome tone.
This commit is contained in:
1 parent
a45dde2039
commit
db6465c17d
12 files changed
+530
-31
No files matched your search
@@ -0,0 +1,85 @@
|
||||
/**
|
||||
* Cycle-import accumulator — flush fault isolation (A6).
|
||||
*
|
||||
* The pre-fix flush let one colliding day throw out of the loop; the
|
||||
* importer's catch then zeroed the stats, so a partial write reported
|
||||
* `samplesConsumed: 0` over rows already in the table. The flush is now
|
||||
* fault-isolated per day and its stats stay honest: what landed counts,
|
||||
* what failed is named.
|
||||
*
|
||||
* Watched red: with the per-day try/catch removed from `flush()` the
|
||||
* partial-failure test throws instead of reporting
|
||||
* `{ daysUpserted: 1, daysFailed: 1 }`.
|
||||
*/
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const mocks = vi.hoisted(() => ({
|
||||
upsert: vi.fn(),
|
||||
findManyCycles: vi.fn(async () => []),
|
||||
findUniqueProfile: vi.fn(async () => null),
|
||||
updateManyProfile: vi.fn(async () => ({ count: 0 })),
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/db", () => ({
|
||||
prisma: {
|
||||
menstrualCycle: { findMany: mocks.findManyCycles },
|
||||
cycleProfile: {
|
||||
findUnique: mocks.findUniqueProfile,
|
||||
updateMany: mocks.updateManyProfile,
|
||||
},
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/cycle/day-log-write", () => ({
|
||||
upsertCycleDayLog: (...args: unknown[]) => mocks.upsert(...args),
|
||||
}));
|
||||
vi.mock("@/lib/cycle/gate", () => ({
|
||||
isCycleAvailableForUser: vi.fn(async () => true),
|
||||
}));
|
||||
vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn() }));
|
||||
|
||||
import { CycleImportAccumulator } from "../import-accumulator";
|
||||
|
||||
const FLOW = "HKCategoryTypeIdentifierMenstrualFlow";
|
||||
|
||||
beforeEach(() => {
|
||||
mocks.upsert.mockReset();
|
||||
});
|
||||
|
||||
describe("CycleImportAccumulator.flush — per-day fault isolation", () => {
|
||||
it("keeps counting after a failed day and names the failure", async () => {
|
||||
const acc = new CycleImportAccumulator("u1", "Europe/Berlin");
|
||||
expect(
|
||||
acc.consume(FLOW, "2026-05-10", "HKCategoryValueMenstrualFlowMedium"),
|
||||
).toBe(true);
|
||||
expect(
|
||||
acc.consume(FLOW, "2026-05-11", "HKCategoryValueMenstrualFlowLight"),
|
||||
).toBe(true);
|
||||
|
||||
mocks.upsert
|
||||
.mockRejectedValueOnce(new Error("colliding day"))
|
||||
.mockResolvedValueOnce({ existed: false });
|
||||
|
||||
const stats = await acc.flush();
|
||||
|
||||
expect(stats.samplesConsumed).toBe(2);
|
||||
expect(stats.daysUpserted).toBe(1);
|
||||
expect(stats.daysInserted).toBe(1);
|
||||
expect(stats.daysFailed).toBe(1);
|
||||
expect(stats.firstFailureReason).toBe("colliding day");
|
||||
});
|
||||
|
||||
it("reports clean stats when every day lands", async () => {
|
||||
const acc = new CycleImportAccumulator("u1", "Europe/Berlin");
|
||||
acc.consume(FLOW, "2026-05-10", "3");
|
||||
mocks.upsert.mockResolvedValue({ existed: false });
|
||||
|
||||
const stats = await acc.flush();
|
||||
expect(stats).toMatchObject({
|
||||
samplesConsumed: 1,
|
||||
daysUpserted: 1,
|
||||
daysFailed: 0,
|
||||
firstFailureReason: null,
|
||||
samplesSkippedModuleDisabled: 0,
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -37,8 +37,11 @@ export const HK_CYCLE_DAY_EXTERNAL_PREFIX = "hkcycle:";
|
||||
/** A zeroed cycle-import result — the no-cycle / aborted-flush fallback. */
|
||||
export const EMPTY_CYCLE_IMPORT_STATS: CycleImportStats = {
|
||||
samplesConsumed: 0,
|
||||
samplesSkippedModuleDisabled: 0,
|
||||
daysUpserted: 0,
|
||||
daysInserted: 0,
|
||||
daysFailed: 0,
|
||||
firstFailureReason: null,
|
||||
goalNudged: false,
|
||||
};
|
||||
|
||||
@@ -54,10 +57,24 @@ interface DayBucket {
|
||||
export interface CycleImportStats {
|
||||
/** Reproductive samples consumed into a day-bucket. */
|
||||
samplesConsumed: number;
|
||||
/**
|
||||
* Reproductive samples the export carried but the fold dropped because
|
||||
* cycle tracking is disabled for the account. Named so the "read 900k /
|
||||
* imported 880k" gap stops being unexplained.
|
||||
*/
|
||||
samplesSkippedModuleDisabled: number;
|
||||
/** Day-logs upserted at flush. */
|
||||
daysUpserted: number;
|
||||
/** Day-logs that the write created (vs refreshed). */
|
||||
daysInserted: number;
|
||||
/**
|
||||
* Day-logs whose write FAILED at flush. The flush is fault-isolated per
|
||||
* day, so `daysUpserted` counts what actually landed rather than being
|
||||
* zeroed by one colliding day.
|
||||
*/
|
||||
daysFailed: number;
|
||||
/** The first per-day failure's message, for the operator-facing line. */
|
||||
firstFailureReason: string | null;
|
||||
/** Whether a CycleProfile goal nudge fired. */
|
||||
goalNudged: boolean;
|
||||
}
|
||||
@@ -87,6 +104,16 @@ export class CycleImportAccumulator {
|
||||
return this.byDay.size > 0;
|
||||
}
|
||||
|
||||
/** Samples consumed so far — for the module-disabled skip accounting. */
|
||||
sampleCount(): number {
|
||||
return this.samplesConsumed;
|
||||
}
|
||||
|
||||
/** Accumulated day-buckets — for the whole-flush-failed accounting. */
|
||||
dayCount(): number {
|
||||
return this.byDay.size;
|
||||
}
|
||||
|
||||
/**
|
||||
* Fold one reproductive HK sample into its day-bucket. `dayKey` is the
|
||||
* already-resolved user-local `YYYY-MM-DD`. `rawValue` is the sample's
|
||||
@@ -177,8 +204,11 @@ export class CycleImportAccumulator {
|
||||
async flush(): Promise<CycleImportStats> {
|
||||
const stats: CycleImportStats = {
|
||||
samplesConsumed: this.samplesConsumed,
|
||||
samplesSkippedModuleDisabled: 0,
|
||||
daysUpserted: 0,
|
||||
daysInserted: 0,
|
||||
daysFailed: 0,
|
||||
firstFailureReason: null,
|
||||
goalNudged: false,
|
||||
};
|
||||
if (this.byDay.size === 0) return stats;
|
||||
@@ -230,15 +260,26 @@ export class CycleImportAccumulator {
|
||||
};
|
||||
|
||||
const cycleId = owningCycleId(dayKey);
|
||||
const result = await upsertCycleDayLog(
|
||||
this.userId,
|
||||
entry,
|
||||
this.userTimezone,
|
||||
cycleId,
|
||||
encryptSensitive,
|
||||
);
|
||||
stats.daysUpserted += 1;
|
||||
if (!result.existed) stats.daysInserted += 1;
|
||||
// Fault-isolated per day: one colliding day must not zero the stats
|
||||
// for the days that DID land (the pre-fix shape reported
|
||||
// `samplesConsumed: 0` after partial writes — a false "nothing
|
||||
// happened" over rows already in the table).
|
||||
try {
|
||||
const result = await upsertCycleDayLog(
|
||||
this.userId,
|
||||
entry,
|
||||
this.userTimezone,
|
||||
cycleId,
|
||||
encryptSensitive,
|
||||
);
|
||||
stats.daysUpserted += 1;
|
||||
if (!result.existed) stats.daysInserted += 1;
|
||||
} catch (err: unknown) {
|
||||
stats.daysFailed += 1;
|
||||
stats.firstFailureReason ??=
|
||||
err instanceof Error ? err.message : String(err);
|
||||
continue;
|
||||
}
|
||||
|
||||
// The most-recent contraceptive method across the import drives the
|
||||
// single goal nudge (applied once after the day loop).
|
||||
|
||||
@@ -15,6 +15,14 @@ vi.mock("@/lib/arrivals/emit-shared", () => ({
|
||||
emitDataArrival: vi.fn().mockResolvedValue(undefined),
|
||||
}));
|
||||
|
||||
// The cycle-fold gate — mutable so the module-off drop accounting can be
|
||||
// exercised without a DB. Default OFF, matching every pre-existing fixture
|
||||
// (none carries reproductive records, so the value was never read before).
|
||||
const cycleGate = vi.hoisted(() => ({ enabled: false }));
|
||||
vi.mock("@/lib/cycle/gate", () => ({
|
||||
isCycleAvailableForUser: vi.fn(async () => cycleGate.enabled),
|
||||
}));
|
||||
|
||||
import { Prisma, type PrismaClient } from "@/generated/prisma/client";
|
||||
import {
|
||||
hashSampleKey,
|
||||
@@ -1172,3 +1180,49 @@ describe("streamParseExportXml — memory ceiling", () => {
|
||||
expect(heapDelta).toBeLessThan(100 * 1024 * 1024);
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* A6 — reproductive samples with cycle tracking OFF must be counted, not
|
||||
* silently discarded. The pre-fix importer read them into `recordsRead`,
|
||||
* dropped the accumulator, and returned zeroed cycle stats — the
|
||||
* "read 900k / imported 880k" unexplained gap.
|
||||
*
|
||||
* Watched red: with the module-off branch removed from the importer
|
||||
* (cycle reset to EMPTY_CYCLE_IMPORT_STATS when the gate is off) the
|
||||
* first assertion fails with `samplesSkippedModuleDisabled: 0`.
|
||||
*/
|
||||
describe("streamParseExportXml — cycle module off", () => {
|
||||
function cycleExportXml(): string {
|
||||
return `<?xml version="1.0" encoding="UTF-8"?>
|
||||
<HealthData locale="en_US">
|
||||
<ExportDate value="2026-05-15 14:32:01 +0200"/>
|
||||
<Record type="HKCategoryTypeIdentifierMenstrualFlow" value="HKCategoryValueMenstrualFlowMedium" startDate="2026-05-10 08:00:00 +0200" endDate="2026-05-10 08:00:00 +0200"/>
|
||||
<Record type="HKQuantityTypeIdentifierBodyMass" value="81.5" unit="kg" startDate="2026-05-10 07:00:00 +0200" endDate="2026-05-10 07:00:00 +0200"/>
|
||||
</HealthData>`;
|
||||
}
|
||||
|
||||
it("names the dropped reproductive samples in the cycle stats", async () => {
|
||||
cycleGate.enabled = false;
|
||||
const tmp = mkdtempSync(join(tmpdir(), "healthlog-parser-test-"));
|
||||
const xmlPath = join(tmp, "export.xml");
|
||||
writeFileSync(xmlPath, cycleExportXml());
|
||||
const prisma = makeFakePrisma();
|
||||
|
||||
const result = await streamParseExportXml({
|
||||
xmlPath,
|
||||
userId: "user-1",
|
||||
userTimezone: "Europe/Berlin",
|
||||
// eslint-disable-next-line @typescript-eslint/no-explicit-any
|
||||
prisma: prisma as any,
|
||||
});
|
||||
|
||||
// The dropped sample is named, so recordsRead minus the per-type
|
||||
// reads stays explained.
|
||||
expect(result.cycle.samplesSkippedModuleDisabled).toBe(1);
|
||||
expect(result.cycle.samplesConsumed).toBe(0);
|
||||
expect(result.cycle.daysUpserted).toBe(0);
|
||||
// The sibling weight record is unaffected.
|
||||
expect(result.perType.WEIGHT?.inserted).toBe(1);
|
||||
expect(result.totals.recordsRead).toBe(2);
|
||||
});
|
||||
});
|
||||
@@ -1105,17 +1105,33 @@ export async function streamParseExportXml(
|
||||
// reproductive samples AND the account has cycle tracking enabled. The
|
||||
// empty-accumulator short-circuit also keeps the no-cycle import path
|
||||
// (and its unit tests) free of any cycle DB round-trip. A flush failure
|
||||
// (e.g. a single colliding day) must never abort the whole import — fold
|
||||
// the error into zeroed cycle stats and continue.
|
||||
if (cycleAccumulator.hasSamples() && (await cycleAccumulator.isEnabled())) {
|
||||
try {
|
||||
cycle = await cycleAccumulator.flush();
|
||||
rowsUpserted += cycle.daysUpserted;
|
||||
} catch (err: unknown) {
|
||||
cycle = { ...EMPTY_CYCLE_IMPORT_STATS };
|
||||
console.warn(
|
||||
`cycle import flush failed: ${err instanceof Error ? err.message : String(err)}`,
|
||||
);
|
||||
// must never abort the whole import — per-day failures are isolated
|
||||
// inside `flush()` (its stats stay honest); a whole-flush failure (the
|
||||
// hoisted lookups) is folded into stats that still name the consumed
|
||||
// samples and the reason instead of reading as "nothing happened".
|
||||
if (cycleAccumulator.hasSamples()) {
|
||||
if (await cycleAccumulator.isEnabled()) {
|
||||
try {
|
||||
cycle = await cycleAccumulator.flush();
|
||||
rowsUpserted += cycle.daysUpserted;
|
||||
} catch (err: unknown) {
|
||||
const reason = err instanceof Error ? err.message : String(err);
|
||||
cycle = {
|
||||
...EMPTY_CYCLE_IMPORT_STATS,
|
||||
samplesConsumed: cycleAccumulator.sampleCount(),
|
||||
daysFailed: cycleAccumulator.dayCount(),
|
||||
firstFailureReason: reason,
|
||||
};
|
||||
console.warn(`cycle import flush failed: ${reason}`);
|
||||
}
|
||||
} else {
|
||||
// Cycle tracking is off for the account: the reproductive samples
|
||||
// are DROPPED, and the drop is named so the recordsRead-vs-imported
|
||||
// gap stays explained in the result the user sees.
|
||||
cycle = {
|
||||
...EMPTY_CYCLE_IMPORT_STATS,
|
||||
samplesSkippedModuleDisabled: cycleAccumulator.sampleCount(),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in new issue
Block a user