fix: prefer explicit config contextWindow/maxTokens over auto-discovered values
Problem: When a model is found in the pi-ai registry (auto-discovered from Ollama), the explicit config in clawdbot.json was completely ignored. This caused models to use the registry's contextWindow (e.g., 8192 from Ollama's /api/show) instead of the user's configured value (e.g., 131072). Error seen: low context window: ollama/glm-4.7:cloud ctx=8192 (warn<32000) source=model blocked model (context window too small): ollama/glm-4.7:cloud ctx=8192 Fix: When a model exists in both the registry AND explicit config, merge the config's contextWindow/maxTokens into the registry model. Only positive numbers are accepted as overrides. Logic: - Model not in registry → use config (existing behavior) - Model in registry + valid config contextWindow/maxTokens → merge config values - Model in registry + no valid overrides → use registry (existing behavior) test: add tests for config contextWindow/maxTokens override behavior Tests cover: - Registry model used when no config override - Config contextWindow/maxTokens override registry values - Invalid values (0, negative) are ignored, registry used instead - Config model used when not in registry - Error returned when model not found anywhere
This commit is contained in:
parent
6859e1e6a6
commit
62c3f1c100
@ -1,15 +1,45 @@
|
|||||||
import { describe, expect, it } from "vitest";
|
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
import { buildInlineProviderModels } from "./model.js";
|
vi.mock("@mariozechner/pi-coding-agent", () => ({
|
||||||
|
discoverAuthStorage: () => ({ profiles: {} }),
|
||||||
|
discoverModels: () => ({
|
||||||
|
find: (provider: string, modelId: string) => {
|
||||||
|
if (provider === "ollama" && modelId === "test-model") {
|
||||||
|
return {
|
||||||
|
id: "test-model",
|
||||||
|
name: "test-model",
|
||||||
|
provider: "ollama",
|
||||||
|
api: "openai-completions",
|
||||||
|
reasoning: false,
|
||||||
|
input: ["text"],
|
||||||
|
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||||
|
contextWindow: 8192,
|
||||||
|
maxTokens: 4096,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
}));
|
||||||
|
|
||||||
const makeModel = (id: string) => ({
|
vi.mock("../agent-paths.js", () => ({
|
||||||
|
resolveClawdbotAgentDir: () => "/tmp/test-agent",
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock("../model-compat.js", () => ({
|
||||||
|
normalizeModelCompat: <T>(model: T) => model,
|
||||||
|
}));
|
||||||
|
|
||||||
|
import { buildInlineProviderModels, resolveModel } from "./model.js";
|
||||||
|
|
||||||
|
const makeModel = (id: string, overrides?: { contextWindow?: number; maxTokens?: number }) => ({
|
||||||
id,
|
id,
|
||||||
name: id,
|
name: id,
|
||||||
reasoning: false,
|
reasoning: false,
|
||||||
input: ["text"] as const,
|
input: ["text"] as const,
|
||||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||||
contextWindow: 1,
|
contextWindow: overrides?.contextWindow ?? 1,
|
||||||
maxTokens: 1,
|
maxTokens: overrides?.maxTokens ?? 1,
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("buildInlineProviderModels", () => {
|
describe("buildInlineProviderModels", () => {
|
||||||
@ -27,3 +57,91 @@ describe("buildInlineProviderModels", () => {
|
|||||||
]);
|
]);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("resolveModel", () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
vi.clearAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses registry model when no config override exists", () => {
|
||||||
|
const result = resolveModel("ollama", "test-model");
|
||||||
|
|
||||||
|
expect(result.model).toBeDefined();
|
||||||
|
expect(result.model?.contextWindow).toBe(8192);
|
||||||
|
expect(result.model?.maxTokens).toBe(4096);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("overrides registry contextWindow with valid config value", () => {
|
||||||
|
const cfg = {
|
||||||
|
models: {
|
||||||
|
providers: {
|
||||||
|
ollama: {
|
||||||
|
models: [makeModel("test-model", { contextWindow: 131072, maxTokens: 32000 })],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const result = resolveModel("ollama", "test-model", undefined, cfg);
|
||||||
|
|
||||||
|
expect(result.model).toBeDefined();
|
||||||
|
expect(result.model?.contextWindow).toBe(131072);
|
||||||
|
expect(result.model?.maxTokens).toBe(32000);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ignores config contextWindow of 0", () => {
|
||||||
|
const cfg = {
|
||||||
|
models: {
|
||||||
|
providers: {
|
||||||
|
ollama: {
|
||||||
|
models: [makeModel("test-model", { contextWindow: 0 })],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const result = resolveModel("ollama", "test-model", undefined, cfg);
|
||||||
|
|
||||||
|
expect(result.model?.contextWindow).toBe(8192); // registry value
|
||||||
|
});
|
||||||
|
|
||||||
|
it("ignores config contextWindow of negative number", () => {
|
||||||
|
const cfg = {
|
||||||
|
models: {
|
||||||
|
providers: {
|
||||||
|
ollama: {
|
||||||
|
models: [makeModel("test-model", { contextWindow: -1 })],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const result = resolveModel("ollama", "test-model", undefined, cfg);
|
||||||
|
|
||||||
|
expect(result.model?.contextWindow).toBe(8192); // registry value
|
||||||
|
});
|
||||||
|
|
||||||
|
it("uses config model when not in registry", () => {
|
||||||
|
const cfg = {
|
||||||
|
models: {
|
||||||
|
providers: {
|
||||||
|
ollama: {
|
||||||
|
models: [makeModel("not-in-registry", { contextWindow: 65536 })],
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
};
|
||||||
|
|
||||||
|
const result = resolveModel("ollama", "not-in-registry", undefined, cfg);
|
||||||
|
|
||||||
|
expect(result.model).toBeDefined();
|
||||||
|
expect(result.model?.contextWindow).toBe(65536);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("returns error when model not in registry and no config", () => {
|
||||||
|
const result = resolveModel("ollama", "unknown-model");
|
||||||
|
|
||||||
|
expect(result.error).toContain("Unknown model");
|
||||||
|
expect(result.model).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@ -35,6 +35,20 @@ export function buildModelAliasLines(cfg?: ClawdbotConfig) {
|
|||||||
.map((entry) => `- ${entry.alias}: ${entry.model}`);
|
.map((entry) => `- ${entry.alias}: ${entry.model}`);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Find explicit model config matching provider/modelId */
|
||||||
|
function findInlineModelConfig(
|
||||||
|
cfg: ClawdbotConfig | undefined,
|
||||||
|
provider: string,
|
||||||
|
modelId: string,
|
||||||
|
): InlineModelEntry | undefined {
|
||||||
|
const providers = cfg?.models?.providers ?? {};
|
||||||
|
const inlineModels = buildInlineProviderModels(providers);
|
||||||
|
const normalizedProvider = normalizeProviderId(provider);
|
||||||
|
return inlineModels.find(
|
||||||
|
(entry) => normalizeProviderId(entry.provider) === normalizedProvider && entry.id === modelId,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
export function resolveModel(
|
export function resolveModel(
|
||||||
provider: string,
|
provider: string,
|
||||||
modelId: string,
|
modelId: string,
|
||||||
@ -49,22 +63,19 @@ export function resolveModel(
|
|||||||
const resolvedAgentDir = agentDir ?? resolveClawdbotAgentDir();
|
const resolvedAgentDir = agentDir ?? resolveClawdbotAgentDir();
|
||||||
const authStorage = discoverAuthStorage(resolvedAgentDir);
|
const authStorage = discoverAuthStorage(resolvedAgentDir);
|
||||||
const modelRegistry = discoverModels(authStorage, resolvedAgentDir);
|
const modelRegistry = discoverModels(authStorage, resolvedAgentDir);
|
||||||
const model = modelRegistry.find(provider, modelId) as Model<Api> | null;
|
const registryModel = modelRegistry.find(provider, modelId) as Model<Api> | null;
|
||||||
if (!model) {
|
const inlineConfig = findInlineModelConfig(cfg, provider, modelId);
|
||||||
const providers = cfg?.models?.providers ?? {};
|
|
||||||
const inlineModels = buildInlineProviderModels(providers);
|
// Case 1: Model not in registry - use inline config or fallback
|
||||||
const normalizedProvider = normalizeProviderId(provider);
|
if (!registryModel) {
|
||||||
const inlineMatch = inlineModels.find(
|
if (inlineConfig) {
|
||||||
(entry) => normalizeProviderId(entry.provider) === normalizedProvider && entry.id === modelId,
|
|
||||||
);
|
|
||||||
if (inlineMatch) {
|
|
||||||
const normalized = normalizeModelCompat(inlineMatch as Model<Api>);
|
|
||||||
return {
|
return {
|
||||||
model: normalized,
|
model: normalizeModelCompat(inlineConfig as Model<Api>),
|
||||||
authStorage,
|
authStorage,
|
||||||
modelRegistry,
|
modelRegistry,
|
||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
const providers = cfg?.models?.providers ?? {};
|
||||||
const providerCfg = providers[provider];
|
const providerCfg = providers[provider];
|
||||||
if (providerCfg || modelId.startsWith("mock-")) {
|
if (providerCfg || modelId.startsWith("mock-")) {
|
||||||
const fallbackModel: Model<Api> = normalizeModelCompat({
|
const fallbackModel: Model<Api> = normalizeModelCompat({
|
||||||
@ -80,11 +91,24 @@ export function resolveModel(
|
|||||||
} as Model<Api>);
|
} as Model<Api>);
|
||||||
return { model: fallbackModel, authStorage, modelRegistry };
|
return { model: fallbackModel, authStorage, modelRegistry };
|
||||||
}
|
}
|
||||||
return {
|
return { error: `Unknown model: ${provider}/${modelId}`, authStorage, modelRegistry };
|
||||||
error: `Unknown model: ${provider}/${modelId}`,
|
|
||||||
authStorage,
|
|
||||||
modelRegistry,
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Case 2: Model in registry - override contextWindow/maxTokens from config if valid
|
||||||
|
if (inlineConfig) {
|
||||||
|
const ctx = inlineConfig.contextWindow;
|
||||||
|
const max = inlineConfig.maxTokens;
|
||||||
|
const useCtx = typeof ctx === "number" && ctx > 0;
|
||||||
|
const useMax = typeof max === "number" && max > 0;
|
||||||
|
if (useCtx || useMax) {
|
||||||
|
const model: Model<Api> = {
|
||||||
|
...registryModel,
|
||||||
|
...(useCtx && { contextWindow: ctx }),
|
||||||
|
...(useMax && { maxTokens: max }),
|
||||||
|
};
|
||||||
return { model: normalizeModelCompat(model), authStorage, modelRegistry };
|
return { model: normalizeModelCompat(model), authStorage, modelRegistry };
|
||||||
}
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
return { model: normalizeModelCompat(registryModel), authStorage, modelRegistry };
|
||||||
|
}
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user