feat(prompt-editor): Add new ProseMirror-based implementation #6272

fkling · 2024-12-06T14:52:13Z

This PR adds a new version of the prompt editor, based on ProseMirror.

Video demo

sg-prompt-editor.mp4

Implementation notes

ProseMirror

We use ProseMirror with a custom schema, that only contains paragraph nodes, text nodes and mention nodes (the mention "chips"). Programmatic changes to the editor value happen via transactions, which are applied to the current state and produce a new state. More on that in the next section.

Main 'components'

The 'meat' of the input is modeled as an xstate statechart in promptInput.ts and consists of three main pieces:

The editor itself, whose state gets updated either via user interaction or programmatically, via events.
The data fetching for the mentions menu, including which provider is currently selected.
The mentions menu itself, specifically its visibility state and which item is selected.

Initially I implemented each of these completely separately, but combined them all into a single statechart. While I normally prefer modularity, all of these parts have intricate connections, and all of them make the prompt input what it is, so it seemed to be reasonable to group them together. The nature of the statechart still makes them easy to reason about IMO.
I chose to implement the whole system as a statechart so that we have a mostly descriptive way to specify how the input should behave, in a central place.

Most importantly though, any operation that can change the value of the input is implemented as a state transition. This has the advantage that we can see in a single place which operations are available and how they will affect the input. And we can also easily test them.

promptInput-react.ts is a thin wrapper around the implementation that makes the input and mentions menu data available via hooks and abstracts from the state machine.

PromptEditor.tsx renders the actual UI elements based on the state and provides the data from the 'outside world'. Specifically this component provides the implementation for fetching the mentions data.

Auxiliary modules that support the above are:

MentionsMenu.tsx - for rendering the mentions menu. It uses existing components for rendering the menu items.
MentionsView.tsx - for rendering the mentions 'chips'. I couldn't use the existing mentions node component because it was strongly depending on lexical specific data (but maybe that could be abstracted).
lexical-interop.tsx - makes it possible to use the new component as a drop in replacement (I hope) by converting lexical editor state to and from prosemirror state, based on my understanding of the lexical state that I saw in storybook. I didn't spend a lot of time testing this yet, but I'll iterate on this as needed.

Styling

I'm not quite sure what current guidelines are around using CSS directly and using tailwind. I used tailwind classes mostly where I wanted to replicate the style of the existing input. Happy to eventually clean that up.

Differences to the current implementation

Renders a decoration around the @mention in the input
@mention's need to be explicitly triggered (like in linear). When you 'leave' and @mention it turns into normal text.
Currently does not memoize results from providers. I first need to understand better why/when/how we want this. Given how it's implemented it should be easy to add though.
Some difference regarding styling and copy for the mentions menu.

Things that I wanted to do but couldn't (yet)

I really wanted to provide a better experience for the mentions menu while fetching the mentions data (e.g. showing a loading indicator). But given the nature of the API for fetching the data this was not easily doable. It's something I want to investigate when working on the new designs for the menu.
Eventually we want to use the input also in the prompt library for creating prompts. For this we might want to introduce a new node type for placeholders. I don't have enough context on that project yet so I didn't do anything in this direction, but adding support for this shouldn't be too difficult.
There is no support for more complex node types like lists or code blocks. I think we want to add those eventually but I also need more information on this first.

Test plan

Manual testing and unit tests. This PR doesn't impact any of the code that is currently in use, it only adds the new component so that it can be used in the future.

fkling · 2024-12-06T14:52:15Z

This change is part of the following stack:

feat(prompt-editor): Add new ProseMirror-based implementation #6272 ◀
- feat(onebox): Use new prompt editor when onebox is enabled #6288

_{Change managed by git-spice.}

dominiccooney

Some feedback inline. I got 2/3 of the way through this but it is huge so let me send some feedback and look at the tests in a future iteration.

Let's be more helpful with comments and more consistent with lints.

A number of questions inline about the details of the triggering, etc.

lib/prompt-editor/src/v2/MentionView.module.css

dominiccooney · 2024-12-10T08:01:28Z

lib/prompt-editor/src/v2/MentionsMenu.module.css

@@ -0,0 +1,17 @@
+.popover-dimensions {
+    width: clamp(300px, 65vw, 440px);


One sore point I have with the existing editor is sometimes the popovers are wider than the sidebar. Does the vw units help us avoid this problem here?

I've copied this from the current implementation, so probably not.
However this new implementation uses a separate plugin for popover management, which includes logic to resize the popover to the available width of the viewport (which I assume is just the sidebar in an IDE).

Yeah, the old version uses radix popover which doesn't set any strict boundaries for popover elements (like max-width, max-height) and also does not shift popover properly if there is no enough space between target and viewport boundaries, floating UI should solve the problem

lib/prompt-editor/src/v2/MentionsMenu.module.css

lib/prompt-editor/src/v2/MentionsMenu.tsx

dominiccooney · 2024-12-11T09:00:31Z

lib/prompt-editor/src/v2/plugins/atMention.ts

+        inputRules({
+            rules: [
+                new InputRule(
+                    // Trigger on @, at beginning or after space


Would be great to test this with multiple lines.

Also, today I can write multiple at-mentions without a space between them (one is inserted, but is not required...)

Also, today I can go to the middle of my composition and start typing at-mentions, but the terminal $ is going to stop that (...unless it is interpreted as a multiline regex, in which case I can type them at the end of lines but not in the middle 🤷 )

Also, today I can write multiple at-mentions without a space between them (one is inserted, but is not required...)

The new version will require a space or beginning of the line before the @.

Also, today I can go to the middle of my composition and start typing at-mentions, but the terminal $ is going to stop that

The input for the regular expression is the some text before the cursor, i.e. the end of the text will always be the text before the cursor, not the end for the line or whole input.

From the docs:

The rule applies when the user typed something and the text directly in front of the cursor matches match, which should end with $.

dominiccooney · 2024-12-11T09:04:22Z

lib/prompt-editor/src/v2/plugins/placeholder.ts

+        if (view.state.doc.childCount === 1 && view.state.doc.firstChild?.textContent === '') {
+            view.dom.setAttribute('data-placeholder', placeholderPluginKey.getState(view.state) ?? '')
+        } else {
+            view.dom.removeAttribute('data-placeholder')


Er... if I delete the text, shouldn't the placeholder come back? That's how the platform's placeholders work.

Or are these plugins pure functions or something, and ProseMirror is going to recreate the element and its data-placeholder?

When text is deleted then if (...) is true. That's because even when the input is empty, the document still contains an empty paragraph node. The else branch is taken when the editor contains text.

lib/prompt-editor/src/v2/plugins/readonly.ts

lib/prompt-editor/src/v2/promptInput-react.ts

thenamankumar · 2024-12-12T06:10:11Z

lib/prompt-editor/src/v2/MentionView.tsx

+    )
+}
+
+function tooltipForContextItem(item: SerializedContextItem): string | undefined {


For future, we should make the context item define its tooltip.

vovakulikov

I tested this manually and didn't find any big problems so approve to unblock. I tried the case when I switched on and then switched back to the older version and something is definitely going on there, some messages aren't picked up right with lexical probably because of some shifts in message schema (lexical state)

But this problem is ok for now, I suppose we still have to live with the exical state for a while in Cody Agent lexical state serialization. (Not this PR problem)

vovakulikov · 2024-12-20T15:17:22Z

lib/prompt-editor/src/index.ts

@@ -2,6 +2,7 @@ export { PromptEditor, type PromptEditorRefAPI } from './PromptEditor'
 export { ContextItemMentionNode, MENTION_CLASS_NAME } from './nodes/ContextItemMentionNode'
 export { MentionMenu } from './mentions/mentionMenu/MentionMenu'
 export { BaseEditor } from './BaseEditor'
+export { PromptEditor as PromptEditorV2 } from './v2/PromptEditor'


We probably should re-export this later independently so I could publish this package without a big amount of Cody Agent dependencies in order to use it in the Sourcegraph prompt library (can do this in the follow up PR though)

vovakulikov · 2024-12-20T15:19:29Z

lib/prompt-editor/src/v2/MentionsMenu.module.css

@@ -0,0 +1,17 @@
+.popover-dimensions {
+    width: clamp(300px, 65vw, 440px);


Yeah, the old version uses radix popover which doesn't set any strict boundaries for popover elements (like max-width, max-height) and also does not shift popover properly if there is no enough space between target and viewport boundaries, floating UI should solve the problem

…nput

I've made many of the requested changes; this is low risk because it doesn't affect any production code; the new editor will be opt-in via feature flag.

fkling requested review from vovakulikov and thenamankumar December 6, 2024 15:32

fkling self-assigned this Dec 6, 2024

fkling marked this pull request as ready for review December 6, 2024 15:32

kalanchan requested a review from a team December 6, 2024 22:56

fkling mentioned this pull request Dec 9, 2024

feat(onebox): Use new prompt editor when onebox is enabled #6288

Merged

dominiccooney previously requested changes Dec 11, 2024

View reviewed changes

thenamankumar reviewed Dec 12, 2024

View reviewed changes

fkling mentioned this pull request Dec 16, 2024

feat(onebox): Add ability to select search results as follow up context #6347

Merged

fkling force-pushed the fkling/prompt-editor branch from 0768e58 to 28a42d3 Compare December 16, 2024 14:03

stefanhengl self-requested a review December 20, 2024 09:29

vovakulikov approved these changes Dec 20, 2024

View reviewed changes

fkling force-pushed the fkling/prompt-editor branch from 28a42d3 to 806b534 Compare December 21, 2024 12:57

fkling added 16 commits December 21, 2024 13:57

Progress

0661cfd

Refactor

10a282b

Progress

d97c593

Better encapsulation

506dccb

Refactor

2110477

Added react hooks abstraction layer

5c84cd0

Progress

76851cd

Finish api and implementation

6150ac3

Refector/rename mention view

e662340

Use floating ui to manage mentions menu position

28e6f31

Support missing behavior;add comments; add tests

b19dd71

Update dependencies

5064fcb

Run biome

efaab9b

Cleanup

501c43a

Add separate story

eb49f26

fix mentions menu

2436141

fkling added 2 commits December 21, 2024 13:57

Address first round of feedback

37e6011

Update new prompt input to support recent changes to current prompt i…

17a9c08

…nput

fkling force-pushed the fkling/prompt-editor branch from 806b534 to 17a9c08 Compare December 21, 2024 12:57

fkling requested a review from dominiccooney December 21, 2024 13:06

fkling merged commit abf27bb into main Dec 21, 2024
38 of 42 checks passed

fkling deleted the fkling/prompt-editor branch December 21, 2024 13:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(prompt-editor): Add new ProseMirror-based implementation #6272

feat(prompt-editor): Add new ProseMirror-based implementation #6272

fkling commented Dec 6, 2024 •

edited

Loading

fkling commented Dec 6, 2024 •

edited

Loading

dominiccooney left a comment

dominiccooney Dec 10, 2024

fkling Dec 11, 2024

vovakulikov Dec 20, 2024

dominiccooney Dec 11, 2024

fkling Dec 11, 2024

dominiccooney Dec 11, 2024

fkling Dec 11, 2024

thenamankumar Dec 12, 2024

vovakulikov left a comment

vovakulikov Dec 20, 2024

vovakulikov Dec 20, 2024

		@@ -0,0 +1,17 @@
		.popover-dimensions {
		width: clamp(300px, 65vw, 440px);

feat(prompt-editor): Add new ProseMirror-based implementation #6272

feat(prompt-editor): Add new ProseMirror-based implementation #6272

Conversation

fkling commented Dec 6, 2024 • edited Loading

Video demo

Implementation notes

ProseMirror

Main 'components'

Styling

Differences to the current implementation

Things that I wanted to do but couldn't (yet)

Test plan

fkling commented Dec 6, 2024 • edited Loading

dominiccooney left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vovakulikov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fkling commented Dec 6, 2024 •

edited

Loading

fkling commented Dec 6, 2024 •

edited

Loading