Export skeleton trees as CSV #8898
Conversation
📝 WalkthroughWalkthroughAdds a new CSV helper module, migrates CSV serialization/saving to it, updates several CSV export call sites to use the helper, enables downloading skeleton tree nodes and edges as CSV (packaged into a ZIP) from the Skeleton tab, removes the CSV row helper from utils, and updates docs and release notes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (1)
32-34: Fix CSV header typo: segmendId → segmentIdUser-facing column name.
- `segmendId,segmentName,groupId,groupName,volumeInVoxel,volumeIn${dataSourceUnit}3,boundingBoxTopLeftPositionX,boundingBoxTopLeftPositionY,boundingBoxTopLeftPositionZ,boundingBoxSizeX,boundingBoxSizeY,boundingBoxSizeZ`; + `segmentId,segmentName,groupId,groupName,volumeInVoxel,volumeIn${dataSourceUnit}3,boundingBoxTopLeftPositionX,boundingBoxTopLeftPositionY,boundingBoxTopLeftPositionZ,boundingBoxSizeX,boundingBoxSizeY,boundingBoxSizeZ`;frontend/javascripts/admin/statistic/time_tracking_overview.tsx (2)
115-118: Guard is ineffective; can still crash on null listUse a proper null/empty check before mapping.
- if (filteredTimeEntries?.length === null) { + if (filteredTimeEntries == null || filteredTimeEntries.length === 0) { return; }
48-51: Fix state initialization for selectedProjectIdsThis currently passes a type in value position; it won’t compile.
- const [selectedProjectIds, setSelectedProjectIds] = useState(Array<string>); + const [selectedProjectIds, setSelectedProjectIds] = useState<Array<string>>([]);
🧹 Nitpick comments (5)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (2)
3-3: Use shared CSV saver and drop direct file-saver importRoute CSV saving through csv_helpers for consistency and less duplication.
-import saveAs from "file-saver"; ... -import { transformToCSVRow } from "viewer/model/helpers/csv_helpers"; +import { transformToCSVRow, saveAsCSV } from "viewer/model/helpers/csv_helpers";Also applies to: 16-16
62-99: Refactor exportStatisticsToCSV to use saveAsCSVSimplifies logic and aligns with other exports.
- const segmentStatisticsAsString = segmentInformation - .map((row) => { - const maybeAdditionalCoords = hasAdditionalCoords ? [row.additionalCoordinates] : []; - return transformToCSVRow([ - ...maybeAdditionalCoords, - row.segmentId, - row.segmentName, - row.groupId, - row.groupName, - row.volumeInVoxel, - row.volumeInUnit3, - ...row.boundingBoxTopLeft, - ...row.boundingBoxPosition, - ]); - }) - .join("\n"); + const csvLines = segmentInformation.map((row) => { + const maybeAdditionalCoords = hasAdditionalCoords ? [row.additionalCoordinates] : []; + return transformToCSVRow([ + ...maybeAdditionalCoords, + row.segmentId, + row.segmentName, + row.groupId, + row.groupName, + row.volumeInVoxel, + row.volumeInUnit3, + ...row.boundingBoxTopLeft, + ...row.boundingBoxPosition, + ]); + }); ... - const csv = [csv_header, segmentStatisticsAsString].join("\n"); const filename = groupIdToExport === -1 ? `segmentStatistics_${tracingIdOrDatasetName}.csv` : `segmentStatistics_${tracingIdOrDatasetName}_group-${groupIdToExport}.csv`; - const blob = new Blob([csv], { - type: "text/plain;charset=utf-8", - }); - saveAs(blob, filename); + saveAsCSV([csv_header], csvLines, filename);frontend/javascripts/admin/statistic/time_tracking_overview.tsx (2)
89-114: Reuse saveAsCSV for spans exportRemoves manual Blob handling and keeps exports uniform.
- const timeEntriesAsString = timeSpans - .map((row) => { - return transformToCSVRow([ - row.userId, - row.userEmail, - row.datasetOrganization, - row.datasetName, - row.annotationId, - row.annotationState, - row.timeSpanCreated, - Math.ceil(row.timeSpanTimeMillis / 1000), - row.taskId, - row.projectName, - row.taskTypeId, - row.taskTypeSummary, - ]); - }) - .join("\n"); // rows starting on new lines - const csv = [TIMETRACKING_CSV_HEADER_SPANS, timeEntriesAsString].join("\n"); - const filename = `timetracking-user-export-${userId}.csv`; - const blob = new Blob([csv], { - type: "text/plain;charset=utf-8", - }); - saveAs(blob, filename); + const csvLines = timeSpans.map((row) => + transformToCSVRow([ + row.userId, + row.userEmail, + row.datasetOrganization, + row.datasetName, + row.annotationId, + row.annotationState, + row.timeSpanCreated, + Math.ceil(row.timeSpanTimeMillis / 1000), + row.taskId, + row.projectName, + row.taskTypeId, + row.taskTypeSummary, + ]), + ); + const filename = `timetracking-user-export-${userId}.csv`; + saveAsCSV(TIMETRACKING_CSV_HEADER_SPANS, csvLines, filename);
119-128: CSV per-user export looks good; minor rounding consistencyConsider using the same rounding mode as spans (ceil vs round) for consistency across exports.
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (1)
23-24: Remove redundant null check on stateState is non-null by type.
- const position = ( - applyTransform && state != null ? getNodePosition(node, state) : node.untransformedPosition + const position = ( + applyTransform ? getNodePosition(node, state) : node.untransformedPosition ).map(Math.floor);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
frontend/javascripts/admin/statistic/time_tracking_overview.tsx(2 hunks)frontend/javascripts/libs/utils.ts(0 hunks)frontend/javascripts/viewer/model/helpers/csv_helpers.ts(1 hunks)frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx(2 hunks)frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx(5 hunks)
💤 Files with no reviewable changes (1)
- frontend/javascripts/libs/utils.ts
🧰 Additional context used
🧬 Code graph analysis (2)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (3)
frontend/javascripts/viewer/store.ts (2)
WebknossosState(571-591)SkeletonTracing(162-172)frontend/javascripts/viewer/model/accessors/skeletontracing_accessor.ts (1)
getNodePosition(253-255)frontend/javascripts/viewer/model/accessors/flycam_accessor.ts (1)
getAdditionalCoordinatesAsString(288-298)
frontend/javascripts/admin/statistic/time_tracking_overview.tsx (1)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (2)
transformToCSVRow(68-74)saveAsCSV(76-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
🔇 Additional comments (7)
frontend/javascripts/admin/statistic/time_tracking_overview.tsx (1)
18-18: LGTM on moving CSV utilities to csv_helpersGood consolidation.
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (2)
76-82: LGTM on saveAsCSVSimple, reusable, and consistent with callers.
37-45: All referenced node fields exist on Node type
TheNodeinterface infrontend/javascripts/viewer/model/types/tree_types.tsdefinesuntransformedPosition,additionalCoordinates,rotation,radius,viewport,mag,bitDepth,interpolation, andtimestamp, matching the CSV helper’s usage.frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (4)
65-67: LGTM on CSV helper imports with aliasesClear intent at call sites.
783-795: LGTM: Clear NML menu labelsGood UX clarity.
796-809: LGTM: Added CSV export actions (with transformed variant)Matches new helpers and UX pattern.
1041-1041: LGTM: Wire annotationId into propsNeeded for edges CSV.
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (1)
31-33: Fix CSV header typo: “segmendId” → “segmentId”This is user-facing and will leak into exported files.
Apply this diff:
-const getSegmentStatisticsCSVHeader = (dataSourceUnit: string) => - `segmendId,segmentName,groupId,groupName,volumeInVoxel,volumeIn${dataSourceUnit}3,boundingBoxTopLeftPositionX,boundingBoxTopLeftPositionY,boundingBoxTopLeftPositionZ,boundingBoxSizeX,boundingBoxSizeY,boundingBoxSizeZ`; +const getSegmentStatisticsCSVHeader = (dataSourceUnit: string) => + `segmentId,segmentName,groupId,groupName,volumeInVoxel,volumeIn${dataSourceUnit}3,boundingBoxTopLeftPositionX,boundingBoxTopLeftPositionY,boundingBoxTopLeftPositionZ,boundingBoxSizeX,boundingBoxSizeY,boundingBoxSizeZ`;frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
884-888: Fix spinner title for CSV exportsWhen exporting CSV, the modal still says “Preparing NML”. Use a generic title.
Apply this diff:
- if (this.state.isDownloading) { - title = "Preparing NML"; + if (this.state.isDownloading) { + title = "Preparing Download";
♻️ Duplicate comments (1)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
562-591: Robust CSV export with proper error handling and spinner resetThe try/catch/finally addresses prior feedback; errors are surfaced via toast and the spinner always resets.
Two small improvements:
- Name the ZIP with context (helps users distinguish downloads).
- Optional: guard
annotationIdnullability if it can be undefined in some flows.Apply this diff for the filename:
await writer.close(); - saveAs(await blobWriter.getData(), "tree_export.zip"); + const fileName = `visible_trees_${annotationId ?? "export"}.zip`; + saveAs(await blobWriter.getData(), fileName);Is
annotationIdguaranteed to be set when exporting? If not, confirmexportEdgesAsCSVhandles a missing ID gracefully.
🧹 Nitpick comments (2)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (2)
68-81: Normalize nullable fields to empty cells in CSV
transformToCSVRowstringifies values;null/undefinedbecome the literal strings “null”/“undefined”. Prefer empty cells. Minimal fix here forgroupId.Apply this diff:
row.segmentId, row.segmentName, - row.groupId, + row.groupId ?? "", row.groupName,Option (global): make
transformToCSVRowmapnull/undefinedto""once, so all callers benefit.
83-91: Pass header as columns array tosaveAsCSV(align with its API)Currently a pre-joined header string is wrapped in an array. Passing columns keeps semantics consistent and avoids double-joining.
Apply this diff:
- const csv_header = hasAdditionalCoords - ? [ADDITIONAL_COORDS_COLUMN, getSegmentStatisticsCSVHeader(voxelSize.unit)].join(",") - : getSegmentStatisticsCSVHeader(voxelSize.unit); + const baseHeader = getSegmentStatisticsCSVHeader(voxelSize.unit).split(","); + const csvHeader = hasAdditionalCoords + ? [ADDITIONAL_COORDS_COLUMN, ...baseHeader] + : baseHeader; const filename = groupIdToExport === -1 ? `segmentStatistics_${tracingIdOrDatasetName}.csv` : `segmentStatistics_${tracingIdOrDatasetName}_group-${groupIdToExport}.csv`; - saveAsCSV([csv_header], segmentStatisticsAsRows, filename); + saveAsCSV(csvHeader, segmentStatisticsAsRows, filename);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/javascripts/admin/statistic/time_tracking_overview.tsx(2 hunks)frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx(3 hunks)frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/admin/statistic/time_tracking_overview.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (1)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (2)
transformToCSVRow(68-74)saveAsCSV(76-82)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
- GitHub Check: frontend-tests
🔇 Additional comments (7)
frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segment_statistics_modal.tsx (2)
6-6: LGTM: import source updated correctly
pluralizeremains in libs/utils while CSV helpers are pulled from the new module. Looks good.
15-15: Good move to central CSV helpersUsing
saveAsCSVandtransformToCSVRowfrom a single place reduces duplication.frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (5)
17-17: LGTM: use Zip wrapper for writingUsing
libs/zipjs_wrapperfor writing while keeping@zip.js/zip.jsfor reading ZIPs is fine.
64-67: LGTM: CSV helper aliasesClear aliases (
getTreesAsCSV/getTreeEdgesAsCSV) improve readability.
784-800: Menu copy reads well and is consistentRenaming to include “NML” removes ambiguity; transformed variants are properly tooltiped and gated.
801-815: CSV menu entries: good UX parity with NMLCSV options mirror NML (incl. transformed gating). Nicely done.
1047-1047: LGTM:annotationIdplumbed to propsRequired by edges CSV export; mapping looks correct.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
566-587: Great: robust try/catch/finally with user feedbackSpinner is reset on error and an error toast is shown. This addresses the earlier review note.
🧹 Nitpick comments (5)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (5)
559-588: Spinner title says “Preparing NML” during CSV exportWhile exporting CSV, the modal still shows “Preparing NML”. Make the title generic (“Preparing Download”) or set it conditionally.
Outside this block, adjust render():
// Lines ~881-885 if (this.state.isDownloading) { title = "Preparing Download"; } else if (this.state.isUploading) { title = "Importing NML"; }
559-588: Name ZIP after the annotation for consistencyUse the same base name as NML export to help users correlate files.
Apply within this block:
try { - const treesCsv = getTreeNodesAsCSV(Store.getState(), skeletonTracing, applyTransforms); - const edgesCsv = getTreeEdgesAsCSV(annotationId, skeletonTracing); + const state = Store.getState(); + const treesCsv = getTreeNodesAsCSV(state, skeletonTracing, applyTransforms); + const edgesCsv = getTreeEdgesAsCSV(annotationId, skeletonTracing); @@ - await writer.close(); - saveAs(await blobWriter.getData(), "tree_export.zip"); + await writer.close(); + const zipName = getNmlName(state).replace(/\.nml$/i, "_csv.zip"); + saveAs(await blobWriter.getData(), zipName);
559-588: Toast copy: be explicit that CSV export failedMinor wording improvement for clarity.
- } catch (e) { - Toast.error("Could not export trees. See the console for details."); + } catch (e) { + Toast.error("Failed to export CSV (nodes+edges). See console for details."); console.error(e);
559-588: Enable ZIP compression for large CSVs (if wrapper supports it)CSV compresses very well; adding deflate can significantly reduce download size.
If
zipjs_wrapperforwards options, consider:// either on writer construction const writer = new Zip.ZipWriter(blobWriter /*, { level: 9 }*/); // or per-entry await writer.add("nodes.csv", new Zip.TextReader(treesCsv) /*, { level: 9 }*/); await writer.add("edges.csv", new Zip.TextReader(edgesCsv) /*, { level: 9 }*/);Please confirm the wrapper’s API before applying.
17-17: Use single zip.js import from libs/zipjs_wrapper
Import BlobReader, BlobWriter, ZipReader, and Entry from "libs/zipjs_wrapper" in frontend/javascripts/viewer/view/right-border-tabs/trees_tab_view.tsx instead of mixing direct "@zip.js/zip.js" imports—this ensures the CSP-safe workerScripts configuration is applied project-wide and prevents duplicate code paths.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts(1 hunks)frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/viewer/model/helpers/csv_helpers.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts (2)
getTreeNodesAsCSV(6-52)getTreeEdgesAsCSV(54-67)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: frontend-tests
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
🔇 Additional comments (3)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (3)
64-64: CSV helpers import looks goodTypes and call sites below match the helper signatures.
785-797: Menu entries/readability: labels and gating look good
- Clear NML vs. CSV labels.
- Transformed variants correctly gated by
isSkeletonLayerTransformed.Also applies to: 798-804, 805-813
1043-1043: Prop wiring for annotationId looks correctMatches CSV helpers’ expectation for edges export context.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
docs/skeleton_annotation/import_export.md (5)
17-24: Clarify export contents, add missing “comments” column, and align terminology
- Add explicit mention that node comments are included (per PR TODO “export comments (done)”).
- Avoid implying that “rotation (x,y,z)” is a per-node attribute; if rotation refers to viewport/camera, move it under viewport. Please confirm.
- Tidy wording (bit depth vs. bitdepth), and make ND examples concrete (e.g., t, c).
- Make it clear that CSV export downloads a ZIP (nodes.csv + edges.csv).
Apply:
-The NML or CSV files will contain the following information: -- All nodes with their corresponding annotation and tree -- The node's position and rotation (x,y,z) - - For ND datasets: additional coordinates -- The magnification, interpolation, radius, bitdepth and viewport information of the node -- The time the node was added -- All edges with their corresponding tree, source and target node +Exports (NML or CSV) contain: +- All nodes with their corresponding annotation and tree. +- Node position (x, y, z). For nD datasets: additional coordinates (e.g., t, c). +- Node properties: radius, interpolation, magnification, and bit depth. +- Viewport information (e.g., center and rotation), if available. +- Timestamp when the node was added. +- Node comments. +- All edges with their corresponding tree, source node, and target node.
20-20: Fix markdown list indentation (MD007)Indent sub-items by 2 spaces, not 4, to satisfy markdownlint.
Minimal fix (if you keep a sub-bullet here):
- - For ND datasets: additional coordinates + - For nD datasets: additional coordinates
1-1: Update title to reflect CSV supportThe page title still suggests NML only.
-# Importing & Exporting Skeletons as NML Files +# Importing & Exporting Skeletons (NML/CSV)
12-16: Document the new CSV download flow (Skeleton tab → More → Download visible trees as CSV)Add a dedicated step for CSV export and mention the ZIP structure.
2. If you need more fine-grained control over which trees to download, use the `Download Selected Trees` option. From the `Skeleton` Tab, click on `More` and select `Download Selected Trees` from the menu. All visible trees (checkmark in front of the name) will be downloaded as an NML file. This is especially useful if you need to only download a single tree of an otherwise much larger annotation.  - +3. To export visible trees as CSV, open the `Skeleton` tab, click `More` → `Download visible trees as CSV`. The download is a ZIP containing `nodes.csv` and `edges.csv`.Please confirm the exact menu label (“Download visible trees as CSV”) and placement.
15-15: Remove duplicated imageThe image at Line 15 is identical to Line 13.
-
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
docs/data/export_ui.md(1 hunks)docs/skeleton_annotation/import_export.md(1 hunks)frontend/javascripts/viewer/model/helpers/csv_helpers.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/data/export_ui.md
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/viewer/model/helpers/csv_helpers.ts
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/skeleton_annotation/import_export.md
20-20: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build-smoketest-push
- GitHub Check: backend-tests
- GitHub Check: frontend-tests
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
539-559: Wrap NML export in try/catch/finally to avoid stuck spinnerIf serializeToNml or saveAs throws, isDownloadingNML remains true and the modal never closes. Align with the CSV flow.
this.setState({ - isDownloadingNML: true, + isDownloadingNML: true, }); - // Wait 1 second for the Modal to render - const [buildInfo] = await Promise.all([getBuildInfo(), Utils.sleep(1000)]); - const state = Store.getState(); - const nml = serializeToNml( - state, - state.annotation, - skeletonTracing, - buildInfo, - applyTransforms, - ); - this.setState({ - isDownloadingNML: false, - }); - const blob = new Blob([nml], { - type: "text/plain;charset=utf-8", - }); - saveAs(blob, getNmlName(state)); + try { + // Wait 1 second for the Modal to render + const [buildInfo] = await Promise.all([getBuildInfo(), Utils.sleep(1000)]); + const state = Store.getState(); + const nml = serializeToNml( + state, + state.annotation, + skeletonTracing, + buildInfo, + applyTransforms, + ); + const blob = new Blob([nml], { type: "text/plain;charset=utf-8" }); + saveAs(blob, getNmlName(state)); + } catch (e) { + Toast.error("Could not export NML. See the console for details."); + // @ts-ignore + console.error(e); + } finally { + this.setState({ isDownloadingNML: false }); + }
♻️ Duplicate comments (1)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (1)
883-887: Addressed past feedback: shows “Preparing CSV”This resolves the earlier review about the spinner text.
🧹 Nitpick comments (3)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (3)
561-571: Optional: short-circuit when no visible treesAvoid producing header-only CSVs by checking visibility up-front.
if (!skeletonTracing) { return; } this.setState({ isDownloadingCSV: true, }); try { + const hasVisible = skeletonTracing + .trees + .values() + .filter((t) => t.isVisible) + .toArray().length > 0; + if (!hasVisible) { + Toast.info("No visible trees to export."); + return; + }
787-789: Nit: keep label order consistentNML entries use “Download Visible Trees NML”, while CSV transformed entry below uses “Download Visible Trees (Transformed) CSV”. Consider “Download Visible Trees CSV” and “Download Visible Trees CSV (Transformed)” for symmetry.
800-815: CSV actions look good; consider disabling when nothing is exportableGreat addition. Optionally set disabled when there are no visible trees to preempt a no-op.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
frontend/javascripts/viewer/model/helpers/csv_helpers.ts(1 hunks)frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx(11 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- frontend/javascripts/viewer/model/helpers/csv_helpers.ts
🧰 Additional context used
🧬 Code graph analysis (1)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (2)
frontend/javascripts/viewer/model/helpers/nml_helpers.ts (1)
getNmlName(116-129)frontend/javascripts/viewer/model/helpers/csv_helpers.ts (2)
getTreeNodesAsCSV(6-51)getTreeEdgesAsCSV(53-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: backend-tests
- GitHub Check: build-smoketest-push
- GitHub Check: frontend-tests
🔇 Additional comments (8)
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx (8)
17-17: Verify mixed ZipJS imports are intentionalYou import read APIs from @zip.js/zip.js and write APIs from libs/zipjs_wrapper. If this is to work around bundling/typing quirks, add a short comment to prevent “cleanup refactors” later. Otherwise, consider consolidating to one source to avoid duplicate code paths.
64-65: Good: centralizing CSV logic via helpersUsing getTreeNodesAsCSV/getTreeEdgesAsCSV keeps the UI lean and reuses a single CSV contract.
106-108: State flags look fineSeparate flags for NML/CSV keep the modal title accurate.
323-325: Constructor defaults are correctBoth download flags initialized to false.
795-799: LGTM: transformed NML entryLabel and tooltip read clearly.
897-897: LGTM: unified downloading gateThe modal now opens for either NML or CSV.
906-906: LGTM: modal open conditionTied to download/upload flags as intended.
1048-1049: Prop addition is correctannotationId wired into props for edges CSV; matches helper signature.
frontend/javascripts/viewer/view/right-border-tabs/trees_tab/skeleton_tab_view.tsx
Show resolved
Hide resolved
|
Awesome, thanks for applying the changes. Retesting went well 👍 |
2 participants
URL of deployed dev instance (used for testing):
Steps to test:
TODOs:
Issues:
Context
https://scm.slack.com/archives/C5AKLAV0B/p1757060937665459
(Please delete unneeded items, merge only when none are left open)
$PR_NUMBER.mdfile inunreleased_changesor use./tools/create-changelog-entry.py)