-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: integrate Gemini grounding sources into assistant message #6374
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
Conversation
- Modified streaming logic to append grounding sources to the last text chunk instead of yielding as separate message - Added tracking of content yielding to ensure sources only appear when content exists - Added comprehensive test coverage for grounding functionality including edge cases - Fixes issue where grounding sources appeared as separate message bubbles Fixes #6372
|
|
||
| let lastUsageMetadata: GenerateContentResponseUsageMetadata | undefined | ||
| let pendingGroundingMetadata: GroundingMetadata | undefined | ||
| let lastTextChunk: string | null = null |
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.
The variable lastTextChunk is assigned but never used. Consider removing it to clean up the code.
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.
I've reviewed this PR and found it successfully addresses the Gemini grounding sources integration issue. The implementation correctly integrates sources into assistant messages rather than displaying them as separate messages.
Critical Issues (Must Fix):
- Unused variable in gemini.ts:97 -
lastTextChunkis declared and assigned but never used, creating dead code that should be removed.
Important Suggestions (Should Consider):
-
Refactor grounding logic for reusability - The grounding metadata handling in lines 140-147 of gemini.ts could be extracted into a separate method for better maintainability and reusability between
createMessageandcompletePrompt. -
Enhanced test coverage - Missing test case for when grounding metadata exists but
extractCitationsOnlyreturns null (e.g., malformed grounding chunks without web URIs).
Minor Improvements (Nice to Have):
-
Add JSDoc comments - Consider adding documentation to the new
hasYieldedContentlogic to clarify its purpose for future maintainers. -
Robust mock setup - The translation mock in gemini.spec.ts could be more robust by handling additional translation keys that might be added in the future.
Positive Notes:
- Clean integration with existing streaming logic
- Comprehensive test coverage for main functionality
- Proper handling of edge cases (no content, no web sources)
- Successfully addresses the fragmented conversation experience described in issue #6372
The core functionality works well and the approach is sound. The critical issue with the unused variable should be addressed before merging.
|
This didn't fix the issue, closing |
Fixes #6372
Problem
When using the Gemini API with grounding enabled (
enableGrounding: true), the grounding sources (citations) were displayed as a separate message after the assistant's response, rather than being integrated into the assistant's message itself. This created a poor user experience where the conversation flow appeared fragmented.Solution
Modified the streaming logic in
src/api/providers/gemini.tsto:Changes
createMessagemethod to buffer and integrate grounding sourcescompletePromptmethod grounding integrationTesting
Before/After
Before:
After:
Important
Integrate Gemini grounding sources into assistant messages, modifying
gemini.tsand adding tests ingemini.spec.tsto ensure seamless message flow.createMessageandcompletePromptingemini.ts.gemini.spec.tsfor grounding sources integration, handling metadata without web sources, and edge cases with no content.gemini.spec.tsfor consistent test results.This description was created by
for a9174a8. You can customize this summary. It will automatically update as commits are pushed.