Skip to content

Bounty #427: Add ledger entry type filters#511

Open
jtc268 wants to merge 2 commits into
ramimbo:mainfrom
jtc268:codex/b427-ledger-type-filter
Open

Bounty #427: Add ledger entry type filters#511
jtc268 wants to merge 2 commits into
ramimbo:mainfrom
jtc268:codex/b427-ledger-type-filter

Conversation

@jtc268
Copy link
Copy Markdown

@jtc268 jtc268 commented May 27, 2026

Refs #427

This adds type filters to the public ledger so contributors can scan one workflow at a time instead of reading every reserve, payment, release, transfer, claim, and genesis row together.

Before:

  • /ledger always showed the latest ledger rows as one mixed stream.
  • /api/v1/ledger could limit rows but could not filter by entry type.

After:

  • /ledger shows filter links for All, Bounty Reserve, Bounty Payment, Bounty Release, GitHub claim, Wallet transfer, and Genesis.
  • /api/v1/ledger?type=bounty_payment returns only payment rows while keeping proof hashes attached.
  • Blank or type=all stays unfiltered; unsupported types return a bounded 400.

Checked against the ledger entry types produced by app/ledger/service.py.

Validation:

  • ./.venv/bin/python -m pytest tests/test_bounty_pages.py::test_ledger_page_and_api_filter_by_entry_type tests/test_bounty_pages.py::test_ledger_and_proof_pages_make_bounty_payments_scannable tests/test_ledger_views.py -q -> 4 passed
  • ./.venv/bin/python -m ruff check app/ledger_views.py app/main.py app/public_routes.py tests/test_bounty_pages.py -> passed
  • ./.venv/bin/python -m ruff format --check app/ledger_views.py app/main.py app/public_routes.py tests/test_bounty_pages.py -> passed
  • ./.venv/bin/python -m mypy app/ledger_views.py app/public_routes.py app/main.py -> passed
  • git diff --check -> clean

Summary by CodeRabbit

  • New Features

    • Filter ledger entries by type via a new query parameter on the ledger page and API.
    • UI filter navigation shows available entry types and highlights the selected filter.
    • "All" option and case-insensitive type matching supported.
  • Bug Fixes

    • Invalid type selections return a clear, user-friendly error (400).
  • Tests

    • End-to-end tests added to verify API and page filtering behavior.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 001b72a8-f2f2-4a78-a1ad-997703aed8e2

📥 Commits

Reviewing files that changed from the base of the PR and between 09a99b3 and 5d707a8.

📒 Files selected for processing (1)
  • tests/test_bounty_pages.py

📝 Walkthrough

Walkthrough

This PR adds ledger entry type filtering across the backend API and frontend HTML routes. A new validation layer enforces known entry types and maps "all"/empty to no filter. The API endpoint accepts a type query parameter, the HTML route reads it from the URL, and the template renders with dynamically passed type options. Filter UI styling highlights the active filter link.

Changes

Ledger entry type filtering

Layer / File(s) Summary
Ledger type validation and filtering logic
app/ledger_views.py
LEDGER_TYPE_LABELS and LEDGER_TYPE_FILTER_ERROR constants are introduced. normalize_ledger_type_filter() trims/lowers input, maps "all"/empty to None, validates against known types, and raises ValueError for unrecognized entries. recent_ledger_entries() accepts optional entry_type parameter, normalizes it, and conditionally filters the query.
API endpoint filtering
app/main.py
Imports type labels and error message. /api/v1/ledger endpoint accepts optional type query parameter, passes it to recent_ledger_entries(), and converts validation ValueError into a 400 response. Public routes wiring passes type options to handlers.
HTML route and template filtering
app/public_routes.py, app/templates/ledger.html
register_public_routes() signature updated to accept api_ledger callback with page size and optional entry type, plus ledger_type_options parameter. /ledger HTML route reads optional type query parameter, calls API with pagination, and passes selected_type and type_options to template. Template dynamically derives labels from passed options instead of hardcoded mapping; conditionally displays filter status only when type is set and not "all".
Filter navigation styling
app/static/style.css
Adds .filter-nav link styles with special highlighted state for the active filter using aria-current="page" attribute.
End-to-end filtering test
tests/test_bounty_pages.py
New test creates bounty with payment proof, closes it, then verifies API filtering by type=bounty_payment (correct entry type and proof hash), API rejection of type=bogus with 400 and explicit error, and HTML page rendering with correct filter selection and content visibility.

