Skip to content

Commit 36d752a

Browse files
authored
ENH: Upaded AI agents for consistency and make physicsnemo optional (#60)
* ENH: Upaded AI agents for consistency and make physicsnemo optional * BUG: Updated from review-pr skill * STYLE: Typo in .gitignore
1 parent 7c9a379 commit 36d752a

29 files changed

Lines changed: 797 additions & 201 deletions

.agents/agents/architecture.md

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,18 @@ Use `docs/API_MAP.md` to locate classes and signatures without manual searching.
2727

2828
## Design invariants to preserve
2929

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

3643
## Output format — always produce all six sections
3744

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

45-
Flag any change at the ITK↔PyVista boundary or the RAS→Y-up transform as **high-risk**.
52+
Flag any change at the ITK↔PyVista boundary or the LPS→USD-Y-up transform as **high-risk**.

.agents/agents/implementation.md

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,18 +26,32 @@ Key modules: `physiomotion4d_base.py`, `segment_chest_*.py`, `register_images_*.
2626

2727
## Code rules
2828

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

3648
## Data shapes — state them explicitly
3749

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

4357
## What not to do

.agents/agents/testing.md

Lines changed: 49 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,45 +1,68 @@
11
---
22
name: PhysioMotion4D Testing Agent
3-
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.
3+
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.
44
tools: Read, Edit, Write, Bash, Glob, Grep
55
---
66

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

1011
## Test architecture
1112

1213
- `tests/conftest.py` — session-scoped fixtures chaining: download → convert → segment → register
1314
- `tests/baselines/` — stored via Git LFS; fetch with `git lfs pull`
14-
- `src/physiomotion4d/test_tools.py` — baseline comparison utilities
15+
- `src/physiomotion4d/test_tools.py` — baseline comparison utilities (`TestTools`)
1516
- Markers (all opt-in via `--run-<bucket>`): `slow`, `requires_gpu`,
16-
`requires_simpleware`, `experiment`, `tutorial`. Tests that need
17-
downloadable data fetch it through the session fixtures and run by default.
18-
19-
## Run commands (use `py`, not `python`)
20-
21-
```bash
22-
py -m pytest tests/ -v # fast, recommended (slow/GPU/etc auto-skipped)
23-
py -m pytest tests/test_contour_tools.py -v # single file
24-
py -m pytest tests/test_contour_tools.py::TestContourTools -v # single class
25-
py -m pytest tests/ -v --run-slow # opt into slow tests
26-
py -m pytest tests/ -v --run-gpu --run-slow # typical local GPU profile (CI runner adds --run-simpleware --run-experiments --run-tutorials)
27-
py -m pytest tests/ --create-baselines # create missing baselines
17+
`requires_simpleware`, `experiment`, `tutorial`. The `requires_data` marker
18+
no longer exists — tests that need downloadable data pull it through the
19+
session fixtures and run by default.
20+
21+
## Run commands
22+
23+
Activate `.\venv` first (`.\venv\Scripts\Activate.ps1`); `python` then resolves
24+
to the project interpreter. If activation is impossible, use
25+
`.\venv\Scripts\python.exe -m ...` directly.
26+
27+
```powershell
28+
python -m pytest tests/ -v # fast, recommended (slow/GPU/etc auto-skipped)
29+
python -m pytest tests/test_contour_tools.py -v # single file
30+
python -m pytest tests/test_contour_tools.py::TestContourTools -v # single class
31+
python -m pytest tests/ -v --run-slow # opt into slow tests
32+
python -m pytest tests/ -v --run-gpu --run-slow # typical local GPU profile (CI runner adds --run-simpleware --run-experiments --run-tutorials)
33+
python -m pytest tests/ --create-baselines # create missing baselines
2834
```
2935

3036
## Writing tests — rules
3137

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

4467
## Naming
4568

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
---
2+
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.
3+
---
4+
5+
Audit PhysioMotion4D source for hard-rule violations.
6+
7+
$ARGUMENTS
8+
9+
By default, audit every Python file modified since `HEAD`. If $ARGUMENTS names
10+
specific files or directories, audit those instead.
11+
12+
## Determine the file set
13+
14+
```powershell
15+
# Default: changed, non-deleted .py files since HEAD
16+
git diff --diff-filter=d --name-only HEAD -- '*.py'
17+
```
18+
19+
If a path was passed in $ARGUMENTS, expand it to all `.py` files under that
20+
path (recursively). Skip files that no longer exist on disk.
21+
22+
## Rules to check
23+
24+
For each file, read the **entire file** (rules below depend on surrounding
25+
context such as class inheritance), then flag every occurrence of:
26+
27+
### Base class and logging
28+
- [ ] A class that orchestrates workflow / segmentation / registration / USD
29+
conversion but does **not** inherit from `PhysioMotion4DBase`.
30+
- [ ] A `print(` call inside the body of a class that inherits from
31+
`PhysioMotion4DBase` (it must use `self.log_info()` / `self.log_debug()`).
32+
Standalone scripts and helper / data-container classes may use `print()`.
33+
34+
### USD / coordinate conventions
35+
- [ ] An `import` of `physiomotion4d.vtk_to_usd` (or `from ... vtk_to_usd ...`)
36+
from a file that is **not** `src/physiomotion4d/convert_vtk_to_usd.py`
37+
and is **not** itself inside `src/physiomotion4d/vtk_to_usd/`.
38+
Experiments, CLIs, tests, and tutorials must use `ConvertVTKToUSD`.
39+
- [ ] A docstring or comment claiming PyVista surfaces are in **RAS** — they
40+
are in **LPS** internally; convert to USD Y-up only at export.
41+
42+
### Windows multiprocessing
43+
- [ ] A module-level instantiation of `SegmentChestTotalSegmentator` (or a
44+
module-level call into it) that is not guarded by
45+
`if __name__ == "__main__":`. Required on Windows because
46+
`torch.multiprocessing` re-imports the module in child workers.
47+
48+
### Code style
49+
- [ ] `X | None` in a type hint (use `Optional[X]`; ruff `UP007` is suppressed).
50+
- [ ] `Any` in a public signature without a comment explaining why.
51+
- [ ] A docstring delimited with `'''` (use `"""`).
52+
- [ ] A string literal using `"..."` for ordinary inline strings (use `'...'`;
53+
docstrings stay on `"""`).
54+
- [ ] A line longer than 88 characters.
55+
- [ ] An emoji or other non-ASCII glyph inside a `.py` file (Windows cp1252
56+
encoding has broken builds; keep emojis out of source).
57+
58+
### Public API hygiene
59+
- [ ] A public method (no leading underscore) without a NumPy-style docstring.
60+
- [ ] An array / image parameter or return value whose docstring does not
61+
state shape and axis order.
62+
63+
## Output
64+
65+
Group findings by file. For each finding, print:
66+
67+
```text
68+
<path>:<line> <rule short name> <one-line excerpt>
69+
```
70+
71+
End with a one-line summary: total findings per rule category.
72+
73+
Do **not** auto-fix. The point is to surface violations the user can decide
74+
how to address. If `$ARGUMENTS` includes `--fix`, ask before mutating anything
75+
and limit fixes to the trivially mechanical rules (line length is not one of
76+
them — `ruff format` covers that).
Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
---
2+
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.
3+
---
4+
5+
Regenerate the PhysioMotion4D API map and report what changed.
6+
7+
$ARGUMENTS
8+
9+
Instructions:
10+
11+
1. Run the generator from the active `.\venv` (activate it first if needed,
12+
or call `.\venv\Scripts\python.exe` directly):
13+
14+
```powershell
15+
python utils/generate_api_map.py
16+
```
17+
18+
2. Inspect the diff:
19+
20+
```powershell
21+
git diff -- docs/API_MAP.md
22+
```
23+
24+
3. Report one of:
25+
- **No change** — public API surface is identical; nothing to commit.
26+
- **Changed** — list each added, removed, or signature-modified entry
27+
(class.method, file, one-line summary). Group by file.
28+
29+
4. If `docs/API_MAP.md` changed, remind the user to stage it together with
30+
the source changes that produced it so the API map never lags the code.
31+
32+
5. Do not edit `docs/API_MAP.md` by hand. If the generator output looks wrong,
33+
investigate `utils/generate_api_map.py` and the source it scans, not the
34+
generated file.
35+
36+
6. Do not commit. The user controls when to stage and commit.

.agents/skills/review-pr/SKILL.md

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
---
2+
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.
3+
---
4+
5+
Process review comments on a GitHub PR using the project's PR-review driver.
6+
7+
$ARGUMENTS
8+
9+
## Usage shape
10+
11+
`$ARGUMENTS` is typically just a PR number, optionally followed by flags
12+
forwarded to `utils/ai_agent_github_reviews.py`. Examples:
13+
14+
- `/review-pr 42`
15+
- `/review-pr 42 --agent claude`
16+
- `/review-pr 42 --since-last-push`
17+
- `/review-pr 42 --prompt-only`
18+
- `/review-pr 42 --no-resolve`
19+
20+
If `$ARGUMENTS` is empty, ask the user for the PR number before doing anything.
21+
22+
## Preconditions
23+
24+
1. Confirm `gh` is installed and authenticated:
25+
```powershell
26+
gh auth status
27+
```
28+
If not authenticated, stop and tell the user to run `gh auth login`.
29+
30+
2. Confirm the current branch is **not** `main` and not detached. If it is,
31+
warn the user — applied edits will land on whichever branch is checked
32+
out. Ask before proceeding.
33+
34+
3. Confirm the working tree has no unstaged Python edits the user would not
35+
want mixed with applied review suggestions:
36+
```powershell
37+
git status --short
38+
```
39+
If there are pending changes, surface them and ask whether to continue.
40+
41+
## Run
42+
43+
Invoke the driver from the active `.\venv` (activate first if needed):
44+
45+
```powershell
46+
python utils/ai_agent_github_reviews.py --pr <NUMBER> [flags...]
47+
```
48+
49+
Defaults worth knowing:
50+
- `--agent codex` is the script default. Pass `--agent claude` to drive
51+
Claude Code instead.
52+
- Without `--no-resolve`, every processed inline-comment thread is marked
53+
resolved on GitHub after the agent finishes.
54+
- With `--since-last-push`, only comments posted after this clone last saw
55+
the remote PR head branch move are included.
56+
- `--prompt-only` prints the prompt without invoking an agent or resolving
57+
anything.
58+
59+
## After the run
60+
61+
1. Read `pr_<NUMBER>_review_summary.md` written to the repo root. Report:
62+
- Total comments processed.
63+
- APPLY / REVISE / REJECT counts.
64+
- Any rejection that cited a hard rule from AGENTS.md.
65+
66+
2. Show the resulting working-tree diff so the user can decide what to keep:
67+
```powershell
68+
git diff --stat
69+
git diff
70+
```
71+
72+
3. Do **not** stage or commit. The script applies edits as pending
73+
working-tree changes only; the user controls staging
74+
(`git add -p`) and commit.
75+
76+
4. Delete `pr_<NUMBER>_review_summary.md` after reporting — the working-tree
77+
diff is the durable record; the summary is intermediate state.
78+
79+
5. If the run failed:
80+
- "gh CLI not found" → tell the user to install via
81+
`winget install GitHub.cli` then `gh auth login`.
82+
- "claude CLI not found" → tell the user to install via
83+
`winget install Anthropic.ClaudeCode`, or retry with `--agent codex`.
84+
- Any other error: surface the script's stderr verbatim and stop.

.github/workflows/README.md

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,13 @@ GPU tests require self-hosted runners with:
144144

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

150151
# Run GPU tests
151-
pytest tests/ -v -m "not slow"
152-
pytest tests/ -v -m "slow" # For long-running tests
152+
pytest tests/ -v --run-gpu
153+
pytest tests/ -v --run-all # Match CI: enable every --run-* bucket
153154
```
154155

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

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

223-
# Run all tests (including GPU)
224-
pytest tests/ -m "not slow"
225+
# Run GPU tests
226+
pytest tests/ --run-gpu
225227

226-
# Run slow tests
227-
pytest tests/ -m "slow"
228+
# Enable every --run-* bucket at once (slow, GPU, simpleware,
229+
# physicsnemo, experiments, tutorials)
230+
pytest tests/ --run-all
228231
```
229232

230233
## Coverage Reports

0 commit comments

Comments
 (0)