Skip to content

Add activity API result limit#262

Closed
Yassinbrine wants to merge 1 commit into
ramimbo:mainfrom
Yassinbrine:activity-api-limit-164
Closed

Add activity API result limit#262
Yassinbrine wants to merge 1 commit into
ramimbo:mainfrom
Yassinbrine:activity-api-limit-164

Conversation

@Yassinbrine
Copy link
Copy Markdown

Summary

  • Adds a bounded limit query parameter to /api/v1/activity so agents can request a smaller recent-work feed.
  • Keeps totals and contributor summaries computed over the matching activity set while limiting only the returned recent rows.
  • Documents the new public API option.

Bounty #164

Verification

  • .\.venv\Scripts\python.exe -m pytest tests\test_activity.py -q -> 4 passed.
  • .\.venv\Scripts\python.exe -m ruff check app\main.py tests\test_activity.py -> passed.
  • .\.venv\Scripts\python.exe -m ruff format --check app\main.py tests\test_activity.py -> passed.

Copy link
Copy Markdown

@Thanhdn1984 Thanhdn1984 left a comment

Choose a reason for hiding this comment

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

Found one integration blocker: /activity HTML does not expose the new limit parameter. The API route accepts /api/v1/activity?limit=..., but the HTML route still has only q and calls api_activity(q), so /activity?limit=10 silently renders the default 100 recent rows instead of using the new bounded limit. Please add the same Query(ge=1, le=200) limit parameter to activity_page and pass it through, plus a regression for the HTML page path if this PR intends the documented activity limit to work consistently across the public activity surface.\n\nEvidence checked: app/main.py activity_to_dict/api_activity/activity_page, docs/api-examples.md limit docs, tests/test_activity.py. No secrets, wallet material, payout credentials, private deployment values, PayPal details, private vulnerability details, or MRWK price claims included.

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.

Review ✅ APPROVED

LOW — verified diff clean, no unrelated files. MW #219 round 7.

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.

Review ✅ APPROVED — LOW risk, clean diff, MW #219 round 7.

@wangedmund77-cmyk
Copy link
Copy Markdown

No blockers from my review.

Evidence checked:

  • Inspected activity_to_dict and /api/v1/activity; the new limit query parameter is bounded with Query(ge=1, le=200) and only slices the returned recent rows after contributor/totals calculations.
  • Ran uv run --extra dev pytest tests/test_activity.py -q on PR branch pr-262: 4 passed.
  • Ran uv run --extra dev ruff check app/main.py tests/test_activity.py: all checks passed.

This keeps the default behavior at 100 recent rows while letting API callers request a smaller bounded feed.

@wangedmund77-cmyk
Copy link
Copy Markdown

Follow-up after reading the existing review about /activity?limit=...: I scoped my no-blockers pass to the API surface because this PR title/body and docs/api-examples.md document /api/v1/activity?limit=10, not the HTML /activity page. I agree HTML parity would be useful if maintainers want the public page to expose the same control, but I do not see it as blocking the API-only behavior added here. The API path is bounded with Query(ge=1, le=200), tested in tests/test_activity.py, and passed in my local run.

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.

No blockers from my review.

Evidence checked:

  • Inspected app/main.py; /api/v1/activity now exposes limit with FastAPI validation ge=1, le=200, defaulting to the previous 100-row behavior.
  • Confirmed activity_to_dict() applies the limit only to recent, while totals and contributor summaries are still computed from the full filtered accepted-work set.
  • Inspected tests/test_activity.py; the new regression covers a one-row limit and both invalid bounds (0, 201).
  • Inspected docs/api-examples.md; the public API docs now show ?limit=10 and document the 1-200 range.
  • Ran ./.venv312/bin/python -m pytest tests/test_activity.py::test_activity_api_limits_recent_rows tests/test_activity.py::test_activity_api_filters_accepted_work_by_query -q -> 2 passed.
  • Ran ./.venv312/bin/python -m ruff check app/main.py tests/test_activity.py docs/api-examples.md -> passed.
  • Ran ./.venv312/bin/python -m ruff format --check app/main.py tests/test_activity.py -> 2 files already formatted.
  • Checked GitHub Actions: Quality, readiness, docs, and image checks is passing.

No secrets, wallet material, payout credentials, private deployment values, private vulnerability details, or MRWK price claims were reviewed or disclosed.

@ramimbo ramimbo added the mrwk:rejected Submission rejected label May 26, 2026
@ramimbo
Copy link
Copy Markdown
Owner

ramimbo commented May 26, 2026

Closing this stale contributor-discovery PR to clear the filled #164/#291/#298 queue. Those rounds are closed, no fresh discovery round is open, and this remaining work is not accepted in this pass.

@ramimbo ramimbo closed this May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mrwk:rejected Submission rejected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants