Skip to content

Align context usage display, refresh chat after compaction#66485

Open
0riginal-claw wants to merge 1 commit intoopenclaw:mainfrom
0riginal-claw:fix/context-usage-alignment
Open

Align context usage display, refresh chat after compaction#66485
0riginal-claw wants to merge 1 commit intoopenclaw:mainfrom
0riginal-claw:fix/context-usage-alignment

Conversation

@0riginal-claw
Copy link
Copy Markdown

…tion\n\n- /status percent now uses canonical totalTokens when fresh, matching chat context notice\n- Refresh chat history on compaction completion to clear stale pre-compaction tokens in UI\n\nFixes openclaw#66483
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Apr 14, 2026

Greptile Summary

This PR fixes two related display issues: the /status context percentage now uses totalTokens (matching renderContextNotice), and chat history is refreshed after a compaction completes so the UI reflects the compacted transcript and cleared token counters.

One minor inconsistency remains: the new totalTokensFresh === true guard in /status is stricter than the !== false check in renderContextNotice, so when totalTokensFresh is undefined the chat bar still shows a context warning but the /status context line is silently hidden.

Confidence Score: 5/5

  • Safe to merge; all findings are P2 style suggestions with no correctness or data-integrity impact.
  • Both changes are narrow, UI-only improvements. The compaction history refresh is idempotent, and the context-percentage fix uses a more accurate numerator. The two P2 comments (guard strictness vs. renderContextNotice, redundant try/catch) do not block merge.
  • No files require special attention.
Prompt To Fix All With AI
This is a comment left during a code review.
Path: ui/src/ui/chat/slash-command-executor.ts
Line: 384-387

Comment:
**`totalTokensFresh === true` is stricter than `renderContextNotice`**

`renderContextNotice` (in `views/chat.ts:533`) hides only when `totalTokensFresh === false`; it still renders the bar when the flag is `undefined`. The new guard here requires `totalTokensFresh === true`, so when the field is absent (e.g. older session rows) `/status` silently drops the context line while the chat view still shows it. Using `!== false` would produce behaviour that actually matches the notice.

```suggestion
    const pct =
      (session.totalTokensFresh !== false && typeof session.totalTokens === "number" && ctx > 0)
        ? Math.round(Math.min((session.totalTokens / ctx) * 100, 100))
        : null;
```

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/app-gateway.ts
Line: 445-457

Comment:
**`try/catch { // ignore }` is redundant here**

Every synchronous access inside the block is already guarded by optional chaining and explicit type-checks, so none of these lines can throw. The `void loadChatHistory(...)` call discards the promise, so any async rejection is already swallowed independently of the surrounding `try`. The catch block is dead code and the blanket suppression adds noise without protecting anything.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "UI: align context usage display with bac..." | Re-trigger Greptile

Comment on lines +384 to +387
const pct =
(session.totalTokensFresh === true && typeof session.totalTokens === "number" && ctx > 0)
? Math.round(Math.min((session.totalTokens / ctx) * 100, 100))
: null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 totalTokensFresh === true is stricter than renderContextNotice

renderContextNotice (in views/chat.ts:533) hides only when totalTokensFresh === false; it still renders the bar when the flag is undefined. The new guard here requires totalTokensFresh === true, so when the field is absent (e.g. older session rows) /status silently drops the context line while the chat view still shows it. Using !== false would produce behaviour that actually matches the notice.

Suggested change
const pct =
(session.totalTokensFresh === true && typeof session.totalTokens === "number" && ctx > 0)
? Math.round(Math.min((session.totalTokens / ctx) * 100, 100))
: null;
const pct =
(session.totalTokensFresh !== false && typeof session.totalTokens === "number" && ctx > 0)
? Math.round(Math.min((session.totalTokens / ctx) * 100, 100))
: null;
Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/ui/chat/slash-command-executor.ts
Line: 384-387

Comment:
**`totalTokensFresh === true` is stricter than `renderContextNotice`**

`renderContextNotice` (in `views/chat.ts:533`) hides only when `totalTokensFresh === false`; it still renders the bar when the flag is `undefined`. The new guard here requires `totalTokensFresh === true`, so when the field is absent (e.g. older session rows) `/status` silently drops the context line while the chat view still shows it. Using `!== false` would produce behaviour that actually matches the notice.

```suggestion
    const pct =
      (session.totalTokensFresh !== false && typeof session.totalTokens === "number" && ctx > 0)
        ? Math.round(Math.min((session.totalTokens / ctx) * 100, 100))
        : null;
```

How can I resolve this? If you propose a fix, please make it concise.

Comment thread ui/src/ui/app-gateway.ts
Comment on lines +445 to +457
try {
const data = (payload?.data ?? {}) as Record<string, unknown>;
const phase = typeof data.phase === "string" ? data.phase : "";
const completed = data.completed === true;
const sessionKey = typeof payload?.sessionKey === "string" ? payload.sessionKey : undefined;
if (payload?.stream === "compaction" && phase === "end" && completed) {
if (!sessionKey || sessionKey === host.sessionKey) {
void loadChatHistory(host as unknown as ChatState);
}
}
} catch {
// ignore
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 try/catch { // ignore } is redundant here

Every synchronous access inside the block is already guarded by optional chaining and explicit type-checks, so none of these lines can throw. The void loadChatHistory(...) call discards the promise, so any async rejection is already swallowed independently of the surrounding try. The catch block is dead code and the blanket suppression adds noise without protecting anything.

Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/ui/app-gateway.ts
Line: 445-457

Comment:
**`try/catch { // ignore }` is redundant here**

Every synchronous access inside the block is already guarded by optional chaining and explicit type-checks, so none of these lines can throw. The `void loadChatHistory(...)` call discards the promise, so any async rejection is already swallowed independently of the surrounding `try`. The catch block is dead code and the blanket suppression adds noise without protecting anything.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9652519c38

ℹ️ 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".

Comment on lines +385 to +386
(session.totalTokensFresh === true && typeof session.totalTokens === "number" && ctx > 0)
? Math.round(Math.min((session.totalTokens / ctx) * 100, 100))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle undefined totalTokensFresh as fresh in /usage

The new guard requires totalTokensFresh === true before showing context usage, but this codebase treats only an explicit false as stale (for example, resolveFreshSessionTotalTokens and chat context rendering both accept missing freshness as usable). With this change, sessions that have valid totalTokens but omit totalTokensFresh (older gateway payloads or compatibility-shaped rows) lose the context percentage line entirely, which regresses /usage output even when the data is otherwise fresh enough to display.

Useful? React with 👍 / 👎.

Comment thread ui/src/ui/app-gateway.ts
Comment on lines +451 to +452
if (!sessionKey || sessionKey === host.sessionKey) {
void loadChatHistory(host as unknown as ChatState);
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Ignore sessionless compaction events from unrelated runs

This condition reloads chat history whenever a compaction-end event has no sessionKey, which means an unrelated sessionless compaction event can trigger loadChatHistory for the currently open chat. Because loadChatHistory clears streaming/tool state, that can interrupt an active conversation in the visible session even though the compaction belonged to another run.

Useful? React with 👍 / 👎.

@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented Apr 28, 2026

Codex review: needs changes before merge.

What this changes:

The PR changes Control UI /usage context percentage calculation and adds an agent compaction-complete handler that reloads chat history.

Required change before merge:

This is a narrow Control UI repair with concrete failing review findings, clear files, and targeted validation; it does not require product or security approval first.

Security review:

Security review cleared: The PR only changes Control UI event handling and slash-command display code; it adds no dependencies, scripts, workflows, permissions, package-resolution changes, or secret-handling paths.

Review findings:

  • [P2] Handle missing freshness as usable in /usage — ui/src/ui/chat/slash-command-executor.ts:384-387
  • [P2] Scope compaction reloads to owned events — ui/src/ui/app-gateway.ts:451-453
Review details

Best possible solution:

Preserve current main’s /usage snapshot semantics and add a compaction-complete history refresh only when the event is clearly owned by the active session or active run, with targeted tests for matching, unrelated, and missing-sessionKey cases.

Do we have a high-confidence way to reproduce the issue?

Yes. A static event-path reproduction is clear: a session row with valid totalTokens but omitted totalTokensFresh loses the PR’s context line, and a completed compaction event without sessionKey can trigger loadChatHistory for the visible chat.

Is this the best way to solve the issue?

No. The direction is useful, but the implementation should keep the shipped /usage logic and scope compaction reloads to owned events instead of accepting all sessionless compaction completions.

Full review comments:

  • [P2] Handle missing freshness as usable in /usage — ui/src/ui/chat/slash-command-executor.ts:384-387
    The new guard requires totalTokensFresh === true, but current UI semantics treat only explicit false as stale. Older or compatibility-shaped session rows with valid totalTokens and no freshness flag would lose the context percentage even though main and the chat notice still display it.
    Confidence: 0.91
  • [P2] Scope compaction reloads to owned events — ui/src/ui/app-gateway.ts:451-453
    The !sessionKey branch reloads the visible chat for any sessionless completed compaction event. Because loadChatHistory clears streaming/tool state, an unrelated compaction can interrupt the active session; reload only when the event matches the active session or active run.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.88

Acceptance criteria:

  • pnpm test ui/src/ui/app-gateway.node.test.ts ui/src/ui/chat/slash-command-executor.node.test.ts
  • pnpm exec oxfmt --check --threads=1 ui/src/ui/app-gateway.ts ui/src/ui/chat/slash-command-executor.ts ui/src/ui/app-gateway.node.test.ts ui/src/ui/chat/slash-command-executor.node.test.ts
  • pnpm check:changed

What I checked:

  • Current main keeps missing freshness usable for /usage: /usage computes context from contextSnapshotTotal and treats only totalTokensFresh === false as stale, so omitted freshness still displays a valid context percentage. (ui/src/ui/chat/slash-command-executor.ts:385, a256745323ab)
  • Current main has regression coverage for snapshot-based /usage: The node test expects Context: 31% from totalTokens: 1250 over contextTokens: 4000 while preserving the cumulative total display. (ui/src/ui/chat/slash-command-executor.node.test.ts:528, a256745323ab)
  • Current main does not reload history from compaction agent events: The agent event branch still delegates to handleAgentEvent(...) and returns without a compaction-complete loadChatHistory(...) path. (ui/src/ui/app-gateway.ts:675, a256745323ab)
  • Main's existing reload path is session-scoped and active-run aware: session.message reloads only when the payload session key matches the active session and defers while a chat run is active, which is the safer ownership pattern for the compaction refresh. (ui/src/ui/app-gateway.ts:643, a256745323ab)
  • History reload clears streaming state: loadChatHistory resets tool-stream state and clears chatStream, so an unrelated reload can visibly disrupt an active conversation. (ui/src/ui/controllers/chat.ts:433, a256745323ab)
  • PR diff accepts sessionless compaction reloads: The PR reloads when !sessionKey || sessionKey === host.sessionKey, which allows any sessionless completed compaction event to refresh the active chat. (ui/src/ui/app-gateway.ts:451, 9652519c38b0)

Likely related people:

  • BunsDev: Merged related Control UI context freshness work in PR Fix Control UI context freshness and compaction CTA #71297, and that PR explicitly called this PR related but outside its scope. (role: recent adjacent maintainer; confidence: medium; commits: 7a3f5d597c3a; files: ui/src/ui/app-gateway.ts, ui/src/ui/chat/context-notice.ts, ui/src/ui/controllers/sessions.ts)
  • Peter Steinberger: Local current-line blame in this grafted checkout attributes the touched Control UI files to the current main snapshot; this is useful for routing but not strong feature-introduction evidence. (role: current-main maintainer; confidence: low; commits: 9300d48244ca, a256745323ab; files: ui/src/ui/app-gateway.ts, ui/src/ui/chat/slash-command-executor.ts)

Codex review notes: model gpt-5.5, reasoning high; reviewed against a256745323ab.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant