Feature/develop/hydra robustness

[ooctipus]

Description

This PR robustify hydra to discover presets in deeply nested dictionary

and provide test cases to verify the behavior.

Before deeply nested preset are undiscovered

Type of change

• Bug fix (non-breaking change which fixes an issue)

Screenshots

Please attach before and after screenshots of the change if applicable.

Checklist

• I have read and understood the contribution guidelines
• I have run the pre-commit checks with ./isaaclab.sh --format
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• I have updated the changelog and the corresponding version in the extension's config/extension.toml file
• I have added my name to the CONTRIBUTORS.md or my name already exists there

[greptile-apps]

Greptile Summary

This PR fixes collect_presets to recursively discover PresetCfg instances inside deeply nested dicts (e.g., EventTerm.params.terms.*.params) by extracting a new _collect_from_dict helper. It also adds extensive unit test coverage for the Hydra preset system.

• Bug fix in collect_presets: The old code only iterated one level of dict values for dataclass fields. The new _collect_from_dict helper recurses into nested dicts to find deeply buried PresetCfg instances.
• Missing symmetric fix in resolve_preset_defaults: The same shallow-dict-traversal pattern exists in resolve_preset_defaults (lines 284-287), but was not updated. This means deeply nested PresetCfg instances discovered by the fixed collect_presets won't be resolved to their defaults before serialization, likely causing test failures and runtime issues.
• Test coverage: Adds ~380 lines of tests covering deep nesting, preset factory, error handling, _parse_val, idempotency, and class-vs-instance attribute priority.

Confidence Score: 2/5

• The fix is incomplete — resolve_preset_defaults has the same shallow-dict bug that was fixed in collect_presets, which will cause deeply nested presets to not be resolved.
• While the collect_presets fix is correct, the symmetric fix in resolve_preset_defaults was missed. This means the newly added deep-nesting tests that call resolve_preset_defaults are expected to fail, and runtime usage with deeply nested dict presets will also break. The score of 2 reflects a partially correct fix with a critical gap.
• Pay close attention to source/isaaclab_tasks/isaaclab_tasks/utils/hydra.py — the resolve_preset_defaults function needs the same recursive dict traversal fix that was applied to collect_presets.

Important Files Changed

Filename	Overview
source/isaaclab_tasks/isaaclab_tasks/utils/hydra.py	Adds _collect_from_dict for recursive dict traversal in collect_presets, but the symmetric fix for resolve_preset_defaults is missing — deeply nested PresetCfg instances won't be resolved before serialization.
source/isaaclab_tasks/test/test_hydra.py	Adds comprehensive test coverage for deeply nested dicts, preset factory, error handling, and edge cases. Test test_resolve_preset_defaults_deep_nested_dicts likely fails due to missing fix in resolve_preset_defaults.
source/isaaclab_tasks/config/extension.toml	Version bump from 1.5.11 to 1.5.12, consistent with changelog.
source/isaaclab_tasks/docs/CHANGELOG.rst	Adds 1.5.12 changelog entry documenting the fix and new tests. Follows project RST conventions.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[collect_presets cfg] --> B{Is cfg PresetCfg?}
    B -->|Yes| C[Extract fields as preset dict]
    B -->|No| D[Iterate dir cfg attributes]
    D --> E{Has __dataclass_fields__?}
    E -->|Yes, PresetCfg| F[Extract preset dict + recurse alternatives]
    E -->|Yes, regular| G[Recurse collect_presets]
    E -->|No| H{isinstance dict?}
    H -->|Yes| I["_collect_from_dict (NEW)"]
    H -->|No| J[Skip]
    I --> K{dict_val has __dataclass_fields__?}
    K -->|Yes| L[Recurse collect_presets]
    K -->|No| M{dict_val is dict?}
    M -->|Yes| N["Recurse _collect_from_dict (NEW)"]
    M -->|No| O[Skip]

    style I fill:#90EE90
    style N fill:#90EE90
    style H fill:#90EE90

Loading

Comments Outside Diff (1)

1. source/isaaclab_tasks/isaaclab_tasks/utils/hydra.py, line 284-287 (link)

   resolve_preset_defaults also needs recursive dict traversal

   collect_presets was correctly updated to recurse into nested dicts via _collect_from_dict, but resolve_preset_defaults still only handles one level of dict nesting. When a dict value is itself a dict (e.g., params = {"terms": {"step_one": InnerTermCfg()}}), the inner dict is skipped because it doesn't have __dataclass_fields__.

   This means deeply nested PresetCfg instances (the exact scenario this PR targets) won't be resolved to their default values before serialization. The tests like test_resolve_preset_defaults_deep_nested_dicts should fail because OffsetCfg and FractionCfg inside the dict→dict→configclass→dict chain are never visited.

   A fix analogous to _collect_from_dict is needed here — when dict_val is itself a dict, recurse into it:

   With a helper like:

   def _resolve_from_dict(d: dict) -> None:
       for key, val in d.items():
           if isinstance(val, PresetCfg) and hasattr(val, "__dataclass_fields__"):
               default = getattr(val, "default", None)
               if default is not None:
                   d[key] = default
                   if hasattr(default, "__dataclass_fields__"):
                       resolve_preset_defaults(default)
           elif hasattr(val, "__dataclass_fields__"):
               resolve_preset_defaults(val)
           elif isinstance(val, dict):
               _resolve_from_dict(val)

_{Last reviewed commit: 15d3db7}