-
Notifications
You must be signed in to change notification settings - Fork 6
feat: insights chat #575
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
base: main
Are you sure you want to change the base?
feat: insights chat #575
Conversation
Signed-off-by: anilb <[email protected]>
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.
Pull Request Overview
This PR introduces a chat functionality for insights data, implementing a multi-agent system that routes user queries to appropriate data sources and tools. The system uses Amazon Bedrock for AI processing and connects to Tinybird for data retrieval.
- Implements a three-agent architecture (router, text-to-sql, pipe) for handling different types of queries
- Adds streaming chat endpoints with integration to Model Context Protocol (MCP) for Tinybird
- Includes comprehensive prompt templates for SQL generation and tool routing
Reviewed Changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
frontend/server/api/chat/stream.ts | Creates streaming chat API endpoint with request validation |
frontend/package.json | Adds AI SDK dependencies for Amazon Bedrock, MCP, and Zod |
frontend/lib/chat/prompts/text-to-sql.ts | Comprehensive prompt template for SQL query generation |
frontend/lib/chat/prompts/router.ts | Router agent prompt for query analysis and routing |
frontend/lib/chat/prompts/pipe.ts | Pipe agent prompt for tool execution |
frontend/lib/chat/data-copilot.ts | Main orchestrator handling agent workflow and streaming |
frontend/lib/chat/agents/text-to-sql.ts | Text-to-SQL agent implementation with schema validation |
frontend/lib/chat/agents/router.ts | Router agent for determining query handling strategy |
frontend/lib/chat/agents/pipe.ts | Pipe agent for executing Tinybird tools |
frontend/lib/chat/agents/base-agent.ts | Abstract base class providing common agent functionality |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (2)
frontend/package.json:52
- The version 3.25.76 of zod does not exist. The latest stable version is 3.25.12. Consider using "^3.25.0" or check for the correct version number.
"zod": "^3.25.76"
frontend/lib/chat/data-copilot.ts:43
- The model identifier "us.anthropic.claude-sonnet-4-20250514-v1:0" appears to reference Claude Sonnet 4 from May 2025, which would be after my knowledge cutoff. Please verify this model exists in Amazon Bedrock or use a known model identifier.
const model = bedrock("us.anthropic.claude-sonnet-4-20250514-v1:0");
protected getProviderOptions(_input: TextToSqlAgentInput): any { | ||
return { | ||
bedrock: { | ||
reasoningConfig: { type: "enabled", budgetTokens: 3000 }, | ||
}, | ||
}; |
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.
The reasoningConfig with budgetTokens appears to be a provider-specific feature that may not be available in all Amazon Bedrock models. Consider adding error handling or documentation about which models support this feature.
protected getProviderOptions(_input: TextToSqlAgentInput): any { | |
return { | |
bedrock: { | |
reasoningConfig: { type: "enabled", budgetTokens: 3000 }, | |
}, | |
}; | |
/** | |
* Returns provider-specific options for Bedrock models. | |
* Note: reasoningConfig with budgetTokens is only supported by certain Bedrock models (e.g., Anthropic Claude 3). | |
* If the model does not support this feature, it will be omitted. | |
*/ | |
protected getProviderOptions(input: TextToSqlAgentInput): any { | |
const model = input.model; | |
// Check if model supports reasoningConfig (assume a property or method, e.g., supportsReasoningConfig) | |
if (model && (model.supportsReasoningConfig || (typeof model.isReasoningConfigSupported === "function" && model.isReasoningConfigSupported()))) { | |
return { | |
bedrock: { | |
reasoningConfig: { type: "enabled", budgetTokens: 3000 }, | |
}, | |
}; | |
} else { | |
// Optionally log or throw an error if strict enforcement is desired | |
// console.warn("reasoningConfig is not supported by the selected Bedrock model."); | |
return { | |
bedrock: {}, | |
}; | |
} |
Copilot uses AI. Check for mistakes.
let cleanText = text.replaceAll(/```json\s*\n?/g, ""); | ||
cleanText = cleanText.replaceAll(/\n?```/g, ""); |
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.
The replaceAll method with regex was added in ES2021. For broader compatibility, consider using replace with the global flag: text.replace(/```json\s*\n?/g, '')
let cleanText = text.replaceAll(/```json\s*\n?/g, ""); | |
cleanText = cleanText.replaceAll(/\n?```/g, ""); | |
let cleanText = text.replace(/```json\s*\n?/g, ""); | |
cleanText = cleanText.replace(/\n?```/g, ""); |
Copilot uses AI. Check for mistakes.
let cleanText = text.replaceAll(/```json\s*\n?/g, ""); | ||
cleanText = cleanText.replaceAll(/\n?```/g, ""); |
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.
The replaceAll method with regex was added in ES2021. For broader compatibility, consider using replace with the global flag: cleanText.replace(/\n?```/g, '')
let cleanText = text.replaceAll(/```json\s*\n?/g, ""); | |
cleanText = cleanText.replaceAll(/\n?```/g, ""); | |
let cleanText = text.replace(/```json\s*\n?/g, ""); | |
cleanText = cleanText.replace(/\n?```/g, ""); |
Copilot uses AI. Check for mistakes.
No description provided.