fix(sessions+compression+tool-card): stale unread dot, timer leak, tool card duplication (#3020, #2973)

[mysoul12138]

Summary

Three bugfixes, rebased onto current master (post-#3069):

• Session completion unread dot persists after navigating away from actively-viewed session #3020: Sync viewed-count in the polling path for actively-viewed sessions so navigating away doesn't show a stale unread dot. Defensive clear of completion-unread marker in _setSessionViewedCount.

• Auto-compression running state keeps timing until the whole session ends #2973: Clear elapsed-timer attributes and interval when a live compression card transitions from running to done, preventing the orphan timer from overwriting the completed card state. Guarded by active-session check to avoid killing a timer driving another session's display.

• Tool card duplication: Fix tool card header/detail showing identical content after tool completion. Route result to tc.snippet only, preserving tc.preview as the last streaming progress text.

What's NOT in this PR

• Cron sessions hidden from sidebar when 'Cron Jobs' project chip selected #3019 changes (dropped — handled by fix: show cron sessions in project filter #3069)
• ensure_cron_project system flag (may be a follow-up)
• CLI tool-result snippet 200→4000 bump (separate PR)
• test_pr_3019_3020_2973.py (tests need updating for new scope)

Test plan

• Manual: view session with new messages → navigate away → no stale dot
• Manual: trigger auto-compression → timer clears on completion
• Manual: run terminal tool → card header shows progress, detail shows result (no duplication)

🤖 Generated with Claude Code

[nesquena-hermes]

Review — three changes, mixed verdict

Pulled the PR worktree and read all three diffs against origin/master. Two of the three look good; the third has a regression risk I want to flag before merge.

#3020 (unread dot) — LGTM

The _setSessionViewedCount change in static/sessions.js:142-147:

counts[sid] = next;
_saveSessionViewedCounts();
// If the viewed count is now current, any prior completion-unread marker is
// stale — clear it so _hasUnreadForSession doesn't short-circuit (#3020).
_clearSessionCompletionUnread(sid);

…plus the _markPollingCompletionUnreadTransitions change at :407-419:

if (completedObservedStream || completedPersistedObservedStream || completedWithNewMessages) {
  if (!_isSessionActivelyViewedForList(sid)) {
    _markSessionCompletionUnread(sid, s.message_count);
  } else {
    // Sync viewed count so we don't flag stale unread on tab switch (#3020)
    _setSessionViewedCount(sid, messageCount);
  }
}

This matches the earlier triage prescription on #3020 exactly — sync the viewed count for actively-viewed sessions on every polling cycle, and defensively clear the completion-unread marker inside the count setter. Belt-and-braces, good.

#2973 (compression timer leak) — LGTM

The appendLiveCompressionCard patch at static/ui.js:5675-5689 clears data-compression-started-at and data-compression-message on the completion path, and conditionally calls _clearCompressionElapsedTimer() only when no other session has a running auto-compression:

const _activeCompState = _compressionStateForCurrentSession();
if (!_activeCompState || !_activeCompState.automatic || _activeCompState.phase !== 'running') {
  _clearCompressionElapsedTimer();
}

The active-session guard is important — _compressionElapsedTimer at static/ui.js:2392-2393 is module-global, not per-session, so unconditionally clearing it on every compressed event would kill the timer driving a different session's display. The guard is correct.

Tool-card duplication — please reconsider

Reading the live tool event flow:

• tool event at static/messages.js:1598 creates tc={name, preview:d.preview||'', args, snippet:'', done:false, tid:...} and pushes it.
• tool_complete at :1650 (origin/master) does tc.preview = d.preview || tc.preview || ''; (overwrites header with result).

The PR's replacement at the same site:

if(d.preview){
  tc.snippet=tc.snippet||d.preview;
  if(!tc.preview) tc.preview=d.preview;
}

Trace through the two real cases against buildToolCard at static/ui.js:6905-6937 (hasDetail = tc.snippet || hasArgs, previewText = tc.preview||displaySnippet||''):

With-progress tool (agent emitted progress text via tool event):

• Old: header = result, no detail block. Loses progress text.
• New: header = last progress text, detail = result. Better.

No-progress tool (most tools — tool event arrives with empty preview):

• Old: tc.preview = result, tc.snippet = '', hasDetail = false. Header = result, no detail.
• New: tc.preview = result (empty fills from fallback), tc.snippet = result, hasDetail = true. Header = result, detail = result. Duplication introduced.

The "no progress" case is the common one — most non-streaming tools (shell exec, read_file, search) don't emit interim tool events with a preview payload. They go straight from tool-with-empty-preview to tool_complete-with-result-snippet. After this PR, those cards render the same text in both the header pill and the expanded detail body.

Suggested fix

Either:

1. Make detail-block rendering skip the duplicate case. In buildToolCard at static/ui.js:6909, change hasDetail to:

   const hasDetail = (tc.snippet && tc.snippet !== tc.preview) || (tc.args && Object.keys(tc.args).length>0);

   Cheap, contained to one file, no logic change in the event handler.

2. Or only set tc.snippet when d.preview differs from tc.preview. In the tool_complete handler:

   if(d.preview){
     if(tc.preview && tc.preview !== d.preview){
       tc.snippet = tc.snippet || d.preview;   // genuine progress->result split
     } else {
       tc.preview = d.preview;                 // no-progress: keep old behavior
     }
   }

I'd lean toward option 1 — same effect with less branching, and it also covers the reload-path duplication at static/ui.js:6660-6695 where derived entries get snippet:... populated but no preview, so reload-time also produces equal-content header/detail on master.

Other notes

• S.toolCalls accumulation: static/messages.js:1605 in the tool handler still blindly pushes a new tc on every progress event. For a tool with two progress events plus a completion, you end up with three entries in inflight.toolCalls, only one of which gets matched in the tool_complete reverse-search at :1639-1644 (the first not-done one). The others stay done:false. Not introduced by this PR — already on master — but worth filing separately if it's affecting per-card render count.
• The "What's NOT in this PR" list is helpful. Dropping test_pr_3019_3020_2973.py is fine for now, but I'd want regression coverage on the Session completion unread dot persists after navigating away from actively-viewed session #3020 path before this lands so the actively-viewed-count-sync logic doesn't silently regress.

Verdict

#3020 part: LGTM. #2973 part: LGTM. Tool-card-duplication part: please reconsider — under the current shape it fixes one class of card (with-progress) by regressing another (no-progress, which is most tools). I'd be inclined to merge the first two changes now and pull the third out for revision with option 1 above. Happy to look at a v2 of just the tool-card change.

[nesquena-hermes]

Shipped in v0.51.161 (Release EG, stage-batch43) via release PR #3147 — thank you, @mysoul12138! 🎉

Your change (stale unread dot + compression-timer leak + tool-card dedup (#3020, #2973)) is now on master, authorship preserved.

This PR was cherry-picked onto current master (its branch base was several releases stale; a naive merge would have reverted intervening releases).

GitHub didn't auto-close because the release merged the reparented commit rather than your branch's exact head SHA, so closing manually. Full diff + tests verified on master; the merged release branch passed the full sequential pytest suite (6823 tests, 0 failures), Opus-advisor reviewed. Closing as shipped.