Skip to content

Add add_generation_prompt=True when tokenizing judge prompts - #1347

Open
Nakul-Sinha wants to merge 2 commits into
huggingface:mainfrom
Nakul-Sinha:fix/judge-add-generation-prompt
Open

Nakul-Sinha wants to merge 2 commits into
huggingface:mainfrom
Nakul-Sinha:fix/judge-add-generation-prompt

Conversation

@Nakul-Sinha

Copy link
Copy Markdown

Fixes #1346

Problem

JudgeLM.__call_vllm renders judge prompts with apply_chat_template(p) and no add_generation_prompt. The rendered text therefore ends at the close of the user turn (for example ...<|im_end|>\n) and never opens the assistant turn, so the model has to infer that a reply is what comes next.

Most of the time it does. Intermittently it instead continues the conversation by starting a fresh user turn, and the resulting text does not parse into a score. The reporter saw this with Qwen2.5-7B-Instruct as the judge.

Change

Pass add_generation_prompt=True when tokenizing.

This matches how the rest of the codebase renders chat prompts. Both other apply_chat_template call sites in src/ already pass it:

  • src/lighteval/tasks/prompt_manager.py:82
  • src/lighteval/tasks/prompt_manager.py:132

src/lighteval/metrics/utils/llm_as_judge.py:304 was the only call site that did not, so this brings the judge in line with the existing convention rather than introducing a new one.

Testing

I was not able to exercise the vLLM judge path locally, since it needs a vLLM runtime and a GPU that I do not have. What I did check:

  • python -m py_compile src/lighteval/metrics/utils/llm_as_judge.py passes.
  • Reviewed every apply_chat_template call site in src/ to confirm the argument is consistent with the two in prompt_manager.py and that no other call site was missed.

A maintainer with a vLLM setup may want to confirm against a judge model before merging.

AI assistance was used for this change.

@fukalous fukalous 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.

The behavior change checks out: with the Qwen2.5 tokenizer, the old render stops after the closed user turn, while this flag appends the assistant generation prefix. I also exercised __call_vllm with a fake tokenizer/pipeline and confirmed the token IDs and outputs are forwarded unchanged. Ruff/format checks and the 28 existing prompt-manager tests pass.

One test gap remains: there is still no coverage of the vLLM judge call site, so removing this keyword later would leave the suite green and recreate #1346. Please add a CPU-only unit test with a fake tokenizer and pipe that asserts apply_chat_template(prompt, add_generation_prompt=True) and the resulting prompt_token_ids; it does not require vLLM or a GPU.

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.

[BUG] Why isn't apply_chat_template called with add_generation_prompt=True in LLM as a judge?

2 participants