Honor MCP bounty sort filter#500
Conversation
Bounty ramimbo#427: app/mcp.py
Bounty ramimbo#427: app/mcp_tools.py
Bounty ramimbo#427: tests/test_api_mcp.py
📝 WalkthroughWalkthroughThe ChangesSort filter for list_bounties
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/test_api_mcp.py (1)
352-423:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStrengthen this regression test so it actually detects ignored
sort.Right now the fixture makes
high_pool_bountyboth newer (higherid) and higher-available, sosort="available"and default ordering return the same top result. This test can pass even if sort forwarding is broken.Proposed test adjustment
- open_bounty = create_bounty( - session, - repo="ramimbo/mergework", - issue_number=284, - issue_url="https://github.com/ramimbo/mergework/issues/284", - title="Agent MCP workflow filters", - reward_mrwk="100", - acceptance="Agents should find open MCP bounty workflow work.", - ) high_pool_bounty = create_bounty( session, repo="ramimbo/mergework", issue_number=287, issue_url="https://github.com/ramimbo/mergework/issues/287", title="Available pool workflow filters", reward_mrwk="25", max_awards=8, acceptance="Agents should find the largest available open bounty pool.", ) + open_bounty = create_bounty( + session, + repo="ramimbo/mergework", + issue_number=284, + issue_url="https://github.com/ramimbo/mergework/issues/284", + title="Agent MCP workflow filters", + reward_mrwk="100", + acceptance="Agents should find open MCP bounty workflow work.", + ) @@ - assert [item["id"] for item in default_payload] == [high_pool_bounty.id, open_bounty.id] + assert [item["id"] for item in default_payload] == [open_bounty.id, high_pool_bounty.id]As per coding guidelines:
tests/**/*.py: “Do not request docstrings. Focus on whether tests prove the changed behavior and include negative, replay, boundary, or regression cases where relevant.”
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d78bc9e6-3d0f-4e8b-b76b-be488d5b0af8
📒 Files selected for processing (3)
app/mcp.pyapp/mcp_tools.pytests/test_api_mcp.py
|
Closing this as a duplicate after noticing the earlier PR #468 covers the same MCP list_bounties sort path. I do not want to compete with or confuse the existing claim. |
GHX5T-SOL
left a comment
GHX5T-SOL
left a comment
There was a problem hiding this comment.
Reviewed current head 70ecce1858983a8d51a1bb7658feece47e613c2d for the #447 review bounty.
I would not merge or pay this as-is for two concrete reasons:
-
This duplicates the already-open PR #468 on the same surface. PR #468 also updates
app/mcp.py,app/mcp_tools.py, andtests/test_api_mcp.py; it wires MCPlist_bountiesto the sharednormalize_bounty_sort/sort_bountiescontract, updates the same tool description, and adds sort+limit regression coverage. It is earlier, open, non-draft,CLEAN, has successful CI/CodeRabbit, and already has current-head human approvals. PR #500 repeats that MCP sort path instead of adding a distinct fix. -
The new
sort=availableregression does not prove that sort is honored. In this head,high_pool_bountyis created afteropen_bounty, so it is both the highest-available row and the newest row byid. That means thesort=available&limit=1assertion can pass even if the sort argument is ignored and results are just newest-first. To make the regression meaningful, the available-pool winner needs to differ from the default newest winner, or the test should otherwise assert an order that fails under the oldBounty.id.desc().limit(...)behavior.
Validation run locally on this head:
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest 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 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest -q-> 415 passed./.venv/bin/python -m ruff check app/mcp.py app/mcp_tools.py tests/test_api_mcp.py tests/test_mcp_tools.py-> passed./.venv/bin/python -m ruff format --check app/mcp.py app/mcp_tools.py tests/test_api_mcp.py tests/test_mcp_tools.py-> 4 files already formattedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m mypy app-> successPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python scripts/docs_smoke.py-> docs smoke okgit diff --check upstream/main...HEAD-> cleangit diff --no-ext-diff upstream/main...HEAD | gitleaks stdin --no-banner --redact --exit-code 1-> no leaks found
No secrets, private data, wallet material, production mutation, price/liquidity/exchange/off-ramp claims, or private endpoints were used.
2 participants
Bounty #406
Summary:
list_bountiessortargument through the shared bounty sorting contractavailable,reward, andawardsmatch the public API behaviorEvidence:
GET /api/v1/bounties?status=open&sort=available&limit=3returned open bounties ordered by available pool: MRWK bounty: detailed governance MVP PR review #459, MRWK bounty: useful bug reports and small fixes, round 5 #406, MRWK bounty: review open MergeWork PRs with evidence, round 12 #447list_bountieswithstatus=open,sort=available,limit=3ignoredsortand returned newest order: MRWK bounty: detailed governance MVP PR review #459, MRWK bounty: 150 MRWK - document agent-friendly bounty post template #456, MRWK bounty: review open MergeWork PRs with evidence, round 12 #447Verification:
Summary by CodeRabbit
New Features
sortfilter to bounty listing functionality, complementing existing status, search, and limit filters.Tests