Repository navigation
fix: quiet-zone, validation and allocation defects; add agent docs - #1
Merged
Merged
Conversation
Seven confirmed defects, none of which the test suite caught — every check (tests, clippy, fmt, every feature combination) passed before this change. Each fix comes with the test that would have caught it. - QR human-readable text was drawn one module below the symbol, inside the four-module quiet zone ISO/IEC 18004 requires: 708 pixels of ink in the band a scanner uses to find the symbol. Correct for Code 128, which has no vertical quiet zone; wrong for anything matrix. Linear output is unchanged. - MAX_ENTROPY_BITS and the ID length limit had been chosen independently, so a 512-bit policy minted IDs that TrackingId::parse then rejected. The limit is now derived from the widest policy, with a static assertion against drift. - TrackingId derived serde's Deserialize with #[serde(transparent)], so any string bypassed parse entirely — "no-separator-here" was accepted and then reported body() == "separator", silently dropping the rest. - BitMatrix::row panicked on an out-of-range row while get and set tolerated one, in a crate that promises not to panic on bad input. - The image size limit was per-axis, so 18500x18500 passed validation and asked the rasteriser for 1.27 GiB in one allocation. - IdGenerator::validate accepted non-hexadecimal bodies the generator could never have produced; a multi-byte prefix was reported as too long rather than as containing disallowed characters. - The Code 128 bar blitter clamped only the far end of its slice. Also adds AGENTS.md (repo map, invariants, and the four shapes these bugs kept taking), a README table mapping every Error variant to cause and fix, and excludes editor and agent tooling from the published crate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M0MJP4PJ9S5FY5QN2X6RD5J3
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThe change adds tracking ID validation, serde parsing, rendering limits, QR quiet-zone placement, safe matrix access, Code 128 bounds handling, regression tests, repository guidance, and package exclusions. ChangesValidation, Rendering, and Repository Safeguards
Estimated code review effort: 3 (Moderate) | ~25 minutes ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Seven confirmed defects, each with the test that catches it. None were caught by
the existing suite: tests, clippy
-D warnings,cargo fmt --checkand everyfeature combination all passed before this change. These are coverage gaps, not
regressions.
Fixes
MAX_ENTROPY_BITS(512 → a 128-character body) and the format's length limit (128) were chosen independently, so a 512-bit policy minted IDs thatTrackingId::parsethen rejected. The limit is now derived from the widest policy, withconst _: () = assert!(…)so they cannot drift again. IDs that parsed before still parse.#[serde(transparent)]+ derivedDeserializelet any string become aTrackingId."no-separator-here"was accepted and then reportedbody() == "separator", silently dropping the rest. Deserialisation now runsparse. Serialisation unchanged.BitMatrix::rowget/settolerated one, in a crate that promises no panics on bad input. Returns&[]now;from_row's pre-existing panic is documented.IdGenerator::validatePKG-ZZZZZZZZpassed under the default policy though the generator only emits0-9A-F. Entropy characters are now checked; the check character still uses the full 36-character alphabet. A multi-byte prefix is reported as a charset error rather than as too long.code128emits one — this is against a future version bump.Also
AGENTS.md— repo map, the invariants a change must not break, where to add asymbology or renderer, and the four shapes every bug in this crate has taken
so far.
CLAUDE.mdpoints at it.Errorvariant to cause and fix..claude/,.entire/,CLAUDE.md,hello-entire.md) excluded from the published crate — publishing isirreversible, so the tarball should carry only what a consumer needs.
Test plan
cargo test --all-features— 131 pass, up from 116cargo clippy --all-features --all-targets -- -D warnings— cleancargo fmt --all --check— cleancargo build --target thumbv7em-none-eabihf --no-default-features --features code128,svgcargo run --example labelround-tripsreverting the change and watching it go red, then restoring it. The rendered
QR label was inspected visually: clear margin on all four sides, text below.
Reviewer notes
gets an error instead of a silently-wrong value, and
entropy_bits(512)isaffected by the length rework. This is 0.3.0-shaped.
Cargo.tomlis left at0.2.0 and everything is under
[Unreleased]— the version bump and tag aredeliberately not part of this PR, since tagging publishes irreversibly.
KNOWN-ISSUES.mdwas added and removed within this work: it existed to trackthese defects, and the detail now lives in
CHANGELOG.md.🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation