Repository navigation
Python: Fix Handlebars messages helper ignoring its argument - #14537
PRABHU KIRAN VANDRANKI (VANDRANKI) wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The newly introduced no-argument fallback lacks integration coverage.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Fixes the Handlebars messages helper to honor its argument while retaining context fallback behavior.
Changes:
- Uses the first helper argument when provided.
- Adds regression tests for argument selection.
| File | Description |
|---|---|
handlebars_system_helpers.py |
Corrects chat-history resolution. |
test_handlebars_prompt_template.py |
Tests explicit argument handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| async def test_helpers_chat_history_messages_uses_argument(kernel: Kernel): | ||
| template = """{{messages history}}""" | ||
| target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) | ||
| chat_history = ChatHistory() | ||
| chat_history.add_user_message("User message") | ||
| rendered = await target.render(kernel, KernelArguments(history=chat_history)) | ||
| assert ( | ||
| rendered.strip() == """<chat_history><message role="user"><text>User message</text></message></chat_history>""" | ||
| ) |
There was a problem hiding this comment.
Good catch. I added test_helpers_chat_history_messages_without_argument_uses_chat_history_variable in b5adc4a. It renders a bare {{messages}} with only chat_history in the arguments and checks the output, so the no-argument fallback branch is now covered. It passes. It would also pass on main, since bare {{messages}} worked before. It guards the fallback so this change cannot break it.

Motivation and Context
The Handlebars
messageshelper ignores its argument.{{messages my_history}}always readschat_historyfrom the context, so it raisesKeyErrorwhen that variable is missing and renders the wrong history when both exist. The Jinja2messages(history)helper uses its argument.Fixes #14535
Description
_messagesinhandlebars_system_helpers.pynow uses the first positional argument. If no argument is given it falls back to thechat_historycontext variable, so existing{{messages chat_history}}templates behave the same. The unusedoptionsparameter was removed. For a non-block helper pybars passes the first argument in that slot, which is why the argument was never visible. A bare{{messages}}used to raiseTypeErrorand now works through the fallback.Added two tests (argument with a different variable name, and argument preferred over a
chat_historyvariable). Both fail onmain.tests/unit/prompt_template,tests/unit/kernelandtests/unit/functionspass. ruff check clean on the touched files, mypy clean on the helper module.Contribution Checklist