Skip to content

[bugfix] encoders: keep Qwen2.5-VL importable on transformers 5 - #1791

Merged
SolitaryThinker merged 2 commits into
hao-ai-lab:mainfrom
alanhuangyoo:fix/qwen2-5-vl-sliding-window-cache
Sep 25, 2026
Merged

SolitaryThinker merged 2 commits into
hao-ai-lab:mainfrom
alanhuangyoo:fix/qwen2-5-vl-sliding-window-cache

Conversation

@alanhuangyoo

Copy link
Copy Markdown
Contributor

Summary

fastvideo/models/encoders/qwen2_5_vl_custom.py cannot be imported on any transformers this
repo asks for. pyproject.toml requires transformers>=5.0.0; the module has, at module scope
and unguarded:

from transformers.cache_utils import Cache, DynamicCache, SlidingWindowCache, StaticCache
transformers 5.3.0
ImportError: cannot import name 'SlidingWindowCache' from 'transformers.cache_utils'

transformers 5.0 removed the cache-level class — a sliding window became a property of the
cache's layers (DynamicSlidingWindowLayer, StaticSlidingWindowLayer). Cache,
DynamicCache and StaticCache are all still there; only this one went.

transformers class SlidingWindowCache
4.57.1 yes
5.0.0, 5.3.0, 5.16.1 no

This is not a dead file. Reason1TextEncoder lives in registry.py:88-89, registered for
Qwen2_5_VLForConditionalGeneration, and is used by the Cosmos 2.5 and Kandinsky 5 pipeline
configs, so the failure lands on those pipelines.

It went unnoticed because fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py — which
imports this module and would have failed at collection — is not in the explicit test list
ci-macos-mlx.yml runs.

The change

Import the removed class behind try/except ImportError, matching how this file already
handles torch.distributed.tensor.Shard and the Qwen2.5-VL config classes:

try:
    from transformers.cache_utils import SlidingWindowCache
except ImportError:
    SlidingWindowCache = ()

isinstance(x, ()) is False, which is the right answer on transformers 5: with no such class,
no cache object can be an instance of one. The two call sites
(using_sliding_window_cache = isinstance(past_key_values, SlidingWindowCache) and the
config.sliding_window branch below it) then behave as they did before for anything transformers
5 can hand them.

On this encoder's own path the question does not arise either way — reason1.py:311 calls with
use_cache=False, and a cache is only built if use_cache and past_key_values is None, so
past_key_values is None there.

Not attempting to reconstruct the old behaviour from the new layer types: upstream Qwen2.5-VL
has had no _update_causal_mask and no SlidingWindowCache reference since 4.57 at the latest,
so this vendored copy has no upstream counterpart left to mirror. That is a bigger question than
making the module importable again.

Test

Added to the existing encoder test file:
test_sliding_window_cache_check_survives_transformers_5.

Load-bearing — same test file against a clean upstream/main worktree, same environment:

$ pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py     # upstream/main
ERROR fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py
Interrupted: 1 error during collection
  ImportError: cannot import name 'SlidingWindowCache' from 'transformers.cache_utils'

$ pytest fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py     # this branch
7 passed

Also confirmed from fastvideo.models.encoders.reason1 import Reason1TextEncoder imports again.

Lint, at the versions pinned in .pre-commit-config.yaml:

ruff 0.11.12   106 findings on this file before the change, 106 after — none new
yapf 0.43.0    no diff

(The 106 are pre-existing in this vendored file; I have not touched them.)

transformers 5.0 removed cache_utils.SlidingWindowCache; a sliding window is a
property of the cache's layers now (DynamicSlidingWindowLayer /
StaticSlidingWindowLayer). The import in qwen2_5_vl_custom was unguarded and at
module scope, so on transformers>=5.0.0 - what pyproject asks for - the module
could not be imported at all, taking Reason1TextEncoder with it.

Nothing is an instance of the removed class, so the fallback makes the two
isinstance() checks answer False instead of raising. This encoder runs with
use_cache=False, so past_key_values is None on that path regardless.
@mergify mergify Bot added type: bugfix Bug fix scope: infra CI, tests, Docker, build labels Aug 31, 2026
@mergify

mergify Bot commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

Merge Protections

🔴 1 of 1 protections blocking · waiting on 👀 reviews and 🤖 CI

Protection Waiting on
🔴 PR merge requirements 👀 reviews and 🤖 CI

🔴 PR merge requirements

Waiting for

  • #approved-reviews-by>=1
  • check-success=fastcheck-passed
  • check-success=full-suite-passed
This rule is failing.
  • #approved-reviews-by>=1
  • check-success=fastcheck-passed
  • check-success=full-suite-passed
  • check-success~=pre-commit
  • title~=(?i)^\[(feat|feature|bugfix|fix|refactor|perf|ci|doc|docs|misc|chore|kernel|new.?model|skill|skills|infra)\]

@mergify mergify Bot added the scope: model Model architecture (DiTs, encoders, VAEs) label Aug 31, 2026
@alanhuangyoo

Copy link
Copy Markdown
Contributor Author

Correction to this PR's description, and it matters for anyone trying to reproduce it.

The body says transformers 5 removed cache_utils.SlidingWindowCache. That is true of the versions I tested against and false of current ones — it came back. If you check on a recent 5.x the import works fine and this PR looks pointless, so here is the measured range:

