Skip to content

Conversation

@tomerqodo
Copy link

Benchmark PR from qodo-benchmark#423

@greptile-apps
Copy link

greptile-apps bot commented Jan 21, 2026

Greptile Summary

This PR fixes a streaming bug where conversation variables were output before being updated by the Variable Assigner node. The fix adds a blocks_variable_output method to the v1 Variable Assigner that prevents streaming responses from accessing conversation variables until they've been assigned their new values. It also corrects the input value handling in CLEAR mode to show the cleared value instead of the original income value.

  • Added blocks_variable_output() method to signal when conversation variables are being updated (api/core/workflow/nodes/variable_assigner/v1/node.py:36-43)
  • Fixed input value reporting in NodeRunResult to use the updated variable value when CLEAR mode is used (api/core/workflow/nodes/variable_assigner/v1/node.py:102-111)
  • Added comprehensive test coverage validating the streaming behavior with proper event verification

Confidence Score: 4/5

  • This PR is safe to merge with minor style improvements needed
  • The fix correctly addresses the streaming bug with proper logic and comprehensive tests. The only issue is the use of legacy typing imports which violates the project's coding style guide, but this is a non-critical style issue that doesn't affect functionality
  • api/core/workflow/nodes/variable_assigner/v1/node.py requires a minor update to use modern Python 3.12+ type syntax

Important Files Changed

Filename Overview
api/core/workflow/nodes/variable_assigner/v1/node.py Added blocks_variable_output method and fixed input value handling in CLEAR mode, but uses legacy typing imports (Set, Tuple) instead of modern syntax
api/tests/fixtures/workflow/test_streaming_conversation_variables_v1_overwrite.yml New test fixture for validating v1 Variable Assigner streaming behavior with conversation variable overwrites
api/tests/unit_tests/core/workflow/graph_engine/test_streaming_conversation_variables.py Added comprehensive test for streaming conversation variable updates with proper event validation

Sequence Diagram

sequenceDiagram
    participant User
    participant WorkflowEngine
    participant ResponseCoordinator
    participant VariableAssignerNode
    participant VariablePool
    participant AnswerNode

    User->>WorkflowEngine: Start workflow with query
    WorkflowEngine->>ResponseCoordinator: Initialize with conversation variables
    ResponseCoordinator->>ResponseCoordinator: Build paths to response nodes
    ResponseCoordinator->>VariableAssignerNode: Check blocks_variable_output(conv_var)
    VariableAssignerNode-->>ResponseCoordinator: Returns True (blocks streaming)
    
    WorkflowEngine->>VariableAssignerNode: Execute node
    VariableAssignerNode->>VariablePool: Get original conversation variable
    VariableAssignerNode->>VariablePool: Get input value (sys.query)
    VariableAssignerNode->>VariableAssignerNode: Create updated variable
    VariableAssignerNode->>VariablePool: Update conversation variable
    VariableAssignerNode->>WorkflowEngine: Emit NodeRunStreamChunkEvent
    VariableAssignerNode-->>WorkflowEngine: Return NodeRunResult
    
    WorkflowEngine->>ResponseCoordinator: Node completed
    ResponseCoordinator->>ResponseCoordinator: Remove blocking edge
    ResponseCoordinator->>AnswerNode: Start streaming with updated conv_var
    AnswerNode->>VariablePool: Reference updated conv_var
    AnswerNode->>User: Stream response with correct value
Loading

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

3 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@@ -1,5 +1,5 @@
from collections.abc import Mapping, Sequence
from typing import TYPE_CHECKING, Any
from typing import TYPE_CHECKING, Any, Set, Tuple
Copy link

Choose a reason for hiding this comment

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

style: uses legacy typing imports Set and Tuple instead of modern Python 3.12+ syntax

Suggested change
from typing import TYPE_CHECKING, Any, Set, Tuple
from typing import TYPE_CHECKING, Any

Then update line 36 to use set[tuple[str, ...]] instead of Set[Tuple[str, ...]]

Context Used: Context from dashboard - CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: api/core/workflow/nodes/variable_assigner/v1/node.py
Line: 2:2

Comment:
**style:** uses legacy typing imports `Set` and `Tuple` instead of modern Python 3.12+ syntax

```suggestion
from typing import TYPE_CHECKING, Any
```

Then update line 36 to use `set[tuple[str, ...]]` instead of `Set[Tuple[str, ...]]`

**Context Used:** Context from `dashboard` - CLAUDE.md ([source](https://app.greptile.com/review/custom-context?memory=a5f96311-5bd1-49f2-9828-2ee0c089c012))

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

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.

3 participants