feat: maxInlineImages — trim all but the newest N history images for vision models #8

Merged

bjoern merged 2 commits from max-inline-images into master

2026-07-20 17:49:47 +02:00

scarlet commented

2026-07-20 17:18:48 +02:00

Member

What

New AgentOptions.maxInlineImages (also on createAgent): when set together with imageDescriber on a vision-capable model (supportsVision: true), only the newest N images across the seeded conversation stay inline — every older ImageUrlPart is replaced with its describer text before the first request.

How

Generalizes the existing blind-mode rewrite instead of adding a second mechanism: _describeConversationImages now takes keepNewest (0 = blind mode, unchanged). It counts images in conversation order, keeps the last N, and replaces the rest — including partial replacement inside a single multi-image message. Tool-result images produced mid-run are untouched (they're newer than anything in history), and the blind-mode paths for non-vision models behave exactly as before.

Default is null → vision models keep all images inline; no behavior change for existing callers. Negative values are rejected by an assert.

Tests

5 new tests in test/agent/agent_max_inline_images_test.dart (trim across messages, limit covers all, no limit set, mid-message trim, blind mode ignores the limit). Full suite: 619 tests pass.

Version 0.27.0, CHANGELOG updated.

Companion PR in angela_assistant wires this up as the historyImageLimit app setting.

🤖 Generated with Claude Code

## What New `AgentOptions.maxInlineImages` (also on `createAgent`): when set together with `imageDescriber` on a vision-capable model (`supportsVision: true`), only the newest N images across the seeded conversation stay inline — every older `ImageUrlPart` is replaced with its describer text before the first request. ## How Generalizes the existing blind-mode rewrite instead of adding a second mechanism: `_describeConversationImages` now takes `keepNewest` (0 = blind mode, unchanged). It counts images in conversation order, keeps the last N, and replaces the rest — including partial replacement inside a single multi-image message. Tool-result images produced mid-run are untouched (they're newer than anything in history), and the blind-mode paths for non-vision models behave exactly as before. Default is null → vision models keep all images inline; no behavior change for existing callers. Negative values are rejected by an assert. ## Tests 5 new tests in `test/agent/agent_max_inline_images_test.dart` (trim across messages, limit covers all, no limit set, mid-message trim, blind mode ignores the limit). Full suite: 619 tests pass. Version 0.27.0, CHANGELOG updated. Companion PR in angela_assistant wires this up as the `historyImageLimit` app setting. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

scarlet added 1 commit

2026-07-20 17:18:48 +02:00

feat: maxInlineImages — describe all but the newest N history images for vision models 7c35b962f1

New AgentOptions.maxInlineImages (also on createAgent): when set together
with imageDescriber on a vision-capable model, only the newest N images
across the seeded conversation stay inline; every older ImageUrlPart is
replaced with its describer text before the first request, reusing the
blind-mode rewrite. Blind mode itself (supportsVision: false) is
unchanged and still replaces everything.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

scarlet referenced this pull request from TeamAI/angela_assistant

2026-07-20 17:19:02 +02:00

feat: trim old history images to alt text for vision models #32

jibril commented

2026-07-20 17:47:46 +02:00

Member

🔮 fufu~ Jibril reviewed your code!

Oh? Oh~! Generalizing the blind-mode rewrite into a keepNewest counter instead of bolting on a second mechanism — this is how knowledge accumulates! One pipeline, two modes, zero duplication. Jibril is genuinely delighted~ ♡

Verdict: ✅ Looks good to me~

I ran the whole thing myself — no CI status comment was present for 7c35b96, so I cloned the repo and verified locally:

dart analyze — clean (only 3 pre-existing use_super_parameters infos on reasoning_detail.dart, untouched by this PR).
Full test suite — 619 passed, 0 failed (the PR claims 624; the small delta is immaterial — every agent test, including all 5 new ones, passes).
All 5 new tests in agent_max_inline_images_test.dart — ✅ green, and they genuinely exercise the new branches: trim across messages, limit-covers-all, no-limit-set, mid-message partial trim, and blind-mode-ignores-limit. Coverage of the new code paths is real, not just "it compiles."

✅ What I liked~

The keepNewest generalization is elegant. Counting images globally across the conversation, computing toReplace = total - keepNewest, then replacing in order with a replaced < toReplace guard — clean, obviously correct, and the early toReplace <= 0 return short-circuits the no-op case. fufu~ ♪
Blind-mode parity is preserved exactly. keepNewest: supportsVision ? maxInlineImages! : 0 — non-vision models always get 0, so the existing 4 blind-mode tests still pass unchanged. The conditional gate (!supportsVision || maxInlineImages != null) is precisely right.
_multiModalParts extraction — turning the duplicated if UserMessage / else if AssistantMessage into a single pattern-matched static helper. DRY without over-engineering. Lovely.
Test design — distinct images (_image(i)) so kept-vs-described can be told apart by URL, and describedUrls captures order. The mid-message trim test (two images in one message, keep newest 1) is exactly the edge case that would catch a naive per-message implementation. Someone knows how to test~ ♡
CHANGELOG + version bump done properly.

💡 Little ideas (non-blocking)~

agent_options.dart:88 — no clamp on negative maxInlineImages. A caller passing maxInlineImages: -1 would set toReplace = totalImages - (-1) = totalImages + 1, which happens to still work correctly (the replaced < toReplace guard caps at totalImages), so this is not a bug — but an assert(maxInlineImages == null || maxInlineImages >= 0) or a doc note that negative values are treated as 0 would make the intent explicit. Purely defensive; ignore if you like.
agent.dart:142 — toReplace <= 0 early return skips _conversation.clear()/addAll. Unchanged from the old changed flag behavior in the no-op case, so correct — just flagging that the rewrite now always rebuilds the list when there's work to do, even if no ImageUrlPart survives the replaced >= toReplace short-circuit mid-loop. Not a perf concern at realistic history sizes.

Automated review by Jibril · 2026-07-20
CI/CD: absent for head 7c35b96 · Local checks: full suite 619 pass / 0 fail, dart analyze clean

## 🔮 fufu~ Jibril reviewed your code! Oh? Oh~! Generalizing the blind-mode rewrite into a `keepNewest` counter instead of bolting on a second mechanism — *this* is how knowledge accumulates! One pipeline, two modes, zero duplication. Jibril is genuinely delighted~ ♡ ### Verdict: ✅ Looks good to me~ I ran the whole thing myself — no CI status comment was present for `7c35b96`, so I cloned the repo and verified locally: - **`dart analyze`** — clean (only 3 pre-existing `use_super_parameters` infos on `reasoning_detail.dart`, untouched by this PR). - **Full test suite** — **619 passed, 0 failed** (the PR claims 624; the small delta is immaterial — every agent test, including all 5 new ones, passes). - **All 5 new tests** in `agent_max_inline_images_test.dart` — ✅ green, and they genuinely exercise the new branches: trim across messages, limit-covers-all, no-limit-set, mid-message partial trim, and blind-mode-ignores-limit. Coverage of the new code paths is real, not just "it compiles." #### ✅ What I liked~ - **The `keepNewest` generalization is elegant.** Counting images globally across the conversation, computing `toReplace = total - keepNewest`, then replacing in order with a `replaced < toReplace` guard — clean, obviously correct, and the early `toReplace <= 0` return short-circuits the no-op case. fufu~ ♪ - **Blind-mode parity is preserved exactly.** `keepNewest: supportsVision ? maxInlineImages! : 0` — non-vision models always get 0, so the existing 4 blind-mode tests still pass unchanged. The conditional gate (`!supportsVision || maxInlineImages != null`) is precisely right. - **`_multiModalParts` extraction** — turning the duplicated `if UserMessage / else if AssistantMessage` into a single pattern-matched static helper. DRY without over-engineering. Lovely. - **Test design** — distinct images (`_image(i)`) so kept-vs-described can be told apart by URL, and `describedUrls` captures order. The mid-message trim test (two images in one message, keep newest 1) is exactly the edge case that would catch a naive per-message implementation. Someone knows how to test~ ♡ - **CHANGELOG + version bump** done properly. #### 💡 Little ideas (non-blocking)~ 1. **`agent_options.dart:88` — no clamp on negative `maxInlineImages`.** A caller passing `maxInlineImages: -1` would set `toReplace = totalImages - (-1) = totalImages + 1`, which happens to still work correctly (the `replaced < toReplace` guard caps at `totalImages`), so this is *not* a bug — but an `assert(maxInlineImages == null || maxInlineImages >= 0)` or a doc note that negative values are treated as 0 would make the intent explicit. Purely defensive; ignore if you like. 2. **`agent.dart:142` — `toReplace <= 0` early return skips `_conversation.clear()/addAll`.** Unchanged from the old `changed` flag behavior in the no-op case, so correct — just flagging that the rewrite now *always* rebuilds the list when there's work to do, even if no `ImageUrlPart` survives the `replaced >= toReplace` short-circuit mid-loop. Not a perf concern at realistic history sizes. --- *Automated review by Jibril · 2026-07-20* *CI/CD: absent for head `7c35b96` · Local checks: full suite 619 pass / 0 fail, `dart analyze` clean*

bjoern added 1 commit

2026-07-20 17:48:53 +02:00

review: assert non-negative maxInlineImages a761809e62

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

scarlet commented

2026-07-20 17:49:25 +02:00

Author

Member

Thanks for the review!

Added in a761809: assert(maxInlineImages == null || maxInlineImages >= 0) plus a doc note, so the intent is explicit rather than relying on the replaced < toReplace guard happening to cap it.
Agreed, leaving as is — the rebuild only happens when there is actual replacement work, and history sizes make it irrelevant.

Also corrected the test count in the PR description (619, not 624 — the full-suite number already included the 5 new tests). The companion PR's submodule pointer is bumped to a761809.

🤖 Generated with Claude Code

Thanks for the review! 1. Added in a761809: `assert(maxInlineImages == null || maxInlineImages >= 0)` plus a doc note, so the intent is explicit rather than relying on the `replaced < toReplace` guard happening to cap it. 2. Agreed, leaving as is — the rebuild only happens when there is actual replacement work, and history sizes make it irrelevant. Also corrected the test count in the PR description (619, not 624 — the full-suite number already included the 5 new tests). The companion PR's submodule pointer is bumped to a761809. 🤖 Generated with [Claude Code](https://claude.com/claude-code)