fix(routing): decouple simulator route from checker_context.api_route

[akshaylive]

Summary

• Decouples the simulator's route resolution from checker_context.api_route: a task with simulation.enabled: true no longer needs to avoid an experiment-wide route: litellm default, since the simulator never read that override in the first place. This fixes a real false-positive (303/359 llm_judge tasks in a downstream skills-eval repo were broken by exactly this combination).
• Reinstates a narrow route: litellm + agent_judge rejection guard. The first commit removed the old blanket guard entirely to fix the simulator case above, which also silently reopened a real misrouting hole for agent_judge (it would silently run through the harness's own ambient LiteLLM proxy credentials instead of failing loudly). A full /coder-eval-code-review-full pass on this branch caught this — 6 of 8 review axes independently flagged it — and it's fixed here by restoring a guard scoped to just that combination.
• Minor cleanup: removes a redundant duplicate resolve_route(settings) call, and fixes two doc/comment spots left stale by the decoupling.

Test plan

• ruff check / ruff format --check clean
• pyright: 0 errors, 0 warnings (repo-wide)
• make lint (custom architectural rules): 376/376 pass
• Full pytest: 4752 passed, 6 skipped, 1 failed (pre-existing, unrelated live-SDK flake in test_claude_settings_enforcement_live.py, confirmed present on main too)
• New/updated route-resolution tests pass in isolation (153/153)

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com

[uipreliga]

Review: coder_eval — pr:144

Scope: pr:144 · branch akshaya/scope-litellm-route-to-llm-judge · c46c241 · 2026-08-28T00:33Z · workflow variant

Change class: simple — one focused routing-seam change in orchestrator.py (simulator route decoupled from checker_context.api_route, litellm rejection guard narrowed from agent_judge+simulation to agent_judge only), with tests and docs updated in lockstep; 5 files, +146/-92.

The codebase remains strong — security, error handling and harness quality are clean, types are near-perfect, and the architecture holds — but this change ships one score-changing regression (the simulated user now follows the agent's own open-weight LiteLLM route instead of the pinned Claude route at src/coder_eval/orchestrator.py:1507), and the surrounding test and prose surfaces do not catch or describe it, so the bottom line is: fix the pin plus its missing test before merge, and the remaining items are ordinary maintainability cleanup.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	9.4 / 10	0	0	1	1	Orchestrator.simulator_route is a write-once public alias of self.route with no override path — a speculative surface plus ~13 lines of comment across three sites
2. Type Safety	9.9 / 10	0	0	0	1	New simulator_route Optional is the only route the simulation seam does not narrow with an assert, and None is a meaningful value in UserSimulator
3. Test Health	8.4 / 10	0	1	1	1	The agent-on-LiteLLM branch — where the simulator now follows onto the gateway route instead of the pinned Claude eval route — is untested; mutation-restoring the old pinning leaves the full suite green
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	9.5 / 10	0	0	1	0	Route-seam exhaustiveness fake gained an inert simulator_route kwarg the function under test never reads, instead of covering the seam the PR actually makes reachable
6. Error Handling & Resilience	10 / 10	0	0	0	0	—
7. API Surface & Maintainability	6.5 / 10	1	0	1	0	simulator_route = self.route drops the pin-to-Claude guard: on the litellm backend the simulated user is sent to the agent's own open-weight gateway (wrong/absent model, plus the hidden evaluation prompt leaving to a third-party proxy)
8. Evaluation Harness Quality	10 / 10	0	0	0	0	—

Overall Score: 9.2 / 10 · Weakest Axis: API Surface & Maintainability at 6.5 / 10
Totals: 🔴 1 · 🟠 1 · 🟡 4 · 🔵 3 across 8 axes.

Blockers

1. [Axis 3] The agent-on-LiteLLM branch — where the simulator now follows onto the gateway route instead of the pinned Claude eval route — is untested; mutation-restoring the old pinning leaves the full suite green (tests/test_orchestrator.py:173) — All four new _resolve_routes tests neutralise the agent's own route with the same stub — monkeypatch.setattr(orchestrator_module, "resolve_route", lambda _s: DirectRoute(judge_transport="anthropic")) (tests/test_orchestrator.py:157, 173, 191, 205) — so self.route is a DirectRoute in every one of them. The behaviour this PR ships is self.simulator_route = self.route (src/coder_eval/orchestrator.py:1507): before the PR the simulator got eval_route, which resolve_evaluation_route deliberately PINS to Bedrock/Direct when the agent is on LiteLLM (src/coder_eval/models/routing.py:337-343, "Agent on LiteLLM (open-weight, no backend_override): the agent route cannot serve a Claude judge, so pin evaluation to Bedrock ... or Direct"). After the PR the simulated user runs on the agent's own LiteLLM/gateway route — a different model plays the interlocutor, so a dialog-mode task's transcript, and therefore its score, changes. That case is untested. Verified by mutation: restoring the old pinning for exactly that branch (self.simulator_route = self.route if not isinstance(self.route, LiteLLMRoute) else resolve_evaluation_route(settings, self.route)) leaves the FULL suite green — 4739 passed, 11 failed, identical to the unmutated baseline (the 11 are pre-existing ModuleNotFoundError: No module named 'litellm.types' / live-gateway failures). Add a test in TestSimulatorRouteDecoupledFromCheckerContext that stubs resolve_route to LiteLLMRoute(model="zai.glm-5") and asserts orchestrator.simulator_route is orchestrator.route and isinstance(orchestrator.simulator_route, LiteLLMRoute) while orchestrator.eval_route is the pinned Bedrock/Direct route — that is the assertion that pins the new contract and makes the scoring consequence explicit.
2. [Axis 7] simulator_route = self.route drops the pin-to-Claude guard: on the litellm backend the simulated user is sent to the agent's own open-weight gateway (wrong/absent model, plus the hidden evaluation prompt leaving to a third-party proxy) (src/coder_eval/orchestrator.py:1507) — _resolve_routes() now aliases the simulator to the AGENT's route:

        # Same resolution as self.route (subprocess-safe by construction) — not
        # yet independently overridable, so this is an alias rather than a
        # second call, until simulator_route grows its own override mechanism.
        self.simulator_route = self.route          # orchestrator.py:1507

and _simulation_dialog_loop passes it through (route=self.simulator_route, orchestrator.py:2229), replacing the previous route=self.eval_route.

Two distinct silently-changed behaviours result, both on realistic configs, with no runtime signal at all — no error, no deprecation warning, and no new environment_info key (_record_route_environment_info records only api_routing / eval_routing / eval_model; its own new comment at orchestrator.py:1559-1560 concedes "The simulator is NOT part of this"):

(1) An existing YAML with checker_context: {api_route: {route: bedrock}} and simulation.enabled: true, run against an agent on direct, previously drove the simulated user on BedrockRoute; it now drives it on DirectRoute. The YAML still parses and is still accepted — the knob is simply ignored for the simulator. A different simulated user produces a different dialog, and the agent is graded on that dialog, so score / final_status move for identical agent configuration.

(2) Worse, and unrelated to any knob: when the agent itself resolves to LiteLLMRoute (no checker_context at all), resolve_evaluation_route used to pin the simulator away from it — models/routing.py:365-372: # agent_route is LiteLLMRoute → pin evaluation to a constant Claude backend. … return BedrockRoute(region=settings.aws_region, ...). That pin is now gone: the simulator is handed the agent's LiteLLMRoute, and UserSimulator._resolve_model (simulation/user_simulator.py:246-254) falls into the non-Bedrock branch return to_anthropic_alias(model), shipping claude-sonnet-4-6 at the agent's open-weight LiteLLM proxy. Either the proxy does not serve it (simulator raises → simulator_failures → dialog terminates ERROR) or it serves something else — either way the measuring instrument now varies with the thing being measured, which is exactly what the surviving comment at orchestrator.py:1557-1558 ("pinned to Claude when the agent is on LiteLLM") and the guide's own "the simulator is part of the measuring instrument, not the thing being measured" (docs/TASK_DEFINITION_GUIDE.md:1616) exist to prevent. Note this is not a restoration of pre-#137 behaviour: git show 727bb7b^:src/coder_eval/orchestrator.py line 2130 already read route=self.eval_route, so the pin predates #137.

The justification the PR writes into the code and the docs for dropping the guard is false in case (2). orchestrator.py:428-431 asserts the simulator route is "subprocess-safe by construction", and docs/TASK_DEFINITION_GUIDE.md:1330 asserts "pointing it at an arbitrary litellm-fronted gateway … isn't representable" — but case (2) represents exactly that, via self.route instead of via checker_context.

Fix: keep the simulator pinned off an open-weight backend. Minimal correct form is self.simulator_route = resolve_evaluation_route(settings, self.route) (no checker_context overrides passed) — that decouples the simulator from checker_context.api_route as intended while preserving the LiteLLM→Claude pin, so _reject_litellm_agent_judge_if_unsupported's narrowing stays valid. Add a test for agent on LiteLLMRoute + simulation.enabled asserting simulator_route is not a LiteLLMRoute (no such test exists in the diff). Additionally, record simulator_routing in _record_route_environment_info so the change is auditable in run artifacts, and re-label the commit fix(routing)!: / add a BREAKING CHANGE: trailer so python-semantic-release (pyproject.toml:375-395) does not ship a documented-knob narrowing as a silent patch bump.

Non-blocking, but please consider before merge

1. [Axis 1] Orchestrator.simulator_route is a write-once public alias of self.route with no override path — a speculative surface plus ~13 lines of comment across three sites (src/coder_eval/orchestrator.py:428) — _resolve_routes sets it unconditionally to the value it already has: line 1507 self.simulator_route = self.route, and the only reader is line 2229 route=self.simulator_route. The attribute is therefore 100% derivable from existing state and has no independent producer. Its own comment concedes this twice — line 428-427: # Defaults to the same resolution as self.route (subprocess-safe by construction); intended to grow its own override mechanism later, unrelated to checker_context. and lines 1504-1506: # Same resolution as self.route (subprocess-safe by construction) -- not yet independently overridable, so this is an alias rather than a second call, until simulator_route grows its own override mechanism. That is textbook speculative generality, which CLAUDE.md forbids ("YAGNI: Don't add complexity until actually needed") and the shared rubric forbids ("no speculative features, no 'just in case' code"). The cost is not zero: it adds a nullable ApiRoute | None field, ~13 lines of comment across three sites (428-427, 1504-1506, 2225-2228), a hand-maintained invariant asserted in a comment at line 1561 (simulator_route always mirrors self.route) that nothing enforces, and a field that had to be threaded into two test fakes (tests/test_route_seam_exhaustiveness.py:94, tests/test_litellm_route.py:353) — where it has already drifted into a state production cannot produce. Recommendation: delete the attribute and pass route=self.route at orchestrator.py:2229 with the one-line rationale comment already written there; reintroduce a named seam in the PR that actually adds the override mechanism. If the named seam is kept for readability, at minimum make it a @property returning self.route so the mirror invariant is structural rather than a comment.
2. [Axis 3] test_allows_disabled_agent_judge deleted with no replacement — the and c.enabled conjunct of the surviving guard (src/coder_eval/orchestrator.py:1537) is unreachable by any test in the suite (tests/test_orchestrator.py:178) — The old class had test_allows_disabled_agent_judge (asserting AgentJudgeCriterion(..., enabled=False) + a LiteLLM eval route does NOT raise). The rewrite kept only the enabled case, test_agent_judge_rejects_litellm_eval_route (tests/test_orchestrator.py:178), and dropped the disabled one. The guard it protected is still live: if any(isinstance(c, AgentJudgeCriterion) and c.enabled for c in self.task.success_criteria): (src/coder_eval/orchestrator.py:1537). Verified by mutation: deleting and c.enabled from that line leaves the FULL suite green — 4739 passed, 11 failed, byte-identical to baseline — so a task carrying a disabled agent_judge alongside checker_context.api_route.route: litellm would start hard-failing at setup with nothing to catch it. grep -rn "enabled=False" tests/ | grep -i judge returns only tests/test_agent_judge_criterion.py:952,965 and tests/test_llm_judge_criterion.py:951, none of which reach _reject_litellm_agent_judge_if_unsupported. Re-add the disabled-agent_judge no-raise case to TestSimulatorRouteDecoupledFromCheckerContext. While there, tighten the raise assertion: pytest.raises(ValueError, match="agent_judge") matched the OLD multi-offender message too, so the rewritten single-offender text is not actually asserted — match on "an enabled agent_judge criterion" and add assert "simulation" not in str(excinfo.value).
3. [Axis 5] Route-seam exhaustiveness fake gained an inert simulator_route kwarg the function under test never reads, instead of covering the seam the PR actually makes reachable (tests/test_route_seam_exhaustiveness.py:95) — The only change to this seam registry is route=r, eval_route=r, simulator_route=r, result=SimpleNamespace(environment_info={}), agent=None at line 95, but Orchestrator._record_route_environment_info (orchestrator.py:1547-1590 at PR HEAD) reads only self.result, self.route, self.eval_route and self.agent -- it never touches simulator_route. The added kwarg is therefore inert: it cannot make the test pass or fail, and it gives the false impression that the new third route consumer is covered here.

The seam this PR actually creates is UserSimulator._resolve_model (src/coder_eval/simulation/user_simulator.py:250-253), an isinstance(route, BedrockRoute) / else chain that -- for the first time -- can now receive a LiteLLMRoute (see finding 1). It is absent from this module's own seam inventory in the docstring at lines 3-5 ("_build_sdk_env, _format_routing, ROUTE_NAMES, the llm_judge dispatch (_invoke_tool_channel), and Orchestrator._record_route_environment_info"), and grep -rn "_resolve_model" tests/ returns nothing, so no test exercises it for any route at all.

Recommendation: drop the inert simulator_route=r kwarg at line 95, add UserSimulator._resolve_model to the docstring's seam list, and add def test_simulator_model_resolution_handles_every_route(): for r in _INSTANCES: assert UserSimulator._resolve_model("anthropic.claude-sonnet-4-6", r) plus a per-route assertion on the value, so a route that the simulator cannot serve fails here rather than mid-dialog.
4. [Axis 7] Rename/semantics ripple incomplete: prose across src, tests and docs still says the simulator shares eval_route/checker_context.api_route (src/coder_eval/orchestrator.py:1474) — The PR updates four regions of docs/TASK_DEFINITION_GUIDE.md but leaves every other statement of the old contract in place, so the codebase now documents both meanings of the same knob. Confirmed by grep -rniE "simulat" src docs CLAUDE.md README.md experiments tasks | grep -iE "eval_route|evaluation side|checker_context":

• src/coder_eval/orchestrator.py:1474 (IN SCOPE — the docstring of _eval_route_overrides, the very method that reads checker_context.api_route): "agent_judge, the simulator all share one eval_route), decoupled from" — directly contradicts orchestrator.py:1507 twelve lines below in the same file.
• src/coder_eval/orchestrator.py:1565 (IN SCOPE): "# AGENT's route) — record the judge/simulator's own model separately so" — eval_model no longer describes the simulator's model at all.
• src/coder_eval/models/tasks.py:94 — the Pydantic Field(description=...) on ApiRouteContext.route: "Backend the WHOLE evaluation side (llm_judge/agent_judge/simulator) calls." This is a user-facing schema surface.
• src/coder_eval/models/tasks.py:494 — the checker_context field description: "the backend the WHOLE evaluation side (llm_judge, agent_judge, the simulator) calls, ".
• src/coder_eval/models/routing.py:312-314 — resolve_evaluation_route's docstring: "agent_judge criteria and the simulated user — which must stay on a constant Claude backend regardless of the agent under test, so grading and simulation stay comparable across models."
• docs/DIALOG_MODE.md:63-64 — the dedicated simulation guide, unedited: "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". This is the page a simulation user reads first, and it now states the exact opposite of docs/TASK_DEFINITION_GUIDE.md:1616.
• docs/AB_EXPERIMENTS.md:133 — the variant-key table: "checker_context | dict | Backend/model override for the evaluation side (judge, simulator)".

Update all six. Note CE030 (doc/schema parity) only checks that a field name is mentioned, not that the prose is true, so nothing mechanically guards this class of drift — worth a .claude/harness-candidates.md entry.

Nits

1. [Axis 1] Test-class hygiene: four identical monkeypatch.setattr lines, a 17-line docstring narrating cross-repo PR history, and a non-static sibling helper (tests/test_orchestrator.py:157) — The line monkeypatch.setattr(orchestrator_module, "resolve_route", lambda _s: DirectRoute(judge_transport="anthropic")) is repeated verbatim at lines 157, 173, 191 and 205 — every test in the class, never varied; fold it into the _orchestrator helper (line 128) or an autouse fixture. The class docstring at lines 110-125 narrates history rather than the contract — see PR #2864 (skills repo) review, which is what prompted this: 303/359 llm_judge tasks there have simulation.enabled (line 118-120) cites a private external repo that no reader of this OSS-bound repo can verify, and line 185's this closes the gap the simulator-decoupling PR temporarily left open describes the PR rather than the behavior under test. Also _litellm_api_route (line 146) is a plain instance method while its sibling _orchestrator (line 127-128) is a @staticmethod — make them consistent.
2. [Axis 2] New simulator_route Optional is the only route the simulation seam does not narrow with an assert, and None is a meaningful value in UserSimulator (src/coder_eval/orchestrator.py:2229) — _simulation_dialog_loop narrows every other Optional it consumes immediately before building the simulator — orchestrator.py:2215-2218 reads:

        assert self.task.simulation is not None
        assert self.agent is not None
        assert self.success_checker is not None
        assert self.task.agent is not None

but the newly introduced route attribute is passed unnarrowed at line 2229:

            route=self.simulator_route,

where the field is declared self.simulator_route: ApiRoute | None = None (orchestrator.py:428). UserSimulator.__init__ accepts route: ApiRoute | None = None (src/coder_eval/simulation/user_simulator.py:159) and treats None as a supported mode rather than an error — it logs "User simulator: Claude Code agent backend (default route, model=%s)" (user_simulator.py:233) and lets the Claude Code subprocess pick whatever ambient credential/endpoint is in the environment. So an Optional flows into an Optional parameter whose None branch silently means "unpinned backend", and pyright cannot flag it. Today _resolve_routes (line 1507, self.simulator_route = self.route) always runs before this path, so there is no live bug; this is a hardening nit, and the same shape existed pre-PR when route=self.eval_route was passed. Add assert self.simulator_route is not None alongside the four asserts at 2215-2218 so the invariant is machine-checked and a future ordering change fails loudly instead of running the simulated user on an unpinned route. (Note the codebase already applies exactly this convention to the sibling field: assert self.route is not None at orchestrator.py:1555.)
3. [Axis 3] test_non_litellm_override_leaves_simulator_route_unaffected asserts only isinstance, never the identity its own docstring claims (tests/test_orchestrator.py:210) — The docstring at tests/test_orchestrator.py:196-197 states "simulator_route always equals the agent's own resolution, independent of any checker_context.api_route override", but the body only asserts assert isinstance(orchestrator.simulator_route, DirectRoute) (line 210) and assert orchestrator.simulator_route is not orchestrator.eval_route (line 211). It never asserts orchestrator.simulator_route is orchestrator.route — the invariant the production comment at src/coder_eval/orchestrator.py:1559-1560 ("simulator_route always mirrors self.route, so it never diverges from api_routing above") depends on. Verified by mutation: replacing line 1507 with a hardcoded self.simulator_route = DirectRoute(judge_transport="anthropic") leaves all 153 tests in the three changed files passing. Note this also leaves the two halves of the feature tested only in isolation: tests/test_litellm_route.py:353 hand-sets simulator_route=simulator_route on a SimpleNamespace fake, so nothing joins _resolve_routes to _simulation_dialog_loop. Change line 210 to assert orchestrator.simulator_route is orchestrator.route.

What's Missing

Parallel paths:

• 🟠 The LiteLLM→Claude pin was moved off the simulator but never re-established on the new producer: resolve_evaluation_route (src/coder_eval/models/routing.py:365-371) still holds the only "agent is on LiteLLM → pin to a constant Claude backend" logic, and it is now reachable by eval_route alone; _resolve_routes (orchestrator.py:1507) hands the simulator the raw self.route. Decoupling from checker_context did not require dropping the pin — self.simulator_route = resolve_evaluation_route(settings, self.route) with no overrides gives both. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 7: simulator_route = self.route drops the pin-to-Claude guard)
• 🟡 Only docs/TASK_DEFINITION_GUIDE.md was updated; the parallel prose surfaces that state the old contract were not — orchestrator.py:1474 and :1565 (in the changed file itself), the user-facing Pydantic Field(description=...) at models/tasks.py:94 and :494, models/routing.py:311-314, docs/DIALOG_MODE.md:63-64 (the page a simulation user reads first, now the exact opposite of the guide), and docs/AB_EXPERIMENTS.md:133. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 7: Rename/semantics ripple incomplete)
• 🟡 The surviving litellm + agent_judge rejection still lives only in Orchestrator._resolve_routes (orchestrator.py:1519-1543), i.e. per task, after sandbox creation, at run time. The parallel resolution-time validation surface got no equivalent: cli/plan_command.py:139 and orchestration/experiment.py:721 call validate_early_stop(resolved) at plan time but nothing checks checker_context.api_route.route: litellm against an enabled agent_judge. The PR's own motivating scenario is an experiment-wide checker_context default over a large suite — exactly the case that should fail once at coder-eval plan, not N times mid-batch. CLAUDE.md's convention for this class of guardrail is "a hard error at resolution (plan and run)". (trigger: src/coder_eval/orchestrator.py)

Tests:

• 🟠 No test in the suite ever puts the AGENT on a LiteLLMRoute while resolving routes — all four new tests stub resolve_route to DirectRoute (tests/test_orchestrator.py:157, 173, 191, 205) — so the single branch whose behavior this PR actually changes at runtime (simulator follows the agent onto the gateway) has zero coverage. (trigger: tests/test_orchestrator.py) (restates: Axis 3: agent-on-LiteLLM branch untested; mutation-restoring the old pinning leaves the suite green)
• 🟡 The rewrite deleted the disabled-agent_judge no-raise case and added no replacement, leaving the and c.enabled conjunct at orchestrator.py:1537 unreachable by any test (mutation-verified: deleting it keeps the suite green). Re-add it, and tighten pytest.raises(ValueError, match="agent_judge") to match the new single-offender wording plus assert "simulation" not in str(excinfo.value) — nothing currently asserts the rewritten message. (trigger: tests/test_orchestrator.py) (restates: Axis 3: test_allows_disabled_agent_judge deleted with no replacement)
• 🟡 Nothing joins the producer to the consumer: no test asserts that _simulation_dialog_loop actually passes simulator_route into UserSimulator. tests/test_simulation_integration.py:79-81 and :132-134 wrap UserSimulator(*args, **kwargs) in a factory that could assert kwargs["route"] and does not, and the only route= assertion (tests/test_litellm_route.py:353) is against a hand-built SimpleNamespace, so both halves are tested only in isolation. (trigger: tests/test_orchestrator.py) (restates: Axis 3: test_non_litellm_override_leaves_simulator_route_unaffected asserts only isinstance)
• 🟡 The seam the PR newly makes reachable — UserSimulator._resolve_model (src/coder_eval/simulation/user_simulator.py:236-256) and the ClaudeCodeAgent(..., route=self._route) construction at user_simulator.py:302 — has no test for any route: grep -rn "_resolve_model" tests/ returns nothing, and none of the 18 UserSimulator(...) constructions in tests/test_user_simulator.py passes a route at all. The added simulator_route=r kwarg at tests/test_route_seam_exhaustiveness.py:95 is inert (the function under test never reads it) and covers none of this. (trigger: tests/test_route_seam_exhaustiveness.py) (restates: Axis 5: Route-seam exhaustiveness fake gained an inert simulator_route kwarg)

Downstream consumers:

• 🟡 Simulator cost accounting assumes the simulator runs on a Claude route and was not updated. simulator_cost_usd (models/results.py:988-1016) prices simulator tokens at the pinned SimulationConfig.model alias off the static rate card, and it feeds eval_overhead_cost_usd / total_cost_usd (docs/REPORT_SCHEMA.md:64-66, reports_experiment.py:124-181). Now that the simulator can land on the agent's LiteLLM gateway, that spend is billed by the gateway but reported at Anthropic list prices; worse, the simulator's ClaudeCodeAgent is built without cost_log_tags (user_simulator.py:302, vs. orchestrator.py:1662-1673 for the agent), so its proxy calls carry no x-ce-run-id/x-ce-task-id headers and litellm_cost.apply_actual_cost — which corrects only agent turns — silently drops them from the real bill. (trigger: src/coder_eval/orchestrator.py)

Display & mapping dicts:

• 🟡 _record_route_environment_info (orchestrator.py:1554-1590) gained no simulator_routing key even though the simulator's route is now an independent dimension — ROUTE_NAMES already maps every route type, so this is a one-line addition. The comment at :1560-1561 justifies the omission with "simulator_route always mirrors self.route", which is true only while the alias exists. Nothing else surfaces it either: the logger.info("API routing: ...") line at orchestrator.py:1509 logs the agent route only, so a run artifact (reports_html's environment_info panel, run.json) cannot tell an operator which backend the simulated user actually talked to. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 7: simulator_route = self.route drops the pin-to-Claude guard)

Daily/nightly:

• 🟠 Blast radius on the motivating consumer pipeline is unstated. The PR's own test docstring (tests/test_orchestrator.py:118-121) cites 303/359 llm_judge tasks with simulation.enabled in the downstream skills repo; on a Bedrock/Direct backend they simply start working, but on an open-weight (API_BACKEND=litellm) run they flip from a loud setup failure to a scored run whose simulated user is pointed at the agent's gateway. Neither the commit body nor docs/TASK_DEFINITION_GUIDE.md says what that does to those nightly numbers, and there is no warning log or run-artifact marker to detect it after the fact. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 7: simulator_route = self.route drops the pin-to-Claude guard)
• 🟡 No CI path exercises litellm + simulation. The live job at .github/workflows/pr-checks.yml:846-884 runs tests/test_litellm_judge_live.py, which covers the checker_context.api_route.route: litellm judge end-to-end but contains no simulation; grep -n 'simulat' .github/workflows/pr-checks.yml returns nothing, and the single in-repo dialog task (tasks/python_cli_simulated_judged/echo_simulated_judged.yaml) never runs on the litellm backend. The newly-legal combination the docs now advertise as safe would first be exercised by a consumer's nightly run, not by this repo's gates. (trigger: tests/test_litellm_route.py)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] CE046 — evaluation-side consumers may only receive an evaluation route. New BaseRule at tests/lint/rules/ce046_eval_side_route_source.py, wired into ALL_RULES in tests/lint/runner.py (runs over src/ only, which is where the offender lives). The rule carries a small registry of evaluation-side constructors/callables and the kwarg that carries their route — {"UserSimulator": "route", "SubAgentRunner": "route", ...} — and requires that kwarg's expression to be either self.eval_route or a direct resolve_evaluation_route(...) call. Any other expression is a violation. To defeat aliasing, the rule first builds a one-pass intra-class alias map (self.simulator_route = self.route → simulator_route ≡ route) and resolves the kwarg through it before judging, so renaming the attribute does not launder the agent route past the check. Suppressible only with # noqa: CE046 plus a rationale comment. Prevents: The critical finding: self.simulator_route = self.route (src/coder_eval/orchestrator.py:1507) passed at route=self.simulator_route (orchestrator.py:2229) dropped the LiteLLM→Claude pin, sending the simulated user (the measuring instrument) to the agent's own open-weight gateway — a hard 400 on the repo's own litellm-config.yaml, i.e. final_status changes for identical agent output. The alias-resolution step is what makes the rule fire here rather than being trivially bypassed.
• [ce-lint] CE047 — no write-once self-alias attribute. New BaseRule at tests/lint/rules/ce047_no_self_alias_attribute.py (+ ALL_RULES entry). Per class, collect every self.<name> = … assignment across all methods; if <name> is assigned exactly once in the whole class and the RHS is a bare self.<other>, it is a violation: express it as @property def <name>(self): return self.<other> (mirror invariant becomes structural, not a comment) or delete it and use self.<other> at the one call site. Declaration-only annotations (self.x: T | None = None in __init__) count as the sentinel, not as a second producer. Prevents: Finding 1 (medium/high, cross-axis 1+7): simulator_route is declared at orchestrator.py:428, assigned once at :1507 from self.route, read once at :2229, and defended by ~13 lines of comment across three sites plus a hand-asserted mirror invariant at :1560-1561 that nothing enforces — textbook speculative generality under CLAUDE.md's YAGNI rule. Also removes the root cause of finding 6's inert simulator_route=r kwarg in the test fake and of the three-way-impossible fake state at tests/test_litellm_route.py:353.
• [pyright] Make the route required where None is not a supported mode. Drop the = None default and the | None from UserSimulator.__init__(route: ApiRoute | None = None) (src/coder_eval/simulation/user_simulator.py:159) — None there silently means "unpinned ambient backend", which is never a legitimate evaluation configuration. With CE047 turning simulator_route into a non-Optional @property, pyright then rejects any Optional route reaching the simulator at type-check time rather than at dialog time, and no assert … is not None narrowing line is needed. Prevents: Finding 2 (Type Safety): an ApiRoute | None flowing unnarrowed into an ApiRoute | None parameter whose None branch runs the simulated user on whatever ambient credentials the environment holds — the only route in _simulation_dialog_loop not narrowed alongside the four asserts at orchestrator.py:2215-2218.
• [ce-lint] CE048 — route-dispatch seam registry completeness. New BaseRule (tests/lint/rules/ce048_route_seam_registered.py, ALL_RULES entry) that flags any function in src/ containing an isinstance(x, <DirectRoute|BedrockRoute|LiteLLMRoute>) test or a case <Route>() pattern whose module::qualname is not listed in a module-level SEAM_FUNCTIONS frozenset exported by tests/test_route_seam_exhaustiveness.py (the rule imports that set; the test file stays the single registry, so adding a seam means adding a fixture-driven check in the same change). Adopt with the current six dispatch sites registered: orchestrator._format_routing, orchestrator.Orchestrator._record_route_environment_info, agents.claude_code_agent.ClaudeCodeAgent._build_sdk_env, criteria.llm_judge._invoke_tool_channel (×2), models.routing.resolve_evaluation_route. Prevents: Finding 6 (medium, cross-axis 1+2+3+5+7): UserSimulator._resolve_model (user_simulator.py:251) is a live isinstance(route, BedrockRoute)/else dispatch that no test touches (grep -rn "_resolve_model" tests/ returns nothing) and that this PR made reachable by LiteLLMRoute for the first time; the inert simulator_route=r kwarg added at tests/test_route_seam_exhaustiveness.py:95 gave the false impression it was covered. The rule makes an unregistered dispatch fail make lint instead of failing mid-dialog.
• [ce-lint] CE049 — route-consumer prose parity (whole-tree, doc-surface family). A dedicated @pytest.mark.lint class in tests/test_custom_lint.py (like CE026/CE030 — it reads Markdown and Pydantic Field(description=...) strings, so it is not a per-file BaseRule). Derive the SSOT set of evaluation-route consumers from src/ (symbols that read self.eval_route / take an eval_route parameter → today {llm_judge, agent_judge}), then assert no surface in a fixed registry names a consumer outside that set: models/tasks.py (ApiRouteContext.route :94 and checker_context :494 descriptions), models/routing.py::resolve_evaluation_route docstring (:311-314), orchestrator.py::_eval_route_overrides docstring (:1474) and _record_route_environment_info comment (:1565), docs/DIALOG_MODE.md, docs/AB_EXPERIMENTS.md, docs/TASK_DEFINITION_GUIDE.md. Prevents: Finding 8 (medium): seven statements across five files still assert the simulator shares eval_route/checker_context.api_route, including two user-facing Pydantic field descriptions and docs/DIALOG_MODE.md:63-64, which now says the exact opposite of the same PR's docs/TASK_DEFINITION_GUIDE.md:1616. CE030 cannot catch this class — its own source documents the match as "the bare name appears wrapped in backticks anywhere in the doc … a floor, not a proof", so prose that names the right field and describes the wrong contract passes.
• [ce-lint] CE050 — no change-history narration in comments or docstrings. Regex rule over src/ (BaseRule + comment tokens) and, via a sibling whole-tree lint class, over tests/: forbid PR #\d+, this PR, the <…> PR, and the speculative-generality idioms not yet …able, until <X> grows, intended to grow. Git is the history surface; a cross-repo PR number is unverifiable to any reader of this OSS-bound repo, and "intended to grow later" is the marker CLAUDE.md's YAGNI rule exists to catch. Adoption cost is real and should be stated up front: grep -rniE "PR #[0-9]+|this PR\b" src/ tests/ currently returns ~20 hits, so land the rule with a one-time cleanup (most are equally expressible as a statement of the contract) rather than a broad grandfather list. Prevents: Finding 2 (Test Health/Quality): the 17-line class docstring at tests/test_orchestrator.py:110-125 narrating a private cross-repo review ("see PR #2864 (skills repo) review") and tests/test_orchestrator.py:185 ("this closes the gap the simulator-decoupling PR temporarily left open"), plus the two self-conceding speculative comments in finding 1 (orchestrator.py:422-427 and :1504-1506).

Harness improvements (not statically reachable):

• Diff-scoped mutation gate. Add make mutate-changed: run mutmut/cosmic-ray restricted to the src/ lines the branch changes vs main (git diff main...HEAD -U0 → line ranges), fail on any surviving mutant, and wire it as a required PR job for hot modules (orchestrator.py, checker.py, criteria/, models/routing.py). Budget it by only mutating changed lines so the job stays minutes, not hours. Why not static: Mutant survival is a property of executing the test suite against a mutated tree — no AST or type check can tell whether an assertion actually discriminates the behavior it names. Prevents: All three Test Health findings, each of which was proven by a surviving mutant on a line this very PR touched: the agent-on-LiteLLM branch (orchestrator.py:1507 mutated back to the old pin → full suite still green), the dropped and c.enabled conjunct (orchestrator.py:1537 → suite byte-identical), and simulator_route hardcoded to a fresh DirectRoute → all 153 tests in the three changed files still passed.
• Deleted-test guard in CI. A PR job that diffs def test_* / async def test_* names between the merge base and HEAD; every removed name must either reappear (rename) or be accounted for by an explicit Removes-test: <name> — <reason> commit trailer. Print the removed names in the job summary so a reviewer sees them without reading the diff. Why not static: Requires comparing two git revisions; a lint rule sees one tree and a deleted test is, by construction, not in it. Prevents: Finding 4 (medium): test_allows_disabled_agent_judge was deleted with no replacement, leaving the and c.enabled conjunct of the live guard at orchestrator.py:1537 unreachable by any test in the suite — a task with a disabled agent_judge plus checker_context.api_route.route: litellm would start hard-failing at setup with nothing to catch it.
• Record every resolved route in the run artifact, and assert the key set. Extend _record_route_environment_info to write simulator_routing (and simulator_model) next to api_routing/eval_routing, then add an artifact-contract test asserting the recorded routing keys equal the set of ApiRoute-typed attributes the orchestrator resolves — so a new route consumer cannot be added without becoming auditable in run.json/task.json. Why not static: It is a contract about the content of a finalized run record, observable only after a run produces the artifact; a static check cannot know which keys a consumer downstream (evalboard, the nightly rollup) needs to diff runs against each other. Prevents: The critical finding's audit half — the simulator's backend changed with no error, no warning, and no new artifact key (_record_route_environment_info's own new comment at orchestrator.py:1560-1561 concedes "The simulator is NOT part of this"), so a cross-run score shift would have been undiagnosable; also finding 8's drift, since the artifact then states the true consumer set.
• Real-wiring test for the simulation route path. Build an actual Orchestrator with simulation.enabled, monkeypatch UserSimulator with a capturing double, run _resolve_routes → _simulation_dialog_loop, and assert the captured route is orchestrator.route/orchestrator.eval_route (identity, not isinstance) for each agent backend. Today the two halves are only tested against hand-built SimpleNamespace fakes, so nothing joins them. Why not static: It is an object-identity/wiring property across two methods resolved from runtime settings; a static check can constrain the source expression (CE046) but cannot prove the object that actually reaches the constructor. Prevents: Finding 5 (low) — test_non_litellm_override_leaves_simulator_route_unaffected asserts only isinstance(..., DirectRoute) while its docstring claims identity, and a hardcoded DirectRoute(...) at orchestrator.py:1507 leaves all 153 tests green — and finding 6's fake-only coverage, including tests/test_litellm_route.py:353's three-way-distinct route state that production can never produce.
• Measuring-instrument invariance matrix. Parametrize _resolve_routes over {agent backend: direct, bedrock, litellm} × {checker_context.api_route: unset, bedrock, direct, litellm} and assert, for every cell, that the judge route and the simulator route are never a LiteLLMRoute unless explicitly opted in — the machine-checked form of the guide's rule that "the simulator is part of the measuring instrument, not the thing being measured" (docs/TASK_DEFINITION_GUIDE.md:1616). Why not static: The invariant is the output of resolve_evaluation_route's pin logic under real Settings, i.e. a runtime resolution over a config matrix, not a syntactic property of any one call site. Prevents: The critical finding (agent-on-LiteLLM → simulator on the agent's open-weight gateway) and finding 3's uncovered branch — all four new tests stub resolve_route to DirectRoute, so the agent is never on LiteLLMRoute in any of them.
• Release-labeling gate for knob-semantics changes. A PR job that flags a diff touching user-facing knob semantics — Field(description=...) strings on TaskDefinition/ApiRouteContext/checker_context, or the contract sections of docs/TASK_DEFINITION_GUIDE.md/DIALOG_MODE.md — when the commit subject is a plain feat:/fix: with no ! and no BREAKING CHANGE: trailer, so python-semantic-release (pyproject.toml:375-395) cannot ship a documented-knob narrowing as a silent patch bump. Why not static: Needs commit metadata and a base-revision comparison; the released version is a function of the commit message, which is not part of the source tree a lint rule inspects. Prevents: The critical finding's release half: checker_context.api_route silently stopped applying to the simulator — an accepted, still-parsing YAML knob that is now ignored for one of its three documented consumers — while the change was labeled as an ordinary non-breaking commit.

Top 5 Priority Actions

1. Restore the pin-off-open-weight guard at src/coder_eval/orchestrator.py:1507 — replace self.simulator_route = self.route with self.simulator_route = resolve_evaluation_route(settings, self.route) (no checker_context overrides), which keeps the intended decoupling from checker_context.api_route while preventing every simulation.enabled task on the litellm backend from POSTing the pinned Claude simulator model at the agent's gateway (the repo's own litellm/litellm-config.yaml serves no Claude alias, so the dialog hard-fails to ERROR for identical agent output), and record simulator_routing in _record_route_environment_info so the route is auditable in run artifacts.
2. Add the missing agent-on-LiteLLM test to TestSimulatorRouteDecoupledFromCheckerContext in tests/test_orchestrator.py — all four new tests stub resolve_route to DirectRoute (lines 157, 173, 191, 205), so the only branch the PR actually changes at runtime is untested and restoring the old pinning leaves the full suite green; assert orchestrator.simulator_route is orchestrator.route (also fixing tests/test_orchestrator.py:210, which asserts only isinstance and never the identity its docstring claims) and that eval_route stays pinned to Bedrock/Direct.
3. Re-add the deleted test_allows_disabled_agent_judge case near tests/test_orchestrator.py:178 and tighten its sibling to pytest.raises(ValueError, match="an enabled agent_judge criterion") plus assert "simulation" not in str(excinfo.value) — deleting and c.enabled from the live guard at src/coder_eval/orchestrator.py:1537 currently leaves the suite byte-identically green, so a task with a disabled agent_judge plus checker_context.api_route.route: litellm could start hard-failing at setup unnoticed.
4. Finish the semantics ripple across the seven stale statements that still assert the simulator shares eval_route/checker_context.api_route — src/coder_eval/orchestrator.py:1474 and :1565 (in scope, and :1474 contradicts a comment this same change added 58 lines below), the two user-facing Pydantic field descriptions at src/coder_eval/models/tasks.py:94 and :494, resolve_evaluation_route's docstring at src/coder_eval/models/routing.py:311-314, docs/DIALOG_MODE.md:63-64 (which now states the exact opposite of docs/TASK_DEFINITION_GUIDE.md:1616), and docs/AB_EXPERIMENTS.md:133 — since CE030 only checks that a field name is mentioned, not that the prose is true.
5. Remove the speculative surface: make simulator_route (src/coder_eval/orchestrator.py:428) a @property returning self.route or drop it for a direct route=self.route at :2229 so the mirror invariant is structural rather than a 13-line comment across three sites, and drop the inert simulator_route=r kwarg at tests/test_route_seam_exhaustiveness.py:95 (never read by _record_route_environment_info) in favour of a real per-route test of UserSimulator._resolve_model (src/coder_eval/simulation/user_simulator.py:236-256), which has zero coverage today.

---

Stats: 1 🔴 · 1 🟠 · 4 🟡 · 3 🔵 across 8 axes reviewed.

[uipreliga]

Fix what you agree with and 🚢

[uipreliga]

Fix what you agree with and 🚢

[akshaylive]

Addressed in a9a1f07. Thanks for the very thorough review — the critical finding was real: collapsing simulator_route to a bare self.route alias in c46c241 silently dropped the LiteLLM→Claude pin, so an agent on an open-weight LiteLLM backend would have sent the simulated user to its own gateway instead of a stable Claude backend.

Fixed by routing simulator_route through resolve_evaluation_route(settings, self.route) (no overrides) — same pin logic eval_route already uses, minus the checker_context overrides, so the intended decoupling from the judge-only knob is preserved while the pin comes back.

Also addressed:

• Added the missing "agent on LiteLLMRoute" regression test (mutation-verified against the exact bug you found).
• Re-added the disabled-agent_judge no-raise case and tightened the rejection assertion to the new single-offender message.
• Recorded simulator_routing/simulator_model in environment_info for audit parity with eval_routing/eval_model.
• Fixed the remaining stale prose: _eval_route_overrides docstring, the ApiRouteContext.route/checker_context field descriptions, docs/DIALOG_MODE.md, and docs/AB_EXPERIMENTS.md.
• Folded the repeated per-test resolve_route stub into an autouse fixture, made _litellm_api_route a staticmethod for consistency with its sibling.

Not addressed (left as follow-ups, not blockers):

• The proposed CE046–CE050 lint rules and the plan-time (vs. run-time) validation surface for route: litellm + agent_judge — real ideas, but larger scope than this fix; happy to open follow-up issues if useful.
• The @property-for-simulator_route suggestion is moot now that it has a genuinely independent resolution (via resolve_evaluation_route), not a bare alias.
• Skipped adding an assert self.simulator_route is not None in _simulation_dialog_loop — it broke 3 unrelated unit tests in test_run_limits_orchestrator.py that construct a bare Orchestrator without calling _resolve_routes(), and you'd already flagged it as "no live bug today," so reverted rather than expand scope into that file.

Full suite green: ruff/pyright clean, make lint 376/376, pytest 4754 passed / 6 skipped / 1 pre-existing unrelated live-SDK flake (confirmed present on main too).