From 938129a7e79dbab130274189de1ad39e8cec0acd Mon Sep 17 00:00:00 2001 From: Stanley Phoong Date: Mon, 10 Aug 2026 16:08:26 -0700 Subject: [PATCH] fix(benchmark): don't reject an accuracy-only run for listing many endpoints An accuracy-only run that lists more than one endpoint fails during setup: Failed to connect to endpoint: 1 validation error for HTTPClientConfig Value error, num_workers (1) must be a multiple of the number of endpoint URLs (4) ... Got remainder 1. and exits 3 before any dataset is planned or scored. Two forced choices collide. setup_benchmark pins num_workers=1 and max_connections=1 for every TestMode.ACC run, deliberately, so the compliance gate's single_stream assertion holds. HTTPClientConfig separately requires num_workers to divide the endpoint count so each endpoint gets equal workers. One worker cannot divide four endpoints, so the run is refused. What makes this a defect rather than a tight constraint is that the rejected client does no work. Both SWE-bench scorers set SKIP_ENDPOINT_PHASE, so no sample is ever issued through it -- the same run logs "Expected samples: 0" moments earlier. A validator is rejecting a configuration on behalf of a component that never runs, and it takes the whole run down with it. Give the idle client a single endpoint when the run will issue nothing, so the divisibility invariant still means what it says for runs that do issue. Scorers that fan work out across endpoints themselves read the endpoint list from the run's config.yaml rather than from this client, so this does not narrow the run. The proper fix is to skip building an issuer at all when nothing will be issued. That requires a null issuer type, because BenchmarkSession takes a non-None issuer, and is a larger change than this defect warrants on its own; endpoints[:1] is the narrow form of it, not the intended end state. Only reachable with more than one endpoint in accuracy-only mode: a single-endpoint run divides exactly and never surfaces it. Tests: `TestAccuracyOnlyIdleIssuer` covers the failing case (four endpoints, one worker, zero samples -- fails against the previous code) plus the two boundaries it must not disturb: a single-endpoint accuracy run, and an accuracy run that will actually issue, which still receives every endpoint. docs/evaluation/DESIGN.md records why the idle issuer exists and how narrowly this applies. --- docs/evaluation/DESIGN.md | 15 ++++ .../commands/benchmark/execute.py | 19 +++++ tests/unit/commands/test_benchmark.py | 82 +++++++++++++++++++ 3 files changed, 116 insertions(+) diff --git a/docs/evaluation/DESIGN.md b/docs/evaluation/DESIGN.md index 1074c343d..245ea592b 100644 --- a/docs/evaluation/DESIGN.md +++ b/docs/evaluation/DESIGN.md @@ -93,6 +93,21 @@ Code execution cannot be done safely in-process. The evaluation server runs in a with resource limits. This is a deliberate architecture choice — not a shortcut — and is documented prominently in the dataset README. +**An externally-scored accuracy-only run builds an idle issuer** + +A scorer with `SKIP_ENDPOINT_PHASE` evaluates through its own service, so the run +issues no samples and `total_samples` is zero. `BenchmarkSession` still requires a +non-None issuer, so one is built and never used. Accuracy-only runs are pinned to +`num_workers=1` for deterministic single-stream ordering, and `HTTPClientConfig` +requires `num_workers` to divide the endpoint count, so that idle client is handed a +single endpoint. This is scoped strictly to the zero-sample case: a run that will +issue keeps the full endpoint list and the divisibility invariant applies to it +unchanged. + +Scorers that themselves fan work across several endpoints read the endpoint list from +the run's `config.yaml`, not from this client, so the narrowing does not narrow the +run. + ## Integration Points | Component | Role | diff --git a/src/inference_endpoint/commands/benchmark/execute.py b/src/inference_endpoint/commands/benchmark/execute.py index a6ad406a5..3ac96dd65 100644 --- a/src/inference_endpoint/commands/benchmark/execute.py +++ b/src/inference_endpoint/commands/benchmark/execute.py @@ -705,6 +705,25 @@ async def _create_issuer( """Create the HTTP endpoint client + sample issuer, or raise SetupError.""" config = ctx.config endpoints = config.endpoint_config.endpoints + if ctx.accuracy_only and ctx.total_samples == 0: + # This client will not issue a single sample: every accuracy dataset is + # scored externally (Scorer.SKIP_ENDPOINT_PHASE), which is what makes + # total_samples zero. It is built only so the session has an issuer. + # + # It still has to satisfy HTTPClientConfig, which requires num_workers + # to divide the endpoint count -- and accuracy-only runs are forced to + # num_workers=1 for deterministic ordering, so any run listing more than + # one endpoint is rejected before it starts. Hand the idle client a + # single endpoint so the invariant holds; it drives none of them. + # + # Scorers that fan out across endpoints themselves (swe_bench_fleet) + # read the real endpoint list from the run's config.yaml, not from this + # client, so narrowing it here does not narrow the run. + # + # The proper fix is to not build an issuer at all when nothing will be + # issued; that needs a null issuer, because BenchmarkSession requires a + # non-None one. This is the narrow version of that change. + endpoints = endpoints[:1] logger.info(f"Connecting: {endpoints}") try: api_type: APIType = config.endpoint_config.api_type diff --git a/tests/unit/commands/test_benchmark.py b/tests/unit/commands/test_benchmark.py index a5ad0e6a4..c0b08ae83 100644 --- a/tests/unit/commands/test_benchmark.py +++ b/tests/unit/commands/test_benchmark.py @@ -3559,3 +3559,85 @@ def _run_audit(cfg, base_report_dir): benchmark_spy.assert_not_called() assert audit_calls == [tmp_path / "audit"] + + +class TestAccuracyOnlyIdleIssuer: + """An accuracy-only run whose datasets are all scored externally. + + `setup_benchmark` pins num_workers=1 and max_connections=1 for every + TestMode.ACC run so the compliance gate's single_stream assertion holds, and + `HTTPClientConfig` separately requires num_workers to divide the endpoint + count. One worker cannot divide four endpoints, so listing several endpoints + took the whole run down during setup -- for a client that issues nothing. + """ + + def _ctx(self, tmp_path, endpoints: list[str], *, total_samples: int = 0): + config = OfflineConfig( + endpoint_config={"endpoints": endpoints}, + model_params={"name": "test-model"}, + datasets=[{"path": "test.jsonl"}], + settings=OfflineSettings(client=HTTPClientConfig(num_workers=1)), + ) + ctx = _make_benchmark_context(config, tmp_path, test_mode=TestMode.ACC) + return dataclasses.replace(ctx, total_samples=total_samples) + + async def _capture_endpoints(self, ctx) -> list[str]: + captured: list[str] = [] + + async def _create(http_config, loop): + captured.extend(http_config.endpoint_urls) + return MagicMock() + + with ( + patch.object(execute_mod.HTTPEndpointClient, "create", new=_create), + patch.object(execute_mod, "HttpClientSampleIssuer", MagicMock()), + ): + await execute_mod._create_issuer(ctx, asyncio.get_event_loop()) + return captured + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_many_endpoints_do_not_reject_an_idle_accuracy_run(self, tmp_path): + ctx = self._ctx( + tmp_path, + [f"http://engine-{i}:8000" for i in range(4)], + total_samples=0, + ) + + captured = await self._capture_endpoints(ctx) + + assert len(captured) == 1 + assert captured[0].startswith("http://engine-0:8000") + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_a_single_endpoint_run_is_unchanged(self, tmp_path): + ctx = self._ctx(tmp_path, ["http://engine-0:8000"], total_samples=0) + + captured = await self._capture_endpoints(ctx) + + assert len(captured) == 1 + + @pytest.mark.unit + @pytest.mark.asyncio + async def test_an_issuing_accuracy_run_still_gets_every_endpoint(self, tmp_path): + """The narrowing is scoped to a client that issues nothing. + + A run that will issue samples keeps the full endpoint list, so the + divisibility invariant still means what it says where it matters. + """ + endpoints = [f"http://engine-{i}:8000" for i in range(2)] + config = OfflineConfig( + endpoint_config={"endpoints": endpoints}, + model_params={"name": "test-model"}, + datasets=[{"path": "test.jsonl"}], + settings=OfflineSettings(client=HTTPClientConfig(num_workers=2)), + ) + ctx = dataclasses.replace( + _make_benchmark_context(config, tmp_path, test_mode=TestMode.ACC), + total_samples=8, + ) + + captured = await self._capture_endpoints(ctx) + + assert len(captured) == 2