fix: Add display set split/store logic to metadata#2738
Open
wayfarer3130 wants to merge 9 commits into
Open
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR introduces display-set metadata contracts for grouping DICOM series instances, implements split-rule-driven grouping, registers the metadata provider, adds viewport APIs for mounting display sets, builds demo helpers for display-set selection, and migrates multiple core and tools examples to use the new display-set-based flow alongside dynamic viewport type selection. ChangesDisplay set metadata, viewport APIs, and example migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
wayfarer3130
force-pushed
the
fix/dwi-wrong-wl-split
branch
from
0c48d0c to
5cfd79a
Compare
wayfarer3130
changed the title
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (5)
packages/core/examples/displaySets/index.ts (1)
225-231: ⚡ Quick winValidate viewport type before casting.
Lines 227 and 230 cast
viewportto specific types without runtime validation. IfenableElementsilently failed or returned an unexpected viewport type, the casts would succeed but method calls could throw at runtime.♻️ Suggested defensive check
if (row.hint === 'volume3d') { // 3D needs a preset to be visible; setDisplaySets already loaded the volume. - (viewport as Types.IVolumeViewport).setProperties({ preset: 'CT-Bone' }); + if ('setProperties' in viewport) { + (viewport as Types.IVolumeViewport).setProperties({ preset: 'CT-Bone' }); + } } if (row.hint === 'video') { - (viewport as Types.IVideoViewport).play(); + if ('play' in viewport) { + (viewport as Types.IVideoViewport).play(); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/core/examples/displaySets/index.ts` around lines 225 - 231, The code casts viewport to Types.IVolumeViewport and Types.IVideoViewport and calls setProperties/play without runtime checks, which can throw if enableElement returned a different viewport; update the branches handling row.hint === 'volume3d' and 'video' to first validate the viewport type (e.g., check for presence of expected methods/properties such as setProperties for volume and play for video or use a type guard) before casting and calling setProperties or play on the viewport instance to safely bail or log an error if the viewport is not the expected type.packages/metadata/src/displayset/ImageStackDisplaySet.ts (1)
13-37: 💤 Low valueConsider refactoring to eliminate code duplication.
The loops in
collectUnderlyingImageIds(lines 15-19) andcollectImageIds(lines 28-32) perform identical operations: iterating instances and collectinginstance.imageIdvalues.collectImageIdscould delegate tocollectUnderlyingImageIdsand only handle the fallback logic.♻️ Proposed refactor
function collectImageIds( instances: NaturalizedInstance[], underlyingImageIds: string[] ): string[] { - const imageIds: string[] = []; - for (const instance of instances) { - if (instance.imageId) { - imageIds.push(instance.imageId); - } - } + const imageIds = collectUnderlyingImageIds(instances); if (imageIds.length === 0) { return [...underlyingImageIds]; } return imageIds; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/metadata/src/displayset/ImageStackDisplaySet.ts` around lines 13 - 37, Duplicate loops collectUnderlyingImageIds and collectImageIds both iterate instances to gather instance.imageId; refactor by making collectImageIds call collectUnderlyingImageIds(instances) to get imageIds, then if that result is empty return [...underlyingImageIds] else return the collected ids; update/remove the duplicated loop in collectImageIds and keep collectUnderlyingImageIds as the single source of truth (refer to functions collectUnderlyingImageIds and collectImageIds).packages/metadata/src/displayset/displaySetProvider.ts (1)
5-11: ⚡ Quick winAdd type annotations to provider function parameters.
The
displaySetAddProviderfunction has untyped parameters (next,_data,options). Adding type annotations would improve type safety and IDE autocomplete support.🔧 Suggested typing improvement
-function displaySetAddProvider(next, query: string, _data, options) { +function displaySetAddProvider( + next: (...args: unknown[]) => unknown, + query: string, + _data: unknown, + options?: { displaySet?: unknown } +) { const displaySet = options?.displaySet; if (displaySet) { return displaySet; } return next(query, _data, options); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/metadata/src/displayset/displaySetProvider.ts` around lines 5 - 11, The provider function displaySetAddProvider should have explicit TypeScript types added for its parameters: annotate next as a function type (e.g. (query: string, data?: unknown, options?: DisplayOptions) => DisplaySetType) so callers and returns are typed, type _data as unknown or a more specific payload type used by callers, and type options as an interface with an optional displaySet?: DisplaySetType (or use the existing DisplaySet/DisplayOptions types in the codebase). Update the function signature of displaySetAddProvider to use these types and import or declare DisplaySetType/DisplayOptions as needed so tooling and autocomplete pick up the shapes of next, _data, and options.packages/metadata/package.json (1)
1-1: Pipeline failures are unrelated CI configuration issues.The reported pipeline failures concern a pnpm version conflict between
packageManager(pnpm@11.5.2) and GitHub Action configuration (11.4.0). This is a CI/workflow configuration issue outside the scope of the code changes in this file. ThepackageManagerfield is not modified in this diff and likely resides in the repository rootpackage.jsonor workflow files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/metadata/package.json` at line 1, CI failures are caused by a pnpm version mismatch between the repository/packageManager field and the GitHub Actions workflow; do not change packages/metadata/package.json in this PR—either update the workflow action that installs pnpm to use pnpm@11.5.2 (matching the packageManager field) or update the root package.json packageManager field to the version used by the workflow so they align; look for the packageManager field in the root package.json and the pnpm install step in your GitHub Actions workflow to make the change.packages/metadata/src/displayset/buildSeriesInfo.ts (1)
16-19: ⚡ Quick winValidate
NumberOfFramesbefore numeric conversion.If
NumberOfFramescontains a malformed string,Number()returnsNaN, silently corrupting thenumberOfFramestotal. Consider usingparseInt()with validation or explicitly checking for valid numeric values.🛡️ Proposed validation
if (instance.NumberOfFrames) { - numberOfFrames += Number(instance.NumberOfFrames); + const frames = parseInt(String(instance.NumberOfFrames), 10); + if (!isNaN(frames)) { + numberOfFrames += frames; + } } else if (instance.Rows) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/metadata/src/displayset/buildSeriesInfo.ts` around lines 16 - 19, The code adds instance.NumberOfFrames to numberOfFrames without validating it, so malformed strings yield NaN; change the branch in buildSeriesInfo (the logic that reads instance.NumberOfFrames) to validate and parse the value first (e.g., use parseInt or Number and then test with Number.isFinite / Number.isInteger or isNaN) and only add the parsed numeric value to numberOfFrames when valid, otherwise treat it as zero or log/skip the invalid value; reference the variables instance.NumberOfFrames and numberOfFrames and the function/build block in buildSeriesInfo.ts when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 183: package.json declares "packageManager": "pnpm@11.5.2" but CI
workflows pin pnpm to 11.4.0 causing ERR_PNPM_BAD_PM_VERSION; update the pnpm
version pin in the referenced GitHub Actions workflows
(.github/workflows/validate-codemod-registry.yml, test.yml, playwright.yml,
format-check.yml, docusaurus-build.yml, build-docs.yml) and the PNPM_VERSION env
in ohif-downstream.yml to 11.5.2 (or remove explicit pnpm version pins so the
runner respects packageManager), or alternatively change the packageManager
value back to "pnpm@11.4.0" to keep CI consistent. Ensure you update all
occurrences of pnpm@11.4.0 and PNPM_VERSION to 11.5.2 (or remove pins) so pnpm
install --frozen-lockfile no longer errors.
In `@packages/core/examples/displaySets/index.ts`:
- Around line 180-193: The loop that builds the details rows uses line.innerHTML
= `<strong>${label}:</strong> ${value}` which injects untrusted DICOM metadata
into HTML; change this to create elements and set textContent to avoid HTML
parsing: for each (label, value) create a container div, create a <strong>
element and set its textContent to `${label}:`, append it to the container, then
append a text node or set the container.textContent (or append a text node for
value) using the string `value` (not innerHTML); also replace
statusLine.innerText with statusLine.textContent for consistency. Update the
code paths that reference row.detailsElement, the loop over lines, and the
statusLine creation to use textContent-safe methods.
In `@packages/core/src/RenderingEngine/BaseVolumeViewport.ts`:
- Around line 1511-1513: The code casts dataSet.volumeId to string without
checking type before calling resolveViewportVolumeId, which can hide non-string
values; update the logic in BaseVolumeViewport (around the
resolveViewportVolumeId call) to validate dataSet.volumeId is a string (e.g.,
typeof dataSet.volumeId === 'string' && dataSet.volumeId.length > 0) before
using it, otherwise fall back to entry.displaySetId; ensure the computed
volumeId passed into resolveViewportVolumeId is guaranteed to be a string or
null/undefined so downstream code can rely on its type.
- Around line 1515-1520: The volume is being created with
createAndCacheVolume(...) but volume.load() is not awaited, causing potential
race conditions when the viewport mounts; change the flow to await the load
(e.g., await volume.load()) before proceeding to any mounting or calling
setVolumes, and wrap the await in a try/catch to handle and log load errors so
failures don’t leave the cache in an inconsistent state (refer to
cache.getVolume, createAndCacheVolume, and volume.load).
In `@packages/core/src/RenderingEngine/StackViewport.ts`:
- Around line 1998-2004: The code uses dataSet.initialImageIdIndex directly and
can pass invalid values (NaN, negative, out-of-range, non-integer) into
setStack; before calling setStack, validate and clamp initialImageIdIndex: read
dataSet.initialImageIdIndex, coerce to a finite integer (e.g., via
Number.isFinite and Math.floor), default to 0 on invalid input, then clamp it to
the range 0 .. Math.max(0, dataSet.imageIds.length - 1); pass that sanitized
value to setStack instead of the raw value so setStack always receives a valid
index.
In `@packages/metadata/src/displayset/displayset.test.ts`:
- Around line 56-57: The test assertions are out of sync with the `volume3d`
rule in defaultDisplaySetSplitRules.ts which sets viewportTypes: ['volume',
'volume3d', 'stack'] (so the first element is 'volume'); update the test in
displayset.test.ts to assert expect(displaySet.viewportTypes[0]).toBe('volume')
and adjust expect(displaySet.preferredViewportType) to match the actual
preferredViewportType produced by the rule (or, if the intent was to prefer
'volume3d', change the rule in defaultDisplaySetSplitRules.ts where the
'volume3d' rule is defined so its viewportTypes order and preferredViewportType
reflect that intent).
In `@packages/metadata/src/displayset/ImageStackDisplaySet.ts`:
- Around line 85-102: ImageStackDisplaySet.fromImageIds currently drops
unresolved imageIds silently; modify it to surface missing-ID handling by either
routing through the existing resolveInstances utility or by adding
options.skipMissing?: boolean and options.onMissing?: (imageId: string) => void
to the fromImageIds signature, then use those options to call onMissing for each
undefined instance and throw or skip based on skipMissing before calling
ImageStackDisplaySet.fromInstances; reference the method name
ImageStackDisplaySet.fromImageIds, the helper resolveInstances, and the
downstream ImageStackDisplaySet.fromInstances so reviewers can locate the
change.
In `@packages/metadata/src/displayset/resolveInstances.ts`:
- Line 4: Update the doc comment for the skipMissing parameter in
resolveInstances (or the top-level comment in resolveInstances.ts) to accurately
state that when skipMissing is true missing ids are skipped silently, and when
skipMissing is false the function throws an error (with optional warning
behavior removed), rather than "omitted with optional warn"; reference the
skipMissing parameter name and the resolveInstances function so readers can find
the behavior.
In `@packages/tools/examples/videoNavigation/index.ts`:
- Around line 259-261: Ensure you check that displaySet.instances exists and has
at least one element before using displaySet.instances[0].imageId; update the
code around the call to viewport.setDisplaySets to validate (e.g., if
(!displaySet || !Array.isArray(displaySet.instances) ||
displaySet.instances.length === 0) then handle the empty case or skip calling
viewport.setDisplaySets) so you never access instances[0] on an empty array and
include a clear fallback or error handling path.
In `@packages/tools/examples/videoSegmentation/index.ts`:
- Around line 193-204: The code assumes displaySet.instances has elements when
deriving videoId; update the block after createDisplaySets to validate that
displaySet.instances is an array with length > 0 (and that instances[0].imageId
exists) before accessing instances[0].imageId; if the check fails, throw a clear
Error (e.g., "No instances found in display set") or handle the empty case, and
only assign videoId once the guard passes—apply this change around the
displaySets/displaySet/videoId usage to prevent runtime crashes.
In `@packages/tools/examples/videoSplineROITools/index.ts`:
- Around line 286-290: The code assumes displaySet.instances has at least one
element before accessing instances[0]; update the logic around the displaySet
extraction to validate that displaySet.instances is an array with length > 0 (or
at least one element) before reading displaySet.instances[0].imageId and
assigning videoId; if empty, throw a clear Error (e.g., "No instances found in
display set") or handle the empty-case appropriately so videoId is not accessed
from an empty array. Ensure references to displaySet.instances and the videoId
assignment are updated accordingly.
In `@packages/tools/examples/videoTools/index.ts`:
- Around line 179-183: The code assumes displaySet.instances has elements before
accessing instances[0]; update the block that sets videoId to first verify
displaySet.instances is an array with length > 0 (e.g., check
Array.isArray(displaySet.instances) && displaySet.instances.length > 0) and
throw a clear Error if empty, then safely read displaySet.instances[0].imageId
to set videoId; reference the existing variables displaySets, displaySet, and
videoId when making this change.
---
Nitpick comments:
In `@packages/core/examples/displaySets/index.ts`:
- Around line 225-231: The code casts viewport to Types.IVolumeViewport and
Types.IVideoViewport and calls setProperties/play without runtime checks, which
can throw if enableElement returned a different viewport; update the branches
handling row.hint === 'volume3d' and 'video' to first validate the viewport type
(e.g., check for presence of expected methods/properties such as setProperties
for volume and play for video or use a type guard) before casting and calling
setProperties or play on the viewport instance to safely bail or log an error if
the viewport is not the expected type.
In `@packages/metadata/package.json`:
- Line 1: CI failures are caused by a pnpm version mismatch between the
repository/packageManager field and the GitHub Actions workflow; do not change
packages/metadata/package.json in this PR—either update the workflow action that
installs pnpm to use pnpm@11.5.2 (matching the packageManager field) or update
the root package.json packageManager field to the version used by the workflow
so they align; look for the packageManager field in the root package.json and
the pnpm install step in your GitHub Actions workflow to make the change.
In `@packages/metadata/src/displayset/buildSeriesInfo.ts`:
- Around line 16-19: The code adds instance.NumberOfFrames to numberOfFrames
without validating it, so malformed strings yield NaN; change the branch in
buildSeriesInfo (the logic that reads instance.NumberOfFrames) to validate and
parse the value first (e.g., use parseInt or Number and then test with
Number.isFinite / Number.isInteger or isNaN) and only add the parsed numeric
value to numberOfFrames when valid, otherwise treat it as zero or log/skip the
invalid value; reference the variables instance.NumberOfFrames and
numberOfFrames and the function/build block in buildSeriesInfo.ts when making
this change.
In `@packages/metadata/src/displayset/displaySetProvider.ts`:
- Around line 5-11: The provider function displaySetAddProvider should have
explicit TypeScript types added for its parameters: annotate next as a function
type (e.g. (query: string, data?: unknown, options?: DisplayOptions) =>
DisplaySetType) so callers and returns are typed, type _data as unknown or a
more specific payload type used by callers, and type options as an interface
with an optional displaySet?: DisplaySetType (or use the existing
DisplaySet/DisplayOptions types in the codebase). Update the function signature
of displaySetAddProvider to use these types and import or declare
DisplaySetType/DisplayOptions as needed so tooling and autocomplete pick up the
shapes of next, _data, and options.
In `@packages/metadata/src/displayset/ImageStackDisplaySet.ts`:
- Around line 13-37: Duplicate loops collectUnderlyingImageIds and
collectImageIds both iterate instances to gather instance.imageId; refactor by
making collectImageIds call collectUnderlyingImageIds(instances) to get
imageIds, then if that result is empty return [...underlyingImageIds] else
return the collected ids; update/remove the duplicated loop in collectImageIds
and keep collectUnderlyingImageIds as the single source of truth (refer to
functions collectUnderlyingImageIds and collectImageIds).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d98b3480-8b3c-4da7-a312-0acc2575fc1f
📒 Files selected for processing (52)
package.jsonpackages/core/examples/displaySets/index.tspackages/core/examples/dynamicVolume/index.tspackages/core/examples/video/index.tspackages/core/src/RenderingEngine/BaseVolumeViewport.tspackages/core/src/RenderingEngine/ECGViewport.tspackages/core/src/RenderingEngine/StackViewport.tspackages/core/src/RenderingEngine/VideoViewport.tspackages/core/src/RenderingEngine/Viewport.tspackages/core/src/RenderingEngine/WSIViewport.tspackages/docs/docs/concepts/cornerstone-metadata/index.mdpackages/metadata/jest.config.jspackages/metadata/package.jsonpackages/metadata/src/displayset/BaseDisplaySet.tspackages/metadata/src/displayset/IDisplaySet.tspackages/metadata/src/displayset/ImageStackDisplaySet.tspackages/metadata/src/displayset/buildSeriesInfo.tspackages/metadata/src/displayset/createDisplaySetFromGroup.tspackages/metadata/src/displayset/defaultDisplaySetSplitRules.tspackages/metadata/src/displayset/displaySetProvider.tspackages/metadata/src/displayset/displayset.test.tspackages/metadata/src/displayset/groupInstancesBySplitRules.tspackages/metadata/src/displayset/index.tspackages/metadata/src/displayset/isEcgInstance.tspackages/metadata/src/displayset/isImageSopClass.tspackages/metadata/src/displayset/isVideoInstance.tspackages/metadata/src/displayset/isWsiInstance.tspackages/metadata/src/displayset/registerDisplaySetMetadata.tspackages/metadata/src/displayset/resolveInstances.tspackages/metadata/src/displayset/splitSeriesInstanceGroupsFromImageIds.tspackages/metadata/src/displayset/types.tspackages/metadata/src/displayset/viewportTypes.tspackages/metadata/src/enums/MetadataModules.tspackages/metadata/src/index.tspackages/metadata/src/registerDefaultProviders.tspackages/metadata/src/types/MetadataModuleTypes.tspackages/metadata/src/types/index.tspackages/metadata/src/utilities/index.tspackages/tools/examples/dynamicCINETool/index.tspackages/tools/examples/generateImageFromTimeData/index.tspackages/tools/examples/localAdvanced/index.tspackages/tools/examples/videoColor/index.tspackages/tools/examples/videoContourSegmentation/index.tspackages/tools/examples/videoGroup/index.tspackages/tools/examples/videoNavigation/index.tspackages/tools/examples/videoRange/index.tspackages/tools/examples/videoSegmentation/index.tspackages/tools/examples/videoSplineROITools/index.tspackages/tools/examples/videoTools/index.tsutils/ExampleRunner/example-runner-cli.jsutils/demo/helpers/index.jsutils/demo/helpers/splitDisplaySetsFromImageIds.ts
| "not op_mini all" | ||
| ], | ||
| "packageManager": "pnpm@11.4.0" | ||
| "packageManager": "pnpm@11.5.2" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find GitHub Actions workflows specifying pnpm version 11.4.0
rg -n "version.*11\.4\.0" .github/workflows/Repository: cornerstonejs/cornerstone3D
Length of output: 460
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm package.json packageManager value
echo "== package.json around packageManager =="
nl -ba package.json | sed -n '170,200p'
# 2) Inspect pnpm version usage in the workflows flagged by the initial search
echo
echo "== Workflow snippets containing version: 11.4.0 =="
for f in \
.github/workflows/validate-codemod-registry.yml \
.github/workflows/test.yml \
.github/workflows/playwright.yml \
.github/workflows/docusaurus-build.yml \
.github/workflows/build-docs.yml \
.github/workflows/format-check.yml
do
echo "--- $f ---"
rg -n "version:\s*11\.4\.0" "$f"
# show a bit of surrounding context
line=$(rg -n "version:\s*11\.4\.0" "$f" | head -n1 | cut -d: -f1)
start=$((line-8)); end=$((line+8))
nl -ba "$f" | sed -n "${start},${end}p"
doneRepository: cornerstonejs/cornerstone3D
Length of output: 173
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json around packageManager =="
cat -n package.json | sed -n '170,200p'
echo
echo "== Workflow snippets containing version: 11.4.0 =="
for f in \
.github/workflows/validate-codemod-registry.yml \
.github/workflows/test.yml \
.github/workflows/playwright.yml \
.github/workflows/docusaurus-build.yml \
.github/workflows/build-docs.yml \
.github/workflows/format-check.yml
do
echo "--- $f ---"
rg -n "version:\s*11\.4\.0" "$f" || true
line=$(rg -n "version:\s*11\.4\.0" "$f" | head -n1 | cut -d: -f1 || true)
if [[ -n "${line:-}" ]]; then
start=$((line-8)); end=$((line+8))
cat -n "$f" | sed -n "${start},${end}p"
fi
doneRepository: cornerstonejs/cornerstone3D
Length of output: 4060
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in \
.github/workflows/validate-codemod-registry.yml \
.github/workflows/test.yml \
.github/workflows/playwright.yml \
.github/workflows/docusaurus-build.yml \
.github/workflows/build-docs.yml \
.github/workflows/format-check.yml
do
echo "===== $f ====="
# show workflow name and top-level jobs block headers + any mention of OHIF / Docusaurus / Format Check
rg -n '^(name:|jobs:)' "$f" || true
rg -n '(OHIF|Docusaurus|format check|Format Check|downstream)' "$f" || true
echo
doneRepository: cornerstonejs/cornerstone3D
Length of output: 711
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== All occurrences of 11.4.0 in workflows =="
rg -n "11\.4\.0" .github/workflows/ || true
echo
echo "== OHIF / downstream mentions in workflows =="
rg -n "(OHIF|downstream validation|downstream)" .github/workflows/ || trueRepository: cornerstonejs/cornerstone3D
Length of output: 4877
🏁 Script executed:
#!/bin/bash
set -euo pipefail
cat -n .github/workflows/ohif-downstream.yml | sed -n '1,200p'Repository: cornerstonejs/cornerstone3D
Length of output: 6556
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== workflows using pnpm/action-setup =="
rg -n "pnpm/action-setup@" .github/workflows/
echo
echo "== pnpm versions pinned in workflows =="
rg -n "version:\s*11\.[0-9]+\.[0-9]+" .github/workflows/ || true
echo
echo "== explicit PNPM_VERSION env values =="
rg -n "PNPM_VERSION:\s*11\.[0-9]+\.[0-9]+" .github/workflows/ || trueRepository: cornerstonejs/cornerstone3D
Length of output: 1172
Fix CI pnpm version mismatch (packageManager pnpm@11.5.2 vs workflows pnpm@11.4.0)
package.json sets "packageManager": "pnpm@11.5.2", but GitHub Actions pins pnpm to 11.4.0 via pnpm/action-setup (and ohif-downstream.yml sets PNPM_VERSION: 11.4.0). These workflows run pnpm install --frozen-lockfile, so pnpm will fail with ERR_PNPM_BAD_PM_VERSION, blocking CI.
Update the pinned pnpm versions in:
.github/workflows/validate-codemod-registry.yml.github/workflows/test.yml.github/workflows/playwright.yml.github/workflows/format-check.yml.github/workflows/docusaurus-build.yml.github/workflows/build-docs.yml.github/workflows/ohif-downstream.yml(env.PNPM_VERSION)
Set them to 11.5.2 (or remove the explicit version pin to respect packageManager), or revert package.json back to pnpm@11.4.0.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@package.json` at line 183, package.json declares "packageManager":
"pnpm@11.5.2" but CI workflows pin pnpm to 11.4.0 causing
ERR_PNPM_BAD_PM_VERSION; update the pnpm version pin in the referenced GitHub
Actions workflows (.github/workflows/validate-codemod-registry.yml, test.yml,
playwright.yml, format-check.yml, docusaurus-build.yml, build-docs.yml) and the
PNPM_VERSION env in ohif-downstream.yml to 11.5.2 (or remove explicit pnpm
version pins so the runner respects packageManager), or alternatively change the
packageManager value back to "pnpm@11.4.0" to keep CI consistent. Ensure you
update all occurrences of pnpm@11.4.0 and PNPM_VERSION to 11.5.2 (or remove
pins) so pnpm install --frozen-lockfile no longer errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
OHIF_REF: ohifohifupdateplaywightimgaes
Context
Changes & Results
Testing
Checklist
PR
semantic-release format and guidelines.
Code
etc.)
Public Documentation Updates
additions or removals.
Tested Environment
Summary by CodeRabbit