Studio: Add dynamic code block to execute wp-cli commands UI AI assistant #232

katinthehatsite · 2024-06-11T14:13:47Z

Related to https://github.com/Automattic/dotcom-forge/issues/7526

Proposed Changes

This PR adds that dynamic code block that allows to execute CLI commands inside that block. It also does the following:

if the code block contains more than 1 CLI command, the Run button is disabled
the Run button is disabled for code blocks that don't have CLI commands
when you execute the CLI command, see the output, navigate to another tab and come back, you can see the results of the output preserved

Testing Instructions

Pull the changes from this branch locally
Start the app with STUDIO_AI=true npm start
Navigate to the Assitant tab
Ask the assistant to give you a CLI command to list users
Observe that it produces a code block
Run the command with Run button
Confirm that you see the output being rendered
Navigate to a different tab
Come back to the AI Assistant
Observe that the output of the CLI command is preserved:

Capture d’écran, le 2024-06-13 à 10 26 19

Try asking for different CLI commands and checking the output

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors?

katinthehatsite · 2024-06-12T19:52:04Z

A note that I noticed that sometimes CLI output could be strange - for instance, I asked the Chat Assistant to give me CLI commands and it suggested the following but running that command gave strange output that requires action:

Capture d’écran, le 2024-06-12 à 21 49 47

We might need to add a better validation of the commands that we want to run to make sure it does not result into the actionable output like this.

katinthehatsite · 2024-06-13T07:38:56Z

package.json

@@ -115,6 +115,7 @@
 		"hpagent": "1.2.0",
 		"react-markdown": "^9.0.1",
 		"semver": "^7.6.0",
+		"strip-ansi": "^7.1.0",


This was necessary to strip ANSI that comes in the output from CLI - otherwise, the Error and Success messages contain ANSI characters and manually removing them is quite tedious as it is hard to match all of them. I felt like this was appropriate to use

kozer · 2024-06-13T08:58:48Z

@katinthehatsite nice work!
I left some comments/suggestions.
Other than that it works as expected

Note that there are some errors in buildkite/unit-tests that are related to the new library you introduced.

kozer · 2024-06-13T08:45:33Z

src/hooks/use-execute-cli.ts

+		}, 2300 );
+	}, [ content, id, projectPath, updateMessage ] );
+
+	return {


[Suggestion]: I find it a bit repetitive. As we call it like this:

const { cliOutput, cliStatus, cliTime, isRunning, handleExecute, setCliOutput, setCliStatus, setCliTime, } = useExecuteWPCLI( content, projectPath, updateMessage, id );

it is clear enough that this comes from cli.
I suggest to rename them to:

output, status, time setOutput, setStatus, setTime

kozer · 2024-06-13T08:48:20Z

src/components/chat-messages.tsx

+			if ( blocks ) {
+				const block = blocks?.find( ( block ) => block.codeBlockContent === content );
+				if ( block ) {
+					setCliOutput( block?.cliOutput ? stripAnsi( block.cliOutput ) : null );


[Suggestion] I suggest to change that to setBlock, as this is what those variables actually represent

katinthehatsite · 2024-06-13T09:08:36Z

Something that still needs to be looked into but documenting it here for the reviewers:

when you use Run button and the output appears, the chat scrolls to the bottom of the page. It is because of:

studio/src/components/content-tab-assistant.tsx

Line 41 in f756e2a

useEffect( () => {

that scrolls to the bottom every time an array of messages updates. It would need to be somehow adjusted to scroll through when the messages are added vs. not scroll through when the output is produced (this happens with updateMessage)
another thing that the Run button does not go to Run again once the output is generated. This needs to be adjusted as well

sejas · 2024-06-17T10:29:44Z

Let's also avoid displaying the Run button if the wp command has some placeholder < >.

katinthehatsite · 2024-06-17T11:35:13Z

src/hooks/use-assistant.ts


-export interface Message {
+export type Message = {
+	id?: number;


I am not 100% sure if this should stay optional or must be mandatory.

…-block

katinthehatsite · 2024-06-17T18:23:19Z

Hi @sejas, since our conversation earlier this morning, I have done the following:

Refactored the Run and Run again buttons, reused some of the components that we already have and removed the redundant logic;
Fixed the failing unit test;
Resolved conflicts with trunk and all the latest changes how now been integrated with trunk
Added a check that looks for < and > characters and disables the Run button for these commands

The only other thing I see is that it seems that E2E tests are timing out on Windows 🤔

For the horizontal scroll, I suggest we open a separate issue and handle it in the follow-up. I am not seeing an obvious fix right away and I would prefer to merge the PR asap as it is starting to grow quite large. What do you think?

Let me know what you think of the changes!

…te-cli-button-to-execute-in-code-block

sejas

Works as expected.
It keeps the result after changing sites and tabs.

@katinthehatsite , great work!

0Hwa8f.mp4

sejas · 2024-06-19T13:54:18Z

src/components/chat-message.tsx

+		const containsWPCommand = /\bwp\s/.test( content );
+		const wpCommandCount = ( content.match( /\bwp\s/g ) || [] ).length;
+		const containsSingleWPCommand = wpCommandCount === 1;
+		const containsAngleBrackets = /<.*>/.test( content );


We can add more conditions to make this code block only available in a few commands.

I noticed that the AI also uses braces {} for argument placeholders. I propose including this symbol in the regular expression. WDYT?

I created a PR with improvements in this area.

Noting that I have also seen the Assistant exclude wrapping characters altogether for placeholder content, which complicates things a good bit. I suppose the only solution for this case would be adjusting the Assistant prompt in hopes that it avoids this approach or searching for example terms (which is undoubtedly brittle).

Oh, I haven't encountered that but yes, that definitely complicates the detection of placeholders. As you pointed out, seems the best approach would be to tweak the prompt to ensure it uses the same placeholder to avoid cases like this one.

dcalhoun

I noted a few issues after pulling these changes. I share them here merely to raise awareness. I'll open a PR addressing them.

dcalhoun · 2024-06-19T16:36:21Z

src/components/copy-text-button.tsx

@@ -34,6 +40,7 @@ export function CopyTextButton( {
 		}
 		setTimeoutId( setTimeout( () => setShowCopied( false ), timeoutConfirmation ) );
 	}, [ text, timeoutConfirmation, timeoutId ] );
+	console.log( 'Icon Size:', iconSize );


I suggest we remove this console log. I presume its addition was oversight.

dcalhoun · 2024-06-19T16:39:17Z

src/components/content-tab-assistant.tsx

-	} ) => (
-		<>
-			{ messages.map( ( message, index ) => (
-				<ChatMessage key={ index } id={ `message-${ index }` } isUser={ message.role === 'user' }>


It appears the id attribute was removed. The removal introduces a regression where screen readers are no longer able to navigate each message as a group, which was enabled in #248.

I presume this was oversight and likely originates from a merge conflict resolution.

Thanks for catching these and adding the PR to fix. Yeah that was a large conflict merge

Enable Run cli button

8920dfb

katinthehatsite self-assigned this Jun 11, 2024

Kateryna Kodonenko added 18 commits June 11, 2024 16:34

Fix timer for the time of execution

43f23b8

Move state to code block to avoid the run button running on all blocks

82d6d78

Display output from cli for error and success cases

59c2763

Hide the button for code blocks that have no CLI commands

538d974

Ensure only code blocks with one wp command execute

40fc4aa

Add logic for updating messages in useAssistant

263cec7

Add logic for storing the messages

09b8318

Fix type on updateMessage

5791cc1

Handle undefined ID

8fe342c

Fix types on authenticated view

1578661

Fix some more types

d27d9a4

Fix up another warning

6306c38

Cleanup

53afeaf

Update trunk changes

9b3b938

Merge trunk

85ff515

Split content tab assistant and chat message

c3bea19

Fix weird re-render

f0ac874

Fix unit tests

a01f80f

Kateryna Kodonenko added 3 commits June 12, 2024 22:04

Strip ANSI codes from errors

fc2aacd

Move the loggic of processing cli output into a hook

b5511e3

Ensure that the DOM updates for the scroll to work

f756e2a

katinthehatsite commented Jun 13, 2024

View reviewed changes

katinthehatsite added the Studio AI Assistant label Jun 13, 2024

katinthehatsite requested a review from kozer June 13, 2024 08:27

kozer reviewed Jun 13, 2024

View reviewed changes

Kateryna Kodonenko added 2 commits June 17, 2024 12:08

Fix jest configuration

16a8e4a

Fix the jest setup

4a5ece6

Fix failing unit test

018fd1b

katinthehatsite commented Jun 17, 2024

View reviewed changes

katinthehatsite and others added 9 commits June 17, 2024 15:17

Merge branch 'trunk' into add/integrate-cli-button-to-execute-in-code…

ebbd82e

…-block

Fix linter on use-assistant

c488aa0

Fix linter on content-tab-assistant

470ca6f

Refactor the copy button in code block

6613ab4

Refactor button logic for Run and Run again

f2560bf

Fix styling on run button

1f1e306

Fix buttons

8843fbc

Final touches on button styles

885b059

Hide commands that include placeholders

140c36c

katinthehatsite requested a review from sejas June 17, 2024 18:24

Resolve conflict

fb0ab91

sejas mentioned this pull request Jun 19, 2024

Refactor the Chat Message into its own file #273

Merged

1 task

sejas added 5 commits June 19, 2024 13:53

Refactor component name to ChatMessage

dbf1f49

Merge branch 'trunk' of github.com:Automattic/studio into add/integra…

fa98b69

…te-cli-button-to-execute-in-code-block

Fix args passed to executeWPCliInline to be one string

fd49bf6

Use meesage id in useExecuteWPCLI

bffee48

Add empty line on package.json

34261f1

sejas approved these changes Jun 19, 2024

View reviewed changes

sejas merged commit 995de23 into trunk Jun 19, 2024
10 checks passed

sejas deleted the add/integrate-cli-button-to-execute-in-code-block branch June 19, 2024 14:00

dcalhoun reviewed Jun 19, 2024

View reviewed changes

dcalhoun mentioned this pull request Jun 19, 2024

fix: Reinstate Assistant a11y message groups #275

Merged

1 task

This was referenced Jun 26, 2024

Improve validation of wp-cli commands that contain placeholder content #309

Merged

test: Assert Assistant code block #305

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Studio: Add dynamic code block to execute wp-cli commands UI AI assistant #232

Studio: Add dynamic code block to execute wp-cli commands UI AI assistant #232

katinthehatsite commented Jun 11, 2024 •

edited

Loading

katinthehatsite commented Jun 12, 2024

katinthehatsite Jun 13, 2024

kozer commented Jun 13, 2024

kozer Jun 13, 2024

kozer Jun 13, 2024

katinthehatsite commented Jun 13, 2024 •

edited

Loading

sejas commented Jun 17, 2024

katinthehatsite Jun 17, 2024

katinthehatsite commented Jun 17, 2024 •

edited

Loading

sejas left a comment

sejas Jun 19, 2024

fluiddot Jun 26, 2024

fluiddot Jun 26, 2024

dcalhoun Jun 26, 2024 •

edited

Loading

fluiddot Jun 26, 2024

dcalhoun left a comment

dcalhoun Jun 19, 2024

dcalhoun Jun 19, 2024 •

edited

Loading

katinthehatsite Jun 20, 2024

Studio: Add dynamic code block to execute wp-cli commands UI AI assistant #232

Studio: Add dynamic code block to execute wp-cli commands UI AI assistant #232

Conversation

katinthehatsite commented Jun 11, 2024 • edited Loading

Proposed Changes

Testing Instructions

Pre-merge Checklist

katinthehatsite commented Jun 12, 2024

Choose a reason for hiding this comment

kozer commented Jun 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katinthehatsite commented Jun 13, 2024 • edited Loading

sejas commented Jun 17, 2024

Choose a reason for hiding this comment

katinthehatsite commented Jun 17, 2024 • edited Loading

sejas 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

dcalhoun Jun 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcalhoun left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dcalhoun Jun 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katinthehatsite commented Jun 11, 2024 •

edited

Loading

katinthehatsite commented Jun 13, 2024 •

edited

Loading

katinthehatsite commented Jun 17, 2024 •

edited

Loading

dcalhoun Jun 26, 2024 •

edited

Loading

dcalhoun Jun 19, 2024 •

edited

Loading