YAshhh29 commented on PR #71575:
URL: https://github.com/apache/airflow/pull/71575#issuecomment-5734852067

   Thanks @kaxil — all five are addressed, split one commit per point: 
`e1de6303` (the `_digest` normalization), `b1fd367b` (counter), `d00158f8` 
(step in the warnings), `1fd22218` (the missing coverage case), `f85947d4` (the 
docs paragraph). The `_digest` one was the real find: it's a pre-existing bug, 
but my previous commit made it worse rather than introducing it, so I've 
treated it as the root fix and let the rest follow from there.
   
   
   I reproduced each case before changing anything, on pydantic-ai 2.29.0:
   
   | case | fingerprint today |
   |---|---|
   | `datetime` tool argument | `None` |
   | `Decimal` tool argument | `None` |
   | `ToolOrOutput` in `tool_choice` | `None` |
   
   `_digest` now runs the payload through `to_jsonable_python` before 
`json.dumps`. I checked the three properties that make that safe instead of 
assuming them: `PydanticSerializationError` subclasses `ValueError`, so the 
existing `except (TypeError, ValueError)` catches it unchanged; `object()` 
still raises, so an unverifiable request still fingerprints as `None`; and 
plain JSON values come back byte-identical, so entries already written keep 
matching. There's a test pinning that last one to the pre-normalization sha256, 
so a future pydantic change can't quietly shift stored fingerprints.
   
   Worth flagging: `httpx.Timeout` still fails under pydantic too, so 
`_TRANSPORT_ONLY_SETTINGS` is still load-bearing. I've narrowed that comment to 
draw the line at what pydantic can't serialize, since merely non-JSON values 
are fine now.
   
   No new dependency — this provider already imports from `pydantic_core` in 
`utils/tool_definition.py`.
   
   Tests: 98 in `providers/common/ai/tests/unit/common/ai/durable`, nine of 
them new here. Reverting just the `to_jsonable_python` call fails exactly the 
five pydantic-native tests while the stability and `object()` tests still pass, 
which is the signature I was after — the change only moves what it's meant to.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to