From 5b0468eaeed808e8b4f090bdc3811ed3e7f787b5 Mon Sep 17 00:00:00 2001 From: Vito Sansevero Date: Mon, 26 Oct 2026 22:47:13 -0700 Subject: [PATCH] fix(lora): fingerprint every scanned sidecar, kept or skipped (#362, phase 3) Discovery keeps every metadata file it saw (scanned_files, resolved and sorted; scanned is now its length). The fingerprint hashes each one's path, size, modification time and whether it was kept, so a skipped sidecar added, removed or edited between the review and Start refuses Start with plan-changed, as the review's counts promised. --- py/lora_import.py | 23 +++++----- py/lora_manager_layout.py | 20 ++++++--- tests/test_lora_import.py | 72 +++++++++++++++++++++++++++++++ tests/test_lora_manager_layout.py | 25 +++++++++++ 4 files changed, 124 insertions(+), 16 deletions(-) diff --git a/py/lora_import.py b/py/lora_import.py index e405d9db..23f053cf 100644 --- a/py/lora_import.py +++ b/py/lora_import.py @@ -12,9 +12,10 @@ last writes to the database: the sidecars (``discover``). In ``alongside`` mode a sidecar beside its model is kept when ``.safetensors`` exists; in ``centralized`` mode every sidecar under the sidecar root is kept when its ``file_path`` names - a model inside a LoRA root. ``fingerprint`` covers the kept sidecars (path, - size and modification time) and the layout; ``preview`` adds the counts a - review shows. Nothing is written and nothing is requested. + a model inside a LoRA root. ``fingerprint`` covers every scanned sidecar, + kept or skipped (path, size, modification time, kept or not, #362) and the + layout; ``preview`` adds the counts a review shows. Nothing is written + and nothing is requested. 2. **stage**: read each sidecar and build its record. Its LoraManager preview and example images are linked where they are (``local_media``); only a LoRA without any local image downloads its CivitAI example images into the @@ -69,9 +70,10 @@ def discover(lora_manager_path: Optional[str]) -> Discovery: def fingerprint(discovery: Discovery) -> str: - """What a review saw: the layout, then each kept sidecar's path, size and - modification time (a LoraManager save that changes unrelated settings - leaves it as it was).""" + """What a review saw: the layout, then every scanned sidecar, kept or + skipped, with its path, size, modification time and whether it was kept + (#362). A LoraManager save that changes unrelated settings leaves it as + it was.""" layout = discovery.layout digest = hashlib.sha256() for value in ( @@ -87,13 +89,14 @@ def fingerprint(discovery: Discovery) -> str: digest.update(f"{value}\n".encode("utf-8", "surrogatepass")) for name in discovery.unreadable: digest.update(f"unreadable\0{name}\n".encode("utf-8", "surrogatepass")) - for sidecar in discovery.sidecars: - path = sidecar.metadata_path + kept = {str(sidecar.metadata_path) for sidecar in discovery.sidecars} + for path in discovery.scanned_files: + flag = 1 if str(path) in kept else 0 try: stat = path.stat() - entry = f"{path}\0{stat.st_size}\0{stat.st_mtime_ns}\n" + entry = f"{path}\0{stat.st_size}\0{stat.st_mtime_ns}\0{flag}\n" except OSError: - entry = f"{path}\0missing\n" + entry = f"{path}\0missing\0{flag}\n" digest.update(entry.encode("utf-8", "surrogatepass")) return "sha256:" + digest.hexdigest() diff --git a/py/lora_manager_layout.py b/py/lora_manager_layout.py index dc3716e3..b9e81a37 100644 --- a/py/lora_manager_layout.py +++ b/py/lora_manager_layout.py @@ -126,13 +126,20 @@ class Sidecar: @dataclass(frozen=True) class Discovery: - """The sidecars kept, how many metadata files were seen, and those unread.""" + """The sidecars kept, every metadata file seen, and those unread.""" layout: LoraManagerLayout sidecars: List[Sidecar] - scanned: int + # Every ``*.metadata.json`` seen, kept or skipped: resolved, once each, + # sorted by ``str(path)``. The import's fingerprint covers all of them. + scanned_files: Tuple[Path, ...] unreadable: List[str] + @property + def scanned(self) -> int: + """How many metadata files were seen.""" + return len(self.scanned_files) + class _SettingsError(Exception): """The settings file cannot be used, for the reason it carries.""" @@ -482,13 +489,13 @@ def discover_sidecars(layout: LoraManagerLayout) -> Discovery: cannot see every folder must not replace the collection with part of it. """ if layout.error is not None: - return Discovery(layout, [], 0, []) + return Discovery(layout, [], (), []) if layout.mode == MODE_CENTRALIZED: folders = [layout.sidecar_root] else: folders = list(layout.lora_roots) seen = set() - scanned = 0 + scanned: List[Path] = [] unreadable: List[str] = [] sidecars = [] for folder in folders: @@ -498,12 +505,13 @@ def discover_sidecars(layout: LoraManagerLayout) -> Discovery: if key in seen: continue seen.add(key) - scanned += 1 + scanned.append(resolved) model = _model_for(resolved, layout, unreadable) if model is not None: sidecars.append(Sidecar(resolved, Path(os.path.realpath(model)))) sidecars.sort(key=lambda sidecar: str(sidecar.metadata_path)) - return Discovery(layout, sidecars, scanned, unreadable) + scanned.sort(key=str) + return Discovery(layout, sidecars, tuple(scanned), unreadable) # ── images ────────────────────────────────────────────────────────── diff --git a/tests/test_lora_import.py b/tests/test_lora_import.py index e3af65ed..6aaee2ed 100644 --- a/tests/test_lora_import.py +++ b/tests/test_lora_import.py @@ -959,6 +959,22 @@ class LoraImportRefusalTests(LoraImportAPITestCase): self.assertEqual(job.scope["failure"], "plan-changed") clear.assert_not_called() + async def test_start_refused_when_a_skipped_sidecar_changed(self): + # #362 CUJ 4: the review counted the orphan as skipped. + self._two_loras() + orphan = self.loras / "orphan.metadata.json" + orphan.write_text(json.dumps({"model_name": "orphan"})) + resp, plan = await self._post(PREVIEW) + self.assertEqual(resp.status, 200, plan) + self.assertEqual((plan["lora_files"], plan["skipped"]), (3, 1)) + before = _snapshot(self.api.db) + orphan.write_text(orphan.read_text() + " ") + resp, body = await self._post(APPLY, {"token": plan["token"]}) + self.assertEqual(resp.status, 409, body) + self.assertEqual(body["code"], "plan-changed") + self.assertEqual(self.api._jobs.list(), []) + self.assertEqual(_snapshot(self.api.db), before) + async def test_a_pair_saved_after_the_review_blocks_the_job(self): self._two_loras() resp, plan = await self._post(PREVIEW) @@ -1210,6 +1226,62 @@ class LoraImportStageTests(unittest.TestCase): ) self.assertNotEqual(lora_import.fingerprint(self.discover()), first) + # #362: a skipped sidecar is part of what the review counted. + + def _assert_fingerprint_follows(self, change): + first = lora_import.fingerprint(self.discover()) + self.assertEqual(lora_import.fingerprint(self.discover()), first) + change() + self.assertNotEqual(lora_import.fingerprint(self.discover()), first) + + def _assert_add_edit_remove_change_it(self, path, text): + self._assert_fingerprint_follows(lambda: path.write_text(text)) + self._assert_fingerprint_follows( + lambda: path.write_text(path.read_text() + " ") + ) + self._assert_fingerprint_follows(path.unlink) + + def _centralized(self): + sidecars = self.tmp / "sidecars" + sidecars.mkdir() + self.settings_file( + { + "sidecar_storage_mode": "centralized", + "sidecar_storage_path": str(sidecars), + } + ) + model = self.loras / "kept.safetensors" + model.write_bytes(b"m") + (sidecars / "kept.metadata.json").write_text( + json.dumps({"file_path": str(model)}) + ) + return sidecars + + def test_fingerprint_follows_a_centralized_sidecar_whose_model_is_missing(self): + sidecars = self._centralized() + gone = self.loras / "gone.safetensors" + self._assert_add_edit_remove_change_it( + sidecars / "gone.metadata.json", json.dumps({"file_path": str(gone)}) + ) + self.assertEqual(len(self.discover().sidecars), 1) + + def test_fingerprint_follows_a_checkpoint_sidecar_outside_the_lora_roots(self): + sidecars = self._centralized() + checkpoints = self.tmp / "checkpoints" + checkpoints.mkdir() + ckpt = checkpoints / "base.safetensors" + ckpt.write_bytes(b"c") + self._assert_add_edit_remove_change_it( + sidecars / "base.metadata.json", json.dumps({"file_path": str(ckpt)}) + ) + + def test_fingerprint_follows_an_alongside_sidecar_without_its_model(self): + _write_metadata(self.loras, "kept") + self._assert_add_edit_remove_change_it( + self.loras / "orphan.metadata.json", json.dumps({"model_name": "orphan"}) + ) + self.assertEqual(len(self.discover().sidecars), 1) + def test_fingerprint_follows_a_missing_file_between_stat_calls(self): meta = _write_metadata(self.loras, "one") discovery = self.discover() diff --git a/tests/test_lora_manager_layout.py b/tests/test_lora_manager_layout.py index 52003fa2..1d209a38 100644 --- a/tests/test_lora_manager_layout.py +++ b/tests/test_lora_manager_layout.py @@ -501,6 +501,31 @@ class CentralizedDiscoveryTests(LayoutTestCase): self.assertEqual(discovery.scanned, 3) self.assertEqual(discovery.sidecars, []) + def test_every_scanned_file_is_listed_resolved_and_sorted(self): + # #362: the fingerprint covers every scanned sidecar, kept or skipped. + ckpt = self.model(self.checkpoints, "base") + alpha = self.model(self.loras, "alpha") + self.sidecar(self.sidecars / "z", "base", {"file_path": str(ckpt)}) + self.sidecar(self.sidecars / "a", "alpha", {"file_path": str(alpha)}) + self.sidecar(self.sidecars / "m", "nopath", {"model_name": "x"}) + discovery = discover_sidecars(self.load()) + files = discovery.scanned_files + self.assertIsInstance(files, tuple) + self.assertEqual(len(files), 3) + self.assertEqual(discovery.scanned, len(files)) + self.assertEqual([str(f) for f in files], sorted(str(f) for f in files)) + self.assertEqual( + [f.name for f in files], + [ + f"alpha{METADATA_SUFFIX}", + f"nopath{METADATA_SUFFIX}", + f"base{METADATA_SUFFIX}", + ], + ) + for path in files: + self.assertEqual(str(path), os.path.realpath(path)) + self.assertEqual(len(discovery.sidecars), 1) + def test_leftover_sidecars_beside_models_are_not_scanned(self): model = self.model(self.loras, "alpha") self.sidecar(self.loras, "alpha", {"file_path": str(model)})