diff --git a/core/registry.py b/core/registry.py index 0e78ab8f..85b1f75b 100644 --- a/core/registry.py +++ b/core/registry.py @@ -52,7 +52,9 @@ class Registry: for _spec, fn in self.providers(capability): try: answer = fn(*args, **kwargs) - except Exception: # noqa: BLE001 + except Exception as exc: # noqa: BLE001 + from . import log + log.failed(f"{capability} provider {_spec.id}", exc) # skipped, but never silently continue if answer is not None: return answer diff --git a/core/tests/test_module_imports.py b/core/tests/test_module_imports.py index fa5a2f68..b2324ad4 100644 --- a/core/tests/test_module_imports.py +++ b/core/tests/test_module_imports.py @@ -64,26 +64,47 @@ def _module_dirs(): return {(p.parent.parent.name, p.parent.name) for p in MODULES.glob("*/*/__init__.py") if "\nID = " in "\n" + p.read_text()} -def _other_module_imports(path: Path): - """Imports at MODULE level that reach into a different module: `from ...group.other import` (three dots), or a - sibling module of the same group (`from ..other import`, two dots). Plain shared files are fine.""" - parts = path.relative_to(MODULES).parts - if len(parts) < 3: - return [] - own = parts[:2] - mods = _module_dirs() +def _import_nodes(body): + """Import statements that run when the file is imported: top level, and inside module-level try / if / with + blocks. Function and class bodies run later, on demand, and are where a guarded import may live.""" + for node in body: + if isinstance(node, (ast.Import, ast.ImportFrom)): + yield node + elif isinstance(node, (ast.Try, ast.If, ast.With)): + for part in ("body", "orelse", "finalbody"): + yield from _import_nodes(getattr(node, part, [])) + for handler in getattr(node, "handlers", []): + yield from _import_nodes(handler.body) + + +def _other_module_imports(path: Path, mods=None, root: Path = MODULES): + """Imports that run at import time and reach into a module other than the file's own: `from ...group.other`, + `from ..other` (a sibling module), `from ...group import other`, `from .. import other`, or an absolute + `modules.group.other`. Plain shared files are fine to import, but a shared file may not import a module either.""" + parts = path.relative_to(root).parts + mods = _module_dirs() if mods is None else mods + group = parts[0] + own = parts[:2] if len(parts) >= 3 else None # a file at group level belongs to no module found = [] - for node in ast.parse(path.read_text()).body: # module level only: a guarded import inside a function is how a module may ask another for help - if isinstance(node, ast.ImportFrom) and node.module: - target = node.module.split(".") - if node.level >= 3 and len(target) >= 2: - hit = tuple(target[:2]) != own and tuple(target[:2]) in mods + for node in _import_nodes(ast.parse(path.read_text()).body): + names = [a.name for a in node.names] + if isinstance(node, ast.Import): + targets = [tuple(n.split(".")[1:3]) for n in names if n.split(".")[0] == "modules"] + else: + mod = (node.module or "").split(".") if node.module else [] + if node.level == 0: + targets = [tuple(mod[1:3])] if mod[:1] == ["modules"] else [] + elif node.level >= 3: + targets = [tuple(mod[:2])] if len(mod) >= 2 else [(mod[0], n) for n in names] if mod else [] elif node.level == 2: - hit = (own[0], target[0]) in mods and target[0] != own[1] + targets = [(group, mod[0])] if mod else [(group, n) for n in names] + elif node.level == 1 and own is None: # a group-level helper: its `.x` is a sibling of the modules + targets = [(group, mod[0])] if mod else [(group, n) for n in names] else: - hit = False - if hit: - found.append((node.lineno, f"from {'.' * node.level}{node.module} import")) + targets = [] + for t in targets: + if len(t) == 2 and t in mods and t != own: + found.append((node.lineno, f"imports module {t[0]}/{t[1]}")) return found @@ -94,3 +115,26 @@ def test_a_module_does_not_depend_on_another_module_at_import(path): f"{path.relative_to(MODULES)} imports another module at line {offences[0][0]} ({offences[0][1]}). " "If that module fails to import, this one would be lost with it. Import inside the function that needs it, " "guarded, or ask through PROVIDES / USES.") + + +def test_the_guard_sees_every_way_to_import_another_module_at_load(tmp_path): + """Positive cases: the guard has to be shown to fail, or it only proves it passes.""" + for g, m in (("sampling", "a"), ("sampling", "b"), ("loaders", "c")): + d = tmp_path / g / m + d.mkdir(parents=True) + (d / "__init__.py").write_text('ID = "x"\n') + mods = {("sampling", "a"), ("sampling", "b"), ("loaders", "c")} + cases = { + "from ..b import x": True, "from ...loaders.c import x": True, "from .. import b": True, + "from ...loaders import c": True, "import modules.sampling.b": True, "from modules.sampling.b import x": True, + "try:\n from ..b import x\nexcept ImportError:\n x = None": True, "if True:\n from ..b import x": True, + "from . import helper": False, "from ..._core import log": False, "from ..prompt_taste import x": False, + "def f():\n from ..b import x": False, + } + for src, bad in cases.items(): + f = tmp_path / "sampling" / "a" / "mod.py" + f.write_text(src + "\n") + assert bool(_other_module_imports(f, mods, tmp_path)) is bad, src + helper = tmp_path / "sampling" / "helper.py" + helper.write_text("from .b import x\n") + assert _other_module_imports(helper, mods, tmp_path), "a shared helper may not import a module" diff --git a/core/tests/test_ui_module_ids.py b/core/tests/test_ui_module_ids.py index 80e71be3..c2fc48b8 100644 --- a/core/tests/test_ui_module_ids.py +++ b/core/tests/test_ui_module_ids.py @@ -22,6 +22,8 @@ def test_every_module_id_the_ui_names_exists(): named.setdefault(i, p) for m in re.finditer(r'hasModule\("([a-z0-9_]+)"\)', t): named.setdefault(m.group(1), p) + for m in re.finditer(r'/api/m/([a-z0-9_]+)/', t): + named.setdefault(m.group(1), p) for m in re.finditer(r'TAB_MODULE\s*=\s*\{([^}]*)\}', t): for i in re.findall(r':\s*"([a-z0-9_]+)"', m.group(1)): named.setdefault(i, p) diff --git a/modules/loaders/diffusion_model/nodes.py b/modules/loaders/diffusion_model/nodes.py index 3820807d..841604d0 100644 --- a/modules/loaders/diffusion_model/nodes.py +++ b/modules/loaders/diffusion_model/nodes.py @@ -27,8 +27,8 @@ def _model_file_choices(): an existing pipeline's saved choice keeps its position in the list and nothing a user already picked moves. """ - return list(folder_paths.get_filename_list("diffusion_models")) + \ - gguf_support.gguf_names("diffusion_models") + ggufs = gguf_support.gguf_names("diffusion_models") if gguf_support.backend() else [] # no pack and no gguf package: nothing could load it + return list(folder_paths.get_filename_list("diffusion_models")) + ggufs diff --git a/modules/loaders/gguf_support.py b/modules/loaders/gguf_support.py index 9edc113b..72ef1cf0 100644 --- a/modules/loaders/gguf_support.py +++ b/modules/loaders/gguf_support.py @@ -102,7 +102,7 @@ def _walk(root: str, limit: int = 5000, depth: int = 12): if cut: from .._core import log log.once(f"gguf_walk_cut:{root}", log.WARNING, "FunPack .gguf listing", - f"{root}: stopped looking inside {len(cut)} folder(s) (deeper than {depth} levels or more than {limit} folders in all), so .gguf files there are not listed") + f"{root}: stopped looking inside {len(cut)} folder(s) (more than {depth - 1} levels deep or more than {limit} folders in all), so .gguf files there are not listed") def gguf_names(folder: str) -> list[str]: diff --git a/modules/sampling/modifiers/nodes.py b/modules/sampling/modifiers/nodes.py index 7015b862..e8a43b5e 100644 --- a/modules/sampling/modifiers/nodes.py +++ b/modules/sampling/modifiers/nodes.py @@ -273,6 +273,11 @@ class FunPackLoadModifiers(io.ComfyNode): # no-op while reporting that it had been applied. problems = [] chosen = {} + known = registry_mod.current().specs + for module_id in (settings or {}): + if module_id not in known and module_id not in control_mod.RESERVED: + log.once(f"stale_module:{module_id}", log.WARNING, "FunPack Load Modifiers", + f"saved settings for {module_id!r} ignored: no module with that id is installed") for spec in ordered: values, said = schema_mod.check_values( spec, (settings or {}).get(spec.id), keep_bad=full_control) diff --git a/modules/sampling/prompt_nudge/tests/test_prompt_nudge.py b/modules/sampling/prompt_nudge/tests/test_prompt_nudge.py index c4d4cc9a..51ad51f1 100644 --- a/modules/sampling/prompt_nudge/tests/test_prompt_nudge.py +++ b/modules/sampling/prompt_nudge/tests/test_prompt_nudge.py @@ -109,7 +109,7 @@ def test_the_sounds_text_channels_are_left_alone(): def test_a_split_negative_call_does_not_teach_the_negative_prompt(tiny_h3): - from modules.sampling.score_slider import cond_row + from modules.sampling.prompt_taste import cond_row c = torch.ones(2, 3, 4) assert cond_row(c, {"cond_or_uncond": [1, 0]}) == 1 assert cond_row(c[:1], {"cond_or_uncond": [1]}) is None diff --git a/modules/sampling/score_slider/tests/test_score_slider.py b/modules/sampling/score_slider/tests/test_score_slider.py index 4696c625..59eee3d0 100644 --- a/modules/sampling/score_slider/tests/test_score_slider.py +++ b/modules/sampling/score_slider/tests/test_score_slider.py @@ -29,7 +29,7 @@ def _rows(vectors): def test_the_direction_is_liked_minus_the_average_and_needs_three_liked(): - from modules.sampling.score_slider import direction + from modules.sampling.prompt_taste import direction e0, e1 = torch.eye(4)[0], torch.eye(4)[1] few = _rows([(e0, 1.0), (e0, 1.0), (e1, -1.0)]) assert direction(few)[0] is None @@ -38,7 +38,7 @@ def test_the_direction_is_liked_minus_the_average_and_needs_three_liked(): def test_similar_prompts_pick_their_own_direction(): - from modules.sampling.score_slider import direction + from modules.sampling.prompt_taste import direction a, b, n = torch.tensor([1., 0, 0, 0]), torch.tensor([0., 1, 0, 0]), torch.tensor([0., 0, 1, 0]) rows = _rows([(a + 0.1 * n, 1.0), (a + 0.1 * n, 1.0), (b, 1.0), (b, 1.0), (b, 1.0), (n, -1.0)]) near_a, how = direction(rows, current=a, similar=True)