Merge pull request 'fix(agent): recover the front-loaded answer over a summary closer' (#25) from fix/finalize-summary-closer into main

All tidiness, no behavior change: the leading-marker class is one shared
constant for citationLabelRe and summaryCloserRe (hand-copying it is how
'+' went missing the first time); the deliberate 'all' duplication across
summaryCopulas/summaryArticle is now stated at both sites; the weak-final
switch case assigns modeBackRef explicitly; test comments state the
constraint they guard instead of which reviewer asked for them.

Co-Authored-By: Claude Fable 5 <[email protected]>

agent/finalize.go

@@ -14,8 +14,8 @@ import (
 // their answer to the final, tool-free turn. But some models — notably several
 // open-weight ones — "front-load" their full answer into an earlier turn that
 // ALSO calls a tool (e.g. answer text alongside a citation call), then close
-// with a degenerate terminal turn that is not itself the answer. Two shapes are
+// with a degenerate terminal turn that is not itself the answer. Three shapes
-// recovered from the transcript (zero extra model calls):
+// are recovered from the transcript (zero extra model calls):
 //
 //   - a trivial back-reference ("(Already answered above.)", "see above", …):
 //     the real answer sits earlier, so recover it and DISCARD the worthless
@@ -25,25 +25,44 @@ import (
 //     glm-5.2 "cite" pattern behind mort issue #1418). The citations are real,
 //     useful content — unlike a back-reference — so recover the prior answer and
 //     KEEP the citations, appended below it.
+//   - a bookkeeping closer ("Citations are logged. Short version: …"): the
+//     model acknowledged the citation round and compressed the answer it had
+//     already written into a one-liner (mort run b3cb9ee9 — a 2,089-char answer
+//     shrank to a 153-byte closer at delivery). The compression is strictly
+//     poorer than the front-loaded answer, so recover the prior turn and
+//     DISCARD the closer — but only when the prior turn clearly dwarfs it,
+//     because unlike a back-reference this closer DOES carry answer content
+//     (see modeSummary).
 //
-// A citations addendum is tested first and wins over the back-reference test (a
+// A citations addendum is tested first and wins over the other two (a short
-// short terminal can be both), so its links are never discarded. When the
+// terminal can match more than one shape), so its links are never discarded.
-// terminal text stands on its own it is returned unchanged; when it is
+// The back-reference test wins over the summary-closer test: a terminal
-// degenerate but nothing better can be recovered, it is returned as-is (a bare
+// matching both ("Citations are logged. As I said above…") carries no answer
-// sources list still beats nothing).
+// content of its own, so the looser back-ref recovery bar — not the summary
+// closer's dwarf ratio — is the right one. When the terminal text stands
+// on its own it is returned unchanged; when it is degenerate but nothing
+// better can be recovered, it is returned as-is (a compressed answer still
+// beats nothing).
 //
 // msgs must already include the terminal assistant message as its last element
 // (the loop appends it before calling this); terminal is that message's text.
 func finalOutput(msgs []llm.Message, terminal string) string {
-	citations := isCitationsOnly(terminal)
+	mode := modeBackRef
-	if !citations && !isWeakFinal(terminal) {
+	switch {
+	case isCitationsOnly(terminal):
+		mode = modeCitations
+	case isWeakFinal(terminal):
+		mode = modeBackRef
+	case isSummaryCloser(terminal):
+		mode = modeSummary
+	default:
 		return terminal
 	}
-	rec, ok := lastSubstantiveAssistantText(msgs, terminal, citations)
+	rec, ok := lastSubstantiveAssistantText(msgs, terminal, mode)
 	if !ok {
 		return terminal
 	}
-	if citations {
+	if mode == modeCitations {
 		// Preserve the citations addendum below the recovered answer, unless the
 		// recovered turn already carries it (guards against a duplicate sources
 		// block when the front-loaded turn included its own citations). The
@@ -65,11 +84,64 @@ func stripURLAngles(s string) string {
 	return strings.NewReplacer("<", "", ">", "").Replace(s)
 }
 
+// recoveryMode selects the bar a prior assistant turn must clear to replace
+// the terminal turn (see isSubstantiveAnswer) and what finalOutput does with
+// the terminal once recovery succeeds.
+type recoveryMode int
+
+const (
+	// modeBackRef: the terminal is empty or a pure back-reference — worthless
+	// on its own, so any real prior answer replaces it and it is discarded.
+	modeBackRef recoveryMode = iota
+	// modeCitations: the terminal is a sources-only addendum — not a rival
+	// answer, so the dwarf ratio is skipped and the addendum is kept, appended
+	// below the recovered answer.
+	modeCitations
+	// modeSummary: the terminal acknowledges the citation round and may carry
+	// a short compression of the front-loaded answer. Unlike a back-reference
+	// it DOES contain answer content, so it is only replaced when a prior turn
+	// clearly dwarfs it — the ratio is mandatory at every length, the recovery
+	// scan stops at the most recent user message (a compression can only be of
+	// THIS turn's answer; never resurrect one from an earlier question), and
+	// the closer is discarded (its content is a strict subset of what it
+	// replaced).
+	modeSummary
+)
+
 // backRefRe matches a terminal turn that merely points back to an earlier
 // message instead of stating the answer ("(Already answered above.)",
 // "see above", "as I said", ...).
 var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(said|mentioned|stated|noted)|answered (that )?above|per my (previous|earlier))`)
 
+// summaryCloserRe matches a terminal turn that OPENS with a bookkeeping
+// acknowledgment of the citation round — "Citations are logged.", "Sources
+// cited.", "Logged the citations." — the shape a model produces when it
+// front-loaded its answer into an earlier cite-call turn and closes by
+// acknowledging the tool results, often followed by a "Short version: …"
+// compression of the answer it already wrote. The ack clause must end at a
+// sentence terminator ([.!]) DIRECTLY after the verb: "The citations are
+// recorded in the court transcript…" is a real answer about citations, not
+// bookkeeping, and must never match. A compression marker without the ack
+// ("Short version: no.") is deliberately out of scope — a user who asked for
+// brevity would be answered with exactly that shape, and misclassifying it
+// would hijack a legitimate answer; an unmatched closer merely keeps today's
+// behavior (fail closed). Assembled from named fragments so the alternations
+// stay legible and extendable.
+const (
+	summaryPreface = `((done|all set|ok(ay)?)[\s,.!:—-]+)?` // optional "Done —" style opener
+	summaryNouns   = `(citations?|sources?|references?|claims?)`
+	// "all" appears here AND in summaryArticle on purpose: as a quantifier
+	// between noun and verb ("Citations all logged.") and as a determiner
+	// before the noun ("All claims cited.", "Logged all the citations.").
+	summaryCopulas = `((are|were|have\s+been|all)\s+)*`
+	summaryVerbs   = `(logged|recorded|cited|saved|noted|captured|filed)`
+	summaryArticle = `((all|the)\s+)*` // star, not ?: "Logged all the citations."
+)
+
+var summaryCloserRe = regexp.MustCompile(`(?i)^` + leadingMarkers + summaryPreface +
+	`(` + summaryArticle + summaryNouns + `\s+` + summaryCopulas + summaryVerbs +
+	`|logged\s+` + summaryArticle + summaryNouns + `)[.!]`)
+
 // preambleRe matches intent-announcing prefixes ("Let me search...", "I'll
 // check...") so a preamble is never mistaken for the answer during recovery.
 var preambleRe = regexp.MustCompile(`(?i)^(let me|let'?s|i'?ll|i will|first[, ]|sure[,. ]|okay[,. ]|on it|checking)`)
@@ -83,7 +155,15 @@ var preambleRe = regexp.MustCompile(`(?i)^(let me|let'?s|i'?ll|i will|first[, ]|
 // the colon/dash separator. Anchored at ^ so a normal answer that merely
 // mentions "sources" mid-sentence, or ends with a "Sources:" section AFTER its
 // prose, is never matched.
-var citationLabelRe = regexp.MustCompile(`(?i)^[\s>#*_+-]*(sources?|references?|citations?|works cited|further reading)\b[\s*_]*[:：\-—]`)
+var citationLabelRe = regexp.MustCompile(`(?i)^` + leadingMarkers +
+	`(sources?|references?|citations?|works cited|further reading)\b[\s*_]*[:：\-—]`)
+
+// leadingMarkers tolerates markdown noise before a label: emphasis (*, _),
+// list (-, +, *), block-quote (>), and ATX-heading (#) markers, with their
+// whitespace. Shared by citationLabelRe and summaryCloserRe so the two
+// classifiers cannot drift apart (the first draft of the summary class
+// dropped '+' by hand-copying this set).
+const leadingMarkers = `[\s>#*_+-]*`
 
 // linkRe matches a whole markdown link "[label](url)" or a bare URL. Used both
 // to require that a citations terminal carries at least one link and to strip
@@ -114,6 +194,12 @@ const (
 	// be at most len/N of the whole, so a prose answer that merely opens with
 	// "Source:" and cites a URL mid-sentence is not mistaken for a bare list.
 	citationDominatedDivisor = 3
+	// summaryCloserMaxChars bounds a summary closer: room for the ack sentence
+	// plus a couple of compression sentences (the b3cb9ee9 closer was 153
+	// bytes — Go len(), which is what every threshold here compares). Beyond
+	// this the "short version" is substantial enough that replacing it risks
+	// losing content the front-loaded turn never had.
+	summaryCloserMaxChars = 300
 )
 
 // isWeakFinal reports whether a terminal turn's text fails to stand on its own
@@ -152,14 +238,39 @@ func isCitationsOnly(s string) bool {
 	return len(residue) <= len(t)/citationDominatedDivisor
 }
 
+// isSummaryCloser reports whether a terminal turn is a bookkeeping closer: it
+// opens with a complete "citations are logged"-style ack sentence (see
+// summaryCloserRe) and is short enough that whatever follows the ack can only
+// be a compression of an earlier, fuller answer. Whether that fuller answer
+// actually exists is modeSummary's job — the dwarf ratio in
+// isSubstantiveAnswer keeps a matching closer in place when nothing earlier
+// clearly outweighs it.
+func isSummaryCloser(s string) bool {
+	t := strings.TrimSpace(s)
+	if t == "" || len(t) > summaryCloserMaxChars {
+		return false
+	}
+	return summaryCloserRe.MatchString(t)
+}
+
 // lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal
 // turn and empty tool-only turns) for the most recent assistant turn whose text
-// reads like a real answer. citations selects the recovery bar (see
+// reads like a real answer. mode selects the recovery bar (see
 // isSubstantiveAnswer). Returns ("", false) when nothing qualifies.
-func lastSubstantiveAssistantText(msgs []llm.Message, terminal string, citations bool) (string, bool) {
+func lastSubstantiveAssistantText(msgs []llm.Message, terminal string, mode recoveryMode) (string, bool) {
 	tt := strings.TrimSpace(terminal)
 	for i := len(msgs) - 1; i >= 0; i-- {
 		m := msgs[i]
+		if mode == modeSummary && m.Role == llm.RoleUser {
+			// A summary closer compresses THIS turn's front-loaded answer, so
+			// the scan must not cross into an earlier question: once the dwarf
+			// ratio has rejected the current turn's text, walking further back
+			// would resurrect a stale answer to a DIFFERENT question — strictly
+			// worse than keeping the closer. (A mid-run steer message is also a
+			// user-role boundary; recovery then fails closed, which is fine.)
+			// The other modes keep their historical unbounded scan.
+			break
+		}
 		if m.Role != llm.RoleAssistant {
 			continue
 		}
@@ -167,7 +278,7 @@ func lastSubstantiveAssistantText(msgs []llm.Message, terminal string, citations
 		if txt == "" || txt == tt {
 			continue // the terminal turn itself, or an empty tool-only turn
 		}
-		if isSubstantiveAnswer(txt, tt, citations) {
+		if isSubstantiveAnswer(txt, tt, mode) {
 			return txt, true
 		}
 	}
@@ -177,20 +288,29 @@ func lastSubstantiveAssistantText(msgs []llm.Message, terminal string, citations
 // isSubstantiveAnswer reports whether txt (a prior assistant turn) reads like a
 // real answer rather than a preamble, relative to the terminal text.
 //
-// A sufficiently long turn (>= recoverMinChars) is accepted unconditionally: a
+// modeSummary demands the dwarf ratio FIRST, at every length: a summary closer
-// multi-hundred-char turn is an answer even when it opens conversationally
+// carries a real (compressed) answer, so replacing it is only justified when
-// ("Sure, here's…", "Let me explain: …"), so the preamble filter is NOT applied
+// the prior turn is clearly the fuller original it was compressed from.
-// to it — applying it there would drop a legitimate long front-loaded answer.
+//
-// Only in the borderline band does a turn have to clear a floor, not read like a
+// A sufficiently long turn (>= recoverMinChars) is otherwise accepted
-// short planning preamble ("Let me look that up…"), and — unless the terminal is
+// unconditionally: a multi-hundred-char turn is an answer even when it opens
-// a citations addendum (not a rival answer, so its length is irrelevant) — also
+// conversationally ("Sure, here's…", "Let me explain: …"), so the preamble
-// clearly dwarf the terminal.
+// filter is NOT applied to it — applying it there would drop a legitimate long
-func isSubstantiveAnswer(txt, terminal string, citations bool) bool {
+// front-loaded answer. Only in the borderline band does a turn have to clear a
+// floor, not read like a short planning preamble ("Let me look that up…"),
+// and — for modeBackRef only — also clearly dwarf the terminal (a citations
+// addendum is not a rival answer, so its length is irrelevant; a summary
+// closer already proved the ratio above).
+func isSubstantiveAnswer(txt, terminal string, mode recoveryMode) bool {
+	dwarfs := len(txt) >= recoverRatio*len(terminal)
+	if mode == modeSummary && !dwarfs {
+		return false
+	}
 	if len(txt) >= recoverMinChars {
 		return true
 	}
 	if len(txt) < recoverFloorChars || preambleRe.MatchString(txt) {
 		return false
 	}
-	return citations || len(txt) >= recoverRatio*len(terminal)
+	return mode != modeBackRef || dwarfs
 }

agent/finalize_test.go

@@ -72,6 +72,46 @@ func TestIsCitationsOnly(t *testing.T) {
 	}
 }
 
+// b3cb9ee9Closer is the verbatim terminal turn from mort run b3cb9ee9: a
+// 2,089-char answer was front-loaded into the cite-call turn and this 153-byte
+// compression (151 runes — the em dash is 3 bytes, and byte length is what the
+// thresholds compare) was all that got delivered.
+const b3cb9ee9Closer = "Citations are logged. Short version: the bulk of that ~$64M was AIPAC and dark-money super PACs, not the party committees — and it still wasn't enough."
+
+func TestIsSummaryCloser(t *testing.T) {
+	cases := []struct {
+		name string
+		in   string
+		want bool
+	}{
+		{"b3cb9ee9-verbatim", b3cb9ee9Closer, true},
+		{"ack-only", "Citations are logged.", true},
+		{"ack-no-copula", "Citations logged.", true},
+		{"claims-cited", "All claims cited.", true},
+		{"verb-first", "Logged the citations.", true},
+		{"done-prefix", "Done — citations logged.", true},
+		{"ack-then-tldr", "Sources have been recorded! TL;DR: the GPU was the bottleneck.", true},
+		{"references-noted", "References noted. In short: yes, it ships Tuesday.", true},
+		{"plus-list-marker", "+ Citations are logged.", true},
+		{"logged-all-the", "Logged all the citations.", true},
+
+		{"empty", "", false},
+		{"ack-continues-midsentence", "The citations are recorded in the court transcript, which shows the filing dates.", false},
+		{"ack-verb-then-clause", "Citations are logged in Zotero whenever you click the save button.", false},
+		{"compression-without-ack", "Short version: yes.", false}, // deliberately out of scope
+		{"mentions-citations-midsentence", "The paper's citations are what got it retracted.", false},
+		{"crisp-number", "42", false},
+		{"over-cap", "Citations are logged. " + strings.Repeat("The long version has many more details worth keeping. ", 6), false}, // >300: too substantial to replace
+	}
+	for _, c := range cases {
+		t.Run(c.name, func(t *testing.T) {
+			if got := isSummaryCloser(c.in); got != c.want {
+				t.Errorf("isSummaryCloser(%q) = %v, want %v", c.in, got, c.want)
+			}
+		})
+	}
+}
+
 func asst(text string, tools ...llm.ToolCall) llm.Message {
 	m := llm.Message{Role: llm.RoleAssistant}
 	if text != "" {
@@ -83,7 +123,8 @@ func asst(text string, tools ...llm.ToolCall) llm.Message {
 
 func TestFinalOutput(t *testing.T) {
 	cite := []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}}
-	longAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 6)) // >200
+	longAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 6))  // >200
+	hugeAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 12)) // >3x the b3cb9ee9 closer
 	// A sources/citations-only terminal — the glm-5.2 "cite" shape behind mort
 	// issue #1418: the prose answer was front-loaded into the tool-call turn and
 	// the terminal turn carried only the citations.
@@ -103,6 +144,9 @@ func TestFinalOutput(t *testing.T) {
 	// A >=200-byte real answer that merely OPENS with a conversational word
 	// ("Sure,"). The preamble filter must NOT veto it (gadfly regression guard).
 	longConversationalAnswer := "Sure, here's the rundown: it currently sells for about $2,700 used on eBay, typically $2,400 to $2,900 depending on condition and bundle, with the sealed Founders Edition commanding the top of that range while used AIB cards go a bit lower."
+	// Matches BOTH the summary ack and backRefRe, within the 120-byte weak cap,
+	// and long enough (>~92 bytes) that longAnswer would fail the summary bar.
+	bothMatchCloser := "Citations are logged. As I mentioned above, the full detail on the money sources is in my earlier message."
 
 	tests := []struct {
 		name     string
@@ -256,6 +300,109 @@ func TestFinalOutput(t *testing.T) {
 			terminal: sources,
 			want:     longConversationalAnswer + "\n\n" + sources,
 		},
+		{
+			// The b3cb9ee9 shape: full answer front-loaded into the cite turn,
+			// then a summary closer. The closer is discarded — its content is a
+			// strict compression of the recovered answer.
+			name: "summary closer discarded when the front-loaded answer dwarfs it",
+			msgs: []llm.Message{
+				llm.UserText("where did the $64M come from?"),
+				asst(hugeAnswer, cite...),
+				llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
+				asst(b3cb9ee9Closer),
+			},
+			terminal: b3cb9ee9Closer,
+			want:     hugeAnswer,
+		},
+		{
+			// The dwarf ratio is mandatory for a summary closer at EVERY length:
+			// a prior turn that is longer but not clearly the fuller original
+			// (here ~275 chars vs a 151-char closer, under the 3x bar) must not
+			// displace a closer that carries real answer content.
+			name: "summary closer kept when the prior turn does not dwarf it",
+			msgs: []llm.Message{
+				llm.UserText("q?"),
+				asst(longAnswer, cite...),
+				llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
+				asst(b3cb9ee9Closer),
+			},
+			terminal: b3cb9ee9Closer,
+			want:     b3cb9ee9Closer,
+		},
+		{
+			// An ack-only closer ("Citations are logged.") is tiny, so even a
+			// modest front-loaded answer clears the ratio and replaces it.
+			name: "ack-only summary closer recovered over a modest answer",
+			msgs: []llm.Message{
+				llm.UserText("q?"),
+				asst(longAnswer, cite...),
+				llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
+				asst("Citations are logged."),
+			},
+			terminal: "Citations are logged.",
+			want:     longAnswer,
+		},
+		{
+			name: "summary closer with only a preamble prior keeps the closer",
+			msgs: []llm.Message{
+				llm.UserText("q?"),
+				asst("Let me gather the numbers.", cite...),
+				llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
+				asst(b3cb9ee9Closer),
+			},
+			terminal: b3cb9ee9Closer,
+			want:     b3cb9ee9Closer,
+		},
+		{
+			// The modeSummary scan must stop at the most recent user message.
+			// Here the current turn's answer sits in the 1x-3x band (rejected
+			// by the ratio) while a dwarfing answer to a DIFFERENT question
+			// sits in history — resurrecting it would be strictly worse than
+			// keeping the closer.
+			name: "summary closer never resurrects a stale answer across the user boundary",
+			msgs: []llm.Message{
+				llm.UserText("earlier, unrelated question?"),
+				asst(hugeAnswer),
+				llm.UserText("q?"),
+				asst(conciseAnswer, cite...),
+				llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
+				asst(b3cb9ee9Closer),
+			},
+			terminal: b3cb9ee9Closer,
+			want:     b3cb9ee9Closer,
+		},
+		{
+			// The boundary must not break the legitimate multi-turn case: the
+			// dwarfing front-loaded answer in THIS turn's window is recovered
+			// even with history behind it.
+			name: "summary closer recovery still works with history present",
+			msgs: []llm.Message{
+				llm.UserText("earlier, unrelated question?"),
+				asst(longAnswer),
+				llm.UserText("q?"),
+				asst(hugeAnswer, cite...),
+				llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
+				asst(b3cb9ee9Closer),
+			},
+			terminal: b3cb9ee9Closer,
+			want:     hugeAnswer,
+		},
+		{
+			// A closer matching BOTH the ack shape and a back-reference
+			// carries no answer content, so the back-ref test must win and the
+			// ordinary recovery bar apply — under the summary bar this
+			// ~106-byte terminal would demand a ~318-byte prior and wrongly
+			// keep the closer over longAnswer.
+			name: "back-reference wins over the summary ack when both match",
+			msgs: []llm.Message{
+				llm.UserText("q?"),
+				asst(longAnswer, cite...),
+				llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
+				asst(bothMatchCloser),
+			},
+			terminal: bothMatchCloser,
+			want:     longAnswer,
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.name, func(t *testing.T) {
@@ -326,6 +473,37 @@ func TestRun_HealthyTerminalUnchanged(t *testing.T) {
 	}
 }
 
+// TestRun_RecoversFrontLoadedAnswerOverSummaryCloser reproduces mort run
+// b3cb9ee9 end-to-end: the model front-loads its full answer into the
+// cite-call turn, the cite results come back, and the terminal turn is only a
+// bookkeeping ack plus a one-line compression. The delivered output must be
+// the front-loaded answer, with no extra model call.
+func TestRun_RecoversFrontLoadedAnswerOverSummaryCloser(t *testing.T) {
+	hugeAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 12))
+	fp := fake.New("fp")
+	fp.Enqueue("test-model",
+		fake.ReplyWith(llm.Response{
+			Parts:        []llm.Part{llm.Text(hugeAnswer)},
+			ToolCalls:    []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}},
+			FinishReason: llm.FinishToolCalls,
+			Usage:        llm.Usage{InputTokens: 10, OutputTokens: 5},
+		}),
+		fake.Reply(b3cb9ee9Closer),
+	)
+
+	a := New(newModel(t, fp), "sys", WithToolbox(citeToolbox(t)))
+	res, err := a.Run(context.Background(), "where did the $64M come from?")
+	if err != nil {
+		t.Fatalf("Run: %v", err)
+	}
+	if res.Output != hugeAnswer {
+		t.Errorf("Output = %q, want recovered front-loaded answer", res.Output)
+	}
+	if n := len(fp.Calls()); n != 2 {
+		t.Errorf("model calls = %d, want 2 (no extra nudge turn)", n)
+	}
+}
+
 // TestRun_RecoversFrontLoadedAnswerWithCitations reproduces mort issue #1418
 // end-to-end: the model front-loads the prose answer into the tool-call turn
 // and closes with a sources-only terminal turn. The delivered output must be

imagegen/faceswap.go

@@ -0,0 +1,137 @@
+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
+
+	// Yaw is how far the head is turned from camera, in degrees, or nil when
+	// the provider does not report pose. Exposed on ENUMERATION, not just
+	// after the fact, because it is how a caller picks a face a swap will
+	// actually work on: past roughly ±45° the features carrying identity are
+	// edge-on, and the result reads as a generic person however good the
+	// transfer is. A bounding box cannot show this.
+	Yaw *float64
+}
+
+// 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]
+}
+
+// SwappedFace is the MEASURED outcome for one face the provider replaced.
+//
+// It exists because "the call returned an image" and "the likeness
+// transferred" are different claims that look identical from outside, and a
+// caller that cannot tell them apart will go looking for another way to
+// check. The one it reaches for — asking a vision model who the result looks
+// like — is wrong in exactly the cases that matter: a VLM shown a jogger in a
+// Georgetown cap holding McDonald's cups answers "Bill Clinton" whoever's
+// face is on him, so it reports failure on a correct swap.
+type SwappedFace struct {
+	// Index is the face's position in the provider's left-to-right ordering.
+	Index int
+
+	// Width, Height are the replaced face's pixel size in the TARGET.
+	// Meaningful only against ImageWidth/ImageHeight: a 138px face is large
+	// in a 400px picture and nearly invisible in a 2000px one, and it is the
+	// ratio, not the absolute size, that decides whether a person notices.
+	Width, Height int
+
+	// ImageWidth, ImageHeight are the target image's dimensions, repeated on
+	// every entry so a single face is self-describing without the caller
+	// holding onto the rest of the response.
+	ImageWidth, ImageHeight int
+
+	// Yaw is how far the head is turned from camera, in degrees, or nil when
+	// the provider does not report pose. The best single predictor of whether
+	// a swap will READ as the source person: past roughly ±45° the features
+	// carrying identity are edge-on and the result looks like a generic
+	// person rather than a specific one.
+	Yaw *float64
+
+	// IdentitySimilarity is cosine similarity between the source face and the
+	// face actually present in the result, 0-1, or nil when the provider
+	// could not measure it. Above ~0.5 the identity transferred; a LOW value
+	// is the only evidence that a swap genuinely failed.
+	IdentitySimilarity *float64
+}
+
+// FractionOfImage is the swapped face's width as a share of the image's, 0-1.
+// The number that predicts whether a person will SEE the change: the swap
+// that prompted all this replaced a 138px face in a 1010px-wide photo — 14%,
+// correct by every measure and invisible at a glance — while the same code on
+// a 168px face in a 385px picture (44%) is unmistakable. Returns 0 when the
+// dimensions are unknown.
+func (f SwappedFace) FractionOfImage() float64 {
+	if f.ImageWidth <= 0 || f.Width <= 0 {
+		return 0
+	}
+	return float64(f.Width) / float64(f.ImageWidth)
+}
+
+// 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)
+}

imagegen/imagegen.go

@@ -68,6 +68,15 @@ type Result struct {
 	// Images are the generated images, in the order the backend returned them.
 	Images []Image
 
+	// SwappedFaces is the measured outcome of a FaceSwap, one entry per face
+	// replaced. Empty for every other operation, and empty from a provider
+	// that does not measure. TYPED rather than tucked into Raw: a caller has
+	// to act on this — it is the only way to distinguish a swap that
+	// transferred the likeness from one that returned an image and nothing
+	// more — and a value reachable solely by type-asserting an `any` is one
+	// nobody discovers in time to use.
+	SwappedFaces []SwappedFace
+
 	// Raw is the provider-native response object, an escape hatch for
 	// provider-specific fields. May be nil; never required for normal use.
 	Raw any

provider/llamaswap/audio.go

@@ -338,27 +338,41 @@ func parseVoices(raw []byte) ([]string, error) {
 // varies. contentType sets the request Content-Type when body is non-nil.
 // A response larger than maxBytes is an error, never a silent truncation.
 func (p *Provider) doRaw(ctx context.Context, method, path, model, contentType string, body io.Reader, maxBytes int64) ([]byte, string, error) {
-	if err := p.requireBaseURL(); err != nil {
+	data, hdr, err := p.doRawHeaders(ctx, method, path, model, contentType, body, maxBytes)
+	if err != nil {
 		return nil, "", err
 	}
+	return data, hdr.Get("Content-Type"), nil
+}
+
+// doRawHeaders is doRaw with the WHOLE response header rather than just
+// Content-Type. Only the face swap needs it — the shim reports whether the
+// likeness actually transferred in X-Swap-Report, and that answer would be
+// thrown away by a function that keeps one header — so doRaw stays the
+// signature 25 other call sites use and delegates here. Two bodies would be
+// two places for the size cap and the status check to drift apart.
+func (p *Provider) doRawHeaders(ctx context.Context, method, path, model, contentType string, body io.Reader, maxBytes int64) ([]byte, http.Header, error) {
+	if err := p.requireBaseURL(); err != nil {
+		return nil, nil, err
+	}
 	req, err := p.newRequest(ctx, method, path, contentType, body)
 	if err != nil {
-		return nil, "", err
+		return nil, nil, err
 	}
 	resp, err := p.client.Do(req)
 	if err != nil {
-		return nil, "", fmt.Errorf("llama-swap: do request: %w", err)
+		return nil, nil, fmt.Errorf("llama-swap: do request: %w", err)
 	}
 	defer resp.Body.Close()
 	if resp.StatusCode/100 != 2 {
-		return nil, "", p.apiError(resp, model)
+		return nil, nil, p.apiError(resp, model)
 	}
 	data, err := io.ReadAll(io.LimitReader(resp.Body, maxBytes+1))
 	if err != nil {
-		return nil, "", fmt.Errorf("llama-swap: read response: %w", err)
+		return nil, nil, fmt.Errorf("llama-swap: read response: %w", err)
 	}
 	if int64(len(data)) > maxBytes {
-		return nil, "", fmt.Errorf("llama-swap: response exceeds %d bytes", maxBytes)
+		return nil, nil, fmt.Errorf("llama-swap: response exceeds %d bytes", maxBytes)
 	}
-	return data, resp.Header.Get("Content-Type"), nil
+	return data, resp.Header, nil
 }

provider/llamaswap/faceswap.go

@@ -0,0 +1,236 @@
+// 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"`
+		Yaw    *float64 `json:"yaw"`
+	} `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, Yaw: f.Yaw}
+		// 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
+	// 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, respHdr, err := m.p.doRawHeaders(ctx, http.MethodPost, path, m.id, contentType, body, maxFaceSwapResponseBytes)
+	if err != nil {
+		return nil, err
+	}
+	respType := respHdr.Get("Content-Type")
+	if len(raw) == 0 {
+		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))}
+	}
+	// 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}},
+		SwappedFaces: parseSwapReport(respHdr.Get("X-Swap-Report")),
+	}, nil
+}
+
+// swapReport mirrors the shim's X-Swap-Report header.
+type swapReport struct {
+	Image []int `json:"image"`
+	Faces []struct {
+		Index              int      `json:"index"`
+		Size               []int    `json:"size"`
+		Yaw                *float64 `json:"yaw"`
+		IdentitySimilarity *float64 `json:"identity_similarity"`
+	} `json:"faces"`
+}
+
+// parseSwapReport decodes the measured outcome. A missing or malformed header
+// yields nil rather than an error: an older shim does not send it, and a swap
+// that produced a good image must not fail because the diagnostics alongside
+// it were unreadable.
+func parseSwapReport(header string) []imagegen.SwappedFace {
+	header = strings.TrimSpace(header)
+	if header == "" {
+		return nil
+	}
+	var rep swapReport
+	if err := json.Unmarshal([]byte(header), &rep); err != nil {
+		return nil
+	}
+	out := make([]imagegen.SwappedFace, 0, len(rep.Faces))
+	for _, f := range rep.Faces {
+		sf := imagegen.SwappedFace{
+			Index:              f.Index,
+			Yaw:                f.Yaw,
+			IdentitySimilarity: f.IdentitySimilarity,
+		}
+		if len(f.Size) == 2 {
+			sf.Width, sf.Height = f.Size[0], f.Size[1]
+		}
+		if len(rep.Image) == 2 {
+			sf.ImageWidth, sf.ImageHeight = rep.Image[0], rep.Image[1]
+		}
+		out = append(out, sf)
+	}
+	if len(out) == 0 {
+		return nil
+	}
+	return out
+}
+
+// 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"
+	}
+}

provider/llamaswap/faceswap_test.go

@@ -0,0 +1,344 @@
+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"
+)
+
+// 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)
+	}
+}
+
+// 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")
+	}
+}
+
+// TestFaceSwapParsesSwapReport: the measured outcome is the whole reason the
+// header exists — a caller that cannot tell "the likeness transferred" from
+// "an image came back" goes and asks a vision model, which is wrong in
+// exactly the cases that matter.
+func TestFaceSwapParsesSwapReport(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.Header().Set("Content-Type", "image/png")
+		w.Header().Set("X-Swap-Report",
+			`{"image":[1010,1200],"faces":[{"index":2,"size":[138,172],"yaw":-82.2,"identity_similarity":0.791}]}`)
+		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})
+	if err != nil {
+		t.Fatalf("FaceSwap: %v", err)
+	}
+	if len(res.SwappedFaces) != 1 {
+		t.Fatalf("SwappedFaces = %d, want 1 — the measurement was dropped", len(res.SwappedFaces))
+	}
+	f := res.SwappedFaces[0]
+	if f.Index != 2 || f.Width != 138 || f.Height != 172 {
+		t.Errorf("face = %+v, want index 2 at 138x172", f)
+	}
+	if f.Yaw == nil || *f.Yaw != -82.2 {
+		t.Errorf("yaw = %v, want -82.2 — the pose signal is how a caller knows a profile swap will not read", f.Yaw)
+	}
+	if f.IdentitySimilarity == nil || *f.IdentitySimilarity != 0.791 {
+		t.Errorf("identity_similarity = %v, want 0.791", f.IdentitySimilarity)
+	}
+	// 138/1010 — the number that says "correct, and invisible at a glance".
+	if got := f.FractionOfImage(); got < 0.13 || got > 0.14 {
+		t.Errorf("FractionOfImage = %.3f, want ~0.137", got)
+	}
+}
+
+// TestFaceSwapSurvivesMissingReport: an older shim sends no header at all. A
+// swap that produced a good image must not fail because the diagnostics
+// beside it were absent or malformed.
+func TestFaceSwapSurvivesMissingReport(t *testing.T) {
+	for name, hdr := range map[string]string{
+		"absent":    "",
+		"garbage":   "not json at all",
+		"wrongtype": `{"faces":"nope"}`,
+	} {
+		t.Run(name, func(t *testing.T) {
+			srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				w.Header().Set("Content-Type", "image/png")
+				if hdr != "" {
+					w.Header().Set("X-Swap-Report", hdr)
+				}
+				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})
+			if err != nil {
+				t.Fatalf("a %s report failed the whole swap: %v", name, err)
+			}
+			if len(res.Images) != 1 {
+				t.Fatal("image lost")
+			}
+			if res.SwappedFaces != nil {
+				t.Errorf("SwappedFaces = %+v, want nil for a %s report", res.SwappedFaces, name)
+			}
+		})
+	}
+}

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)
+	for _, file := range files {
-	if err != nil {
+		fw, err := w.CreateFormFile(file.field, file.filename)
-		return nil, "", fmt.Errorf("llama-swap: %s: %w", wrap, err)
+		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 := 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

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))
+	return imageFilename(mimeType, "frame")
-	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"
-	}
 }
 
 // formatInt renders an optional int pointer for a form field; nil = "" (omit).