YAshhh29 commented on code in PR #71575:
URL: https://github.com/apache/airflow/pull/71575#discussion_r4048361827
##########
providers/common/ai/src/airflow/providers/common/ai/durable/caching_model.py:
##########
@@ -112,6 +114,11 @@ async def request(
)
response = await self.wrapped.request(messages, model_settings,
model_request_parameters)
+ if fingerprint is None:
+ # Storing this would write an entry the guard above can never
accept,
+ # once per step, each write rewriting the whole cache blob.
+ log.debug("Durable: not caching model response that cannot be
verified on replay", step=step)
+ return response
Review Comment:
You're right, and I've reverted that half. "Executed" is the correct reading
— `agent.py` renders these as `executed %d new steps` and `agent.rst` describes
the line as replayed vs executed fresh, so suppressing the increment would
print `executed 0 new steps` right after paying for a model call. That hides
exactly the retry cost the new docs paragraph is there to surface.
Both counters now increment before the early return, and the two debug lines
distinguish a step that was cached from one that ran without being persisted.
The two tests that asserted `cached_model == 0` now assert `== 1`.
I kept the increment rather than adding a separate not-persisted counter —
the summary line reads to me as being about cost, not storage accounting. Happy
to add the second counter if you'd rather see both numbers.
That also settles the `save_tool_result` question I raised: under the
"executed" reading its silent skip isn't an inconsistency, so there's nothing
to fold in.
--
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]
