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()