feat(imagegen): face swap (identity transfer), a separate operation from Edit #23
@@ -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 {
|
||||
|
gitea-actions
commented
🟠 FaceSwapper lacks a paired FaceSwapProvider/ModelOption surface, unlike every other optional imagegen capability maintainability · flagged by 1 model
🪰 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>
|
||||
// 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)
|
||||
}
|
||||
@@ -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/<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, 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 {
|
||||
|
gitea-actions
commented
⚪ Negative Index rejected even when All=true, where index is documented as ignored correctness, error-handling · flagged by 2 models
🪰 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>
|
||||
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)
|
||||
|
gitea-actions
commented
🔴 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
🪰 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>
|
||||
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 {
|
||||
|
gitea-actions
commented
🟡 imageFilename near-duplicates initImageFilename (video.go) in the same package; should be shared maintainability · flagged by 1 model
🪰 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>
|
||||
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/") != ""
|
||||
}
|
||||
@@ -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 {
|
||||
|
gitea-actions
commented
🟡 swapImg duplicates editInit verbatim in the same package maintainability · flagged by 1 model
🪰 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>
|
||||
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
|
||||
|
gitea-actions
commented
🟡 Dead apiErr variable (declared then maintainability · flagged by 2 models
🪰 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>
|
||||
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")
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
|
||||
⚪ DetectedFace.Width/Height are redundant with Box, creating two sources of truth
maintainability · flagged by 1 model
initImageFilenameatvideo.go:141-154is the same MIME→filename switch in the same package (jpeg/webp/png), andimageFilenameis a strict generalization. Real duplication. - Finding 2 confirmed:faceswap_test.go:165-169declaresvar apiErr *llm.APIError, never assigns it, and discards it with_ = apiErr. Dead/abandoned code. - Finding 3 confirmed:DetectedFace.Width/Heightatimagegen/faceswap.go:58-60duplicate whatBoxalready encodes. Low-c…🪰 Gadfly · advisory