Preliminary Review — Nightingale Process Management

Multi-area review generated 2026-06-15 by a 6-agent automated audit (architecture, design, security, UX/UI, data integrity, performance). Preliminary: survey-level breadth, highest-impact findings. Each finding cites file:line where possible. Findings are starting points for human verification, not confirmed defects.

Summary

Area	Maturity	Findings
Architecture & code organization	🟢 Strong	3 low, 3 info
Design & aesthetics	🟢 Strong	3 low, 5 info
Security & privacy	🟡 Solid	3 low, 7 info
UX/UI affordances & accessibility	🟡 Solid	3 medium, 3 low, 2 info
Data integrity & financial correctness	🟡 Solid	2 medium, 2 low, 1 info
Performance, reliability & testing	🟡 Solid	1 critical, 2 high, 4 medium, 2 low

Overall: a strong, mature codebase for a payroll/PII product. No critical security or financial-correctness defects surfaced. The most material gaps are operational reliability (no retries on external Wise/Hubstaff calls), test coverage of the server-action/RLS layer, and some UX polish (data provenance, responsive forms).

Top findings across all areas (Critical / High / Medium)

• 🔴 Critical — Wise integration has no retry logic on transient failures (Performance, reliability & testing)
  src/server/integrations/wise/client.ts:70-92 — Implement exponential backoff retry (2-3 attempts) for network errors and transient 5xx on Wise calls. Return different error codes for retryable vs. non-retryable failures. Add structured logging of Wise request/retry attempts (include request duration, retry count) so operations can debug delays.
• 🟠 High — Server actions lack systematic test coverage (Performance, reliability & testing)
  src/server/actions/ (all files) vs. tests/ (0 action tests) — Create unit tests for high-risk actions: test authorization checks (non-owner rejection), input validation failures, database error handling (simulate constraint violations), and rollback paths. Start with hire.ts (multi-step), invoices.ts (state machine), and paymentAdjustments.ts (locking logic). Aim for ~100 action-specific tests.
• 🟠 High — Missing loading.tsx/Suspense boundaries on slow pages (Performance, reliability & testing)
  src/app/admin/page.tsx:97-145, src/app/contractor/page.tsx:77-119 — Wrap slow data loads in Suspense. Example: <Suspense fallback={}> for the overage query. Add loading.tsx at src/app/admin/loading.tsx and src/app/contractor/loading.tsx with placeholder cards. Consider pagination or WHERE limits on time_entries (select 'contractor_id, assignment_id, entry_date' .limit(2000) can return 2000+ rows).
• 🟡 Medium — Multi-column form layouts without responsive justification (UX/UI affordances & accessibility)
  src/components/admin/ContractorProfileForm.tsx:59, 104, 140 — Either (1) add a comment explaining why 3-col layout is justified for this admin form (e.g., 'name fields are quick, grouped for efficiency'), or (2) revert to single-column primary layout with multi-column only on lg+ screens. If justified, update ux-ui-guidelines.md to document when multi-column is acceptable.
• 🟡 Medium — Placeholder text used alongside labels in some forms (UX/UI affordances & accessibility)
  src/components/admin/WiseRecipientForm.tsx:39, 49, 59, 70; src/components/admin/FacilityIntakeForm.tsx (multiple) — Reserve placeholder for in-field examples only where the hint text in does not fully clarify format (e.g., placeholder='74ecdaf6-175d-4a93-...' is acceptable for UUIDs). Remove placeholders from fields that already have visible labels and optional/(required) indicators. If placeholder adds value, move the hint to instead.
• 🟡 Medium — Data provenance / last-updated timestamps absent from payroll and PII pages (UX/UI affordances & accessibility)
  src/app/contractor/page.tsx (estimate card, recent pay section); src/app/contractor/payslips/page.tsx (payment amounts) — Add 'Calculated at [timestamp]' to pay estimate card; add 'Locked [timestamp]' or 'Processed [timestamp]' to payslip rows. On admin payroll pages, surface when a period was closed and payment amounts finalized. Use formatWhen pattern observed in audit page.
• 🟡 Medium — 13th-month pay truncates per-period bases instead of rounding entitlement (Data integrity & financial correctness)
  src/lib/pay/thirteenthMonth.ts:15; src/server/actions/thirteenthMonth.ts:103 — Remove Math.trunc on individual periods. Change line 15 to: const total = periodBasicCentavos.reduce((sum, c) => sum + c, 0). Add test with fractional period bases (e.g., 1667 each) to verify stable entitlement year-over-year.
• 🟡 Medium — Wise reconciliation variance tolerance defaults to 0 (exact match only), risky in FX scenarios (Data integrity & financial correctness)
  src/server/services/wise/reconcile.ts:99, 122; src/lib/wise/reconcile.ts:121 — Add company-level setting for variance tolerance (default 50 centavos for PHP, 5 cents for USD). Pass dynamically: reconcileTransfers(payments, transfers, {varianceToleranceCentavos: tolerance}). Document FX rounding expectation in ADR-0020.
• 🟡 Medium — RLS policy enforcement not tested; server actions verify once but not twice (Performance, reliability & testing)
  src/server/actions/invoices.ts:74-150 — Add integration tests that query the database directly as a service client, then verify RLS is active. Example: fetch invoice as company A's service client, then as company B's service client, verify B cannot see A's invoice. Document RLS assumptions in ADR.
• 🟡 Medium — Query limits on dashboard and list pages may scan too much data (Performance, reliability & testing)
  src/app/admin/page.tsx:112-120, src/app/admin/invoices/page.tsx (if using similar patterns) — Add explicit date range filters: e.g., .gte('entry_date', lastWeekStart) for time entries. Document indexes required: (company_id, status, created_at desc) on invoices; (company_id, assignment_id, entry_date) on time_entries. Run EXPLAIN ANALYZE on slow queries in production to confirm index usage.
• 🟡 Medium — No retry/fallback for Hubstaff sync; stale data silently used (Performance, reliability & testing)
  src/server/integrations/hubstaff/api.ts:149-200 (pagination loop) — Implement simple retry-on-timeout logic: catch timeouts, sleep 2s, retry the current page up to 3 times. If max retries exceeded, return Err with context (which page failed, how many rows fetched so far). Log at warn level so ops can audit syncs.
• 🟡 Medium — 'use client' proliferation in components (106 files) may inflate bundle (Performance, reliability & testing)
  src/components/ (106 'use client' files) — Run next/bundle-analyzer to profile client bundle size. Identify 3-5 heavily-imported client components (e.g., Button) and measure impact if converted to server-renderable versions. No immediate action needed if bundle is <150KB, but document the audit.

---

Architecture & code organization

Maturity: 🟢 Strong

