Repository navigation
cli: Remove unreachable recovery and search paths - #105
Conversation
This comment has been minimized.
This comment has been minimized.
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted release-gate re-review performed at the maintainer's request and disclosed per docs/developer/AI_POLICY.md.
ACK d886238.
This is the same two-commit cleanup previously reviewed as #98, replayed directly on refreshed #57. Stable patch-ids match #98 exactly. The changes only remove provably redundant correction-plan state and the already-type-checked share filter; correction scheduling, capture accounting, wallet behavior, and public input validation are unchanged.
Current-base verification: 101 focused CLI/correction tests passed, and the installed source is 5,179 logical lines under the authorized <5200 cap. No correctness blocker found.
d886238 to
52c23b3
Compare
BenWestgate
left a comment
There was a problem hiding this comment.
Exact-head release-gate re-review: ACK 52c23b3. The two-commit cleanup removes only redundant correction-plan state and a share filter after the all-Share type check; no correction scheduling, capture accounting, wallet behavior, or public validation changes. On the full refreshed restore stack through #95, 935 tests pass normally and under python -O; Ruff, format, strict mypy, correction constants, all 57 frozen differential cases, and the production-size test pass. No code blocker found. Both commits are agent-authored and still require responsible-human rewrite/squash before integration.
52c23b3 to
361feb7
Compare
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head re-review: ACK 361feb7.
The current diff is still the narrow behavior-preserving cleanup: _correction_plan() returns only the target lengths it actually uses, _search_many() drops an unread reduced parameter, and recovery passes the already-validated all-Share list directly. No scheduling, capture accounting, wallet behavior, or public validation changes are introduced. There are no inline review threads, and exact-head Python-package run 660 succeeded.
No code blocker found. The two agent-authored commits still require responsible-human rewrite/squash under repository policy before integration.
BenWestgate
left a comment
There was a problem hiding this comment.
Codex current-head review at 361feb7: no findings. This removes redundant correction-plan state and an unnecessary runtime share filter without changing the search envelope or validation boundary.
361feb7 to
03b0f8a
Compare
|
@codex review Current head was mechanically restacked onto the refreshed audit stack after #57's documentation-only follow-up. Please review this exact head for correctness/regressions; the composed stack passes 941 tests normally and under |
This comment has been minimized.
This comment has been minimized.
|
@codex review Please review current head |
This comment has been minimized.
This comment has been minimized.
Drop correction-plan state that no caller or search path reads, remove the corresponding unused `_search_many()` parameter, and pass the already-validated all-Share recovery list directly. This is behavior-preserving cleanup only: correction scheduling, capture accounting, wallet behavior, and public validation remain unchanged.
03b0f8a to
9ea48de
Compare
|
@codex review Please review the current head |
This comment has been minimized.
This comment has been minimized.
|
@codex review |
This comment has been minimized.
This comment has been minimized.
BenWestgate
left a comment
There was a problem hiding this comment.
AI-assisted current-head review: ACK 9ea48de. The removed correction-plan state was constant at every live call site (primary == all targets, reduced == empty, timed search enabled), so passing the targets directly preserves the search envelope. The recovery cast follows an explicit all(isinstance(..., Share)) guard and removes only the redundant second filter. Exact-head Python-package CI is green and there are no unresolved review threads. No correctness or release blocker found.
|
@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. |
|
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". |
Restore the focused cleanup review boundary after the historical #98 cleanup was absorbed into later stack work.
The PR is one squashed behavior-preserving cleanup commit:
_correction_planprimary/reduced/timed state and_search_many(reduced=...)argument;recover_secret()instead of filtering it a second time.No correction scheduling, capture accounting, wallet behavior, or public input validation changes.
Review shape
Current head
9ea48deis stacked directly on currentreviewability-v1after #57 was merged. The branch now records one squashed commit authored by BenWestgate; the earlier agent-authored replay is no longer the live head.The production-size gate remains the maintainer-authorized
<5200.Validation
Exact-head Python-package run 37479416800 completed successfully. The composed refreshed stack through #95 also passed all 941 tests normally and under
python -O, Ruff check/format, mypy, correction-constant verification, all 57 frozen differential correction cases, andgit diff --check.A fresh Codex review has been requested for
9ea48de; the prior ACK covered the mechanically equivalent pre-rewrite head and all inline threads are resolved.Human review/integration order is #105 → #99 → #80 → #81 → #95.
Refs #46 and #98.