feat: v0.4.1 — ChatGPT tool-output content types and conv_id fix
First real-data export against v0.4.0 surfaced 66 unknown blocks across three content types — captured live and added. Added: - execution_output (Code Interpreter / container.exec / python tool output) → tool_result block. output=content.text, tool_name=author.name, is_error=metadata.aggregate_result.status, summary=metadata.reasoning_title - system_error → error tool_result with tool_name=author.name - tether_browsing_display: spinner placeholders (empty result+summary) skip silently with DEBUG log; defensive populated-case branch maps to tool_result (untested in real data) - tool_result block schema: optional `summary` field rendered as italic line between header and fence - tool_result rendering: tool_name appears in header when present (e.g. `📤 Result: container.exec`); existing tool_name=None calls unchanged - _ROLE_LABELS["tool"] = ("🔧 Tool", "tool") Fixed: - chatgpt.normalize_conversation reads `conversation_id` as fallback for `id`. Live API uses conversation_id; fixtures use id. Pre-fix: empty id in YAML frontmatter and missing context in WARNING logs. Tests: 11 new (192 total, 0 failures). Fixture extended with 4 tool-output cases (execution_output success, empty execution_output that should skip, system_error, tether_browsing_display spinner). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
JesseMarkowitz
69
tests/fixtures/chatgpt_conversation.json
vendored
69
tests/fixtures/chatgpt_conversation.json
vendored
@@ -112,7 +112,7 @@
|
||||
"node-image-only": {
|
||||
"id": "node-image-only",
|
||||
"parent": "node-mm-user-rev",
|
||||
"children": [],
|
||||
"children": ["node-exec-output"],
|
||||
"message": {
|
||||
"id": "node-image-only",
|
||||
"author": {"role": "user"},
|
||||
@@ -124,6 +124,73 @@
|
||||
]
|
||||
}
|
||||
}
|
||||
},
|
||||
"node-exec-output": {
|
||||
"id": "node-exec-output",
|
||||
"parent": "node-image-only",
|
||||
"children": ["node-exec-output-empty"],
|
||||
"message": {
|
||||
"id": "node-exec-output",
|
||||
"author": {"role": "tool", "name": "container.exec", "metadata": {}},
|
||||
"create_time": 1704067600.0,
|
||||
"content": {
|
||||
"content_type": "execution_output",
|
||||
"text": "Hello from container.exec\nLine 2 of output"
|
||||
},
|
||||
"metadata": {
|
||||
"aggregate_result": {"status": "success", "messages": []},
|
||||
"reasoning_title": "Reading skill documentation"
|
||||
}
|
||||
}
|
||||
},
|
||||
"node-exec-output-empty": {
|
||||
"id": "node-exec-output-empty",
|
||||
"parent": "node-exec-output",
|
||||
"children": ["node-system-error"],
|
||||
"message": {
|
||||
"id": "node-exec-output-empty",
|
||||
"author": {"role": "tool", "name": "python", "metadata": {}},
|
||||
"create_time": 1704067610.0,
|
||||
"content": {
|
||||
"content_type": "execution_output",
|
||||
"text": ""
|
||||
},
|
||||
"metadata": {}
|
||||
}
|
||||
},
|
||||
"node-system-error": {
|
||||
"id": "node-system-error",
|
||||
"parent": "node-exec-output-empty",
|
||||
"children": ["node-tether-spinner"],
|
||||
"message": {
|
||||
"id": "node-system-error",
|
||||
"author": {"role": "tool", "name": "web", "metadata": {}},
|
||||
"create_time": 1704067620.0,
|
||||
"content": {
|
||||
"content_type": "system_error",
|
||||
"name": "tool_error",
|
||||
"text": "Error: Error from browse service: Error calling browse service: 503"
|
||||
},
|
||||
"metadata": {}
|
||||
}
|
||||
},
|
||||
"node-tether-spinner": {
|
||||
"id": "node-tether-spinner",
|
||||
"parent": "node-system-error",
|
||||
"children": [],
|
||||
"message": {
|
||||
"id": "node-tether-spinner",
|
||||
"author": {"role": "tool", "name": "file_search", "metadata": {}},
|
||||
"create_time": 1704067630.0,
|
||||
"content": {
|
||||
"content_type": "tether_browsing_display",
|
||||
"result": "",
|
||||
"summary": "",
|
||||
"assets": null,
|
||||
"tether_id": null
|
||||
},
|
||||
"metadata": {"command": "spinner", "status": "running"}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -371,6 +371,37 @@ class TestRenderBlocks:
|
||||
assert "❌ **Result (error)**" in out
|
||||
assert "📤" not in out
|
||||
|
||||
def test_tool_result_with_tool_name_in_header(self):
|
||||
out = render_blocks_to_markdown(
|
||||
[make_tool_result_block("done", tool_name="container.exec")]
|
||||
)
|
||||
assert "📤 **Result: container.exec**" in out
|
||||
|
||||
def test_tool_result_error_with_tool_name(self):
|
||||
out = render_blocks_to_markdown(
|
||||
[make_tool_result_block("503", tool_name="web", is_error=True)]
|
||||
)
|
||||
assert "❌ **Result (error): web**" in out
|
||||
|
||||
def test_tool_result_summary_renders_as_italic_line(self):
|
||||
out = render_blocks_to_markdown(
|
||||
[
|
||||
make_tool_result_block(
|
||||
"output",
|
||||
tool_name="container.exec",
|
||||
summary="Reading skill documentation",
|
||||
)
|
||||
]
|
||||
)
|
||||
# Summary line is italic, lives between header and fence,
|
||||
# all inside the blockquote prefix.
|
||||
assert "> *Reading skill documentation*" in out
|
||||
# Order: header before summary before fence
|
||||
header_idx = out.index("Result: container.exec")
|
||||
summary_idx = out.index("Reading skill documentation")
|
||||
fence_idx = out.index("output")
|
||||
assert header_idx < summary_idx < fence_idx
|
||||
|
||||
def test_image_placeholder_rendering(self):
|
||||
out = render_blocks_to_markdown(
|
||||
[make_image_placeholder(ref="file-123", source="user_upload")]
|
||||
|
||||
@@ -15,6 +15,7 @@ from src.blocks import (
|
||||
BLOCK_TYPE_TOOL_RESULT,
|
||||
BLOCK_TYPE_TOOL_USE,
|
||||
BLOCK_TYPE_UNKNOWN,
|
||||
render_blocks_to_markdown,
|
||||
)
|
||||
from src.loss_report import LossReport
|
||||
|
||||
@@ -462,3 +463,181 @@ class TestClaudeNormalization:
|
||||
report = LossReport()
|
||||
result = p.normalize_conversation(raw, report)
|
||||
assert report.messages_rendered == len(result["messages"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# v0.4.1 — execution_output, system_error, tether_browsing_display, conv_id
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestChatGPTToolOutputs:
|
||||
"""v0.4.1 ChatGPT tool-role content_types map onto tool_result blocks."""
|
||||
|
||||
def _get_provider(self):
|
||||
from src.providers.chatgpt import ChatGPTProvider
|
||||
p = ChatGPTProvider.__new__(ChatGPTProvider)
|
||||
import requests
|
||||
p._session = requests.Session()
|
||||
p._org_id = None
|
||||
p._project_ids = []
|
||||
p._project_map = {}
|
||||
p._project_name_cache = {}
|
||||
return p
|
||||
|
||||
def test_execution_output_emits_tool_result_with_metadata(self):
|
||||
raw = json.loads((FIXTURES / "chatgpt_conversation.json").read_text())
|
||||
p = self._get_provider()
|
||||
result = p.normalize_conversation(raw)
|
||||
|
||||
exec_msgs = [
|
||||
m for m in result["messages"]
|
||||
if any(
|
||||
b.get("type") == BLOCK_TYPE_TOOL_RESULT
|
||||
and b.get("tool_name") == "container.exec"
|
||||
for b in (m.get("blocks") or [])
|
||||
)
|
||||
]
|
||||
assert exec_msgs, "expected execution_output to render as tool_result"
|
||||
block = next(
|
||||
b for b in exec_msgs[0]["blocks"] if b.get("type") == BLOCK_TYPE_TOOL_RESULT
|
||||
)
|
||||
assert block["output"].startswith("Hello from container.exec")
|
||||
assert block["is_error"] is False
|
||||
assert block["summary"] == "Reading skill documentation"
|
||||
|
||||
def test_execution_output_message_role_is_tool(self):
|
||||
raw = json.loads((FIXTURES / "chatgpt_conversation.json").read_text())
|
||||
p = self._get_provider()
|
||||
result = p.normalize_conversation(raw)
|
||||
tool_msgs = [m for m in result["messages"] if m["role"] == "tool"]
|
||||
assert tool_msgs, "tool-role messages must pass through (filter lifted in v0.4.0)"
|
||||
|
||||
def test_empty_execution_output_skipped(self, caplog):
|
||||
raw = json.loads((FIXTURES / "chatgpt_conversation.json").read_text())
|
||||
p = self._get_provider()
|
||||
with caplog.at_level(logging.DEBUG, logger="src.providers.chatgpt"):
|
||||
result = p.normalize_conversation(raw)
|
||||
|
||||
# The empty execution_output (author.name="python") must NOT appear.
|
||||
python_msgs = [
|
||||
m for m in result["messages"]
|
||||
if any(
|
||||
b.get("type") == BLOCK_TYPE_TOOL_RESULT and b.get("tool_name") == "python"
|
||||
for b in (m.get("blocks") or [])
|
||||
)
|
||||
]
|
||||
assert not python_msgs, "empty execution_output should be skipped"
|
||||
assert any("Skipping empty execution_output" in r.message for r in caplog.records)
|
||||
|
||||
def test_system_error_emits_error_tool_result(self):
|
||||
raw = json.loads((FIXTURES / "chatgpt_conversation.json").read_text())
|
||||
p = self._get_provider()
|
||||
result = p.normalize_conversation(raw)
|
||||
|
||||
web_err = [
|
||||
m for m in result["messages"]
|
||||
if any(
|
||||
b.get("type") == BLOCK_TYPE_TOOL_RESULT
|
||||
and b.get("tool_name") == "web"
|
||||
and b.get("is_error") is True
|
||||
for b in (m.get("blocks") or [])
|
||||
)
|
||||
]
|
||||
assert web_err, "system_error should render as tool_result with is_error=True"
|
||||
block = next(b for b in web_err[0]["blocks"] if b.get("tool_name") == "web")
|
||||
assert "503" in block["output"]
|
||||
|
||||
def test_tether_browsing_display_spinner_skipped(self, caplog):
|
||||
raw = json.loads((FIXTURES / "chatgpt_conversation.json").read_text())
|
||||
p = self._get_provider()
|
||||
with caplog.at_level(logging.DEBUG, logger="src.providers.chatgpt"):
|
||||
result = p.normalize_conversation(raw)
|
||||
|
||||
spinner_msgs = [
|
||||
m for m in result["messages"]
|
||||
if any(
|
||||
b.get("type") == BLOCK_TYPE_TOOL_RESULT and b.get("tool_name") == "file_search"
|
||||
for b in (m.get("blocks") or [])
|
||||
)
|
||||
]
|
||||
assert not spinner_msgs, "spinner tether_browsing_display should be skipped"
|
||||
assert any("tether_browsing_display spinner" in r.message for r in caplog.records)
|
||||
|
||||
def test_tether_browsing_display_populated_renders_defensively(self):
|
||||
"""Defensive case (never observed in real data) — populated browse renders."""
|
||||
conv = {
|
||||
"id": "test-tether",
|
||||
"title": "T",
|
||||
"create_time": 1700000000.0,
|
||||
"update_time": 1700000001.0,
|
||||
"mapping": {
|
||||
"root": {"id": "root", "message": None, "parent": None, "children": ["m1"]},
|
||||
"m1": {
|
||||
"id": "m1",
|
||||
"parent": "root",
|
||||
"children": [],
|
||||
"message": {
|
||||
"id": "m1",
|
||||
"author": {"role": "tool", "name": "browser"},
|
||||
"content": {
|
||||
"content_type": "tether_browsing_display",
|
||||
"result": "Found 3 results about kubernetes ingress.",
|
||||
"summary": "ingress search",
|
||||
"assets": None,
|
||||
"tether_id": None,
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
}
|
||||
p = self._get_provider()
|
||||
result = p.normalize_conversation(conv)
|
||||
assert any(
|
||||
b.get("type") == BLOCK_TYPE_TOOL_RESULT and b.get("tool_name") == "browser"
|
||||
for m in result["messages"]
|
||||
for b in (m.get("blocks") or [])
|
||||
)
|
||||
|
||||
|
||||
class TestChatGPTConvIdFallback:
|
||||
"""v0.4.1: live ChatGPT detail responses use conversation_id, not id."""
|
||||
|
||||
def _get_provider(self):
|
||||
from src.providers.chatgpt import ChatGPTProvider
|
||||
p = ChatGPTProvider.__new__(ChatGPTProvider)
|
||||
import requests
|
||||
p._session = requests.Session()
|
||||
p._org_id = None
|
||||
p._project_ids = []
|
||||
p._project_map = {}
|
||||
p._project_name_cache = {}
|
||||
return p
|
||||
|
||||
def test_falls_back_to_conversation_id(self):
|
||||
raw = {
|
||||
"conversation_id": "live-chatgpt-uuid",
|
||||
"title": "T",
|
||||
"create_time": 1700000000.0,
|
||||
"update_time": 1700000001.0,
|
||||
"mapping": {
|
||||
"root": {"id": "root", "message": None, "parent": None, "children": []},
|
||||
},
|
||||
}
|
||||
p = self._get_provider()
|
||||
result = p.normalize_conversation(raw)
|
||||
assert result["id"] == "live-chatgpt-uuid"
|
||||
|
||||
def test_id_takes_precedence_when_both_present(self):
|
||||
raw = {
|
||||
"id": "from-id",
|
||||
"conversation_id": "from-conversation-id",
|
||||
"title": "T",
|
||||
"create_time": 1700000000.0,
|
||||
"update_time": 1700000001.0,
|
||||
"mapping": {
|
||||
"root": {"id": "root", "message": None, "parent": None, "children": []},
|
||||
},
|
||||
}
|
||||
p = self._get_provider()
|
||||
result = p.normalize_conversation(raw)
|
||||
assert result["id"] == "from-id"
|
||||
|
||||
Reference in New Issue
Block a user