This is a well-architected, mature HR/payroll codebase with exceptional discipline around money handling, security, and separation of concerns. The project demonstrates clear adherence to documented conventions, comprehensive ADR coverage, strict TypeScript strictness, and thoughtful domain-driven design. The two-client RLS model, branded money types, and explicit Result<T,E> pattern for error handling are standout architectural decisions that prevent entire classes of bugs.

Strengths

• Excellent security architecture: two-client model (RLS + service role) with defense-in-depth, verified explicitly in every server action, and comprehensive audit logging masked for PII
• Branded money types (Cents/Centavos) preventing currency mixing and float-arithmetic errors; symmetric rounding with allocateByWeights guarantees exact penny-perfect splits
• Result<T,E> discriminated union pattern for fallible domain logic forcing explicit error handling at call sites; thrown errors reserved for truly unexpected failures
• 44 migrations in version-numbered order with clear intent (0001-0044); ADRs cover critical decisions (ADR-0006 money, ADR-0004 two-client, ADR-0025 hardening)
• Consistent named exports across 318 TypeScript files; @/ alias heavily used (805+ imports); strict tsconfig with noUncheckedIndexedAccess, exactOptionalPropertyTypes
• Server-only boundary enforced via 'server-only' imports for sensitive modules; createServerSupabase and createServiceClient properly gated
• 38 server actions with Zod validation at trust boundaries; no inline SQL, all queries through parameterized Supabase clients
• Test coverage mirroring src/ layout (32 test files); pure domain logic in lib/ tested in isolation (interest, payroll, reconciliation, dates, names)
• Biome linter/formatter configured consistently (organizeImports, noConsoleLog, noExplicitAny warnings); guardrails script in package.json
• Environment validation at startup via Zod (src/server/env.ts) with fail-fast, aggregated errors; secrets properly ignored in .env.local

Findings

• 🔵 Low — Two as any casts in AI extraction (Anthropic SDK integration)
  src/server/integrations/ai/anthropic.ts:129-130
  File src/server/integrations/ai/anthropic.ts uses as any twice for SDK type compatibility when constructing image and document sources for the Claude API. While narrowly scoped and necessary due to SDK type misalignment, these bypass type safety at the boundary.
  Recommendation: Extract the source construction logic to a helper that documents the SDK's expected shape, or open an issue with @anthropic-ai/sdk to export a narrower type for source construction. Current approach is acceptable as a pragmatic workaround.
• 🔵 Low — Minor type safety gap in JSON casting for payroll RPC
  src/server/actions/payroll.ts:128
  File src/server/actions/payroll.ts casts payment lines as unknown as Json when passing to the prepare_payroll_draft RPC. The cast is necessary at the Supabase boundary (RPC args are typed Json), but it's a momentary type escape.
  Recommendation: Add a type-safe adapter function that explicitly maps PaymentLine to a Json-compatible shape; this would document the boundary contract and eliminate the escape.
• 🔵 Low — Server components in App Router directly querying database
  src/app/contractor/page.tsx:84-91
  Pages like src/app/contractor/page.tsx directly call createServerSupabase and chain .from/.select queries in the Server Component body. While correct (RLS applies), this couples component rendering to data fetching; larger pages may benefit from fetching in a service layer first.
  Recommendation: This is a minor style preference. If the codebase grows, consider extracting repeated patterns into src/server/services (already done for payroll, audit, email, etc.). Current approach is pragmatic for simple read-heavy pages.
• ⚪ Info — Multi-tenant TODO remains in profile bootstrap
  src/server/auth/profile.ts:36
  File src/server/auth/profile.ts contains // TODO(multi-tenant): map email domain -> company. The system currently seeds a single company; when multi-tenancy is enabled, this logic must be revisited to map incoming admin email domains to the correct company.
  Recommendation: Create ADR-0036 (or similar) documenting the planned multi-tenant domain-to-company mapping strategy before tenants are supported in production.
• ⚪ Info — Large generated types.ts file (2,914 lines) not excluded from Biome checks
  biome.json:10
  File src/db/types.ts is auto-generated from Supabase schema and included in Biome linting despite being Supabase-generated. The file is properly ignored in biome.json, so this is not a problem, but the configuration is correct.
  Recommendation: Confirm in CI/CD that pnpm run lint is not failing on types.ts regen. Current ignore list is correct.
• ⚪ Info — UX/UI guidelines are thorough but compliance enforcement is manual
  docs/ux-ui-guidelines.md, src/components/
  docs/ux-ui-guidelines.md is well-written and covers 60-30-10 color, A11y, forms, tables, feedback timing. Compliance is currently enforced via code review; there are no lint rules or Playwright tests validating WCAG AA contrast, focus states, or responsive card stacks.
  Recommendation: Consider a future Playwright suite (axe-core, lighthouse) to validate a11y automatically. For now, document in PR template a checklist referencing the guidelines.

Top recommendations

1. Migrate the as any casts in anthropic.ts and payroll.ts RPC to explicit type-safe adapter functions; this maintains the strong type discipline across the codebase.
2. Establish a multi-tenancy ADR (domain → company mapping) and guard the profile bootstrap before production multi-tenant support, even if it remains seeded-single-company for now.
3. Add a Playwright test suite (even smoke-level) validating core user flows (contractor login → hours submission, admin login → payroll preview/close) to catch regressions in the complex payroll state machines.
4. Document the payment workflow state machine (draft → prepared → processing → completed → failed) as an ADR so that future edits to payroll.ts and migrations are guided by the same mental model.

---

Design & aesthetics

Maturity: 🟢 Strong

The design system is cohesive, well-documented, and demonstrates strong attention to visual consistency and accessibility. The UI kit is built on CVA-based component variants, Tailwind utilities, and semantic HTML. The app follows the 60/30/10 color rule rigorously, reserves semantic colors (red/yellow/green) strictly for status, and implements complete interactive states across core components. Spacing, typography, and button variants are consistent throughout. Key strengths include WCAG AA focus indicators site-wide, proper aria attributes, mobile card-stack tables, and form handling with inline validation. Minor areas for improvement are lack of dark mode support (intentional for clinical/trusted software), sparse skeleton/loading state usage in async pages, and a handful of ad-hoc inline styles on alert CTAs.

Strengths

• Design-system coherence: Single unified color palette (brand indigo #281D87, neutral grayscale, semantic status colors) applied consistently across all components via Tailwind tokens and CVA variants
• 60/30/10 rule enforced: Brand color (indigo) appears only on primary actions and links (10%); secondary/neutral surfaces dominate (60%); semantic red/yellow/green strictly reserved for status—never for branding
• Complete interactive states: All buttons, links, form controls implement default, hover, focus-visible, active, disabled, and loading states; transition-colors used consistently
• Component reuse via UI kit: 24 well-factored components (Button, Badge, Alert, Dialog, Table, Form fields, etc.) with CVA-based variants; no duplicated styling logic across app
• Accessibility as default: Global focus-visible outline (2px brand-600), semantic HTML (table, dialog, menu roles), aria-label/aria-describedby patterns, full keyboard navigation in complex widgets (RowActions menu, Tooltip), prefers-reduced-motion respected
• Form & input design: Single-column layouts, visible labels with optional/required signaling, inline error display next to field, correct input types (email, tel, number), preserve input on error
• Mobile-first responsive: Table card-stack pattern on <640px, flexible grid layouts, touch targets ≥44px (min-h-9 / min-h-11), flexible padding (p-4/p-5/p-6/p-8)
• Visual hierarchy established: PageHeader (h1 2xl bold), Section titles (h2 lg semibold), body text (slate-700/900), secondary (slate-500), tertiary (slate-400), emphasis via weight not color alone
• Empty states designed: EmptyState component with title, description, and CTA as per guidelines; used on empty invoice lists, no-data scenarios
• Loading feedback: Spinner (animate-spin) for inline feedback, Skeleton (animate-pulse) for >300ms placeholders, Button loading prop disables & shows spinner, toast notifications for explicit success
• Data-ink minimization: Tables stripped of unnecessary borders/gridlines (only essential cell/row borders), minimal shadows, clean Alert/Badge designs without decorative gradients
• Brand consistency: Nightingale logo (lockup/mark variants), indigo theme throughout, professional medical/clinical tone—no playful easing or unnecessary animations
• Spacing rhythm consistent: gap-1/1.5/2/3/4, px-2/3/4/5, py-1/2/3, p-5/6/8 used repeatedly; avoids one-off spacing values

Findings

• 🔵 Low — Ad-hoc inline CTA styles in admin dashboard alerts
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/app/admin/page.tsx:50-54
  Alert row CTAs (AlertRow component, admin/page.tsx:50-54) use a className template string with inline conditionals for danger/info tone colors. This duplicates the Alert component's tone logic and makes future color changes fragile.
  Recommendation: Extract a reusable AlertCTA or AlertAction component that accepts tone prop and renders styled buttons/links, keeping color logic in one place.
• 🔵 Low — No visible loading states on async-rendered pages
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/components/ui/Spinner.tsx:18-23 (defined but not used)
  Admin pages (dashboard, invoices, contractors) and contractor calendar fetch data server-side and render the full page without skeleton placeholders. If server rendering takes >300ms, users see blank/hanging screens. The Skeleton component is defined but unused across the app.
  Recommendation: For pages with async data fetches, wrap dynamic sections in Suspense boundaries with Skeleton fallbacks. See ux-guidelines §System Status & Feedback: 'use loading indicators or skeleton screens for anything over ~300ms'.
• 🔵 Low — Motion tokens not explicitly documented
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/components/admin/AdminShell.tsx:60, 70, 86
  The AdminShell reveal animation uses duration-700 ease-out for the intro phase and duration-200 for normal toggles, but these values are hardcoded in the component. No central motion.ts or @layer utilities file documents animation durations/easing curves, making consistency harder to maintain.
  Recommendation: Create src/lib/motion.ts or extend globals.css with @layer utilities defining REVEAL_DURATION, NORMAL_TRANSITION, etc., then reference those constants in components. This aligns with the design-system principle of centralizing tokens.
• ⚪ Info — Input component missing light error background in aria-invalid state
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/components/ui/form.tsx:10
  The form.tsx Input component styles aria-invalid=true with a red border only (border-red-500), but does not add a light background. Alert/Badge danger tones use a bg-red-50 base; the input should match for visual consistency.
  Recommendation: Add 'aria-[invalid=true]:bg-red-50' to the control base class in form.tsx:10 so invalid inputs inherit the same warm-red background as Alert tone=danger, improving visibility.
• ⚪ Info — No dark mode support (intentional for clinical/trusted context)
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/app/globals.css:22-24
  The app declares color-scheme: light in globals.css and uses no dark: variants. This is appropriate for a clinical/payroll system (trusted software benefits from consistent, familiar light UI), but is worth documenting as a deliberate design choice.
  Recommendation: Add a brief comment in CLAUDE.md or the design guidelines explaining why dark mode is not supported: 'Clinical & financial software benefits from a predictable, high-contrast light UI. Dark mode adds complexity without trust gain.' This helps future contributors understand the constraint.
• ⚪ Info — Table row hover states could improve scanability
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/components/ui/Table.tsx:38-58
  Data-heavy tables (invoices, contractors, facilities) render tbody rows with no hover background. In high-data-density contexts (20+ rows), a subtle hover state (e.g., bg-slate-50) helps users track their mouse and reduces cognitive load.
  Recommendation: Add an optional 'hoverable' prop to Table/Tbody that applies 'hover:bg-slate-50' to Td cells, or apply it by default. This is a low-cost usability win for dense admin tables.
• ⚪ Info — Field-level help text and aria-describedby missing
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/components/ui/form.tsx:12-43
  The Field component (form.tsx:12-43) supports optional hint text below the field, which renders as small gray text. However, the input elements do not have aria-describedby linking to the hint, so screen readers will not announce the hint on focus.
  Recommendation: Generate a unique ID for the hint paragraph (if present) and add aria-describedby to the input, passing that ID. This ensures screen-reader users hear hints immediately on focus, aligning with WCAG 2.1 §1.3.1 (Info & Relationships).
• ⚪ Info — Brand indigo color usage conservative but well-applied
  /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/tailwind.config.ts:14-18
  The Nightingale brand color (#281D87, brand-500/600) is applied sparingly: primary button, active tab underline, breadcrumb links, brand-500 in CVA defaults. No abuse of the brand color for secondary elements or branding-driven data visualization, which aligns with the 60/30/10 rule and clinical UX best practices.
  Recommendation: Maintain this restraint. Continue to reserve brand color only for primary CTAs, primary navigation, and focus indicators. Do not introduce brand-colored backgrounds, badges, or chart elements—let status colors (green/yellow/red) carry those meanings.

Top recommendations

1. Add skeleton/loading states to async-fetched pages: The home page, admin pages, and contractor calendar appear to SSR or fetch without visible load indication. Add Skeleton placeholders for >300ms delays (see ux-guidelines §System Status).
2. Reduce ad-hoc inline CTA styles on admin dashboard: Alert row CTAs (line 50-54, AdminShell) use inline className template strings for danger/info variants; extract a reusable AlertCTA component to centralize Alert-contextual button styling.
3. Document motion tokens explicitly: The AdminShell reveal animation (duration-700 ease-out on intro) and sidebar transition (duration-200) work well, but lack a documented tokens file; add a motion.ts or extend globals.css with @layer utilities for all duration/easing used.
4. Ensure form focus styles are consistent outside the UI kit: Pages like AdminInvoices and contractor forms use Field/Input components, which inherit the global focus-visible ring; spot-check any custom form renderings for missing focus outlines.
5. Consider a visual polish pass on data-heavy tables: Invoice and contractor tables are functional, but row-level actions (kebab menu) and variance flags could benefit from light hover backgrounds (e.g. bg-slate-50 on tbody tr) to improve scanability.
6. Add error/invalid state styling to Input when aria-invalid=true: The form.tsx Input supports aria-invalid=true but only styles the border (border-red-500); add a light red background (bg-red-50) for higher visibility, matching Alert tone=danger palette.

---

Security & privacy

Maturity: 🟡 Solid

This is a well-architected, security-conscious HR/payroll application with defense-in-depth controls. The two-client RLS + service-role model is properly implemented across nearly all server actions. Authentication, session control, and PII handling follow best practices. A pre-push guardrail gate prevents common mistakes. No critical vulnerabilities were found. Maturity is solid because the design is sound and most controls are in place, but a few manual checklist items remain (RLS integration tests, GitHub Actions CI, a11y/security audit, and staging dry-run per ADR-0025).

Strengths

• Two-client model (user+RLS read path, service-role write path) is correctly implemented in ~50 actions with consistent company_id scoping preventing cross-tenant reads/writes
• Session control surface (sessions table, force-logout, per-role timeout) is enforced at middleware layer before request reaches handlers; revocation is atomic
• Auth gate (proxy.ts) uses RLS client so no service-role secret enters edge bundle; OAuth domain validation is server-side authoritative
• PII masking for SSN/bank/tax IDs is enforced at audit layer (regex-based last-4 rule); bank account numbers stored last-4 only; check OCR omits routing/account
• Contractor invitation tokens are hashed (SHA-256), single-use (token_hash + status check), and expire server-side; contractor onboarding is properly multi-step and company-scoped
• Sensitive single-use tokens (invites, magic links) use crypto.subtle + crypto.getRandomValues (Web Crypto), constant-time comparison (safeEqual) prevents timing leaks
• Webhook auth (Hubstaff) and cron auth use constant-time comparison of shared secrets; fails CLOSED (empty secret → no auth)
• All money operations are in integer centavos, not floats; invoice/payroll writes use RPC with optimistic concurrency guards (idempotency keys, row-lock conditions)
• Password reset for contractors is via Supabase email recovery (no custom token generation), and admin password reset generates strong temporary passwords
• Zod validation at every trust boundary (FormData in server actions); all inputs are parameterized (no SQL concatenation)
• Guardrails script (pre-push hook) prevents funding endpoints and secrets in NEXT_PUBLIC_ vars from being committed
• Environment secrets validated at boot via Zod; integrations (Wise, Gmail, Hubstaff) validate lazily so app boots without all credentials

Findings

• 🔵 Low — RLS policies for contractor invoice read access use EXISTS subquery
  src/db/migrations/0003_rls_policies.sql (invoice policies)
  Migration 0005 (referenced in ADR-0004) uses an EXISTS policy on invoices so contractors can read their own invoices via a contractor self-select policy on line items. This is correct (avoids a SECURITY DEFINER view), but it's less obvious than a direct policy and should be audited if invoice logic changes.
  Recommendation: Add a comment in the RLS migration explaining the contractor invoice read path and why EXISTS is necessary. Ensure integration tests (currently a planned checklist item in ADR-0025) verify a contractor cannot read another's invoices.
• 🔵 Low — Time entry status transitions are enforced by RLS but not exhaustively guarded in app logic
  src/db/migrations/0003_rls_policies.sql (time_entries policies)
  src/server/services/timesheet.ts and the cron lock logic rely on status enums ('draft', 'locked', 'approved', 'submitted') and RLS policies to prevent state violations. A contractor cannot unlock a 'locked' entry by RLS, but the app logic does not always validate state transitions on write (e.g., no explicit check that you can only move from 'draft' → 'submitted', not 'locked' → 'submitted').
  Recommendation: Document the valid time entry state machine in a schema comment or ADR. Add a database constraint (CHECK) or trigger to enforce transitions, or add explicit validation in timesheet services. This prevents a future refactor from introducing a state-bypass bug.
• 🔵 Low — Session timeouts are user-configurable per profile, but no UI minimum/maximum enforced
  src/proxy.ts:141
  src/proxy.ts reads profile.session_timeout_minutes, which an admin can set to any value. There is no database constraint on the range (e.g., 10 min–48 hours). A staff admin could accidentally set a very high timeout or a very low one.
  Recommendation: Add a CHECK constraint or range validation (e.g., 15–480 minutes) on the profiles table. Document the intent in a comment. This prevents accidental misconfiguration that weakens or breaks session control.
• ⚪ Info — Single-tenant bootstrap awaits multi-tenant implementation
  src/server/auth/profile.ts:36
  src/server/auth/profile.ts:36 has a TODO: the first admin to sign in gets assigned to the first seeded company. Multi-tenant domain→company mapping is not yet implemented. In a staging/production multi-tenant setup, all admins will land in the same company unless this is fixed.
  Recommendation: Before multi-tenant go-live, implement the mapping of email domain → company_id. Suggest a companies.allowed_domains table or env-based domain configuration. Design should prevent one admin from joining a company not owned by their domain.
• ⚪ Info — Contractor self-service write bypasses RLS via service client
  src/server/actions/contractorProfile.ts:54
  src/server/actions/contractorProfile.ts and other contractor self-update actions use service-role client (which bypasses RLS) with only .eq('id', contractor.contractorId) — no company_id check. This is safe here because contractorId comes from the authenticated session (getCurrentContractor()), but it differs from the pattern in admincontractorProfile which includes .eq('company_id', admin.companyId). ADR-0009 acknowledges this: 'no contractor self-update RLS'.
  Recommendation: Consider adding .eq('company_id', contractor.companyId) as a defensive redundancy even though the session scope should prevent abuse. This makes the intent clearer and catches any logic bugs in getCurrentContractor(). Not urgent; document the pattern in ADR-0009.
• ⚪ Info — Audit log redaction is regex-based, not exhaustive
  src/server/services/audit.ts:77
  src/server/services/audit.ts:77 masks keys matching /account_number|password|secret|token|service_role|ssn|tax_id/i and fields >80 chars are truncated. A new PII field (e.g., 'credit_card', 'routing_number') would not be caught until the regex is updated. No automated discovery of PII column names.
  Recommendation: Consider adding a metadata registry of PII columns at the schema level (e.g., a comment or constraint) so audit code can auto-detect what to mask. For now, document the masking rule in CLAUDE.md and add a pre-commit check (or ADR) that any PII column name must match the regex or be added explicitly.
• ⚪ Info — Contractor webhook (Hubstaff) passes companyId=null for org-wide delivery
  src/server/integrations/hubstaff/sync.ts:26-32
  src/server/integrations/hubstaff/sync.ts:32 allows companyId=null for single-tenant webhooks. If this is ever deployed multi-tenant, the webhook will apply hours to any contractor matching the hubstaff_member_id across the entire system, bypassing tenant isolation. The CSV import correctly scopes to admin.companyId, but the webhook does not.
  Recommendation: Document that the webhook (and org-wide Hubstaff sync) is single-tenant only. Before multi-tenant go-live, either (a) derive companyId from the Hubstaff org payload, or (b) remove org-wide support and require an admin to trigger syncs per company. Add a type-level check to prevent future null passes.
• ⚪ Info — IP and User-Agent logged for auth events but not normalized
  src/server/auth/session.ts:26, src/proxy.ts:42
  src/server/auth/session.ts and proxy routes log x-forwarded-for, x-real-ip, and user-agent for debugging revoked/timed-out sessions. These are extracted from headers without normalization and could contain malicious input (e.g., a large header). No direct exploit, but increases log storage and makes querying harder.
  Recommendation: Add length limits (e.g., x-forwarded-for: split + take first 45.255 chars) and schema constraints on sessions.ip and sessions.user_agent columns. This improves log quality without changing security.
• ⚪ Info — No rate limiting on public endpoints (contractor login, password reset, invitation acceptance)
  src/server/actions/contractor-auth.ts
  src/server/actions/contractor-auth.ts has no rate limits on acceptInvitation, loginContractor, or requestContractorPasswordReset. A brute-force or account enumeration attack is not constrained by the app (though Supabase Auth may have some built-in limits).
  Recommendation: Add rate limiting to contractor auth endpoints (e.g., 5 attempts / 15 min per IP, 3 resets/day per email). Options: (1) Redis-backed rate limiter in middleware, (2) Supabase realtime events to track attempts, (3) a simple in-memory cache with TTL per request. This is particularly important for invitation links (single-use token, but brute-force could exhaust valid tokens).
• ⚪ Info — No explicit CSRF protection documented for form submissions
  src/components/contractor/ProfileForm.tsx (and all formAction usages)
  Server actions in Next.js 13+ do not require CSRF tokens by default (they are POST-only to the same origin via formAction). However, no documentation or comments explain this assumption, which could mislead a future contributor into adding an unsafe form submission.
  Recommendation: Add a comment in CLAUDE.md or one representative server action explaining that Next.js SSR forms are CSRF-protected by default (POST-only, same-origin formAction). Link to Next.js security docs. This prevents accidental CSRF if someone later adds a GET endpoint or cross-origin form.

Top recommendations

1. Complete the integration test suite (ADR-0025 checklist): RLS per role/tenant against real Postgres, concurrency-safe numbering, magic-link single-use. This closes the last verification gap.
2. Add database constraints for valid state transitions (time entry status machine, session timeout range, password complexity) so schema enforces correctness independent of app code.
3. Before multi-tenant production: implement email domain → company_id mapping in profile bootstrap (ADR-0005 TODO). Add unit tests to verify domain isolation.
4. Deploy rate limiting on public contractor auth endpoints (login, password reset, invite acceptance) to prevent brute-force and account enumeration. Use Redis or simple in-memory cache with TTL.
5. Add integration tests for contractor invoice RLS (verify contractor A cannot read contractor B's invoices via Hubstaff sync or direct read). Document the EXISTS policy in migration comments.
6. Enable Supabase leaked-password protection and webhooks for suspicious activity (see ADR-0025). Integrate into alerting so admins are notified of repeated failed logins.

---

UX/UI affordances & accessibility

Maturity: 🟡 Solid

The app demonstrates strong foundational UX/UI discipline with clear adherence to accessibility standards, proper form patterns, and thoughtful component design. Forms follow single-column layout defaults with visible labels and semantic HTML. Destructive actions use risk-proportional confirmation modals. However, multi-column form layouts appear on some admin forms without responsive fallback documentation, placeholder text is used alongside labels in a few cases, and data provenance/last-updated signals are sparse on sensitive payroll pages — critical for trust in a PII-heavy application.

Strengths

• Form design is semantic-first: Field component uses visible labels, inline errors (role=alert), and maintains input state on error recovery
• Destructive action pattern is mature: DangerConfirm modal with risk-proportional confirmation (typed word for high-stake actions) plus consequence description
• Interactive states well-implemented: buttons show disabled/loading, form controls have visible focus indicators (focus-visible outline in brand-600), hover states
• Accessibility fundamentals solid: global focus-visible support (WCAG 2.4.7), prefers-reduced-motion media query respected, semantic HTML (role=alert, role=tooltip, native dialog)
• Keyboard navigation enabled: tab order logical, native inputs (email, tel, date) trigger correct OS keyboards, Dialog component traps focus with Escape/backdrop dismiss
• Mobile-responsive tables: tbl-stack pattern converts rows to stacked cards <640px with data-label for field names — avoids shrink-to-unreadable
• Clear feedback system: toasts with aria-live=polite, explicit success confirmation (notify + Alert components), <100ms response with button state changes
• Money formatting consistent: centavos/cents helpers used throughout, formatPhp/formatUsd applied consistently, tabular-nums class used
• Badge + dot pattern prevents color-alone information: status indicators use both semantic color and dot shape
• Empty states user-friendly: explain what goes here + first action (EmptyState component used in calendar, payslips, contractors pages)

Findings

• 🟡 Medium — Multi-column form layouts without responsive justification
  src/components/admin/ContractorProfileForm.tsx:59, 104, 140
  ContractorProfileForm uses sm:grid-cols-3 to lay out first/middle/last name, Type/Status/Profession, and Phone/Start/End-date fields on desktop. While these collapse to single column on mobile (<640px), the three-column layout on small tablets (640–900px) may reduce readability per Baymard guidelines ("multi-column forms are consistently harder to complete"). No comment in code explains the domain rationale for this deviation.
  Recommendation: Either (1) add a comment explaining why 3-col layout is justified for this admin form (e.g., 'name fields are quick, grouped for efficiency'), or (2) revert to single-column primary layout with multi-column only on lg+ screens. If justified, update ux-ui-guidelines.md to document when multi-column is acceptable.
• 🟡 Medium — Placeholder text used alongside labels in some forms
  src/components/admin/WiseRecipientForm.tsx:39, 49, 59, 70; src/components/admin/FacilityIntakeForm.tsx (multiple)
  WiseRecipientForm (lines 39, 49, 59, 70, 82) and FacilityIntakeForm (multiple instances) use placeholder text in addition to visible labels. While guidelines require visible labels (not placeholder-only), the inclusion of placeholder hints alongside labels is borderline — some users may rely on placeholder as a live label if they tab quickly. This is not a violation but risks cognitive load.
  Recommendation: Reserve placeholder for in-field examples only where the hint text in does not fully clarify format (e.g., placeholder='74ecdaf6-175d-4a93-...' is acceptable for UUIDs). Remove placeholders from fields that already have visible labels and optional/(required) indicators. If placeholder adds value, move the hint to instead.
• 🟡 Medium — Data provenance / last-updated timestamps absent from payroll and PII pages
  src/app/contractor/page.tsx (estimate card, recent pay section); src/app/contractor/payslips/page.tsx (payment amounts)
  Contractor home page displays estimated pay, recent payments, and holiday balance without showing when estimates were calculated or when payment data was last synced. Payslips page shows period dates and amounts but no 'calculated at' or 'locked at' timestamp. Guidelines require 'show data provenance (source + last-updated) on payroll/PII values; never re-ask for data the system already has.' Absence of these signals erodes trust in sensitive financial data.
  Recommendation: Add 'Calculated at [timestamp]' to pay estimate card; add 'Locked [timestamp]' or 'Processed [timestamp]' to payslip rows. On admin payroll pages, surface when a period was closed and payment amounts finalized. Use formatWhen pattern observed in audit page.
• 🔵 Low — Inline validation (on blur/input) not consistently implemented
  Form components throughout (CreateAssignmentForm, ProfileForm, ContractorProfileForm, etc.)
  Forms use server-side validation via useActionState, displaying errors only after submission. No client-side onBlur or onChange validation visible (e.g., email format check, required field highlight-on-blur). Guidelines recommend inline validation for improved UX ('Validate inline (on blur or as the user types), not only on submit').
  Recommendation: Consider adding client-side validation for low-friction fields (email, phone, required fields) using HTML5 validation attributes and aria-invalid state. Wire aria-invalid={!!error} on inputs with Field error state. Preserve server validation as fallback for security-critical checks (Zod validation on actions).
• 🔵 Low — Skeletons/loading states not widely used for async data
  src/components/ui/Skeleton.tsx (exists but rarely used); async pages (admin/contractors, admin/invoices, contractor/calendar)
  Server components load data directly; no skeleton screens visible for operations >300ms. Guidelines recommend: 'Use loading indicators or skeleton screens for anything over ~300ms.' Most admin list pages (contractors, facilities) appear to load synchronously; harder to assess latency without profiling. Contractor calendar page does show an empty state when no assignment exists, but no loading skeleton while assignment data loads.
  Recommendation: Profile data load times; if any critical page loads >300ms, add Skeleton placeholders before data arrives (e.g., skeleton rows in table, skeleton cards). For Suspense boundaries, use Skeleton components to reserve layout space and prevent CLS.
• 🔵 Low — Contrast testing for semantic colors not documented
  src/components/ui/Alert.tsx:8–14; src/components/ui/Badge.tsx:10–19; src/app/globals.css:1–43 (color theme)
  Alert and Badge components use semantic colors (red for danger, green for success, amber for warning) that should meet WCAG AA (4.5:1 normal text, 3:1 UI components). Code defines colors via Tailwind tokens (e.g., border-red-200 bg-red-50 text-red-700) but no contrast ratios are documented. Assuming Tailwind defaults meet AA, but explicit verification is missing.
  Recommendation: Add a note to CLAUDE.md or ADR documenting that semantic color palette was chosen to meet WCAG AA (cite the Tailwind scale used, e.g., Tailwind v3 default colors meet AA at text-700 on bg-50/100). Optionally audit one representative page (e.g., admin home) with a tool like WebAIM Contrast Checker to verify in situ.
• ⚪ Info — Submit button state management is defensive but could be clearer
  src/components/ui/Button.tsx:45; src/components/admin/CreateAssignmentForm.tsx:95; src/components/contractor/ProfileForm.tsx:87
  Button component disables on loading (disabled={disabled || loading}), and many forms double-disable (Button type='submit' loading={pending} disabled={pending}). This is redundant but safe. However, some forms show loading text ('Creating…') while others don't, creating inconsistent UX.
  Recommendation: Standardize: either always show 'Action...' text when pending, or always omit it. Consider adding loading text to all forms via a pattern. Current inconsistency is minor but reduces predictability.
• ⚪ Info — Touch targets appear adequately sized but not systematically validated
  src/components/ui/Button.tsx:17–20; src/components/ui/RowActions.tsx:19
  Buttons use min-h-11 (44px) and min-h-9 (36px) variants, meeting the 44x44px guideline. RowActions buttons (size-9, likely 36px) are slightly under but include gap spacing. No systematic documented review of touch target sizing across all interactive elements.
  Recommendation: Document in ADR or CLAUDE.md that primary targets are ≥44px (min-h-11, min-h-9), and secondary targets (icon buttons) use 36px with sufficient spacing. Optionally add a note that mobile testing is assumed prior to release.

Top recommendations

1. Audit and document multi-column form layouts: clarify when 3-col is justified for admin efficiency, or revert to single-column primary with responsive breakpoints. Update guidelines if exceptions are intentional.
2. Surface data provenance on all payroll and PII pages: add 'Calculated at [time]', 'Locked [time]', or 'Last synced [time]' timestamps to pay estimates, payslips, and contractor details. Critical for trust in a financial app.
3. Consolidate inline validation strategy: decide whether to add client-side validation (HTML5 + aria-invalid) for low-friction fields. Document the pattern in an ADR if implementing; otherwise, clarify in CLAUDE.md that server-only validation is intentional.
4. Profile and add skeleton screens for async data loads >300ms: measure contractor list, invoice list, and other admin pages; if any exceed 300ms, add Skeleton components to reserve space and prevent layout shift.
5. Document color accessibility: add a note to CLAUDE.md or an ADR confirming that semantic color palette meets WCAG AA and documenting which Tailwind scale is in use (or run an audit to verify specific pages).

---

Data integrity & financial correctness

Maturity: 🟡 Solid

The payroll system demonstrates strong foundational correctness for contractor pay computation and batch processing. Money is consistently handled as integer centavos (no float drift), rounding is symmetric and well-tested, and the pay algorithm fixes a critical spec bug (29–43% underpayment). The period-close pipeline is atomic with TOCTOU guards via row locking. However, five findings—one medium-severity (13th-month rounding, Wise variance tolerance), two low-severity (close-lock tolerance, breakdown storage), and two info (assignment active flag, transfer prioritization)—reduce observability and operational clarity at scale.

Strengths

• Integer centavos end-to-end with branded types preventing float mixing
• Pay algorithm fixes spec's 29–43% underpayment bug; full-time contractor earns exactly bi-monthly rate every period
• Atomic prepare/close lifecycle with TOCTOU guards via row locking and FOR UPDATE
• Idempotency safeguards: per-contractor key + unique(idempotency_key) prevents double-pay; 13th-month partial unique index blocks concurrent duplicates
• Holiday proration correctly overlaps multi-day spans across week boundaries via interval OVERLAP
• Wise reconciliation preserves (never hides) variances; snapshots orphans for owner review
• Rate determinism ensures preview and close pick same assignment across separate queries
• UTC-only CalendarDate math prevents timezone leakage into period/day logic

Findings

• 🟡 Medium — 13th-month pay truncates per-period bases instead of rounding entitlement
  src/lib/pay/thirteenthMonth.ts:15; src/server/actions/thirteenthMonth.ts:103
  thirteenthMonthCentavos (lib/pay/thirteenthMonth.ts:15) applies Math.trunc() to each period's base before summing, losing fractional centavos. A contractor with six periods at 1667 centavos (₱16.67) yields (1666×6)/12=833, not (1667×6)/12≈834. Under PD 851, true entitlement is sum-then-divide-then-round.
  Recommendation: Remove Math.trunc on individual periods. Change line 15 to: const total = periodBasicCentavos.reduce((sum, c) => sum + c, 0). Add test with fractional period bases (e.g., 1667 each) to verify stable entitlement year-over-year.
• 🟡 Medium — Wise reconciliation variance tolerance defaults to 0 (exact match only), risky in FX scenarios
  src/server/services/wise/reconcile.ts:99, 122; src/lib/wise/reconcile.ts:121
  reconcileWisePayoutsForCompany (src/server/services/wise/reconcile.ts:99) calls reconcileTransfers with default options (tolerance=0). Any transfer differing by even 1 centavo stays processing indefinitely. Exchange rate rounding at Wise's end will cause variances in multi-currency batches.
  Recommendation: Add company-level setting for variance tolerance (default 50 centavos for PHP, 5 cents for USD). Pass dynamically: reconcileTransfers(payments, transfers, {varianceToleranceCentavos: tolerance}). Document FX rounding expectation in ADR-0020.
• 🔵 Low — Close-and-lock rounding tolerance on hours is implicit and undocumented
  src/db/migrations/0041_fix_close_lock_for_update.sql:71
  close_and_lock_payroll_batch (0041_fix_close_lock_for_update.sql:71) compares round(v_sum, 2) <> round(p_expected_hours, 2), silently tolerating ±0.005 hours (±18 seconds) per entry. Tolerance is not exposed or documented.
  Recommendation: Add explicit comment explaining the 2-decimal tolerance guards subsecond clock drift. Consider a named constant HOURS_TOLERANCE_NUMERIC = 0.01 so it can be reviewed/tuned if needed.
• 🔵 Low — Breakdown JSON stores rounded hours; raw unrounded values are lost
  src/lib/payroll/index.ts:66–69 (round2 calls in serializeBreakdown)
  serializeBreakdown (src/lib/payroll/index.ts:61–72) applies round2() to all week hours before storing in payments.breakdown. If a contractor audits a payslip later, intermediate computation values are unavailable.
  Recommendation: Optionally store both hoursWorked (rounded for display) and hoursWorkedRaw (unrounded for audit). Or document that breakdown is display-precision-only and canonical audit is the pay engine input (time_entries + leave_requests).
• ⚪ Info — Assignment active flag is not checked during pay run computation
  src/server/services/payroll.ts:102–113 (assignment query)
  computePayRun (src/server/services/payroll.ts:102–113) filters assignments by rate and effective_date but not active boolean. A deactivated assignment is still paid. Likely intentional (prevent voids of historical pay) but undocumented.
  Recommendation: Confirm intent in code comment. Optionally warn owner in UI if an assignment was deactivated during or just before the pay period.

Top recommendations

1. Fix 13th-month rounding: Remove Math.trunc on per-period bases. Add test with fractional amounts to verify stable year-to-date entitlement under PD 851.
2. Add Wise variance tolerance: Implement company-level setting (default 50 centavos PHP) passed to reconcileTransfers(). Required for FX or multi-currency batches.
3. Formalize TOCTOU tolerance: Make ±0.005 hour tolerance explicit via constant and comment in close_and_lock_payroll_batch.
4. Document assignment active-flag behavior and emit warning if deactivated during period.
5. Refactor Wise matcher: Use multi-pass or scoring system (reference > recipient_amount > amount_window) to prioritize highest-confidence matches first, ensuring deterministic pairing in large batches.

---

Performance, reliability & testing

Maturity: 🟡 Solid

The codebase demonstrates solid engineering fundamentals with well-architected domain logic (pay engine with comprehensive test coverage at 2,615 test lines), disciplined error handling via Result types, and thoughtful external integration patterns. However, critical gaps exist: no retry logic on Wise reconciliation/transfers despite network fragility, server actions lack systematic test coverage (0 action tests found), loading states are sparse (only 2 error.tsx files across dozens of routes), and query batching in some areas could be tighter. Strengths in logging discipline and caching revalidation strategy don't fully compensate for unguarded edge cases in payment flows—the system handles expected paths well but has brittle failure modes when integrations falter.

Strengths

• Pay engine is exceptionally well-tested (243 test cases covering proration, holidays, 13th month edge cases; tests verify exact centavo precision)
• Strong discipline on logging: no console.log in committed code; centralized JSON logger that masks PII (CLAUDE.md convention enforced)
• Result<T, E> pattern consistently used in domain logic forces callers to handle errors; good separation of expected failures (Result) vs. programmer errors (throw)
• Careful query batching in payroll compute (Promise.all for contractors/time_entries/leaves/facilities, no per-contractor round-trips) and home page (11 queries batched at once)
• Error boundaries in place (error.tsx on admin/contractor layouts) with ErrorRecovery component for graceful fallback
• Good external API client patterns: Hubstaff has token refresh retry on 401, rate-limit handling; Wise reconciliation returns typed Result errors rather than throwing
• Zod validation at every trust boundary (server actions validate FormData before use); input schemas marshalled through dedicated schema modules
• Clean 'use client' discipline: only 106 client components (typical size codebase), concentrated in UI components; most routes remain server-rendered
• RevalidatePath called systematically after mutations (invoices, payroll, announcements, leave, adjustments); ISR/revalidation strategy is cohesive

Findings

• 🔴 Critical — Wise integration has no retry logic on transient failures
  src/server/integrations/wise/client.ts:70-92
  listRecentTransfers() in /Users/olivertrinidad/Documents/GitHub/NPM-Helper-App/src/server/integrations/wise/client.ts (lines 63-92) catches errors and returns err() but never retries on network timeouts or transient 5xx responses. reconcileWisePayoutsForCompany() accepts failure silently and snapshots the error to settings. If Wise is briefly unreachable, payouts are unreconciled until manual trigger, and no alerting informs the owner.
  Recommendation: Implement exponential backoff retry (2-3 attempts) for network errors and transient 5xx on Wise calls. Return different error codes for retryable vs. non-retryable failures. Add structured logging of Wise request/retry attempts (include request duration, retry count) so operations can debug delays.
• 🟠 High — Server actions lack systematic test coverage
  src/server/actions/ (all files) vs. tests/ (0 action tests)
  32 server action files (src/server/actions/*.ts) with no corresponding unit tests found. Actions like createHire, generateInvoice, addPaymentAdjustment implement complex business logic (hire.ts has 5-step transaction with rollback; invoices.ts validates per-role constraints). These are tested only via integration/E2E if at all, leaving mutation edge cases untested (e.g., race conditions, partially-failed state transitions).
  Recommendation: Create unit tests for high-risk actions: test authorization checks (non-owner rejection), input validation failures, database error handling (simulate constraint violations), and rollback paths. Start with hire.ts (multi-step), invoices.ts (state machine), and paymentAdjustments.ts (locking logic). Aim for ~100 action-specific tests.
• 🟠 High — Missing loading.tsx/Suspense boundaries on slow pages
  src/app/admin/page.tsx:97-145, src/app/contractor/page.tsx:77-119
  Admin home page (src/app/admin/page.tsx) runs 11 concurrent queries synchronously; contractor page runs Promise.all on 5 queries. No loading.tsx or Suspense boundary. If any query times out (e.g., time_entries table scan over 2000 rows, limit(2000) on line 115), the entire page hangs. Only 2 error.tsx files exist; no fallback loading states.
  Recommendation: Wrap slow data loads in Suspense. Example: <Suspense fallback={}> for the overage query. Add loading.tsx at src/app/admin/loading.tsx and src/app/contractor/loading.tsx with placeholder cards. Consider pagination or WHERE limits on time_entries (select 'contractor_id, assignment_id, entry_date' .limit(2000) can return 2000+ rows).
• 🟡 Medium — RLS policy enforcement not tested; server actions verify once but not twice
  src/server/actions/invoices.ts:74-150
  Server actions verify session auth once (e.g., getCurrentAdmin on line 74 of invoices.ts) and forward admin.companyId to queries. RLS should filter, but if RLS is misconfigured, no second check catches it. No integration tests verify that a contractor cannot see another company's invoices via RLS bypass.
  Recommendation: Add integration tests that query the database directly as a service client, then verify RLS is active. Example: fetch invoice as company A's service client, then as company B's service client, verify B cannot see A's invoice. Document RLS assumptions in ADR.
• 🟡 Medium — Query limits on dashboard and list pages may scan too much data
  src/app/admin/page.tsx:112-120, src/app/admin/invoices/page.tsx (if using similar patterns)
  Admin home page selects time_entries with .limit(2000) and no date range filter (line 115). Invoices selects .limit(1000) without date range (line 120). If a facility bills weekly for 2 years, that's 100+ invoices per facility; with 10 facilities, the home page scans unnecessarily. No indexes cited for (company_id, status) or (entry_date) columns.
  Recommendation: Add explicit date range filters: e.g., .gte('entry_date', lastWeekStart) for time entries. Document indexes required: (company_id, status, created_at desc) on invoices; (company_id, assignment_id, entry_date) on time_entries. Run EXPLAIN ANALYZE on slow queries in production to confirm index usage.
• 🟡 Medium — No retry/fallback for Hubstaff sync; stale data silently used
  src/server/integrations/hubstaff/api.ts:149-200 (pagination loop)
  fetchHubstaffDailyEntries (src/server/integrations/hubstaff/api.ts:141-149+) handles 401 refresh but no retry on transient network errors or 500s. If sync fails mid-pagination (e.g., page 50 of 200 times out), the partial result is returned, and that day's hours go unrecorded. No warning surfaces to the UI.
  Recommendation: Implement simple retry-on-timeout logic: catch timeouts, sleep 2s, retry the current page up to 3 times. If max retries exceeded, return Err with context (which page failed, how many rows fetched so far). Log at warn level so ops can audit syncs.
• 🟡 Medium — 'use client' proliferation in components (106 files) may inflate bundle
  src/components/ (106 'use client' files)
  While discipline is good, 106 client components is sizable. No bundle analysis cited. Some UI primitives (Button, Badge, Table) are marked 'use client' even though they could be server components if no interactivity is needed in specific contexts.
  Recommendation: Run next/bundle-analyzer to profile client bundle size. Identify 3-5 heavily-imported client components (e.g., Button) and measure impact if converted to server-renderable versions. No immediate action needed if bundle is <150KB, but document the audit.
• 🔵 Low — cron service has no retry logic or scheduled alerting
  src/server/services/cron.ts:29-104
  lockEndedAssignments and accrueOverdueInterest in src/server/services/cron.ts run on schedule but don't log failures or retry on transient DB errors. If a cron execution fails partway (e.g., interest update succeeds for 50 invoices, then Postgres goes down), the rest are silently skipped.
  Recommendation: Wrap each cron task in try/catch; log the outcome (success count, error) at info level. Consider idempotency keys (e.g., daily reconciliation records a checkpoint in settings so reruns are safe). Add a health check endpoint that verifies cron has run in the last 25 hours.
• 🔵 Low — Test coverage weighted toward happy path; edge cases for payment state transitions unclear
  tests/lib/pay/pay.test.ts vs. src/server/services/payroll.ts
  Pay engine tests (243 lines) are excellent for proration logic but don't test the DB-level payment-lock trigger behavior or concurrent close + adjustment scenarios. Payroll closure is one of the highest-risk flows (locks $$ in DB).
  Recommendation: Add integration test for payroll close: create draft batch, add adjustment mid-close, verify payment is locked and adjustment is rejected with 'disbursed' error. This scenario lives in /src/server/actions/payroll.ts (closePayrollBatch) but is untested.

Top recommendations

1. Implement retry logic (exponential backoff, 2-3 attempts) on Wise and Hubstaff API calls; return typed error codes so callers can distinguish retryable (network) from non-retryable (auth) failures. Add structured logging (duration, attempt count, error message).
2. Create ~100 unit tests for server actions, starting with hire.ts (rollback), invoices.ts (state machine), and paymentAdjustments.ts (locking). Use Zod to generate test data; mock database client to simulate constraint violations.
3. Add Suspense boundaries and loading.tsx on the admin and contractor home pages. Wrap Promise.all calls in Suspense; create placeholder skeletons for slow-loading sections (overage, invoices, health allowance). Set a 3-5 second timeout on queries so users see 'Taking longer than usual' instead of hanging.
4. Document and verify RLS is active: add integration test that fetches data as two different company service clients; verify the second cannot see the first's data. Create an ADR for RLS assumptions (e.g., 'company_id filters all tables').
5. Audit and index key query patterns: add (company_id, status, created_at desc) on invoices, (company_id, assignment_id, entry_date) on time_entries. Run EXPLAIN ANALYZE on slow queries (home page, invoice list, payroll preview) to confirm index usage; document in a QUERIES.md file.

---

Preliminary, automated review — re-run after addressing the Critical/High items.