From e3025dee779c9f7d05936446a926a39022728590 Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Tue, 16 Jun 2026 10:12:35 -0700 Subject: [PATCH] Reviewer eval admin: configurable runs + stacked form layout (#1540) Drive dashboard-triggered reviewer eval runs with per-run model, effort, score mode, severity threshold, cap, limit, and concurrency overrides, plus per-example start/finish/error logging in the eval target. Rework the admin eval form from the label-left/control-right SettingsRow (which crushed the description column when packing 3-4 wide inputs) into stacked field groups with captioned inputs in a responsive grid. Co-authored-by: open-swe[bot] --- agent/dashboard/eval_jobs.py | 98 +++++- agent/dashboard/routes.py | 102 +++++- evals/reviewer/run_eval.py | 144 ++++++-- evals/reviewer/target.py | 43 ++- tests/test_eval_jobs.py | 52 +++ tests/test_reviewer_eval_run.py | 86 ++++- ui/src/lib/api.ts | 27 +- ui/src/routes/admin_.evals.tsx | 607 ++++++++++++++++++++++++++++---- 8 files changed, 1036 insertions(+), 123 deletions(-) diff --git a/agent/dashboard/eval_jobs.py b/agent/dashboard/eval_jobs.py index 9acf2092..6a09d299 100644 --- a/agent/dashboard/eval_jobs.py +++ b/agent/dashboard/eval_jobs.py @@ -17,7 +17,7 @@ import sys import uuid from datetime import UTC, datetime from pathlib import Path -from typing import Any, Literal +from typing import Any, Literal, TypedDict from langgraph_sdk import get_client @@ -27,7 +27,7 @@ EVALS_NAMESPACE: list[str] = ["evals"] REVIEWER_EVAL_KEY = "reviewer" DEFAULT_EVAL_PROJECT = "open-swe-evals" _MODULE = "evals.reviewer.run_eval" -_LOG_TAIL_CHARS = 4000 +_LOG_TAIL_CHARS = 12000 _EXPERIMENT_URL_RE = re.compile(r"https://\S*smith\.langchain\.com/\S+") # The owning worker refreshes the heartbeat this often while the subprocess # runs; a record is only reconciled as failed once its heartbeat is older than @@ -36,6 +36,37 @@ _HEARTBEAT_INTERVAL_SECONDS = 10 _HEARTBEAT_STALE_SECONDS = 60 EvalStatus = Literal["idle", "running", "completed", "failed"] +ScoreMode = Literal["all_findings", "surfaced_findings"] +Severity = Literal["low", "medium", "high", "critical"] + + +class ReviewerEvalConfig(TypedDict): + dataset_name: str + experiment_prefix: str + max_concurrency: int + langsmith_project: str + langgraph_url: str + assistant_id: str + model_id: str + reasoning_effort: str + score_mode: ScoreMode + severity_threshold: Severity + cap: int + + +DEFAULT_REVIEWER_EVAL_CONFIG: ReviewerEvalConfig = { + "dataset_name": "openswe-reviewer-v1", + "experiment_prefix": "openswe-review-confidence", + "max_concurrency": 5, + "langsmith_project": DEFAULT_EVAL_PROJECT, + "langgraph_url": "", + "assistant_id": "reviewer", + "model_id": "google_genai:gemini-3.5-flash", + "reasoning_effort": "medium", + "score_mode": "all_findings", + "severity_threshold": "medium", + "cap": 4, +} # Identifies this process so heartbeat ownership can be reasoned about across # workers that share the persisted record. @@ -70,12 +101,47 @@ def _eval_project() -> str: return os.environ.get("EVAL_LANGSMITH_PROJECT") or DEFAULT_EVAL_PROJECT +def _resolve_eval_config(config: ReviewerEvalConfig | None = None) -> ReviewerEvalConfig: + resolved: ReviewerEvalConfig = { + **DEFAULT_REVIEWER_EVAL_CONFIG, + "langsmith_project": _eval_project(), + "langgraph_url": _resolve_langgraph_url() or "", + } + if config is not None: + resolved.update(config) + return resolved + + +def _config_cli_args(config: ReviewerEvalConfig) -> list[str]: + values: list[tuple[str, object]] = [ + ("--dataset-name", config["dataset_name"]), + ("--experiment-prefix", config["experiment_prefix"]), + ("--max-concurrency", config["max_concurrency"]), + ("--langsmith-project", config["langsmith_project"]), + ("--assistant-id", config["assistant_id"]), + ("--model-id", config["model_id"]), + ("--reasoning-effort", config["reasoning_effort"]), + ("--score-mode", config["score_mode"]), + ("--severity-threshold", config["severity_threshold"]), + ("--cap", config["cap"]), + ] + if config["langgraph_url"]: + values.append(("--langgraph-url", config["langgraph_url"])) + args: list[str] = [] + for flag, value in values: + args.extend([flag, str(value)]) + return args + + def _idle_record() -> dict[str, Any]: + config = _resolve_eval_config() return { "name": REVIEWER_EVAL_KEY, "status": "idle", - "langsmith_project": _eval_project(), + "run_name": config["experiment_prefix"], + "langsmith_project": config["langsmith_project"], "limit": None, + "config_snapshot": config, "started_at": None, "finished_at": None, "created_by": None, @@ -163,6 +229,7 @@ async def get_reviewer_eval_status() -> dict[str, Any]: async def start_reviewer_eval( *, limit: int | None, + config: ReviewerEvalConfig | None = None, created_by: str, ) -> dict[str, Any]: """Launch the reviewer eval subprocess and persist a ``running`` record. @@ -176,10 +243,12 @@ async def start_reviewer_eval( if existing and existing.get("status") == "running" and _is_heartbeat_fresh(existing): raise RuntimeError("a reviewer eval is already running") - project = _eval_project() + config_snapshot = _resolve_eval_config(config) + project = config_snapshot["langsmith_project"] cmd = [sys.executable, "-m", _MODULE] if limit is not None and limit > 0: cmd += ["--limit", str(limit)] + cmd += _config_cli_args(config_snapshot) env = { **os.environ, @@ -205,6 +274,10 @@ async def start_reviewer_eval( { **_idle_record(), "status": "failed", + "run_name": config_snapshot["experiment_prefix"], + "langsmith_project": project, + "limit": limit, + "config_snapshot": config_snapshot, "finished_at": _now_iso(), "created_by": created_by, "error": f"Failed to launch eval: {exc}", @@ -216,8 +289,10 @@ async def start_reviewer_eval( { **_idle_record(), "status": "running", + "run_name": config_snapshot["experiment_prefix"], "langsmith_project": project, "limit": limit, + "config_snapshot": config_snapshot, "started_at": _now_iso(), "finished_at": None, "created_by": created_by, @@ -226,7 +301,14 @@ async def start_reviewer_eval( "heartbeat": _now_iso(), } ) - asyncio.create_task(_monitor(proc, created_by=created_by, limit=limit, project=project)) + asyncio.create_task( + _monitor( + proc, + created_by=created_by, + limit=limit, + config_snapshot=config_snapshot, + ) + ) return record @@ -295,7 +377,7 @@ async def _monitor( *, created_by: str, limit: int | None, - project: str, + config_snapshot: ReviewerEvalConfig, ) -> None: heartbeat = asyncio.create_task(_heartbeat_loop(proc)) tail = "" @@ -322,8 +404,10 @@ async def _monitor( { **record, "status": status, - "langsmith_project": project, + "run_name": config_snapshot["experiment_prefix"], + "langsmith_project": config_snapshot["langsmith_project"], "limit": limit, + "config_snapshot": config_snapshot, "created_by": created_by, "finished_at": _now_iso(), "pid": proc.pid, diff --git a/agent/dashboard/routes.py b/agent/dashboard/routes.py index c7f97b90..34d3136a 100644 --- a/agent/dashboard/routes.py +++ b/agent/dashboard/routes.py @@ -5,12 +5,12 @@ from __future__ import annotations import hmac import logging import os -from typing import Any +from typing import Any, Literal import httpx from fastapi import APIRouter, BackgroundTasks, Depends, HTTPException, Request from fastapi.responses import RedirectResponse, Response, StreamingResponse -from pydantic import BaseModel +from pydantic import BaseModel, field_validator, model_validator from .admin import is_admin from .agent_instructions import ( @@ -33,6 +33,8 @@ from .enabled_repos import ( set_review_repo_enabled, ) from .eval_jobs import ( + DEFAULT_REVIEWER_EVAL_CONFIG, + ReviewerEvalConfig, cancel_reviewer_eval, get_reviewer_eval_status, start_reviewer_eval, @@ -54,7 +56,7 @@ from .oauth import ( require_session, sanitize_redirect_to, ) -from .options import SUPPORTED_MODELS +from .options import SUPPORTED_MODEL_IDS, SUPPORTED_MODELS, model_supports_effort from .profiles import ( ProfileUpdate, get_profile, @@ -567,8 +569,96 @@ async def admin_delete_user_mapping( return {"deleted": deleted} +ScoreMode = Literal["all_findings", "surfaced_findings"] +Severity = Literal["low", "medium", "high", "critical"] + + class ReviewerEvalStartBody(BaseModel): limit: int | None = None + dataset_name: str = DEFAULT_REVIEWER_EVAL_CONFIG["dataset_name"] + experiment_prefix: str = DEFAULT_REVIEWER_EVAL_CONFIG["experiment_prefix"] + max_concurrency: int = DEFAULT_REVIEWER_EVAL_CONFIG["max_concurrency"] + langsmith_project: str = DEFAULT_REVIEWER_EVAL_CONFIG["langsmith_project"] + langgraph_url: str = DEFAULT_REVIEWER_EVAL_CONFIG["langgraph_url"] + assistant_id: str = DEFAULT_REVIEWER_EVAL_CONFIG["assistant_id"] + model_id: str = DEFAULT_REVIEWER_EVAL_CONFIG["model_id"] + reasoning_effort: str = DEFAULT_REVIEWER_EVAL_CONFIG["reasoning_effort"] + score_mode: ScoreMode = DEFAULT_REVIEWER_EVAL_CONFIG["score_mode"] + severity_threshold: Severity = DEFAULT_REVIEWER_EVAL_CONFIG["severity_threshold"] + cap: int = DEFAULT_REVIEWER_EVAL_CONFIG["cap"] + + @field_validator( + "dataset_name", + "experiment_prefix", + "langsmith_project", + "assistant_id", + "model_id", + "reasoning_effort", + mode="before", + ) + @classmethod + def _normalize_required_string(cls, value: object) -> str: + if not isinstance(value, str): + raise ValueError("must be a string") + text = value.strip() + if not text: + raise ValueError("must not be blank") + return text + + @field_validator("langgraph_url", mode="before") + @classmethod + def _normalize_optional_string(cls, value: object) -> str: + if value is None: + return "" + if not isinstance(value, str): + raise ValueError("must be a string") + return value.strip() + + @field_validator("limit") + @classmethod + def _validate_limit(cls, value: int | None) -> int | None: + if value is not None and value <= 0: + raise ValueError("limit must be positive") + return value + + @field_validator("max_concurrency") + @classmethod + def _validate_max_concurrency(cls, value: int) -> int: + if value <= 0: + raise ValueError("max_concurrency must be positive") + return value + + @field_validator("cap") + @classmethod + def _validate_cap(cls, value: int) -> int: + if value < 0: + raise ValueError("cap must be non-negative") + return value + + @model_validator(mode="after") + def _validate_model_effort(self) -> ReviewerEvalStartBody: + if self.model_id not in SUPPORTED_MODEL_IDS: + raise ValueError(f"unsupported reviewer eval model: {self.model_id}") + if not model_supports_effort(self.model_id, self.reasoning_effort): + raise ValueError( + f"effort {self.reasoning_effort!r} not supported by model {self.model_id!r}" + ) + return self + + def eval_config(self) -> ReviewerEvalConfig: + return { + "dataset_name": self.dataset_name, + "experiment_prefix": self.experiment_prefix, + "max_concurrency": self.max_concurrency, + "langsmith_project": self.langsmith_project, + "langgraph_url": self.langgraph_url, + "assistant_id": self.assistant_id, + "model_id": self.model_id, + "reasoning_effort": self.reasoning_effort, + "score_mode": self.score_mode, + "severity_threshold": self.severity_threshold, + "cap": self.cap, + } @router.get("/admin/evals/reviewer") @@ -587,7 +677,11 @@ async def admin_start_reviewer_eval( if status.get("status") == "running": raise HTTPException(409, "a reviewer eval is already running") try: - return await start_reviewer_eval(limit=body.limit, created_by=session["sub"]) + return await start_reviewer_eval( + limit=body.limit, + config=body.eval_config(), + created_by=session["sub"], + ) except RuntimeError as exc: raise HTTPException(409, str(exc)) from exc diff --git a/evals/reviewer/run_eval.py b/evals/reviewer/run_eval.py index 1aecef4b..c068d024 100644 --- a/evals/reviewer/run_eval.py +++ b/evals/reviewer/run_eval.py @@ -10,7 +10,7 @@ import argparse import logging import os import tomllib -from collections.abc import Iterable +from collections.abc import Iterable, Mapping from pathlib import Path from typing import Any, Literal, TypedDict @@ -28,6 +28,22 @@ CONFIG_PATH = Path(__file__).with_name("config.toml") DEFAULT_LANGSMITH_PROJECT = "open-swe-evals" ScoreMode = Literal["all_findings", "surfaced_findings"] Severity = Literal["low", "medium", "high", "critical"] +_VALID_SCORE_MODES: set[str] = {"all_findings", "surfaced_findings"} +_VALID_SEVERITIES: set[str] = {"low", "medium", "high", "critical"} + +_ENV_MAPPING: dict[str, str] = { + "dataset_name": "REVIEWER_EVAL_DATASET_NAME", + "experiment_prefix": "REVIEWER_EVAL_EXPERIMENT_PREFIX", + "max_concurrency": "REVIEWER_EVAL_MAX_CONCURRENCY", + "langgraph_url": "LANGGRAPH_URL", + "langsmith_project": "LANGSMITH_PROJECT", + "assistant_id": "REVIEWER_ASSISTANT_ID", + "model_id": "REVIEWER_EVAL_MODEL_ID", + "reasoning_effort": "REVIEWER_EVAL_REASONING_EFFORT", + "score_mode": "REVIEWER_EVAL_SCORE_MODE", + "severity_threshold": "REVIEWER_EVAL_SEVERITY_THRESHOLD", + "cap": "REVIEWER_EVAL_CAP", +} class ReviewerEvalConfig(TypedDict, total=False): @@ -44,6 +60,28 @@ class ReviewerEvalConfig(TypedDict, total=False): cap: int +DEFAULT_CONFIG: ReviewerEvalConfig = { + "dataset_name": "openswe-reviewer-v1", + "experiment_prefix": "openswe-reviewer-baseline", + "max_concurrency": 5, + "langgraph_url": "", + "langsmith_project": DEFAULT_LANGSMITH_PROJECT, + "assistant_id": "reviewer", + "model_id": "google_genai:gemini-3.5-flash", + "reasoning_effort": "medium", + "score_mode": "all_findings", + "severity_threshold": "medium", + "cap": 4, +} + + +def _parse_int(value: str) -> int | None: + try: + return int(value) + except ValueError: + return None + + def _load_config() -> ReviewerEvalConfig: if not CONFIG_PATH.exists(): return {} @@ -87,11 +125,11 @@ def _coerce_config(raw: dict[str, Any]) -> ReviewerEvalConfig: config["max_concurrency"] = max_concurrency score_mode = raw.get("score_mode") - if score_mode in {"all_findings", "surfaced_findings"}: + if score_mode in _VALID_SCORE_MODES: config["score_mode"] = score_mode severity_threshold = raw.get("severity_threshold") - if severity_threshold in {"low", "medium", "high", "critical"}: + if severity_threshold in _VALID_SEVERITIES: config["severity_threshold"] = severity_threshold cap = raw.get("cap") @@ -100,17 +138,45 @@ def _coerce_config(raw: dict[str, Any]) -> ReviewerEvalConfig: return config -def _apply_config_to_env(config: ReviewerEvalConfig) -> None: - env_mapping = { - "langgraph_url": "LANGGRAPH_URL", - "assistant_id": "REVIEWER_ASSISTANT_ID", - "model_id": "REVIEWER_EVAL_MODEL_ID", - "reasoning_effort": "REVIEWER_EVAL_REASONING_EFFORT", - "score_mode": "REVIEWER_EVAL_SCORE_MODE", - "severity_threshold": "REVIEWER_EVAL_SEVERITY_THRESHOLD", - "cap": "REVIEWER_EVAL_CAP", +def _load_env_config(env: Mapping[str, str] = os.environ) -> ReviewerEvalConfig: + raw: dict[str, Any] = {} + for config_key, env_key in _ENV_MAPPING.items(): + value = env.get(env_key) + if value is None or value == "": + continue + if config_key in {"max_concurrency", "cap"}: + parsed = _parse_int(value) + if parsed is not None: + raw[config_key] = parsed + else: + raw[config_key] = value + return _coerce_config(raw) + + +def _config_from_args(args: argparse.Namespace) -> ReviewerEvalConfig: + raw: dict[str, Any] = {} + for key in _ENV_MAPPING: + value = getattr(args, key, None) + if value is not None: + raw[key] = value + return _coerce_config(raw) + + +def _resolve_config(cli_config: ReviewerEvalConfig | None = None) -> ReviewerEvalConfig: + resolved: ReviewerEvalConfig = { + **DEFAULT_CONFIG, + **_load_config(), + **_load_env_config(), } - for config_key, env_key in env_mapping.items(): + if cli_config is not None: + resolved.update(cli_config) + return resolved + + +def _apply_config_to_env(config: ReviewerEvalConfig) -> None: + for config_key, env_key in _ENV_MAPPING.items(): + if config_key == "langsmith_project": + continue value = config.get(config_key) if value is not None: os.environ[env_key] = str(value) @@ -120,10 +186,10 @@ def _apply_config_to_env(config: ReviewerEvalConfig) -> None: def _apply_langsmith_project(project: str | None) -> None: """Route eval traces to a dedicated LangSmith project. - A project already set in the environment (e.g. by the admin-triggered job) - wins so callers can override the config default. + ``project`` is expected to be the resolved value after CLI/env/config + precedence has already been applied. """ - resolved = os.environ.get("LANGSMITH_PROJECT") or project or DEFAULT_LANGSMITH_PROJECT + resolved = project or os.environ.get("LANGSMITH_PROJECT") or DEFAULT_LANGSMITH_PROJECT os.environ["LANGSMITH_PROJECT"] = resolved os.environ["LANGCHAIN_PROJECT"] = resolved os.environ.setdefault("LANGSMITH_TRACING", "true") @@ -144,22 +210,58 @@ async def _cleanup_threads(thread_ids: Iterable[str]) -> None: async def main() -> None: + logging.basicConfig( + level=os.environ.get("REVIEWER_EVAL_LOG_LEVEL", "INFO"), + format="%(asctime)s %(levelname)s %(name)s: %(message)s", + ) load_dotenv() - config = _load_config() - _apply_config_to_env(config) ap = argparse.ArgumentParser() ap.add_argument("--limit", type=int, default=None, help="Run only the first N examples.") + ap.add_argument("--dataset-name", dest="dataset_name") + ap.add_argument("--experiment-prefix", dest="experiment_prefix") + ap.add_argument("--max-concurrency", dest="max_concurrency", type=int) + ap.add_argument("--langgraph-url", dest="langgraph_url") + ap.add_argument("--langsmith-project", dest="langsmith_project") + ap.add_argument("--assistant-id", dest="assistant_id") + ap.add_argument("--model-id", dest="model_id") + ap.add_argument("--reasoning-effort", dest="reasoning_effort") + ap.add_argument("--score-mode", dest="score_mode", choices=sorted(_VALID_SCORE_MODES)) + ap.add_argument( + "--severity-threshold", + dest="severity_threshold", + choices=sorted(_VALID_SEVERITIES), + ) + ap.add_argument("--cap", type=int) ap.add_argument( "--no-cleanup", action="store_true", help="Skip deleting LangGraph threads after the experiment finishes.", ) args = ap.parse_args() + config = _resolve_config(_config_from_args(args)) + _apply_config_to_env(config) - dataset_name = config.get("dataset_name", "openswe-reviewer-v1") - experiment_prefix = config.get("experiment_prefix", "openswe-reviewer-baseline") - max_concurrency = config.get("max_concurrency", 5) + dataset_name = config["dataset_name"] + experiment_prefix = config["experiment_prefix"] + max_concurrency = config["max_concurrency"] + logger.info( + "Starting reviewer eval: dataset=%s experiment_prefix=%s max_concurrency=%s " + "model=%s effort=%s score_mode=%s severity_threshold=%s cap=%s project=%s " + "assistant_id=%s langgraph_url=%s limit=%s", + dataset_name, + experiment_prefix, + max_concurrency, + config["model_id"], + config["reasoning_effort"], + config["score_mode"], + config["severity_threshold"], + config["cap"], + config["langsmith_project"], + config["assistant_id"], + config["langgraph_url"] or "(default)", + args.limit, + ) data: str | list[Example] if args.limit: diff --git a/evals/reviewer/target.py b/evals/reviewer/target.py index 64f48a18..4b3a0bc8 100644 --- a/evals/reviewer/target.py +++ b/evals/reviewer/target.py @@ -9,6 +9,7 @@ verbatim form martian published. from __future__ import annotations +import logging import os import threading from typing import Any, Literal, cast @@ -17,6 +18,8 @@ from langgraph_sdk import get_client from agent.reviewer_findings import Finding, Severity, filter_findings_for_publish +logger = logging.getLogger(__name__) + DEFAULT_REVIEWER_ASSISTANT_ID = "reviewer" DEFAULT_LANGGRAPH_URL = "http://localhost:2024" ScoreMode = Literal["all_findings", "surfaced_findings"] @@ -110,19 +113,41 @@ def _build_configurable(inputs: dict[str, Any]) -> dict[str, Any]: async def review_pr(inputs: dict[str, Any]) -> dict[str, Any]: """LangSmith target: run the reviewer agent on one PR.""" + repo = inputs.get("repo", "") + pr_number = inputs.get("pr_number") + pr_url = inputs.get("pr_url", "") + logger.info( + "Starting reviewer eval example: repo=%s pr=%s url=%s", + repo, + pr_number, + pr_url, + ) client = get_client(url=get_langgraph_url()) thread = await client.threads.create() thread_id: str = thread["thread_id"] _record_thread_id(thread_id) - result = await client.runs.wait( - thread_id, - assistant_id=get_reviewer_assistant_id(), - input={"messages": [{"role": "user", "content": _build_user_message(inputs)}]}, - config={"configurable": _build_configurable(inputs)}, - ) - if get_score_mode() == "surfaced_findings": - return {"comments": await _extract_surfaced_comments(client, thread_id)} - return {"comments": _extract_comments(result)} + try: + result = await client.runs.wait( + thread_id, + assistant_id=get_reviewer_assistant_id(), + input={"messages": [{"role": "user", "content": _build_user_message(inputs)}]}, + config={"configurable": _build_configurable(inputs)}, + ) + if get_score_mode() == "surfaced_findings": + comments = await _extract_surfaced_comments(client, thread_id) + else: + comments = _extract_comments(result) + logger.info( + "Finished reviewer eval example: repo=%s pr=%s comments=%d thread_id=%s", + repo, + pr_number, + len(comments), + thread_id, + ) + return {"comments": comments} + except Exception: + logger.exception("Reviewer eval example failed: repo=%s pr=%s", repo, pr_number) + raise def _extract_comments(result: Any) -> list[dict[str, Any]]: diff --git a/tests/test_eval_jobs.py b/tests/test_eval_jobs.py index fe926b40..b748f45a 100644 --- a/tests/test_eval_jobs.py +++ b/tests/test_eval_jobs.py @@ -73,6 +73,8 @@ async def test_start_reviewer_eval_launches_subprocess() -> None: assert record["status"] == "running" assert record["limit"] == 3 + assert record["run_name"] == "openswe-review-confidence" + assert record["config_snapshot"]["model_id"] == "google_genai:gemini-3.5-flash" assert record["pid"] == 4321 assert record["heartbeat"] is not None assert record["worker_id"] == eval_jobs._WORKER_ID @@ -80,10 +82,60 @@ async def test_start_reviewer_eval_launches_subprocess() -> None: args, kwargs = create.call_args assert "--limit" in args and "3" in args + assert "--experiment-prefix" in args and "openswe-review-confidence" in args + assert "--model-id" in args and "google_genai:gemini-3.5-flash" in args + assert "--reasoning-effort" in args and "medium" in args assert kwargs["env"]["LANGSMITH_PROJECT"] == eval_jobs.DEFAULT_EVAL_PROJECT assert kwargs["env"]["LANGGRAPH_URL"] == "https://lg.test" +@pytest.mark.asyncio +async def test_start_reviewer_eval_uses_config_snapshot() -> None: + proc = MagicMock() + proc.pid = 4321 + proc.returncode = None + create = AsyncMock(return_value=proc) + config: eval_jobs.ReviewerEvalConfig = { + **eval_jobs.DEFAULT_REVIEWER_EVAL_CONFIG, + "experiment_prefix": "custom-run", + "langsmith_project": "custom-project", + "model_id": "anthropic:claude-opus-4-8", + "reasoning_effort": "high", + "score_mode": "surfaced_findings", + "severity_threshold": "high", + "cap": 2, + } + + def _consume(coro): + coro.close() + return MagicMock() + + with ( + patch.object(eval_jobs.asyncio, "create_subprocess_exec", new=create), + patch.object(eval_jobs.asyncio, "create_task", new=_consume), + patch.object(eval_jobs, "_get_record", new=AsyncMock(return_value=None)), + patch.object(eval_jobs, "_put_record", new=AsyncMock(side_effect=lambda r: r)), + ): + record = await eval_jobs.start_reviewer_eval( + limit=None, + config=config, + created_by="octo", + ) + + assert record["run_name"] == "custom-run" + assert record["langsmith_project"] == "custom-project" + assert record["config_snapshot"] == config + + args, kwargs = create.call_args + assert "--experiment-prefix" in args and "custom-run" in args + assert "--langsmith-project" in args and "custom-project" in args + assert "--model-id" in args and "anthropic:claude-opus-4-8" in args + assert "--score-mode" in args and "surfaced_findings" in args + assert "--severity-threshold" in args and "high" in args + assert "--cap" in args and "2" in args + assert kwargs["env"]["LANGSMITH_PROJECT"] == "custom-project" + + @pytest.mark.asyncio async def test_start_reviewer_eval_rejects_when_running() -> None: running = MagicMock() diff --git a/tests/test_reviewer_eval_run.py b/tests/test_reviewer_eval_run.py index 1b8cbfd9..6ab1ed54 100644 --- a/tests/test_reviewer_eval_run.py +++ b/tests/test_reviewer_eval_run.py @@ -3,11 +3,14 @@ from __future__ import annotations import os from unittest.mock import patch +from evals.reviewer import run_eval from evals.reviewer.run_eval import ( DEFAULT_LANGSMITH_PROJECT, _apply_config_to_env, _apply_langsmith_project, _coerce_config, + _load_env_config, + _resolve_config, ) @@ -46,7 +49,11 @@ def test_reviewer_eval_config_sets_target_env() -> None: with patch.dict(os.environ, {}, clear=True): _apply_config_to_env( { + "dataset_name": "dataset", + "experiment_prefix": "experiment", + "max_concurrency": 2, "langgraph_url": "https://example.test", + "langsmith_project": "project", "assistant_id": "reviewer", "model_id": "anthropic:claude-opus-4-8", "reasoning_effort": "high", @@ -56,7 +63,12 @@ def test_reviewer_eval_config_sets_target_env() -> None: } ) + assert os.environ["REVIEWER_EVAL_DATASET_NAME"] == "dataset" + assert os.environ["REVIEWER_EVAL_EXPERIMENT_PREFIX"] == "experiment" + assert os.environ["REVIEWER_EVAL_MAX_CONCURRENCY"] == "2" assert os.environ["LANGGRAPH_URL"] == "https://example.test" + assert os.environ["LANGSMITH_PROJECT"] == "project" + assert os.environ["LANGCHAIN_PROJECT"] == "project" assert os.environ["REVIEWER_ASSISTANT_ID"] == "reviewer" assert os.environ["REVIEWER_EVAL_MODEL_ID"] == "anthropic:claude-opus-4-8" assert os.environ["REVIEWER_EVAL_REASONING_EFFORT"] == "high" @@ -84,8 +96,76 @@ def test_apply_langsmith_project_falls_back_to_default() -> None: assert os.environ["LANGSMITH_PROJECT"] == DEFAULT_LANGSMITH_PROJECT -def test_apply_langsmith_project_env_overrides_config() -> None: +def test_apply_langsmith_project_uses_resolved_config_over_env() -> None: with patch.dict(os.environ, {"LANGSMITH_PROJECT": "from-env"}, clear=True): _apply_langsmith_project("from-config") - assert os.environ["LANGSMITH_PROJECT"] == "from-env" - assert os.environ["LANGCHAIN_PROJECT"] == "from-env" + assert os.environ["LANGSMITH_PROJECT"] == "from-config" + assert os.environ["LANGCHAIN_PROJECT"] == "from-config" + + +def test_load_env_config_reads_all_supported_keys() -> None: + env = { + "REVIEWER_EVAL_DATASET_NAME": "dataset-env", + "REVIEWER_EVAL_EXPERIMENT_PREFIX": "experiment-env", + "REVIEWER_EVAL_MAX_CONCURRENCY": "3", + "LANGGRAPH_URL": "https://lg.env", + "LANGSMITH_PROJECT": "project-env", + "REVIEWER_ASSISTANT_ID": "reviewer-env", + "REVIEWER_EVAL_MODEL_ID": "openai:gpt-5.5", + "REVIEWER_EVAL_REASONING_EFFORT": "xhigh", + "REVIEWER_EVAL_SCORE_MODE": "surfaced_findings", + "REVIEWER_EVAL_SEVERITY_THRESHOLD": "critical", + "REVIEWER_EVAL_CAP": "1", + } + + assert _load_env_config(env) == { + "dataset_name": "dataset-env", + "experiment_prefix": "experiment-env", + "max_concurrency": 3, + "langgraph_url": "https://lg.env", + "langsmith_project": "project-env", + "assistant_id": "reviewer-env", + "model_id": "openai:gpt-5.5", + "reasoning_effort": "xhigh", + "score_mode": "surfaced_findings", + "severity_threshold": "critical", + "cap": 1, + } + + +def test_resolve_config_prefers_cli_then_env_then_toml() -> None: + with ( + patch.object( + run_eval, + "_load_config", + return_value={ + "dataset_name": "dataset-config", + "experiment_prefix": "experiment-config", + "model_id": "anthropic:claude-opus-4-8", + "reasoning_effort": "high", + "langsmith_project": "project-config", + }, + ), + patch.dict( + os.environ, + { + "REVIEWER_EVAL_MODEL_ID": "google_genai:gemini-3.5-flash", + "REVIEWER_EVAL_REASONING_EFFORT": "medium", + "LANGSMITH_PROJECT": "project-env", + }, + clear=True, + ), + ): + config = _resolve_config( + { + "experiment_prefix": "experiment-cli", + "model_id": "openai:gpt-5.5", + "reasoning_effort": "xhigh", + } + ) + + assert config["dataset_name"] == "dataset-config" + assert config["experiment_prefix"] == "experiment-cli" + assert config["model_id"] == "openai:gpt-5.5" + assert config["reasoning_effort"] == "xhigh" + assert config["langsmith_project"] == "project-env" diff --git a/ui/src/lib/api.ts b/ui/src/lib/api.ts index 6a32026d..d806b79e 100644 --- a/ui/src/lib/api.ts +++ b/ui/src/lib/api.ts @@ -410,11 +410,34 @@ export function reviewChatApiBase( return path; } +export type ReviewerEvalScoreMode = "all_findings" | "surfaced_findings"; +export type ReviewerEvalSeverity = "low" | "medium" | "high" | "critical"; + +export interface ReviewerEvalConfig { + dataset_name: string; + experiment_prefix: string; + max_concurrency: number; + langsmith_project: string; + langgraph_url: string; + assistant_id: string; + model_id: string; + reasoning_effort: string; + score_mode: ReviewerEvalScoreMode; + severity_threshold: ReviewerEvalSeverity; + cap: number; +} + +export interface ReviewerEvalStartRequest extends ReviewerEvalConfig { + limit: number | null; +} + export interface ReviewerEvalStatus { name: string; status: "idle" | "running" | "completed" | "failed"; + run_name?: string; langsmith_project: string; limit: number | null; + config_snapshot?: ReviewerEvalConfig; started_at: string | null; finished_at: string | null; created_by: string | null; @@ -554,10 +577,10 @@ export const api = { ), getReviewerEval: () => request("/admin/evals/reviewer"), - startReviewerEval: (limit: number | null) => + startReviewerEval: (body: ReviewerEvalStartRequest) => request("/admin/evals/reviewer", { method: "POST", - body: JSON.stringify({ limit }), + body: JSON.stringify(body), }), cancelReviewerEval: () => request("/admin/evals/reviewer", { method: "DELETE" }), diff --git a/ui/src/routes/admin_.evals.tsx b/ui/src/routes/admin_.evals.tsx index 0b11dff5..acbf9d5b 100644 --- a/ui/src/routes/admin_.evals.tsx +++ b/ui/src/routes/admin_.evals.tsx @@ -1,14 +1,30 @@ import { Navigate, createFileRoute } from "@tanstack/react-router" import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query" import { useEffect, useRef, useState } from "react" +import type { ReactNode } from "react" -import type { ReviewerEvalStatus } from "@/lib/api" +import type { + ModelOption, + ReviewerEvalConfig, + ReviewerEvalScoreMode, + ReviewerEvalSeverity, + ReviewerEvalStartRequest, + ReviewerEvalStatus, +} from "@/lib/api" import { AppShell, SettingsSection } from "@/components/AppShell" import { Button } from "@/components/ui/button" import { Input } from "@/components/ui/input" +import { + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from "@/components/ui/select" import { Skeleton } from "@/components/ui/skeleton" import { api } from "@/lib/api" import { useSession } from "@/lib/session" +import { cn } from "@/lib/utils" export const Route = createFileRoute("/admin_/evals")({ component: ReviewerEvalPage }) @@ -38,10 +54,129 @@ function ReviewerEvalPage() { ) } +const DEFAULT_REVIEWER_EVAL_CONFIG: ReviewerEvalConfig = { + dataset_name: "openswe-reviewer-v1", + experiment_prefix: "openswe-review-confidence", + max_concurrency: 5, + langsmith_project: "open-swe-evals", + langgraph_url: "", + assistant_id: "reviewer", + model_id: "google_genai:gemini-3.5-flash", + reasoning_effort: "medium", + score_mode: "all_findings", + severity_threshold: "medium", + cap: 4, +} + +interface ReviewerEvalFormState { + dataset_name: string + experiment_prefix: string + max_concurrency: string + langsmith_project: string + langgraph_url: string + assistant_id: string + model_id: string + reasoning_effort: string + score_mode: ReviewerEvalScoreMode + severity_threshold: ReviewerEvalSeverity + cap: string + limit: string +} + +function formFromConfig( + config: ReviewerEvalConfig, + limit: number | null = null +): ReviewerEvalFormState { + return { + dataset_name: config.dataset_name, + experiment_prefix: config.experiment_prefix, + max_concurrency: String(config.max_concurrency), + langsmith_project: config.langsmith_project, + langgraph_url: config.langgraph_url, + assistant_id: config.assistant_id, + model_id: config.model_id, + reasoning_effort: config.reasoning_effort, + score_mode: config.score_mode, + severity_threshold: config.severity_threshold, + cap: String(config.cap), + limit: limit ? String(limit) : "", + } +} + +function parsePositiveInt(label: string, value: string): number { + const n = Number(value.trim()) + if (!Number.isInteger(n) || n <= 0) { + throw new Error(`${label} must be a positive whole number`) + } + return n +} + +function parseOptionalPositiveInt(label: string, value: string): number | null { + return value.trim() ? parsePositiveInt(label, value) : null +} + +function parseNonNegativeInt(label: string, value: string): number { + const n = Number(value.trim()) + if (!Number.isInteger(n) || n < 0) { + throw new Error(`${label} must be a non-negative whole number`) + } + return n +} + +function requireText(label: string, value: string): string { + const text = value.trim() + if (!text) throw new Error(`${label} is required`) + return text +} + +function FieldGroup({ + label, + description, + children, +}: { + label: string + description?: string + children: ReactNode +}) { + return ( +
+
+ {label} + {description && ( + {description} + )} +
+ {children} +
+ ) +} + +function Field({ + label, + className, + children, +}: { + label: string + className?: string + children: ReactNode +}) { + return ( +
+ + {label} + + {children} +
+ ) +} + function ReviewerEvalRunner() { const qc = useQueryClient() - const [limit, setLimit] = useState("") + const [draft, setDraft] = useState(() => + formFromConfig(DEFAULT_REVIEWER_EVAL_CONFIG) + ) const [error, setError] = useState(null) + const initialized = useRef(false) const status = useQuery({ queryKey: ["reviewerEval"], @@ -49,9 +184,61 @@ function ReviewerEvalRunner() { refetchInterval: (query) => query.state.data?.status === "running" ? 5000 : false, }) + const options = useQuery({ queryKey: ["options"], queryFn: api.options }) const data = status.data const running = data?.status === "running" + const currentModel: ModelOption | undefined = + options.data?.models.find((m) => m.id === draft.model_id) ?? + options.data?.models[0] + + useEffect(() => { + if (initialized.current || !data?.config_snapshot) return + initialized.current = true + setDraft(formFromConfig(data.config_snapshot, data.limit)) + }, [data?.config_snapshot, data?.limit]) + + useEffect(() => { + if (!currentModel) return + if (currentModel.id !== draft.model_id) { + setDraft((current) => ({ + ...current, + model_id: currentModel.id, + reasoning_effort: currentModel.default_effort, + })) + return + } + if (!currentModel.efforts.includes(draft.reasoning_effort)) { + setDraft((current) => ({ + ...current, + reasoning_effort: currentModel.default_effort, + })) + } + }, [currentModel, draft.model_id, draft.reasoning_effort]) + + const setField = ( + key: TKey, + value: ReviewerEvalFormState[TKey] + ) => { + setDraft((current) => ({ ...current, [key]: value })) + } + + const buildRequest = (): ReviewerEvalStartRequest => { + return { + dataset_name: requireText("Dataset", draft.dataset_name), + experiment_prefix: requireText("Run name", draft.experiment_prefix), + max_concurrency: parsePositiveInt("Max concurrency", draft.max_concurrency), + langsmith_project: requireText("LangSmith project", draft.langsmith_project), + langgraph_url: draft.langgraph_url.trim(), + assistant_id: requireText("Assistant ID", draft.assistant_id), + model_id: requireText("Model", draft.model_id), + reasoning_effort: requireText("Effort", draft.reasoning_effort), + score_mode: draft.score_mode, + severity_threshold: draft.severity_threshold, + cap: parseNonNegativeInt("Cap", draft.cap), + limit: parseOptionalPositiveInt("Limit", draft.limit), + } + } const onSuccess = (next: ReviewerEvalStatus) => { qc.setQueryData(["reviewerEval"], next) @@ -61,11 +248,7 @@ function ReviewerEvalRunner() { const start = useMutation({ mutationFn: () => { - const n = limit.trim() ? Number(limit.trim()) : null - if (n !== null && (!Number.isInteger(n) || n <= 0)) { - throw new Error("Limit must be a positive whole number") - } - return api.startReviewerEval(n) + return api.startReviewerEval(buildRequest()) }, onSuccess, onError, @@ -77,82 +260,335 @@ function ReviewerEvalRunner() { }) return ( - -
-
- setLimit(e.target.value)} - /> - - {running && ( - - )} -
- - {data && ( -
- - Status: {data.status} - {data.langsmith_project ? ` · ${data.langsmith_project}` : ""} - {data.limit ? ` · limit ${data.limit}` : ""} - - {data.started_at && ( - Started: {new Date(data.started_at).toLocaleString()} - )} - {data.finished_at && ( - - Finished: {new Date(data.finished_at).toLocaleString()} +
+ + setField("experiment_prefix", e.target.value)} + /> + + + setField("dataset_name", e.target.value)} + /> + + + setField("limit", e.target.value)} + /> + + + setField("max_concurrency", e.target.value)} + /> + +
+ + +
+ + + + + + +
+ + +
+ + + + + + + + setField("cap", e.target.value)} + /> + +
+ + +
+ + setField("langsmith_project", e.target.value)} + /> + + + setField("langgraph_url", e.target.value)} + /> + + + setField("assistant_id", e.target.value)} + /> + +
+ +
+
+ Start + + Only one reviewer eval can run at a time. - )} - {data.experiment_url && ( - +
+ + {running && ( + + )} +
- )} +
+ {error &&

{error}

} + - {error &&

{error}

} + + + + + ) +} + +function ReviewerEvalStatusView({ data }: { data: ReviewerEvalStatus | null }) { + if (!data) { + return ( +
+ Loading reviewer eval status…
- + ) + } + + const config = data.config_snapshot + return ( +
+ + + + + + + + + + + + + {data.started_at && ( + + )} + {data.finished_at && ( + + )} + {data.experiment_url && ( + + View experiment in LangSmith + + )} + {data.error && {data.error}} +
+ ) +} + +function StatusLine({ + label, + value, + strong = false, +}: { + label: string + value: string | null | undefined + strong?: boolean +}) { + return ( + + {label}:{" "} + + {value || "—"} + + ) } function ReviewerEvalLogs() { - const status = useQuery({ queryKey: ["reviewerEval"], queryFn: api.getReviewerEval }) + const status = useQuery({ + queryKey: ["reviewerEval"], + queryFn: api.getReviewerEval, + refetchInterval: (query) => + query.state.data?.status === "running" ? 5000 : false, + }) const logTail = status.data?.log_tail ?? null const running = status.data?.status === "running" const scrollRef = useRef(null) const [follow, setFollow] = useState(true) + const [copied, setCopied] = useState(false) useEffect(() => { if (follow && scrollRef.current) { @@ -160,19 +596,36 @@ function ReviewerEvalLogs() { } }, [logTail, follow]) + const copyLogs = async () => { + if (!logTail) return + await navigator.clipboard.writeText(logTail) + setCopied(true) + window.setTimeout(() => setCopied(false), 1500) + } + return ( - setFollow(e.target.checked)} - /> - Follow - +
+ + +
} >