chore(translation): finish slime->vime / sglang->vllm cleanup missed in #107 sync - #196
Closed
aoshen02 wants to merge 12 commits into
Closed
chore(translation): finish slime->vime / sglang->vllm cleanup missed in #107 sync#196aoshen02 wants to merge 12 commits into
aoshen02 wants to merge 12 commits into
Conversation
…dep) vime/agent/parsing.py delegated reasoning + function-call parsing to sglang.srt.* (lazy imports), a genuine SGLang runtime coupling carried over in the #107 sync. sglang is not a vime dependency; vLLM ships equivalent parsers. - reasoning: vllm.reasoning.ReasoningParserManager.get_reasoning_parser() -> extract_reasoning(); falls back to the </think> split if the parser can't be constructed for the tokenizer (matches prior behavior). - tool calls: vllm.tool_parsers.ToolParserManager.get_tool_parser() -> extract_tool_calls(); maps ToolCall.function -> {"name","input"}. Visible text = info.content or "" (whole-output tool call -> empty text). - thread the model tokenizer (already in the adapters) into parse_model_output; the XML tool-call fallback is unchanged. Validated against vllm reasoning/tool_parsers + 14/14 test_agent_adapters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#107 sync Under-translations carried over by the #107 sync wave, brought in line with vime's established conventions: - Env vars SLIME_TEST_* / SLIME_FANOUT_TEST_* -> VIME_TEST_* / VIME_FANOUT_TEST_* in the new test files + _fanout_test_helpers.py. Also fixes pr-test.yml, which set SLIME_TEST_* while the tests (and the pr-test.yml.j2 template) already read VIME_TEST_* — the matrix knobs (use_deepep/use_fp8_rollout/enable_eval) were silently dropped. - scripts/run-minimax-m2.sh: checkpoint dirs MiniMax-M2.5_slime/ -> _vime/. - test_loss_cp_invariance.py: in-repo path slime/backends/... -> vime/backends/...; "slime's" -> "vime's" in the inherited-loss prose. - test_{cp_utils,metric_report,metric_report_dist}.py: "slime imports" comments -> "vime imports". - vime/utils/dp_schedule.py: "ray/sglang-importing" -> "ray/vllm-importing". - vime/ray/rollout.py: "in slime's convention" -> "in vime's convention". - agent adapter tests: bogus X-Slime-Session-Id header -> X-Custom-Session-Id (a custom header that is deliberately ignored), fixture query data "slime" -> "vime". Provenance/attribution kept per the translation spec §5 (upstream slime/ sglang links, "copied from SGLang", "counterpart of slime...", "sglang-shaped" normalization, README/docs attribution). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…sing The coding-agent run script carried a sglang->vLLM arg-map comment block (a translation-era note) that also referenced a non-existent translation_guide.md. Replace it with a concise --vllm- prefix pointer to usage.md. Also neutralize the streaming test docstring's "sglang args become vLLM args" to "the rollout-engine args differ" (keeping the "counterpart of slime's test" provenance line). Brings sglang occurrences to the pre-#107 baseline (5), all of which are intentional keeps (sglang-shaped normalization, slime/sglang counterpart provenance, "copied from SGLang" attribution). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per the translation spec, slime/sglang must map specifically to vime/vllm, not to a generic substitute: - test_agent_adapters: the deliberately-ignored bogus header was renamed X-Slime-Session-Id -> X-Custom-Session-Id; correct mirror is X-Vime-Session-Id. - test_qwen3_4B_streaming_partial_rollout docstring: "sglang args become vLLM args" had been genericized to "the rollout-engine args differ"; restore the explicit sglang->vLLM phrasing (it correctly names slime's engine vs vime's in a counterpart docstring). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request migrates the agent reasoning and tool-call parsing logic from SGLang to vLLM, and rebrands references from 'slime' to 'vime' across documentation, scripts, and tests. The review identified two critical runtime issues in the new vLLM parsing integration: a missing 'model' parameter when instantiating ChatCompletionRequest which will raise a Pydantic ValidationError, and an incorrect attribute reference to 'info.tools_called' instead of 'info.tool_calls' which will raise an AttributeError.
| """ | ||
| from vllm.entrypoints.openai.chat_completion.protocol import ChatCompletionRequest | ||
|
|
||
| kwargs: dict[str, Any] = {"messages": []} |
Contributor
There was a problem hiding this comment.
The ChatCompletionRequest class from vLLM's OpenAI protocol requires a model parameter. Instantiating it without a model will raise a Pydantic ValidationError at runtime, causing the reasoning and tool-call parsers to fail and silently fall back to the XML parser.
Suggested change
| kwargs: dict[str, Any] = {"messages": []} | |
| kwargs: dict[str, Any] = {"model": "mock-model", "messages": []} |
|
|
||
| parser = ToolParserManager.get_tool_parser(tool_parser_name)(tokenizer) | ||
| info = parser.extract_tool_calls(body_text, _empty_chat_request(tools_schema)) | ||
| if info.tools_called: |
Contributor
There was a problem hiding this comment.
In vLLM's ExtractedToolCalls class, the attribute containing the list of tool calls is named tool_calls, not tools_called. Referencing info.tools_called will raise an AttributeError at runtime, causing the tool-call parser to always fail and silently fall back to the XML parser.
Suggested change
| if info.tools_called: | |
| if info.tool_calls: |
…ructor Codex review: some vLLM tool parsers (e.g. qwen3coder — the parser for the coding-agent's Qwen3-Coder models) coerce argument types from the tools set at construction (self.tools), not only from the request. Build the ChatCompletionRequest once and pass request.tools to the parser ctor in addition to extract_tool_calls(request). All ToolParser subclasses accept the tools kwarg (verified against vllm). Also: README "engine-side" -> "vLLM-side" parser configuration (be specific). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rt+construct Earlier the vLLM port wrapped the whole import + parser construction + parse in a broad try/except. slime guards only the parse call (parse_non_stream) and the per-call json.loads — not the import or construction. Restructure to mirror slime exactly: import + construct directly, and keep try/except only around extract_tool_calls and json.loads (the two slime also guards). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
slime's coding_agent_rl runs with --sglang-tool-call-parser qwen3_coder /
--sglang-reasoning-parser qwen3 and the adapter reuses them; vime had
dropped the parser flags and relied on the XML fallback (with an invented
README paragraph). Wire them properly so vime parses Qwen3.6 output with the
real parsers like slime:
- arguments.py: add --vllm-tool-call-parser (hand-written orchestration
extra, adapter-side, excluded from `vllm serve` forwarding).
--vllm-reasoning-parser is NOT hand-added — it already auto-generates from
AsyncEngineArgs.reasoning_parser (forwards to the engine AND is read by the
adapter, double-duty like slime's flag).
- run script: pass --vllm-tool-call-parser qwen3_coder /
--vllm-reasoning-parser qwen3 (mirrors slime's SGLANG_ARGS).
- README: replace the invented "XML fallback / not wired" paragraph with the
slime-mirroring instruction (configure the parsers; the adapter applies
them client-side; names must match the served model; XML is the fallback).
generate.py already reads args.vllm_{tool_call,reasoning}_parser, so the
adapter now receives qwen3_coder/qwen3 and parse_model_output uses vLLM's
vllm.tool_parsers / vllm.reasoning (the #196 port) instead of falling back.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Per the mirror principle, the README/run-script text must not add words slime doesn't have. Two over-additions removed: - run script: drop the 6-line "vLLM EngineArgs are passed with a --vllm- prefix..." comment block; slime has only the section banner "# ============ rollout engine ============". - README: replace the invented "raw token-generation mode / client-side / XML fallback" paragraph with slime's actual sentence (translated): "The vLLM server must expose Qwen3.6's tool-call and reasoning parsers so claude-code's tool invocations are parsed correctly:" + the VLLM_ARGS block. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…> MTP) slime's run script runs speculative decoding active inline (--sglang-speculative-algorithm EAGLE + num-steps 3 + eagle-topk 1 + num-draft-tokens 4, no draft model = the model's built-in MTP head). vime had commented it out behind an invented explanatory comment. Mirror slime: translate to a single active --vllm-speculative-config. Per docs/en/advanced/speculative-decoding.md, MTP-layer models use method "mtp" (not "eagle", which needs a separate draft model), and num-steps 3 -> num_speculative_tokens 3; the sglang-only eagle-topk / num-draft-tokens tuning has no vLLM equivalent and is dropped. Removes the vime-invented comment block. Note: config translated from slime + vime's MTP doc; not GPU-validated here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The qwen2.5-0.5B reproducibility script I ported carried two comments slime
doesn't have ("Bitwise reproduction depends on a fixed parallel layout..."
and "The NCCL_ALGO / NVTE / CUBLAS settings below are required..."). slime
has neither. Remove them to mirror slime's text.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Small comment/prose over-additions (text slime doesn't have), trimmed: - geo3k_vlm/README.md (B200): collapse the invented FA4 auto-dispatch paragraph (source link + "if you hit a kernel issue" guidance) to one factual line. (vLLM does auto-prefer FA4 on Blackwell per fa_utils.py, so the fact is kept; slime's sglang "use sdpa" sentence does not apply.) - geo3k_vlm_multi_turn/run_geo3k_vlm_multi_turn.py: drop the "vLLM 0.22.0 needs eager mode..." comment + PR link (slime has none); keep the flag. - geo3k_vlm_multi_turn/env_geo3k.py: drop the invented "matching SkyRL's env" provenance clause. - utils/external_utils/command_utils.py: trim the 8-line pkill comment to 2 lines (drop the "dangerous / no longer needed" editorializing); slime's pkill comments are terse. - fully_async/README.md: fix the misleading "promoted from this example into the core package" wording (slime: the worker already lives in core). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Docs had grown well past slime with vime-authored scaffolding that slime doesn't have. Reduced each to slime's content (translated sglang->vllm), keeping only faithful 1:1 translations plus the minimal wording where vLLM's API genuinely differs: - developer_guide/profiling.md (330->73): drop the Typical-flow list, Verify section, VLLM_RPC_TIMEOUT caveat, Troubleshooting table, and the ~150-line Full Runnable Example. Keep slime's Sleep/engine-list/automated-tool/stress -test sections + one compact "Enabling the vLLM Profiler" note (vLLM only registers /start_profile with --vllm-profiler-config; sglang needs no flag). - examples/qwen3-30B-A3B.md: drop the expanded multi-node guide (Topology table, Ray bring-up, smoke test, parameters table, troubleshooting). Restore slime's 3-bullet note + the redundant-experts example (--sglang-ep-num-redundant-experts -> --vllm-eplb-config). - examples/qwen3-4B.md: drop the IPC/NCCL weight-sync caveat, the Ray placement-group paragraph, and the decoupled-VLLM_ARGS/--train-memory-margin block. Restore slime's dp_size note + the server-concurrency/cudagraph block (--sglang-* -> --vllm-*). - developer_guide/debug.md: remove the entire vime-only "Ray Distributed Debugger" section (absent from slime). - advanced/speculative-decoding.md: collapse the TorchSpec/Speculators feature catalog to slime's single parenthetical. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 9, 2026
Collaborator
Author
|
Superseded — split into two focused PRs per reviewer request:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The #107 sync wave (mega-PRs #137/#138/#143/#145/#155/#156/#158) ported large slime chunks into vime but left some
sglang/slimereferences untranslated. This finishes the translation perSGLANG_TO_VLLM_TRANSLATION.md— mirror slime, translate specifically tovllm/vime(never to a genericengine/rollout/backend), and keep §5 provenance.Counts (vs the pre-#107 baseline
6011ae9f)slime=36 is the legitimate floor: README/CONTRIBUTING/docker attribution, upstreamslime #1916/#1924links, the reproducibility PR link, the #180 "built on slime" attribution, and "counterpart ofslime…" provenance.sglang=6 are all intentional keeps (the "sglang-shaped" normalization that mirrors slime'smeta_infoshape, theslime.rollout.sglang_streaming_rolloutcounterpart provenance + behavioral contrast, "copied from SGLang" attribution, and one counterpart-docstring "sglang args become vLLM args").Changes
vime/agent/parsing.py— replacesglang.srtreasoning + function-call parsers (a genuine SGLang runtime coupling; sglang is not a vime dep) with vLLM'svllm.reasoning/vllm.tool_parsers. Tokenizer threaded from the adapters;</think>+ XML fallbacks preserved. Validated against a real vLLM (hermestool parse + fallbacks) andtest_agent_adapters14/14.SLIME_TEST_*/SLIME_FANOUT_TEST_*→VIME_TEST_*(new test files +_fanout_test_helpers.py). Also fixespr-test.yml, which setSLIME_TEST_*while the tests and the.j2template already readVIME_TEST_*— the matrix knobs (use_deepep/use_fp8_rollout/enable_eval) were silently dropped.scripts/run-minimax-m2.shckpt dirs_slime/→_vime/;slime/backends/...→vime/backends/...; "slime imports"→"vime imports";dp_schedule.py"sglang-importing"→"vllm-importing";ray/rollout.py"slime's convention"→"vime's convention"; bogus headerX-Slime-Session-Id→X-Vime-Session-Id; fixture data"slime"→"vime".--sglang-*→--vllm-*arg-map comment block (it also referenced a non-existenttranslation_guide.md), replaced with a--vllm-prefix pointer tousage.md.Verification
test_agent_adapters14/14; touched filespy_compileclean; run scriptbash -nclean.test_loss_cp_invariancefailures in local CPU env are pre-existing/environmental (confirmed against the unmodified file); this PR only edits its comments/path string.