fix(evals): use non-blocking sleep in async rate limiter

[Shailendra005]

Summary

RateLimiter.alimit in phoenix-evals calls AdaptiveTokenBucket.on_rate_limit_error, which runs a synchronous time.sleep(self.cooldown). Inside a coroutine this blocks the whole asyncio event loop for the cooldown duration, stalling every concurrent eval task whenever a single request is rate-limited.

This adds an async_on_rate_limit_error that uses await asyncio.sleep(self.cooldown) and awaits it from alimit. The synchronous limit path is unchanged.

This mirrors the existing server-side implementation in src/phoenix/server/rate_limiters.py, which already provides both a sync on_rate_limit_error and an async_on_rate_limit_error. The evals copy simply never got the async variant.

Why this is in scope

Per CONTRIBUTING.md, this is a "small reliability fix" (not a feature): ~30 lines of source plus a regression test, no API or behavior change to the public limit/alimit contract.

Testing

Added test_alimit_does_not_block_event_loop_during_cooldown: it decorates an always-rate-limited async function, runs a concurrent ticker coroutine, and asserts the ticker keeps advancing during the cooldown. It fails on the current code (blocking sleep starves the ticker) and passes with this change. Full test_rate_limiters.py suite passes; ruff check and ruff format --check are clean.

Fixes #15403

[github-actions]

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

[Shailendra005]

I have read the CLA Document and I hereby sign the CLA

[ehutt]

@Shailendra005 Thanks for this — good catch, and the fix is right. time.sleep inside a coroutine freezes the whole event loop for the full cooldown, which is exactly the bug. I like that you matched the existing pattern in src/phoenix/server/rate_limiters.py instead of inventing a new one.

I verified the fix works and that the new test really does fail without it. Two comments below on test coverage.

[ehutt]

Two small things with this test: it uses max_rate_limit_retries=0, so the retry loop never runs and the second changed call site (rate_limiters.py:311-313) is never tested — pytest --cov shows lines 306-314 as missed. And ticks >= 5 is a timing threshold that could get flaky on a busy CI runner.

This file already has an async_warp_time helper (line 53) for this. It lets you assert the real invariant — time.sleep is never called — and covers both call sites:

Suggested change

	async def test_alimit_does_not_block_event_loop_during_cooldown():
	class _RateLimitError(Exception):
	pass
	cooldown = 0.3
	limiter = RateLimiter(
	rate_limit_error=_RateLimitError,
	max_rate_limit_retries=0,
	initial_per_second_request_rate=1000,
	cooldown_seconds=cooldown,
	)
	@limiter.alimit
	async def always_rate_limited():
	raise _RateLimitError
	ticks = 0
	stop = asyncio.Event()
	async def ticker():
	nonlocal ticks
	while not stop.is_set():
	await asyncio.sleep(0.01)
	ticks += 1
	async def run_limited():
	try:
	await always_rate_limited()
	except RateLimitError:
	pass
	finally:
	stop.set()
	await asyncio.gather(ticker(), run_limited())
	# A blocking time.sleep in the cooldown would freeze the event loop and starve
	# the ticker. With a non-blocking await asyncio.sleep, the ticker keeps running
	# concurrently through the ~0.3s cooldown.
	assert ticks >= 5
	async def test_alimit_cooldown_never_blocks_the_event_loop():
	class _RateLimitError(Exception):
	pass
	limiter = RateLimiter(
	rate_limit_error=_RateLimitError,
	max_rate_limit_retries=1,
	initial_per_second_request_rate=1000,
	cooldown_seconds=5,
	)
	@limiter.alimit
	async def always_rate_limited():
	raise _RateLimitError
	# A blocking time.sleep in the cooldown would freeze the event loop.
	with async_warp_time(time.time()):
	with mock.patch("time.sleep") as mock_sync_sleep:
	try:
	await always_rate_limited()
	except RateLimitError:
	pass
	assert not mock_sync_sleep.called, "async path must not block with time.sleep"

I ran this — under 5ms instead of 0.3s, and it still fails if you revert alimit to the blocking call. Same shape as the existing server test in tests/unit/server/test_rate_limiters.py:435.

[ehutt]

Small gap: the early-return guard here isn't covered by the new test (coverage reports line 126 missed). This is the branch that stops several concurrent failures from each halving the rate, so it's worth a quick test — call async_on_rate_limit_error twice in a row and assert the rate only halved once.