fix(cli): match model files by relative path when verifying - #2104
fix(cli): match model files by relative path when verifying#2104ebarkhordar wants to merge 1 commit into
Conversation
`kt model verify` fetches the remote SHA256 set for *.safetensors, *.json and *.py at any depth, keyed by repo-relative path, but the local scan only globbed *.safetensors non-recursively and keyed hashes by basename. On a healthy model every config/code file, and every file in a subdirectory, was reported "missing" and the model was flagged as potentially corrupted. Enumerate local files recursively over the same three suffixes and match each remote entry by its full relative path. This also distinguishes files that share a basename across directories, e.g. config.json vs inference/config.json. Fixes kvcache-ai#2100
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
No rush on this one. The repo's CI has not run this branch: both workflow runs on the head commit (Book-CI, Deploy) are held pending approval, so the only evidence is local, where |
|
#2142 fixes the other half of #2100 in this same file, and the two interact on the reporting side, so the merge order is worth choosing deliberately. The diffs do not overlap. #2142 changes one loop in The interaction, read off the two diffs rather than run: on a first verify both paths hash only Merging #2142 first, or both together, avoids that. Happy to rebase this on top of it if that is easier. |
What does this PR do?
Fixes #2100
kt model verifyreports every.jsonand.pyfile, and every file in asubdirectory, as
missingon a healthy model, then flags the model aspotentially corrupted (
sha256_status: failed). In the issue, 16 present fileswere called missing (5 top-level configs plus 11 files under
encoding/andinference/); only the 46 weight shards passed.Root cause
fetch_model_sha256returns the remote hash set for*.safetensors,*.jsonand
*.pyat any depth, keyed by repo-relative path (e.g.inference/config.json).The local side did not mirror that:
*.safetensors(in theverify_modelcommand flow it even built the correct three-pattern list, then discarded it via
files_list=files_to_hash if files_to_verify else None), so no.json/.pyhash existed to match against.
encoding/andinference/werenever scanned.
calculate_local_sha256keyed results by basename and the comparison matched bybasename, so
config.jsonandinference/config.jsoncollided into one entry.The fix
list_local_model_files()enumerates local files recursively over the samethree suffixes the remote set uses.
calculate_local_sha256()keys each hash by the path relative to the modeldirectory (POSIX), so same-named files in different directories stay distinct.
compare_local_to_official()matches each remote entry by its full relativepath. Both reachable callers (
kt model verifyand the pre-run/pre-quantpre_operation_verification) now use these, replacing three near-identicalbasename-matching loops.
Invariant: a file verifies iff its repo-relative path is present locally with a
matching hash; basename is never used for matching.
Verification
Reproduced and fixed in a clean
python:3.11-slimcontainer against HEADd1a3ed8.model_verifier.pyis pure stdlib +requests, so the tests runwithout building the extension.
New tests in
kt-kernel/test/per_commit/test_model_verifier_relpath.py(registeredfor the CPU
defaultsuite):test_calculate_local_sha256_keys_by_relative_path: withconfig.jsonandinference/config.jsonpresent, both relative-path keys are produced withdistinct hashes. Fails on the unfixed code (the two collapse into one
config.jsonentry), passes here.test_list_local_model_files_recursive_all_patterns: the scan finds every.safetensors/.json/.pyfile at any depth.test_healthy_model_verifies_clean: the issue [Bug] 无法正确识别模型文件 #2100 layout (top-level configs +encoding/+inference/) reports zero missing and zero mismatched.test_basename_collision_matched_by_relative_path:config.jsonandinference/config.jsoneach match their own remote entry.test_genuinely_missing_and_corrupt_files_still_detected: a deleted subdir fileis still reported missing and a tampered file is still reported mismatched, so
the change does not weaken verification.
What I did not verify: I did not run against a live HuggingFace/ModelScope repo
(no network in the sandbox); the remote side is exercised through the documented
relative-path key format that
fetch_model_sha256returns.Two unused functions in the same module,
verify_model_integrityandverify_model_integrity_with_progress, are imported but never called; I left themuntouched to keep this change focused on the reachable verify paths.
Before submitting