pankajkoti commented on code in PR #74737:
URL: https://github.com/apache/airflow/pull/74737#discussion_r4283142816
##########
providers/common/ai/docs/approval_gates.rst:
##########
@@ -21,15 +21,145 @@ Approval gates for LLM operators
================================
.. seealso::
- This page covers the one-shot approve, edit or reject gate on
``LLMOperator`` and its
- subclasses. For a multi-round review loop on ``AgentOperator`` with a chat
UI and REST
- API, see :doc:`hitl_review`.
+ For a multi-round review loop on ``AgentOperator`` with a chat UI and REST
API, see
+ :doc:`hitl_review`.
-Set ``require_approval=True`` to pause the task after the LLM generates its
-output and wait for a human reviewer to approve or reject it via the Airflow
-HITL interface. Optionally allow the reviewer to edit the output before
-approving with ``allow_modifications=True``, and set a deadline with
-``approval_timeout``.
+Set ``require_approval=True`` and the task stops after the model answers. A
person
+reads the output in the Airflow UI and approves it, edits it, or rejects it,
and only an
+approved answer reaches the tasks downstream. Use it where a wrong answer is
costly to
+publish or act on. To review each tool call an agent makes rather than its
final answer,
+see :doc:`tool_approval`.
+
+Approval gates need Airflow 3.1 or later. On an older Airflow version the
+operator raises ``AirflowOptionalProviderFeatureException:
require_approval=True needs Airflow 3.1+.``
+when it is constructed, so the Dag file
+fails to import, and with it every Dag defined in that file. A dynamically
mapped
Review Comment:
Minor, formatting only. The exception name is on one long line (about 102
characters) and the sentence then breaks after "when it is constructed, so the
Dag file". Re-wrapping would read better:
```suggestion
Approval gates need Airflow 3.1 or later. On an older Airflow version the
operator
raises ``AirflowOptionalProviderFeatureException: require_approval=True
needs Airflow 3.1+.``
when it is constructed, so the Dag file fails to import, and with it every
Dag
defined in that file. A dynamically mapped
```
##########
providers/common/ai/docs/approval_gates.rst:
##########
@@ -54,56 +184,71 @@ text. A model with a serializer that cannot be reversed,
or a required field ex
the dump, shows text that does not validate as shown, so write an edit in the
form the model
accepts.
-Human-in-the-loop review needs Airflow 3.1+, whether ``require_approval`` or a
-``decision_policy`` with ``on_uncertain="review"`` opens it. On an older
Airflow version the
-operator raises ``AirflowOptionalProviderFeatureException`` when it is
constructed, so the Dag file
-fails to import, and with it every Dag defined in that file. A dynamically
mapped
-task (``.expand()``) is only constructed when it runs, so there the same error
-surfaces as a task failure -- still before the model is called.
+Timeouts
+--------
-When ``approval_timeout`` expires without a review, the task fails by default.
-Set ``on_approval_timeout="approve"`` to return the generated output instead,
so
-an unattended pipeline keeps moving. ``"reject"`` answers the review with a
-rejection, which still fails this operator; only
-:class:`~airflow.providers.common.ai.operators.llm_branch.LLMBranchOperator`
-turns a rejection into a downstream skip. The chosen option is also
-pre-highlighted as the default in the review form, so ``"reject"`` makes
-Reject the primary button:
+``approval_timeout`` bounds the wait; the default ``None`` waits indefinitely.
On Airflow
+3.3 and later the scheduler resolves expired reviews, checking every
+``[scheduler] trigger_timeout_check_interval`` seconds (15 by default), so a
review can
+stay open that long past its deadline. On Airflow 3.1 and 3.2 the triggerer
enforces the
+deadline. ``on_approval_timeout="approve"`` keeps an
+unattended pipeline moving with output nobody reviewed. ``"reject"`` still
fails this
+operator; only
+:class:`~airflow.providers.common.ai.operators.llm_branch.LLMBranchOperator`
turns a
+rejection into a downstream skip. The chosen option is also pre-highlighted as
the
+default in the review form, so ``"reject"`` makes Reject the primary button.
+
+This example approves the output if nobody answers within 24 hours, and logs
each new
+review through a notifier:
.. exampleinclude::
/../../ai/src/airflow/providers/common/ai/example_dags/example_llm.py
:language: python
:start-after: [START howto_operator_llm_approval]
:end-before: [END howto_operator_llm_approval]
-A pending review is not surfaced as a notification. Pass
+It runs as-is with the same ``pydanticai_default`` connection.
+
+Notifying reviewers
+-------------------
+
+Airflow sends no notification when a review opens. Pass
``approval_notifiers`` to tell the reviewers about it through any Airflow
notifier (Slack, email, ...), the way
:class:`~airflow.providers.standard.operators.hitl.HITLOperator` does with
``notifiers``. The notifiers run once the review is open and can reference
the review ``{{ task.subject }}`` and ``{{ task.body }}`` in their templates,
-as the example above does. The ``@task.llm`` decorator and the operator
-subclasses accept the same parameter. A notifier whose delivery fails is
-logged and the task still waits for the review; a template error fails the
-task. A retry re-runs the LLM and re-notifies with the regenerated output,
-while the open review keeps the original subject and body.
+as the example under `Timeouts`_ does: its notifier writes the subject and
body to the
+task log, just before ``Pausing task as AWAITING_INPUT.`` The ``@task.llm``
decorator
+and the ``LLMOperator`` subclasses (``LLMBranchOperator``,
``LLMSQLQueryOperator``,
+``LLMSchemaCompareOperator``, ``LLMFileAnalysisOperator``) accept the same
parameter. A notifier that raises is logged as
+``Approval notifier <notifier> failed; the review stays open`` and the task
still waits for
+the review; a Jinja ``TemplateError`` in a notifier's template fails the task.
A retry re-runs the LLM and re-notifies with the regenerated output; what the
+reopened review shows depends on the Airflow version, as described under
+`What each answer does`_.
Review Comment:
Minor, formatting only. This paragraph wraps unevenly. The subclass list
line (about 122 characters) and the `TemplateError` line (about 157 characters)
run well past the width used in the rest of the file, and the sentence "what
the reopened review shows..." is cut mid-clause. Re-wrapping keeps the source
easy to diff later:
```suggestion
and the ``LLMOperator`` subclasses (``LLMBranchOperator``,
``LLMSQLQueryOperator``,
``LLMSchemaCompareOperator``, ``LLMFileAnalysisOperator``) accept the same
parameter. A notifier that raises is logged as
``Approval notifier <notifier> failed; the review stays open`` and the task
still
waits for the review. A Jinja ``TemplateError`` in a notifier's template
fails the
task. A retry re-runs the LLM and re-notifies with the regenerated output.
What the
reopened review shows depends on the Airflow version, as described under
`What each answer does`_.
```
##########
providers/common/ai/docs/approval_gates.rst:
##########
@@ -21,15 +21,145 @@ Approval gates for LLM operators
================================
.. seealso::
- This page covers the one-shot approve, edit or reject gate on
``LLMOperator`` and its
- subclasses. For a multi-round review loop on ``AgentOperator`` with a chat
UI and REST
- API, see :doc:`hitl_review`.
+ For a multi-round review loop on ``AgentOperator`` with a chat UI and REST
API, see
+ :doc:`hitl_review`.
-Set ``require_approval=True`` to pause the task after the LLM generates its
-output and wait for a human reviewer to approve or reject it via the Airflow
-HITL interface. Optionally allow the reviewer to edit the output before
-approving with ``allow_modifications=True``, and set a deadline with
-``approval_timeout``.
+Set ``require_approval=True`` and the task stops after the model answers. A
person
+reads the output in the Airflow UI and approves it, edits it, or rejects it,
and only an
+approved answer reaches the tasks downstream. Use it where a wrong answer is
costly to
+publish or act on. To review each tool call an agent makes rather than its
final answer,
+see :doc:`tool_approval`.
+
+Approval gates need Airflow 3.1 or later. On an older Airflow version the
+operator raises ``AirflowOptionalProviderFeatureException:
require_approval=True needs Airflow 3.1+.``
+when it is constructed, so the Dag file
+fails to import, and with it every Dag defined in that file. A dynamically
mapped
Review Comment:
might need to verify it once if considered and applied on the wrapping
--
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]
