kaxil commented on code in PR #71575:
URL: https://github.com/apache/airflow/pull/71575#discussion_r3963555700
##########
providers/common/ai/src/airflow/providers/common/ai/durable/caching_model.py:
##########
@@ -95,7 +95,7 @@ async def request(
cached, cached_fingerprint = self.storage.load_model_response(key)
if cached is not None:
- if cached_fingerprint == fingerprint:
+ if fingerprint is not None and cached_fingerprint == fingerprint:
Review Comment:
An entry stored with `fingerprint=None` can never be replayed now: this line
needs `fingerprint is not None`, and a real digest won't equal a stored `None`
either. Line 115 still saves with `fingerprint=fingerprint` (same in
`caching_toolset.py:92`), so a run that can't fingerprint writes a dead entry
per step and reads it back next attempt only to log the warning below, and
`ObjectStorageDurableCache._save_cache` rewrites the whole blob each time.
Worth skipping the save when `fingerprint is None`?
##########
providers/common/ai/src/airflow/providers/common/ai/durable/fingerprint.py:
##########
@@ -34,9 +34,8 @@
Fields that pydantic-ai regenerates on every attempt (message-level
``timestamp``/``run_id``/``conversation_id`` and part-level ``timestamp``)
are excluded from the fingerprint. Requests that cannot be serialized to
-JSON fingerprint as ``None``, which degrades that step to unverified
-positional replay (the pre-fingerprint behavior) rather than disabling
-caching.
+JSON fingerprint as ``None`` and re-run live instead of replaying without
Review Comment:
The comment on `_TRANSPORT_ONLY_SETTINGS` (line 66) still says an
unserializable setting would "silently disable replay verification for every
step", which was the old consequence. After this change it disables replay
entirely for the run, which makes that frozenset load-bearing:
`ModelSettings.timeout` being `float | httpx.Timeout` was already one member
that needed stripping, and any other non-JSON member now costs the whole
feature rather than just verification. Worth updating that comment along with
these docstrings.
##########
providers/common/ai/docs/operators/agent.rst:
##########
@@ -300,6 +300,9 @@ cache:
never replays responses that belong to a different conversation.
4. After successful completion, the cached steps are deleted.
+If a model request or tool call cannot be fingerprinted, that step runs live on
Review Comment:
"That step" undersells it. The realistic causes are a non-JSON value in
model settings (present on every request) or in the message history (which
stays there), so it's every model step from that point on, not one, and durable
execution ends up doing nothing for the run. I ran both through
`fingerprint_model_request` and every subsequent step came back `None`. Worth
saying that here so a full-price retry isn't a surprise.
--
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]
