-
Notifications
You must be signed in to change notification settings - Fork 62
feat: gpt5.2, conversation branch #79
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
Open
Junyi-99
wants to merge
10
commits into
main
Choose a base branch
from
feat/gpt-5.2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
1b0c1e2
support gpt 5.2
Junyi-99 5fbff6b
support for reasoning
Junyi-99 074270b
feat: message edit
Junyi-99 355135e
fix: branch id
Junyi-99 eed3292
fix: message branches
Junyi-99 e3bed9c
fix: compatability
Junyi-99 175a717
update faro
Junyi-99 98580ce
fix: UpdatedAt timestamp not updating when appending messages to bran…
Copilot 7f3ef5f
Merge remote-tracking branch 'origin/main' into feat/gpt-5.2
Junyi-99 9cca5b4
Merge branch 'main' into feat/gpt-5.2
Junyi-99 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Some comments aren't visible on the classic Files Changed page.
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,6 +2,8 @@ package chat | |
|
|
||
| import ( | ||
| "context" | ||
| "time" | ||
|
|
||
| "paperdebugger/internal/api/mapper" | ||
| "paperdebugger/internal/libs/contextutil" | ||
| "paperdebugger/internal/libs/shared" | ||
|
|
@@ -137,7 +139,7 @@ func (s *ChatServerV2) createConversation( | |
| } | ||
|
|
||
| // appendConversationMessage appends a message to the conversation and writes it to the database | ||
| // Returns the Conversation object | ||
| // Returns the Conversation object and the active branch | ||
| func (s *ChatServerV2) appendConversationMessage( | ||
| ctx context.Context, | ||
| userId bson.ObjectID, | ||
|
|
@@ -146,52 +148,79 @@ func (s *ChatServerV2) appendConversationMessage( | |
| userSelectedText string, | ||
| surrounding string, | ||
| conversationType chatv2.ConversationType, | ||
| ) (*models.Conversation, error) { | ||
| parentMessageId string, | ||
| ) (*models.Conversation, *models.Branch, error) { | ||
| objectID, err := bson.ObjectIDFromHex(conversationId) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| conversation, err := s.chatServiceV2.GetConversationV2(ctx, userId, objectID) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| // Ensure branches are initialized (migrate legacy data if needed) | ||
| conversation.EnsureBranches() | ||
|
|
||
| var activeBranch *models.Branch | ||
|
|
||
| // Handle branching / edit mode | ||
| if parentMessageId != "" { | ||
| // Create a new branch for the edit | ||
| activeBranch = conversation.CreateNewBranch("", parentMessageId) | ||
| if activeBranch == nil { | ||
| return nil, nil, shared.ErrBadRequest("Failed to create new branch") | ||
| } | ||
| } else { | ||
| // Normal append - use active (latest) branch | ||
| activeBranch = conversation.GetActiveBranch() | ||
| if activeBranch == nil { | ||
| // This shouldn't happen after EnsureBranches, but handle it | ||
| return nil, nil, shared.ErrBadRequest("No active branch found") | ||
| } | ||
| } | ||
|
|
||
| // Now we get the branch, we can append the message to the branch. | ||
| userMsg, userOaiMsg, err := s.buildUserMessage(ctx, userMessage, userSelectedText, surrounding, conversationType) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| bsonMsg, err := convertToBSONV2(userMsg) | ||
| if err != nil { | ||
| return nil, err | ||
| return nil, nil, err | ||
| } | ||
| conversation.InappChatHistory = append(conversation.InappChatHistory, bsonMsg) | ||
| conversation.OpenaiChatHistoryCompletion = append(conversation.OpenaiChatHistoryCompletion, userOaiMsg) | ||
|
|
||
| // Append to the active branch | ||
| activeBranch.InappChatHistory = append(activeBranch.InappChatHistory, bsonMsg) | ||
| activeBranch.OpenaiChatHistoryCompletion = append(activeBranch.OpenaiChatHistoryCompletion, userOaiMsg) | ||
| activeBranch.UpdatedAt = bson.NewDateTimeFromTime(time.Now()) | ||
|
|
||
| if err := s.chatServiceV2.UpdateConversationV2(conversation); err != nil { | ||
| return nil, err | ||
| return nil, nil, err | ||
| } | ||
|
|
||
| return conversation, nil | ||
| return conversation, activeBranch, nil | ||
| } | ||
|
|
||
| // prepare creates a new conversation if conversationId is "", otherwise appends a message to the conversation | ||
| // conversationType can be switched multiple times within a single conversation | ||
| func (s *ChatServerV2) prepare(ctx context.Context, projectId string, conversationId string, userMessage string, userSelectedText string, surrounding string, modelSlug string, conversationType chatv2.ConversationType) (context.Context, *models.Conversation, *models.Settings, error) { | ||
| // Returns: context, conversation, activeBranch, settings, error | ||
| func (s *ChatServerV2) prepare(ctx context.Context, projectId string, conversationId string, userMessage string, userSelectedText string, surrounding string, modelSlug string, conversationType chatv2.ConversationType, parentMessageId string) (context.Context, *models.Conversation, *models.Branch, *models.Settings, error) { | ||
| actor, err := contextutil.GetActor(ctx) | ||
| if err != nil { | ||
| return ctx, nil, nil, err | ||
| return ctx, nil, nil, nil, err | ||
| } | ||
|
|
||
| project, err := s.projectService.GetProject(ctx, actor.ID, projectId) | ||
| if err != nil && err != mongo.ErrNoDocuments { | ||
| return ctx, nil, nil, err | ||
| return ctx, nil, nil, nil, err | ||
| } | ||
|
|
||
| userInstructions, err := s.userService.GetUserInstructions(ctx, actor.ID) | ||
| if err != nil { | ||
| return ctx, nil, nil, err | ||
| return ctx, nil, nil, nil, err | ||
| } | ||
|
|
||
| var latexFullSource string | ||
|
|
@@ -200,18 +229,20 @@ func (s *ChatServerV2) prepare(ctx context.Context, projectId string, conversati | |
| latexFullSource = "latex_full_source is not available in debug mode" | ||
| default: | ||
| if project == nil || project.IsOutOfDate() { | ||
| return ctx, nil, nil, shared.ErrProjectOutOfDate("project is out of date") | ||
| return ctx, nil, nil, nil, shared.ErrProjectOutOfDate("project is out of date") | ||
| } | ||
|
|
||
| latexFullSource, err = project.GetFullContent() | ||
| if err != nil { | ||
| return ctx, nil, nil, err | ||
| return ctx, nil, nil, nil, err | ||
| } | ||
| } | ||
|
|
||
| var conversation *models.Conversation | ||
| var activeBranch *models.Branch | ||
|
|
||
| if conversationId == "" { | ||
| // Create a new conversation | ||
| conversation, err = s.createConversation( | ||
| ctx, | ||
| actor.ID, | ||
|
|
@@ -225,31 +256,38 @@ func (s *ChatServerV2) prepare(ctx context.Context, projectId string, conversati | |
| modelSlug, | ||
| conversationType, | ||
| ) | ||
| if err != nil { | ||
| return ctx, nil, nil, nil, err | ||
| } | ||
| // For new conversations, ensure branches and get the active one | ||
| conversation.EnsureBranches() | ||
| activeBranch = conversation.GetActiveBranch() | ||
| } else { | ||
| conversation, err = s.appendConversationMessage( | ||
| // Append to an existing conversation | ||
| conversation, activeBranch, err = s.appendConversationMessage( | ||
| ctx, | ||
| actor.ID, | ||
| conversationId, | ||
| userMessage, | ||
| userSelectedText, | ||
| surrounding, | ||
| conversationType, | ||
| parentMessageId, | ||
| ) | ||
| } | ||
|
|
||
| if err != nil { | ||
| return ctx, nil, nil, err | ||
| if err != nil { | ||
| return ctx, nil, nil, nil, err | ||
| } | ||
| } | ||
|
|
||
| ctx = contextutil.SetProjectID(ctx, conversation.ProjectID) | ||
| ctx = contextutil.SetConversationID(ctx, conversation.ID.Hex()) | ||
|
|
||
| settings, err := s.userService.GetUserSettings(ctx, actor.ID) | ||
| if err != nil { | ||
| return ctx, conversation, nil, err | ||
| return ctx, conversation, activeBranch, nil, err | ||
| } | ||
|
|
||
| return ctx, conversation, settings, nil | ||
| return ctx, conversation, activeBranch, settings, nil | ||
| } | ||
|
|
||
| func (s *ChatServerV2) CreateConversationMessageStream( | ||
|
|
@@ -259,15 +297,17 @@ func (s *ChatServerV2) CreateConversationMessageStream( | |
| ctx := stream.Context() | ||
|
|
||
| modelSlug := req.GetModelSlug() | ||
| ctx, conversation, settings, err := s.prepare( | ||
| ctx, conversation, activeBranch, settings, err := s.prepare( | ||
| ctx, | ||
| req.GetProjectId(), | ||
| req.GetConversationId(), | ||
| req.GetUserMessage(), | ||
| req.GetUserSelectedText(), | ||
|
|
||
| req.GetSurrounding(), | ||
| modelSlug, | ||
| req.GetConversationType(), | ||
| req.GetParentMessageId(), | ||
| ) | ||
| if err != nil { | ||
| return s.sendStreamError(stream, err) | ||
|
|
@@ -278,12 +318,13 @@ func (s *ChatServerV2) CreateConversationMessageStream( | |
| APIKey: settings.OpenAIAPIKey, | ||
| } | ||
|
|
||
| openaiChatHistory, inappChatHistory, err := s.aiClientV2.ChatCompletionStreamV2(ctx, stream, conversation.ID.Hex(), modelSlug, conversation.OpenaiChatHistoryCompletion, llmProvider) | ||
| // Use active branch's history for the LLM call | ||
| openaiChatHistory, inappChatHistory, err := s.aiClientV2.ChatCompletionStreamV2(ctx, stream, conversation.ID.Hex(), modelSlug, activeBranch.OpenaiChatHistoryCompletion, llmProvider) | ||
| if err != nil { | ||
| return s.sendStreamError(stream, err) | ||
| } | ||
|
|
||
| // Append messages to the conversation | ||
| // Append messages to the active branch | ||
| bsonMessages := make([]bson.M, len(inappChatHistory)) | ||
| for i := range inappChatHistory { | ||
| bsonMsg, err := convertToBSONV2(&inappChatHistory[i]) | ||
|
|
@@ -292,16 +333,17 @@ func (s *ChatServerV2) CreateConversationMessageStream( | |
| } | ||
| bsonMessages[i] = bsonMsg | ||
|
Comment on lines
333
to
334
|
||
| } | ||
| conversation.InappChatHistory = append(conversation.InappChatHistory, bsonMessages...) | ||
| conversation.OpenaiChatHistoryCompletion = openaiChatHistory | ||
| activeBranch.InappChatHistory = append(activeBranch.InappChatHistory, bsonMessages...) | ||
| activeBranch.OpenaiChatHistoryCompletion = openaiChatHistory | ||
| activeBranch.UpdatedAt = bson.NewDateTimeFromTime(time.Now()) | ||
| if err := s.chatServiceV2.UpdateConversationV2(conversation); err != nil { | ||
| return s.sendStreamError(stream, err) | ||
| } | ||
|
|
||
| if conversation.Title == services.DefaultConversationTitle { | ||
| go func() { | ||
| protoMessages := make([]*chatv2.Message, len(conversation.InappChatHistory)) | ||
| for i, bsonMsg := range conversation.InappChatHistory { | ||
| protoMessages := make([]*chatv2.Message, len(activeBranch.InappChatHistory)) | ||
| for i, bsonMsg := range activeBranch.InappChatHistory { | ||
| protoMessages[i] = mapper.BSONToChatMessageV2(bsonMsg) | ||
| } | ||
| title, err := s.aiClientV2.GetConversationTitleV2(ctx, protoMessages, llmProvider) | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -29,7 +29,10 @@ func (s *ChatServerV2) GetConversation( | |
| return nil, err | ||
| } | ||
|
|
||
| // Use specified branch_id if provided, otherwise use active branch | ||
| branchID := req.GetBranchId() | ||
|
|
||
| return &chatv2.GetConversationResponse{ | ||
| Conversation: mapper.MapModelConversationToProtoV2(conversation), | ||
| Conversation: mapper.MapModelConversationToProtoV2WithBranch(conversation, branchID), | ||
|
Comment on lines
+32
to
+36
|
||
| }, nil | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing nil check for activeBranch in new conversations
High Severity
For new conversations,
GetActiveBranch()is called without a nil check afterEnsureBranches(). This is inconsistent with the existing conversation path inappendConversationMessage, which properly checks for nil at lines 175-179. IfGetActiveBranch()returns nil (e.g., ifEnsureBranches()doesn't create a branch), accessingactiveBranch.OpenaiChatHistoryCompletionat line 319 will cause a nil pointer dereference and crash the server.Additional Locations (1)
internal/api/chat/create_conversation_message_stream_v2.go#L318-L319