mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 06:53:14 +00:00
feat: tune reviewer for precision — web/wiki tools + recalibrated prompt (#1312)
* feat: tune reviewer for precision — web/wiki tools + recalibrated prompt Reviewer agent now has web_search, fetch_url, and http_request alongside the finding tools, so it can verify library semantics and consult the DeepWiki auto-generated wiki for public repos (https://deepwiki.com/<owner>/<repo>) before flagging cross-file or architectural concerns. Prompt rewritten to push precision over recall: - explicit severity ladder pushing reviews toward bimodal high/low instead of defaulting to medium - ≤200-char description target (gold set averages ~186 chars; we were at ~436) - mandatory docs / wiki / code lookup before flagging concurrency, security, or perf — the three categories that dominated false positives - "do not flag" list covering compiler/linter-catchable nits, speculative claims without a concrete attacker/interleaving/scale, style preferences the codebase doesn't share, and test-quality nits on non-test diffs - smart file-selection guidance for large PRs (deprioritize generated / vendored / pure-rename hunks) Eval config switched to openai:gpt-5.5 + high reasoning effort for the next benchmark run. * trim prompt * subagent prompting * confidence ratings * added medium * enforce confidence threshold * . * reviewer: precision-tuned prompt + drop confidence gate Rewrites the reviewer system prompt around a defensibility bar (anchor + failure mode + maintainer wouldn't say "not a bug"), an explicit do-not-file list (style nits, speculation, scope-policing, same-bug fan-out), and a checklist of 10 bug archetypes drawn from a per-PR audit of the eval golden set. The audit showed 145 FPs in the last eval split ~28% speculative, ~26% style-nit, ~31% real-but-unscored (mostly same-archetype fan-out); the new prompt targets each class directly. Confidence is still recorded on every finding for post-hoc calibration but no longer gates publication — the audit showed the gate was a no-op (agent self-rated 65% of findings "high" regardless), and the prompt's defensibility bar is the actual discipline. Drops CONFIDENCE_ORDER, CONFIDENCE_THRESHOLD, the confidence_threshold kwarg on filter_findings_for_publish, the confidence_filtered score_mode, and the min_confidence kwarg on the eval target's _extract_comments — all dead once the gate is gone. Also removes the "informational" severity tier from the Severity enum, SEVERITY_ORDER, and all validators / tests / docstrings. It was reserved for FYI observations the dataset never rewards. * benchmax * adding google provider * slight steering * tuning * more tuning * fix * cleanup * reducing overfitting * Add per-repo review style profiles and inject them into the reviewer. Dashboard users can analyze historical PR review feedback per repository, edit the resulting style guide, and have it loaded from LangGraph Store at reviewer runtime (including Martian eval runs) keyed by owner/name. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix review style job errors leaking exception details to clients. Return generic dashboard messages while logging full stack traces server-side. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
aca40ca397
commit
82852f9eda
38 changed files with 2119 additions and 208 deletions
|
|
@ -363,7 +363,9 @@ LANGSMITH_TRACING_PROJECT_ID_PROD=""
|
|||
LANGSMITH_URL_PROD="https://smith.langchain.com"
|
||||
|
||||
# === LLM ===
|
||||
ANTHROPIC_API_KEY="" # Anthropic API key (default provider)
|
||||
ANTHROPIC_API_KEY="" # Anthropic API key
|
||||
OPENAI_API_KEY="" # OpenAI API key (when using openai: models)
|
||||
GOOGLE_API_KEY="" # Google AI API key (when using google_genai: models)
|
||||
|
||||
# === GitHub App (required) ===
|
||||
GITHUB_APP_ID="" # From step 3c
|
||||
|
|
|
|||
145
agent/dashboard/review_style_jobs.py
Normal file
145
agent/dashboard/review_style_jobs.py
Normal file
|
|
@ -0,0 +1,145 @@
|
|||
"""Kick off and sync per-repo review style analysis runs."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from langgraph_sdk import get_client
|
||||
|
||||
from ..review_style_collector import (
|
||||
collect_review_samples,
|
||||
format_samples_for_analyzer,
|
||||
generate_review_style_thread_id,
|
||||
)
|
||||
from .review_styles import (
|
||||
get_review_style,
|
||||
mark_analysis_failed,
|
||||
mark_analysis_running,
|
||||
update_review_style,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
_ASSISTANT_ID = "review_style_analyzer"
|
||||
_LANGGRAPH_URL = "http://localhost:2024"
|
||||
|
||||
|
||||
async def start_review_style_analysis(
|
||||
full_name: str,
|
||||
*,
|
||||
github_token: str,
|
||||
created_by: str,
|
||||
) -> dict[str, Any]:
|
||||
"""Collect samples, persist metadata, and start the analyzer graph."""
|
||||
owner, repo = full_name.split("/", 1)
|
||||
try:
|
||||
samples = await collect_review_samples(github_token, owner, repo)
|
||||
except Exception:
|
||||
logger.exception("Failed to collect review samples for %s", full_name)
|
||||
await mark_analysis_failed(full_name, "sample collection failed")
|
||||
record = await get_review_style(full_name)
|
||||
return record or {
|
||||
"full_name": full_name,
|
||||
"status": "failed",
|
||||
"error": "Sample collection failed. Please retry later.",
|
||||
}
|
||||
|
||||
samples_text = format_samples_for_analyzer(samples)
|
||||
thread_id = generate_review_style_thread_id(owner, repo)
|
||||
|
||||
client = get_client(url=_LANGGRAPH_URL)
|
||||
configurable: dict[str, Any] = {
|
||||
"thread_id": thread_id,
|
||||
"review_style_full_name": full_name,
|
||||
"review_style_github_token": github_token,
|
||||
"review_style_samples_text": samples_text,
|
||||
"review_style_top_reviewers": samples.top_reviewers,
|
||||
"review_style_prs_sampled": samples.prs_scanned,
|
||||
"review_style_reviews_sampled": samples.reviews_scanned,
|
||||
}
|
||||
if not samples.samples:
|
||||
logger.info(
|
||||
"No pre-collected samples for %s (%s merged PRs scanned); analyzer will fetch via API",
|
||||
full_name,
|
||||
samples.prs_scanned,
|
||||
)
|
||||
|
||||
await mark_analysis_running(
|
||||
full_name,
|
||||
thread_id=thread_id,
|
||||
run_id=None,
|
||||
top_reviewers=samples.top_reviewers,
|
||||
prs_sampled=samples.prs_scanned,
|
||||
reviews_sampled=samples.reviews_scanned,
|
||||
)
|
||||
|
||||
try:
|
||||
run = await client.runs.create(
|
||||
thread_id,
|
||||
_ASSISTANT_ID,
|
||||
input={
|
||||
"messages": [
|
||||
{
|
||||
"role": "user",
|
||||
"content": (
|
||||
f"Analyze review style for `{full_name}`. Browse merged PR "
|
||||
"review feedback with `GH_TOKEN=dummy gh` until you have enough "
|
||||
"human examples, then save the repository-specific prompt."
|
||||
),
|
||||
}
|
||||
]
|
||||
},
|
||||
config={"configurable": configurable},
|
||||
if_not_exists="create",
|
||||
)
|
||||
run_id = run.get("run_id") if isinstance(run, dict) else getattr(run, "run_id", None)
|
||||
record = await update_review_style(
|
||||
full_name,
|
||||
{"analysis_run_id": run_id, "created_by": created_by},
|
||||
)
|
||||
return record
|
||||
except Exception:
|
||||
logger.exception("Failed to start review style analyzer for %s", full_name)
|
||||
await mark_analysis_failed(full_name, "run start failed")
|
||||
record = await get_review_style(full_name)
|
||||
return record or {
|
||||
"full_name": full_name,
|
||||
"status": "failed",
|
||||
"error": "Failed to start analysis. Please retry later.",
|
||||
}
|
||||
|
||||
|
||||
async def sync_review_style_run_status(full_name: str) -> dict[str, Any]:
|
||||
"""Refresh store status from the latest analyzer run when still running."""
|
||||
record = await get_review_style(full_name)
|
||||
if not record or record.get("status") != "running":
|
||||
return record or {}
|
||||
|
||||
thread_id = record.get("analysis_thread_id")
|
||||
run_id = record.get("analysis_run_id")
|
||||
if not isinstance(thread_id, str) or not thread_id:
|
||||
return record
|
||||
|
||||
client = get_client(url=_LANGGRAPH_URL)
|
||||
try:
|
||||
if isinstance(run_id, str) and run_id:
|
||||
run = await client.runs.get(thread_id, run_id)
|
||||
else:
|
||||
runs = await client.runs.list(thread_id, limit=1)
|
||||
items = runs if isinstance(runs, list) else (runs.get("runs") or [])
|
||||
run = items[0] if items else None
|
||||
if not run:
|
||||
return record
|
||||
status = run.get("status") if isinstance(run, dict) else getattr(run, "status", None)
|
||||
if status in ("success", "completed"):
|
||||
return await get_review_style(full_name) or record
|
||||
if status in ("error", "failed", "timeout", "interrupted"):
|
||||
logger.warning("Review style analyzer run failed for %s (status=%s)", full_name, status)
|
||||
return await mark_analysis_failed(
|
||||
full_name,
|
||||
"Analysis run failed. Please retry later.",
|
||||
)
|
||||
except Exception:
|
||||
logger.debug("Could not sync run status for %s", full_name, exc_info=True)
|
||||
return record
|
||||
196
agent/dashboard/review_styles.py
Normal file
196
agent/dashboard/review_styles.py
Normal file
|
|
@ -0,0 +1,196 @@
|
|||
"""Per-repository review style profiles in LangGraph Store.
|
||||
|
||||
Each record holds a synthesized custom prompt (editable in the dashboard),
|
||||
analysis metadata, and the status of the background style-analysis run.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from datetime import UTC, datetime
|
||||
from typing import Any, Literal
|
||||
|
||||
from langgraph_sdk import get_client
|
||||
from pydantic import BaseModel, Field, field_validator
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
REVIEW_STYLES_NAMESPACE: list[str] = ["review_styles"]
|
||||
|
||||
AnalysisStatus = Literal["idle", "running", "completed", "failed"]
|
||||
|
||||
|
||||
def normalize_repo_full_name(raw: str) -> str:
|
||||
"""Normalize user input to ``owner/repo``."""
|
||||
v = raw.strip()
|
||||
for prefix in ("https://github.com/", "http://github.com/", "github.com/"):
|
||||
if v.lower().startswith(prefix):
|
||||
v = v[len(prefix) :]
|
||||
v = v.strip("/")
|
||||
if v.endswith(".git"):
|
||||
v = v[:-4]
|
||||
parts = [p for p in v.split("/") if p]
|
||||
if len(parts) != 2:
|
||||
raise ValueError("full_name must be owner/repo")
|
||||
return f"{parts[0]}/{parts[1]}"
|
||||
|
||||
|
||||
class ReviewStyleCreate(BaseModel):
|
||||
full_name: str = Field(..., description="GitHub repo in owner/name form")
|
||||
|
||||
@field_validator("full_name", mode="before")
|
||||
@classmethod
|
||||
def _valid_full_name(cls, v: str) -> str:
|
||||
return normalize_repo_full_name(v)
|
||||
|
||||
|
||||
class ReviewStylePromptUpdate(BaseModel):
|
||||
custom_prompt: str
|
||||
|
||||
@field_validator("custom_prompt")
|
||||
@classmethod
|
||||
def _non_empty(cls, v: str) -> str:
|
||||
if not v.strip():
|
||||
raise ValueError("custom_prompt cannot be empty")
|
||||
return v
|
||||
|
||||
|
||||
def _client():
|
||||
return get_client()
|
||||
|
||||
|
||||
async def _get_value(key: str) -> dict[str, Any] | None:
|
||||
try:
|
||||
item = await _client().store.get_item(REVIEW_STYLES_NAMESPACE, key)
|
||||
except Exception as e:
|
||||
logger.debug("store get_item failed for %s: %s", key, e)
|
||||
return None
|
||||
if item is None:
|
||||
return None
|
||||
value = item.get("value") if isinstance(item, dict) else getattr(item, "value", None)
|
||||
return value if isinstance(value, dict) else None
|
||||
|
||||
|
||||
def _now_iso() -> str:
|
||||
return datetime.now(UTC).isoformat()
|
||||
|
||||
|
||||
def _default_record(full_name: str, created_by: str) -> dict[str, Any]:
|
||||
owner, name = full_name.split("/", 1)
|
||||
return {
|
||||
"full_name": full_name,
|
||||
"owner": owner,
|
||||
"name": name,
|
||||
"status": "idle",
|
||||
"custom_prompt": None,
|
||||
"analysis_summary": None,
|
||||
"top_reviewers": [],
|
||||
"prs_sampled": 0,
|
||||
"reviews_sampled": 0,
|
||||
"analysis_thread_id": None,
|
||||
"analysis_run_id": None,
|
||||
"error": None,
|
||||
"created_by": created_by,
|
||||
"created_at": _now_iso(),
|
||||
"updated_at": _now_iso(),
|
||||
}
|
||||
|
||||
|
||||
async def get_review_style(full_name: str) -> dict[str, Any] | None:
|
||||
return await _get_value(full_name)
|
||||
|
||||
|
||||
async def list_review_styles() -> list[dict[str, Any]]:
|
||||
result = await _client().store.search_items(REVIEW_STYLES_NAMESPACE, limit=1000)
|
||||
items = result.get("items") if isinstance(result, dict) else getattr(result, "items", [])
|
||||
out: list[dict[str, Any]] = []
|
||||
for item in items or []:
|
||||
value = item.get("value") if isinstance(item, dict) else getattr(item, "value", None)
|
||||
if isinstance(value, dict):
|
||||
out.append(value)
|
||||
out.sort(key=lambda r: r.get("full_name", ""))
|
||||
return out
|
||||
|
||||
|
||||
async def create_review_style(full_name: str, created_by: str) -> dict[str, Any]:
|
||||
existing = await get_review_style(full_name)
|
||||
if existing:
|
||||
return existing
|
||||
value = _default_record(full_name, created_by)
|
||||
await _client().store.put_item(REVIEW_STYLES_NAMESPACE, full_name, value)
|
||||
return value
|
||||
|
||||
|
||||
async def update_review_style(full_name: str, patch: dict[str, Any]) -> dict[str, Any]:
|
||||
existing = await get_review_style(full_name) or _default_record(
|
||||
full_name, patch.get("created_by", "")
|
||||
)
|
||||
value = {**existing, **patch, "updated_at": _now_iso()}
|
||||
await _client().store.put_item(REVIEW_STYLES_NAMESPACE, full_name, value)
|
||||
return value
|
||||
|
||||
|
||||
async def set_custom_prompt(full_name: str, custom_prompt: str) -> dict[str, Any]:
|
||||
return await update_review_style(full_name, {"custom_prompt": custom_prompt})
|
||||
|
||||
|
||||
async def mark_analysis_running(
|
||||
full_name: str,
|
||||
*,
|
||||
thread_id: str,
|
||||
run_id: str | None,
|
||||
top_reviewers: list[str],
|
||||
prs_sampled: int,
|
||||
reviews_sampled: int,
|
||||
) -> dict[str, Any]:
|
||||
return await update_review_style(
|
||||
full_name,
|
||||
{
|
||||
"status": "running",
|
||||
"analysis_thread_id": thread_id,
|
||||
"analysis_run_id": run_id,
|
||||
"top_reviewers": top_reviewers,
|
||||
"prs_sampled": prs_sampled,
|
||||
"reviews_sampled": reviews_sampled,
|
||||
"error": None,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
async def mark_analysis_completed(
|
||||
full_name: str,
|
||||
*,
|
||||
custom_prompt: str,
|
||||
analysis_summary: str,
|
||||
top_reviewers: list[str],
|
||||
prs_sampled: int,
|
||||
reviews_sampled: int,
|
||||
) -> dict[str, Any]:
|
||||
return await update_review_style(
|
||||
full_name,
|
||||
{
|
||||
"status": "completed",
|
||||
"custom_prompt": custom_prompt,
|
||||
"analysis_summary": analysis_summary,
|
||||
"top_reviewers": top_reviewers,
|
||||
"prs_sampled": prs_sampled,
|
||||
"reviews_sampled": reviews_sampled,
|
||||
"error": None,
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
async def mark_analysis_failed(full_name: str, error: str) -> dict[str, Any]:
|
||||
return await update_review_style(full_name, {"status": "failed", "error": error})
|
||||
|
||||
|
||||
async def get_repo_custom_prompt(owner: str, repo: str) -> str | None:
|
||||
"""Return the custom prompt supplement for a repo, if configured."""
|
||||
full_name = f"{owner}/{repo}"
|
||||
record = await get_review_style(full_name)
|
||||
if not record:
|
||||
return None
|
||||
prompt = record.get("custom_prompt")
|
||||
if isinstance(prompt, str) and prompt.strip():
|
||||
return prompt.strip()
|
||||
return None
|
||||
|
|
@ -36,6 +36,16 @@ from .profiles import (
|
|||
upsert_access_token,
|
||||
upsert_profile,
|
||||
)
|
||||
from .review_style_jobs import start_review_style_analysis, sync_review_style_run_status
|
||||
from .review_styles import (
|
||||
ReviewStyleCreate,
|
||||
ReviewStylePromptUpdate,
|
||||
create_review_style,
|
||||
get_review_style,
|
||||
list_review_styles,
|
||||
normalize_repo_full_name,
|
||||
set_custom_prompt,
|
||||
)
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
|
|
@ -325,3 +335,109 @@ async def list_repos(
|
|||
if r.get("full_name")
|
||||
],
|
||||
}
|
||||
|
||||
|
||||
async def _assert_repo_available_for_style_analysis(full_name: str, token: str) -> None:
|
||||
"""Ensure the repo exists and is readable for style learning.
|
||||
|
||||
Public repositories are allowed without the GitHub App installed on them.
|
||||
Private repositories require the authenticated user to have read access.
|
||||
"""
|
||||
full_name = normalize_repo_full_name(full_name)
|
||||
headers = {
|
||||
"Authorization": f"Bearer {token}",
|
||||
"Accept": "application/vnd.github+json",
|
||||
"X-GitHub-Api-Version": "2022-11-28",
|
||||
}
|
||||
owner, name = full_name.split("/", 1)
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.get(
|
||||
f"https://api.github.com/repos/{owner}/{name}",
|
||||
headers=headers,
|
||||
)
|
||||
if r.status_code == 404:
|
||||
raise HTTPException(404, "repository not found")
|
||||
if r.status_code == 403:
|
||||
raise HTTPException(403, "no access to this private repository")
|
||||
if r.status_code != 200:
|
||||
raise HTTPException(502, f"github API error ({r.status_code})")
|
||||
body = r.json()
|
||||
if body.get("private") is not True:
|
||||
return
|
||||
# Private repo: 200 from GitHub implies the user's token can read it.
|
||||
|
||||
|
||||
@router.get("/review-styles")
|
||||
async def api_list_review_styles(
|
||||
session: dict[str, Any] = _SESSION_DEP,
|
||||
) -> list[dict[str, Any]]:
|
||||
records = await list_review_styles()
|
||||
out: list[dict[str, Any]] = []
|
||||
for record in records:
|
||||
if record.get("status") == "running":
|
||||
synced = await sync_review_style_run_status(record["full_name"])
|
||||
out.append(synced)
|
||||
else:
|
||||
out.append(record)
|
||||
return out
|
||||
|
||||
|
||||
@router.post("/review-styles")
|
||||
async def api_create_review_style(
|
||||
body: ReviewStyleCreate,
|
||||
session: dict[str, Any] = _SESSION_DEP,
|
||||
) -> dict[str, Any]:
|
||||
token = await get_access_token(session["sub"])
|
||||
if not token:
|
||||
raise HTTPException(401, "github token unavailable, re-login required")
|
||||
await _assert_repo_available_for_style_analysis(body.full_name, token)
|
||||
return await create_review_style(body.full_name, session["sub"])
|
||||
|
||||
|
||||
@router.get("/review-styles/{full_name:path}")
|
||||
async def api_get_review_style(
|
||||
full_name: str,
|
||||
session: dict[str, Any] = _SESSION_DEP,
|
||||
) -> dict[str, Any]:
|
||||
full_name = normalize_repo_full_name(full_name)
|
||||
record = await get_review_style(full_name)
|
||||
if not record:
|
||||
raise HTTPException(404, "review style not found")
|
||||
if record.get("status") == "running":
|
||||
record = await sync_review_style_run_status(full_name)
|
||||
return record
|
||||
|
||||
|
||||
@router.put("/review-styles/{full_name:path}")
|
||||
async def api_update_review_style_prompt(
|
||||
full_name: str,
|
||||
body: ReviewStylePromptUpdate,
|
||||
session: dict[str, Any] = _SESSION_DEP,
|
||||
) -> dict[str, Any]:
|
||||
full_name = normalize_repo_full_name(full_name)
|
||||
record = await get_review_style(full_name)
|
||||
if not record:
|
||||
raise HTTPException(404, "review style not found")
|
||||
return await set_custom_prompt(full_name, body.custom_prompt)
|
||||
|
||||
|
||||
@router.post("/review-styles/{full_name:path}/analyze")
|
||||
async def api_analyze_review_style(
|
||||
full_name: str,
|
||||
session: dict[str, Any] = _SESSION_DEP,
|
||||
) -> dict[str, Any]:
|
||||
full_name = normalize_repo_full_name(full_name)
|
||||
token = await get_access_token(session["sub"])
|
||||
if not token:
|
||||
raise HTTPException(401, "github token unavailable, re-login required")
|
||||
await _assert_repo_available_for_style_analysis(full_name, token)
|
||||
record = await get_review_style(full_name)
|
||||
if not record:
|
||||
record = await create_review_style(full_name, session["sub"])
|
||||
if record.get("status") == "running":
|
||||
raise HTTPException(409, "analysis already running")
|
||||
return await start_review_style_analysis(
|
||||
full_name,
|
||||
github_token=token,
|
||||
created_by=session["sub"],
|
||||
)
|
||||
|
|
|
|||
160
agent/review_style_analyzer.py
Normal file
160
agent/review_style_analyzer.py
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
"""Review style analyzer graph.
|
||||
|
||||
Uses the same sandbox + ``gh`` pattern as the reviewer agent. The dashboard
|
||||
user's OAuth token is injected into the LangSmith GitHub proxy so ``gh`` works
|
||||
on public repos even when the GitHub App is not installed on them.
|
||||
"""
|
||||
# ruff: noqa: E402
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
import os
|
||||
import warnings
|
||||
|
||||
from langgraph.graph.state import RunnableConfig
|
||||
from langgraph.pregel import Pregel
|
||||
|
||||
warnings.filterwarnings("ignore", module="langchain_core._api.deprecation")
|
||||
warnings.filterwarnings("ignore", message=".*Pydantic V1.*", category=UserWarning)
|
||||
|
||||
from deepagents import create_deep_agent
|
||||
from deepagents.backends.protocol import SandboxBackendProtocol
|
||||
from langchain.agents.middleware import ModelCallLimitMiddleware
|
||||
|
||||
from .integrations.langsmith import _configure_github_proxy
|
||||
from .middleware import SanitizeToolInputsMiddleware, ToolErrorMiddleware
|
||||
from .review_style_guidance import REVIEWER_STYLE_THEMES
|
||||
from .server import (
|
||||
DEFAULT_LLM_MAX_TOKENS,
|
||||
DEFAULT_LLM_MODEL_ID,
|
||||
DEFAULT_RECURSION_LIMIT,
|
||||
ensure_sandbox_for_thread,
|
||||
graph_loaded_for_execution,
|
||||
)
|
||||
from .tools.save_review_style import save_review_style_prompt
|
||||
from .utils.model import DEFAULT_LLM_REASONING, make_model, provider_model_kwargs
|
||||
from .utils.sandbox_paths import aresolve_sandbox_work_dir
|
||||
from .utils.sandbox_state import unwrap_sandbox_backend
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
STYLE_ANALYZER_MODEL_CALL_LIMIT = 80
|
||||
|
||||
STYLE_ANALYZER_PROMPT = """You are a code-review style analyst for `{repo_owner}/{repo_name}`.
|
||||
|
||||
Sandbox: `{working_dir}`. Use the shell (``execute``) to run GitHub commands.
|
||||
|
||||
**Always invoke gh as:** `GH_TOKEN=dummy gh <command>`
|
||||
|
||||
# How to research (required)
|
||||
|
||||
Browse historical **merged** PR review feedback until you have catalogued at least
|
||||
**8 substantive human** review comments (not bots). Suggested commands:
|
||||
|
||||
```
|
||||
GH_TOKEN=dummy gh pr list --repo {repo_owner}/{repo_name} --state merged --limit 30
|
||||
GH_TOKEN=dummy gh api repos/{repo_owner}/{repo_name}/pulls/<PR_NUMBER>/reviews
|
||||
GH_TOKEN=dummy gh api repos/{repo_owner}/{repo_name}/pulls/<PR_NUMBER>/comments
|
||||
GH_TOKEN=dummy gh api repos/{repo_owner}/{repo_name}/issues/<PR_NUMBER>/comments
|
||||
```
|
||||
|
||||
If the first batch is sparse, increase `--limit` or walk older PR numbers. Skip
|
||||
`[bot]` accounts and obvious automation (codecov, dependabot, etc.).
|
||||
|
||||
Identify the top ~5 human reviewers by volume and note phrasing, severity, and
|
||||
what they ignore.
|
||||
|
||||
# When you may call `save_review_style_prompt`
|
||||
|
||||
Only after real research. Your `custom_prompt` (400–1200 words) must teach our
|
||||
reviewer agent this repo's norms:
|
||||
|
||||
- What the team routinely flags vs skips (paraphrased patterns, not invented quotes)
|
||||
- Severity calibration
|
||||
- Tone and test expectations
|
||||
- Repo-specific conventions
|
||||
- Anti-patterns reviewers here avoid
|
||||
|
||||
`analysis_summary`: 2–4 sentences for the dashboard.
|
||||
Pass `prs_sampled`, `reviews_sampled`, and `top_reviewers` (comma-separated logins).
|
||||
|
||||
Do **not** save a generic guide after one or two commands. Only after ~25+ merged
|
||||
PRs with zero human feedback may you save a short conservative guide and say so in
|
||||
`analysis_summary`.
|
||||
|
||||
# Alignment with our reviewer agent
|
||||
|
||||
{reviewer_themes}
|
||||
|
||||
# Optional preloaded samples
|
||||
|
||||
The user message may include pre-collected samples — verify and extend with ``gh``.
|
||||
"""
|
||||
|
||||
|
||||
async def _configure_sandbox_github_proxy(
|
||||
sandbox_backend: SandboxBackendProtocol,
|
||||
github_token: str,
|
||||
) -> None:
|
||||
if os.getenv("SANDBOX_TYPE", "langsmith") != "langsmith":
|
||||
return
|
||||
backend = unwrap_sandbox_backend(sandbox_backend)
|
||||
await asyncio.to_thread(_configure_github_proxy, backend.id, github_token)
|
||||
|
||||
|
||||
async def get_review_style_analyzer(config: RunnableConfig) -> Pregel:
|
||||
thread_id = config["configurable"].get("thread_id")
|
||||
config["recursion_limit"] = DEFAULT_RECURSION_LIMIT
|
||||
|
||||
if thread_id is None or not graph_loaded_for_execution(config):
|
||||
return create_deep_agent(system_prompt="", tools=[]).with_config(config)
|
||||
|
||||
sandbox_backend = await ensure_sandbox_for_thread(thread_id)
|
||||
work_dir = await aresolve_sandbox_work_dir(sandbox_backend)
|
||||
|
||||
configurable = config["configurable"]
|
||||
full_name = str(configurable.get("review_style_full_name") or "owner/repo")
|
||||
owner, _, name = full_name.partition("/")
|
||||
samples_text = str(configurable.get("review_style_samples_text") or "")
|
||||
github_token = configurable.get("review_style_github_token")
|
||||
if isinstance(github_token, str) and github_token:
|
||||
await _configure_sandbox_github_proxy(sandbox_backend, github_token)
|
||||
|
||||
model_id = DEFAULT_LLM_MODEL_ID
|
||||
model_kwargs = provider_model_kwargs(
|
||||
model_id,
|
||||
None,
|
||||
max_tokens=DEFAULT_LLM_MAX_TOKENS,
|
||||
openai_reasoning_default=DEFAULT_LLM_REASONING,
|
||||
)
|
||||
|
||||
system_prompt = STYLE_ANALYZER_PROMPT.format(
|
||||
repo_owner=owner or "<owner>",
|
||||
repo_name=name or "<repo>",
|
||||
working_dir=work_dir,
|
||||
reviewer_themes=REVIEWER_STYLE_THEMES.strip(),
|
||||
)
|
||||
user_context = (
|
||||
f"Repository: `{full_name}`\n\n"
|
||||
f"{samples_text}\n\n"
|
||||
"Research review style with `GH_TOKEN=dummy gh ...` via execute, then call "
|
||||
"`save_review_style_prompt` once you have enough evidence."
|
||||
)
|
||||
system_prompt = f"{system_prompt}\n\n{user_context}"
|
||||
|
||||
return create_deep_agent(
|
||||
model=make_model(model_id, **model_kwargs),
|
||||
system_prompt=system_prompt,
|
||||
tools=[save_review_style_prompt],
|
||||
backend=sandbox_backend,
|
||||
middleware=[
|
||||
SanitizeToolInputsMiddleware(),
|
||||
ModelCallLimitMiddleware(
|
||||
run_limit=STYLE_ANALYZER_MODEL_CALL_LIMIT,
|
||||
exit_behavior="end",
|
||||
),
|
||||
ToolErrorMiddleware(),
|
||||
],
|
||||
).with_config(config)
|
||||
325
agent/review_style_collector.py
Normal file
325
agent/review_style_collector.py
Normal file
|
|
@ -0,0 +1,325 @@
|
|||
"""Collect historical PR review samples from GitHub for style analysis."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import uuid
|
||||
from collections import Counter
|
||||
from dataclasses import dataclass, field
|
||||
from typing import Any
|
||||
|
||||
import httpx
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
|
||||
DEFAULT_MAX_PRS = 20
|
||||
DEFAULT_MAX_REVIEWERS = 10
|
||||
DEFAULT_MAX_SAMPLES_PER_REVIEWER = 6
|
||||
MIN_COMMENT_CHARS = 20
|
||||
GITHUB_API = "https://api.github.com"
|
||||
_BOT_SUFFIX = "[bot]"
|
||||
|
||||
|
||||
def generate_review_style_thread_id(owner: str, repo: str) -> str:
|
||||
stable_key = f"{owner}/{repo}/review-style"
|
||||
return str(uuid.uuid5(uuid.NAMESPACE_URL, stable_key))
|
||||
|
||||
|
||||
@dataclass
|
||||
class ReviewSample:
|
||||
pr_number: int
|
||||
reviewer_login: str
|
||||
kind: str
|
||||
body: str
|
||||
state: str = ""
|
||||
path: str | None = None
|
||||
submitted_at: str | None = None
|
||||
|
||||
|
||||
@dataclass
|
||||
class ReviewStyleSamples:
|
||||
full_name: str
|
||||
owner: str
|
||||
name: str
|
||||
top_reviewers: list[str] = field(default_factory=list)
|
||||
samples: list[ReviewSample] = field(default_factory=list)
|
||||
prs_scanned: int = 0
|
||||
reviews_scanned: int = 0
|
||||
|
||||
|
||||
def _github_headers(token: str) -> dict[str, str]:
|
||||
return {
|
||||
"Authorization": f"Bearer {token}",
|
||||
"Accept": "application/vnd.github+json",
|
||||
"X-GitHub-Api-Version": "2022-11-28",
|
||||
}
|
||||
|
||||
|
||||
async def _paginate(
|
||||
client: httpx.AsyncClient,
|
||||
url: str,
|
||||
*,
|
||||
headers: dict[str, str],
|
||||
cap: int = 500,
|
||||
) -> list[Any]:
|
||||
out: list[Any] = []
|
||||
next_url: str | None = url
|
||||
first = True
|
||||
while next_url and len(out) < cap:
|
||||
params = {"per_page": "100"} if first else None
|
||||
r = await client.get(next_url, headers=headers, params=params)
|
||||
r.raise_for_status()
|
||||
page = r.json()
|
||||
if isinstance(page, list):
|
||||
out.extend(page)
|
||||
next_url = None
|
||||
link = r.headers.get("Link", "")
|
||||
for part in link.split(","):
|
||||
segments = [s.strip() for s in part.split(";")]
|
||||
if len(segments) >= 2 and 'rel="next"' in segments[1] and segments[0].startswith("<"):
|
||||
next_url = segments[0][1:-1]
|
||||
break
|
||||
first = False
|
||||
return out
|
||||
|
||||
|
||||
def _is_bot_login(login: str | None) -> bool:
|
||||
if not login:
|
||||
return True
|
||||
return login.endswith(_BOT_SUFFIX) or login.endswith("-bot")
|
||||
|
||||
|
||||
def _is_bot_user(user: dict[str, Any] | None) -> bool:
|
||||
if not isinstance(user, dict):
|
||||
return True
|
||||
if user.get("type") == "Bot":
|
||||
return True
|
||||
login = user.get("login")
|
||||
return _is_bot_login(login if isinstance(login, str) else None)
|
||||
|
||||
|
||||
def _substantive_body(body: str | None) -> str | None:
|
||||
text = (body or "").strip()
|
||||
if len(text) < MIN_COMMENT_CHARS:
|
||||
return None
|
||||
return text[:4000]
|
||||
|
||||
|
||||
async def _recent_merged_prs(
|
||||
client: httpx.AsyncClient,
|
||||
*,
|
||||
owner: str,
|
||||
repo: str,
|
||||
headers: dict[str, str],
|
||||
max_prs: int,
|
||||
) -> list[dict[str, Any]]:
|
||||
"""Return recently merged PRs via the issues search API (reliable on busy repos)."""
|
||||
r = await client.get(
|
||||
f"{GITHUB_API}/search/issues",
|
||||
headers=headers,
|
||||
params={
|
||||
"q": f"repo:{owner}/{repo} is:pr is:merged",
|
||||
"sort": "updated",
|
||||
"order": "desc",
|
||||
"per_page": min(max_prs, 100),
|
||||
},
|
||||
)
|
||||
r.raise_for_status()
|
||||
body = r.json()
|
||||
items = body.get("items", []) if isinstance(body, dict) else []
|
||||
merged: list[dict[str, Any]] = []
|
||||
for item in items:
|
||||
if not isinstance(item, dict):
|
||||
continue
|
||||
number = item.get("number")
|
||||
if not isinstance(number, int):
|
||||
continue
|
||||
merged.append({"number": number, "title": item.get("title", "")})
|
||||
if not merged:
|
||||
logger.warning(
|
||||
"search returned 0 merged PRs for %s/%s (status=%s total_count=%s)",
|
||||
owner,
|
||||
repo,
|
||||
r.status_code,
|
||||
body.get("total_count") if isinstance(body, dict) else "?",
|
||||
)
|
||||
return merged
|
||||
|
||||
|
||||
async def collect_review_samples(
|
||||
token: str,
|
||||
owner: str,
|
||||
repo: str,
|
||||
*,
|
||||
max_prs: int = DEFAULT_MAX_PRS,
|
||||
max_reviewers: int = DEFAULT_MAX_REVIEWERS,
|
||||
max_samples_per_reviewer: int = DEFAULT_MAX_SAMPLES_PER_REVIEWER,
|
||||
) -> ReviewStyleSamples:
|
||||
"""Sample recent merged PR feedback to identify reviewer style."""
|
||||
full_name = f"{owner}/{repo}"
|
||||
headers = _github_headers(token)
|
||||
|
||||
raw_entries: list[tuple[str, int, ReviewSample]] = []
|
||||
reviewer_counts: Counter[str] = Counter()
|
||||
|
||||
async with httpx.AsyncClient(timeout=90.0) as client:
|
||||
merged_prs = await _recent_merged_prs(
|
||||
client, owner=owner, repo=repo, headers=headers, max_prs=max_prs
|
||||
)
|
||||
|
||||
for pr in merged_prs:
|
||||
pr_number = pr.get("number")
|
||||
if not isinstance(pr_number, int):
|
||||
continue
|
||||
|
||||
reviews_url = f"{GITHUB_API}/repos/{owner}/{repo}/pulls/{pr_number}/reviews"
|
||||
for review in await _paginate(client, reviews_url, headers=headers, cap=100):
|
||||
if not isinstance(review, dict):
|
||||
continue
|
||||
user = review.get("user")
|
||||
if _is_bot_user(user if isinstance(user, dict) else None):
|
||||
continue
|
||||
login = (user or {}).get("login") if isinstance(user, dict) else None
|
||||
if not isinstance(login, str):
|
||||
continue
|
||||
body = _substantive_body(review.get("body"))
|
||||
if not body:
|
||||
continue
|
||||
reviewer_counts[login] += 1
|
||||
raw_entries.append(
|
||||
(
|
||||
login,
|
||||
pr_number,
|
||||
ReviewSample(
|
||||
pr_number=pr_number,
|
||||
reviewer_login=login,
|
||||
kind="review",
|
||||
state=str(review.get("state") or ""),
|
||||
body=body,
|
||||
submitted_at=review.get("submitted_at"),
|
||||
),
|
||||
)
|
||||
)
|
||||
|
||||
comments_url = f"{GITHUB_API}/repos/{owner}/{repo}/pulls/{pr_number}/comments"
|
||||
for comment in await _paginate(client, comments_url, headers=headers, cap=200):
|
||||
if not isinstance(comment, dict):
|
||||
continue
|
||||
user = comment.get("user")
|
||||
if _is_bot_user(user if isinstance(user, dict) else None):
|
||||
continue
|
||||
login = (user or {}).get("login") if isinstance(user, dict) else None
|
||||
if not isinstance(login, str):
|
||||
continue
|
||||
body = _substantive_body(comment.get("body"))
|
||||
if not body:
|
||||
continue
|
||||
path = comment.get("path")
|
||||
reviewer_counts[login] += 1
|
||||
raw_entries.append(
|
||||
(
|
||||
login,
|
||||
pr_number,
|
||||
ReviewSample(
|
||||
pr_number=pr_number,
|
||||
reviewer_login=login,
|
||||
kind="inline",
|
||||
body=body,
|
||||
path=str(path) if isinstance(path, str) else None,
|
||||
submitted_at=comment.get("created_at"),
|
||||
),
|
||||
)
|
||||
)
|
||||
|
||||
issue_comments_url = f"{GITHUB_API}/repos/{owner}/{repo}/issues/{pr_number}/comments"
|
||||
for comment in await _paginate(client, issue_comments_url, headers=headers, cap=100):
|
||||
if not isinstance(comment, dict):
|
||||
continue
|
||||
user = comment.get("user")
|
||||
if _is_bot_user(user if isinstance(user, dict) else None):
|
||||
continue
|
||||
login = (user or {}).get("login") if isinstance(user, dict) else None
|
||||
if not isinstance(login, str):
|
||||
continue
|
||||
body = _substantive_body(comment.get("body"))
|
||||
if not body:
|
||||
continue
|
||||
reviewer_counts[login] += 1
|
||||
raw_entries.append(
|
||||
(
|
||||
login,
|
||||
pr_number,
|
||||
ReviewSample(
|
||||
pr_number=pr_number,
|
||||
reviewer_login=login,
|
||||
kind="issue",
|
||||
body=body,
|
||||
submitted_at=comment.get("created_at"),
|
||||
),
|
||||
)
|
||||
)
|
||||
|
||||
top_reviewers = [login for login, _ in reviewer_counts.most_common(max_reviewers)]
|
||||
top_set = set(top_reviewers)
|
||||
|
||||
per_reviewer: Counter[str] = Counter()
|
||||
samples: list[ReviewSample] = []
|
||||
|
||||
for login, _pr_number, sample in raw_entries:
|
||||
if login not in top_set:
|
||||
continue
|
||||
if per_reviewer[login] >= max_samples_per_reviewer:
|
||||
continue
|
||||
samples.append(sample)
|
||||
per_reviewer[login] += 1
|
||||
|
||||
return ReviewStyleSamples(
|
||||
full_name=full_name,
|
||||
owner=owner,
|
||||
name=repo,
|
||||
top_reviewers=top_reviewers,
|
||||
samples=samples,
|
||||
prs_scanned=len(merged_prs),
|
||||
reviews_scanned=len(raw_entries),
|
||||
)
|
||||
|
||||
|
||||
def format_samples_for_analyzer(samples: ReviewStyleSamples) -> str:
|
||||
"""Render collected samples as context for the style-analyzer agent."""
|
||||
lines = [
|
||||
f"# Recent review samples for {samples.full_name}",
|
||||
"",
|
||||
f"Recently merged PRs scanned: {samples.prs_scanned}",
|
||||
f"Review summaries + inline comments collected: {samples.reviews_scanned}",
|
||||
f"Top reviewers ({len(samples.top_reviewers)}): {', '.join(samples.top_reviewers) or '(none)'}",
|
||||
"",
|
||||
]
|
||||
if not samples.samples:
|
||||
lines.append(
|
||||
"Pre-collection found no substantive review text on recent merged PRs. "
|
||||
"You must browse merged PRs yourself with `GH_TOKEN=dummy gh` (reviews, "
|
||||
"pull comments, and issue comments) before saving."
|
||||
)
|
||||
return "\n".join(lines)
|
||||
|
||||
by_reviewer: dict[str, list[ReviewSample]] = {}
|
||||
for s in samples.samples:
|
||||
by_reviewer.setdefault(s.reviewer_login, []).append(s)
|
||||
|
||||
for login in samples.top_reviewers:
|
||||
reviewer_samples = by_reviewer.get(login, [])
|
||||
if not reviewer_samples:
|
||||
continue
|
||||
lines.append(f"## Reviewer: @{login}")
|
||||
for s in reviewer_samples:
|
||||
if s.kind == "inline":
|
||||
loc = f" ({s.path})" if s.path else ""
|
||||
lines.append(f"### PR #{s.pr_number} inline comment{loc}")
|
||||
elif s.kind == "issue":
|
||||
lines.append(f"### PR #{s.pr_number} issue comment")
|
||||
else:
|
||||
state = f", state={s.state}" if s.state else ""
|
||||
lines.append(f"### PR #{s.pr_number} review summary{state}")
|
||||
lines.append(s.body)
|
||||
lines.append("")
|
||||
return "\n".join(lines)
|
||||
17
agent/review_style_guidance.py
Normal file
17
agent/review_style_guidance.py
Normal file
|
|
@ -0,0 +1,17 @@
|
|||
"""Reviewer themes to steer repository style analysis."""
|
||||
|
||||
REVIEWER_STYLE_THEMES = """
|
||||
The downstream reviewer agent looks for high-signal, diff-anchored defects — not nits.
|
||||
When learning this repo's style, note how human reviewers align (or don't) with:
|
||||
|
||||
**Usually flag:** correctness regressions, wrong operators/variables, async footguns,
|
||||
read-modify-write races, API/signature drift, nil/None deref, security boundaries (SSRF,
|
||||
auth/cache asymmetry), broken tests, migration/ORM bypass, template/React contract breaks.
|
||||
|
||||
**Usually skip:** rename/style preferences, speculative "might break someday", scope-policing,
|
||||
pre-existing issues, duplicate findings for the same bug across files, generic perf opinions.
|
||||
|
||||
**Severity:** tie labels to user-visible/runtime consequence, not taste.
|
||||
|
||||
**Tone:** prefer direct, concrete failure modes; cite the line; suggest only tiny obvious fixes.
|
||||
"""
|
||||
|
|
@ -31,7 +31,6 @@ from deepagents import create_deep_agent
|
|||
from langchain.agents.middleware import ModelCallLimitMiddleware
|
||||
|
||||
from .middleware import (
|
||||
ExcludeToolsMiddleware,
|
||||
SanitizeToolInputsMiddleware,
|
||||
SlackAssistantStatusMiddleware,
|
||||
ToolErrorMiddleware,
|
||||
|
|
@ -42,134 +41,283 @@ from .reviewer_findings import (
|
|||
from .server import (
|
||||
DEFAULT_LLM_MAX_TOKENS,
|
||||
DEFAULT_LLM_MODEL_ID,
|
||||
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,
|
||||
)
|
||||
from .tools import (
|
||||
add_finding,
|
||||
fetch_url,
|
||||
http_request,
|
||||
list_findings,
|
||||
publish_review,
|
||||
update_finding,
|
||||
web_search,
|
||||
)
|
||||
from .utils.auth import resolve_github_token
|
||||
from .utils.github_token import get_github_token_from_thread
|
||||
from .utils.model import ModelKwargs, make_model
|
||||
from .utils.model import DEFAULT_LLM_REASONING, make_model, provider_model_kwargs
|
||||
from .utils.sandbox_paths import aresolve_sandbox_work_dir
|
||||
|
||||
REVIEWER_PROMPT_TEMPLATE = """You are an expert code reviewer.
|
||||
REVIEWER_PROMPT_TEMPLATE = """You are a specialized code reviewer agent. Your job is to review one GitHub PR and publish a single review.
|
||||
|
||||
Your job is to review one GitHub pull request, find real issues, record them
|
||||
as structured findings, and publish a single GitHub review with the most
|
||||
important findings as inline comments — with a concrete `suggestion` block
|
||||
only when the fix is small enough (≤4 lines) that the user can scan it and
|
||||
click "Commit suggestion".
|
||||
Sandbox: `{working_dir}`. Invoke `gh` as `GH_TOKEN=dummy gh <command>`.
|
||||
|
||||
### Working environment
|
||||
|
||||
You are operating in a remote Linux sandbox at `{working_dir}`.
|
||||
|
||||
- The `gh` CLI is installed and authenticated by a sandbox proxy. Always
|
||||
invoke it as `GH_TOKEN=dummy gh <command>`.
|
||||
- The `execute` tool runs shell commands. Default timeout ~30 minutes.
|
||||
- `read_file`, `grep`, `glob` are available for code exploration.
|
||||
|
||||
### Fetching the diff
|
||||
|
||||
**Your first step is to fetch the PR diff yourself.** Run:
|
||||
Fetch the diff:
|
||||
|
||||
```
|
||||
GH_TOKEN=dummy gh pr diff {pr_number} --repo {repo_owner}/{repo_name}
|
||||
```
|
||||
|
||||
For a re-review (the user message says "A new commit has been pushed"), fetch
|
||||
the diff between the previously reviewed SHA and the new HEAD instead:
|
||||
Re-review (user message says "A new commit has been pushed"):
|
||||
|
||||
```
|
||||
GH_TOKEN=dummy gh api repos/{repo_owner}/{repo_name}/compare/<last_reviewed_sha>...<head_sha> -H "Accept: application/vnd.github.v3.diff"
|
||||
```
|
||||
|
||||
If you want to read full file context to validate a finding, clone the repo:
|
||||
Clone repo first so that you can grep for full file context:
|
||||
|
||||
```
|
||||
GH_TOKEN=dummy gh repo clone {repo_owner}/{repo_name} && cd {repo_name} && git checkout <head_sha>
|
||||
```
|
||||
|
||||
Cloning is optional — for most PRs the diff alone is enough.
|
||||
Tools: `add_finding`, `update_finding`, `list_findings`, `publish_review`.
|
||||
Call `publish_review` once at the end.
|
||||
|
||||
### How to review
|
||||
Re-review: for each open finding, `update_finding(id, status="resolved")` if
|
||||
fixed, `update_finding` with new fields + `note` if changed, otherwise do
|
||||
nothing. Add net-new findings with `add_finding`.
|
||||
|
||||
1. Fetch the diff (above). **Review the diff that's there. Don't review
|
||||
pre-existing code.**
|
||||
2. For each real issue you find in the diff, call **`add_finding`** with:
|
||||
- `severity`: one of `informational`, `low`, `medium`, `high`, `critical`.
|
||||
Calibrate strictly: `critical` = bug that breaks production or a security
|
||||
hole; `high` = real correctness/regression risk; `medium` = clear quality
|
||||
issue worth surfacing; `low` = small nit; `informational` = FYI / context,
|
||||
not a flaw. Inflated severities erode trust — be honest.
|
||||
- `category`: e.g. `correctness`, `security`, `perf`, `style`, `flag`.
|
||||
- `file`, `start_line`: anchor the comment to a single line inside the
|
||||
PR diff — the call site, the signature line, the conditional that's
|
||||
actually wrong. The tool always anchors to one line because GitHub
|
||||
renders multi-line ranges as walls of context that bury the comment.
|
||||
`end_line` is accepted for API compatibility but ignored.
|
||||
- `description`: what's wrong, in 1–4 sentences. Markdown is fine.
|
||||
- `suggestion`: **only** include for small, obvious fixes that fit in 4
|
||||
lines or fewer — a one-liner rename, a missing guard, a typo, a flipped
|
||||
condition. Anything longer reads as a rewrite rather than a review and
|
||||
is dropped by the tool. For non-trivial fixes, leave `suggestion` unset
|
||||
and let the description explain what's wrong; the author decides how to
|
||||
fix it.
|
||||
3. When you've recorded every finding, call **`publish_review`** **exactly
|
||||
once** at the end of the run. It batches eligible findings into a single
|
||||
GitHub PR Review with inline comments + suggestion blocks, and stores the
|
||||
GitHub comment IDs back so re-reviews can later resolve threads.
|
||||
- Do **not** write a summary or top-level take — `publish_review` formats
|
||||
the review body itself. Your only job is to record findings (or none)
|
||||
and call the tool. Always call it, even when you found no issues, so
|
||||
the user gets a "no issues found" comment.
|
||||
# The bar: file a finding only if it passes these criteria
|
||||
|
||||
### Re-reviewing on a new commit
|
||||
1. You can anchor it to a specific changed line and quote that line.
|
||||
2. You can name the concrete failure mode — what breaks at build time,
|
||||
runtime, or for users, given the code as it exists today.
|
||||
3. **Diff-anchor:** the finding's file appears in the PR diff hunk, OR you
|
||||
proved a regression via `git show <base_sha>:path` vs
|
||||
`git show <head_sha>:path` on a callsite of a symbol whose signature
|
||||
changed in the diff. Do not file bugs in unrelated files or subsystems
|
||||
based on inference alone.
|
||||
|
||||
If the user message says **"A new commit has been pushed"**, this is a
|
||||
re-review. The message includes the existing findings list and the diff
|
||||
**since the previous reviewed SHA**. Your job is to:
|
||||
# Do NOT file
|
||||
|
||||
- For each existing **open** finding, decide whether the new commits:
|
||||
- **resolved** it — call `update_finding(id, status="resolved")`.
|
||||
- **left it unchanged** — do nothing.
|
||||
- **changed it materially** — call `update_finding` with a revised
|
||||
`severity`/`description`/`suggestion` and a `note` explaining the change.
|
||||
- Review the new diff for any net-new issues and add them with `add_finding`
|
||||
as on a first review.
|
||||
- Finally call `publish_review` once. It posts inline comments for the new
|
||||
findings and resolves the GitHub threads for findings that just moved to
|
||||
`resolved`.
|
||||
- **Style / naming / convention nits.** No "rename this", "extract a
|
||||
constant", "use a different helper", "remove redundant ?.", "metric label
|
||||
is ambiguous", "this could be cleaner". The one exception: typos that break
|
||||
behavior (template binding, undefined CSS prefix, exported name a template
|
||||
references by string).
|
||||
- **Speculation.** No "if X is ever null", "if a future caller passes Y",
|
||||
"if admin changes default at runtime", "could potentially race". You need
|
||||
a concrete trigger reachable from the current code.
|
||||
- **Scope-policing / architectural critique.** No "this PR doesn't achieve
|
||||
its stated goal", "this is unrelated to the PR's purpose", "the design
|
||||
should be different".
|
||||
- **Pre-existing issues** not introduced by this diff.
|
||||
- **Out-of-diff / wrong-subsystem speculation.** Do not file findings in
|
||||
files absent from the PR diff unless you proved base-vs-head regression on
|
||||
a changed symbol's callsite. Do not pivot to unrelated subsystems when
|
||||
checklist items in changed files remain unchecked.
|
||||
- **Same-bug fan-out.** If the same defect appears in N files (e.g.
|
||||
`forEach(async ...)` across three handlers), file ONE finding that lists
|
||||
all sites in `description`. Not N findings.
|
||||
|
||||
You may use `list_findings()` at any time to inspect what's persisted.
|
||||
# Common defect patterns
|
||||
|
||||
### Hard rules
|
||||
Walk these every review. Most real defects fall into one of these:
|
||||
|
||||
- **You are read-only.** Do NOT commit. Do NOT push. Do NOT open or update
|
||||
PRs. Do NOT use `gh pr review` or `gh api ... /reviews` directly — use the
|
||||
`publish_review` tool instead so the findings list and GitHub stay in sync.
|
||||
- **Only review the diff.** Do not flag pre-existing code that the PR didn't
|
||||
touch. Anchor every finding to a line that the PR actually changes.
|
||||
- **One finding per distinct issue.** Don't split one bug into three findings,
|
||||
and don't merge unrelated issues into one.
|
||||
- **Suggestions are for small, obvious fixes only.** If the fix is more than
|
||||
~4 lines, skip the `suggestion` field — the description alone is more
|
||||
useful than a long rewrite. Description-only findings are the default;
|
||||
`suggestion` is the exception for trivially-actionable changes.
|
||||
- **Skip nits on a clean PR.** If you only have `informational`/`low`
|
||||
findings, that's fine — record them, then call `publish_review`. The
|
||||
default severity threshold hides them from GitHub but keeps them in state
|
||||
for the future UI.
|
||||
- **Refactor regression** — nil-check, logging, async-ness, lock scope, or
|
||||
sentinel handling dropped vs. base. Compare each touched function's old
|
||||
body to its new body.
|
||||
- **Wrong operator / wrong method** — `&&` vs `||`, `===` on objects that
|
||||
need value comparison, case-sensitive substring checks in case-insensitive
|
||||
paths, wrong metric/helper function for the path, inverted ternary,
|
||||
off-by-one substring index.
|
||||
- **Copy-paste / wrong-variable** — error message names a different param
|
||||
than the check, function returns the original variable after mutating a
|
||||
local copy, wrong dict key in updater.
|
||||
- **Async footguns** — `forEach(async ...)`, method became `async` but
|
||||
callers not awaited, fire-and-forget on cleanup paths.
|
||||
- **Read-modify-write that should be atomic** — `counter: row.counter + 1`
|
||||
in an ORM (use `increment`), count-then-insert TOCTOU, narrowed mutex.
|
||||
- **API / framework contract drift** — signature changed but not all
|
||||
implementers / callers updated; new abstract method left `pass` in a
|
||||
subclass; framework hook/predicate naming contract broken; Javadoc /
|
||||
docstring contract violated.
|
||||
- **Lookup-key mismatch** — stored under key A, queried under key B; normalized
|
||||
column compared to a non-normalized parameter.
|
||||
- **Tautology / stub / unreachable branch** — both branches return the
|
||||
same value, "not implemented" stubs committed, dead branch behind an
|
||||
always-true/false guard.
|
||||
- **Falsy-edge / truthy-zero** — `if x:` when x can be 0.0; `if result:`
|
||||
when result is a valid empty collection; `if not None` traps.
|
||||
- **Nil/None deref** — optional access without guard, `Optional.get()`
|
||||
without `isPresent`, accessing `metadata["key"]` that may not exist.
|
||||
- **Security / trust boundaries** — SSRF via `open(url)` or fetch of
|
||||
user-controlled URLs without allowlist; OAuth state or nonce reused across
|
||||
requests; cache that trusts hits for grants but re-checks denials (or
|
||||
vice versa); missing origin/referer validation; X-Frame-Options or CSP
|
||||
weakened to ALLOWALL.
|
||||
- **Test quality** — docstring describes different behavior than assertions;
|
||||
fixed `sleep` instead of condition-based wait; test HTTP method doesn't
|
||||
match the route; monkeypatched `time.sleep` that makes the test not wait.
|
||||
- **Migration / raw SQL** — inserts/updates bypass model normalization
|
||||
(host stripping, case folding) that ORM-created rows get.
|
||||
- **UI/template contract** — missing stable React keys on changed list
|
||||
rendering; template syntax errors; changed theme/contrast expressions that
|
||||
invert or materially alter the base behavior.
|
||||
- **Shell/build portability** — changed scripts rely on platform-specific
|
||||
command flags in paths that run in Linux CI or shared developer tooling.
|
||||
|
||||
# When to read beyond the diff
|
||||
|
||||
The diff is the starting point, not the whole job. Spend the grep budget
|
||||
when:
|
||||
|
||||
- **Title says rename / refactor / move / extract / split** → mandatory
|
||||
base-vs-head pass on every touched function. Was the nil-check preserved?
|
||||
The logging? traceID or log fields? The async-ness? The lock scope?
|
||||
Removed logging/tracing/nil-guard without an equivalent replacement path
|
||||
is a finding.
|
||||
- **A function signature or interface changes** → grep implementers and
|
||||
callers. Are they all updated?
|
||||
- **A new lookup helper appears** → find where the data was stored. Do the
|
||||
keys match?
|
||||
- **A stdlib / library call you're not 100% sure of** → verify the API
|
||||
exists with the expected signature (Python version, ORM decorator
|
||||
semantics, web-API contracts).
|
||||
- **Auth / permissions / caching code** → trace the resolution path. What
|
||||
does this actually return when the cache hits? When it misses? Don't just
|
||||
suggest tidying.
|
||||
- **Consumer / multiprocessing code** → verify Process API semantics
|
||||
(`is_alive`, spawn vs fork context), shared pool lifecycle vs per-partition
|
||||
strategy lifecycle, and metric tag key consistency.
|
||||
|
||||
# Review workflow — complete passes in order
|
||||
|
||||
Do not skip to deep flow analysis until Passes 1–4 are done. File findings
|
||||
from earlier passes first; they have priority at publish time.
|
||||
|
||||
## Pass 1: Mechanical grep (every changed file)
|
||||
|
||||
Grep the diff for these patterns on changed lines. Each hit is a candidate
|
||||
finding unless already covered:
|
||||
|
||||
- `Optional.get()` / `.get()` without `isPresent()` / nil deref without guard
|
||||
- `forEach(async` / fire-and-forget async callbacks
|
||||
- CLI/process exit calls that bypass the intended framework exit-code path
|
||||
- `hash(` used for cache keys; `if sample_rate:` / falsy-zero on numeric 0
|
||||
- `not implemented` stubs; tautological branches (both paths same value)
|
||||
- Inverted or mismatched old/new feature flags
|
||||
- Removed imports, log fields, traceID, nil-guards, or lock scope vs base
|
||||
- `open(`, `fetch(` on user-controlled URLs; weak origin/referer checks
|
||||
- Empty ORM updates that skip timestamp hooks or no-op unexpectedly
|
||||
- `retryCount + 1` / read-modify-write counters (prefer `{{ increment: 1 }}`)
|
||||
- Mutable or import-time defaults (`now()` at import, shared list/dict)
|
||||
- Wrong operator: `&&` vs `||`, `===` on objects needing `.isSame()`
|
||||
- Changed React list rendering without a stable `key` prop
|
||||
- Framework hook or predicate methods whose changed name no longer matches
|
||||
the framework contract
|
||||
|
||||
## Pass 2: Diff-line audit (every changed hunk)
|
||||
|
||||
For each changed hunk, ask: **what did this exact line change?** Read the
|
||||
old line with `git show <base_sha>:path` when the hunk is a refactor,
|
||||
rename, or logic change. Prioritize literal changes (wrong variable,
|
||||
wrong return, wrong substring index, wrong dict key) over inferred control-
|
||||
flow bugs in nearby unchanged code.
|
||||
|
||||
## Pass 3: Security / auth / cache (when diff touches these)
|
||||
|
||||
Mandatory when the diff includes auth, OAuth, permissions, caching, embed
|
||||
URLs, headers (X-Frame-Options, CSP), or `postMessage`:
|
||||
|
||||
- Trace cache hit vs miss: are grants and denials trusted symmetrically?
|
||||
- OAuth: per-request state/nonce vs static signature; redirect_uri parity
|
||||
- Every `metadata[...]`, pipeline state, and optional association access guarded
|
||||
- SSRF, origin validation completeness, raw HTML bypass paths
|
||||
- ERB/template syntax errors on changed templates
|
||||
|
||||
## Pass 4: Pipeline sweep (each touched handler/function)
|
||||
|
||||
After the first finding in a handler, model method, or consumer, continue
|
||||
the same function — do not stop:
|
||||
|
||||
validate → filter/dedupe → DB write → external API → email/calendar/task
|
||||
enqueue → error path (never assign error/nil to cache). Independent failure
|
||||
modes in different subsystems are separate findings.
|
||||
|
||||
On signature/interface changes: grep all implementers and callers.
|
||||
|
||||
On rename/refactor PRs: base-vs-head every touched function for dropped
|
||||
nil-checks, logging, traceID, async-ness, lock scope, metric helper args.
|
||||
|
||||
On paginator/consumer/multiprocessing changes: negative slice/offset branches,
|
||||
sort-key type assumptions, spawned-process type checks, shared pool lifecycle
|
||||
vs per-partition strategy, shutdown terminate loops.
|
||||
|
||||
## Pass 5: Deep flow (only if cap slots remain)
|
||||
|
||||
Only after Passes 1–4. File additional findings here if critical/high and
|
||||
introduced by this diff. Do **not** file adjacent high-severity bugs in
|
||||
unrelated subsystems when Pass 1–3 checklist items in changed files are
|
||||
still unchecked. Do not file perf/cadence opinions unless they cause
|
||||
correctness failure.
|
||||
|
||||
# Before publish_review
|
||||
|
||||
1. Call `list_findings`. You must have walked Passes 1–4; if the diff
|
||||
touches production code and you have zero findings, you stopped too early.
|
||||
2. **Dedup:** reject duplicate `(file, line, failure_mode)` entries.
|
||||
3. **Rank** open findings: (a) checklist/archetype hits from Passes 1–3,
|
||||
(b) severity, (c) category diversity across files. Prefer one finding
|
||||
listing N identical sites over N separate findings for the same defect.
|
||||
4. Keep only the strongest small set of findings. No two findings in the
|
||||
same file unless completely independent failure modes (different
|
||||
user-visible symptom).
|
||||
5. Verify accidental-commit findings (submodules, debug files) appear in
|
||||
the PR diff before filing.
|
||||
6. Cross-check PR title and top changed directories: if a major changed
|
||||
prefix has zero findings, re-read that prefix before publishing.
|
||||
|
||||
# Severity rubric (tied to runtime consequence)
|
||||
|
||||
- `critical` — panic, crash, data loss, auth bypass, security regression.
|
||||
- `high` — wrong result for users; clear correctness bug.
|
||||
- `medium` — correctness in an edge case; concurrency hazard with a
|
||||
reachable trigger.
|
||||
- `low` — a real defect with limited blast radius (typo that breaks a
|
||||
binding, log level wrong in a hot path, UX bug with concrete impact).
|
||||
|
||||
Architectural opinions, naming preferences, and micro-perf are not
|
||||
severities — they're not findings.
|
||||
|
||||
# Other rules
|
||||
|
||||
- Read-only. Do not commit, push, or use `gh pr review` / `gh api .../reviews`.
|
||||
- One finding per defect (with the fan-out rule above for cross-file bugs).
|
||||
- Include `suggestion` only when the fix is ≤4 lines and obvious.
|
||||
- Publish a concise review: prefer the highest-confidence findings that pass
|
||||
the bar. Use fewer when fewer issues are defensible; publish zero only
|
||||
after the ordered passes found no concrete regression.
|
||||
"""
|
||||
|
||||
|
||||
REVIEWER_EVAL_PROMPT_SUFFIX = """
|
||||
# Eval mode — calibration
|
||||
|
||||
This run is scored against a closed set of golden review comments per PR.
|
||||
The dataset expects 1-5 comments per PR (mean ~2).
|
||||
|
||||
- **Hard minimum: at least 1 finding per review.** Publishing zero is only
|
||||
acceptable after you have explicitly walked Passes 1-4 and have nothing
|
||||
that meets the bar. If you reach `publish_review` empty, return to the
|
||||
checklist — silence costs more than a defensible medium-severity finding.
|
||||
- **Hard cap: at most 3 findings per review.**
|
||||
- Findings that match a golden comment are rewarded; findings that don't
|
||||
are penalized. Missing a golden comment is also penalized. Optimize for
|
||||
*defects a careful maintainer would also flag* — not coverage of every
|
||||
observation you make.
|
||||
"""
|
||||
|
||||
|
||||
|
|
@ -179,13 +327,27 @@ def _reviewer_system_prompt(
|
|||
repo_owner: str,
|
||||
repo_name: str,
|
||||
pr_number: int | str,
|
||||
reviewer_eval: bool = False,
|
||||
repo_style_prompt: str | None = None,
|
||||
) -> str:
|
||||
return REVIEWER_PROMPT_TEMPLATE.format(
|
||||
prompt = REVIEWER_PROMPT_TEMPLATE.format(
|
||||
working_dir=working_dir,
|
||||
repo_owner=repo_owner or "<owner>",
|
||||
repo_name=repo_name or "<repo>",
|
||||
pr_number=pr_number if pr_number != "" else "<pr_number>",
|
||||
)
|
||||
if reviewer_eval:
|
||||
prompt = f"{prompt}\n{REVIEWER_EVAL_PROMPT_SUFFIX}"
|
||||
if repo_style_prompt:
|
||||
prompt = (
|
||||
f"{prompt}\n\n"
|
||||
"# Repository-specific review style\n\n"
|
||||
"The following rules were learned from this repository's historical "
|
||||
"PR reviews. Apply them when they agree with the global bar above; "
|
||||
"they refine tone, severity, and what this team typically flags.\n\n"
|
||||
f"{repo_style_prompt}"
|
||||
)
|
||||
return prompt
|
||||
|
||||
|
||||
def _build_first_review_context(
|
||||
|
|
@ -206,11 +368,11 @@ def _build_first_review_context(
|
|||
f"- head_sha: {head_sha}\n\n"
|
||||
f"Fetch the diff yourself with "
|
||||
f"`GH_TOKEN=dummy gh pr diff {pr_number} --repo {repo_owner}/{repo_name}`, "
|
||||
f"then review only what's in that diff.\n\n"
|
||||
f"This is a first review — there are no existing findings. Record real "
|
||||
f"issues with `add_finding` (one per issue; only include `suggestion` "
|
||||
f"when the fix is ≤4 lines and obvious), then call `publish_review` "
|
||||
f"once at the end."
|
||||
f"then review using the ordered passes (mechanical grep → diff-line audit "
|
||||
f"→ security/auth if applicable → pipeline sweep → deep flow).\n\n"
|
||||
f"This is a first review — there are no existing findings. Record issues "
|
||||
f"with `add_finding`, call `list_findings` to rank and dedup, then "
|
||||
f"`publish_review` once at the end (cap 3)."
|
||||
)
|
||||
|
||||
|
||||
|
|
@ -340,23 +502,29 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
|
|||
)
|
||||
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.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
|
||||
model_kwargs = provider_model_kwargs(
|
||||
model_id,
|
||||
reasoning_effort,
|
||||
max_tokens=DEFAULT_LLM_MAX_TOKENS,
|
||||
openai_reasoning_default=DEFAULT_LLM_REASONING,
|
||||
)
|
||||
|
||||
reviewer_eval = (
|
||||
config["configurable"].get("reviewer_eval") is True
|
||||
or config["configurable"].get("eval") is True
|
||||
)
|
||||
repo_style_prompt: str | None = None
|
||||
if repo_owner and repo_name:
|
||||
from .dashboard.review_styles import get_repo_custom_prompt
|
||||
|
||||
repo_style_prompt = await get_repo_custom_prompt(repo_owner, repo_name)
|
||||
system_prompt = _reviewer_system_prompt(
|
||||
f"{work_dir}/{repo_name}" if repo_name else work_dir,
|
||||
repo_owner=repo_owner,
|
||||
repo_name=repo_name,
|
||||
pr_number=pr_number if isinstance(pr_number, int) else "",
|
||||
reviewer_eval=reviewer_eval,
|
||||
repo_style_prompt=repo_style_prompt,
|
||||
)
|
||||
if review_context:
|
||||
system_prompt = f"{system_prompt}\n\n{review_context}"
|
||||
|
|
@ -364,13 +532,20 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
|
|||
return create_deep_agent(
|
||||
model=make_model(model_id, **model_kwargs),
|
||||
system_prompt=system_prompt,
|
||||
tools=[add_finding, update_finding, list_findings, publish_review],
|
||||
tools=[
|
||||
add_finding,
|
||||
update_finding,
|
||||
list_findings,
|
||||
publish_review,
|
||||
web_search,
|
||||
fetch_url,
|
||||
http_request,
|
||||
],
|
||||
backend=sandbox_backend,
|
||||
middleware=[
|
||||
SanitizeToolInputsMiddleware(),
|
||||
ModelCallLimitMiddleware(run_limit=MODEL_CALL_RECURSION_LIMIT, exit_behavior="end"),
|
||||
ToolErrorMiddleware(),
|
||||
SlackAssistantStatusMiddleware(),
|
||||
ExcludeToolsMiddleware(excluded=frozenset({"task"})),
|
||||
],
|
||||
).with_config(config)
|
||||
|
|
|
|||
|
|
@ -39,18 +39,22 @@ def clip_suggestion(suggestion: str | None) -> tuple[str | None, bool]:
|
|||
return suggestion, False
|
||||
|
||||
|
||||
Severity = Literal["informational", "low", "medium", "high", "critical"]
|
||||
Severity = Literal["low", "medium", "high", "critical"]
|
||||
Confidence = Literal["low", "medium", "high"]
|
||||
FindingStatus = Literal["open", "resolved", "dismissed"]
|
||||
DiffSide = Literal["LEFT", "RIGHT"]
|
||||
|
||||
SEVERITY_ORDER: dict[Severity, int] = {
|
||||
"informational": 0,
|
||||
"low": 1,
|
||||
"medium": 2,
|
||||
"high": 3,
|
||||
"critical": 4,
|
||||
"low": 0,
|
||||
"medium": 1,
|
||||
"high": 2,
|
||||
"critical": 3,
|
||||
}
|
||||
|
||||
# Confidence is recorded on every finding for post-hoc calibration analysis
|
||||
# but does not gate publication — the system prompt's defensibility bar is
|
||||
# the discipline.
|
||||
|
||||
|
||||
class Finding(TypedDict, total=False):
|
||||
"""A single review finding.
|
||||
|
|
@ -61,6 +65,7 @@ class Finding(TypedDict, total=False):
|
|||
|
||||
id: str
|
||||
severity: Severity
|
||||
confidence: Confidence
|
||||
category: str
|
||||
file: str
|
||||
start_line: int | None
|
||||
|
|
@ -108,6 +113,7 @@ def new_finding(
|
|||
end_line: int | None,
|
||||
description: str,
|
||||
sha: str,
|
||||
confidence: Confidence = "medium",
|
||||
side: DiffSide = "RIGHT",
|
||||
suggestion: str | None = None,
|
||||
diff_hunk: str | None = None,
|
||||
|
|
@ -117,6 +123,7 @@ def new_finding(
|
|||
return {
|
||||
"id": finding_id or new_finding_id(),
|
||||
"severity": severity,
|
||||
"confidence": confidence,
|
||||
"category": category,
|
||||
"file": file,
|
||||
"start_line": start_line,
|
||||
|
|
@ -297,16 +304,16 @@ def filter_findings_for_publish(
|
|||
- sorted by severity descending, then file/start_line for stable ordering
|
||||
- capped at ``cap`` to avoid review spam
|
||||
"""
|
||||
threshold_rank = SEVERITY_ORDER[severity_threshold]
|
||||
severity_rank = SEVERITY_ORDER[severity_threshold]
|
||||
eligible = [
|
||||
finding
|
||||
for finding in findings
|
||||
if finding.get("status", "open") == "open"
|
||||
and SEVERITY_ORDER.get(finding.get("severity", "informational"), 0) >= threshold_rank
|
||||
and SEVERITY_ORDER.get(finding.get("severity", "low"), 0) >= severity_rank
|
||||
]
|
||||
eligible.sort(
|
||||
key=lambda f: (
|
||||
-SEVERITY_ORDER.get(f.get("severity", "informational"), 0),
|
||||
-SEVERITY_ORDER.get(f.get("severity", "low"), 0),
|
||||
f.get("file", ""),
|
||||
f.get("start_line") or 0,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -64,12 +64,11 @@ 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,
|
||||
DEFAULT_LLM_REASONING,
|
||||
ModelKwargs,
|
||||
OpenAIReasoning,
|
||||
fallback_model_id_for,
|
||||
make_model,
|
||||
provider_model_kwargs,
|
||||
)
|
||||
from .utils.sandbox import create_sandbox
|
||||
from .utils.sandbox_paths import aresolve_sandbox_work_dir
|
||||
|
|
@ -333,53 +332,11 @@ async def ensure_sandbox_for_thread(thread_id: str) -> SandboxBackendProtocol:
|
|||
|
||||
|
||||
DEFAULT_LLM_MODEL_ID = "openai:gpt-5.5"
|
||||
DEFAULT_LLM_REASONING: OpenAIReasoning = {"effort": "medium"}
|
||||
DEFAULT_LLM_MAX_TOKENS = 64_000
|
||||
DEFAULT_RECURSION_LIMIT = 9_999
|
||||
MODEL_CALL_RECURSION_LIMIT = 5_000 # ~half the recursion limit to account for tool calls
|
||||
|
||||
|
||||
def _openai_reasoning_for(profile_effort: str | None) -> OpenAIReasoning | None:
|
||||
"""Return an OpenAI reasoning kwarg from a (validated) profile effort.
|
||||
|
||||
Anthropic-only efforts like ``"max"`` are dropped — OpenAI's effort
|
||||
Literal doesn't accept them. Falls back to the default effort when the
|
||||
profile didn't override.
|
||||
"""
|
||||
effort = profile_effort or DEFAULT_LLM_REASONING.get("effort")
|
||||
if effort == "none":
|
||||
return {"effort": "none"}
|
||||
if effort == "low":
|
||||
return {"effort": "low"}
|
||||
if effort == "medium":
|
||||
return {"effort": "medium"}
|
||||
if effort == "high":
|
||||
return {"effort": "high"}
|
||||
if effort == "xhigh":
|
||||
return {"effort": "xhigh"}
|
||||
return None
|
||||
|
||||
|
||||
_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.
|
||||
|
||||
Latest Claude models use adaptive thinking with an effort hint instead of
|
||||
manual thinking token budgets.
|
||||
"""
|
||||
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:
|
||||
sandbox_backend = SANDBOX_BACKENDS.get(thread_id)
|
||||
if sandbox_backend is None:
|
||||
|
|
@ -436,18 +393,11 @@ async def get_agent(config: RunnableConfig) -> Pregel:
|
|||
model_id = overridden_model
|
||||
profile_effort = overridden_effort
|
||||
|
||||
model_kwargs: ModelKwargs = {"max_tokens": DEFAULT_LLM_MAX_TOKENS}
|
||||
if model_id.startswith("openai:"):
|
||||
reasoning = _openai_reasoning_for(profile_effort)
|
||||
if reasoning is not None:
|
||||
model_kwargs["reasoning"] = reasoning
|
||||
elif model_id.startswith("anthropic:"):
|
||||
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
|
||||
model_kwargs = provider_model_kwargs(
|
||||
model_id,
|
||||
profile_effort,
|
||||
max_tokens=DEFAULT_LLM_MAX_TOKENS,
|
||||
)
|
||||
|
||||
fallback_model_id = os.environ.get("LLM_FALLBACK_MODEL_ID") or fallback_model_id_for(model_id)
|
||||
fallback_middleware: list[Any] = []
|
||||
|
|
|
|||
|
|
@ -10,6 +10,7 @@ from langgraph.config import get_config
|
|||
from ..reviewer_diff import is_range_in_diff
|
||||
from ..reviewer_findings import (
|
||||
MAX_SUGGESTION_LINES,
|
||||
Confidence,
|
||||
DiffSide,
|
||||
Finding,
|
||||
Severity,
|
||||
|
|
@ -22,6 +23,7 @@ from ..reviewer_findings import (
|
|||
|
||||
def add_finding(
|
||||
severity: str,
|
||||
confidence: str,
|
||||
category: str,
|
||||
file: str,
|
||||
description: str,
|
||||
|
|
@ -47,7 +49,8 @@ def add_finding(
|
|||
issue truly isn't anchored to a line.
|
||||
|
||||
Args:
|
||||
severity: One of ``informational``, ``low``, ``medium``, ``high``, ``critical``.
|
||||
severity: One of ``low``, ``medium``, ``high``, ``critical``.
|
||||
confidence: One of ``low``, ``medium``, ``high``.
|
||||
category: Short category label (``correctness``, ``security``, ``perf``,
|
||||
``style``, ``flag``, etc.). Free-form; used for grouping in the UI.
|
||||
file: Repo-relative path of the file the finding refers to.
|
||||
|
|
@ -79,8 +82,10 @@ def add_finding(
|
|||
if start_line is None and end_line is not None:
|
||||
start_line = end_line
|
||||
|
||||
if severity not in {"informational", "low", "medium", "high", "critical"}:
|
||||
if severity not in {"low", "medium", "high", "critical"}:
|
||||
return {"success": False, "error": f"Invalid severity: {severity}"}
|
||||
if confidence not in {"low", "medium", "high"}:
|
||||
return {"success": False, "error": f"Invalid confidence: {confidence}"}
|
||||
if side not in {"LEFT", "RIGHT"}:
|
||||
return {"success": False, "error": f"Invalid side: {side}"}
|
||||
if start_line is not None and end_line is not None and end_line < start_line:
|
||||
|
|
@ -116,6 +121,7 @@ def add_finding(
|
|||
|
||||
finding: Finding = new_finding(
|
||||
severity=_cast_severity(severity),
|
||||
confidence=_cast_confidence(confidence),
|
||||
category=category,
|
||||
file=file,
|
||||
start_line=start_line,
|
||||
|
|
@ -144,5 +150,9 @@ def _cast_severity(value: str) -> Severity:
|
|||
return value # type: ignore[return-value]
|
||||
|
||||
|
||||
def _cast_confidence(value: str) -> Confidence:
|
||||
return value # type: ignore[return-value]
|
||||
|
||||
|
||||
def _cast_side(value: str) -> DiffSide:
|
||||
return value # type: ignore[return-value]
|
||||
|
|
|
|||
|
|
@ -69,7 +69,7 @@ def publish_review(
|
|||
Dictionary with ``success``, ``review_id``, ``surfaced_count``,
|
||||
``hidden_count``, ``resolved_thread_count``.
|
||||
"""
|
||||
if severity_threshold not in {"informational", "low", "medium", "high", "critical"}:
|
||||
if severity_threshold not in {"low", "medium", "high", "critical"}:
|
||||
return {"success": False, "error": f"Invalid severity_threshold: {severity_threshold}"}
|
||||
|
||||
config = get_config()
|
||||
|
|
|
|||
53
agent/tools/save_review_style.py
Normal file
53
agent/tools/save_review_style.py
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
"""Tool: persist synthesized per-repo review style prompt."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from typing import Any
|
||||
|
||||
from langgraph.config import get_config
|
||||
|
||||
from ..dashboard.review_styles import mark_analysis_completed, mark_analysis_failed
|
||||
|
||||
|
||||
def save_review_style_prompt(
|
||||
custom_prompt: str,
|
||||
analysis_summary: str = "",
|
||||
top_reviewers: str = "",
|
||||
prs_sampled: int = 0,
|
||||
reviews_sampled: int = 0,
|
||||
) -> dict[str, Any]:
|
||||
"""Save the synthesized repository-specific review style prompt.
|
||||
|
||||
Call this once at the end of style analysis with the final prompt text
|
||||
that should be injected into the reviewer agent for this repository.
|
||||
"""
|
||||
config = get_config()
|
||||
configurable = config.get("configurable") or {}
|
||||
full_name = configurable.get("review_style_full_name")
|
||||
if not isinstance(full_name, str) or "/" not in full_name:
|
||||
return {"ok": False, "error": "review_style_full_name missing from config"}
|
||||
|
||||
reviewers_from_args = [r.strip() for r in top_reviewers.split(",") if r.strip()]
|
||||
reviewers_from_config = configurable.get("review_style_top_reviewers") or []
|
||||
merged_reviewers = reviewers_from_args or (
|
||||
list(reviewers_from_config) if isinstance(reviewers_from_config, list) else []
|
||||
)
|
||||
prs_count = prs_sampled or int(configurable.get("review_style_prs_sampled") or 0)
|
||||
reviews_count = reviews_sampled or int(configurable.get("review_style_reviews_sampled") or 0)
|
||||
|
||||
if not custom_prompt.strip():
|
||||
asyncio.run(mark_analysis_failed(full_name, "custom_prompt was empty"))
|
||||
return {"ok": False, "error": "custom_prompt cannot be empty"}
|
||||
|
||||
record = asyncio.run(
|
||||
mark_analysis_completed(
|
||||
full_name,
|
||||
custom_prompt=custom_prompt.strip(),
|
||||
analysis_summary=analysis_summary.strip(),
|
||||
top_reviewers=merged_reviewers,
|
||||
prs_sampled=prs_count,
|
||||
reviews_sampled=reviews_count,
|
||||
)
|
||||
)
|
||||
return {"ok": True, "full_name": full_name, "status": record.get("status")}
|
||||
|
|
@ -19,6 +19,7 @@ def update_finding(
|
|||
finding_id: str,
|
||||
status: str | None = None,
|
||||
severity: str | None = None,
|
||||
confidence: str | None = None,
|
||||
description: str | None = None,
|
||||
suggestion: str | None = None,
|
||||
note: str | None = None,
|
||||
|
|
@ -35,6 +36,8 @@ def update_finding(
|
|||
status: New status (``open``, ``resolved``, ``dismissed``).
|
||||
Use ``resolved`` when the new commits address the issue.
|
||||
severity: New severity, if reassessing.
|
||||
confidence: New confidence rating (``low``, ``medium``, ``high``), if
|
||||
new commits change how sure you are the finding is a real issue.
|
||||
description: New description body, if revising.
|
||||
suggestion: New replacement text. Pass an empty string to clear it.
|
||||
Capped at 4 lines — longer values are dropped (the finding keeps
|
||||
|
|
@ -47,14 +50,10 @@ def update_finding(
|
|||
"""
|
||||
if status is not None and status not in {"open", "resolved", "dismissed"}:
|
||||
return {"success": False, "error": f"Invalid status: {status}"}
|
||||
if severity is not None and severity not in {
|
||||
"informational",
|
||||
"low",
|
||||
"medium",
|
||||
"high",
|
||||
"critical",
|
||||
}:
|
||||
if severity is not None and severity not in {"low", "medium", "high", "critical"}:
|
||||
return {"success": False, "error": f"Invalid severity: {severity}"}
|
||||
if confidence is not None and confidence not in {"low", "medium", "high"}:
|
||||
return {"success": False, "error": f"Invalid confidence: {confidence}"}
|
||||
|
||||
updates: dict[str, Any] = {}
|
||||
suggestion_dropped = False
|
||||
|
|
@ -62,6 +61,8 @@ def update_finding(
|
|||
updates["status"] = status
|
||||
if severity is not None:
|
||||
updates["severity"] = severity
|
||||
if confidence is not None:
|
||||
updates["confidence"] = confidence
|
||||
if description is not None:
|
||||
updates["description"] = description
|
||||
if suggestion is not None:
|
||||
|
|
|
|||
|
|
@ -8,10 +8,12 @@ OPENAI_RESPONSES_WS_BASE_URL = "wss://api.openai.com/v1"
|
|||
# primary provider a fair chance before the fallback middleware kicks in.
|
||||
DEFAULT_MAX_RETRIES = 6
|
||||
|
||||
DEFAULT_LLM_REASONING: "OpenAIReasoning" = {"effort": "medium"}
|
||||
|
||||
OpenAIReasoningEffort = Literal["none", "low", "medium", "high", "xhigh"]
|
||||
AnthropicThinkingType = Literal["adaptive"]
|
||||
AnthropicEffort = Literal["low", "medium", "high", "xhigh", "max"]
|
||||
GoogleThinkingLevel = Literal["minimal", "low", "medium", "high"]
|
||||
|
||||
|
||||
class OpenAIReasoning(TypedDict, total=False):
|
||||
|
|
@ -27,10 +29,14 @@ class ModelKwargs(TypedDict, total=False):
|
|||
reasoning: OpenAIReasoning | None
|
||||
thinking: AnthropicThinking | None
|
||||
effort: AnthropicEffort | None
|
||||
thinking_level: GoogleThinkingLevel | None
|
||||
temperature: float | None
|
||||
max_retries: int | None
|
||||
|
||||
|
||||
_ANTHROPIC_EFFORTS: set[AnthropicEffort] = {"low", "medium", "high", "xhigh", "max"}
|
||||
|
||||
|
||||
def make_model(model_id: str, **kwargs: Unpack[ModelKwargs]):
|
||||
model_kwargs: dict[str, object] = kwargs.copy()
|
||||
model_kwargs.setdefault("max_retries", DEFAULT_MAX_RETRIES)
|
||||
|
|
@ -46,11 +52,90 @@ def fallback_model_id_for(primary_model_id: str) -> str | None:
|
|||
"""Return the cross-provider fallback model id for a given primary, if any.
|
||||
|
||||
Anthropic primaries fall back to OpenAI and vice versa. Returns ``None``
|
||||
when the provider has no configured cross-provider fallback (e.g. local
|
||||
or self-hosted providers we don't want to silently route off-host).
|
||||
when the provider has no configured cross-provider fallback (e.g. Google,
|
||||
local, or self-hosted providers we don't want to silently route off-host).
|
||||
"""
|
||||
if primary_model_id.startswith("anthropic:"):
|
||||
return "openai:gpt-5.5"
|
||||
if primary_model_id.startswith("openai:"):
|
||||
return "anthropic:claude-opus-4-5"
|
||||
return None
|
||||
|
||||
|
||||
def is_gemini_3_family(model_id: str) -> bool:
|
||||
model_name = model_id.split(":", 1)[-1]
|
||||
return model_name.startswith("gemini-3")
|
||||
|
||||
|
||||
def openai_reasoning_for(
|
||||
profile_effort: str | None,
|
||||
*,
|
||||
default_effort: OpenAIReasoningEffort | None = None,
|
||||
) -> OpenAIReasoning | None:
|
||||
"""Return an OpenAI reasoning kwarg from a profile effort string."""
|
||||
effort = profile_effort or default_effort or DEFAULT_LLM_REASONING.get("effort")
|
||||
if effort == "none":
|
||||
return {"effort": "none"}
|
||||
if effort == "low":
|
||||
return {"effort": "low"}
|
||||
if effort == "medium":
|
||||
return {"effort": "medium"}
|
||||
if effort == "high":
|
||||
return {"effort": "high"}
|
||||
if effort == "xhigh":
|
||||
return {"effort": "xhigh"}
|
||||
return None
|
||||
|
||||
|
||||
def anthropic_thinking_for(profile_effort: str | None) -> AnthropicThinking | None:
|
||||
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 google_thinking_level_for(profile_effort: str | None) -> GoogleThinkingLevel | None:
|
||||
"""Map profile effort to Gemini 3+ ``thinking_level``."""
|
||||
if profile_effort == "none":
|
||||
return "minimal"
|
||||
if profile_effort == "low":
|
||||
return "low"
|
||||
if profile_effort == "medium":
|
||||
return "medium"
|
||||
if profile_effort in ("high", "xhigh", "max"):
|
||||
return "high"
|
||||
return None
|
||||
|
||||
|
||||
def provider_model_kwargs(
|
||||
model_id: str,
|
||||
profile_effort: str | None,
|
||||
*,
|
||||
max_tokens: int,
|
||||
openai_reasoning_default: OpenAIReasoning | None = None,
|
||||
) -> ModelKwargs:
|
||||
"""Build provider-specific kwargs for ``make_model`` from a model id and effort."""
|
||||
kwargs: ModelKwargs = {"max_tokens": max_tokens}
|
||||
if model_id.startswith("openai:"):
|
||||
reasoning = openai_reasoning_for(profile_effort)
|
||||
if reasoning is not None:
|
||||
kwargs["reasoning"] = reasoning
|
||||
elif openai_reasoning_default is not None:
|
||||
kwargs["reasoning"] = openai_reasoning_default
|
||||
elif model_id.startswith("anthropic:"):
|
||||
thinking = anthropic_thinking_for(profile_effort)
|
||||
if thinking is not None:
|
||||
kwargs["thinking"] = thinking
|
||||
effort = anthropic_effort_for(profile_effort)
|
||||
if effort is not None:
|
||||
kwargs["effort"] = effort
|
||||
elif model_id.startswith("google_genai:") and is_gemini_3_family(model_id):
|
||||
thinking_level = google_thinking_level_for(profile_effort)
|
||||
if thinking_level is not None:
|
||||
kwargs["thinking_level"] = thinking_level
|
||||
return kwargs
|
||||
|
|
|
|||
|
|
@ -60,6 +60,25 @@ 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.
|
||||
|
||||
## Per-repo review style prompts
|
||||
|
||||
At runtime the reviewer loads a custom style guide from LangGraph Store when
|
||||
`configurable.repo` is set (`owner` + `name` → store key `owner/name`). This
|
||||
applies to **eval runs too**, as long as a completed style profile exists for
|
||||
that repo.
|
||||
|
||||
The Martian benchmark uses these upstream repos (10 PRs each):
|
||||
|
||||
- `getsentry/sentry`
|
||||
- `keycloak/keycloak`
|
||||
- `grafana/grafana`
|
||||
- `discourse/discourse`
|
||||
- `calcom/cal.com`
|
||||
|
||||
Before scoring with repo-specific styles, run **Review styles** analysis in the
|
||||
dashboard for each repo (or copy prompts into store). Re-run `make dev` so the
|
||||
reviewer graph sees the same store.
|
||||
|
||||
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.
|
||||
|
|
|
|||
|
|
@ -1,15 +1,18 @@
|
|||
dataset_name = "openswe-reviewer-v1"
|
||||
experiment_prefix = "openswe-reviewer-verified-deployed"
|
||||
max_concurrency = 2
|
||||
experiment_prefix = "openswe-review-confidence"
|
||||
max_concurrency = 5
|
||||
|
||||
# Leave blank to use LANGGRAPH_URL or local dev.
|
||||
langgraph_url = ""
|
||||
assistant_id = "reviewer"
|
||||
model_id = "anthropic:claude-opus-4-7"
|
||||
reasoning_effort = "high"
|
||||
# models: openai:gpt-5.5, anthropic:claude-opus-4-7, google_genai:gemini-3.5-flash
|
||||
model_id = "google_genai:gemini-3.5-flash"
|
||||
reasoning_effort = "medium"
|
||||
|
||||
# Use "surfaced_findings" to score only findings that pass the production
|
||||
# severity threshold and cap.
|
||||
# score_mode:
|
||||
# - "all_findings" — score every add_finding the agent emits (no gating).
|
||||
# - "surfaced_findings" — only findings that pass the production severity
|
||||
# threshold and cap.
|
||||
score_mode = "all_findings"
|
||||
severity_threshold = "medium"
|
||||
cap = 4
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ 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"]
|
||||
Severity = Literal["low", "medium", "high", "critical"]
|
||||
|
||||
|
||||
class ReviewerEvalConfig(TypedDict, total=False):
|
||||
|
|
@ -87,7 +87,7 @@ def _coerce_config(raw: dict[str, Any]) -> ReviewerEvalConfig:
|
|||
config["score_mode"] = score_mode
|
||||
|
||||
severity_threshold = raw.get("severity_threshold")
|
||||
if severity_threshold in {"informational", "low", "medium", "high", "critical"}:
|
||||
if severity_threshold in {"low", "medium", "high", "critical"}:
|
||||
config["severity_threshold"] = severity_threshold
|
||||
|
||||
cap = raw.get("cap")
|
||||
|
|
|
|||
|
|
@ -24,7 +24,7 @@ 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"}
|
||||
_VALID_SEVERITIES: set[Severity] = {"low", "medium", "high", "critical"}
|
||||
|
||||
_THREAD_IDS: set[str] = set()
|
||||
_THREAD_IDS_LOCK = threading.Lock()
|
||||
|
|
|
|||
|
|
@ -3,7 +3,8 @@
|
|||
"python_version": "3.12",
|
||||
"graphs": {
|
||||
"agent": "agent.server:get_agent",
|
||||
"reviewer": "agent.reviewer:get_reviewer_agent"
|
||||
"reviewer": "agent.reviewer:get_reviewer_agent",
|
||||
"review_style_analyzer": "agent.review_style_analyzer:get_review_style_analyzer"
|
||||
},
|
||||
"dependencies": ["."],
|
||||
"http": {
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ dependencies = [
|
|||
"langchain-modal>=0.0.3",
|
||||
"langchain-runloop>=0.0.4",
|
||||
"exa-py>=2.12.1",
|
||||
"langchain-google-genai>=4.2.2",
|
||||
]
|
||||
|
||||
[project.optional-dependencies]
|
||||
|
|
|
|||
|
|
@ -1,13 +1,13 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from agent.server import _anthropic_effort_for, _anthropic_thinking_for
|
||||
from agent.utils.model 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"
|
||||
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
|
||||
assert anthropic_thinking_for("unknown") is None
|
||||
assert anthropic_effort_for("unknown") is None
|
||||
|
|
|
|||
26
tests/test_google_model.py
Normal file
26
tests/test_google_model.py
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
from agent.utils.model import (
|
||||
google_thinking_level_for,
|
||||
is_gemini_3_family,
|
||||
provider_model_kwargs,
|
||||
)
|
||||
|
||||
|
||||
def test_gemini_3_family_detection() -> None:
|
||||
assert is_gemini_3_family("google_genai:gemini-3.5-flash") is True
|
||||
assert is_gemini_3_family("google_genai:gemini-2.5-flash") is False
|
||||
|
||||
|
||||
def test_google_thinking_level_maps_effort() -> None:
|
||||
assert google_thinking_level_for("medium") == "medium"
|
||||
assert google_thinking_level_for("high") == "high"
|
||||
assert google_thinking_level_for("unknown") is None
|
||||
|
||||
|
||||
def test_provider_model_kwargs_for_google() -> None:
|
||||
kwargs = provider_model_kwargs(
|
||||
"google_genai:gemini-3.5-flash",
|
||||
"high",
|
||||
max_tokens=16_000,
|
||||
)
|
||||
assert kwargs["max_tokens"] == 16_000
|
||||
assert kwargs["thinking_level"] == "high"
|
||||
16
tests/test_normalize_repo.py
Normal file
16
tests/test_normalize_repo.py
Normal file
|
|
@ -0,0 +1,16 @@
|
|||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.dashboard.review_styles import normalize_repo_full_name
|
||||
|
||||
|
||||
def test_normalize_repo_full_name_accepts_urls() -> None:
|
||||
assert normalize_repo_full_name("https://github.com/langchain-ai/langgraph") == (
|
||||
"langchain-ai/langgraph"
|
||||
)
|
||||
|
||||
|
||||
def test_normalize_repo_full_name_rejects_invalid() -> None:
|
||||
with pytest.raises(ValueError):
|
||||
normalize_repo_full_name("not-a-repo")
|
||||
27
tests/test_review_style_collector.py
Normal file
27
tests/test_review_style_collector.py
Normal file
|
|
@ -0,0 +1,27 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from agent.review_style_collector import (
|
||||
ReviewSample,
|
||||
ReviewStyleSamples,
|
||||
format_samples_for_analyzer,
|
||||
)
|
||||
|
||||
|
||||
def test_format_samples_for_analyzer_groups_by_reviewer() -> None:
|
||||
samples = ReviewStyleSamples(
|
||||
full_name="acme/widget",
|
||||
owner="acme",
|
||||
name="widget",
|
||||
top_reviewers=["alice", "bob"],
|
||||
samples=[
|
||||
ReviewSample(1, "alice", "review", "Missing nil check on line 42.", state="COMMENTED"),
|
||||
ReviewSample(2, "bob", "inline", "Race in cache update path.", path="cache.go"),
|
||||
],
|
||||
prs_scanned=5,
|
||||
reviews_scanned=10,
|
||||
)
|
||||
text = format_samples_for_analyzer(samples)
|
||||
assert "acme/widget" in text
|
||||
assert "## Reviewer: @alice" in text
|
||||
assert "PR #1" in text
|
||||
assert "Race in cache" in text
|
||||
54
tests/test_review_styles_store.py
Normal file
54
tests/test_review_styles_store.py
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from agent.dashboard.review_styles import (
|
||||
create_review_style,
|
||||
get_repo_custom_prompt,
|
||||
set_custom_prompt,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_repo_custom_prompt_returns_trimmed_text() -> None:
|
||||
with patch(
|
||||
"agent.dashboard.review_styles.get_review_style",
|
||||
new_callable=AsyncMock,
|
||||
return_value={"custom_prompt": " Flag nil deref aggressively.\n"},
|
||||
):
|
||||
prompt = await get_repo_custom_prompt("acme", "repo")
|
||||
assert prompt == "Flag nil deref aggressively."
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_create_review_style_puts_new_record() -> None:
|
||||
mock_put = AsyncMock()
|
||||
with (
|
||||
patch(
|
||||
"agent.dashboard.review_styles._get_value", new_callable=AsyncMock, return_value=None
|
||||
),
|
||||
patch("agent.dashboard.review_styles._client") as mock_client,
|
||||
):
|
||||
mock_client.return_value.store.put_item = mock_put
|
||||
record = await create_review_style("acme/repo", "octo")
|
||||
assert record["full_name"] == "acme/repo"
|
||||
assert record["status"] == "idle"
|
||||
mock_put.assert_awaited_once()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_set_custom_prompt_updates_store() -> None:
|
||||
with (
|
||||
patch(
|
||||
"agent.dashboard.review_styles.get_review_style",
|
||||
new_callable=AsyncMock,
|
||||
return_value={"full_name": "acme/repo", "status": "completed"},
|
||||
),
|
||||
patch(
|
||||
"agent.dashboard.review_styles.update_review_style", new_callable=AsyncMock
|
||||
) as mock_up,
|
||||
):
|
||||
await set_custom_prompt("acme/repo", "Use direct tone.")
|
||||
mock_up.assert_awaited_once()
|
||||
|
|
@ -8,6 +8,32 @@ from langgraph.graph.state import RunnableConfig
|
|||
from agent import reviewer
|
||||
|
||||
|
||||
def test_reviewer_system_prompt_formats_without_keyerror() -> None:
|
||||
prompt = reviewer._reviewer_system_prompt(
|
||||
"/workspace/repo",
|
||||
repo_owner="acme",
|
||||
repo_name="repo",
|
||||
pr_number=42,
|
||||
)
|
||||
assert "acme/repo" in prompt
|
||||
assert "Common defect patterns" in prompt
|
||||
assert "benchmark" not in prompt.lower()
|
||||
assert "golden" not in prompt.lower()
|
||||
assert "at least 1 finding" not in prompt.lower()
|
||||
|
||||
|
||||
def test_reviewer_system_prompt_includes_repo_style_section() -> None:
|
||||
prompt = reviewer._reviewer_system_prompt(
|
||||
"/workspace/repo",
|
||||
repo_owner="acme",
|
||||
repo_name="repo",
|
||||
pr_number=42,
|
||||
repo_style_prompt="Always flag missing tests for API changes.",
|
||||
)
|
||||
assert "Repository-specific review style" in prompt
|
||||
assert "missing tests for API" in prompt
|
||||
|
||||
|
||||
class _DummyAgent:
|
||||
def with_config(self, config: dict[str, object]) -> _DummyAgent:
|
||||
self.config = config
|
||||
|
|
@ -93,3 +119,50 @@ async def test_reviewer_applies_eval_model_and_effort_overrides() -> None:
|
|||
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"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reviewer_injects_repo_style_during_eval() -> None:
|
||||
config: RunnableConfig = {
|
||||
"configurable": {
|
||||
"__is_for_execution__": True,
|
||||
"thread_id": "reviewer-thread-id",
|
||||
"reviewer_eval": True,
|
||||
"eval": True,
|
||||
"repo": {"owner": "getsentry", "name": "sentry"},
|
||||
"pr_number": 1,
|
||||
"pr_url": "https://github.com/getsentry/sentry/pull/1",
|
||||
"base_sha": "base",
|
||||
"head_sha": "head",
|
||||
},
|
||||
"metadata": {},
|
||||
}
|
||||
captured: dict[str, str] = {}
|
||||
|
||||
def fake_create_deep_agent(*, system_prompt: str, **kwargs: object) -> _DummyAgent:
|
||||
captured["system_prompt"] = system_prompt
|
||||
return _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.dashboard.review_styles.get_repo_custom_prompt",
|
||||
new_callable=AsyncMock,
|
||||
return_value="Flag table rerender regressions.",
|
||||
),
|
||||
patch("agent.reviewer.make_model", return_value=MagicMock()),
|
||||
patch("agent.reviewer.create_deep_agent", side_effect=fake_create_deep_agent),
|
||||
):
|
||||
await reviewer.get_reviewer_agent(config)
|
||||
|
||||
assert "Repository-specific review style" in captured["system_prompt"]
|
||||
assert "Flag table rerender regressions" in captured["system_prompt"]
|
||||
|
|
|
|||
|
|
@ -47,12 +47,42 @@ def test_eval_target_passes_model_overrides(monkeypatch: pytest.MonkeyPatch) ->
|
|||
assert configurable["reviewer_reasoning_effort"] == "high"
|
||||
|
||||
|
||||
def _result_with_findings(findings: list[dict[str, Any]]) -> dict[str, Any]:
|
||||
return {"messages": [{"tool_calls": [{"name": "add_finding", "args": f} for f in findings]}]}
|
||||
|
||||
|
||||
def test_extract_comments_includes_all_confidences() -> None:
|
||||
result = _result_with_findings(
|
||||
[
|
||||
{
|
||||
"file": "a.py",
|
||||
"severity": "high",
|
||||
"confidence": "low",
|
||||
"description": "lo",
|
||||
"start_line": 1,
|
||||
"end_line": 1,
|
||||
},
|
||||
{
|
||||
"file": "b.py",
|
||||
"severity": "high",
|
||||
"confidence": "high",
|
||||
"description": "hi",
|
||||
"start_line": 2,
|
||||
"end_line": 2,
|
||||
},
|
||||
]
|
||||
)
|
||||
comments = target._extract_comments(result)
|
||||
assert {c["file"] for c in comments} == {"a.py", "b.py"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_extract_surfaced_comments_uses_publish_filter(
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
high = new_finding(
|
||||
severity="high",
|
||||
confidence="high",
|
||||
category="correctness",
|
||||
file="a.py",
|
||||
start_line=10,
|
||||
|
|
@ -63,6 +93,7 @@ async def test_extract_surfaced_comments_uses_publish_filter(
|
|||
)
|
||||
low = new_finding(
|
||||
severity="low",
|
||||
confidence="high",
|
||||
category="style",
|
||||
file="b.py",
|
||||
start_line=20,
|
||||
|
|
|
|||
|
|
@ -24,6 +24,7 @@ from agent.reviewer_findings import (
|
|||
def _f(**overrides: Any) -> Finding:
|
||||
base = new_finding(
|
||||
severity="high",
|
||||
confidence="high",
|
||||
category="correctness",
|
||||
file="foo.py",
|
||||
start_line=10,
|
||||
|
|
@ -53,8 +54,7 @@ def test_new_finding_defaults() -> None:
|
|||
|
||||
def test_severity_order_monotonic() -> None:
|
||||
assert (
|
||||
SEVERITY_ORDER["informational"]
|
||||
< SEVERITY_ORDER["low"]
|
||||
SEVERITY_ORDER["low"]
|
||||
< SEVERITY_ORDER["medium"]
|
||||
< SEVERITY_ORDER["high"]
|
||||
< SEVERITY_ORDER["critical"]
|
||||
|
|
@ -67,7 +67,6 @@ def test_filter_findings_for_publish_drops_below_threshold_and_resolved() -> Non
|
|||
_f(id="f_b", severity="low", file="b.py"),
|
||||
_f(id="f_c", severity="critical", file="c.py", start_line=2, end_line=2),
|
||||
_f(id="f_d", severity="high", file="d.py", status="resolved"),
|
||||
_f(id="f_e", severity="informational", file="e.py"),
|
||||
]
|
||||
surfaced = filter_findings_for_publish(findings, severity_threshold="medium", cap=10)
|
||||
assert [f["id"] for f in surfaced] == ["f_c", "f_a"]
|
||||
|
|
|
|||
|
|
@ -20,6 +20,7 @@ from agent.reviewer_publish import (
|
|||
def _f(**overrides: Any) -> Finding:
|
||||
base = new_finding(
|
||||
severity="high",
|
||||
confidence="high",
|
||||
category="correctness",
|
||||
file="src/foo.py",
|
||||
start_line=10,
|
||||
|
|
|
|||
|
|
@ -28,6 +28,7 @@ def test_add_finding_rejects_invalid_severity() -> None:
|
|||
with patch("agent.tools.add_finding.get_config", return_value=_config()):
|
||||
result = add_finding(
|
||||
severity="trivial",
|
||||
confidence="high",
|
||||
category="x",
|
||||
file="foo.py",
|
||||
description="d",
|
||||
|
|
@ -42,6 +43,7 @@ def test_add_finding_rejects_out_of_diff_lines() -> None:
|
|||
with patch("agent.tools.add_finding.get_config", return_value=_config()):
|
||||
result = add_finding(
|
||||
severity="high",
|
||||
confidence="high",
|
||||
category="correctness",
|
||||
file="foo.py",
|
||||
description="d",
|
||||
|
|
@ -52,6 +54,21 @@ def test_add_finding_rejects_out_of_diff_lines() -> None:
|
|||
assert "not part of the PR diff" in result["error"]
|
||||
|
||||
|
||||
def test_add_finding_rejects_invalid_confidence() -> None:
|
||||
with patch("agent.tools.add_finding.get_config", return_value=_config()):
|
||||
result = add_finding(
|
||||
severity="high",
|
||||
confidence="certain",
|
||||
category="correctness",
|
||||
file="foo.py",
|
||||
description="d",
|
||||
start_line=11,
|
||||
end_line=11,
|
||||
)
|
||||
assert result["success"] is False
|
||||
assert "confidence" in result["error"].lower()
|
||||
|
||||
|
||||
def test_add_finding_persists_to_thread_metadata() -> None:
|
||||
captured: list[Any] = []
|
||||
|
||||
|
|
@ -66,6 +83,7 @@ def test_add_finding_persists_to_thread_metadata() -> None:
|
|||
):
|
||||
result = add_finding(
|
||||
severity="medium",
|
||||
confidence="high",
|
||||
category="style",
|
||||
file="foo.py",
|
||||
description="rename",
|
||||
|
|
@ -84,6 +102,7 @@ def test_add_finding_persists_to_thread_metadata() -> None:
|
|||
assert persisted["suggestion"] == "renamed = 1"
|
||||
assert persisted["status"] == "open"
|
||||
assert persisted["first_seen_sha"] == "sha-head"
|
||||
assert persisted["confidence"] == "high"
|
||||
|
||||
|
||||
def test_add_finding_allows_file_level_with_no_lines() -> None:
|
||||
|
|
@ -98,6 +117,7 @@ def test_add_finding_allows_file_level_with_no_lines() -> None:
|
|||
):
|
||||
result = add_finding(
|
||||
severity="low",
|
||||
confidence="medium",
|
||||
category="style",
|
||||
file="missing.py",
|
||||
description="file-level note",
|
||||
|
|
@ -133,6 +153,7 @@ def test_add_finding_drops_long_suggestion() -> None:
|
|||
):
|
||||
result = add_finding(
|
||||
severity="medium",
|
||||
confidence="high",
|
||||
category="style",
|
||||
file="foo.py",
|
||||
description="rewrite",
|
||||
|
|
@ -162,6 +183,7 @@ def test_add_finding_keeps_short_suggestion() -> None:
|
|||
):
|
||||
result = add_finding(
|
||||
severity="medium",
|
||||
confidence="medium",
|
||||
category="style",
|
||||
file="foo.py",
|
||||
description="rename",
|
||||
|
|
@ -190,6 +212,7 @@ def test_add_finding_always_collapses_to_single_line() -> None:
|
|||
):
|
||||
result = add_finding(
|
||||
severity="low",
|
||||
confidence="low",
|
||||
category="style",
|
||||
file="foo.py",
|
||||
description="anchor on start_line",
|
||||
|
|
|
|||
|
|
@ -32,6 +32,13 @@ export function AppHeader({ user }: { user: SessionUser }) {
|
|||
>
|
||||
Profile
|
||||
</Link>
|
||||
<Link
|
||||
to="/review-styles"
|
||||
className="text-muted-foreground hover:text-foreground"
|
||||
activeProps={{ className: "text-foreground font-medium" }}
|
||||
>
|
||||
Review styles
|
||||
</Link>
|
||||
{user.is_admin && (
|
||||
<Link
|
||||
to="/admin"
|
||||
|
|
|
|||
|
|
@ -86,6 +86,26 @@ export interface ReposPayload {
|
|||
repositories: Array<Repository>;
|
||||
}
|
||||
|
||||
export type ReviewStyleStatus = "idle" | "running" | "completed" | "failed";
|
||||
|
||||
export interface ReviewStyle {
|
||||
full_name: string;
|
||||
owner?: string;
|
||||
name?: string;
|
||||
status: ReviewStyleStatus;
|
||||
custom_prompt: string | null;
|
||||
analysis_summary: string | null;
|
||||
top_reviewers: Array<string>;
|
||||
prs_sampled: number;
|
||||
reviews_sampled: number;
|
||||
analysis_thread_id: string | null;
|
||||
analysis_run_id: string | null;
|
||||
error: string | null;
|
||||
created_by?: string;
|
||||
created_at?: string;
|
||||
updated_at?: string;
|
||||
}
|
||||
|
||||
export const api = {
|
||||
me: () => request<SessionUser>("/me"),
|
||||
options: () => request<{ models: Array<ModelOption> }>("/options"),
|
||||
|
|
@ -93,6 +113,23 @@ export const api = {
|
|||
saveProfile: (body: ProfileUpdate) =>
|
||||
request<Profile>("/profile", { method: "PUT", body: JSON.stringify(body) }),
|
||||
repos: () => request<ReposPayload>("/repos"),
|
||||
listReviewStyles: () => request<Array<ReviewStyle>>("/review-styles"),
|
||||
createReviewStyle: (full_name: string) =>
|
||||
request<ReviewStyle>("/review-styles", {
|
||||
method: "POST",
|
||||
body: JSON.stringify({ full_name }),
|
||||
}),
|
||||
getReviewStyle: (full_name: string) =>
|
||||
request<ReviewStyle>(`/review-styles/${encodeURIComponent(full_name)}`),
|
||||
saveReviewStylePrompt: (full_name: string, custom_prompt: string) =>
|
||||
request<ReviewStyle>(`/review-styles/${encodeURIComponent(full_name)}`, {
|
||||
method: "PUT",
|
||||
body: JSON.stringify({ custom_prompt }),
|
||||
}),
|
||||
analyzeReviewStyle: (full_name: string) =>
|
||||
request<ReviewStyle>(`/review-styles/${encodeURIComponent(full_name)}/analyze`, {
|
||||
method: "POST",
|
||||
}),
|
||||
adminListProfiles: () => request<Array<Profile>>("/admin/profiles"),
|
||||
adminSaveProfile: (login: string, body: ProfileUpdate & { email?: string }) =>
|
||||
request<Profile>(`/admin/profiles/${encodeURIComponent(login)}`, {
|
||||
|
|
|
|||
19
ui/src/lib/repo.test.ts
Normal file
19
ui/src/lib/repo.test.ts
Normal file
|
|
@ -0,0 +1,19 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import { normalizeRepoFullName } from "./repo";
|
||||
|
||||
describe("normalizeRepoFullName", () => {
|
||||
it("accepts owner/repo", () => {
|
||||
expect(normalizeRepoFullName("langchain-ai/langgraph")).toBe("langchain-ai/langgraph");
|
||||
});
|
||||
|
||||
it("accepts github URLs", () => {
|
||||
expect(normalizeRepoFullName("https://github.com/withmartian/code-review-benchmark")).toBe(
|
||||
"withmartian/code-review-benchmark",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects invalid input", () => {
|
||||
expect(normalizeRepoFullName("not-a-repo")).toBeNull();
|
||||
});
|
||||
});
|
||||
13
ui/src/lib/repo.ts
Normal file
13
ui/src/lib/repo.ts
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
/** Normalize GitHub repo input to `owner/name`. */
|
||||
export function normalizeRepoFullName(raw: string): string | null {
|
||||
let v = raw.trim();
|
||||
for (const prefix of ["https://github.com/", "http://github.com/", "github.com/"]) {
|
||||
if (v.toLowerCase().startsWith(prefix)) {
|
||||
v = v.slice(prefix.length);
|
||||
}
|
||||
}
|
||||
v = v.replace(/\/$/, "").replace(/\.git$/, "");
|
||||
const parts = v.split("/").filter(Boolean);
|
||||
if (parts.length !== 2) return null;
|
||||
return `${parts[0]}/${parts[1]}`;
|
||||
}
|
||||
|
|
@ -9,11 +9,17 @@
|
|||
// Additionally, you should also exclude this file from your linter and/or formatter to prevent it from being checked or modified.
|
||||
|
||||
import { Route as rootRouteImport } from './routes/__root'
|
||||
import { Route as ReviewStylesRouteImport } from './routes/review-styles'
|
||||
import { Route as ProfileRouteImport } from './routes/profile'
|
||||
import { Route as LoginRouteImport } from './routes/login'
|
||||
import { Route as AdminRouteImport } from './routes/admin'
|
||||
import { Route as IndexRouteImport } from './routes/index'
|
||||
|
||||
const ReviewStylesRoute = ReviewStylesRouteImport.update({
|
||||
id: '/review-styles',
|
||||
path: '/review-styles',
|
||||
getParentRoute: () => rootRouteImport,
|
||||
} as any)
|
||||
const ProfileRoute = ProfileRouteImport.update({
|
||||
id: '/profile',
|
||||
path: '/profile',
|
||||
|
|
@ -40,12 +46,14 @@ export interface FileRoutesByFullPath {
|
|||
'/admin': typeof AdminRoute
|
||||
'/login': typeof LoginRoute
|
||||
'/profile': typeof ProfileRoute
|
||||
'/review-styles': typeof ReviewStylesRoute
|
||||
}
|
||||
export interface FileRoutesByTo {
|
||||
'/': typeof IndexRoute
|
||||
'/admin': typeof AdminRoute
|
||||
'/login': typeof LoginRoute
|
||||
'/profile': typeof ProfileRoute
|
||||
'/review-styles': typeof ReviewStylesRoute
|
||||
}
|
||||
export interface FileRoutesById {
|
||||
__root__: typeof rootRouteImport
|
||||
|
|
@ -53,13 +61,14 @@ export interface FileRoutesById {
|
|||
'/admin': typeof AdminRoute
|
||||
'/login': typeof LoginRoute
|
||||
'/profile': typeof ProfileRoute
|
||||
'/review-styles': typeof ReviewStylesRoute
|
||||
}
|
||||
export interface FileRouteTypes {
|
||||
fileRoutesByFullPath: FileRoutesByFullPath
|
||||
fullPaths: '/' | '/admin' | '/login' | '/profile'
|
||||
fullPaths: '/' | '/admin' | '/login' | '/profile' | '/review-styles'
|
||||
fileRoutesByTo: FileRoutesByTo
|
||||
to: '/' | '/admin' | '/login' | '/profile'
|
||||
id: '__root__' | '/' | '/admin' | '/login' | '/profile'
|
||||
to: '/' | '/admin' | '/login' | '/profile' | '/review-styles'
|
||||
id: '__root__' | '/' | '/admin' | '/login' | '/profile' | '/review-styles'
|
||||
fileRoutesById: FileRoutesById
|
||||
}
|
||||
export interface RootRouteChildren {
|
||||
|
|
@ -67,10 +76,18 @@ export interface RootRouteChildren {
|
|||
AdminRoute: typeof AdminRoute
|
||||
LoginRoute: typeof LoginRoute
|
||||
ProfileRoute: typeof ProfileRoute
|
||||
ReviewStylesRoute: typeof ReviewStylesRoute
|
||||
}
|
||||
|
||||
declare module '@tanstack/react-router' {
|
||||
interface FileRoutesByPath {
|
||||
'/review-styles': {
|
||||
id: '/review-styles'
|
||||
path: '/review-styles'
|
||||
fullPath: '/review-styles'
|
||||
preLoaderRoute: typeof ReviewStylesRouteImport
|
||||
parentRoute: typeof rootRouteImport
|
||||
}
|
||||
'/profile': {
|
||||
id: '/profile'
|
||||
path: '/profile'
|
||||
|
|
@ -107,6 +124,7 @@ const rootRouteChildren: RootRouteChildren = {
|
|||
AdminRoute: AdminRoute,
|
||||
LoginRoute: LoginRoute,
|
||||
ProfileRoute: ProfileRoute,
|
||||
ReviewStylesRoute: ReviewStylesRoute,
|
||||
}
|
||||
export const routeTree = rootRouteImport
|
||||
._addFileChildren(rootRouteChildren)
|
||||
|
|
|
|||
299
ui/src/routes/review-styles.tsx
Normal file
299
ui/src/routes/review-styles.tsx
Normal file
|
|
@ -0,0 +1,299 @@
|
|||
import { Navigate, createFileRoute } from "@tanstack/react-router";
|
||||
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
|
||||
import { useEffect, useState } from "react";
|
||||
|
||||
import { AppHeader } from "@/components/AppHeader";
|
||||
import { Badge } from "@/components/ui/badge";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
|
||||
import {
|
||||
Combobox,
|
||||
ComboboxContent,
|
||||
ComboboxEmpty,
|
||||
ComboboxInput,
|
||||
ComboboxItem,
|
||||
ComboboxList,
|
||||
} from "@/components/ui/combobox";
|
||||
import { Input } from "@/components/ui/input";
|
||||
import { Label } from "@/components/ui/label";
|
||||
import { Skeleton } from "@/components/ui/skeleton";
|
||||
import { Textarea } from "@/components/ui/textarea";
|
||||
import { ApiError, api, type ReviewStyle } from "@/lib/api";
|
||||
import { normalizeRepoFullName } from "@/lib/repo";
|
||||
import { useSession } from "@/lib/session";
|
||||
|
||||
export const Route = createFileRoute("/review-styles")({ component: ReviewStylesPage });
|
||||
|
||||
function statusVariant(status: ReviewStyle["status"]) {
|
||||
switch (status) {
|
||||
case "completed":
|
||||
return "default" as const;
|
||||
case "running":
|
||||
return "secondary" as const;
|
||||
case "failed":
|
||||
return "destructive" as const;
|
||||
default:
|
||||
return "outline" as const;
|
||||
}
|
||||
}
|
||||
|
||||
function ReviewStylesPage() {
|
||||
const session = useSession();
|
||||
const qc = useQueryClient();
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
const [addRepo, setAddRepo] = useState("");
|
||||
const [selected, setSelected] = useState<string | null>(null);
|
||||
const [draftPrompt, setDraftPrompt] = useState("");
|
||||
|
||||
const styles = useQuery({
|
||||
queryKey: ["reviewStyles"],
|
||||
queryFn: api.listReviewStyles,
|
||||
enabled: !!session.data,
|
||||
refetchInterval: (q) => {
|
||||
const hasRunning = (q.state.data ?? []).some((r) => r.status === "running");
|
||||
return hasRunning ? 4000 : false;
|
||||
},
|
||||
});
|
||||
|
||||
const repos = useQuery({
|
||||
queryKey: ["repos"],
|
||||
queryFn: async () => {
|
||||
try {
|
||||
return await api.repos();
|
||||
} catch (e) {
|
||||
if (e instanceof ApiError && e.status === 401) return { installations: [], repositories: [] };
|
||||
throw e;
|
||||
}
|
||||
},
|
||||
enabled: !!session.data,
|
||||
});
|
||||
|
||||
const detail = useQuery({
|
||||
queryKey: ["reviewStyle", selected],
|
||||
queryFn: () => api.getReviewStyle(selected!),
|
||||
enabled: !!selected,
|
||||
refetchInterval: (q) => (q.state.data?.status === "running" ? 4000 : false),
|
||||
});
|
||||
|
||||
useEffect(() => {
|
||||
if (detail.data?.custom_prompt != null) {
|
||||
setDraftPrompt(detail.data.custom_prompt);
|
||||
} else if (detail.data && !detail.data.custom_prompt) {
|
||||
setDraftPrompt("");
|
||||
}
|
||||
}, [detail.data?.custom_prompt, detail.data?.full_name]);
|
||||
|
||||
const createStyle = useMutation({
|
||||
mutationFn: (full_name: string) => api.createReviewStyle(full_name),
|
||||
onSuccess: (record) => {
|
||||
void qc.invalidateQueries({ queryKey: ["reviewStyles"] });
|
||||
setSelected(record.full_name);
|
||||
setError(null);
|
||||
},
|
||||
onError: (e: Error) => setError(e.message),
|
||||
});
|
||||
|
||||
const analyze = useMutation({
|
||||
mutationFn: (full_name: string) => api.analyzeReviewStyle(full_name),
|
||||
onSuccess: () => {
|
||||
void qc.invalidateQueries({ queryKey: ["reviewStyles"] });
|
||||
void qc.invalidateQueries({ queryKey: ["reviewStyle", selected] });
|
||||
setError(null);
|
||||
},
|
||||
onError: (e: Error) => setError(e.message),
|
||||
});
|
||||
|
||||
const savePrompt = useMutation({
|
||||
mutationFn: ({ full_name, custom_prompt }: { full_name: string; custom_prompt: string }) =>
|
||||
api.saveReviewStylePrompt(full_name, custom_prompt),
|
||||
onSuccess: () => {
|
||||
void qc.invalidateQueries({ queryKey: ["reviewStyles"] });
|
||||
void qc.invalidateQueries({ queryKey: ["reviewStyle", selected] });
|
||||
setError(null);
|
||||
},
|
||||
onError: (e: Error) => setError(e.message),
|
||||
});
|
||||
|
||||
if (session.isLoading) {
|
||||
return (
|
||||
<main className="container mx-auto p-6">
|
||||
<Skeleton className="h-64 w-full" />
|
||||
</main>
|
||||
);
|
||||
}
|
||||
|
||||
if (!session.data) return <Navigate to="/login" />;
|
||||
|
||||
const configured = new Set((styles.data ?? []).map((s) => s.full_name));
|
||||
const suggestedRepos = (repos.data?.repositories ?? []).filter((r) => !configured.has(r.full_name));
|
||||
const normalizedAddRepo = normalizeRepoFullName(addRepo);
|
||||
const canAdd = normalizedAddRepo !== null && !configured.has(normalizedAddRepo);
|
||||
const active = detail.data ?? styles.data?.find((s) => s.full_name === selected) ?? null;
|
||||
|
||||
const handleAdd = () => {
|
||||
if (!normalizedAddRepo || !canAdd) return;
|
||||
void createStyle.mutateAsync(normalizedAddRepo).then(() => setAddRepo(""));
|
||||
};
|
||||
|
||||
return (
|
||||
<div className="min-h-svh">
|
||||
<AppHeader user={session.data} />
|
||||
<main className="container mx-auto grid grid-cols-1 gap-6 p-6 md:grid-cols-[300px_1fr]">
|
||||
<Card>
|
||||
<CardHeader>
|
||||
<CardTitle>Repositories</CardTitle>
|
||||
<CardDescription>
|
||||
An agent browses recent merged PR review feedback on GitHub, then writes a
|
||||
per-repo style guide for the reviewer.
|
||||
</CardDescription>
|
||||
</CardHeader>
|
||||
<CardContent className="space-y-4">
|
||||
<div className="space-y-2">
|
||||
<Label htmlFor="add-repo">Add repository</Label>
|
||||
<Input
|
||||
id="add-repo"
|
||||
className="w-full"
|
||||
placeholder="owner/repo or https://github.com/owner/repo"
|
||||
value={addRepo}
|
||||
onChange={(e) => setAddRepo(e.target.value)}
|
||||
onKeyDown={(e) => {
|
||||
if (e.key === "Enter") {
|
||||
e.preventDefault();
|
||||
handleAdd();
|
||||
}
|
||||
}}
|
||||
/>
|
||||
<p className="text-muted-foreground text-xs">
|
||||
Any public repository is supported. Private repos require your GitHub account to
|
||||
have read access.
|
||||
</p>
|
||||
{suggestedRepos.length > 0 && (
|
||||
<div className="space-y-1.5">
|
||||
<p className="text-muted-foreground text-xs">Your installations</p>
|
||||
<Combobox
|
||||
items={suggestedRepos.map((r) => r.full_name)}
|
||||
value={addRepo}
|
||||
onValueChange={(v) => setAddRepo(typeof v === "string" ? v : "")}
|
||||
>
|
||||
<ComboboxInput placeholder="Search installed repos…" showClear className="w-full" />
|
||||
<ComboboxContent className="min-w-[var(--anchor-width)]">
|
||||
<ComboboxList className="max-h-48">
|
||||
<ComboboxEmpty>No matches</ComboboxEmpty>
|
||||
{suggestedRepos.map((r) => (
|
||||
<ComboboxItem key={r.full_name} value={r.full_name}>
|
||||
<span className="truncate">{r.full_name}</span>
|
||||
{r.private && (
|
||||
<span className="text-muted-foreground ml-auto text-[10px]">
|
||||
private
|
||||
</span>
|
||||
)}
|
||||
</ComboboxItem>
|
||||
))}
|
||||
</ComboboxList>
|
||||
</ComboboxContent>
|
||||
</Combobox>
|
||||
</div>
|
||||
)}
|
||||
{addRepo.trim() && !normalizedAddRepo && (
|
||||
<p className="text-destructive text-xs">Enter a repository as owner/repo</p>
|
||||
)}
|
||||
{normalizedAddRepo && configured.has(normalizedAddRepo) && (
|
||||
<p className="text-muted-foreground text-xs">Already added</p>
|
||||
)}
|
||||
<Button className="w-full" disabled={!canAdd || createStyle.isPending} onClick={handleAdd}>
|
||||
Add
|
||||
</Button>
|
||||
</div>
|
||||
<ul className="space-y-1">
|
||||
{(styles.data ?? []).map((s) => (
|
||||
<li key={s.full_name}>
|
||||
<button
|
||||
type="button"
|
||||
className={`flex w-full items-center justify-between rounded-md px-2 py-1.5 text-left text-sm hover:bg-muted ${
|
||||
selected === s.full_name ? "bg-muted font-medium" : ""
|
||||
}`}
|
||||
onClick={() => setSelected(s.full_name)}
|
||||
>
|
||||
<span className="truncate">{s.full_name}</span>
|
||||
<Badge variant={statusVariant(s.status)} className="ml-2 shrink-0">
|
||||
{s.status}
|
||||
</Badge>
|
||||
</button>
|
||||
</li>
|
||||
))}
|
||||
</ul>
|
||||
</CardContent>
|
||||
</Card>
|
||||
|
||||
<Card>
|
||||
<CardHeader>
|
||||
<CardTitle>Review style prompt</CardTitle>
|
||||
<CardDescription>
|
||||
Injected into the reviewer agent for PRs on this repo. Edit after analysis or write
|
||||
your own.
|
||||
</CardDescription>
|
||||
</CardHeader>
|
||||
<CardContent className="space-y-4">
|
||||
{!selected || !active ? (
|
||||
<p className="text-muted-foreground text-sm">Select a repository to view or edit.</p>
|
||||
) : (
|
||||
<>
|
||||
<div className="flex flex-wrap items-center gap-2 text-sm">
|
||||
<Badge variant={statusVariant(active.status)}>{active.status}</Badge>
|
||||
{active.top_reviewers.length > 0 && (
|
||||
<span className="text-muted-foreground">
|
||||
Reviewers: {active.top_reviewers.join(", ")}
|
||||
</span>
|
||||
)}
|
||||
{active.prs_sampled > 0 && (
|
||||
<span className="text-muted-foreground">
|
||||
{active.prs_sampled} PRs · {active.reviews_sampled} reviews sampled
|
||||
</span>
|
||||
)}
|
||||
</div>
|
||||
{active.analysis_summary && (
|
||||
<p className="text-muted-foreground text-sm">{active.analysis_summary}</p>
|
||||
)}
|
||||
{active.error && (
|
||||
<p className="text-destructive text-sm">{active.error}</p>
|
||||
)}
|
||||
<div className="flex gap-2">
|
||||
<Button
|
||||
variant="secondary"
|
||||
disabled={active.status === "running" || analyze.isPending}
|
||||
onClick={() => void analyze.mutateAsync(active.full_name)}
|
||||
>
|
||||
{active.status === "running" ? "Analyzing…" : "Run analysis"}
|
||||
</Button>
|
||||
<Button
|
||||
disabled={!draftPrompt.trim() || savePrompt.isPending}
|
||||
onClick={() =>
|
||||
void savePrompt.mutateAsync({
|
||||
full_name: active.full_name,
|
||||
custom_prompt: draftPrompt,
|
||||
})
|
||||
}
|
||||
>
|
||||
Save prompt
|
||||
</Button>
|
||||
</div>
|
||||
<Textarea
|
||||
className="min-h-[360px] font-mono text-sm"
|
||||
value={draftPrompt}
|
||||
onChange={(e) => setDraftPrompt(e.target.value)}
|
||||
placeholder={
|
||||
active.status === "running"
|
||||
? "Analysis in progress…"
|
||||
: "Run analysis or write a custom prompt for this repository."
|
||||
}
|
||||
disabled={active.status === "running"}
|
||||
/>
|
||||
</>
|
||||
)}
|
||||
{error && <p className="text-destructive text-sm">{error}</p>}
|
||||
</CardContent>
|
||||
</Card>
|
||||
</main>
|
||||
</div>
|
||||
);
|
||||
}
|
||||
2
uv.lock
generated
2
uv.lock
generated
|
|
@ -1853,6 +1853,7 @@ dependencies = [
|
|||
{ name = "langchain" },
|
||||
{ name = "langchain-anthropic" },
|
||||
{ name = "langchain-daytona" },
|
||||
{ name = "langchain-google-genai" },
|
||||
{ name = "langchain-modal" },
|
||||
{ name = "langchain-openai" },
|
||||
{ name = "langchain-runloop" },
|
||||
|
|
@ -1883,6 +1884,7 @@ requires-dist = [
|
|||
{ name = "langchain", specifier = ">=1.2.17" },
|
||||
{ name = "langchain-anthropic", specifier = ">=1.4.2" },
|
||||
{ name = "langchain-daytona", specifier = ">=0.0.5" },
|
||||
{ name = "langchain-google-genai", specifier = ">=4.2.2" },
|
||||
{ name = "langchain-modal", specifier = ">=0.0.3" },
|
||||
{ name = "langchain-openai", specifier = ">=1.2.1" },
|
||||
{ name = "langchain-runloop", specifier = ">=0.0.4" },
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue