feat(cli): invert context window display to show usage#20071
feat(cli): invert context window display to show usage#20071keithguerin merged 11 commits intomainfrom
Conversation
Summary of ChangesHello @keithguerin, 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 enhances the user experience related to context window management by shifting the display paradigm from 'tokens left' to 'tokens used'. This change, coupled with dynamic visual cues and clearer notifications, aims to provide users with a more immediate and understandable grasp of their model's context consumption, helping them manage their interactions more effectively and proactively avoid exceeding limits. Highlights
Changelog
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
|
|
Size Change: +1.27 kB (0%) Total Size: 25.8 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Code Review
The pull request successfully inverts the context window display to show usage percentage instead of remaining capacity. It also introduces dynamic coloring for warnings and errors, and simplifies related notifications. However, I identified a regression in responsive UI handling for narrow terminals, which impacts UI consistency, and a TypeScript type mismatch. Additionally, the use of magic numbers for layout should be addressed. These points should be resolved to maintain code quality and stability.
| export const ContextUsageDisplay = ({ | ||
| promptTokenCount, | ||
| model, | ||
| terminalWidth, | ||
| }: { | ||
| promptTokenCount: number; | ||
| model: string; | ||
| terminalWidth: number; | ||
| }) => { |
There was a problem hiding this comment.
The removal of the terminalWidth prop and the conditional label logic is a regression in UI robustness. Previously, the component would abbreviate the label to just % when the terminal width was less than 100 characters to prevent layout overflow in the footer. Restoring this behavior is important for maintaining a clean UI on narrow terminal windows, aligning with the principle of maintaining UI consistency across components. Additionally, the magic number 100 used for the terminal width threshold should be replaced with a named constant to improve readability and maintainability. Furthermore, the model prop type should be updated to string | undefined to match the uiState.currentModel type passed by callers, preventing potential TypeScript errors.
| export const ContextUsageDisplay = ({ | |
| promptTokenCount, | |
| model, | |
| terminalWidth, | |
| }: { | |
| promptTokenCount: number; | |
| model: string; | |
| terminalWidth: number; | |
| }) => { | |
| export const ContextUsageDisplay = ({ | |
| promptTokenCount, | |
| model, | |
| terminalWidth, | |
| }: { | |
| promptTokenCount: number; | |
| model: string | undefined; | |
| terminalWidth: number; | |
| }) => { |
References
- This rule emphasizes maintaining existing UI behavior across components and addressing larger UX changes holistically, rather than in isolated components. The comment highlights a regression in responsive UI that deviates from established behavior.
- This rule states that magic numbers, especially those for layout or padding, should be replaced with named constants to improve readability and maintainability. The comment identifies a hardcoded
100for terminal width as a magic number.
| textColor = theme.status.warning; | ||
| } | ||
|
|
||
| const label = '% context used'; |
There was a problem hiding this comment.
Re-implement the conditional label logic to ensure the UI remains responsive on narrow terminals, maintaining consistency with existing UI behavior. The magic number 100 used for the terminal width threshold should be replaced with a named constant (e.g., MIN_TERMINAL_WIDTH_FOR_FULL_LABEL) to improve readability and maintainability.
| const label = '% context used'; | |
| const label = terminalWidth < MIN_TERMINAL_WIDTH_FOR_FULL_LABEL ? '%' : '% context used'; |
References
- This rule emphasizes maintaining existing UI behavior across components and addressing larger UX changes holistically, rather than in isolated components. The comment highlights a regression in responsive UI that deviates from established behavior.
- This rule states that magic numbers, especially those for layout or padding, should be replaced with named constants to improve readability and maintainability. The comment identifies a hardcoded
100for terminal width as a magic number.
| <ContextUsageDisplay | ||
| promptTokenCount={uiState.sessionStats.lastPromptTokenCount} | ||
| model={uiState.currentModel} | ||
| terminalWidth={uiState.terminalWidth} | ||
| /> |
There was a problem hiding this comment.
Restore passing the terminalWidth prop to ContextUsageDisplay to support responsive label rendering, which aligns with maintaining consistency with existing UI behavior across components.
<ContextUsageDisplay
promptTokenCount={uiState.sessionStats.lastPromptTokenCount}
model={uiState.currentModel}
terminalWidth={uiState.terminalWidth}
/>
References
- This rule emphasizes maintaining existing UI behavior across components and addressing larger UX changes holistically, rather than in isolated components. The comment highlights a regression in responsive UI that deviates from established behavior.
| <ContextUsageDisplay | ||
| promptTokenCount={promptTokenCount} | ||
| model={model} | ||
| terminalWidth={terminalWidth} | ||
| /> |
There was a problem hiding this comment.
Restore passing the terminalWidth prop to ContextUsageDisplay to support responsive label rendering, which aligns with maintaining consistency with existing UI behavior across components.
<ContextUsageDisplay
promptTokenCount={promptTokenCount}
model={model}
terminalWidth={terminalWidth}
/>
References
- This rule emphasizes maintaining existing UI behavior across components and addressing larger UX changes holistically, rather than in isolated components. The comment highlights a regression in responsive UI that deviates from established behavior.
cd28f73 to
0033623
Compare
b6edaa5 to
942707e
Compare
jacob314
left a comment
There was a problem hiding this comment.
Will follow up with a minor code review fixes pr.
3c48a3c to
1af0be4
Compare
|
Pushed minor code review fixes to the pr. |
…#20071) Co-authored-by: jacob314 <jacob314@gmail.com>
…#20071) Co-authored-by: jacob314 <jacob314@gmail.com>
…#20071) Co-authored-by: jacob314 <jacob314@gmail.com>
Closes #20070
Inverts the context window display throughout the app to show used percentage instead of left. Rationale: Align with quota, other Google products, and competitors.
Changes
ContextUsageDisplay.tsxto show 'X% context used'.compressionThresholdis reached; red at 100%.IMPORTANT: This conversation exceeded the compression threshold. A compressed context will be sent for future messages (compressed from: {original} to {new} tokens).Context compressed from X% to Y%. Change threshold in /settings.(Single line, multi-colored).