KastanDay / ai-ta-frontend

Chat with your documents. Upload anything, get answers.
https://www.uiuc.chat/
15 stars 7 forks source link

Add API flag `retrieval_only` - retrieving ONLY contexts, no LLM invoked #182

Closed rohan-uiuc closed 3 weeks ago

rohan-uiuc commented 1 month ago

Contexts are returned by default on non-streaming API. Added flag retrieval_only

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
chat-illinois-edu ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 8:43pm
uiuc-chat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 8, 2024 8:43pm
rohan-uiuc commented 3 weeks ago

Yep, docs is next, starting that. RE:it would be great to add this to the non-streaming API by default. It is added to the non streaming API by default in the last commit.

KastanDay commented 3 weeks ago

Ok I modified the examples on /api

KastanDay commented 3 weeks ago

Added a link to our docs page, too: CleanShot 2024-10-08 at 12 46 31

rohan-uiuc commented 3 weeks ago

The model we host can change any time, we will have to modify this everytime that happens. Hence, 4o-mini in the example request. "How to use NCSA hosted models" is already present in the documentation. Additionally, the performance with 4o-mini is better so I would prefer that as a default. The model mentions llama but there is a field for openai_key in the default example which will only confuse the user. All the various combinations were kept in the documentation to keep default request simple.

KastanDay commented 3 weeks ago

Ok that's a convincing argument. I agree with gpt-4o-mini perf being so much better that we should keep it default.

rohan-uiuc commented 3 weeks ago

Screenshot 2024-10-08 at 3 22 47 PM Screenshot 2024-10-08 at 3 23 04 PM

This information is already available in the documentation.

KastanDay commented 3 weeks ago

Screenshot 2024-10-08 at 3 22 47 PM Screenshot 2024-10-08 at 3 23 04 PM

This information is already available in the documentation.

Agree, but I still think it's worth a callout here. So I'm voting to keep it for now.

rohan-uiuc commented 3 weeks ago

Screenshot 2024-10-08 at 3 22 47 PM Screenshot 2024-10-08 at 3 23 04 PM This information is already available in the documentation.

Agree, but I still think it's worth a callout here. So I'm voting to keep it for now.

Can we move this to a separate PR, do a review of the UI and then push this? I believe this PR is ready to merge without these changes. The new changes donot match fonts styles, increase the text by a lot on this page and are not related to the fix at all, the weird black color is hurting my eyes.

KastanDay commented 3 weeks ago

I like the black code formatting. I could make the contrast less intense if you insist. But we use it elsewhere.

CleanShot 2024-10-08 at 13 41 15

Let's just merge it as is. The styling isn't bad.