feat: dynamic health status + canonical reference ranges + DB migrations#4
feat: dynamic health status + canonical reference ranges + DB migrations#4Chocksy wants to merge 1 commit intozmeyer44:mainfrom
Conversation
Adds dynamic status computation, production-safe migrations, and comprehensive reference range data. Dynamic status system: - useDynamicStatus hook computes optimal/borderline/elevated from reference ranges instead of hardcoded thresholds - Panel config maps metrics to clinical categories with display metadata - Enhanced health-utils with percentage-based range evaluation Database migrations (production-safe): - 0009: Schema additions (observations metadata, user demographics) - 0010: Seed missing metric definitions + optimal ranges via migration (not seeds, to avoid duplicates on redeploy). Includes NULL-safe unique index on optimal_ranges. Backfills calculated observations (HOMA-IR) from same-date glucose+insulin pairs. - 0011: Consolidate duplicate metric codes to canonical forms (hemoglobin_a1c → hba1c, etc.) Other improvements: - Extended observations router with correct/confirm/delete mutations - Observation correction tracking (original values preserved) - Optimal ranges seed data for 40+ biomarkers - Labs detail page uses dynamic status
|
@Chocksy is attempting to deploy a commit to the Zach's Projects Team on Vercel. A member of the Team first needs to authorize it. |
Greptile SummaryThis PR replaces hardcoded abnormality thresholds with a useDynamicStatus hook that computes status from DB-backed optimal/reference ranges, adds inline edit/delete for observations, and delivers production-safe migrations seeding metric definitions, optimal ranges, and derived-metric backfills. The key concern is a stale-closure bug in resultColumns where getStatus is missing from the useMemo dependency array, causing status badges to show Normal for out-of-range values on metrics where displayPrecision does not change on load. Confidence Score: 4/5Safe to merge after fixing the stale getStatus closure; without it dynamic status silently displays wrong badges The missing getStatus dependency is a present display defect that directly undermines the PR primary goal. One-line dep-array fix resolves it. All other findings are P2 style or performance suggestions. apps/web/app/(dashboard)/(main)/labs/[metricCode]/page.tsx - the useMemo dependency array on line 255 Important Files Changed
Sequence DiagramsequenceDiagram
participant Page as LabDetailPage
participant Hook as useDynamicStatus
participant TRPC as tRPC Server
participant DB as Database
Page->>TRPC: observations.list({metricCode})
Page->>TRPC: metrics.list()
Page->>TRPC: optimalRanges.forUser({metricCode})
TRPC->>DB: SELECT observations
TRPC->>DB: SELECT metric_definitions
TRPC->>DB: SELECT optimal_ranges
DB-->>TRPC: rows
TRPC-->>Hook: metricsData + optimalRangesData
Hook->>Hook: useMemo builds rangesMap
Hook-->>Page: getStatus, getRanges, isAbnormal
Page->>Page: resultColumns useMemo captures getStatus
Note over Page: BUG if displayPrecision unchanged on load
Page->>Page: DataTable renders via captured getStatus
Reviews (1): Last reviewed commit: "feat: dynamic health status + canonical ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| }, | ||
| ], | ||
| [metricCode, displayPrecision], | ||
| [metricCode, displayPrecision, deleteMutation], |
There was a problem hiding this comment.
getStatus closure in memoized columns
getStatus is captured in the cell callbacks for the value and status columns (lines 165 and 216) but is not listed in the useMemo dependency array. Because getStatus is not wrapped in useCallback inside useDynamicStatus, it is a new reference each render that closes over rangesMap. When metricsData or optimalRangesData loads asynchronously, a fresh getStatus is produced but resultColumns will not recompute unless displayPrecision also changes. For metrics where displayPrecision is null both before and after data loads (metric absent from metricsData or precision is null), the cells keep using the stale getStatus that sees an empty rangesMap and always returns normal. Status badges show green Normal for out-of-range values while the un-memoized getRowTint on line 519 correctly shows the warning tint.
| [metricCode, displayPrecision, deleteMutation], | |
| [metricCode, displayPrecision, deleteMutation, getStatus], |
| <button | ||
| onClick={() => deleteMutation.mutate({ id: obs.id })} | ||
| className="rounded-md p-1 text-neutral-300 transition-all hover:bg-red-50 hover:text-red-500" | ||
| title="Remove" | ||
| > | ||
| <Trash2 className="h-3 w-3" /> | ||
| </button> | ||
| </div> |
There was a problem hiding this comment.
The Trash2 button fires deleteMutation.mutate directly on click with no confirmation step. The server-side delete procedure does a hard ctx.db.delete with no soft-delete or recycle bin. A single misclick irrecoverably removes a health observation. Consider adding a window.confirm or inline confirm state before triggering the mutation.
| FROM observations g | ||
| JOIN observations i | ||
| ON g.user_id = i.user_id | ||
| AND g.observed_at = i.observed_at | ||
| AND i.metric_code = 'insulin' | ||
| AND i.value_numeric IS NOT NULL | ||
| AND i.value_numeric > 0 | ||
| WHERE g.metric_code = 'glucose' | ||
| AND g.value_numeric IS NOT NULL | ||
| AND g.value_numeric > 0 | ||
| AND NOT EXISTS ( | ||
| SELECT 1 FROM observations ex | ||
| WHERE ex.user_id = g.user_id | ||
| AND ex.metric_code = 'homa_ir' | ||
| AND ex.observed_at = g.observed_at | ||
| ); |
There was a problem hiding this comment.
All four derived-metric backfills join on g.observed_at = i.observed_at using exact timestamp equality. Lab systems sometimes stamp results from the same blood draw with timestamps seconds apart. Any such pair silently produces no derived observation. Consider widening to date-level equality (DATE(g.observed_at) = DATE(i.observed_at)) or documenting this constraint so operators know what to check if expected HOMA-IR rows are missing post-migration.
| dateFrom: z.date().optional(), | ||
| dateTo: z.date().optional(), | ||
| status: z.string().optional(), | ||
| limit: z.number().min(1).max(1000).default(50), |
There was a problem hiding this comment.
The detail page requests at most 200 observations so the higher ceiling is only reachable via direct API calls or future callers. A request for 1000 rows loads everything into memory at once with no streaming. The cursor-based pagination is already in place; consider keeping the ceiling at 200 and encouraging callers to page through results.
1 participant
Summary
useDynamicStatushook computes optimal/borderline/elevated from DB reference ranges instead of hardcoded thresholdsKey Changes
apps/web/hooks/use-dynamic-status.ts— Dynamic status hookapps/web/lib/panel-config.ts— Clinical category mappingapps/web/lib/health-utils.ts— Percentage-based range evaluationpackages/database/drizzle/0010_seed_missing_metric_data.sql— Migration for metric defs + optimal ranges + HOMA-IR backfillpackages/database/drizzle/0011_consolidate_metric_codes.sql— Canonical code consolidationapps/web/server/trpc/routers/observations.ts— Extended CRUD mutationspackages/database/src/seed/data/optimal-ranges.ts— 40+ biomarker rangesTest plan
/labs/hba1cshows data (not empty due to code mismatch)