Rename resolution to mag in local variables #8168
Conversation
…del_initialization, api_latest and plane_material_factory
📝 Walkthrough📝 WalkthroughWalkthroughThis pull request includes significant updates to the WEBKNOSSOS codebase, primarily focusing on renaming terminology from "resolution" to "magnification" for consistency. It introduces asynchronous image file reading from the datastore, improved error messages for job initiation, and new functionality for handling Neuroglancer URIs. The update also addresses various bugs, including performance issues and color changes in segments, while removing Google Analytics integration. No new features or breaking changes are introduced. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Awesome, thanks for taking the time for this! I’m not so sure about CreateNodeAction. I think some incompatibilities may arise. The CreateNodeAction in the backend expects resolution, and will always default to 1 if it is not set. I think it won’t look for mag. I’m not sure how the backend DataLayer maps to the frontend one, so that one may be fine. Is it the one used in Dataset.dataSource.dataLayers? There, the backend will send "resolutions". |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (29)
frontend/javascripts/oxalis/model/data_layer.ts (2)
43-43: LGTM: Function call updated consistently.The
getMagInfocall has been updated to usethis.mags. Consider updating the JSDoc comments (if any exist) for this class to reflect the new terminology.
Line range hint
1-67: Consider adding migration documentation.While the code changes are consistent, it would be helpful to document this terminology change in:
- Class-level JSDoc comments explaining the transition from "resolution" to "mag"
- A note about the intentional retention of "resolution" in external interfaces
frontend/javascripts/oxalis/workers/async_bucket_picker.worker.ts (1)
Based on the search results, I can now provide the final response:
Testing gap identified for async bucket picker worker
The codebase lacks dedicated test coverage for the async bucket picker worker functionality. While there are tests using view modes (MODE_ARBITRARY) in other contexts like URL management and prefetch saga, there are no specific tests for:
- determineBucketsForOblique function
- determineBucketsForFlight function
- PriorityQueue bucket handling
This makes the rename changes more risky as there's no automated verification of the bucket picking logic across different view modes.
🔗 Analysis chain
Line range hint
43-87: Consider testing all view modes.While the changes are straightforward renames, this worker handles critical view functionality. Please ensure that all view modes (arbitrary plane, arbitrary, and default) are tested to verify that bucket picking continues to work correctly with the renamed parameters.
Let me gather more information about the test coverage for the async bucket picker functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Look for test files that should be updated # Search for related test files fd -e spec.ts -e test.ts async_bucket_picker # Search for test cases using 'resolution' in the test files rg -i 'resolution' -g '*.spec.ts' -g '*.test.ts'Length of output: 3280
Script:
#!/bin/bash # Search for test cases related to bucket picking and view modes rg -A 5 "MODE_ARBITRARY|MODE_ARBITRARY_PLANE" -g "*.spec.ts" -g "*.test.ts" # Search for any existing tests for async_bucket_picker rg -A 5 "determineBucketsForOblique|determineBucketsForFlight" -g "*.spec.ts" -g "*.test.ts" # Look for any test utilities or mocks related to bucket picking rg -A 5 "bucketQueue|PriorityQueue" -g "*.spec.ts" -g "*.test.ts"Length of output: 2235
frontend/javascripts/test/model/model_resolutions.spec.ts (3)
Line range hint
19-61: Update property name to maintain consistent terminology.While the test variables have been updated to use "mag", the input data structure still uses "resolutions" as the property name. This should be updated to maintain consistency with the new terminology.
const dataset = { dataSource: { dataLayers: [ { - resolutions: [ + magnifications: [ [16, 16, 2], [2, 2, 1], [4, 4, 1], [8, 8, 1], [32, 32, 4], ] as Vector3[], }, { - resolutions: [[32, 32, 4]] as Vector3[], + magnifications: [[32, 32, 4]] as Vector3[], }, ], }, };
Line range hint
73-134: Update property names in remaining test cases for consistency.Both test cases still use "resolutions" in their input data structures. This should be updated to "magnifications" to maintain consistency with the new terminology.
const dataset = { dataSource: { dataLayers: [ { - resolutions: [ + magnifications: [ [4, 4, 1], [8, 8, 1], [16, 16, 2], [32, 32, 4], ], }, { - resolutions: [ + magnifications: [ [2, 2, 1], [8, 8, 1], [32, 32, 4], ], }, ], }, };
File rename needed as part of terminology migration
The file
model_resolutions.spec.tsshould be renamed tomodel_magnification.spec.tsto align with the codebase's terminology. This is supported by:
- This is the only test file with "resolution" in its name that's directly related to magnification functionality
- The file tests
getMagnificationUnionfunctionality- Other files in the codebase consistently use "magnification" terminology
🔗 Analysis chain
Line range hint
1-7: Update file name to reflect new terminology.The file name
model_resolutions.spec.tsshould be updated tomodel_magnification.spec.tsto maintain consistency with the new terminology.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that other files referencing this test file have been updated rg -l "model_resolutions.spec" --type tsLength of output: 40
Script:
#!/bin/bash # Let's verify if there are any references to "resolution" in test file names fd "resolution.*\.spec\.(ts|js)" # Let's also check for any references to "magnification" in test file names fd "magnification.*\.spec\.(ts|js)" # Let's check for any imports or references to resolution-related terms in test files rg -l "resolution" --type ts --glob "*spec.ts" # Let's verify the usage of getMagnificationUnion to understand the context better rg "getMagnificationUnion" --type ts -A 2Length of output: 3485
frontend/javascripts/test/model/flycam_accessors.spec.ts (1)
Line range hint
1-141: Consider additional improvements for maintainability.While the variable renaming is complete, consider:
- Updating comments to use "magnification" instead of "resolution" for consistency
- Addressing the TypeScript errors (ts-expect-error) instead of suppressing them
frontend/javascripts/oxalis/controller/combinations/move_handlers.ts (1)
Line range hint
78-78: Fix TypeScript type issues.There are two TypeScript type issues that should be addressed:
- OrthoView type mismatch in the includes() check
- Vector type mismatch in moveFlycamOrthoAction parameter
Consider fixing these type issues:
- // @ts-expect-error ts-migrate(2345) + if (OrthoViewValuesWithoutTDView.includes(activeViewport as OrthoView)) {- // @ts-expect-error ts-migrate(2345) + Store.dispatch(moveFlycamOrthoAction(moveVector as Vector3, activeViewport));Also applies to: 134-134
frontend/javascripts/oxalis/model/sagas/clip_histogram_saga.ts (1)
31-31: LGTM! Consider enhancing the comment.The variable renaming from
desiredResolutionIndextodesiredMagIndexaligns with the standardization effort.Consider updating the comment to explicitly mention why we start from mag 2:
- // Find a viable mag to compute the histogram on - // Ideally, we want to avoid mags 1 and 2 to keep + // Find a viable magnification level to compute the histogram on + // We want to avoid magnification levels 1 and 2 to keepCHANGELOG.unreleased.md (1)
30-30: Consider improving the changelog entry specificity.The current changelog entry is quite broad and doesn't clearly communicate the scope of changes. Since this PR specifically targets local variable renaming in certain components, it would be helpful to make this clear to users and developers.
The suggested changes above would:
- Accurately reflect the PR's scope
- Help developers understand which components were affected
- Make it easier to track the progression of the resolution-to-mag renaming effort
frontend/javascripts/oxalis/model/helpers/position_converter.ts (1)
97-99: Consider tracking the TODO as a separate issue.While the variable renaming is correct, there's a TODO comment above this function suggesting it should be converted to use
zoomedAddressToAnotherZoomStepWithInfo. This technical debt should be tracked.Would you like me to create a GitHub issue to track the TODO for converting this function?
frontend/javascripts/oxalis/view/left-border-tabs/modals/add_volume_layer_modal.tsx (1)
157-158: Consider extracting duplicated mag restrictions.The mag restriction calculations are duplicated in two places. Consider extracting these into a shared constant or helper function to improve maintainability.
+ const magRestrictions = { + min: Math.max(...magInfo.getMagByIndexOrThrow(magIndices[0])), + max: Math.max(...magInfo.getMagByIndexOrThrow(magIndices[1])), + }; if (selectedSegmentationLayerName == null) { await addAnnotationLayer(tracing.annotationId, tracing.annotationType, { typ: "Volume", name: newLayerName, fallbackLayerName: undefined, - magRestrictions: { - min: minMagAllowed, - max: maxMagAllowed, - }, + magRestrictions, }); } else { // ... await addAnnotationLayer(tracing.annotationId, tracing.annotationType, { typ: "Volume", name: newLayerName, fallbackLayerName, - magRestrictions: { - min: minMagAllowed, - max: maxMagAllowed, - }, + magRestrictions, mappingName: maybeMappingName, }); }Also applies to: 166-167, 193-194
frontend/javascripts/oxalis/model/bucket_data_handling/wkstore_adapter.ts (1)
Line range hint
1-300: Consider documenting the resolution/mag terminology transition.The codebase appears to be transitioning from "resolution" to "mag" terminology in frontend code while maintaining backward compatibility with backend interfaces. Consider:
- Adding a comment in the codebase explaining this transition strategy
- Creating a tracking issue for the eventual backend updates
- Updating the developer documentation to clarify the terminology change
This will help future contributors understand the current state and planned evolution of the codebase.
frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (1)
Found one remaining instance of "resolutions" that should be renamed to "mags"
The file still contains one instance that needs to be updated:
- Line containing
magInfo = getMagInfo(visibleSegmentationLayer.resolutions)should usemagsinstead ofresolutions🔗 Analysis chain
Line range hint
1-267: Verify complete renaming in the file.Let's ensure no instances of "resolution" remain in the file that should have been renamed to "mag".
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for remaining instances of "resolution" in the file # that might need to be renamed rg -i "resolution" "frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx"Length of output: 179
frontend/javascripts/admin/tasktype/task_type_create_view.tsx (2)
142-142: Consider removing unused destructured variableThe
_isMagRestrictedvariable is destructured but never used (indicated by the underscore prefix). Consider removing it from the destructuring pattern if it's not needed.- isMagRestricted: _isMagRestricted,
Line range hint
1-516: Consider component decomposition for better maintainabilityThe TaskTypeCreateView component handles multiple responsibilities (creation/editing, form validation, conditional rendering). Consider:
- Splitting into separate Create and Edit components
- Extracting validation logic into a separate utility
- Creating dedicated components for different form sections (basic info, settings, magnification restrictions)
This would improve maintainability and make the code more testable.
frontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts (1)
Line range hint
1-671: Consider adding edge case tests.While the current test suite covers the basic scenarios well, consider adding tests for the following edge cases:
- Invalid magnification values
- Empty voxel maps
- Boundary conditions at maximum/minimum magnification levels
Example test case:
test("should handle invalid magnification values gracefully", (t) => { const { cube } = t.context; const sourceVoxelMap = getEmptyVoxelMap(); const labeledVoxelsMap = new Map([[cube.getOrCreateBucket([0, 0, 0, 1]).zoomedAddress, sourceVoxelMap]]); t.throws(() => { sampleVoxelMapToMag( labeledVoxelsMap, cube, [-1, -1, -1], // invalid mag 1, [1, 1, 1], 0, [0, 1, 2], 5, ); }, { instanceOf: Error }); });frontend/javascripts/oxalis/model/sagas/quick_select_heuristic_saga.ts (3)
207-207: Consider caching boundingBox conversion results.The
fromMag1ToMagoperation could be computationally expensive for large bounding boxes. Consider caching the result if it's used multiple times.
Line range hint
599-636: Improve type safety in maskToRGBA function.The function could benefit from stronger type safety for the normalize function and array access.
Consider these improvements:
- const normalize = - max === min - ? (val: number) => (val > 0 ? 255 : 0) - : (val: number) => (255 * (val - min)) / (max - min); + const normalize = (val: number): number => { + if (max === min) { + return val > 0 ? 255 : 0; + } + return Math.floor((255 * (val - min)) / (max - min)); + };
Line range hint
638-702: Consider optimizing fillHolesInPlace for large arrays.The current implementation using a queue might be inefficient for large arrays. Consider using a more efficient data structure or algorithm.
Some suggestions:
- Use a circular buffer instead of an array for the queue to avoid array shifts
- Consider using a scanline fill algorithm for better performance
- Pre-allocate the queue size based on the image dimensions
frontend/javascripts/oxalis/model/sagas/min_cut_saga.ts (2)
70-71: Consider memoizing the condition check.The condition
magIndex === 0 && magsWithIndices.length > 1 && ALWAYS_IGNORE_FIRST_MAG_INITIALLYcould be memoized since it's used in a loop and its value doesn't change during iteration.+ const shouldIgnoreFirstMag = magIndex === 0 && magsWithIndices.length > 1 && ALWAYS_IGNORE_FIRST_MAG_INITIALLY; - if (magIndex === 0 && magsWithIndices.length > 1 && ALWAYS_IGNORE_FIRST_MAG_INITIALLY) { + if (shouldIgnoreFirstMag) {
Line range hint
286-293: Enhance error message with more context.The error message could be more helpful by including the actual bounding box dimensions and the current magnification values.
- yield* call( - [Toast, Toast.warning], - "The bounding box for the selected seeds is too large. Choose a smaller bounding box or lower the distance between the seeds. Alternatively, ensure that lower magnifications exist which can be used.", - ); + yield* call( + [Toast, Toast.warning], + `The bounding box (${boundingBoxMag1.getSize().join('x')}) for the selected seeds is too large at magnification ${magInfo.toString()}. Choose a smaller bounding box or lower the distance between the seeds. Alternatively, ensure that lower magnifications exist which can be used.`, + );frontend/javascripts/oxalis/model/accessors/volumetracing_accessor.ts (2)
258-267: Consider documenting the magic number.The brush size calculation logic is correct, but the magic number
300should be documented to explain its significance as the maximum brush size for magnification level 1.-const MAX_BRUSH_SIZE_FOR_MAG1 = 300; +// Maximum brush size (in pixels) for magnification level 1 +// This value was chosen to balance between precision and coverage area +const MAX_BRUSH_SIZE_FOR_MAG1 = 300;
Line range hint
487-533: Consider extracting fallback magnification logic.The logic for finding a fallback magnification could be moved to a separate function to improve readability and maintainability.
+function findFallbackMag( + magInfo: MagInfo, + requestedZoomStep: number +): { zoomStep: number; mag: Vector3 } | null { + for ( + let fallbackZoomStep = requestedZoomStep + 1; + fallbackZoomStep <= requestedZoomStep + MAX_ZOOM_STEP_DIFF; + fallbackZoomStep++ + ) { + if (magInfo.hasIndex(fallbackZoomStep)) { + return { + zoomStep: fallbackZoomStep, + mag: magInfo.getMagByIndexOrThrow(fallbackZoomStep), + }; + } + } + return null; +} function _getRenderableMagForSegmentationTracing( state: OxalisState, segmentationTracing: VolumeTracing | null | undefined, ): { mag: Vector3; zoomStep: number; } | null | undefined { // ... existing code ... if (renderMissingDataBlack) { return null; } - for ( - let fallbackZoomStep = requestedZoomStep + 1; - fallbackZoomStep <= requestedZoomStep + MAX_ZOOM_STEP_DIFF; - fallbackZoomStep++ - ) { - if (magInfo.hasIndex(fallbackZoomStep)) { - return { - zoomStep: fallbackZoomStep, - mag: magInfo.getMagByIndexOrThrow(fallbackZoomStep), - }; - } - } + return findFallbackMag(magInfo, requestedZoomStep); }frontend/javascripts/router.tsx (1)
Line range hint
650-674: LGTM! Consider adding JSDoc comments.The renaming from
resolutionRestrictionstomagRestrictionsis consistent with the PR objectives. The implementation maintains proper type safety and error handling.Consider adding JSDoc comments to document the purpose and structure of
magRestrictions:+/** + * Restrictions for dataset magnification levels. + * @property {number} min - Minimum allowed magnification level + * @property {number} max - Maximum allowed magnification level (optional) + */ const magRestrictions: APIMagRestrictions = {};frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts (1)
Line range hint
425-442: Consider cleanup of deprecated methodsWhile the renaming from resolution to mag is consistent, this method is marked as deprecated. Consider:
- Adding a timeline for removal of deprecated methods
- Documenting the preferred alternative method in the deprecation notice
- Creating a tracking issue for the eventual removal
frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx (1)
Line range hint
88-90: Avoid suppressing TypeScript errors with@ts-expect-errorUsing
@ts-expect-errorsuppresses TypeScript errors without addressing the underlying issue. Consider properly typing theoptionparameter in thefilterOptionfunction to ensure type safety and resolve the error.frontend/javascripts/oxalis/model/sagas/mesh_saga.ts (1)
253-253: Simplify object property using shorthand syntaxIn the return statement, the property
magInfo: magInfocan be simplified using ES6 shorthand notation.Apply this diff to enhance readability:
return { zoomStep, - magInfo: magInfo, + magInfo, };frontend/javascripts/oxalis/model/sagas/proofread_saga.ts (1)
Line range hint
91-93: Update Global Variable Name from__proofreadCoarseResolutionIndexto__proofreadCoarseMagIndexWithin the
proofreadCoarseMagIndexfunction, the references towindow.__proofreadCoarseResolutionIndexshould be updated towindow.__proofreadCoarseMagIndexto reflect the renaming from "resolution" to "mag".Apply this diff to correct the variable names:
function proofreadCoarseMagIndex(): number { // @ts-ignore - return window.__proofreadCoarseResolutionIndex != null + return window.__proofreadCoarseMagIndex != null ? // @ts-ignore - window.__proofreadCoarseResolutionIndex + window.__proofreadCoarseMagIndex : 3; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (64)
CHANGELOG.unreleased.md(1 hunks)frontend/javascripts/admin/admin_rest_api.ts(2 hunks)frontend/javascripts/admin/tasktype/task_type_create_view.tsx(5 hunks)frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx(1 hunks)frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx(7 hunks)frontend/javascripts/oxalis/api/api_latest.ts(9 hunks)frontend/javascripts/oxalis/controller/combinations/move_handlers.ts(1 hunks)frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts(3 hunks)frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts(1 hunks)frontend/javascripts/oxalis/model/accessors/flycam_accessor.ts(16 hunks)frontend/javascripts/oxalis/model/accessors/tool_accessor.ts(1 hunks)frontend/javascripts/oxalis/model/accessors/volumetracing_accessor.ts(7 hunks)frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx(2 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts(2 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts(9 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/layer_rendering_manager.ts(3 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_arbitrary.ts(2 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_plane.ts(5 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/wkstore_adapter.ts(3 hunks)frontend/javascripts/oxalis/model/data_layer.ts(2 hunks)frontend/javascripts/oxalis/model/helpers/generate_dummy_trees.ts(1 hunks)frontend/javascripts/oxalis/model/helpers/mag_info.ts(4 hunks)frontend/javascripts/oxalis/model/helpers/nml_helpers.ts(2 hunks)frontend/javascripts/oxalis/model/helpers/position_converter.ts(6 hunks)frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts(2 hunks)frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts(3 hunks)frontend/javascripts/oxalis/model/sagas/clip_histogram_saga.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/dataset_saga.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/mesh_saga.ts(10 hunks)frontend/javascripts/oxalis/model/sagas/min_cut_saga.ts(4 hunks)frontend/javascripts/oxalis/model/sagas/prefetch_saga.ts(4 hunks)frontend/javascripts/oxalis/model/sagas/proofread_saga.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/quick_select_heuristic_saga.ts(6 hunks)frontend/javascripts/oxalis/model/sagas/quick_select_ml_saga.ts(8 hunks)frontend/javascripts/oxalis/model/sagas/save_saga.ts(1 hunks)frontend/javascripts/oxalis/model/sagas/volume/helpers.ts(8 hunks)frontend/javascripts/oxalis/model/sagas/volume/volume_interpolation_saga.ts(8 hunks)frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx(5 hunks)frontend/javascripts/oxalis/model/volumetracing/volume_annotation_sampling.ts(6 hunks)frontend/javascripts/oxalis/model_initialization.ts(2 hunks)frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx(3 hunks)frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx(1 hunks)frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx(1 hunks)frontend/javascripts/oxalis/view/context_menu.tsx(1 hunks)frontend/javascripts/oxalis/view/jobs/train_ai_model.tsx(1 hunks)frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx(7 hunks)frontend/javascripts/oxalis/view/left-border-tabs/modals/add_volume_layer_modal.tsx(4 hunks)frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx(5 hunks)frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx(3 hunks)frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segments_view.tsx(6 hunks)frontend/javascripts/oxalis/view/statusbar.tsx(2 hunks)frontend/javascripts/oxalis/workers/async_bucket_picker.worker.ts(4 hunks)frontend/javascripts/router.tsx(2 hunks)frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts(3 hunks)frontend/javascripts/test/fixtures/tasktracing_server_objects.ts(1 hunks)frontend/javascripts/test/geometries/skeleton.spec.ts(2 hunks)frontend/javascripts/test/model/binary/cube.spec.ts(1 hunks)frontend/javascripts/test/model/flycam_accessors.spec.ts(3 hunks)frontend/javascripts/test/model/model_resolutions.spec.ts(6 hunks)frontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts(12 hunks)frontend/javascripts/test/reducers/skeletontracing_reducer.spec.ts(38 hunks)frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts(5 hunks)frontend/javascripts/types/api_flow_types.ts(1 hunks)frontend/stylesheets/trace_view/_tracing_view.less(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- frontend/javascripts/admin/voxelytics/ai_model_list_view.tsx
- frontend/javascripts/types/api_flow_types.ts
🔇 Additional comments (177)
frontend/javascripts/oxalis/model/helpers/generate_dummy_trees.ts (1)
37-37: LGTM! Property rename aligns with project objectives.
The change from resolution to mag in the node properties is consistent with the PR's goal of standardizing terminology across the codebase.
Let's verify the type consistency and usage:
✅ Verification successful
Let me gather more information about the ServerNode type and any remaining usages of "resolution" in the codebase:
Property rename from resolution to mag is correctly implemented
The verification confirms:
- The
ServerNodetype inapi_flow_types.tsusesmag: numberas expected - The
generateDummyTreeshelper is used in test files and maintains consistency with the type definition - The remaining occurrences of "resolution" in the codebase are unrelated to this change as they refer to different concepts (layer resolutions, datasource properties)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the ServerSkeletonTracingTree type definition and its usage
# to ensure the property rename is consistent with the type definition.
# Test 1: Check the type definition
rg -A 10 "type ServerSkeletonTracingTree"
# Test 2: Check for any remaining uses of 'resolution' in related test files
rg -g "*.test.{ts,tsx}" "resolution.*ServerSkeletonTracingTree"
# Test 3: Check for any direct usage of this helper in test files
rg "generateDummyTrees"
Length of output: 2322
Script:
#!/bin/bash
# Check ServerNode type definition to verify mag property
rg -A 10 "type ServerNode"
# Check for any remaining uses of "resolution" in related files
rg "resolution" frontend/javascripts/oxalis/model/
rg "resolution" frontend/javascripts/types/
Length of output: 3587
frontend/javascripts/oxalis/model/data_layer.ts (3)
19-19: LGTM: Property renamed according to PR objectives.
The property renaming from resolutions to mags is consistent with the PR's goal of standardizing terminology.
38-38: LGTM: Error message uses consistent terminology.
The error message appropriately uses "Magnifications" which aligns with the new terminology.
35-35: Verify the intentional property name mismatch.
While this.mags is assigned from layerInfo.resolutions, this appears to be intentional as per PR objectives which mention not updating backend-related names. However, this creates a semantic disconnect between the property names.
✅ Verification successful
Based on the search results, I can now generate the final response:
Property name mismatch is intentional and consistent with the API types.
The assignment this.mags = layerInfo.resolutions is correct and intentional. The API type APIDataLayerBase defines the property as resolutions while internally the application uses mags for better semantic clarity. This is evidenced by:
- The API type definition in
api_flow_types.tsusesresolutions:
type APIDataLayerBase = {
readonly resolutions: Array<Vector3>;
// ...
};- The internal
DataLayerclass indata_layer.tsusesmags:
class DataLayer {
mags: Array<Vector3>;
// ...
}🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if layerInfo type definition still uses "resolutions"
# Expected: Find the type definition to confirm this is intentional
# Search for DataLayerType definition
rg -t typescript "type DataLayerType" -A 10
Length of output: 80
Script:
#!/bin/bash
# Search for type definitions related to layerInfo
rg -t ts "type.*Layer.*" -A 10
# Also search for interface definitions
rg -t ts "interface.*Layer.*" -A 10
# Search for usage of resolutions in ts files
rg -t ts "resolutions.*:" -A 2
Length of output: 109677
frontend/javascripts/oxalis/workers/async_bucket_picker.worker.ts (4)
43-43: LGTM: Parameter rename aligns with PR objectives.
The parameter rename from resolutions to mags is consistent with the PR's goal of standardizing terminology.
87-87: LGTM: Consistent parameter usage across code paths.
The parameter rename is consistently applied in all code paths using determineBucketsForOblique.
67-67: Verify determineBucketsForOblique signature update.
The parameter rename needs to match the function signature in the implementation.
✅ Verification successful
Parameter rename is correctly aligned with function signature
The verification shows that the determineBucketsForOblique function signature in frontend/javascripts/oxalis/model/bucket_data_handling/bucket_picker_strategies/oblique_bucket_picker.ts correctly uses mags as the parameter name, matching the usage in the worker. All calls to this function across the codebase consistently use the mags parameter.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that determineBucketsForOblique function signature has been updated
# to use 'mags' instead of 'resolutions'
# Search for the function definition
ast-grep --pattern 'function determineBucketsForOblique($_) {
$$$
}'
# Also search for any remaining usage of 'resolution' term
rg -i 'resolution' 'frontend/javascripts/oxalis/model/bucket_data_handling/bucket_picker_strategies/oblique_bucket_picker.ts'
Length of output: 196
Script:
#!/bin/bash
# Let's try a broader search to locate the function and its implementation
# Search for files containing 'determineBucketsForOblique'
rg -l "determineBucketsForOblique"
# Once we find the files, let's look at the actual implementation
rg "determineBucketsForOblique" -A 5
# Also search for any file with 'oblique' in its name to ensure we're not missing anything
fd oblique
Length of output: 4852
76-76: Verify determineBucketsForFlight signature update.
The parameter rename needs to match the function signature in the implementation.
✅ Verification successful
Parameter rename is correctly aligned across implementation and usage
The verification shows that both the function definition in flight_bucket_picker.ts and its usage in async_bucket_picker.worker.ts consistently use mags as the parameter name. The function signature and its invocation are properly aligned.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that determineBucketsForFlight function signature has been updated
# to use 'mags' instead of 'resolutions'
# Search for the function definition
ast-grep --pattern 'function determineBucketsForFlight($_) {
$$$
}'
# Also search for any remaining usage of 'resolution' term
rg -i 'resolution' 'frontend/javascripts/oxalis/model/bucket_data_handling/bucket_picker_strategies/flight_bucket_picker.ts'
Length of output: 194
Script:
#!/bin/bash
# Let's try a broader search to locate the file and function
fd -e ts -e js bucket_picker
# Then search for the function name without assuming its exact structure
rg -l "determineBucketsForFlight"
# And finally, let's look for any file containing both "flight" and "bucket" in its name
fd -e ts -e js . -E dist -E build -E node_modules | rg -i "flight.*bucket|bucket.*flight"
Length of output: 768
Script:
#!/bin/bash
# Let's examine the content of flight_bucket_picker.ts and async_bucket_picker.worker.ts
# to see the function definition and its usage
# Check flight_bucket_picker.ts content
rg -A 5 "determineBucketsForFlight" "frontend/javascripts/oxalis/model/bucket_data_handling/bucket_picker_strategies/flight_bucket_picker.ts"
# Check async_bucket_picker.worker.ts content
rg -A 5 "determineBucketsForFlight" "frontend/javascripts/oxalis/workers/async_bucket_picker.worker.ts"
Length of output: 892
frontend/javascripts/test/model/model_resolutions.spec.ts (2)
8-18: LGTM! Test case properly validates basic magnification conversion.
The test case effectively validates the conversion from sparse to dense magnification values.
63-72: LGTM! Edge case properly handled.
The test case effectively validates the behavior when no data layers are present.
frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_arbitrary.ts (2)
60-61: LGTM! Parameter renaming aligns with standardization effort.
The renaming from resolutions to mags and resolutionInfo to magInfo is consistent with the broader effort to standardize magnification terminology across the codebase.
65-65: LGTM! Verify consistent renaming across related files.
The usage of renamed parameters is correct. Let's verify that all related files have been updated consistently.
Also applies to: 85-85
✅ Verification successful
Parameter renaming is consistent across prefetch strategy files
The usage of magInfo.getIndexOrClosestHigherIndex() is consistent across both prefetch strategy files:
prefetch_strategy_arbitrary.ts:65: UsesmagInfo.getIndexOrClosestHigherIndex(activeZoomStep)prefetch_strategy_plane.ts:94: UsesmagInfo.getIndexOrClosestHigherIndex(currentZoomStep)
Both files use the same method signature and parameter naming convention. The mags parameter is also used consistently in both files for bucket position calculations. No issues were found with the parameter renaming.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of "resolution" in related files
# that should have been renamed to "mag"
# Search for resolution-related terms in TypeScript files
echo "Checking for potentially missed renamings:"
rg -t ts "resolutions?(?i)" --glob "frontend/javascripts/**/*" -C 2
# Specifically check related strategy files
echo -e "\nChecking related strategy files:"
fd "prefetch_strategy" frontend/javascripts/oxalis/model/bucket_data_handling/ -x rg -l "resolutions?"
Length of output: 45149
Script:
#!/bin/bash
# Let's check for any references to magInfo and getIndexOrClosestHigherIndex in prefetch strategy files
echo "Checking prefetch strategy files for magInfo and related method usage:"
ast-grep --pattern 'magInfo.getIndexOrClosestHigherIndex($_)' frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_*.ts
echo -e "\nChecking for other magInfo usages in prefetch strategy files:"
ast-grep --pattern 'magInfo.$_($_)' frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_*.ts
echo -e "\nChecking for any mag-related variable declarations:"
ast-grep --pattern 'const $_ = mag$_' frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_*.ts
Length of output: 1704
Script:
#!/bin/bash
# Let's check the implementation details of both prefetch strategy files
echo "Checking prefetch_strategy_arbitrary.ts implementation:"
cat frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_arbitrary.ts
echo -e "\nChecking prefetch_strategy_plane.ts implementation:"
cat frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_plane.ts
Length of output: 11857
frontend/javascripts/test/fixtures/tasktracing_server_objects.ts (2)
23-23: LGTM! Property rename aligns with standardization effort.
The change from resolution to mag is consistent with the PR objectives and maintains the correct value.
23-23: Verify complete migration of resolution terminology in test fixtures.
Let's ensure we haven't missed any other instances of "resolution" in the test fixtures that should be renamed to "mag".
frontend/javascripts/test/model/flycam_accessors.spec.ts (2)
141-141: LGTM! Test assertion maintains consistency with renamed variables.
The variable naming in the test assertion correctly reflects the terminology change while preserving the test's functionality.
Line range hint 83-115: LGTM! Variable renaming aligns with PR objectives.
The renaming from "resolutions" to "mags" is consistent with the PR's goal of standardizing terminology.
Let's verify the naming consistency across the codebase:
frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts (1)
25-25: LGTM! Consistent renaming of resolution to mag.
The changes align with the PR objective of standardizing terminology from "resolution" to "mag" across the codebase. The modification is consistently applied to all node objects in the test fixture.
Let's verify that no instances of "resolution" remain in this file:
Also applies to: 73-73, 93-93
✅ Verification successful
Verified: All "resolution" instances correctly renamed to "mag"
After reviewing the complete file content, I can confirm that:
- All node objects consistently use
mag: 1 - There are no remaining instances of "resolution"
- The changes are properly applied across all nodes in the test fixture
- The value of 1 is consistent with the mock data structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of "resolution" in the file
# Expect: No matches as all instances should have been renamed to "mag"
rg -i "resolution" "frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts"
Length of output: 87
Script:
#!/bin/bash
# Let's first check if the file exists and see its content
cat frontend/javascripts/test/fixtures/skeletontracing_server_objects.ts
# If the file is not found, let's search for it in case the path is incorrect
fd skeletontracing_server_objects.ts
Length of output: 4555
frontend/javascripts/oxalis/controller/combinations/move_handlers.ts (2)
52-54: LGTM! Variable renaming is consistent with PR objectives.
The change from representativeResolution to representativeMag maintains the same functionality while improving terminology consistency.
52-54: Verify consistent renaming across related files.
Let's ensure the resolution → mag renaming is consistent across related files.
✅ Verification successful
Based on the search results, I can now generate the final response:
The resolution → mag renaming appears to be consistent and intentional
The codebase shows a clear pattern where:
- "resolution" is primarily used in shader code (*.glsl.ts files) and internal data structures
- "mag" terminology is used in the application logic and API layers
- Both terms coexist intentionally, serving different semantic purposes
The usage of representativeMag and getActiveMagInfo in the reviewed code aligns with this pattern, as these are part of the application logic layer.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining "resolution" references that might need updating
# and verify consistency of "mag" usage in related files.
echo "Checking for remaining resolution-related terms that might need updating..."
rg -i "resolution" --type ts frontend/javascripts/oxalis/
echo "Verifying consistent usage of new mag terminology..."
rg -i "representativeMag|getActiveMagInfo" --type ts frontend/javascripts/oxalis/
Length of output: 13319
frontend/javascripts/oxalis/model/sagas/clip_histogram_saga.ts (2)
39-40: LGTM! Parameter renaming is consistent.
The parameter renaming in the getViewportData call maintains consistency with the earlier variable renaming.
45-52: LGTM! Verify consistent parameter naming across related files.
The parameter renaming is consistent across all viewport data calls. Let's verify that similar getViewportData calls in related files have been updated.
✅ Verification successful
Parameter naming is consistent across the codebase
The search results show that all instances of getViewportData use consistent parameter naming. The method is defined in api_latest.ts with parameters viewport, layerName, maybeMagIndex, and all calls in clip_histogram_saga.ts follow this pattern with matching parameter names.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of "resolution" in getViewportData calls
rg 'getViewportData.*resolution' --type ts
Length of output: 44
Script:
#!/bin/bash
# Search for all getViewportData calls to verify parameter naming
ast-grep --pattern 'getViewportData($$$)'
Length of output: 299
Script:
#!/bin/bash
# Try a different approach to find getViewportData calls
rg "getViewportData\(" -A 3 --type typescript
Length of output: 82
Script:
#!/bin/bash
# Try with .ts extension explicitly
rg "getViewportData\(" -A 3 -g "*.ts"
Length of output: 1422
frontend/javascripts/oxalis/model/helpers/position_converter.ts (6)
12-16: LGTM! Variable renaming is consistent with PR objectives.
The renaming from resolution to mag maintains the function's logic while improving terminology consistency.
29-30: LGTM! Parameter renaming maintains function clarity.
The parameter renaming from currentResolution to currentMag is consistent with the PR objectives while preserving the mathematical correctness.
43-47: LGTM! Variable renaming preserves calculation accuracy.
The renaming from resolution to mag maintains the floating-point calculation logic while improving terminology consistency.
51-55: LGTM! Function and variable renaming maintains upsampling logic.
The renaming of the function and its variables from resolution to mag terminology is consistent while preserving the correct upsampling calculation.
59-66: LGTM! Type-safe parameter updates with consistent renaming.
The changes maintain type safety while standardizing the magnification terminology.
117-119: LGTM! Consistent terminology in code and comments.
The variable renaming and comment updates maintain consistency with the new magnification terminology while preserving the underlying logic.
Also applies to: 148-156
frontend/javascripts/oxalis/model/sagas/dataset_saga.ts (1)
114-116: LGTM on the variable renaming, but found an inconsistency.
The renaming of resolutionInfo to magInfo aligns well with the PR objectives to standardize terminology. However, dataLayer.resolutions is still using the old terminology.
Let's verify if this property needs to be updated as well:
frontend/javascripts/oxalis/model/bucket_data_handling/prefetch_strategy_plane.ts (5)
90-91: LGTM! Parameter renaming is consistent with PR objectives.
The renaming from 'resolutions' to 'mags' and 'resolutionInfo' to 'magInfo' aligns with the PR's goal of standardizing terminology.
94-94: LGTM! Implementation updates correctly use the renamed parameters.
The method implementation properly adapts to use the new magInfo parameter while maintaining the same functionality.
Also applies to: 102-102
112-112: LGTM! Parameter passing is consistently updated.
Both calls to prefetchImpl correctly use the renamed 'mags' parameter, maintaining consistency throughout the method.
Also applies to: 128-128
145-145: LGTM! Method signature and implementation are properly updated.
The prefetchImpl method consistently uses the renamed 'mags' parameter throughout its implementation, including the call to zoomedAddressToAnotherZoomStep.
Also applies to: 172-172
Line range hint 90-172: Verify complete migration of resolution terminology.
Let's ensure no references to the old terminology remain in related files.
frontend/javascripts/oxalis/model/helpers/mag_info.ts (5)
39-40: LGTM! Variable renaming is consistent with PR objectives.
The renaming from 'resolution' to 'mag' in the loop maintains semantic clarity and aligns with the method's purpose.
52-55: LGTM! Variable renaming improves consistency.
The renaming from 'resolution' to 'mag' and 'resolutionIndex' to 'magIndex' enhances code clarity while preserving the correct mathematical logic.
79-85: LGTM! Consistent renaming in variable and error message.
The changes maintain code clarity and ensure consistent terminology in both the variable name and error message.
93-97: LGTM! Consistent variable renaming with preserved logic.
The renaming from 'resolutionMaybe' to 'magMaybe' improves terminology consistency while maintaining the correct error handling and fallback logic.
Also applies to: 103-112
239-254: LGTM! Consistent renaming while preserving mathematical terminology.
The variable renaming from 'maxResolution' to 'maxMag' and 'resolutionsLookUp' to 'magnificationsLookUp' improves consistency. Mathematical terms like 'resPower' are appropriately preserved as they represent power/exponent concepts rather than resolution terminology.
frontend/javascripts/oxalis/model/sagas/prefetch_saga.ts (4)
106-106: LGTM! Variable renaming is consistent.
The renaming from resolution to mag is applied consistently, maintaining the original functionality while improving terminology clarity.
Also applies to: 113-113
139-140: LGTM! Parameter renaming is consistent.
The prefetch strategy parameters have been correctly renamed while maintaining the original parameter order.
174-175: LGTM! Consistent renaming in arbitrary mode.
The resolution to mag renaming is consistently applied in the arbitrary mode, maintaining parity with the plane mode changes.
200-201: LGTM! Verify consistent renaming across related files.
The parameter renaming is consistent with the changes made in plane mode.
Let's verify that the renaming is consistent across related files:
✅ Verification successful
Based on the search results, I can see that the renaming from "resolution" to "mag" has been consistently applied across the codebase. The search shows that "mag" is now used consistently in parameter names, variable names, and comments throughout the model layer. The term "resolution" only appears in a few places where it's part of descriptive comments or documentation, but not in actual code identifiers.
Parameter renaming is consistent across the codebase
The changes in prefetch_saga.ts align with the broader codebase convention where:
- Parameters use
maginstead ofresolution - Variable names and function parameters consistently use
magterminology - Type definitions and interfaces follow the same naming pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining "resolution" references in related files
# that should have been renamed to "mag"
# Search for resolution-related terms in the frontend code
rg -i 'resolution(?!.*deprecated)' frontend/javascripts/oxalis/model/
Length of output: 311
Script:
#!/bin/bash
# Let's try a different approach to search for resolution/mag related terms
# First, let's check the usage of 'mag' in the codebase
rg -i '\bmag[s]?\b' frontend/javascripts/oxalis/model/ -A 2 -B 2
# Also search for resolution with proper regex
rg -i '\bresolution[s]?\b' frontend/javascripts/oxalis/model/ -A 2 -B 2
Length of output: 123988
frontend/javascripts/test/geometries/skeleton.spec.ts (2)
38-38: LGTM! Variable renamed from resolution to mag.
The renaming aligns with the PR objectives and maintains consistency with the broader codebase changes.
50-50: LGTM! Updated createNodeAction parameter.
The mag parameter is correctly passed to createNodeAction, maintaining consistency with the variable renaming.
frontend/javascripts/test/model/binary/cube.spec.ts (1)
72-72: LGTM! Variable renaming is consistent with PR objectives.
The renaming from resolutionInfo to magInfo aligns with the PR's goal of replacing "resolution" with "mag" in local variables.
Let's verify the consistency of this renaming pattern across other test files:
Also applies to: 76-76
✅ Verification successful
Let me check the actual usage in the other test file to ensure consistency.
Variable renaming is consistently applied across test files
The verification shows that both test files (cube.spec.ts and volume_annotation_sampling.spec.ts) follow the same pattern:
- Both use
magInfoas the variable name - Both instantiate it with
mockedLayer.resolutions - Both use it in similar test setup contexts with DataCube initialization
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of "resolutionInfo" in test files
# and verify the consistent usage of "magInfo"
echo "Checking for any remaining instances of 'resolutionInfo'..."
rg -l "resolutionInfo" "frontend/javascripts/test"
echo "Verifying consistent usage of 'magInfo' with MagInfo class..."
rg -l "magInfo.*=.*new MagInfo" "frontend/javascripts/test"
Length of output: 515
Script:
#!/bin/bash
# Show the context of magInfo usage in both files
echo "Context in volume_annotation_sampling.spec.ts:"
rg "magInfo.*=.*new MagInfo" -B 2 -A 2 "frontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts"
echo -e "\nContext in cube.spec.ts:"
rg "magInfo.*=.*new MagInfo" -B 2 -A 2 "frontend/javascripts/test/model/binary/cube.spec.ts"
Length of output: 731
frontend/javascripts/oxalis/view/left-border-tabs/modals/add_volume_layer_modal.tsx (2)
233-236: LGTM! Props renamed consistently.
The prop names have been updated to use "mag" terminology consistently, following React naming conventions.
132-136: Verify API contract for future backend updates.
While the local variable has been renamed to magInfo, note that it's still accessing selectedSegmentationLayer.resolutions from the API. This intentional inconsistency aligns with the PR objectives, as backend changes were not in scope. However, it would be good to document this for future backend updates.
Let's verify other API usages:
✅ Verification successful
Based on the comprehensive search results, I can now generate the final response:
The API contract is correctly maintained during the resolution-to-mag renaming.
The codebase shows a consistent pattern where:
- Frontend local variables use "mag" terminology
- API interfaces (in
api_flow_types.ts) maintain "resolutions" field getMagInfofunction serves as the adapter between these two conventions, being consistently used across the codebase to handle the translation
The implementation is sound and follows good practices:
- The function is memoized to optimize performance
- It's used consistently across all components that need to handle magnification data
- The API contract remains stable while allowing internal terminology updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other resolution-related API fields that might need future updates
rg -g '*.ts*' -g '*.js*' 'resolutions.*=' frontend/
Length of output: 372
Script:
#!/bin/bash
# Let's check the full context of these files to understand the usage pattern
echo "=== quick_select_heuristic_saga.ts ==="
rg -B 5 -A 5 'resolutions.*=' frontend/javascripts/oxalis/model/sagas/quick_select_heuristic_saga.ts
echo -e "\n=== model_resolutions.spec.ts ==="
rg -B 5 -A 5 'resolutions.*=' frontend/javascripts/test/model/model_resolutions.spec.ts
# Let's also check for any type definitions that might be relevant
echo -e "\n=== Type definitions for resolutions ==="
rg -g '*.ts*' 'interface.*{.*resolutions' frontend/
Length of output: 1517
Script:
#!/bin/bash
# Let's try a different approach to find interface definitions
echo "=== Searching for interfaces with resolutions field ==="
ast-grep --pattern 'interface $_ {
$$$
resolutions: $_
$$$
}'
# Let's also check the imports and usage of getMagInfo
echo -e "\n=== Searching for getMagInfo usage ==="
rg -g '*.ts*' -g '*.js*' 'getMagInfo\(' frontend/
# And check the definition of getMagInfo
echo -e "\n=== Searching for getMagInfo definition ==="
ast-grep --pattern 'function getMagInfo($_) {
$$$
}'
Length of output: 5394
Script:
#!/bin/bash
# Let's find the actual definition of getMagInfo since it might be an exported const
echo "=== Searching for getMagInfo definition ==="
ast-grep --pattern 'export const getMagInfo = $_'
# Let's also check the type definition of the layer interface
echo -e "\n=== Searching for layer type definitions ==="
rg -g '*.ts*' 'type.*Layer.*=' frontend/
# And check the actual implementation of getMagInfo in dataset_accessor
echo -e "\n=== getMagInfo implementation in dataset_accessor ==="
rg -B 10 -A 10 '_getMagInfo' frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts
Length of output: 86910
frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx (3)
93-99: LGTM: Variable renaming is consistent.
The renaming from resolution to mag is consistent with the PR objectives. Note that selectedSegmentationLayer.resolutions remains unchanged as it's a backend-related property.
Line range hint 169-212: LGTM: State and prop renaming is consistent.
The renaming changes in the CreateExplorativeModal function are consistent and maintain the same functionality while improving clarity with the new terminology.
247-251: LGTM: URL parameter construction is consistent.
The URL parameter construction correctly uses the new mag terminology and maintains the same functionality with proper usage of getMagByIndexOrThrow.
frontend/javascripts/oxalis/model/bucket_data_handling/wkstore_adapter.ts (3)
63-63: LGTM: Parameter renaming is consistent.
The renaming from resolutionInfo to magInfo aligns with the PR objectives and maintains type safety.
Also applies to: 68-68
82-82: LGTM: Function signature and implementation updated consistently.
The renaming is applied consistently throughout the function, including method calls and parameter usage.
Also applies to: 84-84, 86-86
198-198: Verify the continued usage of layerInfo.resolutions.
While the local variable has been renamed to magInfo, the property resolutions is still used in layerInfo. This appears to be intentional as per PR objectives which mention that backend-related changes (like APIDataLayerBase.resolutions) were not included in this PR.
Let's verify other usages of this property:
Also applies to: 206-206
✅ Verification successful
Based on the comprehensive search results, I can now generate the final response:
The usage of layerInfo.resolutions is correct and consistent with the codebase architecture.
The property resolutions is a core part of the APIDataLayerBase interface which is used throughout the codebase. The search results show that:
- It's a required property in the base interface:
type APIDataLayerBase = { ... resolutions: Array<Vector3>; ... } - The property is consistently used across the codebase with the
getMagInfoutility - The
getMagInfofunction is a central, memoized utility that's used extensively for handling magnification-related operations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of the resolutions property
rg "layerInfo\.resolutions" --type ts
Length of output: 253
Script:
#!/bin/bash
# Let's check the context of both usages and the type definition of layerInfo
rg "layerInfo\.resolutions" -B 5 -A 5 --type ts
# Also check the type definition
ast-grep --pattern 'interface $_ {
$$$
resolutions: $_;
$$$
}'
Length of output: 2236
Script:
#!/bin/bash
# Let's check the implementation of getMagInfo to understand how resolutions are used
ast-grep --pattern 'function getMagInfo($_) {
$$$
}'
# Also check the APIDataLayerBase interface
ast-grep --pattern 'interface APIDataLayerBase {
$$$
}'
Length of output: 116
Script:
#!/bin/bash
# Let's try a different approach to find getMagInfo and its usage
rg "getMagInfo" -B 5 -A 5 --type ts
# Also search for APIDataLayerBase
rg "APIDataLayerBase" -B 5 -A 5 --type ts
Length of output: 89672
frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (4)
109-109: LGTM: Variable renaming aligns with PR objectives.
The renaming from layersFinestResolution to layersFinestMag maintains semantic meaning while improving terminology consistency.
167-167: LGTM: Consistent use of "mag" terminology.
The variable renaming aligns with the existing function name getBoundingBoxInMag1, maintaining terminology consistency.
144-144: Verify API function parameter updates.
The parameter renaming from resolution to mag in API calls needs verification to ensure the functions have been updated to accept the new parameter name.
Also applies to: 151-151
✅ Verification successful
Parameter renaming is consistent with API implementation
Looking at the API implementation in frontend/javascripts/admin/admin_rest_api.ts, both getSegmentVolumes and getSegmentBoundingBoxes functions are already using the parameter name mag in their signatures, which matches the renamed parameter usage in the calls. No issues found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the API functions have been updated to use the new parameter name.
# Search for function definitions of getSegmentVolumes and getSegmentBoundingBoxes
ast-grep --pattern 'function getSegmentVolumes($_) {
$$$
}'
ast-grep --pattern 'function getSegmentBoundingBoxes($_) {
$$$
}'
Length of output: 133
Script:
#!/bin/bash
# Let's try a different approach to find these API functions
# First, let's search for files containing these function names
rg -l "getSegmentVolumes|getSegmentBoundingBoxes"
# Then let's look at the actual function definitions and their usage
rg "getSegmentVolumes|getSegmentBoundingBoxes" -A 5 -B 5
Length of output: 7603
171-171: Consider updating voxelToVolumeInUnit function.
While the variable renaming is correct, consider updating the parameter name in the voxelToVolumeInUnit function definition to maintain consistency across the codebase.
frontend/javascripts/oxalis/model/bucket_data_handling/layer_rendering_manager.ts (4)
193-193: LGTM! Variable rename maintains semantic meaning.
The renaming from maximumResolutionIndex to maximumMagIndex is consistent with the PR objective and preserves the logic of the code.
199-199: LGTM! Consistent with existing mag-related method names.
The renaming from resolutions to mags aligns well with the existing method name getDenseMags(), showing consistency in the new terminology.
240-240: Verify worker interface changes for asyncBucketPick.
While the renaming is consistent, we should verify that the asyncBucketPick worker has been updated to handle the renamed parameter.
Also applies to: 265-265
✅ Verification successful
Parameter rename is correctly implemented in the worker
The async_bucket_picker.worker.ts implementation already uses mags as the parameter name in the pick function signature. The changes are consistent across both the manager and worker interface.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the worker interface has been updated
# Look for the worker implementation
rg -l "AsyncBucketPickerWorker" | xargs rg "resolutions"
# Check for any remaining resolution-related parameter names in worker files
fd -e worker.ts | xargs rg "resolutions"
Length of output: 339
Script:
#!/bin/bash
# Search for AsyncBucketPicker worker files and implementation
fd -t f "bucket.*worker"
# Once found, check their content
fd -t f "bucket.*worker" -x cat {}
# Also search for any worker-related types or interfaces
rg -g '*.ts' -g '*.tsx' 'interface.*Worker|type.*Worker'
Length of output: 3121
190-191: Verify the consistency of resolution/mag naming.
While renaming resolutionInfo to magInfo aligns with the PR objective, the underlying data structure is still named layer.resolutions. This inconsistency should be tracked.
frontend/javascripts/oxalis/model/sagas/volume/helpers.ts (6)
97-99: LGTM: Consistent renaming of resolution to mag.
The parameter renaming from targetResolution to targetMags aligns with the PR objective while preserving the original functionality.
108-118: LGTM: Clear and consistent terminology update.
The variable renaming and comment updates effectively transition from "resolution" to "mag" terminology while maintaining code clarity.
131-143: LGTM: Comprehensive renaming in sampling sequence processing.
The variable and parameter renaming is thorough and consistent, maintaining the function's logic while improving terminology.
159-164: LGTM: Consistent terminology in sequence processing calls.
The updates to processSamplingSequence calls maintain consistency with the new terminology.
188-189: LGTM: Consistent terminology updates in buffer and layer handling.
The changes in labelWithVoxelBuffer2D and createVolumeLayer functions maintain consistency with the new "mag" terminology while preserving the original functionality.
Also applies to: 265-265, 270-270, 287-287, 292-292
Line range hint 1-292: Verify complete resolution to mag renaming.
While the changes in this file are consistent, let's verify that all relevant occurrences of "resolution" have been updated across the codebase.
frontend/javascripts/oxalis/model/sagas/quick_select_ml_saga.ts (6)
178-179: LGTM: Consistent variable renaming
The renaming from labeledResolution to labeledMag aligns with the PR objectives and improves clarity by using more precise terminology.
199-199: LGTM: Consistent variable usage in vector scaling
The use of labeledMag in the vector scaling operation maintains consistency with the renaming changes while preserving the original functionality.
212-212: LGTM: Consistent variable usage in alignment operation
The use of labeledMag in the alignWithMag method call maintains consistency with the renaming changes.
227-227: LGTM: Consistent parameter passing
The use of labeledMag as a parameter in the getMask function call maintains consistency with the renaming changes.
244-244: LGTM: Consistent variable usage in magnification conversion
The use of labeledMag in the fromMag1ToMag method call maintains consistency with the renaming changes.
259-259: LGTM: Consistent variable usage throughout loop operations
The use of labeledMag is consistent across multiple operations within the loop:
- Array indexing calculation
- Vector operations
- Magnification conversion
Also applies to: 276-276, 285-286
frontend/stylesheets/trace_view/_tracing_view.less (1)
581-584: LGTM! The styling for the magnification status bar icon looks good.
The class name change from resolution-status-bar-icon to mag-status-bar-icon aligns with the PR's objective of standardizing terminology. The styling properties are appropriate for the icon's intended use.
Let's verify that the corresponding template files have been updated to use the new class name:
✅ Verification successful
Styling changes are properly synchronized with template usage
The verification confirms that:
- The old class name
resolution-status-bar-iconis not used anywhere in the codebase - The new class name
mag-status-bar-iconis correctly used in both:- The stylesheet:
frontend/stylesheets/trace_view/_tracing_view.less - The template:
frontend/javascripts/oxalis/view/statusbar.tsx
- The stylesheet:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining uses of the old class name
rg 'resolution-status-bar-icon'
# Search for uses of the new class name to ensure it's being used
rg 'mag-status-bar-icon'
Length of output: 217
frontend/javascripts/oxalis/model/volumetracing/volume_annotation_sampling.ts (5)
21-33: LGTM! Variable and comment updates are consistent.
The renaming from "resolution" to "mag" is applied consistently and maintains the clarity of the code.
112-131: LGTM! Consistent variable renaming in upsampling logic.
The renaming changes are applied consistently throughout the upsampling kernel logic while preserving the original functionality.
143-143: LGTM! Consistent use of renamed variable in map operations.
The variable renaming is consistently applied in the map operations and return statement.
Also applies to: 149-149
169-169: LGTM! Consistent variable renaming in downsampling logic.
The renaming changes are applied consistently in the downsampling function while preserving the original functionality.
Also applies to: 216-218
264-264: LGTM! Consistent use of renamed variable in final operations.
The variable renaming is consistently applied in the final map operations and return statement.
Also applies to: 267-267
frontend/javascripts/oxalis/model/accessors/tool_accessor.ts (1)
277-278: LGTM! Variable renaming aligns with PR objectives.
The renaming from labeledResolution to labeledMag is consistent with the PR's goal of standardizing terminology around magnification.
Let's verify the consistency of this renaming across the codebase:
✅ Verification successful
Let me analyze the search results. I notice that most occurrences of "resolution" are in the form of layer.resolutions which appears to be a property name from an API or data model, rather than local variable names that were targeted for renaming in this PR.
Let's verify if there are any local variable names containing "resolution" that should have been renamed to "mag":
Based on the search results, I can now generate the final response:
✓ Variable renaming is consistent with the codebase
The change from labeledResolution to labeledMag in tool_accessor.ts is correct and consistent. The search results show that remaining occurrences of "resolution" are primarily in the form of the API property resolutions which is part of the data model and not meant to be renamed. No local variables or parameters containing "resolution" that should have been renamed to "mag" were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of "resolution" in variable names
# or function parameters that should have been renamed to "mag"
# Search for variable declarations or parameters containing "resolution"
rg -g '*.{ts,tsx}' '(?:const|let|var|function.*?)\s+\w*resolution\w*\s*[=:]'
# Search for object property access using "resolution"
rg -g '*.{ts,tsx}' '\.\w*resolution\w*\b'
Length of output: 5813
Script:
#!/bin/bash
# Search for local variable declarations containing "resolution"
# excluding property access patterns like ".resolutions"
ast-grep --pattern 'const resolution$_'
ast-grep --pattern 'let resolution$_'
ast-grep --pattern 'var resolution$_'
# Also check function parameters
ast-grep --pattern 'function $_($$$resolution$_$$$)'
Length of output: 13306
frontend/javascripts/admin/tasktype/task_type_create_view.tsx (3)
39-39: LGTM: Type rename aligns with standardization effort
The rename from isResolutionRestricted to isMagRestricted in the FormValues type is consistent with the PR's objective to standardize terminology.
Line range hint 106-113: LGTM: Default values properly structured
The default values for isMagRestricted and the nested magRestrictions object are well-structured and provide sensible defaults for magnification ranges.
Line range hint 414-435: LGTM: Form field updates are comprehensive and consistent
The form field changes are well-implemented:
- Field name updated to reflect new terminology
- Tooltip text properly references "magnifications"
- Validation logic and UI structure remain intact
Also applies to: 439-439
frontend/javascripts/oxalis/view/statusbar.tsx (3)
503-503: LGTM: Variable renaming aligns with PR objectives.
The renaming from "resolution" to "mag" is consistent with the standardization effort and maintains the component's functionality.
530-543: LGTM: UI elements consistently updated with new terminology.
The changes maintain visual consistency by:
- Updating the CSS class name to use "mag" terminology
- Correctly using the renamed variables in the UI rendering
Line range hint 503-545: Verify terminology consistency in messages and documentation.
While the code changes look good, it would be beneficial to ensure that all related documentation and message keys consistently use "magnification" instead of "resolution".
✅ Verification successful
Based on the search results, I can generate the final response now:
Terminology changes are well-documented and consistent.
The codebase shows a clear and intentional transition from "resolution" to "magnification"/"mag" terminology:
- The change is documented in CHANGELOG.unreleased.md with explicit references to terminology updates
- The terminology document (docs/terminology.md) correctly defines magnification terms
- Remaining "resolution" occurrences are primarily in:
- Historical changelog entries
- Technical contexts (e.g., moduleResolution in tsconfig.json)
- Documentation sections explaining the transition or providing context about different formats
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining "resolution" terms in message files and documentation
echo "Checking message files and documentation for terminology consistency..."
rg -i "resolution" --type=json --type=md
Length of output: 9748
frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx (1)
187-187: LGTM! Parameter renaming aligns with standardization effort.
The renaming from resolution to mag in createNodeAction is consistent with the PR objectives to standardize terminology across the codebase.
Let's verify that all dispatches of this action have been updated:
Also applies to: 198-198
✅ Verification successful
Parameter renaming from resolution to mag is consistent and all usages have been updated
The verification shows that all calls to createNodeAction are using the mag parameter name consistently. The search results confirm:
- The action creator definition in
skeletontracing_actions.tsxhas been updated to usemag - All usages of
createNodeActionacross test files and actual implementation are using themagparameter name - No instances of
createNodeActionwithresolutionparameter were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for createNodeAction calls to ensure they use the new parameter name
rg "createNodeAction\(" --type ts --type tsx -A 5 -B 5
# Search for any remaining uses of "resolution" in nearby code
rg "resolution(?!.*_DEPRECATED)" --type ts --type tsx -g "frontend/javascripts/oxalis/**/*"
Length of output: 419
Script:
#!/bin/bash
# Search for createNodeAction calls to ensure they use the new parameter name
rg "createNodeAction" -l
# Search for any remaining uses of "resolution" in relevant files
rg "resolution" --type-add 'ts:*.{ts,tsx}' --type ts
Length of output: 10505
Script:
#!/bin/bash
# Let's search for the actual usage of createNodeAction with context
rg "createNodeAction" -B 3 -A 3 --type-add 'ts:*.{ts,tsx}' --type ts
# Also search for the specific parameter name in the action creator calls
rg "createNodeAction\(.*resolution.*\)" --type-add 'ts:*.{ts,tsx}' --type ts --pcre2
rg "createNodeAction\(.*mag.*\)" --type-add 'ts:*.{ts,tsx}' --type ts --pcre2
Length of output: 98242
frontend/javascripts/oxalis/model/sagas/volume/volume_interpolation_saga.ts (7)
73-73: LGTM! Consistent renaming of resolution-related variables.
The renaming from resolution to mag is consistent with the PR objectives and maintains the existing logic.
Also applies to: 87-89
94-94: LGTM! Consistent parameter renaming.
The parameter renaming maintains consistency with the new terminology.
104-109: LGTM! Improved documentation and consistent renaming.
The added comment provides valuable context about coordinate clipping in different magnifications, making the code more maintainable. The variable renaming is consistent with the PR objectives.
145-145: LGTM! Consistent property renaming in return object.
The property renaming maintains consistency with the new terminology.
285-289: LGTM! Consistent variable renaming in zoom level check.
The variable renaming from isResolutionTooLow to isMagTooLow maintains consistency with the new terminology while preserving the existing logic.
336-340: LGTM! Consistent renaming in box calculations.
The variable renaming in box padding and alignment operations maintains consistency while preserving the mathematical logic.
375-376: LGTM! Consistent renaming in interpolation layer creation.
The parameter and calculation updates maintain consistency with the new terminology while preserving the existing logic.
frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx (4)
Line range hint 531-558: LGTM! Consistent renaming of resolution to mag.
The renaming changes from renderResolutionsTooltip to renderMagsTooltip and resolutionUnion to magUnion are consistent with the PR objectives while maintaining the existing functionality.
Line range hint 560-589: LGTM! Consistent renaming of resolution-related variables.
The renaming changes from getResolutionInfo to getMagInfo and the corresponding variable renames (representativeResolution → representativeMag, isActiveResolutionGlobal → isActiveMagGlobal) are consistent with the PR objectives while maintaining the existing functionality.
613-613: LGTM! Method call updated consistently.
The method call is correctly updated to use the renamed getMagInfo method.
Line range hint 531-613: Verify complete renaming of resolution-related terms.
Let's verify that all resolution-related terms have been renamed to mag in this file.
✅ Verification successful
All resolution-related terms have been successfully renamed to mag
The verification shows that there are no remaining instances of "resolution" or its variations in the file. The changes are complete and consistent with the renaming objectives, as all resolution-related terms have been properly renamed to use "mag" terminology.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining resolution-related terms that should have been renamed.
# Note: Excluding deprecated terms as mentioned in PR objectives.
# Test: Search for resolution-related terms, excluding _DEPRECATED suffix
rg -i 'resolution(?!.*_DEPRECATED)' 'frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx'
# The command should not return any results, indicating all terms have been renamed
Length of output: 355
Script:
#!/bin/bash
# Let's try a simpler approach to find any remaining resolution-related terms
rg -i "resolution" "frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx"
# Also search for common variations that might have been missed
rg -i "resolutions?" "frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx"
Length of output: 195
frontend/javascripts/oxalis/view/jobs/train_ai_model.tsx (2)
514-515: LGTM: Consistent with the resolution to mag rename.
The change from resolutions to mags in the volumeServerTracings mapping aligns with the PR objective of renaming resolution-related variables.
#!/bin/bash
# Let's try a different approach to find the type definition
rg -A 10 "interface.*SegmentationLayer"
# Also search for any usage of resolutions or mags with SegmentationLayer
rg "SegmentationLayer.*\.(resolutions|mags)"
# Search for the specific function
ast-grep --pattern 'const getMagForSegmentationLayer = async $_'frontend/javascripts/test/model/volumetracing/volume_annotation_sampling.spec.ts (3)
38-39: LGTM! Function rename is consistent with PR objectives.
The renaming from sampleVoxelMapToResolution to sampleVoxelMapToMag aligns with the PR's goal of standardizing terminology.
59-59: LGTM! Variable rename maintains consistency.
The change from resolutionInfo to magInfo maintains consistency with the new terminology while preserving the functionality.
Also applies to: 63-63
Line range hint 128-134: LGTM! Test cases updated consistently.
All test cases have been updated to use sampleVoxelMapToMag consistently while maintaining the original test coverage and assertions.
Also applies to: 190-196, 252-258, 314-320, 376-382, 426-432, 507-513, 592-598
frontend/javascripts/oxalis/model/sagas/save_saga.ts (3)
294-294: LGTM: Variable rename aligns with magnification terminology.
The variable rename from segmentationResolutionInfo to segmentationMagInfo maintains consistency with the new magnification terminology.
304-305: LGTM: Method rename maintains consistency.
The method rename from getDenseResolutions to getDenseMags aligns with the new magnification terminology.
Let's verify that this method has been consistently renamed across the codebase:
#!/bin/bash
# Search for any remaining usage of old method name
rg "getDenseResolutions" --type ts301-301: LGTM: Method rename reflects magnification terminology.
The method rename from getIndexByResolution to getIndexByMag maintains consistency with the new magnification terminology.
Let's verify that this method has been consistently renamed across the codebase:
✅ Verification successful
Method rename is consistent across the codebase
Based on the verification results:
- The new method
getIndexByMagis properly implemented inMagInfoclass - All usages across the codebase (
save_saga.ts,proofread_saga.ts,dataset_saga.ts) use the new method name - No occurrences of the old method name
getIndexByResolutionwere found - The implementation in
mag_info.tsshows the method is properly integrated with the class's functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining usage of old method name
rg "getIndexByResolution" --type ts
Length of output: 34
Script:
#!/bin/bash
# Let's search for both old and new method names with context
rg "getIndexBy(Resolution|Mag)" --type ts -A 2
Length of output: 1312
Script:
#!/bin/bash
# Let's also check the class/interface definition
ast-grep --pattern 'interface $_ {
$$$
getIndexByMag($_): $_
$$$
}'
Length of output: 75
Script:
#!/bin/bash
# And check for the old method name in the interface
ast-grep --pattern 'interface $_ {
$$$
getIndexByResolution($_): $_
$$$
}'
Length of output: 82
Script:
#!/bin/bash
# Let's check the class implementation in mag_info.ts
cat frontend/javascripts/oxalis/model/helpers/mag_info.ts
Length of output: 8028
frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx (3)
310-310: LGTM! Variable rename improves clarity.
The rename from selectedLayerResolutionInfo to selectedLayerMagInfo aligns with the PR objective and enhances code clarity.
326-327: LGTM! Consistent usage of renamed variable.
The state management logic correctly uses the renamed variable while maintaining the same functionality.
676-676: Verify MagSlider component prop update.
The prop name has been changed from resolutionInfo to magnificationInfo. Please ensure that the MagSlider component has been updated to accept this new prop name.
✅ Verification successful
Based on the shell script results, I can see that the MagSlider component is defined in the same file and accepts the magnificationInfo prop in its type definition. The component implementation shows:
export function MagSlider({
magnificationInfo,
value,
onChange,
}: {
magnificationInfo: MagInfo;
value: Vector3;
onChange: (v: Vector3) => void;
}) {This confirms that the prop rename from resolutionInfo to magnificationInfo in the review file is consistent with the component's implementation.
The prop rename is correctly implemented.
The MagSlider component already accepts magnificationInfo as a prop, so the change in the review file is valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for MagSlider component definition and its prop types
ast-grep --pattern 'type MagSliderProps = {
$$$
magnificationInfo: $_
$$$
}'
# Search for any remaining usage of resolutionInfo prop
rg 'resolutionInfo.*MagSlider'
Length of output: 117
Script:
#!/bin/bash
# Search for MagSlider component definition with more flexible pattern
ast-grep --pattern 'const MagSlider = ({ $$ }: $_) => {'
# Search for MagSlider component with type definition using a different pattern
ast-grep --pattern 'interface MagSliderProps {
$$$
}'
# Search for any file containing MagSlider component
fd MagSlider --exec cat {}
# Search for any usage of MagSlider component with props
rg -A 2 '<MagSlider'
Length of output: 509
Script:
#!/bin/bash
# Search for MagSlider component in the entire codebase
rg -l "MagSlider" --type typescript --type tsx
# Once we find the file, let's look at its content
fd -e tsx -e ts MagSlider --type f --exec cat {}
# Search for any imports of MagSlider
rg "import.*MagSlider"
Length of output: 321
Script:
#!/bin/bash
# Let's check the content of the file containing MagSlider definition
cat "frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx"
# Also search for any other files that might contain MagSlider
rg -A 5 "MagSlider" --type ts --type tsx --type jsx --type js
Length of output: 34726
frontend/javascripts/oxalis/model/accessors/flycam_accessor.ts (5)
105-113: LGTM! Documentation is clear and consistent.
The documentation block effectively explains the function's purpose and provides helpful examples using the new "magnification" terminology.
Line range hint 380-393: LGTM! Function signature updated consistently.
The function name and parameter have been appropriately renamed from resolution to mag terminology while preserving the original logic.
427-427: LGTM! Variable names updated consistently.
All variables have been renamed from "resolution" to "mag" terminology while maintaining their original semantic meaning.
Also applies to: 581-581, 633-633
Line range hint 620-664: LGTM! Type structure maintained with updated terminology.
The function has been renamed while preserving the type structure and memoization pattern. The implementation remains consistent with the new terminology.
Line range hint 1-666: Verify resolution to mag renaming across the codebase.
While the changes in this file are consistent, let's verify that all references to these renamed functions and variables have been updated across the codebase.
✅ Verification successful
Based on the search results, I can now make a final assessment. The mixed usage of "resolution" and "mag" terminology appears in the codebase, but it's actually part of the ongoing transition where resolutions is a property name in the data structures/types while the logic and variable names use "mag" terminology consistently.
Mixed terminology is expected and correct
The search results show that:
resolutionsappears primarily as a property name in data layer types/interfaces- All the logic accessing this property uses "mag" terminology (e.g.,
getMagInfo(layer.resolutions)) - The property name remains as
resolutionsfor backward compatibility with API/data structures
This pattern is intentional and correct, as changing the property name in types/interfaces would require changes to the API contract and data structures, which is outside the scope of the terminology update.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to old terminology
echo "Checking for potential missed resolution-related references..."
rg -i "resolution(info|restrictions)" --type ts
# Search for mixed usage of resolution/mag terminology
echo "Checking for mixed terminology usage..."
rg -i "resolution.*mag|mag.*resolution" --type ts
Length of output: 6307
frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts (2)
191-195: LGTM: Variable rename maintains consistency
The rename from bucketResolution to bucketMag aligns with the PR objectives and maintains the correct calculation logic.
524-524: LGTM: Comment updated for consistency
The comment accurately reflects the magnification terminology.
Let's check for any remaining instances of "resolution" in comments or strings that might need updating:
✅ Verification successful
Let's search for any instances of "resolution" specifically in comments within the bucket.ts file to ensure consistency:
Let's verify the content of the bucket.ts file to see all comments and ensure we have the full context:
Let's check the specific file content around the comment in question:
Based on the context, I can now generate the final response:
Comment accurately describes the code behavior
The comment "The voxelToLabel is already within the bucket and in the correct magnification" is precise and helpful because:
- It explains that
voxelToLabelcoordinates are already transformed to be relative to the bucket's space - It confirms the coordinates are in the correct magnification level
- This information is important for understanding why no additional coordinate transformations are needed before using
getVoxelIndexByVoxelOffset
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for remaining instances of "resolution" in comments or strings
rg -i "resolution" --type ts
Length of output: 14402
Script:
#!/bin/bash
# Search for "resolution" in comments within bucket.ts
rg -i "resolution" frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts --type ts -g "bucket.ts" -C 2
Length of output: 111
Script:
#!/bin/bash
# Get the content of bucket.ts file with comments
rg -N "^.*//.*|^/\*[\s\S]*?\*/|^.*\*.*" frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts
Length of output: 109677
Script:
#!/bin/bash
# Get the content around line 524 in bucket.ts
rg -A 5 -B 5 "voxelToLabel is already within the bucket" frontend/javascripts/oxalis/model/bucket_data_handling/bucket.ts
Length of output: 800
frontend/javascripts/oxalis/model/sagas/quick_select_heuristic_saga.ts (1)
148-157: LGTM! Variable renaming is consistent.
The renaming from resolution to mag is consistent with the PR objectives and maintains clear, descriptive variable names.
Also applies to: 167-167, 187-187, 291-291, 361-361
frontend/javascripts/oxalis/model/sagas/min_cut_saga.ts (1)
67-71: LGTM: Consistent variable renaming from resolution to mag.
The variable renaming changes are consistent throughout the code and align with the PR objectives. The semantic meaning is preserved, and all related variables are updated accordingly.
Also applies to: 283-284, 302-313, 321-335
frontend/javascripts/oxalis/model/accessors/volumetracing_accessor.ts (3)
246-252: LGTM! Variable naming aligns with PR objectives.
The renaming from volumeResolutions to volumeMags is consistent with the PR's goal, and the logic for calculating zoom steps based on the finest magnification index is correct.
Line range hint 547-551: LGTM! Type definition is consistent.
The return type definition correctly uses mag instead of resolution and properly defines the structure.
645-651: LGTM! Parameter naming and coordinate handling are correct.
The function correctly uses the renamed mag parameter and properly handles coordinate calculations with respect to magnification levels.
frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts (5)
81-91: LGTM: Variable renaming is consistent with PR objectives.
The renaming from resolutionUnionDict to magUnionDict and resolution to mag aligns well with the PR's goal of standardizing terminology.
95-97: LGTM: Loop variable handling is consistent.
The loop implementation maintains the same logic while using consistent terminology.
100-104: LGTM: Object key handling remains correct.
The key sorting and mapping logic is preserved while using the renamed dictionary.
108-112: LGTM: Function maintains correct behavior with updated terminology.
The getWidestMags function correctly uses the updated terminology while preserving its functionality.
116-120: LGTM: Function correctly implements the terminology update.
The getSomeMagInfoForDataset function successfully adopts the new terminology while maintaining its logic for handling magnification data.
frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts (2)
Line range hint 125-149: LGTM! Parameter and property rename is consistent.
The renaming from resolution to mag in the createNode function is consistent with the PR objectives and maintains type safety.
849-849: LGTM! Property mapping rename is consistent.
The renaming from resolution to mag in the serverNodeToMutableNode function is consistent with the PR objectives and maintains type safety.
frontend/javascripts/oxalis/model_initialization.ts (3)
506-507: LGTM! Variable renaming is consistent.
The renaming from resolutions to mags aligns with the PR objective and maintains code clarity.
512-513: LGTM! Consistent usage of new variable names.
The variable tracingMags follows the new naming convention and the logic remains clear.
522-522: Verify if API property should be renamed.
The property name resolutions in the API interface wasn't updated to mags. This seems inconsistent with the PR's objective of renaming resolution to mag.
frontend/javascripts/router.tsx (1)
Line range hint 650-674: Verify consistent usage of the new terminology.
Let's verify that the renaming from "resolution" to "mag" is consistent across the codebase, particularly in URL parameters and API calls.
✅ Verification successful
Let me try with corrected file type flags for ripgrep.
Let me try one more time with the correct ripgrep file type flags.
Based on the search results, I can now generate the final response:
The renaming from "resolution" to "mag" has been consistently implemented across the codebase.
The evidence shows:
- The type
APIMagRestrictionsis used consistently in frontend code - Database migrations (122-resolution-to-mag.sql) properly renamed columns from
settings_resolutionRestrictions_min/maxtosettings_magRestrictions_min/max - All API endpoints and parameters use the "mag" terminology
- Documentation and comments have been updated to use "mag" or "magnification" instead of "resolution"
- The only remaining "resolution" references are in:
- Legacy database migration files (which is expected)
- Documentation explaining that "mags" were previously called "resolutions" (which is helpful context)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of "resolution" in related contexts
echo "Searching for resolution-related terms in the codebase..."
rg -i "resolution.*?restrict|min.*?resolution|max.*?resolution" --type ts --type tsx
# Search for the new terminology to ensure consistent usage
echo "Verifying consistent usage of new terminology..."
rg -i "mag.*?restrict|min.*?mag|max.*?mag" --type ts --type tsx
# Search for potentially missed instances in API types
echo "Checking API type definitions..."
ast-grep --pattern 'type API$_Resolution'
Length of output: 559
Script:
#!/bin/bash
# Search for any remaining instances of "resolution" in related contexts
echo "Searching for resolution-related terms in the codebase..."
rg -i "resolution.*?restrict|min.*?resolution|max.*?resolution" -t typescript -t tsx
# Search for the new terminology to ensure consistent usage
echo "Verifying consistent usage of new terminology..."
rg -i "mag.*?restrict|min.*?mag|max.*?mag" -t typescript -t tsx
# Search for API types with both old and new terminology
echo "Checking API type definitions..."
ast-grep --pattern 'interface API$_ { $$$resolution$$$}'
ast-grep --pattern 'interface API$_ { $$$mag$$$}'
Length of output: 640
Script:
#!/bin/bash
# Search for any remaining instances of "resolution" in related contexts
echo "Searching for resolution-related terms in the codebase..."
rg -i "resolution.*?restrict|min.*?resolution|max.*?resolution" --type-add 'typescript:*.{ts,tsx}'
# Search for the new terminology to ensure consistent usage
echo "Verifying consistent usage of new terminology..."
rg -i "mag.*?restrict|min.*?mag|max.*?mag" --type-add 'typescript:*.{ts,tsx}'
# Let's also check the API types and interfaces more broadly
echo "Checking type definitions..."
ast-grep --pattern 'type APIMagRestrictions = {'
ast-grep --pattern 'type APIResolutionRestrictions = {'
Length of output: 78129
frontend/javascripts/oxalis/model/sagas/volumetracing_saga.tsx (4)
215-219: LGTM: Variable renaming is consistent
The renaming from maybeLabeledResolutionWithZoomStep to maybeLabeledMagWithZoomStep aligns with the PR objectives and maintains the logical flow of the code.
254-254: LGTM: Destructuring assignment updated correctly
The destructuring assignment has been properly updated to use mag instead of resolution, maintaining consistency with the variable renaming.
259-259: LGTM: Variable usage updated consistently
The renamed variable labeledMag is correctly used in the createVolumeLayer function call.
611-616: LGTM: Boolean variable renamed appropriately
The renaming from isResolutionTooLow to isMagTooLow is consistent with the PR objectives while maintaining the logical meaning of the check.
frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts (5)
242-243: LGTM: Consistent renaming of resolution to mag
The renaming is consistent with the PR objectives and the calculations using the mag vector components remain correct.
Also applies to: 257-259
633-640: LGTM: Consistent use of mag in coordinate calculations
The renaming is consistent and the coordinate calculations using mag components are implemented correctly.
Also applies to: 700-700
824-829: LGTM: Consistent usage of mags array for zoom steps
The bounds checking using mags.length is implemented correctly in both zoom step methods.
Also applies to: 843-848
923-926: LGTM: Consistent use of mag in voxel offset calculations
The renaming and usage of mag components in voxel offset calculations is implemented correctly.
Line range hint 1-1000: Verify complete renaming of resolution to mag
Let's verify that all instances of "resolution" have been updated to "mag" in this file, except for the known exceptions mentioned in the PR objectives.
✅ Verification successful
Let me try a different search approach to verify the renaming:
Based on the search results, I can now provide a final response:
All resolution-to-mag renaming verified in data_cube.ts
The search results confirm that in frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts, all instances of "resolution" have been properly renamed to "mag", with the only exceptions being the intentionally deprecated methods:
_labelVoxelInResolution_DEPRECATED- References to this deprecated method in method calls
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of "resolution" that might need to be updated
# Excluding the known exception _labelVoxelInResolution_DEPRECATED
rg -i "resolution" frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts | grep -v "_labelVoxelInResolution_DEPRECATED"
Length of output: 131
Script:
#!/bin/bash
# Search for any remaining instances of "resolution" case-insensitively
# Using ast-grep to find any resolution-related identifiers or strings
ast-grep --pattern '$_resolution$_'
# Also do a plain text search with ripgrep for any remaining "resolution" strings
rg -i "resolution" --type ts
Length of output: 14440
frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts (1)
605-606: LGTM! Parameter renaming aligns with PR objectives.
The renaming of the resolution parameter to mag in the CREATE_NODE case is consistent with the PR's goal of standardizing terminology.
Also applies to: 617-617
frontend/javascripts/oxalis/model/helpers/nml_helpers.ts (2)
47-47: LGTM: Constant declaration is consistent with the PR objective.
The constant DEFAULT_MAG is appropriately named and initialized.
966-966: LGTM: Node property assignment follows the new naming convention.
The property name change from resolution to mag is consistent with the PR objective.
Let's verify the naming consistency across the codebase:
✅ Verification successful
Based on the executed shell scripts and their results, I can now generate the final response:
Property name change from resolution to mag is correctly implemented.
The verification confirms that there are no remaining instances of the old "resolution" property name in NML-related files or interfaces. The change is consistent with the codebase's naming convention.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of "resolution" in related contexts
# that might need to be updated for consistency.
# Search for resolution in property assignments and type definitions
rg -A 2 'resolution:' --type ts
# Search for resolution in variable declarations
rg -A 2 '(const|let|var).*resolution' --type ts
# Search for resolution in function parameters
rg -A 2 'function.*resolution.*\)' --type ts
Length of output: 10500
Script:
#!/bin/bash
# Let's search for any remaining instances of "inResolution" in NML-related files
# and check if there are any other properties that need to be updated for consistency
# Search for "inResolution" in NML-related files
rg -A 2 'inResolution' --type ts
# Search for property assignments in NML files specifically
ast-grep --pattern 'attr, "inResolution"'
# Search for any NML-related type definitions or interfaces
rg -A 5 'interface.*NML' --type ts
Length of output: 129011
Script:
#!/bin/bash
# Let's search for any remaining instances of "resolution" in NML-related files
# and check for any type definitions or interfaces that might need updating
# Search for NML-related files with "resolution" in them
rg -l 'nml.*resolution|resolution.*nml' --type ts
# Search for any NML-related type definitions or interfaces with "resolution"
ast-grep --pattern 'interface $_ {
$$$
resolution: $_
$$$
}'
# Search for any NML property assignments with "resolution"
rg -A 2 'resolution:.*nml' --type ts
Length of output: 159
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts (2)
487-492: LGTM! Consistent renaming of resolution-related variables.
The renaming from resolutionInfo to magInfo and associated method calls is consistent with the PR objective of standardizing terminology.
1115-1119: LGTM! Consistent renaming in getTotalMagCount method.
The variable and method call renaming is consistent with the PR objective of standardizing terminology.
frontend/javascripts/oxalis/view/action-bar/toolbar_view.tsx (1)
869-869: LGTM! Variable renaming is consistent with PR objectives.
The renaming from resolution to mag is implemented correctly and maintains consistency with the broader codebase changes:
maybeMagWithZoomStepreplacesmaybeResolutionWithZoomSteplabeledMaguses the renamed property appropriately
Also applies to: 871-873
frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx (6)
69-69: LGTM: Import renamed to use "mag" terminology.
The import statement has been correctly updated to reflect the new function name, maintaining consistency with the PR's objective.
313-324: LGTM: Component updated to use magnification terminology.
The changes correctly:
- Rename the variable from
resolutionInfotomagInfo - Update the UI text to use "magnifications" instead of "resolutions"
These changes improve clarity and maintain consistency with the new terminology.
Line range hint 1029-1079: LGTM: Data finding logic updated to use magnification terminology.
The handleFindData function has been correctly updated to use the new terminology while maintaining the same functionality.
1107-1125: LGTM: Downsampling logic updated to use magnification terminology.
The getVolumeMagsToDownsample function has been properly updated to use the new terminology:
volumeTargetMaginstead ofvolumeTargetResolution- Consistent use of
magterminology throughout the function
1136-1138: LGTM: Condition check updated to use magnification terminology.
The condition for checking extensive magnifications has been updated to use consistent terminology.
1627-1628: LGTM: Zoom function updated to use magnification terminology.
The onZoomToMag function has been correctly updated:
- Parameter renamed from
resolutiontomag - Uses the renamed
getMaxZoomValueForMagfunction
frontend/javascripts/test/reducers/skeletontracing_reducer.spec.ts (2)
104-104: LGTM: Variable renamed from resolution to mag.
The renaming aligns with the PR objectives and maintains the same test behavior.
Line range hint 106-1417: LGTM: Consistent renaming across all test cases.
The variable has been renamed from resolution to mag consistently throughout all test cases where it's used as a parameter to createNodeAction. The parameter order and values remain unchanged, preserving test behavior.
frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segments_view.tsx (3)
831-843: LGTM! Variable renaming is consistent with the PR objectives.
The renaming from resolutionInfo to magInfoOfVisibleSegmentationLayer and related method calls has been implemented consistently. The logic remains unchanged while improving naming clarity.
896-901: LGTM! Method signature updates maintain consistency.
The method signatures have been updated to use magIndex instead of resolutionIndex, maintaining consistency with the new naming convention.
905-906: LGTM! JSX template updates reflect new naming convention.
The JSX template has been updated consistently to use the new variable names and method calls. All changes maintain the same functionality while using the new "mag" terminology.
Also applies to: 917-917, 920-927, 934-935, 962-972
frontend/javascripts/admin/admin_rest_api.ts (3)
1549-1549: Return type updated correctly for findDataPositionForLayer
The renaming from resolution to mag in the return type and destructuring is consistent with the PR objectives. The change maintains type safety while improving terminology consistency.
Also applies to: 1551-1551, 1558-1558
1567-1567: Return type updated correctly for findDataPositionForVolumeTracing
The renaming from resolution to mag in the return type and destructuring matches the changes made in findDataPositionForLayer, maintaining consistency across similar functions.
Also applies to: 1569-1569, 1574-1574
Line range hint 1549-1574: Verify all usages of these functions
While the changes look good, we should verify that all callers of these functions have been updated to use the new property name.
✅ Verification successful
All callers are correctly using the updated property names
The search results show that all callers of both functions (findDataPositionForLayer and findDataPositionForVolumeTracing) are correctly using the position and mag properties in their destructuring patterns. The only usages found are in frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx, and they are already using the correct property names:
const { position, mag } = await findDataPositionForLayer(...)const { position, mag } = await findDataPositionForVolumeTracing(...)
There are no instances of outdated property usage that need to be updated.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for function calls to ensure callers are updated
echo "Searching for findDataPositionForLayer usages..."
rg "findDataPositionForLayer.*resolution"
echo "Searching for findDataPositionForVolumeTracing usages..."
rg "findDataPositionForVolumeTracing.*resolution"
# Search for any remaining uses of resolution in similar contexts
echo "Searching for other potential resolution usages that might need updating..."
rg "position.*resolution.*Vector3"
Length of output: 523
Script:
#!/bin/bash
# Let's search for all usages of these functions without focusing on 'resolution'
echo "Searching for findDataPositionForLayer usages..."
rg "findDataPositionForLayer" -A 2
echo -e "\nSearching for findDataPositionForVolumeTracing usages..."
rg "findDataPositionForVolumeTracing" -A 2
# Also search for destructuring of these function results
echo -e "\nSearching for destructuring patterns..."
rg "const.*\{.*position.*mag.*\}.*=.*find(DataPositionForLayer|DataPositionForVolumeTracing)"
Length of output: 2548
frontend/javascripts/oxalis/api/api_latest.ts (7)
672-674: LGTM! Consistent variable renaming.
The renaming from resolutions to magInfo aligns with the PR objectives and improves code clarity by using more specific terminology.
678-680: LGTM! Appropriate variable renaming.
The renaming from currentResolution to currentMag is consistent with the PR objectives and maintains code clarity.
1843-1844: LGTM! Consistent renaming across multiple methods.
The renaming from resolutions to magInfo is consistently applied across multiple methods, improving code clarity while maintaining functionality.
Also applies to: 1900-1900, 1906-1906, 1909-1909, 1912-1912
1933-1933: LGTM! Consistent parameter and variable renaming.
The renaming from maybeResolutionIndex to maybeMagIndex in the method signature and related changes are consistent with the PR objectives and improve code clarity.
Also applies to: 1946-1950
1961-1964: LGTM! Consistent variable renaming in viewport calculations.
The renaming to use mag terminology in viewport calculations is consistent with the PR objectives and maintains code clarity.
2107-2108: LGTM! Consistent renaming in URL generation.
The renaming from resolutions to magInfo in the URL generation method is consistent with the PR objectives and maintains code clarity.
2181-2181: LGTM! Correctly preserved deprecated method name.
The deprecated method name _labelVoxelInResolution_DEPRECATED was intentionally not renamed, which is consistent with the PR objectives and good deprecation practices.
frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx (1)
263-263: Comment update improves clarity
The updated comment in the MagSlider function clarifies the use of getMagsWithIndices by noting that it returns a sorted list. This enhances code readability.
frontend/javascripts/oxalis/model/sagas/mesh_saga.ts (5)
243-243: Retrieving magInfo from layer.mags is appropriate
The use of magInfo = getMagInfo(layer.mags) aligns with the updated codebase and ensures consistency in handling magnification information.
276-276: Correct usage of magInfo and zoomStep
Destructuring magInfo and zoomStep from getInfoForMeshLoading ensures that the mesh loading functions receive the correct parameters.
345-345: Ensure magInfo contains the requested zoomStep
Using magInfo.getMagByIndexOrThrow(zoomStep) is appropriate. However, consider handling potential exceptions if zoomStep is not found within magInfo.
If exceptions are already handled upstream or zoomStep is guaranteed to exist in magInfo, no action is needed.
474-474: Consistent retrieval of mag from magInfo
Fetching mag using magInfo.getMagByIndexOrThrow(zoomStep) maintains consistency with the updated magnification handling.
520-520: Passing magInfo to getNeighborPosition
Providing magInfo to calculate neighbor positions ensures accurate mesh generation.
frontend/javascripts/oxalis/model/sagas/proofread_saga.ts (2)
1044-1044: Verify Correctness of magInfo.getFinestMag()
Ensure that magInfo.getFinestMag() returns the expected finest magnification level.
1050-1050: Use of magInfo.getIndexByMag is Correct
The method magInfo.getIndexByMag(agglomerateFileMag) is appropriately used to obtain the index corresponding to the given magnification. This usage aligns with the updated terminology.
frontend/javascripts/oxalis/view/context_menu.tsx (1)
1463-1484: Variable renaming from layersFinestResolution to layersFinestMag is consistent and accurate
The changes from layersFinestResolution to layersFinestMag are properly applied throughout the code segment. All function calls and calculations now use layersFinestMag, which aligns with the updated terminology and ensures consistency across the codebase.
frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx
Show resolved
Hide resolved
frontend/javascripts/test/sagas/volumetracing/volumetracing_saga.spec.ts
Show resolved
Hide resolved
| const labeledZoomStep = magInfo.getClosestExistingIndex( | ||
| requestedZoomStep, | ||
| "The visible color layer and the active segmentation layer don't have any magnifications in common. Cannot select segment.", | ||
| ); |
There was a problem hiding this comment.
Add error handling for the case when no common magnifications exist.
The error message in getClosestExistingIndex is only shown when no closest index is found, but there's no handling for the case when the intersection of magnifications is empty.
Consider adding a check:
const magInfo = getMagInfo(
// Ensure that a magnification is used which exists in the color layer as well as the
// target segmentation layer.
- _.intersectionBy(colorLayer.resolutions, volumeLayer.resolutions, (mag) => mag.join("-")),
+ const commonMags = _.intersectionBy(colorLayer.resolutions, volumeLayer.resolutions, (mag) => mag.join("-"));
+ if (commonMags.length === 0) {
+ throw new Error("The color layer and segmentation layer have no common magnifications.");
+ }
+ commonMags
);Committable suggestion skipped: line range outside the PR's diff.
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts (1)
Let me analyze the results and generate a final response.
Verify consistent renaming of "resolution" to "mag" in the codebase
The codebase still contains several occurrences of "resolution" that should be renamed to "mag" for consistency:
frontend/javascripts/dashboard/dataset/dataset_settings_data_tab.tsx: Line 294 usesres.resolutionfrontend/javascripts/oxalis/model/helpers/nml_helpers.ts: Line 415 usesnode.resolutionwebknossos-datastore/app/com/scalableminds/webknossos/datastore/explore/PrecomputedExplorer.scala: Lines 40 and 63 useresolutionwebknossos-datastore/app/com/scalableminds/webknossos/datastore/dataformats/layers/WKWDataLayers.scala: Multiple occurrences ofresolutionin method names and variablesThese instances should be updated to use "mag" terminology for consistency with the recent changes.
🔗 Analysis chain
Line range hint
125-149: Verify consistent renaming across the codebaseLet's verify that all occurrences of "resolution" have been consistently renamed to "mag" in related components.
Also applies to: 854-858
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for remaining occurrences of "resolution" that should be renamed to "mag" # Look for resolution in type definitions and variable declarations # Search for resolution in type definitions echo "Checking type definitions with 'resolution':" ast-grep --pattern 'interface $_ { $$$ resolution: $_ $$$ }' # Search for resolution variable declarations echo "Checking variable declarations with 'resolution':" ast-grep --pattern 'resolution = $_' # Search for resolution property access echo "Checking property access with 'resolution':" ast-grep --pattern '$_.resolution'Length of output: 404603
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
CHANGELOG.unreleased.md(1 hunks)frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx(2 hunks)frontend/javascripts/oxalis/model/helpers/nml_helpers.ts(2 hunks)frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts(2 hunks)frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts(3 hunks)frontend/javascripts/oxalis/model/sagas/proofread_saga.ts(1 hunks)frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segments_view.tsx(6 hunks)frontend/javascripts/test/reducers/skeletontracing_reducer.spec.ts(38 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- CHANGELOG.unreleased.md
- frontend/javascripts/oxalis/model/actions/skeletontracing_actions.tsx
- frontend/javascripts/oxalis/model/helpers/nml_helpers.ts
- frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer.ts
- frontend/javascripts/oxalis/model/sagas/proofread_saga.ts
- frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segments_view.tsx
- frontend/javascripts/test/reducers/skeletontracing_reducer.spec.ts
🧰 Additional context used
🪛 Biome
frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts
[error] 854-854: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 854-854: Expected an expression but instead found '<<'.
Expected an expression here.
(parse)
[error] 855-855: expected > but instead found mag
Remove mag
(parse)
[error] 855-855: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts
Show resolved
Hide resolved
frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts
Outdated
Show resolved
Hide resolved
|
@fm3 thank you for your insights!
From what I can tell, the frontend As for the DataLayer type, we have two types in the frontend: |
knollengewaechs
force-pushed
the
rename-resolution-to-mag-localvars
branch
from
b798c4e to
6bf39dd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts (1)
Line range hint
220-248: Consider enhancing metadata validation.While the test covers different metadata types well, consider adding explicit assertions to validate the metadata values in the response, rather than relying solely on snapshot testing. This would make the test more robust against unintended metadata changes.
Example enhancement:
await sendUpdateActionsForSkeleton(createdExplorational, saveQueue); const tracings = await api.getTracingsForAnnotation(createdExplorational); + // Validate metadata values explicitly + const updatedTree = tracings[0].trees.find(tree => tree.id === trees[1].id); + t.deepEqual(updatedTree.metadata, metadata, "Tree metadata should match"); t.snapshot(replaceVolatileValues(tracings[0]));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
frontend/javascripts/dashboard/explorative_annotations_view.tsx(1 hunks)frontend/javascripts/oxalis/model/helpers/nml_helpers.ts(3 hunks)frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts(3 hunks)frontend/javascripts/oxalis/model/sagas/update_actions.ts(1 hunks)frontend/javascripts/oxalis/store.ts(1 hunks)frontend/javascripts/oxalis/view/right-border-tabs/connectome_tab/connectome_view.tsx(1 hunks)frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts(2 hunks)frontend/javascripts/test/libs/nml.spec.ts(1 hunks)frontend/javascripts/test/reducers/skeletontracing_reducer.spec.ts(40 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/javascripts/dashboard/explorative_annotations_view.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- frontend/javascripts/oxalis/model/helpers/nml_helpers.ts
- frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts
- frontend/javascripts/test/reducers/skeletontracing_reducer.spec.ts
🔇 Additional comments (9)
frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts (3)
Line range hint 204-216: LGTM! Well-structured test case for complex updates.
The test follows good testing practices:
- Clear arrangement of test data with dummy trees and groups
- Proper action execution with diffing and update actions
- Verification through snapshot comparison
216-216: LGTM! Good addition of type checking.
Adding type checking for the tracing result enhances type safety and helps catch potential issues early.
Line range hint 1-248: Verify alignment with PR objectives.
While these changes add valuable test coverage, they appear unrelated to the PR's primary objective of renaming "resolution" to "mag". Please clarify if these test additions should be in a separate PR to maintain focused changes.
frontend/javascripts/oxalis/model/sagas/update_actions.ts (2)
223-227: LGTM! Type definition maintains backward compatibility.
The ActionCreateNode type correctly maps the properties while maintaining backward compatibility with the backend by keeping resolution in the type definition.
235-243: Verify consumers of createNode action.
The function correctly maps mag to resolution in the action value. However, let's verify that all consumers of this action handle this mapping correctly.
Let's check for any direct consumers of the createNode action:
✅ Verification successful
Let me gather more context about how these files handle the resolution/mag mapping.
Let me check one more aspect to understand how the Node type is structured and how the resolution/mag is handled in the actual node creation.
Based on the gathered information, I can now provide a final response:
The mapping of mag to resolution in createNode action is correct and safe.
The codebase shows that:
- The mapping is part of a larger effort to rename "resolution" to "mag" across the codebase (as seen in CHANGELOG.released.md and migration files)
- The backend (SkeletonUpdateActions.scala) already handles both fields correctly, using
resolutionas an optional parameter - All consumers of the createNode action (in skeleton.ts, skeletontracing_saga.ts, and skeletontracing_reducer.ts) work with the mapped field correctly
The change aligns with the codebase-wide migration from "resolution" to "mag" terminology while maintaining backward compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find consumers of createNode action to verify they handle the mag/resolution mapping correctly
# Search for direct imports and uses of createNode
rg -l "createNode.*from.*update_actions" | xargs rg "createNode\("
# Search for action type checks
rg "action.*name.*createNode"
# Search for ActionCreateNode type usage
rg "ActionCreateNode"
Length of output: 1286
Script:
#!/bin/bash
# Check implementation of createNode in skeleton.ts and skeletontracing_reducer_helpers.ts
ast-grep --pattern 'createNode($$$) { $$$ }'
# Check Node type definition to understand the field structure
rg "type.*Node.*=.*{" -A 10
# Check how the action is handled in reducers
rg "case.*createNode" -A 5
Length of output: 2667
Script:
#!/bin/bash
# Search for Node type definition with better pattern
rg "type\s+Node\s*=" -A 10
# Check the actual node creation implementation in skeleton.ts
rg -A 10 "createNode\(treeId: number, node: Node"
# Check for any direct usage of resolution field
rg "\bresolution\b"
Length of output: 93088
frontend/javascripts/oxalis/store.ts (1)
92-92: LGTM! Verify API communication safety.
The renaming from resolution to mag in the MutableNode type aligns with the PR objectives. Since this type is used in the frontend state management, we should verify that it doesn't affect backend communication.
Let's verify that this type isn't directly used in API communication:
frontend/javascripts/test/libs/nml.spec.ts (1)
37-37: Verify backend compatibility with the 'mag' field.
The renaming from 'resolution' to 'mag' in createDummyNode aligns with the PR objectives. However, we should ensure that the NML serialization/parsing remains compatible with backend expectations.
✅ Verification successful
Based on the search results, I can see that the codebase has been updated to consistently use mag instead of resolution across both frontend and backend code. The change in the test file aligns with this broader refactoring. The key observations:
- Backend code (Scala) uses
magconsistently in various components likeVolumeTracingService,DataLayer, etc. - Frontend code (TypeScript/JavaScript) also uses
magthroughout, including in test files, models, and UI components - The change from
resolutiontomagappears to be a systematic rename across the codebase - The test file's change is consistent with this broader pattern
The change from 'resolution' to 'mag' in the test file is part of a consistent codebase-wide refactoring and is correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if backend code still uses 'resolution' field
# Look for 'resolution' field usage in backend files
# Search for resolution field in backend files
rg -A 5 'resolution.*=|resolution:' --type ts --type scala
# Search for mag field in backend files to ensure it's recognized
rg -A 5 'mag.*=|mag:' --type ts --type scala
Length of output: 439750
frontend/javascripts/oxalis/view/right-border-tabs/connectome_tab/connectome_view.tsx (2)
166-166: LGTM: Property rename from resolution to mag
The change aligns with the PR objectives to improve naming consistency across the codebase.
Line range hint 166-174: Verify type consistency for node consumers
Let's ensure that all consumers of the node object are updated to use mag instead of resolution.
✅ Verification successful
Let me gather more context about the node-related code and the mag property usage to make a more informed decision.
Based on the gathered information, I can now provide the final response:
No type consistency issues with mag property
The codebase shows that the mag property is already consistently used across all node-related types and implementations:
- The
ServerNodeandMutableNodetypes in the frontend both usemagproperty - The backend code shows
NodeDefaults.magbeing used as the default value - No remaining usages of
resolutionproperty were found in node-related code - The change aligns with the existing type definitions and usage patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for any remaining usages of 'resolution' in node-related code
# that might need to be updated to 'mag'
# Look for resolution property access in node-related code
rg -l "resolution.*Node" | xargs rg "resolution"
# Look for MutableNode type usage and resolution
ast-grep --pattern 'type MutableNode = {
$$$
resolution: $_
$$$
}'
Length of output: 335
Script:
#!/bin/bash
# Search for type definitions and usages related to mag property
ast-grep --pattern 'type $_ = {
$$$
mag: $_
$$$
}'
# Search for direct mag property access
rg "\.mag\b" -A 2
# Search for NodeDefaults usage
rg "NodeDefaults" -A 2
Length of output: 50908
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
frontend/javascripts/oxalis/model/helpers/generate_dummy_trees.ts (1)
37-37: Consider documenting the significance of the magnification valueSince this is a test helper generating mock data, it would be helpful to document why a magnification value of 2 is used and how it relates to real-world scenarios.
viewport: 1, + // Default magnification value of 2 represents [explain significance here] mag: 2,frontend/javascripts/test/snapshots/public-test/test-bundle/test/backend-snapshot-tests/annotations.e2e.js.md (1)
Based on the evidence gathered, I can now provide a final response:
Backend uses
magfield in protobuf definitionThe backend's protobuf definition (SkeletonTracing.proto) explicitly defines the field as
mag(not resolution), and the SkeletonUpdateActions.scala shows thatresolutionvalues from requests are mapped toNodeDefaults.mag. This indicates that:
- The backend already expects and uses
maginternally- The frontend test snapshot should be updated to use
maginstead ofresolutionto match the backend's field name🔗 Analysis chain
Line range hint
1513-3108: Verify backend compatibility with the new field nameSince the backend hasn't been updated (as mentioned in PR objectives), we should verify that these test snapshots accurately reflect what the API will return. The backend may still be using "resolution" instead of "mag".
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check if the backend API still uses "resolution" field # Look for backend API response handling and models # Search for resolution field in backend files rg -l "resolution.*=|resolution.*:" --type-add 'backend:*.{scala,java}' -t backend # Search for API response mapping code rg -l "resolution.*toJson|resolution.*fromJson" --type-add 'backend:*.{scala,java}' -t backendLength of output: 1709
Script:
#!/bin/bash # Let's examine the actual usage of resolution in these backend files # to understand if it's used in API responses # Check SkeletonUpdateActions.scala for resolution field usage rg -A 5 -B 5 "resolution" "webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/updating/SkeletonUpdateActions.scala" # Check Dataset.scala as it likely contains API response models rg -A 5 -B 5 "resolution" "app/models/dataset/Dataset.scala" # Look for any JSON/API serialization code related to resolution rg -A 5 -B 5 "resolution.*json" -iLength of output: 13424
Script:
#!/bin/bash # Let's check the NodeDefaults class to understand the mapping between resolution and mag rg -A 5 -B 5 "NodeDefaults" webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/ # Also check if there are any JSON serialization formats that map between resolution and mag rg -A 5 -B 5 "mag.*json|resolution.*json" webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/ # Look for any Proto definitions that might show the field mapping fd -e proto . | xargs rg -l "resolution|mag"Length of output: 8970
Script:
#!/bin/bash # Let's check the proto files to understand the field definitions rg -A 5 -B 5 "resolution|mag" webknossos-datastore/proto/SkeletonTracing.proto # Also check the NodeDefaults class to see how mag is defined rg -A 5 -B 5 "class NodeDefaults" webknossos-datastore/app/com/scalableminds/webknossos/datastore/helpers/ # Look for any field mapping between resolution and mag in the Node class rg -A 5 -B 5 "class Node" webknossos-datastore/proto/SkeletonTracing.protoLength of output: 628
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
frontend/javascripts/test/snapshots/public-test/test-bundle/test/backend-snapshot-tests/annotations.e2e.js.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
CHANGELOG.unreleased.md(1 hunks)frontend/javascripts/oxalis/api/api_latest.ts(9 hunks)frontend/javascripts/oxalis/model/helpers/generate_dummy_trees.ts(1 hunks)frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts(3 hunks)frontend/javascripts/oxalis/model/sagas/update_actions.ts(1 hunks)frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx(1 hunks)frontend/javascripts/router.tsx(2 hunks)frontend/javascripts/test/snapshots/public-test/test-bundle/test/backend-snapshot-tests/annotations.e2e.js.md(60 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- CHANGELOG.unreleased.md
- frontend/javascripts/oxalis/api/api_latest.ts
- frontend/javascripts/oxalis/model/reducers/skeletontracing_reducer_helpers.ts
- frontend/javascripts/oxalis/model/sagas/update_actions.ts
- frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx
- frontend/javascripts/router.tsx
🔇 Additional comments (1)
frontend/javascripts/test/snapshots/public-test/test-bundle/test/backend-snapshot-tests/annotations.e2e.js.md (1)
Line range hint 1513-3108: LGTM: Consistent renaming of resolution to mag in node objects
The changes align with the PR objectives, showing consistent renaming from resolution to mag across all node objects in the test snapshots. The magnification values are properly set to 2 throughout the test cases.
|
Well then! I am excited to hear what you say @MichaelBuessemeyer (and I hope its okay that I requested a review from you; its because we already worked on the previous PR and the bug fixes in between) |
There was a problem hiding this comment.
(and I hope its okay that I requested a review from you; its because we already worked on the previous PR and the bug fixes in between)
Sure thing. Thanks for taking on this task. I only managed to take a look now as #8075 had priority.
I did not edit the shader code
Should I take over this task?
-> Well as I already located the variables that needed renaming, I took the freedom to do the renaming right away to save some time. So that I do not need to refind the code locations
I did not rename deprecated methods (e.g. data_cube.ts _labelVoxelInResolution_DEPRECATED)
But the changes in api_latest.ts look like you just did that 🤔. Maybe the description is out of sync here?
Thanks for the testing notes 🙏, everything went fine and besides my mini comments I did not find anything.
=> I'd say this pr is ready to go. But please first check my commit doing the renaming for the shader relevant code: 67965fa.
If you think that's fine and datasets with multiple resolutions still render correctly, this should be ready to 
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts
Outdated
Show resolved
Hide resolved
frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts
Outdated
Show resolved
Hide resolved
thats really nice, thank you! 🙏
I will test it of course! (edit: that went well :) ) but I have never worked with shader relevant code and dont know anything about the structure and its flow, so maybe someone else can have a quick look at the code, e.g. @philippotto? |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
frontend/javascripts/oxalis/model/helpers/nml_helpers.ts (1)
Line range hint
1-1199: Consider updating documentation and migration guide.While the code changes look good, consider:
- Adding a migration guide entry for users upgrading from older versions
- Updating any relevant documentation about NML file format
- Adding a comment in the code about backward compatibility handling
frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segments_view.tsx (1)
895-900: LGTM! Method renames follow the new convention.The method renames from resolution to mag are consistent. The parameter names and implementation details remain functionally equivalent.
Consider adding JSDoc comments to these methods to document the purpose of the magIndex parameter and its valid range:
+/** Updates the preferred quality setting for mesh precomputation based on the magnification index */ handleQualityChangeForPrecomputation = (magIndex: number) => Store.dispatch(updateTemporarySettingAction("preferredQualityForMeshPrecomputation", magIndex)); +/** Updates the preferred quality setting for ad-hoc mesh generation based on the magnification index */ handleQualityChangeForAdHocGeneration = (magIndex: number) => Store.dispatch( updateTemporarySettingAction("preferredQualityForMeshAdHocComputation", magIndex), );frontend/javascripts/oxalis/api/api_latest.ts (1)
Based on the search results, I can now provide a final assessment of the resolution/mag renaming consistency:
Renaming is mostly consistent but there are a few remaining instances to address
- The codebase has largely adopted "mag" terminology, but there are a few remaining instances of "resolution" that should be updated for consistency:
_labelVoxelInAllResolutions_DEPRECATEDand_labelVoxelInResolution_DEPRECATEDmethods infrontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts(though these are marked as deprecated)MOVIE_RESOLUTIONSenum infrontend/javascripts/types/api_flow_types.tsshould be renamed toMOVIE_QUALITIESor similar since it refers to video quality rather than magnificationwkwResolutionsin datasource schema types should be renamed towkwMagsfor consistency- The message key
dataset.mag_explanationstill uses "resolution" in its text contentThe majority of the codebase correctly uses "mag" terminology, but these remaining instances should be updated to maintain full consistency across the codebase.
🔗 Analysis chain
Line range hint
673-2109: Verify consistent renaming across the codebase.Let's verify that the resolution to mag renaming is consistent throughout the codebase.
Let me try a different search pattern to get more comprehensive results about the resolution/mag usage.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining instances of "resolution" that should have been renamed to "mag" # and verify consistent usage of the new terminology. echo "Checking for remaining resolution-related terms that should have been renamed..." # Look for resolution-related variable declarations ast-grep --pattern 'const $resolution = $_' # Look for resolution-related method calls ast-grep --pattern '$obj.resolution($_)' # Look for resolution-related property access ast-grep --pattern '$obj.resolution' echo "Verifying consistent usage of new mag terminology..." # Look for mag-related declarations to verify consistency ast-grep --pattern 'const $mag = $_' # Look for mag-related method calls to verify consistency ast-grep --pattern '$obj.mag($_)' # Look for mag-related property access to verify consistency ast-grep --pattern '$obj.mag'Length of output: 520
Script:
#!/bin/bash # Search for any remaining instances of "resolution" in the codebase echo "Searching for resolution-related terms..." rg -i "resolution" -g "*.{ts,js,tsx,jsx}" -B 2 -A 2 echo -e "\nSearching for mag-related terms..." rg -i "mag[^a-z]" -g "*.{ts,js,tsx,jsx}" -B 2 -A 2Length of output: 105858
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (21)
CHANGELOG.unreleased.md(1 hunks)frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx(6 hunks)frontend/javascripts/oxalis/api/api_latest.ts(8 hunks)frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts(5 hunks)frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts(1 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts(8 hunks)frontend/javascripts/oxalis/model/bucket_data_handling/wkstore_adapter.ts(3 hunks)frontend/javascripts/oxalis/model/helpers/nml_helpers.ts(3 hunks)frontend/javascripts/oxalis/model/sagas/mesh_saga.ts(10 hunks)frontend/javascripts/oxalis/model_initialization.ts(2 hunks)frontend/javascripts/oxalis/store.ts(1 hunks)frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx(3 hunks)frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx(1 hunks)frontend/javascripts/oxalis/view/jobs/train_ai_model.tsx(1 hunks)frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx(5 hunks)frontend/javascripts/oxalis/view/right-border-tabs/connectome_tab/connectome_view.tsx(1 hunks)frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx(5 hunks)frontend/javascripts/oxalis/view/right-border-tabs/segments_tab/segments_view.tsx(6 hunks)frontend/javascripts/router.tsx(2 hunks)frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts(2 hunks)frontend/javascripts/test/snapshots/public-test/test-bundle/test/backend-snapshot-tests/annotations.e2e.js.md(60 hunks)
🚧 Files skipped from review as they are similar to previous changes (12)
- CHANGELOG.unreleased.md
- frontend/javascripts/oxalis/geometries/materials/plane_material_factory.ts
- frontend/javascripts/oxalis/model/bucket_data_handling/data_cube.ts
- frontend/javascripts/oxalis/model/bucket_data_handling/wkstore_adapter.ts
- frontend/javascripts/oxalis/model_initialization.ts
- frontend/javascripts/oxalis/store.ts
- frontend/javascripts/oxalis/view/action-bar/download_modal_view.tsx
- frontend/javascripts/oxalis/view/action-bar/starting_job_modals.tsx
- frontend/javascripts/oxalis/view/jobs/train_ai_model.tsx
- frontend/javascripts/oxalis/view/right-border-tabs/connectome_tab/connectome_view.tsx
- frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx
- frontend/javascripts/router.tsx
🧰 Additional context used
📓 Learnings (1)
frontend/javascripts/oxalis/model/sagas/mesh_saga.ts (2)
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/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`.
🔇 Additional comments (30)
frontend/javascripts/dashboard/advanced_dataset/create_explorative_modal.tsx (4)
93-99: LGTM! Consistent renaming of resolution to mag.
The variable renaming maintains the same logic while improving terminology consistency across the codebase.
Also applies to: 102-103
105-108: Fix useEffect dependencies.
The biome-ignore comment correctly identifies that setMagIndices should be added to the dependency array.
197-212: LGTM! Consistent variable renaming and prop passing.
The changes maintain the same functionality while improving terminology consistency. Props are correctly passed to the RestrictMagnificationSlider component.
246-248: Verify backend compatibility with URL parameters.
The URL parameters now use minMag and maxMag. Let's verify that the backend expects these parameter names.
✅ Verification successful
The previous search revealed frontend handling of minMag/maxMag, but we need to verify the backend API contract. Let's search for the actual API endpoint definitions and route handlers to ensure these parameters are expected.
The previous search revealed backend files that might contain the API contract. Let's examine them to verify if these parameters are expected by the backend.
Based on the search results, I can now generate the final response:
URL parameters minMag and maxMag are correctly handled by the backend
The backend codebase shows that:
- The parameters are expected and processed in the VolumeTracingController
- They are used to create
MagRestrictionsobjects that control magnification levels - The client code in
WKRemoteTracingStoreClient.scalaproperly sends these parameters
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for backend route handlers that process these URL parameters
# Search for route handlers processing minMag/maxMag parameters
rg -A 5 'minMag|maxMag' --type ts
# Search for createExplorative route definition
rg -A 10 'createExplorative.*Route' --type ts
Length of output: 7543
Script:
#!/bin/bash
# Let's search for the backend route handler and API endpoint definitions
# Search for createExplorational function/route definition
rg -A 10 'createExplorational' --type ts
# Search for API endpoints or route handlers in backend directories
fd -t f . -E "frontend" | xargs rg -l "minMag|maxMag"
# Search for route type definitions
rg -A 5 "type.*Route.*{" --type ts
Length of output: 19987
Script:
#!/bin/bash
# Let's examine the backend route and API contract
# Check the Scala backend files that were found
rg -A 10 "minMag|maxMag" app/models/annotation/WKRemoteTracingStoreClient.scala webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/controllers/VolumeTracingController.scala webknossos-tracingstore/conf/com.scalableminds.webknossos.tracingstore.routes
# Search for createExplorational API endpoint definition
rg -A 10 "createExplorational.*POST" --type scala
# Search for magRestrictions type definition
rg -A 5 "type.*MagRestrictions" --type scala
Length of output: 15602
frontend/javascripts/test/backend-snapshot-tests/annotations.e2e.ts (2)
213-213: Good addition of type checking!
Adding type checking for the tracing results is a good practice that helps ensure type safety and catch potential issues early.
Line range hint 201-215: Verify test coverage for magnification field
Since this PR renames "resolution" to "mag" in CreateNodeAction and other components, we should verify that this test adequately covers the magnification field usage.
Let's check if the test exercises the renamed field:
frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts (5)
82-91: LGTM: Consistent renaming of resolution to mag
The variable renaming aligns with the PR objectives while preserving the original logic.
96-98: LGTM: Duplicate removal logic preserved
The unique magnification filtering logic is maintained while adopting the new terminology.
101-105: LGTM: Key sorting and mapping logic unchanged
The numerical sorting and mapping operations remain functionally identical with updated variable references.
109-113: LGTM: getWidestMags function maintains correct behavior
The function continues to properly identify the layer with the most magnification levels while adopting the new terminology.
117-118: LGTM: getSomeMagInfoForDataset maintains consistency
The function properly handles the magnification union while maintaining the existing logic for distinct magnifications.
Also applies to: 121-121
frontend/javascripts/oxalis/model/helpers/nml_helpers.ts (2)
47-47: LGTM: Constant rename aligns with PR objectives.
The rename from DEFAULT_RESOLUTION to DEFAULT_MAG is consistent with the PR's goal of standardizing terminology.
416-416: Verify backward compatibility with existing NML files.
The renaming of the node property from resolution to mag in both serialization and parsing looks correct. However, we should ensure that:
- Existing NML files using
resolutioncan still be parsed - Generated NML files using
magare compatible with other tools
Also applies to: 967-967
✅ Verification successful
Based on the gathered information, I can now generate the final response:
Backward compatibility with existing NML files is maintained.
The code shows that:
- The NML parser handles both
inMagand legacyresolutionattributes through the_parseIntfunction with a default value (DEFAULT_MAG). - The serialization consistently uses
inMagfor new files. - The codebase has extensive test coverage for various magnification scenarios, including mag-related operations in volume tracing and data sampling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing NML files that might be affected by the resolution to mag rename
# Search for NML files containing resolution attributes
rg -l 'inMag|resolution' --type xml
# Check for potential parsing logic in other files
rg 'resolution.*=.*DEFAULT_RESOLUTION|resolution.*=.*DEFAULT_MAG' --type typescript
Length of output: 158
Script:
#!/bin/bash
# Let's try a more comprehensive search strategy
# Search for files containing resolution or mag related code
rg -t ts -t js 'resolution.*=|mag.*=|inMag|DEFAULT_RESOLUTION|DEFAULT_MAG' -A 2 -B 2
# Search for NML related files and their content
fd -e xml . | xargs cat
# Look for NML parsing/serialization logic
rg -t ts -t js 'parseNml|serializeNml|NmlParser|NmlSerializer' -A 2 -B 2
Length of output: 102796
Script:
#!/bin/bash
# Let's search for NML parsing/serialization logic and resolution/mag handling
ast-grep --pattern 'function parseNml($$$)' -l ts
ast-grep --pattern 'function serializeNml($$$)' -l ts
# Search for resolution/mag handling in NML files
rg -A 5 -B 5 'resolution.*=|inMag.*=' frontend/javascripts/oxalis/model/helpers/nml_helpers.ts
# Look for NML-related tests
rg -A 5 -B 5 'test.*nml.*' -g '*.spec.ts'
Length of output: 1676
Script:
#!/bin/bash
# Let's look at the actual NML parsing implementation to understand how it handles both resolution and mag
rg -A 10 -B 10 'inMag|resolution' frontend/javascripts/oxalis/model/helpers/nml_helpers.ts
# Also check for any migration or backward compatibility code
rg -A 5 -B 5 'migration|backward.*compatibility|legacy' frontend/javascripts/oxalis/model/helpers/nml_helpers.ts
# Look for any NML-related tests that specifically test resolution/mag handling
rg -A 5 -B 5 'test.*resolution|test.*mag' frontend/javascripts/test/
Length of output: 14581
frontend/javascripts/oxalis/model/sagas/mesh_saga.ts (3)
243-254: LGTM: Function signature changes are consistent.
The renaming from resolutionInfo to magInfo in function signatures is consistent with the PR's objective of standardizing terminology.
Also applies to: 276-292, 325-327, 445-446
243-243: LGTM: Implementation changes align with function signatures.
The changes to variable names and parameter references consistently use "mag" terminology.
Also applies to: 474-474, 520-520
1032-1040: Note: Intentional retention of resolutions.
Based on the provided learnings, certain usages of segmentationLayer.resolutions are intentionally retained and should not be changed to segmentationLayer.mags. This is one such case.
Note: This review comment supersedes the previous review comment that suggested changing this usage, as we now have explicit guidance from the learnings.
frontend/javascripts/oxalis/view/left-border-tabs/layer_settings_tab.tsx (4)
313-314: LGTM: Variable renaming in LayerInfoIconWithTooltip
The renaming from resolution to mag is consistent with the PR objectives and maintains the same functionality.
Also applies to: 321-321
1132-1134: LGTM: Variable renaming in getOptionalDownsampleVolumeIcon
The renaming from hasExtensiveResolutions to hasExtensiveMags is consistent with the PR objectives.
1623-1624: LGTM: Function renaming from resolution to mag
The function signature and parameter renaming from resolution to mag is consistent with the PR objectives.
313-314: Verify backend compatibility with the terminology changes
While the frontend changes from 'resolution' to 'mag' look consistent, let's verify that these changes don't affect backend communication, especially for:
- Layer information retrieval
- Volume magnification handling
- Zoom functionality
Also applies to: 1103-1121, 1623-1624
frontend/javascripts/test/snapshots/public-test/test-bundle/test/backend-snapshot-tests/annotations.e2e.js.md (3)
1523-1523: Consistent renaming from resolution to mag
The changes show consistent renaming from "resolution" to "mag" across all node objects in the test snapshots, with a consistent value of 2. This aligns with the PR objectives.
Also applies to: 1543-1543, 1563-1563, 1583-1583, 1603-1603, 1623-1623, 1684-1684, 1704-1704, 1724-1724, 1744-1744, 1764-1764, 1784-1784, 1845-1845, 1865-1865, 1885-1885, 1905-1905, 1925-1925, 1945-1945, 2006-2006, 2026-2026, 2046-2046, 2066-2066, 2086-2086, 2106-2106, 2167-2167, 2187-2187, 2207-2207, 2227-2227, 2247-2247, 2267-2267, 2356-2356, 2376-2376, 2396-2396, 2416-2416, 2436-2436, 2456-2456, 2517-2517, 2537-2537, 2557-2557, 2577-2577, 2597-2597, 2617-2617, 2678-2678, 2698-2698, 2718-2718, 2738-2738, 2758-2758, 2778-2778, 2839-2839, 2859-2859, 2879-2879, 2899-2899, 2919-2919, 2939-2939, 3018-3018, 3038-3038, 3058-3058, 3078-3078, 3098-3098, 3118-3118
Line range hint 1-3138: Comprehensive test coverage for the mag field
The test snapshots provide good coverage of the "mag" field across various scenarios including:
- Annotation information retrieval
- Annotation creation and updates
- Metadata operations
- Different node configurations
This helps ensure the field rename is properly tested across the application's functionality.
Line range hint 1-3138: Verify backend compatibility for the mag field
Given the concerns raised in the PR comments about backend compatibility, we should verify that these test snapshots reflect the correct interaction with the backend.
Let's check for any backend-related files that might need updates:
✅ Verification successful
Let's examine the backend files more specifically for mag/resolution handling in skeleton tracings:
Let's check the NodeDefaults class to understand how the mag field is handled in the backend:
Based on the evidence gathered from the codebase:
- The backend code in
SkeletonUpdateActions.scalashows thatresolutionis used as an alias formagwith the lineresolution getOrElse NodeDefaults.mag - The test snapshots show consistent usage of the
magfield in the node data structure - The backend routes and controllers handle
magparameters consistently - The codebase shows widespread use of
magrather thanresolutionacross models and controllers
Backend compatibility is maintained - The test snapshots correctly reflect the backend's handling of the mag field in skeleton tracings. The backend code explicitly maps any resolution field to mag using NodeDefaults, ensuring compatibility.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for backend files that might reference the resolution field
rg -l "resolution" --type-add 'backend:*.{scala,java}' -tbackend
# Search for API endpoint definitions that might need updates
rg -l "resolution.*=.*1" --type-add 'api:*.{scala,java,ts}' -tapi
Length of output: 2006
Script:
#!/bin/bash
# Check skeleton tracing related files for mag/resolution handling
rg -A 5 "mag|resolution" webknossos-tracingstore/app/com/scalableminds/webknossos/tracingstore/tracings/skeleton/
# Check API models/controllers for mag field usage
rg -A 5 "mag.*:" app/models/
rg -A 5 "mag.*:" app/controllers/
Length of output: 19526
Script:
#!/bin/bash
# Search for NodeDefaults class definition
rg -A 10 "object NodeDefaults" webknossos-tracingstore/
# Check for any mag/resolution field usage in protobuf definitions
rg -A 5 "mag.*=" webknossos-tracingstore/conf/
Length of output: 7302
frontend/javascripts/oxalis/api/api_latest.ts (7)
673-675: LGTM! Variable renaming is consistent.
The renaming from resolutions to magInfo aligns with the PR objectives and maintains consistent terminology.
679-682: LGTM! Good error handling.
The renaming is consistent, and the null check provides better error handling.
1844-1845: LGTM! Consistent variable renaming.
The renaming from resolutions to magInfo maintains consistency with the PR objectives.
1901-1913: LGTM! Consistent method renaming.
The renaming of methods and variables from resolution to mag terminology is consistent throughout this segment.
1947-1951: LGTM! Consistent variable and method renaming.
The renaming from resolution to mag terminology is consistent in the getViewportData method.
1962-1965: LGTM! Consistent variable renaming.
The renaming from resolution to mag terminology is consistent in this segment of the getViewportData method.
2108-2109: LGTM! Consistent variable renaming.
The renaming from resolution to mag terminology is consistent in the _getDownloadUrlForRawDataCuboid method.
| const volumeTargetMag = | ||
| fallbackLayerInfo != null | ||
| ? fallbackLayerInfo.resolutions | ||
| : // This is only a heuristic. At some point, user configuration | ||
| // might make sense here. | ||
| getWidestMags(this.props.dataset); | ||
|
|
||
| const getMaxDim = (resolution: Vector3) => Math.max(...resolution); | ||
| const getMaxDim = (mag: Vector3) => Math.max(...mag); | ||
|
|
||
| const volumeTracingResolutions = segmentationLayer.resolutions; | ||
| const volumeTracingMags = segmentationLayer.mags; | ||
|
|
||
| const sourceMag = _.minBy(volumeTracingResolutions, getMaxDim); | ||
| const sourceMag = _.minBy(volumeTracingMags, getMaxDim); | ||
| if (sourceMag === undefined) { | ||
| return []; | ||
| } | ||
|
|
||
| const possibleMags = volumeTargetResolutions.filter( | ||
| (resolution) => getMaxDim(resolution) >= getMaxDim(sourceMag), | ||
| ); | ||
| const possibleMags = volumeTargetMag.filter((mag) => getMaxDim(mag) >= getMaxDim(sourceMag)); | ||
|
|
||
| const magsToDownsample = _.differenceWith(possibleMags, volumeTracingResolutions, _.isEqual); | ||
| const magsToDownsample = _.differenceWith(possibleMags, volumeTracingMags, _.isEqual); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Inconsistent terminology: 'resolutions' is still used in fallbackLayerInfo
While most of the function has been updated to use 'mag' terminology, fallbackLayerInfo.resolutions still uses the old 'resolution' terminology. Consider updating this for consistency.
- const volumeTargetMag =
- fallbackLayerInfo != null
- ? fallbackLayerInfo.resolutions
- : getWidestMags(this.props.dataset);
+ const volumeTargetMag =
+ fallbackLayerInfo != null
+ ? fallbackLayerInfo.mags // Assuming the property has been renamed in the type definition
+ : getWidestMags(this.props.dataset);Committable suggestion skipped: line range outside the PR's diff.
| magInfoOfVisibleSegmentationLayer, | ||
| } = this.props; | ||
| const defaultOrHigherIndex = resolutionInfo.getIndexOrClosestHigherIndex( | ||
| const defaultOrHigherIndex = magInfoOfVisibleSegmentationLayer.getIndexOrClosestHigherIndex( | ||
| preferredQualityForMeshPrecomputation, | ||
| ); | ||
| const meshfileResolutionIndex = | ||
| const meshfileMagIndex = | ||
| defaultOrHigherIndex != null | ||
| ? defaultOrHigherIndex | ||
| : resolutionInfo.getClosestExistingIndex(preferredQualityForMeshPrecomputation); | ||
| const meshfileResolution = resolutionInfo.getMagByIndexWithFallback( | ||
| meshfileResolutionIndex, | ||
| : magInfoOfVisibleSegmentationLayer.getClosestExistingIndex( | ||
| preferredQualityForMeshPrecomputation, | ||
| ); | ||
| const meshfileMag = magInfoOfVisibleSegmentationLayer.getMagByIndexWithFallback( | ||
| meshfileMagIndex, |
There was a problem hiding this comment.
💡 Codebase verification
The renaming from resolution to mag is still in progress across the codebase
Based on the search results, while the changes in the reviewed file are correct, there are still many instances of "resolution" terminology being used throughout the codebase. Some key observations:
- Many files still use
resolutionsproperty in their types and interfaces - Several components still reference
resolutionin their logic - Some deprecated methods contain "resolution" in their names
This suggests that the terminology standardization effort is not complete, and there might be a need for:
- Consistent renaming across all frontend files
- Updating type definitions and interfaces
- Addressing deprecated methods
🔗 Analysis chain
LGTM! Variable and method name changes are consistent.
The renaming from resolutionInfo to magInfoOfVisibleSegmentationLayer and related method calls is consistent with the PR objectives to standardize terminology. The logic remains unchanged and properly maintains functionality.
Let's verify the consistency of the magnification terminology across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining instances of "resolution" that might need to be updated
rg -i "resolution(?!.*deprecated)" frontend/
Length of output: 286
Script:
#!/bin/bash
# Let's try a simpler approach to find resolution-related terms
rg -i "resolution" frontend/ --type ts --type tsx
Length of output: 77
Script:
#!/bin/bash
# Search for resolution-related terms without file type filtering
rg -i "resolution" frontend/javascripts/
Length of output: 11479
|
great clean up!
I had a quick look and since the changes only looked like simple renamings (as the PR also suggests ^^), it should be fine 👍 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts (1)
117-118: Consider adding a clarifying commentWhile the logic is correct, consider adding a comment explaining the purpose of the
areMagsDistinctcheck and its implications for the MagInfo creation.const magUnion = getMagnificationUnion(dataset); + // Check if each magnification level has only one unique magnification value const areMagsDistinct = magUnion.every((mags) => mags.length <= 1);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
CHANGELOG.unreleased.md(1 hunks)frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts(1 hunks)frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/javascripts/oxalis/view/right-border-tabs/dataset_info_tab_view.tsx
- CHANGELOG.unreleased.md
🔇 Additional comments (4)
frontend/javascripts/oxalis/model/accessors/dataset_accessor.ts (4)
82-91: LGTM: Variable renaming is consistent
The renaming from resolutionUnionDict to magUnionDict and the loop variable change align well with the PR's objective to standardize terminology.
96-98: LGTM: Object iteration maintains functionality
The object iteration logic remains sound while adopting the new terminology.
101-105: LGTM: Key processing logic is preserved
The array processing and sorting logic remains intact while using the renamed dictionary.
82-118: Verify consistent renaming across the codebase
Let's ensure the resolution to magnification renaming is consistent across all related files.
✅ Verification successful
Based on the results from both verification attempts, I can now provide a final response:
Resolution to magnification renaming is consistent
The codebase shows consistent usage of the new "mag" terminology in the reviewed file. The only remaining "resolution" terms are in schema definitions and type interfaces which is expected as they define the data structure contract. No instances of inconsistent usage were found in the implementation code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining "resolution" terms that might need to be updated to "mag"
# Look for resolution-related terms in TypeScript/JavaScript files
rg -t ts -t js 'resolution[^a-zA-Z]|resolutions[^a-zA-Z]' --glob '!**/deprecated/**'
# Look for specific resolution variable declarations
ast-grep --pattern 'const resolution = $_'
Length of output: 65751
Script:
#!/bin/bash
# Let's check for any remaining resolution-related terms in TypeScript files
# specifically in function/variable declarations and class properties
ast-grep --pattern 'resolution: $_'
# Also check for any resolution-related terms in function names
ast-grep --pattern 'function getResolution'
# Check for any resolution-related terms in interface/type definitions
ast-grep --pattern 'interface $_ { resolution: $_; }'
Length of output: 139
URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
in-depth testing
write tests (see slack conversation )
generate_dummy_trees)some notes:
_labelVoxelInResolution_DEPRECATED)Issues:
(Please delete unneeded items, merge only when none are left open)