Repository navigation
Fix bugs found in #511 - #595
Merged
Merged
Conversation
The -include .env line is not part of the #511 fixes and would load local secrets into make. The domains.py and justext/core.py changes are crawler code, so the version is raised as release_check requires.
Collaborator
Author
|
Reviewed independently. Two fixes pushed in 46a8077:
make check passes and the 31 tests in the affected files pass. The select-branch test also passes on the original code, so it documents behaviour rather than guarding the removed branch. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Manual checks
Missing-row 404s (API). Against a running server with a valid JWT:
Confirm that deleting someone else's returns 400.
Moderation flow
Submit a domain, then approve it as a moderator, then reject another. Confirm both decisions are saved and appear in the right list. The code path changed from .get() to .filter().first(), so the success path should be identical, but it's worth one pass.
Domain link groups
The fix matters only where crawled pages link to github.com, en.wikipedia.org or news.ycombinator.com. Crawl or index a batch containing such links, then check that get_domain_score for a linked domain is non-zero. Before the fix those links were silently dropped. Verifying this needs the bloom filter files, so it's a data check rather than a unit check.
jusText extraction
Fetch a page with a
<select>dropdown and check that the extract doesn't contain the option text. The removed branch was unreachable, so the output should not change. A quick comparison on a few real pages would confirm that.Ranking predictor
RankingPredictor is used only by the offline LTR evaluation in evaluate.py, so it doesn't affect live search. Before the fix, predict would have raised on the string index. Running that evaluation end to end is the right check. Scores for pages with few matching terms should now be zero.