Resolve Python CodeQL quality alerts

scripts/dataset_utils/filter_tc_benchmark.py

@@ -113,7 +113,7 @@ def filter_high_quality_samples(
 
         # Then group by scenario and aggregate
         results_scenario = results_by_uid.groupby("scenario").agg(
-            {"result": "mean", "uid": lambda x: list(x)}
+            {"result": "mean", "uid": list}
         )
 
         # Apply bottom threshold

scripts/import_typesense_snapshots.py

@@ -25,7 +25,8 @@ def wait_until_healthy(url: str, timeout: float, max_wait: float):
             resp = s.get(url)
             if resp.status_code == 200:
                 return
-        except Exception:
+        except httpx.HTTPError:
+            # Typesense may reject connections while it is still starting.
             pass
         if time.monotonic() - start > max_wait:
             raise TimeoutError(
@@ -84,6 +85,7 @@ def ensure_collection(
                 self.client.collections[collection_name].delete()
                 print(f"Deleted collection {collection_name}")
             except typesense.exceptions.ObjectNotFound:
+                # The requested collection is already absent.
                 pass
         else:
             # check if collection already exists
@@ -92,6 +94,7 @@ def ensure_collection(
                 collection_exists = True
                 print(f"Found collection {collection_name}")
             except typesense.exceptions.ObjectNotFound:
+                # The collection is created below when it cannot be retrieved.
                 pass
         # Create collection if it doesn't exist
         if not collection_exists:

tests/mcp_test_tool.py

@@ -4,7 +4,6 @@
 
 import argparse
 import enum
-import json
 import os
 import sys
 from pathlib import Path

tests/test_aoai_session.py

@@ -38,11 +38,11 @@ def get_mock_async_client(response_json):
 
 @pytest.mark.asyncio
 @pytest.mark.parametrize("use_certificate", [True, False])
-@patch("thinkingbox.common.llm_session_base.Path.is_file", return_value=True)
-@patch("thinkingbox.common.llm_session_base.ssl.create_default_context")
+@patch("thinkingbox.common.http_client.Path.is_file", return_value=True)
+@patch("thinkingbox.common.http_client.ssl.create_default_context")
 @patch("thinkingbox.common.llm_session_base.httpx.AsyncClient")
 async def test_client_certificate(
-    mock_async_client, mock_ssl_context, _mock_path_exists, use_certificate
+    mock_async_client, mock_ssl_context, _mock_path_is_file, use_certificate
 ):
     # AsyncClient mock needs to return a mock response for the _get_completion call
     mock_async_client.return_value = get_mock_async_client(SIMPLE_RESPONSE)
@@ -78,12 +78,12 @@ async def test_client_certificate(
         assert kwargs.get("verify") is True
 
 
-@patch("thinkingbox.common.llm_session_base.Path.exists")
-@patch("thinkingbox.common.llm_session_base.ssl.create_default_context")
+@patch("thinkingbox.common.http_client.Path.is_file")
+@patch("thinkingbox.common.http_client.ssl.create_default_context")
 def test_get_completion_certificate_file_does_not_exist(
-    _mock_ssl_context, _mock_path_exists
+    _mock_ssl_context, _mock_path_is_file
 ):
-    _mock_path_exists.return_value = False
+    _mock_path_is_file.return_value = False
     config = get_test_aoai_config()
     config.client_certificate = "/invalid/path/to/client.pem"
     with pytest.raises(
@@ -94,9 +94,8 @@ def test_get_completion_certificate_file_does_not_exist(
 
 
 @pytest.mark.asyncio
-@patch("thinkingbox.common.llm_session_base.Path.exists", return_value=True)
 @patch("thinkingbox.common.llm_session_base.httpx.AsyncClient")
-async def test_client_api_key(mock_async_client, _mock_path_exists):
+async def test_client_api_key(mock_async_client):
     # AsyncClient mock needs to return a mock response for the _get_completion call
     mock_async_client.return_value = get_mock_async_client(SIMPLE_RESPONSE)
 
@@ -113,9 +112,8 @@ async def test_client_api_key(mock_async_client, _mock_path_exists):
 
 
 @pytest.mark.asyncio
-@patch("thinkingbox.common.llm_session_base.Path.exists", return_value=True)
 @patch("thinkingbox.common.llm_session_base.httpx.AsyncClient")
-async def test_response_schema_in_payload(mock_async_client, _mock_path_exists):
+async def test_response_schema_in_payload(mock_async_client):
     """response_schema should build the full response_format in the request payload."""
     mock_async_client.return_value = get_mock_async_client(SIMPLE_RESPONSE)
 
@@ -194,9 +192,8 @@ async def test_response_schema_requires_additional_properties_false():
 
 
 @pytest.mark.asyncio
-@patch("thinkingbox.common.llm_session_base.Path.exists", return_value=True)
 @patch("thinkingbox.common.llm_session_base.httpx.AsyncClient")
-async def test_get_completion_reports_usage(mock_async_client, _mock_path_exists):
+async def test_get_completion_reports_usage(mock_async_client):
     mock_async_client.return_value = get_mock_async_client(RESPONSE_WITH_USAGE)
 
     session = AOAISession.from_config(get_test_aoai_config())

tests/test_rubrics_judge.py

@@ -11,7 +11,6 @@
 from thinkingbox.common.rubrics_judge import (
     SYSTEM_PROMPT_PENALTY,
     SYSTEM_PROMPT_POSITIVE,
-    USER_PROMPT_TPL,
     RubricJudge,
 )
 

tests/test_session_proxy.py

@@ -70,6 +70,7 @@ async def wait_for_mcp_server(
                 _ = await client.list_tools()
             return
         except Exception:
+            # Connection failures are expected until the proxy is ready.
             pass
         if time.time() >= deadline:
             raise RuntimeError(f"Timeout trying to connect to {url}")
@@ -313,7 +314,8 @@ async def session_proxy_process(port: int, api_key: str):
                     r = await client.get(f"http://127.0.0.1:{port}/health")
                     if r.status_code == 200:
                         break
-            except Exception:
+            except httpx.HTTPError:
+                # Connection failures are expected until the proxy is ready.
                 pass
             if time.time() >= deadline:
                 raise RuntimeError("Timeout waiting for session proxy to start")
@@ -328,11 +330,11 @@ async def session_proxy_process(port: int, api_key: str):
             await asyncio.wait_for(proc.wait(), timeout=5)
 
 
-def assert_http_status_unauthorized_in_exception_group(exc: BaseException):
+def assert_http_status_unauthorized_in_exception_group(exc: Exception):
     if isinstance(exc, httpx.HTTPStatusError):
         assert exc.response.status_code == 401
-    elif isinstance(exc, BaseExceptionGroup):
-        errs = exc.subgroup(httpx.HTTPStatusError)
+    elif callable(subgroup := getattr(exc, "subgroup", None)):
+        errs = subgroup(httpx.HTTPStatusError)
         assert errs is not None, f"No HTTPStatusError in group: {exc}"
         assert errs.exceptions[0].response.status_code == 401
     else:
@@ -397,7 +399,7 @@ async def test_session_proxy_with_auth():
                 ) as _:
                     pass
                 pytest.fail("Expected HTTPStatusError")
-            except BaseException as exc:
+            except Exception as exc:
                 assert_http_status_unauthorized_in_exception_group(exc)
 
             # MCP endpoint with wrong key -> 401
@@ -413,7 +415,7 @@ async def test_session_proxy_with_auth():
                 ) as _:
                     pass
                 pytest.fail("Expected HTTPStatusError")
-            except BaseException as exc:
+            except Exception as exc:
                 assert_http_status_unauthorized_in_exception_group(exc)
 
             # MCP endpoint with correct key

tests/test_user_simulated_answer.py

@@ -226,22 +226,13 @@ async def test_user_simulator_sanitizes_done_marker():
     assert "<DONE>" not in sent_prompt
 
 
-def test_user_simulator_uses_correct_prompt():
-    """Test that correct system prompt is selected based on flag."""
+def test_user_simulator_prompts_differ_by_end_capability():
+    """Test that ending support changes the user simulator prompt."""
     from thinkingbox.common.user_simulated_answer import (
         SYSTEM_PROMPT,
         SYSTEM_PROMPT_WITH_END,
     )
 
-    mock_llm = MockSession(completions=[[Text(role="assistant", content="test")]])
-
-    # With ending disabled
-    sim_no_end = UserSimulator(llm=mock_llm, can_end_conversation=False)
-
-    # With ending enabled
-    sim_with_end = UserSimulator(llm=mock_llm, can_end_conversation=True)
-
-    # Verify prompts are different
     assert SYSTEM_PROMPT != SYSTEM_PROMPT_WITH_END
     assert "<DONE>" in SYSTEM_PROMPT_WITH_END
     assert "ENDING THE CONVERSATION" in SYSTEM_PROMPT_WITH_END

thinkingbox/cli/infer.py

@@ -95,7 +95,6 @@ async def work(self, work: HydratedTestCase) -> WorkResult[DecodeResult]:
                 )
                 return WorkResult(result=previous_result, is_correct=is_correct)
 
-        result, error = None, None
         start_time = time.time()
         timers = Timers()
         timers.ensure("time_agent")

thinkingbox/common/agent_session.py

@@ -191,8 +191,6 @@ async def decode_turn_iter(
         tr_fw_messages = []
 
         while True:
-            last_msg = self.conversation.messages[-1]
-
             # get new message
             with self.timers.measure("time_agent"):
                 messages = await self.llm.get_completion()

thinkingbox/common/agent_user_loop.py

@@ -325,4 +325,5 @@ def _try_store_raw_messages(agent: AgentSessionBase | None, result: DecodeResult
         # TODO let raw_messages be Any type instead?
         result.raw_messages = agent.get_raw_messages()
     except ValueError:
+        # Raw messages are optional and may not match the serializable schema.
         pass

thinkingbox/common/aoai_responses_session.py

@@ -6,8 +6,6 @@
 from enum import Enum
 from typing import Any, Literal
 
-import httpx
-
 from thinkingbox.common.chat_types import (
     Message,
     ParallelToolCall,

thinkingbox/common/http_client.py

@@ -194,7 +194,6 @@ async def post(self, url: str | httpx.URL, **kwargs) -> httpx.Response:
         base_headers = kwargs.pop("headers", None)
 
         retries = BackoffRetries()
-        action = RetryAction(retry=True, delay=1.0)
 
         # We'll loop until we either return or raise
         while True:
@@ -270,7 +269,6 @@ async def post_iter_sse_chunks(
         base_headers = kwargs.pop("headers", None)
 
         retries = BackoffRetries()
-        action = RetryAction(retry=True, delay=1.0)
 
         while True:
             headers = dict(base_headers) if base_headers else {}

thinkingbox/common/llm_session_base.py

@@ -2,10 +2,8 @@
 # Licensed under the MIT License.
 
 import contextlib
-import ssl
 from abc import ABC, abstractmethod
 from collections.abc import Collection
-from pathlib import Path
 from typing import Any, AsyncIterator
 
 import httpx

thinkingbox/common/testrunner.py

@@ -6,7 +6,6 @@
 import inspect
 import io
 import json
-import logging
 import string
 import sys
 import traceback

thinkingbox/tools/client/worker.py

@@ -116,6 +116,7 @@ async def stop(self):
             try:
                 await self.task
             except asyncio.CancelledError:
+                # Cancellation is the expected outcome after the stop timeout.
                 pass
 
 

thinkingbox/tools/session_proxy.py

@@ -274,6 +274,7 @@ async def on_call_tool(self, context, call_next):
                 if isinstance(parsed, dict):
                     structured = parsed
             except (AttributeError, IndexError, ValueError, TypeError):
+                # Non-JSON text content remains available in the original result.
                 pass
 
         return ToolResult(content=result.content, structured_content=structured)
@@ -523,10 +524,7 @@ async def _session_create_inner(data: SessionCreateRequest) -> ServerResponse:
         # one from propagating to the response
 
         async with sessions_lock:
-            try:
-                del sessions[data.session_id]
-            except KeyError:
-                pass
+            sessions.pop(data.session_id, None)
 
         try:
             await new_session.destroy()
@@ -671,8 +669,8 @@ async def _destroy_inactive_sessions(minutes: float) -> list[str]:
             continue
         try:
             await s.destroy()
-        except Exception:
-            pass
+        except Exception as e:
+            logger.warning("Error destroying expired session %s: %s", s_id, e)
 
     # return ids of destroyed sessions
     return [s_id for (s_id, _) in to_destroy]