fix(sglang): support profile request object API

[nvpohanh]

[by Codex]

Summary

• adapt the SGLang profiling route to both the legacy keyword-argument API and the newer start_profile(ProfileReq) API
• keep version handling in the existing SGLang compatibility module via signature probing
• add focused async tests for both API shapes

Validation

• python3 -m compileall -q components/src/dynamo/sglang/_compat.py components/src/dynamo/sglang/request_handlers/handler_base.py components/src/dynamo/sglang/tests/test_sglang_unit.py
• git diff --check
• standalone compatibility smoke covering both signatures
• regression reproduced in Dynamo 1.1.0 with SGLang commit 5b76f55d: /engine/start_profile returned TypeError: TokenizerControlMixin.start_profile() got an unexpected keyword argument output_dir before cudaProfilerStart

The focused pytest cases were added but not run locally because this VM does not have the Dynamo/SGLang test environment installed.

Overview:

Newer SGLang builds changed TokenizerManager.start_profile from individual keyword arguments to a single ProfileReq. Dynamo still unpacked the HTTP body as kwargs, causing every profiling request to fail with HTTP 500.

Details:

start_profile_compat inspects the installed method signature. It preserves kwargs for SGLang 0.5.11 and constructs ProfileReq(**body) for current builds.

Where should the reviewer start?

components/src/dynamo/sglang/_compat.py

Related Issues

This PR is NOT linked to an issue:

• Confirmed - no related issue

Summary by CodeRabbit

• Bug Fixes

  • Improved profiling setup compatibility so the app works with both newer and older SGLang versions.
  • Added smarter handling when starting profiling, reducing failures caused by differing request formats.
  • Included fallback behavior if signature detection is unavailable, helping keep profiling available in more environments.

• Tests

  • Added coverage for both supported profiling call styles to verify consistent behavior.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

Walkthrough

Adds a compatibility shim in SGLang's _compat.py that detects whether TokenizerManager.start_profile accepts a ProfileReq object or legacy keyword arguments, dispatches accordingly, and updates handler_base.py to use this shim, with new unit tests covering both call paths.

Changes

start_profile compatibility shim

Layer / File(s)	Summary
Compatibility detection and shim implementation components/src/dynamo/sglang/_compat.py	Adds cached signature inspection (_start_profile_accepts_request_object), request builder (_build_profile_request), and start_profile_compat wrapper that dispatches to either a ProfileReq object call or legacy keyword-argument call, with TypeError fallback; exports start_profile_compat via __all__.
Handler integration and tests components/src/dynamo/sglang/request_handlers/handler_base.py, components/src/dynamo/sglang/tests/test_sglang_unit.py	BaseWorkerHandler.start_profile now calls start_profile_compat instead of calling tokenizer_manager.start_profile(**body) directly; new async tests verify both legacy kwarg forwarding and request-object construction paths.

Estimated code review effort: 2 (Simple) | ~12 minutes

Poem:
A rabbit sniffs two APIs old and new,
One wants kwargs, one wants a req too.
With a cached peek at signatures bare,
It builds the right call with careful care.
Tests hop through both paths, clean and true. 🐇

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title is concise and accurately summarizes the main change: adding support for the new SGLang profile request object API.
Description check	✅ Passed	The description follows the template with Overview, Details, reviewer guidance, and a completed Related Issues section.
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.

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

components/src/dynamo/sglang/_compat.py (1)

138-149: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Bound-method unwrap + lru_cache TypeError fallback is correct but adds indirection.

Unwrapping to __func__ before calling the cached signature check (to get cross-instance cache hits) and falling back to .__wrapped__ when the cache key isn't hashable are both valid uses of functools.lru_cache, but this path only matters for the rare case of an unhashable start_profile callable. Consider a short comment explaining why the TypeError fallback exists, to save future readers from re-deriving this.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/src/dynamo/sglang/_compat.py` around lines 138 - 149, Add a brief
inline comment near the start_profile signature check to explain why __func__ is
used before the cached _start_profile_accepts_request_object call and why the
TypeError fallback to __wrapped__ exists. The comment should mention that this
preserves cross-instance cache hits for bound methods and only bypasses
lru_cache when the callable is unhashable, so future readers understand the
indirection in this _compat.py logic.

components/src/dynamo/sglang/tests/test_sglang_unit.py (1)

252-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Hardcoded /tmp/profile flagged by static analysis; harmless here but easy to avoid.

These are just mock parameter values (never touch the filesystem), so the security implications are moot, but the literal string still trips ruff (S108) and ast-grep's hardcoded-tmp-path check across all three occurrences. Swapping to a clearly non-filesystem placeholder (e.g., "profile-output") keeps tests hermetic per repo guidance and silences the noise.

🧹 Proposed fix

-    body = {"output_dir": "/tmp/profile", "start_step": 10, "num_steps": 5}
+    body = {"output_dir": "profile-output", "start_step": 10, "num_steps": 5}

-    request = SimpleNamespace(output_dir="/tmp/profile", start_step=10, num_steps=5)
+    request = SimpleNamespace(output_dir="profile-output", start_step=10, num_steps=5)

-        {"output_dir": "/tmp/profile", "start_step": 10, "num_steps": 5},
+        {"output_dir": "profile-output", "start_step": 10, "num_steps": 5},

As per path instructions: "Tests must remain hermetic and parallel-safe: don't hardcode ports, temp paths, or write files into the repo tree."

Also applies to: 267-267, 275-275

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@components/src/dynamo/sglang/tests/test_sglang_unit.py` at line 252, Replace
the hardcoded temporary path string used in the Sglang test payloads with a
non-filesystem placeholder value so the tests stay hermetic and stop triggering
S108/hardcoded tmp path checks. Update the repeated mock parameter values in
test_sglang_unit.py where the request bodies are built (the occurrences around
the body assignments in the relevant test cases) to use a neutral name like the
profile output placeholder, keeping the same test behavior without referencing
/tmp.

Sources: Path instructions, Linters/SAST tools

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@components/src/dynamo/sglang/_compat.py`:
- Around line 126-129: Move the ProfileReq dependency out of
_build_profile_request and import it at module scope alongside the other
top-level imports in _compat.py. This helper should just construct and return
ProfileReq(**body), with no import statement inside the function body, so the
module follows the repository import rules and makes the SGLang dependency
explicit.

---

Nitpick comments:
In `@components/src/dynamo/sglang/_compat.py`:
- Around line 138-149: Add a brief inline comment near the start_profile
signature check to explain why __func__ is used before the cached
_start_profile_accepts_request_object call and why the TypeError fallback to
__wrapped__ exists. The comment should mention that this preserves
cross-instance cache hits for bound methods and only bypasses lru_cache when the
callable is unhashable, so future readers understand the indirection in this
_compat.py logic.

In `@components/src/dynamo/sglang/tests/test_sglang_unit.py`:
- Line 252: Replace the hardcoded temporary path string used in the Sglang test
payloads with a non-filesystem placeholder value so the tests stay hermetic and
stop triggering S108/hardcoded tmp path checks. Update the repeated mock
parameter values in test_sglang_unit.py where the request bodies are built (the
occurrences around the body assignments in the relevant test cases) to use a
neutral name like the profile output placeholder, keeping the same test behavior
without referencing /tmp.

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

Review profile: CHILL

Plan: Enterprise

Run ID: 6e7af0fc-b563-4017-8bf3-f0879eb5e513

📥 Commits

Reviewing files that changed from the base of the PR and between 1ef2f94 and e20bb29.

📒 Files selected for processing (3)

• components/src/dynamo/sglang/_compat.py
• components/src/dynamo/sglang/request_handlers/handler_base.py
• components/src/dynamo/sglang/tests/test_sglang_unit.py

[dynamo-review-agent]

components/src/dynamo/sglang/llm_engine.py — The unified SglangLLMEngine control route still calls tokenizer_manager.start_profile(**body), so unified SGLang deployments will still fail against ProfileReq-only SGLang versions. Fix: call start_profile_compat here just like BaseWorkerHandler.start_profile.

🤖 AI Fix

In components/src/dynamo/sglang/llm_engine.py, add from dynamo.sglang._compat import start_profile_compat with the other Dynamo SGLang imports and replace await self.engine.tokenizer_manager.start_profile(**body) in SglangLLMEngine.start_profile with await start_profile_compat(self.engine.tokenizer_manager, body).

[ishandhanani]

/ok-to-test 7383f19