kaxil commented on code in PR #72947:
URL: https://github.com/apache/airflow/pull/72947#discussion_r3990782870
##########
providers/common/ai/docs/retry_policies.rst:
##########
@@ -83,6 +83,26 @@ If the LLM call fails (provider down, timeout, bad
credentials), the policy
falls back to ``fallback_rules`` if configured, or to the task's standard
retry behaviour.
+What the model can and cannot do
+--------------------------------
+
+The model answers two questions: retry or not, and how long to wait. It is
+given no tools and there is no way to attach any, so it cannot run code, call
an
+API, read a connection, or reach your data. It sees only the exception's class
+name, the exception message (after redaction and truncation), and the attempt
+count. It returns four fields: ``category``, ``should_retry``,
``suggested_delay_seconds``,
+and ``reasoning``. Of the four fields it returns, only ``should_retry`` and
+``suggested_delay_seconds`` affect the run. ``category`` and ``reasoning`` are
+recorded but nothing branches on them.
Review Comment:
"Recorded" splits by branch, and not in the direction people will expect. A
RETRY carries `category: reasoning` through to `retry_reason` on the task
instance row (truncated to 500, as the section below notes), but a FAIL only
logs it: the terminal-state payload (`TITerminalStatePayload`) has no reason
field, and `retry_reason` is cleared when the attempt enters RUNNING. So the
decisions most worth auditing later, the ones that ended the task early, leave
nothing behind outside the task log.
##########
providers/common/ai/docs/retry_policies.rst:
##########
@@ -83,6 +83,26 @@ If the LLM call fails (provider down, timeout, bad
credentials), the policy
falls back to ``fallback_rules`` if configured, or to the task's standard
retry behaviour.
+What the model can and cannot do
+--------------------------------
+
+The model answers two questions: retry or not, and how long to wait. It is
+given no tools and there is no way to attach any, so it cannot run code, call
an
+API, read a connection, or reach your data. It sees only the exception's class
+name, the exception message (after redaction and truncation), and the attempt
Review Comment:
Two things are missing from this "only" list. The `instructions` system
prompt rides along on every call
(`create_agent(instructions=self.instructions)`), and the bullet below plus the
whole Custom instructions section depend on the model reading it, so "sees
only" reads as a contradiction with the rest of the page. The prompt is also
`attempt {try_number} of {max_tries}`, so the model sees the ceiling and not
just the current attempt, which is what makes examples like "'Authentication
token has expired' AFTER multiple retries -> auth, do NOT retry" work.
Something like "beyond your instructions, it sees only ..." would cover both.
##########
providers/common/ai/docs/retry_policies.rst:
##########
@@ -83,6 +83,26 @@ If the LLM call fails (provider down, timeout, bad
credentials), the policy
falls back to ``fallback_rules`` if configured, or to the task's standard
retry behaviour.
+What the model can and cannot do
+--------------------------------
+
+The model answers two questions: retry or not, and how long to wait. It is
+given no tools and there is no way to attach any, so it cannot run code, call
an
+API, read a connection, or reach your data. It sees only the exception's class
+name, the exception message (after redaction and truncation), and the attempt
+count. It returns four fields: ``category``, ``should_retry``,
``suggested_delay_seconds``,
+and ``reasoning``. Of the four fields it returns, only ``should_retry`` and
+``suggested_delay_seconds`` affect the run. ``category`` and ``reasoning`` are
+recorded but nothing branches on them.
+
+Two limits are worth knowing about:
+
+* RETRY cannot give a task more attempts than ``retries`` allows. FAIL,
though, ends the task
+ straight away even when attempts were left, so a wrong classification costs
+ the task the retries it would otherwise have had.
+* ``suggested_delay_seconds`` is used as returned, with no upper limit. If
particular delays
Review Comment:
The core page's interaction table is a step sharper than this bullet:
`retry_delay` / `retry_exponential_backoff` / `max_retry_delay` are "used when
the policy returns DEFAULT or when `RetryDecision.retry_delay` is None"
(core-concepts/tasks.rst). Both halves are worth pulling in here, since this is
the page someone worried about boundaries will read. A task's own
`max_retry_delay` does not clamp the model's delay, because
`next_retry_datetime()` returns on `retry_delay_override` before it reaches the
clamp. And 0 or a negative value is not used as returned at all: it leaves
`RetryDecision.retry_delay` as None, so the task's `retry_delay` applies (300s
by default). A model told to retry immediately gets five minutes.
--
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]
