Fix Newton mock missing articulation_ids and run all CI tests on every PR

[nvsekkin]

Description

• Run every test job on every PR: Removed the detect-changes job and the per-package if: gating in
  .github/workflows/build.yaml.
• Fix missing articulation_ids on Newton mock views: Added articulation_ids as a lazy wp.array2d(dtype=int) on both mock views, matching the contract (world_count, count_per_world) consumed by _or_reset_masks_from_mask / _scatter_reset_masks_from_ids.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR adds the missing articulation_ids property to both MockNewtonCollectionView and MockNewtonArticulationView classes in the Newton mock interfaces. The implementation follows the existing lazy initialization pattern and correctly shapes the ID arrays according to each class's conventions. This is a straightforward bug fix to make the mock interface complete.

Architecture Impact

Self-contained. This change only affects test mock classes within isaaclab_newton/test/. The articulation_ids property is expected by code that uses the Newton articulation view interface. Without this property, tests that access articulation_ids on these mock objects would fail with AttributeError. The fix ensures the mock interface matches the real Newton API contract.

Implementation Verdict

Ship it — The implementation is correct and follows established patterns in the codebase.

Test Coverage

The PR description acknowledges "I have added tests that prove my fix is effective" is unchecked. However, this is acceptable because:

1. This is a mock class used for testing — it doesn't require its own unit tests
2. The fix addresses CI failures, meaning existing tests exercise this code path
3. The CI passing (once available) will serve as the regression test

CI Status

No CI checks available yet. The PR claims it fixes failing CI — verification should wait for CI results to confirm the fix is complete.

Findings

🔵 Improvement: mock_articulation_view.py:75 — Minor dtype inconsistency
The articulation_ids property uses dtype=int when creating the warp array, while the numpy array uses np.int32. This works because warp's int maps to int32 on most platforms, but for explicit clarity and consistency with the numpy dtype, consider using dtype=wp.int32:

self._articulation_ids = wp.array(ids_np, dtype=wp.int32, device=self._device)

Same applies to line 300. This is cosmetic — the current code is functionally correct.

🔵 Improvement: mock_articulation_view.py:71-78 and 295-302 — Code duplication opportunity
Both MockNewtonCollectionView.articulation_ids and MockNewtonArticulationView.articulation_ids follow nearly identical patterns. If more properties need to be shared in the future, consider extracting a common base class. For now, this is acceptable given the PR's scope.

The implementation is correct:

• MockNewtonCollectionView correctly shapes IDs as (N, B) matching its multi-body convention
• MockNewtonArticulationView correctly shapes IDs as (N, 1) matching its single-articulation convention
• Both use lazy initialization consistent with other properties in these classes
• The sequential ID assignment (np.arange) is reasonable for mock data

[greptile-apps]

Greptile Summary

This PR adds the missing articulation_ids property to both MockNewtonCollectionView and MockNewtonArticulationView in the Newton test mock interfaces, fixing CI test failures. The new properties follow the existing lazy-initialisation pattern: a None-initialised backing field is populated on first access using np.arange reshaped to (N, B) and (N, 1) respectively, then wrapped in a wp.array.

Confidence Score: 5/5

Safe to merge — changes are confined to test mocks with no impact on production code paths.

Only P2 style findings (dtype=int vs wp.int32). The fix is minimal, well-scoped, and consistent with existing patterns in the file.

No files require special attention.

Important Files Changed

Filename	Overview
source/isaaclab_newton/isaaclab_newton/test/mock_interfaces/views/mock_articulation_view.py	Adds missing articulation_ids property (with lazy-init backing field) to both MockNewtonCollectionView and MockNewtonArticulationView; logic and shapes match documented conventions.

Class Diagram

%%{init: {'theme': 'neutral'}}%%
classDiagram
    class MockNewtonCollectionView {
        -_articulation_ids: wp.array | None
        +articulation_ids() wp.array
        +count() int
        +body_names() list~str~
    }
    class MockNewtonArticulationView {
        -_articulation_ids: wp.array | None
        +articulation_ids() wp.array
        +count() int
        +link_count() int
        +joint_dof_count() int
    }
    note for MockNewtonCollectionView "articulation_ids shape: (N, B) — arange(N*B).reshape(N,B)"
    note for MockNewtonArticulationView "articulation_ids shape: (N, 1) — arange(N).reshape(N,1)"

Loading

_{Reviews (1): Last reviewed commit: "Add missing mock property" | Re-trigger Greptile}

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Follow-up Review

The new commit removes the detect-changes job and all associated conditional logic from the CI workflow, making all test jobs run on every PR. This is a significant simplification that eliminates ~250 lines of complex change detection bash scripting.

Previous concerns status: The minor dtype suggestion from the previous review was cosmetic and remains unaddressed, which is fine — the code is functionally correct.

New changes assessment: The workflow simplification is clean and correct. The needs: dependencies are properly updated to remove detect-changes, and all if: conditions now simply check needs.build.result == 'success'. This trades CI efficiency (running all tests always) for reliability (no risk of change detection bugs missing necessary tests).

Implementation Verdict: Ship it — Both the mock property fix and the CI simplification are correct.