fix: prevent duplicate DAGTask names in foreach + split-switch flows by odncode · Pull Request #3204 · Netflix/metaflow

odncode · 2026-05-19T08:55:25Z

Summary

Fixes the duplicate DAGTask name bug reported in #3196.

When a split-switch's matching_conditional_join resolves to the same node as a foreach's matching_join, _visit in _dag_templates emitted two DAGTasks with the same name. Argo rejects this at submit time with:

templates.<flow> sorting failed: duplicated nodeName <step>

Root Cause

The foreach branch appends a join_foreach_task to dag_tasks but did not add the matching_join name to the seen list.

When a later traversal arrives at the same node via the split-switch handler, the seen check does not fire, and a second DAGTask is emitted.

Fix

One-line change:

seen.append(self.graph[node.matching_join].name)

added immediately after:

dag_tasks.append(join_foreach_task)

Closes #3196

greptile-apps · 2026-05-19T08:59:38Z

Greptile Summary

This PR adds a one-line guard to _dag_templates._visit in argo_workflows.py that marks matching_join as seen immediately after the foreach handler creates a DAGTask for it, preventing the split-switch handler from emitting a second identical DAGTask for the same join node.

Fix (argo_workflows.py): seen.append(self.graph[node.matching_join].name) is inserted right after dag_tasks.append(join_foreach_task), which is the correct and minimal location to close the deduplication gap.
Regression test: The new flow (foreach_split_switch_dedup_flow.py) and its integration test are added, but the flow's "skip" branch routes directly to end, causing fan_gate's matching_conditional_join to resolve to end rather than join_step. The duplicate-emission code path is therefore never reached, and the test provides no actual coverage for the fix.

Confidence Score: 4/5

The production fix is correct, but the regression test does not exercise the bug path it is meant to guard.

The change to argo_workflows.py is minimal and logically sound. The accompanying test flow never reaches the duplicate-emission code path because its skip-to-end branch causes the split-switch conditional join to resolve to end rather than join_step, so the test passes identically with or without the fix.

test/ux/core/flows/dag/foreach_split_switch_dedup_flow.py needs to be reworked so that all conditional branches of the split-switch converge at the foreach matching join step.

Important Files Changed

Filename	Overview
metaflow/plugins/argo/argo_workflows.py	One-line fix: adds `matching_join` to `seen` after appending `join_foreach_task`, preventing the split-switch handler from emitting a duplicate DAGTask for the same join node. The logic is correct.
test/ux/core/flows/dag/foreach_split_switch_dedup_flow.py	New flow intended as a regression test, but the skip-to-end branch means `fan_gate`'s `matching_conditional_join` resolves to `end`, not `join_step`. The duplicate-emission path is never triggered, so the test passes both before and after the fix.
test/ux/core/test_argo_compilation.py	New integration test using the Deployer API; correctly structured to check for duplicate DAGTask names, but inherits the test flow's limitation of not exercising the actual bug path.

_{Reviews (6): Last reviewed commit: "test: move argo foreach dedup regression..." | Re-trigger Greptile}

saikonen · 2026-05-27T21:58:44Z

the test cases need some polish. As they are, it is unnecessarily complex for one test setup. Consider the following as an alternative

using Deployer and passing the --only-json flag to it for the argo-workflows implementation. This will give you the same DAG spec that you can do the checks for

instead of inlining the test flow, simply add it as a separate asset, as the deployer can work with filenames directly.

codecov · 2026-05-27T23:14:14Z

Codecov Report

❌ Patch coverage is 0% with 1 line in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (master@90ea1d3). Learn more about missing BASE report.

Files with missing lines	Patch %	Lines
metaflow/plugins/argo/argo_workflows.py	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #3204   +/-   ##
=========================================
  Coverage          ?   28.33%           
=========================================
  Files             ?      381           
  Lines             ?    52369           
  Branches          ?     9247           
=========================================
  Hits              ?    14841           
  Misses            ?    36579           
  Partials          ?      949

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

vivekkhimani · 2026-05-29T17:34:12Z

@odncode thanks for doing this! i'd love to see this go in, so let me know if i can help get it across the finish line

When a split-switch's matching_conditional_join resolves to the same node as a foreach's matching_join, _visit in _dag_templates emitted two DAGTasks with the same name. Argo rejects this at submit time with: templates.<flow> sorting failed: duplicated nodeName <step> Root cause: the foreach branch appended a join DAGTask but did not add the matching_join name to the seen list, allowing a later traversal via the split-switch handler to emit a second DAGTask for the same node. Fix: add seen.append(self.graph[node.matching_join].name) immediately after the foreach join task is appended. Includes a regression test that compiles a minimal repro flow and checks for duplicate task names in the generated Argo JSON. Closes Netflix#3196

Addresses review feedback: the subprocess approach always skipped in CI due to cloud datastore requirements. The new test builds a minimal graph stand-in and calls _dag_templates() directly, consistent with the existing test patterns in test_argo_workflows_cli.py.

Fixed dunder method dispatch issue (class-level not instance-level). Added enable_heartbeat_daemon attribute needed by _dag_templates. Verified locally: new test passes, all 16 existing tests still pass.

Previous mock graph did not trigger the duplicate-emission condition because _parse_conditional_branches requires real graph attributes. Now uses Metaflow's FlowGraph to parse the repro flow directly, bypassing CLI and cloud datastore requirements. Verified: test FAILS without the fix (detects duplicate 'join-step'), PASSES with it. All 16 existing tests unaffected.

greptile-apps · 2026-05-31T18:08:13Z

Want your agent to iterate on Greptile's feedback? Try greploops.

greptile-apps Bot reviewed May 19, 2026

View reviewed changes

Comment thread test/unit/test_argo_foreach_dedup.py Outdated

Comment thread test/unit/test_argo_foreach_dedup.py Outdated

greptile-apps Bot reviewed May 19, 2026

View reviewed changes

Comment thread test/unit/test_argo_foreach_dedup.py Outdated

greptile-apps Bot reviewed May 19, 2026

View reviewed changes

Comment thread test/unit/test_argo_foreach_dedup.py Outdated

saikonen reviewed May 27, 2026

View reviewed changes

odncode added 4 commits May 31, 2026 08:56

test: use MockGraph class and add missing instance attributes

ec6b3b2

Fixed dunder method dispatch issue (class-level not instance-level). Added enable_heartbeat_daemon attribute needed by _dag_templates. Verified locally: new test passes, all 16 existing tests still pass.

talsperre force-pushed the fix-argo-duplicate-dag-task branch from 4574da4 to 3f3bf97 Compare May 31, 2026 09:01

test: move argo foreach dedup regression to ux

93399e4

talsperre force-pushed the fix-argo-duplicate-dag-task branch from 3f3bf97 to 93399e4 Compare May 31, 2026 18:04

greptile-apps Bot reviewed May 31, 2026

View reviewed changes

Comment thread test/ux/core/flows/dag/foreach_split_switch_dedup_flow.py

talsperre merged commit 9ca7692 into Netflix:master May 31, 2026
41 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: prevent duplicate DAGTask names in foreach + split-switch flows#3204

fix: prevent duplicate DAGTask names in foreach + split-switch flows#3204
talsperre merged 5 commits into
Netflix:masterfrom
odncode:fix-argo-duplicate-dag-task

odncode commented May 19, 2026 •

edited by talsperre

Loading

Uh oh!

greptile-apps Bot commented May 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saikonen May 27, 2026

Uh oh!

codecov Bot commented May 27, 2026 •

edited

Loading

Uh oh!

vivekkhimani commented May 29, 2026

Uh oh!

greptile-apps Bot commented May 31, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

odncode commented May 19, 2026 • edited by talsperre Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root Cause

Fix

Uh oh!

greptile-apps Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 4/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saikonen May 27, 2026

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

vivekkhimani commented May 29, 2026

Uh oh!

greptile-apps Bot commented May 31, 2026

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

odncode commented May 19, 2026 •

edited by talsperre

Loading

greptile-apps Bot commented May 19, 2026 •

edited

Loading

codecov Bot commented May 27, 2026 •

edited

Loading