From ff12dd1c15ea8c6b560fa9059be4ba4fbb92735c Mon Sep 17 00:00:00 2001 From: Brice Stacey Date: Wed, 15 Jul 2026 10:13:08 -0400 Subject: [PATCH 1/6] Add provider grouping for Configure LLM Providers modal See #14817 --- .../common/providerGrouping.ts | 83 +++++++++++++++++++ .../test/common/providerGrouping.vitest.ts | 61 ++++++++++++++ 2 files changed, 144 insertions(+) create mode 100644 src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts create mode 100644 src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts diff --git a/src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts b/src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts new file mode 100644 index 000000000000..e97e3afafb27 --- /dev/null +++ b/src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts @@ -0,0 +1,83 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import { IPositronLanguageModelSource } from './interfaces/positronAssistantService.js'; + +/** Section identifiers, in fixed display order. */ +export type ProviderSectionId = 'needs-attention' | 'connected' | 'custom' | 'approved' | 'available'; + +/** A non-empty group of providers to render under one heading. */ +export interface ProviderSection { + id: ProviderSectionId; + items: IPositronLanguageModelSource[]; +} + +const SECTION_ORDER: ProviderSectionId[] = ['needs-attention', 'connected', 'custom', 'approved', 'available']; + +/** Only chat providers (and the copilot-auth completion provider) are shown, mirroring the legacy modal. */ +function isDisplayable(source: IPositronLanguageModelSource): boolean { + return source.type === 'chat' || (source.type === 'completion' && source.provider.id === 'copilot-auth'); +} + +/** Which section a source belongs to, based on sign-in and connection status. */ +function sectionFor(source: IPositronLanguageModelSource): ProviderSectionId { + if (source.signedIn && source.status === 'error') { + return 'needs-attention'; + } + if (source.signedIn) { + return 'connected'; + } + return 'available'; +} + +/** Sort rank within a section: Posit AI first, then stable, preview, experimental. */ +function sortRank(source: IPositronLanguageModelSource): number { + if (source.provider.id === 'posit-ai') { + return 0; + } + switch (source.provider.status) { + case 'preview': + return 2; + case 'experimental': + return 3; + default: + return 1; + } +} + +function compareSources(a: IPositronLanguageModelSource, b: IPositronLanguageModelSource): number { + const rankDiff = sortRank(a) - sortRank(b); + if (rankDiff !== 0) { + return rankDiff; + } + return a.provider.displayName.localeCompare(b.provider.displayName); +} + +/** + * Groups language model sources into ordered, non-empty sections for the + * Configure LLM Providers modal. Custom and Approved sections have no backing + * data yet and will simply be absent until sources land in those buckets. + */ +export function groupProviders(sources: IPositronLanguageModelSource[]): ProviderSection[] { + const buckets = new Map(); + for (const source of sources) { + if (!isDisplayable(source)) { + continue; + } + const id = sectionFor(source); + const items = buckets.get(id) ?? []; + items.push(source); + buckets.set(id, items); + } + + const sections: ProviderSection[] = []; + for (const id of SECTION_ORDER) { + const items = buckets.get(id); + if (items && items.length > 0) { + sections.push({ id, items: items.sort(compareSources) }); + } + } + return sections; +} diff --git a/src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts b/src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts new file mode 100644 index 000000000000..29e7489402ff --- /dev/null +++ b/src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts @@ -0,0 +1,61 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/// + +import { groupProviders } from '../../common/providerGrouping.js'; +import { IPositronLanguageModelSource, PositronLanguageModelType } from '../../common/interfaces/positronAssistantService.js'; + +function source(overrides: Partial & { id: string }): IPositronLanguageModelSource { + const { id, ...rest } = overrides; + return { + type: PositronLanguageModelType.Chat, + provider: { id, displayName: id, settingName: id }, + supportedOptions: [], + defaults: {}, + ...rest, + } as IPositronLanguageModelSource; +} + +describe('groupProviders', () => { + it('buckets signed-in error sources into needs-attention', () => { + const sections = groupProviders([source({ id: 'a', signedIn: true, status: 'error' })]); + expect(sections).toHaveLength(1); + expect(sections[0].id).toBe('needs-attention'); + expect(sections[0].items.map(i => i.provider.id)).toEqual(['a']); + }); + + it('buckets signed-in non-error into connected and signed-out into available', () => { + const sections = groupProviders([ + source({ id: 'a', signedIn: true, status: 'ok' }), + source({ id: 'b', signedIn: false }), + ]); + expect(sections.map(s => s.id)).toEqual(['connected', 'available']); + }); + + it('omits empty sections', () => { + const sections = groupProviders([source({ id: 'b', signedIn: false })]); + expect(sections.map(s => s.id)).toEqual(['available']); + }); + + it('filters out non-chat sources except copilot-auth completion', () => { + const sections = groupProviders([ + source({ id: 'comp', type: PositronLanguageModelType.Completion }), + source({ id: 'copilot-auth', type: PositronLanguageModelType.Completion, signedIn: false }), + ]); + expect(sections).toHaveLength(1); + expect(sections[0].items.map(i => i.provider.id)).toEqual(['copilot-auth']); + }); + + it('sorts posit-ai first, then stable, preview, experimental, then alphabetical', () => { + const sections = groupProviders([ + source({ id: 'zebra', signedIn: false }), + source({ id: 'exp', signedIn: false, provider: { id: 'exp', displayName: 'exp', settingName: 'exp', status: 'experimental' } }), + source({ id: 'posit-ai', signedIn: false }), + source({ id: 'alpha', signedIn: false }), + ]); + expect(sections[0].items.map(i => i.provider.id)).toEqual(['posit-ai', 'alpha', 'zebra', 'exp']); + }); +}); From 3b4ce6d9080eae6af5e476cc6a7f4e8572d669b5 Mon Sep 17 00:00:00 2001 From: Brice Stacey Date: Wed, 15 Jul 2026 10:15:12 -0400 Subject: [PATCH 2/6] Add provider row for Configure LLM Providers modal See #14817 --- .../components/languageModelButton.tsx | 2 +- .../browser/components/providerListItem.tsx | 77 +++++++++++++++++++ .../test/browser/providerListItem.vitest.tsx | 53 +++++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) create mode 100644 src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx create mode 100644 src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx diff --git a/src/vs/workbench/contrib/positronAssistant/browser/components/languageModelButton.tsx b/src/vs/workbench/contrib/positronAssistant/browser/components/languageModelButton.tsx index d712442803b3..703e660aca85 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/components/languageModelButton.tsx +++ b/src/vs/workbench/contrib/positronAssistant/browser/components/languageModelButton.tsx @@ -31,7 +31,7 @@ interface LanguageModelButtonProps { } /** Human-readable label for a provider's maturity status, or undefined for stable providers. */ -function getStatusLabel(status: LanguageModelButtonProps['status']): string | undefined { +export function getStatusLabel(status: LanguageModelButtonProps['status']): string | undefined { switch (status) { case 'preview': return localize('positron.languageModelButton.status.preview', "Preview"); diff --git a/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx b/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx new file mode 100644 index 000000000000..2de70ace19a3 --- /dev/null +++ b/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx @@ -0,0 +1,77 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import React from 'react'; + +import { localize } from '../../../../../nls.js'; +import { positronClassNames } from '../../../../../base/common/positronUtilities.js'; +import { IPositronLanguageModelSource } from '../../common/interfaces/positronAssistantService.js'; +import { LanguageModelIcon, getStatusLabel } from './languageModelButton.js'; + +interface ProviderListItemProps { + source: IPositronLanguageModelSource; + selected: boolean; + /** When false (e.g. the Needs Attention section), the status line is suppressed. */ + showStatus: boolean; + onSelect: () => void; +} + +/** grey (not connected) / green (ok or signed in) / red (error). */ +function statusDotClass(source: IPositronLanguageModelSource): string { + if (source.status === 'error') { + return 'error'; + } + if (source.signedIn || source.status === 'ok') { + return 'connected'; + } + return 'disconnected'; +} + +/** Prefer the provider-supplied message; fall back to a generic connected/not-connected string. */ +function statusText(source: IPositronLanguageModelSource): string { + if (source.statusMessage) { + return source.statusMessage; + } + return source.signedIn + ? localize('positron.configureLLMProvidersModal.connected', "Connected") + : localize('positron.configureLLMProvidersModal.notConnected', "Not connected"); +} + +/** A single provider row: icon, name, maturity label, and (optionally) connection status. */ +export const ProviderListItem = (props: ProviderListItemProps) => { + const { source, selected, showStatus, onSelect } = props; + const maturityLabel = getStatusLabel(source.provider.status); + + const onKeyDown = (e: React.KeyboardEvent) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + onSelect(); + } + }; + + return ( +
+ +
+
+ {source.provider.displayName} + {maturityLabel && {maturityLabel}} +
+ {showStatus && +
+ + {statusText(source)} +
+ } +
+
+ ); +}; diff --git a/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx b/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx new file mode 100644 index 000000000000..d8662a606308 --- /dev/null +++ b/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx @@ -0,0 +1,53 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/// + +import { userEvent } from '@testing-library/user-event'; +import { render, screen } from '@testing-library/react'; +import { ProviderListItem } from '../../browser/components/providerListItem.js'; +import { IPositronLanguageModelSource, PositronLanguageModelType } from '../../common/interfaces/positronAssistantService.js'; + +function source(overrides: Partial & { id: string }): IPositronLanguageModelSource { + const { id, ...rest } = overrides; + return { + type: PositronLanguageModelType.Chat, + provider: { id, displayName: id, settingName: id }, + supportedOptions: [], + defaults: {}, + ...rest, + } as IPositronLanguageModelSource; +} + +describe('ProviderListItem', () => { + it('renders the display name and a maturity label', () => { + render( { }} />); + expect(screen.getByText('Anthropic')).toBeInTheDocument(); + expect(screen.getByText('Preview')).toBeInTheDocument(); + }); + + it('shows the status message when present and showStatus is true', () => { + render( { }} />); + expect(screen.getByText('Signed in via GitHub')).toBeInTheDocument(); + }); + + it('hides the status line when showStatus is false', () => { + render( { }} />); + expect(screen.queryByText('Session expired')).not.toBeInTheDocument(); + }); + + it('calls onSelect when clicked', async () => { + const user = userEvent.setup(); + const onSelect = vi.fn(); + render(); + await user.click(screen.getByRole('button')); + expect(onSelect).toHaveBeenCalledTimes(1); + }); + + it('marks the row selected', () => { + render( { }} />); + expect(screen.getByRole('button')).toHaveClass('selected'); + }); +}); From 72ad4cc58b20e8dbee8471a217deced6d664a93c Mon Sep 17 00:00:00 2001 From: Brice Stacey Date: Wed, 15 Jul 2026 10:18:11 -0400 Subject: [PATCH 3/6] Add provider list for Configure LLM Providers modal See #14817 --- .../browser/components/providerList.tsx | 100 ++++++++++++++++++ .../test/browser/providerList.vitest.tsx | 66 ++++++++++++ 2 files changed, 166 insertions(+) create mode 100644 src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx create mode 100644 src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx diff --git a/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx b/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx new file mode 100644 index 000000000000..48ad65d40847 --- /dev/null +++ b/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx @@ -0,0 +1,100 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +import { useEffect, useState } from 'react'; + +import { localize } from '../../../../../nls.js'; +import { IDisposable } from '../../../../../base/common/lifecycle.js'; +import { usePositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js'; +import { IPositronAssistantConfigurationService, IPositronLanguageModelSource, IShowLanguageModelConfigOptions } from '../../common/interfaces/positronAssistantService.js'; +import { IAuthenticationService } from '../../../../services/authentication/common/authentication.js'; +import { groupProviders, ProviderSectionId } from '../../common/providerGrouping.js'; +import { syncAuthSessions } from '../languageModelSessionSync.js'; +import { ProviderListItem } from './providerListItem.js'; + +interface ProviderListProps { + sources: IPositronLanguageModelSource[]; + options?: IShowLanguageModelConfigOptions; +} + +/** Localized heading per section id. */ +function sectionTitle(id: ProviderSectionId): string { + switch (id) { + case 'needs-attention': + return localize('positron.configureLLMProvidersModal.section.needsAttention', "Providers needing attention"); + case 'connected': + return localize('positron.configureLLMProvidersModal.section.connected', "Connected"); + case 'custom': + return localize('positron.configureLLMProvidersModal.section.custom', "Custom Providers"); + case 'approved': + return localize('positron.configureLLMProvidersModal.section.approved', "Approved Providers"); + case 'available': + return localize('positron.configureLLMProvidersModal.section.available', "Available Providers"); + } +} + +/** The grouped, sectioned provider list shown in the Configure LLM Providers modal. */ +export const ProviderList = (props: ProviderListProps) => { + const services = usePositronReactServicesContext(); + + // Local copy of sources so live auth/config changes re-render the list. + const [sources, setSources] = useState(props.sources); + const [selectedProviderId, setSelectedProviderId] = useState(props.options?.preselectedProviderId); + + // Re-sync if the caller hands us a new sources array. + useEffect(() => setSources(props.sources), [props.sources]); + + // Provider config changes (sign in / out) while the modal is open. + useEffect(() => { + const configService = services.get(IPositronAssistantConfigurationService); + const disposables: IDisposable[] = []; + disposables.push(configService.onChangeProviderConfig(newSource => { + setSources(prev => prev.map(s => s.provider.id === newSource.provider.id ? newSource : s)); + })); + return () => disposables.forEach(d => d.dispose()); + }, [services]); + + // Auth session changes for API-key providers. + useEffect(() => { + const authService = services.get(IAuthenticationService); + const disposable = syncAuthSessions( + authService, + props.sources.map(s => s.provider.id), + (providerId, signedIn) => { + setSources(prev => { + const index = prev.findIndex(s => s.provider.id === providerId); + if (index === -1) { + return prev; + } + const next = [...prev]; + next[index] = { ...prev[index], signedIn }; + return next; + }); + } + ); + return () => disposable.dispose(); + }, [services, props.sources]); + + const sections = groupProviders(sources); + + return ( +
+ {sections.map(section => ( +
+ + {section.items.map(item => ( + setSelectedProviderId(item.provider.id)} + /> + ))} +
+ ))} +
+ ); +}; diff --git a/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx b/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx new file mode 100644 index 000000000000..9ea9566b8e20 --- /dev/null +++ b/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx @@ -0,0 +1,66 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +/// + +import { screen } from '@testing-library/react'; +import { userEvent } from '@testing-library/user-event'; +import { Event } from '../../../../../base/common/event.js'; +import { setupRTLRenderer } from '../../../../../test/vitest/reactTestingLibrary.js'; +import { createTestContainer } from '../../../../../test/vitest/positronTestContainer.js'; +import { ProviderList } from '../../browser/components/providerList.js'; +import { IPositronAssistantConfigurationService, IPositronLanguageModelSource, PositronLanguageModelType } from '../../common/interfaces/positronAssistantService.js'; +import { IAuthenticationService } from '../../../../services/authentication/common/authentication.js'; + +function source(overrides: Partial & { id: string; displayName?: string }): IPositronLanguageModelSource { + const { id, displayName, ...rest } = overrides; + return { + type: PositronLanguageModelType.Chat, + provider: { id, displayName: displayName ?? id, settingName: id }, + supportedOptions: [], + defaults: {}, + ...rest, + } as IPositronLanguageModelSource; +} + +describe('ProviderList', () => { + const ctx = createTestContainer() + .withReactServices() + .stub(IPositronAssistantConfigurationService, { onChangeProviderConfig: Event.None }) + .stub(IAuthenticationService, { onDidChangeSessions: Event.None }) + .build(); + const rtl = setupRTLRenderer(() => ctx.reactServices); + + it('renders a heading per non-empty section', () => { + rtl.render(); + expect(screen.getByText('Connected')).toBeInTheDocument(); + expect(screen.getByText('Available Providers')).toBeInTheDocument(); + }); + + it('does not render empty section headings', () => { + rtl.render(); + expect(screen.queryByText('Connected')).not.toBeInTheDocument(); + expect(screen.queryByText('Providers needing attention')).not.toBeInTheDocument(); + }); + + it('selects a row on click', async () => { + const user = userEvent.setup(); + rtl.render(); + const row = screen.getByRole('button', { name: /Alpha/ }); + await user.click(row); + expect(row).toHaveClass('selected'); + }); + + it('preselects the provider from options', () => { + rtl.render(); + expect(screen.getByRole('button', { name: /Alpha/ })).toHaveClass('selected'); + }); +}); From 6550e3a053c382b96526d081bbb92223e6d51787 Mon Sep 17 00:00:00 2001 From: Brice Stacey Date: Wed, 15 Jul 2026 10:20:15 -0400 Subject: [PATCH 4/6] Render provider list in Configure LLM Providers modal See #14817 --- .../browser/configureLLMProvidersModal.css | 94 +++++++++++++++++++ .../browser/configureLLMProvidersModal.tsx | 16 ++-- 2 files changed, 104 insertions(+), 6 deletions(-) create mode 100644 src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css diff --git a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css new file mode 100644 index 000000000000..45cf840c4045 --- /dev/null +++ b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css @@ -0,0 +1,94 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (C) 2026 Posit Software, PBC. All rights reserved. + * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. + *--------------------------------------------------------------------------------------------*/ + +.provider-list { + display: flex; + flex-direction: column; + gap: 16px; + overflow-y: auto; +} + +.provider-list-section { + display: flex; + flex-direction: column; + gap: 4px; +} + +.provider-list-section-heading { + font-weight: 600; + margin-bottom: 4px; +} + +.provider-list-item { + display: flex; + align-items: center; + gap: 10px; + padding: 8px; + border-radius: 4px; + cursor: pointer; +} + +.provider-list-item:hover { + background-color: var(--vscode-list-hoverBackground); +} + +.provider-list-item.selected { + background-color: var(--vscode-list-activeSelectionBackground); + color: var(--vscode-list-activeSelectionForeground); +} + +.provider-list-item .language-model.icon { + width: 24px; + height: 24px; + flex-shrink: 0; +} + +.provider-list-item-text { + display: flex; + flex-direction: column; + gap: 2px; + min-width: 0; +} + +.provider-list-item-name { + display: flex; + align-items: center; + gap: 8px; +} + +.provider-list-item-label { + font-size: 11px; + padding: 1px 6px; + border-radius: 8px; + background-color: var(--vscode-badge-background); + color: var(--vscode-badge-foreground); +} + +.provider-list-item-status { + display: flex; + align-items: center; + gap: 6px; + font-size: 12px; + color: var(--vscode-descriptionForeground); +} + +.provider-list-item-status-dot { + width: 8px; + height: 8px; + border-radius: 50%; + flex-shrink: 0; +} + +.provider-list-item-status-dot.connected { + background-color: var(--vscode-testing-iconPassed, #388a34); +} + +.provider-list-item-status-dot.error { + background-color: var(--vscode-testing-iconFailed, #e51400); +} + +.provider-list-item-status-dot.disconnected { + background-color: var(--vscode-descriptionForeground); +} diff --git a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx index cd96a088121d..b62a6136f6b0 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx +++ b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx @@ -3,12 +3,16 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ +// CSS. +import './configureLLMProvidersModal.css'; + // Other dependencies. import { localize } from '../../../../nls.js'; import { IPositronLanguageModelConfig, IPositronLanguageModelSource, IShowLanguageModelConfigOptions } from '../common/interfaces/positronAssistantService.js'; import { OKModalDialog } from '../../../browser/positronComponents/positronModalDialog/positronOKModalDialog.js'; import { PositronModalReactRenderer } from '../../../../base/browser/positronModalReactRenderer.js'; import { VerticalStack } from '../../../browser/positronComponents/positronModalDialog/components/verticalStack.js'; +import { ProviderList } from './components/providerList.js'; /** * Hidden feature switch that selects the new "Configure LLM Providers" modal @@ -32,22 +36,24 @@ export const NEW_PROVIDER_MODAL_KEY = 'assistant.newProviderModal'; * are interchangeable at the single call site that reads the feature switch. */ export const showConfigureLLMProvidersModal = ( - _sources: IPositronLanguageModelSource[], + sources: IPositronLanguageModelSource[], _onAction: (source: IPositronLanguageModelSource, config: IPositronLanguageModelConfig, action: string) => Promise, onClose: () => void, - _options?: IShowLanguageModelConfigOptions, + options?: IShowLanguageModelConfigOptions, ) => { const renderer = new PositronModalReactRenderer(); renderer.render(
- +
); }; interface ConfigureLLMProvidersProps { renderer: PositronModalReactRenderer; + sources: IPositronLanguageModelSource[]; + options?: IShowLanguageModelConfigOptions; onClose: () => void; } @@ -69,9 +75,7 @@ const ConfigureLLMProviders = (props: ConfigureLLMProvidersProps) => { onCancel={onClose} > -

- {localize('positron.configureLLMProvidersModal.placeholder', "This is the new provider configuration experience. It's still being built.")} -

+ ; }; From fd987e2f23847849068db3028aee9d7b418a9433 Mon Sep 17 00:00:00 2001 From: Brice Stacey Date: Wed, 15 Jul 2026 11:54:46 -0400 Subject: [PATCH 5/6] Align provider list with the provider-config design spec Rework the list to match IMPLEMENTATION_SPEC.md / the assistant-providers prototype: Connected -> Needs Attention -> Model Providers sections (alphabetical), badge-based rows (maturity / Environment / OAuth / Error) with per-section action buttons, a hardcoded provider description map, and an always-present Custom Provider section. Admin-managed mode is deferred. See #14817 --- .../browser/components/providerList.tsx | 52 +++++++++--- .../browser/components/providerListItem.tsx | 79 +++++++++++++------ .../browser/configureLLMProvidersModal.css | 76 +++++++++++++----- .../common/providerGrouping.ts | 42 ++++------ .../test/browser/providerList.vitest.tsx | 27 +++++-- .../test/browser/providerListItem.vitest.tsx | 50 ++++++++---- .../test/common/providerGrouping.vitest.ts | 38 ++++++--- 7 files changed, 251 insertions(+), 113 deletions(-) diff --git a/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx b/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx index 48ad65d40847..42e524d9d17b 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx +++ b/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx @@ -19,19 +19,34 @@ interface ProviderListProps { options?: IShowLanguageModelConfigOptions; } +/** + * One-line provider descriptions shown for not-yet-connected providers, keyed by + * provider id. Positron provider metadata does not carry a description yet, so + * this static map mirrors the copy from the provider-configuration design + * prototype. Missing ids simply render no description. + */ +const PROVIDER_DESCRIPTIONS: Record = { + 'amazon-bedrock': localize('positron.configureLLMProvidersModal.desc.bedrock', "Access Claude and other models via AWS"), + 'anthropic-api': localize('positron.configureLLMProvidersModal.desc.anthropic', "Access Claude models directly via Anthropic API"), + 'copilot-auth': localize('positron.configureLLMProvidersModal.desc.copilot', "AI models via GitHub Copilot subscription"), + 'deepseek-api': localize('positron.configureLLMProvidersModal.desc.deepseek', "Access DeepSeek reasoning models"), + 'google': localize('positron.configureLLMProvidersModal.desc.google', "Access Gemini models via Google AI Studio"), + 'google-cloud': localize('positron.configureLLMProvidersModal.desc.googleCloud', "Gemini via Google Cloud with enterprise features"), + 'ms-foundry': localize('positron.configureLLMProvidersModal.desc.msFoundry', "Access Azure OpenAI and AI Studio models"), + 'openai-api': localize('positron.configureLLMProvidersModal.desc.openai', "GPT-4o, o1, and OpenAI-compatible endpoints"), + 'posit-ai': localize('positron.configureLLMProvidersModal.desc.positAI', "Managed model service for Positron Desktop"), + 'snowflake-cortex': localize('positron.configureLLMProvidersModal.desc.snowflake', "Access LLMs via Snowflake data platform"), +}; + /** Localized heading per section id. */ function sectionTitle(id: ProviderSectionId): string { switch (id) { - case 'needs-attention': - return localize('positron.configureLLMProvidersModal.section.needsAttention', "Providers needing attention"); case 'connected': - return localize('positron.configureLLMProvidersModal.section.connected', "Connected"); - case 'custom': - return localize('positron.configureLLMProvidersModal.section.custom', "Custom Providers"); - case 'approved': - return localize('positron.configureLLMProvidersModal.section.approved', "Approved Providers"); - case 'available': - return localize('positron.configureLLMProvidersModal.section.available', "Available Providers"); + return localize('positron.configureLLMProvidersModal.section.connected', "Connected Providers"); + case 'needs-attention': + return localize('positron.configureLLMProvidersModal.section.needsAttention', "Needs Attention"); + case 'model-providers': + return localize('positron.configureLLMProvidersModal.section.modelProviders', "Model Providers"); } } @@ -87,14 +102,31 @@ export const ProviderList = (props: ProviderListProps) => { {section.items.map(item => ( setSelectedProviderId(item.provider.id)} /> ))} ))} + +
+ +

+ {localize('positron.configureLLMProvidersModal.customDescription', "Works with any OpenAI-compatible API endpoint that uses the /v1/chat/completions endpoint for chat.")} +

+ {/* Placeholder until the custom-provider flow lands (see #14818). */} + +
); }; diff --git a/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx b/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx index 2de70ace19a3..8c92cd68752a 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx +++ b/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx @@ -7,42 +7,55 @@ import React from 'react'; import { localize } from '../../../../../nls.js'; import { positronClassNames } from '../../../../../base/common/positronUtilities.js'; -import { IPositronLanguageModelSource } from '../../common/interfaces/positronAssistantService.js'; +import { IPositronLanguageModelSource, LanguageModelAutoconfigureType } from '../../common/interfaces/positronAssistantService.js'; +import { ProviderSectionId } from '../../common/providerGrouping.js'; +import { AuthMethod } from '../types.js'; import { LanguageModelIcon, getStatusLabel } from './languageModelButton.js'; interface ProviderListItemProps { source: IPositronLanguageModelSource; + /** Which section the row is rendered in; drives badges and the action label. */ + section: ProviderSectionId; selected: boolean; - /** When false (e.g. the Needs Attention section), the status line is suppressed. */ - showStatus: boolean; + /** One-line description shown for not-yet-connected providers. */ + description?: string; + /** Selects the row. The connect/manage flows (see #14818/#14819) hang off selection. */ onSelect: () => void; } -/** grey (not connected) / green (ok or signed in) / red (error). */ -function statusDotClass(source: IPositronLanguageModelSource): string { - if (source.status === 'error') { - return 'error'; +/** How a connected provider authenticated, shown as a badge. */ +function authBadgeLabel(source: IPositronLanguageModelSource): string | undefined { + const autoconfigure = source.defaults.autoconfigure; + if (autoconfigure?.type === LanguageModelAutoconfigureType.EnvVariable && autoconfigure.signedIn) { + return localize('positron.configureLLMProvidersModal.badge.environment', "Environment"); } - if (source.signedIn || source.status === 'ok') { - return 'connected'; + if (source.supportedOptions.includes(AuthMethod.OAUTH)) { + return localize('positron.configureLLMProvidersModal.badge.oauth', "OAuth"); } - return 'disconnected'; + return undefined; } -/** Prefer the provider-supplied message; fall back to a generic connected/not-connected string. */ -function statusText(source: IPositronLanguageModelSource): string { - if (source.statusMessage) { - return source.statusMessage; +/** The per-section action button label. */ +function actionLabel(section: ProviderSectionId): string { + switch (section) { + case 'connected': + return localize('positron.configureLLMProvidersModal.action.edit', "Edit"); + case 'needs-attention': + return localize('positron.configureLLMProvidersModal.action.fix', "Fix Connection"); + case 'model-providers': + return localize('positron.configureLLMProvidersModal.action.connect', "Connect"); } - return source.signedIn - ? localize('positron.configureLLMProvidersModal.connected', "Connected") - : localize('positron.configureLLMProvidersModal.notConnected', "Not connected"); } -/** A single provider row: icon, name, maturity label, and (optionally) connection status. */ +/** + * A single provider row: icon, name, status/maturity badges, and a per-section + * action button. The whole row is selectable; the action button is a + * placeholder that selects the row until the connect/manage flows land. + */ export const ProviderListItem = (props: ProviderListItemProps) => { - const { source, selected, showStatus, onSelect } = props; + const { source, section, selected, description, onSelect } = props; const maturityLabel = getStatusLabel(source.provider.status); + const authLabel = section === 'connected' ? authBadgeLabel(source) : undefined; const onKeyDown = (e: React.KeyboardEvent) => { if (e.key === 'Enter' || e.key === ' ') { @@ -53,6 +66,7 @@ export const ProviderListItem = (props: ProviderListItemProps) => { return (
{
{source.provider.displayName} - {maturityLabel && {maturityLabel}} + {maturityLabel && {maturityLabel}} + {authLabel && {authLabel}} + {section === 'needs-attention' && + + {localize('positron.configureLLMProvidersModal.badge.error', "Error")} + + }
- {showStatus && -
- - {statusText(source)} -
+ {section === 'needs-attention' && source.statusMessage && +
{source.statusMessage}
} + {section === 'model-providers' && description && +
{description}
+ } +
+
+
); diff --git a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css index 45cf840c4045..efe701323447 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css +++ b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css @@ -6,7 +6,7 @@ .provider-list { display: flex; flex-direction: column; - gap: 16px; + gap: 20px; overflow-y: auto; } @@ -17,10 +17,18 @@ } .provider-list-section-heading { + display: flex; + align-items: center; + gap: 8px; + font-size: 11px; font-weight: 600; + text-transform: uppercase; + letter-spacing: 0.5px; + color: var(--vscode-descriptionForeground); margin-bottom: 4px; } +/* Provider row */ .provider-list-item { display: flex; align-items: center; @@ -28,12 +36,18 @@ padding: 8px; border-radius: 4px; cursor: pointer; + text-align: left; } .provider-list-item:hover { background-color: var(--vscode-list-hoverBackground); } +.provider-list-item:focus-visible { + outline: 1px solid var(--vscode-focusBorder); + outline-offset: -1px; +} + .provider-list-item.selected { background-color: var(--vscode-list-activeSelectionBackground); color: var(--vscode-list-activeSelectionForeground); @@ -50,6 +64,7 @@ flex-direction: column; gap: 2px; min-width: 0; + flex: 1; } .provider-list-item-name { @@ -58,37 +73,60 @@ gap: 8px; } -.provider-list-item-label { +.provider-list-item-display-name { + font-weight: 600; +} + +.provider-list-item-desc, +.provider-list-custom-desc { + font-size: 12px; + color: var(--vscode-descriptionForeground); +} + +.provider-list-item-error { + font-size: 12px; + color: var(--vscode-errorForeground); +} + +/* Badges */ +.provider-list-item-badge { font-size: 11px; - padding: 1px 6px; + line-height: 1.4; + padding: 0 6px; border-radius: 8px; background-color: var(--vscode-badge-background); color: var(--vscode-badge-foreground); + white-space: nowrap; } -.provider-list-item-status { - display: flex; - align-items: center; - gap: 6px; - font-size: 12px; - color: var(--vscode-descriptionForeground); +.provider-list-item-badge.error { + background-color: transparent; + color: var(--vscode-errorForeground); + border: 1px solid var(--vscode-errorForeground); } -.provider-list-item-status-dot { - width: 8px; - height: 8px; - border-radius: 50%; +/* Action buttons */ +.provider-list-item-actions { flex-shrink: 0; } -.provider-list-item-status-dot.connected { - background-color: var(--vscode-testing-iconPassed, #388a34); +.provider-list-item-action, +.provider-list-add-custom { + padding: 3px 10px; + border-radius: 4px; + border: 1px solid var(--vscode-button-border, var(--vscode-contrastBorder, transparent)); + background-color: var(--vscode-button-secondaryBackground); + color: var(--vscode-button-secondaryForeground); + cursor: pointer; + font-size: 12px; } -.provider-list-item-status-dot.error { - background-color: var(--vscode-testing-iconFailed, #e51400); +.provider-list-item-action:hover, +.provider-list-add-custom:hover { + background-color: var(--vscode-button-secondaryHoverBackground); } -.provider-list-item-status-dot.disconnected { - background-color: var(--vscode-descriptionForeground); +.provider-list-add-custom { + align-self: flex-start; + margin-top: 4px; } diff --git a/src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts b/src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts index e97e3afafb27..144cd52549ee 100644 --- a/src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts +++ b/src/vs/workbench/contrib/positronAssistant/common/providerGrouping.ts @@ -5,8 +5,8 @@ import { IPositronLanguageModelSource } from './interfaces/positronAssistantService.js'; -/** Section identifiers, in fixed display order. */ -export type ProviderSectionId = 'needs-attention' | 'connected' | 'custom' | 'approved' | 'available'; +/** Section identifiers for the built-in provider groups, in fixed display order. */ +export type ProviderSectionId = 'connected' | 'needs-attention' | 'model-providers'; /** A non-empty group of providers to render under one heading. */ export interface ProviderSection { @@ -14,10 +14,20 @@ export interface ProviderSection { items: IPositronLanguageModelSource[]; } -const SECTION_ORDER: ProviderSectionId[] = ['needs-attention', 'connected', 'custom', 'approved', 'available']; +const SECTION_ORDER: ProviderSectionId[] = ['connected', 'needs-attention', 'model-providers']; + +/** + * The OpenAI-compatible "Custom Provider" template. It has its own dedicated + * section in the modal (with an "Add custom provider" affordance), so it is + * excluded from the built-in provider groups. + */ +export const CUSTOM_PROVIDER_ID = 'openai-compatible'; /** Only chat providers (and the copilot-auth completion provider) are shown, mirroring the legacy modal. */ function isDisplayable(source: IPositronLanguageModelSource): boolean { + if (source.provider.id === CUSTOM_PROVIDER_ID) { + return false; + } return source.type === 'chat' || (source.type === 'completion' && source.provider.id === 'copilot-auth'); } @@ -29,36 +39,18 @@ function sectionFor(source: IPositronLanguageModelSource): ProviderSectionId { if (source.signedIn) { return 'connected'; } - return 'available'; -} - -/** Sort rank within a section: Posit AI first, then stable, preview, experimental. */ -function sortRank(source: IPositronLanguageModelSource): number { - if (source.provider.id === 'posit-ai') { - return 0; - } - switch (source.provider.status) { - case 'preview': - return 2; - case 'experimental': - return 3; - default: - return 1; - } + return 'model-providers'; } function compareSources(a: IPositronLanguageModelSource, b: IPositronLanguageModelSource): number { - const rankDiff = sortRank(a) - sortRank(b); - if (rankDiff !== 0) { - return rankDiff; - } return a.provider.displayName.localeCompare(b.provider.displayName); } /** * Groups language model sources into ordered, non-empty sections for the - * Configure LLM Providers modal. Custom and Approved sections have no backing - * data yet and will simply be absent until sources land in those buckets. + * Configure LLM Providers modal: Connected, then Needs Attention, then Model + * Providers. Within a section, providers are sorted alphabetically by display + * name. The custom-provider template is handled by a separate section. */ export function groupProviders(sources: IPositronLanguageModelSource[]): ProviderSection[] { const buckets = new Map(); diff --git a/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx b/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx index 9ea9566b8e20..eec38f10d4e6 100644 --- a/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx +++ b/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx @@ -35,23 +35,34 @@ describe('ProviderList', () => { it('renders a heading per non-empty section', () => { rtl.render(); - expect(screen.getByText('Connected')).toBeInTheDocument(); - expect(screen.getByText('Available Providers')).toBeInTheDocument(); + expect(screen.getByText('Connected Providers')).toBeInTheDocument(); + expect(screen.getByText('Model Providers')).toBeInTheDocument(); }); - it('does not render empty section headings', () => { + it('does not render empty built-in section headings', () => { rtl.render(); - expect(screen.queryByText('Connected')).not.toBeInTheDocument(); - expect(screen.queryByText('Providers needing attention')).not.toBeInTheDocument(); + expect(screen.queryByText('Connected Providers')).not.toBeInTheDocument(); + expect(screen.queryByText('Needs Attention')).not.toBeInTheDocument(); + }); + + it('always renders the Custom Provider section with an add button', () => { + rtl.render(); + expect(screen.getByText('Custom Provider')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Add custom provider/ })).toBeInTheDocument(); + }); + + it('shows the built-in description for a known provider', () => { + rtl.render(); + expect(screen.getByText('Access Claude models directly via Anthropic API')).toBeInTheDocument(); }); it('selects a row on click', async () => { const user = userEvent.setup(); rtl.render(); - const row = screen.getByRole('button', { name: /Alpha/ }); + const row = screen.getByRole('button', { name: 'Alpha' }); await user.click(row); expect(row).toHaveClass('selected'); }); @@ -61,6 +72,6 @@ describe('ProviderList', () => { source({ id: 'a', displayName: 'Alpha', signedIn: false }), source({ id: 'b', displayName: 'Bravo', signedIn: false }), ]} />); - expect(screen.getByRole('button', { name: /Alpha/ })).toHaveClass('selected'); + expect(screen.getByRole('button', { name: 'Alpha' })).toHaveClass('selected'); }); }); diff --git a/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx b/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx index d8662a606308..5400b21e4884 100644 --- a/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx +++ b/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx @@ -8,7 +8,8 @@ import { userEvent } from '@testing-library/user-event'; import { render, screen } from '@testing-library/react'; import { ProviderListItem } from '../../browser/components/providerListItem.js'; -import { IPositronLanguageModelSource, PositronLanguageModelType } from '../../common/interfaces/positronAssistantService.js'; +import { AuthMethod } from '../../browser/types.js'; +import { IPositronLanguageModelSource, LanguageModelAutoconfigureType, PositronLanguageModelType } from '../../common/interfaces/positronAssistantService.js'; function source(overrides: Partial & { id: string }): IPositronLanguageModelSource { const { id, ...rest } = overrides; @@ -22,32 +23,51 @@ function source(overrides: Partial & { id: string } describe('ProviderListItem', () => { - it('renders the display name and a maturity label', () => { - render( { }} />); + it('renders the display name and a maturity badge', () => { + render( { }} />); expect(screen.getByText('Anthropic')).toBeInTheDocument(); expect(screen.getByText('Preview')).toBeInTheDocument(); }); - it('shows the status message when present and showStatus is true', () => { - render( { }} />); - expect(screen.getByText('Signed in via GitHub')).toBeInTheDocument(); + it('shows a description and a Connect action in the model-providers section', () => { + render( { }} />); + expect(screen.getByText('Access Claude models')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Connect' })).toBeInTheDocument(); }); - it('hides the status line when showStatus is false', () => { - render( { }} />); - expect(screen.queryByText('Session expired')).not.toBeInTheDocument(); + it('shows an Environment badge and Edit action for an env-var connected provider', () => { + render( { }} />); + expect(screen.getByText('Environment')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Edit' })).toBeInTheDocument(); }); - it('calls onSelect when clicked', async () => { + it('shows an OAuth badge for an oauth connected provider', () => { + render( { }} />); + expect(screen.getByText('OAuth')).toBeInTheDocument(); + }); + + it('shows an Error badge, the error message, and a Fix Connection action in needs-attention', () => { + render( { }} />); + expect(screen.getByText('Error')).toBeInTheDocument(); + expect(screen.getByText('Session expired')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Fix Connection' })).toBeInTheDocument(); + }); + + it('calls onSelect when the row is clicked', async () => { const user = userEvent.setup(); const onSelect = vi.fn(); - render(); - await user.click(screen.getByRole('button')); - expect(onSelect).toHaveBeenCalledTimes(1); + render(); + // The row exposes its display name as an aria-label so it is queryable separately from its action button. + await user.click(screen.getByRole('button', { name: 'Alpha' })); + expect(onSelect).toHaveBeenCalled(); }); it('marks the row selected', () => { - render( { }} />); - expect(screen.getByRole('button')).toHaveClass('selected'); + render( { }} />); + expect(screen.getByRole('button', { name: 'Alpha' })).toHaveClass('selected'); }); }); diff --git a/src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts b/src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts index 29e7489402ff..834a423ad0e5 100644 --- a/src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts +++ b/src/vs/workbench/contrib/positronAssistant/test/common/providerGrouping.vitest.ts @@ -20,24 +20,32 @@ function source(overrides: Partial & { id: string } describe('groupProviders', () => { - it('buckets signed-in error sources into needs-attention', () => { + it('orders sections connected, needs-attention, model-providers', () => { + const sections = groupProviders([ + source({ id: 'avail', signedIn: false }), + source({ id: 'err', signedIn: true, status: 'error' }), + source({ id: 'conn', signedIn: true, status: 'ok' }), + ]); + expect(sections.map(s => s.id)).toEqual(['connected', 'needs-attention', 'model-providers']); + }); + + it('buckets a signed-in error source into needs-attention', () => { const sections = groupProviders([source({ id: 'a', signedIn: true, status: 'error' })]); expect(sections).toHaveLength(1); expect(sections[0].id).toBe('needs-attention'); - expect(sections[0].items.map(i => i.provider.id)).toEqual(['a']); }); - it('buckets signed-in non-error into connected and signed-out into available', () => { + it('buckets a signed-in non-error source into connected and signed-out into model-providers', () => { const sections = groupProviders([ source({ id: 'a', signedIn: true, status: 'ok' }), source({ id: 'b', signedIn: false }), ]); - expect(sections.map(s => s.id)).toEqual(['connected', 'available']); + expect(sections.map(s => s.id)).toEqual(['connected', 'model-providers']); }); it('omits empty sections', () => { const sections = groupProviders([source({ id: 'b', signedIn: false })]); - expect(sections.map(s => s.id)).toEqual(['available']); + expect(sections.map(s => s.id)).toEqual(['model-providers']); }); it('filters out non-chat sources except copilot-auth completion', () => { @@ -49,13 +57,21 @@ describe('groupProviders', () => { expect(sections[0].items.map(i => i.provider.id)).toEqual(['copilot-auth']); }); - it('sorts posit-ai first, then stable, preview, experimental, then alphabetical', () => { + it('excludes the custom-provider template (openai-compatible) from the built-in sections', () => { + const sections = groupProviders([ + source({ id: 'openai-compatible', signedIn: false }), + source({ id: 'openai-api', signedIn: false }), + ]); + expect(sections).toHaveLength(1); + expect(sections[0].items.map(i => i.provider.id)).toEqual(['openai-api']); + }); + + it('sorts alphabetically by display name within a section', () => { const sections = groupProviders([ - source({ id: 'zebra', signedIn: false }), - source({ id: 'exp', signedIn: false, provider: { id: 'exp', displayName: 'exp', settingName: 'exp', status: 'experimental' } }), - source({ id: 'posit-ai', signedIn: false }), - source({ id: 'alpha', signedIn: false }), + source({ id: 'zebra', provider: { id: 'zebra', displayName: 'Zebra', settingName: 'zebra' }, signedIn: false }), + source({ id: 'posit-ai', provider: { id: 'posit-ai', displayName: 'Posit AI', settingName: 'positAI' }, signedIn: false }), + source({ id: 'alpha', provider: { id: 'alpha', displayName: 'Alpha', settingName: 'alpha' }, signedIn: false }), ]); - expect(sections[0].items.map(i => i.provider.id)).toEqual(['posit-ai', 'alpha', 'zebra', 'exp']); + expect(sections[0].items.map(i => i.provider.displayName)).toEqual(['Alpha', 'Posit AI', 'Zebra']); }); }); From e95c7f38f938f1bbd88ac303e05b71791bcf28df Mon Sep 17 00:00:00 2001 From: Brice Stacey Date: Wed, 15 Jul 2026 13:14:05 -0400 Subject: [PATCH 6/6] Make provider rows non-interactive; refine icon and Connect button - Remove row selection; the per-section action button is the only interactive element (connect/manage flows hang off it, see #14818/#14819). - Wrap each provider icon in a 36x36 rounded square (6px radius, subtle border) per the design. - Add an inline plus icon to the Connect and Add custom provider buttons. See #14817 --- .../browser/components/providerList.tsx | 7 +--- .../browser/components/providerListItem.tsx | 42 ++++++------------- .../browser/configureLLMProvidersModal.css | 38 ++++++++++------- .../browser/configureLLMProvidersModal.tsx | 7 ++-- .../test/browser/providerList.vitest.tsx | 17 -------- .../test/browser/providerListItem.vitest.tsx | 28 +++++-------- 6 files changed, 50 insertions(+), 89 deletions(-) diff --git a/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx b/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx index 42e524d9d17b..536057ca3b81 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx +++ b/src/vs/workbench/contrib/positronAssistant/browser/components/providerList.tsx @@ -8,7 +8,7 @@ import { useEffect, useState } from 'react'; import { localize } from '../../../../../nls.js'; import { IDisposable } from '../../../../../base/common/lifecycle.js'; import { usePositronReactServicesContext } from '../../../../../base/browser/positronReactRendererContext.js'; -import { IPositronAssistantConfigurationService, IPositronLanguageModelSource, IShowLanguageModelConfigOptions } from '../../common/interfaces/positronAssistantService.js'; +import { IPositronAssistantConfigurationService, IPositronLanguageModelSource } from '../../common/interfaces/positronAssistantService.js'; import { IAuthenticationService } from '../../../../services/authentication/common/authentication.js'; import { groupProviders, ProviderSectionId } from '../../common/providerGrouping.js'; import { syncAuthSessions } from '../languageModelSessionSync.js'; @@ -16,7 +16,6 @@ import { ProviderListItem } from './providerListItem.js'; interface ProviderListProps { sources: IPositronLanguageModelSource[]; - options?: IShowLanguageModelConfigOptions; } /** @@ -56,7 +55,6 @@ export const ProviderList = (props: ProviderListProps) => { // Local copy of sources so live auth/config changes re-render the list. const [sources, setSources] = useState(props.sources); - const [selectedProviderId, setSelectedProviderId] = useState(props.options?.preselectedProviderId); // Re-sync if the caller hands us a new sources array. useEffect(() => setSources(props.sources), [props.sources]); @@ -104,9 +102,7 @@ export const ProviderList = (props: ProviderListProps) => { key={item.provider.id} description={PROVIDER_DESCRIPTIONS[item.provider.id]} section={section.id} - selected={item.provider.id === selectedProviderId} source={item} - onSelect={() => setSelectedProviderId(item.provider.id)} /> ))} @@ -124,6 +120,7 @@ export const ProviderList = (props: ProviderListProps) => {

{/* Placeholder until the custom-provider flow lands (see #14818). */} diff --git a/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx b/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx index 8c92cd68752a..0f83b6c982e2 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx +++ b/src/vs/workbench/contrib/positronAssistant/browser/components/providerListItem.tsx @@ -3,8 +3,6 @@ * Licensed under the Elastic License 2.0. See LICENSE.txt for license information. *--------------------------------------------------------------------------------------------*/ -import React from 'react'; - import { localize } from '../../../../../nls.js'; import { positronClassNames } from '../../../../../base/common/positronUtilities.js'; import { IPositronLanguageModelSource, LanguageModelAutoconfigureType } from '../../common/interfaces/positronAssistantService.js'; @@ -16,11 +14,10 @@ interface ProviderListItemProps { source: IPositronLanguageModelSource; /** Which section the row is rendered in; drives badges and the action label. */ section: ProviderSectionId; - selected: boolean; /** One-line description shown for not-yet-connected providers. */ description?: string; - /** Selects the row. The connect/manage flows (see #14818/#14819) hang off selection. */ - onSelect: () => void; + /** Invoked by the row's action button. The connect/manage flows (see #14818/#14819) hang off this. */ + onAction?: () => void; } /** How a connected provider authenticated, shown as a badge. */ @@ -48,32 +45,20 @@ function actionLabel(section: ProviderSectionId): string { } /** - * A single provider row: icon, name, status/maturity badges, and a per-section - * action button. The whole row is selectable; the action button is a - * placeholder that selects the row until the connect/manage flows land. + * A single provider row: a rounded-square provider icon, name, status/maturity + * badges, and a per-section action button (the only interactive element - the + * row itself is not clickable). */ export const ProviderListItem = (props: ProviderListItemProps) => { - const { source, section, selected, description, onSelect } = props; + const { source, section, description, onAction } = props; const maturityLabel = getStatusLabel(source.provider.status); const authLabel = section === 'connected' ? authBadgeLabel(source) : undefined; - const onKeyDown = (e: React.KeyboardEvent) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - onSelect(); - } - }; - return ( -
- +
+
+ +
{source.provider.displayName} @@ -93,11 +78,8 @@ export const ProviderListItem = (props: ProviderListItemProps) => { }
-
diff --git a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css index efe701323447..e80064bd8f07 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css +++ b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.css @@ -35,27 +35,25 @@ gap: 10px; padding: 8px; border-radius: 4px; - cursor: pointer; text-align: left; } -.provider-list-item:hover { - background-color: var(--vscode-list-hoverBackground); -} - -.provider-list-item:focus-visible { - outline: 1px solid var(--vscode-focusBorder); - outline-offset: -1px; -} - -.provider-list-item.selected { - background-color: var(--vscode-list-activeSelectionBackground); - color: var(--vscode-list-activeSelectionForeground); +/* Rounded-square provider avatar (36x36, 6px radius per the design). */ +.provider-list-item-icon { + display: flex; + align-items: center; + justify-content: center; + width: 36px; + height: 36px; + border-radius: 6px; + border: 1px solid var(--vscode-widget-border, rgba(128, 128, 128, 0.2)); + background-color: var(--vscode-editorWidget-background); + flex-shrink: 0; } -.provider-list-item .language-model.icon { - width: 24px; - height: 24px; +.provider-list-item-icon .language-model.icon { + width: 22px; + height: 22px; flex-shrink: 0; } @@ -112,6 +110,9 @@ .provider-list-item-action, .provider-list-add-custom { + display: inline-flex; + align-items: center; + gap: 4px; padding: 3px 10px; border-radius: 4px; border: 1px solid var(--vscode-button-border, var(--vscode-contrastBorder, transparent)); @@ -121,6 +122,11 @@ font-size: 12px; } +.provider-list-item-action .codicon, +.provider-list-add-custom .codicon { + font-size: 14px; +} + .provider-list-item-action:hover, .provider-list-add-custom:hover { background-color: var(--vscode-button-secondaryHoverBackground); diff --git a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx index b62a6136f6b0..655db91259b5 100644 --- a/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx +++ b/src/vs/workbench/contrib/positronAssistant/browser/configureLLMProvidersModal.tsx @@ -39,13 +39,13 @@ export const showConfigureLLMProvidersModal = ( sources: IPositronLanguageModelSource[], _onAction: (source: IPositronLanguageModelSource, config: IPositronLanguageModelConfig, action: string) => Promise, onClose: () => void, - options?: IShowLanguageModelConfigOptions, + _options?: IShowLanguageModelConfigOptions, ) => { const renderer = new PositronModalReactRenderer(); renderer.render(
- +
); }; @@ -53,7 +53,6 @@ export const showConfigureLLMProvidersModal = ( interface ConfigureLLMProvidersProps { renderer: PositronModalReactRenderer; sources: IPositronLanguageModelSource[]; - options?: IShowLanguageModelConfigOptions; onClose: () => void; } @@ -75,7 +74,7 @@ const ConfigureLLMProviders = (props: ConfigureLLMProvidersProps) => { onCancel={onClose} > - + ; }; diff --git a/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx b/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx index eec38f10d4e6..855911bb8c95 100644 --- a/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx +++ b/src/vs/workbench/contrib/positronAssistant/test/browser/providerList.vitest.tsx @@ -6,7 +6,6 @@ /// import { screen } from '@testing-library/react'; -import { userEvent } from '@testing-library/user-event'; import { Event } from '../../../../../base/common/event.js'; import { setupRTLRenderer } from '../../../../../test/vitest/reactTestingLibrary.js'; import { createTestContainer } from '../../../../../test/vitest/positronTestContainer.js'; @@ -58,20 +57,4 @@ describe('ProviderList', () => { rtl.render(); expect(screen.getByText('Access Claude models directly via Anthropic API')).toBeInTheDocument(); }); - - it('selects a row on click', async () => { - const user = userEvent.setup(); - rtl.render(); - const row = screen.getByRole('button', { name: 'Alpha' }); - await user.click(row); - expect(row).toHaveClass('selected'); - }); - - it('preselects the provider from options', () => { - rtl.render(); - expect(screen.getByRole('button', { name: 'Alpha' })).toHaveClass('selected'); - }); }); diff --git a/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx b/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx index 5400b21e4884..a02cbbf30095 100644 --- a/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx +++ b/src/vs/workbench/contrib/positronAssistant/test/browser/providerListItem.vitest.tsx @@ -24,50 +24,44 @@ function source(overrides: Partial & { id: string describe('ProviderListItem', () => { it('renders the display name and a maturity badge', () => { - render( { }} />); + render(); expect(screen.getByText('Anthropic')).toBeInTheDocument(); expect(screen.getByText('Preview')).toBeInTheDocument(); }); it('shows a description and a Connect action in the model-providers section', () => { - render( { }} />); + render(); expect(screen.getByText('Access Claude models')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Connect' })).toBeInTheDocument(); }); it('shows an Environment badge and Edit action for an env-var connected provider', () => { - render( { }} />); + })} />); expect(screen.getByText('Environment')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Edit' })).toBeInTheDocument(); }); it('shows an OAuth badge for an oauth connected provider', () => { - render( { }} />); + render(); expect(screen.getByText('OAuth')).toBeInTheDocument(); }); it('shows an Error badge, the error message, and a Fix Connection action in needs-attention', () => { - render( { }} />); + render(); expect(screen.getByText('Error')).toBeInTheDocument(); expect(screen.getByText('Session expired')).toBeInTheDocument(); expect(screen.getByRole('button', { name: 'Fix Connection' })).toBeInTheDocument(); }); - it('calls onSelect when the row is clicked', async () => { + it('calls onAction when the action button is clicked', async () => { const user = userEvent.setup(); - const onSelect = vi.fn(); - render(); - // The row exposes its display name as an aria-label so it is queryable separately from its action button. - await user.click(screen.getByRole('button', { name: 'Alpha' })); - expect(onSelect).toHaveBeenCalled(); - }); - - it('marks the row selected', () => { - render( { }} />); - expect(screen.getByRole('button', { name: 'Alpha' })).toHaveClass('selected'); + const onAction = vi.fn(); + render(); + await user.click(screen.getByRole('button', { name: 'Connect' })); + expect(onAction).toHaveBeenCalledTimes(1); }); });