diff --git a/docs/AB_EXPERIMENTS.md b/docs/AB_EXPERIMENTS.md index 4efb0dde..ab2e4adb 100644 --- a/docs/AB_EXPERIMENTS.md +++ b/docs/AB_EXPERIMENTS.md @@ -130,7 +130,7 @@ From `ExperimentVariant` (`coder_eval/models/experiment.py`): | `initial_prompt_file` | str | Prompt replacement loaded from a file | | `run_limits` | block | Per-key cap overrides (`max_turns`, `task_timeout`, token/USD budgets) | | `driver` | `tempdir`/`docker` | Sandbox driver — enables tempdir-vs-docker arms | -| `checker_context` | dict | Backend/model override for the evaluation side (judge, simulator) — see [Checker Context](TASK_DEFINITION_GUIDE.md#checker-context); **not** currently `-D`-reachable | +| `checker_context` | dict | Backend/model override for the judge side (llm_judge, agent_judge) — has no bearing on the simulator; see [Checker Context](TASK_DEFINITION_GUIDE.md#checker-context); **not** currently `-D`-reachable | The `agent` dict is the lever for most A/B tests. Anything on `AgentConfig` is fair game: `model`, `permission_mode`, `allowed_tools`, `disallowed_tools`, diff --git a/docs/DIALOG_MODE.md b/docs/DIALOG_MODE.md index 622c3c3b..9fff5dc9 100644 --- a/docs/DIALOG_MODE.md +++ b/docs/DIALOG_MODE.md @@ -60,9 +60,11 @@ The mechanics: - The simulator is a **tools-disabled Claude Code agent** with `allowed_tools: []`, an explicit deny-list, and no plugins or settings sources. It is pure text-in / text-out, and it **cannot see the sandbox** — no files, no terminal, no agent reasoning. Only what the agent writes in the chat. -- The simulator runs on the run's resolved *evaluation* `ApiRoute` — the coding agent's own route - (`--backend direct` / `--backend bedrock`) unless `checker_context.api_route.route` overrides it - (see [Checker Context](TASK_DEFINITION_GUIDE.md#checker-context)). The **model** is separately +- The simulator resolves its own `ApiRoute` independently of `checker_context.api_route` (that + override is judge-only — see [Checker Context](TASK_DEFINITION_GUIDE.md#checker-context)) — same + resolution as the coding agent's own route (`--backend direct` / `--backend bedrock`), including + the pin to a constant Claude backend when the agent itself runs on an open-weight LiteLLM route. + The **model** is separately pinned by `simulation.model` (see [Simulation](TASK_DEFINITION_GUIDE.md#simulation)), not inherited from the route — so an A/B varying the subject model doesn't silently vary the simulated user too. diff --git a/docs/TASK_DEFINITION_GUIDE.md b/docs/TASK_DEFINITION_GUIDE.md index b08a9bf4..57151805 100644 --- a/docs/TASK_DEFINITION_GUIDE.md +++ b/docs/TASK_DEFINITION_GUIDE.md @@ -64,7 +64,7 @@ reference: { ... } # Optional reference solution (a directory pre_run: [ ... ] # Optional pre-run commands (before agent starts) post_run: [ ... ] # Optional post-run commands dataset: { ... } # Optional dataset fan-out (one task -> N row-tasks) -checker_context: { ... } # Optional: backend/model for the evaluation side (judge, simulator) +checker_context: { ... } # Optional: backend/model for the judge side (llm_judge/agent_judge) ``` ### `dataset` @@ -1308,11 +1308,11 @@ success_criteria: checker_context: api_route: - route: bedrock # which backend the whole eval side (llm_judge/agent_judge/simulator) uses + route: bedrock # which backend the eval side uses model: claude-haiku-4-5 # model override for that route ``` -- `route` selects which backend the WHOLE evaluation side calls (`llm_judge`, `agent_judge`, and the simulator all share one resolved eval route per run — this isn't per-criterion), **decoupled from the agent's own route** (a Claude agent can be graded by a differently-backed judge, or vice versa). This is a backend *name* (`direct` / `bedrock` / `litellm`), not a route object. For `direct`/`bedrock` credentials are never read from the task, always from the matching environment variables (`ANTHROPIC_API_KEY` for `direct`, `AWS_BEARER_TOKEN_BEDROCK`/`AWS_REGION` for `bedrock`). An unconfigured or unknown backend name raises at dispatch rather than silently falling back. **`route: litellm` dispatches `llm_judge` through the `litellm` library** (the `coder-eval[litellm]` extra, `litellm.acompletion`) rather than assuming one wire protocol — a gateway-routed judge model (e.g. an Azure AI `/openai/v1` deployment) rarely speaks Anthropic Messages, so this lets `model` carry its own provider hint (e.g. `azure/gpt-5.6-luna`) and get that provider's actual request/response shape handled by the library. Unlike the other two backends, `route: litellm` has NO implicit env-var fallback — see `params`/`env_params` below, which is how it's configured. `model` is required for `route: litellm` (there is no default open-weight/gateway model). +- `route` selects which backend `llm_judge` and `agent_judge` call — they share one resolved eval route per run for `direct`/`bedrock`/`litellm` (this isn't per-criterion) — **decoupled from the agent's own route** (a Claude agent can be graded by a differently-backed judge, or vice versa). The simulator is NOT part of this sharing: it always resolves its own route the same way the agent under test does (subprocess-safe by construction), independent of `checker_context.api_route` entirely — see the `route: litellm` note below. This is a backend *name* (`direct` / `bedrock` / `litellm`), not a route object. For `direct`/`bedrock` credentials are never read from the task, always from the matching environment variables (`ANTHROPIC_API_KEY` for `direct`, `AWS_BEARER_TOKEN_BEDROCK`/`AWS_REGION` for `bedrock`). An unconfigured or unknown backend name raises at dispatch rather than silently falling back. **`route: litellm` dispatches `llm_judge` ONLY** through the `litellm` library (the `coder-eval[litellm]` extra, `litellm.acompletion`) rather than assuming one wire protocol — a gateway-routed judge model (e.g. an Azure AI `/openai/v1` deployment) rarely speaks Anthropic Messages, so this lets `model` carry its own provider hint (e.g. `azure/gpt-5.6-luna`) and get that provider's actual request/response shape handled by the library. `agent_judge` spawns a real Claude Code CLI subprocess that only speaks Anthropic Messages — combining `route: litellm` with an enabled `agent_judge` criterion is rejected at resolution time (see below); the simulator is unaffected since it never reads this override. Unlike the other two backends, `route: litellm` has NO implicit env-var fallback — see `params`/`env_params` below, which is how it's configured. `model` is required for `route: litellm` (there is no default open-weight/gateway model). - `model` overrides the model that resolved route uses for **`llm_judge` only** — when the criterion itself leaves `model:` unset (precedence: an explicit per-criterion `model:` always wins; below that, `checker_context.api_route.model`; below that, the built-in `DEFAULT_JUDGE_MODEL`). This floor is deliberate and never the agent's own model — an unpinned judge must grade identically regardless of which model the agent under test is using, so `resolve_evaluation_route` never lets the agent's env-configured model (e.g. `BEDROCK_MODEL`) leak into `route.model` on its own; `route.model` is set only when this override was actually given. This works because every `ApiRoute` (`DirectRoute`/`BedrockRoute`/`LiteLLMRoute`) carries its own `model` field; the orchestrator bakes the override into the resolved route's `model` before any criterion runs, so `llm_judge` just reads `context.route.model` — it never reads `checker_context` directly. **`agent_judge` and the simulator do not honor this override** — `agent_judge`'s sub-agent model comes from the criterion's own `agent:` block (defaulted to a fixed judge model), and the simulator's model is pinned by `SimulationConfig.model` (see [Simulation](#simulation) below) — both independent of `checker_context.api_route.model` by design, for the same "measuring instrument stays fixed" reason. - `params`/`env_params` (**`route: litellm` only**) are how the call is actually configured — there is no fallback to the agent's own `LITELLM_BASE_URL`/`LITELLM_AUTH_TOKEN` env vars, since a gateway-routed judge model rarely reuses the agent's own LiteLLM proxy/credential. They also cover any of the dozens of other provider-specific kwargs `litellm.acompletion` accepts (`aws_access_key_id`, `vertex_project`, `api_version`, ...), which have no dedicated field on `LiteLLMRoute`: ```yaml @@ -1327,7 +1327,7 @@ checker_context: api_key: LITELLM_AUTH_TOKEN ``` `params` is merged straight into the `litellm.acompletion(**kwargs)` call — litellm validates the param names itself, so there's no allowlist to keep in sync here. `env_params` maps a kwarg name to the *name* of an environment variable; the value is resolved right before the call, so no secret is ever written into task/experiment YAML — this is how an arbitrary provider's config, including secrets (IAM keys, an Azure AD token, a service-account path, ...), is representable without a dedicated field per provider. `env_params` is resolved after `params`, so it always wins for the same key. Rejected at task-load time if given without `route: litellm`. For an Azure deployment, pin `api_version` via `params` to whatever API version the agent side is actually configured for (e.g. Codex's `CODEX_API_VERSION`) — the judge has no way to inherit it, and a mismatched version can hit a different shape of the same endpoint. - **`route: litellm` is `llm_judge`-only** — `agent_judge` and the simulator run as real Claude Code CLI subprocesses that speak the Anthropic Messages protocol, so pointing them at an arbitrary litellm-fronted gateway (which may speak an entirely different wire protocol) isn't representable. The orchestrator rejects the combination at resolution time (a clear error, not a silent misroute) if the task has an enabled `agent_judge` criterion or `simulation.enabled: true` alongside `route: litellm` — use `route: bedrock`/`direct` for those instead. + **`route: litellm` has no bearing on the simulator** — the simulator is a real Claude Code CLI subprocess that speaks the Anthropic Messages protocol, so pointing it at an arbitrary litellm-fronted gateway (which may speak an entirely different wire protocol) isn't representable. Rather than rejecting a task that combines `route: litellm` with `simulation.enabled: true`, the simulator always resolves its own route independently of `checker_context.api_route` — the same way the agent under test resolves its route — so `llm_judge` grades through the gateway named here while the simulator behaves exactly as if this override weren't set. A task can freely combine `route: litellm` with `simulation.enabled: true`; there is nothing to disable or remove. **`agent_judge` is NOT decoupled the same way** — it still shares the `llm_judge` eval route, and `agent_judge`'s Claude Code CLI subprocess has no way to honor a `LiteLLMRoute` safely, so the orchestrator rejects `route: litellm` combined with an enabled `agent_judge` criterion at resolution time (a clear error, not a silent misroute) — use `route: bedrock`/`direct` instead if a task needs both. `checker_context` merges shallow-per-namespace across `default_experiment.defaults.checker_context` → `experiment.defaults.checker_context` → `task.checker_context` → `variant.checker_context` (same 4-layer precedence as `agent`/`simulation`). So a judge-model A/B, or a judge-backend A/B, is a variant-level config change, not an edit to every task YAML. @@ -1613,7 +1613,7 @@ simulation: | `check_criteria` | `end_of_dialog` | `end_of_dialog`, `every_turn`, or `both`. | | `model` | `anthropic.claude-sonnet-4-6` | Model that plays the simulated user. Auto-translated to the run's backend (Bedrock inference profile / bare Anthropic alias), the same way [`llm_judge`](#llm_judge)'s `model` is. | -The simulator runs as a tools-disabled Claude Code agent on the run's resolved *evaluation* `ApiRoute` (the coding agent's own route unless [`checker_context.api_route.route`](#checker-context) overrides it), so temperature and sampling are resolved at the route level (same `-b` flag as the coding agent by default) and are not configured on this block. The **model is not**: it is pinned by `model` above. Inheriting it from the route meant `BEDROCK_MODEL` decided who the simulated user was, so an A/B varying the subject model silently varied its interlocutor too. Hold `model` fixed across variants for the same reason you hold a judge model fixed — the simulator is part of the measuring instrument, not the thing being measured. +The simulator runs as a tools-disabled Claude Code agent on its own resolved `ApiRoute` (the same resolution as the coding agent's own route — [`checker_context.api_route.route`](#checker-context) has no bearing on it), so temperature and sampling are resolved at the route level (same `-b` flag as the coding agent by default) and are not configured on this block. The **model is not**: it is pinned by `model` above. Inheriting it from the route meant `BEDROCK_MODEL` decided who the simulated user was, so an A/B varying the subject model silently varied its interlocutor too. Hold `model` fixed across variants for the same reason you hold a judge model fixed — the simulator is part of the measuring instrument, not the thing being measured. **Semantics:** diff --git a/src/coder_eval/models/tasks.py b/src/coder_eval/models/tasks.py index 75c2d866..68cfd324 100644 --- a/src/coder_eval/models/tasks.py +++ b/src/coder_eval/models/tasks.py @@ -91,7 +91,10 @@ class ApiRouteContext(BaseModel): route: ApiBackend | None = Field( default=None, - description="Backend the WHOLE evaluation side (llm_judge/agent_judge/simulator) calls.", + description=( + "Backend the judge side (llm_judge/agent_judge) calls. Has no bearing on the simulator, " + "which always resolves its own route independently." + ), ) model: str | None = Field(default=None, description="Model override for the resolved route.") params: dict[str, Any] | None = Field( @@ -491,8 +494,9 @@ class TaskDefinition(BaseModel): # noqa: CE009 -- soft-launch: see _warn_on_unk description=( "Task-authored config for the success-checking side. Currently carries only `api_route` " "(see ApiRouteContext), e.g. `{api_route: {route: litellm, model: gpt-5}}`: `route` selects " - "the backend the WHOLE evaluation side (llm_judge, agent_judge, the simulator) calls, " - "decoupled from the agent's own route; `model` overrides the model that route uses. Both " + "the backend the judge side (llm_judge, agent_judge) calls, decoupled from the agent's own " + "route; has no bearing on the simulator, which always resolves its own route independently. " + "`model` overrides the model that route uses. Both " "are consumed by the orchestrator (`resolve_evaluation_route`) BEFORE `CheckContext` is " "built and baked into the resolved route's own `model` field — no criterion ever reads " "`checker_context` directly, only `CheckContext.route.model`. Credentials are always " diff --git a/src/coder_eval/orchestrator.py b/src/coder_eval/orchestrator.py index 043df8e3..a2697bc0 100644 --- a/src/coder_eval/orchestrator.py +++ b/src/coder_eval/orchestrator.py @@ -419,7 +419,16 @@ def __init__( # API routing (initialized in _setup) self.route: ApiRoute | None = None - # Route for the evaluation side (llm_judge / agent_judge / simulated user): + # Route for the simulated user, a real Claude Code CLI subprocess like + # the agent under test. Resolved via resolve_evaluation_route(settings, + # self.route) with NO checker_context overrides — decoupled from + # checker_context.api_route (that override is llm_judge-only and has no + # bearing on the simulator) while still inheriting the LiteLLM-agent -> + # pinned-Claude-backend guard, since the simulated user is part of the + # measuring instrument and must not run on the agent's own open-weight + # gateway either. Equals self.route for the Direct/Bedrock backends. + self.simulator_route: ApiRoute | None = None + # Route for the evaluation side (llm_judge / agent_judge): # pinned to a constant Claude backend so grading stays comparable when the # agent runs on an open-weight (LiteLLM) model. Equals self.route for the # Direct/Bedrock backends. @@ -1463,8 +1472,9 @@ def _sync_sandbox_command_path_with_agent(self) -> None: def _eval_route_overrides(self) -> EvalRouteOverrides: """The ``(backend, model)`` pair from ``task.checker_context.api_route``, if any. - A task/variant-authored choice for the WHOLE evaluation side (``llm_judge``, - ``agent_judge``, the simulator all share one ``eval_route``), decoupled from + A task/variant-authored choice for the judge side (``llm_judge``, + ``agent_judge`` share one ``eval_route``; the simulator does NOT — see + ``simulator_route``), decoupled from the agent's own route/model — resolved into a credentialed ``ApiRoute`` (from env vars) by ``resolve_evaluation_route``, which bakes ``model`` into the resolved route's own ``model`` field. Reserved under one ``api_route`` @@ -1485,14 +1495,22 @@ def _eval_route_overrides(self) -> EvalRouteOverrides: ) def _resolve_routes(self) -> None: - """Resolve ``self.route``/``self.eval_route``, log routing, and build - ``self.success_checker``. Shared by the evaluate-only and normal setup - paths in ``_setup`` — identical logic, previously duplicated at each call - site. Requires ``self.sandbox`` to already be set. + """Resolve ``self.route``/``self.eval_route``/``self.simulator_route``, + log routing, and build ``self.success_checker``. Shared by the + evaluate-only and normal setup paths in ``_setup`` — identical logic, + previously duplicated at each call site. Requires ``self.sandbox`` to + already be set. """ assert self.sandbox is not None self.route = resolve_route(settings) overrides = self._eval_route_overrides() + # Decoupled from checker_context.api_route (no overrides passed) so the + # simulator never reads the litellm-judge-only knob, but still routed + # through resolve_evaluation_route (not aliased to self.route) so the + # LiteLLM-agent -> pinned-Claude-backend guard still applies to it: the + # simulated user is part of the measuring instrument and must not run on + # the agent's own open-weight gateway either. + self.simulator_route = resolve_evaluation_route(settings, self.route) self.eval_route = resolve_evaluation_route( settings, self.route, @@ -1501,37 +1519,34 @@ def _resolve_routes(self) -> None: params_override=overrides.params, env_params_override=overrides.env_params, ) - self._reject_litellm_eval_route_if_unsupported() + self._reject_litellm_agent_judge_if_unsupported() logger.info("API routing: %s", _format_routing(self.route, self.task.agent.model if self.task.agent else None)) self.success_checker = SuccessChecker(self.sandbox, route=self.eval_route) - def _reject_litellm_eval_route_if_unsupported(self) -> None: + def _reject_litellm_agent_judge_if_unsupported(self) -> None: """``checker_context.api_route.route: litellm`` dispatches ``llm_judge`` through the ``litellm`` library (protocol-agnostic — see ``invoke_litellm_judge_async``'s module docstring), but ``agent_judge`` - and the simulator run as real Claude Code CLI subprocesses that speak - the Anthropic Messages protocol only. Handing them a ``LiteLLMRoute`` - built from arbitrary ``params``/``env_params`` (which may front an - OpenAI-/Vertex-shaped gateway with no Anthropic-compatible endpoint at - all) would either misroute onto the AGENT's own unrelated LiteLLM - settings or fail with a confusing SDK-level error — the exact - misrouting ``LiteLLMRoute``'s own docstring says must never happen. - Reject the combination loudly at resolution time instead. + runs as a real Claude Code CLI subprocess that speaks the Anthropic + Messages protocol only. Handing it a ``LiteLLMRoute`` built from + arbitrary ``params``/``env_params`` would silently misroute it onto the + AGENT's own unrelated ambient LiteLLM proxy settings (``ClaudeCodeAgent`` + ignores ``LiteLLMRoute.params``/``env_params`` entirely and falls back + to ``settings.litellm_base_url``/``litellm_auth_token``) rather than + raising — the exact misrouting ``LiteLLMRoute``'s own docstring says + must never happen. Reject the combination loudly at resolution time + instead. The simulator doesn't need this check: ``simulator_route`` + never derives from ``checker_context.api_route`` in the first place. """ if not isinstance(self.eval_route, LiteLLMRoute): return - offenders: list[str] = [] if any(isinstance(c, AgentJudgeCriterion) and c.enabled for c in self.task.success_criteria): - offenders.append("an enabled agent_judge criterion") - if self.task.simulation is not None and self.task.simulation.enabled: - offenders.append("simulation.enabled") - if offenders: - named = " and ".join(offenders) msg = ( - f"checker_context.api_route.route: litellm is llm_judge-only (it dispatches through the " - f"litellm library in-process, not a real Claude Code subprocess), but this task also has " - f"{named}, which run as Claude Code sub-agents requiring an Anthropic-compatible endpoint. " - f"Use route: bedrock/direct instead, or remove/disable {named}." + "checker_context.api_route.route: litellm is llm_judge-only (it dispatches through the " + "litellm library in-process, not a real Claude Code subprocess), but this task also has " + "an enabled agent_judge criterion, which runs as a Claude Code sub-agent requiring an " + "Anthropic-compatible endpoint. Use route: bedrock/direct instead, or remove/disable the " + "agent_judge criterion." ) raise ValueError(msg) @@ -1545,19 +1560,28 @@ def _record_route_environment_info(self) -> None: assert self.result is not None assert self.route is not None self.result.environment_info["api_routing"] = ROUTE_NAMES[type(self.route)] - # The evaluation side (llm_judge / agent_judge / simulated user) may run on - # a different, constant backend — pinned to Claude when the agent is on - # LiteLLM — so record it: a run then shows what actually graded/simulated - # it, distinct from the agent's api_routing. + # The judge side (llm_judge / agent_judge) may run on a different, + # constant backend — pinned to Claude when the agent is on LiteLLM — so + # record it: a run then shows what actually graded it, distinct from the + # agent's api_routing. if self.eval_route is not None: self.result.environment_info["eval_routing"] = ROUTE_NAMES[type(self.eval_route)] # bedrock_model/litellm_model below are sourced from self.route (the - # AGENT's route) — record the judge/simulator's own model separately so - # a checker_context.api_route.model override (or the LiteLLM-agent + # AGENT's route) — record the judge's own model separately so a + # checker_context.api_route.model override (or the LiteLLM-agent # pinned-to-Bedrock default) is visible in run artifacts, not just # inferable from the agent's model. if self.eval_route.model: self.result.environment_info["eval_model"] = self.eval_route.model + # The simulator is pinned the same way eval_route is (LiteLLM agent -> + # constant Claude backend) but resolved independently of + # checker_context.api_route — record it separately so a run shows what + # the simulated user actually talked to, distinct from both api_routing + # and eval_routing above. + if self.simulator_route is not None: + self.result.environment_info["simulator_routing"] = ROUTE_NAMES[type(self.simulator_route)] + if self.simulator_route.model: + self.result.environment_info["simulator_model"] = self.simulator_route.model if isinstance(self.route, BedrockRoute): self.result.environment_info["aws_region"] = self.route.region if self.route.model: @@ -2212,10 +2236,11 @@ async def _simulation_dialog_loop(self, initial_prompt: str | None, sandbox_dir: config=sim_config, task_description=self.task.description, initial_prompt=initial_prompt, - # Pin the simulated user to the constant Claude eval route, not the - # agent's (possibly open-weight) route, so the simulator behaves - # identically across the models under test. - route=self.eval_route, + # simulator_route is resolved independently of eval_route/ + # checker_context.api_route (see _resolve_routes) — the simulator + # is a real Claude Code CLI subprocess, same as the agent under + # test, not a checker/judge concern. + route=self.simulator_route, ) await simulator.start() diff --git a/tests/test_litellm_route.py b/tests/test_litellm_route.py index 48256e19..087d6dc3 100644 --- a/tests/test_litellm_route.py +++ b/tests/test_litellm_route.py @@ -317,18 +317,21 @@ def test_rejects_non_string_model(self): class TestEvalRouteWiring: - """The orchestrator must hand the simulated user the eval_route (constant - Claude), never the agent's (possibly open-weight) route — guards the - simulation path the senior review flagged as untested.""" + """The orchestrator must hand the simulated user simulator_route — resolved + independently of eval_route/checker_context.api_route (that override is + llm_judge-only and has no bearing on the simulator, a real Claude Code CLI + subprocess) — and never the agent's (possibly open-weight) route — guards + the simulation path the senior review flagged as untested.""" - async def test_simulator_receives_eval_route_not_agent_route(self, monkeypatch): + async def test_simulator_receives_simulator_route_not_agent_or_eval_route(self, monkeypatch): from pathlib import Path from types import SimpleNamespace from coder_eval import orchestrator as orch_mod from coder_eval.orchestrator import Orchestrator - eval_route = BedrockRoute(region="eu-north-1", model="eu.anthropic.claude-sonnet-4-6") + eval_route = LiteLLMRoute(model="gpt-5.6-luna") # llm_judge-only override + simulator_route = BedrockRoute(region="eu-north-1", model="eu.anthropic.claude-sonnet-4-6") agent_route = LiteLLMRoute(model="zai.glm-5") captured: dict = {} @@ -347,13 +350,16 @@ async def start(self): agent=object(), success_checker=object(), eval_route=eval_route, + simulator_route=simulator_route, route=agent_route, ) with pytest.raises(RuntimeError, match="__stop_dialog__"): await Orchestrator._simulation_dialog_loop(fake, initial_prompt="hi", sandbox_dir=Path("/tmp")) - # If someone reverts to route=self.route this flips to the litellm route. - assert captured["route"] is eval_route + # If someone reverts to route=self.route this flips to the agent route; if + # someone reverts to route=self.eval_route this flips to the litellm override. + assert captured["route"] is simulator_route assert captured["route"] is not agent_route + assert captured["route"] is not eval_route class TestResolveRouteCustom: diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index 511464d1..a1fbeed1 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -6,6 +6,7 @@ import pytest +from coder_eval import orchestrator as orchestrator_module from coder_eval.models import ( AgentKind, BedrockRoute, @@ -105,75 +106,158 @@ def test_record_route_environment_info_direct_none_serialized_as_string(tmp_path assert orchestrator.result.environment_info["judge_transport"] == "none" -class TestRejectLitellmEvalRouteIfUnsupported: +class TestSimulatorRouteDecoupledFromCheckerContext: """checker_context.api_route.route: litellm dispatches llm_judge through the - litellm library in-process, but agent_judge and the simulator are real - Claude Code CLI subprocesses that speak Anthropic Messages only — handing - them an arbitrary litellm-fronted route would misroute or fail silently. - PR #137 review: 'route: litellm breaks the simulator and agent_judge.'""" + litellm library in-process. The simulator is a real Claude Code CLI + subprocess that speaks Anthropic Messages only — handing it an arbitrary + litellm-fronted route would misroute or fail silently. + + self.simulator_route is resolved independently of checker_context.api_route + (that override is judge-only) via resolve_evaluation_route(settings, + self.route) with no overrides — same call shape as eval_route, minus the + checker_context overrides. This decouples the simulator from the judge-only + knob while still inheriting the LiteLLM-agent -> pinned-Claude-backend + guard: the simulated user is part of the measuring instrument and must not + silently follow the agent onto its own open-weight gateway either. + + agent_judge is NOT decoupled the same way — it still shares eval_route, + so the litellm+agent_judge combination is still rejected outright (see + test_agent_judge_rejects_litellm_eval_route).""" @staticmethod - def _orchestrator_with_criteria(tmp_path: Path, success_criteria, simulation=None) -> Orchestrator: + def _orchestrator(tmp_path: Path, success_criteria, simulation=None, api_route=None) -> Orchestrator: + from coder_eval.models import CheckerContext + task_file = Path("tasks/hello_date.yaml") task, _ = load_task(task_file) - task = task.model_copy(update={"success_criteria": success_criteria, "simulation": simulation}) + checker_context = CheckerContext(api_route=api_route) if api_route is not None else None + task = task.model_copy( + update={ + "success_criteria": success_criteria, + "simulation": simulation, + "checker_context": checker_context, + "sandbox": SandboxConfig(driver="tempdir", python=None), + } + ) orchestrator = Orchestrator(task=task, run_dir=tmp_path / "run", variant_id="t") - orchestrator.eval_route = LiteLLMRoute(model="gpt-5.6-luna") + orchestrator.sandbox = Sandbox(task.sandbox, task_id=task.task_id) return orchestrator - def test_llm_judge_only_is_fine(self, tmp_path): - from coder_eval.models import LLMJudgeCriterion + @staticmethod + def _litellm_api_route(): + from coder_eval.models import ApiBackend, ApiRouteContext - orchestrator = self._orchestrator_with_criteria(tmp_path, [LLMJudgeCriterion(description="x", prompt="grade")]) - orchestrator._reject_litellm_eval_route_if_unsupported() # no raise + return ApiRouteContext(route=ApiBackend.LITELLM, model="gpt-5.6-luna") - def test_rejects_enabled_agent_judge(self, tmp_path): - from coder_eval.models import AgentJudgeCriterion + @pytest.fixture(autouse=True) + def _agent_on_direct_by_default(self, monkeypatch): + """Every test in this class puts the AGENT on DirectRoute unless it + explicitly overrides resolve_route itself (see + test_agent_on_litellm_pins_simulator_off_the_agents_own_gateway).""" + monkeypatch.setattr(orchestrator_module, "resolve_route", lambda _s: DirectRoute(judge_transport="anthropic")) - orchestrator = self._orchestrator_with_criteria( - tmp_path, [AgentJudgeCriterion(description="x", prompt="grade")] - ) - with pytest.raises(ValueError, match="agent_judge"): - orchestrator._reject_litellm_eval_route_if_unsupported() - - def test_allows_disabled_agent_judge(self, tmp_path): - from coder_eval.models import AgentJudgeCriterion + def test_llm_judge_only_resolves_litellm_eval_route(self, tmp_path, monkeypatch): + from coder_eval.models import LLMJudgeCriterion - orchestrator = self._orchestrator_with_criteria( - tmp_path, [AgentJudgeCriterion(description="x", prompt="grade", enabled=False)] + orchestrator = self._orchestrator( + tmp_path, [LLMJudgeCriterion(description="x", prompt="grade")], api_route=self._litellm_api_route() ) - orchestrator._reject_litellm_eval_route_if_unsupported() # no raise + orchestrator._resolve_routes() + assert isinstance(orchestrator.eval_route, LiteLLMRoute) + # simulator_route never derives from eval_route/checker_context at all, + # so it's unaffected regardless of whether this task even has a simulator. + assert not isinstance(orchestrator.simulator_route, LiteLLMRoute) - def test_rejects_enabled_simulation(self, tmp_path): + def test_enabled_simulation_never_gets_litellm(self, tmp_path, monkeypatch): from coder_eval.models import LLMJudgeCriterion, SimulationConfig - orchestrator = self._orchestrator_with_criteria( + orchestrator = self._orchestrator( tmp_path, [LLMJudgeCriterion(description="x", prompt="grade")], simulation=SimulationConfig(enabled=True, persona="p", goal="g"), + api_route=self._litellm_api_route(), ) - with pytest.raises(ValueError, match=r"simulation\.enabled"): - orchestrator._reject_litellm_eval_route_if_unsupported() + orchestrator._resolve_routes() # no raise -- this used to be rejected + assert isinstance(orchestrator.eval_route, LiteLLMRoute) + assert not isinstance(orchestrator.simulator_route, LiteLLMRoute) + + def test_agent_judge_rejects_litellm_eval_route(self, tmp_path, monkeypatch): + """agent_judge is NOT decoupled from eval_route the way the simulator + is — it still reads success_checker.route (== eval_route), which would + be an unsupported LiteLLMRoute if checker_context.api_route.route: + litellm is set. Rather than silently sharing it (the previous gap), + _resolve_routes rejects the combination loudly, same as before this + pass — this closes the gap the simulator-decoupling PR temporarily left + open (see _reject_litellm_agent_judge_if_unsupported).""" + from coder_eval.models import AgentJudgeCriterion - def test_allows_disabled_simulation(self, tmp_path): - from coder_eval.models import LLMJudgeCriterion, SimulationConfig + orchestrator = self._orchestrator( + tmp_path, [AgentJudgeCriterion(description="x", prompt="grade")], api_route=self._litellm_api_route() + ) + with pytest.raises(ValueError, match="an enabled agent_judge criterion") as excinfo: + orchestrator._resolve_routes() + # This combination is agent_judge-specific: the (now-moot) simulation + # branch of the old blanket guard must not resurface in the message. + assert "simulation" not in str(excinfo.value) + + def test_disabled_agent_judge_does_not_reject_litellm_eval_route(self, tmp_path, monkeypatch): + """The c.enabled guard on _reject_litellm_agent_judge_if_unsupported is + live: a disabled agent_judge criterion must not trip the rejection.""" + from coder_eval.models import AgentJudgeCriterion - orchestrator = self._orchestrator_with_criteria( + orchestrator = self._orchestrator( tmp_path, - [LLMJudgeCriterion(description="x", prompt="grade")], - simulation=SimulationConfig(enabled=False, persona="p", goal="g"), + [AgentJudgeCriterion(description="x", prompt="grade", enabled=False)], + api_route=self._litellm_api_route(), ) - orchestrator._reject_litellm_eval_route_if_unsupported() # no raise - - def test_non_litellm_eval_route_is_never_checked(self, tmp_path): - """Bedrock/Direct eval routes are always fine with agent_judge/simulation.""" - from coder_eval.models import AgentJudgeCriterion + orchestrator._resolve_routes() # no raise -- the criterion is disabled + assert isinstance(orchestrator.eval_route, LiteLLMRoute) + + def test_agent_on_litellm_pins_simulator_off_the_agents_own_gateway(self, tmp_path, monkeypatch): + """The core contract this decoupling must preserve: when the AGENT + itself resolves to a LiteLLMRoute (no checker_context involved at all), + the simulator must still be pinned to a constant Claude backend, not + silently handed the agent's own open-weight gateway route. Regression + guard for a real bug: aliasing simulator_route straight to self.route + (instead of routing it through resolve_evaluation_route) passes every + other test in this class but fails exactly this one.""" + from coder_eval.models import LiteLLMRoute as _LiteLLMRoute + from coder_eval.models import LLMJudgeCriterion - orchestrator = self._orchestrator_with_criteria( - tmp_path, [AgentJudgeCriterion(description="x", prompt="grade")] + orchestrator = self._orchestrator(tmp_path, [LLMJudgeCriterion(description="x", prompt="grade")]) + monkeypatch.setattr(orchestrator_module, "resolve_route", lambda _s: _LiteLLMRoute(model="zai.glm-5")) + monkeypatch.setattr(orchestrator_module.settings, "aws_bearer_token_bedrock", "token") + monkeypatch.setattr(orchestrator_module.settings, "aws_region", "eu-north-1") + orchestrator._resolve_routes() + assert isinstance(orchestrator.route, LiteLLMRoute) + assert not isinstance(orchestrator.simulator_route, LiteLLMRoute) + assert isinstance(orchestrator.simulator_route, BedrockRoute) + # eval_route and simulator_route both pin to the same Bedrock fallback + # here (no checker_context override on either), but are resolved via + # independent calls -- not aliased to each other. + assert isinstance(orchestrator.eval_route, BedrockRoute) + + def test_non_litellm_override_leaves_simulator_route_unaffected(self, tmp_path, monkeypatch): + """simulator_route is unaffected by any checker_context.api_route + override (bedrock/direct included) — it resolves independently of + overrides, always mirroring the agent's own backend (modulo the + LiteLLM pin), never the judge-only knob.""" + from coder_eval.models import AgentJudgeCriterion, ApiBackend, ApiRouteContext + + orchestrator = self._orchestrator( + tmp_path, + [AgentJudgeCriterion(description="x", prompt="grade")], + api_route=ApiRouteContext(route=ApiBackend.BEDROCK), ) - orchestrator.eval_route = BedrockRoute(region="eu-north-1") - orchestrator._reject_litellm_eval_route_if_unsupported() # no raise + monkeypatch.setattr(orchestrator_module.settings, "aws_bearer_token_bedrock", "token") + monkeypatch.setattr(orchestrator_module.settings, "aws_region", "eu-north-1") + orchestrator._resolve_routes() + assert isinstance(orchestrator.eval_route, BedrockRoute) + assert isinstance(orchestrator.simulator_route, DirectRoute) + assert orchestrator.simulator_route is not orchestrator.eval_route + # Same backend as self.route (DirectRoute), not the checker_context + # override (BedrockRoute) that eval_route picked up above. + assert orchestrator.simulator_route == orchestrator.route def test_record_route_environment_info_bedrock(tmp_path): diff --git a/tests/test_route_seam_exhaustiveness.py b/tests/test_route_seam_exhaustiveness.py index c3fe38e7..3d04d762 100644 --- a/tests/test_route_seam_exhaustiveness.py +++ b/tests/test_route_seam_exhaustiveness.py @@ -89,10 +89,14 @@ def test_record_route_environment_info_handles_every_route(): """Orchestrator._record_route_environment_info records a route-specific dimension for every route (not just the generic api_routing key). A new route missing from its if/elif would record only api_routing — caught here since - pyright can't check the isinstance chain.""" + pyright can't check the isinstance chain. Also covers the simulator_route -> + simulator_routing seam directly (not just via a shared alias with route).""" for r in _INSTANCES: - fake = SimpleNamespace(route=r, eval_route=r, result=SimpleNamespace(environment_info={}), agent=None) + fake = SimpleNamespace( + route=r, eval_route=r, simulator_route=r, result=SimpleNamespace(environment_info={}), agent=None + ) Orchestrator._record_route_environment_info(fake) # type: ignore[arg-type] env = fake.result.environment_info assert env.get("api_routing") == ROUTE_NAMES[type(r)] + assert env.get("simulator_routing") == ROUTE_NAMES[type(r)] assert len(env) > 1, f"{type(r).__name__} recorded no route-specific env info"