Skip to content

Commit 6bca797

Browse files
aditik0303claude
andcommitted
fix(pydantic-ai): address remaining review minors (page 4)
Follow-up to the #359 review pass — the earlier commit covered the blockers + page-3 majors; these are the page-4 minors: - model.py:239 — BuiltinToolReturnPart now fires AFTER_TOOL (symmetric with the built-in TOOL_CALL). A provider-executed built-in tool carries both its call and result inline in the response, so AFTER_TOOL for built-ins fires from on_response, not the next request. - model.py:371 — _coerce_args preserves malformed JSON as {"_raw": args} instead of dropping it to {}, so an arg-based policy can still scan it (a malformed payload must not slip past governance). - model.py:229 — tool_name fallback to "unknown" now logs a warning via a shared _tool_name() helper (used by all three tool-part call sites). Not changed: runtime/factory.py:271 kwargs.get("evaluator") — this is the same magic-string kwarg LangChain #899's factory uses (factory.py:281); it's the host-dispatch contract shared by all adapters, so kept for parity. Tests: added built-in-tool-result AFTER_TOOL coverage; updated _coerce_args variant assertion. 82 pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent f3a7cbc commit 6bca797

2 files changed

Lines changed: 65 additions & 5 deletions

File tree

packages/uipath-pydantic-ai/src/uipath_pydantic_ai/governance/model.py

Lines changed: 36 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
from pydantic_ai import Agent
4646
from pydantic_ai.messages import (
4747
BuiltinToolCallPart,
48+
BuiltinToolReturnPart,
4849
ModelRequest,
4950
TextPart,
5051
ToolCallPart,
@@ -200,24 +201,37 @@ def on_request(self, messages: Any) -> None:
200201
for part in parts:
201202
if isinstance(part, ToolReturnPart):
202203
self._after_tool(
203-
part.tool_name or "unknown",
204+
_tool_name(part),
204205
part.content,
205206
tool_call_id=getattr(part, "tool_call_id", None),
206207
)
207208

208209
# ----- after the model call ---------------------------------------
209210

210211
def on_response(self, response: Any) -> None:
211-
"""Fire AFTER_MODEL (response text) + TOOL_CALL (each tool-call part)."""
212+
"""Fire AFTER_MODEL (response text) + TOOL_CALL / AFTER_TOOL parts.
213+
214+
A provider-executed **built-in** tool carries both its call
215+
(``BuiltinToolCallPart``) and its result (``BuiltinToolReturnPart``)
216+
inline in the model response, so AFTER_TOOL for built-in tools is fired
217+
here — symmetric with the built-in TOOL_CALL — rather than on the next
218+
request (where only user-tool ``ToolReturnPart``s arrive).
219+
"""
212220
parts = getattr(response, "parts", None) or []
213221
self._after_model(self._response_text(parts))
214222
for part in parts:
215223
if isinstance(part, (ToolCallPart, BuiltinToolCallPart)):
216224
self._tool_call(
217-
part.tool_name or "unknown",
225+
_tool_name(part),
218226
part.args,
219227
tool_call_id=getattr(part, "tool_call_id", None),
220228
)
229+
elif isinstance(part, BuiltinToolReturnPart):
230+
self._after_tool(
231+
_tool_name(part),
232+
part.content,
233+
tool_call_id=getattr(part, "tool_call_id", None),
234+
)
221235

222236
# ----- individual evaluate_* wrappers (block-propagate, else swallow) --
223237

@@ -380,6 +394,22 @@ def _pieces() -> Iterable[str]:
380394
return _stringify(content)
381395

382396

397+
def _tool_name(part: Any) -> str:
398+
"""Return ``part.tool_name`` or ``"unknown"``, logging the fallback.
399+
400+
A missing tool name means TOOL_CALL / AFTER_TOOL can't be attributed to a
401+
real tool, so surface it rather than silently reporting ``"unknown"``.
402+
"""
403+
name = getattr(part, "tool_name", None)
404+
if name:
405+
return name
406+
logger.warning(
407+
"governance: %s carries no tool_name; reporting 'unknown'",
408+
type(part).__name__,
409+
)
410+
return "unknown"
411+
412+
383413
def _coerce_args(args: Any) -> Dict[str, Any]:
384414
"""Normalise ``ToolCallPart.args`` (dict / JSON string / None) to a dict."""
385415
if args is None:
@@ -391,7 +421,9 @@ def _coerce_args(args: Any) -> Dict[str, Any]:
391421
parsed = json.loads(args)
392422
return parsed if isinstance(parsed, dict) else {"_": parsed}
393423
except (TypeError, ValueError):
394-
return {}
424+
# Preserve the raw string so an arg-based policy can still scan it;
425+
# a malformed payload must not be a way to slip past governance.
426+
return {"_raw": args}
395427
return {}
396428

397429

packages/uipath-pydantic-ai/tests/governance/test_model.py

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
import pytest
1919
from pydantic_ai import Agent
2020
from pydantic_ai.messages import (
21+
BuiltinToolCallPart,
22+
BuiltinToolReturnPart,
2123
ModelRequest,
2224
ModelResponse,
2325
TextPart,
@@ -225,6 +227,31 @@ def test_on_response_fires_after_model_and_tool_call():
225227
assert tool_call["session_state"]["tool_calls"] == 1
226228

227229

230+
def test_on_response_fires_after_tool_for_builtin_tool_result():
231+
"""A provider-executed built-in tool carries its result inline in the
232+
response (BuiltinToolReturnPart); AFTER_TOOL must fire for it — symmetric
233+
with the built-in TOOL_CALL."""
234+
ev = FakeEvaluator()
235+
cb = _make_callbacks(ev)
236+
response = ModelResponse(
237+
parts=[
238+
BuiltinToolCallPart(
239+
tool_name="web_search", args={"q": "x"}, tool_call_id="b1"
240+
),
241+
BuiltinToolReturnPart(
242+
tool_name="web_search", content={"results": "found"}, tool_call_id="b1"
243+
),
244+
]
245+
)
246+
cb.on_response(response)
247+
hooks = _hooks(ev)
248+
assert "tool_call" in hooks and "after_tool" in hooks
249+
after_tool = [kw for h, kw in ev.calls if h == "after_tool"][0]
250+
assert after_tool["tool_name"] == "web_search"
251+
assert after_tool["tool_call_id"] == "b1"
252+
assert "found" in after_tool["tool_result"]
253+
254+
228255
def test_on_response_coerces_json_string_args():
229256
ev = FakeEvaluator()
230257
cb = _make_callbacks(ev)
@@ -393,7 +420,8 @@ def test_coerce_args_variants():
393420
assert _coerce_args({"a": 1}) == {"a": 1}
394421
assert _coerce_args('{"a": 1}') == {"a": 1}
395422
assert _coerce_args(None) == {}
396-
assert _coerce_args("not json") == {}
423+
# malformed JSON is preserved (not dropped) so arg-based policies can scan it
424+
assert _coerce_args("not json") == {"_raw": "not json"}
397425

398426

399427
def test_block_in_before_model_propagates():

0 commit comments

Comments
 (0)