Repository navigation
Expose the resolved LLM model name on a dedicated XCom key - #74272
Merged
Merged
Conversation
The model that actually answered a run was only reachable buried inside the decision record or the task logs. Downstream tasks, and the UI, need a stable first-class handle on which model ran, so publish it under a dedicated model_name XCom key from both the LLM and the agent operator.
pierrejeambrun
marked this pull request as ready for review
October 6, 2026 10:38
pierrejeambrun
requested review from
Lee-W,
ashb,
gopidesupavan and
kaxil
as code owners
October 6, 2026 10:38
1 task done
kaxil
reviewed
Oct 6, 2026
…e key A separate model_name XCom key meant two lookups to see what answered a run and what it cost. Push model_name as a field inside the usage XCom instead, so both live under one key -- also giving LLMOperator its own usage/cost reporting for the first time, which it computed already but never exposed on XCom at all.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 6, 2026
PR apache#74272 now folds model_name into the usage XCom payload instead of publishing it as its own key. Follow that here: one fetch instead of two, and the model-only case (an LLMOperator run) now also gets the token/cost grid, since LLMOperator publishes full usage too.
Reverts "Fold the resolved model name into the usage XCom instead of a separate key" per @kaxil's review -- a plain "usage" field name is too easy for a user's own XCom to collide with. Keep model_name on its own key, but namespace it (__AIRFLOW__COMMON_AI_MODEL_NAME__) so it can't collide with a user's own "model_name" XCom either. Giving LLMOperator its own usage/cost reporting is deferred to a separate PR.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 6, 2026
PR apache#74272 reverted the usage-XCom consolidation per review -- model_name is back on its own key, now namespaced (__AIRFLOW__COMMON_AI_MODEL_NAME__) to avoid colliding with a user's own XCom. Follow that back here: two XCom lookups instead of one, and the model-only state (no usage grid) returns for operators that don't also publish usage.
A code comment should carry the reasoning, not credit whoever raised it.
1 task done
kaxil
reviewed
Oct 7, 2026
LLMBranchOperator overrides execute() without calling super().execute(), so it never published the model_name XCom despite inheriting from LLMOperator -- @task.llm_branch silently never got the key the PR description promised. Moving the push into a helper both execute()s call keeps the two in sync going forward. Also documents the model_name XCom in observability.rst next to run_id and usage, since the "Scope" bullet there only mentioned AgentOperator and nothing described which operators actually publish this key.
Scoping back to AgentOperator and LLMOperator only for this PR; LLMBranchOperator (and the other LLMOperator subclasses that override execute() without calling super()) will get model_name support in a follow-up PR instead of a partial fix here. This reverts commit af02fc2.
Addresses the other half of kaxil's review: the "Scope" bullet in observability.rst only covered run_id and usage, with nothing describing which operators publish model_name at all. Calls out that the four LLMOperator subclasses overriding execute() don't publish it yet.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 8, 2026
PR apache#74272 now folds model_name into the usage XCom payload instead of publishing it as its own key. Follow that here: one fetch instead of two, and the model-only case (an LLMOperator run) now also gets the token/cost grid, since LLMOperator publishes full usage too.
pierrejeambrun
added a commit
to astronomer/airflow
that referenced
this pull request
Oct 8, 2026
PR apache#74272 reverted the usage-XCom consolidation per review -- model_name is back on its own key, now namespaced (__AIRFLOW__COMMON_AI_MODEL_NAME__) to avoid colliding with a user's own XCom. Follow that back here: two XCom lookups instead of one, and the model-only state (no usage grid) returns for operators that don't also publish usage.
kaxil
approved these changes
Oct 8, 2026
kaxil
left a comment
kaxil
left a comment
Member
There was a problem hiding this comment.
LGTM. The description still says the key is model_name; it's __AIRFLOW__COMMON_AI_MODEL_NAME__ now. Could the "Model name" bullet in observability.rst show that literal key with an xcom_pull example like the run_id one? A Jinja template or REST call can't import the constant.
A Jinja template or REST call can't import MODEL_NAME_XCOM_KEY, so the docs need the actual key and an xcom_pull example like the run_id one.
pierrejeambrun
force-pushed
the
common-ai-model-name-xcom
branch
from
October 8, 2026 13:33
56b80bd to
4ba803e
Compare
1 task done
7 of 36 tasks
6 of 19 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The model that actually answered a run was only reachable buried inside the
decisionXCom record (forLLMOperator) or the task logs. This publishes it under a dedicated, stable__AIRFLOW__COMMON_AI_MODEL_NAME__XCom key (namespaced so it can't collide with a user's own XCom) from both the LLM and the agent operator, so downstream tasks — and the UI — can read which model ran without parsing the decision record.LLMOperator: push the resolvedresponse.model_nameunder__AIRFLOW__COMMON_AI_MODEL_NAME__.AgentOperator: push it in_emit_run_metadata, alongside the existingrun_id/usagerun-metadata XComs.The resolved name is the model that actually responded (e.g.
claude-sonnet-4-5-20250929), which can differ from the configuredmodel_idwhen the model comes from the connection'sextra.modelor a fallback.First of a two-PR series — the follow-up adds a UI panel that surfaces this as a first-class "Model" badge on the task-instance view.
No change to
LLMBranchOperator/LLMSchemaCompareOperator/LLMSQLQueryOperator/LLMFileAnalysisOperator-- each of those overridesexecute()with its own implementation rather than callingLLMOperator's viasuper(), so this doesn't reach them. Support for those four is upcoming work.Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 4.8) following the guidelines