mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
refactor(v1.4.16): apply simplify-review safe suggestions
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 — `<RecommendationFeedback>` 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 — `<RecommendationCard>` 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 — `<ConfidenceMeter>` 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) <noreply@anthropic.com>
This commit is contained in:
1 parent
56614392be
commit
f3025a8dd5
10 files changed
+78
-175
No files matched your search
@@ -1,25 +1,9 @@
|
||||
import { useTranslations } from "@/lib/i18n/context";
|
||||
|
||||
/**
|
||||
* v1.4.16 phase B5d — `<ConfidenceMeter>`.
|
||||
*
|
||||
* 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";
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, string> = {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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") {
|
||||
|
||||
@@ -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<GenerateInsightOutcome> {
|
||||
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,
|
||||
|
||||
@@ -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 ||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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<
|
||||
|
||||
Reference in new issue
Block a user