Repository navigation
Python: fix(python): keep ChatMessageContent.to_prompt a str when encoding is set - #14464
Chen Yufeiyang (feiiiiii5) wants to merge 2 commits into
Conversation
… set to_prompt passed encoding=self.encoding or "unicode" to ElementTree.tostring, so any message carrying an encoding returned bytes with an XML declaration instead of the prompt string, while every other tostring call site in the contents package hardcodes "unicode". jinja2/handlebars _message_to_prompt wrap the result in str(), which then embeds a b'...' literal in the rendered prompt.
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The fix is focused, tested across supported encodings, and has no unresolved blocking issues.
Review effort: Lite
Findings: None
What changed in this PR
Fixes ChatMessageContent.to_prompt() so it always returns a string when an encoding is set.
Changes:
- Forces Unicode XML serialization.
- Documents encoding behavior.
- Adds regression tests for UTF-8, ASCII, and Latin-1.
| File | Description |
|---|---|
python/semantic_kernel/contents/chat_message_content.py |
Ensures prompt serialization returns str. |
python/tests/unit/contents/test_chat_message_content.py |
Adds encoding-specific regression coverage. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Following up — open since 20 September with no review.
root = self.to_element()
return ElementTree.tostring(root, encoding=self.encoding or "unicode", short_empty_elements=False)
I coerce to Nothing else is outstanding from me. If you would rather this went to an issue first, say so and I will move it. |
|
Hi Microsoft (@microsoft) maintainers — #14464 has been quiet since 1 October, so this is the one nudge I planned. Recap: CI note: the three red checks are all automation gates that behave this way on fork PRs — Is there anything you'd like changed before this can go in? |
What this fixes
ChatMessageContent.to_prompt()is annotated-> strbut returnsbytesfor any message that carries anencoding.encodingis a real field on the content, so theornever protects it: settingencoding="utf-8"makestostringtake its bytes branch. Measured onca40aa72:This is user-visible rather than merely untidy:
jinja2_system_helpers._message_to_promptand its handlebars twin render a context variable viastr(context.to_prompt()), so the prompt the model receives literally containsb'<message role="user" ...>'— bytes repr plus quote noise — whenever the message had an encoding set.Every other
tostringcall site in the same package already hardcodesencoding="unicode":chat_history.py:285,:317,:362,chat_message_content.py:275,handlebars_system_helpers.py:37,:55andjinja2_system_helpers.py:29. This one line was the outlier, so the change aligns it rather than introducing a new convention.The message's own encoding is still preserved: it is an attribute on the element and round-trips through
to_element/from_element; it describes the text content, not how we serialize a prompt string.Change
One line, plus a docstring note, and a parametrized regression test over
utf-8,asciiandlatin-1asserting the prompt is astr, has no XML declaration, keeps the non-ASCII text, and re-parses to the same content.How this was verified
cd /private/tmp/sk/tree/python,PYTHONPATH=., the repo's own test filetests/unit/contents/test_chat_message_content.py.Base control —
semantic_kernel/contents/chat_message_content.pyrestored fromca40aa7226531d28a721d0ca0e451d0aaf86dafcwithgit diff -- <that file>printing nothing (so the run measures upstream code with the new tests kept), sha re-echoed:With the patch applied:
46 passed.Enclosing directory, both revisions, same command (
pytest tests/unit/contents): thetest_chat_history.py::test_to_from_file/::test_chat_history_serializecollection errors appear identically at base and at head — they come from optional extras missing in my environment, not from this diff. I did not run the integration or dotnet suites.ruff checkon the two files reports only a pre-existingDfinding atchat_message_content.py:201, present at base too;ruff format --checkis clean. (mypywas not installed in this environment, so the CI type gate is unverified here.)Dedup: searched issues and PRs (all states) for
to_prompt,message_to_promptandencoding unicode prompt; the only hits are merged history (#13738, #5550). The nearbydata_uriMIME-subtype defect I found while investigating is already claimed by open #14304, so it is deliberately not part of this PR.