[Task Clean-up][Newton] Dexterous Part 1/8: Fix cubric fallback and visualizer teardown

[hujc7]

Review Map

• Exact changes: a stacked PR's page shows the cumulative diff of its dependency chain; the link pins the commit range that is the PR's own contribution.
• Links pin specific SHAs and can go stale after a branch update — the table on [DO-NOT-MERGE][Task Clean-up] Dexterous: lumped validation branch (series reference) #6324 is refreshed first.

PR	Status	Depends on	Exact changes
#6410 [Docs] Environment overview regen		—	—
📌 #6411 Part 1/8: Newton cloner/cubric/visualizer fixes (this PR)		—	—
#6412 Part 2/8: OVPhysX articulation + manager runtime		—	—
#6413 Part 3/8: Reorient Direct, torch		—	—
#6414 Part 4/8: MARL-to-single-agent fix + handover/camera Direct		#6413	changes
#6418 Part 5/8: Reorient manager counterparts		#6413	changes
#6421 Part 6/8: Handover + camera manager counterparts		#6413, #6414, #6418	changes
#6415 Part 7/8: Benchmark success-rate utilities + docs		#6413, #6414, #6418, #6421	changes
#6582 Part 8/8: Warp variants → experimental (draft; merges last)		#6413	changes
#6324 [DO-NOT-MERGE] Lumped validation reference		ALL	—

Summary

