feat(extensions): Allow multiple context files #7611
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Summary of Changes
Hello @oomangparekh, 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 enhances the flexibility of extension configurations by introducing support for multiple context files. This change allows extension developers to define and manage their contextual information across several files, improving the structure and maintainability of their extensions. The update includes a new configuration property and ensures backward compatibility by gracefully handling the deprecated single context file property.
Highlights
- Multiple Context Files for Extensions: Extensions can now specify multiple context files using a new
contextFileNamesproperty in theirgemini-extension.jsonconfiguration, allowing for more organized and modular context management. - Deprecation of
contextFileName: The previouscontextFileNameproperty is now deprecated. The system will prioritizecontextFileNames, then fall back tocontextFileName, and finally toGEMINI.mdif neither is specified. - Updated Configuration Logic: The internal logic for loading extension configurations has been updated to correctly parse and utilize the new
contextFileNamesarray, ensuring all specified context files are loaded. - Documentation and Testing: The documentation for extension configuration has been updated to reflect these changes, and new test cases have been added to validate the loading of multiple context files.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
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 .gemini/ folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces support for multiple context files via a new contextFileNames property in gemini-extension.json, deprecating the old contextFileName. While the new functionality and tests are well-implemented, there's a critical regression that breaks backward compatibility. The updated logic no longer correctly handles the case where contextFileName was used with an array of strings, which could cause existing extensions to fail silently when loading context files. My review includes a suggested fix to ensure backward compatibility is maintained.
packages/cli/src/config/extension.ts
Outdated
| function getContextFileNames(config: ExtensionConfig): string[] { | ||
| if (!config.contextFileName) { | ||
| return ['GEMINI.md']; | ||
| } else if (!Array.isArray(config.contextFileName)) { | ||
| if (config.contextFileNames) { | ||
| return config.contextFileNames; | ||
| } | ||
| if (config.contextFileName) { | ||
| return [config.contextFileName]; | ||
| } | ||
| return config.contextFileName; | ||
| return ['GEMINI.md']; | ||
| } |
There was a problem hiding this comment.
This change introduces a regression that breaks backward compatibility for extensions using an array for contextFileName.
Previously, contextFileName could be a string[]. The new implementation in getContextFileNames assumes it's always a string by wrapping it in an array: return [config.contextFileName];.
If an existing extension has "contextFileName": ["a.md", "b.md"], at runtime config.contextFileName will be an array. This function will then incorrectly return [['a.md', 'b.md']]. This nested array will later cause path.join to fail silently by creating an invalid path like .../a.md,b.md, preventing context files from being loaded.
To ensure backward compatibility and prevent silent failures for existing extensions, please restore the logic to handle arrays for contextFileName.
Additionally, it would be beneficial to add a test case that verifies this backward compatibility scenario.
function getContextFileNames(config: ExtensionConfig): string[] {
if (config.contextFileNames) {
return config.contextFileNames;
}
// For backward compatibility, handle if an old config still passes an array.
// We cast to `any` to check the runtime type, as the `ExtensionConfig`
// interface has been updated.
if (Array.isArray((config as any).contextFileName)) {
console.warn(
`[DEPRECATION] The 'contextFileName' property with an array value in extension '${config.name}' is deprecated. Please migrate to 'contextFileNames'.`
);
return (config as any).contextFileName;
}
if (config.contextFileName) {
return [config.contextFileName];
}
return ['GEMINI.md'];
}c228b1f to
96ad02c
Compare
TLDR
Dive Deeper
Reviewer Test Plan
Testing Matrix
Linked issues / bugs