Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughRemoves dashboard Docker/nginx artifacts and many legacy UI hooks/utils; adds Node 20 configs; large dashboard rewrite with new components, routes (including /help), Teal theme, localStorage-backed PA store, mock data, PDF generator, EHR exit helpers, and new NewPAModal+processing flow. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Modal as NewPAModal
participant Store as PA Store\n(localStorage)
participant Processor as PA Processor\n(simulated)
participant Router as Router
User->>Modal: open modal / select patient, service, diagnosis
User->>Modal: submit selection
Modal->>Store: createPARequest(...) → returns id
Modal->>Processor: processPARequest(id)
Processor->>Processor: simulate AI work → compute confidence & criteria
Processor->>Store: updatePARequest(id, { status: 'ready', confidence, criteria })
Store->>Store: persist to localStorage
Modal->>Router: navigate to /analysis/$id
Router->>User: show analysis page
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 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)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…afonso/feature/fhirIntegrationPrototype
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/dashboard/src/components/__tests__/FormPreview.test.tsx (3)
39-53:⚠️ Potential issue | 🟡 MinorThese tests will fail due to implementation mismatches.
Two issues:
- Line 43/50:
.closest('[data-field]')returnsnullbecause the component doesn't adddata-fieldattributes- Line 44: CSS class
bg-[hsl(var(--success)/0.1)]doesn't exist; component usesbg-success/5Update tests to match the actual component implementation, or add the missing attributes/classes to the component.
Option: Update tests to match component
describe('field highlighting', () => { it('FormPreview_FilledField_HasHighlight', () => { render(<FormPreview fieldMappings={mockFieldMappings} showHighlights />); - const field = screen.getByText('John Doe').closest('[data-field]'); - expect(field).toHaveClass('bg-[hsl(var(--success)/0.1)]'); + const field = screen.getByText('John Doe').closest('div'); + expect(field?.parentElement).toHaveClass('bg-success/5'); }); it('FormPreview_WithoutHighlights_NoHighlightClass', () => { render(<FormPreview fieldMappings={mockFieldMappings} showHighlights={false} />); - const field = screen.getByText('John Doe').closest('[data-field]'); - expect(field).not.toHaveClass('bg-[hsl(var(--success)/0.1)]'); + const field = screen.getByText('John Doe').closest('div'); + expect(field?.parentElement).not.toHaveClass('bg-success/5'); }); });Note: A cleaner approach would be adding the
data-fieldattribute to the component as noted in the component review.
55-68:⚠️ Potential issue | 🟡 MinorLoading tests will fail:
data-testid="form-skeleton"missing from component.Both tests query for
form-skeletontest ID which doesn't exist on the component's skeleton container. Add thedata-testidattribute to the component as noted in the component review.
79-88:⚠️ Potential issue | 🟡 MinorSame
data-fieldattribute issue affects this test.Line 85's
.closest('[data-field]')will returnnullfor the same reason as the highlighting tests.Suggested fix
it('FormPreview_DiagnosisCodes_DisplaysFormatted', () => { render(<FormPreview fieldMappings={mockFieldMappings} />); expect(screen.getByText(/M54.5/)).toBeInTheDocument(); - // Check that the diagnosis codes field contains the expected value - const diagnosisField = screen.getByText('Diagnosis Codes').closest('[data-field]'); - expect(diagnosisField).toHaveTextContent('M54.5 - Low back pain'); + // Check that the diagnosis codes value is displayed + expect(screen.getByText('M54.5 - Low back pain')).toBeInTheDocument(); });docs/ARCHITECTURE.md (1)
101-134:⚠️ Potential issue | 🟡 MinorAdd language specifier to project structure block.
Similar to above, add a language specifier for consistency and to satisfy linting.
📝 Proposed fix
-``` +```text authscript/ ├── apps/
🤖 Fix all issues with AI agents
In `@apps/dashboard/src/components/FormPreview.tsx`:
- Around line 101-139: The long-form field container rendered in the
longFormFields.map (the div keyed by key inside the longFormFields mapping) is
missing the data-field attribute that tests expect; update that div to include
data-field={key} (or data-field={`long-${key}`} if you need namespace
consistency) so each long-form block exposes the field identifier for
testing—modify the div inside the longFormFields.map (the same element using
cn(...) and containing formatFieldName(key) and the value paragraph) to include
the data-field attribute.
- Around line 28-41: The loading branch in the FormPreview component returns a
skeleton container without the required test id; update the JSX returned when
loading is true (the outermost div rendered inside the if (loading) block in
FormPreview) to include data-testid="form-skeleton" so tests can find the
skeleton; ensure you add the attribute to the same div that uses cn('space-y-4',
className) and do not change other structure or classNames.
- Around line 76-98: The field container rendered in the regularFields.map lacks
the data-field attribute and uses a Tailwind color class that doesn't match
tests; update the mapped div returned in the regularFields.map (the element
using cn and keyed by key) to include a data-field={key} attribute and change
the success background class branch from 'bg-success/5' to the test-expected
'bg-[hsl(var(--success)/0.1)]' (ensure the conditional still uses showHighlights
and value); keep the existing border and hover classes and leave
formatFieldName, showHighlights, and the Edit3 usage unchanged.
In `@apps/dashboard/src/components/LoadingSpinner.tsx`:
- Around line 9-13: The sizeClasses map uses a non-standard Tailwind class
'border-3' for the 'md' key, which will be ignored; update the sizeClasses
constant (the md entry) in LoadingSpinner.tsx to use a valid Tailwind border
utility (e.g., 'border' for 1px or 'border-2'/'border-4' as needed) or, if you
truly require a 3px border, add a '3': '3px' entry under
theme.extend.borderWidth in your Tailwind config and then keep 'border-3' in
sizeClasses; ensure the change targets the sizeClasses object and the 'md'
mapping specifically.
- Around line 114-116: In LoadingSpinner where steps.map((step, index) => ...)
uses key={step}, replace the unstable key with a stable unique key (e.g.,
key={`${step}-${index}`} or use an explicit id property if steps are objects) to
avoid React reconciliation issues when steps contains duplicate strings; update
the key expression in the JSX returned by the map inside the LoadingSpinner
component accordingly.
In `@apps/dashboard/src/components/NewPAModal.tsx`:
- Around line 167-172: The close button in NewPAModal lacks an accessible label;
update the button element that calls resetAndClose (the one rendering the <X />
icon) to include an appropriate aria-label (e.g., aria-label="Close" or
aria-label="Close modal") so screen readers can announce its purpose, ensuring
the attribute sits on the same button that has className="p-2 rounded-lg
hover:bg-gray-100 transition-colors click-effect" and onClick={resetAndClose}.
- Around line 104-121: The handler handleDiagnosisSelect uses non-null
assertions on selectedPatient and selectedService when calling createPARequest;
replace the assertions with a defensive guard: at the start of
handleDiagnosisSelect check that selectedPatient and selectedService are
present, otherwise early-return (and surface an error/toast/log) so you don't
call createPARequest with undefined values; then call createPARequest with the
validated values, proceed to await processPARequest(newRequest.id), and continue
the existing resetAndClose + navigate flow. Ensure you reference
handleDiagnosisSelect, selectedPatient, selectedService, createPARequest,
processPARequest, resetAndClose, and navigate when making the change.
In `@apps/dashboard/src/components/PARequestCard.tsx`:
- Around line 40-51: The formatTimeAgo function doesn't handle invalid date
strings and can produce "NaN ago"; update formatTimeAgo to validate the parsed
date (e.g., check isNaN(date.getTime()) or isNaN(date.valueOf())) right after
creating date and return a safe fallback like 'Unknown' or an empty string when
invalid, before computing diffMs; keep the rest of the logic unchanged and
reference formatTimeAgo to locate and modify the code.
In `@apps/dashboard/src/components/WorkflowProgress.tsx`:
- Around line 125-134: The compact label branch in WorkflowProgress.tsx omits
styling for the 'error' status, so update the conditional class list used where
compact is true (the <p> rendering step.label) to include the same error class
used in the full mode; specifically add the status === 'error' &&
'text-destructive' clause alongside the existing status checks (status ===
'active', status === 'completed', status === 'pending') so error labels receive
the proper styling.
- Around line 85-105: Update the step container div in WorkflowProgress (the
element using cn(...) and rendering Check/Icon) to expose semantic info for
assistive tech: add an accessible label and state attributes (e.g.
aria-label={`Step ${stepIndex + 1}: ${status}`} or similar using your step
index/label prop), set aria-current="step" when status === 'active', and if this
component represents a measurable progress add role="progressbar" with
aria-valuemin="0" aria-valuemax="{totalSteps}" and aria-valuenow="{stepIndex +
1}"; ensure you reference the existing status variable and the same container
that renders Check/Icon so screen readers receive the step/state information.
In `@apps/dashboard/src/lib/pdfGenerator.ts`:
- Around line 5-350: The generatePAPdf function is injecting user-controlled
values directly into the HTML (e.g., request.patient.name,
request.clinicalSummary, request.criteria[*].label, request.payer,
request.provider, request.procedureName, etc.) and needs HTML-escaping to
prevent XSS; add a small escapeHtml(unsafe: string): string utility and call it
wherever you interpolate potentially untrusted strings in generatePAPdf (wrap
each usage like request.patient.name, request.patient.address,
request.clinicalSummary, request.criteria.map(...label...), request.payer,
request.provider, request.procedureName, request.diagnosis, request.serviceDate,
request.placeOfService, request.id, and any other string fields), ensuring
non-string values are coerced to strings before escaping (e.g., String(value ||
'')) so null/undefined are safe.
In `@apps/dashboard/src/lib/store.ts`:
- Around line 115-143: The ID generation in createPARequest currently uses
requests.length + 1 which can collide after deletions; change newId creation to
scan existing PARequest ids from getPARequests() (e.g., items in requests),
extract the numeric suffix from ids like "PA-###", compute the max numeric value
(treat non-matching ids as 0), increment it and pad to produce the next unique
suffix, then use that value for newId before saving via savePARequests and
setting the cookie CURRENT_PA_KEY.
- Around line 197-206: When deleting a PA in deletePARequest, also clear the
"current PA" cookie if the removed id matches getCurrentPAId() to avoid a stale
current ID; after computing filtered and before returning true, check if
getCurrentPAId() === id and then call the existing setter/helper to clear it
(e.g., setCurrentPAId(null) or removeCurrentPAIdCookie()), or if no helper
exists remove the cookie named "currentPAId" directly, then call
savePARequests(filtered) and return true.
In `@apps/dashboard/src/routes/__root.tsx`:
- Around line 31-40: Update the search input element in the __root.tsx
component: change its type from "text" to "search" and add an accessible name
via aria-label (for example "Search patients and PA requests") so screen readers
have a clear label; ensure the input element you modify is the one inside the
Search Bar div (the element with className "bg-transparent text-sm outline-none
flex-1 text-white placeholder:text-slate-400") and do not rely on the
placeholder alone for accessibility.
- Around line 53-64: The icon-only controls containing the Bell, HelpCircle, and
Settings icons lack accessible names; add descriptive aria-label attributes to
each interactive element (the notification <button> with Bell, the <Link
to="/help"> with HelpCircle, and the settings <button> with Settings) so screen
readers can identify them (e.g., aria-label="Notifications", aria-label="Help",
aria-label="Settings"), ensuring the labels are clear and localizable if needed.
In `@apps/dashboard/src/routes/analysis`.$transactionId.tsx:
- Around line 136-154: The clickable criteria tile currently implemented as a
<div> should be converted to a semantic <button> to enable keyboard access:
replace the outer <div> (the element using cn(...), isEditing, onToggle) with a
<button type="button">, preserve the same className composition (cn('flex
items-center...', style.bg, style.border, isEditing && 'cursor-pointer
hover:opacity-80')), wire onClick to onToggle when isEditing, and add
accessibility attributes such as aria-pressed (derive from met: true/false ->
"true"/"false", null -> "mixed" or omitted) and an appropriate aria-label (use
label) so screen readers get the intent; keep inner elements (icon div using met
and Check/X/AlertCircle, span for label, and "Click to toggle") unchanged.
In `@apps/dashboard/src/routes/smart-launch.tsx`:
- Around line 54-59: handleAnalyze sets status then navigates to
'/analysis/$transactionId' using a fake id `pa-${Date.now()}` which won't exist
in the store; instead create a real PA request (or obtain the transaction id
from your launch/analysis flow) before calling navigate. Update handleAnalyze to
call the existing PA creation function or dispatch the action that returns a
real transaction id (e.g., createPARequest / launchTransaction / submitLaunch)
and use that returned id in navigate({ to: '/analysis/$transactionId', params: {
transactionId } }), keeping the setStatus behavior and the delay if desired.
In `@docs/ARCHITECTURE.md`:
- Around line 30-38: The fenced ASCII diagram block is missing a language
specifier; edit the triple-backtick block that contains the diagram (the ASCII
diagram starting with "┌─────────┐ ┌─────────┐...") and add a language like
text or plaintext (e.g., ```text) immediately after the opening backticks so the
static analyzer recognizes it as plain text.
🧹 Nitpick comments (23)
apps/dashboard/src/index.css (3)
64-68: Consider adding Firefox scrollbar support for cross-browser consistency.The
::-webkit-scrollbarpseudo-elements only work in Chromium-based browsers and Safari. Firefox uses different properties.♻️ Add Firefox scrollbar support
/* Scrollbar */ +* { + scrollbar-width: thin; + scrollbar-color: hsl(var(--muted-foreground) / 0.2) hsl(var(--muted) / 0.5); +} + ::-webkit-scrollbar { width: 8px; height: 8px; } ::-webkit-scrollbar-track { background: hsl(var(--muted) / 0.5); } ::-webkit-scrollbar-thumb { background: hsl(var(--muted-foreground) / 0.2); border-radius: 4px; } ::-webkit-scrollbar-thumb:hover { background: hsl(var(--muted-foreground) / 0.3); }
206-208: Potential conflict with Tailwind's built-inanimate-spin.Tailwind CSS provides a built-in
animate-spinclass (1s linear infinite). This redefinition uses 0.8s, which could cause unexpected behavior if both are loaded, depending on CSS specificity and load order.♻️ Rename to avoid conflict
-.animate-spin { - animation: spin 0.8s linear infinite; -} +.animate-spin-fast { + animation: spin 0.8s linear infinite; +}Alternatively, if the intent is to override Tailwind's default, document this clearly with a comment.
184-189: Stagger classes require initial hidden state to work correctly.These stagger delay classes only set
animation-delay. For staggered fade/slide animations to work, elements also needopacity: 0initially (or useanimation-fill-mode: backwardson the animated element) so they're hidden until their delayed animation starts.♻️ Option: Add initial opacity or use animation-fill-mode
/* Staggered animations for lists */ -.stagger-1 { animation-delay: 0.05s; } -.stagger-2 { animation-delay: 0.1s; } -.stagger-3 { animation-delay: 0.15s; } -.stagger-4 { animation-delay: 0.2s; } -.stagger-5 { animation-delay: 0.25s; } +.stagger-1 { animation-delay: 0.05s; opacity: 0; animation-fill-mode: forwards; } +.stagger-2 { animation-delay: 0.1s; opacity: 0; animation-fill-mode: forwards; } +.stagger-3 { animation-delay: 0.15s; opacity: 0; animation-fill-mode: forwards; } +.stagger-4 { animation-delay: 0.2s; opacity: 0; animation-fill-mode: forwards; } +.stagger-5 { animation-delay: 0.25s; opacity: 0; animation-fill-mode: forwards; }Alternatively, ensure all components using stagger classes also apply the appropriate animation class that sets
animation-fill-mode: forwards.apps/dashboard/src/components/LoadingSpinner.tsx (2)
80-92: Consider responsive grid columns.
grid-cols-4renders all four stat cards in a row, which may overflow or compress on smaller viewports.Responsive variant
- <div className="grid grid-cols-4 gap-4"> + <div className="grid grid-cols-2 md:grid-cols-4 gap-4">
168-178: Loading state layout has conflicting positioning.When
loadingTextis provided, both the spinner (line 169) and the text (line 175) are absolutely positioned at center. Thepl-6padding pushes the text right, but the spinner remains centered, creating visual misalignment rather than a cohesive spinner-and-text layout.Cleaner loading content layout
{loading && ( - <LoadingSpinner size="sm" className="absolute left-1/2 -translate-x-1/2" /> + <span className="absolute inset-0 flex items-center justify-center gap-2"> + <LoadingSpinner size="sm" /> + {loadingText && <span className="whitespace-nowrap">{loadingText}</span>} + </span> )} <span className={cn(loading && 'opacity-0')}> {children} </span> - {loading && loadingText && ( - <span className="absolute left-1/2 -translate-x-1/2 whitespace-nowrap pl-6"> - {loadingText} - </span> - )}apps/dashboard/package.json (1)
9-11: Redundant override entry.This
@tanstack/router-coreoverride is duplicated in the rootpackage.json(lines 37-39). In npm workspaces, root-level overrides apply to all workspaces, making this entry unnecessary. Consider removing it to avoid maintenance drift if only one location gets updated.♻️ Proposed fix
- "overrides": { - "@tanstack/router-core": "1.131.35" - },Keep only the root-level override.
apps/dashboard/src/components/EvidencePanel.tsx (1)
79-83: Optional: compute counts in a single pass.
Avoids three filter scans and keeps the summary derivation tight.♻️ Suggested refactor
- const counts = { - met: evidence.filter(e => e.status === 'MET').length, - notMet: evidence.filter(e => e.status === 'NOT_MET').length, - unclear: evidence.filter(e => e.status === 'UNCLEAR').length, - }; + const counts = evidence.reduce( + (acc, e) => { + if (e.status === 'MET') acc.met += 1; + else if (e.status === 'NOT_MET') acc.notMet += 1; + else acc.unclear += 1; + return acc; + }, + { met: 0, notMet: 0, unclear: 0 } + );Also applies to: 88-109
apps/dashboard/src/components/NewPAModal.tsx (2)
44-54: Unused state variable_selectedDiagnosis.The
_selectedDiagnosisstate is set inhandleDiagnosisSelectbut never read. Either remove it or use it (e.g., for displaying the selected diagnosis in the UI).🧹 Option: Remove if not needed
- const [_selectedDiagnosis, setSelectedDiagnosis] = useState<{ code: string; name: string } | null>(null);And remove
setSelectedDiagnosis(diagnosis)fromhandleDiagnosisSelect, andsetSelectedDiagnosis(null)fromresetAndClose.
56-70: Interval continues running after reaching final step.When
processingStepreaches the last index, the interval keeps firing (returning the same value). Clear the interval when processing completes.♻️ Proposed fix
useEffect(() => { if (step === 'processing') { + let intervalId: ReturnType<typeof setInterval>; - const interval = setInterval(() => { + intervalId = setInterval(() => { setProcessingStep(prev => { if (prev < PROCESSING_STEPS.length - 1) { return prev + 1; } + clearInterval(intervalId); return prev; }); }, 800); - return () => clearInterval(interval); + return () => clearInterval(intervalId); } else { setProcessingStep(0); } }, [step]);apps/dashboard/src/components/ConfidenceMeter.tsx (1)
96-104: Threshold markers hardcode values that duplicate THRESHOLDS constant.The marker labels (50%, 80%) are hardcoded while the actual thresholds are defined in
THRESHOLDS. If thresholds change, these markers would become inconsistent.♻️ Suggested approach
+ <div className="flex justify-between text-[10px] text-muted-foreground"> + <span>0%</span> + <span className="text-warning">{THRESHOLDS.medium * 100}%</span> + <span className="text-success">{THRESHOLDS.high * 100}%</span> + <span>100%</span> + </div>apps/dashboard/src/components/PolicyChecklist.tsx (1)
46-52:transactionIdis unused - criteria are hardcoded.The
transactionIdprop is accepted but not used to fetch actual criteria. For a prototype this is acceptable, but consider adding a TODO or using the ID to look up data from the store.💡 Consider for future
-export function PolicyChecklist({ transactionId: _transactionId, className }: PolicyChecklistProps) { - const criteria: PolicyCriterion[] = [ +export function PolicyChecklist({ transactionId, className }: PolicyChecklistProps) { + // TODO: Fetch criteria from store based on transactionId + // const request = getPARequest(transactionId); + // const criteria = request?.criteria ?? []; + const criteria: PolicyCriterion[] = [apps/dashboard/src/lib/mockData.ts (1)
157-169: Consider adding explicit type annotation to DIAGNOSES.Unlike other arrays,
DIAGNOSESlacks an explicit interface type, relying on inference. For consistency withPATIENTS,PROCEDURES, etc., consider defining aDiagnosisinterface.♻️ Suggested addition
+export interface Diagnosis { + code: string; + name: string; +} + -export const DIAGNOSES = [ +export const DIAGNOSES: Diagnosis[] = [ { code: 'M54.5', name: 'Low Back Pain' },apps/dashboard/src/lib/pdfGenerator.ts (1)
352-382:openPAPdfanddownloadPAPdfare nearly identical.Both functions generate HTML, open a window, write content, and trigger print. The only difference is a 250ms delay in
openPAPdf. Consider consolidating.♻️ Proposed refactor
-export function openPAPdf(request: PARequest) { +function printPAHtml(html: string, delay: number = 0): void { - const html = generatePAPdf(request); const printWindow = window.open('', '_blank'); if (printWindow) { printWindow.document.write(html); printWindow.document.close(); - printWindow.onload = () => { - setTimeout(() => { - printWindow.print(); - }, 250); + if (delay > 0) { + setTimeout(() => printWindow.print(), delay); + } else { + printWindow.print(); + } }; } } -export function downloadPAPdf(request: PARequest) { - const html = generatePAPdf(request); - const printWindow = window.open('', '_blank'); - - if (printWindow) { - printWindow.document.write(html); - printWindow.document.close(); - printWindow.onload = () => { - printWindow.print(); - }; - } +export function openPAPdf(request: PARequest): void { + printPAHtml(generatePAPdf(request), 250); +} + +export function downloadPAPdf(request: PARequest): void { + printPAHtml(generatePAPdf(request)); }apps/dashboard/src/routes/help.tsx (2)
24-80: Consider extracting tab content/Step into separate files.
This file defines several components; splitting them keeps to the “one component per file” standard and improves maintainability. As per coding guidelines: Components: One component per file, use functional components with hooks.
82-94: Define a props interface forStep.
Using an interface keeps prop shapes consistent and extendable. As per coding guidelines: Props: Define interface for props, use destructuring.♻️ Suggested refactor
+interface StepProps { + number: number; + title: string; + children: React.ReactNode; +} -function Step({ number, title, children }: { number: number; title: string; children: React.ReactNode }) { +function Step({ number, title, children }: StepProps) {apps/dashboard/src/routes/analysis.$transactionId.tsx (2)
38-156: Use interfaces for helper component props.
This keeps prop shapes extendable and consistent acrossProgressRing,EditableField, andCriteriaItem. As per coding guidelines: Props: Define interface for props, use destructuring.♻️ Example refactor (apply similarly to the other helpers)
+interface ProgressRingProps { + value: number; + size?: number; +} -function ProgressRing({ value, size = 140 }: { value: number; size?: number }) { +function ProgressRing({ value, size = 140 }: ProgressRingProps) {
158-689: Consider extracting helper components into separate files.
This file currently contains multiple components; splitting them aligns with the component-per-file convention and keeps the route component focused. As per coding guidelines: Components: One component per file, use functional components with hooks.apps/dashboard/src/routes/index.tsx (4)
33-67: Define a props interface forStatCard.
This keeps the prop shape extendable and consistent. As per coding guidelines: Props: Define interface for props, use destructuring.♻️ Suggested refactor
+interface StatCardProps { + label: string; + value: string | number; + change?: string; + positive?: boolean; + icon: React.ElementType; +} -function StatCard({ - label, - value, - change, - positive, - icon: Icon -}: { - label: string; - value: string | number; - change?: string; - positive?: boolean; - icon: React.ElementType; -}) { +function StatCard({ + label, + value, + change, + positive, + icon: Icon +}: StatCardProps) {
125-210: Define a props interface forRequestRow.
This keeps the prop shape extendable and consistent. As per coding guidelines: Props: Define interface for props, use destructuring.♻️ Suggested refactor
+interface RequestRowProps { + request: PARequest; + index?: number; +} -function RequestRow({ request, index = 0 }: { request: PARequest; index?: number }) { +function RequestRow({ request, index = 0 }: RequestRowProps) {
212-231: Define a props interface forActivityItemComponent.
This keeps the prop shape extendable and consistent. As per coding guidelines: Props: Define interface for props, use destructuring.♻️ Suggested refactor
+interface ActivityItemProps { + item: ActivityItem; +} -function ActivityItemComponent({ item }: { item: ActivityItem }) { +function ActivityItemComponent({ item }: ActivityItemProps) {
233-551: Consider moving helper components into separate files.
This file defines multiple components; splitting them keeps the route component focused and aligns with the component-per-file standard. As per coding guidelines: Components: One component per file, use functional components with hooks.apps/dashboard/src/components/WorkflowProgress.tsx (1)
14-20: Icon type annotation could be more precise.The
icon: typeof FileTextworks but couples the type to a specific icon. Consider using a more generic type from lucide-react.♻️ Optional: Use LucideIcon type
+import type { LucideIcon } from 'lucide-react'; import { cn } from '@/lib/utils'; import { FileText, Radio, Brain, Bell, CheckCircle, Check } from 'lucide-react'; // ... -const WORKFLOW_STEPS: { id: WorkflowStep; label: string; description: string; icon: typeof FileText }[] = [ +const WORKFLOW_STEPS: { id: WorkflowStep; label: string; description: string; icon: LucideIcon }[] = [apps/dashboard/src/components/PARequestCard.tsx (1)
151-166: Review button accessibility: consider disabled state handling.When
onReviewis undefined, the button still renders but clicking does nothing (due to optional chaining). Consider either not rendering the button or showing it as disabled when there's no handler.♻️ Optional: Conditionally render or disable button
- <Button - onClick={() => onReview?.(request.id)} + <Button + onClick={onReview ? () => onReview(request.id) : undefined} + disabled={!onReview} className="flex-1 bg-gradient-to-r from-[hsl(243,75%,59%)] to-[hsl(280,75%,55%)] hover:opacity-90 border-0 shadow-primary transition-all" >
| {regularFields.map(([key, value]) => ( | ||
| <div | ||
| key={key} | ||
| data-field={key} | ||
| className={cn( | ||
| 'p-4 rounded-lg border', | ||
| showHighlights && value && 'bg-[hsl(var(--success)/0.1)] border-[hsl(var(--success)/0.3)]' | ||
| 'p-4 rounded-xl border transition-all group hover:shadow-glow', | ||
| showHighlights && value | ||
| ? 'bg-success/5 border-success/20 hover:border-success/40' | ||
| : 'bg-muted/30 border-border/50 hover:border-border' | ||
| )} | ||
| > | ||
| <p className="text-xs font-medium text-muted-foreground mb-2"> | ||
| {formatFieldName(key)} | ||
| <div className="flex items-center justify-between mb-2"> | ||
| <p className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider"> | ||
| {formatFieldName(key)} | ||
| </p> | ||
| {showHighlights && value && ( | ||
| <Edit3 className="h-3 w-3 text-muted-foreground opacity-0 group-hover:opacity-100 transition-opacity" /> | ||
| )} | ||
| </div> | ||
| <p className="text-sm font-medium"> | ||
| {value || <span className="text-muted-foreground italic">Not provided</span>} | ||
| </p> | ||
| <p className="text-sm whitespace-pre-wrap">{value || '—'}</p> | ||
| </div> | ||
| ))} |
There was a problem hiding this comment.
Missing data-field attribute and CSS class mismatch with tests.
Two issues will cause test failures:
- Tests use
.closest('[data-field]')to locate field containers, but the attribute isn't present - Tests expect
bg-[hsl(var(--success)/0.1)]but the component usesbg-success/5
Either update the component to match the tests or update the tests to match the component.
Option A: Update component to match tests
<div
key={key}
+ data-field
className={cn(
'p-4 rounded-xl border transition-all group hover:shadow-glow',
showHighlights && value
- ? 'bg-success/5 border-success/20 hover:border-success/40'
+ ? 'bg-[hsl(var(--success)/0.1)] border-success/20 hover:border-success/40'
: 'bg-muted/30 border-border/50 hover:border-border'
)}
>📝 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.
| {regularFields.map(([key, value]) => ( | |
| <div | |
| key={key} | |
| data-field={key} | |
| className={cn( | |
| 'p-4 rounded-lg border', | |
| showHighlights && value && 'bg-[hsl(var(--success)/0.1)] border-[hsl(var(--success)/0.3)]' | |
| 'p-4 rounded-xl border transition-all group hover:shadow-glow', | |
| showHighlights && value | |
| ? 'bg-success/5 border-success/20 hover:border-success/40' | |
| : 'bg-muted/30 border-border/50 hover:border-border' | |
| )} | |
| > | |
| <p className="text-xs font-medium text-muted-foreground mb-2"> | |
| {formatFieldName(key)} | |
| <div className="flex items-center justify-between mb-2"> | |
| <p className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider"> | |
| {formatFieldName(key)} | |
| </p> | |
| {showHighlights && value && ( | |
| <Edit3 className="h-3 w-3 text-muted-foreground opacity-0 group-hover:opacity-100 transition-opacity" /> | |
| )} | |
| </div> | |
| <p className="text-sm font-medium"> | |
| {value || <span className="text-muted-foreground italic">Not provided</span>} | |
| </p> | |
| <p className="text-sm whitespace-pre-wrap">{value || '—'}</p> | |
| </div> | |
| ))} | |
| {regularFields.map(([key, value]) => ( | |
| <div | |
| key={key} | |
| data-field | |
| className={cn( | |
| 'p-4 rounded-xl border transition-all group hover:shadow-glow', | |
| showHighlights && value | |
| ? 'bg-[hsl(var(--success)/0.1)] border-success/20 hover:border-success/40' | |
| : 'bg-muted/30 border-border/50 hover:border-border' | |
| )} | |
| > | |
| <div className="flex items-center justify-between mb-2"> | |
| <p className="text-[10px] font-semibold text-muted-foreground uppercase tracking-wider"> | |
| {formatFieldName(key)} | |
| </p> | |
| {showHighlights && value && ( | |
| <Edit3 className="h-3 w-3 text-muted-foreground opacity-0 group-hover:opacity-100 transition-opacity" /> | |
| )} | |
| </div> | |
| <p className="text-sm font-medium"> | |
| {value || <span className="text-muted-foreground italic">Not provided</span>} | |
| </p> | |
| </div> | |
| ))} |
🤖 Prompt for AI Agents
In `@apps/dashboard/src/components/FormPreview.tsx` around lines 76 - 98, The
field container rendered in the regularFields.map lacks the data-field attribute
and uses a Tailwind color class that doesn't match tests; update the mapped div
returned in the regularFields.map (the element using cn and keyed by key) to
include a data-field={key} attribute and change the success background class
branch from 'bg-success/5' to the test-expected 'bg-[hsl(var(--success)/0.1)]'
(ensure the conditional still uses showHighlights and value); keep the existing
border and hover classes and leave formatFieldName, showHighlights, and the
Edit3 usage unchanged.
| const sizeClasses = { | ||
| sm: 'w-4 h-4 border-2', | ||
| md: 'w-8 h-8 border-3', | ||
| lg: 'w-12 h-12 border-4', | ||
| }; |
There was a problem hiding this comment.
border-3 is not a standard Tailwind CSS class.
Tailwind's default border-width utilities are border (1px), border-2, border-4, and border-8. The border-3 class on line 11 will be silently ignored, leaving the medium spinner without a visible border.
Proposed fix
const sizeClasses = {
sm: 'w-4 h-4 border-2',
- md: 'w-8 h-8 border-3',
+ md: 'w-8 h-8 border-2',
lg: 'w-12 h-12 border-4',
};Alternatively, if you need a 3px border, extend your Tailwind config:
// tailwind.config.js
module.exports = {
theme: {
extend: {
borderWidth: {
'3': '3px',
}
}
}
}📝 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.
| const sizeClasses = { | |
| sm: 'w-4 h-4 border-2', | |
| md: 'w-8 h-8 border-3', | |
| lg: 'w-12 h-12 border-4', | |
| }; | |
| const sizeClasses = { | |
| sm: 'w-4 h-4 border-2', | |
| md: 'w-8 h-8 border-2', | |
| lg: 'w-12 h-12 border-4', | |
| }; |
🤖 Prompt for AI Agents
In `@apps/dashboard/src/components/LoadingSpinner.tsx` around lines 9 - 13, The
sizeClasses map uses a non-standard Tailwind class 'border-3' for the 'md' key,
which will be ignored; update the sizeClasses constant (the md entry) in
LoadingSpinner.tsx to use a valid Tailwind border utility (e.g., 'border' for
1px or 'border-2'/'border-4' as needed) or, if you truly require a 3px border,
add a '3': '3px' entry under theme.extend.borderWidth in your Tailwind config
and then keep 'border-3' in sizeClasses; ensure the change targets the
sizeClasses object and the 'md' mapping specifically.
| {steps.map((step, index) => ( | ||
| <div | ||
| key={step} |
There was a problem hiding this comment.
Using step name as key may cause issues with duplicate step names.
If steps contains duplicate strings, React will produce key warnings and may incorrectly reconcile elements.
Proposed fix
{steps.map((step, index) => (
<div
- key={step}
+ key={index}
className={cn(📝 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.
| {steps.map((step, index) => ( | |
| <div | |
| key={step} | |
| {steps.map((step, index) => ( | |
| <div | |
| key={index} | |
| className={cn( |
🤖 Prompt for AI Agents
In `@apps/dashboard/src/components/LoadingSpinner.tsx` around lines 114 - 116, In
LoadingSpinner where steps.map((step, index) => ...) uses key={step}, replace
the unstable key with a stable unique key (e.g., key={`${step}-${index}`} or use
an explicit id property if steps are objects) to avoid React reconciliation
issues when steps contains duplicate strings; update the key expression in the
JSX returned by the map inside the LoadingSpinner component accordingly.
| {/* Search Bar */} | ||
| <div className="hidden md:flex items-center gap-2 px-4 py-2 rounded-xl bg-slate-700/50 border border-slate-600 w-80"> | ||
| <Search className="w-4 h-4 text-slate-400" /> | ||
| <input | ||
| type="text" | ||
| placeholder="Search patients, PA requests..." | ||
| className="bg-transparent text-sm outline-none flex-1 text-white placeholder:text-slate-400" | ||
| /> | ||
| <kbd className="px-1.5 py-0.5 rounded bg-slate-600 border border-slate-500 text-[10px] text-slate-300 font-mono">⌘K</kbd> | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/dashboard/src/routes/__root.tsx | head -50Repository: lvlup-sw/authscript
Length of output: 2681
Add an accessible label to the search input.
The search input lacks an accessible name for screen readers. A placeholder alone doesn't satisfy accessibility requirements; use aria-label to provide a proper name. Also update the input type to "search" for semantic correctness.
Suggested fix
<input
- type="text"
+ type="search"
+ aria-label="Search patients and PA requests"
placeholder="Search patients, PA requests..."
className="bg-transparent text-sm outline-none flex-1 text-white placeholder:text-slate-400"
/>📝 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.
| {/* Search Bar */} | |
| <div className="hidden md:flex items-center gap-2 px-4 py-2 rounded-xl bg-slate-700/50 border border-slate-600 w-80"> | |
| <Search className="w-4 h-4 text-slate-400" /> | |
| <input | |
| type="text" | |
| placeholder="Search patients, PA requests..." | |
| className="bg-transparent text-sm outline-none flex-1 text-white placeholder:text-slate-400" | |
| /> | |
| <kbd className="px-1.5 py-0.5 rounded bg-slate-600 border border-slate-500 text-[10px] text-slate-300 font-mono">⌘K</kbd> | |
| </div> | |
| {/* Search Bar */} | |
| <div className="hidden md:flex items-center gap-2 px-4 py-2 rounded-xl bg-slate-700/50 border border-slate-600 w-80"> | |
| <Search className="w-4 h-4 text-slate-400" /> | |
| <input | |
| type="search" | |
| aria-label="Search patients and PA requests" | |
| placeholder="Search patients, PA requests..." | |
| className="bg-transparent text-sm outline-none flex-1 text-white placeholder:text-slate-400" | |
| /> | |
| <kbd className="px-1.5 py-0.5 rounded bg-slate-600 border border-slate-500 text-[10px] text-slate-300 font-mono">⌘K</kbd> | |
| </div> |
🤖 Prompt for AI Agents
In `@apps/dashboard/src/routes/__root.tsx` around lines 31 - 40, Update the search
input element in the __root.tsx component: change its type from "text" to
"search" and add an accessible name via aria-label (for example "Search patients
and PA requests") so screen readers have a clear label; ensure the input element
you modify is the one inside the Search Bar div (the element with className
"bg-transparent text-sm outline-none flex-1 text-white
placeholder:text-slate-400") and do not rely on the placeholder alone for
accessibility.
| <button className="p-2 rounded-lg hover:bg-slate-700 transition-colors relative click-effect"> | ||
| <Bell className="w-5 h-5 text-slate-400" /> | ||
| <span className="absolute top-1.5 right-1.5 w-2 h-2 bg-teal rounded-full" /> | ||
| </button> | ||
|
|
||
| <Link to="/help" className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect"> | ||
| <HelpCircle className="w-5 h-5 text-slate-400" /> | ||
| </Link> | ||
|
|
||
| <button className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect"> | ||
| <Settings className="w-5 h-5 text-slate-400" /> | ||
| </button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/dashboard/src/routes/__root.tsx | head -80Repository: lvlup-sw/authscript
Length of output: 4325
Icon-only controls need accessible names.
These buttons and links contain only icons with no aria-label, visible text, or other accessible name. Add aria-label to clearly identify each control to screen readers.
Suggested fix
- <button className="p-2 rounded-lg hover:bg-slate-700 transition-colors relative click-effect">
+ <button aria-label="Notifications" className="p-2 rounded-lg hover:bg-slate-700 transition-colors relative click-effect">
<Bell className="w-5 h-5 text-slate-400" />
<span className="absolute top-1.5 right-1.5 w-2 h-2 bg-teal rounded-full" />
</button>
- <Link to="/help" className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect">
+ <Link to="/help" aria-label="Help" className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect">
<HelpCircle className="w-5 h-5 text-slate-400" />
</Link>
- <button className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect">
+ <button aria-label="Settings" className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect">
<Settings className="w-5 h-5 text-slate-400" />
</button>📝 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.
| <button className="p-2 rounded-lg hover:bg-slate-700 transition-colors relative click-effect"> | |
| <Bell className="w-5 h-5 text-slate-400" /> | |
| <span className="absolute top-1.5 right-1.5 w-2 h-2 bg-teal rounded-full" /> | |
| </button> | |
| <Link to="/help" className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect"> | |
| <HelpCircle className="w-5 h-5 text-slate-400" /> | |
| </Link> | |
| <button className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect"> | |
| <Settings className="w-5 h-5 text-slate-400" /> | |
| </button> | |
| <button aria-label="Notifications" className="p-2 rounded-lg hover:bg-slate-700 transition-colors relative click-effect"> | |
| <Bell className="w-5 h-5 text-slate-400" /> | |
| <span className="absolute top-1.5 right-1.5 w-2 h-2 bg-teal rounded-full" /> | |
| </button> | |
| <Link to="/help" aria-label="Help" className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect"> | |
| <HelpCircle className="w-5 h-5 text-slate-400" /> | |
| </Link> | |
| <button aria-label="Settings" className="p-2 rounded-lg hover:bg-slate-700 transition-colors click-effect"> | |
| <Settings className="w-5 h-5 text-slate-400" /> | |
| </button> |
🤖 Prompt for AI Agents
In `@apps/dashboard/src/routes/__root.tsx` around lines 53 - 64, The icon-only
controls containing the Bell, HelpCircle, and Settings icons lack accessible
names; add descriptive aria-label attributes to each interactive element (the
notification <button> with Bell, the <Link to="/help"> with HelpCircle, and the
settings <button> with Settings) so screen readers can identify them (e.g.,
aria-label="Notifications", aria-label="Help", aria-label="Settings"), ensuring
the labels are clear and localizable if needed.
| <div | ||
| className={cn( | ||
| 'flex items-center gap-3 p-3 rounded-xl border transition-all', | ||
| style.bg, | ||
| style.border, | ||
| isEditing && 'cursor-pointer hover:opacity-80' | ||
| )} | ||
| onClick={isEditing ? onToggle : undefined} | ||
| > | ||
| <div className={cn('w-6 h-6 rounded-md flex items-center justify-center flex-shrink-0 text-white', style.icon)}> | ||
| {met === true && <Check className="w-3.5 h-3.5" />} | ||
| {met === false && <X className="w-3.5 h-3.5" />} | ||
| {met === null && <AlertCircle className="w-3.5 h-3.5" />} | ||
| </div> | ||
| <span className="text-sm text-foreground flex-1">{label}</span> | ||
| {isEditing && ( | ||
| <span className="text-xs text-muted-foreground">Click to toggle</span> | ||
| )} | ||
| </div> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the file to see the full component structure
wc -l "apps/dashboard/src/routes/analysis.$transactionId.tsx"Repository: lvlup-sw/authscript
Length of output: 133
🏁 Script executed:
# Read the specific lines mentioned and surrounding context
sed -n '110,170p' "apps/dashboard/src/routes/analysis.$transactionId.tsx"Repository: lvlup-sw/authscript
Length of output: 145
🏁 Script executed:
# Find files matching the pattern
fd -t f "analysis.*transactionId" apps/dashboard/Repository: lvlup-sw/authscript
Length of output: 116
🏁 Script executed:
# List all files in the dashboard routes directory
ls -la "apps/dashboard/src/routes/" 2>/dev/null || find apps/dashboard -name "*analysis*" -type fRepository: lvlup-sw/authscript
Length of output: 569
🏁 Script executed:
# Read the file with proper escaping
cat -n 'apps/dashboard/src/routes/analysis.$transactionId.tsx' | sed -n '110,170p'Repository: lvlup-sw/authscript
Length of output: 2535
🏁 Script executed:
# Also check the component function signature and imports
head -100 'apps/dashboard/src/routes/analysis.$transactionId.tsx' | cat -nRepository: lvlup-sw/authscript
Length of output: 3739
🏁 Script executed:
# Find all usages of CriteriaItem to understand how it's used
rg "CriteriaItem" 'apps/dashboard/src/routes/analysis.$transactionId.tsx' -A 2 -B 2Repository: lvlup-sw/authscript
Length of output: 363
🏁 Script executed:
# Check imports to understand available UI libraries
grep -n "import\|from" 'apps/dashboard/src/routes/analysis.$transactionId.tsx' | head -30Repository: lvlup-sw/authscript
Length of output: 472
🏁 Script executed:
# Get more context around CriteriaItem usage
sed -n '250,350p' 'apps/dashboard/src/routes/analysis.$transactionId.tsx'Repository: lvlup-sw/authscript
Length of output: 4130
🏁 Script executed:
# Check if there are any other interactive elements using similar patterns
rg "onClick=" 'apps/dashboard/src/routes/analysis.$transactionId.tsx' -nRepository: lvlup-sw/authscript
Length of output: 550
🏁 Script executed:
# Check the full CriteriaItem component and look for any potential issues with button conversion
sed -n '117,156p' 'apps/dashboard/src/routes/analysis.$transactionId.tsx' | cat -nRepository: lvlup-sw/authscript
Length of output: 1717
🏁 Script executed:
# Look at how CriteriaItem is being called to see full context
sed -n '330,380p' 'apps/dashboard/src/routes/analysis.$transactionId.tsx'Repository: lvlup-sw/authscript
Length of output: 2335
🏁 Script executed:
# Check if there's any keyboard event handling or accessibility attributes already present
sed -n '117,156p' 'apps/dashboard/src/routes/analysis.$transactionId.tsx' | grep -i "key\|aria\|role\|tab"Repository: lvlup-sw/authscript
Length of output: 122
🏁 Script executed:
# Look at other interactive elements in the file to see if they use buttons properly
rg "<button" 'apps/dashboard/src/routes/analysis.$transactionId.tsx' -A 3 | head -40Repository: lvlup-sw/authscript
Length of output: 2212
Make criteria toggles keyboard accessible.
The clickable <div> blocks keyboard navigation. Convert to a <button> to enable Tab, Enter, and Space key support for non-mouse users.
Suggested fix
- return (
- <div
- className={cn(
- 'flex items-center gap-3 p-3 rounded-xl border transition-all',
- style.bg,
- style.border,
- isEditing && 'cursor-pointer hover:opacity-80'
- )}
- onClick={isEditing ? onToggle : undefined}
- >
+ const isInteractive = isEditing && !!onToggle;
+ return (
+ <button
+ type="button"
+ onClick={isInteractive ? onToggle : undefined}
+ disabled={!isInteractive}
+ className={cn(
+ 'flex items-center gap-3 p-3 rounded-xl border transition-all text-left',
+ style.bg,
+ style.border,
+ isInteractive && 'cursor-pointer hover:opacity-80',
+ !isInteractive && 'cursor-default'
+ )}
+ >
<div className={cn('w-6 h-6 rounded-md flex items-center justify-center flex-shrink-0 text-white', style.icon)}>
{met === true && <Check className="w-3.5 h-3.5" />}
{met === false && <X className="w-3.5 h-3.5" />}
{met === null && <AlertCircle className="w-3.5 h-3.5" />}
</div>
<span className="text-sm text-foreground flex-1">{label}</span>
{isEditing && (
<span className="text-xs text-muted-foreground">Click to toggle</span>
)}
- </div>
+ </button>
);🤖 Prompt for AI Agents
In `@apps/dashboard/src/routes/analysis`.$transactionId.tsx around lines 136 -
154, The clickable criteria tile currently implemented as a <div> should be
converted to a semantic <button> to enable keyboard access: replace the outer
<div> (the element using cn(...), isEditing, onToggle) with a <button
type="button">, preserve the same className composition (cn('flex
items-center...', style.bg, style.border, isEditing && 'cursor-pointer
hover:opacity-80')), wire onClick to onToggle when isEditing, and add
accessibility attributes such as aria-pressed (derive from met: true/false ->
"true"/"false", null -> "mixed" or omitted) and an appropriate aria-label (use
label) so screen readers get the intent; keep inner elements (icon div using met
and Check/X/AlertCircle, span for label, and "Click to toggle") unchanged.
| const handleAnalyze = () => { | ||
| setStatus('analyzing'); | ||
| setTimeout(() => { | ||
| navigate({ to: '/analysis/$transactionId', params: { transactionId: `pa-${Date.now()}` } }); | ||
| }, 2000); | ||
| }; |
There was a problem hiding this comment.
Ensure analysis navigation uses a real PA request ID.
pa-${Date.now()} won’t exist in the store, so the analysis page will render “Request Not Found.” Create a PA request (or use a transaction ID produced by your launch/analysis flow) before navigating.
🤖 Prompt for AI Agents
In `@apps/dashboard/src/routes/smart-launch.tsx` around lines 54 - 59,
handleAnalyze sets status then navigates to '/analysis/$transactionId' using a
fake id `pa-${Date.now()}` which won't exist in the store; instead create a real
PA request (or obtain the transaction id from your launch/analysis flow) before
calling navigate. Update handleAnalyze to call the existing PA creation function
or dispatch the action that returns a real transaction id (e.g., createPARequest
/ launchTransaction / submitLaunch) and use that returned id in navigate({ to:
'/analysis/$transactionId', params: { transactionId } }), keeping the setStatus
behavior and the delay if desired.
| ``` | ||
| ┌─────────┐ ┌─────────┐ ┌─────────┐ ┌─────────┐ ┌─────────┐ | ||
| │ DRAFT │───▶│ DETECT │───▶│ PROCESS │───▶│ DELIVER │───▶│ REVIEW │ | ||
| │ │ │ │ │ │ │ │ │ │ | ||
| │ Doctor │ │Signature│ │AI reads │ │Draft PA │ │ Doctor │ | ||
| │ signs │ │detected │ │& maps │ │ready in │ │confirms │ | ||
| │encounter│ │via poll │ │to rules │ │ sidebar │ │& submit │ | ||
| └─────────┘ └─────────┘ └─────────┘ └─────────┘ └─────────┘ | ||
| ``` |
There was a problem hiding this comment.
Add language specifier to fenced code block.
Static analysis flagged this block as missing a language specifier. For ASCII diagrams, use text or plaintext.
📝 Proposed fix
-```
+```text
┌─────────┐ ┌─────────┐ ┌─────────┐ ┌─────────┐ ┌─────────┐🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 30-30: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/ARCHITECTURE.md` around lines 30 - 38, The fenced ASCII diagram block is
missing a language specifier; edit the triple-backtick block that contains the
diagram (the ASCII diagram starting with "┌─────────┐ ┌─────────┐...") and
add a language like text or plaintext (e.g., ```text) immediately after the
opening backticks so the static analyzer recognizes it as plain text.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/dashboard/src/components/__tests__/ConfidenceMeter.test.tsx (2)
13-21:⚠️ Potential issue | 🟡 MinorPotential ambiguity with threshold marker text.
getByText('0%')andgetByText('100%')may match the threshold markers (lines 101, 104 in the component) rather than the percentage display. Forvariant="default", both the percentage span and the marker span could contain "0%" or "100%".Consider using a more specific selector or the aria-label approach used in the first test.
🔍 Suggested fix for deterministic queries
it('ConfidenceMeter_ZeroScore_DisplaysZero', () => { render(<ConfidenceMeter score={0} />); - expect(screen.getByText('0%')).toBeInTheDocument(); + expect(screen.getByRole('meter')).toHaveAttribute('aria-label', 'AI confidence: 0%'); }); it('ConfidenceMeter_FullScore_DisplaysHundred', () => { render(<ConfidenceMeter score={1} />); - expect(screen.getByText('100%')).toBeInTheDocument(); + expect(screen.getByRole('meter')).toHaveAttribute('aria-label', 'AI confidence: 100%'); });
92-107:⚠️ Potential issue | 🟡 MinorEdge case tests have the same ambiguity issue.
Lines 95, 100, and 105 use
getByText('0%')andgetByText('100%')which may match threshold markers. Same fix applies as mentioned for the rendering tests.🔍 Consistent fix using aria-label
it('ConfidenceMeter_NegativeScore_ClampsToZero', () => { render(<ConfidenceMeter score={-0.5} />); - expect(screen.getByText('0%')).toBeInTheDocument(); + expect(screen.getByRole('meter')).toHaveAttribute('aria-label', 'AI confidence: 0%'); }); it('ConfidenceMeter_OverOneScore_ClampsToHundred', () => { render(<ConfidenceMeter score={1.5} />); - expect(screen.getByText('100%')).toBeInTheDocument(); + expect(screen.getByRole('meter')).toHaveAttribute('aria-label', 'AI confidence: 100%'); }); it('ConfidenceMeter_UndefinedScore_ShowsZero', () => { render(<ConfidenceMeter score={undefined as unknown as number} />); - expect(screen.getByText('0%')).toBeInTheDocument(); + expect(screen.getByRole('meter')).toHaveAttribute('aria-label', 'AI confidence: 0%'); });
🧹 Nitpick comments (11)
apps/dashboard/src/components/__tests__/EvidencePanel.test.tsx (1)
105-109: Summary assertions could be more targeted.Using
getByText('2')andgetAllByText('1')may match unintended DOM elements if the component or mock data changes. Consider scoping to the summary container or usingwithin()for more resilient assertions.♻️ Optional: More specific summary assertions
describe('summary', () => { it('EvidencePanel_MultipleItems_ShowsSummaryCount', () => { render(<EvidencePanel evidence={mockEvidence} />); - // Summary shows total count and status breakdown (2 met, 1 not met, 1 unclear) - expect(screen.getByText('4 criteria analyzed')).toBeInTheDocument(); - expect(screen.getByText('2')).toBeInTheDocument(); - const ones = screen.getAllByText('1'); - expect(ones.length).toBeGreaterThanOrEqual(2); + // Summary shows total count + const summaryText = screen.getByText('4 criteria analyzed'); + expect(summaryText).toBeInTheDocument(); + + // Verify status breakdown exists (2 met, 1 not met, 1 unclear) + // Using class selectors to verify counts appear with correct status colors + const successCount = document.querySelector('.text-success'); + const destructiveCount = document.querySelector('.text-destructive'); + const warningCount = document.querySelector('.text-warning'); + + expect(successCount).toHaveTextContent('2'); + expect(destructiveCount).toHaveTextContent('1'); + expect(warningCount).toHaveTextContent('1'); }); });apps/dashboard/src/components/ConfidenceMeter.tsx (1)
18-44: Consider deriving threshold markers from the THRESHOLDS constant.The
getConfigfunction is clean and maintainable. However, the threshold values here (0.8, 0.5) and the hardcoded markers rendered on lines 101-104 ("50%", "80%") could drift if someone updatesTHRESHOLDSbut forgets the UI markers.♻️ Optional: Export thresholds or derive marker values
You could derive the marker percentages from
THRESHOLDSto keep them in sync:-const THRESHOLDS = { high: 0.8, medium: 0.5 }; +const THRESHOLDS = { high: 0.8, medium: 0.5 } as const;Then in the markers section:
- <span className="text-warning">50%</span> - <span className="text-success">80%</span> + <span className="text-warning">{THRESHOLDS.medium * 100}%</span> + <span className="text-success">{THRESHOLDS.high * 100}%</span>apps/dashboard/src/components/__tests__/ConfidenceMeter.test.tsx (1)
24-42: Fragile color assertions based on HSL values and class patterns.These regex patterns (
/160|172|success/,/38|25|warning/,/hsl\(0,/) are brittle:
- They match arbitrary substrings that could appear in unrelated classes
- If gradient HSL values change, tests break without clear indication of why
- Inconsistent patterns across the three tests (some match numbers, one matches full
hsl(0,)This approach couples tests tightly to implementation details. Consider testing the visual outcome differently or at minimum using consistent, more specific patterns.
♻️ More robust approach using data attributes or consistent class checks
Option 1: Add a
data-confidence-levelattribute to the fill element in the component, then test:// In component: <div data-testid="confidence-fill" data-confidence-level="high" ...> // In test: expect(fill).toHaveAttribute('data-confidence-level', 'high');Option 2: If testing classes, use more specific patterns:
it('ConfidenceMeter_HighConfidence_ShowsGreen', () => { render(<ConfidenceMeter score={0.9} />); const fill = screen.getByTestId('confidence-fill'); - expect(fill.className).toMatch(/160|172|success/); + expect(fill.className).toMatch(/from-\[hsl\(160,84%,39%\)\]/); });apps/dashboard/src/components/__tests__/FormPreview.test.tsx (1)
64-69: Consider strengthening theFormPreview_Loading_HidesContenttest.The test passes
mockFieldMappingsbut only asserts the skeleton is present. It doesn't verify that the actual content (field values) is hidden during loading. Consider adding a negative assertion.💡 Suggested improvement
it('FormPreview_Loading_HidesContent', () => { render(<FormPreview fieldMappings={mockFieldMappings} loading />); // When loading, skeleton is shown instead of content expect(screen.getByTestId('form-skeleton')).toBeInTheDocument(); + // Actual field content should not be rendered + expect(screen.queryByText('John Doe')).not.toBeInTheDocument(); });apps/dashboard/src/components/NewPAModal.tsx (2)
51-51: Unused state variable.The
selectedDiagnosisstate value is never read; only its setter is used. If you don't need to display or reference the selected diagnosis elsewhere, you can remove the state entirely and just pass the diagnosis directly tocreatePARequest.♻️ If state is truly unnecessary
- const [, setSelectedDiagnosis] = useState<{ code: string; name: string } | null>(null);And in
resetAndClose, removesetSelectedDiagnosis(null);
114-120: Potential race condition between UI transition and async operation.
transitionToStepstarts a setTimeout that will setisTransitioning = falseafter 500ms, butprocessPARequestis async and runs concurrently. IfprocessPARequestcompletes before 500ms,resetAndClose()andnavigate()fire while the transition overlay is still visible, which could cause a brief flash. Consider awaiting the transition or coordinating the timing.apps/dashboard/src/routes/index.tsx (3)
33-231: Multiple helper components in a single file.This file contains
StatCard,WorkflowPipeline,RequestRow, andActivityItemComponentalongsideDashboardPage. The coding guideline specifies "One component per file." Since these are tightly coupled to the dashboard and not reused elsewhere, this is acceptable for now, but consider extracting them to separate files if they grow or become reusable.
520-541: Hardcoded values in Weekly Summary.Lines 530, 534, and 538 display hardcoded values ("4.2 min", "87%", "94%") rather than computed stats. If this is intentional placeholder/demo data, consider adding a comment. Otherwise, these should derive from actual data.
<div className="flex items-center justify-between py-2 border-b border-dashed"> <span className="text-sm text-muted-foreground">Avg. Processing Time</span> - <span className="font-bold text-foreground">4.2 min</span> + <span className="font-bold text-foreground">{stats.avgProcessingTime ?? '—'}</span> </div>
352-379: Minor: Potentially misleading "pending" label in stats.Line 356-357 shows
change={stats.ready > 0 ?${stats.ready} pending: undefined}for the "Ready for Review" stat, which already showsstats.readyas the value. This makes the change badge redundant. Consider showing a more informative metric (e.g., delta from yesterday).apps/dashboard/src/components/EvidencePanel.tsx (2)
13-41: Consider adding type annotation tostatusConfigfor stricter indexing.Adding
as constor an explicit type would provide better TypeScript inference when indexing and ensure the keys align with theEvidenceItem.statustype.✨ Optional type-safety improvement
-const statusConfig = { +const statusConfig: Record<EvidenceItem['status'], { + icon: typeof CheckCircle2; + gradient: string; + bg: string; + border: string; + text: string; + badge: string; + label: string; +}> = { MET: {Or simply use
as constfor literal inference:-const statusConfig = { +const statusConfig = { MET: { ... }, ... -}; +} as const satisfies Record<string, { icon: typeof CheckCircle2; gradient: string; bg: string; border: string; text: string; badge: string; label: string }>;
79-83: Consider single-pass count aggregation if evidence arrays grow large.Currently iterates 3× over the array. A single
reducewould be O(n), though for typical small arrays this is inconsequential.♻️ Single-pass alternative
- const counts = { - met: evidence.filter(e => e.status === 'MET').length, - notMet: evidence.filter(e => e.status === 'NOT_MET').length, - unclear: evidence.filter(e => e.status === 'UNCLEAR').length, - }; + const counts = evidence.reduce( + (acc, e) => { + if (e.status === 'MET') acc.met++; + else if (e.status === 'NOT_MET') acc.notMet++; + else acc.unclear++; + return acc; + }, + { met: 0, notMet: 0, unclear: 0 } + );
2 participants
Moving the initial dashboard implementation to the main repository.