Implement version-aware instructions delivery #167
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
WalkthroughThis change introduces version-aware instruction delivery for the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant get_instructions
participant Formatting
Client->>Server: /v1/get_instructions (with protocolVersion)
Server->>get_instructions: Call __get_instructions__(ctx)
get_instructions->>get_instructions: Extract protocolVersion from ctx
alt protocolVersion >= threshold
get_instructions->>get_instructions: Prepare InstructionsData object
get_instructions-->>Server: ToolResponse { data: {}, instructions: InstructionsData }
else protocolVersion < threshold or missing
get_instructions->>Formatting: format_all_instructions_as_xml_strings()
Formatting-->>get_instructions: list of XML strings
get_instructions-->>Server: ToolResponse { data: {}, instructions: list[str] }
end
Server-->>Client: JSON response with data and instructions
Possibly related PRs
✨ 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
blockscout_mcp_server/constants.py (1)
85-86: LGTM! Consider enhancing documentation.The constant is well-named and serves its purpose for version-aware instruction delivery. The date format is appropriate for protocol versioning.
Consider adding a more descriptive comment explaining what this threshold represents:
+# Protocol version threshold for modern MCP client features +# Clients with protocol versions >= this date support structured InstructionsData MODERN_PROTOCOL_VERSION_THRESHOLD = "2025-06-18"tests/formatting/test_xml_formatters.py (1)
14-55: Consider adding more comprehensive test coverage.While these tests verify basic XML structure, consider enhancing them to:
- Verify actual content between tags (not just tag presence)
- Validate well-formed XML structure
- Test that constants like
SERVER_VERSIONare properly includedFor example, enhance the version test:
def test_format_mcp_server_version_xml(): version_xml = format_mcp_server_version_xml() assert version_xml.startswith("<mcp_server_version>") assert version_xml.endswith("</mcp_server_version>") + # Verify actual version is included + from blockscout_mcp_server.constants import SERVER_VERSION + assert f"<mcp_server_version>{SERVER_VERSION}</mcp_server_version>" == version_xmlSPEC.md (1)
371-371: Fix markdown formatting for consistency.The static analysis tool flagged emphasis used instead of headings.
-**Protocol Version Detection** +##### Protocol Version Detection - Inspect `ctx.session.client_params.protocolVersion` when available - Versions `>= 2025-06-18` are treated as modern clients - Older or missing versions default to legacy behavior -**Response Format Selection** +##### Response Format Selection 1. **Modern Clients**: return an empty `data` object and place a structured `InstructionsData` in the `instructions` field 2. **Legacy Clients**: return an empty `data` object and provide XML-formatted instruction strings in `instructions`Also applies to: 378-378
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
tests/tools/test_get_instructions.py (2)
82-94: Legacy client test needs more comprehensive validation.While the test correctly sets up a legacy client scenario with an older
protocolVersion, the assertions are minimal and don't verify the complete structure of the XML instruction strings.Consider enhancing the test to validate more aspects of the legacy response:
@pytest.mark.asyncio async def test_get_instructions_legacy_client(mock_ctx): """Legacy clients receive XML instruction strings.""" mock_ctx.session = MagicMock() mock_ctx.session.client_params = MagicMock() mock_ctx.session.client_params.protocolVersion = "2024-01-01" result = await __get_instructions__(ctx=mock_ctx) assert isinstance(result.data, EmptyData) assert isinstance(result.instructions, list) + assert len(result.instructions) > 0 assert any("<error_handling_rules>" in s for s in result.instructions) + assert any("<chain_id_guidance>" in s for s in result.instructions) + assert any("<pagination_rules>" in s for s in result.instructions) + + # Verify progress tracking + assert mock_ctx.report_progress.call_count == 2
96-104: REST API test could be more explicit about the scenario.The test correctly validates that clients without a
protocolVersionparameter receive XML instruction strings, but the test name and documentation could be clearer about what "REST API" means in this context.Consider improving the test clarity:
@pytest.mark.asyncio -async def test_get_instructions_rest_api(mock_ctx): - """No protocol version behaves like legacy client.""" +async def test_get_instructions_no_protocol_version(mock_ctx): + """Clients without protocolVersion parameter receive XML instruction strings like legacy clients.""" result = await __get_instructions__(ctx=mock_ctx) assert isinstance(result.data, EmptyData) assert isinstance(result.instructions, list) assert any("<chain_id_guidance>" in s for s in result.instructions) + + # Verify progress tracking + assert mock_ctx.report_progress.call_count == 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
blockscout_mcp_server/models.py(2 hunks)blockscout_mcp_server/tools/get_instructions.py(5 hunks)tests/api/test_routes.py(2 hunks)tests/tools/test_get_instructions.py(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- blockscout_mcp_server/models.py
- tests/api/test_routes.py
- blockscout_mcp_server/tools/get_instructions.py
🧰 Additional context used
📓 Path-based instructions (3)
**/*.py
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/000-role-and-task.mdc
- .cursor/rules/010-implementation-rules.mdc
- .cursor/rules/300-ruff-lint-and-format.mdc
tests/tools/test_*.py
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/200-development-testing-workflow.mdc
tests/tools/*.py
Instructions used from:
Sources:
📄 CodeRabbit Inference Engine
- .cursor/rules/210-unit-testing-guidelines.mdc
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
PR: blockscout/mcp-server#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-04T20:16:41.182Z
Learning: Read the project technical description in `SPEC.md` to clearly understand how the MCP server communicates with counterparties and important aspects of the server implementation before responding to a user request
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/130-version-management.mdc:0-0
Timestamp: 2025-07-04T20:25:11.277Z
Learning: Follow semantic versioning (MAJOR.MINOR.PATCH) when choosing version numbers for the MCP server.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-04T20:16:41.182Z
Learning: Read `.cursor/rules/110-new-mcp-tool.mdc` for instructions on adding a new MCP tool to the server or modifying an existing one before responding to a user request
Learnt from: akolotov
PR: blockscout/mcp-server#142
File: blockscout_mcp_server/api/routes.py:242-258
Timestamp: 2025-07-10T19:21:50.271Z
Learning: The user akolotov prefers a unified tool registration approach where a single function registers tools in both MCP and REST API instead of having separate registration points in blockscout_mcp_server/server.py and blockscout_mcp_server/api/routes.py. This would eliminate duplication and reduce maintenance burden.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/130-version-management.mdc:0-0
Timestamp: 2025-07-04T20:25:11.277Z
Learning: Applies to blockscout_mcp_server/__init__.py : When updating the version of the MCP server, update the `__version__` variable in `blockscout_mcp_server/__init__.py`.
tests/tools/test_get_instructions.py (19)
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/200-development-testing-workflow.mdc:0-0
Timestamp: 2025-07-04T20:28:28.716Z
Learning: Applies to tests/integration/test_*_integration.py : Add integration tests in tests/integration/test_{tool_module}_integration.py when adding or modifying tool functions that interact with live APIs
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : When testing tools that transform a list of items, programmatically generate the `expected_result` from the `mock_api_response` instead of writing out large, repetitive expected results.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/200-development-testing-workflow.mdc:0-0
Timestamp: 2025-07-04T20:28:28.716Z
Learning: Applies to tests/integration/test_common_helpers.py : Add integration tests in tests/integration/test_common_helpers.py when modifying helper functions in tools/common.py
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : Ensure all external API calls are properly mocked using `unittest.mock.patch` and `AsyncMock`.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/220-integration-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:31:56.480Z
Learning: Applies to tests/integration/test_*_integration.py : Tool-level integration tests targeting high-level MCP tool functions (e.g., get_latest_block, get_tokens_by_address) must be located in files matching tests/integration/test_*_integration.py and should validate data extraction and schema against live API responses.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : For tools returning a `ToolResponse` object, do not parse JSON from string results in your test; instead, mock the serialization function (`json.dumps`) if used internally, and make assertions on the structured `ToolResponse` object and its attributes.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/200-development-testing-workflow.mdc:0-0
Timestamp: 2025-07-04T20:28:28.716Z
Learning: Applies to tests/tools/test_*.py : Create or update the appropriate unit test file when adding or modifying tool functions: tests/tools/test_{tool_module}.py
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/200-development-testing-workflow.mdc:0-0
Timestamp: 2025-07-04T20:28:28.716Z
Learning: Applies to tests/test_server.py : Create or update the unit test file tests/test_server.py when adding or modifying server functionality
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : Group related tests using descriptive class names or clear function naming patterns.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/200-development-testing-workflow.mdc:0-0
Timestamp: 2025-07-04T20:28:28.716Z
Learning: Run integration tests (pytest -m integration) when you have added or modified any existing MCP tool function, modified helper functions in tools/common.py, added or changed any integration test, or changed data extraction or transformation logic
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/110-new-mcp-tool.mdc:0-0
Timestamp: 2025-07-04T20:22:01.665Z
Learning: Applies to blockscout_mcp_server/tools/*.py : All tools MUST return a strongly-typed ToolResponse[YourDataModel] instead of generic ToolResponse[dict].
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : Use the `mock_ctx` pytest fixture from `tests/conftest.py` for mocking the MCP Context object in tests; do not create a manual MagicMock for the context within your test functions.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/110-new-mcp-tool.mdc:0-0
Timestamp: 2025-07-04T20:22:01.665Z
Learning: Applies to blockscout_mcp_server/tools/*.py : Avoid returning ToolResponse[dict] or ToolResponse[Any]; always prefer specific ToolResponse[YourDataModel].
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : For tools using `make_request_with_periodic_progress`, mock the wrapper itself and assert that it was called with the correct arguments (`request_function`, `request_args`, etc.).
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/110-new-mcp-tool.mdc:0-0
Timestamp: 2025-07-04T20:22:01.665Z
Learning: Applies to blockscout_mcp_server/tools/*.py : All tools MUST return a standardized ToolResponse[YourDataModel] object using the build_tool_response helper.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : Always verify the number of calls to `mock_ctx.report_progress` in your assertions to ensure progress tracking is tested.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/110-new-mcp-tool.mdc:0-0
Timestamp: 2025-07-04T20:22:01.665Z
Learning: Applies to blockscout_mcp_server/tools/*.py : For tools that query Blockscout API, use dynamic chain resolution via get_blockscout_base_url and make_blockscout_request.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/210-unit-testing-guidelines.mdc:0-0
Timestamp: 2025-07-04T20:30:09.292Z
Learning: Applies to tests/tools/*.py : Assert that mocked API helper functions (such as `make_blockscout_request`) are called exactly once with the correct `api_path` and `params` in your tests.
Learnt from: CR
PR: blockscout/mcp-server#0
File: .cursor/rules/110-new-mcp-tool.mdc:0-0
Timestamp: 2025-07-04T20:22:01.665Z
Learning: Applies to blockscout_mcp_server/tools/*.py : When making multiple independent API calls, use asyncio.gather with return_exceptions=True and handle exceptions appropriately.
🧬 Code Graph Analysis (1)
tests/tools/test_get_instructions.py (3)
blockscout_mcp_server/models.py (3)
EmptyData(12-15)InstructionsData(60-69)ToolResponse(288-315)blockscout_mcp_server/tools/get_instructions.py (1)
is_modern_protocol_version(30-37)tests/conftest.py (1)
mock_ctx(8-13)
🔇 Additional comments (3)
tests/tools/test_get_instructions.py (3)
1-13: Import statements follow proper structure and include all necessary dependencies.The imports are properly organized at the top of the file and include the new models and helper function needed for the version-aware instruction delivery tests.
17-72: Comprehensive test coverage for modern client behavior with proper mocking.The test correctly validates that modern clients (with
protocolVersion >= "2025-06-18") receive structuredInstructionsDatain theinstructionsfield andEmptyDatain thedatafield. The mocking strategy is thorough, patching all relevant constants and properly setting up the mock context with session parameters. Progress tracking assertions are included as required by the coding guidelines.
74-80: Well-structured unit test for the helper function.The test validates the
is_modern_protocol_versionhelper function with appropriate edge cases including valid dates above/below the threshold,Nonevalues, and invalid date strings. This ensures the version comparison logic works correctly.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Closes #166
https://chatgpt.com/codex/tasks/task_b_6879c87a2e50832388a2022f019da95a
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests