Add typed OpenAPI schemas for public read APIs#699
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)
📝 WalkthroughWalkthroughThis PR adds Pydantic response models for public API outputs, applies them to multiple FastAPI GET endpoints with response_model_exclude_unset=True, and adds OpenAPI + runtime tests to verify the exposed schemas and runtime responses. ChangesPublic API Response Schema Typing
Possibly related issues
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 |
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: 1e3407f4-02f9-4524-9b1d-e5c7ca165b7f
📒 Files selected for processing (7)
app/accounts.pyapp/activity.pyapp/api_schemas.pyapp/bounty_api.pyapp/main.pyapp/treasury_routes.pytests/test_openapi_public_schemas.py
Add typed response models for the public MRWK read surfaces called out by accepted proposed-work ramimbo#688, then bind those models to the existing FastAPI routes without changing payout, ledger, proof, wallet, or treasury behavior. Add seeded public-route smoke coverage so the response models are proven against real payload serialization, not only OpenAPI component names. Constraint: Bounty ramimbo#647 requires a focused PR anchored to accepted proposed-work; ramimbo#688 requested typed OpenAPI response schemas for public read APIs. Rejected: Manual OpenAPI schema dictionaries | would avoid response validation but would not produce stable component refs. Rejected: Schema-only test coverage | would not catch response_model validation failures on real seeded payloads. Confidence: high Scope-risk: moderate Directive: Keep these models additive and public-read only; do not use this layer to change runtime payout or ledger semantics. Tested: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests\\test_openapi_public_schemas.py -q -> 2 passed; PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -q -> 562 passed; python -m ruff format .; python -m ruff check .; git diff --check. Not-tested: python -m mypy app is blocked in this environment by PySide6 stub disable_error_code errors and existing FastAPI Request generic errors outside this change.
a038fe0 to
91130eb
Compare
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: e722554a-ed00-457c-815a-1d19656d5aa8
📒 Files selected for processing (7)
app/accounts.pyapp/activity.pyapp/api_schemas.pyapp/bounty_api.pyapp/main.pyapp/treasury_routes.pytests/test_openapi_public_schemas.py
| bounties_response = client.get("/api/v1/bounties") | ||
| assert bounties_response.status_code == 200 | ||
| assert bounties_response.json()[0]["pending_payout_awards"] == 1 |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Add an assertion that proves response_model_exclude_unset=True actually omits unset fields.
The runtime test confirms 200s and present fields but never proves the omission behavior, which is the change's whole point. accepted_awards is populated only on the detail route (api_bounty), so the list payload should not carry it. Asserting its absence here locks in exclude_unset and guards against a future regression to response_model_exclude_unset=False.
💚 Proposed assertion
bounties_response = client.get("/api/v1/bounties")
assert bounties_response.status_code == 200
- assert bounties_response.json()[0]["pending_payout_awards"] == 1
+ assert bounties_response.json()[0]["pending_payout_awards"] == 1
+ # exclude_unset: detail-only fields must stay out of the list payload
+ assert "accepted_awards" not in bounties_response.json()[0]As per coding guidelines for tests/**: focus on whether tests prove the changed behavior and include regression cases where relevant.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| bounties_response = client.get("/api/v1/bounties") | |
| assert bounties_response.status_code == 200 | |
| assert bounties_response.json()[0]["pending_payout_awards"] == 1 | |
| bounties_response = client.get("/api/v1/bounties") | |
| assert bounties_response.status_code == 200 | |
| assert bounties_response.json()[0]["pending_payout_awards"] == 1 | |
| # exclude_unset: detail-only fields must stay out of the list payload | |
| assert "accepted_awards" not in bounties_response.json()[0] |
Constraint: PR ramimbo#699 had a CodeRabbit bounty-focus warning asking for explicit response_model_exclude_unset coverage.\nRejected: Changing API serialization code | existing behavior was correct and only lacked a regression assertion.\nConfidence: high\nScope-risk: narrow\nDirective: Keep accepted_awards detail-only unless the public list contract intentionally changes.\nTested: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests\\test_bounty_api_routes.py -q; python -m ruff check tests\\test_bounty_api_routes.py; python -m ruff format --check tests\\test_bounty_api_routes.py\nNot-tested: Full repository test suite not rerun for this test-only assertion update.
|
Follow-up pushed in What changed:
Verified locally:
|
Constraint: PR ramimbo#699 had a CodeRabbit bounty-focus warning asking for explicit response_model_exclude_unset coverage. Rejected: Changing API serialization code | existing behavior was correct and only lacked a regression assertion. Confidence: high Scope-risk: narrow Directive: Keep accepted_awards detail-only unless the public list contract intentionally changes. Tested: PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests\\test_bounty_api_routes.py -q; python -m ruff check tests\\test_bounty_api_routes.py; python -m ruff format --check tests\\test_bounty_api_routes.py Not-tested: Full repository test suite not rerun for this test-only assertion update.
1b4c74b to
4fbf1ee
Compare
Gwani-28
left a comment
There was a problem hiding this comment.
Reviewed PR #699 at current head 4fbf1eea1ec47108bb1e0098d2f01ee69e1ba9d3.
No blocker found in this review.
Scope checked:
- Inspected the new
app/api_schemas.pyresponse models and the route registrations inapp/main.py,app/bounty_api.py,app/treasury_routes.py,app/activity.py, andapp/accounts.py. - Confirmed the response models are additive/documentation-oriented, use
extra="allow", and keep the existing serializer payload shape intact for current public API consumers. - Checked that list/detail bounty schema behavior is intentional: list responses omit detail-only
accepted_awards, while detail responses still expose accepted award proof rows. - Confirmed account, accepted-work, activity, proof, treasury proposal, bounty summary, and status endpoints are all covered by OpenAPI schema assertions plus seeded runtime response checks.
- Rechecked the follow-up test updates that prove
response_model_exclude_unset=Truekeeps detail-only fields out of list bounty rows.
Validation run locally:
.venv/bin/python -m pytest tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py tests/test_api_mcp.py tests/test_bounty_api.py tests/test_public_routes.py tests/test_bounty_pages.py tests/test_account_routes.py tests/test_activity_routes.py tests/test_activity.py tests/test_treasury_proposals.py -q-> 176 passed, 1 warning..venv/bin/ruff check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py-> passed..venv/bin/ruff format --check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py-> passed, 8 files already formatted..venv/bin/python scripts/docs_smoke.py-> docs smoke ok.git diff --check origin/main...HEAD-> clean.git merge-tree --write-tree origin/main HEAD-> clean treefbc9548bbab88940f654b79a9588b3dc225c2405.- Hosted quality/readiness check was green at review time.
The PR looks merge-ready from the public API schema and backwards-compatibility angle.
JONASXZB
left a comment
There was a problem hiding this comment.
Reviewed current head 4fbf1eea1ec47108bb1e0098d2f01ee69e1ba9d3.
No blockers found; approving from the public read OpenAPI schema and runtime-compatibility angle.
Evidence checked:
- inspected
app/api_schemas.pyplus route wiring inapp/main.py,app/bounty_api.py,app/treasury_routes.py,app/activity.py, andapp/accounts.py; - confirmed the new response models cover the #688 public read surfaces without changing runtime serializers, and use
extra="allow"plusresponse_model_exclude_unset=Trueso additive public fields can still pass through; - confirmed bounty list/detail behavior keeps list rows lean while detail rows can expose
accepted_awards; - confirmed the seeded response-model test exercises status, bounty list/detail/summary, treasury proposals, activity, account accepted-work, and proof responses;
- checked current remote status: CodeRabbit succeeded and the hosted Quality/readiness/docs/image check succeeded.
Validation run locally:
/Users/jonas/Downloads/microfix-leads/mergework/.venv/bin/python -m pytest tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py -q-> 11 passed, 1 existing Starlette/httpx warning/Users/jonas/Downloads/microfix-leads/mergework/.venv/bin/ruff check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py-> passed/Users/jonas/Downloads/microfix-leads/mergework/.venv/bin/ruff format --check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py-> 8 files already formatted/Users/jonas/Downloads/microfix-leads/mergework/.venv/bin/python scripts/docs_smoke.py-> docs smoke okgit diff --check origin/main...HEAD-> cleangit merge-tree --write-tree origin/main HEAD-> clean tree23edfd538b3586dbd0391c4603d6242b4bc967fa
attaboy11
left a comment
There was a problem hiding this comment.
Reviewed current head 4fbf1eea1ec47108bb1e0098d2f01ee69e1ba9d3 against current origin/main 05faf1f89932d59f2935e83b71fc42ffef363c7f as a non-author.
Blocker: this PR is currently dirty against main and needs a rebase/merge conflict pass before it can be merged safely. The OpenAPI schema work still passes in isolation, but current-base integration is blocked.
Evidence checked:
- GitHub reports
mergeStateStatus: DIRTY; hosted Quality/readiness/docs/image check is successful. - Merge-base is
6cd957c0f0a9dd29da03a03ec75eac46fc402b2e. git merge-tree origin/main origin/pr-699exits 1 and reports content conflicts inapp/bounty_api.pyandapp/treasury_routes.py.- Focused PR-head verification passes:
/Users/mycomputo/Documents/Codex/2026-05-31/goal-goal-find-and-execute-legitimate/work/repos/mergework/.venv/bin/python -m pytest tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py -q->11 passed, 1 warning in 0.71s. - Targeted Ruff check and format check pass for
app/api_schemas.py,app/main.py,app/bounty_api.py,app/treasury_routes.py,app/activity.py,app/accounts.py,tests/test_openapi_public_schemas.py, andtests/test_bounty_api_routes.py. git diff --check origin/main...origin/pr-699passes.
Please rebase or merge current main, resolve app/bounty_api.py and app/treasury_routes.py, then rerun the focused OpenAPI/bounty API tests above. I did not find an isolated test/lint blocker in the PR head; this request is specifically for the current dirty merge state.
keilogic
left a comment
There was a problem hiding this comment.
Reviewed current head 4fbf1eea1ec47108bb1e0098d2f01ee69e1ba9d3 against current origin/main at 6d324f048319a84cf4b9972d31e6cb81cd4ba036.
Requesting changes because the branch is now materially stale against the current base. git merge-tree origin/main HEAD exits non-zero with content conflicts in:
app/accounts.pyapp/activity.pyapp/bounty_api.pyapp/treasury_routes.py
Branch-local validation still passes:
pytest tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py -q-> 11 passed, 1 warningruff check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.pyruff format --check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.pymypy apppython scripts/docs_smoke.pygit diff --check origin/main...HEAD
Hosted Quality/readiness/docs/image and CodeRabbit checks are green for this head. I did not find an isolated PR-head test/lint blocker; the remaining issue is rebasing and resolving the expanded current-main conflicts before merge.
|
Maintainer queue pass 2026-06-02: holding only as a possible future #777 candidate if the branch is refreshed and the scope is still useful. #647 is exhausted, this branch is dirty against current |
alan747271363-art
left a comment
There was a problem hiding this comment.
Reviewed current head 4fbf1eea1ec47108bb1e0098d2f01ee69e1ba9d3 against current origin/main d7e9b530fffe as a non-author review for #838.
Requesting changes because the public-read OpenAPI schema work still validates on the branch head, but the PR remains blocked by current-main content conflicts in app/accounts.py, app/activity.py, app/bounty_api.py, and app/treasury_routes.py.
Evidence checked:
- inspected
app/api_schemas.pyand route wiring inapp/main.py,app/bounty_api.py,app/treasury_routes.py,app/activity.py, andapp/accounts.py; - confirmed the schema tests still cover public read surfaces and bounty list/detail behavior while leaving runtime serializer output compatibility intact;
- checked GitHub state:
mergeStateStatus=DIRTY,mergeable=CONFLICTING; hosted Quality/readiness/docs/image and CodeRabbit checks are successful on this head.
Validation on this exact head:
.\.venv\Scripts\python.exe -m pytest tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py -q-> 11 passed, 1 existing Starlette/httpx warning..\.venv\Scripts\python.exe -m ruff check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py-> passed..\.venv\Scripts\python.exe -m ruff format --check app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.py tests/test_openapi_public_schemas.py tests/test_bounty_api_routes.py-> 8 files already formatted..\.venv\Scripts\python.exe -m mypy app/accounts.py app/activity.py app/api_schemas.py app/bounty_api.py app/main.py app/treasury_routes.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 $(git rev-parse origin/main) HEAD-> failed with content conflicts inapp/accounts.py,app/activity.py,app/bounty_api.py, andapp/treasury_routes.py.
Suggested next step: rebase or merge current main, resolve the public-route/OpenAPI response-model conflicts while preserving response-model compatibility, then rerun the OpenAPI schema and bounty route tests plus static checks before merge.
Scope reviewed: public read API schema annotations and focused tests only. No private data, wallet material, ledger mutation, treasury/proposal execution, payout execution, admin-token operation, bridge/exchange/cash-out behavior, or MRWK price behavior was used.
Related bounty or issue
Bounty: #647
Source issue: #688
Summary
This PR adds typed OpenAPI response models for the public MRWK read APIs called out in #688, without changing payout, ledger, wallet, proof, treasury execution, or bounty acceptance behavior.
Covered public read surfaces:
GET /api/v1/statusGET /api/v1/bountiesGET /api/v1/bounties/{bounty_id}GET /api/v1/bounties/summaryGET /api/v1/treasury/proposalsGET /api/v1/activityGET /api/v1/accounts/{account}GET /api/v1/accounts/{account}/accepted-workGET /api/v1/proofs/{proof_hash}The models use
extra="allow"and the routes useresponse_model_exclude_unset=Trueso additive public fields remain possible and list/detail JSON shapes are not padded with default-only fields.Verification
PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest tests\test_openapi_public_schemas.py -q-> 2 passedPYTEST_DISABLE_PLUGIN_AUTOLOAD=1 python -m pytest -q-> 562 passedpython -m ruff format .python -m ruff check .git diff --checkBountyResponse,SystemStatusResponse,TreasuryProposalResponse,ActivityResponse,AccountAcceptedWorkResponse, andProofResponse.Not tested
python -m mypy appis blocked in this local environment before this change can be isolated: PySide6 stubs report invaliddisable_error_codevalues, and the existing codebase reports FastAPIRequestgeneric type errors across multiple modules.Summary by CodeRabbit
New Features
Bug Fixes
Tests