fix(control-ui): use fresh totals for context usage notice#45335
Closed
KaneOrca wants to merge 3 commits intoopenclaw:mainfrom
Closed
fix(control-ui): use fresh totals for context usage notice#45335KaneOrca wants to merge 3 commits intoopenclaw:mainfrom
KaneOrca wants to merge 3 commits intoopenclaw:mainfrom
Conversation
Contributor
Greptile SummaryThis PR fixes two related issues in the Control UI: (1) the chat context-usage warning and Key changes:
Minor concerns noted:
Confidence Score: 4/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: ui/src/ui/presenter.ts
Line: 38-39
Comment:
**Implicit "fresh" assumption when `totalTokensFresh` is absent**
When `totalTokensFresh` is `undefined` (i.e., the gateway row pre-dates this field), the condition `row?.totalTokensFresh !== false` evaluates to `true`, so `totalTokens` is returned as if it were a fresh snapshot. For sessions that already had `totalTokens` set — but where the API was accumulating it across tool loops — this silently changes the displayed percentage compared to the previous behaviour of always using `inputTokens`.
This is intentional according to the PR description ("prefer fresh totals when available"), but worth noting that this is effectively a **behavior change for any session row that carries `totalTokens` without `totalTokensFresh`**, not purely a bug-fix for sessions that explicitly set `totalTokensFresh: true`.
Consider adding a brief comment to make the intent explicit:
```suggestion
if (typeof total === "number" && Number.isFinite(total) && row?.totalTokensFresh !== false) {
// When totalTokensFresh is undefined (legacy rows), we still prefer totalTokens
// over accumulated inputTokens as the better proxy for current context usage.
return total;
```
How can I resolve this? If you propose a fix, please make it concise.
---
This is a comment left during a code review.
Path: ui/src/ui/views/chat.test.ts
Line: 356-383
Comment:
**Missing complementary test cases for context notice**
The new test confirms that when `totalTokensFresh: true` and the fresh `totalTokens` is below the 85 % threshold, no notice is rendered — which is the core regression being fixed.
Two related scenarios aren't covered and would strengthen confidence:
1. **Positive path**: `totalTokensFresh: true` with `totalTokens / contextTokens ≥ 0.85` — the notice *should* appear using the fresh total, not `inputTokens`.
2. **Fallback path**: `totalTokensFresh: false` with high accumulated `inputTokens` — the notice should still appear via the `inputTokens` fallback.
Without these, a future refactor that accidentally drops the `≥ 0.85` branch for fresh totals would go undetected by the new test.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: 8b8836b |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8b8836b624
ℹ️ 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".
Author
|
Supplemental visual evidence for this UI fix. This is a local minimal repro of the context-usage notice rendering, added to document the before/after visual difference for reviewers.
|
Member
|
Closing as superseded by #71297. The canonical PR keeps the Control UI chat context notice fix narrower and current: fresh This PR overlaps that same Control UI chat warning/styling path, so keeping #71297 as the review path avoids maintaining two implementations of the same fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
totalTokenssnapshots when available instead of accumulatedinputTokens/usagealigned with the same context-usage calculation rule.context-noticestyles so the warning icon renders as a compact inline notice instead of a giant raw SVGChange Type
Scope
Linked Issue/PR
User-visible / Behavior Changes
/usagenow prefer freshtotalTokenssnapshots, so long tool loops do not overstate current context-window usageinputTokensSecurity Impact
NoNoNoNoNoYes, explain risk + mitigation: NoneRepro + Verification
Environment
gpt-5.4session snapshot in Control UISteps
inputTokensgreatly exceed the freshtotalTokenssnapshot after long tool loops./usage.Expected
Actual
/usagereport context usage from freshtotalTokenswhen availableEvidence
Targeted verification run locally:
pnpm exec vitest ui/src/ui/views/chat.test.ts --run->15/15passedpnpm exec vitest --config vitest.config.ts --project unit-node src/ui/chat/slash-command-executor.node.test.ts --run->14/14passedagent:warden:main,.context-noticecount was0, and a style probe reported.context-notice__iconat16px x 16pxHuman Verification
/usageinputTokenswhentotalTokensFreshis false; legacymainalias handling for/usagepnpm build && pnpm check && pnpm test; screenshot attachments were not added to this PR formReview Conversations
Compatibility / Migration
YesNoNoFailure Recovery
Risks and Mitigations
inputTokenswhentotalTokensFresh === falsemainvsagent:warden:main) is not addressed in this PRAI disclosure
AI-assisted change. Final code and targeted verification were reviewed locally.