mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-10 05:12:14 +00:00
fix(dashboard): slim/thick merge falls through on empty slim resolve
The v1.4.39.2 dashboard split fired two parallel useAnalyticsQuery
calls and merged them with `slim?.summaries ?? thick?.summaries`.
JavaScript treats an empty `{}` as truthy, so a zero-data slim
resolve short-circuited the thick fallback even when thick carried
the full per-type payload — the tile strip painted blank and the
chart row never mounted. Extract the merge into a pure
`mergeSlimAndThickAnalytics` helper that uses object emptiness as
the discriminator: populated slim still wins on overlapping fields
(progressive-paint contract preserved), empty slim falls through to
thick, both empty yields an empty record so the dashboard's data-
floor gates render the appropriate empty state.
Eight new behaviour-level unit tests cover the empty-slim, slim-
only, thick-only, both-empty, and `lastSeenByType` precedence cases.
The earlier source-text snapshot test in
`dashboard-analytics-split.test.ts` did not detect the empty-object
short-circuit; the pure-helper extraction is what makes the
regression directly testable.
This commit is contained in:
1 parent
85d3c9a531
commit
730028590b
3 files changed
+279
-21
No files matched your search
+24
-21
@@ -66,6 +66,7 @@ import { useTranslations, useFormatters } from "@/lib/i18n/context";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { useAnalyticsQuery } from "@/lib/queries/use-analytics-query";
|
||||
import type { DataSummary } from "@/lib/analytics/trends";
|
||||
import { mergeSlimAndThickAnalytics } from "@/lib/analytics/merge-slim-thick";
|
||||
import { getBpTargets } from "@/lib/analytics/bp-targets";
|
||||
import {
|
||||
buildTrafficLightBands,
|
||||
@@ -245,28 +246,30 @@ export default function DashboardPage() {
|
||||
const analyticsSlimQuery = useAnalyticsQuery({ slice: "summaries" });
|
||||
const analyticsThickQuery = useAnalyticsQuery();
|
||||
const data = useMemo<AnalyticsData | undefined>(() => {
|
||||
const slim = analyticsSlimQuery.data;
|
||||
const thick = analyticsThickQuery.data;
|
||||
if (!slim && !thick) return undefined;
|
||||
// Slim wins on overlapping fields when both have resolved — slim
|
||||
// and thick compute `summaries` / `lastSeenByType` from the same
|
||||
// SQL helpers so the values agree, and reading from whichever
|
||||
// resolves first keeps the tile strip painting as soon as either
|
||||
// query lands.
|
||||
const summaries =
|
||||
slim?.summaries ?? thick?.summaries ?? ({} as Record<string, DataSummary>);
|
||||
const lastSeenByType =
|
||||
slim?.lastSeenByType ?? thick?.lastSeenByType;
|
||||
// v1.4.39.3 — the merge moved to `mergeSlimAndThickAnalytics` so
|
||||
// the empty-slim-vs-populated-thick edge has direct unit
|
||||
// coverage. Pre-fix the inline `slim?.summaries ?? thick?.summaries`
|
||||
// short-circuited on a truthy-but-empty `{}` from the slim slice
|
||||
// and blanked the tile strip even when thick carried the full
|
||||
// payload — the regression Marc's v1.4.39.3 e2e CI flagged across
|
||||
// eight dashboard / chart specs. The helper falls back to thick
|
||||
// when slim resolves with no content and otherwise keeps the
|
||||
// v1.4.39.2 slim-wins-first progressive-paint contract.
|
||||
const merged = mergeSlimAndThickAnalytics(
|
||||
analyticsSlimQuery.data,
|
||||
analyticsThickQuery.data,
|
||||
);
|
||||
if (!merged) return undefined;
|
||||
return {
|
||||
summaries,
|
||||
lastSeenByType,
|
||||
bpInTargetPct: (thick?.bpInTargetPct ?? null) as number | null,
|
||||
bpInTargetPct7d: thick?.bpInTargetPct7d ?? null,
|
||||
bpInTargetPct30d: thick?.bpInTargetPct30d ?? null,
|
||||
bpInTargetPctAllTime: thick?.bpInTargetPctAllTime ?? null,
|
||||
bpInTargetPctPriorMonth: thick?.bpInTargetPctPriorMonth ?? null,
|
||||
bpInTargetPctPriorYear: thick?.bpInTargetPctPriorYear ?? null,
|
||||
glucoseByContext: thick?.glucoseByContext as
|
||||
summaries: merged.summaries,
|
||||
lastSeenByType: merged.lastSeenByType,
|
||||
bpInTargetPct: merged.bpInTargetPct,
|
||||
bpInTargetPct7d: merged.bpInTargetPct7d,
|
||||
bpInTargetPct30d: merged.bpInTargetPct30d,
|
||||
bpInTargetPctAllTime: merged.bpInTargetPctAllTime,
|
||||
bpInTargetPctPriorMonth: merged.bpInTargetPctPriorMonth,
|
||||
bpInTargetPctPriorYear: merged.bpInTargetPctPriorYear,
|
||||
glucoseByContext: merged.glucoseByContext as
|
||||
| Record<string, DataSummaryType>
|
||||
| undefined,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,141 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
import { mergeSlimAndThickAnalytics } from "@/lib/analytics/merge-slim-thick";
|
||||
import type { DataSummary } from "@/lib/analytics/trends";
|
||||
|
||||
/**
|
||||
* v1.4.39.3 — dashboard slim/thick merge robustness.
|
||||
*
|
||||
* v1.4.39.2 split the dashboard's analytics consumption into a slim
|
||||
* `?slice=summaries` query and the thick default query. The original
|
||||
* inline merge used `slim?.summaries ?? thick?.summaries` which
|
||||
* short-circuited on a truthy-but-empty `{}` from the slim slice and
|
||||
* blanked the tile strip even when thick carried the full payload —
|
||||
* the regression Marc's v1.4.39.3 e2e CI flagged across eight
|
||||
* dashboard / chart specs (chart-overlay-controls, dashboard,
|
||||
* measurement-flow, charts-mobile). The pure helper extracted in
|
||||
* v1.4.39.3 uses object emptiness as the discriminator so a populated
|
||||
* thick payload survives an empty slim resolve.
|
||||
*/
|
||||
|
||||
const stubSummary = (): DataSummary => ({
|
||||
count: 30,
|
||||
latest: 78.5,
|
||||
min: 76,
|
||||
max: 80,
|
||||
mean: 78,
|
||||
avg7: 78.2,
|
||||
avg30: 77.9,
|
||||
slope7: null,
|
||||
slope30: { slope: -0.05, direction: "down", confidence: 0.5 },
|
||||
slope90: null,
|
||||
anomalyCount: 0,
|
||||
});
|
||||
|
||||
describe("mergeSlimAndThickAnalytics", () => {
|
||||
it("returns undefined when neither slim nor thick has resolved", () => {
|
||||
expect(mergeSlimAndThickAnalytics(undefined, undefined)).toBeUndefined();
|
||||
});
|
||||
|
||||
it("prefers slim summaries when slim carries content (progressive paint)", () => {
|
||||
const slim = {
|
||||
summaries: {
|
||||
WEIGHT: stubSummary(),
|
||||
},
|
||||
};
|
||||
const thick = {
|
||||
summaries: {
|
||||
WEIGHT: { ...stubSummary(), latest: 1 },
|
||||
PULSE: stubSummary(),
|
||||
},
|
||||
bpInTargetPct: 78,
|
||||
};
|
||||
const merged = mergeSlimAndThickAnalytics(slim, thick);
|
||||
// Slim wins on overlapping field — the v1.4.39.2 progressive-paint
|
||||
// contract is preserved.
|
||||
expect(merged?.summaries.WEIGHT?.latest).toBe(78.5);
|
||||
// Slim does not carry PULSE; merge keeps slim's record as-is
|
||||
// (matches the "as soon as slim lands, paint" semantic).
|
||||
expect(merged?.summaries.PULSE).toBeUndefined();
|
||||
// Thick still wires the BD-Zielbereich + glucose fields.
|
||||
expect(merged?.bpInTargetPct).toBe(78);
|
||||
});
|
||||
|
||||
it("falls back to thick when slim resolves with an empty summaries record", () => {
|
||||
// The v1.4.39.3 regression: empty slim `{}` is truthy by JS
|
||||
// semantics so the old `??` short-circuited on it, blanking the
|
||||
// tile strip even when thick had data.
|
||||
const slim = { summaries: {} as Record<string, DataSummary> };
|
||||
const thick = {
|
||||
summaries: {
|
||||
WEIGHT: stubSummary(),
|
||||
},
|
||||
bpInTargetPct: 78,
|
||||
};
|
||||
const merged = mergeSlimAndThickAnalytics(slim, thick);
|
||||
expect(merged?.summaries.WEIGHT?.latest).toBe(78.5);
|
||||
expect(merged?.bpInTargetPct).toBe(78);
|
||||
});
|
||||
|
||||
it("falls back to thick when slim has not resolved yet", () => {
|
||||
const thick = {
|
||||
summaries: {
|
||||
WEIGHT: stubSummary(),
|
||||
},
|
||||
bpInTargetPct: 78,
|
||||
};
|
||||
const merged = mergeSlimAndThickAnalytics(undefined, thick);
|
||||
expect(merged?.summaries.WEIGHT?.latest).toBe(78.5);
|
||||
expect(merged?.bpInTargetPct).toBe(78);
|
||||
});
|
||||
|
||||
it("returns slim summaries when only slim has resolved (thick still loading)", () => {
|
||||
const slim = {
|
||||
summaries: { WEIGHT: stubSummary() },
|
||||
};
|
||||
const merged = mergeSlimAndThickAnalytics(slim, undefined);
|
||||
expect(merged?.summaries.WEIGHT?.latest).toBe(78.5);
|
||||
// Thick-only fields collapse to null/undefined so consumers stay
|
||||
// undefined-safe — the BD-Zielbereich tile self-gates on these.
|
||||
expect(merged?.bpInTargetPct).toBeNull();
|
||||
expect(merged?.glucoseByContext).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns empty summaries when both sides resolve empty (legit zero-data tenant)", () => {
|
||||
const merged = mergeSlimAndThickAnalytics(
|
||||
{ summaries: {} },
|
||||
{ summaries: {} },
|
||||
);
|
||||
expect(merged?.summaries).toEqual({});
|
||||
expect(merged?.bpInTargetPct).toBeNull();
|
||||
});
|
||||
|
||||
it("falls back to thick lastSeenByType when slim has none", () => {
|
||||
const slim = { summaries: { WEIGHT: stubSummary() } };
|
||||
const thick = {
|
||||
summaries: { WEIGHT: stubSummary() },
|
||||
lastSeenByType: {
|
||||
WEIGHT: { lastSeenAt: "2026-05-01T00:00:00.000Z", daysAgo: 3 },
|
||||
},
|
||||
};
|
||||
const merged = mergeSlimAndThickAnalytics(slim, thick);
|
||||
expect(merged?.lastSeenByType?.WEIGHT?.daysAgo).toBe(3);
|
||||
});
|
||||
|
||||
it("prefers slim lastSeenByType when slim carries one", () => {
|
||||
const slim = {
|
||||
summaries: { WEIGHT: stubSummary() },
|
||||
lastSeenByType: {
|
||||
WEIGHT: { lastSeenAt: "2026-05-10T00:00:00.000Z", daysAgo: 1 },
|
||||
},
|
||||
};
|
||||
const thick = {
|
||||
summaries: { WEIGHT: stubSummary() },
|
||||
lastSeenByType: {
|
||||
WEIGHT: { lastSeenAt: "2026-05-01T00:00:00.000Z", daysAgo: 10 },
|
||||
},
|
||||
};
|
||||
const merged = mergeSlimAndThickAnalytics(slim, thick);
|
||||
expect(merged?.lastSeenByType?.WEIGHT?.daysAgo).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,114 @@
|
||||
/**
|
||||
* v1.4.39.3 — pure helper that merges the dashboard's slim
|
||||
* (`/api/analytics?slice=summaries`) and thick (`/api/analytics`)
|
||||
* payloads into the single shape the dashboard tile-strip + chart row
|
||||
* consumes.
|
||||
*
|
||||
* Background — v1.4.39.2 split `useAnalyticsQuery` into two parallel
|
||||
* mounts so the per-type tile strip paints as soon as the slim slice
|
||||
* lands and the BD-Zielbereich + glucose tiles stream in from the
|
||||
* thick slice afterwards. The inline `useMemo` that performed the
|
||||
* merge originally used `slim?.summaries ?? thick?.summaries` —
|
||||
* structurally correct on production where both routes agree, but
|
||||
* silently buggy when slim resolves first with an *empty* `{}` (e.g.
|
||||
* a freshly-seeded tenant whose rollup tier hasn't folded yet, or a
|
||||
* test environment that mocks thick but not slim). The empty object
|
||||
* is truthy by JS semantics so `??` short-circuited and the tile
|
||||
* strip painted blank even though thick carried the full payload —
|
||||
* exactly the regression the v1.4.39.3 e2e CI surfaced across eight
|
||||
* dashboard / chart specs.
|
||||
*
|
||||
* Post-fix: object emptiness is the discriminator. A non-empty slim
|
||||
* payload still wins on overlapping fields (the v1.4.39.2 progressive-
|
||||
* paint contract). An empty slim falls through to thick. When both
|
||||
* are empty, the caller receives an empty record and the dashboard's
|
||||
* data-floor gates render the appropriate empty state.
|
||||
*
|
||||
* Kept as a pure helper so the regression is verified with a tight
|
||||
* unit test (`merge-slim-thick.test.ts`) rather than a source-text
|
||||
* snapshot — string matching against `src/app/page.tsx` would not
|
||||
* have caught the empty-object short-circuit bug.
|
||||
*/
|
||||
import type { DataSummary } from "@/lib/analytics/trends";
|
||||
|
||||
export interface AnalyticsSlimLike {
|
||||
summaries?: Record<string, DataSummary> | null;
|
||||
lastSeenByType?: Record<
|
||||
string,
|
||||
{ lastSeenAt: string; daysAgo: number } | null
|
||||
> | null;
|
||||
}
|
||||
|
||||
export interface AnalyticsThickLike extends AnalyticsSlimLike {
|
||||
bpInTargetPct?: number | null;
|
||||
bpInTargetPct7d?: number | null;
|
||||
bpInTargetPct30d?: number | null;
|
||||
bpInTargetPctAllTime?: number | null;
|
||||
bpInTargetPctPriorMonth?: number | null;
|
||||
bpInTargetPctPriorYear?: number | null;
|
||||
glucoseByContext?: Record<string, unknown> | null;
|
||||
}
|
||||
|
||||
export interface MergedDashboardAnalytics {
|
||||
summaries: Record<string, DataSummary>;
|
||||
lastSeenByType?:
|
||||
| Record<string, { lastSeenAt: string; daysAgo: number } | null>
|
||||
| undefined;
|
||||
bpInTargetPct: number | null;
|
||||
bpInTargetPct7d: number | null;
|
||||
bpInTargetPct30d: number | null;
|
||||
bpInTargetPctAllTime: number | null;
|
||||
bpInTargetPctPriorMonth: number | null;
|
||||
bpInTargetPctPriorYear: number | null;
|
||||
glucoseByContext: Record<string, unknown> | undefined;
|
||||
}
|
||||
|
||||
function hasContent(record: Record<string, unknown> | null | undefined): boolean {
|
||||
return record != null && Object.keys(record).length > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Merge a slim and a thick analytics payload into the shape the
|
||||
* dashboard renders against. Either input may be `undefined`
|
||||
* (TanStack query unresolved) or carry an empty record (zero-data
|
||||
* tenant); the helper falls back to whichever side has content
|
||||
* without ever returning a payload that ignores a populated source.
|
||||
*
|
||||
* Returns `undefined` when neither input has resolved yet so the
|
||||
* caller can keep its loading-state branch unchanged.
|
||||
*/
|
||||
export function mergeSlimAndThickAnalytics(
|
||||
slim: AnalyticsSlimLike | undefined,
|
||||
thick: AnalyticsThickLike | undefined,
|
||||
): MergedDashboardAnalytics | undefined {
|
||||
if (!slim && !thick) return undefined;
|
||||
|
||||
const slimSummariesHaveContent = hasContent(slim?.summaries ?? null);
|
||||
const summaries: Record<string, DataSummary> = slimSummariesHaveContent
|
||||
? (slim!.summaries as Record<string, DataSummary>)
|
||||
: ((thick?.summaries as Record<string, DataSummary> | null | undefined) ??
|
||||
(slim?.summaries as Record<string, DataSummary> | null | undefined) ??
|
||||
{});
|
||||
|
||||
const slimFreshnessHasContent = hasContent(slim?.lastSeenByType ?? null);
|
||||
const lastSeenByType:
|
||||
| Record<string, { lastSeenAt: string; daysAgo: number } | null>
|
||||
| undefined = slimFreshnessHasContent
|
||||
? (slim!.lastSeenByType as Record<
|
||||
string,
|
||||
{ lastSeenAt: string; daysAgo: number } | null
|
||||
>)
|
||||
: (thick?.lastSeenByType ?? undefined);
|
||||
|
||||
return {
|
||||
summaries,
|
||||
lastSeenByType,
|
||||
bpInTargetPct: thick?.bpInTargetPct ?? null,
|
||||
bpInTargetPct7d: thick?.bpInTargetPct7d ?? null,
|
||||
bpInTargetPct30d: thick?.bpInTargetPct30d ?? null,
|
||||
bpInTargetPctAllTime: thick?.bpInTargetPctAllTime ?? null,
|
||||
bpInTargetPctPriorMonth: thick?.bpInTargetPctPriorMonth ?? null,
|
||||
bpInTargetPctPriorYear: thick?.bpInTargetPctPriorYear ?? null,
|
||||
glucoseByContext: thick?.glucoseByContext ?? undefined,
|
||||
};
|
||||
}
|
||||
Reference in new issue
Block a user