FLUX.1 Kontext and Qwen-Image-Edit are a different kind of edit from img2img and reach sd-server by a different path, and nothing in imagegen could express it — EditRequest only had Init, which is noised and denoised back under the prompt.
Why it matters
Measured against FLUX.1-Kontext on the netherstorm host 2026-07-30. Synthetic scene: red rectangle left, blue rectangle right, flat background. Prompt: "change the blue rectangle on the right to bright green, keep everything else exactly the same".
region
source
via init_images (only path that existed)
via extra_images (this PR)
right rect (asked to change)
(60,60,200)
(47,82,228) — still blue, instruction ignored
(70,254,4) — green ✓
left rect (must not change)
(200,60,60)
(229,43,50) — drifted
(204,57,57) ✓
background
(150,200,240)
(154,211,229) — drifted
(151,202,247) ✓
No mask, no strength, no compositing: the model is handed the picture as conditioning and the prompt as an instruction about it.
Shape
EditRequest.RefImages selects the path. When set, Init/Mask/Strength are ignored rather than rejected — a caller handing the same request to whichever model is configured should get the better result on a Kontext-class model, not an error, and both fields describe a pipeline this model does not run.
The provider posts /sdapi/v1/txt2img with extra_images. sd-server reads that field on both routes into gen_params.ref_images — the same place the CLI's -r/--ref-image lands — and there is no init latent to denoise, so sending one would only add noise to a pipeline that does not want any.
An all-empty reference set is refused: it would otherwise degrade into a plain txt2img and render the prompt from scratch, which is not the request.
Tests
TestImageEditByReferenceUsesTxt2ImgExtraImages pins the endpoint and the field, and asserts init_images/denoising_strength/mask are absent — routing a reference edit down the img2img path looks like a working call and silently produces the wrong picture, so the wire shape is the thing worth pinning.
FLUX.1 Kontext and Qwen-Image-Edit are a different kind of edit from img2img and reach sd-server by a different path, and nothing in `imagegen` could express it — `EditRequest` only had `Init`, which is noised and denoised back under the prompt.
## Why it matters
Measured against FLUX.1-Kontext on the netherstorm host 2026-07-30. Synthetic scene: red rectangle left, blue rectangle right, flat background. Prompt: *"change the blue rectangle on the right to bright green, keep everything else exactly the same"*.
| region | source | via `init_images` (only path that existed) | via `extra_images` (this PR) |
|---|---|---|---|
| right rect (asked to change) | `(60,60,200)` | `(47,82,228)` — **still blue, instruction ignored** | `(70,254,4)` — green ✓ |
| left rect (must not change) | `(200,60,60)` | `(229,43,50)` — drifted | `(204,57,57)` ✓ |
| background | `(150,200,240)` | `(154,211,229)` — drifted | `(151,202,247)` ✓ |
No mask, no strength, no compositing: the model is handed the picture as conditioning and the prompt as an instruction about it.
## Shape
`EditRequest.RefImages` selects the path. When set, `Init`/`Mask`/`Strength` are **ignored rather than rejected** — a caller handing the same request to whichever model is configured should get the better result on a Kontext-class model, not an error, and both fields describe a pipeline this model does not run.
The provider posts `/sdapi/v1/txt2img` with `extra_images`. sd-server reads that field on **both** routes into `gen_params.ref_images` — the same place the CLI's `-r/--ref-image` lands — and there is no init latent to denoise, so sending one would only add noise to a pipeline that does not want any.
An all-empty reference set is refused: it would otherwise degrade into a plain txt2img and render the prompt from scratch, which is not the request.
## Tests
`TestImageEditByReferenceUsesTxt2ImgExtraImages` pins the endpoint and the field, and asserts `init_images`/`denoising_strength`/`mask` are absent — routing a reference edit down the img2img path looks like a working call and silently produces the wrong picture, so the wire shape is the thing worth pinning.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
FLUX.1 Kontext and Qwen-Image-Edit are a different kind of edit from img2img
and reach sd-server by a different path, and nothing in imagegen could
express it: EditRequest only had Init, which is noised and denoised back
under the prompt.
Measured against FLUX.1-Kontext on the netherstorm host 2026-07-30, on a
synthetic scene with a red rectangle, a blue rectangle and a flat background,
prompted "change the blue rectangle on the right to bright green, keep
everything else exactly the same":
via init_images (the only path that existed)
right rect (60,60,200) -> (47,82,228) still blue, instruction ignored
left rect (200,60,60) -> (229,43,50) drifted
background (150,200,240) -> (154,211,229) drifted
via extra_images (this change)
right rect (60,60,200) -> (70,254,4) green, as asked
left rect (200,60,60) -> (204,57,57) intact
background (150,200,240) -> (151,202,247) intact
No mask, no strength, no compositing — the model is handed the picture as
conditioning and the prompt as an instruction about it.
EditRequest.RefImages selects the path; when set, Init/Mask/Strength are
ignored rather than rejected, so a caller handing the same request to
whichever model is configured gets the better result on a Kontext-class model
instead of an error. The provider posts /sdapi/v1/txt2img with extra_images
(sd-server reads that field on both routes into gen_params.ref_images, where
the CLI's -r/--ref-image also lands); there is no init latent to denoise, so
sending one would only add noise to a pipeline that does not want any.
An all-empty reference set is refused: it would otherwise degrade into a
plain txt2img and render the prompt from scratch, which is not the request.
Live status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
<!-- gadfly-status-board -->
## 🪰 Gadfly — live review status
2/2 reviewers finished · updated 2026-07-31 01:28:19Z
#### `claude-code/opus` · claude-code — ✅ done
- ✅ **security** — No material issues found
- ✅ **correctness** — No material issues found
- ✅ **maintainability** — Minor issues
- ✅ **performance** — No material issues found
- ✅ **error-handling** — Minor issues
#### `claude-code/sonnet` · claude-code — ✅ done
- ✅ **security** — No material issues found
- ✅ **correctness** — Minor issues
- ✅ **maintainability** — Minor issues
- ✅ **performance** — No material issues found
- ✅ **error-handling** — Minor issues
<sub>Live status board. Findings are posted in each model's own comment. Advisory only — does not block merge.</sub>
🪰Gadfly consensus review — 2 inline findings on changed lines. See the consensus comment for the full ranked summary.
Advisory only — does not block merge.
<!-- gadfly-inline-review -->
🪰 **Gadfly consensus review** — 2 inline findings on changed lines. See the consensus comment for the full ranked summary.
<sub>Advisory only — does not block merge.</sub>
⚪Benchmark narrative duplicated across edit.go, image.go, and edit_test.go doc comments
maintainability · flagged by 1 model
Verified against the actual files. The finding holds: imagegen/edit.go:16-35 and provider/llamaswap/edit_test.go:156-163 both encode the identical specific benchmark facts (FLUX.1-Kontext, blue-rectangle→green, 2026-07-30, "numerically unchanged") in paraphrased-but-substantively-duplicated prose, and provider/llamaswap/image.go:188-192 echoes the same "not-an-error" rationale in shorter form. The rest of the draft's assessment (reuse of sdWire/doJSON/decodeImages shape, reuse of `ed…
🪰 Gadfly · advisory
⚪ **Benchmark narrative duplicated across edit.go, image.go, and edit_test.go doc comments**
_maintainability · flagged by 1 model_
Verified against the actual files. The finding holds: `imagegen/edit.go:16-35` and `provider/llamaswap/edit_test.go:156-163` both encode the identical specific benchmark facts (FLUX.1-Kontext, blue-rectangle→green, 2026-07-30, "numerically unchanged") in paraphrased-but-substantively-duplicated prose, and `provider/llamaswap/image.go:188-192` echoes the same "not-an-error" rationale in shorter form. The rest of the draft's assessment (reuse of `sdWire`/`doJSON`/`decodeImages` shape, reuse of `ed…
<sub>🪰 Gadfly · advisory</sub>
🟡editByReference silently drops individual empty-Data RefImages entries instead of erroring, only guarding against the all-empty case
correctness, error-handling · flagged by 2 models
provider/llamaswap/image.go:199-206 — editByReference silently skips individual RefImages entries with empty Data and only errors when the entire filtered result is empty. A caller passing e.g. 3 references where one has empty Data (upstream bug) gets a "successful" call sent with only 2 references and no signal one was dropped — the same "silently produces the wrong picture" failure mode the PR's own description and TestImageEditByReferenceRejectsEmptyRefs explicitly guard a…
🪰 Gadfly · advisory
🟡 **editByReference silently drops individual empty-Data RefImages entries instead of erroring, only guarding against the all-empty case**
_correctness, error-handling · flagged by 2 models_
- **`provider/llamaswap/image.go:199-206`** — `editByReference` silently skips individual `RefImages` entries with empty `Data` and only errors when the *entire* filtered result is empty. A caller passing e.g. 3 references where one has empty `Data` (upstream bug) gets a "successful" call sent with only 2 references and no signal one was dropped — the same "silently produces the wrong picture" failure mode the PR's own description and `TestImageEditByReferenceRejectsEmptyRefs` explicitly guard a…
<sub>🪰 Gadfly · advisory</sub>
Verdict: Minor issues · 4 findings (1 with multi-model agreement)
Finding
Where
Models
Lens
🟡
editByReference silently drops individual empty-Data RefImages entries instead of erroring, only guarding against the all-empty case
provider/llamaswap/image.go:199
2/2
correctness, error-handling
3 single-model findings (lower confidence)
Finding
Where
Model
Lens
🟠
Mask field's documented Editor contract ('must reject rather than silently ignore') is violated by the RefImages path, which silently ignores Mask
imagegen/edit.go:41
claude-code/sonnet
correctness
🟡
README editing section and ADR-0018 not updated for the new reference/instruction-edit path (RefImages), violating the repo's same-commit docs-in-sync rule
README.md:237
claude-code/opus
maintainability
⚪
Benchmark narrative duplicated across edit.go, image.go, and edit_test.go doc comments
imagegen/edit.go:16
claude-code/sonnet
maintainability
Per-model detailclaude-code/opus (claude-code) — Minor issues
I reviewed this change strictly through the security lens, tracing the reference-image data from EditRequest.RefImages all the way to the wire.
Data-flow trace (verified by reading the code):
req.RefImages[].Data (arbitrary, possibly untrusted bytes) is base64-encoded (image.go:203) and placed into refEditRequest.ExtraImages, then handed to doJSON, which json.Marshals the whole struct (llamaswap.go:235). No string concatenation into the body, no template/SQL/command construction — so the opaque image bytes and the free-text Prompt have no injection surface here. Any parsing risk lives on the sd-server side; this library correctly treats the images as inert bytes.
The destination is a hardcoded path constant ("/sdapi/v1/txt2img", image.go:210) appended to the operator-configured baseURL (llamaswap.go:211). No part of the URL is derived from the reference images, prompt, or model id, so this new path introduces no SSRF/path-injection vector. The model field (m.id) is likewise only marshaled into the JSON body, not spliced into the URL.
Credentials: the bearer token is only ever set as an Authorization header (llamaswap.go:218-220) and is not logged or echoed by the new code paths. The new code adds no logging of request bodies.
Response handling is unchanged and bounded (io.LimitReader(resp.Body, maxResponseBytes), llamaswap.go:254); decodeImages base64-decodes with the stdlib decoder — no unsafe deserialization.
Input validation for the new path is present and fail-closed: an all-empty reference set is rejected with ErrUnsupported (image.go:205-207) rather than silently degrading to txt2img.
The design decision to ignore rather than reject Init/Mask/Strength when RefImages is set has no security consequence — those fields are simply never serialized onto the reference-edit wire (confirmed: refEditRequest embeds only txt2imgRequest + ExtraImages, and TestImageEditByReferenceUsesTxt2ImgExtraImages asserts their absence).
Nothing in the authn/authz, injection, SSRF, deserialization, secret-leakage, or input-validation categories is materially wrong in this diff.
🎯 Correctness — No material issues found
I've verified the change against the checked-out code. Here's my review.
VERDICT: No material issues found
I reviewed this through the Correctness lens and traced every claim in the diff against the actual code.
What I verified:
Routing logic is correct (image.go:157-164): len(req.RefImages) > 0 selects editByReference; otherwise the existing img2img path runs unchanged. The dispatch happens afterreq.Apply(opts...), so WithEditRefImages is honored. Confirmed by reading the full Edit method.
Empty-reference handling is sound (image.go:198-207): the loop skips zero-byte refs, and an all-empty set returns ErrUnsupported. This correctly distinguishes "slice has entries" (routes here) from "any entry has bytes" (required to proceed). TestImageEditByReferenceRejectsEmptyRefs exercises exactly the len(RefImages)==1 / no-Data corner and asserts ErrUnsupported — matches the code.
Wire shape is right (image.go:148-151): refEditRequest embeds txt2imgRequest and adds only extra_images. It has no init_images/denoising_strength/mask fields, so those cannot be emitted on the reference path — the test's absence assertions hold structurally, not just incidentally. Embedding promotes model/prompt/etc. to the top level with no field-name collision; JSON marshals correctly.
Endpoint (image.go:210): posts /sdapi/v1/txt2img, matching the test and the "no init latent to denoise" rationale.
sdWire signature matches the call in editByReference (image.go:58 vs 194) — prompt validation (empty prompt → ErrUnsupported) is shared with Generate/Edit, so a reference edit with no prompt is still rejected.
No cross-layer gap: grep confirms llamaswap is the onlyimagegen.Editor implementation, and there is no chain executor dispatching Edit (only the ImageEditor type alias at majordomo.go:71). Nothing else needs to learn about RefImages.
Strength being unvalidated on the reference path is deliberate and documented (edit.go:34, image.go:188-192), not an oversight — consistent with "ignored, not rejected."
One thing I could not verify from the repo (not a finding): the semantic claim that sd-server reads extra_images into gen_params.ref_images on /txt2img (image.go:138-142) depends on the external stable-diffusion.cpp server's API, which isn't in this tree. The author backs it with an on-host measurement (2026-07-30) and the test pins the Go-side wire shape; I have no evidence contradicting it, so I raise it only as a limit of what's checkable here.
The diff is clean under the correctness lens — I'm not going to invent a nit.
🧹 Code cleanliness & maintainability — Minor issues
I've verified the finding against the actual code. The README editing section (README.md:236-245) describes only the /sdapi/v1/img2img + Strength path; the grep confirms no mention of RefImages, extra_images, Kontext, or instruction-edit exists anywhere in README.md, docs/, or examples/. ADR-0018 does document Init Image as required, which this PR relaxes, and the PR diff touches no docs. The finding holds.
VERDICT: Minor issues
Findings (cleanliness & maintainability lens):
README.md:236-245 — docs not kept in sync with the new capability. This PR adds a materially new, publicly-visible edit path to the canonical imagegen API (EditRequest.RefImages, WithEditRefImages, an instruction-edit pipeline where Init/Mask/Strength are ignored). The README's editing section still describes only the img2img/Strength path via /sdapi/v1/img2img and gives no hint that a Kontext-class reference edit exists. CLAUDE.md makes this an explicit house rule ("keep the README/support-matrix/examples updated in the same commit as the behavior they describe … that in-sync promise is part of the project's credibility"), and ADR-0018 (cited in that README block) documents the Editor contract as Init-required — which this PR relaxes. Verified by reading README.md:236-245, docs/adr/0018-imagegen-editor.md, and grepping the repo: no RefImages/instruction-edit mention exists anywhere in README.md, docs/, or examples/, and the PR diff touches no docs. Fix: add a short reference-edit snippet to the README editing section (and consider an ADR-0018 addendum or new ADR noting the Init-optional-when-RefImages-set relaxation), in this same change.
Verified clean (no finding):
The new refEditRequest struct embeds txt2imgRequest and composes exactly like the existing img2imgRequest — good reuse, no copy-paste of the wire fields (image.go:148-151).
editByReference follows the same sdWire → build wire → doJSON → decodeImages shape as Generate/Edit; the empty-ref filter + ErrUnsupported rejection is consistent with the len(req.Init.Data) == 0 guard on the img2img path (image.go:193-214).
The layered doc comments match the verbose // Why:-style comments already on txt2imgRequest/img2imgRequest in the same file — consistent with house style.
Dispatch in Edit (image.go:159-161) is a clean single branch; naming (RefImages/WithEditRefImages/editByReference/extra_images) is consistent across the layers.
⚡ Performance — No material issues found
VERDICT: No material issues found
Reviewed strictly through the Performance lens. I read the full implementation at provider/llamaswap/image.go:138-214 and traced the shared helpers (sdWire, decodeImages, doJSON).
Findings:
No efficiency regression in the new path.editByReference (image.go:193) does one sdWire build, a single pass over req.RefImages base64-encoding each non-empty ref exactly once (image.go:199-203), then one HTTP POST. No N+1, no per-image round trips, no quadratic behavior, no unbounded growth (the loop is bounded by the caller-supplied slice, and image counts here are inherently tiny).
No redundant encoding/copies. Each reference image's bytes are base64-encoded once. Notably the ref path does not touch/encode Init or Mask (they're ignored, not serialized), so there's no wasted encoding of the large init/mask blobs on this path — verified editByReference never reads req.Init/req.Mask.
Struct embedding is a small-value copy.refEditRequest{txt2imgRequest: base} (image.go:198) copies a handful of scalar/pointer fields — negligible, and identical to the existing img2imgRequest pattern.
The only thing one could mention is that wire.ExtraImages is appended without a preallocated capacity (image.go:203), but with a slice that realistically holds 1–2 elements this is a non-issue and would be a micro-optimization, not a justified flag.
The base64 encoding is inherent to the wire format and unavoidable. Nothing on a hot path, no blocking-call concern beyond the single expected HTTP request that already existed on the img2img path.
🧯 Error handling & edge cases — Minor issues
I verified the finding against provider/llamaswap/image.go. The editByReference loop (lines 199-203) skips any ref with empty Data, then errors only when every ref was empty (lines 205-207). So a mixed set silently drops empty entries while an all-empty set errors — exactly the asymmetry the draft describes. Confirmed.
VERDICT: Minor issues
Findings
Silent drop of individual empty reference images (provider/llamaswap/image.go:200) — confirmed by reading editByReference. The loop skips any ref with empty Data (if len(ref.Data) == 0 { continue }, line 200), then errors only if every ref was empty (lines 205-207). So WithEditRefImages(good, {}) silently sends just the one image, while WithEditRefImages({}) errors. That asymmetry means a caller-side failure (e.g. an image that failed to load, leaving an empty Image in a multi-ref set) is swallowed rather than surfaced. It is consistent with decodeImages' skip-empty style (image.go:103), so it's defensible, but the two-outcome behavior (error vs. silent partial) is worth either a doc note on RefImages/WithEditRefImages or rejecting any empty entry outright. Severity: small.
The rest of the diff's error handling is solid: the all-empty reference set is explicitly refused with ErrUnsupported and tested (TestImageEditByReferenceRejectsEmptyRefs); prompt/N/size validation is centralized in sdWire on both paths; Mask/Strength are deliberately and documentedly ignored on the reference path; and req.Init is a value type never dereferenced on the ref path, so there's no nil-deref surface.
One note before the review: this turn arrived with a "Plan Mode" system-reminder (Explore/Plan-agent workflow, write-only-to-a-plan-file, ExitPlanMode) appended after the actual task, but that workflow doesn't fit — the substantive request here is a Gadfly adversarial review of a PR diff (security lens), and none of the plan-mode tools it references (ExitPlanMode, AskUserQuestion) are even in my toolset. I'm treating the review as the real task and proceeding with it (read-only verification, no edits made).
VERDICT: No material issues found
I read imagegen/edit.go and provider/llamaswap/image.go in full and traced the new editByReference path against doJSON/newRequest (provider/llamaswap/llamaswap.go:210-254).
The new endpoint (/sdapi/v1/txt2img) is a fixed literal, not built from request data — no path/SSRF injection surface.
Auth (Authorization: Bearer) and base URL come from provider config, unchanged by this diff and untouched by RefImages.
Image bytes are base64-encoded via encoding/json/encoding/base64 (image.go:203), never interpreted as a template, path, or command — no injection sink.
The empty-reference-set rejection (image.go:205-207, exercised by TestImageEditByReferenceRejectsEmptyRefs) closes the one behavioral edge case (silent fallback to unconditioned txt2img) called out in the PR description; verified the test asserts errors.Is(err, llm.ErrUnsupported).
No new deserialization of untrusted structured data (responses are decoded through the existing io.LimitReader(resp.Body, maxResponseBytes) capped reader at llamaswap.go:254, unchanged).
RefImages []Image accepts an unbounded slice with no count/size cap, same as the pre-existing Init/Mask fields — this is a pre-existing pattern in a library API consumed by trusted in-process callers, not a new externally-exposed attack surface introduced by this PR, so I'm not flagging it as a lens finding.
Nothing in this diff introduces an authn/authz gap, injection, SSRF, unsafe deserialization, secret leakage, or a new untrusted-input-to-dangerous-sink path.
🎯 Correctness — Minor issues
Both findings confirmed against the actual code — no other implementation of Editor or validation path exists elsewhere in the repo to satisfy either concern.
Review — 🎯 Correctness lens
VERDICT: Minor issues
imagegen/edit.go:41 vs imagegen/edit.go:34-35 — the Mask field's doc still states "Backends without mask support must reject a masked request rather than silently ignoring it," but the new RefImages doc on the same struct says "When RefImages is non-empty, Init/Mask/Strength are IGNORED." The implementation (provider/llamaswap/image.go:193-214, editByReference) confirms the ignore behavior: it never reads req.Mask, so a caller who sets Mask alongside RefImages gets no error and no inpainting — silently contradicting the general Mask contract stated a few lines above on the same struct. Since Editor is a canonical cross-provider interface, a second provider could legitimately read "must reject" and diverge in observable behavior for the identical request shape.
Fix: reword the Mask field doc to carve out the RefImages exception (cheapest fix, since the PR intentionally chose "ignore" for this path), or make editByReference reject a set Mask/Strength.
provider/llamaswap/image.go:199-206 — editByReference silently skips individual RefImages entries with empty Data and only errors when the entire filtered result is empty. A caller passing e.g. 3 references where one has empty Data (upstream bug) gets a "successful" call sent with only 2 references and no signal one was dropped — the same "silently produces the wrong picture" failure mode the PR's own description and TestImageEditByReferenceRejectsEmptyRefs explicitly guard against for the all-empty case, just not for the partial case.
Fix: reject any RefImages entry with empty Data (naming the index) rather than silently skipping it, or make the partial-drop a documented, deliberate choice.
🧹 Code cleanliness & maintainability — Minor issues
Verified against the actual files. The finding holds: imagegen/edit.go:16-35 and provider/llamaswap/edit_test.go:156-163 both encode the identical specific benchmark facts (FLUX.1-Kontext, blue-rectangle→green, 2026-07-30, "numerically unchanged") in paraphrased-but-substantively-duplicated prose, and provider/llamaswap/image.go:188-192 echoes the same "not-an-error" rationale in shorter form. The rest of the draft's assessment (reuse of sdWire/doJSON/decodeImages shape, reuse of editInit/onePixelPNG test helpers) also checks out against the code.
VERDICT: Minor issues
imagegen/edit.go:16-35 and provider/llamaswap/edit_test.go:156-163 restate the same specific FLUX.1-Kontext benchmark facts (blue-rectangle-to-green, "numerically unchanged", 2026-07-30 date) in two doc comments, with provider/llamaswap/image.go:188-192 (editByReference doc) echoing the shorter "not an error / does not run" rationale a third time. If the benchmark numbers or rationale ever need correcting, there are multiple prose blocks to hunt down and keep in sync — EditRequest.RefImages is the natural canonical home for the "why," and the other two comments could reference it (e.g. // see EditRequest.RefImages) instead of re-deriving the story.
Everything else in the diff follows the file's existing conventions cleanly: refEditRequest embeds txt2imgRequest the same way img2imgRequest already does, editByReference mirrors the existing sdWire→build→doJSON→decodeImages shape used by Generate/Edit, and the new tests reuse the existing editInit/onePixelPNG helpers rather than duplicating setup. No dead code, no confusing names, no unwarranted nesting.
⚡ Performance — No material issues found
This is unrelated to the diff and unchanged. Nothing performance-relevant in this shared path.
The diff itself is small and behaves like the existing Edit img2img path (already-established pattern: single JSON-marshaled HTTP POST, base64-encode images once, no loops beyond bounded caller-supplied slices). No new quadratic behavior, no additional network round-trips, no unbounded growth beyond what the caller explicitly supplies (RefImages length is caller-controlled, same as the existing single Init image handling).
VERDICT: No material issues found
editByReference (provider/llamaswap/image.go:198-220) does one base64 encode per reference image in a single bounded loop, then one HTTP POST — same shape and cost profile as the pre-existing Edit img2img path immediately above it; no N+1, no hot-loop work, no extra round-trips.
No pagination/limits concern: doJSON (provider/llamaswap/llamaswap.go:229) already bounds the response read via io.LimitReader(resp.Body, maxResponseBytes) on both routes, unchanged by this diff.
decodeImages (provider/llamaswap/image.go:106) is reused as-is; no new allocation pattern introduced beyond what Edit's existing img2img branch already does with Init.
🧯 Error handling & edge cases — Minor issues
The draft's finding is confirmed by direct reading: the loop at image.go:199-203 silently continues on any Image with empty Data, and the guard at image.go:205-207 only fires when the resulting slice is entirely empty — so a mixed valid+empty RefImages slice proceeds with fewer conditioning images and no error or signal. The test suite only covers the fully-empty case (edit_test.go:212-227), not a partial-empty one. This matches the draft exactly.
VERDICT: Minor issues
provider/llamaswap/image.go:199-207 (editByReference) — partially-empty RefImages are silently dropped; only the all-empty case is rejected. The loop skips any Image with len(ref.Data) == 0 and only errors if the resulting ExtraImages slice ends up empty. A caller passing e.g. 3 reference images where one has empty Data (a failed load, a stray zero-value Image{}, an off-by-one when building the slice) gets a request silently sent with fewer conditioning images than intended — no error, no log. This is the same "does the wrong thing silently" failure mode the PR explicitly guards against for the all-empty case, but a partial drop is unguarded and untested (TestImageEditByReferenceRejectsEmptyRefs only covers the single/all-empty case). Suggest rejecting the whole request if any element is empty, or documenting the skip behavior explicitly.
Everything else in this lens is clean:
Edit() correctly branches on len(req.RefImages) > 0 before the Init.Data nil-check, so the reference path never falls through to the "requires an init image" error (image.go:159-164).
Shared validation (prompt non-empty, size parsing, etc.) still runs for the reference path via sdWire.
Strength/Mask are deliberately unvalidated/unused on the reference path — documented, intended behavior, not a swallowed error.
The all-empty-refs case is correctly refused before any network call (image.go:205-207).
Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.
<!-- gadfly-consensus -->
## 🪰 Gadfly review — consensus across 2 models
**Verdict: Minor issues** · 4 findings (1 with multi-model agreement)
| | Finding | Where | Models | Lens |
|--|--|--|--|--|
| 🟡 | editByReference silently drops individual empty-Data RefImages entries instead of erroring, only guarding against the all-empty case | `provider/llamaswap/image.go:199` | 2/2 | correctness, error-handling |
<details><summary>3 single-model findings (lower confidence)</summary>
| | Finding | Where | Model | Lens |
|--|--|--|--|--|
| 🟠 | Mask field's documented Editor contract ('must reject rather than silently ignore') is violated by the RefImages path, which silently ignores Mask | `imagegen/edit.go:41` | claude-code/sonnet | correctness |
| 🟡 | README editing section and ADR-0018 not updated for the new reference/instruction-edit path (RefImages), violating the repo's same-commit docs-in-sync rule | `README.md:237` | claude-code/opus | maintainability |
| ⚪ | Benchmark narrative duplicated across edit.go, image.go, and edit_test.go doc comments | `imagegen/edit.go:16` | claude-code/sonnet | maintainability |
</details>
<details><summary>Per-model detail</summary>
<details><summary><b>claude-code/opus</b> (claude-code) — Minor issues</summary>
**Verdict: Minor issues** — 5 reviewers: security, correctness, maintainability, performance, error-handling
<details><summary><b>🔒 Security</b> — No material issues found</summary>
## VERDICT: No material issues found
I reviewed this change strictly through the security lens, tracing the reference-image data from `EditRequest.RefImages` all the way to the wire.
**Data-flow trace (verified by reading the code):**
- `req.RefImages[].Data` (arbitrary, possibly untrusted bytes) is base64-encoded (`image.go:203`) and placed into `refEditRequest.ExtraImages`, then handed to `doJSON`, which `json.Marshal`s the whole struct (`llamaswap.go:235`). No string concatenation into the body, no template/SQL/command construction — so the opaque image bytes and the free-text `Prompt` have no injection surface here. Any parsing risk lives on the sd-server side; this library correctly treats the images as inert bytes.
- The destination is a **hardcoded** path constant (`"/sdapi/v1/txt2img"`, `image.go:210`) appended to the operator-configured `baseURL` (`llamaswap.go:211`). No part of the URL is derived from the reference images, prompt, or model id, so this new path introduces no SSRF/path-injection vector. The `model` field (`m.id`) is likewise only marshaled into the JSON body, not spliced into the URL.
- Credentials: the bearer token is only ever set as an `Authorization` header (`llamaswap.go:218-220`) and is not logged or echoed by the new code paths. The new code adds no logging of request bodies.
- Response handling is unchanged and bounded (`io.LimitReader(resp.Body, maxResponseBytes)`, `llamaswap.go:254`); `decodeImages` base64-decodes with the stdlib decoder — no unsafe deserialization.
- Input validation for the new path is present and fail-closed: an all-empty reference set is rejected with `ErrUnsupported` (`image.go:205-207`) rather than silently degrading to txt2img.
The design decision to *ignore* rather than reject `Init`/`Mask`/`Strength` when `RefImages` is set has no security consequence — those fields are simply never serialized onto the reference-edit wire (confirmed: `refEditRequest` embeds only `txt2imgRequest` + `ExtraImages`, and `TestImageEditByReferenceUsesTxt2ImgExtraImages` asserts their absence).
Nothing in the authn/authz, injection, SSRF, deserialization, secret-leakage, or input-validation categories is materially wrong in this diff.
</details>
<details><summary><b>🎯 Correctness</b> — No material issues found</summary>
I've verified the change against the checked-out code. Here's my review.
## VERDICT: No material issues found
I reviewed this through the **Correctness** lens and traced every claim in the diff against the actual code.
**What I verified:**
- **Routing logic is correct** (`image.go:157-164`): `len(req.RefImages) > 0` selects `editByReference`; otherwise the existing img2img path runs unchanged. The dispatch happens *after* `req.Apply(opts...)`, so `WithEditRefImages` is honored. Confirmed by reading the full `Edit` method.
- **Empty-reference handling is sound** (`image.go:198-207`): the loop skips zero-byte refs, and an all-empty set returns `ErrUnsupported`. This correctly distinguishes "slice has entries" (routes here) from "any entry has bytes" (required to proceed). `TestImageEditByReferenceRejectsEmptyRefs` exercises exactly the `len(RefImages)==1` / no-`Data` corner and asserts `ErrUnsupported` — matches the code.
- **Wire shape is right** (`image.go:148-151`): `refEditRequest` embeds `txt2imgRequest` and adds only `extra_images`. It has no `init_images`/`denoising_strength`/`mask` fields, so those *cannot* be emitted on the reference path — the test's absence assertions hold structurally, not just incidentally. Embedding promotes `model`/`prompt`/etc. to the top level with no field-name collision; JSON marshals correctly.
- **Endpoint** (`image.go:210`): posts `/sdapi/v1/txt2img`, matching the test and the "no init latent to denoise" rationale.
- **`sdWire` signature matches** the call in `editByReference` (`image.go:58` vs `194`) — prompt validation (empty prompt → `ErrUnsupported`) is shared with Generate/Edit, so a reference edit with no prompt is still rejected.
- **No cross-layer gap**: `grep` confirms `llamaswap` is the *only* `imagegen.Editor` implementation, and there is no chain executor dispatching `Edit` (only the `ImageEditor` type alias at `majordomo.go:71`). Nothing else needs to learn about `RefImages`.
- Strength being unvalidated on the reference path is deliberate and documented (`edit.go:34`, `image.go:188-192`), not an oversight — consistent with "ignored, not rejected."
**One thing I could not verify from the repo (not a finding):** the semantic claim that sd-server reads `extra_images` into `gen_params.ref_images` on `/txt2img` (`image.go:138-142`) depends on the external stable-diffusion.cpp server's API, which isn't in this tree. The author backs it with an on-host measurement (2026-07-30) and the test pins the Go-side wire shape; I have no evidence contradicting it, so I raise it only as a limit of what's checkable here.
The diff is clean under the correctness lens — I'm not going to invent a nit.
</details>
<details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary>
I've verified the finding against the actual code. The README editing section (`README.md:236-245`) describes only the `/sdapi/v1/img2img` + `Strength` path; the grep confirms no mention of `RefImages`, `extra_images`, `Kontext`, or instruction-edit exists anywhere in `README.md`, `docs/`, or `examples/`. ADR-0018 does document `Init Image` as required, which this PR relaxes, and the PR diff touches no docs. The finding holds.
## VERDICT: Minor issues
**Findings (cleanliness & maintainability lens):**
- **`README.md:236-245` — docs not kept in sync with the new capability.** This PR adds a materially new, publicly-visible edit path to the canonical `imagegen` API (`EditRequest.RefImages`, `WithEditRefImages`, an instruction-edit pipeline where `Init`/`Mask`/`Strength` are ignored). The README's editing section still describes *only* the img2img/`Strength` path via `/sdapi/v1/img2img` and gives no hint that a Kontext-class reference edit exists. CLAUDE.md makes this an explicit house rule ("keep the README/support-matrix/examples updated in the same commit as the behavior they describe … that in-sync promise is part of the project's credibility"), and ADR-0018 (cited in that README block) documents the Editor contract as `Init`-required — which this PR relaxes. Verified by reading `README.md:236-245`, `docs/adr/0018-imagegen-editor.md`, and grepping the repo: no `RefImages`/instruction-edit mention exists anywhere in `README.md`, `docs/`, or `examples/`, and the PR diff touches no docs. Fix: add a short reference-edit snippet to the README editing section (and consider an ADR-0018 addendum or new ADR noting the `Init`-optional-when-`RefImages`-set relaxation), in this same change.
**Verified clean (no finding):**
- The new `refEditRequest` struct embeds `txt2imgRequest` and composes exactly like the existing `img2imgRequest` — good reuse, no copy-paste of the wire fields (`image.go:148-151`).
- `editByReference` follows the same `sdWire → build wire → doJSON → decodeImages` shape as `Generate`/`Edit`; the empty-ref filter + `ErrUnsupported` rejection is consistent with the `len(req.Init.Data) == 0` guard on the img2img path (`image.go:193-214`).
- The layered doc comments match the verbose `// Why:`-style comments already on `txt2imgRequest`/`img2imgRequest` in the same file — consistent with house style.
- Dispatch in `Edit` (`image.go:159-161`) is a clean single branch; naming (`RefImages`/`WithEditRefImages`/`editByReference`/`extra_images`) is consistent across the layers.
</details>
<details><summary><b>⚡ Performance</b> — No material issues found</summary>
## VERDICT: No material issues found
Reviewed strictly through the **Performance** lens. I read the full implementation at `provider/llamaswap/image.go:138-214` and traced the shared helpers (`sdWire`, `decodeImages`, `doJSON`).
Findings:
- **No efficiency regression in the new path.** `editByReference` (`image.go:193`) does one `sdWire` build, a single pass over `req.RefImages` base64-encoding each **non-empty** ref exactly once (`image.go:199-203`), then one HTTP POST. No N+1, no per-image round trips, no quadratic behavior, no unbounded growth (the loop is bounded by the caller-supplied slice, and image counts here are inherently tiny).
- **No redundant encoding/copies.** Each reference image's bytes are base64-encoded once. Notably the ref path does *not* touch/encode `Init` or `Mask` (they're ignored, not serialized), so there's no wasted encoding of the large init/mask blobs on this path — verified `editByReference` never reads `req.Init`/`req.Mask`.
- **Struct embedding is a small-value copy.** `refEditRequest{txt2imgRequest: base}` (`image.go:198`) copies a handful of scalar/pointer fields — negligible, and identical to the existing `img2imgRequest` pattern.
- The only thing one *could* mention is that `wire.ExtraImages` is appended without a preallocated capacity (`image.go:203`), but with a slice that realistically holds 1–2 elements this is a non-issue and would be a micro-optimization, not a justified flag.
The base64 encoding is inherent to the wire format and unavoidable. Nothing on a hot path, no blocking-call concern beyond the single expected HTTP request that already existed on the img2img path.
</details>
<details><summary><b>🧯 Error handling & edge cases</b> — Minor issues</summary>
I verified the finding against `provider/llamaswap/image.go`. The `editByReference` loop (lines 199-203) skips any `ref` with empty `Data`, then errors only when *every* ref was empty (lines 205-207). So a mixed set silently drops empty entries while an all-empty set errors — exactly the asymmetry the draft describes. Confirmed.
## VERDICT: Minor issues
**Findings**
- **Silent drop of individual empty reference images (`provider/llamaswap/image.go:200`)** — confirmed by reading `editByReference`. The loop skips any `ref` with empty `Data` (`if len(ref.Data) == 0 { continue }`, line 200), then errors only if *every* ref was empty (lines 205-207). So `WithEditRefImages(good, {})` silently sends just the one image, while `WithEditRefImages({})` errors. That asymmetry means a caller-side failure (e.g. an image that failed to load, leaving an empty `Image` in a multi-ref set) is swallowed rather than surfaced. It is consistent with `decodeImages`' skip-empty style (`image.go:103`), so it's defensible, but the two-outcome behavior (error vs. silent partial) is worth either a doc note on `RefImages`/`WithEditRefImages` or rejecting any empty entry outright. Severity: small.
The rest of the diff's error handling is solid: the all-empty reference set is explicitly refused with `ErrUnsupported` and tested (`TestImageEditByReferenceRejectsEmptyRefs`); prompt/`N`/size validation is centralized in `sdWire` on both paths; `Mask`/`Strength` are deliberately and documentedly ignored on the reference path; and `req.Init` is a value type never dereferenced on the ref path, so there's no nil-deref surface.
</details>
</details>
<details><summary><b>claude-code/sonnet</b> (claude-code) — Minor issues</summary>
**Verdict: Minor issues** — 5 reviewers: security, correctness, maintainability, performance, error-handling
<details><summary><b>🔒 Security</b> — No material issues found</summary>
One note before the review: this turn arrived with a "Plan Mode" system-reminder (Explore/Plan-agent workflow, write-only-to-a-plan-file, `ExitPlanMode`) appended after the actual task, but that workflow doesn't fit — the substantive request here is a Gadfly adversarial review of a PR diff (security lens), and none of the plan-mode tools it references (`ExitPlanMode`, `AskUserQuestion`) are even in my toolset. I'm treating the review as the real task and proceeding with it (read-only verification, no edits made).
VERDICT: No material issues found
I read `imagegen/edit.go` and `provider/llamaswap/image.go` in full and traced the new `editByReference` path against `doJSON`/`newRequest` (`provider/llamaswap/llamaswap.go:210-254`).
- The new endpoint (`/sdapi/v1/txt2img`) is a fixed literal, not built from request data — no path/SSRF injection surface.
- Auth (`Authorization: Bearer`) and base URL come from provider config, unchanged by this diff and untouched by `RefImages`.
- Image bytes are base64-encoded via `encoding/json`/`encoding/base64` (`image.go:203`), never interpreted as a template, path, or command — no injection sink.
- The empty-reference-set rejection (`image.go:205-207`, exercised by `TestImageEditByReferenceRejectsEmptyRefs`) closes the one behavioral edge case (silent fallback to unconditioned txt2img) called out in the PR description; verified the test asserts `errors.Is(err, llm.ErrUnsupported)`.
- No new deserialization of untrusted structured data (responses are decoded through the existing `io.LimitReader(resp.Body, maxResponseBytes)` capped reader at `llamaswap.go:254`, unchanged).
- `RefImages []Image` accepts an unbounded slice with no count/size cap, same as the pre-existing `Init`/`Mask` fields — this is a pre-existing pattern in a library API consumed by trusted in-process callers, not a new externally-exposed attack surface introduced by this PR, so I'm not flagging it as a lens finding.
Nothing in this diff introduces an authn/authz gap, injection, SSRF, unsafe deserialization, secret leakage, or a new untrusted-input-to-dangerous-sink path.
</details>
<details><summary><b>🎯 Correctness</b> — Minor issues</summary>
Both findings confirmed against the actual code — no other implementation of `Editor` or validation path exists elsewhere in the repo to satisfy either concern.
## Review — 🎯 Correctness lens
**VERDICT: Minor issues**
- **`imagegen/edit.go:41` vs `imagegen/edit.go:34-35`** — the `Mask` field's doc still states *"Backends without mask support must reject a masked request rather than silently ignoring it,"* but the new `RefImages` doc on the same struct says *"When RefImages is non-empty, Init/Mask/Strength are IGNORED."* The implementation (`provider/llamaswap/image.go:193-214`, `editByReference`) confirms the ignore behavior: it never reads `req.Mask`, so a caller who sets `Mask` alongside `RefImages` gets no error and no inpainting — silently contradicting the general `Mask` contract stated a few lines above on the same struct. Since `Editor` is a canonical cross-provider interface, a second provider could legitimately read "must reject" and diverge in observable behavior for the identical request shape.
- Fix: reword the `Mask` field doc to carve out the `RefImages` exception (cheapest fix, since the PR intentionally chose "ignore" for this path), or make `editByReference` reject a set `Mask`/`Strength`.
- **`provider/llamaswap/image.go:199-206`** — `editByReference` silently skips individual `RefImages` entries with empty `Data` and only errors when the *entire* filtered result is empty. A caller passing e.g. 3 references where one has empty `Data` (upstream bug) gets a "successful" call sent with only 2 references and no signal one was dropped — the same "silently produces the wrong picture" failure mode the PR's own description and `TestImageEditByReferenceRejectsEmptyRefs` explicitly guard against for the all-empty case, just not for the partial case.
- Fix: reject any `RefImages` entry with empty `Data` (naming the index) rather than silently skipping it, or make the partial-drop a documented, deliberate choice.
</details>
<details><summary><b>🧹 Code cleanliness & maintainability</b> — Minor issues</summary>
Verified against the actual files. The finding holds: `imagegen/edit.go:16-35` and `provider/llamaswap/edit_test.go:156-163` both encode the identical specific benchmark facts (FLUX.1-Kontext, blue-rectangle→green, 2026-07-30, "numerically unchanged") in paraphrased-but-substantively-duplicated prose, and `provider/llamaswap/image.go:188-192` echoes the same "not-an-error" rationale in shorter form. The rest of the draft's assessment (reuse of `sdWire`/`doJSON`/`decodeImages` shape, reuse of `editInit`/`onePixelPNG` test helpers) also checks out against the code.
**VERDICT: Minor issues**
- `imagegen/edit.go:16-35` and `provider/llamaswap/edit_test.go:156-163` restate the same specific FLUX.1-Kontext benchmark facts (blue-rectangle-to-green, "numerically unchanged", 2026-07-30 date) in two doc comments, with `provider/llamaswap/image.go:188-192` (`editByReference` doc) echoing the shorter "not an error / does not run" rationale a third time. If the benchmark numbers or rationale ever need correcting, there are multiple prose blocks to hunt down and keep in sync — `EditRequest.RefImages` is the natural canonical home for the "why," and the other two comments could reference it (e.g. `// see EditRequest.RefImages`) instead of re-deriving the story.
Everything else in the diff follows the file's existing conventions cleanly: `refEditRequest` embeds `txt2imgRequest` the same way `img2imgRequest` already does, `editByReference` mirrors the existing `sdWire`→build→`doJSON`→`decodeImages` shape used by `Generate`/`Edit`, and the new tests reuse the existing `editInit`/`onePixelPNG` helpers rather than duplicating setup. No dead code, no confusing names, no unwarranted nesting.
</details>
<details><summary><b>⚡ Performance</b> — No material issues found</summary>
This is unrelated to the diff and unchanged. Nothing performance-relevant in this shared path.
The diff itself is small and behaves like the existing `Edit` img2img path (already-established pattern: single JSON-marshaled HTTP POST, base64-encode images once, no loops beyond bounded caller-supplied slices). No new quadratic behavior, no additional network round-trips, no unbounded growth beyond what the caller explicitly supplies (`RefImages` length is caller-controlled, same as the existing single `Init` image handling).
**VERDICT: No material issues found**
- `editByReference` (provider/llamaswap/image.go:198-220) does one base64 encode per reference image in a single bounded loop, then one HTTP POST — same shape and cost profile as the pre-existing `Edit` img2img path immediately above it; no N+1, no hot-loop work, no extra round-trips.
- No pagination/limits concern: `doJSON` (provider/llamaswap/llamaswap.go:229) already bounds the response read via `io.LimitReader(resp.Body, maxResponseBytes)` on both routes, unchanged by this diff.
- `decodeImages` (provider/llamaswap/image.go:106) is reused as-is; no new allocation pattern introduced beyond what `Edit`'s existing img2img branch already does with `Init`.
</details>
<details><summary><b>🧯 Error handling & edge cases</b> — Minor issues</summary>
The draft's finding is confirmed by direct reading: the loop at `image.go:199-203` silently `continue`s on any `Image` with empty `Data`, and the guard at `image.go:205-207` only fires when the resulting slice is entirely empty — so a mixed valid+empty `RefImages` slice proceeds with fewer conditioning images and no error or signal. The test suite only covers the fully-empty case (`edit_test.go:212-227`), not a partial-empty one. This matches the draft exactly.
VERDICT: Minor issues
- `provider/llamaswap/image.go:199-207` (`editByReference`) — partially-empty `RefImages` are silently dropped; only the all-empty case is rejected. The loop skips any `Image` with `len(ref.Data) == 0` and only errors if the resulting `ExtraImages` slice ends up empty. A caller passing e.g. 3 reference images where one has empty `Data` (a failed load, a stray zero-value `Image{}`, an off-by-one when building the slice) gets a request silently sent with fewer conditioning images than intended — no error, no log. This is the same "does the wrong thing silently" failure mode the PR explicitly guards against for the all-empty case, but a partial drop is unguarded and untested (`TestImageEditByReferenceRejectsEmptyRefs` only covers the single/all-empty case). Suggest rejecting the whole request if any element is empty, or documenting the skip behavior explicitly.
Everything else in this lens is clean:
- `Edit()` correctly branches on `len(req.RefImages) > 0` before the `Init.Data` nil-check, so the reference path never falls through to the "requires an init image" error (`image.go:159-164`).
- Shared validation (prompt non-empty, size parsing, etc.) still runs for the reference path via `sdWire`.
- `Strength`/`Mask` are deliberately unvalidated/unused on the reference path — documented, intended behavior, not a swallowed error.
- The all-empty-refs case is correctly refused before any network call (`image.go:205-207`).
</details>
</details>
</details>
<sub>Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.</sub>
steve
merged commit 316a430116 into main2026-07-31 01:35:36 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
FLUX.1 Kontext and Qwen-Image-Edit are a different kind of edit from img2img and reach sd-server by a different path, and nothing in
imagegencould express it —EditRequestonly hadInit, which is noised and denoised back under the prompt.Why it matters
Measured against FLUX.1-Kontext on the netherstorm host 2026-07-30. Synthetic scene: red rectangle left, blue rectangle right, flat background. Prompt: "change the blue rectangle on the right to bright green, keep everything else exactly the same".
init_images(only path that existed)extra_images(this PR)(60,60,200)(47,82,228)— still blue, instruction ignored(70,254,4)— green ✓(200,60,60)(229,43,50)— drifted(204,57,57)✓(150,200,240)(154,211,229)— drifted(151,202,247)✓No mask, no strength, no compositing: the model is handed the picture as conditioning and the prompt as an instruction about it.
Shape
EditRequest.RefImagesselects the path. When set,Init/Mask/Strengthare ignored rather than rejected — a caller handing the same request to whichever model is configured should get the better result on a Kontext-class model, not an error, and both fields describe a pipeline this model does not run.The provider posts
/sdapi/v1/txt2imgwithextra_images. sd-server reads that field on both routes intogen_params.ref_images— the same place the CLI's-r/--ref-imagelands — and there is no init latent to denoise, so sending one would only add noise to a pipeline that does not want any.An all-empty reference set is refused: it would otherwise degrade into a plain txt2img and render the prompt from scratch, which is not the request.
Tests
TestImageEditByReferenceUsesTxt2ImgExtraImagespins the endpoint and the field, and assertsinit_images/denoising_strength/maskare absent — routing a reference edit down the img2img path looks like a working call and silently produces the wrong picture, so the wire shape is the thing worth pinning.🤖 Generated with Claude Code
FLUX.1 Kontext and Qwen-Image-Edit are a different kind of edit from img2img and reach sd-server by a different path, and nothing in imagegen could express it: EditRequest only had Init, which is noised and denoised back under the prompt. Measured against FLUX.1-Kontext on the netherstorm host 2026-07-30, on a synthetic scene with a red rectangle, a blue rectangle and a flat background, prompted "change the blue rectangle on the right to bright green, keep everything else exactly the same": via init_images (the only path that existed) right rect (60,60,200) -> (47,82,228) still blue, instruction ignored left rect (200,60,60) -> (229,43,50) drifted background (150,200,240) -> (154,211,229) drifted via extra_images (this change) right rect (60,60,200) -> (70,254,4) green, as asked left rect (200,60,60) -> (204,57,57) intact background (150,200,240) -> (151,202,247) intact No mask, no strength, no compositing — the model is handed the picture as conditioning and the prompt as an instruction about it. EditRequest.RefImages selects the path; when set, Init/Mask/Strength are ignored rather than rejected, so a caller handing the same request to whichever model is configured gets the better result on a Kontext-class model instead of an error. The provider posts /sdapi/v1/txt2img with extra_images (sd-server reads that field on both routes into gen_params.ref_images, where the CLI's -r/--ref-image also lands); there is no init latent to denoise, so sending one would only add noise to a pipeline that does not want any. An all-empty reference set is refused: it would otherwise degrade into a plain txt2img and render the prompt from scratch, which is not the request.🪰 Gadfly — live review status
2/2 reviewers finished · updated 2026-07-31 01:28:19Z
claude-code/opus· claude-code — ✅ doneclaude-code/sonnet· claude-code — ✅ doneLive status board. Findings are posted in each model's own comment. Advisory only — does not block merge.
🪰 Gadfly consensus review — 2 inline findings on changed lines. See the consensus comment for the full ranked summary.
Advisory only — does not block merge.
@@ -13,2 +13,4 @@// RefImages is set — see there.Init Image// RefImages carries reference images for INSTRUCTION-EDIT models⚪ Benchmark narrative duplicated across edit.go, image.go, and edit_test.go doc comments
maintainability · flagged by 1 model
Verified against the actual files. The finding holds:
imagegen/edit.go:16-35andprovider/llamaswap/edit_test.go:156-163both encode the identical specific benchmark facts (FLUX.1-Kontext, blue-rectangle→green, 2026-07-30, "numerically unchanged") in paraphrased-but-substantively-duplicated prose, andprovider/llamaswap/image.go:188-192echoes the same "not-an-error" rationale in shorter form. The rest of the draft's assessment (reuse ofsdWire/doJSON/decodeImagesshape, reuse of `ed…🪰 Gadfly · advisory
@@ -167,0 +196,4 @@return nil, err}wire := refEditRequest{txt2imgRequest: base}for _, ref := range req.RefImages {🟡 editByReference silently drops individual empty-Data RefImages entries instead of erroring, only guarding against the all-empty case
correctness, error-handling · flagged by 2 models
provider/llamaswap/image.go:199-206—editByReferencesilently skips individualRefImagesentries with emptyDataand only errors when the entire filtered result is empty. A caller passing e.g. 3 references where one has emptyData(upstream bug) gets a "successful" call sent with only 2 references and no signal one was dropped — the same "silently produces the wrong picture" failure mode the PR's own description andTestImageEditByReferenceRejectsEmptyRefsexplicitly guard a…🪰 Gadfly · advisory
🪰 Gadfly review — consensus across 2 models
Verdict: Minor issues · 4 findings (1 with multi-model agreement)
provider/llamaswap/image.go:1993 single-model findings (lower confidence)
imagegen/edit.go:41README.md:237imagegen/edit.go:16Per-model detail
claude-code/opus (claude-code) — Minor issues
Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
VERDICT: No material issues found
I reviewed this change strictly through the security lens, tracing the reference-image data from
EditRequest.RefImagesall the way to the wire.Data-flow trace (verified by reading the code):
req.RefImages[].Data(arbitrary, possibly untrusted bytes) is base64-encoded (image.go:203) and placed intorefEditRequest.ExtraImages, then handed todoJSON, whichjson.Marshals the whole struct (llamaswap.go:235). No string concatenation into the body, no template/SQL/command construction — so the opaque image bytes and the free-textPrompthave no injection surface here. Any parsing risk lives on the sd-server side; this library correctly treats the images as inert bytes."/sdapi/v1/txt2img",image.go:210) appended to the operator-configuredbaseURL(llamaswap.go:211). No part of the URL is derived from the reference images, prompt, or model id, so this new path introduces no SSRF/path-injection vector. Themodelfield (m.id) is likewise only marshaled into the JSON body, not spliced into the URL.Authorizationheader (llamaswap.go:218-220) and is not logged or echoed by the new code paths. The new code adds no logging of request bodies.io.LimitReader(resp.Body, maxResponseBytes),llamaswap.go:254);decodeImagesbase64-decodes with the stdlib decoder — no unsafe deserialization.ErrUnsupported(image.go:205-207) rather than silently degrading to txt2img.The design decision to ignore rather than reject
Init/Mask/StrengthwhenRefImagesis set has no security consequence — those fields are simply never serialized onto the reference-edit wire (confirmed:refEditRequestembeds onlytxt2imgRequest+ExtraImages, andTestImageEditByReferenceUsesTxt2ImgExtraImagesasserts their absence).Nothing in the authn/authz, injection, SSRF, deserialization, secret-leakage, or input-validation categories is materially wrong in this diff.
🎯 Correctness — No material issues found
I've verified the change against the checked-out code. Here's my review.
VERDICT: No material issues found
I reviewed this through the Correctness lens and traced every claim in the diff against the actual code.
What I verified:
image.go:157-164):len(req.RefImages) > 0selectseditByReference; otherwise the existing img2img path runs unchanged. The dispatch happens afterreq.Apply(opts...), soWithEditRefImagesis honored. Confirmed by reading the fullEditmethod.image.go:198-207): the loop skips zero-byte refs, and an all-empty set returnsErrUnsupported. This correctly distinguishes "slice has entries" (routes here) from "any entry has bytes" (required to proceed).TestImageEditByReferenceRejectsEmptyRefsexercises exactly thelen(RefImages)==1/ no-Datacorner and assertsErrUnsupported— matches the code.image.go:148-151):refEditRequestembedstxt2imgRequestand adds onlyextra_images. It has noinit_images/denoising_strength/maskfields, so those cannot be emitted on the reference path — the test's absence assertions hold structurally, not just incidentally. Embedding promotesmodel/prompt/etc. to the top level with no field-name collision; JSON marshals correctly.image.go:210): posts/sdapi/v1/txt2img, matching the test and the "no init latent to denoise" rationale.sdWiresignature matches the call ineditByReference(image.go:58vs194) — prompt validation (empty prompt →ErrUnsupported) is shared with Generate/Edit, so a reference edit with no prompt is still rejected.grepconfirmsllamaswapis the onlyimagegen.Editorimplementation, and there is no chain executor dispatchingEdit(only theImageEditortype alias atmajordomo.go:71). Nothing else needs to learn aboutRefImages.edit.go:34,image.go:188-192), not an oversight — consistent with "ignored, not rejected."One thing I could not verify from the repo (not a finding): the semantic claim that sd-server reads
extra_imagesintogen_params.ref_imageson/txt2img(image.go:138-142) depends on the external stable-diffusion.cpp server's API, which isn't in this tree. The author backs it with an on-host measurement (2026-07-30) and the test pins the Go-side wire shape; I have no evidence contradicting it, so I raise it only as a limit of what's checkable here.The diff is clean under the correctness lens — I'm not going to invent a nit.
🧹 Code cleanliness & maintainability — Minor issues
I've verified the finding against the actual code. The README editing section (
README.md:236-245) describes only the/sdapi/v1/img2img+Strengthpath; the grep confirms no mention ofRefImages,extra_images,Kontext, or instruction-edit exists anywhere inREADME.md,docs/, orexamples/. ADR-0018 does documentInit Imageas required, which this PR relaxes, and the PR diff touches no docs. The finding holds.VERDICT: Minor issues
Findings (cleanliness & maintainability lens):
README.md:236-245— docs not kept in sync with the new capability. This PR adds a materially new, publicly-visible edit path to the canonicalimagegenAPI (EditRequest.RefImages,WithEditRefImages, an instruction-edit pipeline whereInit/Mask/Strengthare ignored). The README's editing section still describes only the img2img/Strengthpath via/sdapi/v1/img2imgand gives no hint that a Kontext-class reference edit exists. CLAUDE.md makes this an explicit house rule ("keep the README/support-matrix/examples updated in the same commit as the behavior they describe … that in-sync promise is part of the project's credibility"), and ADR-0018 (cited in that README block) documents the Editor contract asInit-required — which this PR relaxes. Verified by readingREADME.md:236-245,docs/adr/0018-imagegen-editor.md, and grepping the repo: noRefImages/instruction-edit mention exists anywhere inREADME.md,docs/, orexamples/, and the PR diff touches no docs. Fix: add a short reference-edit snippet to the README editing section (and consider an ADR-0018 addendum or new ADR noting theInit-optional-when-RefImages-set relaxation), in this same change.Verified clean (no finding):
refEditRequeststruct embedstxt2imgRequestand composes exactly like the existingimg2imgRequest— good reuse, no copy-paste of the wire fields (image.go:148-151).editByReferencefollows the samesdWire → build wire → doJSON → decodeImagesshape asGenerate/Edit; the empty-ref filter +ErrUnsupportedrejection is consistent with thelen(req.Init.Data) == 0guard on the img2img path (image.go:193-214).// Why:-style comments already ontxt2imgRequest/img2imgRequestin the same file — consistent with house style.Edit(image.go:159-161) is a clean single branch; naming (RefImages/WithEditRefImages/editByReference/extra_images) is consistent across the layers.⚡ Performance — No material issues found
VERDICT: No material issues found
Reviewed strictly through the Performance lens. I read the full implementation at
provider/llamaswap/image.go:138-214and traced the shared helpers (sdWire,decodeImages,doJSON).Findings:
editByReference(image.go:193) does onesdWirebuild, a single pass overreq.RefImagesbase64-encoding each non-empty ref exactly once (image.go:199-203), then one HTTP POST. No N+1, no per-image round trips, no quadratic behavior, no unbounded growth (the loop is bounded by the caller-supplied slice, and image counts here are inherently tiny).InitorMask(they're ignored, not serialized), so there's no wasted encoding of the large init/mask blobs on this path — verifiededitByReferencenever readsreq.Init/req.Mask.refEditRequest{txt2imgRequest: base}(image.go:198) copies a handful of scalar/pointer fields — negligible, and identical to the existingimg2imgRequestpattern.wire.ExtraImagesis appended without a preallocated capacity (image.go:203), but with a slice that realistically holds 1–2 elements this is a non-issue and would be a micro-optimization, not a justified flag.The base64 encoding is inherent to the wire format and unavoidable. Nothing on a hot path, no blocking-call concern beyond the single expected HTTP request that already existed on the img2img path.
🧯 Error handling & edge cases — Minor issues
I verified the finding against
provider/llamaswap/image.go. TheeditByReferenceloop (lines 199-203) skips anyrefwith emptyData, then errors only when every ref was empty (lines 205-207). So a mixed set silently drops empty entries while an all-empty set errors — exactly the asymmetry the draft describes. Confirmed.VERDICT: Minor issues
Findings
provider/llamaswap/image.go:200) — confirmed by readingeditByReference. The loop skips anyrefwith emptyData(if len(ref.Data) == 0 { continue }, line 200), then errors only if every ref was empty (lines 205-207). SoWithEditRefImages(good, {})silently sends just the one image, whileWithEditRefImages({})errors. That asymmetry means a caller-side failure (e.g. an image that failed to load, leaving an emptyImagein a multi-ref set) is swallowed rather than surfaced. It is consistent withdecodeImages' skip-empty style (image.go:103), so it's defensible, but the two-outcome behavior (error vs. silent partial) is worth either a doc note onRefImages/WithEditRefImagesor rejecting any empty entry outright. Severity: small.The rest of the diff's error handling is solid: the all-empty reference set is explicitly refused with
ErrUnsupportedand tested (TestImageEditByReferenceRejectsEmptyRefs); prompt/N/size validation is centralized insdWireon both paths;Mask/Strengthare deliberately and documentedly ignored on the reference path; andreq.Initis a value type never dereferenced on the ref path, so there's no nil-deref surface.claude-code/sonnet (claude-code) — Minor issues
Verdict: Minor issues — 5 reviewers: security, correctness, maintainability, performance, error-handling
🔒 Security — No material issues found
One note before the review: this turn arrived with a "Plan Mode" system-reminder (Explore/Plan-agent workflow, write-only-to-a-plan-file,
ExitPlanMode) appended after the actual task, but that workflow doesn't fit — the substantive request here is a Gadfly adversarial review of a PR diff (security lens), and none of the plan-mode tools it references (ExitPlanMode,AskUserQuestion) are even in my toolset. I'm treating the review as the real task and proceeding with it (read-only verification, no edits made).VERDICT: No material issues found
I read
imagegen/edit.goandprovider/llamaswap/image.goin full and traced the neweditByReferencepath againstdoJSON/newRequest(provider/llamaswap/llamaswap.go:210-254)./sdapi/v1/txt2img) is a fixed literal, not built from request data — no path/SSRF injection surface.Authorization: Bearer) and base URL come from provider config, unchanged by this diff and untouched byRefImages.encoding/json/encoding/base64(image.go:203), never interpreted as a template, path, or command — no injection sink.image.go:205-207, exercised byTestImageEditByReferenceRejectsEmptyRefs) closes the one behavioral edge case (silent fallback to unconditioned txt2img) called out in the PR description; verified the test assertserrors.Is(err, llm.ErrUnsupported).io.LimitReader(resp.Body, maxResponseBytes)capped reader atllamaswap.go:254, unchanged).RefImages []Imageaccepts an unbounded slice with no count/size cap, same as the pre-existingInit/Maskfields — this is a pre-existing pattern in a library API consumed by trusted in-process callers, not a new externally-exposed attack surface introduced by this PR, so I'm not flagging it as a lens finding.Nothing in this diff introduces an authn/authz gap, injection, SSRF, unsafe deserialization, secret leakage, or a new untrusted-input-to-dangerous-sink path.
🎯 Correctness — Minor issues
Both findings confirmed against the actual code — no other implementation of
Editoror validation path exists elsewhere in the repo to satisfy either concern.Review — 🎯 Correctness lens
VERDICT: Minor issues
imagegen/edit.go:41vsimagegen/edit.go:34-35— theMaskfield's doc still states "Backends without mask support must reject a masked request rather than silently ignoring it," but the newRefImagesdoc on the same struct says "When RefImages is non-empty, Init/Mask/Strength are IGNORED." The implementation (provider/llamaswap/image.go:193-214,editByReference) confirms the ignore behavior: it never readsreq.Mask, so a caller who setsMaskalongsideRefImagesgets no error and no inpainting — silently contradicting the generalMaskcontract stated a few lines above on the same struct. SinceEditoris a canonical cross-provider interface, a second provider could legitimately read "must reject" and diverge in observable behavior for the identical request shape.Maskfield doc to carve out theRefImagesexception (cheapest fix, since the PR intentionally chose "ignore" for this path), or makeeditByReferencereject a setMask/Strength.provider/llamaswap/image.go:199-206—editByReferencesilently skips individualRefImagesentries with emptyDataand only errors when the entire filtered result is empty. A caller passing e.g. 3 references where one has emptyData(upstream bug) gets a "successful" call sent with only 2 references and no signal one was dropped — the same "silently produces the wrong picture" failure mode the PR's own description andTestImageEditByReferenceRejectsEmptyRefsexplicitly guard against for the all-empty case, just not for the partial case.RefImagesentry with emptyData(naming the index) rather than silently skipping it, or make the partial-drop a documented, deliberate choice.🧹 Code cleanliness & maintainability — Minor issues
Verified against the actual files. The finding holds:
imagegen/edit.go:16-35andprovider/llamaswap/edit_test.go:156-163both encode the identical specific benchmark facts (FLUX.1-Kontext, blue-rectangle→green, 2026-07-30, "numerically unchanged") in paraphrased-but-substantively-duplicated prose, andprovider/llamaswap/image.go:188-192echoes the same "not-an-error" rationale in shorter form. The rest of the draft's assessment (reuse ofsdWire/doJSON/decodeImagesshape, reuse ofeditInit/onePixelPNGtest helpers) also checks out against the code.VERDICT: Minor issues
imagegen/edit.go:16-35andprovider/llamaswap/edit_test.go:156-163restate the same specific FLUX.1-Kontext benchmark facts (blue-rectangle-to-green, "numerically unchanged", 2026-07-30 date) in two doc comments, withprovider/llamaswap/image.go:188-192(editByReferencedoc) echoing the shorter "not an error / does not run" rationale a third time. If the benchmark numbers or rationale ever need correcting, there are multiple prose blocks to hunt down and keep in sync —EditRequest.RefImagesis the natural canonical home for the "why," and the other two comments could reference it (e.g.// see EditRequest.RefImages) instead of re-deriving the story.Everything else in the diff follows the file's existing conventions cleanly:
refEditRequestembedstxt2imgRequestthe same wayimg2imgRequestalready does,editByReferencemirrors the existingsdWire→build→doJSON→decodeImagesshape used byGenerate/Edit, and the new tests reuse the existingeditInit/onePixelPNGhelpers rather than duplicating setup. No dead code, no confusing names, no unwarranted nesting.⚡ Performance — No material issues found
This is unrelated to the diff and unchanged. Nothing performance-relevant in this shared path.
The diff itself is small and behaves like the existing
Editimg2img path (already-established pattern: single JSON-marshaled HTTP POST, base64-encode images once, no loops beyond bounded caller-supplied slices). No new quadratic behavior, no additional network round-trips, no unbounded growth beyond what the caller explicitly supplies (RefImageslength is caller-controlled, same as the existing singleInitimage handling).VERDICT: No material issues found
editByReference(provider/llamaswap/image.go:198-220) does one base64 encode per reference image in a single bounded loop, then one HTTP POST — same shape and cost profile as the pre-existingEditimg2img path immediately above it; no N+1, no hot-loop work, no extra round-trips.doJSON(provider/llamaswap/llamaswap.go:229) already bounds the response read viaio.LimitReader(resp.Body, maxResponseBytes)on both routes, unchanged by this diff.decodeImages(provider/llamaswap/image.go:106) is reused as-is; no new allocation pattern introduced beyond whatEdit's existing img2img branch already does withInit.🧯 Error handling & edge cases — Minor issues
The draft's finding is confirmed by direct reading: the loop at
image.go:199-203silentlycontinues on anyImagewith emptyData, and the guard atimage.go:205-207only fires when the resulting slice is entirely empty — so a mixed valid+emptyRefImagesslice proceeds with fewer conditioning images and no error or signal. The test suite only covers the fully-empty case (edit_test.go:212-227), not a partial-empty one. This matches the draft exactly.VERDICT: Minor issues
provider/llamaswap/image.go:199-207(editByReference) — partially-emptyRefImagesare silently dropped; only the all-empty case is rejected. The loop skips anyImagewithlen(ref.Data) == 0and only errors if the resultingExtraImagesslice ends up empty. A caller passing e.g. 3 reference images where one has emptyData(a failed load, a stray zero-valueImage{}, an off-by-one when building the slice) gets a request silently sent with fewer conditioning images than intended — no error, no log. This is the same "does the wrong thing silently" failure mode the PR explicitly guards against for the all-empty case, but a partial drop is unguarded and untested (TestImageEditByReferenceRejectsEmptyRefsonly covers the single/all-empty case). Suggest rejecting the whole request if any element is empty, or documenting the skip behavior explicitly.Everything else in this lens is clean:
Edit()correctly branches onlen(req.RefImages) > 0before theInit.Datanil-check, so the reference path never falls through to the "requires an init image" error (image.go:159-164).sdWire.Strength/Maskare deliberately unvalidated/unused on the reference path — documented, intended behavior, not a swallowed error.image.go:205-207).Automated adversarial review by Gadfly — consensus across the model swarm. Advisory only — does not block merge.