fix(sidebar+compression+tool-card): cron filter, unread dot, timer leak, tool card duplication (#3019, #3020, #2973)

[mysoul12138]

Summary

Four coordinated bugfixes:

• Cron sessions hidden from sidebar when 'Cron Jobs' project chip selected #3019: Allow cron sessions with project_id through the sidebar filter so the "Cron Jobs" project chip can display them. Frontend filters cron rows from the default "All" view and triggers a server refetch when the cron chip is clicked. Cron project created with system: true flag to prevent accidental rename breaking the lookup.

• 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. The tool_complete SSE handler set tc.preview to result_snippet, which is the same content buildToolCard displays in the expanded detail section. Route result to tc.snippet only, preserving tc.preview as the last streaming progress text.

Test plan

• test_pr_3019_3020_2973.py — 5 static-analysis tests covering all fixes
• Manual: click "Cron Jobs" chip → cron sessions appear
• 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]

Summary

Read the full diff (3 files, +33/-5), pulled the worktree, and walked each of the three fixes against master. The bundling matches what was triaged in #3019, #3020, and #2973 — a small surgical set rather than three separate PRs is the right call here because they share the same _setSessionViewedCount / sidebar-rendering / compression-card pipeline. Two of the three are clean; #2973 has a subtle interaction worth thinking through.

#3019 — cron sessions hidden from "Cron Jobs" chip

Server-side fix at api/models.py:1983-1991 is correct. Before:

if source == 'cron' or sid.startswith('cron_'):
    return True

After:

if source == 'cron' or sid.startswith('cron_'):
    return not bool(session.get('project_id'))

The cron project ID gets populated at api/models.py:3128 ('project_id': _cron_pid() if is_cron_session(...)), and that's used by both the CLI loader and the index path. So cron sessions that went through _load_cli_sessions_uncached now flow through _hide_from_default_sidebar without being dropped.

Frontend pairing at static/sessions.js:3071-3075 handles the "All" view correctly:

:(_activeProject
  ?profileFiltered.filter(s=>s.project_id===_activeProject)
  :profileFiltered.filter(s=>!s.project_id||(s.source_tag!=='cron'&&!String(s.session_id||'').startsWith('cron_'))));

The !s.project_id || (...) shape means "no project = keep" OR "has project but not cron = keep" — cron sessions with project_id set get filtered out of the default view. Reads correctly.

The refetch-on-cron-chip trigger at sessions.js:3146-3152 is the missing piece — because get_cli_sessions() is gated on show_cli_sessions in api/routes.py:4433, the cron rows don't sit in the default cache for the chip filter to reveal. The targeted refetch makes the chip work in either setting.

One small consideration: the _cronProjectId lookup at sessions.js:2148 uses p.name === 'Cron Jobs' as the match key. That's the canonical name from ensure_cron_project(), but if a user ever renames the cron project via the sidebar context menu, this lookup breaks and the refetch stops happening. Worth either (a) keying off p.is_cron_project if such a flag exists or could be added, or (b) renaming the cron project at api/models.py:2665+ to be immutable for the same reason __none__ is. Not a blocker — current users don't rename Cron Jobs in practice.

#3020 — stale unread dot after navigating away

Two changes work together:

1. Defensive clear in _setSessionViewedCount at sessions.js:144-146. Every time the viewed count is bumped to current, any prior completion-unread marker is now cleared. This is the belt-and-suspenders fix that prevents the dot from surviving a tab switch even if the polling transition path didn't catch the sync.

2. Active-view sync in _markPollingCompletionUnreadTransitions at sessions.js:413-419. Now correctly handles the actively-viewed case by syncing the count instead of just skipping. Previously the if (... && !_isSessionActivelyViewedForList(sid)) guard short-circuited both the unread-mark AND the count-sync; the new structure splits them.

The _setSessionViewedCount change does subtly tighten an invariant — anywhere else in the codebase that called _setSessionViewedCount(sid, n) with n < current_message_count would now clear a legitimate unread marker. I searched the six call sites at sessions.js:139,241,418,487,638,1280 and they all bump to S.session.message_count or equivalent. No regression vector. ✓

#2973 — compression timer leak

This is the one I want to push on. The change at static/ui.js:5397-5403:

} else {
  // Completion or error: clear the elapsed-timer attributes ...
  node.removeAttribute('data-compression-started-at');
  node.removeAttribute('data-compression-message');
  _clearCompressionElapsedTimer();
}

The DOM attribute removal is correct — _compressionLiveCardNode() at line 2180 uses '[data-live-compression-card="1"][data-compression-started-at]', so without the attribute the live-card lookup returns null and the elapsed timer naturally stops being driven by stale DOM state. Good.

But _clearCompressionElapsedTimer() runs unconditionally when state is non-running — even if another session is still mid-compression. The timer is global (_compressionElapsedTimer is a module-level singleton). Concrete scenario:

1. Session A starts auto-compressing → timer starts.
2. User switches to session B which is also auto-compressing → DOM gets new running card → _startCompressionElapsedTimer() is called (no-op if timer already running).
3. Session A completes (via SSE for A) → appendLiveCompressionCard(stateA={...phase:'done'...}) runs against B's DOM (the liveAssistantTurn belongs to the current session) and clears the timer that was driving B's display.

How realistic is this? Probably narrow — auto-compression usually drives the active session only, and the existing _updateCompressionElapsedTimer at line 2201 already has a self-defense at line 2204-2205 that clears the timer when the current session has no running state. So if B is current and still running, the next tick re-starts it via the regular re-render path. Still, the unconditional clear feels brittle.

Safer shape: only clear the timer when the active session has no running compression. Something like:

} else {
  node.removeAttribute('data-compression-started-at');
  node.removeAttribute('data-compression-message');
  const activeState = _compressionStateForCurrentSession();
  if (!activeState || !activeState.automatic || activeState.phase !== 'running') {
    _clearCompressionElapsedTimer();
  }
}

That predicate matches the one already used at ui.js:5261-5262 after the live-card replace block, which suggests the intent everywhere else is "the timer follows the active session's state, not the most recent SSE event." Worth aligning here too.

Test plan

The PR body has a manual checklist but no test additions. For #3019 a unit test on _hide_from_default_sidebar({'source_tag':'cron', 'project_id': 'p_cron'}) returning False vs. {...'project_id': None} returning True would be a one-screen win. For #3020, the _setSessionViewedCount clear-on-sync invariant is asserted indirectly by behavior tests but a direct unit on the localStorage state would be clean. For #2973, gated on the active-session-aware clear shape above.

Verdict

Two of three are ship-ready. The compression timer's unconditional _clearCompressionElapsedTimer() is the only thing I'd ask to harden before merge — a 3-line predicate to make it match the active-session pattern used elsewhere. Otherwise good fix bundle.

[nesquena-hermes]

Triage: HOLD — overlap with #3069 — labels: hold, changes-requested

Thanks for the multi-bug bundle. This PR fixes #3019 + #3020 + #2973 and also bumps the CLI tool-result snippet length from 200 to 4000 chars. There's an overlap problem we want to flag before merging:

#3069 also fixes #3019, with a different approach (introduces a default_hidden flag on session payloads rather than changing _hide_from_default_sidebar directly). #3069 is narrowly scoped to the one bug and is in the merge queue.

We're going to ship #3069's approach for #3019. To avoid wasted work, would you mind:

1. Rebasing this PR onto current master after fix: show cron sessions in project filter #3069 lands
2. Dropping the #3019 change here (_hide_from_default_sidebar modification + the ensure_cron_project system: True flag + the _cronProjectId fetch-on-cron-chip flow)
3. Keeping the Session completion unread dot persists after navigating away from actively-viewed session #3020 fix (the _setSessionViewedCount completion-unread clear + the _markPollingCompletionUnreadTransitions viewed-count sync) — this is clean and we want it
4. Keeping the Auto-compression running state keeps timing until the whole session ends #2973 fix (the appendLiveCompressionCard data-compression-started-at cleanup + active-session-guarded timer clear) — also clean
5. Splitting the CLI tool-result snippet 200→4000 bump out — that's a separate concern and we'd like to evaluate it independently (4000 chars is a 20x bump and may affect rendering on long shell commands; would prefer it as its own PR with a screenshot)

The system flag on ensure_cron_project is a thoughtful belt-and-suspenders and we may absorb that detail into a small follow-up regardless of which #3019 approach wins.

Will revisit once master moves and you've had a chance to slim this down. Thanks for the careful test coverage in test_pr_3019_3020_2973.py — that file structure is exactly what we want.

[mysoul12138]

Superseded by #3116 (slimmed-down: #3020 + #2973 + tool card duplication) and #3117 (snippet limit 200→4000). #3019 changes dropped — handled by #3069.