From f3025a8dd55e7c3e3e2cfd9e07899679a4fdc1cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sun, 10 May 2026 03:10:09 +0200 Subject: [PATCH] refactor(v1.4.16): apply simplify-review safe suggestions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7 of 8 apply-yes simplify findings landed: F2 — `generateInsight()` confidenceContext config-bag dropped. No production caller passed the resolver; the wrapper now derives inputs from the parsed payload directly (n from metricSource.n, recencyDays =0, deviationStdRatio=null). Two confidence-wrapper tests rewritten to test the parsed-payload path. v1.4.17 will re-introduce a resolver injection when the feedback ratchet ships a real second caller. F3 — `aiRecommendationRationaleSchema.referenceId` speculative optional field removed. No consumer reads it; rec-level referenceId is the canonical citation pointer. F4 — `applyLastWorkingCache` doc-comment trimmed: dropped the "NEW array" defensive note (no caller compares the input ref). F5 — `` re-uses the canonical `RecommendationFeedbackRequest` types from the validation schema instead of hand-maintaining the same enum vocabulary. Removes a silent-drift risk if v1.4.17 adds another time-range value. F6 — `formatRelativeTime` doc-comment trimmed: dropped the stale `Intl.RelativeTimeFormat` polyfill claim (baseline-supported since 2018) — kept the genuine WHY (i18n consistency). F7 — `` 26-line slot-architecture header trimmed to a 4-line WHY. The slots being narrated as "future" were filled by B5d / B5e in the same file. F9 — `pickProviderType` doc-comment trimmed: removed the mis-anchored PROMPT_VERSION snapshot paragraph (the function only picks providerType — promptVersion is read elsewhere). F10 — `legacy-payload.ts` `UnknownRec` interface removed; cast inlined as `(rec as { rationale?: unknown })`. F12 — `` 20-line band-policy header trimmed to a 3-line WHY. The bandFor() helper + colour-by-band record below are the documentation now. (F1 + F8 deferred — both flagged "no" in simplify review and need Marc's call.) Co-Authored-By: Claude Opus 4.7 (1M context) --- src/components/insights/confidence-meter.tsx | 22 +---- .../insights/insights-page-hero.tsx | 9 +-- .../insights/recommendation-card.tsx | 28 +------ .../insights/recommendation-feedback.tsx | 16 ++-- .../ai/__tests__/confidence-wrapper.test.ts | 73 +++++++++-------- src/lib/ai/feedback-attribution.ts | 8 +- src/lib/ai/generate-insight.ts | 80 +++++-------------- src/lib/ai/legacy-payload.ts | 6 +- src/lib/ai/provider-runner.ts | 4 +- src/lib/ai/schema.ts | 7 -- 10 files changed, 78 insertions(+), 175 deletions(-) diff --git a/src/components/insights/confidence-meter.tsx b/src/components/insights/confidence-meter.tsx index b4b16d994..96ecdd181 100644 --- a/src/components/insights/confidence-meter.tsx +++ b/src/components/insights/confidence-meter.tsx @@ -1,25 +1,9 @@ import { useTranslations } from "@/lib/i18n/context"; /** - * v1.4.16 phase B5d — ``. - * - * Renders a deterministic 0..100 confidence score (server-computed by - * `computeConfidence()` and overridden in `generateInsight()`) as - * either a 5-bar meter (default) or an SVG ring. - * - * Color bands match research §2.A "three-band visual" extended to 4 - * bands so the under-50 region splits into "low" + "draft": - * - * - >=80: green ("high"), 5 bars / full ring - * - 50..79: yellow ("medium"), 3-4 bars - * - 25..49: orange ("low"), 2 bars / partial ring + caption - * - <25: "draft" pill REPLACES the meter — Apple's - * "quiet-when-unsure" affordance per research §2.A. The rec is - * drafted but the model isn't asserting it. - * - * aria-label always announces the numeric score so a screen reader - * isn't blind to the visual band distinction. Mobile-friendly: the - * bars variant fits in 56 px wide; the ring variant in 28×28. + * Renders a 0..100 confidence score as a 5-bar meter (default) or SVG + * ring. Sub-25 replaces the meter with a "draft" pill ("quiet-when- + * unsure"). aria-label always carries the numeric value. */ export type ConfidenceBand = "high" | "medium" | "low" | "draft"; diff --git a/src/components/insights/insights-page-hero.tsx b/src/components/insights/insights-page-hero.tsx index 9516ff2cb..c6155a2ee 100644 --- a/src/components/insights/insights-page-hero.tsx +++ b/src/components/insights/insights-page-hero.tsx @@ -41,13 +41,8 @@ interface InsightsPageHeroProps { } /** - * Format a relative time using the i18n translation keys. Avoids the - * `Intl.RelativeTimeFormat` polyfill cost on the client AND keeps every - * surface that renders relative times locked to the project's - * established translation vocabulary. - * - * Buckets: <1min → just now; <60min → N min ago; <24h → N h ago; - * else N days ago. + * Bucketed relative-time using i18n translation keys so every surface + * shares vocabulary. Buckets: <1m, <1h, <24h, else days. */ function formatRelativeTime( iso: string, diff --git a/src/components/insights/recommendation-card.tsx b/src/components/insights/recommendation-card.tsx index 373bf4810..ebdccc34c 100644 --- a/src/components/insights/recommendation-card.tsx +++ b/src/components/insights/recommendation-card.tsx @@ -20,30 +20,10 @@ import { import { ConfidenceMeter } from "./confidence-meter"; /** - * v1.4.16 phase B5c — Oura-style RecommendationCard. - * - * Each recommendation lives in its own collapsible card. The - * collapsed row shows: severity badge + rec text + named slot for - * the future confidence-ring (B5d) + chevron toggle. - * - * Expanding reveals: - * - 3-row rationale card (Window / Compared to / Deviation) - * - mini-chart pinned to the rationale's data window via - * HealthChart's new `mini` + `windowOverride` props - * - citation footnote (B5a) when `referenceId` resolves - * - named slot for the future feedback-thumbs (B5e) - * - * Default state: collapsed. Chevron toggles `aria-expanded`. Plain- - * string recs (legacy InsightResult shape) and recs without - * `rationale` render as a non-expandable list item — there's nothing - * to expand to. The legacy-payload CTA at the parent advisor card - * level surfaces the regenerate prompt for those. - * - * Layout slots are deliberately named so B5d (confidence) and B5e - * (feedback) can plug in without touching this file: - * - * - data-slot="rec-confidence-slot" → B5d ConfidenceRing - * - data-slot="rec-feedback-slot" → B5e RecommendationFeedback + * Collapsible recommendation card with severity badge, confidence + * meter, and an expandable rationale block (window / compared-to / + * deviation) + mini-chart. Plain-string and rationale-less recs + * render as a non-expandable row. */ const SEVERITY_BADGE_STYLES: Record = { diff --git a/src/components/insights/recommendation-feedback.tsx b/src/components/insights/recommendation-feedback.tsx index 4a827c5e5..43a6dc4f4 100644 --- a/src/components/insights/recommendation-feedback.tsx +++ b/src/components/insights/recommendation-feedback.tsx @@ -31,18 +31,18 @@ import { useMutation, useQueryClient } from "@tanstack/react-query"; import { ThumbsUp, ThumbsDown, Loader2, Check } from "lucide-react"; import { useTranslations } from "@/lib/i18n/context"; import { useAuth } from "@/hooks/use-auth"; +import type { RecommendationFeedbackRequest } from "@/lib/validations/recommendation-feedback"; +// v1.4.16 phase D reconcile (simplify F5) — re-export the canonical +// types from the validation schema instead of hand-maintaining the +// same enum vocabulary. Drift between the two used to be a real risk +// (a future "last180days" added to the schema would silently break +// the component without a compile error). export type RecommendationFeedbackSeverity = - | "info" - | "suggestion" - | "important" - | "urgent"; + RecommendationFeedbackRequest["recommendationSeverity"]; export type RecommendationFeedbackTimeRange = - | "last7days" - | "last30days" - | "last90days" - | "allTime"; + RecommendationFeedbackRequest["metricSourceTimeRange"]; export interface RecommendationFeedbackProps { recId: string; diff --git a/src/lib/ai/__tests__/confidence-wrapper.test.ts b/src/lib/ai/__tests__/confidence-wrapper.test.ts index 3cc65c59d..aebdf124f 100644 --- a/src/lib/ai/__tests__/confidence-wrapper.test.ts +++ b/src/lib/ai/__tests__/confidence-wrapper.test.ts @@ -1,7 +1,6 @@ -import { describe, it, expect, vi } from "vitest"; +import { describe, it, expect } from "vitest"; import { generateInsight } from "../generate-insight"; import { MockAIProvider } from "../mock-client"; -import type { ConfidenceInputs } from "../confidence"; /** * v1.4.16 phase B5d — wrapper post-validation override. @@ -91,25 +90,34 @@ describe("generateInsight() — confidence override", () => { expect(outcome.parsed.recommendations[0].confidence).toBe(66); }); - it("uses the resolver-supplied ConfidenceInputs when provided", async () => { + it("derives n from metricSource.n on the parsed payload (no caller injection)", async () => { + // v1.4.16 phase D reconcile (simplify F2) — the wrapper used to + // accept an optional `confidenceContext` resolver, but no production + // caller ever passed one. Inputs are derived from the payload alone + // today; v1.4.17's feedback ratchet will re-introduce a resolver + // injection when a real second caller exists. + const fourteenN = { + ...validResponseWithModelConfidence, + recommendations: [ + { + ...validResponseWithModelConfidence.recommendations[0], + metricSource: { + ...validResponseWithModelConfidence.recommendations[0] + .metricSource, + n: 14, + }, + }, + ], + }; const provider = new MockAIProvider({ - responses: JSON.stringify(validResponseWithModelConfidence), + responses: JSON.stringify(fourteenN), }); - const resolver = vi.fn( - (): ConfidenceInputs => ({ - n: 14, - recencyDays: 1, - deviationStdRatio: 2, - }), - ); - const outcome = await generateInsight( - provider, - { systemPrompt: "s", userPrompt: "u" }, - { confidenceContext: resolver }, - ); - expect(resolver).toHaveBeenCalledTimes(1); - // n=14 fresh strong signal → ~80 - expect(outcome.parsed.recommendations[0].confidence).toBe(80); + const outcome = await generateInsight(provider, { + systemPrompt: "s", + userPrompt: "u", + }); + // n=14, recencyDays=0, ratio=null → 10 + 10*log10(14) + 30 + 15 ≈ 66.46 + expect(outcome.parsed.recommendations[0].confidence).toBe(66); }); it("hard-caps confidence to <=15 when n<3 (small-data shield)", async () => { @@ -160,17 +168,19 @@ describe("generateInsight() — confidence override", () => { expect(outcome.parsed.recommendations[0].confidence).toBe(10); }); - it("calls the resolver per-recommendation with the rec passed in", async () => { + it("overrides confidence on every recommendation in payload order", async () => { const twoRecs = { ...validResponseWithModelConfidence, recommendations: [ { ...validResponseWithModelConfidence.recommendations[0], id: "rec-a", + confidence: 99, }, { ...validResponseWithModelConfidence.recommendations[0], id: "rec-b", + confidence: 1, metricSource: { ...validResponseWithModelConfidence.recommendations[0] .metricSource, @@ -182,19 +192,14 @@ describe("generateInsight() — confidence override", () => { const provider = new MockAIProvider({ responses: JSON.stringify(twoRecs), }); - const seenIds: string[] = []; - const resolver = (rec: { id: string }) => { - seenIds.push(rec.id); - return { n: 14, recencyDays: 1, deviationStdRatio: 2 }; - }; - const outcome = await generateInsight( - provider, - { systemPrompt: "s", userPrompt: "u" }, - { confidenceContext: resolver }, - ); - expect(seenIds).toEqual(["rec-a", "rec-b"]); - expect(outcome.parsed.recommendations.every((r) => r.confidence === 80)).toBe( - true, - ); + const outcome = await generateInsight(provider, { + systemPrompt: "s", + userPrompt: "u", + }); + // Model claimed (99, 1); both must be discarded and replaced with + // the deterministic value (n=12 → 66, n=5 → 62 from the saturating + // formula 10 + 10*log10(5) + 30 + 15 ≈ 61.99). + expect(outcome.parsed.recommendations[0].confidence).toBe(66); + expect(outcome.parsed.recommendations[1].confidence).toBe(62); }); }); diff --git a/src/lib/ai/feedback-attribution.ts b/src/lib/ai/feedback-attribution.ts index 74c856c57..491054d11 100644 --- a/src/lib/ai/feedback-attribution.ts +++ b/src/lib/ai/feedback-attribution.ts @@ -46,12 +46,8 @@ function parseAuditDetails(raw: string | null): AuditDetailsShape { } /** - * Pure helper exported for unit tests so we can pin the priority order - * (chainProviderType > providerType > "unknown") without hitting the - * DB. The current PROMPT_VERSION constant is always returned because - * the generator itself doesn't yet stash a per-payload prompt version - * (research §3.B notes this is a v1.4.17 ratchet item; reading the - * constant snapshots whatever's deployed when the rating fires). + * Pick provider attribution from a parsed audit-row details blob. + * Priority: chainProviderType > providerType > "unknown". */ export function pickProviderType(details: AuditDetailsShape): string { if (typeof details.chainProviderType === "string") { diff --git a/src/lib/ai/generate-insight.ts b/src/lib/ai/generate-insight.ts index 8a24cf776..0880b2e97 100644 --- a/src/lib/ai/generate-insight.ts +++ b/src/lib/ai/generate-insight.ts @@ -6,10 +6,9 @@ import { findUncitedRecommendations, InsightSchemaError, type AIInsightResponse, - type AIRecommendation, } from "./schema"; import { computeCitationCoverage } from "./citation-coverage"; -import { computeConfidence, type ConfidenceInputs } from "./confidence"; +import { computeConfidence } from "./confidence"; import { annotate } from "@/lib/logging/context"; /** @@ -174,55 +173,30 @@ function annotateCitationCoverage(parsed: AIInsightResponse): void { }); } -/** - * v1.4.16 phase B5d — confidence resolver. - * - * The wrapper does NOT have direct access to the user's bucket-series - * data, so the caller can pass an optional resolver that maps each - * recommendation to its `ConfidenceInputs`. When omitted, the wrapper - * falls back to inputs derivable from the parsed payload alone: - * - * - n: `metricSource.n` (or 0 when missing — triggers the n<3 hard - * cap so a recommendation cited from no quantified data is never - * dressed up as confident). - * - recencyDays: 0. Defensive default — assumes the cited window - * ends "now" because we have no fresher source. The route layer - * can supply a real value via the resolver. - * - deviationStdRatio: null. Yields the neutral 15-point signal - * contribution so a wrapper-only path never over-claims. - * - * The resolver fires once per recommendation in payload order so the - * caller can side-effect (e.g. log per-rec inputs) deterministically. - */ -export type ConfidenceContextResolver = ( - rec: AIRecommendation, -) => ConfidenceInputs; - -const DEFAULT_CONFIDENCE_RESOLVER: ConfidenceContextResolver = (rec) => ({ - n: rec.metricSource.n ?? 0, - recencyDays: 0, - deviationStdRatio: null, -}); - /** * Apply the deterministic confidence override to every recommendation. - * The model's value (if any) is REPLACED. Returns a Wide-Event - * payload with per-rec model-vs-computed pairs so admin observability - * can chart drift between what the LLM thinks and what the formula - * produces. + * The model's value (if any) is REPLACED. Inputs are derived from the + * parsed payload alone (n from `metricSource.n`, recencyDays=0, + * deviationStdRatio=null) — the route layer doesn't pass bucket-series + * data through here today. v1.4.17's feedback ratchet will need a + * resolver injection; re-introduce one when the second caller exists. + * + * Emits a Wide-Event with per-rec model-vs-computed pairs so admin + * observability can chart drift between what the LLM thinks and what + * the formula produces. */ -function applyConfidenceOverride( - parsed: AIInsightResponse, - resolver: ConfidenceContextResolver, -): void { +function applyConfidenceOverride(parsed: AIInsightResponse): void { const overrides: Array<{ id: string; model: number | null; computed: number; }> = []; for (const rec of parsed.recommendations) { - const inputs = resolver(rec); - const computed = computeConfidence(inputs); + const computed = computeConfidence({ + n: rec.metricSource.n ?? 0, + recencyDays: 0, + deviationStdRatio: null, + }); const modelValue = typeof rec.confidence === "number" ? rec.confidence : null; rec.confidence = computed; @@ -239,21 +213,6 @@ function applyConfidenceOverride( } } -export interface GenerateInsightOptions { - /** - * v1.4.16 phase B5d — per-recommendation `ConfidenceInputs` - * resolver. When supplied, the wrapper uses these inputs to - * `computeConfidence()` and OVERWRITES `rec.confidence`. When - * omitted, the wrapper falls back to a parsed-payload-only default - * (n from `metricSource.n`, recencyDays=0, ratio=null). - * - * The override happens regardless — calibrated probabilities are - * not a small-LLM strength and the deterministic path keeps the - * v1.4.17 feedback ratchet reproducible. - */ - confidenceContext?: ConfidenceContextResolver; -} - /** * Run a schema-enforced insight generation. Up to 2 provider calls. * Throws `InsightSchemaError` (httpStatus 422) if both attempts fail @@ -268,14 +227,11 @@ export interface GenerateInsightOptions { export async function generateInsight( provider: AIProvider, params: CompletionParams, - options: GenerateInsightOptions = {}, ): Promise { - const resolver = - options.confidenceContext ?? DEFAULT_CONFIDENCE_RESOLVER; const first = await tryOnce(provider, params); if (first.ok && first.parsed) { annotateCitationCoverage(first.parsed); - applyConfidenceOverride(first.parsed, resolver); + applyConfidenceOverride(first.parsed); return { parsed: first.parsed, raw: first.raw, @@ -296,7 +252,7 @@ export async function generateInsight( const second = await tryOnce(provider, retryParams); if (second.ok && second.parsed) { annotateCitationCoverage(second.parsed); - applyConfidenceOverride(second.parsed, resolver); + applyConfidenceOverride(second.parsed); return { parsed: second.parsed, raw: second.raw, diff --git a/src/lib/ai/legacy-payload.ts b/src/lib/ai/legacy-payload.ts index 9174bbd76..875f0c6bd 100644 --- a/src/lib/ai/legacy-payload.ts +++ b/src/lib/ai/legacy-payload.ts @@ -15,10 +15,6 @@ * Both predate B5c rationale; both flag. */ -interface UnknownRec { - rationale?: unknown; -} - /** * Returns true when the payload is non-empty and at least one * recommendation lacks a rationale object (or is a plain string from @@ -42,7 +38,7 @@ export function isLegacyInsightPayload(payload: unknown): boolean { if (typeof rec === "string") return true; // Structured rec: rationale must be a populated object. if (rec === null || typeof rec !== "object") return true; - const rationale = (rec as UnknownRec).rationale; + const rationale = (rec as { rationale?: unknown }).rationale; if ( rationale === undefined || rationale === null || diff --git a/src/lib/ai/provider-runner.ts b/src/lib/ai/provider-runner.ts index 2b8a189db..83f049131 100644 --- a/src/lib/ai/provider-runner.ts +++ b/src/lib/ai/provider-runner.ts @@ -192,9 +192,7 @@ export class AllProvidersFailedError extends Error { /** * Reorder the input chain so the cached "last working" provider is - * tried first. Returns a NEW array; original is not mutated. When the - * cache is cold (or the cached provider is not in the input list), the - * original order is preserved. + * tried first. Cold cache or unknown provider → original order. */ function applyLastWorkingCache( userId: string, diff --git a/src/lib/ai/schema.ts b/src/lib/ai/schema.ts index edfe06f42..452aae2dd 100644 --- a/src/lib/ai/schema.ts +++ b/src/lib/ai/schema.ts @@ -116,13 +116,6 @@ export const aiRecommendationRationaleSchema = z.object({ * specific ("+5 bpm above baseline over 7 of 7 days"). */ deviation: z.string().min(1, "rationale.deviation required"), - /** - * Optional pointer into the curated medical-reference bundle. The - * rec-level `referenceId` is the canonical citation source; this - * field stays optional so a future provider that wants to attach a - * reference at rationale-grain can do so without a schema bump. - */ - referenceId: z.string().optional(), }); export type AIRecommendationRationale = z.infer<