Copilot commented on code in PR #59691:
URL: https://github.com/apache/airflow/pull/59691#discussion_r3066488039


##########
airflow-core/src/airflow/models/taskinstance.py:
##########
@@ -2260,6 +2268,54 @@ def _visit_relevant_relatives_for_mapped(mapped_tasks: 
Iterable[tuple[str, int]]
     return visited
 
 
+def resolve_placeholder_map_index(
+    *,
+    task: Operator,
+    relative: Operator,
+    map_index: int,
+    run_id: str,
+    session: Session,
+) -> int | None:

Review Comment:
   `resolve_placeholder_map_index` is a new top-level helper in 
`airflow.models.taskinstance`, but it is only used internally from 
`_get_relevant_map_indexes`. In this module, similar internal helpers are 
prefixed with `_` (e.g. `_get_relevant_map_indexes`, 
`_find_common_ancestor_mapped_group`). To avoid unintentionally expanding the 
public import surface of `airflow.models.taskinstance`, consider renaming this 
to `_resolve_placeholder_map_index` (and updating the call site).



##########
airflow-core/tests/unit/models/test_taskinstance.py:
##########
@@ -3102,6 +3102,95 @@ def g(v):
     assert result == expected
 
 
+def test_downstream_placeholder_handles_upstream_post_expansion(dag_maker, 
session):
+    """
+    Test dynamic task mapping behavior when an upstream placeholder task
+    (map_index = -1) has been replaced by the first expanded task
+    (map_index = 0).
+
+    This verifies that trigger rule evaluation correctly resolves relevant
+    upstream map indexes both when referencing the original placeholder
+    and when referencing the first expanded task instance.
+    """
+
+    with dag_maker(session=session) as dag:
+
+        @task
+        def get_mapping_source():
+            return ["one", "two", "three"]
+
+        @task
+        def mapped_task(x):
+            output = f"{x}"
+            return output
+
+        @task_group(prefix_group_id=False)
+        def the_task_group(x):
+            start = MockOperator(task_id="start")
+            upstream = mapped_task(x)
+
+            # Plain downstream inside task group (no mapping source).

Review Comment:
   The comment "Plain downstream inside task group (no mapping source)." is a 
bit misleading: when the task group is dynamically mapped, this downstream task 
still participates in the group's mapping context (even if it doesn't directly 
consume the mapped argument). Consider rewording to clarify it’s not *directly* 
mapped by an expand input, but is still mapped via the mapped task group.
   ```suggestion
               # Downstream task inside the task group that does not directly
               # consume the expand input, but is still mapped via the mapped
               # task group context.
   ```



-- 
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]

Reply via email to