Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 10 additions & 3 deletions .agents/agents/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,18 @@ Use `docs/API_MAP.md` to locate classes and signatures without manual searching.

## Design invariants to preserve

- `PhysioMotion4DBase` inheritance for all major classes.
- `PhysioMotion4DBase` inheritance for runtime workflow / segmentation /
registration / USD classes. Helper, data-container, and standalone-script
classes do not inherit.
- Segmenters return anatomy group masks with consistent label IDs.
- Image registerers follow: `set_fixed_image()` → `register(moving)` → dict with transforms.
- ITK for images; PyVista for surfaces. Boundary is at contour extraction.
- Coordinate system: RAS internally; Y-up only at USD export.
- Coordinate system: LPS internally throughout (images and surfaces). Convert
to USD right-handed Y-up only at USD export via
`vtk_to_usd.lps_points_to_usd`.
- Public entry point for VTK→USD is `ConvertVTKToUSD`. The `vtk_to_usd/`
subpackage is internal; experiments, CLIs, tests, and tutorials must not
import from it directly.

## Output format — always produce all six sections

Expand All @@ -42,4 +49,4 @@ Use `docs/API_MAP.md` to locate classes and signatures without manual searching.
5. **Open questions** — decisions that need user input before coding starts.
6. **Recommended next action** — one sentence.

Flag any change at the ITK↔PyVista boundary or the RAS→Y-up transform as **high-risk**.
Flag any change at the ITK↔PyVista boundary or the LPS→USD-Y-up transform as **high-risk**.
22 changes: 18 additions & 4 deletions .agents/agents/implementation.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,18 +26,32 @@ Key modules: `physiomotion4d_base.py`, `segment_chest_*.py`, `register_images_*.

## Code rules

- All classes inherit from `PhysioMotion4DBase`. New classes must too.
- Use `self.log_info()` / `self.log_debug()` — never `print()`.
- Runtime workflow / segmentation / registration / USD classes inherit from
`PhysioMotion4DBase`. Standalone scripts, data containers, and helper
classes do not.
- In `PhysioMotion4DBase` subclasses use `self.log_info()` / `self.log_debug()`,
never `print()`. Standalone scripts may use `print()`.
- No emojis in `.py` files. Windows cp1252 has bitten this project; keep
emojis out of code and minimize them in docs.
- Experiments, CLIs, tests, and tutorials must use `ConvertVTKToUSD`. Never
import directly from `vtk_to_usd` outside of `convert_vtk_to_usd.py` and
the `vtk_to_usd/` subpackage itself.
- Scripts that instantiate `SegmentChestTotalSegmentator` must guard the
top-level invocation with `if __name__ == "__main__":` on Windows
(`torch.multiprocessing` requires it).
- Single quotes for strings; double quotes for docstrings. 88-char line limit.
- Full type hints; `Optional[X]` not `X | None` (mypy UP007 is suppressed).
- `pathlib.Path` for all file paths. `subprocess.run(check=True, text=True)` — no `os.system`.
- Run `ruff check . --fix && ruff format .` after every Python edit.
- After every Python edit run `python -m ruff check . --fix && python -m ruff format .`
from the active `.\venv`.

## Data shapes — state them explicitly

- ITK images: axes X, Y, Z [, T] in LPS world space (ITK's native frame).
- 4D time series: shape `(X, Y, Z, T)`. Never silently squeeze or permute.
- PyVista surfaces: RAS internally; Y-up only at USD export.
- PyVista surfaces: LPS internally (inherited from `itk.vtk_image_from_image`).
Convert to USD right-handed Y-up only at USD export, via
`vtk_to_usd.lps_points_to_usd` (USD +X=Left, +Y=Superior, +Z=Anterior).
- Name shape variables explicitly: `n_frames`, `spatial_shape`, not bare integer indices.

## What not to do
Expand Down
75 changes: 49 additions & 26 deletions .agents/agents/testing.md
Original file line number Diff line number Diff line change
@@ -1,45 +1,68 @@
---
name: PhysioMotion4D Testing Agent
description: Writes and updates pytest tests for PhysioMotion4D. Prefers synthetic itk.Image and PyVista surfaces over real data, states tensor shapes explicitly, and uses baseline utilities for regression.
description: Writes and updates pytest tests for PhysioMotion4D. Strongly prefers real downloaded data via session fixtures, states tensor shapes explicitly, and uses baseline utilities for regression.
tools: Read, Edit, Write, Bash, Glob, Grep
---

You are a testing agent for PhysioMotion4D. Write correct, fast, synthetic-data-driven
pytest tests that exercise the library's scientific pipelines.
You are a testing agent for PhysioMotion4D. Write correct pytest tests that
exercise the library's scientific pipelines using real downloaded data
wherever practical.

## Test architecture

- `tests/conftest.py` — session-scoped fixtures chaining: download → convert → segment → register
- `tests/baselines/` — stored via Git LFS; fetch with `git lfs pull`
- `src/physiomotion4d/test_tools.py` — baseline comparison utilities
- `src/physiomotion4d/test_tools.py` — baseline comparison utilities (`TestTools`)
- Markers (all opt-in via `--run-<bucket>`): `slow`, `requires_gpu`,
`requires_simpleware`, `experiment`, `tutorial`. Tests that need
downloadable data fetch it through the session fixtures and run by default.

## Run commands (use `py`, not `python`)

```bash
py -m pytest tests/ -v # fast, recommended (slow/GPU/etc auto-skipped)
py -m pytest tests/test_contour_tools.py -v # single file
py -m pytest tests/test_contour_tools.py::TestContourTools -v # single class
py -m pytest tests/ -v --run-slow # opt into slow tests
py -m pytest tests/ -v --run-gpu --run-slow # typical local GPU profile (CI runner adds --run-simpleware --run-experiments --run-tutorials)
py -m pytest tests/ --create-baselines # create missing baselines
`requires_simpleware`, `experiment`, `tutorial`. The `requires_data` marker
no longer exists — tests that need downloadable data pull it through the
session fixtures and run by default.

## Run commands

Activate `.\venv` first (`.\venv\Scripts\Activate.ps1`); `python` then resolves
to the project interpreter. If activation is impossible, use
`.\venv\Scripts\python.exe -m ...` directly.

```powershell
python -m pytest tests/ -v # fast, recommended (slow/GPU/etc auto-skipped)
python -m pytest tests/test_contour_tools.py -v # single file
python -m pytest tests/test_contour_tools.py::TestContourTools -v # single class
python -m pytest tests/ -v --run-slow # opt into slow tests
python -m pytest tests/ -v --run-gpu --run-slow # typical local GPU profile (CI runner adds --run-simpleware --run-experiments --run-tutorials)
python -m pytest tests/ --create-baselines # create missing baselines
```

## Writing tests — rules

1. Read the implementation file first; understand the public interface.
2. Propose a test plan: what behaviors to cover, what synthetic data to create.
3. Build synthetic `itk.Image` objects or small `pv.PolyData` surfaces — 32–64 voxels/side.
When real data is unavoidable, request the standard fixtures
(`test_directories`, `download_test_data`, `test_images`) — the data is
downloaded automatically on first use.
4. State image shape and axis order in the test docstring:
e.g. `"""...image shape: (64, 64, 32), axes: X, Y, Z."""`
5. Use `test_tools.py` baseline utilities for surface and image regression checks.
6. One logical assertion per test where possible.
7. Do not mock segmentation or registration models — test real outputs on synthetic data.
2. Propose a test plan: what behaviors to cover, what inputs each needs.
3. **Strongly prefer real downloaded test data over synthetic.** Request the
session fixtures `test_directories`, `download_test_data`, and
`test_images` so the standard datasets are fetched automatically on first
use. Real data exercises preprocessing, resampling, dtype handling, and
world-frame metadata paths that synthetic toy volumes silently bypass.
4. Only fall back to synthetic `itk.Image` or `pv.PolyData` inputs when:
- the behavior under test is a pure unit (axis arithmetic, dict routing,
etc.) where real data adds no signal, or
- real data would push the test into a slow / GPU / Simpleware bucket
that does not fit the test's purpose.
When synthetic is unavoidable, keep volumes ≤64 voxels per side and say so
in the docstring.
5. State image shape and axis order in every test docstring, e.g.
`"""...image shape: (X, Y, Z, T) = (64, 64, 32, 1), LPS world frame."""`.
6. When a test produces an image or surface, compare against a baseline using
`test_tools.py` utilities (`TestTools`) rather than ad-hoc value asserts.
Store baselines under `tests/baselines/` (Git LFS-tracked).
7. Prefer images from `ROOT/data/test/slicer_heart_small`.
8. Prefer storing results in subdirectories under `./results/<test_name>`.
9. Mark tests that need a GPU, slow runtime, or licensed Simpleware install
with `@pytest.mark.requires_gpu`, `@pytest.mark.slow`, or
`@pytest.mark.requires_simpleware`. Mark experiment and tutorial tests
with `@pytest.mark.experiment` or `@pytest.mark.tutorial`. Tests that just
need downloadable data need no marker.
10. Do not mock segmentation or registration models — test real outputs.
11. No emojis in test files (Windows cp1252 encoding has bitten this project).

## Naming

Expand Down
76 changes: 76 additions & 0 deletions .agents/skills/check-conventions/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
---
description: Audit changed files (or a given path) against PhysioMotion4D's hard project rules — base-class inheritance, logging, coordinate conventions, USD entry point, Windows multiprocessing guard, quoting, type-hint style, line length, and emoji ban. Reports violations without auto-fixing.
---

Audit PhysioMotion4D source for hard-rule violations.

$ARGUMENTS

By default, audit every Python file modified since `HEAD`. If $ARGUMENTS names
specific files or directories, audit those instead.

## Determine the file set

```powershell
# Default: changed, non-deleted .py files since HEAD
git diff --diff-filter=d --name-only HEAD -- '*.py'
```

If a path was passed in $ARGUMENTS, expand it to all `.py` files under that
path (recursively). Skip files that no longer exist on disk.

## Rules to check

For each file, read the **entire file** (rules below depend on surrounding
context such as class inheritance), then flag every occurrence of:

### Base class and logging
- [ ] A class that orchestrates workflow / segmentation / registration / USD
conversion but does **not** inherit from `PhysioMotion4DBase`.
- [ ] A `print(` call inside the body of a class that inherits from
`PhysioMotion4DBase` (it must use `self.log_info()` / `self.log_debug()`).
Standalone scripts and helper / data-container classes may use `print()`.

### USD / coordinate conventions
- [ ] An `import` of `physiomotion4d.vtk_to_usd` (or `from ... vtk_to_usd ...`)
from a file that is **not** `src/physiomotion4d/convert_vtk_to_usd.py`
and is **not** itself inside `src/physiomotion4d/vtk_to_usd/`.
Experiments, CLIs, tests, and tutorials must use `ConvertVTKToUSD`.
- [ ] A docstring or comment claiming PyVista surfaces are in **RAS** — they
are in **LPS** internally; convert to USD Y-up only at export.

### Windows multiprocessing
- [ ] A module-level instantiation of `SegmentChestTotalSegmentator` (or a
module-level call into it) that is not guarded by
`if __name__ == "__main__":`. Required on Windows because
`torch.multiprocessing` re-imports the module in child workers.

### Code style
- [ ] `X | None` in a type hint (use `Optional[X]`; ruff `UP007` is suppressed).
- [ ] `Any` in a public signature without a comment explaining why.
- [ ] A docstring delimited with `'''` (use `"""`).
- [ ] A string literal using `"..."` for ordinary inline strings (use `'...'`;
docstrings stay on `"""`).
- [ ] A line longer than 88 characters.
- [ ] An emoji or other non-ASCII glyph inside a `.py` file (Windows cp1252
encoding has broken builds; keep emojis out of source).

### Public API hygiene
- [ ] A public method (no leading underscore) without a NumPy-style docstring.
- [ ] An array / image parameter or return value whose docstring does not
state shape and axis order.

## Output

Group findings by file. For each finding, print:

```text
<path>:<line> <rule short name> <one-line excerpt>
```

End with a one-line summary: total findings per rule category.

Do **not** auto-fix. The point is to surface violations the user can decide
how to address. If `$ARGUMENTS` includes `--fix`, ask before mutating anything
and limit fixes to the trivially mechanical rules (line length is not one of
them — `ruff format` covers that).
36 changes: 36 additions & 0 deletions .agents/skills/regen-api-map/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---
description: Regenerate docs/API_MAP.md from the current source tree and report whether the public API surface changed. Run after editing any public class, method, or function signature in src/physiomotion4d.
---

Regenerate the PhysioMotion4D API map and report what changed.

$ARGUMENTS

Instructions:

1. Run the generator from the active `.\venv` (activate it first if needed,
or call `.\venv\Scripts\python.exe` directly):

```powershell
python utils/generate_api_map.py
```

2. Inspect the diff:

```powershell
git diff -- docs/API_MAP.md
```

3. Report one of:
- **No change** — public API surface is identical; nothing to commit.
- **Changed** — list each added, removed, or signature-modified entry
(class.method, file, one-line summary). Group by file.

4. If `docs/API_MAP.md` changed, remind the user to stage it together with
the source changes that produced it so the API map never lags the code.

5. Do not edit `docs/API_MAP.md` by hand. If the generator output looks wrong,
investigate `utils/generate_api_map.py` and the source it scans, not the
generated file.

6. Do not commit. The user controls when to stage and commit.
84 changes: 84 additions & 0 deletions .agents/skills/review-pr/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
---
description: Screen a GitHub PR's review comments by invoking utils/ai_agent_github_reviews.py. Fetches inline threads and PR-level reviews via gh CLI, applies accepted edits as pending working-tree changes, resolves processed threads, and writes a Markdown summary tagged by PR number.
---

Process review comments on a GitHub PR using the project's PR-review driver.

$ARGUMENTS

## Usage shape

`$ARGUMENTS` is typically just a PR number, optionally followed by flags
forwarded to `utils/ai_agent_github_reviews.py`. Examples:

- `/review-pr 42`
- `/review-pr 42 --agent claude`
- `/review-pr 42 --since-last-push`
- `/review-pr 42 --prompt-only`
- `/review-pr 42 --no-resolve`

If `$ARGUMENTS` is empty, ask the user for the PR number before doing anything.

## Preconditions

1. Confirm `gh` is installed and authenticated:
```powershell
gh auth status
```
If not authenticated, stop and tell the user to run `gh auth login`.

2. Confirm the current branch is **not** `main` and not detached. If it is,
warn the user — applied edits will land on whichever branch is checked
out. Ask before proceeding.

3. Confirm the working tree has no unstaged Python edits the user would not
want mixed with applied review suggestions:
```powershell
git status --short
```
If there are pending changes, surface them and ask whether to continue.

## Run

Invoke the driver from the active `.\venv` (activate first if needed):

```powershell
python utils/ai_agent_github_reviews.py --pr <NUMBER> [flags...]
```

Defaults worth knowing:
- `--agent codex` is the script default. Pass `--agent claude` to drive
Claude Code instead.
- Without `--no-resolve`, every processed inline-comment thread is marked
resolved on GitHub after the agent finishes.
- With `--since-last-push`, only comments posted after this clone last saw
the remote PR head branch move are included.
- `--prompt-only` prints the prompt without invoking an agent or resolving
anything.

## After the run

1. Read `pr_<NUMBER>_review_summary.md` written to the repo root. Report:
- Total comments processed.
- APPLY / REVISE / REJECT counts.
- Any rejection that cited a hard rule from AGENTS.md.

2. Show the resulting working-tree diff so the user can decide what to keep:
```powershell
git diff --stat
git diff
```

3. Do **not** stage or commit. The script applies edits as pending
working-tree changes only; the user controls staging
(`git add -p`) and commit.

4. Delete `pr_<NUMBER>_review_summary.md` after reporting — the working-tree
diff is the durable record; the summary is intermediate state.

5. If the run failed:
- "gh CLI not found" → tell the user to install via
`winget install GitHub.cli` then `gh auth login`.
- "claude CLI not found" → tell the user to install via
`winget install Anthropic.ClaudeCode`, or retry with `--agent codex`.
- Any other error: surface the script's stderr verbatim and stop.
23 changes: 13 additions & 10 deletions .github/workflows/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -144,12 +144,13 @@ GPU tests require self-hosted runners with:

**Option 3: Run Locally**
```bash
# Install with CUDA support
uv pip install -e ".[test,cuda13]"
# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner).
# Requires Python >= 3.11 (nvidia-physicsnemo does not support 3.10).
uv pip install -e ".[test,cuda13,physicsnemo]"

# Run GPU tests
pytest tests/ -v -m "not slow"
pytest tests/ -v -m "slow" # For long-running tests
pytest tests/ -v --run-gpu
pytest tests/ -v --run-all # Match CI: enable every --run-* bucket
```

### GitHub-Hosted Runners
Expand Down Expand Up @@ -217,14 +218,16 @@ pytest tests/ -m "unit and not requires_gpu" --cov=physiomotion4d

### GPU Tests
```bash
# Install with CUDA support
uv pip install -e ".[test,cuda13]"
# Install with CUDA + PhysicsNeMo (matches the self-hosted GPU runner).
# Requires Python >= 3.11 (nvidia-physicsnemo does not support 3.10).
uv pip install -e ".[test,cuda13,physicsnemo]"

# Run all tests (including GPU)
pytest tests/ -m "not slow"
# Run GPU tests
pytest tests/ --run-gpu

# Run slow tests
pytest tests/ -m "slow"
# Enable every --run-* bucket at once (slow, GPU, simpleware,
# physicsnemo, experiments, tutorials)
pytest tests/ --run-all
```

## Coverage Reports
Expand Down
Loading
Loading