Skip to content

Fix whitespace-only bounty sort defaults#470

Open
GHX5T-SOL wants to merge 2 commits into
ramimbo:mainfrom
GHX5T-SOL:406-blank-sort-default
Open

Fix whitespace-only bounty sort defaults#470
GHX5T-SOL wants to merge 2 commits into
ramimbo:mainfrom
GHX5T-SOL:406-blank-sort-default

Conversation

@GHX5T-SOL
Copy link
Copy Markdown
Contributor

@GHX5T-SOL GHX5T-SOL commented May 26, 2026

Summary

  • Treat whitespace-only bounty sort query values as omitted/default newest.
  • Keep non-empty invalid sort values rejected with the existing 400 response.
  • Add API and public page regression coverage for the blank-after-trim sort case.

Bounty #406

Evidence

  • Baseline regression failed before the normalizer change: ./.venv/bin/python -m pytest tests/test_bounty_pages.py::test_bounties_page_and_api_sort_public_rows -q returned 400 for /bounties with a whitespace-only sort query.
  • Live bounty preflight: GET https://api.mrwk.ltclab.site/api/v1/bounties/66/attempts?include_expired=false returned attempts: [].

Validation

  • ./.venv/bin/python -m pytest tests/test_bounty_pages.py::test_bounties_page_and_api_sort_public_rows -q -> 1 passed
  • ./.venv/bin/python -m pytest tests/test_bounty_pages.py tests/test_bounty_api_routes.py -q -> 14 passed
  • ./.venv/bin/python -m pytest -q -> 414 passed
  • ./.venv/bin/python -m ruff check . -> passed
  • ./.venv/bin/python -m ruff format --check . -> 79 files already formatted
  • ./.venv/bin/python -m mypy app -> success
  • git diff --check -> clean
  • gitleaks detect --no-banner --redact --source . --log-opts upstream/main..HEAD --exit-code 1 -> no leaks found

Summary by CodeRabbit

  • Bug Fixes

    • Whitespace-only sort parameters now default to "Newest first" instead of causing a validation error, so bounties sort correctly when extra spaces are included.
  • Tests

    • Added API and UI tests to ensure whitespace-only sort values behave like the default "Newest first" ordering.

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: f832c9dd-d319-433f-beda-82454d7418c0

📥 Commits

Reviewing files that changed from the base of the PR and between 44f90fe and e99f857.

📒 Files selected for processing (1)
  • tests/test_bounty_pages.py

📝 Walkthrough

Walkthrough

The normalize_bounty_sort function now strips and lowercases input and returns "newest" for empty or whitespace-only values instead of raising ValueError. New tests verify both API and page routes handle whitespace-only sort parameters the same way.

Changes

Whitespace-only sort parameter handling

Layer / File(s) Summary
Core normalization logic
app/bounty_sorting.py
normalize_bounty_sort strips and lowercases the sort parameter and explicitly returns "newest" when the normalized value is empty.
API and page sort parameter tests
tests/test_bounty_pages.py
Test coverage for whitespace-only sort query parameters verifies both /api/v1/bounties?sort= and /bounties?sort= resolve to the default "newest" ordering and that the UI selects "Newest first".

Possibly related PRs

  • ramimbo/mergework#443: Introduced normalize_bounty_sort and the sort parameter plumbing for both API and page routes; this PR refines the whitespace/empty input handling.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concretely names the changed surface: normalizing whitespace-only bounty sort query values to the default.
Description check ✅ Passed The description covers the required template sections: summary of the fix, evidence of the problem and validation, and all test/tool results provided.
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 changes limited to bounty sorting logic and tests only. No README, docs, or public comments modified. No investment, price, cash-out, or security claims present.
Bounty Pr Focus ✅ Passed PR diff matches stated files and summary. Whitespace-only sort normalized to "newest", invalid values still rejected 400. API and UI tests added. Scope focused on Bounty #406.

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


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 04b9fcb3-38e8-45f2-be79-fab0bf9e81b8

📥 Commits

Reviewing files that changed from the base of the PR and between d8532d4 and 44f90fe.

📒 Files selected for processing (2)
  • app/bounty_sorting.py
  • tests/test_bounty_pages.py

Comment thread tests/test_bounty_pages.py
Copy link
Copy Markdown

@weilixiong weilixiong left a comment

Choose a reason for hiding this comment

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

LGTM — whitespace-only sort defaults, well tested.

Verified:

  • python -m py_compile app/bounty_sorting.py: OK
  • pytest tests/test_bounty_pages.py -k bounties_page_and_api_sort: 1/1 passed (1.71s)
  • git diff: 2 files, +17/-1 lines
  • Whitespace " " correctly falls through to "newest" default
  • Test covers: API whitespace sort → "newest", page HTML whitespace sort → "newest" selected, page sort order correct

Copy link
Copy Markdown

@jtc268 jtc268 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 #470 at e99f857.

Evidence checked:

  • Inspected app/bounty_sorting.py: normalize_bounty_sort now trims/lowercases, defaults None/empty/whitespace to newest, and still rejects non-empty invalid values through BOUNTY_SORT_ERROR.
    • Traced callers in app/bounty_api.py and app/public_routes.py so the same normalization reaches /api/v1/bounties and /bounties.
      • Inspected tests/test_bounty_pages.py: whitespace-only sort now covers API order plus public page order and selected option.
        • Checked the prior CodeRabbit inline thread; the missing order assertion is present on the latest head and the thread is resolved.
          • Current GitHub checks are green: CI success and CodeRabbit success.
            Local validation:
  • ./.venv/bin/python -m pytest tests/test_bounty_pages.py::test_bounties_page_and_api_sort_public_rows tests/test_public_routes.py -q -> 2 passed
    • ./.venv/bin/python -m ruff check app/bounty_sorting.py tests/test_bounty_pages.py -> passed
      • ./.venv/bin/python -m mypy app -> success
        • git diff --check origin/main...HEAD -> clean
          No requested changes from me.

Copy link
Copy Markdown

@rebel117 rebel117 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 #470 at current head for Bounty #447.

Checked files: app/bounty_sorting.py, tests/test_bounty_pages.py.

Evidence:

  • Inspected the diff: the old code (sort or "newest").strip().lower() would return "newest" for None but would pass a whitespace-only string through to the validation check (since .strip() turns " " to "", and "" not in BOUNTY_SORT_OPTIONS would raise ValueError).
  • The new code splits into: default empty string → strip → if empty, return "newest" → otherwise validate against options. This correctly treats whitespace-only sort as "use default" rather than rejecting it.
  • Confirmed the test additions: API route returns 200 with default ordering for sort=" ", HTML page renders with "newest" selected option.
  • Cross-referenced with BOUNTY_SORT_OPTIONS in app/bounty_sorting.py to confirm the valid set is unchanged.
  • Checked that the test uses params={"sort": " "} which preserves the whitespace through query-string parsing.

Verdict: Small but correct fix. Whitespace-only sort values should fall back to default rather than error — this matches how most query parameter handlers work. Tests cover both API and HTML paths. No blockers.

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