-
Notifications
You must be signed in to change notification settings - Fork 10.9k
feat(core): add partUtils module with unit tests #4575
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
feat(core): add partUtils module with unit tests #4575
Conversation
Refactors part-related helper functions into a new module and adds comprehensive unit tests. - Extracted and from into . This improves code organization and reusability. - Added a new test file with thorough unit tests for both functions, covering various cases including invalid inputs and edge cases. - Renamed the module from to to align with the project's naming conventions.
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.
Summary of Changes
Hello @kamja44, 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 significantly improves the codebase's organization and testability by refactoring common utility functions related to content parts into their own module. This move not only enhances code reusability but also ensures the reliability of these critical functionalities through the addition of dedicated and thorough unit tests.
Highlights
- Code Organization & Refactoring: I've extracted the
partToStringandgetResponseTexthelper functions fromsummarizer.tsinto a new, dedicated module:packages/core/src/utils/partUtils.ts. This improves separation of concerns and enhances reusability of these core utility functions. - Improved Test Coverage: Comprehensive unit tests have been added for both
partToStringandgetResponseTextin a new file,packages/core/src/utils/partUtils.test.ts. These tests cover various scenarios, including invalid inputs, edge cases, and different content part types, ensuring the robustness of these utilities. - Naming Convention Alignment: The new utility module was initially named
partHelpers.tsbut was renamed topartUtils.tsto maintain consistency with existing naming conventions within the project.
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 is currently in preview and 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 to provide feedback.
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.
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 effectively refactors part-related utility functions into a new partUtils.ts module, accompanied by a solid set of unit tests. This is a great improvement for code organization and reusability.
My main feedback is on the implementation of getResponseText, where I've identified a correctness issue related to how text parts are filtered and a behavioral inconsistency in its return values. I've provided a suggestion to make the function more robust and predictable.
Overall, this is a valuable change, and with the suggested fix, it will be even better.
packages/core/src/utils/partUtils.ts
Outdated
|
|
||
| import { GenerateContentResponse, PartListUnion } from '@google/genai'; | ||
|
|
||
| export function partToString(part: PartListUnion): string { |
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.
there is a somewhat similar helper method in
geminiRequest.ts
| export function partListUnionToString(value: PartListUnion): string { |
Would it be feasible to unify these two similar util methods?
Ideally geminiRequest.ts could just call the partToString method from partsUtil.ts
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.
That's an excellent observation, @jacob314! Unifying similar utility methods across the codebase, especially by leveraging the newly introduced partToString in partUtils.ts, would indeed further enhance code reusability and maintainability.
@kamja44, this is a valuable suggestion for a follow-up improvement. If it's a straightforward change, you might consider incorporating it into this PR, or creating a separate issue to track this refactoring in geminiRequest.ts.
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.
Hi @jacob314 ,
Thank you for your valuable feedback and for suggesting the unification of the partToString and partListUnionToString methods. I've implemented the changes accordingly, and I believe this results in a cleaner and more robust solution.
Here's a summary of the changes:
-
Unified
partToStringFunction:
ThepartToStringfunction inpackages/core/src/utils/partUtils.tshas been enhanced to support both pure text extraction and detailed summary generation. It now accepts an optionaloptionsparameter with averboseflag:- When
verbose: trueis passed, the function returns human-readable summary strings (e.g.,[Video Metadata],[Function Call: myFunction]), integrating the behavior previously found inpartListUnionToString. - When
verboseis false or omitted, it preserves the original behavior of extracting only the plain text.
- When
-
geminiRequest.tsWrapper:
ThepartListUnionToStringfunction inpackages/core/src/core/geminiRequest.tsnow acts as a thin wrapper that simply delegates topartToString(value, { verbose: true }), preserving existing behavior and API compatibility. -
Consolidated Tests:
All unit tests related to part string conversion have been consolidated intopartUtils.test.ts, covering both default and verbose modes. The redundantgeminiRequest.test.tsfile has been removed.
This approach centralizes the logic, improves reusability, and maintains compatibility with existing interfaces—all while aligning with project conventions.
Please let me know if you have any questions or further suggestions.
Thanks again!
Consolidates part-related string conversion logic into a single partToString function in partUtils.ts, introducing a verbose option for detailed output. Unified Conversion Logic: The partToString function in packages/core/src/utils/partUtils.ts now handles both pure text extraction (default) and detailed summary generation (when verbose: true is passed). This integrates the logic previously found in geminiRequest.ts's partListUnionToString. Wrapper in geminiRequest.ts: packages/core/src/core/geminiRequest.ts's partListUnionToString is now a thin wrapper that calls partUtils.partToString with verbose: true, maintaining its original API and behavior. Consolidated Tests: All tests for part string conversion, including those previously in geminiRequest.test.ts, have been moved and expanded within packages/core/src/utils/partUtils.test.ts. This file now thoroughly tests partToString in both its default and verbose modes. Removed Redundant Test File: packages/core/src/core/geminiRequest.test.ts has been deleted as its tests are now covered by partUtils.test.ts.
jacob314
left a comment
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.
Co-authored-by: Jacob Richman <jacob314@gmail.com>
Co-authored-by: Jacob Richman <jacob314@gmail.com>
Co-authored-by: Jacob Richman <jacob314@gmail.com>
Co-authored-by: Jacob Richman <jacob314@gmail.com>
Co-authored-by: Jacob Richman <jacob314@gmail.com>
Co-authored-by: Jacob Richman <jacob314@gmail.com>

Refactors part-related helper functions into a new partUtils.ts module and adds comprehensive unit tests.
Extracted partToString and getResponseText from summarizer.ts into packages/core/src/utils/partUtils.ts. This improves code organization and reusability.
Added a new test file partUtils.test.ts with thorough unit tests for both functions, covering various cases including invalid inputs and edge cases.
Renamed the module from partHelpers.ts to partUtils.ts to align with the project's naming conventions.
TLDR
This PR extracts partToString and getResponseText into a new partUtils.ts module and adds dedicated unit tests for these utility functions. This improves code organization, reusability, and test coverage.
Dive Deeper
Previously, partToString and getResponseText were co-located within summarizer.ts. While functional, this placement was not ideal for reusability and clear separation of concerns. This PR moves these general-purpose utility functions into their own dedicated module, partUtils.ts, under packages/core/src/utils/.
Furthermore, comprehensive unit tests have been added in partUtils.test.ts to ensure the correctness and robustness of these functions. The tests cover various scenarios, including:
Handling null and undefined inputs for partToString.
Correctly concatenating string and object parts.
Extracting text from GenerateContentResponse objects, including cases with no candidates, no content, or mixed part types.
Adherence to the project's Array linting rule for array types in the test mock.
The module was initially named partHelpers.ts but was renamed to partUtils.ts to maintain consistency with existing utility file naming conventions within the project.
Testing Matrix
Linked issues / bugs