Skip to content

Commit 4c994e7

Browse files
Refactor Legacy and DRTulu parsers to use tool_definitions instead of tool_actors
The parsers previously had a code path using Ray actor handles (tool_actors) to get tool names and parameters. Since the architecture now uses EnvironmentPools, this was dead code for Legacy and completely broken for DRTulu (which raised an error at startup). Changes: - Legacy parser: remove tool_actors path, use only OpenAI-format tool_definitions - DRTulu parser: accept tool_definitions + explicit stop_sequences instead of tool_actors. Stop sequences are fetched from the pool during initialization. - create_tool_parser: replace tool_actors param with stop_sequences - grpo_fast: fetch stop_strings from pool for dr_tulu, remove blocking error - vllm_utils: plumb tool_stop_sequences through to LLMRayActor parser init - Remove ray import from parsers.py (no longer needed) - Update tests to use tool_definitions instead of mock Ray actors Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent fac7d14 commit 4c994e7

File tree

8 files changed

+143
-193
lines changed

8 files changed

+143
-193
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ All notable changes to this project will be documented in this file.
2222
- Documentation and runtime warning for `dataset_mixer_list` format (float=proportion, int=count) (https://github.com/allenai/open-instruct/pull/1434).
2323

2424
### Changed
25+
- Refactor Legacy and DRTulu tool parsers to use OpenAI-format `tool_definitions` instead of Ray `tool_actors`. Removes `import ray` from `parsers.py`, fixes DRTulu parser which was broken after the pool refactor, and fixes `--tool_parser_type` typo in dr_tulu debug script (https://github.com/allenai/open-instruct/pull/1491).
2526
- Replaces lambda collators with a "single_example_collator" (https://github.com/allenai/open-instruct/pull/1472).
2627
- Clarified `activation_memory_budget` guidance in DPO utils with a practical default (`0.5`) and memory/speed tradeoff notes (https://github.com/allenai/open-instruct/pull/1460).
2728
- Let TransformerTrainModule handle FSDP parallelism instead of manual application in DPO (https://github.com/allenai/open-instruct/pull/1458).

open_instruct/environments/tools/parsers.py

Lines changed: 17 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
from dataclasses import dataclass, field
1919
from typing import Any
2020

21-
import ray
2221
from transformers import PreTrainedTokenizer, PreTrainedTokenizerFast
2322
from vllm.entrypoints.openai.protocol import ChatCompletionRequest
2423
from vllm.tool_parsers import ToolParser as VllmNativeToolParser
@@ -55,14 +54,11 @@ class OpenInstructLegacyToolParser(ToolParser):
5554
Tools are invoked via <tool_name>content</tool_name> tags.
5655
The content between tags is passed to the tool's first required parameter.
5756
Only works for tools that take a single string parameter.
57+
58+
Tool names and parameter names are derived from OpenAI-format tool definitions.
5859
"""
5960

60-
def __init__(
61-
self,
62-
tool_actors: list[ray.actor.ActorHandle] | None = None,
63-
output_wrap_name: str = "output",
64-
tool_definitions: list[dict[str, Any]] | None = None,
65-
):
61+
def __init__(self, tool_definitions: list[dict[str, Any]] | None = None, output_wrap_name: str = "output"):
6662
self.output_wrap_name = output_wrap_name
6763

6864
if tool_definitions:
@@ -78,17 +74,6 @@ def __init__(
7874
else:
7975
properties = params.get("properties", {})
8076
self.tool_param_names[name] = next(iter(properties)) if properties else "text"
81-
elif tool_actors:
82-
self.tool_names = [ray.get(actor.get_call_name.remote()) for actor in tool_actors]
83-
self.tool_param_names = {}
84-
for actor, tool_name in zip(tool_actors, self.tool_names):
85-
params = ray.get(actor.get_parameters.remote())
86-
required = params.get("required", [])
87-
if required:
88-
self.tool_param_names[tool_name] = required[0]
89-
else:
90-
properties = params.get("properties", {})
91-
self.tool_param_names[tool_name] = next(iter(properties)) if properties else "text"
9277
else:
9378
self.tool_names = []
9479
self.tool_param_names = {}
@@ -288,21 +273,20 @@ class DRTuluToolParser(ToolParser):
288273
"""
289274
Parser for DR Tulu style tool calls. Delegates actual parsing to the tool itself.
290275
Only detects that a tool call occurred (via stop strings) and passes text to the tool.
276+
277+
Requires exactly one tool (dr_agent_mcp) in tool_definitions.
291278
"""
292279

293-
def __init__(self, tool_actors: list[ray.actor.ActorHandle]):
294-
if len(tool_actors) != 1:
295-
raise ValueError(f"DRTuluToolParser requires exactly one tool (dr_agent_mcp), got {len(tool_actors)}")
280+
def __init__(self, tool_definitions: list[dict[str, Any]], stop_sequences: list[str]):
281+
if len(tool_definitions) != 1:
282+
raise ValueError(f"DRTuluToolParser requires exactly one tool (dr_agent_mcp), got {len(tool_definitions)}")
296283

297-
actor = tool_actors[0]
298-
self.tool_call_name = ray.get(actor.get_call_name.remote())
284+
self.tool_call_name = tool_definitions[0]["function"]["name"]
299285

300286
if self.tool_call_name != "dr_agent_mcp":
301287
raise ValueError(f"DRTuluToolParser requires dr_agent_mcp tool, got {self.tool_call_name}")
302288

303-
stop_strings = ray.get(actor.get_stop_strings.remote())
304-
# Use dict.fromkeys to deduplicate while preserving order
305-
self.stop_sequences = list(dict.fromkeys(stop_strings)) if stop_strings else []
289+
self.stop_sequences = list(dict.fromkeys(stop_sequences)) if stop_sequences else []
306290

307291
def get_tool_calls(self, text: str) -> list[EnvCall]:
308292
for stop in self.stop_sequences:
@@ -322,8 +306,8 @@ def get_available_parsers() -> list[str]:
322306
def create_tool_parser(
323307
parser_type: str,
324308
tokenizer: PreTrainedTokenizer | PreTrainedTokenizerFast,
325-
tool_actors: list[ray.actor.ActorHandle],
326309
tool_definitions: list[dict[str, Any]] | None = None,
310+
stop_sequences: list[str] | None = None,
327311
) -> ToolParser:
328312
"""Create a tool parser instance by type.
329313
@@ -333,8 +317,8 @@ def create_tool_parser(
333317
- "dr_tulu": DRTuluToolParser for <call_tool name="...">content</call_tool> format
334318
- "vllm_*": VllmToolParser variants (vllm_hermes, vllm_llama3_json, vllm_olmo3)
335319
tokenizer: Tokenizer for the model (required for all parser types).
336-
tool_actors: List of Ray actor handles for the tools.
337-
tool_definitions: OpenAI-format tool definitions (required for vllm_* parsers).
320+
tool_definitions: OpenAI-format tool definitions.
321+
stop_sequences: a list of stop sequences to use for stopping generations.
338322
339323
Returns:
340324
A ToolParser instance configured for the specified type.
@@ -343,12 +327,12 @@ def create_tool_parser(
343327
ValueError: If parser_type is unknown.
344328
"""
345329
if parser_type == "legacy":
346-
return OpenInstructLegacyToolParser(
347-
tool_actors=tool_actors, output_wrap_name="output", tool_definitions=tool_definitions
348-
)
330+
return OpenInstructLegacyToolParser(tool_definitions=tool_definitions, output_wrap_name="output")
349331

350332
if parser_type == "dr_tulu":
351-
return DRTuluToolParser(tool_actors)
333+
if tool_definitions is None or stop_sequences is None:
334+
raise ValueError("dr_tulu parser requires both tool_definitions and stop_sequences")
335+
return DRTuluToolParser(tool_definitions=tool_definitions, stop_sequences=stop_sequences)
352336

353337
if parser_type in VLLM_PARSERS:
354338
return create_vllm_parser(parser_type, tokenizer, tool_definitions=tool_definitions)

0 commit comments

Comments
 (0)