diff --git a/agent/reviewer.py b/agent/reviewer.py index 35efd79e..2730ac35 100644 --- a/agent/reviewer.py +++ b/agent/reviewer.py @@ -65,6 +65,7 @@ from .tools import ( web_search, ) from .utils.agents_md import fetch_agents_md +from .utils.api_standards_skill import fetch_api_standards_skill from .utils.github_app import get_github_app_installation_token_with_expiry from .utils.github_token import cache_github_token_for_thread from .utils.model import DEFAULT_LLM_REASONING, make_model, provider_model_kwargs @@ -278,6 +279,7 @@ def _reviewer_system_prompt( org_guidelines: str | None = None, repo_style_prompt: str | None = None, agents_md_content: str | None = None, + api_standards_skill: str | None = None, ) -> str: prompt = REVIEWER_PROMPT_TEMPLATE.format( working_dir=working_dir, @@ -322,6 +324,21 @@ def _reviewer_system_prompt( f"{agents_md_content}\n" "```" ) + if api_standards_skill: + prompt = ( + f"{prompt}\n\n" + "# API standards skill\n\n" + "Apply this skill ONLY when the PR introduces a new API or modifies " + "an existing one (HTTP routes/handlers, RPC or GraphQL endpoints, " + "public SDK/library signatures, request/response schemas, status " + "codes, headers, or other API contracts). When the diff touches such " + "surfaces, verify the change against the best practices below and " + "file a finding when a changed line violates them and clears the " + "global bar above (anchored, concrete failure mode, in-diff). If the " + "PR does not change any API, ignore this section. Do not file " + "style-only nits or pre-existing violations outside the diff.\n\n" + f"{api_standards_skill}" + ) return prompt @@ -788,6 +805,7 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: repo_style_prompt, agents_md_content, org_guidelines, + api_standards_skill, ) = await asyncio.gather( _fetch_diff_context(), _fetch_pr_overview(), @@ -795,6 +813,7 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: _fetch_repo_style_prompt(), _fetch_agents_md_context(), _fetch_org_guidelines(), + fetch_api_standards_skill(), ) pr_diff_text, pr_diff_line_set = diff_context pr_title, pr_body = pr_overview @@ -903,6 +922,7 @@ async def get_reviewer_agent(config: RunnableConfig) -> Pregel: org_guidelines=org_guidelines, repo_style_prompt=repo_style_prompt, agents_md_content=agents_md_content, + api_standards_skill=api_standards_skill, ) if review_context: system_prompt = f"{system_prompt}\n\n{review_context}" diff --git a/agent/utils/api_standards_skill.py b/agent/utils/api_standards_skill.py new file mode 100644 index 00000000..780e009b --- /dev/null +++ b/agent/utils/api_standards_skill.py @@ -0,0 +1,52 @@ +"""Fetch the API-standards skill from the LangSmith Context Hub. + +The reviewer applies this skill when a PR adds or changes APIs, so it can +verify the changes against the team's API best practices. The skill lives in +the Context Hub as a skill repo (default handle ``api-standards``); we pull its +``SKILL.md`` at run start and inject it into the reviewer's system prompt. + +Best-effort: any failure (missing handle, no API key, SDK error) returns +``None`` and the reviewer runs without the supplement. +""" + +from __future__ import annotations + +import asyncio +import logging +import os + +logger = logging.getLogger(__name__) + +API_STANDARDS_SKILL_HANDLE = os.environ.get("API_STANDARDS_SKILL_HANDLE", "api-standards") + + +def _pull_api_standards_skill_sync(handle: str) -> str | None: + from langsmith import Client as LangSmithClient + + client = LangSmithClient() + skill = client.pull_skill(handle) + files = getattr(skill, "files", None) or {} + entry = files.get("SKILL.md") + content = getattr(entry, "content", None) if entry is not None else None + if isinstance(content, str) and content.strip(): + return content.strip() + return None + + +async def fetch_api_standards_skill(handle: str | None = None) -> str | None: + """Return the API-standards ``SKILL.md`` content, or ``None`` on any failure.""" + resolved = handle or API_STANDARDS_SKILL_HANDLE + if not resolved: + return None + try: + content = await asyncio.to_thread(_pull_api_standards_skill_sync, resolved) + except Exception: # noqa: BLE001 + logger.warning("Failed to pull API-standards skill '%s'", resolved, exc_info=True) + return None + if content: + logger.info( + "Loaded API-standards skill '%s' (%d chars) into reviewer prompt", + resolved, + len(content), + ) + return content diff --git a/tests/test_api_standards_skill.py b/tests/test_api_standards_skill.py new file mode 100644 index 00000000..548bb0c1 --- /dev/null +++ b/tests/test_api_standards_skill.py @@ -0,0 +1,35 @@ +from __future__ import annotations + +from unittest.mock import patch + +import pytest + +from agent.utils import api_standards_skill + + +@pytest.mark.asyncio +async def test_fetch_returns_skill_content() -> None: + with patch.object( + api_standards_skill, + "_pull_api_standards_skill_sync", + return_value="Use /v1/ prefixes.", + ): + content = await api_standards_skill.fetch_api_standards_skill("api-standards") + assert content == "Use /v1/ prefixes." + + +@pytest.mark.asyncio +async def test_fetch_returns_none_on_error() -> None: + def _boom(_handle: str) -> str | None: + raise RuntimeError("no api key") + + with patch.object(api_standards_skill, "_pull_api_standards_skill_sync", side_effect=_boom): + content = await api_standards_skill.fetch_api_standards_skill("api-standards") + assert content is None + + +@pytest.mark.asyncio +async def test_fetch_returns_none_when_no_handle_configured() -> None: + with patch.object(api_standards_skill, "API_STANDARDS_SKILL_HANDLE", ""): + content = await api_standards_skill.fetch_api_standards_skill("") + assert content is None diff --git a/tests/test_reviewer.py b/tests/test_reviewer.py index bd4cc6b4..e4a11082 100644 --- a/tests/test_reviewer.py +++ b/tests/test_reviewer.py @@ -62,6 +62,29 @@ def test_reviewer_system_prompt_org_guidelines_precede_repo_style() -> None: ) +def test_reviewer_system_prompt_includes_api_standards_section() -> None: + prompt = reviewer._reviewer_system_prompt( + "/workspace/repo", + repo_owner="acme", + repo_name="repo", + pr_number=42, + api_standards_skill="Always version your endpoints under /v1/.", + ) + assert "API standards skill" in prompt + assert "Always version your endpoints under /v1/." in prompt + assert "introduces a new API or modifies" in prompt + + +def test_reviewer_system_prompt_omits_api_standards_when_absent() -> None: + prompt = reviewer._reviewer_system_prompt( + "/workspace/repo", + repo_owner="acme", + repo_name="repo", + pr_number=42, + ) + assert "API standards skill" not in prompt + + 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",