Skip to content

[Bug] [Question] Is the enable_thinking gate in boundary detection intentional for loss mask behavior? #290

Description

@berserkr

Question: Is the enable_thinking gate in boundary detection intentional for loss mask behavior?

In src/nemotron/data_prep/core/chat_template.py, the boundary detection for is gated on enable_thinking:

if enable_thinking and (
"reasoning_content" not in messages[last_user_idx + 1]
or messages[last_user_idx + 1]["reasoning_content"] == ""
):
template_up_to_here += "<|im_start|>assistant\n"

This means for conversations where no assistant has reasoning_content (enable_thinking=False), the tokens end up in the supervised region (loss=1) rather than the prompt boundary (loss=0).

We noticed 76d770b ("Bug fix for issue 184") explicitly preserved this behavior — the commit message states: "The actual rendering pipeline is preserved byte-for-byte to keep output identical to the original."

Is this intentional? Specifically: is the design intent that non-reasoning conversations should have loss=1 on (training the model on the full transition into content), while reasoning conversations have loss=0 (since the model uses \n → reasoning at inference instead)?

We're asking because at inference the generation prompt provides regardless, so loss=1 vs loss=0 doesn't affect generation — but we want to confirm before changing behavior for reproducibility.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions