Gl/improve code base #7
Merged
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors the codebase to introduce a decorator-based tool registration system, simplifying tool definitions and centralizing server initialization. The changes modernize the architecture by replacing manual tool registration with automatic discovery via decorators.
Key Changes
- Introduced
@tooldecorator for automatic tool registration, replacing manualregister_tools()functions across all tool modules - Consolidated server initialization into
src/mcp_register.pywithcreate_mcp_server(), replacing the previoussrc/server.pyapproach - Simplified calculator tool implementations by removing verbose exception handling and logging, introducing helper functions
_ok()and_err()
Reviewed changes
Copilot reviewed 19 out of 22 changed files in this pull request and generated 31 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tools/init.py | Added new @tool decorator and TOOL_REGISTRY for automatic tool discovery |
| src/mcp_register.py | New file containing create_mcp_server() and module auto-discovery logic |
| src/server.py | Deleted - functionality moved to src/mcp_register.py |
| src/main.py | Updated to use new create_mcp_server() function, modified imports and endpoint structure |
| src/tools/local/calculator.py | Simplified implementations with helper functions, removed verbose error handling |
| src/tools/local/local_file_system.py | Added @tool decorators, removed register_tools() function |
| src/tools/local/shell.py | Added @tool decorators, removed register_tools() function |
| src/tools/search/tavily_tool.py | Added @tool decorator, removed register_tools() function |
| src/tools/files/pdf_to_markdown.py | Added @tool decorator, changed return type to dict, added exception handling |
| src/tools/data/pandas.py | Added @tool decorators, removed register_tools() function |
| src/tools/data/csv.py | Added @tool decorators, removed register_tools() function |
| src/adapter/fast_mcp_fast_api_adapter.py | Simplified _construct_url() logic, removed unused imports |
| src/adapter/routes.py | Refactored _get_tool_categories() to return list-based structure, simplified info routes |
| src/adapter/init.py | Added explicit exports for public API |
| tests/adapter/test_routes.py | Updated assertions to match new category structure (list instead of dict with counts) |
| tests/adapter/test_adapter.py | Updated import path from adapter.adapter to adapter.fast_mcp_fast_api_adapter |
| tests_integration/conftest.py | Changed server startup from src.server to uvicorn command |
| tests_integration/graphite/test_simple_function_call_assistant.py | Added assertions to verify output content |
| CLAUDE.md | New documentation file describing architecture and commands |
| .gitignore | Added .uv-cache/ to ignored files |
Comments suppressed due to low confidence (1)
src/adapter/fast_mcp_fast_api_adapter.py:57
- The logic in
_construct_urlappears incorrect. Whentransport == "http", the function returns the base_url immediately (line 52). However, on line 54-55, there's a check for whether the URL already ends with the transport, which would never be reached for "http" transport due to the early return. This creates inconsistent behavior - "http" transport won't append "/http" even if it doesn't exist, but other transports will. Consider whether "http" transport should actually return early or follow the same logic as other transports.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
update calculator factorial exception Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
improve the whole code base