Skip to content

Fix parsing of nested for-loops that re-iterate the outer collection - #341

Merged
samwaseda merged 2 commits into
mainfrom
copilot/fix-nested-loops-fail
Oct 22, 2026
Merged

samwaseda merged 2 commits into
mainfrom
copilot/fix-nested-loops-fail

Conversation

Copilot AI commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Summary

A for-loop nested inside another for-loop failed to parse when the inner loop iterated over the same collection as the outer one. The parser raised KeyError: Symbol 'w' is not in scope:

r = arange(min_value=min_value, max_value=max_value)
outer_result = []
for v in r:
    inner_result = []
    for w in r:  # KeyError: Symbol 'w' is not in scope
        b = sub(w, x)
        inner_result.append(b)
    ...

Related Issue(s)

Resolves the nested-loop failure reported in #340.

Backward Compatibility

No breaking changes. Previously, referencing the iterated collection inside its own loop body raised a KeyError. It is now accepted. All existing unit tests pass.

Detailed Description

Cause. When a for-loop body was forked, the iterated symbol r was renamed to the loop variable v and r was dropped from the body's scope. An inner for w in r then could not find r, so w was never registered.

Changes in parsers/for_parser.py:

  • parse_for_node keeps the iterated collection's port (r) in the body scope alongside the loop variable, via added_symbols. The body can then use r as a broadcast input while v is scattered from the same for-node port.
  • _wire_inputs dedupes for-node inputs, since r can now appear as both a broadcast and a scattered port.

Resulting recipe. The outer for-node has inputs [x, r]. Its body has inputs [x, r, v], with v fed from r as the scattered edge and r fed from r as the broadcast edge.

Verification. The issue's workflow now parses and evaluates to 2.0, both called directly and via its recipe; I computed the expected value by hand. I added a regression test, TestNestedLoops, to tests/integration/parsers/test_parsing_for_nodes.py. It checks the ports of both the outer and inner for-nodes.

Copilot AI linked an issue Oct 22, 2026 that may be closed by this pull request
Co-authored-by: samwaseda <37879103+samwaseda@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix nested loops failure in mean value calculation Fix parsing of nested for-loops that re-iterate the outer collection Oct 22, 2026
Copilot AI requested a review from samwaseda October 22, 2026 09:14
@samwaseda
samwaseda marked this pull request as ready for review October 22, 2026 09:32
@samwaseda
samwaseda requested a review from liamhuber October 22, 2026 09:32
@codecov

codecov Bot commented Oct 22, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.95%. Comparing base (6deaab4) to head (9174916).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #341   +/-   ##
=======================================
  Coverage   99.95%   99.95%           
=======================================
  Files          53       53           
  Lines        4135     4135           
=======================================
  Hits         4133     4133           
  Misses          2        2           

☔ View full report in Codecov by Harness.
📢 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.

@samwaseda

Copy link
Copy Markdown
Member

@liamhuber Maybe we should talk whether this is a valid way of using nested for loops, because we could also argue that arange should be written as a list of lists instead of a simple list.

@liamhuber liamhuber mentioned this pull request Oct 22, 2026

@liamhuber liamhuber left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Solution LGTM.

Discussion of why I agree we need a solution is over in #340

@samwaseda
samwaseda merged commit 23d380a into main Oct 22, 2026
22 checks passed
@samwaseda
samwaseda deleted the copilot/fix-nested-loops-fail branch October 22, 2026 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nested loops fail

3 participants