fix(videogen): distinct FILENAMES for the two keyframes, not just distinct field names
CI / Tidy (pull_request) Successful in 9m25s
CI / Build & Test (pull_request) Successful in 10m28s

Caught while writing the receiving end. Distinct multipart field names are not
sufficient: backends stage an uploaded frame under a name derived from the
FILENAME, and our own ComfyUI shim posts to /upload/image with overwrite=true.
Both parts were sending initImageFilename(mime) — literally "frame.png" for
each — so the second upload would have clobbered the first and BOTH keyframe
inputs would have resolved to the same stored image.

The failure mode is the worst kind: a clip pinned at both ends to the same
frame renders cleanly, returns 200, and looks like the feature not working
rather than like a bug. Nothing upstream or downstream would report a fault.

writeImagePart now takes the filename stem (frame / frame_last), and the test
asserts the two arrive under different filenames.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01PLjgrxvHjm1sJgUu9zBPH9
This commit is contained in:
2026-08-08 02:50:43 -04:00
co-authored by Claude Opus 5
parent 44fcfbb273
commit dbc96898ab
2 changed files with 33 additions and 9 deletions
+16 -7
View File
@@ -86,7 +86,7 @@ func (m *videoModel) Generate(ctx context.Context, req videogen.Request, opts ..
return nil, err return nil, err
} }
if req.InitImage != nil { if req.InitImage != nil {
if err := writeImagePart(w, "input_reference", req.InitImage); err != nil { if err := writeImagePart(w, "input_reference", "frame", req.InitImage); err != nil {
return nil, err return nil, err
} }
} }
@@ -97,7 +97,7 @@ func (m *videoModel) Generate(ctx context.Context, req videogen.Request, opts ..
// can see they have broken. A backend that does not know the name ignores // can see they have broken. A backend that does not know the name ignores
// the part, which is the same degradation as any other unknown field. // the part, which is the same degradation as any other unknown field.
if req.LastImage != nil { if req.LastImage != nil {
if err := writeImagePart(w, "input_reference_last", req.LastImage); err != nil { if err := writeImagePart(w, "input_reference_last", "frame_last", req.LastImage); err != nil {
return nil, err return nil, err
} }
} }
@@ -151,11 +151,20 @@ func initImageFilename(mimeType string) string {
return imageFilename(mimeType, "frame") return imageFilename(mimeType, "frame")
} }
// writeImagePart attaches one conditioning frame under the given field name. // writeImagePart attaches one conditioning frame under the given field name,
// Shared by the first- and last-frame parts so the two cannot drift in how // with a filename derived from nameStem. Shared by the first- and last-frame
// they encode, which is the usual way a second copy of a block goes wrong. // parts so the two cannot drift in how they encode, which is the usual way a
func writeImagePart(w *multipart.Writer, field string, img *videogen.Image) error { // second copy of a block goes wrong.
fw, err := w.CreateFormFile(field, initImageFilename(img.MIME)) //
// The two frames MUST carry DISTINCT filenames, not merely distinct field
// names. Backends commonly stage an uploaded frame under a name derived from
// the filename — our own ComfyUI shim posts to /upload/image with
// overwrite=true — so two parts sharing "frame.png" would have the second
// clobber the first, and BOTH keyframe inputs would then resolve to the same
// stored image. The clip would render clean, pinned at both ends to the same
// frame, with nothing anywhere reporting a problem.
func writeImagePart(w *multipart.Writer, field, nameStem string, img *videogen.Image) error {
fw, err := w.CreateFormFile(field, imageFilename(img.MIME, nameStem))
if err != nil { if err != nil {
return fmt.Errorf("llama-swap: build video form: %w", err) return fmt.Errorf("llama-swap: build video form: %w", err)
} }
+17 -2
View File
@@ -233,19 +233,22 @@ func TestVideoGenerateNonVideoBodyErrors(t *testing.T) {
// breaking. Asserting the names is what pins it. // breaking. Asserting the names is what pins it.
func TestVideoGenerateSendsBothKeyframes(t *testing.T) { func TestVideoGenerateSendsBothKeyframes(t *testing.T) {
var gotFirst, gotLast []byte var gotFirst, gotLast []byte
var firstName, lastName string
var sawLastPart bool var sawLastPart bool
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if err := r.ParseMultipartForm(32 << 20); err != nil { if err := r.ParseMultipartForm(32 << 20); err != nil {
t.Errorf("parse form: %v", err) t.Errorf("parse form: %v", err)
return return
} }
if f, _, err := r.FormFile("input_reference"); err == nil { if f, hdr, err := r.FormFile("input_reference"); err == nil {
gotFirst, _ = io.ReadAll(f) gotFirst, _ = io.ReadAll(f)
firstName = hdr.Filename
f.Close() f.Close()
} }
if f, _, err := r.FormFile("input_reference_last"); err == nil { if f, hdr, err := r.FormFile("input_reference_last"); err == nil {
sawLastPart = true sawLastPart = true
gotLast, _ = io.ReadAll(f) gotLast, _ = io.ReadAll(f)
lastName = hdr.Filename
f.Close() f.Close()
} }
w.Header().Set("Content-Type", "video/mp4") w.Header().Set("Content-Type", "video/mp4")
@@ -283,6 +286,18 @@ func TestVideoGenerateSendsBothKeyframes(t *testing.T) {
if string(gotFirst) == string(gotLast) { if string(gotFirst) == string(gotLast) {
t.Error("both parts carry identical bytes — the frames are being aliased") t.Error("both parts carry identical bytes — the frames are being aliased")
} }
// DISTINCT FILENAMES, not just distinct field names. Backends stage an
// uploaded frame under a name derived from the filename (our ComfyUI shim
// posts to /upload/image with overwrite=true), so two parts sharing
// "frame.png" would have the second clobber the first and BOTH keyframes
// would resolve to the same stored image — a clip pinned at both ends to
// the same frame, rendering cleanly with nothing reporting a fault.
if firstName == "" || lastName == "" {
t.Fatalf("filenames = %q / %q, want both set", firstName, lastName)
}
if firstName == lastName {
t.Errorf("both parts use filename %q — the second upload would clobber the first", firstName)
}
} }
// LastImage alone (no InitImage) is a legitimate request: pin the destination // LastImage alone (no InitImage) is a legitimate request: pin the destination