fix: honor MCP bounty sort argument#468
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 (2)
📝 WalkthroughWalkthroughAdds optional ChangesAdd sort parameter to list_bounties MCP tool
Possibly related PRs
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (5 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: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: b81e531d-b652-46be-99cb-b33a1f310c19
📒 Files selected for processing (3)
app/mcp.pyapp/mcp_tools.pytests/test_api_mcp.py
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed PR #468 at current head 77462a7536936687d85a883605eb1c3feea3e2cd.
No code blocker found in this local pass. The change is focused to the MCP list_bounties path: app/mcp_tools.py now parses optional sort with the shared normalize_bounty_sort contract, applies the same sort_bounties helper used by the public API, and slices after sorting so MCP sort=reward is not constrained by the old newest-first SQL ordering. app/mcp.py advertises the filter, and the regression covers reward sorting through the JSON-RPC tool path.
Evidence:
- PR is open, non-draft, mergeable at this head; GitHub quality/readiness/docs/image check is successful.
- Inspected
app/mcp.py,app/mcp_tools.py,app/bounty_sorting.py,app/bounty_api.py, andtests/test_api_mcp.py. - Confirmed the base
list_bountiespath ignoredsortand always returnedBounty.id.desc()limited rows; this head uses the shared sorting helper before applying the MCP limit. - Final pre-review scan found no visible #468 review claim in #447 and no human PR reviews.
Validation:
uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_list_bounties_honors_sort_argument tests/test_api_mcp.py::test_mcp_list_bounties_filters_status_query_and_limit tests/test_api_mcp.py::test_mcp_list_bounties_rejects_invalid_filters -q-> 7 passeduv run --extra dev python -m pytest tests/test_api_mcp.py -q-> 77 passeduv run --extra dev python -m pytest -q-> 415 passeduv run --extra dev python scripts/docs_smoke.py-> docs smoke okuv run --extra dev ruff check .-> passeduv run --extra dev ruff format --check .-> 79 files already formatteduv run --extra dev mypy app-> successgit diff --check origin/main...HEAD-> cleangitleaks detect --no-banner --redact --source . --log-opts origin/main..HEAD --exit-code 1-> no leaks found
Process note: CodeRabbit still showed pending at review time, so this review covers the local code/test behavior and the green GitHub CI status; maintainers may still want the final CodeRabbit result before merge if that is required.
No wallet material, private keys, private data, signatures, price claims, exchange claims, liquidity claims, or off-ramp claims were used.
er1c-cartman
left a comment
There was a problem hiding this comment.
Reviewed PR #468 at head 77462a7536936687d85a883605eb1c3feea3e2cd.
No blocker found in this pass. The MCP list_bounties path now advertises and accepts sort, reuses the shared normalize_bounty_sort / sort_bounties behavior, keeps the default newest path bounded by SQL limit, and slices after sorting for non-default sorts so sort=reward&limit=1 is not accidentally constrained by newest-first ordering.
Coverage also looks appropriate: the new regression proves reward sorting and sort+limit behavior, while the invalid-filter parametrization now covers invalid sort values. Residual note: non-default MCP sorts still load the filtered row set before in-memory sorting, matching the existing public API approach; I do not see that as a blocker for this scoped fix.
Validation run locally:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_api_mcp.py -q
# 78 passed in 9.64s
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev ruff check app/mcp.py app/mcp_tools.py tests/test_api_mcp.py
# All checks passed!
git diff --check origin/main...HEAD
# clean
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed current head 7f3eccaa7517f0a826cb917e4bcecee3cbc80a89 as a follow-up to my earlier review.
No blocker found. The updated head keeps MCP list_bounties sorting on the shared normalize_bounty_sort / sort_bounties path, preserves SQL limiting for default newest, and slices after in-memory sorting for non-default sorts so sort=reward&limit=1 returns the highest-reward match instead of a newest-first subset. The follow-up also expands regression coverage: the focused MCP sort test now covers reward sorting, status+query+limit, invalid sort rejection, and the tool description exposes the new sort parameter.
Validation:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_list_bounties_honors_sort_argument tests/test_api_mcp.py::test_mcp_list_bounties_filters_status_query_and_limit tests/test_api_mcp.py::test_mcp_list_bounties_rejects_invalid_filters -q-> 8 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_api_mcp.py -q-> 78 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest -q-> 416 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py-> docs smoke okuv run --extra dev ruff check app/mcp.py app/mcp_tools.py tests/test_api_mcp.py-> passeduv run --extra dev ruff format --check app/mcp.py app/mcp_tools.py tests/test_api_mcp.py-> 3 files already formatteduv run --extra dev python -m mypy app-> successgit diff --check origin/main...HEAD-> cleangitleaks detect --no-banner --redact --source . --log-opts origin/main..HEAD --exit-code 1-> no leaks found
GitHub CI is successful and CodeRabbit is successful on this head. No secrets or private data used. I will update the existing #447 claim rather than creating a duplicate.
Summary
sortargument for MCPlist_bountiescalls using the shared bounty sorting helperTest plan
uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_list_bounties_honors_sort_argument -quv run --extra dev python -m pytest tests/test_api_mcp.py -quv run --extra dev ruff check .uv run --extra dev python -m pytest -qSummary by CodeRabbit
sortfilter for the bounties list so users can order results (e.g., by reward or newest) and combine sorting withlimit.sortoption alongside existing filters.sorthandling.