ai_synth/docs/audit/frontend-report.md

Frontend Audit Report (SolidJS)

Date: 2026-03-26 Scope: frontend/src/ -- SolidJS + Tailwind CSS v4 application Files reviewed: 30+ files across pages, components, API modules, utils, i18n, and types

---

Executive Summary

The frontend is well-structured for a learning project of this scope. It demonstrates solid understanding of SolidJS fundamentals: lazy loading, proper Show/For usage, context-based auth, typed i18n, and a clean API client layer. The codebase is consistent in style and convention.

That said, several structural issues deserve attention. The Settings page has grown to 1026 lines and mixes concerns that should be decomposed. Some setTimeout calls lack onCleanup guards, creating potential memory leaks. A handful of SolidJS-specific anti-patterns (destructured reactive props, children: any) reduce type safety and could cause subtle bugs. The i18n system, while type-safe at the key level, has an XSS surface via innerHTML usage in one location. None of these are critical in a single-tenant self-hosted deployment, but they represent the most impactful areas for improvement.

Overall quality: Good. The top priorities are decomposing the Settings page, adding missing onCleanup guards, and fixing the innerHTML usage.

---

SolidJS Patterns

Reactive Primitives

Signals and derived state -- well done. The AuthContext correctly models derived signals (isAuthenticated, isAdmin) as plain functions over user(), avoiding redundant signals. The createResource usage in ApiKeyManager and Settings for provider/key data is appropriate.

createEffect usage -- mostly correct, one concern. Effects in GenerateSynthesis for auto-redirect and error handling are clean. However, the effect in Settings.tsx (line 81-93) that checks provider availability reads both providers() and settings(), which means it will re-fire whenever either signal changes -- including when the user changes an unrelated setting field. This is harmless (idempotent) but wasteful. Using on() to track only providers would be more precise:

// Current: re-fires on any settings change
createEffect(() => {
  const providerList = providers();
  const currentSettings = settings();  // unintended dependency
  ...
});

// Better: only fires when providers load
createEffect(on(() => providers(), (providerList) => { ... }));

createResource vs onMount inconsistency. Some data fetching uses createResource (ApiKeyManager, Settings providers), while most pages use onMount + manual signal (Home, Sources, LlmLogs, GenerateSynthesis). This is not wrong, but createResource would provide free loading/error states and refetch capabilities. The inconsistency suggests no team-wide convention was established.

Component Composition

Show and For -- correctly used throughout. The callback form of Show (e.g., <Show when={user()}>{(currentUser) => ...}) is used in Navbar and MobileMenu, which is the correct SolidJS pattern for narrowing nullable values. For is used for all list rendering, never .map().

Step status rendering in GenerateSynthesis -- could use Switch/Match. Lines 335-357 use three consecutive <Show> blocks for the three possible step statuses. This is semantically an exclusive match and would be clearer as:

<Switch>
  <Match when={status() === 'done'}>
    <CheckCircle ... />
  </Match>
  <Match when={status() === 'in-progress'}>
    <Loader2 ... />
  </Match>
  <Match when={status() === 'pending'}>
    <Circle ... />
  </Match>
</Switch>

The current approach renders correctly because only one condition is true at a time, but Switch/Match makes the mutual exclusion explicit and avoids evaluating all three conditions.

Performance Concerns

completedSteps() in GenerateSynthesis is computationally expensive per call. This derived function (lines 126-160) iterates over all events and performs multiple findIndex calls on every render triggered by new SSE events. For the typical 4-step pipeline this is negligible, but the algorithmic complexity is O(steps * events). If the event stream grew large, this would become visible. Consider memoizing with createMemo.

For with reactive lookup in ApiKeyManager. The getKeyForProvider function (line 33) scans the apiKeys() resource on every call. Since it is called inside the For loop's render function, it re-evaluates whenever apiKeys changes, which is correct. However, the returned value is not a signal -- it is a plain value computed from reactive data accessed during render. This works in SolidJS because For tracks the parent signal, but it means the entire provider list re-renders when any key changes. For 3 providers this is negligible.

No unnecessary re-renders detected. The project correctly avoids the React-style anti-pattern of destructuring signals at the component top level. Signal reads are deferred to JSX expressions (e.g., {settings().theme} rather than const theme = settings().theme outside JSX).

---

Component Architecture

Oversized Components

Settings.tsx (1026 lines) is the primary concern. This single component handles:

• Settings form (theme, age, items, articles, batch size, history days)
• Provider selection with auto-detection and warning
• Brave Search key management (save, test, delete, toggle)
• Search agent behavior textarea
• Category list with add/remove
• Rate limit configuration
• JSON export/import with optional API key inclusion
• API key manager delegation

This should be decomposed into at least 5 sub-components:

1. SettingsForm -- basic numeric/text fields
2. ProviderSelector -- provider dropdown, model dropdowns, web search badge
3. BraveSearchSection -- key management + toggle (this is essentially a duplicate of ProviderKeyCard logic)
4. CategoryManager -- category list CRUD
5. RateLimitSection -- rate limit inputs and reset

The Brave Search key management (lines 57-63, 134-179, 582-678) duplicates the same save/test/delete pattern already implemented in ProviderKeyCard within ApiKeyManager.tsx. This is a clear DRY violation -- Brave Search should be treated as another provider key card.

Other pages are appropriately sized. Home (240 lines), GenerateSynthesis (404 lines), Sources (~300 lines), and LlmLogs (159 lines) are all reasonable for their complexity.

State Management

Auth context is clean and minimal. It provides the right abstraction surface (user, loading, isAuthenticated, isAdmin, logout, refreshUser). The refreshUser function is available but appears unused in the current codebase -- verify if this is needed.

Toast context is well-implemented. The module-level activeTimers Map for managing auto-dismiss timers is pragmatic. The aria-live="polite" on the toast container is a good accessibility touch.

No global state beyond auth and toast. Settings and other data are fetched per-page via onMount. This is appropriate for the application's complexity level -- introducing a global store would be premature.

The deleteTimers signal in Home.tsx stores setTimeout handles in a reactive signal. This works but is unusual. The timers are stored so they can be cleared on the second (confirming) click. However, these timers are never cleaned up on component unmount -- if the user navigates away while a delete confirmation is pending, the timeout callback will still fire and call setDeletingId on an unmounted component. This should use onCleanup.

Reusability

Button component exists but is rarely used. A well-typed Button component with variant support (primary, secondary, danger), loading state, and icon slot lives in components/ui/Button.tsx. However, across the pages audited, buttons are overwhelmingly created with inline Tailwind classes rather than using this component. This leads to significant class string duplication. For example, the indigo primary button pattern appears in at least 12 places with minor variations.

LoadingSpinner is consistently reused across all pages -- good.

ProviderKeyCard is a good reusable pattern but its logic is duplicated for Brave Search in Settings.tsx as noted above.

No shared form input components. Every input field repeats the full Tailwind class string: "shadow-sm focus:ring-indigo-500 focus:border-indigo-500 block w-full sm:text-sm border-gray-300 rounded-md py-2 px-3 border". A TextInput, NumberInput, and Select component would reduce duplication and ensure visual consistency.

---

Type Safety

Overall: strong. The codebase makes effective use of TypeScript across all layers.

Strengths:

• All API request/response types are defined in types.ts with proper interfaces
• The isApiError type guard is used consistently for error handling
• The i18n system uses TranslationKey (derived from keyof typeof fr) to make translation keys type-checked at compile time
• satisfies is used in the API client for throw expressions (line 60 of client.ts) and API calls (syntheses.ts line 86)
• Props interfaces are defined for all components

Issues:

1. children: any in Layout and AdminLayout. Both layout components type their children prop as any instead of using ParentComponent or JSX.Element. This loses type safety on what can be passed as children. The fix is trivial: use ParentComponent as done in App.tsx's ProtectedLayout.

2. Non-null assertion in ApiKeyManager. Line 249 uses props.apiKey!.key_prefix inside a <Show when={isConfigured()}> block. While logically safe (the Show guard ensures the value exists), the compiler cannot verify this. Using Show's callback form ({(key) => key().key_prefix}) would eliminate the assertion.

3. ApiClient.request returns undefined as T for 204 responses (line 76 of client.ts). This is a type lie -- the caller may not expect undefined. The methods that return void (like delete) work correctly because undefined satisfies void, but if a caller expected a non-void return from a 204, it would fail at runtime. Consider constraining 204-returning methods to Promise<void> at the call site.

4. isApiError type guard is loose. It checks for the presence of status and message properties but does not verify their types. Any object with these two keys would pass. Adding typeof error.status === 'number' would tighten it.

5. DEFAULT_SETTINGS contains hardcoded French text (category names, search behavior). This is a content/i18n boundary violation -- if the app were ever localized, these defaults would need to come from the i18n system.

---

Code Organization

File Structure -- Good

The project follows a clear convention:

• pages/ for route-level components
• components/ for shared UI
• api/ for HTTP communication
• utils/ for pure helpers
• contexts/ for global state
• i18n/ for translations
• types.ts as the single type definition file

This separation is clean and navigable.

Import Patterns -- Consistent

All imports use the ~/ path alias (configured in vite.config.ts), avoiding fragile relative paths. Imports are grouped logically (solid-js, router, icons, local modules) though not enforced by a linter rule.

Separation of Concerns -- Mostly Good

API layer is well-isolated. The client.ts singleton handles all HTTP concerns (CSRF headers, credentials, 401 redirect, error parsing). Individual API modules (auth.ts, settings.ts, etc.) are thin wrappers that only map routes to typed calls. This is excellent.

Business logic leaks into pages. The Settings page contains import/export logic (JSON serialization, file download, file parsing), Brave Search key management, and category validation. These should be extracted into utility functions or a settings service module.

The syntheses.ts API module contains triggerDownload and fetchFile utility functions that are not synthesis-specific. These should live in a shared utils/download.ts module so other features (e.g., CSV export in Sources) could reuse them.

Missing Error Boundaries Per-Route

The application has a single top-level ErrorBoundary wrapping the entire router. An error in any page will replace the entire app with the error screen. Consider wrapping each route's content with a more localized error boundary that preserves the navigation bar.

---

UI/UX Patterns

Error Handling -- Consistent But Could Be Unified

Every page implements its own error display pattern with slight variations:

• Home: red banner with border border-red-200
• GenerateSynthesis: red banner with border-l-4 border-red-400
• LlmLogs: red box with border border-red-200

A shared ErrorAlert component would standardize these.

Error handling across the codebase consistently uses the isApiError type guard to surface server messages or fall back to i18n strings. This is a good pattern.

console.error usage in Sources.tsx (lines 103, 174) and ArticleHistory.tsx (line 104) should be replaced with user-visible error feedback (toast or inline message). Errors silently logged to the console are invisible to the user.

Loading States -- Good

All pages that fetch data show <LoadingSpinner /> during loading via <Show when={!loading()} fallback={<LoadingSpinner />}>. The spinner supports fullPage and size variants.

Form Handling

No form validation library. Settings and Sources pages implement validation inline. This is acceptable at current scale but becomes harder to maintain as forms grow. The Settings page in particular has no client-side validation -- invalid values (e.g., negative max_age_days) are sent to the server.

Controlled inputs use value + onInput. This is the correct SolidJS pattern (not onChange which fires on blur in some browsers). Numeric inputs use parseInt(...) || defaultValue for fallback, which is pragmatic but means a user typing "0" gets the default instead.

The import flow in Settings does not auto-save. After importing a JSON file, the user sees a success message saying "N'oubliez pas d'enregistrer" (don't forget to save). This is a potential UX pitfall -- users may import and navigate away, losing changes.

Accessibility

Good:

• aria-label on icon-only buttons (mobile menu toggle, show/hide key)
• aria-live="polite" on toast container
• aria-hidden="true" on decorative icons
• role="alert" on toast items
• Semantic HTML (<nav>, <main>, <label for="...">)
• Focus ring styles on all interactive elements

Needs attention:

• The <details> elements in LlmLogs use list-none to hide the default marker but provide no accessible name or ARIA attributes.
• The delete confirmation pattern (click once to arm, click again to confirm) has no visible text change on Home page cards -- only a color shift and a tooltip change. Users relying on screen readers would miss this state change.
• No skip-to-content link.
• Color alone is used to distinguish toast types (success=green, error=red, info=blue). The icons provide a secondary cue, which is good, but there is no text prefix like "Error: " or "Success: ".

Security Concern: innerHTML Usage

GenerateSynthesis.tsx line 249 uses innerHTML to render a translated string:

<p innerHTML={t('generate.description', { days: ..., theme: ... })} />

The theme value comes from settings().theme, which is user-controlled content stored in the database. If a user sets their theme to <img src=x onerror=alert(1)>, this would execute arbitrary JavaScript. In a single-tenant self-hosted app this is self-XSS (the user can only attack themselves), but it is still a bad practice. Replace innerHTML with safe text interpolation, or sanitize the value.

---

Prioritized Recommendations

Priority 1 -- Fix Bugs and Security Issues

1. Remove innerHTML usage in GenerateSynthesis.tsx. Replace with safe text rendering. The translation string uses {days} and {theme} placeholders -- restructure it to use separate <span> elements or a template that does not require HTML parsing.

2. Add onCleanup for setTimeout in Home.tsx. The deleteTimers signal stores timeout handles that are never cleared on unmount. Add an onCleanup that iterates and clears all active timers.

3. Add onCleanup for setTimeout in ArticleHistory.tsx (line 95). The 3-second confirmation timeout is not cleaned up on unmount.

4. Add onCleanup for setTimeout in SynthesisDetail.tsx (line 150). The 5-second email success timeout is not cleaned up on unmount.

Priority 2 -- Structural Improvements

5. Decompose Settings.tsx into sub-components (ProviderSelector, BraveSearchSection, CategoryManager, RateLimitSection). Target: each sub-component under 200 lines.

6. Eliminate Brave Search key management duplication by reusing ProviderKeyCard from ApiKeyManager.tsx or extracting the shared pattern.

7. Replace children: any with ParentComponent in Layout.tsx and AdminLayout.tsx.

8. Use the existing Button component more broadly, or remove it if the team prefers inline styling. Current inconsistency is confusing.

Priority 3 -- Code Quality

9. Standardize data fetching. Pick either createResource or onMount + signals and apply consistently. createResource is recommended for simple GET-on-mount patterns.

10. Extract shared form input components (TextInput, NumberInput, Select) to reduce Tailwind class duplication.

11. Create a shared ErrorAlert component for consistent error display across pages.

12. Replace console.error calls in Sources.tsx and ArticleHistory.tsx with user-visible feedback (toast or inline error).

13. Use Switch/Match instead of multiple <Show> blocks for mutually exclusive conditions (GenerateSynthesis step icons).

Priority 4 -- Polish

14. Move triggerDownload and fetchFile from api/syntheses.ts to a shared utility module.

15. Tighten the isApiError type guard to check typeof status === 'number'.

16. Extract business logic from Settings.tsx (JSON export/import, category validation) into dedicated utility functions.

17. Consider route-level error boundaries to preserve the navbar on page errors.

18. Add i18n-aware defaults for DEFAULT_SETTINGS so category names and search behavior are not hardcoded in French.

19. Narrow the createEffect in Settings.tsx (line 81) to track only providers() using on().