Skip to content

Align If execution with iterator frames - #10032

Open
JPPhoto wants to merge 5 commits into
invoke-ai:mainfrom
JPPhoto:iterate-if-frames
Open

JPPhoto wants to merge 5 commits into
invoke-ai:mainfrom
JPPhoto:iterate-if-frames

Conversation

@JPPhoto

@JPPhoto JPPhoto commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fix two If/For scheduling gaps:

  • An empty parent Iterate reached through a selected For final output now completes without creating If executions; downstream Collect receives an empty collection.
  • A direct branch For is gated by a wired per-item condition. Rejected frames do not run the For, its body, or ForReturn.

Examples:

  • Now valid: an Iterate provides a condition and collection; the collection feeds a For, whose output feeds If.true_input. True frames run the For; false frames use the fallback.
  • Now valid: a root condition selects between branches sharing the same inner Iterate axis. The If broadcasts across that axis.
  • Still rejected: a root condition selects between branches with different inner Iterate axes.
  • Still rejected: an outer per-item condition selects a branch with an extra inner Iterate, while the other branch lacks that axis. These shapes cannot be paired safely and raise UnsupportedIfIterationAxesError before execution.

Related Issues / Discussions

Also closes #9918 and closes #9922.

QA Instructions

  • uv run --no-sync pytest -n 4 tests/test_graph_execution_state.py tests/app/services/shared/test_execution_engine_differential.py tests/app/services/shared/test_graph_if_activation.py tests/app/services/shared/test_graph_if_dependencies.py tests/app/services/shared/test_graph_if_runtime.py tests/app/services/shared/test_graph_iterator_ancestry.py tests/app/services/shared/test_graph_materializer.py - 1,345 passed, 2 xfailed.
  • uv run --no-sync pytest -q tests/app/services/session_processor/test_if_processor_sqlite.py - 8 passed.
  • Ruff check/format and git diff --check passed.

Review

The final review found no remaining blockers. An over-broad root-axis rejection found earlier was removed; root-value broadcast and nested cases remain covered.

Compatibility / Rollout

Checklist

  • The PR has a short but descriptive title, suitable for a changelog
  • Meaningful regression coverage added / updated where needed; obsolete tests/code removed
  • Persisted-state and API changes include required migrations / compatibility validation
  • Relevant performance/efficiency opportunities considered; material claims have evidence
  • Material review findings resolved and relevant checks rerun
  • Documentation added / updated (if applicable)
  • Updated What's New copy (if doing a release after this PR)

View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@github-actions github-actions Bot added python PRs that change python files services PRs that change app services python-tests PRs that change python tests labels Oct 25, 2026
@JPPhoto
JPPhoto marked this pull request as ready for review October 25, 2026 16:26
@JPPhoto
JPPhoto requested a review from lstein October 25, 2026 16:26
@JPPhoto JPPhoto moved this to 7.0 Theme: Modular Design in Invoke - Community Roadmap Oct 25, 2026
@JPPhoto JPPhoto added the 7.0.0 label Oct 25, 2026
@JPPhoto
JPPhoto force-pushed the iterate-if-frames branch 2 times, most recently from aff22cf to d62878b Compare October 26, 2026 02:52

@lstein lstein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes for two new stalls, one silent wrong-value change and one over-rejection. Each was reproduced against main (6af2057) and the PR head (11ba2a7). The default and compatibility schedulers behave the same in every case.

Repros use tests.test_nodes helpers (create_edge, create_loop_linkage, AnyTypeTestInvocation, MarkedAnyTypeTestInvocation, FixedCollectionTestInvocation), plus CollectionConcatInvocation and BooleanInvocation. Edges are written as (src, field, dst, field). "Stall" means next() returns None while is_complete() is False.

Blockers

1. Stall with a wired root condition and both branches from For outputs under two nested Iterates (regression)

src(first=[[["a"]], [["b","c"]]]) -> it0 ; it0.item -> cc0.first ; cc0.collection -> it1.collection
for X in (T, F):
  it1.item -> forX.collection ; forX.item -> bodyX.value (Marked marker=X) ; bodyX.value -> retX.output ; loop_linkage(forX, retX)
forT.output_collection -> if.true_input ; forF.output_collection -> if.false_input
BooleanInvocation(value=True|False).value -> if.condition ; if.value -> collect.item
  • main: {(0,): [['Ta']], (1,): [['Tb','Tc']]}, and the False case gives the matching F values.
  • PR: stalls for both values. Nothing runs after it1.
  • Variant: one For whose output feeds true_input directly and false_input through a passthrough also stalls. In that variant the downstream Collect has already emitted {(0,): [], (1,): []}.
  • Unaffected: the literal-condition and one-Iterate versions complete.
  • Likely cause: the new wired-condition gating of a direct For combines with the new continue while the For is unfinalized (graph_materializer.py, selected-edge attachment). The If cannot get frames until the For finalizes, and the For waits for the If's verdict. That is the deadlock the removed comment in graph_if_activation.py described.

2. A shape this PR newly accepts stalls when its inner levels are empty

Use this PR's fixture _outer_condition_with_two_collects_graph(share_branch_axes=True) and change only nodes["source"].first to [[]], [[], []] or [[[]], [[]]].

  • PR: no If executions are created, which is correct, and collect emits {(0,): [], (1,): []}. But true_copy/false_copy are never retired, so is_complete() stays False.
  • Effect: in the session processor this leaves a queue item that never finishes.
  • main: an equivalent graph completes. main cannot load the fixture itself.

Should fix

3. Duplicate If executions at path (), and literal vs computed conditions disagree

src(first=[["c"],["d"],["e"]]) -> it0 ; it0.item -> for.collection ; for body Marked("F") -> ret ; loop_linkage(for, ret)
AnyType(value="R").value -> if.true_input ; for.output_collection -> if.false_input
BooleanInvocation(value=True).value -> if.condition ; if.value -> collect.item
  • PR: three If executions, all with iteration path (). collect gets ['R','R','R']. A non-Collect sink downstream runs once, so three executions collapse onto one frame.
  • main, and the PR with a literal True: one If, giving ['R'].
  • Empty Iterate: the selected root branch disappears. main gives a value; the PR gives [].

The same split shows up in the existing shape Iterate A([["a","b"],["c"]]) -> For -> mid -> if.true_input, with a root fallback on false_input:

condition main PR
literal False ['fb'] ['fb']
computed False ['fb'] ['fb','fb']
computed False, src=[] ['fb'] []
  • The PR updates test_root_condition_if_with_for_output_branch_under_iterate_completes to expect ["fb","fb"]. The new literal test expects ["fb"] for the same graph.
  • On #9789 we agreed to keep ['fb'] for this shape.
  • The README says the If broadcasts over parent frames through a selected For final output. Here it broadcasts while the For branch is unselected.
  • Whichever cardinality is chosen, literal and computed conditions with the same value should agree, and the If executions should carry distinct frame paths.

4. Validation rejects graphs that main runs correctly, and the rule depends on unrelated Collects

src(first=[[[["a"],["b","c"]]], [[["d"]],[[]]], [[["e"]]]]) -> it0 -> cc0 -> it1 -> cc1 -> it2
it2.item -> tm(Marked "t") -> tcol.item ; tcol.collection -> tp.first (Concat) ; tp.collection -> if.true_input
it2.item -> fm(Marked "f") -> fcol.item ; fcol.collection -> fp.first (Concat) ; fp.collection -> if.false_input
it0.item -> cond(FixedCollectionTestInvocation).value ; cond.condition -> if.condition ; if.value -> collect.item
  • main: runs it with correct per-frame values.
  • PR: UnsupportedIfIterationAxesError: ... deeper open iterator axes kept by a Collect: condition ['it0'], branch ['it0', 'it1'].
  • Both branches keep the same [it0, it1] axes with identical frames.
  • Unrelated Collect lifts the rejection: add AnyType("r") -> R(Collect), with R.collection feeding both tp.second and fp.second, and the PR accepts and runs the graph. _validate_collect_branch_axes collects Collects from nx.ancestors over the whole source graph, so any shared Collect satisfies it, not "a shared Collect in that chain" as the README states.
  • The other main-accepts/PR-rejects cases I found have asymmetric axes, and rejecting those looks like legitimate tightening.

5. RecursionError on long passthrough chains (low)

_get_edge_frame_axes now recurses through ordinary nodes.

  • Graph: a per-item wired If (src(["x",""]) -> it -> FixedCollection cond) whose true_input is a chain of 1000 AnyType nodes.
  • PR: raises RecursionError on the first next(). 800 nodes still work.
  • main: fine at 1200 nodes.
  • The Collect branch of the same function already uses an explicit stack; the passthrough path could do the same.

Checked and fine

  • Tests: tests/test_graph_execution_state.py, tests/app/services/shared and test_if_processor_sqlite.py pass: 1972 passed, 2 xfailed.
  • Benchmarks: the slow benchmarks in test_graph_execution_performance.py and test_for_scheduler_performance.py match main within about 1%.
  • Shape harness: I ran the 237-shape If/Iterate/For harness from #9789.
    • 229 shapes are unchanged.
    • 2 shapes that stall on main now complete.
    • 2 values change; these are item 3.
    • 4 differ only in random node IDs in error messages.
  • Direct For under a per-item condition: rejected frames skip the For, its body and ForReturn in every order tried. That includes all-true, all-false, an empty input, a For also consumed outside the If, and a For feeding both branches.
  • Other checks: error paths (set_node_error on the body, ret, For, condition or fallback), nested Ifs around a gated direct For, the A or B and C or D precedence in the activation gate, and the extra empty outer groups kept by the _get_collect_source_iterator_ids change are all fine.
  • Fuzz: a 3000-seed For/Iterate/Collect/If fuzz across the default and compatibility schedulers, plus a save/reload round-trip mid-run, showed no scheduler or round-trip divergence. It found 26 graphs that stall on main and complete on this PR.

@lstein

lstein commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator

Note on the review above: it names 11ba2a7, but the branch was rebased to d62878b while I was writing it. The graph engine files and their tests are identical between the two heads, and all five findings still reproduce on d62878b.

@JPPhoto
JPPhoto requested a review from lstein October 26, 2026 14:19

JPPhoto commented Oct 26, 2026

Copy link
Copy Markdown
Collaborator Author

@lstein Thanks for the review. The reported If-frame cases are resolved. I also cleaned up the axis traversal and Collect path inference to avoid repeated work.

@JPPhoto
JPPhoto force-pushed the iterate-if-frames branch 4 times, most recently from e086358 to 06c04a2 Compare October 27, 2026 01:02
Remove unused compatibility activation queries and consolidate Collect-axis and input-edge ordering logic.

Preserve scheduler-specific comparisons and For-final ancestry semantics.
Handle nested For-final and empty Collect frame scheduling with correct root selection cardinality.

Validate retained Collect axes, reject asymmetric branches, and keep passthrough traversal linear.
Reuse branch destination membership and retained Collect axes during frame inference.

Cover empty-path fallback with an edge-axis lookup regression.

@lstein lstein left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes. I re-reviewed 7968947 against its merge base with main (8359490). I also ran the cases against the PR as it was before your last two commits (685c027, called "pre-fix" below), to separate new regressions from older behavior.

Earlier findings: F1, F2, F4 and F5 are fixed. F3 is not.

New problems: four stalls that main doesn't have. All four come from f2732f1 / 7968947; both main and the pre-fix head complete every case below. Requesting changes for F3 and these four.

Repros use tests.test_nodes helpers. Edges are written as (src, field, dst, field). "Stall" means next() returns None while is_complete() is False. The default and compatibility schedulers behave the same unless noted.

F3 (still open): duplicate If executions at path ()

src(first=[["c"],["d"],["e"]]) -> it0 ; it0.item -> for.collection ; for body Marked("F") -> ret ; loop_linkage(for, ret)
AnyType(value="R").value -> if.true_input ; for.output_collection -> if.false_input
BooleanInvocation(value=True).value -> if.condition ; if.value -> collect.item
  • PR: 3 If executions, all with iteration path (), so collect gets ['R','R','R'].
  • main, and the PR with a literal True: one execution, ['R'].

The README now says "literal and computed root conditions have the same result cardinality", but they still differ on the shape from the first review, src([["a","b"],["c"]]) -> Iterate A -> For -> mid -> if.true_input with a root fallback on false_input:

condition main PR
literal False ['fb'] ['fb']
computed False ['fb'] ['fb','fb']

Please make computed and literal conditions agree, and give each If execution its own frame path. Alternatively, narrow the README.

New regressions

R1 (blocker): a literal If whose unselected branch is a For's final output stalls

For(id="for", collection=["a","b"]) ; for.item -> body(Marked "b:") -> ret ; loop_linkage(for, ret)
for.output_collection -> if.true_input ; AnyType(value="fb").value -> if.false_input
If(condition=False) ; if.value -> sink(AnyType).value      # or a Collect.item sink
  • PR: only fallback runs, then the run stalls.
  • main and pre-fix: 'fb'.
  • Mirror case: the same stall happens with the For on false_input and If(condition=True).
  • Not affected: computed conditions, and the selected-For cases.
  • Coverage: this is the most basic For-in-If shape, and the 2023-test suite doesn't cover it.

Cause:

  • The new gate at graph_if_activation.py:64 (branch_field == _get_source_level_selected_field(if)) now makes the unselected direct For wait for the If to reject its branch.
  • But a literal If still has to pass _has_unmaterializable_for_final_input in the materializer's readiness check, which is skipped only for wired conditions. That check waits for every direct For-final input to finish, including the gated one.
  • The literal early return added to _is_source_inactive doesn't break the cycle at root level, because nothing queues the For for retirement.
  • Likely fix: for a literal If, check only the selected field's inputs, as _get_if_readiness_ancestors already does. The If can then resolve, and _queue_rejected_activation_branch retires the For.

