fix(status): avoid misleading context usage when totalTokensFresh=false#45270
fix(status): avoid misleading context usage when totalTokensFresh=false#45270xyooz wants to merge 2 commits intoopenclaw:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adjusts /status context rendering so it doesn’t display misleading “context used” values when the session’s stored token total is known to be stale (totalTokensFresh=false). Instead, it shows unknown context occupancy unless a fresh total is available (including via transcript-derived usage).
Changes:
- Stop deriving
totalTokensfrominputTokens + outputTokenswhen the stored total is explicitly stale (and generally avoid synthesizing totals for status). - Preserve transcript-usage override behavior and treat transcript-derived totals as fresh for display purposes.
- Add a regression test ensuring stale totals render as unknown context usage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/auto-reply/status.ts | Prevents status from displaying synthesized/overstated context usage when totals are stale; keeps transcript override as the source of truth when enabled. |
| src/auto-reply/status.test.ts | Adds coverage for totalTokensFresh=false to ensure /status shows ?/context instead of a synthetic total. |
Greptile SummaryThis PR fixes a display correctness bug in Key changes:
Confidence Score: 5/5
Prompt To Fix All With AIThis is a comment left during a code review.
Path: src/auto-reply/status.ts
Line: 482
Comment:
**Redundant `!totalTokensFresh` guard**
`totalTokensFresh` is defined on line 468 as `typeof totalTokens === "number"`, so it is `false` if and only if `totalTokens` is `undefined`. In that case `!totalTokens` is already truthy (since `undefined` is falsy), making `!totalTokensFresh` redundant in this condition.
The only case where `totalTokens` is a defined value but conceptually "not fresh" is `totalTokens === 0`, which is already covered by the `totalTokens === 0` clause.
This doesn't cause a bug, but the extra clause adds noise. Consider removing it to keep the intent clear:
```suggestion
if (!totalTokens || totalTokens === 0 || candidate > totalTokens) {
```
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/auto-reply/status.test.ts
Line: 634-656
Comment:
**Consider adding a test for explicit stale `totalTokens`**
This test covers the case where `totalTokens` is absent from the entry and `totalTokensFresh` is false, which is the original bug scenario. A complementary test where `totalTokens` is explicitly set to a large value *and* marked stale would give fuller coverage of the fix logic at line 464–467 (both branches of the ternary are exercised). The expected result would still be `"Context: ?/32k"` since a stale stored total should be ignored whether or not it is present in the entry.
How can I resolve this? If you propose a fix, please make it concise.Last reviewed commit: d5ddc75 |
| if (logUsage) { | ||
| const candidate = logUsage.promptTokens || logUsage.total; | ||
| if (!totalTokens || totalTokens === 0 || candidate > totalTokens) { | ||
| if (!totalTokens || !totalTokensFresh || totalTokens === 0 || candidate > totalTokens) { |
There was a problem hiding this comment.
Redundant !totalTokensFresh guard
totalTokensFresh is defined on line 468 as typeof totalTokens === "number", so it is false if and only if totalTokens is undefined. In that case !totalTokens is already truthy (since undefined is falsy), making !totalTokensFresh redundant in this condition.
The only case where totalTokens is a defined value but conceptually "not fresh" is totalTokens === 0, which is already covered by the totalTokens === 0 clause.
This doesn't cause a bug, but the extra clause adds noise. Consider removing it to keep the intent clear:
| if (!totalTokens || !totalTokensFresh || totalTokens === 0 || candidate > totalTokens) { | |
| if (!totalTokens || totalTokens === 0 || candidate > totalTokens) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auto-reply/status.ts
Line: 482
Comment:
**Redundant `!totalTokensFresh` guard**
`totalTokensFresh` is defined on line 468 as `typeof totalTokens === "number"`, so it is `false` if and only if `totalTokens` is `undefined`. In that case `!totalTokens` is already truthy (since `undefined` is falsy), making `!totalTokensFresh` redundant in this condition.
The only case where `totalTokens` is a defined value but conceptually "not fresh" is `totalTokens === 0`, which is already covered by the `totalTokens === 0` clause.
This doesn't cause a bug, but the extra clause adds noise. Consider removing it to keep the intent clear:
```suggestion
if (!totalTokens || totalTokens === 0 || candidate > totalTokens) {
```
How can I resolve this? If you propose a fix, please make it concise.| it("does not infer context usage from input/output when totalTokens is stale", () => { | ||
| const text = buildStatusMessage({ | ||
| agent: { | ||
| model: "anthropic/claude-opus-4-5", | ||
| contextTokens: 32_000, | ||
| }, | ||
| sessionEntry: { | ||
| sessionId: "stale-total", | ||
| updatedAt: 0, | ||
| inputTokens: 298_437, | ||
| outputTokens: 147, | ||
| totalTokensFresh: false, | ||
| contextTokens: 32_000, | ||
| }, | ||
| sessionKey: "agent:main:main", | ||
| sessionScope: "per-sender", | ||
| queue: { mode: "collect", depth: 0 }, | ||
| includeTranscriptUsage: false, | ||
| modelAuth: "api-key", | ||
| }); | ||
|
|
||
| expect(normalizeTestText(text)).toContain("Context: ?/32k"); | ||
| }); |
There was a problem hiding this comment.
Consider adding a test for explicit stale totalTokens
This test covers the case where totalTokens is absent from the entry and totalTokensFresh is false, which is the original bug scenario. A complementary test where totalTokens is explicitly set to a large value and marked stale would give fuller coverage of the fix logic at line 464–467 (both branches of the ternary are exercised). The expected result would still be "Context: ?/32k" since a stale stored total should be ignored whether or not it is present in the entry.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/auto-reply/status.test.ts
Line: 634-656
Comment:
**Consider adding a test for explicit stale `totalTokens`**
This test covers the case where `totalTokens` is absent from the entry and `totalTokensFresh` is false, which is the original bug scenario. A complementary test where `totalTokens` is explicitly set to a large value *and* marked stale would give fuller coverage of the fix logic at line 464–467 (both branches of the ternary are exercised). The expected result would still be `"Context: ?/32k"` since a stale stored total should be ignored whether or not it is present in the entry.
How can I resolve this? If you propose a fix, please make it concise.|
Addressed review suggestions in a follow-up commit:\n\n- Removed the redundant |
|
This pull request has been automatically marked as stale due to inactivity. |
|
Thanks for the context here. I did a careful shell check against current Close this PR as implemented on current main. The stale So I’m closing this as already implemented rather than keeping a duplicate issue open. Review detailsBest possible solution: Keep the current-main implementation in Security review: Security review cleared: The PR only changes status rendering and tests, with no dependency, CI, secret, install, release, or code execution surface changes. What I checked:
Likely related people:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 6e5a703dd2db; fix evidence: commit e5dc0e6d15ea, main fix timestamp 2026-04-29T04:02:04Z. |
Summary
Fix status context rendering when session token totals are stale.
When
totalTokensFresh=false, status previously fell back toinputTokens + outputTokens, which can overstate context usage and report misleading values like299k/200k (149%).This change avoids that fallback for stale totals and reports unknown context occupancy unless fresh totals (or transcript usage) are available.
Changes
src/auto-reply/status.tstotalTokensfrominput+outputwhentotalTokensFreshis explicitly false.src/auto-reply/status.test.tstotalTokensFresh=falseto ensure status shows unknown context usage instead of syntheticinput+outputusage.User impact
/statuscontext line semantically accurate in stale-token states.Validation
pnpm vitest run src/auto-reply/status.test.tsNotes
This is a display/accounting correctness fix; it does not alter compaction trigger behavior itself.
Fixes #45268