-
Notifications
You must be signed in to change notification settings - Fork 158
fix: Changed the prompt to accurately answer scheduling or time based meeting questions and Readme changes #593
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
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 refines the SQL prompt for meeting scheduling queries, updates infrastructure templates, and aligns documentation with renamed parameters.
- Introduces detailed scheduling logic (most recent past / nearest future) in SQL prompts across backend and infra files
- Updates
templateHashininfra/main.jsonto reflect infra template changes - Removes Azure OpenAI region parameter and renames a Foundry project parameter in documentation
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/App/backend/plugins/chat_with_data_plugin.py | Adds scheduling/time-based meeting logic to the SQL prompt |
| infra/main.json | Bumps templateHash and synchronizes the updated SQL prompt |
| infra/main.bicep | Mirrors the scheduling prompt update in the Bicep template |
| docs/DeploymentGuide.md | Removes the Azure OpenAI Location parameter from deployment guide |
| docs/CustomizingAzdParameters.md | Renames RESOURCE_GROUP_NAME_FOUNDRY to AZURE_EXISTING_AI_PROJECT_RESOURCE_ID |
Comments suppressed due to low confidence (2)
src/App/backend/plugins/chat_with_data_plugin.py:113
- There are no tests covering the new scheduling SQL logic. Consider adding unit or integration tests to validate that queries for past and future meetings return the correct results.
When answering scheduling or time-based meeting questions, always use the StartTime column from ClientMeetings table. Use correct logic to return the most recent past meeting (last/previous) or the nearest future meeting (next/upcoming), and ensure only StartTime column is used for meeting timing comparisons.
docs/CustomizingAzdParameters.md:23
- You've renamed the parameter here but haven’t updated any Bicep or ARM templates to match. Ensure the infrastructure files reference
AZURE_EXISTING_AI_PROJECT_RESOURCE_IDrather than the oldRESOURCE_GROUP_NAME_FOUNDRY.
| `AZURE_EXISTING_AI_PROJECT_RESOURCE_ID` | string | `<Existing AI Foundry Project Resource Id>` | Reuses an existing AI Foundry Project Resource Id instead of provisioning a new one. |
| ALWAYS use ClientId = {clientid} in the query filter. | ||
| ALWAYS select Client Name (Column: Client) in the query. | ||
| Query filters are IMPORTANT. Add filters like AssetType, AssetDate, etc. if needed. | ||
| When answering scheduling or time-based meeting questions, always use the StartTime column from ClientMeetings table. Use correct logic to return the most recent past meeting (last/previous) or the nearest future meeting (next/upcoming), and ensure only StartTime column is used for meeting timing comparisons. |
Copilot
AI
Jul 9, 2025
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.
[nitpick] This prompt is duplicated in multiple files (plugin, JSON, Bicep). Consider centralizing it as a shared constant or template to reduce duplication and drift.
| "solutionPrefix": "[format('ca{0}', padLeft(take(variables('uniqueId'), 12), 12, '0'))]", | ||
| "abbrs": "[variables('$fxv#0')]", | ||
| "functionAppSqlPrompt": "Generate a valid T-SQL query to find {query} for tables and columns provided below:\n 1. Table: Clients\n Columns: ClientId, Client, Email, Occupation, MaritalStatus, Dependents\n 2. Table: InvestmentGoals\n Columns: ClientId, InvestmentGoal\n 3. Table: Assets\n Columns: ClientId, AssetDate, Investment, ROI, Revenue, AssetType\n 4. Table: ClientSummaries\n Columns: ClientId, ClientSummary\n 5. Table: InvestmentGoalsDetails\n Columns: ClientId, InvestmentGoal, TargetAmount, Contribution\n 6. Table: Retirement\n Columns: ClientId, StatusDate, RetirementGoalProgress, EducationGoalProgress\n 7. Table: ClientMeetings\n Columns: ClientId, ConversationId, Title, StartTime, EndTime, Advisor, ClientEmail\n Always use the Investment column from the Assets table as the value.\n Assets table has snapshots of values by date. Do not add numbers across different dates for total values.\n Do not use client name in filters.\n Do not include assets values unless asked for.\n ALWAYS use ClientId = {clientid} in the query filter.\n ALWAYS select Client Name (Column: Client) in the query.\n Query filters are IMPORTANT. Add filters like AssetType, AssetDate, etc. if needed.\n Only return the generated SQL query. Do not return anything else.", | ||
| "functionAppSqlPrompt": "Generate a valid T-SQL query to find {query} for tables and columns provided below:\n 1. Table: Clients\n Columns: ClientId, Client, Email, Occupation, MaritalStatus, Dependents\n 2. Table: InvestmentGoals\n Columns: ClientId, InvestmentGoal\n 3. Table: Assets\n Columns: ClientId, AssetDate, Investment, ROI, Revenue, AssetType\n 4. Table: ClientSummaries\n Columns: ClientId, ClientSummary\n 5. Table: InvestmentGoalsDetails\n Columns: ClientId, InvestmentGoal, TargetAmount, Contribution\n 6. Table: Retirement\n Columns: ClientId, StatusDate, RetirementGoalProgress, EducationGoalProgress\n 7. Table: ClientMeetings\n Columns: ClientId, ConversationId, Title, StartTime, EndTime, Advisor, ClientEmail\n Always use the Investment column from the Assets table as the value.\n Assets table has snapshots of values by date. Do not add numbers across different dates for total values.\n Do not use client name in filters.\n Do not include assets values unless asked for.\n ALWAYS use ClientId = {clientid} in the query filter.\n ALWAYS select Client Name (Column: Client) in the query.\n Query filters are IMPORTANT. Add filters like AssetType, AssetDate, etc. if needed.\n When answering scheduling or time-based meeting questions, always use the StartTime column from ClientMeetings table. Use correct logic to return the most recent past meeting (last/previous) or the nearest future meeting (next/upcoming), and ensure only StartTime column is used for meeting timing comparisons.\n Only return the generated SQL query. Do not return anything else.", |
Copilot
AI
Jul 9, 2025
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.
[nitpick] The same scheduling prompt appears here as in the plugin and Bicep. Extract it into a shared parameter or external file to keep all instances in sync.
| | `AZURE_LOCATION` | string | `japaneast` | Sets the Azure region for resource deployment. | | ||
| | `AZURE_ENV_LOG_ANALYTICS_WORKSPACE_ID` | string | `<Existing Workspace Id>` | Reuses an existing Log Analytics Workspace instead of provisioning a new one. | | ||
| | `RESOURCE_GROUP_NAME_FOUNDRY` | string | `<Existing AI Foundry Project>` | Reuses an existing AI Foundry Project instead of provisioning a new one. | | ||
| | `AZURE_EXISTING_AI_PROJECT_RESOURCE_ID` | string | `<Existing AI Foundry Project Resource Id>` | Reuses an existing AI Foundry Project Resource Id instead of provisioning a new one. | |
Copilot
AI
Jul 9, 2025
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.
[nitpick] In the default value and description, use 'ID' (all caps) rather than 'Id' for consistency with parameter naming conventions.
| | `AZURE_EXISTING_AI_PROJECT_RESOURCE_ID` | string | `<Existing AI Foundry Project Resource Id>` | Reuses an existing AI Foundry Project Resource Id instead of provisioning a new one. | | |
| | `AZURE_EXISTING_AI_PROJECT_RESOURCE_ID` | string | `<Existing AI Foundry Project Resource ID>` | Reuses an existing AI Foundry Project Resource ID instead of provisioning a new one. | |
|
🎉 This PR is included in version 1.5.2 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Purpose
This pull request introduces updates to documentation, infrastructure templates, and backend logic to enhance clarity, consistency, and functionality. Key changes include updates to parameter names and descriptions in documentation, improvements to SQL query generation logic, and synchronization of prompts across infrastructure and backend files.
Documentation Updates:
RESOURCE_GROUP_NAME_FOUNDRYtoAZURE_EXISTING_AI_PROJECT_RESOURCE_IDindocs/CustomizingAzdParameters.mdto better reflect its purpose. Updated its description for clarity.docs/DeploymentGuide.mdto simplify deployment instructions.SQL Query Logic Enhancements:
infra/main.bicepto handle scheduling and time-based meeting queries. It now ensures correct logic for identifying the most recent past or nearest future meeting using theStartTimecolumn.infra/main.jsonandsrc/App/backend/plugins/chat_with_data_plugin.pyto maintain consistent behavior across infrastructure and backend components. [1] [2]Infrastructure Template Updates:
templateHashininfra/main.jsonto reflect changes in the infrastructure template.Does this introduce a breaking change?
Golden Path Validation
Deployment Validation
What to Check
Verify that the following are valid
Other Information