Skip to content

gui: Apply Tails field-test feedback - #77

Open
BenWestgate wants to merge 5 commits into
codex/gui-wallet-refresh-v1from
codex/gui-tails-field-test
Open

BenWestgate wants to merge 5 commits into
codex/gui-wallet-refresh-v1from
codex/gui-tails-field-test

Conversation

@BenWestgate

@BenWestgate BenWestgate commented Sep 30, 2026 •

Copy link
Copy Markdown
Owner

Follow-up to #65/#66. This keeps the four Tails field-test findings in one focused presentation/copy review unit because they overlap the same GUI screen module.

CI follow-ups did not change GUI semantics:

  • the first run exposed the separately enforced GUI review-size budget (2,056 logical lines versus <2050); bootstrap-only cleanup preserves the budget rather than raising it;
  • the next run passed all 975 tests on Linux/macOS but exposed a Windows-only source-reading portability bug in the GUI boundary tests plus one Ruff formatting check. The tests now read Python source explicitly as UTF-8 and the bootstrap cleanup is Ruff-formatted;
  • the following exact-head run proved the remaining count was exactly 2,050, so one non-runtime main() docstring line was removed. The production GUI is now 2,049 logical review lines under the unchanged <2050 gate.

Current head: 72b5aae. The PR changes two GUI source files plus the GUI boundary test. It does not change codex32 parsing, card contents, Bitcoin Core import semantics, secret-handling channels, or #66 wallet polling behavior.

The final fit requirement in #74 still needs the normal manual Tails guest-resolution check before the GUI integration candidate is frozen. #78 is stacked after this PR for #75; #76 is explicitly deferred to the same visual pass because the six home actions already use six distinct bundled book-art files and the tester needs to identify which rendered graphics are being confused.

Closes #71
Closes #72
Closes #73
Closes #74

Disclosure: AI assistance was used for this authorized branch-to-branch follow-up. Human review remains required before integration.

@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 area: gui Graphical user interface behavior. area: security Security invariants, hardening, and security-sensitive boundaries. gate: adversarial review Resolve, merge, or explicitly defer before the next full adversarial review. labels Sep 30, 2026 — with ChatGPT Codex Connector

@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 release-gate review, posted at the maintainer's request.

Code ACK 2caf9c8, with one manual qualification item remaining before integration: verify the finished wallet-identity screen at the supported Tails guest resolution for #74.

  • #71 is copy-only: the preset behavior is unchanged and only “(recommended)” is removed.
  • #72 keeps uppercase four-character groups and existing per-group highlighting; the FlowBox may use the full row and wraps only when width requires it instead of forcing four columns.
  • #73 changes operator guidance only. Passphrase transport, clearing, wallet creation, and secret-handling code are unchanged.
  • #74 retains every identity field and replaces the vertically heavy preference rows with a selectable two-column grid; no release-specific window geometry is introduced.
  • #66 wallet polling/selection behavior is untouched.

The two curly-quote literal-vs-escape changes in this one-file diff are source-spelling-only and have identical runtime text; they are not a correctness blocker.

Automatic Codex review did not run because the account review-usage limit was reached. Do not spend another review request on this head; use this code review plus the normal CI, then do the Tails visual fit check.

@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.

Updated release-gate review for current head fc648a3.

Code ACK. The second commit is mechanical review-budget cleanup only: it removes the module docstring and shortens two equivalent GTK constructor/call layouts in app.py; runtime behavior is unchanged. The resulting source count should be 2,048 logical GUI lines, retaining the enforced <2050 budget rather than raising it.

The only non-CI qualification still required is the manual Tails guest-resolution fit check for #74. No further automated Codex review request is warranted on this head.

@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.

Updated release-gate review for current head 4edeb06.

Code ACK, subject to the already-recorded manual Tails fit check for #74.

The two CI follow-ups are mechanical and justified by the failures they expose: app.py is now exactly Ruff's requested formatting while the GUI stays below the unchanged <2050 review budget, and test_gui_boundaries.py now decodes Python source explicitly as UTF-8 instead of relying on the Windows locale. The latter is the correct source-file contract and removes the cp1252-only failures without changing production behavior.

No further automated Codex review request is warranted on this head; use the current CI plus the manual Tails fit check.

@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.

Updated release-gate review for current head 72b5aae.

Code ACK, subject only to the already-recorded manual Tails fit check for #74. The only change since the prior reviewed head removes the non-runtime main() docstring after CI measured the GUI at exactly 2,050 logical lines; current production count is therefore 2,049 under the unchanged <2050 gate. No semantics changed.

@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.

Release-gate ACK 72b5aae for automated/code review. The exact-head Python-package run 464 completed successfully across the six matrix jobs, and the current head remains within the unchanged GUI review budget. No automated code blocker remains.

The only remaining acceptance item is the already-recorded manual Tails guest-resolution check for the wallet-identity fit (#74); #78 has the analogous manual home-height check. Do not trigger another automated review round.

Copy link
Copy Markdown
Owner Author

Agent release-gate review at exact head 72b5aae: reviewed the current field-test delta. It keeps the requested changes presentation/copy-only: removes the preset policy label, preserves card-group reading order while allowing natural wrap, replaces implementation-heavy encryption copy without changing secret transport, and compacts the final identity record without dropping fields. The UTF-8 test fixes and line-budget cleanup do not change runtime behavior. git diff --check is clean and exact-head run 36659417882 succeeded. No code-review blocker found. The supported Tails-resolution visual check for the finished record still remains a manual qualification step before freezing the GUI candidate.

This branch has not been deployed

No deployments
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: security Security invariants, hardening, and security-sensitive 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