From 2161c6583894341526148d223a44cb81c6e2c85a Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Tue, 31 Mar 2026 16:14:09 -0600 Subject: [PATCH 1/2] test: add transport-wiring regression tests for #459 The existing test_client_limits_respect_max_parallel_requests only checks the limits property, which was always computed correctly even before the fix. Add mock-capture tests that patch HTTPTransport and AsyncHTTPTransport constructors, trigger lazy init via completion() / acompletion(), and assert the constructors received the correct limits. These tests fail on the pre-fix code (assert_called_once fails because the old code never explicitly constructed the transport). Made-with: Cursor --- .../clients/test_native_http_clients.py | 70 ++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py b/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py index 1e58f0c1..9c5f9b86 100644 --- a/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py +++ b/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py @@ -295,9 +295,14 @@ async def test_acompletion_lazy_initializes_async_client( # Connection pool size regression tests (issue #459) # --------------------------------------------------------------------------- +_HTTP_TRANSPORT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.HTTPTransport" +_ASYNC_HTTP_TRANSPORT_PATCH = ( + "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.AsyncHTTPTransport" +) + def test_client_limits_respect_max_parallel_requests() -> None: - """Connection pool limits must reflect max_parallel_requests (regression for issue #459). + """Connection pool limits must reflect max_parallel_requests. pool_max = max(32, 2 * max_parallel_requests) = max(32, 600) = 600 """ @@ -309,3 +314,66 @@ def test_client_limits_respect_max_parallel_requests() -> None: concurrency_mode=ClientConcurrencyMode.SYNC, ) assert client.limits.max_connections == 600 + + +@patch(_HTTP_TRANSPORT_PATCH) +@patch(_SYNC_CLIENT_PATCH) +def test_sync_pool_limits_forwarded_to_transport( + mock_client_cls: MagicMock, + mock_transport_cls: MagicMock, +) -> None: + """Regression for #459: limits must reach HTTPTransport, not just httpx.Client. + + The pre-fix code passed limits= to httpx.Client which silently ignores it + when a custom transport= is provided. The fix constructs HTTPTransport + with the correct limits before wrapping it in RetryTransport. This test + fails on the pre-fix code because HTTPTransport was never constructed + explicitly (assert_called_once fails). + """ + mock_client_cls.return_value = MagicMock( + post=MagicMock(return_value=mock_httpx_response(_make_openai_chat_response())) + ) + client = OpenAICompatibleClient( + provider_name=_OPENAI_PROVIDER, + endpoint=_OPENAI_ENDPOINT, + api_key="sk-test", + max_parallel_requests=300, + concurrency_mode=ClientConcurrencyMode.SYNC, + ) + client.completion(_make_chat_request(_OPENAI_MODEL)) + + mock_transport_cls.assert_called_once() + limits = mock_transport_cls.call_args.kwargs["limits"] + assert limits.max_connections == 600 + assert limits.max_keepalive_connections == 300 + + +@patch(_ASYNC_HTTP_TRANSPORT_PATCH) +@patch(_ASYNC_CLIENT_PATCH) +@pytest.mark.asyncio +async def test_async_pool_limits_forwarded_to_transport( + mock_client_cls: MagicMock, + mock_transport_cls: MagicMock, +) -> None: + """Regression for #459: limits must reach AsyncHTTPTransport for async clients. + + Same issue as the sync path — the pre-fix code never explicitly constructed + AsyncHTTPTransport, so RetryTransport created a default pool with 100 + connections regardless of max_parallel_requests. + """ + mock_client_cls.return_value = MagicMock( + post=AsyncMock(return_value=mock_httpx_response(_make_openai_chat_response())) + ) + client = OpenAICompatibleClient( + provider_name=_OPENAI_PROVIDER, + endpoint=_OPENAI_ENDPOINT, + api_key="sk-test", + max_parallel_requests=300, + concurrency_mode=ClientConcurrencyMode.ASYNC, + ) + await client.acompletion(_make_chat_request(_OPENAI_MODEL)) + + mock_transport_cls.assert_called_once() + limits = mock_transport_cls.call_args.kwargs["limits"] + assert limits.max_connections == 600 + assert limits.max_keepalive_connections == 300 From 02ccff3751814e380892f611589d60104566bb39 Mon Sep 17 00:00:00 2001 From: Nabin Mulepati Date: Wed, 1 Apr 2026 13:41:06 -0600 Subject: [PATCH 2/2] =?UTF-8?q?test:=20address=20review=20=E2=80=94=20para?= =?UTF-8?q?metrize=20over=20AnthropicClient,=20use=20helpers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Parametrize all three #459 regression tests over both OpenAICompatibleClient and AnthropicClient (wiring lives in HttpModelClient, so both subclasses need coverage). Use the existing _make_openai_client / _make_anthropic_client helpers with **kwargs instead of constructing clients directly. Move transport patch constants up with the other module-level constants. Made-with: Cursor --- .../clients/test_native_http_clients.py | 70 +++++++++++-------- 1 file changed, 42 insertions(+), 28 deletions(-) diff --git a/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py b/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py index 9c5f9b86..45125191 100644 --- a/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py +++ b/packages/data-designer-engine/tests/engine/models/clients/test_native_http_clients.py @@ -23,6 +23,10 @@ _ANTHROPIC_ENDPOINT = "https://api.anthropic.com/v1" _SYNC_CLIENT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.Client" _ASYNC_CLIENT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.AsyncClient" +_HTTP_TRANSPORT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.HTTPTransport" +_ASYNC_HTTP_TRANSPORT_PATCH = ( + "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.AsyncHTTPTransport" +) def _make_openai_client( @@ -30,6 +34,7 @@ def _make_openai_client( concurrency_mode: ClientConcurrencyMode = ClientConcurrencyMode.SYNC, sync_client: MagicMock | None = None, async_client: MagicMock | None = None, + **kwargs: Any, ) -> OpenAICompatibleClient: return OpenAICompatibleClient( provider_name=_OPENAI_PROVIDER, @@ -38,6 +43,7 @@ def _make_openai_client( concurrency_mode=concurrency_mode, sync_client=sync_client, async_client=async_client, + **kwargs, ) @@ -46,6 +52,7 @@ def _make_anthropic_client( concurrency_mode: ClientConcurrencyMode = ClientConcurrencyMode.SYNC, sync_client: MagicMock | None = None, async_client: MagicMock | None = None, + **kwargs: Any, ) -> AnthropicClient: return AnthropicClient( provider_name=_ANTHROPIC_PROVIDER, @@ -54,6 +61,7 @@ def _make_anthropic_client( concurrency_mode=concurrency_mode, sync_client=sync_client, async_client=async_client, + **kwargs, ) @@ -295,32 +303,44 @@ async def test_acompletion_lazy_initializes_async_client( # Connection pool size regression tests (issue #459) # --------------------------------------------------------------------------- -_HTTP_TRANSPORT_PATCH = "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.HTTPTransport" -_ASYNC_HTTP_TRANSPORT_PATCH = ( - "data_designer.engine.models.clients.adapters.http_model_client.lazy.httpx.AsyncHTTPTransport" -) +_POOL_LIMITS_CASES = [ + pytest.param(_make_openai_client, id="openai"), + pytest.param(_make_anthropic_client, id="anthropic"), +] +_SYNC_TRANSPORT_WIRING_CASES = [ + pytest.param(_make_openai_client, _OPENAI_MODEL, _make_openai_chat_response(), id="openai"), + pytest.param(_make_anthropic_client, _ANTHROPIC_MODEL, _make_anthropic_chat_response(), id="anthropic"), +] -def test_client_limits_respect_max_parallel_requests() -> None: +_ASYNC_TRANSPORT_WIRING_CASES = [ + pytest.param(_make_openai_client, _OPENAI_MODEL, _make_openai_chat_response(), id="openai"), + pytest.param(_make_anthropic_client, _ANTHROPIC_MODEL, _make_anthropic_chat_response(), id="anthropic"), +] + + +@pytest.mark.parametrize("client_factory", _POOL_LIMITS_CASES) +def test_client_limits_respect_max_parallel_requests(client_factory: Callable[..., Any]) -> None: """Connection pool limits must reflect max_parallel_requests. pool_max = max(32, 2 * max_parallel_requests) = max(32, 600) = 600 """ - client = OpenAICompatibleClient( - provider_name=_OPENAI_PROVIDER, - endpoint=_OPENAI_ENDPOINT, - api_key="sk-test", - max_parallel_requests=300, + client = client_factory( concurrency_mode=ClientConcurrencyMode.SYNC, + max_parallel_requests=300, ) assert client.limits.max_connections == 600 +@pytest.mark.parametrize(("client_factory", "model_name", "response_json"), _SYNC_TRANSPORT_WIRING_CASES) @patch(_HTTP_TRANSPORT_PATCH) @patch(_SYNC_CLIENT_PATCH) def test_sync_pool_limits_forwarded_to_transport( mock_client_cls: MagicMock, mock_transport_cls: MagicMock, + client_factory: Callable[..., Any], + model_name: str, + response_json: dict[str, Any], ) -> None: """Regression for #459: limits must reach HTTPTransport, not just httpx.Client. @@ -330,17 +350,12 @@ def test_sync_pool_limits_forwarded_to_transport( fails on the pre-fix code because HTTPTransport was never constructed explicitly (assert_called_once fails). """ - mock_client_cls.return_value = MagicMock( - post=MagicMock(return_value=mock_httpx_response(_make_openai_chat_response())) - ) - client = OpenAICompatibleClient( - provider_name=_OPENAI_PROVIDER, - endpoint=_OPENAI_ENDPOINT, - api_key="sk-test", - max_parallel_requests=300, + mock_client_cls.return_value = MagicMock(post=MagicMock(return_value=mock_httpx_response(response_json))) + client = client_factory( concurrency_mode=ClientConcurrencyMode.SYNC, + max_parallel_requests=300, ) - client.completion(_make_chat_request(_OPENAI_MODEL)) + client.completion(_make_chat_request(model_name)) mock_transport_cls.assert_called_once() limits = mock_transport_cls.call_args.kwargs["limits"] @@ -348,12 +363,16 @@ def test_sync_pool_limits_forwarded_to_transport( assert limits.max_keepalive_connections == 300 +@pytest.mark.parametrize(("client_factory", "model_name", "response_json"), _ASYNC_TRANSPORT_WIRING_CASES) @patch(_ASYNC_HTTP_TRANSPORT_PATCH) @patch(_ASYNC_CLIENT_PATCH) @pytest.mark.asyncio async def test_async_pool_limits_forwarded_to_transport( mock_client_cls: MagicMock, mock_transport_cls: MagicMock, + client_factory: Callable[..., Any], + model_name: str, + response_json: dict[str, Any], ) -> None: """Regression for #459: limits must reach AsyncHTTPTransport for async clients. @@ -361,17 +380,12 @@ async def test_async_pool_limits_forwarded_to_transport( AsyncHTTPTransport, so RetryTransport created a default pool with 100 connections regardless of max_parallel_requests. """ - mock_client_cls.return_value = MagicMock( - post=AsyncMock(return_value=mock_httpx_response(_make_openai_chat_response())) - ) - client = OpenAICompatibleClient( - provider_name=_OPENAI_PROVIDER, - endpoint=_OPENAI_ENDPOINT, - api_key="sk-test", - max_parallel_requests=300, + mock_client_cls.return_value = MagicMock(post=AsyncMock(return_value=mock_httpx_response(response_json))) + client = client_factory( concurrency_mode=ClientConcurrencyMode.ASYNC, + max_parallel_requests=300, ) - await client.acompletion(_make_chat_request(_OPENAI_MODEL)) + await client.acompletion(_make_chat_request(model_name)) mock_transport_cls.assert_called_once() limits = mock_transport_cls.call_args.kwargs["limits"]