Support load code review skills from URLs on demand

bugbug/tools/code_review/agent.py

@@ -25,11 +25,13 @@
     AgentResponse,
     CodeReviewToolResponse,
     GeneratedReviewComment,
+    Skill,
 )
 from bugbug.tools.code_review.database import ReviewCommentsDB
 from bugbug.tools.code_review.langchain_tools import (
     CodeReviewContext,
     create_find_function_definition_tool,
+    create_load_skill_tool,
     expand_context,
 )
 from bugbug.tools.code_review.prompts import (
@@ -70,6 +72,7 @@ def __init__(
         verbose: bool = True,
         target_software: str = "Mozilla Firefox",
         todo_enabled: bool = True,
+        skills: Optional[list[Skill]] = None,
     ) -> None:
         super().__init__()
 
@@ -98,6 +101,9 @@ def __init__(
         if function_search:
             tools.append(create_find_function_definition_tool(function_search))
 
+        if skills:
+            tools.append(create_load_skill_tool(skills))
+
         self._agent_model = llm
 
         middleware = []
@@ -176,6 +182,11 @@ def create(cls, **kwargs):
 
             kwargs["suggestion_filterer"] = SuggestionFilteringTool.create()
 
+        if "skills" not in kwargs:
+            from bugbug.tools.code_review.prompts import REVIEW_SKILLS
+
+            kwargs["skills"] = REVIEW_SKILLS
+
         return cls(**kwargs)
 
     def count_tokens(self, text):

bugbug/tools/code_review/data_types.py

@@ -1,5 +1,9 @@
-from pydantic import BaseModel, Field
+import re
 
+import httpx
+from pydantic import BaseModel, Field, PrivateAttr
+
+from bugbug.tools.core.connection import get_http_client
 from bugbug.tools.core.data_types import InlineComment
 
 
@@ -37,3 +41,44 @@ class CodeReviewToolResponse(BaseModel):
     details: dict = Field(
         description="Additional details about the tool's execution, such as which models were used and any relevant metadata."
     )
+
+
+class SkillLoadError(Exception):
+    """Raised when a Skill's body cannot be loaded."""
+
+
+_FRONTMATTER_RE = re.compile(r"\A---\s*\n.*?\n---\s*\n", re.DOTALL)
+
+
+def _strip_frontmatter(text: str) -> str:
+    return _FRONTMATTER_RE.sub("", text, count=1)
+
+
+class Skill(BaseModel):
+    """A reusable instruction set the agent can load on demand."""
+
+    name: str = Field(
+        description="A unique identifier the agent uses to load the skill."
+    )
+    url: str = Field(description="HTTPS URL of the skill.md file.")
+    description: str = Field(
+        description="Short summary shown to the agent so it can decide when to load this skill."
+    )
+
+    _cached_body: str | None = PrivateAttr(default=None)
+
+    async def load(self) -> str:
+        """Return the skill body, fetching and caching it on first use."""
+        if self._cached_body is not None:
+            return self._cached_body
+
+        try:
+            response = await get_http_client().get(self.url, timeout=30)
+            response.raise_for_status()
+        except httpx.HTTPError as e:
+            raise SkillLoadError(
+                f"Could not load skill '{self.name}' from {self.url}"
+            ) from e
+
+        self._cached_body = _strip_frontmatter(response.text)
+        return self._cached_body

bugbug/tools/code_review/langchain_tools.py

@@ -13,6 +13,7 @@
 from requests import HTTPError
 
 from bugbug.code_search.function_search import FunctionSearch
+from bugbug.tools.code_review.data_types import Skill, SkillLoadError
 from bugbug.tools.core.platforms.base import Patch
 
 logger = getLogger(__name__)
@@ -95,3 +96,37 @@ def find_function_definition(
         return functions[0].source
 
     return find_function_definition
+
+
+def create_load_skill_tool(skills: list[Skill]):
+    skills_by_name = {skill.name: skill for skill in skills}
+    available_names = ", ".join(skills_by_name)
+
+    catalog_lines = "\n".join(
+        f"- **{skill.name}**: {skill.description}" for skill in skills
+    )
+    description = (
+        "Load the contents of a named skill to guide the review. Use this when "
+        "the patch touches an area covered by one of the skills below; otherwise, "
+        "do not call it.\n\n"
+        "Available skills:\n"
+        f"{catalog_lines}\n\n"
+        "Args:\n"
+        "    name: The name of the skill to load (must match one of the names above).\n\n"
+        "Returns:\n"
+        "    The skill content as Markdown."
+    )
+
+    @tool(description=description)
+    async def load_skill(name: str) -> str:
+        skill = skills_by_name.get(name)
+        if skill is None:
+            return f"Unknown skill '{name}'. Available: {available_names}."
+
+        try:
+            return await skill.load()
+        except SkillLoadError:
+            logger.exception("Failed to load skill '%s'", name)
+            return f"Failed to load skill '{name}'. Please proceed without it."
+
+    return load_skill

bugbug/tools/code_review/prompts.py

@@ -5,6 +5,8 @@
 
 """Prompt templates for code review agent."""
 
+from bugbug.tools.code_review.data_types import Skill
+
 TARGET_SOFTWARE: str | None = None
 
 
@@ -213,3 +215,10 @@
 +++ b/{filename}
 {raw_hunk}
 """
+
+
+# The agent exposes these to the model via a load_skill tool that fetches
+# the URL on demand, strips YAML frontmatter, and caches the result.
+REVIEW_SKILLS: list[Skill] = [
+    # Skill(name="...", url="https://...", description="..."),
+]

pyproject.toml

@@ -80,6 +80,7 @@ test = [
     "jsonschema==4.26.0",
     "pre-commit==4.5.1",
     "pytest==9.0.3",
+    "pytest-asyncio==1.3.0",
     "pytest-cov==7.1.0",
     "pytest-responses==0.5.1",
     "responses==0.21.0",

tests/test_code_review.py

@@ -1,7 +1,14 @@
+import logging
 import os
+from unittest.mock import AsyncMock, MagicMock, patch
 
+import httpx
+import pytest
 from unidiff import PatchSet
 
+from bugbug.tools.code_review import data_types, langchain_tools
+from bugbug.tools.code_review.data_types import Skill, _strip_frontmatter
+from bugbug.tools.code_review.langchain_tools import create_load_skill_tool
 from bugbug.tools.code_review.utils import find_comment_scope
 
 FIXTURES_DIR = os.path.join(os.path.dirname(__file__), "fixtures/phabricator")
@@ -56,3 +63,122 @@ def test_find_comment_scope():
 
             for line_number, expected_scope in target_hunks.items():
                 assert find_comment_scope(patched_file, line_number) == expected_scope
+
+
+def _mock_client_returning(text: str) -> MagicMock:
+    response = MagicMock()
+    response.text = text
+    response.raise_for_status = MagicMock()
+    client = MagicMock()
+    client.get = AsyncMock(return_value=response)
+    return client
+
+
+def test_strip_frontmatter_present():
+    text = "---\nname: mozilla\ndescription: foo\n---\nThe body.\n"
+    assert _strip_frontmatter(text) == "The body.\n"
+
+
+def test_strip_frontmatter_absent():
+    text = "Just a body, no frontmatter.\n"
+    assert _strip_frontmatter(text) == text
+
+
+def test_strip_frontmatter_unterminated():
+    text = "---\nname: mozilla\nstill no closing marker\n"
+    assert _strip_frontmatter(text) == text
+
+
+@pytest.mark.asyncio
+async def test_skill_load_caches():
+    skill = Skill(
+        name="a",
+        url="https://example.com/a.md",
+        description="example",
+    )
+    client = _mock_client_returning("---\nname: a\n---\nbody\n")
+    with patch.object(data_types, "get_http_client", return_value=client):
+        first = await skill.load()
+        second = await skill.load()
+    assert first == "body\n"
+    assert second == "body\n"
+    assert client.get.await_count == 1
+
+
+@pytest.mark.asyncio
+async def test_load_skill_unknown_name():
+    skills = [
+        Skill(
+            name="mozilla-style",
+            url="https://example.com/mozilla-style.md",
+            description="Mozilla style guide",
+        )
+    ]
+    tool = create_load_skill_tool(skills)
+    client = _mock_client_returning("ignored")
+    with patch.object(data_types, "get_http_client", return_value=client):
+        result = await tool.ainvoke({"name": "nonexistent"})
+    assert "Unknown skill 'nonexistent'" in result
+    assert "mozilla-style" in result
+    assert client.get.await_count == 0
+
+
+@pytest.mark.asyncio
+async def test_load_skill_happy_path():
+    skills = [
+        Skill(
+            name="mozilla-style",
+            url="https://example.com/mozilla-style.md",
+            description="Mozilla style guide",
+        )
+    ]
+    tool = create_load_skill_tool(skills)
+    client = _mock_client_returning(
+        "---\nname: mozilla-style\ndescription: ignored\n---\nUse 4-space indents.\n"
+    )
+    with patch.object(data_types, "get_http_client", return_value=client):
+        result = await tool.ainvoke({"name": "mozilla-style"})
+    assert result == "Use 4-space indents.\n"
+
+
+@pytest.mark.asyncio
+async def test_load_skill_fetch_failure(caplog):
+    skills = [
+        Skill(
+            name="mozilla-style",
+            url="https://example.com/mozilla-style.md",
+            description="Mozilla style guide",
+        )
+    ]
+    tool = create_load_skill_tool(skills)
+    client = MagicMock()
+    client.get = AsyncMock(side_effect=httpx.ConnectError("boom"))
+    with (
+        patch.object(data_types, "get_http_client", return_value=client),
+        caplog.at_level(logging.ERROR, logger=langchain_tools.logger.name),
+    ):
+        result = await tool.ainvoke({"name": "mozilla-style"})
+    assert "Failed to load skill 'mozilla-style'" in result
+    assert any(
+        "Failed to load skill 'mozilla-style'" in record.message
+        for record in caplog.records
+    )
+
+
+def test_load_skill_tool_description_lists_skills():
+    skills = [
+        Skill(
+            name="mozilla-style",
+            url="https://example.com/mozilla-style.md",
+            description="Mozilla style guide",
+        ),
+        Skill(
+            name="security-checklist",
+            url="https://example.com/sec.md",
+            description="Common security pitfalls",
+        ),
+    ]
+    tool = create_load_skill_tool(skills)
+    for skill in skills:
+        assert skill.name in tool.description
+        assert skill.description in tool.description