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

This comment has been minimized.

@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

This comment has been minimized.

@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