feat: IMG TTV node support reference model

[shaohuzhang1]

feat: IMG TTV node support reference model

[f2c-ci-robot]

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.

Details

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.

[f2c-ci-robot]

[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.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[shaohuzhang1]

The code is mostly correct but can be optimized for better readability and functionality. Here are some suggestions:

Optimal Code

1. Reduce Redundancy:

   • Use ternary operators where possible to reduce repetition.

2. Improve Logic:

   • Ensure that all logic related to model selection and validation is centralized to maintain consistency.

3. Handle State Consistency:

   • Make sure state changes in one component (e.g., cascader or single input) are propagated and reflected correctly.

Here's an improved version of the specified part of your code:

<template>
  <!-- ... existing template content ... -->
</template>

<script lang="ts" setup>
// ... imports and other setup ...

const form = reactive({
  model_id: '',
  model_id_type: 'custom', // Default to custom selection
  model_id_reference: [], // Initialize reference array with empty values
  system: '',
  prompt: defaultPrompt,
  negative_prompt: '',
  guidance_scale_1: 7.5,
  aspect_ratio: '',
  height: 512,
  width: 512,
});

const form_data = computed(() => ({
  ...props.nodeModel.properties.node_data || {},
}));

const model_change = (value: string[]) => {
  form.model_id_reference = value;
};

// Validation function remains the same...

const refreshParam = () => {
  // Refresh parameter logic here...
};

// ... rest of the script ...
</script>

Explanation

• Default Selection: The model_id_type starts as "custom". If not provided in the saved data, it defaults this way.

• Initialization: The model_id_reference array initializes to an empty array when the node first loads.

• Reactive Form Object: The form object uses reactive instead of computed, which allows you to directly modify its properties.

• Consistent Data Model: The entire saved node data is mapped to the reactive form using spread syntax (...). This maintains consistency and reduces redundant checks.

This approach ensures that both single-model and multiple-selection options work consistently across different forms and nodes, optimizing both performance and user experience.

[shaohuzhang1]

The code is mostly well-designed but contains some minor inefficiencies and unnecessary logic.

1. Prop Validation: The prop validation in template literals can be improved for clarity and maintainability:

<span>{{ (form_data.model_id_type === 'reference') ? $t("workflow.variable.placeholder") : $t('workflow.nodes.textToVideoGenerate.model.requiredMessage')}}</span>

It's better to extract this into a helper function or constant for easier readability.

2. Custom Model Select Component Logic: The custom model select component needs to properly handle cases where no models are provided by the API (e.g., an error response). This might require adding additional checks to ensure model_options is not null before trying to iterate over it.

3. Model Reference Handling: When switching between "Reference" and "Custom", ensure that you correctly manage state related to references if they do have any dependencies on non-reference selections.

4. Error Logging: Ensure proper logging of errors across asynchronous operations, such as fetching model options through loadSharedApi. Catching these errors separately would help with debugging.

5. Optimization: Avoid calling multiple methods inside the same event handler unless necessary. For instance, handling events like refreshForm should ideally only update its internal states without performing external validations.

6. Validation Cleanup: The cleanup code after validate() can also be cleaner. Instead of throwing immediately upon encountering an error during initial form validation, catch them and store them appropriately for later user feedback.

Here's an optimized version of the critical parts mentioned above:

Template Improvements

<span :class="{ colorDanger: !valid }">{{ displayRequiredMessage }}</span>

And define a calculated property for displayRequiredMessage:

const form_data = computed({
  get: () => {
    // Handle loading state here
    const properties = props.nodeModel.properties ?? {}
    return deepMerge({}, defaults, properties.node_data) || {}
  },
})

Then add the required method calculation outside of the render tree:

const requiredMessageStyleClass = ''
let valid = false;

if (!this.$refs.aichatNodeFormRef.getRules(this.form).some(r => r.required)) {
  requiredMessageStyleClass = 'color-danger';
}

// If there are existing errors, mark as invalid instead of just styling the label
const isValidRule = this.$refs.aichatNodeFormRef.rules.prompt.some(rule =>
  rule.validator(null, '', {})
);

valid = rules.every(rule => rule.validator(valid));
requiredMessageContent = this.displayRequiredMessage();
requiredMessageStyleClass = `${requiredMessageStyleClass} ${isValidRule ? "" : "-none"`;  

Additional Enhancements

1. Error Messages: Provide more informative error messages when users try to submit invalid data. E.g., suggest what specific field needs attention.

2. Dynamic State Update: Ensure that updating states based on form changes does not cause infinite loops inadvertently due to nested reactive updates.

This approach improves maintainability and enhances the functionality of your form components by leveraging Vue.js' powerful reactive programming capabilities effectively.

[shaohuzhang1]

The provided code has several issues that need to be addressed:

1. Undefined Prop: The model_id_type and model_id_reference props being accessed without checks might lead to errors on some nodes.

2. Duplicate Code: There is duplicate model selection logic in both the referenced node ID list and custom dropdown. This can be optimized.

3. Inconsistent Logic: The logic for handling different values of form_data.model_id_type seems inconsistent, causing redundancy.

4. Potential Security Risks: Directly accessing object properties using dynamic keys (props.nodeModel.properties.node_data[props.field]) without proper validation could expose your application to injection attacks.

Here's an improved version of the code with these issues resolved:

const form_data = computed({
  get: () => {
    if (props.nodeModel.properties.node_data) {
      return props.nodeModel.properties.node_data;
    } else {
      return { ...defaultProps };
    }
  },
});

watch(
  () => props.nodeModel.properties.node_data,
  newValue => {
    Object.assign(form_data.value, newValue);
  }
);

function handleModelTypeChange(value) {
  if (value === 'reference') {
    form_data.value.model_id_reference.splice(0);
  }
}

const ModelSelectRef = ref(null);
const NodeCascaderRef = ref(null);

// Use this function to refresh parameter settings when needed.
async function openAIParamSettingDialog(id): Promise<any> {
  // Implementation here...
}

function wheel() {
  // Wheel event handler implementation here...
}

Key points:

• Simplified logic by removing redundant code.
• Used Vue's computed hook appropriately.
• Added a watcher for node data changes to ensure the component updates.
• Refactored handleModelTypeChange function separately from template rendering.
• Dropped unnecessary dependencies like _defineProp.
• Removed unused elements and comments.

This should help improve stability and performance while maintaining clarity and integrity.