mirror of
https://github.com/Sea-Haven-Industries/open-swe.git
synced 2026-09-30 09:13:14 +00:00
feat: inject org-wide guidelines into reviewer prompt (#1431)
Adds an admin-managed, org-wide review guidelines field to team settings that the reviewer injects into every PR review across all repos, alongside the existing per-repo style prompt and AGENTS.md context. Repo-specific rules take precedence when they conflict. Co-authored-by: open-swe[bot] <open-swe@users.noreply.github.com>
This commit is contained in:
parent
449cb5d1a8
commit
f3512841fc
6 changed files with 237 additions and 3 deletions
|
|
@ -13,7 +13,7 @@ from datetime import UTC, datetime
|
|||
from typing import Any, Literal
|
||||
|
||||
from langgraph_sdk import get_client
|
||||
from pydantic import BaseModel, model_validator
|
||||
from pydantic import BaseModel, field_validator, model_validator
|
||||
|
||||
from .options import (
|
||||
SUPPORTED_MODEL_IDS,
|
||||
|
|
@ -30,6 +30,10 @@ TEAM_SETTINGS_KEY = "default"
|
|||
TriggerMode = Literal["every_push", "once_per_pr", "manual"]
|
||||
AutofixMode = Literal["off", "low", "medium", "high"]
|
||||
|
||||
# Cap the org-wide guidelines so a runaway value can't dominate the reviewer
|
||||
# prompt. Generous enough for a detailed policy, small enough to stay bounded.
|
||||
ORG_GUIDELINES_MAX_CHARS = 10_000
|
||||
|
||||
|
||||
class TeamSettingsUpdate(BaseModel):
|
||||
trigger_mode: TriggerMode = "every_push"
|
||||
|
|
@ -38,6 +42,7 @@ class TeamSettingsUpdate(BaseModel):
|
|||
review_trace_links: bool = True
|
||||
autofix_mode: AutofixMode = "off"
|
||||
autofix_severity_threshold: AutofixMode = "medium"
|
||||
org_guidelines: str | None = None
|
||||
default_agent_model: str | None = None
|
||||
default_agent_reasoning_effort: str | None = None
|
||||
default_agent_subagent_model: str | None = None
|
||||
|
|
@ -48,6 +53,22 @@ class TeamSettingsUpdate(BaseModel):
|
|||
default_reviewer_subagent_model: str | None = None
|
||||
default_reviewer_subagent_reasoning_effort: str | None = None
|
||||
|
||||
@field_validator("org_guidelines", mode="before")
|
||||
@classmethod
|
||||
def _normalize_org_guidelines(cls, v: object) -> str | None:
|
||||
if v is None:
|
||||
return None
|
||||
if not isinstance(v, str):
|
||||
raise ValueError("org_guidelines must be a string")
|
||||
text = v.strip()
|
||||
if not text:
|
||||
return None
|
||||
if len(text) > ORG_GUIDELINES_MAX_CHARS:
|
||||
raise ValueError(
|
||||
f"org_guidelines must be at most {ORG_GUIDELINES_MAX_CHARS} characters"
|
||||
)
|
||||
return text
|
||||
|
||||
@model_validator(mode="after")
|
||||
def _validate_model_pairs(self) -> TeamSettingsUpdate:
|
||||
_validate_model_effort_pair(
|
||||
|
|
@ -108,6 +129,7 @@ def _default_settings() -> dict[str, Any]:
|
|||
"review_trace_links": True,
|
||||
"autofix_mode": "off",
|
||||
"autofix_severity_threshold": "medium",
|
||||
"org_guidelines": None,
|
||||
"default_agent_model": fallback_model,
|
||||
"default_agent_reasoning_effort": fallback_effort,
|
||||
"default_agent_subagent_model": fallback_model,
|
||||
|
|
@ -152,6 +174,7 @@ async def upsert_team_settings(update: TeamSettingsUpdate) -> dict[str, Any]:
|
|||
"review_trace_links": update.review_trace_links,
|
||||
"autofix_mode": update.autofix_mode,
|
||||
"autofix_severity_threshold": update.autofix_severity_threshold,
|
||||
"org_guidelines": update.org_guidelines,
|
||||
"default_agent_model": update.default_agent_model,
|
||||
"default_agent_reasoning_effort": update.default_agent_reasoning_effort,
|
||||
"default_agent_subagent_model": update.default_agent_subagent_model,
|
||||
|
|
@ -225,6 +248,15 @@ async def get_team_review_trace_links_enabled() -> bool:
|
|||
return bool(settings.get("review_trace_links", True))
|
||||
|
||||
|
||||
async def get_org_review_guidelines() -> str | None:
|
||||
"""Return the org-wide reviewer guidelines supplement, if configured."""
|
||||
settings = await get_team_settings()
|
||||
value = settings.get("org_guidelines")
|
||||
if isinstance(value, str) and value.strip():
|
||||
return value.strip()
|
||||
return None
|
||||
|
||||
|
||||
async def get_team_default_subagent_model(
|
||||
role: Literal["agent", "reviewer"],
|
||||
) -> tuple[str, str]:
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ warnings.filterwarnings("ignore", message=".*Pydantic V1.*", category=UserWarnin
|
|||
from deepagents import create_deep_agent
|
||||
from langchain.agents.middleware import ModelCallLimitMiddleware
|
||||
|
||||
from .dashboard.team_settings import get_team_default_model_pair
|
||||
from .dashboard.team_settings import get_org_review_guidelines, get_team_default_model_pair
|
||||
from .middleware import (
|
||||
SanitizeThinkingBlocksMiddleware,
|
||||
SanitizeToolInputsMiddleware,
|
||||
|
|
@ -275,6 +275,7 @@ def _reviewer_system_prompt(
|
|||
repo_name: str,
|
||||
pr_number: int | str,
|
||||
reviewer_eval: bool = False,
|
||||
org_guidelines: str | None = None,
|
||||
repo_style_prompt: str | None = None,
|
||||
agents_md_content: str | None = None,
|
||||
) -> str:
|
||||
|
|
@ -286,6 +287,17 @@ def _reviewer_system_prompt(
|
|||
)
|
||||
if reviewer_eval:
|
||||
prompt = f"{prompt}\n{REVIEWER_EVAL_PROMPT_SUFFIX}"
|
||||
if org_guidelines:
|
||||
prompt = (
|
||||
f"{prompt}\n\n"
|
||||
"# Organization-wide review guidelines\n\n"
|
||||
"These guidelines were set by a workspace admin and apply to every "
|
||||
"repository this reviewer covers. Apply them when they agree with the "
|
||||
"global bar above; they refine tone, severity, and what this "
|
||||
"organization typically flags. Repository-specific rules below take "
|
||||
"precedence when they conflict.\n\n"
|
||||
f"{org_guidelines}"
|
||||
)
|
||||
if repo_style_prompt:
|
||||
prompt = (
|
||||
f"{prompt}\n\n"
|
||||
|
|
@ -743,6 +755,13 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
|
|||
|
||||
return await get_repo_custom_prompt(repo_owner, repo_name)
|
||||
|
||||
async def _fetch_org_guidelines() -> str | None:
|
||||
try:
|
||||
return await get_org_review_guidelines()
|
||||
except Exception: # noqa: BLE001
|
||||
logger.exception("Failed to load org-wide review guidelines; continuing without them")
|
||||
return None
|
||||
|
||||
async def _fetch_agents_md_context() -> str | None:
|
||||
if not repo_owner or not repo_name or not base_sha:
|
||||
return None
|
||||
|
|
@ -768,12 +787,14 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
|
|||
existing_threads_block,
|
||||
repo_style_prompt,
|
||||
agents_md_content,
|
||||
org_guidelines,
|
||||
) = await asyncio.gather(
|
||||
_fetch_diff_context(),
|
||||
_fetch_pr_overview(),
|
||||
_fetch_existing_threads_block(),
|
||||
_fetch_repo_style_prompt(),
|
||||
_fetch_agents_md_context(),
|
||||
_fetch_org_guidelines(),
|
||||
)
|
||||
pr_diff_text, pr_diff_line_set = diff_context
|
||||
pr_title, pr_body = pr_overview
|
||||
|
|
@ -879,6 +900,7 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel:
|
|||
repo_name=repo_name,
|
||||
pr_number=pr_number if isinstance(pr_number, int) else "",
|
||||
reviewer_eval=reviewer_eval,
|
||||
org_guidelines=org_guidelines,
|
||||
repo_style_prompt=repo_style_prompt,
|
||||
agents_md_content=agents_md_content,
|
||||
)
|
||||
|
|
|
|||
|
|
@ -36,6 +36,32 @@ def test_reviewer_system_prompt_includes_repo_style_section() -> None:
|
|||
assert "missing tests for API" in prompt
|
||||
|
||||
|
||||
def test_reviewer_system_prompt_includes_org_guidelines_section() -> None:
|
||||
prompt = reviewer._reviewer_system_prompt(
|
||||
"/workspace/repo",
|
||||
repo_owner="acme",
|
||||
repo_name="repo",
|
||||
pr_number=42,
|
||||
org_guidelines="Flag any new endpoint that lacks input validation.",
|
||||
)
|
||||
assert "Organization-wide review guidelines" in prompt
|
||||
assert "lacks input validation" in prompt
|
||||
|
||||
|
||||
def test_reviewer_system_prompt_org_guidelines_precede_repo_style() -> None:
|
||||
prompt = reviewer._reviewer_system_prompt(
|
||||
"/workspace/repo",
|
||||
repo_owner="acme",
|
||||
repo_name="repo",
|
||||
pr_number=42,
|
||||
org_guidelines="Org rule text.",
|
||||
repo_style_prompt="Repo rule text.",
|
||||
)
|
||||
assert prompt.index("Organization-wide review guidelines") < prompt.index(
|
||||
"Repository-specific review style"
|
||||
)
|
||||
|
||||
|
||||
def test_finding_reply_context_wraps_reply_as_untrusted_data() -> None:
|
||||
prompt = reviewer._build_finding_reply_context(
|
||||
pr_url="https://github.com/acme/repo/pull/1",
|
||||
|
|
@ -338,6 +364,67 @@ async def test_reviewer_injects_repo_style_during_eval() -> None:
|
|||
assert "Flag table rerender regressions" in captured["system_prompt"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reviewer_inlines_org_guidelines_into_system_prompt() -> None:
|
||||
config: RunnableConfig = {
|
||||
"configurable": {
|
||||
"__is_for_execution__": True,
|
||||
"thread_id": "reviewer-thread-id",
|
||||
"source": "github",
|
||||
"repo": {"owner": "acme", "name": "repo"},
|
||||
"pr_number": 9,
|
||||
"pr_url": "https://github.com/acme/repo/pull/9",
|
||||
"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.get_github_app_installation_token_with_expiry",
|
||||
new_callable=AsyncMock,
|
||||
return_value=("gh-token", None),
|
||||
),
|
||||
patch(
|
||||
"agent.reviewer.ensure_sandbox_for_thread",
|
||||
new_callable=AsyncMock,
|
||||
return_value=MagicMock(),
|
||||
),
|
||||
patch(
|
||||
"agent.reviewer.aresolve_sandbox_work_dir",
|
||||
new_callable=AsyncMock,
|
||||
return_value="/workspace",
|
||||
),
|
||||
patch(
|
||||
"agent.reviewer.get_org_review_guidelines",
|
||||
new_callable=AsyncMock,
|
||||
return_value="Never approve a PR that disables a CI gate.",
|
||||
),
|
||||
patch(
|
||||
"agent.dashboard.review_styles.get_repo_custom_prompt",
|
||||
new_callable=AsyncMock,
|
||||
return_value=None,
|
||||
),
|
||||
patch(
|
||||
"agent.reviewer.fetch_agents_md",
|
||||
new_callable=AsyncMock,
|
||||
return_value=None,
|
||||
),
|
||||
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 "Organization-wide review guidelines" in captured["system_prompt"]
|
||||
assert "disables a CI gate" in captured["system_prompt"]
|
||||
|
||||
|
||||
def test_reviewer_system_prompt_includes_agents_md_section() -> None:
|
||||
prompt = reviewer._reviewer_system_prompt(
|
||||
"/workspace/repo",
|
||||
|
|
|
|||
47
tests/test_team_settings_org_guidelines.py
Normal file
47
tests/test_team_settings_org_guidelines.py
Normal file
|
|
@ -0,0 +1,47 @@
|
|||
from __future__ import annotations
|
||||
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from agent.dashboard.team_settings import (
|
||||
ORG_GUIDELINES_MAX_CHARS,
|
||||
TeamSettingsUpdate,
|
||||
get_org_review_guidelines,
|
||||
)
|
||||
|
||||
|
||||
def test_org_guidelines_blank_normalizes_to_none() -> None:
|
||||
assert TeamSettingsUpdate(org_guidelines=" ").org_guidelines is None
|
||||
assert TeamSettingsUpdate(org_guidelines=None).org_guidelines is None
|
||||
|
||||
|
||||
def test_org_guidelines_trimmed() -> None:
|
||||
update = TeamSettingsUpdate(org_guidelines=" Flag CI gate removals.\n")
|
||||
assert update.org_guidelines == "Flag CI gate removals."
|
||||
|
||||
|
||||
def test_org_guidelines_rejects_oversized() -> None:
|
||||
with pytest.raises(ValidationError):
|
||||
TeamSettingsUpdate(org_guidelines="x" * (ORG_GUIDELINES_MAX_CHARS + 1))
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_org_review_guidelines_returns_trimmed_text() -> None:
|
||||
with patch(
|
||||
"agent.dashboard.team_settings.get_team_settings",
|
||||
new_callable=AsyncMock,
|
||||
return_value={"org_guidelines": " Always check auth.\n"},
|
||||
):
|
||||
assert await get_org_review_guidelines() == "Always check auth."
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_org_review_guidelines_returns_none_when_unset() -> None:
|
||||
with patch(
|
||||
"agent.dashboard.team_settings.get_team_settings",
|
||||
new_callable=AsyncMock,
|
||||
return_value={"org_guidelines": None},
|
||||
):
|
||||
assert await get_org_review_guidelines() is None
|
||||
|
|
@ -109,6 +109,7 @@ export interface TeamSettings {
|
|||
review_trace_links: boolean;
|
||||
autofix_mode: AutofixMode;
|
||||
autofix_severity_threshold: AutofixMode;
|
||||
org_guidelines?: string | null;
|
||||
default_agent_model?: string | null;
|
||||
default_agent_reasoning_effort?: string | null;
|
||||
default_agent_subagent_model?: string | null;
|
||||
|
|
|
|||
|
|
@ -13,8 +13,10 @@ import {
|
|||
SelectTrigger,
|
||||
SelectValue,
|
||||
} from "@/components/ui/select";
|
||||
import { Button } from "@/components/ui/button";
|
||||
import { Skeleton } from "@/components/ui/skeleton";
|
||||
import { Switch } from "@/components/ui/switch";
|
||||
import { Textarea } from "@/components/ui/textarea";
|
||||
import { ApiError, api } from "@/lib/api";
|
||||
import { useSession } from "@/lib/session";
|
||||
|
||||
|
|
@ -52,6 +54,7 @@ const DEFAULT_SETTINGS: TeamSettings = {
|
|||
review_trace_links: true,
|
||||
autofix_mode: "off",
|
||||
autofix_severity_threshold: "medium",
|
||||
org_guidelines: null,
|
||||
default_agent_model: null,
|
||||
default_agent_reasoning_effort: null,
|
||||
default_agent_subagent_model: null,
|
||||
|
|
@ -71,10 +74,14 @@ function ReviewPage() {
|
|||
enabled: !!session.data,
|
||||
});
|
||||
const [local, setLocal] = useState<TeamSettings>(DEFAULT_SETTINGS);
|
||||
const [guidelinesDraft, setGuidelinesDraft] = useState("");
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
if (settings.data) setLocal(settings.data);
|
||||
if (settings.data) {
|
||||
setLocal(settings.data);
|
||||
setGuidelinesDraft(settings.data.org_guidelines ?? "");
|
||||
}
|
||||
}, [settings.data]);
|
||||
|
||||
const save = useMutation({
|
||||
|
|
@ -108,6 +115,15 @@ function ReviewPage() {
|
|||
TRIGGER_MODES.find((m) => m.value === current.trigger_mode)?.description ??
|
||||
"Open SWE Review will automatically review every push to a PR";
|
||||
|
||||
const trimmedGuidelines = guidelinesDraft.trim();
|
||||
const savedGuidelines = current.org_guidelines ?? "";
|
||||
const guidelinesDirty = trimmedGuidelines !== savedGuidelines.trim();
|
||||
|
||||
const saveGuidelines = () => {
|
||||
if (!canEdit) return;
|
||||
persist({ org_guidelines: trimmedGuidelines || null });
|
||||
};
|
||||
|
||||
return (
|
||||
<AppShell
|
||||
user={session.data}
|
||||
|
|
@ -131,6 +147,35 @@ function ReviewPage() {
|
|||
</Link>
|
||||
</SettingsSection>
|
||||
|
||||
<SettingsSection
|
||||
title="Organization Guidelines"
|
||||
description="Org-wide instructions injected into every review, across all repositories. Repository-specific style prompts take precedence when they conflict."
|
||||
>
|
||||
<div className="flex flex-col gap-2 p-4">
|
||||
<Textarea
|
||||
className="min-h-[200px] w-full font-mono text-xs"
|
||||
value={guidelinesDraft}
|
||||
onChange={(e) => setGuidelinesDraft(e.target.value)}
|
||||
placeholder="e.g. Always flag missing input validation on new API endpoints. Prefer structured logging over print statements."
|
||||
disabled={!canEdit}
|
||||
/>
|
||||
{canEdit && (
|
||||
<div className="flex items-center gap-2">
|
||||
<Button
|
||||
size="sm"
|
||||
disabled={!guidelinesDirty || save.isPending}
|
||||
onClick={saveGuidelines}
|
||||
>
|
||||
Save guidelines
|
||||
</Button>
|
||||
{guidelinesDirty && (
|
||||
<span className="text-xs text-muted-foreground">Unsaved changes</span>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
</SettingsSection>
|
||||
|
||||
<SettingsSection title="Configuration">
|
||||
<div className="divide-y divide-border">
|
||||
<SettingsRow
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue