diff --git a/CONTEXT.md b/CONTEXT.md index eb16c695..ab13b44a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -24,6 +24,49 @@ The bare `chat_id` value shown to and accepted from users (the channel prefix is stripped for display, re-prepended to form the session key). Presentation term; in code the value lives in the `chat_id` field and the composite is the `session_key`. +**Model binding**: +A model id together with the provider whose credential serves it, as one value +(`raven/providers/binding.py`). The pairing is the point: a model id alone does +not say which key reaches it, and updating one half is how one vendor's key ends +up on another vendor's endpoint. A turn resolves its binding once at `run_turn` +entry and holds it in a context var for the whole turn tree, so everything under +that turn -- the loop, the context engine's LLM-backed segments, the skill gate +and rewriter, the consolidator, and any task the turn detaches -- reads the same +pair. _Avoid_: "the current model" / "the active provider" for this; both name +one half. + +**Session binding**: +The model binding one conversation runs on. Sessions that never switched have no +entry and resolve to the **default binding**; a switch writes only that +session's entry, so it moves no other conversation and does not change what a +new one starts on. Stored on the session record so it survives a restart. + +**Default binding**: +What a session with no binding of its own runs on: `agents.defaults` from config, +verbatim. Changed by a `scope="default"` switch, which leaves sessions that +already chose their own model where they are. + +**Provider pool**: +The one place a model id is resolved to the credential that serves it +(`raven/providers/pool.py`), caching a provider per (vendor, model) and dropping +the cache when the credentials behind it change. Also what turns a **subsystem +pin** into a pair. Constructing a `ModelBinding` from an already-resolved pair +happens in several places; deciding *which* provider a model id pairs with +happens only here. + +**Subsystem pin**: +A model configured for one subsystem rather than for the conversation, as a +model and the provider serving it (`context.curator_model` + +`curator_provider`, `skill_forge.llm_gate_model` + `llm_gate_provider`). Both +halves because an id alone is ambiguous the moment a gateway is configured: +`openrouter` + `anthropic/claude-haiku-4-5` and `anthropic` + +`claude-haiku-4-5` are both valid and name different credentials. With the +provider set nothing is derived; with it unset the vendor is guessed from the +id, which is what configs predating the field get. A pin that cannot be paired +is reported and dropped, and the subsystem follows the conversation's model -- +a bare pinned id sent on the conversation's key is exactly the mis-pairing +above. Unset out of the box -- no subsystem ships a vendor default. + **Turn**: One complete agent reaction: from an inbound message entering the agent loop to the agent's final response, including every LLM call and tool execution in between. diff --git a/docs/Raven-vs-OpenClaw-Hermes.md b/docs/Raven-vs-OpenClaw-Hermes.md index 8d54d53e..2132a6e0 100644 --- a/docs/Raven-vs-OpenClaw-Hermes.md +++ b/docs/Raven-vs-OpenClaw-Hermes.md @@ -34,7 +34,7 @@ **Hermes 的做法**:`ContextCompressor` — 一套固定的 4 阶段(裁剪工具输出 → 保护边界 → 中段摘要 → 增量更新)。触发式、被动、不可恢复。 -**我们的做法**:Curator 是一个**独立的小模型 Agent**(默认 gemini-2.5-flash),有自己的 11 个内部工具: +**我们的做法**:Curator 是一个**独立的小模型 Agent**(默认跟随对话所用的模型;可通过 context.curator_model 单独指定一个更小更快的模型),有自己的 11 个内部工具: ``` curator_check_budget — 理解当前 token 压力 diff --git a/raven/agent/loop/main.py b/raven/agent/loop/main.py index 1ddadc64..b1d9591b 100644 --- a/raven/agent/loop/main.py +++ b/raven/agent/loop/main.py @@ -44,6 +44,7 @@ from raven.memory_engine.base import TokenBudget from raven.memory_engine.consolidate.consolidator import MemoryConsolidator, MemoryStore from raven.providers.base import LLMProvider, LLMResponse, ToolCallRequest +from raven.providers.binding import ModelBinding, active_binding, use_binding from raven.providers.capabilities import image_placeholder_text, supports_image_tool_result from raven.sandbox import SandboxConfig, SandboxExecutor, SandboxInitError, build_executor from raven.session.manager import Session, SessionManager @@ -79,6 +80,7 @@ from raven.context_engine import ContextEngine from raven.memory_engine.backend import MemoryBackend from raven.proactive_engine.schedulers.cron.service import CronService + from raven.providers.pool import ProviderPool from raven.routing.router import ModelRouter from raven.sandbox.debug_server import SandboxDebugServer from raven.skill_hub import SkillHubClient @@ -310,6 +312,7 @@ def __init__( now_fn: Callable | None = None, context_config: "ContextConfig | None" = None, runtime_config: "RuntimeConfig | None" = None, + provider_pool: "ProviderPool | None" = None, interactive: bool = True, jina_api_key: str | None = None, max_concurrent_subagents: int = 4, @@ -365,9 +368,14 @@ def __init__( # Returning None means "fall through to normal flow". self.decision_consumer = decision_consumer self.channels_config = channels_config - self.provider = provider self.workspace = workspace - self.model = model or provider.get_default_model() + # The model a turn runs on is per session, so it cannot live in two + # attributes on a process-wide loop. ``_default_binding`` is what a + # session starts on; ``provider``/``model`` below read whichever + # binding the running turn entered. + self._provider_pool = provider_pool + self._default_binding = ModelBinding(provider, model or provider.get_default_model()) + self._session_bindings: dict[str, ModelBinding] = {} # Resolved lazily on the first tool result that carries an image. Keyed # by model, not a single flag: the loop is a long-lived singleton and # takes a per-call model (strategies rewrite it, and the model chain @@ -462,7 +470,7 @@ def __init__( config=context_config, builder=self.context, provider=provider, - model=self.model, + model=self._default_binding.model, context_window_tokens=context_window_tokens, get_tool_definitions=self.tools.get_definitions, now_fn=now_fn, @@ -473,6 +481,7 @@ def __init__( skill_forge_router_config=skill_forge_router_config, skill_forge_config=skill_forge_config, skill_hub_client=self._skill_hub_client, + provider_pool=provider_pool, ) # Runtime discipline (5th pillar). Bug2 uses ``runtime.checkpoint``; @@ -508,7 +517,7 @@ def __init__( self.subagents = SubagentManager( provider=provider, workspace=workspace, - model=self.model, + model=self._default_binding.model, brave_api_key=brave_api_key, jina_api_key=jina_api_key, web_proxy=web_proxy, @@ -543,7 +552,7 @@ def __init__( self.memory_consolidator = MemoryConsolidator( workspace=workspace, provider=provider, - model=self.model, + model=self._default_binding.model, sessions=self.sessions, context_window_tokens=context_window_tokens, build_messages=self.context.build_messages, @@ -553,6 +562,13 @@ def __init__( self._consolidation_tasks: set[asyncio.Task] = set() + # ``self.subagents``, ``self.context_engine`` and + # ``self.memory_consolidator`` were each handed ``provider`` earlier in + # this constructor. Inside a turn they read the turn's binding; the + # reference they hold is only the fallback for work that runs outside + # one, and ``set_default_binding`` is what keeps that fallback current. + # Add the call there when adding another holder. + # Phase B-3: the L4 facade (``DefaultMemoryEngine`` / # ``MemoryEngine`` ABC) has been retired. AgentLoop now holds # the underlying subsystems directly: @@ -607,6 +623,107 @@ def _apply_disabled_tools(self) -> None: if self.tools.has(name): self.tools.unregister(name) + @property + def provider(self) -> LLMProvider: + """The provider of the binding the running turn entered. + + A property, not an attribute: the model is per session now, so there + is no single answer to cache on the loop. Outside a turn (startup, a + one-shot CLI call) this is the configured default. + """ + binding = active_binding() + return binding.provider if binding is not None else self._default_binding.provider + + @property + def model(self) -> str: + """The model id of the binding the running turn entered.""" + binding = active_binding() + return binding.model if binding is not None else self._default_binding.model + + @property + def provider_pool(self) -> "ProviderPool | None": + """Where a model id becomes a model id plus the credential for it.""" + return self._provider_pool + + @property + def default_binding(self) -> ModelBinding: + """What a session with no switch of its own runs on.""" + return self._default_binding + + def binding_for_session(self, session_key: str) -> ModelBinding: + """The binding this session runs on: its own switch, else the default. + + A new session has no entry, so it starts on the configured default + rather than on whatever the last session switched to. + """ + return self._session_bindings.get(session_key, self._default_binding) + + def session_model(self, session_key: str) -> str: + """What to show this session's user, which is not the global default.""" + return self.binding_for_session(session_key).model + + def has_session_binding(self, session_key: str) -> bool: + """Did this session switch, or is it just following the default? + + ``session_model`` cannot answer that -- it falls back to the default, + so it never returns None. Callers that must distinguish "chose this" + from "inherited this" ask here. + """ + return session_key in self._session_bindings + + def restore_session_model(self, session_key: str, model: str, provider_name: str | None = None) -> None: + """Put a resumed session back on the model it was last switched to. + + Session overrides live in memory, so without this a restart moves every + switched session back to the default and the user's choice lasts + exactly as long as the process. The model comes from the session + record. A model that can no longer be built (a credential since + removed) leaves the session on the default rather than failing the + resume. + """ + pool = self._provider_pool + if pool is None or not model: + return + try: + self.set_session_binding(session_key, pool.bind(model, provider_name)) + except (SystemExit, RuntimeError, ValueError) as exc: + logger.warning("session {!r} cannot resume on {!r} ({}); using the default", session_key, model, exc) + + def set_session_binding(self, session_key: str, binding: ModelBinding) -> None: + """Switch one session, leaving every other session where it was. + + Applied immediately and still safe mid-turn: a turn resolves its + binding once at ``run_turn`` entry and holds it in a context var for + its whole tree, including anything it detaches. So a switch during a + turn cannot move that turn -- it lands on the next one -- and no + parking is needed to arrange that. + """ + self._session_bindings[session_key] = binding + + def clear_session_binding(self, session_key: str) -> None: + """Drop a session's override so it follows the default again.""" + self._session_bindings.pop(session_key, None) + + def set_default_binding(self, binding: ModelBinding) -> None: + """Change what new sessions start on. + + Sessions that already switched keep their own binding; sessions that + never did pick this up on their next turn. Subsystem fallbacks are + re-pointed too, for the paths that run outside a turn and therefore + have no binding to read. + """ + self._default_binding = binding + self.subagents.set_provider(binding.provider, binding.model) + self.context_engine.set_provider(binding.provider, binding.model) + self.memory_consolidator.set_provider(binding.provider, binding.model) + + def set_provider(self, provider: LLMProvider, model: str) -> None: + """Change the default binding. Kept for callers that are not + session-aware (the gateway, a CLI one-shot); the session-scoped path + is ``set_session_binding``. + """ + self.set_default_binding(ModelBinding(provider, model)) + def configure_personalization(self, enable: bool) -> None: """Global switch for the 4-step personalization flow (PAHF-inspired). @@ -2558,14 +2675,52 @@ async def run_turn( inline_tool_stream: bool = False, usage_sink: dict[str, Any] | None = None, text_sink: dict[str, Any] | None = None, + ) -> TurnOutcome: + """Bind the turn to its session's model; see ``_run_turn`` for the turn. + + This is where a session's model becomes the one thing everything under + the turn reads: the loop's own ``provider``/``model``, the context + engine's LLM-backed segments, the skill gate and rewriter, the + consolidator, and anything the turn detaches (a subagent inherits the + context it was created in). + + Resolving once here is also what makes a mid-turn switch harmless + without any parking. The binding is captured before the first read and + held for the tree, so a switch that lands while this turn runs is + simply not visible to it -- it takes effect on the session's next + turn. Turns from other sessions run under their own binding + concurrently, which is the point. + """ + session_key = req.conversation or f"{req.source.channel}:{req.source.chat_id}" + with use_binding(self.binding_for_session(session_key)): + return await self._run_turn( + req, + emit, + drain, + stream=stream, + inline_tool_stream=inline_tool_stream, + usage_sink=usage_sink, + text_sink=text_sink, + ) + + async def _run_turn( + self, + req: TurnRequest, + emit: Emit, + drain: Drain, + *, + stream: bool = True, + inline_tool_stream: bool = False, + usage_sink: dict[str, Any] | None = None, + text_sink: dict[str, Any] | None = None, ) -> TurnOutcome: """Spine-native turn entry: consume a TurnRequest, fan the agent's output onto the single ``emit``, return a TurnOutcome. Collapses the legacy output paths (a str return + the five callbacks) onto one boundary. Named ``run_turn`` rather than ``run``: ``run`` is the runtime keep-alive - (executor / debug server / MCP up, then idle). A spine runner wraps this - method to satisfy the TurnRunner protocol. + (executor / debug server / MCP up, then idle). A spine runner calls the + public ``run_turn`` to satisfy the TurnRunner protocol. ``stream`` is the canon Q2-D assembly switch: a streaming outlet (TUI) wires it True so the reply goes out as StreamDelta and dissolves (b2 — no diff --git a/raven/agent/subagent/manager.py b/raven/agent/subagent/manager.py index 1d17e90e..bdfbf4ec 100644 --- a/raven/agent/subagent/manager.py +++ b/raven/agent/subagent/manager.py @@ -16,6 +16,7 @@ from raven.agent.tools.web import WebFetchTool, WebSearchTool from raven.config.schema import ExecToolConfig from raven.providers.base import LLMProvider +from raven.providers.binding import ModelBinding, resolve from raven.sandbox import SandboxConfig, build_executor from raven.security.trust import wrap_untrusted from raven.tracing import semconv, trace @@ -50,7 +51,6 @@ def __init__( ): from raven.config.schema import ExecToolConfig - self.provider = provider self.workspace = workspace # Spine submit, late-bound (the scheduler pins its home loop at # construction and is built inside each entry point's run loop; this @@ -58,7 +58,7 @@ def __init__( # set_submit before any announce; the result re-injection submits a # SUBAGENT-origin turn. self._submit = None - self.model = model or provider.get_default_model() + self._fallback = ModelBinding(provider, model or provider.get_default_model()) self.brave_api_key = brave_api_key self.jina_api_key = jina_api_key self.web_proxy = web_proxy @@ -75,6 +75,23 @@ def __init__( # pruned to the rolling window on access, so it self-bounds. self._session_spawn_times: dict[str, deque[float]] = {} + def set_provider(self, provider: LLMProvider, model: str) -> None: + """Adopt the provider a live ``/model`` switch just built. + + Only the out-of-turn fallback moves. A spawn requested during a turn + takes that turn's binding, so a subagent follows the conversation + that asked for it rather than whatever this manager was built with. + """ + self._fallback = ModelBinding(provider, model) + + @property + def provider(self) -> LLMProvider: + return resolve(None, self._fallback).provider + + @property + def model(self) -> str: + return resolve(None, self._fallback).model + async def spawn( self, task: str, @@ -107,7 +124,15 @@ async def spawn( display_label = label or task[:30] + ("..." if len(task) > 30 else "") origin = {"channel": origin_channel, "chat_id": origin_chat_id, "session_key": quota_key} - bg_task = asyncio.create_task(self._run_subagent(task_id, task, display_label, origin)) + # The binding of the turn that asked for this spawn, snapshotted here + # rather than where the task starts running: it queues behind the + # concurrency gate and a sandbox boot first, and a switch landing in + # that window would hand it an endpoint chosen after it was asked for. + # A subagent has no model of its own, so it follows its conversation. + binding = resolve(None, self._fallback) + bg_task = asyncio.create_task( + self._run_subagent(task_id, task, display_label, origin, binding.provider, binding.model) + ) self._running_tasks[task_id] = bg_task if session_key: self._session_tasks.setdefault(session_key, set()).add(task_id) @@ -131,6 +156,8 @@ async def _run_subagent( task: str, label: str, origin: dict[str, str], + provider: LLMProvider, + model: str, ) -> None: """Execute the subagent task and announce the result.""" logger.info("Subagent [{}] starting task: {}", task_id, label) @@ -141,7 +168,7 @@ async def _run_subagent( async with self._gate: executor = build_executor(self._sandbox_config, self.workspace, self._owned_ids) async with executor: - await self._run_subagent_inner(task_id, task, label, origin, executor) + await self._run_subagent_inner(task_id, task, label, origin, executor, provider, model) except Exception as e: error_msg = f"Error: {str(e)}" logger.error("Subagent [{}] failed: {}", task_id, e) @@ -154,6 +181,8 @@ async def _run_subagent_inner( label: str, origin: dict[str, str], executor: Any, + provider: LLMProvider, + model: str, ) -> None: try: # Build subagent tools (no message tool, no spawn tool) @@ -191,10 +220,10 @@ async def _run_subagent_inner( while iteration < max_iterations: iteration += 1 - response = await self.provider.chat_with_retry( + response = await provider.chat_with_retry( messages=messages, tools=tools.get_definitions(), - model=self.model, + model=model, ) if response.has_tool_calls: diff --git a/raven/cli/agent_commands.py b/raven/cli/agent_commands.py index 715c64f4..c4127607 100644 --- a/raven/cli/agent_commands.py +++ b/raven/cli/agent_commands.py @@ -320,7 +320,10 @@ def agent( registry=plugin_registry, ) + from raven.providers.pool import ProviderPool + agent_loop = AgentLoop( + provider_pool=ProviderPool(lambda: load_runtime_config(None, None)), provider=provider, now_fn=parse_fake_now(fake_now), workspace=config.workspace_path, diff --git a/raven/cli/gateway_commands.py b/raven/cli/gateway_commands.py index 9edfccca..2d77372a 100644 --- a/raven/cli/gateway_commands.py +++ b/raven/cli/gateway_commands.py @@ -222,7 +222,10 @@ def gateway( backend = maybe_build_memory_backend(config.workspace_path, ec_config) # Create agent with cron service + from raven.providers.pool import ProviderPool + agent = AgentLoop( + provider_pool=ProviderPool(lambda: load_runtime_config(None, None)), provider=provider, now_fn=parse_fake_now(fake_now), workspace=config.workspace_path, diff --git a/raven/cli/tui_commands.py b/raven/cli/tui_commands.py index caf78ee0..79aea73b 100644 --- a/raven/cli/tui_commands.py +++ b/raven/cli/tui_commands.py @@ -438,7 +438,10 @@ def _build_tui_agent_loop(): registry=plugin_registry, ) + from raven.providers.pool import ProviderPool + agent_loop = AgentLoop( + provider_pool=ProviderPool(lambda: load_runtime_config(None, None)), provider=provider, workspace=config.workspace_path, model=config.agents.defaults.model, diff --git a/raven/config/raven.py b/raven/config/raven.py index 07f160da..8eee895a 100644 --- a/raven/config/raven.py +++ b/raven/config/raven.py @@ -74,8 +74,27 @@ class ContextConfig(_Base): fast_path_threshold: float = 0.60 """Curator Fast Path cutoff. Below this % of budget → zero-LLM pass-through.""" - curator_model: str = "gemini-2.5-flash" - """Model used by the Curator agent loop (Slow Path). Kept small & fast.""" + curator_model: str | None = None + """Model for the Curator agent loop (Slow Path). Unset means the Curator + runs on the model of the conversation it is curating. + + Worth setting, and worth setting to something small: one slow-path pass is + a bounded agent loop of up to 12 tool-calling requests, so it is per-turn + housekeeping rather than an answer, and unset means a long conversation + pays conversation-model prices for it. Pair it with ``curator_provider``: + a model id alone does not say which credential serves it, and a model id + without a key of its own is not a configured subsystem -- the Curator + falls back to the conversation rather than send that id on the + conversation's key.""" + + curator_provider: str | None = None + """Which configured provider serves ``curator_model``. + + Set this whenever the id alone is ambiguous, which is most of the time + once a gateway is involved: ``openrouter`` with ``anthropic/claude-haiku-4-5`` + and ``anthropic`` with ``claude-haiku-4-5`` are both valid, name different + credentials and different bills, and only you know which was meant. Unset + falls back to deriving the vendor from the id.""" curator_timeout_seconds: float = 30.0 """Max wall time for one Curator slow-path invocation before fallback.""" @@ -668,13 +687,12 @@ class SmartRoutingConfig(_Base): """SmartRouter configuration.""" enabled: bool = False - tiers: dict[str, list[str]] = Field( - default_factory=lambda: { - "light": ["gemini-2.5-flash", "claude-haiku-4-5"], - "medium": ["claude-sonnet-4-6", "gpt-4.1-mini"], - "heavy": ["claude-opus-4-6", "gpt-4.1"], - } - ) + tiers: dict[str, list[str]] = Field(default_factory=dict) + """Which models each tier may route to. Empty out of the box: the table + this replaced named six models across three vendors, for users who may + hold no key for any of them, and routing has no meaning without models to + choose between -- so enabling this means listing your own.""" + default_tier: Literal["light", "medium", "heavy"] = "heavy" """Fallback tier when routing is uncertain — conservative default.""" @@ -686,7 +704,8 @@ class ToolResultLifecycleConfig(_Base): full_retention_turns: int = 3 summary_retention_turns: int = 10 placeholder_text: str = "[Tool result archived — retrievable via Curator]" - summary_model: str = "gemini-2.5-flash" + summary_model: str | None = None + """Unset means the conversation's own model. See ``curator_model``.""" class TokenWiseConfig(_Base): @@ -965,7 +984,17 @@ class SkillForgeConfig(_Base): llm_gate_model: str | None = None """Optional model override for gate calls. ``None`` → use the - provider's default chat model (typically the agent's main model).""" + provider's default chat model (typically the agent's main model). + + Pair it with ``llm_gate_provider``: an id alone does not say which + credential serves it.""" + + llm_gate_provider: str | None = None + """Which configured provider serves ``llm_gate_model``. + + Same rule as ``context.curator_provider``: set it when the id alone is + ambiguous (a gateway serving another vendor's model), leave it unset to + derive the vendor from the id.""" llm_gate_temperature: float = 0.0 """Sampling temperature for gate calls. 0.0 for deterministic @@ -1003,7 +1032,7 @@ class SkillForgeConfig(_Base): rewrites — e.g. ``"claude-opus-4-6"``.""" # --- Detect / extraction gating (wired into everos) --- - detect_model: str = "gemini-2.5-flash" + detect_model: str | None = None """LLM used for the cheap per-turn classification work — today that's the everos boundary detector (multi-turn task split). A smaller / faster model than ``evolve_model`` is intentional: boundary diff --git a/raven/context_engine/assembler.py b/raven/context_engine/assembler.py index a2c990dd..debf165d 100644 --- a/raven/context_engine/assembler.py +++ b/raven/context_engine/assembler.py @@ -37,6 +37,7 @@ if TYPE_CHECKING: from raven.context_engine.curator import TurnContext + from raven.providers.base import LLMProvider class ContextAssembler(ContextEngine): @@ -64,6 +65,15 @@ def owns_compaction(self) -> bool: # the full append-only log and skips the host MemoryConsolidator. return True + def set_provider(self, provider: "LLMProvider", model: str) -> None: + # Duck-typed on purpose: only the builders that actually call an LLM + # implement it, and putting it on the SegmentBuilder protocol would + # force an empty override onto every purely textual builder. + for builder in self._builders: + setter = getattr(builder, "set_provider", None) + if callable(setter): + setter(provider, model) + async def assemble( self, session_key: str, diff --git a/raven/context_engine/base.py b/raven/context_engine/base.py index 398387ff..3b8e8722 100644 --- a/raven/context_engine/base.py +++ b/raven/context_engine/base.py @@ -35,6 +35,7 @@ # for ``ContextEngine``, so referencing ``TurnContext`` only in type # hints keeps the loop unbroken. from raven.context_engine.curator import TurnContext + from raven.providers.base import LLMProvider # --------------------------------------------------------------------------- @@ -142,6 +143,17 @@ def owns_compaction(self) -> bool: lets the engine manage history compaction itself (Curator archives messages out-of-band).""" + def set_provider(self, provider: "LLMProvider", model: str) -> None: + """Adopt the provider a live ``/model`` switch just built. + + Segments that call an LLM hold the provider handed to them at + construction; without this they keep calling the old one for the + rest of the process. Concrete rather than abstract so a future + implementation with no LLM-backed segment is not forced to write an + empty override; ``ContextAssembler`` is the only one today and does + override it. + """ + @abstractmethod async def assemble( self, diff --git a/raven/context_engine/curator.py b/raven/context_engine/curator.py index 9864b213..26cda66f 100644 --- a/raven/context_engine/curator.py +++ b/raven/context_engine/curator.py @@ -24,6 +24,7 @@ from raven.memory_engine.base import AssembledContext, TokenBudget from raven.memory_engine.consolidate.consolidator import MemoryStore from raven.providers.base import LLMProvider +from raven.providers.binding import ModelBinding, resolve from raven.utils.helpers import ( ensure_dir, estimate_message_tokens, @@ -338,8 +339,7 @@ def __init__( get_tool_definitions: Callable[[], list[dict[str, Any]]], context_window_tokens: int, ): - self.provider = provider - self.model = model + self._fallback = ModelBinding(provider, model) self.get_tool_definitions = get_tool_definitions self.context_window_tokens = context_window_tokens self.trimmer = HistoryTrimmer( @@ -352,6 +352,21 @@ def __init__( # CuratorSegmentBuilder before any build/validate call. self.prefix: "AssembledPrefix | None" = None + @property + def provider(self) -> "LLMProvider": + """The provider of the turn's binding; the build-time one outside a turn.""" + return resolve(None, self._fallback).provider + + @property + def model(self) -> str: + """The model of the turn's binding; the build-time one outside a turn.""" + return resolve(None, self._fallback).model + + def set_provider(self, provider: LLMProvider, model: str) -> None: + """Adopt the provider a live ``/model`` switch just built.""" + self._fallback = ModelBinding(provider, model) + self.trimmer.set_provider(provider, model) + @staticmethod def working_state_segment(working_state: str | None) -> str: """Render segment 6 text (``# Curator Working State``) or ``""``.""" diff --git a/raven/context_engine/factory.py b/raven/context_engine/factory.py index 11f522d3..56a75a54 100644 --- a/raven/context_engine/factory.py +++ b/raven/context_engine/factory.py @@ -59,6 +59,7 @@ QueryRewriter, SkillForgeRouter, ) + from raven.providers.pool import ProviderPool from raven.skill_hub import SkillHubClient @@ -77,9 +78,14 @@ def build_context_engine( skill_forge_router_config: "SkillForgeRouterConfig | None" = None, skill_forge_config: "SkillForgeConfig | None" = None, skill_hub_client: "SkillHubClient | None" = None, + provider_pool: "ProviderPool | None" = None, ) -> ContextEngine: """Build the one :class:`ContextAssembler` from a flat SegmentBuilder list. + ``provider_pool``, when supplied, is what turns a subsystem's pinned model + into a pinned model *and its own credential*. Without it a pin has no + credential of its own and the subsystem follows the conversation's model. + ``config.engine`` is no longer a dispatch key — there is a single engine. The field is retained in :class:`ContextConfig` for config back-compat but is ignored here. ``builder`` is used only as the @@ -108,6 +114,7 @@ def build_context_engine( rewriter, gate = _build_rewriter_and_gate( provider=provider, + provider_pool=provider_pool, skill_forge_config=skill_forge_config, skill_forge_router_config=skill_forge_router_config, ) @@ -134,6 +141,11 @@ def build_context_engine( get_tool_definitions=get_tool_definitions, ), CuratorSegmentBuilder( + pin=( + provider_pool.bind_pin(config.curator_model, getattr(config, "curator_provider", None)) + if provider_pool + else None + ), workspace=workspace, config=config, provider=provider, @@ -218,6 +230,7 @@ def _build_rewriter_and_gate( provider: LLMProvider, skill_forge_config: "SkillForgeConfig | None", skill_forge_router_config: "SkillForgeRouterConfig", + provider_pool: "ProviderPool | None" = None, ) -> "tuple[QueryRewriter | None, LLMGateFilter | None]": """Construct the optional rewriter + gate from the parent SkillForge config. Both fall to ``None`` when their respective flag is off or @@ -249,6 +262,14 @@ def _build_rewriter_and_gate( max_select=int(getattr(skill_forge_config, "llm_gate_max_select", 2) or 2), legacy_top_k=int(skill_forge_router_config.top_k or 5), model=getattr(skill_forge_config, "llm_gate_model", None) or None, + pin=( + provider_pool.bind_pin( + getattr(skill_forge_config, "llm_gate_model", None), + getattr(skill_forge_config, "llm_gate_provider", None), + ) + if provider_pool + else None + ), temperature=float(getattr(skill_forge_config, "llm_gate_temperature", 0.0)), max_tokens=int(getattr(skill_forge_config, "llm_gate_max_tokens", 8192) or 8192), ) diff --git a/raven/context_engine/history_trimmer.py b/raven/context_engine/history_trimmer.py index d40d08ba..b9d592d2 100644 --- a/raven/context_engine/history_trimmer.py +++ b/raven/context_engine/history_trimmer.py @@ -27,6 +27,7 @@ from typing import Any, Callable from raven.providers.base import LLMProvider +from raven.providers.binding import ModelBinding, resolve from raven.utils.helpers import estimate_prompt_tokens_chain # Provider-safe message keys. Anything else on a session message @@ -75,11 +76,25 @@ def __init__( get_tool_definitions: Callable[[], list[dict[str, Any]]], context_window_tokens: int, ) -> None: - self.provider = provider - self.model = model + self._fallback = ModelBinding(provider, model) self.get_tool_definitions = get_tool_definitions self.context_window_tokens = context_window_tokens + @property + def provider(self) -> "LLMProvider": + """The provider of the turn's binding; the build-time one outside a turn.""" + return resolve(None, self._fallback).provider + + @property + def model(self) -> str: + """The model of the turn's binding; the build-time one outside a turn.""" + return resolve(None, self._fallback).model + + def set_provider(self, provider: LLMProvider, model: str) -> None: + """Adopt the provider a live ``/model`` switch just built, so token + estimates keep matching the model actually being called.""" + self._fallback = ModelBinding(provider, model) + # ------------------------------------------------------------------ # Pure history-shaping helpers (no token estimation / no I/O) # ------------------------------------------------------------------ diff --git a/raven/context_engine/segments/curator.py b/raven/context_engine/segments/curator.py index c443021b..764660aa 100644 --- a/raven/context_engine/segments/curator.py +++ b/raven/context_engine/segments/curator.py @@ -44,6 +44,7 @@ ) from raven.memory_engine.consolidate.consolidator import MemoryStore from raven.providers.base import LLMProvider +from raven.providers.binding import ModelBinding, resolve from raven.tracing import semconv, trace @@ -64,12 +65,17 @@ def __init__( get_tool_definitions: Callable[[], list[dict[str, Any]]], now_fn: Callable[[], datetime] | None = None, max_steps: int = 12, + pin: "ModelBinding | None" = None, ) -> None: self.workspace = workspace self.config = config - self.provider = provider - self.model = model - self.curator_model = config.curator_model or model + self._fallback = ModelBinding(provider, model) + # ``context.curator_model`` paired with its own credential, or None + # when it names a vendor Raven has no credentials for. None means the + # curator runs on the conversation's model, which is the configured + # rule for an unconfigured subsystem. + self._pin = pin + self._pin_warned = False self.context_window_tokens = context_window_tokens self.get_tool_definitions = get_tool_definitions self.max_steps = max_steps @@ -82,6 +88,51 @@ def __init__( ) self._turn_ids: dict[str, str] = {} + def set_provider(self, provider: LLMProvider, model: str) -> None: + """Adopt the provider a live ``/model`` switch just built. + + Only the out-of-turn fallback moves. Which model the curator runs on + is decided per call by ``_curator_binding``, so a session switching + models is already covered without touching anything here. + """ + self._fallback = ModelBinding(provider, model) + self.assembler.set_provider(provider, model) + + @property + def provider(self) -> LLMProvider: + return resolve(None, self._fallback).provider + + @property + def model(self) -> str: + return resolve(None, self._fallback).model + + @property + def curator_model(self) -> str: + """What the slow path is actually called with.""" + return self._curator_binding().model + + def _curator_binding(self) -> ModelBinding: + """Its own pinned pair if it has one, else the conversation's model. + + Unset out of the box, so the default is to follow the conversation. + + A pin becomes a pair only when the factory was given a + :class:`~raven.providers.pool.ProviderPool` and that pool could build + the pin's vendor from configured credentials. Either half missing + leaves ``_pin`` None and the curator follows the conversation, which is + reported once. Worth configuring properly: the slow path is a bounded + loop of up to ``max_steps`` tool-calling requests, which is per-turn + housekeeping, not an answer. + """ + if self.config.curator_model and self._pin is None and not self._pin_warned: + self._pin_warned = True + logger.warning( + "context.curator_model={!r} has no usable credentials of its own; " + "the curator follows the conversation's model instead", + self.config.curator_model, + ) + return resolve(self._pin, self._fallback) + async def build(self, ctx: AssemblyContext) -> Segment | None: if ctx.prefix is None: raise RuntimeError("CuratorSegmentBuilder requires ctx.prefix (phase B)") @@ -193,10 +244,11 @@ async def _slow_path(self, state: CuratorState, turn_id: str) -> Segment | None: "tools": registry.tool_names, }, ) - response = await self.provider.chat_with_retry( + binding = self._curator_binding() + response = await binding.provider.chat_with_retry( messages=messages, tools=registry.get_definitions(), - model=self.curator_model, + model=binding.model, max_tokens=2048, temperature=0.1, ) diff --git a/raven/context_engine/segments/skills.py b/raven/context_engine/segments/skills.py index 948a8465..dfd53b7a 100644 --- a/raven/context_engine/segments/skills.py +++ b/raven/context_engine/segments/skills.py @@ -41,6 +41,7 @@ from raven.memory_engine.skill_forge.gate import LLMGateFilter from raven.memory_engine.skill_forge.rewriter import QueryRewriter from raven.memory_engine.skill_forge.types import RouterHit + from raven.providers.base import LLMProvider from raven.skill_hub import SkillHubClient log = logging.getLogger(__name__) @@ -73,6 +74,14 @@ def __init__( self._hub_client = hub_client self._get_tool_definitions = get_tool_definitions + def set_provider(self, provider: "LLMProvider", model: str) -> None: + """Hand a live ``/model`` switch down to the two LLM users in this + segment. The router itself holds no provider.""" + if self._rewriter is not None: + self._rewriter.set_provider(provider, model) + if self._gate is not None: + self._gate.set_provider(provider, model) + @trace.instrument("skill.inject", kind="skill", detached=True, extract=semconv.skill_inject_skills) async def build(self, ctx: AssemblyContext) -> Segment | None: if self._router is None: diff --git a/raven/memory_engine/consolidate/consolidator.py b/raven/memory_engine/consolidate/consolidator.py index 853ce093..305e7098 100644 --- a/raven/memory_engine/consolidate/consolidator.py +++ b/raven/memory_engine/consolidate/consolidator.py @@ -13,6 +13,7 @@ from loguru import logger +from raven.providers.binding import ModelBinding, resolve from raven.tracing import semconv, trace from raven.utils.helpers import ensure_dir, estimate_message_tokens, estimate_prompt_tokens_chain @@ -1711,8 +1712,7 @@ def __init__( enable_foresight: bool = False, ): self.store = MemoryStore(workspace, now_fn=now_fn) - self.provider = provider - self.model = model + self._fallback = ModelBinding(provider, model) self.sessions = sessions self.context_window_tokens = context_window_tokens self._build_messages = build_messages @@ -1723,6 +1723,20 @@ def __init__( self.enable_foresight = enable_foresight self._locks: weakref.WeakValueDictionary[str, asyncio.Lock] = weakref.WeakValueDictionary() + @property + def provider(self) -> "LLMProvider": + """The provider of the turn's binding; the build-time one outside a turn.""" + return resolve(None, self._fallback).provider + + @property + def model(self) -> str: + """The model of the turn's binding; the build-time one outside a turn.""" + return resolve(None, self._fallback).model + + def set_provider(self, provider: "LLMProvider", model: str) -> None: + """Adopt the provider a live ``/model`` switch just built.""" + self._fallback = ModelBinding(provider, model) + def get_lock(self, session_key: str) -> asyncio.Lock: """Return the shared consolidation lock for one session.""" return self._locks.setdefault(session_key, asyncio.Lock()) diff --git a/raven/memory_engine/skill_forge/gate.py b/raven/memory_engine/skill_forge/gate.py index d9fbe33d..1670f064 100644 --- a/raven/memory_engine/skill_forge/gate.py +++ b/raven/memory_engine/skill_forge/gate.py @@ -24,6 +24,7 @@ from typing import TYPE_CHECKING from raven.memory_engine.skill_forge.types import RouterHit +from raven.providers.binding import ModelBinding, active_binding from raven.tracing import semconv, trace if TYPE_CHECKING: @@ -53,13 +54,58 @@ def __init__( model: str | None = None, temperature: float = 0.0, max_tokens: int = 8192, + pin: "ModelBinding | None" = None, ) -> None: - self._provider = provider + self._fallback_provider = provider self._max_select = max_select self._legacy_top_k = legacy_top_k self._model = model self._temperature = temperature self._max_tokens = max_tokens + # A pinned model paired with its own credential. Built by the caller + # from ``skill_forge.llm_gate_model``; None when unset or when that + # vendor has no credentials, in which case the gate follows the turn. + self._pin = pin + self._pin_warned = False + + def set_provider(self, provider: "LLMProvider", model: str) -> None: + """Move the out-of-turn fallback. + + Which model the gate runs on inside a turn is decided per call by + ``_binding``, so a session switching models needs nothing here. This + is only for the paths that filter skills outside a turn. + """ + self._fallback_provider = provider + + def _binding(self) -> tuple["LLMProvider", str | None]: + """Its own pinned pair if it has one, else the turn's model. + + Unpinned is the common case and the configured intent: the gate reads + the same model the conversation is on, whichever session that is. A + pin that named a vendor with no credentials never became a pair, so + it is reported once and then ignored rather than sent on the turn's + key -- that combination 401s every call and is swallowed by the top-N + fallback below, which is how it stayed invisible. + """ + if self._model and self._pin is None and not self._pin_warned: + self._pin_warned = True + log.warning( + "skill_forge.llm_gate_model=%r has no usable credentials of its own; " + "the gate follows the conversation's model instead", + self._model, + ) + if self._pin is not None: + return self._pin.provider, self._pin.model + turn = active_binding() + if turn is not None: + return turn.provider, turn.model + # Outside a turn: the provider it was built with, and no model at all, + # which is what tells that provider to use its own default. + # Never ``self._model``: an unpaired pin sent on this provider's key + # is the mis-pairing the pool exists to prevent. No model at all tells + # the provider to use its own default, which is what an unpinned gate + # gets anyway. + return self._fallback_provider, None @trace.instrument("skill.gate", kind="skill", extract=semconv.skill_gate) async def filter( @@ -72,12 +118,13 @@ async def filter( return [] catalog, by_id = self._build_catalog(candidates) prompt = self._build_prompt(task, catalog, available_tools) + gate_provider, gate_model = self._binding() try: resp = await asyncio.wait_for( - self._provider.chat_with_retry( + gate_provider.chat_with_retry( messages=[{"role": "user", "content": prompt}], - model=self._model or None, + model=gate_model, max_tokens=self._max_tokens, temperature=self._temperature, ), diff --git a/raven/memory_engine/skill_forge/rewriter.py b/raven/memory_engine/skill_forge/rewriter.py index 7009e03b..22cf2710 100644 --- a/raven/memory_engine/skill_forge/rewriter.py +++ b/raven/memory_engine/skill_forge/rewriter.py @@ -21,6 +21,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING +from raven.providers.binding import active_binding from raven.tracing import semconv, trace if TYPE_CHECKING: @@ -70,10 +71,30 @@ def __init__( max_tokens: int = 8192, temperature: float = 0.3, ) -> None: - self._provider = provider + self._fallback_provider = provider self._max_tokens = max_tokens self._temperature = temperature + def set_provider(self, provider: "LLMProvider", model: str) -> None: + """Adopt the provider a live ``/model`` switch just built. + + Only the out-of-turn fallback moves. Inside a turn the rewriter reads + that turn's binding, so a session switching models is already covered. + The rewriter has no model of its own, so it follows the conversation. + """ + self._fallback_provider = provider + + def _call_provider(self) -> "LLMProvider": + """The turn's provider inside a turn; the built-in one outside one. + + The rewriter has no model of its own, so it follows the conversation + by taking its provider and passing no model at all -- which lands on + that provider's default. For a pooled binding those coincide, because + the pool builds each provider with the bound model as its default. + """ + turn = active_binding() + return turn.provider if turn is not None else self._fallback_provider + @trace.instrument("skill.rewrite", kind="skill", extract=semconv.skill_rewrite) async def analyze(self, query: str) -> RewriteResult: truncated = (query or "").strip()[:_QUERY_MAX_LENGTH] @@ -83,7 +104,7 @@ async def analyze(self, query: str) -> RewriteResult: prompt = _REWRITE_PROMPT.format(query=truncated) try: resp = await asyncio.wait_for( - self._provider.chat_with_retry( + self._call_provider().chat_with_retry( messages=[{"role": "user", "content": prompt}], max_tokens=self._max_tokens, temperature=self._temperature, diff --git a/raven/providers/binding.py b/raven/providers/binding.py new file mode 100644 index 00000000..a9b60c8e --- /dev/null +++ b/raven/providers/binding.py @@ -0,0 +1,84 @@ +"""The model a piece of work is running on, scoped to that work. + +A model id and the credential that serves it are one pair, so they travel +together as a :class:`ModelBinding` rather than as two attributes someone +can update by halves. + +The binding is held in a :class:`~contextvars.ContextVar` rather than +passed down, for two reasons. The turn path reads it at sixteen places +across three packages -- ``raven.agent`` (the loop and the subagent +manager), ``raven.context_engine`` (the curator and the history trimmer) +and ``raven.memory_engine`` (the skill gate, the rewriter and the +consolidator) -- and threading a parameter through all of them would touch +far more code than it explains. +More importantly, ``asyncio.create_task`` copies the current context, so +work detached during a turn (a subagent, a consolidation task) keeps the +binding it was started under for its whole life, which is exactly the +semantics those tasks need: a subagent spawned before a model switch must +not finish on the model chosen after it. + +Nothing here builds providers. :mod:`raven.providers.pool` does that, and +:class:`~raven.agent.loop.main.AgentLoop` decides which binding a turn +runs under. +""" + +from __future__ import annotations + +from contextlib import contextmanager +from contextvars import ContextVar +from dataclasses import dataclass +from typing import TYPE_CHECKING, Iterator + +if TYPE_CHECKING: + from raven.providers.base import LLMProvider + + +@dataclass(frozen=True) +class ModelBinding: + """A model id and the provider whose credential serves it.""" + + provider: "LLMProvider" + model: str + + def __post_init__(self) -> None: + if not self.model: + raise ValueError("a ModelBinding needs a model id") + + +_ACTIVE: ContextVar["ModelBinding | None"] = ContextVar("raven_active_model_binding", default=None) + + +def active_binding() -> "ModelBinding | None": + """The binding the current turn runs under, or None outside a turn. + + Callers outside a turn (startup, a CLI one-shot, a test) get None and + should fall back to whatever default they were built with. + """ + return _ACTIVE.get() + + +@contextmanager +def use_binding(binding: "ModelBinding") -> Iterator["ModelBinding"]: + """Run a block -- and everything it awaits or spawns -- on one binding.""" + token = _ACTIVE.set(binding) + try: + yield binding + finally: + _ACTIVE.reset(token) + + +def resolve(pin: "ModelBinding | None", fallback: "ModelBinding") -> "ModelBinding": + """Which binding should a subsystem use for this call? + + Precedence is the configured rule: a subsystem with a model of its own, + paired with credentials of its own, uses that; otherwise it follows the + model of the turn it is running under; outside a turn it uses the fallback + it was built with. + + ``pin`` is already a pair -- a bare pinned model id never reaches here, + because a model id without a credential is what mis-pairs one vendor's key + with another's endpoint. + """ + if pin is not None: + return pin + return _ACTIVE.get() or fallback diff --git a/raven/providers/pool.py b/raven/providers/pool.py new file mode 100644 index 00000000..545cf63e --- /dev/null +++ b/raven/providers/pool.py @@ -0,0 +1,183 @@ +"""Builds a :class:`ModelBinding` for a model id, and caches it. + +One place answers "which credential serves this model", so a per-session +model, a subsystem pin and the configured default all resolve the same way +instead of each guessing. Caching matters because a session switching back +and forth must not rebuild a provider per turn -- building one imports +LiteLLM and writes vendor env vars. + +Provider resolution has to go through a config copy with the model +substituted: :meth:`Config._match_provider` short-circuits on a forced +``agents.defaults.provider``, so asking the live config about another +vendor's model would answer with the forced one and pair the wrong key. +""" + +from __future__ import annotations + +from collections.abc import Callable +from typing import TYPE_CHECKING + +from loguru import logger + +from raven.providers.binding import ModelBinding + +if TYPE_CHECKING: + from raven.config.schema import Config + + +class ProviderPool: + """Resolve and cache one provider per (provider name, model) pair.""" + + def __init__(self, config: "Config | Callable[[], Config]") -> None: + # A supplier, not a snapshot: a credential fixed after start (an OAuth + # re-login, an edited config file) has to be visible without a + # restart, which the old per-switch config reload gave for free. + # Cached bindings are dropped when the config that produced them is no + # longer the current one. + self._supplier = config if callable(config) else (lambda: config) + self._cache: dict[tuple[str, str], ModelBinding] = {} + self._cache_key: str | None = None + + @property + def config(self) -> "Config": + return self._supplier() + + def _live_cache(self) -> dict[tuple[str, str], ModelBinding]: + """Drop cached providers when the credentials behind them changed. + + Not identity on the config object: a supplier that re-reads the file + returns a new object every call, which would clear the cache every + time and defeat the pool. A fingerprint of what a provider is actually + built from is the thing that has to match. + """ + fingerprint = self._credentials_fingerprint() + if self._cache_key != fingerprint: + self._cache_key = fingerprint + self._cache = {} + return self._cache + + def _credentials_fingerprint(self) -> str: + """What a built provider depends on: the keys, bases and headers.""" + import hashlib + import json + + try: + providers = self.config.providers.model_dump(exclude_none=True) + except Exception: + return "" + return hashlib.sha256(json.dumps(providers, sort_keys=True, default=str).encode()).hexdigest() + + def bind(self, model: str, provider_name: str | None = None) -> ModelBinding: + """Build (or reuse) the provider that serves ``model``. + + ``provider_name`` is what the caller already knows -- the picker sends + one, and ``agents.defaults.provider`` is one. Absent or ``"auto"``, the + vendor is derived from the model id, the same derivation + ``config.set model`` uses for a hand-typed id. + """ + resolved = self._resolve_provider_name(model, provider_name) + cache = self._live_cache() + key = (resolved, model) + cached = cache.get(key) + if cached is not None: + return cached + + from raven.cli._helpers import make_provider + + cfg = self.config.model_copy(deep=True) + cfg.agents.defaults.model = model + cfg.agents.defaults.provider = resolved + binding = ModelBinding(make_provider(cfg), model) + cache[key] = binding + return binding + + def bind_pin(self, model: str | None, provider_name: str | None = None) -> ModelBinding | None: + """A subsystem's own model, on its own credential -- or None. + + This is what lets a pinned subsystem run off the session's model. None + means the pin is unusable, and the caller should fall back to the + session's binding rather than send one vendor's key to another. + + ``provider_name`` is the configured half of the pair, and when present + nothing is derived: an id alone cannot say whether ``anthropic`` or a + gateway reselling it is meant, and those are different credentials and + different bills. Absent, the vendor is guessed -- which is what a + config written before the provider field existed gets. + """ + if not model: + return None + if provider_name and provider_name != "auto": + configured = provider_name + if not self._has_credentials(configured, model): + # Explicitly configured and still unusable: a config error the + # user can fix, and silence here is what let a pinned + # subsystem look configured while never running. + logger.warning( + "pinned model {!r} names provider {!r}, which has no usable credentials; " + "the subsystem follows the conversation's model instead", + model, + configured, + ) + return None + resolved = configured + else: + # A gateway (or a local deployment) serves whatever id it is handed + # under its own credential, so the pin is already paired -- asking + # whether the upstream vendor has a key of its own would drop a pin + # that works. Bind it through the gateway instead. + gateway = self._configured_gateway() + resolved = gateway if gateway is not None else self._resolve_provider_name(model, None) + if gateway is None and not self._has_credentials(resolved, model): + return None + try: + return self.bind(model, resolved) + except Exception as exc: + # Called from the context-engine factory at construction, so this + # must leave the subsystem following the conversation rather than + # stop the agent from starting. Deliberately broad: building a + # provider imports a vendor module, so the failure modes are not + # only the credential ones. + logger.warning("cannot build a provider for pinned model {!r}: {}", model, exc) + return None + + def _configured_gateway(self) -> str | None: + """The agent's provider, when it is a gateway or a local deployment.""" + from raven.providers.registry import find_by_name + + forced = self.config.agents.defaults.provider + if not forced or forced == "auto": + return None + spec = find_by_name(forced) + if spec is None: + return None + return forced if (spec.is_gateway or spec.is_local) else None + + def _resolve_provider_name(self, model: str, provider_name: str | None) -> str: + if provider_name and provider_name != "auto": + return provider_name + from raven.providers.registry import find_by_model + + spec = find_by_model(model) + return spec.name if spec is not None else "auto" + + def _has_credentials(self, provider_name: str, model: str) -> bool: + """Is there a usable section for this vendor, or only a placeholder? + + Every declared provider exists as an empty section, so presence proves + nothing -- reuse the same check the credential preflight uses. + """ + from raven.config.schema import _has_credentials as section_is_usable + from raven.providers.registry import find_by_name + + if provider_name == "auto": + # No vendor could be derived, so there is no section to check and + # no way to tell a working pin from a mis-paired one. Treat it as + # unusable: the caller's fallback is the session's own binding, + # which is at least a pair. + return False + # ``providers.get`` is the only spelling-insensitive lookup; reading the + # attribute sees one spelling of a name that has several. + section = self.config.providers.get(provider_name) + if section is None: + return False + return bool(section_is_usable(section, find_by_name(provider_name))) diff --git a/raven/session/manager.py b/raven/session/manager.py index 6dda0cb3..132da6bd 100644 --- a/raven/session/manager.py +++ b/raven/session/manager.py @@ -440,6 +440,14 @@ def fork(self, source_key: str, *, title: str | None = None) -> "Session | None" if parent_title: child.metadata["title"] = f"{parent_title} (fork)" child.metadata["parent_session_id"] = source_key + # A fork continues its parent's conversation, so it continues on its + # parent's model. The caller re-points the live binding, but that lives + # in memory only -- without carrying the record too, the fork drops to + # the default the first time it is resumed in a new process. + for slot in ("model", "provider"): + inherited = (source.metadata or {}).get(slot) + if inherited: + child.metadata[slot] = inherited self.save(child) return child diff --git a/raven/tui_rpc/errors.py b/raven/tui_rpc/errors.py index 85be4b8d..3357a2f3 100644 --- a/raven/tui_rpc/errors.py +++ b/raven/tui_rpc/errors.py @@ -12,7 +12,6 @@ | -32006 | skill_not_found | skill_name not indexed | | -32007 | skill_pin_conflict | pin/unpin already in that state | | -32008 | model_not_available | model_id not routable | -| -32009 | model_switch_in_turn | switch attempt while turn live | | -32010 | config_field_readonly | not on hot-changeable whitelist | | -32011 | config_validation_error | Pydantic / semver validation | | -32012 | not_supported_in_v01 | hermes-only stub methods | @@ -101,11 +100,6 @@ class ModelNotAvailableError(RpcError): MESSAGE = "model_not_available" -class ModelSwitchInTurnError(RpcError): - CODE = -32009 - MESSAGE = "model_switch_in_turn" - - class ConfigFieldReadonlyError(RpcError): CODE = -32010 MESSAGE = "config_field_readonly" @@ -174,7 +168,6 @@ class InternalError(RpcError): SkillNotFoundError, SkillPinConflictError, ModelNotAvailableError, - ModelSwitchInTurnError, ConfigFieldReadonlyError, ConfigValidationError, NotSupportedInV01Error, @@ -197,7 +190,6 @@ class InternalError(RpcError): "SkillNotFoundError", "SkillPinConflictError", "ModelNotAvailableError", - "ModelSwitchInTurnError", "ConfigFieldReadonlyError", "ConfigValidationError", "NotSupportedInV01Error", diff --git a/raven/tui_rpc/methods/__init__.py b/raven/tui_rpc/methods/__init__.py index 3b503523..9155e875 100644 --- a/raven/tui_rpc/methods/__init__.py +++ b/raven/tui_rpc/methods/__init__.py @@ -129,7 +129,7 @@ def register_aligned_methods_except_system( # model.{options,save_key,disconnect,add_model,remove_model}: real handlers # must come AFTER register_stub_methods (Dispatcher.register raises on # duplicate; the stub group no longer owns these names). - register_model_methods(dispatcher) + register_model_methods(dispatcher, agent_loop_factory=agent_loop_factory) # harness-command-catalog-dynamic: real ``commands.catalog`` handler; # MUST come after ``register_stub_methods`` because the stub list dropped # its ``commands.catalog`` entry, and ``Dispatcher.register`` raises on diff --git a/raven/tui_rpc/methods/config.py b/raven/tui_rpc/methods/config.py index 91239a26..c5c13e1a 100644 --- a/raven/tui_rpc/methods/config.py +++ b/raven/tui_rpc/methods/config.py @@ -26,8 +26,11 @@ import json import re +from collections.abc import Callable from pathlib import Path -from typing import TYPE_CHECKING, Any, Callable +from typing import TYPE_CHECKING, Any + +from loguru import logger from raven.cli._helpers import load_runtime_config, make_provider from raven.providers.registry import find_by_model, find_by_name @@ -35,9 +38,7 @@ ConfigFieldReadonlyError, ConfigValidationError, ModelNotAvailableError, - ModelSwitchInTurnError, ) -from raven.tui_rpc.methods.turn import is_turn_active if TYPE_CHECKING: from raven.tui_rpc.dispatcher import Dispatcher @@ -322,10 +323,21 @@ def _set_model( raw_value: Any, agent_loop_factory: "AgentLoopFactory | None", ) -> dict: - """Switch the global model (and provider) and reassign the live loop. + """Switch the model this session runs on, or the default new ones start on. + + Two scopes, because they answer different questions. With a + ``session_id`` (what the picker sends) the switch is scoped to that + session: no other session moves, and ``agents.defaults`` is left alone so + a new session still starts on the configured default. Pass + ``scope="default"``, or omit ``session_id``, to change that default + instead; sessions that already switched keep their own model. - Build the provider from the prospective config BEFORE persisting, so a - rebuild failure aborts cleanly with the on-disk model untouched. + Either way the provider is built before anything is persisted or applied, + so a rebuild failure aborts with the on-disk model untouched. + + A switch during a turn is not refused. The running turn holds the binding + it started on for its whole tree, so the new model takes effect on the + session's next turn -- which is what a user asking mid-answer means. """ if not isinstance(raw_value, str) or not raw_value: raise ConfigValidationError( @@ -359,40 +371,122 @@ def _set_model( ) session_id = params.get("session_id") - if isinstance(session_id, str) and session_id and is_turn_active(session_id): - raise ModelSwitchInTurnError( - f"cannot switch model while session {session_id!r} has an active turn", - data={"session_id": session_id}, + scope = params.get("scope") + if scope not in (None, "session", "default"): + raise ConfigValidationError( + "config.set model scope must be 'session' or 'default'", + data={"field": "scope", "got": repr(scope)}, ) - - payload = _load_config() - previous = _get_nested(payload, "agents.defaults.model") + has_session = isinstance(session_id, str) and bool(session_id) + if scope == "session" and not has_session: + # Never widen a scope the caller narrowed: falling through to the + # default branch here would write agents.defaults and move every + # session that never switched. The TUI sends a session_id that is null + # until the first session.create resolves, so this is reachable. + raise ConfigValidationError( + "config.set model scope 'session' needs a session_id", + data={"field": "session_id", "got": repr(session_id)}, + ) + session_scoped = scope != "default" and has_session loop = agent_loop_factory() if agent_loop_factory is not None else None - built_provider = None + binding = None if loop is not None: runtime = load_runtime_config(None, None) runtime.agents.defaults.model = raw_value if new_provider is not None: runtime.agents.defaults.provider = new_provider try: - built_provider = make_provider(runtime) + binding = _build_binding(loop, runtime, raw_value, new_provider) except (SystemExit, RuntimeError, ValueError) as exc: raise ModelNotAvailableError( f"cannot build provider for model {raw_value!r}", data={"model": raw_value, "error": str(exc)}, ) from exc + if session_scoped: + if loop is None: + # Nothing was built, so nothing was validated -- do not report a + # switch that did not happen. + return {"applied": False, "previous": None, "value": raw_value, "scope": "session"} + previous = loop.session_model(session_id) + loop.set_session_binding(session_id, binding) + _remember_session_model(loop, session_id, raw_value, new_provider) + return { + "applied": True, + "previous": previous, + "value": raw_value, + "scope": "session", + "session_id": session_id, + "applies_to_session": True, + } + + # A default-scoped switch still moves the asking conversation when that + # conversation never chose a model of its own, because it reads the + # default. Answered here rather than inferred from the scope: the client + # cannot see which sessions have their own binding. + follows_default = None + if loop is not None and has_session: + has_own = getattr(loop, "has_session_binding", None) + if callable(has_own): + follows_default = not has_own(session_id) + + payload = _load_config() + previous = _get_nested(payload, "agents.defaults.model") _set_nested(payload, "agents.defaults.model", raw_value) if new_provider is not None: _set_nested(payload, "agents.defaults.provider", new_provider) _save_config(payload) if loop is not None: - loop.provider = built_provider - loop.model = raw_value + # Not a two-attribute assignment: the subagent manager, the context + # engine and the consolidator each hold a fallback for work that runs + # outside a turn, and this is what re-points them. + loop.set_default_binding(binding) + + return { + "applied": True, + "previous": previous, + "value": raw_value, + "scope": "default", + "applies_to_session": follows_default, + } + + +def _remember_session_model(loop: Any, session_key: str, model: str, provider_name: str | None) -> None: + """Persist the choice on the session, so a restart does not undo it. + + Stored on the session record rather than in ``agents.defaults``: it is + this conversation's model, and a new conversation must still start on the + configured default. + """ + sessions = getattr(loop, "sessions", None) + if sessions is None: + return + try: + session = sessions.get_or_create(session_key) + session.metadata["model"] = model + if provider_name: + session.metadata["provider"] = provider_name + sessions.save(session) + except Exception: + logger.warning("could not persist the model on session {!r}", session_key) + + +def _build_binding(loop: Any, runtime: Any, model: str, provider_name: str | None) -> Any: + """One provider per (vendor, model), reused across sessions and switches. + + Building one imports LiteLLM and writes vendor env vars, so a session + flipping between two models must not pay for it twice. The pool is the + loop's; without one (an older wiring, a test) fall back to building + directly. + """ + from raven.providers.binding import ModelBinding - return {"applied": True, "previous": previous, "value": raw_value} + pool = getattr(loop, "provider_pool", None) + if pool is not None: + return pool.bind(model, provider_name) + return ModelBinding(make_provider(runtime), model) def register_config_methods( diff --git a/raven/tui_rpc/methods/model.py b/raven/tui_rpc/methods/model.py index 044eb436..20a692d5 100644 --- a/raven/tui_rpc/methods/model.py +++ b/raven/tui_rpc/methods/model.py @@ -18,6 +18,7 @@ from __future__ import annotations import asyncio +from functools import partial from typing import TYPE_CHECKING, Any from pydantic import ValidationError @@ -56,6 +57,7 @@ if TYPE_CHECKING: from raven.tui_rpc.dispatcher import Dispatcher + from raven.tui_rpc.methods.session import AgentLoopFactory def _parse(model_cls: type, params: dict) -> Any: @@ -187,9 +189,20 @@ def _current_selection() -> tuple[str, str | None]: # --------------------------------------------------------------------------- -async def model_options(params: dict) -> dict: - _parse(ModelOptionsParams, params) +async def model_options(params: dict, *, agent_loop_factory: "AgentLoopFactory | None" = None) -> dict: + """Which models exist, and which one *this conversation* is on. + + The session matters: the model is per conversation now, so answering from + ``agents.defaults`` would star the wrong row for every session that has + switched -- the picker would disagree with the status bar it sits under. + """ + parsed = _parse(ModelOptionsParams, params) current_model, current_provider = _current_selection() + session_model = _session_model(agent_loop_factory, getattr(parsed, "session_id", None)) + if session_model: + current_model = session_model + spec = find_by_model(session_model) + current_provider = spec.name if spec else current_provider entries = await _entries_off_loop(current_provider) return { "model": current_model, @@ -302,9 +315,27 @@ async def model_remove_model(params: dict) -> dict: } -def register_model_methods(dispatcher: "Dispatcher") -> None: +def _session_model(agent_loop_factory: "AgentLoopFactory | None", session_id: str | None) -> str | None: + """This session's own model, or None when it never switched.""" + if not agent_loop_factory or not session_id: + return None + try: + loop = agent_loop_factory() + except Exception: + return None + # ``session_model`` falls back to the default, so it never answers None -- + # asking it alone would override a forced ``agents.defaults.provider`` for + # every session, including the ones that never switched. + has_own = getattr(loop, "has_session_binding", None) + if not callable(has_own) or not has_own(session_id): + return None + reader = getattr(loop, "session_model", None) + return reader(session_id) if callable(reader) else None + + +def register_model_methods(dispatcher: "Dispatcher", *, agent_loop_factory: "AgentLoopFactory | None" = None) -> None: """Register the five ``model.*`` handlers on a dispatcher instance.""" - dispatcher.register("model.options", model_options) + dispatcher.register("model.options", partial(model_options, agent_loop_factory=agent_loop_factory)) dispatcher.register("model.save_key", model_save_key) dispatcher.register("model.disconnect", model_disconnect) dispatcher.register("model.add_model", model_add_model) diff --git a/raven/tui_rpc/methods/session.py b/raven/tui_rpc/methods/session.py index f1f10f7a..11353d45 100644 --- a/raven/tui_rpc/methods/session.py +++ b/raven/tui_rpc/methods/session.py @@ -132,16 +132,48 @@ def _baseline_usage( } +def _restore_session_model(agent_loop: "AgentLoop", session_key: str) -> None: + """Re-apply the model this session was last switched to. + + Overrides live in the loop's memory, so a restart would otherwise move + every switched session back to the default. The session record is the only + place that choice survives, and this is the only reader of it. + """ + restore = getattr(agent_loop, "restore_session_model", None) + sessions = getattr(agent_loop, "sessions", None) + if not callable(restore) or sessions is None: + return + try: + record = sessions.peek(session_key) + except Exception: + return + metadata = getattr(record, "metadata", None) or {} + model = metadata.get("model") + if model: + restore(session_key, model, metadata.get("provider")) + + def _default_session_info( agent_loop: "AgentLoop | None", config: "Config", + session_key: str | None = None, ) -> dict[str, Any]: """Build the init bundle returned by ``session.create`` / ``session.resume``. ``agent_loop=None`` triggers graceful fallback (``tools={}``, ``skills={}``, zero usage, ``lazy=True``); version is always real (cached at module load). + + The model reported is the one this session runs on, not the configured + default: a session that switched has its own, and reporting the default + would show every other session's user the wrong model. With no + ``session_key`` (a session being created) the default is the right answer, + because that is what a new session starts on. """ model_id = config.agents.defaults.model + if session_key and agent_loop is not None: + session_model = getattr(agent_loop, "session_model", None) + if callable(session_model): + model_id = session_model(session_key) info: dict[str, Any] = { "model": model_id, "model_id": model_id, @@ -284,8 +316,10 @@ async def session_resume( """ agent_loop = _safe_invoke_factory(agent_loop_factory) config = load_config() - info = _default_session_info(agent_loop, config) session_key = params.get("session_id") + if isinstance(session_key, str) and session_key and agent_loop is not None: + _restore_session_model(agent_loop, session_key) + info = _default_session_info(agent_loop, config, session_key if isinstance(session_key, str) else None) if session_key: try: @@ -379,6 +413,12 @@ async def session_delete( config = load_config() mgr = _manager_for(agent_loop, config) removed = mgr.delete(session_key) + if removed and agent_loop is not None: + # Overrides are per session and live for the process; a deleted + # session must not leave one behind. + clear = getattr(agent_loop, "clear_session_binding", None) + if callable(clear): + clear(session_key) return {"deleted": session_key if removed else None} @@ -535,6 +575,14 @@ async def session_branch( config = load_config() mgr = _manager_for(agent_loop, config) child = mgr.fork(session_key, title=(name or None)) + if child is not None and agent_loop is not None: + # A fork continues its parent's conversation, so it continues on the + # parent's model; without this it would silently drop to the default. + binding_for = getattr(agent_loop, "binding_for_session", None) + setter = getattr(agent_loop, "set_session_binding", None) + has_own = getattr(agent_loop, "has_session_binding", None) + if callable(binding_for) and callable(setter) and callable(has_own) and has_own(session_key): + setter(child.key, binding_for(session_key)) if child is None: return {"session_id": None, "title": None} return { diff --git a/raven/tui_rpc/models.py b/raven/tui_rpc/models.py index 6355bf09..0c944399 100644 --- a/raven/tui_rpc/models.py +++ b/raven/tui_rpc/models.py @@ -606,6 +606,11 @@ class ConfigGetResult(_Strict): class ConfigSetParams(_Strict): key: str value: JsonValue + # Model-switch extras. ``scope`` decides the reach of a ``key="model"`` + # switch: this conversation, or the default a new one starts on. + session_id: str | None = None + provider: str | None = None + scope: Literal["session", "default"] | None = None class ConfigSetResult(_Strict): @@ -615,6 +620,14 @@ class ConfigSetResult(_Strict): # already includes ``null``; the schema's redundant ``oneOf: [JsonValue, # null]`` collapses to the same canonical "any" form. previous: JsonValue = Field(...) + # Present on a model switch: what was applied, and where it reached. + value: str | None = None + scope: Literal["session", "default"] | None = None + session_id: str | None = None + # Does the asking conversation now run this model? A default-scoped switch + # moves the sessions that never chose one, so scope alone cannot answer it + # and a client that guesses shows a model the conversation is not on. + applies_to_session: bool | None = None # --------------------------------------------------------------------------- diff --git a/tests/test_agent_loop_model_switch.py b/tests/test_agent_loop_model_switch.py new file mode 100644 index 00000000..8ee8fb68 --- /dev/null +++ b/tests/test_agent_loop_model_switch.py @@ -0,0 +1,158 @@ +"""A live model switch must reach everything holding the old provider. + +``config.set key="model"`` builds a fresh provider and hands it to +``AgentLoop.set_provider``. The loop is not the only holder: the subagent +manager, the context engine's LLM-backed segments and the consolidator +each captured the provider they were built with. When the switch stopped +at ``loop.provider``, those three kept calling the abandoned endpoint -- +subagent spawns and the skill rewriter/gate failed to authenticate while +the main loop worked fine. + +This file covers the out-of-turn fallback only -- the reference each holder +keeps for work that runs with no turn bound. What a turn actually runs on is +per session and lives in ``test_agent_loop_session_model.py``. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +from raven.agent.loop.main import AgentLoop +from raven.config.raven import ContextConfig, SkillForgeConfig +from raven.context_engine.assembler import ContextAssembler +from raven.context_engine.segments.curator import CuratorSegmentBuilder +from raven.context_engine.segments.skills import SkillsSegmentBuilder +from raven.providers.base import LLMResponse +from raven.providers.binding import ModelBinding + +NEW_MODEL = "anthropic/claude-opus-4-8" + + +class _Provider: + """Minimal provider stand-in; identity is what the assertions track.""" + + def __init__(self, name: str = "old") -> None: + self.name = name + + def get_default_model(self) -> str: + return "fake/model" + + async def chat_with_retry(self, **kwargs) -> LLMResponse: + return LLMResponse(content="ok", finish_reason="stop") + + +class _Recorder: + """Holder that remembers the provider/model it was last pointed at.""" + + def __init__(self) -> None: + self.provider: object = "old-provider" + self.model = "old-model" + + def set_provider(self, provider: object, model: str) -> None: + self.provider = provider + self.model = model + + +class _TextOnlyBuilder: + """A segment that never calls an LLM, so it has no set_provider.""" + + name = "identity" + order = 1 + needs_prefix = False + + async def build(self, ctx): # pragma: no cover - never invoked here + return None + + +def _loop(tmp_path) -> AgentLoop: + return AgentLoop( + provider=_Provider(), + workspace=tmp_path, + model="fake/model", + context_config=ContextConfig(), + skill_forge_config=SkillForgeConfig(), + ) + + +# --------------------------------------------------------------------------- +# Fan-out +# --------------------------------------------------------------------------- + + +def test_set_provider_reaches_every_holder() -> None: + loop = object.__new__(AgentLoop) + loop.subagents = _Recorder() + loop.context_engine = _Recorder() + loop.memory_consolidator = _Recorder() + loop._provider_pool = None + loop._default_binding = ModelBinding(_Provider("old"), "old-model") + loop._session_bindings = {} + + new_provider = _Provider("new-provider") + loop.set_provider(new_provider, NEW_MODEL) + + assert loop.provider is new_provider + assert loop.model == NEW_MODEL + for holder in (loop.subagents, loop.context_engine, loop.memory_consolidator): + assert holder.provider is new_provider + assert holder.model == NEW_MODEL + + +def test_switch_reaches_the_real_holders_a_loop_builds(tmp_path) -> None: + """The stubbed fan-out above proves the dispatcher; this proves the + receivers. Every holder here is the class a real run uses, reached by + walking the engine the factory actually assembled -- so a setter that is + renamed, dropped, or quietly wrong fails here instead of in production. + """ + loop = _loop(tmp_path) + engine = loop.context_engine + assert isinstance(engine, ContextAssembler) + + skills = next(b for b in engine._builders if isinstance(b, SkillsSegmentBuilder)) + curator = next(b for b in engine._builders if isinstance(b, CuratorSegmentBuilder)) + assert skills._gate is not None, "llm_gate_enabled defaults True; the gate is a holder" + assert skills._rewriter is not None + + new_provider = _Provider("new") + loop.set_provider(new_provider, NEW_MODEL) + + assert loop.provider is new_provider + assert loop.subagents.provider is new_provider + assert loop.subagents.model == NEW_MODEL + assert loop.memory_consolidator.provider is new_provider + assert skills._gate._fallback_provider is new_provider + assert skills._rewriter._fallback_provider is new_provider + assert curator.provider is new_provider + assert curator.assembler.provider is new_provider + assert curator.assembler.trimmer.provider is new_provider + + +def test_fan_out_targets_still_exist_on_a_real_loop(tmp_path) -> None: + """Guard against silent drift. ``ContextAssembler.set_provider`` walks its + builders duck-typed and skips anything without the method, so a renamed + holder or setter would leave the fan-out green while it quietly stops + covering that subsystem. + """ + loop = _loop(tmp_path) + for attr in ("subagents", "context_engine", "memory_consolidator"): + holder = getattr(loop, attr, None) + assert holder is not None, f"AgentLoop.{attr} is gone; set_provider still fans out to it" + assert callable(getattr(holder, "set_provider", None)), f"AgentLoop.{attr} lost set_provider" + + +def test_assembler_forwards_to_llm_backed_builders_only() -> None: + llm_builder = _Recorder() + llm_builder.name = "skills" + llm_builder.order = 5 + llm_builder.needs_prefix = False + text_builder = _TextOnlyBuilder() + + assembler = ContextAssembler([llm_builder, text_builder], lambda: []) + new_provider = SimpleNamespace(name="new-provider") + + # The text-only builder has no set_provider; walking must skip it rather + # than blow up, which is why the fan-out is duck-typed. + assembler.set_provider(new_provider, NEW_MODEL) + + assert llm_builder.provider is new_provider + assert llm_builder.model == NEW_MODEL diff --git a/tests/test_agent_loop_session_model.py b/tests/test_agent_loop_session_model.py new file mode 100644 index 00000000..a28286dd --- /dev/null +++ b/tests/test_agent_loop_session_model.py @@ -0,0 +1,622 @@ +"""The model is per conversation, and one conversation's switch is its own. + +Five rules, in the order a user would state them: + +1. different sessions can be on different models; +2. switching one session does not move another; +3. a new session starts on the configured default, not on whatever the last + session switched to; +4. a subsystem with a model *and credentials* of its own uses them; without + both it follows the model of the conversation it is running under; +5. a switch that arrives while a turn is running takes effect on the next + turn, not in the middle of this one. + +Rule 5 is not a mechanism here, it is a consequence: ``run_turn`` resolves the +session's binding once and holds it in a context var for the whole turn tree, +so a switch landing mid-turn is simply not visible to that turn. The same +context copy is what makes a detached subagent finish on the model it was +spawned under. +""" + +from __future__ import annotations + +import asyncio + +import pytest + +from raven.agent.loop.main import AgentLoop +from raven.config.raven import ContextConfig, SkillForgeConfig +from raven.context_engine.segments.curator import CuratorSegmentBuilder +from raven.context_engine.segments.skills import SkillsSegmentBuilder +from raven.providers.base import LLMResponse +from raven.providers.binding import ModelBinding, active_binding +from raven.spine.message import ChatType, Source +from raven.spine.turn import Origin, TurnRequest + + +class _Provider: + def __init__(self, name: str) -> None: + self.name = name + + def get_default_model(self) -> str: + return f"{self.name}/default" + + async def chat_with_retry(self, **kwargs) -> LLMResponse: + return LLMResponse(content="ok", finish_reason="stop") + + +def _loop(tmp_path) -> AgentLoop: + return AgentLoop( + provider=_Provider("boot"), + workspace=tmp_path, + model="boot/model", + context_config=ContextConfig(), + skill_forge_config=SkillForgeConfig(), + ) + + +def _req(session_key: str) -> TurnRequest: + return TurnRequest( + origin=Origin.USER, + source=Source(channel="tui", chat_id="default", sender_id="user", chat_type=ChatType.DM), + text="hi", + conversation=session_key, + ) + + +def _binding(name: str, model: str) -> ModelBinding: + return ModelBinding(_Provider(name), model) + + +async def _run(loop: AgentLoop, session_key: str, body) -> object: + loop._run_turn = body + return await loop.run_turn(_req(session_key), None, None) + + +# --------------------------------------------------------------------------- +# 1 + 2 + 3: scope +# --------------------------------------------------------------------------- + + +def test_a_session_without_a_switch_is_on_the_default(tmp_path) -> None: + loop = _loop(tmp_path) + assert loop.session_model("tui:a") == "boot/model" + assert loop.binding_for_session("tui:a") is loop.default_binding + + +def test_two_sessions_can_be_on_two_models(tmp_path) -> None: + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + loop.set_session_binding("tui:b", _binding("prov-b", "vendor-b/model")) + + assert loop.session_model("tui:a") == "vendor-a/model" + assert loop.session_model("tui:b") == "vendor-b/model" + + +def test_switching_one_session_leaves_the_others_alone(tmp_path) -> None: + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + + assert loop.session_model("tui:b") == "boot/model", "an untouched session stays on the default" + assert loop.default_binding.model == "boot/model", "a session switch is not a default change" + + +def test_a_new_session_starts_on_the_default_not_the_last_switch(tmp_path) -> None: + """Rule 3. A session-scoped switch is deliberately not sticky: the next + session created reads the configured default again. + """ + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + + assert loop.session_model("tui:fresh") == "boot/model" + + +def test_changing_the_default_moves_only_sessions_that_never_switched(tmp_path) -> None: + loop = _loop(tmp_path) + loop.set_session_binding("tui:pinned", _binding("prov-a", "vendor-a/model")) + + loop.set_default_binding(_binding("prov-new", "vendor-new/model")) + + assert loop.session_model("tui:pinned") == "vendor-a/model" + assert loop.session_model("tui:drifting") == "vendor-new/model" + + +def test_dropping_a_session_override_returns_it_to_the_default(tmp_path) -> None: + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + loop.clear_session_binding("tui:a") + + assert loop.session_model("tui:a") == "boot/model" + + +# --------------------------------------------------------------------------- +# The turn boundary +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_a_turn_runs_on_its_own_session_model(tmp_path) -> None: + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + + seen: dict[str, object] = {} + + async def _body(*args, **kwargs): + # Everything under the turn reads the same pair, including the holders + # that used to keep a reference of their own. + seen["loop"] = (loop.provider.name, loop.model) + seen["subagents"] = (loop.subagents.provider.name, loop.subagents.model) + seen["consolidator"] = loop.memory_consolidator.provider.name + return "done" + + assert await _run(loop, "tui:a", _body) == "done" + assert seen["loop"] == ("prov-a", "vendor-a/model") + assert seen["subagents"] == ("prov-a", "vendor-a/model") + assert seen["consolidator"] == "prov-a" + + +@pytest.mark.asyncio +async def test_outside_a_turn_the_loop_reports_the_default(tmp_path) -> None: + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + + assert active_binding() is None + assert loop.model == "boot/model" + + +@pytest.mark.asyncio +async def test_two_concurrent_turns_each_keep_their_own_model(tmp_path) -> None: + """Rules 1 and 2 have to hold while both turns are in flight, which is the + case a shared provider could never express: a user turn and a cron turn run + at the same time on this loop. + """ + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + loop.set_session_binding("cron:job", _binding("prov-cron", "vendor-cron/model")) + + a_started = asyncio.Event() + release_a = asyncio.Event() + seen: dict[str, str] = {} + + async def _body(req, *args, **kwargs): + key = req.conversation + if key == "tui:a": + a_started.set() + await release_a.wait() + seen[key] = loop.model + return key + + loop._run_turn = _body + a = asyncio.create_task(loop.run_turn(_req("tui:a"), None, None)) + await a_started.wait() + await loop.run_turn(_req("cron:job"), None, None) + release_a.set() + await a + + assert seen["cron:job"] == "vendor-cron/model" + assert seen["tui:a"] == "vendor-a/model", "the cron turn must not have moved the user turn" + + +@pytest.mark.asyncio +async def test_a_switch_mid_turn_lands_on_the_next_turn(tmp_path) -> None: + """Rule 5, with no parking involved: the running turn holds the binding it + entered on, so the switch is invisible to it and current for the next one. + """ + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-old", "vendor-old/model")) + + started = asyncio.Event() + release = asyncio.Event() + during: list[str] = [] + + async def _body(req, *args, **kwargs): + started.set() + await release.wait() + during.append(loop.model) + return "done" + + loop._run_turn = _body + running = asyncio.create_task(loop.run_turn(_req("tui:a"), None, None)) + await started.wait() + + loop.set_session_binding("tui:a", _binding("prov-new", "vendor-new/model")) + release.set() + await running + + assert during == ["vendor-old/model"], "the turn in flight must not move" + + after: list[str] = [] + + async def _next(req, *args, **kwargs): + after.append(loop.model) + return "done" + + await _run(loop, "tui:a", _next) + assert after == ["vendor-new/model"] + + +@pytest.mark.asyncio +async def test_the_turn_binding_is_released_when_the_turn_raises(tmp_path) -> None: + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + + async def _boom(*args, **kwargs): + raise RuntimeError("turn failed") + + loop._run_turn = _boom + with pytest.raises(RuntimeError): + await loop.run_turn(_req("tui:a"), None, None) + + assert active_binding() is None + assert loop.model == "boot/model" + + +# --------------------------------------------------------------------------- +# Rule 4: subsystems +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_a_spawned_subagent_keeps_its_conversations_model(tmp_path) -> None: + """A subagent has no model of its own, so it follows the conversation that + spawned it -- and keeps doing so after that conversation switches, because + it is a detached task that outlives the turn. + """ + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + + captured: dict[str, object] = {} + + async def _capture(task_id, task, label, origin, provider, model): + captured["pair"] = (provider.name, model) + + loop.subagents._run_subagent = _capture + loop.subagents._gate = asyncio.Semaphore(0) + + async def _body(*args, **kwargs): + await loop.subagents.spawn("do it", label="it", session_key="tui:a") + return "done" + + await _run(loop, "tui:a", _body) + loop.set_session_binding("tui:a", _binding("prov-new", "vendor-new/model")) + await asyncio.sleep(0) + + assert captured["pair"] == ("prov-a", "vendor-a/model") + + +@pytest.mark.asyncio +async def test_an_unconfigured_subsystem_follows_the_conversation(tmp_path) -> None: + """The gate and the curator are unpinned here (no credentials for the + default ``curator_model``), so both read the turn's model. + """ + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + skills = next(b for b in loop.context_engine._builders if isinstance(b, SkillsSegmentBuilder)) + curator = next(b for b in loop.context_engine._builders if isinstance(b, CuratorSegmentBuilder)) + + seen: dict[str, object] = {} + + async def _body(*args, **kwargs): + seen["gate"] = skills._gate._binding()[1] + seen["curator"] = curator.curator_model + seen["rewriter"] = skills._rewriter._call_provider().name + return "done" + + await _run(loop, "tui:a", _body) + assert seen["gate"] == "vendor-a/model" + assert seen["curator"] == "vendor-a/model" + assert seen["rewriter"] == "prov-a" + + +@pytest.mark.asyncio +async def test_a_configured_subsystem_uses_its_own_pair(tmp_path) -> None: + """With a model *and* a credential of its own, a subsystem stops following + the conversation -- that is the whole point of configuring one. + """ + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + skills = next(b for b in loop.context_engine._builders if isinstance(b, SkillsSegmentBuilder)) + curator = next(b for b in loop.context_engine._builders if isinstance(b, CuratorSegmentBuilder)) + + pin = _binding("prov-pin", "vendor-pin/small") + skills._gate._pin = pin + curator._pin = pin + + seen: dict[str, object] = {} + + async def _body(*args, **kwargs): + seen["gate"] = skills._gate._binding()[1] + seen["curator"] = curator.curator_model + return "done" + + await _run(loop, "tui:a", _body) + assert seen["gate"] == "vendor-pin/small" + assert seen["curator"] == "vendor-pin/small" + + +def test_the_gate_sends_no_model_outside_a_turn(tmp_path) -> None: + """An unpaired pin must never go out on the fallback provider's key. With + no turn bound there is nothing to follow, so the gate asks the provider for + its own default rather than posting a model id it has no credential for. + """ + loop = _loop(tmp_path) + skills = next(b for b in loop.context_engine._builders if isinstance(b, SkillsSegmentBuilder)) + skills._gate._model = "openai/gpt-5-mini" + skills._gate._pin = None + + assert active_binding() is None + assert skills._gate._binding()[1] is None + + +@pytest.mark.asyncio +async def test_a_request_without_a_conversation_falls_back_to_its_channel_key(tmp_path) -> None: + """Non-TUI channels and cron arrive with no ``conversation``; the + ``channel:chat_id`` key is the only one they get, so a switch stored under + it has to be the one their turn runs on. + """ + loop = _loop(tmp_path) + loop.set_session_binding("whatsapp:12345", _binding("prov-wa", "vendor-wa/model")) + + seen: list[str] = [] + + async def _body(*args, **kwargs): + seen.append(loop.model) + return "done" + + loop._run_turn = _body + req = TurnRequest( + origin=Origin.USER, + source=Source(channel="whatsapp", chat_id="12345", sender_id="u", chat_type=ChatType.DM), + text="hi", + ) + await loop.run_turn(req, None, None) + + assert seen == ["vendor-wa/model"] + + +def test_a_configured_pin_survives_a_real_factory_build(tmp_path) -> None: + """The pin only becomes a pair when a pool is wired, and every entry point + must wire one -- without it a correctly credentialed ``curator_model`` is + silently ignored and the user is told it has no credentials. + """ + from raven.config.schema import Config + from raven.providers.pool import ProviderPool + + cfg = Config() + cfg.agents.defaults.model = "claude-opus-4-5" + cfg.agents.defaults.provider = "auto" + cfg.providers.anthropic.api_key = "sk-ant" + cfg.providers.gemini.api_key = "AIza" + + context_config = ContextConfig(curator_model="gemini-2.5-flash") + loop = AgentLoop( + provider=_Provider("boot"), + workspace=tmp_path, + model="boot/model", + context_config=context_config, + skill_forge_config=SkillForgeConfig(), + provider_pool=ProviderPool(cfg), + ) + curator = next(b for b in loop.context_engine._builders if isinstance(b, CuratorSegmentBuilder)) + + assert curator._pin is not None, "a credentialed curator_model must become a pair" + assert curator.curator_model == "gemini-2.5-flash" + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + assert curator.curator_model == "gemini-2.5-flash", "a configured subsystem does not follow the turn" + + +def test_the_factory_hands_the_pool_the_pin_the_user_configured(tmp_path) -> None: + """Both halves of the configured pair have to reach the pool. + + A gateway serving another vendor's model is exactly the case the id cannot + express: derived from ``claude-haiku-4-5`` the vendor is Anthropic, and the + curator would run on the Anthropic key while the user asked for the + gateway. Dropping the provider argument at the factory leaves the pin + looking configured and pointed at the wrong bill. + """ + from raven.config.schema import Config + from raven.providers.pool import ProviderPool + + cfg = Config() + cfg.agents.defaults.model = "claude-opus-4-5" + cfg.agents.defaults.provider = "auto" + cfg.providers.anthropic.api_key = "sk-ant" + cfg.providers.openrouter.api_key = "sk-or" + + context_config = ContextConfig(curator_model="claude-haiku-4-5", curator_provider="openrouter") + loop = AgentLoop( + provider=_Provider("boot"), + workspace=tmp_path, + model="boot/model", + context_config=context_config, + skill_forge_config=SkillForgeConfig(), + provider_pool=ProviderPool(cfg), + ) + curator = next(b for b in loop.context_engine._builders if isinstance(b, CuratorSegmentBuilder)) + + assert curator._pin is not None + assert curator._pin.provider.api_key == "sk-or", "the configured provider serves the pin" + + +def test_the_factory_hands_the_pool_the_gate_pin_the_user_configured(tmp_path) -> None: + """Same wiring, the other pin.""" + from raven.config.schema import Config + from raven.context_engine.segments.skills import SkillsSegmentBuilder + from raven.providers.pool import ProviderPool + + cfg = Config() + cfg.agents.defaults.model = "claude-opus-4-5" + cfg.agents.defaults.provider = "auto" + cfg.providers.anthropic.api_key = "sk-ant" + cfg.providers.openrouter.api_key = "sk-or" + + loop = AgentLoop( + provider=_Provider("boot"), + workspace=tmp_path, + model="boot/model", + context_config=ContextConfig(), + skill_forge_config=SkillForgeConfig( + llm_gate_model="claude-haiku-4-5", + llm_gate_provider="openrouter", + ), + provider_pool=ProviderPool(cfg), + ) + skills = next(b for b in loop.context_engine._builders if isinstance(b, SkillsSegmentBuilder)) + + assert skills._gate is not None + assert skills._gate._pin is not None + assert skills._gate._pin.provider.api_key == "sk-or" + + +def test_a_stored_model_is_restored_onto_a_resumed_session(tmp_path) -> None: + """The write half is useless without this read half. A switch has to + survive a restart, or the user's choice lasts exactly as long as the + process -- and the persistence looks like it works while doing nothing. + """ + from raven.config.schema import Config + from raven.providers.pool import ProviderPool + + cfg = Config() + cfg.agents.defaults.model = "claude-opus-4-5" + cfg.agents.defaults.provider = "auto" + cfg.providers.anthropic.api_key = "sk-ant" + loop = AgentLoop( + provider=_Provider("boot"), + workspace=tmp_path, + model="boot/model", + context_config=ContextConfig(), + skill_forge_config=SkillForgeConfig(), + provider_pool=ProviderPool(cfg), + ) + + assert loop.session_model("tui:a") == "boot/model", "a fresh process has no override" + + loop.restore_session_model("tui:a", "claude-sonnet-4-5") + + assert loop.session_model("tui:a") == "claude-sonnet-4-5" + assert loop.has_session_binding("tui:a") + + +def test_a_stored_model_that_cannot_be_built_leaves_the_default(tmp_path) -> None: + """A credential removed since the switch must not fail the resume.""" + from raven.config.schema import Config + from raven.providers.pool import ProviderPool + + cfg = Config() + cfg.agents.defaults.model = "claude-opus-4-5" + loop = AgentLoop( + provider=_Provider("boot"), + workspace=tmp_path, + model="boot/model", + context_config=ContextConfig(), + skill_forge_config=SkillForgeConfig(), + provider_pool=ProviderPool(cfg), + ) + + loop.restore_session_model("tui:a", "gemini-2.5-flash") + + assert loop.session_model("tui:a") == "boot/model" + assert not loop.has_session_binding("tui:a") + + +def test_has_session_binding_distinguishes_chosen_from_inherited(tmp_path) -> None: + """``session_model`` falls back to the default, so it cannot answer this -- + and callers that override a forced provider need the difference. + """ + loop = _loop(tmp_path) + assert not loop.has_session_binding("tui:a") + + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + + assert loop.has_session_binding("tui:a") + assert not loop.has_session_binding("tui:b") + + +def test_a_configured_gate_pin_survives_a_real_factory_build(tmp_path) -> None: + """The gate's pin is built in the same factory as the curator's but was + uncovered -- and a gate failure is swallowed by its top-N fallback, so + losing the pin degrades silently by design. + """ + from raven.config.schema import Config + from raven.providers.pool import ProviderPool + + cfg = Config() + cfg.agents.defaults.model = "claude-opus-4-5" + cfg.agents.defaults.provider = "auto" + cfg.providers.anthropic.api_key = "sk-ant" + cfg.providers.openai.api_key = "sk-openai" + + loop = AgentLoop( + provider=_Provider("boot"), + workspace=tmp_path, + model="boot/model", + context_config=ContextConfig(), + skill_forge_config=SkillForgeConfig(llm_gate_model="openai/gpt-5-mini"), + provider_pool=ProviderPool(cfg), + ) + skills = next(b for b in loop.context_engine._builders if isinstance(b, SkillsSegmentBuilder)) + + assert skills._gate is not None + assert skills._gate._pin is not None, "a credentialed llm_gate_model must become a pair" + assert skills._gate._binding()[1] == "openai/gpt-5-mini" + + loop.set_session_binding("tui:a", _binding("prov-a", "vendor-a/model")) + assert skills._gate._binding()[1] == "openai/gpt-5-mini", "a configured subsystem does not follow the turn" + + +@pytest.mark.asyncio +async def test_a_spawn_holds_its_binding_through_the_gate_and_the_sandbox_boot(tmp_path) -> None: + """The binding is taken in ``spawn``, not where the task starts running: a + spawn waits on the concurrency gate and a sandbox boot first, and a switch + landing in that window would hand it an endpoint chosen after it was asked + for. Driven through the real ``_run_subagent`` so the window is genuinely + open -- stubbing it would prove only that ``spawn`` passes a pair. + """ + import raven.agent.subagent.manager as manager_mod + from raven.providers.base import LLMResponse as _Resp + + served: list[str] = [] + + class _Recording(_Provider): + async def chat_with_retry(self, **kwargs) -> _Resp: + served.append(self.name) + return _Resp(content="done", finish_reason="stop") + + class _StubExecutor: + @property + def is_sandboxed(self) -> bool: + return False + + async def exec(self, command: str, **kwargs): # pragma: no cover - unused + raise NotImplementedError + + async def __aenter__(self): + return self + + async def __aexit__(self, *exc): + return False + + loop = _loop(tmp_path) + loop.set_session_binding("tui:a", ModelBinding(_Recording("started-with"), "started/model")) + loop.subagents._submit = lambda *a, **k: None + loop.subagents._gate = asyncio.Semaphore(0) + + original_build = manager_mod.build_executor + manager_mod.build_executor = lambda cfg, workspace, owned_ids=None: _StubExecutor() + try: + + async def _body(*args, **kwargs): + await loop.subagents.spawn("do it", label="it", session_key="tui:a") + return "done" + + await _run(loop, "tui:a", _body) + loop.set_session_binding("tui:a", ModelBinding(_Recording("switched-to"), "switched/model")) + loop.subagents._gate.release() + for _ in range(50): + await asyncio.sleep(0) + if served: + break + finally: + manager_mod.build_executor = original_build + + assert served == ["started-with"], "the spawn ran on the model its conversation had when it asked" diff --git a/tests/test_provider_pool.py b/tests/test_provider_pool.py new file mode 100644 index 00000000..4566f9ab --- /dev/null +++ b/tests/test_provider_pool.py @@ -0,0 +1,406 @@ +"""Where a model id becomes a model id plus the credential that serves it. + +A pin like ``context.curator_model`` is a model id and nothing else, so +resolving it has to answer a second question: which configured provider +serves that vendor. Answering it in one place is what stops a subsystem +from sending one vendor's key to another's endpoint -- either the pool +returns a complete pair, or it returns nothing and the caller follows the +conversation instead. +""" + +from __future__ import annotations + +import os + +import pytest + +from raven.config.schema import Config +from raven.providers.binding import ModelBinding, active_binding, resolve, use_binding + + +@pytest.fixture(autouse=True) +def _restore_env(): + """Building a keyed provider writes the vendor's env var; keep that here.""" + before = dict(os.environ) + yield + os.environ.clear() + os.environ.update(before) + + +def _config(model: str = "claude-opus-4-5", **keys: str) -> Config: + cfg = Config() + cfg.agents.defaults.model = model + cfg.agents.defaults.provider = "auto" + for name, key in keys.items(): + section = cfg.providers.get(name) + assert section is not None, name + section.api_key = key + return cfg + + +def _pool(model: str = "claude-opus-4-5", **kwargs): + from raven.providers.pool import ProviderPool + + return ProviderPool(_config(model, **kwargs)) + + +# --------------------------------------------------------------------------- +# Binding a model +# --------------------------------------------------------------------------- + + +def test_a_model_gets_its_own_vendors_key() -> None: + """The point of the pool: the credential follows the model id, not whatever + provider the caller happened to be holding. + """ + pool = _pool(anthropic="sk-ant", gemini="AIza") + + assert pool.bind("gemini-2.5-flash").provider.api_key == "AIza" + assert pool.bind("claude-sonnet-4-5").provider.api_key == "sk-ant" + + +def test_the_same_pair_is_built_once() -> None: + """Building a provider imports LiteLLM and writes vendor env vars, so a + session flipping between two models must not pay for it per turn. + """ + pool = _pool(anthropic="sk-ant") + first = pool.bind("claude-opus-4-5") + + assert pool.bind("claude-opus-4-5") is first + + +def test_two_models_of_one_vendor_are_two_bindings() -> None: + pool = _pool(anthropic="sk-ant") + + assert pool.bind("claude-opus-4-5") is not pool.bind("claude-sonnet-4-5") + + +def test_an_explicit_provider_name_wins_over_derivation() -> None: + """The picker sends the provider alongside the model; a gateway serving + another vendor's id would be mis-derived without it. + """ + pool = _pool("anthropic/claude-opus-4-5", openrouter="sk-or") + binding = pool.bind("anthropic/claude-opus-4-5", "openrouter") + + assert binding.provider.api_key == "sk-or" + + +# --------------------------------------------------------------------------- +# Pins +# --------------------------------------------------------------------------- + + +def test_a_pin_with_credentials_becomes_a_pair() -> None: + pool = _pool(anthropic="sk-ant", gemini="AIza") + pin = pool.bind_pin("gemini-2.5-flash") + + assert pin is not None + assert pin.model == "gemini-2.5-flash" + assert pin.provider.api_key == "AIza" + + +def test_a_pin_without_credentials_is_not_a_pair() -> None: + """The case that used to 401 every call: a model id whose vendor has no key. + None tells the caller to follow the conversation instead of sending this id + on the conversation's credential. + """ + pool = _pool(anthropic="sk-ant") + + assert pool.bind_pin("gemini-2.5-flash") is None + assert pool.bind_pin("openai/gpt-5-mini") is None + + +def test_an_unset_pin_is_not_a_pair() -> None: + pool = _pool(anthropic="sk-ant") + + assert pool.bind_pin(None) is None + assert pool.bind_pin("") is None + + +def test_a_pin_whose_vendor_cannot_be_derived_is_not_a_pair() -> None: + """With no vendor there is no section to check, so there is no way to tell a + working pin from a mis-paired one. Following the conversation is at least a + pair. + """ + pool = _pool(anthropic="sk-ant") + + assert pool.bind_pin("some-unknown-local-model") is None + + +def test_a_declared_but_empty_section_is_not_credentials() -> None: + """Every vendor exists as an empty section whether or not it was configured, + so presence proves nothing. + """ + pool = _pool(anthropic="sk-ant") + assert pool.config.providers.get("gemini") is not None + + assert pool.bind_pin("gemini-2.5-flash") is None + + +# --------------------------------------------------------------------------- +# The binding context +# --------------------------------------------------------------------------- + + +class _Stub: + def get_default_model(self) -> str: + return "stub/default" + + +def test_a_binding_needs_a_model() -> None: + with pytest.raises(ValueError): + ModelBinding(_Stub(), "") + + +def test_nothing_is_bound_outside_a_turn() -> None: + assert active_binding() is None + + +def test_a_binding_is_visible_for_its_block_and_no_longer() -> None: + binding = ModelBinding(_Stub(), "vendor/model") + with use_binding(binding): + assert active_binding() is binding + assert active_binding() is None + + +def test_a_binding_is_restored_when_the_block_raises() -> None: + with pytest.raises(RuntimeError): + with use_binding(ModelBinding(_Stub(), "vendor/model")): + raise RuntimeError("boom") + assert active_binding() is None + + +def test_bindings_nest() -> None: + outer = ModelBinding(_Stub(), "outer/model") + inner = ModelBinding(_Stub(), "inner/model") + with use_binding(outer): + with use_binding(inner): + assert active_binding() is inner + assert active_binding() is outer + + +def test_resolve_prefers_a_pin_then_the_turn_then_the_fallback() -> None: + pin = ModelBinding(_Stub(), "pin/model") + turn = ModelBinding(_Stub(), "turn/model") + fallback = ModelBinding(_Stub(), "fallback/model") + + assert resolve(None, fallback) is fallback + with use_binding(turn): + assert resolve(None, fallback) is turn + assert resolve(pin, fallback) is pin, "a configured subsystem does not follow the turn" + + +@pytest.mark.asyncio +async def test_a_detached_task_keeps_the_binding_it_was_created_under() -> None: + """This is what makes a spawned subagent finish on the model it started on: + ``create_task`` copies the context, so a later switch cannot reach it. + """ + import asyncio + + started = ModelBinding(_Stub(), "started/model") + seen: list[str | None] = [] + release = asyncio.Event() + + async def _detached() -> None: + await release.wait() + binding = active_binding() + seen.append(binding.model if binding else None) + + with use_binding(started): + task = asyncio.create_task(_detached()) + + # Outside the block now, and a different binding is current. + with use_binding(ModelBinding(_Stub(), "switched/model")): + release.set() + await task + + assert seen == ["started/model"] + + +def test_a_config_supplier_is_re_read_and_drops_stale_bindings() -> None: + """A credential fixed after start (an OAuth re-login, an edited file) has + to be visible without a restart, which a snapshot would prevent. + """ + from raven.providers.pool import ProviderPool + + broken = _config(anthropic="") + fixed = _config(anthropic="sk-ant") + current = [broken] + pool = ProviderPool(lambda: current[0]) + + assert pool.bind_pin("claude-opus-4-5") is None, "no key yet" + + current[0] = fixed + pin = pool.bind_pin("claude-opus-4-5") + assert pin is not None + assert pin.provider.api_key == "sk-ant" + + +def test_a_gateway_serves_a_pin_its_upstream_vendor_has_no_key_for() -> None: + """A gateway bills its own key for whatever id it is handed, so asking + whether the upstream vendor has a key of its own would drop a pin that + works -- which is how a working setup got broken once already. + """ + cfg = _config("anthropic/claude-opus-4-5", openrouter="sk-or") + cfg.agents.defaults.provider = "openrouter" + + from raven.providers.pool import ProviderPool + + pin = ProviderPool(cfg).bind_pin("gemini-2.5-flash") + + assert pin is not None + assert pin.model == "gemini-2.5-flash" + assert pin.provider.api_key == "sk-or" + + +def test_an_unbuildable_pin_is_reported_not_raised(monkeypatch) -> None: + """``bind_pin`` is called from the factory at construction; a vendor whose + provider cannot be built must leave the subsystem following the + conversation, not stop the agent from starting. + """ + import raven.cli._helpers as helpers + + pool = _pool(anthropic="sk-ant", gemini="AIza") + + def _boom(_cfg): + raise RuntimeError("cannot build") + + monkeypatch.setattr(helpers, "make_provider", _boom) + assert pool.bind_pin("gemini-2.5-flash") is None + + +def test_a_credential_added_after_boot_is_visible_without_a_restart() -> None: + """The production wiring hands the pool a loader, not the boot config: a + user who connects a provider in the picker and then picks its model must + not be told the model is unavailable. + """ + from raven.providers.pool import ProviderPool + + cfg = _config() + pool = ProviderPool(lambda: cfg) + + assert pool.bind_pin("gemini-2.5-flash") is None, "no key yet" + + cfg.providers.gemini.api_key = "AIza-added-after-boot" + pin = pool.bind_pin("gemini-2.5-flash") + + assert pin is not None + assert pin.provider.api_key == "AIza-added-after-boot" + + +def test_a_re_reading_supplier_still_reuses_bindings() -> None: + """Invalidating on config identity would clear the cache on every call for + a supplier that re-reads, which defeats the pool. Only a credential change + may drop it. + """ + from raven.providers.pool import ProviderPool + + def _fresh(): + # A new object every call, same contents -- what a file loader does. + return _config(anthropic="sk-ant") + + pool = ProviderPool(_fresh) + first = pool.bind("claude-opus-4-5") + + assert pool.bind("claude-opus-4-5") is first + + +def test_a_gateway_pin_that_cannot_be_built_is_reported_not_raised(monkeypatch) -> None: + """The gateway branch is reached from the factory at construction too, so + it needs the same guard as the direct-vendor branch -- otherwise a missing + gateway key stops the agent from starting. + """ + import raven.cli._helpers as helpers + + cfg = _config("anthropic/claude-opus-4-5", openrouter="sk-or") + cfg.agents.defaults.provider = "openrouter" + + from raven.providers.pool import ProviderPool + + pool = ProviderPool(cfg) + + def _boom(_cfg): + raise RuntimeError("no gateway key") + + monkeypatch.setattr(helpers, "make_provider", _boom) + assert pool.bind_pin("gemini-2.5-flash") is None + + +# --------------------------------------------------------------------------- +# A pin configured as a pair +# --------------------------------------------------------------------------- + + +def test_a_configured_provider_beats_the_gateway_guess() -> None: + """The id alone cannot say which credential was meant. + + On a gateway, ``anthropic/claude-...`` served by the gateway and + ``claude-...`` served by Anthropic direct are both valid, name different + credentials and different bills. Guessing picks one; the configured pair + says which. + """ + cfg = _config("anthropic/claude-opus-4-5", openrouter="sk-or", anthropic="sk-ant") + cfg.agents.defaults.provider = "openrouter" + + from raven.providers.pool import ProviderPool + + pin = ProviderPool(cfg).bind_pin("claude-haiku-4-5", "anthropic") + + assert pin is not None + assert pin.model == "claude-haiku-4-5" + assert pin.provider.api_key == "sk-ant", "the configured provider serves it, not the gateway" + + +def test_a_configured_provider_beats_the_vendor_the_id_names() -> None: + """The other direction: a gateway reselling a vendor's model. Deriving from + the id would send it to Anthropic direct on a key the user may not hold. + """ + cfg = _config("claude-opus-4-5", openrouter="sk-or", anthropic="sk-ant") + + from raven.providers.pool import ProviderPool + + pin = ProviderPool(cfg).bind_pin("anthropic/claude-haiku-4-5", "openrouter") + + assert pin is not None + assert pin.provider.api_key == "sk-or", "the gateway serves it, not the vendor in the id" + + +def test_a_configured_provider_without_credentials_is_not_a_pair() -> None: + """Explicitly configured and still unusable is a config error, and the + subsystem follows the conversation rather than borrowing another key. + """ + cfg = _config("claude-opus-4-5", anthropic="sk-ant") + + from raven.providers.pool import ProviderPool + + assert ProviderPool(cfg).bind_pin("gemini-2.5-flash", "gemini") is None + + +def test_an_unpaired_pin_still_derives_its_vendor() -> None: + """Configs written before the provider field existed keep working.""" + cfg = _config("claude-opus-4-5", anthropic="sk-ant", gemini="AIza") + + from raven.providers.pool import ProviderPool + + pin = ProviderPool(cfg).bind_pin("gemini-2.5-flash") + + assert pin is not None + assert pin.provider.api_key == "AIza" + + +def test_a_pin_that_cannot_be_built_at_all_does_not_stop_the_agent(monkeypatch) -> None: + """``bind_pin`` runs in the context-engine factory at construction, and + building a provider imports a vendor module -- so the failures are not only + the credential ones the narrower guard covered. + """ + import raven.cli._helpers as helpers + + cfg = _config("claude-opus-4-5", anthropic="sk-ant") + + from raven.providers.pool import ProviderPool + + def _boom(_cfg): + raise ModuleNotFoundError("no module named 'litellm'") + + monkeypatch.setattr(helpers, "make_provider", _boom) + assert ProviderPool(cfg).bind_pin("claude-haiku-4-5", "anthropic") is None diff --git a/tests/test_read_file_image.py b/tests/test_read_file_image.py index d282211c..44b1550d 100644 --- a/tests/test_read_file_image.py +++ b/tests/test_read_file_image.py @@ -721,11 +721,14 @@ def test_capability_cache_is_keyed_by_model() -> None: """The loop is a long-lived singleton taking a per-call model, so a verdict learned for one model must not answer for another.""" from raven.agent.loop.main import AgentLoop + from raven.providers.binding import ModelBinding from raven.providers.litellm_provider import LiteLLMProvider loop = object.__new__(AgentLoop) - loop.provider = object.__new__(LiteLLMProvider) - loop.model = "claude-opus-4-5" + # The pair, not one half of it: ``provider``/``model`` are read-only views + # onto whichever binding is current. + loop._default_binding = ModelBinding(object.__new__(LiteLLMProvider), "claude-opus-4-5") + loop._session_bindings = {} loop._image_tool_result_ok = {} assert loop._supports_image_tool_result("claude-opus-4-5") is True diff --git a/tests/test_rpc_schema_match.py b/tests/test_rpc_schema_match.py index 2c205dc4..b9930491 100644 --- a/tests/test_rpc_schema_match.py +++ b/tests/test_rpc_schema_match.py @@ -420,7 +420,6 @@ def test_method_result_matches_schema( -32006: "skill_not_found", -32007: "skill_pin_conflict", -32008: "model_not_available", - -32009: "model_switch_in_turn", -32010: "config_field_readonly", -32011: "config_validation_error", -32012: "not_supported_in_v01", diff --git a/tests/test_sandbox_unit.py b/tests/test_sandbox_unit.py index a66561b0..93945e63 100644 --- a/tests/test_sandbox_unit.py +++ b/tests/test_sandbox_unit.py @@ -1168,12 +1168,17 @@ def _patched_build(cfg, workspace, owned_ids=None): subagent_mod.build_executor = _patched_build try: # Patch the inner method so the agent loop completes quickly - async def _fast_inner(task_id, task, label, origin, executor): + async def _fast_inner(task_id, task, label, origin, executor, provider, model): await manager._announce_result(task_id, label, task, "done", origin, "ok") manager._run_subagent_inner = _fast_inner await manager._run_subagent( - "t1", "test task", "test", {"channel": "cli", "chat_id": "direct", "session_key": "cli:direct"} + "t1", + "test task", + "test", + {"channel": "cli", "chat_id": "direct", "session_key": "cli:direct"}, + manager.provider, + manager.model, ) finally: subagent_mod.build_executor = original diff --git a/tests/test_session_manager.py b/tests/test_session_manager.py index 6db033e8..0baf26d5 100644 --- a/tests/test_session_manager.py +++ b/tests/test_session_manager.py @@ -715,6 +715,39 @@ def test_fork_inherits_last_consolidated(tmp_path: Path): assert child.last_consolidated == 1 +def test_fork_inherits_the_model_the_parent_chose(tmp_path: Path): + """A fork continues its parent's conversation, so it continues on its model. + + The branch handler re-points the live binding, but that is in memory only. + Without the record the fork reads the default the first time it is resumed + in a new process, which is a silent downgrade rather than a visible one. + """ + mgr = SessionManager(tmp_path) + src = _seed(mgr, "cli:src08", ("user", "a")) + src.metadata["model"] = "anthropic/claude-opus-4-8" + src.metadata["provider"] = "anthropic" + mgr.save(src) + + child = mgr.fork("cli:src08") + + reloaded = SessionManager(tmp_path).get_or_create(child.key) + assert reloaded.metadata["model"] == "anthropic/claude-opus-4-8" + assert reloaded.metadata["provider"] == "anthropic" + + +def test_fork_of_an_unswitched_parent_carries_no_model(tmp_path: Path): + """The parent never chose one, so the fork must start on the default too -- + not on a model copied out of nowhere. + """ + mgr = SessionManager(tmp_path) + _seed(mgr, "cli:src09", ("user", "a")) + + child = mgr.fork("cli:src09") + + assert "model" not in child.metadata + assert "provider" not in child.metadata + + def test_fork_resets_pending_clarification(tmp_path: Path): """The child does not carry the source's clarification wait-state.""" mgr = SessionManager(tmp_path) diff --git a/tests/test_subagent_manager.py b/tests/test_subagent_manager.py index 1f65a772..15298c3f 100644 --- a/tests/test_subagent_manager.py +++ b/tests/test_subagent_manager.py @@ -94,7 +94,7 @@ async def _drive(monkeypatch, *, max_concurrent: int, spawn_n: int) -> int: state = {"current": 0, "peak": 0} release = asyncio.Event() - async def _stub_inner(task_id, task, label, origin, executor) -> None: + async def _stub_inner(task_id, task, label, origin, executor, provider, model) -> None: state["current"] += 1 state["peak"] = max(state["peak"], state["current"]) await release.wait() @@ -146,6 +146,8 @@ async def _capture_announcement(task_id, label, task, result, origin, status) -> "delete", {"channel": "tui", "chat_id": "default", "session_key": "tui:session-a"}, executor, + manager.provider, + manager.model, ) assert executor.commands == [] @@ -247,7 +249,7 @@ async def test_cancel_by_session_cancels_live_task(monkeypatch): entered = asyncio.Event() release = asyncio.Event() - async def _blocking_inner(task_id, task, label, origin, executor) -> None: + async def _blocking_inner(task_id, task, label, origin, executor, provider, model) -> None: entered.set() await release.wait() # never set — keeps the task live until cancelled diff --git a/tests/test_tui_rpc_config.py b/tests/test_tui_rpc_config.py index caa18cd3..018d8b38 100644 --- a/tests/test_tui_rpc_config.py +++ b/tests/test_tui_rpc_config.py @@ -22,7 +22,6 @@ ConfigFieldReadonlyError, ConfigValidationError, ModelNotAvailableError, - ModelSwitchInTurnError, ) from raven.tui_rpc.methods.config import ( CONFIG_WRITABLE_KEYS, @@ -137,13 +136,49 @@ async def test_config_set_missing_key_param_raises_validation(fake_home: Path) - # ---------------------------------------------------------------------------- +class _FakeLoop: + """Stand-in for AgentLoop's half of the switch contract. + + Records the ``set_provider`` calls: assigning ``provider``/``model`` + directly would leave the subagent manager, the context engine and the + consolidator on the old provider, so the handler must go through the + method, not the attributes. + """ + + def __init__(self, provider: object, model: str) -> None: + self.provider = provider + self.model = model + self.switches: list[tuple[object, str]] = [] + self.session_bindings: dict[str, object] = {} + self.provider_pool = None + + def session_model(self, session_key: str) -> str: + binding = self.session_bindings.get(session_key) + return binding.model if binding is not None else self.model + + def has_session_binding(self, session_key: str) -> bool: + return session_key in self.session_bindings + + def set_session_binding(self, session_key: str, binding: object) -> None: + self.session_bindings[session_key] = binding + + def set_default_binding(self, binding: object) -> None: + self.provider = binding.provider + self.model = binding.model + self.switches.append((binding.provider, binding.model)) + + def set_provider(self, provider: object, model: str) -> None: + self.provider = provider + self.model = model + self.switches.append((provider, model)) + + async def test_config_set_model_reassigns_loop_and_persists(fake_home: Path, monkeypatch) -> None: import raven.tui_rpc.methods.config as config_mod - loop = SimpleNamespace(provider="old-prov", model="old-model") + loop = _FakeLoop("old-prov", "old-model") new_provider = SimpleNamespace(name="new-prov") - monkeypatch.setattr(config_mod, "is_turn_active", lambda _key: False) monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: new_provider) monkeypatch.setattr( config_mod, @@ -156,15 +191,19 @@ async def test_config_set_model_reassigns_loop_and_persists(fake_home: Path, mon "key": "model", "value": "anthropic/claude-opus-4-8", "provider": "anthropic", - "session_id": "tui:default", + "scope": "default", }, agent_loop_factory=lambda: loop, ) assert result["applied"] is True assert result["value"] == "anthropic/claude-opus-4-8" + assert result["scope"] == "default" assert loop.model == "anthropic/claude-opus-4-8" assert loop.provider is new_provider + # Routed through set_provider, so everything holding the old provider + # (subagents, context-engine segments, consolidator) gets told too. + assert loop.switches == [(new_provider, "anthropic/claude-opus-4-8")] cfg = json.loads((fake_home / ".raven" / "config.json").read_text()) assert cfg["agents"]["defaults"]["model"] == "anthropic/claude-opus-4-8" @@ -184,20 +223,69 @@ async def test_config_set_model_bare_derives_provider(fake_home: Path) -> None: assert cfg["agents"]["defaults"]["provider"] == "anthropic" -async def test_config_set_model_rejected_during_active_turn(fake_home: Path, monkeypatch) -> None: +async def test_config_set_model_is_scoped_to_the_session_that_asked(fake_home: Path, monkeypatch) -> None: + """A session switching its own model must not move anyone else's, and must + not rewrite the default a new session starts on. + """ import raven.tui_rpc.methods.config as config_mod - monkeypatch.setattr(config_mod, "is_turn_active", lambda _key: True) + (fake_home / ".raven").mkdir() + (fake_home / ".raven" / "config.json").write_text( + json.dumps({"agents": {"defaults": {"model": "anthropic/claude-sonnet-4-5"}}}) + ) + + new_provider = SimpleNamespace(name="new-prov") + loop = _FakeLoop("old-prov", "anthropic/claude-sonnet-4-5") - with pytest.raises(ModelSwitchInTurnError): - await config_set( - { - "key": "model", - "value": "anthropic/claude-opus-4-8", - "session_id": "tui:default", - }, - agent_loop_factory=lambda: SimpleNamespace(provider=None, model="x"), - ) + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: new_provider) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + result = await config_set( + { + "key": "model", + "value": "anthropic/claude-opus-4-8", + "provider": "anthropic", + "session_id": "tui:a", + }, + agent_loop_factory=lambda: loop, + ) + + assert result["scope"] == "session" + assert result["session_id"] == "tui:a" + assert loop.session_bindings["tui:a"].model == "anthropic/claude-opus-4-8" + assert "tui:b" not in loop.session_bindings, "another session must not move" + assert loop.switches == [], "a session switch is not a default change" + + on_disk = json.loads((fake_home / ".raven" / "config.json").read_text()) + assert on_disk["agents"]["defaults"]["model"] == "anthropic/claude-sonnet-4-5", ( + "a new session must still start on the configured default" + ) + + +async def test_config_set_model_is_not_refused_mid_turn(fake_home: Path, monkeypatch) -> None: + """The running turn holds the binding it started on, so the switch lands on + the session's next turn rather than being rejected. + """ + import raven.tui_rpc.methods.config as config_mod + + new_provider = SimpleNamespace(name="new-prov") + loop = _FakeLoop("old-prov", "old-model") + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: new_provider) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + result = await config_set( + {"key": "model", "value": "anthropic/claude-opus-4-8", "provider": "anthropic", "session_id": "tui:busy"}, + agent_loop_factory=lambda: loop, + ) + assert result["applied"] is True async def test_config_set_model_unconstructable_preserves_previous(fake_home: Path, monkeypatch) -> None: @@ -211,7 +299,6 @@ async def test_config_set_model_unconstructable_preserves_previous(fake_home: Pa def _boom(_cfg): raise RuntimeError("no api key") - monkeypatch.setattr(config_mod, "is_turn_active", lambda _key: False) monkeypatch.setattr(config_mod, "make_provider", _boom) monkeypatch.setattr( config_mod, @@ -219,7 +306,7 @@ def _boom(_cfg): lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), ) - loop = SimpleNamespace(provider="keep-prov", model="anthropic/claude-sonnet-4-5") + loop = _FakeLoop("keep-prov", "anthropic/claude-sonnet-4-5") with pytest.raises(ModelNotAvailableError): await config_set( { @@ -233,6 +320,7 @@ def _boom(_cfg): # Loop untouched and on-disk model preserved. assert loop.model == "anthropic/claude-sonnet-4-5" assert loop.provider == "keep-prov" + assert loop.switches == [] cfg = json.loads((fake_home / ".raven" / "config.json").read_text()) assert cfg["agents"]["defaults"]["model"] == "anthropic/claude-sonnet-4-5" @@ -415,3 +503,239 @@ async def test_a_bare_id_the_pinned_provider_lists_itself_keeps_the_pin(fake_hom assert result["applied"] is True cfg = json.loads((fake_home / ".raven" / "config.json").read_text()) assert cfg["agents"]["defaults"]["provider"] == "mistral" + + +async def test_a_session_switch_is_written_to_the_session_record(fake_home: Path, monkeypatch, tmp_path) -> None: + """The in-memory override dies with the process, so the record is the only + place the choice survives -- and a write nobody reads is worse than none. + """ + import raven.tui_rpc.methods.config as config_mod + from raven.session.manager import SessionManager + + new_provider = SimpleNamespace(name="new-prov") + loop = _FakeLoop("old-prov", "old-model") + loop.sessions = SessionManager(tmp_path) + + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: new_provider) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + await config_set( + { + "key": "model", + "value": "anthropic/claude-opus-4-8", + "provider": "anthropic", + "session_id": "tui:a", + }, + agent_loop_factory=lambda: loop, + ) + + stored = loop.sessions.peek("tui:a") + assert stored is not None + assert stored.metadata["model"] == "anthropic/claude-opus-4-8" + assert stored.metadata["provider"] == "anthropic" + + +async def test_a_session_switch_reports_the_model_it_replaced(fake_home: Path, monkeypatch) -> None: + """``previous`` is the session's own model, not the global default.""" + import raven.tui_rpc.methods.config as config_mod + + loop = _FakeLoop("old-prov", "boot-model") + loop.session_bindings["tui:a"] = SimpleNamespace(provider=object(), model="was-on-this") + + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: SimpleNamespace(name="new-prov")) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + result = await config_set( + {"key": "model", "value": "anthropic/claude-opus-4-8", "provider": "anthropic", "session_id": "tui:a"}, + agent_loop_factory=lambda: loop, + ) + + assert result["previous"] == "was-on-this" + + +async def test_an_unknown_scope_is_rejected(fake_home: Path) -> None: + """A client typo must not silently degrade to a session switch.""" + with pytest.raises(ConfigValidationError): + await config_set( + {"key": "model", "value": "anthropic/claude-opus-4-8", "session_id": "tui:a", "scope": "globl"}, + agent_loop_factory=None, + ) + + +async def test_a_switch_goes_through_the_pool_when_the_loop_has_one(fake_home: Path, monkeypatch) -> None: + """The pool is what makes a switch reuse a provider instead of rebuilding + one per switch; without this the production path is never exercised. + """ + import raven.tui_rpc.methods.config as config_mod + + asked: list[tuple[str, str | None]] = [] + pooled = SimpleNamespace(provider=SimpleNamespace(name="pooled"), model="anthropic/claude-opus-4-8") + + class _Pool: + def bind(self, model: str, provider_name: str | None = None): + asked.append((model, provider_name)) + return pooled + + loop = _FakeLoop("old-prov", "old-model") + loop.provider_pool = _Pool() + + def _must_not_build(_cfg): + raise AssertionError("a loop with a pool must not build its own provider") + + monkeypatch.setattr(config_mod, "make_provider", _must_not_build) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + await config_set( + {"key": "model", "value": "anthropic/claude-opus-4-8", "provider": "anthropic", "session_id": "tui:a"}, + agent_loop_factory=lambda: loop, + ) + + assert asked == [("anthropic/claude-opus-4-8", "anthropic")] + assert loop.session_bindings["tui:a"] is pooled + + +# ---------------------------------------------------------------------------- +# Scope is never widened +# ---------------------------------------------------------------------------- + + +async def test_a_session_scope_without_a_session_id_is_refused_not_widened(fake_home: Path, monkeypatch) -> None: + """An explicit ``scope="session"`` with no session must not fall through to + the default branch. + + The TUI sends ``session_id: ctx.sid``, which is null until the first + ``session.create`` resolves and after a failed one. Widening the scope + there rewrites ``agents.defaults.model`` on disk and moves every session + that never chose its own model -- from a request that asked for the + opposite. + """ + import raven.tui_rpc.methods.config as config_mod + + (fake_home / ".raven").mkdir() + (fake_home / ".raven" / "config.json").write_text( + json.dumps({"agents": {"defaults": {"model": "anthropic/claude-sonnet-4-5"}}}) + ) + + loop = _FakeLoop("old-prov", "anthropic/claude-sonnet-4-5") + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: SimpleNamespace(name="new-prov")) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + for absent in (None, ""): + with pytest.raises(ConfigValidationError): + await config_set( + { + "key": "model", + "value": "anthropic/claude-opus-4-8", + "scope": "session", + "session_id": absent, + }, + agent_loop_factory=lambda: loop, + ) + + assert loop.switches == [], "a refused switch must not move the default binding" + assert loop.session_bindings == {} + on_disk = json.loads((fake_home / ".raven" / "config.json").read_text()) + assert on_disk["agents"]["defaults"]["model"] == "anthropic/claude-sonnet-4-5" + + +async def test_a_default_scope_with_a_session_id_still_writes_the_default(fake_home: Path, monkeypatch) -> None: + """``/model X --default`` sends both ``scope="default"`` and the caller's + session id, and the scope has to win. + + Without the scope conjunct the session id alone decides, and the switch + silently becomes an override on the asking session -- the file is never + written, so nothing a new session starts on ever changes. + """ + import raven.tui_rpc.methods.config as config_mod + + (fake_home / ".raven").mkdir() + (fake_home / ".raven" / "config.json").write_text( + json.dumps({"agents": {"defaults": {"model": "anthropic/claude-sonnet-4-5"}}}) + ) + + new_provider = SimpleNamespace(name="new-prov") + loop = _FakeLoop("old-prov", "anthropic/claude-sonnet-4-5") + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: new_provider) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + result = await config_set( + { + "key": "model", + "value": "anthropic/claude-opus-4-8", + "provider": "anthropic", + "scope": "default", + "session_id": "tui:a", + }, + agent_loop_factory=lambda: loop, + ) + + assert result["scope"] == "default" + assert loop.switches == [(new_provider, "anthropic/claude-opus-4-8")] + assert "tui:a" not in loop.session_bindings, "a default switch is not a session override" + on_disk = json.loads((fake_home / ".raven" / "config.json").read_text()) + assert on_disk["agents"]["defaults"]["model"] == "anthropic/claude-opus-4-8" + + +async def test_a_default_switch_reports_whether_it_moved_the_asking_session(fake_home: Path, monkeypatch) -> None: + """The scope alone cannot tell a client whether to repaint the status bar. + + A session that never chose a model reads the default, so a default-scoped + switch moves it; a session with its own binding stays where it is. The + client cannot see the difference, so the server answers it. + """ + import raven.tui_rpc.methods.config as config_mod + + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: SimpleNamespace(name="new-prov")) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + loop = _FakeLoop("old-prov", "old-model") + loop.session_bindings["tui:chose"] = SimpleNamespace(provider="own-prov", model="own/model") + + params = {"key": "model", "value": "anthropic/claude-opus-4-8", "provider": "anthropic", "scope": "default"} + + followed = await config_set({**params, "session_id": "tui:followed"}, agent_loop_factory=lambda: loop) + assert followed["applies_to_session"] is True + + chose = await config_set({**params, "session_id": "tui:chose"}, agent_loop_factory=lambda: loop) + assert chose["applies_to_session"] is False + + +async def test_a_session_switch_always_applies_to_its_own_session(fake_home: Path, monkeypatch) -> None: + import raven.tui_rpc.methods.config as config_mod + + monkeypatch.setattr(config_mod, "make_provider", lambda _cfg: SimpleNamespace(name="new-prov")) + monkeypatch.setattr( + config_mod, + "load_runtime_config", + lambda *a, **k: SimpleNamespace(agents=SimpleNamespace(defaults=SimpleNamespace(model="", provider="auto"))), + ) + + result = await config_set( + {"key": "model", "value": "anthropic/claude-opus-4-8", "session_id": "tui:a"}, + agent_loop_factory=lambda: _FakeLoop("old-prov", "old-model"), + ) + assert result["applies_to_session"] is True diff --git a/tests/test_tui_rpc_model.py b/tests/test_tui_rpc_model.py index e3f5c825..b0e64dd4 100644 --- a/tests/test_tui_rpc_model.py +++ b/tests/test_tui_rpc_model.py @@ -622,3 +622,89 @@ def test_every_provider_stores_a_model_id_that_finds_it_again(spec) -> None: resolved = find_by_model(stored) assert resolved is not None and resolved.name == spec.name, f"{stored} resolves to {resolved and resolved.name}" + + +async def test_options_reports_the_session_model_when_that_session_switched() -> None: + """The picker sits under the status bar; reading the global default here is + how they end up showing two models for one conversation. + """ + from types import SimpleNamespace + + from raven.tui_rpc.methods.model import model_options + + loop = SimpleNamespace( + has_session_binding=lambda key: key == "tui:a", + session_model=lambda key: "anthropic/claude-opus-4-8", + ) + + result = await model_options({"session_id": "tui:a"}, agent_loop_factory=lambda: loop) + + assert result["model"] == "anthropic/claude-opus-4-8" + assert result["provider"] == "anthropic" + + +async def test_options_leaves_an_unswitched_session_on_the_configured_answer() -> None: + """``session_model`` falls back to the default, so asking it alone would + override a forced ``agents.defaults.provider`` for every session. + """ + from types import SimpleNamespace + + from raven.tui_rpc.methods.model import _current_selection, model_options + + configured_model, configured_provider = _current_selection() + loop = SimpleNamespace( + has_session_binding=lambda key: False, + session_model=lambda key: "anthropic/claude-opus-4-8", + ) + + result = await model_options({"session_id": "tui:b"}, agent_loop_factory=lambda: loop) + + assert result["model"] == configured_model + assert result["provider"] == (configured_provider or "") + + +async def test_the_production_registration_makes_model_options_session_aware(fake_home: Path) -> None: + """The picker asks ``model.options`` which model to show as current, and the + answer is per session. + + Registered through the umbrella the production path uses, not by calling + the handler with a factory by hand: without the factory threaded here the + picker reports the configured default to every session, and calling + ``model_options`` directly would never notice. + """ + from raven.tui_rpc.dispatcher import Dispatcher + from raven.tui_rpc.methods import register_aligned_methods_except_system + + _write_config(fake_home, {"agents": {"defaults": {"model": "anthropic/claude-sonnet-4-5"}}}) + + class _Loop: + def has_session_binding(self, session_id: str) -> bool: + return session_id == "tui:switched" + + def session_model(self, session_id: str) -> str: + return "anthropic/claude-opus-4-8" + + d = Dispatcher() + register_aligned_methods_except_system(d, agent_loop_factory=lambda: _Loop()) + + switched = await d.dispatch( + { + "jsonrpc": "2.0", + "id": 1, + "method": "model.options", + "params": {"session_id": "tui:switched"}, + } + ) + assert switched["result"]["model"] == "anthropic/claude-opus-4-8" + + untouched = await d.dispatch( + { + "jsonrpc": "2.0", + "id": 2, + "method": "model.options", + "params": {"session_id": "tui:never-switched"}, + } + ) + assert untouched["result"]["model"] == "anthropic/claude-sonnet-4-5", ( + "a session that never switched still reports the configured default" + ) diff --git a/tests/test_tui_rpc_session.py b/tests/test_tui_rpc_session.py index 53949e0e..946da721 100644 --- a/tests/test_tui_rpc_session.py +++ b/tests/test_tui_rpc_session.py @@ -17,6 +17,7 @@ import json import re from pathlib import Path +from types import SimpleNamespace import pytest @@ -1138,3 +1139,155 @@ async def test_session_export_is_read_only_during_active_turn(tmp_path: Path, mo result = await session_export({"session_id": session_key}) assert result["exported"] is True + + +async def test_session_info_reports_this_sessions_model_not_the_default(monkeypatch) -> None: + """The picker and the status bar sit next to each other; reporting the + global default here would show two models for one conversation. + """ + from unittest.mock import MagicMock + + import raven.tui_rpc.methods.session as session_mod + + # MagicMock so the unrelated skills/tools enumeration in the bundle works; + # only ``session_model`` is under test. + loop = MagicMock() + loop.session_model = lambda key: "vendor-a/model" if key == "tui:a" else "boot/model" + info = session_mod._default_session_info(loop, session_mod.load_config(), "tui:a") + + assert info["model"] == "vendor-a/model" + assert info["model_id"] == "vendor-a/model" + + +async def test_session_info_without_a_session_reports_the_default() -> None: + """A session being created has no model of its own yet; the default is the + right answer, because that is what it will start on. + """ + from unittest.mock import MagicMock + + import raven.tui_rpc.methods.session as session_mod + + config = session_mod.load_config() + loop = MagicMock() + loop.session_model = lambda key: "vendor-a/model" + info = session_mod._default_session_info(loop, config, None) + + assert info["model"] == config.agents.defaults.model + + +async def test_session_resume_puts_the_session_back_on_its_stored_model(tmp_path) -> None: + """The handler, not the helper: a resume that stops passing the session key + would leave every restored session on the default with the suite green. + """ + from unittest.mock import MagicMock + + from raven.session.manager import SessionManager + from raven.tui_rpc.methods.session import session_resume + + sessions = SessionManager(tmp_path) + record = sessions.get_or_create("tui:a") + record.metadata["model"] = "vendor-a/model" + record.metadata["provider"] = "anthropic" + sessions.save(record) + + restored: list[tuple[str, str, str | None]] = [] + loop = MagicMock() + loop.sessions = sessions + loop.restore_session_model = lambda key, model, provider=None: restored.append((key, model, provider)) + loop.session_model = lambda key: "vendor-a/model" + + await session_resume({"session_id": "tui:a"}, agent_loop_factory=lambda: loop) + + assert restored == [("tui:a", "vendor-a/model", "anthropic")] + + +async def test_session_resume_without_a_stored_model_restores_nothing(tmp_path) -> None: + from unittest.mock import MagicMock + + from raven.session.manager import SessionManager + from raven.tui_rpc.methods.session import session_resume + + sessions = SessionManager(tmp_path) + sessions.save(sessions.get_or_create("tui:a")) + + restored: list[object] = [] + loop = MagicMock() + loop.sessions = sessions + loop.restore_session_model = lambda *a, **k: restored.append(a) + loop.session_model = lambda key: "boot/model" + + await session_resume({"session_id": "tui:a"}, agent_loop_factory=lambda: loop) + + assert restored == [] + + +async def test_session_branch_carries_the_parents_model_to_the_child( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """A fork continues its parent's conversation on its parent's model, and + keeps it across a restart. + + Both halves matter and fail independently: dropping the binding hand-off + leaves the child running on the default in this process, and dropping the + record leaves it running on the default in the next one. + """ + cfg = load_config() + cfg.agents.defaults.workspace = str(tmp_path) + monkeypatch.setattr(session_module, "load_config", lambda: cfg) + src_key = "tui:20260610_143052_bb0001" + _write_session(tmp_path, src_key, [{"role": "user", "content": "hi"}]) + + sessions = SessionManager(tmp_path) + parent = sessions.get_or_create(src_key) + parent.metadata["model"] = "vendor-a/model" + parent.metadata["provider"] = "anthropic" + sessions.save(parent) + + parent_binding = SimpleNamespace(provider="prov-a", model="vendor-a/model") + + class _Loop: + def __init__(self) -> None: + self.sessions = sessions + self.bindings: dict[str, object] = {src_key: parent_binding} + + def has_session_binding(self, key: str) -> bool: + return key in self.bindings + + def binding_for_session(self, key: str) -> object: + return self.bindings.get(key, SimpleNamespace(provider="boot", model="boot/model")) + + def set_session_binding(self, key: str, binding: object) -> None: + self.bindings[key] = binding + + loop = _Loop() + result = await session_branch({"session_id": src_key}, agent_loop_factory=lambda: loop) + + child_key = result["session_id"] + assert loop.bindings[child_key] is parent_binding, "the fork must run on its parent's model now" + + reloaded = SessionManager(tmp_path).get_or_create(child_key) + assert reloaded.metadata["model"] == "vendor-a/model", "and after a restart" + assert reloaded.metadata["provider"] == "anthropic" + + +async def test_session_delete_releases_the_sessions_binding(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """A deleted session must not leave its override -- and the live provider + behind it -- held for the life of the process. + """ + cfg = load_config() + cfg.agents.defaults.workspace = str(tmp_path) + monkeypatch.setattr(session_module, "load_config", lambda: cfg) + key = "tui:20260610_100000_bb0002" + _write_session(tmp_path, key, [{"role": "user", "content": "hi"}]) + + cleared: list[str] = [] + + class _Loop: + sessions = None + + def clear_session_binding(self, session_key: str) -> None: + cleared.append(session_key) + + await session_delete({"session_id": key}, agent_loop_factory=lambda: _Loop()) + + assert cleared == [key] diff --git a/ui-tui/CONTEXT.md b/ui-tui/CONTEXT.md index 184d7874..7d6d1a85 100644 --- a/ui-tui/CONTEXT.md +++ b/ui-tui/CONTEXT.md @@ -9,6 +9,18 @@ talks to the Runtime only via TUI-RPC. Single-session per client in v0.1. ## Language +**Model scope** (TUI): +Which conversations a `/model` switch reaches. Plain `/model ` is +session-scoped: it moves this conversation only and does not touch the +configured default, so a new session still starts where it always did. +`/model --default` changes that default instead, leaving conversations +that already chose their own model alone -- but it does move the ones that +never chose, including, usually, the conversation that asked. Which of the two +happened is the server's answer (`applies_to_session`), not something the scope +implies, and it is what decides whether the status bar repaints. The picker +shows the scope it will use. +_Avoid_: "global model switch" -- that was the pre-session behaviour. + **Overlay**: A modal layer over the chat view, tracked in `overlayStore` and driven by keyboard. Kinds split into RPC-driven (Confirm, Approval, Clarify, Sudo, Secret) and user-toggled (Agents, diff --git a/ui-tui/rpc-schema/openrpc.json b/ui-tui/rpc-schema/openrpc.json index 6baa90c9..2f688cfa 100644 --- a/ui-tui/rpc-schema/openrpc.json +++ b/ui-tui/rpc-schema/openrpc.json @@ -876,6 +876,24 @@ "name": "value", "required": true, "schema": { "$ref": "#/components/schemas/JsonValue" } + }, + { + "name": "session_id", + "summary": "Scopes a model switch to one conversation.", + "required": false, + "schema": { "type": "string" } + }, + { + "name": "provider", + "summary": "Vendor for a model switch; derived from the id when absent.", + "required": false, + "schema": { "type": "string" } + }, + { + "name": "scope", + "summary": "How far a model switch reaches: this conversation, or what new ones start on.", + "required": false, + "schema": { "type": "string", "enum": ["session", "default"] } } ], "result": { @@ -891,7 +909,11 @@ { "$ref": "#/components/schemas/JsonValue" }, { "type": "null" } ] - } + }, + "value": { "type": "string" }, + "scope": { "type": "string", "enum": ["session", "default"] }, + "session_id": { "type": "string" }, + "applies_to_session": { "type": "boolean" } } } }, @@ -1753,10 +1775,6 @@ "code": -32008, "message": "model_not_available" }, - "ModelSwitchInTurn": { - "code": -32009, - "message": "model_switch_in_turn" - }, "ConfigFieldReadonly": { "code": -32010, "message": "config_field_readonly" diff --git a/ui-tui/src/__tests__/createSlashHandler.test.ts b/ui-tui/src/__tests__/createSlashHandler.test.ts index d9143938..1d5da738 100644 --- a/ui-tui/src/__tests__/createSlashHandler.test.ts +++ b/ui-tui/src/__tests__/createSlashHandler.test.ts @@ -65,10 +65,109 @@ describe('createSlashHandler', () => { expect(ctx.gateway.rpc).toHaveBeenCalledWith('config.set', { key: 'model', session_id: 'sid-abc', + scope: 'session', value: 'x-model' }) }) + it('sends scope default and leaves the status bar alone when the session kept its own model', async () => { + patchUiState({ sid: 'sid-abc', info: { model: 'session-model', skills: {}, tools: {} } }) + + const ctx = buildCtx({ + gateway: { + ...buildGateway(), + rpc: vi.fn(() => + Promise.resolve({ + applied: true, + previous: null, + value: 'new-default', + scope: 'default', + applies_to_session: false + }) + ) + } + }) + + expect(createSlashHandler(ctx)('/model new-default --default')).toBe(true) + expect(ctx.gateway.rpc).toHaveBeenCalledWith('config.set', { + key: 'model', + session_id: 'sid-abc', + scope: 'default', + value: 'new-default' + }) + await Promise.resolve() + await Promise.resolve() + // This conversation chose its own model, so painting the new default into + // the status bar would show a model it is not on. + expect(getUiState().info?.model).toBe('session-model') + }) + + it('updates the status bar for /model --default when the session was following the default', async () => { + // The common case: a fresh conversation that never switched reads the + // default, so a default-scoped switch moves it immediately. Leaving the bar + // alone here showed the old model for the life of the session, because the + // bar is only refreshed on session.create / session.resume. + patchUiState({ sid: 'sid-abc', info: { model: 'old-default', skills: {}, tools: {} } }) + + const ctx = buildCtx({ + gateway: { + ...buildGateway(), + rpc: vi.fn(() => + Promise.resolve({ + applied: true, + previous: null, + value: 'new-default', + scope: 'default', + applies_to_session: true + }) + ) + } + }) + + expect(createSlashHandler(ctx)('/model new-default --default')).toBe(true) + await Promise.resolve() + await Promise.resolve() + expect(getUiState().info?.model).toBe('new-default') + }) + + it('reports an unapplied switch as an error and leaves the status bar alone', async () => { + patchUiState({ sid: 'sid-abc', info: { model: 'session-model', skills: {}, tools: {} } }) + + const ctx = buildCtx({ + gateway: { + ...buildGateway(), + rpc: vi.fn(() => Promise.resolve({ applied: false, previous: null, value: 'x-model' })) + } + }) + + expect(createSlashHandler(ctx)('/model x-model')).toBe(true) + await Promise.resolve() + await Promise.resolve() + expect(ctx.transcript.sys).toHaveBeenCalledWith('error: model switch was not applied: x-model') + expect(getUiState().info?.model).toBe('session-model') + }) + + it('strips --default from any position and opens the picker when nothing is left', async () => { + patchUiState({ sid: 'sid-abc' }) + + const ctx = buildCtx({ + gateway: { + ...buildGateway(), + rpc: vi.fn(() => Promise.resolve({ applied: true, previous: null, value: 'leading' })) + } + }) + + expect(createSlashHandler(ctx)('/model --default leading')).toBe(true) + expect(ctx.gateway.rpc).toHaveBeenCalledWith( + 'config.set', + expect.objectContaining({ value: 'leading', scope: 'default' }) + ) + + const picker = buildCtx({ gateway: { ...buildGateway(), rpc: vi.fn() } }) + expect(createSlashHandler(picker)('/model --default')).toBe(true) + expect(picker.gateway.rpc).not.toHaveBeenCalled() + }) + it('parses a --provider suffix into a structured provider param', async () => { patchUiState({ sid: 'sid-abc' }) @@ -84,6 +183,7 @@ describe('createSlashHandler', () => { key: 'model', provider: 'openrouter', session_id: 'sid-abc', + scope: 'session', value: 'claude-sonnet-4.6' }) }) diff --git a/ui-tui/src/app/slash/commands/session.ts b/ui-tui/src/app/slash/commands/session.ts index 6f5f956e..344c5846 100644 --- a/ui-tui/src/app/slash/commands/session.ts +++ b/ui-tui/src/app/slash/commands/session.ts @@ -24,7 +24,7 @@ import { DEFAULT_INDICATOR_STYLE, INDICATOR_STYLES, type IndicatorStyle } from ' import { patchOverlayState } from '../../overlayStore.js' import { patchUiState } from '../../uiStore.js' -// v1 model switch is global-scope only. The picker passes ` --provider +// A model switch is per conversation; `--default` also changes what new ones start on. The picker passes ` --provider // `; a bare `/model ` carries no provider. Parse both into the // structured config.set params {key:'model', value, provider?}. const parseModelArg = (arg: string): { provider?: string; value: string } => { @@ -69,20 +69,27 @@ export const sessionCommands: SlashCommand[] = [ help: 'change or show model', name: 'model', run: (arg, ctx) => { - if (ctx.session.guardBusySessionSwitch('change models')) { - return - } - - if (!arg.trim()) { + // No busy guard: the server captures the binding at turn entry, so a + // switch asked for mid-answer lands on the next turn instead of being + // refused. + // `--default` changes what new sessions start on; without it the switch + // is this conversation's alone. Stripped before parsing, in any + // position and however many times, so it never lands in the model id. + const raw = arg.trim() + const asDefault = /(^|\s)--default(\s|$)/.test(raw) + const rest = raw.replace(/(^|\s)--default(?=\s|$)/g, '').trim() + if (!rest) { + // `/model` and `/model --default` both mean "show me the choices". return patchOverlayState({ modelPicker: true }) } - const { provider, value } = parseModelArg(arg) + const { provider, value } = parseModelArg(rest) ctx.gateway .rpc('config.set', { key: 'model', session_id: ctx.sid, + scope: asDefault ? 'default' : 'session', value, ...(provider ? { provider } : {}) }) @@ -91,14 +98,25 @@ export const sessionCommands: SlashCommand[] = [ if (!r.value) { return ctx.transcript.sys('error: invalid response: model switch') } + if (!r.applied) { + // Nothing was built, so nothing was validated -- reporting a + // switch here would leave the bar on a model no turn will use. + return ctx.transcript.sys(`error: model switch was not applied: ${r.value}`) + } - ctx.transcript.sys(`model → ${r.value}`) + ctx.transcript.sys(asDefault ? `default model → ${r.value}` : `model → ${r.value}`) ctx.local.maybeWarn(r) - patchUiState(state => ({ - ...state, - info: state.info ? { ...state.info, model: r.value! } : { model: r.value!, skills: {}, tools: {} } - })) + // Whether this conversation now runs the model is the server's + // answer, not something the scope implies: a default-scoped switch + // does move a session that never chose its own model, and that is + // the common case for `--default`. + if (r.applies_to_session !== false) { + patchUiState(state => ({ + ...state, + info: state.info ? { ...state.info, model: r.value! } : { model: r.value!, skills: {}, tools: {} } + })) + } }) ) } diff --git a/ui-tui/src/components/modelPicker.tsx b/ui-tui/src/components/modelPicker.tsx index 6610d7f9..6c8c5056 100644 --- a/ui-tui/src/components/modelPicker.tsx +++ b/ui-tui/src/components/modelPicker.tsx @@ -1032,7 +1032,7 @@ export function ModelPicker({ gw, launcher, onCancel, onSelect, sessionId, suspe - scope: global + {'scope: this conversation · /model --default sets the new-session default'} {models.length diff --git a/ui-tui/src/gatewayTypes.ts b/ui-tui/src/gatewayTypes.ts index 612d003b..c5b275cd 100644 --- a/ui-tui/src/gatewayTypes.ts +++ b/ui-tui/src/gatewayTypes.ts @@ -102,10 +102,16 @@ export interface ConfigGetValueResponse { export interface ConfigSetResponse { applied?: boolean + // Does the asking conversation now run this model? A default-scoped switch + // moves the sessions that never chose one, so the scope alone cannot answer + // it and a client that guesses paints a model the conversation is not on. + applies_to_session?: boolean credential_warning?: string history_reset?: boolean info?: SessionInfo previous?: null | string + scope?: 'default' | 'session' + session_id?: string value?: string warning?: string } diff --git a/ui-tui/src/rpc/errors.ts b/ui-tui/src/rpc/errors.ts index a2d93b2d..1103f911 100644 --- a/ui-tui/src/rpc/errors.ts +++ b/ui-tui/src/rpc/errors.ts @@ -70,12 +70,6 @@ export class ModelNotAvailableError extends RpcError { this.name = 'ModelNotAvailableError' } } -export class ModelSwitchInTurnError extends RpcError { - constructor(f: JsonRpcErrorObject) { - super(f) - this.name = 'ModelSwitchInTurnError' - } -} export class ConfigFieldReadonlyError extends RpcError { constructor(f: JsonRpcErrorObject) { super(f) @@ -124,7 +118,6 @@ const CODE_TO_CTOR: Record RpcError> = { [-32006]: SkillNotFoundError, [-32007]: SkillPinConflictError, [-32008]: ModelNotAvailableError, - [-32009]: ModelSwitchInTurnError, [-32010]: ConfigFieldReadonlyError, [-32011]: ConfigValidationError, [-32012]: NotSupportedInV01Error, diff --git a/ui-tui/src/rpc/generated.ts b/ui-tui/src/rpc/generated.ts index 6ee8f91b..0b572d32 100644 --- a/ui-tui/src/rpc/generated.ts +++ b/ui-tui/src/rpc/generated.ts @@ -872,6 +872,9 @@ export interface ConfigGetResult { export interface ConfigSetParams { key: string; value: JsonValue; + session_id?: string; + provider?: string; + scope?: 'session' | 'default'; } /** * This interface was referenced by `RavenRpcRoot`'s JSON-Schema @@ -880,6 +883,10 @@ export interface ConfigSetParams { export interface ConfigSetResult { applied: boolean; previous: JsonValue | null; + value?: string; + scope?: 'session' | 'default'; + session_id?: string; + applies_to_session?: boolean; } /** * This interface was referenced by `RavenRpcRoot`'s JSON-Schema diff --git a/ui-tui/src/rpc/index.ts b/ui-tui/src/rpc/index.ts index 888c95ca..0dc5e5f3 100644 --- a/ui-tui/src/rpc/index.ts +++ b/ui-tui/src/rpc/index.ts @@ -16,7 +16,6 @@ export { SkillNotFoundError, SkillPinConflictError, ModelNotAvailableError, - ModelSwitchInTurnError, ConfigFieldReadonlyError, ConfigValidationError, NotSupportedInV01Error,