Skip to content

feat: stop generation on the token the chat template teaches - #59

Merged
Neonkraft merged 2 commits into
mainfrom
feat/align-eos-with-chat-template
Aug 28, 2026
Merged

Neonkraft merged 2 commits into
mainfrom
feat/align-eos-with-chat-template

Conversation

@KonstiNik

Copy link
Copy Markdown
Member

Summary

SFT trains the model to emit whatever the chat template ends a turn with — <|im_end|> under qwen3 — but generate() stops on model.generation_config.eos_token_id, not tokenizer.eos_token, and that value is the model's pretraining eos, which never appears in the SFT data. So the model emits the token it was trained to emit, nothing is listening, and generation runs to max_new_tokens on every prompt.

The fix makes the tokenizer and the model agree with the template. build_tokenizer renders a short probe conversation, takes the added token that render ends on as the terminator, and sets tokenizer.eos_token to it; align_generation_eos then puts that token first in generation_config.eos_token_id, keeping the model's original eos behind it as a secondary stop. Reading the terminator off the render, rather than from a hardcoded map of template to terminator, is deliberate: a map goes stale when a template changes, and it fails the same silent way this PR fixes. Templates that already end on eos_token — every olmo3-* — need no change and get none. Both sft.py and dpo.py use these helpers, so DPO is covered.

Type of change

  • Bug fix
  • New feature
  • Refactor
  • Performance
  • Documentation
  • Maintenance

Validation

pytest tests/ — 147 pass, 19 new; ruff 0.9.10 and black 25.1.0 clean.

test_generate_stops_on_the_template_terminator_after_alignment drives a real model.generate() on a tiny randomly-initialised model, forcing the terminator every step so stopping is the only variable: 20 generated tokens before alignment, 1 after. The olmo3-* no-op is pinned by parametrised tests.

SFT trains the model to emit whatever the template ends a turn with — <|im_end|>
under qwen3 — while generate() reads the model's generation_config, which still
holds the pretraining eos that the data never contains. So the model emits the
token it was trained to emit and nothing is listening, running to max_new_tokens
on every prompt.

The terminator is read off the rendered template rather than from a lookup table,
so it cannot drift from the thing it describes. The model's own eos is kept as a
secondary stop id. A no-op for the olmo3-* templates, which already end a final
turn on eos_token.
Comment thread src/post_training/methods/common.py
return bool(_GENERATION_OPEN_RE.search(template) and _GENERATION_CLOSE_RE.search(template))


def terminator_from_render(rendered: str, added_tokens: dict[str, int]) -> str | None:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This name was a bit confusing for me. Something like infer_eos_token_from_render would be more intuitive.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Renamed to infer_end_token_from_render.

I avoided eos_token because what comes back is a template-side observation. Under qwen3 it's <|im_end|>, which is neither the model's eos nor a stop token until align_generation_eos makes it one. Under the olmo3-* templates, it is the eos, but only coincidentally, because those templates terminate on it.

Does the current choice work for you?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, makes sense :) LGTM, merging.

Comment thread tests/test_eos_alignment.py

@Neonkraft Neonkraft left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rename the terminator_from_render function and it looks good to merge :)

Review feedback: the old name did not say terminator of what. Avoiding both
"eos" and "stop" in the name is deliberate and now stated in the docstring —
what comes back is a template-side observation, and under qwen3 it is
<|im_end|>, which is neither the model's eos nor a stop token until
align_generation_eos makes it one.
@Neonkraft
Neonkraft self-requested a review August 28, 2026 14:39

@Neonkraft Neonkraft left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM!

@Neonkraft
Neonkraft merged commit db9d560 into main Aug 28, 2026
2 checks passed
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