Fix modulus 97 exact multiple - #44
Merged
Merged
Conversation
The reduction used `97 - (sum % 97)`, which returns 97 when the sum is an exact multiple of 97. The reference algorithm stops subtracting at zero, so the expected check digits in that case are 00. The consequence was that no VAT number with check digits 00 could ever validate, while its "...97" counterpart was wrongly accepted. Both branches were affected: the modulus 97 branch when the weighted sum was a multiple of 97, and the 9755 branch when the weighted sum was congruent to 42 mod 97 (i.e. once the 55 offset is added). For example GB200394900 and GB200006400 are valid but were rejected; GB200394997 and GB200006497 are invalid but were accepted. Adding an outer modulo fixes both. `abs()` is now dead, since the result is always in 0..96. Verified exhaustively over the checksum's whole input domain (weighted sums 0..315 by check digits 00..99, both branches) and end to end against a transcription of the reference implementation over 300,000 random numbers.
Refresh uv.lock to current releases, notably mypy 1.20 -> 2.3, pytest 9.0 -> 9.1, ruff 0.15 -> 0.16, coverage 7.13 -> 7.15 and ipython 9.13 -> 9.16. The dev group floors move up to match what is actually tested, since mypy>=1 and pytest>=8 no longer described the versions in the lock. ruff 0.16 formats Python inside Markdown, which the README example did not satisfy, so reflow it. The "#>" output marker becomes a plain "#" comment, which the formatter leaves alone. Bump actions/checkout v4 -> v7 and astral-sh/setup-uv v5 -> v9.0.0. setup-uv publishes no floating v9 tag, so that one is pinned exactly. Drop the python-version-file input from the release workflow: setup-uv has never accepted it, so it was being silently ignored, and uv reads .python-version by itself. Use uv sync --locked in both workflows so a stale lockfile fails CI rather than being silently re-resolved. Python support is unchanged at 3.11-3.14. 3.15 is still at 3.15.0a8 and cannot be added yet: both mypy's and coverage's compiled extensions abort with "SystemError: unknown slot ID 85" on it. Verified by running the workflow commands (uv sync --locked --dev, uv run ./run-tests.sh) against 3.11, 3.12, 3.13 and 3.14.
get_digits_from_string filtered with str.isdigit(), which is true for the
whole Unicode digit property, and then converted with int(), which only
accepts decimal digits. Characters such as the superscript "²" and the
circled "①" therefore passed the filter and raised
ValueError: invalid literal for int() with base 10: '²'
out of both get_digits_from_string and validate_vat_number, where every
other unexpected character is simply ignored.
Filter with str.isdecimal() instead, which is true for exactly the category
Nd characters that int() will convert. Superscripts, circled digits and
vulgar fractions ("½") are now stripped as junk like emoji and punctuation
already were, so "GB²83092723①1" validates rather than raising, and "GB²²²"
is merely too short. Decimal digits outside ASCII, such as the Arabic-Indic
"٣", keep their existing behaviour of converting, since int() handles them.
janrito
approved these changes
Aug 7, 2026
Collaborator
Author
|
thank you @janrito |
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
2 participants
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.
This PR fixes a bug and updates dev dependencies and CI