diff --git a/src/query/query.py b/src/query/query.py index 1ed55c5d3..ce5cf0724 100644 --- a/src/query/query.py +++ b/src/query/query.py @@ -402,6 +402,15 @@ async def _call_model_sync( _t0 = time.monotonic() tool_schemas = [] for tool in tools: + # Filter out internal/hidden tools (is_enabled=False) so they + # don't leak into the API tools[] alongside the advisor schema + # we append below. Some callers pass an unfiltered tool list + # from ``registry.list_tools()``; this guard keeps the API + # from receiving duplicate names. ``getattr`` with default + # True keeps test fakes that don't implement is_enabled working. + is_enabled_fn = getattr(tool, "is_enabled", None) + if callable(is_enabled_fn) and not is_enabled_fn(): + continue tool_schemas.append({ "name": tool.name, "description": tool.prompt(), @@ -1478,6 +1487,12 @@ async def query( # tools ignore the field (the existing default factory was an # empty list, so behavior is unchanged for them). tool_use_context.messages = list(messages) + # Also snapshot the active provider via a dynamic attribute so + # the client-side advisor can reuse it (and its config) when + # the user is on a proxy that probably proxies the advisor + # model too. Set as a plain attribute (not a dataclass field) + # to avoid touching ToolContext's public surface. + setattr(tool_use_context, "_active_provider", params.provider) tool_results = await _run_tools_partitioned( tool_use_blocks, diff --git a/src/tool_system/agent_loop.py b/src/tool_system/agent_loop.py index 7844ca5e8..333490fa2 100644 --- a/src/tool_system/agent_loop.py +++ b/src/tool_system/agent_loop.py @@ -307,8 +307,15 @@ def run_agent_loop( Returns: AgentLoopResult with final text response, usage info, and turn count """ + # Filter by is_enabled() so internal/hidden tools (e.g. the + # client-side AdvisorTool, which is registered but + # ``is_enabled=False`` to keep it out of the default schema) don't + # leak into the tools[] sent to the API. The advisor schema is + # appended per-turn below when advisor activation fires. tool_schemas = [] for tool in tool_registry.list_tools(): + if not tool.is_enabled(): + continue tool_schemas.append({ "name": tool.name, "description": tool.prompt(), @@ -347,12 +354,77 @@ def _check_cancel() -> None: # Use OpenAI formatted messages for non-Anthropic api_messages = openai_messages - call_kwargs: dict[str, Any] = {"tools": tool_schemas} + # Advisor activation — same decision tree as query.py's + # ``_call_model_sync``. We recompute per turn so that mid-session + # ``/advisor`` toggles take effect on the very next turn. The + # main path (REPL/TUI/headless) all routes through this function; + # without this block, the advisor would never actually fire in + # those paths even though the schema + instructions are correctly + # built. See src/utils/advisor.py:decide_advisor_mode. + from src.utils.advisor import ( + ADVISOR_BETA_HEADER, + ADVISOR_MODE_CLIENT_SIDE, + ADVISOR_MODE_INACTIVE, + ADVISOR_MODE_SERVER_SIDE, + ADVISOR_TOOL_INSTRUCTIONS, + build_advisor_tool_schema, + build_client_advisor_tool_schema, + decide_advisor_mode, + ) + advisor_mode = ADVISOR_MODE_INACTIVE + advisor_model_canonical: str | None = None + try: + from src.models.model import canonical_model_name + from src.settings.settings import get_settings + settings = get_settings() + configured = (getattr(settings, "advisor_model", "") or "").strip() + force_client = bool(getattr(settings, "advisor_client_mode", False)) + main_loop_model = getattr(provider, "model", "") or "" + if configured: + candidate = canonical_model_name(configured) + advisor_mode = decide_advisor_mode( + provider, + main_loop_model, + candidate, + force_client_mode=force_client, + ) + if advisor_mode != ADVISOR_MODE_INACTIVE: + advisor_model_canonical = candidate + except Exception: + advisor_mode = ADVISOR_MODE_INACTIVE + advisor_model_canonical = None + + # Build a per-turn tool list (base + maybe advisor at the END + # so the cache_control marker on the last base tool stays in + # place — same discipline as _call_model_sync). + turn_tool_schemas = list(tool_schemas) + if advisor_mode == ADVISOR_MODE_SERVER_SIDE and advisor_model_canonical: + turn_tool_schemas.append(build_advisor_tool_schema(advisor_model_canonical)) + elif advisor_mode == ADVISOR_MODE_CLIENT_SIDE: + turn_tool_schemas.append(build_client_advisor_tool_schema()) + + # Per-turn system prompt — append ADVISOR_TOOL_INSTRUCTIONS at + # the end (cache-friendly position) for both server and client + # modes; both rely on the same instruction text. + turn_system_prompt = effective_system_prompt + if advisor_mode != ADVISOR_MODE_INACTIVE: + if turn_system_prompt: + turn_system_prompt = f"{turn_system_prompt}\n\n{ADVISOR_TOOL_INSTRUCTIONS}" + else: + turn_system_prompt = ADVISOR_TOOL_INSTRUCTIONS + + call_kwargs: dict[str, Any] = {"tools": turn_tool_schemas} if _is_anthropic_provider(provider): - call_kwargs["system"] = effective_system_prompt + call_kwargs["system"] = turn_system_prompt else: if turn == 0: - api_messages = [{"role": "system", "content": effective_system_prompt}, *api_messages] + api_messages = [{"role": "system", "content": turn_system_prompt}, *api_messages] + if advisor_mode == ADVISOR_MODE_SERVER_SIDE: + # Anthropic beta header. SDK auto-converts ``betas`` into the + # ``anthropic-beta`` header. Server-side advisor only fires + # on 1P Anthropic providers; agent_loop's anthropic path + # accepts arbitrary kwargs forwarded to the SDK. + call_kwargs["betas"] = [ADVISOR_BETA_HEADER] response, streamed_live_text = _call_provider_for_turn( provider=provider, api_messages=api_messages, @@ -430,6 +502,15 @@ def _check_cancel() -> None: num_turns=turn_count, ) + # Snapshot active conversation + provider onto the ToolContext + # so tools that need them (currently: the client-side advisor) + # can read them. ``messages`` is the live conversation up to + # and including the assistant message that just emitted these + # tool_use blocks. ``_active_provider`` lets the advisor reuse + # the main provider's class/config when it's a proxy. + tool_context.messages = list(conversation.messages) + setattr(tool_context, "_active_provider", provider) + # Call each tool for tool_use in tool_uses: _check_cancel() diff --git a/src/tool_system/tools/advisor.py b/src/tool_system/tools/advisor.py index ac80701f6..805808702 100644 --- a/src/tool_system/tools/advisor.py +++ b/src/tool_system/tools/advisor.py @@ -55,10 +55,19 @@ def _advisor_call(tool_input: dict[str, Any], context: ToolContext) -> ToolResul abort = getattr(context, "abort_controller", None) abort_signal = getattr(abort, "signal", None) if abort is not None else None + # ``_active_provider`` is set by ``_call_model_sync`` before the + # tool round (dynamic attr, not a dataclass field). Letting + # ``execute_client_advisor`` see it enables the "reuse main + # provider when it's a proxy" routing — without it, an advisor + # running on a litellm-backed openai-compat session would bypass + # the proxy and hit api.anthropic.com directly. + main_provider = getattr(context, "_active_provider", None) + ok, text = execute_client_advisor( advisor_model, forwarded, abort_signal=abort_signal, + main_provider=main_provider, ) return ToolResult( name="advisor", diff --git a/src/utils/advisor.py b/src/utils/advisor.py index a0521adc5..d59edab26 100644 --- a/src/utils/advisor.py +++ b/src/utils/advisor.py @@ -413,32 +413,163 @@ def decide_advisor_mode( return ADVISOR_MODE_CLIENT_SIDE if advisor_routes else ADVISOR_MODE_INACTIVE +CLIENT_ADVISOR_PROMPT_SUFFIX = ( + "Please review the conversation above and give me your advice on what " + "to do next. Be concrete." +) + + +def _tool_use_to_text(block: dict[str, Any]) -> str: + """Render a ``tool_use`` / ``server_tool_use`` / ``mcp_tool_use`` block + as a single-line text summary the advisor can read without needing + the underlying tool schemas.""" + import json as _json + name = block.get("name", "?") + raw_input = block.get("input", {}) + try: + rendered = _json.dumps(raw_input, ensure_ascii=False, default=str) + except Exception: + rendered = str(raw_input) + if len(rendered) > 240: + rendered = rendered[:237] + "..." + return f"[Tool call: {name}({rendered})]" + + +def _tool_result_to_text(block: dict[str, Any]) -> str: + """Render a ``tool_result`` block as a single-line text summary.""" + content = block.get("content") + if isinstance(content, str): + text = content + elif isinstance(content, list): + parts: list[str] = [] + for sub in content: + if isinstance(sub, dict): + if isinstance(sub.get("text"), str): + parts.append(sub["text"]) + elif sub.get("type") == "image": + parts.append("[image]") + else: + parts.append(f"[{sub.get('type', '?')}]") + text = "\n".join(parts) + elif content is None: + text = "" + else: + text = str(content) + if len(text) > 1200: + text = text[:1197] + "..." + is_error = block.get("is_error") + label = "Tool error" if is_error else "Tool result" + return f"[{label}: {text}]" + + +def _flatten_content_for_advisor(content: Any) -> str: + """Reduce a message's content to plain text suitable for the advisor. + + The forwarded conversation must be tool-schema-free (the advisor is + called with ``tools=[]`` — proxies reject ``tool_use``/``tool_result`` + blocks without a matching ``tools=`` array). Replace them with text + summaries that preserve the information ("the worker ran Bash with + ls", "the result was these files") without the typed structure. + + Drops ``thinking`` / ``redacted_thinking`` blocks — the advisor + doesn't need the worker's chain-of-thought as separate signal. + """ + if isinstance(content, str): + return content + if not isinstance(content, list): + return str(content) if content is not None else "" + parts: list[str] = [] + for block in content: + if not isinstance(block, dict): + continue + bt = block.get("type") + if bt == "text": + t = block.get("text") + if isinstance(t, str) and t.strip(): + parts.append(t) + elif bt in ("tool_use", "server_tool_use", "mcp_tool_use"): + parts.append(_tool_use_to_text(block)) + elif bt == "tool_result": + parts.append(_tool_result_to_text(block)) + elif bt in ("thinking", "redacted_thinking"): + continue + elif bt == "image": + parts.append("[image attachment]") + else: + # Unknown block — preserve the type signal but no payload. + parts.append(f"[{bt}]") + return "\n".join(parts).strip() + + def build_advisor_forwarded_messages( messages: list[Any], ) -> list[dict[str, Any]]: - """Normalize + strip advisor-specific blocks before forwarding to the + """Normalize + strip + flatten messages before forwarding to the client-side advisor. - The advisor model should see the *substance* of the conversation — - user's task, worker's text replies, tool calls, tool results — but - NOT the advisor's own prior consultations (which would balloon the - forwarded context and confuse the reviewer about whose advice is - whose). We also strip out the advisor tool schema entry from any - serialized tool list, but since this function only handles the - messages array, the schema-stripping happens at the request-build - site (we don't forward tools[] to the advisor anyway). + Three transforms: + + 1. **Strip prior advisor consultations** — the reviewer shouldn't + see its own past advice as part of the worker's history; that + would let the advisor build on its own (potentially wrong) + earlier output. + 2. **Flatten tool_use/tool_result blocks to text** — the advisor is + called with ``tools=[]``, but proxies (Vertex-fronted Anthropic + in particular) reject ``tool_use``/``tool_result`` blocks when + no ``tools=`` array is sent. Plain text summaries preserve the + "what happened" information while satisfying the API contract. + 3. **Ensure ends-with-user** — the advisor is invoked from inside + an assistant ``tool_use``, so the natural tail is assistant. + Most LLM APIs reject assistant-prefill; append a synthetic user + turn asking for advice (doubles as a clear prompt aligned with + ``CLIENT_ADVISOR_SYSTEM_PROMPT``). - Accepts the same shape as ``normalize_messages_for_api`` produces. Returns a plain list of dicts safe to send to any provider. """ - # Local import — same cycle-avoidance reason as elsewhere in this - # module. ``normalize_messages_for_api`` projects typed Message - # objects to API dicts; we then run the existing advisor-blocks - # stripper to drop the prior consultations. + # Local imports — same cycle-avoidance reason as elsewhere. from src.types.messages import normalize_messages_for_api api_messages = normalize_messages_for_api(messages) - return strip_advisor_blocks(api_messages) + api_messages = strip_advisor_blocks(api_messages) + + flattened: list[dict[str, Any]] = [] + for msg in api_messages: + if not isinstance(msg, Mapping): + continue + role = msg.get("role") + text = _flatten_content_for_advisor(msg.get("content")) + if not text: + continue + flattened.append({"role": role, "content": text}) + + # Ensure the conversation ends with a user message. Vertex-fronted + # Anthropic (and most proxies) reject assistant-prefill. + if not flattened or flattened[-1].get("role") != "user": + flattened.append({"role": "user", "content": CLIENT_ADVISOR_PROMPT_SUFFIX}) + return flattened + + +def _provider_is_using_custom_endpoint(provider: Any) -> bool: + """True if the provider is pointed at a non-default base URL. + + Heuristic for "this provider is a proxy that can serve other + models" (litellm, openrouter, custom-deployment Anthropic shim, + etc.). When True, ``execute_client_advisor`` prefers the SAME + provider+config for the advisor call rather than inferring a + direct upstream from the model name — the user explicitly chose + a proxy, so route through it. + """ + from src.providers import PROVIDER_INFO + base_url = getattr(provider, "base_url", None) + if not base_url: + return False + name = provider.__class__.__name__.replace("Provider", "").lower() + info = PROVIDER_INFO.get(name) + if info is None: + # Unknown class — assume the user customized it for a reason. + return True + default = info.get("default_base_url", "") + return base_url.rstrip("/") != default.rstrip("/") def execute_client_advisor( @@ -446,6 +577,7 @@ def execute_client_advisor( forwarded_messages: list[dict[str, Any]], *, abort_signal: Any = None, + main_provider: Any = None, ) -> tuple[bool, str]: """Run one client-side advisor consultation. @@ -453,11 +585,18 @@ def execute_client_advisor( advisor's advice; when False, ``text`` is a short error message suitable for surfacing as a tool_result with ``is_error=True``. - Provider routing goes through ``infer_provider_for_model`` → - ``get_provider_class`` + ``get_provider_config`` — the same path - the main loop uses for its own provider, so user-configured API - keys / base URLs / auth headers are reused. No advisor-specific - credentials. + Provider routing: + * If ``main_provider`` was passed AND it has a custom base URL + (i.e. the user is talking to a proxy like litellm/openrouter + that proxies arbitrary models), reuse the main provider's class + and config with the advisor's model — the proxy will handle it. + Without this, a litellm user would have their advisor call + bounce direct to api.anthropic.com instead of through their + configured proxy. + * Otherwise, infer the advisor's provider from the model name and + build a fresh provider from the user's saved config for that + provider. This handles the "advisor on a different provider" + case (e.g. Anthropic main + Gemini advisor). Network failures, model errors, and missing-config conditions are all caught and surfaced as ``(False, "...")`` rather than raised — @@ -465,38 +604,89 @@ def execute_client_advisor( advisor consultation should just leave the worker model uninformed and let it continue. """ - provider_name = infer_provider_for_model(advisor_model) - if provider_name is None: - return (False, f"Advisor unavailable: cannot route model {advisor_model!r} to a known provider.") - try: from src.config import get_provider_config from src.providers import get_provider_class - provider_cls = get_provider_class(provider_name) - cfg = dict(get_provider_config(provider_name)) - # The provider config supplies base_url / api_key / etc.; we - # override the model with the advisor's specific choice so it - # doesn't inherit the user's main-loop model from the same - # provider's default. ``ChatProvider.__init__`` expects the - # model on the instance, not in the messages array. - cfg["model"] = advisor_model - provider = provider_cls(**cfg) + if main_provider is not None and _provider_is_using_custom_endpoint( + main_provider + ): + # Reuse main provider's class + config; just swap the model. + provider_cls = main_provider.__class__ + name = provider_cls.__name__.replace("Provider", "").lower() + cfg_raw = dict(get_provider_config(name)) + else: + provider_name = infer_provider_for_model(advisor_model) + if provider_name is None: + return (False, f"Advisor unavailable: cannot route model {advisor_model!r} to a known provider.") + provider_cls = get_provider_class(provider_name) + cfg_raw = dict(get_provider_config(provider_name)) + + # ``get_provider_config`` returns the raw config dict shape + # (api_key, base_url, default_model) which doesn't match the + # Provider ``__init__`` keyword args (api_key, base_url, model). + # Translate explicitly so unknown keys (default_model, plus any + # future config fields like extra_headers) don't get forwarded + # as kwargs and crash the constructor. + provider = provider_cls( + api_key=cfg_raw.get("api_key", ""), + base_url=cfg_raw.get("base_url"), + model=advisor_model, + ) except Exception as e: # noqa: BLE001 — surface as advisor failure return (False, f"Advisor unavailable: failed to construct provider for {advisor_model!r}: {e}") - # The advisor doesn't make tool calls — it just emits advice text — - # so we send an empty tools list. Forward the conversation as - # user-role context wrapped under our advisor system prompt. + # System-prompt delivery is provider-specific: + # * Anthropic-shaped providers (AnthropicProvider / MinimaxProvider) + # expect ``system`` as a top-level kwarg; system-role messages + # in the messages array would be rejected by the API. + # * OpenAI-compatible providers (and Gemini-via-openai-shim) read + # a leading ``{"role": "system", "content": ...}`` message and + # ignore the ``system=`` kwarg silently. + # Detect the provider type to send the right shape — sending both + # forms blindly would either be ignored (best case) or fail + # validation (worst case, on Anthropic). + from src.providers.anthropic_provider import AnthropicProvider + from src.providers.minimax_provider import MinimaxProvider + + is_anthropic_shape = isinstance(provider, (AnthropicProvider, MinimaxProvider)) + + call_kwargs: dict[str, Any] = { + "tools": [], + "max_tokens": 4096, + } + if is_anthropic_shape: + call_kwargs["system"] = CLIENT_ADVISOR_SYSTEM_PROMPT + request_messages = list(forwarded_messages) + else: + # Prepend the system message; OpenAI-compat will honor it + # naturally as the first message in the conversation. + request_messages = [ + {"role": "system", "content": CLIENT_ADVISOR_SYSTEM_PROMPT}, + *forwarded_messages, + ] + + # ``chat_stream_response`` is the cross-provider call that accepts + # ``abort_signal`` uniformly (per BaseProvider) and returns a fully + # accumulated ChatResponse. The plain ``chat()`` path doesn't accept + # ``abort_signal`` consistently across providers — passing it as a + # kwarg would forward an unknown param to the underlying SDK for + # Anthropic (line 239 of anthropic_provider.py forwards unknown + # kwargs straight to ``messages.create``). Streaming under the hood + # but no ``on_text_chunk`` callback — we only need the final text. try: - response = provider.chat( - messages=forwarded_messages, - system=CLIENT_ADVISOR_SYSTEM_PROMPT, - tools=[], - max_tokens=4096, - stream=False, - abort_signal=abort_signal, - ) + try: + response = provider.chat_stream_response( + request_messages, + on_text_chunk=None, + abort_signal=abort_signal, + **call_kwargs, + ) + except (NotImplementedError, AttributeError): + # Older or stub providers may not implement streaming. + # Fall back to plain chat() — drop abort_signal there since + # we can't pass it portably. + response = provider.chat(request_messages, **call_kwargs) except Exception as e: # noqa: BLE001 — surface as advisor failure return (False, f"Advisor unavailable: {type(e).__name__}: {e}") diff --git a/tests/test_advisor_client_side.py b/tests/test_advisor_client_side.py index a145575f7..3d7e4106f 100644 --- a/tests/test_advisor_client_side.py +++ b/tests/test_advisor_client_side.py @@ -251,11 +251,34 @@ def test_returns_plain_dicts_safe_to_send(self) -> None: class TestExecuteClientAdvisor(unittest.TestCase): - """``execute_client_advisor`` integration — provider factory wiring.""" + """``execute_client_advisor`` integration — provider factory wiring. + + The advisor uses ``chat_stream_response`` (cross-provider abort_signal + support) with a fallback to plain ``chat`` for providers that don't + implement it. + """ + + def _make_anthropic_shaped_provider(self, content: str = "advice") -> MagicMock: + """Mock that passes the isinstance check for AnthropicProvider.""" + from src.providers.anthropic_provider import AnthropicProvider + provider = MagicMock(spec=AnthropicProvider) + provider.chat_stream_response = MagicMock( + return_value=MagicMock(content=content) + ) + return provider + + def _make_openai_shape_provider(self, content: str = "advice") -> MagicMock: + """Mock that does NOT pass isinstance for AnthropicProvider — + the function should detect it as OpenAI-shape and prepend a + system-role message instead of passing system=kwarg.""" + provider = MagicMock() # bare MagicMock, no spec + provider.chat_stream_response = MagicMock( + return_value=MagicMock(content=content) + ) + return provider - def test_returns_text_on_success(self) -> None: - fake_provider = MagicMock() - fake_provider.chat = MagicMock(return_value=MagicMock(content="here is advice")) + def test_returns_text_on_success_anthropic(self) -> None: + fake_provider = self._make_anthropic_shaped_provider("here is advice") with patch( "src.providers.get_provider_class", return_value=lambda **kw: fake_provider ): @@ -267,12 +290,35 @@ def test_returns_text_on_success(self) -> None: ) self.assertTrue(ok) self.assertEqual(text, "here is advice") - # The provider got the advisor's system prompt and an empty - # tools list. - kw = fake_provider.chat.call_args.kwargs - self.assertEqual(kw.get("tools"), []) - self.assertIn("system", kw) - self.assertIn("reviewer", kw["system"].lower()) + # Anthropic-shaped → system goes as kwarg, NOT prepended to messages. + call = fake_provider.chat_stream_response.call_args + self.assertEqual(call.kwargs.get("tools"), []) + self.assertIn("system", call.kwargs) + self.assertIn("reviewer", call.kwargs["system"].lower()) + # Messages array unchanged (no system message prepended). + forwarded_messages = call.args[0] + self.assertEqual(forwarded_messages[0].get("role"), "user") + + def test_openai_shape_gets_system_as_first_message(self) -> None: + fake_provider = self._make_openai_shape_provider("advice from openai") + with patch( + "src.providers.get_provider_class", return_value=lambda **kw: fake_provider + ): + with patch( + "src.config.get_provider_config", return_value={"api_key": "test"} + ): + ok, text = execute_client_advisor( + "gpt-5.4", [{"role": "user", "content": "hi"}] + ) + self.assertTrue(ok) + self.assertEqual(text, "advice from openai") + # OpenAI-shape → system prepended as first message, NO system kwarg. + call = fake_provider.chat_stream_response.call_args + self.assertNotIn("system", call.kwargs) + forwarded_messages = call.args[0] + self.assertEqual(forwarded_messages[0]["role"], "system") + self.assertIn("reviewer", forwarded_messages[0]["content"].lower()) + self.assertEqual(forwarded_messages[1]["role"], "user") def test_returns_error_when_model_unroutable(self) -> None: ok, text = execute_client_advisor( @@ -282,8 +328,10 @@ def test_returns_error_when_model_unroutable(self) -> None: self.assertIn("cannot route", text.lower()) def test_returns_error_when_provider_raises(self) -> None: - fake_provider = MagicMock() - fake_provider.chat = MagicMock(side_effect=RuntimeError("network down")) + fake_provider = self._make_anthropic_shaped_provider() + fake_provider.chat_stream_response = MagicMock( + side_effect=RuntimeError("network down") + ) with patch( "src.providers.get_provider_class", return_value=lambda **kw: fake_provider ): @@ -297,8 +345,7 @@ def test_returns_error_when_provider_raises(self) -> None: self.assertIn("network down", text) def test_returns_error_when_response_empty(self) -> None: - fake_provider = MagicMock() - fake_provider.chat = MagicMock(return_value=MagicMock(content="")) + fake_provider = self._make_anthropic_shaped_provider("") with patch( "src.providers.get_provider_class", return_value=lambda **kw: fake_provider ): @@ -311,6 +358,105 @@ def test_returns_error_when_response_empty(self) -> None: self.assertFalse(ok) self.assertIn("no text", text.lower()) + def test_routes_through_main_provider_when_proxy(self) -> None: + # User's main provider is OpenAI pointed at litellm (custom + # base_url). The advisor model is claude-opus-4-7 which would + # normally infer to Anthropic — but the proxy assumption says + # "use the same proxy for both". We expect the advisor to be + # built via OpenAIProvider, NOT AnthropicProvider. + from src.providers.openai_provider import OpenAIProvider + # Make the main provider look like an OpenAI proxy: base_url + # set to something other than the default openai endpoint. + main_provider = MagicMock(spec=OpenAIProvider) + main_provider.base_url = "https://litellm.singula.ai" + main_provider.__class__ = OpenAIProvider + + # Spy on the constructor — the advisor provider should be + # built from OpenAIProvider, not from infer_provider_for_model's + # anthropic result. + constructed = {} + + def _fake_openai_init(**kwargs: Any) -> Any: + constructed["cls"] = "OpenAIProvider" + constructed["kwargs"] = kwargs + inst = MagicMock(spec=OpenAIProvider) + inst.chat_stream_response = MagicMock( + return_value=MagicMock(content="proxied advice") + ) + return inst + + with patch( + "src.config.get_provider_config", + return_value={"api_key": "k", "base_url": "https://litellm.singula.ai"}, + ): + with patch.object(OpenAIProvider, "__new__", lambda cls, **kw: _fake_openai_init(**kw)): + ok, text = execute_client_advisor( + "claude-opus-4-7", + [{"role": "user", "content": "hi"}], + main_provider=main_provider, + ) + self.assertTrue(ok) + self.assertEqual(text, "proxied advice") + self.assertEqual(constructed["cls"], "OpenAIProvider") + # Model swapped to the advisor's choice, base_url preserved + # (came from get_provider_config). + self.assertEqual(constructed["kwargs"]["model"], "claude-opus-4-7") + self.assertEqual( + constructed["kwargs"]["base_url"], "https://litellm.singula.ai" + ) + + def test_uses_inferred_provider_when_main_is_not_proxy(self) -> None: + # 1P Anthropic main loop (default base_url). Advisor model is + # gemini-2.5-pro → should route via inference to Gemini, NOT + # reuse the Anthropic main provider. + from src.providers.anthropic_provider import AnthropicProvider + main_provider = MagicMock(spec=AnthropicProvider) + main_provider.base_url = "https://api.anthropic.com" # default + + fake_gemini = MagicMock() + fake_gemini.chat_stream_response = MagicMock( + return_value=MagicMock(content="gemini says hi") + ) + with patch( + "src.providers.get_provider_class", + return_value=lambda **kw: fake_gemini, + ): + with patch( + "src.config.get_provider_config", return_value={"api_key": "k"} + ): + ok, text = execute_client_advisor( + "gemini-2.5-pro", + [{"role": "user", "content": "hi"}], + main_provider=main_provider, + ) + self.assertTrue(ok) + self.assertEqual(text, "gemini says hi") + + def test_falls_back_to_chat_when_stream_unimplemented(self) -> None: + # Older / stub providers may not implement chat_stream_response. + # The function should fall back to plain chat() gracefully. + from src.providers.anthropic_provider import AnthropicProvider + fake_provider = MagicMock(spec=AnthropicProvider) + fake_provider.chat_stream_response = MagicMock( + side_effect=NotImplementedError("no streaming"), + ) + fake_provider.chat = MagicMock(return_value=MagicMock(content="fallback worked")) + with patch( + "src.providers.get_provider_class", return_value=lambda **kw: fake_provider + ): + with patch( + "src.config.get_provider_config", return_value={"api_key": "test"} + ): + ok, text = execute_client_advisor( + "claude-opus-4-6", [{"role": "user", "content": "hi"}] + ) + self.assertTrue(ok) + self.assertEqual(text, "fallback worked") + # The fallback path did NOT pass abort_signal (sync chat doesn't + # consistently accept it across providers). + fallback_call = fake_provider.chat.call_args + self.assertNotIn("abort_signal", fallback_call.kwargs) + class TestAdvisorTool(unittest.TestCase): """The registered AdvisorTool — wires ctx.messages → execute."""