Fix session export 500 on multimodal/None message content (#1984)
txt/html/md export joined and string-munged message.content directly, so a multimodal turn (content is a list of blocks) crashed export with a TypeError on join (txt) / AttributeError on .replace (html), and None content (tool-only assistant turns) rendered as the literal 'None'. Add a _content_to_text helper that flattens string/list/None to plain text and apply it at the three export sites. JSON export is unchanged (it serializes structured content correctly). Plain-string content is returned unchanged, so existing exports are identical. Co-authored-by: ghreprimand <203024559+ghreprimand@users.noreply.github.com>
This commit is contained in:
@@ -37,6 +37,26 @@ def _public_model(name: str, model: str) -> str:
|
|||||||
return model
|
return model
|
||||||
|
|
||||||
|
|
||||||
|
def _content_to_text(content) -> str:
|
||||||
|
"""Flatten a message's content to plain text for text-based exports.
|
||||||
|
|
||||||
|
History entries carry three shapes: a plain string, a multimodal list of
|
||||||
|
content blocks (vision/image attachments), or None (assistant turns that
|
||||||
|
persisted only native tool_calls). The txt/html/md exporters join and
|
||||||
|
string-munge this value, so a list crashed the export (TypeError on join,
|
||||||
|
AttributeError on .replace) and None rendered as the literal "None".
|
||||||
|
Coerce to the text blocks, returning "" for anything without text.
|
||||||
|
"""
|
||||||
|
if isinstance(content, str):
|
||||||
|
return content
|
||||||
|
if isinstance(content, list):
|
||||||
|
return "\n".join(
|
||||||
|
b.get("text", "") for b in content
|
||||||
|
if isinstance(b, dict) and b.get("text")
|
||||||
|
)
|
||||||
|
return ""
|
||||||
|
|
||||||
|
|
||||||
def _verify_session_owner(request: Request, session_id: str, session_manager=None):
|
def _verify_session_owner(request: Request, session_id: str, session_manager=None):
|
||||||
"""Verify the current user owns the session. Raises 404 if not.
|
"""Verify the current user owns the session. Raises 404 if not.
|
||||||
|
|
||||||
@@ -708,7 +728,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
|||||||
lines = []
|
lines = []
|
||||||
for m in session.history:
|
for m in session.history:
|
||||||
lines.append(f"[{m.role.upper()}]")
|
lines.append(f"[{m.role.upper()}]")
|
||||||
lines.append(m.content)
|
lines.append(_content_to_text(m.content))
|
||||||
lines.append("")
|
lines.append("")
|
||||||
out_name = filename or f"conversation_{safe_name}_{timestamp}.txt"
|
out_name = filename or f"conversation_{safe_name}_{timestamp}.txt"
|
||||||
return Response(
|
return Response(
|
||||||
@@ -731,7 +751,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
|||||||
]
|
]
|
||||||
for m in session.history:
|
for m in session.history:
|
||||||
cls = "user" if m.role == "user" else "ai"
|
cls = "user" if m.role == "user" else "ai"
|
||||||
content = m.content.replace("&", "&").replace("<", "<").replace(">", ">")
|
content = _content_to_text(m.content).replace("&", "&").replace("<", "<").replace(">", ">")
|
||||||
content = content.replace("\n", "<br>")
|
content = content.replace("\n", "<br>")
|
||||||
html_parts.append(f'<div class="msg {cls}"><div class="role">{m.role}</div>{content}</div>')
|
html_parts.append(f'<div class="msg {cls}"><div class="role">{m.role}</div>{content}</div>')
|
||||||
html_parts.append("</body></html>")
|
html_parts.append("</body></html>")
|
||||||
@@ -750,7 +770,7 @@ def setup_session_routes(session_manager: SessionManager, config: dict, webhook_
|
|||||||
markdown_lines.append("\n---\n")
|
markdown_lines.append("\n---\n")
|
||||||
for message in session.history:
|
for message in session.history:
|
||||||
role = message.role.upper()
|
role = message.role.upper()
|
||||||
content = message.content
|
content = _content_to_text(message.content)
|
||||||
markdown_lines.append(f"### {role}")
|
markdown_lines.append(f"### {role}")
|
||||||
markdown_lines.append(f"{content}\n")
|
markdown_lines.append(f"{content}\n")
|
||||||
markdown_lines.append("---\n")
|
markdown_lines.append("---\n")
|
||||||
|
|||||||
50
tests/test_session_export_nonstring_content.py
Normal file
50
tests/test_session_export_nonstring_content.py
Normal file
@@ -0,0 +1,50 @@
|
|||||||
|
"""Regression: session export must tolerate non-string message content.
|
||||||
|
|
||||||
|
A message's ``content`` is a plain string for normal turns, but a multimodal
|
||||||
|
list of content blocks for image/vision turns, and ``None`` for assistant turns
|
||||||
|
that persisted only native tool_calls. The txt/html/md exporters in
|
||||||
|
``routes/session_routes.py`` joined and string-munged ``content`` directly, so:
|
||||||
|
|
||||||
|
- txt: ``"\n".join([..., <list>, ...])`` -> TypeError
|
||||||
|
- html: ``<list>.replace("&", "&")`` -> AttributeError
|
||||||
|
- md: ``f"{<list>}"`` -> raw Python repr in output
|
||||||
|
|
||||||
|
``_content_to_text`` coerces all three shapes to plain text so export degrades
|
||||||
|
gracefully instead of returning a 500.
|
||||||
|
"""
|
||||||
|
from routes.session_routes import _content_to_text
|
||||||
|
|
||||||
|
|
||||||
|
def test_plain_string_passes_through_unchanged():
|
||||||
|
assert _content_to_text("hello world") == "hello world"
|
||||||
|
assert _content_to_text("") == ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_multimodal_list_flattens_to_its_text_blocks():
|
||||||
|
content = [
|
||||||
|
{"type": "text", "text": "describe this"},
|
||||||
|
{"type": "image_url", "image_url": {"url": "data:image/png;base64,AAAA"}},
|
||||||
|
{"type": "text", "text": "thanks"},
|
||||||
|
]
|
||||||
|
assert _content_to_text(content) == "describe this\nthanks"
|
||||||
|
|
||||||
|
|
||||||
|
def test_none_content_becomes_empty_string():
|
||||||
|
# Assistant turns carrying only native tool_calls persist content as None.
|
||||||
|
assert _content_to_text(None) == ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_list_without_text_blocks_is_empty_not_crash():
|
||||||
|
assert _content_to_text([{"type": "image_url", "image_url": {"url": "x"}}]) == ""
|
||||||
|
assert _content_to_text([]) == ""
|
||||||
|
|
||||||
|
|
||||||
|
def test_coerced_output_survives_the_export_operations():
|
||||||
|
# The exact operations that previously crashed must now succeed.
|
||||||
|
history = ["plain", [{"type": "text", "text": "img turn"}], None]
|
||||||
|
texts = [_content_to_text(c) for c in history]
|
||||||
|
# txt export path
|
||||||
|
assert "\n".join(texts) == "plain\nimg turn\n"
|
||||||
|
# html export path
|
||||||
|
for t in texts:
|
||||||
|
assert isinstance(t.replace("&", "&"), str)
|
||||||
Reference in New Issue
Block a user