feat: Script Output Summarization#14098
Conversation
Summary of ChangesHello @mgajda, 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 introduces a new feature to summarize lengthy script outputs before they are processed by the main thinking model. The primary goal is to enhance efficiency and clarity by utilizing the 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.
Code Review
This pull request introduces a new ScriptOutputSummarizationStrategy to handle long script outputs by summarizing them. The core idea is solid, but the current implementation has a critical flaw where the generated summary is not used, and the strategy ordering prevents it from running effectively. I've provided suggestions to fix the implementation, correct the strategy order, and improve the summarization prompt. Addressing these points will make the feature functional as intended.
| const summaryResult = await baseLlmClient.generateContent({ | ||
| modelConfigKey: { model: 'flash-lite' }, // Using flash-lite for summarization | ||
| contents: [summaryPrompt], | ||
| abortSignal: context.signal, | ||
| }); | ||
|
|
||
| const summarizedOutput = summaryResult.text; | ||
|
|
||
| // Return a decision to use the flash-lite model for the summarized output. | ||
| // This implies the next stage of routing or processing will use this summarized output. | ||
| return { | ||
| model: DEFAULT_GEMINI_FLASH_MODEL, // Using the default flash-lite alias | ||
| metadata: { | ||
| source: this.name, | ||
| latencyMs: 0, // Placeholder, actual measurement needed | ||
| reasoning: `Summarized script output using ${DEFAULT_GEMINI_FLASH_MODEL}.`, | ||
| }, | ||
| }; |
There was a problem hiding this comment.
The summarization logic is incomplete and contains a potential type error.
- Summary Not Used: A summary is generated but the
summarizedOutputis never used. The strategy returns aRoutingDecisionto useflash-lite, but the original, unsummarized content would be passed to the model. This defeats the purpose of the summarization. The strategy should modifyshellOutputPart.textwith the summary and returnnullto allow subsequent strategies to process the summarized content. - Incorrect
contentsType: Thecontentsproperty ofgenerateContentexpects aContent[](i.e.,Array<{role: string, parts: Part[]}>), but it's being passed astring[]. This could cause a runtime error.
The suggestion below fixes both issues.
const summaryResult = await baseLlmClient.generateContent({
modelConfigKey: { model: 'flash-lite' }, // Using flash-lite for summarization
contents: [{ role: 'user', parts: [{ text: summaryPrompt }] }],
abortSignal: context.signal,
});
const summarizedOutput = summaryResult.text;
// Replace the long output with the summary in the context for subsequent strategies.
shellOutputPart.text = summarizedOutput;
// Pass to the next strategy to make a final model decision on the summarized content.
return null;| new ClassifierStrategy(), | ||
| new ScriptOutputSummarizationStrategy(), // Added new strategy here |
There was a problem hiding this comment.
The ScriptOutputSummarizationStrategy is placed after the ClassifierStrategy. For the summarization to be effective, it needs to run before the classifier. The current order means that if the ClassifierStrategy makes a decision (e.g., routing to 'pro' model due to the long script output), the ScriptOutputSummarizationStrategy will never be executed. The summarization strategy should run first to shorten the input, and then the classifier can run on the summarized, shorter content.
| new ClassifierStrategy(), | |
| new ScriptOutputSummarizationStrategy(), // Added new strategy here | |
| new ScriptOutputSummarizationStrategy(), | |
| new ClassifierStrategy(), |
| const summaryPrompt = `Summarize the following script output concisely. Preserve key phrases and sentences that are critical to understanding the output verbatim. Present these excerpts clearly, perhaps by quoting them or using a specific marker like [EXCERPT]...[/EXCERPT]. Avoid paraphrasing or distorting the meaning of these excerpts. | ||
|
|
||
| Text to summarize: | ||
| \n\n${actualOutput}\n\n\n\ | ||
| ```\n`; |
There was a problem hiding this comment.
The summaryPrompt contains an unclosed markdown code block. The prompt ends with \``\n` which starts a code block but doesn't close it. This can confuse the model and lead to poor quality summaries. The content to be summarized should be wrapped in a closed code block for clarity.
| const summaryPrompt = `Summarize the following script output concisely. Preserve key phrases and sentences that are critical to understanding the output verbatim. Present these excerpts clearly, perhaps by quoting them or using a specific marker like [EXCERPT]...[/EXCERPT]. Avoid paraphrasing or distorting the meaning of these excerpts. | |
| Text to summarize: | |
| \n\n${actualOutput}\n\n\n\ | |
| ```\n`; | |
| const summaryPrompt = `Summarize the following script output concisely. Preserve key phrases and sentences that are critical to understanding the output verbatim. Present these excerpts clearly, perhaps by quoting them or using a specific marker like [EXCERPT]...[/EXCERPT]. Avoid paraphrasing or distorting the meaning of these excerpts. | |
| Text to summarize: | |
| \`\`\` | |
| ${actualOutput} | |
| \`\`\``; |
|
Hi there @mgajda While we appreciate these pull requests we need a proper feature request to be discussed prior to implementation. Please open a feature request to be discussed with the team here: https://github.com/google-gemini/gemini-cli/issues and not as a file in the PR. Once a design decision has been agreed upon then is the proper time to open a PR. Hope this makes sense 👍 Maybe I did not make myself clear earlier, my apologies for that. |
This PR implements the Script Output Summarization strategy. It allows the CLI to summarize lengthy script outputs using the flash-lite model while preserving key excerpts. Short outputs are skipped. The strategy is integrated into the model router. Related to issue #2.