From af422ddf2f449ea87a1df03e4f7f002b842500b9 Mon Sep 17 00:00:00 2001 From: rcourtman Date: Sun, 10 May 2026 16:51:17 +0100 Subject: [PATCH] Verify Patrol now tests the form's pending model, not the saved one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two real UX bugs in the Verify Patrol panel: 1. The button silently tested the previously-saved model instead of the operator's pending dropdown selection. Clicking Verify after changing the model would re-run preflight against the OLD model and the operator would believe the new selection was verified. 2. The result panel rendered the cached green badge even when the form's current selection was a different model from the one in the cache, with no visual cue that the verified result was for something other than what they were looking at. Fixes: - runPatrolToolPreflight now passes form.patrolModel as the model override on the POST. Empty form value falls through to the configured shared default on the backend. - PatrolPreflightControl computes isStaleAgainstFormSelection by comparing the bare model name in form.patrolModel to the cached result.model. When stale, the panel switches to amber tone with a headline that names both models and a detail line prompting the operator to click Verify Patrol. - stripModelProvider helper handles "provider:model" prefix. Architecture guardrail extended with three assertions covering the form-aware verify path and the staleness indicator wiring. frontend-primitives contract updated. Live verified: changing the dropdown from deepseek-v4-flash to deepseek-v4-pro before saving switches the panel from green "Tool calling verified" to amber "Verified result is for deepseek-v4-flash, your current selection is deepseek/deepseek-v4-pro · Click Verify Patrol to test the pending selection." --- .../subsystems/frontend-primitives.md | 11 +++++--- .../Settings/AIModelSelectionSection.tsx | 27 +++++++++++++++++++ .../__tests__/settingsArchitecture.test.ts | 21 +++++++++++++++ .../components/Settings/useAISettingsState.ts | 8 +++++- 4 files changed, 63 insertions(+), 4 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index 699b48d82..9bace80ca 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -886,10 +886,15 @@ prompt explain the same operator-facing priority. example the Verify Patrol button in `AIModelSelectionSection.tsx`, which must drive the typed `runPatrolPreflight` client through `useAISettingsState.ts` rather - than inlining fetch calls in the section component, and which must - hydrate its result panel from the `patrol_preflight` snapshot on + than inlining fetch calls in the section component, must hydrate + its result panel from the `patrol_preflight` snapshot on `/api/settings/ai` so the "last verified" state survives page - reloads without forcing a re-click) + reloads without forcing a re-click, must pass the form's pending + `patrolModel` as the model override so the click tests the + operator's unsaved dropdown selection rather than whatever was + previously saved, and must surface a stale-cache warning when the + form's selection differs from the cached result's model so the + green badge cannot silently mislead) 2. Keep top-level settings surfaces routed through the canonical settings shell and maintain both `frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts` plus `tests/integration/tests/15-settings-shell-consistency.spec.ts` diff --git a/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx b/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx index 351c4c8ed..f08d6a2b8 100644 --- a/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx +++ b/frontend-modern/src/components/Settings/AIModelSelectionSection.tsx @@ -11,13 +11,34 @@ interface AIModelSelectionSectionProps { state: AISettingsState; } +const stripModelProvider = (modelId: string) => { + const trimmed = modelId.trim(); + const colon = trimmed.indexOf(':'); + return colon === -1 ? trimmed : trimmed.slice(colon + 1); +}; + const PatrolPreflightControl: Component<{ state: AISettingsState }> = (controlProps) => { const { state } = controlProps; const result = state.patrolPreflightResult; + // The cached result may be for a model the operator already moved away + // from in the form (e.g. they changed the dropdown but haven't clicked + // Verify Patrol yet). When that happens, surface a hint so the green + // "verified" badge doesn't silently mislead. The backend reads the + // form's pending patrolModel on Verify Patrol click, so refreshing + // resolves the staleness. + const pendingFormModel = () => stripModelProvider(state.form.patrolModel || ''); + const cachedResultModel = () => result()?.model?.trim() || ''; + const isStaleAgainstFormSelection = () => { + const pending = pendingFormModel(); + const cached = cachedResultModel(); + return pending !== '' && cached !== '' && pending !== cached; + }; + const tone = () => { const r = result(); if (!r) return 'idle'; + if (isStaleAgainstFormSelection()) return 'warning'; if (r.success) return 'success'; if (r.cause === 'model_tool_support_unverified') return 'warning'; return 'error'; @@ -39,6 +60,9 @@ const PatrolPreflightControl: Component<{ state: AISettingsState }> = (controlPr const headline = () => { const r = result(); if (!r) return ''; + if (isStaleAgainstFormSelection()) { + return `Verified result is for ${cachedResultModel()}, your current selection is ${pendingFormModel()}`; + } if (r.success) { return 'Tool calling verified'; } @@ -51,6 +75,9 @@ const PatrolPreflightControl: Component<{ state: AISettingsState }> = (controlPr const detail = () => { const r = result(); if (!r) return ''; + if (isStaleAgainstFormSelection()) { + return 'Click Verify Patrol to test the pending selection.'; + } return r.summary || r.message || ''; }; diff --git a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts index 3e2f450c6..83d4fb20a 100644 --- a/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts +++ b/frontend-modern/src/components/Settings/__tests__/settingsArchitecture.test.ts @@ -256,6 +256,27 @@ describe('settings architecture guardrails', () => { expect(aiModelSelectionSectionSource).toContain('last verified'); }); + it('passes the form\'s pending patrolModel to runPatrolPreflight so Verify Patrol tests the unsaved selection', () => { + // Without this, clicking Verify Patrol after changing the model + // dropdown silently tested the previously-saved model and the + // operator would believe their pending selection was verified. + expect(aiSettingsStateSource).toContain('form.patrolModel'); + expect(aiSettingsStateSource).toContain('pendingModel'); + expect(aiSettingsStateSource).toContain('runPatrolPreflight(pendingModel ? { model: pendingModel } : {})'); + }); + + it('flags the inline preflight panel as stale when the cached result is for a different model than the form\'s current selection', () => { + // Cache may hold a green result for the previously-saved model + // while the operator has changed the dropdown. Show a warning-tone + // panel with copy that names both models so the green badge + // doesn't silently mislead. + expect(aiModelSelectionSectionSource).toContain('isStaleAgainstFormSelection'); + expect(aiModelSelectionSectionSource).toContain('pendingFormModel'); + expect(aiModelSelectionSectionSource).toContain('cachedResultModel'); + expect(aiModelSelectionSectionSource).toContain('Verified result is for'); + expect(aiModelSelectionSectionSource).toContain('Click Verify Patrol to test the pending selection'); + }); + it('keeps contextual settings feature gates free of retired commercial telemetry wrappers', () => { for (const source of [ agentProfilesPanelSource, diff --git a/frontend-modern/src/components/Settings/useAISettingsState.ts b/frontend-modern/src/components/Settings/useAISettingsState.ts index 9b48b7688..8fe8d0366 100644 --- a/frontend-modern/src/components/Settings/useAISettingsState.ts +++ b/frontend-modern/src/components/Settings/useAISettingsState.ts @@ -595,10 +595,16 @@ export const useAISettingsState = () => { // runPatrolToolPreflight verifies the configured Patrol provider+model // can actually call tools end-to-end. Distinct from runProviderPreflight, // which only confirms each provider's model catalog is reachable. + // + // Passes the form's pending patrolModel as a model override so clicking + // Verify Patrol after changing the dropdown actually tests the operator's + // pending selection, not whatever was previously saved. Empty form value + // means "use the shared default" — the backend handles the fallback. const runPatrolToolPreflight = async () => { setPatrolPreflightRunning(true); try { - const result = await runPatrolPreflight(); + const pendingModel = form.patrolModel.trim(); + const result = await runPatrolPreflight(pendingModel ? { model: pendingModel } : {}); setPatrolPreflightResult(result); } catch (error) { const message =