chore(wren-ui): extend ask history to 10#1381
Conversation
WalkthroughThe changes update the handling of historical thread responses across several modules. The adaptor now transforms an array of thread responses instead of a single history, and model interfaces are updated accordingly—removing the steps array in favor of a question string and renaming a property from singular to plural. In the service layer, a method that previously returned one history is replaced by one that retrieves up to 10 responses. These modifications shift the architecture from handling a single historical context to working with multiple thread responses. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant S as AskingService
participant DB as History Store
participant A as WrenAIAdaptor
C->>S: Submit asking task (with threadId)
S->>DB: getAskingHistory(threadId)
DB-->>S: Return array of ThreadResponse objects
S->>A: Call ask(histories)
A->>A: transformHistoryInput(histories)
A-->>S: Return transformed AskHistory[]
S-->>C: Return AI response
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
wren-ui/src/apollo/server/services/askingService.ts (2)
938-943: Consider adding null/undefined check for threadId parameterThe method doesn't validate the threadId parameter. Consider adding a check to handle edge cases.
private async getAskingHistory(threadId: number): Promise<ThreadResponse[]> { + if (!threadId) { + return []; + } return await this.threadResponseRepository.getResponsesWithThread( threadId, 10, ); }
938-943: Consider adding documentation about response orderingThe implementation assumes that
getResponsesWithThreadreturns responses in the desired order, but this isn't explicit. Consider adding a comment to clarify the expected order (e.g., most recent first).private async getAskingHistory(threadId: number): Promise<ThreadResponse[]> { + // Retrieves up to 10 most recent thread responses, ordered by creation time (newest first) return await this.threadResponseRepository.getResponsesWithThread( threadId, 10, ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts(3 hunks)wren-ui/src/apollo/server/models/adaptor.ts(3 hunks)wren-ui/src/apollo/server/services/askingService.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (8)
wren-ui/src/apollo/server/models/adaptor.ts (3)
3-3: Added import for ThreadResponse is correctly used in updated interfacesThis import is necessary to support the interface changes that follow, specifically for the updated
AskInputinterface that now accepts an array of ThreadResponse objects.
52-54: Interface simplification: steps array replaced with question stringThe
AskHistoryinterface has been simplified by replacing the complexstepsarray with a singlequestionstring. This change supports the architectural shift from handling a single detailed history to working with multiple thread responses.
61-65: Renamed history to histories to support multiple thread responsesThe property has been renamed from singular to plural and its type changed from a single
AskHistoryto an array ofThreadResponseobjects, aligning with the PR objective to extend ask history to 10.wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts (3)
28-28: Added import for ThreadResponse to support updated method signatureThe ThreadResponse import is necessary for the updated transformHistoryInput method signature.
105-112: Updated input parameter name to reflect interface changeThe code correctly references the renamed
historiesproperty instead ofhistory, ensuring compatibility with the updatedAskInputinterface.
604-614: Refactored method to handle multiple thread responsesThe method has been updated to:
- Take an array of ThreadResponse objects instead of a single AskHistory
- Handle null/undefined input by returning an empty array
- Map each ThreadResponse to include just sql and question properties
This implementation correctly supports the PR objective of extending ask history to 10 entries.
wren-ui/src/apollo/server/services/askingService.ts (2)
525-541: Updated to use multiple histories instead of a single historyThe method now correctly uses the new
getAskingHistorymethod that returns multiple thread responses, and passes them to thewrenAIAdaptor.askmethod ashistoriesinstead of a singlehistory.
938-943: Implemented retrieval of multiple thread responsesThe method has been completely rewritten to return an array of up to 10 most recent ThreadResponse objects for a given thread, rather than just the most recent one. This implements the core functionality required by the PR objective.
| if (!threadId) { | ||
| return []; | ||
| } |
There was a problem hiding this comment.
I'm curious why this judgment is needed.
There was a problem hiding this comment.
It will throw a knex error if threadId is undefined. just a defensive check.
24e75ee to
c7d56a5
Compare
Description
extend ask history to 10
ref PR: #1377
Summary by CodeRabbit
New Features
Refactor