fix(web): deduplicate model options and restore reasoning scroll (#1933)

This commit is contained in:
Carlos Florêncio
2026-07-24 22:17:55 +01:00
committed by GitHub
parent 1e1b6c03a6
commit 2d950a72fb
3 changed files with 131 additions and 14 deletions
@@ -2,7 +2,11 @@ import { cleanup, fireEvent, render, screen, within } from "@testing-library/rea
import { TooltipProvider } from "@kandev/ui/tooltip";
import { afterEach, describe, expect, it, vi } from "vitest";
import { ModelConfigSelector } from "@/components/model-config-selector";
import {
configOptionToModelOptions,
ModelConfigSelector,
type SelectConfigOption,
} from "@/components/model-config-selector";
afterEach(() => {
cleanup();
@@ -12,6 +16,7 @@ const effortSectionTestId = "config-option-section-effort";
const effortTriggerTestId = "config-option-trigger-effort";
const modelSettingsButtonName = "Model settings";
const providerModelId = "gpt-5.6-sol";
const providerModelName = "GPT-5.6-Sol";
const optionDescription = "Controls how much reasoning the model performs.";
const effortOptionName = "Reasoning Effort";
const makeModelOptions = (count: number) =>
@@ -98,6 +103,38 @@ describe("ModelConfigSelector", () => {
});
describe("ModelConfigSelector filtering", () => {
it("deduplicates repeated model ids from provider config", () => {
const modelConfig: SelectConfigOption = {
type: "select",
id: "model",
name: "Model",
currentValue: providerModelId,
category: "model",
options: [
{
value: providerModelId,
name: providerModelName,
description: "Latest frontier agentic coding model.",
},
{
value: "gpt-5.6-terra",
name: "GPT-5.6-Terra",
description: "Balanced agentic coding model for everyday work.",
},
{
value: "gpt-5.6-terra",
name: "GPT-5.6-Terra",
description: "Balanced agentic coding model for everyday work.",
},
],
};
expect(configOptionToModelOptions(modelConfig).map((model) => model.id)).toEqual([
providerModelId,
"gpt-5.6-terra",
]);
});
it("hides the model filter when there are five or fewer models", () => {
render(
<ModelConfigSelector
@@ -131,7 +168,7 @@ describe("ModelConfigSelector provider descriptions", () => {
it("shows provider descriptions only inside the selected option submenu", () => {
render(
<ModelConfigSelector
modelOptions={[{ id: providerModelId, name: "GPT-5.6-Sol" }]}
modelOptions={[{ id: providerModelId, name: providerModelName }]}
currentModel={providerModelId}
onModelChange={() => {}}
onConfigChange={() => {}}
@@ -166,7 +203,7 @@ describe("ModelConfigSelector provider descriptions", () => {
render(
<TooltipProvider>
<ModelConfigSelector
modelOptions={[{ id: providerModelId, name: "GPT-5.6-Sol" }]}
modelOptions={[{ id: providerModelId, name: providerModelName }]}
currentModel={providerModelId}
onModelChange={() => {}}
triggerSummary="changed"
@@ -201,7 +238,7 @@ describe("ModelConfigSelector provider descriptions", () => {
fireEvent.focus(screen.getByRole("button", { name: modelSettingsButtonName }));
const tooltip = screen.getByRole("tooltip");
expect(tooltip.textContent).toContain("Model: GPT-5.6-Sol");
expect(tooltip.textContent).toContain(`Model: ${providerModelName}`);
expect(tooltip.textContent).toContain("Reasoning Effort: Low");
expect(tooltip.textContent).toContain("Fast Mode: Off");
expect(tooltip.textContent).not.toContain(optionDescription);
@@ -211,7 +248,7 @@ describe("ModelConfigSelector provider descriptions", () => {
render(
<TooltipProvider>
<ModelConfigSelector
modelOptions={[{ id: providerModelId, name: "GPT-5.6-Sol" }]}
modelOptions={[{ id: providerModelId, name: providerModelName }]}
currentModel={providerModelId}
onModelChange={() => {}}
configOptions={[
+15 -8
View File
@@ -77,11 +77,18 @@ export function configOptionToModelOptions(
option: SelectConfigOption | undefined,
): ModelSelectorOption[] {
if (!option) return [];
return option.options.map((item) => ({
id: item.value,
name: item.name,
description: item.description ?? (item.value !== item.name ? item.value : undefined),
}));
const seen = new Set<string>();
return option.options.flatMap((item) => {
if (seen.has(item.value)) return [];
seen.add(item.value);
return [
{
id: item.value,
name: item.name,
description: item.description ?? (item.value !== item.name ? item.value : undefined),
},
];
});
}
function currentOptionValue(option: DynamicConfigOption) {
@@ -258,8 +265,8 @@ function ConfigOptionSubSelector({
)}
</span>
</button>
<ScrollArea
className="max-h-[min(18rem,calc(100vh-8rem))] pr-2"
<div
className="max-h-[min(18rem,calc(100dvh-8rem))] overflow-y-auto overscroll-contain pr-2"
data-testid={`config-option-section-${option.id}`}
>
<div className="space-y-1">
@@ -303,7 +310,7 @@ function ConfigOptionSubSelector({
);
})}
</div>
</ScrollArea>
</div>
</div>
);
}
@@ -4,6 +4,28 @@ import type { ApiClient } from "../../helpers/api-client";
import type { SeedData } from "../../fixtures/test-base";
import { SessionPage } from "../../pages/session-page";
type E2EModelStoreWindow = Window & {
__KANDEV_E2E_STORE__?: {
setState: (
updater: (state: {
sessionModels: {
bySessionId: Record<
string,
{
currentModelId: string;
models: unknown[];
configOptions: {
id: string;
options?: { value: string; name: string; description?: string }[];
}[];
}
>;
};
}) => void,
) => void;
};
};
async function seedAndOpenTask(testPage: Page, apiClient: ApiClient, seedData: SeedData) {
const task = await apiClient.createTaskWithAgent(
seedData.workspaceId,
@@ -24,6 +46,32 @@ async function seedAndOpenTask(testPage: Page, apiClient: ApiClient, seedData: S
return { session, task };
}
async function seedLongReasoningMenu(testPage: Page, sessionId: string) {
await testPage.evaluate((sid) => {
const store = (window as E2EModelStoreWindow).__KANDEV_E2E_STORE__;
if (!store) throw new Error("E2E store bridge is unavailable");
store.setState((state) => {
const sessionModels = state.sessionModels.bySessionId[sid];
if (!sessionModels) throw new Error(`Session ${sid} has no model state`);
sessionModels.configOptions = sessionModels.configOptions.map((option) =>
option.id === "effort"
? {
...option,
options: Array.from({ length: 10 }, (_, index) => ({
value: index === 0 ? "low" : `effort-${index + 1}`,
name: index === 0 ? "Low" : `Reasoning level ${index + 1}`,
description:
index === 0
? "Faster responses with less reasoning"
: `Reasoning depth description for level ${index + 1}`,
})),
}
: option,
);
});
}, sessionId);
}
test.describe("Mobile chat model selector", () => {
test.describe.configure({ retries: 1, timeout: 60_000 });
@@ -33,6 +81,7 @@ test.describe("Mobile chat model selector", () => {
seedData,
}, testInfo) => {
const { task } = await seedAndOpenTask(testPage, apiClient, seedData);
expect(task.session_id).toBeTruthy();
await expect
.poll(async () => {
@@ -44,6 +93,8 @@ test.describe("Mobile chat model selector", () => {
})
.toBe("medium");
await seedLongReasoningMenu(testPage, task.session_id!);
const leftActions = testPage.getByTestId("mobile-chat-toolbar-left-actions");
await expect(leftActions).toBeVisible({ timeout: 15_000 });
await expect(testPage.getByTestId("toolbar-overflow-menu")).not.toBeVisible();
@@ -73,7 +124,29 @@ test.describe("Mobile chat model selector", () => {
testPage.getByText("Controls how much reasoning the mock model uses", { exact: true }),
).toHaveCount(0);
await effortTrigger.tap();
await expect(testPage.getByTestId("config-option-section-effort")).toBeVisible();
const effortSection = testPage.getByTestId("config-option-section-effort");
await expect(effortSection).toBeVisible();
const overflow = await effortSection.evaluate((element) => ({
clientHeight: element.clientHeight,
overflowY: getComputedStyle(element).overflowY,
scrollHeight: element.scrollHeight,
}));
expect(overflow.overflowY).toBe("auto");
expect(overflow.scrollHeight).toBeGreaterThan(overflow.clientHeight);
const effortBox = await effortSection.boundingBox();
expect(effortBox).not.toBeNull();
await testPage.mouse.move(
effortBox!.x + effortBox!.width / 2,
effortBox!.y + effortBox!.height / 2,
);
await testPage.mouse.wheel(0, 800);
await expect
.poll(() => effortSection.evaluate((element) => element.scrollTop))
.toBeGreaterThan(0);
await testInfo.attach("task-model-reasoning-scroll-mobile", {
body: await testPage.screenshot(),
contentType: "image/png",
});
await expect(
testPage.getByText("Controls how much reasoning the mock model uses", { exact: true }),
).toBeVisible();