Mirror: Profile condense override with model-aware token caps (#5845)#43
Mirror: Profile condense override with model-aware token caps (#5845)#43jeremylongshore wants to merge 2 commits intomainfrom
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoRedesign context condensing thresholds with model-aware profile overrides
WalkthroughsDescription• Redesigned context condensing thresholds with model-aware token caps - Adds profile-specific override system supporting percent or token modes - Implements hard limit based on model's context window and reserved tokens - Auto-clamps token overrides when model capacity reduces • Refactored trigger resolution logic into reusable resolveCondenseTrigger function - Centralizes threshold calculation for both willManageContext and manageContext - Supports three modes: global percent, profile percent, profile tokens • Updated UI to show current profile override controls with effective budget display - Removed legacy profile selector from condensing threshold section - Added mode toggle, percent slider, and token input with validation • Added comprehensive test coverage for profile override behavior - Tests token threshold triggering, percent mode conversion, hard overflow handling - Tests UI state management, clamping notices, and independent global slider updates Diagramflowchart LR
A["Global Percent<br/>Setting"] --> B["resolveCondenseTrigger"]
C["Profile Override<br/>Config"] --> B
D["Model Context<br/>Window"] --> B
E["Reserved<br/>Tokens"] --> B
B --> F["Trigger Resolution<br/>Mode + Threshold"]
F --> G["willManageContext<br/>Decision"]
F --> H["manageContext<br/>Action"]
G --> I["Condense or<br/>Truncate"]
H --> I
File Changes1. src/core/context-management/index.ts
|
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Code Review by Qodo
1. react-hooks lint disabled
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| }), | ||
| ) | ||
| } | ||
| }, [currentProfileId, hardLimitTokens, profileCondenseOverrides, t]) // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
1. react-hooks lint disabled 📘 Rule violation ⛯ Reliability
A new eslint-disable-line react-hooks/exhaustive-deps suppression was added, weakening lint enforcement and potentially hiding real hook dependency bugs. This violates the requirement to not disable lint rules.
Agent Prompt
## Issue description
A new lint suppression (`// eslint-disable-line react-hooks/exhaustive-deps`) was introduced. This violates the compliance rule forbidding disabling lint rules and can hide real React hook dependency issues.
## Issue Context
The effect should either have a correct dependency list, or be refactored (e.g., by memoizing callbacks/values or restructuring logic) so it no longer requires suppressing `react-hooks/exhaustive-deps`.
## Fix Focus Areas
- webview-ui/src/components/settings/ContextManagementSettings.tsx[141-156]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| React.useEffect(() => { | ||
| const existingOverride = profileCondenseOverrides[currentProfileId] | ||
| if (!existingOverride || !existingOverride.enabled || existingOverride.mode !== "tokens") { | ||
| return | ||
| } | ||
| } | ||
|
|
||
| const clampedTokens = clampNumber(Math.floor(existingOverride.tokens), 1, hardLimitTokens) | ||
| if (clampedTokens !== existingOverride.tokens) { | ||
| updateCurrentProfileOverride({ tokens: clampedTokens }) | ||
| setTokenClampNotice( | ||
| t("settings:contextManagement.condensingThreshold.profileTokenClamped", { | ||
| tokens: clampedTokens, | ||
| }), | ||
| ) | ||
| } | ||
| }, [currentProfileId, hardLimitTokens, profileCondenseOverrides, t]) // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
2. Token override clobbered 🐞 Bug ✓ Correctness
If a user has an enabled per-profile token-mode override, the Settings UI can clamp it down to 1 token when model metadata is temporarily unavailable (effective budget computed as 0) and persist that reduced value. When model info later loads, the original override value cannot be recovered automatically, causing silent configuration loss.
Agent Prompt
## Issue description
When `condenseModelInfo` is temporarily unavailable (common for dynamic providers while model lists load), the UI computes an effective budget of `0`, which becomes a `hardLimitTokens` of `1`. If the current profile has an enabled token-mode override, the `useEffect` clamp runs and **persists** the reduced value (often to 1), permanently overwriting the user's saved threshold.
## Issue Context
- `useSelectedModel()` can return `info: undefined` while `isReady` is false.
- `SettingsView` uses `condenseModelInfo?.contextWindow ?? 0`, which yields a 0 budget when `info` is undefined.
- `ContextManagementSettings` clamps and persists token override values whenever `hardLimitTokens` changes.
## Fix Focus Areas
- webview-ui/src/components/settings/SettingsView.tsx[276-305]
- webview-ui/src/components/settings/ContextManagementSettings.tsx[103-156]
- webview-ui/src/components/ui/hooks/useSelectedModel.ts[118-150]
## Suggested approach
1. Treat the effective budget as **unknown** when `condenseModelInfo` is undefined (e.g., use `null` instead of `0`) and pass that through.
2. In `ContextManagementSettings`, **skip the clamping+persist effect** unless the budget is known and > 0 (or unless `useSelectedModel().isLoading` is false).
3. Optionally disable token-mode inputs/select until the cap is known, or show a loading indicator.
4. Add a regression test that simulates `condenseEffectiveBudgetTokens` temporarily being 0/unknown and verifies the saved override is not overwritten.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
Failed to generate code suggestions for PR |
|
@greptileai review |
|
Merged into |
2 participants
Mirror of Kilo-Org#5845
This PR mirrors the upstream change for multi-AI review analysis.
Bot Review Tracker
Links