Skip to content

fix: keep compression banner attached to the compaction marker - #2182

Merged
3 commits merged into
nesquena:masterfrom
LumenYoung:fix/compression-banner-anchor
May 13, 2026
Merged

3 commits merged into
nesquena:masterfrom
LumenYoung:fix/compression-banner-anchor

Conversation

@LumenYoung

@LumenYoung LumenYoung commented May 13, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes several related cases where the context compression banner can appear to drift away from the actual compaction boundary in long WebUI conversations.

Case 1: windowed transcript rendering

The compression anchor is stored as an index into the full visible transcript, but renderMessages() renders only a sliced renderVisWithIdx window. When the anchor key cannot be matched, the previous code passed the full visible index directly into the rendered-window array. For long sessions this index is usually outside the current render window, so the compression card falls back to inner.appendChild(node) and appears near the newest messages instead of near the compression boundary.

Case 2: persisted compaction reference messages

Some sessions already contain a persisted [CONTEXT COMPACTION — REFERENCE ONLY] message in the transcript. That message can be a stronger placement signal than anchor metadata, because it preserves the raw transcript position where compaction happened.

Case 3: stale / multiple compaction markers

Long sessions can contain multiple older persisted compaction reference messages. The banner should not blindly attach to the first or latest marker. A persisted marker is only authoritative when its content matches the current compression_anchor_summary; otherwise placement falls back to the current anchor metadata.

This prevents a new compression banner from attaching to an older compaction marker in the same transcript.

This change also keeps anchor matching tolerant of older persisted anchor keys that do not include a timestamp. Some existing sessions have compression_anchor_message_key.ts = null, while the corresponding message later has _ts/timestamp stamped. Strict timestamp comparison makes those anchors fail to match.

Fix

  • Match compression anchors by role/text/attachments even when either side is missing a timestamp.
  • Resolve the anchor against the full visWithIdx list, not the sliced render window.
  • Translate the full visible index into the current renderVisWithIdx window before inserting the compression card.
  • Clamp off-window anchors to the nearest rendered boundary instead of appending to the bottom.
  • If the current transcript contains a persisted compaction reference message matching the current compression_anchor_summary, insert the banner relative to that message's raw transcript position.
  • Ignore stale persisted compaction markers whose content does not match the current compression summary.

Why this is correct

There are now three levels of placement fidelity, in descending priority:

  1. A persisted compaction reference message that matches the current compression summary
  2. A matched saved anchor key
  3. A saved anchor index fallback

This keeps the banner stable across normal long-session windowing, partially damaged sidecars, and transcripts that contain multiple older compaction markers.

Tests

Added / updated coverage for:

  • legacy missing-timestamp anchor keys still matching
  • full visible anchor indexes being translated into render-window indexes
  • persisted compaction reference messages being positioned by their raw transcript location before anchor fallback
  • stale persisted compaction markers being ignored when they do not match the current compression summary

Tested with:

pytest -q tests/test_auto_compression_card.py tests/test_issue2028_compression_anchor_helpers.py

@LumenYoung LumenYoung changed the title fix: keep compression anchor stable in windowed transcript fix: keep compression banner attached to the compaction marker May 13, 2026
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Reading the diff against origin/master for static/ui.js:4470-4488, 4886-4956, 5095-5135, plus the master version of _compressionAnchorIndex and _insertCompressionLikeNode, this is a clean 3-tier placement fallback for the compression banner. The diagnosis splits cleanly into the two real failure modes.

Case 1: anchor index outside the render window (ui.js:4940-4956)

On master, _compressionAnchorIndex was called with the sliced renderVisWithIdx, so anchor lookup could only ever return indexes inside the visible window. When the anchor lived in older messages above the window, the matcher fell through to inner.appendChild(node) and the banner attached to the bottom.

The diff inverts this:

const insertionAnchorFull=_compressionAnchorIndex(
  visWithIdx,
  ...
);
let insertionAnchor=null;
if(typeof insertionAnchorFull==='number'){
  if(insertionAnchorFull<windowStart) insertionAnchor=renderVisWithIdx.length?0:null;
  else if(insertionAnchorFull<windowStart+renderVisWithIdx.length) insertionAnchor=insertionAnchorFull-windowStart;
  else insertionAnchor=renderVisWithIdx.length?renderVisWithIdx.length-1:null;
}

