feat(pr-review): add sub-agent delegation for file-level reviews #164
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,6 +27,13 @@ inputs: | |
| description: "When true, require the reviewer to check the PR description for an Evidence section proving the code works end-to-end (screenshots/videos for frontend changes; commands and runtime output for backend, CLI, or script changes; conversation link when agent-generated). Test output alone does not count." | ||
| required: false | ||
| default: 'false' | ||
| use-sub-agents: | ||
| description: > | ||
| Enable sub-agent delegation for file-level reviews (experimental). | ||
| When true, the agent gets the TaskToolSet and decides at runtime | ||
| whether to delegate based on diff size and complexity. | ||
| required: false | ||
| default: 'false' | ||
| extensions-repo: | ||
| description: GitHub repository for extensions (owner/repo) | ||
| required: false | ||
|
|
@@ -125,6 +132,7 @@ runs: | |
| LLM_BASE_URL: ${{ inputs.llm-base-url }} | ||
| REVIEW_STYLE: ${{ inputs.review-style }} | ||
| REQUIRE_EVIDENCE: ${{ inputs.require-evidence }} | ||
| USE_SUB_AGENTS: ${{ inputs.use-sub-agents }} | ||
| LLM_API_KEY: ${{ inputs.llm-api-key }} | ||
| GITHUB_TOKEN: ${{ inputs.github-token }} | ||
| LMNR_PROJECT_API_KEY: ${{ inputs.lmnr-api-key }} | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -32,6 +32,11 @@ | |||||
| REPO_NAME: Repository name in format owner/repo (required) | ||||||
| REQUIRE_EVIDENCE: Whether to require PR description evidence showing the code | ||||||
| works ('true'/'false', default: 'false') | ||||||
| USE_SUB_AGENTS: Enable sub-agent delegation for file-level reviews | ||||||
| ('true'/'false', default: 'false'). When enabled, the main agent acts | ||||||
| as a coordinator that delegates per-file review work to | ||||||
| file_reviewer sub-agents via the TaskToolSet, then consolidates | ||||||
| findings into a single GitHub PR review. | ||||||
|
|
||||||
| For setup instructions, usage examples, and GitHub Actions integration, | ||||||
| see README.md in this directory. | ||||||
|
|
@@ -50,18 +55,29 @@ | |||||
| from typing import Any | ||||||
|
|
||||||
| from lmnr import Laminar | ||||||
| from openhands.sdk import ( | ||||||
| LLM, | ||||||
| Agent, | ||||||
| AgentContext, | ||||||
| Conversation, | ||||||
| Tool, | ||||||
| get_logger, | ||||||
| register_agent, | ||||||
| ) | ||||||
| from openhands.sdk.context import Skill | ||||||
| from openhands.sdk.context.skills import load_project_skills | ||||||
| from openhands.sdk.conversation import get_agent_final_response | ||||||
| from openhands.sdk.git.utils import run_git_command | ||||||
| from openhands.sdk.plugin import PluginSource | ||||||
|
Contributor
There was a problem hiding this comment. 🔴 Critical: Partially unresolved deprecation issue. While Verify:
The previous critical review thread specifically flagged this import as problematic.
Contributor
There was a problem hiding this comment. 🟢 Clarification on previous review: The concern about Note: |
||||||
| from openhands.tools.delegate import DelegationVisualizer | ||||||
| from openhands.tools.preset.default import get_default_condenser, get_default_tools | ||||||
| from openhands.tools.task import TaskToolSet | ||||||
|
|
||||||
| # Add the script directory to Python path so we can import prompt.py | ||||||
| script_dir = Path(__file__).parent | ||||||
| sys.path.insert(0, str(script_dir)) | ||||||
|
|
||||||
| from prompt import FILE_REVIEWER_SKILL, format_prompt # noqa: E402 | ||||||
|
|
||||||
| logger = get_logger(__name__) | ||||||
|
|
||||||
|
|
@@ -728,6 +744,7 @@ def validate_environment() -> dict[str, Any]: | |||||
| "model": os.getenv("LLM_MODEL", "anthropic/claude-sonnet-4-5-20250929"), | ||||||
| "base_url": os.getenv("LLM_BASE_URL"), | ||||||
| "require_evidence": _get_bool_env("REQUIRE_EVIDENCE"), | ||||||
| "use_sub_agents": _get_bool_env("USE_SUB_AGENTS"), | ||||||
| "pr_info": { | ||||||
| "number": os.getenv("PR_NUMBER"), | ||||||
| "title": os.getenv("PR_TITLE"), | ||||||
|
|
@@ -763,6 +780,47 @@ def fetch_pr_context(pr_number: str) -> tuple[str, str, str]: | |||||
| return pr_diff, commit_id, review_context | ||||||
|
|
||||||
|
|
||||||
| def _create_file_reviewer_agent(llm: LLM) -> Agent: | ||||||
|
Contributor
There was a problem hiding this comment. 🟢 Clarification on previous review: The factory function signature is correct. I verified that |
||||||
| """Factory for file_reviewer sub-agents used during delegation. | ||||||
|
|
||||||
| Each sub-agent receives a skill that defines its review persona and | ||||||
| expected output format. It has read-only terminal and file_editor | ||||||
| access so it can inspect surrounding code context in the PR repo, | ||||||
| but the coordinator handles all GitHub API interaction. | ||||||
| """ | ||||||
| skills = [ | ||||||
| Skill( | ||||||
|
Contributor
There was a problem hiding this comment. 🟠 Important: Verify the factory function signature matches The function accepts If the signature is wrong, agent instantiation will fail at runtime with no static type checking to catch it.
Contributor
There was a problem hiding this comment. 🟢 Nit: Spelling consistency - use US English to match the rest of the codebase (README uses "analyze").
Suggested change
|
||||||
| name="file_review_instructions", | ||||||
| content=FILE_REVIEWER_SKILL, | ||||||
| trigger=None, | ||||||
| ), | ||||||
| ] | ||||||
| return Agent( | ||||||
| llm=llm, | ||||||
| tools=[ | ||||||
| Tool(name="terminal"), | ||||||
| Tool(name="file_editor"), | ||||||
| ], | ||||||
| agent_context=AgentContext(skills=skills), | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def _register_sub_agents() -> None: | ||||||
| """Register the file_reviewer agent type. | ||||||
|
|
||||||
| TaskToolSet auto-registers on import, so no explicit | ||||||
| ``register_tool()`` call is needed. | ||||||
| """ | ||||||
| register_agent( | ||||||
|
Contributor
There was a problem hiding this comment. 🟠 Important: Related to the deprecation concern - verify that |
||||||
| name="file_reviewer", | ||||||
| factory_func=_create_file_reviewer_agent, | ||||||
| description=( | ||||||
| "Reviews one or more files from a PR diff and returns structured " | ||||||
| "findings as a JSON array." | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
|
|
||||||
| def create_conversation( | ||||||
| config: dict[str, Any], | ||||||
| secrets: dict[str, str], | ||||||
|
|
@@ -773,6 +831,9 @@ def create_conversation( | |||||
| handles wiring skills, MCP config, and hooks automatically. | ||||||
| Project-specific skills from the workspace are loaded separately. | ||||||
|
|
||||||
| When ``config["use_sub_agents"]`` is True the coordinator agent is | ||||||
| given the TaskToolSet so it can delegate to file_reviewer sub-agents. | ||||||
|
|
||||||
| Args: | ||||||
| config: Configuration dictionary from validate_environment() | ||||||
| secrets: Secrets to mask in output | ||||||
|
|
@@ -804,9 +865,17 @@ def create_conversation( | |||||
| skills=project_skills, | ||||||
| ) | ||||||
|
|
||||||
| tools = get_default_tools(enable_browser=False) | ||||||
|
|
||||||
| use_sub_agents = config.get("use_sub_agents", False) | ||||||
| if use_sub_agents: | ||||||
| _register_sub_agents() | ||||||
| tools.append(Tool(name=TaskToolSet.name)) | ||||||
|
Contributor
There was a problem hiding this comment. 🟢 Acceptable: No validation that sub-agent registration succeeded or that TaskToolSet.name is valid. If registration silently fails, the coordinator would try to use a non-existent tool. For an experimental feature with manual workflow testing, this is acceptable. If/when this graduates from experimental, consider adding validation: try:
_register_sub_agents()
tools.append(Tool(name=TaskToolSet.name))
logger.info(f"Sub-agent delegation enabled — {TaskToolSet.name} registered")
except Exception as e:
logger.error(f"Sub-agent registration failed: {e}")
raise |
||||||
| logger.info("Sub-agent delegation enabled — TaskToolSet added") | ||||||
|
|
||||||
| agent = Agent( | ||||||
| llm=llm, | ||||||
| tools=tools, | ||||||
| agent_context=agent_context, | ||||||
| system_prompt_kwargs={"cli_mode": True}, | ||||||
| condenser=get_default_condenser( | ||||||
|
|
@@ -816,12 +885,18 @@ def create_conversation( | |||||
|
|
||||||
| # The plugin directory is the parent of the scripts/ directory | ||||||
| plugin_dir = script_dir.parent # plugins/pr-review/ | ||||||
| conversation_kwargs: dict[str, Any] = { | ||||||
| "agent": agent, | ||||||
| "workspace": cwd, | ||||||
| "secrets": secrets, | ||||||
| "plugins": [PluginSource(source=str(plugin_dir))], | ||||||
| } | ||||||
| if use_sub_agents: | ||||||
| conversation_kwargs["visualizer"] = DelegationVisualizer( | ||||||
| name="PR Review Coordinator" | ||||||
| ) | ||||||
|
|
||||||
| return Conversation(**conversation_kwargs) | ||||||
|
|
||||||
|
|
||||||
| def run_review( | ||||||
|
|
@@ -930,9 +1005,11 @@ def main(): | |||||
| config = validate_environment() | ||||||
| pr_info = config["pr_info"] | ||||||
| require_evidence = config["require_evidence"] | ||||||
| use_sub_agents = config["use_sub_agents"] | ||||||
|
|
||||||
| logger.info(f"Reviewing PR #{pr_info['number']}: {pr_info['title']}") | ||||||
| logger.info(f"Require PR evidence: {require_evidence}") | ||||||
| logger.info(f"Sub-agent delegation: {use_sub_agents}") | ||||||
|
|
||||||
| try: | ||||||
| pr_diff, commit_id, review_context = fetch_pr_context(pr_info["number"]) | ||||||
|
|
@@ -952,6 +1029,7 @@ def main(): | |||||
| diff=pr_diff, | ||||||
| review_context=review_context, | ||||||
| require_evidence=require_evidence, | ||||||
| use_sub_agents=use_sub_agents, | ||||||
| ) | ||||||
|
|
||||||
| secrets = {} | ||||||
|
|
||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,10 @@ | |
| - {pr_number} - The PR number | ||
| - {commit_id} - The HEAD commit SHA | ||
| - {review_context} - Previous review comments and thread resolution status | ||
|
|
||
| When sub-agent delegation is enabled (``use_sub_agents=True``), a short | ||
| delegation suffix is appended to the base prompt giving the agent the | ||
| option to delegate file-level reviews via the TaskToolSet. | ||
| """ | ||
|
|
||
| # Template for when there is review context available | ||
|
|
@@ -75,6 +79,78 @@ | |
| Analyze the changes and post your review using the GitHub API. | ||
| """ | ||
|
|
||
| # Appended to PROMPT when use_sub_agents=True. Gives the main agent the | ||
| # option to delegate via the TaskToolSet without duplicating the base prompt. | ||
| _DELEGATION_SUFFIX = """ | ||
| ## Sub-agent Delegation | ||
|
|
||
| You have access to the **task** tool for delegating file-level reviews to | ||
| `file_reviewer` sub-agents. Use it when the diff is large — roughly 4+ files | ||
| or 500+ changed lines. For smaller diffs, just review directly. | ||
|
|
||
| When delegating, split the diff by file (or small group of related files) and | ||
| call the task tool with `subagent_type: "file_reviewer"`. Each sub-agent will | ||
| return a JSON array of findings. Merge them, de-duplicate, drop noise, and | ||
| post a single consolidated review via the GitHub API. | ||
| """ | ||
|
|
||
| # Skill content injected into each file_reviewer sub-agent. | ||
| # Defines the review persona, available tools, and — most importantly — the | ||
| # exact JSON schema the sub-agent must return. | ||
| FILE_REVIEWER_SKILL = """\ | ||
| You are a **file-level code reviewer** sub-agent. | ||
|
|
||
| ## Your Task | ||
|
|
||
| You will receive a diff for one or more files from a pull request. | ||
| Review the changes and return structured findings. | ||
|
|
||
| ## Tools | ||
|
|
||
| You have `terminal` and `file_editor` so you can inspect the full source | ||
| files for surrounding context — use `cat`, `grep`, or `file_editor view` | ||
| when the diff alone is not enough to judge an issue. | ||
|
|
||
| ## Review Style | ||
|
|
||
| Be direct, pragmatic, and thorough. Focus on correctness, security, | ||
| simplicity, and maintainability. Call out real problems; skip trivial noise. | ||
|
|
||
| ## Output Format | ||
|
|
||
| Return a JSON array wrapped in a ```json fenced code block. | ||
| Each element must have exactly these fields: | ||
|
|
||
| | Field | Type | Description | | ||
| |------------|--------|-------------| | ||
| | `path` | string | File path exactly as shown in the diff header (e.g. `src/utils.py`) | | ||
| | `line` | int | Line number in the **new** file where the issue occurs | | ||
| | `severity` | string | One of: `"critical"`, `"major"`, `"minor"`, `"nit"` | | ||
| | `body` | string | Concise description of the issue, including a suggested fix | | ||
|
|
||
| ### Severity guide | ||
| - **critical** — bug, security vulnerability, or data loss | ||
| - **major** — incorrect logic, missing error handling, performance issue | ||
| - **minor** — style, readability, or minor correctness concern | ||
| - **nit** — cosmetic or trivial preference | ||
|
|
||
| ### Example | ||
|
|
||
| ```json | ||
| [ | ||
| {{"path": "src/utils.py", "line": 42, "severity": "major", "body": "Unchecked `None` return — add a guard before accessing `.value`."}}, | ||
| {{"path": "src/utils.py", "line": 78, "severity": "nit", "body": "Unused import `os`."}} | ||
| ] | ||
| ``` | ||
|
|
||
| If you find no issues, return: | ||
| ```json | ||
| [] | ||
| ``` | ||
|
|
||
| When you are done, call the `finish` tool with the JSON array as the message. | ||
| """ | ||
|
|
||
|
|
||
| def format_prompt( | ||
| skill_trigger: str, | ||
|
|
@@ -88,6 +164,7 @@ def format_prompt( | |
| diff: str, | ||
| review_context: str = "", | ||
| require_evidence: bool = False, | ||
| use_sub_agents: bool = False, | ||
| ) -> str: | ||
| """Format the PR review prompt with all parameters. | ||
|
|
||
|
|
@@ -105,6 +182,9 @@ def format_prompt( | |
| the review context section is omitted from the prompt. | ||
| require_evidence: Whether to instruct the reviewer to enforce PR description | ||
| evidence showing the code works. | ||
| use_sub_agents: When True, the agent gets the TaskToolSet and decides | ||
| at runtime whether to delegate file-level reviews to | ||
| sub-agents based on diff size and complexity. | ||
|
|
||
| Returns: | ||
| Formatted prompt string | ||
|
|
@@ -121,7 +201,7 @@ def format_prompt( | |
| _EVIDENCE_REQUIREMENT_SECTION if require_evidence else "" | ||
| ) | ||
|
|
||
| prompt = PROMPT.format( | ||
| skill_trigger=skill_trigger, | ||
| title=title, | ||
| body=body, | ||
|
|
@@ -134,3 +214,8 @@ def format_prompt( | |
| evidence_requirements_section=evidence_requirements_section, | ||
| diff=diff, | ||
| ) | ||
|
|
||
| if use_sub_agents: | ||
| prompt += _DELEGATION_SUFFIX | ||
|
|
||
| return prompt | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟢 Nit: Consider adding a Known Limitations subsection under the experimental feature description that mentions:
This sets appropriate expectations for users trying the experimental feature.