feat: wire the llm-router into the harness and retire the legacy routing surfaces - #248
Closed
ytallo wants to merge 49 commits into
Closed
feat: wire the llm-router into the harness and retire the legacy routing surfaces#248ytallo wants to merge 49 commits into
ytallo wants to merge 49 commits into
Conversation
The engine validates the existing entry value against the schema on re-register; a fresh entry holds null until the operator first writes, so a strict object type rejected every provider re-registration.
The invocation path reports a missing function as 'function_not_found' (engine/src/engine/mod.rs); bare NOT_FOUND is the configuration worker's missing-entry code and must stay Coded, or missing config entries would flip provider availability.
Removes trait Bus / bus.rs, the SdkBus adapter / bus_sdk.rs, FakeBus, the scripted provider, and the ChannelFactory dyn seam. Handlers and stores now take iii_sdk::III directly (thin state.rs wrappers per binary-worker.md §7); TriggerEmitter implements the SDK TriggerHandler itself; channel plumbing is plain functions in channels.rs. Bus-shaped test coverage moves to the engine-backed tests/integration.rs (8 scenarios, self-skipping when no engine is available, storage-worker pattern); pure-logic unit tests are unchanged.
The router owned three custom trigger types, tracking subscribers and fanning out via per-subscriber iii.trigger — a reimplementation of the engine's built-in iii-pubsub worker (publish function + subscribe trigger type). Publish to the same three names as pubsub topics instead; subscribers bind trigger_type "subscribe" with config.topic. Payloads are delivered verbatim (no envelope) and failures stay isolated per subscriber, so behavior is unchanged. Delivery is now concurrent per subscriber (engine-side spawn) instead of sequential, and publishing no longer waits for subscriber completion; all emit sites are fire-and-forget so neither is observable. Subscriptions now live engine-side, removing the registration-replay dance on router restart. New env-gated integration test pins raw-payload delivery to a subscribe-bound probe function.
The local StreamChannelRef/ChannelDirection mirror in types/channel.rs existed because the old Bus seam barred types/ from importing iii_sdk; the seam is gone, leaving a duplicate type and a serde round-trip on every channel mint and open_sink. Use the SDK type everywhere and drop the conversions. ChatRequest/ProviderStreamInput lose their unused PartialEq derive (the SDK type does not implement it); wire shape is identical, as the deleted round-trip itself proved.
…s absent The registry-publish flow (and CI's interface-boot smoke) boots the worker against a bare 'workers: []' engine that has no iii-state worker, so the boot-time state::get died with function_not_found before any function registered and interface collection timed out. Tolerate exactly that error class in the two store loads: warn and start empty. It is safe to special-case — with no state worker a later persist can't overwrite the stored snapshot either. Any other state::get failure still fails the boot. The function_not_found matcher moves from chat.rs to types/errors.rs for reuse, and a new env-gated integration test boots the router against a bare engine and asserts the read surface answers.
…able, provider guide
… failures Refactor error handling in the ChatPipeline to guarantee that a terminal error frame is sent to the sink during pre-stream failures. This change addresses issues where consumers may not receive a terminal frame, particularly when routing to an unknown provider or when invalid input is provided. Additionally, introduce a new test to validate that exactly one terminal error frame is emitted in such scenarios.
synthesize_error stamped every terminal it built as transient — a retryable kind. Correct for the mid-stream no-terminal/idle path, but the pre-stream failures (invalid request, unrouted model, unknown provider, structured-output gate) are permanent: a streaming consumer inspecting error_kind on the frame would retry requests that can never succeed. error_kind is now a caller choice; pre-stream sites pass Permanent, the mid-stream synthesis keeps Transient.
…wned provider Registration flips available back to true, but the register handler only published op:register — subscribers tracking the op:available/unavailable transitions stayed stuck on the prior unavailable. upsert now reports whether the registration recovered a downed provider (decided under the records lock) and the handler emits an explicit op:available event on that transition. Fresh registers and already-up re-registers emit nothing extra.
…rs cannot hang the caller router::complete drained its internal channel to EOF before consuming the pipeline result. A pipeline error that never wrote a frame leaves the channel without an EOF (a zero-write close does not propagate), so the drain blocked for its full 600s budget — reachable by simply killing a provider worker: dispatch fails function_not_found, run returns ProviderUnavailable, the caller times out instead of seeing the typed error. Drive the drain and the pipeline concurrently: a pipeline Err propagates to the caller immediately, for every current and future error path; on Ok the remaining in-flight frames are drained on a short budget instead of the streaming one. Engine-backed regression test registers a provider declaration with no worker behind it and asserts router::complete answers with router/provider_unavailable fast.
…n_key Absent options serialized as JSON null, which provider-side schemas reject where a string or array is expected; every default turn would fail validation. The payload builder now omits absent keys and always carries resolution_key = request_id so providers can dedupe per-turn credential resolution across retry attempts.
Consumers that need the provider before streaming (prompt selection, provisioning metadata) call router::route and pin the result as the explicit provider on router::chat, so preview and execution can never diverge. Same decide(), same inputs, same typed error codes as the chat pipeline.
…, catalog getters provider-resolve.ts becomes the llm-router provider-protocol client: token-persisting registerWithRouter (iii-state scope llm-provider-registration, capped-backoff retry), router::ready re-declare subscription, and token-gated resolveProviderViaRouter. The provider stream schema tolerates null options, accepts the router's resolved max_output_tokens, and takes string|number resolution keys. Catalog reads (getCatalogModel, fetchModelLimit) and the discovery reconcile move to router::models::get/reconcile.
One-time idempotent boot migration copies the harness entry's providers block into the router-owned llm-router entry and seeds routing parity with the old local decide() (anthropic default; gpt-/o<digit>- to openai, kimi-/moonshot-v1- to kimi). The harness entry is re-registered permissions-only; paste-a-key reactivity is the router's configuration trigger now.
All five providers self-declare via router::provider::register (re-declaring on router::ready), resolve credentials via the token-gated resolve, reconcile discovery into the router catalog, and honor the router's resolved max_output_tokens as the clamp override. The Phase-1 provider::<id>::complete functions are gone — router::complete drives the stream function.
Provisioning previews the decision once via router::route, pins the routed
provider on the run request (prompt family becomes a pure provider switch),
and resolves model metadata against the router catalog. The streaming step
calls router::chat with a deterministic request_id
(${session_id}:${started_at_ms}); run::abort fires a best-effort
router::abort with the same id so the upstream actually stops. The
close-without-terminal synthetic stays as defense-in-depth. Compaction
summarises via router::complete. Outer trigger budgets are 320s — they must
exceed the router's 300s stream budget.
The picker lists via router::models::list and router::provider::list, the provider-credentials deep links open the llm-router configuration entry, and the ui::models::changed fanout rebinds from the models state-scope trigger to the router::models::changed pubsub topic.
Deleted: the local provider-router decide() library, the provider registry (harness::provider::register/resolve/list) and its refresh-on-config bridge, the models-catalog module (models::list/get/supports/reconcile + scope 'models'), per-provider Phase-1 complete functions, and the compaction stream collector. The catalog Model type moves to types/model.ts, wire- aligned with the router (provider-side extras are optional — the router catalog doesn't persist them). The harness entry is permissions-only; the harness worker now depends on llm-router instead of models-catalog.
Root permission rules deny the router's spend/credential/catalog-write surface to agents and allow the read surface (models list/get/supports, provider list); the dead harness::provider and Phase-1 router::stream_assistant lines are gone, and the legacy models::* allows are retargeted. The harness worker manifest depends on llm-router; the router README documents router::route, the env-fallback process boundary, and token-loss recovery.
Provider, catalog, orchestrator, and compaction suites mock router::provider::resolve / router::models::get / router::models::reconcile / router::complete (envelope payloads, token gating, no channel plumbing for the summariser). New coverage: provider stream schema null tolerance and router-built payloads, registration-token persistence/retry/terminal rejection, ready-topic re-declare binding, and config-migration idempotency and seeding. Suites for the deleted registry, refresh-on-config, models-catalog, and local decide() are gone with their subjects.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedToo many files! This PR contains 167 files, which is 17 over the limit of 150. To get a review, narrow the scope: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (167)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
skill-check — worker0 verified, 16 skipped (no docs/).
Four for four. Nicely done. |
7 tasks
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
1 participant
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
The Rust
llm-routerworker becomes the single front door for every LLM call: routing, provider registry, credential resolution, model catalog, streaming relay with typed errors/retries/timeouts, and abort. This PR lands the worker (supersedes #241), cuts the harness, providers, and console over to it, and removes the legacy surfaces it replaces.Router (Rust)
provider::<id>::streampayload omits absent options instead of serializing them asnull(provider-side schemas rejectnullwhere a string or array is expected), and always carriesresolution_key = request_idso providers can dedupe per-turn credential resolution across retry attempts.router::routefunction: a read-only routing preview ({model, provider?} → {provider, candidates}) over the samedecide()and error codes asrouter::chat. Consumers that need the provider before streaming pin the preview as the explicitprovideron the chat call, so preview and execution can never diverge.Harness cutover
router::chatwith a deterministicrequest_id(${session_id}:${started_at_ms});run::abortfires a best-effortrouter::abortwith the same id, so the upstream actually stops generating. The close-without-terminal synthetic stays as defense-in-depth behind the router's terminal-frame guarantee. Outer trigger budgets are 320s, above the router's 300s stream budget.router::routecall per turn serves prompt-family selection and model-metadata resolution; the routed provider is persisted on the run request.router::provider::registerwith the registration token persisted in iii-state (re-register without it is rejected), re-declare on therouter::readyevent, resolve credentials through the token-gatedrouter::provider::resolve, reconcile discovery into the router catalog, and honor the router's resolvedmax_output_tokens. The Phase-1provider::<id>::completefunctions are gone —router::completedrives the stream function, including the compaction summariser.harnessconfiguration entry to the router-ownedllm-routerentry via an idempotent boot-time migration that also seeds routing parity with the old localdecide()(anthropic default;gpt-/o<digit>-→ openai,kimi-/moonshot-v1-→ kimi). Theharnessentry is permissions-only now.router::models::list/router::provider::list, provider-credential deep links open thellm-routerentry, and theui::models::changedfanout rebinds to therouter::models::changedpubsub topic.Removed
turn-orchestrator/provider-router.ts(localdecide()), theharness::provider::register/resolve/listregistry and its refresh-on-config bridge, themodels-catalogmodule (models::list/get/supports/reconcile+ state scopemodels), per-providercompletefunctions, and the compaction stream collector. The catalogModeltype moves totypes/model.ts, wire-aligned with the router. Agent permission rules deny the router's spend/credential/catalog-write surface and allow the read surface.Behavior notes
ANTHROPIC_API_KEY, …) now resolves in the llm-router process, not the harness — launch the router with those variables or paste keys into thellm-routerentry (documented in the router README, with the token-loss recovery procedure).request_id(string) instead of the run start time.Test plan
cargo test(54) andcargo test --test integration(12, engine-backed: relay, cancellation, abort, retry, token gate, paste-a-key, route/chat parity)pnpm vitest runinharness/— 119 files, 1359 tests, including new coverage for schema null-tolerance, registration-token persistence/retry/terminal rejection, ready-topic re-declare, and config-migration idempotencypnpm vitest run+tsc -b --noEmitinconsole/web— 654 testsgpt-*heuristic, local catalog-owner), mid-stream abort,/compact, paste-a-key, router/harness restarts