Merge pull request #53 from KraHsu/fix/scene-entity-cfg-resolve-order

Fix instantiate_class_term: resolve SceneEntityCfg before class init

Author: KraHsu

src/genelab/managers/_base.py

@@ -11,17 +11,21 @@
 
 
 def instantiate_class_term(term_cfg: ManagerTermBaseCfg, env: "ManagerBasedRlEnv") -> None:
-    """Resolve ``term_cfg`` against ``env``: class funcs get instantiated, any
-    :class:`SceneEntityCfg` values inside ``params`` get their ``*_ids`` populated.
+    """Resolve ``term_cfg`` against ``env``: any :class:`SceneEntityCfg` values inside
+    ``params`` get their ``*_ids`` populated, then class funcs get instantiated.
 
     Single entrypoint called by every manager's ``__init__`` so per-term setup that
     needs the live env (index resolution, stateful func construction) happens in
     exactly one place. Idempotent re-runs are safe — :meth:`SceneEntityCfg.resolve`
     skips fields whose ids are already filled, and the class-instantiation branch
     only fires when ``func`` is still a class.
+
+    **Order matters**: stateful class terms (e.g. ``feet_swing_height``) cache
+    ``asset_cfg.link_ids`` in their ``__init__``. If resolve ran after the class
+    instantiation, those reads would see ``None``. So resolve goes first.
     """
-    if inspect.isclass(term_cfg.func):
-        term_cfg.func = term_cfg.func(cfg=term_cfg, env=env)
     for value in term_cfg.params.values():
         if isinstance(value, SceneEntityCfg):
             value.resolve(env)
+    if inspect.isclass(term_cfg.func):
+        term_cfg.func = term_cfg.func(cfg=term_cfg, env=env)

tests/test_scene_entity_cfg.py

@@ -93,3 +93,28 @@ def test_instantiate_class_term_ignores_non_scene_entity_params() -> None:
     instantiate_class_term(term_cfg, env)  # type: ignore[arg-type]
     # Sanity: params untouched.
     assert term_cfg.params == {"threshold": 0.5, "names": ("a",), "ranges": (-1.0, 1.0)}
+
+
+def test_instantiate_class_term_resolves_asset_cfg_before_class_init() -> None:
+    """Regression: stateful class terms like ``feet_swing_height`` cache
+    ``asset_cfg.link_ids`` in their ``__init__``. The resolve pass MUST run before
+    the class is instantiated — otherwise the init reads ``None`` and crashes
+    (caught in production by an 8-GPU G1 training run that hit
+    ``ValueError: SceneEntityCfg(name='robot') has no link_ids``).
+    """
+    captured: dict[str, tuple[int, ...] | None] = {}
+
+    class _RecordingTerm:
+        def __init__(self, cfg, env) -> None:  # type: ignore[no-untyped-def]
+            # Mirrors what feet_swing_height.__init__ does — read the cfg at init time.
+            captured["link_ids"] = cfg.params["asset_cfg"].link_ids
+
+        def __call__(self, env) -> None:  # noqa: ARG002 - placeholder
+            return None
+
+    env = _FakeEnv(link_names=["base", "left_foot", "right_foot"], joint_names=[])
+    asset_cfg = SceneEntityCfg(name="robot", link_names=("left_foot", "right_foot"))
+    term_cfg = ManagerTermBaseCfg(func=_RecordingTerm, params={"asset_cfg": asset_cfg})
+    instantiate_class_term(term_cfg, env)  # type: ignore[arg-type]
+    # The recording term's init saw fully resolved link_ids.
+    assert captured["link_ids"] == (1, 2)