From c47a89b82531d68d612db10103c61be036a6c986 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 20:25:57 +0000 Subject: [PATCH 1/7] fix(ai): ship the current OpenAI, Gemini, and Claude model seeds Fresh installs only saw GPT-6 after a manual sync because the shipped OpenAI seeds stopped at gpt-5.5. Add gpt-6-sol/-luna/-astra and gpt-5.6-sol/-luna/ -terra with models.dev limits (1,050,000 context, 128,000 output) and no temperature support, each verified live: listed by /v1/models, a completion succeeds, and temperature: 0.5 is rejected with 400 unsupported_value. The same live check showed the gpt-5.4 family accepts temperature (200), so its seeds no longer mark sampling as unsupported; models.dev agrees. Gemini was stale too: gemini-3-pro-preview was shut down on 2026-03-09, and the GA gemini-3.8/3.7/3.6-flash and 3.5-flash-lite models were missing. The Anthropic preset gains claude-opus-5-5 and claude-fable-5-1. Those additions come from models.dev cross-checked with the vendors' docs (no key for a live completion). An opt-in live test (LIVE_DISCOVERY_TESTS=1) now flags seed drift against models.dev and, with OPENAI_API_KEY, against /v1/models. Amp-Thread-ID: https://ampcode.com/threads/T-01a0df48-9e0d-75cc-af45-26ec84217b88 Co-authored-by: Christian Bager Bach Houmann --- src/ai/Provider.test.ts | 48 ++++++++++++++++++++++++++++ src/ai/Provider.ts | 32 +++++++++++++++---- src/ai/modelSeeds.live.test.ts | 58 ++++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 src/ai/modelSeeds.live.test.ts diff --git a/src/ai/Provider.test.ts b/src/ai/Provider.test.ts index 6786530ff..e2cf68c1d 100644 --- a/src/ai/Provider.test.ts +++ b/src/ai/Provider.test.ts @@ -1,6 +1,8 @@ import { describe, it, expect } from "vitest"; import type { AIProvider } from "./Provider"; import { + CURRENT_MODEL_SEEDS, + DefaultProviders, activeModelRef, ensureProviderIds, getProviderKind, @@ -119,3 +121,49 @@ describe("activeModelRef", () => { expect(activeModelRef("gpt-4o", undefined)).toBeUndefined(); }); }); + +describe("shipped model seeds", () => { + // Values verified live on 2026-09-26: listed by /v1/models, a completion + // succeeds, and temperature: 0.5 is rejected (400 unsupported_value). + it("offer the GPT-6 generation on a fresh install, before any sync", () => { + const openai = DefaultProviders.find((p) => p.id === "openai"); + const byName = new Map(openai?.models.map((m) => [m.name, m])); + for (const name of [ + "gpt-6-sol", + "gpt-6-luna", + "gpt-6-astra", + "gpt-5.6-sol", + "gpt-5.6-luna", + "gpt-5.6-terra", + ]) { + expect(byName.get(name), name).toEqual({ + name, + maxTokens: 1_050_000, + maxOutputTokens: 128_000, + supportsTemperature: false, + }); + } + }); + + it("let the gpt-5.4 family keep a user's temperature (accepted live)", () => { + for (const name of ["gpt-5.4", "gpt-5.4-mini", "gpt-5.4-nano"]) { + const seed = CURRENT_MODEL_SEEDS.openai.find((m) => m.name === name); + expect(seed?.supportsTemperature, name).toBe(true); + } + const gpt55 = CURRENT_MODEL_SEEDS.openai.find((m) => m.name === "gpt-5.5"); + expect(gpt55?.supportsTemperature).toBe(false); + }); + + it("no longer seed gemini-3-pro-preview, which Google shut down", () => { + const names = CURRENT_MODEL_SEEDS.google.map((m) => m.name); + expect(names).not.toContain("gemini-3-pro-preview"); + expect(names).toContain("gemini-3.8-flash"); + }); + + it("list each model once per provider", () => { + for (const [key, seeds] of Object.entries(CURRENT_MODEL_SEEDS)) { + const names = seeds.map((m) => m.name); + expect(new Set(names).size, key).toBe(names.length); + } + }); +}); diff --git a/src/ai/Provider.ts b/src/ai/Provider.ts index 4c13955ed..d12319319 100644 --- a/src/ai/Provider.ts +++ b/src/ai/Provider.ts @@ -185,18 +185,31 @@ export interface Model { * offline fallback: live discovery (models.dev / the provider's models endpoint) * is the source of truth, and auto-sync keeps lists current without plugin * releases. Each entry below was verified live (directory metadata + a real - * completion) on 2026-07-07. When touching this table, re-verify against - * https://models.dev/api.json and the provider APIs — never add ids from memory. + * completion) on 2026-07-07. Refreshed 2026-09-26: the OpenAI list was + * re-verified live (listed by /v1/models, a real completion, and a completion + * with temperature to confirm supportsTemperature); the Google and Anthropic + * additions come from models.dev metadata cross-checked against the vendors' + * model/deprecation docs, without a live completion (no key was available). + * When touching this table, re-verify against https://models.dev/api.json and + * the provider APIs — never add ids from memory. */ export const CURRENT_MODEL_SEEDS: Record< "openai" | "google" | "anthropic", Model[] > = { openai: [ + { name: "gpt-6-sol", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, + { name: "gpt-6-luna", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, + { name: "gpt-6-astra", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, + { name: "gpt-5.6-sol", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, + { name: "gpt-5.6-luna", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, + { name: "gpt-5.6-terra", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, { name: "gpt-5.5", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, - { name: "gpt-5.4", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, - { name: "gpt-5.4-mini", maxTokens: 400_000, maxOutputTokens: 128_000, supportsTemperature: false }, - { name: "gpt-5.4-nano", maxTokens: 400_000, maxOutputTokens: 128_000, supportsTemperature: false }, + // The gpt-5.4 family accepts temperature (live 200 with temperature: 0.5, + // matching models.dev); only gpt-5.5 and newer reject it. + { name: "gpt-5.4", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: true }, + { name: "gpt-5.4-mini", maxTokens: 400_000, maxOutputTokens: 128_000, supportsTemperature: true }, + { name: "gpt-5.4-nano", maxTokens: 400_000, maxOutputTokens: 128_000, supportsTemperature: true }, { name: "gpt-4.1", maxTokens: 1_047_576, maxOutputTokens: 32_768, supportsTemperature: true }, { name: "gpt-4.1-mini", maxTokens: 1_047_576, maxOutputTokens: 32_768, supportsTemperature: true }, { name: "gpt-4o", maxTokens: 128_000, maxOutputTokens: 16_384, supportsTemperature: true }, @@ -205,16 +218,23 @@ export const CURRENT_MODEL_SEEDS: Record< { name: "o4-mini", maxTokens: 200_000, maxOutputTokens: 100_000, supportsTemperature: false }, ], google: [ + { name: "gemini-3.8-flash", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, + { name: "gemini-3.7-flash", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, + { name: "gemini-3.6-flash", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, { name: "gemini-3.5-flash", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, + { name: "gemini-3.5-flash-lite", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, { name: "gemini-3.1-pro-preview", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, { name: "gemini-3.1-flash-lite", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, - { name: "gemini-3-pro-preview", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, + // gemini-3-pro-preview was shut down 2026-03-09 (the id now aliases + // gemini-3.1-pro-preview), so it is no longer seeded. { name: "gemini-3-flash-preview", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, { name: "gemini-2.5-pro", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, { name: "gemini-2.5-flash", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, { name: "gemini-2.5-flash-lite", maxTokens: 1_048_576, maxOutputTokens: 65_536, supportsTemperature: true }, ], anthropic: [ + { name: "claude-opus-5-5", maxTokens: 1_000_000, maxOutputTokens: 128_000, supportsTemperature: false }, + { name: "claude-fable-5-1", maxTokens: 1_000_000, maxOutputTokens: 128_000, supportsTemperature: false }, { name: "claude-fable-5", maxTokens: 1_000_000, maxOutputTokens: 128_000, supportsTemperature: false }, { name: "claude-sonnet-5", maxTokens: 1_000_000, maxOutputTokens: 128_000, supportsTemperature: false }, { name: "claude-opus-4-8", maxTokens: 1_000_000, maxOutputTokens: 128_000, supportsTemperature: false }, diff --git a/src/ai/modelSeeds.live.test.ts b/src/ai/modelSeeds.live.test.ts new file mode 100644 index 000000000..43b3c1f71 --- /dev/null +++ b/src/ai/modelSeeds.live.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from "vitest"; +import { CURRENT_MODEL_SEEDS } from "./Provider"; + +// Drift check for the shipped seed catalog. Opt-in because it hits the network: +// LIVE_DISCOVERY_TESTS=1 pnpm vitest run src/ai/modelSeeds.live.test.ts +// With OPENAI_API_KEY set it also confirms every OpenAI seed is served by +// /v1/models. Only model ids are compared or reported, never account data. +const runLive = process.env.LIVE_DISCOVERY_TESTS === "1"; +const openAIKey = process.env.OPENAI_API_KEY; + +type DirectoryModel = { + limit?: { context?: number; output?: number }; + temperature?: boolean; +}; + +(runLive ? describe : describe.skip)("shipped model seeds (live)", () => { + it("match models.dev context, output, and sampling metadata", async () => { + const response = await fetch("https://models.dev/api.json"); + const directory = (await response.json()) as Record< + string, + { models: Record } + >; + + const drift: string[] = []; + for (const [key, seeds] of Object.entries(CURRENT_MODEL_SEEDS)) { + for (const seed of seeds) { + const entry = directory[key]?.models[seed.name]; + const actual = entry && { + maxTokens: entry.limit?.context, + maxOutputTokens: entry.limit?.output, + supportsTemperature: entry.temperature, + }; + const expected = { + maxTokens: seed.maxTokens, + maxOutputTokens: seed.maxOutputTokens, + supportsTemperature: seed.supportsTemperature, + }; + if (JSON.stringify(actual) !== JSON.stringify(expected)) { + drift.push(`${key}/${seed.name}: directory ${JSON.stringify(actual)}`); + } + } + } + expect(drift).toEqual([]); + }); + + (openAIKey ? it : it.skip)("lists every OpenAI seed in /v1/models", async () => { + const response = await fetch("https://api.openai.com/v1/models", { + headers: { Authorization: `Bearer ${openAIKey}` }, + }); + expect(response.status).toBe(200); + const { data } = (await response.json()) as { data: Array<{ id: string }> }; + const served = new Set(data.map((model) => model.id)); + const missing = CURRENT_MODEL_SEEDS.openai + .map((seed) => seed.name) + .filter((name) => !served.has(name)); + expect(missing).toEqual([]); + }); +}); From 71b3f45a0b4e7a461c5166b46e42efebc3784b68 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 20:25:58 +0000 Subject: [PATCH 2/7] fix(ai): make the provider "Sync now" notice match the list on screen Opening Edit providers starts a quiet sync that merged new models into the provider being edited without re-rendering it. "Sync now" then found nothing new, reported "already up to date", and its reload revealed the models the background sync had added. Linking an API key played no part: models.dev sync needs no key. The edit view now re-renders its model list when the background sync changes it. "Sync now" waits for any in-flight background sync and counts additions against the list the user was looking at when they clicked. Models the background sync adds while a provider is open are also merged into the Cancel snapshot, so Cancel discards only the user's edits. Amp-Thread-ID: https://ampcode.com/threads/T-01a0df48-9e0d-75cc-af45-26ec84217b88 Co-authored-by: Christian Bager Bach Houmann --- docs/src/content/docs/docs/AIAssistant.md | 3 + src/ai/modelSyncService.test.ts | 25 ++- src/ai/modelSyncService.ts | 24 ++- .../AIAssistantProvidersModal.sync.test.ts | 170 ++++++++++++++++++ src/gui/AIAssistantProvidersModal.ts | 63 +++++-- 5 files changed, 264 insertions(+), 21 deletions(-) create mode 100644 src/gui/AIAssistantProvidersModal.sync.test.ts diff --git a/docs/src/content/docs/docs/AIAssistant.md b/docs/src/content/docs/docs/AIAssistant.md index 8685ff80a..4e1fb9219 100644 --- a/docs/src/content/docs/docs/AIAssistant.md +++ b/docs/src/content/docs/docs/AIAssistant.md @@ -185,6 +185,9 @@ imports new models and refreshed context limits from the provider's model source once a day and whenever provider settings open, so model lists stay current without plugin updates. Auto-sync only adds models and updates metadata - it never removes models you have configured. Use **Sync now** to refresh on demand. +Models that arrive while you are editing a provider appear in its list right +away, and the **Sync now** notice counts every model added to the list you were +looking at when you clicked it. Auto-sync is on by default for the built-in OpenAI and Gemini providers and for providers added from a card. It does nothing while **Disable AI & online diff --git a/src/ai/modelSyncService.test.ts b/src/ai/modelSyncService.test.ts index cb7822715..3d5631a9f 100644 --- a/src/ai/modelSyncService.test.ts +++ b/src/ai/modelSyncService.test.ts @@ -43,7 +43,7 @@ vi.mock("src/logger/logManager", () => ({ log: { logMessage: mocks.logMessageMock, logError: vi.fn() }, })); -const { autoSyncEnabledProviders, syncProviderModels } = await import( +const { autoSyncEnabledProviders, diffModelLists, syncProviderModels } = await import( "./modelSyncService" ); @@ -73,7 +73,8 @@ describe("syncProviderModels", () => { const result = await syncProviderModels(undefined, provider); - expect(result).toEqual({ added: 1, updated: 1 }); + expect(result).toMatchObject({ added: 1, updated: 1 }); + expect(result.discovered.map((m) => m.name)).toEqual(["gpt-4o", "gpt-5.5"]); expect(provider.models.map((m) => m.name)).toEqual([ "gpt-4o", "gpt-5.5", @@ -82,6 +83,26 @@ describe("syncProviderModels", () => { }); }); +describe("diffModelLists", () => { + it("counts by name, not by length, so a list that lost entries still reports additions", () => { + // The before list has a model the after list lacks (deleted mid-sync) and + // vice versa: a length delta would report 0 new models. + const before = [ + { name: "gpt-4o", maxTokens: 128000 }, + { name: "removed-by-user", maxTokens: 1000 }, + { name: "o3", maxTokens: 200000, supportsTemperature: false }, + ]; + const after = [ + { name: "gpt-4o", maxTokens: 128000, maxOutputTokens: 16384 }, + { name: "o3", maxTokens: 200000, supportsTemperature: false }, + { name: "gpt-6-sol", maxTokens: 1050000 }, + ]; + + expect(diffModelLists(before, after)).toEqual({ added: 1, updated: 1 }); + expect(diffModelLists(after, after)).toEqual({ added: 0, updated: 0 }); + }); +}); + describe("autoSyncEnabledProviders", () => { beforeEach(() => { mocks.discoverProviderModelsMock.mockReset(); diff --git a/src/ai/modelSyncService.ts b/src/ai/modelSyncService.ts index cb8641c2f..97fa929ef 100644 --- a/src/ai/modelSyncService.ts +++ b/src/ai/modelSyncService.ts @@ -16,33 +16,41 @@ export interface ProviderSyncOutcome { error?: string; } -function countUpdated(before: Model[], after: Model[]): number { +/** + * How `after` differs from `before`, matched by model name: entries that are + * new, and entries whose metadata (context/output/sampling) changed. + */ +export function diffModelLists( + before: Model[], + after: Model[], +): { added: number; updated: number } { const beforeByName = new Map(before.map((m) => [m.name, JSON.stringify(m)])); + let added = 0; let updated = 0; for (const model of after) { const prev = beforeByName.get(model.name); - if (prev !== undefined && prev !== JSON.stringify(model)) updated += 1; + if (prev === undefined) added += 1; + else if (prev !== JSON.stringify(model)) updated += 1; } - return updated; + return { added, updated }; } /** * Discover the provider's current models and merge them into its list: * new models are appended, existing ones get their context/output/sampling * metadata refreshed. Mutates `provider.models`; never removes entries. + * Returns what this call changed plus the discovered list, so callers holding + * another copy of the provider (e.g. an edit snapshot) can merge it too. */ export async function syncProviderModels( app: App | undefined, provider: AIProvider, -): Promise<{ added: number; updated: number }> { +): Promise<{ added: number; updated: number; discovered: Model[] }> { const apiKey = await resolveProviderApiKey(app, provider); const discovered = await discoverProviderModels(provider, apiKey); const before = provider.models; provider.models = mergeModels(provider.models, discovered); - return { - added: provider.models.length - before.length, - updated: countUpdated(before, provider.models), - }; + return { ...diffModelLists(before, provider.models), discovered }; } /** Stable identity for matching a synced provider back into current state. */ diff --git a/src/gui/AIAssistantProvidersModal.sync.test.ts b/src/gui/AIAssistantProvidersModal.sync.test.ts new file mode 100644 index 000000000..2b1fa751f --- /dev/null +++ b/src/gui/AIAssistantProvidersModal.sync.test.ts @@ -0,0 +1,170 @@ +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { App, ButtonComponent, Notice } from "obsidian"; +import type { AIProvider, Model } from "src/ai/Provider"; +import { settingsStore } from "src/settingsStore"; + +// Each discovery call gets its own deferred so a test can decide which request +// (the quiet on-open sync or "Sync now") lands first. +const discovery = vi.hoisted(() => { + const calls: Array<{ resolve: (models: Model[]) => void }> = []; + return { + calls, + discoverProviderModels: () => + new Promise((resolve) => calls.push({ resolve })), + }; +}); + +vi.mock("src/ai/modelDiscoveryService", () => ({ + discoverProviderModels: discovery.discoverProviderModels, +})); +vi.mock("./GenericInputPrompt/GenericInputPrompt", () => ({ + default: { Prompt: vi.fn() }, +})); +vi.mock("./GenericYesNoPrompt/GenericYesNoPrompt", () => ({ + default: { Prompt: vi.fn().mockResolvedValue(true) }, +})); + +import { AIAssistantProvidersModal } from "./AIAssistantProvidersModal"; + +const SHIPPED: Model[] = [ + { name: "gpt-5.5", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, +]; +const DIRECTORY: Model[] = [ + ...SHIPPED, + { name: "gpt-6-sol", maxTokens: 1_050_000, maxOutputTokens: 128_000, supportsTemperature: false }, +]; + +function openAIProvider(): AIProvider { + return { + id: "openai", + name: "OpenAI", + endpoint: "https://api.openai.com/v1", + apiKey: "", + models: SHIPPED.map((model) => ({ ...model })), + autoSyncModels: true, + modelSource: "modelsDev", + }; +} + +function clickButtonByText(modal: AIAssistantProvidersModal, text: string) { + const button = Array.from( + modal.contentEl.querySelectorAll("button"), + ).find((candidate) => candidate.textContent === text); + if (!button) throw new Error(`Button "${text}" not found`); + button.click(); +} + +// The stub Setting renders settingEl > infoEl > nameEl without Obsidian's +// classes, so read each model row's name structurally. +function shownModelNames(modal: AIAssistantProvidersModal): string[] { + return Array.from( + modal.contentEl.querySelectorAll( + ".models-container > div > div:first-child > div:first-child", + ), + ) + .map((el) => el.textContent ?? "") + .filter((name) => name !== "Add model"); +} + +const flush = () => new Promise((resolve) => setTimeout(resolve, 0)); + +/** Resolve the nth discovery request once it has actually been made. */ +async function landDiscovery(index: number, models: Model[]): Promise { + await flush(); + const call = discovery.calls[index]; + if (!call) throw new Error(`Discovery request #${index} was never made`); + call.resolve(models); + await flush(); +} +const notices = () => + (Notice as unknown as { instances: Array<{ message: string }> }).instances.map( + (notice) => notice.message, + ); + +// Regression: the quiet on-open sync used to merge new models into the +// provider being edited without re-rendering, so "Sync now" then revealed +// them while reporting "already up to date" (reproduced in Obsidian 1.13.7). +describe("AIAssistantProvidersModal model sync while editing", () => { + beforeAll(() => { + const modalProto = Object.getPrototypeOf( + AIAssistantProvidersModal.prototype, + ) as { onClose?: () => void }; + modalProto.onClose ??= function onClose() {}; + const btnProto = ButtonComponent.prototype as unknown as { + setDestructive?: () => unknown; + setIcon?: () => unknown; + }; + btnProto.setDestructive ??= function setDestructive(this: unknown) { + return this; + }; + btnProto.setIcon ??= function setIcon(this: unknown) { + return this; + }; + }); + + beforeEach(() => { + discovery.calls.length = 0; + (Notice as unknown as { instances: unknown[] }).instances.length = 0; + settingsStore.setState({ disableOnlineFeatures: false }); + }); + + afterEach(() => { + document.body.innerHTML = ""; + }); + + function openAndEdit(providers: AIProvider[]): AIAssistantProvidersModal { + const modal = new AIAssistantProvidersModal(providers, new App() as App); + clickButtonByText(modal, "Edit"); + return modal; + } + + it("shows models the on-open sync adds after the provider was opened", async () => { + const modal = openAndEdit([openAIProvider()]); + expect(shownModelNames(modal)).toEqual(["gpt-5.5"]); + + await landDiscovery(0, DIRECTORY); + + expect(shownModelNames(modal)).toEqual(["gpt-5.5", "gpt-6-sol"]); + }); + + it("says 'already up to date' only when the list on screen really is", async () => { + const modal = openAndEdit([openAIProvider()]); + await landDiscovery(0, DIRECTORY); + + clickButtonByText(modal, "Sync now"); + await landDiscovery(1, DIRECTORY); + + expect(notices()).toEqual([ + "Synced from the models.dev directory: already up to date.", + ]); + }); + + it("counts models the on-open sync lands after Sync now was clicked", async () => { + const modal = openAndEdit([openAIProvider()]); + + // Click while the on-open sync is still in flight; it then lands first. + clickButtonByText(modal, "Sync now"); + await landDiscovery(0, DIRECTORY); + await landDiscovery(1, DIRECTORY); + + expect(notices()).toEqual([ + "Synced from the models.dev directory: 1 new model(s), 0 updated.", + ]); + expect(shownModelNames(modal)).toEqual(["gpt-5.5", "gpt-6-sol"]); + }); + + it("keeps background-synced models when the user cancels their edits", async () => { + const providers = [openAIProvider()]; + const modal = openAndEdit(providers); + providers[0].name = "Renamed by user"; + + await landDiscovery(0, DIRECTORY); + clickButtonByText(modal, "Cancel"); + + expect(providers[0].name).toBe("OpenAI"); + expect(providers[0].models.map((model) => model.name)).toEqual([ + "gpt-5.5", + "gpt-6-sol", + ]); + }); +}); diff --git a/src/gui/AIAssistantProvidersModal.ts b/src/gui/AIAssistantProvidersModal.ts index 9915dcad8..cfcf82823 100644 --- a/src/gui/AIAssistantProvidersModal.ts +++ b/src/gui/AIAssistantProvidersModal.ts @@ -4,7 +4,7 @@ import { ButtonComponent, Modal, Notice, Setting } from "obsidian"; import type { AIProvider } from "src/ai/Provider"; import { ensureProviderIds } from "src/ai/Provider"; import { mergeModels } from "src/ai/modelsDirectory"; -import { syncProviderModels } from "src/ai/modelSyncService"; +import { diffModelLists, syncProviderModels } from "src/ai/modelSyncService"; import { settingsStore } from "src/settingsStore"; import { ModelDirectoryModal } from "./ModelDirectoryModal"; import { deepClone } from "src/utils/deepClone"; @@ -23,6 +23,12 @@ export class AIAssistantProvidersModal extends Modal { private _selectedProviderClone: AIProvider | null; + /** The edit view's model list, re-rendered in place when a sync changes it. */ + private modelsContainerEl: HTMLElement | null = null; + + /** The quiet on-open sync pass; "Sync now" waits for it before counting. */ + private readonly backgroundSync: Promise; + constructor(providers: AIProvider[], app: App) { super(app); @@ -38,7 +44,7 @@ export class AIAssistantProvidersModal extends Modal { this.open(); this.display(); - void this.autoSyncOnOpen(); + this.backgroundSync = this.autoSyncOnOpen(); } /** @@ -53,14 +59,32 @@ export class AIAssistantProvidersModal extends Modal { for (const provider of this.providers) { if (!provider.autoSyncModels) continue; try { - const { added } = await syncProviderModels(this.app, provider); - changed = changed || added > 0; + const { added, updated, discovered } = await syncProviderModels( + this.app, + provider, + ); + if (added === 0 && updated === 0) continue; + if (provider === this.selectedProvider) { + // The user opened this provider while the sync was in flight, so + // the list on screen and the Cancel snapshot both predate it. A + // background refresh is not a user edit: show it now, and keep it + // if the user cancels their edits. + if (this._selectedProviderClone) { + this._selectedProviderClone.models = mergeModels( + this._selectedProviderClone.models, + discovered, + ); + } + this.renderProviderModels(); + } else { + changed = changed || added > 0; + } } catch { // Quiet by design; "Sync now" surfaces errors. } } - // Refresh whatever view is showing, but never clobber in-progress edits. + // Refresh the provider list, but never clobber in-progress edits. if (changed && !this.selectedProvider) this.reload(); } @@ -85,6 +109,7 @@ export class AIAssistantProvidersModal extends Modal { private reload(): void { this.contentEl.empty(); + this.modelsContainerEl = null; this.display(); } @@ -244,12 +269,20 @@ export class AIAssistantProvidersModal extends Modal { }); } - addProviderModelsSetting(container: HTMLElement) { - const modelsContainer = container.createDiv({ + addProviderModelsSetting(container: HTMLElement) { + this.modelsContainerEl = container.createDiv({ cls: "models-container qa-ai-list-container", }); + this.renderProviderModels(); + } + + /** (Re)render the selected provider's model list and its "Add model" row. */ + private renderProviderModels(): void { + const modelsContainer = this.modelsContainerEl; + if (!modelsContainer || !this.selectedProvider) return; + modelsContainer.empty(); - this.selectedProvider!.models.forEach((model, i) => { + this.selectedProvider.models.forEach((model, i) => { const metadata = [`Context: ${model.maxTokens.toLocaleString()} tokens`]; if (model.maxOutputTokens) { metadata.push(`Output: ${model.maxOutputTokens.toLocaleString()} tokens`); @@ -351,10 +384,18 @@ export class AIAssistantProvidersModal extends Modal { }) .addButton((button) => { button.setButtonText("Sync now").onClick(async () => { + const provider = this.selectedProvider!; + // Report against the list the user is looking at. The quiet + // on-open sync may still be in flight for this provider; wait for + // it instead of racing it, and count whatever it lands too, so the + // notice never says "up to date" while the list visibly changes. + const shown = provider.models.map((model) => ({ ...model })); try { - const { added, updated } = await syncProviderModels( - this.app, - this.selectedProvider!, + await this.backgroundSync; + await syncProviderModels(this.app, provider); + const { added, updated } = diffModelLists( + shown, + provider.models, ); new Notice( added > 0 || updated > 0 From d77216ee4dcb02d63a97abfb3a37c977cc27f9c9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 26 Sep 2026 20:33:09 +0000 Subject: [PATCH 3/7] fix(ai): ignore Sync now after Cancel leaves the edit view Awaiting the on-open background sync left a window where Cancel swapped the live provider for the snapshot while Sync now still held the detached object. Re-check selectedProvider after each await before syncing or posting a notice, and merge Sync now discoveries into the Cancel snapshot so Cancel keeps sync results. Co-authored-by: Christian Bager Bach Houmann --- .../AIAssistantProvidersModal.sync.test.ts | 33 +++++++++++++++++++ src/gui/AIAssistantProvidersModal.ts | 16 ++++++++- 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/src/gui/AIAssistantProvidersModal.sync.test.ts b/src/gui/AIAssistantProvidersModal.sync.test.ts index 2b1fa751f..b5b5ca691 100644 --- a/src/gui/AIAssistantProvidersModal.sync.test.ts +++ b/src/gui/AIAssistantProvidersModal.sync.test.ts @@ -167,4 +167,37 @@ describe("AIAssistantProvidersModal model sync while editing", () => { "gpt-6-sol", ]); }); + + it("does not sync or notice after Cancel while Sync now awaits background sync", async () => { + const providers = [openAIProvider()]; + const modal = openAndEdit(providers); + + clickButtonByText(modal, "Sync now"); + clickButtonByText(modal, "Cancel"); + await landDiscovery(0, DIRECTORY); + await flush(); + + expect(notices()).toEqual([]); + expect(discovery.calls).toHaveLength(1); + expect(providers[0].models.map((model) => model.name)).toEqual([ + "gpt-5.5", + ]); + }); + + it("keeps Sync now results when the user cancels afterwards", async () => { + const providers = [openAIProvider()]; + const modal = openAndEdit(providers); + await landDiscovery(0, SHIPPED); + + clickButtonByText(modal, "Sync now"); + await landDiscovery(1, DIRECTORY); + providers[0].name = "Renamed by user"; + clickButtonByText(modal, "Cancel"); + + expect(providers[0].name).toBe("OpenAI"); + expect(providers[0].models.map((model) => model.name)).toEqual([ + "gpt-5.5", + "gpt-6-sol", + ]); + }); }); diff --git a/src/gui/AIAssistantProvidersModal.ts b/src/gui/AIAssistantProvidersModal.ts index cfcf82823..19a09a173 100644 --- a/src/gui/AIAssistantProvidersModal.ts +++ b/src/gui/AIAssistantProvidersModal.ts @@ -392,11 +392,25 @@ export class AIAssistantProvidersModal extends Modal { const shown = provider.models.map((model) => ({ ...model })); try { await this.backgroundSync; - await syncProviderModels(this.app, provider); + // Cancel/Save may have left the edit view while we waited; + // do not sync or notice against a detached provider object. + if (this.selectedProvider !== provider) return; + const { discovered } = await syncProviderModels( + this.app, + provider, + ); + if (this.selectedProvider !== provider) return; const { added, updated } = diffModelLists( shown, provider.models, ); + // Sync is not a user edit: keep its results if Cancel runs. + if (this._selectedProviderClone) { + this._selectedProviderClone.models = mergeModels( + this._selectedProviderClone.models, + discovered, + ); + } new Notice( added > 0 || updated > 0 ? `Synced from ${sourceDescription}: ${added} new model(s), ${updated} updated.` From 11f3b4a313d385bf840fcd96923cee36058cdae5 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 20:37:52 +0000 Subject: [PATCH 4/7] fix(ai): keep background sync results when Cancel discards an edit Builds on the previous commit. Cancel swaps the edited provider for its snapshot; a background sync still in flight for the discarded object now merges into the snapshot that replaced it, so Cancel keeps those models just as it keeps Sync now's. Sync now's guard checks that the provider is still in the list rather than still selected: Save while waiting keeps the provider, so the sync and its notice still run. The Sync now snapshot merge applies only while that provider is the one being edited; after Save the user may be editing another provider, whose snapshot must not receive these models. Amp-Thread-ID: https://ampcode.com/threads/T-01a0df48-9e0d-75cc-af45-26ec84217b88 Co-authored-by: Christian Bager Bach Houmann --- .../AIAssistantProvidersModal.sync.test.ts | 23 ++++++++++++- src/gui/AIAssistantProvidersModal.ts | 33 +++++++++++++++---- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/src/gui/AIAssistantProvidersModal.sync.test.ts b/src/gui/AIAssistantProvidersModal.sync.test.ts index b5b5ca691..2c3fcf564 100644 --- a/src/gui/AIAssistantProvidersModal.sync.test.ts +++ b/src/gui/AIAssistantProvidersModal.sync.test.ts @@ -177,10 +177,13 @@ describe("AIAssistantProvidersModal model sync while editing", () => { await landDiscovery(0, DIRECTORY); await flush(); - expect(notices()).toEqual([]); + // Sync now must not sync or announce anything for the discarded copy... expect(discovery.calls).toHaveLength(1); + expect(notices()).toEqual([]); + // ...and the background result lands on the snapshot Cancel restored. expect(providers[0].models.map((model) => model.name)).toEqual([ "gpt-5.5", + "gpt-6-sol", ]); }); @@ -200,4 +203,22 @@ describe("AIAssistantProvidersModal model sync while editing", () => { "gpt-6-sol", ]); }); + + it("still syncs when the user saves while Sync now waits", async () => { + const providers = [openAIProvider()]; + const modal = openAndEdit(providers); + + clickButtonByText(modal, "Sync now"); + clickButtonByText(modal, "Save"); + await landDiscovery(0, SHIPPED); + await landDiscovery(1, DIRECTORY); + + expect(notices()).toEqual([ + "Synced from the models.dev directory: 1 new model(s), 0 updated.", + ]); + expect(providers[0].models.map((model) => model.name)).toEqual([ + "gpt-5.5", + "gpt-6-sol", + ]); + }); }); diff --git a/src/gui/AIAssistantProvidersModal.ts b/src/gui/AIAssistantProvidersModal.ts index 19a09a173..ff3df0216 100644 --- a/src/gui/AIAssistantProvidersModal.ts +++ b/src/gui/AIAssistantProvidersModal.ts @@ -29,6 +29,13 @@ export class AIAssistantProvidersModal extends Modal { /** The quiet on-open sync pass; "Sync now" waits for it before counting. */ private readonly backgroundSync: Promise; + /** + * Cancel swaps an edited provider for its snapshot. A background sync that + * was in flight for the discarded object lands its models on the snapshot + * that replaced it instead. + */ + private readonly restoredSnapshots = new WeakMap(); + constructor(providers: AIProvider[], app: App) { super(app); @@ -64,7 +71,10 @@ export class AIAssistantProvidersModal extends Modal { provider, ); if (added === 0 && updated === 0) continue; - if (provider === this.selectedProvider) { + const restored = this.restoredSnapshots.get(provider); + if (restored) { + restored.models = mergeModels(restored.models, discovered); + } else if (provider === this.selectedProvider) { // The user opened this provider while the sync was in flight, so // the list on screen and the Cancel snapshot both predate it. A // background refresh is not a user edit: show it now, and keep it @@ -390,22 +400,29 @@ export class AIAssistantProvidersModal extends Modal { // it instead of racing it, and count whatever it lands too, so the // notice never says "up to date" while the list visibly changes. const shown = provider.models.map((model) => ({ ...model })); + // Cancel while waiting swaps this object out for its snapshot; + // syncing or reporting on the discarded copy would announce + // models nobody will see. + const isCurrent = () => this.providers.includes(provider); try { await this.backgroundSync; - // Cancel/Save may have left the edit view while we waited; - // do not sync or notice against a detached provider object. - if (this.selectedProvider !== provider) return; + if (!isCurrent()) return; const { discovered } = await syncProviderModels( this.app, provider, ); - if (this.selectedProvider !== provider) return; + if (!isCurrent()) return; const { added, updated } = diffModelLists( shown, provider.models, ); // Sync is not a user edit: keep its results if Cancel runs. - if (this._selectedProviderClone) { + // (After Save the user may be editing another provider, whose + // snapshot must not receive these models.) + if ( + provider === this.selectedProvider && + this._selectedProviderClone + ) { this._selectedProviderClone.models = mergeModels( this._selectedProviderClone.models, discovered, @@ -438,6 +455,10 @@ export class AIAssistantProvidersModal extends Modal { const index = this.providers.indexOf(this.selectedProvider); if (index !== -1) { this.providers[index] = this._selectedProviderClone; + this.restoredSnapshots.set( + this.selectedProvider, + this._selectedProviderClone, + ); } this.selectedProvider = null; From 7cf74343831aa97d25ffc5f2c0bdf20a5fb29655 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 20:46:52 +0000 Subject: [PATCH 5/7] fix(ai): count only source-reported models in the Sync now notice A model the user adds or imports while Sync now waits for the background sync was announced as synced. Count only models the sync source reports. Moving the snapshot after the wait would bring back the original bug: the list changing on screen while the notice said "already up to date". Amp-Thread-ID: https://ampcode.com/threads/T-01a0df48-9e0d-75cc-af45-26ec84217b88 Co-authored-by: Christian Bager Bach Houmann --- .../AIAssistantProvidersModal.sync.test.ts | 19 +++++++++++++++++++ src/gui/AIAssistantProvidersModal.ts | 5 ++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/gui/AIAssistantProvidersModal.sync.test.ts b/src/gui/AIAssistantProvidersModal.sync.test.ts index 2c3fcf564..ef4fdaf80 100644 --- a/src/gui/AIAssistantProvidersModal.sync.test.ts +++ b/src/gui/AIAssistantProvidersModal.sync.test.ts @@ -153,6 +153,25 @@ describe("AIAssistantProvidersModal model sync while editing", () => { expect(shownModelNames(modal)).toEqual(["gpt-5.5", "gpt-6-sol"]); }); + it("does not count a model the user adds while Sync now waits", async () => { + const providers = [openAIProvider()]; + const modal = openAndEdit(providers); + + clickButtonByText(modal, "Sync now"); + providers[0].models.push({ name: "my-local-model", maxTokens: 8192 }); + await landDiscovery(0, DIRECTORY); + await landDiscovery(1, DIRECTORY); + + expect(notices()).toEqual([ + "Synced from the models.dev directory: 1 new model(s), 0 updated.", + ]); + expect(providers[0].models.map((model) => model.name)).toEqual([ + "gpt-5.5", + "my-local-model", + "gpt-6-sol", + ]); + }); + it("keeps background-synced models when the user cancels their edits", async () => { const providers = [openAIProvider()]; const modal = openAndEdit(providers); diff --git a/src/gui/AIAssistantProvidersModal.ts b/src/gui/AIAssistantProvidersModal.ts index ff3df0216..0f4b578ee 100644 --- a/src/gui/AIAssistantProvidersModal.ts +++ b/src/gui/AIAssistantProvidersModal.ts @@ -412,9 +412,12 @@ export class AIAssistantProvidersModal extends Modal { provider, ); if (!isCurrent()) return; + // Count only models the source reports, so a model the user + // added by hand while waiting is not announced as synced. + const sourceNames = new Set(discovered.map((m) => m.name)); const { added, updated } = diffModelLists( shown, - provider.models, + provider.models.filter((m) => sourceNames.has(m.name)), ); // Sync is not a user edit: keep its results if Cancel runs. // (After Save the user may be editing another provider, whose From 2949d3ab763c8dc025f310ce076f005b03084749 Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 20:56:44 +0000 Subject: [PATCH 6/7] fix(ai): route every provider sync result through one landing path Review found three more races in the provider modal's sync handling, all from tracking which object a sync result belongs to at each call site: - Cancel while Sync now's own request ran dropped what it found. - A background sync lost its result after repeated Edit/Cancel rounds. - Sync now waited for the whole sequential background pass, so a stalled unrelated provider blocked it; double clicks announced models twice. applySyncResult now lands every finished sync on whatever represents the provider at that moment (following Cancel's snapshot swaps), merges it into an open edit's Cancel snapshot, and re-renders only when that list is on screen. Sync now no longer waits for the background pass: it runs its own request at once, still counts whatever lands on the provider meanwhile, and is disabled while its request runs. Amp-Thread-ID: https://ampcode.com/threads/T-01a0df48-9e0d-75cc-af45-26ec84217b88 Co-authored-by: Christian Bager Bach Houmann --- .../AIAssistantProvidersModal.sync.test.ts | 65 ++++++++- src/gui/AIAssistantProvidersModal.ts | 125 ++++++++++-------- 2 files changed, 129 insertions(+), 61 deletions(-) diff --git a/src/gui/AIAssistantProvidersModal.sync.test.ts b/src/gui/AIAssistantProvidersModal.sync.test.ts index ef4fdaf80..c4f739cf4 100644 --- a/src/gui/AIAssistantProvidersModal.sync.test.ts +++ b/src/gui/AIAssistantProvidersModal.sync.test.ts @@ -187,25 +187,78 @@ describe("AIAssistantProvidersModal model sync while editing", () => { ]); }); - it("does not sync or notice after Cancel while Sync now awaits background sync", async () => { + it("stays quiet when the user cancels while Sync now runs, and keeps what it found", async () => { const providers = [openAIProvider()]; const modal = openAndEdit(providers); + await landDiscovery(0, SHIPPED); clickButtonByText(modal, "Sync now"); clickButtonByText(modal, "Cancel"); - await landDiscovery(0, DIRECTORY); - await flush(); + await landDiscovery(1, DIRECTORY); - // Sync now must not sync or announce anything for the discarded copy... - expect(discovery.calls).toHaveLength(1); + // No list to report against after Cancel, but the restored snapshot + // must not lose what the sync found. expect(notices()).toEqual([]); - // ...and the background result lands on the snapshot Cancel restored. expect(providers[0].models.map((model) => model.name)).toEqual([ "gpt-5.5", "gpt-6-sol", ]); }); + it("lands a background sync on the latest snapshot after repeated Edit/Cancel", async () => { + const providers = [openAIProvider()]; + const modal = openAndEdit(providers); + clickButtonByText(modal, "Cancel"); + clickButtonByText(modal, "Edit"); + clickButtonByText(modal, "Cancel"); + + await landDiscovery(0, DIRECTORY); + + expect(providers[0].models.map((model) => model.name)).toEqual([ + "gpt-5.5", + "gpt-6-sol", + ]); + }); + + it("announces each model once when Sync now is clicked twice", async () => { + const modal = openAndEdit([openAIProvider()]); + await landDiscovery(0, SHIPPED); + + clickButtonByText(modal, "Sync now"); + clickButtonByText(modal, "Sync now"); + await landDiscovery(1, DIRECTORY); + discovery.calls[2]?.resolve(DIRECTORY); + await flush(); + + expect(notices()).toEqual([ + "Synced from the models.dev directory: 1 new model(s), 0 updated.", + ]); + }); + + it("does not wait for another provider's background sync", async () => { + const stalled: AIProvider = { + ...openAIProvider(), + id: "stalled", + name: "Stalled", + endpoint: "https://stalled.example/v1", + }; + const modal = new AIAssistantProvidersModal( + [stalled, openAIProvider()], + new App() as App, + ); + Array.from(modal.contentEl.querySelectorAll("button")) + .filter((button) => button.textContent === "Edit")[1] + .click(); + + // Request #0 is the stalled provider's background sync; it never lands. + clickButtonByText(modal, "Sync now"); + await landDiscovery(1, DIRECTORY); + + expect(notices()).toEqual([ + "Synced from the models.dev directory: 1 new model(s), 0 updated.", + ]); + }); + it("keeps Sync now results when the user cancels afterwards", async () => { const providers = [openAIProvider()]; const modal = openAndEdit(providers); diff --git a/src/gui/AIAssistantProvidersModal.ts b/src/gui/AIAssistantProvidersModal.ts index 0f4b578ee..c8492a51f 100644 --- a/src/gui/AIAssistantProvidersModal.ts +++ b/src/gui/AIAssistantProvidersModal.ts @@ -1,7 +1,7 @@ import { addProviderSecret } from "./ai/providerSettings"; import type { App } from "obsidian"; import { ButtonComponent, Modal, Notice, Setting } from "obsidian"; -import type { AIProvider } from "src/ai/Provider"; +import type { AIProvider, Model } from "src/ai/Provider"; import { ensureProviderIds } from "src/ai/Provider"; import { mergeModels } from "src/ai/modelsDirectory"; import { diffModelLists, syncProviderModels } from "src/ai/modelSyncService"; @@ -26,13 +26,10 @@ export class AIAssistantProvidersModal extends Modal { /** The edit view's model list, re-rendered in place when a sync changes it. */ private modelsContainerEl: HTMLElement | null = null; - /** The quiet on-open sync pass; "Sync now" waits for it before counting. */ - private readonly backgroundSync: Promise; - /** - * Cancel swaps an edited provider for its snapshot. A background sync that - * was in flight for the discarded object lands its models on the snapshot - * that replaced it instead. + * Cancel swaps an edited provider for its snapshot. A sync still in flight + * for the discarded object lands its models on the snapshot that replaced + * it instead (see currentFor). */ private readonly restoredSnapshots = new WeakMap(); @@ -51,7 +48,7 @@ export class AIAssistantProvidersModal extends Modal { this.open(); this.display(); - this.backgroundSync = this.autoSyncOnOpen(); + void this.autoSyncOnOpen(); } /** @@ -63,32 +60,15 @@ export class AIAssistantProvidersModal extends Modal { if (settingsStore.getState().disableOnlineFeatures) return; let changed = false; - for (const provider of this.providers) { + for (const provider of [...this.providers]) { if (!provider.autoSyncModels) continue; try { const { added, updated, discovered } = await syncProviderModels( this.app, provider, ); - if (added === 0 && updated === 0) continue; - const restored = this.restoredSnapshots.get(provider); - if (restored) { - restored.models = mergeModels(restored.models, discovered); - } else if (provider === this.selectedProvider) { - // The user opened this provider while the sync was in flight, so - // the list on screen and the Cancel snapshot both predate it. A - // background refresh is not a user edit: show it now, and keep it - // if the user cancels their edits. - if (this._selectedProviderClone) { - this._selectedProviderClone.models = mergeModels( - this._selectedProviderClone.models, - discovered, - ); - } - this.renderProviderModels(); - } else { - changed = changed || added > 0; - } + this.applySyncResult(provider, discovered, added + updated > 0); + changed = changed || added > 0; } catch { // Quiet by design; "Sync now" surfaces errors. } @@ -98,6 +78,51 @@ export class AIAssistantProvidersModal extends Modal { if (changed && !this.selectedProvider) this.reload(); } + /** + * The object that stands for `provider` now: itself while it is in the + * list, or the snapshot a Cancel swapped in for it (following repeated + * Edit/Cancel rounds). Null once the provider was deleted. + */ + private currentFor(provider: AIProvider): AIProvider | null { + let current: AIProvider | undefined = provider; + while (current && !this.providers.includes(current)) { + current = this.restoredSnapshots.get(current); + } + return current ?? null; + } + + /** + * Land a finished sync, which syncProviderModels already merged into + * `synced`, on whatever represents that provider now. A sync is not a user + * edit, so an open edit's Cancel snapshot receives it too, and the edit + * view re-renders when it shows that provider. + */ + private applySyncResult( + synced: AIProvider, + discovered: Model[], + syncedChanged: boolean, + ): void { + const current = this.currentFor(synced); + if (!current) return; + + let changed = syncedChanged; + if (current !== synced) { + const before = current.models; + current.models = mergeModels(before, discovered); + const diff = diffModelLists(before, current.models); + changed = diff.added + diff.updated > 0; + } + if (!changed || current !== this.selectedProvider) return; + + if (this._selectedProviderClone) { + this._selectedProviderClone.models = mergeModels( + this._selectedProviderClone.models, + discovered, + ); + } + this.renderProviderModels(); + } + private display(): void { const modalName = this.selectedProvider ? `${this.selectedProvider.name}` @@ -396,44 +421,32 @@ export class AIAssistantProvidersModal extends Modal { button.setButtonText("Sync now").onClick(async () => { const provider = this.selectedProvider!; // Report against the list the user is looking at. The quiet - // on-open sync may still be in flight for this provider; wait for - // it instead of racing it, and count whatever it lands too, so the - // notice never says "up to date" while the list visibly changes. + // on-open sync may land on this provider while this request + // runs; it is counted too, so the notice never says "up to + // date" while the list visibly changes. const shown = provider.models.map((model) => ({ ...model })); - // Cancel while waiting swaps this object out for its snapshot; - // syncing or reporting on the discarded copy would announce - // models nobody will see. - const isCurrent = () => this.providers.includes(provider); + button.setDisabled(true); try { - await this.backgroundSync; - if (!isCurrent()) return; - const { discovered } = await syncProviderModels( + const { added, updated, discovered } = await syncProviderModels( this.app, provider, ); - if (!isCurrent()) return; + this.applySyncResult(provider, discovered, added + updated > 0); + // Cancel swapped this provider for its snapshot while the + // request ran. The snapshot has the models now, but the list + // this click was about is gone, so there is nothing to report. + if (!this.providers.includes(provider)) return; + // Count only models the source reports, so a model the user - // added by hand while waiting is not announced as synced. + // added by hand meanwhile is not announced as synced. const sourceNames = new Set(discovered.map((m) => m.name)); - const { added, updated } = diffModelLists( + const counts = diffModelLists( shown, provider.models.filter((m) => sourceNames.has(m.name)), ); - // Sync is not a user edit: keep its results if Cancel runs. - // (After Save the user may be editing another provider, whose - // snapshot must not receive these models.) - if ( - provider === this.selectedProvider && - this._selectedProviderClone - ) { - this._selectedProviderClone.models = mergeModels( - this._selectedProviderClone.models, - discovered, - ); - } new Notice( - added > 0 || updated > 0 - ? `Synced from ${sourceDescription}: ${added} new model(s), ${updated} updated.` + counts.added > 0 || counts.updated > 0 + ? `Synced from ${sourceDescription}: ${counts.added} new model(s), ${counts.updated} updated.` : `Synced from ${sourceDescription}: already up to date.`, ); this.reload(); @@ -441,6 +454,8 @@ export class AIAssistantProvidersModal extends Modal { new Notice( `Sync failed: ${(err as { message?: string }).message ?? err}` ); + } finally { + button.setDisabled(false); } }); button.setCta(); From b36170a66f51b9c0f6689922af46f4e2ee84d50a Mon Sep 17 00:00:00 2001 From: Amp Date: Sat, 26 Sep 2026 21:10:42 +0000 Subject: [PATCH 7/7] fix(ai): let GPT-5.6 and GPT-6 run function tools on Chat Completions Review pointed out that gpt-6-astra tool calls fail. Live checks show it applies to every new seed: gpt-6-* and gpt-5.6-* reason by default, and /v1/chat/completions rejects function tools for them with "Function tools with reasoning_effort are not supported ... set reasoning_effort to 'none'". gpt-5.5 and older default to none, so they were unaffected. When a tool request gets exactly that 400 and the caller did not choose a reasoning effort, chatRequest now retries once with reasoning_effort "none". Other errors, requests without tools, and caller-set efforts are untouched. Verified in Obsidian: an ai.agent tool loop on gpt-6-sol and gpt-5.6-terra fails with the 400 before this change and completes after it. Amp-Thread-ID: https://ampcode.com/threads/T-01a0df48-9e0d-75cc-af45-26ec84217b88 Co-authored-by: Christian Bager Bach Houmann --- docs/src/content/docs/docs/QuickAddAPI.md | 5 + src/ai/OpenAIRequest.toolReasoning.test.ts | 166 +++++++++++++++++++++ src/ai/OpenAIRequest.ts | 42 +++++- 3 files changed, 212 insertions(+), 1 deletion(-) create mode 100644 src/ai/OpenAIRequest.toolReasoning.test.ts diff --git a/docs/src/content/docs/docs/QuickAddAPI.md b/docs/src/content/docs/docs/QuickAddAPI.md index dca134ea2..7ac0ad4d2 100644 --- a/docs/src/content/docs/docs/QuickAddAPI.md +++ b/docs/src/content/docs/docs/QuickAddAPI.md @@ -792,6 +792,11 @@ outright with a provider error - use a current model rather than expecting a bes These accept only the default `temperature` (omit it from `modelOptions`), and QuickAdd automatically sends `maxOutputTokens` as `max_completion_tokens` for them. The agent's default path sets neither, so `quickAddApi.ai.agent({ model: "gpt-5" })` works as-is. + +GPT-5.6 and GPT-6 models reason by default, and OpenAI's Chat Completions API rejects function +tools for them unless reasoning is off. When a tool turn is rejected for that reason, QuickAdd +retries it once with `reasoning_effort: "none"`. If you set `reasoning_effort` yourself in +`modelOptions`, QuickAdd keeps it and shows the provider's error instead. ::: ### `getModels(): string[]` diff --git a/src/ai/OpenAIRequest.toolReasoning.test.ts b/src/ai/OpenAIRequest.toolReasoning.test.ts new file mode 100644 index 000000000..f5c476d96 --- /dev/null +++ b/src/ai/OpenAIRequest.toolReasoning.test.ts @@ -0,0 +1,166 @@ +import { beforeEach, describe, expect, it } from "vitest"; +import type { AIProvider, Model } from "./Provider"; +import type { NormalizedChatRequest } from "./tools/NormalizedTools"; + +import { storeState, mocks, makeApp } from "../../tests/helpers/ai/requestHarness"; + +const { requestUrlMock, noticeMock } = mocks; + +const { chatRequest } = await import("./OpenAIRequest"); + +const openaiProvider: AIProvider = { + name: "OpenAI", + endpoint: "https://api.openai.com/v1", + kind: "openai", + apiKey: "sk", + models: [], + modelSource: "modelsDev", +}; + +const gpt6: Model = { + name: "gpt-6-sol", + maxTokens: 1_050_000, + maxOutputTokens: 128_000, + supportsTemperature: false, +}; + +// Exact live error for gpt-6-sol with function tools on /v1/chat/completions +// (2026-09-26); gpt-5.6-* and the other gpt-6-* models return the same text. +function toolsWhileReasoningFailure(model = "gpt-6-sol") { + return { + status: 400, + json: { + error: { + message: `Function tools with reasoning_effort are not supported for ${model} in /v1/chat/completions. To use function tools, use /v1/responses or set reasoning_effort to 'none'.`, + type: "invalid_request_error", + param: null, + code: null, + }, + }, + }; +} + +function toolCallSuccess() { + return { + status: 200, + json: Promise.resolve({ + id: "1", + model: "gpt-6-sol", + choices: [ + { + finish_reason: "tool_calls", + index: 0, + message: { + role: "assistant", + content: null, + tool_calls: [ + { + id: "call_1", + type: "function", + function: { name: "get_weather", arguments: '{"city":"Paris"}' }, + }, + ], + }, + }, + ], + usage: { prompt_tokens: 1, completion_tokens: 1, total_tokens: 2 }, + created: 0, + }), + }; +} + +function toolRequest( + modelParams: Record = {}, +): NormalizedChatRequest { + return { + messages: [{ role: "user", content: "Weather in Paris?" }], + modelParams, + tools: [ + { + name: "get_weather", + description: "Get weather", + parameters: { + type: "object", + properties: { city: { type: "string" } }, + required: ["city"], + }, + }, + ], + toolChoice: "auto", + }; +} + +function sentBody(callIndex: number): Record { + return JSON.parse(requestUrlMock.mock.calls[callIndex][0].body as string); +} + +beforeEach(() => { + requestUrlMock.mockReset(); + noticeMock.mockReset(); + storeState.disableOnlineFeatures = false; +}); + +describe("function tools on models that reason by default", () => { + it("retries once with reasoning_effort 'none' when the model rejects tools while reasoning", async () => { + requestUrlMock + .mockReturnValueOnce(Promise.resolve(toolsWhileReasoningFailure())) + .mockReturnValueOnce(Promise.resolve(toolCallSuccess())); + + const res = await chatRequest(makeApp(), "sk", gpt6, openaiProvider, toolRequest()); + + expect(res.toolCalls?.map((call) => call.name)).toEqual(["get_weather"]); + expect(requestUrlMock).toHaveBeenCalledTimes(2); + expect(sentBody(0).reasoning_effort).toBeUndefined(); + expect(sentBody(1).reasoning_effort).toBe("none"); + expect(sentBody(1).tools).toEqual(sentBody(0).tools); + }); + + it("keeps a reasoning effort the caller chose and surfaces the error", async () => { + requestUrlMock.mockReturnValueOnce( + Promise.resolve(toolsWhileReasoningFailure()), + ); + + await expect( + chatRequest( + makeApp(), + "sk", + gpt6, + openaiProvider, + toolRequest({ reasoning_effort: "high" }), + ), + ).rejects.toThrow(/reasoning_effort/); + expect(requestUrlMock).toHaveBeenCalledTimes(1); + }); + + it("does not retry other 400s", async () => { + requestUrlMock.mockReturnValueOnce( + Promise.resolve({ + status: 400, + json: { + error: { + message: "Invalid schema for function 'get_weather'.", + type: "invalid_request_error", + }, + }, + }), + ); + + await expect( + chatRequest(makeApp(), "sk", gpt6, openaiProvider, toolRequest()), + ).rejects.toThrow(/Invalid schema/); + expect(requestUrlMock).toHaveBeenCalledTimes(1); + }); + + it("does not add reasoning_effort to a request without tools", async () => { + requestUrlMock.mockReturnValueOnce( + Promise.resolve(toolsWhileReasoningFailure()), + ); + + await expect( + chatRequest(makeApp(), "sk", gpt6, openaiProvider, { + messages: [{ role: "user", content: "hi" }], + }), + ).rejects.toThrow(); + expect(requestUrlMock).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/ai/OpenAIRequest.ts b/src/ai/OpenAIRequest.ts index 35f4d89f5..b17c2afc0 100644 --- a/src/ai/OpenAIRequest.ts +++ b/src/ai/OpenAIRequest.ts @@ -206,10 +206,26 @@ export async function chatRequest( }); try { - const dispatch = (body: Record) => dispatchProviderRequest>({ + const send = (body: Record) => dispatchProviderRequest>({ kind, apiKey, provider: modelProvider, model, body, afterRequest: afterRequestCallback, }); + const dispatch = async (body: Record) => { + try { + return await send(body); + } catch (error) { + const retryBody = toolReasoningRetryBody( + kind, + body, + (error as { message?: string }).message ?? String(error), + ); + if (!retryBody) throw error; + log.logMessage( + `[AI Chat ${requestLogId}] ${model.name} rejected function tools while reasoning; retrying with reasoning_effort "none".`, + ); + return send(retryBody); + } + }; const json = await retrySampling( () => dispatch(body), () => dispatch(buildChatBody(kind, model.name, { @@ -260,6 +276,30 @@ export async function chatRequest( } } +// "Function tools with reasoning_effort are not supported for gpt-6-sol in +// /v1/chat/completions. To use function tools, use /v1/responses or set +// reasoning_effort to 'none'." (verified live 2026-09-26 for the gpt-6 and +// gpt-5.6 families, which reason by default; gpt-5.5 and older default to none). +const TOOLS_NEED_NO_REASONING_RE = + /function tools with reasoning_effort are not supported[\s\S]*reasoning_effort to 'none'/i; + +/** + * The body to retry a Chat Completions tool request with when the model + * rejected function tools because it reasons by default, or null when the + * error is anything else or the caller already chose a reasoning effort. + */ +export function toolReasoningRetryBody( + kind: ReturnType, + body: Record, + errorText: string, +): Record | null { + if (kind !== "openai") return null; + if (!Array.isArray(body.tools) || body.tools.length === 0) return null; + if (body.reasoning_effort !== undefined) return null; + if (!TOOLS_NEED_NO_REASONING_RE.test(errorText)) return null; + return { ...body, reasoning_effort: "none" }; +} + async function retrySampling(attempt: () => Promise, retry: () => Promise, context: { sentKeys: ReturnType; model: Model;