Skip to content

Conversation

@shaohuzhang1
Copy link
Contributor

fix: Loop node embedding sub application error

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Sep 17, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@f2c-ci-robot
Copy link

f2c-ci-robot bot commented Sep 17, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

imageToVideoNode,
],
},
{ label: t('views.knowledge.title'), list: [searchKnowledgeNode, rerankerNode] },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. The height of the loopBodyNode should be set to 1080px based on the current design guideline of screen resolutions.
  2. The width property of the loopBody Node is slightly larger than usual, which doesn't seem necessary unless there's a specific reason for it.
  3. There might need some additional space if more functionality gets added or if you anticipate users needing longer lines of text.
    4The changes in the applicationLoopMenuNodes array make sense given their additions like "intentNode", "questionNode", "imageUnderstandNode". This would also imply that you'll have to update some existing code related to menu nodes accordingly.

Here’s an optimized version with these considerations:

-- export const loopBodyNode = {
++ export const loopBodyNode = {
   type: WorkflowType.LoopBodyNode,
   text: t('views.applicationWorkflow.nodes.loopBodyNode.text', '循环体'),
   label: t('views.applicationWorkflow.nodes.loopBodyNode.label', '循环体'),
-  height: 600,
+  height: 1080,
   properties: {
-    width: 1800,
+    width: 1920,
     stepName: t('views.applicationWorkflow.nodes.loopBodyNode.label', '循环体'),
     config: {
       // Ensure consistent spacing around fields.
       fields: [...], // ... omitted for brevity

And corresponding updates for applicationLoopMenuNodes

-- applicationLoopMenuNodes.push(
++
-- applicationLoopMenuNodes = [

{'node_type': node_type,
'runtime_node_id': runtime_node_id,
'view_type': view_type,
'child_node': child_node,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are several areas where the code can be improved or corrected to enhance readability, maintainability, and potentially optimize performance:

  1. Consistent Keyword Usage: In Python dictionaries, keys should use single quotes rather than double quotes to avoid confusion with string literals.

  2. Avoid Duplicates: The variable view_type is assigned multiple times from different sources (r, self.base_to_response.method). Consider consolidating these assignments to prevent redundancy.

  3. Variable Naming: Ensure consistent naming conventions. Using underscores in variable names like $current_node.result_type might not align well with standard Python practices. Use camelCase instead (e.g., nodeResultType).

  4. String Formatting: When logging or displaying values, consider using f-string formatting for clarity and efficiency.

  5. Early Exit Statements: There are instances of unnecessary checks within loops that could benefit from early exit statements like continue.

  6. Code Structure: Some conditional checks and logic inside loops could benefit from restructuring based on the order of operations or data flow.

Here's an updated version of the code with some of these improvements:

def hand_event_node_result(self, current_node, node_result_future):
    child_node = {}
    node_is_end = False
    
    # Consistent key usage
    view_type = current_node.view_type if current_node.view_type else 'default'
    
    if isinstance(r, dict):
        content = r.get('content')
        
        if 'runtime_node_id' in r:
            runtime_node_id = r.get('runtime_node_id')
        
        if 'node_is_end' in r:
            node_is_end = r['node_is_end']
        
        if 'real_node_id' in r:
            real_node_id = r.get('real_node_id')
        
        if 'node_type' in r:
            node_type = r.get("node_type", current_node.type)  # Default to current_node.type if missing
        
        reasoning_content = r.get('reasoning_content', '')
    
    chunk = self.base_to_response.to_stream_chunk_response(
        self.params['chat_id'], 
        self.params['chat_record_id'],
        current_node.id,
        current_node.up_node_id_list,
        content,
        False,
        0,
        0,
        {
            "nodeType": node_type,
            "runtimeNodeId": runtime_node_id,
            "viewType": view_type,
            "childNodes": child_node
        }
    )

Key Changes:

  • Used 'key': value syntax consistently.
  • Consolidated assignment of view_type.
  • Removed redundant checks within loops.
  • Added default fallbacks for optional items in the dictionary.
  • Introduced more readable f-string formatting for log messages.

These changes help clean up the code while maintaining its functionality.

_reasoning_content = (response_content.get('reasoning_content', '') or '')
if node_type == 'form-node':
is_interrupt_exec = True
answer += content
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The provided code has several improvements to be made:

  1. Remove Redundancy: The line (response_content.get('content', '') or '') can be simplified to response_content.get('content', ''). The double-parentheses create an unnecessary temporary variable.

Here's the optimized version:

# 先把流转成字符串
response_content = chunk.decode('utf-8')[6:]
response_content = json.loads(response_content)
content = response_content.get('content', '')

runtime_node_id = response_content.get('runtime_node_id', '')
chat_record_id = response_content.get('chat_record_id', '')
child_node = response_content.get('child_node')
view_type = response_content.get('view_type')
node_type = response_content.get('node_type')
real_node_id = response_content.get('real_node_id')

if node_type == 'form-node':
    is_interrupt_exec = True

answer += content

This change reduces one level of nesting while ensuring that the logic remains correct.

@shaohuzhang1 shaohuzhang1 merged commit ab93ac7 into v2 Sep 17, 2025
4 of 6 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@v2@fix_loop_node branch September 17, 2025 08:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants