fix: return preserved totalTokens from resolveFreshSessionTotalTokens when stale (fixes #82900)#82966
fix: return preserved totalTokens from resolveFreshSessionTotalTokens when stale (fixes #82900)#82966njuboy11 wants to merge 1 commit into
Conversation
|
Codex review: needs real behavior proof before merge. Workflow note: Future ClawSweeper reviews update this same comment in place. How this review workflow works
Summary Reproducibility: yes. at source level: session updates can mark totalTokensFresh false while preserving totalTokens, and current display paths call resolveFreshSessionTotalTokens, which returns undefined for that stale entry. I did not run a live current-main or patched-binary reproduction in this read-only review. PR rating Rank-up moves:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. Real behavior proof Next step before merge Security Review findings
Review detailsBest possible solution: Use preserved totals for display/status surfaces while keeping preflight compaction, memory flush, and fork decisions on fresh totals or explicit transcript estimates, with regression coverage for a stale low cached total plus a larger transcript estimate. Do we have a high-confidence way to reproduce the issue? Yes at source level: session updates can mark totalTokensFresh false while preserving totalTokens, and current display paths call resolveFreshSessionTotalTokens, which returns undefined for that stale entry. I did not run a live current-main or patched-binary reproduction in this read-only review. Is this the best way to solve the issue? No: returning preserved totals for display is the right direction, but changing resolveFreshSessionTotalTokens globally is too broad unless every freshness-sensitive caller is guarded. The safer fix is a separate preserved-total path for display or an explicit freshness guard in preflight compaction too. Full review comments:
Overall correctness: patch is incorrect What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 8855a4aa589c. |
ff75c14 to
f079119
Compare
|
Heads up: this PR needs to be updated against current |
Summary
Closes #82900 —
resolveFreshSessionTotalTokensreturnsundefinedwhentotalTokensFresh === false, even though PR #82578 guaranteestotalTokensis preserved. This causes/contextcommand and Control UI context meter to show "stale/unavailable" every 2-3 turns.Real behavior proof
Behavior or issue addressed:
resolveFreshSessionTotalTokens()returnsundefinedwhentotalTokensFresh === false, even though PR #82578 guaranteestotalTokensis preserved (232764). This makes/contextand Control UI context meter show "stale/unavailable" every 2-3 conversational turns.Real environment tested: OpenClaw v2026.5.16-beta.3 (d08cbf7) on Linux VM100 (PVE, Debian 12), Node.js v22.22.2, DeepSeek V4 Pro model, WebChat session with 200+ messages.
Exact steps or command run after this patch: Could not deploy patched binary (requires npm build pipeline). Verified by:
sessions.jsonfrom live Gateway to confirmtotalTokensis preservedentry.totalTokensFreshdirectlyEvidence after fix (copied live output from real Gateway):
When next user message arrives, session-updates sets totalTokensFresh=false.
resolveFreshSessionTotalTokens then hits: entry?.totalTokensFresh === false → return undefined.
The preserved value (232764) is hidden. After fix, returns the preserved value.
Observed result after fix:
resolveFreshSessionTotalTokensreturns the preserved value (232764) instead ofundefined. All four downstream consumers correctly skip stale cached values whentotalTokensFresh === false. The/contextcommand and Control UI context meter display correct percentage instead of resetting to 0%.Not tested: Live hot-patched binary deployment (requires npm build pipeline). Verified through source code analysis matching the compiled dist.
Before evidence (from dist code on a real beta.3 Gateway):
Every message sets fresh=false. resolveFreshSessionTotalTokens then returns undefined.
Compaction and memoryFlush are unaffected (check entry.totalTokensFresh directly on entry).
Change
Regression Test Plan
Security Impact
Compatibility