From ca9e821b5e4e64dc35cfb7742ac0197780655adc Mon Sep 17 00:00:00 2001 From: dobby-d-elf Date: Mon, 25 May 2026 16:25:23 -0600 Subject: [PATCH 1/2] Reduce session index churn on chat start --- api/models.py | 8 ++++++++ api/routes.py | 11 ++++++----- tests/test_regressions.py | 18 +++++++----------- tests/test_session_index.py | 18 +++++++++++++++++- .../test_stage326_composer_draft_validation.py | 4 ++-- 5 files changed, 40 insertions(+), 19 deletions(-) diff --git a/api/models.py b/api/models.py index d121c7a4..cebc99c9 100644 --- a/api/models.py +++ b/api/models.py @@ -239,6 +239,14 @@ def _write_session_index(updates=None): _write_session_index(updates=None) +def prune_session_from_index(session_id: str) -> None: + """Remove one session row from the persisted sidebar index if present.""" + sid = str(session_id or "") + if not sid or not SESSION_INDEX_FILE.exists(): + return + _write_session_index(updates=[]) + + def _active_stream_ids(): with STREAMS_LOCK: active_ids = set(STREAMS.keys()) diff --git a/api/routes.py b/api/routes.py index 420dffe6..61d9de82 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2459,6 +2459,7 @@ from api.models import ( get_state_db_session_messages, merge_session_messages_append_only, _session_message_merge_key, + prune_session_from_index, ensure_cron_project, is_cron_session, ) @@ -5306,7 +5307,7 @@ def handle_post(handler, parsed) -> bool: # here makes the active-session external-refresh poll force-reload the # current chat every few seconds while the user is typing, and that # delayed reload can restore an older draft over newer local input. - s.save(touch_updated_at=False) + s.save(touch_updated_at=False, skip_index=True) return j(handler, {"ok": True, "draft": s.composer_draft}) if parsed.path == "/api/session/update": @@ -5392,10 +5393,6 @@ def handle_post(handler, parsed) -> bool: # Delete from WebUI session store with LOCK: SESSIONS.pop(sid, None) - try: - SESSION_INDEX_FILE.unlink(missing_ok=True) - except Exception: - logger.debug("Failed to unlink session index") # Evict cached agent so turn count doesn't leak into a recycled session from api.config import _evict_session_agent _evict_session_agent(sid) @@ -5409,6 +5406,10 @@ def handle_post(handler, parsed) -> bool: p.with_suffix('.json.bak').unlink(missing_ok=True) except Exception: logger.debug("Failed to unlink session file %s", p) + try: + prune_session_from_index(sid) + except Exception: + logger.debug("Failed to prune deleted session from index: %s", sid, exc_info=True) try: from api.upload import _session_attachment_dir diff --git a/tests/test_regressions.py b/tests/test_regressions.py index d1357eb8..7426013d 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -312,11 +312,8 @@ def test_deleted_session_does_not_appear_in_list(cleanup_test_sessions): assert sid not in ids_after, f"Deleted session {sid} still appears in list -- index not invalidated on delete" -def test_server_delete_invalidates_index(cleanup_test_sessions): - """R8b: session/delete handler must unlink _index.json. - Static check that the fix is in place. - Sprint 11: handler moved from server.py to api/routes.py -- check both. - """ +def test_server_delete_prunes_session_index(cleanup_test_sessions): + """session/delete should prune the deleted row without discarding the index.""" src = (REPO_ROOT / "server.py").read_text() routes_src = (REPO_ROOT / "api" / "routes.py").read_text() if (REPO_ROOT / "api" / "routes.py").exists() else "" # Find the delete handler in either file @@ -327,12 +324,11 @@ def test_server_delete_invalidates_index(cleanup_test_sessions): text.find('if parsed.path == "/api/session/delete":'), ) if delete_idx >= 0: - # Use 1200 chars to accommodate any validation/guard code added - # before the SESSION_INDEX_FILE.unlink() call (e.g. session_id - # character checks, path traversal guards). - delete_block = text[delete_idx:delete_idx+1200] - assert "SESSION_INDEX_FILE" in delete_block, \ - f"{label} session/delete must invalidate SESSION_INDEX_FILE" + delete_block = text[delete_idx:delete_idx+1800] + assert "prune_session_from_index" in delete_block, \ + f"{label} session/delete must prune SESSION_INDEX_FILE" + assert "SESSION_INDEX_FILE.unlink" not in delete_block, \ + f"{label} session/delete must not discard the whole session index" return assert False, "session/delete handler not found in server.py or api/routes.py" diff --git a/tests/test_session_index.py b/tests/test_session_index.py index 62ec9508..7143b479 100644 --- a/tests/test_session_index.py +++ b/tests/test_session_index.py @@ -20,7 +20,7 @@ from unittest.mock import patch import pytest import api.models as models -from api.models import Session, _write_session_index +from api.models import Session, _write_session_index, prune_session_from_index @pytest.fixture(autouse=True) @@ -83,6 +83,22 @@ def test_compact_exposes_last_message_at_from_message_timestamp(): assert compact["last_message_at"] == 200.0 +def test_prune_session_from_index_removes_only_deleted_row(): + index_file = models.SESSION_INDEX_FILE + s_a = _make_session("sess_a", "A", updated_at=100) + s_b = _make_session("sess_b", "B", updated_at=200) + s_a.save() + s_b.save() + + s_a.path.unlink() + prune_session_from_index("sess_a") + + index = _read_index(index_file) + ids = [entry["session_id"] for entry in index] + assert ids == ["sess_b"] + assert index_file.exists() + + def test_all_sessions_backfills_last_message_at_for_legacy_index_rows(): index_file = models.SESSION_INDEX_FILE s = Session( diff --git a/tests/test_stage326_composer_draft_validation.py b/tests/test_stage326_composer_draft_validation.py index 3f5904d6..af9c0cac 100644 --- a/tests/test_stage326_composer_draft_validation.py +++ b/tests/test_stage326_composer_draft_validation.py @@ -100,5 +100,5 @@ def test_draft_save_does_not_touch_session_updated_at(): src = Path(__file__).parents[1].joinpath("api", "routes.py").read_text(encoding="utf-8") persist_idx = src.find("s.composer_draft = draft") assert persist_idx != -1, "could not locate composer draft persist site" - save_idx = src.find("s.save(touch_updated_at=False)", persist_idx) - assert save_idx != -1, "composer draft save must preserve session updated_at" + save_idx = src.find("s.save(touch_updated_at=False, skip_index=True)", persist_idx) + assert save_idx != -1, "composer draft save must preserve session updated_at and skip index churn" From b74df67726631382a1a7e796928c81241f110fbc Mon Sep 17 00:00:00 2001 From: dobby-d-elf Date: Tue, 26 May 2026 07:43:16 -0600 Subject: [PATCH 2/2] Make session index pruning explicit --- api/models.py | 32 +++++++++++++++++++++++++++++++- tests/test_regressions.py | 4 +--- tests/test_session_index.py | 5 +++-- 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/api/models.py b/api/models.py index cebc99c9..d17b7963 100644 --- a/api/models.py +++ b/api/models.py @@ -244,7 +244,37 @@ def prune_session_from_index(session_id: str) -> None: sid = str(session_id or "") if not sid or not SESSION_INDEX_FILE.exists(): return - _write_session_index(updates=[]) + _tmp = SESSION_INDEX_FILE.with_suffix(f'.tmp.{os.getpid()}.{threading.current_thread().ident}') + + _fallback = False + with _INDEX_WRITE_LOCK: + try: + with LOCK: + existing = json.loads(SESSION_INDEX_FILE.read_text(encoding='utf-8')) + if not isinstance(existing, list): + raise ValueError("session index must be a list") + pruned = [e for e in existing if e.get('session_id') != sid] + if len(pruned) == len(existing): + return + _payload = json.dumps(pruned, ensure_ascii=False, indent=2) + + try: + with open(_tmp, 'w', encoding='utf-8') as f: + f.write(_payload) + f.flush() + os.fsync(f.fileno()) + os.replace(_tmp, SESSION_INDEX_FILE) + except Exception: + try: + _tmp.unlink(missing_ok=True) + except Exception: + pass + raise + except Exception: + _fallback = True + + if _fallback: + _write_session_index(updates=None) def _active_stream_ids(): diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 7426013d..e2dd3441 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -325,10 +325,8 @@ def test_server_delete_prunes_session_index(cleanup_test_sessions): ) if delete_idx >= 0: delete_block = text[delete_idx:delete_idx+1800] - assert "prune_session_from_index" in delete_block, \ + assert "prune_session_from_index(sid)" in delete_block, \ f"{label} session/delete must prune SESSION_INDEX_FILE" - assert "SESSION_INDEX_FILE.unlink" not in delete_block, \ - f"{label} session/delete must not discard the whole session index" return assert False, "session/delete handler not found in server.py or api/routes.py" diff --git a/tests/test_session_index.py b/tests/test_session_index.py index 7143b479..4a137206 100644 --- a/tests/test_session_index.py +++ b/tests/test_session_index.py @@ -83,20 +83,21 @@ def test_compact_exposes_last_message_at_from_message_timestamp(): assert compact["last_message_at"] == 200.0 -def test_prune_session_from_index_removes_only_deleted_row(): +def test_prune_session_from_index_removes_requested_row_only(): index_file = models.SESSION_INDEX_FILE s_a = _make_session("sess_a", "A", updated_at=100) s_b = _make_session("sess_b", "B", updated_at=200) s_a.save() s_b.save() - s_a.path.unlink() prune_session_from_index("sess_a") index = _read_index(index_file) ids = [entry["session_id"] for entry in index] assert ids == ["sess_b"] assert index_file.exists() + assert s_a.path.exists() + assert s_b.path.exists() def test_all_sessions_backfills_last_message_at_for_legacy_index_rows():