feat: policy intelligence pre-check for EHR demo#34
Merged
Conversation
Foundation for the policy intelligence demo extension. Adds: - PreCheckCriterion interface for pre-sign gap analysis - DEMO_PRECHECK_CRITERIA with 3 met / 2 indeterminate items - DEMO_PA_RESULT_SOURCES for post-sign evidence trail display Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- PAReadinessWidget: amber-accented pre-check gap analysis component with staggered cascade animation and criteria status display - EvidenceTag: monospace provenance marker for sourced evidence - PAResultsPanel: enhanced with evidence trail (text + source tags) - useEhrDemoFlow: extended with flagged state and preCheckCriteria - 53/53 EHR component tests passing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- EncounterSidebar: amber "Policy Check" indicator in flagged state - ehr-demo: auto-triggers flag() on mount, renders PAReadinessWidget in flagged state, transitions to PAResultsPanel on sign - Removed View Policy Criteria button (superseded by live widget) - Amber-to-blue visual transition when signing encounter - 298/298 dashboard tests passing Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a policy pre-check ("flagged") demo flow and UI: new PAReadinessWidget and EvidenceTag components, demo pre-check data and sources, useEhrDemoFlow flag/doc flows, EncounterSidebar preCheckCount prop, route integration with ConfirmationDialog, PAResultsPanel evidence/confirmation UI, tests, and OpenAPI/schema updates. Changes
Sequence DiagramsequenceDiagram
participant User
participant EHR as EhrDemoPage
participant Flow as useEhrDemoFlow
participant Widget as PAReadinessWidget
participant Sidebar as EncounterSidebar
participant Results as PAResultsPanel
User->>EHR: open page
EHR->>Flow: mount -> flag()
Flow->>Flow: wait ~1500ms, set state = "flagged"
Flow->>Flow: populate preCheckCriteria (DEMO_PRECHECK_CRITERIA_INITIAL)
Flow->>EHR: expose preCheckCriteria & preCheckCount
EHR->>Widget: render PAReadinessWidget(criteria)
EHR->>Sidebar: pass preCheckCount
Widget->>User: display progress, criteria, actions
User->>Widget: click criterion / Document gap
Widget->>EHR: onCriterionClick / onGapAction
User->>EHR: sign encounter
EHR->>Flow: sign() -> processing -> reviewing
Flow->>EHR: provide paRequest with results (DEMO_PA_RESULT)
EHR->>Results: render PAResultsPanel with evidence + EvidenceTag
User->>Results: View Confirmation
Results->>EHR: onViewConfirmation -> open ConfirmationDialog
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
apps/dashboard/src/components/ehr/PAResultsPanel.tsx (1)
17-18: KeepPAResultsPaneldata-agnostic by injecting evidence sources via props.Directly importing demo fixtures inside the panel couples a reusable UI component to one dataset. Passing a
criterionSourcesprop from the demo container keeps boundaries cleaner and makes reuse safer.♻️ Suggested prop-based boundary
export interface PAResultsPanelProps { state: EhrDemoState; paRequest: PARequest | null; + criterionSources?: Record<string, { evidence: string; source: string }>; error?: string | null; onSubmit: () => void; @@ -function ReviewingView({ +function ReviewingView({ paRequest, + criterionSources, onSubmit, onCriterionClick, onViewPdf, }: { paRequest: PARequest; + criterionSources?: Record<string, { evidence: string; source: string }>; onSubmit: () => void; @@ - const sourceData = DEMO_PA_RESULT_SOURCES[criterion.label]; + const sourceData = criterionSources?.[criterion.label];Also applies to: 152-167
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/PAResultsPanel.tsx` around lines 17 - 18, PAResultsPanel is coupled to demo data by importing DEMO_PA_RESULT_SOURCES directly; make it data-agnostic by removing that import and accepting a prop (e.g., criterionSources) that provides the evidence sources array. Update the PAResultsPanel component signature to add the criterionSources prop (and its type), replace usages of DEMO_PA_RESULT_SOURCES inside render logic (and where EvidenceTag is mapped) with the new prop, and ensure callers (demo container) pass DEMO_PA_RESULT_SOURCES into PAResultsPanel so behavior is unchanged.apps/dashboard/src/lib/demoData.ts (1)
117-138: Avoid coupling evidence lookup to display labels.Using free-text labels as keys is brittle; small wording changes can silently drop evidence rendering. Prefer stable IDs for criteria and key
DEMO_PA_RESULT_SOURCESby ID instead of label text.♻️ Suggested direction (stable criterion IDs)
+export type CriterionId = + | 'icd10' + | 'red_flags' + | 'conservative_mgmt' + | 'clinical_rationale' + | 'no_duplicate_imaging'; + export interface PreCheckCriterion { + id: CriterionId; label: string; status: 'met' | 'not-met' | 'indeterminate'; evidence?: string; gap?: string; source?: string; } @@ -export const DEMO_PA_RESULT_SOURCES: Record<string, { evidence: string; source: string }> = { - 'Valid ICD-10 for lumbar pathology': { +export const DEMO_PA_RESULT_SOURCES: Record<CriterionId, { evidence: string; source: string }> = { + icd10: { evidence: 'M54.5, M54.51 — low back pain, lumbar radiculopathy left', source: 'Assessment', }, - 'Cauda equina, tumor, infection, major neuro deficit': { + red_flags: { evidence: 'Progressive numbness in left foot over past 3 weeks', source: 'HPI', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/demoData.ts` around lines 117 - 138, DEMO_PA_RESULT_SOURCES currently uses display labels as object keys which is brittle; change the keys to stable criterion IDs (e.g., "icd10_lumbar", "cauda_equina", "conservative_mgmt_4w", "supporting_rationale", "no_recent_imaging") and keep the current label text as a new "label" property inside each value object so evidence lookup is ID-based while UI can still show the human-readable label; update any consumers that reference DEMO_PA_RESULT_SOURCES by label to use the new IDs (or read .label) and ensure functions/components that iterate keys (search, render, or lookup) use the ID keys instead of assuming display strings.apps/dashboard/src/components/ehr/__tests__/useEhrDemoFlow.test.ts (1)
130-257: Add a regression test forreset()whileflag()delay is in flight.Given the delayed async transition, this edge case should be explicitly covered to prevent stale
flag()completions from re-enteringflaggedafter reset/sign.🧪 Suggested test case
+ it('useEhrDemoFlow_Reset_DuringPendingFlag_DoesNotTransitionToFlagged', async () => { + const useEhrDemoFlow = await importHook(); + const { result } = renderHook(() => useEhrDemoFlow()); + + let flagPromise: Promise<void>; + act(() => { + flagPromise = result.current.flag(); + }); + + // Reset before the 1500ms delay finishes + act(() => { + result.current.reset(); + }); + + await act(async () => { + vi.advanceTimersByTime(1500); + await flagPromise!; + }); + + expect(result.current.state).toBe('idle'); + expect(result.current.preCheckCriteria).toBeNull(); + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/__tests__/useEhrDemoFlow.test.ts` around lines 130 - 257, Add a regression test in useEhrDemoFlow.test.ts that simulates calling result.current.flag() and then calls result.current.reset() before the 1500ms flag delay elapses, asserting that after advancing timers past 1500ms the state remains 'idle' (and preCheckCriteria is null) and the later resolution of the original flag promise does not transition state to 'flagged'; Locate the hook via importHook() and renderHook(() => useEhrDemoFlow()), capture the Promise returned by result.current.flag(), call result.current.reset() inside an act() before vi.advanceTimersByTime(1500), then advance timers past 1500ms and await the original flag promise to ensure no stale completion changes state.apps/dashboard/src/components/ehr/PAReadinessWidget.tsx (1)
100-157: Consider using a unique identifier for list keys instead ofcriterion.label.Using
criterion.labelas the React key assumes labels are unique across all criteria. If two criteria share the same label, React may encounter rendering issues. Looking at thePreCheckCriterioninterface inapps/dashboard/src/lib/demoData.ts, there's noidfield. For demo data this is likely fine, but if this component is reused with dynamic data, consider adding a stable unique identifier to the type or falling back to index as a compound key.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/PAReadinessWidget.tsx` around lines 100 - 157, The list key currently uses criterion.label inside the criteria.map in PAReadinessWidget; change this to a stable unique identifier (e.g., add an id field to the PreCheckCriterion type in demoData and use criterion.id) or, if you cannot add ids, use a compound fallback key like `${criterion.label}-${index}` to guarantee uniqueness and avoid React reconciliation bugs; update the key prop in the <li> returned by criteria.map accordingly and adjust any demo data (PreCheckCriterion instances) to include the id when possible.apps/dashboard/src/components/ehr/EncounterSidebar.tsx (1)
13-13: Consider extracting thepreCheckCounttype.The inline object type
{ met: number; total: number }works fine here, but if this shape is reused elsewhere (e.g., in the route or other components), extracting it to a named type would improve maintainability.♻️ Optional type extraction
+export interface PreCheckCount { + met: number; + total: number; +} + interface EncounterSidebarProps { activeStage?: StageName; signed?: boolean; flowState?: EhrDemoState; - preCheckCount?: { met: number; total: number }; + preCheckCount?: PreCheckCount; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/EncounterSidebar.tsx` at line 13, Extract the inline type for preCheckCount into a named type and use it in the EncounterSidebar props: create a type or interface named PreCheckCount (e.g., type PreCheckCount = { met: number; total: number }) and replace the inline `{ met: number; total: number }` with `PreCheckCount` on the preCheckCount prop of the EncounterSidebar component; update any other places (routes, components, or imports) that use this shape to reference the new PreCheckCount type for consistency.apps/dashboard/src/components/ehr/__tests__/PAReadinessWidget.test.tsx (1)
59-61: DOM query via class selector is brittle.Querying
.animate-spincouples the test to Tailwind class names. If the spinner implementation changes (e.g., different animation class), the test breaks. Consider adding adata-testidto the Loader2 wrapper in the component, or usegetByRolewith an accessible name if one is added.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/__tests__/PAReadinessWidget.test.tsx` around lines 59 - 61, The test in PAReadinessWidget.test.tsx is querying the DOM with document.querySelector('.animate-spin'), which ties it to Tailwind classnames; instead, add a stable identifier (e.g., a data-testid on the Loader2 wrapper inside the PAReadinessWidget component) and update the test to use getByTestId('pa-readiness-spinner') or, if accessibility is added, use getByRole('status', { name: /loading/i }) to assert presence; locate the Loader2 wrapper in the PAReadinessWidget component and add the data-testid, then replace the document.querySelector('.animate-spin') usage in the test with the appropriate getByTestId/getByRole query.apps/dashboard/src/routes/ehr-demo.tsx (1)
69-80: Hardcoded order/payer/policyId could be derived from data.The values passed to
PAReadinessWidgetare hardcoded, butDEMO_ORDERS[0]already containscodeandname. Consider deriving from the existing demo data for consistency:♻️ Derive from demo data
<PAReadinessWidget state="ready" criteria={flow.preCheckCriteria} - order={{ code: '72148', name: 'MRI Lumbar Spine w/o Contrast' }} + order={{ code: DEMO_ORDERS[0].code, name: DEMO_ORDERS[0].name }} payer="Blue Cross Blue Shield" policyId="LCD L34220" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/routes/ehr-demo.tsx` around lines 69 - 80, The PAReadinessWidget is being passed hardcoded order/payer/policyId values; instead derive them from the demo data (e.g., use DEMO_ORDERS[0].code and DEMO_ORDERS[0].name for the order, and use available payer/policyId fields from DEMO_ORDERS[0] or from flow if those values live on the flow object) — update the JSX that renders PAReadinessWidget to pass order={{ code: DEMO_ORDERS[0].code, name: DEMO_ORDERS[0].name }} and replace the hardcoded payer and policyId with the corresponding properties from DEMO_ORDERS[0] (or flow) so values stay consistent with the demo data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/ehr/useEhrDemoFlow.ts`:
- Around line 35-40: The delayed flag() transition can overwrite newer state
because it checks `state` before `await delay(1500)` but then unconditionally
calls `setState('flagged')`; to fix, add a cancellation/validation step after
the delay (or use an abort token) to ensure the current state is still the
expected value before calling `setState('flagged')` and
`setPreCheckCriteria(DEMO_PRECHECK_CRITERIA)`; apply the same pattern to the
other delayed transition around lines 89-94 (e.g., the similar async handler)
and ensure `reset()` and `sign()` can flip a cancellation flag (or update a ref)
so stale async completions do not mutate state.
In `@apps/dashboard/src/lib/__tests__/demoData.test.ts`:
- Around line 88-93: The test DEMO_PRECHECK_CRITERIA_LabelsMatchLCDPolicy is too
permissive because it checks policyLabels.some(pl =>
pl.includes(c.label.substring(0, 10))); replace this with an exact-match
assertion: build a normalized set of labels from LCD_L34220_POLICY.criteria
(e.g., trim and lowercase each label) then assert each DEMO_PRECHECK_CRITERIA
item's normalized label is contained in that set (or use an explicit mapping
between DEMO_PRECHECK_CRITERIA and LCD_L34220_POLICY criteria); update the test
to use expect(policyLabelSet.has(normalizedLabel)).toBe(true) so real mismatches
fail.
---
Nitpick comments:
In `@apps/dashboard/src/components/ehr/__tests__/PAReadinessWidget.test.tsx`:
- Around line 59-61: The test in PAReadinessWidget.test.tsx is querying the DOM
with document.querySelector('.animate-spin'), which ties it to Tailwind
classnames; instead, add a stable identifier (e.g., a data-testid on the Loader2
wrapper inside the PAReadinessWidget component) and update the test to use
getByTestId('pa-readiness-spinner') or, if accessibility is added, use
getByRole('status', { name: /loading/i }) to assert presence; locate the Loader2
wrapper in the PAReadinessWidget component and add the data-testid, then replace
the document.querySelector('.animate-spin') usage in the test with the
appropriate getByTestId/getByRole query.
In `@apps/dashboard/src/components/ehr/__tests__/useEhrDemoFlow.test.ts`:
- Around line 130-257: Add a regression test in useEhrDemoFlow.test.ts that
simulates calling result.current.flag() and then calls result.current.reset()
before the 1500ms flag delay elapses, asserting that after advancing timers past
1500ms the state remains 'idle' (and preCheckCriteria is null) and the later
resolution of the original flag promise does not transition state to 'flagged';
Locate the hook via importHook() and renderHook(() => useEhrDemoFlow()), capture
the Promise returned by result.current.flag(), call result.current.reset()
inside an act() before vi.advanceTimersByTime(1500), then advance timers past
1500ms and await the original flag promise to ensure no stale completion changes
state.
In `@apps/dashboard/src/components/ehr/EncounterSidebar.tsx`:
- Line 13: Extract the inline type for preCheckCount into a named type and use
it in the EncounterSidebar props: create a type or interface named PreCheckCount
(e.g., type PreCheckCount = { met: number; total: number }) and replace the
inline `{ met: number; total: number }` with `PreCheckCount` on the
preCheckCount prop of the EncounterSidebar component; update any other places
(routes, components, or imports) that use this shape to reference the new
PreCheckCount type for consistency.
In `@apps/dashboard/src/components/ehr/PAReadinessWidget.tsx`:
- Around line 100-157: The list key currently uses criterion.label inside the
criteria.map in PAReadinessWidget; change this to a stable unique identifier
(e.g., add an id field to the PreCheckCriterion type in demoData and use
criterion.id) or, if you cannot add ids, use a compound fallback key like
`${criterion.label}-${index}` to guarantee uniqueness and avoid React
reconciliation bugs; update the key prop in the <li> returned by criteria.map
accordingly and adjust any demo data (PreCheckCriterion instances) to include
the id when possible.
In `@apps/dashboard/src/components/ehr/PAResultsPanel.tsx`:
- Around line 17-18: PAResultsPanel is coupled to demo data by importing
DEMO_PA_RESULT_SOURCES directly; make it data-agnostic by removing that import
and accepting a prop (e.g., criterionSources) that provides the evidence sources
array. Update the PAResultsPanel component signature to add the criterionSources
prop (and its type), replace usages of DEMO_PA_RESULT_SOURCES inside render
logic (and where EvidenceTag is mapped) with the new prop, and ensure callers
(demo container) pass DEMO_PA_RESULT_SOURCES into PAResultsPanel so behavior is
unchanged.
In `@apps/dashboard/src/lib/demoData.ts`:
- Around line 117-138: DEMO_PA_RESULT_SOURCES currently uses display labels as
object keys which is brittle; change the keys to stable criterion IDs (e.g.,
"icd10_lumbar", "cauda_equina", "conservative_mgmt_4w", "supporting_rationale",
"no_recent_imaging") and keep the current label text as a new "label" property
inside each value object so evidence lookup is ID-based while UI can still show
the human-readable label; update any consumers that reference
DEMO_PA_RESULT_SOURCES by label to use the new IDs (or read .label) and ensure
functions/components that iterate keys (search, render, or lookup) use the ID
keys instead of assuming display strings.
In `@apps/dashboard/src/routes/ehr-demo.tsx`:
- Around line 69-80: The PAReadinessWidget is being passed hardcoded
order/payer/policyId values; instead derive them from the demo data (e.g., use
DEMO_ORDERS[0].code and DEMO_ORDERS[0].name for the order, and use available
payer/policyId fields from DEMO_ORDERS[0] or from flow if those values live on
the flow object) — update the JSX that renders PAReadinessWidget to pass
order={{ code: DEMO_ORDERS[0].code, name: DEMO_ORDERS[0].name }} and replace the
hardcoded payer and policyId with the corresponding properties from
DEMO_ORDERS[0] (or flow) so values stay consistent with the demo data.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 371e065c-a7e9-4507-bd99-d033f7ba7a96
📒 Files selected for processing (15)
apps/dashboard/src/components/ehr/EncounterSidebar.tsxapps/dashboard/src/components/ehr/EvidenceTag.tsxapps/dashboard/src/components/ehr/PAReadinessWidget.tsxapps/dashboard/src/components/ehr/PAResultsPanel.tsxapps/dashboard/src/components/ehr/__tests__/EncounterSidebar.test.tsxapps/dashboard/src/components/ehr/__tests__/EvidenceTag.test.tsxapps/dashboard/src/components/ehr/__tests__/PAReadinessWidget.test.tsxapps/dashboard/src/components/ehr/__tests__/PAResultsPanel.test.tsxapps/dashboard/src/components/ehr/__tests__/useEhrDemoFlow.test.tsapps/dashboard/src/components/ehr/index.tsapps/dashboard/src/components/ehr/useEhrDemoFlow.tsapps/dashboard/src/lib/__tests__/demoData.test.tsapps/dashboard/src/lib/demoData.tsapps/dashboard/src/routes/__tests__/ehr-demo.test.tsxapps/dashboard/src/routes/ehr-demo.tsx
- Remove accidental lightningcss-darwin-arm64 direct dependency from root package.json that caused EBADPLATFORM on Linux CI runners - Regenerate package-lock.json for cross-platform compatibility - Fix race condition in useEhrDemoFlow: flag() now checks a cancellation ref after the delay so reset() prevents stale state transitions - Align DEMO_PRECHECK_CRITERIA labels with LCD_L34220_POLICY labels (neurological vs neuro) and use exact Set-based matching in test Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove unused DEMO_PA_RESULT_SOURCES import from PAResultsPanel test - Remove unused 'within' import from analysis-criteria-dialog test - Run sync:schemas to resolve schema drift in generated files Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/dashboard/src/components/ehr/useEhrDemoFlow.ts (2)
45-71: Consider adding a state guard insign()for defensive clarity.Currently
sign()can be invoked from any state. If the intended flow isidle → flagged → signing, a guard clause would prevent accidental misuse and make the state machine explicit:const sign = useCallback(async () => { + if (state !== 'idle' && state !== 'flagged') return; try { setState('signing');This is optional if callers are already gating the button, but keeps the hook self-documenting.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/useEhrDemoFlow.ts` around lines 45 - 71, Add a defensive state guard at the start of the sign() callback so it only proceeds when the hook is in the expected prior state (e.g., 'flagged' or 'idle' depending on intended flow); in practice, check the current state (the state variable used by useEhrDemoFlow) and return early (or set an error) if it isn't the allowed value, keeping the existing try/catch and flow intact—this makes the state transition in sign() explicit and prevents accidental invocation from other states.
39-39: Extract magic number to a named constant.The 1500ms delay is used inline; consider defining
PRECHECK_DELAY_MSalongside the other delay constants for consistency and clarity.♻️ Suggested refactor
/** Minimum time (ms) for the submit animation. */ const SUBMIT_DELAY_MS = 1_500; + +/** Delay (ms) before showing the policy pre-check widget. */ +const PRECHECK_DELAY_MS = 1_500; @@ - await delay(1500); + await delay(PRECHECK_DELAY_MS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/useEhrDemoFlow.ts` at line 39, Replace the inline magic number 1500ms in useEhrDemoFlow (the await delay(1500) call) with a named constant PRECHECK_DELAY_MS; add PRECHECK_DELAY_MS alongside the existing delay constants in the same module so it’s clear and consistent with other timing constants, then use PRECHECK_DELAY_MS in the delay(...) invocation to improve readability and maintainability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/dashboard/src/components/ehr/useEhrDemoFlow.ts`:
- Around line 45-71: Add a defensive state guard at the start of the sign()
callback so it only proceeds when the hook is in the expected prior state (e.g.,
'flagged' or 'idle' depending on intended flow); in practice, check the current
state (the state variable used by useEhrDemoFlow) and return early (or set an
error) if it isn't the allowed value, keeping the existing try/catch and flow
intact—this makes the state transition in sign() explicit and prevents
accidental invocation from other states.
- Line 39: Replace the inline magic number 1500ms in useEhrDemoFlow (the await
delay(1500) call) with a named constant PRECHECK_DELAY_MS; add PRECHECK_DELAY_MS
alongside the existing delay constants in the same module so it’s clear and
consistent with other timing constants, then use PRECHECK_DELAY_MS in the
delay(...) invocation to improve readability and maintainability.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6276dc60-1af9-4769-bcec-f52f82ebfb9e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (4)
apps/dashboard/src/components/ehr/useEhrDemoFlow.tsapps/dashboard/src/lib/__tests__/demoData.test.tsapps/dashboard/src/lib/demoData.tspackage.json
💤 Files with no reviewable changes (1)
- package.json
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/dashboard/src/lib/demoData.ts
- apps/dashboard/src/lib/tests/demoData.test.ts
- Fix E501 line-too-long violations across policy seeds, tests, and reasoning modules by wrapping long strings and function signatures - Fix E402 import ordering in test_evidence_extractor.py - Fix F841 unused variable in test_form_generator.py - Fix import sorting (I001) via ruff --fix Pre-existing issues surfaced by schema sync triggering Intelligence CI. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shared/schemas/intelligence.openapi.json (1)
1-449: Consider consolidating duplicate OpenAPI specs.This file is identical to
apps/intelligence/openapi.json. Maintaining two copies risks drift—one gets updated while the other doesn't. Options:
- Symlink: Have
shared/schemas/intelligence.openapi.jsonreferenceapps/intelligence/openapi.json- Build-time copy: Generate the shared schema from the source during build
- Single source: Keep only one file and update all references
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shared/schemas/intelligence.openapi.json` around lines 1 - 449, The OpenAPI JSON is duplicated (same "AuthScript Intelligence Service" spec including paths like "/analyze" and components such as "PAFormResponse" and operationId "analyze_analyze_post"), which risks drift; pick one canonical OpenAPI source and remove the duplicate by either making the shared copy a generated artifact or a symlink to the canonical spec, update the build pipeline to generate/copy the canonical spec into the shared location (or create a symlink during repo setup), and add a CI/lint check to fail on divergent OpenAPI files so only the chosen canonical spec is edited going forward.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shared/schemas/intelligence.openapi.json`:
- Around line 1-449: The OpenAPI JSON is duplicated (same "AuthScript
Intelligence Service" spec including paths like "/analyze" and components such
as "PAFormResponse" and operationId "analyze_analyze_post"), which risks drift;
pick one canonical OpenAPI source and remove the duplicate by either making the
shared copy a generated artifact or a symlink to the canonical spec, update the
build pipeline to generate/copy the canonical spec into the shared location (or
create a symlink during repo setup), and add a CI/lint check to fail on
divergent OpenAPI files so only the chosen canonical spec is edited going
forward.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6724eab3-56fb-472e-8985-dbb285db3f55
⛔ Files ignored due to path filters (4)
apps/dashboard/src/api/generated/analysis/analysis.tsis excluded by!**/generated/**apps/dashboard/src/api/generated/intelligence.schemas.tsis excluded by!**/generated/**apps/gateway/Gateway.API/Models/Generated/IntelligenceTypes.csis excluded by!**/generated/**shared/types/src/generated/intelligence.tsis excluded by!**/generated/**
📒 Files selected for processing (4)
apps/dashboard/src/components/ehr/__tests__/PAResultsPanel.test.tsxapps/dashboard/src/routes/__tests__/analysis-criteria-dialog.test.tsxapps/intelligence/openapi.jsonshared/schemas/intelligence.openapi.json
✅ Files skipped from review due to trivial changes (1)
- apps/dashboard/src/routes/tests/analysis-criteria-dialog.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/dashboard/src/components/ehr/tests/PAResultsPanel.test.tsx
- intelligence: use lru_cache instead of manual _cached attribute pattern - intelligence: add Literal type annotation for recommendation variable - intelligence: add PolicyRegistry type annotation for register_all_seeds - intelligence: fix EvidenceItem status type in test helper - dashboard: fix NewPAModal test mock to match Procedure interface Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/intelligence/src/tests/test_confidence_scorer.py (1)
62-71:⚠️ Potential issue | 🟡 MinorUse exact equality for this floor regression test.
The
calculate_confidence()function guarantees a minimum score of 0.05 (viamax(SCORE_FLOOR, ...)at line 79) and rounds to 4 decimals before returning. The current assertionpytest.approx(0.05, abs=0.01)accepts values from 0.04 to 0.06, which is too loose for a regression guard. Since the floor always produces exactly 0.05, assert it as such:- assert result.score == pytest.approx(0.05, abs=0.01) + assert result.score == 0.05🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/intelligence/src/tests/test_confidence_scorer.py` around lines 62 - 71, The test test_all_not_met_hits_floor uses a loose approx assertion; replace the pytest.approx check with an exact equality against the known floor value since calculate_confidence enforces a SCORE_FLOOR and rounds to 4 decimals. Update the assertion in test_all_not_met_hits_floor to assert result.score == 0.05 (referring to calculate_confidence and the SCORE_FLOOR behavior) so the test fails on any deviation from the exact floor.
🧹 Nitpick comments (1)
apps/intelligence/src/tests/test_evidence_extractor.py (1)
80-80: Optional: Add return type hints to test functions for consistency.Several test functions in this file (e.g.,
test_evaluate_criterion_returns_met_evidence_item,test_evaluate_criterion_parses_not_met, etc.) lack-> Nonereturn annotations. The coding guidelines specify complete type annotations for all functions underapps/intelligence/**/*.py.This is pre-existing and low priority, but worth addressing for consistency.
-async def test_evaluate_criterion_returns_met_evidence_item(): +async def test_evaluate_criterion_returns_met_evidence_item() -> None:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/intelligence/src/tests/test_evidence_extractor.py` at line 80, Add explicit return type annotations (-> None) to the test functions that currently lack them (e.g., test_evaluate_criterion_returns_met_evidence_item, test_evaluate_criterion_parses_not_met, and similar tests in this file) to satisfy the project's typing guidelines; locate each async def test_* and append "-> None" before the colon so signatures read "async def test_xyz(...) -> None:" while leaving the function bodies unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/intelligence/src/tests/test_confidence_scorer.py`:
- Around line 62-71: The test test_all_not_met_hits_floor uses a loose approx
assertion; replace the pytest.approx check with an exact equality against the
known floor value since calculate_confidence enforces a SCORE_FLOOR and rounds
to 4 decimals. Update the assertion in test_all_not_met_hits_floor to assert
result.score == 0.05 (referring to calculate_confidence and the SCORE_FLOOR
behavior) so the test fails on any deviation from the exact floor.
---
Nitpick comments:
In `@apps/intelligence/src/tests/test_evidence_extractor.py`:
- Line 80: Add explicit return type annotations (-> None) to the test functions
that currently lack them (e.g.,
test_evaluate_criterion_returns_met_evidence_item,
test_evaluate_criterion_parses_not_met, and similar tests in this file) to
satisfy the project's typing guidelines; locate each async def test_* and append
"-> None" before the colon so signatures read "async def test_xyz(...) -> None:"
while leaving the function bodies unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17b31530-3a88-4121-b150-baebd991ac21
📒 Files selected for processing (17)
apps/intelligence/src/api/analyze.pyapps/intelligence/src/policies/seed/__init__.pyapps/intelligence/src/policies/seed/echocardiogram.pyapps/intelligence/src/policies/seed/epidural_steroid.pyapps/intelligence/src/policies/seed/mri_lumbar.pyapps/intelligence/src/reasoning/confidence_scorer.pyapps/intelligence/src/reasoning/evidence_extractor.pyapps/intelligence/src/tests/test_analyze.pyapps/intelligence/src/tests/test_confidence_scorer.pyapps/intelligence/src/tests/test_echocardiogram_policy.pyapps/intelligence/src/tests/test_evidence_extractor.pyapps/intelligence/src/tests/test_form_generator.pyapps/intelligence/src/tests/test_generic_policy.pyapps/intelligence/src/tests/test_pa_form_model.pyapps/intelligence/src/tests/test_policy_model.pyapps/intelligence/src/tests/test_policy_registry.pyapps/intelligence/src/tests/test_seed_policies.py
💤 Files with no reviewable changes (3)
- apps/intelligence/src/tests/test_policy_model.py
- apps/intelligence/src/reasoning/confidence_scorer.py
- apps/intelligence/src/tests/test_pa_form_model.py
✅ Files skipped from review due to trivial changes (7)
- apps/intelligence/src/tests/test_analyze.py
- apps/intelligence/src/policies/seed/mri_lumbar.py
- apps/intelligence/src/tests/test_form_generator.py
- apps/intelligence/src/tests/test_echocardiogram_policy.py
- apps/intelligence/src/reasoning/evidence_extractor.py
- apps/intelligence/src/api/analyze.py
- apps/intelligence/src/policies/seed/echocardiogram.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
apps/intelligence/src/api/analyze.py (1)
27-35: Specify encoding for file open; notelru_cachereturns the same mutable instance.Two observations:
Encoding:
open()without explicit encoding relies on platform defaults. Specifyencoding="utf-8"for consistent behavior across environments.Cached mutable object:
lru_cachereturns the samePAFormResponseinstance on each call. Since FastAPI serializes it without mutation, this works in practice. However, if any code path ever mutates the returned object, all subsequent callers see the mutation. Consider returning_load_demo_response().model_copy()at the call site if this becomes a concern.♻️ Suggested fix for encoding
`@lru_cache`(maxsize=1) def _load_demo_response() -> PAFormResponse: """Load and cache the canned demo response for MRI Lumbar Spine.""" fixture_path = ( Path(__file__).parent.parent / "fixtures" / "demo_mri_lumbar.json" ) - with open(fixture_path) as f: + with open(fixture_path, encoding="utf-8") as f: data = json.load(f) return PAFormResponse(**data)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/intelligence/src/api/analyze.py` around lines 27 - 35, The _load_demo_response function opens the fixture file without specifying encoding and returns a cached PAFormResponse instance which is mutable; update the file open call in _load_demo_response to include encoding="utf-8" to ensure consistent behavior across platforms, and to avoid shared-mutation surprises either change callers that use _load_demo_response() to call .model_copy() on the returned PAFormResponse or modify _load_demo_response to return a copy (e.g., return PAFormResponse(**data).model_copy() or return a deep copy) so the lru_cached instance is not mutated by consumers.apps/intelligence/src/tests/test_confidence_scorer.py (1)
10-10: Centralize the evidence-status alias.
EvidenceStatusduplicates theEvidenceItem.statusliteral fromapps/intelligence/src/models/pa_form.pyLines 7-15, so the test helpers can drift from the model if a new status is added later. I’d export a shared alias from the model layer and import it here instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/intelligence/src/tests/test_confidence_scorer.py` at line 10, The test file defines a duplicate EvidenceStatus Literal which duplicates EvidenceItem.status in the model; replace the local alias by importing the shared alias exported from the model (the alias that defines EvidenceItem.status in apps/intelligence/src/models/pa_form.py) so tests use the single source of truth; update imports in test_confidence_scorer.py to import EvidenceStatus (or the exported alias name) from the model module and remove the local Literal definition to prevent drift when statuses change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@apps/intelligence/src/api/analyze.py`:
- Around line 27-35: The _load_demo_response function opens the fixture file
without specifying encoding and returns a cached PAFormResponse instance which
is mutable; update the file open call in _load_demo_response to include
encoding="utf-8" to ensure consistent behavior across platforms, and to avoid
shared-mutation surprises either change callers that use _load_demo_response()
to call .model_copy() on the returned PAFormResponse or modify
_load_demo_response to return a copy (e.g., return
PAFormResponse(**data).model_copy() or return a deep copy) so the lru_cached
instance is not mutated by consumers.
In `@apps/intelligence/src/tests/test_confidence_scorer.py`:
- Line 10: The test file defines a duplicate EvidenceStatus Literal which
duplicates EvidenceItem.status in the model; replace the local alias by
importing the shared alias exported from the model (the alias that defines
EvidenceItem.status in apps/intelligence/src/models/pa_form.py) so tests use the
single source of truth; update imports in test_confidence_scorer.py to import
EvidenceStatus (or the exported alias name) from the model module and remove the
local Literal definition to prevent drift when statuses change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2bd5fb7e-846c-4a62-9f1f-856255803698
📒 Files selected for processing (5)
apps/dashboard/src/components/__tests__/NewPAModal.test.tsxapps/intelligence/src/api/analyze.pyapps/intelligence/src/policies/seed/__init__.pyapps/intelligence/src/reasoning/confidence_scorer.pyapps/intelligence/src/tests/test_confidence_scorer.py
Align all demo data surfaces to use LCD_L34220_POLICY labels as the single source of truth for criterion names: - Pre-check, result criteria, and evidence sources now share identical labels (previously 4 different label sets) - Confidence changed from hardcoded 88% to 93%, computed by running the real scoring algorithm against realistic per-criterion evidence confidence values (0.88–0.98) - Fix circular import in intelligence seed/__init__.py (TYPE_CHECKING) - Add test: DEMO_PA_RESULT criteria labels must match LCD policy labels Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace hardcoded DEMO_PRECHECK_CRITERIA with buildPreCheckCriteria() that scans DEMO_ENCOUNTER fields to extract evidence: - Red flag: regex extracts "progressive numbness" snippet from HPI - Conservative mgmt: regex extracts therapy duration from HPI - Clinical rationale: matches "warrant" keyword in assessment - Imaging: checks absence of prior studies All 5 criteria now met from existing chart data, matching the post-sign result (5/5 at 93%). Pre-check becomes the coaching moment: "we already checked — you're covered." Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Introduce a "Document" action on indeterminate pre-check gaps that opens an AI suggestion box, lets the presenter edit and insert text into the encounter note, then save to chart — upgrading criteria from 4/5 to 5/5. Add a submission confirmation receipt dialog with PA details. Enrich the patient header with age, sex, insurance, member ID, and allergies. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
apps/dashboard/src/components/ehr/PAResultsPanel.tsx (1)
18-19: Keep the evidence trail on the request boundary.
PAResultsPanelnow pulls review evidence from the demo-onlyDEMO_PA_RESULT_SOURCESlookup, so any non-demoPARequestor label rename will silently show stale or missing evidence. Passing evidence/source data withpaRequest(or as a prop) keeps this component aligned with its current API surface.Also applies to: 153-169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/ehr/PAResultsPanel.tsx` around lines 18 - 19, PAResultsPanel is reading demo-only DEMO_PA_RESULT_SOURCES inside the component which causes stale/missing evidence for real PARequest objects; change PAResultsPanel to use the evidence/source data coming from the paRequest prop (or add an explicit evidence/source prop) instead of looking up DEMO_PA_RESULT_SOURCES, and update places where EvidenceTag is rendered (see references around PAResultsPanel and the block previously using DEMO_PA_RESULT_SOURCES, including the similar logic at 153-169) so they consume the passed-in evidence fields from paRequest.apps/dashboard/src/lib/demoData.ts (3)
94-100: MakePreCheckCriterionimmutable by default.This interface is exported shared data, so mutable fields make it easy for consumers to patch demo fixtures in place and leak state between renders/tests. Mark the properties
readonlyhere, and considerReadonlyArray<PreCheckCriterion>for the exported lists as well.Suggested type tightening
export interface PreCheckCriterion { - label: string; - status: 'met' | 'not-met' | 'indeterminate'; - evidence?: string; - gap?: string; - source?: string; + readonly label: string; + readonly status: 'met' | 'not-met' | 'indeterminate'; + readonly evidence?: string; + readonly gap?: string; + readonly source?: string; }As per coding guidelines, "Types: Interfaces for extendable shapes, readonly default, no
any".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/demoData.ts` around lines 94 - 100, Update the exported PreCheckCriterion interface so all properties are readonly (e.g., make label, status, evidence, gap, source read-only) to prevent in-place mutation by consumers; then update any exported demo lists (e.g., wherever arrays of PreCheckCriterion are exported) to use ReadonlyArray<PreCheckCriterion> (or readonly PreCheckCriterion[]) to ensure callers cannot mutate fixtures. Also scan for any functions or variables that currently mutate these objects and adjust them to produce new objects instead of mutating in place (references: PreCheckCriterion and the exported demo lists of PreCheckCriterion).
48-58: Make the addendum composition explicit instead of sentinel-basedreplace().Lines 50-56 silently no-op if those anchor phrases are edited later, so the “complete” encounter can drift back toward the incomplete state with no signal. Prefer composing
hpi/assessmentfrom explicit segments, or at least asserting the insertion point exists before replacing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/demoData.ts` around lines 48 - 58, The current DEMO_ENCOUNTER construction mutates hpi and assessment via sentinel-based replace on DEMO_ENCOUNTER_BASE, which silently no-ops if the anchor phrases change; update DEMO_ENCOUNTER to build hpi and assessment explicitly from segments (e.g., base prefix + DEMO_HPI_ADDENDUM + remaining text) or, if keeping replace, assert the anchor exists first and throw/log a clear error when it doesn't; modify the code that references DEMO_ENCOUNTER_BASE.hpi and .assessment and use DEMO_HPI_ADDENDUM and DEMO_ASSESSMENT_ADDENDUM to compose the final strings (or add a check for indexOf before calling replace) so the addendum insertion is explicit and fails loudly if anchors are missing.
230-251: Use the canonical policy labels as the evidence-map keys.This mapping is string-lookup sensitive, and the same labels are repeated again in
DEMO_PA_RESULT.criteriabelow. One rename can silently drop the evidence trail at runtime even thoughLCD_L34220_POLICYis supposed to be the source of truth. Using computed keys from the policy labels keeps the lookup aligned in one place.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/demoData.ts` around lines 230 - 251, DEMO_PA_RESULT_SOURCES currently uses hard-coded string keys that duplicate labels used in DEMO_PA_RESULT.criteria, risking silent mismatches; change DEMO_PA_RESULT_SOURCES to use computed keys derived from the canonical policy label constants (e.g., reference the label names on LCD_L34220_POLICY) so both the source map and DEMO_PA_RESULT.criteria read from the same single source of truth; update any lookups to use those computed keys (instead of literal strings) so renaming a policy label only needs to change LCD_L34220_POLICY and both the evidence map (DEMO_PA_RESULT_SOURCES) and DEMO_PA_RESULT.criteria remain in sync.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/components/ehr/EhrHeader.tsx`:
- Around line 64-66: The member ID span is rendered without a leading space
causing adjacent text to run together; update the JSX where patient.memberId is
checked (the block containing "{patient.memberId && ( <span
className=...>({patient.memberId})</span> )}") to include a visible space before
the "(" — e.g., add a regular space or a non‑breaking space character before the
parenthesis in that span so the output becomes " (ATH60182)".
In `@apps/dashboard/src/components/ehr/EncounterNote.tsx`:
- Around line 172-178: The textarea in EncounterNote.tsx (the element using
textareaRef, value, and setValue) lacks an accessible name; add one by giving
the textarea an id and either associate a visible <label> with htmlFor that id
or add an aria-label/aria-labelledby attribute with descriptive text (e.g., "AI
suggestion" or "Suggested text from AI") so screen readers announce a proper
label for the textbox.
In `@apps/dashboard/src/components/ehr/PAReadinessWidget.tsx`:
- Around line 60-65: The widget currently treats 0/0 as all-met because allMet
is computed as metCount === total; change the logic in PAReadinessWidget so
allMet is only true when there is at least one criterion and all are met (i.e.,
require total > 0 && metCount === total), using the existing variables metCount,
total, criteria and preserving the rest of the rendering logic so the emerald
styling no longer appears for an empty criteria array or while state ===
'checking'.
- Around line 109-123: The row currently chooses Row = 'button' when
onCriterionClick is provided, but the inline "Document" action (rendered when
onGapAction exists) also uses a <button>, creating nested buttons; change the
structure so only one interactive element exists: either make Row a non-button
container (e.g., 'div') and move the row click handler to onClick on that
container with role/button accessibility attributes when onCriterionClick
exists, or keep Row as 'button' but render the "Document" action as a non-button
interactive element (e.g., an anchor or a div with role="button" and onClick) to
avoid nesting; update usages around Row, onCriterionClick, and onGapAction (and
the inline "Document" action rendering) to ensure proper tab/focus behavior and
ARIA roles are preserved.
In `@apps/dashboard/src/lib/demoData.ts`:
- Around line 175-192: The conservative-therapy block must respect a prior
red-flag waiver: when the earlier progressive neurological deficit condition is
met, treat LCD_L34220_POLICY.criteria[2] as satisfied instead of leaving
hasConservative indeterminate; modify the logic around hasConservative /
conservativeSnippet / orderEvidence in the criteria.push for
LCD_L34220_POLICY.criteria[2] to first check the existing
red-flag/neurologic-deficit flag (the same condition used earlier to mark the
progressive neurologic deficit criteria as met) and if true set status: 'met'
and evidence: 'Waived – progressive neurological deficit' (do not fall back to
orderEvidence/MRI), otherwise preserve the current PT/NSAIDs detection using
hpiMentionsPT, hpiMentionsNSAIDs, conservativeSnippet, and only use
orderEvidence when legitimately representing prior conservative care.
In `@apps/dashboard/src/routes/ehr-demo.tsx`:
- Around line 194-196: The results panel is being unmounted when flow.state ===
'error' because showPostSignPanel excludes the 'error' state; change the
condition so PAResultsPanel remains mounted during 'error' (e.g., compute
isSigned as flow.state !== 'idle' && flow.state !== 'flagged', but set
showPostSignPanel to isSigned — not excluding 'error') so PAResultsPanel can
render its ErrorView branch; update the same logic occurrences (including the
block around PAResultsPanel and the similar logic at the 245-258 region) to
ensure error states are passed through to PAResultsPanel rather than hiding the
component.
- Around line 106-169: The portal created by createPortal should be a proper
accessible dialog: add role="dialog" and aria-modal="true" to the outer dialog
container and give the header title an id (e.g.,
"submission-confirmation-title") then set aria-labelledby on the dialog to that
id; ensure the close button (the X element used with onClose) has an accessible
name (aria-label="Close" or a visually-hidden "Close" text). Also add keyboard
and focus management inside the component (useEffect): listen for Escape to call
onClose, save and restore document.activeElement on mount/unmount, move focus
into the dialog (container or first focusable) on mount and implement a simple
focus trap (or use focus-trap-react) so Tab/Shift+Tab cycle within the dialog
while mounted.
---
Nitpick comments:
In `@apps/dashboard/src/components/ehr/PAResultsPanel.tsx`:
- Around line 18-19: PAResultsPanel is reading demo-only DEMO_PA_RESULT_SOURCES
inside the component which causes stale/missing evidence for real PARequest
objects; change PAResultsPanel to use the evidence/source data coming from the
paRequest prop (or add an explicit evidence/source prop) instead of looking up
DEMO_PA_RESULT_SOURCES, and update places where EvidenceTag is rendered (see
references around PAResultsPanel and the block previously using
DEMO_PA_RESULT_SOURCES, including the similar logic at 153-169) so they consume
the passed-in evidence fields from paRequest.
In `@apps/dashboard/src/lib/demoData.ts`:
- Around line 94-100: Update the exported PreCheckCriterion interface so all
properties are readonly (e.g., make label, status, evidence, gap, source
read-only) to prevent in-place mutation by consumers; then update any exported
demo lists (e.g., wherever arrays of PreCheckCriterion are exported) to use
ReadonlyArray<PreCheckCriterion> (or readonly PreCheckCriterion[]) to ensure
callers cannot mutate fixtures. Also scan for any functions or variables that
currently mutate these objects and adjust them to produce new objects instead of
mutating in place (references: PreCheckCriterion and the exported demo lists of
PreCheckCriterion).
- Around line 48-58: The current DEMO_ENCOUNTER construction mutates hpi and
assessment via sentinel-based replace on DEMO_ENCOUNTER_BASE, which silently
no-ops if the anchor phrases change; update DEMO_ENCOUNTER to build hpi and
assessment explicitly from segments (e.g., base prefix + DEMO_HPI_ADDENDUM +
remaining text) or, if keeping replace, assert the anchor exists first and
throw/log a clear error when it doesn't; modify the code that references
DEMO_ENCOUNTER_BASE.hpi and .assessment and use DEMO_HPI_ADDENDUM and
DEMO_ASSESSMENT_ADDENDUM to compose the final strings (or add a check for
indexOf before calling replace) so the addendum insertion is explicit and fails
loudly if anchors are missing.
- Around line 230-251: DEMO_PA_RESULT_SOURCES currently uses hard-coded string
keys that duplicate labels used in DEMO_PA_RESULT.criteria, risking silent
mismatches; change DEMO_PA_RESULT_SOURCES to use computed keys derived from the
canonical policy label constants (e.g., reference the label names on
LCD_L34220_POLICY) so both the source map and DEMO_PA_RESULT.criteria read from
the same single source of truth; update any lookups to use those computed keys
(instead of literal strings) so renaming a policy label only needs to change
LCD_L34220_POLICY and both the evidence map (DEMO_PA_RESULT_SOURCES) and
DEMO_PA_RESULT.criteria remain in sync.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: add8679a-c383-4788-a26e-4c3feeaec036
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (13)
apps/dashboard/src/components/ehr/EhrHeader.tsxapps/dashboard/src/components/ehr/EncounterNote.tsxapps/dashboard/src/components/ehr/PAReadinessWidget.tsxapps/dashboard/src/components/ehr/PAResultsPanel.tsxapps/dashboard/src/components/ehr/__tests__/EncounterSidebar.test.tsxapps/dashboard/src/components/ehr/__tests__/PAReadinessWidget.test.tsxapps/dashboard/src/components/ehr/__tests__/useEhrDemoFlow.test.tsapps/dashboard/src/components/ehr/useEhrDemoFlow.tsapps/dashboard/src/lib/__tests__/demoData.test.tsapps/dashboard/src/lib/demoData.tsapps/dashboard/src/routes/__tests__/ehr-demo.test.tsxapps/dashboard/src/routes/ehr-demo.tsxapps/intelligence/src/policies/seed/__init__.py
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/src/components/ehr/tests/EncounterSidebar.test.tsx
- apps/dashboard/src/routes/tests/ehr-demo.test.tsx
- apps/dashboard/src/components/ehr/useEhrDemoFlow.ts
Comment on lines
+64
to
+66
| {patient.memberId && ( | ||
| <span className="text-xs text-gray-400">({patient.memberId})</span> | ||
| )} |
There was a problem hiding this comment.
Add a space before the member ID.
These adjacent inline spans currently render as ...Blue Cross Blue Shield(ATH60182) because there is no whitespace between them.
✏️ Minimal fix
- {patient.memberId && (
- <span className="text-xs text-gray-400">({patient.memberId})</span>
- )}
+ {patient.memberId && (
+ <span className="text-xs text-gray-400"> ({patient.memberId})</span>
+ )}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {patient.memberId && ( | |
| <span className="text-xs text-gray-400">({patient.memberId})</span> | |
| )} | |
| {patient.memberId && ( | |
| <span className="text-xs text-gray-400"> ({patient.memberId})</span> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ehr/EhrHeader.tsx` around lines 64 - 66, The
member ID span is rendered without a leading space causing adjacent text to run
together; update the JSX where patient.memberId is checked (the block containing
"{patient.memberId && ( <span className=...>({patient.memberId})</span> )}") to
include a visible space before the "(" — e.g., add a regular space or a
non‑breaking space character before the parenthesis in that span so the output
becomes " (ATH60182)".
Comment on lines
+172
to
+178
| <textarea | ||
| ref={textareaRef} | ||
| value={value} | ||
| onChange={(e) => setValue(e.target.value)} | ||
| rows={3} | ||
| className="w-full resize-none rounded border border-blue-100 bg-white p-2.5 text-sm text-gray-700 font-mono leading-relaxed focus:border-blue-300 focus:outline-none focus:ring-1 focus:ring-blue-200" | ||
| /> |
There was a problem hiding this comment.
Label the AI suggestion textarea.
The textarea only has surrounding copy, not an accessible name, so screen readers will announce an unlabeled textbox in the middle of the doc flow.
✏️ Minimal fix
<textarea
ref={textareaRef}
+ aria-label="Edit suggested text before adding it to the note"
value={value}
onChange={(e) => setValue(e.target.value)}
rows={3}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <textarea | |
| ref={textareaRef} | |
| value={value} | |
| onChange={(e) => setValue(e.target.value)} | |
| rows={3} | |
| className="w-full resize-none rounded border border-blue-100 bg-white p-2.5 text-sm text-gray-700 font-mono leading-relaxed focus:border-blue-300 focus:outline-none focus:ring-1 focus:ring-blue-200" | |
| /> | |
| <textarea | |
| ref={textareaRef} | |
| aria-label="Edit suggested text before adding it to the note" | |
| value={value} | |
| onChange={(e) => setValue(e.target.value)} | |
| rows={3} | |
| className="w-full resize-none rounded border border-blue-100 bg-white p-2.5 text-sm text-gray-700 font-mono leading-relaxed focus:border-blue-300 focus:outline-none focus:ring-1 focus:ring-blue-200" | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ehr/EncounterNote.tsx` around lines 172 - 178,
The textarea in EncounterNote.tsx (the element using textareaRef, value, and
setValue) lacks an accessible name; add one by giving the textarea an id and
either associate a visible <label> with htmlFor that id or add an
aria-label/aria-labelledby attribute with descriptive text (e.g., "AI
suggestion" or "Suggested text from AI") so screen readers announce a proper
label for the textbox.
Comment on lines
+60
to
+65
| const metCount = criteria.filter((c) => c.status === 'met').length; | ||
| const total = criteria.length; | ||
| const allMet = metCount === total; | ||
|
|
||
| return ( | ||
| <div className={`rounded-lg border-l-4 bg-white shadow-sm transition-colors duration-700 ${allMet ? 'border-emerald-500' : 'border-amber-500'}`}> |
There was a problem hiding this comment.
Don’t treat 0/0 as “all criteria met.”
When state="checking" and criteria=[], allMet becomes true, so the card renders the emerald ready styling before analysis finishes.
✏️ Minimal fix
- const allMet = metCount === total;
+ const allMet = state === 'ready' && total > 0 && metCount === total;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const metCount = criteria.filter((c) => c.status === 'met').length; | |
| const total = criteria.length; | |
| const allMet = metCount === total; | |
| return ( | |
| <div className={`rounded-lg border-l-4 bg-white shadow-sm transition-colors duration-700 ${allMet ? 'border-emerald-500' : 'border-amber-500'}`}> | |
| const metCount = criteria.filter((c) => c.status === 'met').length; | |
| const total = criteria.length; | |
| const allMet = state === 'ready' && total > 0 && metCount === total; | |
| return ( | |
| <div className={`rounded-lg border-l-4 bg-white shadow-sm transition-colors duration-700 ${allMet ? 'border-emerald-500' : 'border-amber-500'}`}> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ehr/PAReadinessWidget.tsx` around lines 60 -
65, The widget currently treats 0/0 as all-met because allMet is computed as
metCount === total; change the logic in PAReadinessWidget so allMet is only true
when there is at least one criterion and all are met (i.e., require total > 0 &&
metCount === total), using the existing variables metCount, total, criteria and
preserving the rest of the rendering logic so the emerald styling no longer
appears for an empty criteria array or while state === 'checking'.
Comment on lines
+109
to
+123
| const isClickable = !!onCriterionClick; | ||
| const Row = isClickable ? 'button' : 'div'; | ||
|
|
||
| return ( | ||
| <li key={criterion.label}> | ||
| <Row | ||
| type={isClickable ? 'button' : undefined} | ||
| className={`flex w-full items-start gap-2 py-2.5 px-1 text-left transition-colors duration-500 ${ | ||
| isClickable ? 'hover:bg-slate-50 rounded cursor-pointer' : '' | ||
| }`} | ||
| onClick={ | ||
| isClickable | ||
| ? () => onCriterionClick(criterion) | ||
| : undefined | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd PAReadinessWidget.tsxRepository: lvlup-sw/authscript
Length of output: 118
🏁 Script executed:
cat -n apps/dashboard/src/components/ehr/PAReadinessWidget.tsxRepository: lvlup-sw/authscript
Length of output: 8897
Refactor to avoid nesting button elements.
When both onCriterionClick and onGapAction are provided, Row becomes a <button> and the inline "Document" action (lines 149–159) renders another <button> inside it. This creates invalid HTML structure and breaks focus/click behavior on indeterminate rows. Consider restructuring so the row content or the action link is not directly nested within the button, or use only one interactive pattern.
Also applies to: 149-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/components/ehr/PAReadinessWidget.tsx` around lines 109 -
123, The row currently chooses Row = 'button' when onCriterionClick is provided,
but the inline "Document" action (rendered when onGapAction exists) also uses a
<button>, creating nested buttons; change the structure so only one interactive
element exists: either make Row a non-button container (e.g., 'div') and move
the row click handler to onClick on that container with role/button
accessibility attributes when onCriterionClick exists, or keep Row as 'button'
but render the "Document" action as a non-button interactive element (e.g., an
anchor or a div with role="button" and onClick) to avoid nesting; update usages
around Row, onCriterionClick, and onGapAction (and the inline "Document" action
rendering) to ensure proper tab/focus behavior and ARIA roles are preserved.
Comment on lines
+175
to
+192
| // 3. Conservative management — scan HPI for therapy duration | ||
| const hpiMentionsPT = /\d+\s*weeks?\s*(of\s+)?physical therapy/i.test(encounter.hpi); | ||
| const hpiMentionsNSAIDs = /\d+\s*weeks?\s*(of\s+)?NSAIDs/i.test(encounter.hpi); | ||
| const hasConservative = hpiMentionsPT || hpiMentionsNSAIDs; | ||
| const conservativeSnippet = encounter.hpi | ||
| .match(/failed\s+\d+\s+weeks?[^.]*\./i)?.[0]; | ||
| const orderEvidence = DEMO_ORDERS.length > 0 | ||
| ? `Active order: ${DEMO_ORDERS[0].name}` | ||
| : undefined; | ||
| criteria.push({ | ||
| label: LCD_L34220_POLICY.criteria[2].label, | ||
| status: hasConservative ? 'met' : 'indeterminate', | ||
| evidence: hasConservative | ||
| ? (conservativeSnippet?.trim() ?? orderEvidence) | ||
| : undefined, | ||
| source: hasConservative ? 'HPI / Orders' : undefined, | ||
| gap: hasConservative ? undefined : 'Conservative therapy documentation not found in HPI', | ||
| }); |
There was a problem hiding this comment.
Honor the red-flag waiver before marking conservative therapy missing.
Line 169 can already mark progressive neurological deficit as met, but Lines 186-191 still leave 4+ weeks conservative management indeterminate unless PT/NSAIDs are documented. That conflicts with the requirement text on Lines 124-126 (unless red flag symptoms are present) and makes the base encounter look noncompliant when the policy says the waiver path is satisfied. The orderEvidence fallback also risks showing the MRI order as proof of prior conservative care.
Possible fix
const hpiMentionsPT = /\d+\s*weeks?\s*(of\s+)?physical therapy/i.test(encounter.hpi);
const hpiMentionsNSAIDs = /\d+\s*weeks?\s*(of\s+)?NSAIDs/i.test(encounter.hpi);
const hasConservative = hpiMentionsPT || hpiMentionsNSAIDs;
const conservativeSnippet = encounter.hpi
.match(/failed\s+\d+\s+weeks?[^.]*\./i)?.[0];
- const orderEvidence = DEMO_ORDERS.length > 0
- ? `Active order: ${DEMO_ORDERS[0].name}`
- : undefined;
criteria.push({
label: LCD_L34220_POLICY.criteria[2].label,
- status: hasConservative ? 'met' : 'indeterminate',
+ status: hasConservative || hasRedFlag ? 'met' : 'indeterminate',
evidence: hasConservative
- ? (conservativeSnippet?.trim() ?? orderEvidence)
+ ? conservativeSnippet?.trim()
+ : hasRedFlag
+ ? redFlagSnippet?.trim()
: undefined,
- source: hasConservative ? 'HPI / Orders' : undefined,
- gap: hasConservative ? undefined : 'Conservative therapy documentation not found in HPI',
+ source: hasConservative ? 'HPI' : hasRedFlag ? 'CC / HPI' : undefined,
+ gap:
+ hasConservative || hasRedFlag
+ ? undefined
+ : 'Conservative therapy documentation not found in HPI',
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/demoData.ts` around lines 175 - 192, The
conservative-therapy block must respect a prior red-flag waiver: when the
earlier progressive neurological deficit condition is met, treat
LCD_L34220_POLICY.criteria[2] as satisfied instead of leaving hasConservative
indeterminate; modify the logic around hasConservative / conservativeSnippet /
orderEvidence in the criteria.push for LCD_L34220_POLICY.criteria[2] to first
check the existing red-flag/neurologic-deficit flag (the same condition used
earlier to mark the progressive neurologic deficit criteria as met) and if true
set status: 'met' and evidence: 'Waived – progressive neurological deficit' (do
not fall back to orderEvidence/MRI), otherwise preserve the current PT/NSAIDs
detection using hpiMentionsPT, hpiMentionsNSAIDs, conservativeSnippet, and only
use orderEvidence when legitimately representing prior conservative care.
Comment on lines
+106
to
+169
| return createPortal( | ||
| <div | ||
| className="fixed inset-0 z-[9999] flex items-center justify-center bg-black/60 backdrop-blur-sm" | ||
| onClick={onClose} | ||
| > | ||
| <div | ||
| className="relative w-full max-w-md mx-4 overflow-hidden rounded-2xl border border-gray-200 bg-white shadow-2xl" | ||
| onClick={(e) => e.stopPropagation()} | ||
| > | ||
| {/* Header */} | ||
| <div className="flex items-center justify-between border-b border-gray-200 px-5 py-4"> | ||
| <div className="flex items-center gap-3"> | ||
| <div className="flex h-8 w-8 items-center justify-center rounded-lg bg-green-100"> | ||
| <CheckCircle2 className="h-4 w-4 text-green-600" /> | ||
| </div> | ||
| <div> | ||
| <div className="text-sm font-semibold text-gray-900"> | ||
| Submission Confirmation | ||
| </div> | ||
| <div className="text-xs text-gray-500"> | ||
| athenahealth DocumentReference | ||
| </div> | ||
| </div> | ||
| </div> | ||
| <button | ||
| type="button" | ||
| onClick={onClose} | ||
| className="rounded-lg p-2 transition-colors hover:bg-gray-100" | ||
| > | ||
| <X className="h-5 w-5 text-gray-500" /> | ||
| </button> | ||
| </div> | ||
|
|
||
| {/* Receipt rows */} | ||
| <div className="divide-y divide-gray-100 px-5"> | ||
| {rows.map((row) => ( | ||
| <div key={row.label} className="flex items-start gap-3 py-3"> | ||
| <span className="mt-0.5">{row.icon}</span> | ||
| <div className="min-w-0 flex-1"> | ||
| <div className="text-xs font-medium uppercase tracking-wider text-gray-400"> | ||
| {row.label} | ||
| </div> | ||
| <div className="mt-0.5 text-sm text-gray-900 break-words"> | ||
| {row.value} | ||
| </div> | ||
| </div> | ||
| </div> | ||
| ))} | ||
| </div> | ||
|
|
||
| {/* Footer */} | ||
| <div className="border-t border-gray-200 bg-slate-50 px-5 py-3"> | ||
| <div className="flex items-center gap-2 text-xs text-slate-500"> | ||
| <Shield className="h-3.5 w-3.5" /> | ||
| <span> | ||
| PA form written to patient chart via{' '} | ||
| <span className="font-mono">POST /fhir/r4/DocumentReference</span> | ||
| </span> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div>, | ||
| document.body, | ||
| ); |
There was a problem hiding this comment.
Make the confirmation overlay an actual dialog.
This portal lacks dialog semantics (role="dialog", aria-modal), the close button has no accessible name, and there is no Escape/focus management. Keyboard and screen-reader users won’t get a reliable modal interaction here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/routes/ehr-demo.tsx` around lines 106 - 169, The portal
created by createPortal should be a proper accessible dialog: add role="dialog"
and aria-modal="true" to the outer dialog container and give the header title an
id (e.g., "submission-confirmation-title") then set aria-labelledby on the
dialog to that id; ensure the close button (the X element used with onClose) has
an accessible name (aria-label="Close" or a visually-hidden "Close" text). Also
add keyboard and focus management inside the component (useEffect): listen for
Escape to call onClose, save and restore document.activeElement on
mount/unmount, move focus into the dialog (container or first focusable) on
mount and implement a simple focus trap (or use focus-trap-react) so
Tab/Shift+Tab cycle within the dialog while mounted.
Comment on lines
+194
to
+196
| const isSigned = flow.state !== 'idle' && flow.state !== 'flagged'; | ||
| const showPostSignPanel = isSigned && flow.state !== 'error'; | ||
|
|
There was a problem hiding this comment.
Keep the results panel mounted in the error state.
showPostSignPanel filters out flow.state === 'error', so PAResultsPanel never reaches its ErrorView branch. A failed sign or submit will currently drop the whole panel instead of surfacing the error.
✏️ Minimal fix
- const showPostSignPanel = isSigned && flow.state !== 'error';
+ const showPostSignPanel = isSigned;Also applies to: 245-258
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/routes/ehr-demo.tsx` around lines 194 - 196, The results
panel is being unmounted when flow.state === 'error' because showPostSignPanel
excludes the 'error' state; change the condition so PAResultsPanel remains
mounted during 'error' (e.g., compute isSigned as flow.state !== 'idle' &&
flow.state !== 'flagged', but set showPostSignPanel to isSigned — not excluding
'error') so PAResultsPanel can render its ErrorView branch; update the same
logic occurrences (including the block around PAResultsPanel and the similar
logic at the 245-258 region) to ensure error states are passed through to
PAResultsPanel rather than hiding the component.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Extends the EHR demo with a policy intelligence pre-check that demonstrates AuthScript's moat: the structured payer policy knowledge base.
Demo narrative: flag -> coach -> generate -> submit. Doctor never leaves the chart. Amber-to-blue transition visualizes the policy engine at work.
Test plan
Generated with Claude Code