mirror of
https://github.com/pydantic/pydantic-ai-harness.git
synced 2026-07-21 10:55:35 +00:00
fix(cache_stability): validate thresholds; don't overclaim empty cache
Address CodeRabbit review on #327: - Reject out-of-range config at construction: collapse_ratio in 0.0..1.0, min_prefix_tokens and cache_ttl_seconds non-negative. A nonsensical value now fails fast instead of quietly distorting detection. - The model-switch note claimed the new provider's cache "is empty". The monitor only observes that the cache key changed and starts a fresh mark -- it does not establish the provider's cache state. Reword README + docstring.
This commit is contained in:
@@ -21,10 +21,10 @@ what the previous one cached; a large drop is the observable signature of a
|
||||
collapse.
|
||||
|
||||
Keying per provider and model means a mid-run model switch does not warn: a
|
||||
`FallbackModel` failover or a per-step model change hits a different provider's
|
||||
cache, which is empty, so it starts its own high-water mark. Marks are kept per key
|
||||
rather than reset, so switching back to an earlier model within its cache TTL still
|
||||
compares against that model's prefix.
|
||||
`FallbackModel` failover or a per-step model change uses a different cache key, so
|
||||
the monitor starts a fresh high-water mark for it instead of comparing against the
|
||||
previous model's. Marks are kept per key rather than reset, so switching back to an
|
||||
earlier model within its cache TTL still compares against that model's prefix.
|
||||
|
||||
A collapse has two shapes the monitor cannot tell apart, so the warning names both:
|
||||
the cacheable prefix moved, or the provider's cache expired under an unchanged prefix
|
||||
|
||||
@@ -78,8 +78,8 @@ class CacheStabilityMonitor(AbstractCapability[AgentDepsT]):
|
||||
back fewer than `collapse_ratio` of that established prefix, it emits a `CacheBustWarning`.
|
||||
|
||||
Keying per provider and model means a mid-run model switch does not warn: a `FallbackModel`
|
||||
failover or a per-step model change hits a different provider's cache, which is empty, so it
|
||||
starts a fresh high-water mark instead of collapsing an existing one. Marks are kept per key
|
||||
failover or a per-step model change uses a different cache key, so it starts a fresh high-water
|
||||
mark for that key instead of comparing against the previous model's. Marks are kept per key
|
||||
rather than reset, so switching back to an earlier model within its cache TTL still compares
|
||||
against that model's established prefix.
|
||||
|
||||
@@ -129,6 +129,14 @@ class CacheStabilityMonitor(AbstractCapability[AgentDepsT]):
|
||||
_step: int = field(default=0, compare=False)
|
||||
_last_time: datetime | None = field(default=None, compare=False)
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
if not 0.0 <= self.collapse_ratio <= 1.0:
|
||||
raise ValueError('collapse_ratio must be between 0.0 and 1.0')
|
||||
if self.min_prefix_tokens < 0:
|
||||
raise ValueError('min_prefix_tokens must be non-negative')
|
||||
if self.cache_ttl_seconds < 0:
|
||||
raise ValueError('cache_ttl_seconds must be non-negative')
|
||||
|
||||
async def for_run(self, ctx: RunContext[AgentDepsT]) -> AbstractCapability[AgentDepsT]:
|
||||
"""Reset per-run state (per-key high-water marks, step, timing) so each run is judged alone."""
|
||||
return replace(self, _marks={}, _step=0, _last_time=None)
|
||||
|
||||
@@ -216,3 +216,15 @@ async def test_small_gap_keeps_generic_expiry_hedge() -> None:
|
||||
message = str(record[0].message)
|
||||
assert 'e.g. a gap longer than the cache TTL' in message
|
||||
assert 'past the assumed' not in message
|
||||
|
||||
|
||||
def test_invalid_config_rejected() -> None:
|
||||
"""Out-of-range thresholds fail fast at construction rather than distorting detection."""
|
||||
with pytest.raises(ValueError, match='collapse_ratio'):
|
||||
CacheStabilityMonitor[None](collapse_ratio=1.5)
|
||||
with pytest.raises(ValueError, match='collapse_ratio'):
|
||||
CacheStabilityMonitor[None](collapse_ratio=-0.1)
|
||||
with pytest.raises(ValueError, match='min_prefix_tokens'):
|
||||
CacheStabilityMonitor[None](min_prefix_tokens=-1)
|
||||
with pytest.raises(ValueError, match='cache_ttl_seconds'):
|
||||
CacheStabilityMonitor[None](cache_ttl_seconds=-1.0)
|
||||
|
||||
Reference in New Issue
Block a user