Repository navigation
cleanup: Remove dead and unused code - #179
BenWestgate wants to merge 3 commits into
Conversation
|
@codex review |
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: e02e03feb9
ℹ️ 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 static review, 5fd47a6: Concept ACK. The rpartition guard now rejects 10, 12, and 13cash without changing backup policy; regression tests cover these cases. Creation still requires a TTY. The pre-v1 compatibility alias removal is an explicit maintainer decision. CI is queued; no code ACK or runtime claim yet.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! 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". |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This should receive a fingerprint if the user knows it. Maybe not in the target branch but eventually we DO ask for the fingerprint before the hex or existing codex32 secret to improve error detection. So I wouldn't remove it just to add it back, if that is the same thing I am talking about here. |
Drop code that never runs or only aliases other code: - create: Bitcoin Core is always connected and the terminal is always interactive, so the `core is None` and non-TTY branches are dead. The header is always ms, and `_generated_secret` only wrapped `generate_master_seed`. - create --existing: the correction preview never received a fingerprint provider, so drop the parameter. - correction: `_FixedCorrector` always set `erasure_products` to None, so the two-erasure product-table path never ran. - tools: nothing ran the two dependency-free fuzz targets. - errors: drop the pre-1.0 `MismatchedProfile` alias. - bip93: drop the one-line `_validate_basis_prefix` wrapper. No behaviour change. Claude-Session: https://claude.ai/code/session_01XGMpowzJwwgyHMEd63ijDe
A literal 1 separates the application prefix from a backup header. Treating 12 or 10 as standalone thresholds silently changes the creation policy. Preserve bare threshold and ms1 header support.
5fd47a6 to
16d6d0a
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted static review. ACK 16d6d0a. Invalid 10, 12, and 13cash headers are rejected with regression coverage; the intentionally removed pre-v1 alias is documented in review. Both earlier threads are resolved. Current-head CI is queued.
Remove unreachable creation branches, unused correction state and dormant tools. Preserve behavior.
Extracted from #146; based on #178. 148 generation and BCH tests pass.
Peer-review follow-up
The
10,12, and13cashcreation headers now fail rather than silently changing the backup policy; regression tests added (5fd47a6). The pre-v1 compatibility alias removal is intentional. Exact-head CI is pending.