Replace the manual Canon AFInfo2 reader with generated tables - #760
Conversation
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2630ded84a
ℹ️ 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".
| if edge_processor is None or serial_tables is None or not _u16_membership_helper(validation_helper): | ||
| return False |
There was a problem hiding this comment.
Allow validator-free serial edges through verification
When an IFD SubDirectory targets an authenticated ProcessSerialData table but declares no Validate, codegen.py still emits processor: Serial with validation: None. Here validation_helper is necessarily None, so this unconditional helper check rejects the route and expected_ifd_edge instead expects Native/unwalked, causing the official verifier to reject its own freshly generated artifact. Require _u16_membership_helper only when the source edge actually has Validate.
Useful? React with 👍 / 👎.
Canon AFInfo2/AFInfo3 metadata was decoded through a separate manual field sequence and mode map. This replaces that reader with the generated Canon::AFInfo2 table through the shared IFD/serial path, deleting the duplicate arm, eight private offsets, two parent IDs and the 20-value mode enum.
Effective processors and validation operands come from authenticated pinned ExifTool source. Generated ownership does not revive the retired manual decoder on refusal; the reader discards speculative child values/state while preserving earlier parent effects. Old AFInfo geometry and four CanonRaw parent omissions remain explicit separate work. The plan now records shared read/write definitions, separate writer acceptance, and reproducible random-release rehearsals that verify each version against its own native ExifTool; this change activates reading only.
Validation against ExifTool 13.59 and Perl 5.38.2:
Production source and Cargo files retain the accepted 3d0353f behavior; the only later Rust edit is within the legacy Canon test module. Its zero-size fixture now asserts native rejection and its positive case uses the correct size. The full local Cargo unit/integration/doc run passes with 5,993 passes, zero failures and 124 ignored tests in 150.641 seconds. The first hosted test failure and the unavailable local nextest executable remain recorded; hosted checks must pass on this repaired head. Independent whole-source review found no remaining blocker. Native warnings, broader occurrence/group fidelity and generated writing are not claimed by the bounded checks. Failed intermediate attempts and exact evidence identities are recorded in docs/reference/afinfo2-production-plan.md.
All five required hosted checks passed on
2630ded8; complete job logs are preserved with the checkpoint evidence.