From 17d55739bc22bd10a7857f30d0a93ce577f32476 Mon Sep 17 00:00:00 2001 From: Ehsan Barkhordar Date: Thu, 23 Jul 2026 11:37:27 +0000 Subject: [PATCH] fix(cli): match model files by relative path when verifying `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 #2100 --- kt-kernel/python/cli/commands/model.py | 81 ++++------- kt-kernel/python/cli/utils/model_verifier.py | 90 ++++++++---- .../per_commit/test_model_verifier_relpath.py | 136 ++++++++++++++++++ 3 files changed, 226 insertions(+), 81 deletions(-) create mode 100644 kt-kernel/test/per_commit/test_model_verifier_relpath.py diff --git a/kt-kernel/python/cli/commands/model.py b/kt-kernel/python/cli/commands/model.py index 1476ae776..ab4f4c45c 100644 --- a/kt-kernel/python/cli/commands/model.py +++ b/kt-kernel/python/cli/commands/model.py @@ -2401,7 +2401,11 @@ def fetch_hashes(): console.print() # Step 2 & 3: Calculate local SHA256 and compare (with Progress bar) - from kt_kernel.cli.utils.model_verifier import calculate_local_sha256 + from kt_kernel.cli.utils.model_verifier import ( + calculate_local_sha256, + compare_local_to_official, + list_local_model_files, + ) with Progress( SpinnerColumn(), @@ -2414,24 +2418,20 @@ def fetch_hashes(): # Step 2: Calculate local SHA256 hashes (no timeout) local_dir_path = Path(selected_model.path) - # Determine which files to hash + # Determine which files to hash. The remote hash set spans + # *.safetensors, *.json and *.py at any depth, keyed by repo-relative + # path, so the local scan recurses and keeps subdirectory files (#2100). + all_local_files = list_local_model_files(local_dir_path) if files_to_verify: - # Only hash files that need re-verification - clean_filenames = { - Path(f.replace(" (missing)", "").replace(" (hash mismatch)", "").strip()).name - for f in files_to_verify + # Only hash files that need re-verification, matched by relative path. + clean_relpaths = { + f.replace(" (missing)", "").replace(" (hash mismatch)", "").strip() for f in files_to_verify } - # Collect files matching *.safetensors, *.json, *.py - files_to_hash = [] - for pattern in ["*.safetensors", "*.json", "*.py"]: - files_to_hash.extend( - [f for f in local_dir_path.glob(pattern) if f.is_file() and f.name in clean_filenames] - ) + files_to_hash = [ + f for f in all_local_files if f.relative_to(local_dir_path).as_posix() in clean_relpaths + ] else: - # Collect all important files: *.safetensors, *.json, *.py - files_to_hash = [] - for pattern in ["*.safetensors", "*.json", "*.py"]: - files_to_hash.extend([f for f in local_dir_path.glob(pattern) if f.is_file()]) + files_to_hash = all_local_files total_files = len(files_to_hash) @@ -2452,27 +2452,21 @@ def local_hash_callback(msg: str): local_hashes = calculate_local_sha256( local_dir_path, - "*.safetensors", progress_callback=local_hash_callback, - files_list=files_to_hash if files_to_verify else None, + files_list=files_to_hash, ) progress.remove_task(hash_task_id) console.print(f" [green]✓ Calculated {len(local_hashes)} local file hashes[/green]") # Step 3: Compare hashes - # If re-verifying specific files, only compare those files + # If re-verifying specific files, only compare those files (matched by + # relative path so subdirectory files are handled correctly, #2100). if files_to_verify: - # Build set of clean filenames to verify - clean_verify_filenames = { - Path(f.replace(" (missing)", "").replace(" (hash mismatch)", "").strip()).name - for f in files_to_verify - } - # Filter official_hashes to only include files we're re-verifying hashes_to_compare = { filename: hash_value for filename, hash_value in official_hashes.items() - if Path(filename).name in clean_verify_filenames + if filename in clean_relpaths } else: # First-time verification: compare all files @@ -2480,35 +2474,16 @@ def local_hash_callback(msg: str): compare_task_id = progress.add_task("[blue]Comparing hashes...", total=len(hashes_to_compare)) - files_failed = [] - files_missing = [] - files_passed = 0 - - for filename, official_hash in hashes_to_compare.items(): - file_basename = Path(filename).name - - # Find matching local file - local_hash = None - for local_file, local_hash_value in local_hashes.items(): - if Path(local_file).name == file_basename: - local_hash = local_hash_value - break - - if local_hash is None: - files_missing.append(filename) - if verbose: - console.print(f" [red]✗ {file_basename} (missing)[/red]") - elif local_hash.lower() != official_hash.lower(): - files_failed.append(f"{filename} (hash mismatch)") - if verbose: - console.print(f" [red]✗ {file_basename} (hash mismatch)[/red]") - else: - files_passed += 1 - if verbose: - console.print(f" [green]✓ {file_basename}[/green]") + files_passed, files_missing, files_mismatched = compare_local_to_official(hashes_to_compare, local_hashes) + files_failed = [f"{f} (hash mismatch)" for f in files_mismatched] - progress.update(compare_task_id, advance=1) + if verbose: + for f in files_missing: + console.print(f" [red]✗ {f} (missing)[/red]") + for f in files_mismatched: + console.print(f" [red]✗ {f} (hash mismatch)[/red]") + progress.update(compare_task_id, completed=len(hashes_to_compare)) progress.remove_task(compare_task_id) # Build result diff --git a/kt-kernel/python/cli/utils/model_verifier.py b/kt-kernel/python/cli/utils/model_verifier.py index 175cd7049..c6d005bbb 100644 --- a/kt-kernel/python/cli/utils/model_verifier.py +++ b/kt-kernel/python/cli/utils/model_verifier.py @@ -8,9 +8,14 @@ import requests import os from pathlib import Path -from typing import Dict, Any, Literal, Tuple +from typing import Dict, Any, List, Literal, Tuple from concurrent.futures import ProcessPoolExecutor, as_completed +# File suffixes whose SHA256 the remote repository publishes and we therefore +# verify locally. Must stay in sync with fetch_model_sha256, which fetches +# hashes for *.safetensors (weights), *.json (configs) and *.py (model code). +VERIFIABLE_SUFFIXES = (".safetensors", ".json", ".py") + def _compute_file_sha256(file_path: Path) -> Tuple[str, str, float]: """ @@ -33,6 +38,47 @@ def _compute_file_sha256(file_path: Path) -> Tuple[str, str, float]: return file_path.name, sha256_hash.hexdigest(), file_size_mb +def list_local_model_files(local_dir: Path) -> List[Path]: + """List every verifiable model file under local_dir, searching recursively. + + The remote hash set (fetch_model_sha256) spans *.safetensors, *.json and *.py + at any depth, e.g. ``inference/config.json`` or ``encoding/tests/x.json``, so + the local scan must recurse and cover the same suffixes. A top-level, + weights-only glob reports healthy config/code files as missing (issue #2100). + """ + if not local_dir.exists(): + return [] + return sorted(f for f in local_dir.rglob("*") if f.is_file() and f.suffix in VERIFIABLE_SUFFIXES) + + +def compare_local_to_official( + official_hashes: Dict[str, str], local_hashes: Dict[str, str] +) -> Tuple[int, List[str], List[str]]: + """Compare locally computed hashes against the official remote hashes. + + Both dicts are keyed by repo-relative POSIX path (see calculate_local_sha256 + and fetch_model_sha256), so each file is matched by its full relative path. + This distinguishes files that share a basename across subdirectories, e.g. + ``config.json`` and ``inference/config.json`` (issue #2100); matching on the + basename alone collides them and mislabels one as missing or mismatched. + + Returns (files_passed, files_missing, files_mismatched), where files_missing + and files_mismatched are lists of the offending remote relative paths. + """ + files_passed = 0 + files_missing: List[str] = [] + files_mismatched: List[str] = [] + for filename, official_hash in official_hashes.items(): + local_hash = local_hashes.get(filename) + if local_hash is None: + files_missing.append(filename) + elif local_hash.lower() != official_hash.lower(): + files_mismatched.append(filename) + else: + files_passed += 1 + return files_passed, files_missing, files_mismatched + + def check_huggingface_connectivity(timeout: int = 5) -> Tuple[bool, str]: """ Check if HuggingFace is accessible. @@ -268,15 +314,23 @@ def calculate_local_sha256( # Process results as they complete for future in as_completed(future_to_file): completed_count += 1 + file_path = future_to_file[future] try: filename, sha256_hash, file_size_mb = future.result() - result[filename] = sha256_hash + # Key by path relative to local_dir (POSIX) so files that share a + # basename across subdirectories (e.g. config.json vs + # inference/config.json) stay distinct and line up with the remote + # keys, which are repo-relative paths. See issue #2100. + try: + key = file_path.relative_to(local_dir).as_posix() + except ValueError: + key = filename + result[key] = sha256_hash if progress_callback: progress_callback(f" [{completed_count}/{total_files}] ✓ {filename} ({file_size_mb:.1f} MB)") except Exception as e: - file_path = future_to_file[future] if progress_callback: progress_callback(f" [{completed_count}/{total_files}] ✗ {file_path.name} - Error: {str(e)}") @@ -808,7 +862,7 @@ def fetch_with_timeout(repo_type, repo_id, use_mirror, timeout): # Calculate local hashes and compare local_dir = Path(user_model.path) - files_to_hash = [f for f in local_dir.glob("*.safetensors") if f.is_file()] + files_to_hash = list_local_model_files(local_dir) with Progress( SpinnerColumn(), @@ -825,7 +879,7 @@ def hash_callback(msg): if "[" in msg and "/" in msg and "]" in msg and "✓" in msg: progress.advance(task) - local_hashes = calculate_local_sha256(local_dir, "*.safetensors", progress_callback=hash_callback) + local_hashes = calculate_local_sha256(local_dir, progress_callback=hash_callback, files_list=files_to_hash) progress.remove_task(task) console.print(f" [green]✓ Calculated {len(local_hashes)} local hashes[/green]") @@ -833,29 +887,9 @@ def hash_callback(msg): # Compare hashes task = progress.add_task("[blue]Comparing hashes...", total=len(official_hashes)) - - files_failed = [] - files_missing = [] - files_passed = 0 - - for filename, official_hash in official_hashes.items(): - file_basename = Path(filename).name - local_hash = None - - for local_file, local_hash_value in local_hashes.items(): - if Path(local_file).name == file_basename: - local_hash = local_hash_value - break - - if local_hash is None: - files_missing.append(filename) - elif local_hash.lower() != official_hash.lower(): - files_failed.append(f"{filename} (hash mismatch)") - else: - files_passed += 1 - - progress.advance(task) - + files_passed, files_missing, files_mismatched = compare_local_to_official(official_hashes, local_hashes) + files_failed = [f"{f} (hash mismatch)" for f in files_mismatched] + progress.update(task, completed=len(official_hashes)) progress.remove_task(task) console.print() diff --git a/kt-kernel/test/per_commit/test_model_verifier_relpath.py b/kt-kernel/test/per_commit/test_model_verifier_relpath.py new file mode 100644 index 000000000..9e21f8c20 --- /dev/null +++ b/kt-kernel/test/per_commit/test_model_verifier_relpath.py @@ -0,0 +1,136 @@ +"""Regression tests for `kt model verify` file matching (issue #2100). + +`kt model verify` fetches the remote SHA256 set (fetch_model_sha256), which spans +*.safetensors, *.json and *.py at any depth, keyed by repo-relative path +(e.g. inference/config.json). Before the fix the local scan only globbed +*.safetensors non-recursively and keyed hashes by basename, so 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. + +These tests are pure Python (temp files + hashing), no compiled kt_kernel needed. +model_verifier is imported by file location so the suite runs without a build. +""" + +import hashlib +import os +import sys +import unittest +from pathlib import Path + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) +from ci.ci_register import register_cpu_ci + +# Make model_verifier importable under a real top-level name so its worker +# function pickles across the ProcessPoolExecutor (spec-loading under a synthetic +# name would break pickling; the built package uses the real name in production). +_UTILS_DIR = Path(__file__).resolve().parents[2] / "python" / "cli" / "utils" +sys.path.insert(0, str(_UTILS_DIR)) +import model_verifier # noqa: E402 + +register_cpu_ci(est_time=10, suite="default") + + +def _sha256(data: bytes) -> str: + h = hashlib.sha256() + h.update(data) + return h.hexdigest() + + +def _write(root: Path, rel: str, data: bytes) -> None: + p = root / rel + p.parent.mkdir(parents=True, exist_ok=True) + p.write_bytes(data) + + +class TestModelVerifierRelpath(unittest.TestCase): + def setUp(self): + import tempfile + + self._tmp = tempfile.TemporaryDirectory() + self.root = Path(self._tmp.name) + # A healthy model mirroring the layout in issue #2100: weights, top-level + # config/code, and files nested under subdirectories. config.json exists + # both at the top level and under inference/ with DIFFERENT contents. + self.files = { + "model-00001-of-00001.safetensors": b"weights-shard-1", + "config.json": b'{"model_type":"deepseek_v4"}', + "generation_config.json": b'{"temperature":0.6}', + "model.safetensors.index.json": b'{"weight_map":{}}', + "tokenizer.json": b'{"version":"1.0"}', + "tokenizer_config.json": b'{"bos":""}', + "inference/config.json": b'{"tp":8,"note":"different from top-level"}', + "inference/model.py": b"# inference model code\n", + "encoding/encoding_dsv4.py": b"# encoding code\n", + "encoding/tests/test_input_1.json": b'{"case":1}', + } + # Remote hash set: keyed by repo-relative path, exactly as HF returns it. + self.official = {} + for rel, data in self.files.items(): + _write(self.root, rel, data) + self.official[rel] = _sha256(data) + + def tearDown(self): + self._tmp.cleanup() + + def test_calculate_local_sha256_keys_by_relative_path(self): + """Core regression: hashes are keyed by relative path, so a basename that + repeats across subdirectories (config.json vs inference/config.json) stays + distinct. Fails on the pre-fix code, which keyed by basename and collided + the two. Uses only calculate_local_sha256, which exists before the fix.""" + top = self.root / "config.json" + sub = self.root / "inference" / "config.json" + local = model_verifier.calculate_local_sha256(self.root, files_list=[top, sub]) + + self.assertIn("config.json", local) + self.assertIn("inference/config.json", local) + self.assertEqual(local["config.json"], _sha256(self.files["config.json"])) + self.assertEqual(local["inference/config.json"], _sha256(self.files["inference/config.json"])) + # The two must not have collapsed into one entry with a shared hash. + self.assertNotEqual(local["config.json"], local["inference/config.json"]) + + def test_list_local_model_files_recursive_all_patterns(self): + """The local scan finds every verifiable suffix at any depth.""" + found = {p.relative_to(self.root).as_posix() for p in model_verifier.list_local_model_files(self.root)} + self.assertEqual(found, set(self.files.keys())) + + def test_healthy_model_verifies_clean(self): + """The issue #2100 scenario: a healthy model (json, py, subdirs) reports + zero missing and zero mismatched.""" + local = model_verifier.calculate_local_sha256( + self.root, files_list=model_verifier.list_local_model_files(self.root) + ) + passed, missing, mismatched = model_verifier.compare_local_to_official(self.official, local) + self.assertEqual(missing, []) + self.assertEqual(mismatched, []) + self.assertEqual(passed, len(self.official)) + + def test_basename_collision_matched_by_relative_path(self): + """config.json and inference/config.json are matched to their own remote + entry, not each other's, so neither is a false mismatch.""" + local = model_verifier.calculate_local_sha256( + self.root, files_list=model_verifier.list_local_model_files(self.root) + ) + _, missing, mismatched = model_verifier.compare_local_to_official( + { + "config.json": self.official["config.json"], + "inference/config.json": self.official["inference/config.json"], + }, + local, + ) + self.assertEqual((missing, mismatched), ([], [])) + + def test_genuinely_missing_and_corrupt_files_still_detected(self): + """The fix must not turn verification into a rubber stamp: a deleted subdir + file is reported missing, and a tampered file is reported mismatched.""" + (self.root / "inference" / "model.py").unlink() + _write(self.root, "encoding/encoding_dsv4.py", b"# tampered content\n") + local = model_verifier.calculate_local_sha256( + self.root, files_list=model_verifier.list_local_model_files(self.root) + ) + _, missing, mismatched = model_verifier.compare_local_to_official(self.official, local) + self.assertEqual(missing, ["inference/model.py"]) + self.assertEqual(mismatched, ["encoding/encoding_dsv4.py"]) + + +if __name__ == "__main__": + unittest.main()