fix(seg): try bulk data fetch for PerFrameFunctionalGroupsSequence before full Part 10 fallback

.agents/skills/ohif-react-compiler/SKILL.md

@@ -0,0 +1,122 @@
+---
+name: ohif-react-compiler
+description: Rules and workflow for editing React code in the OHIF Viewer, which runs React 19 with the React Compiler enabled and two CI gates that fail when a file stops compiling or lint counts move. Use this skill whenever you create or change anything under platform/*/src, extensions/*/src or modes/*/src that contains JSX or a React hook — including small edits, refactors, and requests like "add a useCallback here" — even if the user does not mention React 19, the compiler, or memoization.
+---
+
+# OHIF React Compiler
+
+This repo compiles its React with the React Compiler. Most of what that means
+is enforced by tooling; this skill tells you how to work with that tooling and
+records the decisions already made so you do not relitigate them.
+
+## How this repo is set up
+
+- React 19. `ref` is an ordinary prop. There is no `forwardRef`, no
+  `propTypes`, and manual memoization is unnecessary.
+- Which directories the compiler applies to is defined once, in
+  `react-compiler.scope.cjs` at the repo root. Read it rather than assuming; the
+  build, the lint rules and the coverage gate all take their scope from it, and
+  adding a directory to its `excluded` list takes that directory out of all
+  three together. Files carrying a `'use no memo'` directive at the top are
+  skipped regardless of directory.
+- Two CI gates hold the current state and fail in **both** directions — when
+  things get worse, and when things get better but the budget was not tightened.
+
+| gate              | command                         | budget file                        |
+| ----------------- | ------------------------------- | ---------------------------------- |
+| compiler coverage | `pnpm run compiler:coverage:ci` | `.react-compiler-budget.json`      |
+| compiler lint     | `pnpm run lint:compiler:ci`     | `.react-compiler-lint-budget.json` |
+
+## The loop — after every React edit
+
+1. Lint the file you touched:
+   `npx eslint --config eslint.config.mjs --no-config-lookup <file>`
+2. `pnpm run compiler:coverage:ci` — did any file start refusing or opt out?
+3. `pnpm run lint:compiler:ci` — did the error or warning counts move?
+4. If either gate says the counts **improved**, tighten the budget to the exact
+   values it prints, in the same commit. Never loosen a budget to make CI pass.
+
+Step 1 is fast and catches most problems. Steps 2 and 3 are what CI runs.
+
+## Hard rules — CI enforces these
+
+- **No `forwardRef`.** Accept `ref` as a regular prop. An ESLint
+  `no-restricted-syntax` rule fails on it.
+- **No `prop-types`.** Use TypeScript types. An ESLint `no-restricted-imports`
+  rule fails on the import.
+- **No `eslint-disable` on any `react-hooks/*` rule.** The compiler treats a
+  suppression as a `Suppression` bailout and refuses the whole function. Fix the
+  code instead.
+- **Do not mutate props, and do not read or write `ref.current` during render.**
+  Both are refusals (`Immutability`, `Refs`). Compute into a local; move ref
+  access into an effect or event handler.
+- **No new `'use no memo'` without all three of:** a reproduced failure, a
+  comment above the directive saying exactly what broke, and the file added to
+  `fileOptOuts` in `.react-compiler-budget.json`. An opt-out is a last resort
+  with a paper trail, not a way to make a refusal go away.
+
+## Decisions already made
+
+- **Do not add `useMemo`, `useCallback` or `React.memo`.** The compiler derives
+  its own memoization from the code and erases hand-written wrappers from the
+  output. Adding one gains nothing and gives the compiler a dependency list it
+  has to verify.
+- **Removing an existing wrapper: diff the compiled output.** Emit the file
+  before and after (see "Reading emitted code"). Identical output means nothing
+  to test. If `const $ = _c(n)` disappears from the output, the compiler has
+  started refusing — the wrapper was load-bearing; stop and look.
+- **A refusal gets fixed, not hidden.** Read the category the gate prints and
+  see [references/refusal-categories.md](references/refusal-categories.md) for
+  what it means and the usual fix.
+- **Public API changes get a migration note** in
+  `platform/docs/docs/migration-guide/3p13-to-3p14/`.
+- **Components that read or mutate cornerstone3D state during render** are the
+  one class that genuinely cannot be compiled yet. That is what the existing
+  opt-outs under `extensions/cornerstone/src/Viewport/` are. Do not extend that
+  set casually; each one has an issue to remove it.
+
+## Reading the gate output
+
+`compiler:coverage:ci` prints every refusal as file, line, category and reason:
+
+```
+extensions/cornerstone/src/components/CinePlayer/CinePlayer.tsx:50
+    Todo: (BuildHIR::lowerExpression) Handle ||= operators in AssignmentExpression
+```
+
+The line points at the offending expression, not the function. A block titled
+"Refusals inside opt-out files" is informational — those files are not being
+memoized anyway.
+
+`lint:compiler:ci` printing **"Budget is stale: the counts improved"** is not an
+error to work around. It is telling you to write the new numbers into the
+budget file.
+
+## Reading emitted code
+
+Use this in two situations only: after fixing a refusal, to confirm the function
+now compiles; and before and after removing a `useMemo` or `useCallback`, to see
+whether the change altered anything. A new component that passes the gates does
+not need it.
+
+To see what the compiler actually produced for a file:
+
+```bash
+node .agents/skills/ohif-react-compiler/assets/emit.mjs <file>
+```
+
+It compiles that one file with the repo's own babel config, compiler included,
+and prints the result. To check whether an edit changed anything the compiler
+emits, run it before and after and diff the two outputs — identical output
+means identical runtime behaviour, so there is nothing to test.
+
+What to look for: `const $ = _c(n)` means the function compiled; `if ($[i] !== x)`
+is a cache guard on input `x`; `t0`, `t1`… are compiler temporaries. A breakpoint
+inside a guard fires once and then stops — that is the cache hitting, not a bug.
+
+## Authoritative sources
+
+- [Rules of React](https://react.dev/reference/rules) — what the compiler assumes.
+- [eslint-plugin-react-hooks](https://react.dev/reference/eslint-plugin-react-hooks)
+  — the compiler's diagnostics as lint rules; this repo uses `recommended-latest`.
+- [React Compiler](https://react.dev/learn/react-compiler) — directives, opt-outs.

.agents/skills/ohif-react-compiler/assets/emit.mjs

@@ -0,0 +1,24 @@
+// Print what the React Compiler emits for one file.
+//
+//   node .agents/skills/ohif-react-compiler/assets/emit.mjs <file>
+//
+// Uses the repo's own babel config (compiler included), found by walking up
+// from the target file, so it works from any working directory. To see whether
+// an edit changed anything the compiler produces, run it before and after and
+// diff the two outputs. Identical output means identical runtime behaviour.
+import { createRequire } from 'node:module';
+import path from 'node:path';
+
+const file = path.resolve(process.argv[2] ?? '');
+if (!process.argv[2]) {
+  console.error('usage: node emit.mjs <file>');
+  process.exit(2);
+}
+
+const babel = createRequire(file)('@babel/core');
+const { code } = babel.transformFileSync(file, {
+  filename: file,
+  cwd: path.dirname(file),
+  rootMode: 'upward',
+});
+console.log(code);

.agents/skills/ohif-react-compiler/references/refusal-categories.md

@@ -0,0 +1,152 @@
+# Refusal categories
+
+When the React Compiler will not memoize a function it emits it unchanged and
+reports a category and a reason. `pnpm run compiler:coverage:ci` prints both,
+with the line of the offending expression. The reason string is the compiler's
+own and is authoritative; this file explains what each category usually means
+in this codebase and how it has been fixed before.
+
+## Sources
+
+- **The category names are the compiler's own.** This file covers the nine this
+  codebase has produced; the compiler defines more, and the gate prints
+  whichever one it hits.
+- **Lint rule names and which are enabled:**
+  [eslint-plugin-react-hooks](https://react.dev/reference/eslint-plugin-react-hooks).
+  This repo uses the `recommended-latest` preset, which leaves some of the
+  plugin's rules off. Every rule named below was checked against the installed
+  plugin.
+- **How to read a diagnostic:**
+  [React Compiler docs](https://react.dev/learn/react-compiler) and the
+  [Rules of React](https://react.dev/reference/rules) the compiler assumes.
+- **The "how it has been fixed" guidance** comes from fixes made in this repo
+  during the React 19 migration, not from documentation. `Hooks`, `Globals` and
+  `UseMemo` were cleared early in that work, so their fix guidance is briefer
+  and less tested than the others'.
+
+Two things to know first:
+
+- **A function reports one category at a time.** Fixing it often exposes a
+  different one underneath. Recompile after every fix and expect a second
+  round.
+- **Most categories have no lint rule that fires here.** `PreserveManualMemo`
+  in particular has a rule enabled as an error that has never once fired on a
+  real case. Only compiling finds these — which is why the coverage gate exists.
+
+## Immutability
+
+*"This value cannot be modified"*, *"Cannot access variable before it is
+declared"*.
+
+Something is being written to that the compiler needs to treat as read-only:
+a prop, a value captured by a closure, or a variable referenced before its
+declaration in a circular pair of callbacks.
+
+Fix by computing into a new local instead of assigning into the existing
+object, and by breaking circular references between callbacks (for example an
+`AbortController` for listener teardown rather than each handler naming the
+other). Check every downstream reader before changing a mutation — today they
+observe the mutated object.
+
+Lint: `react-hooks/immutability` reports some but not all of these.
+
+## Refs
+
+*"Cannot access refs during render"*.
+
+`ref.current` is read or written in the render body rather than in an effect or
+an event handler. The compiler cannot cache across a value it cannot see change.
+
+Fix by moving the access into a `useEffect` or a handler. Where a ref is used to
+carry identity across renders synchronously — "reset X when Y changed, before
+anything renders" — the fix is a design change, not a mechanical one. See the
+`Mode.tsx` issue for a case where the recommendation is to leave it.
+
+Lint: `react-hooks/refs`.
+
+## PreserveManualMemo
+
+*"Existing memoization could not be preserved"*.
+
+A hand-written `useMemo` or `useCallback` has a dependency list narrower than
+what its body actually reads. The compiler infers the real dependencies, finds
+they differ from what was written, and refuses the whole function rather than
+silently change behaviour.
+
+Fix by completing the dependency list, or by deleting the wrapper. In a
+compiled file deleting is usually safe — the compiler re-derives the
+memoization — but diff the compiled output before and after to be sure.
+
+Lint: `react-hooks/preserve-manual-memoization` is enabled and has never fired
+on a real instance here. Do not rely on it.
+
+## Suppression
+
+An `eslint-disable` comment for a `react-hooks/*` rule is present in the
+function. The compiler treats the suppression as a signal that the code
+knowingly breaks a rule, and refuses.
+
+Fix by removing the disable comment and fixing what it was hiding — usually an
+incomplete dependency list.
+
+Lint: `react-hooks/rule-suppression` exists but is not in the preset this repo
+uses.
+
+## RenderSetState
+
+A state setter is called during render rather than in an effect or handler.
+
+Fix by deriving the value instead of storing it, or by moving the call to where
+it belongs. If the setter is guarded so it only fires on a real change, restate
+that as derived state.
+
+Lint: `react-hooks/set-state-in-render`.
+
+## Hooks
+
+A hook is called conditionally, in a loop, or after an early return, so the
+compiler cannot establish a fixed hook order.
+
+Fix by moving every hook above the first `return` and out of any branch. This
+is the same rule `rules-of-hooks` enforces, and it has always applied.
+
+Lint: `react-hooks/rules-of-hooks`; the compiler-specific rule
+`react-hooks/hooks` exists but is not in the preset.
+
+## Globals
+
+A module-scope value is mutated during render.
+
+Fix by moving the mutation into an effect or handler, or by lifting the value
+into state or context so React owns it.
+
+Lint: `react-hooks/globals`.
+
+## UseMemo
+
+`useMemo` is called in a shape the compiler will not handle — for example with
+something other than a plain function as its first argument.
+
+Fix per the reason string; usually the wrapper can simply be removed.
+
+Lint: `react-hooks/use-memo`.
+
+## Todo
+
+*"(BuildHIR::lowerExpression) Handle …"*.
+
+Not a problem with your code. The compiler's front end has not implemented a
+syntax form you used. The one instance in this codebase is a logical assignment
+operator, `||=`.
+
+Fix by rewriting the construct longhand (`x = x || y`). This category shrinks
+with each compiler release.
+
+Lint: `react-hooks/todo` reports these verbatim but is not in the preset.
+
+## Which lint rules are actually on
+
+This repo uses `eslint-plugin-react-hooks` `recommended-latest`, which does not
+enable every rule the plugin ships. The ones that would report `Todo`,
+`Suppression`, and the compiler-specific `hooks` are off. So the linter being
+clean does not mean the compiler is happy; only the coverage gate tells you that.

.agents/skills/ohif-test-agent/SKILL.md

@@ -243,34 +243,35 @@ Reach for the cheapest *faithful* signal, in this order:
 
 1. **A faithful DOM/SVG/state signal exists → assert on it.** Panel counts, dialog and
    overlay text, enabled/disabled state, and any overlay that renders as SVG (a vector
-   overlay's color is readable via `getSvgAttribute`) all have a DOM representation — assert
-   on it directly, no screenshot.
-2. **The thing under test is painted onto the WebGL canvas with no DOM representation → a
-   screenshot is correct and required.** Raster output on the canvas exposes no attribute to
-   read for a painted pixel. Scope a `checkForScreenshot` to the viewport (pane or grid) and
-   assert it — this is the right tool, not a last resort, whenever what you're verifying is
-   the rendered canvas itself.
+   overlay's color is readable via `getSvgAttribute`) are all readable from the DOM —
+   assert on them directly, no screenshot.
+2. **The thing under test exists only as pixels on the WebGL canvas → a screenshot is
+   correct and required.** A painted pixel exposes no element or attribute to read. Capture
+   a viewport pane with `checkForViewportScreenshot`, or scope a `checkForScreenshot` to the
+   grid — this is the right tool, not a last resort, whenever what you're verifying is the
+   rendered canvas itself.
 3. **Never substitute a service/state read for a render assertion.** Reading a service's
    state (any `window.services...`) asserts the *data model*, not the pixels the user sees —
    it passes even when rendering is broken. `page.evaluate(() => window.services...)` is an
    escape hatch for *setup*, not for *appearance* assertions.
 
-For anything drawn onto the WebGL canvas with no DOM signal, compare a screenshot scoped to a specific viewport or the viewport grid:
+For a screenshot comparison scoped to a specific viewport, use `checkForViewportScreenshot` — it hides the viewport's overlay text for the capture:
 
 ```ts
-await checkForScreenshot({
+await checkForViewportScreenshot({
   page,
-  locator: viewportPageObject.grid, // scope to the viewport grid — not the whole page
+  viewport: activeViewport, // captures the viewport pane with its text hidden
   screenshotPath: screenShotPaths.length.lengthDisplayedCorrectly,
 });
 ```
 
-`checkForScreenshot` retries up to 10 times at 500 ms intervals. Use `screenShotPaths.<category>.<name>` rather than a hand-typed string — the tree of valid keys lives in `tests/utils/screenShotPaths.ts`.
+Both helpers retry up to 10 times at 1250 ms intervals by default (`attempts` and `delay` are configurable) (`checkForViewportScreenshot` delegates to `checkForScreenshot`; use the latter directly only for non-viewport locators such as the grid or a panel). Use `screenShotPaths.<category>.<name>` rather than a hand-typed string — the tree of valid keys lives in `tests/utils/screenShotPaths.ts`.
 
 Rules (apply to all new screenshot assertions):
 
 - **Use the object form.** The positional form is legacy; don't introduce it in new code, and don't treat existing positional-form usage as a pattern to copy.
-- **Never screenshot the full app.** Full-page screenshots include panels, toolbars, and dialogs that drift independently of what's under test and make baselines fragile. Scope by passing a `locator` — `viewportPageObject.grid` for the grid, or a specific viewport pane. A bare `normalizedClip: { x: 0, y: 0, width: 1, height: 1 }` with no `locator` is **not** scoping — it clips to the full page. Use `normalizedClip` only to target a sub-region *of a locator* (e.g. a scrollbar strip). If you reach for `fullPage: true`, stop and pick a locator.
+- **No text in baselines.** Overlay text (date, series description, W/L, slice index) drifts with data, locale, and font rendering, so a baseline that contains it is fragile — new viewport baselines must be text-free. Capture viewports through `checkForViewportScreenshot`, which hides all viewport text for the shot; a raw `checkForScreenshot` on a viewport pane bakes the text in.
+- **Never screenshot the full app.** Full-page screenshots include panels, toolbars, and dialogs that drift independently of what's under test and make baselines fragile. Scope by passing a `locator` — `viewportPageObject.grid` for the grid, or a specific viewport pane. A bare `normalizedClip: { x: 0, y: 0, width: 1, height: 1 }` with no `locator` is **not** scoping — it clips to the full page. Use `normalizedClip` only to target a sub-region *of a locator* (e.g. a scrollbar strip). `fullPage: true` only takes effect when no `locator` is passed — that *is* the full-app capture this rule forbids, so pass a `locator` instead. (Through `checkForViewportScreenshot` the flag is inert: the capture is always scoped to the viewport pane.)
 - **Do not tune `maxDiffPixelRatio` or `threshold`** to make a screenshot pass. If a baseline mismatches, regenerate it after a human review of the diff, or fix the underlying flake.
 
 ## Playwright config facts worth remembering
@@ -323,7 +324,7 @@ Before returning a generated OHIF test, confirm all items:
 3. Uses normalized viewport interactions (`normalizedClickAt` / `normalizedDragAt`) unless there is a strong reason otherwise.
 4. Uses a valid canonical StudyInstanceUID and compatible mode.
 5. Handles hydration or measurement tracking prompts when the workflow requires them.
-6. Uses the faithful signal for each assertion — DOM/SVG where the result has a DOM representation, a viewport-scoped screenshot when what's verified is canvas-only raster output, and never a `window.services` state read in place of a render check. Any `checkForScreenshot` call uses the object form, scoped via a `locator` (viewport pane or grid) — no full-app screenshots.
+6. Uses the faithful signal for each assertion — DOM/SVG where the result is readable from the DOM, a viewport-scoped screenshot when what's verified exists only as pixels on the canvas, and never a `window.services` state read in place of a render check. Any `checkForScreenshot` call uses the object form, scoped via a `locator` (viewport pane or grid) — no full-app screenshots.
 7. Replaces `page.waitForTimeout(...)` after viewport-rendering actions with `waitForViewportRenderCycle(page)` (started before the action) — keeps `waitForTimeout` only for non-render waits like the hydration prompt in `beforeEach`.
 8. If execution was skipped, states that explicitly and provides concrete run commands.
 9. Every application control is reached through a page object — no raw `getByTestId`/`getByRole` in the spec for buttons, menus, dialogs, or fields. Any control not already covered was added to the right page object (or a new one), with a source `data-cy` if it lacked one.