Skip to content

[codex] Allow SFT without a teacher - #2720

Merged
mikasenghaas merged 1 commit into
mainfrom
feat/replay-sft
Jun 9, 2026
Merged

mikasenghaas merged 1 commit into
mainfrom
feat/replay-sft

Conversation

@tim0120

@tim0120 tim0120 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • allow orchestrator.training_mode = "sft" without a configured teacher
  • route teacher-backed SFT train rollouts through the teacher pool, and teacherless SFT through the existing student rollout pool
  • keep renderer forcing scoped to teacher-backed SFT; teacherless SFT keeps the normal student renderer path
  • keep prefix-cache salting for student-backed train rollouts while preserving unsalted cache reuse for frozen teacher-backed SFT

Motivation

This is needed for replay-backed SFT workflows where the train env can provide the supervision without a teacher model. PrimeRL should only need the generic capability: SFT can run without orchestrator.teacher; replay-specific taskset/env behavior stays outside this PR.

Validation

  • uvx ruff==0.13.0 check --config=pyproject.toml packages/prime-rl-configs/src/prime_rl/configs/orchestrator.py src/prime_rl/orchestrator/orchestrator.py src/prime_rl/orchestrator/dispatcher.py
  • python3 -m py_compile packages/prime-rl-configs/src/prime_rl/configs/orchestrator.py src/prime_rl/orchestrator/orchestrator.py src/prime_rl/orchestrator/dispatcher.py
  • git diff --check

Notes

The earlier replay-specific package wiring, debug config, docs, tests, Verifiers submodule pointer, token-usage backfill, and zero-adv filter changes have been removed from the effective diff. The PR is now limited to the teacher-optional SFT routing/cache consequences.


Note

Medium Risk
Changes SFT train inference routing and prefix-cache salting when no teacher is set; misconfiguration could affect rollout correctness or cache reuse, but scope is limited to orchestrator config and dispatch.

Overview
SFT can run without orchestrator.teacher. Config validation no longer requires a teacher for training_mode = "sft" (only OPD still mandates one). Forcing renderer = None is limited to teacher-backed SFT; teacherless SFT keeps the normal student renderer path.

Train rollout routing is split: SFT with a teacher still uses the frozen teacher inference pool and disables prefix-cache salting on train rollouts; SFT without a teacher uses the student rollout pool with policy-version cache salt, same as RL/OPD train paths. The dispatcher takes an explicit use_cache_salt flag instead of inferring this from training_mode == "sft" alone.

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

@tim0120
tim0120 force-pushed the feat/replay-sft branch from 8522efd to 750ea0a Compare June 4, 2026 23:27
@tim0120 tim0120 changed the title [codex] Add replay-backed SFT path [codex] Add hosted OPD and replay SFT paths Jun 4, 2026
@tim0120
tim0120 changed the base branch from feat/hosted-opd-teacher-logprobs to main June 4, 2026 23:28
@tim0120
tim0120 force-pushed the feat/replay-sft branch from 750ea0a to 8208ee0 Compare June 4, 2026 23:42
@tim0120 tim0120 changed the title [codex] Add hosted OPD and replay SFT paths [codex] Add replay-backed SFT path Jun 4, 2026
@tim0120
tim0120 force-pushed the feat/replay-sft branch from 8208ee0 to 9e5ef35 Compare June 4, 2026 23:43
Comment thread tests/unit/test_configs.py Outdated
Comment thread tests/unit/test_configs.py Outdated
Comment thread tests/unit/orchestrator/test_sft_replay_env.py Outdated
@tim0120
tim0120 marked this pull request as ready for review June 5, 2026 19:20
Copilot AI review requested due to automatic review settings June 5, 2026 19:20
@tim0120

tim0120 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

Copilot AI 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.

Pull request overview

This PR adds a replay-backed SFT path to the RL orchestrator so SFT can run without a teacher inference server when training environments replay stored message traces (via sft-replay). It also tightens config guardrails for teacherless SFT and ensures replayed message-only trajectories get token usage populated so batching/metrics work correctly.

Changes:

  • Add teacherless SFT validation: only allowed when all train envs are sft-replay and each provides args.taskset.dataset (or args.config.taskset.dataset).
  • Backfill token_usage from tokenized trajectories when replay/teacher paths omit final_input_tokens / final_output_tokens.
  • Wire dependencies/docs/debug config to support and document replay-backed SFT (sft_replay.toml, skills/docs updates, editable deps for verifiers packages/env).

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
uv.lock Adds editable packages/env entries (harnesses, tasksets, sft-replay) and registers sft-replay in env extras.
pyproject.toml Registers sft-replay env and editable verifiers packages; adds override deps for harnesses/tasksets.
packages/prime-rl-configs/src/prime_rl/configs/orchestrator.py Adds teacherless SFT guardrails and drops the default zero_advantage post-batch filter for SFT unless explicitly configured.
src/prime_rl/orchestrator/orchestrator.py Allows SFT train rollouts to come from the teacher when configured, otherwise uses student inference (for replay envs that ignore inference).
src/prime_rl/orchestrator/dispatcher.py Updates comments/docstring to reflect teacher-backed vs replay-backed SFT behavior.
src/prime_rl/orchestrator/trajectories.py Adds _set_token_usage_from_trajectory and triggers it when token usage is missing on tokenized trajectories.
tests/unit/test_configs.py Adds config validation tests for teacherless replay SFT and post-filter behavior.
tests/unit/orchestrator/test_sft_replay_env.py Adds an integration-style unit test covering replay rollout → token backfill → interleave.
configs/debug/training_modes/sft_replay.toml Adds a debug config demonstrating replay-backed SFT through the orchestrator.
configs/debug/training_modes/README.md Documents how to run the new debug replay-backed SFT config.
docs/training.md Documents sft mode as teacher-optional when using sft-replay trace replay.
skills/configs/SKILL.md Adds guidance on the expected config shape for replay-backed SFT datasets.
skills/training/start-run/SKILL.md Updates training-mode guidance to include replay-backed teacherless SFT constraints.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@tim0120
tim0120 force-pushed the feat/replay-sft branch from d4302ad to 35dc8e2 Compare June 5, 2026 19:31
@tim0120
tim0120 marked this pull request as draft June 5, 2026 19:32
@tim0120
tim0120 marked this pull request as ready for review June 5, 2026 20:24
Comment thread packages/prime-rl-configs/src/prime_rl/configs/orchestrator.py Outdated
Comment thread src/prime_rl/orchestrator/train_sink.py Outdated
Comment thread src/prime_rl/orchestrator/trajectories.py Outdated
Comment thread src/prime_rl/orchestrator/trajectories.py Outdated
Comment thread src/prime_rl/orchestrator/dispatcher.py Outdated
Comment thread packages/prime-rl-configs/src/prime_rl/configs/orchestrator.py

@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 and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 16e74f0. Configure here.

Comment thread src/prime_rl/orchestrator/orchestrator.py
@tim0120 tim0120 changed the title [codex] Add replay-backed SFT path [codex] Allow SFT without a teacher Jun 6, 2026
@tim0120
tim0120 requested a review from willccbb June 6, 2026 19:22
Comment thread src/prime_rl/orchestrator/dispatcher.py Outdated
@tim0120
tim0120 force-pushed the feat/replay-sft branch from 033aadd to b04d119 Compare June 7, 2026 20:57
@tim0120
tim0120 force-pushed the feat/replay-sft branch from b04d119 to cdba94e Compare June 7, 2026 20:58
Comment thread src/prime_rl/orchestrator/orchestrator.py
@mikasenghaas
mikasenghaas merged commit 0695f9c into main Jun 9, 2026
18 checks passed
mikasenghaas added a commit that referenced this pull request Jun 14, 2026
Bring the v1 nano branch up to date with main. Submodule pins for verifiers
(7a98b566) and renderers (057f0087) kept at the branch's v1 commits;
research-environments follows main (4c08260, branch never customized it).

Conflict resolutions (favoring the v1 design):
- utils/client.py: keep both imports (main's AsyncRetrying admin-retry + v1 client configs)
- orchestrator.py: keep ROLLOUT_DUMP_EXCLUDE; keep v1 eval logging but graft main's
  per-eval policy_version metric; drop main's orchestrator-side token-export drain (#2641)
- metrics.py / envs.py / configs/orchestrator.py: take v1 versions; drop main's
  orchestrator-side token-export (#2641), per-env advantage (#2721, branch uses top-level
  advantage via train_sink), and sft-no-teacher renderer=None (#2720, branch is renderer-only)
- pyproject.toml: dedupe tasksets/harnesses sources; drop mini-swe-agent-plus-rlm
- uv.lock regenerated; orphan orchestrator/token_export_metrics.py removed

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

5 participants