From 4aa09e8e9a6c1b72291b86298e41424f1064a750 Mon Sep 17 00:00:00 2001 From: Shreyas-Microsoft Date: Mon, 21 Jul 2025 16:27:16 +0530 Subject: [PATCH 01/14] remove worker from dockerfile --- src/App/WebApp.Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/App/WebApp.Dockerfile b/src/App/WebApp.Dockerfile index 48bcd5ff5..786767604 100644 --- a/src/App/WebApp.Dockerfile +++ b/src/App/WebApp.Dockerfile @@ -36,4 +36,4 @@ COPY --from=frontend /home/node/app/static /usr/src/app/static/ WORKDIR /usr/src/app EXPOSE 80 -CMD ["uvicorn", "app:app", "--host", "0.0.0.0", "--port", "80", "--workers", "4", "--log-level", "info", "--access-log"] +CMD ["uvicorn", "app:app", "--host", "0.0.0.0", "--port", "80", "--log-level", "info", "--access-log"] From f2da2557bad622afad3c7e4ae13824011ad45245 Mon Sep 17 00:00:00 2001 From: Shreyas-Microsoft Date: Tue, 22 Jul 2025 16:14:26 +0530 Subject: [PATCH 02/14] agents from chat service --- src/App/app.py | 30 +++++++++--------- src/App/backend/services/chat_service.py | 40 ++++++++++++++++++------ 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/App/app.py b/src/App/app.py index 901494b2b..f4dc6f319 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -76,21 +76,21 @@ def create_app(): app.config["TEMPLATES_AUTO_RELOAD"] = True # Setup agent initialization and cleanup - @app.before_serving - async def startup(): - app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() - logging.info("Wealth Advisor Agent initialized during application startup") - app.search_agent = await AgentFactory.get_search_agent() - logging.info( - "Call Transcript Search Agent initialized during application startup" - ) - - @app.after_serving - async def shutdown(): - await AgentFactory.delete_all_agent_instance() - app.wealth_advisor_agent = None - app.search_agent = None - logging.info("Agents cleaned up during application shutdown") + # @app.before_serving + # async def startup(): + # app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() + # logging.info("Wealth Advisor Agent initialized during application startup") + # app.search_agent = await AgentFactory.get_search_agent() + # logging.info( + # "Call Transcript Search Agent initialized during application startup" + # ) + + # @app.after_serving + # async def shutdown(): + # await AgentFactory.delete_all_agent_instance() + # app.wealth_advisor_agent = None + # app.search_agent = None + # logging.info("Agents cleaned up during application shutdown") # app.secret_key = secrets.token_hex(16) # app.session_interface = SecureCookieSessionInterface() diff --git a/src/App/backend/services/chat_service.py b/src/App/backend/services/chat_service.py index 56a1daf08..05ce3e464 100644 --- a/src/App/backend/services/chat_service.py +++ b/src/App/backend/services/chat_service.py @@ -1,22 +1,37 @@ +import logging from quart import current_app from semantic_kernel.agents import AzureAIAgent, AzureAIAgentThread from semantic_kernel.contents.chat_message_content import ChatMessageContent from semantic_kernel.contents.utils.author_role import AuthorRole +from backend.agents.agent_factory import AgentFactory from backend.common.config import config from backend.services.sqldb_service import get_client_name_from_db +async def ensure_agents(app): + """Ensure both agents are initialized on demand.""" + if not hasattr(app, "wealth_advisor_agent") or app.wealth_advisor_agent is None: + app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() + logging.info("Wealth Advisor Agent initialized on demand") + if not hasattr(app, "search_agent") or app.search_agent is None: + app.search_agent = await AgentFactory.get_search_agent() + logging.info("Search Agent initialized on demand") + +async def delete_agents(app): + """Delete both agents after use.""" + await AgentFactory.delete_all_agent_instance() + app.wealth_advisor_agent = None + app.search_agent = None async def stream_response_from_wealth_assistant(query: str, client_id: str): """ Streams real-time chat response from the Wealth Assistant. Uses Semantic Kernel agent with SQL and Azure Cognitive Search based on the client ID. """ + thread = None try: # Dynamically get the name from the database - selected_client_name = get_client_name_from_db( - client_id - ) # Optionally fetch from DB + selected_client_name = get_client_name_from_db(client_id) # Prepare fallback instructions with the single-line prompt additional_instructions = config.STREAM_TEXT_SYSTEM_PROMPT @@ -37,28 +52,35 @@ async def stream_response_from_wealth_assistant(query: str, client_id: str): "{client_id}", client_id ) + # Lazy agent initialization + await ensure_agents(current_app) agent: AzureAIAgent = current_app.wealth_advisor_agent - thread: AzureAIAgentThread = None message = ChatMessageContent(role=AuthorRole.USER, content=query) sk_response = agent.invoke_stream( messages=[message], - thread=thread, + thread=None, additional_instructions=additional_instructions, ) async def generate(): + nonlocal thread try: # yields deltaText strings one-by-one async for chunk in sk_response: if not chunk or not chunk.content: continue + thread = chunk.thread if hasattr(chunk, "thread") else None yield chunk.content # just the deltaText finally: - thread = chunk.thread if chunk else None - await thread.delete() if thread else None + if thread: + await thread.delete() + # Delete agents after operation + await delete_agents(current_app) return generate except Exception as e: - await thread.delete() if thread else None - raise e + if thread: + await thread.delete() + await delete_agents(current_app) + raise e \ No newline at end of file From 4603e3cb8411334ec7682b7e1dfeb92655f3a158 Mon Sep 17 00:00:00 2001 From: VishalS-Microsoft Date: Wed, 23 Jul 2025 17:58:49 +0530 Subject: [PATCH 03/14] fixed opent telemetry issue CustomDomainInUse, FlagMustBeSetForRestore (#618) --- infra/main.bicep | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/infra/main.bicep b/infra/main.bicep index 7c225c1f8..705259f56 100644 --- a/infra/main.bicep +++ b/infra/main.bicep @@ -74,8 +74,7 @@ param aiDeploymentsLocation string param AZURE_LOCATION string = '' var solutionLocation = empty(AZURE_LOCATION) ? resourceGroup().location : AZURE_LOCATION -var uniqueId = toLower(uniqueString(environmentName, subscription().id, solutionLocation)) - +var uniqueId = toLower(uniqueString(environmentName, subscription().id, solutionLocation, resourceGroup().name)) var solutionPrefix = 'ca${padLeft(take(uniqueId, 12), 12, '0')}' // Load the abbrevations file required to name the azure resources. From 3c511021c94346c2538c8b1e4852a030ab82b0d7 Mon Sep 17 00:00:00 2001 From: Shreyas-Microsoft Date: Thu, 24 Jul 2025 12:01:28 +0530 Subject: [PATCH 04/14] cleanup duplicate agents --- src/App/WebApp.Dockerfile | 2 +- src/App/app.py | 32 +++++++++---------- src/App/backend/services/chat_service.py | 40 ++++++------------------ 3 files changed, 26 insertions(+), 48 deletions(-) diff --git a/src/App/WebApp.Dockerfile b/src/App/WebApp.Dockerfile index 786767604..82a362152 100644 --- a/src/App/WebApp.Dockerfile +++ b/src/App/WebApp.Dockerfile @@ -36,4 +36,4 @@ COPY --from=frontend /home/node/app/static /usr/src/app/static/ WORKDIR /usr/src/app EXPOSE 80 -CMD ["uvicorn", "app:app", "--host", "0.0.0.0", "--port", "80", "--log-level", "info", "--access-log"] +CMD ["uvicorn", "app:app", "--host", "0.0.0.0", "--port", "80", "--workers", "1", "--log-level", "info", "--access-log"] diff --git a/src/App/app.py b/src/App/app.py index f4dc6f319..ca382ba83 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -76,21 +76,21 @@ def create_app(): app.config["TEMPLATES_AUTO_RELOAD"] = True # Setup agent initialization and cleanup - # @app.before_serving - # async def startup(): - # app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() - # logging.info("Wealth Advisor Agent initialized during application startup") - # app.search_agent = await AgentFactory.get_search_agent() - # logging.info( - # "Call Transcript Search Agent initialized during application startup" - # ) - - # @app.after_serving - # async def shutdown(): - # await AgentFactory.delete_all_agent_instance() - # app.wealth_advisor_agent = None - # app.search_agent = None - # logging.info("Agents cleaned up during application shutdown") + @app.before_serving + async def startup(): + app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() + logging.info("Wealth Advisor Agent initialized during application startup") + app.search_agent = await AgentFactory.get_search_agent() + logging.info( + "Call Transcript Search Agent initialized during application startup" + ) + + @app.after_serving + async def shutdown(): + await AgentFactory.delete_all_agent_instance() + app.wealth_advisor_agent = None + app.search_agent = None + logging.info("Agents cleaned up during application shutdown") # app.secret_key = secrets.token_hex(16) # app.session_interface = SecureCookieSessionInterface() @@ -1359,4 +1359,4 @@ def get_users(): return str(e), 500 -app = create_app() +app = create_app() \ No newline at end of file diff --git a/src/App/backend/services/chat_service.py b/src/App/backend/services/chat_service.py index 05ce3e464..56a1daf08 100644 --- a/src/App/backend/services/chat_service.py +++ b/src/App/backend/services/chat_service.py @@ -1,37 +1,22 @@ -import logging from quart import current_app from semantic_kernel.agents import AzureAIAgent, AzureAIAgentThread from semantic_kernel.contents.chat_message_content import ChatMessageContent from semantic_kernel.contents.utils.author_role import AuthorRole -from backend.agents.agent_factory import AgentFactory from backend.common.config import config from backend.services.sqldb_service import get_client_name_from_db -async def ensure_agents(app): - """Ensure both agents are initialized on demand.""" - if not hasattr(app, "wealth_advisor_agent") or app.wealth_advisor_agent is None: - app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() - logging.info("Wealth Advisor Agent initialized on demand") - if not hasattr(app, "search_agent") or app.search_agent is None: - app.search_agent = await AgentFactory.get_search_agent() - logging.info("Search Agent initialized on demand") - -async def delete_agents(app): - """Delete both agents after use.""" - await AgentFactory.delete_all_agent_instance() - app.wealth_advisor_agent = None - app.search_agent = None async def stream_response_from_wealth_assistant(query: str, client_id: str): """ Streams real-time chat response from the Wealth Assistant. Uses Semantic Kernel agent with SQL and Azure Cognitive Search based on the client ID. """ - thread = None try: # Dynamically get the name from the database - selected_client_name = get_client_name_from_db(client_id) + selected_client_name = get_client_name_from_db( + client_id + ) # Optionally fetch from DB # Prepare fallback instructions with the single-line prompt additional_instructions = config.STREAM_TEXT_SYSTEM_PROMPT @@ -52,35 +37,28 @@ async def stream_response_from_wealth_assistant(query: str, client_id: str): "{client_id}", client_id ) - # Lazy agent initialization - await ensure_agents(current_app) agent: AzureAIAgent = current_app.wealth_advisor_agent + thread: AzureAIAgentThread = None message = ChatMessageContent(role=AuthorRole.USER, content=query) sk_response = agent.invoke_stream( messages=[message], - thread=None, + thread=thread, additional_instructions=additional_instructions, ) async def generate(): - nonlocal thread try: # yields deltaText strings one-by-one async for chunk in sk_response: if not chunk or not chunk.content: continue - thread = chunk.thread if hasattr(chunk, "thread") else None yield chunk.content # just the deltaText finally: - if thread: - await thread.delete() - # Delete agents after operation - await delete_agents(current_app) + thread = chunk.thread if chunk else None + await thread.delete() if thread else None return generate except Exception as e: - if thread: - await thread.delete() - await delete_agents(current_app) - raise e \ No newline at end of file + await thread.delete() if thread else None + raise e From 4145ad0d70973bf1ac46951845a28088a502c011 Mon Sep 17 00:00:00 2001 From: Shreyas-Microsoft Date: Thu, 24 Jul 2025 17:32:02 +0530 Subject: [PATCH 05/14] pylint fix --- src/App/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/App/app.py b/src/App/app.py index ca382ba83..901494b2b 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -1359,4 +1359,4 @@ def get_users(): return str(e), 500 -app = create_app() \ No newline at end of file +app = create_app() From 61b442120bd2a862189609895a40a412a92c78e2 Mon Sep 17 00:00:00 2001 From: Harsh-Microsoft Date: Mon, 28 Jul 2025 15:58:41 +0530 Subject: [PATCH 06/14] fix: Implement retry logic for database connection and user fetching with exponential backoff (#623) --- src/App/backend/services/sqldb_service.py | 66 ++++++++++++++--------- src/App/frontend/src/api/api.ts | 35 ++++++++---- 2 files changed, 65 insertions(+), 36 deletions(-) diff --git a/src/App/backend/services/sqldb_service.py b/src/App/backend/services/sqldb_service.py index be1c7b358..4368e4721 100644 --- a/src/App/backend/services/sqldb_service.py +++ b/src/App/backend/services/sqldb_service.py @@ -8,6 +8,8 @@ from backend.common.config import config +import time + load_dotenv() driver = config.ODBC_DRIVER @@ -33,33 +35,47 @@ def dict_cursor(cursor): def get_connection(): - try: - credential = DefaultAzureCredential(managed_identity_client_id=mid_id) + max_retries = 5 + retry_delay = 2 - token_bytes = credential.get_token( - "https://database.windows.net/.default" - ).token.encode("utf-16-LE") - token_struct = struct.pack( - f" str: diff --git a/src/App/frontend/src/api/api.ts b/src/App/frontend/src/api/api.ts index 1cd6442c3..6245ba819 100644 --- a/src/App/frontend/src/api/api.ts +++ b/src/App/frontend/src/api/api.ts @@ -42,20 +42,33 @@ export const getpbi = async (): Promise => { return ''; } +const sleep = (ms: number) => new Promise(resolve => setTimeout(resolve, ms)); + export const getUsers = async (): Promise => { - try { - const response = await fetch('/api/users'); - if (!response.ok) { - throw new Error(`Failed to fetch users: ${response.statusText}`); + const maxRetries = 1; + for (let attempt = 0; attempt <= maxRetries; attempt++) { + try { + const response = await fetch('/api/users', { + signal: AbortSignal.timeout(60000) + }); + if (!response.ok) { + throw new Error(`Failed to fetch users: ${response.statusText}`); + } + const data: User[] = await response.json(); + console.log('Fetched users:', data); + return data; + } catch (error) { + if (attempt < maxRetries && + error instanceof Error) { + console.warn(`Retrying fetch users... (retry ${attempt + 1}/${maxRetries})`); + await sleep(5000); // Simple 5 second delay + } else { + console.error('Error fetching users:', error); + return []; + } } - const data: User[] = await response.json(); - console.log('Fetched users:', data); - return data; - } catch (error) { - console.error('Error fetching users:', error); - return []; - // throw error; } + return []; }; // export const fetchChatHistoryInit = async (): Promise => { From 4f6e90714270e0c2cd4942f0f9a40e2a3716066f Mon Sep 17 00:00:00 2001 From: Shreyas-Microsoft Date: Tue, 29 Jul 2025 18:30:39 +0530 Subject: [PATCH 07/14] feat: working agent management (#626) * working agent management * fix pylint * fix test cases * fix pylint --------- Co-authored-by: Shreyas-Microsoft --- src/App/app.py | 23 ++++-- src/App/backend/agents/agent_factory.py | 73 ++++++++++++++++--- .../backend/plugins/chat_with_data_plugin.py | 35 ++++++++- .../plugins/test_chat_with_data_plugin.py | 44 +++++++++-- 4 files changed, 146 insertions(+), 29 deletions(-) diff --git a/src/App/app.py b/src/App/app.py index 901494b2b..036e59d7c 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -81,16 +81,25 @@ async def startup(): app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() logging.info("Wealth Advisor Agent initialized during application startup") app.search_agent = await AgentFactory.get_search_agent() - logging.info( - "Call Transcript Search Agent initialized during application startup" - ) + logging.info("Call Transcript Search Agent initialized during application startup") + app.sql_agent = await AgentFactory.get_sql_agent() + logging.info("SQL Agent initialized during application startup") @app.after_serving async def shutdown(): - await AgentFactory.delete_all_agent_instance() - app.wealth_advisor_agent = None - app.search_agent = None - logging.info("Agents cleaned up during application shutdown") + try: + logging.info("Application shutdown initiated...") + await AgentFactory.delete_all_agent_instance() + if hasattr(app, 'wealth_advisor_agent'): + app.wealth_advisor_agent = None + if hasattr(app, 'search_agent'): + app.search_agent = None + if hasattr(app, 'sql_agent'): + app.sql_agent = None + logging.info("Agents cleaned up successfully") + except Exception as e: + logging.error(f"Error during shutdown: {e}") + logging.exception("Detailed error during shutdown") # app.secret_key = secrets.token_hex(16) # app.session_interface = SecureCookieSessionInterface() diff --git a/src/App/backend/agents/agent_factory.py b/src/App/backend/agents/agent_factory.py index df81a2caf..634af8e13 100644 --- a/src/App/backend/agents/agent_factory.py +++ b/src/App/backend/agents/agent_factory.py @@ -7,6 +7,7 @@ """ import asyncio +import logging from typing import Optional from azure.ai.projects import AIProjectClient @@ -26,6 +27,7 @@ class AgentFactory: _lock = asyncio.Lock() _wealth_advisor_agent: Optional[AzureAIAgent] = None _search_agent: Optional[dict] = None + _sql_agent: Optional[dict] = None @classmethod async def get_wealth_advisor_agent(cls): @@ -94,18 +96,67 @@ async def delete_all_agent_instance(cls): Delete the singleton AzureAIAgent instances if it exists. """ async with cls._lock: - if cls._wealth_advisor_agent is not None: - await cls._wealth_advisor_agent.client.agents.delete_agent( - cls._wealth_advisor_agent.id - ) - cls._wealth_advisor_agent = None + logging.info("Starting agent deletion process...") + # Delete Wealth Advisor Agent + if cls._wealth_advisor_agent is not None: + try: + agent_id = cls._wealth_advisor_agent.id + logging.info(f"Deleting wealth advisor agent: {agent_id}") + if hasattr(cls._wealth_advisor_agent, 'client') and cls._wealth_advisor_agent.client: + await cls._wealth_advisor_agent.client.agents.delete_agent(agent_id) + logging.info("Wealth advisor agent deleted successfully") + else: + logging.warning("Wealth advisor agent client is None") + except Exception as e: + logging.error(f"Error deleting wealth advisor agent: {e}") + logging.exception("Detailed wealth advisor agent deletion error") + finally: + cls._wealth_advisor_agent = None + + # Delete Search Agent if cls._search_agent is not None: - cls._search_agent["client"].agents.delete_agent( - cls._search_agent["agent"].id - ) - cls._search_agent["client"].close() - cls._search_agent = None + try: + agent_id = cls._search_agent['agent'].id + logging.info(f"Deleting search agent: {agent_id}") + if cls._search_agent.get("client") and hasattr(cls._search_agent["client"], "agents"): + # AIProjectClient.agents.delete_agent is synchronous, don't await it + cls._search_agent["client"].agents.delete_agent(agent_id) + logging.info("Search agent deleted successfully") + + # Close the client if it has a close method + if hasattr(cls._search_agent["client"], "close"): + cls._search_agent["client"].close() + else: + logging.warning("Search agent client is None or invalid") + except Exception as e: + logging.error(f"Error deleting search agent: {e}") + logging.exception("Detailed search agent deletion error") + finally: + cls._search_agent = None + + # Delete SQL Agent + if cls._sql_agent is not None: + try: + agent_id = cls._sql_agent['agent'].id + logging.info(f"Deleting SQL agent: {agent_id}") + if cls._sql_agent.get("client") and hasattr(cls._sql_agent["client"], "agents"): + # AIProjectClient.agents.delete_agent is synchronous, don't await it + cls._sql_agent["client"].agents.delete_agent(agent_id) + logging.info("SQL agent deleted successfully") + + # Close the client if it has a close method + if hasattr(cls._sql_agent["client"], "close"): + cls._sql_agent["client"].close() + else: + logging.warning("SQL agent client is None or invalid") + except Exception as e: + logging.error(f"Error deleting SQL agent: {e}") + logging.exception("Detailed SQL agent deletion error") + finally: + cls._sql_agent = None + + logging.info("Agent deletion process completed") @classmethod async def get_sql_agent(cls) -> dict: @@ -114,7 +165,7 @@ async def get_sql_agent(cls) -> dict: This agent is used to generate T-SQL queries from natural language input. """ async with cls._lock: - if not hasattr(cls, "_sql_agent") or cls._sql_agent is None: + if cls._sql_agent is None: agent_instructions = config.SQL_SYSTEM_PROMPT or """ You are an expert assistant in generating T-SQL queries based on user questions. diff --git a/src/App/backend/plugins/chat_with_data_plugin.py b/src/App/backend/plugins/chat_with_data_plugin.py index f421af7ef..b8317a919 100644 --- a/src/App/backend/plugins/chat_with_data_plugin.py +++ b/src/App/backend/plugins/chat_with_data_plugin.py @@ -11,6 +11,7 @@ from azure.ai.projects import AIProjectClient from azure.identity import DefaultAzureCredential, get_bearer_token_provider from semantic_kernel.functions.kernel_function_decorator import kernel_function +from quart import current_app from backend.common.config import config from backend.services.sqldb_service import get_connection @@ -41,9 +42,14 @@ async def get_SQL_Response( if not input or not input.strip(): return "Error: Query input is required" + thread = None try: + # TEMPORARY: Use AgentFactory directly to debug the issue + logging.info(f"Using AgentFactory directly for SQL agent for ClientId: {ClientId}") from backend.agents.agent_factory import AgentFactory agent_info = await AgentFactory.get_sql_agent() + + logging.info(f"SQL agent retrieved: {agent_info is not None}") agent = agent_info["agent"] project_client = agent_info["client"] @@ -72,23 +78,27 @@ async def get_SQL_Response( role=MessageRole.AGENT ) sql_query = message.text.value.strip() if message else None + logging.info(f"Generated SQL query: {sql_query}") if not sql_query: return "No SQL query was generated." # Clean up triple backticks (if any) sql_query = sql_query.replace("```sql", "").replace("```", "") + logging.info(f"Cleaned SQL query: {sql_query}") # Execute the query conn = get_connection() cursor = conn.cursor() cursor.execute(sql_query) rows = cursor.fetchall() + logging.info(f"Query returned {len(rows)} rows") if not rows: result = "No data found for that client." else: result = "\n".join(str(row) for row in rows) + logging.info(f"Result preview: {result[:200]}...") conn.close() @@ -96,6 +106,14 @@ async def get_SQL_Response( except Exception as e: logging.exception("Error in get_SQL_Response") return f"Error retrieving SQL data: {str(e)}" + finally: + if thread: + try: + logging.info(f"Attempting to delete thread {thread.id}") + await project_client.agents.threads.delete(thread.id) + logging.info(f"Thread {thread.id} deleted successfully") + except Exception as e: + logging.error(f"Error deleting thread {thread.id}: {str(e)}") @kernel_function( name="ChatWithCallTranscripts", @@ -114,12 +132,17 @@ async def get_answers_from_calltranscripts( if not question or not question.strip(): return "Error: Question input is required" + thread = None try: response_text = "" - from backend.agents.agent_factory import AgentFactory - - agent_info: dict = await AgentFactory.get_search_agent() + # Use the singleton search agent from app context + if not hasattr(current_app, 'search_agent') or current_app.search_agent is None: + logging.error("Search agent not found in app context, falling back to AgentFactory") + from backend.agents.agent_factory import AgentFactory + agent_info = await AgentFactory.get_search_agent() + else: + agent_info = current_app.search_agent agent: Agent = agent_info["agent"] project_client: AIProjectClient = agent_info["client"] @@ -190,7 +213,11 @@ async def get_answers_from_calltranscripts( finally: if thread: - project_client.agents.threads.delete(thread.id) + try: + await project_client.agents.threads.delete(thread.id) + logging.info(f"Thread {thread.id} deleted successfully") + except Exception as e: + logging.error(f"Error deleting thread {thread.id}: {str(e)}") if not response_text.strip(): return "No data found for that client." diff --git a/src/App/tests/backend/plugins/test_chat_with_data_plugin.py b/src/App/tests/backend/plugins/test_chat_with_data_plugin.py index 684c947ae..0494b22d6 100644 --- a/src/App/tests/backend/plugins/test_chat_with_data_plugin.py +++ b/src/App/tests/backend/plugins/test_chat_with_data_plugin.py @@ -180,12 +180,14 @@ async def test_get_sql_response_openai_error(self, mock_get_sql_agent, mock_conf assert "OpenAI API error" in result @pytest.mark.asyncio + @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") + @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_success( - self, mock_get_search_agent + self, mock_config, mock_get_search_agent, mock_hasattr ): """Test successful retrieval of answers from call transcripts using AI Search Agent.""" - # Setup mocks for agent factory + # Setup mocks for agent factory (fallback case when current_app.search_agent is None) mock_agent = MagicMock() mock_agent.id = "test-agent-id" @@ -195,6 +197,10 @@ async def test_get_answers_from_calltranscripts_success( "client": mock_project_client, } + # Mock config values + mock_config.AZURE_SEARCH_INDEX = "test-index" + mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" + # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -229,7 +235,7 @@ async def test_get_answers_from_calltranscripts_success( assert "Based on call transcripts" in result assert "investment options" in result - # Verify agent factory was called + # Verify agent factory was called (fallback case) mock_get_search_agent.assert_called_once() # Verify project index was created/updated @@ -249,9 +255,11 @@ async def test_get_answers_from_calltranscripts_success( mock_project_client.agents.runs.create_and_process.assert_called_once() @pytest.mark.asyncio + @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") + @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_no_results( - self, mock_get_search_agent + self, mock_config, mock_get_search_agent, mock_hasattr ): """Test call transcripts search with no results.""" # Setup mocks for agent factory @@ -264,6 +272,10 @@ async def test_get_answers_from_calltranscripts_no_results( "client": mock_project_client, } + # Mock config values + mock_config.AZURE_SEARCH_INDEX = "test-index" + mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" + # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -295,9 +307,11 @@ async def test_get_answers_from_calltranscripts_no_results( assert "No data found for that client." in result @pytest.mark.asyncio + @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") + @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_openai_error( - self, mock_get_search_agent + self, mock_config, mock_get_search_agent, mock_hasattr ): """Test call transcripts with AI Search processing error.""" # Setup mocks for agent factory @@ -310,6 +324,10 @@ async def test_get_answers_from_calltranscripts_openai_error( "client": mock_project_client, } + # Mock config values + mock_config.AZURE_SEARCH_INDEX = "test-index" + mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" + # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -336,9 +354,11 @@ async def test_get_answers_from_calltranscripts_openai_error( assert "Error retrieving data from call transcripts" in result @pytest.mark.asyncio + @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") + @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_failed_run( - self, mock_get_search_agent + self, mock_config, mock_get_search_agent, mock_hasattr ): """Test call transcripts with failed AI Search run.""" # Setup mocks for agent factory @@ -351,6 +371,10 @@ async def test_get_answers_from_calltranscripts_failed_run( "client": mock_project_client, } + # Mock config values + mock_config.AZURE_SEARCH_INDEX = "test-index" + mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" + # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -378,9 +402,11 @@ async def test_get_answers_from_calltranscripts_failed_run( assert "Error retrieving data from call transcripts" in result @pytest.mark.asyncio + @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") + @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_empty_response( - self, mock_get_search_agent + self, mock_config, mock_get_search_agent, mock_hasattr ): """Test call transcripts with empty response text.""" # Setup mocks for agent factory @@ -393,6 +419,10 @@ async def test_get_answers_from_calltranscripts_empty_response( "client": mock_project_client, } + # Mock config values + mock_config.AZURE_SEARCH_INDEX = "test-index" + mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" + # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" From c1d823062bb437d8a5a0ef8521c5e9738d495c3a Mon Sep 17 00:00:00 2001 From: Rafi-Microsoft Date: Wed, 30 Jul 2025 20:01:23 +0530 Subject: [PATCH 08/14] sfi changes v1 --- infra/deploy_app_service.bicep | 4 + infra/main.json | 16 ++-- .../fabric_scripts/create_fabric_items.py | 2 - .../index_scripts/create_search_index.py | 4 +- .../index_scripts/create_sql_tables.py | 12 +-- .../index_scripts/create_update_sql_dates.py | 8 +- src/App/.env.sample | 3 +- src/App/app.py | 8 +- src/App/backend/agents/agent_factory.py | 10 +-- .../backend/helpers/azure_credential_utils.py | 41 ++++++++++ .../backend/plugins/chat_with_data_plugin.py | 7 +- src/App/backend/services/sqldb_service.py | 4 +- .../backend/agents/test_agent_factory.py | 10 +-- .../helpers/test_azure_credential_utils.py | 78 +++++++++++++++++++ .../plugins/test_chat_with_data_plugin.py | 4 +- .../backend/services/test_sqldb_service.py | 6 +- 16 files changed, 167 insertions(+), 50 deletions(-) create mode 100644 src/App/backend/helpers/azure_credential_utils.py create mode 100644 src/App/tests/backend/helpers/test_azure_credential_utils.py diff --git a/infra/deploy_app_service.bicep b/infra/deploy_app_service.bicep index 648fbf16f..fc0967ef6 100644 --- a/infra/deploy_app_service.bicep +++ b/infra/deploy_app_service.bicep @@ -184,6 +184,10 @@ resource Website 'Microsoft.Web/sites@2020-06-01' = { serverFarmId: HostingPlanName siteConfig: { appSettings: [ + { + name: 'APP_ENV' + value: 'Prod' + } { name: 'APPINSIGHTS_INSTRUMENTATIONKEY' value: reference(applicationInsightsId, '2015-05-01').InstrumentationKey diff --git a/infra/main.json b/infra/main.json index 647023052..02d2d31ea 100644 --- a/infra/main.json +++ b/infra/main.json @@ -5,7 +5,7 @@ "_generator": { "name": "bicep", "version": "0.36.177.2456", - "templateHash": "2238194529646818649" + "templateHash": "3253509031453285119" } }, "parameters": { @@ -359,7 +359,7 @@ } }, "solutionLocation": "[if(empty(parameters('AZURE_LOCATION')), resourceGroup().location, parameters('AZURE_LOCATION'))]", - "uniqueId": "[toLower(uniqueString(parameters('environmentName'), subscription().id, variables('solutionLocation')))]", + "uniqueId": "[toLower(uniqueString(parameters('environmentName'), subscription().id, variables('solutionLocation'), resourceGroup().name))]", "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:\r\n 1. Table: Clients\r\n Columns: ClientId, Client, Email, Occupation, MaritalStatus, Dependents\r\n 2. Table: InvestmentGoals\r\n Columns: ClientId, InvestmentGoal\r\n 3. Table: Assets\r\n Columns: ClientId, AssetDate, Investment, ROI, Revenue, AssetType\r\n 4. Table: ClientSummaries\r\n Columns: ClientId, ClientSummary\r\n 5. Table: InvestmentGoalsDetails\r\n Columns: ClientId, InvestmentGoal, TargetAmount, Contribution\r\n 6. Table: Retirement\r\n Columns: ClientId, StatusDate, RetirementGoalProgress, EducationGoalProgress\r\n 7. Table: ClientMeetings\r\n Columns: ClientId, ConversationId, Title, StartTime, EndTime, Advisor, ClientEmail\r\n Always use the Investment column from the Assets table as the value.\r\n Assets table has snapshots of values by date. Do not add numbers across different dates for total values.\r\n Do not use client name in filters.\r\n Do not include assets values unless asked for.\r\n ALWAYS use ClientId = {clientid} in the query filter.\r\n ALWAYS select Client Name (Column: Client) in the query.\r\n Query filters are IMPORTANT. Add filters like AssetType, AssetDate, etc. if needed.\r\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.\r\n Only return the generated SQL query. Do not return anything else.", @@ -744,7 +744,7 @@ "_generator": { "name": "bicep", "version": "0.36.177.2456", - "templateHash": "1124249040831466979" + "templateHash": "1343961496887433815" } }, "parameters": { @@ -1458,7 +1458,7 @@ "_generator": { "name": "bicep", "version": "0.36.177.2456", - "templateHash": "11899270249637077405" + "templateHash": "10199364008784095733" } }, "parameters": { @@ -2244,7 +2244,7 @@ "_generator": { "name": "bicep", "version": "0.36.177.2456", - "templateHash": "10507186896960913919" + "templateHash": "7899619253922538038" } }, "parameters": { @@ -2615,6 +2615,10 @@ "serverFarmId": "[parameters('HostingPlanName')]", "siteConfig": { "appSettings": [ + { + "name": "APP_ENV", + "value": "Prod" + }, { "name": "APPINSIGHTS_INSTRUMENTATIONKEY", "value": "[reference(parameters('applicationInsightsId'), '2015-05-01').InstrumentationKey]" @@ -2894,7 +2898,7 @@ "_generator": { "name": "bicep", "version": "0.36.177.2456", - "templateHash": "11899270249637077405" + "templateHash": "10199364008784095733" } }, "parameters": { diff --git a/infra/scripts/fabric_scripts/create_fabric_items.py b/infra/scripts/fabric_scripts/create_fabric_items.py index 9a718a425..9e32fb2b0 100644 --- a/infra/scripts/fabric_scripts/create_fabric_items.py +++ b/infra/scripts/fabric_scripts/create_fabric_items.py @@ -1,4 +1,3 @@ -from azure.identity import DefaultAzureCredential import base64 import json import requests @@ -8,7 +7,6 @@ import time -# credential = DefaultAzureCredential() from azure.identity import AzureCliCredential credential = AzureCliCredential() diff --git a/infra/scripts/index_scripts/create_search_index.py b/infra/scripts/index_scripts/create_search_index.py index b429a6456..92c726af0 100644 --- a/infra/scripts/index_scripts/create_search_index.py +++ b/infra/scripts/index_scripts/create_search_index.py @@ -44,9 +44,7 @@ "clienttranscripts/meeting_transcripts_metadata/transcripts_metadata.csv" ) -credential = DefaultAzureCredential( - managed_identity_client_id=managed_identity_client_id -) +credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. token_provider = get_bearer_token_provider( credential, "https://cognitiveservices.azure.com/.default" diff --git a/infra/scripts/index_scripts/create_sql_tables.py b/infra/scripts/index_scripts/create_sql_tables.py index be04dbc7a..5a1335569 100644 --- a/infra/scripts/index_scripts/create_sql_tables.py +++ b/infra/scripts/index_scripts/create_sql_tables.py @@ -13,9 +13,7 @@ def get_secrets_from_kv(kv_name, secret_name): key_vault_name = kv_name # Set the name of the Azure Key Vault - credential = DefaultAzureCredential( - managed_identity_client_id=managed_identity_client_id - ) + credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. secret_client = SecretClient( vault_url=f"https://{key_vault_name}.vault.azure.net/", credential=credential ) # Create a secret client object using the credential and Key Vault name @@ -27,9 +25,7 @@ def get_secrets_from_kv(kv_name, secret_name): driver = "{ODBC Driver 18 for SQL Server}" -credential = DefaultAzureCredential( - managed_identity_client_id=managed_identity_client_id -) +credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. token_bytes = credential.get_token( "https://database.windows.net/.default" @@ -49,9 +45,7 @@ def get_secrets_from_kv(kv_name, secret_name): from azure.storage.filedatalake import DataLakeServiceClient account_name = get_secrets_from_kv(key_vault_name, "ADLS-ACCOUNT-NAME") -credential = DefaultAzureCredential( - managed_identity_client_id=managed_identity_client_id -) +credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. account_url = f"https://{account_name}.dfs.core.windows.net" diff --git a/infra/scripts/index_scripts/create_update_sql_dates.py b/infra/scripts/index_scripts/create_update_sql_dates.py index 47e1a6364..87852806c 100644 --- a/infra/scripts/index_scripts/create_update_sql_dates.py +++ b/infra/scripts/index_scripts/create_update_sql_dates.py @@ -12,9 +12,7 @@ def get_secrets_from_kv(kv_name, secret_name): key_vault_name = kv_name # Set the name of the Azure Key Vault - credential = DefaultAzureCredential( - managed_identity_client_id=managed_identity_client_id - ) + credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. secret_client = SecretClient( vault_url=f"https://{key_vault_name}.vault.azure.net/", credential=credential ) # Create a secret client object using the credential and Key Vault name @@ -32,9 +30,7 @@ def get_secrets_from_kv(kv_name, secret_name): from azure.storage.filedatalake import DataLakeServiceClient account_name = get_secrets_from_kv(key_vault_name, "ADLS-ACCOUNT-NAME") -credential = DefaultAzureCredential( - managed_identity_client_id=managed_identity_client_id -) +credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. account_url = f"https://{account_name}.dfs.core.windows.net" diff --git a/src/App/.env.sample b/src/App/.env.sample index 0f69d5442..53ae69bf2 100644 --- a/src/App/.env.sample +++ b/src/App/.env.sample @@ -67,4 +67,5 @@ AZURE_SQL_SYSTEM_PROMPT="Generate a valid T-SQL query to find {query} for tables # Misc APPINSIGHTS_INSTRUMENTATIONKEY= AUTH_ENABLED="false" -USE_INTERNAL_STREAM="True" \ No newline at end of file +USE_INTERNAL_STREAM="True" +APP_ENV="dev" \ No newline at end of file diff --git a/src/App/app.py b/src/App/app.py index 901494b2b..f3e3a9b16 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -6,7 +6,9 @@ import uuid from types import SimpleNamespace -from azure.identity import DefaultAzureCredential, get_bearer_token_provider +from azure.identity import get_bearer_token_provider +from backend.helpers.azure_credential_utils import get_azure_credential +from backend.helpers.azure_credential_utils import get_azure_credential_async from azure.monitor.opentelemetry import configure_azure_monitor # from quart.sessions import SecureCookieSessionInterface @@ -185,7 +187,7 @@ def init_openai_client(use_data=SHOULD_USE_DATA): if not aoai_api_key: logging.debug("No AZURE_OPENAI_KEY found, using Azure AD auth") ad_token_provider = get_bearer_token_provider( - DefaultAzureCredential(), "https://cognitiveservices.azure.com/.default" + get_azure_credential(), "https://cognitiveservices.azure.com/.default" ) # Deployment @@ -233,7 +235,7 @@ def init_cosmosdb_client(): ) if not config.AZURE_COSMOSDB_ACCOUNT_KEY: - credential = DefaultAzureCredential() + credential = get_azure_credential() else: credential = config.AZURE_COSMOSDB_ACCOUNT_KEY diff --git a/src/App/backend/agents/agent_factory.py b/src/App/backend/agents/agent_factory.py index df81a2caf..6bae33808 100644 --- a/src/App/backend/agents/agent_factory.py +++ b/src/App/backend/agents/agent_factory.py @@ -10,8 +10,8 @@ from typing import Optional from azure.ai.projects import AIProjectClient -from azure.identity import DefaultAzureCredential as DefaultAzureCredentialSync -from azure.identity.aio import DefaultAzureCredential +from backend.helpers.azure_credential_utils import get_azure_credential +from backend.helpers.azure_credential_utils import get_azure_credential_async from semantic_kernel.agents import AzureAIAgent, AzureAIAgentSettings from backend.common.config import config @@ -35,7 +35,7 @@ async def get_wealth_advisor_agent(cls): async with cls._lock: if cls._wealth_advisor_agent is None: ai_agent_settings = AzureAIAgentSettings() - creds = DefaultAzureCredential() + creds = await get_azure_credential_async() client = AzureAIAgent.create_client( credential=creds, endpoint=ai_agent_settings.endpoint ) @@ -76,7 +76,7 @@ async def get_search_agent(cls): project_client = AIProjectClient( endpoint=config.AI_PROJECT_ENDPOINT, - credential=DefaultAzureCredentialSync(), + credential=get_azure_credential(), api_version="2025-05-01", ) @@ -137,7 +137,7 @@ async def get_sql_agent(cls) -> dict: project_client = AIProjectClient( endpoint=config.AI_PROJECT_ENDPOINT, - credential=DefaultAzureCredentialSync(), + credential=get_azure_credential(), api_version="2025-05-01", ) diff --git a/src/App/backend/helpers/azure_credential_utils.py b/src/App/backend/helpers/azure_credential_utils.py new file mode 100644 index 000000000..646efb444 --- /dev/null +++ b/src/App/backend/helpers/azure_credential_utils.py @@ -0,0 +1,41 @@ +import os +from azure.identity import ManagedIdentityCredential, DefaultAzureCredential +from azure.identity.aio import ManagedIdentityCredential as AioManagedIdentityCredential, DefaultAzureCredential as AioDefaultAzureCredential + + +async def get_azure_credential_async(client_id=None): + """ + Returns an Azure credential asynchronously based on the application environment. + + If the environment is 'dev', it uses AioDefaultAzureCredential. + Otherwise, it uses AioManagedIdentityCredential. + + Args: + client_id (str, optional): The client ID for the Managed Identity Credential. + + Returns: + Credential object: Either AioDefaultAzureCredential or AioManagedIdentityCredential. + """ + if os.getenv("APP_ENV", "prod").lower() == 'dev': + return AioDefaultAzureCredential() # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in development + else: + return AioManagedIdentityCredential(client_id=client_id) + + +def get_azure_credential(client_id=None): + """ + Returns an Azure credential based on the application environment. + + If the environment is 'dev', it uses DefaultAzureCredential. + Otherwise, it uses ManagedIdentityCredential. + + Args: + client_id (str, optional): The client ID for the Managed Identity Credential. + + Returns: + Credential object: Either DefaultAzureCredential or ManagedIdentityCredential. + """ + if os.getenv("APP_ENV", "prod").lower() == 'dev': + return DefaultAzureCredential() # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in development + else: + return ManagedIdentityCredential(client_id=client_id) diff --git a/src/App/backend/plugins/chat_with_data_plugin.py b/src/App/backend/plugins/chat_with_data_plugin.py index f421af7ef..86b064aac 100644 --- a/src/App/backend/plugins/chat_with_data_plugin.py +++ b/src/App/backend/plugins/chat_with_data_plugin.py @@ -9,7 +9,8 @@ MessageRole, ) from azure.ai.projects import AIProjectClient -from azure.identity import DefaultAzureCredential, get_bearer_token_provider +from azure.identity import get_bearer_token_provider +from backend.helpers.azure_credential_utils import get_azure_credential from semantic_kernel.functions.kernel_function_decorator import kernel_function from backend.common.config import config @@ -202,7 +203,7 @@ async def get_answers_from_calltranscripts( def get_openai_client(self): token_provider = get_bearer_token_provider( - DefaultAzureCredential(), "https://cognitiveservices.azure.com/.default" + get_azure_credential(), "https://cognitiveservices.azure.com/.default" ) openai_client = openai.AzureOpenAI( azure_endpoint=config.AZURE_OPENAI_ENDPOINT, @@ -213,7 +214,7 @@ def get_openai_client(self): def get_project_openai_client(self): project = AIProjectClient( - endpoint=config.AI_PROJECT_ENDPOINT, credential=DefaultAzureCredential() + endpoint=config.AI_PROJECT_ENDPOINT, credential=get_azure_credential() ) openai_client = project.inference.get_azure_openai_client( api_version=config.AZURE_OPENAI_PREVIEW_API_VERSION diff --git a/src/App/backend/services/sqldb_service.py b/src/App/backend/services/sqldb_service.py index 4368e4721..063c08bd1 100644 --- a/src/App/backend/services/sqldb_service.py +++ b/src/App/backend/services/sqldb_service.py @@ -3,7 +3,7 @@ import struct import pyodbc -from azure.identity import DefaultAzureCredential +from backend.helpers.azure_credential_utils import get_azure_credential from dotenv import load_dotenv from backend.common.config import config @@ -40,7 +40,7 @@ def get_connection(): for attempt in range(max_retries): try: - credential = DefaultAzureCredential(managed_identity_client_id=mid_id) + credential = get_azure_credential(client_id=mid_id) token_bytes = credential.get_token( "https://database.windows.net/.default" diff --git a/src/App/tests/backend/agents/test_agent_factory.py b/src/App/tests/backend/agents/test_agent_factory.py index 775cdab4c..676bba7be 100644 --- a/src/App/tests/backend/agents/test_agent_factory.py +++ b/src/App/tests/backend/agents/test_agent_factory.py @@ -21,7 +21,7 @@ def reset_singleton(self): @pytest.mark.asyncio @patch("backend.agents.agent_factory.AzureAIAgent") - @patch("backend.agents.agent_factory.DefaultAzureCredential") + @patch("backend.agents.agent_factory.get_azure_credential_async") @patch("backend.agents.agent_factory.AzureAIAgentSettings") @patch("backend.agents.agent_factory.ChatWithDataPlugin") async def test_get_wealth_advisor_agent_creates_agent_when_none_exists( @@ -74,7 +74,7 @@ async def test_get_wealth_advisor_agent_returns_existing_agent( @pytest.mark.asyncio @patch("backend.agents.agent_factory.config") @patch("backend.agents.agent_factory.AIProjectClient") - @patch("backend.agents.agent_factory.DefaultAzureCredentialSync") + @patch("backend.agents.agent_factory.get_azure_credential") async def test_get_search_agent_creates_agent_when_none_exists( self, mock_credential_sync, mock_ai_project_client, mock_config, reset_singleton ): @@ -112,7 +112,7 @@ async def test_get_search_agent_creates_agent_when_none_exists( @pytest.mark.asyncio @patch("backend.agents.agent_factory.config") @patch("backend.agents.agent_factory.AIProjectClient") - @patch("backend.agents.agent_factory.DefaultAzureCredentialSync") + @patch("backend.agents.agent_factory.get_azure_credential") async def test_get_search_agent_with_default_instructions( self, mock_credential_sync, mock_ai_project_client, mock_config, reset_singleton ): @@ -174,7 +174,7 @@ async def test_multiple_calls_return_same_wealth_advisor_instance( ) mock_agent_class.return_value = mock_agent_instance - with patch("backend.agents.agent_factory.DefaultAzureCredential"): + with patch("backend.agents.agent_factory.get_azure_credential_async"): with patch("backend.agents.agent_factory.AzureAIAgentSettings"): with patch("backend.agents.agent_factory.ChatWithDataPlugin"): # Act @@ -193,7 +193,7 @@ async def test_multiple_calls_return_same_search_agent_instance( with patch( "backend.agents.agent_factory.AIProjectClient" ) as mock_ai_project_client: - with patch("backend.agents.agent_factory.DefaultAzureCredentialSync"): + with patch("backend.agents.agent_factory.get_azure_credential"): mock_config.CALL_TRANSCRIPT_SYSTEM_PROMPT = "Test instructions" mock_config.AI_PROJECT_ENDPOINT = "https://test.endpoint.com" mock_config.AZURE_OPENAI_MODEL = "test-model" diff --git a/src/App/tests/backend/helpers/test_azure_credential_utils.py b/src/App/tests/backend/helpers/test_azure_credential_utils.py new file mode 100644 index 000000000..fd98527f5 --- /dev/null +++ b/src/App/tests/backend/helpers/test_azure_credential_utils.py @@ -0,0 +1,78 @@ +import pytest +import sys +import os +from unittest.mock import patch, MagicMock + +# Ensure src/backend is on the Python path for imports +sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..'))) + +import helpers.azure_credential_utils as azure_credential_utils + +# Synchronous tests + +@patch("helpers.azure_credential_utils.os.getenv") +@patch("helpers.azure_credential_utils.DefaultAzureCredential") +@patch("helpers.azure_credential_utils.ManagedIdentityCredential") +def test_get_azure_credential_dev_env(mock_managed_identity_credential, mock_default_azure_credential, mock_getenv): + """Test get_azure_credential in dev environment.""" + mock_getenv.return_value = "dev" + mock_default_credential = MagicMock() + mock_default_azure_credential.return_value = mock_default_credential + + credential = azure_credential_utils.get_azure_credential() + + mock_getenv.assert_called_once_with("APP_ENV", "prod") + mock_default_azure_credential.assert_called_once() + mock_managed_identity_credential.assert_not_called() + assert credential == mock_default_credential + +@patch("helpers.azure_credential_utils.os.getenv") +@patch("helpers.azure_credential_utils.DefaultAzureCredential") +@patch("helpers.azure_credential_utils.ManagedIdentityCredential") +def test_get_azure_credential_non_dev_env(mock_managed_identity_credential, mock_default_azure_credential, mock_getenv): + """Test get_azure_credential in non-dev environment.""" + mock_getenv.return_value = "prod" + mock_managed_credential = MagicMock() + mock_managed_identity_credential.return_value = mock_managed_credential + credential = azure_credential_utils.get_azure_credential(client_id="test-client-id") + + mock_getenv.assert_called_once_with("APP_ENV", "prod") + mock_managed_identity_credential.assert_called_once_with(client_id="test-client-id") + mock_default_azure_credential.assert_not_called() + assert credential == mock_managed_credential + +# Asynchronous tests + +@pytest.mark.asyncio +@patch("helpers.azure_credential_utils.os.getenv") +@patch("helpers.azure_credential_utils.AioDefaultAzureCredential") +@patch("helpers.azure_credential_utils.AioManagedIdentityCredential") +async def test_get_azure_credential_async_dev_env(mock_aio_managed_identity_credential, mock_aio_default_azure_credential, mock_getenv): + """Test get_azure_credential_async in dev environment.""" + mock_getenv.return_value = "dev" + mock_aio_default_credential = MagicMock() + mock_aio_default_azure_credential.return_value = mock_aio_default_credential + + credential = await azure_credential_utils.get_azure_credential_async() + + mock_getenv.assert_called_once_with("APP_ENV", "prod") + mock_aio_default_azure_credential.assert_called_once() + mock_aio_managed_identity_credential.assert_not_called() + assert credential == mock_aio_default_credential + +@pytest.mark.asyncio +@patch("helpers.azure_credential_utils.os.getenv") +@patch("helpers.azure_credential_utils.AioDefaultAzureCredential") +@patch("helpers.azure_credential_utils.AioManagedIdentityCredential") +async def test_get_azure_credential_async_non_dev_env(mock_aio_managed_identity_credential, mock_aio_default_azure_credential, mock_getenv): + """Test get_azure_credential_async in non-dev environment.""" + mock_getenv.return_value = "prod" + mock_aio_managed_credential = MagicMock() + mock_aio_managed_identity_credential.return_value = mock_aio_managed_credential + + credential = await azure_credential_utils.get_azure_credential_async(client_id="test-client-id") + + mock_getenv.assert_called_once_with("APP_ENV", "prod") + mock_aio_managed_identity_credential.assert_called_once_with(client_id="test-client-id") + mock_aio_default_azure_credential.assert_not_called() + assert credential == mock_aio_managed_credential \ No newline at end of file diff --git a/src/App/tests/backend/plugins/test_chat_with_data_plugin.py b/src/App/tests/backend/plugins/test_chat_with_data_plugin.py index 684c947ae..cc1e91912 100644 --- a/src/App/tests/backend/plugins/test_chat_with_data_plugin.py +++ b/src/App/tests/backend/plugins/test_chat_with_data_plugin.py @@ -15,7 +15,7 @@ def setup_method(self): @patch("backend.plugins.chat_with_data_plugin.config") @patch("backend.plugins.chat_with_data_plugin.openai.AzureOpenAI") @patch("backend.plugins.chat_with_data_plugin.get_bearer_token_provider") - @patch("backend.plugins.chat_with_data_plugin.DefaultAzureCredential") + @patch("backend.plugins.chat_with_data_plugin.get_azure_credential") def test_get_openai_client_success( self, mock_default_credential, @@ -50,7 +50,7 @@ def test_get_openai_client_success( @patch("backend.plugins.chat_with_data_plugin.config") @patch("backend.plugins.chat_with_data_plugin.AIProjectClient") - @patch("backend.plugins.chat_with_data_plugin.DefaultAzureCredential") + @patch("backend.plugins.chat_with_data_plugin.get_azure_credential") def test_get_project_openai_client_success( self, mock_default_credential, mock_ai_project_client, mock_config ): diff --git a/src/App/tests/backend/services/test_sqldb_service.py b/src/App/tests/backend/services/test_sqldb_service.py index 3a3745c3f..bed58d9e9 100644 --- a/src/App/tests/backend/services/test_sqldb_service.py +++ b/src/App/tests/backend/services/test_sqldb_service.py @@ -16,7 +16,7 @@ @patch("backend.services.sqldb_service.pyodbc.connect") # Mock pyodbc.connect @patch( - "backend.services.sqldb_service.DefaultAzureCredential" + "backend.services.sqldb_service.get_azure_credential" ) # Mock DefaultAzureCredential def test_get_connection(mock_credential_class, mock_connect): # Mock the DefaultAzureCredential and get_token method @@ -34,7 +34,7 @@ def test_get_connection(mock_credential_class, mock_connect): # Assert that DefaultAzureCredential and get_token were called correctly mock_credential_class.assert_called_once_with( - managed_identity_client_id=sql_db.mid_id + client_id=sql_db.mid_id ) mock_credential.get_token.assert_called_once_with( "https://database.windows.net/.default" @@ -59,7 +59,7 @@ def test_get_connection(mock_credential_class, mock_connect): @patch("backend.services.sqldb_service.pyodbc.connect") # Mock pyodbc.connect @patch( - "backend.services.sqldb_service.DefaultAzureCredential" + "backend.helpers.azure_credential_utils.get_azure_credential" ) # Mock DefaultAzureCredential def test_get_connection_token_failure(mock_credential_class, mock_connect): # Mock the DefaultAzureCredential and get_token method From 4772ad3a31c23ad14076abd1b687d2ae7830f52b Mon Sep 17 00:00:00 2001 From: Rafi-Microsoft Date: Wed, 30 Jul 2025 20:17:48 +0530 Subject: [PATCH 09/14] sfi changes v2 --- src/App/app.py | 1 - .../tests/backend/helpers/test_azure_credential_utils.py | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/App/app.py b/src/App/app.py index f3e3a9b16..cccc2caf5 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -8,7 +8,6 @@ from azure.identity import get_bearer_token_provider from backend.helpers.azure_credential_utils import get_azure_credential -from backend.helpers.azure_credential_utils import get_azure_credential_async from azure.monitor.opentelemetry import configure_azure_monitor # from quart.sessions import SecureCookieSessionInterface diff --git a/src/App/tests/backend/helpers/test_azure_credential_utils.py b/src/App/tests/backend/helpers/test_azure_credential_utils.py index fd98527f5..8e93d3abf 100644 --- a/src/App/tests/backend/helpers/test_azure_credential_utils.py +++ b/src/App/tests/backend/helpers/test_azure_credential_utils.py @@ -2,14 +2,14 @@ import sys import os from unittest.mock import patch, MagicMock +import backend.helpers.azure_credential_utils as azure_credential_utils # Ensure src/backend is on the Python path for imports sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), '..', '..'))) -import helpers.azure_credential_utils as azure_credential_utils - # Synchronous tests + @patch("helpers.azure_credential_utils.os.getenv") @patch("helpers.azure_credential_utils.DefaultAzureCredential") @patch("helpers.azure_credential_utils.ManagedIdentityCredential") @@ -26,6 +26,7 @@ def test_get_azure_credential_dev_env(mock_managed_identity_credential, mock_def mock_managed_identity_credential.assert_not_called() assert credential == mock_default_credential + @patch("helpers.azure_credential_utils.os.getenv") @patch("helpers.azure_credential_utils.DefaultAzureCredential") @patch("helpers.azure_credential_utils.ManagedIdentityCredential") @@ -43,6 +44,7 @@ def test_get_azure_credential_non_dev_env(mock_managed_identity_credential, mock # Asynchronous tests + @pytest.mark.asyncio @patch("helpers.azure_credential_utils.os.getenv") @patch("helpers.azure_credential_utils.AioDefaultAzureCredential") @@ -60,6 +62,7 @@ async def test_get_azure_credential_async_dev_env(mock_aio_managed_identity_cred mock_aio_managed_identity_credential.assert_not_called() assert credential == mock_aio_default_credential + @pytest.mark.asyncio @patch("helpers.azure_credential_utils.os.getenv") @patch("helpers.azure_credential_utils.AioDefaultAzureCredential") @@ -75,4 +78,4 @@ async def test_get_azure_credential_async_non_dev_env(mock_aio_managed_identity_ mock_getenv.assert_called_once_with("APP_ENV", "prod") mock_aio_managed_identity_credential.assert_called_once_with(client_id="test-client-id") mock_aio_default_azure_credential.assert_not_called() - assert credential == mock_aio_managed_credential \ No newline at end of file + assert credential == mock_aio_managed_credential From 2088121981504530d56a672afef9a348c69c4c67 Mon Sep 17 00:00:00 2001 From: Rafi-Microsoft Date: Wed, 30 Jul 2025 20:24:00 +0530 Subject: [PATCH 10/14] sfi changes v3 --- .../helpers/test_azure_credential_utils.py | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/App/tests/backend/helpers/test_azure_credential_utils.py b/src/App/tests/backend/helpers/test_azure_credential_utils.py index 8e93d3abf..4eb79c332 100644 --- a/src/App/tests/backend/helpers/test_azure_credential_utils.py +++ b/src/App/tests/backend/helpers/test_azure_credential_utils.py @@ -10,9 +10,9 @@ # Synchronous tests -@patch("helpers.azure_credential_utils.os.getenv") -@patch("helpers.azure_credential_utils.DefaultAzureCredential") -@patch("helpers.azure_credential_utils.ManagedIdentityCredential") +@patch("backend.helpers.azure_credential_utils.os.getenv") +@patch("backend.helpers.azure_credential_utils.DefaultAzureCredential") +@patch("backend.helpers.azure_credential_utils.ManagedIdentityCredential") def test_get_azure_credential_dev_env(mock_managed_identity_credential, mock_default_azure_credential, mock_getenv): """Test get_azure_credential in dev environment.""" mock_getenv.return_value = "dev" @@ -27,9 +27,9 @@ def test_get_azure_credential_dev_env(mock_managed_identity_credential, mock_def assert credential == mock_default_credential -@patch("helpers.azure_credential_utils.os.getenv") -@patch("helpers.azure_credential_utils.DefaultAzureCredential") -@patch("helpers.azure_credential_utils.ManagedIdentityCredential") +@patch("backend.helpers.azure_credential_utils.os.getenv") +@patch("backend.helpers.azure_credential_utils.DefaultAzureCredential") +@patch("backend.helpers.azure_credential_utils.ManagedIdentityCredential") def test_get_azure_credential_non_dev_env(mock_managed_identity_credential, mock_default_azure_credential, mock_getenv): """Test get_azure_credential in non-dev environment.""" mock_getenv.return_value = "prod" @@ -46,9 +46,9 @@ def test_get_azure_credential_non_dev_env(mock_managed_identity_credential, mock @pytest.mark.asyncio -@patch("helpers.azure_credential_utils.os.getenv") -@patch("helpers.azure_credential_utils.AioDefaultAzureCredential") -@patch("helpers.azure_credential_utils.AioManagedIdentityCredential") +@patch("backend.helpers.azure_credential_utils.os.getenv") +@patch("backend.helpers.azure_credential_utils.AioDefaultAzureCredential") +@patch("backend.helpers.azure_credential_utils.AioManagedIdentityCredential") async def test_get_azure_credential_async_dev_env(mock_aio_managed_identity_credential, mock_aio_default_azure_credential, mock_getenv): """Test get_azure_credential_async in dev environment.""" mock_getenv.return_value = "dev" @@ -64,9 +64,9 @@ async def test_get_azure_credential_async_dev_env(mock_aio_managed_identity_cred @pytest.mark.asyncio -@patch("helpers.azure_credential_utils.os.getenv") -@patch("helpers.azure_credential_utils.AioDefaultAzureCredential") -@patch("helpers.azure_credential_utils.AioManagedIdentityCredential") +@patch("backend.helpers.azure_credential_utils.os.getenv") +@patch("backend.helpers.azure_credential_utils.AioDefaultAzureCredential") +@patch("backend.helpers.azure_credential_utils.AioManagedIdentityCredential") async def test_get_azure_credential_async_non_dev_env(mock_aio_managed_identity_credential, mock_aio_default_azure_credential, mock_getenv): """Test get_azure_credential_async in non-dev environment.""" mock_getenv.return_value = "prod" From 2cdbaf9a2122c796218a7f4de24f714149b16de4 Mon Sep 17 00:00:00 2001 From: Rafi-Microsoft Date: Wed, 30 Jul 2025 20:33:21 +0530 Subject: [PATCH 11/14] sfi changes v4 --- src/App/tests/backend/services/test_sqldb_service.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/App/tests/backend/services/test_sqldb_service.py b/src/App/tests/backend/services/test_sqldb_service.py index bed58d9e9..3a6866a1b 100644 --- a/src/App/tests/backend/services/test_sqldb_service.py +++ b/src/App/tests/backend/services/test_sqldb_service.py @@ -59,7 +59,7 @@ def test_get_connection(mock_credential_class, mock_connect): @patch("backend.services.sqldb_service.pyodbc.connect") # Mock pyodbc.connect @patch( - "backend.helpers.azure_credential_utils.get_azure_credential" + "backend.services.sqldb_service.get_azure_credential" ) # Mock DefaultAzureCredential def test_get_connection_token_failure(mock_credential_class, mock_connect): # Mock the DefaultAzureCredential and get_token method From 8b5e6d253275877857f0e885ff736bc02fd19eed Mon Sep 17 00:00:00 2001 From: Rafi-Microsoft Date: Wed, 30 Jul 2025 23:04:37 +0530 Subject: [PATCH 12/14] updated scripts to azureclicreds --- infra/scripts/index_scripts/create_search_index.py | 5 +++-- infra/scripts/index_scripts/create_sql_tables.py | 8 ++++---- infra/scripts/index_scripts/create_update_sql_dates.py | 6 +++--- src/App/tests/backend/services/test_sqldb_service.py | 10 +++++----- 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/infra/scripts/index_scripts/create_search_index.py b/infra/scripts/index_scripts/create_search_index.py index 92c726af0..b0a56f118 100644 --- a/infra/scripts/index_scripts/create_search_index.py +++ b/infra/scripts/index_scripts/create_search_index.py @@ -5,7 +5,8 @@ import time import pandas as pd -from azure.identity import DefaultAzureCredential, get_bearer_token_provider +from azure.identity import get_bearer_token_provider +from azure.identity import AzureCliCredential from azure.keyvault.secrets import SecretClient from azure.search.documents import SearchClient from azure.search.documents.indexes import SearchIndexClient @@ -44,7 +45,7 @@ "clienttranscripts/meeting_transcripts_metadata/transcripts_metadata.csv" ) -credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. +credential = AzureCliCredential() token_provider = get_bearer_token_provider( credential, "https://cognitiveservices.azure.com/.default" diff --git a/infra/scripts/index_scripts/create_sql_tables.py b/infra/scripts/index_scripts/create_sql_tables.py index 5a1335569..73bcc33cd 100644 --- a/infra/scripts/index_scripts/create_sql_tables.py +++ b/infra/scripts/index_scripts/create_sql_tables.py @@ -7,13 +7,13 @@ import pandas as pd import pyodbc -from azure.identity import DefaultAzureCredential +from azure.identity import AzureCliCredential from azure.keyvault.secrets import SecretClient def get_secrets_from_kv(kv_name, secret_name): key_vault_name = kv_name # Set the name of the Azure Key Vault - credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. + credential = AzureCliCredential() # Use Azure CLI Credential secret_client = SecretClient( vault_url=f"https://{key_vault_name}.vault.azure.net/", credential=credential ) # Create a secret client object using the credential and Key Vault name @@ -25,7 +25,7 @@ def get_secrets_from_kv(kv_name, secret_name): driver = "{ODBC Driver 18 for SQL Server}" -credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. +credential = AzureCliCredential() # Use Azure CLI Credential token_bytes = credential.get_token( "https://database.windows.net/.default" @@ -45,7 +45,7 @@ def get_secrets_from_kv(kv_name, secret_name): from azure.storage.filedatalake import DataLakeServiceClient account_name = get_secrets_from_kv(key_vault_name, "ADLS-ACCOUNT-NAME") -credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. +credential = AzureCliCredential() # Use Azure CLI Credential account_url = f"https://{account_name}.dfs.core.windows.net" diff --git a/infra/scripts/index_scripts/create_update_sql_dates.py b/infra/scripts/index_scripts/create_update_sql_dates.py index 87852806c..4b75c3328 100644 --- a/infra/scripts/index_scripts/create_update_sql_dates.py +++ b/infra/scripts/index_scripts/create_update_sql_dates.py @@ -6,13 +6,13 @@ import pandas as pd import pymssql -from azure.identity import DefaultAzureCredential +from azure.identity import AzureCliCredential from azure.keyvault.secrets import SecretClient def get_secrets_from_kv(kv_name, secret_name): key_vault_name = kv_name # Set the name of the Azure Key Vault - credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. + credential = AzureCliCredential() # Use Azure CLI Credential secret_client = SecretClient( vault_url=f"https://{key_vault_name}.vault.azure.net/", credential=credential ) # Create a secret client object using the credential and Key Vault name @@ -30,7 +30,7 @@ def get_secrets_from_kv(kv_name, secret_name): from azure.storage.filedatalake import DataLakeServiceClient account_name = get_secrets_from_kv(key_vault_name, "ADLS-ACCOUNT-NAME") -credential = DefaultAzureCredential(managed_identity_client_id=managed_identity_client_id) # CodeQL [SM05139] Okay use of DefaultAzureCredential as it is only used in local environment. +credential = AzureCliCredential() # Use Azure CLI Credential account_url = f"https://{account_name}.dfs.core.windows.net" diff --git a/src/App/tests/backend/services/test_sqldb_service.py b/src/App/tests/backend/services/test_sqldb_service.py index 3a6866a1b..fc61325e8 100644 --- a/src/App/tests/backend/services/test_sqldb_service.py +++ b/src/App/tests/backend/services/test_sqldb_service.py @@ -17,9 +17,9 @@ @patch("backend.services.sqldb_service.pyodbc.connect") # Mock pyodbc.connect @patch( "backend.services.sqldb_service.get_azure_credential" -) # Mock DefaultAzureCredential +) # Mock AzureCliCredential def test_get_connection(mock_credential_class, mock_connect): - # Mock the DefaultAzureCredential and get_token method + # Mock the AzureCliCredential and get_token method mock_credential = MagicMock() mock_credential_class.return_value = mock_credential mock_token = MagicMock() @@ -32,7 +32,7 @@ def test_get_connection(mock_credential_class, mock_connect): # Call the function conn = sql_db.get_connection() - # Assert that DefaultAzureCredential and get_token were called correctly + # Assert that AzureCliCredential and get_token were called correctly mock_credential_class.assert_called_once_with( client_id=sql_db.mid_id ) @@ -60,9 +60,9 @@ def test_get_connection(mock_credential_class, mock_connect): @patch("backend.services.sqldb_service.pyodbc.connect") # Mock pyodbc.connect @patch( "backend.services.sqldb_service.get_azure_credential" -) # Mock DefaultAzureCredential +) # Mock AzureCliCredential def test_get_connection_token_failure(mock_credential_class, mock_connect): - # Mock the DefaultAzureCredential and get_token method + # Mock the AzureCliCredential and get_token method mock_credential = MagicMock() mock_credential_class.return_value = mock_credential mock_token = MagicMock() From 250615747adfe5d4f7b297c0801987f4bc58536a Mon Sep 17 00:00:00 2001 From: Rafi-Microsoft Date: Thu, 31 Jul 2025 16:51:01 +0530 Subject: [PATCH 13/14] sfi changes v5 --- src/App/app.py | 26 +++---- src/App/backend/agents/agent_factory.py | 75 +++---------------- .../backend/plugins/chat_with_data_plugin.py | 35 +-------- .../plugins/test_chat_with_data_plugin.py | 44 ++--------- 4 files changed, 32 insertions(+), 148 deletions(-) diff --git a/src/App/app.py b/src/App/app.py index 515eb1342..12a4d8a57 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -82,25 +82,17 @@ async def startup(): app.wealth_advisor_agent = await AgentFactory.get_wealth_advisor_agent() logging.info("Wealth Advisor Agent initialized during application startup") app.search_agent = await AgentFactory.get_search_agent() - logging.info("Call Transcript Search Agent initialized during application startup") - app.sql_agent = await AgentFactory.get_sql_agent() - logging.info("SQL Agent initialized during application startup") + logging.info( + "Call Transcript Search Agent initialized during application startup" + ) @app.after_serving async def shutdown(): - try: - logging.info("Application shutdown initiated...") - await AgentFactory.delete_all_agent_instance() - if hasattr(app, 'wealth_advisor_agent'): - app.wealth_advisor_agent = None - if hasattr(app, 'search_agent'): - app.search_agent = None - if hasattr(app, 'sql_agent'): - app.sql_agent = None - logging.info("Agents cleaned up successfully") - except Exception as e: - logging.error(f"Error during shutdown: {e}") - logging.exception("Detailed error during shutdown") + await AgentFactory.delete_all_agent_instance() + app.wealth_advisor_agent = None + app.search_agent = None + app.sql_agent = None + logging.info("Agents cleaned up during application shutdown") # app.secret_key = secrets.token_hex(16) # app.session_interface = SecureCookieSessionInterface() @@ -1369,4 +1361,4 @@ def get_users(): return str(e), 500 -app = create_app() +app = create_app() \ No newline at end of file diff --git a/src/App/backend/agents/agent_factory.py b/src/App/backend/agents/agent_factory.py index d1f3956b8..0fe07f7ce 100644 --- a/src/App/backend/agents/agent_factory.py +++ b/src/App/backend/agents/agent_factory.py @@ -7,7 +7,6 @@ """ import asyncio -import logging from typing import Optional from azure.ai.projects import AIProjectClient @@ -27,7 +26,6 @@ class AgentFactory: _lock = asyncio.Lock() _wealth_advisor_agent: Optional[AzureAIAgent] = None _search_agent: Optional[dict] = None - _sql_agent: Optional[dict] = None @classmethod async def get_wealth_advisor_agent(cls): @@ -96,67 +94,18 @@ async def delete_all_agent_instance(cls): Delete the singleton AzureAIAgent instances if it exists. """ async with cls._lock: - logging.info("Starting agent deletion process...") - - # Delete Wealth Advisor Agent if cls._wealth_advisor_agent is not None: - try: - agent_id = cls._wealth_advisor_agent.id - logging.info(f"Deleting wealth advisor agent: {agent_id}") - if hasattr(cls._wealth_advisor_agent, 'client') and cls._wealth_advisor_agent.client: - await cls._wealth_advisor_agent.client.agents.delete_agent(agent_id) - logging.info("Wealth advisor agent deleted successfully") - else: - logging.warning("Wealth advisor agent client is None") - except Exception as e: - logging.error(f"Error deleting wealth advisor agent: {e}") - logging.exception("Detailed wealth advisor agent deletion error") - finally: - cls._wealth_advisor_agent = None - - # Delete Search Agent + await cls._wealth_advisor_agent.client.agents.delete_agent( + cls._wealth_advisor_agent.id + ) + cls._wealth_advisor_agent = None + if cls._search_agent is not None: - try: - agent_id = cls._search_agent['agent'].id - logging.info(f"Deleting search agent: {agent_id}") - if cls._search_agent.get("client") and hasattr(cls._search_agent["client"], "agents"): - # AIProjectClient.agents.delete_agent is synchronous, don't await it - cls._search_agent["client"].agents.delete_agent(agent_id) - logging.info("Search agent deleted successfully") - - # Close the client if it has a close method - if hasattr(cls._search_agent["client"], "close"): - cls._search_agent["client"].close() - else: - logging.warning("Search agent client is None or invalid") - except Exception as e: - logging.error(f"Error deleting search agent: {e}") - logging.exception("Detailed search agent deletion error") - finally: - cls._search_agent = None - - # Delete SQL Agent - if cls._sql_agent is not None: - try: - agent_id = cls._sql_agent['agent'].id - logging.info(f"Deleting SQL agent: {agent_id}") - if cls._sql_agent.get("client") and hasattr(cls._sql_agent["client"], "agents"): - # AIProjectClient.agents.delete_agent is synchronous, don't await it - cls._sql_agent["client"].agents.delete_agent(agent_id) - logging.info("SQL agent deleted successfully") - - # Close the client if it has a close method - if hasattr(cls._sql_agent["client"], "close"): - cls._sql_agent["client"].close() - else: - logging.warning("SQL agent client is None or invalid") - except Exception as e: - logging.error(f"Error deleting SQL agent: {e}") - logging.exception("Detailed SQL agent deletion error") - finally: - cls._sql_agent = None - - logging.info("Agent deletion process completed") + cls._search_agent["client"].agents.delete_agent( + cls._search_agent["agent"].id + ) + cls._search_agent["client"].close() + cls._search_agent = None @classmethod async def get_sql_agent(cls) -> dict: @@ -165,7 +114,7 @@ async def get_sql_agent(cls) -> dict: This agent is used to generate T-SQL queries from natural language input. """ async with cls._lock: - if cls._sql_agent is None: + if not hasattr(cls, "_sql_agent") or cls._sql_agent is None: agent_instructions = config.SQL_SYSTEM_PROMPT or """ You are an expert assistant in generating T-SQL queries based on user questions. @@ -199,4 +148,4 @@ async def get_sql_agent(cls) -> dict: ) cls._sql_agent = {"agent": agent, "client": project_client} - return cls._sql_agent + return cls._sql_agent \ No newline at end of file diff --git a/src/App/backend/plugins/chat_with_data_plugin.py b/src/App/backend/plugins/chat_with_data_plugin.py index 969f44b1e..86b064aac 100644 --- a/src/App/backend/plugins/chat_with_data_plugin.py +++ b/src/App/backend/plugins/chat_with_data_plugin.py @@ -12,7 +12,6 @@ from azure.identity import get_bearer_token_provider from backend.helpers.azure_credential_utils import get_azure_credential from semantic_kernel.functions.kernel_function_decorator import kernel_function -from quart import current_app from backend.common.config import config from backend.services.sqldb_service import get_connection @@ -43,14 +42,9 @@ async def get_SQL_Response( if not input or not input.strip(): return "Error: Query input is required" - thread = None try: - # TEMPORARY: Use AgentFactory directly to debug the issue - logging.info(f"Using AgentFactory directly for SQL agent for ClientId: {ClientId}") from backend.agents.agent_factory import AgentFactory agent_info = await AgentFactory.get_sql_agent() - - logging.info(f"SQL agent retrieved: {agent_info is not None}") agent = agent_info["agent"] project_client = agent_info["client"] @@ -79,27 +73,23 @@ async def get_SQL_Response( role=MessageRole.AGENT ) sql_query = message.text.value.strip() if message else None - logging.info(f"Generated SQL query: {sql_query}") if not sql_query: return "No SQL query was generated." # Clean up triple backticks (if any) sql_query = sql_query.replace("```sql", "").replace("```", "") - logging.info(f"Cleaned SQL query: {sql_query}") # Execute the query conn = get_connection() cursor = conn.cursor() cursor.execute(sql_query) rows = cursor.fetchall() - logging.info(f"Query returned {len(rows)} rows") if not rows: result = "No data found for that client." else: result = "\n".join(str(row) for row in rows) - logging.info(f"Result preview: {result[:200]}...") conn.close() @@ -107,14 +97,6 @@ async def get_SQL_Response( except Exception as e: logging.exception("Error in get_SQL_Response") return f"Error retrieving SQL data: {str(e)}" - finally: - if thread: - try: - logging.info(f"Attempting to delete thread {thread.id}") - await project_client.agents.threads.delete(thread.id) - logging.info(f"Thread {thread.id} deleted successfully") - except Exception as e: - logging.error(f"Error deleting thread {thread.id}: {str(e)}") @kernel_function( name="ChatWithCallTranscripts", @@ -133,17 +115,12 @@ async def get_answers_from_calltranscripts( if not question or not question.strip(): return "Error: Question input is required" - thread = None try: response_text = "" - # Use the singleton search agent from app context - if not hasattr(current_app, 'search_agent') or current_app.search_agent is None: - logging.error("Search agent not found in app context, falling back to AgentFactory") - from backend.agents.agent_factory import AgentFactory - agent_info = await AgentFactory.get_search_agent() - else: - agent_info = current_app.search_agent + from backend.agents.agent_factory import AgentFactory + + agent_info: dict = await AgentFactory.get_search_agent() agent: Agent = agent_info["agent"] project_client: AIProjectClient = agent_info["client"] @@ -214,11 +191,7 @@ async def get_answers_from_calltranscripts( finally: if thread: - try: - await project_client.agents.threads.delete(thread.id) - logging.info(f"Thread {thread.id} deleted successfully") - except Exception as e: - logging.error(f"Error deleting thread {thread.id}: {str(e)}") + project_client.agents.threads.delete(thread.id) if not response_text.strip(): return "No data found for that client." diff --git a/src/App/tests/backend/plugins/test_chat_with_data_plugin.py b/src/App/tests/backend/plugins/test_chat_with_data_plugin.py index 1b942b2af..cc1e91912 100644 --- a/src/App/tests/backend/plugins/test_chat_with_data_plugin.py +++ b/src/App/tests/backend/plugins/test_chat_with_data_plugin.py @@ -180,14 +180,12 @@ async def test_get_sql_response_openai_error(self, mock_get_sql_agent, mock_conf assert "OpenAI API error" in result @pytest.mark.asyncio - @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") - @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_success( - self, mock_config, mock_get_search_agent, mock_hasattr + self, mock_get_search_agent ): """Test successful retrieval of answers from call transcripts using AI Search Agent.""" - # Setup mocks for agent factory (fallback case when current_app.search_agent is None) + # Setup mocks for agent factory mock_agent = MagicMock() mock_agent.id = "test-agent-id" @@ -197,10 +195,6 @@ async def test_get_answers_from_calltranscripts_success( "client": mock_project_client, } - # Mock config values - mock_config.AZURE_SEARCH_INDEX = "test-index" - mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" - # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -235,7 +229,7 @@ async def test_get_answers_from_calltranscripts_success( assert "Based on call transcripts" in result assert "investment options" in result - # Verify agent factory was called (fallback case) + # Verify agent factory was called mock_get_search_agent.assert_called_once() # Verify project index was created/updated @@ -255,11 +249,9 @@ async def test_get_answers_from_calltranscripts_success( mock_project_client.agents.runs.create_and_process.assert_called_once() @pytest.mark.asyncio - @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") - @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_no_results( - self, mock_config, mock_get_search_agent, mock_hasattr + self, mock_get_search_agent ): """Test call transcripts search with no results.""" # Setup mocks for agent factory @@ -272,10 +264,6 @@ async def test_get_answers_from_calltranscripts_no_results( "client": mock_project_client, } - # Mock config values - mock_config.AZURE_SEARCH_INDEX = "test-index" - mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" - # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -307,11 +295,9 @@ async def test_get_answers_from_calltranscripts_no_results( assert "No data found for that client." in result @pytest.mark.asyncio - @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") - @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_openai_error( - self, mock_config, mock_get_search_agent, mock_hasattr + self, mock_get_search_agent ): """Test call transcripts with AI Search processing error.""" # Setup mocks for agent factory @@ -324,10 +310,6 @@ async def test_get_answers_from_calltranscripts_openai_error( "client": mock_project_client, } - # Mock config values - mock_config.AZURE_SEARCH_INDEX = "test-index" - mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" - # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -354,11 +336,9 @@ async def test_get_answers_from_calltranscripts_openai_error( assert "Error retrieving data from call transcripts" in result @pytest.mark.asyncio - @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") - @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_failed_run( - self, mock_config, mock_get_search_agent, mock_hasattr + self, mock_get_search_agent ): """Test call transcripts with failed AI Search run.""" # Setup mocks for agent factory @@ -371,10 +351,6 @@ async def test_get_answers_from_calltranscripts_failed_run( "client": mock_project_client, } - # Mock config values - mock_config.AZURE_SEARCH_INDEX = "test-index" - mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" - # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" @@ -402,11 +378,9 @@ async def test_get_answers_from_calltranscripts_failed_run( assert "Error retrieving data from call transcripts" in result @pytest.mark.asyncio - @patch("backend.plugins.chat_with_data_plugin.hasattr", return_value=False) @patch("backend.agents.agent_factory.AgentFactory.get_search_agent") - @patch("backend.plugins.chat_with_data_plugin.config") async def test_get_answers_from_calltranscripts_empty_response( - self, mock_config, mock_get_search_agent, mock_hasattr + self, mock_get_search_agent ): """Test call transcripts with empty response text.""" # Setup mocks for agent factory @@ -419,10 +393,6 @@ async def test_get_answers_from_calltranscripts_empty_response( "client": mock_project_client, } - # Mock config values - mock_config.AZURE_SEARCH_INDEX = "test-index" - mock_config.AZURE_SEARCH_CONNECTION_NAME = "test-connection" - # Mock project index creation mock_index = MagicMock() mock_index.name = "project-index-test" From 5edf74bcafd6658cf3b8b589bf26f9cf72708876 Mon Sep 17 00:00:00 2001 From: Rafi-Microsoft Date: Thu, 31 Jul 2025 16:57:53 +0530 Subject: [PATCH 14/14] sfi changes v2 --- src/App/app.py | 2 +- src/App/backend/agents/agent_factory.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/App/app.py b/src/App/app.py index 12a4d8a57..3eaa8b7c1 100644 --- a/src/App/app.py +++ b/src/App/app.py @@ -1361,4 +1361,4 @@ def get_users(): return str(e), 500 -app = create_app() \ No newline at end of file +app = create_app() diff --git a/src/App/backend/agents/agent_factory.py b/src/App/backend/agents/agent_factory.py index 0fe07f7ce..6bae33808 100644 --- a/src/App/backend/agents/agent_factory.py +++ b/src/App/backend/agents/agent_factory.py @@ -148,4 +148,4 @@ async def get_sql_agent(cls) -> dict: ) cls._sql_agent = {"agent": agent, "client": project_client} - return cls._sql_agent \ No newline at end of file + return cls._sql_agent