Skip to content

fix(types): suppress mypy false-positive on HIP-gated cascade call - #236

Merged
demandal25 merged 2 commits into
AMD-Ecosystem:amd-integrationfrom
demandal25:fix-ci-pre-commit-and-docs
May 20, 2026
Merged

fix(types): suppress mypy false-positive on HIP-gated cascade call#236
demandal25 merged 2 commits into
AMD-Ecosystem:amd-integrationfrom
demandal25:fix-ci-pre-commit-and-docs

Conversation

@demandal25

@demandal25 demandal25 commented May 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Suppresses a mypy call-overload error at flashinfer/cascade.py:547 that has been failing pre-commit on every PR since cascade attention landed (#221), with a localized # type: ignore[call-overload] and an explanatory comment.

What changed

  • flashinfer/cascade.py:547 — Added # type: ignore[call-overload] to the _HIP_FUSED_CASCADE branch's call to wrapper.run(..., partial_state=(out, lse)), plus a two-line comment explaining the runtime aliasing that mypy cannot see.

No behavior change. mypy now passes.

Root cause

cascade.py imports BatchPrefillWithPagedKVCacheWrapper from flashinfer.prefill (the CUDA module). At runtime on HIP, flashinfer/__init__.py:262 does:

sys.modules["flashinfer.prefill"] = sys.modules["flashinfer.prefill_rocm"]

so wrapper is actually the HIP variant, whose run() accepts partial_state. mypy can't follow that runtime sys.modules swap — it always sees the CUDA prefill.py overloads, which don't declare partial_state — so it errors with:

No overload variant of "run" of "BatchPrefillWithPagedKVCacheWrapper" matches argument types

This was the only mypy error in the tree, so every PR's pre-commit check has shipped red since #221.

Design choice: minimal type: ignore vs broader refactor

Option Cost What it buys
This PR: # type: ignore[call-overload] + comment 4 lines (3 added) mypy passes; runtime semantics unchanged
Move HIP-fused branch into a HIP-side helper + cast(HIPWrapper, wrapper) + fix HIP @overloads ~10 lines, conditional import dance mypy passes; cast documents intent slightly better; HIP @overloads match impl
Add partial_state to CUDA @overloads with a runtime raise NotImplementedError ~6 lines, CUDA-side surface change API parity at type level

Semantically, cast(HIPWrapper, wrapper) and # type: ignore[call-overload] say the same thing to mypy — "trust me, this call is valid" — and carry identical runtime risk.

Reasons for the minimal fix in this PR:

  1. _HIP_FUSED_CASCADE is opt-in and default off (os.environ.get("FLASHINFER_HIP_FUSED_CASCADE", "0") == "1"). It's an experimental kernel-fusion optimization. A heavier mypy ceremony for an opt-in experimental branch is poor ROI.
  2. The underlying architectural wart is _HIP_FUSED_CASCADE itself, not its visible type symptom. Routing through a HIP helper or adding cast tidies the symptom but doesn't remove the leak — cascade.py would still need to know HIP exists.
  3. Scope. This PR exists to unblock CI for every other PR. Rolling in a structural refactor grows the review surface and the chance of merge conflicts with in-flight work.

A future PR that promotes FLASHINFER_HIP_FUSED_CASCADE from experimental to default-on is the right moment to revisit this. The correct fix at that point is probably exposing the fusion through a uniform run_and_merge(...) method on both backends (CUDA's variant being two kernel launches), not adding partial_state to CUDA's signature.

Why not also fix partial_state in prefill_rocm.py's @overloads?

The HIP wrapper's @overload declarations also lack partial_state, which is a latent contract gap (impl signature and overloads don't match). But cascade.py imports from the CUDA prefill.py module, so mypy never checks against the HIP overloads — fixing them has zero effect on this CI failure. Out of scope here; can be cleaned up alongside the future refactor above.

Test plan

  • pre-commit run --all-files — all hooks pass (mypy included)
  • pytest -n auto --reruns 2 -m "not slow" — full fast suite passes (no behavior change expected)
  • CI: pre-commit check goes green

Copilot AI review requested due to automatic review settings May 20, 2026 14:26
cascade.py:547 calls `wrapper.run(..., partial_state=(out, lse))` inside
the `_HIP_FUSED_CASCADE` branch. At runtime on HIP,
flashinfer/__init__.py:262 aliases sys.modules["flashinfer.prefill"] to
prefill_rocm, whose run() accepts partial_state. mypy can't follow that
runtime swap and only sees prefill.py (CUDA), whose run() doesn't have
partial_state — so it errors with "No overload variant of run matches".

Targeted `# type: ignore[call-overload]` with a comment explaining the
runtime aliasing. No change to either wrapper's typed contract: adding
partial_state to CUDA's @overload would misleadingly suggest CUDA
implements a HIP-only feature.

This unblocks pre-commit on every PR.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@demandal25
demandal25 force-pushed the fix-ci-pre-commit-and-docs branch from 3c7d385 to d4e8193 Compare May 20, 2026 14:34
@demandal25 demandal25 changed the title ci: fix pre-commit (mypy) and Read the Docs builds failing on every PR fix(types): suppress mypy false-positive on HIP-gated cascade call May 20, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes two repository-wide CI failures unrelated to individual PR content: mypy errors in pre-commit for a HIP-only code path, and Read the Docs builds failing due to attempting an editable install that requires CUDA/ROCm toolchains.

Changes:

  • Add a targeted # type: ignore[call-overload] on a HIP-gated call site where static typing can’t follow a runtime module alias swap.
  • Make docs/conf.py resilient to import flashinfer failing (e.g., in RTD) by falling back to a default version string.
  • Add a minimal .readthedocs.yaml to build docs without pip install -e . and without GPU toolchains.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

File Description
flashinfer/cascade.py Adds a mypy suppression for a HIP-only call that uses a runtime aliasing pattern mypy can’t model.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@demandal25
demandal25 merged commit fe77c3d into AMD-Ecosystem:amd-integration May 20, 2026
1 check passed
@demandal25
demandal25 deleted the fix-ci-pre-commit-and-docs branch May 20, 2026 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants