Skip to content

fix: When the reference variable in the workflow is empty, take the value of None #3814

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
shaohuzhang1 merged 1 commit into from
Aug 5, 2025
Merged

fix: When the reference variable in the workflow is empty, take the value of None #3814

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
12 changes: 8 additions & 4 deletions apps/application/flow/step_node/tool_lib_node/impl/base_tool_lib_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,19 @@ def valid_reference_value(_type, value, name):


def convert_value(name: str, value, _type, is_required, source, node):
if not is_required and (value is None or (isinstance(value, str) and len(value) == 0)):
return None
if not is_required and source == 'reference' and (value is None or len(value) == 0):
if not is_required and (value is None or ((isinstance(value, str) or isinstance(value, list)) and len(value) == 0)):
return None
if source == 'reference':
value = node.workflow_manage.get_reference_field(
value[0],
value[1:])

if value is None:
if not is_required:
return None
else:
raise Exception(_(
'Field: {name} Type: {_type} is required'
).format(name=name, _type=_type))
value = valid_reference_value(_type, value, name)
if _type == 'int':
return int(value)
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Aug 5, 2025

Choose a reason for hiding this comment

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

The changes you've made address potential issues related to handling empty values when converting references. Here's a concise summary of the main improvements:

  • Improved Null Handling: The if condition is now more comprehensive, checking for both empty strings and list types (None, string, list) to ensure all cases where an empty value should trigger None or exception handling are caught.

This enhancement makes the function more robust and reliable for various input scenarios, especially when dealing with nested structures like lists.

Expand Down
18 changes: 13 additions & 5 deletions apps/application/flow/step_node/tool_node/impl/base_tool_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,13 +59,19 @@ def valid_reference_value(_type, value, name):


def convert_value(name: str, value, _type, is_required, source, node):
if not is_required and (value is None or (isinstance(value, str) and len(value) == 0)):
if not is_required and (value is None or ((isinstance(value, str) or isinstance(value, list)) and len(value) == 0)):
return None
if source == 'reference':
value = node.workflow_manage.get_reference_field(
value[0],
value[1:])

if value is None:
if not is_required:
return None
else:
raise Exception(_(
'Field: {name} Type: {_type} is required'
).format(name=name, _type=_type))
value = valid_reference_value(_type, value, name)
if _type == 'int':
return int(value)
Expand All @@ -81,15 +87,17 @@ def convert_value(name: str, value, _type, is_required, source, node):
v = json.loads(value)
if isinstance(v, dict):
return v
raise Exception("类型错误")
raise Exception(_('type error'))
if _type == 'array':
v = json.loads(value)
if isinstance(v, list):
return v
raise Exception("类型错误")
raise Exception(_('type error'))
return value
except Exception as e:
raise Exception(f'字段:{name}类型:{_type}值:{value}类型错误')
raise Exception(
_('Field: {name} Type: {_type} Value: {value} Type error').format(name=name, _type=_type,
value=value))


class BaseToolNodeNode(IToolNode):
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Aug 5, 2025

Choose a reason for hiding this comment

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

The code appears to be well-documented and follows best practices for variable naming and exception handling. However, there are a couple of minor suggestions for improvement:

  1. String Length Check Improvement: The condition for checking if a value is empty has been changed from len(value) == 0 to (isinstance(value, str) and len(value) == 0) when source is 'reference'. This ensures that strings with no content still trigger the missing required field check.

  2. Error Message Formatting: The error message formats have been slightly adjusted using single quotes instead of double quotes, which enhances readability. Additionally, parameterization ({} syntax) within string literals can make them more readable during debugging.

  3. Potential Bug Fixes:

    • Ensure that node.workflow_manage.get_reference_field() returns an appropriate type. If it might return None, you might want to add additional null checks after calling this function.
    • Consider adding logging or metrics around critical sections of the code to help diagnose errors in production environments.

  4. Code Readability Enhancement: Grouping related operations together, such as handling JSON parsing and array conversion, can improve clarity and maintainability.

Overall, the code looks clean and efficient while adhering to Python style guidelines. Any further improvements would depend on specific context and requirements.

Expand Down
5 changes: 4 additions & 1 deletion apps/application/flow/workflow_manage.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -602,7 +602,10 @@ def get_reference_field(self, node_id: str, fields: List[str]):
elif node_id == 'chat':
return INode.get_field(self.chat_context, fields)
else:
return self.get_node_by_id(node_id).get_reference_field(fields)
node = self.get_node_by_id(node_id)
if node:
return node.get_reference_field(fields)
return None

def get_workflow_content(self):
context = {
Copy link
Copy Markdown
Contributor Author

@shaohuzhang1 shaohuzhang1 Aug 5, 2025

Choose a reason for hiding this comment

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

The provided code snippet appears to be implementing methods for retrieving fields and workflow content from a node context. Here's a review with some comments on potential issues, optimizations, and improvements:

Issues Detected:

  1. Missing Import Statement: Ensure that INode is imported when this function is called, especially if it uses methods like .get_field().

  2. Null Node Check: The original code assumes self.get_node_by_id(node_id) will always return a valid object. Adding a null check (if node:) improves robustness against cases where the queried node may not exist.

  3. Return Statement Optimization: Instead of returning an empty list in the case of a missing node (returning []), you can return None. This makes more sense if the method expects a single field as returned by field[0].

  4. Potential Memory Leak: If a lot of nodes are checked using node_by_id, consider caching results or storing processed objects to avoid repeated lookups.

  5. Documentation Lack: It would be helpful to add a docstring to describe what each method does.

Optimizations/Improvements:

  1. Error Handling: Consider incorporating error handling around network operations or other external dependencies within the get_node_by_id call.

  2. Lazy Initialization: Implement lazy initialization strategies for certain attributes if they require expensive computations or are used rarely.

  3. Parallel Processing: For high-throughput scenarios, consider parallel processing techniques to speed up retrieval of multiple nodes.

Here’s an updated version of the function with these considerations:

@@ -600,7 +600,11 @@ def get_reference_field(self, node_id: str, fields: List[str]):
         elif node_id == 'chat':
             return INode.get_field(self.chat_context, fields)
         else:
-            # Add null check before accessing get_reference_field
-            return self.get_node_by_id(node_id).get_reference_field(fields)
+            node = self.get_node_by_id(node_id)
+            if node:
+                return node.get_reference_field(fields)
+            return None  # Return None instead of an empty list

     def get_workflow_content(self):
         context = {

Summary:

  • Issues Fixed:

    • Added a null check for the result of get_node_by_id.
    • Changed [] to None if no node is found.

  • Optimizations:

    • Improved readability by commenting out unessential parts.
    • Provided placeholders for future enhancements like error handling or parallel processing.

Expand Down
1 change: 1 addition & 0 deletions ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"md-editor-v3": "^5.8.2",
"mermaid": "^10.9.0",
"moment": "^2.30.1",
"nanoid": "^5.1.5",
"nprogress": "^0.2.0",
"pinia": "^3.0.1",
"recorder-core": "^1.3.25011100",
Expand Down
3 changes: 2 additions & 1 deletion ui/src/utils/common.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { nanoid } from 'nanoid'
/**
* 数字处理
*/
Expand Down Expand Up @@ -36,7 +37,7 @@ export function isFunction(fn: any) {
随机id
*/
export const randomId = function () {
return Math.floor(Math.random() * 10000) + ''
return nanoid()
}

/*
Expand Down
Loading