Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions src/benchflow/acp/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ def __init__(self, tool_call_id: str, title: str, kind: str):
self.kind = kind
self.status = ToolCallStatus.PENDING
self.content: list[dict] = []
# ACP ``rawInput`` / ``rawOutput``: the agent's own view of the call.
# codex-acp puts the command and its output here and nowhere else.
self.raw_input: object | None = None
self.raw_output: object | None = None
self.started_at = datetime.now()
self.finished_at: datetime | None = None

Expand All @@ -178,6 +182,13 @@ def update_status(
):
self.finished_at = datetime.now()

def absorb_raw_io(self, update: dict) -> None:
"""Keep the latest ``rawInput`` / ``rawOutput`` an update carries."""
if update.get("rawInput") is not None:
self.raw_input = update["rawInput"]
if update.get("rawOutput") is not None:
self.raw_output = update["rawOutput"]


class ACPSession:
"""Tracks mutable state for one ACP session.
Expand Down Expand Up @@ -417,6 +428,20 @@ def handle_update(self, update: dict) -> None:
update.get("kind", "other"), update.get("title", "")
),
)
# The opening notification may already carry content (codex-acp
# sends file-change diffs this way) and raw I/O.
initial_content = update.get("content")
if isinstance(initial_content, list) and initial_content:
record.content.extend(initial_content)
record.absorb_raw_io(update)
Comment on lines +433 to +436

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Completed file edits remain pending

When an opening tool_call reports a completed file edit, its status remains pending despite capturing its content. Timeout diagnostics then mark finished work incomplete.

Prompt for agents
The tool_call branch in src/benchflow/acp/session.py now captures opening content for codex-acp file-change events, but ToolCallRecord still starts as PENDING and this branch ignores update["status"]. Codex file-change notifications can be complete in this opening event without a later tool_call_update. Parse and apply a present opening status using the same unknown-status handling as the update branch, while preserving PENDING when no status is supplied. Add coverage that a completed opening-only file-change call is terminal and absent from pending_tool_call_ids().
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, fixed in 301c72f: the opening tool_call's status is applied with the same unknown-status handling as updates, PENDING stays the default when absent, and a test covers a completed opening-only edit leaving pending_tool_call_ids().

# An opening call may already be terminal (codex-acp file edits
# arrive completed with no later update); honor its status so it
# never lingers in pending_tool_call_ids().
if update.get("status") is not None:
try:
record.update_status(ToolCallStatus(update["status"]))
except ValueError:
logger.warning(f"Unknown tool call status: {update.get('status')}")
self._record_tool_call(record)

elif update_type == "tool_call_update":
Expand All @@ -440,6 +465,7 @@ def handle_update(self, update: dict) -> None:
status = ToolCallStatus.IN_PROGRESS
content = update.get("content")
record.update_status(status, content)
record.absorb_raw_io(update)
if status in (ToolCallStatus.PENDING, ToolCallStatus.IN_PROGRESS):
self._pending_tool_call_update_counts[tc_id] = (
self._pending_tool_call_update_counts.get(tc_id, 0) + 1
Expand Down
25 changes: 15 additions & 10 deletions src/benchflow/trajectories/_capture.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,21 @@ def _events_to_trajectory(events: list[dict]) -> list[dict]:
for event in events:
if event["type"] == "tool_call":
tc = event["record"]
out.append(
{
"type": "tool_call",
"tool_call_id": tc.tool_call_id,
"kind": tc.kind,
"title": tc.title,
"status": tc.status.value,
"content": tc.content,
}
)
record = {
"type": "tool_call",
"tool_call_id": tc.tool_call_id,
"kind": tc.kind,
"title": tc.title,
"status": tc.status.value,
"content": tc.content,
}
# Only present when the agent sent them, so trajectories of agents
# without raw I/O keep their exact shape.
if getattr(tc, "raw_input", None) is not None:
record["raw_input"] = tc.raw_input
if getattr(tc, "raw_output", None) is not None:
record["raw_output"] = tc.raw_output
out.append(record)
elif event["type"] in ("user_message", "agent_message", "agent_thought"):
out.append({"type": event["type"], "text": event["text"]})
elif event["type"] == "agent_timeout":
Expand Down
33 changes: 32 additions & 1 deletion src/benchflow/trajectories/viewer/payload.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,34 @@ def _tool_content_texts(content: Any) -> list[str]:
return texts


def _raw_io_texts(kind: str, raw_input: Any, raw_output: Any) -> list[str]:
"""Render ACP ``rawInput`` / ``rawOutput`` for a call without content blocks.

codex-acp reports a command as ``rawInput.command`` and its result as
``rawOutput.formatted_output`` plus ``exit_code``; anything else is shown
as JSON so no recorded detail is hidden.
"""
texts: list[str] = []
if isinstance(raw_input, dict) and kind == "execute" and "command" in raw_input:
command = raw_input["command"]
texts.append(
" ".join(str(part) for part in command)
if isinstance(command, list)
else _display_text(command)
)
elif raw_input is not None:
texts.append(dumps_finite(raw_input, ensure_ascii=False, default=str))
if isinstance(raw_output, dict) and "formatted_output" in raw_output:
output = _display_text(raw_output.get("formatted_output"))
exit_code = raw_output.get("exit_code")
if exit_code not in (None, 0):
output = f"{output.rstrip()}\n[exit code {exit_code}]"
texts.append(output)
elif raw_output is not None:
texts.append(dumps_finite(raw_output, ensure_ascii=False, default=str))
return [text for text in texts if text]


def _parse_ts(value: Any) -> float | None:
"""Parse finite epoch or ISO-8601 timestamps into epoch seconds."""
numeric = _finite_float(value)
Expand Down Expand Up @@ -319,7 +347,10 @@ def next_index() -> int:
kind=kind,
title=title,
status=normalize_tool_status(event.get("status")),
content=_tool_content_texts(event.get("content")),
content=_tool_content_texts(event.get("content"))
or _raw_io_texts(
kind, event.get("raw_input"), event.get("raw_output")
),
hue=tool_hue(kind, title),
),
t=started,
Expand Down
175 changes: 175 additions & 0 deletions tests/test_tool_call_raw_io.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
"""Tool-call content and raw I/O capture (issue #1099).

codex-acp reports a command as ``rawInput`` on the opening ``tool_call``, its
output as ``rawOutput`` on the completing ``tool_call_update``, and file-change
diffs as ``content`` on the opening ``tool_call``. Before the fix every one of
those was dropped and the trajectory kept only the title.
"""

from __future__ import annotations

import json
from pathlib import Path

from benchflow.acp.session import ACPSession
from benchflow.trajectories._capture import _events_to_trajectory
from benchflow.trajectories.viewer.payload import _build_acp_payload, _raw_io_texts


def _codex_exec_session() -> ACPSession:
session = ACPSession("s")
session.handle_update(
{
"sessionUpdate": "tool_call",
"toolCallId": "call_1",
"title": "ls /root",
"kind": "execute",
"status": "in_progress",
"rawInput": {"command": "ls /root", "cwd": "/root"},
}
)
session.handle_update(
{
"sessionUpdate": "tool_call_update",
"toolCallId": "call_1",
"status": "completed",
"rawOutput": {"formatted_output": "a.py\nb.py\n", "exit_code": 0},
}
)
return session


def test_session_keeps_raw_input_and_output() -> None:
"""Guards the fix for issue #1099: rawInput from the opening tool_call and
rawOutput from the completing update both survive on the record."""
record = _codex_exec_session().tool_calls[0]
assert record.raw_input == {"command": "ls /root", "cwd": "/root"}
assert record.raw_output == {"formatted_output": "a.py\nb.py\n", "exit_code": 0}


def test_session_keeps_content_sent_on_the_opening_tool_call() -> None:
"""Guards the fix for issue #1099: codex-acp file-change diffs arrive as
content on the opening tool_call, not on an update."""
session = ACPSession("s")
diff = {"type": "diff", "path": "/root/a.py", "oldText": None, "newText": "x = 1\n"}
session.handle_update(
{
"sessionUpdate": "tool_call",
"toolCallId": "call_2",
"title": "Editing files",
"kind": "edit",
"status": "completed",
"content": [diff],
}
)
assert session.tool_calls[0].content == [diff]


def test_opening_call_status_is_honored() -> None:
"""Guards the follow-up on #1099 review: a file edit that codex-acp reports
completed in its opening tool_call (no later update) is terminal, so it
never lingers in pending_tool_call_ids(); an opening call without a status
stays pending as before."""
session = ACPSession("s")
session.handle_update(
{
"sessionUpdate": "tool_call",
"toolCallId": "done",
"title": "Editing files",
"kind": "edit",
"status": "completed",
"content": [
{"type": "diff", "path": "/a", "oldText": None, "newText": "x"}
],
}
)
session.handle_update(
{
"sessionUpdate": "tool_call",
"toolCallId": "open",
"title": "ls",
"kind": "execute",
}
)
session.handle_update(
{
"sessionUpdate": "tool_call",
"toolCallId": "odd",
"title": "t",
"kind": "other",
"status": "not-a-status",
}
)
assert session.pending_tool_call_ids() == ["open", "odd"]
assert session.tool_calls[0].finished_at is not None


def test_trajectory_emits_raw_fields_only_when_present() -> None:
"""Guards the fix for issue #1099 without changing the shape agents that
send no raw I/O produce: the keys appear only when set."""
with_raw = _events_to_trajectory(_codex_exec_session().events)[0]
assert with_raw["raw_input"] == {"command": "ls /root", "cwd": "/root"}
assert with_raw["raw_output"]["exit_code"] == 0

bare = ACPSession("s")
bare.handle_update(
{"sessionUpdate": "tool_call", "toolCallId": "t", "title": "t", "kind": "read"}
)
assert set(_events_to_trajectory(bare.events)[0]) == {
"type",
"tool_call_id",
"kind",
"title",
"status",
"content",
}


def test_raw_io_texts_render_commands_outputs_and_exit_codes() -> None:
"""Execute calls read as command then output; a nonzero exit is stated;
other shapes fall back to JSON so nothing recorded is hidden."""
texts = _raw_io_texts(
"execute",
{"command": ["python", "-c", "print(1)"], "cwd": "/"},
{"formatted_output": "boom\n", "exit_code": 2},
)
assert texts == ["python -c print(1)", "boom\n[exit code 2]"]
assert _raw_io_texts("other", {"server": "mcp", "tool": "t"}, None) == [
'{"server": "mcp", "tool": "t"}'
]
assert _raw_io_texts("execute", None, None) == []


def test_payload_falls_back_to_raw_io_when_content_is_empty(tmp_path: Path) -> None:
"""Guards the viewer side of issue #1099: a codex-style tool call with no
content blocks still renders its command and output; content wins when
both exist."""
traj = tmp_path / "trajectory"
traj.mkdir()
events = [
{
"type": "tool_call",
"tool_call_id": "c1",
"kind": "execute",
"title": "ls /root",
"status": "completed",
"content": [],
"raw_input": {"command": "ls /root", "cwd": "/root"},
"raw_output": {"formatted_output": "a.py\n", "exit_code": 0},
},
{
"type": "tool_call",
"tool_call_id": "c2",
"kind": "execute",
"title": "cat x",
"status": "completed",
"content": [{"type": "content", "content": {"type": "text", "text": "x"}}],
"raw_output": {"formatted_output": "ignored", "exit_code": 0},
},
]
(traj / "acp_trajectory.jsonl").write_text("\n".join(json.dumps(e) for e in events))
(tmp_path / "result.json").write_text("{}")
steps = _build_acp_payload(tmp_path, None).to_payload()["steps"]
tools = [s["tool"] for s in steps if s["kind"] == "tool"]
assert tools[0]["content"] == ["ls /root", "a.py\n"]
assert tools[1]["content"] == ["x"]
Loading