Advertise MCP balance output schema#1035
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds an Changesget_balance outputSchema and validation
Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
NiXouuuu
left a comment
There was a problem hiding this comment.
Approved current head 3175cd5b2250ff662fe56b74a9eebea645d201f6.
Files inspected: app/mcp.py, app/mcp_tools.py, app/ledger/service.py, tests/test_api_mcp.py, docs/agent-guide.md.
Verdict: approved. The new get_balance outputSchema matches the existing structured runtime payload returned by mcp_tools.py: normalized account, formatted balance_mrwk, and integer balance_microunits. The test update checks the advertised schema and verifies the returned structured content has exactly the required output fields, while docs now point agents to the advertised schema instead of text parsing.
Validation on this exact head:
uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_tools_list_and_call tests/test_api_mcp.py tests/test_mcp_tools.py -q-> 131 passed, 1 existing Starlette/httpx warning.uv run --python 3.12 --extra dev ruff check app/mcp.py tests/test_api_mcp.py docs/agent-guide.md-> passed.uv run --python 3.12 --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py-> 2 files already formatted.uv run --python 3.12 --extra dev mypy app/mcp.py-> success.uv run --python 3.12 --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean tree3f02091ecb220c9aaa0495ec0848d48e12c65179.
GitHub state checked before review: open, non-draft, no human reviews, mergeable=MERGEABLE; mergeStateStatus=UNSTABLE because CodeRabbit was still pending at review time, so maintainers should still wait for final hosted status before merging.
Scope boundaries: read-only review of MCP schema/docs/tests. No ledger mutation, wallet behavior, treasury/proposal execution, payout execution, admin-token APIs, private data, credentials, bridge/exchange/off-ramp/cash-out behavior, or MRWK price/value claim was used.
Guciolek
left a comment
There was a problem hiding this comment.
Tight, well-scoped schema work for get_balance. Three things stand out:
- The
outputSchemaexposes both the human-friendly decimal (balance_mrwkwith the^\d+(?:\.\d{1,6})?$pattern) and the raw integer (balance_microunitswithminimum: 0). That dual representation is the right call for an MCP consumer that wants to do arithmetic without precision loss — microunits for math, decimal string for display. additionalProperties: Falseon the output is the right choice forget_balance: the response is a closed tuple of (account, balance_mrwk, balance_microunits), and rejecting extra fields catches accidental field drift early.- The
requiredlist is explicit and non-empty: structured outputs will reject any response that omits one of the three. Good.
The docs line in agent-guide.md is updated to point at the new outputSchema, which keeps the user-facing text in sync.
The new test assertions in test_api_mcp.py cover the schema shape (additionalProperties: False, the required list, the balance_mrwk pattern, the balance_microunits minimum) and a runtime check that the actual structuredContent returned by get_balance has the same set of keys as the required list. That last assertion is the kind of cross-check I like to see — it ties the schema to the real response.
Verified: additive change, no production logic touched, app/mcp.py is +21 lines, no new imports. ruff check and ruff format --check should pass. Approving.
Bounty #946
Summary
outputSchemaforget_balance, matching the structured balance payload already returned at runtime.account, formattedbalance_mrwk, and rawbalance_microunitsfields for schema-aware agents.Before / after
get_balancereturnedstructuredContent, buttools/listonly advertised the input shape.tools/listadvertises the exact structured output contract while preserving the existing text and structured runtime response.Validation
.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py::test_mcp_tools_list_and_call -q-> 1 passed, 1 existing Starlette/httpx warning.venv\Scripts\python.exe -m pytest tests\test_api_mcp.py tests\test_mcp_tools.py -q-> 131 passed, 1 existing Starlette/httpx warning.venv\Scripts\ruff.exe check app\mcp.py tests\test_api_mcp.py docs\agent-guide.md-> All checks passed.venv\Scripts\ruff.exe format --check app\mcp.py tests\test_api_mcp.py-> 2 files already formatted.venv\Scripts\python.exe scripts\docs_smoke.py-> docs smoke okgit diff --check-> clean, Windows CRLF warning only for docsScope: MCP
get_balancetool metadata, focused tests, and agent docs only. No ledger mutation, wallet custody, payout execution, treasury behavior, admin-token behavior, bridge/exchange/cash-out behavior, MRWK price behavior, private data, or secrets changed.Summary by CodeRabbit
Release Notes
Documentation
get_balancetool, explicitly documenting all response fields, including account identifiers and balance values, along with their validation constraints.Tests
get_balancetool to comprehensively validate the output schema structure, required field definitions, and response data format constraints.