diff --git a/deps/renderers b/deps/renderers index 5904fa24aa..fa04acd916 160000 --- a/deps/renderers +++ b/deps/renderers @@ -1 +1 @@ -Subproject commit 5904fa24aa73f83c1694fd85a78d0e746d468284 +Subproject commit fa04acd916ce749c302eec905ecf3962bc438429 diff --git a/docs/training.md b/docs/training.md index 0372fd68c1..d8d4e7e9b8 100644 --- a/docs/training.md +++ b/docs/training.md @@ -144,12 +144,20 @@ Two accepted layouts: If both columns are present, `messages` takes precedence. -**Tool definitions.** For tool-use SFT, add a `tools` column (OpenAI function-calling format) or `tool_defs` ([`verifiers`](https://github.com/PrimeIntellect-ai/verifiers) rollout format). Each row's value can be either a list of dicts or a JSON-encoded string of a list — both are accepted, and `tool_defs` rows are auto-converted to OAI shape before being passed into the chat template's `tools=...` argument. The `chat_template_kwargs` column, if present, is forwarded verbatim into `apply_chat_template`. +**Tool definitions and renderer controls.** For tool-use SFT, add a `tools` column (OpenAI function-calling format) or `tool_defs` ([`verifiers`](https://github.com/PrimeIntellect-ai/verifiers) rollout format). Each row's value can be either a list of dicts or a JSON-encoded string of a list — both are accepted, and `tool_defs` rows are auto-converted to OAI shape before being passed into the renderer. -**Position-dependent chat templates.** Multi-turn SFT under the default tokenization path (`build_incremental_token_mask`) requires that tokenizing the first _k_ turns of a conversation be a strict prefix of tokenizing all _n ≥ k_ turns. Qwen3's upstream template _violates_ this — it strips past `` blocks across user turns, silently corrupting the loss mask. Two fixes: +Renderer-backed SFT reads template controls from the typed `[renderer]` config in the SFT TOML. For example: + +```toml +[renderer] +name = "qwen3" +enable_thinking = false +``` + +If a model needs another template control, add it to that model's renderer config in `renderers` (for example a new field on the relevant `*RendererConfig`) and consume it in the renderer implementation. + +**Renderer-backed tokenization.** SFT tokenization is renderer-only. The [`renderers`](algorithms.md#renderers) package owns message-to-token conversion and loss attribution end-to-end, so position-dependent chat templates (for example templates that strip past `—` blocks across user turns) do not corrupt the loss mask. `[renderer]` defaults to `name = "auto"`; set a typed renderer config only when you need model-specific template controls. Hand-coded renderers ship for Qwen3, Qwen3.5, GLM-5, GLM-4.5, Kimi K2/K2.5, MiniMax M2, DeepSeek V3, Nemotron 3, GPT-OSS. -- **Enable the renderer** (set a typed `[renderer]` config, e.g. `name = "qwen3"`, recommended; defaults to `"auto"` for RL). The [`renderers`](algorithms.md#renderers) package owns tokenization end-to-end and is robust to position-dependent templates. Hand-coded renderers ship for Qwen3, Qwen3.5, GLM-5, GLM-4.5, Kimi K2/K2.5, MiniMax M2, DeepSeek V3, Nemotron 3, GPT-OSS. Not supported for VLMs. -- **Patched chat template** — the prime-rl–patched checkpoints (e.g. `PrimeIntellect/Qwen3-0.6B`, used in `examples/reverse_text/sft.toml`) ship a chat template that preserves thinking. Or supply your own. See [Algorithms § Multi-Turn Trajectories](algorithms.md#multi-turn-trajectories) for the full picture. diff --git a/examples/wordle/rl.toml b/examples/wordle/rl.toml index 990fb185fb..db0c7ed584 100644 --- a/examples/wordle/rl.toml +++ b/examples/wordle/rl.toml @@ -30,9 +30,6 @@ max_completion_tokens = 1024 [inference.parallel] dp = 6 -# Qwen3 finetune with the standard PI template patch (byte-identical to -# PrimeIntellect/Qwen3-0.6B base); always re-emits prior blocks. -# Match that with renderer thinking_retention. +# Fine-tune inherits the PrimeIntellect Qwen3 template byte-for-byte. [orchestrator.renderer] -name = "qwen3" -thinking_retention = "all" +name = "prime-qwen3" diff --git a/packages/prime-rl-configs/src/prime_rl/configs/sft.py b/packages/prime-rl-configs/src/prime_rl/configs/sft.py index 6352bbd875..1760ce8aca 100644 --- a/packages/prime-rl-configs/src/prime_rl/configs/sft.py +++ b/packages/prime-rl-configs/src/prime_rl/configs/sft.py @@ -3,7 +3,8 @@ from typing import Annotated, Literal, TypeAlias from pydantic import Field, model_validator -from renderers import RendererConfig +from renderers import AutoRendererConfig, DefaultRendererConfig, RendererConfig +from renderers.base import MODEL_RENDERER_MAP from prime_rl.configs.shared import ( EnvVars, @@ -175,13 +176,8 @@ class SFTConfig(BaseConfig): tokenizer: TokenizerConfig = TokenizerConfig() - renderer: RendererConfig | None = None - """Typed renderer config (``renderers.RendererConfig`` discriminated - union). When set, SFT tokenizes samples through the ``renderers`` - library (single ``render()`` + ``message_indices`` mask) instead of - the default ``build_incremental_token_mask`` path. Required for chat - templates that render position-dependently (e.g. Qwen3, Qwen3.5). - ``None`` (default) uses the legacy tokenization path.""" + renderer: RendererConfig = AutoRendererConfig() + """Renderer config. Defaults to auto-selecting from the tokenizer model name.""" data: DataConfig = SFTDataConfig() @@ -271,6 +267,28 @@ def validate_deployment(self): raise ValueError("Must use SLURM for multi-node deployment.") return self + @model_validator(mode="after") + def validate_typed_renderer(self): + """Require a typed renderer whenever SFT renders real samples.""" + if self.data.type == "fake" and self.val is None: + return self + + model_id = self.tokenizer.name or self.model.name + if isinstance(self.renderer, AutoRendererConfig): + if model_id in MODEL_RENDERER_MAP: + return self + reason = f"no typed renderer is registered for {model_id!r}" + elif isinstance(self.renderer, DefaultRendererConfig): + reason = "renderer.name='default' selects DefaultRenderer" + else: + return self + + raise ValueError( + f"SFT requires a typed renderer with sampled-token and content attribution, but {reason}. " + "Implement and register the renderer in the renderers package, or explicitly select an existing " + "typed renderer only when its template is verified to match." + ) + @model_validator(mode="after") def validate_pack_function(self): if self.model.cp > 1: @@ -319,9 +337,9 @@ def dont_do_massive_traces(self): @model_validator(mode="after") def validate_renderer_vs_vlm(self): - if self.renderer is not None and self.model.vlm is not None: + if self.model.vlm is not None: raise ValueError( - "renderer is not supported for VLMs in SFT. The renderer tokenizes " + "renderer-only SFT does not support VLMs yet. The renderer tokenizes " "text-only message dicts client-side and cannot handle image inputs." ) return self diff --git a/src/prime_rl/trainer/sft/data.py b/src/prime_rl/trainer/sft/data.py index d0edd13787..8d0422e246 100644 --- a/src/prime_rl/trainer/sft/data.py +++ b/src/prime_rl/trainer/sft/data.py @@ -1,7 +1,7 @@ import json import uuid from collections import defaultdict -from typing import Literal, TypedDict, cast +from typing import Any, Literal, TypedDict, cast import torch from datasets import Dataset, interleave_datasets, load_dataset @@ -15,13 +15,7 @@ from prime_rl.configs.sft import DataConfig, LossMaskConfig, SFTDataConfig from prime_rl.trainer.world import get_world -from prime_rl.utils.chat_template import ( - IncrementalTokenizationError, - build_incremental_token_mask, - deserialize_tool_calls, - normalize_messages, - strip_message_content, -) +from prime_rl.utils.chat_template import deserialize_tool_calls, normalize_messages from prime_rl.utils.logger import get_logger STACKING_DATASET_BUCKET_TIMEOUT = 10 @@ -114,13 +108,33 @@ def __iter__(self): yield fake_sample +def _drop_null_fields(value: Any, path: tuple[str, ...] = ()) -> Any: + """Recursively strip ``None``-valued keys from dict structures. + + PyArrow's JSON loader unifies schemas across rows, so heterogeneous + OAI content blocks (text vs image_url) end up with all union keys + filled with ``None`` where absent. That confuses permissive + content-type predicates inside renderers (e.g. ``"image_url" in item`` + returns ``True`` even when the value is null). Strip the noise before + handing messages off to the renderer. Tool-call arguments are opaque + JSON payloads, so preserve their null values. + """ + if path[-3:] == ("tool_calls", "function", "arguments"): + return value + if isinstance(value, dict): + return {k: _drop_null_fields(v, (*path, k)) for k, v in value.items() if v is not None} + if isinstance(value, list): + return [_drop_null_fields(v, path) for v in value] + return value + + class SFTDataset(StatefulIterableDataset): """A dataset wrapping a HF SFT dataset with prompt/completion or raw messages format.""" def __init__( self, dataset: Dataset, - tokenizer: PreTrainedTokenizer | None, + renderer: Renderer, shuffle: bool = True, seed: int = 0, seq_len: int = 128, @@ -128,24 +142,18 @@ def __init__( loss_mask_config: LossMaskConfig = LossMaskConfig(), max_examples: int | None = None, max_epochs: int | None = None, - renderer: Renderer | None = None, ): super().__init__() self.logger = get_logger() self.dataset = dataset self.num_examples = len(self.dataset) - self.tokenizer = tokenizer + self.renderer = renderer self.shuffle = shuffle self.seed = seed self.seq_len = seq_len self.loss_mask_config = loss_mask_config self.max_examples = max_examples self.max_epochs = max_epochs - self.renderer = renderer - self._warned_chat_template_kwargs = False - - if self.tokenizer is None: - self.logger.warning("No tokenizer provided, will not process examples") # If specified, select a subset of the dataset if self.max_examples is not None: @@ -163,16 +171,14 @@ def __init__( self.data_world_size = get_world().world_size // non_dp_size * num_workers def _process(self, example: dict) -> dict | None: - # Skip processing if no tokenizer was provided - if self.tokenizer is None: - return example - def resolve_messages(example: dict) -> list[dict]: # `messages` takes precedence over explicit split fields and is interpreted - # as a whole-chat training sample with an empty prompt. - if "messages" in example: + # as a whole-chat training sample with an empty prompt. Null-check rather + # than key-check: Arrow schema union adds `messages: null` to + # prompt/completion rows whenever other rows have a `messages` column. + if example.get("messages") is not None: messages = normalize_messages(example["messages"], default_role="assistant") - elif "prompt" in example and "completion" in example: + elif example.get("prompt") is not None and example.get("completion") is not None: messages = normalize_messages(example["prompt"], default_role="user") + normalize_messages( example["completion"], default_role="assistant" ) @@ -182,22 +188,17 @@ def resolve_messages(example: dict) -> list[dict]: "or both 'prompt' and 'completion' columns for SFT" ) - # Deserialize tool call arguments from message list, if present - assumes OAI format - # Reference: https://platform.openai.com/docs/guides/function-calling#handling-function-calls - messages = deserialize_tool_calls(messages) - - # Strip content from all messages so that incremental tokenization works - # NOTE: This has the side effect that we do never train on leading or trailing whitespace - return strip_message_content(messages) + # Strip nulls before deserializing so genuine nulls inside tool-call + # argument strings survive. + messages = [_drop_null_fields(m) for m in messages] + return deserialize_tool_calls(messages) messages = resolve_messages(example) - # Parse available tools, if present - assumes OAI format - # Reference: https://platform.openai.com/docs/guides/function-calling#function-tool-example - # Accepts either `tools` or `tool_defs` (the verifiers rollout format), - # as either a JSON-encoded string of a list or a list of dicts. Tools - # arriving in the verifiers shape are converted to OAI form so any - # downstream chat template can consume them. + # Parse available tools, if present - assumes OAI format. Accepts either + # `tools` or `tool_defs` (the verifiers rollout format), as either a + # JSON-encoded string of a list or a list of dicts; verifiers-shaped + # tools are converted to OAI form for the chat template. raw_tools = example.get("tools", example.get("tool_defs")) if not raw_tools: tools = [] @@ -223,55 +224,37 @@ def should_mask(message: dict) -> bool: assert "role" in message, "Message must have a role" match message["role"]: case "user": - return True if self.loss_mask_config.user else False + return self.loss_mask_config.user case "assistant": - return True if self.loss_mask_config.assistant else False + return self.loss_mask_config.assistant case "system": - return True if self.loss_mask_config.system else False + return self.loss_mask_config.system case "tool": - return True if self.loss_mask_config.tool else False + return self.loss_mask_config.tool case _: raise ValueError(f"Invalid message role: {message['role']}") - if self.renderer is not None: - if example.get("chat_template_kwargs") and not self._warned_chat_template_kwargs: - self.logger.warning( - "Example carries chat_template_kwargs but a renderer is configured; " - "renderers don't forward chat_template_kwargs (model-specific " - "renderers bake their template behavior in). These kwargs will " - "be ignored. Further warnings suppressed for this dataset." - ) - self._warned_chat_template_kwargs = True - - input_ids, loss_mask = build_training_sample( - self.renderer, - messages, - role_to_mask=should_mask, - tools=tools, - ) - else: - try: - input_ids, loss_mask = build_incremental_token_mask( - self.tokenizer, - messages, - role_to_mask=should_mask, - tools=tools, - chat_template_kwargs=example.get("chat_template_kwargs", {}), - collapse_consecutive_tool_messages=True, - ) - except IncrementalTokenizationError as e: - self.logger.warning(f"Skipping example {example.get('__index', '')}: {e}") - return None - - # If EOS token is not found, manually append it - if not self.tokenizer.eos_token_id in input_ids: - self.logger.warning( - f"Did not find EOS token ID {self.tokenizer.eos_token_id} in input_ids. Is something wrong with the chat template? Manually appending EOS token..." - ) - input_ids.append(cast(int, self.tokenizer.eos_token_id)) - loss_mask.append(True) + # Defer to the renderer's sampled_mask by default: a role filter would + # drop sampled stop markers attributed to the next message (e.g. GLM's + # turn-closing <|user|> / <|observation|>). + role_to_mask = None if self.loss_mask_config.assistant else should_mask + + # Non-assistant roles are opted into the loss via the renderer's + # body-only path: the message content is trained, not the role + # scaffolding (e.g. <|im_start|>assistant) the harness emits. + content_sft_roles = {role for role in ("user", "system", "tool") if getattr(self.loss_mask_config, role)} + sample = build_training_sample( + self.renderer, + messages, + role_to_mask=role_to_mask, + tools=tools, + content_sft_roles=content_sft_roles or None, + ensure_final_stop=True, + ) + input_ids = list(sample.token_ids) + loss_mask = list(sample.loss_mask) - # Prepare inputs + # Causal shift: model predicts next token from current. target_ids = input_ids.copy()[1:] loss_mask = loss_mask[1:] input_ids = input_ids[:-1] @@ -280,15 +263,16 @@ def should_mask(message: dict) -> bool: self.logger.warning( f"Skipping example {example.get('__index', '')} because no trainable tokens were found within the context window ({self.seq_len}). This is to prevent NaN loss." ) - return + return None assert len(input_ids) == len(loss_mask) == len(target_ids), ( f"input_ids, loss_mask and target_ids must have the same length, but got {len(input_ids)=}, {len(loss_mask)=}, {len(target_ids)=}" ) assert sum(loss_mask) > 0, "There are no tokens in this sample that contribute to the loss" - assert self.tokenizer.eos_token_id in target_ids, "EOS token ID must be present in target_ids" + assert set(self.renderer.get_stop_token_ids()) & set(target_ids), ( + "A renderer stop token must be present in target_ids" + ) - # Create sample (with one fake target for the last token) return { "input_ids": input_ids, "target_ids": target_ids, @@ -297,9 +281,6 @@ def should_mask(message: dict) -> bool: } def __iter__(self): - """ - Apply chat template and tokenize a single example in prompt + completion format (https://github.com/huggingface/trl/blob/de27d612b026526ba39b88eee348994d7636e033/trl/trainer/sft_trainer.py#L661) - """ dataset = self.dataset.shuffle(seed=self.epoch + self.seed) if self.shuffle else self.dataset while True: self.step += 1 @@ -585,21 +566,25 @@ def setup_dataset( ) -> StatefulIterableDataset: if config.type == "fake": return FakeDataset( - vocab_size=tokenizer.vocab_size, seq_len=config.seq_len, length=config.length, input_ids=config.input_ids + vocab_size=tokenizer.vocab_size, + seq_len=config.seq_len, + length=config.length, + input_ids=config.input_ids, ) elif config.type == "sft": + if renderer is None: + raise ValueError("SFT data requires a renderer.") if raw_dataset is None: raw_dataset = load_sft_dataset(config) return SFTDataset( raw_dataset, - tokenizer, + renderer, shuffle=config.shuffle, seed=config.seed, seq_len=config.seq_len, loss_mask_config=config.loss_mask, non_dp_size=non_dp_size, max_epochs=max_epochs, - renderer=renderer, ) else: raise ValueError(f"Invalid dataset type: {config.type}") diff --git a/src/prime_rl/trainer/sft/train.py b/src/prime_rl/trainer/sft/train.py index 2abb73ec0f..692b5488ea 100644 --- a/src/prime_rl/trainer/sft/train.py +++ b/src/prime_rl/trainer/sft/train.py @@ -5,7 +5,6 @@ from datetime import timedelta from renderers.base import create_renderer -from renderers.default import DefaultRenderer from ring_flash_attn import substitute_hf_flash_attn from torch.nn import CrossEntropyLoss @@ -162,17 +161,12 @@ def train(config: SFTConfig): logger.info(f"Initializing tokenizer ({config.tokenizer})") tokenizer = setup_tokenizer(config.tokenizer) + # Fake data never renders messages, so a model without a hand-coded renderer + # can still be used to benchmark step time / memory. Validation data is + # always real, so it needs the renderer even when training data is fake. renderer = None - if config.renderer is not None: + if config.data.type != "fake" or config.val is not None: renderer = create_renderer(tokenizer, config.renderer) - if isinstance(renderer, DefaultRenderer): - raise ValueError( - f"renderer set for {config.tokenizer.name!r} resolved to DefaultRenderer. " - "DefaultRenderer falls back to incremental apply_chat_template and does NOT " - "fix position-dependent chat templates — the bug the renderer client is meant to solve. " - "Either use a model with a hand-coded renderer (see renderers.base.MODEL_RENDERER_MAP), " - "set [renderer] name= explicitly, or remove the [renderer] block." - ) logger.info(f"Initialized {type(renderer).__name__} for {config.tokenizer.name}") # Set up the optimizer diff --git a/src/prime_rl/utils/chat_template.py b/src/prime_rl/utils/chat_template.py index 400c97c657..fde9f1e92b 100644 --- a/src/prime_rl/utils/chat_template.py +++ b/src/prime_rl/utils/chat_template.py @@ -1,13 +1,5 @@ import json -from typing import Any, Callable - -from transformers.tokenization_utils import PreTrainedTokenizer - - -class IncrementalTokenizationError(ValueError): - """Raised when incremental tokenization produces inconsistent token prefixes.""" - - pass +from typing import Any def normalize_messages(messages: Any, default_role: str) -> list[dict[str, Any]]: @@ -56,79 +48,3 @@ def _deserialize_tool_call(tool_call: dict[str, Any]) -> dict[str, Any]: ) return deserialized_messages - - -def strip_message_content(messages: list[dict[str, Any]]) -> list[dict[str, Any]]: - def _strip(message: dict[str, Any]) -> dict[str, Any]: - content = message.get("content") - if isinstance(content, str): - return {**message, "content": content.strip()} - return message - - return [_strip(message) for message in messages] - - -def should_add_generation_prompt(messages: list[dict[str, Any]], idx: int) -> bool: - role = messages[idx].get("role") - if role not in ("user", "tool"): - return False - if idx + 1 >= len(messages): - return False - return messages[idx + 1].get("role") == "assistant" - - -def render_messages( - tokenizer: PreTrainedTokenizer, - messages: list[dict[str, Any]], - *, - tools: list[dict[str, Any]] | None = None, - chat_template_kwargs: dict[str, Any] | None = None, - add_generation_prompt: bool = False, -) -> list[int]: - kwargs = dict(chat_template_kwargs or {}) - kwargs["add_generation_prompt"] = add_generation_prompt - if tools is not None: - kwargs["tools"] = tools - kwargs["return_dict"] = False - return list(tokenizer.apply_chat_template(messages, **kwargs)) - - -def build_incremental_token_mask( - tokenizer: PreTrainedTokenizer, - messages: list[dict[str, Any]], - *, - role_to_mask: Callable[[dict[str, Any]], bool], - tools: list[dict[str, Any]] | None = None, - chat_template_kwargs: dict[str, Any] | None = None, - collapse_consecutive_tool_messages: bool = False, -) -> tuple[list[int], list[bool]]: - token_mask: list[bool] = [] - prev_ids: list[int] = [] - prev_len = 0 - - for idx, message in enumerate(messages): - role = message.get("role") - if collapse_consecutive_tool_messages and role == "tool" and idx + 1 < len(messages): - if messages[idx + 1].get("role") == "tool": - continue - - cur_ids = render_messages( - tokenizer, - messages[: idx + 1], - tools=tools, - chat_template_kwargs=chat_template_kwargs, - add_generation_prompt=should_add_generation_prompt(messages, idx), - ) - - if prev_ids != cur_ids[:prev_len]: - raise IncrementalTokenizationError( - f"Mismatch in incremental tokenization with chat template at message {idx} (role={role}). " - "This usually means the chat template is not stable under incremental application. " - "The sample will be skipped." - ) - - token_mask.extend([role_to_mask(message)] * (len(cur_ids) - prev_len)) - prev_ids = cur_ids - prev_len = len(cur_ids) - - return prev_ids, token_mask diff --git a/tests/unit/test_configs.py b/tests/unit/test_configs.py index 49243a9f98..57687d5666 100644 --- a/tests/unit/test_configs.py +++ b/tests/unit/test_configs.py @@ -531,6 +531,26 @@ def test_orchestrator_renderer_auto_accepts_mapped_model(): assert config.renderer.name == "auto" +def test_sft_renderer_auto_accepts_prime_qwen_model(): + config = SFTConfig.model_validate({"model": {"name": "PrimeIntellect/Qwen3-0.6B"}}) + assert config.renderer.name == "auto" + + +def test_sft_rejects_default_renderer_for_real_data(): + with pytest.raises(ValidationError, match="requires a typed renderer"): + SFTConfig.model_validate({"renderer": {"name": "default"}}) + + +def test_sft_allows_unused_default_renderer_for_fake_data(): + config = SFTConfig.model_validate( + { + "data": {"type": "fake"}, + "renderer": {"name": "default"}, + } + ) + assert config.renderer.name == "default" + + def test_orchestrator_explicit_renderer_skips_unmapped_check(): """Explicit renderer.name bypasses the auto-resolution check — user opted in.""" config = OrchestratorConfig.model_validate( diff --git a/tests/unit/train/sft/test_sft_dataset.py b/tests/unit/train/sft/test_sft_dataset.py index b8465e59d7..fc72cb533b 100644 --- a/tests/unit/train/sft/test_sft_dataset.py +++ b/tests/unit/train/sft/test_sft_dataset.py @@ -2,64 +2,125 @@ import pytest from datasets import Dataset, interleave_datasets +from renderers import create_renderer +from renderers.base import RenderedTokens from transformers import AutoTokenizer -from prime_rl.trainer.sft.data import SFTDataset +from prime_rl.trainer.sft.data import SFTDataset, _drop_null_fields from prime_rl.trainer.utils import print_sample +_BOS_TOKEN_ID = 0 +_STOP_TOKEN_ID = 1 + + +def _sample_token_ids(value: str) -> list[int]: + return [ord(char) + 2 for char in value] + + +class _DummyRenderer: + def render(self, messages, **kwargs): + content_ids = _sample_token_ids(messages[-1]["content"]) + token_ids = [_BOS_TOKEN_ID, *content_ids, _STOP_TOKEN_ID] + return RenderedTokens( + token_ids=token_ids, + message_indices=[-1, *([len(messages) - 1] * (len(content_ids) + 1))], + sampled_mask=[False, *([True] * (len(content_ids) + 1))], + ) + + def get_stop_token_ids(self): + return [_STOP_TOKEN_ID] + @pytest.fixture(scope="module") def build_dummy_dataset(): - return lambda letter, num_examples: Dataset.from_list([{"text": f"{letter}{i}"} for i in range(num_examples)]) + return lambda letter, num_examples: Dataset.from_list( + [{"messages": [{"role": "assistant", "content": f"{letter}{i}"}]} for i in range(num_examples)] + ) + + +@pytest.fixture +def dummy_renderer(): + return _DummyRenderer() + + +@pytest.mark.parametrize( + "arguments", + [ + pytest.param('{"reasoning_effort": null}', id="json-string"), + pytest.param({"reasoning_effort": None}, id="dict"), + ], +) +def test_drop_null_fields_preserves_tool_call_arguments(arguments): + message = { + "role": "assistant", + "content": [{"type": "text", "text": "Calling a tool", "image_url": None}], + "tool_calls": [{"function": {"name": "listReasoningModels", "arguments": arguments}}], + "metadata": {"arguments": {"unrelated_null": None}}, + } + + cleaned = _drop_null_fields(message) + + assert cleaned["tool_calls"][0]["function"]["arguments"] == arguments + assert cleaned["content"] == [{"type": "text", "text": "Calling a tool"}] + assert cleaned["metadata"] == {"arguments": {}} -def test_init_sft_dataset(build_dummy_dataset): +def test_init_sft_dataset(build_dummy_dataset, dummy_renderer): """Tests basic initialization.""" dataset = build_dummy_dataset("a", 1) - sft_dataset = SFTDataset(dataset, tokenizer=None) + sft_dataset = SFTDataset(dataset, dummy_renderer) assert sft_dataset is not None def test_raise_error_if_no_prompt_and_completion(build_dummy_dataset): """Tests that an error is raised if no supported SFT message fields are provided.""" - dataset = build_dummy_dataset("a", 1) + dataset = Dataset.from_list([{"text": "a0"}]) tokenizer = AutoTokenizer.from_pretrained("Qwen/Qwen3-0.6B") - sft_dataset = SFTDataset(dataset, tokenizer=tokenizer) + sft_dataset = SFTDataset(dataset, create_renderer(tokenizer)) with pytest.raises(ValueError): next(iter(sft_dataset)) @pytest.mark.parametrize("max_epochs", [1, 2, 4]) -def test_sft_first_exhausted(build_dummy_dataset, max_epochs: int): +def test_sft_first_exhausted(build_dummy_dataset, dummy_renderer, max_epochs: int): a = build_dummy_dataset("a", 1) b = build_dummy_dataset("b", 2) ds = [a, b] dataset = interleave_datasets(ds, stopping_strategy="first_exhausted") - dataset = SFTDataset(dataset, tokenizer=None, shuffle=False, max_epochs=max_epochs) + dataset = SFTDataset(dataset, dummy_renderer, shuffle=False, max_epochs=max_epochs) num_samples = 0 sampling_order = [] for x in dataset: - sampling_order.append(x["text"]) + sampling_order.append(x["target_ids"][:-1]) num_samples += 1 assert num_samples == max_epochs * min([len(d) for d in ds]) * len(ds) - assert sampling_order == ["a0", "b0"] * max_epochs + assert sampling_order == [_sample_token_ids("a0"), _sample_token_ids("b0")] * max_epochs @pytest.mark.parametrize("max_epochs", [1, 2, 4]) -def test_sft_all_exhausted(build_dummy_dataset, max_epochs: int): +def test_sft_all_exhausted(build_dummy_dataset, dummy_renderer, max_epochs: int): a = build_dummy_dataset("a", 1) b = build_dummy_dataset("b", 2) ds = [a, b] dataset = interleave_datasets(ds, stopping_strategy="all_exhausted") - dataset = SFTDataset(dataset, tokenizer=None, shuffle=False, max_epochs=max_epochs) + dataset = SFTDataset(dataset, dummy_renderer, shuffle=False, max_epochs=max_epochs) num_samples = 0 sampling_order = [] for x in dataset: - sampling_order.append(x["text"]) + sampling_order.append(x["target_ids"][:-1]) num_samples += 1 assert num_samples == max_epochs * max([len(d) for d in ds]) * len(ds) print(sampling_order) - assert sampling_order == ["a0", "b0", "a0", "b1"] * max_epochs + assert ( + sampling_order + == [ + _sample_token_ids("a0"), + _sample_token_ids("b0"), + _sample_token_ids("a0"), + _sample_token_ids("b1"), + ] + * max_epochs + ) @pytest.mark.parametrize( @@ -70,21 +131,21 @@ def test_sft_all_exhausted(build_dummy_dataset, max_epochs: int): pytest.param((9 / 10, 1 / 10), id="high_low_probs"), ], ) -def test_sft_all_exhausted_with_probs(build_dummy_dataset, probs: list[float]): +def test_sft_all_exhausted_with_probs(build_dummy_dataset, dummy_renderer, probs: list[float]): """Tests that the ratio of samples from different datasets is as specified, in expectation.""" a = build_dummy_dataset("a", int(1e3)) b = build_dummy_dataset("b", int(10e3)) ds = [a, b] dataset = interleave_datasets(ds, stopping_strategy="all_exhausted", probabilities=probs) - dataset = SFTDataset(dataset, tokenizer=None, shuffle=False, max_epochs=1) + dataset = SFTDataset(dataset, dummy_renderer, shuffle=False, max_epochs=1) num_samples = 0 sampling_freq = [] for x in dataset: - sampling_freq.append(x["text"][0]) + sampling_freq.append(x["target_ids"][0]) num_samples += 1 sampling_freq = Counter(sampling_freq) - ratio_a = sampling_freq["a"] / num_samples - ratio_b = sampling_freq["b"] / num_samples + ratio_a = sampling_freq[ord("a") + 2] / num_samples + ratio_b = sampling_freq[ord("b") + 2] / num_samples assert ratio_a > probs[0] * 0.8 and ratio_a < probs[0] * 1.2, ( f"Expected frequency of samples from a to be between {probs[0] * 0.8} and {probs[0] * 1.2}, but got {ratio_a}" ) @@ -93,10 +154,10 @@ def test_sft_all_exhausted_with_probs(build_dummy_dataset, probs: list[float]): ) -def test_sft_dataset_state(build_dummy_dataset): +def test_sft_dataset_state(build_dummy_dataset, dummy_renderer): """Tests the state of the dataset within and across epochs.""" dataset = build_dummy_dataset("", 4) - dataset = SFTDataset(dataset, tokenizer=None, shuffle=False, max_epochs=2) + dataset = SFTDataset(dataset, dummy_renderer, shuffle=False, max_epochs=2) dataiter = iter(dataset) # Initial state @@ -105,24 +166,24 @@ def test_sft_dataset_state(build_dummy_dataset): # Epoch 1 for i in range(4): sample = next(dataiter) - assert sample["text"] == str(i) + assert sample["target_ids"][:-1] == _sample_token_ids(str(i)) assert dataset.state_dict() == {"epoch": 0, "step": i + 1} # Epoch 2 for i in range(4): sample = next(dataiter) - assert sample["text"] == str(i) + assert sample["target_ids"][:-1] == _sample_token_ids(str(i)) assert dataset.state_dict() == {"epoch": 1, "step": 4 + i + 1} with pytest.raises(StopIteration): next(dataiter) -def test_sft_dataset_state_resume(build_dummy_dataset): +def test_sft_dataset_state_resume(build_dummy_dataset, dummy_renderer): """Tests resuming the dataset from checkpoint in between epochs.""" dataset = SFTDataset( build_dummy_dataset("", 4), - tokenizer=None, + dummy_renderer, shuffle=False, max_epochs=2, ) @@ -134,8 +195,7 @@ def test_sft_dataset_state_resume(build_dummy_dataset): # Epoch 1 for i in range(4): sample = next(dataiter) - print(sample["text"]) - assert sample["text"] == str(i) + assert sample["target_ids"][:-1] == _sample_token_ids(str(i)) assert dataset.state_dict() == {"epoch": 0, "step": i + 1} # Resuming from checkpoint cross epoch @@ -143,7 +203,7 @@ def test_sft_dataset_state_resume(build_dummy_dataset): del dataset dataset = SFTDataset( build_dummy_dataset("", 4), - tokenizer=None, + dummy_renderer, shuffle=False, max_epochs=2, ) @@ -153,8 +213,7 @@ def test_sft_dataset_state_resume(build_dummy_dataset): # Epoch 2.1 for i in range(2): sample = next(dataiter) - print(sample["text"]) - assert sample["text"] == str(i) + assert sample["target_ids"][:-1] == _sample_token_ids(str(i)) assert dataset.state_dict() == {"epoch": 1, "step": 4 + i + 1} # Resuming from checkpoint mid epoch @@ -162,7 +221,7 @@ def test_sft_dataset_state_resume(build_dummy_dataset): del dataset dataset = SFTDataset( build_dummy_dataset("", 4), - tokenizer=None, + dummy_renderer, shuffle=False, max_epochs=2, ) @@ -172,8 +231,7 @@ def test_sft_dataset_state_resume(build_dummy_dataset): # Epoch 2.2 for i in range(2, 4): sample = next(dataiter) - print(sample["text"]) - assert sample["text"] == str(i) + assert sample["target_ids"][:-1] == _sample_token_ids(str(i)) assert dataset.state_dict() == {"epoch": 1, "step": 4 + i + 1} with pytest.raises(StopIteration): @@ -194,7 +252,7 @@ def test_multiturn_loss_mask(): ] ) tokenizer = AutoTokenizer.from_pretrained("PrimeIntellect/Qwen3-0.6B") # Properly handles multi-turn think - dataset = SFTDataset(dataset, tokenizer=tokenizer, max_examples=1) + dataset = SFTDataset(dataset, create_renderer(tokenizer), max_examples=1) sample = next(iter(dataset)) print_sample(sample["input_ids"], sample["loss_mask"], tokenizer) @@ -257,7 +315,7 @@ def test_multiturn_loss_mask_with_tools(): dataset = Dataset.from_list([tool_example]) tokenizer = AutoTokenizer.from_pretrained("PrimeIntellect/Qwen3-0.6B") # Properly handles multi-turn think - dataset = SFTDataset(dataset, tokenizer=tokenizer, max_examples=1) + dataset = SFTDataset(dataset, create_renderer(tokenizer), max_examples=1) sample = next(iter(dataset)) print_sample(sample["input_ids"], sample["loss_mask"], tokenizer) @@ -282,10 +340,14 @@ def test_messages_rows_are_equivalent_to_empty_prompt_completion(): ] tokenizer = AutoTokenizer.from_pretrained("PrimeIntellect/Qwen3-0.6B") - messages_dataset = SFTDataset(Dataset.from_list([{"messages": messages}]), tokenizer=tokenizer, max_examples=1) + messages_dataset = SFTDataset( + Dataset.from_list([{"messages": messages}]), + create_renderer(tokenizer), + max_examples=1, + ) split_dataset = SFTDataset( Dataset.from_list([{"prompt": [], "completion": messages}]), - tokenizer=tokenizer, + create_renderer(tokenizer), max_examples=1, ) @@ -304,11 +366,36 @@ def test_messages_take_precedence_over_prompt_and_completion(): "completion": [{"role": "assistant", "content": "Ignored completion"}], } - messages_dataset = SFTDataset(Dataset.from_list([row]), tokenizer=tokenizer, max_examples=1) + messages_dataset = SFTDataset( + Dataset.from_list([row]), + create_renderer(tokenizer), + max_examples=1, + ) expected_dataset = SFTDataset( Dataset.from_list([{"prompt": [], "completion": row["messages"]}]), - tokenizer=tokenizer, + create_renderer(tokenizer), max_examples=1, ) assert next(iter(messages_dataset)) == next(iter(expected_dataset)) + + +def test_null_messages_falls_back_to_prompt_and_completion(): + # Arrow schema union adds `messages: None` to prompt/completion rows when + # other rows in the file have a `messages` column + tokenizer = AutoTokenizer.from_pretrained("PrimeIntellect/Qwen3-0.6B") + prompt = [{"role": "user", "content": "What is 2+2?"}] + completion = [{"role": "assistant", "content": "4"}] + + mixed_row_dataset = SFTDataset( + Dataset.from_list([{"messages": None, "prompt": prompt, "completion": completion}]), + create_renderer(tokenizer), + max_examples=1, + ) + expected_dataset = SFTDataset( + Dataset.from_list([{"prompt": prompt, "completion": completion}]), + create_renderer(tokenizer), + max_examples=1, + ) + + assert next(iter(mixed_row_dataset)) == next(iter(expected_dataset))