Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions @omniroute/opencode-provider/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,16 +129,22 @@ export interface OmniRouteProviderOptions {
apiKey: string;
/** Override the display name shown in OpenCode. Default: `"OmniRoute"`. */
displayName?: string;
/** Override the model catalog. Defaults to `OMNIROUTE_DEFAULT_OPENCODE_MODELS`. */
models?: readonly string[];
/** Override the model catalog. Accepts model ids (strings) or live model entries from `fetchLiveModels`. When entries carry a `contextLength`, it is used directly — no hardcoded map needed. */
models?: readonly (string | { id: string; contextLength?: number })[];
/** Optional human-readable labels keyed by model id. Overridden by `modelCapabilities[id].label`. */
modelLabels?: Record<string, string>;
/**
* Optional capability overrides keyed by model id. Merged on top of
* `OMNIROUTE_DEFAULT_MODEL_CAPABILITIES` for ids in the default catalog;
* for custom ids the override is used verbatim.
*/
modelCapabilities?: Record<string, ModelCapabilities>;
modelCapabilities?: Record<string, ModelCapabilities>;
/**
* Optional per-model context-length overrides (tokens). Takes precedence
* over the static `OMNIROUTE_DEFAULT_MODEL_CONTEXT_LENGTHS` map but is
* superseded by `contextLength` on live model entries passed via `models`.
*/
modelContextLengths?: Record<string, string | number>;
/**
* Primary model for OpenCode (top-level `model` key).
* Emitted as `"omniroute/<id>"`. When omitted the key is not written.
Expand Down Expand Up @@ -248,7 +254,12 @@ export function createOmniRouteProvider(options: OmniRouteProviderOptions): Open
const models: Record<string, OpenCodeModelEntry> = {};
const seen = new Set<string>();
for (const raw of modelList) {
const id = typeof raw === "string" ? raw.trim() : "";
const id =
typeof raw === "object" && raw !== null && "id" in raw
? (raw as { id: string }).id.trim()
: typeof raw === "string"
? raw.trim()
: "";
Comment on lines +257 to +262

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If raw is an object and contains an id property, but raw.id is not a string (e.g., a number, null, or undefined), calling .trim() on it will throw a runtime TypeError. We should explicitly verify that raw.id is a string before attempting to trim it.

Suggested change
const id =
typeof raw === "object" && raw !== null && "id" in raw
? (raw as { id: string }).id.trim()
: typeof raw === "string"
? raw.trim()
: "";
const id =
typeof raw === "object" && raw !== null && "id" in raw && typeof (raw as any).id === "string"
? (raw as { id: string }).id.trim()
: typeof raw === "string"
? raw.trim()
: "";

if (!id || seen.has(id)) continue;
seen.add(id);
const defaults = OMNIROUTE_DEFAULT_MODEL_CAPABILITIES[id] ?? {};
Expand All @@ -266,9 +277,11 @@ export function createOmniRouteProvider(options: OmniRouteProviderOptions): Open
if (typeof merged.temperature === "boolean") entry.temperature = merged.temperature;
if (typeof merged.tool_call === "boolean") entry.tool_call = merged.tool_call;

// Include context window limit when known — OpenCode reads this to
// determine usable context length for compaction & overflow detection.
const contextLength = OMNIROUTE_DEFAULT_MODEL_CONTEXT_LENGTHS[id];
// Context window: live model entry (from API catalog) > modelContextLengths > static defaults
const liveContext = typeof raw === "object" && raw !== null
? (raw as { contextLength?: number }).contextLength
: undefined;
const contextLength = liveContext ?? options.modelContextLengths?.[id] ?? OMNIROUTE_DEFAULT_MODEL_CONTEXT_LENGTHS[id];
if (typeof contextLength === "number" && contextLength > 0) {
entry.limit = { context: contextLength };
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Since options.modelContextLengths is typed as Record<string, string | number>, the values can be strings (e.g., "128000"). If a string is passed, the strict type check typeof contextLength === "number" will fail, and the context limit will be silently ignored. We should parse string values to numbers to ensure they are correctly applied.

Suggested change
const contextLength = liveContext ?? options.modelContextLengths?.[id] ?? OMNIROUTE_DEFAULT_MODEL_CONTEXT_LENGTHS[id];
if (typeof contextLength === "number" && contextLength > 0) {
entry.limit = { context: contextLength };
}
const rawContextLength = liveContext ?? options.modelContextLengths?.[id] ?? OMNIROUTE_DEFAULT_MODEL_CONTEXT_LENGTHS[id];
const contextLength = typeof rawContextLength === "string" ? parseInt(rawContextLength, 10) : rawContextLength;
if (typeof contextLength === "number" && !isNaN(contextLength) && contextLength > 0) {
entry.limit = { context: contextLength };
}

Expand Down Expand Up @@ -508,7 +521,7 @@ export interface OmniRouteLiveModel {
* const config = buildOmniRouteOpenCodeConfig({
* baseURL: "http://localhost:20128",
* apiKey: "sk_omniroute",
* models: models.map((m) => m.id),
* models, // OmniRouteLiveModel[] — contextLength auto-extracted
* modelLabels: Object.fromEntries(models.map((m) => [m.id, m.name])),
* });
* ```
Expand Down
11 changes: 7 additions & 4 deletions src/app/api/v1/models/catalog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,8 +99,9 @@ function intersectStringArrays(arrays: string[][]): string[] {
}

function minKnownNumber(values: Array<number | undefined>): number | undefined {
if (values.length === 0 || !values.every(isPositiveFiniteNumber)) return undefined;
return Math.min(...values);
const knownValues = values.filter(isPositiveFiniteNumber);
if (knownValues.length === 0) return undefined;
return Math.min(...knownValues);
}

const VISION_MODEL_KEYWORDS = [
Expand Down Expand Up @@ -579,9 +580,11 @@ export async function getUnifiedModelsResponse(
if (targets.length === 0) return baseMetadata;

const targetMetadata = targets.map((target) => getComboTargetCatalogMetadata(target));
if (targetMetadata.some((metadata) => metadata === null)) return baseMetadata;

const knownMetadata = targetMetadata as ComboTargetCatalogMetadata[];
const knownMetadata = targetMetadata.filter(
(metadata): metadata is ComboTargetCatalogMetadata => metadata !== null
);
if (knownMetadata.length === 0) return baseMetadata;
const contextLength =
explicitContextLength ??
minKnownNumber(knownMetadata.map((metadata) => metadata.contextLength));
Expand Down
32 changes: 32 additions & 0 deletions tests/unit/models-catalog-route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1340,6 +1340,38 @@ test("v1 models catalog prefers manual combo context_length over auto-calculated
assert.equal(comboModel.context_length, 64000, "manual context_length should override auto-calc");
});

test("v1 models catalog computes combo context_length from known targets when some targets have unknown context", async () => {
await seedConnection("openai", { name: "openai-mixed-context" });
await seedConnection("claude", {
authType: "oauth",
name: "claude-mixed-context",
apiKey: null,
accessToken: "claude-access",
});

// Create a combo with targets: one known (gpt-4o = 128K), one unknown (nonexistent-model).
// The combo should still compute context_length = 128K from the known target.
const combo = await combosDb.createCombo({
name: "mixed-context-combo",
strategy: "priority",
models: ["openai/gpt-4o", "openai/nonexistent-model-xyz"],
});

const response = await v1ModelsCatalog.getUnifiedModelsResponse(
new Request("http://localhost/api/v1/models")
);
const body = (await response.json()) as any;
const comboModel = body.data.find((item) => item.id === "mixed-context-combo");

assert.equal(response.status, 200);
assert.ok(comboModel);
assert.equal(
comboModel.context_length,
128000,
"combo context_length should be the MIN of known target model limits, ignoring unknown targets"
);
});

// Regression test for Issue #2798: noAuth providers (opencode/oc) have no DB connection rows
// but their models must still appear in /v1/models.
test("v1 models catalog includes noAuth provider models when no DB connections exist (#2798)", async () => {
Expand Down
Loading