Skip to content

feat(providers): endpoint failover and provider path close-out - #287

Merged
0xKT merged 78 commits into
mainfrom
refactor/provider_module_redesign
Aug 10, 2026
Merged

feat(providers): endpoint failover and provider path close-out#287
0xKT merged 78 commits into
mainfrom
refactor/provider_module_redesign

Conversation

@0xKT

@0xKT 0xKT commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Closes out the provider path in one PR: the provider module redesign, every
open issue and backlog item that lives on the identity / connection /
routing / capability axes, and multi-endpoint failover as a new feature.
78 commits, one problem per commit, rebased onto current main.

Redesign (base): four provider decisions were implemented outside
raven/providers/ and had drifted copies -- wire form, credential grammar,
pin resolution, price/window ladder, cache dialect. Each now has one owning
module the surfaces call.

Fixes on that base, most visible first:

  • The context window walks one ladder (explicit config > the model's real
    window > documented fallback). Previously the config default 65536 fed
    trimming and budgets raw, so a 200k-window model lost two thirds of its
    context every turn; an unresolvable window now renders the gauge's empty
    state instead of a number that is nobody's. Construction and /model
    switches resolve without touching the network, and a switch parked behind
    a running turn re-resolves the window at adoption, not at the RPC call.
  • A provider without real streaming (azure, codex) no longer renders
    upstream errors as normal assistant text: the terminal stream delta
    carries the classification and joins the same recovery path the
    non-streaming call uses. Both classify their non-200 from the live status
    code (shared ProviderHTTPError) instead of regex-guessing the rendered
    text, which also removes the bare "404" substring match that misfiled a
    400 whose body happened to embed one.
  • Codex SSE failures keep their structured code, so an overloaded backend
    is retried instead of classified unknown.
  • Orphan </think> recovery (backends launched without their reasoning
    parser) is gated to the backend shapes that produce it, so ordinary
    content mentioning the tag is never cut.
  • Fallback hops are vetoed when both identities are certain and disagree
    (previously a cross-vendor hop went out under the wrong key, or silently
    to the wrong backend on a shared model name); the knn path dispatches
    each hop to its own endpoint and inherits model_overrides through the
    rotor.
  • User model_overrides win over shipped extra_body defaults, and the merge
    no longer drops user keys behind a gateway.
  • The credential gate, the reader and the builder answer one way: a spec's
    shipped default address satisfies the gate exactly when the reader would
    serve it (custom runs on a bare key again, azure still demands its
    address), endpoint entries inherit the flat api_base/extra_headers per
    field, and every display face -- provider list, endpoint list, the TUI
    picker -- reports the same resolved view, secrets redacted (extra_headers
    values included, on the flat section field too).
  • The vision probe stops joining operator-chosen names against the vendor
    catalog: it reads through the TUI's lazy proxy to see the Azure
    transport, treats an explicit-selection gateway (custom) as
    caller-chosen, and the background catalog warm is no longer suppressed
    for the life of the process by a stale on-disk table.
  • Skill forge rewriter/gate follow the configured agent model (the only
    auxiliary LLM calls that did not).
  • The onboarding wizard refuses the six litellm vendors a bare API key
    cannot configure, with the actual requirement named, instead of writing
    a section that 401s forever.
  • The OAuth handoff replays a signal swallowed mid-login, so a SIGHUP no
    longer leaves a headless TUI.

New feature -- multi-endpoint failover (several accounts on one vendor):

  • providers.<name>.endpoints (label / apiKey / apiBase / extraHeaders)
    with endpointStrategy: sticky | round_robin; the three credential
    spellings (explicit list, Gemini api_key_list, flat fields) resolve
    through one reader with strict precedence and no key merging.
  • EndpointRotorProvider rotates and fails over with per-endpoint cooldown
    (30s doubling to 300s, process-local state); auth failures rotate --
    another account's key is exactly what a dead key needs -- while
    endpoint-agnostic failures return immediately. Streams rotate only
    before the first delta, so tokens are never replayed.
  • Managed from raven provider endpoint add|remove|list and the TUI model
    picker; the session footer names the active endpoint. Write faces refuse
    a keyless endpoint for key-credential providers (local deployments keep
    their legitimate keyless shape); invalid sections fail loudly instead of
    being read as empty and overwritten.
  • Verified live twice: the rotor directly, and the full stack from a
    config file through make_provider (a dead key 401s, cools, fails over,
    answers). The first live run caught the auth-rotation gap the mocks
    could not.

Also in this PR: the onboarding wizard split (5069 -> 3000 lines, pure
moves with every migrated monkeypatch target mutation-checked), CONTEXT.md
terms (Provider Endpoint, window ladder), benchmark alignment (pinchbench
now prices through the shared ladder instead of a private drifted copy),
and the model picker reads the config twice per open instead of twice per
provider row.

Reviewed adversarially across three external rounds and four internal
panel rounds on two model families: 24 before-merge findings raised in
total, every one either fixed with a mutation-verified test or refuted
with executed evidence (one reviewer finding was withdrawn after a
945-case main-parity sweep); final verdicts RATIFY. The rebase also
adopted the review notes left on #282/#285 that landed on this code:
image-capability verdicts are invalidated on a provider switch, and a
parked switch logs its park and its adoption.

Known follow-ups, named in the review thread (issues to follow): the
fallback routing loop contradicts the registry's explicit-selection note
for custom (pre-existing on main), the remaining bare status substrings
in classify_error (429/5xx, pre-existing), per-hop identity rebuilding for
fallback chains, and the pre-existing SessionInfo shape mismatch.

Type

  • Feature
  • Fix
  • Docs
  • CI / tooling
  • Refactor
  • Other

Verification

  • uv run pytest tests/ -q -- 6128 passed, 33 skipped, run after the
    rebase onto current main; the provider-path test files additionally
    re-run under an empty HOME with identical results.
  • make lint-python clean; commit messages pass commitlint and
    scripts/check_commit_messages.py across all 78 commits.
  • cd ui-tui && npm run lint && npx tsc --noEmit && npx vitest run -- 86
    files, 982 tests passed; npm run gen:rpc -- --check in sync.
  • Live probes (OpenRouter, tiny max_tokens): rotor failover and full-stack
    assembly failover, both passing; logs kept locally.
  • Fixes are pinned by deletion mutations that turn a named test red;
    review rounds ran 22 such mutations and the two survivors were
    themselves fixed (one dead guard deleted, one vacuous test replaced).

Risk

  • Security impact considered
  • Backward compatibility considered
  • Rollback path is clear for risky changes

User-visible behavior changes: contextWindowTokens unset now resolves to
the model's real window (explicit values are fully respected and no longer
overridden); the context gauge shows an empty state when the window is
unknown; azure/codex upstream errors surface as errors instead of
assistant text; the wizard refuses six key-only-unconfigurable vendors
with the real requirement named; custom with only an apiKey starts again
(as on main) and the picker no longer demands an address the gate does
not; key-credential providers refuse keyless endpoints at write time and
endpoint add --api-key becomes optional for local deployments; endpoint
keys and extra_headers values are redacted in provider get/list and
over RPC; a model switch logs when it parks behind a running turn and when
it adopts. Rollback is a straight revert of the squash commit; no data
migration is involved (config additions are opt-in fields).

Related Issues

Fixes #124, fixes #234, fixes #155, fixes #152, fixes #254, fixes #151,
fixes #143, fixes #144, fixes #197. References #281 (not reproducible on
current or reported code; the api_key forwarding it suspected is now pinned
by regression tests), #119 (already fixed by #116; remaining item is the
installer redirect, not provider code).

@gloryfromca

Copy link
Copy Markdown
Contributor

Adversarial review: 11 findings, 4 of them merge blockers

