feat(agent-runtime): support multiple images per turn#472
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughReplaces the hardcoded single-image assumption with configurable multi-image support: adds Changes
Sequence DiagramsequenceDiagram
participant Config as Config Validator
participant Gate as Image Gate
participant Stage as Stage/Emit
participant Observe as Observer
participant Provider as Provider
Note over Config: Startup: validate\nmax_images_per_turn ∈ [1,8]\ndefault=4 if omitted
rect rgba(100, 150, 200, 0.5)
Note over Gate,Provider: Per-turn flow
Gate->>Gate: Check image_count ≤ effective_max
alt Over limit
Gate->>Observe: Emit rejection (attempted_count, effective_limit, images=[])
Observe-->>Gate: TooManyImages
else Within limit
Gate->>Stage: Stage all images in order
Stage->>Observe: Emit ingress (max_images_per_turn, images[], total_byte_len)
Observe-->>Stage: ✓
Stage->>Provider: Dispatch [StagedImage] slice (ordered)
Provider->>Provider: Attach all images in order
Provider-->>Stage: ✓
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying corvus with
|
| Latest commit: |
90d5225
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://cc30121d.corvus-42x.pages.dev |
| Branch Preview URL: | https://feat-multi-image-per-turn-in.corvus-42x.pages.dev |
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-04-10 to 2026-04-10 |
Change the Ok(response) match arm in clients/agent-runtime/src/gateway/mod.rs to an expression-style arm and reformat its arguments across multiple lines for improved readability. No functional behavior was changed.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/agent-runtime/src/observability/traits.rs (1)
561-593:⚠️ Potential issue | 🟡 MinorAlign these fixtures with the emitted single-vs-multi-image contract.
Line 567 builds a 2-image event but still populates
mime_type/byte_len, while Line 632 builds a 1-image provider error and leaves them empty.clients/agent-runtime/src/channels/mod.rs:259-286only emits the top-level pair when there is exactly one staged image. These tests currently admit states the runtime never produces, so they weaken regression coverage around the new turn-level fields.Proposed fix
@@ let event = ImageIngressEvent { channel: "telegram".into(), provider: Some("gemini".into()), model: Some("gemini-2.0-flash".into()), outcome: ImageIngressOutcome::Admitted, reason: None, - image_count: 2, + image_count: 1, max_images_per_turn: None, - images: vec![ - ImageIngressImageMeta { - mime_type: "image/jpeg".into(), - byte_len: 204_800, - }, - ImageIngressImageMeta { - mime_type: "image/png".into(), - byte_len: 102_400, - }, - ], - total_byte_len: Some(307_200), + images: vec![ImageIngressImageMeta { + mime_type: "image/jpeg".into(), + byte_len: 204_800, + }], + total_byte_len: Some(204_800), mime_type: Some("image/jpeg".into()), byte_len: Some(204_800), }; @@ - assert_eq!(event.image_count, 2); - assert_eq!(event.images.len(), 2); - assert_eq!(event.images[1].mime_type, "image/png"); - assert_eq!(event.total_byte_len, Some(307_200)); + assert_eq!(event.image_count, 1); + assert_eq!(event.images.len(), 1); + assert_eq!(event.images[0].mime_type, "image/jpeg"); + assert_eq!(event.total_byte_len, Some(204_800)); assert_eq!(event.mime_type.as_deref(), Some("image/jpeg")); assert_eq!(event.byte_len, Some(204_800)); @@ let event = ImageIngressEvent { channel: "telegram".into(), provider: None, model: None, outcome: ImageIngressOutcome::ProviderError, reason: Some(ImageIngressReason::ProviderError), image_count: 1, max_images_per_turn: None, images: vec![ImageIngressImageMeta { mime_type: "image/webp".into(), byte_len: 333, }], total_byte_len: Some(333), - mime_type: None, - byte_len: None, + mime_type: Some("image/webp".into()), + byte_len: Some(333), };Also applies to: 626-645
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/observability/traits.rs` around lines 561 - 593, The test fixtures for ImageIngressEvent violate the runtime contract that top-level mime_type and byte_len are present only when exactly one image is staged: update the 2-image fixture (the ImageIngressEvent with image_count = 2 and two entries in images) to set mime_type and byte_len to None, and update the 1-image provider-error fixture to populate top-level mime_type and byte_len from the single ImageIngressImageMeta; locate and change the construction and assertions around ImageIngressEvent, ImageIngressImageMeta, image_count, mime_type and byte_len so assertions reflect that multi-image events have None for top-level mime_type/byte_len while single-image events expose them.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/channels/mod.rs`:
- Around line 4831-4887: The test
process_admits_four_images_and_preserves_provider_order currently only checks
counts so it won't detect reordering; modify the ImageAwareProvider stub
(provider_impl) to record a stable per-image identifier (e.g., part id or a
generated token from make_image_part) for each image received instead of only
incrementing image_counts, then update the test to assert the provider_impl's
recorded image sequence equals ["img-a","img-b","img-c","img-d"]; locate usages
around RecordingObserver/RecordingChannel and make_image_part to capture the id
in the provider's load/send handler and add an assertion that the outbound image
order is preserved.
- Around line 1655-1679: The per-image match repeatedly calls
build_telegram_channel/build_whatsapp_channel/build_discord_channel/build_slack_channel
inside the loop, causing repeated config/token cloning and client recreation;
instead, construct the appropriate channel fetcher once before the per-image
loop (using the same match on msg.channel.as_str() to produce a single fetcher
or boxed trait object that implements fetch_and_stage_image) and then call
fetch_and_stage_image(channel_handle, declared_mime.as_deref(), max_bytes).await
on that single fetcher for each image, ensuring you propagate the same
ok_or(media::ImageRejectionReason::FetchFailed)? logic at construction time so
the loop only performs the fetch calls and not builder work or heavy cloning.
- Around line 1618-1631: The emit_image_ingress call is currently passing None
for the turn-level max_images_per_turn, so admitted/provider outcome events lack
the new field; update the call site in mod.rs (the emit_image_ingress
invocation) to pass the effective max images limit (the same value used by
reject_image_turn) instead of None — e.g., replace the None argument with the
effective limit variable (such as route.max_images_per_turn or the same computed
value reject_image_turn uses) so emit_image_ingress receives and emits
max_images_per_turn.
In `@clients/agent-runtime/src/observability/log.rs`:
- Around line 510-517: The test fixture sets image_count: 2 but only provides
one ImageIngressImageMeta in the images vec and a single-image total_byte_len;
update the fixture used in log.rs so the images vector contains two
ImageIngressImageMeta entries (e.g., add a second image with appropriate
mime_type and byte_len), adjust total_byte_len to be the sum of both byte_len
values, and ensure mime_type/max_images_per_turn fields reflect the multi-image
scenario; change the entries near the image_count, images, total_byte_len,
mime_type, and max_images_per_turn fields so the aggregates and counts are
consistent.
In
`@openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.md`:
- Around line 9-19: Add a single blank line immediately after the "### In Scope"
and "### Out of Scope" subheadings so each heading is followed by an empty line
per markdown conventions; update the proposal.md content around the "### In
Scope" and "### Out of Scope" headings to insert the blank lines (preserve the
existing list items and spacing elsewhere).
In
`@openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.md`:
- Around line 1-6: Replace the existing second-level heading "## Verification
Report" with a top-level heading "# Verification Report" at the top of the
document (the section that currently shows Change:
multi-image-per-turn-ingestion) and ensure the file ends with a single trailing
newline (add a newline after the final line).
In `@openspec/specs/channel-image-ingestion/spec.md`:
- Around line 278-297: REQ-11 (Regression Coverage for Multi-Image Channel
Ingestion) is placed before REQ-10 (Deduplication), causing non-sequential
numbering; fix by either swapping the two requirement blocks so REQ-10 precedes
REQ-11 or renumbering the headers so they read sequentially (e.g., change REQ-11
→ REQ-10 and REQ-10 → REQ-11) and update any internal references to those
requirement IDs in the document to match the new ordering.
---
Outside diff comments:
In `@clients/agent-runtime/src/observability/traits.rs`:
- Around line 561-593: The test fixtures for ImageIngressEvent violate the
runtime contract that top-level mime_type and byte_len are present only when
exactly one image is staged: update the 2-image fixture (the ImageIngressEvent
with image_count = 2 and two entries in images) to set mime_type and byte_len to
None, and update the 1-image provider-error fixture to populate top-level
mime_type and byte_len from the single ImageIngressImageMeta; locate and change
the construction and assertions around ImageIngressEvent, ImageIngressImageMeta,
image_count, mime_type and byte_len so assertions reflect that multi-image
events have None for top-level mime_type/byte_len while single-image events
expose them.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9580fabb-3c97-4747-9c01-872a5beb504d
📒 Files selected for processing (18)
clients/agent-runtime/src/channels/media.rsclients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/observability/mod.rsclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/providers/anthropic.rsclients/agent-runtime/src/providers/compatible.rsclients/agent-runtime/src/providers/gemini.rsopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/design.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/runtime-image-pipeline/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/state.yamlopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/tasks.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.mdopenspec/specs/channel-image-ingestion/spec.mdopenspec/specs/runtime-image-pipeline/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: pr-checks
- GitHub Check: sonar
- GitHub Check: submit-gradle
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (8)
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/state.yamlclients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/providers/gemini.rsclients/agent-runtime/src/providers/compatible.rsclients/agent-runtime/src/providers/anthropic.rsclients/agent-runtime/src/observability/mod.rsopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/tasks.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/design.mdopenspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.mdclients/agent-runtime/src/channels/media.rsopenspec/specs/runtime-image-pipeline/spec.mdclients/agent-runtime/src/config/schema.rsopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/runtime-image-pipeline/spec.mdclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/providers/gemini.rsclients/agent-runtime/src/providers/compatible.rsclients/agent-runtime/src/providers/anthropic.rsclients/agent-runtime/src/observability/mod.rsclients/agent-runtime/src/channels/media.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/providers/gemini.rsclients/agent-runtime/src/providers/compatible.rsclients/agent-runtime/src/providers/anthropic.rsclients/agent-runtime/src/observability/mod.rsclients/agent-runtime/src/channels/media.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/providers/gemini.rsclients/agent-runtime/src/providers/compatible.rsclients/agent-runtime/src/providers/anthropic.rsclients/agent-runtime/src/observability/mod.rsclients/agent-runtime/src/channels/media.rsclients/agent-runtime/src/config/schema.rsclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rs
clients/agent-runtime/src/providers/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Providertrait insrc/providers/and register insrc/providers/mod.rsfactory when adding a new provider
Files:
clients/agent-runtime/src/providers/gemini.rsclients/agent-runtime/src/providers/compatible.rsclients/agent-runtime/src/providers/anthropic.rs
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/tasks.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/design.mdopenspec/specs/channel-image-ingestion/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.mdopenspec/specs/runtime-image-pipeline/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/runtime-image-pipeline/spec.md
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/media.rsclients/agent-runtime/src/channels/mod.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/config/schema.rs
🧠 Learnings (4)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
clients/agent-runtime/src/providers/gemini.rsopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/Cargo.toml : Do not add heavy dependencies for minor convenience; justify new crate additions
Applied to files:
clients/agent-runtime/src/observability/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/channels/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/agent-runtime/src/channels/mod.rs
🪛 LanguageTool
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/tasks.md
[style] ~34-~34: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...later image in the same turn fails. - Verify observability keeps mime_type/`byte_l...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/design.md
[style] ~99-~99: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... reaping - Construct StagedImageGuard only after the full staging loop and ignore ...
(ADVERB_REPETITION_PREMIUM)
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.md
[warning] 9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 16-16: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.md
[warning] 1-1: First line in a file should be a top-level heading
(MD041, first-line-heading, first-line-h1)
[warning] 122-122: Files should end with a single newline character
(MD047, single-trailing-newline)
🔇 Additional comments (22)
clients/agent-runtime/src/providers/compatible.rs (1)
1872-1925: Strong regression test for ordered multi-image serialization.This test correctly verifies both image order preservation and per-image base64 payload mapping in the built content blocks.
clients/agent-runtime/src/providers/anthropic.rs (1)
1439-1499: Good coverage for Anthropic multi-image ordering.The assertions validate correct ordering and per-image encoding across mixed MIME types on the last user turn.
clients/agent-runtime/src/providers/gemini.rs (1)
921-977: Gemini multi-image ordering test looks solid.This is a good end-to-end builder-level check that preserves image order and byte fidelity for
inline_data.openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/runtime-image-pipeline/spec.md (1)
1-145: Spec update is comprehensive and aligned with the runtime change.The archived requirements clearly capture ordered full-slice dispatch, configurable limits, deterministic over-limit behavior, and turn-level observability.
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/state.yaml (1)
1-12: Archive state metadata looks correct.Phase progression and terminal archive state are consistent with this change lifecycle.
clients/agent-runtime/src/observability/mod.rs (1)
21-22: Re-export update is correct.Adding
ImageIngressImageMetahere keeps the observability public surface aligned with the updated event schema.clients/agent-runtime/src/observability/log.rs (1)
239-241: Image ingress logging now correctly reflects multi-image turn metadata.Good update to include limit context and aggregate image metadata in the emitted event.
clients/agent-runtime/src/channels/media.rs (3)
14-18: LGTM on the new constants.Clear documentation and sensible defaults. The ceiling of 8 provides operator flexibility while preventing unbounded payload growth.
376-385: LGTM on the config-aware validation helper.The updated signature allows the effective limit to flow from config, removing the hardcoded single-image assumption. The
>comparison correctly admits counts equal to the limit.
623-640: Test coverage for boundary conditions is solid.Tests exercise both the default (4) and ceiling (8) limits, including the rejection edge cases (5 and 9).
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/tasks.md (1)
1-35: Task list is comprehensive and aligns with the implementation.All phases are well-structured covering config, gating, observability, provider regressions, and verification. The incomplete task 5.2 is correctly marked as archive-only.
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.md (1)
46-70: Spec compliance matrix is thorough.The 20/20 scenario coverage with explicit test mappings provides good traceability between requirements and verification.
openspec/specs/channel-image-ingestion/spec.md (2)
63-74: Clear specification of multi-image staging semantics.The requirements for preserving turn association and channel order are well-defined.
109-139: Deterministic rejection behavior is well-specified.The requirement that over-limit turns are rejected as a whole (no partial admission) with user-visible error reporting both count and limit provides clear implementation guidance.
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.md (1)
39-49: Risk mitigation strategies are sound.Provider adapter regression tests, additive observability fields, and bounded config with ceiling address the identified risks appropriately.
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/specs/channel-image-ingestion/spec.md (1)
1-111: Delta spec accurately captures the behavioral changes.The "Previously" annotations clearly document what changed from the MVP single-image behavior, and the scenarios align with the updated main spec.
openspec/specs/runtime-image-pipeline/spec.md (3)
57-75: Provider dispatch contract clearly mandates full slice preservation.The requirement that provider dispatch "MUST NOT collapse a multi-image turn to only the first staged image" directly addresses the MVP limitation.
200-248: Whole-turn rejection semantics are well-defined.The requirement "MUST NOT partially dispatch any subset of their staged images" ensures deterministic behavior and aligns with the channel-ingestion spec.
490-531: Multi-image observability and regression coverage requirements are complete.REQ-9 and REQ-10 ensure turn-level metadata fidelity and regression coverage for the new multi-image behavior.
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/design.md (3)
54-89: Observability schema evolution is well-designed.The decision to keep
mime_type/byte_lenonly for single-image turns while addingimages[]andtotal_byte_lenfor multi-image turns provides backward compatibility without misleading "first image wins" behavior.
91-104: Partial staging cleanup decision addresses the multi-image failure surface.With multi-image turns, mid-loop failures become a normal case. Best-effort cleanup inside
stage_channel_images()keeps temp file growth bounded without changing the overall RAII contract.
228-238: Testing strategy is comprehensive.Coverage spans config validation, count validation boundaries, gate-time rejection, partial staging cleanup, provider ordering, and single-image compatibility.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
openspec/specs/channel-image-ingestion/spec.md (1)
60-74:⚠️ Potential issue | 🟠 MajorThe validation order here no longer matches the runtime.
gate_and_stage_images()rejects over-limit turns beforestage_channel_images()fetches or validates any files. Specifying count validation after MIME/size would require unnecessary downloads and conflicts with the deterministic whole-turn rejection behavior described later in this document.As per coding guidelines,
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openspec/specs/channel-image-ingestion/spec.md` around lines 60 - 74, The spec's validation order is incorrect: runtime first enforces image-count limits in gate_and_stage_images() and rejects whole turns over multimodal.max_images_per_turn before stage_channel_images() fetches/validates files. Update the doc to match runtime by moving the count check (effective default 4 when multimodal.max_images_per_turn is omitted) to step 4a (first) so gate_and_stage_images() behavior is accurately described; keep MIME magic-byte sniffing and MAX_IMAGE_BYTES size checks as subsequent steps performed during stage_channel_images() while preserving staging details (StagedImage, SHA-256 naming, InlineBytes, channel_origin) and the requirement that admitted images per user turn remain ordered and associated with that turn.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/channels/mod.rs`:
- Around line 1690-1705: The runtime now treats Slack as an implemented
image-ingestion channel (see ChannelImageFetcher::Slack and
build_slack_channel), but the channel-image-ingestion spec (spec.md) still marks
Slack as planned/contract-only; either update the spec to document Slack's
staging path and contract details to match the runtime, or remove/guard the
Slack branch in the dispatcher so runtime behavior matches the documented
contract; ensure the fix touches the dispatcher branch using
ChannelImageFetcher::Slack and the channel-image-ingestion spec (spec.md) so
code and docs remain synchronized.
In
`@openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.md`:
- Around line 8-118: The markdown in verify-report.md jumps from H1 to H3 (e.g.,
"### Completeness", "### Build & Tests Execution"), causing MD001; fix by
promoting those H3 headings to H2 (change "###" to "##" for sections like
Completeness, Build & Tests Execution, Spec Compliance Matrix, Correctness,
Coherence, Issues Found, Verdict) or insert a single H2 placeholder under the H1
and keep sub-sections consistent so the document uses a proper H1→H2→H3
hierarchy throughout.
In `@openspec/specs/channel-image-ingestion/spec.md`:
- Around line 258-276: The spec currently requires rejected multi-image turns to
"describe every image in the turn", which conflicts with the implementation in
reject_image_turn() that emits attempted image_count and max_images_per_turn but
leaves images, total_byte_len, mime_type, and byte_len empty/None; update the
spec text and the two rejection scenarios so they require reporting outcome
`Rejected`, reason `TooManyImages`, and that observability MUST report the
attempted image_count and effective max_images_per_turn while MAY NOT include
per-image metadata or raw bytes (images, total_byte_len, mime_type, byte_len may
be omitted/None), referencing the rejection behavior implemented in
reject_image_turn().
---
Outside diff comments:
In `@openspec/specs/channel-image-ingestion/spec.md`:
- Around line 60-74: The spec's validation order is incorrect: runtime first
enforces image-count limits in gate_and_stage_images() and rejects whole turns
over multimodal.max_images_per_turn before stage_channel_images()
fetches/validates files. Update the doc to match runtime by moving the count
check (effective default 4 when multimodal.max_images_per_turn is omitted) to
step 4a (first) so gate_and_stage_images() behavior is accurately described;
keep MIME magic-byte sniffing and MAX_IMAGE_BYTES size checks as subsequent
steps performed during stage_channel_images() while preserving staging details
(StagedImage, SHA-256 naming, InlineBytes, channel_origin) and the requirement
that admitted images per user turn remain ordered and associated with that turn.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: c14528dd-9453-4eb5-9396-8dc131beb3f9
📒 Files selected for processing (7)
clients/agent-runtime/src/channels/mod.rsclients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/observability/traits.rsopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.mdopenspec/specs/channel-image-ingestion/spec.md
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: sonar
- GitHub Check: pr-checks
- GitHub Check: submit-gradle
- GitHub Check: Cloudflare Pages
🧰 Additional context used
📓 Path-based instructions (8)
clients/agent-runtime/src/{security,gateway,tools}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Treat
src/security/,src/gateway/,src/tools/as high-risk surfaces and never broaden filesystem/network execution scope without explicit policy checks
Files:
clients/agent-runtime/src/gateway/mod.rs
clients/agent-runtime/src/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
clients/agent-runtime/src/**/*.rs: Never log secrets, tokens, raw credentials, or sensitive payloads in any logging statements
Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rs
clients/agent-runtime/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Run
cargo fmt --all -- --check,cargo clippy --all-targets -- -D warnings, andcargo testfor code validation, or document which checks were skipped and why
Files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rs
clients/agent-runtime/src/{security,gateway,tools,config}/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Do not silently weaken security policy or access constraints; keep default behavior secure-by-default with deny-by-default where applicable
Files:
clients/agent-runtime/src/gateway/mod.rs
**/*.rs
⚙️ CodeRabbit configuration file
**/*.rs: Focus on Rust idioms, memory safety, and ownership/borrowing correctness.
Flag unnecessary clones, unchecked panics in production paths, and weak error context.
Prioritize unsafe blocks, FFI boundaries, concurrency races, and secret handling.
Files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/observability/log.rsclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rs
**/*
⚙️ CodeRabbit configuration file
**/*: Security first, performance second.
Validate input boundaries, auth/authz implications, and secret management.
Look for behavioral regressions, missing tests, and contract breaks across modules.
Files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/observability/log.rsopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.mdclients/agent-runtime/src/observability/traits.rsclients/agent-runtime/src/channels/mod.rsopenspec/specs/channel-image-ingestion/spec.md
**/*.{md,mdx}
⚙️ CodeRabbit configuration file
**/*.{md,mdx}: Verify technical accuracy and that docs stay aligned with code changes.
For user-facing docs, check EN/ES parity or explicitly note pending translation gaps.
Files:
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.mdopenspec/specs/channel-image-ingestion/spec.md
clients/agent-runtime/src/channels/**/*.rs
📄 CodeRabbit inference engine (clients/agent-runtime/AGENTS.md)
Implement
Channeltrait insrc/channels/with consistentsend,listen, andhealth_checksemantics and cover auth/allowlist/health behavior with tests
Files:
clients/agent-runtime/src/channels/mod.rs
🧠 Learnings (7)
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Preserve CLI contract unless change is intentional and documented; prefer explicit errors over silent fallback for unsupported critical paths
Applied to files:
clients/agent-runtime/src/gateway/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/channels/**/*.rs : Implement `Channel` trait in `src/channels/` with consistent `send`, `listen`, and `health_check` semantics and cover auth/allowlist/health behavior with tests
Applied to files:
clients/agent-runtime/src/gateway/mod.rsclients/agent-runtime/src/channels/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/**/*.rs : Run `cargo fmt --all -- --check`, `cargo clippy --all-targets -- -D warnings`, and `cargo test` for code validation, or document which checks were skipped and why
Applied to files:
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Inspect existing module and adjacent tests before editing; define scope boundary with one concern per PR and avoid mixed feature+refactor+infra patches
Applied to files:
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.md
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/**/*.rs : Avoid unnecessary allocations, clones, and blocking operations to maintain performance and efficiency
Applied to files:
clients/agent-runtime/src/channels/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/main.rs : Keep startup path lean and avoid heavy initialization in command parsing flow
Applied to files:
clients/agent-runtime/src/channels/mod.rs
📚 Learning: 2026-02-17T12:31:17.076Z
Learnt from: CR
Repo: dallay/corvus PR: 0
File: clients/agent-runtime/AGENTS.md:0-0
Timestamp: 2026-02-17T12:31:17.076Z
Learning: Applies to clients/agent-runtime/src/providers/**/*.rs : Implement `Provider` trait in `src/providers/` and register in `src/providers/mod.rs` factory when adding a new provider
Applied to files:
clients/agent-runtime/src/channels/mod.rs
🪛 markdownlint-cli2 (0.22.0)
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/verify-report.md
[warning] 8-8: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
🔇 Additional comments (6)
clients/agent-runtime/src/gateway/mod.rs (1)
2154-2157: Looks good — formatting-only change with no behavior drift.This preserves existing sanitization and success-path semantics in
handle_chat_stream.clients/agent-runtime/src/observability/log.rs (2)
239-241: Good additive multi-image telemetry logging.Lines 239-241 correctly log turn-level limit, ordered image metadata, and aggregate bytes without raw payload content.
511-524: Fixture now matches multi-image aggregates.The admitted ingress fixture is consistent (
image_count, twoimagesentries, andtotal_byte_lensum), which strengthens regression confidence.clients/agent-runtime/src/observability/traits.rs (2)
54-70: Event schema extension is clean and backward-compatible.Adding
images+total_byte_len+max_images_per_turnwhile retaining legacy single-image fields provides a safe migration path for observers.
807-862: Strong targeted coverage for multi-image turn semantics.These tests validate ordered per-image metadata and rejected-turn effective limits, which directly protects the new contract.
openspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/proposal.md (1)
1-64: Proposal scope and risk framing look solid.The document stays focused on the multi-image ingestion concern and keeps implementation, observability, and rollback expectations aligned.
|
1 participant
Related Issues
Closes #331
Refs #266
Summary
Tested Information
cargo fmt --manifest-path clients/agent-runtime/Cargo.toml --all -- --checkcargo clippy --manifest-path clients/agent-runtime/Cargo.toml --all-targets -- -D warningscargo test --manifest-path clients/agent-runtime/Cargo.toml multimodal_max_images_per_turncargo test --manifest-path clients/agent-runtime/Cargo.toml process_admits_four_images_and_preserves_provider_ordercargo test --manifest-path clients/agent-runtime/Cargo.toml process_rejects_when_image_count_exceeds_configured_limitcargo test --manifest-path clients/agent-runtime/Cargo.toml process_rejects_five_images_when_default_limit_is_omittedcargo test --manifest-path clients/agent-runtime/Cargo.toml stage_channel_images_cleans_up_partial_staging_on_failurecargo test --manifest-path clients/agent-runtime/Cargo.toml process_emits_provider_error_with_full_multi_image_metadatacargo test --manifest-path clients/agent-runtime/Cargo.toml build_multimodal_api_messages_preserves_all_images_in_ordercargo test --manifest-path clients/agent-runtime/Cargo.toml image_blocks_preserve_multiple_images_in_ordercargo test --manifest-path clients/agent-runtime/Cargo.toml build_gemini_contents_preserves_all_images_in_ordercargo test --manifest-path clients/agent-runtime/Cargo.toml image_ingress_multi_image_turn_uses_turn_level_fieldscargo test --manifest-path clients/agent-runtime/Cargo.toml image_ingress_rejected_turn_can_include_effective_limitDocumentation Impact
openspec/specs/channel-image-ingestion/spec.mdopenspec/specs/runtime-image-pipeline/spec.mdopenspec/changes/archive/2026-04-10-multi-image-per-turn-ingestion/Breaking Changes
Checklist