-
Notifications
You must be signed in to change notification settings - Fork 478
Fix/embedding traces UI #3665
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
base: main
Are you sure you want to change the base?
Fix/embedding traces UI #3665
Conversation
|
Vishesh Paliwal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
|
Someone is attempting to deploy a commit to the agenta projects Team on Vercel. A member of the Team first needs to authorize it. |
mmabrouk
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.
Hey @Vishesh-Paliwal
I am not sure the check that was done happened in the right place.
Can you please determine/describe the current flow (how and where we discover something is a chat span). If I remember correctly the conditions we had, looked for the span_type. Is that not the case?
Basically can you please update the PR with some research on the source of the issue and the reasoning behind where to put the solution
|
int the file : TraceDrawer/components/TraceContent/components/OverviewTabItem/index.tsx Coming to the problem at hand . because embeddings result also had structure similliar to chat detection logic , it was being rendered as chat . Hence I decided to just add one more check activeTrace?.span_type !== "embedding" so that embedding is treated differently . let me know if this looks good , Also One thing that comes to my mind seeing this kind of pattern is strategy pattern , we can also make common rendering logic available across web repo based on span types , if this is something you feel will be reused at many places now or later . |
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.
mmabrouk
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.
Thanks @Vishesh-Paliwal that makes total sense!
This PR aims to solve issue #3573
Problem: Embedding traces incorrectly shown with chat UI in trace drawer
fix : added a check to ensure only span_type != embeddings are shown with chat UI