Skip to content

fix(mx/rfc): do not enforce check digit, matching python-stdnum default behavior#163

Open
menyinch wants to merge 1 commit into
koblas:mainfrom
menyinch:fix/mx-rfc-check-digit
Open

fix(mx/rfc): do not enforce check digit, matching python-stdnum default behavior#163
menyinch wants to merge 1 commit into
koblas:mainfrom
menyinch:fix/mx-rfc-check-digit

Conversation

@menyinch

Copy link
Copy Markdown

Problem

validate() rejects real, SAT-issued RFCs with InvalidChecksum. Example: PAF070213GI1 (a company RFC in active use) — the published check digit algorithm computes A for it, but SAT issued it with 1.

python-stdnum accepts this number. Its mx.rfc module deliberately skips check digit verification by default because the algorithm does not hold for a significant share of real numbers:

However, it seems a lot of numbers (estimated at around 1.5% of all numbers) are in use with invalid check digits so this test is disabled by default.

python-stdnum only checks the digit when the caller opts in via validate(number, validate_check_digits=True). Its own docstring example VACE-460910-SX6 validates 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:

  • the file header carries the same "disabled by default" note quoted above, and
  • the validate() JSDoc says "It does not check for control letter".

Change

  • Remove the unconditional check digit verification from 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.
  • Update the VACE-460910-SX6 expectation to valid, matching the python-stdnum docstring.
  • Re-enable the previously commented-out XEXX010101000 test (the official generic RFC for foreign residents) — it now passes.
  • Add a regression test for PAF-070213-GI1.

Length, format/regex, blacklist, and date validation are unchanged.

Verification

  • PAF070213GI1, VACE460910SX6, and XEXX010101000 all cross-checked against python-stdnum's stdnum.mx.rfc.validate (valid by default; PAF.../VACE... fail only with validate_check_digits=True, matching the removed behavior).
  • The three test changes fail on main and 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.

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.
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Summary of Changes

Hello, 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

  • Check Digit Validation: Removed the unconditional check digit verification in the RFC validator to align with python-stdnum behavior, as many valid RFCs in the wild do not follow the official algorithm.
  • Test Suite Updates: Updated test expectations for existing RFCs and re-enabled the generic foreign resident RFC test case, while adding a new regression test for a previously failing company RFC.
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 Assist

The 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 /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.

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

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant