fix(defaultRouteInit): pass sorted display sets to hanging protocol for deterministic viewport order

[pedrokohler]

Context

defaultRouteInit.applyHangingProtocol sorts display sets by modality priority and series number into sortedDisplaySets, but the original unsorted displaySets array was passed to hangingProtocolService.run(). Since displaySetService.getActiveDisplaySets() returns display sets in creation order -- which depends on asynchronous network response timing -- viewport ordering was non-deterministic across page loads.

This is a significant source of screenshot-based E2E test flakiness: when viewport positions shuffle between runs, full-page screenshot comparisons fail even though the clinical content is correct. Affected tests include multi-viewport layouts like RTDataOverlayForUnreferencedDisplaySetNoHydration, SEGDataOverlayForUnreferencedDisplaySetNoHydration, ContourSegmentToggleVisibility, ContourSegNavigation, Scoord3dProbe, and other hydration/MPR tests that compare 2x2 grids. These appear intermittently in CI (e.g. runs from Mar 25-27 with 5-7 failures each), pass on retry, and are caused by viewport shuffling rather than rendering bugs.

Beyond testing, deterministic order improves the end-user experience: reopening the same study always produces the same layout.

Changes & Results

• One-word change in platform/app/src/routes/Mode/defaultRouteInit.ts: pass sortedDisplaySets instead of displaySets to hangingProtocolService.run().
• Viewport ordering is now deterministic (sorted by modality priority, then series number) regardless of network timing.

Testing

• Unit tests: 846/846 pass.
• Playwright E2E: full suite ran with --update-snapshots -- regenerated screenshots are byte-identical to existing baselines, confirming the sort order matches what the committed baselines already expect.

Checklist

PR

• My Pull Request title is descriptive, accurate and follows the semantic-release format and guidelines.

Code

• My code has been well-documented (function documentation, inline comments, etc.)

Public Documentation Updates

• The documentation page has been updated as necessary for any public API additions or removals.

Tested Environment

• OS: macOS 15.4
• Node version: 20.9.0
• Browser: Chromium (Playwright headless)

Made with Cursor

Greptile Summary

This PR fixes a one-line bug in defaultRouteInit.applyHangingProtocol where the unsorted displaySets array was being passed to hangingProtocolService.run() even though a sorted copy (sortedDisplaySets) was already computed and used for getStudies(). Since displaySetService.getActiveDisplaySets() returns display sets in network-response order (non-deterministic), viewport layout could shuffle between page loads.

Key changes:

• platform/app/src/routes/Mode/defaultRouteInit.ts: Replace displaySets with sortedDisplaySets in the hangingProtocolService.run() call (line 52), making the argument consistent with the already-sorted input to getStudies() on line 45.

Impact:

• Viewport ordering is now fully deterministic (modality priority first, then series number), regardless of network timing.
• Resolves intermittent E2E screenshot test failures in multi-viewport layouts caused by viewport shuffling rather than rendering bugs.
• Improves end-user experience: reopening the same study always produces the same layout.

Assessment: The fix is minimal, correct, and self-contained. sortedDisplaySets is already constructed inside applyHangingProtocol via [...displaySets].sort(sortCriteria) — a non-mutating spread-then-sort — so no side effects are introduced. Both the hangingProtocolService's internal this.displaySets state and the protocolEngine.run() path now receive the sorted array consistently.

Confidence Score: 5/5

Safe to merge — single-line fix with no side effects, consistent with existing sort logic already applied to getStudies().

The change is a trivially correct one-liner: sortedDisplaySets was already being computed and used in the same function, the fix simply passes it consistently to hangingProtocolService.run(). No new logic, no new dependencies, no mutation of shared state. All 846 unit tests pass and E2E snapshots are byte-identical to baselines.

No files require special attention.

Important Files Changed

Filename	Overview
platform/app/src/routes/Mode/defaultRouteInit.ts	One-line fix: passes the already-computed sortedDisplaySets (sorted by modality priority then series number) to hangingProtocolService.run() instead of the original unsorted displaySets, making viewport ordering deterministic regardless of network response timing.

Sequence Diagram

sequenceDiagram
    participant DS as displaySetService
    participant DRI as applyHangingProtocol()
    participant HP as hangingProtocolService

    DS->>DRI: getActiveDisplaySets() → displaySets (network order)
    Note over DRI: [...displaySets].sort(seriesInfoSortingCriteria)<br/>→ sortedDisplaySets (modality priority + series number)
    DRI->>DRI: getStudies(studyInstanceUIDs, sortedDisplaySets)
    Note over DRI: Before fix: passed unsorted displaySets<br/>After fix: passes sortedDisplaySets
    DRI->>HP: run({ studies, activeStudy, displaySets: sortedDisplaySets }, hangingProtocolId, { stageIndex })
    HP->>HP: this.displaySets = sortedDisplaySets
    HP->>HP: _setProtocol → _updateViewports (deterministic layout)

Loading

_{Reviews (1): Last reviewed commit: "fix: pass sorted display sets to hanging..." | Re-trigger Greptile}

_{(3/5) Reply to the agent's comments like "Can you suggest a fix for this @greptileai?" or ask follow-up questions!}

[netlify]

✅ Deploy Preview for ohif-dev ready!

Name	Link
🔨 Latest commit	6ed7379
🔍 Latest deploy log	https://app.netlify.com/projects/ohif-dev/deploys/69ce914a55025d0009338b75
😎 Deploy Preview	https://deploy-preview-5933--ohif-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[jbocce]

Looks good to me. It kinda bothers me why we were not already doing this. @wayfarer3130 any thoughts?

[wayfarer3130]

Thank you - this has gotten tweaked several times, with the original doing a sort on the underlying series themselves as stored in the display set service.

[wayfarer3130]

I'd really love to have a unit test on this - not sure if that is realistic or not, but it has been broken so many times that I'd like to have some verification if possible.