Repository navigation
docs: Say what correct can and can't repair - #102
BenWestgate wants to merge 1 commit into
Conversation
This comment has been minimized.
This comment has been minimized.
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-generated review (Codex), posted at the maintainer's request.
ACK 4a0b18b. The capability documentation matches the implemented correction behavior.
BenWestgate
added
correction
|
V1 disposition (Codex, 2026-10-04): defer this post-audit correction-capability documentation from the v1 candidate. The docs have a current-head Codex ACK and are useful, but they document follow-up field testing rather than closing a validated finding from the supplied adversarial reviews. The v1 user-facing audit fixes already include the reviewed mixed-case correction behavior and the length-specific recovery guidance. Keep #102 open for post-v1 so the frozen review candidate stops accumulating late documentation scope. Human authorship/review remains required when integrated. |
BenWestgate
removed
the
gate: adversarial review
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head review at 4a0b18b: no findings. This is user-facing correction-capability documentation only and does not change recovery behavior.
4a0b18b to
fbb010b
Compare
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbb010bcbb
ℹ️ 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
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted current-head documentation review.
ACK c3d74ae (static). The earlier misleading correction limits now exclude whole-group omissions and preserve the 15-character long-checksum case. The confirmation warning distinguishes 13 and 15 erasures. Python-package CI passes. Post-v1 deferral remains appropriate.
| It can fix: | ||
|
|
||
| - up to 8 unreadable characters, up to 4 wrong ones, or a mix in which each | ||
| wrong character counts as two unreadable ones; |
There was a problem hiding this comment.
Reworded in 0bf2587. It now says that one wrong character uses up the room of two unreadable ones, and gives examples: 6 unreadable plus 1 wrong, and 4 unreadable plus 2 wrong. Both were measured to work.
Generated by Claude Code
|
|
||
| - up to 8 unreadable characters, up to 4 wrong ones, or a mix in which each | ||
| wrong character counts as two unreadable ones; | ||
| - 13 unreadable characters in a row, or 15 in a 127-character string; |
There was a problem hiding this comment.
Yes. In 0bf2587 all three places now say "long codex32 string", and the first one adds "(127 characters)".
Generated by Claude Code
| wrong character counts as two unreadable ones; | ||
| - 13 unreadable characters in a row, or 15 in a 127-character string; | ||
| - up to 4 missing characters, up to 4 extra ones, or one of each; | ||
| - one or two swaps of neighboring characters, or one swap of distant ones; |
There was a problem hiding this comment.
I'd much rather this say "one swap of characters anywhere in the string" (in addition to to _ other mistakes / unreadable).
But I'm surprised it couldn't do 3-4 adjacent swaps.
There was a problem hiding this comment.
Done in 0bf2587. The line now reads: "one swap of two characters anywhere in the string along with up to 2 wrong ones, or up to 3 swaps of neighboring characters". Measured one search at a time:
- 3 neighboring swaps: fixed in 1.3 to 1.7s.
- 4 neighboring swaps: timed out in all 3 trials.
- 1 distant swap plus 2 wrong: fixed at the 10-second deadline.
- 1 neighboring swap plus 3 wrong: fixed (the line you cACKed).
Generated by Claude Code
| - one or two whole four-character groups missing, extra or swapped, or one | ||
| group with every character wrong. |
There was a problem hiding this comment.
wasn't it:
one or two whole four-character groups, missing, extra, wrong or swapped?
the one group wrong seems wrong since we can correct 8 arbitrary erasures which is how these are solved.
There was a problem hiding this comment.
Missing, extra and swapped groups all work for two groups. Wrong groups only work for one, because the search never treats a whole group as unreadable. A wrong character counts as an error at an unknown position, and the checksum can locate about 4 of those. One fully wrong group is 4 errors, which is fixed. Two fully wrong groups are 8, which timed out in every trial. The 8-erasure capacity only applies if the user marks those characters as ?. So 0bf2587 adds: "If you know which groups are wrong, type their characters as ?: two groups are then 8 unreadable characters, which it can fix."
The search could also try each pair of groups as erasures. With 10 groups that is 45 cases of an 8-erasure fill, which is cheap, but it changes correction, so it would be a separate follow-up.
Generated by Claude Code
There was a problem hiding this comment.
"the search never treats a whole group as unreadable" it is supposed to and it used to correct any two groups where every character or more than most were errors provided there were no other errors in the string.
There was a problem hiding this comment.
The search could also try each pair of groups as erasures. With 10 groups that is 45 cases of an 8-erasure fill, which is cheap, but it changes correction, so it would be a separate follow-up.
The search absolutely used to do this and it still should since it's cheap and useful to try any 2 groups as 4 erasures each. I want that reverted or back in before v1, while this user document can be deferred.
|
|
||
| It can't fix 9 or more scattered unreadable characters, 5 or more missing | ||
| characters outside whole missing groups, more than 13 unreadable characters in a | ||
| row (15 in a 127-character string), or two groups that are both wrong. Some |
There was a problem hiding this comment.
it could on my CPU, what kind of puny CPU did you use?
There was a problem hiding this comment.
Run one search at a time on the same Xeon, two fully wrong groups (8 wrong characters) timed out in 3 of 3 trials, and so did two wrong groups plus 1 more wrong character. My reasoning in the other thread says 8 wrong characters at unknown positions is past what the checksum can locate on any CPU. Do you remember the case you ran? If some characters in those groups were still right, it would fall under "up to 4 wrong".
Generated by Claude Code
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted review of 0bf2587.
One wording fix remains: a timeout in the measured trials is not an unsupported repair. The default search also includes the 74-character target; --bytes 32 narrows that search. Please qualify the claim below. Otherwise the revised examples address the feedback. Documentation-only review; no runtime change.
| If whole groups are missing, give the seed's size with `--bytes`, such as | ||
| `--bytes 32` for a 74-character string. `correct` then tries only that length, | ||
| which is much faster: it fixes two missing groups in a 74-character string at | ||
| once with `--bytes 32` and not at all without it. |
There was a problem hiding this comment.
“not at all without it” overstates the experiment. _correction_plan() includes 74 among the default MS targets; the shared ten-second deadline can prevent completing that search. Suggest: “Specifying the size narrows the search and can let repairs finish before the deadline.”
There was a problem hiding this comment.
A tester called correction a black box: the user guide gave it one paragraph and the limits were only in the developer API guide. List what one string can survive and what it can't, how --bytes narrows the search, and the 13-fill case that the YES prompt guards. Each limit was checked by damaging BIP-93 test vectors and running the correction search one trial at a time, 3 to 4 trials per case. Closes #89 Claude-Session: https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa Claude-Session: https://claude.ai/code/session_01Q1TNm1YeYW8K9W2eBvxmCD
32f1a98 to
9424889
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Requested by Ben · project thread
Before: the user guide gave
correctone paragraph, and its limits were only in the developer API guide.After: a "What
correctcan repair" section in the user guide lists what one string survives, what it doesn't, when to add--bytes 32, and why filling exactly 13 unreadable characters needsYES.How: each limit was measured by damaging BIP-93 test vectors (48, 74 and 127 characters) and running the CLI's correction search, 2 to 4 trials per case. Two items from the issue's starting list didn't hold and are left out: 2 missing plus 2 extra worked in only half the trials within ten seconds, and 3 missing plus 1 extra and two wrong groups never did. Rebased onto
reviewability-v19b99862 and re-measured there on 2026-10-20: every listed limit still holds. Script and results:limits.pyin the project'scorrect-limitsfolder.Docs only. Closes #89
🤖 Generated with Claude Code
https://claude.ai/code/session_015CuLXqAvAovfoVcUmmogwa
https://claude.ai/code/session_01Q1TNm1YeYW8K9W2eBvxmCD