Explorar o código

Stop AI requests from waiting minutes on a busy provider, and tidy model names (#578)

Every AI client now gets one retry and a two minute limit (five for a
self-hosted server). A provider that asks for a pause longer than five
seconds gets no retry, so the person hears at once that it is busy instead
of watching a spinner. The unused test-connection action is removed, and
model names read the way the provider writes them (o3, o4 Mini, no date).
Bernt Christian Egeland hai 1 día
pai
achega
c5acc94df2

+ 102 - 2
src/__tests__/lib/ai-client.test.ts

@@ -1,9 +1,15 @@
 // @vitest-environment node
-import { describe, expect, it, vi } from 'vitest'
+import { afterEach, describe, expect, it, vi } from 'vitest'
 
 vi.mock('@/lib/db', () => ({ db: {} }))
 
-import { createClient } from '@/lib/ai'
+import {
+  AI_REQUEST_TIMEOUT_MS,
+  createClient,
+  MAX_RETRY_AFTER_MS,
+  SELF_HOSTED_TIMEOUT_MS,
+} from '@/lib/ai'
+import { formatModelLabel } from '@/integrations/ai/models'
 
 /**
  * Every provider gets its address spelled out. The SDK reads OPENAI_BASE_URL
@@ -43,3 +49,97 @@ describe('createClient', () => {
     expect(client.apiKey).toBe('none')
   })
 })
+
+/**
+ * Somebody waits on every AI request with a spinner. The SDK's defaults were
+ * a ten minute timeout, two retries, and a sleep for as long as the provider's
+ * Retry-After asked, so a rate-limited account could hold a button busy for
+ * minutes before anything was said.
+ */
+describe('AI request limits', () => {
+  afterEach(() => {
+    vi.unstubAllGlobals()
+  })
+
+  const ASK = { model: 'gpt-4o', messages: [{ role: 'user' as const, content: 'Hi' }] }
+
+  function reply(status: number, headers: Record<string, string>, body: unknown) {
+    return new Response(JSON.stringify(body), {
+      status,
+      headers: { 'content-type': 'application/json', ...headers },
+    })
+  }
+
+  const RATE_LIMITED = {
+    error: { message: 'Rate limit reached', type: 'requests', code: 'rate_limit_exceeded' },
+  }
+  const ANSWER = {
+    id: 'c1',
+    object: 'chat.completion',
+    created: 0,
+    model: 'gpt-4o',
+    choices: [{ index: 0, message: { role: 'assistant', content: 'OK' }, finish_reason: 'stop' }],
+  }
+
+  it('answers at once when the provider asks for a long wait', async () => {
+    const fetchMock = vi.fn(async () => reply(429, { 'retry-after': '600' }, RATE_LIMITED))
+    vi.stubGlobal('fetch', fetchMock)
+    const client = createClient({ provider: 'openai', apiKey: 'k', model: 'gpt-4o' })
+
+    const started = Date.now()
+    await expect(client.chat.completions.create(ASK)).rejects.toMatchObject({ status: 429 })
+    expect(Date.now() - started).toBeLessThan(MAX_RETRY_AFTER_MS)
+    expect(fetchMock).toHaveBeenCalledTimes(1)
+  })
+
+  it('tries once more after a short wait the provider asked for', async () => {
+    const fetchMock = vi
+      .fn()
+      .mockResolvedValueOnce(reply(503, { 'retry-after-ms': '10' }, { error: { message: 'busy' } }))
+      .mockResolvedValueOnce(reply(200, {}, ANSWER))
+    vi.stubGlobal('fetch', fetchMock)
+    const client = createClient({ provider: 'openai', apiKey: 'k', model: 'gpt-4o' })
+
+    const answer = await client.chat.completions.create(ASK)
+    expect(answer.choices[0]?.message.content).toBe('OK')
+    expect(fetchMock).toHaveBeenCalledTimes(2)
+  })
+
+  it('stops after one retry when the provider keeps failing', async () => {
+    const fetchMock = vi.fn(async () =>
+      reply(500, { 'retry-after-ms': '10' }, { error: { message: 'down' } })
+    )
+    vi.stubGlobal('fetch', fetchMock)
+    const client = createClient({ provider: 'anthropic', apiKey: 'k', model: 'claude-x' })
+
+    await expect(client.chat.completions.create(ASK)).rejects.toMatchObject({ status: 500 })
+    expect(fetchMock).toHaveBeenCalledTimes(2)
+  })
+
+  it('waits minutes at most, longer only for a self-hosted server', () => {
+    const openai = createClient({ provider: 'openai', apiKey: 'k', model: 'gpt-4o' })
+    expect(openai.timeout).toBe(AI_REQUEST_TIMEOUT_MS)
+    expect(openai.maxRetries).toBe(1)
+    const local = createClient({
+      provider: 'openai-compatible',
+      apiKey: '',
+      model: 'qwen3:8b',
+      baseUrl: 'http://ollama:11434/v1',
+    })
+    expect(local.timeout).toBe(SELF_HOSTED_TIMEOUT_MS)
+  })
+})
+
+describe('formatModelLabel', () => {
+  it('reads the way the provider writes it', () => {
+    expect(formatModelLabel('o3')).toBe('o3')
+    expect(formatModelLabel('o4-mini')).toBe('o4 Mini')
+    expect(formatModelLabel('gpt-4.1-mini')).toBe('GPT 4.1 Mini')
+    expect(formatModelLabel('chatgpt-4o-latest')).toBe('ChatGPT 4o Latest')
+  })
+
+  it('leaves off a date in either of the forms OpenAI uses', () => {
+    expect(formatModelLabel('gpt-5.4-mini-2026-03-17')).toBe('GPT 5.4 Mini')
+    expect(formatModelLabel('gpt-4o-20240806')).toBe('GPT 4o')
+  })
+})

+ 1 - 17
src/features/ai/Actions/aiActions.ts

@@ -5,12 +5,7 @@ import { withAuth } from '@/lib/with-auth'
 import { PermissionAction, PermissionSubject } from '@/lib/permissions'
 import { getLocale } from 'next-intl/server'
 import type { Locale } from '@/i18n/config'
-import {
-  generateServiceDescription,
-  summarizeServiceHistory,
-  getCommonIssues,
-  testAiConnection,
-} from '@/lib/ai'
+import { generateServiceDescription, summarizeServiceHistory, getCommonIssues } from '@/lib/ai'
 import { AI_MESSAGE_TYPES } from '@/features/ai/constants'
 
 export async function aiGenerateServiceNotes(serviceRecordId: string) {
@@ -157,14 +152,3 @@ export async function aiClearMessage(vehicleId: string, type: string) {
     }
   )
 }
-
-export async function aiTestConnection() {
-  return withAuth(
-    async ({ organizationId }) => {
-      return testAiConnection(organizationId)
-    },
-    {
-      requiredPermissions: [{ action: PermissionAction.READ, subject: PermissionSubject.SETTINGS }],
-    }
-  )
-}

+ 14 - 9
src/integrations/ai/models.ts

@@ -141,16 +141,21 @@ function anthropicModelOrder(id: string): number {
   return versionScore + tierScore
 }
 
-/** A model id as something to read in a dropdown: gpt-4.1-mini becomes GPT 4.1 Mini. */
+/**
+ * A model id as something to read in a dropdown: gpt-4.1-mini becomes
+ * GPT 4.1 Mini, o4-mini becomes o4 Mini. A date on the end, which OpenAI
+ * writes either as -20251001 or as -2026-03-17, is left off.
+ */
 export function formatModelLabel(modelId: string): string {
-  return modelId
-    .replace(/-(\d{8})$/, '') // strip a date suffix such as -20251001
-    .replace(/^gpt-/i, 'GPT-')
-    .replace(/^claude-/i, 'Claude ')
-    .replace(/^o(\d)/i, 'o$1')
-    .replace(/-/g, ' ')
-    .replace(/\b\w/g, (c) => c.toUpperCase())
-    .replace(/Gpt/g, 'GPT')
+  return (
+    modelId
+      .replace(/-(?:\d{8}|\d{4}-\d{2}-\d{2})$/, '')
+      .replace(/-/g, ' ')
+      .replace(/\b\w/g, (c) => c.toUpperCase())
+      .replace(/gpt/gi, 'GPT')
+      // OpenAI writes its o-series in lower case
+      .replace(/^O(\d)/, 'o$1')
+  )
 }
 
 async function fetchModelIds(ctx: ConnectorContext, base: string): Promise<string[]> {

+ 67 - 17
src/lib/ai.ts

@@ -39,14 +39,75 @@ export async function getAiConfig(organizationId: string): Promise<AiConfig> {
   }
 }
 
+/**
+ * How long one request may go unanswered. Somebody is waiting on every AI
+ * request with a spinner, and the SDK's own default is ten minutes. A
+ * reasoning model writing a long summary answers well inside two; a model
+ * running on the workshop's own hardware may be a lot slower, so a
+ * self-hosted server gets longer. For a streamed answer this is the wait for
+ * the first words, not for the whole reply.
+ */
+export const AI_REQUEST_TIMEOUT_MS = 120_000
+export const SELF_HOSTED_TIMEOUT_MS = 300_000
+
+/** One more try when the provider hiccuped: a 500, an overloaded server. */
+const AI_MAX_RETRIES = 1
+
+/**
+ * The longest pause a provider may ask for before that one retry. The SDK
+ * otherwise sleeps for whatever Retry-After says, minutes if a rate-limited
+ * account is told so, with nothing on screen. A provider that wants a longer
+ * pause gets no retry, and the person hears at once that it is busy.
+ */
+export const MAX_RETRY_AFTER_MS = 5_000
+
+function retryAfterMs(headers: Headers): number | undefined {
+  const ms = Number.parseFloat(headers.get('retry-after-ms') ?? '')
+  if (!Number.isNaN(ms)) return ms
+  const value = headers.get('retry-after')
+  if (!value) return undefined
+  const seconds = Number.parseFloat(value)
+  if (!Number.isNaN(seconds)) return seconds * 1000
+  const at = Date.parse(value)
+  return Number.isNaN(at) ? undefined : at - Date.now()
+}
+
+/**
+ * `fetch` for the SDK, marking a refusal that asks for a long pause as not
+ * worth retrying (the SDK obeys `x-should-retry`). Everything else passes
+ * through untouched.
+ */
+async function fetchWithoutLongWaits(
+  input: string | URL | Request,
+  init?: RequestInit
+): Promise<Response> {
+  const response = await fetch(input, init)
+  if (response.ok) return response
+  const wait = retryAfterMs(response.headers)
+  if (wait === undefined || wait <= MAX_RETRY_AFTER_MS) return response
+  const headers = new Headers(response.headers)
+  headers.set('x-should-retry', 'false')
+  return new Response(response.body, {
+    status: response.status,
+    statusText: response.statusText,
+    headers,
+  })
+}
+
 /**
  * Every provider gets its address spelled out. Left blank, the SDK would
  * read OPENAI_BASE_URL from the environment on its own, and completions
  * would quietly go somewhere the connector's key check never looked.
  */
 export function createClient(config: AiConfig): OpenAI {
+  const limits = {
+    maxRetries: AI_MAX_RETRIES,
+    timeout: AI_REQUEST_TIMEOUT_MS,
+    fetch: fetchWithoutLongWaits,
+  }
   if (config.provider === 'anthropic') {
     return new OpenAI({
+      ...limits,
       apiKey: config.apiKey,
       baseURL: `${ANTHROPIC_BASE}/`,
       defaultHeaders: {
@@ -56,13 +117,15 @@ export function createClient(config: AiConfig): OpenAI {
   }
   if (config.provider === 'openai-compatible') {
     return new OpenAI({
+      ...limits,
+      timeout: SELF_HOSTED_TIMEOUT_MS,
       // The SDK refuses to start without a key; a server that wants none
       // ignores whatever the header says.
       apiKey: config.apiKey || 'none',
       baseURL: normalizeBaseUrl(config.baseUrl ?? ''),
     })
   }
-  return new OpenAI({ apiKey: config.apiKey, baseURL: OPENAI_BASE })
+  return new OpenAI({ ...limits, apiKey: config.apiKey, baseURL: OPENAI_BASE })
 }
 
 /**
@@ -340,9 +403,9 @@ function providerTrouble(error: unknown): boolean {
  * Any reply passes, an empty or cut-off one included: what it says is not
  * the question. No temperature is sent, because several models refuse one.
  *
- * It is asked once and not for long. The SDK would otherwise try a refusal
- * three times and wait as long as the provider told it to, with somebody
- * watching a spinner. And a failed test switches the connection off, so when
+ * It is asked once and not for long, shorter than a real request: somebody
+ * is watching a spinner on the connection page, and a busy provider is not
+ * worth a second try here. And a failed test switches the connection off, so when
  * the provider was merely busy the answer is `inconclusive`: the caller
  * falls back to the check it had before rather than turn off an AI
  * connection that works.
@@ -371,16 +434,3 @@ export async function probeAiModel(
     }
   }
 }
-
-export async function testAiConnection(organizationId: string): Promise<boolean> {
-  const config = await getAiConfig(organizationId)
-  const client = createClient(config)
-
-  const response = await client.chat.completions.create({
-    model: config.model,
-    messages: [{ role: 'user', content: 'Say OK' }],
-    ...completionTuning(config, 5),
-  })
-
-  return !!response.choices[0]?.message?.content
-}