test_serve_low_client's concurrency-cap assertion counts handler lifetime, not requests in flight, so it reads peak 3 against max_concurrency 2 under load

[localai-bot]

tests/tools/test_serve_low_client.py::test_request_shape_concurrency_cap_and_error_propagation fails under load with peak 3 <= 2. The defect is in the test's own instrument, not in the client's concurrency cap.

The mechanism

_CompletionHandler counts in-flight requests by incrementing a shared active on entry and decrementing it in a finally:

            self.wfile.write(body)
        finally:
            with self.lock:
                type(self).active -= 1

The decrement runs after the response body has been written. The client is free to receive that response and dispatch its next request before the finished handler reaches its finally, so active transiently counts a handler that has already answered. At max_concurrency=2 the recorded peak reads 3, and the assertion fails.

Nothing is actually over-dispatched: the client never has more than two requests outstanding. The instrument counts handler lifetime while the assertion is about requests in flight, and those two differ by exactly the window between the last write and the finally.

Why it is load-sensitive rather than flaky

The window is short and widens under scheduling pressure. Observed on a box carrying many concurrent sessions; the same case passed 8 of 8 in isolation and 4 of 4 on full-suite re-runs immediately afterwards.

Evidence that it is not a change under test

Found during the third repair round of PR #1776, which touches nothing under tools/ or tests/tools/. tests/tools/test_serve_low_client.py is blob 10220a8969d82a167075224c49ad8f1be131ac60 on both that branch and origin/main — byte-identical. The module imports only the standard library plus tools.bench.*.

Why it matters beyond one red

A load-sensitive assertion in a shared suite trains readers to re-run rather than read, which is how a real regression gets waved through. This repository already carries that cost on test_cpu_x86_llamacpp_floor (#618); a second case of the same shape compounds it.

Owed

• Count what the assertion is about. Decrement before the response is written, or have the client acknowledge completion, or assert on a counter the handler owns for its whole lifetime rather than mixing the two.
• Decide whether peak should be a strict bound at all, given the handler's lifetime necessarily exceeds the request's.

Filed by the operator rather than by the agent that found it: creating an issue is a remote write and that session had no recorded authority for one, so it reported the defect with its diagnosis instead of filing. That was the correct call.

[localai-bot]

Duplicate of #1605, which states the same diagnosis in its title — "peak counts the FIXTURE's handler window, not the client's in-flight window" — and of #1713, which adds the load thresholds (reds at 47, passes at 38) and the sharper framing that the observation is nondeterministic rather than load-thresholded.

Closing this and leaving those two as the record. The one thing here not already in them is the measurement that it is not the change under test: tests/tools/test_serve_low_client.py is blob 10220a8969d82a167075224c49ad8f1be131ac60 on both origin/main and PR #1776, and that branch touches nothing under tools/ or tests/tools/. Recording it here rather than appending to an issue that already says enough.

How this got filed, because the mechanism is the useful part. I ran the ownership check and it WORKED — both #1605 and #1713 printed. I filed anyway, because I had chained the duplicate check and gh issue create into a single shell invocation, so the check's output could not gate the action that followed it. The search was not the failure; the sequencing was.

That is the fourth ownership miss in this session and the first with this cause. The earlier three were: no check at all; a check whose remote search returned a false negative on an identifier-shaped term; and a check that read PR titles but not branch names. A check whose result cannot stop the next step is not a check. Run it, read it, then decide.