Skip to content

Add structured MCP balance responses#990

Merged
ramimbo merged 1 commit into
ramimbo:mainfrom
Peter7896:peter7896/mcp-selector-response-clarity
Jun 6, 2026
Merged

Add structured MCP balance responses#990
ramimbo merged 1 commit into
ramimbo:mainfrom
Peter7896:peter7896/mcp-selector-response-clarity

Conversation

@Peter7896
Copy link
Copy Markdown
Contributor

@Peter7896 Peter7896 commented Jun 6, 2026

Summary

  • Keeps get_balance text output while adding structuredContent with normalized account, balance_mrwk, and balance_microunits.
  • Adds a small MCP text-result wrapper so text-first responses can expose parsed payloads without changing legacy content.
  • Updates MCP docs and regression coverage for the balance response shape.

Bounty

Bounty #934

Evidence

  • Confusion addressed: get_balance previously returned only human-readable text, forcing tool callers to parse strings for account and balance values.
  • Bounty capacity check: MRWK bounty: 150 MRWK - MCP conformance and agent-facing usability follow-up, round 2 #934 is a live MergeWork bounty with multiple awards available at submission time.
  • Intended files: MCP response serialization, balance tool handling, MCP docs, and focused MCP tests.
  • Expected PR size: small, backward-compatible MCP response clarity change with focused tests and docs.
  • Out of scope: ledger mutation, wallet custody, treasury execution, payouts, admin-token behavior, price claims, bridge, exchange, cash-out behavior, private data, and secrets.

Validation

  • .\.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py tests\test_mcp_tools.py
  • .\.venv\Scripts\python.exe scripts\docs_smoke.py
  • .\.venv\Scripts\python.exe -m ruff check app\mcp.py app\mcp_tools.py app\mcp_results.py tests\test_api_mcp.py
  • .\.venv\Scripts\python.exe -m ruff format --check app\mcp.py app\mcp_tools.py app\mcp_results.py tests\test_api_mcp.py
  • .\.venv\Scripts\python.exe -m mypy app\mcp.py app\mcp_tools.py app\mcp_results.py
  • .\.venv\Scripts\python.exe -m py_compile app\mcp.py app\mcp_tools.py app\mcp_results.py
  • git diff --cached --check
  • PR title/body and staged added-line public identity hygiene scans

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 6, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds structured content support to MCP tool results by introducing an MCPTextResult dataclass, extending the handler contract, and updating the get_balance tool to emit both human-readable text and parsed balance fields via structuredContent.

Changes

MCP Structured Results

Layer / File(s) Summary
MCPTextResult data contract
app/mcp_results.py
Frozen dataclass with required text: str and optional `structured_content: dict[str, Any]
Handler contract and response logic
app/mcp.py
MCPToolHandler return type extended to `str
Tool implementation
app/mcp_tools.py
call_mcp_tool return type updated to include MCPTextResult. get_balance now returns MCPTextResult with text display and structuredContent fields (account, balance_mrwk, balance_microunits) instead of plain string.
Test validation
tests/test_api_mcp.py
Import GENESIS_SUPPLY_MICRO constant. Test assertion updated to verify structuredContent is present and balance_microunits matches expected value.
API documentation
docs/agent-guide.md, docs/api-examples.md
Updated MCP endpoint guidance and get_balance response example to document structuredContent fields alongside legacy text response; clarified fallback to text for human-readable not-found messages.

Possibly related PRs

  • ramimbo/mergework#329: Changes to handle_mcp_request JSON-RPC transport and result shaping in app/mcp.py directly intersect with this PR's MCPToolHandler and _tool_result_response expansion.
  • ramimbo/mergework#731: Earlier changes to _tool_result_response logic that emit result.structuredContent for structured payloads, which this PR extends with the new MCPTextResult handling.
  • ramimbo/mergework#398: Prior extraction of call_mcp_tool dispatcher returning str | dict; this PR extends it to also return and emit MCPTextResult.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concretely summarizes the main change: adding structured content to MCP balance responses.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Mergework Public Artifact Hygiene ✅ Passed No investment, price, cash-out, or payout claims found. PR updates MCP balance docs with proper structuredContent guidance, maintaining existing safeguards.
Bounty Pr Focus ✅ Passed All stated files changed appropriately: MCPTextResult added, MCPToolHandler updated, get_balance returns structured content, tests updated, docs clarified. No scope creep.
Description check ✅ Passed PR description follows the required template with all major sections populated: Summary, Evidence (with confusion addressed, bounty check, intended files, expected size, out-of-scope items), and comprehensive validation steps listed.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@elianguitarra elianguitarra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed current head 1c390e3.

Approved. I inspected the MCP response path for get_balance and verified this keeps the legacy text content while adding explicit structuredContent for balance consumers:

  • MCPTextResult is handled before the existing dict/string result branches, so current JSON-object tools and text-only tools keep their behavior.
  • get_balance still normalizes the requested account and uses format_mrwk(get_balance(...)); the new structured payload mirrors the same normalized account and balance source.
  • The added docs update the existing guidance to stop treating balances as text-only, and the test now asserts account, balance_mrwk, and balance_microunits on the MCP response.
  • This overlaps with older PR #885, but #885 is currently DIRTY and has a changes-requested review for its conflict with current main. This PR is clean against current origin/main.

Validation on this exact head:

  • ..venv\Scripts\python.exe -m pytest tests\test_api_mcp.py tests\test_mcp_tools.py -> 131 passed, 1 existing Starlette/httpx warning.
  • ..venv\Scripts\python.exe scripts\docs_smoke.py -> docs smoke ok.
  • ..venv\Scripts\python.exe -m ruff check app\mcp.py app\mcp_tools.py app\mcp_results.py tests\test_api_mcp.py -> passed.
  • ..venv\Scripts\python.exe -m ruff format --check app\mcp.py app\mcp_tools.py app\mcp_results.py tests\test_api_mcp.py -> 4 files already formatted.
  • ..venv\Scripts\python.exe -m mypy app\mcp.py app\mcp_tools.py app\mcp_results.py -> success.
  • git merge-tree --write-tree origin/main HEAD -> clean tree 13b068e3cc7449f379fbbbb0e054bc45367f4ff5.

Scope reviewed: MCP tool result serialization, get_balance structured output, docs, and tests. I did not exercise or rely on wallet registration/signing, ledger mutation beyond test fixtures, treasury/proposal execution, payout execution, admin-token behavior, private data, bridge/exchange/cash-out behavior, or MRWK price behavior.

@ramimbo ramimbo merged commit e3e42b0 into ramimbo:main Jun 6, 2026
2 checks passed
@ramimbo ramimbo added mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded labels Jun 6, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mrwk:accepted Maintainer accepted for payout mrwk:paid Ledger payment recorded

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants