Skip to content

fix: honor MCP bounty sort argument#468

Open
Luzijano wants to merge 2 commits into
ramimbo:mainfrom
Luzijano:fix/mcp-list-bounties-sort
Open

fix: honor MCP bounty sort argument#468
Luzijano wants to merge 2 commits into
ramimbo:mainfrom
Luzijano:fix/mcp-list-bounties-sort

Conversation

@Luzijano
Copy link
Copy Markdown

@Luzijano Luzijano commented May 26, 2026

Summary

  • Honor the sort argument for MCP list_bounties calls using the shared bounty sorting helper
  • Update the MCP tool description to document the sort filter
  • Add coverage proving MCP reward sorting differs from default newest sorting

Test plan

  • uv run --extra dev python -m pytest tests/test_api_mcp.py::test_mcp_list_bounties_honors_sort_argument -q
  • uv run --extra dev python -m pytest tests/test_api_mcp.py -q
  • uv run --extra dev ruff check .
  • uv run --extra dev python -m pytest -q

Summary by CodeRabbit

  • New Features
    • Added an optional sort filter for the bounties list so users can order results (e.g., by reward or newest) and combine sorting with limit.
  • Documentation
    • Updated the bounties tool description to reference the new sort option alongside existing filters.
  • Tests
    • Added tests to verify sorting behavior, sort+limit interaction, and invalid sort handling.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 7019a1d4-cf4d-4379-9d1b-f9675ec26c84

📥 Commits

Reviewing files that changed from the base of the PR and between 77462a7 and 7f3ecca.

📒 Files selected for processing (2)
  • app/mcp_tools.py
  • tests/test_api_mcp.py

📝 Walkthrough

Walkthrough

Adds optional sort support to the MCP list_bounties tool: metadata updated, sorting helpers imported, handler parses sort and either applies SQL limit for "newest" or sorts in Python for other values, and tests updated/added to validate behavior.

Changes

Add sort parameter to list_bounties MCP tool

Layer / File(s) Summary
Sort support declaration and imports
app/mcp.py, app/mcp_tools.py
Tool description updated to document the optional sort filter; normalize_bounty_sort and sort_bounties imported.
Sort implementation in list_bounties handler
app/mcp_tools.py
Handler parses optional sort; for sort == "newest" applies SQL LIMIT and returns results; for other sorts fetches matching bounties, sorts with sort_bounties in Python, then truncates to limit.
Sort feature tests
tests/test_api_mcp.py
Tool-listing expectation updated to include sort; new test test_mcp_list_bounties_honors_sort_argument checks ordering by reward and combination with limit; invalid sort added to invalid-args parametrization.

Possibly related PRs

🚥 Pre-merge checks | ✅ 5 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the main changes and test plan, but is missing key template sections like Evidence (confusion addressed, bounty capacity check, intended surfaces, expected size) and the MRWK bounty/issue reference. Add the missing Evidence section detailing the problem being solved, surfaces affected, and expected PR size. Include a Related bounty/issue reference.
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix: honor MCP bounty sort argument' is concrete and directly names the primary change being made.
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 PR adds MCP bounty sort functionality with no investment, price, cash-out, fabricated payout, or security claims. Code changes are purely technical updates to tool descriptions and sorting logic.
Bounty Pr Focus ✅ Passed PR scope is tightly bounded to MCP bounty sort feature across three files with no scope creep; tool description updated and comprehensive tests verify sort functionality.

✏️ 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

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

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

📥 Commits

Reviewing files that changed from the base of the PR and between d8532d4 and 77462a7.

📒 Files selected for processing (3)
  • app/mcp.py
  • app/mcp_tools.py
  • tests/test_api_mcp.py

Comment thread app/mcp_tools.py
Comment thread tests/test_api_mcp.py
Copy link
Copy Markdown
Contributor

@GHX5T-SOL GHX5T-SOL left a comment

Choose a reason for hiding this comment

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

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, and tests/test_api_mcp.py.
  • Confirmed the base list_bounties path ignored sort and always returned Bounty.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 passed
  • uv run --extra dev python -m pytest tests/test_api_mcp.py -q -> 77 passed
  • uv run --extra dev python -m pytest -q -> 415 passed
  • uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • uv run --extra dev ruff check . -> passed
  • uv run --extra dev ruff format --check . -> 79 files already formatted
  • uv run --extra dev mypy app -> success
  • git diff --check origin/main...HEAD -> clean
  • gitleaks 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.

Copy link
Copy Markdown

@er1c-cartman er1c-cartman left a comment

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
Contributor

@GHX5T-SOL GHX5T-SOL 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 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 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_api_mcp.py -q -> 78 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest -q -> 416 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • uv run --extra dev ruff check app/mcp.py app/mcp_tools.py tests/test_api_mcp.py -> passed
  • uv run --extra dev ruff format --check app/mcp.py app/mcp_tools.py tests/test_api_mcp.py -> 3 files already formatted
  • uv run --extra dev python -m mypy app -> success
  • git diff --check origin/main...HEAD -> clean
  • gitleaks 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants