Fix _is_nonsense_url dropping trailing-slash language roots (/en/)#2035
Conversation
The "very short path" filter measured length on the slash-stripped path
(`len(path.strip('/')) < 3`) but matched the *un-stripped* path against the
language-root whitelist (`path not in ['/', '/en', ...]`). So the canonical
trailing-slash form of a language root — `/en/`, `/de/`, `/fr/`, `/es/`,
`/it/`, which is what sitemaps and urljoin normalization commonly emit — was
filtered out as nonsense, even though `/en` was correctly kept. With
filter_nonsense_urls on (the default), these valid localized landing pages
were silently dropped from seeding results.
Compare the slash-stripped path against the whitelist as well, so a language
root is kept regardless of a trailing slash; genuinely short junk paths
(`/ab/`, `/x`) are still filtered.
Adds tests/unit/test_nonsense_url_language_roots.py; the trailing-slash cases
fail on the old code and pass with this fix.
chuenchen309
left a comment
There was a problem hiding this comment.
Good, precise catch — the stripped-length-vs-unstripped-membership mismatch is a real inconsistency, and the canonical /en/ form of a language root is exactly the kind of URL a seeder shouldn't be discarding. I reproduced the rule on both sides to confirm the fix is complete and regression-free:
| path | before | after |
|---|---|---|
/en |
kept | kept |
/en/ /de/ /fr/ /es/ /it/ |
filtered | kept ✅ |
/ |
kept | kept |
/about, /robots.txt |
kept | kept |
/ab/, /x |
filtered | filtered (still junk ✅) |
The new whitelist ['', 'en', 'de', 'fr', 'es', 'it'] is exactly the slash-stripped image of the old ['/', '/en', ...], so every URL the old code kept is still kept — the change is strictly additive (it only rescues the trailing-slash variants). Nice touch bypassing __init__ via __new__ and stubbing rank_bm25 in the test, matching the sibling unit test.
One pre-existing, out-of-scope note (not this PR's job): uppercase roots like /EN/ are still filtered, since the whitelist is lowercase — same as before, just flagging in case the maintainers want to normalize case separately later. Nothing blocking; the fix reads correct and complete to me.
Summary
AsyncUrlSeeder._is_nonsense_urlfilters "very short paths", but the check is inconsistent:The length test strips slashes (
path.strip('/')), but the whitelist membership uses the un-strippedpath(which keeps trailing slashes). So the canonical trailing-slash form of a language root is dropped as nonsense:/en/en//de/,/fr/,/es/,/it//ab/,/x(real junk)Trailing slashes are the conventional directory form emitted by sitemaps and by
urljoinnormalization, so withfilter_nonsense_urlsenabled (the default) legitimate localized landing pages (/en/,/de/, …) were silently discarded from seeding results.Fix
Compare the slash-stripped path against the whitelist too, so a language root is kept with or without a trailing slash. Genuinely short non-language paths are still filtered.
Testing
Adds
tests/unit/test_nonsense_url_language_roots.py; the trailing-slash language-root cases fail on the current code (5 failed) and pass with this fix, while short-junk/utility URLs stay filtered.