diff --git a/docs/ops/apple-health-distance-repair.md b/docs/ops/apple-health-distance-repair.md index f67263e5e..1b9a10cb6 100644 --- a/docs/ops/apple-health-distance-repair.md +++ b/docs/ops/apple-health-distance-repair.md @@ -112,7 +112,19 @@ metre factor, `sync_version` incremented and `updated_at` bumped so paired iOS clients pick the rows up on their next delta sync, and an audit row (`measurement.apple_health_distance.repaired`) recording the unit, the factor and the row count. The DAY/WEEK/MONTH/YEAR rollups and the cached status -insights for the touched days are recomputed afterwards. +insights for the touched days are recomputed afterwards, outside that +transaction. + +If that recompute fails, the rows are right and every chart is not, and no +later run of this script would notice — it skips a repaired account. So the +audit row is written with `rollupsRefreshed: false` and only flipped to `true` +once the recompute has returned. A run that could not finish it prints the +account and the command to re-run, exits non-zero, and a later dry run repeats +the warning until the backfill has been run: + +```bash +pnpm dlx tsx scripts/backfill-rollups.ts --user +``` A row whose repaired value would leave the plausible range (0–200 000 m) refuses the **whole account**: nothing is written, the offending rows and the diff --git a/scripts/repair-apple-health-distance.ts b/scripts/repair-apple-health-distance.ts index a7ef1fe5a..be77a3b8f 100644 --- a/scripts/repair-apple-health-distance.ts +++ b/scripts/repair-apple-health-distance.ts @@ -28,6 +28,10 @@ * the WHOLE account: one such row is evidence the rows are not the 1000x * class (an account re-imported on the fixed build looks exactly like * this), and its rest days would otherwise be multiplied again. + * - The rollup/status tail runs after the transaction. If it throws, the + * rows are repaired and the derived tier is not; the script names the + * backfill to re-run, the audit row keeps `rollupsRefreshed: false` so a + * later run can still see it, and the exit code is non-zero. * - `--unit=mi` for an imperial archive (default `km`). The stored row keeps * no record of the archive's unit — that is the defect — so the operator * names it; a mismatched unit is why the flag exists rather than a guess. @@ -45,6 +49,7 @@ import { parseArchiveUnit, planAppleHealthDistanceRepair, repairFactorFor, + rollupRepairCommand, } from "@/lib/measurements/repair-apple-health-distance"; function readArchiveUnit(): "km" | "mi" { @@ -79,6 +84,7 @@ async function main(): Promise { let totalUpdated = 0; let totalSkipped = 0; let refused = 0; + let rollupsPending = 0; for (const plan of plans) { if (plan.alreadyRepairedAt) { @@ -86,6 +92,15 @@ async function main(): Promise { ` • ${plan.userId}: already repaired on ` + `${plan.alreadyRepairedAt.toISOString()} — ${plan.candidateCount} row(s) left alone.`, ); + if (plan.rollupsPending) { + rollupsPending += 1; + console.log( + " ! rollups PENDING from that run: the DAY/WEEK/MONTH/YEAR " + + "buckets and the cached status insights still hold the " + + "pre-repair numbers.", + ); + console.log(` re-run: ${rollupRepairCommand(plan.userId)}`); + } continue; } @@ -133,6 +148,18 @@ async function main(): Promise { console.log( ` ✓ repaired ${outcome.updated} row(s), skipped ${outcome.skipped}`, ); + if (outcome.rollupsPending) { + rollupsPending += 1; + console.error( + ` ! ${plan.userId}: rows repaired, but the rollup refresh failed.`, + ); + console.error(outcome.rollupError); + console.error( + " The DAY/WEEK/MONTH/YEAR buckets and the cached status " + + "insights still hold the pre-repair numbers.", + ); + console.error(` re-run: ${rollupRepairCommand(plan.userId)}`); + } } console.log( @@ -143,6 +170,16 @@ async function main(): Promise { "Read the worked examples above and decide before re-running with " + "--apply.", ); + + if (rollupsPending > 0) { + // The rows are right and the derived tier is not, which no later run of + // this script will notice — it skips a repaired account. Fail loudly. + console.error( + `\n${rollupsPending} account(s) need the rollup backfill re-run ` + + "(commands above).", + ); + process.exitCode = 1; + } } main() diff --git a/src/lib/measurements/repair-apple-health-distance.ts b/src/lib/measurements/repair-apple-health-distance.ts index 13d760cb7..bd2947586 100644 --- a/src/lib/measurements/repair-apple-health-distance.ts +++ b/src/lib/measurements/repair-apple-health-distance.ts @@ -97,6 +97,13 @@ export interface AccountRepairPlan { outOfRange: RepairCandidateRow[]; /** Non-null when this account already carries the repair's audit row. */ alreadyRepairedAt: Date | null; + /** + * True when a previous run wrote the rows but its rollup tail did not + * finish — the audit row still says `rollupsRefreshed: false`, so the + * DAY/WEEK/MONTH/YEAR buckets and the cached status insights still hold + * the pre-repair numbers. Meaningless unless `alreadyRepairedAt` is set. + */ + rollupsPending: boolean; /** Candidate rows found, whatever the disposition above. */ candidateCount: number; } @@ -147,12 +154,15 @@ export async function planAppleHealthDistanceRepair( action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, userId: { in: userIds }, }, - select: { userId: true, createdAt: true }, + select: { userId: true, createdAt: true, details: true }, orderBy: { createdAt: "asc" }, }); const repairedAt = new Map(); + const rollupsPending = new Set(); for (const entry of repaired) { - if (entry.userId) repairedAt.set(entry.userId, entry.createdAt); + if (!entry.userId) continue; + repairedAt.set(entry.userId, entry.createdAt); + if (!rollupsRefreshedIn(entry.details)) rollupsPending.add(entry.userId); } const plans = new Map(); @@ -164,6 +174,7 @@ export async function planAppleHealthDistanceRepair( repairable: [], outOfRange: [], alreadyRepairedAt: repairedAt.get(row.userId) ?? null, + rollupsPending: rollupsPending.has(row.userId), candidateCount: 0, }; plans.set(row.userId, plan); @@ -184,6 +195,31 @@ export async function planAppleHealthDistanceRepair( return Array.from(plans.values()); } +/** + * The command that heals the rollup tail when it did not run. + * + * Named here rather than in the script so the message an operator reads is + * the same one the module can be asked for. + */ +export function rollupRepairCommand(userId: string): string { + return `pnpm dlx tsx scripts/backfill-rollups.ts --user ${userId}`; +} + +/** Did a repair audit row record a finished rollup tail? */ +function rollupsRefreshedIn(details: string | null): boolean { + if (!details) return false; + try { + const parsed: unknown = JSON.parse(details); + return ( + typeof parsed === "object" && + parsed !== null && + (parsed as { rollupsRefreshed?: unknown }).rollupsRefreshed === true + ); + } catch { + return false; + } +} + export interface RepairOutcome { userId: string; updated: number; @@ -195,6 +231,15 @@ export interface RepairOutcome { refusedReason: string | null; /** The out-of-range candidates — what a refusal was decided on. */ skippedRows: RepairCandidateRow[]; + /** + * True when the rows were written but the rollup tail was not. The audit + * row still reads `rollupsRefreshed: false`, so a later run can tell this + * apart from a finished repair; the caller must surface + * `rollupRepairCommand(userId)` and exit non-zero. + */ + rollupsPending: boolean; + /** What the rollup tail threw, when it threw. */ + rollupError: unknown; } /** @@ -236,6 +281,8 @@ export async function applyAppleHealthDistanceRepair( updated: 0, skipped, skippedRows: plan.outOfRange, + rollupsPending: false, + rollupError: null, }; if (plan.alreadyRepairedAt) { return { @@ -252,7 +299,20 @@ export async function applyAppleHealthDistanceRepair( } const ids = plan.repairable.map((row) => row.id); - await client.$transaction(async (tx) => { + const details = { + type: REPAIRED_TYPE, + archiveUnit: options.archiveUnit, + factor, + rows: ids.length, + skippedOutOfRange: skipped, + issue: 944, + // Flipped to true only once the rollup tail below has finished. A row + // left at false says "rows repaired, rollups still pre-repair", which is + // what a re-run has to be able to see: the account is skipped from here + // on, so nothing else would ever notice. + rollupsRefreshed: false, + }; + const auditId = await client.$transaction(async (tx): Promise => { // One statement per chunk rather than one per row: an account with a // decade of history carries thousands of rows and an interactive // transaction has a clock on it. `sync_version` + `updated_at` are @@ -272,27 +332,50 @@ export async function applyAppleHealthDistanceRepair( client: tx, userId: plan.userId, actorUserId: null, - details: { - type: REPAIRED_TYPE, - archiveUnit: options.archiveUnit, - factor, - rows: ids.length, - skippedOutOfRange: skipped, - issue: 944, - }, + details, }); + // Read the row back rather than widening `auditLog`'s return type for + // ~600 call sites. Inside the same transaction, and an account can hold + // at most one of these rows — the second run is refused above — so the + // lookup is exact, not a guess at the newest. + const entry = await tx.auditLog.findFirstOrThrow({ + where: { + action: APPLE_HEALTH_DISTANCE_REPAIR_ACTION, + userId: plan.userId, + }, + select: { id: true }, + }); + return entry.id; }); - // Best-effort tail: the DAY/WEEK/MONTH/YEAR rollups and the cached - // status insights hold the pre-repair numbers until they are recomputed. - await afterMeasurementMutation( - plan.userId, - plan.repairable.map((row) => ({ - type: REPAIRED_TYPE, - measuredAt: row.measuredAt, - })), - "repair-apple-health-distance", - ); + // The tail: the DAY/WEEK/MONTH/YEAR rollups and the cached status insights + // hold the pre-repair numbers until they are recomputed. It runs outside + // the transaction, so a throw here leaves the rows repaired and the derived + // tier stale — and the account is skipped from the next run on. Hence the + // flag: it is flipped only after the tail returned, and the caller is told + // to run the backfill and to exit non-zero. + let rollupsPending = false; + let rollupError: unknown = null; + try { + await afterMeasurementMutation( + plan.userId, + plan.repairable.map((row) => ({ + type: REPAIRED_TYPE, + measuredAt: row.measuredAt, + })), + "repair-apple-health-distance", + ); + await client.auditLog.update({ + where: { id: auditId }, + data: { details: JSON.stringify({ ...details, rollupsRefreshed: true }) }, + }); + } catch (err) { + // Either leg failing leaves the flag at false, which is the safe + // direction: an operator re-running the backfill needlessly costs time, + // one who never learns it is due keeps wrong numbers on every chart. + rollupsPending = true; + rollupError = err; + } return { userId: plan.userId, @@ -300,6 +383,8 @@ export async function applyAppleHealthDistanceRepair( skipped, refusedReason: null, skippedRows: plan.outOfRange, + rollupsPending, + rollupError, }; } diff --git a/tests/integration/apple-health-distance-repair.test.ts b/tests/integration/apple-health-distance-repair.test.ts index b3ead4daf..11d40f1eb 100644 --- a/tests/integration/apple-health-distance-repair.test.ts +++ b/tests/integration/apple-health-distance-repair.test.ts @@ -155,7 +155,17 @@ describe("Apple Health export distance repair — integration", () => { factor: 1000, rows: 2, issue: 944, + // Flipped only after the rollup tail returned. A run that repaired the + // rows and lost the tail leaves this false, and the account is skipped + // from here on — so it is the only place the gap can still be seen. + rollupsRefreshed: true, }); + expect(outcome.rollupsPending).toBe(false); + + const second = await planAppleHealthDistanceRepair(prisma, { + archiveUnit: "km", + }); + expect(second[0].rollupsPending).toBe(false); }); it("is idempotent — a second run finds nothing to repair", async () => {