Possibly related PRs

  • ramimbo/mergework#366: Modifies the same recent_ledger_entries() function; #366 extracts/serializes it while this PR extends it with entry type filtering and validation on the same code path.
🚥 Pre-merge checks | ✅ 6
✅ Passed checks (6 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Bounty #427: Add ledger entry type filters' directly and concretely names the changed surface—adding type filters to the ledger—and references the bounty number.
Description check ✅ Passed The description covers the problem (mixed ledger stream, no type filtering), the solution (filter links and API parameter), validation results, and bounty reference, but omits the structured template sections including Evidence (confusion/capacity/scope) and Test Evidence checkboxes.
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.
Mergework Public Artifact Hygiene ✅ Passed No problematic investment, price, cash-out, payout, or security claims detected. README describes MRWK as native ledger coin with optional future snapshots/bridges/onchain paths.
Bounty Pr Focus ✅ Passed PR #511 narrowly implements Bounty #427 ledger type filtering across all 6 claimed files with proper validation, error handling, and comprehensive tests. No scope creep detected.

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


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

Copy link
Copy Markdown
Contributor

@GHX5T-SOL GHX5T-SOL left a comment

Choose a reason for hiding this comment

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

Reviewed current head 09a99b326b13eabe0da2ca457486eb8de80872d9 as a non-author.

I checked the ledger filter path across app/ledger_views.py, app/main.py, app/public_routes.py, app/templates/ledger.html, and the regression coverage in tests/test_bounty_pages.py. The implementation keeps blank and type=all requests unfiltered, normalizes valid type names, returns a bounded 400 for unsupported types, and preserves proof hashes on filtered payment rows. I also checked the HTML route directly: /ledger?type=bogus returns the same bounded 400, while case/whitespace-normalized values such as Bounty_Payment and all render successfully.

Validation:

  • uv run --extra dev pytest -q tests/test_bounty_pages.py::test_ledger_page_and_api_filter_by_entry_type tests/test_bounty_pages.py::test_ledger_and_proof_pages_make_bounty_payments_scannable tests/test_ledger_views.py -> 4 passed
  • uv run --extra dev pytest -q tests/test_bounty_pages.py tests/test_ledger_views.py tests/test_public_routes.py -> 11 passed
  • uv run --extra dev pytest -q -> 415 passed
  • uv run --extra dev ruff format --check . -> 79 files already formatted
  • uv run --extra dev ruff check . -> All checks passed!
  • uv run --extra dev mypy app -> Success: no issues found in 30 source files
  • uv run --extra dev python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check origin/main...HEAD -> clean
  • git diff --no-ext-diff origin/main...HEAD | gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found

GitHub readback showed the quality workflow passing on this head. I do not see a blocker in the reviewed scope.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
tests/test_bounty_pages.py (1)

479-502: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add boundary assertions for normalization paths (type=BOUNTY_PAYMENT and blank/whitespace).

The feature normalizes case and trims empty filters to unfiltered mode, but this test only checks lowercase and all. Add explicit assertions for uppercase and blank/whitespace values to lock in that behavior.

Proposed test additions
     payment_rows = client.get("/api/v1/ledger?type=bounty_payment")
     assert payment_rows.status_code == 200
     assert [row["type"] for row in payment_rows.json()] == ["bounty_payment"]
     assert payment_rows.json()[0]["proof_hash"] == proof.hash
+    uppercase_rows = client.get("/api/v1/ledger?type=BOUNTY_PAYMENT")
+    assert uppercase_rows.status_code == 200
+    assert [row["type"] for row in uppercase_rows.json()] == ["bounty_payment"]
+
+    blank_rows = client.get("/api/v1/ledger?type=   ")
+    assert blank_rows.status_code == 200
+    assert len(blank_rows.json()) >= 1

@@
     all_page = client.get("/ledger?type=all")
     assert all_page.status_code == 200
     assert 'href="/ledger" aria-current="page"' in all_page.text
+    blank_page = client.get("/ledger?type=   ")
+    assert blank_page.status_code == 200
+    assert 'href="/ledger" aria-current="page"' in blank_page.text

As per coding guidelines tests/**/*.py: "Focus on whether tests prove the changed behavior and include negative, replay, boundary, or regression cases where relevant."


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 0aade768-c057-4bf8-ab72-79f875ef3a87

📥 Commits

Reviewing files that changed from the base of the PR and between d8532d4 and 09a99b3.

📒 Files selected for processing (6)
  • app/ledger_views.py
  • app/main.py
  • app/public_routes.py
  • app/static/style.css
  • app/templates/ledger.html
  • tests/test_bounty_pages.py

Copy link
Copy Markdown
Contributor

@GHX5T-SOL GHX5T-SOL left a comment

Choose a reason for hiding this comment

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

Reviewed current head 5d707a8dabde2f7837d77deb967ccab24b4033b3 as non-author GHX5T-SOL.

This is a current-head follow-up to my earlier approval on 09a99b326b13eabe0da2ca457486eb8de80872d9. The new commit only strengthens the ledger type-filter regression after CodeRabbit's boundary note: tests/test_bounty_pages.py now covers uppercase type=BOUNTY_PAYMENT normalization for API and page routes, and blank/whitespace API filters still return the unfiltered ledger stream. That addresses the missing boundary coverage without changing the implementation surface.

Validation on this exact head:

  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python -m pytest -q tests/test_bounty_pages.py::test_ledger_page_and_api_filter_by_entry_type tests/test_bounty_pages.py::test_ledger_and_proof_pages_make_bounty_payments_scannable tests/test_ledger_views.py -> 4 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python -m pytest -q tests/test_bounty_pages.py tests/test_ledger_views.py tests/test_public_routes.py -> 11 passed
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python -m pytest -q -> 415 passed
  • /home/kali/money/mergework/.venv/bin/ruff format --check . -> 79 files already formatted
  • /home/kali/money/mergework/.venv/bin/ruff check . -> All checks passed!
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python -m mypy app -> Success: no issues found in 30 source files
  • PYTEST_DISABLE_PLUGIN_AUTOLOAD=1 /home/kali/money/mergework/.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check upstream/main...HEAD -> clean
  • git diff --no-ext-diff upstream/main...HEAD | gitleaks stdin --no-banner --redact --exit-code 1 -> no leaks found

GitHub readback shows the PR is open, non-draft, mergeable CLEAN, with quality workflow and CodeRabbit successful. I do not see a remaining blocker on the current head.

Copy link
Copy Markdown
Contributor

@GHX5T-SOL GHX5T-SOL left a comment

Choose a reason for hiding this comment

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

Re-reviewed current head 5d707a8dabde2f7837d77deb967ccab24b4033b3 after the added ledger type-normalization coverage. The follow-up commit addresses the CodeRabbit normalization-boundary note by covering uppercase type=BOUNTY_PAYMENT, blank/whitespace filters, and the normalized HTML current-state link. Validation rerun locally: focused ledger tests 4 passed; touched public-route/ledger tests 11 passed; full pytest 415 passed; Ruff format/check passed; mypy app passed; docs smoke passed; git diff --check origin/main...HEAD clean; diff-only Gitleaks found no leaks. GitHub quality checks are passing and CodeRabbit skipped the latest rerun.

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.

2 participants