nodes/olivv-cs-ComfyUI-FunPack
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

feat: GGUF loader support, and fix silent pipeline-editor state loss

Browse Source 
GGUF: port v4's gguf_support.py (backend selection, content-based .gguf
detection, dequantize fallback) and wire it into both the diffusion-model
and CLIP loaders.

Pipeline editor: reopening the Models-and-pipeline settings window was
silently discarding the whole edited pipeline (a loaded preset, every
loader file, every sampler setting) because GET /api/pipeline is
deliberately stateless server-side and boot.js never fed its own
already-saved slots back into a freshly remounted editor -- found live
while walking through the MiniMax-H3 setup flow. Fixed by re-validating
the saved slots through check() on reopen instead of always replaying
the raw stateless default.

Also fixes, found via the extensive_testing adversarial loop over both
of the above: a stale file-listing cache in the pipeline window (no way
to see a newly-added model file without a full page reload), and a
chain of races/leaks in its own refresh/commit error-banner handling
that surfaced across five review rounds -- a rejection from a superseded
request overwriting a fresher success, a failure banner never clearing
on success, banners bleeding across groups on navigation, and a
commit()'s refusal landing on the wrong view after the user backs out
before it answers.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
digital-garbage
2026-09-11 18:14:46 +03:00
co-authored by Claude Sonnet 5
parent 4744d30087
commit 7a226830b1
9 changed files with 1191 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
app/shell/pipeline_window.js
+125 -13
View File
@@ -92,6 +92,24 @@ export function mount({ load, describe, check, search, onApply, presets,
let editing = null; // group name
let selected = null; // slot id
let notes = { refused: [], incomplete: [] };
// Kept separate from `notes.refused`: that field is about an EDIT the
// server rejected (commit()), and stays on screen until the user does
// something about it. A rescan() failure is unrelated to any edit -- and
// sharing one field meant a rescan that happened to succeed a moment later
// could silently wipe out a real, still-unaddressed commit() refusal the
// user had not yet seen or acted on.
let scanFailed = null;
let learnGen = 0; // guards against two learn() calls landing out of order
// Bumped by leave() only: every enter() is reached through the index,
// which only ever renders after a leave(), so leave() alone sees every
// group-to-group transition. Which VIEW an in-flight commit() belongs to
// -- a commit started in one group can still answer after the user has
// backed out to the index. Its DATA (slots) must still apply either way,
// an edit that happened does not un-happen because someone stopped
// looking, but its refusal banner belongs to a group that is no longer on
// screen, and painting it on the index is the same misattribution
// rescanGen/scanFailed already exist to prevent for a scan failure.
let viewGen = 0;
const body = composer.region.stack({ gap: "md", label: "Pipeline", fill: true });
@@ -100,19 +118,38 @@ export function mount({ load, describe, check, search, onApply, presets,
async function refresh() {
const answer = await load();
slots = answer.slots;
notes = { refused: [], incomplete: answer.incomplete || [] };
// `load` can be backed by a plain read (always `refused: []`) or, when a
// previously-saved pipeline is being re-validated on open, by the same
// check() a structural edit uses -- which can refuse. Reading it here
// rather than hardcoding `[]` is what keeps that case from replacing the
// screen with an unexplained empty pipeline.
notes = { refused: answer.refused || [], incomplete: answer.incomplete || [] };
await learn(slots.map((slot) => slot.node));
}
/** Descriptions for classes we have not seen. Asked once per class. */
async function learn(classes) {
const unknown = [...new Set(classes)].filter((c) => c && !nodes.has(c));
if (!unknown.length) return;
const described = await describe(unknown);
/**
* Descriptions for classes we have not seen -- or, with `force`, for
* classes we have. A class's combo choices are a file listing (model
* names, LoRAs), and this window's `nodes` cache lives for as long as the
* window is open: without forcing a re-ask on entry, a file added to disk
* mid-session stayed invisible in an already-opened group until the whole
* page reloaded, which throws the in-progress edit away with it.
*/
async function learn(classes, { force = false } = {}) {
const wanted = [...new Set(classes)].filter(Boolean);
const ask = force ? wanted : wanted.filter((c) => !nodes.has(c));
if (!ask.length) return;
const mine = ++learnGen;
const described = await describe(ask);
// A newer learn() (another force-refresh, or the group being left and
// re-entered) answered first. Applying this older one now would put its
// stale choices back over the fresh ones just drawn -- the exact bug this
// whole mechanism exists to fix, reintroduced by its own race.
if (mine !== learnGen) return;
// Every class asked about gets an entry, `null` included: without it an
// absent node is asked for again on every redraw, and reads on screen as a
// node still being looked up rather than one that is not installed.
for (const name of unknown) nodes.set(name, described[name] ?? null);
for (const name of ask) nodes.set(name, described[name] ?? null);
}
// Which edit is the current one. Two structural edits can be in flight at
@@ -126,12 +163,13 @@ export function mount({ load, describe, check, search, onApply, presets,
/** Send the pipeline as it now stands and keep what the server says of it. */
async function commit(next, action = {}) {
const mine = ++edits;
const myView = viewGen;
let answer;
try {
answer = await check({ slots: next, ...action });
} catch (err) {
if (mine !== edits) return false;
notes = { refused: [err.message], incomplete: notes.incomplete };
if (myView === viewGen) notes = { refused: [err.message], incomplete: notes.incomplete };
draw();
return false;
}
@@ -140,15 +178,25 @@ export function mount({ load, describe, check, search, onApply, presets,
if (mine !== edits) return false;
// A refused edit did not happen, so what is on screen must stay what it
// was: showing the refusal beside the change it refused is how someone
// comes to believe an edit landed when it did not.
// comes to believe an edit landed when it did not. But the refusal is
// only shown if the user is still looking at the view that made the
// edit -- otherwise it paints a group's own rejection onto whatever the
// user has since navigated to, the same misattribution scanFailed exists
// to prevent for a failed rescan.
if (answer.refused.length) {
notes = { refused: answer.refused, incomplete: notes.incomplete };
// `incomplete` describes the WHOLE pipeline, not this one edit, so it
// stays live regardless of view -- only `refused`, which is about
// THIS edit specifically, is view-scoped.
notes = { refused: myView === viewGen ? answer.refused : notes.refused, incomplete: notes.incomplete };
draw();
return false;
}
// The edit happened server-side regardless of where the user is looking
// now, so the data applies unconditionally -- only the refusal notes are
// view-scoped, same reasoning as the branch above.
forgetDraftsFor(slots, answer.slots);
slots = answer.slots;
notes = { refused: [], incomplete: answer.incomplete };
notes = { refused: myView === viewGen ? [] : notes.refused, incomplete: answer.incomplete };
await learn(slots.map((slot) => slot.node));
if (onApply) onApply(slots);
draw();
@@ -225,6 +273,7 @@ export function mount({ load, describe, check, search, onApply, presets,
function messages({ everything = false } = {}) {
return [
...notes.refused.map((text) => composer.banner.danger({ text })),
...(scanFailed ? [composer.banner.danger({ text: scanFailed })] : []),
// Not an error: a fresh install has no model picked, and colouring that
// red says something went wrong when nothing has yet.
...(everything ? notes.incomplete.map((text) => composer.banner.warn({ text })) : []),
@@ -315,17 +364,72 @@ export function mount({ load, describe, check, search, onApply, presets,
// ---- one group: nodes on the left, the selected node's parameters right
function enter(name) {
let rescanGen = 0;
/**
* Force-refresh a group's node descriptions and report if the server
* could not be reached -- describe() is a real fetch and a network hiccup
* or a restarted server leaves it rejecting, same as every other server
* call this window makes. Silently doing nothing here would make a refresh
* failure look identical to "nothing new to show", which defeats the point.
*
* `learnGen` inside learn() only guards which answer gets WRITTEN to the
* `nodes` cache; it says nothing about which rescan() call is allowed to
* touch `scanFailed`. Without its own token, an older rescan's late
* rejection could still slap a stale error banner over a newer rescan's
* fresh, successful result.
*/
async function rescan(classes) {
const mine = ++rescanGen;
try {
await learn(classes, { force: true });
if (mine !== rescanGen) return;
scanFailed = null;
} catch (err) {
if (mine !== rescanGen) return;
scanFailed = err.message;
}
draw();
}
async function enter(name) {
editing = name;
draft = new Map();
selected = (groupsOf(slots, extraGroups).byGroup.get(name) || [])[0]?.id ?? null;
const mine = groupsOf(slots, extraGroups).byGroup.get(name) || [];
selected = mine[0]?.id ?? null;
// A scan failure belongs to the group whose rescan raised it. Carrying
// it into a different group -- or the index, via leave() -- points
// someone at the wrong place to investigate; this group's own rescan()
// below sets its own, current answer moments later.
scanFailed = null;
// Same reasoning as rescanGen above, for an in-flight commit() started
// in whatever group was open before this one: its refusal belongs there,
// not here.
viewGen += 1;
draw();
// Refetched fresh rather than served from `nodes`: a combo's choices are
// a file listing, and a file added to disk since this window opened must
// show up without forcing a full page reload, which would drop the whole
// in-progress edit along with the stale cache.
await rescan(mine.map((slot) => slot.node));
}
function leave() {
editing = null;
draft = null;
selected = null;
scanFailed = null;
// Invalidates any rescan() still in flight for the group being left --
// without this, its answer lands after leave() (with editing now null)
// and a LATE failure writes scanFailed straight onto the index, since
// rescan()'s own guard only checks whether a NEWER rescan() has started,
// never whether the one it belongs to has since been left.
rescanGen += 1;
// Same for an in-flight commit(): its DATA still applies when it lands
// (an edit that happened does not un-happen because someone stopped
// looking), but its refusal banner belongs to the group just left, not
// to the index this draw() is about to show.
viewGen += 1;
draw();
}
@@ -347,6 +451,14 @@ export function mount({ load, describe, check, search, onApply, presets,
label: "Remove", tone: "ghost", disabled: !selected,
onClick: () => remove(selected),
}),
composer.button.sm({
label: "Refresh models", tone: "ghost",
// Same file listing a re-entry already forces -- exposed here too,
// because a file dropped in AFTER this group was opened has no
// other way to appear short of reloading the page, which would
// throw away every edit made in this session.
onClick: () => rescan(mine.map((slot) => slot.node)),
}),
] }),
] });
app/shell/tests/pipeline_window.test.js
+264
View File
@@ -385,6 +385,270 @@ test("a node is described once, however many slots point at it", async () => {
win.close();
});
test("reopening a group re-asks for its node descriptions, so a file added mid-session shows up", async () => {
// The bug this pins: `nodes` used to cache a class's description for the
// life of the window, keyed only by class name. A model file dropped onto
// disk after the window opened was invisible in an already-known node's
// panel with no way to see it short of reloading the page -- which throws
// the whole in-progress pipeline edit away with the stale cache.
let files = ["a.safetensors"];
const custom = server({ slots: [{ id: "l", group: "Loaders", node: "Loader", inputs: {} }] });
custom.describe = async (classes) => {
custom.calls.push({ describe: [...classes] });
return { Loader: { ...DESCRIPTIONS.Loader, widgets: [
{ name: "model_name", type: "COMBO", choices: [...files] },
] } };
};
const win = openWindow(custom);
await win.ready;
await win.enter("Loaders");
const optionsOf = () => [...rowFor(win, "Model name").querySelector("select").options]
.map((o) => o.textContent);
assert.deepEqual(optionsOf().filter((t) => t !== "— not set —"), ["a.safetensors"]);
files.push("b.safetensors");
win.leave();
await win.enter("Loaders");
assert.deepEqual(optionsOf().filter((t) => t !== "— not set —"),
["a.safetensors", "b.safetensors"],
"reopening the group did not pick up the file added since it was last open");
win.close();
});
test("Refresh models re-asks without leaving the group", async () => {
let files = ["a.safetensors"];
const custom = server({ slots: [{ id: "l", group: "Loaders", node: "Loader", inputs: {} }] });
custom.describe = async (classes) => {
custom.calls.push({ describe: [...classes] });
return { Loader: { ...DESCRIPTIONS.Loader, widgets: [
{ name: "model_name", type: "COMBO", choices: [...files] },
] } };
};
const win = openWindow(custom);
await win.ready;
await win.enter("Loaders");
files.push("b.safetensors");
click(button(win, "Refresh models"));
await new Promise(setImmediate);
const optionsOf = [...rowFor(win, "Model name").querySelector("select").options]
.map((o) => o.textContent);
assert.deepEqual(optionsOf.filter((t) => t !== "— not set —"), ["a.safetensors", "b.safetensors"]);
win.close();
});
test("a failed Refresh models says so, instead of looking like there was nothing new", async () => {
const custom = server({ slots: [{ id: "l", group: "Loaders", node: "Loader", inputs: {} }] });
// mount() destructures `describe` once, at open time -- reassigning
// `custom.describe` afterward would not reach it. A stable wrapper that
// reads a mutable flag is what lets a later part of the test change what
// happens on the NEXT call.
let fail = false;
const base = custom.describe;
custom.describe = (classes) => {
if (fail) return Promise.reject(new Error("those nodes could not be described (500)"));
return base(classes);
};
const win = openWindow(custom);
await win.ready;
await win.enter("Loaders");
fail = true;
click(button(win, "Refresh models"));
await new Promise(setImmediate);
assert.match(bannerTexts(win).join(" "), /could not be described/);
win.close();
});
test("an older learn() answering after a newer one does not put its stale choices back", async () => {
// Two overlapping force-refreshes (e.g. two quick clicks of Refresh models,
// or leave+enter fast enough that the first request is still in flight):
// whichever answer comes back LAST must not be allowed to be the OLDER one.
const custom = server({ slots: [{ id: "l", group: "Loaders", node: "Loader", inputs: {} }] });
let deferred = false;
let resolvers = [];
const base = custom.describe;
custom.describe = (classes) => {
if (!deferred) return base(classes);
custom.calls.push({ describe: [...classes] });
return new Promise((resolve) => resolvers.push({ resolve, classes }));
};
const win = openWindow(custom);
await win.ready; // past the initial, non-force learn() before describe() defers
deferred = true;
const first = win.enter("Loaders"); // in flight
const second = win.enter("Loaders"); // also in flight, started after
// Answer the newer request first, then the older one.
resolvers[1].resolve({ Loader: { ...DESCRIPTIONS.Loader, widgets: [
{ name: "model_name", type: "COMBO", choices: ["fresh.safetensors"] },
] } });
await second;
resolvers[0].resolve({ Loader: { ...DESCRIPTIONS.Loader, widgets: [
{ name: "model_name", type: "COMBO", choices: ["stale.safetensors"] },
] } });
await first;
const options = [...rowFor(win, "Model name").querySelector("select").options]
.map((o) => o.textContent);
assert.ok(options.includes("fresh.safetensors"), "the newer answer should have won");
assert.ok(!options.includes("stale.safetensors"), "the older, late-arriving answer overwrote it");
win.close();
});
test("a superseded rescan's late failure does not overwrite a newer rescan's success with a false error", async () => {
const custom = server({ slots: [{ id: "l", group: "Loaders", node: "Loader", inputs: {} }] });
let deferred = false;
let handlers = [];
const base = custom.describe;
custom.describe = (classes) => {
if (!deferred) return base(classes);
return new Promise((resolve, reject) => handlers.push({ resolve, reject }));
};
const win = openWindow(custom);
await win.ready;
deferred = true;
const first = win.enter("Loaders"); // will fail, but only after the second has already won
const second = win.enter("Loaders"); // succeeds
handlers[1].resolve({ Loader: DESCRIPTIONS.Loader });
await second;
handlers[0].reject(new Error("stale network failure from the OLD request"));
await first;
assert.deepEqual(bannerTexts(win), [],
"a rejection from a superseded request must not overwrite the newer success with an error banner");
win.close();
});
test("a successful rescan clears a refusal banner an earlier failed one left behind", async () => {
const custom = server({ slots: [{ id: "l", group: "Loaders", node: "Loader", inputs: {} }] });
let fail = false;
const base = custom.describe;
custom.describe = (classes) => {
if (fail) return Promise.reject(new Error("those nodes could not be described (500)"));
return base(classes);
};
const win = openWindow(custom);
await win.ready;
await win.enter("Loaders");
fail = true;
click(button(win, "Refresh models"));
await new Promise(setImmediate);
assert.match(bannerTexts(win).join(" "), /could not be described/);
fail = false;
click(button(win, "Refresh models"));
await new Promise(setImmediate);
assert.deepEqual(bannerTexts(win), [], "the earlier failure's banner should be gone after a success");
win.close();
});
test("a scan failure does not follow the user to the index or into a different group", async () => {
const custom = server({ slots: [
{ id: "l", group: "Loaders", node: "Loader", inputs: {} },
{ id: "s", group: "Sampling", node: "Sampler",
inputs: { model: ["l", 0], steps: 20, sampler_name: "euler" } },
] });
let fail = false;
const base = custom.describe;
custom.describe = (classes) => {
if (fail) return Promise.reject(new Error("could not be described (500)"));
return base(classes);
};
const win = openWindow(custom);
await win.ready;
fail = true;
await win.enter("Loaders");
assert.match(bannerTexts(win).join(" "), /could not be described/, "sanity: the failure shows in Loaders");
win.leave();
assert.deepEqual(bannerTexts(win), [], "Loaders' scan failure bled into the index");
fail = false;
await win.enter("Sampling");
assert.deepEqual(bannerTexts(win), [], "Loaders' scan failure bled into an unrelated group");
win.close();
});
test("leaving a group before its rescan answers does not let a late failure land on the index", async () => {
const custom = server({ slots: [{ id: "l", group: "Loaders", node: "Loader", inputs: {} }] });
let deferred = false;
let handlers = [];
const base = custom.describe;
custom.describe = (classes) => {
if (!deferred) return base(classes);
return new Promise((resolve, reject) => handlers.push({ resolve, reject }));
};
const win = openWindow(custom);
await win.ready;
deferred = true;
const entering = win.enter("Loaders"); // rescan() now in flight, unresolved
win.leave(); // back out before it answers
assert.deepEqual(bannerTexts(win), [], "sanity: nothing shown right after leaving");
handlers[0].reject(new Error("stale network failure from the left group"));
await entering;
assert.deepEqual(bannerTexts(win), [],
"a rescan answering after its group was left must not paint the index with its failure");
win.close();
});
test("leaving a group before its commit() answers does not let a late refusal land on the index", async () => {
// The commit()-side counterpart of the rescan race above: an edit (Remove,
// Save, wire...) started in one group can still answer after the user has
// backed out to the index. Its DATA must still apply -- an edit that
// happened does not un-happen -- but its refusal banner belongs to the
// group that made it, not to whatever the user is looking at by the time
// the answer arrives.
const custom = server({ refuse: "removing 'l' is ambiguous", slots: [
{ id: "l", group: "Loaders", node: "Loader", inputs: {} },
] });
let deferred = false;
let handlers = [];
const base = custom.check;
custom.check = (body) => {
if (!deferred) return base(body);
return new Promise((resolve) => handlers.push({ resolve, body }));
};
const win = openWindow(custom);
await win.ready;
await win.enter("Loaders");
deferred = true;
click(button(win, "Remove")); // commit() now in flight, unresolved
win.leave(); // back out before it answers
assert.deepEqual(bannerTexts(win), [], "sanity: nothing shown right after leaving");
handlers[0].resolve({ slots: [], refused: ["removing 'l' is ambiguous"], incomplete: [], queueable: false });
await new Promise(setImmediate);
assert.deepEqual(bannerTexts(win), [],
"a commit() refusal answering after its group was left must not paint the index with it");
win.close();
});
test("a load() that comes back refused still shows why, not an empty pipeline", async () => {
// refresh()'s `load` can be backed by a plain stateless read (always
// refused: []) or, when re-validating an already-saved pipeline on open,
// by the same check() a structural edit uses -- which CAN refuse (a real
// network failure/500 from the server mid-request). Silently dropping
// that field would replace the screen with an unexplained empty pipeline.
const win = openWindow({
load: async () => ({ slots: [], refused: ["the pipeline could not be read (500)"], incomplete: [] }),
describe: async () => ({}), check: async () => ({}), search: async () => ({ nodes: [], total: 0 }),
});
await win.ready;
assert.match(bannerTexts(win).join(" "), /could not be read/);
win.close();
});
test("a pipeline that cannot be read says so instead of showing an empty window", async () => {
const win = openWindow({
load: async () => { throw new Error("the pipeline could not be read (500)"); },