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

Fix Cody Chat UI rendering reconciliation #5850

Merged
merged 2 commits into from
Oct 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion vscode/src/chat/chat-view/ChatController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ export class ChatController implements vscode.Disposable, vscode.WebviewViewProv
.then(async response => {
signal.throwIfAborted()
this.chatBuilder.setLastMessageIntent(response?.intent)
this.postViewTranscript()
this.postEmptyMessageInProgress(model)
return response
})
.catch(() => undefined)
Expand Down
31 changes: 21 additions & 10 deletions vscode/webviews/chat/Transcript.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { clsx } from 'clsx'
import debounce from 'lodash/debounce'
import isEqual from 'lodash/isEqual'
import { Search } from 'lucide-react'
import { type FC, memo, useCallback, useMemo, useRef, useState } from 'react'
import { type FC, type MutableRefObject, memo, useCallback, useMemo, useRef } from 'react'
import type { UserAccountInfo } from '../Chat'
import type { ApiPostMessage } from '../Chat'
import { Button } from '../components/shadcn/ui/button'
Expand Down Expand Up @@ -180,7 +180,7 @@ const TranscriptInteraction: FC<TranscriptInteractionProps> = memo(props => {
smartApplyEnabled,
} = props

const [intentResults, setIntentResults] = useState<
const [intentResults, setIntentResults] = useMutatedValue<
Copy link
Contributor

Choose a reason for hiding this comment

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

Why isn't simply useRef enough here?

| { intent: ChatMessage['intent']; allScores?: { intent: string; score: number }[] }
| undefined
| null
Expand All @@ -193,28 +193,27 @@ const TranscriptInteraction: FC<TranscriptInteractionProps> = memo(props => {
editHumanMessage({
messageIndexInTranscript: humanMessage.index,
editorValue,
intent: intentFromSubmit || intentResults?.intent,
intentScores: intentFromSubmit ? undefined : intentResults?.allScores,
intent: intentFromSubmit || intentResults.current?.intent,
intentScores: intentFromSubmit ? undefined : intentResults.current?.allScores,
manuallySelectedIntent: !!intentFromSubmit,
})
},
[humanMessage, intentResults]
[humanMessage.index, intentResults]
)

const onFollowupSubmit = useCallback(
(editorValue: SerializedPromptEditorValue, intentFromSubmit?: ChatMessage['intent']): void => {
submitHumanMessage({
editorValue,
intent: intentFromSubmit || intentResults?.intent,
intentScores: intentFromSubmit ? undefined : intentResults?.allScores,
intent: intentFromSubmit || intentResults.current?.intent,
intentScores: intentFromSubmit ? undefined : intentResults.current?.allScores,
manuallySelectedIntent: !!intentFromSubmit,
})
},
[intentResults]
)

const extensionAPI = useExtensionAPI()

const experimentalOneBoxEnabled = useExperimentalOneBox()

const onChange = useMemo(() => {
Expand All @@ -240,7 +239,7 @@ const TranscriptInteraction: FC<TranscriptInteractionProps> = memo(props => {
})
}
}, 300)
}, [experimentalOneBoxEnabled, extensionAPI])
}, [experimentalOneBoxEnabled, extensionAPI, setIntentResults])

const onStop = useCallback(() => {
getVSCodeAPI().postMessage({
Expand Down Expand Up @@ -290,7 +289,7 @@ const TranscriptInteraction: FC<TranscriptInteractionProps> = memo(props => {
[reSubmitWithIntent]
)

const resetIntent = useCallback(() => setIntentResults(undefined), [])
const resetIntent = useCallback(() => setIntentResults(undefined), [setIntentResults])

return (
<>
Expand All @@ -312,6 +311,7 @@ const TranscriptInteraction: FC<TranscriptInteractionProps> = memo(props => {
className={!isFirstInteraction && isLastInteraction ? 'tw-mt-auto' : ''}
onEditorFocusChange={resetIntent}
/>

{experimentalOneBoxEnabled && humanMessage.intent && (
<InfoMessage>
{humanMessage.intent === 'search' ? (
Expand Down Expand Up @@ -389,6 +389,17 @@ const TranscriptInteraction: FC<TranscriptInteractionProps> = memo(props => {
)
}, isEqual)

function useMutatedValue<T>(value?: T): [MutableRefObject<T | undefined>, setValue: (value: T) => void] {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should document that this should not be used for rendering and won't cause re-rendering.

const valueRef = useRef<T | undefined>(value)

return [
valueRef,
useCallback(value => {
valueRef.current = value
}, []),
]
}

// TODO(sqs): Do this the React-y way.
export function focusLastHumanMessageEditor(): void {
const elements = document.querySelectorAll<HTMLElement>('[data-lexical-editor]')
Expand Down
115 changes: 63 additions & 52 deletions vscode/webviews/chat/cells/messageCell/human/HumanMessageCell.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import {
type ChatMessage,
type SerializedPromptEditorState,
type SerializedPromptEditorValue,
serializedPromptEditorStateFromChatMessage,
} from '@sourcegraph/cody-shared'
import type { PromptEditorRefAPI } from '@sourcegraph/prompt-editor'
import isEqual from 'lodash/isEqual'
import { ColumnsIcon } from 'lucide-react'
import { type FunctionComponent, memo, useMemo } from 'react'
import { type FC, memo, useMemo } from 'react'
import type { UserAccountInfo } from '../../../../Chat'
import { UserAvatar } from '../../../../components/UserAvatar'
import { BaseMessageCell, MESSAGE_CELL_AVATAR_SIZE } from '../BaseMessageCell'
Expand All @@ -16,10 +17,7 @@ import { Tooltip, TooltipContent, TooltipTrigger } from '../../../../components/
import { getVSCodeAPI } from '../../../../utils/VSCodeApi'
import { useConfig } from '../../../../utils/useConfig'

/**
* A component that displays a chat message from the human.
*/
export const HumanMessageCell: FunctionComponent<{
interface HumanMessageCellProps {
message: ChatMessage
userInfo: UserAccountInfo
chatEnabled: boolean
Expand Down Expand Up @@ -47,9 +45,29 @@ export const HumanMessageCell: FunctionComponent<{

/** For use in storybooks only. */
__storybook__focus?: boolean
}> = memo(
({
message,
}

/**
* A component that displays a chat message from the human.
*/
export const HumanMessageCell: FC<HumanMessageCellProps> = ({ message, ...otherProps }) => {
const messageJSON = JSON.stringify(message)

const initialEditorState = useMemo(
() => serializedPromptEditorStateFromChatMessage(JSON.parse(messageJSON)),
[messageJSON]
)

return <HumanMessageCellContent {...otherProps} initialEditorState={initialEditorState} />
}

type HumanMessageCellContent = { initialEditorState: SerializedPromptEditorState } & Omit<
HumanMessageCellProps,
'message'
>
const HumanMessageCellContent = memo<HumanMessageCellContent>(props => {
const {
initialEditorState,
userInfo,
chatEnabled = true,
isFirstMessage,
Expand All @@ -65,50 +83,43 @@ export const HumanMessageCell: FunctionComponent<{
editorRef,
__storybook__focus,
onEditorFocusChange,
}) => {
const messageJSON = JSON.stringify(message)
const initialEditorState = useMemo(
() => serializedPromptEditorStateFromChatMessage(JSON.parse(messageJSON)),
[messageJSON]
)

return (
<BaseMessageCell
speakerIcon={
<UserAvatar
user={userInfo.user}
size={MESSAGE_CELL_AVATAR_SIZE}
sourcegraphGradientBorder={true}
/>
}
speakerTitle={userInfo.user.displayName ?? userInfo.user.username}
cellAction={isFirstMessage && <OpenInNewEditorAction />}
content={
<HumanMessageEditor
userInfo={userInfo}
initialEditorState={initialEditorState}
placeholder={isFirstMessage ? 'Ask...' : 'Ask a followup...'}
isFirstMessage={isFirstMessage}
isSent={isSent}
isPendingPriorResponse={isPendingPriorResponse}
onChange={onChange}
onSubmit={onSubmit}
onStop={onStop}
disabled={!chatEnabled}
isFirstInteraction={isFirstInteraction}
isLastInteraction={isLastInteraction}
isEditorInitiallyFocused={isEditorInitiallyFocused}
editorRef={editorRef}
__storybook__focus={__storybook__focus}
onEditorFocusChange={onEditorFocusChange}
/>
}
className={className}
/>
)
},
isEqual
)
} = props

return (
<BaseMessageCell
speakerIcon={
<UserAvatar
user={userInfo.user}
size={MESSAGE_CELL_AVATAR_SIZE}
sourcegraphGradientBorder={true}
/>
}
speakerTitle={userInfo.user.displayName ?? userInfo.user.username}
cellAction={isFirstMessage && <OpenInNewEditorAction />}
content={
<HumanMessageEditor
userInfo={userInfo}
initialEditorState={initialEditorState}
placeholder={isFirstMessage ? 'Ask...' : 'Ask a followup...'}
isFirstMessage={isFirstMessage}
isSent={isSent}
isPendingPriorResponse={isPendingPriorResponse}
onChange={onChange}
onSubmit={onSubmit}
onStop={onStop}
disabled={!chatEnabled}
isFirstInteraction={isFirstInteraction}
isLastInteraction={isLastInteraction}
isEditorInitiallyFocused={isEditorInitiallyFocused}
editorRef={editorRef}
__storybook__focus={__storybook__focus}
onEditorFocusChange={onEditorFocusChange}
/>
}
className={className}
/>
)
}, isEqual)

const OpenInNewEditorAction = () => {
const {
Expand Down
8 changes: 5 additions & 3 deletions vscode/webviews/tabs/TabsBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ import { getVSCodeAPI } from '../utils/VSCodeApi'
import { View } from './types'

import { CodyIDE, FeatureFlag, isDefined } from '@sourcegraph/cody-shared'
import { type FC, Fragment, forwardRef, useCallback, useMemo, useState } from 'react'
import { type FC, Fragment, forwardRef, memo, useCallback, useMemo, useState } from 'react'
import { Kbd } from '../components/Kbd'
import { Tooltip, TooltipContent, TooltipTrigger } from '../components/shadcn/ui/tooltip'
import { useConfig } from '../utils/useConfig'

import { useExtensionAPI } from '@sourcegraph/prompt-editor'
import { isEqual } from 'lodash'
import { downloadChatHistory } from '../chat/downloadChatHistory'
import { Button } from '../components/shadcn/ui/button'
import { useFeatureFlag } from '../utils/useFeatureFlags'
Expand Down Expand Up @@ -68,7 +69,8 @@ interface TabConfig {
subActions?: TabSubAction[]
}

export const TabsBar: React.FC<TabsBarProps> = ({ currentView, setView, IDE }) => {
export const TabsBar = memo<TabsBarProps>(props => {
const { currentView, setView, IDE } = props
const tabItems = useTabs({ IDE })
const {
config: { webviewType, multipleWebviewsEnabled },
Expand Down Expand Up @@ -183,7 +185,7 @@ export const TabsBar: React.FC<TabsBarProps> = ({ currentView, setView, IDE }) =
</Tabs.List>
</div>
)
}
}, isEqual)

interface ActionButtonWithConfirmationProps {
title: string
Expand Down
23 changes: 23 additions & 0 deletions vscode/webviews/utils/debugger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { useEffect, useRef } from 'react'

export function useTracePropsUpdate(props: any) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a comment here that this is for debugging with advice about how to use it.

From time to team people do mass-deletions based on use. You probably intend for this to stick around.

const prev = useRef(props)

useEffect(() => {
const changedProps = Object.entries(props).reduce(
(prop, [k, v]) => {
if (prev.current[k] !== v) {
prop[k] = [prev.current[k], v]
}
return prop
},
{} as Record<any, any>
)

if (Object.keys(changedProps).length > 0) {
console.log('Changed props:', changedProps)
}

prev.current = props
})
}
25 changes: 15 additions & 10 deletions vscode/webviews/utils/useConfig.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
type ReactNode,
createContext,
useContext,
useMemo,
} from 'react'
import type { ExtensionMessage } from '../../src/chat/protocol'
import type { UserAccountInfo } from '../Chat'
Expand Down Expand Up @@ -43,18 +44,22 @@ export function useConfig(): Config {
}

export function useUserAccountInfo(): UserAccountInfo {
const value = useConfig()
if (!value.authStatus.authenticated) {
const { authStatus, isDotComUser, clientCapabilities, userProductSubscription } = useConfig()

if (!authStatus.authenticated) {
throw new Error(
'useUserAccountInfo must be used within a ConfigProvider with authenticated user'
)
}
return {
isCodyProUser: isCodyProUser(value.authStatus, value.userProductSubscription ?? null),
// Receive this value from the extension backend to make it work
// with E2E tests where change the DOTCOM_URL via the env variable TESTING_DOTCOM_URL.
isDotComUser: value.isDotComUser,
user: value.authStatus,
IDE: value.clientCapabilities.agentIDE,
}
return useMemo<UserAccountInfo>(
() => ({
isCodyProUser: isCodyProUser(authStatus, userProductSubscription ?? null),
// Receive this value from the extension backend to make it work
// with E2E tests where change the DOTCOM_URL via the env variable TESTING_DOTCOM_URL.
isDotComUser: isDotComUser,
user: authStatus,
IDE: clientCapabilities.agentIDE,
}),
[authStatus, isDotComUser, clientCapabilities, userProductSubscription]
)
}
Loading