feat: add TTL cache utility for reducing redundant API calls

[bnusunny]

Adds a thread-safe in-memory TTL cache with per-key expiration for reducing redundant AWS API calls during SAM CLI operations.

[cortex-assistant]

Review: feat: add TTL cache utility

The implementation is clean and the core design is sound — time.monotonic() for expiration, threading.Lock for safety, and a simple dict-based store. However, there are a few issues that should be addressed before merging.

🔴 Must Fix

1. No unit tests — A utility claiming thread-safety needs tests for basic get/set/delete, TTL expiration, concurrent access, and edge cases. This is a hard blocker.

2. Unbounded memory growth — There is no max_size limit. If callers cache many unique keys (e.g., per-resource API responses), the store grows without bound. Expired entries are only cleaned up on access or when size() is called. Add a max_size parameter with eviction (e.g., LRU or oldest-first) on set().

3. TOCTOU race in size() — now = time.monotonic() is captured before acquiring the lock. Time passes between that call and the lock acquisition, so the filtering may keep entries that are actually expired. Move now = time.monotonic() inside the with self._lock: block.

🟡 Should Fix

4. size() mutates state as a side effect — A method named size is expected to be read-only, but it purges expired entries. Either rename it (e.g., cleanup_and_count()) or separate the purge into its own method.

5. Python version compatibility — dict[str, tuple[Any, float]] syntax requires Python 3.9+. If the project supports 3.8, use typing.Dict and typing.Tuple instead.

6. No proactive cleanup — Expired entries only get removed when their specific key is accessed via get() or when size() is called. Consider a purge-on-write strategy (e.g., probabilistic cleanup on set()).

💡 Suggestions

7. set shadows the Python built-in — common in cache APIs but some linters will flag it. put or store are alternatives.
8. No __contains__ / has() method — callers must do cache.get(key) is not None, which is ambiguous if None is a valid cached value.
9. get() returns None for both "key missing" and "key expired" — worth documenting explicitly in the docstring.

---

The core design is solid. With tests and a max-size cap, this would be a great addition. 👍

[cortex-assistant]

🔴 Unbounded growth: The _store dict has no size limit. In long-running processes with many unique cache keys, this will grow without bound since expired entries are only cleaned up lazily.

Suggestion: add a max_size parameter and evict oldest entries on set():

def __init__(self, default_ttl: float = 300.0, max_size: int = 1024):
    self._max_size = max_size

[cortex-assistant]

🔴 TOCTOU race: now is captured before the lock is acquired. Move it inside the with block:

def size(self) -> int:
    with self._lock:
        now = time.monotonic()
        self._store = {k: v for k, v in self._store.items() if v[1] > now}
        return len(self._store)

Also, a method named size shouldn't mutate state as a side effect — consider splitting into a purge() method and a read-only size.

[cortex-assistant]

🟡 Python compat: dict[str, tuple[Any, float]] requires Python 3.9+. If the project supports 3.8, use typing.Dict[str, typing.Tuple[Any, float]] instead.

[cortex-assistant]

I've completed the review of PR #19. Here's a summary of what I did:

• Submitted a REQUEST_CHANGES review with 3 inline comments on the key issues:

  1. Unbounded memory growth (line 14) — no max_size limit on the cache store
  2. TOCTOU race in size() (line 46) — time.monotonic() captured outside the lock, plus size() mutating state as a side effect
  3. Python 3.9+ syntax (line 14) — dict[str, tuple[...]] may break on older Python versions

• Added labels: size:S and enhancement

The main blockers are the missing unit tests and unbounded memory growth. The core design is solid and should be ready to merge after those are addressed.