[JPPhoto]

Summary

The literal-If path fallback carries the supplied source graph through both If-to-If and non-If recursive paths. A focused regression test places an AddInvocation between nested Ifs and verifies constrained-graph fallback behavior; it fails if the non-If recursive call stops forwarding the supplied graph.

Related Issues / Discussions

Follow-up to #9786 review feedback. Issue #9484 and PR #9789 cover collected Iterate/If deadlock work outside this change.

QA Instructions

• PYTHONOPTIMIZE=0 uv run --no-sync pytest -q tests/app/services/shared/test_graph_if_runtime.py tests/test_graph_execution_state.py::test_nested_if_literal_condition_activates_selected_branch -k 'not runtime_import_without_graph' - 13 passed, 1 deselected.
• The excluded import subprocess test fails with free(): invalid pointer; the same failure was reproduced on unchanged main.
• Ruff check and format check passed.
• git diff --check passed.

Review

The PR #9786 root-frame fix is already present in main. The remaining in-scope feedback is addressed, and no material review findings remain.

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)

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[lstein]

Approving. I ran an adversarial pass on 5186471 and found no blockers. This addresses nits 1 and 2 from my #9786 review.

Verified

• CI is green.
• tests/test_graph_execution_state.py + tests/app/services/shared/: 1565 passed, 21 skipped, 2 xfailed. The excluded runtime_import_without_graph test also passed on my machine.
• Ruff check and format check pass.
• Production behavior is unchanged. The only production caller of prepare() is _prepare_until_node_ready, and it passes _get_source_graph_flat(). That is the graph the fallback used before, and _prepare(base_g) has no callers.
• I removed each part of the change one at a time and reran the new test:

Part of the change removed	New test
Fallback ignores source_graph	fails
prepare() call site drops g	fails
_has_admitted_source_mapping call site drops graph	fails
Recursive calls stop passing source_graph along (3 separate cases)	still passes

Attacks that didn't land

• get_node_iterators vs the inline scan: they give the same answer. The one difference is the it_graph or ... check, which treats an empty graph as missing. That only matters for an empty iterator graph, which can't happen because the If node comes from the graph it's checked against.
• Different object or cache state: state._materializer() is cached. state._iterator_graph can route to the For planner's builder, but that runs the same iterator_graph method, and the old code went through the same dispatch.

Non-blocking

1. The recursion change is untested. In the test, the inner If has a condition edge, so it returns [] before it reaches the fallback. Only the top-level If ever uses source_graph, which is why all 3 recursion cases above still pass. A nested literal If that actually reaches the fallback with a constrained graph would cover them.
2. The test checks a state production can't produce. The constrained graph drops iterator and the inner→if edge, but state.graph still has both. The recursion follows state.graph edges, while the fallback checks the trimmed graph. So the test treats putting if in the top-level (non-loop) context as correct even though its selected input inner runs per loop iteration in the real graph. A "honor the supplied graph" contract only holds together if the edge walk also follows g. No caller needs that today, so you could also trim the test to the plain "use the graph you were given" check.
3. One spot in prepare() still ignores the supplied graph. The non-If branch at graph_materializer.py:1815 calls get_node_iterators(next_node_id) without g. That fits the If-only scope, but the title suggests more.
4. My third #9786 nit is still open: iterators on the unselected branch still count as ancestors. That's fine to leave, since it was non-blocking.

[JPPhoto]

@lstein Thanks for the review. My plan:

• I agree the current test does not cover recursive forwarding. I'll add a focused nested literal-If test that reaches the fallback recursively.
• I'll revise the constrained-graph test so it verifies the supplied-graph behavior without asserting end-to-end behavior from the inconsistent state.graph and candidate graph.
• I'll leave the non-If iterator lookup unchanged; this PR is scoped to the If fallback, and production preparation passes the flattened source graph.
• I'll keep rejected-branch iterator handling out of this PR; the unselected-branch deadlock is already tracked in Session deadlocks (never completes, no error) when an If node's branch input comes from a chain containing an Iterate node #9484, and fix(graph): resolve If/Iterate/For branch deadlocks and gate branch For loops #9789 covers related collected-Iterate cases. No separate issue is needed.

[lstein]

Re-reviewed 553d204. No blockers, and my approval stands.

What changed

• The new nested-If test reaches the fallback through the recursive calls. Its constrained graph removes only the iterator→guarded edge, so it no longer deletes a node and an edge that state.graph still has.
• The prepare() assertion that depended on that mismatch is gone.
• The iterator check is back to the inline any() scan, which is now identical to main. The net change is only passing source_graph down through the recursive calls.

Verified

• tests/test_graph_execution_state.py + tests/app/services/shared/: 1566 passed, 21 skipped, 2 xfailed. That includes the import test the PR description excludes.
• Ruff check and format check pass, and CI is green.
• I removed each part of the change one at a time and reran the new test:

Part of the change removed	5186471	553d204
Fallback ignores source_graph	fails	fails
Iterator check forced to "no iterator"	—	fails
Edge→If recursive call drops source_graph	passes	fails
If→edge recursive call drops source_graph	passes	fails
Edge→edge recursive call drops source_graph	passes	passes
prepare() call site drops g	fails	passes
_has_admitted_source_mapping call site drops graph	fails	passes

Non-blocking

1. The two call sites are unguarded again. That's the cost of dropping the prepare() assertion, which my last comment pushed toward. Production can't tell the difference, since every caller passes the full graph.
2. One recursive call is still unchecked: the one made when a non-If node sits between two Ifs. Putting an AddInvocation between inner and if in the test would cover it.
3. The PR description is out of date. It still says the change "reuses the existing iterator discovery helper" and that a regression test "covers preparation". Neither is true after this commit.
4. I agree with leaving the non-If iterator lookup and the unselected-branch iterators out of scope, as you explained.

[JPPhoto]

@lstein I made the test-only change you requested and updated the PR description. This should be ready to merge.