Use mags instead of resolutions in datasource #8958
Conversation
📝 WalkthroughWalkthroughReplace dataset/ layer magnification field shape and name from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (9)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used🧠 Learnings (5)📓 Common learnings📚 Learning: 2024-11-22T17:18:43.411ZApplied to files:
📚 Learning: 2024-11-22T17:18:04.217ZApplied to files:
📚 Learning: 2024-11-22T17:19:07.947ZApplied to files:
📚 Learning: 2024-12-16T15:48:37.065ZApplied to files:
🧬 Code graph analysis (6)frontend/javascripts/viewer/geometries/materials/plane_material_factory.ts (1)
frontend/javascripts/viewer/api/api_latest.ts (1)
frontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (2)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx (4)
frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (10)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/javascripts/viewer/view/action-bar/download_modal_view.tsx (1)
310-327: Use fallback layer mags when exporting fallback data.For traced volumes we export the fallback layer (
selectedLayerInfos.layerName), but the magnification info is now taken fromselectedLayer.mags. Before the refactor this was acceptable because both tracing and fallback sharedresolutions. After the rename, only the fallback layer is guaranteed to expose the correctmags. If a tracing layer no longer carries identicalmags,getMagInfo(selectedLayer.mags)will either see an empty array or the wrong values, leading to wrong defaults in the slider and possibly invalid requests. Please derivemagnificationInfofrom the fallback (selectedLayer.fallbackLayerInfo?.mags ?? selectedLayer.mags) so export continues to use the mags of the actual layer we pass to the backend.To address it, guard the lookup like this:
- const selectedLayerMagInfo = getMagInfo(selectedLayer.mags); + const sourceMags = selectedLayer.fallbackLayerInfo?.mags ?? selectedLayer.mags; + const selectedLayerMagInfo = getMagInfo(sourceMags);
🧹 Nitpick comments (5)
frontend/javascripts/viewer/model/bucket_data_handling/wkstore_adapter.ts (1)
202-202: Confirm mags availability here; add guard or temporary fallback.Switching to
layerInfo.magsis consistent with the PR. Please verify this adapter never receives layers withoutmags(including old cached state/annotations). If not guaranteed, add a fast‑fail or a temporary back‑compat fallback.Option A (fail fast):
- const magInfo = getMagInfo(layerInfo.mags); + if (!layerInfo.mags || layerInfo.mags.length === 0) { + throw new Error(`Missing mags for layer "${layerInfo.name}"`); + } + const magInfo = getMagInfo(layerInfo.mags);Option B (back‑compat until full migration):
- const magInfo = getMagInfo(layerInfo.mags); + const mags = layerInfo.mags ?? (layerInfo as any).resolutions; + if (!mags || mags.length === 0) { + throw new Error(`Missing mags for layer "${layerInfo.name}"`); + } + const magInfo = getMagInfo(mags);Note: Some places intentionally keep
resolutions(e.g., specific sagas). Ensure this change doesn’t affect those code paths. Based on learningsfrontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx (1)
258-262: Avoid recomputing MagInfo twice; hoist to a constMinor perf/readability improvement; keeps memoization effective.
- const adhocMagIndex = getMagInfo(layer.mags).getClosestExistingIndex( - preferredQualityForMeshAdHocComputation, - ); - const adhocMag = getMagInfo(layer.mags).getMagByIndexOrThrow(adhocMagIndex); + const magInfo = getMagInfo(layer.mags); + const adhocMagIndex = magInfo.getClosestExistingIndex( + preferredQualityForMeshAdHocComputation, + ); + const adhocMag = magInfo.getMagByIndexOrThrow(adhocMagIndex);frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (1)
1126-1131: Fallback mags source change — verify availabilityUsing fallbackLayerInfo.mags is semantically correct with the new API. Please verify that fallbackLayerInfo is always populated with mags for legacy datasets; otherwise guard against undefined.
frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx (1)
50-56: Select best mag across all dims (not only X) for closer matchCurrently, selection compares only diff[0]. Consider using the max (or sum) across dimensions to choose the closest match more robustly.
Apply this tweak:
- let bestDifference = [ - Number.POSITIVE_INFINITY, - Number.POSITIVE_INFINITY, - Number.POSITIVE_INFINITY, - ]; + let bestDifference: number[] = [Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY, Number.POSITIVE_INFINITY]; for (const mag of colorLayer.mags) { const diff = datasetScaleInNm.map((dim, i) => Math.abs(Math.log(dim * mag[i]) - Math.log(modelScale[i])), ); - if (bestDifference[0] > diff[0]) { + if (Math.max(...bestDifference) > Math.max(...diff)) { bestDifference = diff; closestMagOfCurrentDS = mag; } }Also applies to: 59-67
frontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.ts (1)
151-152: Validate getMagInfo & getDataForBoundingBox zoomStep semanticsSwitching to
magshere looks correct, but verify:
getMagInfotakes an array of mags (Vector3[])—some code still usesresolutions.- The zoomStep returned by
MagInfo(from the intersection) matches the_zoomStepparameter ofapi.data.getDataForBoundingBox.Run:
rg -nP -C3 'function _getMagInfo\b' --type=ts rg -nP -C3 'export const getMagInfo\b' --type=ts rg -nP -C3 'getDataForBoundingBox' --type=tsOptional: replace
_.intersectionBy(colorLayer.mags, volumeLayer.mags, mag => mag.join("-"))with
_.intersectionWith(colorLayer.mags, volumeLayer.mags, _.isEqual)to avoid string allocations.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (36)
frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx(1 hunks)frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx(2 hunks)frontend/javascripts/test/fixtures/dataset_server_object.ts(2 hunks)frontend/javascripts/test/fixtures/hybridtracing_object.ts(1 hunks)frontend/javascripts/test/fixtures/volumetracing_object.ts(1 hunks)frontend/javascripts/test/model/accessors/flycam_accessors.spec.ts(2 hunks)frontend/javascripts/test/model/binary/cube.spec.ts(3 hunks)frontend/javascripts/test/model/binary/pullqueue.spec.ts(1 hunks)frontend/javascripts/test/model/model_resolutions.spec.ts(6 hunks)frontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts(2 hunks)frontend/javascripts/test/sagas/annotation_tool_disabled_info.spec.ts(2 hunks)frontend/javascripts/types/api_types.ts(1 hunks)frontend/javascripts/viewer/api/api_latest.ts(5 hunks)frontend/javascripts/viewer/geometries/materials/plane_material_factory.ts(1 hunks)frontend/javascripts/viewer/model/accessors/dataset_accessor.ts(4 hunks)frontend/javascripts/viewer/model/accessors/flycam_accessor.ts(3 hunks)frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts(2 hunks)frontend/javascripts/viewer/model/bucket_data_handling/layer_rendering_manager.ts(2 hunks)frontend/javascripts/viewer/model/bucket_data_handling/wkstore_adapter.ts(1 hunks)frontend/javascripts/viewer/model/data_layer.ts(1 hunks)frontend/javascripts/viewer/model/sagas/dataset_saga.ts(1 hunks)frontend/javascripts/viewer/model/sagas/flycam_info_cache_saga.ts(1 hunks)frontend/javascripts/viewer/model/sagas/volume/min_cut_saga.ts(1 hunks)frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts(1 hunks)frontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.ts(1 hunks)frontend/javascripts/viewer/model_initialization.ts(1 hunks)frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/bounding_box_selection_form_item.tsx(1 hunks)frontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/train_ai_model_form.tsx(1 hunks)frontend/javascripts/viewer/view/action-bar/ai_job_modals/tabs/train_ai_model_tab.tsx(1 hunks)frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx(2 hunks)frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx(2 hunks)frontend/javascripts/viewer/view/action-bar/download_modal_view.tsx(1 hunks)frontend/javascripts/viewer/view/context_menu.tsx(1 hunks)frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx(2 hunks)frontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsx(1 hunks)frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/proofread_saga.ts:1039-1039
Timestamp: 2024-11-22T17:18:04.217Z
Learning: In `frontend/javascripts/oxalis/model/sagas/proofread_saga.ts`, when calling `getMagInfo`, the use of `volumeTracingLayer.resolutions` is intentional and should not be changed to `volumeTracingLayer.mags`.
Learnt from: knollengewaechs
PR: scalableminds/webknossos#8266
File: frontend/javascripts/components/mag_selection.tsx:51-60
Timestamp: 2024-12-16T15:48:37.065Z
Learning: In the `MagSelection` component (`frontend/javascripts/components/mag_selection.tsx`), the selected magnification `value` will always be present in `allMags` because only existing values can be selected by the user.
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx:433-434
Timestamp: 2024-11-22T17:19:07.947Z
Learning: In the codebase, certain usages of `segmentationLayer.resolutions` are intentionally retained and should not be changed to `segmentationLayer.mags` during refactoring.
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts:125-125
Timestamp: 2024-11-22T17:17:39.914Z
Learning: In the `createNode` function within `skeletontracing_reducer_helpers.ts`, the property `resolution` is intentionally kept even when the parameter is renamed to `mag`.
📚 Learning: 2024-11-22T17:18:04.217Z
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/proofread_saga.ts:1039-1039
Timestamp: 2024-11-22T17:18:04.217Z
Learning: In `frontend/javascripts/oxalis/model/sagas/proofread_saga.ts`, when calling `getMagInfo`, the use of `volumeTracingLayer.resolutions` is intentional and should not be changed to `volumeTracingLayer.mags`.
Applied to files:
frontend/javascripts/test/fixtures/volumetracing_object.tsfrontend/javascripts/types/api_types.tsfrontend/javascripts/viewer/model/sagas/volume/proofread_saga.tsfrontend/javascripts/test/model/model_resolutions.spec.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsxfrontend/javascripts/viewer/model/bucket_data_handling/wkstore_adapter.tsfrontend/javascripts/viewer/model/sagas/flycam_info_cache_saga.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/components/bounding_box_selection_form_item.tsxfrontend/javascripts/viewer/model/sagas/volume/min_cut_saga.tsfrontend/javascripts/test/model/binary/cube.spec.tsfrontend/javascripts/viewer/view/action-bar/download_modal_view.tsxfrontend/javascripts/test/sagas/annotation_tool_disabled_info.spec.tsfrontend/javascripts/viewer/model/sagas/dataset_saga.tsfrontend/javascripts/admin/voxelytics/ai_model_list_view.tsxfrontend/javascripts/test/fixtures/hybridtracing_object.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/tabs/train_ai_model_tab.tsxfrontend/javascripts/viewer/api/api_latest.tsfrontend/javascripts/viewer/view/context_menu.tsxfrontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsxfrontend/javascripts/viewer/model/accessors/flycam_accessor.tsfrontend/javascripts/test/model/accessors/flycam_accessors.spec.tsfrontend/javascripts/viewer/geometries/materials/plane_material_factory.tsfrontend/javascripts/viewer/model/accessors/volumetracing_accessor.tsfrontend/javascripts/viewer/view/action-bar/create_animation_modal.tsxfrontend/javascripts/viewer/model/accessors/dataset_accessor.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/train_ai_model_form.tsxfrontend/javascripts/test/fixtures/dataset_server_object.tsfrontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsxfrontend/javascripts/viewer/model/data_layer.tsfrontend/javascripts/viewer/model_initialization.tsfrontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.tsfrontend/javascripts/test/model/binary/pullqueue.spec.tsfrontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.tsfrontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsxfrontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsxfrontend/javascripts/viewer/model/bucket_data_handling/layer_rendering_manager.ts
📚 Learning: 2024-11-22T17:18:43.411Z
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts:568-585
Timestamp: 2024-11-22T17:18:43.411Z
Learning: In the file `frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts`, the uniform name `allResolutions` should remain unchanged to maintain consistency with the shader code.
Applied to files:
frontend/javascripts/test/fixtures/volumetracing_object.tsfrontend/javascripts/types/api_types.tsfrontend/javascripts/test/model/model_resolutions.spec.tsfrontend/javascripts/test/fixtures/hybridtracing_object.tsfrontend/javascripts/test/model/accessors/flycam_accessors.spec.tsfrontend/javascripts/viewer/geometries/materials/plane_material_factory.tsfrontend/javascripts/test/fixtures/dataset_server_object.ts
📚 Learning: 2024-11-22T17:17:39.914Z
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts:125-125
Timestamp: 2024-11-22T17:17:39.914Z
Learning: In the `createNode` function within `skeletontracing_reducer_helpers.ts`, the property `resolution` is intentionally kept even when the parameter is renamed to `mag`.
Applied to files:
frontend/javascripts/test/fixtures/volumetracing_object.tsfrontend/javascripts/viewer/model/sagas/volume/proofread_saga.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsxfrontend/javascripts/viewer/model/sagas/flycam_info_cache_saga.tsfrontend/javascripts/viewer/model/sagas/volume/min_cut_saga.tsfrontend/javascripts/test/sagas/annotation_tool_disabled_info.spec.tsfrontend/javascripts/viewer/model/sagas/dataset_saga.tsfrontend/javascripts/test/fixtures/hybridtracing_object.tsfrontend/javascripts/viewer/view/context_menu.tsxfrontend/javascripts/viewer/model/accessors/flycam_accessor.tsfrontend/javascripts/test/model/accessors/flycam_accessors.spec.tsfrontend/javascripts/viewer/geometries/materials/plane_material_factory.tsfrontend/javascripts/viewer/model/accessors/volumetracing_accessor.tsfrontend/javascripts/viewer/view/action-bar/create_animation_modal.tsxfrontend/javascripts/test/fixtures/dataset_server_object.tsfrontend/javascripts/viewer/model_initialization.tsfrontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts
📚 Learning: 2024-11-22T17:19:07.947Z
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx:433-434
Timestamp: 2024-11-22T17:19:07.947Z
Learning: In the codebase, certain usages of `segmentationLayer.resolutions` are intentionally retained and should not be changed to `segmentationLayer.mags` during refactoring.
Applied to files:
frontend/javascripts/test/fixtures/volumetracing_object.tsfrontend/javascripts/types/api_types.tsfrontend/javascripts/test/model/model_resolutions.spec.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsxfrontend/javascripts/viewer/model/bucket_data_handling/wkstore_adapter.tsfrontend/javascripts/test/sagas/annotation_tool_disabled_info.spec.tsfrontend/javascripts/viewer/model/sagas/dataset_saga.tsfrontend/javascripts/admin/voxelytics/ai_model_list_view.tsxfrontend/javascripts/test/fixtures/hybridtracing_object.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/tabs/train_ai_model_tab.tsxfrontend/javascripts/viewer/api/api_latest.tsfrontend/javascripts/viewer/view/context_menu.tsxfrontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsxfrontend/javascripts/viewer/model/accessors/flycam_accessor.tsfrontend/javascripts/test/model/accessors/flycam_accessors.spec.tsfrontend/javascripts/viewer/model/accessors/volumetracing_accessor.tsfrontend/javascripts/viewer/model/accessors/dataset_accessor.tsfrontend/javascripts/test/fixtures/dataset_server_object.tsfrontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsxfrontend/javascripts/viewer/model/data_layer.tsfrontend/javascripts/viewer/model_initialization.tsfrontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsxfrontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsxfrontend/javascripts/viewer/model/bucket_data_handling/layer_rendering_manager.ts
📚 Learning: 2024-12-16T15:48:37.065Z
Learnt from: knollengewaechs
PR: scalableminds/webknossos#8266
File: frontend/javascripts/components/mag_selection.tsx:51-60
Timestamp: 2024-12-16T15:48:37.065Z
Learning: In the `MagSelection` component (`frontend/javascripts/components/mag_selection.tsx`), the selected magnification `value` will always be present in `allMags` because only existing values can be selected by the user.
Applied to files:
frontend/javascripts/viewer/model/sagas/volume/proofread_saga.tsfrontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsxfrontend/javascripts/viewer/view/action-bar/ai_job_modals/components/bounding_box_selection_form_item.tsxfrontend/javascripts/viewer/model/sagas/volume/min_cut_saga.tsfrontend/javascripts/viewer/view/action-bar/download_modal_view.tsxfrontend/javascripts/viewer/api/api_latest.tsfrontend/javascripts/viewer/view/context_menu.tsxfrontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsxfrontend/javascripts/viewer/model/accessors/flycam_accessor.tsfrontend/javascripts/viewer/geometries/materials/plane_material_factory.tsfrontend/javascripts/viewer/model/accessors/volumetracing_accessor.tsfrontend/javascripts/viewer/view/action-bar/create_animation_modal.tsxfrontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.tsfrontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsxfrontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx
📚 Learning: 2025-05-12T13:07:29.637Z
Learnt from: frcroth
PR: scalableminds/webknossos#8609
File: app/models/dataset/Dataset.scala:753-775
Timestamp: 2025-05-12T13:07:29.637Z
Learning: In the `updateMags` method of DatasetMagsDAO (Scala), the code handles different dataset types distinctly:
1. Non-WKW datasets have `magsOpt` populated and use the first branch which includes axisOrder, channelIndex, and credentialId.
2. WKW datasets will have `wkwResolutionsOpt` populated and use the second branch which includes cubeLength.
3. The final branch is a fallback for legacy data.
This ensures appropriate fields are populated for each dataset type.
Applied to files:
frontend/javascripts/viewer/model/sagas/dataset_saga.tsfrontend/javascripts/viewer/model/accessors/dataset_accessor.ts
📚 Learning: 2025-09-08T13:33:47.097Z
Learnt from: MichaelBuessemeyer
PR: scalableminds/webknossos#8824
File: frontend/javascripts/viewer/view/context_menu.tsx:1033-1039
Timestamp: 2025-09-08T13:33:47.097Z
Learning: In frontend/javascripts/viewer/view/context_menu.tsx, the segmentIdLabel intentionally shows the unmapped segment ID (maybeUnmappedSegmentId) in the "within Segment ${id}" text when proofreading is active, as this can be helpful for users. The mapped segment ID (segmentIdAtPosition) is only used as a fallback when the unmapped ID is not available.
Applied to files:
frontend/javascripts/viewer/view/context_menu.tsx
🧬 Code graph analysis (26)
frontend/javascripts/types/api_types.ts (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts (2)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/volume/VolumeTracingService.scala (1)
volumeTracingLayer(641-660)
frontend/javascripts/test/model/model_resolutions.spec.ts (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx (3)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
colorLayer(12-31)frontend/javascripts/viewer/view/action-bar/ai_job_modals/constants.ts (1)
MEAN_VX_SIZE(31-35)
frontend/javascripts/viewer/model/bucket_data_handling/wkstore_adapter.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/model/sagas/flycam_info_cache_saga.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/bounding_box_selection_form_item.tsx (1)
frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
colorLayer(12-31)
frontend/javascripts/viewer/model/sagas/volume/min_cut_saga.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/test/model/binary/cube.spec.ts (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/viewer/view/action-bar/download_modal_view.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/model/sagas/dataset_saga.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/tabs/train_ai_model_tab.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/api/api_latest.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (2)
getMagInfo(39-39)getLayerByName(145-164)
frontend/javascripts/viewer/view/context_menu.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/model/accessors/flycam_accessor.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (2)
getMagInfo(39-39)getLayerByName(145-164)
frontend/javascripts/viewer/geometries/materials/plane_material_factory.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx (2)
frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
colorLayer(12-31)frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/train_ai_model_form.tsx (2)
frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
colorLayer(12-31)frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.ts (1)
frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
colorLayer(12-31)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/model/bucket_data_handling/layer_rendering_manager.ts (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (46)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/bounding_box_selection_form_item.tsx (1)
35-35:mags[0]swap looks correct
colorLayer.magsalready provides the same triple the validator needs, so pulling the first entry keeps the export-size checks working while aligning with the new naming.frontend/javascripts/viewer/view/action-bar/ai_job_modals/tabs/train_ai_model_tab.tsx (1)
21-21: Ensure segmentation layers exposemagsbefore swapping fromresolutions.
getSegmentationLayerByHumanReadableNamestill returns layer objects that (as of the previous rollout) only surfacedresolutions. Switching this call tosegmentationLayer.magswill handgetMagInfoanundefinedargument until the store/backend payload is updated, which would blow up the modal. Please confirm that the layer DTOs in the dataset/tracing store already exposemags, or keep theresolutionsfallback until that migration lands. Based on learningsfrontend/javascripts/viewer/model/bucket_data_handling/layer_rendering_manager.ts (3)
193-193: LGTM: Correct migration from resolutions to mags.The change from
layer.resolutionstolayer.magsaligns with the broader refactoring effort and maintains the same functionality for calculatingmaximumMagIndex.
202-202: LGTM: Consistent use of mags for getDenseMags call.The change from
layer.resolutionstolayer.magsin thegetDenseMags()call is correct and maintains consistency with the previous change on line 193.
193-202: All getMagInfo calls now use layer.mags
Search across the codebase found zero instances of getMagInfo with .resolutions.frontend/javascripts/viewer/model/sagas/flycam_info_cache_saga.ts (1)
110-110: Switch to layer.mags when computing dense mags — looks goodChange aligns with the repo‑wide migration and the worker’s intent.
Please double‑check that AsyncGetMaximumZoomForAllMags (and _getMagInfo) now uniformly expect mags arrays across viewer modules to avoid mixed inputs.
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (1)
102-103: Use mags for finest mag selection — OKConsistent with API/types migration; no control‑flow changes.
Based on learnings
frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx (1)
66-70: Use colorLayer.mags in texture mag selection — OKThis keeps the heuristic unchanged while sourcing from the new mags field.
frontend/javascripts/test/sagas/annotation_tool_disabled_info.spec.ts (2)
24-35: Tests: migrate resolutions → mags for layer[0] — OKMatches the new data shape; comment explains the need for more mags.
38-49: Tests: migrate resolutions → mags for layer[1] — OKConsistent with the updated fixtures and accessors.
frontend/javascripts/test/model/accessors/flycam_accessors.spec.ts (1)
29-35: LGTM - Test fixture property update aligned with PR objective.The change from
resolutionstomagsin the test fixture correctly aligns with the broader codebase migration. The magnification arrays remain identical, ensuring test behavior consistency while updating to the new property naming convention.Also applies to: 43-47
frontend/javascripts/viewer/model/accessors/volumetracing_accessor.ts (2)
205-205: LGTM - Consistent mag data source update.The change from
segmentationLayer.resolutionstosegmentationLayer.magsproperly aligns with the project-wide migration while maintaining identical functionality forgetMagInfocalls.
539-539: LGTM – Mag data source consistently updated. Change aligns with the broader refactoring andgetMagInfobehavior remains unchanged.frontend/javascripts/test/model/binary/cube.spec.ts (1)
54-63: LGTM - Test mock property renamed consistently.The change from
resolutionstomagsin the mocked layer objects correctly reflects the API change while maintaining identical test data and behavior. The MagInfo instantiation properly uses the updated property name.Also applies to: 278-280
frontend/javascripts/test/fixtures/volumetracing_object.ts (1)
62-66: LGTM - Test fixture updated to match API changes.The property rename from
resolutionstomagsin the test fixture data layers correctly aligns with the broader API migration while preserving the same magnification values for test consistency.frontend/javascripts/viewer/view/context_menu.tsx (1)
1699-1699: LGTM - Context menu mag data source updated.The change from
visibleSegmentationLayer.resolutionstovisibleSegmentationLayer.magscorrectly updates the magnification data source while maintaining the samegetMagInfofunctionality for segment volume calculations.frontend/javascripts/viewer/model/data_layer.ts (1)
40-40: LGTM - Data layer initialization updated for mags property.The change from
layerInfo.resolutionstolayerInfo.magscorrectly updates the magnification data source during DataLayer construction. The downstream usage throughgetMagInfo(this.mags)remains consistent.frontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts (1)
71-80: LGTM - Test setup updated for mags property.The change from
resolutionstomagsin the mocked layer correctly aligns the test with the API migration. The MagInfo instantiation and test behavior remain identical.frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx (1)
97-97: LGTM - Modal mag data sources updated consistently.Both changes from
selectedSegmentationLayer.resolutionstoselectedSegmentationLayer.magscorrectly align with the project-wide migration:
- Line 97: RestrictMagnificationSlider mag info derivation
- Line 200: CreateExplorativeModal mag info computation
The magnification functionality and UI behavior remain unchanged.
Also applies to: 200-200
frontend/javascripts/test/model/binary/pullqueue.spec.ts (1)
42-42: LGTM: test mock migrated to magsThe mocked layer now exposes mags, consistent with API types and consumers.
frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (1)
298-300: Use mags for tooltip mag list — OKSwapping to getMagInfo(layer.mags) aligns with the type change and downstream helpers.
frontend/javascripts/viewer/model/sagas/dataset_saga.ts (1)
117-120: Switch to mags for downsampling warning — OKgetMagInfo(dataLayer.mags) matches the new layer shape. Ensure all dataLayers now provide mags at runtime (incl. legacy paths) to avoid null/undefined access.
frontend/javascripts/viewer/model/sagas/volume/min_cut_saga.ts (1)
287-289: Use mags for min-cut mag selection — OKSelecting magInfo from volumeTracingLayer.mags is consistent with the type migration.
frontend/javascripts/viewer/model/sagas/volume/proofread_saga.ts (1)
1239-1247: Proofreading now bases magInfo on mags — double-check intentPreviously, certain proofreading paths intentionally used resolutions. With the global type change, using mags is likely required, but please verify the chosen agglomerateFileMag/zoomstep mirrors prior behavior (finest vs. current mag) and doesn’t regress operations.
Based on learnings
frontend/javascripts/test/model/model_resolutions.spec.ts (5)
7-7: Test suite rename to mags — OK
25-35: Test fixtures use mags — OKShapes align with Vector3[] and convertToDenseMag inputs.
59-59: Helper updated to mags — OK
81-94: getMagUnion test data migrated — OK
108-121: Mixed mags union test migrated — OKfrontend/javascripts/viewer/model/accessors/flycam_accessor.ts (3)
418-425: getCurrentMag now reads mags — OKMatches dataset layer shape and MagInfo API.
631-633: Unrenderable-layer check uses mags — OKmagInfo(layer.mags) keeps logic intact.
679-681: Active mag info derived from mags — OKConsistent with global migration.
frontend/javascripts/types/api_types.ts (1)
95-101: No.resolutionsreferences remain
Scanning returned zero hits; please confirm parsing/serialization populatesmagsfor WKW, Zarr, and legacy formats before merging.frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
21-27: Fixture migrated to mags correctlyProperty rename to mags with same triplets looks consistent with APIColorLayer and wider PR.
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (4)
44-46: Use mags for per-layer MagInfoSwitch to layer.mags for getMagInfoByLayer is correct and consistent with the new API shape.
69-71: Magnification union now based on magsIterating over layer.mags and grouping by max axis preserves previous behavior under the new field name.
Also applies to: 86-91
94-96: Widest mags computed from magsconvertToDenseMag(layer.mags) aligns with the refactor; no issues.
139-140: Visible segmentation layer MagInfo via magsGood use of mags; empty fallback remains handled above.
frontend/javascripts/viewer/geometries/materials/plane_material_factory.ts (1)
920-921: Vertex alignment MagInfo derived from magsUsing getMagInfo(layer.mags) is consistent with dataset_accessor and the PR direction.
frontend/javascripts/viewer/model_initialization.ts (2)
615-617: Robust conversion of tracing magsMapping ServerVolumeTracing.mags {x,y,z} -> Vector3[] with fallback [[1,1,1]] covers legacy tracings.
625-633: Tracing layer now exposes magsSetting mags on the synthesized APISegmentationLayer matches downstream consumers.
frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx (1)
46-48: Mitochondria infers on finest mag via magsgetMagInfo(colorLayer.mags).getFinestMag() is the right source after the rename.
frontend/javascripts/test/fixtures/dataset_server_object.ts (2)
13-20: Color layer fixture: mags migration LGTMTriplets retained, unsorted comment preserved.
36-43: Segmentation layer fixture: mags migration LGTMProperty rename aligns with types and other fixture changes.
frontend/javascripts/viewer/api/api_latest.ts (1)
689-696: API paths now consistently source MagInfo from magsAll updated call sites (segment registration, getDataValue, getDataForBoundingBox, getViewportData, raw download URL) correctly use layer.mags.
Also applies to: 1875-1877, 1932-1941, 1978-1983, 2139-2141
frontend/javascripts/viewer/view/left-border-tabs/modals/add_volume_layer_modal.tsx (1)
148-152: Switch to mags is safe. APISegmentationLayer defines onlymags(noresolutions), and all calls togetMagInfoalready expect amagsarray—no interim guard required.
frontend/javascripts/viewer/view/action-bar/ai_job_modals/forms/train_ai_model_form.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (1)
1143-1143: Fix type mismatch in _.differenceWith comparison.
possibleMagsisVector3[], butvolumeTracingMagsisArray<{ mag: Vector3; path?: string }>. Comparing these with_.isEqualwill never find matches because the shapes differ. Extract themagproperty before comparison.Apply this diff:
- const magsToDownsample = _.differenceWith(possibleMags, volumeTracingMags, _.isEqual); + const magsToDownsample = _.differenceWith(possibleMags, volumeTracingMags.map(m => m.mag), _.isEqual);
♻️ Duplicate comments (1)
frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx (1)
155-157: Verify mags availability on dataset segmentation layers.A previous review flagged that
getSegmentationLayerByNamemight return dataset segmentation layers that only exposeresolutions, notmags. If the migration is incomplete, accessingsegmentationLayer.magscould passundefinedtogetMagInfo, causing a runtime failure.Consider adding a fallback until all dataset layers are confirmed to have the
magsfield:- return getMagInfo(segmentationLayer.mags); + return getMagInfo(segmentationLayer.mags ?? segmentationLayer.resolutions ?? []);Run the following script to check if any dataset segmentation layers in the codebase still use
resolutions:#!/bin/bash # Search for segmentation layer definitions that might still use resolutions field rg -nP --type=ts -C3 'category.*segmentation|segmentation.*category' | rg -A10 -B10 'resolutions'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx(1 hunks)frontend/javascripts/test/fixtures/dataset_server_object.ts(2 hunks)frontend/javascripts/test/fixtures/hybridtracing_object.ts(1 hunks)frontend/javascripts/test/fixtures/volumetracing_object.ts(1 hunks)frontend/javascripts/test/model/accessors/flycam_accessors.spec.ts(2 hunks)frontend/javascripts/test/model/model_resolutions.spec.ts(5 hunks)frontend/javascripts/test/sagas/annotation_tool_disabled_info.spec.ts(1 hunks)frontend/javascripts/types/api_types.ts(1 hunks)frontend/javascripts/viewer/model/accessors/dataset_accessor.ts(5 hunks)frontend/javascripts/viewer/model/data_layer.ts(2 hunks)frontend/javascripts/viewer/model/helpers/mag_info.ts(2 hunks)frontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.ts(1 hunks)frontend/javascripts/viewer/model_initialization.ts(1 hunks)frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/annotations_csv_input.tsx(1 hunks)frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/bounding_box_selection_form_item.tsx(1 hunks)frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx(3 hunks)frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx(2 hunks)frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/javascripts/test/model/model_resolutions.spec.ts
- frontend/javascripts/types/api_types.ts
- frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/bounding_box_selection_form_item.tsx
- frontend/javascripts/viewer/view/action-bar/create_animation_modal.tsx
- frontend/javascripts/viewer/model/sagas/volume/quick_select/quick_select_heuristic_saga.ts
- frontend/javascripts/viewer/model_initialization.ts
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts:125-125
Timestamp: 2024-11-22T17:17:39.914Z
Learning: In the `createNode` function within `skeletontracing_reducer_helpers.ts`, the property `resolution` is intentionally kept even when the parameter is renamed to `mag`.
📚 Learning: 2024-11-22T17:18:04.217Z
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/proofread_saga.ts:1039-1039
Timestamp: 2024-11-22T17:18:04.217Z
Learning: In `frontend/javascripts/oxalis/model/sagas/proofread_saga.ts`, when calling `getMagInfo`, the use of `volumeTracingLayer.resolutions` is intentional and should not be changed to `volumeTracingLayer.mags`.
Applied to files:
frontend/javascripts/test/fixtures/volumetracing_object.tsfrontend/javascripts/admin/voxelytics/ai_model_list_view.tsxfrontend/javascripts/test/fixtures/hybridtracing_object.tsfrontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsxfrontend/javascripts/viewer/view/action-bar/ai_job_modals/components/annotations_csv_input.tsxfrontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsxfrontend/javascripts/viewer/model/accessors/dataset_accessor.tsfrontend/javascripts/viewer/model/helpers/mag_info.tsfrontend/javascripts/test/fixtures/dataset_server_object.tsfrontend/javascripts/viewer/model/data_layer.ts
📚 Learning: 2024-11-22T17:19:07.947Z
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx:433-434
Timestamp: 2024-11-22T17:19:07.947Z
Learning: In the codebase, certain usages of `segmentationLayer.resolutions` are intentionally retained and should not be changed to `segmentationLayer.mags` during refactoring.
Applied to files:
frontend/javascripts/admin/voxelytics/ai_model_list_view.tsxfrontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsxfrontend/javascripts/test/fixtures/dataset_server_object.tsfrontend/javascripts/viewer/model/data_layer.ts
📚 Learning: 2024-12-16T15:48:37.065Z
Learnt from: knollengewaechs
PR: scalableminds/webknossos#8266
File: frontend/javascripts/components/mag_selection.tsx:51-60
Timestamp: 2024-12-16T15:48:37.065Z
Learning: In the `MagSelection` component (`frontend/javascripts/components/mag_selection.tsx`), the selected magnification `value` will always be present in `allMags` because only existing values can be selected by the user.
Applied to files:
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts
🧬 Code graph analysis (8)
frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx (2)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (2)
getMagInfo(39-39)getSegmentationLayerByName(167-178)
frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (1)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/annotations_csv_input.tsx (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx (4)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (1)
getMagInfo(39-39)frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
colorLayer(12-31)frontend/javascripts/viewer/view/action-bar/ai_job_modals/constants.ts (1)
MEAN_VX_SIZE(31-35)frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (2)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)frontend/javascripts/viewer/model/helpers/mag_info.ts (2)
MagInfo(11-225)convertToDenseMags(227-257)
frontend/javascripts/viewer/model/helpers/mag_info.ts (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/test/model/accessors/flycam_accessors.spec.ts (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
frontend/javascripts/viewer/model/data_layer.ts (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
🔇 Additional comments (26)
frontend/javascripts/test/sagas/annotation_tool_disabled_info.spec.ts (1)
24-47: LGTM! Test fixture correctly updated to mags structure.The test fixture has been properly updated to use the new
mags: [{ mag: Vector3 }]structure instead of the oldresolutions: Vector3[]format. The comment on line 24 has also been appropriately updated to reference "mags" instead of "resolutions".frontend/javascripts/test/fixtures/volumetracing_object.ts (1)
62-62: LGTM! Fixture correctly updated to mags structure.The test fixture has been properly updated to use the new
mags: [{ mag: Vector3 }]structure.frontend/javascripts/viewer/model/data_layer.ts (2)
19-19: LGTM! Property type correctly updated.The
magsproperty type has been correctly updated fromArray<Vector3>toArray<{ mag: Vector3 }>to match the new data structure.
40-40: LGTM! Initialization source updated correctly.The initialization has been properly updated to read from
layerInfo.magsinstead of the oldlayerInfo.resolutionsfield.frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx (1)
152-152: LGTM! Fallback mags structure correctly updated.The fallback value has been properly updated to use the new
mags: [{ mag: Vector3 }]structure.frontend/javascripts/test/model/accessors/flycam_accessors.spec.ts (3)
11-11: LGTM!The Vector3 type assertion for
topLeftcorrectly matches the expected type definition.
28-36: LGTM!The migration from
resolutionstomagsis consistent with the PR's objective to reshape magnification data. Each magnification is correctly wrapped in an object with amagproperty, and theadditionalAxesproperty has been properly added.
42-44: LGTM!The
magsproperty correctly replacesresolutionswith the new object-wrapped structure, andadditionalAxeshas been appropriately added.frontend/javascripts/test/fixtures/hybridtracing_object.ts (1)
21-27: LGTM!The
magsproperty correctly replacesresolutionswith each magnification wrapped in an object containing themagproperty. The values remain unchanged, ensuring backward compatibility.frontend/javascripts/test/fixtures/dataset_server_object.ts (2)
13-20: LGTM!The
magsproperty correctly replacesresolutionswith each magnification wrapped in an object. The intentional unsorted ordering (with the "unsorted on purpose" comment) is properly preserved for testing edge cases.
36-43: LGTM!The
magsproperty correctly replacesresolutionswith the same object-wrapped structure as the color layer, maintaining the intentional unsorted ordering for test coverage.frontend/javascripts/viewer/model/helpers/mag_info.ts (2)
45-45: LGTM!The call site correctly uses the renamed
convertToDenseMagsfunction, maintaining consistency with the plural naming convention.
227-227: LGTM!The function rename from
convertToDenseMagtoconvertToDenseMagsuses the plural form, which better reflects that it operates on an array of magnifications. The function body remains unchanged, preserving the existing logic.frontend/javascripts/viewer/model/accessors/dataset_accessor.ts (6)
31-31: LGTM!The import statement correctly references the renamed
convertToDenseMagsfunction from the mag_info helper.
33-34: LGTM!The
_getMagInfofunction correctly adapts to the newmagsdata structure by acceptingArray<{ mag: Vector3 }>and mapping to extract themagproperty for theMagInfoconstructor.
45-45: LGTM!The call site correctly uses
layer.magsinstead oflayer.resolutions, aligning with the new data model.
70-71: LGTM!The
getMagnificationUnionfunction correctly adapts to the newmagsstructure by iterating overlayer.magsand extracting themagproperty from each object.
96-96: LGTM!The
getWidestMagsfunction correctly adapts to uselayer.magsand calls the renamedconvertToDenseMagsfunction, properly extracting themagproperty from each object.
140-140: LGTM!The
_getMagInfoOfVisibleSegmentationLayerfunction correctly usessegmentationLayer.magsinstead ofsegmentationLayer.resolutions, consistent with the broader data model migration.frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx (2)
298-298: LGTM! Migration to mags correctly applied.The change from
layer.resolutionstolayer.magsaligns with the PR's objective to replace resolutions with the new mags structure.
1127-1127: LGTM! Correctly extracts mag values.The destructuring and mapping correctly adapts to the new
magsstructure by extracting themagproperty.frontend/javascripts/viewer/view/action-bar/ai_job_modals/utils.tsx (3)
50-67: LGTM! Magnification selection logic correctly migrated.The loop properly extracts
magfrom eachmagsarray element and maintains the same selection logic. All accesses tomagInfo.magare consistent throughout the calculation.
128-128: No occurrences of volumeTracingMags found; manually verify that any constructors, consumers, and tests handling AnnotationInfoForAITrainingJob are updated to the new{ mag: Vector3 }[][]signature.
46-46: EnsuregetMagInfomemoization handles newmagsreferences
Default lodash memoize uses reference equality, so ifcolorLayer.magsarrays are recreated on each render, the cache will always miss. Verify how and wherecolorLayer.magsis constructed and consider supplying a custom resolver or stabilizing its reference before callinggetMagInfo.frontend/javascripts/viewer/view/action-bar/ai_job_modals/components/annotations_csv_input.tsx (2)
112-112: LGTM! Improved clarity by extracting the transformation.Extracting
volumeTracingMagsinto a separate variable improves readability and aligns with the broader migration fromresolutionstomags.
103-107: Verify emptymagshandling
The current fallback ([{ mag: [1,1,1] }]) only applies whenmagsisundefined, not when it’s an empty array—sovolumeTracingMagsmay contain empty arrays ifmags: []. Confirm whether downstream consumers expect at least one magnification level or if empty arrays are acceptable; adjust the check to treat[]likeundefinedif needed.
frontend/javascripts/viewer/view/left-border-tabs/layer_settings_tab.tsx
Outdated
Show resolved
Hide resolved
philippotto
changed the title
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/javascripts/test/model/model_resolutions.spec.ts (2)
77-102: Minor formatting inconsistency in object literal.Line 83 has
{mag: [4, 4, 1]}without a space after the colon, while lines 84-86, 91-93 use{mag:with a space. Consider consistent spacing for readability.Apply this diff to fix the spacing:
mags: [ - {mag: [4, 4, 1]}, + {mag: [4, 4, 1]}, {mag: [8, 8, 1]}, {mag: [16, 16, 2]}, {mag: [32, 32, 4]},
104-139: Minor formatting inconsistency in object literal.Line 110 has
{mag: [4, 4, 1]}without a space after the colon, while lines 111-113, 118-120 use{mag:with a space. Consider consistent spacing for readability.Apply this diff to fix the spacing:
mags: [ - {mag: [4, 4, 1]}, + {mag: [4, 4, 1]}, {mag: [8, 8, 1]}, {mag: [16, 16, 2]}, {mag: [32, 32, 4]},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/javascripts/test/model/model_resolutions.spec.ts(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2024-11-22T17:18:04.217Z
Learnt from: dieknolle3333
PR: scalableminds/webknossos#8168
File: frontend/javascripts/oxalis/model/sagas/proofread_saga.ts:1039-1039
Timestamp: 2024-11-22T17:18:04.217Z
Learning: In `frontend/javascripts/oxalis/model/sagas/proofread_saga.ts`, when calling `getMagInfo`, the use of `volumeTracingLayer.resolutions` is intentional and should not be changed to `volumeTracingLayer.mags`.
Applied to files:
frontend/javascripts/test/model/model_resolutions.spec.ts
🧬 Code graph analysis (1)
frontend/javascripts/test/model/model_resolutions.spec.ts (1)
frontend/javascripts/viewer/constants.ts (1)
Vector3(14-14)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
🔇 Additional comments (3)
frontend/javascripts/test/model/model_resolutions.spec.ts (3)
1-18: LGTM! Import and test names properly updated.The import statement and test suite names have been correctly updated to reflect the rename from
convertToDenseMagtoconvertToDenseMags. The simple test case correctly exercises the function with a direct array of Vector3 values.
20-64: LGTM! Data structure correctly updated to new mags format.The test properly reflects the new data structure where
resolutions: Vector3[]becomesmags: { mag: Vector3 }[]. Thedensifyhelper correctly extracts themagproperty before passing toconvertToDenseMags, and test expectations remain accurate.
66-75: LGTM! Empty case test remains valid.This test correctly verifies the behavior with an empty dataset and requires no changes related to the mags refactoring.
philippotto
changed the title
daniel-wer
left a comment
daniel-wer
left a comment
There was a problem hiding this comment.
LGTM, didn't notice any issues 👍
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
(Please delete unneeded items, merge only when none are left open)
[ ] Added changelog entryirrelevant to end user