Skip to content
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.

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

perf: Support deepseek #1634

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

perf: Support deepseek #1634

wants to merge 1 commit into from

Conversation

fit2bot
Copy link
Contributor

@fit2bot fit2bot commented Feb 6, 2025

perf: Support deepseek

@fit2bot fit2bot requested a review from a team February 6, 2025 11:26
newContent = response.Choices[0].Delta.Content
}

content += newContent
conn.AnswerCh <- content
}
}
Copy link
Member

Choose a reason for hiding this comment

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

There are no obvious errors or bugs found in this code. However, there could be some redundant lines and inefficient coding practices that can be optimized. For instance, if an input line is shorter than a predefined buffer size (say 2048 bytes), it's better to break up the data into smaller chunks rather than concatenating them in one go, as it may make the system more efficient.

Additionally, if certain fields should always have particular values during initialization (e.g., use "model" instead of importing strings from golang packages when initializing models), you might consider using environment variables or JSON configuration files.

For further details on how I arrived at my suggestions, please refer to each suggestion below and ensure they address only the issue areas mentioned:

  1. Redundant Code Reorganization

    • Extract common parts into separate functions/methods
  2. More Efficient Data Handling / Buffer Sizing

    • Use buffered inputs to handle long responses faster
  3. Better Structure for Initialization and Configuration

    • Utilize environment variables/JSON config files where applicable

CreateTime: time.Now(),
Type: messageType,
Role: openai.ChatMessageRoleAssistant,
IsReasoning: isReasoning,
}
data, _ := json.Marshal(message)
msg := Message{
Copy link
Member

Choose a reason for hiding this comment

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

There were no significant code changes that I could note from the snippet provided.

Potential improvement suggestion:

You can optimize this function to reduce unnecessary loops, especially the if conditions. For example, you don't need to set answerCh.Done() explicitly when there's a single response waiting in chatgptAnswerer, it can be handled with a simple call like chat.Send(session.MessageResponse(answer)).

Here's an optimized version of the sendSessionMessage function:

func (h *chat) sendSessionMessage(id string, answer []byte, messageID int, messageType string) {
 	var reasonType string = ""
  
 	if !openAIConn.IsReasoning { // If only a general response
  	reasonType = "default"
 	} else if openAIConn.IsReasoning && len(openAIConn.RateLimiter.ResponsesInQueue()) <= 1 { // If reasoning with rate-limited responses
    reasonType = "reasoning"
 	}

 	message := struct{ ID int; Content, Type, Role string }{
 		ID:      messageID,
 		Content: string(answer),
 		Type:    messageType,
 		Role:    openai.ChatMessageRoleAssistant,
 	}

 	data, err := json.Marshal(&message)
 	check(err)

 	msg := Message{
 		MessageID:        message.ID,
 		MediaType:        MessageTypeText,
 		Difficulty:       DifficultLevelDefault,
 		CreationTime:     time.Now().UTC(),
 		SessionIndexName: h.session.GetIndexName(),
 		CreatedByUserName: h.User.Name,
 		Title:            h.Title,
 		ChatIdAndUserId:  h.idAndUserIds[id],
 		Priority:         PriorityMediumDefault,
 	}, data

 	err = gevent.Queue(msg)
 	if err != nil {
 		log.Println("Error sending session messages due to gevent failures:")

Keep these comments in mind while analyzing and modifying the existing implementation as needed.
No issue was found between the two versions. However, for future reference, make sure to add appropriate debug lines into your code to analyze runtime errors efficiently.

CreateTime time.Time `json:"create_time,omitempty"`
Type string `json:"type"`
Role string `json:"role"`
IsReasoning bool `json:"is_reasoning"`
}
Copy link
Member

Choose a reason for hiding this comment

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

There seems to be an error on line 164 due to missing curly braces before the JSON fields.

The correct code is:

type ChatGPTMessage struct {
    ID      string            `json:"id"`
    Content string            `json:"content"`
    CreateTime time.Time        `json:"create_time,omitempty"`
    Type     string            `json:"type"`
    Role     string            `json:"role"`
    IsReasoning bool             `json:"is_reasoning"`

}

Regarding optimizations, there isn't much improvement without more specific details about what needs optimized. However, we could consider reducing unnecessary complexity in this particular structure. For example,

  • Use interfaces instead of multiple inheritance
  • Reduce nested types

Here's how the refined version might look:

// ChatGPTModel defines an interface that can create different versions of a Message object.

import (
    "time"
)

type ChatGPTModel interface {
    NewMessageFromInterface(id int) *ChatGPTMessage
}

type ChatGPTTypeA message.Message implements ChatGPTModel

func (c ChatGPTTypeA) NewMessageFromInterface(id int) *ChatGPTMessage {

    return &ChatGPTMessage{
        ID:               strconv.Itoa(id),
        Content:          "",
        CreateTime:       nil,
        Type:             "message",
        Role:             "human",
    }
}

Copy link

sonarqubecloud bot commented Feb 6, 2025

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