open-sauced / app

πŸ• Insights into your entire open source ecosystem.
https://pizza.new
Apache License 2.0
381 stars 202 forks source link

feat: enabled loading an existing conversation in StarSearch #3563

Closed nickytonline closed 2 weeks ago

nickytonline commented 2 weeks ago

Description

Now you can share an existing thread (conversation in StarSearch). We had a rudimentary version of this previously (see #3324), but it was just sharing the prompt and running it.

These shared conversations are the full conversation that gets replayed and does not hit our AI backend at all.

If you are not logged in, you will only be able to replay it once (unless the page is refreshed in the browser) and any other actions will require you to log in.

One thing to note is if you generate a shared URL, it will generate for beta.app.opensauced.pizza on beta and deploy previews, and one in prod, will be prod links. All that said, you'll just need to replace the beta base URL with the deployment preview URL.

TODO:

Note: This PR adds Valibot to the project. It is MIT licensed and a small package (1kb). It's used for some validation in this PR and I plan to use it elsewhere in the application long term.

Related Tickets & Documents

Closes #3551

Mobile & Desktop Screenshots/Recordings

Steps to QA

Here's a few share links you can try:

Associated OG image, https://deploy-preview-3563--oss-insights.netlify.app/og-images/star-search/?share_id=900686cc-8926-47fd-a1d6-0e19da967f48

image

Associated OG image, https://deploy-preview-3563--oss-insights.netlify.app/og-images/star-search/?share_id=ff9b26b7-64e7-460b-9d33-252543bee24d

image

--

Also try and create a new conversation then try to share it.

Tier (staff will fill in)

[optional] What gif best describes this PR or how it makes you feel?

netlify[bot] commented 2 weeks ago

Deploy Preview for oss-insights ready!

Name Link
Latest commit 5faacee9ba08d6e7b6a741118386246d265e4f88
Latest deploy log https://app.netlify.com/sites/oss-insights/deploys/6671a45ecd50250008ad44ab
Deploy Preview https://deploy-preview-3563--oss-insights.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

netlify[bot] commented 2 weeks ago

Deploy Preview for design-insights ready!

Name Link
Latest commit 5faacee9ba08d6e7b6a741118386246d265e4f88
Latest deploy log https://app.netlify.com/sites/design-insights/deploys/6671a45e8252ba00094b0e81
Deploy Preview https://deploy-preview-3563--design-insights.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

BekahHW commented 2 weeks ago

I'm a little bit confused by this:

image

We're making it a two-step process to share and I think that's adding some friction. We already have a note at the bottom of the links that it makes it public. Can we remove this initial message?

One other thing is that when you load the shareable link, we see the starsearch input screen flash first. Could we do a loader or something else? Not sure if that's just bc of having to move between beta and deploy preview.

https://github.com/open-sauced/app/assets/34313413/6c9a6126-b37d-45b2-9473-e34e531114a1

But this is looking great! Super excited to get this shipped.

nickytonline commented 2 weeks ago

I'm a little bit confused by this:

image

We're making it a two-step process to share and I think that's adding some friction. We already have a note at the bottom of the links that it makes it public. Can we remove this initial message?

One other thing is that when you load the shareable link, we see the starsearch input screen flash first. Could we do a loader or something else? Not sure if that's just bc of having to move between beta and deploy preview.

Screen.Recording.2024-06-14.at.4.45.44.PM.mov But this is looking great! Super excited to get this shipped.

I'm taking a page from the ChatGPT book here. They have update link, but what they're doing is generating a public link.

CleanShot 2024-06-14 at 16 59 22

This wasn't part of the flow originally when I chatted with @isabensusan a couple days ago. The reason this flow is necessary is we run into issues with browser security if we try to open a link programmatically. iOS at least prevents a site from doing that, so I generate the share link then the share menu rerenders with the list of links.

The text below was there prior to this intermediate step. Definitely open to suggestions.

For the quick flash, I had something in place for that. I must have undone that. I will look into that.

nickytonline commented 2 weeks ago

@BekahHW, the flash of the StarSearch header is gone if a shared conversation loads now.

nickytonline commented 2 weeks ago

As discussed in standup, I'm going to bring back the functionality to load an initial prompt via the prompt querystring key/value pair so pre-threaded history shares still work. The onlu caveat is if you're not logged in, you'll have to log in. A small price considering we don't have many of the current shared prompts floating around.

nickytonline commented 2 weeks ago

I'm a little bit confused by this:

image

We're making it a two-step process to share and I think that's adding some friction. We already have a note at the bottom of the links that it makes it public. Can we remove this initial message?

One other thing is that when you load the shareable link, we see the starsearch input screen flash first. Could we do a loader or something else? Not sure if that's just bc of having to move between beta and deploy preview.

https://github.com/open-sauced/app/assets/34313413/6c9a6126-b37d-45b2-9473-e34e531114a1

But this is looking great! Super excited to get this shipped.

As mentioned @BekahHW, the flash of the StarSearch header has been taken care of.

For the create link, it's a step that actually makes the thread public. Maybe the button should say make conversation public or singing song those lines?

BekahHW commented 2 weeks ago

@nickytonline I tried the Brandon example that you shared. It's interesting that it says the query is from me. Do we want that to show the original poster?

image

nickytonline commented 2 weeks ago

@nickytonline I tried the Brandon example that you shared. It's interesting that it says the query is from me. Do we want that to show the original poster?

image

That is a good point. We currently don't surface that in the thread history, but we could derive it. I think as a first pass, I could just show the StarSearch logo for a shared thread history. wdyt?

BekahHW commented 2 weeks ago

When you share a thread, the person who opens the thread still has the ability to add a query, but bc it isn't their thread, they get this response and it seems broken. We should put something in place that prevents them from querying.

I think if you don't "own" the thread, then the input box either shouldn't be there or be disabled with a message about why. Something like, "This thread belongs to X. If you would like more information, start your own conversation with StarSearch."

image

nickytonline commented 2 weeks ago

When you share a thread, the person who opens the thread still has the ability to add a query, but bc it isn't their thread, they get this response and it seems broken. We should put something in place that prevents them from querying.

I think if you don't "own" the thread, then the input box either shouldn't be there or be disabled with a message about why. Something like, "This thread belongs to X. If you would like more information, start your own conversation with StarSearch."

image

Really great catch here @BekahHW! I wouldn't remove the input box from a UX perspective as that might confuse the user, but we could disabled it, and have a message above it explaining why like you mention in you alternative suggestion.

I'm going to go ahead and implement that.

There is an edge case where the currently logged in user might own the thread, so we could punt on that edge case for now. I have some ideas, but I think it's best to get this big chunk of work in first.

nickytonline commented 2 weeks ago

Just putting in this draft so I can implement some of the feedback as there is already two approvals.

BekahHW commented 2 weeks ago

@nickytonline I tried the Brandon example that you shared. It's interesting that it says the query is from me. Do we want that to show the original poster? image

That is a good point. We currently don't surface that in the thread history, but we could derive it. I think as a first pass, I could just show the StarSearch logo for a shared thread history. wdyt?

I think that's a good fix for now.

nickytonline commented 2 weeks ago

@nickytonline I tried the Brandon example that you shared. It's interesting that it says the query is from me. Do we want that to show the original poster? image

That is a good point. We currently don't surface that in the thread history, but we could derive it. I think as a first pass, I could just show the StarSearch logo for a shared thread history. wdyt?

I think that's a good fix for now.

Fixed by 6750294358030ce8409c765ddf182fdbe1305880

jpmcb commented 2 weeks ago

Somewhat unrelated, the deploy preview seems to have this weird justified-left bug:


Screenshot 2024-06-18 at 8 43 31 AM Screenshot 2024-06-18 at 8 43 47 AM

I feel like this was fixed in another PR recently ... maybe updating with beta will just fix it ℒ️ ?

nickytonline commented 2 weeks ago

Somewhat unrelated, the deploy preview seems to have this weird justified-left bug:

Screenshot 2024-06-18 at 8 43 31 AM Screenshot 2024-06-18 at 8 43 47 AM I feel like this was fixed in another PR recently ... maybe updating with beta will just fix it ℒ️ ?

Getting latest has fixed this.

nickytonline commented 2 weeks ago

When you share a thread, the person who opens the thread still has the ability to add a query, but bc it isn't their thread, they get this response and it seems broken. We should put something in place that prevents them from querying.

I think if you don't "own" the thread, then the input box either shouldn't be there or be disabled with a message about why. Something like, "This thread belongs to X. If you would like more information, start your own conversation with StarSearch."

image

Fixed by 5faacee9ba08d6e7b6a741118386246d265e4f88 @BekahHW

CleanShot 2024-06-18 at 11 17 49