[release/3.0.0-beta2] Fix MDL module dependency retrieval

[ooctipus]

Summary

• Cherry-pick Fix MDL module dependency retrieval #6034 / 781b06b onto release/3.0.0-beta2
• Collect relative MDL module imports alongside texture resources when retrieving local asset dependencies
• Adds regression coverage for Hospital-style sibling MDL module imports

Validation

• python3 -m py_compile source/isaaclab/isaaclab/utils/assets.py source/isaaclab/test/utils/test_assets.py
• git diff --check refs/remotes/upstream/release/3.0.0-beta2...HEAD
• isolated parser check for _find_mdl_dependencies passed
• focused pytest attempted, but local system Python is missing lazy_loader, so package import collection fails before tests run

[isaaclab-review-bot]

🤖 IsaacLab Review Bot — PR #6043

Summary

Clean cherry-pick of #6034 (781b06b → develop) onto release/3.0.0-beta2. Adds MDL relative module import resolution alongside the existing texture dependency scanning.

✅ Findings (0 issues)

Category	Status
Cherry-pick fidelity	✅ Matches merged #6034
Correctness	✅ Regex patterns correctly parse import, using ... import, relative prefixes (.::, ..::)
Ambiguity handling	✅ Non-wildcard imports enumerate all candidate module depths; wildcard imports resolve to exact path
Comment/string exclusion	✅ _MDL_RESOURCE_RE.sub("") strips quoted strings and comments before import scanning
Changelog	✅ Present (fix-mdl-module-dependencies.rst)
Test coverage	✅ Comprehensive regression test covers ordinary/wildcard/parent-relative/using-import/comment/quoted edge cases
CI	⏳ Pending (just pushed; wheel build passed)

Notes

• The import resolution strategy conservatively generates multiple candidate .mdl paths for ambiguous non-wildcard imports (e.g., import .::A::B → both A.mdl and A/B.mdl). This is intentional to handle MDL's module-vs-symbol ambiguity and ensures assets won't be missed during local mirroring.
• Global/built-in modules (e.g., ::nvidia::core_definitions) are correctly excluded since the regex only matches relative prefixes.

Verdict: ✅ LGTM

No issues found. Straightforward backport with proper changelog and test coverage.

[greptile-apps]

Greptile Summary

This cherry-pick from #6034 fixes MDL local-asset dependency collection to include relative module imports (.:: / ..:: prefixes) in addition to texture resource paths, so that sibling MDL files such as Hospital materials importing OmniUe4 modules are correctly downloaded alongside the primary asset.

• Extracts a new _find_mdl_dependencies function that first collects textures from the original source, strips comments and quoted strings, then scans for using … import … and plain import … statements, delegating relative-path resolution to _find_mdl_import_dependencies.
• _find_mdl_import_dependencies maps :: paths to POSIX .mdl file candidates, emitting all possible prefix lengths for non-wildcard imports to handle the MDL ambiguity between Module::Symbol and nested module paths.
• A new regression test exercises wildcard, non-wildcard, parent-directory, and using-style imports, and confirms that absolute (::nvidia::…), commented, and quoted imports are correctly ignored.

Confidence Score: 4/5

Safe to merge; the new regex-based MDL import scanner is correct for real-world MDL files, non-existent candidate paths are gracefully skipped by the existing downloader, and the regression test thoroughly validates the happy path.

The core logic is sound and well-tested. The only concerns are an edge case where a using statement split across multiple lines would be silently skipped due to missing re.DOTALL on _MDL_USING_IMPORT_RE, and the intentional-but-noisy emission of speculative .mdl candidate paths for non-wildcard ambiguous imports. Neither affects correctness for any known real-world MDL file.

source/isaaclab/isaaclab/utils/assets.py — specifically the _MDL_USING_IMPORT_RE regex and the candidate-length loop in _find_mdl_import_dependencies.

Important Files Changed

Filename	Overview
source/isaaclab/isaaclab/utils/assets.py	Adds _find_mdl_dependencies and _find_mdl_import_dependencies helpers to collect relative MDL module imports alongside texture references; logic is correct for the targeted use case but _MDL_USING_IMPORT_RE lacks re.DOTALL so multi-line using clauses would be silently skipped.
source/isaaclab/test/utils/test_assets.py	Adds regression test covering wildcard imports, non-wildcard ambiguity, parent-directory imports, using … import, and correct filtering of comments and quoted strings.
source/isaaclab/changelog.d/fix-mdl-module-dependencies.rst	New changelog fragment describing the MDL sibling-module import fix.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["_find_asset_dependencies(path)"] --> B{suffix == .mdl?}
    B -- Yes --> C[Read MDL source text]
    C --> D["_find_mdl_dependencies(source)"]
    D --> E["Scan original source for textures via _MDL_RESOURCE_RE"]
    E --> F["Strip strings and comments into source_code"]
    F --> G["Match using-import statements via _MDL_USING_IMPORT_RE"]
    G --> H["_find_mdl_import_dependencies(module_path)"]
    H --> I["Match _MDL_RELATIVE_IMPORT_RE - .:: or ..:: prefix only"]
    I --> J{Wildcard?}
    J -- Yes --> K[Emit one path at full depth]
    J -- No --> L[Emit all candidate paths length 1 to N]
    K --> M[refs set]
    L --> M
    G --> N["Remove using-imports from source_code"]
    N --> O["Match plain import statements via _MDL_IMPORT_RE"]
    O --> H
    M --> P[Return refs]
    B -- No --> Q{suffix in USD extensions?}
    Q -- No --> R[Return empty set]
    Q -- Yes --> S["Parse USD layer via UsdUtils.ModifyAssetPaths"]
    S --> P

Loading

_{Reviews (1): Last reviewed commit: "Fix MDL module dependency retrieval" | Re-trigger Greptile}

[greptile-apps]

_MDL_USING_IMPORT_RE does not match multi-line using statements

(.+?) does not match newlines by default (no re.DOTALL), so a using clause split across lines—e.g. using\n .::Module\n import *;—would be silently skipped. The using statement then doesn't get removed from source_code before _MDL_IMPORT_RE runs, though in practice the leftover import ...; fragment contains only symbols (no relative prefix) so _find_mdl_import_dependencies returns an empty set for it. Real-world MDL files rarely split using across lines, but it's worth documenting or hardening.

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!

[greptile-apps]

Ambiguous non-wildcard imports generate spurious candidate paths

For import .::Helpers::make_color;, candidate_lengths = range(1, len(components) + 1) intentionally emits both Helpers.mdl and Helpers/make_color.mdl. These candidates are forwarded to retrieve_file_path, which gracefully skips non-existent files with a logger.debug(...) call; however, for deeper module paths (e.g. import .::A::B::C::D;) all four .mdl candidates are probed, generating three unavoidable debug-level noise entries. The behavior is correct and the comment explains it, but it may be worth noting in the docstring that the returned set can contain speculative paths that won't exist on the server.

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!