π fix: fixed the group topic copy not right#11730
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideEnhances topic duplication logic so that duplicated messages preserve correct parent-child relationships and tool/plugin associations, and adds tests to validate these behaviors. Sequence diagram for topic duplication with messages and pluginssequenceDiagram
participant TopicModel
participant DB as DatabaseTx
participant Messages
participant MessagePlugins
participant IdGen as IdGenerator
TopicModel->>DB: beginTransaction
TopicModel->>DB: select messages where topicId,userId orderBy createdAt
DB-->>TopicModel: originalMessages
TopicModel->>TopicModel: build messageIds from originalMessages
TopicModel->>DB: select messagePlugins where id in messageIds
DB-->>TopicModel: originalPlugins
TopicModel->>TopicModel: build idMap oldMessageId->newMessageId
loop build toolIdMap
TopicModel->>IdGen: generate tool id for each tool in message.tools
IdGen-->>TopicModel: newToolId
TopicModel->>TopicModel: store toolIdMap oldToolId->newToolId
end
loop for each message in originalMessages (in order)
TopicModel->>TopicModel: newId = idMap[message.id]
TopicModel->>TopicModel: newParentId = idMap[message.parentId]
TopicModel->>TopicModel: remap tools using toolIdMap
TopicModel->>DB: insert into messages (newId,newParentId,topicId,tools,clientId=null)
DB-->>TopicModel: insertedMessage
TopicModel->>TopicModel: plugin = find originalPlugins by plugin.id == message.id
alt plugin exists
TopicModel->>TopicModel: newToolCallId = toolIdMap[plugin.toolCallId]
TopicModel->>DB: insert into messagePlugins (id=newId,toolCallId=newToolCallId,clientId=null)
DB-->>TopicModel: insertedPlugin
end
end
TopicModel->>DB: commitTransaction
ER diagram for topics, messages, and messagePlugins duplication relationshipserDiagram
Topic {
string id
}
Message {
string id
string topicId
string userId
string parentId
json tools
datetime createdAt
}
MessagePlugin {
string id
string toolCallId
string clientId
}
Topic ||--o{ Message : has_messages
Message ||--o{ Message : parent_child
Message ||--o{ MessagePlugin : has_plugins
Tool {
string id
}
Message }o--o{ Tool : embedded_tools
MessagePlugin }o--|| Tool : tool_call_references
Class diagram for TopicModel duplication logic changesclassDiagram
class TopicModel {
+string userId
+duplicateTopicWithMessagesAndPlugins(tx, topicId)
}
class Message {
+string id
+string topicId
+string userId
+string parentId
+json tools
+datetime createdAt
}
class MessagePlugin {
+string id
+string toolCallId
+string clientId
}
class IdGenerator {
+string generateMessagesId()
}
class Tool {
+string id
}
TopicModel --> Message : duplicates
TopicModel --> MessagePlugin : duplicates
TopicModel --> IdGenerator : uses
Message "1" --> "0..*" Message : parent_child
Message "1" --> "0..*" MessagePlugin : has_plugins
Message "1" --> "0..*" Tool : tools_array
MessagePlugin "1" --> "0..1" Tool : toolCallId_refers_to
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
TestGru AssignmentSummary
Tip You can |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The use of
any[]andas anyfortoolsmakes the new logic harder to reason about and risks silent breakage if the shape changes; consider introducing a properTooltype/interface and using that throughout the duplication flow. - In the duplication loop you do
originalPlugins.find((p) => p.id === message.id)on each iteration; for better clarity and performance, build aMapfrommessagePlugins.idto plugin once up front and do O(1) lookups in the loop. - Tool IDs are regenerated via
toolIdMapusingtoolu_${idGenerator('messages')}, which couples tool IDs to the messages ID generator and introduces an extra prefix; it may be cleaner to either reuse the original tool IDs where safe or introduce a dedicated tool ID generator/strategy for more predictable identifiers.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The use of `any[]` and `as any` for `tools` makes the new logic harder to reason about and risks silent breakage if the shape changes; consider introducing a proper `Tool` type/interface and using that throughout the duplication flow.
- In the duplication loop you do `originalPlugins.find((p) => p.id === message.id)` on each iteration; for better clarity and performance, build a `Map` from `messagePlugins.id` to plugin once up front and do O(1) lookups in the loop.
- Tool IDs are regenerated via `toolIdMap` using `toolu_${idGenerator('messages')}`, which couples tool IDs to the messages ID generator and introduces an extra prefix; it may be cleaner to either reuse the original tool IDs where safe or introduce a dedicated tool ID generator/strategy for more predictable identifiers.Help me be more useful! Please click π or π on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
π‘ Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c32519bf96
βΉοΈ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with π.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const originalMessages = await tx | ||
| .select() | ||
| .from(messages) | ||
| .where(and(eq(messages.topicId, topicId), eq(messages.userId, this.userId))); | ||
|
|
||
| // copy messages | ||
| const duplicatedMessages = await Promise.all( | ||
| originalMessages.map(async (message) => { | ||
| const result = (await tx | ||
| .insert(messages) | ||
| .values({ | ||
| ...message, | ||
| clientId: null, | ||
| id: idGenerator('messages'), | ||
| topicId: duplicatedTopic.id, | ||
| }) | ||
| .returning()) as DBMessageItem[]; | ||
|
|
||
| return result[0]; | ||
| }), | ||
| ); | ||
| .where(and(eq(messages.topicId, topicId), eq(messages.userId, this.userId))) | ||
| .orderBy(messages.createdAt); |
There was a problem hiding this comment.
Ensure parent messages are inserted before children
The new duplication path remaps parentId to new IDs, which means inserts must respect parent-child order to satisfy the messages.parentId FK. Ordering only by createdAt doesnβt guarantee parents come first when timestamps are equal or out-of-order (common in imported/backfilled topics), so a child can be inserted before its parent and the transaction will fail with an FK violation. Consider ordering by the parent chain (topological sort) or inserting with parentId null then updating after all rows exist.
Useful? React with πΒ / π.
Codecov Reportβ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## next #11730 +/- ##
==========================================
+ Coverage 74.14% 74.15% +0.01%
==========================================
Files 1191 1191
Lines 94844 94887 +43
Branches 12933 12948 +15
==========================================
+ Hits 70324 70366 +42
- Misses 24430 24431 +1
Partials 90 90
Flags with carried forward coverage won't be shown. Click here to find out more.
π New features to boost your workflow:
|
|
β€οΈ Great PR @ONLY-yours β€οΈ The growth of the project is inseparable from user feedback and contributions. Thank you for your contribution! If you are interested in the lobehub developer community, please join our discord and then DM @arvinxx or @canisminor1990. They will invite you to our private developer channel. We are talking about lobe-chat development and sharing AI newsletters from around the world.
Original Contentβ€οΈ Great PR @ONLY-yours β€οΈ The growth of project is inseparable from user feedback and contribution, thanks for your contribution! If you are interesting with the lobehub developer community, please join our discord and then dm @arvinxx or @canisminor1990. They will invite you to our private developer channel. We are talking about the lobe-chat development or sharing ai newsletter around the world. |
### [Version 1.153.1](v1.153.0...v1.153.1) <sup>Released on **2026-01-23**</sup> #### π Bug Fixes - **misc**: Add advace config back in agent/group profiles, fixed the group topic copy not right. #### π Styles - **misc**: Move plugin store button outside scroll container. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's fixed * **misc**: Add advace config back in agent/group profiles, closes [lobehub#11727](https://github.com/jaworldwideorg/OneJA-Bot/issues/11727) ([403175f](403175f)) * **misc**: Fixed the group topic copy not right, closes [lobehub#11730](https://github.com/jaworldwideorg/OneJA-Bot/issues/11730) ([282c1fb](282c1fb)) #### Styles * **misc**: Move plugin store button outside scroll container, closes [lobehub#11728](https://github.com/jaworldwideorg/OneJA-Bot/issues/11728) ([c484d1a](c484d1a)) </details> <div align="right"> [](#readme-top) </div>
## [Version 2.0.0-next.348](v2.0.0-next.347...v2.0.0-next.348) <sup>Released on **2026-01-23**</sup> #### π Bug Fixes - **copilot**: History popover not refreshing when agentId changes. - **misc**: Fixed the agent group builder tools excaution edge case crash, fixed the group topic copy not right. <br/> <details> <summary><kbd>Improvements and Fixes</kbd></summary> #### What's fixed * **copilot**: History popover not refreshing when agentId changes, closes [#11731](#11731) ([64f39e7](64f39e7)) * **misc**: Fixed the agent group builder tools excaution edge case crash, closes [#11735](#11735) ([5de4742](5de4742)) * **misc**: Fixed the group topic copy not right, closes [#11730](#11730) ([282c1fb](282c1fb)) </details> <div align="right"> [](#readme-top) </div>
|
π This PR is included in version 2.0.0-next.348 π The release is available on: Your semantic-release bot π¦π |
π» Change Type
fix: LOBE-3016
π Related Issue
π Description of Change
π§ͺ How to Test
πΈ Screenshots / Videos
π Additional Information
Summary by Sourcery
Ensure topic duplication correctly preserves message relationships and associated tool/plugin metadata.
Bug Fixes:
Tests: