⚡ Bolt: Pre-compile regex patterns in job_parser.py#276
Open
⚡ Bolt: Pre-compile regex patterns in job_parser.py#276
Conversation
💡 What: Lifted repeatedly compiled regular expressions out of `job_parser.py` parsing methods and pre-compiled them as module-level constants. 🎯 Why: Instantiating `re.compile` within parsing methods (e.g. `soup.find`) or inside loops incurs redundant parsing overhead. Evaluated them exactly once instead. 📊 Impact: Decreases parsing time for large HTML documents. 🔬 Measurement: Run integration tests to ensure extraction remains regression-free. Co-authored-by: anchapin <[email protected]>
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Reviewer's GuidePre-compiles frequently used regular expressions in job_parser.py into module-level constants and updates parsing utilities to accept pre-compiled patterns, reducing redundant regex compilation in hot HTML parsing paths; also documents this optimization pattern in .jules/bolt.md. Class diagram for updated JobParser regex handlingclassDiagram
class JobParser {
+_parse_indeed(html: str) JobDetails
+_parse_generic(html: str) JobDetails
+_find_by_selectors(soup: BeautifulSoup, selectors: List[str]) Optional[Tag]
+_extract_text_by_pattern(text: str, pattern: Union[str, re.Pattern]) Optional[str]
+_extract_salary_from_text(text: str) Optional[str]
+_extract_sections_from_description(description: str) Tuple[List[str], List[str]]
+_extract_items_from_text(text: str) List[str]
+_extract_list_items(element: Tag) List[str]
+_extract_list_by_keyword(html: str, keyword: Union[str, re.Pattern]) List[str]
+_extract_job_type(html: str) Optional[str]
+_extract_experience_level(html: str) Optional[str]
}
class JobParserRegexPatterns {
<<module_constants>>
_INDEED_HEADER_PATTERN : re.Pattern
_INDEED_TITLE_SUFFIX_PATTERN : re.Pattern
_REQ_HEADING_PATTERN : re.Pattern
_RESP_HEADING_PATTERN : re.Pattern
_SALARY_PATTERNS : List[re.Pattern]
_SALARY_CLEAN_PATTERN : re.Pattern
_SALARY_K_PATTERN : re.Pattern
_REQ_SECTION_PATTERN : re.Pattern
_RESP_SECTION_PATTERN : re.Pattern
_NEXT_SECTION_PATTERNS : List[re.Pattern]
_BULLET_PATTERNS : List[re.Pattern]
_COMMA_SEP_PATTERN : re.Pattern
_JOB_TYPE_PATTERNS : List[re.Pattern]
_EXPERIENCE_LEVEL_PATTERNS : List[re.Pattern]
}
JobParser --> JobParserRegexPatterns : uses precompiled patterns
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- For the helpers that accept regexes (e.g.
_extract_text_by_pattern,_extract_list_by_keyword), consider typing the pattern argument asUnion[str, Pattern[str]]instead ofUnion[str, re.Pattern]to play more nicely with type checkers and avoid the need for some of thetype: ignoreannotations when passing precompiled patterns into BeautifulSoup. - In
_SALARY_PATTERNS, only some patterns use capturing groups while others rely on the full match; it may be clearer and less error-prone to either make all patterns use a consistent capturing convention (e.g. always capture the salary part) or to split them into two lists with explicit handling, so future edits don’t accidentally break the group indexing logic.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- For the helpers that accept regexes (e.g. `_extract_text_by_pattern`, `_extract_list_by_keyword`), consider typing the pattern argument as `Union[str, Pattern[str]]` instead of `Union[str, re.Pattern]` to play more nicely with type checkers and avoid the need for some of the `type: ignore` annotations when passing precompiled patterns into BeautifulSoup.
- In `_SALARY_PATTERNS`, only some patterns use capturing groups while others rely on the full match; it may be clearer and less error-prone to either make all patterns use a consistent capturing convention (e.g. always capture the salary part) or to split them into two lists with explicit handling, so future edits don’t accidentally break the group indexing logic.
## Individual Comments
### Comment 1
<location path="cli/integrations/job_parser.py" line_range="849" />
<code_context>
+ for pattern in _JOB_TYPE_PATTERNS:
+ match = pattern.search(html)
if match:
return match.group(1).lower().replace("-", "-")
</code_context>
<issue_to_address>
**issue (bug_risk):** The `replace('-', '-')` call is a no-op and likely not doing the intended normalization.
Right now this only lowercases the job type; the `.replace('-', '-')` has no effect. If you meant to normalize spacing or hyphens, the second argument should differ (e.g. `.replace(' ', '-')` or `.replace('-', ' ')`). Please update to reflect the intended normalization.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| for pattern in _JOB_TYPE_PATTERNS: | ||
| match = pattern.search(html) | ||
| if match: | ||
| return match.group(1).lower().replace("-", "-") |
There was a problem hiding this comment.
issue (bug_risk): The replace('-', '-') call is a no-op and likely not doing the intended normalization.
Right now this only lowercases the job type; the .replace('-', '-') has no effect. If you meant to normalize spacing or hyphens, the second argument should differ (e.g. .replace(' ', '-') or .replace('-', ' ')). Please update to reflect the intended normalization.
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
1 participant
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.
⚡ Bolt: Pre-compile regex patterns in job_parser.py
💡 What: Lifted repeatedly compiled regular expressions out of
job_parser.pyparsing methods and pre-compiled them as module-level constants.🎯 Why: Instantiating
re.compilewithin parsing methods (e.g.soup.find) or inside loops incurs redundant parsing overhead. Evaluated them exactly once instead.📊 Impact: Decreases parsing time for large HTML documents.
🔬 Measurement: Run integration tests to ensure extraction remains regression-free.
PR created automatically by Jules for task 11956294251020149341 started by @anchapin
Summary by Sourcery
Pre-compile regular expression patterns in the job parser to reduce repeated compilation overhead during HTML parsing.
Enhancements:
Documentation: