pref: Generate prompt

[shaohuzhang1]

pref: Generate prompt

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

Your code seems generally clean and well-organized. Here are a few suggestions:

1. Imports: Ensure you import all functions and classes used before they are needed.

2. Code Reorganization:

   • The PromptGenerateSerializer methods could be separated for better modularization.

3. Error Handling:

   • Consider adding more descriptive error messages or handling potential exceptions more gracefully, especially around file reading (use try-except blocks).

4. Performance Optimization:

   • Check if there might be unnecessary calculations or computations that can be optimized (e.g., caching results where applicable).

5. Security Considerations:

   • Be cautious about paths involving environment variables like PROJECT_DIR. Avoid hardcoding sensitive information directly in the path strings.

Here is an improved version of the key areas mentioned above:

# Importing required libraries at the top
import os
import json
from gettext import gettext as _
from typing import List, Dict

import uuid_utils.compat as uuid
from django.db.models import QuerySet
from django.utils.translation importgettext_lazy as _

from langchain_core.messages import HumanMessage, AIMessage, SystemMessage
from rest_framework import serializers

from application.chat_pipeline.pipeline_manage import PipelineManage
from common.handle.base_to_response import BaseToResponse
from common.handle.impl.response.openai_to_response import OpenaiToResponse
from common.handle.impl.response.system_to_response import SystemToResponse
from common.utils.common import flat_map, get_file_content
from knowledge.models import Document, Paragraph
from maxkb.conf import PROJECT_DIR
from models_provider.models import Model, Status
from models_provider.tools import get_model_instance_by_model_workspace_id


class AppApiException(Exception):
    """Custom exception class."""
    pass


def is_valid(self, *, raise_exception=False):
    # Your existing validation logic here


class ChatMessageSerializers(serializers.Serializer):
    message = serializers.CharField(required=True, label=_("User Questions"))
    stream = serializers.BooleanField(required=True,
                                    help_text=_("Indicates whether responses should be streamed."))


class PromptGenerateSerializer(serializers.Serializer):
    workspace_id = serializers.CharField(required=False, label=_('Workspace ID'))
    model_id = serializers.CharField(required=True, label=_('Model ID'))
    prompt = serializers.JSONField(label=_('prompt'))

    def is_valid_application_exists(self, query_set):
        """Check if an application exists based on its ID."""
        if not query_set.exists():
            raise AppApiException(500, _('Application id does not exist'))

    def generate_prompt(self, instance: dict) -> str:
        self.is_valid(raise_exception=True)
        self.validate()

        app = self.is_valid_application_exists(QuerySet(Application).filter(id=self.validated_data['application_id']))

        workspace_id = self.validated_data.get('workspace_id')
        model_id = self.validated_data.get('model_id')

        prompt = instance.get('prompt', "")

        system_content = get_file_content(os.path.join(PROJECT_DIR, "apps", "chat", 'template', 'generate_prompt_system')).format(
            application_name=app.name, desc=app.desc
        )

        model_existence_check = QuerySet(Model).filter(id=model_id, model_type="LLM").exists()
        if not model_existence_check:
            raise Exception(_("Model does not exists or is not an LLM model"))

        model = get_model_instance_by_model_workspace_id(model_id=model_id, workspace_id=workspace_id, **app.model_params_setting)

        def process() -> str:
            for response in model.stream([
                SystemMessage(content=system_content),
                *[HumanMessage(content=m.get('content')) if m.get('role') == 'user' else AIMessage(content=m.get('content')) for m in instance]']):
                yield f"data: {json.dumps({'completion': response.content})}\n\n"

        return to_stream_response_simple(process())

    def validate(self):
        """Additional validation checks such as model type check."""
        model_id = self.validated_data.get('model_id')
        model_type = get_model_instance_by_model_workspace_id(model_id=model_id)._get_meta().verbose_name
        if model_type != 'LLM':
            self.add_error('Invalid model specified:', 'This endpoint only supports LLM models.')

Key Changes:

• Moved Imports: Moved imports to the beginning of each module.
• Separated Method Calls: Slightly restructured method calls within the generate_prompt method.
• Added Validation Application Exists: A helper method that encapsulates the validation step ensuring the existence of an application.
• Detailed Error Message: Added a description/help text to the serializer fields for clarity.
• Encapsulated Additional Checks: Used another method (validate) to handle additional validations beyond basic data validation by calling .is_valid() initially, which then runs other checks.

These changes ensure robustness and maintainability while adhering to best practices in coding conventions and security considerations.

[shaohuzhang1]

The provided code has a few potential issues and improvements:

1. Missing model_params:

   • The function is_valid calls provider.get_model() without passing arguments for model_params. This might cause an error if get_model expects additional parameters.

2. Potential Exception Handling:

   • While catching exceptions (Exception) can be useful for handling unexpected errors, it's important to handle specific exception types rather than just Exception to get more detailed information. For example, you could catch ValueError, TypeError, etc.

3. Logging Instead of Printing Stack Trace:

   • Using traceback.print_exc() prints the stack trace to the console, which is generally not recommended when integrating into larger systems or APIs where logs should be managed centrally.

4. Docstring Consistency:

   • Ensure that all methods have docstrings to describe their purpose and inputs/outputs.

Here are some improvement suggestions:

def is_valid(self,
              model_type: str,
              model_name: str,
              model_credential: dict,
              model_params=None) -> bool:
    """
    Validates whether the given model configuration is acceptable.

    Args:
        model_type (str): Type of the model (e.g., 'transformer', 'gpt').
        model_name (str): Name of the model configuration file.
        model_credential (dict): Credentials required to access the model.
        model_params (dict, optional): Additional parameters for model instantiation.

    Returns:
        bool: True if valid, otherwise False.

    Raises:
        ValueError: If model type is invalid.
        KeyError: If any expected parameter is missing from model_credentials.
        TypeError: If wrong data types are used for parameters.
    """
    if "name" not in model_credentials or "credential" not in model_credentials:
        raise KeyError("Model credentials must contain at least 'name' and 'credential'.")

    try:
        model = provider.get_model(model_type, model_name, model_credential, **(model_params or {}))
        model.check_auth()
        return True

    except ValueError as ve:
        print(f"Invalid input value: {ve}")
        return False

   except TypeError as te:
        print(f"Type mismatch: {te}")
        return False

    except Exception as e:
        print(f"An error occurred: {e}")
        print(traceback.format_exc())  # Consider logging instead
        return False

Key Changes:

• Added self as the first argument to indicate this is an instance method.
• Fixed the typo in modelParams to model_params.
• Ensured model_params is passed with a check model_params or {} to avoid a KeyError.
• Improved exception handling by catching specific types and providing user-friendly messages.
• Updated the docstring to include descriptions and parameters clearly.