refactor(outcome): lift the write-outcome classifier out of the import panel

The rule that a write which wrote nothing must not read as a success was
scoped to the import panel. Provider syncs need the same rule, so the
classifier moves to src/lib/outcome/written-outcome.ts and the tick plus the
success colour move to one presentation component the whole tree renders
through. Adds the sync shape: a run reports what it imported and whether any
part of it failed, and the outcome is resolved from both.
This commit is contained in:
Marc-André Bombeck committed 2026-07-27 22:43:27 +02:00
1 parent 60c30e1e3a
commit 52288b7f77
9 files changed
+288 -157

No files matched your search

@@ -8,9 +8,9 @@
* the thing that has to be right is the sentence at the end of the run.
*
* The rules are the ones the settings import panel already follows, reused
* rather than restated: `classifyImportOutcome` decides the tone from the
* rather than restated: `classifyWrittenOutcome` decides the tone from the
* counts, so an import that wrote nothing can never carry the success tick,
* and `ImportOutcomeLine` owns the tick and the success colour. The skips are
* and `WrittenOutcomeLine` owns the tick and the success colour. The skips are
* grouped by reason with a count, because a file that trips one rule trips it
* on every row and a per-row list is the same sentence a hundred times.
*
@@ -18,11 +18,11 @@
* cards; the medication intake import is the third import surface and it now
* answers the same way.
*/
import { WrittenOutcomeLine } from "@/components/outcome/written-outcome-line";
import {
classifyImportOutcome,
type ImportOutcome,
} from "@/components/settings/import-panel/import-result-state";
import { ImportOutcomeLine } from "@/components/settings/import-panel/import-result-view";
classifyWrittenOutcome,
type WrittenOutcome,
} from "@/lib/outcome/written-outcome";
import { useTranslations } from "@/lib/i18n/context";
/** A skip group exactly as the import job reports it. */
@@ -132,7 +132,7 @@ export function IntakeImportResultView({
);
}
const outcome: ImportOutcome = classifyImportOutcome({
const outcome: WrittenOutcome = classifyWrittenOutcome({
written: result.imported,
skipped: result.skipped,
});
@@ -153,7 +153,7 @@ export function IntakeImportResultView({
return (
<div data-testid="intake-import-result" className="space-y-1.5">
<ImportOutcomeLine
<WrittenOutcomeLine
outcome={outcome}
message={message}
testId="intake-import-outcome"
+33
View File
@@ -0,0 +1,33 @@
"use client";
import { toast } from "sonner";
import type { WrittenOutcome } from "@/lib/outcome/written-outcome";
/**
* The toast twin of `WrittenOutcomeLine`: one mapping from a written outcome
* to the toast variant that reports it.
*
* `toast.success` lives here and is reached only through the `success` key, so
* a surface that reports a write cannot raise a green toast for a run that
* wrote nothing. `empty` uses the neutral variant rather than the success one
* — "nothing to write" is an honest non-event, not an achievement.
*/
export function toastWrittenOutcome(
outcome: WrittenOutcome,
message: string,
): void {
switch (outcome) {
case "success":
toast.success(message);
return;
case "partial":
toast.warning(message);
return;
case "failed":
toast.error(message);
return;
case "empty":
toast.info(message);
}
}
@@ -0,0 +1,57 @@
"use client";
import { AlertCircle, CheckCircle2, Info, TriangleAlert } from "lucide-react";
import type { LucideIcon } from "lucide-react";
import type { WrittenOutcome } from "@/lib/outcome/written-outcome";
/**
* The one mapping from a written outcome to how it looks.
*
* `CheckCircle2` and `text-success` appear on the `success` key and nowhere
* else in this file, so no future edit can attach a tick to a run that wrote
* nothing. Every import card and every provider-sync card renders its result
* line through here; `src/__tests__/success-affordance-guard.test.ts` holds
* that line.
*/
const OUTCOME_PRESENTATION: Record<
WrittenOutcome,
{ Icon: LucideIcon; className: string }
> = {
success: { Icon: CheckCircle2, className: "text-success" },
partial: { Icon: TriangleAlert, className: "text-warning" },
failed: { Icon: AlertCircle, className: "text-destructive" },
empty: { Icon: Info, className: "text-muted-foreground" },
};
/**
* The headline line of a write result. Sits inside the caller's existing
* `aria-live` region where there is one, so the outcome is announced as it
* lands; a `failed` or `partial` line carries `role="alert"` itself, because
* a result that did not write is the case a user must not scroll past.
*/
export function WrittenOutcomeLine({
outcome,
message,
testId,
}: {
outcome: WrittenOutcome;
message: string;
testId: string;
}) {
const { Icon, className } = OUTCOME_PRESENTATION[outcome];
return (
<p
data-testid={testId}
data-outcome={outcome}
role={outcome === "failed" || outcome === "partial" ? "alert" : "status"}
className="text-foreground flex items-start gap-2 text-xs"
>
<Icon
className={`${className} mt-0.5 h-3.5 w-3.5 shrink-0`}
aria-hidden="true"
/>
<span>{message}</span>
</p>
);
}
@@ -11,8 +11,8 @@ import { useTranslations } from "@/lib/i18n/context";
import { cn } from "@/lib/utils";
import { apiFetchRaw, apiGet } from "@/lib/api/api-fetch";
import { ImportCardShell } from "./import-card-shell";
import { classifyImportOutcome } from "./import-result-state";
import { ImportOutcomeLine } from "./import-result-view";
import { WrittenOutcomeLine } from "@/components/outcome/written-outcome-line";
import { classifyWrittenOutcome } from "@/lib/outcome/written-outcome";
/** Terminal states the status poll stops on. */
const TERMINAL_STATES: readonly string[] = ["done", "failed"];
@@ -239,8 +239,8 @@ export function AppleHealthImportCard() {
</div>
)}
{isDone && (
<ImportOutcomeLine
outcome={classifyImportOutcome({
<WrittenOutcomeLine
outcome={classifyWrittenOutcome({
written: status?.result?.totals?.rowsUpserted ?? 0,
// The clinical count is a documented, deliberate exclusion
// (electrocardiograms, lab documents), not a refused row — it
@@ -1,72 +0,0 @@
/**
* Import result classification — the one place that decides whether an
* import reads as a success.
*
* A per-row importer answers HTTP 200 for a file where every single row was
* refused; the transport succeeded, the import did not. Rendering that as a
* green tick is how a self-hoster's 1 597-reading sensor export could write
* nothing and still look finished. So the counts, not the status code, pick
* the state:
*
* - nothing accepted, nothing skipped → `empty` (the file carried no data)
* - nothing accepted, rows skipped → `failed` (never a success tone)
* - some accepted, some skipped → `partial` (a warning, not a tick)
* - all accepted → `success`
*
* Both import cards render from this, and `import-result-state.test.ts` pins
* the invariant that `success` is unreachable while `written === 0`.
*/
export type ImportOutcome = "empty" | "failed" | "partial" | "success";
export interface ImportCounts {
/** Rows that reached the database — inserted + updated. */
written: number;
/** Rows the importer refused, for any reason. */
skipped: number;
}
export function classifyImportOutcome({
written,
skipped,
}: ImportCounts): ImportOutcome {
if (written <= 0) return skipped > 0 ? "failed" : "empty";
return skipped > 0 ? "partial" : "success";
}
export interface SkipReasonGroup {
/** The machine-readable reason as the route reported it. */
reason: string;
count: number;
}
/** A row of the per-row envelope, narrowed to what the grouping needs. */
export interface ImportRowResult {
line: number;
status: string;
reason?: string;
}
const UNKNOWN_REASON = "unknown";
/**
* Collapse the per-row envelope into one entry per skip reason.
*
* A sensor export that trips one validation trips it on every row, so the
* raw list is the same sentence a few thousand times. Grouping turns "scroll
* fifty identical lines" into one fact with a count. Ordered by count
* descending, then by reason, so the same result always renders the same way.
*/
export function groupSkipReasons(
rows: readonly ImportRowResult[],
): SkipReasonGroup[] {
const counts = new Map<string, number>();
for (const row of rows) {
if (row.status !== "skipped") continue;
const reason = row.reason ?? UNKNOWN_REASON;
counts.set(reason, (counts.get(reason) ?? 0) + 1);
}
return [...counts.entries()]
.map(([reason, count]) => ({ reason, count }))
.sort((a, b) => b.count - a.count || a.reason.localeCompare(b.reason));
}
@@ -1,61 +1,12 @@
"use client";
import { AlertCircle, CheckCircle2, Info, TriangleAlert } from "lucide-react";
import type { LucideIcon } from "lucide-react";
import { WrittenOutcomeLine } from "@/components/outcome/written-outcome-line";
import { useTranslations } from "@/lib/i18n/context";
import {
classifyImportOutcome,
classifyWrittenOutcome,
groupSkipReasons,
type ImportOutcome,
type ImportRowResult,
} from "./import-result-state";
/**
* The one mapping from an import outcome to how it looks.
*
* `CheckCircle2` and `text-success` appear exactly once in the import panel,
* on the `success` key, so no future edit can attach a tick to a result that
* wrote nothing. `import-result-guard.test.ts` holds that line.
*/
const OUTCOME_PRESENTATION: Record<
ImportOutcome,
{ Icon: LucideIcon; className: string }
> = {
success: { Icon: CheckCircle2, className: "text-success" },
partial: { Icon: TriangleAlert, className: "text-warning" },
failed: { Icon: AlertCircle, className: "text-destructive" },
empty: { Icon: Info, className: "text-muted-foreground" },
};
/**
* The headline line of an import result. Sits inside each card's existing
* `aria-live="polite"` region, so the outcome is announced as it lands.
*/
export function ImportOutcomeLine({
outcome,
message,
testId,
}: {
outcome: ImportOutcome;
message: string;
testId: string;
}) {
const { Icon, className } = OUTCOME_PRESENTATION[outcome];
return (
<p
data-testid={testId}
data-outcome={outcome}
className="text-foreground flex items-start gap-2 text-xs"
>
<Icon
className={`${className} mt-0.5 h-3.5 w-3.5 shrink-0`}
aria-hidden="true"
/>
<span>{message}</span>
</p>
);
}
type WrittenRowResult,
} from "@/lib/outcome/written-outcome";
export interface CsvImportResult {
inserted: number;
@@ -63,7 +14,7 @@ export interface CsvImportResult {
skipped: number;
total: number;
dryRun: boolean;
rows: ImportRowResult[];
rows: WrittenRowResult[];
}
/**
@@ -118,7 +69,7 @@ export function CsvImportResultView({ result }: { result: CsvImportResult }) {
const labelFor = useSkipReasonLabel();
const written = result.inserted + result.updated;
const outcome = classifyImportOutcome({ written, skipped: result.skipped });
const outcome = classifyWrittenOutcome({ written, skipped: result.skipped });
const groups = groupSkipReasons(result.rows);
const skippedRows = result.rows.filter((row) => row.status === "skipped");
@@ -162,7 +113,7 @@ export function CsvImportResultView({ result }: { result: CsvImportResult }) {
return (
<div data-testid="import-csv-result" className="space-y-1.5">
<ImportOutcomeLine
<WrittenOutcomeLine
outcome={outcome}
message={message}
testId="import-csv-outcome"
@@ -12,8 +12,8 @@ import { apiFetchRaw } from "@/lib/api/api-fetch";
import { ImportCardShell } from "./import-card-shell";
import { MAX_PASTE_CHARS } from "./constants";
import { EXAMPLE_IMPORT, parseImportJson } from "./import-examples";
import { classifyImportOutcome } from "./import-result-state";
import { ImportOutcomeLine } from "./import-result-view";
import { WrittenOutcomeLine } from "@/components/outcome/written-outcome-line";
import { classifyWrittenOutcome } from "@/lib/outcome/written-outcome";
interface JsonImportResult {
measurements: number;
@@ -28,7 +28,7 @@ export function JsonImportCard() {
// concept count envelope, so a payload where every entry was refused used
// to render the success tick. The counts decide the tone.
const jsonOutcome = (r: JsonImportResult) =>
classifyImportOutcome({
classifyWrittenOutcome({
written: r.measurements + r.moodEntries,
skipped: r.skipped,
});
@@ -191,7 +191,7 @@ export function JsonImportCard() {
<div aria-live="polite" className="space-y-2">
{result && (
<ImportOutcomeLine
<WrittenOutcomeLine
outcome={jsonOutcome(result)}
message={jsonMessage(result)}
testId="import-json-result"
@@ -1,42 +1,45 @@
/**
* #640 — an import that wrote nothing must never read as a success.
* #640 — a write that wrote nothing must never read as a success.
*
* The reported failure was not the parser refusing rows; it was the card
* answering that refusal with a green tick, which is what turned a
* recoverable problem into a silent one. These pin the classifier that both
* import cards render from, and the structural guard that keeps the success
* affordance attached to the success outcome alone lives alongside in
* `import-result-guard.test.ts`.
* recoverable problem into a silent one. These pin the classifier every
* import card and every provider-sync card now renders from; the structural
* guard that keeps the success affordance attached to the success outcome
* alone lives in `src/__tests__/success-affordance-guard.test.ts`.
*/
import { describe, it, expect } from "vitest";
import {
classifyImportOutcome,
classifyWrittenOutcome,
groupSkipReasons,
} from "../import-result-state";
resolveSyncOutcome,
} from "../written-outcome";
describe("classifyImportOutcome", () => {
describe("classifyWrittenOutcome", () => {
it("calls the reported production result a failure", () => {
// The audit envelope from the report: 1 597 rows in, nothing written.
expect(classifyImportOutcome({ written: 0, skipped: 1597 })).toBe("failed");
expect(classifyWrittenOutcome({ written: 0, skipped: 1597 })).toBe(
"failed",
);
});
it("calls a file with no data rows empty, not a success", () => {
expect(classifyImportOutcome({ written: 0, skipped: 0 })).toBe("empty");
expect(classifyWrittenOutcome({ written: 0, skipped: 0 })).toBe("empty");
});
it("calls a mixed result partial", () => {
expect(classifyImportOutcome({ written: 3, skipped: 2 })).toBe("partial");
expect(classifyWrittenOutcome({ written: 3, skipped: 2 })).toBe("partial");
});
it("calls a clean result a success", () => {
expect(classifyImportOutcome({ written: 5, skipped: 0 })).toBe("success");
expect(classifyWrittenOutcome({ written: 5, skipped: 0 })).toBe("success");
});
it("never reaches success while nothing was written", () => {
for (let written = 0; written <= 4; written++) {
for (let skipped = 0; skipped <= 4; skipped++) {
const outcome = classifyImportOutcome({ written, skipped });
const outcome = classifyWrittenOutcome({ written, skipped });
if (written === 0) {
expect(outcome).not.toBe("success");
expect(outcome).not.toBe("partial");
@@ -50,6 +53,43 @@ describe("classifyImportOutcome", () => {
});
});
describe("resolveSyncOutcome", () => {
it("calls a run that wrote nothing and failed a failure, not an empty run", () => {
// The defect: every row refused by a constraint, count 0, HTTP 200.
expect(resolveSyncOutcome({ imported: 0, failed: true })).toEqual({
imported: 0,
failed: true,
outcome: "failed",
});
});
it("calls a run that found nothing new empty", () => {
expect(resolveSyncOutcome({ imported: 0, failed: false }).outcome).toBe(
"empty",
);
});
it("calls a run with a failed leg partial even when rows landed", () => {
expect(resolveSyncOutcome({ imported: 12, failed: true }).outcome).toBe(
"partial",
);
});
it("calls a clean run a success", () => {
expect(resolveSyncOutcome({ imported: 12, failed: false }).outcome).toBe(
"success",
);
});
it("never reaches success while the run reported a failure", () => {
for (const imported of [0, 1, 500]) {
expect(resolveSyncOutcome({ imported, failed: true }).outcome).not.toBe(
"success",
);
}
});
});
describe("groupSkipReasons", () => {
it("collapses a repeated reason to one entry with a count", () => {
const rows = Array.from({ length: 1597 }, (_, i) => ({
+122
View File
@@ -0,0 +1,122 @@
/**
* Written-outcome classification — the one place that decides whether a write
* reads as a success.
*
* The rule started in the import panel (`import-result-state.ts`, #640/#650)
* and is unchanged; only its reach is wider. A per-row importer answers HTTP
* 200 for a file where every single row was refused, and a provider sync
* answers HTTP 200 for a run where every reading failed a constraint. The
* transport succeeded, the write did not. Rendering either as a green tick is
* how a self-hoster's sensor export could write nothing and still look
* finished. So the counts, not the status code, pick the state:
*
* - nothing written, nothing refused → `empty` (there was nothing to write)
* - nothing written, rows refused → `failed` (never a success tone)
* - some written, some refused → `partial` (a warning, not a tick)
* - everything written → `success`
*
* `empty` is deliberately its own state rather than a failure: an import of a
* file with no data rows and a sync that finds nothing new are both honest
* non-events, and both would be a lie under a tick.
*
* Every surface that reports a write routes through this, and
* `success-affordance-guard.test.ts` holds the line that the tick and the
* success colour live only in the presentation module this feeds.
*/
export type WrittenOutcome = "empty" | "failed" | "partial" | "success";
export interface WriteCounts {
/** Rows that reached the database — inserted + updated. */
written: number;
/** Rows the write refused, for any reason. */
skipped: number;
}
export function classifyWrittenOutcome({
written,
skipped,
}: WriteCounts): WrittenOutcome {
if (written <= 0) return skipped > 0 ? "failed" : "empty";
return skipped > 0 ? "partial" : "success";
}
/**
* What a provider sync reports back to its route.
*
* Three providers (Fitbit, Google Health, WHOOP) already answered in this
* shape; the rest returned a bare count that narrowed their failures away —
* a Nightscout run whose every SGV row failed to write returned the same `0`
* as a run that found nothing new. The boolean is the honest floor: a sync
* knows a leg / collection / row did not land, but not always how many.
*/
export interface SyncWriteResult {
/** Rows that reached the database this run. */
imported: number;
/** True when any part of the run did not land — a row, a collection, a leg. */
failed: boolean;
}
/** The sync envelope a route answers with: the counts plus the resolved tone. */
export interface ResolvedSyncOutcome extends SyncWriteResult {
outcome: WrittenOutcome;
}
/**
* Resolve a sync's write result to the outcome its card renders.
*
* Server-side on purpose: the verdict is the server's to compute, so the web
* card and the native client cannot drift on how the same run reads. A failed
* leg counts as one refusal — enough to move `success` to `partial` and
* `empty` to `failed`, which is the whole point.
*/
export function resolveSyncOutcome({
imported,
failed,
}: SyncWriteResult): ResolvedSyncOutcome {
return {
imported,
failed,
outcome: classifyWrittenOutcome({
written: imported,
skipped: failed ? 1 : 0,
}),
};
}
export interface SkipReasonGroup {
/** The machine-readable reason as the route reported it. */
reason: string;
count: number;
}
/** A row of the per-row envelope, narrowed to what the grouping needs. */
export interface WrittenRowResult {
line: number;
status: string;
reason?: string;
}
const UNKNOWN_REASON = "unknown";
/**
* Collapse the per-row envelope into one entry per skip reason.
*
* A sensor export that trips one validation trips it on every row, so the
* raw list is the same sentence a few thousand times. Grouping turns "scroll
* fifty identical lines" into one fact with a count. Ordered by count
* descending, then by reason, so the same result always renders the same way.
*/
export function groupSkipReasons(
rows: readonly WrittenRowResult[],
): SkipReasonGroup[] {
const counts = new Map<string, number>();
for (const row of rows) {
if (row.status !== "skipped") continue;
const reason = row.reason ?? UNKNOWN_REASON;
counts.set(reason, (counts.get(reason) ?? 0) + 1);
}
return [...counts.entries()]
.map(([reason, count]) => ({ reason, count }))
.sort((a, b) => b.count - a.count || a.reason.localeCompare(b.reason));
}