Skip to content

[codex] collapse raw proof JSON on public proof page#467

Open
kitwongpixel wants to merge 4 commits into
ramimbo:mainfrom
kitwongpixel:codex/collapse-proof-json
Open

[codex] collapse raw proof JSON on public proof page#467
kitwongpixel wants to merge 4 commits into
ramimbo:mainfrom
kitwongpixel:codex/collapse-proof-json

Conversation

@kitwongpixel
Copy link
Copy Markdown

@kitwongpixel kitwongpixel commented May 26, 2026

Summary

  • Collapse the raw proof JSON on public proof pages behind a
    Details disclosure.
  • Keep the proof hash, issue link, bounty link, ledger link, and submission metadata visible up front.
  • Add a small style tweak so the disclosure reads like a secondary debugging panel instead of taking over the page.

Why

The public proof page is primarily an evidence/scanning page. The raw JSON is useful for debugging, but showing it expanded by default makes the main proof summary harder to scan on narrow screens and pushes the actionable links lower on the page.

Validation

  • ./.venv/bin/python -m pytest tests/test_bounty_pages.py -q
  • ./.venv/bin/python -m ruff check app tests
  • git diff --check

Refs #427

Summary by CodeRabbit

  • New Features

    • Raw proof JSON is now collapsible on the proof detail page, letting users expand or hide it for improved readability.
  • Style

    • Improved spacing and presentation of the raw proof block and its header for a tighter, clearer layout.
  • Tests

    • Added test coverage to verify the collapsible raw proof section displays and contains expected proof fields (e.g., bounty ID and amount).

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Caution

Review failed

Failed to post review comments

📝 Walkthrough

Walkthrough

The PR wraps the pretty-printed proof JSON in a collapsible <details class="raw-proof"> with a “Raw proof JSON” <summary>, adds CSS for the .raw-proof block, and extends tests to assert the new collapsible section and JSON keys.

Changes

Raw Proof Collapsible UI

