Skip to content
Merged
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
16 changes: 12 additions & 4 deletions apps/application/flow/workflow_manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,10 +544,16 @@ def get_answer_text_list(self):
return [[item.to_dict() for item in r] for r in result]

@staticmethod
def dependent_node(up_node_id, node):
def dependent_node(edge, node):
up_node_id = edge.sourceNodeId
if not node.node_chunk.is_end():
return False
if node.id == up_node_id:
if node.context.get('branch_id', None):
if edge.sourceAnchorId == f"{node.id}_{node.context.get('branch_id', None)}_right":
return True
else:
return False
if node.type == 'form-node':
if node.context.get('form_data', None) is not None:
return True
Expand All @@ -560,9 +566,11 @@ def dependent_node_been_executed(self, node_id):
@param node_id: 需要判断的节点id
@return:
"""
up_node_id_list = [edge.sourceNodeId for edge in self.flow.edges if edge.targetNodeId == node_id]
return all([any([self.dependent_node(up_node_id, node) for node in self.node_context]) for up_node_id in
up_node_id_list])
up_edge_list = [edge for edge in self.flow.edges if edge.targetNodeId == node_id]
return all(
[any([self.dependent_node(edge, node) for node in self.node_context if node.id == edge.sourceNodeId]) for
edge in
up_edge_list])

def get_next_node_list(self, current_node, current_node_result):
"""
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Code Review

The provided code appears to be part of a flow processing application that involves nodes, edges, and determining dependencies between them. Below is an analysis of the code with comments on potential issues or areas for improvement:

Issues Detected:

  1. Variable Naming: The variable up_node_id is being reused within different methods without resetting it. This can lead to confusion and potential bugs if used incorrectly.

  2. Branch Condition Handling: In the dependent_node method, there's a conditional check based on both node.type and node.context. It would make more sense to refactor this as separate conditions.

  3. Edge Source Anchor Id Check: The condition for checking sourceAnchorId seems complex and might not always reflect the intended functionality. Consider simplifying the logic.

  4. Dependent Node Be Executed Logic: There are inconsistencies in how dependencies are checked across different parts of the function. Ensure consistency.

Optimization Suggestions:

  1. Avoid Reusing Variable Names: Use distinct variable names throughout the codebase to prevent confusion and improve readability. For example, use target_up_node_ids instead of up_node_id_list.

  2. Separate Conditional Checks: Refactor the dependent_node logic so each dependency (form-data, branch id) has its own independent checks.

  3. Simplify Edge Source Anchors: If possible, simplify the sourceAnchorId comparison to focus only on necessary fields.

  4. Consistent Dependency Checking: Normalize the way dependencies are determined to avoid multiple similar loops.

Here is the revised code with some suggestions incorporated:

def get_answer_text_list(self):
    """
    Extracts and returns answer text lists from the results.
    """
    return [[item.to_dict() for item in r] for r in result]

@staticmethod
def dependent_node(edge, node):
    source_node_id = edge.sourceNodeId
    
    if not node.node_chunk.is_end():
        return False
    elif node.id == source_node_id:
        # Simplify branching check
        branch_condition = bool(node.context.get('branch_id', None))
        
        if branch_condition:
            target_anchor_right = (f"{node.id}_{node.context.get('branch_id', None)}_right")
            return edge.sourceAnchorId == target_anchor_right
        
        form_data_condition = bool(node.context.get('form_data', None))
        
        if form_data_condition:
            return True
            
    if node.type == 'form-node':
        return False  # No need for further checks for other types
    
    return False

def dependent_node_been_executed(self, node_id):
    """
    Determines whether a node depends upon another node having been executed.
    
    :param node_id: The ID of the node to check dependencies for.
    :return: A boolean indicating dependency execution status.
    """
    up_edge_list = [
        e for e in self.flow.edges if e.targetNodeId == node_id
    ]
    
    # Separate loop for each type of dependency
    required_conditions = ['form-data']
    
    for edge in up_edge_list:
        corresponding_node = [n for n in self.node_context if n.id == edge.sourceNodeId][0]
        
        for condition in required_conditions:
            context_value = corresponding_node.context.get(condition, None)
            
            if condition == 'form-data' and context_value is not None:
                return True
                
            # Add additional complex conditionals here if needed
            
    return False

def get_next_node_list(self, current_node, current_node_result):
    """
    Retrieves next nodes based on the current node and its result.
    """

This revised version refactors the dependent_node method using clear conditional statements and separates the dependency-checking processes. Additionally, it normalizes the approach for determining dependencies, ensuring consistent behavior across the code base.

Expand Down
Loading