Make ShapeConfig strict with slots=True

[hujc7]

Description

ModelBuilder.ShapeConfig is a plain @dataclass. Python silently accepts writes to undeclared attribute names on such classes, so a downstream cfg.old_name = value becomes a dead no-op when old_name is renamed upstream.

Seen in practice in isaac-sim/IsaacLab#5289: PR #1732 renamed ShapeConfig.contact_margin → gap, and Isaac Lab's stale write builder.default_shape_cfg.contact_margin = 0.01 continued to run for weeks after the pin bump, created a dead attribute, and the intended 1 cm shape gap was never applied.

Change

Add slots=True to the @dataclass decorator on ShapeConfig. Writes to unknown attribute names now raise AttributeError at the write site, so any future ShapeConfig rename is loud for every consumer.

Scope is intentionally narrow to ShapeConfig. Happy to apply the same to ActuatorEntry, CustomAttribute, CustomFrequency in a follow-up if the approach is accepted.

Compatibility

Sanity-checked by applying the change to an installed Newton copy and verifying:

• Valid writes still work (cfg.gap, cfg.margin, cfg.density).
• cfg.contact_margin = 0.01 (the IsaacLab#5289 case) raises AttributeError: 'ShapeConfig' object has no attribute 'contact_margin'.
• ShapeConfig.copy(), dataclasses.replace, and pickle round-trips all continue to work.
• Audited all default_shape_cfg.<field> = ... writes in Newton's test suite (30 files) — every write targets a declared field.
• Ran newton/tests/test_mesh_backface.py::TestHeightfieldPrismSteepAndRotated (4 tests exercising default_shape_cfg.mu/margin/gap) — all pass.

Breaking for any out-of-tree user that attaches custom attributes to ShapeConfig; those writes were already dead when they used typo'd names, this just makes them loud.

Newton Migration Guide

• The migration guide in docs/migration.rst is up-to date

(No migration guide entry added; the change does not rename or remove any public API. Happy to add a note if maintainers prefer.)

Before your PR is "Ready for review"

• Necessary tests have been added and new examples are tested (no new tests needed; existing ShapeConfig-touching tests continue to pass)
• Documentation is up-to-date
• Code passes formatting and linting checks with pre-commit run -a

Summary by CodeRabbit

• Chores
  • Internal optimization to memory allocation for dataclass attributes.

[linux-foundation-easycla]

• ❌ - login: @hujc7 / name: hujc . The commit (ad34dae) is not authorized under a signed CLA. Please click here to be authorized. For further assistance with EasyCLA, please submit a support request ticket.

[coderabbitai]

📝 Walkthrough

Walkthrough

The ShapeConfig dataclass nested within ActuatorEntry is updated to use slots=True, converting from traditional __dict__-based storage to slot-based attribute storage. No fields, defaults, or methods are modified.

Changes

Cohort / File(s)	Summary
Dataclass Optimization newton/_src/sim/builder.py	Updated ShapeConfig dataclass declaration to use slots=True for memory-efficient attribute storage.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly and concisely describes the main change: adding slots=True to the ShapeConfig dataclass decorator for stricter attribute handling.
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.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@newton/_src/sim/builder.py`:
- Line 213: You changed `@dataclass`(slots=True) on ShapeConfig which breaks
dynamic attribute writes; add a runtime deprecation phase and changelog entry:
emit a DeprecationWarning (from warnings.warn) when ShapeConfig is instantiated
(or on first attribute assignment) explaining that dynamic ad-hoc attributes
will be disallowed and advising users to migrate their ad-hoc data into a
dedicated dict/explicit fields or to subclass/compose a wrapper that holds extra
attributes; include a maintainer-approved explicit exception path if you want
immediate hard-fail (raise AttributeError with the same migration text) and
document which behavior is chosen. Then add an entry in CHANGELOG.md under
[Unreleased] (Deprecated/Changed) mentioning ShapeConfig, the slots=True
behavioral break, the deprecation timeline (when it will be removed or when
hard-fail applies), and concrete migration examples (use explicit fields,
.metadata dict, or subclassing) so users can update their code.

🪄 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.yml

Review profile: CHILL

Plan: Pro

Run ID: 3b7550b9-1f04-4ec6-a9f4-e7f54ca24fa9

📥 Commits

Reviewing files that changed from the base of the PR and between 329f31c and ad34dae.

📒 Files selected for processing (1)

• newton/_src/sim/builder.py

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Add deprecation + changelog path for this behavioral break

slots=True intentionally breaks dynamic attribute writes on ShapeConfig (now AttributeError). Please add a deprecation phase (or explicit maintainer-approved exception) and a [Unreleased] CHANGELOG entry with migration guidance for users who previously attached ad-hoc attributes.

As per coding guidelines: “newton/!(tests)/**/*.py: Breaking changes require a deprecation first.” and “**: Check that any user-facing change includes a corresponding entry in CHANGELOG.md under the [Unreleased] section … For Deprecated, Changed, and Removed entries, verify the entry includes migration guidance…”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@newton/_src/sim/builder.py` at line 213, You changed `@dataclass`(slots=True)
on ShapeConfig which breaks dynamic attribute writes; add a runtime deprecation
phase and changelog entry: emit a DeprecationWarning (from warnings.warn) when
ShapeConfig is instantiated (or on first attribute assignment) explaining that
dynamic ad-hoc attributes will be disallowed and advising users to migrate their
ad-hoc data into a dedicated dict/explicit fields or to subclass/compose a
wrapper that holds extra attributes; include a maintainer-approved explicit
exception path if you want immediate hard-fail (raise AttributeError with the
same migration text) and document which behavior is chosen. Then add an entry in
CHANGELOG.md under [Unreleased] (Deprecated/Changed) mentioning ShapeConfig, the
slots=True behavioral break, the deprecation timeline (when it will be removed
or when hard-fail applies), and concrete migration examples (use explicit
fields, .metadata dict, or subclassing) so users can update their code.