Limit account accepted-work rows#864
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 (1)
📝 WalkthroughWalkthroughAdds a bounded ChangesAccepted-work list limiting
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 |
laughlife
left a comment
There was a problem hiding this comment.
Reviewed current head 8d77908a24a36259965a3122e2ba21067e1d7d56 as a non-author.
Scope checked:
app/accounts.py: addedlimithandling for/api/v1/accounts/{account}/accepted-workand/accounts/{account}with default 100, max 200, repeated-query rejection, and canonical positive integer validation before passing the limit into the account accepted-work contexts.tests/test_account_routes.py: covers the source report behavior:limit=1caps returned accepted-work rows while keeping full summary totals, account page output only shows the latest limited row,limit=01is rejected, and repeated pagelimitis rejected.docs/api-examples.md: documents the accepted-work endpointlimit=1..200behavior and that summary totals still cover full history.
Local validation on this head:
uv run --extra dev pytest tests/test_account_routes.py::test_account_accepted_work_routes_honor_limit -q-> 1 passed, 1 existing Starlette/httpx warning.uv run --extra dev pytest tests/test_account_routes.py -q-> 8 passed, 1 existing Starlette/httpx warning.uv run --extra dev ruff check app/accounts.py tests/test_account_routes.py docs/api-examples.md-> passed.uv run --extra dev ruff format --check app/accounts.py tests/test_account_routes.py-> 2 files already formatted.uv run --extra dev mypy app/accounts.py-> success.uv run --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree origin/main HEAD-> clean tree92d0d6a47cde4f21536581571c1792b652a5488e.
Hosted Quality, readiness, docs, and image checks is successful on the current head. CodeRabbit was still pending when I reviewed, so I did not count it as completed evidence. No actionable issues found in the focused limit fix.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 81915976-53ef-4d21-a58b-8f1793e623ce
📒 Files selected for processing (3)
app/accounts.pydocs/api-examples.mdtests/test_account_routes.py
laughlife
left a comment
There was a problem hiding this comment.
Current-head review for 432d6e03694d01393f57a139d64a1390883a8191.
I rechecked this PR after the new boundary-test commit made my earlier approval stale. The implementation still keeps the accepted-work summary totals on the full account history while capping the returned/listed accepted-work rows, and the added boundary assertions cover limit=200, limit=0, limit=201, non-canonical limit=01, and repeated page limit values.
Validation I ran locally:
uv run --extra dev pytest tests/test_account_routes.py::test_account_accepted_work_routes_honor_limit -q-> 1 passed, 1 existing Starlette/httpx warning.uv run --extra dev pytest tests/test_account_routes.py -q-> 8 passed, 1 existing Starlette/httpx warning.uv run --extra dev ruff check app/accounts.py tests/test_account_routes.py docs/api-examples.md-> passed.uv run --extra dev ruff format --check app/accounts.py tests/test_account_routes.py-> passed.uv run --extra dev mypy app/accounts.py-> passed.uv run --extra dev python scripts/docs_smoke.py-> passed.git diff --check origin/main...HEAD-> clean.git merge-tree origin/main HEAD-> clean tree97e237683fc8590d9a684b418af61d469561b3bc.
The hosted Quality/readiness/docs check is successful. CodeRabbit was still pending at review time, so I am not counting that as completed evidence.
xiefuzheng713-alt
left a comment
There was a problem hiding this comment.
Reviewed PR #864 at current head 432d6e03694d01393f57a139d64a1390883a8191 as a non-author.
I inspected app/accounts.py, tests/test_account_routes.py, and docs/api-examples.md. The change keeps the accepted-work summary as full account history while capping returned/rendered rows with a bounded limit=1..200, uses the existing repeated/non-canonical query guards on both the API and account page, and documents the new default/max behavior. The regression covers the latest-row slice, max limit, too-low/too-high bounds, non-canonical 01, and repeated limit values.
Validation:
uv run --extra dev pytest tests/test_account_routes.py -q-> 8 passed, 1 existing Starlette/httpx warning.uv run --extra dev pytest tests/test_account_routes.py tests/test_docs_public_urls.py -q-> 43 passed, 1 existing Starlette/httpx warning.uv run --extra dev ruff check app/accounts.py tests/test_account_routes.py docs/api-examples.md-> passed.uv run --extra dev ruff format --check app/accounts.py tests/test_account_routes.py-> 2 files already formatted.uv run --extra dev mypy app/accounts.py-> success.uv run --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 tree97e237683fc8590d9a684b418af61d469561b3bc.
GitHub state checked before review: mergeStateStatus=CLEAN; hosted Quality/readiness/docs/image and CodeRabbit statuses are successful. I saw prior human approval on this head, so this is submitted as an independent current-head evidence review, not as first-review eligibility evidence.
Scope checked: public account accepted-work row limiting only. No private data, secrets, wallet material, payout execution, treasury mutation, ledger mutation, bridge/exchange/cash-out, MRWK price behavior, or issue mutation was used.
tudorian95
left a comment
There was a problem hiding this comment.
Reviewed current head 432d6e03694d01393f57a139d64a1390883a8191 for Bounty #838.
What I checked:
- Inspected the focused diff in
app/accounts.py,tests/test_account_routes.py, anddocs/api-examples.md. - Confirmed the live production before-state is still reproducible with harmless GETs:
GET /api/v1/accounts/github:likeacloud7/accepted-workand?limit=1both returned 8 accepted-work rows, andGET /accounts/github:likeacloud7?limit=1still showed 8 ledger/proof rows inside the Accepted work section. - In Docker/uv on a fresh checkout of this PR head:
uv run --extra dev pytest tests/test_account_routes.py::test_account_accepted_work_routes_honor_limit -q-> 1 passed, 1 existing Starlette/httpx warning.uv run --extra dev pytest tests/test_account_routes.py tests/test_account_validation.py -q-> 52 passed, 1 existing warning.uv run --extra dev ruff check app/accounts.py tests/test_account_routes.py-> passed.uv run --extra dev ruff format --check app/accounts.py tests/test_account_routes.py-> passed.uv run --extra dev mypy app/accounts.py-> success.uv run --extra dev python scripts/docs_smoke.py-> docs smoke ok.git diff --check-> clean.
git merge-tree --write-tree origin/main HEADsucceeded with tree97e237683fc8590d9a684b418af61d469561b3bc.- GitHub reports the current head as
MERGEABLE. - Overlap checked: this PR overlaps the API half of #865 and conflicts if #865 is merged first, but #864 has distinct current-head value because it also covers the public account page and documents the default row cap. Maintainers should land/rebase only one combined accepted-work-limit path if both remain open.
I did not find a current-head blocker against origin/main; the remaining concern is coordination with the overlapping #865 work.
Summary
limit=1..200support to/api/v1/accounts/{account}/accepted-work./accounts/{account}page while leaving transaction history unchanged.Bounty #799
Source report: #798 (comment)
Validation
python -m pytest tests\test_account_routes.py::test_account_accepted_work_routes_honor_limit -qpython -m pytest tests\test_account_routes.py tests\test_account_validation.py -qpython -m ruff check app\accounts.py tests\test_account_routes.pypython -m ruff format --check app\accounts.py tests\test_account_routes.pypython -m mypy app\accounts.pypython scripts\docs_smoke.pygit diff --checkgit merge-tree --write-tree origin/main HEADNotes
The existing summary remains uncapped so agents can reconcile full-account totals even when callers request only recent rows.
Summary by CodeRabbit
New Features
Documentation
Tests