feat(vision): back the vision tool with the provider's real vision model on NEAR AI - #4951
Closed
ilblackdragon wants to merge 1 commit into
Closed
ilblackdragon wants to merge 1 commit into
ilblackdragon wants to merge 1 commit into
Conversation
…del on NEAR AI The vision tool was registered with the single configured chat model (`suggest_vision_model(&[configured]).unwrap_or(configured)`), so when the configured NEAR AI model is text-only the image-analysis tool was wired to a model that cannot accept images. The substring `VISION_PATTERNS` heuristic is also unreliable for NEAR AI: across the 46 models the endpoint serves it disagrees with the API's authoritative modality metadata 15 times — missing the whole gpt-4.1/gpt-5/o-series and Qwen3-VL/Gemini-3 lineup, and wrongly flagging a text-only `claude-opus-4-6` deployment and `gemini-2.5-flash-lite`. Use the provider's published per-model modality instead: - nearai_chat: capture `metadata.architecture.inputModalities` into `ModelInfo` (+ `supports_image_input()`), and add `fetch_image_capable_models(base, key)` that returns the ids the endpoint marks image-capable. - vision_models: add `choose_vision_model(configured, image_capable)` — prefer the configured model if it is itself image-capable, else the best available by family (Claude > GPT > Gemini > Qwen > other), else the first capable one. - app.rs: on the NEAR AI path, select the vision-tool model from the verified image-capable set; fall back to the existing name heuristic on any fetch error or for non-NEAR-AI providers (which don't publish this metadata). Heuristic-only providers (OpenAI-direct, Bedrock, Ollama, openai_compatible) are unchanged.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughAdds metadata-driven vision model selection to the NEAR AI integration. ChangesVision Model Capability Detection
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
github-actions
Bot
added
size: L
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the NEAR AI vision tool wiring so it selects an actually image-capable model using the provider’s published per-model modality metadata (metadata.architecture.inputModalities), instead of relying solely on name heuristics that can misclassify models on NEAR AI.
Changes:
- Fetch NEAR AI model modality metadata at tool-registration time and choose a verified image-capable model for the vision tool (with best-effort fallback to heuristics).
- Add
ModelInfo.input_modalities+supports_image_input()and a newfetch_image_capable_models()helper for NEAR AI. - Add a pure, unit-tested
choose_vision_model(configured, image_capable)selector to rank among verified image-capable models.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/app.rs | Uses NEAR AI /v1/model/list modality metadata (best-effort) to select a real vision-capable model for vision tool registration. |
| crates/ironclaw_llm/src/vision_models.rs | Adds choose_vision_model to pick the configured model if image-capable, else rank within verified image-capable models; adds unit tests. |
| crates/ironclaw_llm/src/nearai_chat.rs | Captures inputModalities during model parsing and adds fetch_image_capable_models() based on that metadata; adds parsing test. |
| crates/ironclaw_llm/src/lib.rs | Re-exports fetch_image_capable_models. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+96
to
+99
| let cfg = configured.to_lowercase(); | ||
| if image_capable.iter().any(|m| m.to_lowercase() == cfg) { | ||
| return Some(configured); | ||
| } |
Comment on lines
+210
to
+212
| fn choose_returns_first_capable_when_no_family_matches() { | ||
| // A vision model the family-preference list does not name (e.g. a | ||
| // VL model) is still returned rather than dropped. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_llm/src/nearai_chat.rs`:
- Around line 934-973: The function fetch_image_capable_models currently returns
an empty Vec in both cases: when input modality metadata is absent and when the
endpoint explicitly declares text-only models. These should be distinguished
because the caller should only apply heuristic fallback (VISION_PATTERNS) when
metadata is unavailable, not when metadata explicitly excludes images. Modify
the return type to Result<Option<Vec<String>>, LlmError> where None indicates no
metadata was published and Some(vec) indicates authoritative metadata was found,
then update parse_nearai_models or the filtering logic to return None when no
modalities are present versus Some(empty_vec) or Some(filtered_vec) when
modalities are present. Finally, update the call site in app.rs to only apply
the VISION_PATTERNS fallback when the result is None, not when it is
Some(empty).
In `@crates/ironclaw_llm/src/vision_models.rs`:
- Around line 100-104: The FAMILY_PREFERENCE array in the vision_models.rs file
does not include the OpenAI o-series models (o3 and o4) that were added as
image-capable options. Add "o4" and "o3" to the FAMILY_PREFERENCE array and
position them before non-OpenAI families like "gemini" and "qwen" to ensure
OpenAI models are ranked with higher preference. Additionally, add a regression
test case to verify that when multiple image-capable models are available (such
as both gemini and o4-mini), the o-series model is correctly selected over the
non-OpenAI alternatives.
In `@src/app.rs`:
- Around line 676-714: The code in the AppBuilder::init_tools method calls
fetch_image_capable_models and passes the selected model to
register_vision_tools, but there is no test coverage verifying this integration
works end-to-end. Add a new #[tokio::test] that sets up a local stub for the
/v1/model/list endpoint, calls AppBuilder::init_tools with appropriate
configuration, and verifies that register_vision_tools was invoked with the
image-capable model selected by vision_models::choose_vision_model rather than
falling back to the name heuristic. This ensures the metadata-driven selection
path is actually exercised and the result is properly passed through to the
vision tools registration.
- Around line 687-712: The logic currently allows the name heuristic fallback to
run even when metadata fetch succeeds but returns no image-capable models, which
re-runs suggest_vision_model and can incorrectly select a text-only model.
Refactor the vision_model initialization to distinguish between three states:
metadata fetch failure (use heuristic), metadata fetch success with no capable
models (do not use heuristic), and metadata unavailable (use heuristic). The key
change is in the match block handling the result of fetch_image_capable_models
and the final unwrap_or_else call: only apply the heuristic fallback when the
fetch returns an Err, not when it returns Ok but choose_vision_model yields
None. This requires either tracking a separate flag to indicate authoritative
metadata or restructuring the logic so the fallback heuristic is only called
within the Err branch of the match expression, not in a blanket unwrap_or_else
after the match.
- Around line 684-687: The condition for use_modality_metadata in the
initialization block near the ironclaw_llm::fetch_image_capable_models call is
too broad. Instead of gating on provider.is_none(), which includes Bedrock,
Gemini OAuth, and OpenAI Codex, specifically check if the provider is the NEAR
AI backend. This ensures that only NEAR AI uses the metadata shape from the
/v1/model/list endpoint, while other dedicated backends continue using the
existing heuristic path as intended.
🪄 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 Plus
Run ID: 5028926e-65a3-4888-88b5-7c8e6e6e327f
📒 Files selected for processing (4)
crates/ironclaw_llm/src/lib.rscrates/ironclaw_llm/src/nearai_chat.rscrates/ironclaw_llm/src/vision_models.rssrc/app.rs
Comment on lines
+934
to
+973
| pub async fn fetch_image_capable_models( | ||
| base_url: &str, | ||
| api_key: &str, | ||
| ) -> Result<Vec<String>, LlmError> { | ||
| let base = base_url.trim_end_matches('/'); | ||
| let url = if base.ends_with("/v1") { | ||
| format!("{}/model/list", base) | ||
| } else { | ||
| format!("{}/v1/model/list", base) | ||
| }; | ||
|
|
||
| let response = Client::new() | ||
| .get(&url) | ||
| .header("Authorization", format!("Bearer {}", api_key)) | ||
| .timeout(std::time::Duration::from_secs(15)) | ||
| .send() | ||
| .await | ||
| .map_err(|e| LlmError::RequestFailed { | ||
| provider: "nearai_chat".to_string(), | ||
| reason: format!("Failed to fetch model list: {}", e), | ||
| })?; | ||
|
|
||
| if !response.status().is_success() { | ||
| return Err(LlmError::RequestFailed { | ||
| provider: "nearai_chat".to_string(), | ||
| reason: format!("Model list endpoint returned HTTP {}", response.status()), | ||
| }); | ||
| } | ||
|
|
||
| let body = response.text().await.map_err(|e| LlmError::RequestFailed { | ||
| provider: "nearai_chat".to_string(), | ||
| reason: format!("Failed to read model list response: {}", e), | ||
| })?; | ||
|
|
||
| Ok(parse_nearai_models(&body) | ||
| .into_iter() | ||
| .filter(ModelInfo::supports_image_input) | ||
| .map(|m| m.name) | ||
| .collect()) | ||
| } |
There was a problem hiding this comment.
Do not collapse “no metadata” with “explicitly text-only”.
fetch_image_capable_models() returns Vec::new() both when inputModalities is absent and when the endpoint explicitly reports only ["text"]; src/app.rs then falls back to VISION_PATTERNS, which can reselect the false-positive text-only models this PR is meant to avoid. Return a typed result such as Result<Option<Vec<String>>, LlmError> (None = no modality metadata published, Some(vec) = authoritative metadata seen) or include a metadata_seen flag, then only allow heuristic fallback for the unknown-metadata case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_llm/src/nearai_chat.rs` around lines 934 - 973, The function
fetch_image_capable_models currently returns an empty Vec in both cases: when
input modality metadata is absent and when the endpoint explicitly declares
text-only models. These should be distinguished because the caller should only
apply heuristic fallback (VISION_PATTERNS) when metadata is unavailable, not
when metadata explicitly excludes images. Modify the return type to
Result<Option<Vec<String>>, LlmError> where None indicates no metadata was
published and Some(vec) indicates authoritative metadata was found, then update
parse_nearai_models or the filtering logic to return None when no modalities are
present versus Some(empty_vec) or Some(filtered_vec) when modalities are
present. Finally, update the call site in app.rs to only apply the
VISION_PATTERNS fallback when the result is None, not when it is Some(empty).
Comment on lines
+100
to
+104
| const FAMILY_PREFERENCE: &[&str] = &["claude", "gpt-5", "gpt-4", "gemini", "qwen"]; | ||
| for family in FAMILY_PREFERENCE { | ||
| if let Some(model) = image_capable | ||
| .iter() | ||
| .find(|m| m.to_lowercase().contains(family)) |
There was a problem hiding this comment.
Rank OpenAI o-series before non-OpenAI families.
The PR objective calls out o3 and o4-mini as image-capable NEAR AI models, but the preference list only matches gpt-*; a capable set containing google/gemini-* and openai/o4-mini will choose Gemini. Add o4/o3 (or an openai/ family rule) plus a regression case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/ironclaw_llm/src/vision_models.rs` around lines 100 - 104, The
FAMILY_PREFERENCE array in the vision_models.rs file does not include the OpenAI
o-series models (o3 and o4) that were added as image-capable options. Add "o4"
and "o3" to the FAMILY_PREFERENCE array and position them before non-OpenAI
families like "gemini" and "qwen" to ensure OpenAI models are ranked with higher
preference. Additionally, add a regression test case to verify that when
multiple image-capable models are available (such as both gemini and o4-mini),
the o-series model is correctly selected over the non-OpenAI alternatives.
Comment on lines
+676
to
714
| // Check for vision models. Prefer the provider's authoritative | ||
| // per-model modality metadata (NEAR AI publishes | ||
| // `inputModalities` on `/v1/model/list`) so the vision tool is | ||
| // backed by a real image-capable model even when the configured | ||
| // chat model is text-only. Only the NEAR AI path (no explicit | ||
| // `provider` override) speaks that endpoint shape; everything | ||
| // else falls back to the name heuristic. The fetch is | ||
| // best-effort — any error degrades to the heuristic. | ||
| let use_modality_metadata = self.config.llm.provider.is_none(); | ||
| let mut vision_model: Option<String> = None; | ||
| if use_modality_metadata { | ||
| match ironclaw_llm::fetch_image_capable_models(&api_base, &api_key).await { | ||
| Ok(capable) => { | ||
| vision_model = ironclaw_llm::vision_models::choose_vision_model( | ||
| &model_name, | ||
| &capable, | ||
| ) | ||
| .map(str::to_string); | ||
| if vision_model.is_none() { | ||
| tracing::debug!( | ||
| "Model list reported no image-capable models; \ | ||
| falling back to name heuristic for vision tool" | ||
| ); | ||
| } | ||
| } | ||
| Err(e) => tracing::debug!( | ||
| error = %e, | ||
| "Could not fetch model modality metadata; \ | ||
| falling back to name heuristic for vision tool" | ||
| ), | ||
| } | ||
| } | ||
| let vision_model = vision_model.unwrap_or_else(|| { | ||
| ironclaw_llm::vision_models::suggest_vision_model(&models) | ||
| .unwrap_or(&model_name) | ||
| .to_string() | ||
| }); | ||
| tracing::debug!(vision_model = %vision_model, "Registering vision tool model"); | ||
| tools.register_vision_tools(api_base, api_key, vision_model, None); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
Add caller-side regression coverage for the registration path.
The helper tests do not prove AppBuilder::init_tools passes the metadata-selected model into register_vision_tools. Add a #[tokio::test] that drives this call path with a local /v1/model/list stub and verifies the registered vision tool uses the selected image-capable model. As per coding guidelines, "Test through the caller: when a helper gates a side effect, require a test driving the real call site (handler/factory/manager), not only the helper."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app.rs` around lines 676 - 714, The code in the AppBuilder::init_tools
method calls fetch_image_capable_models and passes the selected model to
register_vision_tools, but there is no test coverage verifying this integration
works end-to-end. Add a new #[tokio::test] that sets up a local stub for the
/v1/model/list endpoint, calls AppBuilder::init_tools with appropriate
configuration, and verifies that register_vision_tools was invoked with the
image-capable model selected by vision_models::choose_vision_model rather than
falling back to the name heuristic. This ensures the metadata-driven selection
path is actually exercised and the result is properly passed through to the
vision tools registration.
Source: Coding guidelines
Comment on lines
+684
to
+687
| let use_modality_metadata = self.config.llm.provider.is_none(); | ||
| let mut vision_model: Option<String> = None; | ||
| if use_modality_metadata { | ||
| match ironclaw_llm::fetch_image_capable_models(&api_base, &api_key).await { |
There was a problem hiding this comment.
Gate modality metadata on the NEAR AI backend, not provider.is_none().
provider == None also covers dedicated backends such as Bedrock, Gemini OAuth, and OpenAI Codex, so this can call NEAR AI’s /v1/model/list path when the active backend does not speak that metadata shape. Keep non-NEAR providers on the existing heuristic path as the PR objective states.
Suggested direction
- let use_modality_metadata = self.config.llm.provider.is_none();
+ let use_modality_metadata = self.config.llm.provider.is_none()
+ && matches!(
+ self.config.llm.backend.as_str(),
+ "nearai" | "near_ai" | "near"
+ );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app.rs` around lines 684 - 687, The condition for use_modality_metadata
in the initialization block near the ironclaw_llm::fetch_image_capable_models
call is too broad. Instead of gating on provider.is_none(), which includes
Bedrock, Gemini OAuth, and OpenAI Codex, specifically check if the provider is
the NEAR AI backend. This ensures that only NEAR AI uses the metadata shape from
the /v1/model/list endpoint, while other dedicated backends continue using the
existing heuristic path as intended.
Comment on lines
+687
to
+712
| match ironclaw_llm::fetch_image_capable_models(&api_base, &api_key).await { | ||
| Ok(capable) => { | ||
| vision_model = ironclaw_llm::vision_models::choose_vision_model( | ||
| &model_name, | ||
| &capable, | ||
| ) | ||
| .map(str::to_string); | ||
| if vision_model.is_none() { | ||
| tracing::debug!( | ||
| "Model list reported no image-capable models; \ | ||
| falling back to name heuristic for vision tool" | ||
| ); | ||
| } | ||
| } | ||
| Err(e) => tracing::debug!( | ||
| error = %e, | ||
| "Could not fetch model modality metadata; \ | ||
| falling back to name heuristic for vision tool" | ||
| ), | ||
| } | ||
| } | ||
| let vision_model = vision_model.unwrap_or_else(|| { | ||
| ironclaw_llm::vision_models::suggest_vision_model(&models) | ||
| .unwrap_or(&model_name) | ||
| .to_string() | ||
| }); |
There was a problem hiding this comment.
Do not fall back to name heuristics after authoritative “no image-capable models”.
When the metadata fetch succeeds but returns no capable models, unwrap_or_else() re-runs suggest_vision_model(&[model_name]) and can pick the exact false-positive text-only deployment the metadata just ruled out. After the helper returns a metadata-presence signal, only use the heuristic on fetch failure or absent metadata; otherwise skip registering the vision tool or surface a clear “no image-capable model” state.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app.rs` around lines 687 - 712, The logic currently allows the name
heuristic fallback to run even when metadata fetch succeeds but returns no
image-capable models, which re-runs suggest_vision_model and can incorrectly
select a text-only model. Refactor the vision_model initialization to
distinguish between three states: metadata fetch failure (use heuristic),
metadata fetch success with no capable models (do not use heuristic), and
metadata unavailable (use heuristic). The key change is in the match block
handling the result of fetch_image_capable_models and the final unwrap_or_else
call: only apply the heuristic fallback when the fetch returns an Err, not when
it returns Ok but choose_vision_model yields None. This requires either tracking
a separate flag to indicate authoritative metadata or restructuring the logic so
the fallback heuristic is only called within the Err branch of the match
expression, not in a blanket unwrap_or_else after the match.
Member
Author
|
This is only relevant for legacy codebase. Going to close, won't fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The vision (image-analysis) tool is registered with the single configured chat model:
So if your configured NEAR AI model is text-only (e.g.
deepseek-v3.2), the vision tool gets wired to a model that cannot accept images. (The image-gen tool right above doesn't have this problem — it falls back to a known-good model,FLUX.2-klein-4B.)Separately, the name-based
VISION_PATTERNSheuristic is unreliable for NEAR AI. Checked against the 46 models the endpoint actually serves, the substring heuristic disagrees with the API's authoritative modality metadata 15 times:openai/gpt-4.1/gpt-5/o3/o4-minifamily,Qwen3-VL,gemini-3.5-flash,gemma-4,kimi-k2.6.claude-opus-4-6deployment andgemini-2.5-flash-litethat the endpoint reports as text-only — matching them would push images at a model that rejects them.Fix
Use the provider's own per-model modality metadata (
metadata.architecture.inputModalities) instead of guessing from names.nearai_chat.rs— captureinputModalitiesintoModelInfo(+supports_image_input()); addfetch_image_capable_models(base_url, api_key)returning the ids the endpoint marks image-capable.vision_models.rs— addchoose_vision_model(configured, image_capable): prefer the configured model if it's itself image-capable, else best-available by family (Claude > GPT > Gemini > Qwen > other), else the first capable one. (Pure + unit-tested.)app.rs— on the NEAR AI path, pick the vision-tool model from the verified image-capable set; fall back to the existing name heuristic on any fetch error or for non-NEAR-AI providers.Heuristic-only providers (OpenAI-direct, Bedrock, Ollama,
openai_compatible) are unchanged — they don't publish this metadata, so they keep usingVISION_PATTERNS.Notes / tradeoffs
provideroverride) speaks the/v1/model/listshape, so the metadata path is gated to it.Validation
cargo build -p ironclaw_llmandcargo build --bin ironclaw— cleancargo clippy -p ironclaw_llm --testsandcargo clippy --bin ironclaw— no warningschoose_vision_model(4 cases) +parse_models_captures_input_modalities_for_vision_detection; fullvision_modelssuite 11/11Context
Follow-up from review discussion on the merged #4871. Independent of #4945 (the post-merge review fixes).