Layer / File(s) Summary
Collapsible raw proof section
app/templates/proof.html
Replaces the standalone pretty-printed JSON <pre> with a <details class="raw-proof"> containing a Raw proof JSON <summary> and the same `proof
Styling for raw proof details
app/static/style.css
Adds .raw-proof rules: top margin for the container, bold/clickable summary with bottom margin, and removes top margin from nested <pre>.
Test assertions for raw proof UI
tests/test_bounty_pages.py
Test asserts the presence of <details class="raw-proof">, the "Raw proof JSON" summary, and that the section contains bounty_id and amount_mrwk JSON keys.
🚥 Pre-merge checks | ✅ 4 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Bounty Pr Focus ⚠️ Warning Diff matches stated scope, but proof.html keeps duplicate uncollapsed raw JSON on line 87 alongside the collapsible section, defeating the collapse objective. Remove line 87 duplicate <pre>{{ proof | tojson(indent=2) }}</pre>. Fix Ruff format failures and attach passing CI/test output.
Description check ❓ Inconclusive The description covers summary, rationale, and validation steps, but omits required template sections including Evidence subsections and Test Evidence checklist. Add missing template sections: Evidence (confusion addressed, capacity check, intended files, expected size, out of scope) and Test Evidence checklist with results.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concretely names the changed surface: collapsing raw proof JSON on the public proof page.
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 PR modifies only UI/test files (CSS, HTML template, tests) with no changes to README, docs, or PR description. Contains no new investment, price, cash-out, payout, or security claims about MRWK.

✏️ 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.

@ramimbo
Copy link
Copy Markdown
Owner

ramimbo commented May 26, 2026

Needs info before maintainer acceptance: this UI change needs passing checks, at least two evidence-backed reviews, and desktop/mobile screenshot evidence for the proof page because it changes public page presentation.

@ramimbo ramimbo added the mrwk:needs-info More information needed label May 26, 2026
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 ef900d26aa656012c4f8b2cb316cd1030c211fbb after the maintainer request for screenshot-backed proof-page review.

No UI/code blocker found in this focused pass. The change is scoped to the proof page: the primary payment facts, issue/bounty/ledger links, proof hash, ledger hash, and submission link remain visible, while the raw proof payload is moved into a closed <details class="raw-proof"> disclosure. That makes the public proof page easier to scan on narrow screens without removing the debugging JSON.

Screenshot evidence from a local seeded proof page:

Desktop 1280px Mobile 390px
mergework-pr467-proof-desktop.png mergework-pr467-proof-mobile.png

Evidence checked:

  • PR is open, draft, and mergeable at this head; no visible #467 review claim existed in #447 before this review.
  • Diff is limited to app/templates/proof.html, app/static/style.css, and tests/test_bounty_pages.py.
  • Seeded a local SQLite database with a bounty payment proof and served this PR locally on 127.0.0.1:8847.
  • Verified /proofs/c49d46bad2766749f137fc8de70b32b7276ac06d035e01d007d843517c80616c returned HTTP 200 with the expected security headers.
  • Verified the rendered HTML contains <details class="raw-proof">, <summary>Raw proof JSON</summary>, and the JSON <pre> inside the disclosure, while the summary fields stay above it.
  • Captured full-page screenshots at desktop 1280x900 and mobile 390x844; both keep the primary proof fields readable and show the raw JSON collapsed.

Validation:

  • ./.venv/bin/python -m pytest tests/test_bounty_pages.py::test_ledger_and_proof_pages_make_bounty_payments_scannable -q -> 1 passed
  • ./.venv/bin/python -m pytest tests/test_bounty_pages.py -q -> 7 passed
  • ./.venv/bin/python -m pytest -q -> 413 passed
  • ./.venv/bin/python scripts/docs_smoke.py -> docs smoke ok
  • ./.venv/bin/python -m ruff check . -> passed
  • ./.venv/bin/python -m ruff format --check . -> 79 files already formatted
  • ./.venv/bin/python -m mypy app -> success, no issues in 30 source files
  • git diff --check origin/main...HEAD -> clean
  • gitleaks detect --no-banner --redact --source . --log-opts origin/main..HEAD --exit-code 1 -> no leaks found

Process note: the PR is still draft and CodeRabbit skipped its normal review because of draft status. I would wait for the author to mark it ready and for the normal project CI/review status to be present before maintainer acceptance, but the focused proof-page behavior and screenshots look good from this local pass.

Copy link
Copy Markdown
Contributor

@yui-stingray yui-stingray 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 ef900d26aa656012c4f8b2cb316cd1030c211fbb against the current origin/main base d8532d4d909f47c689d3b88eb21461243097673f.

I do not see a blocking issue in the implementation itself. This is distinct from the existing screenshot/UI review: I focused only on current-base integration with the newer proof-hash canonicalization assertions.

I specifically checked that this branch, which split before the newer proof-hash canonicalization coverage landed on main, still composes with that current-base behavior. The PR diff applies/merges onto current origin/main without conflicts in the local merge tree, and the resulting test keeps both behaviors together: the raw proof JSON is moved behind <details class="raw-proof">, and uppercase proof-hash routes still render the canonical lowercase hash.

Validation I ran on the local merge tree:

  • TMPDIR=/var/tmp uv run --extra dev python -m pytest --capture=no tests/test_bounty_pages.py::test_ledger_and_proof_pages_make_bounty_payments_scannable -q -> 1 passed
  • TMPDIR=/var/tmp uv run --extra dev python -m pytest --capture=no tests/test_bounty_pages.py -q -> 7 passed
  • TMPDIR=/var/tmp uv run --extra dev ruff check tests/test_bounty_pages.py -> passed
  • git diff --cached --check && git diff --check -> whitespace check clean

Remaining acceptance caveat is process/state rather than a code blocker: the PR is still draft/UNSTABLE, and the maintainer asked for passing checks plus screenshot evidence before acceptance.

Copy link
Copy Markdown

@INDICUMA INDICUMA 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 draft PR #467 at head ef900d26aa656012c4f8b2cb316cd1030c211fbb.

Evidence:

  • Inspected app/templates/proof.html, app/static/style.css, and tests/test_bounty_pages.py.
  • The code change is focused: the existing raw proof JSON stays available but is moved behind a <details class="raw-proof"> disclosure, while the proof hash, issue, bounty, ledger, recipient, amount, and submission rows remain visible above it.
  • The regression assertion checks that the public proof page renders the details disclosure and summary text.
  • No secret, wallet material, price/off-ramp, or payout-guarantee wording is introduced.

Validation:

  • ./.venv312/bin/python -m pytest tests/test_bounty_pages.py -q -> 7 passed
  • ./.venv312/bin/python -m pytest -q -> 413 passed
  • ./.venv312/bin/python -m ruff check tests/test_bounty_pages.py -> passed
  • ./.venv312/bin/python -m ruff format --check tests/test_bounty_pages.py -> 1 file already formatted
  • ./.venv312/bin/python -m mypy app -> success
  • ./.venv312/bin/python scripts/docs_smoke.py -> docs smoke ok
  • git diff --check origin/main...HEAD -> clean

Assessment: I did not find a code-level blocker in this focused diff. The PR is still draft and maintainer acceptance still needs the requested public-page screenshot evidence plus the normal GitHub quality check run after it is marked ready.

@kitwongpixel
Copy link
Copy Markdown
Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 2dabcf07-a921-4f9f-bdc4-82a1f0852a5c

📥 Commits

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

📒 Files selected for processing (3)
  • app/static/style.css
  • app/templates/proof.html
  • tests/test_bounty_pages.py

Comment thread tests/test_bounty_pages.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mrwk:needs-info More information needed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants