DO NOT MERGE - test(pydantic-ai): test iterate-pr flow #5640
DO NOT MERGE - test(pydantic-ai): test iterate-pr flow #5640
Conversation
…Url support Move shared binary/image serialization logic from ai_client.py and invoke_agent.py into a shared _serialize_binary_content_item and _serialize_image_url_item helpers in spans/utils.py. Add support for pydantic-ai's ImageUrl type, with base64 data URLs redacted and regular HTTP URLs preserved. Remove unused tool_definition parameter from execute_tool_span. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨
Bug Fixes 🐛
Documentation 📚
Internal Changes 🔧Openai Agents
Other
Other
🤖 This preview updates automatically when you update the PR. |
Codecov Results 📊✅ 13 passed | Total: 13 | Pass Rate: 100% | Execution Time: 4.53s All tests are passing successfully. ❌ Patch coverage is 0.00%. Project has 14039 uncovered lines. Files with missing lines (6)
Generated by Codecov Action |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| def execute_tool_span( | ||
| tool_name: str, | ||
| tool_args: "Any", | ||
| agent: "Any", | ||
| tool_type: str = "function", | ||
| tool_definition: "Optional[ToolDefinition]" = None, | ||
| tool_name: str, tool_args: "Any", agent: "Any", tool_type: str = "function" | ||
| ) -> "sentry_sdk.tracing.Span": |
There was a problem hiding this comment.
Removal of GEN_AI_TOOL_DESCRIPTION will break existing test
The code removes the logic that sets GEN_AI_TOOL_DESCRIPTION on execute_tool spans (lines 35-40 being deleted). However, the test test_tool_description_in_execute_tool_span in tests/integrations/pydantic_ai/test_pydantic_ai.py (lines 2835-2836) explicitly asserts that this field should be present and contain the tool's docstring. Since the test file is not listed as being modified in this PR, this change will cause test failures.
Verification
Verified by reading the test file at lines 2799-2836 which shows test_tool_description_in_execute_tool_span asserting SPANDATA.GEN_AI_TOOL_DESCRIPTION in tool_span['data']. Confirmed the test file is not in the PR's list of modified files. Checked grep results showing the deleted code was the only place setting this field for pydantic_ai execute_tool spans.
Identified by Warden code-review · JDR-KXH
| data_url_matches = DATA_URL_BASE64_REGEX.match(item.url) | ||
|
|
||
| if data_url_matches: | ||
| mime_type = data_url_matches[1] or "image" | ||
| return { | ||
| "type": "image", | ||
| "mime_type": mime_type, | ||
| "content": BLOB_DATA_SUBSTITUTE, | ||
| } | ||
|
|
||
| return { | ||
| "type": "image", | ||
| "content": str(item.url), |
There was a problem hiding this comment.
Regex fails to match valid base64 data URLs with complex MIME types
The DATA_URL_BASE64_REGEX pattern ^data:([a-zA-Z]+/[a-zA-Z]+);base64,... only matches simple MIME types with alphanumeric characters. Valid MIME types like image/svg+xml or application/vnd.api+json (containing + or .) will not match, causing _serialize_image_url_item to fall through to the non-data-URL branch and expose the full base64-encoded image content in span data instead of redacting it.
Verification
Verified by reading consts.py (line 7-9) which defines the regex as ^data:([a-zA-Z]+/[a-zA-Z]+);base64,.... The character class [a-zA-Z]+ in the MIME subtype does not include +, ., or - characters. Compared with parse_data_uri in sentry_sdk/ai/utils.py (lines 43-72) which uses string splitting and handles edge cases gracefully.
Also found at 2 additional locations
sentry_sdk/integrations/pydantic_ai/consts.py:7-9sentry_sdk/integrations/pydantic_ai/spans/ai_client.py:165-166
Identified by Warden code-review · NKQ-KBQ
dingsdax
changed the title
dingsdax
changed the title
The previous commit removed tool_definition from execute_tool_span, which inadvertently dropped gen_ai.tool.description from spans. Restore description support by extracting the description string from tool.tool_def in patches/tools.py and passing it as Optional[str] to execute_tool_span, avoiding the ToolDefinition type coupling while preserving the behavior. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| For data URLs containing base64-encoded images, the content is redacted. | ||
| For regular HTTP URLs, the URL string is preserved. | ||
| """ | ||
| data_url_matches = DATA_URL_BASE64_REGEX.match(item.url) |
There was a problem hiding this comment.
Bug: The function _serialize_image_url_item will raise a TypeError because it attempts a regex match on a Pydantic Url object without first converting it to a string.
Severity: HIGH
Suggested Fix
Convert item.url to a string before performing the regex match, similar to how it is handled elsewhere in the function. Change DATA_URL_BASE64_REGEX.match(item.url) to DATA_URL_BASE64_REGEX.match(str(item.url)).
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: sentry_sdk/integrations/pydantic_ai/spans/utils.py#L29
Potential issue: The function `_serialize_image_url_item` calls
`DATA_URL_BASE64_REGEX.match(item.url)` without first converting `item.url` to a string.
According to Pydantic V2's documentation, URL fields are stored as
`pydantic_core._pydantic_core.Url` objects, not strings. The `re.match` function expects
a string or bytes-like object, so this operation will raise a `TypeError`. While this
error is caught in the `ai_client.py` integration, it is not handled in
`invoke_agent.py`, which will cause the instrumentation to crash when an `ImageUrl` is
processed.
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is kicking off a free cloud agent to fix these issues. This run is complimentary, but you can enable autofix for all future PRs in the Cursor dashboard.
| # Group 1: MIME type (e.g. "image/png"), Group 2: base64 data | ||
| DATA_URL_BASE64_REGEX = re.compile( | ||
| r"^data:([a-zA-Z]+/[a-zA-Z]+);base64,([A-Za-z0-9+/\-_]+={0,2})$" | ||
| ) |
There was a problem hiding this comment.
Regex fails to match common MIME types, leaking data
High Severity
DATA_URL_BASE64_REGEX uses [a-zA-Z]+/[a-zA-Z]+ for the MIME type, which only matches pure alphabetic characters. Valid MIME types like video/mp4, audio/mp3, image/svg+xml, image/x-icon, and font/woff2 contain digits, hyphens, or plus signs and won't match. When a valid base64 data URL fails to match, _serialize_image_url_item falls through to the non-redacted path and exposes the full base64-encoded content in span data. An existing parse_data_uri utility in sentry_sdk/ai/utils.py already handles data URI parsing robustly per RFC 2397 and is used elsewhere in the codebase.
Additional Locations (1)
| from pydantic_ai.messages import BinaryContent, ImageUrl # type: ignore | ||
| except ImportError: | ||
| BinaryContent = None | ||
| ImageUrl = None |
There was a problem hiding this comment.
Unused runtime imports of BinaryContent and ImageUrl
Low Severity
BinaryContent and ImageUrl are imported at runtime in spans/utils.py but never referenced anywhere in the file. The helper functions _serialize_image_url_item and _serialize_binary_content_item both accept item: "Any" and access attributes directly — the isinstance checks happen in the calling files (ai_client.py and invoke_agent.py), which have their own imports. These are dead runtime imports that add unnecessary coupling to pydantic_ai.messages.
1 participant
Extract shared content serialization logic from
ai_client.pyandinvoke_agent.pyinto dedicated helpers inspans/utils.py, and add support for pydantic-ai'sImageUrlmessage type.Previously, both
ai_client.pyandinvoke_agent.pyduplicated the same inline logic for serializingBinaryContentitems into span data — including theget_modality_from_mime_typecall andBLOB_DATA_SUBSTITUTEredaction. This refactor centralizes that into_serialize_binary_content_item.The new
_serialize_image_url_itemhelper handlesImageUrlcontent items (introduced in recent pydantic-ai versions). It detects base64-encoded data URLs via a regex inconsts.pyand redacts their content while preserving regular HTTP URLs verbatim.The
tool_definitionparameter was also removed fromexecute_tool_span— it was only used to setGEN_AI_TOOL_DESCRIPTION, and the pydantic-aiToolDefinitiontype is an internal API that added an unnecessary coupling.