-
Notifications
You must be signed in to change notification settings - Fork 104
feat(advisor): review-gate advisor backend and advisor route type #359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
1262408
295624d
196ff9a
bc455db
8dced63
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,6 +158,21 @@ Direct Rust bindings for migrated concrete processors/backends are exposed from | |
| | Response component | Plain Python/Rust object | `async process(ctx, response) -> ChatResponse` | Post-process (logging, stats) | | ||
| | `TranslationEngine` | `switchyard_rust.translation` | `async translate(ctx, request, response) -> Any` | Convert to client's wire format | | ||
|
|
||
| ### Multi-call backends | ||
|
|
||
| `AdvisorLoopBackend` (`switchyard/lib/backends/advisor_loop_backend.py`) pairs the executor with a | ||
| stronger advisor that reviews the executor's first no-tool-call turn once per session (APPROVE | ||
| returns it; REDO feeds the advisor's plan back and re-invokes the executor). The trigger is | ||
| proxy-side, so it fires even for executors that rarely call tools; advisor text goes into the | ||
| first user message, never the newest turn, so the upstream cache prefix stays stable across a | ||
| session. It is multi-call — one `call(...)` issues several upstream requests before returning one | ||
| `ChatResponse`, so "exactly one `LLMBackend` per chain" holds at the chain level only — and it | ||
| does its own stats accounting into the classifier bucket, so it must not be wrapped in | ||
| `StatsLlmBackend` (which rejects Python-only backends); the route-bundle builder injects the | ||
| accumulator through the constructor instead. Executor and advisor targets dispatch independently | ||
| on `LlmTarget.format`; `responses` is rejected at `AdvisorConfig` validation. Compose with a | ||
| `type: advisor` route (`switchyard/cli/route_bundle.py`) or an `AdvisorPresets` helper. | ||
|
|
||
| ## Project Structure | ||
|
|
||
| ``` | ||
|
|
@@ -181,6 +196,8 @@ switchyard/ | |
| │ │ ├── openai_llm_backend.py # OpenAiPassthroughBackend | ||
| │ │ ├── openai_native_backend.py # OpenAiNativeBackend | ||
| │ │ ├── anthropic_native_llm_backend.py # AnthropicNativeBackend | ||
| │ │ ├── advisor_loop_backend.py # AdvisorLoopBackend (advisor review gate) | ||
| │ │ ├── advisor_config.py # AdvisorConfig (+ advisor_prompts, advisor_presets) | ||
| │ │ ├── llm_target.py # LlmTarget, BackendFormat | ||
| │ │ ├── multi_llm_backend.py # MultiLlmBackend helpers | ||
| │ │ ├── stats_llm_backend.py # StatsLlmBackend | ||
|
|
@@ -242,7 +259,7 @@ and their transitives never appear in downstream vulnerability scans. | |
| ```bash | ||
| export OPENROUTER_API_KEY="sk-or-..." | ||
|
|
||
| # Serve the minimal Python YAML bundle (noop and passthrough only). | ||
| # Serve the minimal Python YAML bundle (noop, passthrough, and advisor routes). | ||
| switchyard serve --routes examples/route.yaml --port 4000 | ||
|
Comment on lines
+262
to
263
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Correct the route-bundle description.
🤖 Prompt for AI Agents |
||
|
|
||
| # Launch against the packaged OpenRouter deployment. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -881,9 +881,21 @@ if [[ "\${SERVER_ENABLED}" == "1" ]]; then | |
| DOCKER_RUN_ARGS=( | ||
| -d --rm | ||
| --name "\${SWITCHYARD_DOCKER_CONTAINER}" | ||
| --network "\${SWITCHYARD_DOCKER_NETWORK}" | ||
| --network-alias "\${SWITCHYARD_DOCKER_SERVICE_NAME}" | ||
| -p "127.0.0.1:$(q "${PORT}"):$(q "${PORT}")" | ||
| ) | ||
| if [[ "\${SWITCHYARD_DOCKER_NETWORK_MODE:-bridge}" == "host" ]]; then | ||
| # Host networking: for upstreams only routable from the host (VPN / | ||
| # corp-internal gateways that Docker bridge networks cannot reach). | ||
| # Pair with --harbor-server-url http://<host-ip>:<port> so task | ||
| # containers reach the server at the host address. | ||
| DOCKER_RUN_ARGS+=(--network host) | ||
|
Comment on lines
+885
to
+890
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift Require a reachable Harbor URL for host networking. When Fail fast unless The network contract is defined by 🤖 Prompt for AI Agents |
||
| else | ||
| DOCKER_RUN_ARGS+=( | ||
| --network "\${SWITCHYARD_DOCKER_NETWORK}" | ||
| --network-alias "\${SWITCHYARD_DOCKER_SERVICE_NAME}" | ||
| -p "127.0.0.1:$(q "${PORT}"):$(q "${PORT}")" | ||
| ) | ||
| fi | ||
|
Comment on lines
+885
to
+897
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Validate and document The Use an explicit The accepted-value contract should be visible in the user-facing 🤖 Prompt for AI Agents |
||
| DOCKER_RUN_ARGS+=( | ||
| -v "\${REPO_ROOT}:\${REPO_ROOT}:ro" | ||
| -v "\${RUN_DIR}:\${RUN_DIR}" | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -132,6 +132,17 @@ def route_bundle_strategy_summary(route_bundle: str, default_model: str) -> str: | |
| target = route.get("target") | ||
| model = target.get("model") if isinstance(target, _Mapping) else target | ||
| return f"passthrough: model={model or first_key}" | ||
| if route_type == "advisor": | ||
| tiers = {} | ||
| for field in ("executor", "advisor"): | ||
| tier = route.get(field) | ||
| tiers[field] = ( | ||
| tier.get("model") if isinstance(tier, _Mapping) else tier | ||
| ) | ||
| return ( | ||
| f"advisor: executor={tiers['executor']}, " | ||
| f"advisor={tiers['advisor']}" | ||
| ) | ||
|
Comment on lines
+135
to
+145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Annotate
Also note the rendered summary shows As per coding guidelines: "Use type hints throughout; code must satisfy strict mypy checking." 🐛 Proposed fix if route_type == "advisor":
- tiers = {}
+ tiers: dict[str, object] = {}
for field in ("executor", "advisor"):
tier = route.get(field)
tiers[field] = (
tier.get("model") if isinstance(tier, _Mapping) else tier
)🤖 Prompt for AI AgentsSource: Coding guidelines |
||
| except Exception: | ||
| pass | ||
| return f"route: {default_model}" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
List the rehomed advisor files explicitly.
Add separate tree entries for
advisor_prompts.pyandadvisor_presets.py. The project structure is a file map, but the current entry lists onlyadvisor_config.pyand hides two modules in a parenthetical.🤖 Prompt for AI Agents