YAshhh29 commented on code in PR #71575:
URL: https://github.com/apache/airflow/pull/71575#discussion_r4234274441
##########
providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py:
##########
@@ -123,44 +362,86 @@ 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 fingerprinted, which prevents
the
+ step from being replayed or cached. Model settings, the tool definitions
in the
+ request parameters and the message history are normally carried into every
+ later request, so such a value in any of them usually degrades every
+ subsequent model step of the run the same way. ``step`` is attached to the
+ warning so the log names where it began.
"""
try:
+ # Messages and parameters are hashed from pydantic's json-mode dump,
as they
+ # always were, so stored fingerprints still match. The python-mode
dumps only
+ # guide ``_order_sets``, which runs where a set or a non-string key
needs it.
+ messages_guide = ModelMessagesTypeAdapter.dump_python(messages)
Review Comment:
You're right, that was a regression against main. Both guide dumps now catch
`TypeError`, and when there is no guide the json-mode dump is hashed on its
own, unordered, the way main does it. A frozenset of frozen dataclasses or
models and a dict keyed by a frozen model all give main's digest again.
I ran the tests on pydantic 2.12.0 and 2.13.4, since the dict-key case takes
a different path on each, and the test compares against main's digest so it
holds on both. `_render` still raises for tool args, as you said. The comment
above the dumps is corrected too.
##########
providers/common/ai/docs/durable_execution.rst:
##########
@@ -107,6 +107,58 @@ cache:
never replays responses that belong to a different conversation.
4. After successful completion, the cached steps are deleted.
+Plain JSON arguments and settings are fingerprinted exactly as before. Anything
+else is fingerprinted from pydantic's JSON rendering, so ordinary types that
are
+not JSON -- a ``datetime`` or ``Decimal`` tool argument, a dataclass in
+``tool_choice``, the bytes in a ``BinaryContent``, a dict keyed by date --
+fingerprint normally. Bytes in tool arguments and settings are rendered as
base64,
+so binary data that is not valid UTF-8 fingerprints too; bytes inside a
pydantic
+model follow that model's ``ser_json_bytes`` setting instead, which renders
them as
+UTF-8 text by default. Because a tool call is fingerprinted from how its
arguments
+render, a field excluded from serialization and a secret value, which renders
+masked, take no part in it.
+
+A step whose request cannot be fingerprinted is not cached, and on retry it
runs
+live rather than replaying an unverified entry. That happens when pydantic
cannot
Review Comment:
Agreed, "pydantic cannot render" was pointing at the wrong thing. With the
fallback in, the paragraph now says this only applies to tool arguments and
settings, where it was never fingerprinted anyway, and that in the message
history these hash as they always did. It mentions dataclass instances as well
as models.
##########
providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py:
##########
@@ -102,19 +159,201 @@ def _strip_volatile(messages_dump: list[dict[str, Any]])
-> list[dict[str, Any]]
return stripped
+def _check_guide(guide: Any) -> bool:
+ """
+ Refuse an iterator, and report whether the JSON rendering needs
``_order_sets``.
+
+ ``guide`` is pydantic's python-mode dump of the payload. It has the shape
of the
+ json-mode rendering that is hashed -- the same keys, aliases, exclusions,
+ computed fields, extras and serializer output -- but keeps sets as sets
and dict
+ keys as they are, and wraps an iterator in a ``SerializationIterator``
without
+ reading it. So this walk runs no user code, and nothing has been consumed
yet.
+
+ Raises ``TypeError`` if the payload renders through an iterator anywhere.
+ Rendering it to JSON would consume it, and pydantic validates an
``Iterable[T]``
+ tool parameter lazily into an iterator; a tool return that holds a
generator
+ would reach the model empty. Returns ``True`` if the guide holds a set, or
a dict
+ key that is not a string, since only those can make the JSON rendering
depend on
+ the hash seed or lose a key.
+ """
+ needs_check = False
+ pending = [guide]
+ seen: set[int] = set()
+ while pending:
+ item = pending.pop()
+ if id(item) in seen:
+ # The dump shares this object, or left a cycle in place: walked
already.
+ continue
+ seen.add(id(item))
+ children: Iterable[Any]
+ if isinstance(item, dict):
+ needs_check = needs_check or any(not isinstance(key, str) for key
in item)
+ children = item.values()
+ elif isinstance(item, (list, tuple, deque)):
+ children = item
+ elif isinstance(item, (set, frozenset)):
+ needs_check = True
+ children = item
+ elif isinstance(item, Iterator):
+ raise TypeError(f"cannot fingerprint a value that renders through
a {type(item).__name__}")
+ elif isinstance(item, enum.Enum):
+ # Python mode keeps an Enum member; JSON renders its value.
+ children = (item.value,)
+ else:
+ continue
+ pending.extend(child for child in children if type(child) not in
_LEAF_TYPES)
+ return needs_check
+
+
+def _json_order(member: Any) -> str:
+ return json.dumps(member, sort_keys=True)
+
+
+def _template(value: Any) -> Any:
+ """
+ Say where the sets are inside one set member, or ``None`` if that cannot
be said.
+
+ ``_LEAF`` for a member with no set inside it, ``("set", inner)`` for a set
whose
+ members all have the template ``inner``, and ``("sequence", parts)`` for a
tuple
+ with a set somewhere in it.
+ """
+ if isinstance(value, enum.Enum):
+ return _template(value.value)
+ if isinstance(value, (set, frozenset)):
+ inner = _member_template(value)
+ return None if inner is None else ("set", inner)
+ if isinstance(value, (list, tuple, deque)):
+ parts = tuple(_template(item) for item in value)
+ if None in parts:
+ return None
+ return _LEAF if all(part == _LEAF for part in parts) else ("sequence",
parts)
+ if isinstance(value, dict):
+ # No set member dumps to a dict (python mode refuses a set of models),
so
+ # nothing is known about one.
+ return None
+ return _LEAF
+
+
+def _member_template(members: Iterable[Any]) -> Any:
+ """Return the template every member of a set shares, or ``None`` if they
differ."""
+ templates = {_template(member) for member in members}
+ if not templates:
+ return _LEAF
+ return templates.pop() if len(templates) == 1 else None
+
+
+def _apply_template(template: Any, rendered: Any) -> Any:
+ """Sort the lists that ``template`` puts a set at, in the rendering of one
set member."""
+ if template == _LEAF or not isinstance(rendered, list):
+ return rendered
+ kind, inner = template
+ if kind == "set":
+ return sorted((_apply_template(inner, item) for item in rendered),
key=_json_order)
+ if len(rendered) != len(inner):
+ return rendered
+ return [_apply_template(part, item) for part, item in zip(inner, rendered)]
+
+
+def _order_sets(guide: Any, rendered: Any) -> Any:
+ """
+ Return ``rendered`` with every list that pydantic rendered from a set
sorted.
+
+ ``rendered`` is the json-mode rendering that is hashed, and ``guide`` the
+ python-mode dump of the same payload (see ``_check_guide``). They are
walked
+ side by side: a dict or a sequence pairs up by position, because both dumps
+ keep the same order, and a list is sorted where the guide holds a set. A
set's
+ members cannot pair up by position -- the dump builds a new set, which may
+ iterate in another order -- so the members' shared template says where any
set
+ inside one of them sits (see ``_template``). Anything that does not line
up is
+ kept exactly as rendered, so nothing that did not come from a set is
reordered.
+
+ Raises ``TypeError`` where distinct dict keys render alike, such as ``1``
and
+ ``"1"``, or ``None`` and ``nan`` in a message dump: the guide then holds
more keys
+ than the rendering, and hashing it would let two different payloads share a
+ digest. A dict whose keys are all strings cannot collide, so one that
renders
+ fewer keys was reshaped by a serializer that applies only in JSON mode,
and is
+ kept as rendered.
+ """
+ if isinstance(guide, enum.Enum):
+ # An Enum member renders as its value.
+ return _order_sets(guide.value, rendered)
+ if isinstance(guide, (set, frozenset)):
+ if not isinstance(rendered, list) or len(rendered) != len(guide):
+ return rendered
+ template = _member_template(guide)
+ if template is None:
+ return rendered
+ return sorted((_apply_template(template, item) for item in rendered),
key=_json_order)
Review Comment:
Took your approach. A list is now sorted only when the set, rendered on its
own and ordered the same way, gives the same members. Your `tags`/`rows` swap
gives different digests for `[1, 2]` and `[2, 1]` on both the tool path and the
history, and matches main's digest.
For the other direction: when a set can't be found (mixed member shapes, a
renamed set field, a reshaped one), the step is refused if the set's order
follows the hash seed. If it doesn't, like a set of ints, it is kept as
rendered so it still matches main.
One limit I couldn't close: a JSON-only serializer that puts a list with
exactly the set's members in the set's place still gets sorted. I don't see a
way to tell those apart. The docs now carry the `["z-first", "a"]` caveat.
##########
providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py:
##########
@@ -102,19 +159,201 @@ def _strip_volatile(messages_dump: list[dict[str, Any]])
-> list[dict[str, Any]]
return stripped
+def _check_guide(guide: Any) -> bool:
+ """
+ Refuse an iterator, and report whether the JSON rendering needs
``_order_sets``.
+
+ ``guide`` is pydantic's python-mode dump of the payload. It has the shape
of the
+ json-mode rendering that is hashed -- the same keys, aliases, exclusions,
+ computed fields, extras and serializer output -- but keeps sets as sets
and dict
+ keys as they are, and wraps an iterator in a ``SerializationIterator``
without
+ reading it. So this walk runs no user code, and nothing has been consumed
yet.
+
+ Raises ``TypeError`` if the payload renders through an iterator anywhere.
+ Rendering it to JSON would consume it, and pydantic validates an
``Iterable[T]``
+ tool parameter lazily into an iterator; a tool return that holds a
generator
+ would reach the model empty. Returns ``True`` if the guide holds a set, or
a dict
+ key that is not a string, since only those can make the JSON rendering
depend on
+ the hash seed or lose a key.
+ """
+ needs_check = False
+ pending = [guide]
+ seen: set[int] = set()
+ while pending:
+ item = pending.pop()
+ if id(item) in seen:
+ # The dump shares this object, or left a cycle in place: walked
already.
+ continue
+ seen.add(id(item))
+ children: Iterable[Any]
+ if isinstance(item, dict):
+ needs_check = needs_check or any(not isinstance(key, str) for key
in item)
+ children = item.values()
+ elif isinstance(item, (list, tuple, deque)):
+ children = item
+ elif isinstance(item, (set, frozenset)):
+ needs_check = True
+ children = item
+ elif isinstance(item, Iterator):
+ raise TypeError(f"cannot fingerprint a value that renders through
a {type(item).__name__}")
+ elif isinstance(item, enum.Enum):
+ # Python mode keeps an Enum member; JSON renders its value.
+ children = (item.value,)
+ else:
+ continue
+ pending.extend(child for child in children if type(child) not in
_LEAF_TYPES)
+ return needs_check
+
+
+def _json_order(member: Any) -> str:
+ return json.dumps(member, sort_keys=True)
+
+
+def _template(value: Any) -> Any:
+ """
+ Say where the sets are inside one set member, or ``None`` if that cannot
be said.
+
+ ``_LEAF`` for a member with no set inside it, ``("set", inner)`` for a set
whose
+ members all have the template ``inner``, and ``("sequence", parts)`` for a
tuple
+ with a set somewhere in it.
+ """
+ if isinstance(value, enum.Enum):
+ return _template(value.value)
+ if isinstance(value, (set, frozenset)):
+ inner = _member_template(value)
+ return None if inner is None else ("set", inner)
+ if isinstance(value, (list, tuple, deque)):
+ parts = tuple(_template(item) for item in value)
+ if None in parts:
+ return None
+ return _LEAF if all(part == _LEAF for part in parts) else ("sequence",
parts)
+ if isinstance(value, dict):
+ # No set member dumps to a dict (python mode refuses a set of models),
so
+ # nothing is known about one.
+ return None
+ return _LEAF
+
+
+def _member_template(members: Iterable[Any]) -> Any:
+ """Return the template every member of a set shares, or ``None`` if they
differ."""
+ templates = {_template(member) for member in members}
+ if not templates:
+ return _LEAF
+ return templates.pop() if len(templates) == 1 else None
+
+
+def _apply_template(template: Any, rendered: Any) -> Any:
+ """Sort the lists that ``template`` puts a set at, in the rendering of one
set member."""
+ if template == _LEAF or not isinstance(rendered, list):
+ return rendered
+ kind, inner = template
+ if kind == "set":
+ return sorted((_apply_template(inner, item) for item in rendered),
key=_json_order)
+ if len(rendered) != len(inner):
+ return rendered
+ return [_apply_template(part, item) for part, item in zip(inner, rendered)]
+
+
+def _order_sets(guide: Any, rendered: Any) -> Any:
+ """
+ Return ``rendered`` with every list that pydantic rendered from a set
sorted.
+
+ ``rendered`` is the json-mode rendering that is hashed, and ``guide`` the
+ python-mode dump of the same payload (see ``_check_guide``). They are
walked
+ side by side: a dict or a sequence pairs up by position, because both dumps
+ keep the same order, and a list is sorted where the guide holds a set. A
set's
+ members cannot pair up by position -- the dump builds a new set, which may
+ iterate in another order -- so the members' shared template says where any
set
+ inside one of them sits (see ``_template``). Anything that does not line
up is
+ kept exactly as rendered, so nothing that did not come from a set is
reordered.
+
+ Raises ``TypeError`` where distinct dict keys render alike, such as ``1``
and
+ ``"1"``, or ``None`` and ``nan`` in a message dump: the guide then holds
more keys
+ than the rendering, and hashing it would let two different payloads share a
+ digest. A dict whose keys are all strings cannot collide, so one that
renders
+ fewer keys was reshaped by a serializer that applies only in JSON mode,
and is
+ kept as rendered.
+ """
+ if isinstance(guide, enum.Enum):
+ # An Enum member renders as its value.
+ return _order_sets(guide.value, rendered)
+ if isinstance(guide, (set, frozenset)):
+ if not isinstance(rendered, list) or len(rendered) != len(guide):
+ return rendered
+ template = _member_template(guide)
+ if template is None:
+ return rendered
+ return sorted((_apply_template(template, item) for item in rendered),
key=_json_order)
+ if isinstance(guide, dict):
+ if not isinstance(rendered, dict):
+ return rendered
+ if len(rendered) != len(guide):
+ if len(rendered) < len(guide) and any(not isinstance(key, str) for
key in guide):
+ raise TypeError("dict keys collide once rendered as JSON")
Review Comment:
Both fixed the way you suggested. Non-string keys are rendered on their own,
and it only counts as a collision when that gives fewer keys than the guide.
The filtered `dict[int, int]` hashes like main again and `{1: "a", "1": "b"}`
is still refused.
The rendered keys also have to match the guide's keys in order now, so a
reordered int-keyed dict is no longer paired with its neighbour. Your `{2:
{"p", "q"}, 1: [...]}` pair is refused, and the same with a set of ints gives
two different digests. I kept one extra refusal for `nan`/`inf` keys, because
the message dump renders those as `None` and rendering the keys alone doesn't
show it. The docstring line you quoted is rewritten.
--
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]
