-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(wren-ui): Reasoning enhancement & SQL pair in asking flow #1471
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
Conversation
WalkthroughThe changes update both the client and server sides as well as UI components to integrate SQL pair support throughout the application. New fields and enum members related to SQL pairs are added to GraphQL types and fragments, while server models, resolvers, and adaptors are enhanced to include SQL pair identifiers and fetching logic. Additionally, UI components are updated to change display logic and prop naming, and a lazy query is introduced in the Home page to preload thread data after creation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomePage
participant GraphQLServer
participant Router
User->>HomePage: Initiate thread creation
HomePage->>GraphQLServer: Execute createThread mutation
GraphQLServer-->>HomePage: Return threadId
HomePage->>GraphQLServer: Execute lazy query (useThreadLazyQuery)
GraphQLServer-->>HomePage: Return thread data
HomePage->>Router: Navigate to thread URL
sequenceDiagram
participant Resolver
participant SQLPairRepository
Resolver->>SQLPairRepository: findOneBy({ id: sqlpairId })
SQLPairRepository-->>Resolver: Return SQLPair data
Resolver-->>Client: Return candidate including sqlPair
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
wren-ui/src/components/pages/home/prompt/Result.tsx (1)
162-163: Updated IntentionFinished to maintain understanding feedbackThe component now returns the Understanding component instead of null, providing continuous visual feedback to users. This implements the "Understanding Feedback" requirement from the PR objectives.
While functionally correct, consider making the component name more representative of its behavior since "IntentionFinished" now always shows the "Understanding" UI state.
-const IntentionFinished = (props: Props) => { +// This component handles when the intention is finished but we still want to show the understanding UI +const IntentionFinishedWithFeedback = (props: Props) => {wren-ui/src/pages/home/index.tsx (1)
126-128: Effective thread preloading before navigation.Preloading the thread data before navigation is an excellent optimization that should help prevent blank pages during transitions. The async/await pattern ensures proper sequencing of operations.
However, consider adding error handling for the preloadThread operation:
- await preloadThread({ variables: { threadId } }); - router.push(Path.Home + `/${threadId}`); + try { + await preloadThread({ variables: { threadId } }); + router.push(Path.Home + `/${threadId}`); + } catch (preloadError) { + console.error('Thread preload failed:', preloadError); + // Still navigate to preserve functionality if preload fails + router.push(Path.Home + `/${threadId}`); + }wren-ui/src/components/pages/home/preparation/PreparationSteps.tsx (1)
143-153: New component for SQL pair timeline steps.The
SQLPairTimelineStepscomponent implements a specialized timeline for SQL pair scenarios, following the same pattern as other timeline variants in this file.Consider adding a comment above this component to explain its purpose, similar to how you might have documentation for the other timeline components.
+// Timeline steps to show when using a question-SQL pair function SQLPairTimelineSteps(props: Props) { const { className } = props; return ( <Timeline className={className}> <Timeline.Item dot={fileDone}> <SQLPairFinished /> </Timeline.Item> </Timeline> ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
wren-ui/src/apollo/client/graphql/__types__.ts(1 hunks)wren-ui/src/apollo/client/graphql/home.generated.ts(7 hunks)wren-ui/src/apollo/client/graphql/home.ts(1 hunks)wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts(1 hunks)wren-ui/src/apollo/server/models/adaptor.ts(2 hunks)wren-ui/src/apollo/server/resolvers/askingResolver.ts(1 hunks)wren-ui/src/apollo/server/schema.ts(1 hunks)wren-ui/src/components/pages/home/preparation/PreparationStatus.tsx(1 hunks)wren-ui/src/components/pages/home/preparation/PreparationSteps.tsx(5 hunks)wren-ui/src/components/pages/home/preparation/index.tsx(1 hunks)wren-ui/src/components/pages/home/preparation/step/SQLPairFinished.tsx(1 hunks)wren-ui/src/components/pages/home/prompt/Result.tsx(1 hunks)wren-ui/src/components/pages/home/promptThread/AnswerResult.tsx(1 hunks)wren-ui/src/pages/home/index.tsx(3 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
wren-ui/src/pages/home/index.tsx (1)
wren-ui/src/apollo/client/graphql/home.generated.ts (1)
useThreadLazyQuery(457-460)
wren-ui/src/apollo/client/graphql/__types__.ts (1)
wren-ui/src/apollo/server/repositories/sqlPairRepository.ts (1)
SqlPair(4-11)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (go)
🔇 Additional comments (24)
wren-ui/src/apollo/server/schema.ts (2)
574-574: Added SQL_PAIR enum value to support SQL pair candidatesThe new
SQL_PAIRenum value correctly extends theResultCandidateTypeenum to support SQL pair-based candidates, aligning with the PR objectives to show SQL pair asking functionality.
581-581: Added sqlPair field to ResultCandidate typeThis field appropriately extends the
ResultCandidatetype to include an associatedSqlPairobject, which is necessary for the system to reference and display SQL pair information in the UI.wren-ui/src/components/pages/home/preparation/step/SQLPairFinished.tsx (1)
1-16: New component to display SQL pair matching statusThe
SQLPairFinishedcomponent provides clear feedback to the user when a matching SQL pair is found, addressing the PR objective to show SQL pair asking during the preparation phase.Implementation is simple and effective, using appropriate styling classes for visual consistency.
wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts (1)
714-714: Added sqlpairId property to transform methodThe addition of the
sqlpairIdproperty in thetransformAskResultmethod correctly handles the SQL pair identifier from the candidate object, ensuring it's properly converted to a number type or set to null when not present.This change works well with the GraphQL schema updates and enables the resolver to fetch the complete SQL pair object when needed.
wren-ui/src/apollo/server/resolvers/askingResolver.ts (2)
651-653: Added SQL pair lookup for enhanced response dataThe implementation correctly retrieves the SQL pair object using the repository when a
sqlpairIdis present, paralleling the existing pattern for view objects.
658-658: Included sqlPair in the response objectThis change properly includes the retrieved SQL pair in the returned candidate object, completing the data flow from the adapter through to the GraphQL response.
wren-ui/src/components/pages/home/preparation/PreparationStatus.tsx (1)
59-63: Enhanced finished status logic to include SQL pairsThe condition now accounts for both view presence (
showView) and SQL pair presence (showSqlPair), making the UI correctly report "1 step" in either case. This improvement aligns with the PR objectives to show SQL pair asking during preparation.wren-ui/src/apollo/client/graphql/home.ts (1)
71-76: Added SQL pair support to CommonAskingTask fragmentThis addition enables GraphQL queries to fetch SQL pair data (id, question, sql, projectId) from candidates, which is essential for implementing the SQL pair asking functionality mentioned in the PR objectives.
wren-ui/src/apollo/client/graphql/__types__.ts (2)
1089-1089: Added sqlPair field to ResultCandidate typeThis type enhancement allows candidates to include SQL pair data, which supports the SQL pair asking feature. The optional nature of the field (using Maybe<>) ensures backward compatibility.
1096-1096: Added SQL_PAIR to ResultCandidateType enumThis enum addition complements the sqlPair field by providing a dedicated type identifier for SQL pair candidates, enabling proper type discrimination in the application logic.
wren-ui/src/apollo/server/models/adaptor.ts (2)
91-96: Well-documented extension of the AskCandidateType enum.The addition of
SQL_PAIRto theAskCandidateTypeenum is clearly documented with a comment explaining that when this type is used, thesqlpairIdwill be returned. This is a good practice that helps maintain code clarity.
124-124: Good addition of sqlpairId to match the enum extension.The addition of the optional
sqlpairIdproperty to theAskResulttype properly complements theSQL_PAIRenum value added above. This ensures type safety when working with SQL pair candidates.wren-ui/src/components/pages/home/promptThread/AnswerResult.tsx (1)
241-242: Improved prop naming with minimized instead of isAnswerFinished.The change from
isAnswerFinishedtominimizedmakes the prop naming more descriptive of its actual purpose - controlling whether the preparation component is minimized. This improvement makes the code more maintainable and self-documenting.wren-ui/src/pages/home/index.tsx (1)
102-104: Good implementation of thread preloading with lazy query.Setting up the lazy query with
cache-and-networkfetch policy is appropriate for preloading thread data, as it will both check the cache and fetch from the network to ensure data freshness.wren-ui/src/components/pages/home/preparation/index.tsx (3)
18-18: Good prop renaming for better semantics.Changing from
isAnswerFinishedtominimizedmakes the prop name clearer and more descriptive of its actual function in controlling the component's expanded/collapsed state.
22-23: Clean update of props destructuring.The props destructuring has been properly updated to match the renamed prop.
28-30: Improved state management with minimized prop.Using the minimized prop to control the active state of the Preparation component is more intuitive than the previous approach. This change also simplifies the component's logic by directly tying the visual state to the minimized prop.
wren-ui/src/components/pages/home/preparation/PreparationSteps.tsx (5)
10-10: New import for SQL pair support.The addition of the
SQLPairFinishedcomponent import supports the new SQL pair functionality mentioned in the PR objectives.
51-51: Prop name change fromisAnswerFinishedtominimized.This prop name change aligns with the PR objective to minimize the answer tab when it appears. The new name more clearly represents the state being a UI display property rather than a process state.
64-67: Added SQL pair detection and improved step visibility logic.The new
showSqlPairvariable correctly detects the presence of a SQL pair in the first candidate of the asking task. The visibility logic for the preparation steps has been simplified by removing unnecessary conditions.
79-79: Renamed wrapping condition to match new prop name.The renamed condition from
!isAnswerFinishedto!minimizedmaintains consistent naming throughout the component, matching the prop change on line 51.
83-83: Added conditional rendering for SQL pair timeline.This line implements the SQL pair functionality mentioned in the PR objectives, displaying a specialized timeline when a SQL pair is detected.
wren-ui/src/apollo/client/graphql/home.generated.ts (2)
14-14: Added sqlPair field to GraphQL fragments and queries.The SQL pair field has been consistently added to all relevant GraphQL fragments and queries. This ensures that SQL pair data is properly fetched and made available throughout the application, supporting the new SQL pair functionality mentioned in the PR objectives.
Also applies to: 16-16, 30-30, 42-42, 49-49, 85-85, 101-101, 174-174, 181-181, 188-188, 196-196
258-263: Added sqlPair object type with required fields.The sqlPair object includes all necessary fields for proper functioning:
id: For unique identificationquestion: The original question associated with the SQLsql: The SQL query itselfprojectId: To associate with the correct projectThese fields align with the expected structure in a well-designed GraphQL schema.
Description
Summary by CodeRabbit