YAshhh29 commented on code in PR #71575:
URL: https://github.com/apache/airflow/pull/71575#discussion_r4112446347
##########
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
+ # fingerprints are unaffected.
+ dumped = ModelMessagesTypeAdapter.dump_python(messages)
Review Comment:
You're right, and your larger point turned out to be the real fix. I dropped
the hand-written renderer: messages and request parameters go back to
pydantic's json-mode dump, exactly as on main, and tool args and settings go
through `to_jsonable_python(bytes_mode="base64")`. The only things added on top
are sorting lists that came from sets and refusing iterators. A PNG in the
prompt and a bytes tool return now digest the same as main.
##########
providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py:
##########
@@ -98,11 +127,56 @@ def _strip_volatile(messages_dump: list[dict[str, Any]])
-> list[dict[str, Any]]
return stripped
+def _canonical(value: Any) -> Any:
+ """
+ Render ``value`` as JSON-safe data whose encoding is identical on every
attempt.
+
+ ``to_jsonable_python`` on its own is not a safe fingerprint input, for two
+ reasons that both matter here because tool arguments arrive as live Python
+ objects that pydantic has already validated.
+
+ It renders a ``set`` in iteration order, and for string members that order
+ follows the interpreter's hash seed. Every task attempt is a fresh
process, so
+ a ``set[str]`` argument would hash differently each time and never replay
--
+ worse than declining to cache, because the step re-runs live on every
retry.
+ Members are therefore ordered by their own JSON encoding.
+
+ It also *consumes* an iterator, and pydantic validates an ``Iterable[T]``
+ parameter lazily into a ``ValidatorIterator``. Normalizing the arguments
would
+ drain the tool's own input before the tool ran, so the tool would see an
empty
+ sequence and that wrong result would be cached under the fingerprint of the
+ full one. Such a value is refused instead, which degrades the step to the
+ not-cached path rather than corrupting it.
+ """
+ if value is None or isinstance(value, (str, bool, int, float)):
+ return value
+ if isinstance(value, Iterator):
+ # Hashing this means draining it, leaving nothing for the tool to read.
+ raise TypeError(f"cannot fingerprint {type(value).__name__} without
consuming it")
+ if isinstance(value, Mapping):
+ return {key: _canonical(item) for key, item in value.items()}
Review Comment:
Covered by the same change, since keys are rendered by pydantic now, so
`dict[date, float]`, enum and UUID keys digest as on main. One case main hashed
ambiguously, `{1: "a", "1": "b"}`, now refuses to fingerprint, because both
keys render as `"1"`.
--
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]
