-
-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Enhanced Chat Embed History View #4281
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
Conversation
Improved "Thinking" View
|
This update includes several follow-up fixes and refinements to the recently introduced features in the Chat Embed History view, addressing issues that were discovered during testing. Key Changes
These refinements have significantly improved the stability and reliability of the new features, providing a much smoother and more polished user experience. |
shatfield4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionality wise, this works great! I do like what you did here with getting the markdown to render properly in the chat embed settings page. We are going to want to stay away from how you implemented the sessions endpoint in the backend here. Although functionally it works great, this does add a lot of complexity to how this is talking back to the frontend.
I do like the idea you have here about using colors for each of the sessions but lets remove this since the way it is currently setup, if we have more sessions that the colors available then colors will be repeated and be confusing to users.
In order to get this merged, let's do the following:
-
Remove the sorting by sessions for now, we can do this later in another PR just due to the complexity this adds to the current feature
-
Let's also revert the backend changes you have here too for getting all unique sessions. What you can do in the future is append this to the /embed/chats endpoint in each call since they are limited to 20 items for pagination anyways and this will not query the entire DB for all unique sessions, just the 20 items it is returning
-
In the frontend, leave what you did here for getting markdown to render properly but let's remove the colored sessions
-
The show thinking checkbox is great but when choosing the color for showing thinking, let's not use a hardcoded value here but check out in our main frontend
index.cssandtailwind.config.jsso that you can see how we make theme variable so that the colors work for both default and light themes
Great work so far, and always remember that simpler is better since this will help our codebase be maintainable and easily readable for future developers that work on this.
|
Thanks for the great feedback. I've updated the PR based on your suggestions:
These changes simplify the implementation while keeping the core features. Let me know what you think. |
shatfield4
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made some changes to the UI, LGTM
Pull Request Type
Relevant Issues
resolves #4280
What is in this change?
This pull request introduces several new features and improvements to the chat embed history page, enhancing its usability and providing more powerful tools for reviewing chat logs.
Markdown Rendering for Responses:
MarkdownRenderercomponent has been created to handle the parsing and rendering of markdown, leveraging the existingmarkdown-itlibrary."Thinking" Step Visibility Toggle:
<think>tags in the model's response. This is useful for debugging and understanding the model's thought process without cluttering the main view.Enlarged Response Modal:
Additional Information
Developer Validations
yarn lintfrom the root of the repo & committed changes