Clear regular prompt message if using Re-Ask Option #616

rjwignar · 2024-04-25T04:56:07Z

This fixes #587

Background

7cc1e2f allowed web handlers and slash commands to be executed with the Re-ask option (issue described in #541) by passing the prompt text to the onPrompt call in MessagesView::onResubmitClick().

Before this change, retrying with a regular prompt message would trigger the regular re-try routine (since the prompt text wasn't passed to the onPrompt call in MessagesView::onResubmitClick():

chatcraft.org/src/Chat/ChatBase.tsx

Lines 283 to 297 in 9f73dea

    
           } else { 
        
             // If there isn't any prompt text, see if the final message in the chat was a human 
        
             // message or a function response. If it was either, we'll allow sending that through 
        
             // again (e.g., if you modified something and want to retry, or want to share the 
        
             // response from the function). Otherwise bail now. 
        
             const finalMessage = chat.messages({ includeAppMessages: false }).at(-1); 
        
             if ( 
        
               !( 
        
                 finalMessage instanceof ChatCraftHumanMessage || 
        
                 finalMessage instanceof ChatCraftFunctionResultMessage 
        
               ) 
        
             ) { 
        
               return; 
        
             } 
        
           }

After this change however, if re-asking with a regular prompt message, the updated message will be passed to onPrompt (as prompt) and add a duplicate message to the chat (line 278):

chatcraft.org/src/Chat/ChatBase.tsx

Lines 275 to 278 in 9f73dea

    
           if (prompt) { 
        
             // Add this prompt message to the chat 
        
             promptMessage = new ChatCraftHumanMessage({ text: prompt, imageUrls, user }); 
        
             await chat.addMessage(promptMessage);

Changes

I added a boolean parameter to ChatBase::onPrompt called retry (false by default):

    async (prompt?: string, imageUrls?: string[], retry: boolean = false) => {
      // ...
      try {
        // If retrying, set the prompt to null to avoid duplicate message
        if (retry) {
          prompt = "";
        }
        // If the prompt text exist, package it up as a human message and add to the chat
        // ...

If retry is True, the prompt will be cleared to prevent message duplication.

retry is False by default because I only want to explicitly set it to True when calling onPrompt() from onResubmitClick():

type MessagesViewProps = {
  // ...
  onPrompt: (prompt?: string, imageUrls?: string[], retry?: boolean) => void;
};
// ... 

function MessagesView({
          onResubmitClick={async (promptText?: string) => {
            await deleteMessages(message.id, "after");
            onPrompt(promptText, undefined, true); // pass prompt text and true for retry, don't include any imageURLs
          }}
          // ...
        })

I had to update the onPrompt signature in MessagesViewProps for syntax reasons. If there's a better way to implement this I'm open to suggestions.

Comparison to Production (updated with Image as Input observations)

Below are the observations I've made on the Re-Ask options before and after 7cc1e2f, as well as for this Pull Request:

Task	Observations pre- `7cc1e2f` (tested using `adcfd2d`)	Observations post- `7cc1e2f` (tested using Production as of `e4967a9`) (Prompt duplication bug observed)	PR Observations
Ask with "help" (to invoke /help)	Executes /help command	Executes /help command	Executes /help command
Re-ask with "help" (to invoke /help)	Response from LLM (undesired)	Executes /help command	Executes /help command
Ask with web handler	Executes web handler	Executes web handler	Executes web handler
Re-ask with web handler	Response from LLM (undesired)	Executes web handler	Executes web handler
Ask with slash command (/import)	Invokes /import	Invokes /import	Invokes /import
Re-ask with slash command (/import)	Response from LLM (undesired)	Invokes /import	Invokes /import
Ask with regular prompt	Response from LLM	Response from LLM	Response from LLM
Re-ask with regular prompt	Response from LLM	Response from LLM, duplicates new message (undesired)	Response from LLM, no message duplication
Ask with Image as Input (no prompt)	Appropriate LLM response	Appropriate LLM response	Appropriate LLM response
Re-ask with Image as Input (no prompt, same image ²)	Appropriate LLM Response	Appropriate LLM Response (no message duplication as no prompt present, expected)	Appropriate LLM (no message duplication as no prompt present, expected)
Ask with prompt and Image as Input	Appropriate LLM Response	Appropriate LLM Response	Appropriate LLM Response
Re-ask with prompt and Image as Input (same image ²)	Appropriate LLM Response (no message duplication, desired result)	Appropriate LLM Response (message duplication with text only, undesired ³)	Appropriate LLM Response (no message duplication, desired result)

My PR preserves the changes from 7cc1e2f but prevent the message duplication when re-trying with a regular prompt message.

…ompt definition, updated onPrompt call in onResubmitClick

rjwignar · 2024-04-25T04:57:43Z

Moving to draft because I haven't yet checked how re-asking with an Image has been affected through 7cc1e2f and/or my PR (if at all).
Will mark this PR as ready in the morning once I've checked.

tarasglek · 2024-04-25T10:36:16Z

seems to work well! thanks for jumping on this

rjwignar · 2024-04-25T16:46:36Z

I've added my Observations for Re-asking with Image as Input ¹ before and after the message duplication bug.

Task	Observations pre- `7cc1e2f` (tested using `adcfd2d`)	Observations post- `7cc1e2f` (tested using Production as of `e4967a9`) (Prompt duplication bug observed)	PR Observations
Ask with Image as Input (no prompt)	Appropriate LLM response	Appropriate LLM response	Appropriate LLM response
Re-ask with Image as Input (no prompt, same image ²)	Appropriate LLM Response	Appropriate LLM Response (no message duplication as no prompt present, expected)	Appropriate LLM (no message duplication as no prompt present, expected)
Ask with prompt and Image as Input	Appropriate LLM Response	Appropriate LLM Response	Appropriate LLM Response
Re-ask with prompt and Image as Input (same image ²)	Appropriate LLM Response (no message duplication, desired result)	Appropriate LLM Response (message duplication with text only, undesired ³)	Appropriate LLM Response (no message duplication, desired result)

1 Image as Input tested using gpt-4-1106-vision-preview
2 Message editing allows text editing only, not image
3 Duplication example provided below:

Future Considerations

As of e4967a9, we currently don't pass any imageUrls (from Image as Input) in the onPrompt() call in onResubmitClick() during the Re-Ask Option.

However, if this changes in the future, we might see this bug again, in the form of Input Image(s) duplication.
If this bug appears in the future, we might be able to fix it by extending the changes in this PR to clear imageUrls on retry, with something like:

        // If retrying, empty the prompt and imageUrls to avoid duplicate message
        if (retry) {
          prompt = "";
          imageUrls = [];
        }

cloudflare-workers-and-pages · 2024-04-25T16:48:26Z

Deploying chatcraft-org with Cloudflare Pages

Latest commit:	`f46f172`
Status:	✅ Deploy successful!
Preview URL:	https://bbb872a0.console-overthinker-dev.pages.dev
Branch Preview URL:	https://issue-587.console-overthinker-dev.pages.dev

View logs

tarasglek · 2024-04-29T08:45:55Z

@humphd this looks good to me, can you take a look?

humphd

I think this is fine, but I'd change the onPrompt signature.

humphd · 2024-04-29T12:23:27Z

src/components/MessagesView.tsx

@@ -99,7 +99,7 @@ function MessagesView({
          isLoading={isLoading}
          onResubmitClick={async (promptText?: string) => {
            await deleteMessages(message.id, "after");
-            onPrompt(promptText);
+            onPrompt(promptText, undefined, true); // pass prompt text and true for retry, don't include any imageURLs


A better pattern to use for this is to convert the args for onPrompt to be an Object:

async ({ prompt, imageUrls, retry = false }: { prompt?: string; imageUrls?; string[], retry?; boolean } = {}) => {

Now you can call it like this:

onPrompt({ prompt: promptText, retry: true });

This deals with undefined/optional args much better, and makes it self-documenting.

@humphd
I updated the onPrompt signature in ChatBase and MessagesView, but to get this working I also had to make the same updates to the onPrompt and onSendClick (and any calls to these) signatures in various other components due to errors like the ones below:

These changes still fix the duplication issues observed and preserve the other behaviours I've tested.
The onPrompt/onSendClick signature updates don't seem like they'll break anything (as long as any associated calls have also been updated accordingly), but I can't confirm anything at this time.

Should we put my recent commits (everything from bbdc1ab onward) on the backburner for a follow-up PR?

…g function results back to LLM

…nature

…ature

humphd

You could also define a reusable type for these options, since you have to use them in so many places.

humphd · 2024-05-01T22:01:41Z

src/Chat/ChatBase.tsx

+      prompt?: string;
+      imageUrls?: string[];
+      retry?: boolean;
+    }) => {


A trick you can use when all properties on the object are optional is to set the options object to have a default value:

async ({ prompt, imageUrls, retry = false, }: { prompt?: string; imageUrls?: string[]; retry?: boolean; } = {}) => {

The value of this is that it lets callers do onPrompt() and it will work.

Something like this?

async ({ prompt = "", imageUrls = [], retry = false, //...

No, see my code above. I'm setting the value to {} by default.

Oh! I see it now. My bad

I followed your advice and added a reusable type called OnPromptFunction.
I tried the original trick you suggested but got this error when I applied it to the signature on other files on some files:

So I ended up using a different syntax suggested by ChatCraft:

export type OnPromptFunction = (options?: { prompt?: string; imageUrls?: string[]; retry?: boolean; }) => void;

I wasn't sure where to place the reusable type, so I placed it in a new file, src/lib/OnPromptFunction.ts.

src/Chat/ChatBase.tsx

src/components/Message/FunctionResultMessage.tsx

src/Chat/ChatBase.tsx

humphd · 2024-05-05T14:37:58Z

src/Chat/ChatBase.tsx

+  const onPrompt: OnPromptFunction = useCallback(
+    async (options?: { prompt?: string; imageUrls?: string[]; retry?: boolean }) => {
+      let prompt = options?.prompt;
+      const { imageUrls, retry } = options || {};


I'm still confused why you don't use the pattern I mentioned and assign = {} as the default instead of doing this.

The pattern was fine for the onPrompt implementation in ChatBase, but when I tried using the same pattern throughout the onPrompt/onSendClick props TypeScript reports this error (props/type can't have default values):

To create a common type for both the onPrompt implementation and the onPrompt component props I had to use an options parameter and make that optional.

Although, I think if we apply the OnPromptFunction to the component props only, we can still use the original pattern in the onPrompt implementation, we just won't be able to turn that pattern into a reusable type because it uses a parameter initializer/default value(s)

humphd · 2024-05-05T14:38:24Z

src/components/PromptForm/index.tsx

@@ -5,11 +5,12 @@ import DesktopPromptForm from "./DesktopPromptForm";
 import useMobileBreakpoint from "../../hooks/use-mobile-breakpoint";
 import { useSettings } from "../../hooks/use-settings";
 import { ChatCraftChat } from "../../lib/ChatCraftChat";
+import { OnPromptFunction } from "../../lib/OnPromptFunction";


Move your exported types into this file.

I hope I did this correctly.
I put PromptFunctionOptions inside OnPromptFunction.ts with OnPromptFunction.

rjwignar added 3 commits April 24, 2024 22:23

added retry parameter to onPrompt

7c018f8

added prompt clearing on retry

8eb69f9

updated MessagesViewProps onPrompt signature to match ChatBase's onPr…

6b66b2f

…ompt definition, updated onPrompt call in onResubmitClick

rjwignar self-assigned this Apr 25, 2024

rjwignar marked this pull request as draft April 25, 2024 04:58

updated retry comment

961ce25

rjwignar marked this pull request as ready for review April 25, 2024 16:51

humphd reviewed Apr 29, 2024

View reviewed changes

rjwignar added 8 commits April 29, 2024 21:25

converted ChatBase::onPrompt args into an Object

bbdc1ab

updated retry to optional argument, updated onPrompt call when sendin…

094e32c

…g function results back to LLM

updated onSendClick signature

901eae1

updated onSendClick signature, and onSendClick calls to match new sig…

1d4e921

…nature

updated onSendClick signature and onSendClick calls to match new sign…

282d331

…ature

updated onPrompt signature

7bc0a99

updated onPrompt signature and any onPrompt calls to match new signature

490cebe

updated onPrompt call to match updated onPrompt signature in MessageBase

01a659c

humphd requested changes May 1, 2024

View reviewed changes

rjwignar added 7 commits May 3, 2024 10:42

updated onPrompt signature with default empty options object

0f0603a

updated onPrompt() call to match improved onPrompt signature

7f52314

updated MessageBaseProps onPrompt signature

a26772b

updated onPrompt() call in FunctionResultMessage

7973c16

updated onPrompt signature to match MessageBaseProps

1a98769

updated onPrompt signatures

c1ad0d7

updated onSendClick signatures in PromptForm components

7a3099d

rjwignar added 2 commits May 3, 2024 15:56

added OnPromptFunction type alias

049c488

replaced onSendClick/onPrompt signatures with OnPromptFunction

948acae

humphd requested changes May 5, 2024

View reviewed changes

rjwignar added 2 commits May 5, 2024 15:17

added PromptFunctionOptions

6c26e32

updated onPrompt signature with PromptFunctionOptions

f46f172

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clear regular prompt message if using Re-Ask Option #616

Clear regular prompt message if using Re-Ask Option #616

rjwignar commented Apr 25, 2024 •

edited

Loading

rjwignar commented Apr 25, 2024 •

edited

Loading

tarasglek commented Apr 25, 2024 •

edited

Loading

rjwignar commented Apr 25, 2024

cloudflare-workers-and-pages bot commented Apr 25, 2024 •

edited

Loading

tarasglek commented Apr 29, 2024

humphd left a comment

humphd Apr 29, 2024

rjwignar May 1, 2024 •

edited

Loading

humphd left a comment

humphd May 1, 2024

rjwignar May 1, 2024

humphd May 1, 2024

rjwignar May 1, 2024

rjwignar May 3, 2024

humphd May 5, 2024

rjwignar May 5, 2024 •

edited

Loading

humphd May 5, 2024

rjwignar May 5, 2024

	} else {
	// If there isn't any prompt text, see if the final message in the chat was a human
	// message or a function response. If it was either, we'll allow sending that through
	// again (e.g., if you modified something and want to retry, or want to share the
	// response from the function). Otherwise bail now.
	const finalMessage = chat.messages({ includeAppMessages: false }).at(-1);
	if (
	!(
	finalMessage instanceof ChatCraftHumanMessage \|\|
	finalMessage instanceof ChatCraftFunctionResultMessage
	)
	) {
	return;
	}
	}

	if (prompt) {
	// Add this prompt message to the chat
	promptMessage = new ChatCraftHumanMessage({ text: prompt, imageUrls, user });
	await chat.addMessage(promptMessage);

Clear regular prompt message if using Re-Ask Option #616

Are you sure you want to change the base?

Clear regular prompt message if using Re-Ask Option #616

Conversation

rjwignar commented Apr 25, 2024 • edited Loading

Background

Changes

Comparison to Production (updated with Image as Input observations)

rjwignar commented Apr 25, 2024 • edited Loading

tarasglek commented Apr 25, 2024 • edited Loading

rjwignar commented Apr 25, 2024

Future Considerations

cloudflare-workers-and-pages bot commented Apr 25, 2024 • edited Loading

Deploying chatcraft-org with Cloudflare Pages

tarasglek commented Apr 29, 2024

humphd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjwignar May 1, 2024 • edited Loading

Choose a reason for hiding this comment

humphd 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

rjwignar May 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rjwignar commented Apr 25, 2024 •

edited

Loading

rjwignar commented Apr 25, 2024 •

edited

Loading

tarasglek commented Apr 25, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Apr 25, 2024 •

edited

Loading

rjwignar May 1, 2024 •

edited

Loading

rjwignar May 5, 2024 •

edited

Loading