#292 perf(mlx): idle-evict the classifier model to free ~7 GB when idle

[Anonymous]

Originally created by: Akarsh-Hegde

Problem

The MLX classifier model (Qwen3.5-9B-OptiQ-4bit) holds ~7.1 GB of Metal unified memory while resident, but classification is bursty (the daemon polls every 60s and only classifies when there's pending work). The server eager-loaded the model at startup and pinned it for the whole process life — so an otherwise-idle Mac carried ~7 GB indefinitely. This is the single largest contributor to Meridian's memory footprint.

A subtlety that matters for how you verify this: ps/Activity Monitor's per-process Memory column cannot see the model — it's Metal unified memory. They undercount it by ~6.5 GB. The honest number is mx.get_active_memory().

Change

Lazy-load the model on first request and evict it after MLX_IDLE_EVICT_S (default 120s) idle. del + gc.collect() + mx.clear_cache() reclaims the full 7.1 GB; cold reload is ~3–4s. A model_session() in-flight guard guarantees the evictor never frees the model mid-inference.

• run_task_linker_mlx.py — model_session() (in-flight tracking) + maybe_evict_idle() (the reclaim) + model_resident(); cache-miss load made atomic.
• server.py — drop the eager startup load; background idle-evictor task; wrap all four inference paths in model_session(); /info now reports model_resident + active_memory_gb (the only honest footprint).
• health/mlx.rs — meridian doctor now reports resident state + GB honestly instead of claiming "model loaded" after an eviction; mlx_ready comments corrected.
• docs — MLX_IDLE_EVICT_S in .env.example + CLAUDE.md.
• tests — TestModelEviction.

Tunable: raise MLX_IDLE_EVICT_S to keep the model warm longer; 0 pins it (old behavior).

Live evidence (mx.get_active_memory() vs ps)

   stage                     resident   mlx_active(REAL)   ps / Activity Monitor
   ─────────────────────────────────────────────────────────────────────────────
A  server up, no request      False        0.0 GB                71 MB   ← lazy: nothing loaded
B  POST /classify (load)      True         7.1 GB             1,724 MB   ← model resident
C  ~30s idle → evicted        False        0.0 GB               192 MB   ← 7.1 GB freed

Idle footprint of the classifier: 7.1 GB → ~0. Note how ps/Activity Monitor never shows the 7.1 GB and even lags the eviction — that's why meridian doctor//info now report mx.get_active_memory().

Daemon tolerance (the cold-load latency)

The first classify after an eviction now blocks ~4s for the reload. Verified safe:

Concern	Finding
Classify timeout	call_mlx_server → classification_timeout_s = 120s ≫ ~4s reload
Connect timeout (5s)	only covers the instant local TCP connect; load happens server-side after the request is accepted
Readiness gate	mlx_ready() keys on loaded_at (server-up), so an evicted model still reads "ready" → pipeline proceeds and lazy-loads — no skipped sessions, no deadlock

Test plan

• [x] Both Python modules + test file compile (py_compile)
• [x] Real-model run: 0 → 7.1 GB load → 0 GB evict; reclaim confirmed via mx.get_active_memory()
• [x] TestModelEviction assertions pass (in-flight tracking, TTL gating, disabled-when-0, no-op-when-in-flight, evict-when-idle)
• [x] Existing _get_model cache contract preserved (load-once + import-error path)
• [x] cargo check + cargo clippy clean; full pre-push suite (fmt, clippy, cargo test, UI build/tests, security audit) green

🤖 Generated with Claude Code

[Anonymous]

Originally posted by: adityaharishch

🔍 Code review — idle-evict the MLX classifier model

Reviewed the full diff plus the surrounding concurrency model (the Rust llm_gate, mlx_ready, and every _get_model/model_session call site). Strong PR — the design is correct and the memory win is real and well-evidenced. No blocking issues; findings below are one question + nits.

Verified correct (the parts that matter)

• Eviction can never free the model mid-inference. model_session() bumps _in_flight under the lock, before _get_model() runs, so even a cold load is covered; maybe_evict_idle() bails on _in_flight > 0. Confirmed all four production inference paths are wrapped (server.py:188/420/549, run_task_linker_mlx.py:748) and the only direct _get_model() is the one inside model_session itself — no unwrapped production use.
• Evictor never stalls inference — acquire(blocking=False) skips a tick instead of blocking, and runs in a threadpool so it never touches the event loop.
• Double-checked locking on the cache-miss load is correct (re-check under lock).
• mlx_ready semantics are unchanged — the code already keyed on loaded_at (server-up), so an evicted model still reads "ready" and the pipeline lazy-reloads. The reload (~4s) sits comfortably under the 120s classify timeout. The comment-only clarification is accurate.
• Honest reporting via mx.get_active_memory() + the autouse clear_model_cache fixture isolating _model_cache between tests.

Findings

1. (Question) The server's "no concurrent generation" invariant lives entirely outside the server. _in_flight gates eviction, not concurrency — N threadpool workers can call model(...) at once. The only thing preventing that is the daemon's single global llm_gate (one-permit semaphore, acquired by classify / worklog-synth / coding-agent-summarise). But non-gated callers bypass it: the eval harness DirectHttpStrategy, a manual curl, or /openai/chat/completions. Run any of those concurrently with the daemon and two generations overlap on the shared MLX model. This is pre-existing (the server never serialized), so not a regression — but since this PR newly documents the invariant in model_session's docstring, it's the natural moment to either (a) confirm nothing non-gated runs alongside the daemon, or (b) add a server-side Semaphore(1) around _generate so the server is self-safe regardless of caller. You explicitly chose not to add a second serialization point — reasonable — so flagging as confirm-or-harden, your call.

2. (Nit) active_memory_gb is process-wide, not model-specific. mx.get_active_memory() returns total active Metal memory for the process; the docstring calls it "the model's footprint." Fine as a proxy (the model dominates), but slightly imprecise — e.g. it'd include any transient allocation during a load. Consider "process Metal active memory (≈ the model when resident)".

3. (Nit) Very low MLX_IDLE_EVICT_S can thrash mid-burst. If the TTL drops below the gap between sessions within a classification burst, the model evicts + reloads (~4s) repeatedly. Default 120s is safe; a one-line caveat in the .env.example/CLAUDE.md note ("values below ~30s risk reload thrash between sessions") would steer tuners away from the footgun.

4. (Nit) MLX_IDLE_EVICT_S=0 still spawns a no-op evictor task. The lifespan creates the task unconditionally in the non-Apple branch; _idle_evictor then returns immediately when ttl <= 0. Harmless, but if _mlx._IDLE_EVICT_S > 0: around the create_task would avoid the throwaway task + cancel.

Cleared (so they aren't re-raised)

• "Eviction races a concurrent load" — false; _in_flight is incremented before _get_model(), so the evictor sees it busy.
• "model_session leaks _in_flight if _get_model() raises" — false; @contextmanager runs the finally as the generator unwinds, so the counter is balanced on load failure.
• "Tests leak _model_cache across cases" — false; the autouse clear_model_cache fixture clears it before and after each test.

_{🤖 Assisted review via Claude Code.}

[Anonymous]

Originally posted by: Akarsh-Hegde

✅ Conflict resolved + review nits addressed

Pushed c7e0844 (merge of main into this branch).

Merge conflict

One conflict, in services/agents/run_task_linker_mlx.py — the import block. main (via [#289]) added import datetime as _dt; this branch added import gc. Kept both, alphabetised. Verified post-merge that both symbols are still used (_dt in _local_day, gc in maybe_evict_idle) and that no function definitions were duplicated — [#289]'s plan-focus path (_fetch_plan_focus/_local_day/_fetch_pm_tasks(focus_keys=…)) and this PR's eviction path (model_session/maybe_evict_idle/model_active_memory_gb) coexist cleanly. Everything else auto-merged.

Review findings addressed

• Nit 4 — MLX_IDLE_EVICT_S=0 spawned a no-op evictor task. server.py now only create_task(_idle_evictor) when _IDLE_EVICT_S > 0, with a distinct "idle-eviction disabled" log line for the 0 case — no throwaway task to cancel on shutdown.
• Nit 2 — active_memory_gb docstring imprecise. model_active_memory_gb() docstring now says process-wide Metal active memory (≈ the model when resident — it dominates, though a transient load allocation can briefly inflate it) rather than implying a model-specific figure.
• Nit 3 — low-TTL thrash footgun. Added a caveat to both .env.example and the CLAUDE.md env table: avoid MLX_IDLE_EVICT_S below ~30s, since a TTL shorter than the gap between sessions in a classification burst causes repeated mid-burst evict+reload (~3s) thrash.

Findings reviewed, no change needed

• Finding 1 (concurrency question). The server-side "no concurrent generation" invariant is enforced by the daemon's single global llm_gate, which the prior fix commit (67f7fa1) already documented in model_session(). Keeping a single serialization point (no second server-side Semaphore(1)) — the deliberate choice flagged as "your call". Non-gated callers (eval DirectHttpStrategy, manual curl) remain a pre-existing consideration, not a regression of this PR.
• Bot — server.py:92 "statement has no effect". Already resolved by 67f7fa1, which rewrote shutdown to evictor.cancel() + await evictor under contextlib.suppress(CancelledError). Current line is an effectful await.
• Bot — mixed import style in test_run_task_linker_mlx.py. The flagged lines are this PR's new TestModelEviction, which already uses the file's dominant import agents.run_task_linker_mlx as m convention (44 occurrences). The minority from agents.run_task_linker_mlx import SessionClassification lines are pre-existing TestSessionClassificationSchema tests this PR doesn't touch — rewriting them would be out-of-scope churn, so left as-is.

Validation

cargo fmt --check, cargo clippy --all-targets -D warnings, cargo test, and UI tests all pass (pre-push suite). Python syntax verified; the eviction tests weren't modified.

_{🤖 Changes made via Claude Code.}

[Anonymous]

Ticket changed by: Akarsh-Hegde

• status: open --> closed

[Anonymous]

Originally posted by: adityaharishch

🎉 This PR is included in version 1.54.1 🎉

The release is available on:

• GitHub release
• v1.54.1
• npm package (@latest dist-tag)

Your semantic-release bot 📦🚀