Implement initial version of web handlers #519

Amnish04 · 2024-03-21T23:23:09Z

This is my initial attempt in implementing the idea of what we can call web handlers as discussed in #507 .

I have:

Written a WebHandler class registering/searching web handlers and executing their logic i.e. sending request to handler url based on request config (so far request method)
Added logic to check if the prompt matches with match pattern of one of the registered Web Handlers. The search is sequential as mentioned in the issue, and the first matched web handler processes the prompt url. If there is no match, the prompt is released to the regular control flow (slash command or sent to AI).
Currently, I have hard coded 2 Web Handlers as mentioned in issue, but we can later create UI to configure as many as user wants in localStorage.

Demo:

Prompt: https://github.com/tarasglek/chatcraft.org/pull/519

What I don't like is how we are currently allowing prompt like import <url> to be registered as match patterns. This causes ambiguity and I had to write specialized logic to handle edge cases like

url = extractFirstUrl(url) ?? ""; // When the input is not a url itself

as an example.

I feel like we should restrict web handler match patterns to just urls, and let import command serve its own purpose.

Maybe I am wrong and don't understand fully where we are going with this.

@humphd @tarasglek Please let me know what you think.

tarasglek · 2024-03-22T12:36:52Z

Prompt: #519 <-- seems like wrong url?

tarasglek · 2024-03-22T12:38:01Z

src/Chat/ChatBase.tsx

@@ -156,6 +157,26 @@ function ChatBase({ chat }: ChatBaseProps) {
        prompt = "/help";
      }

