diff --git a/CHANGELOG.md b/CHANGELOG.md index 5cc5f8ea7..b3d20d2f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Silenced a false-positive `MaxListenersExceededWarning` logged on every request proxied through an external rewrite. [#1697](https://github.com/sourcebot-dev/sourcebot/pull/1697) +- Fixed Ask model selections reverting to a stale default after the model configuration changed during a browser session. [#1710](https://github.com/sourcebot-dev/sourcebot/pull/1710) - Upgraded `next` to `^16.3.8`. [#1709](https://github.com/sourcebot-dev/sourcebot/pull/1709) ## [5.1.15] - 2026-09-29 diff --git a/packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx b/packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx index 956f958f7..df9208aae 100644 --- a/packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx +++ b/packages/web/src/app/(app)/askgh/[owner]/[repo]/components/landingPage.tsx @@ -87,6 +87,7 @@ export const LandingPage = ({ }} className="min-h-[50px]" isRedirecting={isLoading} + languageModels={languageModels} selectedSearchScopes={selectedSearchScopes} searchContexts={[]} askCommands={askCommands} diff --git a/packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx b/packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx index 9e22d12ee..b01372b7f 100644 --- a/packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx +++ b/packages/web/src/app/(app)/chat/components/landingPageChatBox.tsx @@ -50,6 +50,7 @@ export const LandingPageChatBox = ({ }} className="min-h-[50px]" isRedirecting={isLoading} + languageModels={languageModels} selectedSearchScopes={selectedSearchScopes} searchContexts={searchContexts} askCommands={askCommands} diff --git a/packages/web/src/app/(app)/layout.tsx b/packages/web/src/app/(app)/layout.tsx index ef299a775..1248a760f 100644 --- a/packages/web/src/app/(app)/layout.tsx +++ b/packages/web/src/app/(app)/layout.tsx @@ -32,7 +32,6 @@ import { RoleProvider } from "@/features/auth/roleProvider"; import { HasLicenseProvider } from "@/features/billing/hasLicenseProvider"; import { tryGetLatestSourcebotTag } from "./components/banners/actions"; import { LanguageModelProvider } from "@/features/chat/languageModelContext"; -import { getConfiguredLanguageModelsInfo } from "@/features/chat/utils.server"; import { NavigationGuardProvider } from "next-navigation-guard"; import { getRepositorySyncCounts } from "@/features/repos/repositorySyncCounts.server"; import { getConnectionSyncCounts } from "@/features/connections/connectionSyncCounts.server"; @@ -183,15 +182,13 @@ export default async function Layout(props: LayoutProps) { timeoutMs: 3000 }); - const languageModels = await getConfiguredLanguageModelsInfo(); - return ( - + {/* Keep one guard provider above both sidebar and content so browser history is tracked before guarded routes mount. */} diff --git a/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx b/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx index 79856b8ad..a2d5e6d76 100644 --- a/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx +++ b/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx @@ -126,7 +126,7 @@ export const ChatThread = ({ const [failedMcpServers, setFailedMcpServers] = useState([]); const [isFailedMcpBannerVisible, setIsFailedMcpBannerVisible] = useState(false); - const { selectedLanguageModel } = useSelectedLanguageModel(); + const { selectedLanguageModel } = useSelectedLanguageModel(languageModels); // Refs to capture the latest request params for the transport body. // The transport is created once (useMemo) but params change over time, @@ -566,6 +566,7 @@ export const ChatThread = ({ isTurnInProgress={isTurnInProgress} isNetworkActive={isNetworkActive} onStop={stop} + languageModels={languageModels} selectedSearchScopes={selectedSearchScopes} searchContexts={searchContexts} askCommands={askCommands} diff --git a/packages/web/src/features/chat/components/chatBox/chatBox.tsx b/packages/web/src/features/chat/components/chatBox/chatBox.tsx index 57eff7b4a..447d41d4b 100644 --- a/packages/web/src/features/chat/components/chatBox/chatBox.tsx +++ b/packages/web/src/features/chat/components/chatBox/chatBox.tsx @@ -3,7 +3,7 @@ import { Button } from "@/components/ui/button"; import { Tooltip, TooltipTrigger } from "@/components/ui/tooltip"; import { FileMentionComponent, MentionChip } from "@/features/chat/components/mentionChip"; -import { AttachmentData, CustomEditor, MentionElement, RenderElementPropsFor, SearchScope } from "@/features/chat/types"; +import { AttachmentData, CustomEditor, LanguageModelInfo, MentionElement, RenderElementPropsFor, SearchScope } from "@/features/chat/types"; import { insertMention, slateContentToString } from "@/features/chat/utils"; import { createPastedTextAttachment, getSubmittedTextBytes, PendingAttachment, PendingImageAttachment, readFilesAsAttachments, shouldAutoConvertPaste, toAttachmentData, uploadImageAttachment } from "@/features/chat/attachmentUtils"; import { AttachmentButton } from "./attachmentButton"; @@ -56,6 +56,7 @@ interface ChatBoxProps { isTurnInProgress?: boolean; isNetworkActive?: boolean; isDisabled?: boolean; + languageModels: LanguageModelInfo[]; selectedSearchScopes: SearchScope[]; searchContexts: SearchContextQuery[]; askCommands: AskCommandDefinition[]; @@ -78,6 +79,7 @@ const ChatBoxComponent = ({ isDisabled, isLoginWallEnabled, isAuthenticated, + languageModels, selectedSearchScopes, searchContexts, maxImageBytes = ATTACHMENT_MAX_IMAGE_BYTES, @@ -107,7 +109,7 @@ const ChatBoxComponent = ({ }).flat(), askCommands, }); - const { selectedLanguageModel } = useSelectedLanguageModel(); + const { selectedLanguageModel } = useSelectedLanguageModel(languageModels); const { toast } = useToast(); const isMac = useIsMac(); const isAskEnabled = useHasEntitlement('ask'); diff --git a/packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx b/packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx index 9a9edb7eb..f4006d394 100644 --- a/packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx +++ b/packages/web/src/features/chat/components/chatBox/chatBoxToolbar.tsx @@ -36,7 +36,7 @@ export const ChatBoxToolbar = ({ onDisabledMcpServerIdsChange, isAuthenticated, }: ChatBoxToolbarProps) => { - const { selectedLanguageModel, setSelectedLanguageModel } = useSelectedLanguageModel(); + const { selectedLanguageModel, setSelectedLanguageModel } = useSelectedLanguageModel(languageModels); return ( <> diff --git a/packages/web/src/features/chat/languageModelContext.tsx b/packages/web/src/features/chat/languageModelContext.tsx index aca76d846..b0be7c4d5 100644 --- a/packages/web/src/features/chat/languageModelContext.tsx +++ b/packages/web/src/features/chat/languageModelContext.tsx @@ -1,80 +1,39 @@ 'use client'; -import { createContext, useEffect, useMemo, type ReactNode } from "react"; +import { createContext, useMemo, type ReactNode } from "react"; import { useLocalStorage } from "usehooks-ts"; import { LanguageModelInfo } from "./types"; -import { getLanguageModelKey } from "./utils"; export interface SelectedLanguageModelContextValue { - languageModels: LanguageModelInfo[]; - selectedLanguageModel: LanguageModelInfo | undefined; + storedLanguageModel: LanguageModelInfo | undefined; setSelectedLanguageModel: (model: LanguageModelInfo | undefined) => void; } export const SelectedLanguageModelContext = createContext(null); interface LanguageModelProviderProps { - languageModels: LanguageModelInfo[]; children: ReactNode; } -// Single owner of the selected-language-model state. Mounted once (in the (app) -// layout), so the selection lives in one place instead of being re-derived by a -// `useSelectedLanguageModel` hook in every consumer. Previously each consumer -// ran its own reset effect against the shared "selectedLanguageModel" -// localStorage key, and because usehooks-ts broadcasts a storage event on every -// write, those instances re-triggered each other into a rapid write loop when a -// model was removed. +// Single owner of the persisted model preference, mounted in the (app) layout. +// It must not hold the configured model list: layouts do not re-render on +// client-side navigation, so that list would go stale after a config change. +// `useSelectedLanguageModel` resolves the preference against the page's list. export const LanguageModelProvider = ({ - languageModels, children, }: LanguageModelProviderProps) => { - const fallbackLanguageModel = languageModels.length > 0 ? languageModels[0] : undefined; - const [selectedLanguageModel, setSelectedLanguageModel] = useLocalStorage( + const [storedLanguageModel, setSelectedLanguageModel] = useLocalStorage( "selectedLanguageModel", - fallbackLanguageModel, + undefined, { initializeWithValue: false, } ); - // Handle the case where the selected language model is no longer available. - // Reset to the fallback language model in this case. Only write when the - // resolved selection actually differs (compared by key, since the stored - // value is a fresh object reference on every read) — otherwise the effect - // would re-write on every render. - useEffect(() => { - const selectedKey = selectedLanguageModel - ? getLanguageModelKey(selectedLanguageModel) - : undefined; - - const isSelectedModelAvailable = selectedKey !== undefined && languageModels.some( - (model) => getLanguageModelKey(model) === selectedKey - ); - - if (isSelectedModelAvailable) { - return; - } - - const fallbackKey = fallbackLanguageModel - ? getLanguageModelKey(fallbackLanguageModel) - : undefined; - - if (fallbackKey !== selectedKey) { - setSelectedLanguageModel(fallbackLanguageModel); - } - }, [ - fallbackLanguageModel, - languageModels, - selectedLanguageModel, - setSelectedLanguageModel, - ]); - const value = useMemo(() => ({ - languageModels, - selectedLanguageModel, + storedLanguageModel, setSelectedLanguageModel, - }), [languageModels, selectedLanguageModel, setSelectedLanguageModel]); + }), [storedLanguageModel, setSelectedLanguageModel]); return ( diff --git a/packages/web/src/features/chat/useSelectedLanguageModel.test.tsx b/packages/web/src/features/chat/useSelectedLanguageModel.test.tsx new file mode 100644 index 000000000..3d5566fc1 --- /dev/null +++ b/packages/web/src/features/chat/useSelectedLanguageModel.test.tsx @@ -0,0 +1,173 @@ +import { cleanup, fireEvent, render, screen } from "@testing-library/react"; +import { afterEach, beforeAll, beforeEach, describe, expect, test, vi } from "vitest"; +import { LanguageModelSelector } from "./components/chatBox/languageModelSelector"; +import { LanguageModelProvider } from "./languageModelContext"; +import { LanguageModelInfo } from "./types"; +import { useSelectedLanguageModel } from "./useSelectedLanguageModel"; + +vi.mock("./components/chatBox/modelProviderLogo", () => ({ + ModelProviderLogo: () => null, +})); + +const STORAGE_KEY = "selectedLanguageModel"; + +const makeModel = (model: string, overrides: Partial = {}): LanguageModelInfo => ({ + provider: "anthropic", + model, + inputModalities: ["text"], + supportedDocumentTypes: [], + ...overrides, +}); + +const oldModels = [makeModel("old-default"), makeModel("old-other")]; +const newModels = [makeModel("new-default"), makeModel("new-other")]; + +// Mirrors a chat page: the toolbar selector and the chat thread each resolve +// the selection against the page's model list. +const Toolbar = ({ languageModels }: { languageModels: LanguageModelInfo[] }) => { + const { selectedLanguageModel, setSelectedLanguageModel } = useSelectedLanguageModel(languageModels); + return ( + + ); +}; + +const Thread = ({ languageModels }: { languageModels: LanguageModelInfo[] }) => { + const { selectedLanguageModel } = useSelectedLanguageModel(languageModels); + return ( + + {selectedLanguageModel?.model ?? "none"} + + ); +}; + +const ChatPage = ({ languageModels }: { languageModels: LanguageModelInfo[] }) => ( + <> + + + +); + +// The provider is mounted once by the (app) layout and survives client-side +// navigation, while each page renders with a freshly loaded model list. +const renderSession = (languageModels: LanguageModelInfo[]) => { + const result = render( + + + + ); + + return { + navigateWithModels: (nextModels: LanguageModelInfo[]) => result.rerender( + + + + ), + }; +}; + +const selectModel = (currentModel: string, nextModel: string) => { + fireEvent.click(screen.getByRole("button", { name: currentModel })); + fireEvent.click(screen.getByRole("option", { name: nextModel })); +}; + +const getStoredModel = () => { + const stored = window.localStorage.getItem(STORAGE_KEY); + return stored ? (JSON.parse(stored) as LanguageModelInfo).model : undefined; +}; + +beforeAll(() => { + // cmdk relies on these browser APIs, which jsdom does not implement. + globalThis.ResizeObserver ??= class { + observe() {} + unobserve() {} + disconnect() {} + }; + Element.prototype.scrollIntoView ??= () => {}; +}); + +// Node 25 ships a global localStorage that shadows jsdom's, so install a fresh +// in-memory store for each test. +const installMockLocalStorage = () => { + const store = new Map(); + const storage: Storage = { + get length() { + return store.size; + }, + clear: () => store.clear(), + getItem: (key: string) => store.get(key) ?? null, + key: (index: number) => Array.from(store.keys())[index] ?? null, + removeItem: (key: string) => { + store.delete(key); + }, + setItem: (key: string, value: string) => { + store.set(key, value); + }, + }; + Object.defineProperty(window, "localStorage", { configurable: true, value: storage }); + Object.defineProperty(globalThis, "localStorage", { configurable: true, value: storage }); +}; + +beforeEach(() => { + installMockLocalStorage(); +}); + +afterEach(() => { + cleanup(); +}); + +describe("useSelectedLanguageModel", () => { + test("restores the stored model on mount", () => { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(oldModels[1])); + + renderSession(oldModels); + + expect(screen.getByRole("button", { name: "old-other" })).toBeTruthy(); + expect(screen.getByTestId("submitted-model").textContent).toBe("old-other"); + }); + + test("keeps a newly configured model selected after the config changes mid-session", () => { + const session = renderSession(oldModels); + session.navigateWithModels(newModels); + + selectModel("new-default", "new-other"); + + expect(screen.getByRole("button", { name: "new-other" })).toBeTruthy(); + expect(screen.getByTestId("submitted-model").textContent).toBe("new-other"); + expect(getStoredModel()).toBe("new-other"); + }); + + test("converges on the first configured model when the selected model is removed", () => { + const session = renderSession(oldModels); + selectModel("old-default", "old-other"); + expect(getStoredModel()).toBe("old-other"); + + session.navigateWithModels(newModels); + + expect(screen.getByRole("button", { name: "new-default" })).toBeTruthy(); + expect(screen.getByTestId("submitted-model").textContent).toBe("new-default"); + // Reconciliation is derived, not written back, so consumers cannot + // trigger each other into a storage write loop. + expect(getStoredModel()).toBe("old-other"); + }); + + test("uses the current config's capabilities for a stored model", () => { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(oldModels[1])); + const session = renderSession(oldModels); + + session.navigateWithModels([oldModels[0], makeModel("old-other", { inputModalities: ["text", "image"] })]); + + expect(screen.getByTestId("submitted-model").dataset.modalities).toBe("text,image"); + }); + + test("selects nothing when no models are configured", () => { + window.localStorage.setItem(STORAGE_KEY, JSON.stringify(oldModels[0])); + + renderSession([]); + + expect(screen.getByTestId("submitted-model").textContent).toBe("none"); + }); +}); diff --git a/packages/web/src/features/chat/useSelectedLanguageModel.ts b/packages/web/src/features/chat/useSelectedLanguageModel.ts index 8983ccf52..adcb02e98 100644 --- a/packages/web/src/features/chat/useSelectedLanguageModel.ts +++ b/packages/web/src/features/chat/useSelectedLanguageModel.ts @@ -1,15 +1,33 @@ 'use client'; -import { useContext } from "react"; +import { useContext, useMemo } from "react"; import { SelectedLanguageModelContext } from "./languageModelContext"; +import { LanguageModelInfo } from "./types"; +import { getLanguageModelKey } from "./utils"; -export const useSelectedLanguageModel = () => { +// Returns the list's own entry rather than the stored copy so capabilities +// reflect the current config. Unavailable selections fall back without writing +// to storage, so multiple consumers cannot trigger each other into a write loop. +export const useSelectedLanguageModel = (languageModels: LanguageModelInfo[]) => { const context = useContext(SelectedLanguageModelContext); if (!context) { throw new Error("useSelectedLanguageModel must be used within a LanguageModelProvider"); } - const { selectedLanguageModel, setSelectedLanguageModel } = context; + const { storedLanguageModel, setSelectedLanguageModel } = context; + + const selectedLanguageModel = useMemo(() => { + if (storedLanguageModel) { + const storedKey = getLanguageModelKey(storedLanguageModel); + const match = languageModels.find((model) => getLanguageModelKey(model) === storedKey); + if (match) { + return match; + } + } + + return languageModels.length > 0 ? languageModels[0] : undefined; + }, [storedLanguageModel, languageModels]); + return { selectedLanguageModel, setSelectedLanguageModel,