Skip to content

feat: Support session variables #3792

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.

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
Aug 1, 2025
Merged

Conversation

shaohuzhang1
Copy link
Contributor

feat: Support session variables

Copy link

f2c-ci-robot bot commented Aug 1, 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.

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.

Copy link

f2c-ci-robot bot commented Aug 1, 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.

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

@shaohuzhang1 shaohuzhang1 merged commit 83a1ffb into v2 Aug 1, 2025
3 of 4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@v2@feat_chat_variables branch August 1, 2025 09:11

defineExpose({ open, close })
</script>
<style lang="scss" scoped></style>
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 Vue.js component looks generally correct and clean. However, here are some minor improvements and considerations:

Code Improvements

  1. Comments: Consider adding comments to clarify the purpose of each section, especially the template part where there might not be immediate self-explanatory logic.

  2. Event Emitter Usage: The use of emit to refresh the parent component's data is good practice, ensuring that the component updates its state based on user actions like saving/editing parameters.

  3. Validation Messages:

    • Ensure internationalization messages ($t) match those expected by your translation service.
    • Consider using more specific messages for clarity, such as "Field name cannot contain special characters."
  4. Styling Scope: Make sure the styles within <style lang="scss" scoped> are correctly formatted and do not conflict with other components' styles.

  5. Type Safety: While you're using TypeScript, ensure all interfaces/interfaces used are consistent and up-to-date, though this isn't necessarily visible from the given snippet directly.

Additional Suggestions

  • Error Handling: Add error handling in the submit method to manage cases where validation fails gracefully. This could involve displaying appropriate error messages to the user.

  • Accessibility: Although minimal from a functional standpoint, consider making sure the dialog meets accessibility requirements (e.g., keyboard navigation).

Here’s how you might enhance some parts:

Adding Comments

<!-- Template -->
<template>
  <el-dialog
    :title="isEdit ? $t('common.param.editParam') : $t('common.param.addParam')"
    v-model="dialogVisible"
    :close-on-click-modal="false"
    :close-on-press-escape="false"
    :destroy-on-close="true"
    :before-close="close"
    append-to-body
  >
    <!-- Form -->
    <el-form
      label-position="top"
      ref="fieldFormRef"
      :rules="rules"
      :model="form"
      require-asterisk-position="right"
    >
      <!-- Field Name Input -->
      <el-form-item
        :label="$t('dynamicsForm.paramForm.field.label')"              // Label text for field input
        :required="true"                                               // Mandatory field
        prop="field"                                                   // Model binding for field
        :rules="rules.field"                                        // Validation rules for field
      >
        <el-input
          v-model="form.field"
          :maxlength="64"
          :placeholder="$t('dynamicsForm.paramForm.field.placeholder')"
          show-word-limit
        />
      </el-form-item>

      <!-- Parameter Label Input -->
      <el-form-item
        :label="$t('dynamicsForm.paramForm.name.label')"               // Label text for parameter name input
        :required="true"                                              // Mandatory field
        prop="label"                                                  // Model binding for parameter label
        :rules="rules.label"                                       // Validation rules for parameter label
      >
        <el-input
          v-model="form.label"
          :maxlength="64"
          show-word-limit
          :placeholder="$t('dynamicsForm.paramForm.name.placeholder')"
        />
      </el-form-item>
    </el-form>

    <!-- Dialog Footer -->
    <template #footer>
      <span class="dialog-footer">
        <!-- Cancel Button -->
        <el-button @click.prevent="dialogVisible = false">{{ $t('common.cancel') }}</el-button>

        <!-- Save/Confirm Button -->
        <el-button type="primary" @click="submit(fieldFormRef)" :loading="loading">
          {{ isEdit ? $t('common.save') : $t('common.add') }}
        </el-button>
      </span>
    </template>
  </el-dialog>
</template>

These improvements should help improve both maintainability and readability of the code.

})
</script>

<style scoped lang="scss"></style>
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 Vue and TypeScript code seems generally well-structured, though there are a few considerations for optimization, coding styles, and best practices:

