Skip to content

gui: Warn the checksum completer, not just the recoverer - #10

Closed
BenWestgate wants to merge 2 commits into
gui-reference-v1from
gui-completion-warning
Closed

BenWestgate wants to merge 2 commits into
gui-reference-v1from
gui-completion-warning

Conversation

@BenWestgate

@BenWestgate BenWestgate commented Sep 20, 2026 •

Copy link
Copy Markdown
Owner

Superseded by #65 (gui-reviewability-v1). The clean GUI rebuild contains this focused fix and its coverage:

  • the low-discrimination gate explicitly covers both checksum completion and damaged-card recovery;
  • 13-symbol and 15-symbol whole-checksum cases are tested;
  • user/developer GUI documentation carries the same warning and no-completer policy;
  • the GTK walkthrough verifies the candidate remains hidden until literal YES and that checksum replacement is not presented as a repair strategy.

Keeping this older divergent GUI branch open would duplicate review and integration work. Review the behavior in #65/#66 and the eventual frozen GUI integration candidate instead.

Original scope: warn before GUI checksum completion and cover both checksum lengths.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@BenWestgate

Copy link
Copy Markdown
Owner Author

Concept ACK

@BenWestgate

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 11e2a35ff7

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/codex32_gui/pages.py Outdated
A full Codex32 checksum is 13 symbols on short cards and 15 on long cards. Entering question marks for that entire suffix reaches the same low-discrimination gate as a badly damaged card, so the warning must also address someone completing a hand-written backup.

Explain that earlier transcription mistakes become undetectable once the checksum is completed, and explicitly forbid replacing a failing checksum to make a card validate. Keep the route undiscoverable in the GUI, where worksheet completion is not the target workflow.

Cover both 13-symbol and 15-symbol checksum-completion routes.

Refs BlockstreamResearch/codex32#78

Validation: 34 GUI-reading tests passed; ruff check passed; ruff format --check passed; git diff --check passed.
@BenWestgate
BenWestgate force-pushed the gui-completion-warning branch from 11e2a35 to 8f1001f Compare September 21, 2026 09:38
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@BenWestgate BenWestgate added gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. area: gui Graphical user interface behavior. area: wallet/core Wallet integration and Bitcoin Core boundaries. labels Sep 24, 2026
@BenWestgate

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Keep it up!

Reviewed commit: 8f1001f619

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

AI-generated review (Claude), posted at the maintainer's request.

Concept ACK 8f1001f. One merge blocker with #28:

  • GUI budget (non-comment lines under src/codex32_gui, limit < 2000): base 1848, #10 1869, #28 1983, #10+#28 merged 2004. Whichever lands second fails test_the_gui_keeps_its_own_size_budget.
  • The copy is accurate: 13/15 trailing ? is a whole short/long checksum, and the added test covers both lengths.

Comment thread src/codex32_gui/pages.py Outdated
Compress the gate docstring without changing behavior. This leaves the combined #10 + #28 GUI at 1,997 non-comment lines, below the enforced 2,000-line budget.

@BenWestgate BenWestgate left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

AI-generated review, posted at the maintainer's request.

ACK 3d6483e. The docstring-only follow-up fixes the #28 combined GUI-budget blocker.

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

Labels

area: gui Graphical user interface behavior. area: wallet/core Wallet integration and Bitcoin Core boundaries. gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant