mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-07 04:12:04 +00:00
fix(ops): stop the distance repair losing its rollup tail in silence
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.
This commit is contained in:
1 parent
69e4ba7b8a
commit
610083aa4d
4 files changed
+166
-22
No files matched your search
@@ -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 <userId>
|
||||
```
|
||||
|
||||
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
|
||||
|
||||
@@ -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<void> {
|
||||
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<void> {
|
||||
` • ${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<void> {
|
||||
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<void> {
|
||||
"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()
|
||||
|
||||
@@ -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<string, Date>();
|
||||
const rollupsPending = new Set<string>();
|
||||
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<string, AccountRepairPlan>();
|
||||
@@ -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<string> => {
|
||||
// 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,
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
Reference in new issue
Block a user