DynamicCache.crop(0) retains the full KV cache and oversized negative crops retain stale tokens

[HoneyTyagii]

DynamicCache.crop(0) retains the full KV cache and oversized negative crops retain stale tokens

System Info

• Transformers source revision: 71554973d2c3123a23f2a1d6b7b63568b4e4df62
• OS: Windows 10 (10.0.26200)
• Python: 3.11.9
• PyTorch: 2.13.0+cpu

Who can help?

Generation/cache maintainers.

Information

The problem occurs in a minimal standalone script and does not depend on a model, dataset, GPU, or network access.

Reproduction

import torch
from transformers import DynamicCache

key = torch.arange(3, dtype=torch.float32).reshape(1, 1, 3, 1)

cache = DynamicCache()
cache.update(key, key.clone(), layer_idx=0)
cache.crop(0)
print(cache.get_seq_length())  # 3, expected 0

cache = DynamicCache()
cache.update(key, key.clone(), layer_idx=0)
cache.crop(-5)
print(cache.get_seq_length())  # 1, expected 0

DynamicLayer.crop handles every max_length <= 0 as a relative crop:

if max_length <= 0:
    max_length = self.get_seq_length() - abs(max_length)

For crop(0), this computes the current sequence length and triggers the early return, so nothing is removed. For a three-token cache, crop(-5) computes -2; Python slicing with [..., :-2, :] then retains one stale token instead of producing an empty cache.

Expected behavior

• crop(0) should empty initialized key and value tensors along the sequence dimension.
• A negative value should remove that many trailing tokens.
• Removing at least the current sequence length should either empty the cache or raise a clear validation error. It should never silently retain tokens.

This is important for rollback and cache-reuse flows, including speculative or custom generation. A complete rollback can silently preserve old key/value states, causing later decoding to attend to stale tokens or creating cache-position and attention-mask length mismatches.

Suggested implementation direction

1. Treat only max_length < 0 as relative removal, keeping zero as an absolute target.
2. Clamp the computed target to zero, or explicitly reject oversized negative removal.
3. Audit sliding-window, hybrid, and composite cache layers so tensors and length metadata remain consistent.
4. Add parameterized regression coverage for 2, 0, -1, -3, and -5, plus a full rollback followed by an append.

I searched existing open and closed issues for crop(0), DynamicCache.crop, negative cache crops, and stale cache tokens, and did not find a report for this behavior.

[niikkkhiil]

I'd like to work on this

[HoneyTyagii]

Sounds good, @niikkkhiil. Go ahead and work on it.

[zucchini-nlp]

Treat only max_length < 0 as relative removal, keeping zero as an absolute target.
Clamp the computed target to zero, or explicitly reject oversized negative removal.

These ones are reasonable changes. Afaik spec decoding wouldn't ask to crop beyond its current length or crop to zero, so it will not be breaking

Audit sliding-window, hybrid, and composite cache layers so tensors and length metadata remain consistent.

@Cyrilvallez has an open PR to support spec decoding better for sliding models, which might partially solve it. In any case let's wait for
Cyril for cache before opening a PR

[Cyrilvallez]

Actually, cropping to positive values, including zero, will very soon be deprecated and removed. This is because total length and cached states length are not correlated in general, e.g. for sliding window or linear attention conv states. So it does not make sense to crop to absolute values. What makes sense is removing a given number of tokens from the cached states. After #47447, we will internally only crop negatively. We can indeed clamp the value to 0 though, although it's not really a real issue