• Fixes Newton cloner label rows, the cubric IAdapter version audit (exact-match fallback to the CPU hierarchy path), and visualizer teardown.
• Retains an in-tree ignore_paths workaround for custom-frequency USD traversal; it becomes redundant once the Newton pin advance (Pin Newton to v1.4.0 and override the Isaac Sim MuJoCo pins #6584) merges — this PR then only needs a rebase.

Stacking

• Independent; based on develop.

Review history

• Approved. The Newton pin + MuJoCo overrides were split out to Pin Newton to v1.4.0 and override the Isaac Sim MuJoCo pins #6584 via revert commits (2026-07-17) so this PR's CI runs against develop's pins.

[greptile-apps]

Greptile Summary

This PR fixes three Newton backend bugs found during dexterous training: (1) empty MuJoCo custom-frequency rows created from ignored environment subtrees during USD import, (2) detached articulation links caused by accepting unvalidated cubric IAdapter minor versions, and (3) crashes during interpreter shutdown when close() re-queried a potentially-torn-down SimulationContext.

• Cloner fix: _add_global_stage_to_builder monkey-patches builder.add_custom_frequency during add_usd to scope MuJoCo-frequency prim filters to exclude ignored subtrees; a try/finally block guarantees filter restoration on failure.
• Cubric fix: _verify_iadapter_version now requires an exact version match (minor != _IA_EXPECTED_MINOR) instead of the previous semver-compatible check (minor < _IA_EXPECTED_MINOR), since higher minor versions produced incorrect transform behavior with Isaac RTX.
• Visualizer fix: NewtonVisualizationMarkers caches the registry reference at __init__ time so close() never calls SimulationContext.instance(); double-close is made idempotent. All three fixes have dedicated regression tests.

Confidence Score: 4/5

Safe to merge; all three fixes are targeted, regression-tested, and have been validated by full dexterous training runs.

The three fixes are well-scoped and backed by dedicated tests. The only notable concern is in _add_global_stage_to_builder, where re.compile(path).match(prim_path) is used for path filtering without anchoring at the end or escaping metacharacters — this could cause /World/envs to mistakenly exclude prims under a path like /World/envs_global. In the current usage patterns of IsaacLab this doesn't trigger, but it's a subtle correctness gap worth addressing.

source/isaaclab_newton/isaaclab_newton/cloner/newton_clone_utils.py — the regex path filter in _scope_filter deserves a second look for edge-case scenes with prim paths that share a prefix with the ignored subtrees.

Important Files Changed

Filename	Overview
source/isaaclab_newton/isaaclab_newton/cloner/newton_clone_utils.py	Adds _add_global_stage_to_builder which patches builder.add_custom_frequency to scope MuJoCo-frequency filters during USD import; restore logic via try/finally is correct, but the regex-based path filter uses re.match without escaping or end-anchoring which can over-exclude prims.
source/isaaclab_newton/isaaclab_newton/cloner/replicate.py	Swaps builder.add_usd for _add_global_stage_to_builder to fix empty custom-frequency rows from ignored env subtrees; straightforward call-site change.
source/isaaclab_newton/isaaclab_newton/physics/_cubric.py	Tightens IAdapter version check from semver-compatible (minor >=) to exact-match (minor ==), removing the proceed-under-semver-contract path that produced detached articulation links with Isaac RTX.
source/isaaclab_newton/isaaclab_newton/physics/newton_manager.py	Applies the same _add_global_stage_to_builder fix for the non-clone single-builder path; the import is done inline inside the else block rather than at the module top.
source/isaaclab_visualizers/isaaclab_visualizers/newton/newton_visualization_markers.py	Caches vis_marker_registry at init time instead of re-querying SimulationContext during close(), preventing AttributeError when module globals are torn down during interpreter shutdown; double-close is also made idempotent.
source/isaaclab_newton/test/cloner/test_rename_builder_labels.py	Adds TestIgnoredCloneCustomFrequencies with two tests: one verifying tendon rows are not created from ignored env subtrees, another verifying filter restoration after import failure; coverage is thorough.
source/isaaclab_newton/test/physics/test_cubric.py	New regression test for the exact-match version change; constructs an in-memory ctypes plugin descriptor and asserts that v0.2 is rejected while v0.1 is accepted.
source/isaaclab_visualizers/test/test_newton_adapter.py	Adds test_newton_marker_close_does_not_query_simulation_context verifying the shutdown-safe close() path; double-close and idempotency are both exercised.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_add_global_stage_to_builder(builder, stage, ignore_paths, schema_resolvers)"]
    A --> B["Snapshot original usd_prim_filter\nfor each custom_frequency"]
    B --> C["Wrap each existing filter\nwith _scope_filter (ignores ignored_paths)"]
    C --> D["Intercept builder.add_custom_frequency\nvia instance setattr"]
    D --> E["builder.add_usd(stage, ignore_paths,\nschema_resolvers)"]
    E -->|New frequency registered| F["_add_scoped_custom_frequency:\nwrap filter, record original"]
    E -->|Existing frequency re-registered| G["Check callbacks match;\nreturn early if same"]
    E -->|Import error| H["Exception propagates to finally"]
    E --> I["finally block:\nrestore all usd_prim_filter refs\ndelete instance override"]
    H --> I
    F --> I
    G --> I
    I --> J["Return stage_info"]

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A["_add_global_stage_to_builder(builder, stage, ignore_paths, schema_resolvers)"]
    A --> B["Snapshot original usd_prim_filter\nfor each custom_frequency"]
    B --> C["Wrap each existing filter\nwith _scope_filter (ignores ignored_paths)"]
    C --> D["Intercept builder.add_custom_frequency\nvia instance setattr"]
    D --> E["builder.add_usd(stage, ignore_paths,\nschema_resolvers)"]
    E -->|New frequency registered| F["_add_scoped_custom_frequency:\nwrap filter, record original"]
    E -->|Existing frequency re-registered| G["Check callbacks match;\nreturn early if same"]
    E -->|Import error| H["Exception propagates to finally"]
    E --> I["finally block:\nrestore all usd_prim_filter refs\ndelete instance override"]
    H --> I
    F --> I
    G --> I
    I --> J["Return stage_info"]

Loading

_{Reviews (1): Last reviewed commit: "Fix Newton cloner rows, cubric fallback,..." | Re-trigger Greptile}

[greptile-apps]

Regex path matching can produce false-positive exclusions

re.compile(path) uses the raw prim-path string as a regex pattern, and pattern.match(prim_path) anchors only at the start (not the end). This means /World/envs also matches /World/envs_global/..., and /World/envs/env_1 matches /World/envs/env_10/.... Any global prim whose path begins with one of the ignore_paths strings as a substring would be silently dropped from the custom-frequency traversal, even though Newton's own add_usd excludes only the intended exact subtrees.

For a correct prefix check, replace the regex compile and match with a plain string prefix comparison that also requires a / separator or an exact match (e.g. prim_path == path or prim_path.startswith(path + "/")), or at minimum use re.compile(re.escape(path) + r"(/|$)") to prevent mid-segment matches.

[greptile-apps]

Inline import of private cross-module function

_add_global_stage_to_builder is now used in three separate modules (replicate.py, visualization_builder.py, and here), which gives it de-facto public cross-module status. The leading underscore and this inline from import are inconsistent with that reach. Moving the import to the top-level imports of newton_manager.py (alongside the other newton_clone_utils imports already used in the file) would make the dependency explicit, and promoting the function to a non-underscore name would align its visibility with its actual API surface.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[AntoineRichard]

Couple of small things otherwise it looks good

[AntoineRichard]

Is this needed? I can see why but does it belongs in this PR?

[hujc7]

There's a fix on newton side that's merged: newton-physics/newton#3406. The main problem is that it was pinned to older mjcwarp and updating newton needs newer version.
This can be done on a separate PR to update to the latest, at least resolve the dep issue I think.

[AntoineRichard]

I didn't know that was a thing, that looks lovely /s

[AntoineRichard]

Changes are reasonable otherwise.

[AntoineRichard]

I think this is also in another PRs. Worth checking out like the solver coupling PR. https://github.com/isaac-sim/IsaacLab/pull/5834/changes#diff-1b68f4d0395901f033293c2fbb2f489b98135587b9e94bbe7fb560bff647498f

[hujc7]

I think it's different context though in the same file.

[StafaH]

Major: Register custom frequencies before attempting to scope them

The fresh global builders used by these call sites have no MuJoCo custom frequencies, and the repository's pinned Newton ModelBuilder.add_usd() does not call add_custom_frequency() despite this statement. Consequently original_filters is empty and the method interception never sees a registration. The newly added test_global_import_does_not_create_tendon_rows_from_ignored_envs fails with KeyError: 'mujoco:tendon', so this workaround does not establish the behavior it claims. Configure the global builder with the active solver's custom attributes before importing, preferably through the same shared builder setup used for source builders. Once registration is explicit, delete the speculative add_custom_frequency monkeypatch and duplicate callback-identity checks; the pinned importer has no path that needs them. Keep the regression assertion proving one global tendon row plus the replicated source rows.

[hujc7]

This will be removed once newton bump is done. Fix is already merged on newton side

[StafaH]

Minor: Exercise the constructor-to-shutdown registry lifecycle

Constructing the marker with object.__new__ and assigning _registry manually bypasses the new constructor behavior that caches sim.vis_marker_registry. This test would still pass if that assignment were later removed, even though real markers would again have no registry available once SimulationContext is torn down. Monkeypatch SimulationContext.instance() to return a fake context, instantiate NewtonVisualizationMarkers normally with a minimal marker config, then make instance() return None before calling close() twice. That directly protects registration, cached ownership, shutdown cleanup, and idempotency in one test.

[hujc7]

Adopted in 3443004 — the test now constructs NewtonVisualizationMarkers normally against a monkeypatched SimulationContext (a fake context exposing vis_marker_registry), then flips instance() to None and calls close() twice, asserting registration, cached ownership, shutdown cleanup, and idempotency as you described.