feat(imagegen): face swap (identity transfer), a separate operation from Edit #23

Merged
steve merged 2 commits from feat/imagegen-faceswap into main 2026-07-31 16:55:09 +00:00
5 changed files with 545 additions and 19 deletions
+78
View File
@@ -0,0 +1,78 @@
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
}
Outdated
Review

DetectedFace.Width/Height are redundant with Box, creating two sources of truth

maintainability · flagged by 1 model

  • Finding 1 confirmed: initImageFilename at video.go:141-154 is the same MIME→filename switch in the same package (jpeg/webp/png), and imageFilename is a strict generalization. Real duplication. - Finding 2 confirmed: faceswap_test.go:165-169 declares var apiErr *llm.APIError, never assigns it, and discards it with _ = apiErr. Dead/abandoned code. - Finding 3 confirmed: DetectedFace.Width/Height at imagegen/faceswap.go:58-60 duplicate what Box already encodes. Low-c…

🪰 Gadfly · advisory

⚪ **DetectedFace.Width/Height are redundant with Box, creating two sources of truth** _maintainability · flagged by 1 model_ - **Finding 1** confirmed: `initImageFilename` at `video.go:141-154` is the same MIME→filename switch in the same package (jpeg/webp/png), and `imageFilename` is a strict generalization. Real duplication. - **Finding 2** confirmed: `faceswap_test.go:165-169` declares `var apiErr *llm.APIError`, never assigns it, and discards it with `_ = apiErr`. Dead/abandoned code. - **Finding 3** confirmed: `DetectedFace.Width/Height` at `imagegen/faceswap.go:58-60` duplicate what `Box` already encodes. Low-c… <sub>🪰 Gadfly · advisory</sub>
// 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]
}
Review

🟠 FaceSwapper lacks a paired FaceSwapProvider/ModelOption surface, unlike every other optional imagegen capability

maintainability · flagged by 1 model

  • imagegen/faceswap.go:65FaceSwapper breaks the package's established optional-capability pattern. Every other optional surface in this package (Segmenter/SegmentationProvider in segment.go, Colorizer/FaceRestorer/FaceRestoreProvider in restore.go/facerestore.go, BackgroundRemover/BackgroundRemovalProvider and Upscaler/UpscaleProvider in background.go/upscale.go) ships a paired XProvider interface (Name() + `XModel(id string, opts ...XModelOption) (X, e…

🪰 Gadfly · advisory

🟠 **FaceSwapper lacks a paired FaceSwapProvider/ModelOption surface, unlike every other optional imagegen capability** _maintainability · flagged by 1 model_ - **`imagegen/faceswap.go:65`** — `FaceSwapper` breaks the package's established optional-capability pattern. Every other optional surface in this package (`Segmenter`/`SegmentationProvider` in `segment.go`, `Colorizer`/`FaceRestorer`/`FaceRestoreProvider` in `restore.go`/`facerestore.go`, `BackgroundRemover`/`BackgroundRemovalProvider` and `Upscaler`/`UpscaleProvider` in `background.go`/`upscale.go`) ships a paired `XProvider` interface (`Name()` + `XModel(id string, opts ...XModelOption) (X, e… <sub>🪰 Gadfly · advisory</sub>
// 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)
}
+186
View File
@@ -0,0 +1,186 @@
// faceswap.go implements imagegen.FaceSwapper against the InsightFace shim
// (buffalo_l + inswapper_128) reached through llama-swap's /upstream
// passthrough (ADR-0024):
//
// POST /upstream/<id>/v1/faces multipart file -> JSON
// POST /upstream/<id>/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}
// 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)
}
// Only when it will actually be sent: under All the index is documented
Outdated
Review

Negative Index rejected even when All=true, where index is documented as ignored

correctness, error-handling · flagged by 2 models

  • provider/llamaswap/faceswap.go:100-102 (minor) — Confirmed: the negative-index check if req.Index != nil && *req.Index < 0 runs unconditionally, before req.All is considered, even though imagegen/faceswap.go:28 documents "All swaps every detected face and ignores Index." So FaceSwapRequest{All: true, Index: &negativeIndex} is rejected despite Index being irrelevant under All. Low real-world impact (nil is the normal unset state), but it's an inconsistency between the doc…

🪰 Gadfly · advisory

⚪ **Negative Index rejected even when All=true, where index is documented as ignored** _correctness, error-handling · flagged by 2 models_ - **`provider/llamaswap/faceswap.go:100-102`** (minor) — Confirmed: the negative-index check `if req.Index != nil && *req.Index < 0` runs unconditionally, before `req.All` is considered, even though `imagegen/faceswap.go:28` documents "`All` swaps every detected face and ignores `Index`." So `FaceSwapRequest{All: true, Index: &negativeIndex}` is rejected despite `Index` being irrelevant under `All`. Low real-world impact (nil is the normal unset state), but it's an inconsistency between the doc… <sub>🪰 Gadfly · advisory</sub>
// 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")
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 {
Review

🔴 JSON/non-image response with no Content-Type header is silently accepted and mislabeled image/png instead of rejected

correctness, error-handling, maintainability, security · flagged by 2 models

  • provider/llamaswap/faceswap.go:134 — The non-image-response guard (respType != "" && !isImageContentType(respType)) is bypassed whenever the upstream response omits a Content-Type header, because doRaw (audio.go:363) returns resp.Header.Get("Content-Type") which is "" for a missing header. In that case the code falls through to sniffImageMIME(raw) (image.go:237-243), which defaults to image/png for any payload it can't identify as an image — so a JSON error body served without…

🪰 Gadfly · advisory

🔴 **JSON/non-image response with no Content-Type header is silently accepted and mislabeled image/png instead of rejected** _correctness, error-handling, maintainability, security · flagged by 2 models_ - `provider/llamaswap/faceswap.go:134` — The non-image-response guard (`respType != "" && !isImageContentType(respType)`) is bypassed whenever the upstream response omits a `Content-Type` header, because `doRaw` (audio.go:363) returns `resp.Header.Get("Content-Type")` which is `""` for a missing header. In that case the code falls through to `sniffImageMIME(raw)` (image.go:237-243), which defaults to `image/png` for any payload it can't identify as an image — so a JSON error body served without… <sub>🪰 Gadfly · advisory</sub>
return nil, &llm.APIError{Provider: m.p.name, Model: m.id, Message: "face swap response contained no image"}
}
// 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 (sniffed %q, Content-Type %q): %s",
detected, respType, truncateForError(raw))}
}
Review

🟡 imageFilename near-duplicates initImageFilename (video.go) in the same package; should be shared

maintainability · flagged by 1 model

  • provider/llamaswap/faceswap.go:147imageFilename near-duplicates the existing initImageFilename. initImageFilename(mimeType) at video.go:141-154 is the same MIME→filename switch in the same package (jpeg/webp/png), and the new imageFilename(mimeType, base) is a strict generalization of it (parameterized prefix, plus gif/bmp). This is copy-paste that should be shared: initImageFilename could become return imageFilename(mimeType, "frame") (its default already resolves to…

🪰 Gadfly · advisory

🟡 **imageFilename near-duplicates initImageFilename (video.go) in the same package; should be shared** _maintainability · flagged by 1 model_ - **`provider/llamaswap/faceswap.go:147` — `imageFilename` near-duplicates the existing `initImageFilename`.** `initImageFilename(mimeType)` at `video.go:141-154` is the same MIME→filename switch in the *same package* (jpeg/webp/png), and the new `imageFilename(mimeType, base)` is a strict generalization of it (parameterized prefix, plus gif/bmp). This is copy-paste that should be shared: `initImageFilename` could become `return imageFilename(mimeType, "frame")` (its default already resolves to… <sub>🪰 Gadfly · advisory</sub>
// 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"
}
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"
}
}
+261
View File
@@ -0,0 +1,261 @@
package llamaswap
import (
"context"
"encoding/base64"
"errors"
"io"
"mime"
"mime/multipart"
"net/http"
"net/http/httptest"
"strings"
"testing"
"gitea.stevedudenhoeffer.com/steve/majordomo/imagegen"
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
)
Review

🟡 swapImg duplicates editInit verbatim in the same package

maintainability · flagged by 1 model

  • provider/llamaswap/faceswap_test.go:18-24swapImg is a byte-for-byte duplicate of editInit in edit_test.go:16-23 (same package llamaswap): decode onePixelPNG, wrap in imagegen.Image{MIME: "image/png", Data: raw}. Reuse editInit (or rename it to a capability-neutral name and use it from both files) instead of adding another near-identical helper.

🪰 Gadfly · advisory

🟡 **swapImg duplicates editInit verbatim in the same package** _maintainability · flagged by 1 model_ - **`provider/llamaswap/faceswap_test.go:18-24`** — `swapImg` is a byte-for-byte duplicate of `editInit` in `edit_test.go:16-23` (same package `llamaswap`): decode `onePixelPNG`, wrap in `imagegen.Image{MIME: "image/png", Data: raw}`. Reuse `editInit` (or rename it to a capability-neutral name and use it from both files) instead of adding another near-identical helper. <sub>🪰 Gadfly · advisory</sub>
// 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 := editInit(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 := 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)
}
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 := editInit(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 := 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)
}
}
Review

🟡 Dead apiErr variable (declared then _ = apiErr) — abandoned errors.As check, remove or wire up

maintainability · flagged by 2 models

  • **provider/llamaswap/faceswap_test.go:165 — dead apiErrinTestFaceSwapRejectsNonImageResponse.** var apiErr *llm.APIErroris declared and then discarded with_ = apiErr; nothing ever assigns to it (it looks like an abandoned errors.As(err, &apiErr)type assertion). The test only checksstrings.Contains. This is confusing leftover code — either drop the two lines or turn it into a real errors.Asassertion so the test actually pins that an*llm.APIError` is returned. Small.

🪰 Gadfly · advisory

🟡 **Dead apiErr variable (declared then `_ = apiErr`) — abandoned errors.As check, remove or wire up** _maintainability · flagged by 2 models_ - **`provider/llamaswap/faceswap_test.go:165 — dead `apiErr` in `TestFaceSwapRejectsNonImageResponse`.** `var apiErr *llm.APIError` is declared and then discarded with `_ = apiErr`; nothing ever assigns to it (it looks like an abandoned `errors.As(err, &apiErr)` type assertion). The test only checks `strings.Contains`. This is confusing leftover code — either drop the two lines or turn it into a real `errors.As` assertion so the test actually pins that an `*llm.APIError` is returned. Small. <sub>🪰 Gadfly · advisory</sub>
// 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(), editInit(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(), 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")
}
}
+19 -6
View File
@@ -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
+1 -13
View File
@@ -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).