refactor: rename review tools for clarity and consistency
- Renamed `review_code` tool to `codereview` for better naming convention - Renamed `review_changes` tool to `precommit` to better reflect its purpose - Updated all tool descriptions to remove "Triggers:" sections and improve clarity - Updated all imports and references throughout the codebase - Renamed test files to match new tool names - Updated server.py tool registrations - All existing functionality preserved with improved naming This refactoring improves code organization and makes tool purposes clearer. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -156,10 +156,10 @@ def test_review_changes_docker_path_translation():
|
||||
utils.file_utils.CONTAINER_WORKSPACE = container_workspace
|
||||
|
||||
# Import after reloading to get updated environment
|
||||
from tools.review_changes import ReviewChanges
|
||||
from tools.precommit import Precommit
|
||||
|
||||
# Create tool instance
|
||||
tool = ReviewChanges()
|
||||
tool = Precommit()
|
||||
|
||||
# Test path translation in prepare_prompt
|
||||
request = tool.get_request_model()(
|
||||
@@ -209,10 +209,10 @@ def test_review_changes_docker_path_error():
|
||||
utils.file_utils.CONTAINER_WORKSPACE = container_workspace
|
||||
|
||||
# Import after reloading to get updated environment
|
||||
from tools.review_changes import ReviewChanges
|
||||
from tools.precommit import Precommit
|
||||
|
||||
# Create tool instance
|
||||
tool = ReviewChanges()
|
||||
tool = Precommit()
|
||||
|
||||
# Test path translation with an inaccessible path
|
||||
request = tool.get_request_model()(
|
||||
|
||||
@@ -18,9 +18,9 @@ from mcp.types import TextContent
|
||||
from config import MCP_PROMPT_SIZE_LIMIT
|
||||
from tools.analyze import AnalyzeTool
|
||||
from tools.chat import ChatTool
|
||||
from tools.codereview import CodeReviewTool
|
||||
from tools.debug import DebugIssueTool
|
||||
from tools.review_changes import ReviewChanges
|
||||
from tools.review_code import ReviewCodeTool
|
||||
from tools.precommit import Precommit
|
||||
from tools.think_deeper import ThinkDeeperTool
|
||||
|
||||
|
||||
@@ -141,9 +141,9 @@ class TestLargePromptHandling:
|
||||
assert output["status"] == "requires_file_prompt"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_code_large_focus(self, large_prompt):
|
||||
"""Test that review_code tool detects large focus_on field."""
|
||||
tool = ReviewCodeTool()
|
||||
async def test_codereview_large_focus(self, large_prompt):
|
||||
"""Test that codereview tool detects large focus_on field."""
|
||||
tool = CodeReviewTool()
|
||||
result = await tool.execute(
|
||||
{
|
||||
"files": ["/some/file.py"],
|
||||
@@ -159,7 +159,7 @@ class TestLargePromptHandling:
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_changes_large_original_request(self, large_prompt):
|
||||
"""Test that review_changes tool detects large original_request."""
|
||||
tool = ReviewChanges()
|
||||
tool = Precommit()
|
||||
result = await tool.execute({"path": "/some/path", "original_request": large_prompt})
|
||||
|
||||
assert len(result) == 1
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
"""
|
||||
Tests for the review_changes tool
|
||||
Tests for the precommit tool
|
||||
"""
|
||||
|
||||
import json
|
||||
@@ -7,22 +7,22 @@ from unittest.mock import Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools.review_changes import ReviewChanges, ReviewChangesRequest
|
||||
from tools.precommit import Precommit, PrecommitRequest
|
||||
|
||||
|
||||
class TestReviewChangesTool:
|
||||
"""Test the review_changes tool"""
|
||||
class TestPrecommitTool:
|
||||
"""Test the precommit tool"""
|
||||
|
||||
@pytest.fixture
|
||||
def tool(self):
|
||||
"""Create tool instance"""
|
||||
return ReviewChanges()
|
||||
return Precommit()
|
||||
|
||||
def test_tool_metadata(self, tool):
|
||||
"""Test tool metadata"""
|
||||
assert tool.get_name() == "review_changes"
|
||||
assert "REVIEW PENDING GIT CHANGES" in tool.get_description()
|
||||
assert "pre-commit review" in tool.get_description()
|
||||
assert tool.get_name() == "precommit"
|
||||
assert "PRECOMMIT VALIDATION" in tool.get_description()
|
||||
assert "pre-commit" in tool.get_description()
|
||||
|
||||
# Check schema
|
||||
schema = tool.get_input_schema()
|
||||
@@ -34,7 +34,7 @@ class TestReviewChangesTool:
|
||||
|
||||
def test_request_model_defaults(self):
|
||||
"""Test request model default values"""
|
||||
request = ReviewChangesRequest(path="/some/absolute/path")
|
||||
request = PrecommitRequest(path="/some/absolute/path")
|
||||
assert request.path == "/some/absolute/path"
|
||||
assert request.original_request is None
|
||||
assert request.compare_to is None
|
||||
@@ -56,21 +56,21 @@ class TestReviewChangesTool:
|
||||
assert "./relative/path" in response["content"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.find_git_repositories")
|
||||
@patch("tools.precommit.find_git_repositories")
|
||||
async def test_no_repositories_found(self, mock_find_repos, tool):
|
||||
"""Test when no git repositories are found"""
|
||||
mock_find_repos.return_value = []
|
||||
|
||||
request = ReviewChangesRequest(path="/absolute/path/no-git")
|
||||
request = PrecommitRequest(path="/absolute/path/no-git")
|
||||
result = await tool.prepare_prompt(request)
|
||||
|
||||
assert result == "No git repositories found in the specified path."
|
||||
mock_find_repos.assert_called_once_with("/absolute/path/no-git", 5)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.find_git_repositories")
|
||||
@patch("tools.review_changes.get_git_status")
|
||||
@patch("tools.review_changes.run_git_command")
|
||||
@patch("tools.precommit.find_git_repositories")
|
||||
@patch("tools.precommit.get_git_status")
|
||||
@patch("tools.precommit.run_git_command")
|
||||
async def test_no_changes_found(self, mock_run_git, mock_status, mock_find_repos, tool):
|
||||
"""Test when repositories have no changes"""
|
||||
mock_find_repos.return_value = ["/test/repo"]
|
||||
@@ -89,15 +89,15 @@ class TestReviewChangesTool:
|
||||
(True, ""), # unstaged files (empty)
|
||||
]
|
||||
|
||||
request = ReviewChangesRequest(path="/absolute/repo/path")
|
||||
request = PrecommitRequest(path="/absolute/repo/path")
|
||||
result = await tool.prepare_prompt(request)
|
||||
|
||||
assert result == "No pending changes found in any of the git repositories."
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.find_git_repositories")
|
||||
@patch("tools.review_changes.get_git_status")
|
||||
@patch("tools.review_changes.run_git_command")
|
||||
@patch("tools.precommit.find_git_repositories")
|
||||
@patch("tools.precommit.get_git_status")
|
||||
@patch("tools.precommit.run_git_command")
|
||||
async def test_staged_changes_review(
|
||||
self,
|
||||
mock_run_git,
|
||||
@@ -126,7 +126,7 @@ class TestReviewChangesTool:
|
||||
(True, ""), # unstaged files (empty)
|
||||
]
|
||||
|
||||
request = ReviewChangesRequest(
|
||||
request = PrecommitRequest(
|
||||
path="/absolute/repo/path",
|
||||
original_request="Add hello message",
|
||||
review_type="security",
|
||||
@@ -143,9 +143,9 @@ class TestReviewChangesTool:
|
||||
assert "## Git Diffs" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.find_git_repositories")
|
||||
@patch("tools.review_changes.get_git_status")
|
||||
@patch("tools.review_changes.run_git_command")
|
||||
@patch("tools.precommit.find_git_repositories")
|
||||
@patch("tools.precommit.get_git_status")
|
||||
@patch("tools.precommit.run_git_command")
|
||||
async def test_compare_to_invalid_ref(self, mock_run_git, mock_status, mock_find_repos, tool):
|
||||
"""Test comparing to an invalid git ref"""
|
||||
mock_find_repos.return_value = ["/test/repo"]
|
||||
@@ -156,14 +156,14 @@ class TestReviewChangesTool:
|
||||
(False, "fatal: not a valid ref"), # rev-parse fails
|
||||
]
|
||||
|
||||
request = ReviewChangesRequest(path="/absolute/repo/path", compare_to="invalid-branch")
|
||||
request = PrecommitRequest(path="/absolute/repo/path", compare_to="invalid-branch")
|
||||
result = await tool.prepare_prompt(request)
|
||||
|
||||
# When all repos have errors and no changes, we get this message
|
||||
assert "No pending changes found in any of the git repositories." in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.ReviewChanges.execute")
|
||||
@patch("tools.precommit.Precommit.execute")
|
||||
async def test_execute_integration(self, mock_execute, tool):
|
||||
"""Test execute method integration"""
|
||||
# Mock the execute to return a standardized response
|
||||
@@ -183,9 +183,9 @@ class TestReviewChangesTool:
|
||||
assert tool.get_default_temperature() == TEMPERATURE_ANALYTICAL
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.find_git_repositories")
|
||||
@patch("tools.review_changes.get_git_status")
|
||||
@patch("tools.review_changes.run_git_command")
|
||||
@patch("tools.precommit.find_git_repositories")
|
||||
@patch("tools.precommit.get_git_status")
|
||||
@patch("tools.precommit.run_git_command")
|
||||
async def test_mixed_staged_unstaged_changes(
|
||||
self,
|
||||
mock_run_git,
|
||||
@@ -211,7 +211,7 @@ class TestReviewChangesTool:
|
||||
(True, "diff --git a/file2.py..."), # diff for file2.py
|
||||
]
|
||||
|
||||
request = ReviewChangesRequest(
|
||||
request = PrecommitRequest(
|
||||
path="/absolute/repo/path",
|
||||
focus_on="error handling",
|
||||
severity_filter="high",
|
||||
@@ -225,10 +225,10 @@ class TestReviewChangesTool:
|
||||
assert "Reviewing: staged and unstaged changes" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.find_git_repositories")
|
||||
@patch("tools.review_changes.get_git_status")
|
||||
@patch("tools.review_changes.run_git_command")
|
||||
@patch("tools.review_changes.read_files")
|
||||
@patch("tools.precommit.find_git_repositories")
|
||||
@patch("tools.precommit.get_git_status")
|
||||
@patch("tools.precommit.run_git_command")
|
||||
@patch("tools.precommit.read_files")
|
||||
async def test_files_parameter_with_context(
|
||||
self,
|
||||
mock_read_files,
|
||||
@@ -257,7 +257,7 @@ class TestReviewChangesTool:
|
||||
# Mock read_files
|
||||
mock_read_files.return_value = "=== FILE: config.py ===\nCONFIG_VALUE = 42\n=== END FILE ==="
|
||||
|
||||
request = ReviewChangesRequest(
|
||||
request = PrecommitRequest(
|
||||
path="/absolute/repo/path",
|
||||
files=["/absolute/repo/path/config.py"],
|
||||
)
|
||||
@@ -271,9 +271,9 @@ class TestReviewChangesTool:
|
||||
assert "CONFIG_VALUE = 42" in result
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@patch("tools.review_changes.find_git_repositories")
|
||||
@patch("tools.review_changes.get_git_status")
|
||||
@patch("tools.review_changes.run_git_command")
|
||||
@patch("tools.precommit.find_git_repositories")
|
||||
@patch("tools.precommit.get_git_status")
|
||||
@patch("tools.precommit.run_git_command")
|
||||
async def test_files_request_instruction(
|
||||
self,
|
||||
mock_run_git,
|
||||
@@ -298,7 +298,7 @@ class TestReviewChangesTool:
|
||||
]
|
||||
|
||||
# Request without files
|
||||
request = ReviewChangesRequest(path="/absolute/repo/path")
|
||||
request = PrecommitRequest(path="/absolute/repo/path")
|
||||
result = await tool.prepare_prompt(request)
|
||||
|
||||
# Should include instruction for requesting files
|
||||
@@ -306,7 +306,7 @@ class TestReviewChangesTool:
|
||||
assert "standardized JSON response format" in result
|
||||
|
||||
# Request with files - should not include instruction
|
||||
request_with_files = ReviewChangesRequest(path="/absolute/repo/path", files=["/some/file.py"])
|
||||
request_with_files = PrecommitRequest(path="/absolute/repo/path", files=["/some/file.py"])
|
||||
|
||||
# Need to reset mocks for second call
|
||||
mock_find_repos.return_value = ["/test/repo"]
|
||||
@@ -317,7 +317,7 @@ class TestReviewChangesTool:
|
||||
]
|
||||
|
||||
# Mock read_files to return empty (file not found)
|
||||
with patch("tools.review_changes.read_files") as mock_read:
|
||||
with patch("tools.precommit.read_files") as mock_read:
|
||||
mock_read.return_value = ""
|
||||
result_with_files = await tool.prepare_prompt(request_with_files)
|
||||
|
||||
@@ -12,9 +12,9 @@ import pytest
|
||||
|
||||
from tools.analyze import AnalyzeTool
|
||||
from tools.chat import ChatTool
|
||||
from tools.codereview import CodeReviewTool
|
||||
from tools.debug import DebugIssueTool
|
||||
from tools.review_changes import ReviewChanges
|
||||
from tools.review_code import ReviewCodeTool
|
||||
from tools.precommit import Precommit
|
||||
from tools.think_deeper import ThinkDeeperTool
|
||||
|
||||
|
||||
@@ -105,9 +105,9 @@ class TestPromptRegression:
|
||||
assert "deeper analysis" in output["content"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_code_normal_review(self, mock_model_response):
|
||||
"""Test review_code tool with normal inputs."""
|
||||
tool = ReviewCodeTool()
|
||||
async def test_codereview_normal_review(self, mock_model_response):
|
||||
"""Test codereview tool with normal inputs."""
|
||||
tool = CodeReviewTool()
|
||||
|
||||
with patch.object(tool, "create_model") as mock_create_model:
|
||||
mock_model = MagicMock()
|
||||
@@ -117,7 +117,7 @@ class TestPromptRegression:
|
||||
mock_create_model.return_value = mock_model
|
||||
|
||||
# Mock file reading
|
||||
with patch("tools.review_code.read_files") as mock_read_files:
|
||||
with patch("tools.codereview.read_files") as mock_read_files:
|
||||
mock_read_files.return_value = "def main(): pass"
|
||||
|
||||
result = await tool.execute(
|
||||
@@ -137,7 +137,7 @@ class TestPromptRegression:
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_changes_normal_request(self, mock_model_response):
|
||||
"""Test review_changes tool with normal original_request."""
|
||||
tool = ReviewChanges()
|
||||
tool = Precommit()
|
||||
|
||||
with patch.object(tool, "create_model") as mock_create_model:
|
||||
mock_model = MagicMock()
|
||||
@@ -147,8 +147,8 @@ class TestPromptRegression:
|
||||
mock_create_model.return_value = mock_model
|
||||
|
||||
# Mock git operations
|
||||
with patch("tools.review_changes.find_git_repositories") as mock_find_repos:
|
||||
with patch("tools.review_changes.get_git_status") as mock_git_status:
|
||||
with patch("tools.precommit.find_git_repositories") as mock_find_repos:
|
||||
with patch("tools.precommit.get_git_status") as mock_git_status:
|
||||
mock_find_repos.return_value = ["/path/to/repo"]
|
||||
mock_git_status.return_value = {
|
||||
"modified": ["file.py"],
|
||||
|
||||
@@ -20,11 +20,11 @@ class TestServerTools:
|
||||
|
||||
# Check all core tools are present
|
||||
assert "think_deeper" in tool_names
|
||||
assert "review_code" in tool_names
|
||||
assert "codereview" in tool_names
|
||||
assert "debug" in tool_names
|
||||
assert "analyze" in tool_names
|
||||
assert "chat" in tool_names
|
||||
assert "review_changes" in tool_names
|
||||
assert "precommit" in tool_names
|
||||
assert "get_version" in tool_names
|
||||
|
||||
# Should have exactly 7 tools
|
||||
|
||||
@@ -7,8 +7,8 @@ from unittest.mock import Mock, patch
|
||||
import pytest
|
||||
|
||||
from tools.analyze import AnalyzeTool
|
||||
from tools.codereview import CodeReviewTool
|
||||
from tools.debug import DebugIssueTool
|
||||
from tools.review_code import ReviewCodeTool
|
||||
from tools.think_deeper import ThinkDeeperTool
|
||||
|
||||
|
||||
@@ -27,7 +27,7 @@ class TestThinkingModes:
|
||||
tools = [
|
||||
(ThinkDeeperTool(), "high"),
|
||||
(AnalyzeTool(), "medium"),
|
||||
(ReviewCodeTool(), "medium"),
|
||||
(CodeReviewTool(), "medium"),
|
||||
(DebugIssueTool(), "medium"),
|
||||
]
|
||||
|
||||
@@ -77,7 +77,7 @@ class TestThinkingModes:
|
||||
)
|
||||
mock_create_model.return_value = mock_model
|
||||
|
||||
tool = ReviewCodeTool()
|
||||
tool = CodeReviewTool()
|
||||
result = await tool.execute(
|
||||
{
|
||||
"files": ["/absolute/path/test.py"],
|
||||
|
||||
@@ -7,7 +7,7 @@ from unittest.mock import Mock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from tools import AnalyzeTool, ChatTool, DebugIssueTool, ReviewCodeTool, ThinkDeeperTool
|
||||
from tools import AnalyzeTool, ChatTool, CodeReviewTool, DebugIssueTool, ThinkDeeperTool
|
||||
|
||||
|
||||
class TestThinkDeeperTool:
|
||||
@@ -54,16 +54,16 @@ class TestThinkDeeperTool:
|
||||
assert "Extended analysis" in output["content"]
|
||||
|
||||
|
||||
class TestReviewCodeTool:
|
||||
"""Test the review_code tool"""
|
||||
class TestCodeReviewTool:
|
||||
"""Test the codereview tool"""
|
||||
|
||||
@pytest.fixture
|
||||
def tool(self):
|
||||
return ReviewCodeTool()
|
||||
return CodeReviewTool()
|
||||
|
||||
def test_tool_metadata(self, tool):
|
||||
"""Test tool metadata"""
|
||||
assert tool.get_name() == "review_code"
|
||||
assert tool.get_name() == "codereview"
|
||||
assert "PROFESSIONAL CODE REVIEW" in tool.get_description()
|
||||
assert tool.get_default_temperature() == 0.2
|
||||
|
||||
@@ -214,9 +214,9 @@ class TestAbsolutePathValidation:
|
||||
assert "./relative/path.py" in response["content"]
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_review_code_tool_relative_path_rejected(self):
|
||||
"""Test that review_code tool rejects relative paths"""
|
||||
tool = ReviewCodeTool()
|
||||
async def test_codereview_tool_relative_path_rejected(self):
|
||||
"""Test that codereview tool rejects relative paths"""
|
||||
tool = CodeReviewTool()
|
||||
result = await tool.execute(
|
||||
{
|
||||
"files": ["../parent/file.py"],
|
||||
|
||||
Reference in New Issue
Block a user