Include More Tree Information in Skeleton CSV Export #9205
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThis PR introduces a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
knollengewaechs
force-pushed
the
export-trees-csv
branch
from
582b7fa to
14ad28c
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@unreleased_changes/9191.md`:
- Around line 1-3: The changelog is missing an entry for the new trees.csv
export; update the "### Added" section to either extend the existing
bounding-box bullet or add a separate bullet that clearly states the addition of
a trees.csv file to the skeleton CSV export (mention "trees.csv" and "skeleton
CSV export"), describe that it contains tree metadata, and note any usage or
enabling details needed so the PR's primary feature is documented.
🧹 Nitpick comments (4)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (1)
6-26: Color format in CSV may benefit from clarification or conversion.The header column is named
colorRGB, buttree.coloroutputs as a Vector3 with normalized values (e.g.,"0.5,0.2,0.8"). WhiletransformToCSVRowcorrectly quotes this, users might expect RGB values in 0-255 range or hex format.Consider either:
- Converting to 0-255 integers:
tree.color.map(c => Math.round(c * 255))- Renaming the header to
colorRGBNormalizedfor clarity🔧 Optional: Convert color to 0-255 integer format
const row = [ annotationId, tree.treeId, tree.name, tree.groupId, - tree.color, + tree.color.map((c) => Math.round(c * 255)), tree.nodes.size(), tree.edges.size(), ];frontend/javascripts/viewer/view/components/setting_input_views.tsx (1)
668-675: Consider aligning event handlers.The ellipsis container has both
onContextMenuandonClicktriggeringonOpenContextMenu. This provides flexibility for right-click and left-click access to the menu.However, note that Line 574 already sets
onContextMenuon the outerdiv, so right-clicking anywhere on the component already opens the context menu. The inneronContextMenuon Line 670 might be redundant (thoughstopPropagationisn't called, so it would bubble up anyway).frontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsx (2)
212-221: Consider proper pluralization for the delete label.When
selectedRowKeys.lengthis 1, the label reads "Delete 1 selected bounding boxes" (plural). Consider using proper singular/plural form.🔧 Suggested fix for pluralization
const multiSelectContextMenu = { items: [ { key: "delete-selected-bboxes", - label: `Delete ${selectedRowKeys.length} selected bounding boxes`, + label: `Delete ${selectedRowKeys.length} selected bounding box${selectedRowKeys.length === 1 ? "" : "es"}`, disabled: !allowUpdate, onClick: deleteSelectedBoundingBoxes, }, ], };
264-276: Consider clearing selection on normal click for better UX.When a user clicks a row without modifier keys, the bounding box becomes active and the view navigates to it, but
selectedRowKeysremains unchanged. This means if the user previously multi-selected 3 items, the checkmarks remain visible even though the user seemingly "moved on" to a different item.Consider clearing the selection on normal click to provide clearer feedback:
🔧 Suggested improvement
onClick: (event) => { hideContextMenu(); if (event.shiftKey || event.ctrlKey || event.metaKey) { setSelectedRowKeys((prev) => prev.includes(bb.id) ? prev.filter((key) => key !== bb.id) : [...prev, bb.id], ); } else { + setSelectedRowKeys([]); handleGoToBoundingBox(bb.id); dispatch(setActiveUserBoundingBoxId(bb.id)); } },
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/javascripts/viewer/model/helpers/csv_helpers.tsfrontend/javascripts/viewer/view/components/setting_input_views.tsxfrontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsxfrontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxunreleased_changes/9191.md
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-12-11T15:25:53.526Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/statistic/time_tracking_overview.tsx:261-279
Timestamp: 2025-12-11T15:25:53.526Z
Learning: Ant Design v6 Select: when using the Select component, consider supplying a prefix prop to render an icon or element before the input for better visual context. Apply this guideline to TS and TSX files across the codebase where Ant Design Select is used; ensure prefix usage is accessible (e.g., provide meaningful aria-label if needed) and avoid unnecessary prefixes on simple inputs.
Applied to files:
frontend/javascripts/viewer/model/helpers/csv_helpers.tsfrontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsxfrontend/javascripts/viewer/view/components/setting_input_views.tsx
📚 Learning: 2025-12-11T15:33:06.880Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/admin/task/task_search_form.tsx:151-151
Timestamp: 2025-12-11T15:33:06.880Z
Learning: In Ant Design v6, do not use optionFilterProp as a standalone prop on Select. Instead, pass it inside showSearch as optionFilterProp, e.g. showSearch={{ optionFilterProp: 'label' }}. showSearch accepts boolean or an object with keys like optionFilterProp, filterOption, autoClearSearchValue. Update all Select components that use optionFilterProp to adopt the new pattern and adjust types accordingly to maintain compatibility and avoid deprecation warnings.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsxfrontend/javascripts/viewer/view/components/setting_input_views.tsx
📚 Learning: 2025-12-18T13:11:55.113Z
Learnt from: hotzenklotz
Repo: scalableminds/webknossos PR: 9156
File: frontend/javascripts/dashboard/dataset/dataset_settings_data_tab.tsx:10-10
Timestamp: 2025-12-18T13:11:55.113Z
Learning: Guideline: When using Ant Design's Flex component in this repository, the align prop accepts standard CSS flexbox values such as 'flex-start', 'flex-end', 'center', 'baseline', and 'stretch'. These map directly to the CSS align-items property. Do not propose or implement changes to use 'end'/'start' for Ant Design Flex components; continue using the standard CSS values (e.g., 'flex-start' and 'flex-end') to ensure correct alignment behavior.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsxfrontend/javascripts/viewer/view/components/setting_input_views.tsx
📚 Learning: 2025-12-11T15:54:47.778Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 9117
File: frontend/javascripts/dashboard/advanced_dataset/dataset_table.tsx:131-134
Timestamp: 2025-12-11T15:54:47.778Z
Learning: In Ant Design v6, Dropdown uses a flat classNames structure: classNames={{ root: '…' }}. Other components (Select, AutoComplete, Cascader, TreeSelect) use a nested structure. The deprecated overlayClassName prop for Dropdown should be replaced with classNames.root. In reviews, flag Dropdown usage that relies on overlayClassName and replace it with classNames={{ root: '…' }}. If you encounter related components, verify the correct classNames shape (flat for Dropdown, nested for others) and update accordingly. This guideline covers TSX files under the frontend codebase where Ant Design components are used.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsxfrontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsxfrontend/javascripts/viewer/view/components/setting_input_views.tsx
📚 Learning: 2025-05-15T19:44:16.110Z
Learnt from: knollengewaechs
Repo: scalableminds/webknossos PR: 8492
File: frontend/javascripts/viewer/model/sagas/skeletontracing_saga.ts:671-674
Timestamp: 2025-05-15T19:44:16.110Z
Learning: Visibility updates for user bounding boxes are intentionally handled separately from other property updates, with dedicated update actions for visibility changes.
Applied to files:
frontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsx
🧬 Code graph analysis (4)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (1)
frontend/javascripts/viewer/store.ts (1)
SkeletonTracing(164-174)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (3)
getTreesAsCSV(6-26)getTreeNodesAsCSV(28-73)getTreeEdgesAsCSV(75-88)
frontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsx (4)
frontend/javascripts/viewer/model/actions/annotation_actions.ts (1)
deleteUserBoundingBoxAction(206-210)frontend/javascripts/components/fast_tooltip.tsx (1)
FastTooltip(54-123)frontend/javascripts/viewer/view/context_menu.tsx (1)
getContextMenuPositionFromEvent(2015-2037)frontend/javascripts/viewer/model/actions/ui_actions.ts (1)
setActiveUserBoundingBoxId(279-284)
frontend/javascripts/viewer/view/components/setting_input_views.tsx (1)
frontend/javascripts/components/fast_tooltip.tsx (1)
FastTooltip(54-123)
🔇 Additional comments (9)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (2)
69-73: LGTM!The import is correctly updated to include
getTreesAsCSValongside the existing CSV helper functions.
581-591: LGTM!The CSV export flow is well-structured:
- Trees metadata is exported first (
trees.csv)- Followed by detailed node data (
nodes.csv)- And edge relationships (
edges.csv)The function calls use correct parameters and the ZIP file structure provides a logical organization.
frontend/javascripts/viewer/view/components/setting_input_views.tsx (3)
370-393: LGTM!The
bboxIdprop and updatedonOpenContextMenusignature properly propagate the bounding box ID for context-aware menu handling. This aligns well with the multi-select functionality in the parent component.
597-607: LGTM!The color picker column is nicely integrated with proper tooltip handling for the disabled state and uses
Flexfor centered alignment.
627-637: LGTM!The inline Delete button provides quick access to deletion and is correctly disabled with an appropriate tooltip when editing is not allowed.
frontend/javascripts/viewer/view/right-border-tabs/bounding_box_tab.tsx (4)
1-3: LGTM!Import changes are appropriate for the new Button-based UI and FastTooltip usage.
44-44: LGTM!The
selectedRowKeysstate correctly tracks multi-selected bounding box IDs as an array of numbers.
59-71: LGTM!Good implementation:
deleteBoundingBoxproperly removes the deleted ID fromselectedRowKeysdeleteSelectedBoundingBoxesiterates through all selected IDs and cleans up state afterward
172-177: LGTM!The UI update from an icon to a proper Button with FastTooltip improves accessibility and click target size.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Steps to test:
trees.csvIssues:
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)