* Create a feature of Process-global LLM concurrency cap
* Added configuration of llm_call of max_concurrent_calls
* Classify limit_burst_rate and expose retry params via config.yaml
* refactor(middleware): encapsulate LLM concurrency state in a dataclass
Address PR #4294 review feedback (github-code-quality bot): the bare
module-level globals _GLOBAL_CONCURRENCY_LOOP / _GLOBAL_CONCURRENCY_LIMIT
were flagged as unused - a false positive, since both are read on the
recreate condition, but the `global`-declaration pattern tripped the
analyzer.
Replace the three globals + `global` declaration with a single
_ConcurrencyState dataclass singleton mutated in place. Behavior is
unchanged (lazy recreate when the running loop or configured limit
changes); the state is now co-located and no longer relies on bare
globals. dataclasses is already an established harness convention.
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(middleware): make LLM concurrency limiter process-wide + jitter burst retry
Addresses PR #4294 review (fancyboi999, CHANGES_REQUESTED) - two P1 issues.
P1 #1: the asyncio.Semaphore limiter was loop-bound, so it recreated per
event loop and the cap was NOT process-wide: lead-agent calls (main loop)
and subagent calls (the isolated persistent loop in subagents/executor.py)
each got their own semaphore, and the sync graph path (wrap_model_call)
bypassed the cap entirely. Recreating on loop/limit change also abandoned
permits held by the prior instance.
Replace it with a _ProcessWideLimiter built on threading primitives (not
loop-bound): one limiter shared across every event loop and both sync/async
wrappers. The cap is mutable via set_limit (never recreates, so in-flight
permits are never abandoned); permits release in finally and async waiters
unregister on cancellation, so cancellation never leaks capacity. Wire it
into wrap_model_call (sync) too - previously a direct handler() call.
P1 #2: the first (and only) burst-rate retry was deterministic at 5000ms.
prev_delay_ms was seeded from the 1000ms normal base, so for burst the
window collapsed to randint(5000, max(5000, 1000*3)) = randint(5000, 5000) -
a fleet that failed together realigned on the same 5s tick. Seed the first
retry from the reason-specific base (prev_delay_ms=None on loop init) so
the burst window is [burst_base, cap] = [5000, 8000], non-degenerate.
Retry-After is still honored verbatim.
Tests: rename semaphore tests -> limiter; add an autouse fixture resetting
the process singleton; add regressions the reviewer asked for - cross-loop
(lead + isolated-loop subagent), two concurrent sync calls, limit-change
while a permit is held (same instance, permit preserved), cancellation
no-leak, and burst first-retry non-degeneracy with default config (real
and seeded RNG) plus a concurrent de-synchronization case. Verified the
burst guard goes red on the old logic ({5000}) and green on the new.
Co-Authored-By: Claude <noreply@anthropic.com>
* Apply suggestions from code review
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* Potential fix for pull request finding 'Statement has no effect'
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
* fix(middleware): lossless limiter handoff, generation-aware cap, burst-rate CB gate
Address the open P1/P2 review findings on #4294:
- P1 #1 (cancellation handoff): reserve the permit for a specific waiter at
dequeue time (grant-at-dequeue, _AsyncWaiter.granted) so a waiter cancelled
in the post-dequeue / pre-reacquire window hands its reservation to the next
waiter (_handoff_granted_permit_locked) instead of stranding it. No
cancellation window remains.
- P1 #2 (hot-reload generation): move cap updates out of the per-attempt path;
give the limiter one generation-aware owner (set_limit_if_newer with a
monotonic instance seq proxy for config freshness) so a stale in-flight run
cannot rewrite a freshly-lowered cap. max_concurrent_calls is now genuinely
hot-reloadable, resolving the reload-boundary inconsistency by option (b) -
no STARTUP_ONLY_FIELDS change (retry params truly hot-reload).
- P2 (circuit breaker): gate _record_failure on reason != "burst_rate" so
burst-rate (limit_burst_rate) exhaustion - a transient slope-throttle, not
"provider down" - does not trip the CB and fast-fail ALL calls.
- P3: clamp the jitter window to the cap before drawing (uniform spread
instead of piling at the cap); document the per-process / GATEWAY_WORKERS
cap semantics in config + the field description.
Tests: add the reviewer-requested regressions (cancel-after-dequeue handoff;
stale-instance-doesn't-overwrite-lowered-cap across sync + isolated-loop async;
burst_rate-exhaustion-doesn't-trip-CB sync + async). Each is red on the prior
buggy logic and green on the fix. _build_middleware now routes llm_call knobs
through AppConfig so __init__ applies the cap. 71 middleware tests pass; 212
across the blast radius (1 pre-existing skip).
Co-Authored-By: Claude <noreply@anthropic.com>
* fix(middleware): startup-only LLM concurrency cap; report effective retry budget
Addresses review feedback on #4294 (fancyboi999 CHANGES_REQUESTED on acfc7617):
P1 - the generation guard measured construction order, not config freshness, so
a stale AppConfig(cap=3) constructed after a fresher AppConfig(cap=1) could
restore the higher cap; and on a downscale 3->1 release() handed excess
permits to queued waiters, keeping in_flight pegged at the old cap. Replace the
pseudo-generation path with a startup-only cap: the first middleware __init__
resolves and freezes the cap; later instances (newer or older config) are
no-ops. No runtime cap mutation means no downscale race and no
freshness/construction-order race. Per-call gate is now `limiter is None` only,
so a reloaded instance with max_concurrent_calls=0 cannot silently drop the
frozen cap. Removes _owner_seq / set_limit_if_newer / _grant_to_queued_locked
/ _next_instance_seq; file 946 -> 926 lines.
P2 - burst-rate calls are capped at 2 attempts but the retry log line, the
llm_retry stream event max_attempts, and the user-facing message still used
self.retry_max_attempts (3), so the frontend showed 1/3 then stopped after
attempt 2. Thread the effective max_attempts (_max_attempts_for) through the
logger, _emit_retry_event, and _build_retry_message.
Also: document max_concurrent_calls as startup-only in the config field
description and config.example.yaml (prose only - the startup-only: prefix is
top-level AppConfig-field granularity and would mislabel the otherwise
hot-reloadable llm_call section / break the reload_boundary drift test).
Rewrite the cap-mutation tests for startup-only semantics; add P2 retry-budget
event tests (sync+async, teeth-verified red on the bug); fix bot nits (empty
except blocks -> gather(return_exceptions=True); bare await statements ->
assigned+asserted).
Co-Authored-By: Claude <noreply@anthropic.com>
---------
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
ac5fd46281