Skip to content

[WIP] Live Collab M3 - Proofreading (without segments list) #8723

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

Open
wants to merge 48 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
b0e9c1d
add explicit release mutex functionality
philippotto Jun 26, 2025
e02d4ce
remove superfluous didShowFailedSimultaneousTracingError logic (unnec…
philippotto Jun 26, 2025
1154a74
acquire mutex in proofreading and release after saving
philippotto Jun 26, 2025
2e8d41a
retry even 409 errors
philippotto Jun 26, 2025
0865908
Revert "retry even 409 errors" because saving should only be done when a
philippotto Jun 26, 2025
fef8705
extract save mutex saga into own module
philippotto Jun 26, 2025
1ed33f1
refactor further
philippotto Jun 26, 2025
b3865fc
disable eager mutex acquisition and also poll for updates when allow …
philippotto Jun 26, 2025
3ecc502
add DISABLE_EAGER_MUTEX_ACQUISITION bool
philippotto Jun 26, 2025
5e96e38
add todo comment
philippotto Jun 26, 2025
0f41c4b
move isMutexAcquired to store
philippotto Jun 27, 2025
33de8ec
refactor mutex acquisition to use less state
philippotto Jun 27, 2025
527f1db
make mutex-acquisition ad-hoc when trying to save
philippotto Jun 27, 2025
9aa961b
in backend, make agglomerateIds and mag optional in proofreading upda…
fm3 Jun 30, 2025
f845a69
implement has newest version mechanism
philippotto Jul 10, 2025
c3cd37c
don't put unused properties into update actions for split and merge a…
philippotto Jul 10, 2025
29a1608
better debugging
philippotto Jul 10, 2025
aa82211
maintain proofreading marker separately from segment position to avoi…
philippotto Jul 12, 2025
4434111
type fixes
philippotto Jul 12, 2025
9acb66e
try to fix 'Duplicate edge in agglomerate graph' error
philippotto Jul 14, 2025
a65323d
repeat local update after merge because the mapping might have been c…
philippotto Jul 14, 2025
824d660
implement BackendMock class to prepare for new test that deals with p…
philippotto Jul 15, 2025
ef785c4
implement agglomerate helper + tests
philippotto Jul 16, 2025
0ec2347
iterate on backend mock that manages versioned agglomerate mapping
philippotto Jul 17, 2025
c0865d3
fix rebase-merge test
philippotto Jul 17, 2025
96d8706
implement rebase-merge-split test
philippotto Jul 18, 2025
7a11456
refactor test
philippotto Jul 18, 2025
af75fb9
fix most old proofreading tests
philippotto Jul 18, 2025
66bf44e
clean up
philippotto Jul 18, 2025
b22737e
fix one test in save saga spec (but the generator style tests are ann…
philippotto Jul 18, 2025
1822d23
split proofreading spec into multiple modules
philippotto Jul 18, 2025
be1bcca
split proofreading specs into three modules
philippotto Jul 18, 2025
81f9497
add some colored logs
philippotto Jul 18, 2025
0370e9e
WIP refactor mutex acquiring
MichaelBuessemeyer Jul 22, 2025
3f66d4e
clean up backend for mutex release
fm3 Jul 23, 2025
4393fad
fix getting annotation mutex
MichaelBuessemeyer Jul 23, 2025
793c15a
refactor allow update, always keep restrictions set by the server
MichaelBuessemeyer Jul 23, 2025
74cdc79
fix EnsureMaySaveNow not getting resolved
MichaelBuessemeyer Jul 24, 2025
75cfde2
WIP add more mutex tests
MichaelBuessemeyer Jul 25, 2025
64f30e2
wip fix on demand mutex fetching test
MichaelBuessemeyer Jul 25, 2025
9336e7e
fix tests
MichaelBuessemeyer Jul 28, 2025
63d9da6
fix tests
MichaelBuessemeyer Jul 29, 2025
fa855ee
test injecting foreign split action
MichaelBuessemeyer Jul 29, 2025
fe491f9
add segment not loaded in tests
MichaelBuessemeyer Jul 30, 2025
26ca604
added tests where the injected actions include not loaded segments in…
MichaelBuessemeyer Jul 30, 2025
a0fad85
WIP: properly fetch mutex on demand
MichaelBuessemeyer Jul 30, 2025
fb79238
fix on demand mutex releasing and acquiring
MichaelBuessemeyer Jul 31, 2025
0c6d932
mark mutex as released in client state after successful release call …
MichaelBuessemeyer Jul 31, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/controllers/AnnotationController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -454,4 +454,12 @@ class AnnotationController @Inject()(
}
}

def releaseMutex(id: ObjectId): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
logTime(slackNotificationService.noticeSlowRequest, durationThreshold = 1 second) {
for {
_ <- annotationMutexService.release(id, request.identity._id) ?~> "annotation.mutex.release.failed"
} yield Ok
}
}

}
9 changes: 9 additions & 0 deletions app/models/annotation/AnnotationMutexService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,9 @@ class AnnotationMutexService @Inject()(val lifecycle: ApplicationLifecycle,
_ <- annotationMutexDAO.upsertOne(mutex.copy(expiry = Instant.in(defaultExpiryTime)))
} yield MutexResult(canEdit = true, None)

