refactor(settings): share the AI parsing form between desktop and phone

The provider, model, key and multimodal state, the hydration from the settings store and the save with its reload on failure now live in one useLlmConnection hook. The phone keeps its picker sheet state.

client/src/components/Settings/LlmConnectionSection.tsx

@@ -1,17 +1,13 @@
-import React, { useEffect, useMemo, useState } from 'react'
+import React from 'react'
 import { Sparkles, Save } from 'lucide-react'
 import { useTranslation } from '../../i18n'
-import { useToast } from '../shared/Toast'
-import { useSettingsStore } from '../../store/settingsStore'
-import type { Settings } from '../../types'
+import { useLlmConnection, type LlmProvider as Provider } from './useLlmConnection'
 import Section from './Section'
 import ToggleSwitch from './ToggleSwitch'
 import { SETTINGS_BUTTON_PRIMARY, SettingRow, SettingRows, SettingsHint } from './settingsKit'
 import { EditorField, GRID_2, INPUT, Segmented } from '../shared/dialogParts'
 import { fs } from '../shared/DialogShell'
 
-type Provider = NonNullable<Settings['llm_provider']>
-
 /**
  * Settings → Integrations → AI parsing. Per-user model used to extract bookings
  * from uploaded files. It only takes effect when the admin has not configured an
@@ -29,71 +25,21 @@ type Provider = NonNullable<Settings['llm_provider']>
  */
 export default function LlmConnectionSection(): React.ReactElement {
   const { t } = useTranslation()
-  const toast = useToast()
-  const settings = useSettingsStore(s => s.settings)
-  const isLoaded = useSettingsStore(s => s.isLoaded)
-  const updateSettings = useSettingsStore(s => s.updateSettings)
-  const loadSettings = useSettingsStore(s => s.loadSettings)
-
-  const [provider, setProvider] = useState<Provider>('openai')
-  const [model, setModel] = useState('')
-  const [apiKey, setApiKey] = useState('')
-  const [multimodal, setMultimodal] = useState(false)
-  const [hasStoredKey, setHasStoredKey] = useState(false)
-  const [saving, setSaving] = useState(false)
-
-  // Hydrate from the loaded settings. llm_api_key arrives masked, so we only use
-  // its presence to drive the placeholder, never the value itself. A stored
-  // 'local' from before #1772 shows as OpenAI (local state only, nothing is
-  // saved until Save is pressed) so the form never offers a value the server
-  // would refuse.
-  useEffect(() => {
-    if (!isLoaded) return
-    const stored = settings.llm_provider || 'openai'
-    setProvider(stored === 'local' ? 'openai' : stored)
-    setModel(settings.llm_model || '')
-    setMultimodal(settings.llm_multimodal === true)
-    setHasStoredKey(!!settings.llm_api_key)
-  }, [isLoaded, settings.llm_provider, settings.llm_model, settings.llm_multimodal, settings.llm_api_key])
-
-  const providerOptions = useMemo(
-    () => [
-      { value: 'openai' as const, label: t('settings.aiParsing.providerOpenai') },
-      { value: 'anthropic' as const, label: t('settings.aiParsing.providerAnthropic') },
-    ],
-    [t],
-  )
-
-  const handleSave = async () => {
-    setSaving(true)
-    try {
-      const payload: Partial<Settings> = {
-        llm_provider: provider,
-        llm_model: model.trim(),
-        // Always cleared: the endpoint is instance configuration now, and this
-        // also drops a value left over from before #1772.
-        llm_base_url: '',
-        llm_multimodal: multimodal,
-      }
-      // Send the key only when the user typed a new one — a blank field means
-      // "keep the stored key".
-      const key = apiKey.trim()
-      if (key) payload.llm_api_key = key
-      await updateSettings(payload)
-      setApiKey('')
-      if (key) setHasStoredKey(true)
-      toast.success(t('settings.aiParsing.toast.saved'))
-    } catch {
-      // updateSettings patches the store before the request and keeps the patch
-      // when the request fails, so a refused save (the 403 from #1772, or any
-      // other error) would leave the form showing a value the server never
-      // stored. Pull the stored settings back in so what is on screen is real.
-      await loadSettings()
-      toast.error(t('settings.aiParsing.toast.saveError'))
-    } finally {
-      setSaving(false)
-    }
-  }
+  const {
+    isLoaded,
+    provider,
+    setProvider,
+    providerOptions,
+    model,
+    setModel,
+    apiKey,
+    setApiKey,
+    multimodal,
+    toggleMultimodal,
+    hasStoredKey,
+    saving,
+    handleSave,
+  } = useLlmConnection()
 
   return (
     <Section title={t('settings.aiParsing.title')} icon={Sparkles}>
@@ -141,7 +87,7 @@ export default function LlmConnectionSection(): React.ReactElement {
         <SettingRow
           label={t('settings.aiParsing.multimodal')}
           hint={t('settings.aiParsing.multimodalHint')}
-          control={<ToggleSwitch on={multimodal} onToggle={() => setMultimodal(v => !v)} label={t('settings.aiParsing.multimodal')} />}
+          control={<ToggleSwitch on={multimodal} onToggle={toggleMultimodal} label={t('settings.aiParsing.multimodal')} />}
         />
       </SettingRows>
 

client/src/components/Settings/useLlmConnection.test.ts

@@ -0,0 +1,96 @@
+// FE-COMP-LLM-HOOK-001 to -006: the AI parsing form both settings shells share.
+import { act, renderHook } from '@testing-library/react';
+import { buildSettings } from '../../../tests/helpers/factories';
+import { resetAllStores, seedStore } from '../../../tests/helpers/store';
+import { useSettingsStore } from '../../store/settingsStore';
+import type { Settings } from '../../types';
+import { useLlmConnection } from './useLlmConnection';
+
+const toast = vi.hoisted(() => ({ success: vi.fn(), error: vi.fn(), warning: vi.fn(), info: vi.fn() }));
+vi.mock('../shared/Toast', () => ({ useToast: () => toast }));
+vi.mock('../../i18n', () => ({ useTranslation: () => ({ t: (k: string) => k }) }));
+
+function seed(over: Partial<Settings> = {}, isLoaded = true) {
+  const updateSettings = vi.fn().mockResolvedValue(undefined);
+  const loadSettings = vi.fn().mockResolvedValue(undefined);
+  seedStore(useSettingsStore, { settings: buildSettings(over), isLoaded, updateSettings, loadSettings });
+  return { updateSettings, loadSettings };
+}
+
+beforeEach(() => {
+  resetAllStores();
+  vi.clearAllMocks();
+});
+
+describe('useLlmConnection', () => {
+  it('FE-COMP-LLM-HOOK-001: waits for the settings, then hydrates them without the key itself', () => {
+    seed({ llm_provider: 'anthropic', llm_model: 'claude', llm_multimodal: true, llm_api_key: '****' }, false);
+    const { result, rerender } = renderHook(() => useLlmConnection());
+    expect(result.current.provider).toBe('openai');
+    expect(result.current.model).toBe('');
+    act(() => useSettingsStore.setState({ isLoaded: true }));
+    rerender();
+    expect(result.current.provider).toBe('anthropic');
+    expect(result.current.model).toBe('claude');
+    expect(result.current.multimodal).toBe(true);
+    expect(result.current.hasStoredKey).toBe(true);
+    expect(result.current.apiKey).toBe('');
+  });
+
+  it('FE-COMP-LLM-HOOK-002: a stored local provider from before #1772 shows as OpenAI', () => {
+    seed({ llm_provider: 'local' });
+    const { result } = renderHook(() => useLlmConnection());
+    expect(result.current.provider).toBe('openai');
+  });
+
+  it('FE-COMP-LLM-HOOK-003: offers exactly the two hosted providers', () => {
+    seed();
+    const { result } = renderHook(() => useLlmConnection());
+    expect(result.current.providerOptions).toEqual([
+      { value: 'openai', label: 'settings.aiParsing.providerOpenai' },
+      { value: 'anthropic', label: 'settings.aiParsing.providerAnthropic' },
+    ]);
+  });
+
+  it('FE-COMP-LLM-HOOK-004: save with a blank key keeps the stored one and always clears the endpoint', async () => {
+    const { updateSettings } = seed({ llm_provider: 'openai', llm_model: 'm' });
+    const { result } = renderHook(() => useLlmConnection());
+    act(() => result.current.setModel('  gpt  '));
+    act(() => result.current.toggleMultimodal());
+    await act(() => result.current.handleSave());
+    expect(updateSettings).toHaveBeenCalledWith({
+      llm_provider: 'openai',
+      llm_model: 'gpt',
+      llm_base_url: '',
+      llm_multimodal: true,
+    });
+    expect(toast.success).toHaveBeenCalledWith('settings.aiParsing.toast.saved');
+    expect(result.current.saving).toBe(false);
+  });
+
+  it('FE-COMP-LLM-HOOK-005: a typed key is sent trimmed, cleared afterwards and marks a stored key', async () => {
+    const { updateSettings } = seed();
+    const { result } = renderHook(() => useLlmConnection());
+    act(() => result.current.setProvider('anthropic'));
+    act(() => result.current.setApiKey(' sk '));
+    await act(() => result.current.handleSave());
+    expect(updateSettings).toHaveBeenCalledWith(
+      expect.objectContaining({ llm_provider: 'anthropic', llm_api_key: 'sk' })
+    );
+    expect(result.current.apiKey).toBe('');
+    expect(result.current.hasStoredKey).toBe(true);
+  });
+
+  it('FE-COMP-LLM-HOOK-006: a refused save reloads the stored settings and toasts the error', async () => {
+    const { updateSettings, loadSettings } = seed();
+    updateSettings.mockRejectedValue(new Error('403'));
+    const { result } = renderHook(() => useLlmConnection());
+    act(() => result.current.setApiKey('sk'));
+    await act(() => result.current.handleSave());
+    expect(loadSettings).toHaveBeenCalled();
+    expect(toast.error).toHaveBeenCalledWith('settings.aiParsing.toast.saveError');
+    expect(toast.success).not.toHaveBeenCalled();
+    expect(result.current.apiKey).toBe('sk');
+    expect(result.current.saving).toBe(false);
+  });
+});

client/src/components/Settings/useLlmConnection.ts

@@ -0,0 +1,98 @@
+import { useEffect, useMemo, useState } from 'react';
+import { useTranslation } from '../../i18n';
+import { useSettingsStore } from '../../store/settingsStore';
+import type { Settings } from '../../types';
+import { useToast } from '../shared/Toast';
+
+export type LlmProvider = NonNullable<Settings['llm_provider']>;
+
+/**
+ * The per-user AI parsing model form behind both settings shells (the desktop
+ * section and its phone twin render their own markup over this). The key is never
+ * prefilled: a blank field keeps the stored key. Only the two hosted providers are
+ * offered (#1772), the endpoint is instance configuration.
+ */
+export function useLlmConnection() {
+  const { t } = useTranslation();
+  const toast = useToast();
+  const settings = useSettingsStore((s) => s.settings);
+  const isLoaded = useSettingsStore((s) => s.isLoaded);
+  const updateSettings = useSettingsStore((s) => s.updateSettings);
+  const loadSettings = useSettingsStore((s) => s.loadSettings);
+
+  const [provider, setProvider] = useState<LlmProvider>('openai');
+  const [model, setModel] = useState('');
+  const [apiKey, setApiKey] = useState('');
+  const [multimodal, setMultimodal] = useState(false);
+  const [hasStoredKey, setHasStoredKey] = useState(false);
+  const [saving, setSaving] = useState(false);
+
+  // Hydrate from the loaded settings. llm_api_key arrives masked, so we only use
+  // its presence to drive the placeholder, never the value itself. A stored
+  // 'local' from before #1772 shows as OpenAI (local state only, nothing is
+  // saved until Save is pressed) so the form never offers a value the server
+  // would refuse.
+  useEffect(() => {
+    if (!isLoaded) return;
+    const stored = settings.llm_provider || 'openai';
+    setProvider(stored === 'local' ? 'openai' : stored);
+    setModel(settings.llm_model || '');
+    setMultimodal(settings.llm_multimodal === true);
+    setHasStoredKey(!!settings.llm_api_key);
+  }, [isLoaded, settings.llm_provider, settings.llm_model, settings.llm_multimodal, settings.llm_api_key]);
+
+  const providerOptions = useMemo(
+    () => [
+      { value: 'openai' as const, label: t('settings.aiParsing.providerOpenai') },
+      { value: 'anthropic' as const, label: t('settings.aiParsing.providerAnthropic') },
+    ],
+    [t]
+  );
+
+  const handleSave = async () => {
+    setSaving(true);
+    try {
+      const payload: Partial<Settings> = {
+        llm_provider: provider,
+        llm_model: model.trim(),
+        // Always cleared: the endpoint is instance configuration now, and this
+        // also drops a value left over from before #1772.
+        llm_base_url: '',
+        llm_multimodal: multimodal,
+      };
+      // Send the key only when the user typed a new one, a blank field means
+      // "keep the stored key".
+      const key = apiKey.trim();
+      if (key) payload.llm_api_key = key;
+      await updateSettings(payload);
+      setApiKey('');
+      if (key) setHasStoredKey(true);
+      toast.success(t('settings.aiParsing.toast.saved'));
+    } catch {
+      // updateSettings patches the store before the request and keeps the patch
+      // when the request fails, so a refused save (the 403 from #1772, or any
+      // other error) would leave the form showing a value the server never
+      // stored. Pull the stored settings back in so what is on screen is real.
+      await loadSettings();
+      toast.error(t('settings.aiParsing.toast.saveError'));
+    } finally {
+      setSaving(false);
+    }
+  };
+
+  return {
+    isLoaded,
+    provider,
+    setProvider,
+    providerOptions,
+    model,
+    setModel,
+    apiKey,
+    setApiKey,
+    multimodal,
+    toggleMultimodal: () => setMultimodal((v) => !v),
+    hasStoredKey,
+    saving,
+    handleSave,
+  };
+}

client/src/mobile/screens/settings/MLlmConnectionSection.tsx

@@ -1,15 +1,11 @@
-import React, { useEffect, useMemo, useState } from 'react'
+import React, { useState } from 'react'
 import { Sparkles, Save, ChevronDown } from 'lucide-react'
 import { useTranslation } from '../../../i18n'
-import { useToast } from '../../../components/shared/Toast'
-import { useSettingsStore } from '../../../store/settingsStore'
-import type { Settings } from '../../../types'
+import { useLlmConnection, type LlmProvider as Provider } from '../../../components/Settings/useLlmConnection'
 import { MSetCard, MSetEyebrow, MSetSelectRow, MSetRow, MSetInput, MSetButton, MSetHint } from './MSettingsUi'
 import MToggle from '../../components/MToggle'
 import MSetPickerSheet from './MSetPickerSheet'
 
-type Provider = NonNullable<Settings['llm_provider']>
-
 /**
  * Mobile-native twin of components/Settings/LlmConnectionSection. Same per-user
  * AI-parsing model logic (provider/model/key/multimodal, key never prefilled,
@@ -19,73 +15,25 @@ type Provider = NonNullable<Settings['llm_provider']>
  */
 export default function MLlmConnectionSection(): React.ReactElement {
   const { t } = useTranslation()
-  const toast = useToast()
-  const settings = useSettingsStore(s => s.settings)
-  const isLoaded = useSettingsStore(s => s.isLoaded)
-  const updateSettings = useSettingsStore(s => s.updateSettings)
-  const loadSettings = useSettingsStore(s => s.loadSettings)
-
-  const [provider, setProvider] = useState<Provider>('openai')
-  const [model, setModel] = useState('')
-  const [apiKey, setApiKey] = useState('')
-  const [multimodal, setMultimodal] = useState(false)
-  const [hasStoredKey, setHasStoredKey] = useState(false)
-  const [saving, setSaving] = useState(false)
+  const {
+    isLoaded,
+    provider,
+    setProvider,
+    providerOptions,
+    model,
+    setModel,
+    apiKey,
+    setApiKey,
+    multimodal,
+    toggleMultimodal,
+    hasStoredKey,
+    saving,
+    handleSave,
+  } = useLlmConnection()
   const [providerOpen, setProviderOpen] = useState(false)
 
-  // Hydrate from the loaded settings. llm_api_key arrives masked, so we only use
-  // its presence to drive the placeholder, never the value itself. A stored
-  // 'local' from before #1772 shows as OpenAI (local state only, nothing is
-  // saved until Save is pressed).
-  useEffect(() => {
-    if (!isLoaded) return
-    const stored = settings.llm_provider || 'openai'
-    setProvider(stored === 'local' ? 'openai' : stored)
-    setModel(settings.llm_model || '')
-    setMultimodal(settings.llm_multimodal === true)
-    setHasStoredKey(!!settings.llm_api_key)
-  }, [isLoaded, settings.llm_provider, settings.llm_model, settings.llm_multimodal, settings.llm_api_key])
-
-  const providerOptions = useMemo(
-    () => [
-      { value: 'openai', label: t('settings.aiParsing.providerOpenai') },
-      { value: 'anthropic', label: t('settings.aiParsing.providerAnthropic') },
-    ],
-    [t],
-  )
   const providerLabel = providerOptions.find(o => o.value === provider)?.label ?? provider
 
-  const handleSave = async () => {
-    setSaving(true)
-    try {
-      const payload: Partial<Settings> = {
-        llm_provider: provider,
-        llm_model: model.trim(),
-        // Always cleared: the endpoint is instance configuration now, and this
-        // also drops a value left over from before #1772.
-        llm_base_url: '',
-        llm_multimodal: multimodal,
-      }
-      // Send the key only when the user typed a new one — a blank field means
-      // "keep the stored key".
-      const key = apiKey.trim()
-      if (key) payload.llm_api_key = key
-      await updateSettings(payload)
-      setApiKey('')
-      if (key) setHasStoredKey(true)
-      toast.success(t('settings.aiParsing.toast.saved'))
-    } catch {
-      // updateSettings patches the store before the request and keeps the patch
-      // when the request fails, so a refused save (the 403 from #1772, or any
-      // other error) would leave the form showing a value the server never
-      // stored. Pull the stored settings back in so what is on screen is real.
-      await loadSettings()
-      toast.error(t('settings.aiParsing.toast.saveError'))
-    } finally {
-      setSaving(false)
-    }
-  }
-
   return (
     <MSetCard title={t('settings.aiParsing.title')} icon={Sparkles} className="mt-3">
       <MSetHint className="mb-3">{t('settings.aiParsing.hint')}</MSetHint>
@@ -127,7 +75,7 @@ export default function MLlmConnectionSection(): React.ReactElement {
           trailing={
             <MToggle
               checked={multimodal}
-              onChange={() => setMultimodal(v => !v)}
+              onChange={toggleMultimodal}
               ariaLabel={t('settings.aiParsing.multimodal')}
             />
           }