From 4f0622a276d161d7a22f763caacaa5059706b227 Mon Sep 17 00:00:00 2001 From: Simon Pinfold Date: Tue, 18 Aug 2026 12:11:09 -0700 Subject: [PATCH] Skip every write for a reference the reset cannot classify A reference whose path is under no configured root is meant to be left alone entirely, but needs_verify was decided before the classification guard, so it was the one write of four that did not skip. Classification now comes first, which also stops an out-of-view row being counted as changed or unchanged when the reset did nothing with it either way. Stamping the version was also the only database access in the runner that could throw into its caller, so a transient lock while stamping aborted the seeder's whole scan -- prune, fast phase and enrichment -- over a step whose own writes were already committed. A failed stamp is now logged and leaves the step pending, like every other failure there. --- app/assets/semantics/__init__.py | 15 +++++-- app/assets/semantics/reproject_derived.py | 16 ++++---- .../assets_test/test_semantics_reset.py | 39 +++++++++++++++++++ 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/app/assets/semantics/__init__.py b/app/assets/semantics/__init__.py index 222ac4796..334943ae2 100644 --- a/app/assets/semantics/__init__.py +++ b/app/assets/semantics/__init__.py @@ -101,9 +101,18 @@ def run_pending_semantics_steps(interrupt_check: InterruptCheck | None = None) - ) return applied - with create_session() as session: - set_semantics_version(session, step.version) - session.commit() + try: + with create_session() as session: + set_semantics_version(session, step.version) + session.commit() + except Exception: + logging.exception( + "Asset semantics step %d (%s) finished but could not be stamped, " + "so it runs again; its own writes are already committed", + step.version, + step.description, + ) + return applied applied += 1 logging.info( diff --git a/app/assets/semantics/reproject_derived.py b/app/assets/semantics/reproject_derived.py index 0f24c6f57..ee899819d 100644 --- a/app/assets/semantics/reproject_derived.py +++ b/app/assets/semantics/reproject_derived.py @@ -129,14 +129,6 @@ def _reproject_batch( summary.absent_files += 1 continue - if unchanged: - summary.unchanged_files += 1 - else: - # Hand it to the verify path rather than re-read the file for hash and size. - summary.changed_files += 1 - if not row.needs_verify: - set_needs_verify.append(row.reference_id) - try: derived_tags = set( normalize_tags(get_path_derived_tags_from_path(row.file_path)) @@ -145,6 +137,14 @@ def _reproject_batch( summary.unclassified_paths += 1 continue + if unchanged: + summary.unchanged_files += 1 + else: + # Hand it to the verify path rather than re-read the file for hash and size. + summary.changed_files += 1 + if not row.needs_verify: + set_needs_verify.append(row.reference_id) + loader_path = compute_loader_path(row.file_path) if loader_path != row.loader_path: loader_paths[row.reference_id] = loader_path diff --git a/tests-unit/assets_test/test_semantics_reset.py b/tests-unit/assets_test/test_semantics_reset.py index 1d88c1192..f75329719 100644 --- a/tests-unit/assets_test/test_semantics_reset.py +++ b/tests-unit/assets_test/test_semantics_reset.py @@ -427,6 +427,24 @@ class TestFileState: "a file that is simply gone is the scanner's business, not unfinished work" ) + def test_unclassified_row_is_not_flagged_for_verify(self, session, comfy_dirs): + """needs_verify is a write like any other; it must skip out-of-view rows too.""" + _register( + session, + _write(comfy_dirs["elsewhere"], "model.safetensors"), + "ref-1", + mtime_ns=1, + ) + + summary = reproject_derived_state() + + session.expire_all() + assert session.get(AssetReference, "ref-1").needs_verify is False, ( + "the one write that did not skip made 'leaves it alone' untrue" + ) + assert summary.unclassified_paths == 1 + assert summary.changed_files == 0 + def test_path_outside_every_known_root_is_left_alone(self, session, comfy_dirs): path = _write(comfy_dirs["elsewhere"], "model.safetensors") _register( @@ -623,6 +641,27 @@ class TestRunner: ): assert run_pending_semantics_steps() == 0 + def test_stamp_failure_does_not_escape_to_the_caller(self, session, comfy_dirs): + """The seeder calls this; a transient lock here must not abort its scan.""" + _register( + session, + _write(comfy_dirs["checkpoints"], "model.safetensors"), + "ref-1", + loader_path=None, + ) + + with patch( + "app.assets.semantics.set_semantics_version", + side_effect=RuntimeError("database is locked"), + ): + assert run_pending_semantics_steps() == 0 + + session.expire_all() + assert get_semantics_version(session) == 0 + assert ( + session.get(AssetReference, "ref-1").loader_path == "model.safetensors" + ), "the step's own committed work survives a failed stamp" + def test_stamp_is_not_advanced_when_a_step_raises(self, session, comfy_dirs): def _explode(_interrupt_check): raise RuntimeError("step failed")