GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||
GeezerVison is showing github.com — feat(pydantic-ai): migrate onto unified harness surface (PR4) by declan-scale · Pull Request #415 · scaleapi/scale-agentex-python Conversation
|
| Filename | Overview |
|---|---|
| src/agentex/lib/adk/_modules/_pydantic_ai_turn.py | New file: PydanticAITurn wraps convert_pydantic_ai_to_agentex_events as a HarnessTurn; on_result callback captures usage after stream exhaustion; usage() is only valid after events are fully consumed (correctly documented single-pass contract). |
| src/agentex/lib/adk/_modules/_pydantic_ai_async.py | Reimplemented to ~10 lines delegating to UnifiedEmitter.auto_send_turn(PydanticAITurn); tool calls now flow through streaming contexts instead of adk.messages.create; trace_id=None intentionally deferred per design note. |
| src/agentex/lib/adk/_modules/_pydantic_ai_sync.py | Additive change: on_result optional callback added to convert_pydantic_ai_to_agentex_events for run-level usage capture; handles both sync and async callables via inspect.iscoroutine. |
| src/agentex/lib/adk/_modules/_pydantic_ai_tracing.py | Deprecation docstrings added; module-level # NOTE: inside triple-quoted docstring should use RST .. note:: directive instead. |
| examples/tutorials/10_async/00_base/harness_pydantic_ai/project/acp.py | New async tutorial: uses auto_send_turn with a tee_messages wrapper to capture pydantic-ai message history alongside usage; correctly saves history after auto_send_turn exhausts the stream. |
| tests/lib/adk/test_pydantic_ai_turn.py | New test: covers PydanticAITurn usage normalization, on_result callback timing, and HarnessTurn protocol conformance. |
| tests/lib/core/harness/test_harness_pydantic_ai_async.py | New integration test for async path: verifies tool request/response ordering, final_text accumulation, usage capture in TurnResult, and span derivation. |
| .github/workflows/harness-integration.yml | Activates the live-matrix job for pydantic-ai across sync/async/temporal channels with offline TestModel tests. |
src/agentex/lib/adk/_modules/_pydantic_ai_sync.py, line 257-261 (link)
This early continue makes the fallback in the ToolCallPartDelta branch unreachable. If a provider emits a tool-call delta before a PartStartEvent, message_index is missing, so the converter skips the event before it can synthesize the tool request from the delta. That drops the tool call and its argument stream for the provider edge this code is trying to handle. Please allocate an Agentex message index and emit a synthetic tool-request start when the first event for an index is a tool-call delta.
This is a comment left during a code review. Path: src/agentex/lib/adk/_modules/_pydantic_ai_sync.py Line: 257-261 Comment: **Delta-first tools drop** This early `continue` makes the fallback in the `ToolCallPartDelta` branch unreachable. If a provider emits a tool-call delta before a `PartStartEvent`, `message_index` is missing, so the converter skips the event before it can synthesize the tool request from the delta. That drops the tool call and its argument stream for the provider edge this code is trying to handle. Please allocate an Agentex message index and emit a synthetic tool-request start when the first event for an index is a tool-call delta. How can I resolve this? If you propose a fix, please make it concise.
src/agentex/lib/adk/_modules/_pydantic_ai_sync.py, line 280-299 (link)
Once tool_call_meta is initialized from PartStartEvent, later deltas cannot fill in missing metadata. If the start event has an empty tool_call_id or tool name and a later ToolCallPartDelta supplies tool_call_id or tool_name_delta, this branch keeps using the stale empty values. The emitted ToolRequestDelta then has blank identifiers, and the later tool response or span close cannot match the request. Please merge non-empty delta metadata into tool_call_meta before constructing the delta.
This is a comment left during a code review. Path: src/agentex/lib/adk/_modules/_pydantic_ai_sync.py Line: 280-299 Comment: **Tool metadata stays blank** Once `tool_call_meta` is initialized from `PartStartEvent`, later deltas cannot fill in missing metadata. If the start event has an empty `tool_call_id` or tool name and a later `ToolCallPartDelta` supplies `tool_call_id` or `tool_name_delta`, this branch keeps using the stale empty values. The emitted `ToolRequestDelta` then has blank identifiers, and the later tool response or span close cannot match the request. Please merge non-empty delta metadata into `tool_call_meta` before constructing the delta. How can I resolve this? If you propose a fix, please make it concise.
src/agentex/lib/core/harness/emitter.py, line 66 (link)
auto_send_turn returns stale empty usage — token counts always lost
usage=turn.usage() is evaluated eagerly as a keyword argument before auto_send iterates turn.events. PydanticAITurn only populates usage via the on_result callback during stream iteration.Supporting artifact from the T-Rex run
Repro output showing stale usage in TurnResult
src/agentex/lib/core/harness/emitter.py, line 66 (link)
auto_send_turn returns stale empty usage instead of captured token counts
auto_send_turn reads turn.usage() before the event stream is consumed.PydanticAITurn updates usage only after the terminal run-result event is consumed.TurnResult.usage keeps the initial empty usage instead of the captured token counts.result.usage had empty token fields while turn.usage() after iteration contained the expected values.Supporting artifact from the T-Rex run
Verbose output showing stale vs real usage values
src/agentex/lib/core/harness/emitter.py, line 66 (link)
auto_send_turn returns stale empty usage instead of captured token counts
auto_send_turn calls turn.usage() on line 66 before passing it to auto_send, but PydanticAITurn only populates usage when the terminal AgentRunResultEvent is consumed during event iteration. The returned TurnResult.usage always has None tokens and 0 LLM calls.usage=turn.usage() eagerly before auto_send consumes turn.events. Since PydanticAITurn._capture only fires when AgentRunResultEvent is yielded during iteration, the usage snapshot is taken too early.turn.usage() after event consumption: have auto_send call a usage callback after exhausting the event iterator, or change auto_send_turn to call auto_send without usage, then patch result.usage = turn.usage() after awaiting.Supporting artifact from the T-Rex run
Verbose output showing stale vs real usage values
src/agentex/lib/core/harness/emitter.py, line 61-67 (link)
PydanticAITurn fills its usage only after turn.events is fully consumed. This call reads turn.usage() before auto_send starts iterating the stream, so async Pydantic AI runs return a TurnResult with empty token fields even when the terminal result event contains real usage.
Supporting artifact from the T-Rex run
Repro output confirming the usage timing bug
Supporting artifact from the T-Rex run
Repro output confirming the bug
Ran code and verified through T-Rex
This is a comment left during a code review. Path: src/agentex/lib/core/harness/emitter.py Line: 61-67 Comment: **Usage is captured early** `PydanticAITurn` fills its usage only after `turn.events` is fully consumed. This call reads `turn.usage()` before `auto_send` starts iterating the stream, so async Pydantic AI runs return a `TurnResult` with empty token fields even when the terminal result event contains real usage. How can I resolve this? If you propose a fix, please make it concise.
Reviews (13): Last reviewed commit: "fix(pydantic-ai): drop removed coalesce_..." | Re-trigger Greptile
Sorry, something went wrong.
There was an error while loading. Please reload this page.
b4c53ca to
cae14d4
Compare
724120b to
6d0f0e8
Compare
There was an error while loading. Please reload this page.
|
@greptile review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
8cd851c to
2e820c7
Compare
2c4fc88 to
a0f4fd2
Compare
|
@greptile review |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
2e820c7 to
37421b6
Compare
a0f4fd2 to
d09515c
Compare
37421b6 to
df3461c
Compare
75df4e6 to
0919e0f
Compare
There was an error while loading. Please reload this page.
ccbd5cf to
e3fa1cc
Compare
e4f331d to
9e4f9af
Compare
|
@michael-chou359 any concerns here? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
c8c63d1 to
05120f3
Compare
9e4f9af to
193917d
Compare
05120f3 to
c9a907c
Compare
193917d to
dd87257
Compare
c9a907c to
a04bf5e
Compare
…or usage capture Adds an `on_result: Callable[[AgentRunResultEvent], Any] | None = None` parameter to `convert_pydantic_ai_to_agentex_events`. When set, the callback is invoked (sync or async) with the terminal `AgentRunResultEvent` that carries the run result and usage. Streaming output is unchanged. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…tests Strengthen backward-compat guarantees for the on_result callback: - test_streaming_output_unchanged_with_callback now asserts model_dump() equality per yielded pair, not just type, proving the callback does not alter streamed message content. - test_async_callback_is_awaited adds a real suspension point (await asyncio.sleep(0)) before its side effect, so the assertion only passes if the converter actually awaits the returned coroutine. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Adds PydanticAITurn, a HarnessTurn wrapping a pydantic-ai event stream, with pydantic_ai_usage_to_turn_usage mapping verified RunUsage fields (requests, input_tokens, output_tokens, cache_read_tokens, total_tokens) onto TurnUsage via defensive getattr; usage() populates after events exhaustion. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…ion + cover real usage accessor Pass getattr results straight through so a real 0 (e.g. a cache-hit with 0 output tokens) stays 0 while a MISSING attribute still degrades to None. Previously `x if x else None` coerced legitimate zeros to None. Adds tests for the 0->0 mapping, the missing-field->None defensive guarantee, and the real result.usage property accessor path the converter uses. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Adds TestCharacterizeWireShapeCurrent to lock the current wire-level delivery shape: text via streaming_task_message_context, tool messages via adk.messages.create. Serves as the before-snapshot for the UnifiedEmitter reimplementation that follows. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…edEmitter (default tracing) Replaces the hand-rolled event loop in _pydantic_ai_async.py with a three-line delegation to UnifiedEmitter.auto_send_turn: turn = PydanticAITurn(stream, model=None, tracing_handler=tracing_handler) emitter = UnifiedEmitter(task_id=task_id, trace_id=None, parent_span_id=None) return (await emitter.auto_send_turn(turn)).final_text Public signature unchanged: stream_pydantic_ai_events(stream, task_id, tracing_handler=None) -> str. Supporting changes: - _pydantic_ai_turn.py: add optional tracing_handler arg (threaded to convert_pydantic_ai_to_agentex_events); add _coalesce_tool_requests() which converts Start(tool_request)+deltas+Done into Full(tool_request) so auto_send receives tool messages in the shape it expects (Option A: no streaming of argument tokens in the async/temporal path). - auto_send.py: reset final_text_parts on Start(text) so multi-step runs return only the last text segment, matching stream_langgraph_events and the existing stream_pydantic_ai_events convention. Wire shape change (AGX1-373 accepted envelope change): Before: tool messages via adk.messages.create After: tool messages via streaming_task_message_context open+close pairs Logical content (tool_call_id, name, arguments, result) is identical; only the delivery channel changed. Test updates: all test assertions updated to the new delivery channel. Two tool_call_with_*_args tests updated to include PartDeltaEvent (the realistic pydantic-ai event sequence for streamed JSON args). Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…sync arg streaming); ref AGX1-377 PydanticAITurn.events feeds BOTH delivery channels (yield_turn for sync, auto_send_turn for async). Applying _coalesce_tool_requests unconditionally would deliver tool requests as a single Full with no ToolRequestDelta tokens, losing the sync converter's documented tool-call-argument token streaming (Task 4 routes the sync/HTTP path through emitter.yield_turn(PydanticAITurn(...))). - Add constructor param coalesce_tool_requests: bool = False. Default OFF means PydanticAITurn(...).events == bare convert_pydantic_ai_to_agentex_events output (Start+Delta+Done for tool calls, arg streaming preserved on yield/sync). - stream_pydantic_ai_events builds the Turn with coalesce_tool_requests=True, because the foundation auto_send currently DROPS tool requests delivered as Start+Delta+Done (AGX1-377). Comment cites AGX1-377 as a temporary workaround to be removed once auto_send handles the streamed tool-request shape natively. - Tests: default-off Turn yields a ToolRequestDelta for streamed args (matches bare converter); coalesce-on Turn yields a single Full(tool_request) with fully-accumulated args and no ToolRequestDelta. Async characterization test still passes (goes through coalesce=True). - Add parts-manager invariant comment to the two corrected async tests. auto_send.py is unchanged (final_text last-segment fix stays; AGX1-377 covers the Start+Delta+Done handling). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ing handler (docstring) - _pydantic_ai_sync.py: add "Recommended: unified surface" section to module docstring showing PydanticAITurn + UnifiedEmitter usage with automatic span derivation; bare converter docstring/code unchanged. - _pydantic_ai_tracing.py: deprecation notes (docstring-only) on module, AgentexPydanticAITracingHandler, and create_pydantic_ai_tracing_handler; no runtime warnings.warn so warnings-as-errors does not break callers; NOTE: comment explains the deferral rationale. - tests/lib/adk/test_pydantic_ai_sync_unified.py: 6 new tests covering the unified sync path: passthrough equality + tool/reasoning span derivation via _FakeTracing injection, no-trace-id no-op, tracer=False suppression. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Register 4 pydantic-ai conformance fixtures (text-only, single tool call, reasoning block, multi-step) that drive both yield_events and auto_send channels and assert logical-delivery + span-signal equivalence. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
… live-matrix rows Add 3 offline integration tests (TestModel + fake streaming/tracing, no API keys or live infra needed) that prove the unified harness surface is correctly wired for each delivery channel: - test_harness_pydantic_ai_sync.py — yield_turn path (12 tests): event ordering (tool_request before tool_response before text), accumulated text, Start/Done pairing, SpanDeriver wiring (OpenSpan/CloseSpan for tool calls on sync path). - test_harness_pydantic_ai_async.py — auto_send_turn path (13 tests): message ordering, ToolRequestContent/ToolResponseContent content verification, matching tool_call_ids, final_text, context open/close lifecycle; documents that span derivation is suppressed when coalesce_tool_requests=True (AGX1-377 note). - test_harness_pydantic_ai_temporal.py — TemporalAgent event_stream_handler path (12 tests + 1 intentional skip): drives TemporalAgent.run_stream_events offline, feeds into _fake_stream_pydantic_ai_events (PydanticAITurn + UnifiedEmitter with injected FakeStreaming), asserts same canonical message order; skip placeholder documents what requires live Temporal+Redis infra. Enable harness-integration.yml live-matrix job (was `if: false`) with a 3-way matrix over [sync, async, temporal], each running its test file via ./scripts/test. Add test file glob to PR path trigger so the workflow re-runs when tests change. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…ing the unified surface Add 3 minimal, deployable tutorial agent projects, each a tiny pydantic-ai agent with one get_weather(city) tool whose message handler goes through the unified harness surface (UnifiedEmitter + PydanticAITurn) EXPLICITLY: - examples/tutorials/00_sync/harness_pydantic_ai (s-harness-pydantic-ai) sync ACP: `async for ev in emitter.yield_turn(PydanticAITurn(stream, model=...))`. Unlike 040_pydantic_ai (bare converter), this gives the sync channel real unified-yield coverage (coalesce off → tool-call arg-token streaming + auto span derivation under the per-turn span). - examples/tutorials/10_async/00_base/harness_pydantic_ai (ab-harness-pydantic-ai) async ACP: `await emitter.auto_send_turn(PydanticAITurn(..., coalesce_tool_requests=True))` called directly (not via stream_pydantic_ai_events). Persists pydantic-ai message history via adk.state. - examples/tutorials/10_async/10_temporal/harness_pydantic_ai (at-harness-pydantic-ai) temporal: TemporalAgent event_stream_handler builds a UnifiedEmitter from RunContext.deps and calls auto_send_turn inside the model activity. Durable workflow + run_worker structured like the temporal-pydantic-ai template. Each UnifiedEmitter is constructed from the ACP/streaming context (task_id + trace_id + parent_span_id) so tracing is automatic. CI discovery: both agentex-tutorials-test.yml and build-and-push-tutorial-agent.yml discover agents dynamically via `find examples/tutorials -name manifest.yaml`, so the 3 agents are picked up with no workflow edits. Directory placement keeps the build-and-push ACP-type inference (`*10_async*` → async) correct: sync under 00_sync, async/temporal under 10_async. Each ships tests/test_agent.py (required by the build validator) as the live integration test. Verified structurally: all 3 manifests parse; `from project.acp import acp` imports cleanly for all 3 under CI-style env; temporal agent/workflow/run_worker import; the sync handler driven offline with TestModel emits the expected tool_request → tool_response → text sequence through yield_turn. Keeps the 3 offline integration tests and the harness-integration.yml live-matrix from the previous commit. tests/lib/core/harness + tests/lib/adk: 230 passed, 1 skipped. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Fix 22 pyright errors introduced in PR 4's new test files: - isinstance narrowing before union-member attribute access (ToolRequestDelta.arguments_delta, TextDelta.text_delta, ToolResponseContent.content, FunctionToolResultEvent.part.content) - reportReturnType in _run_yield_turn: hoist result variable out of async-with - reportImplicitOverride on _RecordingTracer.handle: add @OverRide - reportMissingImports in conformance tests: switch absolute tests.lib... imports to relative .runner imports so pyright's executionEnvironments root matches All 230 tests pass on 3.12 and 3.13. Ruff: clean. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Folds the plan doc (previously the separate #413) into this PR so plan + implementation land together. Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
…ation auto_send delivers streamed tool requests natively (AGX1-377/378) Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
…rials [greptile] PydanticAITurn.__init__ accepts only (stream, model, tracing_handler); the coalesce_tool_requests workaround was removed now that the unified auto_send path delivers streamed tool requests natively (Start+Delta+Done). Two tutorial agents still passed coalesce_tool_requests=True, which would raise TypeError at runtime (P0): - 10_async/00_base/harness_pydantic_ai (acp.py) - 10_async/10_temporal/harness_pydantic_ai (agent.py) Also refresh the now-stale references to the removed parameter in the sync tutorial comment and the cross-channel conformance test docstring (AGX1-377 has landed; streamed tool requests are delivered and asserted). Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
dd87257 to
3745c56
Compare
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.