Prompt Markup strips it before the encoder; Apply Prompt Markup puts weights
and timed windows on attention and blends on the conditioning. Where the words
sit is the H3 module's (text_tokenizer, prompt_rows, video_time_rows). Wired
into both H3 presets.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Both replace what a block allocates, which crashes aimdo's compiler (seen on a
rental with v4's shadow negative). Attached to the model, so any sampler.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
H3's latent upscaler loads through the H3 module (ComfyUI's loader has no
branch for it); an upscale resizes H3's anchor pin so pass 2 stays pinned.
FunPack Sampler takes an optional wired schedule, run exactly as written.
New preset: H3 Reference to Video + second pass.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The H3 module offers anchor_pin, so ALG blurs the pin's latent without
learning H3's payload layout.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
core.patching.strip now also removes our object patches, so a feature switched
off does not keep its patch on a model that goes round again.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
v4 read the prompt length from transformer_options['c_crossattn'], which
ComfyUI never sets, so every block fell back to stock. It also stood down on
any i2v scene (per-token mod rows). Both fixed; tested on a real tiny H3.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Block repeat's video-only mode now really keeps audio single-pass: H3 blocks
write in place, so v4 restored audio from an already-repeated stream.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 2 of extensive_testing on 548d99a found no exploitable defect --
has_block's "contains, not is" semantics are shared with minimax_h3's
own equally consequential is_h3() gate, and nothing in this codebase
nests diffusion_model in a wrapper that could trigger a false positive
today. Recorded as a one-line limit rather than left implicit, per the
round's only (non-blocking) suggestion.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Round 1 of extensive_testing on 79ca8cf found is_h3_model() matched on
diffusion_model's bare class name ("MiniMaxH3Model"), inherited
unchanged from v4. This codebase's own core/traits.py.has_block()
documents exactly why that's unsafe for a name this generic: a
same-named class from an unrelated custom node or a future FunPack
model would be silently treated as H3 and sparsified at 0.90 with
H3-specific audio-prefix pinning -- the identical LTX-collision risk
this file's own docstring already warns about, just not guarded
against strongly enough. modules/models/minimax_h3 already qualifies
its own identical check (MODEL_CLASS = the dotted path); this port
carried over the private, weaker duplicate instead of reusing that
pattern.
Switched to core/traits.py's has_block() with the same qualified dotted
name, kept as a local string (not an import of modules/models/minimax_h3
itself, which pulls in comfy_extras at import time -- this file
deliberately stays importable without ComfyUI on the path). Added a
test exercising the exact collision the bare match would have missed:
a class named MiniMaxH3Model from an unrelated module.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Loader-level attention option: mean-pool Q and a smoothed K into
blocks, score them with one small matmul, attend only the top
1-sparsity fraction of key blocks per query block, via a vendored
Triton kernel. Runs alongside the chosen attention backend rather than
replacing it (one override slot; the backend becomes SLA's dense
fall-through for the text refiner, masked calls, and trailing dense
steps), and refuses cleanly -- naming why -- on anything that is not
MiniMax H3 or a machine without CUDA+Triton.
modules/loaders/sla_block_map.py and sla_kernel.py are near-verbatim
ports of v4's vendored files (no FunPack-specific imports, so nothing
to adapt); sla_attention.py is the loader-facing glue (H3 gate,
override composition, per-step wrapper), same content as v4's with a
"not a module" note added to match gguf_support.py's convention.
Wired into FunPackDiffusionModelLoader as six new optional widgets,
following v4's loaders.py wiring but expressed in v5's io.Schema style.
The Triton kernel itself is unexercisable on this Mac (no CUDA) --
matches how real generation is verified in this project (rented GPU,
not local). Ported the CPU-testable half of v4's test suite (the
override contract, the H3 gate, the per-step wrapper); the three
kernel-correctness tests carry the same CUDA+Triton skip guard v4 used.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The port plan calls for every module under modules/ to declare a
CATEGORY so Settings/Engine panels can group by it. Only alg, sharpen,
appearance, and full_control had one; the other 12 (loaders, empty
latent, media load, minimax_h3, output decode, sampling modifiers/
sampler, system pipeline) are plain infrastructure with no user-facing
settings dial, so they land in "system" except output_decode (post,
matching its STAGE) and the two sampling modules (sampling). Verified
against core/tests (548 passed) -- category validation and the
registry's field-mapping catch a typo here, per the earlier fixture-gap
fix.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two pieces of the "port v4's exact UI onto v5's backend" plan:
- core/contract.py: new optional ModuleSpec.category field (closed set:
continuity, guidance, conditioning, sampling, post, system), validated in
core/schema.py, threaded through core/registry.py's FIELDS and
to_manifest(). Presentation grouping, independent of STAGE (execution
order) -- a module can be stage="sampling" and category="guidance" at
once. Only the 4 existing modules that actually render a settings panel
(alg, sharpen, appearance, full_control) got a category assigned; the
other 12 have none to categorize (no SETTINGS block) and stay
uncategorised, which is valid.
- v4's frontend (movie_editor/frontend/*, ~29k lines, 61 files) copied
verbatim into app/legacy/. core/routes.py's "/" route now serves
legacy/index.html instead of the composer entry point; a <base> tag in
that file resolves its own relative script/link tags against
/funpack/app/legacy/ regardless of what URL it's served at, so every
file moved as a unit with zero per-file path edits. Verified live: all
61 assets load 200 OK, the real v4 onboarding screen renders pixel-for-
pixel (it's the actual DOM/CSS, not a reimplementation), and several v4
API calls (/api/media, /api/projects, /api/git/status) already succeed
against v5's real backend by coincidence of shared route naming -- the
rest 404/405 as expected, since api.js's ~90 functions haven't been
rewired to v5's actual routes yet (next piece of this plan).
The old composer entry point (app/index.html, app/composer/**) stays in
the tree, unreferenced, until a later cleanup pass once nothing imports it.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
keep_proportion/extra_padding/invert_crop (ImageTransformKJ) and format
(SaveVideo) were wrapped in a redundant {"field": "value"} dict instead of
the plain string ComfyUI's own DynamicCombo expansion expects at the wire
level -- every one of them silently failed graph validation and fell back
to the node's own default, undermining the "total_pixels" resolution-fit
behavior this preset exists for. Found by loading the preset in a real
browser against real node schemas; the dev server never had comfy_extras
or KJNodes registered before now, so this was invisible locally until
tools/devserver.py also gained real node registration (init_extra_nodes +
KJNodes' load_custom_node, matching what the pytest `registered` fixture
already did).
The two pipeline_preset tests that were supposed to catch this had too
broad a filter -- it excluded ANY "is not one of" message as an assumed
"combo not chosen yet" state, which in this codebase actually shows up as
"needs X and nothing fills it" instead. Tightened to exempt only the three
loader-file combos genuinely left unset on a fresh install.
Verified live: preset loads with zero combo-validation problems, only the
three expected unset-loader-file messages remain, and Generate correctly
refuses with an honest reason rather than a false success.
Pieces B and C: Scene gains source_image (aspect-ratio reference only --
resolution itself stays a project.video setting, ImageTransformKJ's
"total_pixels" keep_proportion fits the picture's aspect into that budget)
and references (media ids, in order). Inspector's Scene tab gets pick/clear/
remove controls; picking routes through the existing Media bin via a small
pending-pick handshake in boot.js (one bin, one onPick, so "which field
asked" has to be tracked between the click that asked and the click that
answered).
References wire into MiniMaxH3ReferenceToVideo's ref_images autogrow input
dynamically, per run, only for what a scene actually has -- an unwired slot,
not an empty image, is what "not in use" means to that node. The target is
data on the role itself (wireTo), not a slot id boot.js would otherwise have
to know, keeping the "core never names an implementation" contract intact
one layer up into the app.
Verifying this exposed a second real core gap (after the MatchType one):
core/graph.py had no idea what an Autogrow (V3's io.Autogrow) input even
is -- "ref_images" reported as one opaque COMFY_AUTOGROW_V3 slot, its real
per-instance sockets (ref_image_0, ref_image_1, ...) invisible to
declared()/build(), so a wire naming one was refused as an input the node
does not have. Fixed generically in comfy_types.py (autogrow_instances())
and graph.py, confirmed against ComfyUI's own _io.py (finalize_prefix's
dotted join, build_nested_inputs's runtime un-nesting) -- not guessed, and
proven end to end with a real override()+build() test using the exact edit
boot.js makes.
Piece A of the H3 workflow port: the loader/SigmaShift/ImageTransformKJ/
R2V/sampler-at-CFG-1/decode/video-mux chain the H3 user runs by hand today,
offered as a preset (GET /api/pipeline/presets) loadable into the same
editable pipeline core/graph.py already supports -- not a second "default",
which module-load order would make ambiguous.
References are deliberately NOT pre-wired into R2V's ref_images autogrow;
an unwired optional slot is what "not in use" means to that node (absent
from its ref_images dict), not an empty image degrading its <Picture i>
numbering. boot.js wires them per-run based on what's actually picked --
next commit.
Building this against a real ComfyUI+KJNodes registration (not a fake
schema) surfaced a genuine core bug: core/graph.py's from_comfyui() had no
idea what to do with a V3 MatchType socket (ImageTransformKJ's own image
input) -- its bare type name "COMFY_MATCHTYPE_V3" matched nothing, so a
completely legal IMAGE wire into it was refused. Fixed generically in
comfy_types.py (match_type_union()) and graph.py, for any node using
MatchType, not special-cased to this one.
The bridge piece B and C (resolution source, reference images) both need:
Media bin picks have no way into a ComfyUI graph today. One node, media_id
in, IMAGE out, backed by core/media.py's existing store.
Also fixes a real .gitignore bug found along the way: a bare "media/"
pattern matched modules/media/ (this node's own source) as well as the
intended root-level user-data directory. Rooted both media/ and projects/.
Every node's own OUTPUTS were invisible on its own panel in the pipeline
editor -- wiring an input already let you pick a source, but opening the
node that PRODUCES something told you nothing about what it makes or
whether anything reads it. Found live, by a real user just looking at a
node, after two full review rounds that only ever checked the consuming
side. Fixed with a read-only "Outputs" section per node, naming each
output, its type, and who (if anyone) currently wires from it.
Also: model family detection. core/probe.py reads a safetensors header
(tensor names only, no weights) and asks every installed model module,
through the same capability-lookup mechanism already used for
empty_latent/decode, whether the file is its own -- core never names a
family itself. A real, adversarially-found bug in the ported signature:
one of MiniMax H3's three candidate prefixes doesn't correspond to any
prefix ComfyUI's own loader actually resolves to, traced through
comfy/model_detection.py and comfy/sd.py -- the probe could have called a
file H3 that ComfyUI's real loader would refuse. Removed, and pinned with
a dedicated test module. A second finding narrowed the folder search to
only the folders FunPack's own loaders actually read from.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Most FunPack pipelines run at CFG 1, where a negative prompt does
nothing -- it was one more box beside the prompt every Generate reads,
for a field only worth touching occasionally. The reference pipeline's
negative role now targets "project.negative" instead of
"generation.prompt"; the inspector's Project tab hosts it, in a STABLE
node outside the part draw() tears down and rebuilds on every scene
selection, so a field mounted there survives normal use.
Steps, Sampler and Scheduler are on the main window -- the three v4 buried
several clicks deep in Engine Settings. No new UI: the sampler slot declares
roles the way the prompt and the size already do.
What made that nearly free is that the controls now come from the NODE's own
description, through the settingFor/rendererFor pair the pipeline window has
always used. So a combo is a dropdown of the real choices and a number obeys
the real bounds, and prompt.js lost both of its hand-rolled branches: it no
longer decides what a control looks like, it asks.
File: New project, and every project listed with the open one ticked. The
store has been able to do this since the day it arrived and there was no way
in -- 20 lines of menu.
Three faults found on the way, all in the shared input helper:
* ENTER COULD NOT BE TYPED IN A TEXTAREA. `wire()` bound Enter to commit for
every text control, textareas included, so a multi-line value was
impossible -- the prompt above all, which is the longest multi-line thing
in the app. It has been that way in the pipeline window since it was
written; nobody had typed a second line into one.
* A control committed on blur alone, so autofill, a password manager and any
script setting a value went unheard. It listens to `change` as well now,
and commit() only acts on a real difference so the pair costs nothing.
* A control commits on blur, which means text still being typed is not in
the project yet -- and a tab closing is exactly when that is lost. boot
blurs before it saves.
Width, height and length belong to the PROJECT, not to a scene: a scene
cropped on the timeline and regenerated comes back at the project's size,
because the crop was a timeline decision and a regenerate is a new scene.
They sit in the Constructor beside the prompt -- the two things settled in
one sitting -- and travel to the graph on the path the prompt already uses.
No new mechanism: the pipeline declares roles at `project.video` the same
way it declares the prompt, so core still names no node and no setting. A
role whose value is a number gets a number control instead of a box; that
is the whole of the UI change. What the pipeline does not offer is not
shown, so a graph with no length input simply has no Length.
Bounds are the NODE's and stay there -- it already refuses a value outside
them with a reason, beside Generate. A project file is the one thing here
that outlives the code that wrote it, so everything read back out of one is
checked: a whole positive number, or gone.
Also, four faults the browser suite was already red with before this:
* Six tests still clicked a rail toggle that moved into the timeline head.
* At 420px the panel OVERLAYS the centre -- and the named toggles are in
the centre, so the panel covered the control that closes it. That is the
v4 trap this project forbids. They are not offered at that width now; the
View menu, which nothing can cover, is the way in and out.
* A 768px window laid out 860px wide and scrolled sideways. Root cause, and
it was never about one component: a flex item's automatic minimum is its
CONTENT, so every stack, split, panel body and frame region in the app
handed its content's width upward. A scroller that cannot shrink below
what it holds is not a scroller -- the strip of scenes made the WINDOW as
wide as the strip. Floored at the five places every layout routes through.
* A bin test counted `.cx-cell-glyph` across the whole page, so the timeline
drawing cells with the same glyph in them broke a test about the bin.
The layout was assembled by hand and read like it. Measured at 1440x900:
the three columns started at three different heights, a panel head was 41px
when it held a control and 34 when it did not, the gap left of the centre
was 7px and right of it 19, the properties column sat 11px further in than
the others inside a border that meant nothing, and five controls in one
list had four different widths -- a <select> takes the width of its widest
option, so the row listing checkpoint filenames was 124px wider than its
neighbours.
Now: --gutter is the space between regions, between two panels stacked in
a column, and inside the bands; the head band height is the panel head
height is the collapsible head height; --control-col gives every settings
control the same two edges. The frame's bands run edge to edge and the
content is inset by the gutter, so the app has an outline instead of a
margin that changed per region.
Also here, all of it visible in the same screenshot:
* The status sat against the Generate button with 1300px of empty bar to
its left -- everything was in one right-aligned group. What the app says
is at the start of the row now, what you press is at the end.
* A panel body is a column with a gap: two things put in it by two parts
of the app had whatever space their line boxes happened to leave.
* A container query adds no specificity, so the narrow-panel rule written
above the rules it overrides lost -- and flex-basis in a column is
HEIGHT, which turned four settings into a panel two screens long.
* The theme you picked was thrown away on every load: the appearance
module applied its own declared default over the choice the page had
already applied from storage. It adopts the live value now, and a panel
is built after setup so its control shows what setup settled on.
The prompt was a slot input on a CLIPTextEncode, reachable only by opening
the pipeline window and finding the node. A slot can now say that one of
its inputs belongs on the app's surface -- a place and a label -- and the
shell offers a region with that name or does not, exactly as a module's
panel works. Core validates that a role is names and nothing else.
Values for those inputs travel addressed by slot rather than as a whole
pipeline, so the window that owns the structure and a box that owns one
value cannot disagree about the slots between them. An address that is
not there is refused: a prompt typed into a box for a removed slot has
gone nowhere, and a run that starts anyway is not the run that was asked
for.
Region stand-ins are empty states now, taken down once something mounts --
'modules appear here' left above the modules that appeared explained
nothing to nobody.
Every module panel was decoration. The values lived in the browser and
nothing sent them, so a modifier switched on was one the run never heard
about. They now travel with the pipeline on every check, and core places
them wherever a module says they belong -- announced, not known, so the
node that carries them is not privileged.
Settings with nowhere to go are said beside Generate rather than
swallowed, and the warning survives the run it is about: the status line
is redrawn by every message, so a warning put there was gone the moment
the run said Queued.
Also: a settings row in a docked panel squeezed its label to one word per
line, because the control keeps a 140px floor and took it out of the
label. Below the width where they fit side by side, they stack.
One wide modal, a card per group, and a group that opens into a node list
beside that node/'s parameters, saved a group at a time. Groups travel on the
slot, so a pipeline keeps its own arrangement and a new group is a slot
claiming a name nobody used.
Values are drafted until Save; adding, removing, replacing and moving a node
go to the server as they happen, because only it can say whether the result
still wires up.
An unset dropdown reads "not set" rather than showing the first file as though
it had been chosen -- v4 recorded that trap, and picking the entry the box
already shows fires no change, so the one click that looks like the fix does
nothing.
New: region.stack, a frame header, a modal footer that can be refilled.
Not yet: wiring. A socket says so where a control would be.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The wording fix a commit ago was written by hand in two files and the same
mislabel was still sitting in four others -- traits, chain, modifier install and
node schema all reported a thing that loaded, recognised its work and then
raised as one that "did not load", sending a reader after an import error that
never happened. Three hand-written strings would have drifted, so log.broke()
now says it once and every site calls it.
Two of the new tests were vacuous when first written -- "nothing said did not
load" is also true where nothing ran at all, and picking the first provider
found lands on one whose traits exclude the test's model. Both assert the line
is present before asserting the wrong line is not.
One finding, confirmed, plus four sites it did not name.
`log.failed` means "did not load", and both the latent and the decode paths used
it for a provider that loaded, claimed the model and then broke while working --
"did not load -- RuntimeError: ..." sends whoever reads the log looking for an
import error that never happened. The levels were separated for exactly this.
One finding, confirmed.
Two corrections to the commit before this one.
A rank-1 latent's length is in the NODE that builds it and never in the format:
core turns seconds into samples with a 44100 rate and a 2048 hop that
latent_format does not carry, and the same rank means tokens of geometry for
Hunyuan3D. Deriving `length` samples gave StableAudio a latent of 1 where core's
own default is 1024 -- three orders of magnitude short, reported as success.
Refused now, with the reason and the way out: a module for that model.
The sweep claimed to skip H3 and did not. H3 has a rank-3 latent, so it was
derived and passed at 32 channels for a video branch that has 24 -- a test
passing on a model it is not correct for. So the previous message's "produces
exactly what core's nodes compute for every single-tensor model" was wrong on
both counts, and the sweep now says what it actually proves: the derivation
agrees with what a model PUBLISHES, which is a floor and not a certificate.
Rejected from the same report: an absent model module letting H3 fall through to
the generic shape. Core cannot detect that without naming models, which is the
one thing it must not do; a module that fails to import is already reported as a
Warning, which is the level that can see it.
LTX and WAN needed no module: both publish everything the derivation reads, and
it produces exactly what EmptyLTXVLatentVideo and Wan22ImageToVideoLatent
compute, by a different route. Pinned, along with a sweep over all 101 configs
ComfyUI ships -- rank, channel count and no empty axis -- so a family that
arrives after this was written is covered or fails here rather than at the first
sampling step.
H3 remains the only model needing a module of its own, because its latent is not
a single tensor.
Detail declared no traits, which read as "works on everything" and meant "is
offered on everything and drops itself on half of them". Core's unsharp mask
pads in two dimensions and raises on anything with more, so on a model with a
time axis the toggle was present, switchable, and inert -- a knob that does
nothing, which is the fault this project has shipped once already. It now
declares spatial_latent and is absent where it cannot work.
The guard's regression case survives, because the guard is not about a module
being wrong about itself: a hook meets the tensor it was handed. The stub model
now announces a latent rank, without which every trait-declaring modifier was
filtered out and those tests passed by installing nothing at all.
Round seven, one finding, confirmed in part -- the maths is deliberately not
being fixed, the advertisement was the defect.
Full control did half of what it said and never said which half. It disabled
the mid-run crash guard and left every value refusal in place, so the stated
purpose -- a one-step schedule, a width the model cannot use, let them and
warn -- did not work. Worse, a rejected value was quietly replaced by the
default, so asking for 999 gave 0.4 with a warning about 999.
Now one switch turns off both: the value is passed through, every problem is
said as a warning naming that the consequences are the user's, and a run made
without the guards is identifiable as one afterwards. A key NO module
declares is still never kept, whatever the setting -- nothing reads it, so
keeping it cannot honour the request, only disguise the typo.
core/graph.py was thoroughly tested and called from nowhere: the claim that
a built-in node can be replaced or removed was a property of a library
nothing invoked. It has routes now, and tests that drive them over a real
socket -- the modifiers node removed and the sampler rewired, the VAE loader
swapped for core's, and both refusals coming back as reasons.
Those routes separate two things the old shape conflated. "refused" means
the edit did not happen. "incomplete" means it did and the pipeline still is
not ready -- an unset file picker on a fresh install is the normal case, not
a failed edit, and showing them together would say the wrong thing about both.
And build() could not see a missing REQUIRED input at all, because the
schema lookup flattened required and optional into one dict. A slot with no
model file built clean and failed only once queued.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Stopping a generation is a normal thing to do -- a bad seed, a wrong prompt,
a rental minute -- so it is not an error path that can be left approximate.
A leak here is invisible: the run AFTER a cancel just behaves slightly wrong.
The sampler says a run was cancelled and re-raises. It is not ours to
absorb; ComfyUI ends the queue item. But a run that simply stops with
nothing said is indistinguishable from one that broke.
Verified against the real engine: a 40-step generation interrupted mid-way
reports execution_interrupted, and the next one completes as run 2. Tests
cover what must be left behind, which is nothing -- no wrappers on the shared
model after five cancelled runs, the sharp anchor put back on a denoiser that
outlives the run, and no modifier marked as dropped, because a cancelled
modifier did not fail and disabling it next time would punish the user for
stopping.
Temp previews needed no change: core's PreviewImage already writes to the
temp directory under a unique prefix, so where the app SHOWS them is a UI
decision and is recorded as one rather than built here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The run id froze after the first generation. Runs were marked by the
modifiers node, and ComfyUI CACHES a node whose inputs have not changed --
only the seed changes between most generations, so that node executed once
and every later run reported under the first one's id. Per-run log dedup
degraded to per-process with it, which is precisely the v4 fault log.py's
own comment describes, reintroduced through a caching interaction the fix
did not account for.
The sampler marks the run now. It is what generates, and it re-executes
whenever anything about the generation differs. Verified through the real
PromptExecutor with caching on: three seeds, three run ids. The
dropped-modifier record is also emptied there, because it travels on the
model and ComfyUI may hand the same model back -- a modifier that failed
last time deserves this time.
And a bad settings payload was only caught during execution, after the
checkpoint had already loaded. On a rented GPU that is the cost paid before
the guard fires, one layer up from the sampling case this project already
guards. ComfyUI calls validate_inputs inside validate_prompt, so the payload
is now understood before the queue accepts it: a typo'd module id is refused
with nothing loaded. Both doors check the same things, and a test pins that
they agree.
There was no test file for runs at all, which is why the first fault could
be introduced by a fix and survive a green suite.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
v4 could not remove a built-in node from the pipeline and the reason was
structural: the wiring lived in code as a CORE dict nobody could reach, so
"remove this" had nothing to delete and "use a different one" had nothing to
change. Code is not a thing a user can edit.
The pipeline is now data. Three operations over it, each refusing with a
reason rather than half-doing the job:
build turns slots into the prompt ComfyUI queues, checking every link's type
and refusing a graph that cannot work. replace points a slot at any node
producing what its consumers read -- the built-in ones have no special
standing, which is what makes them replaceable rather than merely present.
remove takes a slot out and rewires what it fed to what fed it, when the
node has exactly one input matching each CONSUMED output; v4 required one
for every output a class could produce, so a spare FLOAT nobody wired made a
node unremovable.
Demonstrated against real schemas: FunPack's VAE loader swapped for core's
VAELoader, the modifiers node removed with the sampler rewired to read the
model directly, and removing the empty latent refused because nothing
upstream could feed what it fed.
Then run for real. The default pipeline, with three loader slots replaced by
one checkpoint slot, built from data, accepted by ComfyUI's own
validate_prompt, and executed by its own engine to execution_success and a
picture. Every wiring and type validated on the first attempt; the only
things ComfyUI objected to in the untouched default were the three unset
file pickers, which is correct -- a fresh install cannot know which model
you own.
Added a checkpoint loader, because the split loaders are right for LTX and
H3 and cannot read an SD-family single file at all.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
v4's Chain Sampler became a hub: 8183 lines, roughly seventy optional
inputs, a _sample_chunk that grew six ALG arguments, and a hardcoded list of
knobs silently inert on one model family. Every technique that needed the
loop was added to it, so it collected all of them.
This one picks an algorithm and a schedule from ComfyUI's own registries --
so a sampler it gains appears here without an edit -- and runs them. Three
things stop it becoming a hub again. It never learns what a model-side
modifier is. Sampler-side ones reach it through ONE call site,
chain.process(ctx, latent), which cannot grow because the step travels in a
context object rather than in the signature. And it declares the hook points
it offers, so core hands it only what fits and it never names a modifier.
Some modifiers genuinely cannot ride on a model: they need the step index,
the schedule, or a second pass. Those attach here instead, and one asking
for a hook this sampler does not offer is absent and said so, not
half-running.
Verified against ComfyUI's own guider path with real weights: bit-identical
output. That took correcting the method, not the code -- see below.
MEASUREMENT, corrected: sampling on this Mac's MPS backend is
NONDETERMINISTIC. The same node with the same seed and the same inputs
differs run to run: 2.42 by max latent difference, 0.0008 by mean pixel gap.
Comparing by MAX said this sampler diverged from core by 14.2 and it was
about to be "fixed"; by MEAN against a measured noise floor it is 0.00000
while core against itself is 0.00183. Every comparison here now runs a
control twice first. Re-checked the Detail modifier the same way: 145x the
floor, so that claim stands.
Also matched what core's sampler actually does and this did not:
fix_empty_latent_channels using the ratios our own empty-latent node reports,
the preview and progress callback, the move back to the intermediate device,
and dropping the ratio keys so nothing rescales twice.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Colour is a preference, so it is a setting: hex in, hex out, one spelling,
refused if it is not one. A module handed "red" in one place and "#f00" in
another guesses wrong somewhere.
Image and mask are NOT settings and deliberately have no type. They are
data -- tensors the app uploads and the graph wires -- and ComfyUI already
has socket types for both. Putting a drawn mask in a JSON settings payload
would be inventing a second way to move something that already moves.
Runs now have an id. Previews, progress, results and log lines all have to
be attributable to one generation, and ComfyUI gives a node no way to ask
which run it is in. So FunPack marks its own, every log record carries it,
and the history can be filtered to a single run. Readable rather than a
uuid, because it is going to be read: "run 7" is findable in a wall of
output and a hex string is not.
The colour element was caught by the registry-driven rule test before it was
finished -- an element with no catalogue demo is neither shown nor attacked,
so coverage keeps up with the registry instead of lagging behind it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Cancel reaches a hook as an exception like any other, and a guard that
caught it would leave someone pressing stop on a run that will not stop.
It works today only because ComfyUI made InterruptProcessingException a
BaseException rather than an Exception -- a detail this code depends on and
does not control. Widening the guard to BaseException would break Cancel
with nothing else failing, so the property is now a test rather than an
accident.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
[Log] Info: FunPack Diffusion Model Loader | flux.safetensors loaded, attention sage
[Log] Alert: MiniMaxH3ImageToVideo node | no "first_image" input is connected
[Log] Warning: FunPack LoRA Loader | style.safetensors is empty and will not load
[Log] Critical: torch | cannot allocate 14 GB (3 GB free); the run will not start
The difference between the middle two is the one that matters. Alert is a
choice you may not have made on purpose. Warning is something that will NOT
do what its name says -- the level v4 needed and did not have, which is how
560 handlers came to swallow a failure and carry on, so a feature could be
switched on, do nothing, and never say so.
Info is not decoration either: "loaded on GPU using sage attention" is how
an attention setting that did nothing gets noticed months before the
pictures look wrong enough to investigate.
Every call site moved -- there is no note() left to be vague through -- and
the loaders now say what they actually loaded rather than only what they
were asked for. Records are kept in a bounded ring as well as printed, so a
long rental session cannot turn the log into a memory leak, and served at
/funpack/api/log so a person can read it somewhere other than a terminal
full of ComfyUI's own output. There is a page for it.
Also dropped the preflight this was traded for: min and max are already
declared and already enforced at both doors, and a second protection layer
around a check that exists is ceremony.
Two mistakes worth keeping: a log line reached into the model's shape to
describe it and could have failed the load it was only narrating; and both
new pages passed `children` to an element that takes `rows`, which the kit
accepted and silently dropped. It throws on an unknown element and not on an
unknown prop.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Catching a modifier that fails to INSTALL was never the hard part. The one
that costs a rental is the hook that works at step 0 and raises at step 3,
after the GPU time is spent -- which the Detail modifier does on any video
latent, because core's sharpen kernel cannot take a 5-D tensor.
A module is now handed a patcher that guards every hook it installs, so it
does not opt in to being safe: a module that could forget to is a module
that can break the foundation. A hook that raises is dropped for the rest of
that run, reported once with its traceback, and sampling continues. Thirty
steps on the latent that used to end the run now complete untouched.
Recorded per RUN, not per process, so a long session reports every
generation that went inert instead of only the first after a restart.
Full control turns all of it off. A one-step schedule, a width the model
cannot use, a modifier that throws -- someone exploring the edges wants the
real behaviour and the real traceback, not a careful refusal. One switch
rather than one per guard, because a half-disabled safety net is harder to
reason about than either extreme. It is asked for by capability, so nothing
names the module that answers, and a build without it installed has nobody
saying yes -- which reads as guarded.
Honest about its own limit: a hook shape with no known neutral result is
installed unguarded and named in the status, rather than counted as safe.
Two test faults fixed while writing this. The assertions compared conds_out
against the list passed in -- the same object the hook mutates -- so they
were true whatever the hook did. And the registry fakes were hand-rolled and
missing methods the real one has, which passed until the code called one.
They use the real Registry now.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Decode now takes the MODEL, so a provider answers "is this mine" by identity
instead of by shape. H3 was claiming any latent that happened to have two
parts -- right today because it is the only thing producing one, wrong the
moment a second AV model exists, and wrong in the way that reads as an
unrelated fault rather than an error. It declines when the model is not
wired, and the node says that is what is missing.
That completes the shape a model module should have: one folder saying how
this model behaves at each stage -- what it is, how its latent is built, how
its output is decoded -- instead of that knowledge spreading through the
conditioning and sampling code as special cases.
Added a modifier that runs on SD1.5, because a feature nobody can watch is a
feature nobody can check. It declares no traits, so it applies everywhere,
which is the other half of the rule ALG demonstrates by narrowing. Measured
on this machine at a fixed seed: off and on differ, 0.4 gives a visibly
richer picture that still reads correctly, and 1.2 blows the highlights --
so the range is -1..1, set from what was measured rather than from a round
number. A slider whose top half destroys the image lies about its range.
Two things found while building it. ComfyUI has a real circular import
between nodes_latent and nodes_post_processing that resolves only because
init_extra_nodes reaches one first; a custom node has to break it the same
way. And the accumulation bug returned through a third door: stripping
handled keyed wrappers but not the anonymous function lists on
model_options, so chaining doubled the effect. Tagging is now generic -- it
looks for our mark rather than for a list of hook names that would need
keeping current.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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>
The NaN guard was on one of two doors. A FUNPACK_SETTINGS socket is typed by
a string tag, so a dict reaches the load node without necessarily having
passed through the settings node -- a raw API prompt does exactly that.
Through that door, until_sigma = NaN made ALG a permanent no-op: sigma > nan
is False at every step, so the anchor was never loosened, and the node
reported "1 modifier applied: alg". Nothing about the run looked wrong,
which is the only reason this is worth a commit of its own.
Values are now checked against the declaration at the point they are
consumed. The producer still checks too; that is not redundant, it is where
a person editing JSON gets told immediately.
Also recorded ALG's standing assumption rather than leaving it implicit: the
anchor is latent frame 0. True of everything it can currently be wired into,
untrue in general, and the denoise_mask already carries what would be needed
to do it properly. Left undone because nothing yet pins elsewhere, and a
derivation with no consumer cannot be checked against reality.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Core already turns images into a video and writes the file, and handles
containers, codecs and muxing properly. Writing our own would be a worse
copy of tested code, so there is no FunPack save node and no reason for VHS
either.
What core cannot generalise is decoding a latent that is not one tensor: an
AV latent has two branches wanting different VAEs, and their arrangement is
the model's business. Same rule as the empty latent -- a model module may
claim it, and H3 now does. A nested latent nobody claims is refused rather
than handed to vae.decode, which would raise somewhere confusing or quietly
decode one branch as the picture. A missing audio VAE is named, because
returning no audio looks exactly like a model that makes none.
With this the pipeline closes end to end using FunPack's eight nodes and
core's samplers and video nodes -- verified by loading both together. That
matters more than the node count: until now nothing FunPack produced could
be measured, only reasoned about.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An i2v model shown a sharp reference can satisfy the objective by barely
moving. ALG low-passes the anchor while sigma is high, removing the detail
that shortcut needs, and hands the sharp one back once the picture settles.
It attaches to the model, so it applies to whatever sampler is wired, and it
requires temporal_latent and nothing else -- not "for LTX", not "for H3".
Any model with a time axis and a pinned anchor gets it, including ones
nobody here has heard of. A module with no ComfyUI node of its own: settings
and behaviour, which is what most modules should be.
The packed-layout path is NOT ported. v4 reached into a model-specific
arrangement of the latent to find the video stream; where the anchor is not
a plain [B, C, T, H, W] tensor this stands aside and says so once, rather
than reshaping something it is guessing about.
Tested as far as a machine with no GPU can: the filter really low-passes,
only the anchor frame changes, the choice follows sigma, and the whole thing
runs through ComfyUI's real KSAMPLER with only the innermost loop stubbed --
so the anchor a denoiser sees differs between an early step and a late one.
Whether it looks better needs the rental.
Also guarded route registration. PromptServer.instance only exists once a
server has been built, and importing `server` successfully is not the same
as running inside one -- with ComfyUI on the path the unguarded attribute
took the whole pack down.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The claim in the last commit was too strong. Cloning stops this node
touching the model everyone else holds; it does nothing about the node's own
output coming back in. add_wrapper_with_key APPENDS under the key and a
clone carries the list, so chaining two of these -- two passes, a flattened
subgraph -- installed the same wrapper twice and ran it twice per step at
double strength, while both nodes reported "1 modifier(s) applied".
That is the accumulation this design is supposed to prevent, arriving
through a different door. Every install is now preceded by stripping our own
namespaced keys and saying how many were replaced. Foreign wrappers are
never in scope, which is what the namespace was for.
NaN passed validation: it compares False against both bounds, so a range
check alone never fires, and json.loads accepts NaN and the infinities by
default. A NaN strength reached a modifier and would have poisoned sampling
with nothing reported. Non-finite numbers are now refused by name.
A settings payload of the wrong shape raised AttributeError rather than the
error reporting everything else here uses; the socket type is only a string
match, so anything can arrive on it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two nodes. One carries the values a person picked, as a single validated
payload rather than a socket per setting -- a socket per setting means this
node's shape changes whenever any module gains an option, and every saved
workflow refers to sockets that moved.
The other reads what announced itself, filters it by what the model actually
is, orders it by declared relations, and installs each one through ComfyUI's
own patcher_extension wrappers. The consequence is checked against comfy's
real prepare_model_patcher and WrapperExecutor rather than against our own
dict: a model that has been through this node runs every per-step modifier
inside ComfyUI's own SamplerCustomAdvanced. The sampler is not consulted, so
it cannot own them.
Installing happens on a clone, and clone() copies the wrapper lists per key,
so the model everyone else holds is untouched. Fifty runs leave zero
wrappers behind. The accumulation that once degraded output across a hundred
generations and survived every reset short of a restart is structurally
impossible here rather than something to remember to undo.
The payload is validated against the same declaration that renders the
panel: an undeclared key, a wrong type, a value outside its own bounds and a
bool passed as a number are all refused, and all problems are reported at
once. v4 validated none of it, which is why it needed a hand-mirrored table
of every default and a list of keys that no longer meant anything.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A provider that raised was treated exactly like one that said "not my
model", so the node fell through and derived a shape for the very model the
provider existed to handle -- and reported success. For H3 at length=124
that is 31 latent frames against a real grid of 37: a 16% shorter video,
with nothing downstream saying anything.
The protocol is now stated: return None for "not mine", and let anything
else raise. Recognition happens before anything that can fail, so an
exception means the model was already claimed, and the node refuses rather
than substituting a generic shape.
execute() had no test at all -- the dispatch between claimed and derived is
the point of the node, and both earlier tests called around it. It now runs
through the real registry, and the refusal test fails against the old
fall-through.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
