3 Commits

Author SHA1 Message Date
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 140 additions and 162 deletions
+50 -22
View File
@@ -1,8 +1,12 @@
# Gadfly adversarial review — subscribes to steve/gadfly's reusable workflow and
# INHERITS its default swarm. This stub holds only the triggers, the actor gate,
# secret forwarding, and the allow-list; the swarm config (models, lenses,
# concurrency, timeouts) lives centrally in gadfly's review-reusable.yml so it is
# tuned in ONE place. Advisory only — never blocks a merge.
# Gadfly — agentic adversarial PR reviewer (https://gitea.stevedudenhoeffer.com/steve/gadfly).
#
# Runs the published Gadfly image (pinned to an immutable :sha- tag — act_runner
# caches :latest, and this build is what carries foreman provider-type support)
# as a specialist swarm and posts
# ONE consolidated review comment as gitea-actions. Advisory only — never blocks a
# merge. This reviews majordomo PRs with 6 ollama-cloud models (3-lens suite).
# Gadfly is a simple system — findings are advisory; always double-check before
# acting.
name: Adversarial Review (Gadfly)
@@ -29,26 +33,50 @@ concurrency:
jobs:
review:
# Security: only trusted users may trigger a secret-bearing run via a PR
# comment (pull_request + workflow_dispatch are already trusted). Mirrors the
# allowed_users input below (the in-container belt-and-suspenders check) — both
# lists must stay in sync; a workflow if: can't read a workflow_call input.
# comment (pull_request + workflow_dispatch are already trusted). Mirrors
# GADFLY_ALLOWED_USERS, the in-container belt-and-suspenders check.
if: >-
github.event_name != 'issue_comment'
|| (github.event.issue.pull_request
&& (github.actor == 'steve'
|| github.actor == 'fizi'
|| github.actor == 'dazed'))
# 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
# here automatically; the tradeoff vs a full sha pin is that v1 is mutable.
uses: steve/gadfly/.gitea/workflows/review-reusable.yml@7bc3c982fa7b72367034c673f7812bf05e9c503e
# Least privilege: forward only the review secrets (not `secrets: inherit`,
# which would expose every repo secret). GITEA_TOKEN is the automatic token.
secrets:
OLLAMA_CLOUD_API_KEY: ${{ secrets.OLLAMA_CLOUD_API_KEY }}
CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }}
GADFLY_FINDINGS_URL: ${{ secrets.GADFLY_FINDINGS_URL }}
GADFLY_FINDINGS_TOKEN: ${{ secrets.GADFLY_FINDINGS_TOKEN }}
with:
# Consumer-specific allow-list; everything else is inherited.
allowed_users: "steve,fizi,dazed"
runs-on: ubuntu-latest
# Fleet: 6 ollama-cloud models (lens fan-out), no local Macs. (Trimmed the
# weakest reviewers by grade — m5/qwen3.6, gemma4, gpt-oss, kimi-k2.7 — plus
# the earlier M1 drop.) Plenty of headroom for the cloud lanes.
timeout-minutes: 45
steps:
- uses: docker://gitea.stevedudenhoeffer.com/steve/gadfly:sha-d7f364d
env:
GITEA_API: ${{ github.server_url }}/api/v1/repos/${{ github.repository }}
GITEA_TOKEN: ${{ secrets.GITEA_TOKEN }}
OLLAMA_CLOUD_API_KEY: ${{ secrets.OLLAMA_CLOUD_API_KEY }}
# Cloud-only fleet (no local Macs). Cloud concurrency lives in the
# LENSES: models run a few at a time (ollama-cloud=3) with their 3 lenses
# concurrent (LENS ollama-cloud=3) so comments land sooner.
GADFLY_MODELS: "minimax-m3:cloud,glm-5.2:cloud,glm-5.1:cloud,deepseek-v4-pro:cloud,nemotron-3-super:cloud,qwen3-coder:480b-cloud"
GADFLY_PROVIDER_CONCURRENCY: "ollama-cloud=3"
GADFLY_PROVIDER_LENS_CONCURRENCY: "ollama-cloud=3"
# Default => the 3-lens suite (security, correctness, error-handling).
# Set the repo var GADFLY_SPECIALISTS to override (csv / "all" / "auto").
GADFLY_SPECIALISTS: ${{ vars.GADFLY_SPECIALISTS || 'security,correctness,error-handling' }}
# Per-lens deadline + bounded steps to keep each reviewer's run sane.
GADFLY_TIMEOUT_SECS: "600"
GADFLY_MAX_STEPS: "14"
# Allow-list for the comment trigger (mirrors the job-level if: guard).
GADFLY_ALLOWED_USERS: "steve,fizi,dazed"
# --- findings telemetry: POST runs + findings to the gadfly-reports store ---
# Advisory & off unless GADFLY_FINDINGS_URL is set; failures only log to
# stderr and never affect the review. GADFLY_REPO / GADFLY_PR are derived
# in-container; the URL + token are user-scope secrets.
GADFLY_FINDINGS_URL: ${{ secrets.GADFLY_FINDINGS_URL }}
GADFLY_FINDINGS_TOKEN: ${{ secrets.GADFLY_FINDINGS_TOKEN }}
# --- event context (leave as-is) ---
EVENT_NAME: ${{ github.event_name }}
PR: ${{ github.event.pull_request.number || github.event.issue.number || github.event.inputs.pr_number }}
PR_BRANCH: ${{ github.head_ref }}
IS_DRAFT: ${{ github.event.pull_request.draft }}
COMMENT_BODY: ${{ github.event.comment.body }}
COMMENT_ID: ${{ github.event.comment.id }}
ACTOR: ${{ github.actor }}
+3 -5
View File
@@ -142,11 +142,9 @@ Ship work through PRs and let Gadfly review it before merge:
- **Push to a PR, never straight to `main`.** Branch, push, open a PR.
`.gitea/workflows/adversarial-review.yml` runs Gadfly (the standalone
agentic adversarial reviewer) by subscribing to gadfly's reusable workflow
and inheriting its default swarm — 3 cloud models + the Claude Code engine
(sonnet/opus/opus:max), each running the 5-lens suite (security, correctness,
maintainability, performance, error-handling). The swarm is tuned centrally
in gadfly, not here. Advisory only; it never blocks the merge.
agentic adversarial reviewer) — a fleet of 6 ollama-cloud models, each
running the 3-lens suite (security, correctness, error-handling). Advisory
only; it never blocks the merge.
- **Wait for Gadfly to finish, then read its output.** Don't merge while the
review is still running. Each model posts one consolidated comment; weigh
every finding on its merits and fix the real ones (Gadfly is a simple
-10
View File
@@ -42,13 +42,3 @@ asked for "a new ai image interface as opposed to llm".
callers (additive fields/options).
- No health/failover for image models yet; if needed it can be added as a
separate chain type rather than retrofitting the chat chain.
## Update — optional per-request settings
`Request` gained additive optional overrides — `Steps *int`, `CFGScale *float64`,
`NegativePrompt string`, `Sampler string`, `Seed *int64` — with mirror options
(`WithSteps`, …). nil/"" means "leave the backend's per-model default", so the v1
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`/
`seed` (omitempty). This realizes the "seeds/steps … additive fields" note above;
img2img/masks/streaming remain deferred.
-38
View File
@@ -38,29 +38,6 @@ type Request struct {
// Size is the requested resolution, e.g. "512x512" or "1024x1024";
// "" = provider default.
Size string
// The fields below are optional per-request overrides. Their zero value
// (nil pointer or empty string) means "leave the backend's own default" —
// for stable-diffusion.cpp that is the per-model default baked into the
// llama-swap launch flags. A caller overrides only what it explicitly sets.
// Steps is the number of diffusion steps; nil = backend default.
Steps *int
// CFGScale is the classifier-free-guidance scale; nil = backend default.
// Architecture-sensitive (SDXL likes ~7, Flux wants 1), so prefer leaving
// it nil unless the caller knows the target model.
CFGScale *float64
// NegativePrompt steers generation away from concepts; "" = none.
NegativePrompt string
// Sampler selects the sampling method (e.g. "euler", "euler_a");
// "" = backend default.
Sampler string
// Seed fixes the RNG seed for reproducible output; nil = random.
Seed *int64
}
// Result is the canonical image-generation result.
@@ -83,21 +60,6 @@ func WithN(n int) Option { return func(r *Request) { r.N = n } }
// WithSize sets the requested resolution (e.g. "1024x1024").
func WithSize(size string) Option { return func(r *Request) { r.Size = size } }
// WithSteps overrides the number of diffusion steps.
func WithSteps(n int) Option { return func(r *Request) { r.Steps = &n } }
// WithCFGScale overrides the classifier-free-guidance scale.
func WithCFGScale(s float64) Option { return func(r *Request) { r.CFGScale = &s } }
// WithNegativePrompt sets a negative prompt.
func WithNegativePrompt(s string) Option { return func(r *Request) { r.NegativePrompt = s } }
// WithSampler overrides the sampling method (e.g. "euler", "euler_a").
func WithSampler(s string) Option { return func(r *Request) { r.Sampler = s } }
// WithSeed fixes the RNG seed for reproducible output.
func WithSeed(seed int64) Option { return func(r *Request) { r.Seed = &seed } }
// Apply returns a copy of the request with all options applied. Providers call
// this once at the top of Generate.
func (r Request) Apply(opts ...Option) Request {
+41 -15
View File
@@ -5,10 +5,16 @@
// already satisfies the target's llm.Capabilities. Images that do not fit
// are decoded, downscaled (never upscaled), and re-encoded into an allowed
// format and byte budget. Anything that cannot honestly be made to fit —
// undecodable formats, impossible byte budgets, too many images, images for
// a text-only target — fails with an error wrapping llm.ErrUnsupported so a
// failover chain can advance to a more capable target without a health
// penalty.
// undecodable formats, impossible byte budgets, images for a text-only
// target — fails with an error wrapping llm.ErrUnsupported so a failover
// chain can advance to a more capable target without a health 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
// 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() {
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
// changes the question the caller asked; the honest move is to refuse and
// let a chain try a roomier target.
// Over-cap images are elided in the same copy-on-write pass below: the
// OLDEST excess are replaced with a placeholder and the most-recent
// 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 {
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
copiedMessages := false
seen := 0
for mi := range req.Messages {
copiedParts := false
for pi, part := range req.Messages[mi].Parts {
@@ -68,13 +80,22 @@ func Normalize(req llm.Request, caps llm.Capabilities) (llm.Request, error) {
if !ok {
continue
}
norm, changed, err := normalizeImage(ip, caps)
if err != nil {
return llm.Request{}, fmt.Errorf("media: message %d, part %d: %w", mi, pi, err)
}
if !changed {
continue
seen++
var replacement llm.Part
if seen <= toElide {
replacement = llm.Text(imageOverflowPlaceholder)
} else {
norm, changed, err := normalizeImage(ip, caps)
if err != nil {
return llm.Request{}, fmt.Errorf("media: message %d, part %d: %w", mi, pi, err)
}
if !changed {
continue
}
replacement = norm
}
if !copiedMessages {
out.Messages = make([]llm.Message, len(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
copiedParts = true
}
out.Messages[mi].Parts[pi] = norm
out.Messages[mi].Parts[pi] = replacement
}
}
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
// "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
+39 -9
View File
@@ -149,18 +149,48 @@ func TestNormalizeImagesUnsupported(t *testing.T) {
}
}
func TestNormalizeTooManyImages(t *testing.T) {
img := llm.Image("image/png", encPNG(t, gradient(4, 4)))
func TestNormalizeOverCount(t *testing.T) {
// 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{
llm.UserParts(img, img),
llm.UserParts(img),
llm.UserParts(a, b),
llm.UserParts(c),
}}
_, err := Normalize(req, llm.Capabilities{MaxImagesPerReq: 2})
if !errors.Is(err, llm.ErrUnsupported) {
t.Fatalf("err = %v, want ErrUnsupported", err)
caps := llm.Capabilities{MaxImagesPerReq: 2, MaxImageDimension: 64, MaxImageBytes: 1 << 20, AllowedImageMIME: []string{"image/png"}}
out, err := Normalize(req, caps)
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") {
t.Errorf("err message %q lacks the counts", err)
var imgs []llm.ImagePart
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])
}
}
+7 -21
View File
@@ -27,23 +27,14 @@ type imageModel struct {
id string
}
// imageRequest is the OpenAI /v1/images/generations request shape, plus the
// stable-diffusion.cpp extras llama-swap forwards to sd-server. We always
// request b64_json so the bytes come back inline (no second fetch). The
// optional fields are pointers/omitempty so an unset value is omitted entirely
// and sd-server falls back to the model's own default (a field name a given
// sd-server build doesn't recognize is simply ignored — harmless).
// imageRequest is the OpenAI /v1/images/generations request shape. We always
// request b64_json so the bytes come back inline (no second fetch).
type imageRequest struct {
Model string `json:"model"`
Prompt string `json:"prompt"`
N int `json:"n,omitempty"`
Size string `json:"size,omitempty"`
ResponseFormat string `json:"response_format"`
Steps *int `json:"steps,omitempty"`
CFGScale *float64 `json:"cfg_scale,omitempty"`
NegativePrompt string `json:"negative_prompt,omitempty"`
SampleMethod string `json:"sample_method,omitempty"`
Seed *int64 `json:"seed,omitempty"`
Model string `json:"model"`
Prompt string `json:"prompt"`
N int `json:"n,omitempty"`
Size string `json:"size,omitempty"`
ResponseFormat string `json:"response_format"`
}
type imageResponse struct {
@@ -70,11 +61,6 @@ func (m *imageModel) Generate(ctx context.Context, req imagegen.Request, opts ..
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
-42
View File
@@ -201,48 +201,6 @@ func TestImageGenerate(t *testing.T) {
}
}
func TestImageGenerateSettings(t *testing.T) {
var gotBody map[string]any
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_ = json.NewDecoder(r.Body).Decode(&gotBody)
_, _ = w.Write([]byte(`{"created":1,"data":[{"b64_json":"` + onePixelPNG + `"}]}`))
}))
defer srv.Close()
p := New(WithBaseURL(srv.URL), WithHTTPClient(srv.Client()))
im, _ := p.ImageModel("sd")
// Unset overrides must be omitted entirely so sd-server keeps its own
// per-model defaults.
if _, err := im.Generate(context.Background(), imagegen.Request{Prompt: "x"}); err != nil {
t.Fatalf("Generate: %v", err)
}
for _, k := range []string{"steps", "cfg_scale", "negative_prompt", "sample_method", "seed"} {
if v, ok := gotBody[k]; ok {
t.Errorf("unset request sent %q = %v, want omitted", k, v)
}
}
// Set overrides are forwarded with the sd-server-friendly field names.
gotBody = nil
_, err := im.Generate(context.Background(), imagegen.Request{Prompt: "x"},
imagegen.WithSteps(8),
imagegen.WithCFGScale(3.5),
imagegen.WithNegativePrompt("blurry"),
imagegen.WithSampler("euler"),
imagegen.WithSeed(42),
)
if err != nil {
t.Fatalf("Generate: %v", err)
}
want := map[string]any{"steps": float64(8), "cfg_scale": 3.5, "negative_prompt": "blurry", "sample_method": "euler", "seed": float64(42)}
for k, w := range want {
if gotBody[k] != w {
t.Errorf("%s = %v, want %v", k, gotBody[k], w)
}
}
}
func TestImageGenerateEmptyPrompt(t *testing.T) {
p := New(WithBaseURL("http://example.invalid"))
im, _ := p.ImageModel("sd")