[eejd]

Closes #9.

Re-cut (2026-10-19, owner option B of #1): this PR now targets the new local (fb47616d = upstream main 99967257 + UPSTREAM.md). The old local c7832e5e (v0.7.0 base) and the previous head of this branch b9f2ed8e are archived as tags archive/local-pre-recut-c7832e5 and archive/topic-no-batch-500-b9f2ed8 (all five old topic heads and the evidence integration commit are tagged archive/*; UPSTREAM.md records old -> new). local was moved with --force-with-lease=local:c7832e5e... after the tags were pushed; this branch with --force-with-lease=topic/no-batch-500:b9f2ed8e... (twice more over my own intermediate heads 385cac10 and 2d057436; both are now tagged archive/topic-no-batch-500-385cac1 and archive/topic-no-batch-500-2d05743: the evidence binaries were built from them).

Problem

mlxcel serve --no-batch answered every /v1/completions request with HTTP 500 Server busy: CachePool: max capacity (0) reached.

Cause

The legacy worker builds its scheduler with max_queue_depth = usize::MAX; BatchScheduler::with_config sized the pool as max_batch_size + max_queue_depth, which wraps to 0 in a release build (no overflow-checks), and CachePool::allocate refuses a dense-KV sequence at active.len() >= max_sequences. That is the only reachable wrapping sum on main 99967257 (grep over src/: the other usize::MAX users saturate, compare or are test-only; PrefillQueue::with_capacity and CachePool::new allocate nothing). One more usize::MAX + shape exists at src/distributed/pipeline/serving.rs:903 (start + chunk_size), unreachable (ChunkedPrefillPipeline has no production caller); noted in #9, not changed here.

Fix

max_batch_size.saturating_add(max_queue_depth). Test pool_capacity_tests (scheduler built with an unbounded queue depth must keep pool().max_sequences() == usize::MAX; the bounded case stays 1 + 8). request_memory_tests::scheduler() now delegates to scheduler_with_queue_depth(n).

Evidence

• Test-fast build, cargo test -p mlxcel --lib pool_capacity_tests -- --test-threads=1: before (fix reverted) an_unbounded_queue_depth_does_not_zero_the_cache_pool FAILED left: 0, right: 18446744073709551615; after 2/2 pass. (A first version of the test, driven through enqueue_request on the synthetic NemotronH model, passed before the fix because that model's sequence state is model-owned and skips the capacity check; replaced.)
• AC4 on a port-built destroot binary of local (MLX pin 243bdf09, mlx.metallib 1510 nax symbols; --no-batch, Llama-3.2-1B-4bit, /v1/completions N=33/200/267): 3/3 HTTP 200, text identical to mlx_lm at all three lengths; baseline installed port mlxcel 0.7.0: 3/3 HTTP 500. Full table: engine-fix(mlxcel): build and run upstream main against the sweep/abort/EOS/BOS matrices; decide the delivery route for the four fixes #1.
• The evidence binaries were built from 385cac10/2d057436 of this branch, whose non-test sources are identical to the head (the later commits differ in the pool_capacity_tests module only).

Review dispositions (RevMlxcelB, 2026-10-19): overall correct

• fix(server): --no-batch no longer wraps the cache-pool capacity to zero #14 correct (saturating_add is the only reachable wrapping sum; test fails without it): accepted, no change.
• Unreferenced evidence heads: tagged (above). Done.
• serving.rs:903 same pattern, unreachable: recorded on engine-fix(mlxcel): --no-batch returns HTTP 500 CachePool: max capacity (0) reached on every /v1/completions request #9 as an upstream-note item; no change here.
• UPSTREAM.md said the dated tag form: corrected in draft PR docs: UPSTREAM.md tag form v0.7.0-local01 #17 (plain commit on local, no force-push); the first tag is v0.7.0-local01.
• Six mlxcel-core tests that fail on bare main (not this change): diagnosis(mlxcel): six mlxcel-core --lib tests fail on bare upstream main 99967257 with NAX kernels (PrefillTrim / tile-padded widths) #16; the port deselects exactly those.

Review dispositions: pending (independent re-review)

[eejd]

Independent review (read-only; scratch builds under /developer/Workspaces/scratch/rv14, engine runs under the luigi lock, nothing pushed/merged/tagged). Reviewed head 50be7a89 onto local fb47616d.

Verdict: the fix and its test are correct; no blocking findings.

Fix correctness

• max_batch_size.saturating_add(max_queue_depth) (config.rs:92) is the only sum of this shape on main 99967257 (grep of max_queue_depth *[-+*] / usize::MAX *[-+] over src/; the other sentinel users either saturating_sub/checked_* (DRY window sampling.rs:2382, app.rs budget, cache_args.rs:229) or compare (state.rs:198 depth < max then +1)).
• Downstream of the saturated value: CachePool::max_sequences is read only by allocate_with_layout (active.len() >= max_sequences, cache.rs:6670, never true for usize::MAX) and by the new test; CachePool::new and PrefillQueue::with_capacity allocate nothing proportional to the argument. usize::MAX therefore means "no limit", which is what the legacy worker (model_worker.rs:1338, "unbounded") asks for. A user-supplied huge --max-queue-depth on the batched path goes through the same line and is covered too.
• Release vs debug: [profile.release] has no overflow-checks, and test-fast inherits it, so the unfixed + wraps to 0 in both (the 500 max capacity (0)). A dev/debug build would instead panic at scheduler construction ([INFERENCE]: standard Rust semantics, not exercised here). The fix is right for both.
• Not blocking, claim in the PR body ("main has no other sites"): src/distributed/pipeline/serving.rs:903 (start + self.chunk_size).min(..) has the same shape (chunk_size == usize::MAX when chunking is disabled; wraps for start > 0). Only tests construct ChunkedPrefillPipeline (no production caller in src/), so it is unreachable today. Worth an upstream note, not a change here.

Regression test

Built main+fix and re-ran only the one line reverted, same flags as the port (--profile test-fast --features metal,accelerate, -p mlxcel --lib pool_capacity_tests -- --test-threads=1):

• with fix: 2 passed;
• fix reverted (max_batch_size + max_queue_depth): an_unbounded_queue_depth_does_not_zero_the_cache_pool FAILED left: 0, right: 18446744073709551615; the bounded 1 + 8 case passes.
  So the test fails without the fix for the reason stated.

Behaviour on the new build (test-fast build of 50be7a89, not the port binary)

• bare main 99967257, --no-batch, /v1/completions N=200: HTTP 500 CachePool: max capacity (0) reached (defect reproduces on main).
• 50be7a89: --no-batch N=33/200/267 HTTP 200, text identical to mlx_lm; Llama-3.2-1B default-launch length sweep 18/18 identical to mlx_lm; stock snapshot EOS tokens from config: [128009, 128001, 128008], finish=tool_calls, 30 tokens, 3/3.

History / branches / UPSTREAM.md

• All 8 archive/* tags are annotated tags on eejd/rs-mlxcel; each dereferences (via gh api git/tags) to exactly the SHA in its name and in UPSTREAM.md (728d339d, c7832e5e, ef5c2c0a, 0c516210, 5ea5aeac, c3d5c649, b9f2ed8e, 03c24b30). Old local c7832e5e = v0.7.0 (ec9db8bf) + UPSTREAM.md only (diff: 1 file, +22). The four upstream commits named in the table (e6c9cda9, c480ed67, 7c3bece5, 5a5e73b4) are ancestors of 99967257 and their subjects match the stated PRs. git describe gives v0.7.0-364-g99967257; the MLX pin there is 243bdf09.
• Fork main = 99967257 at review start (gh api branches/main); local = main + exactly one docs-only commit (fb47616d, UPSTREAM.md, 1 file +38); the topic is not merged into local (PR is open, base local, mergeable, no CI configured). Upstream main has since moved to c40e2d59 (+4 commits: lablup#2448, lablup#2449, lablup#2453, lablup#2455; none touches config.rs, the CMake pin or Cargo.lock), so the fork main is 4 behind; that is drift, not a defect.
• Force-pushes of topic/no-batch-500: the first (over b9f2ed8e) is covered by archive/topic-no-batch-500-b9f2ed8 (v0.7.0 base; helper scheduler_pool_capacity + two pure-function tests; superseded: the new test goes through the real BatchScheduler::with_config, which is the better test). The intermediates 385cac10 and 2d057436 are still fetchable by SHA (gh api commits/<sha>) but unreferenced; I diffed their patches against 50be7a89: the non-test hunks (config.rs fix, request_memory_tests.rs) are identical, they differ only in the first pool_capacity_tests draft (an enqueue_request-driven test, which per the PR body passed without the fix, so nothing of value was lost). One housekeeping point: the evidence binaries cited in the PR body and in #1 were built from those two commits; unreferenced commits can be garbage-collected, so tag them (archive/topic-no-batch-500-385cac1, -2d05743) if that provenance should stay checkable.

Finding (outside the diff, in UPSTREAM.md on local, lines 4-7)

UPSTREAM.md states the dated tag v0.7.0.<YYYYMMDD>-local<NN> as the rule. docs/development-model.md (macports-ports-local, lines 284-285, 310) and ADR-0008 §2/§3 define the local tag as v<base>-local<NN> / version <base>-local<NN> with <base> = last upstream release local contains, and reserve <last-release>.<YYYYMMDD> for -devel (no local patches). The owner decision on the tag form is still open (see macports-ports-local#1403); if it goes to v0.7.0-local01, lines 4-7 of UPSTREAM.md need a follow-up commit on local (a plain commit, no force-push). Until then the sentence should read as a proposal.