refactor: gadfly round 2 — both halves of an OpenAI-compat built-in register together
CI / Tidy (pull_request) Successful in 9m25s
CI / Build & Test (pull_request) Successful in 9m50s

Same class of finding as round 1, one level in: I factored the DSN-scheme half
of the kimi/qwen duplication into openaiCompatScheme and left the eager
provider half copy-pasted, so a third built-in still had six lines to clone —
including both credential rules, which is exactly the pair you do not want
re-typed.

registerOpenAICompatBuiltin now installs both halves from one call. The rules
that matter hold by construction for every future caller: WithAPIKey passed
unconditionally (an unset key must not fall through to OPENAI_API_KEY), and
WithAPIKeyName naming that same variable in the 401 hint. Registering kimi and
qwen is now one line each.

Also fixed a cross-reference the ADR got wrong: Qwen's image-input caveat is
README matrix footnote ⁴, not ³ — ³ is kimi's. I wrote "³, shared with kimi"
in the ADR and then gave Qwen its own footnote in the README.

The break-check harness needed fixing before any of this could be trusted:
three of its mutations targeted lines this refactor moved, so they matched
nothing, the code was never broken, and the suite reported "test still passed"
— identical output to a test that genuinely misses the bug. Mutations are now
verified to have landed (sha before/after) and the suite fails loudly if one
doesn't. Two new cases cover the helper: dropping the unconditional WithAPIKey,
and dropping the scheme-half registration. 8/8 apply and are caught.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
This commit is contained in:
2026-08-12 16:25:38 -04:00
co-authored by Claude Opus 5
parent 31d6b59356
commit f1f2b653c3
2 changed files with 37 additions and 34 deletions
+36 -33
View File
@@ -72,6 +72,29 @@ func openaiCompatScheme(wrap func(...openai.Option) []openai.Option) SchemeFacto
}
}
// registerOpenAICompatBuiltin installs BOTH halves of an OpenAI-compat
// built-in: the eager provider under name (credential from keyEnv) and the
// matching name:// DSN scheme. Why both in one call: the two halves are a pair
// — a built-in whose scheme is missing resolves as a spec but not from an
// LLM_* DSN, and the credential rules below have to hold identically in each.
// Adding the next one is a single line rather than six lines to copy.
//
// The two credential rules, holding by construction for every caller:
// - WithAPIKey is passed UNCONDITIONALLY, even when the lookup comes back
// empty. openai.New defaults its key to OPENAI_API_KEY, so anything less
// lets an unset keyEnv silently authenticate as OpenAI.
// - WithAPIKeyName makes the synthetic-401 hint name keyEnv, so a keyless
// call tells the operator the variable that actually fixes it.
func registerOpenAICompatBuiltin(r *Registry, wrap func(...openai.Option) []openai.Option, name, baseURL, keyEnv string) {
r.providers[name] = openai.New(wrap(
openai.WithName(name),
openai.WithBaseURL(baseURL),
openai.WithAPIKey(r.envLookup(keyEnv)),
openai.WithAPIKeyName(keyEnv),
)...)
r.schemes[name] = openaiCompatScheme(wrap)
}
// registerBuiltins installs the built-in providers and env-DSN scheme
// factories into a fresh registry. httpClient, when non-nil, is used by
// every provider and factory the registry itself constructs.
@@ -118,39 +141,19 @@ func registerBuiltins(r *Registry, httpClient *http.Client) {
)...), nil
}
// Kimi (Moonshot AI): OpenAI-compatible Chat Completions, so it reuses the
// openai client (like llama-swap). Defaults to Moonshot's international
// endpoint and the KIMI_API_KEY credential. WithAPIKey is passed
// unconditionally — even empty — so an unset KIMI_API_KEY can never fall
// through to the openai client's OPENAI_API_KEY default; WithAPIKeyName
// makes the missing-key error name KIMI_API_KEY.
r.providers[ProviderKimi] = openai.New(openaiOpts(
openai.WithName(ProviderKimi),
openai.WithBaseURL(kimiBaseURL),
openai.WithAPIKey(r.envLookup("KIMI_API_KEY")),
openai.WithAPIKeyName("KIMI_API_KEY"),
)...)
// kimi:// DSN scheme: an OpenAI-compatible target labeled kimi, base URL
// from the DSN host (e.g. kimi://[email protected]/v1 for China).
r.schemes[ProviderKimi] = openaiCompatScheme(openaiOpts)
// Qwen (Alibaba Model Studio): same shape as kimi — an OpenAI-compatible
// Chat Completions endpoint, so it reuses the openai client rather than a
// new package. Model Studio also exposes an Anthropic-compatible endpoint;
// ADR-0027 records why the OpenAI one is the built-in. Same unconditional
// WithAPIKey + WithAPIKeyName discipline as kimi: an unset QWEN_API_KEY
// must never fall through to OPENAI_API_KEY, and the missing-key error
// must name the variable the operator actually has to set.
r.providers[ProviderQwen] = openai.New(openaiOpts(
openai.WithName(ProviderQwen),
openai.WithBaseURL(qwenBaseURL),
openai.WithAPIKey(r.envLookup("QWEN_API_KEY")),
openai.WithAPIKeyName("QWEN_API_KEY"),
)...)
// qwen:// DSN scheme: an OpenAI-compatible target labeled qwen on any
// Model Studio host (e.g. qwen://[email protected]/compatible-mode/v1
// for China, or a workspace-scoped regional host).
r.schemes[ProviderQwen] = openaiCompatScheme(openaiOpts)
// Third-party endpoints that ARE the openai client at another base URL —
// no new package, mirroring llama-swap's chat path. Each gets the eager
// built-in plus its name:// DSN scheme, and the credential rules hold by
// construction (see registerOpenAICompatBuiltin).
//
// kimi (ADR-0026): Moonshot's international endpoint; China host via
// kimi://[email protected]/v1.
registerOpenAICompatBuiltin(r, openaiOpts, ProviderKimi, kimiBaseURL, "KIMI_API_KEY")
// qwen (ADR-0027): Alibaba Model Studio's international host. Model Studio
// also exposes an Anthropic-compatible endpoint; the ADR records why the
// OpenAI one is the built-in. China / workspace-scoped regional hosts via
// qwen://[email protected]/compatible-mode/v1.
registerOpenAICompatBuiltin(r, openaiOpts, ProviderQwen, qwenBaseURL, "QWEN_API_KEY")
// llama-swap: OpenAI-compatible chat + image generation + management
// endpoints over a model-swapping proxy. Chat reuses the openai client
+1 -1
View File
@@ -87,7 +87,7 @@ blocks there too), where every OpenAI-compat target would get them.
- Chat, streaming, tools, structured output, reasoning effort, and cached-token
accounting all ride the openai client and inherit its fixes.
- Image *inputs* work at the client level, but only the `qwen-vl-*` /
`qwen3-vl-*` models accept them (matrix footnote ³, shared with kimi).
`qwen3-vl-*` models accept them (matrix footnote ⁴; ³ is kimi's).
- Two model-side quirks are Alibaba's, not majordomo's, and are left to the
caller rather than papered over: thinking is **on by default** on some models
(e.g. `qwen3.7-plus`), and Qwen3 *open-source* models require streaming when