Skip to content
Merged
Show file tree
Hide file tree
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
4 changes: 0 additions & 4 deletions ui/src/workflow/common/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1050,10 +1050,6 @@ export const toolWorkflowLibNode = {
stepName: t('workflow.nodes.toolWorlflowNode.label','工作流工具'),
config: {
fields: [
{
label: t('common.result'),
value: '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.

The code snippet is missing some logic related to handling different steps within the toolWorkflow and its node configuration. This likely indicates that this part of the library was intended to manage specific inputs and outputs for various work flow nodes.

However, given that the provided information only contains the definition of a single tool workflow node with a placeholder result field, we cannot fully review or evaluate it comprehensively without additional context. Here's what would be necessary:

  1. Context: Ensure you have access to the full scope of how these tools interact within workflows and any other potential behaviors they might implement.

  2. Interactions: Look at where this node interacts with others (e.g., input/output connections) through parent processes.

  3. Error Handling: Check if there are error checks or fallback mechanisms mentioned in other parts of the codebase to handle issues with this tool workflow node.

  4. Performance: Review if there are any resource-intensive operations within this module that could impact performance.

Without this comprehensive view, I can only offer general observations about an incomplete component. If you provide more details about this functionality within your larger system architecture, we can assess its completeness and suggest improvements further.

Expand Down
3 changes: 3 additions & 0 deletions ui/src/workflow/nodes/tool-workflow-lib-node/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ class ToolWorkflowLibNode extends AppNode {
constructor(props: any) {
super(props, ToolWorkflowLibNodeVue)
}
getConfig(props: any) {
return props.model.properties.config
}
}
export default {
type: 'tool-workflow-lib-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.

Your code is mostly clean and follows best practices. However, here are some minor suggestions for clarity and performance:

  1. Property Access: In getConfig, accessing props.model.properties.config can be improved if you ensure that both model and properties exist before attempting to access .config. Here's a refactored version:

    getConfig(props) {
      const { model, properties } = props;
      return model && properties ? properties.config : undefined;
    }
  2. Docstring: While not strictly necessary in this context (as it’s quite straightforward), adding documentation could help explain the purpose of the getConfig method.

Here's the updated code with these improvements:

class ToolWorkflowLibNode extends AppNode {
  constructor(props: any) {
    super(props, ToolWorkflowLibNodeVue);
   }

   /**
    * Retrieves configuration from the tool workflow library node.
    *
    * @param {any} props - The component.props object containing model and properties.
    * @returns {*} The configuration property or undefined if missing.
    */
   getConfig(props: any) {
     const { model, properties } = props;
     return model && properties ? properties.config : undefined;
   }
}

export default {
  type: 'tool-workflow-lib-node',
};

These changes make the function more robust and clear.

Expand Down
22 changes: 10 additions & 12 deletions ui/src/workflow/nodes/tool-workflow-lib-node/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -150,26 +150,21 @@ const update_field = () => {
loadSharedApi({ type: 'tool', systemType: apiType.value })
.getToolById(props.nodeModel.properties.node_data.tool_lib_id)
.then((ok: any) => {
console.log('ssss', ok.data)

const workflowNodes = ok.data?.work_flow?.nodes || []
const baseNode = workflowNodes.find((n: any) => n.type === 'tool-base-node')

if (baseNode) {
const new_input_list = baseNode.properties.user_input_field_list || []
const new_output_list = baseNode.properties.user_output_field_list || []

let config_field_list: any[] = []
if (new_output_list.length > 0) {
config_field_list = new_output_list.map((item: any) => ({
label: item.label,
value: item.field,
}))
}
const old_config_fields = props.nodeModel.properties.config?.fields || []
const config_field_list = new_output_list.map((item: any) => {
const old = old_config_fields.find((o: any) => o.value === item.field)
return old ? JSON.parse(JSON.stringify(old)) : { label: item.label, value: item.field }
})

const input_title = baseNode.properties.user_input_config?.title
const output_title = baseNode.properties.user_output_config?.title

const old_input_list = props.nodeModel.properties.node_data.input_field_list || []
const merged_input_list = new_input_list.map((item: any) => {
const find_field = old_input_list.find((old_item: any) => old_item.field === item.field)
Expand All @@ -185,11 +180,14 @@ const update_field = () => {
})

set(props.nodeModel.properties.node_data, 'input_field_list', merged_input_list)
set(props.nodeModel.properties.config, 'fields', config_field_list)
set(props.nodeModel.properties, 'config', {
fields: config_field_list,
output_title: output_title,
})
set(props.nodeModel.properties.node_data, 'input_title', input_title)
set(props.nodeModel.properties.config, 'output_title', output_title)
}
set(props.nodeModel.properties, 'status', ok.data.is_active ? 200 : 500)
props.nodeModel.clear_next_node_field(true)
})
.catch(() => {
set(props.nodeModel.properties, 'status', 500)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here's a concise review of the provided code:

General Structure and Logic

  • The function update_field is defined to update node properties from fetched API data.
  • It uses the loadSharedApi function to fetch details about the tool based on apiType.value.
  • It parses and processes workflow_nodes, focusing on nodes with type='tool-base-node'.

Potential Issues/Improvements

  1. Duplicate Code Handling:

    • There seems to be some repetition in how input/output fields are mapped and processed.
  2. Null Check Logic:

    • props.nodeModel.properties.node_data.input_field_list || [] and similar lines assume that these arrays always exist.
    • Consider adding null checks to handle cases where they might not be present.
  3. Object Assignment Order:

    • When updating the .config property, there’s unnecessary duplication between assigning the fields array and setting other keys (output_title, etc.).
  4. Code Readability:

    • Use clearer variable names; e.g., instead of new_input_list, use something like availableInputs.
    • Comment blocks can clarify complex logic steps.
  5. Error Handling:

    • Although catch block handles errors, consider logging them more explicitly for debugging purposes.
  6. Function Call Optimization:

    • Ensure that the call to nodeModel.clear_next_node_field(true) does not lead to any performance bottlenecks or unintended behavior.

Optimizations Suggested

  1. Variable Naming:

    - const new_input_list = baseNode.properties.user_input_field_list || []
    + const availableInputs = baseNode.properties.user_input_field_list || [];
  2. Check for Null Before Assignment:

    if (old_config_fields && old_config_fields.find(o => o.value === item.field)) {
      return JSON.parse(JSON.stringify(old));
    } else {
      return { label: item.label, value: item.field };
    }
  3. Single Config Update:

    set(props.nodeModel.properties, 'config', {
      fields: config_field_list,
      output_title: output_title || '',
    });
  4. Avoid Redundant Calls:
    Replace redundant calls to set when setting the same object multiple times.

  5. Logging Errors:

    .catch(() => {
      set(props.nodeModel.properties, 'status', 500);
      console.error('Failed to update field:', error); // Log the actual error
    });

These improvements will enhance the readability, maintainability, and robustness of the code while ensuring it works correctly across different scenarios.

Expand Down
Loading