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 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
fc977f4068
commit
37430137cb
+14
-5
@@ -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();
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
<link rel="stylesheet" href="/funpack/app/composer/composer.css">
|
||||
<link rel="stylesheet" href="/funpack/app/catalogue/catalogue.css">
|
||||
<title>FunPack modules</title>
|
||||
<div id="app"></div>
|
||||
<script src="/funpack/app/shell/theme.js"></script>
|
||||
<script type="module" src="/funpack/app/catalogue/modules.js"></script>
|
||||
@@ -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();
|
||||
+4
-1
@@ -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 };
|
||||
}
|
||||
|
||||
@@ -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 ...`."
|
||||
)
|
||||
+3
-3
@@ -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"]
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user