-
Notifications
You must be signed in to change notification settings - Fork 8.7k
feat(core): improve import processor regex #10554
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(core): improve import processor regex #10554
Conversation
This commit improves the import processor regex to avoid matching code inside markdown code blocks. The new logic is based on the following rules: - A path is valid if it starts with `./`, `../`, or `/`. - A path is valid if it has a file extension. This change also includes new tests to cover the following cases: - npm-style packages in code blocks - Python decorators - Email addresses - Decorators - Subdirectory imports with and without extensions Fixes google-gemini#2967
Summary of ChangesHello @ksprashu, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the import processor where its overly broad regular expression incorrectly parsed various patterns within markdown code blocks as valid import statements, leading to Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request significantly improves the import processor's logic for identifying import statements in Markdown files. The new validation is more restrictive, correctly avoiding false positives from constructs like npm package names, decorators, and email addresses. The changes in packages/core/src/utils/memoryImportProcessor.ts
are clear and effectively implement the new validation rules. I'm particularly impressed with the comprehensive suite of new tests in packages/core/src/utils/memoryImportProcessor.test.ts
, which cover a wide range of edge cases and ensure the new logic is robust. The updated circular import test is also a great improvement. Overall, this is a high-quality contribution that addresses the reported issue effectively. I have not found any critical or high-severity issues.
This commit improves the import processor regex to avoid matching code inside markdown code blocks.
The new logic is based on the following rules:
./
,../
, or/
.This change also includes new tests to cover the following cases:
Fixes #2967
TLDR
The bug was caused by an overly permissive regex in the import processor, which incorrectly identified patterns in markdown code blocks as import statements. The fix refines the validation logic to correctly identify valid import paths while avoiding false positives.
Dive Deeper
The new logic ensures that a path is considered a valid import only if it starts with an explicit path prefix (./, ../, /) or contains a file extension. This approach is more robust and prevents the incorrect parsing of decorators, npm packages, and email addresses as import paths.
Reviewer Test Plan
Have a Markdown that contains '@' symbols.
Gemini tries to process the following strings and tries to import it.
Eg:
The error when running Gemini CLI is as below.
After the fix, we don't see any errors.
Testing Matrix
Linked issues / bugs