Skip to content

TITO support for SkyRL - #2143

Open
xinze-zheng wants to merge 10 commits into
NovaSky-AI:mainfrom
xinze-zheng:tito
Open

TITO support for SkyRL#2143
xinze-zheng wants to merge 10 commits into
NovaSky-AI:mainfrom
xinze-zheng:tito

Conversation

@xinze-zheng

@xinze-zheng xinze-zheng commented Sep 3, 2026

Copy link
Copy Markdown

This PR implements TITO proxy for SkyRL. A custom generator simply needs to instantiate the proxy and point the url endpoint to the proxy, who are OpenAI-API compatible.

The implementation detail is discussed in #1959.

How it works

  • A generator-owned proxy accepts Harbor Chat Completions requests and forwards tokenized inference through SkyRL’s inference client.
  • A message tree records every successful model call and reuses exact token prefixes.
  • When a new user input requests, Renderer is used to apply chat template and tokenizes the new input with fidelity check.
  • Rewritten histories, including Terminus summarization, naturally create branches in the message tree.
  • A composer converts the each branch in the message tree to a training input. The composer is customizable by the user.

Validation

Two end-to-end run in code-contest with and without summarization enabled.
Purple curve = run without summarization. Red curve = run with summarization. Blue Curve = code-contest.
image


Note

Medium Risk
Touches the rollout→training path (exact tokens, loss masks, rewards) and introduces an in-process HTTP proxy on the critical inference path; incorrect trace/composer logic could silently skew RL training, though baseline Harbor is unchanged.

Overview
Adds token-in/token-out (TITO) support so external agents that speak OpenAI Chat Completions can roll out through SkyRL’s inference client while training on the exact sampled token IDs and logprobs.

A generator-owned FastAPI proxy (TITOProxy) validates requests with vLLM’s OpenAI stack, renders prompts via Prime renderers, runs inference, and commits each turn into a message graph (Trace) that supports prefix reuse, retries, and branching (e.g. summarization). build_trace_generator_output turns trace leaves into GeneratorOutput rows, with masking for errors/timeouts, optional overlong filtering, and step-wise vs single-branch behavior.

Ships a parallel harbor_tito example: TITOHarborGenerator points Harbor trials at per-session proxy URLs, enables interleaved thinking and rollout-detail parity checks, and merges tito.yaml trial overrides. Entrypoints mirror sync, fully-async, and generate-only Harbor flows and require explicit max_seq_len.

Also adds optional harbor-tito extra (harbor + pinned renderers + vllm), forwards NCCL_NET into Ray runtime env, and broad CPU/integration tests for trace, proxy, composer, and Harbor wiring.

Reviewed by Cursor Bugbot for commit d5b0db4. Bugbot is set up for automated code reviews on this repo. Configure here.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces the Harbor TITO integration, routing Harbor's Chat Completions calls through a generator-owned, OpenAI-compatible token-in/token-out (TITO) proxy. Key additions include the TITOProxy server, exact message-graph trace bookkeeping, a thread-safe renderer adapter, and corresponding unit tests. Feedback on these changes identifies a potential resource leak in the proxy startup and shutdown logic, a potential AttributeError when configuring the generator with setdefault on potentially None values, and a potential unhandled 500 error if the Content-Length header is malformed.

Comment thread skyrl/train/generators/tito/proxy.py Outdated
Comment thread examples/train_integrations/harbor_tito/harbor_generator.py
Comment thread skyrl/train/generators/tito/proxy.py Outdated
Comment thread skyrl/train/generators/tito/vllm_openai.py
@xinze-zheng xinze-zheng changed the title Tito TITO support for SkyRL Sep 3, 2026

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

Reviewed by Cursor Bugbot for commit d5b0db4. Configure here.

has_routed_experts = True
rollout_expert_indices.append(row_routed)
else:
rollout_expert_indices.append([])

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Routed experts crash training conversion

Medium Severity

build_trace_generator_output emits rollout_expert_indices as nested Python lists, including empty [] placeholders for masked or missing rows. The trainer’s convert_prompts_responses_to_batch_tensors requires a NumPy array per trajectory with shape [tokens, layers, topk], so any MoE run that returns routes will fail during batch conversion.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d5b0db4. Configure here.

)
rollout_metrics["generate/num_error_trajectories"] = sum(
1 for outcome, rows in zip(outcomes, branch_rows) if outcome.stop_reason == "error" or not rows
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Timeouts counted as error metrics

Low Severity

generate/num_error_trajectories increments when not rows, so agent_timeout outcomes that use an empty sealed Trace are counted as both timeouts and errors. The existing Harbor composer treats these as disjoint cases.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d5b0db4. Configure here.

@SumanthRH SumanthRH self-assigned this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants