Honor activity API pagination#918
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds limit/offset pagination to the /api/v1/activity endpoint: route accepts constrained ChangesActivity API pagination
Possibly related PRs
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
|
Follow-up for hosted CI failure on Updated head: Change since initial PR:
Updated validation:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/api-examples.md (2)
371-433:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the response example to match the pagination request.
Line 351 shows a request with
limit=25&offset=25, but the response example at lines 383-384 shows"limit": 25, "offset": 0. Either update the response example to show"offset": 25, or clarify that the JSON response is for a different request (such as the query-filtered request at line 350, then add a separate pagination example).📝 Proposed fix
Option 1: Update the response to match the paginated request at line 351:
"query": "p3xill", "limit": 25, - "offset": 0, + "offset": 25,Option 2: Add a note that the response is for the query-only request, then show a separate pagination response example after the curl at line 351.
435-438:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate the 100-row cap explanation to reflect pagination.
Line 436 states that
recentis "capped to the latest 100 matching rows." With pagination,recentis capped tolimitrows starting fromoffset, not always 100.📝 Proposed fix
`contributors` is sorted by accepted MRWK amount, while `recent` is sorted by -newest ledger sequence and capped to the latest 100 matching rows. Use +newest ledger sequence and paginated using `limit` and `offset`. Use `proof_hash` with `/api/v1/proofs/<proof_hash>` to inspect the public proof payload for a payment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bbd823e0-403a-46d8-9220-78a3dbcee85a
📒 Files selected for processing (6)
app/activity.pyapp/serializers.pydocs/api-examples.mdtests/test_activity.pytests/test_activity_routes.pytests/test_serializers.py
|
Follow-up for the docs review notes:
Validation rerun:
|
|
Follow-up for the pagination control-character review note. Updated head: Change:
Validation rerun:
|
pqmfei
left a comment
There was a problem hiding this comment.
Reviewed PR #918 at current head 85ea49dbc4eff3b9b304c86aa12f4fc5af1f6a65 as a non-author.
Evidence checked:
- inspected
app/activity.py,app/serializers.py,docs/api-examples.md,tests/test_activity.py,tests/test_activity_routes.py, andtests/test_serializers.py; - verified
/api/v1/activitynow accepts bounded canonicallimitandoffset, rejects control-character, repeated, and non-canonical pagination params before using the coerced integers, and preserves existingq/accountguards; - verified
activity_to_dict()keeps aggregate totals over the full matching activity set while applyingoffset:offset + limitslices tocontributors,pending_payouts, andrecent; - confirmed the latest follow-ups addressed the stale docs example/cap wording and the control-character masking order for pagination params;
- checked #838 public bounty row 109 is open with 30 effective awards remaining, and
scripts/review_bounty_candidates.py --repo ramimbo/mergework --reviewer pqmfei --format textstill reports PR #918 ascandidate_for_fresh_review.
Validation on this exact head:
.\.venv\Scripts\python.exe -m pytest tests\test_activity.py tests\test_activity_routes.py tests\test_serializers.py tests\test_docs_public_urls.py -q-> 56 passed, 1 existing Starlette/httpx warning..\.venv\Scripts\python.exe -m ruff check app\activity.py app\serializers.py tests\test_activity.py tests\test_activity_routes.py tests\test_serializers.py docs\api-examples.md-> passed..\.venv\Scripts\python.exe -m ruff format --check app\activity.py app\serializers.py tests\test_activity.py tests\test_activity_routes.py tests\test_serializers.py-> 5 files already formatted..\.venv\Scripts\python.exe -m mypy app\activity.py app\serializers.py-> success..\.venv\Scripts\python.exe scripts\docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean tree95c4ae00c0c95aaf8ac0578bf5f77b41503cf256.
GitHub state before review: hosted Quality, readiness, docs, and image checks is successful; mergeStateStatus=UNSTABLE only because CodeRabbit is still pending on the latest head, so maintainers should still account for any later bot feedback before merging.
Scope: read-only review of public activity API pagination and docs/tests. No admin-token APIs, bounty creation, payout execution, treasury mutation, ledger mutation, wallet behavior, private data, credentials, exchange, bridge, cash-out, or MRWK price behavior was used or changed.
szx19970521
left a comment
There was a problem hiding this comment.
Reviewed current head 85ea49d for the activity API pagination change.
What I checked:
- �pp/activity.py: limit/offset are bounded, reject control characters, reject repeated params, and reject noncanonical integer forms before forwarding into the activity context.
- �pp/serializers.py: pagination slices contributors, pending_payouts, and
ecent after filtering while preserving full aggregate totals. - docs/api-examples.md: pagination wording and examples now match the current response shape; earlier CodeRabbit docs comments are addressed.
- Tests cover default response shape, limit/offset paging, invalid pagination values, and control-character masking cases.
Validation run locally on this head:
- python -m pytest tests/test_serializers.py tests/test_activity.py tests/test_activity_routes.py tests/test_docs_public_urls.py -q -> 56 passed, 1 existing Starlette/httpx warning.
uff check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py tests/test_serializers.py docs/api-examples.md -> passed.
uff format --check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py tests/test_serializers.py -> 5 files already formatted.
- mypy app/activity.py app/serializers.py -> success.
- python scripts/docs_smoke.py -> docs smoke ok.
Hosted Quality is green and the latest CodeRabbit pass reports no actionable comments. I did not find blocking issues.
alan747271363-art
left a comment
There was a problem hiding this comment.
Reviewed PR #918 at current head 85ea49dbc4eff3b9b304c86aa12f4fc5af1f6a65 against current origin/main (d7e9b530fffec7bd774da7708597648096a37393).
The activity pagination changes still validate on the branch itself, but this PR is now blocked by a current-main merge conflict in app/activity.py. GitHub now reports mergeStateStatus=DIRTY / mergeable=CONFLICTING, and I reproduced the conflict locally:
git merge-tree --write-tree $(git rev-parse origin/main) HEAD
# Auto-merging app/activity.py
# CONFLICT (content): Merge conflict in app/activity.py
Validation on this exact head before posting:
python -m pytest tests/test_activity.py tests/test_activity_routes.py tests/test_serializers.py tests/test_docs_public_urls.py -q-> 56 passed, 1 existing Starlette/httpx warning.python -m ruff check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py tests/test_serializers.py docs/api-examples.md-> passed.python -m ruff format --check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py tests/test_serializers.py-> 5 files already formatted.python -m mypy app/activity.py app/serializers.py-> success.python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.
Requested changes until the branch is rebased/resolved against current origin/main and the same focused activity/docs checks are rerun. The hosted Quality and CodeRabbit checks are green on the PR head, but they do not remove the current merge conflict.
Scope checked: public activity API pagination and related serializer/docs/tests only. No admin APIs, wallet behavior, payout execution, treasury mutation, ledger mutation, private data, credentials, exchange, bridge, cash-out, or MRWK price behavior was used.
heickerv1001-dev
left a comment
There was a problem hiding this comment.
Bounty #838 review: I rechecked current head 85ea49d on app/activity.py, app/serializers.py, tests/test_activity.py, tests/test_activity_routes.py, and docs/api-examples.md. The new limit/offset flow paginates after filtering, keeps totals representative of the full matching set, and the invalid/repeated/control-char cases are covered in tests. I also checked the open PR list for overlapping /api/v1/activity pagination work and did not find another active PR on the same scope. GitHub still reports mergeable_state=dirty, so this needs a rebase before it can merge.
Bounty #799
Source report: #798 (comment)
Summary
limitandoffsetsupport to the public/api/v1/activityendpoint.totalsandpending_totalsrepresentative of the full matching activity set.limit/offsetquery values before silently treating them as canonical intent.Production evidence before fix
Read-only checks against current production on 2026-06-05 UTC:
Duplicate / scope check
activity pagination limit offsetand found no current open PR covering bothlimitandoffseton/api/v1/activity.limitPR from MRWK bounty: 75 MRWK - live verification and bug reports, round 1 #656, but it is currentlyDIRTY/mrwk:needs-info, tied to the exhausted MRWK bounty: 75 MRWK - live verification and bug reports, round 1 #656 route, and does not implementoffsetor the latest MRWK bounty: 75 MRWK - live verification and bug reports, round 2 #798 report's full pagination surface.mainand fixes the source report above in one focused API slice. It does not touch the/activityHTML page, payout execution, treasury mutation, wallet behavior, ledger mutation, admin-token behavior, private data, exchange, bridge, cash-out, or MRWK price behavior.Validation
uv run --python 3.12 --extra dev python -m pytest tests/test_activity.py tests/test_activity_routes.py tests/test_docs_public_urls.py -q-> 45 passed, 1 existing Starlette/httpx warning.uv run --python 3.12 --extra dev ruff check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py docs/api-examples.md-> passed.uv run --python 3.12 --extra dev ruff format --check app/activity.py app/serializers.py tests/test_activity.py tests/test_activity_routes.py-> 4 files already formatted.uv run --python 3.12 --extra dev mypy app/activity.py app/serializers.py-> success.uv run --python 3.12 --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean treeabda6f4c7b8cc6d19c1ef4bc0fd2711b121cfce1.Summary by CodeRabbit
New Features
Improvements
Documentation
Tests