Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(webview): relocate model selector (CODY-5194) #7322

Merged
merged 28 commits into from
Mar 5, 2025
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
cf1e105
feat(chat): move intent selection to toolbar
abeatrix Mar 1, 2025
71d62e7
flipe model and mode
abeatrix Mar 1, 2025
af52d42
refactor(webviews): move model selector to tab bar
abeatrix Mar 1, 2025
1c3d077
fix ui unit test
abeatrix Mar 1, 2025
cbc78a7
Merge branch 'bee/mode-selector' into bee/move-model
abeatrix Mar 1, 2025
d228ee8
update css
julialeex Mar 1, 2025
4e97263
address PR comments
julialeex Mar 3, 2025
fd63bcd
attempt to add shortcut (wip)
julialeex Mar 3, 2025
36510bd
code cleanup
julialeex Mar 3, 2025
72a2bf0
fix tests
julialeex Mar 4, 2025
6569d12
Update vscode/src/commands/index.ts
julialeex Mar 4, 2025
b2a2b1e
Update vscode/src/chat/protocol.ts
julialeex Mar 4, 2025
b9c170c
Merge branch 'main' into bee/move-model
abeatrix Mar 4, 2025
f9b8dfc
Merge branch 'bee/move-model' of https://github.com/sourcegraph/cody …
abeatrix Mar 4, 2025
3056b5e
address PR comments
julialeex Mar 4, 2025
e8e110e
re-implement keyboard shortcut
julialeex Mar 4, 2025
9bc8dc5
Update lib/shared/src/misc/rpc/webviewAPI.ts
julialeex Mar 4, 2025
4b7d457
add cmd+m
julialeex Mar 4, 2025
5fd0c8f
enable model selection via keyboard shortcut and refactor ModelSelect…
abeatrix Mar 4, 2025
948856b
show the model selector only when we are on chat view + fix tests
julialeex Mar 4, 2025
daf3687
update icons
julialeex Mar 4, 2025
ea52b67
add model selector to chat editor
julialeex Mar 4, 2025
b19916d
fix bug
julialeex Mar 4, 2025
00ed8f0
update cmd+n
julialeex Mar 4, 2025
1045d37
small fixes
julialeex Mar 5, 2025
bfead3a
padding
julialeex Mar 5, 2025
706fb0e
Update vscode/webviews/tabs/TabsBar.tsx
julialeex Mar 5, 2025
7facec6
Update vscode/webviews/components/shadcn/ui/toolbar.tsx
julialeex Mar 5, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion vscode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -600,14 +600,15 @@
"key": "alt+l",
"when": "cody.activated"
},

{
"command": "cody.mention.selection",
"key": "alt+l",
"when": "cody.activated && editorTextFocus && editorHasSelection"
},
{
"command": "cody.chat.new",
"key": "shift+alt+/",
"key": "cmd+n",
"when": "cody.activated && !editorTextFocus"
},
{
Expand Down
4 changes: 2 additions & 2 deletions vscode/test/e2e/auth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ test.extend<ExpectedV2Events>({
await expect(sidebar!.getByText('Invalid access token.')).not.toBeVisible()
await expect(sidebar!.getByText('Sign in to Sourcegraph')).not.toBeVisible()
await expect(sidebar!.getByLabel('Chat message')).toBeVisible()
await expect(sidebar!.getByRole('button', { name: 'New Chat' })).toBeVisible()
await expect(sidebar!.getByTestId('tab-chat')).toBeVisible()

// Sign out.
await signOut(page)
Expand Down Expand Up @@ -149,7 +149,7 @@ test.extend<ExpectedV2Events>({
await expect(sidebar!.getByLabel('Chat message')).toBeVisible()

// Open the User Dropdown menu
await expect(sidebar!.getByRole('button', { name: 'New Chat' })).toBeVisible()
await expect(sidebar!.getByTestId('tab-chat')).toBeVisible()
await sidebar!.getByLabel('Account Menu Button').click({ delay: 2000 })

const codeWebview = sidebar!.getByLabel('cody-webview')
Expand Down
8 changes: 7 additions & 1 deletion vscode/test/e2e/chat-input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,10 @@ test.extend<DotcomUrlOverride>({ dotcomUrl: mockServer.SERVER_URL }).extend<Expe
const chatFrame = getChatSidebarPanel(page)
const firstChatInput = getChatInputs(chatFrame).first()

const modelSelect = chatFrame.getByRole('combobox', { name: 'Select a model' }).last()
const chatTab = chatFrame.getByTestId('tab-chat')
await chatTab.click()

const modelSelect = chatFrame.getByTestId('chat-model-selector')

await expect(modelSelect).toBeEnabled()
await expect(modelSelect).toHaveText(/^Claude 3.5 Sonnet/)
Expand All @@ -215,7 +218,10 @@ test.extend<DotcomUrlOverride>({ dotcomUrl: mockServer.SERVER_URL }).extend<Expe
await modelSelect.click()
const modelChoices = chatFrame.getByRole('listbox', { name: 'Suggestions' })
await modelChoices.getByRole('option', { name: 'Claude 3 Haiku' }).click()

const lastChatInput = getChatInputs(chatFrame).last()
await lastChatInput.click()

await expect(lastChatInput).toBeFocused()
await expect(modelSelect).toHaveText(/^Claude 3 Haiku/)
await lastChatInput.fill('to model2')
Expand Down
4 changes: 2 additions & 2 deletions vscode/test/e2e/enterprise-server-sent-models.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
await sidebarSignin(page, sidebar, { enableNotifications: true })
// Open chat.
const [chatFrame] = await createEmptyChatPanel(page)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@abeatrix just realized that if we open a new empty chat panel we can't see the model selector. However do we plan to remove the empty chat panel option?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could just add the tab bar to the editor chat panel to show the model dropdown?

let modelSelect = chatFrame.getByRole('combobox', { name: 'Select a model' }).last()
let modelSelect = chatFrame.getByTestId('chat-model-selector')

// First model in the server list should be selected as default
await expect(modelSelect).toBeEnabled()

Check failure on line 18 in vscode/test/e2e/enterprise-server-sent-models.test.ts

View workflow job for this annotation

GitHub Actions / test-e2e (ubuntu, 5/5)

enterprise-server-sent-models.test.ts:6:5 › allows multiple enterprise models when server-sent models is enabled

1) enterprise-server-sent-models.test.ts:6:5 › allows multiple enterprise models when server-sent models is enabled Error: Timed out 5000ms waiting for expect(locator).toBeEnabled() Locator: frameLocator('.simple-find-part-wrapper + iframe.webview').last().frameLocator('iframe').getByTestId('chat-model-selector') Expected: enabled Received: <element(s) not found> Call log: - expect.toBeEnabled with timeout 5000ms - waiting for frameLocator('.simple-find-part-wrapper + iframe.webview').last().frameLocator('iframe').getByTestId('chat-model-selector') 16 | 17 | // First model in the server list should be selected as default > 18 | await expect(modelSelect).toBeEnabled() | ^ 19 | await expect(modelSelect).toHaveText(/^Opus/) 20 | 21 | // Change selection to Titan and assert it was updated at /home/runner/work/cody/cody/vscode/test/e2e/enterprise-server-sent-models.test.ts:18:31

Check failure on line 18 in vscode/test/e2e/enterprise-server-sent-models.test.ts

View workflow job for this annotation

GitHub Actions / test-e2e (ubuntu, 5/5)

enterprise-server-sent-models.test.ts:6:5 › allows multiple enterprise models when server-sent models is enabled

1) enterprise-server-sent-models.test.ts:6:5 › allows multiple enterprise models when server-sent models is enabled Retry #1 ─────────────────────────────────────────────────────────────────────────────────────── Error: Timed out 5000ms waiting for expect(locator).toBeEnabled() Locator: frameLocator('.simple-find-part-wrapper + iframe.webview').last().frameLocator('iframe').getByTestId('chat-model-selector') Expected: enabled Received: <element(s) not found> Call log: - expect.toBeEnabled with timeout 5000ms - waiting for frameLocator('.simple-find-part-wrapper + iframe.webview').last().frameLocator('iframe').getByTestId('chat-model-selector') 16 | 17 | // First model in the server list should be selected as default > 18 | await expect(modelSelect).toBeEnabled() | ^ 19 | await expect(modelSelect).toHaveText(/^Opus/) 20 | 21 | // Change selection to Titan and assert it was updated at /home/runner/work/cody/cody/vscode/test/e2e/enterprise-server-sent-models.test.ts:18:31
await expect(modelSelect).toHaveText(/^Opus/)

// Change selection to Titan and assert it was updated
Expand All @@ -31,7 +31,7 @@
await chatTab.getByRole('button', { name: /^Close/ }).click()

const [newChatFrame] = await createEmptyChatPanel(page)
modelSelect = newChatFrame.getByRole('combobox', { name: 'Select a model' }).last()
modelSelect = newChatFrame.getByTestId('chat-model-selector')

// First model in the server list should be selected as default
await expect(modelSelect).toBeEnabled()
Expand Down
1 change: 1 addition & 0 deletions vscode/webviews/CodyPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ export const CodyPanel: FunctionComponent<CodyPanelProps> = ({
{/* Hide tab bar in editor chat panels. */}
{config.webviewType !== 'editor' && (
<TabsBar
models={chatModels}
user={user}
currentView={view}
setView={setView}
Expand Down
28 changes: 0 additions & 28 deletions vscode/webviews/chat/Transcript.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,34 +35,6 @@ describe('Transcript', () => {
expectCells([{ message: '' }])
})

test('renders with provided models', () => {
const { container } = render(
<Transcript
{...PROPS}
transcript={[
{ speaker: 'human', text: ps`Hello` },
{ speaker: 'assistant', text: ps`Hi` },
]}
/>
)

// Check if the model selector is rendered
const modelSelector = container.querySelector('[data-testid="chat-model-selector"]')
expect(modelSelector).not.toBeNull()
expect(modelSelector?.textContent).toEqual(FIXTURE_MODELS[0].title)

// Open the menu on click
fireEvent.click(modelSelector!)
const modelPopover = container?.querySelectorAll('[data-testid="chat-model-popover"]')[0]
const modelMenu = modelPopover!.querySelector('[data-testid="chat-model-popover-option"]')
const modelOptions = modelMenu!.querySelectorAll('[data-testid="chat-model-popover-option"]')
expect(modelOptions).toHaveLength(FIXTURE_MODELS.length)

// Check if the model titles are correct
const modelTitles = Array.from(modelOptions!).map(option => option.textContent)
expect(modelTitles.some(title => title === FIXTURE_MODELS[0].title)).toBe(true)
})

test('interaction without context', () => {
render(
<Transcript
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,9 @@ describe('HumanMessageEditor', () => {

test('model selector is showing up with the default model name', () => {
const { container } = renderWithMocks({})
const modelSelector = container.querySelector('[data-testid="chat-model-selector"]')
const tabsBar = container.querySelector('[class*="tabsRoot"]')
const modelSelector = tabsBar?.querySelector('[data-testid="chat-model-selector"]')
expect(modelSelector).not.toBeNull()
expect(modelSelector?.textContent).toEqual(FIXTURE_MODELS[0].title)
})
})
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,9 @@ import {
import clsx from 'clsx'
import { type FunctionComponent, useCallback, useMemo } from 'react'
import type { UserAccountInfo } from '../../../../../../Chat'
import { ModelSelectField } from '../../../../../../components/modelSelectField/ModelSelectField'
import { PromptSelectField } from '../../../../../../components/promptSelectField/PromptSelectField'
import toolbarStyles from '../../../../../../components/shadcn/ui/toolbar.module.css'
import { useActionSelect } from '../../../../../../prompts/PromptsTab'
import { useClientConfig } from '../../../../../../utils/useClientConfig'
import { MediaUploadButton } from './MediaUploadButton'
import { ModeSelectorField } from './ModeSelectorButton'
import { SubmitButton, type SubmitButtonState } from './SubmitButton'
Expand Down Expand Up @@ -117,13 +115,6 @@ export const Toolbar: FunctionComponent<{
/>
)}
<PromptSelectFieldToolbarItem focusEditor={focusEditor} className="tw-ml-1 tw-mr-1" />
<ModelSelectFieldToolbarItem
models={models}
userInfo={userInfo}
focusEditor={focusEditor}
className="tw-mr-1"
extensionAPI={extensionAPI}
/>
{!userInfo?.isDotComUser && omniBoxEnabled && (
<ModeSelectorField
className={className}
Expand Down Expand Up @@ -157,39 +148,3 @@ const PromptSelectFieldToolbarItem: FunctionComponent<{

return <PromptSelectField onSelect={onSelect} onCloseByEscape={focusEditor} className={className} />
}

const ModelSelectFieldToolbarItem: FunctionComponent<{
models: Model[]
userInfo: UserAccountInfo
focusEditor?: () => void
className?: string
extensionAPI: WebviewToExtensionAPI
}> = ({ userInfo, focusEditor, className, models, extensionAPI }) => {
const clientConfig = useClientConfig()
const serverSentModelsEnabled = !!clientConfig?.modelsAPIEnabled

const onModelSelect = useCallback(
(model: Model) => {
extensionAPI.setChatModel(model.id).subscribe({
error: error => console.error('setChatModel:', error),
})
focusEditor?.()
},
[extensionAPI.setChatModel, focusEditor]
)

return (
!!models?.length &&
(userInfo.isDotComUser || serverSentModelsEnabled) && (
<ModelSelectField
models={models}
onModelSelect={onModelSelect}
serverSentModelsEnabled={serverSentModelsEnabled}
userInfo={userInfo}
onCloseByEscape={focusEditor}
className={className}
data-testid="chat-model-selector"
/>
)
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
}

button > .model-title-with-icon .model-name {
font-weight: normal;
font-weight: 600;
}

button > .model-title-with-icon .model-icon,
Expand Down
13 changes: 11 additions & 2 deletions vscode/webviews/components/modelSelectField/ModelSelectField.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ export const ModelSelectField: React.FunctionComponent<{

/** For storybooks only. */
__storybook__open?: boolean

modelSelectorRef?: React.RefObject<{ open: () => void; close: () => void }>
}> = ({
models,
onModelSelect: parentOnModelSelect,
Expand All @@ -44,6 +46,7 @@ export const ModelSelectField: React.FunctionComponent<{
onCloseByEscape,
className,
__storybook__open,
modelSelectorRef,
}) => {
const telemetryRecorder = useTelemetryRecorder()

Expand Down Expand Up @@ -149,8 +152,11 @@ export const ModelSelectField: React.FunctionComponent<{

const onKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLDivElement>) => {
event.preventDefault()
event.stopPropagation()
if (event.key === 'Escape') {
onCloseByEscape?.()
return
}
},
[onCloseByEscape]
Expand All @@ -167,10 +173,12 @@ export const ModelSelectField: React.FunctionComponent<{
data-testid="chat-model-selector"
iconEnd={readOnly ? undefined : 'chevron'}
className={cn('tw-justify-between', className)}
defaultOpen={false}
disabled={readOnly}
__storybook__open={__storybook__open}
tooltip={readOnly ? undefined : 'Select a model'}
tooltip={readOnly ? undefined : 'Switch model (⌘+m)'}
aria-label="Select a model or an agent"
ref={modelSelectorRef}
popoverContent={close => (
<Command
loop={true}
Expand Down Expand Up @@ -265,7 +273,7 @@ export const ModelSelectField: React.FunctionComponent<{
popoverRootProps={{ onOpenChange }}
popoverContentProps={{
className: 'tw-min-w-[325px] tw-w-[unset] tw-max-w-[90%] !tw-p-0',
onKeyDown: onKeyDown,
onKeyDown,
onCloseAutoFocus: event => {
// Prevent the popover trigger from stealing focus after the user selects an
// item. We want the focus to return to the editor.
Expand All @@ -278,6 +286,7 @@ export const ModelSelectField: React.FunctionComponent<{
)
}

ModelSelectField.displayName = 'ModelSelectField'
const ENTERPRISE_MODEL_DOCS_PAGE =
'https://sourcegraph.com/docs/cody/clients/enable-cody-enterprise?utm_source=cody.modelSelector'

Expand Down
59 changes: 35 additions & 24 deletions vscode/webviews/components/shadcn/ui/toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import { ChevronDownIcon } from 'lucide-react'
import {
type ButtonHTMLAttributes,
type ComponentType,
type FunctionComponent,
type KeyboardEventHandler,
type PropsWithChildren,
type ReactNode,
forwardRef,
useCallback,
useEffect,
useImperativeHandle,
useRef,
useState,
} from 'react'
Expand Down Expand Up @@ -42,7 +42,7 @@ interface ToolbarButtonProps
VariantProps<typeof buttonVariants> {
tooltip?: ReactNode
iconStart?: IconComponent
iconEnd?: IconComponent | 'chevron'
iconEnd?: IconComponent | 'chevron' | null

asChild?: boolean
}
Expand Down Expand Up @@ -92,36 +92,33 @@ export const ToolbarButton = forwardRef<HTMLButtonElement, ToolbarButtonProps>(
)
ToolbarButton.displayName = 'ToolbarButton'

export const ToolbarPopoverItem: FunctionComponent<
export const ToolbarPopoverItem = forwardRef<
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QQ: why do we need to update this to forwardRef? It seems like it still work as FunctionComponent?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it still work as FunctionComponent?

What do you mean

why do we need to update this to forwardRef

I need it for the open: () => void

{ open: () => void },
PropsWithChildren<
ButtonHTMLAttributes<HTMLButtonElement> &
Pick<ToolbarButtonProps, 'iconStart' | 'tooltip'> & {
iconEnd: ToolbarButtonProps['iconEnd'] | null
Omit<Pick<ToolbarButtonProps, 'iconStart' | 'tooltip'>, 'iconEnd'> & {
iconEnd: ToolbarButtonProps['iconEnd'] | undefined
popoverContent: (close: () => void) => React.ReactNode

defaultOpen?: boolean

onCloseByEscape?: () => void

popoverRootProps?: Pick<PopoverProps, 'onOpenChange'>
popoverContentProps?: Omit<PopoverContentProps, 'align'>

/** For storybooks only. */
__storybook__open?: boolean
}
>
> = ({
iconEnd = 'chevron',
popoverContent,
defaultOpen,
onCloseByEscape,
popoverRootProps,
popoverContentProps,
__storybook__open,
children,
...props
}) => {
>((props, ref) => {
const {
iconEnd = 'chevron',
popoverContent,
defaultOpen,
onCloseByEscape,
popoverRootProps,
popoverContentProps,
__storybook__open,
children,
} = props
const [isOpen, setIsOpen] = useState(defaultOpen)

const onButtonClick = useCallback(() => {
setIsOpen(isOpen => !isOpen)
}, [])
Expand All @@ -132,6 +129,16 @@ export const ToolbarPopoverItem: FunctionComponent<
}
}, [__storybook__open])

// Expose the open method via ref
useImperativeHandle(
ref,
() => ({
open: () => setIsOpen(true),
close: () => setIsOpen(false),
}),
[]
)

const popoverContentRef = useRef<HTMLDivElement>(null)

const onOpenChange = useCallback(
Expand Down Expand Up @@ -171,15 +178,18 @@ export const ToolbarPopoverItem: FunctionComponent<
[onCloseByEscape, popoverContentProps?.onKeyDown]
)

const finalIconEnd = iconEnd === null ? undefined : iconEnd
const { iconEnd: _, ...restProps } = props

return (
<Popover open={isOpen} onOpenChange={onOpenChange} defaultOpen={defaultOpen}>
<PopoverTrigger asChild={true}>
<ToolbarButton
variant="secondary"
iconEnd={iconEnd ?? undefined}
iconEnd={finalIconEnd}
ref={anchorRef}
onClick={onButtonClick}
{...props}
{...restProps}
>
{children}
</ToolbarButton>
Expand All @@ -194,4 +204,5 @@ export const ToolbarPopoverItem: FunctionComponent<
</PopoverContent>
</Popover>
)
}
})
ToolbarPopoverItem.displayName = 'ToolbarPopoverItem'
Loading
Loading