From 37430137cb6b8ef8e70fa0953cd97c47dd551e89 Mon Sep 17 00:00:00 2001 From: digital-garbage <133010426+digital-garbage@users.noreply.github.com> Date: Sat, 29 Aug 2026 13:20:50 +0300 Subject: [PATCH] foundation: run it, and fix what running it found First real generation through FunPack's nodes: SD1.5 on this Mac, real weights, a real picture of an apple. Everything before this was static. What it proved, on a real model rather than a fixture: the empty latent takes its shape from SD1.5's own format with no family branch anywhere (1, 4, 64, 64, an IMAGE latent from a pack built for video); the traits read off the live model are predict_eps and spatial_latent; and ALG is correctly absent, saying "needs temporal_latent", rather than running on a model with no time axis. What it found: the absolute-core-import bug again, this time INSIDE A FUNCTION BODY. It imported fine, registered fine, passed 258 tests, and raised at execute time -- during the generation. A suite that imports every module cannot see an import that only happens when a node runs, so the check for it is now static and walks the AST: no module may name `core` anywhere, at any nesting, except through the shim. The dev preview loaded one module in nine. It runs system python, which has neither ComfyUI nor torch, so every module that imports either was absent and the app looked almost empty while reporting nothing. It now re-execs under ComfyUI's interpreter. Looking at the app is meant to be how the app gets tested; it has to see what ComfyUI sees. ALG asked for a mount point no region offered, so its panel vanished in silence. The shell now offers one per stage, and a modules page lists what loaded, what asked for a mount nothing offers, and what failed to import -- absence is correct behaviour and indistinguishable from "never installed", so somewhere has to say which. Co-Authored-By: Claude Opus 5 --- app/boot.js | 19 +++++-- app/catalogue/modules.html | 6 ++ app/catalogue/modules.js | 85 +++++++++++++++++++++++++++++ app/shell/layout.js | 5 +- core/tests/test_module_imports.py | 59 ++++++++++++++++++++ modules/_core.py | 6 +- modules/sampling/modifiers/nodes.py | 5 +- tools/devserver.py | 58 ++++++++++++++++++++ 8 files changed, 232 insertions(+), 11 deletions(-) create mode 100644 app/catalogue/modules.html create mode 100644 app/catalogue/modules.js create mode 100644 core/tests/test_module_imports.py diff --git a/app/boot.js b/app/boot.js index 908c5af8..dc4b10bb 100644 --- a/app/boot.js +++ b/app/boot.js @@ -29,12 +29,21 @@ async function start() { } const { mounted, hidden } = await mountAll(manifest); - console.info(`[FunPack] ${mounted.length} module(s) mounted`, - hidden.length ? `· ${hidden.length} hidden` : ""); - for (const { id, why } of hidden) console.warn(`[FunPack] ${id} is hidden: ${why}`); + const failed = manifest.failed || []; - // Handy while there is no generate button to send them anywhere. - window.FunPack = { manifest, values: allValues, mounted: mounted.map((m) => m.id), hidden }; + console.info(`[FunPack] ${mounted.length} module(s) mounted`, + hidden.length ? `· ${hidden.length} hidden` : "", + failed.length ? `· ${failed.length} failed to load` : ""); + for (const { id, why } of hidden) console.warn(`[FunPack] ${id} is hidden: ${why}`); + // These never reached the manifest, so no panel could be missing "in a way + // the user notices" -- which is exactly why they have to be said. A module + // that failed to import looks identical to one nobody installed. + for (const { where, why } of failed) console.warn(`[FunPack] ${where} did not load: ${why}`); + + window.FunPack = { + manifest, values: allValues, failed, hidden, + mounted: mounted.map((m) => m.id), + }; } start(); diff --git a/app/catalogue/modules.html b/app/catalogue/modules.html new file mode 100644 index 00000000..4b255767 --- /dev/null +++ b/app/catalogue/modules.html @@ -0,0 +1,6 @@ + + +FunPack modules +
+ + diff --git a/app/catalogue/modules.js b/app/catalogue/modules.js new file mode 100644 index 00000000..3548ab74 --- /dev/null +++ b/app/catalogue/modules.js @@ -0,0 +1,85 @@ +// What is installed, what announced itself, and what did not make it. +// +// The plan called this a required tool rather than a nicety, and it was right: +// a module that fails to import is ABSENT, which is the correct behaviour and +// also indistinguishable from one nobody installed. Somewhere has to say the +// difference out loud, and it must not be the main app, where an absent feature +// is supposed to be simply absent. + +import { composer } from "../composer/composer.js"; +import { fetchManifest } from "../shell/manifest.js"; +import { build } from "../shell/layout.js"; +import { offered, _reset } from "../shell/mounts.js"; + +const root = document.querySelector("#app"); + +function rows(items) { + return composer.list.rows({ items, reorder: false }); +} + +async function start() { + let manifest; + try { + manifest = await fetchManifest(); + } catch (err) { + root.replaceChildren(composer.emptyState.default({ + icon: "▲", title: "Could not reach FunPack", hint: err.message, + }).node); + return; + } + + // Build the real shell off-screen purely to learn which mount points exist. + // Asking the layout is the only honest answer: nothing else knows. + _reset(); + build(document.createElement("div")); + const available = new Set(offered()); + + const modules = manifest.modules || []; + const failed = manifest.failed || []; + const unclaimed = modules.filter( + (m) => Object.keys(m.settings || {}).length && m.mount && !available.has(m.mount)); + + const sections = []; + + sections.push(composal("Loaded", `${modules.length} module(s) announced themselves.`, + modules.map((m) => ({ + id: m.id, + label: m.id, + hint: `${m.stage} · ${m.status}` + + (m.mount ? ` · ${m.mount}` : " · no panel") + + (Object.keys(m.settings || {}).length ? ` · ${Object.keys(m.settings).length} setting(s)` : ""), + })))); + + if (unclaimed.length) { + sections.push(composal( + "Asking for a mount point nothing offers", + "These loaded correctly and their panels will not appear anywhere. " + + `Offered: ${[...available].join(", ")}.`, + unclaimed.map((m) => ({ id: m.id, label: m.id, hint: `wants "${m.mount}"` })))); + } + + if (failed.length) { + sections.push(composal("Did not load", "Absent from the app, and from the graph.", + failed.map((f, i) => ({ id: `f${i}`, label: f.where, hint: f.why })))); + } + + if (manifest.incompatible?.length) { + sections.push(composal("Not for this model", "Filtered out by what the model is.", + manifest.incompatible.map((m) => ({ + id: m.id, label: m.id, hint: `needs ${m.requires.join(", ")}` })))); + } + + root.replaceChildren(...sections.map((s) => s.node)); +} + +function composal(title, hint, items) { + return composer.panel.default({ + title, + body: items.length + ? composer.group.default({ children: [ + composer.hint.default({ text: hint }), rows(items)] }) + : composer.hint.default({ text: "None." }), + }); +} + +start(); diff --git a/app/shell/layout.js b/app/shell/layout.js index 34dcf746..b864f39f 100644 --- a/app/shell/layout.js +++ b/app/shell/layout.js @@ -28,8 +28,11 @@ export function build(root) { // The mount point IS the contract with modules; the region behind it can be // rearranged freely as long as the name survives. offer("settings.general", general.body); - offer("generation.timing", generation.body); offer("generation.model", generation.body); + offer("generation.latent", generation.body); + offer("generation.sampling", generation.body); + offer("generation.timing", generation.body); + offer("generation.post", generation.body); return { general, generation, page }; } diff --git a/core/tests/test_module_imports.py b/core/tests/test_module_imports.py new file mode 100644 index 00000000..bb811273 --- /dev/null +++ b/core/tests/test_module_imports.py @@ -0,0 +1,59 @@ +"""No module may reach for `core` by absolute name. + +ComfyUI loads a pack by file location and never puts its directory on sys.path, +so `from core import ...` inside a module resolves under pytest and finds nothing +under ComfyUI. This has now cost the same bug twice: once at module level, where +the node quietly failed to register, and once INSIDE A FUNCTION BODY, where it +imported fine, registered fine, passed every test, and raised at execute time -- +during a real generation. + +That second one is why this is a static check and not a runtime one. A test that +imports every module cannot see an import that only happens when a node runs. +""" + +import ast +from pathlib import Path + +import pytest + +MODULES = Path(__file__).resolve().parents[2] / "modules" +SHIM = "_core.py" + + +def _module_files(): + return [p for p in MODULES.rglob("*.py") + if p.name != SHIM and "/tests/" not in str(p) and p.parent.name != "tests"] + + +def _absolute_core_imports(path: Path): + """Every `import core...` / `from core... import` anywhere in the file. + + Walks the AST rather than the text, so an import nested in a function, a + method or a try block is found exactly like a top-level one. + """ + tree = ast.parse(path.read_text(), filename=str(path)) + found = [] + for node in ast.walk(tree): + if isinstance(node, ast.ImportFrom): + # level > 0 is relative, which is what the shim exists to provide. + if node.level == 0 and (node.module or "").split(".")[0] == "core": + found.append((node.lineno, f"from {node.module} import ...")) + elif isinstance(node, ast.Import): + for alias in node.names: + if alias.name.split(".")[0] == "core": + found.append((node.lineno, f"import {alias.name}")) + return found + + +def test_every_module_file_exists(): + assert _module_files(), "found no module files to check" + + +@pytest.mark.parametrize("path", _module_files(), ids=lambda p: str(p.relative_to(MODULES))) +def test_a_module_reaches_core_only_through_the_shim(path): + offences = _absolute_core_imports(path) + assert not offences, ( + f"{path.relative_to(MODULES)} imports core by absolute name at " + + ", ".join(f"line {line} ({what})" for line, what in offences) + + ". Under ComfyUI there is no top-level `core`. Use `from ..._core import ...`." + ) diff --git a/modules/_core.py b/modules/_core.py index 76b26847..cc910ccf 100644 --- a/modules/_core.py +++ b/modules/_core.py @@ -11,8 +11,8 @@ leading underscore keeps it out of the module scan. """ try: # root on sys.path: tests, dev server - from core import log, registry, schema, traits + from core import contract, log, registry, relations, schema, traits except ImportError: # inside ComfyUI: a subpackage of the pack - from ..core import log, registry, schema, traits + from ..core import contract, log, registry, relations, schema, traits -__all__ = ["log", "registry", "schema", "traits"] +__all__ = ["contract", "log", "registry", "relations", "schema", "traits"] diff --git a/modules/sampling/modifiers/nodes.py b/modules/sampling/modifiers/nodes.py index e25f924c..51aa8f78 100644 --- a/modules/sampling/modifiers/nodes.py +++ b/modules/sampling/modifiers/nodes.py @@ -28,7 +28,8 @@ import json from comfy_api.latest import io -from ..._core import log, registry as registry_mod, schema as schema_mod +from ..._core import (log, registry as registry_mod, relations as relations_mod, + schema as schema_mod) CAPABILITY = "modifier" @@ -139,7 +140,7 @@ class FunPackLoadModifiers(io.ComfyNode): @classmethod def execute(cls, model, settings=None) -> io.NodeOutput: from ..._core import traits as traits_mod - from core.relations import order + order = relations_mod.order if settings is not None and not isinstance(settings, dict): raise RuntimeError( diff --git a/tools/devserver.py b/tools/devserver.py index 37e2b444..2d3b022c 100644 --- a/tools/devserver.py +++ b/tools/devserver.py @@ -14,6 +14,58 @@ from urllib.parse import parse_qs, unquote, urlparse sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +def _add_comfyui() -> str: + """Put ComfyUI on the path, because most modules import it. + + Without this the dev server loads only the modules that need nothing from + ComfyUI -- one out of nine -- and the app looks almost empty while reporting + nothing wrong. Looking at the app is supposed to be how the app gets tested, + so the preview has to see what ComfyUI would see. + """ + import os + candidates = [os.environ.get("COMFYUI_ROOT"), + str(Path.home() / "Documents" / "ComfyUI")] + for candidate in candidates: + if candidate and (Path(candidate) / "comfy" / "supported_models.py").is_file(): + sys.path.append(candidate) + return candidate + return "" + + +def _reexec_with_comfyui_python(comfyui: str) -> None: + """Restart under ComfyUI's own interpreter when this one cannot import torch. + + Finding ComfyUI's SOURCE is not enough: the modules import torch, and the + interpreter that launches this may be a bare system python. Getting that + wrong is not a crash, it is nine modules quietly absent and an app that + looks almost empty -- so the server corrects it rather than serving a + misleading page. + """ + import os + if os.environ.get("FUNPACK_DEVSERVER_REEXEC"): + return # already tried; do not loop + try: + import torch # noqa: F401 + return + except ImportError: + pass + + candidate = Path(comfyui) / "venv" / "bin" / "python" + if not candidate.is_file(): + print(f" WARNING: {sys.executable} cannot import torch and there is no " + f"venv at {candidate}. Modules needing it will be absent.") + return + + print(f" re-running under {candidate} (this one has no torch)") + os.environ["FUNPACK_DEVSERVER_REEXEC"] = "1" + os.execv(str(candidate), [str(candidate), *sys.argv]) + + +COMFYUI = _add_comfyui() +if COMFYUI: + _reexec_with_comfyui_python(COMFYUI) + from core import config, routes, serve as static # noqa: E402 P = config.UI_PREFIX @@ -60,4 +112,10 @@ class Handler(BaseHTTPRequestHandler): if __name__ == "__main__": port = int(sys.argv[1]) if len(sys.argv) > 1 else 8188 print(f"FunPack v5 dev server: http://127.0.0.1:{port}{P}/") + if COMFYUI: + print(f" with ComfyUI from {COMFYUI}") + else: + # Said out loud rather than left to be discovered as a half-empty page. + print(" WITHOUT ComfyUI: every module that imports it will be absent. " + "Set COMFYUI_ROOT to fix.") HTTPServer(("127.0.0.1", port), Handler).serve_forever()