Conversation
There was a problem hiding this comment.
Code Review
This pull request migrates the project's documentation from Sphinx to MkDocs Material, introducing comprehensive quickstart guides, core concepts, and benchmark results. It also adds standalone inference examples, a sandbox demo, and updates training recipes to utilize verl's V1 unified trainer in colocate_async mode. The review feedback highlights a critical security issue with hardcoded Modal API credentials in the inference runtime environment. Additionally, several technical improvements are requested, such as lazily initializing asyncio.Semaphore in Ray actors to prevent runtime errors, implementing the stubbed run method in MiniSweAgentAgent to fix failing tests, using with blocks for safe file operations in task_runner.py, utilizing async with for OpenAICompatibleChatModel in the ReAct agent, and avoiding hardcoded paths in training shell scripts.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| MODAL_TOKEN_ID: "ak-MWuXc0JB73Ll3y75lnTl1K" | ||
| MODAL_TOKEN_SECRET: "as-b3iq9Xf3igKAb3DfPQVNwG" |
There was a problem hiding this comment.
Hardcoded Modal API credentials (MODAL_TOKEN_ID and MODAL_TOKEN_SECRET) have been committed to the repository. This poses a significant security risk.
Please replace these credentials with placeholders (similar to examples/quickstart/inference/runtime_env.yaml) and load them via environment variables or a local configuration file.
MODAL_TOKEN_ID: "<modal-token-id>"
MODAL_TOKEN_SECRET: "<modal-token-secret>"| class InferenceActor: | ||
| _semaphore = asyncio.Semaphore(max(1, GLOBAL_CONCURRENCY // NUM_WORKERS)) | ||
|
|
||
| async def run_single(self, sample: dict, task_defaults: dict) -> dict: | ||
| async with self._semaphore: |
There was a problem hiding this comment.
Creating an asyncio.Semaphore at class definition time (outside of any running event loop) is problematic. When this class is imported or instantiated on Ray workers, it can lead to RuntimeError: Task got Future attached to a different loop because the semaphore binds to the loop active during class loading (or no loop at all).
Instead, initialize the semaphore lazily inside the async method or within __init__ once the event loop is running.
class InferenceActor:
def __init__(self) -> None:
self._semaphore = None
async def run_single(self, sample: dict, task_defaults: dict) -> dict:
if self._semaphore is None:
self._semaphore = asyncio.Semaphore(max(1, GLOBAL_CONCURRENCY // NUM_WORKERS))
async with self._semaphore:| class TestEvalActor: | ||
| _semaphore = asyncio.Semaphore(max(1, GLOBAL_CONCURRENCY // NUM_WORKERS)) | ||
|
|
||
| async def run_single(self, sample: dict) -> dict: | ||
| async with self._semaphore: | ||
| task_config = sample["extra_info"]["tools_kwargs"]["task"] |
There was a problem hiding this comment.
Creating an asyncio.Semaphore at class definition time (outside of any running event loop) is problematic. When this class is imported or instantiated on Ray workers, it can lead to RuntimeError: Task got Future attached to a different loop because the semaphore binds to the loop active during class loading (or no loop at all).
Instead, initialize the semaphore lazily inside the async method or within __init__ once the event loop is running.
@ray.remote
class TestEvalActor:
def __init__(self) -> None:
self._semaphore = None
async def run_single(self, sample: dict) -> dict:
if self._semaphore is None:
self._semaphore = asyncio.Semaphore(max(1, GLOBAL_CONCURRENCY // NUM_WORKERS))
async with self._semaphore:| async def run( | ||
| self, | ||
| *, | ||
| sandbox: Sandbox, | ||
| messages: list[dict[str, Any]], | ||
| ) -> AgentResult: | ||
| pass |
There was a problem hiding this comment.
The run method for MiniSweAgentAgent is currently implemented as a stub (pass). However, a comprehensive test suite tests/uni_agent/agents/test_mini_swe_agent_agent.py has been added in this PR that asserts a full implementation of run returning an AgentResult.
Running the test suite will currently fail with an AssertionError because run returns None. Please implement the run method or mark the tests as skipped/expected to fail until the agent is fully implemented.
| entries = raw if isinstance(raw, list) else [raw] | ||
| index: dict[str, dict[str, Any]] = {} | ||
| for entry in entries: |
There was a problem hiding this comment.
The file is opened using open(path) without explicitly closing it or using a with statement. This can lead to unclosed file descriptor leaks.
Additionally, it is highly recommended to specify encoding="utf-8" when opening text files to ensure platform-independent behavior.
import yaml
with open(path, "r", encoding="utf-8") as f:
raw = yaml.safe_load(f)| toolbox = Toolbox.from_specs(cfg.tools, sandbox=sandbox) | ||
| model = OpenAICompatibleChatModel( | ||
| base_url=cfg.model.base_url, | ||
| api_key=cfg.model.api_key, | ||
| model_name=cfg.model.model_name, | ||
| sampling_params={ | ||
| "temperature": cfg.model.temperature, | ||
| "top_p": cfg.model.top_p, | ||
| "top_k": cfg.model.top_k, | ||
| }, | ||
| tools_schemas=toolbox.schemas(), | ||
| ) | ||
|
|
||
| transcript: list[dict[str, Any]] = list(messages) | ||
| for message in messages: | ||
| logger.info(f"{str(message.get('role', '')).upper()} PROMPT:\n{message.get('content', '')}") | ||
| trajectory_info: dict[str, Any] = { | ||
| "steps": 0, | ||
| "num_tool_calls": 0, | ||
| "timeouts": 0, | ||
| "errors": 0, | ||
| "total_tokens": 0, | ||
| "exit_reason": "unknown", | ||
| } | ||
| try: | ||
| async with toolbox.entered(retry=3, timeout=60): | ||
| for step_idx in range(1, cfg.max_steps + 1): | ||
| trajectory_info["steps"] = step_idx | ||
| stop_reason = await self.step(cfg, model, toolbox, transcript, trajectory_info) | ||
| if stop_reason != "completed": | ||
| trajectory_info["exit_reason"] = stop_reason | ||
| break | ||
| else: # loop ran the full step budget without an early stop | ||
| trajectory_info["exit_reason"] = "max_steps" | ||
| logger.warning(f"Reached max steps ({cfg.max_steps}) without finishing.") | ||
| except Exception as exc: # keep the partial transcript; the task buckets the failure | ||
| logger.exception("react loop failed at step %s", trajectory_info["steps"]) | ||
| trajectory_info["exit_reason"] = "unknown_error" | ||
| trajectory_info["error"] = f"{type(exc).__name__}: {exc}" | ||
| finally: | ||
| await model.aclose() # release the reused HTTP session |
There was a problem hiding this comment.
Since OpenAICompatibleChatModel implements __aenter__ and __aexit__ to manage its underlying HTTP session lifecycle, it is much more idiomatic and safer to use an async with block. This ensures that the model session is always closed cleanly, even if an exception is raised during initialization or before entering the main try block, preventing potential resource leaks.
toolbox = Toolbox.from_specs(cfg.tools, sandbox=sandbox)
async with OpenAICompatibleChatModel(
base_url=cfg.model.base_url,
api_key=cfg.model.api_key,
model_name=cfg.model.model_name,
sampling_params={
"temperature": cfg.model.temperature,
"top_p": cfg.model.top_p,
"top_k": cfg.model.top_k,
},
tools_schemas=toolbox.schemas(),
) as model:
transcript: list[dict[str, Any]] = list(messages)
for message in messages:
logger.info(f"{str(message.get('role', '')).upper()} PROMPT:\n{message.get('content', '')}")
trajectory_info: dict[str, Any] = {
"steps": 0,
"num_tool_calls": 0,
"timeouts": 0,
"errors": 0,
"total_tokens": 0,
"exit_reason": "unknown",
}
try:
async with toolbox.entered(retry=3, timeout=60):
for step_idx in range(1, cfg.max_steps + 1):
trajectory_info["steps"] = step_idx
stop_reason = await self.step(cfg, model, toolbox, transcript, trajectory_info)
if stop_reason != "completed":
trajectory_info["exit_reason"] = stop_reason
break
else: # loop ran the full step budget without an early stop
trajectory_info["exit_reason"] = "max_steps"
logger.warning(f"Reached max steps ({cfg.max_steps}) without finishing.")
except Exception as exc: # keep the partial transcript; the task buckets the failure
logger.exception("react loop failed at step %s", trajectory_info["steps"])
trajectory_info["exit_reason"] = "unknown_error"
trajectory_info["error"] = f"{type(exc).__name__}: {exc}"|
|
||
| project_name='Uni-Agent-SWE-Agent' | ||
| exp_name='GRPO-Qwen3-30B-R2E-Fully-Async' | ||
| RAY_DATA_HOME=/mnt/hdfs/yyding |
There was a problem hiding this comment.
Hardcoding RAY_DATA_HOME=/mnt/hdfs/yyding at the top of the script prevents users from overriding this path via environment variables, as the subsequent check RAY_DATA_HOME=${RAY_DATA_HOME:-...} will always see the hardcoded value.
Consider using the standard shell parameter expansion to allow environment overrides.
| RAY_DATA_HOME=/mnt/hdfs/yyding | |
| RAY_DATA_HOME=${RAY_DATA_HOME:-/mnt/hdfs/yyding} |
| # | ||
| # Launch from the repo root so Ray packages both `verl/` and `uni_agent/`. | ||
|
|
||
| RAY_DATA_HOME=/mnt/hdfs/yyding |
There was a problem hiding this comment.
Hardcoding RAY_DATA_HOME=/mnt/hdfs/yyding at the top of the script prevents users from overriding this path via environment variables, as the subsequent check RAY_DATA_HOME=${RAY_DATA_HOME:-...} will always see the hardcoded value.
Consider using the standard shell parameter expansion to allow environment overrides.
| RAY_DATA_HOME=/mnt/hdfs/yyding | |
| RAY_DATA_HOME=${RAY_DATA_HOME:-/mnt/hdfs/yyding} |
) ### What does this PR do? This PR makes Gateway sessions reuse an existing chain when a client rewrites only that chain's latest Assistant message. The Gateway records the token/message/media boundary immediately before each Assistant turn. When the next request preserves the earlier prefix but replaces that Assistant turn, it rolls the chain back to the recorded boundary, re-encodes the replacement suffix, and continues generation on the same chain. This avoids materializing a redundant trajectory while preserving token IDs, response masks, rollout log probabilities, and multimodal inputs. Last-Assistant rollback is enabled by default. Set `actor_rollout_ref.rollout.custom.agent_framework.enable_last_assistant_rollback=false` to retain the previous split-on-rewrite behavior. ### Checklist Before Starting - [x] Search for similar PRs or issues and paste at least one relevant link here: #83, #85, and #87 above - [x] Format the PR title as `[{modules}] {type}: {description}` (checked by CI) ### Test Automated validation on `upstream/main` at `ecdddea` (#87): ```text PYTHONPATH=/tmp/uni-agent-python-isolation:/tmp/uni-agent-verl-78bba31 \ python -m pytest \ tests/uni_agent/gateway \ tests/uni_agent/framework/test_generate_sequences_on_cpu.py -q 166 passed, 4 warnings in 110.78s ``` All hooks pass for files changed by this PR: ```text pre-commit run --from-ref upstream/main --to-ref HEAD --show-diff-on-failure ruff: pass ruff-format: pass mypy: pass compileall: pass ``` `pre-commit run --all-files` additionally reformats `examples/blackbox_recipes/sandbox_client.py`, an unchanged file introduced by #87. That unrelated baseline formatting change is intentionally excluded from this focused PR. #### CC rollout experiment Full rollout configuration: ```text model: Qwen/Qwen3.5-4B samples: 8 rollouts per sample: 4 sessions: 32 max concurrent sessions: 4 max agent turns: 100 prompt length: 90112 response length: 49152 tensor parallelism: 4 GPU memory utilization: 0.90 ``` Results: | Metric | Result | |---|---:| | Successful sessions | 32 / 32 | | Failed session UIDs | 0 | | Sessions that triggered rollback | 32 / 32 (100%) | | Total rollbacks | 229 | | Mean rollbacks per session | 7.16 | | Rollback count P25 / median / P75 / max | 4 / 5.5 / 7 / 27 | | Dropped trainable tokens | 165,733 | | Mean dropped trainable tokens per rollback | 723.7 | | Output trajectories | 50 | | Extra trajectories | 18 | | `max_response_length` materializations | 18 | | Non-length split trajectories | 0 | | Resolved SWE-bench samples | 3 / 8 (37.5%) | All 50 trajectory records satisfied: ```text len(response_ids) == len(response_mask) == len(response_logprobs) ``` The full 8x4 run was collected on the #83-rebased rollback implementation. After rebasing onto #85/#87, the rollback implementation in `session.py` is byte-for-byte identical except that the feature default changes from `false` to `true` in this PR. A fresh #87 smoke run used `MAX_SAMPLES=2`, `N=1`, and the same model/token budgets. It completed 2/2 sessions, produced three trajectories (one `max_response_length` materialization), and all response ID/mask/log-probability arrays remained aligned. The CC recipe currently needs a separate post-#83 reward-wiring compatibility fix. That fix was applied only in the local rollout workspace and is not part of this PR. Observed task-level warnings included agents reaching the configured 100-turn limit, one malformed Tool-call parse, and one 143,813-token prompt exceeding the 140,288-token model context. These were not rollback or token-alignment failures; the full experiment still finalized all 32 sessions. ### API and Usage Example Rollback is enabled when the option is omitted: ```yaml actor_rollout_ref: rollout: custom: agent_framework: gateway_count: 1 ``` To preserve the previous behavior and split a rewritten latest Assistant into a new chain: ```yaml actor_rollout_ref: rollout: custom: agent_framework: enable_last_assistant_rollback: false ``` ### Design & Code Changes - Capture a stable `LastAssistantStart` boundary before every Assistant generation, including response IDs, response masks, log probabilities, message-history length, media lengths, and the prefix hash. - Rank exact-prefix and rollback candidates by reusable service depth, preferring exact continuation on ties and refusing ambiguous deepest rollback matches. - On rollback, truncate the selected chain to its recorded boundary and re-encode only the incoming replacement suffix. - Preserve response-log-probability alignment and fail loudly if stored token truth is already inconsistent. - Track `rollback_count` and `rollback_dropped_trainable_tokens_total` in the session snapshot. - Forward the configuration through framework entry -> Gateway actor -> Gateway session. The default is `true`; explicit `false` remains supported. - Cover exact matches, ambiguous candidates, reserved chains, multimodal suffix re-encoding, response-budget materialization, log-probability alignment, and explicit opt-out behavior. This PR intentionally does not include the independent CC reward compatibility fix or rollout snapshot logging used to collect the experiment metrics. ### Checklist Before Submitting - [x] Read the [Contribute Guide](../CONTRIBUTING.md) - [ ] Run `pre-commit install && pre-commit run --all-files --show-diff-on-failure --color=always` - All PR files pass; the all-files command finds the unrelated #87 baseline formatting issue documented above. - [x] Add or update docs/examples for user-facing changes - [x] Add tests or explain why tests are not practical - [x] Confirm the PR title matches the required format - [x] Confirm the placeholder text in this template has been replaced with real content
### What does this PR do? Fix the Claude Code blackbox recipe after the runtime/task refactor in #83. The recipe still loaded the removed `uni_agent.reward` registry, so in-sandbox SWE-bench evaluation failed on current `main`. This PR migrates the recipe to the task-level `uni_agent.tasks.swe_bench.reward.compute_reward` API and adds the `exec_shell` adapter required by that evaluator. ### Checklist Before Starting - [x] Searched for related changes: #83 introduced the task-level reward API and removed the legacy registry. - [x] Formatted the PR title as `[examples] fix: migrate Claude Code reward to task API`. ### Test - Focused development regression probe: reproduced the missing `uni_agent.reward` import and missing `exec_shell`, then passed all 4 reward-adapter cases after the fix. The probe is not committed because this repository treats executable recipes as integration coverage and does not run recipe pytest suites in CI. - `python -m pytest tests/uni_agent/agents/test_claude_code_agent.py -q` — 8 passed. - `pre-commit run --all-files --show-diff-on-failure --color=always` — all hooks passed. - Integration evidence from the earlier rollback validation rollout, with this reward compatibility patch applied: 32 sessions produced 50 trajectories; all 50 reward records reported eval_completed=True, with no reward-path crash. ### API and Usage Example No new user-facing configuration is required. The existing Claude Code `run_infer.sh` and training recipe now evaluate SWE-bench through the post-#83 task reward API. ```python result = await compute_reward(metadata, sandbox, eval_timeout=600) score = 1.0 if result.get("resolved", False) else 0.0 ``` ### Design & Code Changes - Remove the obsolete recipe-local reward registry lookup. - Lazily call the canonical task-level SWE-bench reward function. - Reject unsupported reward data sources explicitly; this recipe is SWE-bench-specific. - Add `SandboxEnvForReward.exec_shell`, preserving quoted working directories and timeouts. ### Checklist Before Submitting - [x] Read the Contribute Guide and PR template requirements. - [x] Ran full-repository pre-commit checks. - [x] Explained why recipe integration evidence is used instead of adding a new pytest file. - [x] Confirmed the PR title matches the required format. - [x] Replaced all PR template placeholder text.
What does this PR do?
This PR refactors Uni-Agent around a unified set of composable runtime abstractions for agent inference, data synthesis, evaluation, and reinforcement learning.
New docs: https://uni-agent.readthedocs.io/en/latest/
Key changes:
Task,Agent,Sandbox,Tool, andToolboxinterfaces.Related:
Checklist Before Starting
Test
The following targeted checks were run:
API and Usage Example
A workload is now defined through one Task Config:
Tasks can also be instantiated directly:
Task configuration is resolved as:
This allows sample-wise customization while keeping model endpoints and credentials bound to the live runtime.
Design & Code Changes
Core abstractions
Taskowns the episode lifecycle, prompt, metadata, Sandbox, Agent, and reward computation.Agentdefines the solving strategy and supports white-box loops or black-box harnesses.Sandboxprovides a shared lifecycle and execution/data-plane interface across Local, Modal, veFaaS, and Seed backends.Tooldefines one model-visible action;Toolboxbinds stateful Tool instances to one Sandbox.All major abstractions use typed Pydantic configurations, registries, and lazy module loading.
Gateway and verl integration
The verl-managed rollout path is:
The Gateway supports OpenAI and Anthropic-compatible sessions and captures token-level trajectories for the training pipeline.
Inference workflows
Two inference modes are provided:
Examples live under:
Reliability and extensibility
Documentation
Breaking Changes
This PR intentionally replaces several legacy interfaces:
AgentEnv, interaction, deployment, and global reward-registry architecture.Existing integrations will need to migrate to the new registries and configuration format.
Checklist Before Submitting
pre-commit run --all-files --show-diff-on-failure --color=always