From 0ff90d80f6eba1ccb739bb6075490b06163f339a Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Fri, 31 Jul 2026 12:25:12 -0400 Subject: [PATCH 1/2] feat(imagegen): face swap (identity transfer), a separate operation from Edit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Measured against the instruction-edit models on 2026-07-31: asking a diffusion model to put a SPECIFIC person's face into a photo does not work by any route. qwen-image-edit returns the picture essentially unchanged whether asked by name, by attribute, or by supplying the portrait as a second reference image; flux-kontext replaces the face with a different generic person. Identity transfer is a detect/align/blend pipeline, not a better prompt, so it gets its own interface rather than more Edit options. imagegen.FaceSwapper is optional and type-asserted, like Editor — a provider that cannot do this must not have Edit quietly stand in for it. ListFaces is part of the interface, not a convenience: a caller asked to change "the man on the right" needs a stable way to NAME one face, and pixel boxes let it check its own choice. The llamaswap shim orders faces left to right for exactly that reason (insightface's own order is score-ranked and unstable between near-identical images), and a malformed box is a protocol error rather than a zero-filled struct, because a wrong box aims the swap at the wrong person. The provider is the first here to POST more than one file, so buildMultipart gained buildMultipartFiles and now delegates to it — one writer loop, so the two cannot drift in how they escape names or terminate the body. index and all are mutually exclusive ON THE WIRE: the shim ignores index under all=true, and sending both would imply a precedence the caller cannot see. A JSON body is refused rather than returned as image bytes — the shim answers JSON on a semantic miss (no face in the source), and handing that back as a picture would report success while delivering a file that is not one. --- imagegen/faceswap.go | 74 ++++++++++ provider/llamaswap/faceswap.go | 176 +++++++++++++++++++++++ provider/llamaswap/faceswap_test.go | 214 ++++++++++++++++++++++++++++ provider/llamaswap/upstream.go | 25 +++- 4 files changed, 483 insertions(+), 6 deletions(-) create mode 100644 imagegen/faceswap.go create mode 100644 provider/llamaswap/faceswap.go create mode 100644 provider/llamaswap/faceswap_test.go diff --git a/imagegen/faceswap.go b/imagegen/faceswap.go new file mode 100644 index 0000000..635863f --- /dev/null +++ b/imagegen/faceswap.go @@ -0,0 +1,74 @@ +package imagegen + +import "context" + +// FaceSwapRequest transfers an identity from Source into Target. +// +// This is a DIFFERENT OPERATION from Edit, not a better-tuned one. Measured +// against the instruction-edit models on 2026-07-31, asking a diffusion model +// to put a specific person's face into a photo does not work by any route — +// by name, by description, or by supplying the portrait as a reference image. +// Face swapping is a dedicated detect/align/blend pipeline; a provider that +// cannot do it should not pretend Edit is a substitute. +type FaceSwapRequest struct { + // Target is the photo to edit — the pose, expression, lighting and + // everything outside the face are preserved from it. + Target Image + + // Source is a photo of the face to put in. Only the identity travels; + // the source's own pose and expression do not. + Source Image + + // Index selects WHICH face in Target, in the provider's documented + // ordering (llamaswap: left to right by box centre, as reported by + // ListFaces). nil = the largest face, which is right for a portrait and + // wrong for a group — enumerate first when it matters. + Index *int + + // All swaps every detected face and ignores Index. + All bool +} + +// FaceSwapOption mutates a FaceSwapRequest before it is sent. +type FaceSwapOption func(*FaceSwapRequest) + +// WithFaceIndex selects which face in the target to replace. +func WithFaceIndex(i int) FaceSwapOption { return func(r *FaceSwapRequest) { r.Index = &i } } + +// WithAllFaces swaps every detected face. +func WithAllFaces() FaceSwapOption { return func(r *FaceSwapRequest) { r.All = true } } + +// Apply returns a copy of the request with all options applied. +func (r FaceSwapRequest) Apply(opts ...FaceSwapOption) FaceSwapRequest { + for _, opt := range opts { + opt(&r) + } + return r +} + +// DetectedFace is one face located in an image, in PIXEL coordinates. +type DetectedFace struct { + // Index is the face's position in the provider's stable ordering, and + // the value FaceSwapRequest.Index expects. + Index int + // Box is [x0, y0, x1, y1]. + Box [4]int + // Score is the detector's confidence, 0-1. + Score float64 + // Width and Height are the box dimensions, carried so a caller can pick + // "the big face" without recomputing them. + Width, Height int +} + +// FaceSwapper is the optional face-transfer surface. Separate interface so +// existing providers keep compiling; callers type-assert. +type FaceSwapper interface { + // ListFaces enumerates the faces in an image, in the SAME ordering + // FaceSwapRequest.Index uses. Exposed because a caller asked to change + // "the man on the right" needs a way to name one face and to check its + // own choice against pixel boxes. + ListFaces(ctx context.Context, img Image) ([]DetectedFace, error) + + // FaceSwap transfers Source's identity into Target. + FaceSwap(ctx context.Context, req FaceSwapRequest, opts ...FaceSwapOption) (*Result, error) +} diff --git a/provider/llamaswap/faceswap.go b/provider/llamaswap/faceswap.go new file mode 100644 index 0000000..787d609 --- /dev/null +++ b/provider/llamaswap/faceswap.go @@ -0,0 +1,176 @@ +// faceswap.go implements imagegen.FaceSwapper against the InsightFace shim +// (buffalo_l + inswapper_128) reached through llama-swap's /upstream +// passthrough (ADR-0024): +// +// POST /upstream//v1/faces multipart file -> JSON +// POST /upstream//v1/faceswap multipart target, source -> PNG +package llamaswap + +import ( + "context" + "encoding/json" + "fmt" + "mime" + "net/http" + "strconv" + "strings" + + "gitea.stevedudenhoeffer.com/steve/majordomo/imagegen" + "gitea.stevedudenhoeffer.com/steve/majordomo/llm" +) + +// maxFaceSwapResponseBytes bounds the returned PNG. Generous: the shim echoes +// the target's dimensions, and a 4K photo round-trips as a large lossless PNG. +const maxFaceSwapResponseBytes = 64 << 20 + +// FaceSwapModel implements the face-transfer surface. The id selects which +// upstream llama-swap loads. +func (p *Provider) FaceSwapModel(id string) (imagegen.FaceSwapper, error) { + if err := p.requireBaseURL(); err != nil { + return nil, err + } + return &faceSwapModel{p: p, id: id}, nil +} + +type faceSwapModel struct { + p *Provider + id string +} + +// facesResponse mirrors the shim's /v1/faces body. +type facesResponse struct { + Count int `json:"count"` + Faces []struct { + Index int `json:"index"` + Box []int `json:"box"` + Score float64 `json:"score"` + Width int `json:"width"` + Height int `json:"height"` + } `json:"faces"` +} + +// ListFaces implements imagegen.FaceSwapper. +func (m *faceSwapModel) ListFaces(ctx context.Context, img imagegen.Image) ([]imagegen.DetectedFace, error) { + if len(img.Data) == 0 { + return nil, fmt.Errorf("%w: face detection requires image bytes", llm.ErrUnsupported) + } + path, err := upstreamPath(m.id, "/v1/faces") + if err != nil { + return nil, err + } + body, contentType, err := buildMultipart("build faces form", + filePart{field: "file", filename: imageFilename(img.MIME, "image"), data: img.Data}, nil) + if err != nil { + return nil, err + } + raw, _, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, contentType, body, maxFaceSwapResponseBytes) + if err != nil { + return nil, err + } + var parsed facesResponse + if err := json.Unmarshal(raw, &parsed); err != nil { + return nil, &llm.APIError{Provider: m.p.name, Model: m.id, + Message: fmt.Sprintf("faces response is not JSON: %s", truncateForError(raw))} + } + out := make([]imagegen.DetectedFace, 0, len(parsed.Faces)) + for _, f := range parsed.Faces { + df := imagegen.DetectedFace{Index: f.Index, Score: f.Score, Width: f.Width, Height: f.Height} + // A short box would silently index out of range below; treat a + // malformed entry as a protocol error rather than zero-filling it, + // because a wrong box sends the caller at the wrong face. + if len(f.Box) != 4 { + return nil, &llm.APIError{Provider: m.p.name, Model: m.id, + Message: fmt.Sprintf("face %d has a %d-element box, want 4", f.Index, len(f.Box))} + } + copy(df.Box[:], f.Box) + out = append(out, df) + } + return out, nil +} + +// FaceSwap implements imagegen.FaceSwapper. The endpoint always answers PNG. +func (m *faceSwapModel) FaceSwap(ctx context.Context, req imagegen.FaceSwapRequest, opts ...imagegen.FaceSwapOption) (*imagegen.Result, error) { + req = req.Apply(opts...) + if len(req.Target.Data) == 0 { + return nil, fmt.Errorf("%w: face swap requires a target image", llm.ErrUnsupported) + } + if len(req.Source.Data) == 0 { + return nil, fmt.Errorf("%w: face swap requires a source image", llm.ErrUnsupported) + } + if req.Index != nil && *req.Index < 0 { + return nil, fmt.Errorf("%w: face index must be >= 0, got %d", llm.ErrUnsupported, *req.Index) + } + path, err := upstreamPath(m.id, "/v1/faceswap") + if err != nil { + return nil, err + } + + var fields []formField + if req.All { + fields = append(fields, formField{key: "all", value: "true", required: true}) + } else if req.Index != nil { + // Only sent when NOT swapping all: the shim ignores index under + // all=true, and sending both would imply a precedence the caller + // cannot see. + fields = append(fields, formField{key: "index", value: strconv.Itoa(*req.Index), required: true}) + } + + body, contentType, err := buildMultipartFiles("build faceswap form", + []filePart{ + {field: "target", filename: imageFilename(req.Target.MIME, "target"), data: req.Target.Data}, + {field: "source", filename: imageFilename(req.Source.MIME, "source"), data: req.Source.Data}, + }, fields) + if err != nil { + return nil, err + } + raw, respType, err := m.p.doRaw(ctx, http.MethodPost, path, m.id, contentType, body, maxFaceSwapResponseBytes) + if err != nil { + return nil, err + } + if len(raw) == 0 { + return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "face swap response contained no image"} + } + mime := sniffImageMIME(raw) + if respType != "" && !isImageContentType(respType) { + // A JSON error body sniffs as text, not image — say what came back + // rather than handing the caller bytes that are not a picture. + return nil, &llm.APIError{Provider: m.p.name, Model: m.id, + Message: fmt.Sprintf("face swap response is not an image (Content-Type %q): %s", respType, truncateForError(raw))} + } + return &imagegen.Result{Images: []llm.ImagePart{{MIME: mime, Data: raw}}}, nil +} + +// imageFilename picks a multipart filename for an image part. The shim reads +// bytes, not names, but a plausible extension keeps server-side sniffing and +// request logs honest. base distinguishes the parts of a multi-file form +// ("target"/"source") so a log line says which one was malformed. +func imageFilename(mimeType, base string) string { + if base == "" { + base = "image" + } + mt := strings.ToLower(strings.TrimSpace(mimeType)) + if parsed, _, err := mime.ParseMediaType(mt); err == nil { + mt = parsed + } + switch mt { + case "image/jpeg", "image/jpg": + return base + ".jpg" + case "image/webp": + return base + ".webp" + case "image/gif": + return base + ".gif" + case "image/bmp": + return base + ".bmp" + default: + // PNG is the safe default: every caller in this repo either sends PNG + // or sends something the decoder identifies by magic bytes anyway. + return base + ".png" + } +} + +// isImageContentType reports whether a response Content-Type is an image. +// Used to tell a returned picture apart from a JSON error body, which would +// otherwise be handed back as "image" bytes. +func isImageContentType(contentType string) bool { + return mimeFromContentType(contentType, "image/") != "" +} diff --git a/provider/llamaswap/faceswap_test.go b/provider/llamaswap/faceswap_test.go new file mode 100644 index 0000000..9f3d227 --- /dev/null +++ b/provider/llamaswap/faceswap_test.go @@ -0,0 +1,214 @@ +package llamaswap + +import ( + "context" + "encoding/base64" + "io" + "mime" + "mime/multipart" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "gitea.stevedudenhoeffer.com/steve/majordomo/imagegen" + "gitea.stevedudenhoeffer.com/steve/majordomo/llm" +) + +func swapImg(t *testing.T) imagegen.Image { + t.Helper() + raw, err := base64.StdEncoding.DecodeString(onePixelPNG) + if err != nil { + t.Fatalf("decode fixture: %v", err) + } + return imagegen.Image{MIME: "image/png", Data: raw} +} + +// parseParts pulls the multipart form a handler received. +func parseParts(t *testing.T, r *http.Request) (files map[string][]byte, fields map[string]string) { + t.Helper() + _, params, err := mime.ParseMediaType(r.Header.Get("Content-Type")) + if err != nil { + t.Fatalf("content type: %v", err) + } + mr := multipart.NewReader(r.Body, params["boundary"]) + files, fields = map[string][]byte{}, map[string]string{} + for { + p, err := mr.NextPart() + if err == io.EOF { + break + } + if err != nil { + t.Fatalf("next part: %v", err) + } + body, _ := io.ReadAll(p) + if p.FileName() != "" { + files[p.FormName()] = body + } else { + fields[p.FormName()] = string(body) + } + } + return files, fields +} + +// TestFaceSwapSendsBothFiles pins the two-file wire shape. A face swap is the +// first endpoint in this provider taking more than one file, so buildMultipart +// grew a sibling; getting the field NAMES wrong would reach the shim as a +// missing-argument 422 rather than anything self-explanatory. +func TestFaceSwapSendsBothFiles(t *testing.T) { + var gotPath string + var files map[string][]byte + var fields map[string]string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath = r.URL.Path + files, fields = parseParts(t, r) + w.Header().Set("Content-Type", "image/png") + raw, _ := base64.StdEncoding.DecodeString(onePixelPNG) + _, _ = w.Write(raw) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + m, err := p.FaceSwapModel("faceswap") + if err != nil { + t.Fatalf("model: %v", err) + } + img := swapImg(t) + res, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img}, + imagegen.WithFaceIndex(2)) + if err != nil { + t.Fatalf("faceswap: %v", err) + } + if len(res.Images) != 1 { + t.Fatalf("images = %d, want 1", len(res.Images)) + } + if !strings.HasSuffix(gotPath, "/upstream/faceswap/v1/faceswap") { + t.Errorf("path = %q", gotPath) + } + for _, want := range []string{"target", "source"} { + if len(files[want]) == 0 { + t.Errorf("no %q file part — the shim requires both", want) + } + } + if fields["index"] != "2" { + t.Errorf("index = %q, want 2", fields["index"]) + } + if _, ok := fields["all"]; ok { + t.Error("all sent alongside index — the shim ignores index under all=true, so sending both implies a precedence the caller cannot see") + } +} + +// TestFaceSwapAllSuppressesIndex: same reasoning from the other side. +func TestFaceSwapAllSuppressesIndex(t *testing.T) { + var fields map[string]string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, fields = parseParts(t, r) + w.Header().Set("Content-Type", "image/png") + raw, _ := base64.StdEncoding.DecodeString(onePixelPNG) + _, _ = w.Write(raw) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + m, _ := p.FaceSwapModel("faceswap") + img := swapImg(t) + if _, err := m.FaceSwap(context.Background(), + imagegen.FaceSwapRequest{Target: img, Source: img, Index: new(int), All: true}); err != nil { + t.Fatalf("faceswap: %v", err) + } + if fields["all"] != "true" { + t.Errorf("all = %q, want true", fields["all"]) + } + if _, ok := fields["index"]; ok { + t.Error("index sent under all=true") + } +} + +// TestFaceSwapRejectsMissingImages: both are required, and the error should +// name which one rather than surfacing a shim 422. +func TestFaceSwapRejectsMissingImages(t *testing.T) { + p := New(WithBaseURL("http://example.invalid")) + m, _ := p.FaceSwapModel("faceswap") + img := swapImg(t) + for _, tc := range []struct { + name string + req imagegen.FaceSwapRequest + want string + }{ + {"no target", imagegen.FaceSwapRequest{Source: img}, "target"}, + {"no source", imagegen.FaceSwapRequest{Target: img}, "source"}, + } { + _, err := m.FaceSwap(context.Background(), tc.req) + if err == nil || !strings.Contains(err.Error(), tc.want) { + t.Errorf("%s: err = %v, want one naming %q", tc.name, err, tc.want) + } + } +} + +// TestFaceSwapRejectsNonImageResponse: the shim answers JSON on a semantic +// miss (no face found). Returning those bytes as an "image" would hand the +// caller a file that is not a picture and call it success. +func TestFaceSwapRejectsNonImageResponse(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"detail":{"error":"no_face_in_source"}}`)) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + m, _ := p.FaceSwapModel("faceswap") + img := swapImg(t) + _, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img}) + if err == nil { + t.Fatal("a JSON body was accepted as an image") + } + var apiErr *llm.APIError + if !strings.Contains(err.Error(), "no_face_in_source") { + t.Errorf("err = %v, want it to relay the shim's reason", err) + } + _ = apiErr +} + +// TestListFacesParsesOrdering: the shim's left-to-right index is the contract +// callers select against, so it must survive decoding intact. +func TestListFacesParsesOrdering(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if !strings.HasSuffix(r.URL.Path, "/upstream/faceswap/v1/faces") { + t.Errorf("path = %q", r.URL.Path) + } + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"count":2,"faces":[ + {"index":0,"box":[10,20,30,40],"score":0.9,"width":20,"height":20}, + {"index":1,"box":[50,20,90,60],"score":0.8,"width":40,"height":40}]}`)) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + m, _ := p.FaceSwapModel("faceswap") + faces, err := m.ListFaces(context.Background(), swapImg(t)) + if err != nil { + t.Fatalf("list: %v", err) + } + if len(faces) != 2 || faces[0].Index != 0 || faces[1].Index != 1 { + t.Fatalf("faces = %+v", faces) + } + if faces[1].Box != [4]int{50, 20, 90, 60} { + t.Errorf("box = %v", faces[1].Box) + } +} + +// TestListFacesRejectsShortBox: a malformed box would send a caller at the +// wrong face, which is worse than an error. +func TestListFacesRejectsShortBox(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/json") + _, _ = w.Write([]byte(`{"count":1,"faces":[{"index":0,"box":[1,2],"score":0.9}]}`)) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + m, _ := p.FaceSwapModel("faceswap") + if _, err := m.ListFaces(context.Background(), swapImg(t)); err == nil { + t.Fatal("a 2-element box was accepted") + } +} diff --git a/provider/llamaswap/upstream.go b/provider/llamaswap/upstream.go index 48998d2..e9850fa 100644 --- a/provider/llamaswap/upstream.go +++ b/provider/llamaswap/upstream.go @@ -51,14 +51,27 @@ type filePart struct { // writeFormFields). wrap labels errors. Returns the body and its content // type. func buildMultipart(wrap string, file filePart, fields []formField) (*bytes.Buffer, string, error) { + return buildMultipartFiles(wrap, []filePart{file}, fields) +} + +// buildMultipartFiles is buildMultipart for endpoints taking SEVERAL files +// (face swap sends a target and a source). Files are written in the given +// order, then the fields. One writer loop serves both so the two cannot drift +// in how they escape names or terminate the body. +func buildMultipartFiles(wrap string, files []filePart, fields []formField) (*bytes.Buffer, string, error) { + if len(files) == 0 { + return nil, "", fmt.Errorf("llama-swap: %s: no file parts", wrap) + } var buf bytes.Buffer w := multipart.NewWriter(&buf) - fw, err := w.CreateFormFile(file.field, file.filename) - if err != nil { - return nil, "", fmt.Errorf("llama-swap: %s: %w", wrap, err) - } - if _, err := fw.Write(file.data); err != nil { - return nil, "", fmt.Errorf("llama-swap: %s: %w", wrap, err) + for _, file := range files { + fw, err := w.CreateFormFile(file.field, file.filename) + if err != nil { + return nil, "", fmt.Errorf("llama-swap: %s: %w", wrap, err) + } + if _, err := fw.Write(file.data); err != nil { + return nil, "", fmt.Errorf("llama-swap: %s: %w", wrap, err) + } } if err := writeFormFields(w, wrap, fields); err != nil { return nil, "", err From 372bf826aa23c52a0db0da0a9d7fa32ff6ae2ced Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Fri, 31 Jul 2026 12:34:58 -0400 Subject: [PATCH 2/2] fix(llamaswap): a headerless non-image response was returned as a PNG MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gadfly on #23, blocking, 2/2 agreement — and it is the exact defect this whole line of work has been about: a call that succeeds while handing back the wrong bytes. sniffImageMIME falls back to image/png when detection is inconclusive, and the guard only consulted Content-Type. A response with NO Content-Type therefore skipped the check entirely and was labelled a PNG. The shim answers JSON on a semantic miss (no face found in the source or target), which is precisely the body that would have sailed through as a successful image. The check now validates the BYTES — http.DetectContentType must say image/ — and the reported MIME prefers the server's own label only when that label is itself an image type. Break-checked by restoring the header-only condition, which fails the new test. Also from that review: - index is documented as ignored under all=true, so a negative one is no longer rejected there; it is still rejected when it would actually be sent, and both halves are tested. - initImageFilename (video.go) was imageFilename with the base fixed to "frame" and now delegates to it — two copies of one extension table is how they drift. - DetectedFace carried Width/Height alongside Box, two sources of truth for one fact that can disagree after any transform. Now a Size() method derived from Box. - a dead `apiErr` in the test (declared, then `_ = apiErr`) was an abandoned errors.As check; it is wired up and now asserts callers can classify the error. - swapImg duplicated editInit verbatim; removed. Not taken: adding a FaceSwapProvider/ModelOption surface to match the other optional imagegen capabilities (single-model finding). There are no options to carry yet, and inventing an empty option type to look symmetrical would be API surface with nothing behind it. Worth revisiting when a real knob exists. --- imagegen/faceswap.go | 10 ++-- provider/llamaswap/faceswap.go | 40 +++++++++------ provider/llamaswap/faceswap_test.go | 79 +++++++++++++++++++++++------ provider/llamaswap/video.go | 14 +---- 4 files changed, 96 insertions(+), 47 deletions(-) diff --git a/imagegen/faceswap.go b/imagegen/faceswap.go index 635863f..d7b09e8 100644 --- a/imagegen/faceswap.go +++ b/imagegen/faceswap.go @@ -55,9 +55,13 @@ type DetectedFace struct { Box [4]int // Score is the detector's confidence, 0-1. Score float64 - // Width and Height are the box dimensions, carried so a caller can pick - // "the big face" without recomputing them. - Width, Height int +} + +// Size returns the box dimensions. Derived rather than stored: carrying +// width/height alongside Box is two sources of truth for one fact, and the +// pair can disagree after any transform. +func (f DetectedFace) Size() (w, h int) { + return f.Box[2] - f.Box[0], f.Box[3] - f.Box[1] } // FaceSwapper is the optional face-transfer surface. Separate interface so diff --git a/provider/llamaswap/faceswap.go b/provider/llamaswap/faceswap.go index 787d609..a69fc9a 100644 --- a/provider/llamaswap/faceswap.go +++ b/provider/llamaswap/faceswap.go @@ -74,7 +74,7 @@ func (m *faceSwapModel) ListFaces(ctx context.Context, img imagegen.Image) ([]im } out := make([]imagegen.DetectedFace, 0, len(parsed.Faces)) for _, f := range parsed.Faces { - df := imagegen.DetectedFace{Index: f.Index, Score: f.Score, Width: f.Width, Height: f.Height} + df := imagegen.DetectedFace{Index: f.Index, Score: f.Score} // A short box would silently index out of range below; treat a // malformed entry as a protocol error rather than zero-filling it, // because a wrong box sends the caller at the wrong face. @@ -97,7 +97,10 @@ func (m *faceSwapModel) FaceSwap(ctx context.Context, req imagegen.FaceSwapReque if len(req.Source.Data) == 0 { return nil, fmt.Errorf("%w: face swap requires a source image", llm.ErrUnsupported) } - if req.Index != nil && *req.Index < 0 { + // Only when it will actually be sent: under All the index is documented + // as ignored, so rejecting a negative one there would fail a request that + // is perfectly well formed. + if !req.All && req.Index != nil && *req.Index < 0 { return nil, fmt.Errorf("%w: face index must be >= 0, got %d", llm.ErrUnsupported, *req.Index) } path, err := upstreamPath(m.id, "/v1/faceswap") @@ -130,20 +133,34 @@ func (m *faceSwapModel) FaceSwap(ctx context.Context, req imagegen.FaceSwapReque if len(raw) == 0 { return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "face swap response contained no image"} } - mime := sniffImageMIME(raw) - if respType != "" && !isImageContentType(respType) { - // A JSON error body sniffs as text, not image — say what came back - // rather than handing the caller bytes that are not a picture. + // Validate the BYTES, not the header. sniffImageMIME falls back to + // image/png when detection is inconclusive, so trusting it here would + // label a JSON error body as a PNG and return it as a successful image — + // and a header check alone misses the case where the response carries no + // Content-Type at all. The shim answers JSON on a semantic miss (no face + // found), which is exactly the body that would sail through. + detected := http.DetectContentType(raw) + if !strings.HasPrefix(detected, "image/") { return nil, &llm.APIError{Provider: m.p.name, Model: m.id, - Message: fmt.Sprintf("face swap response is not an image (Content-Type %q): %s", respType, truncateForError(raw))} + Message: fmt.Sprintf("face swap response is not an image (sniffed %q, Content-Type %q): %s", + detected, respType, truncateForError(raw))} } - return &imagegen.Result{Images: []llm.ImagePart{{MIME: mime, Data: raw}}}, nil + // Prefer the server's own label when it is an image type (it knows + // subtypes the sniffer does not), else what the bytes actually are. + mimeType := detected + if hdr := mimeFromContentType(respType, "image/"); hdr != "" { + mimeType = hdr + } + return &imagegen.Result{Images: []llm.ImagePart{{MIME: mimeType, Data: raw}}}, nil } // imageFilename picks a multipart filename for an image part. The shim reads // bytes, not names, but a plausible extension keeps server-side sniffing and // request logs honest. base distinguishes the parts of a multi-file form // ("target"/"source") so a log line says which one was malformed. +// +// initImageFilename (video.go) is this function with base fixed to "frame" +// and delegates here — two copies of one extension table is how they drift. func imageFilename(mimeType, base string) string { if base == "" { base = "image" @@ -167,10 +184,3 @@ func imageFilename(mimeType, base string) string { return base + ".png" } } - -// isImageContentType reports whether a response Content-Type is an image. -// Used to tell a returned picture apart from a JSON error body, which would -// otherwise be handed back as "image" bytes. -func isImageContentType(contentType string) bool { - return mimeFromContentType(contentType, "image/") != "" -} diff --git a/provider/llamaswap/faceswap_test.go b/provider/llamaswap/faceswap_test.go index 9f3d227..ca41de9 100644 --- a/provider/llamaswap/faceswap_test.go +++ b/provider/llamaswap/faceswap_test.go @@ -3,6 +3,7 @@ package llamaswap import ( "context" "encoding/base64" + "errors" "io" "mime" "mime/multipart" @@ -15,15 +16,6 @@ import ( "gitea.stevedudenhoeffer.com/steve/majordomo/llm" ) -func swapImg(t *testing.T) imagegen.Image { - t.Helper() - raw, err := base64.StdEncoding.DecodeString(onePixelPNG) - if err != nil { - t.Fatalf("decode fixture: %v", err) - } - return imagegen.Image{MIME: "image/png", Data: raw} -} - // parseParts pulls the multipart form a handler received. func parseParts(t *testing.T, r *http.Request) (files map[string][]byte, fields map[string]string) { t.Helper() @@ -73,7 +65,7 @@ func TestFaceSwapSendsBothFiles(t *testing.T) { if err != nil { t.Fatalf("model: %v", err) } - img := swapImg(t) + img := editInit(t) res, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img}, imagegen.WithFaceIndex(2)) if err != nil { @@ -111,7 +103,7 @@ func TestFaceSwapAllSuppressesIndex(t *testing.T) { p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) m, _ := p.FaceSwapModel("faceswap") - img := swapImg(t) + img := editInit(t) if _, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img, Index: new(int), All: true}); err != nil { t.Fatalf("faceswap: %v", err) @@ -129,7 +121,7 @@ func TestFaceSwapAllSuppressesIndex(t *testing.T) { func TestFaceSwapRejectsMissingImages(t *testing.T) { p := New(WithBaseURL("http://example.invalid")) m, _ := p.FaceSwapModel("faceswap") - img := swapImg(t) + img := editInit(t) for _, tc := range []struct { name string req imagegen.FaceSwapRequest @@ -157,16 +149,18 @@ func TestFaceSwapRejectsNonImageResponse(t *testing.T) { p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) m, _ := p.FaceSwapModel("faceswap") - img := swapImg(t) + img := editInit(t) _, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img}) if err == nil { t.Fatal("a JSON body was accepted as an image") } var apiErr *llm.APIError + if !errors.As(err, &apiErr) { + t.Errorf("err = %T, want *llm.APIError so callers can classify it", err) + } if !strings.Contains(err.Error(), "no_face_in_source") { t.Errorf("err = %v, want it to relay the shim's reason", err) } - _ = apiErr } // TestListFacesParsesOrdering: the shim's left-to-right index is the contract @@ -185,7 +179,7 @@ func TestListFacesParsesOrdering(t *testing.T) { p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) m, _ := p.FaceSwapModel("faceswap") - faces, err := m.ListFaces(context.Background(), swapImg(t)) + faces, err := m.ListFaces(context.Background(), editInit(t)) if err != nil { t.Fatalf("list: %v", err) } @@ -208,7 +202,60 @@ func TestListFacesRejectsShortBox(t *testing.T) { p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) m, _ := p.FaceSwapModel("faceswap") - if _, err := m.ListFaces(context.Background(), swapImg(t)); err == nil { + if _, err := m.ListFaces(context.Background(), editInit(t)); err == nil { t.Fatal("a 2-element box was accepted") } } + +// TestFaceSwapRejectsHeaderlessNonImage is the regression for gadfly's +// blocking finding on #23, agreed by both models. sniffImageMIME falls back +// to image/png when detection is inconclusive, and the original guard only +// looked at Content-Type — so a JSON error body sent WITHOUT a Content-Type +// header was labelled a PNG and returned as a successful image. The shim +// answers JSON on a semantic miss, which is precisely the body that would +// have sailed through. +func TestFaceSwapRejectsHeaderlessNonImage(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + // Explicitly no Content-Type — Go only sets one if we write before + // deleting it, so clear it to model a bare upstream response. + w.Header()["Content-Type"] = nil + _, _ = w.Write([]byte(`{"detail":{"error":"no_face_in_target"}}`)) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + m, _ := p.FaceSwapModel("faceswap") + img := editInit(t) + _, err := m.FaceSwap(context.Background(), imagegen.FaceSwapRequest{Target: img, Source: img}) + if err == nil { + t.Fatal("a headerless JSON body was accepted and would have been returned as image/png") + } + if !strings.Contains(err.Error(), "no_face_in_target") { + t.Errorf("err = %v, want it to relay what actually came back", err) + } +} + +// TestFaceSwapAllowsNegativeIndexUnderAll: index is documented as ignored +// when all=true, so validating it there would reject a well-formed request. +func TestFaceSwapAllowsNegativeIndexUnderAll(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "image/png") + raw, _ := base64.StdEncoding.DecodeString(onePixelPNG) + _, _ = w.Write(raw) + })) + defer srv.Close() + + p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client())) + m, _ := p.FaceSwapModel("faceswap") + img := editInit(t) + neg := -1 + if _, err := m.FaceSwap(context.Background(), + imagegen.FaceSwapRequest{Target: img, Source: img, Index: &neg, All: true}); err != nil { + t.Fatalf("negative index rejected under all=true, where it is ignored: %v", err) + } + // ...but still rejected when it WOULD be sent. + if _, err := m.FaceSwap(context.Background(), + imagegen.FaceSwapRequest{Target: img, Source: img, Index: &neg}); err == nil { + t.Error("negative index accepted when it would actually be sent") + } +} diff --git a/provider/llamaswap/video.go b/provider/llamaswap/video.go index 1586a6e..23f21ef 100644 --- a/provider/llamaswap/video.go +++ b/provider/llamaswap/video.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "fmt" - "mime" "mime/multipart" "net/http" "strconv" @@ -139,18 +138,7 @@ func singleVideoResult(provider, model, verb string, raw []byte, contentType str // frame from its MIME subtype. The name is provider-chosen (never // caller-supplied), so no sanitization is needed. func initImageFilename(mimeType string) string { - mt := strings.ToLower(strings.TrimSpace(mimeType)) - if parsed, _, err := mime.ParseMediaType(mt); err == nil { - mt = parsed - } - switch mt { - case "image/jpeg", "image/jpg": - return "frame.jpg" - case "image/webp": - return "frame.webp" - default: // unknown MIME — PNG is the safe hint - return "frame.png" - } + return imageFilename(mimeType, "frame") } // formatInt renders an optional int pointer for a form field; nil = "" (omit).