Skip to content

Commit

Permalink
Cody Web: Add high-level memoization for chat/transcript UI (#5036)
Browse files Browse the repository at this point in the history
Attempt to fix
https://linear.app/sourcegraph/issue/SRCH-724/prinova-found-an-issue-on-cody-web

The problem that some customers have is somewhere in the react rendering
pipeline, I couldn't reproduce this problem from the issue on my machine
but I noticed that even though we have `useCallback` and `useMemo` hooks
in high-level components we still don't use memoization on components
level for anything, this means that the whole chate (all messages) would
be re-rendered if one of messages has been changed (by re-rendering I
mean react reconciliation).

This PR 
- Removes props drilling from high-level components (it's easy to track
dependencies graph when we don't spread the whole props object to each
component)
- Adds `memo()` wrappers for heavy UI components such as transcript
interactions and its children UI like human input, assistant message,
..etc)

Note: I chained this PR with un-related to this PR telemetry fix just to
avoid double package versions in NPM (I will merge them in sequence as
they will be reviewed)

## Test plan
- Standard high-level manual checks (changes in this PR don't change
anything with the current feature set, standard e2e checks should be
enough to catch possible regressions)
  • Loading branch information
vovakulikov authored Jul 29, 2024
1 parent 8d11278 commit 284ab72
Show file tree
Hide file tree
Showing 8 changed files with 271 additions and 212 deletions.
12 changes: 8 additions & 4 deletions vscode/webviews/Chat.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { clsx } from 'clsx'
import type React from 'react'
import { useCallback, useEffect, useMemo } from 'react'
import { useCallback, useEffect, useMemo, useRef } from 'react'

import type { AuthStatus, ChatMessage, CodyIDE, Guardrails } from '@sourcegraph/cody-shared'
import { Transcript, focusLastHumanMessageEditor } from './chat/Transcript'
Expand Down Expand Up @@ -48,6 +48,10 @@ export const Chat: React.FunctionComponent<React.PropsWithChildren<ChatboxProps>
}) => {
const { reload: reloadMentionProviders } = useChatContextMentionProviders()
const telemetryRecorder = useTelemetryRecorder()

const transcriptRef = useRef(transcript)
transcriptRef.current = transcript

const feedbackButtonsOnSubmit = useCallback(
(text: string) => {
enum FeedbackType {
Expand All @@ -57,7 +61,7 @@ export const Chat: React.FunctionComponent<React.PropsWithChildren<ChatboxProps>
telemetryRecorder.recordEvent('cody.feedback', 'submit', {
metadata: {
feedbackType: text === 'thumbsUp' ? FeedbackType.thumbsUp : FeedbackType.thumbsDown,
lastChatUsedEmbeddings: transcript
lastChatUsedEmbeddings: transcriptRef.current
.at(-1)
?.contextFiles?.some(file => file.source === 'embeddings')
? 1
Expand All @@ -71,12 +75,12 @@ export const Chat: React.FunctionComponent<React.PropsWithChildren<ChatboxProps>
// V2 telemetry exports privateMetadata only for DotCom users
// the condition below is an aditional safegaurd measure
responseText: userInfo.isDotComUser
? truncateTextStart(transcript.toString(), CHAT_INPUT_TOKEN_BUDGET)
? truncateTextStart(transcriptRef.current.toString(), CHAT_INPUT_TOKEN_BUDGET)
: '',
},
})
},
[transcript, userInfo, telemetryRecorder]
[userInfo, telemetryRecorder]
)

const copyButtonOnSubmit = useCallback(
Expand Down
111 changes: 72 additions & 39 deletions vscode/webviews/chat/Transcript.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,8 @@ import {
isAbortErrorOrSocketHangUp,
} from '@sourcegraph/cody-shared'
import type { PromptEditorRefAPI } from '@sourcegraph/prompt-editor'
import {
type ComponentProps,
type FunctionComponent,
useCallback,
useEffect,
useMemo,
useRef,
} from 'react'
import isEqual from 'lodash/isEqual'
import { type FC, memo, useCallback, useEffect, useMemo, useRef } from 'react'
import type { UserAccountInfo } from '../Chat'
import type { ApiPostMessage } from '../Chat'
import { getVSCodeAPI } from '../utils/VSCodeApi'
Expand All @@ -26,19 +20,37 @@ import {
} from './cells/messageCell/assistant/AssistantMessageCell'
import { HumanMessageCell } from './cells/messageCell/human/HumanMessageCell'

export const Transcript: React.FunctionComponent<{
interface TranscriptProps {
chatID: string
chatEnabled: boolean
transcript: ChatMessage[]
userInfo: UserAccountInfo
messageInProgress: ChatMessage | null

guardrails?: Guardrails
postMessage?: ApiPostMessage
isTranscriptError?: boolean

feedbackButtonsOnSubmit: (text: string) => void
copyButtonOnSubmit: CodeBlockActionsProps['copyButtonOnSubmit']
insertButtonOnSubmit?: CodeBlockActionsProps['insertButtonOnSubmit']
isTranscriptError?: boolean
userInfo: UserAccountInfo
chatEnabled: boolean
postMessage?: ApiPostMessage
guardrails?: Guardrails
}> = ({ chatID, transcript, messageInProgress, ...props }) => {
}

export const Transcript: FC<TranscriptProps> = props => {
const {
chatID,
chatEnabled,
transcript,
userInfo,
messageInProgress,
guardrails,
postMessage,
isTranscriptError,
feedbackButtonsOnSubmit,
copyButtonOnSubmit,
insertButtonOnSubmit,
} = props

const interactions = useMemo(
() => transcriptToInteractionPairs(transcript, messageInProgress),
[transcript, messageInProgress]
Expand All @@ -48,13 +60,18 @@ export const Transcript: React.FunctionComponent<{
<div className="tw-px-8 tw-py-6 tw-pt-8 tw-mt-2 tw-flex tw-flex-col tw-gap-10">
{interactions.map((interaction, i) => (
<TranscriptInteraction
chatID={chatID}
// biome-ignore lint/suspicious/noArrayIndexKey: <explanation>
key={`${chatID}-${i}`}
{...props}
transcript={transcript}
messageInProgress={messageInProgress}
chatID={chatID}
chatEnabled={chatEnabled}
userInfo={userInfo}
interaction={interaction}
guardrails={guardrails}
postMessage={postMessage}
isTranscriptError={isTranscriptError}
feedbackButtonsOnSubmit={feedbackButtonsOnSubmit}
copyButtonOnSubmit={copyButtonOnSubmit}
insertButtonOnSubmit={insertButtonOnSubmit}
isFirstInteraction={i === 0}
isLastInteraction={i === interactions.length - 1}
isLastSentInteraction={
Expand Down Expand Up @@ -124,24 +141,33 @@ export function transcriptToInteractionPairs(
return pairs
}

const TranscriptInteraction: FunctionComponent<
ComponentProps<typeof Transcript> & {
interaction: Interaction
isFirstInteraction: boolean
isLastInteraction: boolean
isLastSentInteraction: boolean
priorAssistantMessageIsLoading: boolean
}
> = ({
interaction: { humanMessage, assistantMessage },
isFirstInteraction,
isLastInteraction,
isLastSentInteraction,
priorAssistantMessageIsLoading,
isTranscriptError,
...props
}) => {
interface TranscriptInteractionProps extends Omit<TranscriptProps, 'transcript' | 'messageInProgress'> {
interaction: Interaction
isFirstInteraction: boolean
isLastInteraction: boolean
isLastSentInteraction: boolean
priorAssistantMessageIsLoading: boolean
}

const TranscriptInteraction: FC<TranscriptInteractionProps> = memo(props => {
const {
interaction: { humanMessage, assistantMessage },
isFirstInteraction,
isLastInteraction,
isLastSentInteraction,
priorAssistantMessageIsLoading,
isTranscriptError,
userInfo,
chatEnabled,
feedbackButtonsOnSubmit,
postMessage,
guardrails,
insertButtonOnSubmit,
copyButtonOnSubmit,
} = props

const humanEditorRef = useRef<PromptEditorRefAPI | null>(null)

useEffect(() => {
return getVSCodeAPI().onMessage(message => {
if (message.type === 'updateEditorState') {
Expand Down Expand Up @@ -174,8 +200,9 @@ const TranscriptInteraction: FunctionComponent<
return (
<>
<HumanMessageCell
{...props}
key={humanMessage.index}
userInfo={userInfo}
chatEnabled={chatEnabled}
message={humanMessage}
isFirstMessage={humanMessage.index === 0}
isSent={!humanMessage.isUnsentFollowup}
Expand All @@ -199,8 +226,14 @@ const TranscriptInteraction: FunctionComponent<
{assistantMessage && !isContextLoading && (
<AssistantMessageCell
key={assistantMessage.index}
{...props}
userInfo={userInfo}
chatEnabled={chatEnabled}
message={assistantMessage}
feedbackButtonsOnSubmit={feedbackButtonsOnSubmit}
copyButtonOnSubmit={copyButtonOnSubmit}
insertButtonOnSubmit={insertButtonOnSubmit}
postMessage={postMessage}
guardrails={guardrails}
humanMessage={makeHumanMessageInfo(
{ humanMessage, assistantMessage },
humanEditorRef
Expand All @@ -216,7 +249,7 @@ const TranscriptInteraction: FunctionComponent<
)}
</>
)
}
}, isEqual)

// TODO(sqs): Do this the React-y way.
export function focusLastHumanMessageEditor(): void {
Expand Down
9 changes: 5 additions & 4 deletions vscode/webviews/chat/cells/contextCell/ContextCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ import type { ContextItem, Model } from '@sourcegraph/cody-shared'
import { pluralize } from '@sourcegraph/cody-shared'
import { MENTION_CLASS_NAME } from '@sourcegraph/prompt-editor'
import { clsx } from 'clsx'
import isEqual from 'lodash/isEqual'
import { BrainIcon, MessagesSquareIcon } from 'lucide-react'
import type React from 'react'
import { type FunctionComponent, memo } from 'react'
import { FileLink } from '../../../components/FileLink'
import {
Accordion,
Expand All @@ -22,15 +23,15 @@ import styles from './ContextCell.module.css'
/**
* A component displaying the context for a human message.
*/
export const ContextCell: React.FunctionComponent<{
export const ContextCell: FunctionComponent<{
contextItems: ContextItem[] | undefined
model?: Model['model']
isForFirstMessage: boolean
className?: string

/** For use in storybooks only. */
__storybook__initialOpen?: boolean
}> = ({ contextItems, model, isForFirstMessage, className, __storybook__initialOpen }) => {
}> = memo(({ contextItems, model, isForFirstMessage, className, __storybook__initialOpen }) => {
const usedContext: ContextItem[] = []
const excludedAtContext: ContextItem[] = []
if (contextItems) {
Expand Down Expand Up @@ -156,4 +157,4 @@ export const ContextCell: React.FunctionComponent<{
)}
</Cell>
) : null
}
}, isEqual)
Loading

0 comments on commit 284ab72

Please sign in to comment.