From c24861fcc3d320ea96f46949b52926cf0f99c8c1 Mon Sep 17 00:00:00 2001 From: Mohit Sheth Date: Sun, 28 Jun 2026 00:59:39 -0400 Subject: [PATCH 1/7] Add interactive multi-turn conversations via agentic query handler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace the static help/greeting fallback with a general-purpose agentic handler that passes free-form natural language queries to the LLM with all MCP tools available. The LLM autonomously decides which Orion tools to call based on the user's question. New components: - ConversationManager: thread-safe, in-memory conversation history keyed by Slack thread, with configurable TTL and rolling window - general_query_analyzer: agentic handler following the pr_analyzer pattern — builds messages with conversation history and calls analyze_with_agentic - GENERAL_QUERY_PROMPT: system prompt guiding the LLM on available tools and Slack formatting - Socket listener now handles thread reply messages for multi-turn follow-ups without requiring @mention Also: - Pin mcp>=1.18.0 in requirements.txt - Fix mcp_config.json to use 127.0.0.1 (IPv4) instead of localhost Assisted-by: Claude Signed-off-by: Mohit Sheth --- bugzooka/analysis/general_query_analyzer.py | 74 ++++++ bugzooka/analysis/prompts.py | 34 +++ bugzooka/core/conversation.py | 89 +++++++ .../integrations/slack_socket_listener.py | 248 ++++++++++++------ mcp_config.json | 2 +- requirements.txt | 1 + tests/test_conversation.py | 87 ++++++ tests/test_general_query_analyzer.py | 156 +++++++++++ 8 files changed, 612 insertions(+), 79 deletions(-) create mode 100644 bugzooka/analysis/general_query_analyzer.py create mode 100644 bugzooka/core/conversation.py create mode 100644 tests/test_conversation.py create mode 100644 tests/test_general_query_analyzer.py diff --git a/bugzooka/analysis/general_query_analyzer.py b/bugzooka/analysis/general_query_analyzer.py new file mode 100644 index 0000000..154d204 --- /dev/null +++ b/bugzooka/analysis/general_query_analyzer.py @@ -0,0 +1,74 @@ +"""General-purpose agentic query handler for free-form natural language questions. + +Uses the same agentic LLM loop as pr_analyzer.py, but with a flexible prompt +that lets the LLM autonomously select which MCP tools to call based on the +user's natural language query. Supports multi-turn conversation via message history. +""" + +import logging + +from bugzooka.integrations.mcp_client import initialize_global_resources_async +from bugzooka.core.utils import make_response +from bugzooka.integrations.inference_client import analyze_with_agentic +from bugzooka.analysis.prompts import GENERAL_QUERY_PROMPT +import bugzooka.integrations.mcp_client as mcp_module + +logger = logging.getLogger(__name__) + + +async def analyze_general_query( + text: str, + conversation_messages: list[dict], + channel_id: str | None = None, +) -> dict: + """ + Handle a free-form natural language query using the agentic LLM loop. + + The LLM receives the full conversation history and all available MCP tools, + then autonomously decides which tools to call to answer the user's question. + + :param text: The current user message (already included in conversation_messages) + :param conversation_messages: Full conversation history in OpenAI message format + :param channel_id: Slack channel ID for ES_SERVER routing (optional) + :return: Dictionary with 'success' (bool) and 'message' (str) + """ + if channel_id: + from bugzooka.integrations.mcp_interceptors import current_channel + + current_channel.set(channel_id) + logger.debug("Set channel context for general query: %s", channel_id) + + await initialize_global_resources_async() + + if not mcp_module.mcp_tools: + return make_response( + success=False, + message="No MCP tools available. Please check the MCP server connection.", + ) + + messages = [ + {"role": "system", "content": GENERAL_QUERY_PROMPT["system"]}, + ] + messages.extend(conversation_messages) + + try: + result = await analyze_with_agentic( + messages=messages, + tools=mcp_module.mcp_tools, + ) + + if not result: + logger.warning("LLM returned empty result for general query") + return make_response( + success=False, + message="I couldn't generate a response. Please try rephrasing your question.", + ) + + return make_response(success=True, message=result) + + except Exception as e: + logger.error("Error during general query analysis: %s", e, exc_info=True) + return make_response( + success=False, + message=f"An error occurred while processing your query: {e}", + ) diff --git a/bugzooka/analysis/prompts.py b/bugzooka/analysis/prompts.py index d37c4ef..9be6ffc 100644 --- a/bugzooka/analysis/prompts.py +++ b/bugzooka/analysis/prompts.py @@ -140,6 +140,40 @@ Beginning analysis now. """, } +GENERAL_QUERY_PROMPT = { + "system": """You are PerfScale Jedi, an AI assistant specializing in OpenShift performance analysis. +You help engineers investigate performance metrics, detect regressions, and understand trends. + +You have access to Orion MCP tools that can: +- List available test configs: `get_orion_configs` +- List metrics for a config: `get_orion_metrics`, `get_orion_metrics_with_meta` +- Get raw performance data values: `get_orion_performance_data` — USE THIS for reporting on metrics. It returns actual numeric values you can analyze and present. +- Check for regressions across all configs: `has_openshift_regressed` +- Check networking-specific regressions: `has_networking_regressed` +- Detect nightly build regressions: `has_nightly_regressed` +- Analyze PR performance impact: `openshift_report_on_pr` +- Correlate two metrics: `metrics_correlation` +- Get OpenShift release dates: `get_release_date` + +IMPORTANT tool guidance: +- For performance reports and metric analysis, ALWAYS use `get_orion_performance_data` to fetch raw numeric data. This returns a list of values you can compute stats on (min, max, avg, trend). +- Do NOT use `openshift_report_on` — it returns images that cannot be displayed in Slack. +- When the user asks about "how is X doing", fetch the key metrics using `get_orion_performance_data` for each metric, then summarize the results with actual numbers. +- To find available metrics for a config, call `get_orion_metrics` first. + +General instructions: +- When the user mentions a test or config name partially, match it to the right Orion config file. + Examples: "cudn" or "cudn-density" likely means a config like "small-scale-cudn-density-l2-single-ns.yaml". + If ambiguous, call `get_orion_configs` first to find the right one, or ask the user. +- Pass version numbers as-is to tools (e.g., "5.0", "4.22"). +- Format responses for Slack: use *bold* for headers, `code` for metric names, and code blocks for tables. +- Be concise and data-driven. When showing metrics, include actual values, averages, and trends. +- Highlight any regressions or notable changes in recent data points. +- If tools return no data or errors, explain what happened clearly. +- You are in a multi-turn conversation. Use prior context to understand follow-up questions. +""", +} + # Jira tool prompt - used when Jira MCP tools are available JIRA_TOOL_PROMPT = { "system": ( diff --git a/bugzooka/core/conversation.py b/bugzooka/core/conversation.py new file mode 100644 index 0000000..5b42e4e --- /dev/null +++ b/bugzooka/core/conversation.py @@ -0,0 +1,89 @@ +"""Thread-safe, in-memory conversation history manager for multi-turn Slack interactions.""" + +import os +import threading +import time +from dataclasses import dataclass, field +from typing import Dict, List + +CONVERSATION_TTL_SECONDS = int(os.getenv("CONVERSATION_TTL_SECONDS", "7200")) +MAX_MESSAGES_PER_THREAD = int(os.getenv("MAX_MESSAGES_PER_THREAD", "20")) + + +@dataclass +class ConversationState: + channel_id: str + thread_ts: str + messages: List[dict] = field(default_factory=list) + last_activity: float = field(default_factory=time.time) + + +class ConversationManager: + """Manages per-thread conversation history with TTL-based expiry.""" + + def __init__( + self, + ttl_seconds: int = CONVERSATION_TTL_SECONDS, + max_messages: int = MAX_MESSAGES_PER_THREAD, + ): + self._conversations: Dict[str, ConversationState] = {} + self._lock = threading.Lock() + self._ttl = ttl_seconds + self._max_messages = max_messages + + def _key(self, channel_id: str, thread_ts: str) -> str: + return f"{channel_id}:{thread_ts}" + + def _evict_expired(self) -> None: + now = time.time() + expired = [ + k + for k, v in self._conversations.items() + if now - v.last_activity > self._ttl + ] + for k in expired: + del self._conversations[k] + + def _trim(self, state: ConversationState) -> None: + if len(state.messages) > self._max_messages: + state.messages = state.messages[-self._max_messages :] + + def _get_or_create(self, channel_id: str, thread_ts: str) -> ConversationState: + key = self._key(channel_id, thread_ts) + if key not in self._conversations: + self._conversations[key] = ConversationState( + channel_id=channel_id, + thread_ts=thread_ts, + ) + state = self._conversations[key] + state.last_activity = time.time() + return state + + def append_user_message(self, channel_id: str, thread_ts: str, text: str) -> None: + with self._lock: + self._evict_expired() + state = self._get_or_create(channel_id, thread_ts) + state.messages.append({"role": "user", "content": text}) + self._trim(state) + + def append_assistant_message( + self, channel_id: str, thread_ts: str, text: str + ) -> None: + with self._lock: + self._evict_expired() + state = self._get_or_create(channel_id, thread_ts) + state.messages.append({"role": "assistant", "content": text}) + self._trim(state) + + def get_messages(self, channel_id: str, thread_ts: str) -> List[dict]: + with self._lock: + key = self._key(channel_id, thread_ts) + state = self._conversations.get(key) + if state is None: + return [] + return list(state.messages) + + def clear(self, channel_id: str, thread_ts: str) -> None: + with self._lock: + key = self._key(channel_id, thread_ts) + self._conversations.pop(key, None) diff --git a/bugzooka/integrations/slack_socket_listener.py b/bugzooka/integrations/slack_socket_listener.py index 6f2786e..4a4838e 100644 --- a/bugzooka/integrations/slack_socket_listener.py +++ b/bugzooka/integrations/slack_socket_listener.py @@ -4,9 +4,9 @@ Mentions are processed asynchronously using a thread pool for concurrent handling. """ -import asyncio import concurrent.futures import logging +import re import sys import time from threading import Lock, Event @@ -28,6 +28,8 @@ analyze_performance, parse_perf_summary_args, ) +from bugzooka.analysis.general_query_analyzer import analyze_general_query +from bugzooka.core.conversation import ConversationManager from bugzooka.integrations.slack_client_base import SlackClientBase from bugzooka import telemetry @@ -70,30 +72,50 @@ def __init__(self, logger: logging.Logger, max_workers: int = 5): self.processing_lock = Lock() self.processing_messages: Set[str] = set() + # Conversation history for multi-turn interactions + self.conversation_manager = ConversationManager() + def _should_process_message(self, event: Dict[str, Any]) -> bool: """ Determine if a message should be processed. - Only process app_mention events not sent by the bot itself. + Accepts app_mention events and thread reply messages to known conversations. :param event: Slack event data :return: True if message should be processed """ - # Only process app_mention events - if event.get("type") != "app_mention": - return False + event_type = event.get("type") # Don't process messages from the bot itself if event.get("user") == JEDI_BOT_SLACK_USER_ID: self.logger.debug("Ignoring message from bot itself") return False - return True + # Always process direct @mentions + if event_type == "app_mention": + return True + + # Process thread replies to conversations we're already tracking + if event_type == "message": + thread_ts = event.get("thread_ts") + channel = event.get("channel") + if thread_ts and channel: + msgs = self.conversation_manager.get_messages(channel, thread_ts) + if msgs: + self.logger.debug("Processing thread reply to active conversation") + return True + + return False + + @staticmethod + def _clean_mention_text(text: str) -> str: + """Remove bot @mention tags from message text.""" + return re.sub(r"<@\w+>\s*", "", text).strip() def _process_mention(self, event: Dict[str, Any]) -> None: """ Process an @ mention of the bot (core processing logic). - Checks for "analyze pr: {PR link}" pattern and calls Orion MCP if found. - Otherwise sends greeting message. + Routes to specialized handlers for structured commands, + or falls back to the general agentic query handler. :param event: Slack event data """ @@ -125,9 +147,7 @@ def _process_mention(self, event: Dict[str, Any]) -> None: ) # Analyze PR from text (need to run async function in sync context) - analysis_result = anyio.run( - analyze_pr_with_gemini, text, channel - ) + analysis_result = anyio.run(analyze_pr_with_gemini, text, channel) # Split the result by "====" separator and send each part as a separate message message_content = analysis_result["message"] @@ -171,7 +191,11 @@ def _process_mention(self, event: Dict[str, Any]) -> None: pr_display = ", ".join(f"#{pr}" for pr in pr_numbers) self.logger.info( "Sent PR analysis for %s/%s %s (OpenShift %s) to %s", - org, repo, pr_display, version, user, + org, + repo, + pr_display, + version, + user, ) else: self.logger.warning( @@ -202,20 +226,22 @@ def _process_mention(self, event: Dict[str, Any]) -> None: _error_message = str(e) _error_type = type(e).__name__ finally: - telemetry.emit({ - "command": "analyze_pr", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - "pr_repo": _pr_repo, - "total_tokens": _total_tokens, - "tool_calls_count": _tool_calls_count, - }) + telemetry.emit( + { + "command": "analyze_pr", + "trigger_type": "user_initiated", + "channel_id": channel, + "user_id": user, + "success": _success, + "error_message": _error_message, + "error_type": _error_type, + "duration_ms": int((time.time() - _start) * 1000), + "retry_count": 0, + "pr_repo": _pr_repo, + "total_tokens": _total_tokens, + "tool_calls_count": _tool_calls_count, + } + ) return # Check if message contains "inspect" for nightly regression analysis @@ -234,9 +260,7 @@ def _process_mention(self, event: Dict[str, Any]) -> None: ) # Analyze nightly regression (need to run async function in sync context) - analysis_result = anyio.run( - analyze_nightly_regression, text, channel - ) + analysis_result = anyio.run(analyze_nightly_regression, text, channel) # Send the result message_content = analysis_result["message"] @@ -280,18 +304,20 @@ def _process_mention(self, event: Dict[str, Any]) -> None: _error_message = str(e) _error_type = "mcp_error" finally: - telemetry.emit({ - "command": "inspect_nightly", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - "nightly_version": _nightly_version, - }) + telemetry.emit( + { + "command": "inspect_nightly", + "trigger_type": "user_initiated", + "channel_id": channel, + "user_id": user, + "success": _success, + "error_message": _error_message, + "error_type": _error_type, + "duration_ms": int((time.time() - _start) * 1000), + "retry_count": 0, + "nightly_version": _nightly_version, + } + ) return # Check if message contains "performance summary" @@ -365,54 +391,111 @@ def _process_mention(self, event: Dict[str, Any]) -> None: _error_message = str(e) _error_type = "mcp_error" finally: - telemetry.emit({ - "command": "perf_summary", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - "configs_count": _configs_count, - "versions_count": _versions_count, - }) + telemetry.emit( + { + "command": "perf_summary", + "trigger_type": "user_initiated", + "channel_id": channel, + "user_id": user, + "success": _success, + "error_message": _error_message, + "error_type": _error_type, + "duration_ms": int((time.time() - _start) * 1000), + "retry_count": 0, + "configs_count": _configs_count, + "versions_count": _versions_count, + } + ) return - # Default: Send simple greeting message + # Default: General agentic query handler for free-form questions + thread_ts = event.get("thread_ts", ts) + clean_text = self._clean_mention_text(text) _start = time.time() _success = False _error_message = None _error_type = None + _total_tokens = 0 + _tool_calls_count = 0 try: self.client.chat_postMessage( channel=channel, - text="May the force be with you! :performance_jedi:\n\n" - ":bulb: *Tips:*\n" - "- `analyze pr: , compare with ` - PR performance analysis\n" - "- `inspect [vs ] [for config ] [for days]` - Nightly regression analysis\n" - "- `performance summary [ALL|config1.yaml,config2.yaml] [version ...]` - Performance metrics summary", - thread_ts=ts, + text=":hourglass_flowing_sand: Thinking...", + thread_ts=thread_ts, + ) + + self.conversation_manager.append_user_message( + channel, thread_ts, clean_text + ) + conversation_messages = self.conversation_manager.get_messages( + channel, thread_ts ) - self.logger.info(f"✅ Sent greeting to {user}") - _success = True + + analysis_result = anyio.run( + analyze_general_query, clean_text, conversation_messages, channel + ) + + result_text = analysis_result.get("message", "") + + if analysis_result.get("success"): + self.conversation_manager.append_assistant_message( + channel, thread_ts, result_text + ) + chunks = self.chunk_text(result_text) + for chunk in chunks: + self.client.chat_postMessage( + channel=channel, + text=chunk, + thread_ts=thread_ts, + ) + self.logger.info( + f"Sent general query response to {user} ({len(chunks)} chunk(s))" + ) + _success = True + else: + self.client.chat_postMessage( + channel=channel, + text=result_text, + thread_ts=thread_ts, + ) + self.logger.warning(f"General query failed: {result_text}") + + try: + from bugzooka.integrations.inference_client import get_inference_client + + client = get_inference_client() + _total_tokens = client.last_total_tokens + _tool_calls_count = client.last_tool_calls_count + except Exception: + pass + except Exception as e: - self.logger.error(f"Error sending message: {e}", exc_info=True) + self.logger.error(f"Error processing general query: {e}", exc_info=True) + self.client.chat_postMessage( + channel=channel, + text=f"An error occurred: {str(e)}", + thread_ts=thread_ts, + ) _error_message = str(e) - _error_type = "slack_api_error" + _error_type = type(e).__name__ finally: - telemetry.emit({ - "command": "help", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - }) + is_followup = event.get("thread_ts") is not None + telemetry.emit( + { + "command": "general_query", + "trigger_type": "user_initiated", + "channel_id": channel, + "user_id": user, + "success": _success, + "error_message": _error_message, + "error_type": _error_type, + "duration_ms": int((time.time() - _start) * 1000), + "retry_count": 0, + "total_tokens": _total_tokens, + "tool_calls_count": _tool_calls_count, + "is_followup": is_followup, + } + ) def _submit_mention_for_processing(self, event: Dict[str, Any]) -> None: """ @@ -470,10 +553,19 @@ def _process_socket_request( self.logger.debug(f"Received event type: {event_type}") - if event_type == "app_mention": - # Add eyes emoji reaction immediately for instant visual feedback + # Handle @mentions and thread replies to active conversations + is_thread_reply = ( + event_type == "message" + and event.get("thread_ts") + and event.get("user") != JEDI_BOT_SLACK_USER_ID + and not event.get("bot_id") + ) + + if event_type == "app_mention" or is_thread_reply: ts = event.get("ts") channel = event.get("channel") + + # Add eyes emoji reaction for instant visual feedback try: self.client.reactions_add( name="eyes", diff --git a/mcp_config.json b/mcp_config.json index 8956073..856b235 100644 --- a/mcp_config.json +++ b/mcp_config.json @@ -1,7 +1,7 @@ { "mcp_servers": { "orion_mcp_server": { - "url": "http://localhost:3030/mcp", + "url": "http://127.0.0.1:3030/mcp", "transport": "streamable_http" } } diff --git a/requirements.txt b/requirements.txt index 7bae9e7..5d3ecea 100644 --- a/requirements.txt +++ b/requirements.txt @@ -21,6 +21,7 @@ rh-py-commons>=0.5.0 # LangChain ecosystem (MCP + tools) langchain-core>=1.3.0 langchain-mcp-adapters>=0.3.0 +mcp>=1.18.0 # ML / Embeddings faiss-cpu==1.12.0 diff --git a/tests/test_conversation.py b/tests/test_conversation.py new file mode 100644 index 0000000..e2e3ad6 --- /dev/null +++ b/tests/test_conversation.py @@ -0,0 +1,87 @@ +"""Unit tests for ConversationManager.""" + +import time +import threading +from bugzooka.core.conversation import ConversationManager + + +class TestConversationManager: + def test_new_conversation_created(self): + mgr = ConversationManager() + mgr.append_user_message("C123", "ts1", "hello") + msgs = mgr.get_messages("C123", "ts1") + assert len(msgs) == 1 + assert msgs[0] == {"role": "user", "content": "hello"} + + def test_append_user_and_assistant(self): + mgr = ConversationManager() + mgr.append_user_message("C123", "ts1", "question") + mgr.append_assistant_message("C123", "ts1", "answer") + msgs = mgr.get_messages("C123", "ts1") + assert len(msgs) == 2 + assert msgs[0]["role"] == "user" + assert msgs[1]["role"] == "assistant" + + def test_separate_threads_have_separate_history(self): + mgr = ConversationManager() + mgr.append_user_message("C123", "ts1", "thread 1") + mgr.append_user_message("C123", "ts2", "thread 2") + msgs1 = mgr.get_messages("C123", "ts1") + msgs2 = mgr.get_messages("C123", "ts2") + assert len(msgs1) == 1 + assert len(msgs2) == 1 + assert msgs1[0]["content"] == "thread 1" + assert msgs2[0]["content"] == "thread 2" + + def test_get_messages_returns_copy(self): + mgr = ConversationManager() + mgr.append_user_message("C123", "ts1", "hello") + msgs = mgr.get_messages("C123", "ts1") + msgs.append({"role": "user", "content": "injected"}) + assert len(mgr.get_messages("C123", "ts1")) == 1 + + def test_get_messages_empty_for_unknown_thread(self): + mgr = ConversationManager() + assert mgr.get_messages("C123", "unknown") == [] + + def test_max_messages_trimmed(self): + mgr = ConversationManager(max_messages=4) + for i in range(6): + mgr.append_user_message("C123", "ts1", f"msg {i}") + msgs = mgr.get_messages("C123", "ts1") + assert len(msgs) == 4 + assert msgs[0]["content"] == "msg 2" + assert msgs[-1]["content"] == "msg 5" + + def test_ttl_expiry(self): + mgr = ConversationManager(ttl_seconds=0) + mgr.append_user_message("C123", "ts1", "old message") + time.sleep(0.01) + mgr.append_user_message("C123", "ts2", "new message") + assert mgr.get_messages("C123", "ts1") == [] + assert len(mgr.get_messages("C123", "ts2")) == 1 + + def test_clear(self): + mgr = ConversationManager() + mgr.append_user_message("C123", "ts1", "hello") + mgr.clear("C123", "ts1") + assert mgr.get_messages("C123", "ts1") == [] + + def test_thread_safety(self): + mgr = ConversationManager() + errors = [] + + def writer(thread_id): + try: + for i in range(50): + mgr.append_user_message("C123", f"ts{thread_id}", f"msg {i}") + except Exception as e: + errors.append(e) + + threads = [threading.Thread(target=writer, args=(i,)) for i in range(10)] + for t in threads: + t.start() + for t in threads: + t.join() + + assert len(errors) == 0 diff --git a/tests/test_general_query_analyzer.py b/tests/test_general_query_analyzer.py new file mode 100644 index 0000000..97d7adf --- /dev/null +++ b/tests/test_general_query_analyzer.py @@ -0,0 +1,156 @@ +"""Unit tests for general_query_analyzer.""" + +import sys +from types import ModuleType +from unittest.mock import AsyncMock, patch, MagicMock + +import anyio + +# Mock the commons module before any bugzooka imports +_mock_commons = ModuleType("commons") +_mock_inference = ModuleType("commons.inference") +_mock_inference.InferenceClient = MagicMock +_mock_inference.get_inference_client = MagicMock() +_mock_inference.analyze_with_agentic = AsyncMock() +_mock_inference.InferenceClientError = Exception +_mock_inference.InferenceAPIError = Exception +_mock_inference.InferenceIterationLimitError = Exception +_mock_inference.INFERENCE_MAX_TOKENS = 4096 +_mock_inference.INFERENCE_TEMPERATURE = 0.7 +_mock_inference.INFERENCE_MAX_TOOL_ITERATIONS = 10 +_mock_inference.INFERENCE_API_TIMEOUT = 60 +_mock_commons.inference = _mock_inference +sys.modules.setdefault("commons", _mock_commons) +sys.modules.setdefault("commons.inference", _mock_inference) + +from bugzooka.analysis.general_query_analyzer import analyze_general_query # noqa: E402 + + +def test_analyze_general_query_success(): + conversation = [{"role": "user", "content": "how is cudn doing on 5.0?"}] + + async def _run(): + with patch( + "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", + new_callable=AsyncMock, + ), patch( + "bugzooka.analysis.general_query_analyzer.mcp_module" + ) as mock_mcp, patch( + "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", + new_callable=AsyncMock, + ) as mock_agentic: + mock_mcp.mcp_tools = [MagicMock()] + mock_agentic.return_value = "Performance looks stable." + + result = await analyze_general_query( + "how is cudn doing on 5.0?", conversation + ) + + assert result["success"] is True + assert "stable" in result["message"] + mock_agentic.assert_called_once() + + call_args = mock_agentic.call_args + messages = call_args.kwargs.get("messages") or call_args[0][0] + assert messages[0]["role"] == "system" + assert messages[1]["role"] == "user" + assert messages[1]["content"] == "how is cudn doing on 5.0?" + + anyio.run(_run) + + +def test_analyze_with_conversation_history(): + conversation = [ + {"role": "user", "content": "report on cudn for 5.0"}, + {"role": "assistant", "content": "Here are the metrics..."}, + {"role": "user", "content": "what about ovn-controller CPU?"}, + ] + + async def _run(): + with patch( + "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", + new_callable=AsyncMock, + ), patch( + "bugzooka.analysis.general_query_analyzer.mcp_module" + ) as mock_mcp, patch( + "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", + new_callable=AsyncMock, + ) as mock_agentic: + mock_mcp.mcp_tools = [MagicMock()] + mock_agentic.return_value = "ovn-controller CPU is at 9.0" + + result = await analyze_general_query( + "what about ovn-controller CPU?", conversation + ) + assert result["success"] is True + + call_args = mock_agentic.call_args + messages = call_args.kwargs.get("messages") or call_args[0][0] + assert len(messages) == 4 # system + 3 conversation messages + assert messages[1]["content"] == "report on cudn for 5.0" + assert messages[2]["role"] == "assistant" + assert messages[3]["content"] == "what about ovn-controller CPU?" + + anyio.run(_run) + + +def test_analyze_no_mcp_tools(): + async def _run(): + with patch( + "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", + new_callable=AsyncMock, + ), patch("bugzooka.analysis.general_query_analyzer.mcp_module") as mock_mcp: + mock_mcp.mcp_tools = [] + + result = await analyze_general_query( + "hello", [{"role": "user", "content": "hello"}] + ) + assert result["success"] is False + assert "MCP" in result["message"] + + anyio.run(_run) + + +def test_analyze_empty_llm_result(): + async def _run(): + with patch( + "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", + new_callable=AsyncMock, + ), patch( + "bugzooka.analysis.general_query_analyzer.mcp_module" + ) as mock_mcp, patch( + "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", + new_callable=AsyncMock, + ) as mock_agentic: + mock_mcp.mcp_tools = [MagicMock()] + mock_agentic.return_value = "" + + result = await analyze_general_query( + "test", [{"role": "user", "content": "test"}] + ) + assert result["success"] is False + + anyio.run(_run) + + +def test_analyze_exception_handling(): + async def _run(): + with patch( + "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", + new_callable=AsyncMock, + ), patch( + "bugzooka.analysis.general_query_analyzer.mcp_module" + ) as mock_mcp, patch( + "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", + new_callable=AsyncMock, + ) as mock_agentic: + mock_mcp.mcp_tools = [MagicMock()] + mock_agentic.side_effect = RuntimeError("LLM timeout") + + result = await analyze_general_query( + "test", [{"role": "user", "content": "test"}] + ) + assert result["success"] is False + assert "LLM timeout" in result["message"] + + anyio.run(_run) From 69f61935bb50b9ab0b4e2f49d3b7e44730b77ec9 Mon Sep 17 00:00:00 2001 From: Mohit Sheth Date: Tue, 30 Jun 2026 22:24:04 -0400 Subject: [PATCH 2/7] Fix pre-existing mypy type errors across codebase - Use str | None for optional channel_id params (PEP 484 no implicit Optional) - Fix ContextVar type annotation in mcp_interceptors to accept None default - Change _config type from Optional[dict] to dict in telemetry_client - Add type annotation for batch variable in telemetry drain - Add default values for event dict lookups in slack_socket_listener Assisted-by: Claude Signed-off-by: Mohit Sheth --- .../analysis/nightly_regression_analyzer.py | 7 ++-- bugzooka/analysis/perf_summary_analyzer.py | 3 +- bugzooka/analysis/pr_analyzer.py | 15 +++++--- bugzooka/integrations/mcp_interceptors.py | 3 +- .../integrations/slack_socket_listener.py | 35 ++++++++++++++++--- bugzooka/telemetry/telemetry_client.py | 13 ++++--- 6 files changed, 55 insertions(+), 21 deletions(-) diff --git a/bugzooka/analysis/nightly_regression_analyzer.py b/bugzooka/analysis/nightly_regression_analyzer.py index 73651f2..b7593c0 100644 --- a/bugzooka/analysis/nightly_regression_analyzer.py +++ b/bugzooka/analysis/nightly_regression_analyzer.py @@ -89,7 +89,7 @@ def _parse_nightly_inspect_request(text: str) -> Optional[NightlyInspectRequest] ) -async def analyze_nightly_regression(text: str, channel_id: str = None) -> dict: +async def analyze_nightly_regression(text: str, channel_id: str | None = None) -> dict: """ Parse nightly inspection request and call has_nightly_regressed MCP tool directly. @@ -108,8 +108,11 @@ async def analyze_nightly_regression(text: str, channel_id: str = None) -> dict: # Set channel context for ES encryption interceptor if channel_id: from bugzooka.integrations.mcp_interceptors import current_channel + current_channel.set(channel_id) - logger.debug("Set channel context for nightly regression analysis: %s", channel_id) + logger.debug( + "Set channel context for nightly regression analysis: %s", channel_id + ) # Parse nightly inspect request from text parsed = _parse_nightly_inspect_request(text) diff --git a/bugzooka/analysis/perf_summary_analyzer.py b/bugzooka/analysis/perf_summary_analyzer.py index 42e354b..94b6c15 100644 --- a/bugzooka/analysis/perf_summary_analyzer.py +++ b/bugzooka/analysis/perf_summary_analyzer.py @@ -669,7 +669,7 @@ async def analyze_performance( versions: Optional[List[str]] = None, lookback_days: Optional[int] = None, use_all_configs: bool = False, - channel_id: str = None, + channel_id: str | None = None, ) -> dict: """ Analyze performance metrics for the specified config and version. @@ -687,6 +687,7 @@ async def analyze_performance( # Set channel context for ES encryption interceptor if channel_id: from bugzooka.integrations.mcp_interceptors import current_channel + current_channel.set(channel_id) logger.debug("Set channel context for performance summary: %s", channel_id) diff --git a/bugzooka/analysis/pr_analyzer.py b/bugzooka/analysis/pr_analyzer.py index 79d647c..d9536ce 100644 --- a/bugzooka/analysis/pr_analyzer.py +++ b/bugzooka/analysis/pr_analyzer.py @@ -86,7 +86,7 @@ def _parse_pr_request(text: str) -> Optional[Tuple[str, str, list, str]]: return org, repo, pr_numbers, version -async def analyze_pr_with_gemini(text: str, channel_id: str = None) -> dict: +async def analyze_pr_with_gemini(text: str, channel_id: str | None = None) -> dict: """ Parse PR analysis request and analyze PR performance using Gemini with MCP. @@ -102,6 +102,7 @@ async def analyze_pr_with_gemini(text: str, channel_id: str = None) -> dict: # Set channel context for ES encryption interceptor if channel_id: from bugzooka.integrations.mcp_interceptors import current_channel + current_channel.set(channel_id) logger.debug("Set channel context for PR analysis: %s", channel_id) @@ -125,7 +126,10 @@ async def analyze_pr_with_gemini(text: str, channel_id: str = None) -> dict: pr_display = ", ".join(f"#{pr}" for pr in pr_numbers) logger.info( "PR analysis requested for %s/%s %s (OpenShift %s)", - org, repo, pr_display, version, + org, + repo, + pr_display, + version, ) # Ensure MCP client is initialized @@ -153,8 +157,11 @@ async def analyze_pr_with_gemini(text: str, channel_id: str = None) -> dict: system_prompt = PR_PERFORMANCE_ANALYSIS_PROMPT["system"] user_prompt = PR_PERFORMANCE_ANALYSIS_PROMPT["user"].format( - org=org, repo=repo, pr_numbers=pr_numbers_str, - pr_urls=pr_urls, version=version, + org=org, + repo=repo, + pr_numbers=pr_numbers_str, + pr_urls=pr_urls, + version=version, ) assistant_prompt = PR_PERFORMANCE_ANALYSIS_PROMPT["assistant"] diff --git a/bugzooka/integrations/mcp_interceptors.py b/bugzooka/integrations/mcp_interceptors.py index e3b6d73..d4967b6 100644 --- a/bugzooka/integrations/mcp_interceptors.py +++ b/bugzooka/integrations/mcp_interceptors.py @@ -14,7 +14,6 @@ from typing import Callable, Awaitable from langchain_mcp_adapters.interceptors import ( - ToolCallInterceptor, MCPToolCallRequest, MCPToolCallResult, ) @@ -26,7 +25,7 @@ # Context variable to track current Slack channel # This is set by analyzers before calling MCP tools and read by interceptor -current_channel: ContextVar[str] = ContextVar('current_channel', default=None) +current_channel: ContextVar[str | None] = ContextVar("current_channel", default=None) class HeaderEncryptionInterceptor: diff --git a/bugzooka/integrations/slack_socket_listener.py b/bugzooka/integrations/slack_socket_listener.py index 4a4838e..ab978e6 100644 --- a/bugzooka/integrations/slack_socket_listener.py +++ b/bugzooka/integrations/slack_socket_listener.py @@ -31,6 +31,11 @@ from bugzooka.analysis.general_query_analyzer import analyze_general_query from bugzooka.core.conversation import ConversationManager from bugzooka.integrations.slack_client_base import SlackClientBase +from bugzooka.integrations.image_collector import ( + ImageCollector, + set_collector, + reset_collector, +) from bugzooka import telemetry @@ -122,10 +127,10 @@ def _process_mention(self, event: Dict[str, Any]) -> None: if not self._should_process_message(event): return - user = event.get("user", "Unknown") - ts = event.get("ts") - channel = event.get("channel") - text = event.get("text", "") + user: str = event.get("user", "Unknown") + ts: str = event.get("ts", "") + channel: str = event.get("channel", "") + text: str = event.get("text", "") self.logger.info(f"📩 Processing mention from {user} at ts {ts}") @@ -417,6 +422,10 @@ def _process_mention(self, event: Dict[str, Any]) -> None: _error_type = None _total_tokens = 0 _tool_calls_count = 0 + collector = ImageCollector() + # Must set collector inside the worker thread — ThreadPoolExecutor + # does not propagate the calling thread's contextvars. + token = set_collector(collector) try: self.client.chat_postMessage( channel=channel, @@ -432,7 +441,7 @@ def _process_mention(self, event: Dict[str, Any]) -> None: ) analysis_result = anyio.run( - analyze_general_query, clean_text, conversation_messages, channel + analyze_general_query, conversation_messages, channel ) result_text = analysis_result.get("message", "") @@ -448,6 +457,20 @@ def _process_mention(self, event: Dict[str, Any]) -> None: text=chunk, thread_ts=thread_ts, ) + + if collector.has_images(): + for img in collector.get_images(): + try: + self.client.files_upload_v2( + channel=channel, + content=collector.decode_image(img), + filename=img["filename"], + title=f"Chart from {img['tool_name']}", + thread_ts=thread_ts, + ) + except Exception as upload_err: + self.logger.error("Failed to upload image: %s", upload_err) + self.logger.info( f"Sent general query response to {user} ({len(chunks)} chunk(s))" ) @@ -479,6 +502,8 @@ def _process_mention(self, event: Dict[str, Any]) -> None: _error_message = str(e) _error_type = type(e).__name__ finally: + collector.clear() + reset_collector(token) is_followup = event.get("thread_ts") is not None telemetry.emit( { diff --git a/bugzooka/telemetry/telemetry_client.py b/bugzooka/telemetry/telemetry_client.py index 41f22b9..39fa569 100644 --- a/bugzooka/telemetry/telemetry_client.py +++ b/bugzooka/telemetry/telemetry_client.py @@ -8,7 +8,6 @@ import logging import queue import threading -import time from datetime import datetime, timezone from typing import Optional @@ -17,7 +16,7 @@ _queue: queue.Queue = queue.Queue(maxsize=1000) _thread: Optional[threading.Thread] = None _es_client = None -_config: Optional[dict] = None +_config: dict = {} _channel_team_mappings: dict = {} _shutdown_event = threading.Event() _started = False @@ -90,9 +89,7 @@ def emit(event: dict) -> None: _queue.qsize(), ) except queue.Full: - logger.warning( - "Telemetry queue full, dropping event: %s", event.get("command") - ) + logger.warning("Telemetry queue full, dropping event: %s", event.get("command")) except Exception as e: logger.warning("Telemetry emit error: %s", e) @@ -131,7 +128,7 @@ def _flush_thread() -> None: def _drain_and_flush(batch_size: int) -> None: """Drain up to batch_size events from queue and write to ES.""" - batch = [] + batch: list[dict] = [] while len(batch) < batch_size: try: event = _queue.get_nowait() @@ -164,7 +161,9 @@ def _bulk_write(events: list) -> None: else: logger.debug("Telemetry flushed %d events to %s", success, index_name) except Exception as e: - logger.error("Telemetry bulk write failed: %s. Dropping %d events.", e, len(events)) + logger.error( + "Telemetry bulk write failed: %s. Dropping %d events.", e, len(events) + ) def _ensure_index_template() -> None: From ff14134248ae5c809219e5fa6d76dd18e5a17916 Mon Sep 17 00:00:00 2001 From: Mohit Sheth Date: Tue, 30 Jun 2026 22:24:49 -0400 Subject: [PATCH 3/7] Add chart image upload support and code quality improvements Route MCP tool execution through invoke_mcp_tool in the general query analyzer so ImageContent blocks from Orion tools (openshift_report_on, metrics_correlation) are intercepted and uploaded to Slack threads as chart images via files_upload_v2. New components: - ImageCollector: per-request image store using contextvars, collects base64 images from MCP tool results for later Slack upload - Custom tool executor in general_query_analyzer that drives chat_with_tools_async directly instead of analyze_with_agentic, enabling image interception Code quality fixes from review: - Use ContextVar.reset(token) instead of set(None) for proper cleanup - Remove dead text parameter from analyze_general_query - Handle image-only tool results without collector (prevent base64 dump) - Fix MIME extension detection with lookup dict - Modernize typing imports in conversation.py - Update tests to match new executor architecture, add tool routing tests - Update README with free-form queries and multi-turn conversation docs Assisted-by: Claude Signed-off-by: Mohit Sheth --- README.md | 18 ++ bugzooka/analysis/general_query_analyzer.py | 35 ++-- bugzooka/analysis/prompts.py | 13 +- bugzooka/core/conversation.py | 7 +- bugzooka/integrations/image_collector.py | 69 +++++++ bugzooka/integrations/mcp_client.py | 33 +++- tests/test_general_query_analyzer.py | 189 ++++++++++++++------ 7 files changed, 286 insertions(+), 78 deletions(-) create mode 100644 bugzooka/integrations/image_collector.py diff --git a/README.md b/README.md index d26cc29..9a4f154 100644 --- a/README.md +++ b/README.md @@ -144,6 +144,19 @@ BugZooka can generate a configurable performance summary across metrics for one * `@PerfScale Jedi inspect 4.22.0-0.nightly-2026-01-05-203335 vs 4.22.0-0.nightly-2026-01-01-123456` - Calls `has_nightly_regressed` tool in [orion-mcp](https://github.com/cloud-bulldozer/orion-mcp) and campares two given nightlies. * `@PerfScale Jedi inspect 4.22.0-0.nightly-2026-01-05-203335 for config trt-external-payload-node-density.yaml` - Calls `has_nightly_regressed` tool in [orion-mcp](https://github.com/cloud-bulldozer/orion-mcp) for a given nightly checks regression only for a given orion configuration file instead of the [default](https://github.com/cloud-bulldozer/orion-mcp/blob/main/orion_mcp.py#L470). +#### **Free-Form Queries (Agentic)** +The bot handles any natural-language question by passing it to the LLM with all MCP tools available. The LLM autonomously picks which Orion tools to call. + +``` +@PerfScale Jedi how is cudn-density doing on 5.0? +@PerfScale Jedi show me podReadyLatency_P99 for node-density-cni on 4.22 +``` + +Responses include numeric summaries and chart images uploaded directly to the Slack thread. + +#### **Multi-Turn Conversations** +Thread replies to an existing bot conversation are automatically processed without requiring another @mention. Conversation history is kept in-memory per thread with a configurable TTL (default: 2 hours, `CONVERSATION_TTL_SECONDS`). + **Note**: All the triggers that start with a bot mention (.i.e. `@PerfScale Jedi`) run in socket mode. All socket mode features can be used in any slack channel without needing to host your own on premise openshift deployment. ## **Configurables** @@ -375,9 +388,11 @@ BugZooka/ │ │ ├── __init__.py │ │ ├── config.py # Configuration management │ │ ├── constants.py # Application constants +│ │ ├── conversation.py # Per-thread conversation history manager │ │ └── utils.py # Shared utility functions │ ├── integrations/ # External service integrations │ │ ├── __init__.py +│ │ ├── image_collector.py # Collects images from MCP tool results for Slack upload │ │ ├── inference_client.py # Inference client re-exports (from py-commons) │ │ ├── mcp_client.py # MCP protocol client implementation │ │ ├── rag_client_util.py # RAG vector store utilities @@ -389,6 +404,7 @@ BugZooka/ │ ├── failure_keywords.py # Failure pattern detection │ ├── log_analyzer.py # Main log analysis orchestration │ ├── log_summarizer.py # Log summarization functionality +│ ├── general_query_analyzer.py # Agentic handler for free-form queries │ ├── pr_analyzer.py # PR performance analysis with Gemini+MCP │ ├── prompts.py # AI prompts and templates │ ├── prow_analyzer.py # Prow-specific CI/CD analysis @@ -414,6 +430,8 @@ BugZooka/ │ ├── __init__.py │ ├── conftest.py # Pytest configuration │ ├── helpers.py # Test utilities +│ ├── test_conversation.py # Conversation manager tests +│ ├── test_general_query_analyzer.py # General query analyzer tests │ ├── test_slack_fetcher.py # Slack fetcher tests │ └── test_slack_socket_listener.py # Socket mode tests ├── Dockerfile # Container image definition diff --git a/bugzooka/analysis/general_query_analyzer.py b/bugzooka/analysis/general_query_analyzer.py index 154d204..0819756 100644 --- a/bugzooka/analysis/general_query_analyzer.py +++ b/bugzooka/analysis/general_query_analyzer.py @@ -1,15 +1,20 @@ """General-purpose agentic query handler for free-form natural language questions. -Uses the same agentic LLM loop as pr_analyzer.py, but with a flexible prompt -that lets the LLM autonomously select which MCP tools to call based on the -user's natural language query. Supports multi-turn conversation via message history. +Unlike pr_analyzer.py which uses analyze_with_agentic from commons, this module +drives chat_with_tools_async directly so tool calls route through invoke_mcp_tool. +This lets the ImageCollector intercept image content blocks from MCP tools. """ import logging -from bugzooka.integrations.mcp_client import initialize_global_resources_async +from langchain_core.utils.function_calling import convert_to_openai_tool + +from bugzooka.integrations.mcp_client import ( + initialize_global_resources_async, + invoke_mcp_tool, +) from bugzooka.core.utils import make_response -from bugzooka.integrations.inference_client import analyze_with_agentic +from bugzooka.integrations.inference_client import get_inference_client from bugzooka.analysis.prompts import GENERAL_QUERY_PROMPT import bugzooka.integrations.mcp_client as mcp_module @@ -17,17 +22,12 @@ async def analyze_general_query( - text: str, conversation_messages: list[dict], channel_id: str | None = None, ) -> dict: """ Handle a free-form natural language query using the agentic LLM loop. - The LLM receives the full conversation history and all available MCP tools, - then autonomously decides which tools to call to answer the user's question. - - :param text: The current user message (already included in conversation_messages) :param conversation_messages: Full conversation history in OpenAI message format :param channel_id: Slack channel ID for ES_SERVER routing (optional) :return: Dictionary with 'success' (bool) and 'message' (str) @@ -52,9 +52,20 @@ async def analyze_general_query( messages.extend(conversation_messages) try: - result = await analyze_with_agentic( + client = get_inference_client() + tools_by_name = {tool.name: tool for tool in mcp_module.mcp_tools} + openai_tools = [convert_to_openai_tool(t) for t in mcp_module.mcp_tools] + + async def execute_tool(tool_name, tool_args): + tool = tools_by_name.get(tool_name) + if not tool: + return f"Error: Tool '{tool_name}' not found" + return await invoke_mcp_tool(tool, tool_args) + + result = await client.chat_with_tools_async( messages=messages, - tools=mcp_module.mcp_tools, + tools=openai_tools, + execute_tool_func=execute_tool, ) if not result: diff --git a/bugzooka/analysis/prompts.py b/bugzooka/analysis/prompts.py index 9be6ffc..3159cff 100644 --- a/bugzooka/analysis/prompts.py +++ b/bugzooka/analysis/prompts.py @@ -147,18 +147,19 @@ You have access to Orion MCP tools that can: - List available test configs: `get_orion_configs` - List metrics for a config: `get_orion_metrics`, `get_orion_metrics_with_meta` -- Get raw performance data values: `get_orion_performance_data` — USE THIS for reporting on metrics. It returns actual numeric values you can analyze and present. +- Get raw performance data values: `get_orion_performance_data` — returns actual numeric values you can compute stats on (min, max, avg, trend). +- Generate visual charts: `openshift_report_on` — generates chart images that are automatically uploaded to Slack. Use with `options="image"` for charts, `options="json"` for raw data, or `options="both"` for both. - Check for regressions across all configs: `has_openshift_regressed` - Check networking-specific regressions: `has_networking_regressed` - Detect nightly build regressions: `has_nightly_regressed` - Analyze PR performance impact: `openshift_report_on_pr` -- Correlate two metrics: `metrics_correlation` +- Correlate two metrics: `metrics_correlation` — generates a scatter plot image that is automatically uploaded to Slack. - Get OpenShift release dates: `get_release_date` IMPORTANT tool guidance: -- For performance reports and metric analysis, ALWAYS use `get_orion_performance_data` to fetch raw numeric data. This returns a list of values you can compute stats on (min, max, avg, trend). -- Do NOT use `openshift_report_on` — it returns images that cannot be displayed in Slack. -- When the user asks about "how is X doing", fetch the key metrics using `get_orion_performance_data` for each metric, then summarize the results with actual numbers. +- ALWAYS call `openshift_report_on` alongside `get_orion_performance_data` when reporting on any metric. The chart image is automatically uploaded to the Slack thread — you do not need to embed it in your text response. +- Use `get_orion_performance_data` for the numeric summary (min, max, avg, trend). +- Use `openshift_report_on` with `options="image"` to generate the visual chart. Call both tools in parallel when possible. - To find available metrics for a config, call `get_orion_metrics` first. General instructions: @@ -169,7 +170,7 @@ - Format responses for Slack: use *bold* for headers, `code` for metric names, and code blocks for tables. - Be concise and data-driven. When showing metrics, include actual values, averages, and trends. - Highlight any regressions or notable changes in recent data points. -- If tools return no data or errors, explain what happened clearly. +- If a tool returns a result like "No data found for metric X", that is a SUCCESSFUL tool call — the service is working, but no data exists for that query. Tell the user the specific metric or config was not found and suggest alternatives (e.g., call `get_orion_metrics` to list available metrics). NEVER say you are having trouble communicating with the service when the tool actually returned a response. - You are in a multi-turn conversation. Use prior context to understand follow-up questions. """, } diff --git a/bugzooka/core/conversation.py b/bugzooka/core/conversation.py index 5b42e4e..8b4b7e1 100644 --- a/bugzooka/core/conversation.py +++ b/bugzooka/core/conversation.py @@ -4,7 +4,6 @@ import threading import time from dataclasses import dataclass, field -from typing import Dict, List CONVERSATION_TTL_SECONDS = int(os.getenv("CONVERSATION_TTL_SECONDS", "7200")) MAX_MESSAGES_PER_THREAD = int(os.getenv("MAX_MESSAGES_PER_THREAD", "20")) @@ -14,7 +13,7 @@ class ConversationState: channel_id: str thread_ts: str - messages: List[dict] = field(default_factory=list) + messages: list[dict] = field(default_factory=list) last_activity: float = field(default_factory=time.time) @@ -26,7 +25,7 @@ def __init__( ttl_seconds: int = CONVERSATION_TTL_SECONDS, max_messages: int = MAX_MESSAGES_PER_THREAD, ): - self._conversations: Dict[str, ConversationState] = {} + self._conversations: dict[str, ConversationState] = {} self._lock = threading.Lock() self._ttl = ttl_seconds self._max_messages = max_messages @@ -75,7 +74,7 @@ def append_assistant_message( state.messages.append({"role": "assistant", "content": text}) self._trim(state) - def get_messages(self, channel_id: str, thread_ts: str) -> List[dict]: + def get_messages(self, channel_id: str, thread_ts: str) -> list[dict]: with self._lock: key = self._key(channel_id, thread_ts) state = self._conversations.get(key) diff --git a/bugzooka/integrations/image_collector.py b/bugzooka/integrations/image_collector.py new file mode 100644 index 0000000..91602e2 --- /dev/null +++ b/bugzooka/integrations/image_collector.py @@ -0,0 +1,69 @@ +"""Per-request collector for images returned by MCP tool calls. + +Uses a contextvars.ContextVar so the collector is implicitly available +to invoke_mcp_tool() without threading it through every function signature. +The Slack handler sets/clears the collector around each request. +""" + +import base64 +import contextvars +import logging +from typing import Optional + +logger = logging.getLogger(__name__) + +_current_collector: contextvars.ContextVar[ + Optional["ImageCollector"] +] = contextvars.ContextVar("_current_collector", default=None) + + +def get_collector() -> Optional["ImageCollector"]: + return _current_collector.get() + + +def set_collector(collector: Optional["ImageCollector"]) -> contextvars.Token: + return _current_collector.set(collector) + + +def reset_collector(token: contextvars.Token) -> None: + _current_collector.reset(token) + + +_MIME_EXTENSIONS = { + "image/png": "png", + "image/jpeg": "jpg", + "image/gif": "gif", + "image/webp": "webp", + "image/svg+xml": "svg", +} + + +class ImageCollector: + """Collects base64 images from MCP tool results during a single request.""" + + def __init__(self): + self._images: list[dict] = [] + + def add_image(self, base64_data: str, mime_type: str, tool_name: str) -> None: + ext = _MIME_EXTENSIONS.get(mime_type, "png") + self._images.append( + { + "base64_data": base64_data, + "mime_type": mime_type, + "tool_name": tool_name, + "filename": f"{tool_name}_{len(self._images)}.{ext}", + } + ) + logger.info("Collected image from tool %s (%s)", tool_name, mime_type) + + def get_images(self) -> list[dict]: + return list(self._images) + + def has_images(self) -> bool: + return len(self._images) > 0 + + def decode_image(self, image: dict) -> bytes: + return base64.b64decode(image["base64_data"]) + + def clear(self) -> None: + self._images.clear() diff --git a/bugzooka/integrations/mcp_client.py b/bugzooka/integrations/mcp_client.py index 228b851..fa67136 100644 --- a/bugzooka/integrations/mcp_client.py +++ b/bugzooka/integrations/mcp_client.py @@ -110,10 +110,16 @@ async def invoke_mcp_tool(tool: Any, args: dict) -> str: """ Invoke an MCP tool with the given arguments. + If the result contains image content blocks (from MCP ImageContent), + they are collected via the ImageCollector context var for later upload + to Slack. Only text content is returned to the LLM. + :param tool: The MCP tool object :param args: Arguments to pass to the tool :return: Tool result as string """ + from bugzooka.integrations.image_collector import get_collector + if hasattr(tool, "ainvoke"): result = await tool.ainvoke(args) else: @@ -124,13 +130,30 @@ async def invoke_mcp_tool(tool: Any, args: dict) -> str: if isinstance(result, str): return result elif isinstance(result, list) and len(result) > 0: - # List of message dicts: [{'type': 'text', 'text': '...', 'id': '...'}] - if isinstance(result[0], dict) and 'text' in result[0]: - return result[0]['text'] + if isinstance(result[0], dict): + text_parts = [] + has_images = False + collector = get_collector() + tool_name = getattr(tool, "name", "unknown_tool") + for block in result: + if block.get("type") == "text": + text_parts.append(block["text"]) + elif block.get("type") == "image": + has_images = True + if collector: + b64 = block.get("base64", "") + mime = block.get("mime_type", "image/png") + collector.add_image(b64, mime, tool_name) + + if text_parts: + return "\n".join(text_parts) + if has_images: + return "[Chart image generated successfully]" + return str(result) # List of ToolMessage objects - elif hasattr(result[0], 'content'): + elif hasattr(result[0], "content"): return result[0].content - elif hasattr(result, 'content'): + elif hasattr(result, "content"): # ToolMessage or similar message object return result.content diff --git a/tests/test_general_query_analyzer.py b/tests/test_general_query_analyzer.py index 97d7adf..3ed37b4 100644 --- a/tests/test_general_query_analyzer.py +++ b/tests/test_general_query_analyzer.py @@ -9,49 +9,63 @@ # Mock the commons module before any bugzooka imports _mock_commons = ModuleType("commons") _mock_inference = ModuleType("commons.inference") -_mock_inference.InferenceClient = MagicMock -_mock_inference.get_inference_client = MagicMock() -_mock_inference.analyze_with_agentic = AsyncMock() -_mock_inference.InferenceClientError = Exception -_mock_inference.InferenceAPIError = Exception -_mock_inference.InferenceIterationLimitError = Exception -_mock_inference.INFERENCE_MAX_TOKENS = 4096 -_mock_inference.INFERENCE_TEMPERATURE = 0.7 -_mock_inference.INFERENCE_MAX_TOOL_ITERATIONS = 10 -_mock_inference.INFERENCE_API_TIMEOUT = 60 -_mock_commons.inference = _mock_inference +_mock_inference.InferenceClient = MagicMock # type: ignore[attr-defined] +_mock_inference.get_inference_client = MagicMock() # type: ignore[attr-defined] +_mock_inference.analyze_with_agentic = AsyncMock() # type: ignore[attr-defined] +_mock_inference.InferenceClientError = Exception # type: ignore[attr-defined] +_mock_inference.InferenceAPIError = Exception # type: ignore[attr-defined] +_mock_inference.InferenceIterationLimitError = Exception # type: ignore[attr-defined] +_mock_inference.INFERENCE_MAX_TOKENS = 4096 # type: ignore[attr-defined] +_mock_inference.INFERENCE_TEMPERATURE = 0.7 # type: ignore[attr-defined] +_mock_inference.INFERENCE_MAX_TOOL_ITERATIONS = 10 # type: ignore[attr-defined] +_mock_inference.INFERENCE_API_TIMEOUT = 60 # type: ignore[attr-defined] +_mock_commons.inference = _mock_inference # type: ignore[attr-defined] sys.modules.setdefault("commons", _mock_commons) sys.modules.setdefault("commons.inference", _mock_inference) from bugzooka.analysis.general_query_analyzer import analyze_general_query # noqa: E402 +def _make_mock_client(return_value="Performance looks stable."): + mock_client = MagicMock() + mock_client.chat_with_tools_async = AsyncMock(return_value=return_value) + return mock_client + + +def _patch_convert_to_openai_tool(): + return patch( + "bugzooka.analysis.general_query_analyzer.convert_to_openai_tool", + side_effect=lambda t: {"type": "function", "function": {"name": t.name}}, + ) + + def test_analyze_general_query_success(): conversation = [{"role": "user", "content": "how is cudn doing on 5.0?"}] async def _run(): + mock_client = _make_mock_client("Performance looks stable.") + mock_tool = MagicMock() + mock_tool.name = "get_orion_performance_data" + with patch( "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", new_callable=AsyncMock, ), patch( "bugzooka.analysis.general_query_analyzer.mcp_module" ) as mock_mcp, patch( - "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", - new_callable=AsyncMock, - ) as mock_agentic: - mock_mcp.mcp_tools = [MagicMock()] - mock_agentic.return_value = "Performance looks stable." + "bugzooka.analysis.general_query_analyzer.get_inference_client", + return_value=mock_client, + ), _patch_convert_to_openai_tool(): + mock_mcp.mcp_tools = [mock_tool] - result = await analyze_general_query( - "how is cudn doing on 5.0?", conversation - ) + result = await analyze_general_query(conversation) assert result["success"] is True assert "stable" in result["message"] - mock_agentic.assert_called_once() + mock_client.chat_with_tools_async.assert_called_once() - call_args = mock_agentic.call_args - messages = call_args.kwargs.get("messages") or call_args[0][0] + call_kwargs = mock_client.chat_with_tools_async.call_args.kwargs + messages = call_kwargs["messages"] assert messages[0]["role"] == "system" assert messages[1]["role"] == "user" assert messages[1]["content"] == "how is cudn doing on 5.0?" @@ -67,25 +81,26 @@ def test_analyze_with_conversation_history(): ] async def _run(): + mock_client = _make_mock_client("ovn-controller CPU is at 9.0") + mock_tool = MagicMock() + mock_tool.name = "get_orion_metrics" + with patch( "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", new_callable=AsyncMock, ), patch( "bugzooka.analysis.general_query_analyzer.mcp_module" ) as mock_mcp, patch( - "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", - new_callable=AsyncMock, - ) as mock_agentic: - mock_mcp.mcp_tools = [MagicMock()] - mock_agentic.return_value = "ovn-controller CPU is at 9.0" + "bugzooka.analysis.general_query_analyzer.get_inference_client", + return_value=mock_client, + ), _patch_convert_to_openai_tool(): + mock_mcp.mcp_tools = [mock_tool] - result = await analyze_general_query( - "what about ovn-controller CPU?", conversation - ) + result = await analyze_general_query(conversation) assert result["success"] is True - call_args = mock_agentic.call_args - messages = call_args.kwargs.get("messages") or call_args[0][0] + call_kwargs = mock_client.chat_with_tools_async.call_args.kwargs + messages = call_kwargs["messages"] assert len(messages) == 4 # system + 3 conversation messages assert messages[1]["content"] == "report on cudn for 5.0" assert messages[2]["role"] == "assistant" @@ -102,9 +117,7 @@ async def _run(): ), patch("bugzooka.analysis.general_query_analyzer.mcp_module") as mock_mcp: mock_mcp.mcp_tools = [] - result = await analyze_general_query( - "hello", [{"role": "user", "content": "hello"}] - ) + result = await analyze_general_query([{"role": "user", "content": "hello"}]) assert result["success"] is False assert "MCP" in result["message"] @@ -113,21 +126,22 @@ async def _run(): def test_analyze_empty_llm_result(): async def _run(): + mock_client = _make_mock_client("") + mock_tool = MagicMock() + mock_tool.name = "test_tool" + with patch( "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", new_callable=AsyncMock, ), patch( "bugzooka.analysis.general_query_analyzer.mcp_module" ) as mock_mcp, patch( - "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", - new_callable=AsyncMock, - ) as mock_agentic: - mock_mcp.mcp_tools = [MagicMock()] - mock_agentic.return_value = "" + "bugzooka.analysis.general_query_analyzer.get_inference_client", + return_value=mock_client, + ), _patch_convert_to_openai_tool(): + mock_mcp.mcp_tools = [mock_tool] - result = await analyze_general_query( - "test", [{"role": "user", "content": "test"}] - ) + result = await analyze_general_query([{"role": "user", "content": "test"}]) assert result["success"] is False anyio.run(_run) @@ -135,22 +149,95 @@ async def _run(): def test_analyze_exception_handling(): async def _run(): + mock_client = MagicMock() + mock_client.chat_with_tools_async = AsyncMock( + side_effect=RuntimeError("LLM timeout") + ) + mock_tool = MagicMock() + mock_tool.name = "test_tool" + with patch( "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", new_callable=AsyncMock, ), patch( "bugzooka.analysis.general_query_analyzer.mcp_module" ) as mock_mcp, patch( - "bugzooka.analysis.general_query_analyzer.analyze_with_agentic", - new_callable=AsyncMock, - ) as mock_agentic: - mock_mcp.mcp_tools = [MagicMock()] - mock_agentic.side_effect = RuntimeError("LLM timeout") + "bugzooka.analysis.general_query_analyzer.get_inference_client", + return_value=mock_client, + ), _patch_convert_to_openai_tool(): + mock_mcp.mcp_tools = [mock_tool] - result = await analyze_general_query( - "test", [{"role": "user", "content": "test"}] - ) + result = await analyze_general_query([{"role": "user", "content": "test"}]) assert result["success"] is False assert "LLM timeout" in result["message"] anyio.run(_run) + + +def test_execute_tool_routes_through_invoke_mcp_tool(): + """Verify the custom execute_tool closure calls invoke_mcp_tool.""" + conversation = [{"role": "user", "content": "test"}] + + async def _run(): + mock_client = _make_mock_client("done") + mock_tool = MagicMock() + mock_tool.name = "get_orion_configs" + + with patch( + "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", + new_callable=AsyncMock, + ), patch( + "bugzooka.analysis.general_query_analyzer.mcp_module" + ) as mock_mcp, patch( + "bugzooka.analysis.general_query_analyzer.get_inference_client", + return_value=mock_client, + ), _patch_convert_to_openai_tool(), patch( + "bugzooka.analysis.general_query_analyzer.invoke_mcp_tool", + new_callable=AsyncMock, + return_value="tool result", + ) as mock_invoke: + mock_mcp.mcp_tools = [mock_tool] + + await analyze_general_query(conversation) + + # Extract the execute_tool_func that was passed to chat_with_tools_async + call_kwargs = mock_client.chat_with_tools_async.call_args.kwargs + execute_func = call_kwargs["execute_tool_func"] + + # Call it directly to verify it routes through invoke_mcp_tool + result = await execute_func("get_orion_configs", {}) + mock_invoke.assert_called_once_with(mock_tool, {}) + assert result == "tool result" + + anyio.run(_run) + + +def test_execute_tool_unknown_tool(): + """Verify execute_tool returns error for unknown tool names.""" + conversation = [{"role": "user", "content": "test"}] + + async def _run(): + mock_client = _make_mock_client("done") + mock_tool = MagicMock() + mock_tool.name = "known_tool" + + with patch( + "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", + new_callable=AsyncMock, + ), patch( + "bugzooka.analysis.general_query_analyzer.mcp_module" + ) as mock_mcp, patch( + "bugzooka.analysis.general_query_analyzer.get_inference_client", + return_value=mock_client, + ), _patch_convert_to_openai_tool(): + mock_mcp.mcp_tools = [mock_tool] + + await analyze_general_query(conversation) + + call_kwargs = mock_client.chat_with_tools_async.call_args.kwargs + execute_func = call_kwargs["execute_tool_func"] + + result = await execute_func("nonexistent_tool", {}) + assert "not found" in result + + anyio.run(_run) From 3443f12015b47db55fa27b3f6eb82f27d24724fb Mon Sep 17 00:00:00 2001 From: Mohit Sheth Date: Tue, 30 Jun 2026 22:28:05 -0400 Subject: [PATCH 4/7] Refactor _process_mention into command dispatcher pattern MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extract the 400-line _process_mention method into focused components: - _run_command: shared wrapper handling ack message, try/except, telemetry emission, and error posting to Slack - _handle_pr_analysis: PR-specific response formatting (section splitting) - _handle_nightly_inspection: nightly regression response formatting - _handle_perf_summary: performance summary arg parsing and multi-message - _handle_general_query: conversation history, image collection, chunking _process_mention is now a ~30-line dispatcher that pattern-matches the command and delegates to the appropriate handler via _run_command. Adding a new command handler no longer requires copying 80 lines of boilerplate — just write a handler method and add one elif clause. Assisted-by: Claude Signed-off-by: Mohit Sheth --- .../integrations/slack_socket_listener.py | 546 +++++++----------- 1 file changed, 220 insertions(+), 326 deletions(-) diff --git a/bugzooka/integrations/slack_socket_listener.py b/bugzooka/integrations/slack_socket_listener.py index ab978e6..44bbdee 100644 --- a/bugzooka/integrations/slack_socket_listener.py +++ b/bugzooka/integrations/slack_socket_listener.py @@ -116,323 +116,187 @@ def _clean_mention_text(text: str) -> str: """Remove bot @mention tags from message text.""" return re.sub(r"<@\w+>\s*", "", text).strip() - def _process_mention(self, event: Dict[str, Any]) -> None: - """ - Process an @ mention of the bot (core processing logic). - Routes to specialized handlers for structured commands, - or falls back to the general agentic query handler. - - :param event: Slack event data - """ - if not self._should_process_message(event): - return + def _run_command( + self, + *, + command_name: str, + channel: str, + thread_ts: str, + user: str, + ack_text: str, + handler: Any, + ) -> None: + _start = time.time() + _success = False + _error_message = None + _error_type = None + extra_telemetry: Dict[str, Any] = {} + try: + self.client.chat_postMessage( + channel=channel, text=ack_text, thread_ts=thread_ts + ) + extra_telemetry = handler() + _success = True + except Exception as e: + self.logger.error("Error processing %s: %s", command_name, e, exc_info=True) + self.client.chat_postMessage( + channel=channel, + text=f"An error occurred: {str(e)}", + thread_ts=thread_ts, + ) + _error_message = str(e) + _error_type = type(e).__name__ + finally: + telemetry.emit( + { + "command": command_name, + "trigger_type": "user_initiated", + "channel_id": channel, + "user_id": user, + "success": _success, + "error_message": _error_message, + "error_type": _error_type, + "duration_ms": int((time.time() - _start) * 1000), + "retry_count": 0, + **extra_telemetry, + } + ) - user: str = event.get("user", "Unknown") - ts: str = event.get("ts", "") - channel: str = event.get("channel", "") - text: str = event.get("text", "") + def _handle_pr_analysis( + self, text: str, channel: str, thread_ts: str, user: str + ) -> Dict[str, Any]: + analysis_result = anyio.run(analyze_pr_with_gemini, text, channel) + message_content = analysis_result["message"] + separator = "=" * 80 - self.logger.info(f"📩 Processing mention from {user} at ts {ts}") - - # Check if message contains "analyze pr" - if "analyze pr" in text.lower(): - _start = time.time() - _success = False - _error_message = None - _error_type = None - _pr_repo = None - _total_tokens = 0 - _tool_calls_count = 0 - try: - # Send initial acknowledgment + if separator in message_content: + sections = message_content.split(separator) + if sections: self.client.chat_postMessage( channel=channel, - text="🔍 Analyzing PR performance... This may take a few moments.", - thread_ts=ts, + text=f":robot_face: *PR Performance Analysis (AI generated)*\n\n{sections[0].strip()}", + thread_ts=thread_ts, ) - - # Analyze PR from text (need to run async function in sync context) - analysis_result = anyio.run(analyze_pr_with_gemini, text, channel) - - # Split the result by "====" separator and send each part as a separate message - message_content = analysis_result["message"] - separator = "=" * 80 # 80 equals signs - - # Check if separator exists in the message - if separator in message_content: - # Split by separator - sections = message_content.split(separator) - - # Send first section with the header - if sections: - first_section = sections[0].strip() - self.client.chat_postMessage( - channel=channel, - text=f":robot_face: *PR Performance Analysis (AI generated)*\n\n{first_section}", - thread_ts=ts, - ) - - # Send remaining sections (tables) as separate messages - for i, section in enumerate(sections[1:], start=1): - section = section.strip() - if section: # Only send non-empty sections - self.client.chat_postMessage( - channel=channel, - text=section, - thread_ts=ts, - ) - self.logger.debug(f"Sent section {i} of PR analysis") - else: - # No separator found, send everything in one message + for i, section in enumerate(sections[1:], start=1): + section = section.strip() + if section: self.client.chat_postMessage( - channel=channel, - text=f":robot_face: *PR Performance Analysis (AI generated)*\n\n{message_content}", - thread_ts=ts, - ) - - if analysis_result["success"]: - org, repo, pr_numbers, version = analysis_result["pr_info"] - _pr_repo = f"{org}/{repo}" - pr_display = ", ".join(f"#{pr}" for pr in pr_numbers) - self.logger.info( - "Sent PR analysis for %s/%s %s (OpenShift %s) to %s", - org, - repo, - pr_display, - version, - user, + channel=channel, text=section, thread_ts=thread_ts ) - else: - self.logger.warning( - f"⚠️ PR analysis failed: {analysis_result['message']}" - ) - _success = True - - try: - from bugzooka.integrations.inference_client import ( - get_inference_client, - ) - - client = get_inference_client() - _total_tokens = client.last_total_tokens - _tool_calls_count = client.last_tool_calls_count - except Exception: - pass - - except Exception as e: - self.logger.error( - f"Error processing PR summarization: {e}", exc_info=True - ) - self.client.chat_postMessage( - channel=channel, - text=f"❌ Unexpected error: {str(e)}", - thread_ts=ts, - ) - _error_message = str(e) - _error_type = type(e).__name__ - finally: - telemetry.emit( - { - "command": "analyze_pr", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - "pr_repo": _pr_repo, - "total_tokens": _total_tokens, - "tool_calls_count": _tool_calls_count, - } - ) - return - - # Check if message contains "inspect" for nightly regression analysis - if "inspect" in text.lower(): - _start = time.time() - _success = False - _error_message = None - _error_type = None - _nightly_version = None - try: - # Send initial acknowledgment - self.client.chat_postMessage( - channel=channel, - text=":mag: Analyzing nightly build for regressions... This may take a few moments.", - thread_ts=ts, - ) + self.logger.debug("Sent section %d of PR analysis", i) + else: + self.client.chat_postMessage( + channel=channel, + text=f":robot_face: *PR Performance Analysis (AI generated)*\n\n{message_content}", + thread_ts=thread_ts, + ) - # Analyze nightly regression (need to run async function in sync context) - analysis_result = anyio.run(analyze_nightly_regression, text, channel) + extra: Dict[str, Any] = {} + if analysis_result["success"]: + org, repo, pr_numbers, version = analysis_result["pr_info"] + extra["pr_repo"] = f"{org}/{repo}" + self.logger.info( + "Sent PR analysis for %s/%s %s (OpenShift %s) to %s", + org, + repo, + ", ".join(f"#{p}" for p in pr_numbers), + version, + user, + ) + else: + self.logger.warning("PR analysis failed: %s", analysis_result["message"]) - # Send the result - message_content = analysis_result["message"] + try: + from bugzooka.integrations.inference_client import get_inference_client + + client = get_inference_client() + extra["total_tokens"] = client.last_total_tokens + extra["tool_calls_count"] = client.last_tool_calls_count + except Exception: + pass + return extra + + def _handle_nightly_inspection( + self, text: str, channel: str, thread_ts: str, user: str + ) -> Dict[str, Any]: + analysis_result = anyio.run(analyze_nightly_regression, text, channel) + message_content = analysis_result["message"] + + self.client.chat_postMessage( + channel=channel, + text=f"*Nightly Regression Analysis*\n\n{message_content}", + thread_ts=thread_ts, + ) - self.client.chat_postMessage( - channel=channel, - text=f"*Nightly Regression Analysis*\n\n{message_content}", - thread_ts=ts, + extra: Dict[str, Any] = {} + if analysis_result["success"]: + nightly_info = analysis_result.get("nightly_info") + if nightly_info: + extra["nightly_version"] = nightly_info[0] + self.logger.info( + "Sent nightly regression analysis for %s to %s", + nightly_info[0], + user, ) + else: + self.logger.info("Sent nightly regression analysis to %s", user) + else: + self.logger.warning( + "Nightly regression analysis failed: %s", analysis_result["message"] + ) + return extra - if analysis_result["success"]: - nightly_info = analysis_result.get("nightly_info") - if nightly_info: - ( - nightly_version, - previous_nightly, - config, - lookback, - ) = nightly_info - _nightly_version = nightly_version - self.logger.info( - f"Sent nightly regression analysis for {nightly_version} to {user}" - ) - else: - self.logger.info(f"Sent nightly regression analysis to {user}") - else: - self.logger.warning( - f"Nightly regression analysis failed: {analysis_result['message']}" - ) - _success = True + def _handle_perf_summary( + self, text: str, channel: str, thread_ts: str, user: str + ) -> Dict[str, Any]: + configs, versions, lookback_days, use_all_configs = parse_perf_summary_args( + text + ) - except Exception as e: - self.logger.error( - f"Error processing nightly regression analysis: {e}", exc_info=True - ) - self.client.chat_postMessage( - channel=channel, - text=f"Unexpected error: {str(e)}", - thread_ts=ts, - ) - _error_message = str(e) - _error_type = "mcp_error" - finally: - telemetry.emit( - { - "command": "inspect_nightly", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - "nightly_version": _nightly_version, - } - ) - return + result = anyio.run( + analyze_performance, + configs, + versions, + lookback_days, + use_all_configs, + channel, + ) - # Check if message contains "performance summary" - if "performance summary" in text.lower(): - _start = time.time() - _success = False - _error_message = None - _error_type = None - _configs_count = 0 - _versions_count = 0 - try: - # Send initial acknowledgment + if result["success"]: + messages = result.get("messages", []) + for msg in messages: self.client.chat_postMessage( - channel=channel, - text="📊 Gathering performance summary... This may take a moment.", - thread_ts=ts, + channel=channel, text=msg, thread_ts=thread_ts ) + self.logger.info( + "Sent performance summary to %s (%d message(s))", user, len(messages) + ) + else: + self.client.chat_postMessage( + channel=channel, + text=result.get("message", "Unknown error"), + thread_ts=thread_ts, + ) + self.logger.warning("Performance summary failed: %s", result.get("message")) - # Parse configs, versions, lookback days, and verbose flag - ( - configs, - versions, - lookback_days, - use_all_configs, - ) = parse_perf_summary_args(text) - _configs_count = len(configs) - _versions_count = len(versions) - - # Run async function in sync context - result = anyio.run( - analyze_performance, - configs, - versions, - lookback_days, - use_all_configs, - channel, - ) - - # Send the result(s) - may be multiple messages to avoid Slack limit - if result["success"]: - messages = result.get("messages", []) - for msg in messages: - self.client.chat_postMessage( - channel=channel, - text=msg, - thread_ts=ts, - ) - self.logger.info( - f"✅ Sent performance summary to {user} ({len(messages)} message(s))" - ) - else: - self.client.chat_postMessage( - channel=channel, - text=result.get("message", "Unknown error"), - thread_ts=ts, - ) - self.logger.warning( - f"⚠️ Performance summary failed: {result.get('message')}" - ) - _success = True - - except Exception as e: - self.logger.error( - f"Error processing performance summary: {e}", exc_info=True - ) - self.client.chat_postMessage( - channel=channel, - text=f"❌ Unexpected error: {str(e)}", - thread_ts=ts, - ) - _error_message = str(e) - _error_type = "mcp_error" - finally: - telemetry.emit( - { - "command": "perf_summary", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - "configs_count": _configs_count, - "versions_count": _versions_count, - } - ) - return + return {"configs_count": len(configs), "versions_count": len(versions)} - # Default: General agentic query handler for free-form questions + def _handle_general_query( + self, + event: Dict[str, Any], + text: str, + channel: str, + ts: str, + user: str, + ) -> None: thread_ts = event.get("thread_ts", ts) clean_text = self._clean_mention_text(text) - _start = time.time() - _success = False - _error_message = None - _error_type = None - _total_tokens = 0 - _tool_calls_count = 0 collector = ImageCollector() # Must set collector inside the worker thread — ThreadPoolExecutor # does not propagate the calling thread's contextvars. token = set_collector(collector) - try: - self.client.chat_postMessage( - channel=channel, - text=":hourglass_flowing_sand: Thinking...", - thread_ts=thread_ts, - ) + def handler() -> Dict[str, Any]: self.conversation_manager.append_user_message( channel, thread_ts, clean_text ) @@ -443,7 +307,6 @@ def _process_mention(self, event: Dict[str, Any]) -> None: analysis_result = anyio.run( analyze_general_query, conversation_messages, channel ) - result_text = analysis_result.get("message", "") if analysis_result.get("success"): @@ -453,9 +316,7 @@ def _process_mention(self, event: Dict[str, Any]) -> None: chunks = self.chunk_text(result_text) for chunk in chunks: self.client.chat_postMessage( - channel=channel, - text=chunk, - thread_ts=thread_ts, + channel=channel, text=chunk, thread_ts=thread_ts ) if collector.has_images(): @@ -472,55 +333,88 @@ def _process_mention(self, event: Dict[str, Any]) -> None: self.logger.error("Failed to upload image: %s", upload_err) self.logger.info( - f"Sent general query response to {user} ({len(chunks)} chunk(s))" + "Sent general query response to %s (%d chunk(s))", + user, + len(chunks), ) - _success = True else: self.client.chat_postMessage( - channel=channel, - text=result_text, - thread_ts=thread_ts, + channel=channel, text=result_text, thread_ts=thread_ts ) - self.logger.warning(f"General query failed: {result_text}") + self.logger.warning("General query failed: %s", result_text) + extra: Dict[str, Any] = {"is_followup": event.get("thread_ts") is not None} try: from bugzooka.integrations.inference_client import get_inference_client client = get_inference_client() - _total_tokens = client.last_total_tokens - _tool_calls_count = client.last_tool_calls_count + extra["total_tokens"] = client.last_total_tokens + extra["tool_calls_count"] = client.last_tool_calls_count except Exception: pass + return extra - except Exception as e: - self.logger.error(f"Error processing general query: {e}", exc_info=True) - self.client.chat_postMessage( + try: + self._run_command( + command_name="general_query", channel=channel, - text=f"An error occurred: {str(e)}", thread_ts=thread_ts, + user=user, + ack_text=":hourglass_flowing_sand: Thinking...", + handler=handler, ) - _error_message = str(e) - _error_type = type(e).__name__ finally: collector.clear() reset_collector(token) - is_followup = event.get("thread_ts") is not None - telemetry.emit( - { - "command": "general_query", - "trigger_type": "user_initiated", - "channel_id": channel, - "user_id": user, - "success": _success, - "error_message": _error_message, - "error_type": _error_type, - "duration_ms": int((time.time() - _start) * 1000), - "retry_count": 0, - "total_tokens": _total_tokens, - "tool_calls_count": _tool_calls_count, - "is_followup": is_followup, - } + + def _process_mention(self, event: Dict[str, Any]) -> None: + """ + Process an @ mention of the bot. + Routes to specialized handlers for structured commands, + or falls back to the general agentic query handler. + """ + if not self._should_process_message(event): + return + + user: str = event.get("user", "Unknown") + ts: str = event.get("ts", "") + channel: str = event.get("channel", "") + text: str = event.get("text", "") + text_lower = text.lower() + + self.logger.info("Processing mention from %s at ts %s", user, ts) + + if "analyze pr" in text_lower: + self._run_command( + command_name="analyze_pr", + channel=channel, + thread_ts=ts, + user=user, + ack_text="🔍 Analyzing PR performance... This may take a few moments.", + handler=lambda: self._handle_pr_analysis(text, channel, ts, user), + ) + elif "inspect" in text_lower: + self._run_command( + command_name="inspect_nightly", + channel=channel, + thread_ts=ts, + user=user, + ack_text=":mag: Analyzing nightly build for regressions... This may take a few moments.", + handler=lambda: self._handle_nightly_inspection( + text, channel, ts, user + ), + ) + elif "performance summary" in text_lower: + self._run_command( + command_name="perf_summary", + channel=channel, + thread_ts=ts, + user=user, + ack_text="📊 Gathering performance summary... This may take a moment.", + handler=lambda: self._handle_perf_summary(text, channel, ts, user), ) + else: + self._handle_general_query(event, text, channel, ts, user) def _submit_mention_for_processing(self, event: Dict[str, Any]) -> None: """ From 9197bf1170f0ee5e63f73b3dbc4f270d5a4a8555 Mon Sep 17 00:00:00 2001 From: Mohit Sheth Date: Tue, 30 Jun 2026 22:52:04 -0400 Subject: [PATCH 5/7] Add handler tests and fix stale tests for command dispatcher Add tests for previously untested command handlers: - PR analysis dispatch and response splitting by ==== separator - Performance summary dispatch with multi-message response - General query dispatch with analyzer mock Fix existing tests broken by the dispatcher refactor: - Replace stale greeting test with general query dispatch test - Mock analyzer in submit test to avoid hitting live MCP - Fix error handling test to mock analyzer instead of postMessage Remove trivial edge-case tests that don't add value: - conversation: returns-copy, empty-for-unknown, clear - analyzer: empty LLM result, unknown tool name - listener: PR exception (same _run_command path as nightly), perf summary failure, general query exception Assisted-by: Claude Signed-off-by: Mohit Sheth --- tests/test_conversation.py | 17 --- tests/test_general_query_analyzer.py | 54 ------- tests/test_slack_socket_listener.py | 205 ++++++++++++++++++++------- 3 files changed, 152 insertions(+), 124 deletions(-) diff --git a/tests/test_conversation.py b/tests/test_conversation.py index e2e3ad6..6a04d03 100644 --- a/tests/test_conversation.py +++ b/tests/test_conversation.py @@ -33,17 +33,6 @@ def test_separate_threads_have_separate_history(self): assert msgs1[0]["content"] == "thread 1" assert msgs2[0]["content"] == "thread 2" - def test_get_messages_returns_copy(self): - mgr = ConversationManager() - mgr.append_user_message("C123", "ts1", "hello") - msgs = mgr.get_messages("C123", "ts1") - msgs.append({"role": "user", "content": "injected"}) - assert len(mgr.get_messages("C123", "ts1")) == 1 - - def test_get_messages_empty_for_unknown_thread(self): - mgr = ConversationManager() - assert mgr.get_messages("C123", "unknown") == [] - def test_max_messages_trimmed(self): mgr = ConversationManager(max_messages=4) for i in range(6): @@ -61,12 +50,6 @@ def test_ttl_expiry(self): assert mgr.get_messages("C123", "ts1") == [] assert len(mgr.get_messages("C123", "ts2")) == 1 - def test_clear(self): - mgr = ConversationManager() - mgr.append_user_message("C123", "ts1", "hello") - mgr.clear("C123", "ts1") - assert mgr.get_messages("C123", "ts1") == [] - def test_thread_safety(self): mgr = ConversationManager() errors = [] diff --git a/tests/test_general_query_analyzer.py b/tests/test_general_query_analyzer.py index 3ed37b4..81b1c98 100644 --- a/tests/test_general_query_analyzer.py +++ b/tests/test_general_query_analyzer.py @@ -124,29 +124,6 @@ async def _run(): anyio.run(_run) -def test_analyze_empty_llm_result(): - async def _run(): - mock_client = _make_mock_client("") - mock_tool = MagicMock() - mock_tool.name = "test_tool" - - with patch( - "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", - new_callable=AsyncMock, - ), patch( - "bugzooka.analysis.general_query_analyzer.mcp_module" - ) as mock_mcp, patch( - "bugzooka.analysis.general_query_analyzer.get_inference_client", - return_value=mock_client, - ), _patch_convert_to_openai_tool(): - mock_mcp.mcp_tools = [mock_tool] - - result = await analyze_general_query([{"role": "user", "content": "test"}]) - assert result["success"] is False - - anyio.run(_run) - - def test_analyze_exception_handling(): async def _run(): mock_client = MagicMock() @@ -210,34 +187,3 @@ async def _run(): assert result == "tool result" anyio.run(_run) - - -def test_execute_tool_unknown_tool(): - """Verify execute_tool returns error for unknown tool names.""" - conversation = [{"role": "user", "content": "test"}] - - async def _run(): - mock_client = _make_mock_client("done") - mock_tool = MagicMock() - mock_tool.name = "known_tool" - - with patch( - "bugzooka.analysis.general_query_analyzer.initialize_global_resources_async", - new_callable=AsyncMock, - ), patch( - "bugzooka.analysis.general_query_analyzer.mcp_module" - ) as mock_mcp, patch( - "bugzooka.analysis.general_query_analyzer.get_inference_client", - return_value=mock_client, - ), _patch_convert_to_openai_tool(): - mock_mcp.mcp_tools = [mock_tool] - - await analyze_general_query(conversation) - - call_kwargs = mock_client.chat_with_tools_async.call_args.kwargs - execute_func = call_kwargs["execute_tool_func"] - - result = await execute_func("nonexistent_tool", {}) - assert "not found" in result - - anyio.run(_run) diff --git a/tests/test_slack_socket_listener.py b/tests/test_slack_socket_listener.py index ae5926f..306d0d1 100644 --- a/tests/test_slack_socket_listener.py +++ b/tests/test_slack_socket_listener.py @@ -138,37 +138,34 @@ def test_should_not_process_bot_self_mention( assert listener._should_process_message(event) is False - def test_process_mention_sends_greeting( + def test_process_mention_general_query( self, mock_socket_mode_client, mock_web_client ): - """Test processing app_mention sends greeting message.""" + """Test processing a free-form question routes to general query handler.""" logger = logging.getLogger("test") with patch("bugzooka.core.config.SLACK_BOT_TOKEN", "xoxb-test-token"): with patch("bugzooka.core.config.SLACK_APP_TOKEN", "xapp-test-token"): - listener = SlackSocketListener(logger=logger) + with patch( + "bugzooka.integrations.slack_socket_listener.analyze_general_query" + ) as mock_analyze: + mock_analyze.return_value = { + "success": True, + "message": "Performance looks stable.", + } - event = create_app_mention_event( - text="<@UBOTID> hello", - user="U12345", - ts="1234567890.123456", - ) + listener = SlackSocketListener(logger=logger) - # Call the core processing logic directly - listener._process_mention(event) - - # Verify greeting message was sent (reaction happens earlier in the flow) - mock_web_client.return_value.chat_postMessage.assert_called_once_with( - channel=CHANNEL_ID, - text=( - "May the force be with you! :performance_jedi:\n\n" - ":bulb: *Tips:*\n" - "- `analyze pr: , compare with ` - PR performance analysis\n" - "- `inspect [vs ] [for config ] [for days]` - Nightly regression analysis\n" - "- `performance summary [ALL|config1.yaml,config2.yaml] [version ...]` - Performance metrics summary" - ), - thread_ts="1234567890.123456", - ) + event = create_app_mention_event( + text="<@UBOTID> how is cudn doing on 5.0?", + user="U12345", + ts="1234567890.123456", + ) + + listener._process_mention(event) + + mock_analyze.assert_called_once() + assert mock_web_client.return_value.chat_postMessage.call_count >= 2 def test_submit_mention_for_processing( self, mock_socket_mode_client, mock_web_client @@ -178,22 +175,26 @@ def test_submit_mention_for_processing( with patch("bugzooka.core.config.SLACK_BOT_TOKEN", "xoxb-test-token"): with patch("bugzooka.core.config.SLACK_APP_TOKEN", "xapp-test-token"): - listener = SlackSocketListener(logger=logger, max_workers=2) + with patch( + "bugzooka.integrations.slack_socket_listener.analyze_general_query" + ) as mock_analyze: + mock_analyze.return_value = { + "success": True, + "message": "Done", + } - event = create_app_mention_event( - text="<@UBOTID> async test", - user="U12345", - ts="1234567890.123456", - ) + listener = SlackSocketListener(logger=logger, max_workers=2) - # Call submission wrapper - listener._submit_mention_for_processing(event) + event = create_app_mention_event( + text="<@UBOTID> async test", + user="U12345", + ts="1234567890.123456", + ) - # Wait for thread to complete - listener.executor.shutdown(wait=True) + listener._submit_mention_for_processing(event) + listener.executor.shutdown(wait=True) - # Verify it was processed - mock_web_client.return_value.chat_postMessage.assert_called_once() + assert mock_web_client.return_value.chat_postMessage.call_count >= 1 def test_submit_mention_prevents_duplicates( self, mock_socket_mode_client, mock_web_client @@ -307,27 +308,29 @@ def test_process_socket_request_non_app_mention( def test_process_mention_error_handling( self, mock_socket_mode_client, mock_web_client ): - """Test that errors during mention processing are properly logged.""" + """Test that errors during mention processing are handled gracefully.""" logger = logging.getLogger("test") with patch("bugzooka.core.config.SLACK_BOT_TOKEN", "xoxb-test-token"): with patch("bugzooka.core.config.SLACK_APP_TOKEN", "xapp-test-token"): - listener = SlackSocketListener(logger=logger) + with patch( + "bugzooka.integrations.slack_socket_listener.analyze_general_query" + ) as mock_analyze: + mock_analyze.side_effect = RuntimeError("Analysis failed") - # Mock chat_postMessage to raise an exception - mock_web_client.return_value.chat_postMessage.side_effect = Exception( - "Test error" - ) + listener = SlackSocketListener(logger=logger) - event = create_app_mention_event( - text="<@UBOTID> test", ts="1234567890.123456" - ) + event = create_app_mention_event( + text="<@UBOTID> test", ts="1234567890.123456" + ) - # Should not raise exception - listener._process_mention(event) + # Should not raise exception + listener._process_mention(event) - # Verify chat_postMessage was attempted - mock_web_client.return_value.chat_postMessage.assert_called_once() + # Verify error message was posted + calls = mock_web_client.return_value.chat_postMessage.call_args_list + error_texts = [c.kwargs.get("text", "") for c in calls] + assert any("error" in t.lower() for t in error_texts) def test_process_mention_triggers_nightly_analysis( self, mock_socket_mode_client, mock_web_client @@ -464,12 +467,108 @@ def test_process_mention_nightly_analysis_exception( ts="1234567890.123456", ) - # Should not raise exception listener._process_mention(event) - # Verify error message was sent calls = mock_web_client.return_value.chat_postMessage.call_args_list - error_call = calls[-1] - assert "Unexpected error" in error_call.kwargs.get( - "text", error_call[1].get("text", "") + error_texts = [c.kwargs.get("text", "") for c in calls] + assert any("error" in t.lower() for t in error_texts) + + def test_process_mention_triggers_pr_analysis( + self, mock_socket_mode_client, mock_web_client + ): + """Test analyze pr command triggers PR analysis handler.""" + logger = logging.getLogger("test") + + with patch("bugzooka.core.config.SLACK_BOT_TOKEN", "xoxb-test-token"): + with patch("bugzooka.core.config.SLACK_APP_TOKEN", "xapp-test-token"): + with patch( + "bugzooka.integrations.slack_socket_listener.analyze_pr_with_gemini" + ) as mock_analyze: + mock_analyze.return_value = { + "success": True, + "message": "Performance looks good", + "pr_info": ("openshift", "ovn-kubernetes", ["123"], "4.19"), + } + + listener = SlackSocketListener(logger=logger) + + event = create_app_mention_event( + text="<@UBOTID> analyze pr: https://github.com/openshift/ovn-kubernetes/pull/123, compare with 4.19", + user="U12345", + ts="1234567890.123456", ) + + listener._process_mention(event) + + mock_analyze.assert_called_once() + assert mock_web_client.return_value.chat_postMessage.call_count >= 2 + + def test_process_mention_pr_analysis_with_separator( + self, mock_socket_mode_client, mock_web_client + ): + """Test PR analysis splits response by ==== separator into multiple messages.""" + logger = logging.getLogger("test") + + with patch("bugzooka.core.config.SLACK_BOT_TOKEN", "xoxb-test-token"): + with patch("bugzooka.core.config.SLACK_APP_TOKEN", "xapp-test-token"): + with patch( + "bugzooka.integrations.slack_socket_listener.analyze_pr_with_gemini" + ) as mock_analyze: + separator = "=" * 80 + mock_analyze.return_value = { + "success": True, + "message": f"Impact Assessment{separator}Table 1{separator}Table 2", + "pr_info": ("openshift", "ovn-kubernetes", ["123"], "4.19"), + } + + listener = SlackSocketListener(logger=logger) + + event = create_app_mention_event( + text="<@UBOTID> analyze pr: https://github.com/openshift/ovn-kubernetes/pull/123, compare with 4.19", + user="U12345", + ts="1234567890.123456", + ) + + listener._process_mention(event) + + # ack + 3 sections (header + 2 tables) + assert mock_web_client.return_value.chat_postMessage.call_count == 4 + + def test_process_mention_triggers_perf_summary( + self, mock_socket_mode_client, mock_web_client + ): + """Test performance summary command triggers perf summary handler.""" + logger = logging.getLogger("test") + + with patch("bugzooka.core.config.SLACK_BOT_TOKEN", "xoxb-test-token"): + with patch("bugzooka.core.config.SLACK_APP_TOKEN", "xapp-test-token"): + with patch( + "bugzooka.integrations.slack_socket_listener.analyze_performance" + ) as mock_analyze, patch( + "bugzooka.integrations.slack_socket_listener.parse_perf_summary_args" + ) as mock_parse: + mock_parse.return_value = ( + ["config1.yaml"], + ["4.19"], + 7, + False, + ) + mock_analyze.return_value = { + "success": True, + "messages": ["Summary table 1", "Summary table 2"], + } + + listener = SlackSocketListener(logger=logger) + + event = create_app_mention_event( + text="<@UBOTID> performance summary 7d", + user="U12345", + ts="1234567890.123456", + ) + + listener._process_mention(event) + + mock_parse.assert_called_once() + mock_analyze.assert_called_once() + # ack + 2 summary messages + assert mock_web_client.return_value.chat_postMessage.call_count == 3 From ba878884e09ee801862a28711a29189b746a0b98 Mon Sep 17 00:00:00 2001 From: Mohit Sheth Date: Wed, 1 Jul 2026 14:30:13 -0400 Subject: [PATCH 6/7] Add nightly PR/changes guidance to general query prompt Tell the LLM to use has_nightly_regressed when users ask about PRs or changes between nightly builds, not just for regression detection. Assisted-by: Claude Signed-off-by: Mohit Sheth --- bugzooka/analysis/prompts.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bugzooka/analysis/prompts.py b/bugzooka/analysis/prompts.py index 3159cff..48f5324 100644 --- a/bugzooka/analysis/prompts.py +++ b/bugzooka/analysis/prompts.py @@ -151,7 +151,7 @@ - Generate visual charts: `openshift_report_on` — generates chart images that are automatically uploaded to Slack. Use with `options="image"` for charts, `options="json"` for raw data, or `options="both"` for both. - Check for regressions across all configs: `has_openshift_regressed` - Check networking-specific regressions: `has_networking_regressed` -- Detect nightly build regressions: `has_nightly_regressed` +- Detect nightly build regressions: `has_nightly_regressed` — compares two nightlies or checks a single nightly for regressions. Use this when the user asks about nightly regressions, what changed between nightlies, or what PRs/changes landed between two nightly builds. Accepts `nightly_version` and optional `previous_nightly` for comparison. - Analyze PR performance impact: `openshift_report_on_pr` - Correlate two metrics: `metrics_correlation` — generates a scatter plot image that is automatically uploaded to Slack. - Get OpenShift release dates: `get_release_date` From 3552a1934c5320d117e5bc8abdd4bd226f1ee54a Mon Sep 17 00:00:00 2001 From: Mohit Sheth Date: Wed, 1 Jul 2026 16:36:40 -0400 Subject: [PATCH 7/7] Address PR review feedback from CodeRabbit - mcp_client: add fallback for MCP image payload keys (data/mimeType) alongside langchain-normalized keys (base64/mime_type) - conversation: evict expired conversations in get_messages() so stale threads are not accepted for processing - slack_socket_listener: replace silent except/pass with debug log for inference telemetry, gate thread replies with _should_process_message before adding reaction or submitting work - telemetry_client: revert _config to Optional[dict] = None to preserve disabled-by-default guard before start() runs, add asserts for mypy - test_conversation: adjust TTL test to use 1s TTL now that get_messages also evicts expired entries Assisted-by: Claude Signed-off-by: Mohit Sheth --- bugzooka/core/conversation.py | 1 + bugzooka/integrations/mcp_client.py | 6 ++++-- bugzooka/integrations/slack_socket_listener.py | 14 +++----------- bugzooka/telemetry/telemetry_client.py | 5 ++++- tests/test_conversation.py | 4 ++-- 5 files changed, 14 insertions(+), 16 deletions(-) diff --git a/bugzooka/core/conversation.py b/bugzooka/core/conversation.py index 8b4b7e1..eb13c9f 100644 --- a/bugzooka/core/conversation.py +++ b/bugzooka/core/conversation.py @@ -76,6 +76,7 @@ def append_assistant_message( def get_messages(self, channel_id: str, thread_ts: str) -> list[dict]: with self._lock: + self._evict_expired() key = self._key(channel_id, thread_ts) state = self._conversations.get(key) if state is None: diff --git a/bugzooka/integrations/mcp_client.py b/bugzooka/integrations/mcp_client.py index fa67136..16203e6 100644 --- a/bugzooka/integrations/mcp_client.py +++ b/bugzooka/integrations/mcp_client.py @@ -141,8 +141,10 @@ async def invoke_mcp_tool(tool: Any, args: dict) -> str: elif block.get("type") == "image": has_images = True if collector: - b64 = block.get("base64", "") - mime = block.get("mime_type", "image/png") + b64 = block.get("base64") or block.get("data", "") + mime = block.get("mime_type") or block.get( + "mimeType", "image/png" + ) collector.add_image(b64, mime, tool_name) if text_parts: diff --git a/bugzooka/integrations/slack_socket_listener.py b/bugzooka/integrations/slack_socket_listener.py index 44bbdee..ec1ce31 100644 --- a/bugzooka/integrations/slack_socket_listener.py +++ b/bugzooka/integrations/slack_socket_listener.py @@ -213,7 +213,7 @@ def _handle_pr_analysis( extra["total_tokens"] = client.last_total_tokens extra["tool_calls_count"] = client.last_tool_calls_count except Exception: - pass + self.logger.debug("Unable to read inference telemetry", exc_info=True) return extra def _handle_nightly_inspection( @@ -351,7 +351,7 @@ def handler() -> Dict[str, Any]: extra["total_tokens"] = client.last_total_tokens extra["tool_calls_count"] = client.last_tool_calls_count except Exception: - pass + self.logger.debug("Unable to read inference telemetry", exc_info=True) return extra try: @@ -472,15 +472,7 @@ def _process_socket_request( self.logger.debug(f"Received event type: {event_type}") - # Handle @mentions and thread replies to active conversations - is_thread_reply = ( - event_type == "message" - and event.get("thread_ts") - and event.get("user") != JEDI_BOT_SLACK_USER_ID - and not event.get("bot_id") - ) - - if event_type == "app_mention" or is_thread_reply: + if not event.get("bot_id") and self._should_process_message(event): ts = event.get("ts") channel = event.get("channel") diff --git a/bugzooka/telemetry/telemetry_client.py b/bugzooka/telemetry/telemetry_client.py index 39fa569..fc62d8f 100644 --- a/bugzooka/telemetry/telemetry_client.py +++ b/bugzooka/telemetry/telemetry_client.py @@ -16,7 +16,7 @@ _queue: queue.Queue = queue.Queue(maxsize=1000) _thread: Optional[threading.Thread] = None _es_client = None -_config: dict = {} +_config: Optional[dict] = None _channel_team_mappings: dict = {} _shutdown_event = threading.Event() _started = False @@ -112,6 +112,7 @@ def shutdown() -> None: def _flush_thread() -> None: """Daemon thread: drain queue and bulk-write to ES periodically.""" + assert _config is not None flush_interval = _config["flush_interval"] batch_size = _config["batch_size"] @@ -148,6 +149,7 @@ def _bulk_write(events: list) -> None: ) return + assert _config is not None index_name = _config["index_prefix"] actions = [{"_index": index_name, "_source": event} for event in events] @@ -171,6 +173,7 @@ def _ensure_index_template() -> None: if _es_client is None: return + assert _config is not None template_name = f"{_config['index_prefix']}-template" template_body = { "index_patterns": [f"{_config['index_prefix']}"], diff --git a/tests/test_conversation.py b/tests/test_conversation.py index 6a04d03..0a53417 100644 --- a/tests/test_conversation.py +++ b/tests/test_conversation.py @@ -43,9 +43,9 @@ def test_max_messages_trimmed(self): assert msgs[-1]["content"] == "msg 5" def test_ttl_expiry(self): - mgr = ConversationManager(ttl_seconds=0) + mgr = ConversationManager(ttl_seconds=1) mgr.append_user_message("C123", "ts1", "old message") - time.sleep(0.01) + time.sleep(1.1) mgr.append_user_message("C123", "ts2", "new message") assert mgr.get_messages("C123", "ts1") == [] assert len(mgr.get_messages("C123", "ts2")) == 1