Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive conversation branching functionality to the LLM API, enabling users to create alternative conversation paths through edit, regenerate, and delete operations. The implementation follows a "swap-to-MAIN" pattern where modified branches become the primary conversation while preserving the original as a backup.
Key changes:
- Implements full branching API with CRUD operations for branches (create, list, get, delete, activate)
- Adds message action endpoints (edit, regenerate) that automatically create branches and swap them to MAIN
- Introduces MessageActionService to handle branch creation and swapping logic
- Updates repository layer with complete branch persistence and item filtering by branch
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/automation/conversations-postman-scripts.json | Adds 19 comprehensive test cases covering all branching operations |
| services/llm-api/internal/interfaces/httpserver/routes/v1/v1_route.go | Registers new BranchRoute in the V1 API router |
| services/llm-api/internal/interfaces/httpserver/routes/v1/conversation/conversation_route.go | Updates deleteItem documentation and adds branch parameter support |
| services/llm-api/internal/interfaces/httpserver/routes/v1/conversation/branch_route.go | Implements new route handlers for branch operations and message actions |
| services/llm-api/internal/interfaces/httpserver/routes/routes_provider.go | Adds BranchHandler and BranchRoute to dependency injection |
| services/llm-api/internal/interfaces/httpserver/requests/conversation/conversation.go | Adds Branch query parameter for filtering items |
| services/llm-api/internal/interfaces/httpserver/handlers/conversationhandler/conversation_handler.go | Updates ListItems and DeleteItem to support branching |
| services/llm-api/internal/interfaces/httpserver/handlers/conversationhandler/branch_handler.go | Implements handler logic for all branch and message action operations |
| services/llm-api/internal/interfaces/httpserver/handlers/chathandler/chat_handler.go | Adds branch awareness and duplicate message detection |
| services/llm-api/internal/infrastructure/database/repository/conversationrepo/conversation_repository.go | Implements all branch repository methods with database operations |
| services/llm-api/internal/domain/provider.go | Adds MessageActionService to service provider |
| services/llm-api/internal/domain/conversation/message_action_service.go | New service implementing edit, regenerate, and delete with branch swapping |
| services/llm-api/internal/domain/conversation/item.go | Adds Branch filter field to ItemFilter |
| services/llm-api/internal/domain/conversation/conversation_service.go | Updates AddItemsToConversation for branch validation and routing |
| services/llm-api/internal/domain/conversation/conversation.go | Adds CreateBranchMetadata helper and SwapBranchToMain interface method |
| services/llm-api/cmd/server/wire_gen.go | Updates wire-generated dependency injection with branching components |
Comments suppressed due to low confidence (2)
services/llm-api/internal/interfaces/httpserver/handlers/chathandler/chat_handler.go:1025
- The comment "Build the user input item" is redundant as it simply repeats what the next line of code does. Consider adding a more meaningful comment explaining why this is separated from the items append operation or remove the comment entirely.
// Extract content from item - handle both text and multimodal content
services/llm-api/internal/interfaces/httpserver/handlers/chathandler/chat_handler.go:1044
- The duplicate detection logic retrieves all items from the branch without pagination, which could cause performance issues and memory problems for conversations with thousands of messages. Only the last item needs to be checked, so consider using pagination with limit=1 and order=desc to fetch only the most recent item.
textParts = append(textParts, *content.TextString)
multiContent = append(multiContent, openai.ChatMessagePart{
Type: openai.ChatMessagePartTypeText,
Text: *content.TextString,
})
} else if content.Text != nil && content.Text.Text != "" {
textParts = append(textParts, content.Text.Text)
multiContent = append(multiContent, openai.ChatMessagePart{
Type: openai.ChatMessagePartTypeText,
Text: content.Text.Text,
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| mainBranch, err := q.ConversationBranch.WithContext(ctx). | ||
| Where(q.ConversationBranch.ConversationID.Eq(conversationID)). | ||
| Where(q.ConversationBranch.Name.Eq("MAIN")). | ||
| First() | ||
|
|
||
| if err == nil && mainBranch != nil { | ||
| // MAIN branch record exists - rename it to backup | ||
| _, err = q.ConversationBranch.WithContext(ctx). | ||
| Where(q.ConversationBranch.ConversationID.Eq(conversationID)). | ||
| Where(q.ConversationBranch.Name.Eq("MAIN")). | ||
| Update(q.ConversationBranch.Name, oldMainBackupName) | ||
| if err != nil { | ||
| return "", platformerrors.AsError(ctx, platformerrors.LayerRepository, err, "failed to rename MAIN branch to backup") | ||
| } | ||
| } else { | ||
| // MAIN branch record doesn't exist - create backup branch for existing MAIN items | ||
| // Count existing MAIN items | ||
| count, err := q.ConversationItem.WithContext(ctx). | ||
| Where(q.ConversationItem.ConversationID.Eq(conversationID)). | ||
| Where(q.ConversationItem.Branch.Eq("MAIN")). | ||
| Count() | ||
| if err != nil { | ||
| return "", platformerrors.AsError(ctx, platformerrors.LayerRepository, err, "failed to count MAIN items") | ||
| } | ||
|
|
||
| if count > 0 { | ||
| // Create a branch record for the backup | ||
| now := time.Now() | ||
| description := "Backup of original MAIN branch" | ||
| backupBranch := &dbschema.ConversationBranch{ | ||
| ConversationID: conversationID, | ||
| Name: oldMainBackupName, | ||
| Description: &description, | ||
| ItemCount: int(count), | ||
| } | ||
| backupBranch.CreatedAt = now | ||
| backupBranch.UpdatedAt = now | ||
|
|
||
| if err := q.ConversationBranch.WithContext(ctx).Create(backupBranch); err != nil { | ||
| return "", platformerrors.AsError(ctx, platformerrors.LayerRepository, err, "failed to create backup branch") | ||
| } | ||
| } else { | ||
| // No MAIN items exist, no backup needed | ||
| oldMainBackupName = "" | ||
| } | ||
| } |
There was a problem hiding this comment.
The SwapBranchToMain function has complex branching logic handling multiple scenarios (existing MAIN branch record vs. only MAIN items). This complexity makes the function harder to maintain and test. Consider extracting the backup creation logic into a separate helper function like "createMainBackup" to improve readability and testability.
services/llm-api/internal/interfaces/httpserver/routes/v1/conversation/branch_route.go
Show resolved
Hide resolved
services/llm-api/internal/interfaces/httpserver/handlers/conversationhandler/branch_handler.go
Show resolved
Hide resolved
...-api/internal/infrastructure/database/repository/conversationrepo/conversation_repository.go
Show resolved
Hide resolved
...-api/internal/infrastructure/database/repository/conversationrepo/conversation_repository.go
Show resolved
Hide resolved
|
|
||
| // Verify it's an assistant message | ||
| if assistantItem.Role == nil || *assistantItem.Role != ItemRoleAssistant { | ||
| return nil, platformerrors.NewError(ctx, platformerrors.LayerDomain, platformerrors.ErrorTypeValidation, "can only regenerate assistant messages", nil, "b2c3d4e5-f6a7-4b8c-9d0e-1f2a3b4c5d6e") |
There was a problem hiding this comment.
The error message "can only regenerate assistant messages" could be more descriptive. Consider changing it to "only assistant messages can be regenerated, cannot regenerate user or system messages" to provide clearer guidance about what went wrong.
| return nil, platformerrors.NewError(ctx, platformerrors.LayerDomain, platformerrors.ErrorTypeValidation, "can only regenerate assistant messages", nil, "b2c3d4e5-f6a7-4b8c-9d0e-1f2a3b4c5d6e") | |
| return nil, platformerrors.NewError(ctx, platformerrors.LayerDomain, platformerrors.ErrorTypeValidation, "only assistant messages can be regenerated, cannot regenerate user or system messages", nil, "b2c3d4e5-f6a7-4b8c-9d0e-1f2a3b4c5d6e") |
This PR adds comprehensive conversation branching functionality to the LLM API, enabling users to create alternative conversation paths through edit, regenerate, and delete operations. The implementation follows a "swap-to-MAIN" pattern where modified branches become the primary conversation while preserving the original as a backup.
Key changes:
Implements full branching API with CRUD operations for branches (create, list, get, delete, activate)
Adds message action endpoints (edit, regenerate) that automatically create branches and swap them to MAIN
Introduces MessageActionService to handle branch creation and swapping logic
Updates repository layer with complete branch persistence and item filtering by branch