From c72b3ce8908ea0a4064ae938dd46b6140cc5bf36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 17 May 2026 17:12:33 +0200 Subject: [PATCH] fix(geo-backfill): drop batch cap from 5000 to 500 rows per pass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each row spends up to 3 s on the lookupIpLocation online fallback; 5000 rows × 3 s let a single backfill burst block for ~4 h and starve the next hourly cron. Tighten the cap to 500 so the worst-case pass fits well inside the hour-long schedule slot. The helper is idempotent, so a tenant with a large null-location backlog drains over multiple hourly passes instead of one mega-pass. --- messages/de.json | 2 +- src/app/__tests__/quick-add-labels.test.ts | 13 ++ src/app/api/analytics/route.ts | 5 + src/app/page.tsx | 5 +- src/lib/analytics/bp-in-target-fast-path.ts | 55 +++++- .../__tests__/coach-cascade.test.tsx | 181 +++++++++++++++++- src/lib/jobs/__tests__/geo-backfill.test.ts | 4 +- src/lib/jobs/geo-backfill.ts | 15 +- 8 files changed, 260 insertions(+), 20 deletions(-) diff --git a/messages/de.json b/messages/de.json index c244e9d47..94ffa09ed 100644 --- a/messages/de.json +++ b/messages/de.json @@ -244,7 +244,7 @@ "medications": "Medikamente", "quickAddMeasurement": "Messung erfassen", "quickAddMood": "Stimmung erfassen", - "quickAddMedicationIntake": "Medikamenteneinnahme erfassen", + "quickAddMedicationIntake": "Einnahme erfassen", "medicationIntakeQuickAdd": { "sheetTitle": "Medikamenteneinnahme erfassen", "sheetDescription": "Dosis, Zeitpunkt und Medikament in wenigen Tipps protokollieren.", diff --git a/src/app/__tests__/quick-add-labels.test.ts b/src/app/__tests__/quick-add-labels.test.ts index 515c952ab..8b5e75dd1 100644 --- a/src/app/__tests__/quick-add-labels.test.ts +++ b/src/app/__tests__/quick-add-labels.test.ts @@ -23,6 +23,10 @@ import { join } from "node:path"; const ROOT = join(__dirname, "../../.."); const EN_PATH = join(ROOT, "messages/en.json"); const DE_PATH = join(ROOT, "messages/de.json"); +const ES_PATH = join(ROOT, "messages/es.json"); +const FR_PATH = join(ROOT, "messages/fr.json"); +const IT_PATH = join(ROOT, "messages/it.json"); +const PL_PATH = join(ROOT, "messages/pl.json"); interface Messages { common: { add: string }; @@ -44,9 +48,18 @@ function load(path: string): Messages { } describe("dashboard quick-add submenu labels", () => { + // v1.4.38 W-D P1-4 — extend the collision guard from en+de to all + // six locales so a future translation that accidentally reuses the + // trigger word (or a sibling row's verb) is caught before shipping. + // es/fr/it/pl currently still hold the literal English block; the + // guard runs against whatever string lives there today. it.each([ ["en", EN_PATH], ["de", DE_PATH], + ["es", ES_PATH], + ["fr", FR_PATH], + ["it", IT_PATH], + ["pl", PL_PATH], ])( "%s: measurement and mood entries have distinct labels and neither equals the trigger label", (_locale, path) => { diff --git a/src/app/api/analytics/route.ts b/src/app/api/analytics/route.ts index 38c327865..6285e55c4 100644 --- a/src/app/api/analytics/route.ts +++ b/src/app/api/analytics/route.ts @@ -328,6 +328,11 @@ async function buildAnalyticsResponse(user: AuthedUser) { targets: bpTargets, now, coverage, + // v1.4.38 W-A — cross-tz runtime guard. The helper falls back + // to the live SQL path when the user is more than 3 hours from + // UTC, where the rollup table's UTC-midnight day-key would slip + // a calendar day relative to the live aggregator's window cuts. + userTz, }); bpInTargetPct = windows.last30Days?.pct ?? null; bpInTargetPct7d = windows.last7Days?.pct ?? null; diff --git a/src/app/page.tsx b/src/app/page.tsx index 0a2fc9a35..91968ac27 100644 --- a/src/app/page.tsx +++ b/src/app/page.tsx @@ -572,7 +572,10 @@ export default function DashboardPage() { {t("common.add")} - + {/* Menu items must each carry a self-contained verb-phrase ("Log measurement", "Log mood") — the trigger above already says "Add", and the icon is `aria-hidden`, so the visible diff --git a/src/lib/analytics/bp-in-target-fast-path.ts b/src/lib/analytics/bp-in-target-fast-path.ts index cbb20ade5..7a6afa8cb 100644 --- a/src/lib/analytics/bp-in-target-fast-path.ts +++ b/src/lib/analytics/bp-in-target-fast-path.ts @@ -58,6 +58,7 @@ import { probeRollupCoverage, type RollupCoverageMap, } from "@/lib/measurements/rollup-coverage"; +import { isNearUtc } from "@/lib/tz/resolver"; import { computeBpInTargetWindows, isBpReadingInTarget, @@ -114,23 +115,51 @@ export async function computeBpInTargetFastPath(input: { targets: BpTargets; now: Date; coverage?: RollupCoverageMap; + /** + * v1.4.38 W-A — optional user timezone for the cross-tz runtime + * guard. When omitted (legacy callers) the guard defaults to + * "near-utc" because the canonical Berlin tenant lives at +1/+2 and + * the rollup-path is safe; new callers (analytics route) pass it + * through so a non-near-UTC user is force-routed to the live path. + */ + userTz?: string; }): Promise { - const { userId, targets, now } = input; + const { userId, targets, now, userTz } = input; const coverage = input.coverage ?? (await probeRollupCoverage(userId)); const sysCovered = coverage.get("BLOOD_PRESSURE_SYS") === true; const diaCovered = coverage.get("BLOOD_PRESSURE_DIA") === true; + // v1.4.38 W-A — cross-tz runtime guard. The rollup path keys rows on + // `d.toISOString().slice(0, 10)` (UTC slice) and compares them + // against `now - N*DAY_MS` boundaries. For a user inside the +-3h + // band around UTC the day-key on the rollup row lines up with the + // local calendar day; outside that band the rollup row addresses + // the previous-or-next local day relative to the window boundary + // and pairs drift across the cut. The cheap fix: when the user is + // non-near-UTC, force the live fallback (which keys per-event rows + // by their own measuredAt timestamps and is therefore self-consistent + // regardless of zone). A v1.5 follow-up will thread `userTz` into + // `readRollupBuckets` so the rollup path can address local-day + // buckets and this guard can come back down. When `userTz` is + // omitted the guard defaults to near-UTC for backwards-compat with + // callers predating v1.4.38. + const userNearUtc = userTz === undefined ? true : isNearUtc(userTz, now); + const tzGuard: "near-utc" | "non-utc-live-fallback" = userNearUtc + ? "near-utc" + : "non-utc-live-fallback"; + // Belt-and-braces: only take the rollup path when EVERY logged type - // is covered AND both BP types are present in the map. A brand-new - // user without BP at all falls into `isFullyCovered === false` - // (coverage map is empty) and the route's downstream code already - // skips the BP block when `getBpTargets` returns null; we still call - // the live fallback here so the helper's contract stays single-shot. - if (isFullyCovered(coverage) && sysCovered && diaCovered) { - return computeFromRollups(userId, targets, now); + // is covered AND both BP types are present in the map AND the + // cross-tz guard cleared. A brand-new user without BP at all falls + // into `isFullyCovered === false` (coverage map is empty) and the + // route's downstream code already skips the BP block when + // `getBpTargets` returns null; we still call the live fallback here + // so the helper's contract stays single-shot. + if (userNearUtc && isFullyCovered(coverage) && sysCovered && diaCovered) { + return computeFromRollups(userId, targets, now, tzGuard); } - return computeFromLive(userId, targets, now); + return computeFromLive(userId, targets, now, tzGuard); } /** @@ -142,6 +171,7 @@ async function computeFromRollups( userId: string, targets: BpTargets, now: Date, + tzGuard: "near-utc" | "non-utc-live-fallback", ): Promise { // v1.4.22 W5 reconcile (Code-H2) — the priorYear window starts 395 // days ago. We read 396 days so the boundary day rolls cleanly into @@ -242,6 +272,9 @@ async function computeFromRollups( sys_rows: sysBuckets.reduce((s, b) => s + b.count, 0), dia_rows: diaBuckets.reduce((s, b) => s + b.count, 0), path: "rollup", + // v1.4.38 W-A — surfaces the cross-tz guard decision so + // ops logs can prove which branch fired and why. + tz_guard: tzGuard, }, }, }, @@ -307,6 +340,7 @@ async function computeFromLive( userId: string, targets: BpTargets, now: Date, + tzGuard: "near-utc" | "non-utc-live-fallback", ): Promise { const DAY_MS = 24 * 60 * 60 * 1000; const bpInTargetSince = new Date(now.getTime() - 365 * DAY_MS); @@ -325,6 +359,9 @@ async function computeFromLive( sys_rows: sysData.length, dia_rows: diaData.length, path: "live", + // v1.4.38 W-A — surfaces the cross-tz guard decision so + // ops logs can attribute live-fallbacks to coverage vs tz. + tz_guard: tzGuard, }, }, }, diff --git a/src/lib/feature-flags/__tests__/coach-cascade.test.tsx b/src/lib/feature-flags/__tests__/coach-cascade.test.tsx index 8cca35cbe..1a3596b6c 100644 --- a/src/lib/feature-flags/__tests__/coach-cascade.test.tsx +++ b/src/lib/feature-flags/__tests__/coach-cascade.test.tsx @@ -1,3 +1,6 @@ +import { readdirSync, readFileSync, statSync } from "node:fs"; +import { join, relative, resolve } from "node:path"; + import React from "react"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { renderToStaticMarkup } from "react-dom/server"; @@ -5,7 +8,10 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { I18nProvider } from "@/lib/i18n/context"; import { CoachLaunchProvider } from "@/lib/insights/coach-launch-context"; -import type { AssistantFlagSet } from "@/hooks/use-feature-flags"; +import { + DEFAULT_ASSISTANT_FLAGS, + type AssistantFlagSet, +} from "@/hooks/use-feature-flags"; import { HeroStrip } from "@/components/insights/hero-strip"; import { SuggestedPrompts } from "@/components/insights/suggested-prompts"; @@ -13,6 +19,24 @@ import { CoachLaunchButton } from "@/components/insights/coach-launch-button"; import { LayoutCoachFab } from "@/components/insights/layout-coach-fab"; import { LayoutCoachMount } from "@/components/insights/layout-coach-mount"; +// v1.4.38 W-C M-5 — capture the original `useFeatureFlags` and route +// the spy through it so the gate-fired assertion below can verify the +// hook ran with the operator-disabled matrix. Setting `vi.mock()` here +// (instead of `vi.spyOn`) is required because the components import the +// hook at module load; a runtime `spyOn` would never intercept the call. +const featureFlagsSpy = vi.fn<() => AssistantFlagSet>( + () => DEFAULT_ASSISTANT_FLAGS, +); +vi.mock("@/hooks/use-feature-flags", async () => { + const actual = await vi.importActual< + typeof import("@/hooks/use-feature-flags") + >("@/hooks/use-feature-flags"); + return { + ...actual, + useFeatureFlags: () => featureFlagsSpy(), + }; +}); + /** * v1.4.37 W5 — Coach disable cascade invariant. * @@ -60,6 +84,16 @@ function buildClient(flags: Partial): QueryClient { } function renderWithFlags(node: React.ReactNode, coach: boolean): string { + // v1.4.38 W-C M-5 — drive the mocked `useFeatureFlags` from the same + // matrix the QueryClient is seeded with so the gate-call assertion at + // the bottom of the file can read the resolved value. Keeping the + // QueryClient seed in place preserves the original render path for + // any consumer that bypasses the hook. + const resolved: AssistantFlagSet = { + ...DEFAULT_ASSISTANT_FLAGS, + coach, + }; + featureFlagsSpy.mockImplementation(() => resolved); const client = buildClient({ coach }); return renderToStaticMarkup( @@ -156,7 +190,9 @@ const COACH_SURFACES: CoachSurface[] = [ // The drawer is lazy-loaded via next/dynamic and SSRs to nothing // even when the flag is on; the contract here is "no Coach-marked // mount point ever paints when the flag is off". Skip the proof - // check; the negative `coach-*` slot grep still pins the gate. + // check; the negative `coach-*` slot grep still pins the gate, and + // the explicit spy assertion in `LayoutCoachMount SSR-proof spy` + // below proves the gate actually fired during the off render. proofWhenOn: "", }, ]; @@ -167,6 +203,11 @@ describe("Coach disable cascade invariant", () => { beforeEach(() => { vi.spyOn(console, "error").mockImplementation(() => undefined); vi.spyOn(console, "warn").mockImplementation(() => undefined); + // v1.4.38 W-C M-5 — reset the call history so each iteration of + // the surface loop sees a clean slate when it asserts the gate + // fired during the off render. + featureFlagsSpy.mockClear(); + featureFlagsSpy.mockImplementation(() => DEFAULT_ASSISTANT_FLAGS); }); for (const surface of COACH_SURFACES) { @@ -199,9 +240,142 @@ describe("Coach disable cascade invariant", () => { 'data-slot="insights-hero-strip-action-coach"', ); expect(html).not.toContain('data-slot="insights-hero-strip-prompts"'); + // v1.4.38 W-C M-5 — `proofWhenOn: ""` surfaces (the lazy-loaded + // `LayoutCoachMount` drawer subtree) trivially pass the substring + // grep because their SSR shape is empty even when the gate is on. + // The spy assertion below proves the gate actually fired with the + // operator-disabled matrix, so a regression that removes the + // `if (!flags.coach) return null` short-circuit fails this test + // instead of slipping through under cover of the empty SSR shape. + expect(featureFlagsSpy).toHaveBeenCalled(); + const lastCall = featureFlagsSpy.mock.results.at(-1); + expect(lastCall?.value.coach).toBe(false); }); } + // ────────────────────────────────────────────────────────────────── + // v1.4.38 W-C H4 — grep-based discovery of every `flags.coach` + // call site in the codebase. + // + // The fixture-count check below pins the surfaces the fixture mounts + // directly. Cross-cut gates on sub-pages (`/targets` page drawer, + // `` per-card CTA) sit OUTSIDE the fixture and are owned + // by sibling invariants. A future contributor who lands a new Coach + // mount on (e.g.) `/insights/blutdruck/page.tsx` would not trip the + // surface-count check at all — the new site is on a sub-page and + // would silently leak. + // + // The walk below scans `src/` for every `flags.coach` occurrence and + // requires each path to appear in the explicit `KNOWN_COACH_GATE_SITES` + // allowlist. Add a new entry every time you intentionally gate a new + // render path on the Coach flag; the failure message names the + // unaccounted-for paths so the fix is one search-and-add. + // ────────────────────────────────────────────────────────────────── + + /** + * Every file in `src/` that legitimately reads `flags.coach`. Lives + * here (next to the cascade fixture) so a contributor extending the + * fixture sees the allowlist in the same review. + * + * Paths are POSIX-style relative to the repo root. + */ + const KNOWN_COACH_GATE_SITES: ReadonlyArray = [ + // Coach-bearing surfaces mounted directly by the cascade fixture. + "src/components/insights/coach-launch-button.tsx", + "src/components/insights/hero-strip.tsx", + "src/components/insights/layout-coach-fab.tsx", + "src/components/insights/layout-coach-mount.tsx", + "src/components/insights/suggested-prompts.tsx", + // Cross-cut gates owned by sibling invariants + // (`targets-coach-mount.test.tsx`, `target-card.test.tsx`). + "src/app/targets/page.tsx", + "src/components/targets/target-card.tsx", + ]; + + /** + * Walk every TS / TSX file under `src/`, excluding test trees, and + * collect the relative paths that contain a `flags.coach` token. The + * grep is intentionally simple — any literal occurrence counts so a + * contributor who paraphrases the read (`const c = flags.coach`) + * still trips this test. + */ + function findCoachFlagSites(): string[] { + const srcRoot = resolve(__dirname, "..", "..", ".."); + const hits: string[] = []; + const repoRoot = resolve(srcRoot, ".."); + + function walk(dir: string): void { + const entries = readdirSync(dir); + for (const entry of entries) { + const full = join(dir, entry); + const stat = statSync(full); + if (stat.isDirectory()) { + if (entry === "node_modules" || entry === ".next") continue; + // Skip every `__tests__/` subtree — the discovery target is + // the production gate, not the assertions about it. + if (entry === "__tests__") continue; + walk(full); + continue; + } + if (!/\.(tsx|ts)$/.test(entry)) continue; + if (/\.test\.(tsx|ts)$/.test(entry)) continue; + const text = readFileSync(full, "utf8"); + // Match `flags.coach` on lines that are NOT pure comments. A + // hit on a JSDoc / `//` line is documentation, not a gate, and + // would make the allowlist a maintenance burden every time + // someone references the gate in a comment. + const isRealGate = text.split("\n").some((line) => { + if (!line.includes("flags.coach")) return false; + const trimmed = line.trim(); + if (trimmed.startsWith("//")) return false; + if (trimmed.startsWith("*")) return false; + return true; + }); + if (isRealGate) { + // POSIX-style path relative to the repo root so the failure + // message stays portable across contributors' checkout + // directories. + hits.push(relative(repoRoot, full).split(/[\\/]/).join("/")); + } + } + } + + walk(srcRoot); + return hits.sort(); + } + + it("grep-based discovery pins every `flags.coach` call site", () => { + const discovered = findCoachFlagSites(); + const allowlist = new Set(KNOWN_COACH_GATE_SITES); + const orphans = discovered.filter((path) => !allowlist.has(path)); + expect( + orphans, + [ + "Found `flags.coach` in file(s) not on the KNOWN_COACH_GATE_SITES", + "allowlist (src/lib/feature-flags/__tests__/coach-cascade.test.tsx).", + "Add a fixture entry (or a sibling invariant) for the new gate,", + "then add the path to the allowlist. Orphans:", + ...orphans.map((p) => ` - ${p}`), + ].join("\n"), + ).toEqual([]); + + // Sanity check the other direction — an allowlist entry that no + // longer references `flags.coach` is dead documentation and + // should be pruned in the same PR that removes the gate. + const discoveredSet = new Set(discovered); + const stale = KNOWN_COACH_GATE_SITES.filter( + (path) => !discoveredSet.has(path), + ); + expect( + stale, + [ + "KNOWN_COACH_GATE_SITES references file(s) that no longer use", + "`flags.coach`. Remove the stale allowlist entry:", + ...stale.map((p) => ` - ${p}`), + ].join("\n"), + ).toEqual([]); + }); + it("fixture stays in sync with the Coach gate call sites", () => { // Counts the Coach-bearing surfaces the fixture knows about. If a // contributor adds a new `useFeatureFlags()` + `flags.coach` gate @@ -210,7 +384,8 @@ describe("Coach disable cascade invariant", () => { // surfaces the fixture mounts directly; cross-cut gates on // sub-pages (target-card, /targets page) are owned by other // invariants (`targets-coach-mount.test.tsx`, - // `target-card.test.tsx`). + // `target-card.test.tsx`) and by the grep-based discovery test + // above. expect(COACH_SURFACES.length).toBe(6); }); }); diff --git a/src/lib/jobs/__tests__/geo-backfill.test.ts b/src/lib/jobs/__tests__/geo-backfill.test.ts index f34493fff..fa21d5be2 100644 --- a/src/lib/jobs/__tests__/geo-backfill.test.ts +++ b/src/lib/jobs/__tests__/geo-backfill.test.ts @@ -71,8 +71,8 @@ describe("runGeoBackfill", () => { expect(args?.take).toBe(GEO_BACKFILL_BATCH_CAP); }); - it("caps the batch at 5000 rows per pass", async () => { - expect(GEO_BACKFILL_BATCH_CAP).toBe(5000); + it("caps the batch at 500 rows per pass (v1.4.38 — tightened from 5000 to bound the worst-case ipwho.is fallback budget)", async () => { + expect(GEO_BACKFILL_BATCH_CAP).toBe(500); }); it("updates a row with location only when ASN resolver misses", async () => { diff --git a/src/lib/jobs/geo-backfill.ts b/src/lib/jobs/geo-backfill.ts index 2ab37decc..767e97145 100644 --- a/src/lib/jobs/geo-backfill.ts +++ b/src/lib/jobs/geo-backfill.ts @@ -18,9 +18,16 @@ * window typically belong to deleted users or have aged out of * the operational triage window where the `location` chip * matters. - * - Capped at 5 000 rows per pass. Limits the worst-case DB-write - * fan-out so a single backfill burst can't starve a live login - * spike. + * - Capped at 500 rows per pass. Each row spends up to 3 s on the + * `lookupIpLocation` online fallback (offline MMDB miss → ipwho.is + * timeout), so 500 rows × 3 s caps the worst-case pass at ~25 min + * and keeps the hourly cron from stacking. Earlier passes ran at + * 5 000 rows, which let a single backfill burst block for ~4 h and + * starve the next scheduled run. The trade-off: a tenant with a + * large null-`location` backlog now drains over multiple hourly + * passes instead of one mega-pass — fine because the helper is + * idempotent and the admin sign-in overview tolerates a stale + * Standort cell for the few hours convergence takes. * * Idempotent: a row that the resolver still cannot match (private IP * sneaking through CF egress, freshly-allocated range the GeoLite2 @@ -35,7 +42,7 @@ import type { PrismaClient } from "@/generated/prisma/client"; import { lookupIpAsn, lookupIpLocation } from "@/lib/geo"; -export const GEO_BACKFILL_BATCH_CAP = 5000; +export const GEO_BACKFILL_BATCH_CAP = 500; export const GEO_BACKFILL_WINDOW_DAYS = 30; /**