Skip to content

feat: added the ability to select tags for multiple tags#2254

Merged
andrescrz merged 4 commits intocomet-ml:mainfrom
Gmin2:multi-trace-tag
Jul 14, 2025
Merged

feat: added the ability to select tags for multiple tags#2254
andrescrz merged 4 commits intocomet-ml:mainfrom
Gmin2:multi-trace-tag

Conversation

@Gmin2
Copy link
Copy Markdown
Contributor

@Gmin2 Gmin2 commented May 25, 2025

Details

Added the ability to Tag multiple traces at once within a Project

Issues

Resolves #1009

Testing

Screencast.from.2025-05-25.16-52-38.mp4

/claim #1009

@vincentkoc
Copy link
Copy Markdown
Member

@Gmin2 will review as soon as possible. Thanks for your contribution Can you verbally agree to the CLA.md with "i accept the CLA" in a comment please.

@Gmin2
Copy link
Copy Markdown
Contributor Author

Gmin2 commented May 25, 2025

i accept the CLA

@vincentkoc
Copy link
Copy Markdown
Member

@Gmin2 thanks. Changes look good, thanks for the video also. Will get one of the product engineers to take a look and review over the next few days.

Copy link
Copy Markdown
Collaborator

@aadereiko aadereiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution! The changes look good. There's just one thing I'd like to change: the way the success message is displayed.

@aadereiko aadereiko requested a review from Copilot May 27, 2025 08:24
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR introduces the ability to add tags to multiple traces (or spans) within a project in one go. Key changes include:

  • Adding a clear row selection callback in TracesSpansTab to reset selections.
  • Integrating an AddTagDialog component in the TracesActionsPanel with a new optional onClearSelection prop.
  • Creating the AddTagDialog component to handle batch tag additions via individual mutation calls.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
apps/opik-frontend/src/components/pages/TracesPage/TracesSpansTab/TracesSpansTab.tsx Added a clear selection callback and passed it to child export component.
apps/opik-frontend/src/components/pages/TracesPage/TracesSpansTab/TracesActionsPanel.tsx Updated import order, added AddTagDialog integration, and included an onClearSelection prop.
apps/opik-frontend/src/components/pages-shared/traces/AddTagDialog/AddTagDialog.tsx Introduced a new dialog component for adding tags to multiple rows with individual mutation calls.

Copy link
Copy Markdown
Collaborator

@aadereiko aadereiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've discussed it with our FE team and decided that we need to do updates of traces and spans in batches here. So, it will likely require the BE work as well. Will you consider doing it?

@Gmin2
Copy link
Copy Markdown
Contributor Author

Gmin2 commented May 27, 2025

We've discussed it with our FE team and decided that we need to do updates of traces and spans in batches here. So, it will likely require the BE work as well. Will you consider doing it?

yes, can yu share how you have plan to do that?

@aadereiko
Copy link
Copy Markdown
Collaborator

@Gmin2

yes, can yu share how you have plan to do that?

So, implementing the back-end part to support batch updates should also be included in this PR and should be done by the person seeking the bounty. Could you implement the back-end part?

@vincentkoc vincentkoc requested a review from aadereiko May 30, 2025 08:22
@aadereiko
Copy link
Copy Markdown
Collaborator

@Gmin2
We've discussed this PR within our team and realized that the backend changes were not part of the initial requirements. We'll handle those improvements ourselves later. On your side, you can simply add a limit of up to 10 traces.

Also, you have some eslint issue:

Error:    49:29  error  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any
Error:    68:13  error  Insert `,`                                prettier/prettier
Error:    85:[13](https://github.com/comet-ml/opik/actions/runs/15342488982/job/43181043429?pr=2254#step:5:14)  error  Insert `,`                                prettier/prettier
Error:   105:15  error  'error' is defined but never used         @typescript-eslint/no-unused-vars
Error:   [14](https://github.com/comet-ml/opik/actions/runs/15342488982/job/43181043429?pr=2254#step:5:15)2:29  error  Insert `⏎`                                prettier/prettier

Please, before submitting a PR, run npm run lint from opik-frontend repo to see what stylistic error you need to fix :)

@Gmin2
Copy link
Copy Markdown
Contributor Author

Gmin2 commented Jun 3, 2025

Resolved the linting issues !

@Gmin2
Copy link
Copy Markdown
Contributor Author

Gmin2 commented Jun 11, 2025

can i get approved on this?

@aadereiko
Copy link
Copy Markdown
Collaborator

hey @Gmin2 , could you please add limits of adding tags to up to 10 entities and we can approve?

@Gmin2
Copy link
Copy Markdown
Contributor Author

Gmin2 commented Jun 13, 2025

hey @Gmin2 , could you please add limits of adding tags to up to 10 entities and we can approve?

hey @aadereiko just done that could you review it once !

@aadereiko
Copy link
Copy Markdown
Collaborator

@Gmin2
we'll add error handling on our end and merge this PR soon, thanks! I guess your work here is done
FYI @vincentkoc

@Gmin2
Copy link
Copy Markdown
Contributor Author

Gmin2 commented Jun 18, 2025

@Gmin2 we'll add error handling on our end and merge this PR soon, thanks! I guess your work here is done FYI @vincentkoc

cc @vincentkoc

@vincentkoc
Copy link
Copy Markdown
Member

@Gmin2 yes I have seen this. Wont be long once the team make the additional changes and this PR is merged you will be paid out. Easier to keep to this approach, but as discussed consider this one locked in. If any delays on our side I will process the bounty manually.

@aadereiko aadereiko self-assigned this Jun 25, 2025
Copy link
Copy Markdown
Contributor

@andriidudar andriidudar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @Gmin2 ,
Thank you for your recent changes and for contributing to the Opik project — much appreciated!

I’ve reviewed and approved your PR, and it’s now been merged. Great work!

@andriidudar andriidudar removed the request for review from aadereiko July 14, 2025 09:00
@andrescrz andrescrz requested a review from aadereiko July 14, 2025 09:01
@andrescrz andrescrz dismissed aadereiko’s stale review July 14, 2025 09:01

Reviewed by other team member.

@andrescrz andrescrz merged commit 1a7216c into comet-ml:main Jul 14, 2025
1 check passed
rmenziejr pushed a commit to rmenziejr/opik that referenced this pull request Jul 17, 2025
* feat: added the ability to select tags for multiple tags

* fix: introduced an array to store all the mutation promises

* fix: linting issues resolved

* feat: limit aadding of tags to 10 entities
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR]: UI - Ability to Tag multiple traces at once within a Project

6 participants