ref: Extract public page routes#394
Conversation
Move read-only public page route registration into app.public_routes while keeping app.main focused on API, auth, and app assembly boundaries. Add a characterization test for public bounty page context normalization. Refs ramimbo#390 Co-Authored-By: GPT-5 Codex <codex@openai.com>
|
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 (3)
📝 WalkthroughWalkthroughThis PR extracts public-facing HTML route registration from ChangesPublic Routes Refactoring
Sequence DiagramsequenceDiagram
participant Client
participant RegisterPublicRoutes as register_public_routes
participant ContextBuilders as context builders
participant Templates as Jinja2Templates
participant APIs as injected APIs
Client->>RegisterPublicRoutes: GET /bounties?status=OPEN&q=proof
RegisterPublicRoutes->>APIs: list_bounties_by_status()
APIs-->>RegisterPublicRoutes: bounty_list
RegisterPublicRoutes->>ContextBuilders: public_bounties_context(bounties, status, q)
ContextBuilders-->>RegisterPublicRoutes: normalized context with summaries
RegisterPublicRoutes->>Templates: render bounties.html(context)
Templates-->>Client: HTML response
Client->>RegisterPublicRoutes: GET /wallets/{address}
RegisterPublicRoutes->>ContextBuilders: wallet_page_context(session, address)
ContextBuilders->>APIs: wallet_by_address(normalized_address)
APIs-->>ContextBuilders: wallet data + ledger transactions
ContextBuilders-->>RegisterPublicRoutes: wallet context
RegisterPublicRoutes->>Templates: render wallet.html(context)
Templates-->>Client: HTML response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 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 |
ayskobtw-lil
left a comment
There was a problem hiding this comment.
No blockers from my review on current head f948b96.
Evidence checked:
- Inspected
app/public_routes.py, the removed public route registrations inapp/main.py, andtests/test_public_routes.py. - Verified
register_public_routes()preserves the moved/bounties,/bounties/{id},/ledger,/ledger/{sequence},/wallets,/wallets/{address},/transfer,/proofs/{hash}, and/docsroute behavior while keeping wallet DB lookups scoped tosession_scope(). - Verified the public bounties context keeps status/query display normalization and still delegates actual filtering to
list_bounties_by_status(). - Confirmed the GitHub hosted quality check is green from the PR metadata.
Local validation:
python -m pytest tests/test_public_routes.py tests/test_bounty_pages.py tests/test_wallet_api.py::test_wallet_pages_expose_transfer_and_github_claim_flows tests/test_api_mcp.py::test_head_requests_match_get_routes_without_body tests/test_api_mcp.py::test_docs_page_lists_live_ltclab_urls tests/test_api_mcp.py::test_explorer_links_ledger_proof_and_account tests/test_api_mcp.py::test_ledger_page_highlights_bounty_payment_and_release tests/test_api_mcp.py::test_ledger_page_uses_wrapping_entry_cards -q-> 13 passedpython -m ruff check app/public_routes.py app/main.py tests/test_public_routes.py-> passedpython -m ruff format --check app/public_routes.py app/main.py tests/test_public_routes.py-> 3 files already formattedpython -m mypy app/public_routes.py app/main.py-> successgit diff --check origin/main...HEAD-> clean
rebel117
left a comment
There was a problem hiding this comment.
Reviewed the public page route extraction. Clean and faithful.
A few observations:
-
Route registration uses explicit dependency injection via keyword arguments (db_url, templates, helper callables) rather than reaching for module-level state — this is a good pattern that makes the routes testable independently.
-
Context helpers (
public_bounties_context,wallets_page_context,wallet_page_context) are well-extracted, keeping the route handlers thin. Thehubroute staying in main.py is reasonable since it has unique context building viamergework_hub_context. -
The test covers the status/query normalization in
public_bounties_context. The route handlers themselves are thin TemplateResponse wrappers so minimal test coverage is acceptable for an extraction. -
No behavioral changes detected — all URL paths, query parameters, template names, and response shapes are preserved.
Line reduction in main.py is significant (61 lines of route definitions replaced by a single function call).
Extract the read-only public page route registration from
app/main.pyintoapp/public_routes.py.This keeps
app/main.pyfocused on application assembly, API/auth boundaries, and shared wiring while giving the public bounties, ledger, wallet, proof, transfer, and docs pages a coherent route module. Public behavior is intended to stay unchanged; the route module receives existing API/page callbacks fromcreate_appinstead of owning unrelated API logic.Bounty #390
Validation:
./.venv/bin/python -m pytest -q-> 378 passed./.venv/bin/python -m ruff check .-> passed./.venv/bin/python -m ruff format --check .-> 67 files already formatted./.venv/bin/python -m mypy app-> success./.venv/bin/python scripts/docs_smoke.py-> docs smoke okSummary by CodeRabbit
Refactor
Tests