fix(api): defer streaming response until referenced variables are updated#1
Conversation
📝 WalkthroughWalkthroughAdded a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
1 similar comment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@api/core/workflow/nodes/variable_assigner/v1/node.py`:
- Around line 36-43: blocks_variable_output currently compares
self.node_data.assigned_variable_selector (a Sequence[str]) directly against
variable_selectors (a Set[Tuple[str, ...]]), which will never match; convert the
selector to a tuple before the membership test. Update the method
blocks_variable_output to compute something like
tuple(self.node_data.assigned_variable_selector) and check that against
variable_selectors so the set membership works as intended, leaving the rest of
the method and return semantics unchanged.
| def blocks_variable_output(self, variable_selectors: Set[Tuple[str, ...]]) -> bool: | ||
| """ | ||
| Check if this Variable Assigner node blocks the output of specific variables. | ||
|
|
||
| Returns True if this node updates any of the requested conversation variables. | ||
| """ | ||
| assigned_selector = self.node_data.assigned_variable_selector | ||
| return assigned_selector in variable_selectors |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n api/core/workflow/nodes/variable_assigner/v1/node.py | head -60Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 2571
🏁 Script executed:
# Search for assigned_variable_selector definition in the codebase
rg "assigned_variable_selector" --type py -B 2 -A 2Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 7839
🏁 Script executed:
# Find the class that contains this method and understand the node_data structure
ast-grep --pattern $'class $_ {
$$$
def blocks_variable_output(self, variable_selectors: Set[Tuple[str, ...]]) -> bool {
$$$
}
$$$
}'Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 76
🏁 Script executed:
# Look at the node_data object definition
fd "node.py" api/core/workflow/nodes/variable_assigner --type fRepository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 199
🏁 Script executed:
rg "blocks_variable_output" --type py -B 3 -A 3Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 3467
🏁 Script executed:
# Check the node_data definition more thoroughly
cat -n api/core/workflow/nodes/variable_assigner/v1/node_data.pyRepository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 577
🏁 Script executed:
# Look at variable_pool to understand how selectors are used
rg "variable_pool.get\|variable_pool.add" --type py -B 2 -A 2 | head -80Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 76
🏁 Script executed:
cat -n api/core/workflow/nodes/variable_assigner/v2/node.py | sed -n '36,50p'Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 846
🏁 Script executed:
# Check base class to understand the interface
cat -n api/core/workflow/nodes/base/node.py | sed -n '1,50p'Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 1967
🏁 Script executed:
# Look at how variable_selectors is constructed when calling blocks_variable_output
cat -n api/core/workflow/graph_engine/response_coordinator/coordinator.py | sed -n '1,100p'Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 4307
🏁 Script executed:
# Find where variable_selectors is constructed and passed to blocks_variable_output
rg "variable_selectors" api/core/workflow/graph_engine/response_coordinator/coordinator.py -B 5 -A 5Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 1202
🏁 Script executed:
# Check the actual v2 implementation to compare
cat -n api/core/workflow/nodes/variable_assigner/v2/node.py | sed -n '36,50p'Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 846
🏁 Script executed:
# Also check if there's any test that directly tests blocks_variable_output
rg "blocks_variable_output" api/tests --type py -B 5 -A 5Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 1550
🏁 Script executed:
cat -n api/core/workflow/nodes/variable_assigner/v2/node.py | head -60Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 2775
🏁 Script executed:
# Search for assigned_variable_selector definition in v2
rg "assigned_variable_selector" api/core/workflow/nodes/variable_assigner/v2/ -B 2 -A 2Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 76
🏁 Script executed:
cat -n api/core/workflow/nodes/variable_assigner/v2/node.py | sed -n '65,85p'Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 1085
🏁 Script executed:
# Check the v2 node_data definition
cat -n api/core/workflow/nodes/variable_assigner/v2/entities.py | head -50Repository: agentic-review-benchmarks/dify-combined-coderabbit
Length of output: 1208
Convert assigned_variable_selector to tuple for set membership check.
assigned_variable_selector is typed as Sequence[str] but variable_selectors is Set[Tuple[str, ...]]. A sequence will never match a tuple in the membership check, so this method will always return False. Convert the selector to a tuple before comparison.
Suggested fix
- assigned_selector = self.node_data.assigned_variable_selector
- return assigned_selector in variable_selectors
+ assigned_selector = tuple(self.node_data.assigned_variable_selector)
+ return assigned_selector in variable_selectors📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def blocks_variable_output(self, variable_selectors: Set[Tuple[str, ...]]) -> bool: | |
| """ | |
| Check if this Variable Assigner node blocks the output of specific variables. | |
| Returns True if this node updates any of the requested conversation variables. | |
| """ | |
| assigned_selector = self.node_data.assigned_variable_selector | |
| return assigned_selector in variable_selectors | |
| def blocks_variable_output(self, variable_selectors: Set[Tuple[str, ...]]) -> bool: | |
| """ | |
| Check if this Variable Assigner node blocks the output of specific variables. | |
| Returns True if this node updates any of the requested conversation variables. | |
| """ | |
| assigned_selector = tuple(self.node_data.assigned_variable_selector) | |
| return assigned_selector in variable_selectors |
🤖 Prompt for AI Agents
In `@api/core/workflow/nodes/variable_assigner/v1/node.py` around lines 36 - 43,
blocks_variable_output currently compares
self.node_data.assigned_variable_selector (a Sequence[str]) directly against
variable_selectors (a Set[Tuple[str, ...]]), which will never match; convert the
selector to a tuple before the membership test. Update the method
blocks_variable_output to compute something like
tuple(self.node_data.assigned_variable_selector) and check that against
variable_selectors so the set membership works as intended, leaving the rest of
the method and return semantics unchanged.
3 participants
Benchmark PR from agentic-review-benchmarks#1
Summary by CodeRabbit
Bug Fixes
Tests