From 54fc2592211b5376b2c2b1b31a2df2d232dc5be5 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Wed, 17 Dec 2025 18:30:19 +0000 Subject: [PATCH] fix(ai): improve AI settings UX with validation and smart fallbacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backend: - Add smart provider fallback when selected model's provider isn't configured - Automatically switch to a model from a configured provider instead of failing - Log warning when fallback occurs for visibility Frontend (AISettings.tsx): - Add helper functions to check if model's provider is configured - Group model dropdown: configured providers first, unconfigured marked with ⚠️ - Add inline warning when selecting model from unconfigured provider - Validate on save that model's provider is configured (or being added) - Warn before clearing last configured provider (would disable AI) - Warn before clearing provider that current model uses - Add patrol interval validation (must be 0 or >= 10 minutes) - Show red border + inline error for invalid patrol intervals 1-9 - Update patrol interval hint: '(0=off, 10+ to enable)' These changes prevent confusing '500 Internal Server Error' and 'AI is not enabled or configured' errors when model/provider mismatch. --- .github/workflows/test-e2e.yml | 78 +++++++++ .../src/components/Settings/AISettings.tsx | 135 ++++++++++++--- .../Settings/UpdatesSettingsPanel.tsx | 1 + internal/ai/correlation/detector.go | 18 +- internal/ai/service.go | 57 ++++++- tests/integration/QUICK_START.md | 38 +---- tests/integration/README.md | 63 ++++--- tests/integration/docker-compose.test.yml | 25 ++- tests/integration/package.json | 6 +- tests/integration/playwright.config.ts | 9 +- tests/integration/scripts/posttest.mjs | 44 +++++ tests/integration/scripts/pretest.mjs | 70 ++++++++ tests/integration/scripts/run-tests.sh | 87 +++++----- tests/integration/tests/01-core-e2e.spec.ts | 158 ++++++++++++++++++ tests/integration/tests/helpers.ts | 115 +++++++++++++ 15 files changed, 766 insertions(+), 138 deletions(-) create mode 100644 .github/workflows/test-e2e.yml create mode 100644 tests/integration/scripts/posttest.mjs create mode 100644 tests/integration/scripts/pretest.mjs create mode 100644 tests/integration/tests/01-core-e2e.spec.ts diff --git a/.github/workflows/test-e2e.yml b/.github/workflows/test-e2e.yml new file mode 100644 index 000000000..00bad61f8 --- /dev/null +++ b/.github/workflows/test-e2e.yml @@ -0,0 +1,78 @@ +name: Core E2E Tests + +on: + pull_request: + branches: + - main + paths: + - 'frontend-modern/**' + - 'internal/**' + - 'tests/integration/**' + - 'Dockerfile' + - '.github/workflows/test-e2e.yml' + push: + branches: + - main + - master + paths: + - 'frontend-modern/**' + - 'internal/**' + - 'tests/integration/**' + - 'Dockerfile' + - '.github/workflows/test-e2e.yml' + workflow_dispatch: + +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + e2e: + name: Playwright Core E2E + runs-on: ubuntu-latest + timeout-minutes: 45 + + steps: + - name: Checkout code + uses: actions/checkout@v4 + + - name: Set up Node.js + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: tests/integration/package-lock.json + + - name: Install Playwright dependencies + working-directory: tests/integration + run: | + npm ci + npx playwright install --with-deps chromium + + - name: Build Docker images for test environment + run: | + docker build -t pulse-mock-github:test ./tests/integration/mock-github-server + docker build -t pulse:test -f Dockerfile . + + - name: Run E2E suite + working-directory: tests/integration + env: + PULSE_E2E_BOOTSTRAP_TOKEN: 0123456789abcdef0123456789abcdef0123456789abcdef + run: npm test + + - name: Upload Playwright report + if: always() + uses: actions/upload-artifact@v4 + with: + name: playwright-report + path: tests/integration/playwright-report/ + retention-days: 30 + + - name: Upload test videos and screenshots + if: failure() + uses: actions/upload-artifact@v4 + with: + name: test-failures + path: tests/integration/test-results/ + retention-days: 7 + diff --git a/frontend-modern/src/components/Settings/AISettings.tsx b/frontend-modern/src/components/Settings/AISettings.tsx index 80e9585c4..8cade299f 100644 --- a/frontend-modern/src/components/Settings/AISettings.tsx +++ b/frontend-modern/src/components/Settings/AISettings.tsx @@ -38,6 +38,24 @@ function getProviderFromModelId(modelId: string): string { return 'ollama'; } +// Check if a provider is configured based on settings +function isProviderConfigured(provider: string, settings: AISettingsType | null): boolean { + if (!settings) return false; + switch (provider) { + case 'anthropic': return settings.anthropic_configured; + case 'openai': return settings.openai_configured; + case 'deepseek': return settings.deepseek_configured; + case 'ollama': return settings.ollama_configured; + default: return false; + } +} + +// Check if a model's provider is configured +function isModelProviderConfigured(modelId: string, settings: AISettingsType | null): boolean { + const provider = getProviderFromModelId(modelId); + return isProviderConfigured(provider, settings); +} + // Group models by provider for optgroup rendering function groupModelsByProvider(models: { id: string; name: string; description?: string }[]): Map { const grouped = new Map(); @@ -243,11 +261,39 @@ export const AISettings: Component = () => { const handleSave = async (event?: Event) => { event?.preventDefault(); + // Frontend validation: warn if model's provider isn't configured + const selectedModel = form.model.trim(); + if (selectedModel && form.enabled) { + const modelProvider = getProviderFromModelId(selectedModel); + if (!isProviderConfigured(modelProvider, settings())) { + // Check if any API key is being added in this save for this provider + const isAddingCredential = + (modelProvider === 'anthropic' && form.anthropicApiKey.trim()) || + (modelProvider === 'openai' && form.openaiApiKey.trim()) || + (modelProvider === 'deepseek' && form.deepseekApiKey.trim()) || + (modelProvider === 'ollama' && form.ollamaBaseUrl.trim()); + + if (!isAddingCredential) { + notificationStore.error( + `Cannot save: Model "${selectedModel}" requires ${PROVIDER_DISPLAY_NAMES[modelProvider] || modelProvider} to be configured. ` + + `Please add an API key for ${PROVIDER_DISPLAY_NAMES[modelProvider] || modelProvider} or select a different model.` + ); + return; + } + } + } + + // Validate patrol interval (must be 0 or >= 10) + if (form.patrolIntervalMinutes > 0 && form.patrolIntervalMinutes < 10) { + notificationStore.error('Patrol interval must be at least 10 minutes (or 0 to disable)'); + return; + } + setSaving(true); try { const payload: Record = { provider: form.provider, - model: form.model.trim(), + model: selectedModel, }; // Only include base_url if it's set or if provider is ollama @@ -384,7 +430,25 @@ export const AISettings: Component = () => { }; const handleClearProvider = async (provider: string) => { - if (!confirm(`Clear ${provider} credentials? You'll need to re-enter them to use this provider.`)) { + // Check if this is the last configured provider + const s = settings(); + const configuredCount = [s?.anthropic_configured, s?.openai_configured, s?.deepseek_configured, s?.ollama_configured].filter(Boolean).length; + const isLastProvider = configuredCount === 1 && isProviderConfigured(provider, s); + + // Check if current model uses this provider + const currentModel = form.model.trim(); + const modelUsesProvider = currentModel && getProviderFromModelId(currentModel) === provider; + + let confirmMessage = `Clear ${PROVIDER_DISPLAY_NAMES[provider] || provider} credentials?`; + if (isLastProvider) { + confirmMessage = `⚠️ This is your only configured provider! Clearing it will disable AI until you configure another provider. Continue?`; + } else if (modelUsesProvider) { + confirmMessage = `Your current model uses ${PROVIDER_DISPLAY_NAMES[provider] || provider}. Clearing this will require selecting a different model. Continue?`; + } else { + confirmMessage += ` You'll need to re-enter credentials to use this provider.`; + } + + if (!confirm(confirmMessage)) { return; } @@ -545,7 +609,8 @@ export const AISettings: Component = () => { m.id === form.model)}> - + {/* Show configured providers first */} + isProviderConfigured(p, settings()))}> {([provider, models]) => ( @@ -558,8 +623,32 @@ export const AISettings: Component = () => { )} + {/* Show unconfigured providers in a separate section with warning */} + !isProviderConfigured(p, settings()))}> + {([provider, models]) => ( + + + {(model) => ( + + )} + + + )} + + {/* Warning if selected model's provider is not configured */} + +

+ + + + This model requires {PROVIDER_DISPLAY_NAMES[getProviderFromModelId(form.model)] || getProviderFromModelId(form.model)} to be configured. + Add an API key below or select a different model. +

+
{/* Advanced Model Selection - Collapsible */} @@ -1017,22 +1106,30 @@ export const AISettings: Component = () => {
{/* Patrol Interval - Compact */} -
- - { - const value = parseInt(e.currentTarget.value, 10); - if (!isNaN(value)) setForm('patrolIntervalMinutes', Math.max(0, value)); - }} - min={0} - max={10080} - step={15} - disabled={saving()} - /> - min (0 = disabled) +
+
+ + 0 && form.patrolIntervalMinutes < 10 + ? 'border-red-300 dark:border-red-600' + : 'border-gray-300 dark:border-gray-600' + }`} + value={form.patrolIntervalMinutes} + onInput={(e) => { + const value = parseInt(e.currentTarget.value, 10); + if (!isNaN(value)) setForm('patrolIntervalMinutes', Math.max(0, value)); + }} + min={0} + max={10080} + step={15} + disabled={saving()} + /> + min (0=off, 10+ to enable) +
+ 0 && form.patrolIntervalMinutes < 10}> +

Minimum interval is 10 minutes

+
{/* Alert Analysis Toggle - Compact */} diff --git a/frontend-modern/src/components/Settings/UpdatesSettingsPanel.tsx b/frontend-modern/src/components/Settings/UpdatesSettingsPanel.tsx index daf72adf7..4ca4074d1 100644 --- a/frontend-modern/src/components/Settings/UpdatesSettingsPanel.tsx +++ b/frontend-modern/src/components/Settings/UpdatesSettingsPanel.tsx @@ -591,6 +591,7 @@ sudo tar -xzf pulse-${props.updateInfo()?.latestVersion}-linux-amd64.tar.gz -C /