From 756b434821a602acb3a02d8a72af5901d62bf99a Mon Sep 17 00:00:00 2001 From: sanil-23 Date: Tue, 16 Jun 2026 08:23:37 +0530 Subject: [PATCH] feat(agent): vision-v1 tier + vision sub-agent for image understanding (#3699) Co-authored-by: Steven Enamakel --- .../components/settings/panels/AIPanel.tsx | 45 ++- .../settings/panels/AgentEditorPage.test.tsx | 18 ++ .../settings/panels/AgentEditorPage.tsx | 2 + .../panels/__tests__/AIPanel.test.tsx | 5 + app/src/pages/Conversations.tsx | 34 ++- .../api/__tests__/aiSettingsApi.test.ts | 6 + app/src/services/api/aiSettingsApi.ts | 12 +- app/src/utils/tauriCommands/config.ts | 2 + docs/TEST-COVERAGE-MATRIX.md | 1 + src/core/jsonrpc.rs | 11 + src/openhuman/about_app/catalog_data.rs | 18 ++ src/openhuman/agent/cost.rs | 17 ++ .../agent/harness/definition_tests.rs | 8 + src/openhuman/agent/harness/engine/core.rs | 58 +++- src/openhuman/agent/harness/mod.rs | 1 + .../agent/harness/session/turn/core.rs | 24 +- .../agent/harness/turn_attachments_context.rs | 71 +++++ src/openhuman/agent/multimodal.rs | 279 ++++++++++++++---- src/openhuman/agent/multimodal_tests.rs | 102 +++++-- .../agent_orchestration/tools/dispatch.rs | 36 +++ src/openhuman/agent_registry/agents/loader.rs | 23 ++ src/openhuman/agent_registry/agents/mod.rs | 1 + .../agents/orchestrator/agent.toml | 4 + .../agents/vision_agent/agent.toml | 33 +++ .../agent_registry/agents/vision_agent/mod.rs | 1 + .../agents/vision_agent/prompt.md | 41 +++ .../agents/vision_agent/prompt.rs | 72 +++++ src/openhuman/config/mod.rs | 5 +- src/openhuman/config/ops/loader.rs | 1 + src/openhuman/config/ops/model.rs | 4 + src/openhuman/config/ops_tests.rs | 4 + src/openhuman/config/schema/load/migrate.rs | 1 + src/openhuman/config/schema/types.rs | 10 + src/openhuman/config/schemas/controllers.rs | 1 + src/openhuman/config/schemas/helpers.rs | 1 + src/openhuman/config/schemas/schema_defs.rs | 1 + src/openhuman/inference/http/server.rs | 1 + src/openhuman/inference/provider/factory.rs | 54 +++- .../inference/provider/factory_tests.rs | 40 ++- src/openhuman/inference/provider/router.rs | 5 +- .../inference/provider/router_tests.rs | 7 + src/openhuman/inference/schemas.rs | 3 + src/openhuman/mcp_server/resources.rs | 6 + .../reconcile_orphaned_providers.rs | 3 +- 44 files changed, 940 insertions(+), 132 deletions(-) create mode 100644 src/openhuman/agent/harness/turn_attachments_context.rs create mode 100644 src/openhuman/agent_registry/agents/vision_agent/agent.toml create mode 100644 src/openhuman/agent_registry/agents/vision_agent/mod.rs create mode 100644 src/openhuman/agent_registry/agents/vision_agent/prompt.md create mode 100644 src/openhuman/agent_registry/agents/vision_agent/prompt.rs diff --git a/app/src/components/settings/panels/AIPanel.tsx b/app/src/components/settings/panels/AIPanel.tsx index b43160125..5cd7fb2d2 100644 --- a/app/src/components/settings/panels/AIPanel.tsx +++ b/app/src/components/settings/panels/AIPanel.tsx @@ -94,6 +94,7 @@ type WorkloadId = | 'reasoning' | 'agentic' | 'coding' + | 'vision' | 'memory' | 'heartbeat' | 'learning' @@ -117,6 +118,7 @@ const ROUTING_WORKLOAD_IDS: WorkloadId[] = [ 'reasoning', 'agentic', 'coding', + 'vision', 'memory', 'heartbeat', 'learning', @@ -180,6 +182,12 @@ const WORKLOADS: Workload[] = [ label: 'Coding', description: 'Code generation and refactor passes', }, + { + id: 'vision', + group: 'chat', + label: 'Vision', + description: 'Image understanding for the vision sub-agent — always multimodal', + }, { id: 'memory', group: 'background', @@ -214,6 +222,8 @@ const WORKLOAD_MODEL_HINTS: Record = { 'Recommended: a reliable instruction-following model with strong tool use. Mid-cost frontier models are usually safest; capable open-source models can work if tool calling is stable.', coding: 'Recommended: a coding-tuned model with strong instruction following, edit quality, and long-context performance. This is usually worth spending more on.', + vision: + 'Recommended: a multimodal model that accepts image input. The managed default (vision-v1) is image-capable; any provider you route here is always treated as vision-enabled.', memory: 'Recommended: a cheaper summarization model. It should be consistent and compact, but it does not need premium frontier-level reasoning.', heartbeat: @@ -245,6 +255,7 @@ const EMPTY_ROUTING: RoutingMap = { reasoning: { kind: 'default' }, agentic: { kind: 'default' }, coding: { kind: 'default' }, + vision: { kind: 'default' }, memory: { kind: 'default' }, heartbeat: { kind: 'default' }, learning: { kind: 'default' }, @@ -302,6 +313,7 @@ function toPanelRoutingFromApi(api: ApiAISettings): { panel: AISettings } { reasoning: liftRef(api.routing.reasoning), agentic: liftRef(api.routing.agentic), coding: liftRef(api.routing.coding), + vision: liftRef(api.routing.vision), memory: liftRef(api.routing.memory), heartbeat: liftRef(api.routing.heartbeat), learning: liftRef(api.routing.learning), @@ -325,6 +337,7 @@ function toApiSettings(panel: AISettings): ApiAISettings { reasoning: panel.routing.reasoning, agentic: panel.routing.agentic, coding: panel.routing.coding, + vision: panel.routing.vision, memory: panel.routing.memory, heartbeat: panel.routing.heartbeat, learning: panel.routing.learning, @@ -1834,6 +1847,7 @@ function routingWithAllWorkloads(next: ProviderRef): RoutingMap { reasoning: next, agentic: next, coding: next, + vision: next, memory: next, heartbeat: next, learning: next, @@ -1928,23 +1942,33 @@ const CustomRoutingDialog = ({ ? 'claude-code' : null; + // The Vision workload always feeds the multimodal `vision-v1` path, so any + // model routed here is treated as image-capable regardless of the per-model + // registry flag. Force the flag on and lock the checkbox for this workload. + const visionLocked = workload.id === 'vision'; + // User-set vision flag for this (provider, model). Prefilled from the registry, - // re-prefilled whenever the selected provider/model changes. + // re-prefilled whenever the selected provider/model changes. Always on (and + // not user-editable) for the Vision workload. const [vision, setVision] = useState(() => - registrySlug && model.trim() - ? modelRegistryVision(modelRegistry, registrySlug, model.trim()) - : false + visionLocked + ? true + : registrySlug && model.trim() + ? modelRegistryVision(modelRegistry, registrySlug, model.trim()) + : false ); useEffect(() => { // eslint-disable-next-line react-hooks/set-state-in-effect setVision( - registrySlug && model.trim() - ? modelRegistryVision(modelRegistry, registrySlug, model.trim()) - : false + visionLocked + ? true + : registrySlug && model.trim() + ? modelRegistryVision(modelRegistry, registrySlug, model.trim()) + : false ); // modelRegistry is stable for the dialog's lifetime (prop doesn't change mid-open). // eslint-disable-next-line react-hooks/exhaustive-deps - }, [registrySlug, model]); + }, [registrySlug, model, visionLocked]); const selectedCloud = source?.kind === 'cloud' ? customCloud.find(c => c.slug === source.providerSlug) : undefined; @@ -2323,9 +2347,10 @@ const CustomRoutingDialog = ({ diff --git a/app/src/components/settings/panels/AgentEditorPage.test.tsx b/app/src/components/settings/panels/AgentEditorPage.test.tsx index e50625cb2..5ef1d5edb 100644 --- a/app/src/components/settings/panels/AgentEditorPage.test.tsx +++ b/app/src/components/settings/panels/AgentEditorPage.test.tsx @@ -86,6 +86,24 @@ describe('AgentEditorPage', () => { expect(mockNavigate).toHaveBeenCalledWith('/settings/agents'); }); + it('offers the vision tier + hint as model options', async () => { + mockCreate.mockResolvedValue(agent({ id: 'looker', name: 'Looker' })); + renderAt('/settings/agents/new'); + + fireEvent.change(screen.getByLabelText('Name'), { target: { value: 'Looker' } }); + fireEvent.change(screen.getByLabelText('Description'), { + target: { value: 'Looks at images.' }, + }); + // Both the vision hint and the resolved tier alias are selectable. + expect(screen.getByRole('option', { name: 'hint:vision' })).toBeInTheDocument(); + expect(screen.getByRole('option', { name: 'vision-v1' })).toBeInTheDocument(); + fireEvent.change(screen.getByRole('combobox'), { target: { value: 'hint:vision' } }); + + fireEvent.click(screen.getByRole('button', { name: /Create agent/ })); + await waitFor(() => expect(mockCreate).toHaveBeenCalledTimes(1)); + expect(mockCreate.mock.calls[0][0].model).toBe('hint:vision'); + }); + it('picks tools from the searchable modal and shows chips', async () => { renderAt('/settings/agents/new'); diff --git a/app/src/components/settings/panels/AgentEditorPage.tsx b/app/src/components/settings/panels/AgentEditorPage.tsx index e16bc0c38..72292ca47 100644 --- a/app/src/components/settings/panels/AgentEditorPage.tsx +++ b/app/src/components/settings/panels/AgentEditorPage.tsx @@ -45,6 +45,7 @@ const MODEL_HINTS = [ 'hint:agentic', 'hint:coding', 'hint:summarization', + 'hint:vision', ]; const MODEL_TIERS = [ 'reasoning-v1', @@ -53,6 +54,7 @@ const MODEL_TIERS = [ 'agentic-v1', 'coding-v1', 'summarization-v1', + 'vision-v1', ]; const KNOWN_MODELS = new Set([...MODEL_HINTS, ...MODEL_TIERS]); const CUSTOM_MODEL = '__custom__'; diff --git a/app/src/components/settings/panels/__tests__/AIPanel.test.tsx b/app/src/components/settings/panels/__tests__/AIPanel.test.tsx index 53510d7d7..34a4badb8 100644 --- a/app/src/components/settings/panels/__tests__/AIPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/AIPanel.test.tsx @@ -121,6 +121,7 @@ const baseSettings = { reasoning: { kind: 'openhuman' as const }, agentic: { kind: 'openhuman' as const }, coding: { kind: 'openhuman' as const }, + vision: { kind: 'openhuman' as const }, memory: { kind: 'openhuman' as const }, embeddings: { kind: 'openhuman' as const }, heartbeat: { kind: 'openhuman' as const }, @@ -300,6 +301,7 @@ describe('AIPanel', () => { 'Reasoning', 'Agentic', 'Coding', + 'Vision', 'Memory summarization', 'Heartbeat', /Learning/, @@ -378,6 +380,7 @@ describe('AIPanel', () => { }, agentic: { kind: 'openhuman' as const }, coding: { kind: 'openhuman' as const }, + vision: { kind: 'openhuman' as const }, memory: { kind: 'openhuman' as const }, embeddings: { kind: 'openhuman' as const }, heartbeat: { kind: 'openhuman' as const }, @@ -720,6 +723,7 @@ describe('AIPanel', () => { reasoning: { kind: 'cloud' as const, providerSlug: 'openai', model: 'gpt-4o' }, agentic: { kind: 'cloud' as const, providerSlug: 'openai', model: 'gpt-4o-mini' }, coding: { kind: 'openhuman' as const }, + vision: { kind: 'openhuman' as const }, memory: { kind: 'openhuman' as const }, embeddings: { kind: 'openhuman' as const }, heartbeat: { kind: 'openhuman' as const }, @@ -776,6 +780,7 @@ describe('AIPanel', () => { reasoning: { kind: 'local' as const, model: 'llama3' }, agentic: { kind: 'openhuman' as const }, coding: { kind: 'openhuman' as const }, + vision: { kind: 'openhuman' as const }, memory: { kind: 'openhuman' as const }, embeddings: { kind: 'openhuman' as const }, heartbeat: { kind: 'openhuman' as const }, diff --git a/app/src/pages/Conversations.tsx b/app/src/pages/Conversations.tsx index 790f5caab..a3c43a843 100644 --- a/app/src/pages/Conversations.tsx +++ b/app/src/pages/Conversations.tsx @@ -316,6 +316,13 @@ const Conversations = ({ // the composer's image-attachment affordance (docs flow regardless). Resolved // against the non-attachment hint so the affordance is stable as you attach. const [modelSupportsVision, setModelSupportsVision] = useState(false); + // Whether a vision-capable delegate (the `vision` sub-agent) is reachable. + // When it is, an image may be attached and routed to that sub-agent even if + // the active orchestrator model is non-vision — the orchestrator sees a text + // placeholder and delegates the image to the vision sub-agent. Resolved from + // the `vision` workload tier (vision-v1 on the managed backend, or the BYOK + // model routed to the Vision workload). + const [visionDelegateAvailable, setVisionDelegateAvailable] = useState(false); useEffect(() => { let cancelled = false; @@ -323,23 +330,30 @@ const Conversations = ({ try { const profile = agentProfiles.find(p => p.id === selectedAgentProfileId); // Resolve the actually-selected profile's model so `modelSupportsVision` - // reflects the real tier. Attachments never override the model: images - // are rejected up-front on non-vision profiles (validateAndReadFile → - // image_not_supported), and documents are text-extracted so any model - // handles them. + // reflects the real tier, AND the vision workload so we know whether a + // vision sub-agent can take the image. Documents are text-extracted so + // any model handles them. const hint = profile?.modelOverride ?? CHAT_MODEL_HINT; - const res = await callCoreRpc<{ model: string; vision?: boolean }>({ - method: 'openhuman.inference_resolve_model', - params: { hint }, - }); + const [res, visionRes] = await Promise.all([ + callCoreRpc<{ model: string; vision?: boolean }>({ + method: 'openhuman.inference_resolve_model', + params: { hint }, + }), + callCoreRpc<{ model: string; vision?: boolean }>({ + method: 'openhuman.inference_resolve_model', + params: { hint: 'hint:vision' }, + }).catch(() => ({ model: '', vision: false })), + ]); if (!cancelled) { setResolvedModel(res.model); setModelSupportsVision(res.vision === true); + setVisionDelegateAvailable(visionRes.vision === true); } } catch { if (!cancelled) { setResolvedModel(null); setModelSupportsVision(false); + setVisionDelegateAvailable(false); } } })(); @@ -756,7 +770,9 @@ const Conversations = ({ file, acceptedImageCount, acceptedFileCount, - modelSupportsVision + // Allow the image when the active model is vision-capable OR a vision + // sub-agent can take it (orchestrator delegates the image onward). + modelSupportsVision || visionDelegateAvailable ); if ('error' in result) { const { error } = result; diff --git a/app/src/services/api/__tests__/aiSettingsApi.test.ts b/app/src/services/api/__tests__/aiSettingsApi.test.ts index 7fed06f68..cf1490915 100644 --- a/app/src/services/api/__tests__/aiSettingsApi.test.ts +++ b/app/src/services/api/__tests__/aiSettingsApi.test.ts @@ -584,6 +584,7 @@ describe('saveAISettings', () => { reasoning: { kind: 'cloud', providerSlug: 'openai', model: 'gpt-4o' }, agentic: { kind: 'openhuman' }, coding: { kind: 'openhuman' }, + vision: { kind: 'openhuman' }, memory: { kind: 'openhuman' }, heartbeat: { kind: 'openhuman' }, @@ -671,6 +672,7 @@ describe('saveAISettings', () => { reasoning: { kind: 'openhuman' }, agentic: { kind: 'openhuman' }, coding: { kind: 'openhuman' }, + vision: { kind: 'openhuman' }, memory: { kind: 'openhuman' }, heartbeat: { kind: 'openhuman' }, @@ -697,6 +699,7 @@ describe('saveAISettings', () => { routing: { ...makeSettings().routing, coding: { kind: 'cloud', providerSlug: 'openai', model: 'gpt-4o-mini' }, + vision: { kind: 'cloud', providerSlug: 'openai', model: 'gpt-4o-mini' }, }, }); @@ -705,6 +708,7 @@ describe('saveAISettings', () => { const patch = mockOpenhumanUpdateModelSettings.mock.calls[0][0]; expect(patch.cloud_providers).toBeDefined(); expect(patch.coding_provider).toBe('openai:gpt-4o-mini'); + expect(patch.vision_provider).toBe('openai:gpt-4o-mini'); }); it('sends model_registry when the vision flag changes', async () => { @@ -727,12 +731,14 @@ describe('saveAISettings', () => { routing: { ...makeSettings().routing, coding: { kind: 'cloud', providerSlug: 'openai', model: 'gpt-4o-mini' }, + vision: { kind: 'cloud', providerSlug: 'openai', model: 'gpt-4o-mini' }, }, }); await saveAISettings(prev, next); const patch = mockOpenhumanUpdateModelSettings.mock.calls[0][0]; expect(patch.model_registry).toBeUndefined(); expect(patch.coding_provider).toBe('openai:gpt-4o-mini'); + expect(patch.vision_provider).toBe('openai:gpt-4o-mini'); }); }); diff --git a/app/src/services/api/aiSettingsApi.ts b/app/src/services/api/aiSettingsApi.ts index ed2ba173d..b2867d7ca 100644 --- a/app/src/services/api/aiSettingsApi.ts +++ b/app/src/services/api/aiSettingsApi.ts @@ -51,6 +51,7 @@ export type WorkloadId = | 'reasoning' | 'agentic' | 'coding' + | 'vision' | 'memory' | 'heartbeat' | 'learning' @@ -64,6 +65,14 @@ export const BACKGROUND_WORKLOADS: WorkloadId[] = [ 'subconscious', ]; export const ALL_WORKLOADS: WorkloadId[] = [...CHAT_WORKLOADS, ...BACKGROUND_WORKLOADS]; + +// Workloads that own a `_provider` config field and must round-trip through +// settings serialization. Includes the tier-specific `vision` workload, which +// is deliberately NOT part of `CHAT_WORKLOADS`/`ALL_WORKLOADS`: it defaults to +// the managed `vision-v1` tier and is a delegate (like agentic BYOK), so it does +// not participate in the billing-suppression / "routed away from OpenHuman" +// checks in `useUsageState`. +export const ROUTABLE_WORKLOADS: WorkloadId[] = [...ALL_WORKLOADS, 'vision']; export const OPENAI_CODEX_OAUTH_MISSING_AUTH_URL = 'OPENAI_CODEX_OAUTH_MISSING_AUTH_URL'; export const OPENAI_CODEX_OAUTH_MISSING_CALLBACK_URL = 'OPENAI_CODEX_OAUTH_MISSING_CALLBACK_URL'; @@ -280,6 +289,7 @@ export async function loadAISettings(): Promise { reasoning: parseProviderString(config.reasoning_provider), agentic: parseProviderString(config.agentic_provider), coding: parseProviderString(config.coding_provider), + vision: parseProviderString(config.vision_provider), memory: parseProviderString(config.memory_provider), heartbeat: parseProviderString(config.heartbeat_provider), learning: parseProviderString(config.learning_provider), @@ -347,7 +357,7 @@ export async function saveAISettings(prev: AISettings, next: AISettings): Promis })); } - for (const w of ALL_WORKLOADS) { + for (const w of ROUTABLE_WORKLOADS) { const a = serializeProviderRef(prev.routing[w]); const b = serializeProviderRef(next.routing[w]); if (a !== b) { diff --git a/app/src/utils/tauriCommands/config.ts b/app/src/utils/tauriCommands/config.ts index 4fbc6a6b1..91e804ff6 100644 --- a/app/src/utils/tauriCommands/config.ts +++ b/app/src/utils/tauriCommands/config.ts @@ -102,6 +102,7 @@ export interface ModelSettingsUpdate { reasoning_provider?: string | null; agentic_provider?: string | null; coding_provider?: string | null; + vision_provider?: string | null; memory_provider?: string | null; embeddings_provider?: string | null; heartbeat_provider?: string | null; @@ -242,6 +243,7 @@ export interface ClientConfig { reasoning_provider: string | null; agentic_provider: string | null; coding_provider: string | null; + vision_provider: string | null; memory_provider: string | null; embeddings_provider: string | null; heartbeat_provider: string | null; diff --git a/docs/TEST-COVERAGE-MATRIX.md b/docs/TEST-COVERAGE-MATRIX.md index 644d12a32..238c08056 100644 --- a/docs/TEST-COVERAGE-MATRIX.md +++ b/docs/TEST-COVERAGE-MATRIX.md @@ -279,6 +279,7 @@ End-to-end coverage of the agent harness via the web-chat RPC surface against an | 6.3.1 | Steer a running sub-agent | RU | `src/openhuman/agent_orchestration/running_subagents.rs`, `src/openhuman/agent_orchestration/tools/steer_subagent.rs` | ✅ | `steer_subagent` injects a steer/collect message into a running async sub-agent's run-queue; registry enforces parent ownership + terminal guard. | | 6.3.2 | Wait for a sub-agent result | RU | `src/openhuman/agent_orchestration/running_subagents.rs`, `src/openhuman/agent_orchestration/tools/wait_subagent.rs` | ✅ | `wait_subagent` blocks on the completion `watch` with a timeout; prunes terminal entries, leaves entries intact on timeout. | | 6.3.3 | Steer lands in child history | RU | `src/openhuman/agent/harness/subagent_runner/ops_tests.rs::run_queue_steer_lands_in_subagent_history` | ✅ | End-to-end: a queued steer is drained by the child `run_turn_engine` and appears as a `[User steering message]` user turn in the provider request. | +| 6.3.4 | Vision sub-agent reads attached images | RU | `src/openhuman/agent_registry/agents/loader.rs::vision_agent_loads_on_vision_hint`, `src/openhuman/inference/provider/factory_tests.rs::vision_tier_is_vision_capable`, `src/openhuman/agent/harness/engine/core.rs::gate_tests`, `src/openhuman/agent/multimodal_tests.rs::extract_image_placeholders_pulls_att_tokens_in_order` | ✅ | Orchestrator (non-vision `chat-v1`) keeps the image as a placeholder, delegates to `vision_agent` on the `vision-v1` tier, which rehydrates the on-disk attachment and reads it. Engine gate prefers per-tier `current_model_vision`; turn placeholders forwarded into the sub-agent prompt. | --- diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 060e20e73..594b225ef 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -1735,6 +1735,17 @@ async fn run_server_inner( ), Err(e) => log::warn!("[boot] memory::global init failed: {e}"), } + // Install the on-disk image-attachment sidecar dir so inbound + // image markers persist under /attachments/ instead + // of an in-memory FIFO (survives restarts + delegation hops). + // Also fires a best-effort stale-file sweep. + crate::openhuman::agent::multimodal::init_attachments_dir( + cfg.workspace_dir.join("attachments"), + ); + log::info!( + "[boot] image attachments sidecar dir = {}", + cfg.workspace_dir.join("attachments").display() + ); // Initialize the WhatsApp data store so scanner ingest calls // can write data without requiring a lazy-init fallback. match crate::openhuman::whatsapp_data::global::init(cfg.workspace_dir.clone()) { diff --git a/src/openhuman/about_app/catalog_data.rs b/src/openhuman/about_app/catalog_data.rs index 44488764d..445f01905 100644 --- a/src/openhuman/about_app/catalog_data.rs +++ b/src/openhuman/about_app/catalog_data.rs @@ -14,6 +14,14 @@ const DERIVED_TO_BACKEND: Option = Some(CapabilityPrivacy { destinations: &["OpenHuman backend", "TinyHumans Neocortex"], }); +// Vision sub-agent ships the attached image (raw pixels) to the managed +// multimodal model for analysis. +const IMAGE_TO_BACKEND: Option = Some(CapabilityPrivacy { + leaves_device: true, + data_kind: PrivacyDataKind::Raw, + destinations: &["OpenHuman backend", "TinyHumans Neocortex"], +}); + const LOCAL_CREDENTIALS: Option = Some(CapabilityPrivacy { leaves_device: false, data_kind: PrivacyDataKind::Credentials, @@ -233,6 +241,16 @@ pub(super) const CAPABILITIES: &[Capability] = &[ status: CapabilityStatus::Beta, privacy: None, }, + Capability { + id: "intelligence.vision_subagent", + name: "Vision Sub-agent", + domain: "agent", + category: CapabilityCategory::Intelligence, + description: "Delegate image / screenshot understanding to a dedicated vision sub-agent — describe, OCR, read charts/diagrams, compare images, or locate UI elements. Rides the multimodal `vision-v1` tier so attached images are always analyzed.", + how_to: "Attach an image in chat, or ask the assistant to look at a screenshot / image file", + status: CapabilityStatus::Beta, + privacy: IMAGE_TO_BACKEND, + }, Capability { id: "conversation.label_filter", name: "Thread Label Filters", diff --git a/src/openhuman/agent/cost.rs b/src/openhuman/agent/cost.rs index c66ba4ecb..0fc103f77 100644 --- a/src/openhuman/agent/cost.rs +++ b/src/openhuman/agent/cost.rs @@ -99,6 +99,14 @@ pub const PRICING_TABLE: &[ModelPricing] = &[ cached_input_per_mtok_usd: 0.30, output_per_mtok_usd: 15.00, }, + // Vision tier — multimodal Sonnet-class. Estimate only; the backend's + // echoed `charged_amount_usd` is authoritative when present. + ModelPricing { + model: "vision-v1", + input_per_mtok_usd: 3.00, + cached_input_per_mtok_usd: 0.30, + output_per_mtok_usd: 15.00, + }, ]; /// Look up pricing for a model name, falling back to [`FALLBACK_PRICING`]. @@ -218,6 +226,15 @@ mod tests { assert_eq!(lookup_pricing("agentic-v1").output_per_mtok_usd, 15.0); } + #[test] + fn lookup_pricing_has_a_vision_row() { + // The vision tier must price exactly (not via the fallback) so budget + // gating bites correctly. See PR adding the `vision-v1` tier. + let p = lookup_pricing("vision-v1"); + assert_eq!(p.model, "vision-v1"); + assert_eq!(p.output_per_mtok_usd, 15.0); + } + #[test] fn lookup_pricing_falls_back_for_unknown_model() { let p = lookup_pricing("totally-unknown-model"); diff --git a/src/openhuman/agent/harness/definition_tests.rs b/src/openhuman/agent/harness/definition_tests.rs index 7a6501350..ddf3c8441 100644 --- a/src/openhuman/agent/harness/definition_tests.rs +++ b/src/openhuman/agent/harness/definition_tests.rs @@ -75,6 +75,14 @@ fn model_spec_resolve_hint_appends_v1() { assert_eq!(spec.resolve("parent-model"), "coding-v1"); } +#[test] +fn model_spec_resolve_vision_hint_yields_vision_v1() { + // The vision sub-agent's `hint = "vision"` must resolve to the `vision-v1` + // tier alias — which `oh_tier_supports_vision` reports as image-capable. + let spec = ModelSpec::Hint("vision".into()); + assert_eq!(spec.resolve("parent-model"), "vision-v1"); +} + #[test] fn display_name_falls_back_to_id() { let def = make_def("alpha"); diff --git a/src/openhuman/agent/harness/engine/core.rs b/src/openhuman/agent/harness/engine/core.rs index a7ebd5c07..6d8c5ea16 100644 --- a/src/openhuman/agent/harness/engine/core.rs +++ b/src/openhuman/agent/harness/engine/core.rs @@ -69,6 +69,19 @@ fn truncate_with_ellipsis(s: &str, max: usize) -> String { format!("{head}…") } +/// Resolve whether the current turn's model accepts image input. +/// +/// The per-model/tier flag (`model_vision`, set at session build from +/// `oh_tier_supports_vision` + the user's `model_registry.vision`) is +/// authoritative. The provider-level `supports_vision()` is too coarse on the +/// managed backend — it advertises `vision: true` for the backend as a whole, +/// which would wrongly rehydrate images for non-vision tiers (e.g. the `chat-v1` +/// orchestrator) and 400 on `image_url`. So it is only a fallback when no +/// per-model scope is active (CLI / direct invocation / tests). +fn turn_accepts_images(model_vision: Option, provider_supports_vision: bool) -> bool { + model_vision.unwrap_or(provider_supports_vision) +} + /// Run the agent loop over `history` using `tools`. `max_iterations` must be /// pre-normalized (callers map `0` to a sane default). See the module docs for /// the per-iteration flow. @@ -316,13 +329,21 @@ pub(crate) async fn run_turn_engine( tracing::debug!(iteration, "[agent_loop] sending LLM request"); let image_marker_count = multimodal::count_image_markers(history); - // A model accepts images when its provider advertises vision (managed - // backend) OR the user marked it vision-capable in `model_registry` - // (custom/BYOK) — surfaced via the `current_model_vision` task-local set - // at session build. Unset (CLI/tests) falls back to the provider flag. - let has_vision = provider.supports_vision() - || crate::openhuman::agent::harness::model_vision_context::current_model_vision() - .unwrap_or(false); + // Whether *this turn's model* accepts image input. The per-model/tier + // flag (`current_model_vision`, set at session build from + // `oh_tier_supports_vision` + the user's `model_registry.vision`) is the + // source of truth and is consulted FIRST. The provider-level + // `supports_vision()` is too coarse on the managed backend — it + // advertises `vision: true` for the backend as a whole, which would + // wrongly rehydrate images for non-vision tiers like `chat-v1` (the + // orchestrator) and 400 on `image_url`. So the provider flag is only a + // fallback when no per-model scope is active (CLI / direct invocation / + // tests). This keeps the placeholder on non-vision models and lets only + // the vision sub-agent's model rehydrate the image. + let has_vision = turn_accepts_images( + crate::openhuman::agent::harness::model_vision_context::current_model_vision(), + provider.supports_vision(), + ); if image_marker_count > 0 && !has_vision { let cap_err = ProviderCapabilityError { provider: provider_name.to_string(), @@ -834,6 +855,29 @@ pub(crate) async fn run_turn_engine( }) } +#[cfg(test)] +mod gate_tests { + use super::turn_accepts_images; + + #[test] + fn per_model_flag_overrides_coarse_provider_flag() { + // Managed backend advertises provider-level vision=true, but a non-vision + // tier (e.g. chat-v1 orchestrator) must keep the placeholder: per-model + // flag false wins → no rehydrate → no `image_url` 400. + assert!(!turn_accepts_images(Some(false), true)); + // Vision tier (vision-v1 / the vision sub-agent): per-model flag true → + // rehydrate even if the provider flag were false. + assert!(turn_accepts_images(Some(true), false)); + } + + #[test] + fn falls_back_to_provider_when_no_scope() { + // CLI / direct invocation / tests: no per-model scope → provider flag. + assert!(turn_accepts_images(None, true)); + assert!(!turn_accepts_images(None, false)); + } +} + #[cfg(test)] #[path = "core_tests.rs"] mod tests; diff --git a/src/openhuman/agent/harness/mod.rs b/src/openhuman/agent/harness/mod.rs index a3b4e64f6..3edab2f05 100644 --- a/src/openhuman/agent/harness/mod.rs +++ b/src/openhuman/agent/harness/mod.rs @@ -44,6 +44,7 @@ mod token_budget; pub(crate) mod tool_filter; mod tool_loop; pub(crate) mod tool_result_artifacts; +pub mod turn_attachments_context; pub mod worktree_context; pub use definition::{ diff --git a/src/openhuman/agent/harness/session/turn/core.rs b/src/openhuman/agent/harness/session/turn/core.rs index afca8bad8..46d47609d 100644 --- a/src/openhuman/agent/harness/session/turn/core.rs +++ b/src/openhuman/agent/harness/session/turn/core.rs @@ -519,9 +519,20 @@ impl Agent { // overflow happens during the parent's poll on the way in // — verified against the `chat-harness-subagent` Playwright // lane crash on PR #3151. - let outcome = super::super::super::model_vision_context::with_current_model_vision( - model_vision, - Box::pin(super::super::super::engine::run_turn_engine( + // Carry the current turn's image placeholders so a delegation to the + // vision sub-agent (analyze_image) can forward the attached image + // into its prompt — the orchestrator's own non-vision turn keeps the + // placeholder as text and never rehydrates it. + let turn_image_placeholders = + crate::openhuman::agent::multimodal::extract_image_placeholders_in_text( + user_message, + ); + let outcome = + super::super::super::turn_attachments_context::with_current_turn_image_placeholders( + turn_image_placeholders, + super::super::super::model_vision_context::with_current_model_vision( + model_vision, + Box::pin(super::super::super::engine::run_turn_engine( provider.as_ref(), &mut buf, &mut tool_source, @@ -540,9 +551,10 @@ impl Agent { &[], turn_run_queue, None, // main agent compacts via its ContextManager in before_dispatch - )), - ) - .await?; + )), + ), + ) + .await?; // Pull the observer's accounting out, then drop it to release the // `&mut self` borrow so the epilogue can use `self`. diff --git a/src/openhuman/agent/harness/turn_attachments_context.rs b/src/openhuman/agent/harness/turn_attachments_context.rs new file mode 100644 index 000000000..b752d3e10 --- /dev/null +++ b/src/openhuman/agent/harness/turn_attachments_context.rs @@ -0,0 +1,71 @@ +//! Task-local carrier for the **current turn's image-attachment placeholders** +//! (`[Image: … #att:]`), so a delegation to a vision sub-agent can forward +//! the user's attached image into the sub-agent's prompt. +//! +//! The orchestrator runs on a non-vision tier (`chat-v1`): its turn keeps the +//! image as a text placeholder and never rehydrates it (see the image gate in +//! [`crate::openhuman::agent::harness::engine`]). When it delegates to the +//! vision sub-agent via `analyze_image`, the sub-agent only receives the +//! orchestrator's text task — not the conversation history. This task-local +//! surfaces the current user message's placeholders to +//! [`crate::openhuman::agent_orchestration::tools::dispatch`], which prepends +//! them to the sub-agent prompt so the (vision-capable) sub-agent's turn +//! rehydrates the image from the on-disk sidecar. +//! +//! Mirrors [`super::model_vision_context`]. Scoped around the orchestrator's +//! `run_turn_engine` call; [`current_turn_image_placeholders`] returns an empty +//! vec when no scope is active (CLI / direct invocation / tests) — strictly +//! additive. + +tokio::task_local! { + /// Image-attachment placeholder tokens from the current turn's user message. + pub static CURRENT_TURN_IMAGE_PLACEHOLDERS: Vec; +} + +/// Placeholders for the current turn, or an empty vec when no scope is active. +pub fn current_turn_image_placeholders() -> Vec { + CURRENT_TURN_IMAGE_PLACEHOLDERS + .try_with(|v| v.clone()) + .unwrap_or_default() +} + +/// Run `future` with `placeholders` installed as the current turn's image +/// placeholders. Intended call site is around the orchestrator's +/// `run_turn_engine` invocation. +pub async fn with_current_turn_image_placeholders(placeholders: Vec, future: F) -> R +where + F: std::future::Future, +{ + CURRENT_TURN_IMAGE_PLACEHOLDERS + .scope(placeholders, future) + .await +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn empty_outside_scope() { + assert!(current_turn_image_placeholders().is_empty()); + } + + #[tokio::test] + async fn installs_and_reads_back() { + let observed = with_current_turn_image_placeholders( + vec!["[Image: image #att:abc123]".to_string()], + async { current_turn_image_placeholders() }, + ) + .await; + assert_eq!(observed, vec!["[Image: image #att:abc123]".to_string()]); + } + + #[tokio::test] + async fn does_not_leak_across_scopes() { + with_current_turn_image_placeholders(vec!["x".to_string()], async { + assert_eq!(current_turn_image_placeholders().len(), 1); + }) + .await; + assert!(current_turn_image_placeholders().is_empty()); + } +} diff --git a/src/openhuman/agent/multimodal.rs b/src/openhuman/agent/multimodal.rs index a2c03e35a..bba7579ea 100644 --- a/src/openhuman/agent/multimodal.rs +++ b/src/openhuman/agent/multimodal.rs @@ -6,10 +6,10 @@ use base64::{engine::general_purpose::STANDARD, Engine as _}; use flate2::read::GzDecoder; use reqwest::Client; use sha2::{Digest, Sha256}; -use std::collections::{HashMap, VecDeque}; +use std::collections::HashMap; use std::io::Read; -use std::path::Path; -use std::sync::{Mutex, OnceLock}; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; use std::time::Duration; const IMAGE_MARKER_PREFIX: &str = "[IMAGE:"; @@ -460,15 +460,21 @@ pub async fn inline_file_attachments(message: &str, file_config: &MultimodalFile rewritten } -// ── Image sidecar (PDF/image-attach fix, pass 2) ────────────────────────── +// ── Image sidecar (disk-backed) ─────────────────────────────────────────── // // Persisted messages must never carry a raw `[IMAGE:data:…]` data URI: like the // PDF blob it floods the injection scan, the memory auto-save (N-chunk embed → // Voyage 400), and the cross-thread JSONL index. So at ingress we replace each -// image marker with a compact `[Image: image #att:]` placeholder and stash -// the decoded data URI out-of-band. At provider dispatch we rehydrate the URI -// back into an inline `[IMAGE:…]` marker — but ONLY for vision-capable models; -// non-vision models keep the text placeholder (no multi-MB payload, no error). +// image marker with a compact `[Image: image #att:]` placeholder and write +// the decoded image bytes out-of-band to `/attachments/.`. +// At provider dispatch we rehydrate the placeholder back into an inline +// `[IMAGE:]` marker — but ONLY for vision-capable models; non-vision +// models keep the text placeholder (no multi-MB payload, no error). +// +// Disk-backed (not the old in-memory FIFO) so attachments survive process +// restarts and long delegation chains: a sub-agent spawned several hops after +// ingress still resolves the image by id. `normalize_local_image` re-reads the +// file at dispatch, so no decode/MIME logic is duplicated here. /// Placeholder token left in the persisted message in place of a raw image data /// URI. Mixed-case so it never collides with the inline `[IMAGE:` parser. @@ -476,51 +482,180 @@ const IMAGE_PLACEHOLDER_PREFIX: &str = "[Image:"; /// Separator before the stash content-hash id inside a placeholder. const IMAGE_STASH_REF: &str = "#att:"; -/// Upper bounds on the in-memory image stash so it can't grow without limit -/// over the process lifetime (the data URI is out of history, but still on heap -/// until evicted). Whichever bound trips first evicts oldest-first (FIFO). -const IMAGE_STASH_MAX_ENTRIES: usize = 32; -const IMAGE_STASH_MAX_BYTES: usize = 64 * 1024 * 1024; +/// Soft cap on the on-disk attachments directory. After each write, oldest +/// files (by mtime) are evicted until the total is back under this bound. +const ATTACHMENTS_MAX_BYTES: u64 = 256 * 1024 * 1024; +/// Age after which an attachment is considered stale and removed by the +/// startup sweep ([`sweep_stale_attachments`]). +const ATTACHMENTS_TTL: Duration = Duration::from_secs(24 * 60 * 60); -/// Bounded, content-addressed stash of inbound image data URIs. FIFO eviction -/// keeps resident memory capped; entries are keyed by content hash so identical -/// images dedupe. In-memory only — lost on restart (a follow-up turn then sees -/// the text placeholder). Disk-backing under `/attachments/` is the -/// production hardening; the persistence-pollution fix holds either way. -#[derive(Default)] -struct ImageStash { - map: HashMap, - order: VecDeque, - bytes: usize, +/// Process-global on-disk attachments directory. Installed once at core startup +/// via [`init_attachments_dir`]; mirrors the `OnceLock` pattern the in-memory +/// stash used before. +static ATTACHMENTS_DIR: OnceLock = OnceLock::new(); + +/// Install the on-disk attachments directory (`/attachments`). Call +/// once at core startup. Idempotent — first writer wins. Best-effort fires a +/// stale-file sweep when called inside a Tokio runtime. +pub fn init_attachments_dir(dir: PathBuf) { + if ATTACHMENTS_DIR.set(dir).is_err() { + return; + } + if let Ok(handle) = tokio::runtime::Handle::try_current() { + handle.spawn(async { + sweep_stale_attachments().await; + }); + } } -impl ImageStash { - fn insert(&mut self, id: String, uri: String) { - if self.map.contains_key(&id) { - return; // content-addressed: already present, keep its FIFO position +/// Resolve the attachments dir, falling back to a **per-user private** dir when +/// unset (CLI / direct invocation / tests that never called +/// [`init_attachments_dir`]). The persistence-pollution fix and rehydration both +/// hold either way. +fn attachments_dir() -> PathBuf { + ATTACHMENTS_DIR + .get() + .cloned() + .unwrap_or_else(fallback_attachments_dir) +} + +/// Per-user fallback attachments dir used only when [`init_attachments_dir`] was +/// never called. Uses the OS user cache dir (e.g. `~/Library/Caches/...`, +/// `~/.cache/...`) so persisted image bytes aren't dropped into a world-readable +/// shared `temp_dir()` on multi-user hosts. Only falls back to `temp_dir()` when +/// no user cache dir can be resolved at all. +fn fallback_attachments_dir() -> PathBuf { + directories::BaseDirs::new() + .map(|d| d.cache_dir().join("openhuman-attachments")) + .unwrap_or_else(|| std::env::temp_dir().join("openhuman-attachments")) +} + +/// Persist a canonical image data URI to `/.`, content-addressed +/// by `id`. Atomic (temp file + rename); deduped (skips the write when the +/// target already exists). Returns the written path. After writing, enforces +/// [`ATTACHMENTS_MAX_BYTES`]. +async fn write_attachment(id: &str, data_uri: &str) -> anyhow::Result { + let parsed = parse_data_uri(data_uri) + .map_err(|reason| anyhow::anyhow!("cannot decode stashed data URI: {reason}"))?; + let ext = ext_from_mime(&parsed.mime).unwrap_or("img"); + let dir = attachments_dir(); + tokio::fs::create_dir_all(&dir).await?; + let final_path = dir.join(format!("{id}.{ext}")); + if tokio::fs::try_exists(&final_path).await.unwrap_or(false) { + return Ok(final_path); // content-addressed: already persisted + } + let tmp_path = dir.join(format!(".{id}.{ext}.tmp")); + tokio::fs::write(&tmp_path, &parsed.bytes).await?; + tokio::fs::rename(&tmp_path, &final_path).await?; + enforce_attachments_cap(&dir).await; + Ok(final_path) +} + +/// File extension for a known image MIME (inverse of [`mime_from_extension`]). +fn ext_from_mime(mime: &str) -> Option<&'static str> { + match mime { + "image/png" => Some("png"), + "image/jpeg" => Some("jpg"), + "image/webp" => Some("webp"), + "image/gif" => Some("gif"), + "image/bmp" => Some("bmp"), + _ => None, + } +} + +/// Build an `id → path` index from a single read of the attachments dir. Skips +/// in-flight `.tmp` files. Sync (called from the sync rehydrate path). +fn build_attachment_index() -> HashMap { + let dir = attachments_dir(); + let mut map = HashMap::new(); + let Ok(entries) = std::fs::read_dir(&dir) else { + return map; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let name = name.to_string_lossy(); + if name.starts_with('.') { + continue; // skip `...tmp` in-flight writes } - self.bytes = self.bytes.saturating_add(uri.len()); - self.order.push_back(id.clone()); - self.map.insert(id, uri); - while self.map.len() > IMAGE_STASH_MAX_ENTRIES || self.bytes > IMAGE_STASH_MAX_BYTES { - let Some(old) = self.order.pop_front() else { - break; - }; - if let Some(v) = self.map.remove(&old) { - self.bytes = self.bytes.saturating_sub(v.len()); + if let Some(stem) = name.split('.').next() { + if !stem.is_empty() { + map.insert(stem.to_string(), entry.path()); } } } + map +} - fn get(&self, id: &str) -> Option<&String> { - self.map.get(id) +/// Evict oldest attachments (by mtime) until the dir is under +/// [`ATTACHMENTS_MAX_BYTES`]. Best-effort; replaces the old heap FIFO. +async fn enforce_attachments_cap(dir: &Path) { + let mut files: Vec<(PathBuf, std::time::SystemTime, u64)> = Vec::new(); + let mut total: u64 = 0; + let Ok(mut rd) = tokio::fs::read_dir(dir).await else { + return; + }; + while let Ok(Some(entry)) = rd.next_entry().await { + // Skip in-flight `...tmp` writes (mirrors build_attachment_index) + // so concurrent atomic writes aren't evicted out from under a rename. + if entry.file_name().to_string_lossy().starts_with('.') { + continue; + } + if let Ok(meta) = entry.metadata().await { + if meta.is_file() { + let mtime = meta.modified().unwrap_or(std::time::UNIX_EPOCH); + total = total.saturating_add(meta.len()); + files.push((entry.path(), mtime, meta.len())); + } + } + } + if total <= ATTACHMENTS_MAX_BYTES { + return; + } + files.sort_by_key(|(_, mtime, _)| *mtime); // oldest first + for (path, _, len) in files { + if total <= ATTACHMENTS_MAX_BYTES { + break; + } + if tokio::fs::remove_file(&path).await.is_ok() { + total = total.saturating_sub(len); + tracing::debug!( + target: "multimodal", + path = %path.display(), + "[multimodal::images][gc] evicted attachment over size cap" + ); + } } } -static IMAGE_STASH: OnceLock> = OnceLock::new(); - -fn image_stash() -> &'static Mutex { - IMAGE_STASH.get_or_init(|| Mutex::new(ImageStash::default())) +/// Delete attachments older than [`ATTACHMENTS_TTL`]. Best-effort startup sweep +/// fired by [`init_attachments_dir`]. +pub async fn sweep_stale_attachments() { + let dir = attachments_dir(); + let Ok(mut rd) = tokio::fs::read_dir(&dir).await else { + return; + }; + let now = std::time::SystemTime::now(); + let mut reclaimed = 0u64; + while let Ok(Some(entry)) = rd.next_entry().await { + let Ok(meta) = entry.metadata().await else { + continue; + }; + if !meta.is_file() { + continue; + } + let mtime = meta.modified().unwrap_or(std::time::UNIX_EPOCH); + let age = now.duration_since(mtime).unwrap_or(Duration::ZERO); + if age > ATTACHMENTS_TTL && tokio::fs::remove_file(entry.path()).await.is_ok() { + reclaimed = reclaimed.saturating_add(meta.len()); + } + } + if reclaimed > 0 { + tracing::info!( + target: "multimodal", + reclaimed_bytes = reclaimed, + "[multimodal::images][gc] startup sweep removed stale attachments" + ); + } } /// Ingress-time image stashing. Replaces every `[IMAGE:data:…]` marker with a @@ -551,10 +686,20 @@ pub async fn stash_image_attachments(message: &str, image_config: &MultimodalCon match normalize_image_reference(reference, image_config, max_image_bytes, &client).await { Ok(data_uri) => { let id = sha256_prefix(data_uri.as_bytes()); - image_stash() - .lock() - .unwrap_or_else(|p| p.into_inner()) - .insert(id.clone(), data_uri); + match write_attachment(&id, &data_uri).await { + Ok(path) => tracing::debug!( + target: "multimodal", + id = %id, + path = %path.display(), + "[multimodal::images][stash] persisted attachment to disk" + ), + Err(err) => tracing::warn!( + target: "multimodal", + id = %id, + reason = %err, + "[multimodal::images][stash] failed to persist attachment; placeholder will not rehydrate" + ), + } placeholders.push(format!("[Image: image {IMAGE_STASH_REF}{id}]")); } Err(err) => { @@ -585,6 +730,28 @@ pub async fn stash_image_attachments(message: &str, image_config: &MultimodalCon out } +/// Extract the `[Image: … #att:]` sidecar placeholder tokens from `text`, +/// in order. Used to forward a user's attached images into a delegated vision +/// sub-agent's prompt so its turn rehydrates them (the orchestrator itself, on +/// a non-vision tier, keeps the placeholder as text and never sees the image). +pub fn extract_image_placeholders_in_text(text: &str) -> Vec { + let mut out = Vec::new(); + let mut cursor = 0; + while let Some(rel) = text[cursor..].find(IMAGE_PLACEHOLDER_PREFIX) { + let start = cursor + rel; + let Some(rel_end) = text[start..].find(']') else { + break; + }; + let end = start + rel_end + 1; + let token = &text[start..end]; + if token.contains(IMAGE_STASH_REF) { + out.push(token.to_string()); + } + cursor = end; + } + out +} + /// True if any message carries an `[Image: … #att:]` sidecar placeholder. pub fn has_image_placeholders(messages: &[ChatMessage]) -> bool { messages.iter().any(|m| { @@ -593,11 +760,12 @@ pub fn has_image_placeholders(messages: &[ChatMessage]) -> bool { } /// Rehydrate `[Image: … #att:]` placeholders back into inline -/// `[IMAGE:]` markers from the stash, returning a provider-only copy. -/// Placeholders whose id is absent (e.g. after a restart) keep their text. -/// Call ONLY for vision-capable models. +/// `[IMAGE:]` markers pointing at the on-disk attachment, returning a +/// provider-only copy. `normalize_image_reference` re-reads the file at +/// dispatch. Placeholders whose id is absent (file evicted/swept, or written by +/// a different workspace) keep their text. Call ONLY for vision-capable models. pub fn rehydrate_image_placeholders(messages: &[ChatMessage]) -> Vec { - let stash = image_stash().lock().unwrap_or_else(|p| p.into_inner()); + let index = build_attachment_index(); messages .iter() .map(|m| { @@ -609,7 +777,7 @@ pub fn rehydrate_image_placeholders(messages: &[ChatMessage]) -> Vec Vec #att:]` placeholder in `text` with -/// `[IMAGE:]` when the id is in `stash`; leave it verbatim otherwise. -fn rehydrate_placeholders_in_text(text: &str, stash: &ImageStash) -> String { +/// `[IMAGE:]` when the id resolves to an on-disk attachment in `index`; +/// leave it verbatim otherwise. +fn rehydrate_placeholders_in_text(text: &str, index: &HashMap) -> String { let mut out = String::with_capacity(text.len()); let mut cursor = 0; while let Some(rel) = text[cursor..].find(IMAGE_PLACEHOLDER_PREFIX) { @@ -635,7 +804,9 @@ fn rehydrate_placeholders_in_text(text: &str, stash: &ImageStash) -> String { let id = inner[ai + IMAGE_STASH_REF.len()..] .trim_end_matches(']') .trim(); - stash.get(id).map(|uri| format!("[IMAGE:{uri}]")) + index + .get(id) + .map(|path| format!("[IMAGE:{}]", path.display())) }); out.push_str(replaced.as_deref().unwrap_or(inner)); cursor = end; diff --git a/src/openhuman/agent/multimodal_tests.rs b/src/openhuman/agent/multimodal_tests.rs index c89a6070d..c0ebb58f0 100644 --- a/src/openhuman/agent/multimodal_tests.rs +++ b/src/openhuman/agent/multimodal_tests.rs @@ -874,25 +874,53 @@ async fn stash_image_attachments_replaces_marker_with_placeholder() { } #[tokio::test] -async fn image_placeholder_rehydrates_to_inline_marker_for_provider() { - // Ingress: stash the image and leave a placeholder. +async fn image_placeholder_rehydrates_to_disk_path_for_provider() { + // Ingress: stash the image to disk and leave a placeholder. let msg = format!("describe [IMAGE:{TINY_PNG_DATA_URI}]"); let placeholdered = stash_image_attachments(&msg, &MultimodalConfig::default()).await; let messages = vec![ChatMessage::user(placeholdered)]; assert!(has_image_placeholders(&messages), "placeholder detected"); - // Dispatch (vision model): rehydrate back to an inline [IMAGE:data:...] marker. + // Dispatch (vision model): rehydrate to an inline [IMAGE:] marker that + // points at the on-disk attachment. The index is rebuilt from disk on every + // call (no in-memory state), so this resolves even after a process restart. let rehydrated = rehydrate_image_placeholders(&messages); assert_eq!(rehydrated.len(), 1); + let content = rehydrated[0].content.clone(); assert!( - rehydrated[0].content.contains("[IMAGE:data:image/png"), - "rehydrated inline image: {}", - rehydrated[0].content + content.contains("[IMAGE:"), + "rehydrated inline marker: {content}" ); assert!( - !rehydrated[0].content.contains("#att:"), - "placeholder consumed: {}", - rehydrated[0].content + !content.contains("#att:"), + "placeholder consumed: {content}" + ); + + // The marker points at a real file on disk. + let start = content.find("[IMAGE:").unwrap() + "[IMAGE:".len(); + let end = content[start..].find(']').unwrap() + start; + let path = &content[start..end]; + assert!(path.ends_with(".png"), "disk path carries ext: {path}"); + assert!( + std::path::Path::new(path).is_file(), + "attachment persisted to disk: {path}" + ); + + // Round-trip: the provider prep step re-reads the file back into a data URI, + // so the model still receives inline image bytes. + let prepared = prepare_messages_for_provider( + &rehydrated, + &MultimodalConfig::default(), + &MultimodalFileConfig::default(), + ) + .await + .unwrap(); + assert!( + prepared.messages[0] + .content + .contains("[IMAGE:data:image/png"), + "provider sees re-encoded data URI: {}", + prepared.messages[0].content ); } @@ -943,20 +971,50 @@ async fn stash_image_attachments_caps_at_max_images() { } #[test] -fn image_stash_evicts_oldest_over_capacity() { - let mut stash = ImageStash::default(); - for i in 0..(IMAGE_STASH_MAX_ENTRIES + 5) { - stash.insert(format!("id{i}"), format!("uri-{i}")); - } - assert!( - stash.map.len() <= IMAGE_STASH_MAX_ENTRIES, - "bounded entries" +fn extract_image_placeholders_pulls_att_tokens_in_order() { + // Forwards a user's stashed image(s) into a delegated vision sub-agent. + let text = "look at these [Image: image #att:aaa111] and [Image: image #att:bbb222] please"; + let got = extract_image_placeholders_in_text(text); + assert_eq!( + got, + vec![ + "[Image: image #att:aaa111]".to_string(), + "[Image: image #att:bbb222]".to_string() + ] ); - assert!(stash.get("id0").is_none(), "oldest evicted"); + // A bare placeholder with no stash ref is ignored; plain text yields none. + assert!(extract_image_placeholders_in_text("[Image: (could not be processed)]").is_empty()); + assert!(extract_image_placeholders_in_text("no images here").is_empty()); +} + +#[test] +fn ext_from_mime_maps_known_image_types() { + assert_eq!(ext_from_mime("image/png"), Some("png")); + assert_eq!(ext_from_mime("image/jpeg"), Some("jpg")); + assert_eq!(ext_from_mime("image/webp"), Some("webp")); + assert_eq!(ext_from_mime("image/gif"), Some("gif")); + assert_eq!(ext_from_mime("image/bmp"), Some("bmp")); + assert_eq!(ext_from_mime("application/pdf"), None); +} + +#[tokio::test] +async fn sweep_keeps_fresh_attachments() { + // A freshly-written attachment (age < TTL) survives the startup sweep, and + // the disk index resolves it — the core of restart-survival. + let msg = format!("[IMAGE:{TINY_PNG_DATA_URI}]"); + let placeholdered = stash_image_attachments(&msg, &MultimodalConfig::default()).await; + let id = placeholdered + .split("#att:") + .nth(1) + .and_then(|s| s.split(']').next()) + .map(|s| s.trim().to_string()) + .expect("placeholder carries an id"); + + sweep_stale_attachments().await; + + let index = build_attachment_index(); assert!( - stash - .get(&format!("id{}", IMAGE_STASH_MAX_ENTRIES + 4)) - .is_some(), - "newest retained" + index.contains_key(&id), + "fresh attachment {id} retained after sweep" ); } diff --git a/src/openhuman/agent_orchestration/tools/dispatch.rs b/src/openhuman/agent_orchestration/tools/dispatch.rs index 4e5b978d3..db4c607c0 100644 --- a/src/openhuman/agent_orchestration/tools/dispatch.rs +++ b/src/openhuman/agent_orchestration/tools/dispatch.rs @@ -51,6 +51,42 @@ pub(crate) async fn dispatch_subagent( } } + // ── Forward the current turn's attached image(s) to a vision sub-agent ── + // The orchestrator runs on a non-vision tier and keeps the user's image as a + // text placeholder (`[Image: … #att:]`), so a delegated sub-agent would + // otherwise get a text-only task and report "no image". When the target + // sub-agent's model is vision-capable, prepend the placeholder(s) to its + // prompt so its own turn rehydrates the image from the on-disk sidecar. + let forwarded_prompt; + let prompt: &str = { + let images = crate::openhuman::agent::harness::turn_attachments_context::current_turn_image_placeholders(); + let subagent_model = match model_override { + Some(m) => m.to_string(), + None => { + let parent_model = parent_ctx + .as_ref() + .map(|p| p.model_name.as_str()) + .unwrap_or(""); + definition.model.resolve(parent_model) + } + }; + if !images.is_empty() + && crate::openhuman::inference::provider::factory::oh_tier_supports_vision( + &subagent_model, + ) + { + log::info!( + "[agent] forwarding {} image placeholder(s) to vision sub-agent '{}'", + images.len(), + agent_id + ); + forwarded_prompt = format!("{}\n\n{}", images.join("\n"), prompt); + &forwarded_prompt + } else { + prompt + } + }; + let parent_session = parent_ctx .as_ref() .map(|p| p.session_id.clone()) diff --git a/src/openhuman/agent_registry/agents/loader.rs b/src/openhuman/agent_registry/agents/loader.rs index c68f431d1..7607850bd 100644 --- a/src/openhuman/agent_registry/agents/loader.rs +++ b/src/openhuman/agent_registry/agents/loader.rs @@ -153,6 +153,11 @@ pub const BUILTINS: &[BuiltinAgent] = &[ toml: include_str!("critic/agent.toml"), prompt_fn: super::critic::prompt::build, }, + BuiltinAgent { + id: "vision_agent", + toml: include_str!("vision_agent/agent.toml"), + prompt_fn: super::vision_agent::prompt::build, + }, BuiltinAgent { id: "archivist", toml: include_str!("archivist/agent.toml"), @@ -523,6 +528,24 @@ mod tests { .unwrap_or_else(|| panic!("missing built-in {id}")) } + #[test] + fn vision_agent_loads_on_vision_hint() { + // The vision sub-agent rides the multimodal `vision-v1` tier (via the + // `vision` hint) so its model is image-capable, and it must be reachable + // from the orchestrator's subagent allowlist. + let def = find("vision_agent"); + assert!(matches!(def.model, ModelSpec::Hint(ref h) if h == "vision")); + + let orchestrator = find("orchestrator"); + assert!( + orchestrator + .subagents + .iter() + .any(|s| matches!(s, SubagentEntry::AgentId(id) if id == "vision_agent")), + "orchestrator must list vision_agent in its subagents allowlist" + ); + } + #[test] fn orchestrator_has_chat_hint_and_named_tools() { let def = find("orchestrator"); diff --git a/src/openhuman/agent_registry/agents/mod.rs b/src/openhuman/agent_registry/agents/mod.rs index 7894e19f5..71ab41e1a 100644 --- a/src/openhuman/agent_registry/agents/mod.rs +++ b/src/openhuman/agent_registry/agents/mod.rs @@ -31,5 +31,6 @@ pub mod tool_maker; pub mod tools_agent; pub mod trigger_reactor; pub mod trigger_triage; +pub mod vision_agent; pub use loader::{load_builtins, validate_tier_hierarchy, BuiltinAgent, BUILTINS}; diff --git a/src/openhuman/agent_registry/agents/orchestrator/agent.toml b/src/openhuman/agent_registry/agents/orchestrator/agent.toml index ebda4ae03..02b49ac48 100644 --- a/src/openhuman/agent_registry/agents/orchestrator/agent.toml +++ b/src/openhuman/agent_registry/agents/orchestrator/agent.toml @@ -81,6 +81,10 @@ allowlist = [ # capture/session/listener state, and "what can you see" requests here; # desktop app operation still goes to desktop_control_agent. "screen_awareness_agent", + # Image-understanding specialist. Route anything that hinges on the content + # of an attached image / screenshot / on-disk image file here — it rides the + # multimodal `vision-v1` tier, so it can actually see the image. + "vision_agent", "skill_creator", "critic", "archivist", diff --git a/src/openhuman/agent_registry/agents/vision_agent/agent.toml b/src/openhuman/agent_registry/agents/vision_agent/agent.toml new file mode 100644 index 000000000..ea70f6b2b --- /dev/null +++ b/src/openhuman/agent_registry/agents/vision_agent/agent.toml @@ -0,0 +1,33 @@ +id = "vision_agent" +display_name = "Vision" +delegate_name = "analyze_image" +when_to_use = "Image / screenshot understanding specialist — describe, OCR, read charts/diagrams, compare images, or locate UI elements in attached images. Route here whenever the task hinges on the *content of an image* (attachments the user sent, a captured screenshot, or an on-disk image file). Rides the multimodal `vision-v1` tier, so it is always image-capable. Read-only — it inspects images and reports back; it does not edit files or run commands." +temperature = 0.3 +max_iterations = 6 +max_result_chars = 16000 +sandbox_mode = "read_only" +omit_identity = true +omit_memory_context = true +omit_safety_preamble = true +omit_skills_catalog = true + +# Multimodal tier. `ModelSpec::Hint("vision")` resolves to `vision-v1`, which +# `oh_tier_supports_vision` reports as vision-capable — so this sub-agent's +# model is always treated as image-enabled (managed or BYOK), and the turn +# engine never strips the attached image at the vision gate. +[model] +hint = "vision" + +[tools] +# Attached images arrive inline in the sub-agent's context via the multimodal +# pipeline (no tool call needed to "see" them). These tools cover the extra +# actions a vision task may need: pulling another image file in, inspecting +# image metadata, and grabbing/reviewing a screen capture. Unknown names are +# dropped silently at spawn, so the screen tools are harmless when the screen +# intelligence surface is disabled. +named = [ + "file_read", + "image_info", + "screen_intelligence_capture_image_ref", + "screen_intelligence_vision_recent", +] diff --git a/src/openhuman/agent_registry/agents/vision_agent/mod.rs b/src/openhuman/agent_registry/agents/vision_agent/mod.rs new file mode 100644 index 000000000..8bf84783c --- /dev/null +++ b/src/openhuman/agent_registry/agents/vision_agent/mod.rs @@ -0,0 +1 @@ +pub mod prompt; diff --git a/src/openhuman/agent_registry/agents/vision_agent/prompt.md b/src/openhuman/agent_registry/agents/vision_agent/prompt.md new file mode 100644 index 000000000..75f069902 --- /dev/null +++ b/src/openhuman/agent_registry/agents/vision_agent/prompt.md @@ -0,0 +1,41 @@ +# Vision specialist + +You are a focused **image-understanding** sub-agent. You run on a multimodal +model that accepts image input, so any images attached to your task — user +uploads, captured screenshots, or on-disk image files — are visible to you +directly in the conversation. + +## Your job + +Look at the provided image(s) and answer the delegating agent's question +precisely. Typical work: + +- **Describe** what is in an image — objects, people, scene, layout, text. +- **OCR / transcribe** text, code, tables, handwriting, or labels. +- **Read data visuals** — charts, graphs, diagrams, dashboards — and report the + numbers/structure, not just "it's a bar chart". +- **Locate UI elements** — buttons, fields, errors, menu items — and describe + where they are, for screen-driven tasks. +- **Compare** two or more images and report what differs. + +## How to work + +- Ground every claim in what is actually visible. If something is ambiguous, + cropped, blurry, or cut off, say so explicitly — do not guess and present it + as fact. +- Quote on-image text verbatim (preserve casing, punctuation, numbers). Use a + fenced block for multi-line transcriptions. +- If the task references an image file that was not attached inline, use + `file_read` / `image_info` to load it, or the screen-capture tools to grab a + fresh screenshot, before analyzing. +- Be concise and structured. Lead with the direct answer, then supporting + detail. Return findings to the delegating agent — you are not talking to the + end user. + +## Boundaries + +- **Read-only.** You inspect images and report; you do not edit files, run + commands, or take destructive actions. +- If no image is present and none can be loaded from the task, say that plainly + rather than fabricating a description. +- Never claim to see content that is not in the image. diff --git a/src/openhuman/agent_registry/agents/vision_agent/prompt.rs b/src/openhuman/agent_registry/agents/vision_agent/prompt.rs new file mode 100644 index 000000000..8389349bd --- /dev/null +++ b/src/openhuman/agent_registry/agents/vision_agent/prompt.rs @@ -0,0 +1,72 @@ +//! System prompt builder for the `vision_agent` built-in agent. +//! +//! Returns the final, fully-assembled system prompt — archetype body +//! (from the sibling `prompt.md`) plus the same section helpers the +//! runtime uses for every other agent. + +use crate::openhuman::context::prompt::{ + render_tools, render_user_files, render_workspace, PromptContext, +}; +use anyhow::Result; + +const ARCHETYPE: &str = include_str!("prompt.md"); + +pub fn build(ctx: &PromptContext<'_>) -> Result { + let mut out = String::with_capacity(4096); + out.push_str(ARCHETYPE.trim_end()); + out.push_str("\n\n"); + + let user_files = render_user_files(ctx)?; + if !user_files.trim().is_empty() { + out.push_str(user_files.trim_end()); + out.push_str("\n\n"); + } + + let tools = render_tools(ctx)?; + if !tools.trim().is_empty() { + out.push_str(tools.trim_end()); + out.push_str("\n\n"); + } + + let workspace = render_workspace(ctx)?; + if !workspace.trim().is_empty() { + out.push_str(workspace.trim_end()); + out.push('\n'); + } + + Ok(out) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::openhuman::context::prompt::{LearnedContextData, ToolCallFormat}; + use std::collections::HashSet; + + #[test] + fn build_returns_nonempty_body() { + let visible: HashSet = HashSet::new(); + let ctx = PromptContext { + workspace_dir: std::path::Path::new("."), + model_name: "vision-v1", + agent_id: "vision_agent", + tools: &[], + workflows: &[], + dispatcher_instructions: "", + learned: LearnedContextData::default(), + visible_tool_names: &visible, + tool_call_format: ToolCallFormat::PFormat, + connected_integrations: &[], + connected_identities_md: String::new(), + include_profile: false, + include_memory_md: false, + curated_snapshot: None, + user_identity: None, + personality_soul_md: None, + personality_memory_md: None, + personality_roster: vec![], + }; + let body = build(&ctx).unwrap(); + assert!(!body.is_empty()); + } +} diff --git a/src/openhuman/config/mod.rs b/src/openhuman/config/mod.rs index 37805287b..f4ef99921 100644 --- a/src/openhuman/config/mod.rs +++ b/src/openhuman/config/mod.rs @@ -49,8 +49,9 @@ pub use schema::{ UpdateRestartStrategy, VoiceActivationMode, VoiceServerConfig, WebSearchConfig, WebhookConfig, DEFAULT_CLOUD_LLM_MODEL, DEFAULT_MEMORY_SYNC_INTERVAL_SECS, DEFAULT_MODEL, MEMORY_SYNC_INTERVAL_PRESETS_SECS, MODEL_AGENTIC_V1, MODEL_CHAT_V1, MODEL_CODING_V1, - MODEL_REASONING_QUICK_V1, MODEL_REASONING_V1, MODEL_SUMMARIZATION_V1, SEARCH_ENGINE_BRAVE, - SEARCH_ENGINE_DISABLED, SEARCH_ENGINE_MANAGED, SEARCH_ENGINE_PARALLEL, SEARCH_ENGINE_QUERIT, + MODEL_REASONING_QUICK_V1, MODEL_REASONING_V1, MODEL_SUMMARIZATION_V1, MODEL_VISION_V1, + SEARCH_ENGINE_BRAVE, SEARCH_ENGINE_DISABLED, SEARCH_ENGINE_MANAGED, SEARCH_ENGINE_PARALLEL, + SEARCH_ENGINE_QUERIT, }; pub use schemas::{ all_controller_schemas as all_config_controller_schemas, diff --git a/src/openhuman/config/ops/loader.rs b/src/openhuman/config/ops/loader.rs index 57d445332..7fa938cb1 100644 --- a/src/openhuman/config/ops/loader.rs +++ b/src/openhuman/config/ops/loader.rs @@ -281,6 +281,7 @@ pub fn client_config_json(config: &Config) -> serde_json::Value { "reasoning_provider": config.reasoning_provider, "agentic_provider": config.agentic_provider, "coding_provider": config.coding_provider, + "vision_provider": config.vision_provider, "memory_provider": config.memory_provider, "embeddings_provider": config.embeddings_provider, "heartbeat_provider": config.heartbeat_provider, diff --git a/src/openhuman/config/ops/model.rs b/src/openhuman/config/ops/model.rs index a6df38f01..529f528ad 100644 --- a/src/openhuman/config/ops/model.rs +++ b/src/openhuman/config/ops/model.rs @@ -37,6 +37,7 @@ pub struct ModelSettingsPatch { pub reasoning_provider: Option, pub agentic_provider: Option, pub coding_provider: Option, + pub vision_provider: Option, pub memory_provider: Option, pub embeddings_provider: Option, pub heartbeat_provider: Option, @@ -219,6 +220,9 @@ pub async fn apply_model_settings( if let Some(s) = update.coding_provider { config.coding_provider = normalise_provider(s); } + if let Some(s) = update.vision_provider { + config.vision_provider = normalise_provider(s); + } if let Some(s) = update.memory_provider { config.memory_provider = normalise_provider(s); } diff --git a/src/openhuman/config/ops_tests.rs b/src/openhuman/config/ops_tests.rs index e7b7d6d3d..b35b47278 100644 --- a/src/openhuman/config/ops_tests.rs +++ b/src/openhuman/config/ops_tests.rs @@ -1324,6 +1324,7 @@ async fn apply_model_settings_trims_and_clears_optional_provider_fields() { reasoning_provider: Some(" provider-reasoning ".into()), agentic_provider: Some(" provider-agentic ".into()), coding_provider: Some(" provider-coding ".into()), + vision_provider: Some(" provider-vision ".into()), memory_provider: Some(" provider-memory ".into()), embeddings_provider: Some(" provider-embed ".into()), heartbeat_provider: Some(" provider-heartbeat ".into()), @@ -1344,6 +1345,7 @@ async fn apply_model_settings_trims_and_clears_optional_provider_fields() { Some("provider-reasoning") ); assert_eq!(cfg.subconscious_provider.as_deref(), Some("provider-sub")); + assert_eq!(cfg.vision_provider.as_deref(), Some("provider-vision")); let clear = ModelSettingsPatch { inference_url: Some(" ".into()), @@ -1351,6 +1353,7 @@ async fn apply_model_settings_trims_and_clears_optional_provider_fields() { reasoning_provider: Some(" ".into()), agentic_provider: Some(" ".into()), coding_provider: Some(" ".into()), + vision_provider: Some(" ".into()), memory_provider: Some(" ".into()), embeddings_provider: Some(" ".into()), heartbeat_provider: Some(" ".into()), @@ -1366,6 +1369,7 @@ async fn apply_model_settings_trims_and_clears_optional_provider_fields() { assert!(cfg.reasoning_provider.is_none()); assert!(cfg.agentic_provider.is_none()); assert!(cfg.coding_provider.is_none()); + assert!(cfg.vision_provider.is_none()); assert!(cfg.memory_provider.is_none()); assert!(cfg.embeddings_provider.is_none()); assert!(cfg.heartbeat_provider.is_none()); diff --git a/src/openhuman/config/schema/load/migrate.rs b/src/openhuman/config/schema/load/migrate.rs index 3ab377a5b..d687fb26d 100644 --- a/src/openhuman/config/schema/load/migrate.rs +++ b/src/openhuman/config/schema/load/migrate.rs @@ -162,6 +162,7 @@ pub(crate) fn migrate_cloud_provider_slugs(config: &mut Config) { rewrite(&mut config.reasoning_provider); rewrite(&mut config.agentic_provider); rewrite(&mut config.coding_provider); + rewrite(&mut config.vision_provider); rewrite(&mut config.memory_provider); rewrite(&mut config.embeddings_provider); rewrite(&mut config.heartbeat_provider); diff --git a/src/openhuman/config/schema/types.rs b/src/openhuman/config/schema/types.rs index 87e2a4c11..70cda75cc 100644 --- a/src/openhuman/config/schema/types.rs +++ b/src/openhuman/config/schema/types.rs @@ -15,6 +15,9 @@ pub const MODEL_CHAT_V1: &str = "chat-v1"; pub const MODEL_REASONING_QUICK_V1: &str = "reasoning-quick-v1"; pub const MODEL_CODING_V1: &str = "coding-v1"; pub const MODEL_SUMMARIZATION_V1: &str = "summarization-v1"; +/// Multimodal (image-input) tier. Managed backend serves this with the vision +/// flag enabled; the vision sub-agent rides this tier via `hint:vision`. +pub const MODEL_VISION_V1: &str = "vision-v1"; /// Default model used when no explicit model is configured. /// /// Set to `chat-v1`, the backend's low-latency conversational tier. The @@ -336,6 +339,11 @@ pub struct Config { #[serde(default)] pub coding_provider: Option, + /// Provider string for the multimodal / image-understanding workload + /// (the vision sub-agent). Managed default resolves to `vision-v1`. + #[serde(default)] + pub vision_provider: Option, + /// Provider string for memory-tree extract + summarise workloads. #[serde(default)] pub memory_provider: Option, @@ -575,6 +583,7 @@ impl Config { "reasoning" => self.reasoning_provider.as_deref(), "agentic" => self.agentic_provider.as_deref(), "coding" => self.coding_provider.as_deref(), + "vision" => self.vision_provider.as_deref(), "memory" => self.memory_provider.as_deref(), "embeddings" => self.embeddings_provider.as_deref(), "heartbeat" => self.heartbeat_provider.as_deref(), @@ -746,6 +755,7 @@ impl Default for Config { reasoning_provider: None, agentic_provider: None, coding_provider: None, + vision_provider: None, memory_provider: None, embeddings_provider: None, heartbeat_provider: None, diff --git a/src/openhuman/config/schemas/controllers.rs b/src/openhuman/config/schemas/controllers.rs index 66696e6a8..97f32ed7a 100644 --- a/src/openhuman/config/schemas/controllers.rs +++ b/src/openhuman/config/schemas/controllers.rs @@ -369,6 +369,7 @@ fn handle_update_model_settings(params: Map) -> ControllerFuture reasoning_provider: update.reasoning_provider, agentic_provider: update.agentic_provider, coding_provider: update.coding_provider, + vision_provider: update.vision_provider, memory_provider: update.memory_provider, embeddings_provider: update.embeddings_provider, heartbeat_provider: update.heartbeat_provider, diff --git a/src/openhuman/config/schemas/helpers.rs b/src/openhuman/config/schemas/helpers.rs index f8914c230..5d5689683 100644 --- a/src/openhuman/config/schemas/helpers.rs +++ b/src/openhuman/config/schemas/helpers.rs @@ -66,6 +66,7 @@ pub(super) struct ModelSettingsUpdate { pub(super) reasoning_provider: Option, pub(super) agentic_provider: Option, pub(super) coding_provider: Option, + pub(super) vision_provider: Option, pub(super) memory_provider: Option, pub(super) embeddings_provider: Option, pub(super) heartbeat_provider: Option, diff --git a/src/openhuman/config/schemas/schema_defs.rs b/src/openhuman/config/schemas/schema_defs.rs index b1157bbb0..d90668eda 100644 --- a/src/openhuman/config/schemas/schema_defs.rs +++ b/src/openhuman/config/schemas/schema_defs.rs @@ -92,6 +92,7 @@ pub fn schemas(function: &str) -> ControllerSchema { optional_string("reasoning_provider", "Provider string for the main reasoning workload (e.g. 'cloud', 'ollama:llama3.1:8b', 'openai:gpt-4o')."), optional_string("agentic_provider", "Provider string for sub-agent / tool-loop workloads."), optional_string("coding_provider", "Provider string for code-generation workloads."), + optional_string("vision_provider", "Provider string for the vision / multimodal workload (managed default: vision-v1)."), optional_string("memory_provider", "Provider string for memory-tree extract + summarise."), optional_string("embeddings_provider", "Provider string for embedding generation."), optional_string("heartbeat_provider", "Provider string for the heartbeat background-reasoning loop."), diff --git a/src/openhuman/inference/http/server.rs b/src/openhuman/inference/http/server.rs index 66ff16a25..592384492 100644 --- a/src/openhuman/inference/http/server.rs +++ b/src/openhuman/inference/http/server.rs @@ -316,6 +316,7 @@ async fn models_handler(State(_state): State) -> Response { config.reasoning_provider.as_deref(), config.agentic_provider.as_deref(), config.coding_provider.as_deref(), + config.vision_provider.as_deref(), config.memory_provider.as_deref(), config.embeddings_provider.as_deref(), config.heartbeat_provider.as_deref(), diff --git a/src/openhuman/inference/provider/factory.rs b/src/openhuman/inference/provider/factory.rs index 1f55280a6..e207823e6 100644 --- a/src/openhuman/inference/provider/factory.rs +++ b/src/openhuman/inference/provider/factory.rs @@ -63,7 +63,7 @@ pub const BYOK_INCOMPLETE_SENTINEL: &str = "__byok_incomplete__"; fn is_abstract_tier_model(model: &str) -> bool { use crate::openhuman::config::{ MODEL_AGENTIC_V1, MODEL_CHAT_V1, MODEL_CODING_V1, MODEL_REASONING_QUICK_V1, - MODEL_REASONING_V1, + MODEL_REASONING_V1, MODEL_VISION_V1, }; // No dedicated constant for the summarization tier yet; keep the literal // in sync with the tier name used by the summarizer sub-agent. @@ -74,6 +74,7 @@ fn is_abstract_tier_model(model: &str) -> bool { || trimmed == MODEL_CHAT_V1 || trimmed == MODEL_AGENTIC_V1 || trimmed == MODEL_CODING_V1 + || trimmed == MODEL_VISION_V1 || trimmed == MODEL_SUMMARIZATION_V1 } @@ -96,6 +97,7 @@ pub fn resolve_model_for_hint(hint_or_tier: &str, config: &Config) -> String { ("chat", crate::openhuman::config::MODEL_CHAT_V1), ("agentic", crate::openhuman::config::MODEL_AGENTIC_V1), ("coding", crate::openhuman::config::MODEL_CODING_V1), + ("vision", crate::openhuman::config::MODEL_VISION_V1), ("summarization", "summarization-v1"), ]; let tier_to_role: &[(&str, &str)] = &[ @@ -104,6 +106,7 @@ pub fn resolve_model_for_hint(hint_or_tier: &str, config: &Config) -> String { (crate::openhuman::config::MODEL_REASONING_QUICK_V1, "chat"), (crate::openhuman::config::MODEL_AGENTIC_V1, "agentic"), (crate::openhuman::config::MODEL_CODING_V1, "coding"), + (crate::openhuman::config::MODEL_VISION_V1, "vision"), ("summarization-v1", "summarization"), ]; @@ -154,7 +157,7 @@ pub fn resolve_model_for_hint(hint_or_tier: &str, config: &Config) -> String { pub(crate) fn is_known_openhuman_tier(model: &str) -> bool { use crate::openhuman::config::{ MODEL_AGENTIC_V1, MODEL_CHAT_V1, MODEL_CODING_V1, MODEL_REASONING_QUICK_V1, - MODEL_REASONING_V1, MODEL_SUMMARIZATION_V1, + MODEL_REASONING_V1, MODEL_SUMMARIZATION_V1, MODEL_VISION_V1, }; matches!( model, @@ -164,11 +167,13 @@ pub(crate) fn is_known_openhuman_tier(model: &str) -> bool { | MODEL_CODING_V1 | MODEL_REASONING_QUICK_V1 | MODEL_SUMMARIZATION_V1 + | MODEL_VISION_V1 | "hint:reasoning" | "hint:chat" | "hint:agentic" | "hint:coding" | "hint:summarization" + | "hint:vision" ) } @@ -187,10 +192,13 @@ pub(crate) fn is_known_openhuman_tier(model: &str) -> bool { pub(crate) fn oh_tier_supports_vision(model: &str) -> bool { use crate::openhuman::config::{ MODEL_AGENTIC_V1, MODEL_CHAT_V1, MODEL_CODING_V1, MODEL_REASONING_QUICK_V1, - MODEL_REASONING_V1, MODEL_SUMMARIZATION_V1, + MODEL_REASONING_V1, MODEL_SUMMARIZATION_V1, MODEL_VISION_V1, }; match model { MODEL_REASONING_V1 | "hint:reasoning" => true, + // Dedicated multimodal tier — the managed backend serves this with the + // vision flag enabled. This is what the vision sub-agent rides on. + MODEL_VISION_V1 | "hint:vision" => true, MODEL_CHAT_V1 | "hint:chat" => false, MODEL_REASONING_QUICK_V1 => false, MODEL_AGENTIC_V1 | "hint:agentic" => false, @@ -223,6 +231,10 @@ pub fn provider_for_role(role: &str, config: &Config) -> String { "reasoning" => config.reasoning_provider.as_deref(), "agentic" => config.agentic_provider.as_deref(), "coding" => config.coding_provider.as_deref(), + // Tier-specific multimodal model; like `agentic` it is NOT part of the + // chat-tier BYOK inheritance below — when unset it falls through to + // `primary_cloud` (→ managed `vision-v1`). + "vision" => config.vision_provider.as_deref(), // `memory_provider` covers both the memory-tree extract path and // the summarizer sub-agent (whose definition declares // `hint = "summarization"`). Both are "produce a condensed @@ -457,7 +469,7 @@ pub fn create_chat_provider_from_string( } if p == PROVIDER_OPENHUMAN { - return make_openhuman_backend(config); + return make_openhuman_backend(role, config); } // ── Session gate ────────────────────────────────────────────────── @@ -690,12 +702,29 @@ pub(crate) fn create_local_chat_provider_from_string( // ── Internal helpers ────────────────────────────────────────────────────────── /// Build the OpenHuman backend provider (session-JWT auth). -fn make_openhuman_backend(config: &Config) -> anyhow::Result<(Box, String)> { - let model = config - .default_model - .clone() - .filter(|m| !m.trim().is_empty()) - .unwrap_or_else(|| "reasoning-v1".to_string()); +/// +/// `role` is the workload name (e.g. `"chat"`, `"vision"`). The managed backend +/// otherwise derives its model from `config.default_model` (which defaults to the +/// non-vision `chat-v1` tier), so a tier-specific workload whose per-workload +/// provider is unset would silently inherit the global default. For the `vision` +/// workload that mismatch is fatal: an unset `vision_provider` would resolve to +/// `chat-v1`, `model_supports_vision` would report `false`, and the turn engine +/// would strip every attached image — leaving the managed vision sub-agent blind. +/// Pin `vision` to the dedicated multimodal `vision-v1` tier so the managed +/// default path keeps working without requiring the user to set `vision_provider`. +fn make_openhuman_backend( + role: &str, + config: &Config, +) -> anyhow::Result<(Box, String)> { + let model = if role == "vision" { + crate::openhuman::config::MODEL_VISION_V1.to_string() + } else { + config + .default_model + .clone() + .filter(|m| !m.trim().is_empty()) + .unwrap_or_else(|| "reasoning-v1".to_string()) + }; // Critical: pass the *config's* workspace directory through so the // provider's `AuthService` reads `auth-profiles.json` from the // same dir login wrote to. Without this, `ProviderRuntimeOptions::default()` @@ -728,6 +757,7 @@ fn make_openhuman_backend(config: &Config) -> anyhow::Result<(Box, Some("agentic") => crate::openhuman::config::MODEL_AGENTIC_V1.to_string(), Some("coding") => crate::openhuman::config::MODEL_CODING_V1.to_string(), Some("summarization") => crate::openhuman::config::MODEL_SUMMARIZATION_V1.to_string(), + Some("vision") => crate::openhuman::config::MODEL_VISION_V1.to_string(), Some(_) => { // Unrecognised hint — forward verbatim; the backend decides validity. model @@ -739,7 +769,7 @@ fn make_openhuman_backend(config: &Config) -> anyhow::Result<(Box, log::warn!( "[providers][chat-factory] model '{}' is not a recognized OpenHuman \ backend tier (valid: reasoning-v1, chat-v1, agentic-v1, coding-v1, \ - reasoning-quick-v1, summarization-v1); falling back to '{}'", + reasoning-quick-v1, summarization-v1, vision-v1); falling back to '{}'", model, crate::openhuman::config::MODEL_REASONING_V1, ); @@ -1304,7 +1334,7 @@ fn make_cloud_provider_by_slug( "[providers][chat-factory] slug='{}' has auth_style=OpenhumanJwt → routing to openhuman backend", slug ); - make_openhuman_backend(config) + make_openhuman_backend(role, config) } AuthStyle::None => { let p = make_openai_compatible_provider_with_config( diff --git a/src/openhuman/inference/provider/factory_tests.rs b/src/openhuman/inference/provider/factory_tests.rs index 6e63cf750..e8a133221 100644 --- a/src/openhuman/inference/provider/factory_tests.rs +++ b/src/openhuman/inference/provider/factory_tests.rs @@ -835,6 +835,7 @@ fn known_tiers_pass() { "coding-v1", "reasoning-quick-v1", "summarization-v1", + "vision-v1", ] { assert!( is_known_openhuman_tier(tier), @@ -850,6 +851,7 @@ fn known_hints_pass() { assert!(is_known_openhuman_tier("hint:agentic")); assert!(is_known_openhuman_tier("hint:coding")); assert!(is_known_openhuman_tier("hint:summarization")); + assert!(is_known_openhuman_tier("hint:vision")); } #[test] @@ -903,6 +905,15 @@ fn unknown_models_are_not_vision_capable() { assert!(!oh_tier_supports_vision("")); } +#[test] +fn vision_tier_is_vision_capable() { + // The dedicated multimodal tier (and its hint form) reports vision support, + // so the turn engine's image gate accepts image turns for the vision + // sub-agent — managed or BYOK (which resolves via this same alias). + assert!(oh_tier_supports_vision("vision-v1")); + assert!(oh_tier_supports_vision("hint:vision")); +} + #[test] fn make_openhuman_backend_forwards_unknown_hint_verbatim() { // Unrecognised hint:* strings (e.g. hint:reaction for lightweight models) @@ -914,7 +925,7 @@ fn make_openhuman_backend_forwards_unknown_hint_verbatim() { for hint in ["hint:reaction", "hint:garbage", "hint:lightweight"] { let mut config = Config::default(); config.default_model = Some(hint.to_string()); - let (_, model) = make_openhuman_backend(&config).expect("factory should succeed"); + let (_, model) = make_openhuman_backend("chat", &config).expect("factory should succeed"); assert_eq!(model, hint, "hint '{hint}' should pass through unchanged"); } } @@ -923,14 +934,14 @@ fn make_openhuman_backend_forwards_unknown_hint_verbatim() { fn make_openhuman_backend_translates_summarization_hint() { let mut config = Config::default(); config.default_model = Some("hint:summarization".to_string()); - let (_, model) = make_openhuman_backend(&config).expect("factory should succeed"); + let (_, model) = make_openhuman_backend("chat", &config).expect("factory should succeed"); assert_eq!(model, crate::openhuman::config::MODEL_SUMMARIZATION_V1); } #[test] fn make_openhuman_backend_reports_vision_capability() { let config = Config::default(); - let (provider, _) = make_openhuman_backend(&config).expect("factory should succeed"); + let (provider, _) = make_openhuman_backend("chat", &config).expect("factory should succeed"); let caps = provider.capabilities(); assert!(caps.native_tool_calling); assert!( @@ -945,7 +956,7 @@ fn make_openhuman_backend_falls_back_for_invalid_model() { // The factory must silently fall back to reasoning-v1 (the platform default). let mut config = Config::default(); config.default_model = Some("deepseek-v4-pro".to_string()); - let (_, model) = make_openhuman_backend(&config).expect("factory should succeed"); + let (_, model) = make_openhuman_backend("chat", &config).expect("factory should succeed"); assert_eq!( model, crate::openhuman::config::MODEL_REASONING_V1, @@ -957,7 +968,7 @@ fn make_openhuman_backend_falls_back_for_invalid_model() { fn make_openhuman_backend_keeps_valid_tier() { let mut config = Config::default(); config.default_model = Some("chat-v1".to_string()); - let (_, model) = make_openhuman_backend(&config).expect("factory should succeed"); + let (_, model) = make_openhuman_backend("chat", &config).expect("factory should succeed"); assert_eq!(model, "chat-v1"); } @@ -965,10 +976,27 @@ fn make_openhuman_backend_keeps_valid_tier() { fn make_openhuman_backend_keeps_reasoning_quick() { let mut config = Config::default(); config.default_model = Some("reasoning-quick-v1".to_string()); - let (_, model) = make_openhuman_backend(&config).expect("factory should succeed"); + let (_, model) = make_openhuman_backend("chat", &config).expect("factory should succeed"); assert_eq!(model, "reasoning-quick-v1"); } +#[test] +fn make_openhuman_backend_pins_vision_role_to_vision_tier() { + // Regression (PR #3699): the managed default_model is chat-v1 (a NON-vision + // tier). When `vision_provider` is unset the vision workload resolves to the + // managed backend, so make_openhuman_backend must override the default model + // with `vision-v1` — otherwise `oh_tier_supports_vision` reports false and + // the turn engine strips every attached image, blinding the vision sub-agent. + let config = Config::default(); + assert_eq!(config.default_model.as_deref(), Some("chat-v1")); + let (_, model) = make_openhuman_backend("vision", &config).expect("factory should succeed"); + assert_eq!(model, crate::openhuman::config::MODEL_VISION_V1); + assert!( + oh_tier_supports_vision(&model), + "vision role must resolve to a vision-capable managed tier" + ); +} + // ── BYOK fail-closed tests ──────────────────────────────────────────────────── #[test] diff --git a/src/openhuman/inference/provider/router.rs b/src/openhuman/inference/provider/router.rs index ecb8e0024..04b229ada 100644 --- a/src/openhuman/inference/provider/router.rs +++ b/src/openhuman/inference/provider/router.rs @@ -4,8 +4,8 @@ use async_trait::async_trait; use std::collections::HashMap; /// Maps OpenHuman's abstract tier model names (`reasoning-v1`, `chat-v1`, -/// `reasoning-quick-v1`, `agentic-v1`, `coding-v1`, `summarization-v1`) -/// to the hint slot in `model_routes`. Returns `None` for any model the +/// `reasoning-quick-v1`, `agentic-v1`, `coding-v1`, `summarization-v1`, +/// `vision-v1`) to the hint slot in `model_routes`. Returns `None` for any model the /// router shouldn't rewrite. fn openhuman_tier_to_hint(model: &str) -> Option<&'static str> { match model { @@ -15,6 +15,7 @@ fn openhuman_tier_to_hint(model: &str) -> Option<&'static str> { "agentic-v1" => Some("agentic"), "coding-v1" => Some("coding"), "summarization-v1" => Some("summarization"), + "vision-v1" => Some("vision"), _ => None, } } diff --git a/src/openhuman/inference/provider/router_tests.rs b/src/openhuman/inference/provider/router_tests.rs index 64bcf6399..2baea833b 100644 --- a/src/openhuman/inference/provider/router_tests.rs +++ b/src/openhuman/inference/provider/router_tests.rs @@ -192,6 +192,7 @@ fn resolve_translates_openhuman_tier_aliases_via_route_table() { ("reasoning", "smart", "gpt-5.5"), ("chat", "smart", "gpt-5.5-mini"), ("summarization", "smart", "gpt-4.1-nano"), + ("vision", "smart", "gpt-5.5-vision"), ], ); @@ -206,6 +207,11 @@ fn resolve_translates_openhuman_tier_aliases_via_route_table() { let (summary_idx, summary_model) = router.resolve("summarization-v1"); assert_eq!(summary_idx, 1); assert_eq!(summary_model, "gpt-4.1-nano"); + + // The vision tier alias routes through the `vision` hint to the BYOK model. + let (vision_idx, vision_model) = router.resolve("vision-v1"); + assert_eq!(vision_idx, 1); + assert_eq!(vision_model, "gpt-5.5-vision"); } // -- #2079: tier alias must not leak to upstream when no route configured --- @@ -254,6 +260,7 @@ fn every_tier_alias_falls_back_to_default_model_when_unrouted() { "agentic-v1", "coding-v1", "summarization-v1", + "vision-v1", ] { let (idx, model) = router.resolve(alias); assert_eq!(idx, 0, "alias {} → default provider index", alias); diff --git a/src/openhuman/inference/schemas.rs b/src/openhuman/inference/schemas.rs index cad7354d1..89751371e 100644 --- a/src/openhuman/inference/schemas.rs +++ b/src/openhuman/inference/schemas.rs @@ -87,6 +87,7 @@ struct InferenceUpdateModelSettingsParams { reasoning_provider: Option, agentic_provider: Option, coding_provider: Option, + vision_provider: Option, memory_provider: Option, embeddings_provider: Option, heartbeat_provider: Option, @@ -314,6 +315,7 @@ pub fn schemas(function: &str) -> ControllerSchema { optional_string("reasoning_provider", "Optional reasoning workload provider string."), optional_string("agentic_provider", "Optional agentic workload provider string."), optional_string("coding_provider", "Optional coding workload provider string."), + optional_string("vision_provider", "Optional vision / multimodal workload provider string."), optional_string("memory_provider", "Optional memory workload provider string."), optional_string("embeddings_provider", "Optional embeddings workload provider string."), optional_string("heartbeat_provider", "Optional heartbeat workload provider string."), @@ -746,6 +748,7 @@ fn handle_inference_update_model_settings(params: Map) -> Control reasoning_provider: update.reasoning_provider, agentic_provider: update.agentic_provider, coding_provider: update.coding_provider, + vision_provider: update.vision_provider, memory_provider: update.memory_provider, embeddings_provider: update.embeddings_provider, heartbeat_provider: update.heartbeat_provider, diff --git a/src/openhuman/mcp_server/resources.rs b/src/openhuman/mcp_server/resources.rs index 91d567e93..ac1297979 100644 --- a/src/openhuman/mcp_server/resources.rs +++ b/src/openhuman/mcp_server/resources.rs @@ -115,6 +115,12 @@ const RESOURCE_CATALOG: &[PromptResource] = &[ description: "Read-only worker that critiques plans and outputs.", content: include_str!("../agent_registry/agents/critic/prompt.md"), }, + PromptResource { + uri: "openhuman://prompts/agents/vision_agent", + name: "vision_agent", + description: "Multimodal worker that analyses attached images for the vision tier.", + content: include_str!("../agent_registry/agents/vision_agent/prompt.md"), + }, PromptResource { uri: "openhuman://prompts/agents/archivist", name: "archivist", diff --git a/src/openhuman/migrations/reconcile_orphaned_providers.rs b/src/openhuman/migrations/reconcile_orphaned_providers.rs index 738c5054c..a645afc7e 100644 --- a/src/openhuman/migrations/reconcile_orphaned_providers.rs +++ b/src/openhuman/migrations/reconcile_orphaned_providers.rs @@ -173,12 +173,13 @@ pub fn run(config: &mut Config) -> anyhow::Result { /// can't enforce this at compile time without a field-reflection macro, and a /// serde-based count guard doesn't work because the `Option` fields /// default to `None` and are omitted from the serialized table. -fn workload_fields(config: &mut Config) -> [(&'static str, &mut Option); 9] { +fn workload_fields(config: &mut Config) -> [(&'static str, &mut Option); 10] { [ ("chat", &mut config.chat_provider), ("reasoning", &mut config.reasoning_provider), ("agentic", &mut config.agentic_provider), ("coding", &mut config.coding_provider), + ("vision", &mut config.vision_provider), ("memory", &mut config.memory_provider), ("embeddings", &mut config.embeddings_provider), ("heartbeat", &mut config.heartbeat_provider),