feat(wren-ui): API enhancement #1781
Conversation
WalkthroughThis update introduces a comprehensive set of new API endpoints and supporting utilities for question-answering, SQL generation, summary generation, instruction management, and SQL pair management, including both synchronous and streaming (SSE) variants. It expands type definitions, error handling, and adds new utility modules for server-sent events and validation. Several existing types are made more flexible by allowing optional or nullable fields. Changes
Sequence Diagram(s)Streaming Question-Answering Flow (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
♻️ Duplicate comments (1)
wren-ui/src/apollo/server/utils/sseTypes.ts (1)
1-1: Consider relocating this file as suggested in previous review.A previous reviewer suggested moving this file under
/modelsdirectory, which would better align with the project's architectural patterns for type definitions.
🧹 Nitpick comments (11)
wren-ui/src/pages/api/v1/models.ts (1)
51-64: Consider adding requestPayload for consistency.Even though GET requests typically don't have a body, consider explicitly passing
requestPayload: undefinedfor consistent API history tracking.await respondWithSimple({ res, statusCode: 200, responsePayload: { hash: lastDeploy.hash, models, relationships, views, }, projectId: project.id, apiType: ApiType.GET_MODELS, startTime, + requestPayload: undefined, headers: req.headers as Record<string, string>, });wren-ui/src/pages/api/v1/ask.ts (1)
144-145: Use optional chaining for safer property access.Replace the logical AND operators with optional chaining for cleaner and safer code.
- if (match && match[1]) { + if (match?.[1]) {Also applies to: 268-269
wren-ui/src/pages/api/v1/knowledge/sql_pairs/index.ts (1)
71-77: Extract validation constants for better maintainability.Magic numbers should be extracted as named constants for easier maintenance and consistency across the codebase.
+const MAX_SQL_LENGTH = 10000; +const MAX_QUESTION_LENGTH = 1000; + if (sql.length > 10000) { - throw new ApiError('SQL is too long (max 10000 characters)', 400); + throw new ApiError(`SQL is too long (max ${MAX_SQL_LENGTH} characters)`, 400); } if (question.length > 1000) { - throw new ApiError('Question is too long (max 1000 characters)', 400); + throw new ApiError(`Question is too long (max ${MAX_QUESTION_LENGTH} characters)`, 400); }wren-ui/src/pages/api/v1/generate_summary.ts (1)
146-151: Consider a more robust streaming data parser.The current regex-based parsing of streamed chunks might be fragile if the message format changes. Consider using a more structured approach or at least documenting the expected format.
Would you like me to suggest a more robust streaming parser that handles edge cases like partial chunks or format variations?
wren-ui/src/apollo/server/utils/sseUtils.ts (1)
94-115: Add logging for unexpected AskResultStatus values.The default case silently returns
SQL_GENERATION_UNDERSTANDING, which might hide unexpected status values. Consider logging a warning to help with debugging.+import { getLogger } from '@server/utils'; + +const logger = getLogger('SSE_UTILS'); + export const getSqlGenerationState = (status: AskResultStatus): StateType => { switch (status) { // ... existing cases ... default: + logger.warn(`Unexpected AskResultStatus: ${status}`); return StateType.SQL_GENERATION_UNDERSTANDING; } };wren-ui/src/pages/api/v1/knowledge/instructions/index.ts (2)
49-52: Extract boolean conversion logic to a helper function.The same boolean conversion logic is repeated. Extract it to reduce duplication and improve maintainability.
+const normalizeIsGlobal = (isDefault: any): boolean => { + return typeof isDefault === 'boolean' ? isDefault : Boolean(isDefault); +}; + // In handleGetInstructions: -const isGlobalValue = - typeof instruction.isDefault === 'boolean' - ? instruction.isDefault - : Boolean(instruction.isDefault); +const isGlobalValue = normalizeIsGlobal(instruction.isDefault); // In handleCreateInstruction: -const isGlobalValue = - typeof newInstruction.isDefault === 'boolean' - ? newInstruction.isDefault - : Boolean(newInstruction.isDefault); +const isGlobalValue = normalizeIsGlobal(newInstruction.isDefault);Also applies to: 147-150
95-97: Clarify the validation logic for instruction type determination.The current validation using
isNilfor both fields might be confusing. Consider making the logic more explicit.-if (isNil(isGlobal) && isNil(questions)) { - throw new ApiError('isGlobal or questions is required', 400); -} +// If isGlobal is not specified, we need questions to determine instruction type +if (isGlobal === undefined && (!questions || questions.length === 0)) { + throw new ApiError('Either isGlobal must be specified or questions must be provided', 400); +}wren-ui/src/pages/api/v1/async/ask.ts (2)
238-243: Improve null safety with optional chaining.The code can be simplified and made safer using optional chaining as suggested by static analysis.
- const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); - if (match && match[1]) { + const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); + if (match?.[1]) { // Send incremental content updates - explanation += match[1]; - sendContentBlockDelta(res, match[1]); + explanation += match[1]; + sendContentBlockDelta(res, match[1]); }
395-400: Improve null safety with optional chaining.Consistent with the previous suggestion, use optional chaining for cleaner and safer code.
- const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); - if (match && match[1]) { + const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); + if (match?.[1]) { summary += match[1]; // Send incremental content updates - sendContentBlockDelta(res, match[1]); + sendContentBlockDelta(res, match[1]); }wren-ui/src/apollo/server/utils/sseTypes.ts (1)
12-25: Review state transition logic for consistency.The
StateTypeenum contains comprehensive SQL generation states, but there's potential inconsistency in naming patterns. Some states have specific terminal statuses (SUCCESS,FAILED,STOPPED) while others use more general terms (START,END). Consider ensuring consistent naming conventions across all state transitions.Additionally, verify that these states align with the actual SQL generation pipeline flow to prevent state management issues.
wren-ui/src/apollo/server/utils/apiUtils.ts (1)
128-149: Improve parameter typing for better type safety.The function parameters are typed as
any, which reduces type safety and IntelliSense support. Consider using proper interfaces or types for theproject,deployService, andqueryServiceparameters.export const validateSql = async ( sql: string, - project: any, - deployService: any, - queryService: any, + project: { id: number }, + deployService: DeployService, + queryService: QueryService, ) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
wren-ui/src/apollo/client/graphql/__types__.ts(4 hunks)wren-ui/src/apollo/client/graphql/dashboard.generated.ts(1 hunks)wren-ui/src/apollo/server/repositories/apiHistoryRepository.ts(2 hunks)wren-ui/src/apollo/server/resolvers/apiHistoryResolver.ts(1 hunks)wren-ui/src/apollo/server/schema.ts(1 hunks)wren-ui/src/apollo/server/services/instructionService.ts(2 hunks)wren-ui/src/apollo/server/utils/apiUtils.ts(2 hunks)wren-ui/src/apollo/server/utils/error.ts(3 hunks)wren-ui/src/apollo/server/utils/index.ts(1 hunks)wren-ui/src/apollo/server/utils/sseTypes.ts(1 hunks)wren-ui/src/apollo/server/utils/sseUtils.ts(1 hunks)wren-ui/src/pages/api-management/history.tsx(2 hunks)wren-ui/src/pages/api/v1/ask.ts(1 hunks)wren-ui/src/pages/api/v1/async/ask.ts(1 hunks)wren-ui/src/pages/api/v1/async/generate_sql.ts(1 hunks)wren-ui/src/pages/api/v1/generate_sql.ts(1 hunks)wren-ui/src/pages/api/v1/generate_summary.ts(1 hunks)wren-ui/src/pages/api/v1/knowledge/instructions/[id].ts(1 hunks)wren-ui/src/pages/api/v1/knowledge/instructions/index.ts(1 hunks)wren-ui/src/pages/api/v1/knowledge/sql_pairs/[id].ts(1 hunks)wren-ui/src/pages/api/v1/knowledge/sql_pairs/index.ts(1 hunks)wren-ui/src/pages/api/v1/models.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (19)
📓 Common learnings
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: cyyeh
PR: Canner/WrenAI#1112
File: wren-ai-service/src/pipelines/generation/sql_question.py:76-77
Timestamp: 2025-01-14T07:41:14.564Z
Learning: In the WrenAI codebase, extensive error handling for JSON parsing and array access operations in pipeline post-processing functions is not required, as indicated by the maintainer's preference to skip such suggestions.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
wren-ui/src/apollo/server/utils/error.ts (3)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1585
File: wren-ui/src/pages/api/v1/generate_sql.ts:98-106
Timestamp: 2025-04-24T16:10:43.308Z
Learning: In the generate_sql API, allow users to specify language codes not predefined in the WrenAILanguage enum, passing them through directly rather than strictly validating against the enum.
wren-ui/src/apollo/server/utils/index.ts (1)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
wren-ui/src/apollo/server/services/instructionService.ts (3)
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/services/instructionService.ts:107-146
Timestamp: 2025-03-17T09:16:53.878Z
Learning: In the WrenAI project, the instructionResolver merges the instruction ID (from args.where) and project ID (from the current project context) with instruction data before passing to the instructionService, allowing the GraphQL schema to have separate input types while the service layer works with complete objects.
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts:477-494
Timestamp: 2025-03-12T09:51:33.688Z
Learning: The instruction deletion API in the WrenAI service handles bulk deletion atomically - either all instructions are deleted or none are. There's no need for handling partial deletion scenarios.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
wren-ui/src/pages/api-management/history.tsx (2)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
wren-ui/src/apollo/server/resolvers/apiHistoryResolver.ts (1)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
wren-ui/src/pages/api/v1/generate_sql.ts (4)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1585
File: wren-ui/src/pages/api/v1/generate_sql.ts:98-106
Timestamp: 2025-04-24T16:10:43.308Z
Learning: In the generate_sql API, allow users to specify language codes not predefined in the WrenAILanguage enum, passing them through directly rather than strictly validating against the enum.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:599-604
Timestamp: 2025-03-17T19:26:10.835Z
Learning: Error handling for non-existent asking tasks in WrenAI is handled at the resolver level rather than in the service implementation.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:586-604
Timestamp: 2025-03-17T19:32:09.267Z
Learning: In WrenAI's askingService.ts, errors should be allowed to bubble up to the top-level error handler rather than being caught and logged at the method level.
wren-ui/src/apollo/server/schema.ts (4)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
Learnt from: andreashimin
PR: Canner/WrenAI#1117
File: wren-ui/src/apollo/server/backgrounds/chart.ts:86-86
Timestamp: 2025-01-15T03:50:47.868Z
Learning: In the WrenAI codebase, ChartType is maintained as a string in the web database, while the ChartType enum is specifically used for AI adaptor input/output interfaces. Therefore, string case conversions like toUpperCase() are intentional for maintaining data consistency in the web DB.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1585
File: wren-ui/src/pages/api/v1/generate_sql.ts:98-106
Timestamp: 2025-04-24T16:10:43.308Z
Learning: In the generate_sql API, allow users to specify language codes not predefined in the WrenAILanguage enum, passing them through directly rather than strictly validating against the enum.
wren-ui/src/apollo/server/repositories/apiHistoryRepository.ts (4)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: andreashimin
PR: Canner/WrenAI#1117
File: wren-ui/src/apollo/server/backgrounds/chart.ts:86-86
Timestamp: 2025-01-15T03:50:47.868Z
Learning: In the WrenAI codebase, ChartType is maintained as a string in the web database, while the ChartType enum is specifically used for AI adaptor input/output interfaces. Therefore, string case conversions like toUpperCase() are intentional for maintaining data consistency in the web DB.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
Learnt from: andreashimin
PR: Canner/WrenAI#1117
File: wren-ui/src/apollo/server/repositories/threadResponseRepository.ts:37-37
Timestamp: 2025-01-15T03:51:14.432Z
Learning: In the ThreadResponseRepository, the `chartType` property is intentionally typed as string in the database layer, while the ChartType enum is used specifically for AI adaptor input/output in the application layer.
wren-ui/src/pages/api/v1/ask.ts (2)
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:599-604
Timestamp: 2025-03-17T19:26:10.835Z
Learning: Error handling for non-existent asking tasks in WrenAI is handled at the resolver level rather than in the service implementation.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:586-604
Timestamp: 2025-03-17T19:32:09.267Z
Learning: In WrenAI's askingService.ts, errors should be allowed to bubble up to the top-level error handler rather than being caught and logged at the method level.
wren-ui/src/pages/api/v1/knowledge/instructions/index.ts (2)
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/services/instructionService.ts:107-146
Timestamp: 2025-03-17T09:16:53.878Z
Learning: In the WrenAI project, the instructionResolver merges the instruction ID (from args.where) and project ID (from the current project context) with instruction data before passing to the instructionService, allowing the GraphQL schema to have separate input types while the service layer works with complete objects.
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts:477-494
Timestamp: 2025-03-12T09:51:33.688Z
Learning: The instruction deletion API in the WrenAI service handles bulk deletion atomically - either all instructions are deleted or none are. There's no need for handling partial deletion scenarios.
wren-ui/src/pages/api/v1/async/generate_sql.ts (3)
Learnt from: wwwy3y3
PR: Canner/WrenAI#1585
File: wren-ui/src/pages/api/v1/generate_sql.ts:98-106
Timestamp: 2025-04-24T16:10:43.308Z
Learning: In the generate_sql API, allow users to specify language codes not predefined in the WrenAILanguage enum, passing them through directly rather than strictly validating against the enum.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:586-604
Timestamp: 2025-03-17T19:32:09.267Z
Learning: In WrenAI's askingService.ts, errors should be allowed to bubble up to the top-level error handler rather than being caught and logged at the method level.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:599-604
Timestamp: 2025-03-17T19:26:10.835Z
Learning: Error handling for non-existent asking tasks in WrenAI is handled at the resolver level rather than in the service implementation.
wren-ui/src/pages/api/v1/async/ask.ts (2)
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:599-604
Timestamp: 2025-03-17T19:26:10.835Z
Learning: Error handling for non-existent asking tasks in WrenAI is handled at the resolver level rather than in the service implementation.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:586-604
Timestamp: 2025-03-17T19:32:09.267Z
Learning: In WrenAI's askingService.ts, errors should be allowed to bubble up to the top-level error handler rather than being caught and logged at the method level.
wren-ui/src/pages/api/v1/knowledge/instructions/[id].ts (3)
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/adaptors/wrenAIAdaptor.ts:477-494
Timestamp: 2025-03-12T09:51:33.688Z
Learning: The instruction deletion API in the WrenAI service handles bulk deletion atomically - either all instructions are deleted or none are. There's no need for handling partial deletion scenarios.
Learnt from: onlyjackfrost
PR: Canner/WrenAI#1392
File: wren-ui/src/apollo/server/services/instructionService.ts:107-146
Timestamp: 2025-03-17T09:16:53.878Z
Learning: In the WrenAI project, the instructionResolver merges the instruction ID (from args.where) and project ID (from the current project context) with instruction data before passing to the instructionService, allowing the GraphQL schema to have separate input types while the service layer works with complete objects.
Learnt from: paopa
PR: Canner/WrenAI#1376
File: wren-ai-service/src/pipelines/indexing/instructions.py:100-107
Timestamp: 2025-03-13T03:48:24.664Z
Learning: The `InstructionsCleaner.run()` method in the instructions indexing pipeline only accepts `instruction_ids` and `project_id` parameters, and doesn't support a `delete_all` parameter for unconditional deletion.
wren-ui/src/apollo/server/utils/sseTypes.ts (1)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
wren-ui/src/apollo/client/graphql/dashboard.generated.ts (1)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
wren-ui/src/apollo/client/graphql/__types__.ts (4)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: andreashimin
PR: Canner/WrenAI#1117
File: wren-ui/src/apollo/server/backgrounds/chart.ts:86-86
Timestamp: 2025-01-15T03:50:47.868Z
Learning: In the WrenAI codebase, ChartType is maintained as a string in the web database, while the ChartType enum is specifically used for AI adaptor input/output interfaces. Therefore, string case conversions like toUpperCase() are intentional for maintaining data consistency in the web DB.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1585
File: wren-ui/src/pages/api/v1/generate_sql.ts:98-106
Timestamp: 2025-04-24T16:10:43.308Z
Learning: In the generate_sql API, allow users to specify language codes not predefined in the WrenAILanguage enum, passing them through directly rather than strictly validating against the enum.
wren-ui/src/pages/api/v1/knowledge/sql_pairs/[id].ts (2)
Learnt from: cyyeh
PR: Canner/WrenAI#1112
File: wren-ai-service/src/web/v1/routers/sql_question.py:59-66
Timestamp: 2025-01-14T07:45:11.981Z
Learning: In the WrenAI codebase, UUID validation for query IDs in API endpoints is not required, as demonstrated in the SQL question router's GET endpoint.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
wren-ui/src/apollo/server/utils/apiUtils.ts (4)
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:599-604
Timestamp: 2025-03-17T19:26:10.835Z
Learning: Error handling for non-existent asking tasks in WrenAI is handled at the resolver level rather than in the service implementation.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:586-604
Timestamp: 2025-03-17T19:32:09.267Z
Learning: In WrenAI's askingService.ts, errors should be allowed to bubble up to the top-level error handler rather than being caught and logged at the method level.
🧬 Code Graph Analysis (6)
wren-ui/src/apollo/server/services/instructionService.ts (2)
wren-ui/src/apollo/client/graphql/__types__.ts (1)
Instruction(607-616)wren-ui/src/apollo/server/repositories/instructionRepository.ts (1)
Instruction(11-19)
wren-ui/src/pages/api/v1/models.ts (1)
wren-ui/src/apollo/server/utils/apiUtils.ts (3)
ApiError(154-170)respondWithSimple(220-253)handleApiError(258-311)
wren-ui/src/pages/api/v1/async/ask.ts (4)
wren-ui/src/apollo/server/utils/sseTypes.ts (4)
ContentBlockStartEvent(64-70)ContentBlockDeltaEvent(72-78)ContentBlockStopEvent(80-82)AsyncAskRequest(33-38)wren-ui/src/apollo/server/utils/sseUtils.ts (6)
sendSSEEvent(16-19)sendMessageStart(24-30)sendStateUpdate(54-68)getSqlGenerationState(94-115)endStream(120-128)sendError(73-89)wren-ui/src/apollo/server/utils/apiUtils.ts (4)
ApiError(154-170)MAX_WAIT_TIME(17-17)isAskResultFinished(19-26)validateSummaryResult(79-93)wren-ui/src/apollo/server/models/adaptor.ts (3)
WrenAIError(11-14)TextBasedAnswerInput(178-185)TextBasedAnswerResult(193-197)
wren-ui/src/apollo/server/utils/sseUtils.ts (1)
wren-ui/src/apollo/server/utils/sseTypes.ts (5)
StreamEvent(92-99)MessageStartEvent(44-46)MessageStopEvent(48-54)StateEvent(56-62)ErrorEvent(84-90)
wren-ui/src/apollo/client/graphql/__types__.ts (1)
wren-ui/src/apollo/server/models/dashboard.ts (1)
DashboardSchedule(28-35)
wren-ui/src/apollo/server/utils/apiUtils.ts (3)
wren-ui/src/common.ts (1)
components(225-225)wren-ui/src/apollo/server/models/adaptor.ts (2)
WrenAIError(11-14)TextBasedAnswerResult(193-197)wren-ui/src/apollo/server/repositories/apiHistoryRepository.ts (1)
ApiHistory(30-42)
🪛 Biome (1.9.4)
wren-ui/src/pages/api/v1/generate_summary.ts
[error] 148-148: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
wren-ui/src/pages/api/v1/ask.ts
[error] 145-145: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 269-269: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
wren-ui/src/pages/api/v1/async/ask.ts
[error] 239-239: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 396-396: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (26)
wren-ui/src/apollo/server/utils/error.ts (1)
61-62: LGTM! Consistent error code implementation.The new
SQL_EXECUTION_ERRORenum value and corresponding error messages follow the established pattern in the codebase. All required message mappings are properly included.Also applies to: 129-130, 163-163
wren-ui/src/pages/api-management/history.tsx (2)
73-78: Excellent improvement! Proper HTTP status code handling.The change from checking only
status === 200tostatus >= 200 && status < 300correctly treats all 2xx status codes as successful, which aligns with standard HTTP semantics.
106-108: Good defensive programming with optional chaining.Adding optional chaining (
payload?.questionandpayload?.sql) prevents potential runtime errors when the payload is null or undefined.wren-ui/src/apollo/server/utils/index.ts (1)
9-10: LGTM! Proper module organization.The addition of SSE-related exports follows the established pattern and centralizes access to SSE utilities.
wren-ui/src/apollo/server/resolvers/apiHistoryResolver.ts (1)
151-159: LGTM! Logical handling of different payload types.The addition of array detection properly handles cases where response payloads are arrays, returning them directly while preserving the existing sanitization logic for object payloads. This approach accommodates the expanded API types introduced in this PR.
wren-ui/src/apollo/server/schema.ts (1)
11-23: LGTM! Schema additions align with API enhancements.The new ApiType enum values properly support the expanded API functionality for summaries, instructions, SQL pairs, models, and asynchronous operations.
wren-ui/src/pages/api/v1/generate_sql.ts (1)
3-16: Good refactoring to centralize common utilities.Moving the utility functions to a shared module promotes code reuse and maintainability across the API endpoints.
wren-ui/src/apollo/server/repositories/apiHistoryRepository.ts (1)
15-27: LGTM! Enum values match schema definitions.The new ApiType enum values properly align with the GraphQL schema additions.
wren-ui/src/apollo/client/graphql/dashboard.generated.ts (1)
59-59: LGTM! Generated type correctly reflects schema changes.The generated type properly makes the
schedulefield and its nested properties optional and nullable, consistent with the GraphQL schema updates for flexible dashboard scheduling.wren-ui/src/pages/api/v1/knowledge/sql_pairs/[id].ts (3)
29-40: LGTM! Robust ID validation with proper error handling.The validation correctly handles edge cases and provides clear error messages for invalid inputs.
57-75: LGTM! Comprehensive input validation with appropriate constraints.The validation logic correctly:
- Handles partial updates with undefined checks
- Enforces reasonable length limits (10KB for SQL, 1KB for questions)
- Uses async SQL validation for syntax checking
- Provides clear error messages
78-98: LGTM! Clean update logic with proper response handling.The implementation correctly merges partial updates and returns the updated entity with appropriate API logging metadata.
wren-ui/src/pages/api/v1/async/ask.ts (2)
167-203: LGTM! Excellent polling implementation with comprehensive state tracking.The polling logic properly:
- Handles status changes with detailed SSE updates
- Includes timeout protection with MAX_WAIT_TIME
- Tracks poll count and provides rich debugging information
- Uses appropriate delay between polls
254-259: LGTM! Proper client disconnect handling.The implementation correctly handles client disconnection by destroying the stream and rejecting the promise, preventing resource leaks.
wren-ui/src/pages/api/v1/knowledge/instructions/[id].ts (3)
16-28: LGTM! Excellent documentation of instruction types and business rules.The comments clearly explain the two types of instructions and their constraints, making the business logic easy to understand.
81-84: LGTM! Correct enforcement of business rules for global instructions.The logic properly ensures that global instructions cannot have questions, maintaining data consistency.
96-99: LGTM! Proper type conversion handling.The boolean conversion correctly handles the database field type while maintaining type safety for the API response.
wren-ui/src/apollo/client/graphql/__types__.ts (3)
79-96: LGTM! Comprehensive API type extensions support new functionality.The added API types correctly cover all the new endpoints introduced in this PR:
- Async operations (ASYNC_ASK, ASYNC_GENERATE_SQL)
- Instruction management (CREATE/UPDATE/DELETE/GET_INSTRUCTIONS)
- SQL pair management (CREATE/UPDATE/DELETE/GET_SQL_PAIRS)
- Additional operations (GENERATE_SUMMARY, GET_MODELS, RUN_SQL)
310-312: LGTM! Dashboard schedule flexibility improvements.Making the
frequency,hour, andminutefields optional provides better flexibility for dashboard scheduling configurations, consistent with the generated GraphQL types.
400-400: LGTM! Consistent optional schedule field.The optional
schedulefield inDetailedDashboardaligns with the dashboard schedule flexibility changes and generated types.wren-ui/src/apollo/server/utils/sseTypes.ts (1)
40-99: Well-structured event interfaces with proper TypeScript patterns.The event interfaces follow a consistent pattern by extending
BaseEventand using discriminated unions through thetypeproperty. TheStreamEventunion type properly captures all possible events, enabling type-safe event handling in SSE consumers.wren-ui/src/apollo/server/utils/apiUtils.ts (5)
17-17: Good practice using a named constant for timeout values.The
MAX_WAIT_TIMEconstant improves maintainability and consistency across API handlers that need timeout functionality.
19-26: Comprehensive terminal state checking.The
isAskResultFinishedfunction correctly identifies all terminal states including the error condition, providing a reliable way to determine task completion.
34-72: Robust validation with detailed error context.The
validateAskResultfunction provides comprehensive validation with appropriate error codes and additional context (likeinvalidSqlandexplanationQueryId). The error handling covers different result types and failure scenarios effectively.Note that both
MISLEADING_QUERYandGENERALresult types throw the sameNON_SQL_QUERYerror code, which appears intentional for consistent client-side handling.
95-118: Well-designed history transformation with proper filtering.The
transformHistoryInputfunction correctly filters for relevant API types (including both sync and async variants) and ensures required fields are present before transformation. The function handles edge cases gracefully with the null check.
220-253: Appropriate design for CRUD operations.The
respondWithSimplefunction correctly omits theresponseIdfrom the response payload while still logging it to the API history, making it suitable for simple CRUD operations where clients don't need to track response IDs.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
wren-ui/src/pages/api/v1/stream/ask.ts (2)
237-243: Use optional chaining for cleaner codeThe regex match extraction can be simplified using optional chaining.
- const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); - if (match && match[1]) { - // Send incremental content updates - explanation += match[1]; - sendContentBlockDelta(res, match[1]); - } + const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); + if (match?.[1]) { + // Send incremental content updates + explanation += match[1]; + sendContentBlockDelta(res, match[1]); + }
393-401: Use optional chaining and consider extracting common streaming logicSimilar to the explanation streaming, this code can use optional chaining. Additionally, the streaming logic is duplicated between explanation and summary streaming.
- const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); - if (match && match[1]) { - summary += match[1]; - // Send incremental content updates - sendContentBlockDelta(res, match[1]); - } + const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); + if (match?.[1]) { + summary += match[1]; + // Send incremental content updates + sendContentBlockDelta(res, match[1]); + }Consider extracting the common streaming logic into a reusable function to reduce code duplication:
async function streamContent( stream: any, req: NextApiRequest, res: NextApiResponse, onChunk: (content: string) => void, ): Promise<void> { return new Promise<void>((resolve, reject) => { stream.on('data', (chunk) => { const chunkString = chunk.toString('utf-8'); const match = chunkString.match(/data: {"message":"([\s\S]*?)"}/); if (match?.[1]) { onChunk(match[1]); sendContentBlockDelta(res, match[1]); } }); stream.on('end', () => resolve()); stream.on('error', (error) => reject(error)); req.on('close', () => { stream.destroy(); reject(new Error('Client disconnected')); }); }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
wren-ui/src/apollo/client/graphql/__types__.ts(4 hunks)wren-ui/src/apollo/server/repositories/apiHistoryRepository.ts(2 hunks)wren-ui/src/apollo/server/schema.ts(1 hunks)wren-ui/src/apollo/server/utils/apiUtils.ts(2 hunks)wren-ui/src/pages/api/v1/stream/ask.ts(1 hunks)wren-ui/src/pages/api/v1/stream/generate_sql.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- wren-ui/src/apollo/server/schema.ts
- wren-ui/src/apollo/server/repositories/apiHistoryRepository.ts
- wren-ui/src/apollo/server/utils/apiUtils.ts
- wren-ui/src/apollo/client/graphql/types.ts
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:10.593Z
Learning: The typo in error code enum names ("IDENTIED_AS_GENERAL" and "IDENTIED_AS_MISLEADING_QUERY" instead of "IDENTIFIED_AS_GENERAL" and "IDENTIFIED_AS_MISLEADING_QUERY") in wren-ui/src/apollo/server/utils/error.ts was recognized but intentionally not fixed as it was considered out of scope for PR #1414.
Learnt from: cyyeh
PR: Canner/WrenAI#1112
File: wren-ai-service/src/pipelines/generation/sql_question.py:76-77
Timestamp: 2025-01-14T07:41:14.564Z
Learning: In the WrenAI codebase, extensive error handling for JSON parsing and array access operations in pipeline post-processing functions is not required, as indicated by the maintainer's preference to skip such suggestions.
Learnt from: andreashimin
PR: Canner/WrenAI#1414
File: wren-ui/src/apollo/server/utils/error.ts:0-0
Timestamp: 2025-03-18T10:28:26.608Z
Learning: The enum names `IDENTIED_AS_GENERAL` and `IDENTIED_AS_MISLEADING_QUERY` in GeneralErrorCodes enum in wren-ui/src/apollo/server/utils/error.ts have typos (missing "FI" in "IDENTIFIED") but fixing them is not in scope for the current PR.
wren-ui/src/pages/api/v1/stream/generate_sql.ts (2)
Learnt from: wwwy3y3
PR: Canner/WrenAI#1585
File: wren-ui/src/pages/api/v1/generate_sql.ts:98-106
Timestamp: 2025-04-24T16:10:43.308Z
Learning: In the generate_sql API, allow users to specify language codes not predefined in the WrenAILanguage enum, passing them through directly rather than strictly validating against the enum.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:586-604
Timestamp: 2025-03-17T19:32:09.267Z
Learning: In WrenAI's askingService.ts, errors should be allowed to bubble up to the top-level error handler rather than being caught and logged at the method level.
wren-ui/src/pages/api/v1/stream/ask.ts (2)
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:599-604
Timestamp: 2025-03-17T19:26:10.835Z
Learning: Error handling for non-existent asking tasks in WrenAI is handled at the resolver level rather than in the service implementation.
Learnt from: wwwy3y3
PR: Canner/WrenAI#1388
File: wren-ui/src/apollo/server/services/askingService.ts:586-604
Timestamp: 2025-03-17T19:32:09.267Z
Learning: In WrenAI's askingService.ts, errors should be allowed to bubble up to the top-level error handler rather than being caught and logged at the method level.
🪛 Biome (1.9.4)
wren-ui/src/pages/api/v1/stream/ask.ts
[error] 239-239: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 396-396: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (go)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
wren-ui/src/pages/api/v1/stream/generate_sql.ts (1)
35-201: Clean implementation of streaming SQL generationThe handler follows a clear structure with proper validation, SSE setup, polling mechanism, and comprehensive error handling. The language configuration correctly allows flexibility per the retrieved learnings.
This pull request introduces several enhancements and fixes across multiple files, focusing on API functionality, error handling, and support for server-sent events (SSE). Key changes include adding new API types, improving error handling, and introducing SSE types for real-time streaming support.
API Enhancements:
CREATE_INSTRUCTION,UPDATE_INSTRUCTION,GENERATE_SUMMARY, and others toApiTypeinwren-ui/src/apollo/client/graphql/__types__.tsandwren-ui/src/apollo/server/repositories/apiHistoryRepository.ts. These changes expand the system's capabilities for handling various operations. [1] [2] [3]Error Handling Improvements:
SQL_EXECUTION_ERROR) and corresponding messages inwren-ui/src/apollo/server/utils/error.tsto improve error classification and debugging. [1] [2] [3]Dashboard Scheduling Updates:
DashboardScheduleandDetailedDashboardtypes to make certain fields optional, improving flexibility in handling dashboard schedules. [1] [2] [3]Server-Sent Events (SSE) Support:
EventType,StateType, and related interfaces inwren-ui/src/apollo/server/utils/sseTypes.tsto enable real-time streaming APIs with structured event handling.Utility Enhancements:
wren-ui/src/apollo/server/utils/apiUtils.ts, improving robustness and error handling for AI and SQL-related operations. [1] [2]Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores