Skip to content

fix(desktop): show unknown context usage after compaction - #5548

Open
faga295 wants to merge 5 commits into
apache:mainfrom
faga295:fix/compact_context_show_unknown
Open

faga295 wants to merge 5 commits into
apache:mainfrom
faga295:fix/compact_context_show_unknown

Conversation

@faga295

@faga295 faga295 commented Sep 20, 2026 •

Copy link
Copy Markdown
Contributor

Summary

After successful compaction, the context gauge displays its existing gauge icon with the localized Usage / 用量 label instead of retaining the pre-compaction percentage. The tooltip explains that usage will update when the next request completes (简体中文:上下文已压缩,用量将在下一次请求完成后更新。), with matching Traditional Chinese and English copy. A newer provider measurement restores the reading, including during an ongoing turn; failed-open compaction preserves valid usage.

Carry explicit unavailable/measured/stale readings through the conversation and WorkHub composers, and use request completion timestamps to reject stale live snapshots. Keep measured tokens paired with their metered context window.

Fixes #5547

Before:
image

After:
image

Verification

Earlier validation recorded for the initial implementation (before the label and tooltip refinement):

  • npm run lint
  • npm run format:check
  • npm run build
  • npm run typecheck
  • npx knip --workspace apps/desktop
  • npx knip --workspace packages/ui
  • git diff --check upstream/main...HEAD
  • node --test apps/desktop/dist/main/__tests__/latest-request-usage.test.js apps/desktop/dist/main/__tests__/live-context-usage.test.js packages/ui/dist/__tests__/composer-context-usage.test.js — 35 tests passed, 0 failed.

Validation for the label and tooltip refinement:

  • UI TypeScript build passed.
  • Biome checks passed for all three changed files.
  • Composer context usage and conversation copy suites passed: 8 tests, 0 failures.
  • git diff --check passed.

Component assertions verify that both stale and unavailable usage render Usage, a subsequent measured reading restores 10%, and trace opening and window precedence still work. No manual UI screenshot/recording or full workspace test-suite run was performed in this submission. Tests were not rerun against the unmodified base.

AI use

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: OpenAI Codex inspected the existing implementation, updated the post-compaction label and localized tooltips, adjusted the component assertion, ran the validation described above, and updated this PR and the linked issue. Earlier implementation tooling provenance was not independently verified.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes — described under Summary above
  • No

@github-actions github-actions Bot added the effort/L Under 1000 readable lines label Sep 20, 2026
@faga295
faga295 force-pushed the fix/compact_context_show_unknown branch from b48ac9f to e02ab01 Compare September 22, 2026 09:44

@hqhq1025 hqhq1025 left a comment

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.

Reviewed the current head e02ab01af442d7cd0e8589928f5dbe5cf72f0f00 (14 files, +416/−147). The latest-usage selector walks the transcript backwards for a matching request anchor or context_compacted note, and accepts live diagnostics only when completedAt is strictly later than that note (apps/desktop/src/renderer/application/contracts/session-inspector/latest-request-usage.ts:41-110). Desktop carries the Host settlement time through (apps/desktop/src/renderer/application/contracts/session-inspector/live-context-usage.ts:63-90); Composer and WorkHub use the unknown/stale projection. I checked the zero/unknown, routing, compaction, and late-diagnostics regression cases (apps/desktop/src/main/__tests__/latest-request-usage.test.ts:75-259). I found no substantiated P0–P3 code issue in this review.

The current test check passes, but this branch conflicts with current main in apps/desktop/src/renderer/app-shell.tsx; it is not ready to merge. Please resolve the conflict and revalidate the resulting head. I did not run local tests or Electron E2E (Node 18/no installed dependencies), and have not verified real Host/transcript timestamp causality. This is not a merge approval.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@faga295
faga295 force-pushed the fix/compact_context_show_unknown branch 2 times, most recently from 778663b to b7bd38a Compare September 27, 2026 15:03

@hqhq1025 hqhq1025 left a comment

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.

I re-reviewed the current 14-file PR at 9fd2536fd0c7d2181e59630bcd95e7fa5977004d, including the resolved AppShell integration and the latest completedAt assertion. The selector treats a context_compacted transcript note as a boundary, and Composer/WorkHub now share the measured/stale/unavailable projection. I found one P3 ordering issue, detailed inline: an older live diagnostics snapshot can override a newer post-compaction request anchor while the debounced diagnostics refresh is pending or fails. The current tests cover the boundary against an older snapshot, but not a newer anchor against an older snapshot.

The current-head test check is in progress. GitHub reports the PR mergeable with base c0787020 (the current main at review time); the PR diff passes git diff --check. I did not run tests locally (Node 18/no installed dependencies), Electron E2E, or verify real Host/transcript timestamp causality. There is no storage schema migration in this change. This is not merge approval.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the latest review on this head found no blocking issues, and it now merges cleanly.

One thing we need before merging: the AI-use section says OpenAI Codex handled the later refinements, but also that "earlier implementation tooling provenance was not independently verified", and none of the four commits carries a Generated-by trailer.

Could you confirm which generative tool(s), if any, wrote the initial implementation (cb7450f9 and de8b02bd)? Per CONTRIBUTING.md, the contributor of record owns the provenance of the change, and each commit with material AI-authored content needs a Generated-by: <tool> trailer. You don't need to rewrite the commits — once you confirm, we'll carry the trailers into the squash commit. Please also note that contributions produced with Grok models can't be accepted in this project.

This comment was drafted with automated assistance and posted by a maintainer-side reviewer.

@faga295
faga295 force-pushed the fix/compact_context_show_unknown branch from 9fd2536 to 9a3066a Compare September 27, 2026 16:32
@faga295

faga295 commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

@hqhq1025 @Astro-Han Thanks for the review, I’ve addressed all comments, and all commits follow the CONTRIBUTING.md.

@hqhq1025 hqhq1025 left a comment

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.

Reviewed the current head 9a3066aece6e4c1a258ff492973218cac28dc884 (14 changed files, +460/−148). The follow-up preserves the post-compaction anchor timestamp, so an anchor can replace a retained pre-compaction diagnostics snapshot; the added regression covers that case. I found one P2 in the new ordering rule: for an ordinary completed request, the transcript anchor is written after the diagnostics completedAt, so the resolver also replaces the current per-request snapshot with a different input + output measurement and drops its metered window. This makes the composer gauge inaccurate in the common path. Please fix the causal comparison and add a same-request regression before merging.

The current-head test check passed; the PR is mergeable, though its merge state is blocked. I did not run local suites or an Electron UI session. The PR discloses Codex for this follow-up; the earlier implementation commit has no tool trailer, and its tooling provenance remains unverified.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@faga295
faga295 force-pushed the fix/compact_context_show_unknown branch 2 times, most recently from 15462f9 to 7739e11 Compare September 27, 2026 17:08

@hqhq1025 hqhq1025 left a comment

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.

I re-reviewed head 7739e117eb43178d262f28068e8e72a32b876fa4, focusing on the nine-file follow-up and the earlier context-usage paths. The previous same-request timestamp finding is addressed: the anchor now carries the provider request's settlement time, not the later transcript row's write time. The runtime obtains it from the completed main attempt, the core validator accepts the optional field, and the renderer uses it to keep an equal-time diagnostics snapshot and its metered window. The added same-request test distinguishes this fix from the prior head; the post-compaction/new-anchor case remains covered. The optional field preserves legacy record decoding, and the Host compatibility epoch advances from 195 to 196 for the changed closed transcript shape. I found no substantiated new P0–P3 finding in this scope.

At publication, the current-head test check is still in progress. The PR is open/mergeable; the merge-tree against current main and git diff --check are clean. I did not run local suites (this checkout has Node 18 and no dependencies), a real Electron UI, or a cross-version Client–Host session. This is not merge approval.

Automated review notice: This comment was posted by an automated review agent operated by hqhq1025. It is not an independent human review and does not replace one.

@Astro-Han Astro-Han left a comment

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.

Independent second review of head 7739e117 (different model lineage from the earlier review on this head).

The main paths look right. The pre-turn fold, manual /compact, the failed-open path, and the equal-timestamp case for a same-request anchor and snapshot all behave as intended. The previous P2, the ordering between a same-request anchor and its snapshot, is fixed. Compat epoch 196 is the correct next value (origin/main is at 195), and it is needed because the anchor schema check is strict. Old anchors without completedAt still decode. WorkHub and the conversation composer share the same resolver.

P2: a stale pre-compaction reading can still show as measured, which is the #5547 symptom. At settlement, ai-sdk-turn.ts:2384-2391 appends the context_compacted note before the token_usage row. The row's lastRequestAnchor (tokens and completedAt) only advances on a successful request. So in a multi-step turn like this one:

  1. step 1 succeeds (T1);
  2. step 2 hits context overflow;
  3. recovery compaction succeeds;
  4. the retry overflows again and the turn ends with an error.

