Skip to content

docs: Prepare the v1 review handoff #38

Description

@BenWestgate

Before freezing the v1 review candidate:

  • Add docs/developer/reviewing.md with the exact base and pre-handoff integration commits, scope, review order, PR stack, evidence-regeneration commands, and intentional exclusions.
  • Replace the inherited Bitcoin Core boilerplate and dead links in CONTRIBUTING.md.
  • Mark the alignment benchmark as a snapshot of commit 6802d86; separate historical measurements from current verification, and identify the committed host-specific raw benchmark files as historical evidence rather than reproducible current measurements.
  • Correct the documented package-level public API count. Current reviewability-v1 actually exports 24 names while docs/developer/api.md still says 25; after wallet: Privatize Core descriptor records #64 removes the obsolete public core_descriptors export, the frozen v1 package __all__ should contain 23 names. api: Expose reference-vector helpers #53 adds supported reference-vector helpers at their owning modules and deliberately does not add them to package-level codex32.__all__.
  • Update the stale GUI budget in docs/developer/api.md to the separately enforced <2050 logical-line limit. The maintainer explicitly approved gui: Refresh empty wallets automatically #66's GUI-specific increase on 2026-09-30; this is separate from the library <5200 limit.
  • Document the Core boundary precisely: ms32 secret and ms32 share require Bitcoin Core for fingerprint-aware recovery/output; ms32 correct connects to Core when a master-seed correction needs fingerprint ranking/output, while valid/no-result paths may finish before that connection. The corresponding generic codex32 secret / share / correct commands are the Core-independent fallback.
  • Record the deliberate mid-recovery secret behavior: if the operator supplies a complete valid S while entering shares, recovery stops using the partial share set and deliberately switches to that supplied secret. PR cli: Announce recovery secret switch #95 makes that mode switch explicit to the operator; it belongs in the frozen library/CLI candidate before the handoff.
  • Record the deliberate parser divergence for an unshared secret with threshold digit 1: v1 remains stricter than the BIP-93 reference decoder and accepts only the project's documented unshared/shared header forms. State this explicitly so interoperability reviewers do not mistake the difference for an untracked parser bug.
  • Point reviewers to the existing installed-package and GUI size-budget tests. The v1 installed-package cap is the maintainer-authorized <5200; do not take a pre-release refactor solely to recover the old <5000 target.
  • Record the deliberate v1 exception for mixed-case correction scheduling: the public correction engine and standalone CLI retain parallel orchestration through v1 because their search-planning contracts differ (CorrectionContext generic reachable lengths versus ms32 --bytes/profile/tie-break behavior). Both paths use the same required-before-optional ordering and capture-accounting contract and are covered by focused regressions plus the frozen differential verifier. Centralizing them is post-v1 architecture work; do not take a pre-release refactor solely to remove roughly 100 lines.
  • Include the late user-facing v1 documentation before the handoff: docs: Restore the qr steps for offline signing #93 restores the reviewed Bitcoin Core offline-signing QR workflow, docs: Size recovery cards to the backup length #96 supplies the 48- and 74-character printable recovery cards, and docs: Answer first-time questions in the user guide #97 answers the first-time recovery questions against the final wallet: Require the recorded fingerprint before import #57 contract. docs: Size recovery cards to the backup length #96 requires a letter-landscape print-preview of both templates. docs: Answer first-time questions in the user guide #97 must link those two templates from its length answer and state that 54/61/67/127-character backups do not yet have dedicated printable cards. These Claude/agent-authored documentation commits require responsible-human rewrite/squash before integration.
  • Freeze and identify both the library/CLI candidate and the GUI integration candidate. The library/CLI candidate must include wallet: Check existing seed before sharing #81's ms32 create --existing wallet-record decision before any new share ceremony, cli: Remove unreachable recovery and search paths #105's final unreachable-path cleanup (the patch-identical replacement for historical cli: Remove unreachable recovery and search paths #98), and cli: Announce recovery secret switch #95's explicit mid-recovery secret-switch notice. The GUI candidate must include gui: Add optional graphical interface #65 plus gui: Refresh empty wallets automatically #66, Tails field-test PR gui: Apply Tails field-test feedback #77 (closing gui: Keep card-count choices neutral #71–gui: Fit the finished wallet identity on one screen #74), gui: Let the home window choose its height #78 (closing gui: Size the home window to its content #75), and the reviewed restore-authentication behavior. gui: Refresh empty wallets automatically #66's automatic wallet refresh must preserve an explicit selection when possible, disable Continue if that wallet disappears, retry transient read-only refresh failures, and allow read-only poll workers to end with the process while mutation/relocking workers remain non-daemon. gui: Apply Tails field-test feedback #77/gui: Let the home window choose its height #78 still require the supported Tails guest-resolution visual checks before the candidate is frozen. gui: Give home actions distinct artwork #76 is explicitly deferred until that Tails visual pass identifies which already-distinct bundled book graphics are being confused; do not guess an asset replacement from source filenames alone.
  • The final fresh adversarial review must cover the GUI as well as the library and CLI, with manual GUI test steps recorded for behavior that CI cannot exercise.
  • Link the guide from the final human-authored PR description, which must pin the complete candidate commit or commits. Human integration may squash/rewrite stacked AI-assisted follow-ups where the repository authorship policy requires it, after dependency order is settled.

Human integration order

Use this order to avoid repeatedly invalidating reviewed stacks:

  1. Wallet: use Core for setup and remove test crypto deps #7 and its focused real-Core fixture follow-up ci: Verify wallet fixtures against Bitcoin Core #51 are merged into reviewability-v1.
  2. Review/integrate the correction line through correct: Interpret mixed-case damage #42. Define correction exit statuses #45 and the behavior-preserving cleanup Remove pre-review CLI dead code #46 are already merged into correct: Interpret mixed-case damage #42's head branch. The current correct: Interpret mixed-case damage #42 head also contains the focused grouped-input/immutable-prefix follow-up from the latest review. Current-head CI is green and all inline review threads are resolved; the final deadline thread was closed after benchmark evidence showed both required interpretations finish well inside the shared 10-second deadline. Review Remove pre-review CLI dead code #46's focused cleanup diff as part of this stacked correct: Interpret mixed-case damage #42 review; it is no longer a separate post-correct: Interpret mixed-case damage #42 integration step. Before integration, fold the final grouped-prefix fixup into the mixed-case commit while retaining Define correction exit statuses #45's exit-status change and Remove pre-review CLI dead code #46's cleanup as separate atomic review units.
  3. Integrate the refreshed restore line in order wallet: Require the recorded fingerprint before import #57 → cli: Remove unreachable recovery and search paths #105 → wallet: Distinguish unavailable Bails checks #80 → wallet: Check existing seed before sharing #81 → cli: Announce recovery secret switch #95. wallet: Require the recorded fingerprint before import #57 is replayed directly on current correct: Interpret mixed-case damage #42 with an unchanged reviewed patch-id. cli: Remove unreachable recovery and search paths #105 is the patch-identical replacement for historical cli: Remove unreachable recovery and search paths #98 and deliberately moves immediately after wallet: Require the recorded fingerprint before import #57: its reviewed dead-code cleanup reduces that tip to 5,179 logical lines, allowing wallet: Distinguish unavailable Bails checks #80, wallet: Check existing seed before sharing #81 and cli: Announce recovery secret switch #95 to remain below the authorized <5200 cap without another line-saving patch. wallet: Distinguish unavailable Bails checks #80's two reviewed patch-ids and wallet: Check existing seed before sharing #81's five reviewed patch-ids are unchanged. cli: Announce recovery secret switch #95 then makes the already-intended behavior explicit when a complete valid secret replaces a partial share-recovery session. Rerun the identity-mismatch regression, wallet: Distinguish unavailable Bails checks #80 no-record/identifier regressions, wallet: Check existing seed before sharing #81 early-gate regressions, cli: Remove unreachable recovery and search paths #98 correction/CLI regressions, cli: Announce recovery secret switch #95 recovery-mode-switch regression, the real-Core fixture, and the final full suite on the resolved tip. cli: Remove unreachable recovery and search paths #105/wallet: Distinguish unavailable Bails checks #80/wallet: Check existing seed before sharing #81/cli: Announce recovery secret switch #95 are agent-authored follow-ups and require the repository's responsible-human rewrite/squash policy before integration.
  4. Refresh onto that settled correct: Interpret mixed-case damage #42 → wallet: Require the recorded fingerprint before import #57 → cli: Remove unreachable recovery and search paths #105 → wallet: Distinguish unavailable Bails checks #80 → wallet: Check existing seed before sharing #81 → cli: Announce recovery secret switch #95 tip, then integrate the small overlapping foundation fixes: wallet: Validate Bitcoin Core state types #12, bip93: Reject non-ASCII normalized input #13, bip93: reject HRPs longer than 83 characters #33, and wallet: Privatize Core descriptor records #64. For wallet: Privatize Core descriptor records #64, replay only the still-relevant API-surface cleanup onto Wallet: use Core for setup and remove test crypto deps #7's Core-native implementation; do not restore its obsolete pre-Wallet: use Core for setup and remove test crypto deps #7 _bitcoin_core.py import-record hunk. Where two overlap, preserve the already-reviewed behavior and perform only the mechanical restack needed by the moved base.
  5. Integrate the audit/security/release support work that does not own runtime behavior: docs: Record security audit verdict #23, docs: Define trusted-computer boundary #59, and release: Qualify exact artifacts before publish #52.
  6. Refresh api: Expose reference-vector helpers #53 exactly once after bip93: Reject non-ASCII normalized input #13, bip93: reject HRPs longer than 83 characters #33, correct: Interpret mixed-case damage #42/wallet: Require the recorded fingerprint before import #57, and wallet: Privatize Core descriptor records #64 are settled; Wallet: use Core for setup and remove test crypto deps #7 is already in the base. Preserve its supported module-level vector API and centralize the ASCII-only lower helper there. After wallet: Privatize Core descriptor records #64, package-level codex32.__all__ should contain 23 names; api: Expose reference-vector helpers #53's module-level helper publication does not change that count. Rewrite/squash its Codex-authored follow-up under the responsible human author before merge.
  7. Refresh the late user-facing documentation on the settled runtime/API tip: docs: Restore the qr steps for offline signing #93, then docs: Size recovery cards to the backup length #96, then docs: Answer first-time questions in the user guide #97. docs: Restore the qr steps for offline signing #93 is already agent-reviewed and CI-green. docs: Size recovery cards to the backup length #96 is agent-reviewed and CI-green but still needs the two-template letter-landscape print-preview. docs: Answer first-time questions in the user guide #97's current head is CI-green and already fixes the random-index/default wording; finish the remaining recovery-card link/rarer-length qualification, rerun its review, then rewrite/squash the Claude/agent-authored documentation commits under the responsible human author.
  8. Rebase the clean GUI stack onto the settled library/CLI tip and review it in order: gui: Add optional graphical interface #65 → gui: Refresh empty wallets automatically #66 → gui: Apply Tails field-test feedback #77 → gui: Let the home window choose its height #78. Replay/squash only the reviewed GUI restore-authentication delta from gui: Require the recorded fingerprint before import #28; do not merge gui: Require the recorded fingerprint before import #28's duplicated historical library snapshot. Resolve gui: Give home actions distinct artwork #76 from actual Tails visual evidence, then run the recorded Tails/manual GUI qualifications.
  9. Only after those tips are frozen, open the focused handoff-document PR required by this issue and pin the exact candidate commits.
  10. Run final artifact qualification and a fresh adversarial review over the frozen library, CLI, GUI, and user-facing recovery documentation before any human-authored master integration.

Keep planning notes out of the shipped documentation. This handoff is intentionally the last documentation change, after the implementation, packaging, security, GUI, and user-facing documentation integration stacks are settled; opening its PR earlier would make the required commit identifiers stale.

Refs #5.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    area: guiGraphical user interface behavior.area: packaging/releasePackaging, artifacts, compatibility, and release qualification.documentationImprovements or additions to documentationgate: adversarial reviewResolve, merge, or explicitly defer before the next full adversarial review.

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions