[maxkra15]

Description

Expose VBD rigid-contact history and contact-only stabilization, plus collision contact matching. Native defaults remain unchanged.

• Allocate standalone VBD history/contact capacity before solver construction and graph capture; reuse the existing initializer later.
• Clear shared matching at the common forward() reset boundary, including specialized solver hooks.
• Grow matching/sorting storage with solver-requested contact capacity. Fixes Newton contact buffer growth misses implicit deterministic sorting from contact matching #7783.
• Keep coupled private-pipeline lifecycle Newton-owned. Reject unsupported coupled rigid-history views; particle-only and external-rigid views remain accepted.

Related: #6733 and #7913. Coupled correspondence support is tracked in Newton #4532; broader allocation ordering is a separate follow-up in #8478. Neither requires a universal contacts-first rewrite in this PR.

Migration: none. New runtime dependencies: none. This PR is independent of the tablecloth example.

Type of change

• Non-breaking configuration additions
• Bug fix and documentation update

Release backport

• Backport after merging into develop

Validation

Merged upstream develop at 2224495df6d66cfe46c35fda6ed8926417f7f3e4 and resolved the four overlaps with merged #8360, preserving both sets of configuration fields and regression assertions. Validated merge commit 9fc59f9574d3f5b0f0c9cb9fe09e8ff88ba9e9f5 on an NVIDIA L40, pinned Newton 1.7.0.dev0 (5ea82eef6f7911388025a8df6ccdb172e2fed286) and Warp 1.17.0. Each file ran separately with uv run --frozen --no-sync python -m pytest -q:

Test file	Passed
source/isaaclab_newton/test/physics/test_vbd_core.py	11
source/isaaclab_newton/test/physics/test_newton_manager_abstraction.py	114
source/isaaclab_contrib/test/coupling/test_coupler.py	73
source/isaaclab_contrib/test/coupling/test_coupler_runtime.py	8

206 passed, zero failed or skipped. The capture/reset regression now proves that a prior contact match exists before checking reset. Existing upstream deprecation/fixture warnings remain.

ISAACLAB_CHANGELOG_BASE_REF=origin/develop UV_FROZEN=1 uv run --no-sync isaaclab -f passed, including the changelog gate. A clean warning-as-error documentation build passed using uv run --frozen --no-sync python -m sphinx -b html -W --keep-going -E -a -j 4 docs /tmp/vbd-history-merge-8107-20261013.TiDyS7/docs.

Earlier failure-sensitive mutations for allocation, capacity and specialized MPM reset are documented in the addressed review threads; they are not counted as extra tests in the total above. New-head CI is requested separately.

Checklist

• Read the contribution guide and followed package conventions.
• Ran formatting/lint, focused tests and the test-audit gate.
• Updated documentation and package changelog fragments.
• Existing entry in CONTRIBUTORS.md.

[greptile-apps]

Confidence Score: 4/5

[High risk] Adds contact history and stabilization to the physics solver.

The PR is not yet safe to merge because proxy-local contact matches can still survive environment resets.

Findings

1.  Coupled VBD skips matching check ▶
2.  Proxy contact matches survive resets ▶
3.  Particle-only VBD entries rejected ▶

Summary

The PR exposes VBD rigid-contact history and contact stabilization, adds collision-pipeline matching and reset handling, and prevents unsupported rigid-contact history in coupled entries.

• Standalone VBD constructs matching resources before solver creation.
• Coupled-entry validation is broader than its documented rigid-body restriction.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  C[VBD configuration] --> S{Standalone or coupled?}
  S -->|Standalone rigid VBD| P[Matching collision pipeline]
  P --> H[Construct solver with contact history]
  S -->|Coupled entry| V[Validate entry history setting]
  V --> E[Construct entry solver]

Loading

_{Reviews (3) · Last reviewed commit: "Fix VBD contact matching lifecycle"}

[isaaclab-review-bot]

Isaac Lab Review Bot

The change additively exposes VBD rigid-contact history and contact stabilization, introduces collision-pipeline contact matching, and resets matching state for reset worlds. The documented defaults preserve existing behavior, and the supplied test covers matching, solver history allocation, and reset behavior.

• Design and architecture: The solver configuration, VBD manager, and collision pipeline retain their existing ownership boundaries. The history-enabled standalone VBD path initializes contacts before solver construction to satisfy the allocation-order requirement; the later base lifecycle call creates some ordering coupling, but the supplied evidence does not establish that it rebuilds or invalidates those resources.
• API: The new public fields are additive and default to Newton-compatible disabled or unset values, preserving existing configurations and coupled/external-rigid paths. Their requirements and behavior are documented, and the user-visible change has an appropriate package changelog fragment.
• Implementation: The implementation validates that standalone VBD contact history has a matching-enabled collision configuration, initializes matching before constructing VBD, and clears matching state for reset worlds. The focused regression test exercises the new configuration forwarding, matching behavior, history capacity, and reset clearing. The candidate concern about duplicate initialization depends on unprovided behavior of _initialize_contacts and does not demonstrate a concrete defect.

No blocking issues. No inline issue met the actionable-evidence threshold; the assessment above records the review feedback.

Automated review; human maintainers own approval decisions.

[maxkra15]

run-ci

[AntoineRichard]

🤖 AI-assisted review — generated with Claude Code against the repo's test-audit and coding-style skills and the contributing guide; reviewed and posted by @antoiner.

The production change looks good to me: the matching mode is a typed Literal, a misconfiguration fails hard instead of warning, and matching is reset once in base forward(). Most comments are about the tests (~130 test lines for ~60 production lines, with the same contract asserted in three places) and about trimming prose.

Tests

1. test_vbd_contact_history_initializes_matches_and_resets covers five contracts: the validation error, forwarding of alpha and history, matching, the capture step, and the reset. It does this through ~15 monkeypatches of NewtonManager private state. I'd suggest splitting it by owner. Alpha and history forwarding could be one more overrides row in test_vbd_rigid_solver_controls, with no model needed. One integration test would then cover "history preallocated before capture → reset clears matches".
2. The capacity test in test_newton_manager_abstraction.py and the VBD test build the same sphere-on-ground scene, call collide twice and assert rigid_contact_match_index >= 0. Please keep the matching assertion in one of them only. Replacing the fake pipeline with a real CPU model is a nice improvement.
3. The MPM forward() test now also covers the matching reset through a _RecordingPipeline mock, and its docstring has changed meaning. The VBD test already covers the reset with a real pipeline, so I'd revert this test to develop.
4. test_coupler.py: the passing row adds if error_type is None: return inside a parametrized test and renames the test. Please drop the row and keep the original name. If you keep the row, add a one-line justification (it guards the not integrate_with_external_rigid_solver clause).

Code
5. coupler.py: as Greptile noted, a particle-only VBD entry with rigid_contact_history=True is rejected even though it has no rigid contacts. Please fix this or list it under known limitations.
6. forward(): if Newton's reset_contact_matching is already a no-op when matching is disabled, the != "disabled" clause can go.
7. vbd_manager._build_solver now calls _initialize_contacts() before the solver exists. Please add a sentence to the PR body explaining why (Newton sizes history from the pipeline when it constructs VBD) and why the later lifecycle call is a no-op.

Docs / changelog
8. The coupler paragraph in tune_vbd.rst repeats the table rows and docstrings. I'd keep the table rows and the code block and cut the paragraph to one sentence. The cfg docstrings could also be 1–2 lines each.
9. isaaclab_contrib/changelog.d/…fixed.rst: nothing was broken before, since this restricts a field added in this PR. .changed fits better, or fold it into the newton .added fragment and give contrib a .skip.

PR shape

• Title: add the area prefix, e.g. [Newton] Expose VBD contact history and contact stabilization.
• Body: add Migration: none. Dependencies: none. Also add a Known limitations section covering:
  • coupled rigid entries can't use history;
  • the coupler's entry-local pipelines aren't reset by forward() (Greptile P1, still open);
  • particle-only entries are rejected.
• Validation: replace "Focused Newton manager and coupler reset tests — 14 passed" with the exact uv run --frozen python -m pytest … commands.

Scope is otherwise one concern, with no lockfile churn. 👍

[AntoineRichard]

🤖 AI-assisted review (follow-up): test trims suggested by Claude Code, applying the repo's test-audit skill. Posted by @antoiner.

Concrete trims that would roughly halve the test diff (~130 → ~60 lines) without dropping any contract:

1. test_vbd_core.py::test_vbd_contact_history_initializes_matches_and_resets (~75 → ~35 lines)
   • Move the rigid_avbd_contact_alpha / rigid_contact_history forwarding into test_vbd_rigid_solver_controls as one more overrides row. That needs no model or monkeypatching, and the solver-attribute asserts at L165 can go.
   • Drop the first collide loop and its match_index >= 0 assert (L168–172), since the capacity test already covers matching after two collides.
   • Keep only what this PR uniquely owns: the solver is built with history before capture, and forward() leaves match_index < 0.
   • Drop the "body rose after one step" assert, or keep it only for the CUDA capture case. It's weak and not what the PR is about.
2. test_newton_manager_abstraction.py::test_collision_pipeline_matches_expanded_contact_capacity: keep both rows and the real model, but drop the collide loop and match-index assert (~8 lines). rigid_contact_max == 2 is the contract here. If you want one "matching works at the expanded capacity" check, keep it here and remove it from the VBD test.
3. test_forward_dispatches_active_mpm_reset_hook_through_base_manager: revert to develop (net ~−13 lines). The VBD test already proves the reset with a real pipeline, so the _RecordingPipeline mock isn't needed.
4. test_coupler.py: keep the new rejecting row, but drop the passing integrate_with_external_rigid_solver=True row, the if error_type is None branch, and the rename (net ~−8 lines).
5. Devices: if the CUDA row exists only to exercise graph capture, use test_devices(DeviceScope.CUDA) and remove the if is_cuda / else branch. That gives one row and no branching inside the test.

[maxkra15]

Addressed the review in 88a4b657b and merged the latest develop (a7886e1fd) in f0e800581.

• Moved coupled-history validation to the resolved entry view. Particle-only VBD now runs; owned and received rigid bodies remain protected.
• Consolidated forwarding into the existing overrides test, removed duplicate matching assertions, scoped capture to CUDA, and restored the error-only coupling test.
• Shortened the docstrings and tuning-page paragraph, changed the contrib fragment to .changed, and updated the title, allocation-order explanation, limitations, and exact validation commands.

I retained two small checks after mutation testing: a real collision is needed to catch undersized sorting storage even when rigid_contact_max == 2, and the specialized MPM reset assertion catches a bypass that the real VBD test does not. The reset test also retains its two collision calls to establish prior matches before clearing them.

Newton 1.6.1 already resets private proxy pipelines. A real CPU/CUDA sticky-pipeline check confirmed matched contacts become unmatched after public coupled reset, so no duplicate Isaac Lab traversal was added.

After the merge and dependency update: 203 tests passed across the four separately run files; the clean documentation build, formatting, and changelog validation passed. Inline threads include the individual fixes and evidence.

[maxkra15]

run-ci

[mmichelis]

Looks good! A few questions I left that I would be curious to hear your answers to.

[maxkra15]

run-ci

[maxkra15]

run-ci

[maxkra15]

run-ci