R2: R1 nested under a wired outer If

src(first=[["a","c"],["c","b"]]) -> it0
it0.item -> forT.collection ; forT.item -> bT(Marked "T") -> rT ; loop_linkage(forT, rT)
it0.item -> forF.collection ; forF.item -> bF(Marked "F") -> rF ; loop_linkage(forF, rF)
forT.output_collection -> iif.true_input ; forF.output_collection -> iif.false_input ; If(id="iif", condition=False|True)
iif.value -> p(Marked "p:").value ; p.value -> if.true_input ; AnyType("R").value -> if.false_input
BooleanInvocation(value=True).value -> if.condition ; if.value -> out(Collect).item
  • PR: stalls for either literal value on iif.
  • main and pre-fix: ["p:['Fa','Fc']", "p:['Fc','Fb']"].
  • Not affected: the same graph without the outer If, or with the outer condition False.

R3: unselected direct For under an Iterate stalls after a mid-run save/reload

src(first=[["a","b"],["c"],["d"]]) -> it ; it.item -> for.collection ; for body Marked("F") -> ret ; loop_linkage(for, ret)
for.output_collection -> if.true_input ; AnyType("R").value -> if.false_input ; If(condition=False) ; if.value -> out(Collect).item
  • Trigger: dump_execution_state then load_execution_state, after the last it execution and before the If (step 5 here).
  • PR: stalls. After the load, for, body and ret report inactive but are never retired.
  • Without the reload, and at every other step: the PR gives ['R'].
  • main and pre-fix: ['R'] at every step.
  • Fuzz: a reload sweep over 3000 seeds found 11 such cases at steps 4–5, all with this shape.

R4: a wired root condition selecting a Collect group over a For under an empty Iterate stalls

src(first=[[], [[[]], [["c","a","c"]]]]) -> it0 -> cc0(Concat) -> it1 -> cc1(Concat) -> it2
es(Concat, first=[]) -> eit(Iterate) ; eit.item -> for.collection ; for body Marked("F") -> ret ; loop_linkage(for, ret)
for.output_collection -> if.true_input
it1.item -> cm(Marked "x") -> col(Collect).item ; col.collection -> p(Marked "F:") -> if.false_input
BooleanInvocation(value=False).value -> if.condition ; if.value -> out(Collect).item
  • PR: stalls after the Iterate chain; cm, col, p, if and out never run.
  • main: ['F:[]', "F:['x[[]]', \"x[['c', 'a', 'c']]\"]"].
  • pre-fix: completes, but with [].

The README's new "axis reached only through an unselected For final output" exception describes exactly this shape. Two more fuzz seeds give the same stall.

Performance

A per-item wired If whose true_input is a chain of N passthrough nodes:

N main pre-fix PR
200 0.30s 0.40s 0.93s
400 1.10s 1.36s 3.49s
800 3.68s 4.89s 13.3s
  • Where the time goes: cProfile at N=400 puts 6.9 of 10.0s in the new _has_empty_if_frame. _is_source_inactive calls it for every node on the branch chain, and each call:
    • runs _get_if_branch_frame_axes with a fresh {} cache;
    • runs its own walk over the branch looking for For final outputs.
  • Cheap fix: return early unless some frame Iterate has executed with no items, or cache the axes per If.
  • Wide graphs are unaffected: 25, 50 or 100 Ifs on one Iterate run the same as main.

Checked and fine

  • Tests: tests/test_graph_execution_state.py, tests/app/services/shared and test_if_processor_sqlite.py: 2023 passed, 2 xfailed.
  • Shape harness: the 237-shape harness from #9789 matches main on 233 shapes and fixes 2 of main's stalls. It differs on two: R1, and the computed-False result count under F3.
  • Fuzz: 3000 seeds over For/Iterate/Collect/If graphs, plus another 3000 that add nested Ifs, root-level Fors and Fors under empty Iterates. Run on the default and compatibility schedulers, with save/reload at several steps.
    • No regressions beyond the ones above.
    • It fixes 38 graphs that stall on main.
  • Mechanisms that held up: the iterative _get_edge_frame_axes, _has_unresolved_if_frame_axes (it never deferred forever in any probed case), the new fallbacks in _get_effective_iteration_paths_for_edge, and the empty-iterator mapping fallback.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

7.0.0 python PRs that change python files python-tests PRs that change python tests services PRs that change app services

Projects

Status: 7.0 Theme: Modular Design

2 participants