fix(stirrup_agent): DSv4 GDPVal multi-node run fixes - #1497
Closed
agronskiy wants to merge 6 commits into
Closed
Conversation
… absent DynamicMaxTokensChatCompletionsClient.__call__ only checked msg.reasoning_content when parsing the response. vLLM >= 0.16.0 (and specifically DeepSeek-V4's `--reasoning-parser deepseek_v4`) emits the field as `reasoning` per the Responses-API convention. Without the fallback, reasoning silently dropped from the AssistantMessage and was never threaded back into the next-turn request — the agent forgot its plan every turn and walked the max_turns ceiling without ever successfully calling `finish`. Observed end-to-end on DSv4-Pro GDPVal r3 (slurm job 2855690): across ~3 h of cluster wall-time and 1118 successful code_exec invocations, zero successful `finish` calls and zero rollouts persisted; the ResponseReasoningInterceptor reported reasoning_words=0 for every single response across the run while cache.db responses showed message.reasoning populated. Also pin `choice.message.reasoning = None` in the `_make_response` test fixture (mirroring the existing `reasoning_content = None` pin) so the new `elif hasattr(msg, "reasoning")` branch is not triggered by MagicMock auto-attr access in the three sampling/cap tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Alex Gronskiy <agronskiy@nvidia.com>
… tool-call results
Upstream stirrup catches `pydantic.ValidationError` in `Agent.run_tool`
and returns the bare string "Tool arguments are not valid" as the
`ToolResult` content, dropping all the pydantic error detail on the
floor (e.g. "paths: Input should be a valid list, input_type=str").
The agent has no signal about which field failed or what type was
expected, so it retries the same broken shape forever.
Observed on DSv4-Pro GDPVal r5/r7: the model consistently emitted
`paths` as a JSON string literal ('"[]"', '"[\"foo.txt\"]"',
'"single.pdf"', etc.) instead of a JSON array. All ~660 finish-tool
calls in r5 (2h7m elapsed) failed with the same bare-string error;
zero ✓ finishes, max_turn stuck at 18 for 71 min as rollouts looped
on the same malformed shape. r7 (concurrency=48) hit the same wall
just sooner.
This patch installs a one-shot monkey-patch on
`stirrup.core.agent.Agent.run_tool` at import time (guarded by an
`_gym_surfacing_patched` attribute to prevent re-application). On the
failure path it re-runs `tool.parameters.model_validate_json` to
capture the pydantic error, then rebuilds the ToolMessage with a
detailed content including field-by-field error messages and a 500-char
preview of the submitted arguments. Success path is untouched.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Signed-off-by: Alex Gronskiy <agronskiy@nvidia.com>
…not self._tools) The ValidationError-surfacing wrapper iterated self._tools to find the tool for the failed call. That list mixes plain Tool instances with provider objects (e.g. ApptainerCodeExecToolProvider) which don't have a .name attribute — the `t.name` access raised AttributeError on every failed-finish path, breaking 45 rollouts on r8 before scancel. Switch to self._active_tools.get(tool_call.name) — the same dict upstream stirrup uses in its own run_tool. _active_tools is built during __aenter__ via `if isinstance(tool, Tool): self._active_tools[tool.name] = tool`, so it's the safe lookup path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Alex Gronskiy <agronskiy@nvidia.com>
…epseek_v4 parser bug DeepSeek-V4-Pro served by vLLM 0.20.0 (the wedu image vllm-deepseekv4-v0200-cu130-ray-arm64.sqsh) emits non-string-typed tool-call args as JSON-encoded strings. The model produces `<｜DSML｜parameter ... string="false">[...]</｜DSML｜parameter>` per the chat template, but vLLM's --tool-call-parser deepseek_v4 in 0.20.0 doesn't honor the string="false" flag and forwards the inner JSON verbatim as a literal string. Stirrup's FinishParams rejects with "paths: Input should be a valid list, type=list_type" and the agent loops forever on the same broken shape (1605 ✗ finish / 0 ✓ finish across r9's full 4h walltime). Upstream fix landed in vLLM PR #41801 (merged 2026-05-06), but the wedu image predates it. Until the image is rebuilt: - responses_api_agents/stirrup_agent/finish_tool_coercing.py: new module with CoercingFinishParams (pydantic field_validator(mode= "before") on `paths` that accepts list (passthrough), JSON-encoded string array, or bare filename string) and COERCING_FINISH_TOOL wrapping stirrup's _validating_finish_executor. - responses_api_agents/stirrup_agent/nemo_client.py: third monkey-patch at module-import time replaces SIMPLE_FINISH_TOOL in stirrup.tools .finish, stirrup.tools, and stirrup.core.agent with the coercing variant. Agent.__init__ defaults pick it up via the existing ``finish_tool if finish_tool is not None else SIMPLE_FINISH_TOOL`` fallback. Idempotency tag on the tool object prevents double-patching. - responses_api_agents/stirrup_agent/tests/test_finish_tool_coercing.py: 12 cases covering the 4 known broken shapes from r5 client log (`"[]"`, `"[\"a.txt\"]"`, `"[\"a.txt\",\"b.pdf\"]"`, bare filename) plus correct-shape passthrough, required-field enforcement, dict rejection, and non-string item stringification. When the wedu image is rebuilt against vLLM main >= #41801, this coercion becomes a no-op (the first isinstance(v, list) branch always takes) and both this module and the monkey-patch can be removed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Alex Gronskiy <agronskiy@nvidia.com>
The Stirrup agent's ref-file staging dir was read only from the worker's
os.environ (GDPVAL_REF_FILES_DIR) inside _run_stirrup_agent, which runs as a
@ray.remote(runtime_env={py_executable}) task. On a non-Ray model deployment
(sglang), where a bare `ray start` cluster is launched just for Gym, that
worker does not reliably inherit the deployment-container env, so the read
returned None and staging fell back to node-local /tmp — breaking the
cross-node upload of reference files (100% rollout failure on multi-node).
Resolve it in the server process (which has the env var) and thread it through
`params` into the task, exactly like persist_deliverables_dir — so the worker
no longer depends on env inheritance. Falls back to os.environ if unset.
Signed-off-by: Alex Gronskiy <agronskiy@nvidia.com>
agronskiy
force-pushed
the
agronskiy/feat/gdpval-ref-files-dir-param
branch
from
June 2, 2026 13:00
be98528 to
de712ae
Compare
…-ref-files-dir-param
Contributor
|
it seems 2-5 are missing |
Contributor
|
following up, is 2-5 expected here? |
Contributor
|
cc @vadam5 |
Contributor
Author
|
We now run with sglang, vllm is too buggy for DSv4, closing. Thanks @marta-sd for the ping! |
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
2 participants
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.
Summary
Five fixes that unblock DeepSeek-V4-Pro GDPVal rollouts on multi-node sglang deployments. The Stirrup agent is the upstream agent loop used for GDPVal-style code+filesystem tasks; on the multi-node sglang path each of these issues independently caused 100% rollout failure.
1. Pass
GDPVAL_REF_FILES_DIRas a Ray task paramThe ref-file staging directory was read from
os.environinside a@ray.remote(runtime_env=...)task. When the Ray cluster is started standalone next to a sglang model server (not Ray-managed), the remote worker doesn't inherit the deployment-container env, so staging silently fell back to a node-local/tmpand cross-node ref-file uploads broke. Now resolved in the server process and threaded throughparams, mirroring howpersist_deliverables_diris passed.2. Coerce
finish.pathsstring→listvLLM 0.20.0's
--tool-call-parser deepseek_v4ignores thestring="false"flag on tool-call argument tags and forwards JSON-encoded args as bare strings. Stirrup'sFinishParamsthen rejectspathswith"Input should be a valid list, type=list_type"and the agent retries the same broken shape indefinitely. Added aCoercingFinishParamswith a pydanticfield_validator(mode="before")that accepts a real list (passthrough), a JSON-encoded array string, or a bare filename string, and monkey-patched it in place ofSIMPLE_FINISH_TOOL.Upstream fix landed in vLLM #41801; the coercion becomes a no-op once the inference image is rebuilt against that.
3. Tool lookup via
self._active_tools(notself._tools)The ValidationError-surfacing wrapper (#4 below) iterated
self._toolsto locate the failing tool. That list contains bothToolinstances and provider objects (e.g.ApptainerCodeExecToolProvider) which don't expose.name, so the lookup raisedAttributeErroron every failed-finish path. Switched toself._active_tools.get(tool_call.name), which is the dict upstream Stirrup itself uses insiderun_tool.4. Surface pydantic
ValidationErrordetail in failed tool-call resultsUpstream Stirrup catches
pydantic.ValidationErrorinAgent.run_tooland returns the bare string"Tool arguments are not valid"as the tool result, dropping all field-level error detail. The model has no signal about what shape was expected and loops on the same malformed call. Added an import-time monkey-patch (idempotent) that re-runstool.parameters.model_validate_jsonon the failure path and rebuilds theToolMessagewith per-field error messages plus a truncated preview of the submitted arguments.5. Fall back to
msg.reasoningwhenreasoning_contentis absentDynamicMaxTokensChatCompletionsClientonly readmsg.reasoning_content. vLLM ≥ 0.16.0 with--reasoning-parser deepseek_v4emits the field asreasoning(Responses-API convention), so reasoning was silently dropped from theAssistantMessageand never threaded into the next turn. The agent effectively forgot its plan every turn and walked tomax_turnswithout ever callingfinishsuccessfully. Added anelif hasattr(msg, "reasoning")fallback and pinnedreasoning = Nonein the test fixture so MagicMock auto-attr access doesn't trigger the new branch in unrelated tests.Test plan
pytest responses_api_agents/stirrup_agent/tests/ -x— including the newtest_finish_tool_coercing.py(12 cases: passthrough, four observed broken shapes, required-field, dict-reject, non-string-item stringification)finishcalls end-to-end🤖 Generated with Claude Code