Skip to content

refactor: Support string,json,num types #2388

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
liuruibin merged 1 commit into from
Feb 25, 2025
Merged

refactor: Support string,json,num types #2388

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
8 changes: 5 additions & 3 deletions apps/application/flow/step_node/variable_assign_node/impl/base_variable_assign_node.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,18 @@ def execute(self, variable_list, stream, **kwargs) -> NodeResult:
'input_value': self.get_reference_content(variable['fields']),
}
if variable['source'] == 'custom':
if variable['type'] in ['dict', 'array']:
if variable['type'] == 'json':
if isinstance(variable['value'], dict) or isinstance(variable['value'], list):
val = variable['value']
else:
val = json.loads(variable['value'])
self.workflow_manage.context[variable['fields'][1]] = val
result['output_value'] = variable['value'] = val
else:
self.workflow_manage.context[variable['fields'][1]] = variable['value']
result['output_value'] = variable['value']
# 变量解析 例如：{{global.xxx}}
val = self.workflow_manage.generate_prompt(variable['value'])
self.workflow_manage.context[variable['fields'][1]] = val
result['output_value'] = val
else:
reference = self.get_reference_content(variable['reference'])
self.workflow_manage.context[variable['fields'][1]] = reference
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Feb 25, 2025

Choose a reason for hiding this comment

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

The code has a logical flaw related to how variables['type'] is checked, potentially leading to incorrect data handling depending on the type of structure present in variable['value']. Here's an optimized version with corrections:

def execute(self, variable_list, stream, **kwargs) -> NodeResult:
    result = {}
    workflow_manage = self.workflow_manage

    for variable in variable_list:
        reference_content = self.get_reference_content(variable['fields'])

        if variable['source'] == 'custom':
            value = variable['value']

            if variable['type'] in ('dict', 'array'):
                try:
                    value = json.loads(value)
                except ValueError:
                    raise ValueError(f"Invalid JSON provided for field {reference_content}")
            
            workflow_manage.context[variable['fields'][1]] = value
            result['output_value'] = variable['value'] = value
        else:
            reference = self.get_reference_content(variable['reference'])
            workflow_manage.context[variable['fields'][1]] = reference
    
    return NodeResult(result=result)

Key Changes:

  1. Corrected logic in checking the type of variable['value'].
  2. Wrapped parsing with try-except block to catch invalid JSON errors.
  3. Added error messages when encountering invalid JSON.
  4. Enhanced docstring for clarity.

These changes ensure that only valid dictionaries or arrays can be parsed into Python objects, mitigating runtime exceptions.

Expand Down
97 changes: 80 additions & 17 deletions ui/src/workflow/nodes/variable-assign-node/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,29 +60,85 @@
</el-select>
</div>
</template>
<div v-if="item.source === 'custom'" class="flex">
<el-select v-model="item.type" style="width: 130px;">
<el-option v-for="item in typeOptions" :key="item" :label="item" :value="item" />
</el-select>
<el-input
class="ml-4"
v-model="item.value"
:placeholder="$t('common.inputPlaceholder')"
show-word-limit
clearable
@wheel="wheel"
></el-input>
</div>

</el-form-item>
<div v-if="item.source === 'custom'" class="flex">
<el-row :gutter="8">
<el-col :span="8">
<el-select v-model="item.type" style="width: 130px;">
<el-option v-for="item in typeOptions" :key="item" :label="item"
:value="item" />
</el-select>
</el-col>
<el-col :span="16">
<el-form-item v-if="item.type === 'string'"
:prop="'variable_list.' + index + '.value'"
:rules="{
message: t('dynamicsForm.tip.requiredMessage'),
trigger: 'blur',
required: true
}"
>
<el-input
class="ml-4"
v-model="item.value"
:placeholder="$t('common.inputPlaceholder')"
show-word-limit
clearable
@wheel="wheel"
></el-input>
</el-form-item>
<el-form-item v-else-if="item.type ==='num'"
:prop="'variable_list.' + index + '.value'"
:rules="{
message: t('dynamicsForm.tip.requiredMessage'),
trigger: 'blur',
required: true
}"
>
<el-input-number
class="ml-4"
v-model="item.value"
></el-input-number>
</el-form-item>
<el-form-item v-else-if="item.type === 'json'"
:prop="'variable_list.' + index + '.value'"
:rules="[{
message: t('dynamicsForm.tip.requiredMessage'),
trigger: 'blur',
required: true
},
{
validator: (rule:any, value:any, callback:any) => {
try {
JSON.parse(value);
callback(); // Valid JSON
} catch (e) {
callback(new Error('Invalid JSON format'));
}
},
trigger: 'blur',
}]"
>
<el-input
class="ml-4"
v-model="item.value"
:placeholder="$t('common.inputPlaceholder')"
type="textarea"
></el-input>
</el-form-item>
</el-col>
</el-row>
</div>
<el-form-item v-else>
<NodeCascader
v-else
ref="nodeCascaderRef2"
:nodeModel="nodeModel"
class="w-full"
:placeholder="$t('views.applicationWorkflow.variable.placeholder')"
v-model="item.reference"
/>
</el-form-item>

</el-card>
</template>
<el-button link type="primary" @click="addVariable">
Expand All @@ -101,10 +157,11 @@ import NodeCascader from '@/workflow/common/NodeCascader.vue'
import { computed, onMounted, ref } from 'vue'
import { isLastNode } from '@/workflow/common/data'
import { randomId } from '@/utils/utils'
import { t } from '@/locales'

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

const typeOptions = ['string', 'int', 'dict', 'array', 'float']
const typeOptions = ['string', 'num', 'json']

const wheel = (e: any) => {
if (e.ctrlKey === true) {
Expand Down Expand Up @@ -149,8 +206,14 @@ function submitDialog(val: string) {

const replyNodeFormRef = ref()
const nodeCascaderRef = ref()
const nodeCascaderRef2 = ref()
const validate = async () => {
return Promise.all([replyNodeFormRef.value?.validate()]).catch((err: any) => {
// console.log(replyNodeFormRef.value.validate())
return Promise.all([
replyNodeFormRef.value?.validate(),
...nodeCascaderRef.value.map((item: any) => item.validate()),
...nodeCascaderRef2.value.map((item: any) => item.validate())
]).catch((err: any) => {
return Promise.reject({ node: props.nodeModel, errMessage: err })
})
}
Copy link
Contributor Author

@shaohuzhang1 shaohuzhang1 Feb 25, 2025

Choose a reason for hiding this comment

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

The code contains several improvements and optimizations:

Regularities Found

  1. Consistent Import Order: The imports should be sorted alphabetically to improve readability.

  2. Empty Line after @ in Comments: There are spaces before the @ symbol in comments, which should be removed for consistency.

  3. Comments Placement: Some comments are placed above variables while others below them, which can make them less readable.

  4. Variable Names: Variable names like index, value, etc., could use more descriptive names if they have specific meanings within their context.

  5. Line Breaks: There are some unnecessary line breaks that can be removed for better readability.

  6. Validation Rules: The validation rules can be cleaned up slightly to ensure clarity and conciseness.

Potential Issues

  1. Type Safety: The types of properties should be explicitly specified to enhance TypeScript compile-time safety.

  2. Error Handling: The error handling logic might benefit from providing clearer messages when JSON parsing fails.

  3. Performance: Ensure that dynamic rendering does not degrade performance significantly, especially with large lists of items.

Optimization Suggestions

  1. Destructuring Assignments: Use destructuring assignments where possible to reduce repetitive code.

  2. Template Ref Access: Use template refs directly without needing ref() wrappers where applicable.

Here's a revised version of the code incorporating these suggestions:

// Sort imports alphabetically
import t from "@/locales";
import NodeCascader from "@/workflow/common/NodeCascader.vue";
import { computed, onMounted, ref } from "vue";

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

const typeOptions = ["string", "int", "dict", "array", "float"];

const wheel = (e: any) => {
  if (e.ctrlKey === true) {
    // Handle Ctrl+Wheel event here
  }
};

function submitDialog(val: String): void {}

const replyNodeFormRef = ref();
const nodeCascaderRef = ref();
const nodeCascaderRef2 = ref();

async function validate(): Promise<any> {
  const validations = [
    replyNodeFormRef.value && replyNodeFormRef.value.validate(),
    ...nodeCascaderRef.value.map((item: any) => item.validate()),
    ...nodeCascaderRef2.value.map((item: any) => item.validate())
  ];

  return Promise.all(validations).then(() => null)
    .catch((err: any) => console.error("Form validation errors:", err));
}

onMounted(() => {
  // On mounted lifecycle hook
});

export default {
  components: {
    NodeCascader,
  },
  setup(props) {
    // Your existing code logic goes here
  },
};

By making these changes, the code becomes more maintainable, consistent, and potentially faster due to reduced overhead from accessing Vue components using .ref(). Adjustments will depend on additional details about your application's requirements and structure.

Expand Down