Skip to content

Per-User State in Annotations: Camera, Visibility Toggles #8542

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

Merged
merged 118 commits into from
Jun 17, 2025

Conversation

fm3
Copy link
Member

@fm3 fm3 commented Apr 17, 2025

In shared annotations with multiple authors, some changes are now stored per user. This means that other users won’t see all those changes if their own diverge. This includes the current position and zoom, visibilities of trees, bounding boxes, and segments (as specified with the checkboxes in the lists), as well as which groups are expanded in the lists. The annotation owner’s user state is used as a fallback for users who haven’t explicitly changed these values themselves.

User-Specific state for annotations and tracings
  • Annotation
    • Camera (editPos/rot/zoomLevel)
  • Skeleton
    • activeNodeId
    • Tree visibilities
    • TreeGroup expanded states
    • boundingBox visibilities
  • Volume
    • activeSegmentId
    • SegmentGroup expanded states
    • boundingBox visibilities
    • segment visibilities
Update actions that now mutate (only) the user state
  • UpdateCameraAnnotationAction
  • UpdateSegmentGroupsExpandedStateVolumeAction
  • UpdateSegmentVisibilityVolumeAction
  • UpdateTreeGroupVisibilitySkeletonAction
  • UpdateTreeGroupsExpandedStateSkeletonAction
  • UpdateTreeVisibilitySkeletonAction
  • UpdateUserBoundingBoxVisibilitySkeletonAction
  • UpdateUserBoundingBoxVisibilityVolumeAction
  • UpdateActiveNodeSkeletonAction
  • UpdateActiveSegmentIdVolumeAction

https://annotationuserstate.webknossos.xyz/

Steps to test:

Note: since new update actions are introduced here, annotations that are edited on this branch cannot be loaded after switching back to master. Refresh your db to be sure everything is clean

  • Edit annotation with multiple users, user-state should be saved and used after reload
  • Annotation owner’s state should be used as a fallback (per element)
  • Layer Deletion/Addition (bbox visibilities should be moved to user state of new precedence layer)
  • NML download should “render” the visibilities etc so that it matches what the requesting user also sees in the browser
  • Merging annotations should preserve user states (despite id remapping of segments/trees etc)
  • Duplicating annotations should preserve the user state for the requesting user (who will be owner of the new annotation).

TODOs:

Backend
  • User state in annotation, skeleton, volume proto
  • Define new update actions
    • Annotation
    • Skeleton
    • Volume
  • Implement applying new update actions
  • When layer precedence changes, move relevant user states (bbox visibilities)
  • Better Naming (now we have class UpdateUserStateSkeletonAction and trait UserStateVolumeUpdateAction…)
  • Render user state for NML writing
    • Camera
    • Skeleton specifics
    • Volume specifics
  • Update with the new UpdateSegmentGroupVisibilityVolumeAction and UpdateSegmentVisibilityVolumeAction
  • discuss what NML parser should do (create a user state? or use old properties?)
  • discuss what duplicate should do (the owner changes, should user state be mutated?)
  • check other tracingstore routes for potential interactions with user state
  • Merge User State when merging Annotations (beware of remapped ids)
    • merge skeleton user states
    • merge volume user states
    • adapt user states to new owner if needed
    • add unit tests for render logic
    • add unit tests for merge logic
  • Refresh snapshots (blocked by Replace docker compose script in yarn #8541)
  • Cleanup
  • Make sure that everything still behaves as expected after Make user bounding box updates more efficient #8492 is merged
Frontend
  • Read relevant properties from user state, with fallback hierarchy
  • Create new update actions
  • Don’t update everything immediately after load
  • fix tests

Issues:


  • Updated changelog
  • Removed dev-only changes like prints and application.conf edits
  • Considered common edge cases
  • Needs datastore update after deployment

Copy link
Contributor

coderabbitai bot commented Apr 17, 2025

📝 Walkthrough

Walkthrough

This change introduces per-user state storage for annotation UI elements, including position, zoom, visibility, and expansion states, across backend, frontend, and protocol layers. It refactors update actions, reducers, and sagas to handle user-dependent states, updates protocol buffers and APIs, and incorporates new test cases and utility functions to support and validate this multi-user state management.

Changes

Files/Groups Change Summary
Protocol Buffers (*.proto) Added new messages for per-user state (AnnotationUserStateProto, SkeletonUserStateProto, VolumeUserStateProto), new types for ID-boolean pairs (Id32WithBool, Id64WithBool), and updated existing messages to include user state arrays.
Backend Scala: Controllers, Services, Models Refactored duplication and merging logic to pass and handle user/owner IDs, added user state rendering and merging utilities (AnnotationUserStateUtils), updated annotation and tracing services to manage per-user state, extended update actions for user-specific changes, and updated controllers to accept user context parameters.
Backend Scala: Tracingstore Added/updated controllers and services to handle per-user state in duplication/merging, introduced user state update actions (e.g., UpdateCameraAnnotationAction), and utilities for ID-boolean conversions (IdWithBoolUtils).
Backend Scala: Tests Added test suites for user state rendering and merging (AnnotationUserStateTestSuite), and updated existing serialization tests to include user context.
Frontend TypeScript: Types and Utilities Introduced new types for user states (APIAnnotationUserState, SkeletonUserState, VolumeUserState), added utility functions for set/group diffs and safe object creation, and updated existing types to support per-user state arrays.
Frontend: Reducers, Actions, Sagas Refactored actions and reducers to process user-specific state, replaced legacy update actions with new granular user state update actions, integrated user state into initialization and diffing logic, separated camera annotation updates into dedicated saga, and updated sagas to remove flycam dependencies from tracing diffs.
Frontend: Model Initialization Updated initialization logic to prioritize user state over global annotation state for position, zoom, and rotation.
Frontend: Tests Updated and expanded tests to cover new user state logic and more granular update actions, including snapshot tests and saga tests.
Frontend: Miscellaneous Improved logging messages, replaced lodash zipObject with a safe custom utility, updated UI components to use new action creators, and added browser feature detection for Set difference.

Assessment against linked issues

Objective (Issue #) Addressed Explanation
Store user-dependent annotation UI state separately from shared annotation data (8531)
Refactor update actions and reducers to support per-user state (8531)
Update protocol buffers and backend APIs for user state (8531)
Ensure frontend initialization and diffing logic uses per-user state (8531)

Assessment against linked issues: Out-of-scope changes

No out-of-scope changes detected. All functional modifications relate directly to the objective of storing and handling user-dependent annotation state.

Possibly related PRs

Suggested reviewers

  • philippotto
  • daniel-wer

Poem

In a warren of code, the rabbits convene,
To store your view state, both hidden and seen.
Each bunny now keeps their own UI delight,
Bounding boxes and trees, just for their sight.
With zooms and positions, all tucked away,
Collaboration’s a hop, not a fray!
🐇✨


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Ok here is what I noticed during testing. For each of these points I do not know whether BE or FE causes this as I did not further investigation:

  • [ ] [see follow-up issue] When I have an annotation like this:


    (Note the two added but empty volume layers) The reuploaded nml yields an annotation where the volume1 layer is active and not the volume layer as in the state of the downloaded annotation. Not sure how much thats related. And maybe that's expected as I did an nml only download. But the reuploaded annotation seems to have the volume1 and volume2 layers.
  • Moreover, nml reupload does not seem to store the correct active node id or no active node id is stored at all
  • And I think the segment group merging is broken: Her I have merged 2 annotations and this is the new owners perspective -> meaning the two user states of the 2 merged annotations and what the result was.
    Source 1:


    Source 2:


    Result:


    notable differences: The "from 2" and "from 2 2" groups are empty in the merged annotation And I think the sub segments from groups "from 2" and "from 2 2" are now in "Group 1" and "Group 2".
  • [Expected because non-owner user states are dismissed on merge] Moreover, the skeleton user state also seems inconsistent: In this case, the user is not the owner of the merged annotation. Source 1 and Source 2 show the user's state of the two annotations which were merged by another user and then the result is this users state of the merged annotation.
    Source 1 (Where group 1 and group3 each has multiple inviisble trees as children)


    Source 2:


    Result:


    Here the group 1 from source 1 is not invisible and collapsed and in group2 tree with _001 is visible now although it is not in source 1. Moreover, trees from source 2 with _003 and _004 are no longer invisible in the merged annotation.
  • [ ] [can't reproduce, see slack] I managed to get into a strange state of an annotation where nml reupload would make bbox with id 1 invisible though in the source it is visible. The same goes for duplicating. Not sure how to share this annotation state as the nml already has visibility set to false for this bounding box :/

@fm3
Copy link
Member Author

fm3 commented Jun 5, 2025

Thanks so much for putting so much effort into testing!

  • Annotation layer visibility is still stored in the View Configuration in postgres and is not part of the new versioned user state. It would probably make sense to get that in there though. @philippotto what do you think, is it worth tackling this in this PR as well? Or should we go for a follow-up here? It is certainly not a blocker for live collaboration, I’d say.

  • Currently, if the View Configuration is empty, the first layer is set to enabled/visible. However, which layer actually is the first layer is not a trivial question to answer. This was not touched in this PR, but I investigated a bit and wrote my findings in issue Annotation Layer Ordering is Inconsistent #8673

  • Segment Group merging is also broken on master. The remapped segment group ids were never actually set for the segments. I pushed a fix here now. Good find!

  • Non-Owner user state being lost on annotation merge is by design. We decided to not carry all user states of all users when a merge is performed. We just ensure that the user triggering the merge sees the same as before (whether they were the old owner or not). The same goes for duplicate (and thus also copy to my account)

  • Active Node display seems to be a frontend issue. NML to reproduce: https://scm.slack.com/archives/C5AKLAV0B/p1749132401019729?thread_ts=1748848876.388159&cid=C5AKLAV0B

  • Boundingbox visibility in NML worked for me, I could not reproduce this.

I’d be happy to have a look at the two things I could not reproduce again, maybe we can find a way together.

@philippotto
Copy link
Member

  • Annotation layer visibility is still stored in the View Configuration in postgres and is not part of the new versioned user state. It would probably make sense to get that in there though. @philippotto what do you think, is it worth tackling this in this PR as well? Or should we go for a follow-up here? It is certainly not a blocker for live collaboration, I’d say.

let's have a chat about this 🤙 I agree that it makes sense to tackle this, but I cannot say whether it's easy enough to do in the same pr.

I turned @MichaelBuessemeyer comment into checkboxes. Since I fixed the "active node" issue, I ticked that off. I also crossed out or checked what seems to be a non-issue or already done. please have a second look whether I got everything right.

@fm3 fm3 requested a review from MichaelBuessemeyer June 12, 2025 08:37
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
frontend/javascripts/test/helpers/apiHelpers.ts (1)

203-205: Dispatch order: reset first, restart sagas second

restartSagaAction() cancels and immediately re-starts the root saga tree.
Triggering it before resetStoreAction() means the freshly spawned sagas observe the old store state for at least one event loop tick, then have the ground pulled away when the store is reset.
Flipping the order guarantees that sagas attach to a pristine store from the outset and avoids rare race conditions in tests that listen for takeLatest on initialised state slices.

-  Store.dispatch(restartSagaAction());
-  Store.dispatch(resetStoreAction());
+  // Reset the store to its initial state *before* bringing the sagas back online
+  Store.dispatch(resetStoreAction());
+  Store.dispatch(restartSagaAction());
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2b08ccc and 0d8f102.

📒 Files selected for processing (14)
  • frontend/javascripts/admin/rest_api.ts (1 hunks)
  • frontend/javascripts/libs/utils.ts (4 hunks)
  • frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts (3 hunks)
  • frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts (2 hunks)
  • frontend/javascripts/test/fixtures/tasktracing_server_objects.ts (2 hunks)
  • frontend/javascripts/test/fixtures/volumetracing_server_objects.ts (2 hunks)
  • frontend/javascripts/test/helpers/apiHelpers.ts (2 hunks)
  • frontend/javascripts/test/sagas/saga_integration.spec.ts (3 hunks)
  • frontend/javascripts/test/sagas/save_saga.spec.ts (3 hunks)
  • frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts (24 hunks)
  • frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts (4 hunks)
  • frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_1.spec.ts (1 hunks)
  • frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_2.spec.ts (1 hunks)
  • frontend/javascripts/viewer/model/sagas/save_saga.ts (7 hunks)
✅ Files skipped from review due to trivial changes (1)
  • frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_1.spec.ts
🚧 Files skipped from review as they are similar to previous changes (12)
  • frontend/javascripts/test/sagas/volumetracing/volumetracing_saga_integration_2.spec.ts
  • frontend/javascripts/admin/rest_api.ts
  • frontend/javascripts/test/sagas/save_saga.spec.ts
  • frontend/javascripts/test/fixtures/tasktracing_server_objects.ts
  • frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts
  • frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts
  • frontend/javascripts/test/fixtures/volumetracing_server_objects.ts
  • frontend/javascripts/test/sagas/saga_integration.spec.ts
  • frontend/javascripts/viewer/model/sagas/save_saga.ts
  • frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts
  • frontend/javascripts/test/sagas/skeletontracing_saga.spec.ts
  • frontend/javascripts/libs/utils.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: backend-tests
  • GitHub Check: build-smoketest-push
  • GitHub Check: frontend-tests
🔇 Additional comments (1)
frontend/javascripts/test/helpers/apiHelpers.ts (1)

41-41: Confirm availability of newly-imported action

resetStoreAction is now imported from viewer/model/actions/actions.
Double-check that this symbol is indeed exported by that barrel file; otherwise the test helper will fail to compile at runtime.
No further issues spotted with the revised import list.

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Latest backend commit (e756def) looks good to me. But I haven't tested the fix. Should I?

@fm3
Copy link
Member Author

fm3 commented Jun 13, 2025

Should be fine, I tested it locally. Thanks!

@philippotto
Copy link
Member

@MichaelBuessemeyer @daniel-wer both of you left a lgtm. this means we can merge it? :)

Copy link
Member

@daniel-wer daniel-wer left a comment

Choose a reason for hiding this comment

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

Ready to be merged from my side 👍

@daniel-wer
Copy link
Member

One more question: What's the rollback strategy with the new update actions? Should we have an intermediate PR which only introduces the new actions so we can rollback to that or do you think it's fine like it is?

@fm3
Copy link
Member Author

fm3 commented Jun 16, 2025

As discussed in Slack, a full rollback would be tedious. Instead, we’ll create backups of both fossildb and postgres content before upgrading and in case of errors we can’t hotfix, we’ll restore that snapshot instead.

I did another round of testing and encountered no problems.

@MichaelBuessemeyer If there are no other open points, could you hit approve so that we can merge this?

Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer left a comment

Choose a reason for hiding this comment

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

Neat, looks good. Thanks for re testing

@fm3 fm3 merged commit 2dd6ffb into master Jun 17, 2025
5 checks passed
@fm3 fm3 deleted the annotation-user-state branch June 17, 2025 08:42
@coderabbitai coderabbitai bot mentioned this pull request Jul 4, 2025
11 tasks
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.

Milestone 1: Store User-dependent information separately
4 participants