+      // If we have a web handler registered for this url
+      if (prompt && WebHandler.getMatchingHandler(prompt)) {


does prompt here mean current chat message?

Yes, I think promptText is a better name. But it's already in use later in the same function

tarasglek · 2024-03-22T12:38:25Z

src/Chat/ChatBase.tsx

@@ -156,6 +157,26 @@ function ChatBase({ chat }: ChatBaseProps) {
        prompt = "/help";
      }

+      // If we have a web handler registered for this url
+      if (prompt && WebHandler.getMatchingHandler(prompt)) {
+        const handler = WebHandler.getMatchingHandler(prompt);


should do const handler above if and then reuse it

Makes sense, just pushed in latest commit

tarasglek · 2024-03-22T12:40:27Z

src/lib/WebHandler.ts

+      new WebHandler({
+        handlerUrl: "https://taras-scrape2md.web.val.run/",
+        method: HttpMethod.GET,
+        matchPattern: /^https:\/\/\S+/,


you should fix match pattern to have $ at end to match entire message only

Thanks for noticing, I've made the change.

humphd · 2024-03-22T14:36:27Z

Not critical, but I'd vote to get rid of /proxy ... style regexes as you wrote above, and only do this for URLs. Leave /slash commands for built-in things, and this is something different.

WebHandler is a nice name for this. I'm excited to see this happen so quickly.

cloudflare-workers-and-pages · 2024-03-22T16:07:39Z

Deploying chatcraft-org with Cloudflare Pages

Latest commit:	`d4514ca`
Status:	✅ Deploy successful!
Preview URL:	https://634725c3.console-overthinker-dev.pages.dev
Branch Preview URL:	https://amnish04-issue-507.console-overthinker-dev.pages.dev

View logs

Amnish04 · 2024-03-22T16:10:43Z

@humphd Agreed, I've removed the /proxy handler. Maybe we can have further validation for regex patterns in a future PR where we implement UI for handler registration

tarasglek · 2024-03-22T18:41:18Z

src/Chat/ChatBase.tsx

+
+      if (prompt && handler) {
+        try {
+          const result = await handler!.executeHandler(prompt); // We know handler is always there


would just do const executedHandlerResult = WebHandler.executeHandler(prompt ?? "");

s/prompt/message/

eg should just execute first matching handler

i think you also wanna set some sort of network spinner given that you doing network io

i see when i run it, there is a spinner..don't understand why it works :)

would just do const executedHandlerResult = WebHandler.executeHandler(prompt ?? "");

That will work, but I would still have to have a function to know if the prompt text matches a web handler.

tarasglek · 2024-03-22T18:43:39Z

src/lib/WebHandler.ts

+    this.matchPattern = matchPattern;
+  }
+
+  isMatchingHandler(url: string) {


s/url/message/ almost everywhere

Sorry I didn't get that :)

Ohhh, you mean I should rename url to message

Didn't realize you were talking in sed command 😅

Just pushed 9cf7f2d

tarasglek · 2024-03-22T18:44:35Z

src/lib/WebHandler.ts

+      `${command}\n\n` +
+      // If it's already markdown, dump it into the message as is;
+      // otherwise, wrap it in a code block with appropriate type
+      (type === "markdown" ? content : `\`\`\`${type}\n${content}` + `\n\`\`\``);


i think text/plain is also ok to show directly

Yes, I noticed it doesn't make any difference if I skip this step. Will make the change

@tarasglek Just pushed the changes

tarasglek · 2024-03-22T18:46:49Z

src/lib/WebHandler.ts

+  }
+
+  async executeHandler(url: string): Promise<string> {
+    url = extractFirstUrl(url) ?? ""; // When the input is not a url itself


i don't understand what this code is doing. needs a comment re algo here

This was for handling non-url prompts. But now that we have decided to not allow those, we can remove this

tarasglek · 2024-03-22T18:49:23Z

src/lib/WebHandler.ts

+      new WebHandler({
+        handlerUrl: "https://taras-scrape2md.web.val.run/",
+        method: HttpMethod.GET,
+        matchPattern: /^https:\/\/\S+$/,


i think you also need to make this pattern do /^(https://\S+)$/

so the algo is you apply regexp..then if it has a match group, eg ()..pass that to url. if it doesn't just pass whole message(tho that should only work for post i think)

you can then get fancy and if regexp uses named captch groups ala https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Named_capturing_group then you can pass them as matchName=matchValue form

i think you also need to make this pattern do /^(https://\S+)$/

Do you mean wrapping in parenthesis?

so the algo is you apply regexp..then if it has a match group, eg ()..pass that to url. if it doesn't just pass whole message(tho that should only work for post i think)

Yes, I go through each registered Web Handler, and try to match the prompt against match pattern of the handler. If one matches, I invoke that handler's configured handlerUrl which takes the prompt text as url query param for now.

Maybe we can make how the prompt is sent with request configurable too in the future

tarasglek · 2024-03-22T18:52:58Z

I would love it if you could expose this as a webhandler section in prefs(maybe hidden by default under advanced), so i could enter yaml directly

tarasglek · 2024-03-22T18:53:28Z

Until it's editable there isn't too much value in specifying handlers this way

tarasglek · 2024-03-22T18:56:42Z

if i put

https://github.com/tarasglek/chatcraft.org/pull/519/files some trash

into a message. then it's handled as a call to llm.

if i do edit, cut it down to

https://github.com/tarasglek/chatcraft.org/pull/519/files

and press re-ask it's also passed to llm. Should be webhandled

Amnish04 · 2024-03-22T21:51:49Z

I would love it if you could expose this as a webhandler section in prefs(maybe hidden by default under advanced), so i could

I was actually thinking of making it similar to how Functions are done.

We could have an accordion item labelled "Web Handlers"

And each web handler item would lead to a separate page with a web handler config form anf all those configs could be stored in user's localStorage.

Amnish04 · 2024-03-22T22:14:17Z

if i put
https://github.com/tarasglek/chatcraft.org/pull/519/files some trash
into a message. then it's handled as a call to llm.

if i do edit, cut it down to
https://github.com/tarasglek/chatcraft.org/pull/519/files
and press re-ask it's also passed to llm. Should be webhandled

@tarasglek Nice catch, we should fix it. Can we do it in a follow up?

tarasglek · 2024-03-25T10:41:58Z

I would love it if you could expose this as a webhandler section in prefs(maybe hidden by default under advanced), so i could

I was actually thinking of making it similar to how Functions are done.

We could have an accordion item labelled "Web Handlers"

And each web handler item would lead to a separate page with a web handler config form anf all those configs could be stored in user's localStorage.

I am not super excited about ui for this. I would like the web handlers to be exposed as a yaml configuration so I could copy/paste it between devices, share it with friends. The UI is less practical than text here. If we had a ui as a higher level layer to generate that config, that's fine, but then you have to worry about round-trip editing, etc. Seems not worth it.

tarasglek · 2024-03-25T10:42:40Z

We can delay re-ask to followup, but webhandlers(agree this is a cool name) should land with config editing.

Amnish04 · 2024-03-30T00:02:30Z

@tarasglek @humphd Just pushed some new changes.

The user can now enter YAML configurations for Web Handlers (as Taras suggested) with a modal that a pops up by clicking the new Web Handlers menu item in our profile image drop menu.

Configuration Modal:

This config is persisted in localStorage

with the help of a use-web-handlers hook I wrote for its management.

Now, we can configure as many patterns we want to and the first match is executed.

Based on my config, if I enter a Youtube Url the request is redirected to the api of one of my projects (I have intentionally set CORS headers to *).

Result:

humphd · 2024-03-30T13:24:14Z

Can you change the font in that textarea to be monospaced?

Another idea (follow-up) would be to use the code I already have for the CodeMirror editor that we use in functions, so we get a better dev experience.

Amnish04 · 2024-03-30T16:02:51Z

@humphd Agreed, I should have used that in the first place. Just filed a follow up.

Also, I've changed the fontFamily of TextArea to monospace.

I think we should extend this functionality by allowing to configure more options in follow-ups. For example, how the message should be sent to the handler url - Body or Query Param. If query param, then what should it be called.

Please let me know if you have any more suggestions.

tarasglek · 2024-04-01T12:58:42Z

tried this out, mostly worked great!
I would prefer to put web handlers as a section in main options dialog, but this is good to land as is. can do that as a follow up

Small issue:

https://amnish04-issue-507.console-overthinker-dev.pages.dev/api/share/chatcraft_dev/zPCmCwNdNudNGP32X05tV
is breaking up the web handler link weird

Amnish04 · 2024-04-03T16:05:20Z

@tarasglek Thanks for reviewing! I am trying to avoid having it in the User Settings as the modal is going to be too long. And since Web Handlers could potentially grow in complexity, I would prefer it be separate thing :)

Also, we were disusing the weird Markdown today in the meeting. Not sure, but we're probably adding redundant backticks somewhere. I just filed #547 for that

Amnish04 requested review from humphd and tarasglek March 21, 2024 23:23

Amnish04 self-assigned this Mar 21, 2024

Amnish04 marked this pull request as draft March 21, 2024 23:32

Amnish04 marked this pull request as ready for review March 21, 2024 23:33

tarasglek reviewed Mar 22, 2024

View reviewed changes

Amnish04 added this to the Release 1.6 milestone Mar 22, 2024

tarasglek reviewed Mar 22, 2024

View reviewed changes

Amnish04 force-pushed the amnish04/issue-507 branch from e11439f to 197ca00 Compare March 22, 2024 22:25

Amnish04 modified the milestones: Release 1.6, Release 1.7 Mar 23, 2024

Amnish04 mentioned this pull request Mar 30, 2024

Use CodeMirror editor for Web Handlers configuration #540

Closed

Amnish04 added 7 commits March 30, 2024 12:06

Implement initial version of web handlers

a7126b4

Remove "/proxy" handler and refactoring

3b899b9

Refactor and remove unnecessary code

d0b9e16

s/url/message/ 😜

cb8189e

Remove unused code

d91970a

Implement configuration of Web Handlers by Users

3e3fa87

Change font family to monospace for Web Handlers textarea

d4514ca

Amnish04 force-pushed the amnish04/issue-507 branch from 532e5b4 to d4514ca Compare March 30, 2024 16:43

Amnish04 mentioned this pull request Mar 30, 2024

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

Closed

mingming-ma modified the milestones: Release 1.7, Release 1.8 Mar 31, 2024

humphd requested a review from tarasglek March 31, 2024 22:41

Amnish04 mentioned this pull request Apr 3, 2024

Markdown responses messed up with redundant Backticks #547

Open

Amnish04 merged commit ae1e9f1 into main Apr 3, 2024
4 checks passed

Amnish04 mentioned this pull request Apr 11, 2024

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

Merged

Implement initial version of web handlers #519

Implement initial version of web handlers #519

Conversation

Amnish04 commented Mar 21, 2024 • edited by tarasglek Loading

tarasglek commented Mar 22, 2024

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

humphd commented Mar 22, 2024

cloudflare-workers-and-pages bot commented Mar 22, 2024 • edited Loading

Deploying chatcraft-org with Cloudflare Pages

Amnish04 commented Mar 22, 2024

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

tarasglek Mar 22, 2024 • edited Loading

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

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

tarasglek commented Mar 22, 2024

tarasglek commented Mar 22, 2024

tarasglek commented Mar 22, 2024

Amnish04 commented Mar 22, 2024

Amnish04 commented Mar 22, 2024

tarasglek commented Mar 25, 2024

tarasglek commented Mar 25, 2024

Amnish04 commented Mar 30, 2024

humphd commented Mar 30, 2024

Amnish04 commented Mar 30, 2024 • edited Loading

tarasglek commented Apr 1, 2024 • edited Loading

Amnish04 commented Apr 3, 2024 • edited Loading

Amnish04 commented Mar 21, 2024 •

edited by tarasglek

Loading

cloudflare-workers-and-pages bot commented Mar 22, 2024 •

edited

Loading

tarasglek Mar 22, 2024 •

edited

Loading

Amnish04 commented Mar 30, 2024 •

edited

Loading

tarasglek commented Apr 1, 2024 •

edited

Loading

Amnish04 commented Apr 3, 2024 •

edited

Loading