transformers from transformers.cache_utils import SlidingWindowCache
5.0.0 ImportError
5.4.0 ImportError
5.5.0 ImportError
5.6.0 OK
5.7.0 OK
5.8.0 OK — <class 'transformers.cache_utils.StaticCache'>
5.12.1 OK — same alias
5.16.1 OK — same alias

It was removed in 5.0 and restored in 5.6 as an alias for StaticCache, not as a distinct class.

Two consequences:

  1. The bug is real but bounded. pyproject.toml declares transformers>=5.0.0, so 5.0–5.5 are inside the supported range, and on those the module-scope import fails and takes Reason1TextEncoder — and the Cosmos 2.5 and Kandinsky 5 pipelines — down with it. It is not "all of transformers 5".
  2. On 5.6+ this change is a strict no-op. The try succeeds, SlidingWindowCache binds to exactly what main binds it to, and the isinstance check is unchanged. The except ImportError arm only ever runs on 5.0–5.5.

That makes it safer than the description implies: it cannot change behaviour on any version where the import currently works.

The test asserts isinstance(DynamicCache(), SlidingWindowCache) is False, which holds in both regimes — () when absent, and StaticCache when present (a DynamicCache is not a StaticCache). Confirmed on 5.12.1:

SlidingWindowCache = <class 'transformers.cache_utils.StaticCache'>
isinstance(DynamicCache(), SlidingWindowCache) = False

Sorry for the imprecision — the original claim was written against the version I had, and I did not check whether it held across the declared floor.

Status: pre-commit, fastcheck-passed and the eight buildkite/pr-fastcheck/* lanes are green. Mergify additionally wants full-suite-passed and one approval; full-suite is gated on the ready label, which needs write permission.

@SolitaryThinker

Copy link
Copy Markdown
Collaborator

Pushed c9cd284: corrected the transformers version range in the cache-guard comment and made the new test able to fail.

Fixed

  • Comment said "transformers 5 dropped the cache-level class", which is only true for 5.0–5.5 — 5.6 restored the name as an alias for StaticCache (confirmed in 5.14.1, 5.15.0, 5.17.0: SlidingWindowCache = StaticCache). The comment now states the range, so nobody reads the fallback as covering all of transformers 5 (fastvideo/models/encoders/qwen2_5_vl_custom.py:36-40).
  • The added test could not fail in CI: the encoder lane resolves transformers ≥5.6, where the guarded import succeeds, so the except ImportError arm was never executed and isinstance(DynamicCache(), SlidingWindowCache) was trivially False. The test now pins the binding to transformers.cache_utils.SlidingWindowCache when that name exists, asserts the () sentinel when it does not, and keeps the isinstance check — so a fallback bound to something non-isinstance-safe, or a guard that shadows a name transformers still exports, now fails (fastvideo/tests/encoders/test_qwen2_5_vl_vision_dtype.py:100-124).

Verified

  • python3 -m py_compile on both changed files: OK.
  • All added lines ≤ 120 columns.
  • Static check of the assertions against transformers 5.15.0: DynamicCache(Cache) and StaticCache(Cache) are siblings, and SlidingWindowCache = StaticCache (cache_utils.py:2092), so both branches hold.
  • No local test run (this sandbox has no project env with the full dependency set); CI is the gate.

Left open

  • The except ImportError arm still is not executed by CI — forcing it needs a reload of the vendored module with the name hidden, which is a test-design choice rather than a mechanical fix. The strengthened assertions cover the fallback's contract instead.
  • except ImportError is broader than the failure it targets; getattr(transformers.cache_utils, "SlidingWindowCache", ()) would be narrower, but the try/except matches the two guards already in this file (Shard, Qwen2_5_VLConfig), so left as-is.
  • Dropping the shim and using StaticCache directly at :1289/:1387 would change behaviour for a plain StaticCache on 5.0–5.5 (sliding mask skipped when sequence_length <= target_length). That is a design decision beyond restoring importability, so not applied.
  • Test placement: it sits in a dtype-focused file; a test_qwen2_5_vl_import_compat.py would be more discoverable. Nit, not moved.
  • Not rebased: the branch is 32 commits behind main, but main has not touched either file, so the fix applies cleanly and the existing CI lanes stay meaningful.

Note on the PR description

  • The body's claim that the changed expression is off the encoder's path holds for Cosmos 2.5 (reason1.py:311 passes use_cache=False), but not for Kandinsky 5: it goes through the generic TextEncodingStage → Reason1TextEncoder.forward, which forwards use_cache=None to Qwen2_5_VLModel.forward, where it falls back to config.use_cache (True) and a DynamicCache is built. So isinstance(past_key_values, SlidingWindowCache) at :1289 is evaluated on that path — the False answer is load-bearing there, though unchanged by this PR.

@alanhuangyoo

Copy link
Copy Markdown
Contributor Author

Thanks — both fixes look right to me. I had read the 5.0 removal as covering all of 5.x and missed that 5.6 brought the name back as an alias, and you are right that the encoder lane resolves ≥5.6, so the fallback arm was never reached in CI.

Mergify still shows blocked; as far as I can tell it is waiting on a review, not on anything in the diff.

@SolitaryThinker
SolitaryThinker merged commit 9dd2a83 into hao-ai-lab:main Sep 25, 2026
3 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope: infra CI, tests, Docker, build scope: model Model architecture (DiTs, encoders, VAEs) type: bugfix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants