Extract MCP work-proof guidance helpers#382
Merged
ramimbo merged 3 commits intoMay 26, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughExtract work-proof guidance logic from app/main.py into a new app/mcp_work_proof module providing: work_proof_guidance (text), work_proof_submission_requirements (structured), work_proof_guidance_json (bounty-specific JSON), and generic_work_proof_guidance_json; update main.py imports and add tests validating behaviors. ChangesMCP Work-Proof Guidance Refactoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/mcp_work_proof.py`:
- Around line 42-47: The function work_proof_submission_requirements currently
accepts only can_submit: bool|None which conflates "bounty closed" and "bounty
open but full"; change its signature to accept an explicit availability status
(e.g., availability: Literal['available','full','closed', None] or an enum) and
update its internal branching to use that status to produce distinct
next_actions for 'closed' vs 'full'; update callers such as
work_proof_guidance_json to pass the new availability status instead of a
boolean, and adjust any other call sites that pass can_submit (including the
other referenced usages) so the UI can show "wait for reopen" when closed and
"join waitlist / check later" when full.
- Around line 161-177: The generic JSON schema returned by
generic_work_proof_guidance_json() is missing keys present in
work_proof_guidance_json() which makes the response shape inconsistent; update
generic_work_proof_guidance_json() to include the same keys (add "max_awards",
"awards_paid", "available_mrwk", and "title") with None (or appropriate
None-equivalent) placeholders so both functions return the same key set and
consumers/schema validation remain stable.
In `@tests/test_mcp_work_proof.py`:
- Around line 6-21: Add a parallel test function (e.g.,
test_work_proof_submission_requirements_choose_open_bounty_for_open_state) that
calls work_proof_submission_requirements with the same bounty_id and
issue_number but can_submit=True, then assert reference_formats and
attempt_endpoint still match, assert that the next_actions no longer require the
"choose_open_bounty" action (i.e., there is no action with id
"choose_open_bounty" marked required=True or that action is absent), and assert
that "price claims" is not present in
requirements["public_metadata_must_avoid"]; use the same function name
work_proof_submission_requirements to locate the logic.
- Around line 15-19: The test currently accesses requirements["next_actions"][0]
directly which can raise unclear errors if the list is empty or reordered;
update the assertion in tests/test_mcp_work_proof.py to locate the action by id
(e.g., use next(...) to find the dict where item["id"] == "choose_open_bounty"),
assert that the found action is not None, and then compare its keys/values to
the expected {"id":"choose_open_bounty","required":True,"text":...};
alternatively assert the list length or that
any(item["id"]=="choose_open_bounty" and item["required"] and
item["text"]==expected_text for item in requirements["next_actions"]) to make
the check robust.
- Around line 31-32: The assertions are brittle because they index into
guidance["submission_requirements"]["next_actions"] and guidance["safety_rules"]
directly; change them to robust checks such as asserting the list has expected
length (e.g., len(guidance["submission_requirements"]["next_actions"]) > 0) and
using any(...) to find an element with id "select_bounty" (e.g., any(a.get("id")
== "select_bounty" for a in
guidance["submission_requirements"]["next_actions"])), and similarly use
any("private keys" in s for s in guidance["safety_rules"]) to confirm presence
rather than relying on [0].
- Line 3: Add tests for the remaining helpers by calling work_proof_guidance and
work_proof_guidance_json and asserting their returned structures contain
expected elements (check keys/ids rather than list positions); add a new case
for work_proof_submission_requirements(..., can_submit=True) and assert the
returned can_submit flag and related fields are correct; replace brittle
position-based checks like next_actions[0] with searches that assert the
presence and properties of an action by its id (e.g., find action where
action["id"] == "<expected_id>") so tests are resilient to ordering.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1fa2f31e-8717-4488-bd95-faf0bc77a0b8
📒 Files selected for processing (3)
app/main.pyapp/mcp_work_proof.pytests/test_mcp_work_proof.py
There was a problem hiding this comment.
♻️ Duplicate comments (1)
tests/test_mcp_work_proof.py (1)
90-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAvoid order-coupled assertion for
next_actions.This assertion is still brittle on list ordering; assert by action
idpresence (or use_action(...)) instead of[0].🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_mcp_work_proof.py` at line 90, The assertion is brittle because it assumes ordering of guidance["submission_requirements"]["next_actions"]; change the check in tests/test_mcp_work_proof.py to assert presence by action id instead of indexing (e.g., verify that an element with id "confirm_award_slot" exists in guidance["submission_requirements"]["next_actions"] or use the helper _action(...) if available), so the test passes regardless of list order.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@tests/test_mcp_work_proof.py`:
- Line 90: The assertion is brittle because it assumes ordering of
guidance["submission_requirements"]["next_actions"]; change the check in
tests/test_mcp_work_proof.py to assert presence by action id instead of indexing
(e.g., verify that an element with id "confirm_award_slot" exists in
guidance["submission_requirements"]["next_actions"] or use the helper
_action(...) if available), so the test passes regardless of list order.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: c0de5168-73bf-4c07-a92b-ababc319984a
📒 Files selected for processing (3)
app/mcp_work_proof.pytests/test_api_mcp.pytests/test_mcp_work_proof.py
tolga-tom-nook
approved these changes
May 26, 2026
Contributor
tolga-tom-nook
left a comment
tolga-tom-nook
left a comment
There was a problem hiding this comment.
Reviewed head f41fa4f for the #340 review bounty.
PASS: I verified the extraction keeps submit_work_proof behavior intact while moving the work-proof guidance boundary into app/mcp_work_proof.py. The PR also appears to have addressed the material schema/availability concerns from the bot review: generic and bounty-specific JSON guidance now share the nullable accounting/title keys, and open/full/closed/unknown submission availability is represented separately in work_proof_submission_requirements().
Evidence checked:
- inspected
app/mcp_work_proof.py,app/main.py, andtests/test_mcp_work_proof.py - confirmed
app/main.pyimports/calls the extracted guidance helpers without route behavior changes - confirmed full bounty/full/open/generic guidance paths have focused tests
Validation run locally:
uv run --extra dev python -m pytest tests/test_mcp_work_proof.py tests/test_api_mcp.py::test_mcp_submit_work_proof_returns_bounty_specific_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_keeps_generic_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_returns_structured_bounty_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_returns_structured_generic_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_terminal_bounty_availability tests/test_api_mcp.py::test_mcp_submit_work_proof_scopes_issue_number_by_repo tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_invalid_bounty_selectors tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_ambiguous_issue_number -q→ 24 passeduv run --extra dev python -m ruff check app/mcp_work_proof.py app/main.py tests/test_mcp_work_proof.py→ passeduv run --extra dev python -m mypy app→ passeduv run --extra dev python -m pytest -q→ 359 passed
No requested changes from my review.
aiautotool
approved these changes
May 26, 2026
aiautotool
left a comment
aiautotool
left a comment
There was a problem hiding this comment.
Reviewed the MCP work-proof guidance extraction against origin/main. The text guidance path remains equivalent, the structured guidance keeps the bounty-specific fields while adding the generic fields covered by tests, and the new availability branch correctly distinguishes closed from open-but-full bounties.\n\nLocal evidence:\n- python -m pytest tests/test_mcp_work_proof.py plus the listed submit_work_proof MCP regression tests -q -> 24 passed\n- python -m ruff check app/mcp_work_proof.py app/main.py tests/test_mcp_work_proof.py tests/test_api_mcp.py -> pass\n- python -m ruff format --check app/mcp_work_proof.py app/main.py tests/test_mcp_work_proof.py tests/test_api_mcp.py -> pass\n- python -m mypy app/main.py app/mcp_work_proof.py -> pass\n- git diff --check origin/main...HEAD -> pass
tolga-tom-nook
approved these changes
May 26, 2026
Contributor
tolga-tom-nook
left a comment
tolga-tom-nook
left a comment
There was a problem hiding this comment.
Follow-up re-review after head changed from f41fa4f to 298fb82.
PASS: the new commit only stabilizes the work-proof guidance test assertion; I rechecked the focused guidance surface and did not find behavior drift.
Validation rerun locally on head 298fb82:
./.venv/bin/python -m pytest tests/test_mcp_work_proof.py tests/test_api_mcp.py::test_mcp_submit_work_proof_returns_bounty_specific_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_keeps_generic_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_returns_structured_bounty_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_returns_structured_generic_guidance tests/test_api_mcp.py::test_mcp_submit_work_proof_structures_terminal_bounty_availability tests/test_api_mcp.py::test_mcp_submit_work_proof_scopes_issue_number_by_repo tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_invalid_bounty_selectors tests/test_api_mcp.py::test_mcp_submit_work_proof_rejects_ambiguous_issue_number -q-> 24 passed./.venv/bin/python -m ruff check app/mcp_work_proof.py app/main.py tests/test_mcp_work_proof.py-> passed./.venv/bin/python -m mypy app-> passed./.venv/bin/python -m pytest -q-> 359 passed
No requested changes from my follow-up review.
ramimbo
added
mrwk:accepted
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #377
Refs #378
Summary
Complexity reduced
app/main.py no longer owns the long nested work-proof guidance builders inside _call_mcp_tool. The MCP tool dispatch now calls a focused module for guidance formatting, so future submit_work_proof changes can be reviewed without expanding the already-large route/app entrypoint.
Evidence
Validation
Summary by CodeRabbit
Refactor
Tests