feat(renderers): single source of truth for renderer config (gateway + both backends)#706
Open
jeffreysijuntan wants to merge 27 commits into
Open
feat(renderers): single source of truth for renderer config (gateway + both backends)#706jeffreysijuntan wants to merge 27 commits into
jeffreysijuntan wants to merge 27 commits into
Conversation
…uation mangling (#656) * fix(sandbox): add [environment].replay_dockerfile toggle + fix RUN-continuation mangling Non-docker backends (modal/daytona) replay a task's Dockerfile RUN steps on top of the booted image. Two defects (#655): 1. Multi-line `RUN ... \` steps were rejoined with "\n" instead of a space, producing invalid shell (observed: `bash: syntax error near '&&'`). 2. The replay assumes the configured image is a *base*. Terminal-bench / Harbor tasks boot a *fully-built* image, so replaying the RUN steps double-applies the build (`git clone ... already exists`, missing COPY'd files, etc.). Fixes: - `_dockerfile_run_commands` joins `\`-continuations with a space so multi-line RUN steps stay valid when re-exec'd via `bash -c`. - New `[environment].replay_dockerfile` toggle. Default `true` preserves SWE-bench base-image behavior; `false` skips replay for prebuilt task images. Honored across the resolution cold path, daytona + modal snapshot builds, and the snapshot env-key fingerprint. - Tests for the continuation join, COPY skipping, and the toggle (incl. the task.toml -> metadata path). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style: apply ruff-format to satisfy pre-commit Collapse multi-line expressions onto single lines per ruff-format (line-length=200), fixing the failing pre-commit check on PR #656. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
save_config() rebuilt config.json from scratch, writing only provider/model/api_keys/base_url. Any other key was dropped — so `rllm model swap`/`setup` silently wiped the `ui_api_key` written by `rllm login`, logging the user out of the rLLM UI. Merge into the existing file instead (mirroring save_ui_config). Adds a regression test. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…#658) * feat(gateway): cumulative token mode for the in-process (Tinker) path `cumulative_token_mode` (drift-free multi-turn token forwarding: turn 2+ is rewritten to a pre-tokenized prompt built by renderers.bridge_to_next_turn, avoiding decode→re-encode drift) previously only worked for the HTTP-proxy (vLLM/verl) path — the cumulative handlers always routed to an HTTP worker's /v1/completions, so backends that run in-process via `local_handler` (Tinker) fell back to re-rendering + re-tokenizing the full conversation every turn. This wires the same feature through the local_handler path, reusing the existing backend-agnostic TokenAccumulator + Prime Intellect `renderers` bridge: - tinker_adapter: the handler now detects a pre-tokenized `prompt` (list[int]) and samples straight from it via the engine's existing get_token_output_from_token_input + assemble_model_output, returning a completions-style body (prompt_token_ids + choices[].token_ids) — the shape the gateway's cumulative handler already extracts token IDs from. The chat (messages) path is unchanged. - proxy: _handle_cumulative_non_streaming routes to local_handler when present (no HTTP worker); added _handle_cumulative_streaming_local to synthesize an SSE stream from the single in-process completion (mirrors _handle_streaming_local), with the same token ingest. Net effect: Tinker rollouts get the same prefix-extension guarantee verl has — turn N's prompt tokens are byte-for-byte prior turns' prompt+completion tokens, so the sequence the optimizer trains on matches what was generated. Tests (no Tinker service / model required): - tests/test_tinker_adapter_cumulative.py — token-prompt path samples from tokens; chat path unchanged; non-int prompt falls through. - rllm-model-gateway/tests/unit/test_cumulative_token_mode_local.py — proxy local cumulative non-streaming + streaming ingest + chat translation. Existing cumulative/accumulator suites (28) still pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * style(gateway): wrap long SSE chunk lines to satisfy ruff E501 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(cookbooks): add swe-rl recipe (rllm-swesmith → SWE-bench Verified)
End-to-end SWE-RL cookbook that pairs rLLM's native `rllm-swesmith`
training set with `harbor:swebench-verified` for eval, driving the
in-tree `mini-swe-agent` harness inside per-task sandboxes. Default
model is Qwen/Qwen3.5-9B + LoRA-32, GRPO + async + compact filtering,
64 parallel Daytona sandboxes.
No custom AgentFlow or evaluator: the harness owns the action loop,
each task's `tests/test.sh` is the verifier (pytest for swesmith, the
official SWE-bench harness for Verified), and the gateway captures
trajectories transparently.
Files: prepare_data.py (dataset pull), train.py + train_{tinker,verl}.sh
(recipes), test.py (catalog + harness smoke tests), README.md, pyproject.toml.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(cookbooks/swe-rl): use rllm.rollout.* keys; drop stale gateway overrides
The unified trainer config exposes sampling params at
`rllm.rollout.{train,val}.{temperature,top_p}`. The `sampling.*` paths
copied from `examples/harbor_swe/train_harbor.sh` predate the unified
config and break Hydra struct validation. Same template also passed
`rllm.gateway.public_url` / `rllm.gateway.sampling_params_priority`,
neither of which exist in the current schema — drop them.
Verified by resolving the config with --cfg job: no validation errors.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(swe-rl): run on rLLM-native SandboxedAgentFlow path; unblock async training
Switch the cookbook off the remote Harbor runtime onto rLLM's own
SandboxedAgentFlow path (AgentFlowEngine) and fix the bugs that surfaced
once real rollouts ran end-to-end on Modal sandboxes.
train.py / train_*.sh:
- Pass MiniSweAgentHarness as agent_flow so AgentTrainer auto-wires
SandboxTaskHooks + per-task verifiers via AgentFlowEngine (not the
remote_runtime/RemoteAgentFlowEngine path). Sandbox backend selected by
SWE_SANDBOX_BACKEND (default modal).
- Load the val split as "default" (Harbor-pulled name), not "test".
- Async training requires train_batch_size=1 and raise_on_error=false; set
both. Effective batch is mini_batch_size(16) groups x group_size(8).
- Add SWE_VAL_MAX to cap the 500-task SWE-bench-Verified val set.
- Drop stale rllm.remote_runtime.* overrides; doc the sandbox backends.
rllm/data/utils.py:
- task_from_row now roots the Task at the row's task_path and merges
task.toml/Dockerfile metadata, so per-task verifier + image resolution
work on the training path (fixes "No verifier configured").
rllm/gateway/tinker_adapter.py:
- Translate TerminationEvent into an OpenAI-standard 400
context_length_exceeded instead of a 500. litellm maps it to a
non-retryable ContextWindowExceededError, so an over-length in-sandbox
agent stops immediately instead of retrying until the run-timeout
SIGKILL — which was stalling group completion and wedging async steps.
rllm/harnesses/mini_swe_agent.py:
- Retry the in-sandbox uv install (Modal->GitHub egress resets the
connection intermittently and aborted the install under set -e).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* feat(gateway): cumulative token mode for the in-process (Tinker) path
`cumulative_token_mode` (drift-free multi-turn token forwarding: turn 2+ is
rewritten to a pre-tokenized prompt built by renderers.bridge_to_next_turn,
avoiding decode→re-encode drift) previously only worked for the HTTP-proxy
(vLLM/verl) path — the cumulative handlers always routed to an HTTP worker's
/v1/completions, so backends that run in-process via `local_handler` (Tinker)
fell back to re-rendering + re-tokenizing the full conversation every turn.
This wires the same feature through the local_handler path, reusing the
existing backend-agnostic TokenAccumulator + Prime Intellect `renderers`
bridge:
- tinker_adapter: the handler now detects a pre-tokenized `prompt` (list[int])
and samples straight from it via the engine's existing
get_token_output_from_token_input + assemble_model_output, returning a
completions-style body (prompt_token_ids + choices[].token_ids) — the shape
the gateway's cumulative handler already extracts token IDs from. The chat
(messages) path is unchanged.
- proxy: _handle_cumulative_non_streaming routes to local_handler when present
(no HTTP worker); added _handle_cumulative_streaming_local to synthesize an
SSE stream from the single in-process completion (mirrors
_handle_streaming_local), with the same token ingest.
Net effect: Tinker rollouts get the same prefix-extension guarantee verl has —
turn N's prompt tokens are byte-for-byte prior turns' prompt+completion tokens,
so the sequence the optimizer trains on matches what was generated.
Tests (no Tinker service / model required):
- tests/test_tinker_adapter_cumulative.py — token-prompt path samples from
tokens; chat path unchanged; non-int prompt falls through.
- rllm-model-gateway/tests/unit/test_cumulative_token_mode_local.py — proxy
local cumulative non-streaming + streaming ingest + chat translation.
Existing cumulative/accumulator suites (28) still pass.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(swe-rl): enable cumulative token mode + raise context window
Enables the in-process cumulative-token path (cherry-picked from #658) in the
tinker recipe for testing: rllm.gateway.cumulative_token_mode=true +
renderer_family=qwen3.5. Also raises training.max_length=65536 /
data.max_prompt_length=57344 so long mini-swe-agent trajectories fit, and
save_freq=10.
NOTE: the cumulative feature commit on this branch is a cherry-pick of #658 for
local testing — drop it (rebase onto main) once #658 merges to avoid duplication.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(swe-rl): add synchronous tinker training script
train_tinker_sync.sh: on-policy variant of train_tinker.sh for testing —
drops async_training (synchronous generate→train) and uses a real
data.train_batch_size (default 4; effective batch = train_batch_size x
group_size). Same model/sandbox/context/cumulative settings otherwise.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(swe-rl): disable cumulative token mode (incompatible with thinking model)
Qwen3.5 is a thinking model (<think>...</think> per assistant turn). The normal
re-render path strips prior-turn reasoning from history (mini-swe-agent stores
the think-stripped message.content), but cumulative mode carries forward the RAW
completion tokens (tinker_engine completion_ids=response_tokens, incl. <think>),
which renderers.bridge_to_next_turn concatenates verbatim. The model then sees an
ever-growing stack of its own prior reasoning — out-of-distribution for multi-turn
— degrading into short, submit-early, zero-reward trajectories.
Renderer mismatch was ruled out: the bridge (PrimeIntellect qwen3.5) and the
engine renderer are token-identical; the bridged prompt is well-formed. The
issue is purely reasoning carry-forward. Cumulative mode is fine for
non-thinking models / single-turn; for thinking models, correct multi-turn
rollout requires stripping prior reasoning (i.e. re-tokenization), which is
fundamentally at odds with exact-token reuse.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(swe-rl): switch to Qwen3.5-4B + terminus2 harness
- agent_flow -> Terminus2Harness (was MiniSweAgentHarness) in train.py;
docstring/script prose updated accordingly.
- model.name / MODEL_PATH -> Qwen/Qwen3.5-4B; renderer_family -> qwen3.5;
experiment names -> r2egym-terminus2-qwen3.5-4b[-sync/-verl].
- re-enable rllm.gateway.cumulative_token_mode in the tinker scripts.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test(swe-rl): bump n_parallel_tasks to 128 (tinker + sync)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* Add train_fireworks.sh Co-Authored-By: 1stprinciple <ztyharbin@gmail.com> * Support FireworksEngine in gateway Co-Authored-By: 1stprinciple <ztyharbin@gmail.com> * add max_prompt_length limit Co-Authored-By: 1stprinciple <ztyharbin@gmail.com> * use tokenizer_model when exists. Co-Authored-By: 1stprinciple <ztyharbin@gmail.com> * style(gateway): fix ruff formatting (trailing whitespace) Co-Authored-By: 1stprinciple <ztyharbin@gmail.com> * feat(swe-rl): scale Fireworks rollout deployment to 4 replicas Set fireworks_config.rollout_deployment_replica_count=4 in both the async and sync Fireworks scripts (the prior fireworks_config.replica_count key does not exist in the backend config). Co-Authored-By: 1stprinciple <ztyharbin@gmail.com> --------- Co-authored-by: 1stprinciple <ztyharbin@gmail.com>
…ks (#668) * feat(algorithm): add ECHO env-observation loss across verl/tinker/fireworks ECHO (arXiv:2605.24517) augments GRPO with an auxiliary cross-entropy loss on environment-observation tokens — the tool/terminal output the policy conditions on but that GRPO masks out of its loss. Advantages are identical to GRPO; the only change is the extra loss term, so every rollout (including failures) becomes dense supervision at no extra rollout cost. Selectable via the unified trainer on all three backends: rllm.algorithm.adv_estimator=echo # lambda defaults to paper's 0.05 rllm.algorithm.env_loss_coef=<lambda> # override; 0.0 == plain GRPO Shared core (backend-agnostic): - rLLMAdvantageEstimator.ECHO + env_loss_coef on AlgorithmConfig; None resolves to 0.05 for echo, else 0.0 (sentinel distinguishes unset from explicit 0). - echo registered as a GRPO-advantage alias in the estimator registry. Per-backend env-CE term (the only backend-specific part): - verl: added in CustomPPOLoss, reusing the existing forward pass log-probs and GRPO's loss_agg_mode/global-batch normalization. Single pass, exact, free. - tinker: gradient-accumulated cross_entropy pass (weights=lambda on obs tokens), returned separately so policy-pass logprobs stay aligned. Clean because tinker's optim_step applies no extra normalization. - fireworks: same second pass, but normalization is folded into weights/advantages and GradAccNormalization is forced to NONE (its counting normalization spans passes and would otherwise rescale the policy gradient). Defaults to 0.0 everywhere, so existing GRPO runs are unchanged. Adds ECHO config tests and documents the switch in the swe-rl cookbook README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(design): auxiliary-loss framework (AuxiliaryLoss) + ECHO/SDAR clients Design doc for a general token-level auxiliary-loss abstraction so algorithms like ECHO and SDAR are a registered spec + config instead of bespoke per-backend surgery. Covers: named token masks (data layer), the AuxiliaryLoss interface (mask/target/weight/requires), a registry, the aux_losses config list, the two backend executors (in-process for verl; gradient-accumulated cross_entropy pass for tinker/fireworks), ECHO and gap-gated SDAR as reference clients, the verl/managed capability model with graceful degradation, and an incremental migration plan that ports the just-merged ECHO onto the framework first. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(algorithm): generalize ECHO into an AuxiliaryLoss framework Prototype of the auxiliary-loss abstraction from design/auxiliary-losses.md (migration steps 1-3): token-level auxiliary losses become a registered spec + config instead of bespoke per-backend code, and ECHO is ported onto it as the first client. Behavior-preserving for ECHO. Shared core (rllm/trainer/algorithms/aux_loss.py): - AuxiliaryLoss spec (mask / weight / requires), register_aux_loss registry, EnvPredictionLoss, and build_aux_losses() with env_loss_coef / adv_estimator=echo back-compat sugar. New algorithm.aux_losses config list (a stack of {type, coef, ...} specs). Backend executors (the only backend-specific part): - verl: CustomPPOLoss._apply_aux_losses folds each term into the single existing forward/backward pass (replaces _add_env_loss). - tinker: _get_aux_loss_futures submits one gradient-accumulated cross_entropy pass per loss (replaces _build_env_datum/_get_env_loss_futures). - fireworks: _build_aux_datums + the normalization fold / GradAccNormalization.NONE now driven by build_aux_losses() rather than env_loss_coef. - Shared managed datum builder in rllm/trainer/tinker/aux_loss.py (aux_positions + build_aux_ce_datum), used by both tinker and fireworks. Adding a new aux loss is now one registered class + config, no per-backend surgery. SDAR's dynamic-weight + teacher-forward extensions (the `requires` field) are the next milestone and currently raise NotImplementedError. Tests: tests/unified_trainer/test_aux_loss.py covers the registry, config resolution + ECHO sugar, the managed datum builder, and the executor math (incl. empty-observation safety). ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style(tinker): satisfy ruff UP038 in aux_loss isinstance Use `int | float` instead of `(int, float)` in the isinstance call (rllm/trainer/tinker/aux_loss.py:51) to fix the pre-commit ruff failure on PR #668. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(algorithms): rewrite RL algorithms overview + register in nav (#675) The core-concepts/rl-algorithms page was stale (documented an old kl_ctrl/AgentPPOTrainer API, only GRPO/PPO/ReMax) and unlinked from the nav. Rewrite it as an accurate overview of the current algorithm surface: - built-in advantage estimators (grpo, reinforce, reinforce_plus_plus_baseline, prpo, rloo, echo) with exact adv_estimator strings - ECHO + the auxiliary-loss framework (env_loss_coef / aux_losses, per-backend cost, metrics) - how to build custom RL algorithms: custom advantage estimator (@register_rllm_adv_estimator) and custom auxiliary loss (@register_aux_loss) - config quick reference + cross-links to advantage-estimator / configuration / capability-matrix / backends Registers the page under the "Advanced algorithms" nav group. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…#674) End-to-end terminal-agent RL recipe, sibling of cookbooks/swe-rl: - train on a local set of Harbor-format terminal-agent tasks (a .tar.zst you provide via TB_TRAIN_TARBALL), eval on harbor:terminal-bench@<TB_EVAL_VERSION> (default 2.0; flip to 2.1 once published) - terminus2 harness runs inside per-task sandboxes; per-task tests/test.sh is the verifier (writes /logs/verifier/reward.txt) - tinker / tinker-sync / verl / fireworks / fireworks-sync train scripts - README documents the ECHO switch (rllm.algorithm.adv_estimator=echo) The training dataset is not public yet, so the README/scripts describe it generically and take the tarball path from TB_TRAIN_TARBALL. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
#677) * feat(harness): configurable rollout limits — uncapped turns, longer timeout, Modal lifetime headroom Stop cutting sandboxed CLI rollouts short, and make the limits configurable rather than hardcoded: - terminus2: max_turns defaults to None (no artificial cap — Harbor's own default) instead of 50. RLLM_TERMINUS_MAX_TURNS is emitted only when set, and the in-sandbox driver treats an absent value as uncapped. - BaseCliHarness: per-rollout run_timeout default 1800 -> 3600s; configure() now consumes an `agent_timeout` override. - eval CLI: add `--agent-timeout SECONDS` (routed through configure(); also published as RLLM_HARNESS_RUN_TIMEOUT_S so the sandbox lifetime tracks it). - modal_backend: derive the sandbox lifetime as run_timeout + install_timeout + headroom so the container can't be reaped before the agent's own timeout fires — the exit-137 / "Sandbox already shut down" failure. The lifetime clock starts at create, before install + run + verify, so a flat default collided with the run timeout. RLLM_MODAL_SANDBOX_TIMEOUT_S still overrides. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(cookbooks): wire TERMINUS_MAX_TURNS knob + tune terminal-rl fireworks config - terminal-rl & swe-rl train.py: read the TERMINUS_MAX_TURNS env knob (empty/0 = uncapped) and pass it to Terminus2Harness(max_turns=...). - all train_*.sh: default TERMINUS_MAX_TURNS=100 (overridable per run), so the operational cap lives in the scripts rather than the harness. - terminal-rl/train_fireworks.sh: grow the context window to match longer, uncapped rollouts — training.max_length 64k->128k, data.max_prompt_length 56k->120k, enable gateway.cumulative_token_mode, and lower async staleness_threshold to 0.5. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Surface the loaded flow's effective knobs as a "Config" row in the `rllm eval` header (alongside Benchmark / Model / Agent / Evaluator): max turns, temperature, run/install timeouts, max concurrent. Knobs are read off the agent instance via a curated (attr, label, formatter) spec, so a flow that doesn't expose one simply omits that chip and an agent exposing none gets no row — graceful across harnesses (react, CLI agents) and harbor runtimes. Rendered as dim-key / bold-value chips joined by a dim "·"; spaces within a chip are non-breaking so a pair like "run timeout 3600s" never splits across a line wrap. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(fireworks): per-trajectory session affinity + prompt-cache metric Fireworks reuses a replica's prompt-prefix KV only when a trajectory's turns route to the same replica (keyed on a session id via request headers). The rollout path set no such header, so multi-turn requests scattered across replicas and re-prefilled the (growing) prompt every turn. (a) Session affinity: - Gateway threads the per-rollout session id into the in-process handler body (rllm_session_id) at all local-handler call sites. - tinker_adapter forwards it only to engines advertising supports_session_affinity (Fireworks), so the tinker path is unchanged. - FireworksEngine sets x-multi-turn-session-id / x-session-affinity per request. Since DeploymentSampler.async_completions_stream has no per-call header hook, _inference_headers is monkeypatched to merge headers stashed in an async-safe ContextVar set around each call. (b) Prompt-cache metric: - _record_prompt_cache accumulates ServerMetrics.cached_prompt_tokens / prompt_tokens, adds a per-call prompt_cache_hit_ratio to the metrics dict, and logs a rolling hit-ratio every 50 completions. Low ratio => affinity not pinning a trajectory to one replica. Both are automatic — no config change. Verify by watching the "Fireworks prompt cache: hit_ratio=..." log climb across a rollout. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(fireworks): rename session headers + drop in-process prompt-cache recording - Rename affinity_headers -> session_headers (the per-request x-multi-turn-session-id / x-session-affinity headers). - Drop the in-process prompt-cache recording (_record_prompt_cache, the rolling counters, and the per-call prompt_cache_hit_ratio metric); read cache behaviour from the Fireworks console instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…n the picker Two CLI UX fixes for the Fireworks eval path. Sampling params: - `rllm eval` now seeds temperature=1.0, top_p=1.0 (mirrors the cookbook training rollout), overridable by --sampling-params / --temperature / --top-p and always shown in the header's gateway-enforced "Sampling" row. - Removed the duplicate temperature from the "Config" panel — it showed the harness's requested value (0.7) while the gateway enforced a different one (e.g. 1.0), so the same param appeared twice with conflicting values. - Terminus2Harness default temperature 0.7 -> 1.0 (and the driver fallback) so nothing lingers at 0.7 when sampling isn't gateway-enforced. Fireworks deployment fetch: - fetch_fireworks_deployed_models only queried the deployedModels endpoint (populated when a custom/fine-tuned model is bound to a deployment), so a dedicated deployment of a base model — which has no deployedModel entry and is addressed for inference by its accounts/<acct>/deployments/<id> name — was invisible in the `rllm setup` picker, even though that's the exact id evals run against. Now also lists READY deployments by their id (e.g. gpdl1exc). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The agentflow_engine tqdm bar only updated the completed count; add a set_postfix with the cumulative mean episode reward so the current score is visible live during a run instead of only per-rollout lines. Falls back to is_correct for error/empty episodes. Display only. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… metrics, prompt-cache affinity
Terminal-bench eval on Modal scored ~30-33% vs official Terminus-2 ~40%, with
flaky "Command failed" warnings. Root cause was infra, not model quality.
The Modal sandbox lifetime was derived from a global env var (a flat 4800s),
while the agent runs for the per-task agent_timeout (600-12000s) and the
verifier then runs in the same box. 14 of 89 terminal-bench@2.0 tasks have
agent+verifier > 4800s, so the box was reaped before the verifier's exec
("Sandbox already shut down") -> the whole episode became a 0-step ERROR
(all agent work discarded). With retry_limit=1, each became a permanent zero.
Changes:
- eval/_resolution: size the Modal sandbox per-task, max(override,
agent_timeout + verifier_timeout + install + slack), so the box always
outlives the agent + verifier it hosts.
- eval/runner: eval retry_limit 1 -> 2, so transient infra failures get one
retry instead of becoming permanent zeros (matches the official harness).
- engine/agentflow_engine: live running accuracy/reward on the progress bar
(acc N/done=% reward=...) so the score is visible as the eval runs.
- sandbox/protocol + modal_backend: classify an exec killed at its own timeout
as SandboxCommandTimeout instead of a generic "Command failed (exit -1)";
cli_harness catches it and logs at INFO ("reached its time budget") — an
agent spending its full per-task budget is expected, the verifier still
scores the partial state.
- gateway proxy: inject x-session-affinity / x-multi-turn-session-id from the
rollout session id on the HTTP path, so eval multi-turn trajectories reuse
Fireworks' per-replica prompt cache. Previously only the in-process training
path set affinity; eval (HTTP proxy path) sent none.
Note: modal_backend.py also carries a pre-existing _CreateRateLimiter (Modal
create-rate pacing) that shares the new `import time`; it is bundled here
because it lives in the same file and is also eval-reliability related.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ask agent_timeout
BaseCliHarness.run() resolved the rollout exec timeout as
`task.metadata.get("agent_timeout", self.run_timeout)`, so a task carrying a
per-task agent_timeout always won and an operator-set
RLLM_HARNESS_RUN_TIMEOUT_S was only a fallback that never fired. Harbor tasks
always carry one — task_from_row merges task.toml's [agent].timeout_sec into
metadata — so e.g. training tb-opus-pass with RLLM_HARNESS_RUN_TIMEOUT_S=1800
silently ran rollouts to the task's 3600s budget. The intended wall-clock cap
was ignored.
Make run_timeout a hard ceiling when an operator sets it (env var or
--agent-timeout): effective timeout = min(per-task agent_timeout, run_timeout).
When it's unset, the task's own agent_timeout governs, so eval still honors each
benchmark's per-task budget (no regression to the terminal-bench parity work).
A run_timeout_is_cap flag (captured from the env at import, flipped on by
configure() for --agent-timeout) distinguishes the two.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…xy resilience (#684) * feat(sandbox/modal): isolate each run's sandboxes by run id Name the Modal App per-run (rllm-sandbox-<run_id>) and stamp each sandbox with a name + rllm_run_id tag, so on a shared account you can list/terminate just one run's (leaked) sandboxes via 'modal app stop rllm-sandbox-<run_id>'. run_id comes from RLLM_RUN_ID (sanitized) or a random per-process id (new rllm.env.rllm_run_id()). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(eval): proxy-death monitor, empty-completion canary, streaming UI uploads proxy.py: capture the litellm proxy's stdout+stderr (was DEVNULL) and add a monitor thread that loudly reports if the proxy dies mid-run — previously a dead proxy was silent and every rollout returned empty with no visible cause. agentflow_engine.py: flag rollouts whose LLM completions are ALL empty (upstream down — dead proxy / no healthy workers / model) and stream each enriched+scored episode to the UI as it finishes. runner.py + tracking.py: invoke on_episode_complete streaming (per-rollout) instead of an end-of-run burst; make the UI drain timeout env-tunable (RLLM_UI_DRAIN_TIMEOUT_S, default 600s). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * build(deps): bump fireworks-ai to 1.2.0a83 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(gateway): raise worker health-check timeout 5s -> 20s Under a concurrency spike the single litellm proxy worker is slow to answer /health while busy forwarding requests; a 5s timeout marked it dead (3 strikes) and route() then raised 'No healthy workers available', failing agent turns. 20s tolerates the transient slowness. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(eval): auto-pick free port for eval proxy so concurrent jobs don't collide The auto-started LiteLLM proxy hardcoded port 4000, so two concurrent `rllm eval` jobs collided on it — and the loser's startup probe could see the winner's proxy answering and then clobber its routing via /admin/reload. EvalProxyManager now binds a free port by default (proxy_port=None), with a retry-on-bind-race so concurrent launches each settle on their own port. Add --proxy-port to pin a deterministic port when debugging a single run. The gateway layer was already free-port, so port 4000 was the only collision point. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: signalrush <269811712+signalrush@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rLLM's native sandbox path (SandboxTaskHooks + rllm/sandbox backends, used when a non-`harbor:` agent runs a harbor-sourced dataset) reimplements task setup/exec instead of calling Harbor, and had drifted from how Harbor's Modal environment actually starts and configures a task. Bring it back into line: - Prebuilt images boot as-is. A task that declares its own [environment].docker_image ships a fully-built image; replaying its Dockerfile RUN steps on top double-applies the build. Default replay_dockerfile off in that case (matches Harbor's should_use_prebuilt_docker_image). SWE-bench-style tasks (no docker_image: base image + RUN extras) keep replay on. - Resource limits actually apply. Harbor task.toml uses size strings (memory = '4G'); rLLM only read memory_mb/storage_mb, so the limit was silently dropped to the Modal default. Normalize the string forms to *_mb. - Real user switching on Modal. exec() now applies `user` via `su <user> -s /bin/bash -c` (matching harbor.environments.modal) instead of ignoring it and running everything as root. - Task env reaches every exec. [environment].env is exported into every command (ModalSandbox.set_env), mirroring Harbor's per-exec Secret, instead of a one-shot `export` that didn't survive a fresh shell. - Keepalive on sandbox create, so the box stays alive regardless of the image's ENTRYPOINT/CMD (matches Harbor). Verifier/reward dirs locked away from agent_user are now chowned to the verifier's user (root unless a distinct verifier_user is set) so the now-su'd verifier can still write reward files. Tests: tests/eval/test_harbor_parity.py + tests/sandbox/test_modal_backend.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ion on the queued log line (#690) * feat(trainer): per-task datum/step/segment breakdown when a task is queued A task is consumed as one GRPO group, which prefix-merges into training rows; previously only batch-level merge metrics were visible. Log per-task at INFO when the group is finalized: number of groups, trajectories, steps (turns), and datums (rows after prefix-merge), plus steps/datum. _segment_count() applies the same prefix-extension check the backend transform uses, so the per-task datum count matches what's trained and the merge ratio is visible per task, not just batch aggregate (steps/datum == 1 is a clean cumulative trajectory; >1 a prefix break). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(trainer): add per-task group reward distribution to the queued log line Extend the per-task "Task ... queued" accounting (cherry-picked from feat/native-renderers) with the GRPO group's reward distribution: avg/min/max over its trajectories. Advantages are driven by within-group reward spread, so a min==max group carries no learning signal — surfacing this per task makes a degenerate group visible at a glance, alongside the steps->datums merge ratio. _traj_reward() mirrors the reward extraction in _log_prompt_group_finished (trajectory-level reward, falling back to the last step's reward) so the per-task average matches the per-episode rewards logged there. Unscored trajectories are excluded; an all-unscored group reports 0.0 without error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Reproduce Ai2's Tmax recipe (arXiv:2606.23321) in rLLM. Adds: - rllm/data/tmax_builder.py: native builder for the TMax-15K corpus. The Harbor-registry copy their README cites (tmax/TMax-15K-Harbor) is not in the public Harbor registry, so build from Hugging Face instead — join allenai/TMax-15K (test_final_state pytest verifier + description) with allenai/tmax-15k-open-instruct (per-task prebuilt images) by task_id, and materialize Harbor task dirs. Reward = test_final_state.py pytest pass/fail -> /tmp/rllm/reward.json. Same pattern as r2egym/swesmith builders. - rllm/registry/datasets.json: tmax-15k catalog entry (builder-backed). - cookbooks/tmax/: prepare_data.py, train.py, train_verl.sh (faithful full-FT DPPO->GRPO + centered advantages, group 32, 65K, lr 1e-6, no KL, 64 turns), train_fireworks.sh / train_tinker.sh (LoRA variants), test.py, README. Sibling of cookbooks/terminal-rl. Builder join + materialization validated on real HF data; in-container reward execution needs Docker Hub image access. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
#692) * feat(gateway): classify and explain TokenAccumulator resets in the log Cumulative-token mode previously logged a single ambiguous line on reset ("Resetting TokenAccumulator (was at turn N, M messages)") that collapsed four distinct causes and omitted the session id, so it was impossible to tell why a reset happened. A turn-1 reset in particular could be an upstream retry, a rewritten history, an assistant-only delta, or a renderer that can't bridge — all indistinguishable. Introduce a ResetReason taxonomy named by the observable structural relationship between the incoming request and the accumulated snapshot: - duplicate: incoming list identical to the last processed turn (conversation did not advance) — likely upstream retry - prefix_changed: snapshot-covered messages shrank or diverged (history compacted/edited, or session id reused) - empty_delta: clean extension, but only assistant message(s) added - renderer_no_bridge: valid extension the renderer declined A single plan_turn() classifier replaces the split is_cumulative / extract_new_messages logic at the proxy call site. reset() now logs at INFO with the session id, the machine-readable reason, a short plain-language explanation, reason-specific diagnostics (age_s, first_divergent_index, ...), and a reset_count that surfaces reset storms on one session. Switch the snapshot from a whole-list fingerprint to per-message fingerprints so prefix_changed can report the first divergent message and a short preview. Tests: per-reason classification + reset-logging coverage added; existing cumulative-mode unit + integration tests still pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(gateway): add snapshot/incoming token counts to TokenAccumulator reset log The reset line now reports snapshot_tokens (prompt+completion of the last ingested turn = the accumulated history size right before the reset) and, for prefix_changed resets, incoming_tokens (best-effort token count of the incoming, typically compacted, request). Comparing snapshot_tokens against the model's context limit makes token-limit-triggered compaction obvious from the log, and incoming_tokens shows how far the history shrank. snapshot_tokens is free (the accumulator already holds the token IDs); incoming_tokens is computed via renderer.render_ids guarded so diagnostics can never break the reset path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style(gateway): satisfy ruff-format on TokenAccumulator reset log string Join the reset log format string onto one line; ruff-format (pre-commit) rejected the manual two-line split. No behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…hful (#693) * feat(sandbox): make Daytona backend training-ready & Harbor-eval-faithful Bring the Daytona sandbox backend to parity with what e4e91cf (+#684) did for Modal, so TERMINAL_SANDBOX_BACKEND=daytona reproduces Harbor's Daytona eval performance and trains stably at GRPO scale. All Path-B execution semantics now match how Harbor's Daytona environment starts/configures a task. P0 — fidelity blockers (Daytona reward now matches Harbor): - Per-exec user switching: exec honors `user` via `su <user> -s /bin/bash -c` (resolving int UIDs via getent), so agent and verifier run under their declared users and the verifier-dir ownership setup in _resolution actually isolates. Mirrors modal_backend._build_exec_command and harbor's Daytona env. - SandboxCommandTimeout + guaranteed hard-kill: timed commands are wrapped in coreutils `timeout -k 10 <t>` (SIGTERM then SIGKILL) with the SDK timeout as a +60s backstop. Exit 124/137 and SDK-level timeout exceptions map to SandboxCommandTimeout, which cli_harness treats as "expected, score anyway" rather than a failure. A runaway agent can no longer hang the rollout. - set_env: declared [environment].env is re-exported into every exec (Daytona shells are independent), so the verifier reliably observes task env. P1 — training-scale stability: - Create retry + rate-limiting: a _CreateRateLimiter token bucket (RLLM_DAYTONA_SANDBOX_CREATE_RPS/_BURST) plus a _create retry loop with linear backoff on transient/rate-limit/5xx errors; a vanished snapshot / real validation error stays non-transient and cold-falls-back. Prevents GRPO's concurrent-create burst from losing rollouts. - Task-sized lifetime: _resolution sizes Daytona auto_stop_interval from agent+verifier+install+slack (RLLM_DAYTONA_SANDBOX_AUTOSTOP_MIN floor), so a long or stalled rollout isn't reaped mid-run. Also filter build_daytona_snapshot's Resources(**...) to compute-only keys (required for the new lifecycle kwarg; also fixes create_timeout previously leaking into Resources()). Tests: 22 new unit tests covering su/env wrapping, transient classification, the rate limiter, exec timeout->SandboxCommandTimeout (124/137, SDK exception, and the 124-without-timeout edge), and the create retry loop — all against fakes so no SDK/API key is required. Design doc at design/daytona-training-readiness.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(sandbox): unify sandbox lifetime knob into provider-agnostic RLLM_SANDBOX_TIMEOUT_S The sandbox-lifetime override was Modal-specific (RLLM_MODAL_SANDBOX_TIMEOUT_S), and the just-added Daytona path used a separate RLLM_DAYTONA_SANDBOX_AUTOSTOP_MIN. Two names for one concept ("how long may a sandbox live") is wrong now that the same training scripts can target either backend. Introduce a single provider-agnostic knob, RLLM_SANDBOX_TIMEOUT_S (seconds): - rllm/env.py: new sandbox_timeout_override_s() reads it, falling back to the deprecated RLLM_MODAL_SANDBOX_TIMEOUT_S (one-time warning) so existing scripts keep working. - _resolution.py: compute the per-task lifetime floor (agent+verifier+install+ slack) ONCE and raise it to the override, then each backend expresses it in its own unit — Modal's hard `timeout` in seconds, Daytona's idle `auto_stop_interval` in minutes. Dedupes the arithmetic that was copy-pasted across both branches. - modal_backend._default_sandbox_timeout(): read the shared helper. - Drop RLLM_DAYTONA_SANDBOX_AUTOSTOP_MIN (was unreleased, added in this branch). Adopt the new name in the clean tmax scripts (train_tinker.sh, train_verl.sh) and document it in both cookbook READMEs. The provider-specific create-rate knobs (*_CREATE_RPS/_BURST) stay separate — they tune different platform rate limits. Tests: provider-agnostic override floors both backends; legacy alias honored with canonical precedence; per-task sizing unchanged when no override is set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(sandbox): run Daytona sandboxes ephemeral by default RL rollout and warm-queue sandboxes are strictly single-use (create → run → verify → delete on close()), so default DaytonaSandbox to ephemeral=True with auto_delete_interval=0, matching how harbor's Daytona environment always runs. Two payoffs: - Higher quota: Daytona's ephemeral tier carries a larger per-sandbox limit, so tasks declaring more than the standard 10 GB disk (tmax declares 20 GB) now schedule instead of failing every create with "Disk request 20GB exceeds maximum allowed per sandbox (10GB)". This is why we honor the task's declared storage rather than clamping it — ephemeral lifts the ceiling. - Leak-proofing: an ephemeral sandbox is deleted (not left stopped) when it stops, so a SIGKILL'd run that never reaches close()/atexit no longer strands a stopped box forever — the failure mode behind the ~1.3 TB of leaked stopped sandboxes seen on the shared account. ephemeral is an overridable kwarg (=False falls back to auto_delete_interval=-1, never-delete). Tested via a fake daytona module so no SDK/API key is required. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
#694) * fix(harness): restore TerminationReason on the SandboxedAgentFlow path CLI-harness rollouts (terminus2 / mini-swe-agent) always came back ENV_DONE, so compact filtering couldn't distinguish timeouts/errors/length-truncation and the policy was punished for infrastructure failures (a timed-out or sandbox-reaped rollout shouldn't train as a reward-0 sample). - BaseCliHarness.run() now returns an outcome Episode carrying the reason it observed: TIMEOUT on SandboxCommandTimeout, ERROR (+ details) on exec failure, None on a clean exit; stamps metadata["max_turns"] when the harness caps turns. Stays stateless (returns via value) so the shared-instance parallel rollouts are unaffected. - AgentFlowEngine grows derive_termination_reason() and replaces the blunt ENV_DONE fallback in _finish_episode: last finish_reason=="length" -> MAX_RESPONSE_LENGTH_EXCEEDED, turns at cap -> MAX_TURNS_EXCEEDED, else ENV_DONE. Only runs when the harness left it None, so harness-set TIMEOUT/ERROR win. - Relocate TerminationReason/TerminationEvent from rllm.workflows.workflow to rllm.types (they're used across engine, harnesses, eval, trainer — no longer workflow-specific). workflow.py re-exports them so existing imports keep working; first-party rllm/ import sites updated to the canonical location. MAX_PROMPT_LENGTH_EXCEEDED detection is deferred (TODO in derive_termination_reason). Tests: updated tests/harnesses/test_cli_harness.py (Episode return + TIMEOUT/ERROR /max_turns classification) and added tests/engine/test_termination_derivation.py. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refine: don't infer MAX_RESPONSE_LENGTH/MAX_TURNS on the CLI-harness path Reading harbor's Terminus-2 showed neither is recoverable from gateway traces: - finish_reason=="length" is not episode-terminal here. On a truncated response the agent appends an error and recursively re-queries the LLM (terminus_2.py _query_llm), so it continues — a trailing "length" is coincidental, not the cause. Combined with base.yaml's mask_max_response_length_exceeded=True, the old heuristic could silently drop otherwise-fine (incl. successful) episodes. - The trace count is LLM *calls*, not turns (retries, length-recovery re-queries and summarization inflate it), so n_steps>=max_turns can't reliably detect the cap; the true turn count lives only inside the sandbox driver. derive_termination_reason now returns ENV_DONE for clean exits; the meaningful signal is the harness-set TIMEOUT/ERROR (the actual fix). Drop the now-unused max_turns metadata stamping. Tests pin "trailing length is not terminal" as a regression guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refine: inline ENV_DONE fallback, drop trivial derive_termination_reason It only ever returned ENV_DONE; inline it in _finish_episode and delete its test. Tighten the harness comments/docstrings. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…695) * feat(gateway): rllm tunnel CLI + ngrok backend + daemon auto-pickup Make stable-tunnel setup ergonomic and shareable. Free cloudflared quick tunnels get 429-throttled; ngrok with a reserved domain is stable but previously required a manual `ngrok http` in a side terminal + a hardcoded URL pinned per-script (only valid for one person's account). Adds an `rllm tunnel` CLI that runs a backend as a detached daemon and records its live URL to ~/.rllm/tunnel.json; training auto-discovers it with no per-run config and nothing personal in shared scripts. - gateway/tunnel.py: extract a shared _Tunnel base from CloudflaredTunnel; add NgrokTunnel (ngrok http --url --log json, actionable hints for authtoken/1-session/domain errors); create_tunnel() dispatch; spawn_detached() daemon + state-file helpers; resolve_auto_tunnel() ($RLLM_GATEWAY_TUNNEL > running daemon > cloudflared quick tunnel) - hooks.enable_gateway_tunnel: resolve via resolve_auto_tunnel and warn on quick-tunnel fallback (was a silent cloudflared default) - eval/config.py: load_/save_tunnel_config (per-user backend/domain/port) - cli/tunnel.py + main.py: new `rllm tunnel {setup,up,status,down}` group - cli/train.py: sandbox-routing header shows the resolved tunnel URL - base.yaml: document "ngrok"/"ngrok:<domain>" + daemon auto-pickup - tmax/train_fireworks.sh: drop hardcoded ngrok URL (now portable) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * update training scripts * cookbooks(terminal-rl): gateway port 9090 + drop hardcoded ngrok tunnel train_fireworks.sh now matches tmax: gateway on 9090 (the rllm tunnel daemon default) with no hardcoded rllm.gateway.tunnel, so it auto-discovers the tunnel via `rllm tunnel up`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style: ruff format (join implicit string concatenations) Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
`rllm view` returned a 400 "Bad run id" for any run whose directory name contains '@' (e.g. terminal-bench@2.0) or '#' (model#deployment). The SPA fetches /api/runs/<id>/index with the id percent-encoded, but the handler validated the still-encoded string against a charset regex excluding '%'/'@'/'#', and never URL-decoded it before the filesystem lookup. Decode the run id + filename with urllib.parse.unquote before validation and lookup (both the /index and /episodes/<file> routes), and permit '@'/'#' in the safe-id/safe-file patterns. Path-traversal is still blocked by the ".."/"/"/"\\" guard and the _under_root check, both of which now run on the decoded value. Co-authored-by: signalrush <269811712+signalrush@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ode bridge Adds rllm.renderers: one bridge-capable renderer interface (prime-rl's protocol) for every backend's models. get_renderer resolves prime-rl native -> TinkerRendererAdapter (wraps tinker_cookbook AND Fireworks-cookbook renderers, e.g. deepseek_v4) -> chat-template fallback. BridgingRendererMixin synthesizes bridge_to_next_turn for any deterministic renderer and is verified byte-identical to prime-rl's hand-tuned qwen3 bridge. - gateway: resolve the cumulative-mode renderer via rllm.renderers (+ renderer_name config). The chat-template fallback is best-effort (runtime prefix-check guards; drift resets, never corrupts) instead of a hard error. - FireworksEngine: opt-in unified renderer via renderer_name/renderer_family (the DeepSeek-V4 path); default keeps the chat-template path so existing Fireworks models do not regress. Tests: 48 pass (renderer + gateway), ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… ChatTemplateParser (#702) * fix(fireworks): use unified renderer for turn-0 and skip ChatTemplateParser when pinned FireworksEngine built ChatTemplateParser unconditionally, whose construction eagerly runs apply_chat_template (verify_equivalence). Some served templates reject the probe messages — e.g. GLM-5.2 raises "'str object' has no attribute 'items'" — crashing engine init before training. The GLM script already pins RENDERER_FAMILY=glm-5, but it was only wired to the gateway; FireworksEngine never received it, so it fell back to chat_parser. Now: - fireworks_backend passes rllm.gateway.renderer_family to FireworksEngine, so the engine renders turn 0 with the same renderer the gateway uses for the cumulative bridge (turns 1+) — keeping them consistent. - FireworksEngine resolves the renderer first; when it pins a native (prime-rl or tinker) renderer it uses it and skips ChatTemplateParser entirely. Default (renderer_family=auto, no renderer_name) is unchanged — chat_parser as before. Test: pinned family uses the renderer and leaves chat_parser None; default still builds chat_parser. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(renderers): auto-detect Fireworks-cookbook models (GLM-5.2) without config The Fireworks cookbook ships renderer *implementations* (glm5, deepseek_v4, ...) but no model->renderer map, and tinker_cookbook's recommender doesn't know them, so nothing auto-routed GLM-5.2 to the cookbook renderer — you had to pin renderer_family=glm-5. - registry: resolve() now owns a Fireworks-cookbook prefix map (GLM-5.x -> glm5, DeepSeek-V4 -> deepseek_v4), consulted after prime-rl's exact-match map; imports training.renderer so cookbook renderers self-register; degrades gracefully if the cookbook isn't installed (never hard-fails). - FireworksEngine: resolve the renderer the same way the gateway does (always), so engine (turn 0) and gateway (turns 1+) pick the same renderer. Adopt it when pinned or when auto-detection lands on a cookbook renderer (e.g. GLM-5.2), and skip ChatTemplateParser; prime-rl models under plain auto keep chat_parser (no regression for working qwen runs). GLM-5.2 now trains with no renderer config: auto -> cookbook glm5 on both engine and gateway. Tests: cookbook prefix mapping + graceful fallback. 14 pass, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…(both backends) (#705) * fix(fireworks): parse completions via unified renderer in assemble_model_output Skipping ChatTemplateParser when a unified renderer is active left the inherited assemble_model_output asserting chat_parser is set (bypass_render_with_parser=True), crashing on the first response: AssertionError: chat_parser must be set when bypass_render_with_parser=True Override assemble_model_output in FireworksEngine: when a unified renderer is active, parse the completion via renderer.parse_response (ParsedResponse -> content/reasoning/ tool_calls); otherwise delegate to the inherited chat_parser path. Covers both turn 0 (get_model_response) and turns 1+ (the gateway token-prompt path), which both call it. Test: assemble_model_output parses via the renderer with chat_parser=None. 15 pass, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(fireworks): make bypass_render_with_parser reflect renderer ownership bypass_render_with_parser was hardcoded True even when a unified renderer was active — a lie (True means "render/parse with ChatTemplateParser"), and the root reason assemble_model_output asserted on a None chat_parser. Set it to `self.renderer is None`: False when the renderer owns rendering+parsing, True when ChatTemplateParser does. Flipping the flag alone isn't sufficient — the parent's bypass=False branch is hardcoded to a tinker_cookbook renderer ((Message, term) parse_response), which the unified renderer (ParsedResponse) isn't — so the assemble_model_output override stays. This just stops the attribute from lying (and misleading completer.py etc.). Test: bypass_render_with_parser is False with a renderer, True without. 15 pass, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(engine): unify renderer handling in shared TinkerEngine for both backends Per review: the unified rllm.renderers handling belonged in the shared TinkerEngine, not a FireworksEngine-only override. Now both backends render+parse through one path. - TinkerEngine: add self.unified_renderer (default None) + a shared _render_prompt_token_input helper. assemble_model_output and the render path branch on the unified renderer first, then ChatTemplateParser, then the tinker_cookbook renderer (incl. VLM image chunks). Base behavior is unchanged (unified_renderer stays None for the Tinker backend), so chat_parser / tinker-renderer / VLM paths are preserved. - FireworksEngine: set self.unified_renderer via rllm.renderers.resolve and reuse the shared render+parse; drop the assemble_model_output override and the inline render. self.renderer stays None (the shared legacy/VLM branch is unreachable here). No external code reads rollout_engine.renderer, so introducing unified_renderer is safe. 15 renderer tests + 55 engine tests pass (the 1 fail / 4 errors are pre-existing env issues: verl not installed, torch.LongTensor). ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(fireworks): honor explicit bypass_render_with_parser=True (escape hatch) FireworksEngine ignored the config's bypass_render_with_parser (it was swallowed by **kwargs) and always resolved a unified renderer. Make it a real parameter: when True, skip the unified renderer and use ChatTemplateParser — an explicit escape hatch, matching TinkerEngine (which already honors bypass=True via its unbuilt unified path). Default (False) keeps auto-resolution. Test: bypass_render_with_parser=True with renderer_family pinned -> unified_renderer None, chat_parser built, bypass True. 16 tests pass, ruff clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…teway + backends
The gateway (turn-1+ cumulative bridge) and the trainer rollout engine (turn-0
render + completion parse) resolved their renderer independently from different
config keys — renderer_family from rllm.gateway, renderer_name only from
rollout_engine and never plumbed to the gateway. Any model pinned by name
diverged across turns. This makes one block the single source of truth.
- New `rllm.renderer.{family,name}` block; `renderer_settings(cfg)` reads it,
falling back (deprecated, warns) to rllm.gateway.renderer_family and
rollout_engine.renderer_name so existing configs keep working.
- Gateway: manager + GatewayConfig + `--renderer-name` now plumb renderer_name;
both family and name come from rllm.renderer.
- Backends: Fireworks and Tinker both resolve (family,name) via renderer_settings
and pass them to their engines.
- TinkerEngine converges onto the unified renderer for text models (was: legacy
tinker_cookbook renderer only). VLM keeps the legacy chunk path; explicit
bypass_render_with_parser=True forces ChatTemplateParser; unknown models fall
back to ChatTemplateParser.
- TinkerRendererAdapter now faithfully converts OpenAI<->tinker messages/tools
(tool_calls, tool results, images, tool specs) and parses tinker tool-calls, so
it's a true drop-in for the legacy path (and fixes the cookbook deepseek_v4/glm5
tool path on both backends).
- Migrate tracked cookbook scripts to rllm.renderer.family.
Tests: 19 renderer (incl. TinkerEngine adopt-unified / bypass / VLM-legacy) + 37
gateway pass; ruff clean. NOTE: live Tinker/Fireworks rollouts, the gateway
subprocess CLI, and VLM are not exercisable offline — needs a real smoke test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
64d8cd4 to
b52b798
Compare
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
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.
Problem
The gateway (turn-1+ cumulative bridge) and the trainer rollout engine (turn-0 render + completion parse) resolved their renderer independently from different config keys:
renderer_family←rllm.gateway.renderer_familyrenderer_name←rollout_engine.renderer_name(engine only; never plumbed to the gateway)So any model pinned by name (cookbook
glm5,deepseek_v4, …) diverged: engine used it, gateway fell back to family/auto → the turn-1+ bridge didn't match the engine's turn-0 tokens. Not GLM-specific — general.Fix: one source of truth
rllm.renderer.{family,name}block.renderers.renderer_settings(cfg)reads it, with deprecated fallbacks (warns) torllm.gateway.renderer_familyandrollout_engine.renderer_nameso existing/untracked scripts keep working.manager.py+GatewayConfig+ a new--renderer-namenow carry both family and name fromrllm.renderer.fireworks_backend,tinker_backend) resolve(family, name)viarenderer_settingsand pass them to their engines — so gateway and engine resolve the same renderer.tinker_cookbookrenderer only). VLM keeps the legacybuild_generation_prompt(...).chunkspath (image chunks); explicitbypass_render_with_parser=TrueforcesChatTemplateParser; truly-unknown models fall back toChatTemplateParser.TinkerRendererAdaptermade a faithful drop-in: converts OpenAI↔tinker messages/tools (tool_calls, tool results, images, tool specs) and parses tinker tool-calls. This also fixes the cookbookdeepseek_v4/glm5tool path (the old adapter passed OpenAI tools whereToolSpecwas expected and didn't parse tinker tool-calls).rllm.renderer.family.Result
rllm.renderer.{family,name}is the one place; gateway + Fireworks + Tinker all resolve identically. Pin any model by family or name and both sides agree. Old keys still work (deprecated).Testing
renderer_settingsnew-block + fallbacks; FW bypass escape hatch; bridge parity) + 37 gateway tests pass. ruff clean.base.yamlcomposes.The live Tinker rollout path (tool-using + VLM), the Fireworks deployment, and the gateway subprocess
--renderer-namecan't be validated here. The adapter's OpenAI↔tinker conversion is ported from TinkerEngine's proven legacy helpers and unit-tested for resolution/wiring, but a real Tinker run with tools (and one VLM run) should be smoke-tested before relying on it.🤖 Generated with Claude Code