zlei6/ComfyUI
1
0
Fork 0
mirror of https://github.com/Comfy-Org/ComfyUI.git synced 2026-08-20 08:15:11 +08:00
Code Issues Packages Projects Releases Wiki Activity

chore: comment cleanup

Browse Source 
Comment-Gate: 44 quarantined
This commit is contained in:
Simon Pinfold
2026-08-17 23:32:55 -07:00
parent e5aa56ef8b
commit 41f4210938
10 changed files with 29 additions and 145 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
alembic_db/versions/0007_add_asset_semantics_version.py
View File

@@ -1,11 +1,6 @@
"""
Add asset_semantics_version table.
Alembic records the *shape* of the assets tables. This table records the
*meaning* of their contents: which generation of the derivation logic produced
the values currently stored in them. The two move independently, so they are
tracked independently.
Revision ID: 0007_add_asset_semantics_version
Revises: 0006_add_loader_path
Create Date: 2026-08-18

9
app/assets/database/models.py
View File

@@ -24,14 +24,7 @@ from app.database.models import Base
class AssetSemanticsVersion(Base):
"""Which generation of the derivation logic produced this database's rows.
Alembic tracks the *shape* of the assets tables; this tracks the *meaning*
of what is stored in them. A row can be structurally current and still hold
values a superseded rule computed, so the two versions move independently.
Reset steps in ``app.assets.semantics`` bring such rows forward and then
advance this stamp. Always a single row, keyed on id 1.
"""
"""Which generation of the derivation logic wrote these rows -- deliberately not the Alembic version, which tracks shape rather than meaning."""
__tablename__ = "asset_semantics_version"

24
app/assets/database/queries/semantics.py
View File

@@ -1,4 +1,3 @@
"""Queries backing the asset semantics reset (see ``app.assets.semantics``)."""
from dataclasses import dataclass
@@ -20,21 +19,17 @@ from app.assets.database.queries.common import (
from app.assets.database.queries.tags import ensure_tags_exist
from app.assets.helpers import get_utc_now
# The version table holds exactly one row; the id is a constant, not a sequence.
_VERSION_ROW_ID = 1
# Tags this subsystem derives from the filesystem rather than being told.
AUTOMATIC_TAG_ORIGIN = "automatic"
def get_semantics_version(session: Session) -> int:
"""Return the stored semantics version, or 0 if none has been stamped."""
row = session.get(AssetSemanticsVersion, _VERSION_ROW_ID)
return int(row.version) if row is not None else 0
def set_semantics_version(session: Session, version: int) -> None:
"""Stamp the semantics version. Caller commits."""
row = session.get(AssetSemanticsVersion, _VERSION_ROW_ID)
if row is None:
session.add(
@@ -50,7 +45,6 @@ def set_semantics_version(session: Session, version: int) -> None:
@dataclass(frozen=True)
class DerivedStateRow:
"""A file-backed reference and the state a reprojection may rewrite."""
reference_id: str
file_path: str
@@ -66,13 +60,7 @@ def get_file_backed_references_page(
after_id: str | None,
limit: int,
) -> list[DerivedStateRow]:
"""Return up to ``limit`` file-backed references ordered by id, after ``after_id``.
Keyset pagination on the primary key so a reprojection can walk the whole
table in bounded transactions without holding one open across the walk.
Soft-deleted references are included: their derived columns are as stale as
anyone else's, and an undelete would serve them.
"""
"""Soft-deleted references are deliberately included: their derived columns are as stale as anyone else's."""
query = (
sa.select(
AssetReference.id,
@@ -108,7 +96,6 @@ def get_file_backed_references_page(
def get_tags_by_reference(
session: Session, reference_ids: list[str]
) -> dict[str, dict[str, str]]:
"""Return {reference_id: {tag_name: origin}} for the given references."""
if not reference_ids:
return {}
@@ -129,7 +116,6 @@ def get_tags_by_reference(
def bulk_set_loader_paths(
session: Session, loader_paths: dict[str, str | None]
) -> None:
"""Set loader_path on the given references. Caller commits."""
if not loader_paths:
return
session.execute(
@@ -142,7 +128,6 @@ def bulk_set_loader_paths(
def bulk_add_automatic_tags(session: Session, links: list[tuple[str, str]]) -> None:
"""Attach (reference_id, tag_name) links with automatic origin. Caller commits."""
if not links:
return
@@ -171,12 +156,7 @@ def bulk_add_automatic_tags(session: Session, links: list[tuple[str, str]]) -> N
def bulk_remove_automatic_tags(session: Session, links: list[tuple[str, str]]) -> None:
"""Detach (reference_id, tag_name) links, but only automatic ones.
The origin is re-asserted in the statement rather than trusted from the
caller's snapshot, so a manual tag can never be removed by this path.
Caller commits.
"""
"""The origin is re-asserted below rather than trusted from the caller's snapshot."""
if not links:
return

4
app/assets/seeder.py
View File

@@ -549,9 +549,7 @@ class _AssetSeeder:
)
return
# Stale rows are brought forward before anything reads or extends
# them. A database already at the current semantics version costs a
# single row read here.
# Must precede the prune and the scan: both read and extend these rows.
run_pending_semantics_steps(interrupt_check=self._is_cancelled)
if self._prune_first:

27
app/assets/semantics/__init__.py
View File

@@ -1,18 +1,10 @@
"""Reset steps that bring stale asset rows forward to the current semantics.
Alembic migrates the *shape* of the assets tables. Nothing migrates their
*meaning*: a row whose columns are structurally current can still hold values
that a superseded rule computed, and the steady-state scan will not repair one,
because it only ever looks at paths it has not seen before.
A reset step closes that gap. Each step is numbered, applied in order from the
version stamped in the database up to ``CURRENT_SEMANTICS_VERSION``, and stamped
only once it finishes, so an interrupted run resumes rather than half-applying.
Steps must be idempotent: re-running one on rows it has already fixed changes
nothing.
The registry, not any individual step, is the durable part. A step is free to be
as broad or as surgical as its particular drift calls for.
Alembic migrates the shape of the assets tables; these steps migrate the meaning
of what is in them. A step must be idempotent, is applied in order from the
version stamped in the database, and is stamped only once it finishes, so an
interrupted run resumes rather than half-applying. How broad or surgical a step
is depends on the drift it repairs; the registry is what stays.
"""
import logging
@@ -51,14 +43,7 @@ CURRENT_SEMANTICS_VERSION = SEMANTICS_STEPS[-1].version
def run_pending_semantics_steps(interrupt_check: InterruptCheck | None = None) -> int:
"""Apply every reset step this database has not been stamped for.
A database already at ``CURRENT_SEMANTICS_VERSION`` costs one indexed row
read and nothing else -- no filesystem is touched to discover there is
nothing to do, which is the overwhelmingly common case.
Returns the number of steps applied.
"""
"""Returns steps applied. A database already at the current version costs one indexed row read."""
if not can_create_session():
return 0

58
app/assets/semantics/reproject_derived.py
View File

@@ -1,30 +1,17 @@
"""Semantics step 1: re-derive the reference state that comes from the path.
Three columns are pure functions of a file's location and whether it is there:
``loader_path``, the backend tags a path implies, and ``is_missing``. Every one
of them was computed once, when the reference was first written, by whatever
rules were current that day. ``loader_path`` in particular was added to existing
databases as a NULL column and no path has ever filled it in, so references
older than that column serve a null loader path forever.
Recomputes ``loader_path``, the backend tags a path implies, and ``is_missing``.
This step recomputes those three from the filesystem as it stands now. It reads
no file contents: ``verify_file_unchanged`` answers whether a file still matches
what the database recorded, and the only thing that answer is used for is
deciding *not* to touch content-derived state. Nothing here re-hashes, so a
library of untouched 500GB models costs one ``stat`` each.
It must keep reading no file contents, so an untouched model library costs one
``stat`` per file. That rules out repairing anything derived from content --
``hash``, ``size_bytes``, mime -- so a file that has changed underneath its row
goes to the existing ``needs_verify`` path instead.
What the step will not touch:
- ``hash`` and ``size_bytes`` -- content facts, unrecoverable without reading
the file. A file that has changed underneath its row is handed to the existing
``needs_verify`` path instead, exactly as the scanner would flag it.
- Anything a person chose: manual and upload-origin tags, ``user_metadata``,
``preview_id``, ``deleted_at``, ``job_id``, ``name``.
- References whose file is gone, or whose path is not under any root this
install currently knows about. Retiring the former is the scanner's job under
its own rules; the latter cannot be classified at all, and a misconfigured
``extra_model_paths.yaml`` must not be able to strip tags off assets that are
merely out of view.
It must also leave alone anything a person chose (manual and upload-origin tags,
``user_metadata``, ``preview_id``, ``deleted_at``, ``job_id``, ``name``),
references whose file is gone, and references whose path is under no root this
install currently knows about -- a misconfigured ``extra_model_paths.yaml`` must
not be able to strip tags off assets that are merely out of view.
"""
import logging
@@ -54,14 +41,11 @@ from app.assets.services.path_utils import (
)
from app.database.db import create_session
# Bounds each transaction. Small enough that an interrupt loses little work,
# large enough that the walk is not dominated by session setup.
_BATCH_SIZE = 500
@dataclass
class ReprojectionSummary:
"""What a reprojection pass did, for the log line."""
scanned: int = 0
unchanged_files: int = 0
@@ -89,12 +73,7 @@ class ReprojectionSummary:
def reproject_derived_state(
interrupt_check: InterruptCheck | None = None,
) -> ReprojectionSummary:
"""Re-derive path-derived state for every file-backed reference.
Walks the reference table in keyset-paginated batches, committing each one,
so a kill mid-walk leaves committed batches reprojected and the rest
untouched -- both states the step handles on its next run.
"""
"""Each batch commits on its own, so a kill mid-walk is safe to resume."""
summary = ReprojectionSummary()
vocabulary = get_path_derived_tag_vocabulary()
after_id: str | None = None
@@ -142,9 +121,7 @@ def _reproject_batch(
if unchanged:
summary.unchanged_files += 1
else:
# The file moved on from what the row records, so its hash, size and
# mime no longer describe it. Re-deriving those means reading the
# file, which this step does not do; flag it for the path that does.
# 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)
@@ -174,8 +151,8 @@ def _reproject_batch(
tags_to_remove.append((row.reference_id, tag_name))
if row.is_missing:
# The file is right there. Nothing else un-flags this: the scanner
# excludes already-missing references from its temp reconciliation.
# Nothing else un-flags this: the scanner excludes already-missing
# references from its temp reconciliation.
clear_missing.append(row.reference_id)
bulk_set_loader_paths(session, loader_paths)
@@ -191,12 +168,7 @@ def _reproject_batch(
def _classify_file(row: DerivedStateRow) -> tuple[bool, bool]:
"""Return (file is present, file still matches what the row recorded).
Mirrors the scanner's stat handling so the two agree about what counts as a
present file: a permission error means the file is there but unreadable, any
other OS error means treat it as gone.
"""
"""Mirrors the scanner's stat handling: permission denied means present-but-unreadable, any other OS error means gone."""
try:
stat_result = os.stat(row.file_path, follow_symlinks=True)
except FileNotFoundError:

8
app/assets/semantics/step.py
View File

@@ -1,4 +1,3 @@
"""What a semantics reset step is. See ``app.assets.semantics`` for the why."""
from dataclasses import dataclass
from typing import Callable
@@ -16,12 +15,7 @@ class SemanticsStepInterrupted(Exception):
@dataclass(frozen=True)
class SemanticsStep:
"""One numbered reset step.
``apply`` takes an optional interrupt check and returns a summary of what it
did, which is logged. It must be idempotent and must raise
``SemanticsStepInterrupted`` rather than return if it stops early.
"""
"""``apply`` must be idempotent, and must raise SemanticsStepInterrupted rather than return early."""
version: int
description: str

8
app/assets/services/path_utils.py
View File

@@ -326,13 +326,7 @@ def get_path_derived_tags_from_path(path: str) -> list[str]:
def get_path_derived_tag_vocabulary() -> set[str]:
"""Every tag get_path_derived_tags_from_path can emit under the current config.
Bounds which tags a re-derivation may take away: a stored automatic tag
inside this vocabulary that the current rules no longer emit was produced
by a superseded rule, while one outside it came from somewhere else and is
none of the derivation's business.
"""
"""Bounds which stored tags a re-derivation may take away; one outside this set came from elsewhere."""
vocabulary = {"input", "output", "temp", "models"}
vocabulary.update(_KNOWN_SUBFOLDER_TAGS)
for folder_name, _bases, _extensions in get_comfy_models_folders():

30
tests-unit/assets_test/test_semantics_reset.py
View File

@@ -1,9 +1,4 @@
"""Tests for the asset semantics reset (app/assets/semantics).
Runs standalone against in-memory SQLite:
pytest tests-unit/assets_test/test_semantics_reset.py --noconftest
"""
"""Tests for the asset semantics reset (app/assets/semantics)."""
import os
import tempfile
@@ -34,17 +29,12 @@ from app.assets.services.file_utils import get_mtime_ns
@pytest.fixture(autouse=True)
def autoclean_unit_test_assets():
"""Override the package autouse fixture; these tests need no server."""
"""Override parent autouse fixture - these tests don't need server cleanup."""
yield
@pytest.fixture
def session_factory():
"""A session factory over one shared in-memory database.
StaticPool keeps every session on the same connection, so the batching walk
sees its own committed writes across sessions.
"""
engine = create_engine(
"sqlite:///:memory:",
poolclass=StaticPool,
@@ -62,14 +52,12 @@ def session_factory():
@pytest.fixture
def session(session_factory) -> Session:
"""A session for the test's own reads and writes."""
with session_factory() as sess:
yield sess
@pytest.fixture
def comfy_dirs():
"""Point every asset root at a throwaway tree with one model category."""
with tempfile.TemporaryDirectory() as base:
dirs = {
name: Path(base) / name
@@ -118,10 +106,6 @@ def _register(
job_id: str | None = None,
tags: dict[str, str] | None = None,
) -> AssetReference:
"""Insert an Asset + AssetReference (+ tags as {name: origin}) and commit.
``asset_hash=""`` means "any unique hash"; assets.hash is unique-indexed.
"""
if asset_hash == "":
asset_hash = f"blake3:{ref_id}"
if mtime_ns is None:
@@ -168,7 +152,6 @@ def _tags(session: Session, ref_id: str) -> dict[str, str]:
def _snapshot(session: Session) -> list[tuple]:
"""Every field the reset could plausibly touch, for equality across runs."""
session.expire_all()
refs = session.execute(select(AssetReference).order_by(AssetReference.id)).scalars()
rows = [
@@ -195,7 +178,6 @@ def _snapshot(session: Session) -> list[tuple]:
class TestLoaderPathReprojection:
def test_null_loader_path_is_backfilled(self, session, comfy_dirs):
"""The drift 0006 left behind: a column added with no backfill."""
path = _write(comfy_dirs["checkpoints"], "flux/model.safetensors")
_register(session, path, "ref-1", loader_path=None)
@@ -224,7 +206,6 @@ class TestLoaderPathReprojection:
assert summary.loader_paths_rewritten == 0
def test_unloadable_extension_loses_its_loader_path(self, session, comfy_dirs):
"""The current rule gives no loader path to a file its category cannot load."""
path = _write(comfy_dirs["checkpoints"], "notes.txt")
_register(session, path, "ref-1", loader_path="notes.txt")
@@ -269,7 +250,6 @@ class TestHashPreservation:
def test_changed_file_keeps_its_hash_and_is_flagged_for_verify(
self, session, comfy_dirs
):
"""A file that moved on is handed to the existing verify path, not re-read."""
path = _write(comfy_dirs["checkpoints"], "big.safetensors")
_register(
session,
@@ -335,13 +315,11 @@ class TestIntentIsPreserved:
tags = _tags(session, "ref-1")
assert tags["favourite"] == "manual"
assert tags["uploaded"] == "upload"
# ...while the derived state around them was still brought forward.
assert ref.loader_path == "curated.safetensors"
def test_manual_tag_inside_the_derived_vocabulary_is_not_removed(
self, session, comfy_dirs
):
"""A person may tag an input file 'models'; that is their business."""
path = _write(comfy_dirs["input"], "photo.png")
_register(session, path, "ref-1", tags={"models": "manual"})
@@ -363,7 +341,6 @@ class TestTagReprojection:
}
def test_superseded_automatic_tag_is_removed(self, session, comfy_dirs):
"""An older rule tagged by directory alone; extensions now gate the tag."""
path = _write(comfy_dirs["checkpoints"], "model.safetensors")
_register(
session,
@@ -384,7 +361,6 @@ class TestTagReprojection:
def test_automatic_tag_outside_the_vocabulary_is_left_alone(
self, session, comfy_dirs
):
"""'missing' is automatic but not path-derived; the scanner owns it."""
path = _write(comfy_dirs["checkpoints"], "model.safetensors")
_register(session, path, "ref-1", tags={"missing": "automatic"})
@@ -424,7 +400,6 @@ class TestFileState:
assert summary.absent_files == 1
def test_path_outside_every_known_root_is_left_alone(self, session, comfy_dirs):
"""A root missing from the config must not strip tags off its assets."""
path = _write(comfy_dirs["elsewhere"], "model.safetensors")
_register(
session,
@@ -488,7 +463,6 @@ class TestIdempotence:
def test_statements_chunked_by_bind_param_limit_stay_correct(
self, session, comfy_dirs
):
"""The bind-param chunking must not drop or duplicate a tag."""
for index in range(4):
_register(
session,

1
tests-unit/seeder_test/test_seeder.py
View File

@@ -498,7 +498,6 @@ class TestSeederMarkMissing:
class TestSeederSemanticsReset:
"""The scan brings stale rows forward before it reads or extends them."""
def test_semantics_reset_runs_before_any_scan_work(
self, fresh_seeder: _AssetSeeder