Skip to content

Add require_approval preflight check to @task.llm_schema_compare - #71688

Merged
gopidesupavan merged 1 commit into
apache:mainfrom
ColtenOuO:common-ai-schema-compare-require-approval
Sep 4, 2026
Merged

gopidesupavan merged 1 commit into
apache:mainfrom
ColtenOuO:common-ai-schema-compare-require-approval

Conversation

@ColtenOuO

Copy link
Copy Markdown
Contributor

Sumarry

@task.llm, @task.llm_branch, and @task.llm_sql all reject a non-string Sequence[UserContent] prompt combined with require_approval=True before the LLM ever runs, raising a TypeError that names the decorator the caller actually used (e.g. @task.llm_branch: ...).

@task.llm_schema_compare was missing this decorator-level check. The same misuse still fails before any LLM call (the underlying operator has its own fallback guard), but the error instead names the internal _LLMSchemaCompareDecoratedOperator class -- an implementation detail the caller never wrote, which is confusing when debugging.

This adds the same reject_sequence_with_unsupported_feature preflight call that the other three decorators already use, so all four give a consistent, decorator-named error message.

Reproduce

Reproduced by running two tasks against the same misuse (require_approval=True with a Sequence[UserContent] prompt) -- one via @task.llm_branch, one via @task.llm_schema_compare -- and comparing the task logs.

from __future__ import annotations

from pydantic_ai.messages import ImageUrl

from airflow.providers.standard.operators.empty import EmptyOperator
from airflow.sdk import DAG, task

with DAG(
    dag_id="demo_require_approval_inconsistency",
    schedule=None,
    catchup=False,
):

    @task.llm_branch(
        llm_conn_id="demo_llm_conn",
        require_approval=True,
    )
    def branch_with_bad_prompt():
        return ["Compare these schemas:", ImageUrl(url="https://example.com/x.png")]

    @task.llm_schema_compare(
        llm_conn_id="demo_llm_conn",
        db_conn_ids=["demo_db_conn_a", "demo_db_conn_b"],
        table_names=["demo_table"],
        require_approval=True,
    )
    def schema_compare_with_bad_prompt():
        return ["Compare these schemas:", ImageUrl(url="https://example.com/x.png")]

    branch_with_bad_prompt() >> EmptyOperator(task_id="downstream_placeholder")
    schema_compare_with_bad_prompt()

Before this change:

INFO - Failure caused by _LLMSchemaCompareDecoratedOperator: require_approval=True is not supported with a non-string prompt (got list). The approval review body renders the prompt as text; passing a Sequence[UserContent] would expose object reprs (and any embedded bytes) in the human review UI. Return a str prompt, or disable require_approval.
INFO - Failure caused by @task.llm_branch: Sequence[UserContent] prompts are not supported with require_approval=True. Return a str prompt, or disable require_approval.

Note the first line names _LLMSchemaCompareDecoratedOperator -- an internal class the caller never wrote -- while the second names @task.llm_branch, the decorator actually used.

After this change:

The same misuse via @task.llm_schema_compare instead raises:

@task.llm_schema_compare: Sequence[UserContent] prompts are not supported with require_approval=True. Return a str prompt, or disable require_approval.

now consistent with the other three decorators.

@task.llm, @task.llm_branch, and @task.llm_sql all reject a non-string
Sequence[UserContent] prompt combined with require_approval=True before
the LLM runs, with an error naming the decorator the caller actually used.
@task.llm_schema_compare was missing this decorator-level check, so the
same misuse instead fell through to the underlying operator's own guard --
which names the internal _LLMSchemaCompareDecoratedOperator class rather
than the decorator, confusing anyone who only ever wrote
@task.llm_schema_compare.
@gopidesupavan

Copy link
Copy Markdown
Member

no harm having this check, but does anyone really uses images to compare schema 🤔 :)

@gopidesupavan
gopidesupavan merged commit 6d4808b into apache:main Sep 4, 2026
83 checks passed
@ColtenOuO

Copy link
Copy Markdown
Contributor Author

no harm having this check, but does anyone really uses images to compare schema 🤔 :)

Yeah... I don't think anyone would actually do this either, but since users can somehow pull off such crazy edge cases, I figured we might as well keep the error feedback consistent across all four decorators 🤣...

imrichardwu pushed a commit to imrichardwu/airflow that referenced this pull request Sep 11, 2026
…che#71688)

@task.llm, @task.llm_branch, and @task.llm_sql all reject a non-string
Sequence[UserContent] prompt combined with require_approval=True before
the LLM runs, with an error naming the decorator the caller actually used.
@task.llm_schema_compare was missing this decorator-level check, so the
same misuse instead fell through to the underlying operator's own guard --
which names the internal _LLMSchemaCompareDecoratedOperator class rather
than the decorator, confusing anyone who only ever wrote
@task.llm_schema_compare.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants