feat(ui): bulk annotate traces and spans from the selection bar - #7940
feat(ui): bulk annotate traces and spans from the selection bar#7940adsqx wants to merge 2 commits into
Conversation
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| await api.put(`${TRACES_REST_ENDPOINT}feedback-scores`, { | ||
| scores: scoresChunk.map((score) => ({ | ||
| id: score.id, | ||
| // The backend groups the batch by project name and derives project_id from it, | ||
| // falling back to the default project when it is blank, so it has to be sent. | ||
| project_name: projectName, | ||
| name: score.name, |
There was a problem hiding this comment.
Cross-project feedback scores are persisted
Each chunk sends id: score.id and project_name: projectName unchecked, so mergeProjectsAndScores can assign an entity from another project or workspace to the requested project_id — should we verify each ID against the authenticated workspace and project before inserting, or reject the batch?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/api/traces/useTraceFeedbackScoreBatchSetMutation.ts` around
lines 29-35, review the `mutationFn` payload that sends each trace ID together with the
caller-supplied `projectName`; this pair is not sufficient to establish ownership and
cannot be trusted for authorization. Update the corresponding backend batch
feedback-score endpoint and span equivalent to resolve the requested project and
authenticated workspace, verify every submitted entity ID belongs to both, and reject
the entire batch on any mismatch before insertion. Add regression tests covering
cross-project and cross-workspace IDs.
| await queryClient.invalidateQueries({ queryKey: [TRACES_KEY] }); | ||
| await queryClient.invalidateQueries({ queryKey: ["traces-columns"] }); | ||
| await queryClient.invalidateQueries({ queryKey: ["traces-statistic"] }); | ||
|
|
||
| const traceIds = [...new Set(variables.scores.map((score) => score.id))]; | ||
| await Promise.all( | ||
| traceIds.map((traceId) => | ||
| queryClient.invalidateQueries({ | ||
| queryKey: [TRACE_KEY, { traceId }], | ||
| }), | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Bulk annotations leave dependent views stale
useSpanFeedbackScoreBatchSetMutation omits [TRACES_KEY], traces-columns, traces-statistic, and experiment caches, while useTraceFeedbackScoreBatchSetMutation omits experiment-items-statistic, experiments-columns, `[
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/api/traces/useTraceFeedbackScoreBatchSetMutation.ts` around
lines 54-65, update the `onSettled` cache invalidation logic to also invalidate
`experiment-items-statistic`, `experiments-columns`, `["experiment"]`, and
`[COMPARE_EXPERIMENTS_KEY]`, matching `useTraceFeedbackScoreSetMutation`. In
`apps/opik-frontend/src/api/traces/useSpanFeedbackScoreBatchSetMutation.ts` around lines
55-59, add the missing `[TRACES_KEY]`, `traces-columns`, and `traces-statistic`
invalidations plus the same experiment-facing keys, so both batch mutations refresh
every cache consumed by trace, span, and experiment views.
| {canAnnotateTraceSpanThread && ( | ||
| <AddAnnotationDialog | ||
| key={`annotate-${resetKeyRef.current}`} | ||
| rows={selectedRows} | ||
| open={open === 5} |
There was a problem hiding this comment.
The annotate action hard-codes 5 in open === 5 and setOpen(5), so adding or reordering actions can target the wrong dialog — should we define ANNOTATE_ACTION = 5 and use it in both places?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/v2/pages-shared/traces/TracesActionsPanel/TracesActionsPanel.tsx
around lines 151-155, refactor the `TracesActionsPanel` annotate action to avoid the raw
numeric sentinel `5`. Define a named constant (e.g. `ANNOTATE_ACTION = 5`) or typed
action enum near the action state declarations, and use it consistently in the
`AddAnnotationDialog` open check (`open === 5`) and the annotate button's `setOpen(5)`
call around lines 185-193, so future action additions or reordering are safe.
| return useMutation({ | ||
| mutationFn: async ({ | ||
| projectName, | ||
| scores, | ||
| }: UseSpanFeedbackScoreBatchSetMutationParams) => { | ||
| for (const scoresChunk of chunk(scores, MAX_FEEDBACK_SCORES_PER_BATCH)) { | ||
| await api.put(`${SPANS_REST_ENDPOINT}feedback-scores`, { | ||
| scores: scoresChunk.map((score) => ({ | ||
| id: score.id, | ||
| // The backend groups the batch by project name and derives project_id from it, | ||
| // falling back to the default project when it is blank, so it has to be sent. | ||
| project_name: projectName, |
There was a problem hiding this comment.
Duplicated batch mutation logic drifts
This hook duplicates the chunking and FeedbackScoreBatchEntry/FEEDBACK_SCORE_TYPE.ui payload mapping from useTraceFeedbackScoreBatchSetMutation, so future batching or payload changes can make trace and span updates diverge — should we extract a shared helper in the traces API package, parameterized by endpoint while preserving each hook's cache invalidation?
Want Baz to fix this for you? Activate Fixer
| <div className="max-h-80 overflow-y-auto py-2"> | ||
| <FeedbackScoresEditor | ||
| feedbackScores={feedbackScores} | ||
| onUpdateFeedbackScore={onUpdateFeedbackScore} | ||
| onDeleteFeedbackScore={onDeleteFeedbackScore} | ||
| header={<FeedbackScoresEditor.Header title="Feedback scores" />} | ||
| footer={ | ||
| <FeedbackScoresEditor.Footer entityCopy={entityCopy.many} /> | ||
| } | ||
| /> | ||
| </div> |
There was a problem hiding this comment.
Ad-hoc dialog scrolling breaks shared behavior
The dialog body uses a raw <div className="max-h-80 overflow-y-auto">, so scrolling and accessibility/layout integration remain feature-owned instead of shared — should we use DialogAutoScrollBody (or the repository equivalent) and place FeedbackScoresEditor inside it?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/v2/pages-shared/traces/AddAnnotationDialog/AddAnnotationDialog.tsx`
around lines 134-144, refactor the dialog body wrapping `FeedbackScoresEditor` to use
the shared `DialogAutoScrollBody` (or the repository’s equivalent) instead of a raw
`div` with `max-h-80 overflow-y-auto`. Import and configure the shared primitive as
needed, preserving the existing scrolling/layout behavior while delegating accessibility
and dialog integration to the shared component.
| export type FeedbackScoreBatchEntry = { | ||
| id: string; | ||
| name: string; | ||
| value: number; | ||
| categoryName?: string; | ||
| reason?: string; |
There was a problem hiding this comment.
Bulk annotation rejects valid decimal scores
FeedbackScoreBatchEntry.value coerces the valid backend value 999999999.999999999 through Number(...) to 1000000000, so both batch hooks serialize an invalid value: score.value and the backend rejects it at @DecimalMax — should we preserve decimal precision at the batch boundary or explicitly constrain/normalize the supported range?
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
apps/opik-frontend/src/lib/feedback-scores.tsx around lines 643-648, update the
`FeedbackScoreBatchEntry` value representation so valid backend `BigDecimal` values such
as `999999999.999999999` cannot be rounded into an invalid wire value. Prefer preserving
decimal values as strings through validation and batch serialization, or explicitly
constrain and normalize the supported range consistently across the editor, type, and
request payloads; add or update tests covering the maximum-precision value.
Match useTraceFeedbackScoreSetMutation so experiment and compare views do not keep stale scores after bulk annotate.
| await queryClient.invalidateQueries({ queryKey: [SPANS_KEY] }); | ||
| await queryClient.invalidateQueries({ queryKey: ["spans-columns"] }); | ||
| await queryClient.invalidateQueries({ queryKey: ["spans-statistic"] }); | ||
| await queryClient.invalidateQueries({ queryKey: [TRACE_KEY] }); | ||
| await queryClient.invalidateQueries({ queryKey: [TRACES_KEY] }); | ||
| await queryClient.invalidateQueries({ queryKey: ["traces-columns"] }); | ||
| await queryClient.invalidateQueries({ queryKey: ["traces-statistic"] }); | ||
| await queryClient.invalidateQueries({ | ||
| queryKey: ["experiment-items-statistic"], | ||
| }); | ||
| await queryClient.invalidateQueries({ | ||
| queryKey: ["experiments-columns"], |
There was a problem hiding this comment.
Cross-entity invalidation lacks coverage
The dialog test mocks both hooks, so regressions in broad span-batch invalidation or per-ID [TRACE_KEY, { traceId }] invalidation and deduplication still pass. Should we add direct hook tests covering [SPANS_KEY], [TRACE_KEY], [TRACES_KEY], the experiment keys, and per-ID deduplication, as .agents/skills/opik-frontend/testing.md and .agents/skills/opik-frontend/performance.md suggest?
Want Baz to fix this for you? Activate Fixer You can also update your AI coding guidelines based on this comment by apply pr to [branch name]
Other fix methods
Prompt for AI Agents
Before applying, verify this suggestion against the current code. In
`apps/opik-frontend/src/api/traces/useSpanFeedbackScoreBatchSetMutation.ts` around lines
60-71, add direct hook tests that execute a successful span batch mutation and assert
invalidation of `[SPANS_KEY]`, `[TRACE_KEY]`, `[TRACES_KEY]`, and all experiment-related
keys. Also add direct coverage for `useTraceFeedbackScoreSetMutation`’s per-ID
`[TRACE_KEY, { traceId }]` invalidation, verifying duplicate trace IDs are deduplicated.
Follow the frontend testing guidance and keep the existing broad cross-entity
invalidation behavior intact.
1 participant
/claim #1010
Fixes #1010
Adds bulk annotation to the v2 traces and LLM-calls selection bar using FeedbackScoresEditor. Feedback scores use chunked batch PUT requests with project names, are gated by canAnnotateTraceSpanThread, and route spans and traces to their respective endpoints.
This follows #7066 and is not #7680, which targets v1.