Adversarial review round 1 caught this: out is not provably this hook's own
private tensor. A chained h3_block_repeat span_loop passthrough returns
args["img"] verbatim with no copy for non-head blocks in its span, and
h3_repr_steering installs as the OUTER wrapper around it (_install_block_
influence then _install_h3_repr_steering in _sample_chunk) -- so `out` here
can be the H3 model loop's own live `h`. Removing the clone entirely (as the
previous commit did) reintroduced exactly the aliasing hazard block_repeat's
own _repeat() explicitly guards against one function away. Restored the
clone, kept unconditional (not gated behind the safe/unsafe branch) so it
still doesn't need the host sync this fix exists to remove.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
comfy_aimdo's dynamic-VRAM block prefetch pipelines weight cast/unpin against
CUDA stream timing; a CPU sync (bool()/item() of a CUDA tensor) or a surprise
allocation mid-loop can race it into reclaiming a weight buffer a still-queued
kernel needs -- a silent segfault, no Python exception possible. All three H3
rating-driven dit hooks did this on every block, every step:
- h3_repr_steering's injection guard and h3_q_steering's injection guard both
did `if torch.isfinite(x) and x > 0:` (forces a host sync) before writing
the steered rows, and h3_repr_steering cloned the whole block output first.
Replaced with a GPU-only torch.where guard; dropped the now-unnecessary
clone in h3_repr_steering (out is this hook's own private tensor already).
- h3_repr_steering.capture() now .cpu()s its descriptor immediately instead
of leaving it on GPU competing for VRAM headroom for the rest of the run.
- h3_av_decouple's attention override re-checked `.any()` on its video/audio/
other masks every single attention call; now decided once per unique
seq_len alongside the mask itself (typically once per generation).
Unvalidated against a real GPU (none available here) -- worth confirming on
the rental that REINS no longer crashes silently under dynamic VRAM offload.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The direct "ref:<id>" picker for a widget-level reference (VHS_LoadVideo's
"video", etc.) forced reselecting the field by hand every time the
underlying reference file changed. Adds the same numbered indirection
Input Sources already gives sockets: "Reference video 1" is whatever is
marked first among video references, not a specific file, so re-marking
in the Media Bin re-points the widget with no node-config change.
Adversarial review of this addition, run to convergence, found and fixed
in order:
- a real installed file whose name collided with the sentinel pattern
("ref#video:1", "ref:whatever") could be silently hijacked instead of
used as-is -- fixed by having a literal installed choice always win,
identically in the builder and the picker;
- the self-heal fallback for a stale plain value could reach into
reference-slot space and silently attach a live Media Bin dependency
the user never picked;
- fixing that reopened the underlying onChange(undefined) hole the
fallback chain had been papering over, wiping a stale plain value
outright when nothing was installed -- restored the original
choices-only self-heal invariant explicitly;
- a reference-capable widget could be added to a Linked Inputs group,
whose own value editor builds a spec with no ref_kind -- silently
wiping the shared reference across every member on next render.
Simplest safe fix: that kind of field doesn't link.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A reference's file lives in ComfyUI's own input folder (so the node can
load it), which means it was also showing up a second time in the
combo's plain choice list under its on-disk name
(funpack_movie_<id>.mp4) -- indistinguishable from any other reference,
right next to the friendly "R1 - name" entry for the same file. Drop
the raw duplicate; the friendly one is always the better pick.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial review of the previous commit found two real gaps in the
"reference into a file-upload widget" feature: unmarking a wired
reference in the Media Bin silently rewrote and persisted a plain
default over it on the node panel's next render (the exact silent
substitution the feature was meant to avoid, just via a path the
builder's own report never sees), and a wrong-kind reference reported
the same "no longer in the media bin" message as a genuinely missing
one, sending the user to check the wrong thing.
A second pass on the fix itself found the new "is this a reference"
check wasn't scoped to reference-capable fields, so an ordinary combo
with an installed file literally named "ref:..." would misreport.
Scoped it to spec.ref_kind.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
VHS_LoadVideo's "video" field (and LoadImage/LoadAudio/LoadVideo's own
file fields) are plain combo widgets, not wireable sockets, so a marked
reference video had no way in except a manual upload or path -- which
meant no way to force VHS_LoadVideo's own framerate/frame-cap controls
onto a reference clip without re-uploading it outside the Editor.
The node panel's combo fields for these known upload widgets now offer
the marked Media Bin references of the matching kind, using the same
"ref:<id>" values as Input sources. The builder resolves them to the
reference's filename at build time; an unresolvable or wrong-kind
reference is reported instead of silently defaulting.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The panel rebuilds its whole DOM on any relevant store change,
including marking/unmarking a reference -- which only actually
changes one card's badge. That recreated every <img>/<video> thumb
from scratch each time, so every preview visibly reloaded and every
video redecoded its first frame for an unrelated single-item toggle.
Cache the built preview node per media id and re-append (move, not
recreate) it across rebuilds instead. Verified live: toggling a
reference mark on/off now leaves the same DOM node instances in
place. Adversarially reviewed; holds up.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
For nodes with parallel image/video autogrow reference lists (MiniMax
H3's ref_images/ref_videos and anything following the same naming
convention): when both end up filled at the same index, only the
video is sent -- letting you switch between image-as-reference and
video-as-reference runs without manually clearing the one you're not
using. Per-node opt-out toggle, on by default.
Adversarial review (three rounds) found and fixed a real gap: dropping
an image reference from the middle of a multi-reference list left a
non-contiguous index sequence, which the V3 autogrow expansion this
project already relies on cannot parse. Surviving entries are now
re-indexed to close the gap.
Also fixes an unrelated bug the same review surfaced in the Linked
Inputs picker: clicking "Save link"/"Add" while an unrelated node-page
edit sat buffered in the deferSave queue silently persisted that edit
too, bypassing the normal unsaved-changes confirm. It now runs through
the same confirm/discard path as leaving the page any other way.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial review of the previous commit found two ways an
incompatible node input could silently corrupt an existing linked
input's shared value: no kind check at all, and (after fixing that) a
same-kind combo with a different choices list still slipping through
and getting overwritten via the widget's own stale-value self-heal,
which then broadcast to every other member of the link.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Models > Linked inputs could only create a new link; there was no way
to add more node inputs to one that already existed.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Round 2 of review confirmed the class-scoping fix closes the cross-wire
bug with no new defect, but flagged that the new test only exercised
two DIFFERENT loader classes -- it couldn't have caught an over-fix
into a unique node per socket, losing the original one-loader
shared-by-multiple-sockets design the class-scoped id is supposed to
preserve. Added the missing case.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial review found two real bugs in the new prevvideo feature:
1. The loader node id was a bare "prevvideo_load", copied from
prevframe's pattern -- but unlike prevframe (always LoadImage),
prevvideo's loader class varies per destination socket type
(LoadVideo vs VHS_LoadVideo). Two prevvideo sockets needing
different loaders in the same run would share one node via
graph.setdefault, silently wiring a mismatched-type output into
the second socket while the builder reported both as cleanly
wired. Fixed by scoping the id to the loader class, matching the
sibling ref#/ref: bin-reference path's own ref_load_{id}_{cls}
pattern -- prevvideo should have followed that one, not
prevframe's single-class shortcut.
2. store.js's declared-failure notice used .find() across
report.unsatisfied, so a run where BOTH a prevframe socket and a
prevvideo socket failed to retrieve the predecessor's render would
silently drop one of the two messages. Now collects every match.
Added a two-socket regression test proving the loader-scoping fix
(distinct node ids, correct class per socket).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Same idea as "prevframe" but passing the entire previous scene's
rendered clip through instead of extracting one frame from it: a new
"prevvideo" input source wires whatever LoadVideo-family node the
destination socket needs, pointed straight at the predecessor's own
TEMP file via ComfyUI's "[temp]" annotation -- no ffmpeg, no copy, no
media-bin entry.
Refactored the shared "resolve and sanitize prev_scene_media into a
safe TEMP path" logic out of _extract_prev_scene_frame into
_resolve_prev_scene_media so both paths share one place that trusts
none of the client-supplied filename/subfolder as literal paths.
Reuses the R2V pipeline's own _reference_loader/_widget_defaults to
pick LoadVideo vs VHS_LoadVideo by what the destination socket wants,
same as the existing "ref#"/"ref:" sources. Missing predecessor stays
silent; a predecessor that rendered but can't be retrieved is declared
through the same report.unsatisfied -> state.notice path as prevframe.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Round 3 found the extraction's OUTPUT filename was built from the
requesting scene id with no sanitization at all -- unlike the input
side, which round 2 already locked down. A scene_ids-driven request
(bypassing _solo()'s only_scene validation) or a project imported with
a crafted Scene.id could write the extracted frame outside ComfyUI's
TEMP directory. Fixed at the sink: scene_id is now stripped to a safe
character set before it ever reaches a path, regardless of where it
came from, plus a scene_scope membership check as defense in depth.
Also found: the "declared, not silent" warning for a failed extraction
(added in round 1) was returned by the API in report.unsatisfied, but
store.js never read that field -- it reached no one. Now surfaced
through the same state.notice path as the existing "ignored settings"
warning, concatenated rather than overwriting it when both fire in the
same run (round 4 caught the two clobbering each other).
Added a regression test proving the old code would have written outside
tempdir with this exact payload, and that the fix keeps it contained.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Second adversarial pass on the previous fix found two real regressions
it introduced: a non-dict prev_scene_media (malformed client JSON) threw
AttributeError inside the request's try block, failing the ENTIRE
generate call instead of gracefully leaving the slot unconnected; and
the extraction filename was scoped to target.scenes[0].id, which is
the first scene of whatever generative unit _solo() expanded the
request into — not the scene actually being generated — so two
concurrent solo requests for different scenes in the same unit would
clobber each other's extracted frame.
Also swapped the hand-rolled tempdir-containment check for
folder_paths.is_within_directory, which the codebase already has and
resolves symlinks before comparing (the hand-rolled version did not).
Added direct tests for _extract_prev_scene_frame itself (malformed
input, traversal, missing ffmpeg, scene-scoped naming) — round 1's
fix had none, only builder-level tests of its output.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial review of the previous commit found: the extracted frame was
written into ComfyUI's global input/ folder and never cleaned up (an
actual visible asset, contradicting the point of not touching the media
bin); the client-supplied filename/subfolder round-trip through the saved
project JSON unsanitized, allowing path traversal into ffmpeg's input;
and a missing/garbage-collected predecessor render failed identically
to "scene 1", giving the user no way to tell the two apart.
Now extracts into ComfyUI's TEMP folder (loaded via LoadImage's "[temp]"
annotation — wiped on restart like any other preview, never input/ or
the bin), sanitizes filename/subfolder against traversal, and reports a
distinct "could not be retrieved" warning when a predecessor existed but
its render is gone. Output filename is scene-scoped instead of a
process-random hash, closing the accumulation and cross-request-race
angles as a side effect. Added builder tests for all three prevframe
outcomes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
New "prevframe" input source: server extracts the preceding scene's
last rendered frame via ffmpeg into ComfyUI's input folder (never the
media bin) and the builder wires it through a plain LoadImage. Scene 1
or a solo re-run with nothing rendered before it leaves the socket
unconnected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adversarial review of the leak fix found the new tests only exercised REINS
stacked on itself, leaving 6 of 7 install mechanisms and the entire
optimized_attention_override chain (av_decouple/attn_temperature/q_steering)
untested. Add a test stacking REINS -> av_decouple -> q_steering -- different
mechanisms chaining onto each other, the realistic multi-run leak shape -- and
confirm one strip call unwinds both the per-block dit dict and the single-slot
attention override chain.
patches_replace["dit"] and optimized_attention_override installs had no tag+strip
protection, unlike the scene-wrapper and block-hook mechanisms fixed for the same bug
class earlier. An interrupted run left these hooks on the shared model; the next run
chained a new layer onto the leaked one instead of starting clean, growing unbounded
across interrupted runs -- the likely cause of the live "potential memory leak...
circular references" warnings and slow full-GC pauses.
REINS' equivalent strength dial caps at 2.0, but Q-steering only reaches the
output through softmax rather than a direct hidden-state addition, and live
testing this session found strength 1.0 barely moved the output on a small
block range. Raised well past REINS' number to give room to search before
concluding the mechanism itself is weak.
REINS ("h3_repr_steering") only ever captured while its own toggle was on --
turning it off also stopped it from learning, with no way to bank rating
history separately. New h3_repr_steering_passive_capture keeps the existing
unconditional per-block capture running (forcing strength to 0 so nothing is
ever injected) even while the main toggle is off. Exposed a real dormant bug
in REINS' own console print: it claimed "applying learned direction(s)"
whenever a direction existed, regardless of strength -- harmless before
because the toggle and strength widget always moved together, not once
passive capture legitimately runs at strength 0 with real data available.
Fixed to report "capturing only" in that case, mirroring h3_q_steering's own
already-fixed version of the same print.
Also: h3_q_steer_block/h3_q_steer_strength (from the H3 query steering
feature) were never wired into the Movie Editor's Settings UI, only exposed
as raw ComfyUI node widgets -- missing this project's own rule that every
new sampler toggle lands in the Editor in the same change. Added both,
plus the new passive-capture checkbox, to Engine Settings > Chain Guidance
alongside REINS' existing controls.
Two real faults found by a third adversarial review round on 4154746:
- _install_h3_av_decouple's own bail check (`_strength <= 0.0`) disagreed with
_sample_chunk's install guard (`h3_av_decouple > 0.0`) for a NaN strength:
every ordered comparison is False for NaN under IEEE 754, so `<= 0.0` let a
NaN through as "on" while the guard treated the same value as "off" --
reproducing the exact av_decouple-defeats-q_steering bug the last fix
closed, just reached through a different value. Both now use the same
`not (x > 0.0)` predicate so they can't disagree.
- h3_q_steering was installed before h3_explore_temperature, so if both
target the same block, temperature's Q-scaling reached q_steering's
capture before it ran -- banking a temperature-distorted "natural Q"
descriptor, contradicting the mechanism's own documented ordering promise
(capture reflects the network's natural Q, same claim h3_repr_steering
makes for hidden state). q_steering is now installed last (outermost),
after both av_decouple and attn_temperature, so it always sees Q first.
av_decouple still can't compose with q_steering at all regardless of
order (unrelated masking issue, already handled by skipping install).
Regression tests for both, plus a NaN case added to h3_av_decouple's own
existing zero-strength no-op test.
Two real faults found by a second adversarial review round on 608a97a:
- h3_av_decouple, installed after q_steering, becomes the OUTER
optimized_attention_override and always calls its inner override with a
non-None mask for the video-row partition -- exactly the case q_steering's
own override treats as "already handled, back off". With both enabled,
q_steering captured and injected nothing while still printing "applying"
and paying the clone/hook cost. The two can't currently compose, so
q_steering is now skipped entirely (with a declared reason) whenever
h3_av_decouple is also active, rather than silently no-op.
- conditioning.py's q_steer commit call was silent on no_pending/no_key,
justified by a comment claiming REINS' own commit is silent there too --
that was wrong, REINS is loud on exactly those outcomes. Fixed to match.
Also tightened the shape-mismatch warning's wording: it checks total element
count, not the heads/head_dim split, so a same-total-different-split
mismatch would still reshape onto the wrong head boundaries undetected --
documented as a known narrow gap rather than fixed, since a real fix would
mean changing the persistence format REINS itself also depends on.
Added regression tests for the av_decouple interaction (both directions:
guard fires when it should, stays quiet when it shouldn't).
Three real faults found by adversarial review of 299941a:
- sample()'s own default for h3_q_steer_block was "0,1" while INPUT_TYPES
declared "" -- any saved workflow predating this widget omits the key
entirely, so ComfyUI falls back to sample()'s default, silently turning on
capture/clone/hook for every H3 generation with a refinement key. Now both
default to blank, matching every sibling knob in this file.
- The "applying learned direction(s)" console line fired whenever a direction
was available, not when it was actually applied -- strength=0 still printed
it, which is the exact silent-success-message trap this project has already
been burned by. Now split into an "applying" line (strength>0) and a
"capturing only" line (strength=0).
- A direction banked under a different heads*head_dim layout (model swap on
the same refinement_key) hit an unguarded .view() and would hard-crash
generation instead of degrading. Now checked and skipped with a one-time
notice, same pattern as the existing batch>1 guard.
Added regression tests for all three, plus two coverage-gap tests the review
flagged (kind isolation against REAL persistence, not mocks; two named blocks
steering with independent directions in one run).
Same liked-minus-disliked mean-difference as h3_repr_steering, captured off Q inside
optimized_attention_override instead of a block's hidden state, own kind="q_steer" sidecar
so the two vector spaces never collide. Only Q is biased: a uniform additive bias on K is a
provable softmax no-op, and on V it's mathematically identical to h3_repr_steering's own
mechanism, so neither is a genuinely different lever. Off by default (blank block spec).
h3_repr_steering.py's persistence functions take an optional kind= param (default unchanged)
so this reuses capture()/direction()/commit() verbatim instead of a parallel implementation.
optimized_attention_override was a single dict key that h3_av_decouple claimed
outright -- a second mechanism on the same slot would silently replace it with
no error, the exact bug class dit_patches chaining already guards against
elsewhere. Fixed so any install chains through whatever's already there.
New h3_explore_temperature (+ block spec): flattens attention's own softmax at
named blocks instead of adding ungated latent noise -- variety stays bounded
by the prompt since it can't attend to anything not already in context.
Deliberately NOT wired to REINS or any rating data: neither mechanism's
behavior is understood well enough yet to make coupling them debuggable. A
plain manual dial, same as block repeat. Off by default.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
No way to read H3's actual trained attention-logit scale from this machine
(checkpoint isn't here) -- so normalizing the knob to 0-1 against a hidden
made-up multiplier (12.0) was a second guess wearing a calibrated costume,
and exactly the kind of thing that produces another flat live-tested curve.
The knob is now the raw softmax logit penalty directly (0-50, step 0.5), so
a live sweep searches the actual unit instead of my conversion factor.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Live 0-1 strength sweep came back flat -- confirmed the mechanism, not just the
value, was wrong. H3's Q/K get RMSNorm'd before attention, so scaling a key
toward zero pushes its logit toward the *mean* of an already near-zero-centered
distribution, not toward excluded. Switched to what actually suppresses a
softmax entry: an additive negative bias on the targeted logits, still passed
through the same per-query-group split (still no S^2 mask, still same total
FLOPs -- the bias is a per-key vector broadcast over the group's queries).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
H3 has no cross-attn to mask (unlike LTXAV's Bounded Attention), so a dense
attention mask would be GB-scale over its ~37k-token packed sequence. Uses
comfy's own optimized_attention_override extension point instead: runs the
same joint attention as separate video-query/audio-query passes against a K
where the other modality's rows are scaled down, at the same total FLOPs as
one pass. New h3_av_decouple sampler input + Engine Settings entry, off by
default (0 installs nothing).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The sweep's rating feature was wired against REINS' single 'pending'
capture (one slot per key, overwritten by whatever generates next),
which meant only the MOST RECENT result in a whole batch could ever be
rated -- the opposite of what batch training actually does: rate every
result you generated, independently, in any order.
New h3_repr_capture_slot sampler input (blank by default = unchanged
ordinary one-at-a-time flow) saves a run's capture under an explicit
slot instead of the single pending. New h3_repr_steering.save_capture_slot
/ commit_slot / discard_slot mirror save_pending/commit but keyed, so a
whole batch stays independently rateable the same way batch_training.py's
per-item .cond.pt files already do for value-function training.
New server routes commit/discard a slot directly -- no extra generation
needed, unlike the old 'fire another render with rating attached'
workaround that existed only because of the single-slot limitation.
Every gallery card is now rateable (Like/Dislike/Skip), not just the
last one.
The 'applying learned direction(s)' line read the same whether a block
was steering off the bare 2/2 minimum or twenty rated runs -- and those
are not equally trustworthy. Now prints each steered block's own
(liked/disliked) count alongside it, so a style shift from a
just-barely-qualified block is distinguishable from one with real
history behind it.
REINS only ever reads a rating's SIGN (direction() splits liked vs disliked
on weight > 0/< 0, magnitude unused), so the full nuanced rating taxonomy
was overkill here. Like/Dislike send the plain H3-scale digit ratings
('9'/'2') the pipeline already accepts, well clear of the sign-flip
midpoint. Skip costs nothing -- no follow-up generation, just stops the
row asking.
Also fixes the rated state resetting to blank on the next re-render: the
outcome (liked/disliked/skipped) is now tracked per result and the row
shows it instead of the buttons again.
h3_repr_steering_block goes from a fixed combo (18 candidate blocks) to
free text -- a single block, a range, or a comma list, same syntax as
Repeat block(s) (31-40 / 4,5,6). Each named block steers with its OWN
learned direction from its OWN liked/disliked history at that block --
there is no shared 'the direction', just N independent ones applied at
their own positions in the same forward pass. A block short on rated
data at itself just captures, same as any unnamed candidate block.
CANDIDATE_BLOCKS widened from 18 curated blocks to all 50 -- every block
is now both capturable and steerable, not just the ones on the old grid.
Fixed a pre-existing sync gap: h3_block_repeat_last_steps was never added
to settings_card.py's ownership/defaults tables, breaking two golden-file
tests as soon as they ran (test_settings_card_noise.py).
Renamed the sweep panel (H3 Block Repeat Sweep -> H3 Combo Sweep) and the
REINS settings labels (now lead with 'REINS' instead of 'Representation
steering'), per-request.
New combinable line format: reins:strength;block and/or sweep:blocks;
seam|noseam;times[;laststeps], joined with '|' -- e.g.
'reins:0.15;49|sweep:40-41;noseam;1;0' or just 'reins:0.1;49' alone.
Whichever half is omitted is explicitly OFF for that line, not whatever
Engine Settings has -- every line fully states what it tests.
Seed checkbox, default OFF: each line gets its own fresh random seed
regardless of the project's Engine Settings seed. On: one seed picked
once for the whole sweep (the old behaviour).
Rate button on the gallery: only the MOST RECENT card is rateable, since
REINS' pending capture is a single slot overwritten by the next run that
samples -- rating any earlier card would silently commit against the
wrong run's capture. Fires one more generation (same config, rating
attached via node_overrides on the 'studio' node) since a rating isn't a
free action, it rides the next generation the same way it does
everywhere else in this codebase.
Removed Picture detail (h3_video_detail) from Engine Settings -- unused,
backend default (1.0) is already a no-op so hiding it is safe.
The node widget's min went to -50 for the head-window feature, but the
Editor's Engine Settings field metadata is a SEPARATE declaration and
still had min:0 -- so the UI silently refused negative values the node
itself already accepted. Widened to match, updated label/hint for both
directions.
Same widget, sign now carries direction: positive N = final N steps
(tail, refine only -- what every laststeps test so far has used).
Negative N = first |N| steps instead -- apply while structure is still
forming, then get out of the way and let the untouched rest of the
schedule resolve it, instead of the perturbed step being close to the
last word on the output.
Widget min widened 0 -> -50 (backward compatible: 0 still means every
step, existing positive values unchanged). Sweep panel accepts negative
laststeps in the config line (e.g. 40-41;noseam;1;-2).
Overrides VHS_VideoCombine's filename_prefix per run (same node_overrides
mechanism as everything else in the sweep) to funpack_sweep_<config>_<short>,
so the temp file on disk is self-identifying without needing the manual
Save-to-Media-bin step at all.
The checkbox gated the config panel that IS the whole point of a dedicated
settings entry -- pointless now that it lives in its own section instead of
being folded into something else. Config text also moves to a module-level
variable so it survives closing and reopening the panel instead of resetting
on every mount.
Each gallery result gets a 'Save to Media bin' button that names the file
from its own sweep config (sweep_10-11-13-seam-5_<ts>.mp4) via the same
import-clip route Save Video uses -- server-side copy, no re-upload.
Reused media-card/tmp-card/media-thumb classes, which are sized for a
clickable thumbnail strip, not for watching. New dedicated sw-sweep-grid/
sw-sweep-card classes give each result a real 16:9 player instead of a
sliver you had to open in a separate tab to see.
The generic input{width:100%; min-height:...} rule (meant for text fields)
was also catching the raw checkbox, since nothing scoped it down the way
every other checkbox in the app does (.field-toggle, .eng-field). Same fix
as those: width auto, plus a small flex label so it sits inline with its
text instead of centered alone on its own line.
Fourth optional ;laststeps field wires h3_block_repeat_last_steps per config
line, so the sweep can test the tail-window against the full-schedule
default in the same run instead of needing a separate mechanism.
Save video (transport bar, next to Save frame): copies the current clip's
render straight into the Media bin server-side via the existing import-clip
route -- no download-then-reupload round trip through the local machine.
H3 Block Repeat Sweep (Settings > Learning): runs one scene through several
block-repeat configs back to back on one fixed seed, gallery of labelled
results. One config per line: blocks;seam|noseam;times. Each run is a
one-off node_overrides on the built graph (same mechanism as the existing
anchor-guide i2v bypass) -- project Engine Settings are never touched, so
nothing needs restoring after and a crash mid-sweep leaves no residue.
Every step was being repeated, including the earliest near-pure-noise ones
where a block is still deciding layout, not detail -- the likely reason
repeat re-rolls the shot instead of sharpening it. New h3_block_repeat_last_steps
(0 = every step, unchanged default) windows the repeat to the tail via the
exact step index off transformer_options[sample_sigmas/sigmas].
More runs: it reliably brings the preferred style (skin texture,
outlines — replicated across two reference images) and guarantees
nothing about the rest. Motion, anatomy and coherence are still a
re-roll. Calling it the best result was over-read from the first few
runs, and the constraint map and tooltip both said so.
Also retracts the span-loop "dead" verdict from the previous commit:
10-21 as a span is visually better than the other loops while ruining
audio, so the loop is dead at mid-depth and alive shallow. That also
breaks the displacement model on its own — 10-21 is a WIDER span (+12)
than 31-40 (+10) and fares better, so depth modulates tolerance on top
of displacement, as the per-block sweep already found.
One finding hardened rather than softened: the ~10-21 band is where
audio is fragile, and it now replicates across two INDEPENDENT
mechanisms (per-block @2x replaced the soundtrack; a span loop ruins it
too). Different operation, same band, same casualty, video fine or
better in both.
Built on the hypothesis that seam count explained why 31,40 survives two
extra passes while 31-40 collapses. Tested across nine spans: the loop
is worse than per-block repeat everywhere. 31-40 per-block @1x is the
sweep's best result; the identical blocks as a span loop at identical
cost are awful. Fewer seams made it WORSE, so the hypothesis is dead.
What fits instead is input displacement — how far each block's input is
from what it was trained on. Per-block repeat: a block's second call
sees the stream advanced by ONE block, regardless of how many blocks are
selected. Span loop: the first block of the span sees it advanced by
SPAN_LENGTH. 31-35 (+5) tolerable, 31-40 (+10) awful, 0-49 (+50)
destroyed — monotonic in width. Displacement also suppresses motion
specifically: "reference refuses to move" appears in every span-loop
result and in none of the per-block ones.
Also corrects an overclaim in the video-only docs. It restores audio
rows at the exit of the repeated blocks, but the blocks AFTER them still
process audio while attending to twice-processed video, so protection
ends at the span exit and not at the output. That is why the checkbox
did not save audio under a span loop.
The toggle stays — it is a real mechanism, cheap, and 31-35 was usable.
It is documented as losing to per-block rather than removed.
A different operation, not a re-spelling. Per-block repeat hands every
block its OWN doubled output, which is a distribution the NEXT block has
never seen, so a ten-block span creates ten seams. A span loop leaves
every block receiving input from its normal predecessor and creates
exactly ONE, at the wrap from last back to first.
That is the obvious candidate explanation for the sweep's central
puzzle: 31,40 and 35,40 survive two extra passes while 31-40 collapses
at the same count. Same total extra block evaluations, so the same cost
— only the seam count differs.
The span's first block does the work (walking
model.model.diffusion_model.blocks, which the hook cannot reach since
original_block closes over one block only) and the rest of the span
becomes a pass-through, because the model's own loop still calls them.
Refuses a non-contiguous selection with a note rather than silently
looping min..max, which would run the blocks deliberately left out.
Under video-only, text and audio rows get the span ONCE — not zero
times, which would skip those blocks entirely for audio and is a
different, worse intervention than "do not repeat them".
Band 31-40 is the best video result in the sweep and invents unprompted
dialogue on every single run of it — a trade that the bisection proved
cannot be dropped by narrowing, because neither half reproduces either
effect.
H3 packs video, text and audio into ONE sequence, so a plain repeat
doubles the block for the audio rows too. This restores text and audio
rows to their single-pass value after the extra passes, keeping the
repeat for video only.
It does NOT undo audio's influence on the second pass — attention had
already mixed the twice-processed audio rows into the video rows before
they were restored — so it fixes the audio that comes OUT, not the video
path. Whether the video gain survives that is the experiment.
Off by default so the measured sweep stays reproducible, and it falls
back to repeating everything when the mask code meets a shape it has
never seen, rather than silently dropping the feature.
Two corrections from continued testing:
- 31-40's unprompted dialogue has appeared on EVERY run of that band,
including the cross-image replication. It is a trade (better video
texture/outlines/motion, paid for in audio), not a win with an
occasional blemish, and calling it "the best result" without that
clause oversells it.
- Everything past 40 dislikes a second pass, INCLUDING blocks that did
nothing at all on the first. So "no visible effect" is not headroom —
a null result at one pass must not be read as permission to push
harder. That is now a stated rule, since the opposite reading is the
natural one.
The Details panel on h3_block_repeat now carries what was actually
measured instead of a description of the idea, because the field is free
text and the dead zones are not guessable:
0-10 destroys everything — never
10, 15 indistinguishable from plain — pointless
11-20 video good, AUDIO AWFUL; survives 2 passes on video only
20-30 mild and mostly positive — safe band
31-41 best results, ONE PASS ONLY (collapses at 2, seen twice)
42-46 no significant difference
47-49 destroys detail / noise / broken audio — never
Two rules run through it: repeat tolerance falls off with depth, and
audio breaks shallower than video. Also states the headline the sweep
actually produced — this re-rolls the shot, it does not sharpen (detail
stayed within ±5% everywhere useful while the picture moved 17-60%).
Nothing is clamped or refused. Most cells are one observation by eye and
the map is a note to the person typing, not a gate.
dpVerdict gated on structure >= 0.85 and returned "the shot moved"
otherwise — so `detail` and `edge_aligned` were never consulted. The
block-repeat sweep's best row was structure 0.831, which means all 19
comparisons short-circuited and the "sharper" branch was unreachable for
the entire experiment. Reporting that as "not one row read sharper" was
true and misleading: sharpness was never evaluated at all.
The threshold was also wrong on its own terms. Blocks 10/15/44/45 were
judged indistinguishable from plain by eye and measured 0.73-0.83, so
the cliff was rejecting pairs a person calls the same shot.
Now: how far the shot moved is a PREFIX, not a verdict, and the detail
half is always reported alongside it ("shot moved 17% · detail
unchanged"), which is strictly more than either half alone. Only below
0.35 — where the two runs share almost nothing — is a detail comparison
refused outright.
Tested against the sweep's real rows, including a hypothetical genuine
sharpen to prove the branch is now reachable.
