-
Notifications
You must be signed in to change notification settings - Fork 23
Fix get-documents to return JSON instead of Python objects #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Fixes #16: The get-documents tool was returning Python object string representations like '<meilisearch.models.document.DocumentsResults object at 0x...>' instead of proper JSON-formatted data. Changes: - documents.py: Enhanced get_documents() to properly serialize DocumentsResults and Document objects to JSON-compatible dictionaries - server.py: Improved json_serializer to handle Meilisearch model objects and added proper JSON formatting for get-documents tool response - Added default values for offset/limit to prevent None parameter API errors - tests: Added specific test for issue #16 to verify JSON serialization The fix ensures get-documents returns valid JSON with accessible document fields instead of Python object representations. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
WalkthroughThe changes update the handling and serialization of document retrieval in the Meilisearch MCP server. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant DocumentsModule
Client->>Server: Call "get-documents" tool
Server->>DocumentsModule: get_documents(offset, limit, ...)
DocumentsModule-->>Server: DocumentsResults (object)
Server->>Server: Serialize DocumentsResults to JSON
Server-->>Client: Response with JSON-formatted document data
Assessment against linked issues
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/meilisearch_mcp/server.py (1)
21-23: Enhance the json_serializer function with proper error handling.The addition to handle Meilisearch model objects by accessing their
__dict__is correct and addresses the core issue. However, consider adding error handling for edge cases.Consider this improvement for robustness:
# Handle Meilisearch model objects by using their __dict__ if available if hasattr(obj, '__dict__'): - return obj.__dict__ + try: + return obj.__dict__ + except AttributeError: + pass return str(obj)src/meilisearch_mcp/documents.py (1)
32-56: Complex but necessary serialization logic with a minor structural improvement needed.The document serialization logic correctly handles the conversion of Meilisearch model objects to JSON-serializable format. The approach of looking for private attributes containing actual data is pragmatic for handling wrapped objects.
Address the static analysis hint by removing the unnecessary
elseafterreturn:- if hasattr(result, '__dict__'): - result_dict = result.__dict__.copy() - # ... rest of the logic ... - return result_dict - else: - return result + if hasattr(result, '__dict__'): + result_dict = result.__dict__.copy() + # ... rest of the logic ... + return result_dict + return result🧰 Tools
🪛 Pylint (3.3.7)
[refactor] 33-56: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
tests/test_mcp_client.py (1)
320-369: Comprehensive test with minor import optimization needed.The test thoroughly validates the fix by:
- Creating a test index and document
- Verifying the response doesn't contain Python object representations
- Confirming valid JSON structure and content
- Properly parsing and validating the JSON response
Fix the import redefinitions flagged by static analysis:
async def test_get_documents_returns_json_not_python_object(self, server): """Test that get-documents returns JSON-formatted text, not Python object string representation (issue #16)""" - import time + import time test_index = f"test_issue16_{int(time.time() * 1000)}" # Create index and add a test document await simulate_mcp_call(server, "create-index", {"uid": test_index}) test_document = {"id": 1, "title": "Test Document", "content": "Test content"} await simulate_mcp_call(server, "add-documents", { "indexUid": test_index, "documents": [test_document] }) # Wait for indexing - import asyncio await asyncio.sleep(0.5)Since
asyncioandjsonare already imported at module level (lines 10, 12), remove the redundant local imports.🧰 Tools
🪛 Ruff (0.11.9)
335-335: Redefinition of unused
asynciofrom line 10Remove definition:
asyncio(F811)
361-361: Redefinition of unused
jsonfrom line 12Remove definition:
json(F811)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/meilisearch_mcp/documents.py(1 hunks)src/meilisearch_mcp/server.py(2 hunks)tests/test_mcp_client.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/meilisearch_mcp/server.py (1)
src/meilisearch_mcp/documents.py (1)
get_documents(11-59)
tests/test_mcp_client.py (1)
src/meilisearch_mcp/server.py (2)
create_server(27-29)cleanup(524-527)
🪛 Ruff (0.11.9)
tests/test_mcp_client.py
335-335: Redefinition of unused asyncio from line 10
Remove definition: asyncio
(F811)
361-361: Redefinition of unused json from line 12
Remove definition: json
(F811)
🪛 Pylint (3.3.7)
src/meilisearch_mcp/documents.py
[refactor] 33-56: Unnecessary "else" after "return", remove the "else" and de-indent the code inside it
(R1705)
🔇 Additional comments (3)
src/meilisearch_mcp/server.py (1)
324-336: Excellent fix for the JSON serialization issue.The implementation correctly addresses issue #16 by:
- Providing sensible default values for
offsetandlimitto prevent API errors- Using
json.dumps()with the enhancedjson_serializerto ensure proper JSON formatting- Maintaining consistent response format with other tools
The solution ensures that clients receive valid JSON instead of Python object string representations like
<meilisearch.models.document.DocumentsResults object at 0x...>.src/meilisearch_mcp/documents.py (1)
21-30: Good approach to parameter filtering.Building the parameters dictionary dynamically and excluding
Nonevalues is the correct approach to avoid API errors. This prevents passingNonevalues that could cause issues with the Meilisearch API.tests/test_mcp_client.py (1)
308-319: Well-structured test fixture for issue #16.The test class and fixture setup properly isolate the test and ensure cleanup. Good practice using environment variables for configuration.
Summary
Fixes #16: The
get-documentstool was returning Python object string representations instead of proper JSON-formatted data.Problem
The issue reported that
get-documentswas returning:Instead of actual JSON with accessible document fields.
Solution
Core Changes
get_documents()to properly serializeDocumentsResultsandDocumentobjects to JSON-compatible dictionariesjson_serializer()to handle Meilisearch model objects using__dict__attributesjson.dumps()for the get-documents tool responseoffset(0) andlimit(20) to prevent None parameter API errorsTest Coverage
TestIssue16GetDocumentsJsonSerializationwith focused test for this specific issueBefore vs After
Before:
After:
Testing
Test confirms:
Closes #16
🤖 Generated with Claude Code
Summary by CodeRabbit