fix(mx/rfc): do not enforce check digit, matching python-stdnum default behavior#163
fix(mx/rfc): do not enforce check digit, matching python-stdnum default behavior#163menyinch wants to merge 1 commit into
Conversation
The RFC check digit algorithm is not reliable for real, SAT-issued
numbers: python-stdnum estimates ~1.5% of RFCs in circulation have
check digits that do not match the published algorithm, so its
validate() skips check digit verification unless the caller opts in
via validate_check_digits=True.
The TypeScript port enforced the check digit unconditionally, so real
RFCs such as PAF070213GI1 (expected digit 'A' per the algorithm) and
the generic foreign-resident RFC XEXX010101000 were rejected with
InvalidChecksum, contradicting both the reference implementation and
this module's own JSDoc ('It does not check for control letter').
Remove the check digit enforcement (and the [1-9A-V][1-9A-Z][0-9A]
serial pattern check, which python-stdnum also only applies under the
opt-in flag), update the VACE-460910-SX6 expectation to match the
python-stdnum docstring, and re-enable the previously commented-out
XEXX010101000 test.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request modifies the Mexican RFC validation logic to stop enforcing check digit verification by default. This change addresses issues where legitimate, SAT-issued RFCs were being incorrectly rejected due to discrepancies between the published algorithm and real-world usage. The implementation now mirrors the behavior of python-stdnum, ensuring better compatibility with existing tax identifiers. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request removes the checksum validation logic from the Mexican Tax Number (RFC) validator, allowing RFCs that do not conform to the check digit algorithm to pass validation. The test suite has been updated to enable previously disabled tests and add a new test case for SAT-issued company RFCs. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Problem
validate()rejects real, SAT-issued RFCs withInvalidChecksum. Example: PAF070213GI1 (a company RFC in active use) — the published check digit algorithm computesAfor it, but SAT issued it with1.python-stdnum accepts this number. Its
mx.rfcmodule deliberately skips check digit verification by default because the algorithm does not hold for a significant share of real numbers:python-stdnum only checks the digit when the caller opts in via
validate(number, validate_check_digits=True). Its own docstring exampleVACE-460910-SX6validates by default and only fails under the opt-in flag.The TypeScript port enforces the check digit unconditionally, which also contradicts this module's own documentation:
validate()JSDoc says "It does not check for control letter".Change
validate()(including the legacy-alphabet fallback added in [Fix] Update RFC validation to include legacy alphabet #143, which was working around the same underlying issue but cannot cover numbers like this one), and the[1-9A-V][1-9A-Z][0-9A]serial pattern check that python-stdnum also applies only under the opt-in flag.VACE-460910-SX6expectation to valid, matching the python-stdnum docstring.XEXX010101000test (the official generic RFC for foreign residents) — it now passes.PAF-070213-GI1.Length, format/regex, blacklist, and date validation are unchanged.
Verification
PAF070213GI1,VACE460910SX6, andXEXX010101000all cross-checked against python-stdnum'sstdnum.mx.rfc.validate(valid by default;PAF.../VACE...fail only withvalidate_check_digits=True, matching the removed behavior).mainand pass with this fix; full suite: 1613 passed, 1613 total. Prettier + ESLint clean.If strict check digit validation is still wanted as an opt-in (like python-stdnum's flag), I'm happy to add it in a follow-up — but the default should not reject genuine RFCs.