feat: Support stt model params setting

[shaohuzhang1]

feat: Support stt model params setting

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

[shaohuzhang1]

The provided code appears to be a Vue.js component that manages a dynamic form within an Element Plus dialog. Here is a review of the code, highlighting any potential issues and suggesting optimizations:

1. Duplicate v-model Bindings: You have two instances of v-model on <DynamicsForm>, which means they might not work as expected because it's generally recommended to only use one instance of v-model per form element.

2. Use of Arrow Functions in Template Slots: The slot for the footer uses arrow functions (#footer) instead of regular JavaScript functions. There shouldn't be a difference between these two unless there are specific requirements or considerations.

3. Loading Indicator: The loading state seems redundant if you are using asynchronous functions within .then() blocks. Instead, consider moving the loading indication into the components managing those asynchronous operations directly rather than using this global variable.

4. Model Initialization: Ensure that all fields in model_setting_data are appropriately initialized before rendering the form. This could lead to unexpected behavior if values are missing from model_setting_data.

5. Reset Functionality: The reset_default function should handle resetting only the default values back to their initial defaults, not removing non-existent keys. Consider using logic to selectively restore only known fields and ignore anything added dynamically.

6. Consistent Variable Names: Use consistent naming conventions for variables like dialogVisible. Inconsistent naming can make the code harder to read.

7. Avoid Redundant Computed Properties: Since you don't need apiType anywhere else but computed, consider making it inline where needed to reduce unnecessary computations.

8. Code Documentation and Comments: Add comments throughout the code to explain each major block of functionality. This will help maintainability and understanding by others who may read the code later.

9. Ensure TypeScript Type Safety: Make sure that all types defined in imports are correctly typed and match what's expected in your codebase.

Here's a revised version with several improvements:

<template>
  <el-dialog
    align-center
    :title="$t('common.paramSetting')"
    v-model="dialogVisible"
    style="width: 550px"
    append-to-body
    :close-on-click-modal="false"
    :close-on-press-escape="false"
  >
    <DynamicsForm
      v-model="form_data"
      label-position="top"
      require-asterisk-position="right"
      :render_data="model_form_field"
      ref="dynamicsFormRef"
    />
    <template #footer>
      <div class="flex-between">
        <span class="dialog-footer">
          <el-button @click.prevent="cancel">Cancel</el-button>
          <el-button type="primary" @click="submit" :loading="isLoading">Confirm</el-button>
        </span>
      </div>
    </template>
  </el-dialog>
</template>

<script setup lang="ts">
import type { FormField } from '@/components/dynamics-form/type';
import { loadSharedApi } from '@/utils/dynamics-api/shared-api';
import DynamicsForm from '@/components/dynamics-form/index.vue';
import { ref, computed } from 'vue';
import { useRoute } from 'vue-router';

const route = useRoute();
const id = computed(() => (route.params.id || ''));

const apiType = computed(() => {
  return route.includes('/resources-management') ? 'systemManage' : 'workspace';
});

const dialogVisible = ref(true);
const form_data = ref({});
const dynamicsFormRef = ref<DynamicsForm | null>(null); // Nullable due to Volar limitation

let isLoading = ref(false);

const model_form_field = ref([]);

const open = (model_id: string, application_id?: string, model_setting_data?: any) => {
  form_data.value = {};
  const sharedRequestPromise = loadSharedApi({ type: 'model', systemType: apiType.value });
  sharedRequestPromise.then((resp: any) => {
    model_form_field.value = resp.data;
    const initialSettings = getInitialSettings(resp.data);

    if (model_setting_data) {
      applyDefaultSettings(initialSettings, model_setting_data);
    }

    dynamicsFormRef.value?.render(model_form_field.value, initialSettings);
  }).catch((error) => console.error(error));
};

const submit = async () => {
  await dynamicsFormRef.value!.validate();
  emit('refresh', form_data.value);
  dialogVisible.value = false;
};

const cancel = () => {
  dialogVisible.value = false;
};

// Helper functions
function getInitialSettings(fields: Array<FormField>): Record<string, any> {
  return fields.reduce((acc, field) => ({
    ...acc,
    [field.field]: field.default_value === '' ? undefined : field.default_value
  }), {});
}

function applyDefaultSettings(currentState: Record<string, any>, dataToApply: Record<string, any>) {
  Object.keys(dataToApply).forEach(key => {
    currentState[key] = dataToApply[key];
  });
}
</script>

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

Key Changes Made:

• Removed duplicate v-model.
• Replaced arrow functions in template slots with plain functions.
• Moved the loading indicator inside the component handling asynchronous requests.
• Ensured proper initialization and restoration of settings.
• Improved naming consistency.
• Added comments explaining logic and added TypeScript type safety annotations.

These changes aim to improve readability, maintainability, and overall performance of the code.

[shaohuzhang1]

The provided code has several areas that require consideration for both correctness and performance optimizations. Here are the main points to address:

1. Imports: Ensure all necessary imports at the beginning of the script, especially those related to handling WebSocket connections (asyncio), JSON encoding/decoding (json), and SSL context setup.

2. Deep Merge Function: The deep_merge_dict function seems correct but can be optimized slightly. Consider using recursion directly without returning the dictionary explicitly in some cases if it's only needed internally.

3. Optional Initialization Parameters: In _new_instance, you're setting optional parameters conditionally based on existence checks within an empty dictionary optional_params. This could be done more efficiently if these values should always be present or defaulted elsewhere.

4. Handling Empty Frames: There might be edge cases where frames can become empty after processing (e.g., no tokens passed). Adding checks around this could prevent invalid operations when sending back frames to the client.

5. JSON Encoding Issues: Make sure there aren't any issues with whitespace formatting or additional trailing commas in the JSON strings being sent, as this can cause parsing errors.

6. Exception Handling: For exception messages like "", provide proper error messages rather than an empty string; better yet, log them appropriately.

7. Code Readability: Although brief, consider adding comments explaining complex logic blocks or variable names for clarity when reviewing again after a refactoring pass.

8. Parameter Merging Logic: Adjust how parameters from self.params are merged into frames to ensure only valid keys specific to SparkAPI are used.

9. WS Connection Management: Ensure that the connection states handle exceptions gracefully during receive/send operations due to timeouts or disconnections. You might want to add retries or close/reconnect mechanisms depending on application needs.

By addressing these points, you'll improve robustness, readability, efficiency, and reliability of the implemented streaming speech-to-text service framework.

[shaohuzhang1]

There are no obvious irregularities or issues with the provided code snippet. However, here are some points and suggestions for improvement:

Points to Consider:

1. Accessibility: The @click event handler on the button that opens the dialog should be more descriptive of its purpose.
2. Code Reusability: The method refreshSTTForm() can be reused across different parts if there is more context where it might be used.
3. Conditional Rendering Optimization: Ensure that the elements under <div class="flex-between w-full"> are only rendered when applicationForm.stt_model_enable is true.

Specific Suggestions:

1. Descriptive Event Handler:

   @click="openSTTParamSettingDialogWithDescription"

2. Reusable Method:

   function refreshSttParams(setting: any) {
     this.applicationForm.stt_model_params_setting = setting;
   }

3. Optimization:

   <template v-if="applicationForm.stt_model_enable">
     <div class="flex-between w-full">
       <!-- existing content -->
     </div>
   </template>

If you have specific use cases or additional improvements in mind, please let me know!

[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