Document get_balance MCP account selectors#891
Conversation
The get_balance MCP tool description was just "Get an account balance", giving clients no guidance on the account selector format. Document the four formats normalized_account accepts: github:<login>, mrwk1 wallet addresses, treasury:mrwk, and reserve:bounty:<id>. Description-only change; no ledger/wallet/treasury behavior is altered.
|
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 (1)
📝 WalkthroughWalkthroughThe PR expands the MCP tool description for Changesget_balance tool documentation
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8e0e1276-a9bf-44c2-abb0-27c65fd06b53
📒 Files selected for processing (2)
app/mcp.pytests/test_api_mcp.py
| assert "github:" in description | ||
| assert "mrwk1" in description | ||
| assert "treasury:mrwk" in description | ||
| assert "reserve:bounty:" in description |
There was a problem hiding this comment.
Strengthen selector-format assertions to match the documented contract.
Line 298 and Line 301 currently assert only prefixes ("github:", "reserve:bounty:"), so the test still passes if <login> or <id> guidance regresses. Assert the full documented tokens.
As per coding guidelines, "tests/**/*.py: ... focus on whether tests prove the changed behavior and include ... regression cases where relevant."
Proposed test tightening
- assert "github:" in description
+ assert "github:<login>" in description
assert "mrwk1" in description
assert "treasury:mrwk" in description
- assert "reserve:bounty:" in description
+ assert "reserve:bounty:<id>" in description
xiefuzheng713-alt
left a comment
There was a problem hiding this comment.
Reviewed current head 5f79b753213da8bb61809225d7527d317051edc2 as a non-author.
The change itself is narrow and the new selector-description assertion passes, but this head still needs a formatting fix before it is merge-ready. The hosted Quality check is failing, and I reproduced the local formatter failure on the changed test file:
uv run --python 3.12 --extra dev ruff format --check app/mcp.py tests/test_api_mcp.py-> fails becausetests/test_api_mcp.pywould be reformatted.uv run --python 3.12 --extra dev ruff format --diff tests/test_api_mcp.pyshows the newbalance_tool = next(...)expression should be on one line.
Other validation I ran:
uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_get_balance_description_documents_account_selectors -q-> 1 passed, 1 existing warning.uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py -q-> 106 passed, 1 existing warning.uv run --python 3.12 --extra dev ruff check app/mcp.py tests/test_api_mcp.py-> passed.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 treeb70268e7d972b0275c35dbc2844d150e3dc904b9.
Requesting changes only for the formatting failure; no MCP behavior blocker found.
xiefuzheng713-alt
left a comment
There was a problem hiding this comment.
Reviewed current head 6f5af6c191202f1638acd0cb0b9d2f9c49770af0 as a non-author after the formatting follow-up.
Evidence checked:
- inspected
app/mcp.pyandtests/test_api_mcp.py; - confirmed
get_balancenow documents the supported account selector formats:github:<login>,mrwk1...,treasury:mrwk, andreserve:bounty:<id>; - confirmed the new tools/list regression covers those selector hints without changing balance lookup behavior;
- confirmed the previous formatter failure from head
5f79b753213da8bb61809225d7527d317051edc2is fixed on this head; - verified GitHub reports
mergeStateStatus=CLEAN, hosted Quality/readiness/docs/image success, and CodeRabbit success.
Validation run on this exact head:
uv run --python 3.12 --extra dev python -m pytest tests/test_api_mcp.py -q-> 106 passed, 1 existing warning.uv run --python 3.12 --extra dev ruff check app/mcp.py tests/test_api_mcp.py-> 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 tree6007e446d909290da0404eeb4fa7a78050039781.
No blocker found. Scope is MCP tool-list description/test coverage only; no ledger, wallet, treasury, payout, admin-token, private data, secrets, exchange, bridge, cash-out, or MRWK price behavior changed.
Summary
get_balanceMCP account selector formats in the tool description.tools/listkeeps exposing selector guidance forgithub:<login>,mrwk1...,treasury:mrwk, andreserve:bounty:<id>.Test Plan
./.venv/bin/python -m pytest tests/test_api_mcp.py./.venv/bin/python -m pytest./.venv/bin/python -m ruff check app/mcp.py tests/test_api_mcp.py./.venv/bin/python -m mypy app/mcp.pyBounty
Caveats / Scope
get_balanceresponse work; it only documents the existing selector formats exposed throughtools/list.This fix was solved by Hermes, Rulislim's autonomous coding agent, on behalf of Rulislim.
Summary by CodeRabbit
Documentation
Tests