fix(compress): make manual /compress durable across reload + restart (#4986)
Manual /compress shrinks the model-facing context_messages but keeps the visible messages[] transcript. The old context returned via two paths (#4836): (1) append-only state.db reconciliation re-appending pre-compression rows, and (2) startup .bak recovery treating the intentional shrink as data loss. Half A (routes.py _handle_session_compress): persist truncation_watermark + truncation_boundary (= watermark of the compressed context), set compression_anchor_mode="manual", refresh last_prompt_tokens, stamp missing timestamps on the compressed context, and delete the now-stale .bak. The boundary==watermark stamp drives the #4772 reconciliation logic onto its conservative path (block replay of pre-compression rows) while post-compression turns still merge once sidecar timestamps advance past the watermark. Half B (session_recovery.py): the .bak recovery guard. MAINTAINER FIX over the original PR — the contributor suppressed recovery whenever compression_anchor_mode=="manual", a flag set once at compress and never cleared, which PERMANENTLY disabled #1558 crash-recovery for any compressed session (real data loss). Recovery is now suppressed only when the session was intentionally compressed AND the .bak is genuinely the pre-compression backup, discriminated by the compaction marker (the same _context_messages_include_ compression_marker signal reconciliation uses): a marked .bak post-dates the compression -> recover; an unmarked .bak with a larger context is the shrink-undoing pre-compression one -> suppress. Fail-open on any error. Resolves an Opus-gate edge in the first cut (length-only heuristic wrongly suppressed a loss that shrank BOTH messages and context). Two new non-vacuous regression tests: post-compression real loss recovers, and the both-shrunk marked-backup case recovers. Co-authored-by: hyl-ailab <hyl-ailab@users.noreply.github.com>
This commit is contained in:
1 parent
6ecb231451
commit
f7d727d061
5 files changed
+481
-7
No files matched your search
@@ -5,6 +5,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Manual `/compress` now sticks across reloads and restarts instead of springing back to the old context size.** A manual compression shrinks the model-facing context but the old transcript could return through two paths: (1) append-only `state.db` reconciliation re-appending pre-compression rows, and (2) startup `.bak` recovery treating the intentional shrink as data loss. `/compress` now persists a truncation watermark/boundary (so reconciliation stops replaying pre-compression rows), refreshes the post-compression token metric, and drops the now-stale backup. Crucially, the `.bak` safeguard is only suppressed for the *pre-compression* backup whose restore would undo the shrink (detected by content — its model-facing context is still the larger uncompressed one) — a backup written *after* the compression that captures genuine post-compression data loss is **still recovered**, so manual compression can never permanently disable crash-recovery for that session. Thanks @hyl-ailab. (#4986, fixes #4836)
|
||||
|
||||
- **Typed composer input is no longer wiped when a saved session auto-opens on boot.** On a hard refresh, boot-restore auto-opens the last session from localStorage — but that load can finish *after* you've already started typing into the fresh composer, and the saved-session draft restore would clobber what you typed. The boot-restore path now passes `preserveActiveInput` through to the existing draft-restore guard, so your in-progress text wins over the saved draft; a blank composer still restores the saved draft as before. Thanks @rodboev. (#5069, fixes #5060)
|
||||
|
||||
- **The Hermes Dashboard now has a show/hide chip in Settings → Appearance.** The Appearance → "Sidebar tabs" grid (where you toggle which tabs appear in the sidebar/rail) gains a "Hermes Dashboard" chip alongside the others, bidirectionally synced with the existing dashboard-mode dropdown (never/always/auto): toggling the chip off sets the dropdown to "never," toggling it on restores the last non-never mode, and changing the dropdown updates the chip. The chip is a proxy for the dashboard config — it never mutates the sidebar `hidden_tabs`/`tab_order` state — and the sync is guarded against a stale config GET clobbering a newer save, with a clean rollback if the save fails. Thanks @rodboev. (#4936, fixes #2724)
|
||||
|
||||
+18
-1
@@ -19138,7 +19138,12 @@ def _handle_session_compress(handler, body):
|
||||
if _sanitize_messages_for_api(s.messages) != original_messages:
|
||||
return bad(handler, "Session was modified during compression; please retry.", 409)
|
||||
|
||||
s.context_messages = copy.deepcopy(compressed)
|
||||
from api.session_ops import _truncation_watermark_for
|
||||
from api.streaming import _stamp_missing_message_timestamps
|
||||
|
||||
compressed_copy = copy.deepcopy(compressed)
|
||||
_stamp_missing_message_timestamps(compressed_copy)
|
||||
s.context_messages = compressed_copy
|
||||
s.active_stream_id = None
|
||||
s.pending_user_message = None
|
||||
s.pending_attachments = []
|
||||
@@ -19153,7 +19158,19 @@ def _handle_session_compress(handler, body):
|
||||
s.compression_anchor_summary = _compact_summary_text(
|
||||
summary_text or _compression_summary_from_messages(compressed) or ""
|
||||
)
|
||||
# Persist an intentional-shrink boundary so append-only state.db
|
||||
# reconciliation does not replay pre-compression rows (#4836).
|
||||
compress_watermark = _truncation_watermark_for(compressed_copy)
|
||||
s.truncation_watermark = compress_watermark
|
||||
s.truncation_boundary = compress_watermark
|
||||
s.compression_anchor_mode = "manual"
|
||||
s.last_prompt_tokens = new_tokens
|
||||
s.save()
|
||||
# Drop stale backups that would undo an intentional manual compress.
|
||||
try:
|
||||
s.path.with_suffix(".json.bak").unlink(missing_ok=True)
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
session_payload = redact_session_data(
|
||||
s.compact() | {
|
||||
|
||||
@@ -65,6 +65,106 @@ def _msg_count(p: Path) -> int:
|
||||
return len(msgs) if isinstance(msgs, list) else -1
|
||||
|
||||
|
||||
def _session_records_intentional_compress_shrink(session_path: Path) -> bool:
|
||||
"""Return True when the live sidecar records an intentional context shrink.
|
||||
|
||||
Manual ``/compress`` keeps the visible transcript but replaces the
|
||||
model-facing ``context_messages`` with a smaller compacted prefix. That
|
||||
operation must not be treated as accidental data loss by the #1558
|
||||
``.bak`` safeguard or startup recovery (#4836).
|
||||
|
||||
NOTE: this only reports *whether* the live session was intentionally
|
||||
compressed. It must NOT, on its own, decide to skip ``.bak`` recovery —
|
||||
a session can be manually compressed and *then* later suffer a genuine
|
||||
#1558 ``messages``-array loss. The caller pairs this with a freshness
|
||||
check (the backup must predate the compression) so a real post-compress
|
||||
loss is still recovered. See ``inspect_session_recovery_status``.
|
||||
"""
|
||||
try:
|
||||
data = json.loads(session_path.read_text(encoding='utf-8'))
|
||||
except (OSError, json.JSONDecodeError, ValueError):
|
||||
return False
|
||||
if not isinstance(data, dict):
|
||||
return False
|
||||
|
||||
context_messages = data.get('context_messages')
|
||||
messages = data.get('messages')
|
||||
has_shorter_context = (
|
||||
isinstance(context_messages, list)
|
||||
and isinstance(messages, list)
|
||||
and len(context_messages) < len(messages)
|
||||
)
|
||||
anchor_summary = str(data.get('compression_anchor_summary') or '').strip()
|
||||
anchor_key = data.get('compression_anchor_message_key')
|
||||
mode = str(data.get('compression_anchor_mode') or '').strip().lower()
|
||||
watermark = data.get('truncation_watermark')
|
||||
|
||||
if mode == 'manual':
|
||||
return True
|
||||
if has_shorter_context and anchor_summary and anchor_key is not None:
|
||||
return True
|
||||
if has_shorter_context and watermark is not None:
|
||||
return True
|
||||
return False
|
||||
|
||||
|
||||
def _backup_predates_intentional_shrink(session_path: Path, bak_path: Path) -> bool:
|
||||
"""True when the ``.bak`` captured the PRE-compression transcript.
|
||||
|
||||
The intentional-compress guard (#4836) must only suppress recovery for a
|
||||
backup whose restore would *undo* the user's deliberate shrink — i.e. a
|
||||
backup taken before ``/compress`` ran, still carrying the large
|
||||
UNCOMPRESSED ``context_messages``. A backup written *after* the
|
||||
compression (a later save that lost data) carries the ALREADY-COMPRESSED
|
||||
context and MUST still be recoverable.
|
||||
|
||||
Time can't tell these apart (the ``.bak`` and main file are written in the
|
||||
same ``save()`` milliseconds apart), so we use the same content-semantic
|
||||
signal reconciliation uses: the compaction marker. A post-compression
|
||||
backup carries the ``[context compaction…]`` marker in its
|
||||
``context_messages`` (it persists across saves); a pre-compression backup
|
||||
does not. We suppress recovery ONLY when the backup is genuinely
|
||||
pre-compression — its context lacks the compaction marker AND is larger
|
||||
than the live compressed context (the shrink hasn't been applied to it).
|
||||
If the backup already carries the marker, it post-dates the compression →
|
||||
a real loss → recover. The length clause is a secondary guard for the rare
|
||||
compression that emits no marker; whenever in doubt we fail OPEN (return
|
||||
False, allow recovery) — for a data-loss safeguard, recovering real data
|
||||
is always the safer error. Fail OPEN on any read/parse error too.
|
||||
"""
|
||||
try:
|
||||
live = json.loads(session_path.read_text(encoding='utf-8'))
|
||||
bak = json.loads(bak_path.read_text(encoding='utf-8'))
|
||||
except (OSError, json.JSONDecodeError, ValueError):
|
||||
return False
|
||||
if not isinstance(live, dict) or not isinstance(bak, dict):
|
||||
return False
|
||||
|
||||
bak_ctx = bak.get('context_messages')
|
||||
bak_ctx = bak_ctx if isinstance(bak_ctx, list) else []
|
||||
|
||||
# If the backup's context already carries the compaction marker, the backup
|
||||
# post-dates the compression (the marker persists across saves) → it is a
|
||||
# recoverable post-compression snapshot, never a shrink-undoing one.
|
||||
try:
|
||||
from api.models import _context_messages_include_compression_marker
|
||||
if _context_messages_include_compression_marker(bak_ctx):
|
||||
return False
|
||||
except Exception:
|
||||
# If the marker check is unavailable, fall through to the length guard.
|
||||
logger.debug("compaction-marker check unavailable in recovery", exc_info=True)
|
||||
|
||||
live_ctx = live.get('context_messages')
|
||||
live_ctx_len = len(live_ctx) if isinstance(live_ctx, list) else 0
|
||||
bak_ctx_len = len(bak_ctx)
|
||||
|
||||
# Secondary guard (unmarked compression): a pre-compression backup's context
|
||||
# is larger than the live compressed context (the shrink hasn't been applied
|
||||
# to it). A backup whose context is already <= the live compressed context
|
||||
# post-dates the compression and represents recoverable post-compress data.
|
||||
return bak_ctx_len > live_ctx_len
|
||||
|
||||
|
||||
def inspect_session_recovery_status(session_path: Path) -> dict:
|
||||
"""Return a status dict describing whether recovery is recommended.
|
||||
|
||||
@@ -86,6 +186,17 @@ def inspect_session_recovery_status(session_path: Path) -> dict:
|
||||
}
|
||||
bak_count = _msg_count(bak_path)
|
||||
if bak_count > live_count:
|
||||
if (
|
||||
_session_records_intentional_compress_shrink(session_path)
|
||||
and _backup_predates_intentional_shrink(session_path, bak_path)
|
||||
):
|
||||
return {
|
||||
"session_id": session_path.stem,
|
||||
"live_messages": live_count,
|
||||
"bak_messages": bak_count,
|
||||
"recommend": "no_action",
|
||||
"intentional_compress_shrink": True,
|
||||
}
|
||||
return {
|
||||
"session_id": session_path.stem,
|
||||
"live_messages": live_count,
|
||||
|
||||
@@ -0,0 +1,334 @@
|
||||
"""Regression tests for #4836 — manual /compress undone by reconciliation/recovery."""
|
||||
from __future__ import annotations
|
||||
|
||||
import contextlib
|
||||
import io
|
||||
import json
|
||||
import sys
|
||||
import time
|
||||
import types
|
||||
|
||||
import api.models as models
|
||||
from api.models import Session, reconciled_state_db_messages_for_session
|
||||
from api.routes import _handle_session_compress, get_session
|
||||
from api.session_recovery import inspect_session_recovery_status, recover_all_sessions_on_startup
|
||||
from tests._pytest_port import BASE
|
||||
|
||||
|
||||
class _FakeHandler:
|
||||
def __init__(self):
|
||||
self.wfile = io.BytesIO()
|
||||
self.status = None
|
||||
self.sent_headers = {}
|
||||
|
||||
def send_response(self, status):
|
||||
self.status = status
|
||||
|
||||
def send_header(self, key, value):
|
||||
self.sent_headers[key] = value
|
||||
|
||||
def end_headers(self):
|
||||
pass
|
||||
|
||||
def payload(self):
|
||||
return json.loads(self.wfile.getvalue().decode("utf-8"))
|
||||
|
||||
|
||||
class _FakeCompressor:
|
||||
def compress(self, messages, current_tokens=None, focus_topic=None):
|
||||
if len(messages) >= 2:
|
||||
return [messages[0], messages[-1]]
|
||||
return list(messages)
|
||||
|
||||
|
||||
class _FakeAgent:
|
||||
last_instance = None
|
||||
|
||||
def __init__(self, **kwargs):
|
||||
self.kwargs = kwargs
|
||||
self.context_compressor = _FakeCompressor()
|
||||
_FakeAgent.last_instance = self
|
||||
|
||||
|
||||
def _install_fake_compression_runtime(monkeypatch, agent_cls):
|
||||
fake_run_agent = types.ModuleType("run_agent")
|
||||
fake_run_agent.AIAgent = agent_cls
|
||||
monkeypatch.setitem(sys.modules, "run_agent", fake_run_agent)
|
||||
|
||||
import api.config as _cfg
|
||||
|
||||
fake_runtime_provider = types.ModuleType("hermes_cli.runtime_provider")
|
||||
fake_runtime_provider.resolve_runtime_provider = lambda requested=None: {
|
||||
"api_key": "fake-key",
|
||||
"provider": requested or "openai",
|
||||
"base_url": "https://api.openai.com/v1",
|
||||
}
|
||||
fake_hermes_cli = types.ModuleType("hermes_cli")
|
||||
fake_hermes_cli.__path__ = []
|
||||
fake_hermes_cli.runtime_provider = fake_runtime_provider
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli", fake_hermes_cli)
|
||||
monkeypatch.setitem(sys.modules, "hermes_cli.runtime_provider", fake_runtime_provider)
|
||||
import hermes_cli.runtime_provider as _rtp
|
||||
|
||||
monkeypatch.setattr(
|
||||
_cfg,
|
||||
"resolve_model_provider",
|
||||
lambda model: ("openai/gpt-5.4-mini", "openai", "https://api.openai.com/v1"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
_cfg,
|
||||
"_get_session_agent_lock",
|
||||
lambda sid: contextlib.nullcontext(),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
_rtp,
|
||||
"resolve_runtime_provider",
|
||||
lambda requested=None: {
|
||||
"api_key": "fake-key",
|
||||
"provider": requested or "openai",
|
||||
"base_url": "https://api.openai.com/v1",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def _msg(role, content, ts):
|
||||
return {"role": role, "content": content, "timestamp": ts, "_ts": ts}
|
||||
|
||||
|
||||
def test_manual_compress_persists_truncation_boundary(monkeypatch, cleanup_test_sessions, tmp_path):
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir(parents=True)
|
||||
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
|
||||
monkeypatch.setattr(models, "SESSION_INDEX_FILE", session_dir / "_index.json")
|
||||
models.SESSIONS.clear()
|
||||
|
||||
original_messages = [
|
||||
_msg("user", "one", 1.0),
|
||||
_msg("assistant", "two", 2.0),
|
||||
_msg("user", "three", 3.0),
|
||||
_msg("assistant", "four", 4.0),
|
||||
]
|
||||
sid = f"issue4836_{time.time_ns()}"
|
||||
cleanup_test_sessions.append(sid)
|
||||
session = Session(
|
||||
session_id=sid,
|
||||
title="Untitled",
|
||||
workspace=str(tmp_path),
|
||||
model="openai/gpt-5.4-mini",
|
||||
messages=original_messages,
|
||||
)
|
||||
session.save(touch_updated_at=False)
|
||||
|
||||
_install_fake_compression_runtime(monkeypatch, _FakeAgent)
|
||||
handler = _FakeHandler()
|
||||
_handle_session_compress(handler, {"session_id": sid})
|
||||
|
||||
assert handler.status == 200
|
||||
loaded = Session.load(sid)
|
||||
assert loaded.compression_anchor_mode == "manual"
|
||||
assert loaded.truncation_watermark is not None
|
||||
assert loaded.truncation_boundary == loaded.truncation_watermark
|
||||
assert loaded.last_prompt_tokens is not None
|
||||
assert len(loaded.context_messages) == 2
|
||||
assert loaded.messages == original_messages
|
||||
|
||||
|
||||
def test_manual_compress_blocks_state_db_replay(monkeypatch, cleanup_test_sessions, tmp_path):
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir(parents=True)
|
||||
monkeypatch.setattr(models, "SESSION_DIR", session_dir)
|
||||
monkeypatch.setattr(models, "SESSION_INDEX_FILE", session_dir / "_index.json")
|
||||
models.SESSIONS.clear()
|
||||
|
||||
sidecar = [
|
||||
_msg("user", "one", 1.0),
|
||||
_msg("assistant", "two", 2.0),
|
||||
_msg("user", "three", 3.0),
|
||||
_msg("assistant", "four", 4.0),
|
||||
]
|
||||
state_db = [
|
||||
_msg("user", "one", 1.0),
|
||||
_msg("assistant", "two", 2.0),
|
||||
_msg("user", "three", 3.0),
|
||||
_msg("assistant", "four", 4.0),
|
||||
_msg("assistant", "", 4.1), # reasoning-only row that used to replay
|
||||
]
|
||||
session = Session(
|
||||
session_id=f"issue4836_reconcile_{time.time_ns()}",
|
||||
title="Untitled",
|
||||
workspace=str(tmp_path),
|
||||
model="openai/gpt-5.4-mini",
|
||||
messages=sidecar,
|
||||
context_messages=sidecar,
|
||||
)
|
||||
session.save(touch_updated_at=False)
|
||||
cleanup_test_sessions.append(session.session_id)
|
||||
|
||||
_install_fake_compression_runtime(monkeypatch, _FakeAgent)
|
||||
handler = _FakeHandler()
|
||||
_handle_session_compress(handler, {"session_id": session.session_id})
|
||||
assert handler.status == 200
|
||||
|
||||
loaded = Session.load(session.session_id)
|
||||
merged = reconciled_state_db_messages_for_session(
|
||||
loaded,
|
||||
prefer_context=True,
|
||||
state_messages=state_db,
|
||||
)
|
||||
assert len(merged) == len(loaded.context_messages)
|
||||
|
||||
|
||||
def test_startup_recovery_skips_intentional_manual_compress(monkeypatch, tmp_path):
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir(parents=True)
|
||||
sid = "issue4836_recovery"
|
||||
live_path = session_dir / f"{sid}.json"
|
||||
bak_path = session_dir / f"{sid}.json.bak"
|
||||
|
||||
live = {
|
||||
"session_id": sid,
|
||||
"title": "Untitled",
|
||||
"workspace": str(tmp_path),
|
||||
"model": "openai/gpt-5.4-mini",
|
||||
"messages": [
|
||||
_msg("user", "one", 1.0),
|
||||
_msg("assistant", "two", 2.0),
|
||||
],
|
||||
"context_messages": [
|
||||
_msg("user", "one", 1.0),
|
||||
],
|
||||
"compression_anchor_summary": "Compressed: 2 -> 1 messages",
|
||||
"compression_anchor_message_key": {"role": "assistant", "ts": 2.0, "text": "two", "attachments": 0},
|
||||
"compression_anchor_mode": "manual",
|
||||
"truncation_watermark": 1.0,
|
||||
"truncation_boundary": 1.0,
|
||||
"message_count": 2,
|
||||
}
|
||||
bak = dict(live)
|
||||
bak["messages"] = live["messages"] + [
|
||||
_msg("user", "three", 3.0),
|
||||
_msg("assistant", "four", 4.0),
|
||||
] * 150
|
||||
# The .bak is the PRE-compression backup: its model-facing context_messages
|
||||
# is still the large UNCOMPRESSED context (the shrink hasn't been applied to
|
||||
# it). Restoring it would undo the intentional shrink → no_action.
|
||||
bak["context_messages"] = bak["messages"]
|
||||
live_path.write_text(json.dumps(live), encoding="utf-8")
|
||||
bak_path.write_text(json.dumps(bak), encoding="utf-8")
|
||||
|
||||
status = inspect_session_recovery_status(live_path)
|
||||
assert status["recommend"] == "no_action"
|
||||
assert status.get("intentional_compress_shrink") is True
|
||||
|
||||
result = recover_all_sessions_on_startup(session_dir)
|
||||
assert result["restored"] == 0
|
||||
restored = json.loads(live_path.read_text(encoding="utf-8"))
|
||||
assert len(restored["messages"]) == 2
|
||||
|
||||
|
||||
def test_startup_recovery_still_fires_for_post_compress_real_loss(monkeypatch, tmp_path):
|
||||
"""A manually-compressed session that LATER loses its messages array must
|
||||
still be recovered from a backup written AFTER the compression (#4836
|
||||
must not permanently disable #1558 crash-recovery for compressed sessions).
|
||||
"""
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir(parents=True)
|
||||
sid = "issue4836_post_compress_loss"
|
||||
live_path = session_dir / f"{sid}.json"
|
||||
bak_path = session_dir / f"{sid}.json.bak"
|
||||
|
||||
# Live sidecar: manually compressed AND then truncated by a genuine loss
|
||||
# (messages array shrank to 1 entry).
|
||||
live = {
|
||||
"session_id": sid,
|
||||
"title": "Untitled",
|
||||
"workspace": str(tmp_path),
|
||||
"model": "openai/gpt-5.4-mini",
|
||||
"messages": [_msg("user", "one", 1.0)],
|
||||
"context_messages": [_msg("user", "one", 1.0)],
|
||||
"compression_anchor_summary": "Compressed earlier",
|
||||
"compression_anchor_message_key": {"role": "assistant", "ts": 2.0, "text": "two", "attachments": 0},
|
||||
"compression_anchor_mode": "manual",
|
||||
"truncation_watermark": 1.0,
|
||||
"truncation_boundary": 1.0,
|
||||
"message_count": 1,
|
||||
}
|
||||
# Backup: a healthy post-compression snapshot (more messages, written AFTER
|
||||
# the compress). Its context_messages is the SAME already-compressed context
|
||||
# as live (the compression persists across saves), so it post-dates the
|
||||
# compression and must be restored when the live file later loses data.
|
||||
bak = dict(live)
|
||||
bak["messages"] = [
|
||||
_msg("user", "one", 1.0),
|
||||
_msg("assistant", "two", 2.0),
|
||||
_msg("user", "three", 3.0),
|
||||
_msg("assistant", "four", 4.0),
|
||||
]
|
||||
bak["context_messages"] = [_msg("user", "one", 1.0)] # same compressed context as live
|
||||
live_path.write_text(json.dumps(live), encoding="utf-8")
|
||||
bak_path.write_text(json.dumps(bak), encoding="utf-8")
|
||||
|
||||
status = inspect_session_recovery_status(live_path)
|
||||
assert status["recommend"] == "restore", (
|
||||
"post-compression real loss must still be recoverable even though the "
|
||||
"session was manually compressed"
|
||||
)
|
||||
|
||||
result = recover_all_sessions_on_startup(session_dir)
|
||||
assert result["restored"] == 1
|
||||
restored = json.loads(live_path.read_text(encoding="utf-8"))
|
||||
assert len(restored["messages"]) == 4
|
||||
assert len(restored["context_messages"]) == 1
|
||||
|
||||
|
||||
def test_startup_recovery_fires_when_loss_shrinks_both_messages_and_context(monkeypatch, tmp_path):
|
||||
"""A post-compression loss that clobbers BOTH messages AND context_messages
|
||||
below the healthy backup's context must STILL be recovered.
|
||||
|
||||
Opus gate edge: a pure length heuristic (bak_ctx_len > live_ctx_len) would
|
||||
misread the healthy backup's larger context as 'pre-compression' and wrongly
|
||||
suppress recovery — permanent silent data loss. The compaction-marker
|
||||
discriminator fixes this: the healthy post-compression backup carries the
|
||||
`[context compaction…]` marker, so it is recognized as post-compression and
|
||||
recovered regardless of relative context lengths.
|
||||
"""
|
||||
session_dir = tmp_path / "sessions"
|
||||
session_dir.mkdir(parents=True)
|
||||
sid = "issue4836_both_shrunk"
|
||||
live_path = session_dir / f"{sid}.json"
|
||||
bak_path = session_dir / f"{sid}.json.bak"
|
||||
|
||||
marker = {"role": "user", "content": "[context compaction] summary of earlier turns", "timestamp": 1.0}
|
||||
# Live: a genuine loss clobbered messages (130->2) AND context (50->1 marker only).
|
||||
live = {
|
||||
"session_id": sid,
|
||||
"title": "Untitled",
|
||||
"workspace": str(tmp_path),
|
||||
"model": "openai/gpt-5.4-mini",
|
||||
"messages": [_msg("user", "one", 1.0), _msg("assistant", "two", 2.0)],
|
||||
"context_messages": [marker],
|
||||
"compression_anchor_summary": "Compressed earlier",
|
||||
"compression_anchor_message_key": {"role": "assistant", "ts": 2.0, "text": "two", "attachments": 0},
|
||||
"compression_anchor_mode": "manual",
|
||||
"truncation_watermark": 1.0,
|
||||
"truncation_boundary": 1.0,
|
||||
"message_count": 2,
|
||||
}
|
||||
# Healthy post-compression backup: MORE messages AND a LARGER compressed
|
||||
# context (still carries the compaction marker) — must be recovered.
|
||||
bak = dict(live)
|
||||
bak["messages"] = [_msg("user", f"m{i}", float(i)) for i in range(130)]
|
||||
bak["context_messages"] = [marker] + [_msg("assistant", f"c{i}", float(i)) for i in range(49)]
|
||||
live_path.write_text(json.dumps(live), encoding="utf-8")
|
||||
bak_path.write_text(json.dumps(bak), encoding="utf-8")
|
||||
|
||||
status = inspect_session_recovery_status(live_path)
|
||||
assert status["recommend"] == "restore", (
|
||||
"a loss shrinking both messages and context must still recover from a "
|
||||
"marked (post-compression) backup — the marker, not length, decides"
|
||||
)
|
||||
|
||||
result = recover_all_sessions_on_startup(session_dir)
|
||||
assert result["restored"] == 1
|
||||
restored = json.loads(live_path.read_text(encoding="utf-8"))
|
||||
assert len(restored["messages"]) == 130
|
||||
+16
-6
@@ -172,14 +172,24 @@ def test_session_compress_roundtrip(monkeypatch, cleanup_test_sessions):
|
||||
loaded = get_session(sid)
|
||||
persisted = Session.load(sid)
|
||||
assert loaded.messages == original_messages
|
||||
assert loaded.context_messages == compressed_messages
|
||||
assert [m.get("role") for m in loaded.context_messages] == [m.get("role") for m in compressed_messages]
|
||||
assert [m.get("content") for m in loaded.context_messages] == [m.get("content") for m in compressed_messages]
|
||||
assert loaded.tool_calls == settled_tool_calls
|
||||
assert persisted.messages == original_messages
|
||||
assert persisted.context_messages == compressed_messages
|
||||
assert [m.get("role") for m in persisted.context_messages] == [m.get("role") for m in compressed_messages]
|
||||
assert [m.get("content") for m in persisted.context_messages] == [m.get("content") for m in compressed_messages]
|
||||
assert persisted.tool_calls == settled_tool_calls
|
||||
assert loaded.compression_anchor_summary == payload["session"]["compression_anchor_summary"]
|
||||
assert loaded.compression_anchor_visible_idx == payload["session"]["compression_anchor_visible_idx"]
|
||||
assert loaded.compression_anchor_message_key == payload["session"]["compression_anchor_message_key"]
|
||||
assert loaded.compression_anchor_mode == "manual"
|
||||
assert loaded.truncation_watermark is not None
|
||||
assert loaded.truncation_boundary == loaded.truncation_watermark
|
||||
assert loaded.last_prompt_tokens is not None
|
||||
assert persisted.compression_anchor_mode == "manual"
|
||||
assert persisted.truncation_watermark == loaded.truncation_watermark
|
||||
assert persisted.truncation_boundary == loaded.truncation_boundary
|
||||
assert not (SESSION_DIR / f"{sid}.json.bak").exists()
|
||||
assert persisted.compression_anchor_visible_idx == 3
|
||||
assert persisted.compression_anchor_message_key == payload["session"]["compression_anchor_message_key"]
|
||||
assert _FakeAgent.last_instance is not None
|
||||
@@ -269,10 +279,10 @@ def test_session_compress_start_is_async_and_reuses_running_job(monkeypatch, cle
|
||||
assert done_payload["session"]["tool_calls"] == settled_tool_calls
|
||||
persisted = Session.load(sid)
|
||||
assert persisted.tool_calls == settled_tool_calls
|
||||
assert persisted.context_messages == [
|
||||
{"role": "user", "content": "one"},
|
||||
{"role": "assistant", "content": "four"},
|
||||
]
|
||||
# /compress now stamps missing message timestamps, so compare role/content
|
||||
# rather than full-dict equality (mirrors test_session_compress_roundtrip).
|
||||
assert [m.get("role") for m in persisted.context_messages] == ["user", "assistant"]
|
||||
assert [m.get("content") for m in persisted.context_messages] == ["one", "four"]
|
||||
|
||||
|
||||
def test_session_compress_status_reports_worker_error_without_raw_paths(monkeypatch, cleanup_test_sessions):
|
||||
|
||||
Reference in new issue
Block a user