Skip to content

feat(advisor): require explicit <provider>:<model> syntax (multi-provider rewrite) - #192

Merged
agentforce314 merged 1 commit into
mainfrom
advisor/explicit-provider
May 21, 2026
Merged

feat(advisor): require explicit <provider>:<model> syntax (multi-provider rewrite)#192
agentforce314 merged 1 commit into
mainfrom
advisor/explicit-provider

Conversation

@agentforce314

Copy link
Copy Markdown
Owner

Summary

Breaking change. The /advisor command no longer infers the provider from the model name. clawcodex is multi-provider — claude-opus-4-7 can live behind anthropic (direct), openai (litellm proxy), openrouter, bedrock, etc. Name-based inference silently routed to the wrong endpoint.

The user's test case surfaced this concretely: with claude-opus-4-7 configured in the openai provider entry (pointing at litellm.singula.ai) but the anthropic entry having no credits, /advisor claude-opus-4-7 would hit api.anthropic.com and 400 with credit balance too low.

New syntax

```
/advisor anthropic:claude-opus-4-7 # direct Anthropic
/advisor openai:claude-opus-4-7 # via litellm/proxy
/advisor openrouter:anthropic/claude-opus-4.1
/advisor gemini:gemini-2.5-pro
```

Splits on the first colon so model names with slashes/colons round-trip cleanly. Validates the provider key against ~/.clawcodex/config.json's providers map.

What changed

Layer Change
src/settings/types.py New advisor_provider: str = "" field.
src/state/app_state.py New AppState.advisor_provider + _on_advisor_provider_change subscription.
src/command_system/builtins.py /advisor requires <provider>:<model> syntax. New helpers _read_current_advisor_provider, _write_advisor_provider, _list_configured_providers.
src/utils/advisor.py execute_client_advisor takes advisor_provider (no inference). decide_advisor_mode requires advisor_provider. format_advisor_status shows openai:opus-4-7 (colon, matches /advisor input).
src/query/query.py Reads settings.advisor_provider and forwards to decide_advisor_mode.
src/tool_system/tools/advisor.py Refuses if either advisor_model or advisor_provider is empty.
Tests 4 files updated: all /advisor invocations rewritten, fixtures set both fields, new tests for test_rejects_missing_colon and test_rejects_unknown_provider.

Dead code removed

  • infer_provider_for_model (the model→provider prefix inference)
  • _provider_is_using_custom_endpoint (the "reuse main provider if it has a custom endpoint" heuristic)
  • TestInferProviderForModel test class
  • Removed from __all__

These were the inference layer that the rewrite explicitly drops.

Critic-driven fixes (one round, all addressed)

  • C1: partial-config status now surfaces advisor_client_mode when ON so users see stored state that would silently activate once both fields are filled.
  • C2: --client alone checks BOTH current_advisor AND current_provider before enabling.
  • C3: stale <model> syntax hint in an error message replaced with <provider>:<model>.
  • S1: status bar uses colon (anthropic:opus-4-7) matching /advisor input syntax. User can copy from bar to command verbatim.
  • S3: dropped fragile if configured guard — corrupt-config silent-skip turned into a loud error.

Breaking change migration

Existing users with settings.advisor_model = "claude-opus-4-7" (no advisor_provider set) will see:

  • Status bar: advisor: ?/opus-4-7 (inactive)
  • /advisor no-args: Advisor: not set\n(Found advisor_model='claude-opus-4-7' but no advisor_provider — clear with /advisor unset then re-run with the explicit syntax.)\nUse "/advisor <provider>:<model>" to enable, e.g.:\n /advisor anthropic:claude-opus-4-7 (direct Anthropic)\n /advisor openai:claude-opus-4-7 (via openai-compat, e.g. litellm)\n /advisor openrouter:anthropic/claude-opus-4.1

No silent failures; clear migration path.

Tests

  • 677 passed across the full advisor + adapter + TUI + helpers + smoke + headless suites.

Live verified

The user's exact scenario:

```
~/.clawcodex/config.json:
settings:
advisor_model: claude-opus-4-7
advisor_provider: openai # was missing — now required
providers.openai.base_url: https://litellm.singula.ai

clawcodex --provider openrouter --model deepseek/deepseek-v4-pro -p "..."
```

Result: deepseek worker called advisor, the advisor request went via openai/litellm (NOT direct Anthropic), opus-4-7 returned real advice — "use heapq.nlargest(k, nums) for O(n log k) time. Handle edge cases: k <= 0 returns [], k >= len(nums) returns sorted...". The previous 400-credit-balance error is gone. Files created, exit 0, num_turns=6.

🤖 Generated with Claude Code

…ider rewrite)

clawcodex is multi-provider (not Claude-specific). The previous /advisor
command silently inferred the provider from the model name's prefix —
`claude-*` always routed to anthropic, regardless of which provider
config the user actually wanted to use. That broke the realistic case
where `claude-opus-4-7` is configured in the openai provider entry
(pointing at litellm.singula.ai) while the anthropic entry has no
credits. The advisor call would hit api.anthropic.com and 400 with
'credit balance too low'.

## What landed

* New `Settings.advisor_provider` + `AppState.advisor_provider`
  fields. Both required alongside `advisor_model` for the advisor
  to be considered active.

* `/advisor` requires `<provider>:<model>` syntax. Examples:
    /advisor anthropic:claude-opus-4-7        (direct Anthropic)
    /advisor openai:claude-opus-4-7           (via litellm/proxy)
    /advisor openrouter:anthropic/claude-opus-4.1
    /advisor gemini:gemini-2.5-pro
  Splits on the first colon so model names with slashes/colons
  round-trip cleanly. Validates the provider key against
  ~/.clawcodex/config.json's `providers` map.

* `execute_client_advisor` takes `advisor_provider` and uses it
  directly — no inference, no "reuse main provider if it has a
  custom endpoint" heuristic. Empty provider → fail-fast with a
  clear error.

* `decide_advisor_mode` requires `advisor_provider`. Server-side
  needs `advisor_provider == "anthropic"` since that's the only
  path with the API beta. Anything else → client-side or INACTIVE.

* Status bar: `advisor: openai:opus-4-7 (client)` — colon-separated
  so the user can copy-paste back into /advisor verbatim.

* Dead code removed: `infer_provider_for_model` and
  `_provider_is_using_custom_endpoint` are no longer called by the
  advisor path. Also dropped from __all__ + the corresponding
  TestInferProviderForModel test class.

## Critic-driven fixes applied (single round, all addressed)

* C1: partial-config status now surfaces `advisor_client_mode`
  when ON, so the user sees stored state that would silently
  activate once both fields are filled.
* C2: `--client alone` checks both `current_advisor` AND
  `current_provider` before enabling; the half-config case is no
  longer accepted as 'I'm forcing client mode' followed by silent
  INACTIVE at runtime.
* C3: stale `/advisor <model>` syntax hint in one error message
  replaced with the new `/advisor <provider>:<model>` form.
* S1: status bar uses colon (`anthropic:opus-4-7`) instead of
  slash, matching the /advisor input syntax. The user can copy
  the status segment directly into the command.
* S3: dropped `if configured` fragile guard in /advisor provider
  validation — corrupt-config silent-skip turned into a loud error.

## Breaking change

Existing users with `settings.advisor_model = "claude-opus-4-7"`
(no `advisor_provider` set) will see the advisor as INACTIVE
until they run `/advisor anthropic:claude-opus-4-7` (or whatever
provider they actually want). The status bar shows
`advisor: ?/opus-4-7 (inactive)` and `/advisor` (no args) prints
a clear migration hint pointing at the new syntax.

## Verified

* 677 tests passed across the full advisor + adapter + TUI + helpers
  + smoke + headless suites.
* Live: with the user's actual scenario (deepseek-v4-pro worker via
  openrouter, claude-opus-4-7 advisor via openai/litellm,
  `advisor_model=claude-opus-4-7 advisor_provider=openai`), the
  advisor call SUCCEEDED via litellm with real opus-4-7 advice:
  "use heapq.nlargest(k, nums) for O(n log k) time. Handle edge
  cases: k <= 0 returns [], k >= len(nums) returns sorted...".
  The previous 400-credit-balance error is gone.

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

feat(advisor): require explicit <provider>:<model> syntax (multi-provider rewrite)
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