feat(testing): OHIF Test Agent Skills

tests/ohif-test-agent/AGENTS.md

@@ -0,0 +1,60 @@
+# OHIF Test Agent (AgentSkills Wrapper)
+
+Use this guide whenever the user asks to write, add, modify, review, or debug Playwright E2E tests for OHIF Viewer workflows.
+
+This wrapper is for any runtime that supports agentskills.io. The technical rules are shared with `SKILL.md`.
+
+## Required workflow
+
+1. Classify the requested test by feature area.
+2. Read `references/patterns-by-feature.md` and choose a seed spec.
+3. Scaffold from `assets/spec-template.ts` or adapt the seed spec.
+4. Confirm fixture and import conventions from:
+   - `references/page-objects.md`
+   - `references/utilities.md`
+5. For method signatures, read source under `tests/pages/` and `tests/utils/`.
+6. Generate test code.
+7. If execution is available, run Playwright for that spec and adjust.
+8. If execution is unavailable, run static validation and report limits.
+
+## Hard rules
+
+- Import `test` and `expect` from `./utils`, never from `@playwright/test`.
+- Use fixture-injected page objects from test args. Do not instantiate page objects with `new`.
+- Use normalized viewport interactions for WebGL (`normalizedClickAt`, `normalizedDragAt`).
+- Use canonical StudyInstanceUID values and valid mode pairing.
+- Handle segmentation hydration and measurement tracking prompts when applicable.
+- Use screenshot comparison for canvas-rendered assertions and DOM assertions for panel/dialog/overlay text state.
+- If a utility is not exported from `./utils`, use deep import from `./utils/<file>`.
+
+## Mandatory self-check before final answer
+
+Confirm all of these are true:
+
+1. Correct import source (`./utils`).
+2. Correct fixture key casing (`DOMOverlayPageObject` with capital D).
+3. Normalized viewport interactions used by default.
+4. UID and mode are compatible with the tested feature.
+5. Prompt dialogs/hydration flow are handled if triggered by that scenario.
+6. Assertion strategy matches render surface (canvas vs DOM).
+
+## Output contract when tests are not executed
+
+If you cannot run tests in this environment, include:
+
+1. The generated test code.
+2. Assumptions made.
+3. Static checks performed.
+4. Exact local commands to run, such as:
+
+```sh
+yarn playwright test tests/<SpecName>.spec.ts
+yarn playwright test tests/<SpecName>.spec.ts --update-snapshots
+```
+
+## Reference map
+
+- Feature seeds and canonical UIDs: `references/patterns-by-feature.md`
+- Page object structural rules: `references/page-objects.md`
+- Utility import/signature rules: `references/utilities.md`
+- Failure diagnosis patterns: `references/failure-triage.md`

tests/ohif-test-agent/assets/spec-template.ts

@@ -0,0 +1,48 @@
+import {
+  checkForScreenshot,
+  expect,
+  screenShotPaths,
+  test,
+  visitStudy,
+  waitForViewportRenderCycle,
+} from './utils';
+
+test.beforeEach(async ({ page }) => {
+  // Pick the right UID + mode for your feature (see references/patterns-by-feature.md)
+  const studyInstanceUID = '1.3.6.1.4.1.25403.345050719074.3824.20170125095438.5';
+  const mode = 'viewer';
+  await visitStudy(page, studyInstanceUID, mode, 2000);
+});
+
+test.describe('FEATURE NAME', () => {
+  test('describes the behaviour in one sentence', async ({
+    page,
+    viewportPageObject,
+    mainToolbarPageObject,
+    rightPanelPageObject,
+    DOMOverlayPageObject, // capital D on purpose
+  }) => {
+    // 1. Arrange — select a tool, open a panel, etc.
+
+    // 2. Act — interact with the viewport.
+    //    Prefer normalized (0–1) coordinates:
+    //    const activeViewport = await viewportPageObject.active;
+    //    await activeViewport.normalizedClickAt([{ x: 0.3, y: 0.3 }, { x: 0.7, y: 0.7 }]);
+
+    //    For actions that re-render the viewport, gate on the render cycle
+    //    instead of sleeping — start the watcher BEFORE the action:
+    //    const cycle = waitForViewportRenderCycle(page);
+    //    await action();
+    //    await cycle;
+
+    // 3. Handle prompts (first measurement prompts for tracking; SEG/RT/SR prompts for hydration)
+    //    await DOMOverlayPageObject.viewport.measurementTracking.confirm.click();
+
+    // 4. Assert canvas output via visual regression
+    //    await checkForScreenshot(page, page, screenShotPaths.YOUR_CATEGORY.YOUR_KEY);
+
+    // 5. Assert DOM state directly
+    //    const count = await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount();
+    //    expect(count).toBe(1);
+  });
+});

tests/ohif-test-agent/instructions.md

@@ -0,0 +1,80 @@
+# Installation Instructions
+
+This file explains how to install this skill package into popular agent runtimes.
+
+## Package contents
+
+This folder is expected to contain:
+
+- `SKILL.md` (core behavior rules)
+- `AGENTS.md` (runtime entrypoint)
+- `references/` (supporting guidance)
+- `assets/` (templates)
+
+## Quick validation after install
+
+After installation in any runtime, validate with a small prompt such as:
+
+"Write an OHIF Playwright test for Length tool and include your pre-output self-check."
+
+A correct response should:
+
+1. Import `test`/`expect` from `./utils`
+2. Use fixture-injected page objects
+3. Use normalized viewport interactions
+4. Use a canonical StudyInstanceUID and compatible mode
+5. Handle prompt flows when relevant
+6. Include static-check notes if execution was not performed
+
+---
+
+## Claude setup (starting point)
+
+1. Locate your Claude skills directory:
+   - Common location: `~/.claude/skills/`
+2. Copy this folder into that directory:
+   - Result should look like: `~/.claude/skills/ohif-test-agent/`
+3. Ensure `SKILL.md` is at the root of the copied folder.
+4. Restart Claude session (or open a new one) so skills are discovered.
+5. Test with a prompt that clearly targets OHIF Playwright E2E behavior.
+
+Notes:
+
+- Claude triggering uses the `name` and `description` frontmatter in `SKILL.md`.
+- Keep `references/` and `assets/` alongside `SKILL.md`; links assume this structure.
+
+---
+
+## Codex setup
+
+Point your Codex runtime at `.github/agents/ohif-test-agent/`.
+
+How you do that depends on how your Codex environment discovers agent instructions (a global config, an IDE setting, a project-level entrypoint, etc.). Whatever the mechanism, the entrypoint you want it to read is:
+
+```
+.github/agents/ohif-test-agent/AGENTS.md
+```
+
+That file is the runtime-facing summary; it pulls in `SKILL.md`, `references/`, and `assets/` from the same folder. Once Codex is configured to load it, issue an OHIF Playwright E2E prompt as a quick smoke test.
+
+Notes:
+
+- `AGENTS.md` is the entrypoint; `SKILL.md` is the detailed behavior rules. Keep both at the package root.
+- The package is source-first: it directs Codex to [tests/pages/](../../../tests/pages/) and [tests/utils/](../../../tests/utils/) for the current method surface rather than duplicating it.
+
+---
+
+## Troubleshooting
+
+If generated tests are wrong or flaky:
+
+1. Confirm the runtime loaded this package (not an older skill).
+2. Check imports are from `./utils` (not `@playwright/test`).
+3. Verify `DOMOverlayPageObject` casing.
+4. Verify UID/mode pairing with `references/patterns-by-feature.md`.
+5. Use `references/failure-triage.md` to classify failures before changing logic.
+
+If links appear broken:
+
+- Keep the folder layout unchanged.
+- Do not move `references/` or `assets/` outside this package.

tests/ohif-test-agent/references/failure-triage.md

@@ -0,0 +1,70 @@
+# Failure triage
+
+Before debugging, classify. Most OHIF test failures are timing or hydration — not real regressions.
+
+| Category | Symptom | Fix |
+|----------|---------|-----|
+| Timing | Element not visible, action timeout | Add / increase the `delay` param of `visitStudy`; for actions that re-render viewports, use `waitForViewportRenderCycle(page)` (started before the action) instead of `waitForTimeout`; wrap the assertion in `expect.toPass({ timeout })` |
+| Selector | Element not found | Verify `data-cy` on the target; confirm the panel is open (`toggle()` / `select()` before interacting); check for capital `D` in `DOMOverlayPageObject` when destructuring |
+| Hydration | Segmentation/RT not interactive | Ensure the `segmentationHydration.yes.click()` fired; add `waitForTimeout(3000)` after `loadSeriesByModality('SEG'\|'RTSTRUCT'\|'SR')` |
+| Data | Study not found, empty viewport | Confirm the UID is in the canonical list (see [patterns-by-feature.md](patterns-by-feature.md)); confirm the mode supports the feature (segmentation tools aren't in `viewer` mode) |
+| Visual drift | Screenshot mismatch but feature works | Re-generate the baseline with `yarn playwright test --update-snapshots`; consider raising `maxDiffPixelRatio` to `0.04` for 3D content |
+| Real regression | Feature is actually broken | Report as a bug — this is the test doing its job |
+
+## Prefer render-cycle waits over sleeps
+
+If you're tempted to add `await page.waitForTimeout(2000)` after an action, ask whether the action re-rendered the viewport. If it did, use:
+
+```ts
+const cycle = waitForViewportRenderCycle(page);
+await action();
+await cycle;
+await check();
+```
+
+The watcher must be created **before** the action — it waits for `needsRender` first, and that transition is gone by the time the action returns. See the "Wait for renders, don't sleep" section in [SKILL.md](../SKILL.md) and [tests/SEGHydrationFromMPR.spec.ts](tests/SEGHydrationFromMPR.spec.ts).
+
+### When the cycle helper times out at `waitForAnyViewportNeedsRender`
+
+Symptom: the test fails inside `waitForAnyViewportNeedsRender` after 5s, with the action having actually completed in the UI. The action just doesn't transition the viewport through `needsRender` synchronously. Known cases:
+
+- Hanging-protocol changes.
+- **RTSTRUCT / contour segmentation hydration confirm.** SEG (labelmap) hydration does fire `needsRender`; contour does not. The fix is to gate on the actual end-state — for hydration in `beforeEach`, `await expect(page.getByTestId('data-row')).toHaveCount(N)` is the right wait.
+
+Don't react by raising the cycle's timeout — the transition isn't coming. Replace the cycle wrapper with an auto-retrying DOM/SVG assertion, or `expect.toPass({ timeout })` around the assertion block.
+
+## The `toPass` pattern
+
+When an assertion needs to wait for async render / propagation:
+
+```ts
+await expect(async () => {
+  await rightPanelPageObject.labelMapSegmentationPanel.panel.segmentByText('Spleen').click();
+  await expect(activeViewport.overlayText.bottomRight.instanceNumber).toContainText('17/');
+}).toPass({ timeout: 10_000 });
+```
+
+`toPass` reruns the assertion block until it succeeds or the timeout expires — cleaner than a hand-rolled retry loop and surfaces the last failure reason if it times out.
+
+## Common `DOMOverlayPageObject` mistake
+
+The fixture key is capital-D `DOMOverlayPageObject`, not lowercase. If your destructure is silently `undefined`, check the casing.
+
+## `press` import mistake
+
+`press` is NOT re-exported from `./utils`. `import { press } from './utils'` resolves to `undefined` and fails at runtime. Use:
+
+```ts
+import { press } from './utils/keyboardUtils';
+await press({ page, key: 'ArrowDown', nTimes: 50 }); // object param, not (page, key)
+```
+
+## Visual regression iteration
+
+Screenshots live under `tests/screenshots/chromium/<testFilePath>/`. To accept new output as the baseline:
+
+```sh
+yarn playwright test tests/YourSpec.spec.ts --update-snapshots
+```
+
+Review the resulting PNGs carefully — an agent-accepted baseline that's subtly wrong is worse than a failing test.

tests/ohif-test-agent/references/page-objects.md

@@ -0,0 +1,90 @@
+# OHIF Page Object guide
+
+> This file documents the **stable structural rules** of the page object system. For the current list of methods and properties on any class, **read the source under [tests/pages/](tests/pages/)** — it is always authoritative, and it evolves as the product does. A static method table in a reference file goes stale the moment someone refactors; the source does not.
+
+## How to discover the API of a page object
+
+1. Find the relevant class in [tests/pages/](tests/pages/). File names match class names.
+2. Read it end-to-end once — most are under a few hundred lines.
+3. Some classes compose sub-objects (e.g. `RightPanelPageObject` holds a measurementsPanel, contourSegmentationPanel, labelMapSegmentationPanel, tmtvPanel, etc.). Those sub-objects usually live in the same file or a sibling under `tests/pages/`.
+4. To see how a method is actually used, grep `tests/` or open the seed spec listed in [patterns-by-feature.md](patterns-by-feature.md). Real usage beats a synthesized signature every time.
+
+Do not try to memorize a method surface from this file — it intentionally does not list one. It lists only the rules you cannot derive from the source by reading a single file.
+
+---
+
+## Stable rules
+
+### Fixture keys (case-sensitive)
+
+These are injected via [tests/utils/fixture.ts](tests/utils/fixture.ts). Destructure them from the test function's first argument — do not `new` them, because the fixture wires sub-objects to the correct `page` and hand-constructed instances skip that wiring.
+
+- `viewportPageObject`
+- `mainToolbarPageObject`
+- `leftPanelPageObject`
+- `rightPanelPageObject`
+- `DOMOverlayPageObject` — **capital D**. A silent `undefined` destructure is almost always a casing typo here.
+- `notFoundStudyPageObject`
+
+If the fixture file is updated and new keys are added, they will show up there first — check it if something feels missing.
+
+### Non-fixture page objects
+
+Some page object classes are not fixture-injected. They are reached through an injected fixture:
+
+- `DicomTagBrowserPageObject` → via `DOMOverlayPageObject.dialog.dicomTagBrowser`
+- `DataOverlayPageObject` → via `viewportPageObject.getById(viewportId).overlayMenu.dataOverlay`
+
+Both can be constructed manually (`new DataOverlayPageObject(page)`) if a test really needs a fresh instance, but the accessor path is the idiomatic one.
+
+### Viewport wrapper vs. viewport instance
+
+`viewportPageObject` is a **wrapper**. You almost always want a specific viewport out of it first:
+
+- `await viewportPageObject.active` — the currently focused viewport
+- `viewportPageObject.getAll()` — every viewport in the grid
+- `viewportPageObject.getNth(i)` — zero-indexed
+- `viewportPageObject.getById(cornerstoneViewportId)` — e.g. `'default'`, `'ctAXIAL'`
+
+The object these return is the one with `normalizedClickAt`, `normalizedDragAt`, `overlayText`, `nthAnnotation`, etc. Reach for the viewport instance first, then call methods on it.
+
+### Panel access order
+
+Every `rightPanelPageObject` sub-panel follows the same three-step idiom: **open the side panel, `.select()` the sub-panel tab, then interact with `.panel.*`**. Skipping either of the first two is the most common cause of "element not found".
+
+```ts
+await rightPanelPageObject.toggle();
+await rightPanelPageObject.measurementsPanel.select();
+const count = await rightPanelPageObject.measurementsPanel.panel.getMeasurementCount();
+```
+
+The exact row/action methods vary by panel — check the source file for the one you need.
+
+### Layout identifiers are camelCase JS properties
+
+`mainToolbarPageObject.layoutSelection.<layout>.click()` — access layouts by camelCase property name (e.g. `threeDFourUp`, `axialPrimary`), not with bracket-escaped DICOM-ish strings like `['3DFourUp']`. This is a convention enforced by how the class exposes its tools.
+
+### Sub-tools auto-open their dropdown
+
+Tools nested inside a toolbar dropdown (measurement tools, more tools, layouts) each expose a `.click()` that opens the parent menu for you. You almost never need to open the menu first. `await mainToolbarPageObject.measurementTools.length.click()` does both the expand and the select.
+
+---
+
+## Page object map — what each class is *for*
+
+This table exists to help you pick the right file to open, not to enumerate methods. Source of truth for any specific method remains the `.ts` file.
+
+| Class | File | Covers |
+|-------|------|--------|
+| ViewportPageObject | [tests/pages/ViewportPageObject.ts](tests/pages/ViewportPageObject.ts) | Cornerstone viewports — clicks, drags, overlays, annotations, crosshairs |
+| MainToolbarPageObject | [tests/pages/MainToolbarPageObject.ts](tests/pages/MainToolbarPageObject.ts) | Top toolbar — measurement tools, more tools, layouts, crosshairs, pan |
+| LeftPanelPageObject | [tests/pages/LeftPanelPageObject.ts](tests/pages/LeftPanelPageObject.ts) | Study browser — thumbnails, load by modality or description |
+| RightPanelPageObject | [tests/pages/RightPanelPageObject.ts](tests/pages/RightPanelPageObject.ts) | Side panels — measurements, contour seg, labelmap seg, TMTV, microscopy |
+| DOMOverlayPageObject | [tests/pages/DOMOverlayPageObject.ts](tests/pages/DOMOverlayPageObject.ts) | DOM overlays — dialogs, hydration/tracking prompts, context menus, tag-browser accessor |
+| NotFoundStudyPageObject | [tests/pages/NotFoundStudyPageObject.ts](tests/pages/NotFoundStudyPageObject.ts) | Study-not-found error page |
+| DicomTagBrowserPageObject | [tests/pages/DicomTagBrowserPageObject.ts](tests/pages/DicomTagBrowserPageObject.ts) | Tag-browser dialog (non-fixture; reach via `DOMOverlayPageObject.dialog`) |
+| DataOverlayPageObject | [tests/pages/DataOverlayPageObject.ts](tests/pages/DataOverlayPageObject.ts) | Data-overlay menu (non-fixture; reach via `viewport.overlayMenu`) |
+
+If the directory adds or renames a file, that diff is your first clue and this table is your second — trust the directory.
+
+For live usage, the seed spec in [patterns-by-feature.md](patterns-by-feature.md) shows how a class is actually called; the source file tells you everything else that's on it.