Skip to content

fix(desktop): persist elapsed-timer across chat navigation - #62300

Closed
DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/desktop-elapsed-timer-persist
Closed

fix(desktop): persist elapsed-timer across chat navigation#62300
DavidMetcalfe wants to merge 1 commit into
NousResearch:mainfrom
DavidMetcalfe:fix/desktop-elapsed-timer-persist

Conversation

@DavidMetcalfe

@DavidMetcalfe DavidMetcalfe commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #62158 — the "time elapsed" counter on the chat loading/stall indicators resets to ~0s when the user navigates away from and back to the chat view, instead of continuing to count the in-flight turn.

Root cause: ResponseLoadingIndicator and StreamStallIndicator called useElapsedSeconds() with no key. The hook starts from Date.now() whenever no registry key is supplied, so each component remount (chat switch, Settings open/close, etc.) restarted the timer from zero.

Fix: Key both indicators on the running assistant message id (run:${id}) via the existing shared keyed useElapsedSeconds(timerKey) hook. The module-level startedAtByKey registry persists the start timestamp across unmount/remount, so the counter continues seamlessly. A new prompt produces a new message id → fresh counter.

runningAssistantMessageIdFromThread requires the last thread message to be an assistant message that is currently running (status.type === 'running'), so no stale timestamp is reused during the pre-append transition (last message = user prompt, or a prior completed turn's assistant message).

activity-timer.ts gets a 1000-entry LRU cap on the registry to bound memory across sessions/runs.

Notes

  • Desktop-only, no backend changes.
  • useThreadRuntime().runId does not exist in the installed @assistant-ui/react (^0.12.28) — verified against node_modules .d.ts. The running assistant message has a stable id in s.thread.messages from run start through streaming + stall (verified in @assistant-ui/core base-thread-runtime-core.js state getter reading mostRecentAssistantMessage). useAuiState(s => s.message.id) is the established pattern in the codebase.
  • Open question: assistant-ui appends the running assistant message as the last message when a turn starts; the status.type === 'running' guard is defensive against any gap where the loading indicator would otherwise key off a stale prior-turn id. If the runtime guarantees the running message is always last+running before loading === 'response', the guard is redundant but harmless.
  • Cross-vendor Antigravity review (Gemini 3.5 Flash + GPT-OSS) returned APPROVE-WITH-NITS; the one MAJOR it flagged (stale-timestamp transition) is fixed by the running guard above and locked by a regression test.

Test plan

  • apps/desktop/src/components/assistant-ui/thread/status.test.tsx (new, 4 tests): timer persists across remount of the same run (9s → 13s); resets on a new run id; does not reuse a stale timestamp when the last message is a user prompt; does not reuse a stale timestamp when the last message is a completed prior-turn assistant.
  • apps/desktop/src/components/chat/activity-timer.test.tsx (+2 tests): key-change reset; navigation away/back persistence.
  • vitest 7/7 pass; tsc -p . --noEmit clean on changed files.

…nning assistant message id)

The 'time elapsed' counter on the loading/stall indicators reset to ~0s when
navigating away/back from the chat view because ResponseLoadingIndicator and
StreamStallIndicator called useElapsedSeconds() without a key, so each remount
restarted the timer from Date.now().

Key both indicators on the running assistant message id (run:${id}) via the
shared keyed useElapsedSeconds hook. The module-level registry persists the
start time across unmount/remount. A new prompt (new message id) starts a fresh
counter.

runningAssistantMessageIdFromThread requires the last thread message to be an
assistant message that is currently running, so no stale timestamp is reused
during the pre-append transition (last message = user prompt or a prior
completed turn). activity-timer.ts gets a 1000-entry LRU cap on the registry.

Verified: vitest 7/7 (status.test.tsx + activity-timer.test.tsx), tsc clean.
Cross-vendor Antigravity review (Flash + GPT-OSS) -> APPROVE-WITH-NITS.

Fixes NousResearch#62158
@alt-glitch alt-glitch added type/bug Something isn't working comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have labels Jul 10, 2026

@teknium1 teknium1 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.

Thanks for tracing the remount behavior and covering both anonymous chat-status timer call sites. The underlying issue remains on current main: ResponseLoadingIndicator and StreamStallIndicator still call useElapsedSeconds without a key at apps/desktop/src/components/assistant-ui/thread/status.tsx:62,150.

Problems

  • apps/desktop/src/components/assistant-ui/thread/status.tsx:70 introduces runningAssistantMessageIdFromThread(), which calls useAuiState at line 71 but is not named as a custom hook. Desktop ESLint enables react-hooks/rules-of-hooks as an error at apps/desktop/eslint.config.mjs:99-100, so this should be renamed to useRunningAssistantMessageIdFromThread (or inlined into the component) before merge.

Suggested changes

  • Rename the helper as a custom hook and retain the current selector/guard behavior; the existing regression coverage can stay unchanged.

Automated hermes-sweeper review.

// turn count continuously, and a new prompt (new message id) resets the timer.
const RUN_KEY_PREFIX = 'run:'

function runningAssistantMessageIdFromThread(): string | undefined {

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.

useAuiState is called inside this function (line 71), but this lowercase name is neither a component nor a custom hook. apps/desktop/eslint.config.mjs:99-100 enables react-hooks/rules-of-hooks as an error. Rename this to useRunningAssistantMessageIdFromThread or move the selector into ResponseLoadingIndicator.

@teknium1 teknium1 added the sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users label Jul 11, 2026
@DavidMetcalfe

Copy link
Copy Markdown
Contributor Author

Closing as superseded — the fix landed on main through a different implementation of the same approach, and the issue this PR targets (#62158) was closed as fixed on 2026-08-15.

Same fix shape, different key source

This PR keyed useElapsedSeconds on the running assistant message id (run:${id}) so the module-level startedAtByKey registry carries the start timestamp across unmount/remount. Current main does the same thing but keys on the per-session turn clock:

Supersession timeline

The PR now conflicts with main after the status.tsx rework, which is consistent with the approach having been replaced wholesale.

Outstanding review item is moot

The hermes-sweeper keep_open review (@teknium1) asked to rename runningAssistantMessageIdFromThread to a custom hook (use… prefix) to satisfy react-hooks/rules-of-hooks. That helper's message-id approach was replaced by the turn-clock keying on main, so there is nothing left to rename.

What wasn't adopted from this PR

  • The 1000-entry LRU cap on startedAtByKey in apps/desktop/src/components/chat/activity-timer.ts. Minor memory-bound hygiene, not part of the reported bug — noting it here in case it's wanted as a follow-up.
  • The regression tests this PR added (remount persistence, new-run reset) have equivalent coverage on main: apps/desktop/src/components/assistant-ui/thread/status.test.tsx ("preserves each running session timer while switching between sessions").

Residual edge for anyone picking this up

StreamStallIndicator counts "quiet for Ns" from the last activity: quietSince is component-local state (status.tsx:214) and the timer is keyed on the turn clock only while compacting (status.tsx:239-242). Navigate away and back mid-pure-stall (no tool draft at the tail) and the stall timer re-qualifies after STREAM_STALL_S (2s) and counts from the remount, losing the pre-navigation quiet mark. The pre-first-token "thinking" counter — this issue's primary symptom — is fixed; the stall-phase remount edge is arguably by-design since stall semantics deliberately count from last activity (2f5926ed05), but if it should survive navigation, the fix is to move the quiet mark into session-scoped state the same way turnStartedAt was moved.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Desktop chat elapsed-time counter resets to ~1s when navigating away and back

3 participants