PromptL nested loops bug - #49
Merged
Merged
Conversation
Root cause: In for-loop compilation, child AST nodes are shared across all
iterations. Each child gets marked with completedAs='step_${i}' after
processing. When a for-loop completes and is re-invoked (e.g., from an outer
loop), the iteration counter restarts at 0, causing completedValue 'step_0'
to match the stale completedAs 'step_0' from a prior invocation. This caused
the first elements of inner loops to be silently skipped.
Fix:
1. Use invocation-scoped completedValue ('step_${invocationCount}_${i}')
so values are unique across for-loop invocations.
2. Clear descendant node status (completedAs, scopePointers) when the
for-loop completes all iterations, preventing stale state from
affecting future invocations.
Added 12 comprehensive test cases covering:
- Nested loops with varying inner array sizes
- Three levels of nesting
- Nested loops with conditionals and index variables
- Nested loops with else blocks
- Chain steps inside nested loops with varying sizes
Co-authored-by: Alex Rodríguez <me@arn.sh>
|
Cursor Agent can help with this pull request. Just |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a bug in nested loops where inner loop elements were skipped due to stale AST node status.
The root cause was that child AST nodes were shared across for-loop iterations, and their
completedAsstatus was not reset. When an outer loop re-invoked an inner loop, thecompletedAsstatus from a previous invocation would cause the first inner elements to be erroneously skipped. The fix introduces aloopInvocationCountto makecompletedValueunique across invocations and aclearNodeStatusfunction to recursively reset node status after loop completion.Slack Thread