fix: Control UI context % shows 100% when actual is ~56%#60649
fix: Control UI context % shows 100% when actual is ~56%#60649progamman wants to merge 1 commit intoopenclaw:mainfrom
Conversation
Root cause: UI used lifetime accumulated tokens (totalTokens) for context percentage, which stays high after compaction. Should use current window tokens (post-compaction) instead. Fix: 1. Add currentWindowTokens field to GatewaySessionRow type 2. Gateway computes it: when totalTokensFresh=false (post-compaction), use transcriptUsage.totalTokens (current window), else use totalTokens 3. UI uses currentWindowTokens for context % calculation, with fallback to totalTokens for backwards compatibility 4. Remove early return when totalTokensFresh=false - now shows correct post-compaction percentage via currentWindowTokens Fixes openclaw#48252
Greptile SummaryThis PR aims to fix a Control UI bug where the context usage percentage shows an inflated value (e.g. 100%) after compaction instead of the true post-compaction window size (~56%). The fix has three parts: adding a Key changes:
Notable concern: The The removal of the premature Confidence Score: 3/5Merge with caution — the UI fix is safe, but the gateway-side The Pay close attention to Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/gateway/session-utils.ts
Line: 1289
Comment:
**`currentWindowTokens` is always equal to `totalTokens`**
The computation produces the same value as `totalTokens` in every reachable code path, so the new field provides no additional data despite the intent in the PR description.
Trace through the two possible states:
**When `totalTokensFresh = true`**
`totalTokens` came from `resolveFreshSessionTotalTokens(entry)`, so `currentWindowTokens = totalTokens` (first branch).
**When `totalTokensFresh = false`**
`resolveFreshSessionTotalTokens(entry)` returned `undefined` (it short-circuits when `entry.totalTokensFresh === false` per `src/config/sessions/types.ts:320`), so `totalTokens` was set from `transcriptUsage?.totalTokens`. Therefore `transcriptUsage?.totalTokens ?? totalTokens` equals `totalTokens` (second branch also collapses to the same value).
The scenario the PR intends to handle — where `resolveFreshSessionTotalTokens` returns a positive lifetime total while `transcriptUsage.totalTokensFresh === false` — would require the entry's `totalTokensFresh` flag to be unset even after compaction. In that case `totalTokensFresh` in `GatewaySessionRow` would be `true`, so the ternary still falls into the first branch (`currentWindowTokens = totalTokens = lifetime total`).
What the intent likely requires is to condition on `transcriptUsage?.totalTokensFresh` directly:
```ts
// Use transcript's current-window count when the transcript signals post-compaction;
// otherwise fall back to the accumulated total.
currentWindowTokens:
transcriptUsage?.totalTokensFresh === false
? (transcriptUsage.totalTokens ?? totalTokens)
: totalTokens,
```
The actual user-visible fix in `chat.ts` (removing the premature `return nothing`) is independent and correct, but the intent of having a semantically distinct `currentWindowTokens` is not achieved by the current implementation.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/gateway/session-utils.types.ts
Line: 51-52
Comment:
**Misleading JSDoc on `currentWindowTokens`**
The comment "Use this for UI context %, **not totalTokens which is lifetime**" is inaccurate. In `GatewaySessionRow`, `totalTokens` is NOT always the lifetime-accumulated value — when `totalTokensFresh === false`, `totalTokens` already holds the current-window figure sourced from the transcript (because `resolveFreshSessionTotalTokens` rejects it and the fallback is `transcriptUsage?.totalTokens`). Calling it "lifetime" here will mislead future readers into thinking the fields carry different semantics when they currently don't.
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: src/gateway/session-utils.ts
Line: 1288-1289
Comment:
**No test coverage for the post-compaction context percentage path**
There are no tests (in `session-utils.test.ts` or elsewhere) that exercise the `currentWindowTokens` field, either for the `totalTokensFresh = true` or `totalTokensFresh = false` branches. Given that the PR is fixing a subtle UI bug around post-compaction token display, a test asserting the computed value of `currentWindowTokens` when transcript data is present (and `entry.totalTokensFresh === false`) would prevent regressions and make the intended semantics explicit.
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: Control UI context % shows 100% whe..." | Re-trigger Greptile |
| totalTokens, | ||
| totalTokensFresh, | ||
| // currentWindowTokens: actual context in use (post-compaction = transcript tokens when fresh=false) | ||
| currentWindowTokens: totalTokensFresh ? totalTokens : (transcriptUsage?.totalTokens ?? totalTokens), |
There was a problem hiding this comment.
currentWindowTokens is always equal to totalTokens
The computation produces the same value as totalTokens in every reachable code path, so the new field provides no additional data despite the intent in the PR description.
Trace through the two possible states:
When totalTokensFresh = true
totalTokens came from resolveFreshSessionTotalTokens(entry), so currentWindowTokens = totalTokens (first branch).
When totalTokensFresh = false
resolveFreshSessionTotalTokens(entry) returned undefined (it short-circuits when entry.totalTokensFresh === false per src/config/sessions/types.ts:320), so totalTokens was set from transcriptUsage?.totalTokens. Therefore transcriptUsage?.totalTokens ?? totalTokens equals totalTokens (second branch also collapses to the same value).
The scenario the PR intends to handle — where resolveFreshSessionTotalTokens returns a positive lifetime total while transcriptUsage.totalTokensFresh === false — would require the entry's totalTokensFresh flag to be unset even after compaction. In that case totalTokensFresh in GatewaySessionRow would be true, so the ternary still falls into the first branch (currentWindowTokens = totalTokens = lifetime total).
What the intent likely requires is to condition on transcriptUsage?.totalTokensFresh directly:
// Use transcript's current-window count when the transcript signals post-compaction;
// otherwise fall back to the accumulated total.
currentWindowTokens:
transcriptUsage?.totalTokensFresh === false
? (transcriptUsage.totalTokens ?? totalTokens)
: totalTokens,The actual user-visible fix in chat.ts (removing the premature return nothing) is independent and correct, but the intent of having a semantically distinct currentWindowTokens is not achieved by the current implementation.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/session-utils.ts
Line: 1289
Comment:
**`currentWindowTokens` is always equal to `totalTokens`**
The computation produces the same value as `totalTokens` in every reachable code path, so the new field provides no additional data despite the intent in the PR description.
Trace through the two possible states:
**When `totalTokensFresh = true`**
`totalTokens` came from `resolveFreshSessionTotalTokens(entry)`, so `currentWindowTokens = totalTokens` (first branch).
**When `totalTokensFresh = false`**
`resolveFreshSessionTotalTokens(entry)` returned `undefined` (it short-circuits when `entry.totalTokensFresh === false` per `src/config/sessions/types.ts:320`), so `totalTokens` was set from `transcriptUsage?.totalTokens`. Therefore `transcriptUsage?.totalTokens ?? totalTokens` equals `totalTokens` (second branch also collapses to the same value).
The scenario the PR intends to handle — where `resolveFreshSessionTotalTokens` returns a positive lifetime total while `transcriptUsage.totalTokensFresh === false` — would require the entry's `totalTokensFresh` flag to be unset even after compaction. In that case `totalTokensFresh` in `GatewaySessionRow` would be `true`, so the ternary still falls into the first branch (`currentWindowTokens = totalTokens = lifetime total`).
What the intent likely requires is to condition on `transcriptUsage?.totalTokensFresh` directly:
```ts
// Use transcript's current-window count when the transcript signals post-compaction;
// otherwise fall back to the accumulated total.
currentWindowTokens:
transcriptUsage?.totalTokensFresh === false
? (transcriptUsage.totalTokens ?? totalTokens)
: totalTokens,
```
The actual user-visible fix in `chat.ts` (removing the premature `return nothing`) is independent and correct, but the intent of having a semantically distinct `currentWindowTokens` is not achieved by the current implementation.
How can I resolve this? If you propose a fix, please make it concise.| /** Current context window tokens (post-compaction). Use this for UI context %, not totalTokens which is lifetime. */ | ||
| currentWindowTokens?: number; |
There was a problem hiding this comment.
currentWindowTokens
The comment "Use this for UI context %, not totalTokens which is lifetime" is inaccurate. In GatewaySessionRow, totalTokens is NOT always the lifetime-accumulated value — when totalTokensFresh === false, totalTokens already holds the current-window figure sourced from the transcript (because resolveFreshSessionTotalTokens rejects it and the fallback is transcriptUsage?.totalTokens). Calling it "lifetime" here will mislead future readers into thinking the fields carry different semantics when they currently don't.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/session-utils.types.ts
Line: 51-52
Comment:
**Misleading JSDoc on `currentWindowTokens`**
The comment "Use this for UI context %, **not totalTokens which is lifetime**" is inaccurate. In `GatewaySessionRow`, `totalTokens` is NOT always the lifetime-accumulated value — when `totalTokensFresh === false`, `totalTokens` already holds the current-window figure sourced from the transcript (because `resolveFreshSessionTotalTokens` rejects it and the fallback is `transcriptUsage?.totalTokens`). Calling it "lifetime" here will mislead future readers into thinking the fields carry different semantics when they currently don't.
How can I resolve this? If you propose a fix, please make it concise.| // currentWindowTokens: actual context in use (post-compaction = transcript tokens when fresh=false) | ||
| currentWindowTokens: totalTokensFresh ? totalTokens : (transcriptUsage?.totalTokens ?? totalTokens), |
There was a problem hiding this comment.
There are no tests (in session-utils.test.ts or elsewhere) that exercise the currentWindowTokens field, either for the totalTokensFresh = true or totalTokensFresh = false branches. Given that the PR is fixing a subtle UI bug around post-compaction token display, a test asserting the computed value of currentWindowTokens when transcript data is present (and entry.totalTokensFresh === false) would prevent regressions and make the intended semantics explicit.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/gateway/session-utils.ts
Line: 1288-1289
Comment:
**No test coverage for the post-compaction context percentage path**
There are no tests (in `session-utils.test.ts` or elsewhere) that exercise the `currentWindowTokens` field, either for the `totalTokensFresh = true` or `totalTokensFresh = false` branches. Given that the PR is fixing a subtle UI bug around post-compaction token display, a test asserting the computed value of `currentWindowTokens` when transcript data is present (and `entry.totalTokensFresh === false`) would prevent regressions and make the intended semantics explicit.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 472498fb2c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| const used = session?.totalTokens ?? 0; | ||
| // Use currentWindowTokens if available (post-compaction), fall back to totalTokens (lifetime) | ||
| // Note: We no longer hide when totalTokensFresh=false because currentWindowTokens is valid | ||
| const used = session?.currentWindowTokens ?? session?.totalTokens ?? 0; |
There was a problem hiding this comment.
Keep stale totals hidden without current-window tokens
This change removed the totalTokensFresh === false guard and now always falls back to totalTokens when currentWindowTokens is missing. In payloads that do not include the new field (for example older gateway responses or any partial row shape), stale totalTokens values are rendered again, which reintroduces incorrect high context percentages instead of suppressing them. The fallback should only use totalTokens when freshness is true, otherwise continue hiding the notice.
Useful? React with 👍 / 👎.
| totalTokens, | ||
| totalTokensFresh, | ||
| // currentWindowTokens: actual context in use (post-compaction = transcript tokens when fresh=false) | ||
| currentWindowTokens: totalTokensFresh ? totalTokens : (transcriptUsage?.totalTokens ?? totalTokens), |
There was a problem hiding this comment.
Derive current-window tokens from stale-state source
currentWindowTokens is keyed off totalTokensFresh, but totalTokensFresh is computed earlier from whether totalTokens is present, so this expression collapses to the same value as totalTokens for any session with tokens. As written, the new field does not provide a distinct post-compaction metric, so switching the UI to currentWindowTokens cannot correct over-reported context usage in cases where totalTokens is already wrong.
Useful? React with 👍 / 👎.
1. openclaw#60667 - Properly drop incomplete-thinking (unsigned) to avoid API errors 2. openclaw#60664 - Handle deletion edits (empty newText) in edit recovery 3. openclaw#60649 - Fix currentWindowTokens to use transcriptUsage?.totalTokensFresh All fixes address Greptile review feedback.
Update: v2 branch with fixCreated v2 branch: progamman/fix/control-ui-context-percentage-v2 Fixed: The currentWindowTokens computation now uses transcriptUsage?.totalTokensFresh directly instead of the derived totalTokensFresh variable. This ensures we get the true post-compaction context window size. |
|
👋 Gentle reminder - this PR is awaiting review. Please let us know if any changes are needed. Thanks! |
|
Closing as superseded by #71297. This PR and #71297 address the same Control UI chat warning failure tracked by #48252: stale/current-context mismatch causing false #71297 is the canonical review path because it fixes the chat notice against stale snapshots, applies live session metadata as gateway events arrive, coalesces session reloads, and adds focused tests for those freshness paths plus the compact recommendation CTA. |
Root cause: UI used lifetime accumulated tokens (totalTokens) for context percentage, which stays high after compaction. Should use current window tokens (post-compaction) instead.
Fix:
Fixes #48252