Cookie Frontend Code Review

← Back to article

Repository: https://github.com/matthewdeaves/cookie Commit: cfaafa8 Date: 2026-01-25 Scope: frontend/src/ (React/TypeScript)

This review was generated by Claude Opus 4.5 through direct analysis of the codebase and metrics collected by the CI dashboard (GitHub Actions: ESLint, test coverage, complexity analysis, security scanning).

This review examines actual code quality beyond what the metrics dashboard measures. Metrics catch symptoms (test coverage, complexity averages). This review looks for structural problems, maintainability issues, and architectural debt.

Executive Summary

The frontend is functional and follows React best practices in component organization. Code quality is acceptable for a prototype built in four days. However, several architectural patterns will become maintenance problems as the app grows:

Critical issues:

App.tsx is a god component (337 lines, 10 state variables, manages 10 screens)
No routing library - manual state-based navigation is fragile
Prop drilling creates tight coupling between screens
useTimers hook has race condition bugs

Moderate issues:

Inconsistent error handling (some failures silent, others show toasts)
Duplicate utility functions (formatTime in 2 places)
Missing performance optimizations (useMemo, useCallback)
20+ loading state booleans across the app (repeated pattern)

1. Architecture & Structure

God Component: App.tsx

Location: frontend/src/App.tsx Lines: 337 total, AppContent component spans lines 28-327

What it manages:

10 screens: Home, Search, RecipeDetail, Collections, CollectionDetail, Favorites, AllRecipes, PlayMode, Settings, ProfileSelector
State for all screens: 10 state variables including currentScreen, selectedRecipeId, selectedCollectionId, searchQuery, playModeRecipe, favoriteRecipeIds, theme, currentProfile, pendingRecipeForCollection, previousScreen
Navigation logic: 15+ handler functions for screen transitions (handleRecipeClick, handleSearch, handleCollectionClick, etc.)
Shared state: favoriteRecipeIds Set, AI availability, theme persistence

Problem: Violates single responsibility principle. Makes testing difficult - you can't test a screen in isolation because it depends on App.tsx state management.

Example of complexity (lines 135-143):

const handleRecipeDetailBack = () => {
  setSelectedRecipeId(null)
  // Go back to previous screen
  if (previousScreen === 'search' || previousScreen === 'all-recipes' || previousScreen === 'favorites') {
    setCurrentScreen(previousScreen)
  } else {
    setCurrentScreen('home')
  }
}

This logic exists because there's no proper routing. Each screen needs custom "back" behaviour.

Better approach:

Use React Router or similar for screen management
Extract navigation context for inter-screen communication
Move screen-specific state into the screens themselves
App.tsx should only manage truly global state (profile, theme, AI status)

2. State Management

Prop Drilling

Props are passed 2-3 levels deep throughout the app. Example chain:

App.tsx
  ├─ onRecipeClick={(id) => {...}}
  └─ Home.tsx
       ├─ onRecipeClick={onRecipeClick}
       └─ RecipeCard.tsx
            └─ onClick={onClick}

Files affected:

All 10 screens receive navigation callbacks from App.tsx
RecipeCard receives callbacks from 4 different screens

Problem: Changes to navigation require updates in multiple files. Adding a new parameter to onRecipeClick means touching App.tsx, Home.tsx, Favorites.tsx, AllRecipes.tsx, CollectionDetail.tsx, and RecipeCard.tsx.

Better approach: Navigation context or routing library.

Settings.tsx State Management

Location: frontend/src/screens/Settings.tsx Lines: 1,314 total, state declarations lines 48-97

State variables: 30 separate useState calls:

API key management: apiKey, testingKey, savingKey
Prompt editing: editingPromptType, editForm, savingPrompt
Source testing: testingSource, testingBatch, batchTestProgress
Profile deletion: showDeleteModal, deletePreview, deletingId, deleting
Reset flow: showResetModal, resetPreview, resetStep, resetConfirmText, resetting

Problem: Related state scattered across 23 variables. Changes to reset flow require updating 4-5 variables in sync. Easy to introduce bugs by forgetting to update one.

Better approach: Use useReducer for related state:

const [resetState, dispatchReset] = useReducer(resetReducer, {
  showModal: false,
  preview: null,
  step: 1,
  confirmText: '',
  inProgress: false,
})

3. Error Handling

Inconsistent Patterns

Silent failures:

App.tsx lines 124-129 - History recording fails silently:

try {
  await api.history.record(recipeId)
} catch (error) {
  console.error('Failed to record history:', error)
  // No user feedback
}

PlayMode.tsx line 25 - AI status fetch failure:

try {
  const status = await api.ai.status()
  setAiAvailable(status.available)
} catch {
  setAiAvailable(false) // Silent fallback
}

Toast spam:

RecipeDetail.tsx - Shows toast for every error, even recoverable ones:
- Line 135: "Failed to load recipe" on fetch error
- Line 172: "Failed to scale recipe" on scaling error
- Line 198: "Failed to generate tips" on AI error

Problem: No error handling strategy. Some failures show toasts, some log to console, some do both, some do neither. Users don't know when something failed vs when the feature isn't available.

Better approach:

Critical errors (can't load recipe): Show toast and error state in UI
Background operations (history recording): Log only, don't interrupt user
Optional features (AI suggestions): Show inline error state, not toast

API Client Error Handling

Location: frontend/src/api/client.ts lines 348-360

Current implementation:

if (!response.ok) {
  let errorMessage: string
  try {
    const error = await response.json()
    errorMessage = error.message || error.detail || 'Request failed'
  } catch {
    errorMessage = await response.text()
  }
  throw new Error(errorMessage)
}

Problems:

Doesn't distinguish HTTP status codes (401, 404, 500 should be handled differently)
No custom error types - everything is Error
Caller can't programmatically handle specific error types

Better approach:

class APIError extends Error {
  constructor(
    message: string,
    public status: number,
    public code?: string
  ) {
    super(message)
  }
}

if (!response.ok) {
  const error = await response.json().catch(() => ({ message: 'Request failed' }))
  throw new APIError(error.message, response.status, error.code)
}

// Callers can then:
catch (error) {
  if (error instanceof APIError && error.status === 401) {
    // Handle auth failure
  }
}

4. Code Duplication

formatTime Function

Appears in 2 files as true duplicates (TimerPanel has a different function for a different purpose):

RecipeCard.tsx lines 20-26:

const formatTime = (minutes: number | null) => {
  if (!minutes) return null
  if (minutes < 60) return `${minutes}m`
  const hours = Math.floor(minutes / 60)
  const mins = minutes % 60
  return mins > 0 ? `${hours}h ${mins}m` : `${hours}h`
}

RecipeDetail.tsx lines 141-147:

const formatTime = (minutes: number | null | undefined) => {
  if (!minutes) return null
  if (minutes < 60) return `${minutes}m`
  const hours = Math.floor(minutes / 60)
  const mins = minutes % 60
  return mins > 0 ? `${hours}h ${mins}m` : `${hours}h`
}

TimerPanel.tsx lines 27-38 (Different function - not a duplicate):

function formatTimerDuration(seconds: number): string {
  const hours = Math.floor(seconds / 3600)
  const minutes = Math.floor((seconds % 3600) / 60)
  const secs = seconds % 60

  if (hours > 0) {
    return `${hours}:${String(minutes).padStart(2, '0')}:${String(secs).padStart(2, '0')}`
  }
  return `${minutes}:${String(secs).padStart(2, '0')}`
}

Issue: RecipeCard and RecipeDetail have identical formatTime functions - true duplicates that should be extracted to lib/utils.ts. TimerPanel's formatTimerDuration is a different function serving a different purpose (seconds-based formatting with clock display format like 1:30:00). Only the first two are duplicates.

Loading State Pattern

The pattern const [loading, setLoading] = useState(false) appears 20+ times:

RemixModal.tsx: loadingSuggestions, creating
Settings.tsx: testingKey, savingKey, editingPromptType, savingPrompt, testingSource, testingBatch, deletingId, deleting, resetting
RecipeDetail.tsx: loading, scalingLoading, tipsLoading, tipsPolling
Home.tsx: loading, discoverLoading

Issue: Each component reinvents async state management. No shared pattern for handling loading/error/success states.

Better approach: Custom hook:

function useAsync<T>() {
  const [state, setState] = useState<{
    loading: boolean
    error: Error | null
    data: T | null
  }>({ loading: false, error: null, data: null })

  const execute = useCallback(async (promise: Promise<T>) => {
    setState({ loading: true, error: null, data: null })
    try {
      const data = await promise
      setState({ loading: false, error: null, data })
      return data
    } catch (error) {
      setState({ loading: false, error: error as Error, data: null })
      throw error
    }
  }, [])

  return { ...state, execute }
}

5. React Hook Issues

useTimers - Race Condition

Location: frontend/src/hooks/useTimers.ts

Critical bug in addTimer (lines 45-64):

const addTimer = useCallback((recipe: Recipe, name: string, durationSeconds: number) => {
  const id = Math.random().toString(36)

  setTimers(prev => [
    ...prev,
    { id, recipe, name, durationSeconds, remaining: durationSeconds, state: 'running' }
  ])

  // Interval starts BEFORE setTimers completes
  const intervalId = setInterval(() => {
    setTimers(prev => {
      const timer = prev.find(t => t.id === id)  // ❌ May not exist yet
      if (!timer) return prev
      // ... update logic
    })
  }, 1000)

  intervalsRef.current.set(id, intervalId)
}, [])

Problem: setInterval starts immediately but setTimers is async. The first interval tick might fire before the timer exists in state, causing timer = undefined.

Correct approach:

setTimers(prev => {
  const newTimer = { id, recipe, name, durationSeconds, remaining: durationSeconds, state: 'running' }

  // Start interval AFTER timer is in state
  const intervalId = setInterval(() => {
    setTimers(timers => {
      const timer = timers.find(t => t.id === id)
      // ... update logic
    })
  }, 1000)

  intervalsRef.current.set(id, intervalId)
  return [...prev, newTimer]
})

useEffect Dependency Issues

Already covered by ESLint warnings, but worth highlighting the pattern:

Code smell: useEffect with guard conditions to prevent infinite loops appears in:

Home.tsx line 65
RecipeDetail.tsx line 123
Search.tsx line 33

Example:

useEffect(() => {
  if (condition1 && condition2 && !loading) {  // Guards
    loadData()
  }
}, [partialDeps])  // Missing deps that would cause re-runs

Why this is a smell: If adding all dependencies would break your code (infinite loop), the logic is in the wrong place. Move data loading to event handlers or use proper dependency management.

6. Type Safety

Good TypeScript Usage

Proper interfaces for all props and data types
API client has generic types: request<T> (line 332)
Minimal use of any types

Issues Found

Unsafe JSON parsing - Settings.tsx line 207:

try {
  const parsed = JSON.parse(error.message)
  if (parsed.message) {
    errorMessage = parsed.message
  }
} catch {
  errorMessage = error.message
}

Assumes error.message is JSON. If it's not, JSON.parse throws and gets caught, but this is brittle.

API client lacks runtime validation:
- request<T> returns typed data but doesn't validate response shape
- If API returns wrong structure, TypeScript types lie at runtime
- Should use runtime validation (Zod, io-ts) for API responses
Inconsistent error typing:
- Some files: catch (error: unknown) (correct)
- Others: catch (error) (implicit any)
- Should standardize on unknown and narrow with type guards

7. Performance Issues

Missing Memoization

Home.tsx line 126 - Set recreated on every render:

const favoriteRecipeIds = new Set(favorites.map((f) => f.recipe.id))

Should be:

const favoriteRecipeIds = useMemo(
  () => new Set(favorites.map((f) => f.recipe.id)),
  [favorites]
)

Settings.tsx - Functions called in loops without memoization:

Line 334: getModelName(prompt.model) called for every prompt in map
Line 345: Date formatting in loops

TimerPanel.tsx - Regex scanning on every render:

Line 89: detectTimes(recipe.instructions) scans entire instruction text
Should use useMemo(() => detectTimes(recipe.instructions), [recipe.instructions])

Unnecessary Re-renders

PlayMode.tsx - Keyboard handler recreated every render (lines 97-110):

useEffect(() => {
  const handleKeyDown = (e: KeyboardEvent) => {
    if (e.key === 'ArrowLeft' || e.key === 'ArrowUp') {
      handlePrevious()
    } else if (e.key === 'ArrowRight' || e.key === 'ArrowDown') {
      handleNext()
    } else if (e.key === 'Escape') {
      onExit()
    }
  }
  window.addEventListener('keydown', handleKeyDown)
  return () => window.removeEventListener('keydown', handleKeyDown)
}, [currentStep, totalSteps, onExit])

handleNext and handlePrevious are defined inline (lines 65, 71) without useCallback, so they change every render. This removes and re-adds the event listener constantly.

Fix: Wrap handlers in useCallback.

8. Code Smells

Magic Numbers

RecipeDetail.tsx:

Line 71: 60000 - "recent recipe" threshold (should be extracted as const RECENT_RECIPE_MS = 60000)
Lines 81-82: Already use named constants POLL_INTERVAL and MAX_POLL_DURATION ✓

useTimers.ts:

Line 64: 1000 - interval delay (should be const TIMER_TICK_MS = 1000)

Dead Code

Home.tsx lines 54-55:

// Refreshed at timestamp could be used for display, keeping for future use
const [, setDiscoverRefreshedAt] = useState<string | null>(null)

This state is set in loadDiscoverSuggestions (line 92) but never read anywhere. The comment "keeping for future use" suggests it's intentionally preserved for future enhancement rather than forgotten dead code.

Complex Conditional Rendering

RecipeDetail.tsx lines 436-458 - Dynamic tabs array with conditional spread:

const tabs = [
  { key: 'ingredients' as const, label: 'Ingredients' },
  { key: 'instructions' as const, label: 'Instructions' },
  { key: 'nutrition' as const, label: 'Nutrition' },
  ...(aiStatus.available ? [{ key: 'tips' as const, label: 'Tips' }] : []),
]

Problem: Hard to read. Mixing static and conditional tabs in one array definition.

Clearer:

const tabs = [
  { key: 'ingredients' as const, label: 'Ingredients' },
  { key: 'instructions' as const, label: 'Instructions' },
  { key: 'nutrition' as const, label: 'Nutrition' },
]
if (aiStatus.available) {
  tabs.push({ key: 'tips' as const, label: 'Tips' })
}

Tight Coupling

RecipeDetail.tsx line 285:

onBack={selectedCollectionId ? handleRecipeDetailBackFromCollection : handleRecipeDetailBack}

Component behaviour depends on parent state (selectedCollectionId from App.tsx). RecipeDetail can't be used independently - it's tightly coupled to App.tsx's navigation state.

Better approach: Pass a single onBack callback. Let the parent decide where to navigate.

9. Missing Patterns

No Error Boundaries

React error boundaries catch rendering errors and prevent the entire app from crashing. None are implemented.

Impact: If any component throws during render, the entire app shows a blank white screen. Users see nothing, not even an error message.

Should have:

<ErrorBoundary fallback={<ErrorScreen />}>
  <App />
</ErrorBoundary>

No Loading Suspense

The app uses manual loading states everywhere (const [loading, setLoading] = useState(false)). React Suspense would simplify this for async data loading.

Current pattern:

{loading ? <Skeleton /> : <Content />}

With Suspense:

<Suspense fallback={<Skeleton />}>
  <Content />
</Suspense>

10. Specific File Issues

Settings.tsx (1,314 lines)

Lines 562-1178: Six tab contents rendered via nested ternaries:

loading ? (
  <LoadingState />
) : activeTab === 'general' ? (
  <GeneralTab />
) : activeTab === 'prompts' ? (
  <PromptsTab />
) : activeTab === 'sources' ? (
  // ... 4 more tabs
)

Problem: 600+ lines of JSX in one expression. Hard to navigate, hard to test.

Should be: Extract each tab to a separate component as described in Frontend Coverage section.

useWakeLock.ts (83 lines of base64)

Lines 18-100: Silent video base64 data (83 lines of encoded video):

const SILENT_VIDEO_BASE64 = 'data:video/mp4;base64,AAAAIGZ0eXBpc29tAAACAG...'

Problem: Makes the file difficult to navigate. 83 lines of base64 data embedded in the code.

Better approach:

Store in separate .ts file: lib/silentVideo.ts
Or load as external asset
Import where needed

11. Positive Patterns Worth Noting

Good Component Organization

File structure is clean:

src/
  screens/      - Page-level components
  components/   - Reusable UI pieces
  hooks/        - Custom React hooks
  contexts/     - React context providers
  api/          - API client and types
  lib/          - Utilities

Proper separation of concerns at the file level.

API Client Design

client.ts has good patterns:

Single base URL configuration
Centralized request handling
Consistent error handling
Type-safe methods organized by resource (recipes, favorites, history, etc.)

useTimers Hook (despite race condition)

Good design overall:

Encapsulates complex timer state
Provides clean API: addTimer, pauseTimer, resumeTimer, removeTimer
Uses refs for intervals (doesn't trigger re-renders)
Just needs the race condition fix

RecipeCard Component

Small, focused, reusable:

112 lines total
Single responsibility: display recipe preview
Takes all data as props (no hidden dependencies)
Used in 4 different screens
This is what all components should look like

12. Recommendations

Critical (Do First)

Fix useTimers race condition - Prevents timers from breaking under certain timing conditions
Move data loading out of useEffect with guards - Replace with onClick handlers or proper dependency management
Add --max-warnings=0 to lint script - Prevent new warnings from being added

High Priority

Implement React Router - Removes navigation complexity from App.tsx
Extract Settings tabs to components - Makes the 1,314-line file maintainable
Consolidate formatTime - Extract to shared utility

Medium Priority

Create useAsync hook - Standardize async operation handling
Add error boundaries - Prevent blank white screen on crashes
Use useReducer in Settings.tsx - Group related state
Add API response validation - Runtime validation with Zod or similar

Low Priority

Move SILENT_VIDEO_BASE64 to separate file - Improves code navigation
Add useMemo for expensive computations - Timer detection, Set creation
Create custom error types - Better error handling in API client

Summary

Overall grade: C+ (acceptable for 4-day prototype, needs work before production)

Strengths:

Clean file organization
Proper TypeScript usage
Good component separation between screens/components
Solid API client design

Weaknesses:

App.tsx god component
No routing library
Race condition in useTimers
Inconsistent error handling
Code duplication in utilities

Key insight: The metrics dashboard caught symptoms (low test coverage, ESLint warnings) but missed the underlying causes (god component, prop drilling, no routing). A human code review was necessary to identify the architectural issues that metrics can't measure.