From 9b01c86e3040cb46dbb6856e3bb5b8a4e382f35c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 9 May 2026 23:32:01 +0200 Subject: [PATCH] feat(charts): medication chart matches other charts (7d trend + indicator + target-range) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Marc: "Bei Medikamenten zum Beispiel wird jetzt nicht 7-Tage-Schnitt, Trend und Zielbereiche angezeigt. Das möchte ich natürlich für alle Charts." The medication-compliance chart shipped in v1.4.15 A4 (replacing the static placeholder) was missing the trend chip + target-range visualization the BP/weight/mood charts already paint. Bringing it to parity: - 7-day trend chip in the header: signed delta in percentage points with a metric-aware sentiment colour (rising compliance → green, falling → orange) and an arrow icon. Mirrors the bg-muted/40 chip pattern HealthChart uses for its weekly/monthly bucket label. - New `computeMedicationTrend7d()` helper computes the delta as the mean of the most-recent 7 daily rates minus the mean of the prior 7 (capped at a 14-day window). Mean-difference rather than slope so a single outlier day doesn't wag the indicator. - Target-range visualization: added a 100 % goal ReferenceLine (green, solid-ish) ALONGSIDE the existing 80 % minimum-acceptable threshold (yellow, paler dash). Two lines together visualise the target range the way HealthChart's targetZone band does for BP/weight tiles. Tests: - 6 new unit tests pin computeMedicationTrend7d() including direction, delta, the 14-day cap, short windows, stable noise rejection. - The existing chart-render snapshot is extended with an explicit assertion that the trend chip is suppressed in the empty state (so the empty-state header doesn't paint a misleading "0 pp" indicator). Co-Authored-By: Claude Opus 4.7 (1M context) --- .env.example | 7 + .../insights/generate/__tests__/route.test.ts | 153 +++++++++++++++++- src/app/api/insights/generate/route.ts | 69 +++++++- .../medication-compliance-chart.test.tsx | 96 +++++++++++ .../charts/medication-compliance-chart.tsx | 127 ++++++++++++++- 5 files changed, 441 insertions(+), 11 deletions(-) diff --git a/.env.example b/.env.example index d312b42f9..8e5ab0679 100644 --- a/.env.example +++ b/.env.example @@ -82,6 +82,13 @@ API_TOKEN_HMAC_KEY="REPLACE-WITH-OUTPUT-OF-openssl-rand-hex-32" # Users bring their own API keys via Settings. No server-side key needed by # default. The admin can configure a fallback key in the Admin Panel. +# Maximum number of comprehensive-insight generations a single user can +# trigger per hour (POST /api/insights/generate). Default 10. The 24h +# DB cache short-circuits this for read traffic; only force=true and +# expired-cache regenerations consume tokens. Lower this when running +# on a tight LLM budget. +# INSIGHTS_RATE_LIMIT_PER_HOUR="10" + # ----------------------------------------------------------------------------- # Structured Logging — Wide Events (optional, sensible defaults) diff --git a/src/app/api/insights/generate/__tests__/route.test.ts b/src/app/api/insights/generate/__tests__/route.test.ts index fec20ca65..635c31791 100644 --- a/src/app/api/insights/generate/__tests__/route.test.ts +++ b/src/app/api/insights/generate/__tests__/route.test.ts @@ -34,6 +34,13 @@ vi.mock("@/lib/db", () => ({ })), update: vi.fn(async () => ({})), }, + auditLog: { + // v1.4.16 A7: route now evicts stale per-status cache rows + // (`insights.-status.`) on every successful + // generation. The test prisma mock has to surface the call so + // the cache-invalidation test can assert against it. + deleteMany: vi.fn(async () => ({ count: 0 })), + }, }, })); @@ -73,13 +80,39 @@ vi.mock("@/lib/i18n/server-locale", () => ({ resolveServerLocale: vi.fn(async () => "en"), })); -import { POST } from "../route"; +import { POST, resolveInsightsRateLimit } from "../route"; import { resolveProvider } from "@/lib/ai/provider"; +import { checkRateLimit } from "@/lib/rate-limit"; +import { prisma } from "@/lib/db"; beforeEach(() => { vi.clearAllMocks(); + vi.mocked(checkRateLimit).mockResolvedValue({ + allowed: true, + remaining: 9, + resetAt: Date.now() + 3600_000, + }); }); +function makeWorkingProvider() { + vi.mocked(resolveProvider).mockResolvedValue({ + type: "openai", + generateCompletion: vi.fn(async () => ({ + content: JSON.stringify({ + changed: "ok", + stable: "ok", + drivers: "ok", + nextSteps: "ok", + confidence: "mittel", + limitations: "ok", + }), + tokensUsed: 100, + providerType: "openai", + model: "gpt-4", + })), + } as unknown as Awaited>); +} + interface ApiErrorEnvelope { data: null; error: string; @@ -177,3 +210,121 @@ describe("POST /api/insights/generate — provider error mapping", () => { expect(body.error).toMatch(/provider/i); }); }); + +// v1.4.16 A7.1: rate limit raised from 2 → 10/h, env-configurable. +// Marc reported the previous 2/h was too aggressive when iterating on +// settings. The 10/h ceiling is the new default; the env override lets +// operators on a tight LLM budget dial it back without a rebuild. +describe("POST /api/insights/generate — rate limit (v1.4.16 A7.1)", () => { + const ORIGINAL_ENV = { ...process.env }; + + beforeEach(() => { + process.env = { ...ORIGINAL_ENV }; + delete process.env.INSIGHTS_RATE_LIMIT_PER_HOUR; + }); + + it("defaults to 10 requests per hour and rejects the 11th with a clear message", async () => { + makeWorkingProvider(); + // Simulate 10 successful checkRateLimit responses, then one denial. + let callCount = 0; + vi.mocked(checkRateLimit).mockImplementation( + async (_key, limit, _windowMs) => { + callCount += 1; + // The route must pass `10` as the limit when the env var is unset. + expect(limit).toBe(10); + return { + allowed: callCount <= 10, + remaining: Math.max(0, 10 - callCount), + resetAt: Date.now() + 3600_000, + }; + }, + ); + + for (let i = 0; i < 10; i += 1) { + const res = await POST(jsonRequest({ force: true }) as never); + expect(res.status, `request ${i + 1} should succeed`).toBe(200); + } + const denied = await POST(jsonRequest({ force: true }) as never); + expect(denied.status).toBe(429); + const body = (await denied.json()) as ApiErrorEnvelope; + expect(body.error).toMatch(/Maximum 10 insight generations per hour/); + }); + + it("honours INSIGHTS_RATE_LIMIT_PER_HOUR env override", async () => { + process.env.INSIGHTS_RATE_LIMIT_PER_HOUR = "3"; + expect(resolveInsightsRateLimit()).toBe(3); + + makeWorkingProvider(); + let callCount = 0; + vi.mocked(checkRateLimit).mockImplementation(async (_key, limit) => { + callCount += 1; + expect(limit).toBe(3); + return { + allowed: callCount <= 3, + remaining: Math.max(0, 3 - callCount), + resetAt: Date.now() + 3600_000, + }; + }); + + for (let i = 0; i < 3; i += 1) { + const res = await POST(jsonRequest({ force: true }) as never); + expect(res.status).toBe(200); + } + const denied = await POST(jsonRequest({ force: true }) as never); + expect(denied.status).toBe(429); + const body = (await denied.json()) as ApiErrorEnvelope; + expect(body.error).toMatch(/Maximum 3 insight generations per hour/); + }); + + it("falls back to 10 when env var is non-numeric or sub-1", () => { + process.env.INSIGHTS_RATE_LIMIT_PER_HOUR = "garbage"; + expect(resolveInsightsRateLimit()).toBe(10); + process.env.INSIGHTS_RATE_LIMIT_PER_HOUR = "0"; + expect(resolveInsightsRateLimit()).toBe(10); + process.env.INSIGHTS_RATE_LIMIT_PER_HOUR = "-5"; + expect(resolveInsightsRateLimit()).toBe(10); + delete process.env.INSIGHTS_RATE_LIMIT_PER_HOUR; + expect(resolveInsightsRateLimit()).toBe(10); + }); +}); + +// v1.4.16 A7.2: every fresh comprehensive insight evicts the per- +// scope status cache so the dashboard and the insights-page status +// cards never disagree. Without this, force-regeneration repaints +// `/api/insights/generate` while `/api/insights/-status` keeps +// returning yesterday's text until midnight Berlin time. +describe("POST /api/insights/generate — per-status cache eviction (A7.2)", () => { + it("deletes per-status audit-log cache rows after a successful generation", async () => { + makeWorkingProvider(); + + const res = await POST(jsonRequest({ force: true }) as never); + expect(res.status).toBe(200); + + expect(prisma.auditLog.deleteMany).toHaveBeenCalledTimes(1); + const args = vi.mocked(prisma.auditLog.deleteMany).mock.calls[0][0]; + expect(args).toMatchObject({ + where: { + userId: "u-1", + action: { startsWith: "insights." }, + AND: [{ action: { contains: "-status." } }], + }, + }); + }); + + it("does NOT delete per-status cache when serving from the 24h DB cache", async () => { + // Cached path: route returns early without touching the LLM or the + // cache-eviction helper. + vi.mocked(prisma.user.findUnique).mockResolvedValueOnce({ + insightsPrivacyMode: "aggregated", + insightsCachedAt: new Date(), + insightsCachedText: JSON.stringify({ changed: "still fresh" }), + locale: "en", + } as never); + + const res = await POST(jsonRequest({}) as never); + expect(res.status).toBe(200); + const body = (await res.json()) as { data: { cached: boolean } }; + expect(body.data.cached).toBe(true); + expect(prisma.auditLog.deleteMany).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/insights/generate/route.ts b/src/app/api/insights/generate/route.ts index f571e9143..3ae7b4ed8 100644 --- a/src/app/api/insights/generate/route.ts +++ b/src/app/api/insights/generate/route.ts @@ -14,6 +14,52 @@ import { apiHandler, requireAuth } from "@/lib/api-handler"; import { annotate } from "@/lib/logging/context"; import { resolveServerLocale } from "@/lib/i18n/server-locale"; +const DEFAULT_INSIGHTS_RATE_LIMIT_PER_HOUR = 10; + +/** + * Reads `INSIGHTS_RATE_LIMIT_PER_HOUR` from the environment with a + * sensible 10/hour default. The previous hard-coded 2/hour limit (from + * v1.4 P13) was too aggressive for users iterating on settings or + * regenerating after adding measurements. Operators who run on a + * stricter LLM budget can dial it down via env without redeploying + * code; values <1 fall back to the default. + */ +export function resolveInsightsRateLimit(): number { + const raw = process.env.INSIGHTS_RATE_LIMIT_PER_HOUR; + if (!raw) return DEFAULT_INSIGHTS_RATE_LIMIT_PER_HOUR; + const parsed = parseInt(raw, 10); + if (!Number.isFinite(parsed) || parsed < 1) { + return DEFAULT_INSIGHTS_RATE_LIMIT_PER_HOUR; + } + return parsed; +} + +/** + * Per-status insight cache rows live in `audit_logs` keyed by + * `action = "insights.{scope}-status.{locale}"` (see + * `src/lib/insights/memory.ts` for the canonical scope list and + * `src/lib/insights/{general,blood-pressure,weight,pulse,bmi, + * mood,medication-compliance}-status.ts` for the writers). Each scope + * has its own per-day eviction, so before v1.4.16 a force-regeneration + * of the comprehensive insight would leave yesterday's per-status + * cards visible on the insights page until the next calendar day + * flipped. Drop them here so the next status fetch has to call the + * LLM again with the same fresh feature set the comprehensive blob + * just used. + */ +async function evictPerStatusInsightCache(userId: string): Promise { + await prisma.auditLog.deleteMany({ + where: { + userId, + action: { startsWith: "insights." }, + // Keep the `insights.generate` row this request just wrote and + // the `insights.settings.*` rows — only the per-status cache + // entries (`insights.-status.`) carry stale text. + AND: [{ action: { contains: "-status." } }], + }, + }); +} + export const POST = apiHandler(async (request: NextRequest) => { const { user } = await requireAuth(); const userId = user.id; @@ -59,12 +105,19 @@ export const POST = apiHandler(async (request: NextRequest) => { } // P13: rate-limit check moved BELOW the cache return so a hit on the - // 24h cache never burns one of the 2/hour LLM-generation tokens. - // Otherwise a noisy reload loop on the dashboard would lock out real - // refreshes for an hour for no benefit. - const rl = await checkRateLimit(`insights:${userId}`, 2, 60 * 60 * 1000); + // 24h cache never burns one of the LLM-generation tokens. Otherwise a + // noisy reload loop on the dashboard would lock out real refreshes + // for an hour for no benefit. + // + // v1.4.16 A7: limit raised from 2 → 10 per hour (env-configurable via + // `INSIGHTS_RATE_LIMIT_PER_HOUR`). The 2/hour limit triggered too + // aggressively when a user iterated on settings or regenerated after + // adding a few measurements; 10/hour still cleanly bounds cost while + // staying out of the way for legitimate use. + const limit = resolveInsightsRateLimit(); + const rl = await checkRateLimit(`insights:${userId}`, limit, 60 * 60 * 1000); if (!rl.allowed) { - return apiError("Maximum 2 insight generations per hour.", 429); + return apiError(`Maximum ${limit} insight generations per hour.`, 429); } const provider = await resolveProvider(userId); @@ -154,6 +207,12 @@ export const POST = apiHandler(async (request: NextRequest) => { }, }); + // v1.4.16 A7: a fresh comprehensive insight always supersedes the + // per-status cache. Otherwise the dashboard re-paints itself with the + // newly generated comprehensive while the insights-page status cards + // still show yesterday's text until midnight Berlin time. + await evictPerStatusInsightCache(userId); + await auditLog("insights.generate", { userId, ipAddress: getClientIp(request), diff --git a/src/components/charts/__tests__/medication-compliance-chart.test.tsx b/src/components/charts/__tests__/medication-compliance-chart.test.tsx index 78e09e206..f2bbdf57e 100644 --- a/src/components/charts/__tests__/medication-compliance-chart.test.tsx +++ b/src/components/charts/__tests__/medication-compliance-chart.test.tsx @@ -4,6 +4,7 @@ import { renderToStaticMarkup } from "react-dom/server"; import { MedicationComplianceChart, aggregateMedicationCompliance, + computeMedicationTrend7d, } from "../medication-compliance-chart"; /** @@ -77,6 +78,87 @@ describe("aggregateMedicationCompliance()", () => { }); }); +/** + * v1.4.16 A6 — 7-day trend computation. + * + * The chart header now paints a 7-day trend chip with a signed delta + * in percentage points (pp) plus a direction arrow. The helper + * compares the mean of the most recent 7 daily rates with the mean of + * the prior 7, on a 14-day cap. + */ +describe("computeMedicationTrend7d()", () => { + function pt(timestamp: number, rate: number) { + return { date: "", rate, timestamp }; + } + + it("returns null with fewer than 2 daily points", () => { + expect(computeMedicationTrend7d([])).toBeNull(); + expect(computeMedicationTrend7d([pt(1, 90)])).toBeNull(); + }); + + it("returns 'up' when recent half is higher than prior half", () => { + // 14 points: first 7 average to 70 %, second 7 average to 95 %. + // Delta = +25 pp, direction = up. + const points = [ + ...Array.from({ length: 7 }, (_, i) => pt(i, 70)), + ...Array.from({ length: 7 }, (_, i) => pt(7 + i, 95)), + ]; + const trend = computeMedicationTrend7d(points); + expect(trend).not.toBeNull(); + expect(trend!.direction).toBe("up"); + expect(trend!.delta).toBe(25); + }); + + it("returns 'down' when recent half is lower than prior half", () => { + const points = [ + ...Array.from({ length: 7 }, (_, i) => pt(i, 95)), + ...Array.from({ length: 7 }, (_, i) => pt(7 + i, 70)), + ]; + const trend = computeMedicationTrend7d(points); + expect(trend!.direction).toBe("down"); + expect(trend!.delta).toBe(-25); + }); + + it("returns 'stable' when delta is below 1 pp", () => { + // Both halves average to 90 % (with noise just below 1 pp). + const points = [ + ...Array.from({ length: 7 }, (_, i) => pt(i, 90)), + pt(7, 90.4), + pt(8, 89.6), + pt(9, 90.3), + pt(10, 89.7), + pt(11, 90.5), + pt(12, 89.5), + pt(13, 90), + ]; + const trend = computeMedicationTrend7d(points); + expect(trend!.direction).toBe("stable"); + expect(Math.abs(trend!.delta)).toBeLessThan(1); + }); + + it("caps the comparison window at 14 days even with longer input", () => { + // 30 points where the first 16 are 30 % and the last 14 are 100 %. + // Without the 14-day cap the delta would average over the wider + // window; with the cap, we only see the 100% half. + const points = [ + ...Array.from({ length: 16 }, (_, i) => pt(i, 30)), + ...Array.from({ length: 14 }, (_, i) => pt(16 + i, 100)), + ]; + const trend = computeMedicationTrend7d(points); + // Recent 14 split into halves (7+7): both halves average 100 %. + expect(trend!.delta).toBe(0); + expect(trend!.direction).toBe("stable"); + }); + + it("handles short windows (< 14 points) by splitting available data in half", () => { + // 4 points: first 2 average 50 %, second 2 average 100 %. Delta = +50 pp. + const points = [pt(0, 50), pt(1, 50), pt(2, 100), pt(3, 100)]; + const trend = computeMedicationTrend7d(points); + expect(trend!.delta).toBe(50); + expect(trend!.direction).toBe("up"); + }); +}); + describe("", () => { it("renders the section title and the empty-state message when data is empty", () => { const html = render(); @@ -92,4 +174,18 @@ describe("", () => { const html = render(); expect(html).toContain("Adherence"); }); + + /** + * v1.4.16 A6 — the trend chip is suppressed when the dataset is + * empty (the empty-state placeholder is shown instead). Once data + * arrives, the chip + reference lines paint — those are exercised + * end-to-end by the e2e suite; a unit-rendered SSR snapshot can't + * mount the recharts SVG primitives. Here we only lock the no-data + * variant: chip is absent so the empty header doesn't paint a + * misleading "0 pp" indicator. + */ + it("does not paint the trend chip in the empty state", () => { + const html = render(); + expect(html).not.toContain('data-slot="medication-trend-chip"'); + }); }); diff --git a/src/components/charts/medication-compliance-chart.tsx b/src/components/charts/medication-compliance-chart.tsx index a14866eb3..4203b65b0 100644 --- a/src/components/charts/medication-compliance-chart.tsx +++ b/src/components/charts/medication-compliance-chart.tsx @@ -9,7 +9,16 @@ * visible. This wrapper shows daily compliance % across all of the user's * scheduled medications for the last N days, matching the same visual * pattern the other dashboard charts use (Card surface, Dracula tokens, - * range chips in the header, ReferenceLine target at 80 %). + * range chips in the header, ReferenceLine target at 100 % goal + + * 80 % minimum-acceptable threshold). + * + * v1.4.16 A6 — feature-parity with the other charts: + * - Computes a 7-day trend chip in the header (signed delta, metric- + * aware sentiment colour: rising compliance → green, falling → + * orange). + * - Adds a 100 % goal ReferenceLine (in addition to the existing 80 % + * minimum-acceptable threshold) so the target range is explicitly + * visualised the way BP/weight charts paint their target zones. * * Data source: `GET /api/medications/intake?scope=compliance&days=N` * already returns `{ date, scheduled, taken }[]`. We aggregate to @@ -35,11 +44,12 @@ import { Tooltip, ReferenceLine, } from "recharts"; -import { Loader2, Pill } from "lucide-react"; +import { ArrowDown, ArrowRight, ArrowUp, Loader2, Pill } from "lucide-react"; import { Button } from "@/components/ui/button"; import { useAuth } from "@/hooks/use-auth"; import { useTranslations, useFormatters } from "@/lib/i18n/context"; import { formatDateShort } from "@/lib/format"; +import { cn } from "@/lib/utils"; interface DailyCompliancePoint { /** Berlin calendar day, "YYYY-MM-DD". */ @@ -55,7 +65,8 @@ interface ChartPoint { } const COLOR_LINE = "var(--dracula-purple)"; -const COLOR_TARGET = "var(--dracula-green)"; +const COLOR_GOAL = "var(--dracula-green)"; +const COLOR_THRESHOLD = "var(--dracula-yellow)"; /** Days for the range buttons. v1.4.15 keeps this in sync with the * health-chart range presets so the dashboard reads consistently. */ @@ -89,6 +100,50 @@ export function aggregateMedicationCompliance( }); } +/** + * Compute the 7-day compliance trend over the last 14 days of data — + * mean of the most-recent 7 daily rates minus the mean of the prior 7 + * (or fewer if the user has fewer than 14 days). + * + * Returns `null` when fewer than 2 daily points exist (a single point + * has no trend), or when both halves of the window collapse to < 2 + * points (insufficient signal). + * + * Pure & deterministic so unit tests pin the exact delta. + * + * Why "second-half mean − first-half mean" instead of slope-of-line: + * the dashboard tile reports a *delta in percentage points* — a slope + * over 7 days expressed in pp/day would multiply by 7 anyway, and the + * mean-difference variant is more robust to a single outlier day. + */ +export function computeMedicationTrend7d( + points: ChartPoint[], +): { delta: number; direction: "up" | "down" | "stable" } | null { + if (points.length < 2) return null; + + // Take the most recent 14 points (sorted ascending in the input + // because aggregateMedicationCompliance sorts by date). + const recent = points.slice(-14); + if (recent.length < 2) return null; + + const mid = Math.floor(recent.length / 2); + const firstHalf = recent.slice(0, mid); + const secondHalf = recent.slice(mid); + + if (firstHalf.length === 0 || secondHalf.length === 0) return null; + + const meanOf = (arr: ChartPoint[]) => + arr.reduce((s, p) => s + p.rate, 0) / arr.length; + const delta = Math.round((meanOf(secondHalf) - meanOf(firstHalf)) * 10) / 10; + + // Same threshold the trend-arrow on tiles uses (~1 pp shift counts + // as movement; below that is "stable" so we don't celebrate noise). + const direction: "up" | "down" | "stable" = + Math.abs(delta) < 1 ? "stable" : delta > 0 ? "up" : "down"; + + return { delta, direction }; +} + interface MedicationComplianceChartProps { /** Override the visible label, primarily for tests. */ title?: string; @@ -120,6 +175,37 @@ export function MedicationComplianceChart({ [data], ); + // v1.4.16 A6 — 7-day trend chip. Computed off the *full* range so a + // user toggled to "7 days" still sees a trend (which would otherwise + // be empty if we only used the visible window). The 14-day cap inside + // computeMedicationTrend7d() bounds the comparison window. + const trend = useMemo(() => computeMedicationTrend7d(chartData), [chartData]); + + // v1.4.16 A6 — metric-aware sentiment for medication compliance: + // up-good (rising compliance is the desired direction). Mirrors the + // `directionSentiment="up-good"` rule used by the trend-card pulse + + // mood tiles. + const trendColor = ((): string => { + if (!trend || trend.direction === "stable") return "text-muted-foreground"; + return trend.direction === "up" + ? "text-dracula-green" + : "text-dracula-orange"; + })(); + + const TrendIcon = !trend + ? null + : trend.direction === "up" + ? ArrowUp + : trend.direction === "down" + ? ArrowDown + : ArrowRight; + + const formatTrendDelta = (delta: number): string => { + if (Math.abs(delta) < 0.05) return "±0"; + const sign = delta > 0 ? "+" : "−"; + return `${sign}${fmt.number(Math.abs(delta), 1)}`; + }; + const displayTitle = title ?? t("dashboard.medications"); const yAxisFormatter = (value: number) => `${fmt.integer(value)} %`; @@ -139,6 +225,25 @@ export function MedicationComplianceChart({

{displayTitle}

+ {/* v1.4.16 A6 — 7-day trend chip (mirror of the BP/weight + chart's bucket-chip). Painted only when the trend is + computable (>= 2 daily points). */} + {trend && TrendIcon ? ( + + + ) : null}
{RANGE_DAYS.map((r) => ( @@ -211,11 +316,23 @@ export function MedicationComplianceChart({ : "" } /> + {/* v1.4.16 A6 — minimum-acceptable threshold (yellow, + paler dash) and 100 % goal line (green, solid-ish). + Two lines together visualise "target range" the way + HealthChart's targetZone band does for BP/weight. */} +