Skip to content

fix: clarify live auto-compression state - #2517

Closed
Michaelyklam wants to merge 1 commit into
nesquena:masterfrom
Michaelyklam:fix/issue-2477-auto-compression-phase
Closed

Michaelyklam wants to merge 1 commit into
nesquena:masterfrom
Michaelyklam:fix/issue-2477-auto-compression-phase

Conversation

@Michaelyklam

Copy link
Copy Markdown
Contributor

Thinking Path

  • Long WebUI runs can spend minutes inside automatic context compression before continuing.
  • The WebUI already receives compressing and compressed SSE events and renders them through compression cards.
  • Current master has a running-state card path, but the automatic card still used the generic automatic-compression label and did not show elapsed time, making long fallback delays easy to misread.
  • This PR keeps the existing SSE/session model and only tightens the live visual state for the running phase.

What Changed

  • Store startedAt when the compressing SSE event creates the live automatic-compression state.
  • Render running automatic-compression cards with the running label/visual family instead of the completed automatic-compression label.
  • Add an elapsed-time line to the running card detail so multi-minute compression/fallback delays are visible.
  • Preserve the existing completed automatic-compression card styling after the compressed event.
  • Add source-level regression coverage for the running timestamp, running label, running variant, and elapsed detail.
  • Add a release-note entry.

Why It Matters

This makes the UI more honest during the specific #2477 failure mode: the backend can still be alive and compressing while the browser looks frozen. A visible running phase plus elapsed time reduces the chance that users refresh, retry, or interrupt a still-useful run.

Verification

  • /home/michael/.hermes/hermes-agent/venv/bin/python -m pytest tests/test_auto_compression_card.py -q — 26 passed
  • node --check static/messages.js
  • node --check static/ui.js
  • git diff --check
  • UI evidence: Before/after automatic compression running card comparison

Risks / Follow-ups

  • This is a narrow live-card visibility fix. It does not add durable replay/persistence for compression fallback provider details or continuation-session handoff events.
  • Elapsed time updates on rerender/SSE activity; it does not start a separate timer loop.
  • Broader run-state consistency work remains tracked by the related streaming/compression issues.

Model Used

AI-assisted change with repository inspection, targeted editing, screenshot generation, and shell-based test verification.

Refs #2477

@Michaelyklam
Michaelyklam force-pushed the fix/issue-2477-auto-compression-phase branch from 2535562 to 58ba09a Compare May 18, 2026 09:15
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Summary

Reading static/ui.js:4953-4979 and static/messages.js:1785-1791 on this branch against origin/master, the diff is small and correctly stamps startedAt:Date.now() when the compressing SSE event creates the live compression state, then formats Elapsed ${_formatTurnDuration(...)} into the running card. The intent matches issue #2477 — long auto-compression windows shouldn't look frozen.

However, I want to flag two things that come out of reading the surrounding code paths.

Code reference

The render itself (static/ui.js:4953-4979 on this branch):

function _autoCompressionCardsHtml(state){
  const fallback='Context auto-compressed to continue the conversation';
  const running=state&&state.phase==='running';
  const detail=running
    ? (String(state.message||'Auto-compressing context...').trim()||'Auto-compressing context...')
    : (String(state.message||fallback).trim()||fallback);
  const startedAt=Number(state&&state.startedAt||0);
  const elapsed=(running&&startedAt>0)
    ? `Elapsed ${_formatTurnDuration(Math.max(0,(Date.now()-startedAt)/1000))}`
    : '';

And the only place _autoCompressionCardsHtml is invoked is from _compressionCardsHtml(state) at static/ui.js:4892, which is itself driven by renderCompressionUi() calls from setCompressionUi(state) at static/ui.js:4881-4889. There is no setInterval or any other periodic trigger that re-renders this card after the initial compressing SSE event.

Diagnosis / Recommendation

1. The elapsed counter does not actually tick. This is the substantive concern. The PR description says "Elapsed time updates on rerender/SSE activity; it does not start a separate timer loop." But during automatic compression, the whole point of issue #2477 is that no SSE activity arrives between compressing and compressed — that's exactly the gap users experience as a frozen UI. So the card renders once at T+0, shows Elapsed 0s, then sits there showing 0s for however long the summarizer LLM call takes, then flips to the completed card. The user sees the same "frozen" experience the issue is complaining about, just with a static "0s" line of text added. To actually solve #2477 the timer has to tick during the silent window, which means a setInterval(rerender, 1000) keyed to the running phase — exactly the shape used by _activityElapsedTimer at static/ui.js:2017:

if(!_activityElapsedTimer)_activityElapsedTimer=setInterval(_updateActiveActivityElapsedTimer,1000);

2. PR #2512 is the same fix and is more complete. It's open in parallel by another author and already lands a 1-second ticking timer (_startCompressionElapsedTimer + _compressionElapsedStartedAt), a 5-minute cap (_COMPRESSION_ELAPSED_MAX_SECONDS), a 5+ min overflow label, and DOM-anchored state at data-compression-started-at so the timer survives full renderMessages() redraws. That PR has had a review pass and the author addressed the feedback in d72b3382. The two PRs touch the same SSE handler in static/messages.js and the same _autoCompressionCardsHtml renderer, so whichever lands first will create a merge conflict for the other.

3. Tests pin only structural strings, not the live behavior. The new assertions in tests/test_auto_compression_card.py check that the source text contains startedAt:Date.now() and Elapsed ${_formatTurnDuration — that catches accidental removal of the lines but doesn't verify the elapsed display actually updates over time, which is the user-visible behavior #2477 cares about.

Recommendation

Two paths, with a clear preference:

let _compressionElapsedTimer=null;
function _startCompressionElapsedTimer(){
  if(_compressionElapsedTimer) return;
  _compressionElapsedTimer=setInterval(renderCompressionUi,1000);
}
function _clearCompressionElapsedTimer(){
  if(!_compressionElapsedTimer) return;
  clearInterval(_compressionElapsedTimer); _compressionElapsedTimer=null;
}

…and wire _startCompressionElapsedTimer() into setCompressionUi(state) when state.phase==='running' && state.automatic, and _clearCompressionElapsedTimer() everywhere else (including clearCompressionUi). Without that, the "elapsed time" line is cosmetic only.

Test plan

A behavior test rather than a string-match test would catch #1: stub Date.now() to advance by 60s, call renderCompressionUi() between, and assert the rendered HTML contains 1m 0s (or whatever _formatTurnDuration(60) returns) on the second render. Right now nothing in tests/test_auto_compression_card.py would notice if the elapsed line said 0s forever.

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Closing as parallel-discovery superseded by #2512

Thanks for the focused slice on auto-compression elapsed time, Michael — closing this in favor of #2512 which targets the same Slice A of #2477 and went a little further:

  • adds a live 1-second elapsed timer-tick instead of the static stamp,
  • carries a 5+min cap framing for the longer-running cases,
  • contributor @dso2ng already addressed maintainer feedback in d72b3382,
  • CI green on both, but fix: show auto-compression elapsed time #2512 has more user-visible signal in the same surface.

No need to rework — the diagnosis was correct, your reading of the issue was the same, just two contributors converging on the same slice. If you spot any feature in #2517 that #2512 doesn't cover (e.g. wording in messages.js), feel free to open a follow-up against the shipped version.

Closing per the parallel-discovery pattern.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants