feat: add multiple _core*.so detection in sanity_check.py

[keivenchang]

Overview:

Adds detection and warning for multiple _core*.so files in sanity_check.py. Multiple .so files cause Python's import system to load the wrong binary module, leading to confusing import errors when switching between maturin build modes or Python versions.

Details:

• Add _check_multiple_core_so() method to detect multiple _core*.so files in lib/bindings/python/src/dynamo
• Display warning with ⚠️ symbol when multiple files are detected, listing count and filenames
• Refactor hardcoded path "lib/bindings/python/src/dynamo" into DYNAMO_RUNTIME_SRC_PATH constant
• Add detailed documentation explaining why multiple .so files are problematic

Where should the reviewer start?

deploy/sanity_check.py - focus on the new _check_multiple_core_so() method (lines ~1818-1855) and how it integrates into the Runtime components section

Related Issues: (use one of the action keywords Closes / Fixes / Resolves / Relates to)

/coderabbit profile chill

Summary by CodeRabbit

• Bug Fixes
  • Enhanced deployment validation to detect and warn about conflicting runtime files.
  • Improved reliability of runtime component discovery during deployment checks.

[coderabbitai]

Walkthrough

A path constant centralizing the runtime directory reference was introduced, an internal helper method for detecting multiple core shared objects was added to DynamoRuntimeInfo, and this detection is now invoked during initialization to surface warnings about conflicting files in the runtime source directory.

Changes

Cohort / File(s)	Summary
Runtime path centralization and core file detection deploy/sanity_check.py	Added constant DYNAMO_RUNTIME_SRC_PATH for runtime directory path; introduced _check_multiple_core_so() method to detect multiple _core*.so files; integrated detection into DynamoRuntimeInfo.__init__() to attach warning nodes; refactored runtime discovery to use the new constant instead of hardcoded paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Paths once scattered, now one,
Constants gleam like morning sun,
Rogue .so files? I'll spot each one,
Warnings hop where strays have run,
Order reigns when tasks are done! 🥕

Pre-merge checks

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The pull request title "feat: add multiple _core*.so detection in sanity_check.py" is fully related to the main change in the changeset. It clearly and concisely describes the primary feature being introduced: the addition of detection for multiple _core*.so files in the sanity check utility. The title uses conventional semantic versioning prefix ("feat:"), is specific about what was added, and avoids vague or generic language. A reviewer scanning the commit history would immediately understand the primary change from this title.
Description Check	✅ Passed	The pull request description follows the required template structure and includes all critical sections with meaningful content. The Overview section provides context on the problem (multiple .so files causing import errors), the Details section comprehensively covers the changes made including the new method, warning display, path refactoring, and documentation, and the "Where should the reviewer start?" section provides specific file and line number guidance. The Related Issues section is present but empty, which is a minor non-critical omission since no relevant issues were identified. Overall, the description is complete, well-organized, and provides sufficient information for reviewers to understand the purpose and scope of the changes.

---

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: 0

🧹 Nitpick comments (1)

deploy/sanity_check.py (1)

1817-1856: Good implementation with clear documentation. Consider logging exceptions.

The method correctly detects multiple _core*.so files and provides helpful context. The comprehensive docstring explains why this is problematic.

Minor improvement: The try-except-pass block at line 1853 silently swallows exceptions, which could hide unexpected errors. While acceptable in a diagnostic tool, consider adding basic logging or at least a comment explaining why exceptions are ignored.

Based on static analysis hints.

Example enhancement:

         try:
             # Find all _core*.so files
             so_files = glob.glob(os.path.join(core_dir, "_core*.so"))

             if len(so_files) > 1:
                 # Multiple .so files found - create warning
                 so_file_names = [os.path.basename(f) for f in so_files]
                 warning_desc = (
                     f"Found {len(so_files)} files: {', '.join(so_file_names)}"
                 )
                 return NodeInfo(
                     label="Multiple _core*.so files detected",
                     desc=warning_desc,
                     status=NodeStatus.WARNING,
                 )
         except Exception:
-            pass
+            # Silently ignore errors during .so detection as this is non-critical diagnostic info
+            pass

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 681951d and 8819ed6.

📒 Files selected for processing (1)

• deploy/sanity_check.py (4 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

deploy/sanity_check.py (1)

deploy/dynamo_check.py (3)

• add_child (140-143)
• NodeInfo (123-279)
• NodeStatus (111-119)

🪛 Ruff (0.14.1)

deploy/sanity_check.py

1853-1854: try-except-pass detected, consider logging the exception

(S110)

---

1853-1853: Do not catch blind exception: Exception

(BLE001)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)

• GitHub Check: trtllm (amd64)
• GitHub Check: trtllm (arm64)
• GitHub Check: sglang
• GitHub Check: operator (amd64)
• GitHub Check: vllm (amd64)
• GitHub Check: Build and Test - dynamo

🔇 Additional comments (4)

deploy/sanity_check.py (4)

99-100: LGTM! Good refactoring to centralize the path constant.

This improves maintainability by having a single source of truth for the runtime source path.

---

1751-1754: LGTM! Clean integration of the multiple .so file check.

The conditional logic correctly adds the warning node only when multiple files are detected.

---

1834-1834: LGTM! Consistent use of the new path constant.

---

1866-1874: LGTM! Path constant properly utilized and documented.

The comment at line 1866 correctly references the constant name, and line 1874 uses it consistently with the refactoring.

[rmccorm4]

sglang check failing here: https://github.com/ai-dynamo/dynamo/actions/runs/18704711963/job/53340577769?pr=3803

#18 3.322 Using Python 3.12.3 environment at: /opt/dynamo/venv
#18 4.282   × No solution found when resolving dependencies:
#18 4.282   ╰─▶ Because there is no version of apache-tvm-ffi==0.1.0b15 and
#18 4.282       flashinfer-python==0.4.0 depends on apache-tvm-ffi==0.1.0b15, we can
#18 4.282       conclude that flashinfer-python==0.4.0 cannot be used.
#18 4.282       And because sglang==0.5.3.post2 depends on flashinfer-python==0.4.0, we
#18 4.282       can conclude that sglang==0.5.3.post2 cannot be used.
#18 4.282       And because only sglang[all]==0.5.3.post2 is available and you require
#18 4.282       sglang[all], we can conclude that your requirements are unsatisfiable.
#18 4.282 
#18 4.282       hint: `apache-tvm-ffi` was requested with a pre-release marker (e.g.,
#18 4.282       apache-tvm-ffi==0.1.0b15), but pre-releases weren't enabled (try:
#18 4.282       `--prerelease=allow`)
#18 ERROR: process "/bin/sh -c cd /opt &&     git clone https://github.com/sgl-project/sglang.git &&     cd sglang &&     git checkout v${SGLANG_VERSION} &&     uv pip install -e \"python[all]\"" did not complete successfully: exit code: 1

should be fixed by this pr here: #3779

Hitting the "Update Branch" button to include the fix and retrigger the jobs