The module redesign holds up well -- the seven owning modules plus the
source-scanning invariants in test_provider_resolution_invariants.py are a
real structural win, and the fix cluster checks out against the diff. What
follows is what an independent adversarial pass found on top of the three
rounds already recorded in the description.

Every item below was verified by executing code against the PR head, not by
reading alone; the repro is given where it is short. Findings that did not
survive verification, and a set of lower-confidence candidates, are excluded.

Merge blockers

1. raven/config/schema.py:255 -- the headline window feature never
activates for existing installs.

contextWindowTokens flipped from 65536 to None with no migration. The
pre-PR bootstrap ran save_config(load_config()), and that model_dump(by_alias=True)
carried no exclude_defaults, so every install onboarded before this PR has a
literal contextWindowTokens: 65536 on disk. After upgrade that is
indistinguishable from a deliberate pin: _context_window_explicit = bool(65536)
(raven/agent/loop/main.py:384), effective_context_window is bypassed, and
refresh_context_window() early-returns. A user on a 200k-window model keeps a
65536 budget and a 65536 gauge forever. Only fresh installs get the feature the
PR is named for. A one-shot migration in _migrate_config (drop the value when
it equals the old default) is the obvious fix, with the acknowledged cost of
clobbering anyone who genuinely chose 65536.

2. raven/providers/auth.py:148 -- credentials gate passes while requests go
out with an empty key.

_present checks the flat api_key first and only then descends into
endpoints, but provider_endpoints gives a non-empty endpoints list
absolute precedence over the flat fields. So the two disagree:

# config: {"providers": {"openrouter": {"apiKey": "sk-or-WORKING"}}}
add_provider_endpoint("openrouter", label="eu", api_base="https://eu.example/v1")
credential_status("openrouter", section).ok   # True  -> "openrouter is configured"
provider_endpoints(section)                   # [ResolvedEndpoint(label='eu', api_key='')]

make_provider's single-endpoint branch (raven/cli/_helpers.py:181) then
builds LiteLLMProvider(api_key="") and every request 401s, while startup,
provider list and routing all report the section healthy. Reachable from the
TUI add-endpoint screen (label is the only required field) and from
provider endpoint add --api-key "". The stop keyless endpoints reading as set commit fixed the adjacent display case but not this combination.

3. raven/providers/base.py:472 -- bare "404" substring misclassifies
unrelated errors.

Executed against classify_error:

message category
Error: retry after 1404ms model_unavailable, should_fallback=True
upstream error id=req_a404bc7f model_unavailable, should_fallback=True
invalid JSON at char 4041 model_unavailable, should_fallback=True

Each one silently burns a fallback model and puts a healthy rotor endpoint into
a 30-300s cooldown for an error no model or endpoint swap can fix. Note that
this PR documents the exact hazard one module over --
raven/providers/prompt_cache.py:108-112 uses re.compile(r"\b400\b") with a
comment explaining that the bare substring matched the 400 inside
retry after 1400ms -- and then ships the unbounded form here. A word-boundary
regex, or better, classifying at the point the live status code still exists in
the Azure non-200 branch, closes it.

4. raven/providers/prompt_cache.py:61 -- Bedrock Claude silently loses
prompt caching.

The deleted _supports_cache_control was find_by_model(model) or find_by_keywords(model),
and its comment named the case it protected: an id routed through a vendor
carrying no spec, bedrock/anthropic.claude-.... accepts_cache_control drops
that fallback on the addressed axis:

find_by_name("bedrock")                                        # None
find_by_model("bedrock/anthropic.claude-sonnet-4-20250514")    # None
# -> addressed is None -> return False before the family axis is consulted
# old expression matched the anthropic spec -> True

Bedrock is a live path (raven/providers/auth.py:219 declares ambient AWS
credentials; _helpers.py bypasses the key gate and falls through to LiteLLM),
so those users lose cached-read pricing with no error and no log.

Should fix

5. raven/tui_rpc/methods/setup.py:96 -- camelCase provider keys skip the
configured check.
The gate iterates raw config keys and resolves each through
ProvidersConfig.get, which cannot resolve the camelCase spelling of a
multi-word provider. Verified: for
{"providers": {"azureOpenai": {"apiKey": "k", "apiBase": "https://x"}}},
model_validate folds the data into azure_openai, but providers.get("azureOpenai")
returns None while get("azure_openai") resolves. Since that is the spelling
save_config itself emits, a fully configured Azure or github_copilot install
is parked on the "setup required" panel.

6. raven/agent/loop/main.py:636 -- /model switch leaves the trimmer on the
old window.
config.py:358-361 mutates the live loop in place and never
rebuilds the engine; HistoryTrimmer captured the window by value at
construction and trim computes max_prompt from its own copy
(history_trimmer.py:195). Switching from a 262k model to an 8k one keeps the
trimmer admitting 262k of history, so every turn 400s on context overflow until
restart. Worth noting finding 1 currently masks this: for upgraders the refresh
is a no-op anyway, so both layers stay stale together and agree.

7. raven/config/update_providers.py:1030 -- provider test ignores
endpoints and api_key_list.
The probe reads cfg.get("api_key") with no
endpoints descent, so a section configured through the new endpoint commands
reports {status: "not_configured", error: "api_key is empty"} while
credential_status(...).ok is True and the runtime serves requests fine. This
is the multi-gate divergence the PR set out to end, reappearing on the PR's own
new feature; it also poisons onboarding's _verify_provider failure submenu.

8. raven/providers/model_catalog_cache.py:55 -- malformed cache raises into
the cost path.
raw.get("version") sits outside the try, so a file whose
top-level JSON is valid but not a dict ([], null, 42) raises
AttributeError. Verified to propagate through resolve_context_window and
token_rates for any model absent from LiteLLM's table -- i.e. into
AgentLoop.__init__ and the per-call cost path, which the function's own
docstring promises never happens.

9. raven/providers/litellm_provider.py:246 -- knn routing endpoints get
orphan-think stripping.
emits_unparsed_reasoning treats the custom spec as
"the self-hosted inference server this normalization exists for", while
per_model_provider._endpoint_provider documents provider_name="custom" as
"the generic OpenAI-compatible gateway spec" and uses it for every routing
endpoint -- including ones fronting hosted vendors. Consequence, executed:

in : "I checked the logs. The culprit is a stray </think> in the template."
out: " in the template."          # everything before the tag moved to reasoning

One spec name is carrying two incompatible meanings; the gate needs an axis
that separates "OpenAI-compatible wire" from "parser-less backend".

10. raven/agent/loop/main.py:1790 and raven/tui_rpc/methods/session.py:125
-- synchronous 10s HTTP fetch on the event loop.
Both call
resolve_context_window with allow_fetch defaulting to True from async code.
For an openrouter/ id missing from LiteLLM's table with both cache tiers past
the 1h TTL, rates._fetch_openrouter_models runs httpx.Client(timeout=10.0).get()
inside the running loop, stalling every concurrent stream and RPC. This PR
passes allow_fetch=False at main.py:389 and in refresh_context_window for
exactly this reason; these two consumers were not updated. The rationale in the
docstring -- that the path "already runs inside an await" -- does not make a
synchronous socket call non-blocking.

11. raven/agent/loop/main.py:389 -- construction now imports litellm.
Verified: litellm is absent from sys.modules before
effective_context_window("gpt-4o", None, allow_fetch=False) and present after.
On a fresh install raven tui builds a LazyProvider specifically to defer that
import, then blocks the main thread on it in the very next constructor call.
allow_fetch=False avoids the network but not the import.

Not included

Three lower-severity items (pinchbench pinning 65_536 and so opting out of the
new ladder; benchmarks/clawbench/stream.py:101 importing a _make_provider
that does not exist, which makes this PR's edit to that constructor unreachable;
three added test comments citing issue numbers against AGENTS.md 1.1) and a set
of candidates that did not complete adversarial verification -- rotor retry
depth on auth failures, the endpoint write path not enforcing the OAuth/Azure
refusal that make_provider enforces, PerModelProvider's missing per-hop cache
strip, and catalog._vendor_id diverging from wire.merge_key. Happy to write
those up if useful.

@0xKT

0xKT commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Thank you -- this is an exceptional review. Every blocker reproduced exactly as
written; nothing had to be argued down. All eleven findings are addressed in
five commits on the branch, except finding 1, which we are deliberately
resolving as a documented stance rather than code (reasoning below).

Fixed, with the finding's own repro turned into a regression test:

  • 2 (gate vs reader precedence) -- _present now mirrors
    provider_endpoints exactly: endpoints set means the flat fields are
    ignored for the gate too. Your flat-key-plus-keyless-endpoint shape is a
    test, red under the old logic. Same commit teaches provider test to probe
    the resolved endpoint instead of the flat field (your finding 7), and fixes
    the camelCase setup gate (finding 5) by iterating the validated instance's
    field names.
  • 3 (bare "404") -- token-boundary regex, same fix as prompt_cache's
    \b400\b; your three collision strings are the test cases.
  • 4 (Bedrock caching) -- an address that resolves to no spec now falls
    through to find_by_keywords, restoring the pre-redesign coverage for
    bedrock/anthropic.claude-...; a resolved spec stays exactly as strict as
    the redesign made it, and suppress() still learns any wrong guess.
  • 9 (knn endpoints) -- LiteLLMProvider takes an explicit
    unparsed_reasoning override and the router passes False; your bisected
    sentence is the regression test.
  • 10 (sync fetch on the loop) -- both remaining inline resolutions moved
    to asyncio.to_thread.
  • 11 (construction imports litellm) -- allow_fetch=False now also means
    "do not import litellm on this caller's behalf"; LazyProvider gained an
    on_built callback (double-checked against the prewarm race) that re-walks
    the ladder once the background import lands, so startup stays fast and the
    window still converges. Verified with a subprocess sys.modules test.
  • 6 (stale trimmer) -- with 1 unmasked this became live, so it is fixed
    rather than deferred: refresh_context_window now cascades into the
    curator's trimmer and the consolidator.
  • 8 (cache read raises) -- isinstance guard; []/null files are a miss.
  • Minors -- clawbench's dead _make_provider import repointed at the
    symbol that exists, and the issue-number comments this branch added to
    tests are gone. pinchbench's literal 65_536 stays: a benchmark pinning
    its window explicitly is a legitimate choice, now that pinning actually
    means pinning.

Finding 1 (written-out 65536 on upgraded installs) -- acknowledged, and
resolved as a stance rather than a migration.
The project's standing rule
for config evolution is: when re-setting is idempotent and recovers the
behavior, retire the old value's effect instead of patching it in a
migration -- migrations are reserved for states raven upgrade cannot
recover from. Deleting the line (or setting it to null) enables per-model
resolution immediately, so this qualifies. You are right that this one fails
silently rather than loudly, which is why it is called out in the release
notes / upgrade guide rather than left to be discovered: existing installs
whose config carries contextWindowTokens: 65536 keep that as an explicit
pin until the line is removed. We accept that trade deliberately -- silently
rewriting a value the user can see in their own config file is the worse
failure mode.

The four candidates you did not finish verifying -- rotor retry depth on
auth, write-path OAuth refusal, per-hop cache strip in PerModelProvider, and
catalog._vendor_id vs wire.merge_key -- we would gladly take as a
follow-up list if you get to them.

@gloryfromca

Copy link
Copy Markdown
Contributor

Follow-up: fixes re-verified, plus the four candidates finished

I re-ran the original repros against 78e7a3b rather than reading the commits.
All ten code fixes hold: the four 404 collision strings now classify
unknown while a real 404 body still resolves model_unavailable; the
flat-key-plus-keyless-endpoint shape now reports not-configured with the
reader's own reason; azureOpenai is detected; bedrock/anthropic.claude-...
is back to True; []/null/42 cache files are a miss; both async sites are
on to_thread; the router passes unparsed_reasoning=False; and
refresh_context_window cascades into the trimmer and consolidator.

The import_litellm fix is better than what I asked for -- a cold process
returns the 65536 fallback and converges through on_built, but a warm one
(litellm already in sys.modules) answers 128000 immediately, so the fast
path is preserved without giving up accuracy. tests/ is 5786 passed, 43
skipped, 1 failed; the failure is test_cli_theme.py::test_bold_accent_renders_styled_not_bare,
which fails identically at the merge base, so it is not yours.

On finding 1, one option that fits your stance without rewriting anyone's
file: keep the pin, but warn once at startup that contextWindowTokens: 65536
is pinning the window and removing the line enables resolution. That turns the
silent failure you already identified into a loud one while leaving the user's
config untouched.

The four candidates, finished

Rotor retry depth on auth -- REFUTED, no action needed. My concern was that
each endpoint would run a full ladder with backoff before rotating, making four
dead keys cost minutes. It does not: the inner ladder returns immediately on a
non-retryable classification, and auth is non-retryable. Measured with four
auth-failing endpoints:

elapsed 0.09s | 4 inner chat() calls | {e0: 1, e1: 1, e2: 1, e3: 1} | final category = auth

One endpoint, one attempt, then rotate. The only residue is that the caller
sees the last endpoint's error with nothing saying three others were tried,
which is a log line at most.

Write path does not enforce the refusal the factory does -- CONFIRMED, minor.
add_provider_endpoint accepts endpoints on azure_openai, openai_codex,
minimax_global and github_copilot; make_provider then refuses to start.
The saving grace is that the refusal is clean and actionable
(azure_openai does not support multiple endpoints -- remove the 'endpoints' field from its config), not a traceback, so this is "you can write a config
that will not boot" rather than a broken runtime. Sharing one predicate
(spec.client != "" or spec.is_oauth) between the factory and the two write
paths would refuse at write time instead, and would also cover the next
provider that gains a dedicated client.

PerModelProvider per-hop strip and can_serve -- CONFIRMED, narrow. The
loop still omits both guards the base loop performs per hop. Reachability is
tighter than I first thought: a hop routed to a custom endpoint is a
LiteLLMProvider, which strips at send time via its own
_supports_cache_control, so the marks never leave. The gap is the hop that
lands on self._fallback when that fallback is not a LiteLLM client --
AzureOpenAIProvider contains no reference to cache_control or
prompt_cache at all, so an Anthropic-family primary whose marks were placed
upstream reaches Azure unstripped. Needs knn routing plus azure/codex as the
base provider, so it is narrow, but the two-loop divergence is what makes it
possible to miss.

catalog._vendor_id vs wire.merge_key -- CONFIRMED, display-only. They do
diverge, on spelling normalization: merge_key canonicalizes the provider and
compares the head against normalize_provider_name(provider), while
_vendor_id compares against the raw provider string.

provider      model                _vendor_id            merge_key
nano_gpt      nano-gpt/gpt-4o      gpt-4o                nano_gpt::gpt-4o
nano-gpt      nano_gpt/gpt-4o      nano_gpt/gpt-4o       nano_gpt::gpt-4o   <-- diverges

The mismatched spelling makes the snapshot lookup miss, so the row degrades to
an id-only label with no cost. Since catalog.py is deliberately display-only
this shapes no request; one normalize_provider_name call closes it.

One more, outside that list

The addressed_to axis still ignores an auto-detected gateway when the caller
passes no provider_name, and your finding-4 fix does not reach it -- that fix
restored the too-strict direction (address resolves to no spec), while this is
the too-lax one (the address resolves to the model id's vendor while the wire
is something else). Verified at 78e7a3b with model
anthropic/claude-sonnet-4-20250514:

construction detected gateway pre-redesign now
no api_base none True True
api_base=https://aihubmix.com/v1 aihubmix False True
api_base=https://openrouter.ai/api/v1 openrouter True True

The registry says aihubmix does not support prompt caching; the old
_supports_cache_control consulted self._gateway first and stripped, the new
one consults find_by_model(model) and sends. Two production constructors hit
this because they pass api_base and no provider_name --
raven/evolver/launch/models.py:47 (the evolver's driver/design/verdict
models, where api_base is a documented spec key) and
raven/cli/_proactive_stack.py:293 (the sentinel planner, whose
evaluator_base_url exists precisely to point at another address). Both are
named in emits_unparsed_reasoning's docstring as no-provider_name
constructors, so the shape was already on your radar for the reasoning gate;
the cache gate just did not get the same treatment. Falling back to
self._gateway when addressed_to is empty would restore it.

I have not verified what aihubmix does with the field on the wire -- it may
400 (recovered by suppress() after one wasted call per model per process) or
ignore it silently -- so I would not call this a blocker.

@gloryfromca

Copy link
Copy Markdown
Contributor

Second pass, deliberately from a different angle than the first: instead of
re-walking the config/gate/classification axes, I read the runtime assembly
(rotor concurrency, provider delegation completeness, the routing wrapper, the
lazy/prewarm seam) and the two write surfaces against each other.

All eleven earlier findings reproduce as closed -- I re-ran each finding's own
repro against 78e7a3b and none survived. The new provider suites pass (214
tests in the ones I ran), so everything below is a gap the tests do not cover.

What did not close is the shape of the problem: the gate, the reader and the
builder still answer the connection question in three places, and two of the
four blockers below are that same divergence landing on this PR's own new
feature.

Must fix

1. raven/providers/per_model_provider.py:113 -- fallback_models=[] disables
two guards, not one.

Each hop is dispatched with an empty chain, so inside the hop's own
chat_with_retry the loop index is always 0, and base.py gates both of its
per-hop protections on if idx and ...:

  • base.py:664 -- the new cross-vendor can_serve veto never fires. With knn
    routing and a cross-vendor fallback_models entry, the unrouted hop goes to
    self._fallback and one vendor's model id is posted under another vendor's
    key. This is the exact case the veto was added for.
  • base.py:675 -- prompt_cache.strip never runs either. This is the more
    expensive half: the breakpoints placed for hop 0's vendor travel to hop 1
    unchanged, and the comment three lines above the gate names the outcome
    ("sending Anthropic's markers on to Gemini is what doubled a prompt"). It is
    billed, not refused, so nothing surfaces.

PerModelProvider does not strip anywhere else, so with knn routing plus any
fallback chain there is no strip in the process at all. This is the
"PerModelProvider's missing per-hop cache strip" candidate from the earlier
not-included list; it is confirmed, and it is a billing bug rather than a
missing veto.

2. raven/providers/auth.py:238 -- api_base became mandatory for
providers.custom, which ships a default one.

Routing requires_api_base through the new gate makes _ADDRESS a hard
requirement, but custom also declares default_api_base and
get_api_base still returns it. Reproduced:
{"providers":{"custom":{"apiKey":"sk-local"}}} builds a provider on main
and raises MissingCredentialsError: custom needs an address on this branch.
An install reached through the shipped default address stops starting.

3. raven/providers/auth.py:148 -- the gate and the builder disagree about
whether endpoints replaces the flat fields.

_present lets a non-empty endpoints list override the flat api_base,
while cli/_helpers.py:166 (make_inner) falls back to
config.get_api_base(model). Reproduced with endpoints sharing one
section-level apiBase -- the shape provider endpoint add produces, since
--api-base is optional: the endpoints resolve to a working base and are
refused at startup. That makes multi-endpoint failover unreachable for every
requires_api_base / is_local provider unless each endpoint repeats the URL.

The same line does it for headers:
extra_headers=ep.extra_headers or (p.extra_headers if p else None) merges the
flat field back in, while endpoints.py's module docstring states that
endpoints set means "the flat fields and api_key_list are both ignored
outright". So the gate, the reader's documented contract, and the builder are
three answers, not one -- worth fixing as one decision rather than per field.

4. raven/providers/base.py:474 -- _STATUS_404 is a new text match, not a
narrowed one.

main's 404 bucket had no "404" substring check, so the comment describing
this as "the same boundary fix as _STATUS_400" describes narrowing something
that never existed here -- the regex widened the bucket rather than tightening
it. And it precedes the 400 bucket, so a swallowed-into-content 400 whose body
quotes a standalone 404 (an upstream body echoing a path, a nested error
payload) classifies model_unavailable + should_fallback: it burns a
fallback model and puts a healthy rotor endpoint into a 30-300s cooldown for an
error no model or endpoint swap can fix.

Should fix

5. ui-tui/src/components/modelPicker.tsx:553 -- the two write surfaces still
disagree about a keyless endpoint.

Add-endpoint validates only label; a blank key is omitted and
ModelAddEndpointParams.api_key defaults to "", so a keyless endpoint is
persisted, passes _present's any(...), and enters the rotation. The CLI's
--api-key is required. Finding 2's gate fix closed the read side; this is the
write side still reachable, and it is the shape that made finding 2 live.

The write path also does not enforce the refusal make_provider:130 enforces:
provider endpoint add azure_openai ... writes successfully and the next
startup fails with "does not support multiple endpoints". Recorded as a
follow-up already -- noting only that it is reachable from three surfaces now,
not one.

6. raven/config/update_providers.py:598 -- a row can read ****set**** with
configured: False.

list_providers computes configured from credential_status (which ignores
the flat fields when endpoints is set) but api_key_redacted from the flat
api_key first. Two consequences: the two columns can contradict each other on
one row, and the (N endpoints) branch at line 601 is unreachable whenever a
flat key remains -- which is the common upgrade shape.

7. raven/providers/per_model_provider.py:64 -- routed sub-providers lose
model_overrides behind a rotor.

overrides = getattr(fallback, "model_overrides", None) or {} reads an
attribute only LiteLLMProvider defines. EndpointRotorProvider does not
expose it (its inners hold it) and neither does LazyProvider, so with knn
routing over a multi-endpoint section every routed model silently falls back to
{}. "User model_overrides win over shipped extra_body defaults" is one of
this PR's own fixes, so the new feature is the thing that opts a config out of
it.

8. raven/agent/loop/main.py:669 -- the cross-thread argument no longer
matches the body.

The docstring justifies safety as "the only write here is one int attribute,
and the GIL makes that assignment atomic", but the method now also writes the
curator assembler, its trimmer, and the consolidator -- from LazyProvider's
prewarm thread, concurrently with a turn that may be reading them. The cascade
itself is the right fix for finding 6; the reasoning above it needs to be
either updated or made true (hop the write onto the loop).

Lower confidence

Where What Why I am not calling it a blocker
provider_commands.py:816, model.endpoints RPC extra_headers is emitted verbatim; _redact_nested_model judges by field name and extra_headers matches no secret rule. A self-hosted gateway's Authorization lives there The flat field has the same gap on main, so this is inherited, not introduced -- flagging it only because the PR states keys are redacted at every display face, and endpoints add two new faces
tui_rpc/methods/model.py _configured_overlays One load_config() per provider row (21 reads + validations per model.options), on top of the existing per-row list_providers() Runs in to_thread, so it costs latency rather than blocking the loop
config/schema.py:351 label: str accepts ""; provider endpoint add --label "" persists it The uniqueness validator still holds, so the damage is cosmetic
endpoint_rotor.py:296 A stream that ends with an immediate StopAsyncIteration is recorded as mark_success Plausibly deliberate

Checked and clean

The rotor's rotation / cooldown / stream-boundary discipline and its delegation
surface (nothing on LLMProvider is left undelegated, and no caller reads
provider.api_key / api_base off it); LazyProvider forwarding and the
on_built prewarm race (the setter re-fires outside the lock, and the
registration at main.py:614 runs after build_context_engine, so the
callback never lands on a half-built loop); the window ladder's
allow_fetch / allow_import seam; the refresh_context_window cascade
(factory.py builds one engine, so there is no second trimmer left stale);
_present on a raw mapping (model_save_key passing a dict is safe);
endpoint_strategy as a Literal, so no rotor ValueError can escape from
config; endpoint label uniqueness; wire.py round-trips and merge_key
precedence; the litellm stream restructure's try/finally and done sentinel;
prompt_cache refusal detection and strip itself; the
MissingCredentialsError handler chain; gracefulExit signal replay; the
onboarding split (ruff F821/F811/F401 clean, circular import
import-time-safe).

On the four candidates left unverified last round: the per-hop cache strip is
finding 1 above and the write-path refusal is folded into finding 5. Rotor
retry depth on auth failures I could not reproduce -- _rotates returning True
for auth while the inner ladder declines to retry it is the right pairing.
catalog._vendor_id vs wire.merge_key do differ in how they compare the
provider name (head == provider against
head == normalize_provider_name(provider)), but I could not construct an
input where that changes the result, so I am not filing it.

@0xKT

0xKT commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for finishing the four candidates and for re-running the originals --
and for the rotor measurement in particular; a refutation with a stopwatch on
it is worth as much as a confirmation.

Everything you confirmed, plus the new finding, is addressed in one commit:

  • Cache gate ignoring the detected gateway (your new one) -- fixed the way
    you suggested: _supports_cache_control falls back to self._gateway when
    no provider_name was given. Your three-row table (no base / aihubmix /
    openrouter) is now a pair of tests, and the aihubmix row asserts False.
  • Write-path refusal -- one predicate
    (registry.endpoints_unsupported_reason) now serves the factory, the ops
    write path and the RPC face, so the config that cannot boot is refused at
    write time, rendered cleanly on the CLI and mapped to the same
    ConfigValidationError shape on RPC.
  • PerModelProvider per-hop guards -- the loop now mirrors the base loop's
    two: can_serve skip and cache-mark strip. Your reachability analysis is
    reflected in the test: the strip case is exercised against a fallback stub
    shaped like the non-LiteLLM client that never strips at send time.
  • Rotor exhaustion residue -- the log line you suggested: on exhaustion it
    names each endpoint tried and its classification before returning the last
    error.
  • catalog._vendor_id -- normalizes the provider spelling the way
    merge_key does; your nano-gpt/nano_gpt divergence pair is the test.
  • Finding 1, your middle path -- adopted. A config still carrying the
    written-out 65536 gets one startup warning naming the exact line to delete;
    the file itself stays untouched. That closes the silent-failure gap while
    keeping the no-rewrite stance.

The rotor refutation needed no code, but the residue you noted rode along
with the exhaustion log above.

@gloryfromca

Copy link
Copy Markdown
Contributor

Correction on my previous comment: I reviewed against 78e7a3b and did not
notice the branch had moved. a5e43588 landed nine minutes before I posted,
so two items in that comment were already answered and one of my conclusions
was wrong. Retracting those first, then what is left.

Retracted -- finding 1 (PerModelProvider per-hop guards). Already fixed in
a5e43588, and fixed correctly: the loop now mirrors both of the base loop's
guards, and the tests exercise the behavior rather than the shape -- the strip
case asserts the marker is actually gone from what the fallback received
(seen[0]["messages"] == [{"role": "system", "content": "sys"}]), and the
can_serve case asserts the skipped hop was never awaited. My comment also
presented this as a candidate not yet confirmed, which was stale; it had been
confirmed in the preceding round.

Corrected -- catalog._vendor_id vs wire.merge_key. I wrote that I could
not construct an input where the divergence changes the result. That was wrong:
the nano_gpt / nano-gpt pair does exactly that, the spelling mismatch makes
the snapshot lookup miss, and it is fixed in a5e43588. My conclusion should
have been "I did not find one", not "there is not one".

Also already fixed -- the write-path refusal half of finding 5. One
predicate (registry.endpoints_unsupported_reason) now serves the factory, the
ops write path and the RPC face, and the test asserts the refusal happens before
disk is touched (assert not cfg_path.exists()). Good shape.

Still open at f3df92e6

I re-checked each of these against the current file contents at head rather
than by reading commit messages. None of the three new commits touches them:

# Where Note
2 providers/auth.py:238 auth.py is untouched by all three commits; providers.custom with only an apiKey still refuses to start despite its shipped default_api_base
3 providers/auth.py:148 and cli/_helpers.py:166 _helpers.py changed only to adopt the shared refusal predicate; ep.api_base or config.get_api_base(model) is unchanged, so the gate and the builder still disagree, headers included
4 providers/base.py:474 base.py untouched; _STATUS_404 is still a widened bucket sitting ahead of the 400 one
5 (first half) ui-tui/src/components/modelPicker.tsx:553 no ui-tui/ changes; a blank key still persists a keyless endpoint into the rotation
6 config/update_providers.py:598 only add_provider_endpoint changed; list_providers can still report ****set**** with configured: False, and the (N endpoints) branch stays unreachable
7 providers/per_model_provider.py:64 the two new guards do not touch this line; getattr(fallback, "model_overrides", None) still reads an attribute neither EndpointRotorProvider nor LazyProvider defines
8 agent/loop/main.py:667 still reads "the only write here is one int attribute", while the body now also writes the assembler, its trimmer and the consolidator

One note worth more than its severity, on #7: the reason it survived a green
suite is that test_sub_providers_inherit_configured_model_overrides builds its
fallback as a LiteLLMProvider, which is the one class that does define
model_overrides. The bug only appears when the fallback is a rotor or a
LazyProvider, so the test passes and the behavior is still wrong. Worth a
parametrized fallback rather than a second test.

On the litellm-import probe change in f3df92e6: I checked it for a weakened
assertion and it is not one. The claim ("litellm" not in sys.modules
mid-process) still runs and is now reported through both stdout and the exit
code, so a regression turns it red either way; what is skipped is interpreter
teardown, which was never part of the claim. Reasonable isolation of a native
atexit segfault.

0xKT and others added 21 commits August 10, 2026 22:35
Four decisions about providers were implemented outside raven/providers/:
the credential-gate exit, the price and context-window ladder, pin
resolution, and the predicate for which wire accepts a cache dialect.
Each had grown a second copy at the surface that needed it, and the
copies had drifted, so the same provider answered the same question
differently depending on which entry point asked.

Each decision now has one module that owns it, and the surfaces call it:

  wire.py          outbound route prefix and inbound storage form
  auth.py          credential requirements as an AND-of-OR grammar
  pin.py           which provider a stored model routes to
  rates.py         price and context window, with an explicit ladder
  prompt_cache.py  who accepts cache_control, and who refused it
  catalog.py       model labels, from a snapshot pinned to a commit

Behaviour this changes for a user: every surface that writes the default
model now also decides its pin, so the model is served by the provider
just configured rather than by whichever one a keyword matched; missing
credentials are reported as the sentence naming what is missing instead
of an exit; a provider that refuses cache_control is learned once and
not asked again.

Co-authored-by: Claude (claude-opus-5[1m]) <noreply@anthropic.com>
The error and response.failed SSE events were collapsed into a bare
"Codex response failed" RuntimeError, so classify_error landed in the
unknown bucket (retryable=False) and an overloaded backend was never
retried. Carry the structured code and message into the exception text;
the existing "overloaded" substring needle then classifies
server_is_overloaded as a retryable server error without touching the
classifier.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The rewriter had no model parameter and the gate only its dedicated
llm_gate_model, so both fell through to provider.default_model on every
call -- the only auxiliary LLM calls in the repo not chained back to the
configured agent model. Thread build_context_engine's model into
_build_rewriter_and_gate: the rewriter now sends it explicitly and the
gate falls back to it when llm_gate_model is unset.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The configured default 65536 flowed raw into trimming, token budgets,
memory consolidation, and the TUI gauge, so a model with a 200k window
lost two thirds of it every turn and the gauge rendered against a
number that was nobody's. The ladder now lives in providers/rates.py
effective_context_window: explicit config wins, then the model's real
window, then a documented fallback. AgentLoop resolves once at
construction and pins explicit values (a pin now also survives the
per-call live lookup), /model switches re-walk the ladder, and an
unknown window reports context_max=0 so the UI shows its empty state
instead of a guess.

Known gap, unchanged scope: trimmer and consolidator keep their
construction-time snapshot after a model switch.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
A backend launched without its reasoning parser (e.g. sglang without
--reasoning-parser) swallows the opening <think> into the prompt
template, so the completion arrives as bare reasoning prose plus an
orphaned </think>. LiteLLM's only fallback anchors on an opening tag
and runs non-streaming only, so the broken protocol was recorded as-is
into trajectories. split_orphan_think in providers/reasoning.py splits
the leading reasoning off once the full text is in hand, wired into the
non-streaming parse and the stream assembly; live deltas stay untouched
because a delta cannot be classified before the closing tag arrives.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The onboarding wizard's "another supported vendor" branch treated every
LiteLLM vendor as a paste-a-key shape, so chatgpt (device flow that
ignores the key), bedrock/sagemaker (AWS credential chain), vertex_ai
(project+location+ADC), azure (base+version+key/Entra) and cloudflare
(key+base/account_id) were written down as configured and then failed
on the first call. auth.py now carries the refusal table with what each
vendor actually needs, and _collect_credentials refuses before
collecting instead of persisting a section that can never authenticate.
chatgpt points at raven's own openai_codex flow; gigachat stays
configurable and only gains a hint that its key is
base64(client_id:client_secret).

Deliberately no registry specs or multi-credential schema for these six
until someone actually needs them served.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Investigating a reported spawn-subagent 401 established that chat() and
chat_stream() both pass the provider's api_key explicitly to
acompletion and that a subagent reuses the main provider instance
verbatim -- the reported asymmetry is not reproducible from the code,
on HEAD or on the release it was reported against. What the
investigation did find is that no test ever asserted the api_key kwarg
arrives, so the explicit-forwarding line could be dropped without
anything turning red. These three tests pin the mechanism on both call
paths and through the subagent manager's shared instance.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…erwriting

model_overrides is already the channel for arbitrary sampling/serving
params -- LiteLLM auto-forwards unknown top-level keys into extra_body
for OpenAI-compatible backends, and a nested structure can be written
as extra_body directly. But both chat() and chat_stream() assigned the
provider's wire-routing extra_body (e.g. the OpenRouter provider pin)
over whatever a model_overrides entry had placed there, so a user's
extra_body keys were silently dropped whenever the request travelled
through a gateway. Merge the two instead, wire keys winning on
collision because routing pins must reach the wire intact. The schema
comment now names model_overrides as the passthrough channel, so no
sampling_extra/request_extra fields get invented for the same decision.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The picker already marks MiniMax as an open-source partner; this adds
the one sentence telling other vendors that the door is open.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Two guards the wizard was missing. The curated picker catalogue is
hand-written and the existing tests only checked one direction, so a
provider added to the registry but never to the shortlist stayed
unreachable with everything green; the new set-equality test turns
either drift direction red. And both prefill paths -- the language
screen defaulting to the active language, and the model prompt
defaulting to the already-configured model -- had no assertions at all
(every _pick_model test passed current_model=None), so deleting either
default left the suite green.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
suspendForHandoff defers signal exits while a device-flow login owns
the terminal, but the handler dropped the signal outright: a SIGHUP
landing in that window (terminal gone, delivered to the whole process
group) killed the login subprocess yet left the TUI process alive with
no controlling terminal -- a headless process only kill could reach.
The handler now records the first deferred signal and the outermost
release replays it through the normal exit path, so the intent the
user's terminal expressed during the handoff is honored right after it.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
chat_with_retry walked the fallback chain by swapping the model string
while the instance's key, base and gateway stayed frozen, so a direct
provider sent another vendor's fallback model out under its own
credentials -- usually a 400, and silently the wrong backend when two
vendors share a model name. can_serve now skips such hops with a
warning (gateways still answer for everything; unresolvable ids still
fail loudly at the wire), comparing vendors through
canonical_provider_name so config aliases do not read as cross-vendor.
PerModelProvider stops funneling the whole chain into the primary
model's endpoint and dispatches each hop to its own routed sub-provider,
matching the base loop's continuation semantics.

Per-hop identity rebuilding and per-hop wire overrides stay with the
multi-endpoint work.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
… table

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…mode

A provider without real streaming (azure, codex) replays its chat()
result through the default chat_stream fallback as one terminal
StreamDelta -- which had nowhere to put finish_reason or
error_classification, and the stream collation then hardcoded the
finish_reason to stop/tool_calls. Net effect on the default streaming
path: an upstream error was rendered to the user as a normal assistant
reply, with no retry, no fallback, and the turn recorded as success.
The terminal delta now carries both fields, and _llm_call_stream
diverts an error delta away from token rendering into the same error
LLMResponse shape the non-streaming path already returns, so the
existing recovery and terminal-error handling apply unchanged.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The 429 and 5xx buckets each carry their literal status as a substring
marker for the degraded string path, but the 404 bucket only had the
wordier phrasings -- so a provider that renders its non-200 body into a
plain string (azure's path) with a route-level "Resource not found"
message fell through to unknown and lost the fallback.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…bout

can_serve treated "my provider_name resolves to nothing" as a mismatch,
so an instance constructed without a provider_name -- the proactive
planner and evolver both build one that way -- skipped every resolvable
fallback hop, and an OAuth identity like github_copilot, whose one
grant serves several upstream vendors, refused models it actually
serves. The veto now requires both sides to be known, non-OAuth and
different; every unresolved identity falls back to failing loudly at
the wire, which is the base class's stated default.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The merge gave the provider's built-in extra_body priority on
collisions, justified by "routing pins that must reach the wire" -- but
the whole _WIRE_OVERRIDES table is one shipped default workaround
(disabling OpenRouter's qwen reasoning mode), not a routing pin, and
the same change set documented model_overrides as the channel that
overrides shipped defaults. A collision is the user deliberately
reversing one, so the user's value wins now; the conflict test mirrors
the one real collision instead of an invented provider pin.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
… place

First stage of multi-endpoint support: ProviderConfig gains an
endpoints list (label + key + base + headers per entry), and
providers/endpoints.py resolves the three spellings a section can
carry -- explicit endpoints, Gemini's api_key_list, the flat fields --
into one uniform list with strict precedence and no merging, so a
stale flat key cannot outlive the endpoint meant to replace it.
Reading the shapes independently is how the Gemini list came to be
declared and never used. Rotation and failover read this list next;
make_provider will refuse the field on OAuth/azure/codex providers
when it is wired up.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The window ladder put a synchronous OpenRouter catalogue fetch (10s
httpx timeout on a cold cache) inside AgentLoop construction and the
/model switch handler -- the former stalls startup, the latter runs on
the asyncio event loop and freezes every turn with it. Both callers now
resolve with allow_fetch=False: an in-process cache of any age answers,
then the on-disk cache of any age, then the documented fallback, and
the network is never touched. The per-call usage path keeps refreshing
normally; it already lives inside an await and is where a stale window
catches up.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
onboard_commands.py had grown past 5000 lines; the EverOS/Step4 cluster
(~1500 lines, 36 symbols) now lives in onboard_everos.py. Pure move,
zero behavior change: shared wizard UI state stays in onboard_commands
and both modules reach across through module references with call-time
attribute access, so import order cannot bite and monkeypatches keep
hitting the code that actually runs. Every migrated patch target in the
test file was mutation-checked one by one against pointing at the old
module -- all 23 turn red, none patch a shell.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
split_orphan_think ran on every provider's every response, so any
ordinary answer that mentioned a bare closing think tag -- a tutorial,
a code sample -- had its leading half silently folded into reasoning.
The parser-less sglang/vLLM shape only comes from a self-hosted
backend, so emits_unparsed_reasoning gates both call sites: custom,
local and unspecced identities normalize, gateways and known
direct vendors leave content alone. The pairing check now looks for
the shared opening-tag prefix, so a block that opens <think> and
closes </thinking> no longer leaks its opener into the reasoning text.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
0xKT and others added 25 commits August 10, 2026 22:45
extra_headers can carry a secret (an auth header some gateways need
alongside the key), and both endpoint display faces handed the values
back verbatim. Each value is now masked on its own with the key names
left visible -- masking the whole dict as one string would also hide
which headers are configured -- in list_provider_endpoints and in the
nested-model redaction get_provider_config applies per endpoint.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Every picker row re-parsed the config from disk twice -- once for the
list_providers mapping rebuilt inside _build_provider_entry, once for
_configured_overlays -- so opening the picker cost two parses per
registry provider (22 observed for 21 rows). _entries_off_loop now
loads the config once and hands each row its mapping and section; the
single-row path keeps loading on its own. A counting test pins the
bound at two parses per open.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…tion

Dropping the 404 text needle left codex's non-200 path -- a plain
RuntimeError whose message starts "HTTP 404:" -- classifying as unknown,
so a real 404 stopped triggering fallback. The status-carrying exception
azure grew for the same reason is promoted to a shared ProviderHTTPError
(subclassing RuntimeError, so any broad handler keeps matching) and
codex raises it too; both providers now classify from the live status
code instead of the rendered text.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
… field

list_provider_endpoints (feeding `provider endpoint list` and the TUI
picker's endpoint rows) read each entry's own fields, so an endpoint
written with only --label/--api-key -- the shape the flat-inheritance
change exists for -- displayed an empty address while requests were
correctly using the section's flat one, reading as "the config did not
take". The display face now consumes provider_endpoints, the same
resolved view every request uses.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The credential gate's spec-default fallback checked only that
default_api_base was non-empty, while Config.get_api_base serves a
default only for a gateway or local deployment -- a second derivation
of the same rule, which happened to agree for the two requires_api_base
specs that exist today and would silently diverge for a spec carrying a
default the reader never hands out. Both now read one registry property
(usable_default_api_base), and an invariant test walks every spec
asserting the gate never passes a bare-key config whose default the
reader would then refuse to serve.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
needs_api_base was derived from the credential shape alone, so the TUI
picker kept blocking a bare-key custom submission client-side -- the
exact refusal the save_key alignment was supposed to end -- while
rendering the address field as "(required)". It now says "an address
must be supplied": true for local deployments (the address is the
credential) and for endpoint-credential specs shipping no usable
default (azure), false for custom, whose shipped localhost default the
gate accepts. The picker's existing check and label read the flag
unchanged.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The per-endpoint extra_headers got per-value redaction while the flat
section field -- where an AiHubMix APP-Code actually lives, per its own
comment -- still printed plaintext through `provider get` without
--show-secrets: one table, two rules. The field now carries the
schema-level secret marker (the direction _is_secret_field's docstring
already names) and _redact learned to mask a dict per value, keys left
visible.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Reporting only the first gap made a bare azure submission a two-round
trip: "requires an API key", then "requires an address" after the key
was supplied. All missing labels are joined into one message, and the
field picked for error.data no longer assumes the first requirement
carries one.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
For a section whose endpoints all lack keys, the gate's hint said
`provider set --api-key` -- which writes the flat field the gate then
ignores exactly because endpoints exist, so following the hint changed
nothing and the gate repeated itself. The hint now names
`provider endpoint add` whenever the section carries endpoints.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The picker-rows hoist left a third disk read in place: every row's
_provider_models fetched its curated model list through
get_provider_config, an uncached raw read and parse per registry row.
The all-rows path now reads the models off the section it already
loaded; the single-row path keeps fetching on its own. The counting
test now also counts raw reads, which the load_config-only bound was
blind to.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The is_local branch ran before the endpoints one, so a local provider
with several keyless endpoints -- the multi-instance shape the write
path explicitly allows -- listed as just "(not needed for local)" with
no trace of them, while the same shape on a keyed provider showed the
count. The endpoints branch now runs first and keeps the local wording
alongside the count.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The early return for names outside api_key/api_base changed no answer:
ResolvedEndpoint carries no other requirement field, so the getattr
already answers False for api_key_list and anything else. Let it.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Three copies of the endpoint-inheritance comment said an entry inherits
when it names "neither" api_base nor extra_headers, while the code falls
back per field independently -- an entry naming only its address still
inherits the flat headers. All three now say so. The
refresh_context_window docstring stops enumerating its cascaded writes
(the enumeration was already one object short); the invariant that
matters is that every transitive write is a GIL-atomic int assignment.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The classification-level test constructed ProviderHTTPError directly,
so reverting the raise site to a plain RuntimeError kept the suite
green -- the exact null-verification shape a mutation run exposed. A
faked non-200 stream now walks chat() end to end and asserts the
classification arrives as model_unavailable; the same mutation turns
it red.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…ecrets

set_provider_fields' OAuth branch matched everything marked is_secret,
so the new secret marker on extra_headers turned
`provider set --extra-headers` into a refusal for OAuth providers --
and the refusal sent the user to `provider login`, which never writes a
header. The guard now names the credential fields its own docstring
promises (api_key, api_key_list); extra_headers is secret to display,
not a credential.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…c spec

The every-spec invariant is vacuous while no registry spec carries a
default_api_base that differs from its usable one -- reverting the gate
to read the raw default kept it green. A synthetic requires_api_base
spec whose default the reader never serves pins the distinction.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
_localize keyed the endpoint-add hint off any truthy endpoints
attribute while _present only treats a real list as the endpoints
shape; a duck-typed section could be told to run endpoint add while the
gate was judging its flat fields. Same isinstance guard on both.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Deleting the isinstance guard in _localize turned nothing red: the
duck-typed shape it protects only ever reaches the gate from test
doubles, so no test exercised it. One does now.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The background warm skipped whenever the in-process table was non-empty,
and _cached_catalog_only adopts a disk table of any age into exactly
that table -- so a long-lived session booting on a days-old cache file
never warmed again for the life of the process. The guard now asks for
freshness (a table with a live timestamp inside the fetch TTL), which a
disk-adopted table deliberately does not have. Also stated honestly in
the re-check comment that the unlocked read narrows the warm race
rather than eliminating it. Both raised in review of the upstream
change this branch carried the catalog reader over from.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…custom

Two holes in the vision probe's exclusion list, both raised in review of
the upstream change and both about a served name wrongly joined against
the vendor catalog -- the silent lose-the-picture direction:

- The TUI's loop holds a LazyProvider, so the isinstance probe for the
  Azure transport answered about the proxy and a bare deployment name
  was looked up as OpenAI's model. The proxy now exposes `unwrapped`
  (the materialized inner, never a building accessor) and the probe
  reads through it.
- A `custom` gateway serves whatever its operator named the model, and
  find_by_model resolves that name to the vendor's spec -- which also
  made the documented vision_override rollback unreachable for it. The
  probe now asks the configured provider which section built it
  (provider_name, delegated through the rotor) and treats an
  explicit-selection gateway (is_gateway with no keywords) as
  caller-chosen.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Two residues review left on the switch fan-out. The image-tool-result
verdict is cached per model id but computed from the provider, so a
swap that kept the model id went on serving the old transport's answer;
adoption now clears it. And a parked switch was invisible end to end --
the RPC had already answered applied=True with the config written, so
the window where replies still come from the old provider left no
trace; the park and the adoption each log one line.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
…call

The RPC called refresh_context_window right after set_provider, but a
switch that parks returns immediately -- the refresh recomputed the old
model's window, and when the parked switch was adopted at turn drain
nothing refreshed again, so the new model ran with the old window for
the rest of the session. The refresh now lives in _adopt_provider, the
one point both the immediate and the parked path go through, and the
RPC call site drops its copy.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
The fetch docstring contradicted its neighbor (the table is double-keyed
by full id and bare alias, and two consumers rely on the alias); the
real park-to-adopt test now proves the window re-resolves at adoption
via a sentinel instead of asserting nothing; the unbuilt lazy proxy's
"reads as not Azure" branch is pinned; and two comments are trimmed --
one restated the docstring eight lines above it, one anchored itself to
a line position that moves.

Co-authored-by: Claude (claude-fable-5) <noreply@anthropic.com>
@0xKT
0xKT force-pushed the refactor/provider_module_redesign branch from f3df92e to acd6b8c Compare August 10, 2026 15:28
@0xKT

0xKT commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the correction, and sorry about the race: your second pass
landed three minutes before our previous reply, so we posted against the
comment set we had already read and missed it. We now re-fetch the thread
before posting anything.

One logistics note first: the branch was rebased onto the current main
(picking up #282 and #285), so the commit ids in earlier comments no
longer resolve -- everything maps one-to-one by subject line. The ids
below are the current ones.

All seven still-open items are addressed, one commit per item, and an
internal adversarial pass over the fix batch surfaced a few more that
rode along -- the full list:

Your seven:

  • feat(security): label untrusted content and contain injection blast radius #2 custom refusing a bare apiKey (c3c7baa) -- confirmed by running
    your repro; a regression against main. The address requirement used by
    requires_api_base specs now falls back to the spec's own default;
    azure ships none and still demands one, and is_local specs keep the
    plain requirement so Ollama's standard port does not make an untouched
    section look configured. Three same-shape holes fell out of fixing it:
    the picker's save_key had its own copy of the address rule and would
    have kept refusing what the gate now runs (it consults
    credential_status instead); needs_api_base on the picker rows was
    derived from the credential shape alone, so the TUI client kept
    blocking the same submission one layer further out (cad1d38); and the
    gate's fallback condition was a second derivation of "which defaults
    does the reader serve" -- both sides now read one registry property,
    with an every-spec invariant test (c443b82).
  • feat: add TUI /model — provider/model config CRUD and between-turns model switch #3 gate/builder disagreement, headers included (cbe24b8) -- your
    repro reproduced, and the shape is worse than stated: a section-level
    apiBase with key-only endpoints (the natural product of
    endpoint add --label --api-key) failed the gate outright. One
    semantics now, owned by provider_endpoints: entries inherit the flat
    api_base/extra_headers per field they do not name, api_key is
    never inherited, and the gate and builder both consume the resolved
    list. The endpoint display faces consume it too (6704f17) -- they were
    still showing the raw empty field for an entry whose requests were
    correctly using the inherited address.
  • docs: promote domain glossary as review baseline (+ package docstring realign) #4 _STATUS_404 (7c4253e) -- your framing was right and ours was
    not: it was a widening, and it sat ahead of the 400 bucket (verified
    with an embedded-404 repro). Fixed the way you suggested rather than by
    bare deletion, and not just for azure: codex renders its non-200 the
    same way, so the status-carrying exception is shared
    (ProviderHTTPError, subclassing RuntimeError) and both providers
    classify from the live status (9335b8d). A faked non-200 stream pins
    codex's raise site end to end (b472901) -- the first version of that
    test only constructed the exception class and survived reverting the
    fix, which a mutation run caught. A rendered azure 429 now also
    classifies as rate-limit instead of whatever its body text matched.
  • docs(context): replace Bus cluster with Spine + add Agent Core missing terms #5 blank-key endpoint from the picker (47e7196) -- the write-side
    gap is real, but we deviated from "require it like the CLI does": a
    blanket rule would break local deployments, where keyless endpoints
    are the legitimate multi-instance shape. The rule lives once in the
    ops layer, derived from the registry's credential kind: key-based
    providers refuse an empty key, local ones keep writing keyless
    entries, the CLI's --api-key becomes optional to match, and the
    picker mirrors the check client-side. label also gained
    min_length=1.
  • docs(context): replace Bus cluster with Spine + add Agent Core missing terms #6 ****set**** with configured: False (4cbb9f0) -- display now
    mirrors the reader's precedence; the (N endpoints) branch runs first
    and is reachable again, including for local deployments, whose
    endpoint count the local wording used to hide entirely (74aa491).
  • docs(glossary): add Consolidator term + fix Profile file casing #7 model_overrides lost behind the rotor (4633d73) -- fixed with
    a delegating property, and your note on the test was taken as written:
    parametrized fallback rather than a second test.
  • feat(install): one-line zero-friction install + packaged tui bundle #8 stale docstring (3d04d83) -- rewritten to state the invariant
    (every transitive write is a GIL-atomic int assignment) rather than an
    enumeration that was already rotting.

Smaller things that rode along: endpoint display faces redact
extra_headers values keeping key names visible, on the flat section
field too (262aa2f, 306a05d); the per-row load_config you flagged is
hoisted -- it was two parses per registry row plus a third raw read for
the curated models, 22 observed for 21 rows, now bounded and pinned by a
counting test (156cd34, 8192538); save_key names every missing
requirement at once instead of one per round trip (23aef6c); and the
gate's key hint now says endpoint add when endpoints exist -- the old
hint told users to write the flat key the gate ignores in exactly that
shape (f888d86).

Your #285 notes came along for the ride. The rebase carried the
catalog reader into providers.rates, which made your inline findings
there ours to fix: the stale-disk-suppresses-warm coupling (the warm
gate now asks for freshness, which a disk-adopted table deliberately
lacks -- b825060), the lazy proxy defeating the Azure isinstance probe
plus custom missing from the caller-chosen list (the proxy exposes
unwrapped, the probe asks the configured provider which section built
it, and an explicit-selection gateway reads as caller-chosen --
ec51874), and the check-then-act comment now says it narrows the race
rather than closing it. From the #282 thread, the image-tool-result
verdict cache is invalidated on adoption and the park/adopt window logs
itself (e9967f7); composing that with our window ladder also surfaced
that a parked switch used to adopt the new model with the old context
window -- the refresh now lives at the adoption point (9ce23f3).

One deliberate non-fix, stating our reasoning rather than silently
skipping it: StopAsyncIteration -> mark_success in the rotor stays.
An empty stream is model-side behavior, not endpoint failure; rotating
on it would walk every endpoint into cooldown for a response no swap
can change. If you have a failure mode in mind where that reasoning
breaks, we would genuinely like to hear it.

Two known follow-ups we are deferring, not hiding: the remaining
bare substring matches ("429", "500"-"504") in classify_error have the
same false-positive shape the 404 one had, but they predate this branch
and sweeping them safely means source classification for every path
that renders a status into text -- follow-up issue territory. Same for
a pre-existing routing quirk this PR restores faithfully from main:
custom with any credentials claims unprefixed model ids ahead of
other gateways (PROVIDERS order), which contradicts the registry's own
"matched only via explicit selection" note -- also on main today, so a
design discussion rather than a fix here.

If nothing else surfaces, a formal review verdict would help us close
the loop on this PR.

@0xKT
0xKT merged commit 56f7095 into main Aug 10, 2026
12 checks passed
@0xKT
0xKT deleted the refactor/provider_module_redesign branch August 10, 2026 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment