Document status API response shape#259
Conversation
MolhamHamwi
left a comment
There was a problem hiding this comment.
No blockers from my review.
Evidence checked:
- Inspected
docs/api-examples.mdandtests/test_docs_public_urls.pyfor the new/api/v1/statusresponse example and regression coverage. - Cross-checked
app/main.pystatus response behavior so the documentedledger_height,latest_proof, and timestamp fields match the actual API shape. - Also inspected the related hardening touched in
app/config.pyandapp/webhooks/github.py, including Postgres URL host/port validation and typed webhook payload-object handling. - Ran
pytest tests/test_docs_public_urls.py tests/test_wallet_api.py tests/test_webhooks.py -q: 51 passed. - Ran Ruff check/format-check on the touched Python/docs test paths and
git diff --check origin/main...HEAD: all passed.
No secrets, wallet material, payout details, private deployment values, private vulnerability details, or MRWK price claims were reviewed or disclosed.
weilixiong
left a comment
There was a problem hiding this comment.
Review ✅ APPROVED
LOW — verified diff clean, no unrelated files. MW #219 round 7.
weilixiong
left a comment
There was a problem hiding this comment.
Review ✅ APPROVED — LOW risk, clean diff, MW #219 round 7.
|
No blockers from my review. Evidence checked:
The caveat about live values changing is important, and it is present in the docs change. |
g8rr5dg2p7-svg
left a comment
There was a problem hiding this comment.
Requesting changes: the new status example hardcodes a live ledger counter, and it is already stale against the public endpoint.
Evidence checked:
- Inspected
docs/api-examples.md; the example documents"ledger_height": 410and the new regression test asserts that exact value. - Queried
GET https://api.mrwk.ltclab.site/api/v1/statusduring review; the live unauthenticated response now returns"ledger_height": 412while the other documented keys remain present. - This is the same kind of moving-counter risk the docs text warns about, but the example and test still lock in one exact live value.
Suggested fix: keep the response-shape documentation, but avoid making ledger_height an exact live value in both the prose example and the regression. For example, use a clearly illustrative placeholder or assert the field name/type instead of the current counter. That keeps the docs accurate after new ledger entries are appended.
No secrets, wallet material, payout credentials, private deployment values, private vulnerability details, or MRWK price claims were reviewed or disclosed.
|
Thanks for the review. I addressed the stale live-counter issue by changing the status example to use clearly illustrative values and updating the regression to assert the documented field names/types instead of the exact live ledger counter. Validation run after the change:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis pull request adds documentation for the ChangesAPI Status Endpoint Documentation
🎯 1 (Trivial) | ⏱️ ~3 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
iccccccccccccc
left a comment
There was a problem hiding this comment.
Re-checked the current head (c45a903) after the stale live-counter feedback, and this looks good to me now.
What I checked:
docs/api-examples.mdnow says the/api/v1/statuspayload uses illustrative values, so the example is documenting the response shape rather than freezing a live ledger height or treasury balance.tests/test_docs_public_urls.pynow checks that the status fields are documented with numeric-looking values, and it explicitly guards against the old staleledger_height: 410example coming back.- The warning that ledger-derived values can change is still present, which is the right caveat for this endpoint.
Validation run locally on this PR head:
.\.venv\Scripts\python -m pytest tests/test_docs_public_urls.py::test_api_examples_document_status_response_shape -q-> 1 passed.\.venv\Scripts\python -m pytest tests/test_docs_public_urls.py -q-> 14 passed.\.venv\Scripts\python scripts\docs_smoke.py-> docs smoke ok.\.venv\Scripts\python -m ruff check tests/test_docs_public_urls.py-> passed.\.venv\Scripts\python -m ruff format --check --preview docs/api-examples.md tests/test_docs_public_urls.py-> passedgit diff --check origin/main...HEAD-> passed
No secrets, wallet material, private payout details, deployment values, private vulnerability details, or MRWK price claims were reviewed or disclosed.
Bounty #229
What changed:
GET /api/v1/statusresponse shape indocs/api-examples.md.Evidence:
GET https://api.mrwk.ltclab.site/api/v1/status.name,ticker,genesis_supply_mrwk,ledger_height,active_bounties,treasury_balance_mrwk, andfuture_path.app/main.py::api_status(), which returns those same keys from ledger and bounty state.Verification:
uv run --python 3.12 --extra dev python -m pytest tests/test_docs_public_urls.py::test_api_examples_document_status_response_shape -q-> 1 passeduv run --python 3.12 --extra dev python -m pytest tests/test_docs_public_urls.py -q-> 14 passeduv run --python 3.12 --extra dev python scripts/docs_smoke.py-> docs smoke okuv run --python 3.12 --extra dev ruff check docs/api-examples.md tests/test_docs_public_urls.py-> all checks passeduv run --python 3.12 --extra dev ruff format --check --preview docs/api-examples.md tests/test_docs_public_urls.py-> 2 files already formatteduv run --python 3.12 --extra dev python -m pytest -q-> 206 passed, 2 warningsgit diff --check-> cleanNo private keys, wallet secrets, real signatures, payout credentials, deployment values, private vulnerability details, or MRWK price claims are included.
Summary by CodeRabbit
Release Notes
/api/v1/statusendpoint response, including sample payloads with service metadata and ledger-related information such as current ledger height, active bounty count, and treasury balance.