Skip to content

Simplify bounty sorting key selection#879

Open
mauricemohr88-debug wants to merge 2 commits into
ramimbo:mainfrom
mauricemohr88-debug:codex/bounty-sorting-cleanup-846
Open

Simplify bounty sorting key selection#879
mauricemohr88-debug wants to merge 2 commits into
ramimbo:mainfrom
mauricemohr88-debug:codex/bounty-sorting-cleanup-846

Conversation

@mauricemohr88-debug
Copy link
Copy Markdown

@mauricemohr88-debug mauricemohr88-debug commented Jun 4, 2026

Refs #846

Summary

  • replaces the duplicated sort_bounties() branch bodies with named sort-key helpers and a single key dispatch table
  • adds focused unit coverage for sort normalization and all supported bounty sort modes
  • preserves existing newest/reward/available/awards ordering behavior and tie-breaks by bounty id

Validation

  • uv run --python 3.12 --extra dev python -m pytest tests/test_bounty_sorting.py tests/test_bounty_pages.py::test_bounties_page_and_api_sort_public_rows tests/test_api_mcp.py::test_mcp_list_bounties_honors_sort_argument -q -> 10 passed, 1 existing Starlette/httpx warning
  • uv run --python 3.12 --extra dev ruff check app/bounty_sorting.py tests/test_bounty_sorting.py -> passed
  • uv run --python 3.12 --extra dev ruff format --check app/bounty_sorting.py tests/test_bounty_sorting.py -> 2 files already formatted
  • uv run --python 3.12 --extra dev mypy app/bounty_sorting.py -> success
  • git diff --check -> clean
  • git merge-tree --write-tree origin/main HEAD -> clean tree abda6f4c7b8cc6d19c1ef4bc0fd2711b121cfce1

Scope: maintainability-only cleanup in the shared bounty sorting helper. No public API contract, ledger behavior, wallet behavior, treasury behavior, payout behavior, or bounty lifecycle semantics are changed.

Summary by CodeRabbit

  • Refactor

    • Improved bounty sorting internals for maintainability without changing user-visible sorting behavior.
  • Tests

    • Added comprehensive tests validating sort option handling (including input normalization and rejection of invalid inputs) and ensuring correct ordering across supported sort criteria.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jun 4, 2026

Review Change Stack

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: 878a7c14-04c0-471a-b97d-26f7476d0b68

📥 Commits

Reviewing files that changed from the base of the PR and between 1df5ba1 and cdff05b.

📒 Files selected for processing (1)
  • app/bounty_sorting.py

📝 Walkthrough

Walkthrough

Refactors bounty sorting to use typed per-mode key functions registered in BOUNTY_SORT_KEYS and updates sort_bounties to use that registry; adds tests covering normalize_bounty_sort normalization and sort_bounties ordering for supported modes.

Changes

Bounty Sorting Refactoring

Layer / File(s) Summary
Sorting implementation refactoring
app/bounty_sorting.py
BountySortKey type alias and imports added; per-mode sort key helpers (_newest_sort_key, _reward_sort_key, _available_sort_key, _awards_sort_key) compute comparable keys (Decimal conversions and id tie-breakers) and are registered in BOUNTY_SORT_KEYS. sort_bounties uses normalize_bounty_sort and selects the key from BOUNTY_SORT_KEYS to sort with reverse=True.
Sorting test suite
tests/test_bounty_sorting.py
Parametrized tests validate normalize_bounty_sort canonicalization (None/empty/whitespace/case) and rejection of unsupported/control inputs. Integration test builds bounty rows and asserts sort_bounties returns expected ID orders for default/newest and for "reward", "available", and "awards".

Estimated code review effort

3 (Moderate) — ~25 minutes

Possibly related PRs

  • ramimbo/mergework#468: MCP list_bounties changes depend directly on the updated sorting implementation from this PR.
  • ramimbo/mergework#443: Both PRs focus on the shared bounty sorting implementation and its sort modes; this PR refactors core logic and adds tests.
  • ramimbo/mergework#470: Overlapping changes around whitespace/empty input handling for normalize_bounty_sort.

Suggested labels

mrwk:accepted, mrwk:paid

Suggested reviewers

  • eliasx45
  • weilixiong
  • jtc268
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Simplify bounty sorting key selection' directly names the main refactoring: replacing duplicated branches with a dispatch table of sort-key helpers.
Description check ✅ Passed The description covers summary (refactoring goal and coverage), validation (test commands and tool results), and scope clarification, but omits the template's 'Evidence' section details.
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 contains no investment, price, cash-out, payout, or security claims. Code and summary describe only bounty sorting refactoring logic.
Bounty Pr Focus ✅ Passed Diff matches stated files, maintains public signatures, adds helper functions and registry, includes tests for all sort modes, scope limited to bounty sorting.

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

@xiefuzheng713-alt xiefuzheng713-alt left a comment

Choose a reason for hiding this comment

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

Approved for the current head cdff05b4b8f17e8173334af9b498fbb7d3809173.

I reviewed app/bounty_sorting.py and tests/test_bounty_sorting.py. The refactor keeps the same public sorting contract while moving the duplicated branch bodies into named key helpers: newest still sorts by descending id, reward/available still use Decimal(str(...)) with id tie-breaks, and available/awards still prefer the effective fields with the same fallback fields.

Validation I ran locally:

  • uv run --python 3.12 --extra dev python -m pytest tests/test_bounty_sorting.py tests/test_bounty_pages.py::test_bounties_page_and_api_sort_public_rows tests/test_api_mcp.py::test_mcp_list_bounties_honors_sort_argument -q -> 10 passed, 1 existing Starlette/httpx warning.
  • uv run --python 3.12 --extra dev ruff check app/bounty_sorting.py tests/test_bounty_sorting.py -> passed.
  • uv run --python 3.12 --extra dev ruff format --check app/bounty_sorting.py tests/test_bounty_sorting.py -> 2 files already formatted.
  • uv run --python 3.12 --extra dev mypy app/bounty_sorting.py -> success.
  • git diff --check -> clean.
  • git merge-tree --write-tree origin/main HEAD -> clean tree 6baa80a23af966ca53050e160949780bfebb29ad.

I also checked that this is not my PR, there were no prior human reviews on this head, and the only existing PR comment was the non-actionable CodeRabbit walkthrough. The GitHub Quality check was still in progress when I checked, with no failure observed; the focused local checks cover the changed helper and its public API/page/MCP callers.

@stmr
Copy link
Copy Markdown

stmr commented Jun 4, 2026

Bounty #838 current-head review for PR #879.

Reviewed head cdff05b4b8f17e8173334af9b498fbb7d3809173. Files inspected: app/bounty_sorting.py, tests/test_bounty_sorting.py.

Verdict: no blocker found. The refactor moves each sort mode into named key helpers and a dispatch table, while preserving descending sort semantics and the id tie-breaks for newest, reward, available MRWK, and award slots. The new tests cover normalization, invalid/control-character rejection, and all supported orderings.

GitHub state checked: mergeStateStatus=CLEAN; hosted Quality, readiness, docs, and image checks passed; CodeRabbit passed. Scope is bounty sorting maintainability and tests only. No public API contract, ledger, wallet, treasury, payout, admin-token, private data, bridge, exchange, cash-out, or MRWK price behavior changed.

Copy link
Copy Markdown
Contributor

@MolhamHamwi MolhamHamwi 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 cdff05b for the #838 review bounty. I inspected app/bounty_sorting.py and tests/test_bounty_sorting.py, checked the GitHub PR state, and ran python3 -m pytest tests/test_bounty_sorting.py -q locally: 8 passed.

Functionally the refactor keeps the existing ordering behavior: each extracted key function still uses the same descending sort key and the effective_* fallbacks remain equivalent to the previous inline lambdas. The new parametrized tests cover normalization, invalid/control-character rejection, and the supported sort orders.

However, this branch is currently not merge-ready because GitHub reports mergeable=CONFLICTING / mergeStateStatus=DIRTY against the current base. Please rebase or merge current main and rerun the sorting tests before maintainers consider it ready.

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.

4 participants