Skip to content

Add Mobile300 Prime-RL token-suffix staging#864

Merged
bingran-you merged 1 commit into
mainfrom
bry/prime-rl-mobile300-token-suffix
Jun 30, 2026
Merged

Add Mobile300 Prime-RL token-suffix staging#864
bingran-you merged 1 commit into
mainfrom
bry/prime-rl-mobile300-token-suffix

Conversation

@bingran-you

Copy link
Copy Markdown
Collaborator

Summary

Fixes the Mobile300 PR828 Prime-RL SFT compatibility path in BenchFlow by staging the dataset the way the historical custom trainer actually trained it:

  • render each row with the historical custom-trainer chat-template path
  • keep the exact rendered token suffix instead of message-level tail truncation
  • omit tool_defs/tools for the compatibility profile, matching the archived PR828 rows
  • use a generated passthrough Prime-RL chat template so Prime-RL trains on the staged suffix without rerendering tool schemas
  • use assistant-only loss plus an unlabeled EOS pad message so stack packing gives one 8192-token row per sample

This keeps the fix in BenchFlow’s Prime-RL wrapper and avoids patching Prime-RL internals.

Why

The Mobile300 regression was not caused by the BenchFlow result JSONL schema alone. The key mismatch was that Prime-RL was tokenizing/truncating/loss-masking rows differently from the old custom trainer, causing a behavior collapse with many zero-tool eval attempts. The env0-mobile300-pr828 compatibility profile now encodes those historical semantics explicitly.

Validation

  • uv run pytest tests/test_train_cli.py -q
  • uv run ruff check src/benchflow/training/backends/prime_rl.py src/benchflow/cli/train.py tests/test_train_cli.py
  • uv run ruff format --check src/benchflow/training/backends/prime_rl.py src/benchflow/cli/train.py tests/test_train_cli.py
  • uv run ty check src/benchflow/training/backends/prime_rl.py

Spendful validation is intentionally not in this PR; the next step is a real Mobile300 dry-run/train launch on the H100 from this branch.

@bingran-you
bingran-you temporarily deployed to pypi-internal-preview June 30, 2026 13:36 — with GitHub Actions Inactive
@bingran-you
bingran-you merged commit 76cb44c into main Jun 30, 2026
9 checks passed
@bingran-you
bingran-you deleted the bry/prime-rl-mobile300-token-suffix branch June 30, 2026 13:44

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 08934d5fc5

ℹ️ 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".

Comment on lines +862 to +867
rendered = tokenizer.apply_chat_template(
messages,
tools=tools,
tokenize=False,
add_generation_prompt=False,
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve per-row chat template kwargs when rendering

When a local JSONL row already carries chat_template_kwargs, this new suffix renderer calls apply_chat_template without forwarding them and _custom_trainer_token_suffix_row later drops the field from the staged row. In that scenario—for example Qwen rows that set enable_thinking=False—the staged token suffix is rendered with tokenizer defaults, so custom-trainer-token-suffix trains on a different token tail than the row/custom-trainer path requested.

Useful? React with 👍 / 👎.

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.

1 participant