The clamping is correct — off-top maps to the first rendered row, off-bottom maps to the last. Combined with _insertCompressionLikeNode (ui.js:5095-5115), which still walks renderVisWithIdx[anchorIdx] then back-references assistantSegments / userRows from the rendered DOM, the lookup never indexes past renderVisWithIdx.length. Good.

Case 2: legacy missing-timestamp anchor keys (ui.js:4474-4480)

const anchorTs=String(anchorKey.ts??'');
const candidateTs=String(candidate.ts??'');
if(
  candidate.role===String(anchorKey.role||'') &&
  (!anchorTs||!candidateTs||candidateTs===anchorTs) &&
  ...

This loosens the timestamp match: empty on either side now passes. Risk surface: two different messages with the same role + text + attachment count but different timestamps will now collide if either side is missing a ts. In practice, anchor keys are sourced from messages with normalized text snippets (_compressionMessageAnchorKey capped at 160 chars by _compressionAnchorIndex upstream), and the matcher walks visWithIdx in reverse, so the most-recent match wins. That's the right disambiguation for "find the compression boundary" — if two identical messages exist, the boundary is more likely the newer one.

Case 3: real reference message wins over saved anchor (ui.js:4890-4892, 5174-5176)

const referenceMessage=S.messages.find(m=>_isContextCompactionMessage(m));
const referenceMessageRawIdx=referenceMessage?S.messages.findIndex(m=>m===referenceMessage):-1;
...
if(referenceNode&&referenceMessageRawIdx>=0) _insertCompressionLikeNodeByRawIdx(referenceNode, referenceMessageRawIdx);
else _insertCompressionLikeNode(referenceNode);

_insertCompressionLikeNodeByRawIdx (master, ui.js:5117-5135) walks renderVisWithIdx for the first rawIdx greater than the target and inserts before it. Using the on-screen reference message's raw position is genuinely a stronger signal than saved anchor metadata when both exist and disagree. The three-tier priority described in the PR body (real reference message → matched anchor key → anchor index fallback) is what the code actually does.

One subtle thing: the reference-node ternary in S.messages.findIndex(m=>m===referenceMessage) re-walks the list redundantly — S.messages.find already returns the object, and you could pull the index from a single findIndex call. Cosmetic.

Tests

tests/test_auto_compression_card.py:217-253 source-greps the three new assertions:

  • (!anchorTs||!candidateTs||candidateTs===anchorTs) for legacy ts tolerance
  • _compressionAnchorIndex(\n visWithIdx, plus the window-clamping bounds for the index translation
  • the reference-message raw-idx branch with the if/else fallthrough

Source-grep tests are brittle but acceptable here — the assertions name the exact strings, so any future rewrite that breaks the contract will fail loudly.

Verdict

Diagnosis is sound, fix is layered correctly, tests pin the contract. Minor cosmetic: the findIndex could be the single source. Merge once a maintainer eyeballs the timestamp-tolerance widening on case 2 — it's a small behavior change for sessions in the wild.

@LumenYoung

Copy link
Copy Markdown
Contributor Author

Hi, I found during usage another related on compression error and I appended a new commit into the current PR and re-edited the PR content to reflect this. Please review again if needed.

@nesquena-hermes nesquena-hermes closed this pull request by merging all changes into nesquena:master in f5be6e3 May 13, 2026
pull Bot pushed a commit to TKaxv-7S/hermes-webui that referenced this pull request May 13, 2026
fix: keep compression banner attached to the compaction marker (LumenYoung)
SysAdminDoc pushed a commit to SysAdminDoc/hermes-webui that referenced this pull request Jun 26, 2026
fix: keep compression banner attached to the compaction marker (LumenYoung)
bernyforce pushed a commit to bernyforce/hermes-webui that referenced this pull request Jul 29, 2026
fix: keep compression banner attached to the compaction marker (LumenYoung)
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