fix(cache): make the ttl parameter actually control expiry (#450)

CacheManager.set(key, data, ttl=...) stored the number and no read path
ever consulted it. Expiry came from a max_age inferred from substrings
in the key -- "live", "odds", "stock" -- so all 52 callers passing a ttl
were writing a value that did nothing. The docstring said so outright:
"stored for compatibility but expiration is still controlled via max_age
when reading". It is easier to read that as a note than as a defect,
which is presumably how it survived.

Both cache layers already hold the record when they decide, so each now
prefers an explicit ttl and falls back to max_age when there is none.
The caller that wrote the record knows what its data is; a substring
guess is a reasonable default for records that never said, and a poor
override for records that did.

Measured against a device's real cache of 8,875 entries carrying a ttl,
the inferred and intended values disagreed nearly everywhere:

    stocks    max_age  600  vs ttl    1800   4903 entries
    news      max_age 3600  vs ttl     600   1770 entries
    odds      max_age 1800  vs ttl    3600   1301 entries
    images    max_age  300  vs ttl 2592000     20 entries

In every case the ttl matches what the plugin plainly intended: stock
quotes cached for half an hour rather than ten minutes, headlines
refreshed every ten minutes rather than hourly, bird photographs that
never change kept for a month rather than five minutes.

Two things make this safe to land now. No sports_live entry carries a
ttl at all -- the live-score path does not use set(ttl=) -- so live
freshness is untouched, which matters with a season two weeks out. And
replaying the change against that real cache, 997 currently-expired
entries become live while not one live entry becomes expired, so there
is no invalidation spike on deploy.


Claude-Session: https://claude.ai/code/session_01Udr6MfaFLUPhX5Fgo67Jf5

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>

src/cache/disk_cache.py

@@ -112,6 +112,22 @@ class DiskCache:
                     record_ts = None
             
             now = time.time()
+
+            # An explicit per-entry ttl wins over the caller's max_age. The
+            # caller that wrote the record knows what its data is; max_age is
+            # inferred from substrings in the key ("live", "odds", "stock") and
+            # is only a fallback for records that never said. Until now the ttl
+            # was stored and ignored, so `set(key, data, ttl=...)` did nothing
+            # at all -- 48 plugin call sites and 4 in the core were writing a
+            # number no read path consulted.
+            effective_max_age = max_age
+            if isinstance(record, dict):
+                stored_ttl = record.get('ttl')
+                if isinstance(stored_ttl, (int, float)) and not isinstance(stored_ttl, bool) \
+                        and stored_ttl >= 0:
+                    effective_max_age = stored_ttl
+            max_age = effective_max_age
+
             # max_age=None means "never expires" (mirrors MemoryCache and the
             # cache_manager docstring). Guard it explicitly — otherwise the
             # comparison below raises TypeError and the record is treated as a

src/cache/memory_cache.py

@@ -57,6 +57,16 @@ class MemoryCache:
             if timestamp is None:
                 return None
             
+            # An explicit per-entry ttl wins over the caller's max_age, matching
+            # DiskCache. max_age is inferred from substrings in the key and is
+            # only a fallback for records that did not say what they wanted.
+            record = self._cache[key]
+            if isinstance(record, dict):
+                stored_ttl = record.get('ttl')
+                if isinstance(stored_ttl, (int, float)) and not isinstance(stored_ttl, bool) \
+                        and stored_ttl >= 0:
+                    max_age = stored_ttl
+
             # Check expiration
             if max_age is not None and (now - timestamp) > max_age:
                 # Expired - remove it

src/cache_manager.py

@@ -594,8 +594,10 @@ class CacheManager:
         Args:
             key: Cache key
             data: Data to cache
-            ttl: Optional time-to-live in seconds (stored for compatibility but
-                 expiration is still controlled via max_age when reading)
+            ttl: Time-to-live in seconds for this entry. Takes precedence over
+                 the max_age a reader would otherwise apply, which is inferred
+                 from the key and is only a fallback for entries that did not
+                 say. Omit it to keep that inferred behaviour.
         """
         cache_data = {
             'data': data,

test/test_cache_ttl_honoured.py

@@ -0,0 +1,130 @@
+"""Tests that a per-entry ttl actually controls expiry.
+
+Regression under test: `CacheManager.set(key, data, ttl=...)` stored the value
+and no read path ever consulted it. Expiry came from a `max_age` inferred from
+substrings in the key ("live", "odds", "stock"), so every caller passing `ttl=`
+-- 48 sites across the plugins and 4 in the core -- was writing a number that
+did nothing. The old docstring admitted as much: "stored for compatibility but
+expiration is still controlled via max_age when reading".
+
+Measured against a real device's cache (8,873 entries carrying a ttl), the
+inferred value and the intended one disagreed almost everywhere:
+
+    stocks    max_age  600  vs ttl   1800   4903 entries
+    news      max_age 3600  vs ttl    600   1770 entries
+    odds      max_age 1800  vs ttl   3600   1301 entries
+    images    max_age  300  vs ttl 2592000    20 entries
+
+No `sports_live` entry carries a ttl, so live scores keep their inferred
+30-second freshness either way.
+"""
+
+import time
+
+import pytest
+
+from src.cache.memory_cache import MemoryCache
+from src.cache.disk_cache import DiskCache
+
+
+@pytest.fixture
+def disk(tmp_path):
+    return DiskCache(cache_dir=str(tmp_path))
+
+
+def _record(ttl=None, age=0.0):
+    rec = {"data": {"v": 1}, "timestamp": time.time() - age}
+    if ttl is not None:
+        rec["ttl"] = ttl
+    return rec
+
+
+class TestDiskCacheHonoursTtl:
+    def test_ttl_longer_than_max_age_keeps_the_entry(self, disk):
+        # The odds case: written wanting an hour, expired at 30 minutes.
+        disk.set("odds_espn_football_nfl_401", _record(ttl=3600, age=1900))
+        assert disk.get("odds_espn_football_nfl_401", max_age=1800) is not None
+
+    def test_ttl_shorter_than_max_age_expires_the_entry(self, disk):
+        # The news case: written wanting 10 minutes, kept for an hour.
+        disk.set("news_NHL_1", _record(ttl=600, age=900))
+        assert disk.get("news_NHL_1", max_age=3600) is None
+
+    def test_without_a_ttl_max_age_still_applies(self, disk):
+        disk.set("plain_key", _record(age=400))
+        assert disk.get("plain_key", max_age=300) is None
+        disk.set("plain_key2", _record(age=100))
+        assert disk.get("plain_key2", max_age=300) is not None
+
+    def test_a_fresh_entry_within_its_ttl_survives(self, disk):
+        disk.set("k", _record(ttl=600, age=10))
+        assert disk.get("k", max_age=30) is not None
+
+    def test_ttl_zero_expires_immediately(self, disk):
+        # 0 means zero seconds, not "forever" -- max_age=None is how a caller
+        # asks for no expiry.
+        disk.set("k", _record(ttl=0, age=1))
+        assert disk.get("k", max_age=99999) is None
+
+    @pytest.mark.parametrize("bad", ["600", None, True, False, -5, {"a": 1}])
+    def test_a_nonsense_ttl_falls_back_to_max_age(self, disk, bad):
+        # Including bools: True is an int in Python and must not become a 1s ttl.
+        rec = _record(age=400)
+        rec["ttl"] = bad
+        disk.set("k_%s" % type(bad).__name__, rec)
+        assert disk.get("k_%s" % type(bad).__name__, max_age=300) is None
+
+
+class TestMemoryCacheHonoursTtl:
+    def test_ttl_longer_than_max_age_keeps_the_entry(self):
+        m = MemoryCache()
+        m.set("k", _record(ttl=3600))
+        m._timestamps["k"] = time.time() - 1900
+        assert m.get("k", max_age=1800) is not None
+
+    def test_ttl_shorter_than_max_age_expires_the_entry(self):
+        m = MemoryCache()
+        m.set("k", _record(ttl=600))
+        m._timestamps["k"] = time.time() - 900
+        assert m.get("k", max_age=3600) is None
+
+    def test_without_a_ttl_max_age_still_applies(self):
+        m = MemoryCache()
+        m.set("k", _record())
+        m._timestamps["k"] = time.time() - 400
+        assert m.get("k", max_age=300) is None
+
+    def test_both_layers_agree(self, tmp_path):
+        """A record must not be live in one layer and expired in the other."""
+        rec = _record(ttl=3600, age=1900)
+        d = DiskCache(cache_dir=str(tmp_path))
+        d.set("k", rec)
+        m = MemoryCache()
+        m.set("k", rec)
+        m._timestamps["k"] = rec["timestamp"]
+        assert (d.get("k", max_age=1800) is not None) == (m.get("k", max_age=1800) is not None)
+
+
+class TestEndToEnd:
+    def test_set_then_get_respects_the_ttl(self, tmp_path, monkeypatch):
+        """The behaviour a caller of CacheManager.set(ttl=...) expects."""
+        from src.cache_manager import CacheManager
+
+        cm = CacheManager()
+        cm._disk_cache_component = DiskCache(cache_dir=str(tmp_path))
+        cm._memory_cache_component = MemoryCache()
+
+        cm.set("odds_espn_football_nfl_401", {"spread": 6.5}, ttl=3600)
+
+        # Age the stored record past the inferred max_age for odds (1800s) but
+        # within the ttl the caller asked for.
+        path = cm._disk_cache_component.get_cache_path("odds_espn_football_nfl_401")
+        import json
+        rec = json.load(open(path))
+        rec["timestamp"] = time.time() - 1900
+        json.dump(rec, open(path, "w"))
+        cm._memory_cache_component.clear() if hasattr(
+            cm._memory_cache_component, "clear") else None
+
+        got = cm.get_with_auto_strategy("odds_espn_football_nfl_401")
+        assert got is not None, "the ttl the caller asked for was ignored"