Remove <done> from vLLM stop tokens

[dzorlu]

Summary

• Remove <done> from vLLM stop tokens to fix premature generation termination
• Qwen3 thinking models output <done> inside <think> sections when describing their plan
• This caused vLLM to stop generation before the model could actually call tools

Problem

<think>
...I'll inform the user and include the <done>   ← vLLM STOPS HERE
</think>

Model was planning to say <done>, not actually signaling completion.

Solution

• Only stop on </tool_call> (for tool execution)
• Let model naturally finish with <|im_end|> (EOS token)
• env.step() already detects <done> in the full response to mark episode completion

Files Changed

• skyrl-train/tasks/openenv-fleet-grpo-qwen3-8b.yaml
• skyrl-train/tasks/openenv-fleet-grpo.yaml

🤖 Generated with Claude Code

[vercel]

Someone is attempting to deploy a commit to the Tyler's projects Team on Vercel.

A member of the Team first needs to authorize it.

[gemini-code-assist]

Code Review

This pull request introduces a major new feature: integration with Fleet-hosted environments for reinforcement learning training. It adds a comprehensive set of new modules, including a custom environment, dataset preparation scripts, S3 checkpointing, and extensive documentation and tests. The change to remove <done> from vLLM stop tokens, as mentioned in the title, is a small but important part of this larger integration. My review focuses on the new Fleet integration code. I've identified a critical issue with asynchronous code handling that could lead to runtime errors, as well as some high-severity issues related to external dependencies and error handling. Additionally, there are opportunities to improve maintainability by reducing code duplication and to fix inaccuracies in documentation. Overall, this is a substantial and valuable contribution that will be ready for merging after addressing the feedback.

[gemini-code-assist]

The step method is synchronous but calls asyncio.get_event_loop().run_until_complete() to execute an async function. Since the training loop is initiated with asyncio.run(), an event loop is already running. This will cause a RuntimeError: This event loop is already running.

To fix this, the step method should be defined as async def and use await to call self.openenv_task_env.step_async(). This change might require updating the BaseTextEnv class, but it's the correct approach for handling asynchronous operations within an async application. A similar issue exists on lines 268-270.

    async def step(self, action: str) -> BaseTextEnvStepOutput:
        """
        Execute one step in the Fleet environment.

        Parses the action for tool calls, executes via OpenEnv's FleetTaskEnv,
        and returns observation. Reward is computed by the verifier on completion.
        """
        self.turns += 1
        self.chat_history.append({"role": "assistant", "content": action})

        max_turns_reached = self.turns >= self.max_turns

        # Check if agent signals completion
        agent_done = "<done>" in action.lower() or "[done]" in action.lower()

        # Parse tool call from LLM response
        tool_call = parse_tool_call(action)

        tool_result = None
        error = None
        reward = 0.0

        # Execute tool call if present via OpenEnv
        if tool_call and self.openenv_task_env:
            # Build action dict for OpenEnv
            openenv_action = {
                "tool": tool_call["name"],
                "params": tool_call.get("arguments", {}),
                "done": agent_done,
            }

            try:
                # Use async step method
                obs, reward, done, info = await self.openenv_task_env.step_async(openenv_action)
                tool_result = obs.get("observation")
                if "tool_error" in info:
                    error = info["tool_error"]
            except Exception as e:
                error = str(e)
        elif agent_done and self.openenv_task_env:
            # Agent signaled done without tool call
            openenv_action = {"done": True}
            try:
                obs, reward, done, info = await self.openenv_task_env.step_async(openenv_action)
            except Exception as e:
                error = str(e)

        # Check if episode is done
        episode_done = agent_done or max_turns_reached

        # Build observation message
        if max_turns_reached:
            return BaseTextEnvStepOutput(
                observations=[],
                reward=reward,
                done=True,
                metadata={"done_reason": "max_turns", "task_key": self.task_key},
            )

        # Build response observation
        if error:
            obs_content = f"Error: {error}"
        elif tool_result:
            if isinstance(tool_result, dict):
                obs_content = f"Tool result:\n{json.dumps(tool_result, indent=2)}"
            else:
                obs_content = f"Tool result:\n{tool_result}"
        elif agent_done:
            obs_content = "Task marked as complete."
        elif not tool_call:
            obs_content = 'No tool call found. Use <tool_call>{"name": "...", "arguments": {...}}</tool_call> format.'
        else:
            obs_content = "Action executed."

        new_obs = {"role": "user", "content": obs_content}
        self.chat_history.append(new_obs)

        metadata = {
            "task_key": self.task_key,
            "turn": self.turns,
            "tool_call": tool_call,
            "tool_result": tool_result,
            "error": error,
            "done_reason": "agent_done" if agent_done else None,
        }

        return BaseTextEnvStepOutput(
            observations=[new_obs],
            reward=reward,
            done=episode_done,
            metadata=metadata,
        )

[gemini-code-assist]

The init method catches any exception during self.openenv_task_env.reset(), logs an error, and then continues. This can mask critical environment setup failures, leading to silent errors or unpredictable behavior later in the training process. The test test_init_reset_fails correctly expects a RuntimeError in this scenario. It would be better to re-raise the exception to make the failure explicit and prevent the system from continuing in a bad state.

Suggested change

	except Exception as e:
	logger.error(f"Failed to reset Fleet environment for task {self.task_key}: {e}")
	self._init_failed = True
	obs = {}
	except Exception as e:
	logger.error(f"Failed to reset Fleet environment for task {self.task_key}: {e}")
	raise RuntimeError(f"Failed to reset Fleet environment for task {self.task_key}: {e}") from e

[gemini-code-assist]

The setup script installs OpenEnv from a personal git branch (deniz/fleet_client). This introduces a dependency on a branch that may not be stable or could be deleted, making the build process fragile and difficult to reproduce. It is highly recommended to use a released version from PyPI or a stable branch from the main repository. This same issue is present on line 148.

  uv pip install openenv-client fleet-python

[gemini-code-assist]

The repository paths for Fleet SDK and OpenEnv are hardcoded to a local user directory. For this documentation to be useful to all contributors, these should be replaced with the public repository URLs.

Suggested change

	- Fleet SDK repo: `/Users/deniz/repos/fleet-sdk`
	- OpenEnv repo: `/Users/deniz/repos/OpenEnv`
	- Fleet SDK repo: https://github.com/fleet-ai/fleet-sdk
	- OpenEnv repo: https://github.com/fleet-ai/OpenEnv

[gemini-code-assist]

The comment here states that the module uses the Fleet SDK directly with no OpenEnv dependency. This is incorrect, as the implementation in integrations/fleet/env.py uses OpenEnvFleetTaskEnv. The comment should be updated to accurately reflect the dependency on OpenEnv.

Suggested change

	# It uses the Fleet SDK directly (no OpenEnv dependency).
	# It uses OpenEnv's FleetTaskEnv as an abstraction layer.

[gemini-code-assist]

The logic for uploading evaluation results to S3 is duplicated in both the evaluate and evaluate_step_wise functions (lines 254-269). To improve maintainability and avoid redundancy, this logic should be refactored into a separate helper function.