Conversation
wbarnha
marked this pull request as ready for review
July 22, 2026 12:57
Without uchardet's generic language pass, two ranking safeguards were lost, both now restored inside the overlay: - nsUTF8Prober never rejects invalid byte sequences and its confidence floor (0.5) always clears CANDIDATE_THRESHOLD, so non-UTF-8 multibyte input (GBK, EUC-KR, EUC-TW, ...) was frequently mislabeled as UTF-8. The overlay now runs a strict incremental UTF-8 validity DFA over the raw stream and demotes an invalid-UTF-8 candidate to a fixed 0.25 last-resort confidence: above nsUniversalDetector's MINIMUM_THRESHOLD (0.20) so near-ASCII data no other prober claims still gets the same weak UTF-8 guess upstream produces, below CANDIDATE_THRESHOLD (0.30) so it can never outrank a real candidate. The DFA state lives in the otherwise-unused langDetectors[0][0] slot because the upstream header is not overlaid and the class layout must stay unchanged. - Upstream multiplies multibyte prober confidences by language-model confidence before candidate selection, which suppressed weak false positives such as Big5 claiming a near-ASCII Windows-1252 file at conf ~0.4 (issue #33). Non-UTF-8 multibyte probers now need to clear a 0.5 threshold on their own: genuine detections measure >= ~0.7 while such false positives stay around 0.4. Verified: full pytest suite passes (127 passed, 1 skipped, including the previously failing zh/euc-tw sample and the issue #33 guard); synthetic multi-encoding benchmark shows GBK/EUC-KR/EUC-TW mislabels eliminated with unchanged throughput; ASan/UBSan run over the overlay is clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xkaxun6yxtcCTsdH1GEDCM
The issue #57 CC-News corpus is almost entirely valid UTF-8, so cChardet's UTF-8 fast path answers before the detection engine ever runs. That left the exact code the encoding-only overlay changes -- and the freedesktop uchardet engine that regressed in work item #38 -- unexercised by CI, which is why the earlier overlay's UTF-8 mislabeling of GBK/EUC-KR text was not caught. Add a deterministic, network-free non-UTF-8 corpus and wire it into both the test suite and the benchmark job: - benchmarks/make_nonutf8_corpus.py generates labeled single- and multi-byte non-UTF-8 documents (ISO-8859-15, Windows-1251, Shift_JIS, EUC-JP, GBK, Big5, EUC-KR) from short committed multilingual snippets. - benchmark.py scores detection by decode-equivalence when the corpus is labeled, and counts UTF-8 mislabels (non-UTF-8 bytes reported as UTF-8 -- the dangerous case that makes a downstream open(encoding=...) mojibake). - compare.py renders the accuracy column and gains --max-utf8-mislabel-rate; the plain-list CC-News path is unchanged. - benchmark.yml runs the three builds over the non-UTF-8 corpus and gates on a UTF-8 mislabel rate of 0.02 plus a lenient 0.50 throughput floor. The freedesktop engine is inherently slower than the old PyYoshi engine on non-UTF-8, so the mislabel rate -- not raw throughput -- is the signal that uniquely catches the overlay regression, while the throughput floor still trips if the language-model slowdown is reintroduced. - src/tests/test_nonutf8_detection.py asserts, on every PR, that no multibyte document is mislabeled as UTF-8, that multibyte accuracy stays >= 99%, and that the issue #33 Big5 false positive stays suppressed. Verified against the pre-fix overlay: the new test and the benchmark gate both fail on it (multibyte UTF-8 mislabels, 18.7% overall mislabel rate) and both pass on the current overlay (0 mislabels). Full suite: 130 passed, 1 skipped.
The first CI run measured the current build at 0.51x of v2.2.1 on the non-UTF-8 corpus, right at the 0.50 floor -- flaky. That corpus of many small documents does not reproduce uchardet #38's large-stream language-model blow-up (the per-call overhead dominates, so the language pass costs only ~1.6x here, not orders of magnitude), so the throughput ratio cannot robustly separate a good build from a regressed one. Drop the floor to 0.35 as a loose backstop for a catastrophic slowdown; the UTF-8 mislabel rate remains the primary, crisp regression signal.
This was referenced Jul 23, 2026
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.
Summary
Adds a GitHub Actions benchmark workflow for cChardet that:
benchmark-action/github-action-benchmarkas a complementary continuous benchmark history checkNotes
The
github-action-benchmarkintegration records current-branch throughput in MB/s and restores/saves benchmark history through GitHub Actions cache. It writes results into the Actions job summary without requiring agh-pagesbranch or extra repository write permissions on PRs.Validation
Full issue-corpus benchmarking was left to CI because the local Python 3.12 environment exposed an existing native allocator crash; the workflow runs on Python 3.11.