WealthySmart/codebase-review/03-frontend.md

Frontend Review — WealthySmart

Hat: Senior Frontend Engineer (React 19 / TypeScript / Vite / Tailwind 4 / Hono / CopilotKit). Focus: data fetching, state, type safety, auth flow, accessibility, the Hono gateway.

Executive Summary

WealthySmart's frontend is a well-structured React 19 + Vite + TypeScript app behind a Hono gateway server. The codebase shows good practices in component organization, Tailwind styling, and accessibility basics. The dominant weakness is the hand-rolled data-fetching layer: no request cancellation, no caching/deduplication, and inconsistent error/loading states — several pages swallow API errors into console.error and render empty UI. The CopilotKit integration is sophisticated but leans on heavy middleware workarounds (message dedup, snapshot reconciliation) that indicate upstream fragility and currently swallow their own errors. Auth uses httpOnly cookies (ws_token) over same-origin requests — a solid choice — but the 401 handling can redirect-loop and the auth probe treats network failure as logged-out.

Strengths

• Type safety — comprehensive interfaces in api.ts for Transaction, RecurringItem, Budget types; only a couple of any escapes.
• Component architecture — clean decomposition into pages, hooks (useBudget), and contexts (Theme, Privacy, Auth).
• Accessibility fundamentals — focus rings, aria-labels, title attributes on interactive elements; privacy mode via data-sensitive masking; logical heading hierarchy.
• Error messages where they exist — TransactionModal handles 409 conflicts with a user-readable message.
• Format helpers — locale-aware CRC formatting (formatAmount, formatCRC) used consistently in most modules.
• Hono gateway — cleanly proxies FastAPI and hosts the CopilotKit runtime; DeduplicateToolCallMiddleware / ReconcileSnapshotMiddleware address known CopilotKit issues head-on.
• Responsive baseline — mobile menu, adaptive layouts; tables are the weak spot (see FE/UX findings).

Findings

[FE-01] No Request Cancellation in useBudget — High

Location: src/hooks/useBudget.ts:52–59 Evidence:

useEffect(() => {
  fetchProjection();
  fetchRecurringItems();
}, [fetchProjection, fetchRecurringItems]);

Why it matters: Changing year/selectedMonth while a request is in flight lets the stale response overwrite newer state. No AbortController or request-ID tracking anywhere in the app. Recommendation: Abort on dependency change (return () => ctrl.abort()), or adopt TanStack Query which solves this class wholesale (see FE-10).

[FE-02] Analytics Swallows Errors, No Loading State — Medium

Location: src/pages/Analytics.tsx:77–95 Evidence: Promise.all([...]).then(...).catch(console.error) — failures leave silently empty charts; no skeleton during fetch. Recommendation: loading / error / retry states with visible UI.

[FE-03] Transaction Fetch Races in Budget Page — High

Location: src/pages/Budget.tsx:44–77 Evidence: fetchTransactions re-fires on txSearch/txSource/selectedMonth changes with no cancellation; last-to-resolve wins even if stale. Recommendation: AbortController per request keyed to the effect; or query library.

[FE-04] Unguarded Array Indexing in Pensions ROI Fallback — Medium

Location: src/pages/Pensions.tsx:292–315 Evidence: Fallback computes chartData[len - 1][key] - chartData[len - 2][key] - ...; with len < 2 this silently yields 0/negative garbage. Recommendation: Guard if (len < 2) { acc[key] = 0; } before indexing.

[FE-05] Post-Upload Refetch Can Hang the Upload UI — Medium

Location: src/pages/Pensions.tsx:343–365 Evidence: await loadData() after upload has no timeout; "Procesando…" persists indefinitely if the backend stalls. Recommendation: Timeout the refetch and surface an actionable message.

[FE-06] catch (err: any) in TransactionModal — Medium

Location: src/components/TransactionModal.tsx:97 Evidence: err.response?.status === 409 assumes a shape; non-HTTP errors (network) silently fall through to console.error with no user message. Recommendation: Type-guard with the app's ApiError; always set a user-visible error in the else branch.

[FE-07] localStorage Read in Effect Instead of Lazy Initializer — Low

Location: src/contexts/theme-context.tsx:16–24, src/contexts/privacy-context.tsx:13–15 Evidence: Initial theme read happens in useEffect, causing a flash of wrong theme on mount. Recommendation: useState(() => localStorage.getItem("theme") ?? ...) lazy initializer.

[FE-08] No Security Headers from the Hono Gateway — Low

Location: frontend/server.ts:379–416 Evidence: No CSP, X-Frame-Options, X-Content-Type-Options, or Referrer-Policy on served responses. Recommendation: One small Hono middleware (pairs with backend SEC-07; the user-facing HTML is served from here, so this is the layer that matters most for CSP/frame protection).

[FE-09] Coupled Effects in useBudget — Medium

Location: src/hooks/useBudget.ts:27–35 Evidence: Projection and recurring-items fetches share one effect; failure modes and re-run triggers are entangled. Recommendation: One effect per concern with explicit dependencies.

[FE-10] No Query Caching or Deduplication — High

Location: src/lib/api.ts:15–66 and all consumers Evidence: Every api.get() is a fresh fetch; parallel consumers of the same endpoint each hit the network; every page re-fetches everything on mount. Why it matters: This is the root cause behind FE-01/02/03/09/24 and ARCH-13. One architectural decision fixes the whole class. Recommendation: Adopt TanStack Query: per-key caching, dedup, cancellation, retries, isLoading/isError states, and mutation invalidation. Migrate page by page (Budget first).

[FE-11] CopilotKit Middleware Swallows Parse Errors — Medium

Location: frontend/server.ts:350–368 Evidence: catch { return; } in beforeRequestMiddleware — malformed bodies silently skip orphan-tool-call pairing, producing confusing downstream runtime failures. Recommendation: console.error with context before returning; given the documented history of card-persistence bugs (see reference_agui_client memory), observability here pays for itself.

[FE-12] Month Detail Rendered Without Existence Check — Medium

Location: src/pages/Budget.tsx:115–154 Evidence: Month navigation renders detail UI without checking monthDetail exists for the selection. Recommendation: monthDetail ? <MonthlyDetail/> : loading ? <Skeleton/> : <EmptyState/>.

[FE-13] XSS Posture Is Fine — Watch dangerouslySetInnerHTML — Informational

Location: src/components/TransactionList.tsx:129, src/components/chat/ChatCards.tsx:128 Evidence: Merchant/category strings render through JSX (auto-escaped). No dangerouslySetInnerHTML found. Not a current bug — recorded as a guardrail since this data originates from emails (attacker-influenceable).

[FE-14] No Runtime Validation of API Responses — Medium

Location: src/lib/api.ts and all consumers Evidence: Response types are compile-time assertions only; backend schema drift produces undefined rendering or crashes far from the cause. Recommendation: Either Zod-parse at the API-client boundary for the few critical shapes, or (better long-term) generate types from FastAPI's OpenAPI schema once ARCH-16 lands (openapi-typescript).

[FE-15] Auth Probe Treats Network Failure as Logged Out — Medium

Location: src/AuthContext.tsx:22–29 Evidence: .catch(() => setAuthenticated(false)) — a transient network blip on app load kicks an authenticated user to the login screen. Recommendation: Distinguish network errors (retry once, or keep an "unknown" state) from a real 401.

[FE-16] Missing aria-pressed on Toggle Buttons — Low

Location: src/pages/Pensions.tsx:515–530 (fund visibility toggles) Evidence: Toggles convey state visually only. Recommendation: aria-pressed={visibleFunds.has(key)}; audit other toggles (privacy mode, theme).

[FE-17] en-US Dates in an es-CR App — Low

Location: src/pages/Analytics.tsx:238–240 (chart labelFormatter), also src/lib/format.ts:19–21 Evidence: Chart tooltips show "Jan", "Feb" while the page is in Spanish. Recommendation: Centralize on es-CR (see UX-19 for the full localization sweep).

[FE-18] useBudget Return Object Not Memoized — Low

Location: src/hooks/useBudget.ts:86–103 Evidence: Fresh object identity every render; consumers using it as a dependency re-run unnecessarily. Recommendation: Wrap in useMemo (moot if migrated to TanStack Query).

[FE-19] Unbounded Upload-Result List — Low

Location: src/pages/Pensions.tsx:822–865 Evidence: All returned snapshots render without scroll containment or truncation. Recommendation: Slice to ~10 with "and N more".

[FE-20] Dead Code: AgentHomeClient.tsx — Low

Location: src/components/AgentHomeClient.tsx vs src/pages/Asistente.tsx Evidence: Two near-identical components; only Asistente.tsx is routed. Recommendation: Delete AgentHomeClient.tsx.

[FE-21] (Withdrawn) — the shadcn re-export concern was speculative; no concrete evidence found. Dropped.

[FE-22] 401 Handler Can Redirect-Loop — Medium

Location: src/lib/api.ts:48–52 Evidence: On 401: call logout, window.location.replace("/login"). If any call from the login page itself 401s, this loops. Recommendation: Skip the redirect when already on /login.

[FE-23] Proxy Has No Error Handling for Backend Failures — Medium

Location: frontend/server.ts:385–402 Evidence: proxyToBackend returns fetch(target, init) with no try/catch; a connection refusal becomes an opaque 500. Recommendation: try/catch → log → c.json({ error: "Backend unavailable" }, 502).

[FE-24] No Timeouts on Any Fetch — Medium

Location: All pages Evidence: No AbortSignal.timeout() anywhere; hung backends mean indefinite spinners. Recommendation: Default AbortSignal.timeout(30_000) in the shared request() helper in api.ts — one change covers the app.

Quick Wins (under 1 hour each)

1. es-CR locale in Analytics chart labels — FE-17
2. Delete AgentHomeClient.tsx — FE-20
3. Security-headers middleware in server.ts — FE-08
4. Guard /login in the 401 redirect — FE-22
5. try/catch + 502 in proxyToBackend — FE-23
6. AbortSignal.timeout(30000) in the shared request helper — FE-24
7. Guard chartData.length >= 2 in Pensions ROI — FE-04
8. aria-pressed on fund toggles — FE-16
9. Log CopilotKit middleware parse failures — FE-11
10. Lazy initializers for theme/privacy contexts — FE-07

Strategic recommendation: Most Medium/High findings here share one root cause — the absence of a query layer. Adopting TanStack Query (Plan Phase 3) retires FE-01, FE-03, FE-09, FE-10, FE-18, FE-24 and ARCH-13 in one move, and makes FE-02-style error states the default rather than per-page work.