Skip to content

ref: Extract public page routes#394

Open
campersurfer wants to merge 1 commit into
ramimbo:mainfrom
campersurfer:campersurfer/ref/extract-public-page-routes
Open

ref: Extract public page routes#394
campersurfer wants to merge 1 commit into
ramimbo:mainfrom
campersurfer:campersurfer/ref/extract-public-page-routes

Conversation

@campersurfer
Copy link
Copy Markdown

@campersurfer campersurfer commented May 26, 2026

Extract the read-only public page route registration from app/main.py into app/public_routes.py.

This keeps app/main.py focused 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 from create_app instead 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 ok

Summary by CodeRabbit

  • Refactor

    • Reorganized internal routing architecture to improve code organization and maintainability of public-facing endpoints.
  • Tests

    • Added test coverage for input normalization in bounty filtering, ensuring consistent processing of filter parameters.

Review Change Stack

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>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 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: 23d6f7c1-06d4-4dc1-9303-77e4567b0038

📥 Commits

Reviewing files that changed from the base of the PR and between a1795ed and f948b96.

📒 Files selected for processing (3)
  • app/main.py
  • app/public_routes.py
  • tests/test_public_routes.py

📝 Walkthrough

Walkthrough

This PR extracts public-facing HTML route registration from app/main.py into a new dedicated module app/public_routes.py. The module provides context builders for template rendering and centralizes route wiring, allowing main.py to delegate setup via a single register_public_routes() function call.

Changes

Public Routes Refactoring

Layer / File(s) Summary
Public routes module with context builders and registration
app/public_routes.py
New module provides helper functions public_bounties_context, wallets_page_context, and wallet_page_context to normalize inputs and serialize data for Jinja2 template rendering. Main register_public_routes() function wires HTML GET endpoints (/bounties, /ledger, /wallets, /transfer, /proofs, /docs) using injected API callables and session-scoped database access where needed.
Route delegation in main.py
app/main.py
Imports updated to use register_public_routes from the new module; public route definitions removed and replaced with centralized register_public_routes() call passing app, db_url, templates, and relevant handler functions.
Context builder validation
tests/test_public_routes.py
New test confirms public_bounties_context normalizes filter inputs (whitespace stripping and case handling) and includes computed summary fields in returned context.

Sequence Diagram

sequenceDiagram
  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
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

  • ramimbo/mergework#366: Refactors app/main.py to use ledger view helpers (ledger_entry_to_dict, recent_ledger_entries) matching the imports introduced in this PR's main.py updates.
  • ramimbo/mergework#388: Parallel refactoring that extracts account route registration from app/main.py into a dedicated register_account_routes module, using the same delegation pattern as this PR's public routes extraction.

Suggested reviewers

  • newmattock
  • tolga-tom-nook
  • bitdamii
🚥 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 'ref: Extract public page routes' directly and clearly describes the main refactoring change: extracting public page route registration from main.py into a new module.
Description check ✅ Passed The PR description includes a clear summary of the extraction, context for why it's being done, the affected bounty reference, and comprehensive validation evidence covering all required checks (pytest, ruff check, ruff format, mypy, and docs smoke).
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

@ayskobtw-lil ayskobtw-lil 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 on current head f948b96.

Evidence checked:

  • Inspected app/public_routes.py, the removed public route registrations in app/main.py, and tests/test_public_routes.py.
  • Verified register_public_routes() preserves the moved /bounties, /bounties/{id}, /ledger, /ledger/{sequence}, /wallets, /wallets/{address}, /transfer, /proofs/{hash}, and /docs route behavior while keeping wallet DB lookups scoped to session_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 passed
  • python -m ruff check app/public_routes.py app/main.py tests/test_public_routes.py -> passed
  • python -m ruff format --check app/public_routes.py app/main.py tests/test_public_routes.py -> 3 files already formatted
  • python -m mypy app/public_routes.py app/main.py -> success
  • git diff --check origin/main...HEAD -> clean

Copy link
Copy Markdown

@rebel117 rebel117 left a comment

Choose a reason for hiding this comment

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

Reviewed the public page route extraction. Clean and faithful.

A few observations:

  1. 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.

  2. Context helpers (public_bounties_context, wallets_page_context, wallet_page_context) are well-extracted, keeping the route handlers thin. The hub route staying in main.py is reasonable since it has unique context building via mergework_hub_context.

  3. 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.

  4. 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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants