Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions api/core/workflow/nodes/variable_assigner/v1/node.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from collections.abc import Mapping, Sequence
from typing import TYPE_CHECKING, Any
from typing import TYPE_CHECKING, Any, Set, Tuple

from core.variables import SegmentType, Variable
from core.workflow.constants import CONVERSATION_VARIABLE_NODE_ID
Expand Down Expand Up @@ -33,6 +33,15 @@ def __init__(
graph_runtime_state=graph_runtime_state,
)

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
Comment on lines +36 to +43

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n api/core/workflow/nodes/variable_assigner/v1/node.py | head -60

Repository: 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 2

Repository: 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 f

Repository: agentic-review-benchmarks/dify-combined-coderabbit

Length of output: 199


🏁 Script executed:

rg "blocks_variable_output" --type py -B 3 -A 3

Repository: 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.py

Repository: 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 -80

Repository: 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 5

Repository: 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 5

Repository: agentic-review-benchmarks/dify-combined-coderabbit

Length of output: 1550


🏁 Script executed:

cat -n api/core/workflow/nodes/variable_assigner/v2/node.py | head -60

Repository: 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 2

Repository: 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 -50

Repository: 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.

Suggested change
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.


@classmethod
def version(cls) -> str:
return "1"
Expand Down Expand Up @@ -89,10 +98,17 @@ def _run(self) -> NodeRunResult:
self.graph_runtime_state.variable_pool.add(assigned_variable_selector, updated_variable)

updated_variables = [common_helpers.variable_to_processed_data(assigned_variable_selector, updated_variable)]

# Prepare input value for result
if self.node_data.write_mode == WriteMode.CLEAR:
result_input_value = updated_variable.to_object()
else:
result_input_value = income_value.to_object()

return NodeRunResult(
status=WorkflowNodeExecutionStatus.SUCCEEDED,
inputs={
"value": income_value.to_object(),
"value": result_input_value,
},
# NOTE(QuantumGhost): although only one variable is updated in `v1.VariableAssignerNode`,
# we still set `output_variables` as a list to ensure the schema of output is
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
app:
description: Validate v1 Variable Assigner blocks streaming until conversation variable is updated.
icon: 🤖
icon_background: '#FFEAD5'
mode: advanced-chat
name: test_streaming_conversation_variables_v1_overwrite
use_icon_as_answer_icon: false
dependencies: []
kind: app
version: 0.5.0
workflow:
conversation_variables:
- description: ''
id: 6ddf2d7f-3d1b-4bb0-9a5e-9b0c87c7b5e6
name: conv_var
selector:
- conversation
- conv_var
value: default
value_type: string
environment_variables: []
features:
file_upload:
allowed_file_extensions:
- .JPG
- .JPEG
- .PNG
- .GIF
- .WEBP
- .SVG
allowed_file_types:
- image
allowed_file_upload_methods:
- local_file
- remote_url
enabled: false
fileUploadConfig:
audio_file_size_limit: 50
batch_count_limit: 5
file_size_limit: 15
image_file_size_limit: 10
video_file_size_limit: 100
workflow_file_upload_limit: 10
image:
enabled: false
number_limits: 3
transfer_methods:
- local_file
- remote_url
number_limits: 3
opening_statement: ''
retriever_resource:
enabled: true
sensitive_word_avoidance:
enabled: false
speech_to_text:
enabled: false
suggested_questions: []
suggested_questions_after_answer:
enabled: false
text_to_speech:
enabled: false
language: ''
voice: ''
graph:
edges:
- data:
isInIteration: false
isInLoop: false
sourceType: start
targetType: assigner
id: start-source-assigner-target
source: start
sourceHandle: source
target: assigner
targetHandle: target
type: custom
zIndex: 0
- data:
isInLoop: false
sourceType: assigner
targetType: answer
id: assigner-source-answer-target
source: assigner
sourceHandle: source
target: answer
targetHandle: target
type: custom
zIndex: 0
nodes:
- data:
desc: ''
selected: false
title: Start
type: start
variables: []
height: 54
id: start
position:
x: 30
y: 253
positionAbsolute:
x: 30
y: 253
selected: false
sourcePosition: right
targetPosition: left
type: custom
width: 244
- data:
answer: 'Current Value Of `conv_var` is:{{#conversation.conv_var#}}'
desc: ''
selected: false
title: Answer
type: answer
variables: []
height: 106
id: answer
position:
x: 638
y: 253
positionAbsolute:
x: 638
y: 253
selected: true
sourcePosition: right
targetPosition: left
type: custom
width: 244
- data:
assigned_variable_selector:
- conversation
- conv_var
desc: ''
input_variable_selector:
- sys
- query
selected: false
title: Variable Assigner
type: assigner
write_mode: over-write
height: 84
id: assigner
position:
x: 334
y: 253
positionAbsolute:
x: 334
y: 253
selected: false
sourcePosition: right
targetPosition: left
type: custom
width: 244
viewport:
x: 0
y: 0
zoom: 0.7
Original file line number Diff line number Diff line change
Expand Up @@ -45,3 +45,33 @@ def test_streaming_conversation_variables():
runner = TableTestRunner()
result = runner.run_test_case(case)
assert result.success, f"Test failed: {result.error}"


def test_streaming_conversation_variables_v1_overwrite_waits_for_assignment():
fixture_name = "test_streaming_conversation_variables_v1_overwrite"
input_query = "overwrite-value"

case = WorkflowTestCase(
fixture_path=fixture_name,
use_auto_mock=False,
mock_config=MockConfigBuilder().build(),
query=input_query,
inputs={},
expected_outputs={"answer": f"Current Value Of `conv_var` is:{input_query}"},
)

runner = TableTestRunner()
result = runner.run_test_case(case)
assert result.success, f"Test failed: {result.error}"

events = result.events
conv_var_chunk_events = [
event
for event in events
if isinstance(event, NodeRunStreamChunkEvent) and event.selector == ["conversation", "conv_var"]
]

assert conv_var_chunk_events, "Expected conversation variable chunk events to be emitted"
assert all(event.chunk == input_query for event in conv_var_chunk_events), (
"Expected streamed conversation variable value to match the input query"
)