⚡ Bolt: Precompile LaTeX escape regex#454
Conversation
Hoisted `LATEX_SPECIAL_CHARS` dictionary and `LATEX_ESCAPE_PATTERN` regex compilation to the module level in `app.py` and `resume_generator_latex.py` to prevent unnecessary recompilation overhead on every function call. Co-authored-by: aafre <8656674+aafre@users.noreply.github.com>
|
👋 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. |
There was a problem hiding this comment.
Code Review
This pull request optimizes LaTeX escaping by hoisting regex compilation and character mappings to module-level constants in app.py and resume_generator_latex.py, reducing overhead during PDF generation. Feedback indicates that this change solidifies code duplication between the two files, and it is recommended to refactor this shared logic into a centralized utility module to adhere to DRY principles and improve maintainability.
| # Pre-compiled LaTeX escape mapping to avoid recreating on every call | ||
| LATEX_SPECIAL_CHARS = { | ||
| "\\": r"\textbackslash{}", | ||
| "&": r"\&", | ||
| "%": r"\%", | ||
| "$": r"\$", | ||
| "#": r"\#", | ||
| # "_": r"\_", # Don't escape: used for markdown italic/bold (_text_ and __text__) | ||
| "{": r"\{", | ||
| "}": r"\}", | ||
| # "~": r"\textasciitilde{}", # Don't escape: used for markdown strikethrough (~~text~~) | ||
| "^": r"\textasciicircum{}", | ||
| "<": r"\textless{}", | ||
| ">": r"\textgreater{}", | ||
| "|": r"\textbar{}", | ||
| "-": r"{-}", | ||
| } | ||
|
|
||
| LATEX_ESCAPE_PATTERN = re.compile("|".join(re.escape(key) for key in LATEX_SPECIAL_CHARS.keys())) |
There was a problem hiding this comment.
This change hoists constants for performance, but it also solidifies code duplication between app.py and resume_generator_latex.py. The LATEX_SPECIAL_CHARS dictionary, LATEX_ESCAPE_PATTERN regex, and the _escape_latex function are now identical in both files. This violates the DRY (Don't Repeat Yourself) principle and makes future maintenance harder, as any change would need to be synchronized.
To improve maintainability, this shared logic should be refactored into a dedicated utility module (e.g., latex_utils.py).
Suggested Refactoring:
- Create a new file
latex_utils.py. - Move
LATEX_SPECIAL_CHARS,LATEX_ESCAPE_PATTERN,escape_latex(renamed from_escape_latex), and other related functions like_prepare_latex_datainto it. - In both
app.pyandresume_generator_latex.py, replace the duplicated code with an import:from latex_utils import escape_latex
This would centralize the logic, making future updates easier and less error-prone.
References
- To ensure consistency and maintainability across the repository, avoid duplicating configuration data or logic (such as LaTeX escape mappings) in multiple files. Instead, define shared constants and functions in a centralized module.
LATEX_SPECIAL_CHARSdictionary andLATEX_ESCAPE_PATTERNregex compilation to the module level inapp.pyandresume_generator_latex.py._escape_latex(which is called heavily for every node/string during PDF generation) causes unnecessary overhead and slows down PDF rendering._escape_latexbased on isolated benchmarking.python3 -m pytest tests/to verify correctness is preserved.PR created automatically by Jules for task 7270678968453006995 started by @aafre