Repository navigation
correction: Restore two-group erasure priority - #165
BenWestgate wants to merge 1 commit into
Conversation
The two corrupted-group search class already enumerates candidate masks, but competitor scheduling can exhaust its ten-second deadline before visiting the cheap eight-erasure BCH checks. Put these probes alongside single-operation candidates so damaged cards with two wrong full groups can produce a tentative correction within the existing bounded search. Leave admission accounting, false-reconstruction mass, correction confirmation, and the disclosure gate unchanged. Add a regression that fails under the previous scheduling order and update the security model to document the new priority. Refs #102
BenWestgate
left a comment
BenWestgate
left a comment
There was a problem hiding this comment.
Focused security review of exact head 6cfd6799e31aa11d258a81c0fae726468d4fb541: security ACK; no reportable findings.
Reviewed all three changed files and followed scheduler behavior through fixed BCH, capture-volume accounting, competing-candidate ranking, deadlines, and the CLI confirmation/disclosure paths. The new early tier covers a pre-existing admitted class (two damaged four-character groups / eight erasures). It changes scheduling only: no new hypothesis, capture-mass budget, scoring rule, deadline, or output gate. Fixed BCH is still attempted first. Equal-or-better competitors remain eligible; expiration preserves search_complete=False and suppresses incomplete primary ties. Weak-discrimination checks still use the full admitted mass, and an operator must confirm the entire card before accepting a suggestion.
Validation: 163 focused correction/competitor/disclosure tests passed; the head's Python-package CI is green. Synthetic 16/32/64-byte timing probes recovered two corrupted groups with the new priority where the old order exhausted a short deadline, while inexpensive common one/two-character corrections remained fast. A short timeout can yield a best-so-far candidate with incomplete-search metadata; that pre-existing conditional CLI behavior must not be represented as a uniqueness proof.
Review is limited to this patch. Keep the final integrated v1 release-qualification gates, including real-Core and GUI/Tails checks.
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 |
|
Codex Review: Didn't find any major issues. Swish! 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". |
#102 reported the missing practical ability to correct two separate corrupted four-character groups by treating all eight symbols as erasures (#102 (comment)).
Problem: The two-group erasure class remains implemented and covered by the conservative capture-mass ledger, but CLI competitor scheduling puts its inexpensive BCH probes behind expensive structural hypotheses. In a synthetic 48-character example with two nonadjacent groups damaged, the CLI returned no correction in ten seconds even though the library could reconstruct it.
Fix: Move existing two-group erasure probes alongside single-operation searches in the bounded scheduler. No new correction class or expanded admission budget is introduced. A new behavioral regression confirms the correct candidate is found within three seconds; it fails against the previous schedule. Document the priority in the security model.
Safety: Parser validation, BCH checksum verification, false-reconstruction bound, ten-second search deadline, disclosure/confirmation gates and output status remain unchanged. A correction remains an untrusted proposal requiring operator verification.
Checks: Normal suite: 907 passed; the only failure was the installed-entry-point test due to the disposable checkout lacking a console script. That test passed after installation. Optimized-mode focused correction, competitor and disclosure tests: 163 passed. The frozen differential correction verifier passed all 57 cases; mypy checked 21 source files with no issues; full-project Ruff lint and formatting checks passed. Exact modified blobs were verified against the local tested checkout.
Draft pending focused security review (change affects correction scheduling). Intended as independent follow-up to #155 → #157 → #161; integrate and retest on the final v1 candidate.
Refs #102.