Skip to content

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

Draft
kitwongpixel wants to merge 1 commit into
ramimbo:mainfrom
kitwongpixel:codex/collapse-proof-json
Draft

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

Conversation

@kitwongpixel
Copy link
Copy Markdown

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 26, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 80d3bb1a-6ba9-406f-ab75-29cc09e42955

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

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