test(ui): content-first fallback regression tests (medications + registry) — #015#1054
Conversation
…egistry Locks in the content-first behavior shipped in #1024, including the clinical-governance guarantee CodeRabbit flagged (no authoritative verification badge before live governance reconciles). registry-content-first.dom.test.tsx (RegistryRecordLoader): - content-first: the public fixture paints during loading, not a spinner - governance safety: a fixture's `locallyVerified` is neutralized to false in the provisional paint, then reconciled from authoritative governance on ready - owner-only slugs (no fixture) still show the spinner medication-content-first.dom.test.tsx (MedicationRecordPage): - content-first: the fallback record paints during loading, not the skeleton - the live record swaps in on resolve - no fallback -> skeleton while loading / error state on a failed fetch - graceful: a failed live fetch keeps showing fallback content, not an error Addresses outstanding-issues #15. Tests-only; no runtime change. Verified: typecheck, lint, and the full offline unit suite pass (9 new tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbhUVWVJRwDibC2YtJ6aRX
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds DOM test coverage for content-first fallback behavior in medication and registry record loaders, including provisional rendering, live-data replacement, validation reconciliation, loading states, and fetch-error handling. ChangesContent-first fallback tests
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/medication-content-first.dom.test.tsx (1)
58-63: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winTest doesn't actually exercise the fallback-governance neutralization on error.
This test never passes
fallbackGovernance, so the guarded branch it's meant to validate —governance = data?.governance ?? (error ? undefined : fallbackGovernance)inmedication-record-page.tsx— always evaluates toundefinedregardless of whether the ternary'serrorcheck works correctly. If that guard regressed (e.g., dropped theerror ?check entirely), this test would still pass, which undercuts the "authoritative governance reconciles" coverage claimed in the PR description.✅ Suggested strengthening
it("keeps showing fallback content when the live fetch fails (graceful content-first)", () => { hook.detail = { data: null, loading: false, error: "boom" }; - renderPage(<MedicationRecordPage slug="example" fallbackRecord={record} />); + renderPage( + <MedicationRecordPage + slug="example" + fallbackRecord={record} + fallbackGovernance={{ sourceStatus: "current", validationStatus: "approved" }} + />, + ); expect(screen.getByRole("heading", { name: record.name })).toBeInTheDocument(); expect(screen.queryByText("boom")).not.toBeInTheDocument(); + // fallbackGovernance must not be presented as authoritative once the fetch failed + expect(screen.queryByText(/locally verified/i)).not.toBeInTheDocument(); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/medication-content-first.dom.test.tsx` around lines 58 - 63, Strengthen the test case in the “keeps showing fallback content when the live fetch fails” block by passing a non-empty fallbackGovernance value to MedicationRecordPage, then assert the rendered result confirms governance is neutralized when hook.detail.error is present. Keep the existing fallback content assertions and ensure the test would fail if the error guard in the governance reconciliation logic were removed.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/medication-content-first.dom.test.tsx`:
- Around line 58-63: Strengthen the test case in the “keeps showing fallback
content when the live fetch fails” block by passing a non-empty
fallbackGovernance value to MedicationRecordPage, then assert the rendered
result confirms governance is neutralized when hook.detail.error is present.
Keep the existing fallback content assertions and ensure the test would fail if
the error guard in the governance reconciliation logic were removed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 5546fce9-a8a0-4f2e-b12d-08f22d00b468
📒 Files selected for processing (2)
tests/medication-content-first.dom.test.tsxtests/registry-content-first.dom.test.tsx
CodeRabbit review on #1054: the "graceful on error" test never passed fallbackGovernance, so it did not actually exercise the `governance = data?.governance ?? (error ? undefined : fallbackGovernance)` guard in medication-record-page.tsx — it would still pass if the `error ?` check were dropped. Partial-mock medication-badges (keep all real exports so the detail body renders; wrap medicationIdentityBadges in a call-through spy) and add two tests asserting the exact governance the page hands down: present while loading, dropped to undefined once the fetch fails. Now fails if the error guard regresses. (CodeRabbit's suggested `/locally verified/i` assertion was vacuous — there is no such text; the badge label is "Reviewed" and `isReviewed` also has record-based logic, so asserting the governance argument via the spy is the robust signal.) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UbhUVWVJRwDibC2YtJ6aRX
Summary
Tests-only. Adds focused regression coverage for the content-first detail rendering shipped in #1024 — the behavior that previously had no direct unit test (only an e2e smoke on the medication page and a regression test that mocks the clients out). Closes ledger item #15. No runtime code changes.
Two new
*.dom.test.tsxfiles (9 tests), following the existingregistry-retry.dom.test.tsxpattern (vi.hoistedholder + mocked hook + render-prop children / real provider):tests/registry-content-first.dom.test.tsx(RegistryRecordLoader)locallyVerified: trueis neutralized tofalsein the provisional paint, then reconciled from authoritative governance whenready(unverified → false,locally_reviewed → true).tests/medication-content-first.dom.test.tsx(MedicationRecordPage)RAG impact: no retrieval behaviour change — tests-only, no retrieval/ranking/selection/eval or product code touched.
Verification
npm run typecheck— cleannpm run lint— cleannpm run test— full offline unit suite green, 9 new tests pass (the only failure is the pre-existing sandbox-onlypdf-extraction-budgetflake, unrelated to this change — a Python child-process timing test, no worker/PDF files touched here).npm run verify:pr-local— not run in-session; the above cover this tests-only change. CI runs the full gate.Risk and rollout
Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_01UbhUVWVJRwDibC2YtJ6aRX
Generated by Claude Code
Summary by CodeRabbit