From 9681761cdf93e2a95848440279a8ad861160c88d Mon Sep 17 00:00:00 2001 From: Frank Song Date: Thu, 14 May 2026 08:47:04 +0800 Subject: [PATCH] fix: cap _summary_cache with OrderedDict LRU Refs #2215 Fix A: replace plain dict _summary_cache with OrderedDict-based LRU capped at 16 entries to prevent unbounded memory growth from long-running update summary generations. Add regression coverage for the bounded LRU behavior: cache hits refresh recency, a new entry at capacity evicts the least-recently used key, and cache size never exceeds the cap. --- api/updates.py | 8 +++++- tests/test_update_banner_fixes.py | 46 ++++++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 2 deletions(-) diff --git a/api/updates.py b/api/updates.py index 0bbca829..b91fafe1 100644 --- a/api/updates.py +++ b/api/updates.py @@ -14,6 +14,7 @@ import re import subprocess import threading import time +from collections import OrderedDict from pathlib import Path from urllib.parse import urlparse @@ -26,7 +27,8 @@ except ImportError: _AGENT_DIR = None _update_cache = {'webui': None, 'agent': None, 'checked_at': 0} -_summary_cache = {} +_SUMMARY_CACHE_MAX = 16 +_summary_cache: OrderedDict = OrderedDict() _cache_lock = threading.Lock() _check_in_progress = False _apply_lock = threading.Lock() # prevents concurrent stash/pull/pop on same repo @@ -498,6 +500,8 @@ def summarize_update_payload(updates: dict, llm_callback=None, *, target: str | if use_cache: with _cache_lock: cached = _summary_cache.get(cache_key) + if cached: + _summary_cache.move_to_end(cache_key) if cached: result = dict(cached) result['cached'] = True @@ -526,6 +530,8 @@ def summarize_update_payload(updates: dict, llm_callback=None, *, target: str | } if use_cache: with _cache_lock: + if len(_summary_cache) >= _SUMMARY_CACHE_MAX and cache_key not in _summary_cache: + _summary_cache.popitem(last=False) _summary_cache[cache_key] = dict(result) return result diff --git a/tests/test_update_banner_fixes.py b/tests/test_update_banner_fixes.py index de46a892..c5f541be 100644 --- a/tests/test_update_banner_fixes.py +++ b/tests/test_update_banner_fixes.py @@ -839,6 +839,51 @@ class TestWhatsNewSummaryToggle: assert second['summary'] == first['summary'] assert second['cached'] is True assert changed['summary'] != first['summary'] + + def test_update_summary_cache_is_bounded_lru(self): + import api.updates as upd + + upd._summary_cache.clear() + calls = [] + + def payload(n): + return { + 'webui': { + 'behind': n + 1, + 'current_sha': f'old-{n}', + 'latest_sha': f'new-{n}', + 'compare_url': f'https://example.test/webui/{n}', + }, + } + + def fake_llm(_system, prompt): + calls.append(prompt) + return f'- Generated summary #{len(calls)}' + + try: + for i in range(upd._SUMMARY_CACHE_MAX): + upd.summarize_update_payload(payload(i), llm_callback=fake_llm) + + assert len(upd._summary_cache) == upd._SUMMARY_CACHE_MAX + assert len(calls) == upd._SUMMARY_CACHE_MAX + + first_again = upd.summarize_update_payload(payload(0), llm_callback=fake_llm) + assert first_again['cached'] is True + assert len(calls) == upd._SUMMARY_CACHE_MAX + + upd.summarize_update_payload(payload(upd._SUMMARY_CACHE_MAX), llm_callback=fake_llm) + assert len(upd._summary_cache) == upd._SUMMARY_CACHE_MAX + + still_cached = upd.summarize_update_payload(payload(0), llm_callback=fake_llm) + assert still_cached['cached'] is True + assert len(calls) == upd._SUMMARY_CACHE_MAX + 1 + + evicted = upd.summarize_update_payload(payload(1), llm_callback=fake_llm) + assert evicted['cached'] is False + assert len(calls) == upd._SUMMARY_CACHE_MAX + 2 + finally: + upd._summary_cache.clear() + def test_update_summary_can_be_generated_per_target_and_cached_separately(self): import api.updates as upd @@ -932,4 +977,3 @@ class TestCheckForUpdatesButton: assert count >= 5, ( f"settings_check_now found in only {count} locale blocks (expected ≥5: en, ru, es, zh, zh-Hant)" ) -