Skip to content

Live Stream: Show compression recovery from session state - #5655

Closed
franksong2702 wants to merge 1 commit into
nesquena:masterfrom
franksong2702:franksong2702/4685-session-level-recovery-card
Closed

franksong2702 wants to merge 1 commit into
nesquena:masterfrom
franksong2702:franksong2702/4685-session-level-recovery-card

Conversation

@franksong2702

@franksong2702 franksong2702 commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Thinking Path

#5538 shipped the first #4685 recovery path: compression-exhausted sessions recommend a focused continuation child session, and generic continue prompts are intercepted. That depends on users seeing the recovery card. The normal SSE path stores _compressionRecovery on the terminal assistant message, but rebuilt or legacy sessions can still expose the active recovery payload at session.compression_recovery without a message marker. In that state the send box can block generic continuation while the visible card/action is missing.

Touched contract family: Live-to-final / compression recovery state. Evidence used: #4685 shipped recovery payload semantics, compression_recovery session payload, and the render path in static/ui.js.

What Changed

  • Keep message-level _compressionRecovery as the preferred source for the recovery card.
  • When the active session still has compression_exhausted recovery state but the message marker is absent, render the existing recovery card on the last assistant terminal message.
  • Gate that fallback to isLastAssistant && isTurnFinalAssistant so active session-level recovery does not add cards to historical assistant turns.
  • Guard the helper lookup for partial renderMessages() harnesses that do not load the full UI module.
  • Add a focused regression test for the session-level fallback.

Why It Matters

Without this fallback, a recovered/rebuilt compression-exhausted session can enter an inconsistent UX: the app knows recovery is required and intercepts generic continue, but the user does not get the visible "Start focused continuation" action. This keeps #4685's shipped first slice usable when session-level state survives but message-level display metadata is missing.

Release-note-ready wording: show the focused continuation recovery card when a compression-exhausted session only has session-level recovery metadata.

UI evidence: this does not introduce a new visual state; it reuses the existing compression recovery card and changes only when that existing card appears. The new regression guards the missing-card state.

Verification

  • node --check static/ui.js
  • ./scripts/test.sh tests/test_compression_recovery_action.py -q -> 16 passed
  • ./scripts/test.sh tests/test_compression_recovery_action.py tests/test_auto_compression_terminal_failure.py -q -> 30 passed
  • ./scripts/test.sh tests/test_anchor_fallback_ownership.py::test_render_messages_keeps_anchor_owned_turn_out_of_legacy_activity_rebuilds -q -> 1 passed

Risks / Follow-ups

Model Used

AI-assisted. Provider: OpenAI. Model: GPT-5 Codex. Notable tool use: local shell, focused repo tests, GitHub CLI.

@franksong2702
franksong2702 force-pushed the franksong2702/4685-session-level-recovery-card branch 2 times, most recently from e47431d to 77f54ee Compare July 6, 2026 01:43
@cutter-sh

cutter-sh Bot commented Jul 6, 2026

Copy link
Copy Markdown

🎬 Cutter preview — PR #5655

/chat
/chat — When context compression is exhausted, the assistant reply now surfaces a recovery card to start a focused continuation.

@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds a session-level fallback for the compression recovery card: when the active session has compression_exhausted recovery state but the last assistant message lacks the _compressionRecovery marker, the fallback path calls _activeCompressionRecoveryPayload() to source the card payload from S.session.compression_recovery. A new structural test verifies the fallback code is present in ui.js.

  • Fallback rendering path: Extends renderMessages() to check _activeCompressionRecoveryPayload() for the last assistant terminal message when m._compressionRecovery is absent, keeping the recovery card visible for rebuilt/legacy sessions.
  • Guard clause added: Wraps the fallback in a typeof _activeCompressionRecoveryPayload==='function' check to protect partial renderMessages() test harnesses that do not load the full UI module.
  • New regression test: Adds a static string-pattern assertion in test_compression_recovery_action.py confirming the fallback structure is present in the source.

Confidence Score: 4/5

Safe to merge for the primary intended use case (session has compression_recovery property but message marker is absent), but has a defect in the legacy message-scan fallback path that can produce duplicate recovery cards.

The new fallback delegates to _activeCompressionRecoveryPayload(), which has a secondary message-scan path for sessions where S.session has no compression_recovery property. In that path, if a non-last message already holds _compressionRecovery (rendered by the primary branch), the scan returns the same payload and the last assistant message renders a second recovery card. The PR explicitly guards against duplicate cards on historical turns, but the guard only prevents the fallback from applying to historical turns — it does not prevent a historical turn rendered via the primary path from producing a card alongside the fallback card on the last message.

