Skip to content

feat(advisor): client-side mode for 3P providers (any provider, any model) - #182

Merged
agentforce314 merged 1 commit into
mainfrom
advisor/3p-client-mode
May 20, 2026
Merged

feat(advisor): client-side mode for 3P providers (any provider, any model)#182
agentforce314 merged 1 commit into
mainfrom
advisor/3p-client-mode

Conversation

@agentforce314

Copy link
Copy Markdown
Owner

Summary

  • Extends /advisor to work on any provider (GLM, Gemini, OpenAI, OpenRouter, Bedrock-shimmed Anthropic, etc.) by adding a client-side execution path. The server-side Anthropic beta stays the optimized default when applicable; client-side activates for 3P providers, non-server-side advisor models, or when forced via --client.
  • New flags: /advisor <model> --client (force client-side), /advisor --no-client (clear), /advisor unset (clear all).
  • Now also supports cross-provider pairings: e.g. GLM main loop with Anthropic Opus advisor, or Anthropic main with Gemini advisor.

Mode-decision table

Provider Force --client Advisor model Mode
1P Anthropic off opus-4-6 / sonnet-4-6 Server-side (unchanged, optimal)
1P Anthropic on any routable Client-side
1P Anthropic off non-server-side (e.g. haiku, gemini) Client-side
3P n/a any routable Client-side
any any unroutable model Inactive

What changes

Activation

  • decide_advisor_mode(provider, main_model, advisor_model, force_client_mode) returns server_side / client_side / inactive.
  • infer_provider_for_model(model) does exact-match in PROVIDER_INFO, then well-known prefix fallback (claude-* → anthropic, gemini-* → gemini, zai/* → glm, <vendor>/* → openrouter, etc.). Rejects ambiguous strings rather than guessing.

Client-side path

  • AdvisorTool registered in src/tool_system/tools/advisor.py with is_enabled=lambda: False (hidden from /tools listings; registry still finds it for dispatch).
  • _call_model_sync injects the regular-tool-shape schema only when client-side mode is active for the request. Cache-friendly: appended after the regular tools and after the cached system blocks.
  • AdvisorTool.call(input, ctx) reads ctx.messages, strips prior advisor blocks via build_advisor_forwarded_messages, calls execute_client_advisor.
  • execute_client_advisor resolves the advisor's provider via the existing get_provider_class + get_provider_config (reuses user-configured API keys), makes one non-streaming chat() call with the advisor's own system prompt and empty tools, returns (ok, text_or_error). Never raises — degrades to a tool_result error on failure.

/advisor command

  • Drops strict is_valid_advisor_model rejection (the opus-4-6/sonnet-4-6 server-side gate). Now only requires the model to route via infer_provider_for_model.
  • Parses --client / --no-client flags (order-insensitive).
  • Status output shows the actual mode picked: Advisor: <model> — active (server-side|client-side|inactive) [--client forced?].

Settings + state

  • SettingsSchema.advisor_client_mode: bool = False
  • AppState.advisor_client_mode with _on_advisor_client_mode_change (persists to global settings + invalidates cache, matches the existing advisor_model handler pattern).

TUI

  • AgentBridge._emit_advisor_events extended: also detects client-side tool_use(name=\"advisor\") on assistant messages plus the matching tool_result on subsequent user messages. Emits the same AdvisorEventMessage types so the existing widget renders both flavors.

Loosened gates

  • can_user_configure_advisor: was 1P-only, now env-only. /advisor works on any provider.

Test plan

Verified locally (/Users/ericlee2/workspace/clawcodex/.venv/bin/pytest):

  • tests/test_advisor_helpers.py — 32 passed (1 updated: 3P provider now allowed)
  • tests/test_advisor_orphan_pairing.py — 6 passed
  • tests/test_advisor_chat_response_roundtrip.py — 6 passed
  • tests/test_advisor_command.py — 14 passed (4 updated for new semantics)
  • tests/test_advisor_request_wiring.py — 13 passed (3 inactive cases now correctly activate client-side)
  • tests/test_advisor_client_side.py — 30 new tests (mode decision table, model routing, forwarded message stripping, execute_client_advisor success/failure/empty, AdvisorTool call paths, schema shape)
  • tests/test_app_state.py — 16 passed
  • tests/integration/test_advisor_smoke.py — 3 passed
  • tests/test_tool_system_tools.py + test_tool_registry_pipeline.py — 99 passed (regression)
  • tests/test_settings*.py + tests/test_query*.py — 70 passed (regression)
  • Total: 120 advisor tests + 169 collateral tests passed

Design choices

  1. Server-side stays the optimized path — when 1P + valid models, no behavior change. One roundtrip, prompt-cache friendly, beta-header gated.
  2. Cache stability preserved — both modes append schema + instructions at the end. Toggling /advisor doesn't churn the cached prefix.
  3. Reviewer doesn't see its own prior consultationsbuild_advisor_forwarded_messages strips advisor blocks before forwarding. Each consultation is independent.
  4. Failure mode is degradationexecute_client_advisor catches every exception and surfaces as is_error=True tool_result. The turn doesn't die.
  5. Reuses existing provider config — no new API key surface. The advisor's provider goes through the same registry as the main loop.

🤖 Generated with Claude Code

Extends `/advisor` to work on any provider by adding a client-side
execution path. Server-side (Anthropic beta) stays the default when
applicable; client-side activates for 3P providers, non-server-side
advisor models, or when forced via `--client`.

## What changes

- New activation predicate `decide_advisor_mode` returns one of
  `server_side`/`client_side`/`inactive` per turn.
- Client-side path registers `advisor` as a hidden tool in the
  registry. When active, `_call_model_sync` appends its schema +
  injects `ADVISOR_TOOL_INSTRUCTIONS` to the system prompt (both
  Anthropic and 3P branches). Dispatcher routes the call to
  `AdvisorTool.call`, which forwards stripped conversation history
  to whatever provider routes the configured advisor model.
- `/advisor` command:
  - Accepts any resolvable model (drops strict
    `is_valid_advisor_model` rejection — now routes via
    `infer_provider_for_model`).
  - Parses `--client` / `--no-client` flags.
  - Status output shows the actual mode picked.
- New settings field `advisor_client_mode: bool = False` (force
  client-side even on 1P).
- `AppState.advisor_client_mode` with persistence + cache
  invalidation handler.
- TUI: `_emit_advisor_events` extended to detect client-side
  `tool_use(name=advisor)` + matching user-side `tool_result`.
- `can_user_configure_advisor` loosened to env-only (was 1P-only) —
  /advisor is now valid on any provider.

## Test plan

- [x] tests/test_advisor_helpers.py — 32 passed (1 updated)
- [x] tests/test_advisor_orphan_pairing.py — 6 passed
- [x] tests/test_advisor_chat_response_roundtrip.py — 6 passed
- [x] tests/test_advisor_command.py — 14 passed (4 updated)
- [x] tests/test_advisor_request_wiring.py — 13 passed (3 updated)
- [x] tests/test_advisor_client_side.py — **30 new tests**
- [x] tests/test_app_state.py — 16 passed
- [x] tests/integration/test_advisor_smoke.py — 3 passed
- [x] tests/test_tool_system_tools.py + test_tool_registry_pipeline.py — 99 passed (regression)
- [x] tests/test_settings*.py + tests/test_query*.py — 70 passed (regression)
- **Total: 120 advisor tests + 169 collateral tests passed**

## Design choices

Documented in `my-docs/advisor-mode.md` (local). Key calls:
1. Server-side stays the optimized path (1 roundtrip, cache-friendly)
   when applicable. Client-side is the fallback for everything else.
2. Cache stability: schema + instructions appended at the end of
   both tool list and system prompt — toggling `/advisor` doesn't
   churn the cached prefix.
3. AdvisorTool is hidden (`is_enabled=False`), not absent.
   `_call_model_sync` injects the schema only when client-side mode
   is active for the request.
4. Reviewer doesn't see its own prior consultations
   (`build_advisor_forwarded_messages` strips advisor blocks).
5. Failure mode is degradation: `execute_client_advisor` catches
   every exception and surfaces as a tool_result error rather than
   killing the turn.
6. Reuses existing provider config — no new API keys/env vars.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@agentforce314
agentforce314 merged commit cd9e9bb into main May 20, 2026
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
…ient-mode

feat(advisor): client-side mode for 3P providers (any provider, any model)
singlaamitesh pushed a commit to singlaamitesh/clawcodex that referenced this pull request Jul 7, 2026
…fficient /advisor)

Two new entries at the top of the README News section:

1. Codebase stats: 890 files / 183,768 lines (up from 894 / 177,428
   on 2026-05-16; net +6.3k lines / -4 files in five days). The file
   delta is the agent_loop.py consolidation; the line additions are
   the /advisor multi-provider rewrite + status-bar cost work.

2. /advisor mode as a token-efficient coding agent — narrates the
   PR agentforce314#181agentforce314#193 arc as one story:
   - Cheap worker (haiku-4-5) + expensive reviewer (opus-4-7) only
     at decision points ≈ 6× cheaper than opus-only on typical sessions
   - Explicit <provider>:<model> syntax (agentforce314#192)
   - Cross-provider routing verified (deepseek worker + opus advisor
     via litellm, agentforce314#182/agentforce314#184/agentforce314#192)
   - Reviewer-quality prompt (agentforce314#188) — Gaps/Risks/Do-next format
   - Live cost + token visibility in status bar (agentforce314#190/agentforce314#191/agentforce314#193)
   - /advisor slash command + dedicated TUI row (agentforce314#181)

Docs-only — no code or test changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant