Repository navigation
Branch a chat - #95
sanskaraggarwal2025 wants to merge 2 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @sanskaraggarwal2025, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request significantly refactors the backend chat service to implement a hierarchical chat history model, addressing the 'reference approach' for chat messages. The core change involves modifying how chat messages are retrieved, moving from a flat list to a recursive query that includes messages from parent chats up to their branch points. Concurrently, the chat branching mechanism has been updated to no longer duplicate historical messages, instead relying on this new hierarchical retrieval to reference past conversations. This ensures a more efficient and logically consistent representation of chat history, with new validation checks added to maintain data integrity during branching operations.
Highlights
- Hierarchical Chat Message Retrieval: The
GetChatMessagesfunction now utilizes a recursive Common Table Expression (CTE) to retrieve messages. This allows it to fetch not only messages from the current chat but also from its entire accessible parent chat hierarchy, up to the point where a branch was made. This is a fundamental shift towards a hierarchical chat history. - New Validation Methods for Chat Hierarchy: Two new DAO methods,
ChatExistsandMessageExistsInChatHierarchy, have been introduced. These methods are crucial for validating the existence and accessibility of chats and specific messages within the new hierarchical structure, ensuring data integrity during operations like branching. - Reference-Based Chat Branching: The
BranchChatDAO method has been modified to no longer copy historical messages to the newly created branch. This change signifies a move to a reference-based branching model, where new branches implicitly reference the history of their parent chats, rather than duplicating data. - Updated Chat Branching Service Logic: The
BranchAChatservice logic has been updated to remove the previousIsMainBranchrestriction. It now leverages the newly introducedChatExistsandMessageExistsInChatHierarchyDAO methods to perform robust validation, allowing users to branch from any valid and accessible message within their chat history.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a 'reference approach' for chat branching, where branched chats refer to their parent's messages instead of copying them. This is achieved by modifying GetChatMessages to use a recursive SQL query to walk the chat hierarchy. New DAO methods ChatExists and MessageExistsInChatHierarchy are added for validation.
My review has identified a critical bug in the implementation of MessageExistsInChatHierarchy that could lead to incorrect authorization checks for messages in parent chats. I have also pointed out some leftover debugging statements that should be removed. Overall, the changes align with the goal of a referential branching model, but the identified issues need to be addressed before merging.
| WHERE cm.id = ? AND cm.user_id = ? | ||
| AND ( | ||
| ch.level = 0 -- Current chat, any message is valid | ||
| OR (ch.level > 0 AND cm.id <= ch.parent_message_id) -- Parent chats, only up to branch point |
There was a problem hiding this comment.
The logic for checking messages in parent chats is incorrect. ch.parent_message_id refers to the parent of the chat at the current level of recursion, not the message ID from which the original chat was branched.
Additionally, parent_message_id is a TEXT column, while cm.id is likely an integer. The comparison cm.id <= ch.parent_message_id will perform a lexicographical (string) comparison instead of a numerical one, which will produce incorrect results. You need to CAST parent_message_id to an integer for the comparison to be correct.
The correct approach is to get the parent_message_id from the original chat (at level = 0) and use that for comparison, similar to how it's done in GetChatMessages.
| OR (ch.level > 0 AND cm.id <= ch.parent_message_id) -- Parent chats, only up to branch point | |
| OR (ch.level > 0 AND cm.id <= (SELECT CAST(parent_message_id AS INTEGER) FROM chat_hierarchy WHERE level = 0)) -- Parent chats, only up to branch point |
| } | ||
|
|
||
| newChatId := uuid.New().String() | ||
| fmt.Println("newChatId", newChatId) |
There was a problem hiding this comment.
This fmt.Println call appears to be a leftover debugging statement and should be removed before merging.
| return "", fmt.Errorf("failed to create branch: %v", err) | ||
| } | ||
|
|
||
| fmt.Println("newChatId", newChatId) |
There was a problem hiding this comment.
This fmt.Println call appears to be a leftover debugging statement and should be removed before merging.
| AddChatMessage(userID string, chatId string, role string, content string) error | ||
| AddChatMessageWithTokens(userID string, chatId string, role string, content string, model string, inputTokens int, outputTokens int) (int64, error) | ||
| GetChatMessages(userID string, chatId string) ([]ChatMessageRow, error) | ||
| ChatExists(userID string, chatId string) (bool, error) |
There was a problem hiding this comment.
DoesChatExist is more readable
| AddChatMessageWithTokens(userID string, chatId string, role string, content string, model string, inputTokens int, outputTokens int) (int64, error) | ||
| GetChatMessages(userID string, chatId string) ([]ChatMessageRow, error) | ||
| ChatExists(userID string, chatId string) (bool, error) | ||
| MessageExistsInChatHierarchy(userID string, chatId string, messageId string) (bool, error) |
There was a problem hiding this comment.
DoesMessageExistInChatHistory
xask00
changed the title
…to get-messages-reference-approach
|
Warning Rate limit exceeded@sanskaraggarwal2025 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 13 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
No description provided.