Feat: Profile condense override with model-aware token caps#5845
Feat: Profile condense override with model-aware token caps#5845Neonsy wants to merge 2 commits intoKilo-Org:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 98ae2e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Neonsy
changed the title
Neonsy
force-pushed
the
feat/profile-context-override-and-none-percentage-value-option
branch
from
cdb4aa8 to
9b84b90
Compare
|
I like it!! |
@abdulrahimpds suggested it on Discord |
Neonsy
force-pushed
the
feat/profile-context-override-and-none-percentage-value-option
branch
from
9b84b90 to
8be03a8
Compare
|
@Neonsy will look into it soon |
|
I probably won't change the core PR though, cause this current setup makes the most sense You cannot have a fixed size limit as global, cause models have different max values |
Neonsy
force-pushed
the
feat/profile-context-override-and-none-percentage-value-option
branch
from
8be03a8 to
f224829
Compare
Neonsy
force-pushed
the
feat/profile-context-override-and-none-percentage-value-option
branch
5 times, most recently
from
d8f0678 to
3fda336
Compare
Neonsy
force-pushed
the
feat/profile-context-override-and-none-percentage-value-option
branch
from
3fda336 to
98ae2e5
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| export const TOKEN_BUFFER_PERCENTAGE = 0.1 | ||
|
|
||
| // kilocode_change start | ||
| export type ProfileCondenseOverride = { |
There was a problem hiding this comment.
[SUGGESTION]: Duplicate type definition — ProfileCondenseOverride is already defined and exported from @roo-code/types via packages/types/src/global-settings.ts (line 64). Consider importing it instead of redefining it here to keep a single source of truth.
The same shape is also duplicated as ProfileCondenseOverrideValue in ContextManagementSettings.tsx and inline in vscode-extension-host.ts. Consolidating to one canonical type from @roo-code/types would reduce maintenance burden.
| export const profileCondenseOverrideSchema = z.object({ | ||
| enabled: z.boolean(), | ||
| mode: profileCondenseOverrideModeSchema, | ||
| percent: z.number(), |
There was a problem hiding this comment.
[SUGGESTION]: The Zod schema uses bare z.number() for percent and tokens without range constraints. While the application layer clamps values (e.g., clampNumber in the UI and resolveCondenseTrigger on the backend), adding .min()/.max() here would provide defense-in-depth validation at the schema level.
For example:
percent: z.number().min(0).max(100),
tokens: z.number().min(0),| }), | ||
| ) | ||
| } | ||
| }, [currentProfileId, hardLimitTokens, profileCondenseOverrides, t]) // eslint-disable-line react-hooks/exhaustive-deps |
There was a problem hiding this comment.
[SUGGESTION]: The eslint-disable-line react-hooks/exhaustive-deps suppresses the missing updateCurrentProfileOverride dependency. While this is safe in practice (since profileCondenseOverrides is already in the deps array), wrapping updateCurrentProfileOverride in React.useCallback would eliminate the need for the suppression and make the dependency chain explicit.
| "defaultProfile": "افتراضي عام", | ||
| "defaultDescription": "إذا وصل السياق للنسبة يختصر (لكل الملفات).", | ||
| "profileDescription": "نسبة مخصصة لهذا الملف فقط.", | ||
| "profileOverrideLabel": "Override for this profile", |
There was a problem hiding this comment.
[WARNING]: All 8 new translation keys (profileOverrideLabel, currentProfileLabel, hardLimitLabel, mode.percent, mode.tokens, percentEquivalent, tokensUnit, profileTokenClamped) are untranslated English strings in all 21 non-English locale files. These should be translated or flagged for localization follow-up.
This comment applies to all non-English locale files changed in this PR (ar, ca, cs, de, es, fr, hi, id, it, ja, ko, nl, pl, pt-BR, ru, sk, th, tr, uk, vi, zh-CN, zh-TW).
Code Review SummaryStatus: 4 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)Issues found in unchanged code or cross-cutting concerns:
Files Reviewed (38 files)
|
|
Hi! Thank you for taking the time to contribute to this project—we really appreciate it. We are currently working on re-platforming the core of our VS Code and JetBrains extensions to be based on our new Kilo CLI, with a complete rebuild based on OpenCode as our new foundation, which means this PR unfortunately won't be compatible with the new architecture. If you think this feature or fix is still relevant, we'd love for you to check out the new version and consider contributing it there. The new code is now in this repository. Alternatively, if you think this is an urgent fix to the old vscode extension (like a recent model change, or an urgent bug fix), we’d welcome the PR at https://github.com/Kilo-Org/kilocode-legacy |
4 participants
Context
Implemented a surgical redesign of condense trigger settings so global behavior stays percent-based, while profile-level behavior can be explicitly overridden per settings-selected profile using either
% of hard limitor a fixed token threshold.This addresses UX confusion around percentage-only control by allowing direct token targeting without changing global semantics.
Implementation
Added a new
profileCondenseOverridessetting shape and threaded it through shared types, extension state, provider serialization, webview settings payload flow, and runtime threshold resolution.Key changes:
Override for this profileenable checkbox% of hard limit/Fixed token limit5..100) with computed token equivalentwillManageContextandmanageContextshare the same logic.prevContextTokens > allowedTokensstill forces context management).profileThresholdsas backward-compatible read fallback; new writes useprofileCondenseOverrides.How to Test
Context Management.%mode:Fixed token limitmode:cd webview-ui && pnpm test src/components/settings/__tests__/ContextManagementSettings.spec.tsxcd webview-ui && pnpm test src/components/settings/__tests__/SettingsView.change-detection.spec.tsxcd webview-ui && pnpm test src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsxcd src && pnpm test core/context-management/__tests__/context-management.spec.tscd src && pnpm test core/webview/__tests__/ClineProvider.spec.tscd src && pnpm check-typescd src && pnpm lintpnpm buildNote
Research done by myself (mostly) but implementation was fully done by AI this time.
I have only tested the basics, so I'm not aware of edge cases.
@abdulrahimpds @mikij @bernaferrari @kevinvandijk