From 834efbc33cc7b89fb9b9a9fc937f4bb496fd617f Mon Sep 17 00:00:00 2001 From: Johannes du Plessis Date: Mon, 18 May 2026 15:47:13 -0700 Subject: [PATCH] feat: Adds ability to run evals against deployment (#1311) * feat: tighten reviewer eval workflow Require the reviewer to verify and dedupe findings before recording them, and make benchmark runs safe to execute against deployed reviewer graphs without posting GitHub reviews. * chore: move reviewer eval settings to config Load reviewer benchmark settings from the default eval config file so deployed eval runs do not require a wide CLI surface. * feat: allow reviewer eval model overrides Pass reviewer model and reasoning effort from the eval config into reviewer runs so isolated benchmark deployments can test Opus 4.7 high thinking. * fix: use adaptive thinking for Opus 4.7 Switch Opus 4.7 model overrides to Anthropic adaptive thinking with effort instead of the deprecated budgeted thinking payload rejected by the API. * refactor: use latest Anthropic effort API Remove legacy Anthropic budget-token thinking support and route Anthropic efforts through adaptive thinking plus effort. * revert prompting --- agent/reviewer.py | 24 +++++- agent/server.py | 34 ++++----- agent/tools/publish_review.py | 49 ++++++++++++ agent/utils/model.py | 5 +- evals/reviewer/README.md | 18 ++++- evals/reviewer/config.toml | 15 ++++ evals/reviewer/run_eval.py | 116 +++++++++++++++++++++++++---- evals/reviewer/target.py | 110 +++++++++++++++++++++++++-- tests/test_anthropic_effort.py | 13 ++++ tests/test_reviewer.py | 39 ++++++++++ tests/test_reviewer_eval_run.py | 60 +++++++++++++++ tests/test_reviewer_eval_target.py | 94 +++++++++++++++++++++++ tests/test_reviewer_publish.py | 39 ++++++++++ 13 files changed, 571 insertions(+), 45 deletions(-) create mode 100644 evals/reviewer/config.toml create mode 100644 tests/test_anthropic_effort.py create mode 100644 tests/test_reviewer_eval_run.py create mode 100644 tests/test_reviewer_eval_target.py diff --git a/agent/reviewer.py b/agent/reviewer.py index 6553cf2c..cd60ab06 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -45,6 +45,9 @@ from .server import ( DEFAULT_LLM_REASONING, DEFAULT_RECURSION_LIMIT, MODEL_CALL_RECURSION_LIMIT, + _anthropic_effort_for, + _anthropic_thinking_for, + _openai_reasoning_for, ensure_sandbox_for_thread, graph_loaded_for_execution, ) @@ -329,10 +332,25 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: head_sha=head_sha, ) - model_id = os.environ.get("LLM_MODEL_ID", DEFAULT_LLM_MODEL_ID) + configured_model_id = config["configurable"].get("reviewer_model_id") + model_id = ( + configured_model_id + if isinstance(configured_model_id, str) and configured_model_id + else os.environ.get("LLM_MODEL_ID", DEFAULT_LLM_MODEL_ID) + ) + configured_effort = config["configurable"].get("reviewer_reasoning_effort") + reasoning_effort = configured_effort if isinstance(configured_effort, str) else None model_kwargs: ModelKwargs = {"max_tokens": DEFAULT_LLM_MAX_TOKENS} - if model_id == DEFAULT_LLM_MODEL_ID: - model_kwargs["reasoning"] = DEFAULT_LLM_REASONING + if model_id.startswith("openai:"): + reasoning = _openai_reasoning_for(reasoning_effort) + model_kwargs["reasoning"] = reasoning if reasoning is not None else DEFAULT_LLM_REASONING + elif model_id.startswith("anthropic:"): + thinking = _anthropic_thinking_for(reasoning_effort) + if thinking is not None: + model_kwargs["thinking"] = thinking + effort = _anthropic_effort_for(reasoning_effort) + if effort is not None: + model_kwargs["effort"] = effort system_prompt = _reviewer_system_prompt( f"{work_dir}/{repo_name}" if repo_name else work_dir, diff --git a/agent/server.py b/agent/server.py index b055db63..923fbbd6 100644 --- a/agent/server.py +++ b/agent/server.py @@ -64,6 +64,7 @@ from .utils.auth import resolve_github_token from .utils.authorship import resolve_triggering_user_identity from .utils.github_app import get_github_app_installation_token from .utils.model import ( + AnthropicEffort, AnthropicThinking, ModelKwargs, OpenAIReasoning, @@ -359,30 +360,24 @@ def _openai_reasoning_for(profile_effort: str | None) -> OpenAIReasoning | None: return None -# Mapping mirrors Claude Code's effort levels for Opus 4.7. Numbers are tuned -# so each level meaningfully separates from the next while leaving headroom -# under DEFAULT_LLM_MAX_TOKENS (64k) for the model's actual output. -_ANTHROPIC_THINKING_BUDGETS: dict[str, int] = { - "low": 1_024, - "medium": 4_000, - "high": 12_000, - "xhigh": 32_000, - "max": 60_000, -} +_ANTHROPIC_EFFORTS: set[AnthropicEffort] = {"low", "medium", "high", "xhigh", "max"} def _anthropic_thinking_for(profile_effort: str | None) -> AnthropicThinking | None: """Map a profile effort string to an Anthropic thinking kwarg. - Returns ``None`` when the effort doesn't have a known budget so we leave - the model's default thinking behaviour alone. + Latest Claude models use adaptive thinking with an effort hint instead of + manual thinking token budgets. """ - if not profile_effort: - return None - budget = _ANTHROPIC_THINKING_BUDGETS.get(profile_effort) - if budget is None: - return None - return {"type": "enabled", "budget_tokens": budget} + if profile_effort in _ANTHROPIC_EFFORTS: + return {"type": "adaptive"} + return None + + +def _anthropic_effort_for(profile_effort: str | None) -> AnthropicEffort | None: + if profile_effort in _ANTHROPIC_EFFORTS: + return profile_effort + return None def _get_cached_sandbox_backend(thread_id: str) -> SandboxBackendProtocol: @@ -450,6 +445,9 @@ async def get_agent(config: RunnableConfig) -> Pregel: thinking = _anthropic_thinking_for(profile_effort) if thinking is not None: model_kwargs["thinking"] = thinking + effort = _anthropic_effort_for(profile_effort) + if effort is not None: + model_kwargs["effort"] = effort fallback_model_id = os.environ.get("LLM_FALLBACK_MODEL_ID") or fallback_model_id_for(model_id) fallback_middleware: list[Any] = [] diff --git a/agent/tools/publish_review.py b/agent/tools/publish_review.py index 9ea87620..77f29276 100644 --- a/agent/tools/publish_review.py +++ b/agent/tools/publish_review.py @@ -90,6 +90,15 @@ def publish_review( if not isinstance(head_sha, str) or not head_sha: return {"success": False, "error": "Missing head_sha in run config"} + if _is_reviewer_eval_mode(configurable): + return asyncio.run( + _publish_review_eval_dry_run_async( + head_sha=head_sha, + severity_threshold=_cast_severity(severity_threshold), + cap=cap, + ) + ) + token = get_github_token() if not token: return {"success": False, "error": "No GitHub token available"} @@ -125,6 +134,46 @@ def _cast_severity(value: str) -> Severity: return value # type: ignore[return-value] +def _is_reviewer_eval_mode(configurable: dict[str, Any]) -> bool: + return configurable.get("reviewer_eval") is True or configurable.get("eval") is True + + +async def _publish_review_eval_dry_run_async( + *, + head_sha: str, + severity_threshold: Severity, + cap: int, +) -> dict[str, Any]: + """Simulate publish_review for benchmark runs without posting to GitHub.""" + thread_id = get_thread_id_from_runtime() + findings = await list_findings_async(thread_id) + unpublished_findings = [ + f for f in findings if not isinstance(f.get("github_review_comment_id"), int) + ] + open_unpublished = [f for f in unpublished_findings if f.get("status", "open") == "open"] + eligible = filter_findings_for_publish( + unpublished_findings, + severity_threshold=severity_threshold, + cap=cap, + ) + inline_comments = [ + payload + for finding in eligible + if (payload := render_inline_comment_payload(finding)) is not None + ] + + await set_reviewer_thread_metadata(thread_id, last_reviewed_sha=head_sha) + + return { + "success": True, + "dry_run": True, + "review_id": None, + "surfaced_count": len(inline_comments), + "hidden_count": max(len(open_unpublished) - len(inline_comments), 0), + "resolved_thread_count": 0, + } + + async def _publish_review_async( *, owner: str, diff --git a/agent/utils/model.py b/agent/utils/model.py index b954c29a..696c627c 100644 --- a/agent/utils/model.py +++ b/agent/utils/model.py @@ -10,7 +10,8 @@ DEFAULT_MAX_RETRIES = 6 OpenAIReasoningEffort = Literal["none", "low", "medium", "high", "xhigh"] -AnthropicThinkingType = Literal["enabled", "disabled"] +AnthropicThinkingType = Literal["adaptive"] +AnthropicEffort = Literal["low", "medium", "high", "xhigh", "max"] class OpenAIReasoning(TypedDict, total=False): @@ -19,13 +20,13 @@ class OpenAIReasoning(TypedDict, total=False): class AnthropicThinking(TypedDict, total=False): type: AnthropicThinkingType - budget_tokens: int class ModelKwargs(TypedDict, total=False): max_tokens: int | None reasoning: OpenAIReasoning | None thinking: AnthropicThinking | None + effort: AnthropicEffort | None temperature: float | None max_retries: int | None diff --git a/evals/reviewer/README.md b/evals/reviewer/README.md index c657a752..484fc2c2 100644 --- a/evals/reviewer/README.md +++ b/evals/reviewer/README.md @@ -10,6 +10,7 @@ root for the full design. evals/reviewer/ ├── golden_comments/ # 50 PRs × golden comments (copied from martian benchmark) ├── build_dataset.py # martian JSON → LangSmith dataset (resolves SHAs via gh) +├── config.toml # default benchmark run config ├── judge.py # claude-opus-4-5 pairwise match evaluator + aggregate ├── target.py # invokes the reviewer graph over langgraph_sdk └── run_eval.py # client.aevaluate entrypoint @@ -45,9 +46,7 @@ example schema, and must emit a `submit_review` tool call (or set `state["review"]["comments"]`) with `[{file, line, severity, body}, ...]`. ```bash -uv run python -m evals.reviewer.run_eval \ - --experiment-prefix openswe-reviewer-baseline \ - --max-concurrency 5 +uv run python -m evals.reviewer.run_eval ``` Smoke-test with 3 PRs first: @@ -56,6 +55,19 @@ Smoke-test with 3 PRs first: uv run python -m evals.reviewer.run_eval --limit 3 ``` +The runner reads benchmark settings from `evals/reviewer/config.toml`. Set the +deployment URL there (or leave it blank to use `LANGGRAPH_URL` / local dev). +The target sets `reviewer_eval` for every run, so `publish_review` does not post +to GitHub. + +By default the judge scores final `add_finding` calls. Set +`score_mode = "surfaced_findings"` in the config to score only findings that +would pass the production threshold/cap. + +`model_id` and `reasoning_effort` in the config are passed to the reviewer run, +so isolated benchmark deployments can test a specific model/effort without +changing deployment-wide defaults. + ## Comparing against Devin Review Both tools are scored on the same 50 PRs with the same judge model diff --git a/evals/reviewer/config.toml b/evals/reviewer/config.toml new file mode 100644 index 00000000..dfc7907b --- /dev/null +++ b/evals/reviewer/config.toml @@ -0,0 +1,15 @@ +dataset_name = "openswe-reviewer-v1" +experiment_prefix = "openswe-reviewer-verified-deployed" +max_concurrency = 2 + +# Leave blank to use LANGGRAPH_URL or local dev. +langgraph_url = "" +assistant_id = "reviewer" +model_id = "anthropic:claude-opus-4-7" +reasoning_effort = "high" + +# Use "surfaced_findings" to score only findings that pass the production +# severity threshold and cap. +score_mode = "all_findings" +severity_threshold = "medium" +cap = 4 diff --git a/evals/reviewer/run_eval.py b/evals/reviewer/run_eval.py index ca026306..a4fe385a 100644 --- a/evals/reviewer/run_eval.py +++ b/evals/reviewer/run_eval.py @@ -1,17 +1,18 @@ """Run the reviewer eval against the LangSmith dataset. Usage: - uv run python -m evals.reviewer.run_eval \\ - --dataset-name openswe-reviewer-v1 \\ - --experiment-prefix openswe-reviewer-baseline \\ - --max-concurrency 5 + uv run python -m evals.reviewer.run_eval """ from __future__ import annotations import argparse import logging +import os +import tomllib from collections.abc import Iterable +from pathlib import Path +from typing import Any, Literal, TypedDict from dotenv import load_dotenv from langgraph_sdk import get_client @@ -19,12 +20,97 @@ from langsmith import Client, aevaluate from langsmith.schemas import Example from evals.reviewer.judge import aggregate_pr, judge_match -from evals.reviewer.target import LANGGRAPH_URL, drain_thread_ids, review_pr +from evals.reviewer.target import drain_thread_ids, get_langgraph_url, review_pr load_dotenv() logger = logging.getLogger(__name__) +CONFIG_PATH = Path(__file__).with_name("config.toml") +ScoreMode = Literal["all_findings", "surfaced_findings"] +Severity = Literal["informational", "low", "medium", "high", "critical"] + + +class ReviewerEvalConfig(TypedDict, total=False): + dataset_name: str + experiment_prefix: str + max_concurrency: int + langgraph_url: str + assistant_id: str + model_id: str + reasoning_effort: str + score_mode: ScoreMode + severity_threshold: Severity + cap: int + + +def _load_config() -> ReviewerEvalConfig: + if not CONFIG_PATH.exists(): + return {} + with CONFIG_PATH.open("rb") as f: + raw = tomllib.load(f) + return _coerce_config(raw) + + +def _coerce_config(raw: dict[str, Any]) -> ReviewerEvalConfig: + config: ReviewerEvalConfig = {} + dataset_name = raw.get("dataset_name") + if isinstance(dataset_name, str) and dataset_name: + config["dataset_name"] = dataset_name + + experiment_prefix = raw.get("experiment_prefix") + if isinstance(experiment_prefix, str) and experiment_prefix: + config["experiment_prefix"] = experiment_prefix + + langgraph_url = raw.get("langgraph_url") + if isinstance(langgraph_url, str) and langgraph_url: + config["langgraph_url"] = langgraph_url + + assistant_id = raw.get("assistant_id") + if isinstance(assistant_id, str) and assistant_id: + config["assistant_id"] = assistant_id + + model_id = raw.get("model_id") + if isinstance(model_id, str) and model_id: + config["model_id"] = model_id + + reasoning_effort = raw.get("reasoning_effort") + if isinstance(reasoning_effort, str) and reasoning_effort: + config["reasoning_effort"] = reasoning_effort + + max_concurrency = raw.get("max_concurrency") + if isinstance(max_concurrency, int) and max_concurrency > 0: + config["max_concurrency"] = max_concurrency + + score_mode = raw.get("score_mode") + if score_mode in {"all_findings", "surfaced_findings"}: + config["score_mode"] = score_mode + + severity_threshold = raw.get("severity_threshold") + if severity_threshold in {"informational", "low", "medium", "high", "critical"}: + config["severity_threshold"] = severity_threshold + + cap = raw.get("cap") + if isinstance(cap, int) and cap >= 0: + config["cap"] = cap + 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", + } + for config_key, env_key in env_mapping.items(): + value = config.get(config_key) + if value is not None: + os.environ[env_key] = str(value) + async def _cleanup_threads(thread_ids: Iterable[str]) -> None: """Delete LangGraph threads created during the eval. @@ -32,7 +118,7 @@ async def _cleanup_threads(thread_ids: Iterable[str]) -> None: Underlying sandboxes are reclaimed by the provider's TTL — this only drops the LangGraph checkpoint/metadata records. """ - sdk = get_client(url=LANGGRAPH_URL) + sdk = get_client(url=get_langgraph_url()) for tid in thread_ids: try: await sdk.threads.delete(tid) @@ -41,10 +127,10 @@ async def _cleanup_threads(thread_ids: Iterable[str]) -> None: async def main() -> None: + config = _load_config() + _apply_config_to_env(config) + ap = argparse.ArgumentParser() - ap.add_argument("--dataset-name", default="openswe-reviewer-v1") - ap.add_argument("--experiment-prefix", default="openswe-reviewer-baseline") - ap.add_argument("--max-concurrency", type=int, default=5) ap.add_argument("--limit", type=int, default=None, help="Run only the first N examples.") ap.add_argument( "--no-cleanup", @@ -53,12 +139,16 @@ async def main() -> None: ) args = ap.parse_args() + 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) + data: str | list[Example] if args.limit: client = Client() - data = list(client.list_examples(dataset_name=args.dataset_name, limit=args.limit)) + data = list(client.list_examples(dataset_name=dataset_name, limit=args.limit)) else: - data = args.dataset_name + data = dataset_name try: await aevaluate( @@ -66,8 +156,8 @@ async def main() -> None: data=data, evaluators=[judge_match], summary_evaluators=[aggregate_pr], - experiment_prefix=args.experiment_prefix, - max_concurrency=args.max_concurrency, + experiment_prefix=experiment_prefix, + max_concurrency=max_concurrency, num_repetitions=1, ) finally: diff --git a/evals/reviewer/target.py b/evals/reviewer/target.py index cd8afbb6..adaa83c0 100644 --- a/evals/reviewer/target.py +++ b/evals/reviewer/target.py @@ -11,12 +11,20 @@ from __future__ import annotations import os import threading -from typing import Any +from typing import Any, Literal, cast +from dotenv import load_dotenv from langgraph_sdk import get_client -REVIEWER_ASSISTANT_ID = os.getenv("REVIEWER_ASSISTANT_ID", "reviewer") -LANGGRAPH_URL = os.getenv("LANGGRAPH_URL", "http://localhost:2024") +from agent.reviewer_findings import Finding, Severity, filter_findings_for_publish + +load_dotenv() + +DEFAULT_REVIEWER_ASSISTANT_ID = "reviewer" +DEFAULT_LANGGRAPH_URL = "http://localhost:2024" +ScoreMode = Literal["all_findings", "surfaced_findings"] +_VALID_SCORE_MODES: set[ScoreMode] = {"all_findings", "surfaced_findings"} +_VALID_SEVERITIES: set[Severity] = {"informational", "low", "medium", "high", "critical"} _THREAD_IDS: set[str] = set() _THREAD_IDS_LOCK = threading.Lock() @@ -40,6 +48,31 @@ def drain_thread_ids() -> set[str]: return snapshot +def get_langgraph_url() -> str: + return os.getenv("LANGGRAPH_URL", DEFAULT_LANGGRAPH_URL) + + +def get_reviewer_assistant_id() -> str: + return os.getenv("REVIEWER_ASSISTANT_ID", DEFAULT_REVIEWER_ASSISTANT_ID) + + +def get_score_mode() -> ScoreMode: + value = os.getenv("REVIEWER_EVAL_SCORE_MODE", "all_findings") + if value in _VALID_SCORE_MODES: + return cast(ScoreMode, value) + return "all_findings" + + +def get_reviewer_model_id() -> str | None: + value = os.getenv("REVIEWER_EVAL_MODEL_ID") + return value if value else None + + +def get_reviewer_reasoning_effort() -> str | None: + value = os.getenv("REVIEWER_EVAL_REASONING_EFFORT") + return value if value else None + + def _build_user_message(inputs: dict[str, Any]) -> str: return ( f"Review pull request {inputs['pr_url']}.\n\n" @@ -58,8 +91,10 @@ def _build_user_message(inputs: dict[str, Any]) -> str: def _build_configurable(inputs: dict[str, Any]) -> dict[str, Any]: repo = inputs.get("repo", "") owner, _, name = repo.partition("/") if isinstance(repo, str) else ("", "", "") - return { + configurable: dict[str, Any] = { "__is_for_execution__": True, + "reviewer_eval": True, + "eval": True, "repo": {"owner": owner, "name": name}, "pr_number": inputs.get("pr_number"), "pr_url": inputs.get("pr_url", ""), @@ -67,20 +102,29 @@ def _build_configurable(inputs: dict[str, Any]) -> dict[str, Any]: "head_sha": inputs.get("head_sha", ""), "branch_name": inputs.get("head_ref", ""), } + model_id = get_reviewer_model_id() + if model_id: + configurable["reviewer_model_id"] = model_id + reasoning_effort = get_reviewer_reasoning_effort() + if reasoning_effort: + configurable["reviewer_reasoning_effort"] = reasoning_effort + return configurable async def review_pr(inputs: dict[str, Any]) -> dict[str, Any]: """LangSmith target: run the reviewer agent on one PR.""" - client = get_client(url=LANGGRAPH_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=REVIEWER_ASSISTANT_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)} @@ -118,3 +162,57 @@ def _extract_comments(result: Any) -> list[dict[str, Any]]: } ) return comments + + +async def _extract_surfaced_comments(client: Any, thread_id: str) -> list[dict[str, Any]]: + thread = await client.threads.get(thread_id) + metadata = thread.get("metadata") if isinstance(thread, dict) else None + findings_value = metadata.get("findings") if isinstance(metadata, dict) else None + findings = _coerce_findings(findings_value) + surfaced = filter_findings_for_publish( + findings, + severity_threshold=_score_severity_threshold(), + cap=_score_cap(), + ) + return [_normalize_finding(finding) for finding in surfaced] + + +def _coerce_findings(value: Any) -> list[Finding]: + if not isinstance(value, list): + return [] + findings: list[Finding] = [] + for item in value: + if not isinstance(item, dict): + continue + if not isinstance(item.get("id"), str): + continue + findings.append(cast(Finding, item)) + return findings + + +def _normalize_finding(finding: Finding) -> dict[str, Any]: + line = finding.get("end_line") + if line is None: + line = finding.get("start_line") + return { + "file": finding.get("file"), + "line": line, + "body": finding.get("description", ""), + "severity": finding.get("severity"), + } + + +def _score_severity_threshold() -> Severity: + value = os.getenv("REVIEWER_EVAL_SEVERITY_THRESHOLD", "medium") + if value in _VALID_SEVERITIES: + return cast(Severity, value) + return "medium" + + +def _score_cap() -> int: + raw = os.getenv("REVIEWER_EVAL_CAP", "4") + try: + cap = int(raw) + except ValueError: + return 4 + return max(cap, 0) diff --git a/tests/test_anthropic_effort.py b/tests/test_anthropic_effort.py new file mode 100644 index 00000000..64509fe8 --- /dev/null +++ b/tests/test_anthropic_effort.py @@ -0,0 +1,13 @@ +from __future__ import annotations + +from agent.server import _anthropic_effort_for, _anthropic_thinking_for + + +def test_anthropic_uses_adaptive_thinking_and_effort() -> None: + assert _anthropic_thinking_for("high") == {"type": "adaptive"} + assert _anthropic_effort_for("high") == "high" + + +def test_anthropic_ignores_unknown_effort() -> None: + assert _anthropic_thinking_for("unknown") is None + assert _anthropic_effort_for("unknown") is None diff --git a/tests/test_reviewer.py b/tests/test_reviewer.py index fffb558f..defa6167 100644 --- a/tests/test_reviewer.py +++ b/tests/test_reviewer.py @@ -54,3 +54,42 @@ async def test_reviewer_uses_cached_thread_token_for_slack_review_request() -> N assert metadata["github_token_encrypted"] == "encrypted-token" mock_get_thread_token.assert_awaited_once_with("reviewer-thread-id") mock_resolve_token.assert_not_called() + + +@pytest.mark.asyncio +async def test_reviewer_applies_eval_model_and_effort_overrides() -> None: + config: RunnableConfig = { + "configurable": { + "__is_for_execution__": True, + "thread_id": "reviewer-thread-id", + "repo": {"owner": "acme", "name": "repo"}, + "pr_number": 1, + "pr_url": "https://github.com/acme/repo/pull/1", + "base_sha": "base", + "head_sha": "head", + "reviewer_model_id": "anthropic:claude-opus-4-7", + "reviewer_reasoning_effort": "high", + }, + "metadata": {}, + } + dummy_agent = _DummyAgent() + + with ( + patch( + "agent.reviewer.ensure_sandbox_for_thread", + new_callable=AsyncMock, + return_value=MagicMock(), + ), + patch( + "agent.reviewer.aresolve_sandbox_work_dir", + new_callable=AsyncMock, + return_value="/workspace", + ), + patch("agent.reviewer.make_model", return_value=MagicMock()) as make_model, + patch("agent.reviewer.create_deep_agent", return_value=dummy_agent), + ): + await reviewer.get_reviewer_agent(config) + + assert make_model.call_args.args == ("anthropic:claude-opus-4-7",) + assert make_model.call_args.kwargs["thinking"] == {"type": "adaptive"} + assert make_model.call_args.kwargs["effort"] == "high" diff --git a/tests/test_reviewer_eval_run.py b/tests/test_reviewer_eval_run.py new file mode 100644 index 00000000..b546d1ad --- /dev/null +++ b/tests/test_reviewer_eval_run.py @@ -0,0 +1,60 @@ +from __future__ import annotations + +import os +from unittest.mock import patch + +from evals.reviewer.run_eval import _apply_config_to_env, _coerce_config + + +def test_reviewer_eval_config_coerces_known_values() -> None: + config = _coerce_config( + { + "dataset_name": "dataset", + "experiment_prefix": "experiment", + "max_concurrency": 2, + "langgraph_url": "https://example.test", + "assistant_id": "reviewer", + "model_id": "anthropic:claude-opus-4-7", + "reasoning_effort": "high", + "score_mode": "surfaced_findings", + "severity_threshold": "medium", + "cap": 4, + "unknown": "ignored", + } + ) + + assert config == { + "dataset_name": "dataset", + "experiment_prefix": "experiment", + "max_concurrency": 2, + "langgraph_url": "https://example.test", + "assistant_id": "reviewer", + "model_id": "anthropic:claude-opus-4-7", + "reasoning_effort": "high", + "score_mode": "surfaced_findings", + "severity_threshold": "medium", + "cap": 4, + } + + +def test_reviewer_eval_config_sets_target_env() -> None: + with patch.dict(os.environ, {}, clear=True): + _apply_config_to_env( + { + "langgraph_url": "https://example.test", + "assistant_id": "reviewer", + "model_id": "anthropic:claude-opus-4-7", + "reasoning_effort": "high", + "score_mode": "surfaced_findings", + "severity_threshold": "high", + "cap": 3, + } + ) + + assert os.environ["LANGGRAPH_URL"] == "https://example.test" + assert os.environ["REVIEWER_ASSISTANT_ID"] == "reviewer" + assert os.environ["REVIEWER_EVAL_MODEL_ID"] == "anthropic:claude-opus-4-7" + assert os.environ["REVIEWER_EVAL_REASONING_EFFORT"] == "high" + assert os.environ["REVIEWER_EVAL_SCORE_MODE"] == "surfaced_findings" + assert os.environ["REVIEWER_EVAL_SEVERITY_THRESHOLD"] == "high" + assert os.environ["REVIEWER_EVAL_CAP"] == "3" diff --git a/tests/test_reviewer_eval_target.py b/tests/test_reviewer_eval_target.py new file mode 100644 index 00000000..647f1f2c --- /dev/null +++ b/tests/test_reviewer_eval_target.py @@ -0,0 +1,94 @@ +from __future__ import annotations + +from typing import Any + +import pytest + +from agent.reviewer_findings import new_finding +from evals.reviewer import target + + +def test_eval_target_marks_runs_as_eval_dry_run(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("REVIEWER_EVAL_MODEL_ID", raising=False) + monkeypatch.delenv("REVIEWER_EVAL_REASONING_EFFORT", raising=False) + configurable = target._build_configurable( + { + "repo": "acme/repo", + "pr_number": 1, + "pr_url": "https://github.com/acme/repo/pull/1", + "base_sha": "base", + "head_sha": "head", + "head_ref": "branch", + } + ) + + assert configurable["reviewer_eval"] is True + assert configurable["eval"] is True + assert configurable["__is_for_execution__"] is True + assert "source" not in configurable + + +def test_eval_target_passes_model_overrides(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("REVIEWER_EVAL_MODEL_ID", "anthropic:claude-opus-4-7") + monkeypatch.setenv("REVIEWER_EVAL_REASONING_EFFORT", "high") + + configurable = target._build_configurable( + { + "repo": "acme/repo", + "pr_number": 1, + "pr_url": "https://github.com/acme/repo/pull/1", + "base_sha": "base", + "head_sha": "head", + "head_ref": "branch", + } + ) + + assert configurable["reviewer_model_id"] == "anthropic:claude-opus-4-7" + assert configurable["reviewer_reasoning_effort"] == "high" + + +@pytest.mark.asyncio +async def test_extract_surfaced_comments_uses_publish_filter( + monkeypatch: pytest.MonkeyPatch, +) -> None: + high = new_finding( + severity="high", + category="correctness", + file="a.py", + start_line=10, + end_line=10, + description="high signal", + sha="head", + finding_id="f_high", + ) + low = new_finding( + severity="low", + category="style", + file="b.py", + start_line=20, + end_line=20, + description="low signal", + sha="head", + finding_id="f_low", + ) + + class Threads: + async def get(self, _thread_id: str) -> dict[str, Any]: + return {"metadata": {"findings": [high, low]}} + + class Client: + threads = Threads() + + monkeypatch.setenv("REVIEWER_EVAL_SEVERITY_THRESHOLD", "medium") + monkeypatch.setenv("REVIEWER_EVAL_CAP", "4") + + comments = await target._extract_surfaced_comments(Client(), "tid") + + assert comments == [ + { + "file": "a.py", + "line": 10, + "body": "high signal", + "severity": "high", + } + ] diff --git a/tests/test_reviewer_publish.py b/tests/test_reviewer_publish.py index 360132bb..035f39c0 100644 --- a/tests/test_reviewer_publish.py +++ b/tests/test_reviewer_publish.py @@ -86,6 +86,45 @@ def test_render_review_body_no_findings_message() -> None: assert "" in body +def test_publish_review_eval_mode_does_not_call_github() -> None: + from agent.tools.publish_review import publish_review + + findings = [ + _f(id="f_high", severity="high", file="a.py", start_line=1, end_line=1), + _f(id="f_low", severity="low", file="b.py", start_line=2, end_line=2), + ] + + with ( + patch( + "agent.tools.publish_review.get_config", + return_value={ + "configurable": { + "thread_id": "tid", + "repo": {"owner": "o", "name": "r"}, + "pr_number": 7, + "head_sha": "sha", + "reviewer_eval": True, + }, + "metadata": {}, + }, + ), + patch("agent.tools.publish_review.get_thread_id_from_runtime", return_value="tid"), + patch("agent.tools.publish_review.list_findings_async", AsyncMock(return_value=findings)), + patch("agent.tools.publish_review.set_reviewer_thread_metadata", AsyncMock()) as set_meta, + patch("agent.tools.publish_review.get_github_token") as get_token, + patch("agent.tools.publish_review.post_pull_request_review", AsyncMock()) as post_review, + ): + result = publish_review() + + assert result["success"] is True + assert result["dry_run"] is True + assert result["surfaced_count"] == 1 + assert result["hidden_count"] == 1 + get_token.assert_not_called() + post_review.assert_not_called() + set_meta.assert_awaited_once_with("tid", last_reviewed_sha="sha") + + @pytest.mark.asyncio async def test_resolve_review_thread_returns_true_on_success() -> None: response = MagicMock()