Skip to content

Document status API response shape#259

Closed
Lucas-FManager wants to merge 2 commits into
ramimbo:mainfrom
Lucas-FManager:lucas/docs-status-response-229
Closed

Document status API response shape#259
Lucas-FManager wants to merge 2 commits into
ramimbo:mainfrom
Lucas-FManager:lucas/docs-status-response-229

Conversation

@Lucas-FManager
Copy link
Copy Markdown

@Lucas-FManager Lucas-FManager commented May 25, 2026

Bounty #229

What changed:

  • Documented the public GET /api/v1/status response shape in docs/api-examples.md.
  • Clarified which fields are stable service metadata and which live values can change with ledger/bounty activity.
  • Added docs regression coverage for the status example fields.

Evidence:

  • Compared the example against the live unauthenticated endpoint: GET https://api.mrwk.ltclab.site/api/v1/status.
  • The live response includes name, ticker, genesis_supply_mrwk, ledger_height, active_bounties, treasury_balance_mrwk, and future_path.
  • Cross-checked 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 passed
  • uv run --python 3.12 --extra dev python -m pytest tests/test_docs_public_urls.py -q -> 14 passed
  • uv run --python 3.12 --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • uv run --python 3.12 --extra dev ruff check docs/api-examples.md tests/test_docs_public_urls.py -> all checks passed
  • uv run --python 3.12 --extra dev ruff format --check --preview docs/api-examples.md tests/test_docs_public_urls.py -> 2 files already formatted
  • uv run --python 3.12 --extra dev python -m pytest -q -> 206 passed, 2 warnings
  • git diff --check -> clean

No private keys, wallet secrets, real signatures, payout credentials, deployment values, private vulnerability details, or MRWK price claims are included.

Summary by CodeRabbit

Release Notes

  • Documentation
    • Enhanced API reference documentation with detailed examples of the /api/v1/status endpoint response, including sample payloads with service metadata and ledger-related information such as current ledger height, active bounty count, and treasury balance.

Review Change Stack

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 docs/api-examples.md and tests/test_docs_public_urls.py for the new /api/v1/status response example and regression coverage.
  • Cross-checked app/main.py status response behavior so the documented ledger_height, latest_proof, and timestamp fields match the actual API shape.
  • Also inspected the related hardening touched in app/config.py and app/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.

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 docs/api-examples.md and tests/test_docs_public_urls.py; the status example documents the current public response keys and explicitly warns that ledger-derived counts/balances can change.
  • Ran uv run --extra dev pytest tests/test_docs_public_urls.py -q on PR branch pr-259: 14 passed.
  • Ran uv run --extra dev python scripts/docs_smoke.py: docs smoke ok.
  • Ran uv run --extra dev ruff check tests/test_docs_public_urls.py: all checks passed.

The caveat about live values changing is important, and it is present in the docs change.

Copy link
Copy Markdown

@g8rr5dg2p7-svg g8rr5dg2p7-svg left a comment

Choose a reason for hiding this comment

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

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": 410 and the new regression test asserts that exact value.
  • Queried GET https://api.mrwk.ltclab.site/api/v1/status during review; the live unauthenticated response now returns "ledger_height": 412 while 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.

@Lucas-FManager
Copy link
Copy Markdown
Author

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:

  • uv run --extra dev pytest tests/test_docs_public_urls.py -q
  • uv run --extra dev python scripts/docs_smoke.py
  • uv run --extra dev ruff check tests/test_docs_public_urls.py
  • uv run --extra dev ruff format --check --preview docs/api-examples.md tests/test_docs_public_urls.py
  • git diff --check

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 25, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: bb95c9bd-65d0-4eb2-8b47-b8fd286eacef

📥 Commits

Reviewing files that changed from the base of the PR and between 3cb4522 and c45a903.

📒 Files selected for processing (2)
  • docs/api-examples.md
  • tests/test_docs_public_urls.py

📝 Walkthrough

Walkthrough

This pull request adds documentation for the /api/v1/status endpoint to the API examples guide and includes a test that validates the documentation contains the endpoint example with specific response-shape fields and formats.

Changes

API Status Endpoint Documentation

Layer / File(s) Summary
Status endpoint documentation and validation
docs/api-examples.md, tests/test_docs_public_urls.py
Documentation describes the /api/v1/status response payload including service metadata and ledger fields (height, active bounties, treasury balance) with a representative JSON example; a test validates the documentation includes the endpoint example with correctly formatted numeric and quoted fields while asserting certain illustrative values are absent.

🎯 1 (Trivial) | ⏱️ ~3 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: documenting the status API response shape, which aligns with the core objective of adding API response documentation.
Description check ✅ Passed The description includes a clear summary of changes, evidence from live API verification and code review, comprehensive test verification results, and a security checklist confirming no sensitive data is included.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@iccccccccccccc iccccccccccccc left a comment

Choose a reason for hiding this comment

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

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.md now says the /api/v1/status payload 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.py now checks that the status fields are documented with numeric-looking values, and it explicitly guards against the old stale ledger_height: 410 example 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 -> passed
  • git 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.

@ramimbo
Copy link
Copy Markdown
Owner

ramimbo commented May 26, 2026

Closing this stale API-docs PR to clear the filled #229 queue. #229 was filled and closed, no fresh public API examples round was opened, and this remaining docs 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.

7 participants