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

Improvements related to Assistant prompt limit #545

Merged
merged 5 commits into from
Sep 17, 2024

Conversation

fluiddot
Copy link
Contributor

@fluiddot fluiddot commented Sep 17, 2024

Related to 9081-gh-Automattic/dotcom-forge.

Proposed Changes

  • Update the default value of the prompt limit to 200 (defined in constant LIMIT_OF_PROMPTS_PER_USER). This value matches the one defined in WPCOM.
  • In the unauthenticated view of the Assistant tab, use the constant LIMIT_OF_PROMPTS_PER_USER to render the value.
  • Ensure prompt usage is fetched when switching accounts. This was a bug I spotted when testing this PR, the prompt usage never updated despite logging in with different accounts.
  • Fix styles of Try again button displayed when getting a failed message in Assistant. Now the button is separated from the error message as defined in the designs.

Original design:
Captura de pantalla 2024-09-17 a las 12 20 26

Testing Instructions

Important

Use D161624-code for properly testing this PR.

Prompt limit

  • Run the app with the command npm start.
  • Log in to WPCOM with an A8C account.
  • Open the settings panel.
  • Observe in the AI assistant section that the max monthly prompts is 1000.
  • Log out from WPCOM.
  • Navigate to the Assistant tab.
  • Observe the message mentions that accounts get 200 prompts for free each month.
Captura de pantalla 2024-09-17 a las 12 45 29
  • Log in with a non-A8C account.
  • Open the settings panel.
  • Observe in the AI assistant section that the max monthly prompts is 200.

Try again button

  • Apply the following patch:
diff --git forkSrcPrefix/src/components/content-tab-assistant.tsx forkDstPrefix/src/components/content-tab-assistant.tsx
index 2736c84ca14a61ad1ddc3d58651829ad9da4082c..6c5e0b51e957484139d815b6718d95e7c72f317c 100644
--- forkSrcPrefix/src/components/content-tab-assistant.tsx
+++ forkDstPrefix/src/components/content-tab-assistant.tsx
@@ -166,9 +166,7 @@ export const AuthenticatedView = memo(
 					>
 						{ message.content }
 					</ChatMessage>
-					{ message.failedMessage && (
-						<ErrorNotice handleSend={ handleSend } messageContent={ message.content } />
-					) }
+					{ true && <ErrorNotice handleSend={ handleSend } messageContent={ message.content } /> }
 				</>
 			),
 			[ handleSend, siteId, updateMessage ]
  • Log in to WPCOM with an A8C account.
  • Navigate to the Assistant tab.
  • Submit a prompt.
  • Observe an error notice is displayed below the prompt.
  • Observe the Try again button is separated from the message and matches the original design (referenced above).
Captura de pantalla 2024-09-17 a las 12 49 22

Pre-merge Checklist

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

@fluiddot fluiddot requested a review from a team September 17, 2024 10:48
@fluiddot fluiddot self-assigned this Sep 17, 2024
@wojtekn
Copy link
Contributor

wojtekn commented Sep 17, 2024

It works as expected.

I confirmed the styling fix using the last text. I just wanted to note that it showed an error message and then a successful response below.

@fluiddot
Copy link
Contributor Author

[...] I just wanted to note that it showed an error message and then a successful response below.

Yes, this is expected when applying the patch, as it forces rendering the error message. Despite the error notice, the behavior of the assistant should be the normal one.

@fluiddot fluiddot merged commit 10bf7c4 into trunk Sep 17, 2024
13 of 14 checks passed
@fluiddot fluiddot deleted the update/assistant-prompt-limit branch September 17, 2024 12:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Studio AI Assistant [Type] Enhancement Improvement upon an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants