Skip to content

fix(desktop): show a spawned session under its provider, not "Custom endpoint" - #330

Merged
OmarB97 merged 1 commit into
mainfrom
fix/model-picker-custom-endpoint-attribution-fork-20260802
Aug 2, 2026
Merged

fix(desktop): show a spawned session under its provider, not "Custom endpoint"#330
OmarB97 merged 1 commit into
mainfrom
fix/model-picker-custom-endpoint-attribution-fork-20260802

Conversation

@OmarB97

@OmarB97 OmarB97 commented Aug 2, 2026

Copy link
Copy Markdown
Owner

What does this PR do?

A session started with hermes desktop spawn -m <model> --provider <name> was rendered in the model picker under a "Custom endpoint" section — a duplicate of the provider's own entry, carrying its own independent thinking/effort badge — even when the override pair exactly matched a configured catalog entry.

The per-session override itself is correct and unchanged (#298/#318 design: spawn overrides deliberately bypass the composer stores). The bug is purely in how the picker payload describes that session.

Root cause. resolve_runtime_provider() reports every user-defined provider as provider="custom" plus a base_url — bare "custom" is the resolved billing class, documented as "NOT a routable provider identity" (hermes_cli/runtime_provider.py). So a session spawned with --provider ai-router has agent.provider == "custom":

resolve_runtime_provider(requested='ai-router')
  → provider='custom'  base_url='http://10.55.0.3:8000/v1'
    requested_provider='ai-router'  source='custom_provider:ai-router'

model.options overlays those agent attributes onto the disk config (tui_gateway/server.py), so the picker substrate sees bare custom for a session that is really running a configured provider. Two independent sites then mishandled it:

  1. hermes_cli/model_switch.py, section 3b synthesizes the {"slug": "custom", "name": "Custom endpoint"} row for the one-off model.provider: custom + model.base_url: config shape. Its dedup guard only scanned custom_providers: — it never consulted the rows section 3 had already emitted from the providers: dict. An endpoint declared under providers: therefore got a second, anonymous row aliasing it. This is the exact shape every ~/.hermes/profiles/*/config.yaml uses.

  2. hermes_cli/inventory.py, _single_named_custom_match() — the helper that maps bare custom back to a named row — only considered slugs starting with custom:. providers: entries own their endpoint under a bare slug (ai-router, taro), so they were invisible to it and the payload's top-level provider stayed "custom". The renderer decides which entry is live with group.provider.slug === optionsProvider (apps/desktop/src/app/shell/model-menu-panel.tsx), so 'ai-router' !== 'custom' meant the AI-ROUTER entry never got the check mark and fell back to the model's remembered preset for the thinking/effort badge instead of the live session state — the "independent badge" half of the report.

A third row surfaced once attribution was fixed: with explicit_only (the desktop chat picker's default, apps/desktop/src/lib/model-options.ts) the filter re-adds a configured-current placeholder to keep the live selection visible. Once the real provider row carries that job, the placeholder is a duplicate section and has to go — _hide_shadow_bare_custom_row already existed for exactly this, but only recognised source == "canonical".

Why endpoint identity, not model membership. The obvious predicate — "does the configured row list this model?" — is wrong. A row's model list is one live /models probe away from changing, and the current model is injected into the current row further down (model_switch.py) regardless. Keying on membership made the picker's sections appear and disappear with endpoint reachability; I hit exactly that flicker while testing (the same run gave different sections depending on whether http://10.55.0.3:8000/v1 answered).

Why exactly one owner. Rows with distinct credentials or wire protocols can legitimately share an endpoint (tenants behind one proxy URL — section 3's own grouping comment calls this out). None of them can claim the session alone, so both sites decline that ambiguity in lockstep and the synthesized row stays. Had suppression used "any owner" while attribution required "exactly one", the multi-tenant case would have been left marking a credential-less placeholder as current, with api_url: null and a false "not authenticated" warning.

Unmatched overrides are untouched: a genuinely novel base_url/model still gets its "Custom endpoint" section.

Related Issue

None filed — reported directly against the desktop picker (observed 2026-08-02 with ai-router + deepseek-v4-flash-0731-ds4).

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • hermes_cli/model_switch.py — section 3b gains an _endpoint_owners guard: skip synthesizing the bare "Custom endpoint" row when exactly one already-emitted row serves the same normalized base_url. The pre-existing custom_providers: check is unchanged; this adds the providers: side it never covered.
  • hermes_cli/inventory.py
    • _single_named_custom_match() — candidates widen from custom:-prefixed slugs to any user-defined row at the endpoint, still requiring an unambiguous single owner. Short-circuits when a source == "model-config" row is present, so section 3b's decision stays authoritative rather than being re-derived from models (which is mutated later).
    • _hide_shadow_bare_custom_row() — also drops the configured-current placeholder once attribution has moved off bare custom. Behaviour for every other provider is unchanged.
    • Docstrings on both note this is display attribution only.
  • tests/hermes_cli/test_inventory.py — 7 tests: attribution to a providers: row, URL normalization (trailing slash + case), attribution surviving an unlisted model, the placeholder drop under explicit_only, and three guardrails pinning what must not change (unmatched endpoint keeps its section, ambiguous endpoint declines, non-custom providers pass through).
  • tests/hermes_cli/test_model_switch_custom_providers.py — 3 tests at the list_authenticated_providers level: no synthesized row for an already-named endpoint, one still synthesized for an unnamed endpoint, and one still synthesized when several owners share an endpoint.

No renderer changes. The composer's persisted selection is never written: $currentModel/$currentProvider are only touched by an explicit user pick, and applyRuntimeInfo still writes only the unpersisted $activeSessionModel/$activeSessionProvider mirror added by #318.

How to Test

1. The new tests fail on origin/main and pass here. Reverting only the two hermes_cli/ files:

FAILED test_model_switch_custom_providers.py::test_bare_custom_row_is_not_synthesized_for_an_already_named_endpoint
FAILED test_inventory.py::test_bare_custom_attributes_to_configured_row_serving_the_endpoint
FAILED test_inventory.py::test_bare_custom_attribution_ignores_trailing_slash_and_case
FAILED test_inventory.py::test_bare_custom_attribution_survives_an_unlisted_model
FAILED test_inventory.py::test_explicit_only_drops_the_placeholder_custom_row_after_attribution

The four guardrail tests pass on both trees — that is the point of them.

2. Live check on the real RPC surface. Driving the actual model.options handler in an isolated HERMES_HOME whose config.yaml declares ai-router under providers: only (the profile shape), with a session whose agent carries what resolve_runtime_provider really returns:

Before (pristine origin/main):

spawn --provider ai-router -m deepseek-v4-flash-0731-ds4   (MATCHING)
   payload.provider = 'custom'
   row slug='custom'     name='Custom endpoint'  is_current=True
   row slug='ai-router'  name='ai-router'        is_current=True     <- both marked current
   renderer would mark live: ['Custom endpoint']

spawn onto a novel base_url/model                          (UNMATCHED)
   payload.provider = 'custom'
   row slug='custom'     name='Custom endpoint'  is_current=True
   renderer would mark live: ['Custom endpoint']

After:

spawn --provider ai-router -m deepseek-v4-flash-0731-ds4   (MATCHING)
   payload.provider = 'ai-router'
   row slug='ai-router'  name='ai-router'        is_current=True
   renderer would mark live: ['ai-router']                           <- no Custom endpoint row

spawn onto a novel base_url/model                          (UNMATCHED)
   payload.provider = 'custom'
   row slug='custom'     name='Custom endpoint'  is_current=True
   row slug='ai-router'  name='ai-router'        is_current=False
   renderer would mark live: ['Custom endpoint']                     <- preserved

renderer would mark live applies the picker's own predicate (provider.slug === payload.provider and the model present in that row) to the payload.

3. Multi-owner regression check. Two providers: entries on one URL with distinct key_env produce a payload byte-identical to origin/main — the synthesized row keeps source: model-config and the real api_url, rather than degrading to a configured-current placeholder with api_url: null and authenticated: false.

4. Suite: tests/hermes_cli/{test_inventory,test_model_switch_custom_providers,test_custom_provider_model_switch,test_user_providers_model_switch,test_status_model_provider,test_web_server}.py tests/test_tui_gateway_server.py1019 passed, 0 failed. The five picker suites were also baselined on the untouched tree first (157 passed) so a pre-existing failure couldn't be mistaken for a regression.

Desktop TS tests were not run (node_modules is not installed in this checkout). The change is Python-only and use-session-actions.test.tsx / use-session-state-cache.test.tsx import TS modules exclusively, so the #318 guarantees they pin cannot be affected by this diff.

One user-visible side effect worth calling out: per-model effort/fast presets are keyed on modelPresetKey(provider.slug, model). A user whose session previously showed the model under the custom row has presets stored at custom::<model>; after this fix the live row is ai-router, so those presets orphan and the model falls back to defaults once. Self-healing, and the new key is the correct one, but it will look like a reset.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)

🤖 Generated with Claude Code

…endpoint"

`hermes desktop spawn --provider <name>` bound a correct per-session
override, but the model picker rendered the session under a separate
"Custom endpoint" section with its own thinking/effort badge, duplicating
the provider's real entry.

resolve_runtime_provider() reports EVERY user-defined provider as
provider="custom" + base_url — bare "custom" is the resolved billing
class, not a routable identity — so `--provider ai-router` reaches the
picker substrate looking exactly like a one-off endpoint. Two sites then
mishandled it:

- model_switch.py section 3b synthesized the "Custom endpoint" row for
  any bare-custom session. Its dedup guard scanned only custom_providers:
  and never the rows section 3 had already emitted from providers:, so an
  endpoint declared there got a second anonymous row aliasing it. That is
  the shape every profiles/*/config.yaml uses.
- inventory._single_named_custom_match() matched only `custom:`-prefixed
  slugs. providers: entries own their endpoint under a bare slug, so the
  payload's top-level provider stayed "custom"; the renderer marks a row
  live with `provider.slug === payload.provider`, so the real entry never
  got the check mark and its badge fell back to the stored preset instead
  of live session state.

Both now key on endpoint identity, and both require exactly one owner.
Endpoint identity rather than model membership because a row's model list
is one live /models probe away from changing, which made sections flicker
with endpoint reachability. Exactly one because rows with distinct
credentials can share an endpoint (tenants behind one proxy URL) and none
can claim the session alone — the two sites must agree, or suppressing
the synthesized row would leave only rows that do not own it.

_hide_shadow_bare_custom_row() also drops the `configured-current`
placeholder that the explicit-only filter adds, once the attributed row
carries that job.

Display attribution only: no renderer change, and the composer's
persisted selection is untouched — spawn overrides still write only the
unpersisted active-session mirror (#318).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@OmarB97
OmarB97 merged commit d6b4e86 into main Aug 2, 2026
22 checks passed
OmarB97 pushed a commit that referenced this pull request Aug 2, 2026
… deepcopy

`find_custom_provider_identity` calls `load_config()`, which deepcopies the
whole config — and load_config's own docstring names
`get_provider_request_timeout` as the per-API-turn hot spot that must use
`load_config_readonly()` precisely to avoid that. Routing bare-custom
attribution through the general helper put the deepcopy right back on that
path, for every turn of every session on a user-declared endpoint.

Match the endpoint against the providers dict `_provider_config` has already
loaded. A timeout can only be read from a `providers:` entry anyway, so when
one owns the URL there is nothing left for the general helper to find; it stays
as the fallback for the `custom_providers:`-only shape and the no-base_url
case. Requires exactly one owner, the same rule #330 established — rows with
distinct credentials can share an endpoint and none can claim it alone.

URL normalization mirrors `runtime_provider._normalize_base_url_for_match` so
the fast path and the general helper can never disagree about whether two URLs
are the same endpoint.

Also documents `first_chunk_timeout_seconds` in cli-config.yaml.example (with
why it outranks `stale_timeout_seconds`, and that these must be declared under
the named provider entry rather than "custom"), and adds it to the known
provider keys in config.py — without which setting it warns on every load.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…he request prefix is stable (#334)

* fix(timeouts): resolve provider timeouts under the endpoint's real config entry

`resolve_runtime_provider()` reports EVERY user-declared endpoint as the bare
string "custom" — the resolved billing class, not a routable identity (#330).
Timeout resolution keyed straight off that runtime id, so it looked up
`providers["custom"]`, a key that by construction never exists, and reported
"nothing configured". `providers.<name>.stale_timeout_seconds` and
`request_timeout_seconds` were therefore silently unreachable on exactly the
local endpoints whose long cold prefills are the reason those knobs exist.

Both resolvers now take the live base_url and, on a bare-custom provider,
attribute it to the `providers:` entry that owns the endpoint — the same
endpoint-identity rule #330 established for the picker, via the existing
`find_custom_provider_identity` / `canonical_custom_identity` helpers whose
docstring already requires every persist/restore path to do this.

Attribution is by endpoint identity whenever a base_url is available, and only
falls back to `config.model.provider` when there is no endpoint to match on.
`canonical_custom_identity` always permits that fallback, which is right for
credential recovery and wrong here: a genuinely ad-hoc endpoint would otherwise
inherit an unrelated provider's timeouts.

The reverse lookup runs only when the direct key misses AND the provider is
bare custom, so named and built-in providers keep their existing single
dict-lookup cost on every API turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(diagnostics): env-gated per-turn request-prefix hash with divergence offset

When a long session starts re-prefilling from scratch every turn, the client
could not answer the only question that matters: did WE rewrite the prefix, or
did something evict the server's cache? Provider-reported cached_tokens tells
you a miss happened, never why.

HERMES_PREFIX_PROBE=1 records a rolling hash over each outbound request's
cacheable prefix — tool schemas first (chat templates render them ahead of the
conversation), then one element per message in wire order — and logs where it
first differs from the previous call in the same session, by element index,
role label and reusable character count.

Cumulative rather than per-element hashes, so the earliest differing index IS
the first prefix divergence: the point past which no cached work survives. A
pure append reports no divergence at all, which is the append-only contract a
healthy agentic turn must satisfy. Serialization sorts dict keys so key
ordering — which no chat template depends on and Python does not guarantee
across rebuilds — cannot masquerade as real drift. Sampling params are excluded
since they do not participate in the cached prefix.

State is file-backed under the session log dir rather than held on the agent,
because the cases worth catching are exactly the ones where the agent object
does not survive: a compute-host crash, an app update, a fresh per-turn agent
on the gateway path.

Hooked at the existing preflight seam in conversation_loop, where api_kwargs is
final and the request has not left yet. Gated by an env var alongside its
sibling HERMES_DUMP_REQUESTS two lines up; ships off, and every path is wrapped
so a diagnostic can never fail a turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(provider-parity): drop dead tuple assignments in _make_agent

Three statements sat between the kwargs assembly and the AIAgent
construction in _make_agent. Each trailing comma made them 1-element
tuple assignments to locals: base_url shadowed the function parameter
and api_key was a fresh unused local. Both ran after kwargs had already
captured the real values, so neither reached the returned AIAgent.

A merge artifact from an earlier edit. Removing them leaves behavior
unchanged; the file's 93 tests still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(dflash): scale the local first-chunk watchdog with context; prove the prefix

Two halves of the same deep-session failure.

WATCHDOG. The local DFlash first-chunk budget fell through to a step ladder
(>100k -> 300s, >50k -> 240s, ...) that flat-capped at 300s, so a 200k-token
turn on a 262k-window model got the same deadline as a 101k one. A ~95k-token
turn landed in the 240s bucket and had its connection killed mid-prefill while
the server was still healthily working. The budget is now continuous:
base + per-1k prefill cost, widen-only.

Only the per-1k prefill term generalizes — that is arithmetic, and a bigger
prompt takes proportionally longer on any backend. The cold-start allowance
encodes a measurement of one deployment (llama-swap model load on taro,
138.5s) and stays gated to the managed-W2 route it was measured on, along with
that route's 360s floor. So this widens every local DFlash lane by its real
prefill cost without lending one lane's number to another.

Adds `first_chunk_timeout_seconds` (per-model, then per-provider) so an
operator can pin the pre-first-chunk phase directly. It outranks
`stale_timeout_seconds` because the two measure different things: waiting for
the FIRST chunk is queue admission + model load + prefill of the whole prompt,
while the stale timeout measures the gap between chunks once generation is
under way. A lane can legitimately want minutes for one and seconds for the
other, so declaring both must not be contradictory.

PROOF. tests/agent/test_request_prefix_stability.py drives 12 real turns with
tool calls against an in-process HTTP provider and asserts every request is a
byte-identical extension of the one before it — under a long-lived agent AND
under a fresh agent per turn reloading history from the store, which is the
crash / app-update / gateway path. It also pins the system-prompt bytes and
tool-schema bytes constant across turns, and the goal-loop continuation as
append-only using the real template.

That is the measurement that rules the client out: the serialized prefix does
not drift turn to turn, so a re-prefill on a stable prefix is the server's KV
slot being taken, not our payload changing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(timeouts): resolve custom-endpoint attribution without a per-turn deepcopy

`find_custom_provider_identity` calls `load_config()`, which deepcopies the
whole config — and load_config's own docstring names
`get_provider_request_timeout` as the per-API-turn hot spot that must use
`load_config_readonly()` precisely to avoid that. Routing bare-custom
attribution through the general helper put the deepcopy right back on that
path, for every turn of every session on a user-declared endpoint.

Match the endpoint against the providers dict `_provider_config` has already
loaded. A timeout can only be read from a `providers:` entry anyway, so when
one owns the URL there is nothing left for the general helper to find; it stays
as the fallback for the `custom_providers:`-only shape and the no-base_url
case. Requires exactly one owner, the same rule #330 established — rows with
distinct credentials can share an endpoint and none can claim it alone.

URL normalization mirrors `runtime_provider._normalize_base_url_for_match` so
the fast path and the general helper can never disagree about whether two URLs
are the same endpoint.

Also documents `first_chunk_timeout_seconds` in cli-config.yaml.example (with
why it outranks `stale_timeout_seconds`, and that these must be declared under
the named provider entry rather than "custom"), and adds it to the known
provider keys in config.py — without which setting it warns on every load.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(dflash): correct the idempotency claim on the prefill-scaled timeout

f(payload, f(payload, base)) adds the prefill term twice, so base must always
be a fixed starting point — a constant or a configured value — never a figure
this function already produced. Both callers pass _DFLASH_LOCAL_TIMEOUT_DEFAULT_S
or the legacy override, so neither double-counts today; the docstring was the
only thing that would have let a future caller do it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…nd scale the local first-chunk watchdog (#342)

* fix(timeouts): resolve provider timeouts under the endpoint's real config entry

`resolve_runtime_provider()` reports EVERY user-declared endpoint as the bare
string "custom" — the resolved billing class, not a routable identity (#330).
Timeout resolution keyed straight off that runtime id, so it looked up
`providers["custom"]`, a key that by construction never exists, and reported
"nothing configured". `providers.<name>.stale_timeout_seconds` and
`request_timeout_seconds` were therefore silently unreachable on exactly the
local endpoints whose long cold prefills are the reason those knobs exist.

Both resolvers now take the live base_url and, on a bare-custom provider,
attribute it to the `providers:` entry that owns the endpoint — the same
endpoint-identity rule #330 established for the picker, via the existing
`find_custom_provider_identity` / `canonical_custom_identity` helpers whose
docstring already requires every persist/restore path to do this.

Attribution is by endpoint identity whenever a base_url is available, and only
falls back to `config.model.provider` when there is no endpoint to match on.
`canonical_custom_identity` always permits that fallback, which is right for
credential recovery and wrong here: a genuinely ad-hoc endpoint would otherwise
inherit an unrelated provider's timeouts.

The reverse lookup runs only when the direct key misses AND the provider is
bare custom, so named and built-in providers keep their existing single
dict-lookup cost on every API turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* feat(diagnostics): env-gated per-turn request-prefix hash with divergence offset

When a long session starts re-prefilling from scratch every turn, the client
could not answer the only question that matters: did WE rewrite the prefix, or
did something evict the server's cache? Provider-reported cached_tokens tells
you a miss happened, never why.

HERMES_PREFIX_PROBE=1 records a rolling hash over each outbound request's
cacheable prefix — tool schemas first (chat templates render them ahead of the
conversation), then one element per message in wire order — and logs where it
first differs from the previous call in the same session, by element index,
role label and reusable character count.

Cumulative rather than per-element hashes, so the earliest differing index IS
the first prefix divergence: the point past which no cached work survives. A
pure append reports no divergence at all, which is the append-only contract a
healthy agentic turn must satisfy. Serialization sorts dict keys so key
ordering — which no chat template depends on and Python does not guarantee
across rebuilds — cannot masquerade as real drift. Sampling params are excluded
since they do not participate in the cached prefix.

State is file-backed under the session log dir rather than held on the agent,
because the cases worth catching are exactly the ones where the agent object
does not survive: a compute-host crash, an app update, a fresh per-turn agent
on the gateway path.

Hooked at the existing preflight seam in conversation_loop, where api_kwargs is
final and the request has not left yet. Gated by an env var alongside its
sibling HERMES_DUMP_REQUESTS two lines up; ships off, and every path is wrapped
so a diagnostic can never fail a turn.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(provider-parity): drop dead tuple assignments in _make_agent

Three statements sat between the kwargs assembly and the AIAgent
construction in _make_agent. Each trailing comma made them 1-element
tuple assignments to locals: base_url shadowed the function parameter
and api_key was a fresh unused local. Both ran after kwargs had already
captured the real values, so neither reached the returned AIAgent.

A merge artifact from an earlier edit. Removing them leaves behavior
unchanged; the file's 93 tests still pass.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* fix(dflash): scale the local first-chunk watchdog with context; prove the prefix

Two halves of the same deep-session failure.

WATCHDOG. The local DFlash first-chunk budget fell through to a step ladder
(>100k -> 300s, >50k -> 240s, ...) that flat-capped at 300s, so a 200k-token
turn on a 262k-window model got the same deadline as a 101k one. A ~95k-token
turn landed in the 240s bucket and had its connection killed mid-prefill while
the server was still healthily working. The budget is now continuous:
base + per-1k prefill cost, widen-only.

Only the per-1k prefill term generalizes — that is arithmetic, and a bigger
prompt takes proportionally longer on any backend. The cold-start allowance
encodes a measurement of one deployment (llama-swap model load on taro,
138.5s) and stays gated to the managed-W2 route it was measured on, along with
that route's 360s floor. So this widens every local DFlash lane by its real
prefill cost without lending one lane's number to another.

Adds `first_chunk_timeout_seconds` (per-model, then per-provider) so an
operator can pin the pre-first-chunk phase directly. It outranks
`stale_timeout_seconds` because the two measure different things: waiting for
the FIRST chunk is queue admission + model load + prefill of the whole prompt,
while the stale timeout measures the gap between chunks once generation is
under way. A lane can legitimately want minutes for one and seconds for the
other, so declaring both must not be contradictory.

PROOF. tests/agent/test_request_prefix_stability.py drives 12 real turns with
tool calls against an in-process HTTP provider and asserts every request is a
byte-identical extension of the one before it — under a long-lived agent AND
under a fresh agent per turn reloading history from the store, which is the
crash / app-update / gateway path. It also pins the system-prompt bytes and
tool-schema bytes constant across turns, and the goal-loop continuation as
append-only using the real template.

That is the measurement that rules the client out: the serialized prefix does
not drift turn to turn, so a re-prefill on a stable prefix is the server's KV
slot being taken, not our payload changing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(timeouts): resolve custom-endpoint attribution without a per-turn deepcopy

`find_custom_provider_identity` calls `load_config()`, which deepcopies the
whole config — and load_config's own docstring names
`get_provider_request_timeout` as the per-API-turn hot spot that must use
`load_config_readonly()` precisely to avoid that. Routing bare-custom
attribution through the general helper put the deepcopy right back on that
path, for every turn of every session on a user-declared endpoint.

Match the endpoint against the providers dict `_provider_config` has already
loaded. A timeout can only be read from a `providers:` entry anyway, so when
one owns the URL there is nothing left for the general helper to find; it stays
as the fallback for the `custom_providers:`-only shape and the no-base_url
case. Requires exactly one owner, the same rule #330 established — rows with
distinct credentials can share an endpoint and none can claim it alone.

URL normalization mirrors `runtime_provider._normalize_base_url_for_match` so
the fast path and the general helper can never disagree about whether two URLs
are the same endpoint.

Also documents `first_chunk_timeout_seconds` in cli-config.yaml.example (with
why it outranks `stale_timeout_seconds`, and that these must be declared under
the named provider entry rather than "custom"), and adds it to the known
provider keys in config.py — without which setting it warns on every load.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(dflash): correct the idempotency claim on the prefill-scaled timeout

f(payload, f(payload, base)) adds the prefill term twice, so base must always
be a fixed starting point — a constant or a configured value — never a figure
this function already produced. Both callers pass _DFLASH_LOCAL_TIMEOUT_DEFAULT_S
or the legacy override, so neither double-counts today; the docstring was the
only thing that would have let a future caller do it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* docs(config): document the pre-first-chunk timeout beside its siblings

`first_chunk_timeout_seconds` was documented only in
cli-config.yaml.example. The timeout table in the configuration guide is
where users actually look when a local turn is being killed, and it listed
every other layer but this one.

Also states the rule that trips people up: these keys go under the NAMED
provider entry, not under `custom`. Every user-declared endpoint resolves to
the billing class `custom` at runtime, and Hermes maps it back to the entry
that owns the endpoint URL — so a `providers.custom:` block would never be
consulted.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
OmarB97 added a commit that referenced this pull request Aug 2, 2026
…on (#344)

* fix(runtime): match the managed-W2 allowlist against the recovered provider name

`_is_managed_local_w2_route` gates the 360s managed-W2 first-chunk floor and
the cold-start-inclusive context budget. It compared `agent.provider` against
`_MANAGED_W2_PROVIDERS`, a set of `providers:` config-entry names. Those two
never meet: both return sites of
`hermes_cli/runtime_provider.py::_resolve_named_custom_runtime` hardcode
`"provider": "custom"` for every user-declared endpoint, keeping the entry name
only in `requested_provider` / `source`. So the floor has never applied in
production, on any lane.

Confirmed end to end against a temp HERMES_HOME declaring `providers.ai-router`:
`resolve_runtime_provider(requested="ai-router")` returns `provider='custom'`,
and `_is_managed_local_w2_route(that agent, "deepseek-v4-flash-w2")` was False.
A previous fix added `ai-router` to the allowlist for exactly this symptom and
was itself dead code; nothing went red because every test assigns
`provider = "ai-router"` straight onto a fake agent, bypassing resolution.

The fix recovers the config-entry name the way `hermes_cli/timeouts.py`
already does for provider timeouts, and matches the allowlist against that:

- `hermes_cli/timeouts.py` gains `resolve_provider_config_key()`, a public
  wrapper over the existing `_recover_custom_provider_key`. The private name is
  deliberately kept — `test_named_provider_lookup_bypasses_custom_recovery_path`
  monkeypatches it, and the wrapper calls through so that patch still bites.
  The wrapper loads the `providers:` section when the caller has not, which is
  what engages the exactly-one-owner rule (#330): two entries declaring the
  same endpoint means neither owns it and recovery returns None.
- `_is_managed_local_w2_route` now tests the model FIRST and returns early, so
  a non-W2 turn never pays for the config read; then the direct allowlist check
  (a bare entry name still matches, keeping stubbed callers valid); then
  recovery.

Narrowness is preserved deliberately. Recovery only ever returns a name a
configured entry actually owns, so an arbitrary LAN provider serving a
W2-named model stays False. Recovery is by ENDPOINT IDENTITY and requires a
base_url: with none, `resolve_provider_config_key` would fall back to
`config.model.provider`, which would hand this lane's hardcoded timing
exception to an endpoint we cannot identify — so no base_url means not this
lane. The recovery is wrapped so a malformed config can never raise into a
request path.

Effect on the real desktop lane (bare `custom` + ai-router endpoint, W2):
first-chunk budget 180s -> 360s on a small prompt, 543s -> 723s at 90.7k
tokens. The between-chunk stale timeout is unchanged.

The comment block above `_MANAGED_W2_PROVIDERS` is rewritten to record that
these are recovered config-entry names, not `agent.provider` values, so the
next person does not re-add a slug that silently does nothing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test(runtime): pin the managed-W2 route to a RESOLVED agent, not a stubbed one

Every test of the managed-W2 route assigned `provider = "ai-router"` straight
onto a fake agent. Runtime never produces that value, so the whole allowlist
could be — and was — dead code while the suite stayed green.

Re-derived, with justification:

- `test_declared_endpoint_resolves_to_the_bare_custom_billing_class` asserted
  `_is_managed_local_w2_route(...) is False` for an agent built through real
  resolution, pinning the BUG as documentation. Renamed to
  `test_bare_custom_billing_class_is_still_a_managed_w2_route` and flipped to
  `is True`. The `agent.provider == "custom"` half is kept verbatim: that
  premise is still a fact, only the conclusion drawn from it changed.
- `test_real_resolution_still_gets_a_context_scaled_first_chunk_budget`
  asserted 543.0 at 90.7k tokens — the GENERIC budget, correct only while the
  managed branch was unreachable. Split in two, because one number was
  conflating two things:
    * `test_real_resolution_reaches_the_managed_w2_floor` uses a small prompt so
      the 360s floor is isolated (at 90k the context term dominates and would
      mask a floor that never applied);
    * `test_real_resolution_gets_the_cold_start_inclusive_budget` re-derives
      90.7k as 723.0 and asserts the 180.0 delta from the old 543.0 IS the
      measured llama-swap cold start.
  Both keep the "built through the real resolver" shape, which is the part that
  stops this regressing.
- `test_ai_router_is_recognised_as_a_managed_w2_route`: assertion untouched
  (the direct bare-entry-name branch is still a real contract). Its docstring
  said "the provider allowlist missed the one the desktop actually uses",
  which is the exact misunderstanding that produced the dead code — rewritten
  to say this covers the STUBBED shape only.

The deliberate narrowness is unchanged and still enforced by
`test_an_arbitrary_lan_provider_is_NOT_a_managed_w2_route`,
`test_remote_endpoint_is_not_a_managed_w2_route`,
`test_non_w2_model_is_not_a_managed_w2_route` and
`test_managed_local_w2_timeout_floor_is_route_and_model_specific`, all passing
unmodified.

New coverage:

- `custom:<name>` runtime id also matches (carries the entry name outright).
- A configured but NOT allowlisted endpoint (`ko-3090`) recovers its name and
  still gets the generic 180s deadline.
- Two `providers:` entries sharing one base_url are ambiguous, so recovery
  returns nothing (exactly-one-owner, #330).
- Documented choice, `test_no_base_url_refuses_the_config_provider_fallback`:
  with no base_url the underlying helper WOULD recover an allowlisted name
  from `config.model.provider` (asserted, so the refusal cannot silently
  become unreachable), and this route check refuses it anyway. The floor is a
  hardcoded exception for specific hardware; granting it to an endpoint we
  cannot identify is how an ad-hoc endpoint would inherit it.
- Cost guard: a non-W2 model must not reach the lookup. Verified to bite —
  moving the model test after the recovery turns it red with
  "config lookup ran for a non-W2 model". A named provider likewise never
  pays, and a raising recovery degrades to the generic deadline.
- `tests/hermes_cli/test_timeouts.py` covers `resolve_provider_config_key`
  directly, including that it stays a delegating wrapper — the private
  `_recover_custom_provider_key` name is monkeypatched by an existing test,
  so inlining it would silently defeat that patch.

Red-check: reverting `_is_managed_local_w2_route` to its pre-fix body fails 5
of these; the suite before this change did not fail at all.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* style(runtime): one logger.debug argument per line

Cosmetic only — no behavior change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Omar Baradei <omar@kostudios.io>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant