misc: Update artifacts docstring and MetaInfoHash #1967
Conversation
WalkthroughThe PR adds two frozen dataclasses ( Changes
Sequence DiagramsequenceDiagram
participant Module as JIT Module
participant Cubin as cubin_loader
participant Store as File Store
rect rgb(200,220,255)
Note over Module,Cubin: Old flow (static hash)
Module->>Cubin: get_cubin(..., MetaInfoHash.ARTIFACT)
Cubin->>Store: fetch artifact by static hash
Store-->>Cubin: flashinferMetaInfo.h
Cubin-->>Module: header
end
rect rgb(220,255,200)
Note over Module,Cubin: New flow (dynamic via checksums.txt)
Module->>Cubin: get_cubin(..., CheckSumHash.ARTIFACT) // request checksums.txt
Cubin->>Store: fetch checksums.txt
Store-->>Cubin: checksums_bytes
Module->>Cubin: get_meta_hash(checksums_bytes)
Cubin->>Cubin: parse checksums_bytes → meta_hash
Cubin-->>Module: meta_hash
Module->>Cubin: get_cubin(..., meta_hash)
Cubin->>Store: fetch artifact by meta_hash
Store-->>Cubin: flashinferMetaInfo.h
Cubin-->>Module: header
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
jimmyzho
requested review from
Anerudhan,
aleozlx,
bkryu,
cyx-6,
joker-eph,
nvmbreughe,
wenscarl,
yongwww and
yzh119
as code owners
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
flashinfer/artifacts.py (2)
82-98: Docstring addition for ArtifactPath is clear.Paths read well; keep trailing “/” convention consistent across values.
Consider marking these as typing.Final for immutability.
114-135: CheckSumHash mapping looks fine.Good centralization of checksums and URL mapping.
- Consider adding a brief “MetaInfoHash deprecated in favor of checksums.txt” note above MetaInfoHash or remove it if unused to avoid confusion.
- Optionally freeze this container (e.g., module-level constants or typing.Final) to prevent accidental mutation.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
flashinfer/artifacts.py(2 hunks)flashinfer/jit/attention/modules.py(3 hunks)flashinfer/jit/cubin_loader.py(1 hunks)flashinfer/jit/fused_moe.py(2 hunks)flashinfer/jit/gemm/core.py(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
flashinfer/jit/gemm/core.py (2)
flashinfer/artifacts.py (2)
ArtifactPath(83-97)CheckSumHash(114-135)flashinfer/jit/cubin_loader.py (2)
get_cubin(191-210)get_meta_hash(139-147)
flashinfer/jit/attention/modules.py (2)
flashinfer/jit/cubin_loader.py (2)
get_cubin(191-210)get_meta_hash(139-147)flashinfer/artifacts.py (2)
ArtifactPath(83-97)CheckSumHash(114-135)
flashinfer/jit/fused_moe.py (2)
flashinfer/artifacts.py (2)
ArtifactPath(83-97)CheckSumHash(114-135)flashinfer/jit/cubin_loader.py (2)
get_cubin(191-210)get_meta_hash(139-147)
🪛 GitHub Actions: Build FlashInfer Docs
flashinfer/jit/attention/modules.py
[error] 31-31: SyntaxError: trailing comma not allowed without surrounding parentheses (modules.py, line 31). Likely an invalid import statement due to a trailing comma.
🪛 GitHub Actions: pre-commit
flashinfer/jit/attention/modules.py
[error] 31-31: Trailing comma not allowed without surrounding parentheses.
flashinfer/jit/cubin_loader.py
[error] 147-147: F541: f-string without any placeholders. Remove extraneous 'f' prefix.
🪛 Ruff (0.14.1)
flashinfer/jit/attention/modules.py
31-31: Trailing comma not allowed
(invalid-syntax)
flashinfer/jit/cubin_loader.py
147-147: Avoid specifying long messages outside the exception class
(TRY003)
147-147: f-string without any placeholders
Remove extraneous f prefix
(F541)
🔇 Additional comments (6)
flashinfer/jit/attention/modules.py (1)
1571-1602: FMHA meta-info retrieval looks good; relies on get_meta_hash returning str.Flow is correct. Once get_meta_hash decodes bytes and returns str, this block is fine.
flashinfer/jit/fused_moe.py (2)
20-24: Updated imports to CheckSumHash/get_meta_hash look correct.
183-193: Checksum → meta-hash flow LGTM; contingent on cubin_loader fix.Once get_meta_hash returns str, this block should work as intended.
flashinfer/jit/gemm/core.py (3)
23-23: Imports updated to CheckSumHash/get_meta_hash — OK.Also applies to: 33-34
364-369: trtllm_gen_gemm: checksum-driven header resolution LGTM.No issues; depends on get_meta_hash fix to return str.
Also applies to: 371-374
514-519: low-latency GEMM: same pattern LGTM.Looks consistent with above.
Also applies to: 521-524
| def get_meta_hash(checksums_bytes: bytes) -> str: | ||
| """ | ||
| Load the file from local cache (checksums.txt) | ||
| and get the hash of corresponding flashinferMetaInfo.h file | ||
| Parse the checksums.txt file and get the hash of corresponding flashinferMetaInfo.h file | ||
| """ | ||
| local_path = FLASHINFER_CUBIN_DIR / safe_urljoin(checksum_path, "checksums.txt") | ||
| with open(local_path, "r") as f: | ||
| for line in f: | ||
| sha256, filename = line.strip().split() | ||
| if ".h" in filename: | ||
| return sha256 | ||
| raise ValueError(f"Invalid path: checksums.txt not found in {checksum_path}") | ||
| for line in checksums_bytes.splitlines(): | ||
| sha256, filename = line.strip().split() | ||
| if ".h" in filename: | ||
| return sha256 | ||
| raise ValueError(f"Invalid checksums.txt, no flashinferMetaInfo.h found") | ||
|
|
There was a problem hiding this comment.
Bytes/str bug in get_meta_hash; also lint F541.
Currently iterates bytes, compares str ".h" to bytes, returns bytes; will TypeError/mismatch and fail hash compare. Also f-string has no placeholders.
Apply:
-def get_meta_hash(checksums_bytes: bytes) -> str:
- """
- Parse the checksums.txt file and get the hash of corresponding flashinferMetaInfo.h file
- """
- for line in checksums_bytes.splitlines():
- sha256, filename = line.strip().split()
- if ".h" in filename:
- return sha256
- raise ValueError(f"Invalid checksums.txt, no flashinferMetaInfo.h found")
+def get_meta_hash(checksums_bytes: bytes) -> str:
+ """
+ Parse checksums.txt content (bytes) and return the sha256 for the MetaInfo header.
+ """
+ text = checksums_bytes.decode("utf-8", "replace")
+ for line in text.splitlines():
+ parts = line.strip().split()
+ if len(parts) != 2:
+ continue
+ sha256, filename = parts # both str
+ name = filename.lower()
+ if name.endswith("metainfo.h"):
+ return sha256
+ raise ValueError("Invalid checksums.txt, no flashinferMetaInfo.h found")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_meta_hash(checksums_bytes: bytes) -> str: | |
| """ | |
| Load the file from local cache (checksums.txt) | |
| and get the hash of corresponding flashinferMetaInfo.h file | |
| Parse the checksums.txt file and get the hash of corresponding flashinferMetaInfo.h file | |
| """ | |
| local_path = FLASHINFER_CUBIN_DIR / safe_urljoin(checksum_path, "checksums.txt") | |
| with open(local_path, "r") as f: | |
| for line in f: | |
| sha256, filename = line.strip().split() | |
| if ".h" in filename: | |
| return sha256 | |
| raise ValueError(f"Invalid path: checksums.txt not found in {checksum_path}") | |
| for line in checksums_bytes.splitlines(): | |
| sha256, filename = line.strip().split() | |
| if ".h" in filename: | |
| return sha256 | |
| raise ValueError(f"Invalid checksums.txt, no flashinferMetaInfo.h found") | |
| def get_meta_hash(checksums_bytes: bytes) -> str: | |
| """ | |
| Parse checksums.txt content (bytes) and return the sha256 for the MetaInfo header. | |
| """ | |
| text = checksums_bytes.decode("utf-8", "replace") | |
| for line in text.splitlines(): | |
| parts = line.strip().split() | |
| if len(parts) != 2: | |
| continue | |
| sha256, filename = parts # both str | |
| name = filename.lower() | |
| if name.endswith("metainfo.h"): | |
| return sha256 | |
| raise ValueError("Invalid checksums.txt, no flashinferMetaInfo.h found") |
🧰 Tools
🪛 GitHub Actions: pre-commit
[error] 147-147: F541: f-string without any placeholders. Remove extraneous 'f' prefix.
🪛 Ruff (0.14.1)
147-147: Avoid specifying long messages outside the exception class
(TRY003)
147-147: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
In flashinfer/jit/cubin_loader.py around lines 139 to 148, fix the bytes/str
mismatch and the f-string lint: ensure checksums_bytes is decoded to text (e.g.,
decode to UTF-8 or accept str input), iterate over text lines, split each line
into sha256 and filename (use split(maxsplit=1)), check filename.endswith('.h')
(string comparison), return the sha256 as a str, and replace the f-string in the
raise with a normal string literal to avoid F541.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
flashinfer/jit/attention/modules.py (1)
1571-1602: Header filename casing inconsistent; can 404 on case‑sensitive storage.Here header_name is "flashInferMetaInfo" (capital I), while fused_moe uses "flashinferMetaInfo". Mismatch can fail HTTP fetch and include path resolution.
Align to one canonical name (recommend: "flashinferMetaInfo" to match typical artifact naming):
- header_name = "flashInferMetaInfo" + header_name = "flashinferMetaInfo"Optional: avoid hardcoding; derive the header basename from checksums (future-proof).
♻️ Duplicate comments (1)
flashinfer/jit/attention/modules.py (1)
31-31: Import fix looks good (trailing comma removed).Unblocks build/pre-commit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
flashinfer/artifacts.py(2 hunks)flashinfer/jit/attention/modules.py(3 hunks)flashinfer/jit/cubin_loader.py(1 hunks)flashinfer/jit/fused_moe.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- flashinfer/artifacts.py
🧰 Additional context used
🧬 Code graph analysis (2)
flashinfer/jit/attention/modules.py (2)
flashinfer/jit/cubin_loader.py (2)
get_cubin(192-211)get_meta_hash(139-148)flashinfer/artifacts.py (2)
ArtifactPath(83-98)CheckSumHash(115-137)
flashinfer/jit/fused_moe.py (2)
flashinfer/artifacts.py (2)
ArtifactPath(83-98)CheckSumHash(115-137)flashinfer/jit/cubin_loader.py (2)
get_cubin(192-211)get_meta_hash(139-148)
🪛 Ruff (0.14.1)
flashinfer/jit/cubin_loader.py
148-148: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy Docs
🔇 Additional comments (1)
flashinfer/jit/fused_moe.py (1)
20-24: Imports updated to CheckSumHash/get_meta_hash — LGTM.
| def get_meta_hash(checksums_bytes: bytes) -> str: | ||
| """ | ||
| Load the file from local cache (checksums.txt) | ||
| and get the hash of corresponding flashinferMetaInfo.h file | ||
| Parse the checksums.txt file and get the hash of corresponding flashinferMetaInfo.h file | ||
| """ | ||
| local_path = FLASHINFER_CUBIN_DIR / safe_urljoin(checksum_path, "checksums.txt") | ||
| with open(local_path, "r") as f: | ||
| for line in f: | ||
| sha256, filename = line.strip().split() | ||
| if ".h" in filename: | ||
| return sha256 | ||
| raise ValueError(f"Invalid path: checksums.txt not found in {checksum_path}") | ||
| checksums_lines = checksums_bytes.decode("utf-8").splitlines() | ||
| for line in checksums_lines: | ||
| sha256, filename = line.strip().split() | ||
| if ".h" in filename: | ||
| return sha256 | ||
| raise ValueError("Invalid checksums.txt, no flashinferMetaInfo.h found") |
There was a problem hiding this comment.
get_meta_hash: match MetaInfo header precisely; handle decoding/split robustly.
Current logic returns the first ".h" sha, which can select the wrong file and break header fetch. Also may ValueError on malformed lines and assume UTF‑8 without fallback.
Apply:
-def get_meta_hash(checksums_bytes: bytes) -> str:
- """
- Parse the checksums.txt file and get the hash of corresponding flashinferMetaInfo.h file
- """
- checksums_lines = checksums_bytes.decode("utf-8").splitlines()
- for line in checksums_lines:
- sha256, filename = line.strip().split()
- if ".h" in filename:
- return sha256
- raise ValueError("Invalid checksums.txt, no flashinferMetaInfo.h found")
+def get_meta_hash(checksums_bytes: bytes) -> str:
+ """
+ Parse checksums.txt content (bytes) and return the sha256 for the MetaInfo header.
+ """
+ text = checksums_bytes.decode("utf-8", errors="replace")
+ for line in text.splitlines():
+ parts = line.strip().split(maxsplit=1)
+ if len(parts) != 2:
+ continue
+ sha256, filename = parts # both str
+ if filename.lower().endswith("metainfo.h"):
+ return sha256
+ raise ValueError("No MetaInfo header entry in checksums.txt")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_meta_hash(checksums_bytes: bytes) -> str: | |
| """ | |
| Load the file from local cache (checksums.txt) | |
| and get the hash of corresponding flashinferMetaInfo.h file | |
| Parse the checksums.txt file and get the hash of corresponding flashinferMetaInfo.h file | |
| """ | |
| local_path = FLASHINFER_CUBIN_DIR / safe_urljoin(checksum_path, "checksums.txt") | |
| with open(local_path, "r") as f: | |
| for line in f: | |
| sha256, filename = line.strip().split() | |
| if ".h" in filename: | |
| return sha256 | |
| raise ValueError(f"Invalid path: checksums.txt not found in {checksum_path}") | |
| checksums_lines = checksums_bytes.decode("utf-8").splitlines() | |
| for line in checksums_lines: | |
| sha256, filename = line.strip().split() | |
| if ".h" in filename: | |
| return sha256 | |
| raise ValueError("Invalid checksums.txt, no flashinferMetaInfo.h found") | |
| def get_meta_hash(checksums_bytes: bytes) -> str: | |
| """ | |
| Parse checksums.txt content (bytes) and return the sha256 for the MetaInfo header. | |
| """ | |
| text = checksums_bytes.decode("utf-8", errors="replace") | |
| for line in text.splitlines(): | |
| parts = line.strip().split(maxsplit=1) | |
| if len(parts) != 2: | |
| continue | |
| sha256, filename = parts # both str | |
| if filename.lower().endswith("metainfo.h"): | |
| return sha256 | |
| raise ValueError("No MetaInfo header entry in checksums.txt") |
🧰 Tools
🪛 Ruff (0.14.1)
148-148: Avoid specifying long messages outside the exception class
(TRY003)
🤖 Prompt for AI Agents
In flashinfer/jit/cubin_loader.py around lines 139-148, the parser currently
returns the first ".h" entry and can crash on malformed lines or non-UTF8 bytes;
change it to decode checksums_bytes with errors='replace' (or 'ignore'),
normalize/remove BOM, iterate each line and split with maxsplit=1 to avoid
ValueErrors on extra whitespace, strip surrounding quotes from the filename and
use the basename, then only return the checksum when the filename exactly
matches "flashinferMetaInfo.h" (also validate checksum looks like a 64-char hex
string); skip malformed lines and raise a clear ValueError if no exact match is
found.
| # Check if checksums.txt exists in the cubin directory | ||
| checksum_path = f"{ArtifactPath.TRTLLM_GEN_BMM}/checksums.txt" | ||
| checksum = get_cubin(checksum_path, CheckSumHash.TRTLLM_GEN_BMM) | ||
| assert checksum, f"Failed to get checksums.txt from {checksum_path}" | ||
| meta_hash = get_meta_hash(checksum) | ||
|
|
||
| # use `get_cubin` to get "flashinferMetaInfo.h" | ||
| metainfo = get_cubin( | ||
| f"{include_path}/{header_name}.h", | ||
| MetaInfoHash.TRTLLM_GEN_BMM, | ||
| meta_hash, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
Checksum → MetaInfo flow is correct; ensure name parity across modules.
Logic is good, but depends on get_meta_hash correctly matching MetaInfo and consistent header_name across files. After fixing get_meta_hash and aligning header_name in attention, this path should be stable.
To verify naming consistency across the repo:
🏁 Script executed:
#!/bin/bash
# Find all header_name assignments and their values
rg -n -C1 -P 'header_name\s*=\s*["\']flash(?:infer|Infer)MetaInfo["\']'Length of output: 158
🏁 Script executed:
#!/bin/bash
# Search for header_name assignments across Python files
rg -n -C2 -e 'header_name\s*=\s*' --type py
# Search for MetaInfo occurrences across Python files
rg -n 'MetaInfo' --type pyLength of output: 4624
Unify header_name casing across modules
- In flashinfer/jit/attention/modules.py (e.g. lines 1574, 1588), change
"flashInferMetaInfo"to"flashinferMetaInfo"to match fused_moe.py and gemm/core.py. Update any artifacts.py entries and tests to use the same casing.
2 participants
📌 Description
Amendment to PR 1761, appending docstring to two artifactory path classes and deprecating need to update MetaInfoHash by directly accessing the checksum.txt file.
🔍 Related Issues
🚀 Pull Request Checklist
Thank you for contributing to FlashInfer! Before we review your pull request, please make sure the following items are complete.
✅ Pre-commit Checks
pre-commitby runningpip install pre-commit(or used your preferred method).pre-commit install.pre-commit run --all-filesand fixed any reported issues.🧪 Tests
unittest, etc.).Reviewer Notes
Summary by CodeRabbit
New Features
Refactor