feat(advisor): land phases 2-4 (provider, query, command+TUI) - #181
Merged
Conversation
Stacked on #PR1. The Anthropic provider's `_build_chat_response`
projects the SDK stream into `(content, tool_uses, ...)` for the
agent loop, but drops everything else — including server-side advisor
blocks. Once the next stack layer wires activation, those blocks need
to round-trip through history so the next turn's API call sees a
matched `server_tool_use(name=advisor)` + `advisor_tool_result` pair.
- `src/providers/base.py`: adds
`ChatResponse.raw_content_blocks: Optional[list[dict]] = None`,
the channel for passthrough blocks the projector chose not to
flatten.
- `src/providers/anthropic_provider.py`: extends `_build_chat_response`
to collect blocks where `type=='advisor_tool_result'` or
`(type=='server_tool_use' and name=='advisor')`, serialize via
`block.model_dump(exclude_none=True)`, and attach to
`raw_content_blocks`. The SDK's lenient `construct_type` preserves
the original fields on unknown discriminators (verified
empirically against `anthropic==0.88.0`), so the round-trip is
faithful.
Other server tools (web_search, code_execution, etc.) are
deliberately NOT scooped up — they have their own SDK projection.
## Test plan
- [x] `tests/test_advisor_chat_response_roundtrip.py` (6 tests):
builds a real SDK ParsedMessage via `accumulate_event` for
each of the three discriminated `advisor_tool_result` content
shapes (`advisor_result`, `advisor_redacted_result`,
`advisor_tool_result_error`), the orphan case, the non-advisor
server-tool exclusion, and a mixed-block case (text + tool_use
+ advisor pair).
- [x] Pins the SDK round-trip contract — a future SDK upgrade (e.g.
Pydantic v3, stricter discriminator handler) that silently
drops extra fields on unknown discriminators would fail the
`encrypted_content` and `error_code` assertions locally.
- [x] No regressions in adjacent provider tests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Stacked on #PR2. With helpers + provider preservation in place, this
is the actual server-side parity wiring — same beta header, same
schema, same instructions, same cache-preserving order as TS
typescript/src/services/api/claude.ts.
- `src/state/app_state.py`: adds `advisor_model: str | None = None`
field + `_on_advisor_model_change` handler. The handler persists
to `settings.advisor_model` via the shared default ConfigManager
AND invalidates the settings cache, so mid-session toggles via the
reactive store reach the next API call without a process restart.
Registered in `_FIELD_HANDLERS` to satisfy the coverage contract.
- `src/query/query.py`: `_call_model_sync` now:
1. Computes `advisor_active` via a single try/except wrapper:
`is_advisor_enabled(provider)` (first-party Anthropic AND env
not disabled) AND `get_settings().advisor_model` non-empty AND
`model_supports_advisor(provider.model)` AND
`is_valid_advisor_model(candidate)`. Any exception → inactive.
2. When inactive, strips historical advisor blocks via
`strip_advisor_blocks` (the API 400s on advisor blocks without
the beta header).
3. When active, appends the advisor schema AFTER the cached tool
list (preserves the `cache_control` marker position), sets
`betas=[ADVISOR_BETA_HEADER]`, and appends
`ADVISOR_TOOL_INSTRUCTIONS` to the system prompt's tail. The
append order mirrors TS claude.ts:1395, 1411-1421 — toggling
/advisor doesn't bust the cache prefix.
4. After the response, preserves `raw_content_blocks` (from PR2)
into `assistant_blocks` so the pair survives history replay.
A stale `advisor_model` setting under a non-supporting base model is
silently ignored — never sent to the API. Mirrors the TS "skipping
advisor — base model X does not support advisor" branch.
## Test plan
- [x] `tests/test_advisor_request_wiring.py` (12 tests): full
activation matrix on a first-party Anthropic provider, plus
defensive activation (synthetic exception in the gate → turn
proceeds without advisor). Verifies the beta header attaches,
schema appends AFTER regular tools, instructions append to
both string and block-list system prompts, advisor blocks
survive in history when active. Negative cases: env disabled,
custom base_url, unsupported base model, invalid advisor
model, settings unset — none send the schema/header.
- [x] `tests/integration/test_advisor_smoke.py` (3 tests): mocked
Anthropic stream produces text + server_tool_use(advisor) +
advisor_tool_result + text — verify pair preserved across
turns. Interrupt scenario: orphan use stripped on next-turn
replay even when beta IS active (orphan strip is unconditional
because the API rejects orphans regardless of header).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Stacked on #PR3. The user-facing surfaces — top of the stack, depends
on every layer below.
## /advisor slash command
- `src/command_system/builtins.py`: `ADVISOR_COMMAND` + handler
porting typescript/src/commands/advisor.ts branches:
* no args → report set/unset/inactive
* `unset` | `off` → clear
* `<model>` → resolve via `resolve_model` + validate via
`validate_model_name` + reject non-advisor models
Writes go through the reactive AppState store when wired, otherwise
fall back to a direct settings write through the shared default
ConfigManager. Both paths invalidate the settings cache so the
next API call observes the change without a restart.
- `src/command_system/types.py`: `CommandContext` gains optional
`app_state_store` and `provider` fields (defaults `None` for
backwards compat with all existing call sites).
- `src/command_system/engine.py`: `create_command_context` accepts
the two new optional kwargs.
## TUI rendering
- `src/tui/widgets/messages/assistant_advisor.py` (NEW): dedicated
row widget mirroring typescript/src/components/messages/
AdvisorMessage.tsx — `Advising using <model>` while running,
`✓ Advisor reviewed` (collapsed) or full text (verbose) on done,
`✗ Advisor unavailable (<code>)` on error.
- `src/tui/messages.py`: `AdvisorEventMessage` (`kind=start|result`).
- `src/tui/widgets/transcript_view.py`: `append_advisor_event` mounts
/ updates the new row; `_advisor_rows` tracked separately from
generic tool rows for eviction-sweep symmetry.
- `src/tui/agent_bridge.py`: `_emit_advisor_events` scans the
conversation after each turn (cursor-advanced + stale-defended),
posts start + result events for advisor pairs. `reset_advisor_dedup`
is wired to `/clear` so the dedup set doesn't suppress legitimate
re-renders post-clear.
- `src/tui/screens/repl.py`: `on_advisor_event_message` routes to
the transcript.
- `src/tui/app.py`: passes the active provider through to the
command context; resets advisor dedup on `/clear`.
## Test plan
- [x] `tests/test_advisor_command.py` (14 tests): all 5 command
branches (no-arg unset/set/inactive, unset, off, valid model,
invalid, rejected non-advisor) × both write paths (store +
settings). Plus settings-cache invalidation round-trip via
both paths.
- [x] No regressions in 577 TUI + transcript + command-system tests.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This was referenced May 20, 2026
singlaamitesh
pushed a commit
to singlaamitesh/clawcodex
that referenced
this pull request
Jul 7, 2026
…02-04 feat(advisor): land phases 2-4 (provider, query, command+TUI)
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#181 → agentforce314#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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
main. The original stacked PRs (feat(advisor) 2/4: preserve advisor blocks in ChatResponse round-trip #177, feat(advisor) 3/4: wire activation into the query layer #178, feat(advisor) 4/4: /advisor slash command + dedicated TUI row #179) merged into each other's branches but never bubbled up tomain— this PR cherry-picks the three feature commits onto a fresh branch from currentmain.ChatResponse.raw_content_blocksopaque passthrough +AnthropicProvider._build_chat_responserecognizes advisorserver_tool_use/advisor_tool_resultblocks and round-trips them viamodel_dump._call_model_syncactivation predicate (4 ANDed gates), beta header + cache-preserving tool-schema append + system-prompt instruction injection;AppState.advisor_modelwith_on_advisor_model_changepersistence + cache invalidation./advisor <model>slash command +AssistantAdvisorMessagetranscript row +AgentBridgeadvisor-event scanning with dedup.Phase 01 is already on
mainvia #176.Test plan
Verified locally on the cherry-picked tree (
/Users/ericlee2/workspace/clawcodex/.venv/bin/pytest):tests/test_advisor_helpers.py— 32 passedtests/test_advisor_orphan_pairing.py— 6 passedtests/test_advisor_chat_response_roundtrip.py— 6 passedtests/test_advisor_command.py— 14 passedtests/test_advisor_request_wiring.py— 12 passedtests/test_app_state.py— 16 passedtests/integration/test_advisor_smoke.py— 3 passedNotes
2cdc3142(the phase-01 commit already onmain).🤖 Generated with Claude Code