Coalesce concurrent identical model-discovery requests - #115
Merged
Merged
Conversation
When the same backend is configured under two provider ids (the built-in litellm provider plus a custom type:"litellm" entry at the same gateway), a discovery pass issued identical duplicate HTTP requests concurrently. ProviderRegistry now owns an in-flight-only coalescer: opted-in fetchers (LiteLLM's two registrars) join a concurrent identical request through the narrow coalesceModelRequest() method and share one decoded immutable payload, then run their own parse/stamp pass. - Identity: operation namespace + method + normalized URL + a SHA-256 fingerprint of the effective headers (raw secrets are never retained as map keys or logged). - The executor receives only a coalescer-owned AbortSignal; the flight aborts once every participant detaches, bounded by callers' existing fetcher deadlines (no second caller-owned timer). - In-flight only: completed results stay in each fetcher's own TTL cache; failure/timeout is never retained; settlement cleanup is identity-safe. - clearModelCache(providerId), clearAllModelCaches(), and re-registration retire flights so post-clear calls never join pre-clear ones; attached callers finish, and clearing one provider never cancels another's caller on a shared flight. No disposal API — per-registry isolation replaces it.
The coalescer's header fingerprint used node:crypto, which is reachable from @assistant/core's browser-facing public entry via ProviderRegistry and broke the platform:"browser" bundle (guarded by the parent repo's core-browser-public-entry and positron-webview-build-options tests). WebCrypto's async digest would force an async fingerprint through identity construction, so use a small synchronous pure-TS SHA-256 (sha256.ts) with NIST/RFC 6234 known-answer vector tests.
Replace participant-based flight retirement with monotonic invalidation barriers: each flight is stamped with a creation sequence, provider clear/re-registration records a per-provider barrier, and clear-all records a shared barrier. A provider now joins only flights newer than its barrier, so a post-clear call can no longer join a pre-clear flight it had no caller on and cache the stale payload for the fetcher TTL. Also remove a flight's map entry before aborting it once every caller has detached (and reject joins to already-aborted flights), so an abort-insensitive executor that never settles can no longer leave the aborted flight joinable and doom every later call to join it and time out. Move requestCoalescer onto CachedModelFetcherRequestConfig so the fetchFresh variant — which bypasses it at runtime — rejects it at compile time, and fix the test helper's boolean waits to use vi.waitUntil (vi.waitFor retries only on thrown errors, so the boolean waits could resolve early).
The structural union of the request variant and the provider-owned fetchFresh variant was not exclusive: an object carrying fetchFresh plus request-only fields (apiUrl, createHeaders, requestCoalescer, ...) could satisfy the union through an intermediate variable while the runtime variant check silently ignored one side. Add never-typed exclusion fields on both variants and move the runtime check behind a type predicate, since the exclusion fields defeat in-operator narrowing. Runtime semantics are unchanged.
Extend the registry-owned in-flight request coalescer beyond LiteLLM to every provider whose discovery is a single GET: openai-compatible, openai, ollama, lmstudio, openrouter, anthropic, deepseek, and gemini. The same backend configured under two provider ids now issues one base-list request regardless of provider type; enrichment passes (Ollama's per-model /api/show) still run per provider afterward. Providers that carry a credential in the request URL (Gemini's ?key=) keep it out of the coalescer's retained identity key through two new fetcher hooks: identityUrl strips the query parameter, and identityHeaders folds the secret into the hashed header fingerprint. fetchFresh providers (portkey, databricks) have no base request to coalesce; singleton-only providers (opencode) have no duplicate-entry scenario. Covered by a parameterized per-provider wiring test plus a secret-hygiene test for the identity hooks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the same backend is configured under two provider entries — say the built-in provider plus a custom entry pointing at one gateway, or two custom entries on one server — hosts launch all provider model fetches in parallel, producing identical concurrent requests. This PR extends the registry-owned, in-flight-only request coalescer from LiteLLM to every provider whose discovery is a single GET: openai-compatible, openai, ollama, lmstudio, openrouter, anthropic, deepseek, and gemini. A second call with the same request identity — operation namespace, HTTP method, normalized URL, and a fingerprint of the auth and response-affecting headers — joins the in-flight request instead of starting a new one. Each joiner parses and labels its own results from the shared decoded payload, so per-provider behavior is unchanged.
Coalescing is deliberately conservative. Requests that differ in URL, method, headers, or credentials never share. A call made after a provider's cache is cleared never joins a flight started before the clear, so a retry always gets a fresh answer. Failures and timeouts are never retained, and settlement removes entries identity-safely, so no disposal API is needed. Raw secrets are never used as map keys or logged — the header fingerprint is a SHA-256 hash, implemented in pure TypeScript so the browser bundle keeps working (with known-answer vector tests pinning hash exactness, since the hash keys the identity map).
Two boundaries are worth knowing. Providers that carry a credential in the request URL (Gemini's
?key=query parameter) keep it out of the coalescer's retained identity key: a new fetcher hook strips the parameter from the identity URL and folds the key into the hashed fingerprint instead. And only the base model-list request joins — a provider's per-model enrichment pass (Ollama's/api/showcalls) still runs per provider afterward. Providers that own their whole fetch (Portkey's paginated discovery, Databricks) have no base request to coalesce, and singleton-only providers have no duplicate-entry scenario.The companion PR in the assistant monorepo (branch
fix-provider-hang) updates the submodule pin to this work; it should merge only after this one lands and the pin is repointed at the merged commit.