[peterd-NV]

Summary

Adds unit tests for isaac_autodata_core classes and modules. Test included for:

• algorithms
• data_generator
• datagen_info
• pool
• selection_strategy
• transforms
• waypoint

Summary by CodeRabbit

• Bug Fixes
  • Improved coordinated motion delta/pose calculations to handle rotated object frames correctly.
  • Refined optional noise behavior so translations and rotations are perturbed consistently while keeping rotations valid.
• Tests
  • Added/expanded unit test coverage for algorithms, data generation validation and immutability, datagen info serialization, demo pools, selection strategies, pose transforms, and waypoint/trajectory handling.
  • Updated CI to include the core test suite in premerge runs.
• Documentation
  • Clarified CI test coverage details in the README.

[hguillen]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[njawale42]

I have reviewed the resolutions to Stephan's comments. Lgtm!

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: a8a61646-aa73-4673-be8a-d3a1c07725f0

📥 Commits

Reviewing files that changed from the base of the PR and between 265930a and 2f95bae.

📒 Files selected for processing (2)

• .github/workflows/ci.yml
• README.md

---

📝 Walkthrough

Walkthrough

Pose transform composition and noise handling are updated. New unit tests cover algorithms, data generation, metadata, pooling, selection strategies, transforms, waypoint trajectories, and CI test-suite wiring.

Changes

Core validation and transform coverage

Layer / File(s)	Summary
Pose delta and noise transforms isaac_autodata_core/transforms.py, isaac_autodata_tests/core/test_transforms.py	Uses explicit inverse-pose composition, adds local uniform translation and Euler rotation noise, and tests transform, noise, and error-handling behavior.
Algorithm and generator contracts isaac_autodata_tests/core/__init__.py, isaac_autodata_tests/core/test_algorithms.py, isaac_autodata_tests/core/test_data_generator.py	Tests algorithm registries, construction attributes, DataGenerator validation, SkillGen planner requirements, and frozen GenerationResult fields.
DatagenInfo serialization and ownership isaac_autodata_tests/core/test_datagen_info.py	Tests defaults, serialized fields and key names, copy semantics, reference storage, and deep-copy behavior.
DataGenInfoPool ingestion and boundaries isaac_autodata_tests/core/test_pool.py	Tests signal tables, per-EEF boundary extraction, passthrough actions, missing data, and invalid offset handling across supported algorithms.
Selection strategy behavior isaac_autodata_tests/core/test_selection_strategy.py	Tests strategy registration, construction, random selection bounds, nearest-neighbor behavior, and object-frame transformation order.
Waypoint and trajectory operations isaac_autodata_tests/core/test_waypoint.py	Tests waypoint sequences, copying, concatenation, splitting, interpolation, merging, fixed holds, indexing, and flattening.
CI test scope .github/workflows/ci.yml, README.md	Includes core unit tests in the premerge CI path and documents the expanded test scope.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description only lists covered modules and omits the required Detailed description with reason, changes, and impact.	Add a Detailed description section covering why the change was needed, what changed, and the impact of the new tests.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: adding unit tests for isaac_autodata_core.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch peterd/core_tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@isaac_autodata_tests/core/test_algorithms.py`:
- Around line 39-41: Update the `@pytest.mark.parametrize` decorator on
test_get_algorithm_no_kwargs to pass argnames as the tuple ("name", "cls")
instead of the comma-separated string "name, cls", keeping the existing
parameter values unchanged.

In `@isaac_autodata_tests/core/test_data_generator.py`:
- Around line 29-44: Add concrete return type annotations to
_MockDatastream.get_eef_names, get_subtasks, and get_task_constraints, using
types consistent with their returned values and the subtasks_by_eef structure,
so the mock interface satisfies Ruff ANN202.

In `@isaac_autodata_tests/core/test_selection_strategy.py`:
- Around line 42-45: Add an assertion to test_make_returns_registered_instance()
verifying that make_selection_strategy("nearest_neighbor_robot_distance")
returns an instance of its corresponding registered strategy class, covering the
third factory registration.

In `@isaac_autodata_tests/core/test_waypoint.py`:
- Around line 41-58: Add a regression test in the sequence-from-poses tests
using action_noise=torch.tensor(0.1) with multiple poses, asserting every
waypoint receives scalar noise. Update WaypointSequence.from_poses to detect
zero-dimensional tensor noise and broadcast or expand it to the trajectory
length before indexing, while preserving existing float and per-step tensor
behavior.
- Around line 173-203: Add content assertions to the three tests: in
test_trajectory_merge_without_bridge_concatenates, assert X positions are [0.0,
1.0]; in test_trajectory_merge_with_fixed_hold, assert they are [0.0, 1.0, 1.0,
1.0, 2.0]; and in test_trajectory_get_full_sequence_flattens, assert they are
[0.0, 1.0, 2.0], using each waypoint’s position data.
- Line 138: Annotate the nested helper function `_wp_x` with an explicit return
type, such as `-> float`, matching the value it returns and satisfying the
repository’s type-checking rules.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: d27f89c3-898c-4d84-966b-b10502a6db5f

📥 Commits

Reviewing files that changed from the base of the PR and between 889a732 and 265930a.

📒 Files selected for processing (9)

• isaac_autodata_core/transforms.py
• isaac_autodata_tests/core/__init__.py
• isaac_autodata_tests/core/test_algorithms.py
• isaac_autodata_tests/core/test_data_generator.py
• isaac_autodata_tests/core/test_datagen_info.py
• isaac_autodata_tests/core/test_pool.py
• isaac_autodata_tests/core/test_selection_strategy.py
• isaac_autodata_tests/core/test_transforms.py
• isaac_autodata_tests/core/test_waypoint.py