fix: [Application, Knowledge Base] Workflow Export Now Supports Exporting Tool Workflows

[shaohuzhang1]

fix: [Application, Knowledge Base] Workflow Export Now Supports Exporting Tool Workflows

[shaohuzhang1]

The code looks mostly correct but contains a few areas that could be improved:

1. Line Length: The line QuerySet(ToolWorkflow).bulk_create([...]) exceeds the maximum recommended line length of 80 characters, which can make it harder to read.

2. Tool Workflow Reset Method: The reset_workflow method should use the provided update_tool_map on all nodes of both main flow and loop bodies. However, there's a small mistake where the condition to check for a workflow type (node.get('type') == 'workflow') might need to include 'tool-workflow-lib-node'.

3. Export Functionality: In the export function, the logic for handling skill tools should be more flexible. Currently, it assumes these tool objects have a file_content attribute that needs conversion into base64 data. Consider checking if file_content exists before attempting the conversion.

Here are some specific improvements to suggest:

Line Length Improvement

QuerySet(ToolWorkflow).bulk_create([ToolWorkflow(workspace_id=ws_id, work_flow=self.reset_workflow(twf, update_tool_map), tool_id=tw_id)
                                    for tw in tool_list if tw.tool_type == ToolType.WORKFLOW])

Correction in Workflow Type Check Inside reset_workflow

if node.get('type') in ['workflow', 'tool-workflow-lib-node']:
    ...

Enhanced Export Handling for Skill Tools

Ensure skill tools have a file content attribute before converting it to base64.

for tool in tool_list:
    if tool.tool_type == ToolType.SKILL:
        if not hasattr(tool, 'file_content'):
            # Additional error logging or default value setting
            pass
        # Convert file content to base64 string here...
        ...

def convert_file_to_base64(file_content):
    return base64.b64encode(file_content.read()).decode('utf-8')

These changes aim to improve readability while maintaining functionality. If you want further optimizations or additional considerations, let me know!

[shaohuzhang1]

1. Imports and Variable Naming:

   • The variable name _result in get_tool_id_list() is misleading because it suggests that this variable will store results, but it ends up being an empty list.
   • In save_workflow_mapping(), the comment about filtering out files using a specific key ((str(item.target_type) + str(item.target_id))) may be redundant since the actual filter logic does not include such functionality.

2. Logic of get_tool_id_list():

   • There's no clear separation between different types of nodes (like tools and non-tools) when collecting IDs from workflows.

3. Improvements for Recursive Handling:

   • Using recursion might make sense for exploring all children in the workflow tree structure to collect tool ID lists. Consider renaming variables like response to reflect their intended purpose or use more descriptive names.

4. Edge Cases in Functions:

   • Implement checks for edge cases where there are no matching records or unexpected data structures.

Here’s an optimized version incorporating some suggested changes:

def get_tool_id_list(workflow, with_deep=False):
    _result = set()
    
    if 'nodes' in workflow.get('nodes'):
        for node in workflow['nodes']:
            if node.get('type') == 'tool-lib-node':
                mcp_tool_id = node.get('node_data', {}).get('mcp_tool_id')
                if mcp_tool_id:
                    _result.add(mcp_tool_id)
    
    if with_deep:
        # Assuming workflow_map includes mappings for nested tools
        from maxkb.map.model.workflowMap import WorkflowMapping
        
        # This loop assumes every record maps to multiple related ids based on target_key_str format
        for mapping in map.records():
            for record in mapping.related_records_with_target_type(str(node.target_type)):
                _result.update(map.target_ids_of_record(record, str(mapping.target_key_str)))
        
        # Recursively fetch additional workflow tools
        for id in _result:
            wf_tools = ToolWorkflowService.fetch_related_workflows_for_tool(id)
            for wf_tool in wf_tools:
                child_tool_id_list = get_tool_id_list(wf_tool.work_flow, True)
                _result.update(child_tool_id_list)
                
    return sorted(_result)

# Similar improvements can be applied similarly to `get_child_tool_id_list`

This version uses sets to avoid duplicate entries, which improves efficiency especially when dealing with large datasets. It also outlines general guidelines that could guide further optimizations depending on actual usage patterns and performance needs.

[shaohuzhang1]

Your code appears to be generally well-structured, but there are a few areas that could benefit from improvements:

Improvements:

1. Static Methods for Workflow Resetting:

   • The reset_workflow method can be made static as it doesn't need access to an instance of KnowledgeWorkflowModelSerializer. This improves readability and makes it reusable.

2. Tool Dictionary Creation:

   • In the _to_tool_dict method, ensure that the dictionary is correctly formatted regardless of whether the tool type is workflow or not. You might want to separate handling based on the tool type.

3. Error Handling in Export Method:

   • Consider logging exceptions or adding more informative error messages in the handleExport method to help debug issues during exports.

4. Consistent Use of Query Sets:

   • Ensure that all queries use appropriate query sets for better performance and security. For example, consider using .select_related() and .prefetch_related() where applicable.

5. Validation in Importer:

   • Before creating objects, consider validating data integrity or constraints (e.g., unique keys).

6. Code Comments:

   • Add comments for complex parts of the code to enhance readability for others who may maintain it.

Optimizations:

• Bulk Operations:

  • Ensure that bulk operations like QuerySet.objects.bulk_create() are used properly. Make sure you have indexed fields where necessary to optimize database performance.

• Logging:

  • Implement logging around critical sections of the code to trace its flow and identify any issues without causing service unavailability.

Here's your updated code snippet with some improvements applied:

@@ -38,7 +38,7 @@
 from system_manage.models import AuthTargetType
 from system_manage.models.resource_mapping import ResourceType
 from system_manage.serializers.user_resource_permission import UserResourcePermissionSerializer
-from tools.models import Tool, ToolScope
+from tools.models import Tool, ToolScope, ToolType, ToolWorkflow
 from tools.serializers.tool import ToolExportModelSerializer
 from users.models import User

@@ -64,6 +64,10 @@ def hand_node(node, update_tool_map):
         mcp_tool_id = (node.get('properties', {}).get('node_data', {}).get('mcp_tool_id') or '')
         node.get('properties', {}).get('node_data', {})['mcp_tool_id'] = update_tool_map.get(mcp_tool_id,
                                                                                              mcp_tool_id)
+    if node.get('type') == 'tool-workflow-lib-node':
+        tool_lib_id = (node.get('properties', {}).get('node_data', {}).get('tool_lib_id') or '')
+        node.get('properties', {}).get('node_data', {})['tool_lib_id'] = update_tool_map.get(tool_lib_id,
+                                                                                             tool_lib_id)
 
 
 class KnowledgeWorkflowModelSerializer(serializers.ModelSerializer):
@@ -356,6 +360,12 @@ def import_(self, instance: dict, is_import_tool, with_valid=True):
             if is_import_tool:
                 if len(tool_model_list) > 0:
                     QuerySet(Tool).bulk_create(tool_model_list)
+                    QuerySet(ToolWorkflow).bulk_create(
+                        [ToolWorkflow(workspace_id=workspace_id,
+                                      work_flow=self._reset_workflow(tool.get('work_flow'), update_tool_map),
+                                      tool_id=tool.get('id'))
+                         for tool in tool_list if tool.get('tool_type') == ToolType.WORKFLOW])
+

These changes should improve the robustness and maintainability of your codebase.