def release(annotationId: ObjectId, userId: ObjectId): Fox[Unit] =
annotationMutexDAO.deleteForUser(annotationId, userId)

def publicWrites(mutexResult: MutexResult): Fox[JsObject] =
for {
userOpt <- Fox.runOptional(mutexResult.blockedByUser)(user => userDAO.findOne(user)(GlobalAccessContext))
Expand Down Expand Up @@ -114,4 +117,10 @@ class AnnotationMutexDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionC
def deleteExpired(): Fox[Int] =
run(q"DELETE FROM webknossos.annotation_mutexes WHERE expiry < NOW()".asUpdate)

def deleteForUser(annotationId: ObjectId, userId: ObjectId): Fox[Unit] =
for {
_ <- run(
q"DELETE FROM webknossos.annotation_mutexes WHERE _annotation = $annotationId AND _user = $userId".asUpdate)
} yield ()

}
4 changes: 3 additions & 1 deletion conf/messages
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,9 @@ annotation.getNewestVersion.failed=Could not get the newest version information
annotation.idForTracing.failed=Could not find the annotation id for this tracing id.
annotation.editableMapping.getAgglomerateGraph.failed=Could not look up an agglomerate graph for requested agglomerate.
annotation.editableMapping.getAgglomerateIdsForSegments.failed=Could not look up agglomerate ids for requested segments.
annotation.duplicate.failed=Failed to duplicate annotation
annotation.duplicate.failed=Failed to duplicate annotation.
annotation.mutex.failed=Failed to aquire annotation editing mutex.
annotation.mutex.release.failed=Failed to release annotation editing mutex.

mesh.file.listChunks.failed=Failed to load chunk list for segment {0} from mesh file “{1}”
mesh.file.loadChunk.failed=Failed to load mesh chunk for segment
Expand Down
1 change: 1 addition & 0 deletions conf/webknossos.latest.routes
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ DELETE /annotations/:id
POST /annotations/:id/merge/:mergedTyp/:mergedId controllers.AnnotationController.mergeWithoutType(id: ObjectId, mergedTyp: String, mergedId: ObjectId)
GET /annotations/:id/download controllers.AnnotationIOController.downloadWithoutType(id: ObjectId, version: Option[Long], skipVolumeData: Option[Boolean], volumeDataZipFormat: Option[String])
POST /annotations/:id/acquireMutex controllers.AnnotationController.tryAcquiringAnnotationMutex(id: ObjectId)
DELETE /annotations/:id/mutex controllers.AnnotationController.releaseMutex(id: ObjectId)

GET /annotations/:typ/:id/info controllers.AnnotationController.info(typ: String, id: ObjectId, timestamp: Option[Long])
DELETE /annotations/:typ/:id controllers.AnnotationController.cancel(typ: String, id: ObjectId)
Expand Down
6 changes: 6 additions & 0 deletions frontend/javascripts/admin/rest_api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,12 @@ export async function acquireAnnotationMutex(
return { canEdit, blockedByUser };
}

export async function releaseAnnotationMutex(annotationId: string): Promise<void> {
await Request.receiveJSON(`/api/annotations/${annotationId}/mutex`, {
method: "DELETE",
});
}

export async function getTracingForAnnotationType(
annotation: APIAnnotation,
annotationLayerDescriptor: AnnotationLayerDescriptor,
Expand Down
4 changes: 2 additions & 2 deletions frontend/javascripts/navbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -978,8 +978,8 @@ const mapStateToProps = (state: WebknossosState): StateProps => ({
isInAnnotationView: state.uiInformation.isInAnnotationView,
hasOrganizations: state.uiInformation.hasOrganizations,
othersMayEdit: state.annotation.othersMayEdit,
blockedByUser: state.annotation.blockedByUser,
allowUpdate: state.annotation.restrictions.allowUpdate,
blockedByUser: state.save.mutexState.blockedByUser,
allowUpdate: state.annotation.isUpdatingCurrentlyAllowed,
isLockedByOwner: state.annotation.isLockedByOwner,
annotationOwnerName: formatUserName(state.activeUser, state.annotation.owner),
isAnnotationOwner: isAnnotationOwnerAccessor(state),
Expand Down
1 change: 0 additions & 1 deletion frontend/javascripts/test/fixtures/volumetracing_object.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const stateWithoutDatasetInitialization = update(defaultState, {
restrictions: {
$set: {
branchPointsAllowed: true,
initialAllowUpdate: true,
allowUpdate: true,
allowFinish: true,
allowAccess: true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export const annotation: APIAnnotation = {
allowUpdate: true,
allowFinish: true,
allowDownload: true,
allowSave: true,
},
annotationLayers: [
{
Expand Down
202 changes: 202 additions & 0 deletions frontend/javascripts/test/helpers/agglomerate_mapping_helper.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
import _ from "lodash";

export class AgglomerateMapping {
/*
* This class is used to mock the backend in the proofreading
* tests. The class maintains an agglomerate mapping which consists
* of segment ids (nodes) and edges. The components in that graph
* define to which agglomerate ids segment ids are mapped.
* The class supports adding and removing edges. Each operation
* creates a new version so that map requests can be paramaterized
* with the version.
*/

public segmentIds: number[];

// adjacency list of the *current* graph (edges themselves are *not* versioned)
private readonly adjacencyList = new Map<number, Set<number>>();

// snapshot of the component‑ID map after every operation, versions[0] = initial state
private versions: Array<Map<number, number>> = [];

public currentVersion = -1; // newest version index
private largestMappedId: number; // monotone counter for fresh IDs

constructor(
public readonly edges: Array<[number, number]>,
initialVersion: number = 0,
) {
this.segmentIds = _.uniq(edges.flat());

this.largestMappedId = Math.max(...this.segmentIds);
const initialVersionMap = new Map<number, number>();
for (const segmentId of this.segmentIds) {
this.adjacencyList.set(segmentId, new Set());
initialVersionMap.set(segmentId, segmentId); // each segment is its own component at v0
}
this.commit(initialVersionMap);

for (const edge of edges) {
this.addEdge(edge[0], edge[1]);
}

this.resetVersionCounter(initialVersion);
}

addEdge(segmentIdA: number, segmentIdB: number): void {
/*
* Add an edge and record the new version. All segment ids
* that are present in the component defined by segmentIdB
* are remapped to the mapped id of segmentIdA.
*/
this.ensureNode(segmentIdA);
this.ensureNode(segmentIdB);
this.adjacencyList.get(segmentIdA)!.add(segmentIdB);
this.adjacencyList.get(segmentIdB)!.add(segmentIdA);

const previousVersionMap = this.versions[this.currentVersion];
const nextVersionMap = new Map(previousVersionMap); // shallow copy for copy‑on‑write
const componentIdA = previousVersionMap.get(segmentIdA)!;
const componentIdB = previousVersionMap.get(segmentIdB)!;

// Only merge if the components differ
if (componentIdA !== componentIdB) {
for (const [segmentId, componentId] of nextVersionMap) {
if (componentId === componentIdB) nextVersionMap.set(segmentId, componentIdA);
}
}

this.commit(nextVersionMap);
}

removeEdge(segmentIdA: number, segmentIdB: number): void {
/*
* Remove an edge, possibly splitting a component, and record the new version.
* The source component (defined by segmentIdA) will keep its mapped id.
* The target component is reassigned to a new id.
*/
this.ensureNode(segmentIdA);
this.ensureNode(segmentIdB);
this.adjacencyList.get(segmentIdA)!.delete(segmentIdB);
this.adjacencyList.get(segmentIdB)!.delete(segmentIdA);

console.log("removing edge", segmentIdA, segmentIdB);

const previousVersionMap = this.versions[this.currentVersion];
const keepComponentId = previousVersionMap.get(segmentIdA)!;
const nextVersionMap = new Map<number, number>();
const visitedSegmentIds = new Set<number>();

for (const startSegmentId of this.segmentIds) {
if (visitedSegmentIds.has(startSegmentId)) continue;

// BFS to collect this component
const bfsStack = [startSegmentId];
const componentSegmentIds: number[] = [];
visitedSegmentIds.add(startSegmentId);

while (bfsStack.length) {
const currentSegmentId = bfsStack.pop()!;
componentSegmentIds.push(currentSegmentId);
for (const neighborSegmentId of this.adjacencyList.get(currentSegmentId) ?? []) {
if (!visitedSegmentIds.has(neighborSegmentId)) {
visitedSegmentIds.add(neighborSegmentId);
bfsStack.push(neighborSegmentId);
}
}
}

// Determine the ID to assign:
let newComponentId: number;
if (
componentSegmentIds.some(
(segmentId) => previousVersionMap.get(segmentId) !== previousVersionMap.get(segmentIdA),
)
) {
// Unrelated component → keep previous IDs
for (const segmentId of componentSegmentIds) {
nextVersionMap.set(segmentId, previousVersionMap.get(segmentId)!);
}
continue;
} else if (componentSegmentIds.includes(segmentIdA)) {
// Component that includes `segmentIdA` keeps its ID
newComponentId = keepComponentId;
} else {
// New component → assign fresh ID
newComponentId = ++this.largestMappedId;
}

for (const segmentId of componentSegmentIds) {
nextVersionMap.set(segmentId, newComponentId);
}
}

this.commit(nextVersionMap);
}

mapSegment(segmentId: number, version: number): number {
/*
* Look up the component‑ID of `segmentId` at an arbitrary version.
*/
const versionMap = this.getMap(version);
const componentId = versionMap.get(segmentId);
if (componentId === undefined) throw new Error("Unknown segmentId");
return componentId;
}

getMap(version: number): Map<number, number> {
/*
* Get the entire mapping for a specific version.
*/
if (version == null || version < 0 || version > this.currentVersion)
throw new RangeError(`Invalid version: ${version}`);
return this.versions[version];
}

private resetVersionCounter(initialVersion: number) {
/*
* Reset the most recent version to be stored as version `initialVersion`.
* If `initialVersion` is greater than 0, all previous versions are set to the
* newest version.
*/
const newestMap = this.versions.at(-1);
if (newestMap == null) {
throw new Error("No initial version of map found.");
}
this.versions = Array.from({ length: initialVersion + 1 }, () => newestMap);
this.currentVersion = initialVersion;
}

bumpVersion() {
/*
* Increment the version even though nothing changed (necessary
* because the backend also stores other things than the agglomerate mapping
* in the annotation).
*/
const newestMap = this.versions.at(-1);
if (newestMap == null) {
throw new Error("No initial version of map found.");
}
this.commit(newestMap);
}

/* Helpers */

private ensureNode(segmentId: number): void {
if (!this.adjacencyList.has(segmentId)) {
throw new Error(`Segment ${segmentId} was not in the original set`);
}
}

private commit(newVersionMap: Map<number, number>): void {
/*
* Mush mapping snapshot and advance the global version counter
*/
this.versions.push(newVersionMap);
this.currentVersion++;
console.log(
`Committed v=${this.currentVersion} with mapping: `,
newVersionMap.entries().toArray(),
);
}
}
Loading