refactor(agent): hold TurnModelSource across all turn paths (tinyagents Motion A, #4249)

docs/tinyagents-drift-ledger.md

@@ -63,6 +63,8 @@ work started. Counts include Rust files only.
 | P1-5 | First-class reasoning channel host cutover | **CLOSED** | TinyAgents `v1.6.0` already carries typed reasoning via `ContentBlock::Thinking`, `ContentBlock::RedactedThinking`, `MessageDelta::reasoning`, and stream reconstruction that preserves thinking blocks. OpenHuman now writes new non-streaming `reasoning_content` into `ContentBlock::Thinking` instead of `ProviderExtension`, while still reading legacy `ProviderExtension` reasoning from persisted transcripts and continuing to echo `ChatMessage::extra_metadata` for provider replay. Local validation: `cargo fmt --check` passed; two targeted `cargo test --lib --manifest-path Cargo.toml ...` attempts for the new conversion tests timed out during host test compilation before executing, so runtime verification is deferred to GitHub runners. |
 | P1-6 | Git-worktree `WorkspaceIsolation` provider | **RELEASED / HOST WRAPPER RETAINED** | TinyAgents PR [tinyagents#25](https://github.com/tinyhumansai/tinyagents/pull/25) shipped in `v1.7.0`. OpenHuman's wrapper remains for global event-bus emissions, `OutsideWorkspace`, and host policy mapping; adapter deletion waits for a focused wrapper-thinning pass. Local TinyAgents validation before merge: `cargo fmt --check`; `timeout 180s cargo clippy --all-targets -- -D warnings`; targeted worktree tests for create/list/status/diff/remove plus overlap and sanitize filters. |
 | P1-7 | Tool display metadata and timeout semantics | **RELEASED / HOST TRAIT RETAINED** | TinyAgents PR [tinyagents#26](https://github.com/tinyhumansai/tinyagents/pull/26) shipped in `v1.7.0`. Host `ToolPolicy` projection now fills the new `ToolRuntime.timeout` field, but OpenHuman's `Tool` trait still owns richer legacy display/timeout semantics until the Phase 2 tool model reconciliation. Local TinyAgents validation before merge: `cargo fmt --check`; `timeout 180s cargo clippy --all-targets -- -D warnings`; `timeout 120s cargo test display_`; `timeout 120s cargo test tool_policy_deserializes_without_display_metadata`; `timeout 120s cargo test timeout_policy_uses_richer_timeout_semantics`. |
+| P1-8 | Model-layer inversion: host callers still name `Box<dyn Provider>` / call `create_chat_provider` instead of the crate `ChatModel` | **IN PROGRESS** | `create_chat_model(...) -> Arc<dyn ChatModel<()>>` exists (`inference/provider/factory.rs:922`) as a zero-behavior-change shim wrapping the existing provider stack via `ProviderModel` (`tinyagents/model.rs`, built only in `factory.rs::chat_model_from_provider`). Baseline at branch point: ~7 runtime `create_chat_provider` call sites and 25 non-test files outside `inference/provider/` still name `dyn Provider` (incl. the seam `tinyagents/{mod,model,routes}.rs`, which legitimately keep it). **Done:** one-shot callers migrated — cron model-id resolve (`cron/scheduler.rs`), accessibility vision-locate (`accessibility/{automate,vision_click}.rs`); both Cargo worlds green. `agent_meetings/summary`, `memory/chat`, `learning/linkedin_enrichment`, `memory_tree`, `subconscious` runtime paths already on `create_chat_model`. **Deferred (own slice):** `tinyflows/caps.rs` (round-trips tool_calls + reasoning into a JSON envelope — needs seam converter helpers exposed); `src/bin/inference_probe.rs` (debug bin). |
+| P1-9 | Harness turn path (`Agent`/`AgentTurnRequest`) carries `Arc<dyn Provider>`, not a crate `ChatModel` | **BLOCKED ON DESIGN — one coupled refactor** | Investigation finding: the plan's Buckets 2–4 (routing/channels, agent harness, subagent runner) are **not independently landable** — `Provider` flows end-to-end: producers (`channels/runtime/dispatch/processor.rs` → `AgentTurnRequest.provider`; `agent/harness/session/builder/factory.rs` → `Agent.provider`; `subagent_runner/ops/provider.rs`) → `agent/bus.rs` / `harness/graph.rs::run_channel_turn_via_graph` → seam `build_turn_models(provider: Arc<dyn Provider>, …)` (`tinyagents/mod.rs:1139`). The channel graph reads Provider-trait capability methods before building the model: `supports_native_tools`, `supports_vision`, `effective_context_window` (**async**), `telemetry_provider_id`. `ProviderModel::profile()` already carries tool_calling / image_in / streaming / context-window, so a `ChatModel`-accepting `build_turn_models` is feasible — but the **async context-window resolution** and **telemetry id** must be re-homed (into the factory at ChatModel construction, or passed as params). Net: one atomic change across ~30 files (incl. ~10 test files) on the live channel/session turn path — must land as its own reviewed PR with streaming/cost/multimodal behavior-parity testing (the flagged regression surface: #4460 thread_id task-locals, $0-turn cost, tool timeline). `routing/provider.rs::IntelligentRoutingProvider` stays a `Provider` impl (provider-stack member, Phase-3 → `ModelRegistry`); it gets wrapped via `chat_model_from_provider` at the producer boundary. **BLOCKER (found while executing):** the harness cannot hold `Arc<dyn ChatModel>` in Phase 1 as the plan assumed. `build_turn_models` needs the raw `Provider` for (a) workload-route projection — `routes::build_route_models(provider: &Arc<dyn Provider>)` re-instantiates a `ProviderModel` per tier alias with distinct model strings + per-route `with_vision`/`with_reasoning` flags, which a single baked `ChatModel` cannot re-alias — and (b) the separate-error-slot summarizer. The crate `ChatModel` trait exposes no `as_any`/downcast, so the `Provider` cannot be recovered from an `Arc<dyn ChatModel>`. Therefore the true harness inversion is gated on **Phase 3** (replace `RouterProvider`/route-projection with the crate `ModelRegistry`), an upstream `vendor/tinyagents` change — not host-only. Achievable host-only step instead: wrap the harness-held `Arc<dyn Provider>` in a seam-owned newtype (e.g. `tinyagents::TurnModelSource`) so no `agent/` code names the `Provider` trait and all Provider handling is confined to the seam + factory, making the Phase-3 swap seam-local. **PROGRESS:** `docs/tinyagents-phase3-router-registry-design.md` records the corrected premise (router→registry already crate-wired in `assemble_turn_harness`; no upstream gap; work is host-only Motion A). `TurnModelSource` (pub seam type) landed + `TurnModels` extended with `provider_id`/`context_window`/`native_tools`/`supports_vision`. **Channel/bus turn path fully migrated** (commit `30c7dfd92`): `AgentTurnRequest.provider → turn_model_source`; `run_channel_turn_via_graph` reads caps off the built crate models; channels/triage producers wrap at the bus boundary; lib + the 3 bus integration tests green; zero behavior change. **Subagent-runner path migrated** (commit `8db888712`): `agent_graph::AgentTurnRequest.provider → turn_model_source`; `run_subagent_via_graph` takes the source (reads vision/native-tool caps + telemetry id off the built models, resolves context window via the source); `SubagentCheckpoint` cap-hit summary now runs on a crate `ChatModel` (via `TurnModelSource::build_summarizer`) instead of `provider.chat`; runner wraps its resolved `subagent_provider` at both dispatch sites. Core lib green; changed files clean under `--lib --tests`; zero behavior change. **Agent session path migrated** (commit `9112330b9`): `Agent`/`AgentBuilder`, `ParentExecutionContext`, and `ChatTurnGraph` hold a `TurnModelSource`/built `TurnModels`; core builds the tiered model set up front (reads vision off it), `ParentExecutionContext` carries the source, and the streaming cap-hit checkpoint keeps `provider.chat` via a `source.provider()` escape hatch (crate `ChatModel::invoke` has no delta sink). Extract tool migrated (commit `6106ced83`). **Motion A is structurally complete:** no agent-harness struct (`Agent`, `AgentBuilder`, `ParentExecutionContext`, `ChatTurnGraph`, both `AgentTurnRequest`s) holds `Arc<dyn Provider>`; both Cargo worlds green; zero behavior change. `TurnModelSource` gained `is_local_provider()` + a `provider()` escape hatch used only at seam-boundary resolution sites. **Remaining `dyn Provider` in `agent/` (Motion B, not Motion A):** provider-*resolution/build* boundaries that construct a provider to wrap into a source — `session/builder/factory.rs` (`create_chat_provider`/`create_routed_provider`), `subagent_runner/ops/provider.rs::resolve_subagent_provider` (kept `Arc<dyn Provider>` to avoid churning its 9 unit tests), `tools/delegate.rs`, `triage/routing.rs`, and the builder `.provider()/.provider_arc()` setters — plus test files. These vanish when Motion B registers crate-native `providers::openai` clients directly. Pre-existing full-`--tests` breakage in unrelated modules (config load, web, ollama, sandbox, reliable_tests, memory) is untouched and orthogonal. |
 
 ## Host Validation Notes
 

docs/tinyagents-phase3-router-registry-design.md

@@ -0,0 +1,137 @@
+# Phase 3 — RouterProvider → crate ModelRegistry: design & ground truth
+
+**Status:** design (starting Phase 3). Grounded in a full read of both the crate
+(`vendor/tinyagents/src/harness/model`, `harness/agent_loop`, `harness/retry`,
+`registry/`) and the host seam (`src/openhuman/tinyagents/{mod,routes,model}.rs`,
+`inference/provider/{router,factory}.rs`).
+**Relates to:** `docs/tinyagents-inference-migration-plan.md` (Phase 3),
+`docs/tinyagents-drift-ledger.md` (P1-9), issue #4249.
+
+---
+
+## 1. The premise correction (important)
+
+The plan framed Phase 3 as "implement RouterProvider → crate ModelRegistry in
+`vendor/tinyagents`." **Investigation shows the registry migration is already
+done at the seam, and the crate already does everything the host router needs —
+there is no hard upstream gap.** Specifically, `assemble_turn_harness`
+(`tinyagents/mod.rs:1240-1353`) already:
+
+- Registers the primary and every workload-tier route into the crate
+  `ModelRegistry` (`harness.register_model(name, model)`; primary via
+  `set_default_model`).
+- Wires cross-route fallback as a **crate** `RunPolicy.fallback =
+  routes::route_fallback_policy(model)` — traversed natively by
+  `agent_loop::invoke_model_resolving`.
+- Gates vision via the crate: `RequiredCapabilitiesMiddleware` stamps
+  `image_in` onto the `ModelRequest`, and the crate's `resolve_request` filters
+  named candidates by `ModelProfile::satisfies`.
+- Emits the `FallbackSelected` parity event via `FallbackObserverMiddleware`.
+
+The two things the crate *lacks* (per the crate audit) are **not needed** by the
+host:
+
+- **Capability-based *selection*** (scan the registry for a capable model): the
+  host never does this — the **caller picks the tier** upstream
+  (`subagent_runner/ops/graph.rs` sets `model = vision-v1` for image turns); the
+  middleware only *validates/enforces*, it doesn't select.
+- **Capability-aware *fallback***: the host's fallback chains are hand-built to
+  be capability-safe (`routes::same_family_fallbacks`: `vision-v1 → []`; the
+  same-family text alternates are all text-capable), so the crate's
+  non-capability-filtered `next_after` is benign here.
+
+**Conclusion:** RouterProvider is *already* projected onto the crate registry.
+What remains is host-side, and needs no crate release.
+
+## 2. What actually survives (the real remaining work)
+
+`RouterProvider` (`inference/provider/router.rs`) survives only as the **per-call
+BYOK alias resolver *inside* the wrapped `Provider`** — at dispatch it maps a
+tier alias (`reasoning-v1`, …) → concrete `(provider, model)` (issue #2079: raw
+aliases 400 on OpenAI/DeepSeek). The registered tier models are still
+`ProviderModel`s that wrap this host `Provider`. So the harness holds a
+`Provider` for exactly one reason: **`build_turn_models` builds the per-turn
+primary + routes + summarizer `ProviderModel`s from one `Provider` handle**
+(`tinyagents/mod.rs:1139-1175`, `routes::build_route_models`).
+
+Two independent motions remain, in priority order:
+
+### Motion A — Harness holds crate `ChatModel`s (this Phase; host-only)
+Move `build_turn_models` construction from the harness turn path
+(`agent/harness/graph.rs::run_channel_turn_via_graph`, session turn path) to the
+**producer/factory boundary**, so `agent/` holds crate model types, not
+`Provider`. The harness turn path today reads four `Provider` methods before
+building — all are already available without the trait:
+
+| harness reads today | crate-native source |
+| --- | --- |
+| `provider.supports_native_tools()` | `TurnModels.primary.profile().tool_calling` |
+| `provider.supports_vision()` | `…profile().modalities.image_in` |
+| `provider.effective_context_window(model).await` | resolved at build time → `…profile().max_input_tokens` |
+| `provider.telemetry_provider_id()` | new `TurnModels.provider_id: String` |
+
+So `TurnModels` becomes the unit the harness holds. Design points:
+
+1. **Extend `TurnModels`** with `provider_id: String` and small accessors
+   (`native_tools()`, `supports_vision()`, `context_window()`) reading
+   `primary.profile()`. Removes every raw-`Provider` read in the harness graph.
+2. **New factory entry** `inference::provider::factory::create_turn_models(
+   role_or_model, config, temperature) -> anyhow::Result<TurnModels>`: builds the
+   `Provider` internally (existing `create_chat_provider` path), resolves the
+   async `effective_context_window`, computes `telemetry_provider_id`, and calls
+   the seam `build_turn_models`. All `Provider` naming stays in
+   `inference/provider/` + the seam.
+3. **Lifecycle:** `build_turn_models` is per-`(provider, model)`; the harness
+   builds a fresh `TurnModels` per turn today. Keep that — the producer builds
+   `TurnModels` at turn-request assembly (channels processor, session turn entry,
+   subagent runner) instead of the harness graph doing it. `error_slot` stays
+   per-turn (correct — it recovers *this* turn's provider error).
+4. **Type swap:** `AgentTurnRequest.provider: Arc<dyn Provider>` → carries the
+   built `TurnModels` (+ the `model`/`provider_name` it already has);
+   `Agent`/`AgentBuilder.provider` likewise. `run_channel_turn_via_graph` /
+   session `turn/graph.rs` receive `TurnModels`, read caps via the accessors, and
+   pass it straight to `run_turn_via_tinyagents_shared` (which already takes
+   `TurnModels`). `IntelligentRoutingProvider` stays a `Provider` impl
+   (provider-stack member); the factory wraps it before `build_turn_models`.
+
+Exit: no `agent/` file names the `Provider` trait; `ProviderModel` built only in
+the seam (`model.rs` / `build_turn_models`), reached only via the factory entry.
+Zero behavior change (same `ProviderModel`s, same registry/fallback wiring).
+
+### Motion B — Registered models become crate-native (later; the big LOC win)
+Replace `ProviderModel` wrappers with crate `providers::openai` clients built
+from config (BYOK slugs, Ollama/LM Studio base URLs), registering per-tier
+clients directly so `RouterProvider`'s per-call alias resolution disappears
+(each tier is its own registered client). This is inference-plan **Phase 2**
+(client swap, deletes `compatible*.rs`) + the remainder of Phase 3, and keeps the
+bespoke providers (managed backend, claude-code, codex) as host `ChatModel`
+impls (Phase 4). Out of scope for Motion A; unblocked by it.
+
+## 3. Optional crate nicety (not required)
+If we later want capability-*aware* fallback (so a hand-built chain isn't the
+only safety net), the one-line crate change is to re-apply `model_eligible`
+to `FallbackPolicy::next_after` targets in `invoke_model_resolving`
+(`vendor/tinyagents/src/harness/agent_loop/model_call.rs:186-204`). File as a
+separate small upstream PR only if Motion B introduces capability-divergent
+fallback chains. Not needed for Motion A.
+
+## 4. First implementation slice (Motion A)
+1. `TurnModels`: add `provider_id` + `native_tools()/supports_vision()/context_window()` accessors (seam-internal; behavior-neutral).
+2. `create_turn_models(...)` factory entry (wraps `create_chat_provider` +
+   async context-window resolve + `build_turn_models`).
+3. Cut `run_channel_turn_via_graph` to take `TurnModels` and read caps via
+   accessors (delete the 4 raw-provider reads); channel producer
+   (`channels/runtime/dispatch/processor.rs`) builds `TurnModels` via the factory
+   and puts it on `AgentTurnRequest`.
+4. Repeat for the session turn path + subagent runner; swap `Agent.provider`.
+5. Update tests to build `TurnModels`/crate `MockModel` instead of hand-rolled
+   `Provider` impls.
+6. Verify: both Cargo worlds green; `json_rpc_e2e`; streaming/cost/tool-timeline
+   parity on a mock-backend turn (the #4460 / $0-turn / tool-timeline hazards).
+
+## 5. Verification & parity locks (unchanged from the plan)
+Provider-string grammar, `inference.*` RPC, tier alias set, fallback ordering
+(single same-family alternate; vision primary-only), the once-per-logical-call
+FIFO usage push (charged-USD-over-estimate precedence, graceful degradation), and
+the `FallbackSelected` event must all be preserved. These are already crate-wired
+— Motion A only moves *where the models are built*, not how they route.

src/openhuman/accessibility/automate.rs

@@ -724,13 +724,14 @@ impl AutomateBackend for RealBackend {
         description: &str,
     ) -> Result<Option<(i32, i32)>, String> {
         // Use the main `chat` provider's vision model (per plan): reliable UI
-        // grounding, and the fallback only fires when AX is empty (rare).
-        let (provider, model) =
-            crate::openhuman::inference::provider::create_chat_provider("chat", &self.config)
+        // grounding, and the fallback only fires when AX is empty (rare). The
+        // crate `ChatModel` bakes the resolved model + temperature; the vision
+        // request pins temperature 0.0 itself.
+        let model =
+            crate::openhuman::inference::provider::create_chat_model("chat", &self.config, 0.0)
                 .map_err(|e| format!("vision provider unavailable: {e}"))?;
         let coords =
-            super::vision_click::locate_via_vision(&*provider, &model, screenshot, description)
-                .await?;
+            super::vision_click::locate_via_vision(&model, screenshot, description).await?;
         Ok(coords.map(|(px, py)| super::vision_click::image_to_screen(geom, px, py)))
     }
 

src/openhuman/accessibility/vision_click.rs

@@ -172,16 +172,28 @@ pub(crate) fn capture_window_geometry(
 /// Ask the vision model for the target's pixel coordinates within `screenshot`.
 /// Returns `Ok(None)` when the model reports the element isn't visible.
 pub(crate) async fn locate_via_vision(
-    provider: &dyn crate::openhuman::inference::provider::Provider,
-    model: &str,
+    model: &std::sync::Arc<dyn tinyagents::harness::model::ChatModel<()>>,
     screenshot_data_uri: &str,
     description: &str,
 ) -> Result<Option<(i32, i32)>, String> {
+    use tinyagents::harness::message::Message;
+    use tinyagents::harness::model::ModelRequest;
+    // The image rides as an `[IMAGE:<uri>]` marker in the user text (promoted to a
+    // real image part in the provider request builder); the marker survives the
+    // crate `Message` → host `ChatMessage` round-trip verbatim. The vision call is
+    // deterministic, so pin temperature 0.0 on the request (overrides the model's
+    // construction temperature).
     let user = build_locate_user(description, screenshot_data_uri);
-    let raw = provider
-        .chat_with_system(Some(locate_system_prompt()), &user, model, 0.0)
+    let request = ModelRequest::new(vec![
+        Message::system(locate_system_prompt().to_string()),
+        Message::user(user),
+    ])
+    .with_temperature(0.0);
+    let response = model
+        .invoke(&(), request)
         .await
         .map_err(|e| format!("vision model call failed: {e}"))?;
+    let raw = response.text();
     log::debug!("{LOG_PREFIX} locate raw response: {raw:?}");
     parse_locate_response(&raw)
 }