kaxil commented on code in PR #76244:
URL: https://github.com/apache/airflow/pull/76244#discussion_r4437635869
##########
providers/common/ai/docs/operators/llm.rst:
##########
@@ -206,6 +206,19 @@ With structured output:
:start-after: [START howto_decorator_llm_structured]
:end-before: [END howto_decorator_llm_structured]
+The returned prompt is rendered as a Jinja template, like the operator's
``prompt``
+argument, so ``{{ ds }}`` and other template variables work in it. Text you
insert into
+the returned string yourself, for example with an f-string, is rendered as
part of the
+template too. To include text that comes from a function argument, such as a
document or
+an upstream task's output, reference the argument by name in the template
instead of
+formatting it into the string. The function's arguments are available when the
prompt is
Review Comment:
This only holds for arguments passed by keyword. The decorators merge
`op_kwargs` into the context (`context_merge(context, self.op_kwargs)`), but a
positional call puts the value in `op_args`, which never gets there, and
neither does a parameter left at its default. So applying this to the two
examples right above (`summarize("...")`, `extract("...")`) raises
`UndefinedError: 'text' is undefined` under the default `StrictUndefined`; the
pipeline example only works because it uses `.expand(ticket=...)`.
Could the decorators merge the bound call instead, e.g.
`inspect.signature(self.python_callable).bind_partial(*self.op_args,
**self.op_kwargs).arguments` (without `apply_defaults()`, so a `ti=None`
default can't shadow the real `ti`)? Or say keyword arguments only here. Either
way the f-string examples above and the Usage docstrings in `llm.py`,
`agent.py` and `llm_branch.py` still do what this paragraph says not to.
--
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]
