Fix nested step dependency and argument inheritance issues#258
Merged
Conversation
barnabasJ
approved these changes
Aug 8, 2025
This commit addresses multiple related issues with nested step dependencies and argument inheritance in Reactor: **Core Issues Fixed:** - Steps inside nested contexts (around, group, switch, map) couldn't depend on parent scope results - Compose steps inside maps couldn't access the map's arguments - Missing dependency tracking for cross-scope references **Key Changes:** 1. **Enhanced Planner (lib/reactor/planner.ex)**: - Added extract_nested_dependencies() to identify cross-scope dependencies - Added support for nested step dependency tracking via new nested_steps/1 callback - Properly handles result() references from nested steps to parent scope 2. **Improved Map Step (lib/reactor/step/map.ex)**: - Implemented nested_steps/1 callback to expose contained steps to planner - Enhanced argument inheritance for nested compose steps - Added support for cross-scope element references - Fixed element argument resolution for nested contexts 3. **Enhanced Step Protocol (lib/reactor/step.ex)**: - Added optional nested_steps/1 callback for steps containing other steps - Provides default empty implementation via use Reactor.Step 4. **Map DSL Improvements (lib/reactor/dsl/map.ex)**: - Added context passing for argument inheritance - Compose steps now automatically inherit map arguments when not explicitly provided - Supports nested map contexts with proper argument propagation 5. **Runtime Support (lib/reactor/executor/step_runner.ex)**: - Added nested dependency collection and context passing - Enables runtime access to cross-scope dependencies 6. **Compose Builder Fix (lib/reactor/builder/compose.ex)**: - Improved error handling for missing arguments - Better error reporting when both extra and missing arguments exist **Test Coverage:** - Added comprehensive tests for nested dependency resolution - Added tests for compose steps in maps with argument inheritance - Added tests for cross-scope dependency scenarios - Verified proper error handling for invalid patterns **Backwards Compatibility:** - All changes are backwards compatible - Existing functionality unchanged - New features are opt-in via the nested_steps callback Fixes issues with compose steps in maps not having access to map arguments and resolves dependency planning issues for nested step contexts.
38ff044 to
14d97d5
Compare
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
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.
Summary
This PR resolves critical issues with nested step dependencies and argument inheritance in Reactor, addressing problems where steps inside nested contexts couldn't properly access parent scope results and where compose steps in maps couldn't inherit map arguments.
Issues Fixed
Closes #206 - Steps preceding a switch only get executed once
Closes #252 - Compose step inside map can't use map's arguments
Root Cause Analysis
The core issues stemmed from two related problems:
Missing Dependency Tracking: The planner wasn't identifying when steps inside nested contexts (around, group, switch, map) depended on results from parent scope steps, leading to incomplete dependency graphs and steps executing out of order.
Argument Inheritance Gaps: Compose steps inside maps couldn't access the map's arguments, requiring manual workarounds and breaking the expected inheritance model.
Key Changes
1. Enhanced Dependency Resolution (
lib/reactor/planner.ex)extract_nested_dependencies()function to identify cross-scope dependenciesresult()calls2. New Step Protocol Extension (
lib/reactor/step.ex)nested_steps/1callback for steps that contain other stepsuse Reactor.Step3. Map Step Improvements (
lib/reactor/step/map.ex)nested_steps/1callback to expose contained steps to planner4. Map DSL Context Passing (
lib/reactor/dsl/map.ex)5. Runtime Support (
lib/reactor/executor/step_runner.ex)6. Improved Error Handling (
lib/reactor/builder/compose.ex)Examples
Issue #206 - Dependency Resolution (Before: Broken)
Issue #206 - Dependency Resolution (After: Fixed)
Issue #252 - Map Argument Inheritance (Before: Broken)
Issue #252 - Map Argument Inheritance (After: Fixed)
Important Notes
For cross-scope dependencies: You still need to explicitly declare arguments that reference parent scope results using
result(). What we fixed is ensuring the planner properly tracks these dependencies so execution order is correct.For map argument inheritance: Compose steps now automatically inherit arguments from their containing map, eliminating the need for manual workarounds.
Test Coverage
Backwards Compatibility
✅ Fully backwards compatible
nested_steps/1callbackPerformance Impact
nested_steps/1Testing
All tests pass including:
Migration Guide
No migration required! This is a pure enhancement that:
For users who were working around the compose-in-map issue, you can now remove manual argument passing workarounds as they'll be inherited automatically.