10 Commits

Author SHA1 Message Date
steve 6abb399f5b fix(agent): recover front-loaded answer past a citations-only terminal turn (#11)
CI / Tidy (push) Successful in 9m35s
CI / Build & Test (push) Successful in 10m31s
finalOutput now recovers the front-loaded answer when the terminal turn is a
sources/citations-only addendum ("Sources: [x](url), ..."), not just when it is
empty or a back-reference. It recovers the prior substantive answer and appends
the (real) citations below it. Guards: citation-DOMINANCE (a prose answer that
merely opens with "Source: ... http://..." is left as the answer), ^-anchored
heading, citations recovery decoupled from the terminal-length ratio (concise
answers recover too), preamble filter applied only in the borderline band
(long answers opening with "Sure,"/"Let me" are not vetoed), and a dedup that
ignores <url> angle-bracket wrappers. Healthy terminal answers unchanged; zero
extra model calls.

Fixes mort #1418. Gadfly-reviewed (6 reviewers) + adversarially pre-verified;
all findings graded, real ones addressed.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-10 16:47:32 +00:00
steve fe44a6da26 Merge pull request 'fix(llamaswap): use A1111 /sdapi/v1/txt2img so seed is honored' (#10) from fix/llamaswap-a1111-seed into main
CI / Tidy (push) Successful in 9m25s
CI / Build & Test (push) Successful in 9m42s
2026-06-29 03:09:29 +00:00
steve a213c18263 fix(llamaswap): use A1111 /sdapi/v1/txt2img so seed is honored
CI / Tidy (pull_request) Successful in 9m24s
CI / Build & Test (pull_request) Successful in 9m45s
Adversarial Review (Gadfly) / review (pull_request) Successful in 11m30s
The OpenAI /v1/images/generations endpoint ignores `seed` on our
stable-diffusion.cpp build — every render of a given prompt comes back
byte-identical, so a drawbot batch of N collapsed to one image. Switch the
image provider to sd-server's A1111 /sdapi/v1/txt2img endpoint, which honors
`seed` (verified live: distinct seeds -> distinct images on SDXL and
Qwen-Image). Size is split into width/height; llama-swap still routes by the
`model` field. Tests + ADR-0016 updated.
2026-06-28 22:56:25 -04:00
Steve Dudenhoeffer 88d3fc3279 chore: repin gadfly reusable to @5007597 (structured findings + consensus + inline review)
Adopts gadfly's review-representation overhaul: one ranked consensus comment
across the swarm + an advisory COMMENT-state inline PR review, on image
sha-3095ebf. Swarm config still rides the owner variables.

[skip ci]
2026-06-28 22:13:24 -04:00
steve 531fe1922e Merge pull request 'feat(imagegen): optional per-request generation settings' (#9) from feat/imagegen-settings into main
CI / Tidy (push) Successful in 9m25s
CI / Build & Test (push) Successful in 9m47s
2026-06-29 02:00:40 +00:00
steve a744cdc335 feat(imagegen): optional per-request generation settings
CI / Tidy (pull_request) Successful in 9m27s
CI / Build & Test (pull_request) Successful in 9m47s
Add Steps, CFGScale, NegativePrompt, Sampler, Seed to imagegen.Request
(pointer/empty = leave the backend's per-model default), with mirror
options, and forward them in the llamaswap wire payload as the
stable-diffusion.cpp fields (steps/cfg_scale/negative_prompt/
sample_method/seed). Unset fields are omitted so sd-server keeps its
baked defaults.

Lets callers (e.g. mort drawbots) override only what they explicitly set.
2026-06-28 19:05:49 -04:00
steve 8b924700fb Merge pull request 'fix(media): drop oldest images on over-count instead of refusing' (#8) from fix/image-overflow-drop-oldest into main
CI / Tidy (push) Successful in 9m23s
CI / Build & Test (push) Successful in 9m45s
2026-06-28 22:43:20 +00:00
steve 70b7aebd86 test(media): match the overflow placeholder by const, not substring (gadfly #8)
CI / Tidy (pull_request) Successful in 9m25s
CI / Build & Test (pull_request) Successful in 9m49s
ragnaros/qwen3.6-27b noted TestNormalizeOverCount matched 'omitted' by substring;
the test is in-package, so assert == imageOverflowPlaceholder instead — robust to
wording changes. No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-28 18:33:01 -04:00
steve 52bb910f4d media: address gadfly review — single-pass elide, drop helpers, stronger test
CI / Tidy (pull_request) Successful in 9m27s
CI / Build & Test (pull_request) Successful in 9m41s
Review fixes (no behavior change):
- Fold the over-cap elide INTO the existing copy-on-write normalize pass: one
  loop now replaces the first toElide (oldest) images with the placeholder and
  size-normalizes the rest, so the Messages slice is copied at most once (the
  prior dropOldestImages + the normalize loop double-copied when overflow and a
  transform both applied — the dominant review finding, 5 models).
- Remove dropOldestImages (the name implied removal; it substituted) and the
  one-shot hasImagePart helper — both subsumed by the single pass.
- Trim the 9-line inline comment that restated the package doc.
- Test: rename TestNormalizeTooManyImages_DropsOldest → TestNormalizeOverCount
  (file convention) and assert the EXACT survivors ([b, c], in order) + a
  content-based non-mutation check (first input part is still image a, which a
  len check wouldn't catch).

Build + media + majordomo suites green (-race).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-28 18:06:17 -04:00
steve d71aca4c3a fix(media): drop oldest images on over-count instead of refusing the request
Adversarial Review (Gadfly) / review (pull_request) Has been cancelled
CI / Tidy (pull_request) Successful in 9m27s
CI / Build & Test (pull_request) Successful in 9m44s
media.Normalize refused (ErrUnsupported) when a request carried more images than
the target's MaxImagesPerReq, on the theory that a failover chain would try a
roomier target. In practice the chain's targets share the same cap — an agent loop
that accumulates a preview image per iteration (e.g. scaddy's write_scad) blows
past the cap, EVERY target rejects ("9 images, target allows at most 8"), and the
run dies. Observed live on ollama-cloud (cap 8).

Now: over-count keeps the most-recent MaxImagesPerReq images and replaces each
older one with a short text placeholder ("[earlier image omitted to fit this
model's per-request image limit]"), preserving each message's turn structure and
telling the model an image was elided. The most-recent images are the relevant
ones in an iterative run. Copy-on-write; the input request is never mutated. The
per-model threshold stays configurable via Capabilities.MaxImagesPerReq (0 still
means no image support); SupportsImages / MIME / byte-budget / dimension behavior
is unchanged, and the provider-side count backstop remains.

Test: TestNormalizeTooManyImages_DropsOldest — 3 images, cap 2 → 2 kept (the most
recent), 1 placeholder, no error, oldest dropped, input unmutated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-28 17:38:21 -04:00
8 changed files with 450 additions and 95 deletions
+1 -1
View File
@@ -41,7 +41,7 @@ jobs:
# Tracks gadfly's v1 release tag — a curated pointer re-moved on each release # Tracks gadfly's v1 release tag — a curated pointer re-moved on each release
# (unlike @main, which moves on every push). Central swarm tuning propagates # (unlike @main, which moves on every push). Central swarm tuning propagates
# here automatically; the tradeoff vs a full sha pin is that v1 is mutable. # here automatically; the tradeoff vs a full sha pin is that v1 is mutable.
uses: steve/gadfly/.gitea/workflows/review-reusable.yml@7bc3c982fa7b72367034c673f7812bf05e9c503e uses: steve/gadfly/.gitea/workflows/review-reusable.yml@5007597cf921dc3f0a83c708878facfe65fd8e8b
# Least privilege: forward only the review secrets (not `secrets: inherit`, # Least privilege: forward only the review secrets (not `secrets: inherit`,
# which would expose every repo secret). GITEA_TOKEN is the automatic token. # which would expose every repo secret). GITEA_TOKEN is the automatic token.
secrets: secrets:
+115 -17
View File
@@ -14,21 +14,55 @@ import (
// their answer to the final, tool-free turn. But some models — notably several // 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 // 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 // ALSO calls a tool (e.g. answer text alongside a citation call), then close
// with a trivial pointer such as "(Already answered above.)". Returning only // with a degenerate terminal turn that is not itself the answer. Two shapes are
// the terminal text would discard the real answer, which is still present // recovered from the transcript (zero extra model calls):
// earlier in the transcript. When the terminal text is weak (empty, or a short //
// back-reference) fall back to the last substantive assistant content in msgs. // - a trivial back-reference ("(Already answered above.)", "see above", …):
// the real answer sits earlier, so recover it and DISCARD the worthless
// closer.
// - a sources/citations-only addendum ("Sources: [x](…), [y](…)"): the model
// front-loaded the prose answer and closed with just its citations (the
// 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 citations addendum is tested first and wins over the back-reference test (a
// short terminal can be both), so its links are never discarded. 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 bare
// sources list still beats nothing).
// //
// msgs must already include the terminal assistant message as its last element // 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. // (the loop appends it before calling this); terminal is that message's text.
func finalOutput(msgs []llm.Message, terminal string) string { func finalOutput(msgs []llm.Message, terminal string) string {
if !isWeakFinal(terminal) { citations := isCitationsOnly(terminal)
if !citations && !isWeakFinal(terminal) {
return terminal return terminal
} }
if rec, ok := lastSubstantiveAssistantText(msgs, terminal); ok { rec, ok := lastSubstantiveAssistantText(msgs, terminal, citations)
if !ok {
return terminal
}
if citations {
// 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
// containment test ignores <url> angle-bracket wrappers so a turn that
// listed the same sources unwrapped still suppresses the duplicate.
if tail := strings.TrimSpace(terminal); !strings.Contains(stripURLAngles(rec), stripURLAngles(tail)) {
return rec + "\n\n" + tail
}
}
return rec return rec
}
// stripURLAngles removes the <…> wrappers Discord uses to suppress link embeds,
// so the citations dedup compares URLs regardless of that formatting delta.
func stripURLAngles(s string) string {
if !strings.ContainsAny(s, "<>") {
return s
} }
return terminal return strings.NewReplacer("<", "", ">", "").Replace(s)
} }
// backRefRe matches a terminal turn that merely points back to an earlier // backRefRe matches a terminal turn that merely points back to an earlier
@@ -40,19 +74,46 @@ var backRefRe = regexp.MustCompile(`(?i)(already answered|see above|as (i )?(sai
// check...") so a preamble is never mistaken for the answer during recovery. // 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)`) var preambleRe = regexp.MustCompile(`(?i)^(let me|let'?s|i'?ll|i will|first[, ]|sure[,. ]|okay[,. ]|on it|checking)`)
// citationLabelRe matches a terminal turn that OPENS with a sources/citations
// heading — the shape a model produces when it front-loads its answer into an
// earlier tool-call turn and closes with only its sources. Leading markdown
// emphasis (*, _), list (-, +, *), block-quote (>), and ATX-heading (#) markers
// — with their whitespace, since \s is in the class — are tolerated before the
// label, as is closing emphasis (** / __) plus whitespace between the label and
// 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*_]*[:\-—]`)
// 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
// links out when measuring how citation-dominated the terminal is.
var linkRe = regexp.MustCompile(`\[[^\]]*\]\([^)]*\)|https?://\S+`)
// citationResidueCutset is trimmed from the ends of a citations terminal's
// non-link remainder before measuring it — list bullets, separators, and the
// short per-source annotations models add in parentheses.
const citationResidueCutset = " \t\r\n,;.:|·•*_()[]—-"
const ( const (
// weakFinalMaxChars bounds how long a back-reference closer can be. A // weakFinalMaxChars bounds how long a back-reference closer can be. A
// genuine final answer that merely contains "as I said" mid-sentence is // genuine final answer that merely contains "as I said" mid-sentence is
// longer than this, so it is never treated as weak. // longer than this, so it is never treated as weak.
weakFinalMaxChars = 120 weakFinalMaxChars = 120
// recoverMinChars: a prior assistant turn this long is treated as a real // recoverMinChars: a prior assistant turn this long is treated as a real
// answer regardless of how it opens. // answer regardless of how it opens (the preamble filter is not applied at
// this length — see isSubstantiveAnswer).
recoverMinChars = 200 recoverMinChars = 200
// recoverFloorChars / recoverRatio gate the borderline band: a shorter // recoverFloorChars / recoverRatio gate the borderline band: a shorter
// prior turn must still clearly dwarf the (very short) terminal and not // prior turn must clear the floor and — unless the terminal is a citations
// look like a preamble. // addendum, which is not a rival answer — also clearly dwarf the (very
// short) terminal. See isSubstantiveAnswer.
recoverFloorChars = 80 recoverFloorChars = 80
recoverRatio = 3 recoverRatio = 3
// citationDominatedDivisor: a citations terminal's non-link remainder must
// 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
) )
// isWeakFinal reports whether a terminal turn's text fails to stand on its own // isWeakFinal reports whether a terminal turn's text fails to stand on its own
@@ -65,10 +126,37 @@ func isWeakFinal(s string) bool {
return len(t) <= weakFinalMaxChars && backRefRe.MatchString(t) return len(t) <= weakFinalMaxChars && backRefRe.MatchString(t)
} }
// isCitationsOnly reports whether a terminal turn is essentially just a
// sources/citations addendum: it OPENS with a citations heading, carries at
// least one link, and — once the heading and links are removed — is dominated
// by that citation structure (only list punctuation and short per-source
// annotations remain). The dominance check is what separates a bare sources
// list (recover the front-loaded answer, keep the links) from a real prose
// answer that merely opens with "Source:" and references a URL mid-sentence
// (leave it as the answer). Unlike a back-reference closer the links are worth
// keeping, so finalOutput appends them to the recovered answer.
//
// A citations terminal whose sources are bare domains (no scheme, no markdown
// link) is intentionally out of scope — there is no reliable link signal, so it
// is left as-is rather than risk misclassifying prose.
func isCitationsOnly(s string) bool {
t := strings.TrimSpace(s)
if !citationLabelRe.MatchString(t) {
return false
}
body := citationLabelRe.ReplaceAllString(t, "")
if !linkRe.MatchString(body) {
return false
}
residue := strings.Trim(linkRe.ReplaceAllString(body, ""), citationResidueCutset)
return len(residue) <= len(t)/citationDominatedDivisor
}
// lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal // lastSubstantiveAssistantText scans msgs newest→oldest (skipping the terminal
// turn and empty tool-only turns) for the most recent assistant turn whose text // turn and empty tool-only turns) for the most recent assistant turn whose text
// reads like a real answer. Returns ("", false) when nothing qualifies. // reads like a real answer. citations selects the recovery bar (see
func lastSubstantiveAssistantText(msgs []llm.Message, terminal string) (string, bool) { // isSubstantiveAnswer). Returns ("", false) when nothing qualifies.
func lastSubstantiveAssistantText(msgs []llm.Message, terminal string, citations bool) (string, bool) {
tt := strings.TrimSpace(terminal) tt := strings.TrimSpace(terminal)
for i := len(msgs) - 1; i >= 0; i-- { for i := len(msgs) - 1; i >= 0; i-- {
m := msgs[i] m := msgs[i]
@@ -79,7 +167,7 @@ func lastSubstantiveAssistantText(msgs []llm.Message, terminal string) (string,
if txt == "" || txt == tt { if txt == "" || txt == tt {
continue // the terminal turn itself, or an empty tool-only turn continue // the terminal turn itself, or an empty tool-only turn
} }
if isSubstantiveAnswer(txt, tt) { if isSubstantiveAnswer(txt, tt, citations) {
return txt, true return txt, true
} }
} }
@@ -88,11 +176,21 @@ func lastSubstantiveAssistantText(msgs []llm.Message, terminal string) (string,
// isSubstantiveAnswer reports whether txt (a prior assistant turn) reads like a // isSubstantiveAnswer reports whether txt (a prior assistant turn) reads like a
// real answer rather than a preamble, relative to the terminal text. // real answer rather than a preamble, relative to the terminal text.
func isSubstantiveAnswer(txt, terminal string) bool { //
// A sufficiently long turn (>= recoverMinChars) is accepted unconditionally: a
// multi-hundred-char turn is an answer even when it opens conversationally
// ("Sure, here's…", "Let me explain: …"), so the preamble filter is NOT applied
// 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
// short planning preamble ("Let me look that up…"), and — unless the terminal is
// a citations addendum (not a rival answer, so its length is irrelevant) — also
// clearly dwarf the terminal.
func isSubstantiveAnswer(txt, terminal string, citations bool) bool {
if len(txt) >= recoverMinChars { if len(txt) >= recoverMinChars {
return true return true
} }
return len(txt) >= recoverFloorChars && if len(txt) < recoverFloorChars || preambleRe.MatchString(txt) {
len(txt) >= recoverRatio*len(terminal) && return false
!preambleRe.MatchString(txt) }
return citations || len(txt) >= recoverRatio*len(terminal)
} }
+179
View File
@@ -36,6 +36,42 @@ func TestIsWeakFinal(t *testing.T) {
} }
} }
func TestIsCitationsOnly(t *testing.T) {
cases := []struct {
name string
in string
want bool
}{
{"sources-md-links", "Sources: [pcprice.watch](https://pcprice.watch/x), [ebay](https://ebay.com/1).", true},
{"lowercase-bare-url", "sources: see https://example.com/a", true},
{"bold-label-colon-inside", "**Sources:** [a](https://a), [b](https://b)", true},
{"bold-label-colon-outside", "**Sources**: [a](https://a), [b](https://b)", true}, // colon after the closing **
{"references-dash", "References — [a](https://a)", true},
{"citations-label", "Citations: https://x/y", true},
{"leading-list-marker", "- Sources: [a](https://a)", true},
{"atx-heading", "## Sources: [a](https://a), [b](https://b)", true}, // ATX heading marker + its trailing space
{"further-reading", "Further reading: https://example.com/deep-dive", true},
{"annotated-multi-source", "Sources: [pcprice.watch](https://a) (tracker), [eBay](https://b) (sold), [bestvaluegpu](https://c) (retail), [resaleprices](https://d) (asking).", true}, // the reported issue-1418 shape
{"backref-plus-links-is-citations", "References: as noted above, [pcprice.watch](https://pcprice.watch/x).", true}, // a back-ref phrase inside a real sources list is still citations
{"empty", "", false},
{"label-but-no-link", "Source: internal analysis, no URL here", false},
{"prose-then-sources", "It sells for ~$2,700. Sources: [a](https://a)", false}, // answer first → not a pure addendum
{"source-led-prose-answer", "Source: According to https://cdc.gov the flu vaccine is 40-60% effective, and the CDC recommends annual vaccination for everyone over six months old.", false}, // a prose answer that merely opens with a "Source:" label
{"mentions-sources-midsentence", "The sources of the leak were never confirmed.", false},
{"link-without-label", "Here is the link you asked for: [a](https://a)", false},
{"bare-domains-out-of-scope", "Sources: pcprice.watch (used ~$200), ebay.com (sold listings)", false}, // bare domains: no scheme or markdown link to key on
{"crisp-number", "42", false},
}
for _, c := range cases {
t.Run(c.name, func(t *testing.T) {
if got := isCitationsOnly(c.in); got != c.want {
t.Errorf("isCitationsOnly(%q) = %v, want %v", c.in, got, c.want)
}
})
}
}
func asst(text string, tools ...llm.ToolCall) llm.Message { func asst(text string, tools ...llm.ToolCall) llm.Message {
m := llm.Message{Role: llm.RoleAssistant} m := llm.Message{Role: llm.RoleAssistant}
if text != "" { if text != "" {
@@ -48,6 +84,25 @@ func asst(text string, tools ...llm.ToolCall) llm.Message {
func TestFinalOutput(t *testing.T) { func TestFinalOutput(t *testing.T) {
cite := []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}} 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
// 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.
sources := "Sources: [pcprice.watch](https://pcprice.watch/x), [ebay](https://www.ebay.com/itm/1)."
answerWithSources := longAnswer + "\n\n" + sources
// A concise (>80, <200 byte) front-loaded answer + a long citations terminal:
// the ratio arm can't be met against the long terminal, so citations mode
// must fall back to the floor.
conciseAnswer := "It sells for about $2,700 used on eBay, typically $2,400 to $2,900 depending on condition and bundle."
longSources := "Sources: [pcprice.watch](https://pcprice.watch/gpu/rtx5090) (tracker), [ebay](https://www.ebay.com/sch/rtx5090) (sold), [newegg](https://newegg.com/rtx5090) (retail), [pcpartpicker](https://pcpartpicker.com/rtx5090) (history)."
// A substantive answer that merely OPENS with "Source:" (not a bare list).
sourceLedAnswer := "Source: https://nvd.nist.gov/vuln/detail/CVE-2024-1234 — this is the authoritative NVD entry for the vulnerability, rated CVSS 9.8 critical."
// A borderline-band (80200 byte) turn that opens like a planning preamble:
// it clears the floor, but the preamble filter still vetoes it (the filter
// applies only in the borderline band; a >=200-byte turn is accepted as-is).
preambleTurn := "Let me look that up across a few different sites and then compile the full comparison for you here."
// 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."
tests := []struct { tests := []struct {
name string name string
@@ -110,6 +165,97 @@ func TestFinalOutput(t *testing.T) {
terminal: "(see above)", terminal: "(see above)",
want: "(see above)", // preamble excluded; falls back to terminal want: "(see above)", // preamble excluded; falls back to terminal
}, },
{
name: "citations-only terminal recovers front-loaded answer and keeps sources",
msgs: []llm.Message{
llm.UserText("q?"),
asst(longAnswer, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(sources),
},
terminal: sources,
want: answerWithSources, // answer recovered, citations appended
},
{
name: "citations-only terminal but only a preamble prior: keeps the sources",
msgs: []llm.Message{
llm.UserText("q?"),
asst("Let me gather the sources.", cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(sources),
},
terminal: sources,
want: sources, // nothing substantive to recover → keep the addendum
},
{
name: "citations already in the recovered answer are not duplicated",
msgs: []llm.Message{
llm.UserText("q?"),
asst(answerWithSources, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(sources),
},
terminal: sources,
want: answerWithSources, // recovered turn already carries the sources
},
{
// #1418 persisted for CONCISE answers: a <200-char front-loaded
// answer must still be recovered against a long citations terminal
// (the ratio arm is skipped in citations mode).
name: "concise front-loaded answer recovered against a long citations terminal",
msgs: []llm.Message{
llm.UserText("q?"),
asst(conciseAnswer, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(longSources),
},
terminal: longSources,
want: conciseAnswer + "\n\n" + longSources,
},
{
// A substantive answer that merely OPENS with "Source:" and cites a
// URL mid-sentence is NOT a citations addendum — return it verbatim,
// never prepend the prior planning turn.
name: "source-led substantive answer is not hijacked by a prior turn",
msgs: []llm.Message{
llm.UserText("what's the authoritative URL?"),
asst("I'll look up the CVE in the NVD database, cross-reference the vendor advisory, and confirm the canonical URL before I answer.", cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(sourceLedAnswer),
},
terminal: sourceLedAnswer,
want: sourceLedAnswer,
},
{
// A borderline-length turn that opens like a preamble is vetoed
// during recovery; the older real answer is recovered instead. (A
// >=200-byte turn would be accepted verbatim — see the next case.)
name: "borderline preamble is skipped; older real answer recovered",
msgs: []llm.Message{
llm.UserText("q?"),
asst(conciseAnswer, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(preambleTurn, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c2", Name: "cite", Content: "ok"}),
asst(sources),
},
terminal: sources,
want: conciseAnswer + "\n\n" + sources,
},
{
// Guards the gadfly regression: a LONG (>=200-byte) front-loaded
// answer that merely opens with a conversational word ("Sure, …")
// must still be recovered — the preamble filter must not veto it.
name: "long answer opening with a conversational word is still recovered",
msgs: []llm.Message{
llm.UserText("q?"),
asst(longConversationalAnswer, cite...),
llm.ToolResultsMessage(llm.ToolResult{ID: "c1", Name: "cite", Content: "ok"}),
asst(sources),
},
terminal: sources,
want: longConversationalAnswer + "\n\n" + sources,
},
} }
for _, tc := range tests { for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
@@ -179,3 +325,36 @@ func TestRun_HealthyTerminalUnchanged(t *testing.T) {
t.Errorf("Output = %q, want terminal answer unchanged", res.Output) t.Errorf("Output = %q, want terminal answer unchanged", res.Output)
} }
} }
// 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
// the recovered answer with the citations appended (not the bare sources list),
// with no extra model call.
func TestRun_RecoversFrontLoadedAnswerWithCitations(t *testing.T) {
longAnswer := strings.TrimSpace(strings.Repeat("Free group calls are capped at sixty minutes. ", 6))
sources := "Sources: [docs](https://example.com/docs), [pricing](https://example.com/pricing)."
fp := fake.New("fp")
fp.Enqueue("test-model",
fake.ReplyWith(llm.Response{
Parts: []llm.Part{llm.Text(longAnswer)},
ToolCalls: []llm.ToolCall{{ID: "c1", Name: "cite", Arguments: json.RawMessage(`{}`)}},
FinishReason: llm.FinishToolCalls,
Usage: llm.Usage{InputTokens: 10, OutputTokens: 5},
}),
fake.Reply(sources),
)
a := New(newModel(t, fp), "sys", WithToolbox(citeToolbox(t)))
res, err := a.Run(context.Background(), "is there a meet time limit?")
if err != nil {
t.Fatalf("Run: %v", err)
}
want := longAnswer + "\n\n" + sources
if res.Output != want {
t.Errorf("Output = %q, want recovered answer + citations %q", res.Output, want)
}
if n := len(fp.Calls()); n != 2 {
t.Errorf("model calls = %d, want 2 (no extra nudge turn)", n)
}
}
+9
View File
@@ -52,3 +52,12 @@ contract is unchanged for callers that don't set them. `provider/llamaswap`
forwards them to sd-server as `steps`/`cfg_scale`/`negative_prompt`/`sample_method`/ forwards them to sd-server as `steps`/`cfg_scale`/`negative_prompt`/`sample_method`/
`seed` (omitempty). This realizes the "seeds/steps … additive fields" note above; `seed` (omitempty). This realizes the "seeds/steps … additive fields" note above;
img2img/masks/streaming remain deferred. img2img/masks/streaming remain deferred.
## Update — A1111 txt2img endpoint (seed support)
`provider/llamaswap` now POSTs to sd-server's **`/sdapi/v1/txt2img`** (A1111)
instead of the OpenAI `/v1/images/generations`. That OpenAI endpoint **ignores
`seed`** on the stable-diffusion.cpp build we run — every render of a prompt is
byte-identical, so a batch of N collapses to one image. `/sdapi/v1/txt2img`
honours `seed`, restoring real per-render variety. llama-swap still routes by
the `model` field in the body; `Size` is split into `width`/`height`.
+35 -9
View File
@@ -5,10 +5,16 @@
// already satisfies the target's llm.Capabilities. Images that do not fit // already satisfies the target's llm.Capabilities. Images that do not fit
// are decoded, downscaled (never upscaled), and re-encoded into an allowed // are decoded, downscaled (never upscaled), and re-encoded into an allowed
// format and byte budget. Anything that cannot honestly be made to fit — // format and byte budget. Anything that cannot honestly be made to fit —
// undecodable formats, impossible byte budgets, too many images, images for // undecodable formats, impossible byte budgets, images for a text-only
// a text-only target — fails with an error wrapping llm.ErrUnsupported so a // target — fails with an error wrapping llm.ErrUnsupported so a failover
// failover chain can advance to a more capable target without a health // chain can advance to a more capable target without a health penalty.
// penalty. //
// Over-count is the exception: a request carrying more images than
// MaxImagesPerReq does NOT fail — the oldest images are replaced with a short
// text placeholder and the most-recent MaxImagesPerReq are kept, because a hard
// refuse exhausts a chain whose targets share the same cap (e.g. an agent loop
// accumulating a preview image per iteration). MaxImagesPerReq remains the
// per-model knob (0 = no image support).
// //
// Why a separate package: every provider would otherwise duplicate the same // Why a separate package: every provider would otherwise duplicate the same
// decode/scale/encode pipeline. Providers keep only a cheap capability // decode/scale/encode pipeline. Providers keep only a cheap capability
@@ -52,15 +58,21 @@ func Normalize(req llm.Request, caps llm.Capabilities) (llm.Request, error) {
if !caps.SupportsImages() { if !caps.SupportsImages() {
return llm.Request{}, fmt.Errorf("media: %w: target does not accept image input (request carries %d image(s))", llm.ErrUnsupported, total) return llm.Request{}, fmt.Errorf("media: %w: target does not accept image input (request carries %d image(s))", llm.ErrUnsupported, total)
} }
// Why error instead of dropping the overflow: silently removing an image // Over-cap images are elided in the same copy-on-write pass below: the
// changes the question the caller asked; the honest move is to refuse and // OLDEST excess are replaced with a placeholder and the most-recent
// let a chain try a roomier target. // MaxImagesPerReq kept (see the package doc for why we elide rather than
// refuse). toElide is how many of the first images, front-to-back, to drop.
toElide := 0
if total > caps.MaxImagesPerReq { if total > caps.MaxImagesPerReq {
return llm.Request{}, fmt.Errorf("media: %w: request carries %d images, target allows at most %d per request", llm.ErrUnsupported, total, caps.MaxImagesPerReq) toElide = total - caps.MaxImagesPerReq
} }
// Single copy-on-write pass: for each image, the first toElide become a text
// placeholder; the rest are size-normalized against caps. The Messages slice
// and an affected message's Parts slice are copied at most once.
out := req out := req
copiedMessages := false copiedMessages := false
seen := 0
for mi := range req.Messages { for mi := range req.Messages {
copiedParts := false copiedParts := false
for pi, part := range req.Messages[mi].Parts { for pi, part := range req.Messages[mi].Parts {
@@ -68,6 +80,12 @@ func Normalize(req llm.Request, caps llm.Capabilities) (llm.Request, error) {
if !ok { if !ok {
continue continue
} }
seen++
var replacement llm.Part
if seen <= toElide {
replacement = llm.Text(imageOverflowPlaceholder)
} else {
norm, changed, err := normalizeImage(ip, caps) norm, changed, err := normalizeImage(ip, caps)
if err != nil { if err != nil {
return llm.Request{}, fmt.Errorf("media: message %d, part %d: %w", mi, pi, err) return llm.Request{}, fmt.Errorf("media: message %d, part %d: %w", mi, pi, err)
@@ -75,6 +93,9 @@ func Normalize(req llm.Request, caps llm.Capabilities) (llm.Request, error) {
if !changed { if !changed {
continue continue
} }
replacement = norm
}
if !copiedMessages { if !copiedMessages {
out.Messages = make([]llm.Message, len(req.Messages)) out.Messages = make([]llm.Message, len(req.Messages))
copy(out.Messages, req.Messages) copy(out.Messages, req.Messages)
@@ -86,12 +107,17 @@ func Normalize(req llm.Request, caps llm.Capabilities) (llm.Request, error) {
out.Messages[mi].Parts = parts out.Messages[mi].Parts = parts
copiedParts = true copiedParts = true
} }
out.Messages[mi].Parts[pi] = norm out.Messages[mi].Parts[pi] = replacement
} }
} }
return out, nil return out, nil
} }
// imageOverflowPlaceholder replaces an image elided to fit a target's
// per-request image cap. It keeps the message turn intact and tells the model
// an earlier image was omitted rather than silently changing the conversation.
const imageOverflowPlaceholder = "[earlier image omitted to fit this model's per-request image limit]"
// Info reports an image part's sniffed format ("jpeg", "png", "gif", or // Info reports an image part's sniffed format ("jpeg", "png", "gif", or
// "webp") and pixel dimensions. It is a cheap metadata read — the pixels are // "webp") and pixel dimensions. It is a cheap metadata read — the pixels are
// never decoded. webp is recognized by signature but not decodable with the // never decoded. webp is recognized by signature but not decodable with the
+39 -9
View File
@@ -149,18 +149,48 @@ func TestNormalizeImagesUnsupported(t *testing.T) {
} }
} }
func TestNormalizeTooManyImages(t *testing.T) { func TestNormalizeOverCount(t *testing.T) {
img := llm.Image("image/png", encPNG(t, gradient(4, 4))) // 3 distinguishable images across 2 messages; cap = 2. Over-count no longer
// errors — the OLDEST image is replaced with a placeholder and the most-recent
// two (the relevant ones in an iterative run) are kept, in order.
a := llm.Image("image/png", encPNG(t, gradient(2, 2))).(llm.ImagePart)
b := llm.Image("image/png", encPNG(t, gradient(4, 4))).(llm.ImagePart)
c := llm.Image("image/png", encPNG(t, gradient(8, 8))).(llm.ImagePart)
req := llm.Request{Messages: []llm.Message{ req := llm.Request{Messages: []llm.Message{
llm.UserParts(img, img), llm.UserParts(a, b),
llm.UserParts(img), llm.UserParts(c),
}} }}
_, err := Normalize(req, llm.Capabilities{MaxImagesPerReq: 2}) caps := llm.Capabilities{MaxImagesPerReq: 2, MaxImageDimension: 64, MaxImageBytes: 1 << 20, AllowedImageMIME: []string{"image/png"}}
if !errors.Is(err, llm.ErrUnsupported) { out, err := Normalize(req, caps)
t.Fatalf("err = %v, want ErrUnsupported", err) if err != nil {
t.Fatalf("over-count should not error: %v", err)
} }
if !strings.Contains(err.Error(), "3 images") || !strings.Contains(err.Error(), "at most 2") { var imgs []llm.ImagePart
t.Errorf("err message %q lacks the counts", err) placeholders := 0
for _, m := range out.Messages {
for _, p := range m.Parts {
switch v := p.(type) {
case llm.ImagePart:
imgs = append(imgs, v)
case llm.TextPart:
if v.Text == imageOverflowPlaceholder {
placeholders++
}
}
}
}
// The exact survivors are the most-recent two, in order: b then c (a elided).
if len(imgs) != 2 || !bytes.Equal(imgs[0].Data, b.Data) || !bytes.Equal(imgs[1].Data, c.Data) {
t.Fatalf("kept %d images; want exactly [b, c] (the most-recent two)", len(imgs))
}
if placeholders != 1 {
t.Errorf("placeholders = %d, want 1 for the elided oldest image", placeholders)
}
// Input request untouched (copy-on-write): the first part is still image a,
// not a placeholder — a len check alone wouldn't catch in-place substitution.
first, ok := req.Messages[0].Parts[0].(llm.ImagePart)
if !ok || !bytes.Equal(first.Data, a.Data) {
t.Errorf("input request was mutated; first part = %+v", req.Messages[0].Parts[0])
} }
} }
+59 -43
View File
@@ -5,6 +5,7 @@ import (
"encoding/base64" "encoding/base64"
"fmt" "fmt"
"net/http" "net/http"
"strconv"
"strings" "strings"
"gitea.stevedudenhoeffer.com/steve/majordomo/imagegen" "gitea.stevedudenhoeffer.com/steve/majordomo/imagegen"
@@ -27,34 +28,32 @@ type imageModel struct {
id string id string
} }
// imageRequest is the OpenAI /v1/images/generations request shape, plus the // txt2imgRequest is the stable-diffusion.cpp sd-server A1111 request shape
// stable-diffusion.cpp extras llama-swap forwards to sd-server. We always // (POST /sdapi/v1/txt2img). We use this endpoint rather than the OpenAI
// request b64_json so the bytes come back inline (no second fetch). The // /v1/images/generations one because that endpoint IGNORES `seed` on this
// optional fields are pointers/omitempty so an unset value is omitted entirely // sd-server build — every render of a given prompt comes back byte-identical,
// and sd-server falls back to the model's own default (a field name a given // so a batch of N collapses to one image. /sdapi/v1/txt2img honours `seed`,
// sd-server build doesn't recognize is simply ignored — harmless). // giving real variety. llama-swap still routes by the `model` field in the
type imageRequest struct { // body. Optional fields are pointers/omitempty so an unset value falls back to
// the model's baked default (the per-model --steps/--cfg-scale/etc. flags).
type txt2imgRequest struct {
Model string `json:"model"` Model string `json:"model"`
Prompt string `json:"prompt"` Prompt string `json:"prompt"`
N int `json:"n,omitempty"` NegativePrompt string `json:"negative_prompt,omitempty"`
Size string `json:"size,omitempty"` Seed *int64 `json:"seed,omitempty"`
ResponseFormat string `json:"response_format"`
Steps *int `json:"steps,omitempty"` Steps *int `json:"steps,omitempty"`
CFGScale *float64 `json:"cfg_scale,omitempty"` CFGScale *float64 `json:"cfg_scale,omitempty"`
NegativePrompt string `json:"negative_prompt,omitempty"` Width *int `json:"width,omitempty"`
Height *int `json:"height,omitempty"`
SampleMethod string `json:"sample_method,omitempty"` SampleMethod string `json:"sample_method,omitempty"`
Seed *int64 `json:"seed,omitempty"` BatchCount int `json:"batch_count,omitempty"`
} }
type imageResponse struct { type txt2imgResponse struct {
Created int64 `json:"created"` Images []string `json:"images"`
Data []struct {
B64JSON string `json:"b64_json"`
URL string `json:"url"`
} `json:"data"`
} }
// Generate implements imagegen.Model via POST {base}/v1/images/generations. // Generate implements imagegen.Model via POST {base}/sdapi/v1/txt2img.
func (m *imageModel) Generate(ctx context.Context, req imagegen.Request, opts ...imagegen.Option) (*imagegen.Result, error) { func (m *imageModel) Generate(ctx context.Context, req imagegen.Request, opts ...imagegen.Option) (*imagegen.Result, error) {
req = req.Apply(opts...) req = req.Apply(opts...)
if strings.TrimSpace(req.Prompt) == "" { if strings.TrimSpace(req.Prompt) == "" {
@@ -64,37 +63,35 @@ func (m *imageModel) Generate(ctx context.Context, req imagegen.Request, opts ..
return nil, fmt.Errorf("%w: image count N must be >= 0, got %d", llm.ErrUnsupported, req.N) return nil, fmt.Errorf("%w: image count N must be >= 0, got %d", llm.ErrUnsupported, req.N)
} }
wire := imageRequest{ width, height, err := parseSize(req.Size)
Model: m.id, if err != nil {
Prompt: req.Prompt, return nil, fmt.Errorf("%w: %v", llm.ErrUnsupported, err)
N: req.N,
Size: req.Size,
ResponseFormat: "b64_json",
Steps: req.Steps,
CFGScale: req.CFGScale,
NegativePrompt: req.NegativePrompt,
SampleMethod: req.Sampler,
Seed: req.Seed,
} }
var resp imageResponse wire := txt2imgRequest{
if err := m.p.doJSON(ctx, http.MethodPost, "/v1/images/generations", m.id, &wire, &resp); err != nil { Model: m.id,
Prompt: req.Prompt,
NegativePrompt: req.NegativePrompt,
Seed: req.Seed,
Steps: req.Steps,
CFGScale: req.CFGScale,
Width: width,
Height: height,
SampleMethod: req.Sampler,
BatchCount: req.N,
}
var resp txt2imgResponse
if err := m.p.doJSON(ctx, http.MethodPost, "/sdapi/v1/txt2img", m.id, &wire, &resp); err != nil {
return nil, err return nil, err
} }
out := &imagegen.Result{Raw: &resp} out := &imagegen.Result{Raw: &resp}
for i, d := range resp.Data { for i, b64 := range resp.Images {
if d.B64JSON == "" { if b64 == "" {
// Why error rather than skip: a url-only entry means the backend continue
// ignored response_format; we don't fetch remote content (mirrors
// llm.ImagePart's bytes-only contract), so surface it.
return nil, &llm.APIError{
Provider: m.p.name,
Model: m.id,
Message: fmt.Sprintf("image %d returned no inline b64_json data", i),
} }
} raw, err := base64.StdEncoding.DecodeString(b64)
raw, err := base64.StdEncoding.DecodeString(d.B64JSON)
if err != nil { if err != nil {
return nil, fmt.Errorf("llama-swap: decode image %d: %w", i, err) return nil, fmt.Errorf("llama-swap: decode image %d: %w", i, err)
} }
@@ -110,6 +107,25 @@ func (m *imageModel) Generate(ctx context.Context, req imagegen.Request, opts ..
return out, nil return out, nil
} }
// parseSize splits a "WxH" string into width/height pointers. "" yields
// (nil, nil) so the model's own default resolution applies.
func parseSize(size string) (*int, *int, error) {
size = strings.TrimSpace(size)
if size == "" {
return nil, nil, nil
}
parts := strings.SplitN(strings.ToLower(size), "x", 2)
if len(parts) != 2 {
return nil, nil, fmt.Errorf("invalid size %q (want WxH)", size)
}
w, err1 := strconv.Atoi(strings.TrimSpace(parts[0]))
h, err2 := strconv.Atoi(strings.TrimSpace(parts[1]))
if err1 != nil || err2 != nil || w <= 0 || h <= 0 {
return nil, nil, fmt.Errorf("invalid size %q (want WxH)", size)
}
return &w, &h, nil
}
// sniffImageMIME identifies the image format from its leading bytes, defaulting // sniffImageMIME identifies the image format from its leading bytes, defaulting
// to image/png (stable-diffusion.cpp emits PNG) when detection is inconclusive. // to image/png (stable-diffusion.cpp emits PNG) when detection is inconclusive.
func sniffImageMIME(data []byte) string { func sniffImageMIME(data []byte) string {
+6 -9
View File
@@ -166,11 +166,11 @@ func TestRunningRaw(t *testing.T) {
func TestImageGenerate(t *testing.T) { func TestImageGenerate(t *testing.T) {
var gotBody map[string]any var gotBody map[string]any
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path != "/v1/images/generations" { if r.URL.Path != "/sdapi/v1/txt2img" {
t.Errorf("path = %q", r.URL.Path) t.Errorf("path = %q", r.URL.Path)
} }
_ = json.NewDecoder(r.Body).Decode(&gotBody) _ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = w.Write([]byte(`{"created":1,"data":[{"b64_json":"` + onePixelPNG + `"}]}`)) _, _ = w.Write([]byte(`{"images":["` + onePixelPNG + `"]}`))
})) }))
defer srv.Close() defer srv.Close()
@@ -192,12 +192,9 @@ func TestImageGenerate(t *testing.T) {
if len(res.Images[0].Data) == 0 { if len(res.Images[0].Data) == 0 {
t.Error("decoded image has no bytes") t.Error("decoded image has no bytes")
} }
// response_format must be forced to b64_json, and options applied. // Size is split into width/height ints for the A1111 endpoint.
if gotBody["response_format"] != "b64_json" { if gotBody["width"] != float64(512) || gotBody["height"] != float64(512) {
t.Errorf("response_format = %v, want b64_json", gotBody["response_format"]) t.Errorf("width/height = %v/%v, want 512/512", gotBody["width"], gotBody["height"])
}
if gotBody["size"] != "512x512" {
t.Errorf("size = %v, want 512x512", gotBody["size"])
} }
} }
@@ -205,7 +202,7 @@ func TestImageGenerateSettings(t *testing.T) {
var gotBody map[string]any var gotBody map[string]any
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_ = json.NewDecoder(r.Body).Decode(&gotBody) _ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = w.Write([]byte(`{"created":1,"data":[{"b64_json":"` + onePixelPNG + `"}]}`)) _, _ = w.Write([]byte(`{"images":["` + onePixelPNG + `"]}`))
})) }))
defer srv.Close() defer srv.Close()