POC: bring viv-dicomweb-test work into SLIM and add basic bulk annotations support - #381
POC: bring viv-dicomweb-test work into SLIM and add basic bulk annotations support #381igoroctaviano wants to merge 55 commits into
Conversation
 |
|
Overall Grade |
Security Reliability Complexity Hygiene  |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
Jul 31, 2026 9:21p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
|
@igoroctaviano it would be helpful to indicate somewhere (in the "info" panel perhaps?) whether given instance is using viv or not. I deployed the viewer with |
PR description outdated. Just use the viv route before /studies |
|
|
Here's a quick dashboard to click through studies that have ANN series: https://datastudio.google.com/reporting/83a69414-337a-4ef4-ad54-4fc22b53f843/page/p1jyF/edit |
Resolve pnpm migration conflicts: drop bun.lock, move overrides to pnpm.overrides (keeping deck.gl pins from viv-loader), regenerate lockfile.
|
˜:wq .com:ImagingDataCommons/slim into feat/viv-loader
Bring in merged PR #395 (Slim logger startup config, Button typing, react-antd-icons types) and drop dmv.setLogLevel in favor of logger.configure.
|
Visit the preview URL for this PR (updated for commit f79338d): https://idc-external-006--pr381-feat-viv-loader-pd36u319.web.app (expires Tue, 21 Jul 2026 19:10:07 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 88aacecd98ba54d2f9c8d201a9444e43d1ad8307 |
Fix correctness, concurrency, and structural issues found in the feat/viv-loader review: - Infer OF (not OL) for PointCoordinatesData when DICOM JSON omits VR; harden Range streaming against ignored Range headers / 416 EOF; stop silently completing truncated downloads; remove POLYLINE off-by-one phantom vertices; keep finite zero coordinates as valid. - Deduplicate prefix emission into bulkPrefixEmitter and coordinate transform/validity helpers into annotationCoords (shared with the center-out worker kernel). Transfer only first-vertex XY to the worker. - Make DicomLoader ICC sync single-flight and dispose-guarded; promise- memoize viewer creation; LRU + generation-guard the tile cache; skip out-of-range synthetic dyadic tiles; suppress only bridged tile aborts. - Extract bulk-annotation orchestration into useVivBulkAnnotations with per-group hydrate generations, error-phase reporting, constant scatter accessors, and binary streaming centroid data; shrink VivSlideViewport. - Apply persisted ICC preference on classic slide switch; reset viv annotation panel state on series change; restore sidebar contrast under disabled antd menu items; purify preference reads and share one storage listener; fix craco @vivjs include and TypedArray typings; tune the XHR retry hook (idempotent methods only, shorter backoff).
Include 502/503/504 in the default retryable status list and correct the open() comment, matching the follow-up on fix/code-review-master-fixes.
Track in-flight hydrates as uid→dispatchGen ownership so a hydrate that settles after a fast hide→show can no longer delete the newer hydrate's marker, clear its streamed artifacts, or spawn a concurrent duplicate. Also: clear pending done/error timers on fresh progress reports; epoch- guard the tile-throttle batch counter across series resets; invalidate graphic caches when the catalog effect restarts; defer legacy LOD threshold key deletion until the first preference write.
Validate buffered Range-ignored payloads against graphicIndex (same truncation throw as the streaming routes), cap mid-stream HTTP 200 restarts, and match Range EOF only via status 416 or explicit wording. Widen fetchGraphicIndex to Int32Array|Uint32Array and normalize at the call site. Terminate the center-out worker with a sentinel error that skips the sync mega-sort fallback. Replace the Boolean(aborted) lint hit with a helper that re-reads AbortSignal after await.
Add a Cursor rule and CONTRIBUTING note that explanatory comments use /** */ rather than // (tooling directives excepted). Convert explanatory comments in the Viv bulk-annotation path and the retry hook to that style.
open() does not clear responseType; we re-apply it after retry open() for safety. Align the JSDoc with that.
When dicomweb-client attaches an HTTP status, only retry 429/5xx so 416 EOF and rejected 206 no longer burn multi-second backoff. After a partial stream paint fails, own the error instead of monolithic re-emit that duplicates layer ids.
Catalog effect cleanup previously bumped gens without clearing the in-flight Map, so a stale settle could reinstall graphicCache after invalidate. Also restore empty-response as Range EOF and JSDoc the fetchGraphicIndex OL note.
|
2 participants
Summary
POC port of viv-dicomweb-test into SLIM: Viv + Deck.gl slide viewing on dedicated
/viv/...routes plus bulk simple annotations (polygons) wired through the same DICOMweb + DMV decode path as OpenLayers.What changed (vs
master)Verified against
feat/viv-loader↔masterin the Slim repo (18 files, ~4.3k insertions).Routing: New React Router paths that mirror existing case URLs but prefix
/viv:/viv/studies/:studyInstanceUID/*and the GCP Healthcare store variant.ParametrizedCaseViewerpassesisVivRouteand optionalvivSettingsfrom config when the pathname starts with/viv/.src/viv/module:dicomLoader.ts(DICOMweb tiling + DMV integration),VivSlideViewport.tsx,loadBulkAnnotationLayers.ts,vivDisplayDefaults.ts,README.md. Includes request abort/cancel handling and viewport work (incl. 8-bit SM tile positioning).UI integration:
CaseViewer.tsxswitches the slide viewport toVivSlideViewporton Viv routes, addsVivSettingsDrawerand an annotation rail wired toAnnotationGroupList, and adjusts study URLs so navigation stays under/viv/….Dependencies:
@deck.gl/*,@vivjs/layers,@vivjs/loaders,@luma.gl/*;dicomweb-clientraised to^0.11.2(plustypes/dicomweb-client). Overrides pin Deck versions;bun.lockupdated.Build:
craco.config.js— Babel transpilation fornode_modulesunder@deck.gl,@luma.gl,@math.gl,@probe.gl,@hms-dbmi/viv.DICOM web client hook:
DicomWebManager.applyToPrimaryDicomwebClientso Viv tiles can attach to the primary store’srequestHooks(same XHR pipeline asVolumeImageViewer).Config types:
AppConfig.d.ts— optionalvivSettings(selections,channelsVisible,contrastLimits,colors,initialViewState). There is nouseVivflag; Viv is strictly/viv/...URLs.ICC profiles:
iccProfilesPreference.tsand updates inSlideViewer.tsxso ICC enable/disable persists (fixes ICC behavior with the Viv path).Types:
types/dicom-microscopy-viewer/index.d.tsextended for DMV APIs the Viv loader uses.App.tsx: registers the Viv routes.Branch & local setup
feat/viv-loader.dicom-microscopy-viewercheckout with Viv-facing API changes until a matching release is published. From DMV (correct branch):bun link. From the Slim checkout:bun link dicom-microscopy-viewer, thenbun installif needed.bun unlink dicom-microscopy-viewerin Slim restores registry installs. Details:src/viv/README.md.How to test
Add this server URL:
https://proxy.imaging.datacommons.cancer.gov/current/viewer-only-no-downloads-see-tinyurl-dot-com-slash-3j3d9jyp/dicomWebOpen the Viv-prefixed study URL (same study/series as before,
/vivafter the origin):http://localhost:3000/viv/studies/2.25.106918873973188798943205935727506273925/series/1.3.6.1.4.1.5962.99.1.1088146757.1503397867.1637470719301.2.0(Optional) In
public/config/*.js, setvivSettingsfor channel/contrast/colors/initialViewStateon/viv/....Toggle annotation group visibility. Expect a long first pass while every polygon is built (~23 s in one run); after that, pan/zoom stays responsive and memory behaved better than earlier OpenLayers attempts for this POC.
Possible TODO's
src/viv/: split network/DICOMweb, decode (DMV), and Deck.gl layer build sodicomLoader/ viewport files do not become one mega-module; document slide space → Deck props for the next contributor.SlideViewer/VolumeImageViewer; avoid duplicated fetch/decode when DMV already exposes primitives.bun link, and note which DMV version this tracks./vivyet — prioritize or gate behind flags (see README limitations).vivSettingssnippet topublic/configexample docs (paths are/viv/..., not a boolean config switch).src/viv/README.mdlimitations withdicomLoader(8-/16-bit handling and mixed-bit-depth restriction per series).