feat(testdata): real-world corpus + ES CIF letter-check fix - #4
Merged
Conversation
The `C,D,F,G,J,L,M,N,U,V` letter group is documented by the Spanish tax authority as accepting either a digit or letter check-digit. The existing code rejected the letter form outright, so real-world CIFs like ArcelorMittal Spain `N0181056C` (foreign entity, letter-check) failed validation. Fix: the default arm of `cifCheck` / `esCIF` now accepts either form. ABEH stays digit-only and PQRSW stays letter-only, matching the AEAT tables. Regression tests added under both providers.
Add a hand-collected corpus of ~1300 real business identifiers (SIREN,
SIRET, VAT, national register numbers for BE/DE/ES/FR/IT/NL) sourced
from official registries, Wikipedia infoboxes and corporate legal
notices. The corpus lives under testdata/ and is consumed by four new
real-world tests (one per provider) plus a native-algorithm test under
providers/euid/. This gives the library a broader real-world safety
net than the current ~10 known-values-per-country coverage.
Layout:
testdata/
registers/<cc>-valide.txt raw national register (source of truth)
euid/<cc>-valide.txt derived from registers, "<CC><REG>." prefix
vat/<cc>-valide.txt hand-collected VAT numbers (~25 per country)
siret/fr-valide.txt FR SIRETs (source of truth for FR)
siren/fr-valide.txt derived from SIRETs (siret[:9], dedupped)
MANIFEST.md sources, counts, gaps
The derived files (euid/, siren/fr, registers/fr) are regenerated by
`go run ./hack/derive-corpus`, itself packaged as a separate module
under a Go workspace so its dev-tool dependencies stay isolated from
the library.
Countries covered (initial pass): BE (42), DE (100), ES (26), FR (113
SIRETs), IT (100), NL (81) for registers; ~25 VAT each. Remaining 21
EU countries: to be added later.
Data-quality notes:
- Numbers are collected first, tests run second, and failing entries
are triaged one by one: real lib bug → fix; wrong transcription →
remove. That process surfaced the ES CIF letter-check fix (separate
commit) and revealed a batch of fabricated Wikipedia-sourced NL VAT
numbers, replaced by btw-zoeken.nl-verified values.
Workspace / tooling:
- go.work + hack/derive-corpus/go.mod isolate the CLI.
- CI's go-version-file switched from go.mod to go.work.
- Makefile's `generate` target now loops over both modules like the
other targets.
Tests: 2027 passing, 100 % statement coverage kept across all provider
packages.
Coverage Report for CI Build 28661634249Coverage decreased (-5.8%) to 94.171%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
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.
Summary
testdata/, consumed by four new real-world tests (one per provider) plus a native-algorithm test underproviders/euid/.derive-corpusCLI (regenerates the derivedeuid/*,siren/fr,registers/frfiles from source lists) as an isolated module under a new Go workspace (go.work+hack/derive-corpus/go.mod).C,D,F,G,J,L,M,N,U,V) now accepts both digit- and letter-check forms. Foreign entities like ArcelorMittal SpainN0181056Cpreviously failed.Testdata layout
Countries covered (initial pass): BE (42), DE (100), ES (26), FR (113 SIRETs → 113 SIRENs), IT (100), NL (81) for registers; ~25 VAT each. Remaining 21 EU countries: to be added in a later PR.
Data-quality process
Per the request: collect numbers from reliable sources first, run the tests second, triage failures one by one. That surfaced two things:
Nprefix (foreign entities) rejected legitimate letter-check CIFs — the "either" group incifCheck/esCIFwas hard-coded to digit-only. Fixed to accept both forms, matching the Spanish tax authority docs. Regression tests added invat_test.goandeuid_test.go.Workspace / tooling
go.work+hack/derive-corpus/go.modisolate the CLI so its future deps don't leak into the library module.go-version-fileswitched fromgo.modtogo.work.Makefile'sgeneratetarget now loops over both modules likebuild/test/tidy.hack/derive-corpusfully lint-clean (wrapcheck-wrapped errors,0o600file perms, log-based CLI output).Test plan
make test— 2027 tests, 100 % statement coverage across every provider package.make lint— 0 issues.go run ./hack/derive-corpusidempotent (no diff on re-run).go run ./hack/derive-corpusworks both from repo root and from insidehack/derive-corpus/.Commits
fix(vat,euid): accept letter check for ES CIF "either" group— isolated lib bugfix + regression tests.feat(testdata): real-world business-id corpus + integration tests— corpus, tests, workspace, CLI.