* review-stack 1/4: code (37 files, +3217/-3958)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: path not under tests-unit/ or tests/
Question: Is the logic change right?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 2/4: tests-removed (24 files, +274/-8220)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: test file deleted, or modified with deleted/(added+deleted) >= 0.9
Question: For each dropped assertion: obsolete by a ruling, or covered by a tests-new test?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 3/4: tests-changed (13 files, +1043/-1218)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: remaining modified test files (incl. conftest.py / helpers)
Question: Did the edits weaken an existing check?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 4/4: tests-new (46 files, +8601/-0)
Review-and-land stack for synap5e/feat/asset-record-content-split, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: test file added
Question: Is the code layer well covered?
Source tip: 7007d18582
Merge-base: 783545f689
* review-stack 5/6: code (13 files, +351/-104)
Review-and-land stack for synap5e/feat/assets-di, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: path not under tests-unit/ or tests/
Question: Is the logic change right?
Source tip: eca2c74bff
Merge-base: 20d59d2a5f
* review-stack 6/6: tests (8 files, +753/-238)
Review-and-land stack for synap5e/feat/assets-di, generated by review-stack.py. Once approved,
merges DOWN into the layer below (a fast-forward); only the bottom layer
squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: every changed file under tests-unit/ or tests/ (added, modified, or deleted)
Question: Is the code layer well covered, and did any edit weaken an existing check?
Source tip: eca2c74bff
Merge-base: 20d59d2a5f
* review-stack 7/8: ported-fixes (42 files, +1361/-180)
Review-and-land stack for synap5e/feat/assets-di-v2, generated by
review-stack.py conventions (hand-built continuation layer; see the PR body).
Once approved, merges DOWN into the layer below (a fast-forward); only the
bottom layer squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: the 11 base-branch fix/docs commits 595cd6e4..94d7185b cherry-picked across the DI refactor (7efdd1d7 excluded, superseded by layer 8)
Question: was each base fix ported faithfully across the DI refactor?
Source tip: 6841881069284803b902b4a9e33bdcda13126771
Merge-base: 7fdfb40f4b
* review-stack 8/8: defensive-parity (4 files, +36/-3)
Review-and-land stack for synap5e/feat/assets-di-v2, generated by
review-stack.py conventions (hand-built continuation layer; see the PR body).
Once approved, merges DOWN into the layer below (a fast-forward); only the
bottom layer squash-merges into the real base. See ~/adocs/review-stack.md.
Rule: match-or-improve master's dependency defenses — NoAssets selection when DB deps unavailable (7efdd1d7's outcome via the DI seam), requirements warning before assets imports, blake3 in the guarded dependency set
Question: does each degradation path now match or improve master's behavior?
Source tip: ebc2cfeebc
Merge-base: 7fdfb40f4b
* fix(assets): only discard content rows this operation actually inserted
CR-9: Enumerated all six create_content call sites. Only scanner seeding and the three ingest registration paths track IDs for failure cleanup.
* fix(assets): reject hash-only uploads with FEATURE_DISABLED when hashing is off
CodeRabbit finding CR-2: reject hash-only multipart uploads before create_from_hash when hashing is disabled.
* fix(assets): seed persists the stat it verified
CR-7: persist the fresh seed-time restat instead of walk-time spec values.
* fix(assets): route database lock failures to the lock guidance
CR-16: route file-lock startup failures through the existing lock guidance and exit path.
* fix(assets): drop the inaccurate temp-cleanup claim from the shutdown warning
References CR-10.
* fix(assets): walk the output root after execution so undeclared outputs register promptly
Custom nodes that write files into the output directory without declaring
them in output_ui only became assets when the next full walk happened - a
frontend GET /object_info or a restart. Headless and API-only sessions never
trigger either, so those files never converged into the asset database.
The post-execution hook now requests a FULL scan of the output root instead of
an enrich-only pass. The seeder's pending-request queue was generalised from
enrich-specific to carrying a scan phase, so the request starts immediately
when the seeder is idle and coalesces (escalating to FULL on a phase mismatch)
when a scan is already running. queue_output_enrichment is renamed to
queue_output_scan across the protocol, the NoAssets no-op and the call site.
References FIX-6.
* chore(assets): remove seeder paths orphaned by the output-scan change
45c2f96e rerouted both former enrich call sites to start()/enqueue_scan(),
leaving two seeder methods that look live but are not. Review round F2
raised this along with four smaller items; the user's disposition was to
fix all six here.
- Delete start_enrich: zero callers repo-wide after 45c2f96e.
- Delete enqueue_enrich: no production callers; its ~18 call sites in
tests/test_asset_seeder.py move to enqueue_scan(phase=ScanPhase.ENRICH)
with their semantics unchanged. The deletion forces the half-done class
renames (TestEnqueueEnrich* -> TestEnqueueScan*, consistent with the
already-renamed TestPendingScanDrain) and restores the module docstring
that was dropped rather than reworded.
- Document at manager.queue_output_scan that ScanPhase.FULL per debounce
window is the deliberate, user-ratified trade, so it is not optimised
back to ENRICH without revisiting the decision.
- Document that SeedAssetSpec.size_bytes/mtime_ns are walk-time
diagnostics only - production persists the seed-time restat since CR-7.
- Export create_content_reporting_insert from the queries facade and fold
scanner.py's direct-module import into the existing facade block.
- Harden test_queue_output_scan_does_not_duplicate_declared_output against
a vacuous pass: it now asserts the seeder finished without errors and
that an undeclared sibling written into the same directory WAS
registered by the same scan, proving the walk actually ran.
No production behaviour changes beyond the two deletions.
References F2-cleanup.
* chore: comment cleanup
Comment-Gate: 18 quarantined
* fix(assets): preserve pause across the seeder's pending-scan drain
pause() runs before every prompt, while pending-scan enqueue and resume only run inside the debounced gc-interval gate. If the active scan finishes just after the next prompt's pause, its finally block resets the seeder to idle and the pending drain starts a replacement with the run gate open, so resume becomes a no-op.
Capture pausedness under the lock before resetting to idle, then start the drained scan already paused. Setting the state and gate before launching the thread avoids the start-then-reclear window and lets resume release the existing scan checkpoints.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
* test(assets): pin job_id absence for scan-discovered assets
Owner ruling, recorded 2026-09-03 in the stack-9-hardening planning notepad: scan-discovered assets — including undeclared outputs found by the post-execution walk — carry job_id = None, always; only emission-time registration (output_ui declaration) attributes a job; attributing walk finds to the most recent prompt would be a temporal-correlation guess that is wrong exactly when prompts interleave; None is honest provenance. Do NOT add proximity-based attribution heuristics to the scanner. Ratified against Jacob Segal's cross-job-attribution concern (2026-09-08 review meeting) — a wrongly-attributed asset could mean one user's cloud job sees another user's asset.
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
* [review-stack 10/10] assets-tests (#16218)
* test(execution): run the battery with assets enabled and assert asset-system health at teardown
* test(execution): cover list-shaped outputs registering assets
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
---------
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
* [review-stack 11/11] review-fixes (#16261)
* fix(assets): only exit on database file-lock timeout when assets are enabled
* test(assets): pin live_contents_under_prefixes path-filtering semantics
* perf(assets): push live-content prefix filtering into SQL
* test(assets): declare per-entry intent in the path-prefix corpus
* test(assets): normalize POSIX-literal path expectations for Windows
* test(assets): force observable stat changes and close-before-mutate on Windows-sensitive rewrites
* test(assets): force an observable mtime change in the hash-mode split test
---------
Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
Co-authored-by: guill <jacob.e.segal@gmail.com>
* feat(api-nodes): add Comfy Cloud partner nodes
Ports the Comfy Cloud provider client from #15282, trimmed to the surface we
intend to ship and with three fixes.
- Ship 4 capability nodes (text-to-image, text-to-video, image-to-video,
image-edit) plus 7 named workflows; drop the other 16 POC classes. The
capability aliases resolve backend-side, so the model behind a shape can be
swapped without a client release.
- Pin output-URL validation to the backend's bucket allowlist rather than
accepting any bucket on storage.googleapis.com.
- Name the three timeouts (platform run ceiling, poll budget, output download)
and poll under an explicit budget instead of the stock 40-minute default.
- Narrow the workflow contract to the shipped set.
Tests: 67 pass across tests-unit/comfy_api_nodes_test.
* feat(api-nodes): add the Z-Image Turbo Comfy Cloud node
Z-Image Turbo is the most selected Desktop template by a wide margin
(168k selections in 90 days, 34% ahead of the next), and it was not in
the curated set. At 8 steps it also finishes far quicker than the
existing text-to-image pipeline, which matters on a surface billed per
GPU-second.
* feat(api-nodes): add Flux 2, Ideogram 4, LongCat and Capybara cloud nodes
Four new OSS model families, each biased to the newest shipped templates
and each validated by running its graph on a real GPU before the manifest
was frozen. Duplicates were deliberately left out: Krea 2 Turbo and
Z-Image Turbo int8 are models already curated at a different
quantisation, and shipping them would give a user two nodes that do the
same thing.
* fix(api-nodes): accept the partner-node asset buckets for Comfy Cloud outputs
Staging and prod point PARTNER_NODES_ASSET_STORAGE_BUCKET_NAME, the key
comfy-cloud shares with the other partner providers, at
partner-nodes-assets and partner-nodes-assets-staging. The node accepted
only comfy-cloud-assets*, so an output from either would have been
rejected after the GPU had already run and the caller had been billed.
Both bucket families are ours; what this list defends against is an output
URL naming a bucket we do not own, and the signed-URL and host checks are
unchanged. A test pins the set, because the matching list lives in cloud
and this half ships on the ComfyUI release train, so a one-sided widening
would persist until the next release.
* fix(api-nodes): quote the rate Comfy Cloud actually charges
The badge advertised $0.001295/GPU-second while Metronome charged
$0.00185, so every run under-quoted the user by about 43%. Two live runs
billed at the higher rate, and the live rate card confirms it: the
'GPU Hour Usage' metric sums gpu_seconds and prices per gpu_type, with
rtx_pro_6000 at 0.185 cents per GPU-second.
0.001295 was a hardcoded constant with no counterpart anywhere in cloud,
so it was never anchored to the rate card. This one is not either, which
is why the comment says so plainly: nothing links them, and a pricing
change has to be mirrored here by hand and ride the release train.
* fix(api-nodes): show a real message when Comfy Cloud is unavailable
Two problems met on the same path. comfy-api's error envelope is flat,
{"error": code, "message": text}, but _friendly_http_message only
unwrapped a NESTED error object, so the message was skipped and the user
was shown the raw JSON of the whole body. And 503 is in _RETRY_STATUS, so
a provider that is switched off was retried three times with backoff
before producing that.
The user pressed Run and waited through the backoff to read a payload. Now
the flat envelope is surfaced verbatim, which also fixes every other
comfy-api error a node can hit, and a refusal carrying a known terminal
code short-circuits the retry. A 503 without such a code is still
transient and still retried.
* refactor(api-nodes): name every Comfy Cloud node for its provider
Twelve of the sixteen nodes carried a bare model name, so a user searching
"flux" saw the paid cloud node and the free local one side by side with
nothing on the name to tell them apart. Prefix them all, matching the
convention every other partner uses (Bria FIBO Image Edit, ElevenLabs Text
to Speech), and drop the task suffix where the model only ships one.
node_id is unchanged: it is the class_type saved inside user workflows.
* refactor(api-nodes): describe what each Comfy Cloud node actually does
Sixteen nodes shared three generic bodies ("Runs this image workflow on Comfy
Cloud"), so the tooltip never said which model runs or why you would pick one
node over the one beside it. Each node now describes its own graph: the model,
the sampler budget where it drives cost, and the constraints worth knowing
before you wire it up (Krea 2 bakes in the darkbrush LoRA, LTX cannot exceed
the supplied audio, MiniMax generates its soundtrack in the same pass).
The rate suffix stated one number five ways over two lines. It now states it
once and answers the question a local user is actually asking, which is whether
they need a Cloud plan. It stays in the description rather than moving to the
price badge because that badge only renders on Nodes 2.0, and a plain local
install still defaults to the classic canvas.
Drops COMFY_CLOUD_GPU_HOUR_USD and COMFY_CLOUD_GPU_HOUR_CREDITS, which after
this have no callers outside the test asserting them.
* fix(api-nodes): apply the Comfy Cloud bucket allowlist to the normalized path
Three findings from review, all in the client half.
The bucket allowlist read the first segment of the path as written, but a
client resolves dot segments before it sends the request. So
.../comfy-cloud-assets/../other-bucket/output.png advertised an allowed
bucket to the check and fetched from another one on the wire, which is the
whole point of pinning the bucket. The proxy branch already normalized; the
signed-URL branch now does too, and two traversal cases join the reject
parametrize.
_with_input_sockets clears socketless on every widget, so a graph can link
any node output into prompt, instruction or negative_prompt. A linked int
reached value.strip() and raised AttributeError instead of naming the field
the way the BOOLEAN, INT and FLOAT branches all do.
_UINT32_MAX had no references, and the oversized-image test only ever
reached the per-side branch: (1, 8193, 1, 3) is 8193 pixels total, so the
megapixel limit was never exercised even though the shared error text made
the assertion pass.
Left alone deliberately: reparenting the four video nodes onto
_ComfyCloudWorkflowNode is a redesign of the execution path this port is
meant to carry across unchanged; bounding decoded audio by its peak float32
representation rather than its raw size is a real tightening but sits in
shared decode used by every API node, and the guard this PR adds is already
a bound where there was none; and exempting disk-backed file handles from
the in-memory download cap changes behaviour that a test here asserts on
purpose, with no caller passing such a handle today.
* refactor(api-nodes): cut the Comfy Cloud surface nothing here uses
A leanness pass ahead of core review. No behaviour change to the execution
path, polling, cancel wiring, auth or the output-URL allowlist.
Shared util/ narrowed to what this feature requires. Three of the four
shared files were carrying changes Comfy Cloud does not need, and two of
them would have altered behaviour for every other API node:
- util/conversions.py and util/__init__.py revert entirely. The 256 MiB
decoded-audio bound guarded audio_bytes_to_audio_input, which this PR
never calls but elevenlabs, fishaudio, sonilo, heygen and bytedance all
do; it would have started rejecting large decodes for them. The
bytes | BytesIO widening existed only for download_url_to_audio_input.
- download_url_to_audio_input is removed. It had no caller anywhere,
having come across with the port for the audio nodes this PR drops.
This was disclosed as a loose end in the PR body.
- The 512 MiB in-memory download cap is removed for the same reason: not
required here, and every existing BytesIO download would have gained a
new failure mode from an unrelated PR.
- The can_reset_sink guards are removed as dead defence. Every sink in
the tree is a path, a BytesIO or an open file, all of them resettable.
- allow_redirects comes off download_url_to_file_3d, which no node here
calls.
What stays in util/ is exactly what the feature needs, and each now says
why in the diff: allow_redirects on the two download helpers Comfy Cloud
uses (a redirect would fetch from a bucket _validated_output_url never
vetted), the matching status >= 300 check so a refused redirect fails
instead of writing the redirect body as the image, the BytesIO reset
before a retry so a mid-stream failure does not concatenate two bodies,
and client.py's flat-envelope message and terminal-refusal no-retry,
which the kill switch depends on.
Request model narrowed to the shipped inputs. ComfyCloudWorkflowInputs
declared 43 fields; 15 are set by the 16 nodes. The other 28 are the
POC's dropped audio and 3D nodes. Same argument the PR already makes for
narrowing ComfyCloudWorkflow. _TEXT_LIMITS loses the 7 keys that name no
input id on any shipped node.
Node boilerplate collapsed, not restructured. Five nodes differed only in
workflow id, name and blurb, each repeating an identical define_schema and
execute; they now subclass _ComfyCloudPromptSeedImageNode and declare
nothing else. The three copies of the IO.Schema construction collapse into
_cloud_schema, which also makes cls.category load-bearing instead of being
set and ignored. The four video nodes keep their own base and
_run_video_workflow, unchanged.
Also: the bucket-allowlist comment pointed at server/middleware/comfy_cloud.go
for a set declared in config/config.go; the two MiniMax nodes repeated the
eight aspect ratios inline, in a different order from _ASPECT_RATIOS but the
same set, and now share it; the best-effort cancel says why it swallows
rather than using a bare pass; the video section had lines up to 353 chars
against a ~120 norm elsewhere in comfy_api_nodes.
Tests: the two removed util bounds take their tests with them; the retry
reset keeps its coverage and the refused redirect gains some. Test names
carrying POC vocabulary ("legacy nodes" for what the PR calls the primary
capability surface, IMAGE_POC_NODES) are renamed for what they cover, and
Z-Image Turbo joins the parametrized set so the new shared base is
exercised end to end.
tests-unit 1547 passed / 10 skipped, ruff clean, pylint 10.00/10.
* fix(api-nodes): let the Wan 2.2 graph supply its own negative prompt
The negative_prompt widget shipped with the default "graph tested Chinese
quality negative". That is a note describing a value, not the value.
It is not only cosmetic. The backend holds the real one --
comfyCloudWan22DefaultNegativePrompt in comfy_cloud_workflows.go, the
30-term Chinese quality negative the frozen graph was tuned with -- and
binds it to node 78 only when the caller supplies none:
negative := comfyCloudWan22DefaultNegativePrompt
if inputs.NegativePrompt != nil { negative = *inputs.NegativePrompt }
negative_prompt is a required argument of execute() and was always sent,
so that branch was unreachable from ComfyUI and every default run replaced
a 30-term quality negative with a five-word English placeholder.
CodeRabbit flagged the placeholder and proposed default="". That does not
fix it: "" is still non-nil on the wire, so the graph default stays
unreachable and every default run submits an empty negative instead.
Default to empty AND omit the field when it is blank, so an untouched
widget reaches the backend as absent and the graph's own negative applies.
A typed value still wins. Nothing is mirrored client-side, so there is no
second copy of a frozen-graph constant to drift -- the failure mode this
PR already calls out for the price badge.
Verified against cloud 6341e21e9c: the manifest's node 78 text and the Go
constant are the same string.
tests-unit/comfy_api_nodes_test 80 passed, full tests-unit 1550 passed /
10 skipped, ruff clean, pylint 10.00/10.
* refactor(api-nodes): cut the Comfy Cloud comments back to the repo norm
The file ran at 6.2% comment lines against 0.1-1.0% for every other partner
node. What survives is what cannot be read off the code: the bucket set's
coupling to cloud's config.go, the price being a hand-maintained mirror of the
rate card, why the output path is normalised before the allowlist is applied,
and why negative_prompt is omitted rather than sent empty.
* refactor(api-nodes): stop requiring response fields the node never reads
polling_url and cancel_url are declared on the generate response but nothing
reads them: _task_endpoints derives both paths from task_id, which is what
keeps a backend-supplied URL from being followed. Requiring them could only
ever fail a call over a field we ignore, so they are optional now. Drops
output_urls, which nothing reads at all.
* refactor(api-nodes): put the task back in every Comfy Cloud node name
Dropping the task where a model shipped only one node left names like
"Comfy Cloud Krea 2" and "Comfy Cloud LongCat", which say nothing about what
the node does. Every other provider with a large surface keeps it: Kling 3.0
Image to Video, Wan 2.7 Text to Video, ByteDance First-Last-Frame to Video.
Also adopts the house spellings for the shapes that have one: First-Last-Frame
to Video rather than First & Last Frame, Upscale Image rather than Upscale, and
"with Audio" for the MiniMax pair, which is how Kling marks the same thing.
node_id is untouched.
* refactor(api-nodes): give the workflow ids a shape that can outlive v1
The ids are the wire contract and they freeze the moment a release carries
them: an old client keeps sending the old string forever, and cloud deploys
independently of the release train, so renaming one later is a break rather
than a refactor. The old set could not survive that. "text-to-image" claimed a
generic name unqualified; "creative-image" and "image-audio-performance" were
template filenames rather than descriptions; "2-3", "0-1" and "2-2" mangled
version numbers; and "image.qwen-image-edit-2511.v1" stuttered.
Now <model>/<task>, mirroring the node display names so the two cannot drift:
"Comfy Cloud Flux 2 Text to Image" is flux-2/text-to-image.
The four aliases become default/*, which is the part that carries meaning: the
prefix says the model behind the id can change while the id does not. Nothing
in the old names said so, and that was the most dangerous ambiguity in the set.
Drops the .v1 suffix. The id is frozen either way, so a pre-registered v1 buys
nothing; an incompatible change needs a new id regardless.
* feat(api-nodes): expose the rest of each Comfy Cloud pipeline's controls
The nodes showed a prompt, sometimes a seed, and hid everything the
curated graph could actually do. Flux 2 has a turbo switch and two step
counts, Ideogram 4 has its own rendering-speed presets, LongCat and
Capybara have negative prompts, Wan 2.2 and MiniMax H3 have a resolution
stage, SeedVR2 has a seed and four colour-correction modes. All of it is
on the node now.
The split is what keeps the node readable: the first view stays the
controls someone actually reaches for, and the sampler dials sit behind
the advanced flag. Aspect ratio is a dropdown even where the graph takes
a width and a height, because the ratio is the choice and the pixels are
this pipeline's own render size.
Seeds now start at 42 everywhere. They were 0 on the image nodes and
fifteen-digit constants on the video ones, which read as meaningful and
was not.
* feat(api-nodes): cut the shipped set to seven nodes
Matches the registry: four capability nodes, plus MiniMax H3 text-to-video and
Z-Image Turbo (the only two with real Desktop template demand) and Flux 2, which
is what exercises the turbo switch and the advanced input split.
The nine removed nodes keep their node_id and workflow id in history, so
re-adding one is a revert rather than a redesign.
* feat(api-nodes): give Flux 2 a LoRA picker and both image nodes a model picker
The advanced section of the two controlled image nodes tuned the schedule but
never the weights, so Flux 2 could only ever run the one Turbo LoRA and Z-Image
Turbo the one checkpoint.
Both pickers are combos of KEYS, not filenames. Cloud maps each key to the file
it holds, which is what keeps the picker from becoming a free-text weight path
the caller could point anywhere.
- Flux 2 gains `lora` (12 options: two turbo accelerators plus ten styles) and
`model` (fp8 or bf16 dev). The LoRA is loaded only while turbo is on, so
turbo off is still the no-LoRA dev pass; the tooltip says so, and says a
style entry wants turbo_steps raised.
- Z-Image Turbo gains `model` across its three distilled precisions.
Both are advanced, so the plain view is unchanged at four controls and five.
* feat(api-nodes): give the two default image nodes a speed switch and weight picker
The capability nodes showed a prompt and a seed and hid two controls their
pipelines already carried: a speed/quality switch, and behind it the weights.
The pickers are combos of KEYS naming the TRADE-OFF, not the model. A default/*
id is a pointer whose model is re-pointed over time, and a saved graph stores
the key, so the key has to outlive the move: fast, balanced and quality do, a
model name would not, and a test holds that line.
- Text to Image gains turbo in the plain view, the way Flux 2 has it, plus
model across two precisions and lora across two accelerators, both advanced.
The LoRA only matters while turbo is on and the tooltip says so.
- Image Edit gains turbo and model across three precisions. That graph has no
LoRA to point at, so there is no lora input, and its tooltip is honest that
the switch is a bare short schedule with nothing to soften the loss.
- Text to Video and Image to Video are unchanged. Their pipelines have no
interchangeable sibling weight.
Plain view stays at three controls and four.
* feat(api-nodes): add the default-pointer pickers and name pinned weights fully
Adds model/lora/turbo controls to the two default image pointers, and renames
the pinned pickers' options from "bf16"/"dev-fp8" to the full model names, which
is what a local subgraph picker shows for the same weight.
The default/* options stay trade-off tiers rather than model names, since those
ids re-point and a saved graph stores the option.
* refactor(api-nodes): drop the four capability nodes
Mirrors the registry. Tests that used them as generic stand-ins now run against
a shipped node and drive it from its own schema defaults, so the next workflow
added does not break them again.
Two tests went rather than being contorted: the batch-rejection guard, which no
shipped workflow can reach now that nothing takes an IMAGE input, and the
key-format assertion, which contradicted naming the pickers after their weights.
What protects that path is the server-side allowlist, which the Go side tests.
* feat(api-nodes): let a Comfy Cloud node return audio or a 3D model
Output was a boolean, image-or-video, which blocked the 3D and audio workflows
in the new catalog. It is now an explicit kind with a table mapping each to its
schema output and its runner.
The download helpers already existed; download_url_to_file_3d just could not
refuse a redirect, so allow_redirects is threaded through it. That matters:
the bucket pin decides where output bytes may come from, and following a
redirect hands that decision back to the server we just checked. A test asserts
every kind passes allow_redirects=False, and it fails if any one stops.
* feat(api-nodes): add the Mage-Flow and MiniMax Music 3 Comfy Cloud nodes
Two Mage-Flow text-to-image nodes, full and turbo, and the first node to use
the audio output kind: MiniMax Music 3 text to audio.
Each exposes every widget its frozen graph carries, because control is the
reason to reach for a pipeline like this rather than a hosted endpoint. The
plain view keeps the prompt, the framing, the seed and one quality-against-
speed switch; the rest is advanced. Mage-Flow frames itself with a ratio and a
pixel budget rather than a width and a height, because its graph sizes through
a resolution selector. MiniMax Music 3 takes lyrics as a second prompt, caps
length with max_duration, and switches between a tiled and a single-pass
waveform decode.
The two Mage-Flow nodes share a base class: the same pipeline at two schedule
lengths, differing only in step and cfg defaults and which checkpoints load.
Its seed is capped at int64 rather than uint64, which is what the music graph's
seed node accepts.
Sampler and scheduler are three entries each rather than ComfyUI's 45 and 9:
the dropdowns list only options executed against these graphs on a cloud GPU,
since cloud rejects a key its allowlist does not hold.
* feat(api-nodes): add the three MiniMax H3 video Comfy Cloud nodes
Image to Video takes a first frame and an optional last frame, running
fl2va between the two keyframes when both are connected. Video
Continuation takes the closing frame of a previous clip so several can
be chained into a longer sequence. Reference to Video takes up to four
reference images through an autogrow input, numbered by connection
order so the prompt can address them as <Picture 1> upwards.
All three open on the headline controls -- media, prompt, aspect ratio,
resolution, duration and seed, plus reference-to-video's ref_image_size
quality-against-speed switch -- and keep steps, denoise and the text
encoder picker behind the advanced flag.
The turbo LoRA branch these templates carry is not exposed: its
candidate weights have catalog entries but no bytes in the mirror, so
cloud's frozen graphs leave the branch out until they are uploaded.
* fix(api-nodes): give MiniMax H3 duration whole seconds
The graph quantises length to 17-frame steps at 24fps, so a 0.01s step
offered 1001 settable values that collapsed to 15 outcomes, and both ends
escaped their own bounds (5.00 -> 5.17s, 15.00 -> 15.08s). Only 8.00 was
honest. An int slider matches Kling and LTXV, the two other video nodes
here, and reaches 11 of those 15 lengths; the four it drops are decimals
nobody would type.
Server-side stays FLOAT: node 132 is a PrimitiveFloat and an integer
decodes into it cleanly, so no manifest or digest changes.
* refactor(api-nodes): rename the MiniMax H3 video nodes for what they do
Per review. The node with two keyframes was called Image to Video and the
single-keyframe node was called Video Continuation, which reads backwards:
Image to Video -> First-Last Frame to Video
Video Continuation -> Image to Video
Text to Video with Audio -> Text to Video
Class names, workflow ids and descriptions all follow, so class_type, id
and display name cannot drift. "with Audio" goes because every H3 video
node produces sound; naming it on one implied the others do not.
Also reorders the shared inputs to seed, aspect ratio, shot shape, then
sampling, then weights, and applies the same order to every node so
moving between them does not move the knobs.
Renaming a class_type is free only until a release carries it.
* feat(api-nodes): expose sampler, scheduler and text encoder on the Cloud nodes
Per review. Mirrors the cloud bindings:
Flux 2 sampler + text_encoder
Z-Image Turbo sampler + scheduler + text_encoder
MiniMax H3 sampler + scheduler on the three registry-backed nodes
Flux 2 has no scheduler to expose; its tooltip says why so it does not
read as an omission.
Not added: the one-entry VAE pickers and an H3 model picker. Each has a
single valid weight, so the control cannot change the output, and an
input is far harder to remove than to add.
Text to Video still runs the legacy builder and keeps its smaller surface
until that is migrated.
* feat(api-nodes): give Text to Video the same surface as its siblings
It was the only H3 node still built from its own input helper, so it
offered 6 inputs where the others offered 11 or 12. Moves it onto
_minimax_h3_inputs and re-homes the class beside the other H3 nodes so it
sits after the shared helper it now uses.
Adds denoise, sampler, scheduler and text_encoder to match, and retires
the now-unused _minimax_inputs helper.
* refactor(comfy-cloud): simplify node surfaces per design review
Applies alexisrolland's review. Categories become comfy cloud/{image,
video,audio}; widget order is now prompt, negative_prompt, seed,
aspect_ratio across every node; and the weights, sampler and step
pickers users should not have to touch are gone.
Removing a widget is behaviour-preserving here, verified against the
frozen graphs rather than assumed: every value dropped is already baked
into the manifest at the same default the node was sending. z-image
bakes steps 8, shift 3, res_multistep/simple and z_image_turbo_bf16;
flux-2 bakes euler, guidance 4, flux2_dev_fp8mixed and the turbo LoRA at
strength 1; mage-flow and mage-flow-turbo each bake their own steps
(30/4), cfg (5/1) and checkpoint, which is why the two variants no
longer need per-subclass defaults at all.
Also: negative_prompt and audio_quality are no longer advanced (a second
prompt and a headline output choice, not tuning dials); tiled_decode and
the tile controls are gone from Music 3 as lower-end-hardware knobs that
do not apply on cloud; seed moves after the lyrics prompt.
Constants left with no caller are deleted with their widgets.
NOT applied, and answered on the PR: megapixels on Flux 2 and Z-Image.
Only mage-flow has a megapixels binding on the cloud side; those two
graphs size themselves from explicit width/height, so the widget would
need a cloud manifest change to do anything.
* feat(comfy-cloud): mark the nodes BETA in name and description
The curated workflow set is expected to change: a node can gain or lose
options, and a workflow can be retired. Say so on the node rather than
only in release notes.
Suffix, not prefix. The add-node menu and node search sort
alphabetically, so a leading "[BETA]" would file all nine under "["
instead of under their model, and someone searching "flux" would no
longer see the node next to the local one it sits beside.
The description carries the same notice and leads with it, because the
price badge only renders on Nodes 2.0 and a plain local install still
defaults to the classic canvas, where the description is all the user
gets.
Free to remove later: display_name is not stored in a saved graph.
node_id and the input names are the parts that are permanent.
* fix(comfy-cloud): accept the first-last-frame workflow id (BE-12049)
ComfyCloudMiniMaxH3FirstLastFrameToVideoNode submits
"minimax-h3/first-last-frame-to-video", but the ComfyCloudWorkflow union
still named the id by its old name, "minimax-h3/video-continuation". The
union is what pydantic validates the outgoing request against, so that
one node failed with a ValidationError on Run while the other eight
worked. Found by QA on staging.
A rename that reached the node and the server registry but not the
client union.
Why nothing caught it: the only parity check read workflow ids out of
CONTROLLED_IMAGE_NODES, which covers the two image nodes, and asserted a
subset. Video and audio ids were never compared, and a union entry
naming no node was invisible in either direction.
Replaced with an exact two-way comparison between the ids the module
names and the ids the union declares. Ids are matched by shape rather
than by call site, because the three ways a node reaches its id differ:
the H3 nodes pass a literal, Mage Flow passes a ClassVar, and Music 3
goes through the _OUTPUT_KINDS dispatch table. Verified the test fails
on the original bug before fixing it.
* Apply suggestion from @alexisrolland
* docs(comfy-cloud): the node rate is the list price, never a promo
ComfyUI is pulled rather than pushed, so a user keeps whatever value they
last pulled. A promotional rate shipped in this constant does not expire
when the promotion does: the rate card returns to list, that user's node
still quotes the discount, and they are charged more than they were
shown, indefinitely and invisibly to us.
Quoting above the charged rate is the safe direction. Quoting below it is
the failure this comment exists to prevent, and it is the same shape as
the 43% under-quote this file already corrected once.
Records why the weekend promo runs in Metronome only.
* chore(comfy-cloud): keep sending caption_cfg after its widget was removed
Alexis's suggestion removed the caption_cfg widget. The value is still
sent, deliberately, because this one differs from every other control
dropped in this pass: the node default and the frozen graph DISAGREE.
The manifest bakes cfg_scale 1.7 at node 37:13 while the node has always
sent 1.5, so going silent would change what the GPU runs rather than
preserve it, and every run QA'd so far used 1.5.
Reconciling the two is a manifest decision rather than a node one.
* Use resolution selectors for Cloud image nodes
Amp-Thread-ID: https://ampcode.com/threads/T-01a06d0c-5a85-77c9-9753-cd5bb340998e
Co-authored-by: Amp <amp@ampcode.com>
* fix(comfy-cloud): the GPU rate is 0.001295, not 0.00185
Reverts a wrong "correction". An earlier commit changed the advertised
rate from 0.001295 to 0.00185, read off rtx_pro_6000's Metronome rate
card entry. The card entry is not what this path charges, and the node
has been over-quoting every run by about 43% since.
Verified by dividing a real billed event rather than reading a card:
credits_used 1.98 / gpu_seconds 7.244173 = 0.273323 credits/GPU-second
0.273323 / 211 credits-per-USD = $0.0012954/GPU-second
Which is 0.001295, the original value.
Display precision goes to 6dp so the quote reads $0.001295 rather than
rounding to $0.00129 and being wrong in the other direction.
The comment now says to verify against a charge, not the rate card,
because that is the mistake that produced this.
Note the live rate is currently HALVED for a promotion. That number is
deliberately not here: this constant is the list price, for the
pull-based reason recorded above it.
* Update nodes
* Fix linting
* Disable unit test for disabled nodes
---------
Co-authored-by: Deep Mehta <deep@comfy.org>
Co-authored-by: Alexis Rolland <alexisrolland@hotmail.com>
Co-authored-by: Amp <amp@ampcode.com>
* [Partner Nodes] feat(MiniMax): add H3 Max option to H3 text-to-video and first-last-frame nodes
Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
* [Partner Nodes] fix(downscale): never make a downscaled image more elongated than its source
Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
* [Partner Nodes] fix(M3-Max): add check fpr max prompt length
Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
* [Partner Nodes] chore(Minimax): correct route name
Signed-off-by: bigcat88 <bigcat88@icloud.com>
---------
Signed-off-by: Alexander Piskun <bigcat88@icloud.com>
Signed-off-by: bigcat88 <bigcat88@icloud.com>
Co-authored-by: Alexander Piskun <13381981+bigcat88@users.noreply.github.com>
Co-authored-by: Alexander Piskun <bigcat88@icloud.com>
* fix: respect user directory for default database
* refactor: use None default for --database-url instead of argv scan
Per review: default --database-url to None and treat a non-None value
as explicit at resolution time. Removes the sys.argv scan and the
database_url_explicit attribute. database_default_path now serves
directly as the legacy copy source. Adds regression tests for the
unchanged no-flag default path and explicit URLs at the legacy
location.
* refactor: rename legacy database to .bak after copy
Per review: after copying the legacy install-dir database to the
effective user directory, rename the original to comfyui.db.bak so a
later launch without --user-directory cannot silently fall back to a
diverged copy, while keeping the file around for recovery. Also hoist
the database_default_path import to module scope.
* fix: guard legacy migration on existing .bak and rename before copy
Per review: bail out of the legacy migration when comfyui.db.bak
already exists, so only the first run migrates and later launches with
a fresh --user-directory cannot grab a database another instance is
using. Rename before copy so os.replace fails fast if the legacy DB is
held open by a running instance.
---------
Co-authored-by: guill <jacob.e.segal@gmail.com>
preview_url was assembled from a /api/view link whose type was chosen by
matching the asset's tags against "input" then "output". Anything written
anywhere else - temp above all, where preview nodes put their images - fell
off the end of that chain and came back with no preview at all. Tags are
user-editable, so removing one also silently destroyed the URL.
Derive the URL from where the file actually sits instead. That covers every
root /api/view serves, temp included, and no longer depends on tags or on a
filename in user_metadata. A file outside those roots, or content no client
can render from its own bytes, gets no preview URL rather than one that
cannot work.
Nominated previews are resolved a page at a time rather than per row, so a
list costs one extra query however long it is. A preview that is soft-deleted
or not visible to the caller drops out of that lookup and is no longer
advertised.
Co-authored-by: guill <jacob.e.segal@gmail.com>
* Keep temp-directory assets visible while their files exist
Assets written to the temp directory were flagged as missing and dropped
from GET /api/assets, even with the file sitting on disk. One list of
directories was answering two different questions -- where the scanner
looks for new files, and which files ComfyUI considers its own -- and
temp belongs only in the second, so every temp reference was disowned by
the prune that runs at startup and on POST /api/assets/prune.
Ownership now covers temp. Discovery still does not: the temp directory
is wiped before the scan runs, and assets written there are already
registered with a hash, mime type and dimensions, so walking it would
find nothing. Temp references are instead reconciled against the
filesystem directly, so a temp file that really is gone is still retired
rather than lingering as a broken entry.
get_prefixes_for_root becomes get_scan_prefixes_for_root so the two
questions are told apart by name rather than by comment.
* Cover the unhashed temp asset in the reconciliation tests
The existing temp tests all registered hashed assets, so they never
exercised the path an unhashed asset takes when its file is gone: the
orphaned rows are removed rather than kept as missing, exactly as under
any other root.
* Stop adding an opaque alpha channel to API node images
bytesio_to_image_tensor converted every downloaded image to RGBA, so nodes
whose API returns no transparency still emitted a 4 channel IMAGE. Keep the
alpha when the decoded image has one, stay RGB when it does not.
---------
Signed-off-by: bigcat88 <bigcat88@icloud.com>
Co-authored-by: bigcat88 <bigcat88@icloud.com>
VAEDecode unwraps a NestedTensor latent (video/audio pair) to its
video component before calling vae.decode(). VAEDecodeTiled skipped
this unwrap and passed the NestedTensor straight into
vae.decode_tiled(), which fails deep in the MiniMax H3 video VAE when
a real tensor's .to() is called with the NestedTensor as an argument.
Fixes#15468.
* Implement tags_all/tags_any/tags_none on the assets list API (BE-6600)
Adds the three canonically-named tag filter params to GET /api/assets and
GET /api/assets/tags/refine:
- tags_all: asset carries every tag (replaces include_tags)
- tags_any: asset carries at least one tag (new)
- tags_none: asset carries no tag (replaces exclude_tags)
Clauses intersect; tags_none always wins. include_tags/exclude_tags remain
as permanent deprecated aliases and behave exactly as before when used on
their own.
Invalid combinations return 400 INVALID_TAG_FILTER, but only when the
request uses at least one new-name parameter (non-empty after
normalisation):
- mixed spellings of one slot (include_tags with tags_all, exclude_tags
with tags_none)
- the same tag in the effective all-list and none-list (query can never
match)
Old-names-only requests gain no new error paths: include_tags=a&exclude_tags=a
still returns an empty 200. tags_any/tags_none overlap stays valid (dead
term, not a dead query).
* Address review findings: positional-compat, deprecation metadata, test matrix
- Move any_tags to the end of the four touched signatures: inserting it
mid-signature silently misbound pre-existing positional callers (e.g.
a caller passing name_contains positionally would have it consumed as
any_tags).
- Mark include_tags/exclude_tags Field(deprecated=True) on both list
schemas so generated schema metadata matches the contract, not just a
comment (schemas_out.py already uses this form for Asset.name).
- Add tests: legal cross-slot old/new combinations, repeated query-key
concatenation (pins Core behavior; outside the cross-platform
contract), tags_any two-page cursor consistency (total/has_more/
no-overlap), refine-route mixed-spelling rejection + legacy-conflict
preservation, and schema deprecation metadata.
* Pin tag-value opacity: case-sensitive matching, byte-exact conflict check
The prod tag survey (~/comfy/prod-model-tag-shape.md) found live
case-distinct tag pairs (SEEDVR2/seedvr2) that resolve differently, so
the contract now states tag values are opaque byte-strings. Pin that:
case-distinct tags filter separately, and a case-distinct all/none pair
is not an INVALID_TAG_FILTER conflict.
* Document tags_all/tags_any/tags_none in openapi.yaml, deprecate aliases
Add the three tag-filter parameters to both listAssets and
getAssetTagHistogram parameter blocks and mark include_tags/exclude_tags
deprecated: true, keeping the spec in step with the runtime schemas so
generated clients can discover the new filters while the aliases stay
present for existing consumers.
* Move schemas_in import to module scope in test_list_filter
Review feedback: no import cycle requires the local import.
* Silence per-request DeprecationWarning in the tag-filter remap shim
Reading the deprecated include_tags/exclude_tags fields by attribute
fires pydantic's DeprecationWarning on every list/refine request even
for callers using only the new names. The warning is aimed at API
clients, not the server's own remap; read via model_dump instead.
* Cap tag-filter lists at 100 entries, all spellings
Review finding: unbounded tag lists fan out into one correlated EXISTS
per tag on both page and count statements. Cap each list at 100
normalized entries with 400 INVALID_TAG_FILTER naming the parameter.
Applies to the legacy spellings as well — a deliberate, decided
exception to the old-names-behave-identically rule, since a cap only on
new names would leave the same fan-out reachable through the aliases.
* Strip process narration from comments
Comments carried decision dates, contract cross-references, and review
context. Keep only the constraints the code cannot show, one line each.
Simon Pinfold