diff --git a/python/samples/concepts/prompt_templates/azure_chat_gpt_api_handlebars.py b/python/samples/concepts/prompt_templates/azure_chat_gpt_api_handlebars.py index 3b15966391b8..cce70393587a 100644 --- a/python/samples/concepts/prompt_templates/azure_chat_gpt_api_handlebars.py +++ b/python/samples/concepts/prompt_templates/azure_chat_gpt_api_handlebars.py @@ -39,7 +39,7 @@ chat_function = kernel.add_function( prompt_template_config=PromptTemplateConfig( template="""{{system_message}}{{#each chat_history}} - {{#message role=role}}{{~content~}}{{/message}} {{/each}}""", + {{message_to_prompt}} {{/each}}""", template_format="handlebars", allow_dangerously_set_content=True, ), diff --git a/python/samples/concepts/prompt_templates/azure_chat_gpt_api_jinja2.py b/python/samples/concepts/prompt_templates/azure_chat_gpt_api_jinja2.py index 230c3b337a1e..9ee8ebb65bf1 100644 --- a/python/samples/concepts/prompt_templates/azure_chat_gpt_api_jinja2.py +++ b/python/samples/concepts/prompt_templates/azure_chat_gpt_api_jinja2.py @@ -38,7 +38,7 @@ chat_function = kernel.add_function( prompt_template_config=PromptTemplateConfig( - template="""{{system_message}}{% for item in chat_history %}{{ message(item) }}{% endfor %}""", + template="""{{system_message}}{% for item in chat_history %}{{ message_to_prompt(item) }}{% endfor %}""", template_format="jinja2", allow_dangerously_set_content=True, ), diff --git a/python/semantic_kernel/prompt_template/utils/handlebars_system_helpers.py b/python/semantic_kernel/prompt_template/utils/handlebars_system_helpers.py index d85d85a26679..ea60447f2cb9 100644 --- a/python/semantic_kernel/prompt_template/utils/handlebars_system_helpers.py +++ b/python/semantic_kernel/prompt_template/utils/handlebars_system_helpers.py @@ -5,6 +5,7 @@ import re from collections.abc import Callable from enum import Enum +from xml.etree.ElementTree import Element, SubElement, tostring # nosec B405 logger: logging.Logger = logging.getLogger(__name__) @@ -26,23 +27,32 @@ def _message_to_prompt(this, *args, **kwargs): def _message(this, options, *args, **kwargs): + from semantic_kernel.contents.chat_message_content import ChatMessageContent from semantic_kernel.contents.const import CHAT_MESSAGE_CONTENT_TAG - # everything in kwargs, goes to - # everything in options, goes in between options - start = f"<{CHAT_MESSAGE_CONTENT_TAG}" + # When the context is a ChatMessageContent, delegate to to_element() so that + # the XML contract is consistent with the Jinja2 path. + if isinstance(this.context, ChatMessageContent): + message = this.context.to_element() + return tostring(message, encoding="unicode", short_empty_elements=False) + + # Fallback: build the element manually from kwargs and block content. + from semantic_kernel.contents.const import TEXT_CONTENT_TAG + + message = Element(CHAT_MESSAGE_CONTENT_TAG) for key, value in kwargs.items(): if isinstance(value, Enum): value = value.value if value is not None: - start += f' {key}="{value}"' - start += ">" - end = f"" + message.set(key, str(value)) try: - content = options["fn"](this) + content = str(options["fn"](this)) except Exception: # pragma: no cover content = "" - return f"{start}{content}{end}" + if content: + text_elem = SubElement(message, TEXT_CONTENT_TAG) + text_elem.text = content + return tostring(message, encoding="unicode", short_empty_elements=False) def _set(this, *args, **kwargs): diff --git a/python/semantic_kernel/prompt_template/utils/jinja2_system_helpers.py b/python/semantic_kernel/prompt_template/utils/jinja2_system_helpers.py index 921cd1be3982..480198d8d3cc 100644 --- a/python/semantic_kernel/prompt_template/utils/jinja2_system_helpers.py +++ b/python/semantic_kernel/prompt_template/utils/jinja2_system_helpers.py @@ -3,7 +3,7 @@ import logging import re from collections.abc import Callable -from enum import Enum +from xml.etree.ElementTree import tostring # nosec B405 logger: logging.Logger = logging.getLogger(__name__) @@ -25,17 +25,8 @@ def _message_to_prompt(context): def _message(item): - from semantic_kernel.contents.const import CHAT_MESSAGE_CONTENT_TAG - - start = f"<{CHAT_MESSAGE_CONTENT_TAG}" - role = item.role - content = item.content - if isinstance(role, Enum): - role = role.value - start += f' role="{role}"' - start += ">" - end = f"" - return f"{start}{content}{end}" + message = item.to_element() + return tostring(message, encoding="unicode", short_empty_elements=False) # Wrap the _get function to safely handle calls without arguments diff --git a/python/tests/unit/prompt_template/test_handlebars_prompt_template.py b/python/tests/unit/prompt_template/test_handlebars_prompt_template.py index ab0f648f7891..6efea53f50c7 100644 --- a/python/tests/unit/prompt_template/test_handlebars_prompt_template.py +++ b/python/tests/unit/prompt_template/test_handlebars_prompt_template.py @@ -261,6 +261,108 @@ async def test_helpers_message(kernel: Kernel): assert "Assistant message" in rendered +async def test_helpers_message_escapes_xml_metacharacters(kernel: Kernel): + template = """ +{{#each chat_history}} + {{#message role=role}} + {{~content~}} + {{/message}} +{{/each}} +""" + target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) + chat_history = ChatHistory() + chat_history.add_user_message('What does a < b & a > c & "d" mean?') + + rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) + + assert "<" in rendered + assert "&" in rendered + # ElementTree does not escape > in text content (valid XML); verify round-trip works regardless. + assert "a > c" in rendered or "a > c" in rendered + assert '"d"' in rendered + assert ChatHistory.from_rendered_prompt(rendered) == chat_history + + +async def test_helpers_message_uses_text_element(kernel: Kernel): + """Verify handlebars {{#message}} wraps content in like the Jinja2 path.""" + template = """ +{{#each chat_history}} + {{#message role=role}} + {{~content~}} + {{/message}} +{{/each}} +""" + target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) + chat_history = ChatHistory() + chat_history.add_user_message("User message") + chat_history.add_assistant_message("Assistant message") + rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) + assert 'User message' in rendered + assert 'Assistant message' in rendered + chat_history2 = ChatHistory.from_rendered_prompt(rendered) + assert chat_history2 == chat_history + + +async def test_helpers_message_empty_content(kernel: Kernel): + """Empty message content should produce , not self-closing.""" + template = """ +{{#each chat_history}} + {{#message role=role}} + {{~content~}} + {{/message}} +{{/each}} +""" + target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) + chat_history = ChatHistory() + chat_history.add_user_message("") + rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) + assert "" not in rendered + assert ChatHistory.from_rendered_prompt(rendered) is not None + + +async def test_helpers_message_fallback_empty_content(kernel: Kernel): + """Fallback path (non-ChatMessageContent context) with empty block content. + + Should produce , not self-closing. + """ + template = '{{#message role="user"}}{{/message}}' + target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) + rendered = await target.render(kernel, KernelArguments()) + assert '' in rendered + assert "/>" not in rendered + assert ChatHistory.from_rendered_prompt(rendered) is not None + + +async def test_helpers_message_fallback_with_content(kernel: Kernel): + """Fallback path wraps block content in a child element.""" + template = '{{#message role="user"}}Hello world{{/message}}' + target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) + rendered = await target.render(kernel, KernelArguments()) + assert 'Hello world' in rendered + chat_history = ChatHistory.from_rendered_prompt(rendered) + assert chat_history is not None + assert len(chat_history) == 1 + assert chat_history[0].content == "Hello world" + + +async def test_helpers_message_escapes_greater_than(kernel: Kernel): + """ElementTree does not escape > in text; verify round-trip still works.""" + template = """ +{{#each chat_history}} + {{#message role=role}} + {{~content~}} + {{/message}} +{{/each}} +""" + target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) + chat_history = ChatHistory() + chat_history.add_user_message("Is a > b true?") + rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) + assert "a > b" in rendered or "a > b" in rendered + assert ChatHistory.from_rendered_prompt(rendered) == chat_history + + async def test_helpers_message_to_prompt(kernel: Kernel): template = """{{#each chat_history}}{{message_to_prompt}} {{/each}}""" target = create_handlebars_prompt_template(template, allow_dangerously_set_content=True) diff --git a/python/tests/unit/prompt_template/test_handlebars_prompt_template_e2e.py b/python/tests/unit/prompt_template/test_handlebars_prompt_template_e2e.py index 5c77625b85e2..fc5226cecab1 100644 --- a/python/tests/unit/prompt_template/test_handlebars_prompt_template_e2e.py +++ b/python/tests/unit/prompt_template/test_handlebars_prompt_template_e2e.py @@ -2,6 +2,7 @@ from semantic_kernel import Kernel +from semantic_kernel.contents import AuthorRole from semantic_kernel.contents.chat_history import ChatHistory from semantic_kernel.functions import kernel_function from semantic_kernel.functions.kernel_arguments import KernelArguments @@ -94,9 +95,46 @@ async def test_chat_history_round_trip(self, kernel: Kernel): chat_history.add_user_message("User message") chat_history.add_assistant_message("Assistant message") rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) - assert ( - rendered.strip() - == """User message Assistant message""" + expected = ( + 'User message' + ' Assistant message' ) + assert rendered.strip() == expected chat_history2 = ChatHistory.from_rendered_prompt(rendered) assert chat_history2 == chat_history + + async def test_chat_history_round_trip_with_xml_metacharacters(self, kernel: Kernel): + # Arrange + template = """{{#each chat_history}}{{#message role=role}}{{~content~}}{{/message}} {{/each}}""" + target = create_handlebars_prompt_template(template) + chat_history = ChatHistory() + chat_history.add_user_message("What does a < b mean in Python?") + chat_history.add_assistant_message('Use "&" carefully in XML and HTML.') + + rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) + + assert "<" in rendered + assert "&" in rendered + assert '"&"' in rendered + assert ChatHistory.from_rendered_prompt(rendered) == chat_history + + async def test_message_helper_preserves_system_role_with_xml_metacharacters(self, kernel: Kernel): + # Arrange + template = ( + """{{system_message}}{{#each chat_history}}{{#message role=role}}{{~content~}}{{/message}} {{/each}}""" + ) + target = create_handlebars_prompt_template(template) + system_message = "You are a helpful assistant." + chat_history = ChatHistory() + chat_history.add_user_message("What does a < b mean in Python?") + + rendered = await target.render( + kernel, + KernelArguments(system_message=system_message, chat_history=chat_history), + ) + + parsed = ChatHistory.from_rendered_prompt(rendered) + assert parsed.messages[0].role == AuthorRole.SYSTEM + assert parsed.messages[0].content == system_message + assert parsed.messages[1].role == AuthorRole.USER + assert parsed.messages[1].content == "What does a < b mean in Python?" diff --git a/python/tests/unit/prompt_template/test_jinja2_prompt_template.py b/python/tests/unit/prompt_template/test_jinja2_prompt_template.py index ef5a11983a04..6710de7a5e37 100644 --- a/python/tests/unit/prompt_template/test_jinja2_prompt_template.py +++ b/python/tests/unit/prompt_template/test_jinja2_prompt_template.py @@ -264,6 +264,20 @@ async def test_helpers_message(kernel: Kernel): assert "Assistant message" in rendered +async def test_helpers_message_escapes_xml_metacharacters(kernel: Kernel): + template = """{% for item in chat_history %}{{ message(item) }}{% endfor %}""" + target = create_jinja2_prompt_template(template, allow_dangerously_set_content=True) + chat_history = ChatHistory() + chat_history.add_user_message('What does a < b & "c" mean?') + + rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) + + assert "<" in rendered + assert "&" in rendered + assert '"c"' in rendered + assert ChatHistory.from_rendered_prompt(rendered) == chat_history + + async def test_helpers_message_to_prompt(kernel: Kernel): template = """ {% for chat in chat_history %} diff --git a/python/tests/unit/prompt_template/test_jinja2_prompt_template_e2e.py b/python/tests/unit/prompt_template/test_jinja2_prompt_template_e2e.py index 9e057b706d70..54fbc541ac79 100644 --- a/python/tests/unit/prompt_template/test_jinja2_prompt_template_e2e.py +++ b/python/tests/unit/prompt_template/test_jinja2_prompt_template_e2e.py @@ -1,6 +1,7 @@ # Copyright (c) Microsoft. All rights reserved. +from semantic_kernel.contents import AuthorRole from semantic_kernel.contents.chat_history import ChatHistory from semantic_kernel.functions import kernel_function from semantic_kernel.functions.kernel_arguments import KernelArguments @@ -98,9 +99,81 @@ async def test_chat_history_round_trip(kernel: Kernel): chat_history.add_user_message("User message") chat_history.add_assistant_message("Assistant message") rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) - assert ( - rendered.strip() - == """User messageAssistant message""" + expected = ( + 'User message' + 'Assistant message' ) + assert rendered.strip() == expected chat_history2 = ChatHistory.from_rendered_prompt(rendered) assert chat_history2 == chat_history + + +async def test_from_rendered_prompt_backward_compat_old_format(kernel: Kernel): + """from_rendered_prompt handles the old format without wrapper for backward compatibility.""" + old_format = 'User messageAssistant message' + parsed = ChatHistory.from_rendered_prompt(old_format) + assert len(parsed) == 2 + assert parsed[0].role == AuthorRole.USER + assert parsed[0].content == "User message" + assert parsed[1].role == AuthorRole.ASSISTANT + assert parsed[1].content == "Assistant message" + + +async def test_chat_history_round_trip_with_xml_metacharacters(kernel: Kernel): + template = """{% for item in chat_history %}{{ message(item) }}{% endfor %}""" + target = create_jinja2_prompt_template(template) + chat_history = ChatHistory() + chat_history.add_user_message("What does a < b mean in Python?") + chat_history.add_assistant_message('Use "&" carefully in XML and HTML.') + + rendered = await target.render(kernel, KernelArguments(chat_history=chat_history)) + + assert "<" in rendered + assert "&" in rendered + assert '"&"' in rendered + assert ChatHistory.from_rendered_prompt(rendered) == chat_history + + +async def test_message_helper_preserves_system_role_with_xml_metacharacters(kernel: Kernel): + template = """{{system_message}}{% for item in chat_history %}{{ message(item) }}{% endfor %}""" + target = create_jinja2_prompt_template(template) + system_message = "You are a helpful assistant." + chat_history = ChatHistory() + chat_history.add_user_message("What does a < b mean in Python?") + + rendered = await target.render( + kernel, + KernelArguments(system_message=system_message, chat_history=chat_history), + ) + + parsed = ChatHistory.from_rendered_prompt(rendered) + assert parsed.messages[0].role == AuthorRole.SYSTEM + assert parsed.messages[0].content == system_message + assert parsed.messages[1].role == AuthorRole.USER + + assert parsed.messages[1].content == "What does a < b mean in Python?" + + +def test_from_rendered_prompt_backward_compat_old_format_no_text_wrapper(): + """from_rendered_prompt must handle the old format without wrapper.""" + old_format = 'User messageAssistant message' + parsed = ChatHistory.from_rendered_prompt(old_format) + assert len(parsed.messages) == 2 + assert parsed.messages[0].role == AuthorRole.USER + assert parsed.messages[0].content == "User message" + assert parsed.messages[1].role == AuthorRole.ASSISTANT + assert parsed.messages[1].content == "Assistant message" + + +def test_from_rendered_prompt_new_text_element_format(): + """from_rendered_prompt must handle the new format with wrapper.""" + new_format = ( + 'User message' + 'Assistant message' + ) + parsed = ChatHistory.from_rendered_prompt(new_format) + assert len(parsed.messages) == 2 + assert parsed.messages[0].role == AuthorRole.USER + assert parsed.messages[0].content == "User message" + assert parsed.messages[1].role == AuthorRole.ASSISTANT + assert parsed.messages[1].content == "Assistant message"