From d80689c434d801e7a16c8639b076bc2618fc4107 Mon Sep 17 00:00:00 2001 From: Felix Kling Date: Sat, 15 Feb 2025 11:37:43 +0100 Subject: [PATCH] fix: Make it possible to dogfood new prompt editor (#7094) Closes https://linear.app/sourcegraph/issue/SRCH-1667/no-results-returned-when-enabling-feature-for-queries This PR fixes an issue with the prompt input being cleared out when a query is submitted and the new input is enabled. ## Root cause The root cause for this issue is twofold: 1. Evaluating feature flags is always asynchronous. The first time `HumanMessageEditor` is rendered we will render the old editor. 1. In the same component, `useImperativeHandle` is only run once when the component is mounted. This means we never update the ref with the ref from the new editor once the feature flag updates. The consequence of this is that everywhere else where `editorRef` is used we have a reference to the _old_ editor API, which has been destroyed since. Reading the value from it will of course be empty, which in turn causes the input value of the rendered editor to be overwritten. ## Solution It seems obvious to remove the empty dependency list from the `useImperativeHandle` call to make that the ref is always updated. We do this in other places too (`Transcript.tsx`). However, Vova pointed out that there can still be race conditions between accessing `humanEditoRef` and the feature flag loading and suggested that we should retrieve the feature flag value before we render any UI. I've extended the object returned by `getConfigForWebview` but I'm not sure whether that's the right place to put it. I've also heard that we want to make as few network requests as possible during initialization, which is understandable. > [!IMPORTANT] > I'd like some guidance from the cody team on where put this setting to ensure avoid the aforementioned problem. ## Test plan 1. Enabled the feature flag for myself on S2. 1. Ran the standalone web demo (`cd client/web; pnpm dev`) 1. Verified that the new input used (typing `@...` looks slightly different) 1. Submitting a query does not clear the input and generates an appropriate response. --- lib/prompt-editor/src/v2/promptInput.ts | 11 ++--------- vscode/src/chat/chat-view/ChatController.ts | 6 ++++++ vscode/src/chat/protocol.ts | 1 + vscode/webviews/App.story.tsx | 1 + .../messageCell/human/editor/HumanMessageEditor.tsx | 13 ++++++------- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/lib/prompt-editor/src/v2/promptInput.ts b/lib/prompt-editor/src/v2/promptInput.ts index fbe91d3dae29..5dab521b6f52 100644 --- a/lib/prompt-editor/src/v2/promptInput.ts +++ b/lib/prompt-editor/src/v2/promptInput.ts @@ -183,18 +183,11 @@ const prosemirrorActor = fromCallback { switch (event.type) { case 'focus': - doFocus() - // HACK(sqs): Needed in VS Code webviews to actually get it to focus - // on initial load, for some reason. - setTimeout(doFocus) + editor.focus() + editor.dispatch(editor.state.tr.scrollIntoView()) break case 'blur': editor.dom.blur() diff --git a/vscode/src/chat/chat-view/ChatController.ts b/vscode/src/chat/chat-view/ChatController.ts index a582b5394127..f92cc556757a 100644 --- a/vscode/src/chat/chat-view/ChatController.ts +++ b/vscode/src/chat/chat-view/ChatController.ts @@ -15,6 +15,7 @@ import { DOTCOM_URL, type DefaultChatCommands, type EventSource, + FeatureFlag, type Guardrails, ModelUsage, type NLSSearchDynamicFilter, @@ -36,6 +37,7 @@ import { extractContextFromTraceparent, featureFlagProvider, firstResultFromOperation, + firstValueFrom, forceHydration, graphqlClient, hydrateAfterPostMessage, @@ -528,6 +530,9 @@ export class ChatController implements vscode.Disposable, vscode.WebviewViewProv private async getConfigForWebview(): Promise { const { configuration, auth } = await currentResolvedConfig() + const experimentalPromptEditorEnabled = await firstValueFrom( + featureFlagProvider.evaluatedFeatureFlag(FeatureFlag.CodyExperimentalPromptEditor) + ) const sidebarViewOnly = this.extensionClient.capabilities?.webviewNativeConfig?.view === 'single' const isEditorViewType = this.webviewPanelOrView?.viewType === 'cody.editorPanel' const webviewType = isEditorViewType && !sidebarViewOnly ? 'editor' : 'sidebar' @@ -545,6 +550,7 @@ export class ChatController implements vscode.Disposable, vscode.WebviewViewProv multipleWebviewsEnabled: !sidebarViewOnly, internalDebugContext: configuration.internalDebugContext, allowEndpointChange: configuration.overrideServerEndpoint === undefined, + experimentalPromptEditorEnabled, } } diff --git a/vscode/src/chat/protocol.ts b/vscode/src/chat/protocol.ts index f61d959c0402..67c7ea3329be 100644 --- a/vscode/src/chat/protocol.ts +++ b/vscode/src/chat/protocol.ts @@ -260,6 +260,7 @@ export interface ConfigurationSubsetForWebview multipleWebviewsEnabled?: boolean | undefined | null endpointHistory?: string[] | undefined | null allowEndpointChange: boolean + experimentalPromptEditorEnabled: boolean } /** diff --git a/vscode/webviews/App.story.tsx b/vscode/webviews/App.story.tsx index 9630a24d5da1..98c32bc279dd 100644 --- a/vscode/webviews/App.story.tsx +++ b/vscode/webviews/App.story.tsx @@ -31,6 +31,7 @@ const dummyVSCodeAPI: VSCodeWrapper = { smartApply: false, hasEditCapability: false, allowEndpointChange: true, + experimentalPromptEditorEnabled: false, }, clientCapabilities: CLIENT_CAPABILITIES_FIXTURE, authStatus: { diff --git a/vscode/webviews/chat/cells/messageCell/human/editor/HumanMessageEditor.tsx b/vscode/webviews/chat/cells/messageCell/human/editor/HumanMessageEditor.tsx index 4cf0b6800035..cd367aa27557 100644 --- a/vscode/webviews/chat/cells/messageCell/human/editor/HumanMessageEditor.tsx +++ b/vscode/webviews/chat/cells/messageCell/human/editor/HumanMessageEditor.tsx @@ -1,7 +1,6 @@ import { type ChatMessage, FAST_CHAT_INPUT_TOKEN_BUDGET, - FeatureFlag, type Model, ModelTag, type SerializedPromptEditorState, @@ -33,7 +32,6 @@ import { type ClientActionListener, useClientActionListener } from '../../../../ import { promptModeToIntent } from '../../../../../prompts/PromptsTab' import { useTelemetryRecorder } from '../../../../../utils/telemetry' import { useConfig } from '../../../../../utils/useConfig' -import { useFeatureFlag } from '../../../../../utils/useFeatureFlags' import { useLinkOpener } from '../../../../../utils/useLinkOpener' import { useOmniBox } from '../../../../../utils/useOmniBox' import styles from './HumanMessageEditor.module.css' @@ -102,7 +100,7 @@ export const HumanMessageEditor: FunctionComponent<{ const telemetryRecorder = useTelemetryRecorder() const editorRef = useRef(null) - useImperativeHandle(parentEditorRef, (): PromptEditorRefAPI | null => editorRef.current, []) + useImperativeHandle(parentEditorRef, (): PromptEditorRefAPI | null => editorRef.current) // The only PromptEditor state we really need to track in our own state is whether it's empty. const [isEmptyEditorValue, setIsEmptyEditorValue] = useState( @@ -124,9 +122,6 @@ export const HumanMessageEditor: FunctionComponent<{ ? 'emptyEditorValue' : 'submittable' - // TODO: Finish implementing "current repo not indexed" handling for v2 editor - const experimentalPromptEditorEnabled = useFeatureFlag(FeatureFlag.CodyExperimentalPromptEditor) - const onSubmitClick = useCallback( (intent?: ChatMessage['intent'], forceSubmit?: boolean): void => { if (!forceSubmit && submitState === 'emptyEditorValue') { @@ -163,7 +158,10 @@ export const HumanMessageEditor: FunctionComponent<{ ) const omniBoxEnabled = useOmniBox() - const { isDotComUser } = useConfig() + const { + isDotComUser, + config: { experimentalPromptEditorEnabled }, + } = useConfig() const onEditorEnterKey = useCallback( (event: KeyboardEvent | null): void => { @@ -425,6 +423,7 @@ export const HumanMessageEditor: FunctionComponent<{ [linkOpener] ) + // TODO: Finish implementing "current repo not indexed" handling for v2 editor const Editor = experimentalPromptEditorEnabled ? PromptEditorV2 : PromptEditor return (