The transcript then gets note(T3) followed by token_usage(anchor.completedAt=T1). selectLatestRequestUsage scans backwards, hits the token row first and returns {kind:'tokens', at:T1}, and never sees the newer note. Replaying this sequence through the compiled selector and resolver yields {kind:'measured', tokens:90000} instead of stale. No test covers the order "note before the row, but the anchor is older than the note". Possible fixes:

  • in the selector, keep scanning for a same-turnId context_compacted note whose ts is later than anchor.completedAt; or
  • at runtime, omit the anchor when a fold happened after the last successful request.

P3 (non-blocking):

  • CHANGELOG line 49 says the gauge renders ?. It actually renders the localized "Usage"/"用量" label, so stale and unavailable look identical except for the tooltip.
  • A mid-turn fold's note is only written at settlement, so the gauge never shows the compacted state while that turn is still running. The CHANGELOG wording about mid-turn recovery reads broader than that. This is a pre-existing runtime limitation.
  • Between the note append and the token_usage append, the gauge briefly flips to compacted. If the token row write fails, that state persists until the next turn.
  • The composer tests don't assert the stale tooltip copy, so stale and unavailable aren't distinguished in tests.

Verified: npm run build:test and the 6 touched test files pass locally. Not verified: that the note ts and the telemetry now() share a clock (both are Host-side), whether a user Stop skips settlement, a runtime end-to-end replay of the P2 sequence, and manual UI behaviour.


Automated review (Claude lineage) by the Qronos review line on behalf of @Astro-Han; findings were checked against the code but please verify before acting.

return {
kind: 'tokens',
tokens: anchor.inputTokens + output,
// The row is persisted after request settlement (and sometimes after a

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: this returns on the first anchored token_usage row. However, settlement writes the context_compacted note before this row, and the anchor's completedAt can predate that note when the fold happened after the last successful request, for example when a post-fold retry fails. In that case the newer compaction is skipped and the pre-fold count shows as measured. Consider continuing the scan for a same-turn note with ts > anchor.completedAt, or not writing the anchor at runtime in that case.

Comment thread CHANGELOG.md Outdated
count is stale rather than current — showing it read as a live measurement of
what the session was about to send. The gauge walks the transcript backwards and
whichever fact comes first decides: a `context_compacted` note newer than every
measurement renders `?` with a tooltip saying why, and a measurement newer than

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.

P3: the gauge now renders the localized Usage / 用量 label rather than ?.

@me2seeks me2seeks left a comment

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.

Automated review notice: This comment was posted by an automated review agent operated by me2seeks make. It is not an independent human review and does not replace one.

Summary

After compaction, the context gauge stops showing the pre-fold percentage and renders an explicit unavailable/stale reading until a post-fold measurement lands, with typed unavailable/measured/stale readings carried through both composers and settlement timestamps used to order live snapshots against the compaction boundary. The issue is real (base kept the last anchor indefinitely). The direction is sound: ledger order decides transcript state, timestamps only arbitrate the live snapshot, failed-open folds correctly don't count as boundaries, and the epoch bump 195→196 for the new anchor field is correct protocol hygiene. ContextDiagnosticsResult.completedAt exists (packages/runtime-host/src/protocol/context.ts:83), so the live path is well-typed.

Findings

  1. [P3] CHANGELOG.md claims the post-compaction gauge renders ?, but the shipped implementation renders the gauge icon with the localized Usage/用量 label and a tooltip (packages/ui/src/composer.tsx ContextUsageAction; the PR body itself says so). User-facing doc contradicts the code.
  2. [P3] resolveContextUsage compares the compaction note's write ts against the live snapshot's completedAt (apps/desktop/src/renderer/application/contracts/session-inspector/latest-request-usage.ts:83-90); the code comment itself concedes "the note may be recorded later than the actual fold". A late-written note suppresses a genuinely post-fold live measurement as stale until the next anchored usage row lands. Ledger order in selectLatestRequestUsage makes this window narrow, but the timestamp is the weaker of the two signals.
  3. [P3] PR is mergeable:CONFLICTING, CI was pending at snapshot time, and the body states tests were "not rerun against the unmodified base" with the "tests fail without it" box unchecked — the mutation evidence is asserted, not shown, for the final revision.

Verdict

needs-changes — conflicts must be resolved and the CHANGELOG/code mismatch fixed; core logic is otherwise sound.

@faga295
faga295 force-pushed the fix/compact_context_show_unknown branch from 7739e11 to 3c45c23 Compare October 4, 2026 17:27
liuzhaochen03 added 5 commits October 5, 2026 01:51
Treat measurements taken before a successful compaction as stale until a newer request settles. Update the composer display, localized tooltip, tests, and changelog.

Generated-by: Codex
Preserve the selected request anchor timestamp so the context usage resolver can order it against a retained diagnostics snapshot. Cover the post-compaction sequence in resolver tests.

Generated-by: Codex
Persist the provider request completion time on usage anchors so a later transcript write cannot displace the same request snapshot and its metered window. Preserve legacy anchors and bump the runtime-host compatibility epoch for the new field.

Generated-by: Codex
The stale reading keeps the gauge chip on its localized Usage label with the compacted-context tooltip; it never rendered a question mark. Align the entry with the shipped ContextUsageAction.

Generated-by: Codex
Describe what resolveContextUsage consumes (live Turn snapshot and durable transcript anchors/notes) and the ordering rule it applies after a compaction.

Generated-by: Codex
@faga295
faga295 force-pushed the fix/compact_context_show_unknown branch from 3c45c23 to 0208b53 Compare October 4, 2026 17:53

@Astro-Han Astro-Han left a comment

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.

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review: 7739e117 (our last reviewed head) to 0208b53f (5 commits, 23 files, +545/-159). The PR was rebased from merge-base a8b8d503 onto dcef6427, which is current main. Comparing (old head vs its merge-base) with (new head vs its merge-base), the only substantive changes are:

  • 0d0ada18 rewrites the CHANGELOG entry so it describes the localized Usage label rather than ?. This fixes the earlier P3.
  • 0208b53f rewrites the doc comment on resolveContextUsage. There is no logic change.
  • The rebase adapts the renderer to the Composer reads that moved below the shell on main. conversation-workspace.ts now memoizes the object-valued selectLatestRequestUsage result per messages identity, so useSyncExternalStore keeps a stable snapshot. conversation-readers.tsx and conversation-owner.test.ts now carry latestRequestUsage. These look correct.
  • The compat epoch moves to 205. That is correct, because main is at 204.

The selector, resolver, runtime anchor stamping and telemetry are byte-identical to 7739e117 apart from that doc comment.

Prior findings

  • hqhq1025 P2 (same-request anchor vs snapshot ordering) and P3 (a pre-fold live snapshot overriding a post-fold anchor): fixed, as already confirmed at 7739e117.
  • Astro-Han P2 (a stale reading shows as measured when the fold postdates the anchor): not fixed. The code is unchanged; see the inline comment.
  • P3 CHANGELOG ?: fixed.
  • P3 stale tooltip not asserted: still open. composer-context-usage.test.tsx:145 only checks the Usage label, so a stale reading and an unavailable reading are indistinguishable in tests.
  • P3 note/row write gap and the CHANGELOG wording about mid-turn recovery: unchanged, and still non-blocking.

One P2 (carried over). No new findings in the delta.

CI test passes on this head. GitHub reports MERGEABLE (merge state BLOCKED). git merge-tree against main and git diff --check are both clean. Not run locally: the test suites (CI covers them on this head), a real Electron UI, and an end-to-end runtime replay of the P2 sequence. I traced that sequence by hand through the unchanged selector and resolver. This is not merge approval.

if (!Number.isFinite(anchor.inputTokens) || anchor.inputTokens <= 0) return undefined;
const output = Number.isFinite(anchor.outputTokens ?? 0) ? Math.max(0, anchor.outputTokens ?? 0) : 0;
return anchor.inputTokens + output;
return {

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 (carried over from the 7739e117 review, still unfixed): a fold that postdates the anchor still renders as measured, which is the #5547 symptom. Settlement appends the context_compacted note before the token_usage row (packages/runtime/src/ai-sdk-turn.ts:2416-2418). The row's anchor tokens come from lastStepInputTokens, which only advances when a step finishes (ai-sdk-turn.ts:1922), and its completedAt comes from the last completed main request. Settlement also persists usage before it rethrows terminalProviderError (:2432). Take this sequence:

  1. step 1 succeeds at T1;
  2. step 2 overflows;
  3. recovery compaction succeeds;
  4. the retry fails.

The ledger then ends with note(ts=T3) followed by token_usage(anchor.completedAt=T1). This loop scans backwards, hits the token row first and returns {kind:'tokens', at:T1} without ever seeing the newer note. resolveContextUsage then yields measured, either from the anchor or from the T1 live snapshot. Neither is stale.

Suggested fix: when the anchor carries completedAt, keep scanning for a context_compacted note with ts > anchor.completedAt (same turnId), and return compacted in that case. A fix in the selector is preferable to dropping the anchor at runtime, because the runtime also uses that anchor for the next turn's proactive fold decision. Please add a resolver test for the order "note before row, anchor older than the note".

This branch has not been deployed

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

Labels

effort/L Under 1000 readable lines

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(desktop): context usage remains stale after compaction

4 participants