Implement MCP tool registry and integration in PRReviewer#2356
Open
benny338813 wants to merge 5 commits into
Open
Implement MCP tool registry and integration in PRReviewer#2356benny338813 wants to merge 5 commits into
benny338813 wants to merge 5 commits into
Conversation
added 4 commits
April 29, 2026 01:39
Contributor
Review Summary by QodoImplement MCP tool registry and LLM tool calling integration
WalkthroughsDescription• Implement tool registry system for dynamic tool registration • Add MCP (Model Context Protocol) integration framework • Refactor tool instantiation to use registry pattern • Support tool calling with LLM integration Diagramflowchart LR
A["Tool Classes"] -->|register| B["ToolRegistry"]
B -->|lookup| C["PRAgent"]
C -->|get_tools| D["MCPHandler"]
D -->|call_tool| E["LLM with Tools"]
E -->|tool_calls| D
File Changes1. pr_agent/tools/registry.py
|
Contributor
Code Review by Qodo
1. Ruff E302: missing blank lines
|
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+28
to
+50
| # command2class = { | ||
| # "auto_review": PRReviewer, | ||
| # "answer": PRReviewer, | ||
| # "review": PRReviewer, | ||
| # "review_pr": PRReviewer, | ||
| # "describe": PRDescription, | ||
| # "describe_pr": PRDescription, | ||
| # "improve": PRCodeSuggestions, | ||
| # "improve_code": PRCodeSuggestions, | ||
| # "ask": PRQuestions, | ||
| # "ask_question": PRQuestions, | ||
| # "ask_line": PR_LineQuestions, | ||
| # "update_changelog": PRUpdateChangelog, | ||
| # "config": PRConfig, | ||
| # "settings": PRConfig, | ||
| # "help": PRHelpMessage, | ||
| # "similar_issue": PRSimilarIssue, | ||
| # "add_docs": PRAddDocs, | ||
| # "generate_labels": PRGenerateLabels, | ||
| # "help_docs": PRHelpDocs, | ||
| # } | ||
|
|
||
| # commands = list(command2class.keys()) |
Contributor
There was a problem hiding this comment.
1. Commented-out command2class block 📘 Rule violation ⚙ Maintainability
A large command2class mapping is left in the code as commented-out lines, which is dead code and adds maintenance noise. This violates the requirement to avoid commented-out code in submitted changes.
Agent Prompt
## Issue description
A large commented-out `command2class` block was added, which is dead code and violates the no-commented-out-code requirement.
## Issue Context
The tool dispatch was replaced by `ToolRegistry`, so this legacy mapping should be removed rather than kept commented.
## Fix Focus Areas
- pr_agent/agent/pr_agent.py[28-50]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| if not tool_class: | ||
| get_logger().warning(f"Unknown command: {action}") | ||
| return False | ||
|
|
Contributor
There was a problem hiding this comment.
2. Whitespace-only line has trailing spaces 📘 Rule violation ⚙ Maintainability
A whitespace-only line with indentation spaces was added, which will fail pre-commit trailing-whitespace hooks. This violates the repository’s pre-commit hygiene requirements.
Agent Prompt
## Issue description
A blank line containing spaces was introduced, which violates pre-commit trailing-whitespace rules.
## Issue Context
This will typically fail CI/pre-commit and should be cleaned up.
## Fix Focus Areas
- pr_agent/agent/pr_agent.py[110-110]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+39
to
64
| def __init__(self, pr_url: str, ai_handler: partial[BaseAiHandler,] = LiteLLMAIHandler, args: list = None): | ||
| super().__init__(pr_url, ai_handler=ai_handler, args=args) | ||
| self.git_provider = get_git_provider_with_context(pr_url) | ||
| self.main_pr_language = get_main_pr_language( | ||
| self.git_provider.get_languages(), self.git_provider.get_files() | ||
| self.ai_handler = ai_handler() | ||
| self.vars = { | ||
| "title": self.git_provider.pr.title, | ||
| "branch": self.git_provider.get_pr_branch(), | ||
| "description": self.git_provider.get_pr_description(), | ||
| "language": get_main_pr_language( | ||
| self.git_provider.get_languages(), self.git_provider.get_files() | ||
| ), | ||
| "commit_messages_str": self.git_provider.get_commit_messages(), | ||
| "extra_instructions": get_settings().pr_description_prompt.extra_instructions, | ||
| 'duplicate_prompt_examples': get_settings().config.get('duplicate_prompt_examples', False), | ||
| "enable_custom_labels": get_settings().config.enable_custom_labels, | ||
| } | ||
| self.token_handler = TokenHandler( | ||
| self.git_provider.pr, | ||
| self.vars, | ||
| get_settings().pr_description_prompt.system, | ||
| get_settings().pr_description_prompt.user | ||
| ) | ||
|
|
||
| self.pr_id = self.git_provider.get_pr_id() | ||
| self.keys_fix = ["filename:", "language:", "changes_summary:", "changes_title:", "description:", "title:"] | ||
|
|
Contributor
There was a problem hiding this comment.
9. Prdescription init crashes 🐞 Bug ≡ Correctness
PRDescription.__init__ now duplicates initialization logic and references self.main_pr_language even though it is no longer set, causing AttributeError during tool construction.
Agent Prompt
### Issue description
`PRDescription.__init__` contains two initialization blocks and uses `self.main_pr_language` without defining it, causing an `AttributeError` and inconsistent object state.
### Issue Context
This appears to be a partial refactor to `PRTool`/registry, leaving old initialization in place.
### Fix Focus Areas
- pr_agent/tools/pr_description.py[39-103]
### Expected change
Remove the duplicated initialization and ensure `self.main_pr_language` (or equivalent) is set exactly once before being used (and align `self.vars` / `TokenHandler` creation to that single source of truth).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+228
to
+238
| if finish_reason == "tool_calls" and tool_calls: | ||
| # Execute MCP tools | ||
| for tool_call in tool_calls: | ||
| tool_name = tool_call.function.name | ||
| tool_args = json.loads(tool_call.function.arguments) | ||
| get_logger().info(f"Executing tool: {tool_name} with args {tool_args}") | ||
| tool_result = await handler.call_tool(tool_name, tool_args) | ||
|
|
||
| # Add tool result to user prompt | ||
| user_prompt += f"\n\nTool Result ({tool_name}): {json.dumps(tool_result)}" | ||
|
|
Contributor
There was a problem hiding this comment.
10. Mcp tool_calls parsing crash 🐞 Bug ≡ Correctness
When MCP tools are enabled, PRReviewer assumes each tool_call has .function.name/.arguments, but the LiteLLM response passes tool_calls as dicts, so tool execution will crash before any MCP tool can run.
Agent Prompt
### Issue description
`PRReviewer` parses `tool_calls` using attribute access (`tool_call.function.name`), but the handler returns tool calls as dictionaries from the OpenAI/LiteLLM response.
### Issue Context
Tool calls are typically shaped like:
```json
{"id": "...", "type": "function", "function": {"name": "...", "arguments": "{...}"}}
```
### Fix Focus Areas
- pr_agent/tools/pr_reviewer.py[228-238]
- pr_agent/algo/ai_handlers/litellm_ai_handler.py[458-470]
### Expected change
Update parsing to support dict tool calls (e.g., `tool_call["function"]["name"]` and `tool_call["function"]["arguments"]`), and add defensive handling for missing/invalid JSON arguments so MCP execution can proceed or fail gracefully.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
|
Persistent review updated to latest commit 463e639 |
Comment on lines
+1
to
+5
| from abc import ABC, abstractmethod | ||
| from typing import List, Optional | ||
|
|
||
| class PRTool(ABC): | ||
| def __init__(self, pr_url: str, ai_handler=None, args: Optional[List[str]] = None): |
Contributor
There was a problem hiding this comment.
1. Ruff e302: missing blank lines 📘 Rule violation ⚙ Maintainability
New Python modules do not follow Ruff/PEP8 spacing rules (e.g., missing two blank lines after imports before top-level definitions). This can cause Ruff CI failures and violates the repository formatting conventions.
Agent Prompt
## Issue description
New Python files violate standard top-level spacing (likely Ruff E302), due to insufficient blank lines between imports and `def`/`class` definitions.
## Issue Context
The repository enforces Ruff formatting; these spacing issues can break CI/pre-commit.
## Fix Focus Areas
- pr_agent/tools/base.py[1-5]
- utils/generate_prompt_report.py[1-6]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
+11
to
+51
| from pr_agent.tools.registry import ToolRegistry | ||
| from pr_agent.tools.pr_reviewer import PRReviewer | ||
| from pr_agent.tools.pr_similar_issue import PRSimilarIssue | ||
| from pr_agent.tools.pr_update_changelog import PRUpdateChangelog | ||
|
|
||
| command2class = { | ||
| "auto_review": PRReviewer, | ||
| "answer": PRReviewer, | ||
| "review": PRReviewer, | ||
| "review_pr": PRReviewer, | ||
| "describe": PRDescription, | ||
| "describe_pr": PRDescription, | ||
| "improve": PRCodeSuggestions, | ||
| "improve_code": PRCodeSuggestions, | ||
| "ask": PRQuestions, | ||
| "ask_question": PRQuestions, | ||
| "ask_line": PR_LineQuestions, | ||
| "update_changelog": PRUpdateChangelog, | ||
| "config": PRConfig, | ||
| "settings": PRConfig, | ||
| "help": PRHelpMessage, | ||
| "similar_issue": PRSimilarIssue, | ||
| "add_docs": PRAddDocs, | ||
| "generate_labels": PRGenerateLabels, | ||
| "help_docs": PRHelpDocs, | ||
| } | ||
|
|
||
| commands = list(command2class.keys()) | ||
|
|
||
| # Ensure all tools are imported to trigger registration | ||
| import pr_agent.tools.pr_add_docs | ||
| import pr_agent.tools.pr_code_suggestions | ||
| import pr_agent.tools.pr_config | ||
| import pr_agent.tools.pr_description | ||
| import pr_agent.tools.pr_generate_labels | ||
| import pr_agent.tools.pr_help_docs | ||
| import pr_agent.tools.pr_help_message | ||
| import pr_agent.tools.pr_line_questions | ||
| import pr_agent.tools.pr_questions | ||
| import pr_agent.tools.pr_similar_issue | ||
| import pr_agent.tools.pr_update_changelog | ||
| import pr_agent.tools.ticket_pr_compliance_check | ||
|
|
||
| # command2class = { | ||
| # "auto_review": PRReviewer, | ||
| # "answer": PRReviewer, | ||
| # "review": PRReviewer, | ||
| # "review_pr": PRReviewer, | ||
| # "describe": PRDescription, | ||
| # "describe_pr": PRDescription, | ||
| # "improve": PRCodeSuggestions, | ||
| # "improve_code": PRCodeSuggestions, | ||
| # "ask": PRQuestions, | ||
| # "ask_question": PRQuestions, | ||
| # "ask_line": PR_LineQuestions, | ||
| # "update_changelog": PRUpdateChangelog, | ||
| # "config": PRConfig, | ||
| # "settings": PRConfig, | ||
| # "help": PRHelpMessage, | ||
| # "similar_issue": PRSimilarIssue, | ||
| # "add_docs": PRAddDocs, | ||
| # "generate_labels": PRGenerateLabels, | ||
| # "help_docs": PRHelpDocs, | ||
| # } | ||
|
|
||
| # commands = list(command2class.keys()) | ||
| commands = ToolRegistry.get_all_commands() |
Contributor
There was a problem hiding this comment.
2. Azure webhook importerror 🐞 Bug ☼ Reliability
pr_agent/servers/azuredevops_server_webhook.py imports command2class from pr_agent.agent.pr_agent, but this PR removed that symbol (only a commented block remains). The Azure DevOps Server webhook process will fail at import time with ImportError, breaking all ADO webhook handling.
Agent Prompt
### Issue description
Azure DevOps Server webhook imports and uses `command2class`, but `command2class` was removed from `pr_agent.agent.pr_agent`, causing an import-time failure.
### Issue Context
The PR migrated command dispatching to `ToolRegistry`, but `azuredevops_server_webhook.py` still depends on the old mapping for both imports and command parsing.
### Fix Focus Areas
- pr_agent/servers/azuredevops_server_webhook.py[22-33]
- pr_agent/agent/pr_agent.py[11-51]
### Proposed fix
- Replace `from pr_agent.agent.pr_agent import PRAgent, command2class` with an import of `commands` (or `ToolRegistry`) and build `available_commands_rgx` from that list.
- Alternatively, re-introduce a backwards-compatible `command2class` export (as a thin adapter around `ToolRegistry`) if external integrations rely on it.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Comment on lines
19
to
+28
| @abstractmethod | ||
| async def chat_completion(self, model: str, system: str, user: str, temperature: float = 0.2, img_path: str = None): | ||
| async def chat_completion( | ||
| self, | ||
| model: str, | ||
| system: str, | ||
| user: str, | ||
| temperature: float = 0.2, | ||
| img_path: str = None, | ||
| tools: Optional[List[Dict]] = None, | ||
| ): |
Contributor
There was a problem hiding this comment.
3. Non-lite handlers incompatible 🐞 Bug ☼ Reliability
The PR changed tool call sites to unpack chat_completion into three values and added a tools kwarg to the base interface, but OpenAIHandler and LangChainOpenAIHandler still return two values and don’t accept tools. Selecting these handlers (or enabling MCP with them) will trigger unpacking errors and/or TypeError on the unexpected kwarg.
Agent Prompt
### Issue description
The `chat_completion` contract was extended (added `tools` kwarg and a third return value for tool calls), but only `LiteLLMAIHandler` was updated. Other AI handlers now violate the interface and will break when configured.
### Issue Context
Multiple tools now assume `response, finish_reason, tool_calls = ...` (or `_`), and MCP-enabled `PRReviewer` passes `tools=...`.
### Fix Focus Areas
- pr_agent/algo/ai_handlers/base_ai_handler.py[19-28]
- pr_agent/tools/pr_add_docs.py[85-97]
- pr_agent/algo/ai_handlers/openai_ai_handler.py[45-64]
- pr_agent/algo/ai_handlers/langchain_ai_handler.py[70-102]
### Proposed fix
- Update `OpenAIHandler.chat_completion` and `LangChainOpenAIHandler.chat_completion` to accept `tools: Optional[List[Dict]] = None` (ignore or implement tool calling as appropriate).
- Update them to return a 3-tuple `(resp, finish_reason, tool_calls)` with `tool_calls=None` when not supported.
- Ensure all `BaseAiHandler` implementations follow the same signature and return shape so callers don’t need handler-specific branching.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
4 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
No description provided.