Migrate langdetect to langid.py with lazy loading (#1208)#1899
Open
nethum529 wants to merge 1 commit into
Open
Migrate langdetect to langid.py with lazy loading (#1208)#1899nethum529 wants to merge 1 commit into
nethum529 wants to merge 1 commit into
Conversation
langid.py offers faster, more accurate detection across a wider range of languages than langdetect. Import it lazily inside is_meaning_string() so the model is only loaded when language detection is actually needed, rather than on every import of garak.langproviders.base. langdetect raised LangDetectException on inputs with no linguistic features (treated as not-meaningful); langid never raises, so that path is preserved with an explicit no-alphabetic-character guard, and the cheap length/repetition checks now short-circuit before any model load. Closes NVIDIA#1208 Signed-off-by: nethum529 <nethumweerasinghe.nw@gmail.com>
leondz
reviewed
Jul 6, 2026
leondz
left a comment
leondz
left a comment
Collaborator
There was a problem hiding this comment.
couple of minor changes, i like it in general
Comment on lines
103
to
104
| def is_meaning_string(text: str) -> bool: | ||
| """Check if the input text is a meaningless sequence or invalid for translation.""" |
Collaborator
There was a problem hiding this comment.
we should rename function and change this comment - it's too cryptic
suggest "check the input text is suitable for translation"
and def is_suitable_for_translation
| ) | ||
| def test_is_meaning_string_true_for_non_english(text): | ||
| # Non-English, translatable text should be flagged as meaningful | ||
| assert is_meaning_string(text) is True |
Collaborator
There was a problem hiding this comment.
good, i like that we have coverage here
Comment on lines
+52
to
+65
| def test_langproviders_base_uses_langid_lazily(): | ||
| # Migration contract for issue #1208: langdetect is gone, and langid is | ||
| # imported lazily inside is_meaning_string rather than at module load. | ||
| import inspect | ||
|
|
||
| from garak.langproviders import base | ||
|
|
||
| module_source = inspect.getsource(base) | ||
| assert "langdetect" not in module_source | ||
| module_top = module_source.split("\ndef ")[0] | ||
| assert "import langid" not in module_top | ||
| assert "import langid" in inspect.getsource(base.is_meaning_string) | ||
|
|
||
|
|
Collaborator
There was a problem hiding this comment.
this is a bit too in the weeds and locks us in to a specific implementation pattern, i think it should be dropped
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.
Migrates the language-detection dependency used by the language service from
langdetecttolangid.py, and loads it lazily. Closes #1208.langid.pyprovides faster and more accurate detection across a wider range of languages thanlangdetect, and importing it insideis_meaning_string()means the model is only loaded when language detection is actually exercised (i.e. reverse-translation judging) rather than on every import ofgarak.langproviders.base.What changed
garak/langproviders/base.pyfrom langdetect import ...(and the now-unusedimport logging).is_meaning_string()now uses a lazily-importedlangid.classify().langdetectpreviously raisedLangDetectExceptionon inputs with no linguistic features (e.g. pure digits/punctuation), which the old code treated as "not meaningful".langidnever raises, so that path is replicated with an explicit "no alphabetic character → not meaningful" guard. The cheap length/repetition checks now run first so trivial inputs short-circuit before any model load.pyproject.toml/requirements.txt: swaplangdetect==1.0.9→langid==1.1.6.tests/langservice/test_langprovision.py: add unit tests foris_meaning_string(the function previously had none), covering non-English (→ translatable), English, too-short, repetitive, and no-feature inputs, plus a structural test assertinglangdetectis gone andlangidis imported lazily.Verification
python -m pytest tests/langservice/test_langprovision.py -k "is_meaning_string or langid"→ 14 passedpython -m pytest tests/test_reqs.py→ passed (requirements.txt / pyproject.toml stay consistent)langdetectuninstalled (no other module imported it):python -m pytest --co→ 4664 tests, exit 0black --check garak/langproviders/base.py tests/langservice/test_langprovision.py→ cleanlangidis not imported at module load, only whenis_meaning_string()runs