fix(coding-agent): custom models for built-in providers and list-models error surfacing (#3072)
* fix(coding-agent): surface models.json load errors on stderr in --list-models When models.json has validation errors, --list-models silently discarded custom models and overrides without any user-visible feedback. Now prints the error to stderr via chalk.yellow warning before listing available models. * fix(coding-agent): allow custom models for built-in providers in models.json Built-in providers (openrouter, anthropic, etc.) already have api and baseUrl on every model, and auth comes from env vars / auth storage. Relax validation so custom models under built-in providers don't need redundant baseUrl, apiKey, or api fields. Inherit them from the first built-in model for that provider. fixes #2921 --------- Co-authored-by: Mario Zechner <badlogicgames@gmail.com>
This commit is contained in:
@@ -38,7 +38,9 @@ How to disable it:
|
||||
|
||||
- Full `openRouterRouting` support in `models.json`, including fallbacks, parameter requirements, data collection, ZDR, ignore lists, quantizations, provider sorting, max price, and preferred throughput and latency constraints. See [docs/models.md](docs/models.md).
|
||||
- `PI_CODING_AGENT=true` environment variable set at startup so subprocesses can detect they are running inside the coding agent.
|
||||
|
||||
- Updated `antigravity-image-gen.ts` example extension to use User-Agent version `1.21.9` ([#2901](https://github.com/badlogic/pi-mono/pull/2901) by [@aadishv](https://github.com/aadishv))
|
||||
- Fixed `--list-models` silently swallowing `models.json` load errors; errors are now printed to stderr ([#3072](https://github.com/badlogic/pi-mono/issues/3072))
|
||||
- Fixed custom models for built-in providers (e.g. `openrouter`) being silently dropped from `--list-models` by inheriting `api`/`baseUrl` from built-in model definitions and no longer requiring `apiKey` for providers with existing auth ([#2921](https://github.com/badlogic/pi-mono/issues/2921) and [#3072](https://github.com/badlogic/pi-mono/issues/3072))
|
||||
### Added
|
||||
|
||||
- Added full `openRouterRouting` field support in `models.json`, including fallbacks, parameter requirements, data collection, ZDR, ignore lists, quantizations, provider sorting, max price, and preferred throughput and latency constraints ([#2904](https://github.com/badlogic/pi-mono/pull/2904) by [@zmberber](https://github.com/zmberber))
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
|
||||
import type { Api, Model } from "@mariozechner/pi-ai";
|
||||
import { fuzzyFilter } from "@mariozechner/pi-tui";
|
||||
import chalk from "chalk";
|
||||
import type { ModelRegistry } from "../core/model-registry.js";
|
||||
|
||||
/**
|
||||
@@ -25,6 +26,11 @@ function formatTokenCount(count: number): string {
|
||||
* List available models, optionally filtered by search pattern
|
||||
*/
|
||||
export async function listModels(modelRegistry: ModelRegistry, searchPattern?: string): Promise<void> {
|
||||
const loadError = modelRegistry.getError();
|
||||
if (loadError) {
|
||||
console.error(chalk.yellow(`Warning: errors loading models.json:\n${loadError}`));
|
||||
}
|
||||
|
||||
const models = modelRegistry.getAvailable();
|
||||
|
||||
if (models.length === 0) {
|
||||
|
||||
@@ -463,7 +463,10 @@ export class ModelRegistry {
|
||||
}
|
||||
|
||||
private validateConfig(config: ModelsConfig): void {
|
||||
const builtInProviders = new Set<string>(getProviders());
|
||||
|
||||
for (const [providerName, providerConfig] of Object.entries(config.providers)) {
|
||||
const isBuiltIn = builtInProviders.has(providerName);
|
||||
const hasProviderApi = !!providerConfig.api;
|
||||
const models = providerConfig.models ?? [];
|
||||
const hasModelOverrides =
|
||||
@@ -476,8 +479,8 @@ export class ModelRegistry {
|
||||
`Provider ${providerName}: must specify "baseUrl", "compat", "modelOverrides", or "models".`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// Custom models are merged into provider models and require endpoint + auth.
|
||||
} else if (!isBuiltIn) {
|
||||
// Non-built-in providers with custom models require endpoint + auth.
|
||||
if (!providerConfig.baseUrl) {
|
||||
throw new Error(`Provider ${providerName}: "baseUrl" is required when defining custom models.`);
|
||||
}
|
||||
@@ -485,15 +488,18 @@ export class ModelRegistry {
|
||||
throw new Error(`Provider ${providerName}: "apiKey" is required when defining custom models.`);
|
||||
}
|
||||
}
|
||||
// Built-in providers with custom models: baseUrl/apiKey/api are optional,
|
||||
// inherited from built-in models. Auth comes from env vars / auth storage.
|
||||
|
||||
for (const modelDef of models) {
|
||||
const hasModelApi = !!modelDef.api;
|
||||
|
||||
if (!hasProviderApi && !hasModelApi) {
|
||||
if (!hasProviderApi && !hasModelApi && !isBuiltIn) {
|
||||
throw new Error(
|
||||
`Provider ${providerName}, model ${modelDef.id}: no "api" specified. Set at provider or model level.`,
|
||||
);
|
||||
}
|
||||
// For built-in providers, api is optional — inherited from built-in models.
|
||||
|
||||
if (!modelDef.id) throw new Error(`Provider ${providerName}: model missing "id"`);
|
||||
// Validate contextWindow/maxTokens only if provided (they have defaults)
|
||||
@@ -507,27 +513,43 @@ export class ModelRegistry {
|
||||
|
||||
private parseModels(config: ModelsConfig): Model<Api>[] {
|
||||
const models: Model<Api>[] = [];
|
||||
const builtInProviders = new Set<string>(getProviders());
|
||||
|
||||
// Cache built-in defaults (api, baseUrl) per provider, extracted from first model.
|
||||
const builtInDefaultsCache = new Map<string, { api: string; baseUrl: string }>();
|
||||
const getBuiltInDefaults = (providerName: string): { api: string; baseUrl: string } | undefined => {
|
||||
if (!builtInProviders.has(providerName)) return undefined;
|
||||
if (builtInDefaultsCache.has(providerName)) return builtInDefaultsCache.get(providerName);
|
||||
const builtIn = getModels(providerName as KnownProvider) as Model<Api>[];
|
||||
if (builtIn.length === 0) return undefined;
|
||||
const defaults = { api: builtIn[0].api, baseUrl: builtIn[0].baseUrl };
|
||||
builtInDefaultsCache.set(providerName, defaults);
|
||||
return defaults;
|
||||
};
|
||||
|
||||
for (const [providerName, providerConfig] of Object.entries(config.providers)) {
|
||||
const modelDefs = providerConfig.models ?? [];
|
||||
if (modelDefs.length === 0) continue; // Override-only, no custom models
|
||||
|
||||
const builtInDefaults = getBuiltInDefaults(providerName);
|
||||
|
||||
for (const modelDef of modelDefs) {
|
||||
const api = modelDef.api || providerConfig.api;
|
||||
const api = modelDef.api ?? providerConfig.api ?? builtInDefaults?.api;
|
||||
if (!api) continue;
|
||||
|
||||
const baseUrl = modelDef.baseUrl ?? providerConfig.baseUrl ?? builtInDefaults?.baseUrl;
|
||||
if (!baseUrl) continue;
|
||||
|
||||
const compat = mergeCompat(providerConfig.compat, modelDef.compat);
|
||||
this.storeModelHeaders(providerName, modelDef.id, modelDef.headers);
|
||||
|
||||
// Provider baseUrl is required when custom models are defined.
|
||||
// Individual models can override it with modelDef.baseUrl.
|
||||
const defaultCost = { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 };
|
||||
models.push({
|
||||
id: modelDef.id,
|
||||
name: modelDef.name ?? modelDef.id,
|
||||
api: api as Api,
|
||||
provider: providerName,
|
||||
baseUrl: modelDef.baseUrl ?? providerConfig.baseUrl!,
|
||||
baseUrl,
|
||||
reasoning: modelDef.reasoning ?? false,
|
||||
input: (modelDef.input ?? ["text"]) as ("text" | "image")[],
|
||||
cost: modelDef.cost ?? defaultCost,
|
||||
|
||||
@@ -192,6 +192,49 @@ describe("ModelRegistry", () => {
|
||||
});
|
||||
|
||||
describe("custom models merge behavior", () => {
|
||||
test("built-in provider custom models inherit api and baseUrl without explicit fields", () => {
|
||||
// Built-in providers already have api/baseUrl on every model, and auth
|
||||
// comes from env vars / auth storage. No need to specify them.
|
||||
writeRawModelsJson({
|
||||
openrouter: {
|
||||
models: [
|
||||
{
|
||||
id: "fake-provider/fake-model",
|
||||
name: "Fake model",
|
||||
reasoning: true,
|
||||
input: ["text"],
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
const registry = ModelRegistry.create(authStorage, modelsJsonPath);
|
||||
expect(registry.getError()).toBeUndefined();
|
||||
|
||||
const model = registry.find("openrouter", "fake-provider/fake-model");
|
||||
expect(model).toBeDefined();
|
||||
expect(model?.api).toBe("openai-completions");
|
||||
expect(model?.baseUrl).toBe("https://openrouter.ai/api/v1");
|
||||
});
|
||||
|
||||
test("non-built-in provider custom models still require baseUrl and apiKey", () => {
|
||||
writeRawModelsJson({
|
||||
"my-custom-provider": {
|
||||
models: [
|
||||
{
|
||||
id: "my-model",
|
||||
api: "openai-completions",
|
||||
reasoning: false,
|
||||
input: ["text"],
|
||||
},
|
||||
],
|
||||
},
|
||||
});
|
||||
|
||||
const registry = ModelRegistry.create(authStorage, modelsJsonPath);
|
||||
expect(registry.getError()).toContain("baseUrl");
|
||||
});
|
||||
|
||||
test("custom provider with same name as built-in merges with built-in models", () => {
|
||||
writeModelsJson({
|
||||
anthropic: providerConfig("https://my-proxy.example.com/v1", [{ id: "claude-custom" }]),
|
||||
|
||||
Reference in New Issue
Block a user