Skip to content

Conversation

@shaohuzhang1
Copy link
Contributor

fix: Models rerank stt

@f2c-ci-robot
Copy link

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

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
Copy link

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

Details 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


def get_model_params_setting_form(self, model_name):
return TencentSSTModelParams()
pass
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 snippet appears to be incomplete, with only the opening line of TencentSSTModelParams repeated and missing closing curly brackets (}). This will likely cause an syntax error when running the code. There are also no other major issues present beyond this formatting problem.

To optimize the code, consider adding docstrings to each class method if they don't already exist. Additionally, ensure that all imported modules are properly defined at the beginning of the file.

Here's a corrected version:

from models_provider.base_model_provider import BaseModelCredential, ValidCode

class TencentSSTModelParams(BaseForm):
    EngSerViceType = forms.SingleSelect(
        TooltipLabel(_('Engine model type'), _('If not passed, the default value is 16k_zh (Chinese universal)')),
        required=True,
        default_value='16k_zh',
        option_list=[
            {"value": "8k_zh", "label": _("Chinese telephone universal")},
            {"value": "8k_en", "label": _("English telephone universal")},
            {"value": "16k_zh", "label": _("Commonly used in Chinese")},
            {"value": "16k_zh-PY", "label": _("Chinese, English, and Guangdong")},
            {"value": "16k_zh_medical", "label": _("Chinese medical")},
            {"value": "16k_en", "label": _("English")},
            {"value": "16k_yue", "label": _("Cantonese")},
            {"value": "16k_ja", "label": _("Japanese")},
            {"value": "16k_ko", "label": _("Korean")},
            {"value": "16k_vi", "label": _("Vietnamese")},
            {"value": "16k_ms", "label": _("Malay language")},
            {"value": "16k_id", "label": _("Indonesian language")},
            {"value": "16k_fil", "label": _("Filipino language")},
            {"value": "16k_th", "label": _("Thai")},
            {"value": "16k_pt", "label": _("Portuguese")},
            {"value": "16k_tr", "label": _("Turkish")},
            {"value": "16k_ar", "label": _("Arabic")},
            {"value": "16k_es", "label": _("Spanish")},
            {"value": "16k_hi", "label": _("Hindi")},
            {"value": "16k_fr", "label": _("French")},
            {"value": "16k_de", "label": _("German")},
            {"value": "16k_zh_dialect", "label": _("Multiple dialects, supporting 23 dialects")}
        ],
        value_field='value',
        text_field='label'
    )

class TencentSTTModelCredential(BaseForm, BaseModelCredential):
    REQUIRED_FIELDS = ["SecretId", "SecretKey"]

    def SecretId(self):
        """
        Returns the secret ID field.
        
        :return: The SecretId form field instance.
        """
        return forms.CharField('SecretId', required=True)

    def SecretKey(self):
        """
        Returns the secret key field.
        
        :return: The SecretKey form field instance.
        """
        return forms.PasswordInputField('SecretKey', required=True)

    def get_model_params_setting_form(self, model_name):
        """
        Retrieves the model parameters setting form based on the given model name.
        
        :param model_name: Name of the model.
        :type model_name: str
        :return: An instance of TencentSSTModelParams.
        :rtype: TencentSSTModelParams
        """
        # Implement logic to get model-specific params form
        return TencentSSTModelParams()

This should improve readability and maintainability.

@zhanweizhang7 zhanweizhang7 merged commit 20cf018 into v2 Aug 26, 2025
3 of 5 checks passed
@zhanweizhang7 zhanweizhang7 deleted the pr@v2@fix_models_rerank_stt branch August 26, 2025 03:58
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 code looks mostly correct, but there are a few issues to consider:

  1. Indentation: The code has inconsistent indentation between lines 3 and 22. It would be good practice to use consistent spacing to avoid confusion.

  2. Duplicate Code: There is repetition in the AliyunBaiLianOmiStTModelParams class definition starting from line 7 to line 28. While this might not affect functionality, it can clutter the code and make maintenance more difficult. Consider using inheritance or composition if these fields are common across different classes.

  3. Comments Formatting: The comments around parameters seem to have mixed formats (#) which could lead to inconsistencies. Consistently using # throughout the file might improve readability.

  4. End of Line Characters: The trailing newline character at the end of the file is unnecessary. This can cause unexpected behavior when working with source control systems that manage files based on differences.

Here's an improved version of the code with some adjustments:

"""Module containing credential settings form for ALiYun BaiLian OMI STT model."""

from ..base_model_provider import BaseModelCredential
from django.utils.translation import gettext_lazy as _

class AliyunBaiLianOmiSTTModelParams(forms.Form):
    """Form settings to customize parameters for the ALiYun BaiLian OMI STT model."""
    
    cue_word = forms.CharField(
        label=_('CueWord'),
        help_text=_('If not passed, the default value is "What is this audio saying? Only answer the audio content"'),
        required=True,
        initial='这段音频在说什么,只回答音频的内容',
       )

class AliyunBaiLianOmiSTTModelCredential(BaseForm, BaseModelCredential):
    """Form credential settings to configure access credentials for the ALiYun BaiLian OMI STT model."""
    
    # Add any specific credential-related fields here

    def encryption_dict(self, model: Dict[str, object]) -> Dict[str, object]:
        """
        Generate dictionary representing encrypted model configuration.

        Parameters:
            model (Dict[str, object]): Configuration model data.

        Returns:
            Dict[str, object]: Encrypted configuration dictionary.
        """
        # Implement encryption logic here
        raise NotImplementedError("Encryption not implemented")

    def get_model_params_setting_form(self, model_name) -> FormType:
        """Get the form used to set up parameter configurations for the model."""
        
        return AliyunBaiLianOmiSTTModelParams()

Key Changes:

  • Consistent Indentation: Used 4 spaces per level of indentation.
  • Removed Duplicate Code: Unified the form settings into one AliyunBaiLianOmiSTTModelParams class.
  • Comments Improve Readability: Clarified comments and removed extra symbols like #.
  • File Cleanup: Removed trailing newline characters.
  • Added docstrings and type hints wherever appropriate for better understanding and maintainability.

These changes should help improve the overall structure and clarity of the code while maintaining its functionality.

raise Exception('Error: ', response.message)

except Exception as err:
maxkb_logger.error(f":Error: {str(err)}: {traceback.format_exc()}")
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 seems to be for converting speech from an audio file to text using a translation API. The function speech_to_text handles both successful and unsuccessful responses. Here's a brief critique:

Irregularities:

  1. Logging of Error: In the exception block, logging is used without specifying log level. It would be good practice to use appropriate log levels like error, debug, etc., to better trace errors.

  2. Traceback Handling: When you capture exceptions (Exception as err), it's common to include the traceback at some point. However, the current implementation doesn't explicitly append the traceback.

  3. API Endpoint: There's no information about which endpoint this API uses. While the code suggests it involves speech-to-text translation, the exact functionality may vary based on what API is being used.

  4. Result Format: The response format "message" is specified, but not all APIs return such a formatted JSON. Ensure that the correct fields exist in the response to correctly extract the translation text.

  5. Response Status Check: A specific status code (e.g., 200) needs to be checked before attempting to access the response content in case an HTTP error occurs. Currently, only successful responses are processed.

def speech_to_text(self, audio_file):
    try:
        # Assuming 'chatGPT' is initialized elsewhere (client setup for API calls)
        response = chatGPT.complete(
            model_id=model,
            prompt=prompt,
            messages=messages,
            result_format="message"
        )

        if response.status_code == 200:
            text = response["output"]["choices"][0].get("message", {}).get("content", []).get(0).get("text")
            return text

        elif response.status_code >= 400:
            raise Exception(f"HTTP request failed with status {response.status_code}: '{response.json().get('detail', {})['message']}'")

        else:
            raise Exception("Unexpected response received")

    except Exception as err:
        maxkb_logger.error(f"Error during speech-to-text conversion process: {err}", exc_info=True)

    return None

Optimization Suggestions:

  1. Error Message Customization: Customize the error message to provide more context about why the conversion might have failed, especially when encountering non-successful status codes.

  2. Rate Limits Management: Implement rate limiting logic in production to manage API usage limits effectively.

  3. Retry Mechanism: Add a retry mechanism for transient failures, though note that excessive retries can lead to abuse detection if your service provider has ratelimiting policies.

  4. Testing: Thoroughly test various scenarios, including edge cases (e.g., handling empty input files or invalid media type).

By addressing these points, the code improves robustness and usability, making it easier to integrate into larger applications.

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.

3 participants