[OPIK-6525] [BE][FE] Annotation queue single-pass mode#6792
Conversation
github-actions
Bot
added
java
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
Backend Tests - Integration Group 14 31 files 31 suites 5m 29s ⏱️ For more details on these failures and errors, see this check. Results for commit 541d8ba. ♻️ This comment has been updated with latest results. |
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
miguelgrc
force-pushed
the
miguelg/OPIK-6525-annotation-queue-single-pass
branch
from
a225c5e to
977c397
Compare
| queueItems.forEach((item) => { | ||
| const itemId = getAnnotationQueueItemId(item); | ||
| const state = getItemState( | ||
| item, | ||
| feedbackScoreNames, |
There was a problem hiding this comment.
itemStates is built only from queueItems/server data and never reflects the mutated currentAnnotationState, so AnnotationView’s allDone/isNextDisabled can keep Next disabled and keep hiding Finish annotating after the last DEFAULT item. Can we fold the local annotation state into the memo or optimistically patch the computed state/comments instead of waiting on the refetch?
Finding type: Logical Bugs | Severity: 🔴 High
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/SMEFlowPage/SMEFlowContext.tsx around lines 276-314 (the
useMemo that builds `itemStates` by iterating `queueItems` and calling `getItemState`),
`itemStates` is currently derived only from server-provided
`item.feedback_scores`/`item.comments` and ignores the local `currentAnnotationState`
that `updateComment`/`updateFeedbackScore` mutate. Refactor this computation so that
when `item.id === currentItem?.id`, you optimistically compute the state using
`currentAnnotationState` (comment text and `scores`) instead of the stale server fields,
reusing the existing `isItemProcessedByUser`/annotator count logic or feeding those
helpers a small “synthetic” item built from local state. Ensure the memo
dependencies include `currentItem`/`currentIndex` and `currentAnnotationState` (and
anything those helpers need like `annotationQueue?.feedback_definition_names` and
`currentUserName`) so `AnnotationView` updates immediately without waiting for refetch.
There was a problem hiding this comment.
By design — itemStates reflects server-confirmed state, not optimistic local state. The sidebar updates within ~1s (debounce fires → mutation → query invalidation → recompute). Adding a second source of truth for optimistic updates would add complexity for a barely perceptible delay. Acceptable tradeoff.
There was a problem hiding this comment.
Thanks, that makes sense — I'll keep this server-confirmed-state vs optimistic-state tradeoff in mind and save it to memory once the PR is merged.
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
miguelgrc
force-pushed
the
miguelg/OPIK-6525-annotation-queue-single-pass
branch
from
977c397 to
e909e93
Compare
| const idx = defaultItemIds.indexOf(currentItem?.id ?? ""); | ||
| const nextId = defaultItemIds[(idx + 1) % defaultItemIds.length]; | ||
| if (nextId === currentItem?.id) return undefined; | ||
| return queueItems.find((item) => item.id === nextId); |
There was a problem hiding this comment.
nextDefaultItem and handleNext both scan the queue on each render/hotkey, so stepping through MAX_QUEUE_ITEMS = 15_000 can get O(n²) — should we memoize a Map<itemId, index> / Map<itemId, item> lookup?
Finding type: choose data structures | Severity: 🟢 Low
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/SMEFlowPage/SMEFlowContext.tsx around lines 258-265 (the
`nextDefaultItem` useMemo) and lines ~289-303 (the `handleNextDefault` callback),
refactor the logic to avoid repeated `indexOf`/`find`/linear scans over large arrays
(`MAX_QUEUE_ITEMS = 15000`). Build memoized lookup structures keyed by annotation queue
item id (e.g., a Map from itemId -> index, and optionally itemId -> item) from
`queueItems`/`allItemIds`, then use direct map access to determine the next default item
and its target index in constant time. Ensure these maps are recomputed only when
`queueItems`/`defaultItemIds`/`allItemIds` change, and update the dependency arrays
accordingly.
There was a problem hiding this comment.
Both nextDefaultItem and the single-pass memo are useMemo — they only recompute when queueItems changes (poll every 30s), not on every render. handleNext is a useCallback that reads the already-computed nextDefaultItem. No O(n²) — it's O(n) once per poll, then O(1) lookups.
There was a problem hiding this comment.
Thanks for the clarification — that makes sense given the useMemo/useCallback setup. I’ll save this to memory once the PR is merged.
github-actions
Bot
added
the
tests
|
🌙 Nightly cleanup: The test environment for this PR ( |
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
Backend Tests - Integration Group 6158 tests 158 ✅ 3m 45s ⏱️ Results for commit 0e0948a. ♻️ This comment has been updated with latest results. |
- Add annotators_per_item field to annotation queues (BE migration + model + DAO) - Return comment ID in create comment response for proper update/delete lifecycle - Redesign SME annotation view with sidebar, auto-save, and Figma-aligned layout - Sidebar shows queue items with state indicators (default/scored/completed) - Auto-save comments and scores with 1s debounce, flush on navigation - Single "Next" button navigates to next unreviewed item, "Finish annotating" when done - 30s polling keeps sidebar states in sync across annotators - Add "Number of annotators per item" field to queue creation form
- Memoize sidebar preview text computation to avoid re-running prettifyMessage on every render - Fix Finish annotating button not flushing pending saves before navigating to completion - Fix comment create error not resetting lastSaved text, preventing retry - Fix stale CREATE response corrupting next item's state after navigation (ignoreNextCreateResponseRef) - Add @min(1) validation to annotatorsPerItem on AnnotationQueue and AnnotationQueueUpdate - Add trailing newline to migration file - Add 17 unit tests for useAnnotationPersistence covering comment CRUD, score operations, debounce, flush, reset, stale response handling, and error recovery - Move item state logic (ITEM_STATE, getItemState, getDistinctAnnotatorCount, isItemProcessedByUser) from SMEFlowContext to lib/annotation-queues for reuse and testability - Add 13 unit tests for item state computation covering all three states, annotator counting, deduplication, and score name filtering - Unify Next button and Cmd+Enter hotkey into single handleNext callback - Disable annotation panel (comments + feedback scores) for completed items - Expand Opik logo text in SME page header - Fix queue creation form number input to allow clearing (z.coerce.number)
- Add 4 BE integration tests for annotatorsPerItem: explicit value round-trip, null defaults to 1, reject 0, reject 1001 - Add @max(1000) validation to AnnotationQueue and AnnotationQueueUpdate - Add newAnnotationQueue() test helper to ensure valid annotatorsPerItem across all 32 test sites - Move AnnotationState type to lib/annotation-queues to break circular dependency (SMEFlowContext ↔ useAnnotationPersistence) - Rename handleClick to handleItemClick in ItemsSidebar
Backend Tests - Integration Group 11 25 files 25 suites 8m 11s ⏱️ Results for commit 0e0948a. ♻️ This comment has been updated with latest results. |
miguelgrc
force-pushed
the
miguelg/OPIK-6525-annotation-queue-single-pass
branch
from
1198460 to
0e0948a
Compare
miguelgrc
added
test-environment
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
| (item.comments || []).forEach((comment) => { | ||
| if (comment.created_by) authors.add(comment.created_by); | ||
| }); |
There was a problem hiding this comment.
Could we gate getDistinctAnnotatorCount on comments_enabled (or pass the flag in), otherwise legacy comments can still satisfy annotators_per_item and make getItemState return COMPLETED too early?
Finding type: Logical Bugs | Severity: 🔴 High
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/annotation-queues.ts around lines 137-157, the
getDistinctAnnotatorCount function currently adds every comment.created_by into the
authors Set unconditionally. Update this logic so comment authors are only counted when
comments are enabled: either add a commentsEnabled boolean parameter to
getDistinctAnnotatorCount (and thread it through getItemState/isItemProcessedByUser as
needed) or filter item.comments before invoking these helpers. Ensure that when
comments_enabled=false, legacy/non-scored comments cannot cause getItemState to return
ITEM_STATE.COMPLETED before any score-based annotations are made.
There was a problem hiding this comment.
By design — comments_enabled controls whether the UI shows the comment form, not whether existing comments count as annotations. If someone commented before comments were disabled, that review should still count toward completion. Un-completing previously-done items would be a worse UX.
There was a problem hiding this comment.
Thanks, that makes sense. I'll remember this after the PR is merged.
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
…asn't annotated - Completed + user not an annotator: hide comments/scores, show check icon + message - Completed + user is an annotator: show editable comments/scores - Not completed: normal editable UI
miguelgrc
added
test-environment
|
🔄 Test environment deployment process has started Phase 1: Deploying base version You can monitor the progress here. |
|
✅ Test environment is now available! To configure additional Environment variables for your environment, run [Deploy Opik AdHoc Environment workflow] (https://github.com/comet-ml/comet-deployment/actions/workflows/deploy_opik_adhoc_env.yaml) Access Information
The deployment has completed successfully and the version has been verified. |
|
🌙 Nightly cleanup: The test environment for this PR ( |
2 participants
Details
Implements single-pass mode for annotation queues, allowing multiple annotators to work simultaneously without re-annotating the same traces. The queue automatically serves the next unannotated item based on a configurable "annotators per item" threshold.
Backend
annotators_per_itemcolumn (UInt16, default 1) toannotation_queuestableAnnotationQueue,AnnotationQueueUpdate, andAnnotationQueueDAOupdated to support the new field through create, update, and find operations{ id: commentId }in the response body (was empty 201), enabling the frontend to track comment IDs for update/delete lifecycleFrontend
AddEditAnnotationQueueDialogItemsSidebarcomponent showing all queue items with 3-state indicators (default/scored/completed), input/output previews usingprettifyMessage, and scroll-to-active behavioruseAnnotationPersistencehook. Uses refs instead of state diffing to avoid React batching bugs. Comment creates capture server ID viaonSuccessfor proper update/delete lifecyclerefetchIntervalkeeps sidebar states in sync across concurrent annotatorsBaseTraceDataTypeIcon, "Annotate" panel header, and proper spacing/paddingValidationAlert,AnnotationTreeStateContext, old submit/previous/next buttons, and stale hotkey definitionsChange checklist
Issues
Testing
Documentation
N/A