Skip to content

Bounty #427: Add wallet row limit control#503

Open
er1c-cartman wants to merge 2 commits into
ramimbo:mainfrom
er1c-cartman:fix-427-wallet-row-limit
Open

Bounty #427: Add wallet row limit control#503
er1c-cartman wants to merge 2 commits into
ramimbo:mainfrom
er1c-cartman:fix-427-wallet-row-limit

Conversation

@er1c-cartman
Copy link
Copy Markdown

@er1c-cartman er1c-cartman commented May 27, 2026

Summary

Bounty #427 / Refs #427.

This adds a focused row-count control to the public /wallets page so contributors can cap the registered-wallet table without losing the wallet generation and registration workflow above it.

  • /wallets?limit=N now accepts a bounded limit=1..200 query parameter.
  • The registered-wallet section includes a Rows selector with 25/50/100/200 presets and preserves custom valid values such as limit=1.
  • Invalid row limits are rejected by FastAPI validation instead of silently falling back.

Before / After

  • Before: /wallets always queried and rendered the newest 100 registered wallets, with no page control for a smaller scan set.
  • After: /wallets?limit=1 renders a single registered-wallet row and marks that selected limit in the form; /wallets?limit=0 returns 422.

Evidence

Preflight for bounty #73 / issue #427 showed status=open, awards_remaining=2, and no active attempts.

Open PR search for wallet row-limit work only found the separate wallet summary-card PR #489, which does not add row controls.

Validation

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_wallet_api.py::test_wallets_page_honors_limit_filter -q -> 1 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_wallet_api.py -q -> 33 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_wallet_api.py tests/test_public_routes.py -q -> 34 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest -q -> 415 passed
  • ./.venv/bin/python -m ruff check app/public_routes.py tests/test_wallet_api.py -> passed
  • ./.venv/bin/python -m ruff format --check app/public_routes.py tests/test_wallet_api.py -> 2 files already formatted
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m mypy app/public_routes.py -> success
  • ./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check -> clean

Safety

No wallet material, private keys, cookies, OAuth state, browser storage, access tokens, signatures, private data, price claims, investment claims, liquidity claims, bridge promises, exchange claims, or fabricated payout claims are included.

Summary by CodeRabbit

  • New Features

    • Public wallets page now lets users choose how many wallets to display (default 100). Preset options: 25, 50, 100, 200; non-preset values within 1–200 are accepted and preserved when submitted.
  • Tests

    • Added tests covering the wallets page limit: valid selections, enforcement of the 1–200 range, and correct row counts when limits are applied.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

Adds a validated limit query parameter to the public /wallets page, threads it through the context to the DB query, renders a limit selector in the template, and adds tests that verify validation and result filtering.

Changes

Wallets page limit pagination

Layer / File(s) Summary
Context function and limit options logic
app/public_routes.py
wallets_page_context(session: Session, limit: int = 100) queries wallets with limit, builds limit_options from presets ([25,50,100,200]) and includes the requested limit if missing; typing imports updated to include Annotated.
Route handler limit parameter validation
app/public_routes.py
/wallets route now accepts limit: Annotated[int, Query(1, 200)] (default 100) and passes it into wallets_page_context.
Template limit selector form
app/templates/wallets.html
Adds a GET form before the wallets table rendering a limit <select> from limit_options, using selected_limit to preserve the current choice.
Test coverage for limit parameter and filtering
tests/test_wallet_api.py
Enhances smoke test to assert default selected 100. Adds test_wallets_page_honors_limit_filter which creates three wallets, verifies GET /wallets?limit=1 shows one wallet and GET /wallets?limit=0 returns 422.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed Title is concrete and names the changed surface: a wallet row limit control feature, directly reflecting the main change across all modified files.
Description check ✅ Passed Description follows the template structure with complete sections: Summary, Evidence, Before/After, Validation, and Safety checks. All required information is present and specific.
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 modifies wallet row limit feature without introducing investment, price, cash-out, off-ramp, fabricated payout claims, or private security details in public artifacts.
Bounty Pr Focus ✅ Passed PR modified only the 3 stated files for Bounty #427. Diff matches feature scope: limit query parameter (1-200 validation), selector form in HTML, and test coverage including boundary cases.

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

@jtc268
Copy link
Copy Markdown

jtc268 commented May 27, 2026

Reviewed current head 9bf431cfa1fd404040be904a3766f005f5fae954.

No blockers found. The change is focused to the public wallet listing: app/public_routes.py binds limit with Query(ge=1, le=200), passes it directly into the registered-wallet query, and returns selected_limit/limit_options for the template. app/templates/wallets.html renders the row-count selector without changing the wallet generation, registration, private-key, or signed-transfer flows. tests/test_wallet_api.py covers the default selected 100, a custom limit=1 render with one wallet row, and the lower-bound 422 case.

Validation I ran on this head:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_wallet_api.py -q -> 33 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest tests/test_wallet_api.py tests/test_public_routes.py -q -> 34 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m pytest -q -> 415 passed
  • uv run --extra dev ruff check app/public_routes.py tests/test_wallet_api.py -> passed
  • uv run --extra dev ruff format --check app/public_routes.py tests/test_wallet_api.py -> 2 files already formatted
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m mypy app/public_routes.py -> success
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python -m mypy app scripts/submission_quality_gate.py scripts/docs_smoke.py scripts/pr_queue_health.py -> success
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check origin/main...HEAD -> clean

GitHub readback showed the PR open, non-draft, mergeable, and the Quality, readiness, docs, and image checks workflow successful. CodeRabbit was still pending at review time after a short wait, so I did not treat it as a completed signal.

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: e55f2c15-6eca-4a13-9064-39a0538eb71f

📥 Commits

Reviewing files that changed from the base of the PR and between d8532d4 and 9bf431c.

📒 Files selected for processing (3)
  • app/public_routes.py
  • app/templates/wallets.html
  • tests/test_wallet_api.py

Comment thread app/public_routes.py
Copy link
Copy Markdown
Contributor

@tolga-tom-nook tolga-tom-nook 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 9bf431cfa1fd404040be904a3766f005f5fae954 for the wallet row-limit UX change.

Evidence checked:

  • Diff is focused to /wallets: app/public_routes.py, app/templates/wallets.html, and tests/test_wallet_api.py.
  • The new limit query param is bounded with Query(ge=1, le=200) before use in the SQL LIMIT, preserving the existing newest-first ordering and preventing unbounded wallet table renders.
  • The template renders stable row-count controls and preserves the selected option, including custom valid limits such as 1 used by the regression test.
  • Existing wallet generation/link/transfer page content remains covered by the expanded wallet page test.

Validation run locally:

  • ./.venv/bin/python -m pytest tests/test_wallet_api.py tests/test_public_routes.py -q — 34 passed
  • ./.venv/bin/python -m ruff check app/public_routes.py tests/test_wallet_api.py — passed
  • ./.venv/bin/python -m ruff format --check app/public_routes.py tests/test_wallet_api.py — passed
  • ./.venv/bin/python -m mypy app/public_routes.py — passed

Assessment: no blockers found. This is a small, coherent bounty #427 UX improvement that gives the public wallets page the same kind of bounded row-control behavior already being added to other high-volume public lists.

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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
app/public_routes.py (1)

4-4: ⚠️ Potential issue | 🟠 Major

Fix missing/insufficient guideline checks (ruff repo-wide + mypy).

  • app/public_routes.py: ruff format --check (“already formatted”) and ruff check (“All checks passed!”) are OK.
  • mypy type checking wasn’t completed: mypy command not found (exit 127).
  • Run the required repo-wide ruff format --check . and ruff check ., and install/run mypy app to satisfy the app/**/*.py guideline.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c9d04dcd-9ce1-4cde-bfbb-5c581ec7991d

📥 Commits

Reviewing files that changed from the base of the PR and between 9bf431c and ef4995d.

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

@tinyopsstudio
Copy link
Copy Markdown

Reviewed PR #503 at current head ef4995d7fee1c73d5a050205c798c1584ade0e67, after the follow-up commit that moved the wallet limit presets into WALLET_LIMIT_OPTIONS.

No blocker found. The current head keeps the behavior focused on the public /wallets list: the route still bounds limit with Query(ge=1, le=200), the database query uses that bounded value, and the template preserves both preset options and valid custom values such as limit=1. I also checked the CodeRabbit concern about missing mypy evidence; local mypy passes through the project virtualenv on the touched app module.

Validation run on this head:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m pytest tests/test_wallet_api.py tests/test_public_routes.py -q -> 34 passed
  • ./.venv/bin/python -m ruff check app/public_routes.py tests/test_wallet_api.py -> passed
  • ./.venv/bin/python -m ruff format --check app/public_routes.py tests/test_wallet_api.py -> 2 files already formatted
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python -m mypy app/public_routes.py -> success
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 ./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check origin/main...HEAD -> clean

GitHub readback shows the PR is open, non-draft, CLEAN, and the quality/readiness workflow is successful. This gives the wallet row-limit change a current-head review after the post-approval commit.

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