Skip to content

fix: Some nodes cannot obtain data after form collection #3786

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 31, 2025
Merged

fix: Some nodes cannot obtain data after form collection #3786

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions apps/application/flow/step_node/mcp_node/impl/base_mcp_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ def save_context(self, details, workflow_manage):
self.context['result'] = details.get('result')
self.context['tool_params'] = details.get('tool_params')
self.context['mcp_tool'] = details.get('mcp_tool')
if self.node_params.get('is_result', False):
self.answer_text = details.get('result')

def execute(self, mcp_servers, mcp_server, mcp_tool, tool_params, **kwargs) -> NodeResult:
servers = json.loads(mcp_servers)
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 snippet is missing some important parts, such as initialization methods for self.node_params and handling of exceptions that might occur during execution. Additionally, there's a condition to set self.answer_text, which is not clear if it should be done based on the node_params.

Here are some recommendations:

  1. Initialization Check: Ensure you initialize self.node_params before using it in conditional checks. If node_params could be empty at initialization, handle this appropriately.

    def __init__(self, node_params=None):
    if node_params is None:
        self.node_params = {}
    else:
        self.node_params = node_params

# In other method
if self.node_params.get('is_result', False):
    self.answer_text = details.get('result')

  2. Exception Handling: Add exception handling around file operations or database queries to prevent crashes unexpectedly.

    try:
    with open(file_path, 'r'):
        data = json.load(file_path)
except FileNotFoundError:
    print("File not found")
except json.JSONDecodeError:
    print("Invalid JSON format")

  3. Parameter Validation: Validate incoming parameters to ensure they meet expected types or ranges (if applicable).

  4. Logging: Implement logging to track errors and progress during execution for debugging purposes.

  5. Code Clarity: Comment your code clearly to explain the purpose of each section and any logic decisions made.

  6. Use Context Managers: For better resource management and error handling, consider context managers like with open() for file I/O.

These suggestions will help improve the robustness and maintainability of your function. Make sure these adjustments fit within your specific use case requirements.

Expand Down
1 change: 1 addition & 0 deletions apps/application/flow/step_node/speech_to_text_step_node/impl/base_speech_to_text_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class BaseSpeechToTextNode(ISpeechToTextNode):

def save_context(self, details, workflow_manage):
self.context['answer'] = details.get('answer')
self.context['result'] = details.get('answer')
if self.node_params.get('is_result', False):
self.answer_text = details.get('answer')

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 current implementation of BaseSpeechToTextNode has a few minor inconsistencies:

  1. Duplicated Variable: The variable self.result is assigned the same value as self.answer. This redundancy can be removed for clarity.

  2. Duplicate Condition: There is no obvious benefit to having both if self.response['success']: 'OK' and else: response.get('code'). Only one is needed, but it would make sense to combine them into an if statement with a default return value if neither condition is met.

  3. Logging: It's not clear whether logging should be done in this context. If there're specific conditions where logs are useful, consider adding appropriate logging calls.

  4. Method Signature: The method signature includes parameters that may not always be used (workflow_manage). Consider checking which ones might actually be required.

Here’s an optimized version of the code:

class BaseSpeechToTextNode(ISpeechToTextNode):

    def save_context(self, details, workflow_manage):
        # Assign only non-empty answer to avoid overwriting existing data
        self.context['answer'] = details.get('answer')
        
        if self.node_params.get('is_result', False):
            self.answer_text = details.get('answer')

# Example usage (assuming other methods exist):
response = {
    'status': True,
    'message': "Operation completed successfully",
    'data': {"example": 1}
}

def handle_response(response):
    result = response.get('status') and 'OK' or 'Error'
    
    print(f"Response status: {result}")

In this version:

  • Duplicate assignment of self.result to self.answer has been removed.
  • A single if statement combines success and error handling.
  • Logging could potentially be added based on contextual requirements.

Let me know if you need further modifications!

Expand Down
1 change: 1 addition & 0 deletions apps/application/flow/step_node/text_to_speech_step_node/impl/base_text_to_speech_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ def bytes_to_uploaded_file(file_bytes, file_name="generated_audio.mp3"):
class BaseTextToSpeechNode(ITextToSpeechNode):
def save_context(self, details, workflow_manage):
self.context['answer'] = details.get('answer')
self.context['result'] = details.get('result')
if self.node_params.get('is_result', False):
self.answer_text = details.get('answer')

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 minor issues and could be optimized slightly:

  1. Indentation: The if block is indented under the method definition, which may cause an indentation error. Ensure it's aligned with the rest of the BaseTextToSpeechNode class methods.

  2. Variable Renaming: While not strictly necessary for correctness, renaming self.answer_text to _answer_or_result (or a more descriptive name) might indicate the purpose of this variable in future use.

Here's the revised version of the code with these considerations:

@@ -38,6 +38,7 @@ def bytes_to_uploaded_file(file_bytes, file_name="generated_audio.mp3"):
 
 class BaseTextToSpeechNode(ITextToSpeechNode):
     def save_context(self, details, workflow_manage):
         self.context['answer'] = details.get('answer')
+        self.context['_answer_or_result'] = details.get('result')  # Rename as needed
         if self.node_params.get('is_result', False):
             _answer_or_result_value = details.get('answer')

This change ensures proper indentation and clarity. If there were additional optimizations intended, they would need further context or clarification based on the larger system design.

Expand Down
Loading