From 319a857fbd53401e811579efe428033fd414f387 Mon Sep 17 00:00:00 2001 From: Liang-Chun Tsai Date: Tue, 18 Aug 2026 14:54:37 -0700 Subject: [PATCH 1/4] Remove unused imports flagged by CodeQL Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- tests/mcp_test_tool.py | 1 - tests/test_rubrics_judge.py | 1 - thinkingbox/common/aoai_responses_session.py | 2 -- thinkingbox/common/llm_session_base.py | 2 -- thinkingbox/common/testrunner.py | 1 - 5 files changed, 7 deletions(-) diff --git a/tests/mcp_test_tool.py b/tests/mcp_test_tool.py index 7d89c11..2298117 100644 --- a/tests/mcp_test_tool.py +++ b/tests/mcp_test_tool.py @@ -4,7 +4,6 @@ import argparse import enum -import json import os import sys from pathlib import Path diff --git a/tests/test_rubrics_judge.py b/tests/test_rubrics_judge.py index 64a8da3..431e3fb 100644 --- a/tests/test_rubrics_judge.py +++ b/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, ) diff --git a/thinkingbox/common/aoai_responses_session.py b/thinkingbox/common/aoai_responses_session.py index f936217..388b747 100644 --- a/thinkingbox/common/aoai_responses_session.py +++ b/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, diff --git a/thinkingbox/common/llm_session_base.py b/thinkingbox/common/llm_session_base.py index 422e105..f389287 100644 --- a/thinkingbox/common/llm_session_base.py +++ b/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 diff --git a/thinkingbox/common/testrunner.py b/thinkingbox/common/testrunner.py index 626b3a5..3aa0492 100644 --- a/thinkingbox/common/testrunner.py +++ b/thinkingbox/common/testrunner.py @@ -6,7 +6,6 @@ import inspect import io import json -import logging import string import sys import traceback From ed119467e661522c08c272b0420ecb61c521ce09 Mon Sep 17 00:00:00 2001 From: Liang-Chun Tsai Date: Tue, 18 Aug 2026 15:28:56 -0700 Subject: [PATCH 2/4] Fix AOAI test patch targets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- tests/test_aoai_session.py | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/tests/test_aoai_session.py b/tests/test_aoai_session.py index e9c912b..bc83472 100644 --- a/tests/test_aoai_session.py +++ b/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()) From 85a736f31e0dfca6fd32918dda2f5d158cf9957d Mon Sep 17 00:00:00 2001 From: Liang-Chun Tsai Date: Tue, 18 Aug 2026 15:46:36 -0700 Subject: [PATCH 3/4] Resolve remaining Python CodeQL alerts Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c685bf42-ecb9-40bc-aaaf-dfe2d501bfd4 --- scripts/dataset_utils/filter_tc_benchmark.py | 2 +- scripts/import_typesense_snapshots.py | 5 ++++- tests/test_session_proxy.py | 14 +++++++------ tests/test_user_simulated_answer.py | 22 +++++++++++++------- thinkingbox/cli/infer.py | 1 - thinkingbox/common/agent_session.py | 2 -- thinkingbox/common/agent_user_loop.py | 1 + thinkingbox/common/http_client.py | 2 -- thinkingbox/tools/client/worker.py | 1 + thinkingbox/tools/session_proxy.py | 10 ++++----- 10 files changed, 33 insertions(+), 27 deletions(-) diff --git a/scripts/dataset_utils/filter_tc_benchmark.py b/scripts/dataset_utils/filter_tc_benchmark.py index a79a2d2..18d0420 100644 --- a/scripts/dataset_utils/filter_tc_benchmark.py +++ b/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 diff --git a/scripts/import_typesense_snapshots.py b/scripts/import_typesense_snapshots.py index 2faa125..f2d307a 100755 --- a/scripts/import_typesense_snapshots.py +++ b/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: diff --git a/tests/test_session_proxy.py b/tests/test_session_proxy.py index 029f31d..3ada706 100644 --- a/tests/test_session_proxy.py +++ b/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 diff --git a/tests/test_user_simulated_answer.py b/tests/test_user_simulated_answer.py index 98437ce..5642c5a 100644 --- a/tests/test_user_simulated_answer.py +++ b/tests/test_user_simulated_answer.py @@ -226,23 +226,29 @@ async def test_user_simulator_sanitizes_done_marker(): assert "" not in sent_prompt -def test_user_simulator_uses_correct_prompt(): +@pytest.mark.asyncio +async def test_user_simulator_uses_correct_prompt(): """Test that correct system prompt is selected based on flag.""" 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) + sim_no_end = UserSimulator( + llm=MockSession(completions=[[Text(role="assistant", content="test")]]), + can_end_conversation=False, + ) + sim_with_end = UserSimulator( + llm=MockSession(completions=[[Text(role="assistant", content="test")]]), + can_end_conversation=True, + ) - # With ending enabled - sim_with_end = UserSimulator(llm=mock_llm, can_end_conversation=True) + await sim_no_end.generate([], "context") + await sim_with_end.generate([], "context") - # Verify prompts are different assert SYSTEM_PROMPT != SYSTEM_PROMPT_WITH_END + assert sim_no_end.history[0][0].content == SYSTEM_PROMPT + assert sim_with_end.history[0][0].content == SYSTEM_PROMPT_WITH_END assert "" in SYSTEM_PROMPT_WITH_END assert "ENDING THE CONVERSATION" in SYSTEM_PROMPT_WITH_END assert "ENDING THE CONVERSATION" not in SYSTEM_PROMPT diff --git a/thinkingbox/cli/infer.py b/thinkingbox/cli/infer.py index 3cddecc..a63308b 100644 --- a/thinkingbox/cli/infer.py +++ b/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") diff --git a/thinkingbox/common/agent_session.py b/thinkingbox/common/agent_session.py index a049e89..2e2816b 100644 --- a/thinkingbox/common/agent_session.py +++ b/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() diff --git a/thinkingbox/common/agent_user_loop.py b/thinkingbox/common/agent_user_loop.py index acfb16a..0c073cd 100644 --- a/thinkingbox/common/agent_user_loop.py +++ b/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 diff --git a/thinkingbox/common/http_client.py b/thinkingbox/common/http_client.py index 128c977..1d9f57e 100644 --- a/thinkingbox/common/http_client.py +++ b/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 {} diff --git a/thinkingbox/tools/client/worker.py b/thinkingbox/tools/client/worker.py index 97bd0e4..d802923 100644 --- a/thinkingbox/tools/client/worker.py +++ b/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 diff --git a/thinkingbox/tools/session_proxy.py b/thinkingbox/tools/session_proxy.py index 3f217fb..8bd9463 100644 --- a/thinkingbox/tools/session_proxy.py +++ b/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] From fabaa757459c0461404a83cfbb95e864c92dcc01 Mon Sep 17 00:00:00 2001 From: Liang-Chun Tsai Date: Tue, 18 Aug 2026 15:58:56 -0700 Subject: [PATCH 4/4] Simplify user simulator prompt test Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: c685bf42-ecb9-40bc-aaaf-dfe2d501bfd4 --- tests/test_user_simulated_answer.py | 19 ++----------------- 1 file changed, 2 insertions(+), 17 deletions(-) diff --git a/tests/test_user_simulated_answer.py b/tests/test_user_simulated_answer.py index 5642c5a..4cbd136 100644 --- a/tests/test_user_simulated_answer.py +++ b/tests/test_user_simulated_answer.py @@ -226,29 +226,14 @@ async def test_user_simulator_sanitizes_done_marker(): assert "" not in sent_prompt -@pytest.mark.asyncio -async 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, ) - sim_no_end = UserSimulator( - llm=MockSession(completions=[[Text(role="assistant", content="test")]]), - can_end_conversation=False, - ) - sim_with_end = UserSimulator( - llm=MockSession(completions=[[Text(role="assistant", content="test")]]), - can_end_conversation=True, - ) - - await sim_no_end.generate([], "context") - await sim_with_end.generate([], "context") - assert SYSTEM_PROMPT != SYSTEM_PROMPT_WITH_END - assert sim_no_end.history[0][0].content == SYSTEM_PROMPT - assert sim_with_end.history[0][0].content == SYSTEM_PROMPT_WITH_END assert "" in SYSTEM_PROMPT_WITH_END assert "ENDING THE CONVERSATION" in SYSTEM_PROMPT_WITH_END assert "ENDING THE CONVERSATION" not in SYSTEM_PROMPT