Argo: conditional-join depends computed as && instead of || depending on step-name alphabetical order

[m-hemery]

Follow-up from a report on the Outerbounds community Slack — sharing the full trace + repro here.

Summary

We have a flow with an outer condition= switch where one branch reaches a merge step directly, and the other branch goes through a step that loops back on itself (self-loop condition=) before reaching the same merge step. Only one branch ever runs per execution, so we'd expect merge's depends to be A.Succeeded || B.Succeeded. Depending on how the steps are named, we instead sometimes get (A.Succeeded) && (B.Succeeded), so merge is permanently Omitted (omitted: depends condition not met) whichever branch actually runs.

This is fully reproducible and name-dependent: two flows with the exact same graph shape and decorators, differing only in two step names, compile to different depends operators. We just re-verified this from scratch on metaflow==2.19.36 (latest release as of today, in a brand new venv, unrelated to our original environment), so it isn't specific to our setup or already fixed by something else.

Our working hypothesis for the root cause

We instrumented _parse_conditional_branches (metaflow/plugins/argo/argo_workflows.py) locally to understand the mechanism. Below is our best explanation, but we want to be upfront that this is our interpretation of what we observed while instrumenting, not something confirmed by a maintainer — happy to be corrected on the details as long as the underlying repro/behavior below is accepted as a real bug.

1. Every switch-type node gets visited twice: once via propagation from its parent switch, and once independently as its own "root".
The outer loop that seeds the traversal is for n in self.graph: _visit(n, []) — it calls _visit for every node in the graph, not just start. For a regular step this is a no-op (the function returns immediately unless the incoming context is non-empty), but for a split-switch-type node the guard is bypassed regardless of incoming context, so it always proceeds — meaning every switch gets visited a second time, from scratch, with an empty context, in addition to being reached "properly" via propagation from whatever switch encloses it.

2. self.graph's iteration order is alphabetical by step name.
This traces back to graph.py's FlowGraph._create_nodes, which builds the node dict via for element in dir(flow): ... — dir() always returns names alphabetically. We logged the traversal order for two graphs that differ only in step names — the visit order was after_poll, end, long_path, merge, poll, shortcut, start for one naming and end, long_path, merge, shortcut, start, step_b_loop, step_c for the other; both are exactly alphabetical.

3. Whichever of the two visits (propagated vs. independent-root) happens last in that alphabetical order wins, because node_conditional_parents[name] = ... is a plain overwrite, not a merge.
If the inner switch's name sorts before the outer switch's name (e.g. poll before start), the outer switch's full/correct propagation runs last and its value survives. If the inner switch's name sorts after the outer switch's name (e.g. step_b_loop after start), the inner switch's own from-scratch, empty-context visit runs last and overwrites the correct value with an incomplete one — silently dropping the outer switch from the node's tracked ancestry. From there, merge (downstream of the inner switch) never gets credited as a valid join for the outer switch, and the compiler falls back to default AND semantics for its depends.

This hypothesis is at least consistent with what we observed: renaming only step_b_loop to aaa_loop (nothing else changed, same shape, same decorators) — moving it to sort before start — fixes merge's depends back to ||.

Minimal repro

from metaflow import FlowSpec, step


class ConditionalJoinBugFlow(FlowSpec):
    @step
    def start(self):
        self.use_shortcut = True
        self.next(
            {True: self.shortcut, False: self.long_path},
            condition="use_shortcut",
        )

    @step
    def shortcut(self):
        self.next(self.merge)

    @step
    def long_path(self):
        self.next(self.step_b_loop)

    @step
    def step_b_loop(self):
        self.should_continue = False
        self.next(
            {True: self.step_b_loop, False: self.step_c},
            condition="should_continue",
        )

    @step
    def step_c(self):
        self.next(self.merge)

    @step
    def merge(self):
        self.next(self.end)

    @step
    def end(self):
        print("done")


if __name__ == "__main__":
    ConditionalJoinBugFlow()

Compiled with:

python conditional_join_bug_flow.py --environment=pypi argo-workflows --name repro create --only-json

Resulting DAG (relevant task only):

merge -> (step-c.Succeeded) && (shortcut.Succeeded)

should be ||. Renaming step_b_loop/step_c to e.g. poll/after_poll (or anything that sorts before start) makes it compile correctly.

On our real flow (6 nested switches on one branch), this generalizes: the outer branch's end/join node only stays correctly attributed to the outermost switch if every switch between it and that outer switch happens to sort alphabetically before it — which is fragile and not something we want to rely on as a fix, but is at least consistent with what we're seeing.

Note

This looks distinct from #3230 / #3043, which are about runtime input-paths selection once a join is reached. This one is a compile-time bug in how conditional-join depends expressions get generated in the first place — the &&/|| operator itself is wrong before the workflow even starts running.

[saikonen]

Thank you for the detailed report 👌 Looking to get this fixed as soon as possible.