[WIP] Live Collab M3 - Follow ups for agglomerate skeletons and meshes #9102
Open
MichaelBuessemeyer wants to merge 266 commits intomasterfrom
Open
[WIP] Live Collab M3 - Follow ups for agglomerate skeletons and meshes #9102MichaelBuessemeyer wants to merge 266 commits intomasterfrom
MichaelBuessemeyer wants to merge 266 commits intomasterfrom
Conversation
Contributor
|
Important Review skippedToo many files! This PR contains 166 files, which is 16 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (166)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…of own merge action while rebasing, the proofread saga is doing this already
…freading helper mesh auto reloading
| @@ -12,7 +12,7 @@ import type ApiLoader from "./api_loader"; | |||
| // for debugging or one off scripts. | |||
| export const WkDevFlags = { | |||
| logActions: false, | |||
| liveCollab: false, | |||
| liveCollab: true, | |||
Contributor
Author
There was a problem hiding this comment.
TODO: undo
frontend/javascripts/viewer/model/sagas/volume/proofreading/proofread_saga.ts
Outdated
Show resolved
Hide resolved
…erly splittable partitions
…os into live-m3-follow-up
philippotto
reviewed
Mar 17, 2026
frontend/javascripts/viewer/model/sagas/volume/proofreading/proofread_saga.ts
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/model/reducers/volumetracing_reducer_helpers.ts
Show resolved
Hide resolved
frontend/javascripts/test/sagas/proofreading/proofreading_auxiliary_mesh_loading.spec.ts
Outdated
Show resolved
Hide resolved
…er initiated actions - missing change from previous refactoring
…e 8000 timeouts & remove unused import
…sion present for necessary requests
Contributor
Author
MichaelBuessemeyer
left a comment
MichaelBuessemeyer
left a comment
There was a problem hiding this comment.
Pending replys:
So left over TODOs:
- Live testing
- Code cleanup
- merge
- profit
| } | ||
|
|
||
| export const initialBucketOverrides: Array<{ position: Vector3; value: number }> = [ | ||
| { position: [100, 100, 100], value: 1337 }, // todop: can we change the positions to to 1337³ etc? |
Contributor
Author
There was a problem hiding this comment.
I think this is not possible as the mocked datasets bounds are not high enough. But we could adjust the mock to implement this I guess
| context.tearDownPullQueues(); | ||
| // Saving after each test and checking that the root saga did indeed crash. | ||
| expect(hasRootSagaCrashed()).toBe(true); | ||
| expect(hasRootSagaCrashed()).toBe(false); |
Contributor
Author
There was a problem hiding this comment.
Done in 7c8af0c
The problem why I used spawn was that the subscribe helper saga needed to be able to start the mutex fetching saga in case it was needed. A simple call / fork would have forced the subscribe helper saga to wait until the mutex fetching is done. But as the mutex fetching goes on forever unless stopped, the subscribe saga would never terminate unless spawn was used.
Now the subscribe saga helper dispatches an action to which the "rool level" mutex saga listenes to and then on demand starts fetching / releasing the annotation mutex. This way the subscribe helper saga is no longer blocked and the "rool level" mutex saga successfully crashes as well when the mutex fetching loop errors.
After a thourough implementation time without any tests run all were instantly green 🎉 -> maybe a good sign that this is a good way to implement this :D
frontend/javascripts/viewer/model/reducers/skeletontracing_reducer.ts
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/model/sagas/volume/proofreading/proofread_saga.ts
Outdated
Show resolved
Hide resolved
frontend/javascripts/viewer/model/sagas/volume/proofreading/proofread_saga.ts
Show resolved
Hide resolved
frontend/javascripts/test/sagas/proofreading/proofreading_auxiliary_mesh_loading.spec.ts
Outdated
Show resolved
Hide resolved
Contributor
Author
There was a problem hiding this comment.
Fixes
Bug: If the initial proofreading actions is a split operation, the operation fails
in commit 50e354c
1 task
MichaelBuessemeyer
added a commit
that referenced
this pull request
Mar 23, 2026
Fixes a bug noticed in #9102. Might be good to get this easy fix merged before #9102 is merged. Should be an easy win to fix this imo. ### URL of deployed dev instance (used for testing): - https://___.webknossos.xyz ### Steps to test: - Open an annotation with an agglomerate mapping - Activate the agglomerate mapping - Activeate the proofread tool - Make a split -> should succeed. - Try the same on master -> should result in an error ### Issues: - fixes bug noticed in #9102 ------ (Please delete unneeded items, merge only when none are left open) - [x] Updated [changelog](../blob/master/CHANGELOG.unreleased.md#unreleased) --------- Co-authored-by: Daniel <daniel.werner@scalableminds.com>
- Accept certain test behaviour - remove oudated comment in test - rename type
Contributor
Author
|
Bugs / TODOs from latest test session:
|
… well when forwarding such actions
…glomerate skeleton actions acutally notice that this is an agglomerate skeleton
…ther users indefinitely
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PRs goal is to keep agglomerate meshes and skeleton in sync with the currently loaded mapping state of a user. Primarily this is needed for consistency in a live collab annotation.
See design doc for this steps changes and its related hand-over notes: https://www.notion.so/scalableminds/Design-Doc-Meeting-Notes-Live-Collaboration-1adb51644c6380dfafd4c8f5999830f9#1c1b51644c6380b989e5e1bb68a3c7d1
For major conceptional changes, see the doc link above
URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
isProofreadingAuxiliaryMesh. Is it needed? Shouldn’t all meshes be refreshed automatically if the underlying mapping changes? Or shouldn’t they?annotationVersiondiffEdgeCollectionswithuseDeepEqualityCheck = trueworks as intendeddiffEdgeCollectionstaking a lot of time to airbrake to see if this becomes a bottle neck in productionisProofreadingAuxiliaryMeshproperty?getPositionForSegmentIdwk_dev.tschangesWritten tests:
In general regarding existing test files: Moving functions helping with the tests to
proofreading_skeleton_interaction.spec.ts:proofreading_auxiliary_mesh_loading.spec.ts: The following cases should load and auto-reload the correct agglomerate meshes upon editable mapping changes:proofreading_agglomerate_skeleton_syncing.spec.tsDeferred TODOs:
Issues:
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)