Skip to content

Conditionally add the generation prompt to the multi-turn chat template#676

Merged
SumanthRH merged 1 commit intoNovaSky-AI:mainfrom
ebronstein:multiturn_gen_prompt
Nov 17, 2025
Merged

Conditionally add the generation prompt to the multi-turn chat template#676
SumanthRH merged 1 commit intoNovaSky-AI:mainfrom
ebronstein:multiturn_gen_prompt

Conversation

@ebronstein
Copy link
Contributor

Currently, the Generator always adds the generation prompt to the input_ids for multi-turn chat templating. This should only happen if the trajectory is not done.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly addresses a bug where a generation prompt was unconditionally added during multi-turn chat templating, even for completed trajectories. The change to use add_generation_prompt=not done is a clean and accurate fix. It prevents appending an unnecessary generation prompt at the end of a trajectory and makes the logic consistent with the case where there are no new observations. The change is sound and improves the correctness of the generator.

@SumanthRH SumanthRH merged commit 8ad3d5e into NovaSky-AI:main Nov 17, 2025
3 checks passed
li-boxuan pushed a commit to li-boxuan/SkyRL that referenced this pull request Nov 23, 2025
…rn chat template (NovaSky-AI#676)

Currently, the Generator always adds the generation prompt to the
`input_ids` for multi-turn chat templating. This should only happen if
the trajectory is not done.
dzorlu pushed a commit to fleet-ai/SkyRL that referenced this pull request Feb 4, 2026
…rn chat template (NovaSky-AI#676)

Currently, the Generator always adds the generation prompt to the
`input_ids` for multi-turn chat templating. This should only happen if
the trajectory is not done.
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