steve/majordomo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(media): drop oldest images on over-count instead of refusing #8

Merged
steve merged 3 commits from fix/image-overflow-drop-oldest into main 2026-06-28 22:43:20 +00:00
Conversation 9 Commits 3 Files Changed 2
+80 -24
3 Commits
Author SHA1 Message Date
steveandClaude Opus 4.8 70b7aebd86 test(media): match the overflow placeholder by const, not substring (gadfly #8)
CI / Tidy (pull_request) Successful in 9m25s
Details
CI / Build & Test (pull_request) Successful in 9m49s
Details 
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) <[email protected]>
 2026-06-28 18:33:01 -04:00
steveandClaude Opus 4.8 52bb910f4d media: address gadfly review — single-pass elide, drop helpers, stronger test
CI / Tidy (pull_request) Successful in 9m27s
Details
CI / Build & Test (pull_request) Successful in 9m41s
Details 
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) <[email protected]>
 2026-06-28 18:06:17 -04:00
steveandClaude Opus 4.8 d71aca4c3a fix(media): drop oldest images on over-count instead of refusing the request
Adversarial Review (Gadfly) / review (pull_request) Has been cancelled
Details
CI / Tidy (pull_request) Successful in 9m27s
Details
CI / Build & Test (pull_request) Successful in 9m44s
Details 
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) <[email protected]>
 2026-06-28 17:38:21 -04:00