static/ui.js — specifically the interaction between the new fallback at line 13817 and the message-scan branch inside _activeCompressionRecoveryPayload() (lines 447–453).

Important Files Changed

Filename Overview
static/ui.js Adds session-level recovery card fallback for last assistant turn; the fallback delegates to _activeCompressionRecoveryPayload() which has a secondary message-scan path that can produce duplicate cards in legacy sessions where a non-last message already holds _compressionRecovery.
tests/test_compression_recovery_action.py Adds a static structural assertion test (string-matching on source code) verifying the fallback logic is present; does not exercise behavioral rendering or cover the message-scan duplicate-card scenario.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[renderMessages loop — assistant message] --> B{m._compressionRecovery?}
    B -- yes --> C[recoveryPayload = m._compressionRecovery]
    B -- no --> D{isLastAssistant AND isTurnFinalAssistant?}
    D -- no --> E[recoveryPayload = null]
    D -- yes --> F{typeof _activeCompressionRecoveryPayload === 'function'?}
    F -- no --> E
    F -- yes --> G[call _activeCompressionRecoveryPayload]
    G --> H{S.session.compression_recovery present?}
    H -- yes, exhausted --> I[return session-level payload ✅]
    H -- yes, cleared --> J[return null ✅]
    H -- property absent legacy session --> K[scan messages for _compressionRecovery]
    K --> L{non-last message has _compressionRecovery?}
    L -- yes --> M[return that payload ⚠️ duplicate card risk]
    L -- no --> E
    C --> N[_compressionRecoveryHtml rendered]
    I --> N
    M --> N
    E --> O[no recovery card]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[renderMessages loop — assistant message] --> B{m._compressionRecovery?}
    B -- yes --> C[recoveryPayload = m._compressionRecovery]
    B -- no --> D{isLastAssistant AND isTurnFinalAssistant?}
    D -- no --> E[recoveryPayload = null]
    D -- yes --> F{typeof _activeCompressionRecoveryPayload === 'function'?}
    F -- no --> E
    F -- yes --> G[call _activeCompressionRecoveryPayload]
    G --> H{S.session.compression_recovery present?}
    H -- yes, exhausted --> I[return session-level payload ✅]
    H -- yes, cleared --> J[return null ✅]
    H -- property absent legacy session --> K[scan messages for _compressionRecovery]
    K --> L{non-last message has _compressionRecovery?}
    L -- yes --> M[return that payload ⚠️ duplicate card risk]
    L -- no --> E
    C --> N[_compressionRecoveryHtml rendered]
    I --> N
    M --> N
    E --> O[no recovery card]
Loading

Reviews (2): Last reviewed commit: "Render session recovery card from active..." | Re-trigger Greptile

Comment thread static/ui.js
const recoveryHtml=(!isUser&&m._compressionRecovery) ? _compressionRecoveryHtml(m._compressionRecovery, (S.session&&S.session.session_id)||'') : '';
const recoveryPayload=(!isUser&&m._compressionRecovery)
? m._compressionRecovery
: (!isUser&&isLastAssistant&&isTurnFinalAssistant&&typeof _activeCompressionRecoveryPayload==='function' ? _activeCompressionRecoveryPayload() : null);

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.

P2 The isTurnFinalAssistant gate in the fallback branch is always true whenever isLastAssistant is true, making the combined condition equivalent to isLastAssistant alone. isLastAssistant is defined as !isUser && vi === renderVisWithIdx.length-1; for the very last item, nextRendered is undefined, so !nextRendered is true and isTurnFinalAssistant resolves to true unconditionally. The &&isTurnFinalAssistant part provides no additional filtering and could mislead future readers into thinking it guards against a distinct scenario (turn-final but non-last messages).

Suggested change
: (!isUser&&isLastAssistant&&isTurnFinalAssistant&&typeof _activeCompressionRecoveryPayload==='function' ? _activeCompressionRecoveryPayload() : null);
: (!isUser&&isLastAssistant&&typeof _activeCompressionRecoveryPayload==='function' ? _activeCompressionRecoveryPayload() : null);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

🔬 Gate certification — GREEN ✅ (full gate: Codex + Fable-UX + suite) · ⏸️ visible recovery card → Nathan sign-off

Certified head: sha:a1c1454a (clean rebase, branch gate-rebase/5655-recovery-from-session-state) · PR: #5655 · franksong2702 (T1), Live Stream: Show compression recovery from session state
Verdict: Full gate clean. A correct #4685/#5538 follow-up (#5538 merged) — the compression-recovery action card now re-appears on the last final-assistant message from live session state after a reload/reconnect, not only from the inline message data. Codex SAFE, Fable-UX SHIP-UX, full suite green.

What I ran (rebased worktree /tmp/wt-rebase-5655) — full gate: Codex + Fable + suite

Gate Result
Rebase onto current master git apply clean (composes with merged #5538)
Codex (reproduce) SAFE TO SHIP — 0 findings
Fable-UX SHIP-UX — card re-appears after reload, no duplicate, correct placement
Full pytest suite 12176 passed, 0 failed
PR test ✅ 16/16

Findings

✅ Correct + composes with #5538: recoveryPayload = m._compressionRecovery (existing inline) OR — for the last final-assistant message (isLastAssistant && isTurnFinalAssistant) — falls back to _activeCompressionRecoveryPayload() (reads live recovery state). So after a reload/reconnect the recovery card re-surfaces from session state (the #4685 goal) instead of being lost. Codex confirmed: the OR short-circuit prevents a double-render when the inline _compressionRecovery also exists, the isLastAssistant && isTurnFinalAssistant guards prevent a card on a mid-transcript/non-final row, _activeCompressionRecoveryPayload() is null-safe, and — importantly — the button still targets recovery.source_session_id||sessionId (my #5538 lineage-isolation fix preserved, no wrong-session-recovery regression). Fable confirmed the card re-appears cleanly with no duplicate and consistent placement. Full suite green.

Recommendation to the next agent / Nathan

GREEN — merge from branch gate-rebase/5655-recovery-from-session-state (sha:a1c1454a), NOT the PR's stale head 77f54eeb — after a quick visual sign-off (visible recovery card). A clean follow-up to the merged #5538 recovery action — makes the recovery survive a reload by reading session state, no duplicate, preserves the source-session-id lineage fix. Codex SAFE + Fable SHIP-UX + full suite green. concept 4/5. Author @franksong2702 (T1). crit=3.


Gate-certifier layer (warm-up → gate → release). I do not merge/tag/deploy. Recovery card now re-surfaces from live session state on the last final-assistant message (OR short-circuit prevents double-render, isLastAssistant+isTurnFinalAssistant guards, null-safe, source_session_id preserved per #5538); Codex SAFE + Fable-UX SHIP-UX + full suite green (0 failed) + 16 tests. Composes with merged #5538. Visible → Nathan. Cert valid for sha:a1c1454a.

@nesquena-hermes nesquena-hermes added the gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent label Jul 6, 2026
@franksong2702

Copy link
Copy Markdown
Contributor Author

Base refresh applied for Frank's PR branch.

  • Rebased franksong2702/4685-session-level-recovery-card onto current origin/master (9738ebb61).
  • Pushed refreshed head 859d25b0 to fork-pr / Frank's PR branch.
  • Carried the same two-file recovery-card diff from the certified gate branch (a1c1454a), now on the latest base.

Local verification:

  • ./scripts/test.sh tests/test_compression_recovery_action.py -q -> 16 passed
  • python3 scripts/ruff_lint.py --diff origin/master -> no new violations
  • node --check static/ui.js -> passed

GitHub CI restarted on 859d25b0; browser-smoke, lint, and all test shards were green when read back. Greptile was still in progress at that readback.

nesquena-hermes added a commit that referenced this pull request Jul 6, 2026
Release: compression-recovery card on reloaded sessions (#5655)
@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.897 (deployed live). Thanks @franksong2702 — the recovery card now also renders from the active session-level recovery state on the final assistant message, so a rebuilt/restored compression-exhausted session still surfaces the 'Start focused continuation' action. Gate confirmed it's scoped to the last/turn-final assistant (no double-render) and a cleared recovery stays hidden. 🙏

@nesquena-hermes

Copy link
Copy Markdown
Collaborator

Shipped in v0.51.897 (merged via release branch; the code is in master + deployed live — verified the fallback render is in the served ui.js). Closing as shipped. Thanks again @franksong2702. 🙏

pull Bot pushed a commit to soitun/hermes-webui that referenced this pull request Jul 6, 2026
pull Bot pushed a commit to soitun/hermes-webui that referenced this pull request Jul 6, 2026
… sessions (nesquena#5655)

Follow-up to nesquena#5538: the recovery card now also renders from _activeCompressionRecoveryPayload() on the final assistant message when a rebuilt/legacy session lacks the per-message _compressionRecovery marker (guarded to last/turn-final assistant only; no double-render; cleared recovery still hidden). Gate: Codex SAFE, Fable SHIP-UX, suite 12179/0, browser clean.

Co-authored-by: Frank Song <franksong2702@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gate-pass Full gate passed (Codex+Opus+suite+browser); queued Tier 1 for release agent

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants