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

Fix Web Handlers not executing with Re-ask option #581

Merged
merged 2 commits into from
Apr 12, 2024
Merged

Conversation

Amnish04
Copy link
Collaborator

In #519, I implemented initial support for Web Handlers, which extend ChatCraft's URL handling capabilities.

However, @tarasglek noticed that when a non-matching prompt is edited to a valid url pattern that is configured in Web Handlers configuration, prompt is still sent to LLM instead of invoking the corresponding Web Handler.

I researched a bit and found that the reason for this to happen was that the Resubmit handler function for the message form was not passing the prompt text to the onPrompt function.

This is why the handler wasn't able to determine any specifics about the prompt. I am now passing the text to that handler so onPrompt in ChatBase knows that it needs to be passed to a Web Handler.

Before:

onResubmitClick={async () => {
  await deleteMessages(message.id, "after");
  onPrompt();
}}

After:

onResubmitClick={async (promptText?: string) => {
  await deleteMessages(message.id, "after");
  onPrompt(promptText);
}}

This fixes #541

@Amnish04 Amnish04 self-assigned this Apr 11, 2024
@Amnish04 Amnish04 requested a review from tarasglek April 11, 2024 23:29
@rjwignar rjwignar self-requested a review April 12, 2024 04:30
Copy link
Collaborator

@rjwignar rjwignar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Amnish04 The fix looks great and I can confirm it works:
Production Bug (re-asking with a valid PR URL sends message to LLM):
webHandlerBugProd

Observed Fix (re-asking with a valid PR URL prompts web handler):
webHandlerBugFix

Before I approve this, can you address my comment?

src/components/Message/MessageBase.tsx Outdated Show resolved Hide resolved
Copy link

cloudflare-workers-and-pages bot commented Apr 12, 2024

Deploying chatcraft-org with  Cloudflare Pages  Cloudflare Pages

Latest commit: a3c31cc
Status: ✅  Deploy successful!
Preview URL: https://1fd7ec67.console-overthinker-dev.pages.dev
Branch Preview URL: https://amnish04-issue-541.console-overthinker-dev.pages.dev

View logs

@Amnish04 Amnish04 force-pushed the amnish04/issue-541 branch from 71992d3 to a3c31cc Compare April 12, 2024 13:03
@Amnish04
Copy link
Collaborator Author

@rjwignar I've fixed the typo you mentioned

@rjwignar
Copy link
Collaborator

@Amnish04 Thanks for clearing that up!
LGTM!

@Amnish04
Copy link
Collaborator Author

@rjwignar Great, thanks for reviewing!

@Amnish04 Amnish04 merged commit 7cc1e2f into main Apr 12, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking Re-ask button should check if the updated text needs to be handled by a Web Handler
2 participants