Detect encoding damage in labels and synonyms - #953
Open
gaurav wants to merge 8 commits into
Open
Conversation
Babel has no encoding validation anywhere, while three ingest points read their source with a single-byte codec: datacollect.py reads PubChem as latin-1, and unii.py reads UNII as windows-1252. When the source bytes are really UTF-8, that misread produces mojibake -- 'é' becomes 'é' -- and nothing downstream notices, because mojibake is valid UTF-8, just wrong. find_encoding_issue() detects it with a single-byte round-trip: if the text re-encodes to the codec that damaged it and those bytes decode as UTF-8 into something different, it was misread. Legitimate non-ASCII fails one of those two steps, so 'Ménière disease' and 'Nα-acetyltransferase' stay clean. Both cp1252 and latin-1 are tried, since they damage bytes 0x80-0x9f differently and only the matching one can name the original text. An isascii() gate keeps it affordable across a full build: nearly every biomedical label is pure ASCII and exits in nanoseconds. Tab is deliberately not treated as damage -- load_extra_labels() splits with maxsplit=1 so a label containing one survives. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Reports encoding damage instead of raising, so a build's downloads and outputs can be surveyed before the raising check is relied on -- and so a build that fails it can be triaged without re-running anything. Scans labels, synonyms, and compendium/synonym JSONL, picking the line shape from the content rather than the filename (Babel's TSVs have no extension). Exits 1 when anything is found so a script can gate on it. Thin CLI per the src/tools convention: all the logic is in src/synonyms/encoding.py, shared with the pipeline's check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Wires check_encoding() into the five places in src/node.py where labels and synonyms are read: the per-prefix labels and synonyms files, the two common/ files, and the explicit labels dict passed to write_compendium(). This is the load side rather than the write side because there is no single place labels are written -- twenty-odd datahandlers each write their own babel_downloads/<PREFIX>/labels. There is exactly one place they are all read, which is also where SynonymFilter hooks in, so each string is checked once per prefix load and the error names the file the damage lives in. Raising rather than warning is deliberate: mojibake is valid UTF-8, so a warning would scroll past and the bad name would ship to Node Normalization. Re-running one download rule is far cheaper than finding it in a release. config.yaml gets encoding_check_enabled and encoding_check_allowlist so a false positive can be worked around without a code change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
check_for_encoding_issues() scans Node and Synonyms Parquet for damaged labels and preferred names, writing reports/duckdb/encoding_issues.tsv. Still worth having with the raising check upstream: it covers builds made before that check existed, and anything reaching a compendium through the properties path rather than a labels file. DuckDB can't do the round-trip, so this is the pattern half of the detector -- Ã/Â-lead mojibake, doubly damaged U+FFFD, replacement characters and C0 controls. A streaming filter with no aggregate, so it needs none of the two-pass treatment its duplicate-detection siblings do. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
docs/Development.md covers what the check does, why it raises rather than warns, why it sits on the load side, and what to do when it fires. docs/sources/CLAUDE.md gets the rule the check exists to enforce: read source files as UTF-8, and reach for a single-byte codec only with evidence, since doing so converts a loud UnicodeDecodeError into silent mojibake. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An agent whose build dies with "RuntimeError: Encoding issue in ..." needs to find the allowlist and the survey tool; the subsystem-gotchas list is where they will look. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds wiring tests for the three label/synonym paths that had none: the per-prefix labels file via NodeFactory.load_extra_labels(), the explicit labels dict passed to write_compendium(), and the common/ labels and synonyms files. Each raising case is paired with a legitimate-non-ASCII case. A false positive is the expensive failure here -- the check aborts the build, so 'Ménière disease' loading untouched matters as much as 'étude' being caught. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
It inherited the locale default, so the same compendium could decode differently -- or fail -- depending on the machine the tool ran on. Compendia are written as UTF-8; read them that way. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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.
Context
Babel has no encoding validation anywhere — no
unicodedata, no mojibake detection, nonormalization. Meanwhile three ingest points read their source with a single-byte codec:
src/datahandlers/datacollect.py— PubChemCID-Title.gz/CID-Synonym-filtered.gzaslatin-1src/datahandlers/unii.py—UNII_RECORDS_ENCODING = "windows-1252", plus a second file aslatin-1Reading UTF-8 bytes that way produces mojibake (
é→é), and nothing downstream notices, becausemojibake is valid UTF-8 — just wrong text. It flows through the compendia and the synonyms files
and lands in Node Normalization as if it were a real name. The one place Python's codec is touched
on the way out (
duckdb_exporters.py) raises on invalid UTF-8, which this isn't.What this adds
src/synonyms/encoding.pysrc/node.pybabel-check-encodingcheck_for_encoding_issuesinduckdb_reports.pyreports/duckdb/encoding_issues.tsvThe detector
Non-ASCII text is re-encoded to the codec that would have damaged it and decoded as UTF-8. If that
round-trip succeeds and changes the string, it was mojibake — and the repair is reported, which is
what makes the error actionable (
éalone is a puzzle; "probably meant to be'é'" is adiagnosis). Legitimate non-ASCII fails one of the two steps, so
Ménière diseaseandNα-acetyl-L-lysinestay clean. Secondary signals: C0/C1 controls,U+FFFD, a stray BOM.Both
cp1252andlatin-1are tried. They damage bytes0x80-0x9fdifferently, and only thematching one can name the original:
An
isascii()gate keeps this affordable at full-build scale — nearly every biomedical label ispure ASCII and exits in nanoseconds, so only the rare non-ASCII string pays for the round-trip.
Why the load side, and why it raises
There is no single write choke point: twenty-odd datahandlers each write their own
babel_downloads/<PREFIX>/labelswith a barewrite(f"{curie}\t{label}\n"). There is a singleread choke point — the same one
SynonymFilteralready hooks into. Checking there means each stringis validated once per prefix load, and the error names the file the damage lives in.
Raising rather than warning was a deliberate call: a warning scrolls past in a build log, and the
bad name ships. Re-running one download rule is far cheaper than finding it in a release.
encoding_check_enabledandencoding_check_allowlistinconfig.yamlare the escape hatches, soa false positive doesn't need a code change to work around.
No separate write-side pass in
write_compendium()— everything it writes already arrived throughone of the five load sites, so it would re-check the same strings and report nothing new.
The check is armed but unvalidated against real data. There's no local
babel_downloads, andpulling a snapshot is a large download I didn't want to start unprompted. A full build could halt on
PubChem or UNII. Please run one of these first:
against a
stars.renci.orgsnapshot — or setencoding_check_enabled: falsefor the first buildand read
reports/duckdb/encoding_issues.tsvinstead.If that shows real damage, fixing the latin-1 reads is a follow-up PR, not this one: changing
PubChem's decode alters millions of records, and
AGENTS.mdis clear that a parsing change needsthe whole file characterized first. This PR produces exactly that evidence.
Also out of scope: NFC/NFKC normalization. That's a deduplication concern, not a corruption one.
Testing
385 unit tests pass;
ruff,snakefmt, andrumdlare clean;check_for_encoding_issuesresolvesin a
snakemake --dry-run. New coverage intests/synonyms/test_encoding.py,tests/tools/check_encoding/,tests/node/test_synonym_factory.py,tests/reports/test_duckdb_reports.py— weighted toward the false-positive cases, since those are what would halt a build.
One bug worth surfacing: I first treated an embedded tab as damage.
NodeFactory.load_extra_labels()splits with
maxsplit=1specifically to preserve tab-containing labels, andtest_load_extra_labels_tab_in_labelpins that. The full suite caught it; tab is now explicitlyexcluded, with a comment pointing at the loader.
🤖 Generated with Claude Code