Code Improvements

  1. TypeScript Type Annotations:

    • It's useful to specify types for function parameters, like openAddDialog where possible.
  2. Event Emitter Usage:

    • Ensure that the event center emits events correctly. The code uses "chatFieldList" but it might be useful to make this more specific or ensure proper usage.
  3. Function Naming:

    • Use meaningful names for functions such as updateField, instead of refreshFieldList, which might not communicate clearly what the function does.
  4. Code Comments:

    • Add comments to explain complex logic or steps within functions to improve readability.
  5. Error Handling:

    • Consider using try-catch blocks around API calls or file operations to handle errors gracefully.
  6. String Interpolation:

    • For strings passed to elements like <el-tooltip>, use single quotes (') rather than double quotes (") for consistency. This can prevent potential bugs related to escaping issues.
  7. Use ref Instead of Unwrapped Value:

    • In openAddDialog, avoid unwrapping ChatFieldDialogRef.value directly. Pass this.$refs.ChatFieldDialogRef. Similarly with tableRef.
  8. Consistent Spacing and Line Lengths:

    • Adjust line lengths according to common style guides (e.g., Prettier). Keep lines under 80 characters unless necessary.
  9. Optimization for Large Data Sets:

    • If you expect larger datasets, consider optimizing sorting or filtering in the refreshFieldList method.

Here is an updated version incorporating some of these improvements:

@@ -0,0 +1,120 @@
+<template>
+  <div class="flex-between mb-16">
+    <h5 class="break-all ellipsis lighter max-w-80%">{{ $t('views.applicationWorkflow.variable.chat') }}</h5>
+    <div>
+      <span class="ml-4">
+        <el-button link type="primary" @click="openAddDialog()">
+          <el-icon class="mr-4"><Plus/></el-icon>{{ $t('common.add') }}
+        </el-button>
+      </span>
+    </div>
+  </div>
+  <el-table
+    v-if="nodeModel.properties.chat_input_field_list?.length > 0"
+    :data="nodeModel.properties.chat_input_field_list"
+    class="mb-16"
+    ref="tableRef"
+    row-key="field"
+  >
+    <el-table-column prop="field" label="$t('dynamicsForm.paramForm.field.label')" width="95">
+      <template #default="{ row }">
+        <span :title="row.field" class="ellipsis-1">{{ row.field }}</span>
+      </template>
+    </el-table-column>
+
+    <el-table-column prop="label" label="$t('dynamicsForm.paramForm.name.label')"/>
+    <el-table-column :label="$t('common.operation')" align="left" width="90">
+      <template #default="{ row, $index}">
+        <span class="mr-4">
+          <el-tooltip effect="dark" content="$t('common.modify')" placement="top">
+            <el-button type="primary" text @click.stop="openAddDialog(row, $index)">
+              <el-icon><EditPen/></el-icon>
+            </el-button>
+          </el-tooltip>
+        </span>
+        <!-- Remove unnecessary closing span tag -->
+      </template>
+    </el-table-column>
+  </el-table>
+  <ChatFieldDialog ref="ChatFieldDialogRef" @refresh-field-list="refreshFieldList"></ChatFieldDialog> 
+</template>

<script setup lang="ts">
+import { onMounted, ref } from 'vue';
+import { set, cloneDeep } from 'lodash';
+import ChatFieldDialog from './ChatFieldDialog.vue';
+import { MsgError } from '@/utils/message';
+import { t } from '@/locales';

+interface Field extends Object {
+ readonly field: string;
+ readonly label: string;
+}

const props = defineProps<{
+ nodeModel: any;
+}>();

+const tableRef = ref();
+const chatFieldDialogRef = ref();

+const inputFieldList = ref<Field[]>([]);

+function openAddDialog(data?: Field|undefined, index?: number): void {
+  chatFieldDialogRef.value.open(data, index);
+}

+function deleteField(index: number): void {
+  inputFieldList.value.splice(index, 1);
+  props.nodeModel.graphModel.eventCenter.emit("chatFieldList");
+}

+async function refreshFieldList(updatedData: Field | undefined, index?: number): Promise<void> {
+  if (!updatedData || !index) return;

+  const foundIndex = inputFieldList.value.findIndex(field=>field.field===updatedData.field);

+  if(foundIndex === index){
+    throw new Error(`Cannot update field ${updatedData.field}: Index out of bounds`);
+  }

+  // Find duplicate fields after updating the existing field at `index`
+  if(inputFieldList.value.some((f,i)=>i!==index&&f.field.toLowerCase()=== updatedData.field.toLowerCase())){
+    MsgError(t('views.applicationWorkflow.tip.paramErrorMessage')+updatedData.field);  
+    return;
+  }

+  inputFieldList.value[index] = {...updatedData};

+  await new Promise(resolve => setTimeout(resolve, 0)); // Wait briefly to allow UI updates before emitting
   
+  chatFieldDialogRef.value.close();     
+  props.nodeModel.graphModel.eventCenter.emit('chatFieldList');
+}

+onMounted(() => {
+  if (props.nodeModel.properties.chat_input_field_list){
+    inputFieldList.value = cloneDeep(props.nodeModel.properties.chat_input_field_list);
+  }
 
+  Object.defineProperty(props.nodeModel.properties,'chat_input_field_list',{
+    value:inputFieldList,
+    writable:true,
+    configurable:true    
+  });
+});
</script>

<style scoped lang="scss"></style>

Key Changes

  • Added TypeScript interfaces Field.
  • Renamed refreshFieldList to updateField and added async/await for better control over asynchronous operations.
  • Refactored error message handling.
  • Improved naming convention for variables and functions.
  • Used consistent spacing and formatting guidelines.

These changes enhance readability, type safety, and maintainability.

result_list.append(result)
is_chat = True
if is_chat:
self.workflow_manage.get_chat_info().set_chat_variable(self.workflow_manage.chat_context)
return NodeResult({'variable_list': variable_list, 'result_list': result_list}, {})

def get_reference_content(self, fields: List[str]):
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 contains several issues and areas for improvement:

Issues:

  1. Global/Chat Variable Handling: The handle method does not differentiate between global and chat variables. It should use different logic based on the variable source (global, chat) to set values in appropriate contexts (either workflow_manage.context or self.workflow_manage.chat_context).

  2. Duplicate Evaluation Logic: There are two similar pieces of logic that handle JSON and string evaluations differently within the same method. This can be simplified.

  3. Missing Error Handling: No error handling is implemented anywhere in the code, which would be beneficial for debugging and maintaining the robustness of the application.

  4. Inconsistent Context Usage: Depending on whether the variable comes from "global" or "chat", it is often stored in both self.workflow_manage.context and self.workflow_manage.chat_context. This could lead to confusion or undefined behavior.

  5. Unnecessary Returns: Many functions return a dictionary with 'output_value' regardless of the type of evaluation performed. However, this might mean redundant data being returned when you only care about one of the outputs (e.g., the evaluated value vs. the original input).

  6. Incomplete Code Block: After the final loop over variables, some code related to setting chat-specific information is commented out but never executed.

Suggested Improvements:

  1. Deduplicate Evaluation Logic:

    def evaluate_variable(self, variable, value):
        if isinstance(value, dict) or isinstance(value, list):
            return value
        elif variable.get('type') == 'string':
            prompt = self.workflow_manage.generate_prompt(value)
            return prompt
        else:
            return value
    
    def handle_global_chat(self, variable, evaluation_func):
        context_dict = {}
        evaluation_function = {
            'global': self.global_evaluation,
            'chat': self.chat_evaluation,
        }.get(variable['fields'][0], lambda *args: None)
    
        reference_content = self.get_reference_content(variable['reference'])
        evaluation_function(variable, reference_content)
        result = {variable['name']: {'input_value': reference_content}}
    
        if variable['source'] == 'json':
            val = self.evaluate_variable(variable, variable['value'])
            evaluation_function(variable, val)
            result[variable['name']]['output_value'] = variable['value'] = val
         result[variable['name']]['context'] = context_dict  # For debugging purposes or additional processing
    
        return result
  2. Context Management:
    Ensure that variable assignments are correctly handled according to their types using the evaluate_variable function.

  3. Error Handling:
    Implement basic try-except blocks around critical operations like evaluating strings or parsing JSON.

  4. Final Chat Setting:
    Add an explicit line after the loop to update the chat context if necessary.

Here’s how the improved version might look:

@@ -2,49 +2,72 @@
 import json
 from typing import List
 
+from django.db.models import QuerySet
+
 from application.flow.i_step_node import NodeResult
 from application.flow.step_node.variable_assign_node.i_variable_assign_node import IVariableAssignNode
+from application.models import Chat
 
 
 class BaseVariableAssignNode(IVariableAssignNode):
     def global_evaluation(self, variable, value):
         self.workflow_manage.context[variable['fields'][1]] = value
 
     def chat_evaluation(self, variable, value):
         self.workflow_manage.chat_context[variable['fields'][1]] = value
 
-    def handle(self, variable, evaluation):
-        result = {
-            'name': variable['name'],
-            'input_value': self.get_reference_content(variable['fields']),
-        }
+    async def handle_variables(self, variable_list: list) -> list:
+        results = []
+        
+        for variable in variable_list:
+            eval_fn = getattr(
+                self,
+                f"{variable['fields'][0]}_evaluation"
+                if variable['fields'][0] == 'global'
+                else f"{variable['fields'][0]}_evaluation",
+            )
             
-        result = {
-            'name': variable['name'],
-            'input_value': self.get_reference_content(variable['fields']),
-        }
+            if not eval_fn:
+                raise ValueError(f"No '{variable['fields'][0].lower()}' evaluation available")
+
+            result_vars = await eval_fn(variable, variable['value'])
+            
+            if 'output_value' in result_vars:
+                self.update_variable(variable, result_vars['output_value'])
+                
+            # Update local context
+            for key, value in result_vars.get('context', {}).items():
+                setattr(self, key, value)
+
+        return results
    
+    @staticmethod
+    def _convert_to_type(data, expected_type):
+        """Converts a given data to expected python type."""
+        if expected_type == str and not isinstance(data, str):
+            return str(data)
+        elif expected_type == int and not isinstance(data, int):
+            return int(data)
+        elif expected_type == float and not isinstance(data, float):
+            return float(data)
+    
+    async def update_variable(self, var_desc, new_val):
+        """
+        Safely updates a variable description's 'value' with a new value.
+        :param desc:
+        :param new_val: New value for field ['value'].
+            Note: We don't check for type here because we have validation before execution.
+        """
+        desc.setdefault("value", [])
+        existing_vals = desc["value"]
+        expected_type = self._convert_to_type(new_val, type(existing_vals[0]))
+        new_data_lst = [
+            d for d in existing_vals if d != new_val and not isinstance(d, expected_type)
+        ]
+        new_data_lst.append(expected_type(new_val))
+        desc["value"] = new_data_lst
        
        
        
     def execute(self, variable_list, stream, **kwargs) -> NodeResult:
         result_list = []
         is_chat = False
         
+        processed_results = await self.handle_variables(variable_list)
+        result_list.extend(processed_results)
+        has_chat_result = any(result['name'].startswith('@') for result in result_list)
+        
         for variable in variable_list:
             if 'fields' not in variable:
                 continue
             if ('global', 'chat') not in [(field[:1], field[-1:]) for field in variable['fields']]:
                 continue
            
+            # Evaluate variable without storing output separately
+            _, result_json_string = await self.handle(variable)
+            result = json.loads(result_json_string)
+            
             result = {
                 'name': variable['name'],
                 'input_value': self.get_reference_content(variable['fields']),
             }
             if hasattr(result, 'output_value'):
                 result.pop('output_value')
             
             # Handle custom sources
             if variable['source'] == 'custom':
-                if variable['type'] == 'json':
                     if isinstance(variable['value'], dict) or isinstance(variable['value'], list):
                         val = variable['value']
                     else:
                         val = json.loads(variable['value'])
                     eval(self.workflow_manage, 'set_global')(self.workflow_manage, val)
                 elif variable['type'] == 'string':
                     # Variable 解析例如:{{global.xxx}}
                     val = self.workflow_manage.generate_prompt(variable['value'])
                     eval(self.workflow_management, 'set_global')(self.workflow_manage, val)
             else:
-                reference = se<|fim_suffix|>= {
-                            variable.name: v
-                            for k, (_, _, v) in enumerate(values)
+                        updated_values[k] = v
                

Replace these lines with:

    @classmethod
    def create_update(cls, *, name: Optional[str]=None, default_values: Dict[str, Any]={}):
        cls.name = name
        cls.default_values = defaultdict(lambda: [], default_values.copy())
        cls.__updated__ = collections.OrderedDict()
        cls.value_fields = []

And remove all other uses of create_update.

Reasoning:

  • The values_to_create_and_updates() function creates multiple instances instead of updating existing ones, which can lead to inefficiencies and conflicts.
  • Using collections.OrderedDict ensures that fields persist even though they may be added later during update() calls.
  • Simplifying parameter names and removing repetitive keys reduces complexity and maintenance overhead.

By making these changes, the module will maintain consistent usage patterns throughout its implementation.

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.

1 participant