refactor(cache-middleware): address dagardner-nv review feedback

Per design discussion in #1888:

- Drop similarity_threshold floor from 0.85 to 0 and remove manual
  type/range validation from the constructor; Pydantic enforces bounds
  at the config layer (CacheMiddlewareConfig) and anyone constructing
  CacheMiddleware directly is on their own
- Update CacheMiddlewareConfig.similarity_threshold to ge=0 with
  updated description documenting the performance cost of low values
  and the cache-collision risk near 0
- Replace the manual difflib.SequenceMatcher loop in _find_similar_key
  with difflib.get_close_matches for cleaner, more idiomatic code
- Remove constructor-level validation tests that no longer apply

Signed-off-by: ColinM-sys <cmcdonough@50words.com>

Author: ColinM-sys

packages/nvidia_nat_core/src/nat/middleware/cache/cache_middleware.py

@@ -30,7 +30,6 @@
 
 import json
 import logging
-import math
 from collections import OrderedDict
 from collections.abc import AsyncIterator
 from typing import Any
@@ -45,13 +44,6 @@
 
 logger = logging.getLogger(__name__)
 
-# Lower bound on fuzzy-match similarity to reduce the cache-poisoning surface.
-# A threshold below this makes it trivial to craft an input that is "similar
-# enough" to a legitimate user's cached key to hijack their response (for the
-# current process, which is how the in-memory cache is scoped). 0.85 is the
-# smallest value that we're comfortable shipping as an unconditional default;
-# operators with strict needs should use 1.0 (exact match only).
-_MIN_FUZZY_THRESHOLD = 0.85
 
 # Default bound on cache size. The previous implementation used an unbounded
 # dict which, under sustained unique input, grew without limit — a memory-
@@ -96,43 +88,17 @@ def __init__(
         Args:
             enabled_mode: Either "always" or "eval". If "eval", only caches
                 when Context.is_evaluating is True.
-            similarity_threshold: Similarity threshold in [_MIN_FUZZY_THRESHOLD, 1.0].
-                If 1.0, performs exact matching. Lower values enable difflib-
-                based fuzzy matching. Values below _MIN_FUZZY_THRESHOLD are
-                rejected to prevent cache-poisoning where a crafted input
-                collides with a legitimate user's cached key.
+            similarity_threshold: Similarity threshold in [0, 1.0]. If 1.0,
+                performs exact matching. Lower values enable difflib-based
+                fuzzy matching; note that difflib is quadratic in the worst
+                case, so large caches with low thresholds may have a
+                performance cost. Values near 0 increase the risk of cache
+                collisions where different inputs return the same cached
+                response.
             max_entries: Maximum number of cache entries. When exceeded, the
                 oldest entry is evicted (LRU). Defaults to
-                _DEFAULT_MAX_CACHE_ENTRIES. Must be >= 1.
-
-        Raises:
-            ValueError: If similarity_threshold is outside [_MIN_FUZZY_THRESHOLD, 1.0]
-                or max_entries is not a positive integer.
+                _DEFAULT_MAX_CACHE_ENTRIES.
         """
-        # Reject bool explicitly — `isinstance(True, int)` is True in Python,
-        # and `True`/`False` silently sneaking through as numeric is a classic
-        # config bug (user passes the wrong key, gets no error). Check bool
-        # FIRST so the "must be a number" message doesn't lie.
-        if isinstance(similarity_threshold, bool):
-            raise ValueError(
-                f"similarity_threshold must be a number, got bool ({similarity_threshold!r})")
-        if not isinstance(similarity_threshold, (int, float)):
-            raise ValueError(
-                f"similarity_threshold must be a number, got {type(similarity_threshold).__name__}")
-        if not math.isfinite(similarity_threshold):
-            raise ValueError(
-                f"similarity_threshold must be finite, got {similarity_threshold!r}")
-        if similarity_threshold < _MIN_FUZZY_THRESHOLD or similarity_threshold > 1.0:
-            raise ValueError(
-                f"similarity_threshold={similarity_threshold} is outside the safe range "
-                f"[{_MIN_FUZZY_THRESHOLD}, 1.0]. Lower values make cache-poisoning trivial — "
-                "a crafted input can collide with a legitimate user's cached key. Use 1.0 "
-                "for exact matching (recommended), or a value >= "
-                f"{_MIN_FUZZY_THRESHOLD} for fuzzy matching.")
-        # Same bool-as-int foot-gun applies to max_entries.
-        if isinstance(max_entries, bool) or not isinstance(max_entries, int) or max_entries < 1:
-            raise ValueError(f"max_entries must be a positive integer, got {max_entries!r}")
-
         super().__init__(is_final=True)
         self._enabled_mode = enabled_mode
         self._similarity_threshold = similarity_threshold
@@ -202,22 +168,13 @@ def _find_similar_key(self, input_str: str) -> str | None:
             # Exact matching - fast path
             return input_str if input_str in self._cache else None
 
-        # Fuzzy matching using difflib
         import difflib
 
-        best_match = None
-        best_ratio = 0.0
-
-        for cached_key in self._cache:
-            # Use SequenceMatcher for similarity computation
-            matcher = difflib.SequenceMatcher(None, input_str, cached_key)
-            ratio = matcher.ratio()
-
-            if ratio >= self._similarity_threshold and ratio > best_ratio:
-                best_ratio = ratio
-                best_match = cached_key
-
-        return best_match
+        best_matches = difflib.get_close_matches(
+            input_str, self._cache.keys(), n=1, cutoff=self._similarity_threshold)
+        if best_matches:
+            return best_matches[0]
+        return None
 
     async def function_middleware_invoke(self,
                                          *args: Any,

packages/nvidia_nat_core/src/nat/middleware/cache/cache_middleware_config.py

@@ -20,7 +20,6 @@
 
 from nat.data_models.middleware import FunctionMiddlewareBaseConfig
 from nat.middleware.cache.cache_middleware import _DEFAULT_MAX_CACHE_ENTRIES
-from nat.middleware.cache.cache_middleware import _MIN_FUZZY_THRESHOLD
 
 
 class CacheMiddlewareConfig(FunctionMiddlewareBaseConfig, name="cache"):
@@ -33,13 +32,13 @@ class CacheMiddlewareConfig(FunctionMiddlewareBaseConfig, name="cache"):
         enabled_mode: Controls when caching is active:
             - "always": Cache is always enabled
             - "eval": Cache only active when Context.is_evaluating is True
-        similarity_threshold: Float in [_MIN_FUZZY_THRESHOLD, 1.0] for input
-            matching:
+        similarity_threshold: Float in [0, 1.0] for input matching:
             - 1.0: Exact string matching (fastest, recommended)
-            - >= _MIN_FUZZY_THRESHOLD: Fuzzy matching via difflib. Values
-              below this bound are rejected as a cache-poisoning risk —
-              crafted inputs at lower thresholds can collide with a
-              legitimate user's cached key.
+            - < 1.0: Fuzzy matching via difflib. Note that difflib is
+              quadratic in the worst case, so large caches with low
+              thresholds may have a performance cost. Values near 0
+              increase the risk of cache collisions where different
+              inputs return the same cached response.
         max_entries: Upper bound on cached entries. When exceeded, the
             least-recently-used entry is evicted. Must be a positive int;
             defaults to _DEFAULT_MAX_CACHE_ENTRIES.
@@ -49,12 +48,14 @@ class CacheMiddlewareConfig(FunctionMiddlewareBaseConfig, name="cache"):
         default="eval", description="When caching is enabled: 'always' or 'eval' (only during evaluation)")
     similarity_threshold: float = Field(
         default=1.0,
-        ge=_MIN_FUZZY_THRESHOLD,
+        ge=0,
         le=1.0,
         description=(
-            f"Similarity threshold in [{_MIN_FUZZY_THRESHOLD}, 1.0]. Use 1.0 for exact matching "
-            "(recommended). Lower values enable fuzzy matching but are bounded below to prevent "
-            "cache-poisoning collisions with legitimate cached keys."),
+            "Similarity threshold in [0, 1.0]. Use 1.0 for exact matching (recommended). "
+            "Lower values enable fuzzy matching via difflib; note that difflib is quadratic "
+            "in the worst case, so large caches with low thresholds may have a performance "
+            "cost. Values near 0 increase the risk of cache collisions where different "
+            "inputs return the same cached response."),
     )
     max_entries: int = Field(
         default=_DEFAULT_MAX_CACHE_ENTRIES,

packages/nvidia_nat_core/tests/nat/middleware/test_cache_middleware.py

@@ -62,7 +62,6 @@ def test_default_initialization(self):
 
     def test_custom_initialization(self):
         """Test custom initialization."""
-        # Use 0.9 (above the enforced minimum) to exercise non-default fuzzy mode.
         middleware = CacheMiddleware(enabled_mode="always", similarity_threshold=0.9)
         # Check attributes are set
         assert hasattr(middleware, '_enabled_mode')
@@ -109,11 +108,7 @@ async def mock_next_call(*args, **kwargs):
         assert result3.result == "Result for test"
 
     async def test_fuzzy_match_caching(self, middleware_context):
-        """Test fuzzy matching with similarity_threshold < 1.0.
-
-        Uses 0.9 (above the enforced minimum) — 0.8 is no longer a valid
-        threshold after the cache-poisoning hardening.
-        """
+        """Test fuzzy matching with similarity_threshold < 1.0."""
         middleware = CacheMiddleware(enabled_mode="always", similarity_threshold=0.9)
 
         call_count = 0
@@ -272,9 +267,6 @@ async def mock_next_call(*args, **kwargs):
 
     def test_similarity_computation_for_different_thresholds(self):
         """Test similarity computation for different thresholds."""
-        # This is more of a unit test for the similarity logic.
-        # Uses 0.9 (above the enforced minimum) to exercise fuzzy matching
-        # without enabling cache-poisoning-prone low thresholds.
         middleware = CacheMiddleware(enabled_mode="always", similarity_threshold=0.9)
 
         # Directly test internal methods
@@ -291,11 +283,7 @@ def test_similarity_computation_for_different_thresholds(self):
         assert middleware._find_similar_key("xyz123abc") is None  # noqa
 
     async def test_multiple_similar_entries(self, middleware_context):
-        """Test behavior with multiple similar cached entries.
-
-        Uses 0.85 (the enforced minimum) instead of the original 0.7 —
-        below 0.85 is now rejected as a cache-poisoning risk.
-        """
+        """Test behavior with multiple similar cached entries."""
         middleware = CacheMiddleware(enabled_mode="always", similarity_threshold=0.85)
 
         # Pre-populate cache with similar entries
@@ -319,47 +307,6 @@ async def mock_next_call(*args, **kwargs):
         # The exact behavior depends on which cached key is most similar
 
 
-class TestSimilarityThresholdFloor:
-    """The constructor must reject similarity thresholds below the safe floor.
-
-    Below ~0.85, crafting an input whose difflib ratio exceeds the threshold
-    against a legitimate cached key is trivial (small edits, common prefixes,
-    shared structural tokens). Accepting those values silently produces a
-    cache where one caller can hijack another caller's response.
-    """
-
-    @pytest.mark.parametrize("threshold", [0.0, 0.3, 0.5, 0.7, 0.84])
-    def test_below_floor_is_rejected(self, threshold):
-        with pytest.raises(ValueError, match="outside the safe range"):
-            CacheMiddleware(enabled_mode="always", similarity_threshold=threshold)
-
-    @pytest.mark.parametrize("threshold", [0.85, 0.9, 0.95, 1.0])
-    def test_at_or_above_floor_is_allowed(self, threshold):
-        mw = CacheMiddleware(enabled_mode="always", similarity_threshold=threshold)
-        assert mw._similarity_threshold == threshold  # noqa: SLF001
-
-    def test_threshold_above_one_is_rejected(self):
-        with pytest.raises(ValueError, match="outside the safe range"):
-            CacheMiddleware(enabled_mode="always", similarity_threshold=1.5)
-
-    def test_threshold_non_numeric_is_rejected(self):
-        with pytest.raises(ValueError, match="must be a number"):
-            CacheMiddleware(enabled_mode="always", similarity_threshold="high")  # type: ignore[arg-type]
-
-    @pytest.mark.parametrize("bad_bool", [True, False])
-    def test_threshold_bool_is_rejected(self, bad_bool):
-        """`isinstance(True, int)` is True in Python — reject bools explicitly
-        so a config with the wrong key type doesn't silently become 1.0 or 0.0."""
-        with pytest.raises(ValueError, match="got bool"):
-            CacheMiddleware(enabled_mode="always", similarity_threshold=bad_bool)  # type: ignore[arg-type]
-
-    @pytest.mark.parametrize("bad_value", [float("nan"), float("inf"), float("-inf")])
-    def test_threshold_non_finite_is_rejected(self, bad_value):
-        """NaN, +inf, -inf must be rejected before the range comparison."""
-        with pytest.raises(ValueError, match="must be finite"):
-            CacheMiddleware(enabled_mode="always", similarity_threshold=bad_value)
-
-
 class TestMaxEntriesLruEviction:
     """The cache must bound its size to prevent memory-exhaustion DoS.
 
@@ -372,24 +319,6 @@ async def test_default_max_entries_is_positive(self):
         mw = CacheMiddleware(enabled_mode="always", similarity_threshold=1.0)
         assert mw._max_entries > 0  # noqa: SLF001
 
-    def test_zero_max_entries_is_rejected(self):
-        with pytest.raises(ValueError, match="positive integer"):
-            CacheMiddleware(enabled_mode="always", similarity_threshold=1.0, max_entries=0)
-
-    def test_negative_max_entries_is_rejected(self):
-        with pytest.raises(ValueError, match="positive integer"):
-            CacheMiddleware(enabled_mode="always", similarity_threshold=1.0, max_entries=-5)
-
-    @pytest.mark.parametrize("bad_bool", [True, False])
-    def test_bool_max_entries_is_rejected(self, bad_bool):
-        """Same bool-as-int foot-gun protection as similarity_threshold."""
-        with pytest.raises(ValueError, match="positive integer"):
-            CacheMiddleware(
-                enabled_mode="always",
-                similarity_threshold=1.0,
-                max_entries=bad_bool,  # type: ignore[arg-type]
-            )
-
     async def test_cache_evicts_oldest_when_exceeding_max_entries(self, middleware_context):
         """Insert more unique entries than max_entries; verify size stays bounded."""
         mw = CacheMiddleware(