Add checked_apply helper for forwarding configclass fields onto upstream dataclasses

[hujc7]

Description

Adds isaaclab.utils.checked_apply for forwarding the declared fields of one dataclass onto another, raising AttributeError if the target is missing a declared field.

The use case is Isaac Lab configclasses that mirror an upstream library's dataclass (for example, Newton's ShapeConfig). Bare setattr loops are fragile: if upstream renames or removes a field, every write becomes a silent no-op (the failure mode PR #5289 fixed for ShapeConfig.contact_margin → margin). With checked_apply, the failure surfaces at startup with a clear message instead of degrading runtime behavior.

API

from isaaclab.utils import checked_apply

@configclass
class NewtonShapeCfg:
    margin: float = 0.0
    gap: float = 0.01

# at apply site (one line, no per-field setattr noise)
checked_apply(cfg.default_shape_cfg, builder.default_shape_cfg)

Internally:

1. Iterates dataclasses.fields(src) — single source of truth for declared fields.
2. Raises AttributeError if target lacks a declared field.
3. Rejects non-dataclass src with TypeError.

What's included

1. source/isaaclab/isaaclab/utils/configclass.py — checked_apply function (lives next to @configclass since it operates on dataclasses).
2. source/isaaclab/isaaclab/utils/__init__.pyi — export.
3. source/isaaclab/test/utils/test_configclass.py — three tests (forwards all fields, raises on missing target field, rejects non-dataclass src).
4. source/isaaclab/docs/CHANGELOG.rst — 4.6.13 entry.
5. source/isaaclab/config/extension.toml — version bump.

Dependents

This PR is a dependency for the rough-terrain Newton stack:

1. PR [Rough Locomotion] Part 1: Foundation + quadrupeds on Newton (deps merged in) #5248 — quadrupeds rough terrain, uses checked_apply to forward NewtonShapeCfg onto Newton's upstream ShapeConfig. Without it, default_shape_cfg.margin is left at Newton's upstream default of 0.0, breaking all non-Anymal-D robots on triangle-mesh terrain.
2. PR [Rough Locomotion] Part 2: H1/Cassie bipeds on Newton #5298 — bipeds (chains on [Rough Locomotion] Part 1: Foundation + quadrupeds on Newton (deps merged in) #5248).
3. PR [WIP][Rough Locomotion] Part 3: G1 on Newton (max_iterations 3000→5000, no physics tuning) #5312 — G1 (chains on [Rough Locomotion] Part 2: H1/Cassie bipeds on Newton #5298).

Type of change

• New feature (non-breaking).

Checklist

• Tests added (3 new in test_configclass.py)
• Pre-commit checks pass
• CHANGELOG + extension.toml bumped
• No new dependencies

[isaaclab-review-bot]

🤖 Isaac Lab Review Bot

Summary

This PR introduces upstream_set, a defensive helper that guards against silent dead-attribute writes to upstream library configuration objects (dataclasses, modules). The motivation is sound—PR #5289 showed a real-world case where a Newton rename caused a stale assignment to silently become a no-op. The implementation is clean, well-tested, and correctly migrates the single existing cross-dep write site.

Architecture Impact

Self-contained. The new upstream_set utility has no dependencies beyond the standard library (dataclasses). The single migration point in NewtonManager.create_builder is low-risk—it affects builder initialization, not hot-path physics stepping. No other callers are impacted.

Implementation Verdict

Ship it — Minor documentation suggestion below, but nothing blocking.

Test Coverage

Thorough. The 5 unit tests cover:

• ✅ Happy path: declared field is set correctly
• ✅ Undeclared field raises AttributeError with helpful message containing field names
• ✅ Failed write doesn't create spurious attribute
• ✅ Module attribute that exists is set
• ✅ Module attribute that doesn't exist raises

The tests use a synthetic _DummyCfg dataclass and types.ModuleType, which is appropriate—no need to depend on Newton's actual ShapeConfig in the isaaclab package tests.

CI Status

No CI results available yet. The PR description notes local pre-commit passed; awaiting CI run.

Findings

🔵 Improvement: source/isaaclab/isaaclab/utils/upstream.py:24 — Missing type annotation for value parameter

The value parameter lacks a type hint. While Any is semantically correct here (the function is type-agnostic), adding it improves IDE support and documentation consistency:

def upstream_set(obj: object, name: str, value: Any) -> None:

This requires from typing import Any (or from __future__ import annotations already present handles forward refs, but Any still needs import).

🔵 Improvement: source/isaaclab/isaaclab/utils/upstream.py:41-45 — Consider using type(obj) directly for module type path

For modules, type(obj).__module__ is builtins and type(obj).__name__ is module, producing builtins.module which is less helpful than the actual module's __name__. Consider:

if isinstance(obj, types.ModuleType):
    type_path = obj.__name__
else:
    type_path = f"{type(obj).__module__}.{type(obj).__name__}"

This would produce fake_upstream has no attribute 'quiet' instead of builtins.module has no attribute 'quiet'. Minor improvement for debugging module config writes.

🔵 Improvement: source/isaaclab/test/utils/test_upstream.py:52-55 — Test could verify error message content for module case

The module test verifies the exception is raised but doesn't assert on the message content. For consistency with test_undeclared_field_raises_attribute_error, consider:

def test_module_attribute_that_does_not_exist_raises():
    mod = types.ModuleType("fake_upstream")
    with pytest.raises(AttributeError) as excinfo:
        upstream_set(mod, "quiet", True)
    assert "quiet" in str(excinfo.value)

🔵 Improvement: source/isaaclab/isaaclab/utils/init.pyi:7-57 — Unrelated import reordering

The diff shows the .pyi stub was reformatted (alphabetized imports). This is fine—likely from running ruff format—but the diff is noisier than necessary. Not a problem, just noting it's unrelated to the feature.

---

Overall this is a well-executed, focused PR that addresses a real problem with an appropriately minimal solution. The convention documented in AGENTS.md ensures future cross-dep writes will use the helper.

[greptile-apps]

Greptile Summary

This PR adds checked_apply, a small utility that forwards every declared field of a source dataclass onto a target object, raising AttributeError if the target is missing any field. It replaces ad-hoc setattr loops that silently become no-ops when upstream APIs rename fields, and comes with three tests, a CHANGELOG entry, and a matching version bump.

Confidence Score: 5/5

Safe to merge — all findings are minor style suggestions with no impact on correctness or behavior.

The implementation is correct and well-tested. The two comments are P2 style preferences (consolidating the dual dataclasses import and using is_dataclass over a dunder check) that do not affect runtime behavior.

No files require special attention.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/utils/configclass.py	Adds checked_apply — a small, well-documented helper that iterates dataclasses.fields(src), validates each name against target, and forwards values via setattr. Logic is correct; only minor style concerns (redundant dual-import of dataclasses, dunder-attribute guard instead of is_dataclass).
source/isaaclab/isaaclab/utils/init.pyi	Adds checked_apply to __all__ and to the lazy-loader import from .configclass. Consistent with how configclass and resolve_cfg_presets are already exposed.
source/isaaclab/test/utils/test_configclass.py	Three focused tests covering the happy path (all fields forwarded, extra target fields untouched), missing-field error, and non-dataclass rejection. Good coverage of the specified contract.
source/isaaclab/docs/CHANGELOG.rst	New 4.6.13 entry correctly documents the added helper with accurate date and Sphinx cross-reference.
source/isaaclab/config/extension.toml	Patch version bumped from 4.6.12 to 4.6.13, consistent with the CHANGELOG entry.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["checked_apply(src, target)"] --> B{src has\n__dataclass_fields__?}
    B -- No --> C[raise TypeError]
    B -- Yes --> D["for f in dataclasses.fields(src)"]
    D --> E{hasattr\ntarget, f.name?}
    E -- No --> F[raise AttributeError\nwith target path + field name]
    E -- Yes --> G["setattr(target, f.name,\ngetattr(src, f.name))"]
    G --> D
    D -- done --> H[return None]

Loading

_{Reviews (2): Last reviewed commit: "Add checked_apply helper for forwarding ..." | Re-trigger Greptile}

[hujc7]

@greptileai Review — please re-review the latest commits.

@isaaclab-review-bot Please re-review.

Force-pushed with significant scope change since the previous review:

1. Renamed upstream_set → checked_apply, changed API shape from single-field (obj, name, value) to bulk-apply (src_dataclass, target) driven by dataclasses.fields(src).
2. Moved the helper from new file isaaclab/utils/upstream.py into existing isaaclab/utils/configclass.py (dataclass utilities belong together).
3. Dropped the Newton create_builder migration from this PR — it's covered by dependent PR [Rough Locomotion] Part 1: Foundation + quadrupeds on Newton (deps merged in) #5248 where the consumer (NewtonShapeCfg) is also introduced.
4. Dropped the AGENTS.md update (no longer needed).
5. Tests moved from new test_upstream.py into existing test_configclass.py.