AgentCanvas / Pages / Developer Guide / Core / Decisions / Eval / ADR-eval-002
2026-04-17 14:00
Date
2026-04-17 14:00
Status
accepted
Field
eval
Old ID
ADR-028

Context

Eval Phase 1 (ADR-eval-001) ships a working BatchEvalRunner (agent_loop/eval_batch.py:117) that runs a graph episode-by-episode through a single LoopRunner, sharing the canvas Play code path via BaseController (ADR-server-002). The two env nodesets in tree — EnvHabitatNodeSet (workspace/nodesets/server/habitat.py:1442) wrapping a single habitat.Env and EnvMP3DNodeSet (workspace/nodesets/server/matterport3d.py:2329) wrapping a single MP3DEnvManager (explicitly singleton at matterport3d.py:94) — both expose set_episode/reset_episode against one in-process simulator; neither uses habitat.VectorEnv or any vectorised binding. The eval loop is consequently sequential (for i in range(total) at eval_batch.py:176) and the API is single-tenant by construction (one _current_run slot at api/execution/eval.py:28, single-mutex ExecutionGuard at state.py:26). For a VLN method evaluated over the 1839 R2R-CE val_unseen episodes — the headline number every paper in the roadmap reports — wall-clock runs at one-episode-at-a-time even on a multi-GPU box; for AAS workflows (TODO F8/F9/F10) where each candidate graph needs a metric on the same split, this becomes the rate limiter for the entire research loop. Roadmap TODO F5 ("Eval Phase 2") names env reset between episodes and run comparison; TODO F7 names Docker server mode for heavy env nodesets — both presuppose the orchestration layer this ADR introduces. Two distinct gains are on the table: env-side throughput (run K episodes against K env subprocesses) and inference-side efficiency (don't load the policy/LLM K times — batch K samples through one model per step). They have to be designed together because the batching collector needs a stable batch key that only exists once the per-worker graph instances are addressable.

Decision

Six rules, landing across three phases (PA foundation → PB multi-worker env → PC batched inference). All six are decided here so that PB/PC can be implemented without re-opening the design.

(1) EnvWorkerPool between BatchEvalRunner and LoopRunner (PA-3, PB-2): a new agent_loop/env_worker_pool.py owns the episode queue and the N concurrent worker coroutines. BatchEvalRunner.execute() no longer iterates episodes itself — it builds a pool, hands it the graph + episode iterator + per-step budget, and await pool.run() returns the per-episode results. At worker_count=1 the pool is a single slot exercising the same code path as today. Each worker pulls the next episode from an asyncio.Queue, sets it via its own bound controller proxy, runs a fresh LoopRunner to completion, collects metrics, and pulls again. asyncio.gather over the N workers; stop_event.set() drains the queue gracefully so in-flight episodes finish but no new ones start.

(2) Per-LoopRunner controller_overrides binding (PA-1): LoopRunner.__init__ gains controller_overrides: dict[str, BaseController] | None = None. Any executor-side resolution that today calls the global get_controller(name) registry (components/controllers.py:241) now goes through a runner-scoped resolver that checks controller_overrides first, then falls back to the global registry for nodesets the pool didn't shard. Single-worker eval and canvas Play see no change because the override map is empty. Worker K passes {"env_habitat": tagged_proxy_K} so its LoopRunner only ever talks to its own env subprocess. Per-runner binding is preferred over a contextvars-based "current worker" magic — it survives nodes that spawn their own asyncio tasks without copying context, and it makes the binding visible at the runner construction site rather than implicit in task-local state.

(3) worker_count and per_step_budget_sec on EvalConfig (PA-2): EvalConfig (eval_batch.py:50) gains worker_count: int = 1 and per_step_budget_sec: float | None = None. When None, the budget resolves from a new default_per_step_budget_sec: ClassVar[float] on BaseNodeSet (default 5.0; Habitat declares 2.0, LLM-heavy nodesets like MapGPT declare 30.0). The per-episode timeout is asyncio.wait_for(LoopRunner.run, timeout=max_steps * per_step_budget_sec); an exhausted timeout marks the episode error with error="timeout", releases the worker, and the queue continues — one stuck worker can never block asyncio.gather. Worker count is fixed by the user via the eval form (and StartEvalV2Request), not auto-detected from GPU count, because the right N depends on whether the bottleneck is env stepping (wants N≈GPU count) or LLM I/O (wants N much higher) — the user knows their hardware and graph shape, the framework doesn't.

(4) ExecutionGuard stays single-mutex (no change): one batch eval run holds ExecutionGuard for the whole batch, all N workers run under that one holder, canvas Play returns 409 for the duration. No carve-out for "bulk" mode, no per-nodeset locking, no per-worker sub-holders. This deliberately keeps the guard's invariant ("at most one execution at a time") that the canvas Play path and BatchEvalRunner already rely on. Coexisting canvas Play during a batch eval is rejected as a v1 feature: it would force per-nodeset dependency analysis on every Play click and the UX win is small relative to the engineering cost.

(5) WorkspaceComponentRegistry spawns N tagged env-nodeset servers (PB-1): ensure_nodesets_for_graph gains a worker_count: int = 1 parameter. For nodesets the eval pool wants sharded (env nodesets — recognised today by their controller carrying eval metadata, see registry.get_eval_metadata_for_nodeset), it spawns N AutoServerApp subprocesses on N consecutive ports and registers N RemoteControllerProxy instances tagged "{name}#{k}" for k ∈ [0, N). Non-env nodesets (parse/format/control nodes) stay singleton in the main process — they're stateless across episodes and N copies of an LLM prompt formatter would be pure waste. Pool teardown stops all N subprocesses via the existing BaseServer.stop() path; orphan-process protection is the same subprocess.Popen lifecycle that single-worker server mode already uses (server/base_server.py:130). Spawning happens at EvalWorkerPool.__aenter__ time so a v1 batch run that doesn't use server-mode env nodesets pays no extra startup cost.

(6) Batched inference tier with batched/batch_dim ClassVar opt-in (PC-1, PC-2, PC-3): heavy GPU nodes (policy_cma, future local-vLLM bindings) opt into a shared model server instead of being duplicated per worker. Five sub-rules: (6a) BaseCanvasNode gains batched: ClassVar[bool] = False and batch_dim: ClassVar[str] = "" — author marks intent; default False = no behaviour change for any existing node. (6b) The batch key is (node.id, config_hash) — workers run the same graph JSON, so node.id "policy_forward_42" exists in W0..W(N-1) and is by construction the same logical operation; appending config_hash (sha of the resolved config dict) prevents accidental cross-batching when two policy nodes in one graph point at different checkpoints. The registry walks the graph at load: nodes with batched=True get their in-process handler swapped for a BatchedClient keyed by (node.id, config_hash) that submits inputs to a BatchedInferenceServer and awaits its slice. (6c) The server is pure-functional; the client carries state — RNN hidden states live with the worker (passed in/out via explicit hidden_in/hidden_out ports on the node), so BatchedInferenceServer never holds per-worker state and the batched forward pass is just model.forward(stack(samples), stack(hidden_ins)) → (stack(actions), stack(hidden_outs)). This deliberately avoids the trap of a stateful per-worker dict on the server, which would re-introduce the cross-worker coupling the worker pool is meant to eliminate. policy_cma__forward (workspace/nodesets/server/policy_cma.py) is the v1 user — its hidden state becomes explicit ports, which is also a cleanup the existing single-worker code already wanted (currently the state lives on _ServerContext at server/auto_server_app.py:33, an attribute-bag side channel). (6d) Flush trigger: batch_size==worker_count OR flush_timeout_ms elapsed (default 50 ms). Fixed-size flush alone would deadlock if any worker stalls or terminates early; pure timeout would sacrifice the batching win. The OR rule is the production-standard dynamic-batching shape (Triton, vLLM, TF-Serving). The timeout is per-batch-key, restarts on each new submission, and is configurable per node via the same default_per_step_budget_sec mechanism. (6e) AutoServerApp extension via a @batched_handler decorator instead of a separate BatchedModelServer base — keeps one server-mode story (one subprocess type, one manifest, one /health) rather than splitting it into batched-vs-not. The decorator marks a ServerFunction as batched; the server-side dispatcher accumulates calls per (node.id, config_hash) and flushes via the rule in (6d). At worker_count=1 the BatchedClient is functionally a no-op (queue of size 1 flushes immediately on every call), so single-worker behaviour is bit-identical to today.

Out of scope (explicit, deferred to follow-up ADRs): (i) Docker isolation for env subprocesses (TODO F7) — the subprocess.Popen boundary in (5) is designed so swapping in container spawn is a one-file change; not done here. (ii) Per-node parallelism / Pregel supersteps (TODO F3) — different problem, orthogonal axis. (iii) Eval Phase 2 run-comparison UI and replay viewer — separate feature. (iv) LLM-side dynamic batching via local vLLM — Phase C ships PyTorch policy batching only; LLM calls continue through litellm's existing request-level concurrency, which is already efficient because the upstream provider does its own server-side batching. (v) Multi-graph A/B in one batch — needs the run-registry refactor (today's _current_run global slot blocks it) and is its own design.

Alternatives

(a) Vectorised env inside the nodeset (habitat.VectorEnv or N MatterSim bindings sharing scenes) — rejected: forces a batch dim through every wire type touched by env outputs (IMAGE becomes (N,H,W,3), ACTION becomes (N,), STEP_RESULT becomes a list of N dicts), which contradicts ADR-dataflow-005's "wires move semantic items one-at-a-time" rule and breaks every existing node downstream of the env. The N-binding approach also doesn't scale to MP3D where MP3DEnvManager is explicitly singleton at matterport3d.py:94 and the C++ MatterSim binding doesn't cleanly support N independent simulators in one process. (b) Auto-pick worker_count from torch.cuda.device_count() — rejected: assumes Habitat-style env-per-GPU shape, bad fit for I/O-bound LLM-only graphs (MapGPT, NavGPT) where N=1 GPU could service N=8 workers because the bottleneck is OpenAI/Anthropic round-trip latency, not local compute. The user knows their bottleneck; a config field is honest. (c) contextvars-based per-worker controller resolution instead of controller_overrides on LoopRunner — rejected: a node that does asyncio.create_task(...) without copying context would silently route to the wrong worker's env, which is exactly the bug class the worker pool is meant to eliminate. Per-runner binding makes the wiring visible at the construction site. (d) Server-side per-worker state on BatchedInferenceServer (server holds hidden_states[worker_id]) instead of client-carried state via explicit ports — rejected: cross-worker coupling on the server (one worker's slow flush blocks another's read of its own hidden state); harder to checkpoint; harder to migrate workers across server restarts; and it leaks a "worker_id" concept into the server contract that should stay internal to the pool. Pure-functional server + explicit hidden-state ports also mean the same node can be swapped between batched and unbatched mode by toggling batched=True without changing its port surface. (e) Author-declared "model identity" string (e.g. model_id = "cma_v1") as the batch key, instead of (node.id, config_hash) — rejected: it asks the author to invent a global namespace and risks two unrelated graphs sharing a batch key by accident; node.id plus config_hash already gives the right scope (same graph, same config = same logical op) for free, with no author burden. (f) Eval and canvas coexist during a batch run via per-nodeset locks on ExecutionGuard — rejected: small UX win (canvas Play during a long eval is rare and confusing — the user expects the batch to be the only thing using the env), large engineering cost (per-Play dependency analysis against the active eval's nodeset set), and ExecutionGuard stops being a one-line invariant. Read-only canvas inspection during a run is already supported (no Play, just opening graphs and viewing logs).

Rationale

The system has two scarce resources — env subprocesses (CPU/GPU/memory bound by the simulator) and policy/LLM models (VRAM bound by the checkpoint). v1 batches both correctly: env subprocesses scale horizontally via the worker pool, models scale vertically via the batched inference tier. Splitting the design into Phase A / B / C is load-bearing — Phase A is a refactor that ships zero behaviour change at worker_count=1, which means it can land and bake before Phase B introduces concurrency, and Phase B can validate the worker-isolation invariants (one env per worker, no controller bleed) before Phase C adds the cross-worker rendezvous of the batched inference server. The (node.id, config_hash) batch key is the smallest decision in the ADR but the load-bearing one for batched inference: it answers the question "which calls collapse into one forward pass" without inventing a new namespace, and it falls out of an invariant that already exists (workers run the same graph JSON). The pure-functional server with client-carried hidden state is the second load-bearing decision: it keeps the server stateless, which means the batched inference tier can be horizontally scaled later (a pool of model servers, not just N workers behind one server) without re-designing the contract. The ExecutionGuard non-decision is a non-decision deliberately — every line of code that this ADR doesn't change is a line that doesn't need re-validating, and a single global mutex during eval is a property the canvas already assumed. The opt-in batched=True ClassVar means existing nodes are unaffected, and at worker_count=1 the BatchedClient is a no-op, so the code paths converge: a v1 user who just sets worker_count=4 sees env throughput improve linearly; a v1 user who additionally sets batched=True on policy_cma sees VRAM stay flat. The two TODOs this directly progresses (F5 worker-side, F7 Docker-side) line up cleanly: F7 becomes a one-file swap of the spawn boundary in rule (5), F5's "env reset between episodes" is solved by the fresh-LoopRunner-per-episode + tagged-proxy invariant in rule (1)+(2). The two TODOs it doesn't progress (F3 per-node parallelism, run-comparison UI) are out of scope on purpose — orthogonal axes that would muddy the design if folded in.

Affected docs

tutorials/batch-eval.md (new — author guide for worker_count, batched=True, per-worker debugging), core/roadmap.md (mark F5 partial: worker pool ✓, run comparison UI ✗; mark F7 unblocked, design ready), core/glossary.md (add EnvWorkerPool, BatchedInferenceServer, BatchedClient, controller_overrides rows), core/codebase-map.md (add agent_loop/env_worker_pool.py, server/batched_inference.py), .claude/PROJECT_OVERVIEW.md (next rebuild)

Follow-up: ADR-server-003 names the N-tagged-subprocess shape as one of two env deployment modes (replicated, the implicit default here) and adds shared as the pure-functional counterpart — reusing this ADR's Phase-C tier end-to-end.