YAshhh29 commented on code in PR #71575:
URL: https://github.com/apache/airflow/pull/71575#discussion_r4112452294


##########
providers/common/ai/tests/unit/common/ai/durable/test_fingerprint.py:
##########
@@ -210,3 +224,199 @@ def test_arg_order_does_not_matter(self):
         assert fingerprint_tool_call("t", {"a": 1, "b": 2}, "id1") == 
fingerprint_tool_call(
             "t", {"b": 2, "a": 1}, "id1"
         )
+
+
+class TestPydanticNativeValues:
+    """Values that are not JSON types but hash the same on every attempt must 
still fingerprint.
+
+    Tool arguments reach ``fingerprint_tool_call`` already coerced by 
pydantic, and
+    ``tool_choice`` accepts a dataclass while genuinely affecting the 
response, so it
+    cannot be stripped as transport-only. Since a step that cannot be 
fingerprinted is
+    no longer cached at all, refusing these values would stop an ordinary 
typed tool
+    from ever being cached.
+    """
+
+    def test_datetime_tool_argument_fingerprints(self):
+        when = datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc)
+
+        assert fingerprint_tool_call("t", {"when": when}, "id1") is not None
+
+    def test_decimal_tool_argument_fingerprints(self):
+        assert fingerprint_tool_call("t", {"amount": Decimal("10.5")}, "id1") 
is not None
+
+    def test_datetime_tool_argument_is_stable_and_distinguishing(self):
+        early = datetime.datetime(2026, 1, 1, tzinfo=datetime.timezone.utc)
+        late = datetime.datetime(2026, 6, 1, tzinfo=datetime.timezone.utc)
+
+        assert fingerprint_tool_call("t", {"when": early}, "id1") == 
fingerprint_tool_call(
+            "t", {"when": early}, "id1"
+        )
+        assert fingerprint_tool_call("t", {"when": early}, "id1") != 
fingerprint_tool_call(
+            "t", {"when": late}, "id1"
+        )
+
+    def test_tool_choice_dataclass_fingerprints(self):
+        fp = fingerprint_model_request(
+            "m",
+            make_messages(),
+            {"tool_choice": ToolOrOutput(function_tools=["my_tool"])},
+            ModelRequestParameters(),
+        )
+
+        assert fp is not None
+
+    def test_tool_choice_dataclass_still_affects_the_fingerprint(self):
+        one = fingerprint_model_request(
+            "m",
+            make_messages(),
+            {"tool_choice": ToolOrOutput(function_tools=["a"])},
+            ModelRequestParameters(),
+        )
+        other = fingerprint_model_request(
+            "m",
+            make_messages(),
+            {"tool_choice": ToolOrOutput(function_tools=["b"])},
+            ModelRequestParameters(),
+        )
+
+        assert one is not None
+        assert one != other
+
+    def test_value_pydantic_cannot_serialize_still_returns_none(self):
+        """Normalization must not turn a genuinely unserializable value into a 
hash."""
+        assert fingerprint_tool_call("t", {"v": object()}, "id1") is None
+
+    def test_plain_payload_digest_is_unchanged_by_normalization(self):

Review Comment:
   Added a test helper that hashes the json-mode dump the way main does, and 
cases that must equal it: non-UTF-8 `BinaryContent`, a bytes tool return, date 
and mixed keys, NaN, a datetime, and an agent's `instruction_parts`. Reverting 
either dump to python mode now fails them. I compute main's digest in the test 
rather than hard-coding it, since a fixed digest would break whenever 
pydantic-ai adds a field. There's also a subprocess test for a set inside 
`ToolReturnPart.content` across hash seeds.
   



##########
providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py:
##########
@@ -119,13 +195,21 @@ def fingerprint_model_request(
     output mode and schema, native tools, ...) so any change to what is sent
     to the model invalidates the cached response.
 
-    Returns ``None`` when the request cannot be serialized; ``None`` compares
-    equal to ``None``, so requests that cannot be fingerprinted degrade to
-    unverified positional replay rather than disabling caching.
+    Returns ``None`` when the request cannot be serialized even through 
pydantic,
+    which prevents the step from being replayed or cached. Because model 
settings
+    and message history are carried into every later request, such a value in
+    either usually degrades every subsequent model step of the run the same 
way.
+    ``step`` is attached to that warning so the log names where it began.
     """
     try:
-        dumped = ModelMessagesTypeAdapter.dump_python(messages, mode="json")
-        params = 
_MODEL_REQUEST_PARAMETERS_ADAPTER.dump_python(model_request_parameters, 
mode="json")
+        # ``mode="python"``, not ``mode="json"``: a json-mode dump renders a 
set as
+        # a list in iteration order, and a tool that returned a set puts one 
in the
+        # message history, where it would reach the hash already unstably 
ordered.
+        # Python mode leaves it a set for ``_canonical`` to order. For values 
that
+        # are not sets the two modes produce the same digest, so stored

Review Comment:
   With the json-mode dump back, that claim holds again. On 2.44, 
`Agent(instructions="Be terse.")` gives the same digest as main (it drifted at 
the previous head), and NaN renders as `null` again. Checked on 2.33 and 2.44.
   



-- 
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