Skip to content

Commit acb16a3

Browse files
are-cesclaude
andcommitted
LCORE-2914: address PR review feedback for A2A executor refactor
- Fix conversation_id not passed to _convert_stream_to_events (regression) - Use Optional[Type] per contributing guide conventions - Merge doubled pydantic_ai.messages import, alias A2A TextPart instead - Use tool_call_id + tool_name in tool call status events - Rename mismatched test, add AgentRunError and compacted turn tests - Fix stream mock to yield AgentRunResultEvent instead of setting .result - Add test asserting artifact carries conversation_id Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 29bd62d commit acb16a3

2 files changed

Lines changed: 24 additions & 22 deletions

File tree

src/app/endpoints/a2a.py

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727
TaskState,
2828
TaskStatus,
2929
TaskStatusUpdateEvent,
30-
TextPart,
30+
)
31+
from a2a.types import (
32+
TextPart as A2ATextPart,
3133
)
3234
from a2a.utils import new_agent_text_message, new_task
3335
from fastapi import APIRouter, Depends, HTTPException, Request, status
@@ -41,11 +43,9 @@
4143
PartDeltaEvent,
4244
PartEndEvent,
4345
PartStartEvent,
46+
TextPart,
4447
TextPartDelta,
4548
)
46-
from pydantic_ai.messages import (
47-
TextPart as PydanticTextPart,
48-
)
4949
from pydantic_ai.run import AgentRunResult
5050
from starlette.responses import Response, StreamingResponse
5151

@@ -80,8 +80,8 @@
8080
# Task store and context store are created lazily based on configuration.
8181
# For multi-worker deployments, configure 'a2a_state' with 'sqlite' or 'postgres'
8282
# to share state across workers.
83-
_TASK_STORE: TaskStore | None = None
84-
_CONTEXT_STORE: A2AContextStore | None = None
83+
_TASK_STORE: Optional[TaskStore] = None
84+
_CONTEXT_STORE: Optional[A2AContextStore] = None
8585

8686

8787
async def _get_task_store() -> TaskStore:
@@ -111,7 +111,7 @@ async def _get_context_store() -> A2AContextStore:
111111

112112

113113
def _build_a2a_parts_from_agent_result(
114-
run_result: AgentRunResult[str] | None,
114+
run_result: Optional[AgentRunResult[str]],
115115
accumulated_text: list[str],
116116
) -> list[Part]:
117117
"""Convert a pydantic-ai agent run result to A2A Parts.
@@ -133,7 +133,7 @@ def _build_a2a_parts_from_agent_result(
133133
final_text = "".join(accumulated_text)
134134
if not final_text:
135135
return []
136-
return [Part(root=TextPart(text=final_text))]
136+
return [Part(root=A2ATextPart(text=final_text))]
137137

138138

139139
class TaskResultAggregator:
@@ -467,7 +467,7 @@ async def _convert_stream_to_events(
467467
prompt: str,
468468
task_id: str,
469469
context_id: str,
470-
conversation_id: str | None = None,
470+
conversation_id: Optional[str] = None,
471471
) -> AsyncIterator[Any]:
472472
"""Run an agent and convert stream events to A2A events.
473473
@@ -486,7 +486,7 @@ async def _convert_stream_to_events(
486486

487487
artifact_id = str(uuid.uuid4())
488488
text_parts: list[str] = []
489-
run_result: AgentRunResult[str] | None = None
489+
run_result: Optional[AgentRunResult[str]] = None
490490

491491
async with agent.run_stream_events(prompt) as stream:
492492
async for event in stream:
@@ -520,7 +520,7 @@ def _dispatch_agent_event( # pylint: disable=too-many-arguments,too-many-positi
520520
context_id: str,
521521
text_parts: list[str],
522522
artifact_id: str,
523-
) -> TaskStatusUpdateEvent | None:
523+
) -> Optional[TaskStatusUpdateEvent]:
524524
"""Map a single pydantic-ai stream event to an A2A status update.
525525
526526
Parameters:
@@ -536,7 +536,7 @@ def _dispatch_agent_event( # pylint: disable=too-many-arguments,too-many-positi
536536
_ = artifact_id
537537

538538
if isinstance(event, PartStartEvent):
539-
if isinstance(event.part, PydanticTextPart):
539+
if isinstance(event.part, TextPart):
540540
text_parts.append(event.part.content)
541541
return self._text_status_event(event.part.content, task_id, context_id)
542542

@@ -553,7 +553,7 @@ def _dispatch_agent_event( # pylint: disable=too-many-arguments,too-many-positi
553553
status=TaskStatus(
554554
state=TaskState.working,
555555
message=new_agent_text_message(
556-
f"Tool call: {event.part.tool_name}",
556+
f"Tool call: {event.part.tool_call_id} ({event.part.tool_name})",
557557
context_id=context_id,
558558
task_id=task_id,
559559
),
@@ -570,7 +570,7 @@ def _dispatch_agent_event( # pylint: disable=too-many-arguments,too-many-positi
570570
status=TaskStatus(
571571
state=TaskState.working,
572572
message=new_agent_text_message(
573-
f"MCP call: {event.part.tool_name}",
573+
f"Tool call: {event.part.tool_call_id} ({event.part.tool_name})",
574574
context_id=context_id,
575575
task_id=task_id,
576576
),

tests/unit/app/endpoints/test_a2a.py

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -965,7 +965,7 @@ async def test_process_task_streaming_applies_compaction( # pylint: disable=too
965965
result_event = mocker.MagicMock(spec=AgentRunResultEvent)
966966
result_event.result = mock_run_result
967967

968-
async def _event_stream():
968+
async def _event_stream() -> Any:
969969
yield result_event
970970

971971
mock_stream_ctx = mocker.AsyncMock()
@@ -996,7 +996,7 @@ async def test_convert_stream_artifact_carries_conversation_id(
996996
result_event = mocker.MagicMock(spec=AgentRunResultEvent)
997997
result_event.result = mock_run_result
998998

999-
async def _event_stream():
999+
async def _event_stream() -> Any:
10001000
yield result_event
10011001

10021002
mock_stream_ctx = mocker.AsyncMock()
@@ -1015,9 +1015,9 @@ async def _event_stream():
10151015

10161016
artifact_events = [e for e in events if isinstance(e, TaskArtifactUpdateEvent)]
10171017
assert len(artifact_events) == 1
1018-
assert (
1019-
artifact_events[0].artifact.metadata["conversation_id"] == conversation_id
1020-
)
1018+
metadata = artifact_events[0].artifact.metadata
1019+
assert metadata is not None
1020+
assert metadata["conversation_id"] == conversation_id
10211021

10221022
@pytest.mark.asyncio
10231023
async def test_cancel_raises_not_implemented(self, mocker: MockerFixture) -> None:
@@ -1127,6 +1127,7 @@ def test_function_tool_call_emits_status_update(
11271127
text_parts: list[str] = []
11281128
tool_call_part = mocker.MagicMock(spec=ToolCallPart)
11291129
tool_call_part.tool_name = "search_docs"
1130+
tool_call_part.tool_call_id = "call_xyz789"
11301131
event = FunctionToolCallEvent(
11311132
part=tool_call_part,
11321133
event_kind="function_tool_call",
@@ -1136,7 +1137,7 @@ def test_function_tool_call_emits_status_update(
11361137
)
11371138
assert result is not None
11381139
assert isinstance(result, TaskStatusUpdateEvent)
1139-
assert "Tool call: search_docs" in str(result.status.message)
1140+
assert "Tool call: call_xyz789 (search_docs)" in str(result.status.message)
11401141

11411142
def test_native_tool_call_end_emits_status_update(
11421143
self, mocker: MockerFixture
@@ -1145,7 +1146,8 @@ def test_native_tool_call_end_emits_status_update(
11451146
executor = self._make_executor()
11461147
text_parts: list[str] = []
11471148
native_part = mocker.MagicMock(spec=NativeToolCallPart)
1148-
native_part.tool_name = "mcp_server_tool"
1149+
native_part.tool_name = "file_search"
1150+
native_part.tool_call_id = "call_abc123"
11491151
event = PartEndEvent(
11501152
index=0,
11511153
part=native_part,
@@ -1156,7 +1158,7 @@ def test_native_tool_call_end_emits_status_update(
11561158
)
11571159
assert result is not None
11581160
assert isinstance(result, TaskStatusUpdateEvent)
1159-
assert "MCP call: mcp_server_tool" in str(result.status.message)
1161+
assert "Tool call: call_abc123 (file_search)" in str(result.status.message)
11601162

11611163
def test_agent_run_result_returns_none(self, mocker: MockerFixture) -> None:
11621164
"""Test that AgentRunResultEvent is not mapped to a status update."""

0 commit comments

Comments
 (0)