Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
49 changes: 11 additions & 38 deletions apps/gateway/src/chat/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ import {
getProviderSelectionPrice,
googleProviderSupportsAudioFormat,
prepareRequestBody,
resolveMetricsModelId,
type RoutingMetadata,
} from "@llmgateway/actions";
import {
Expand Down Expand Up @@ -509,12 +508,7 @@ function addContentFilterRoutingMetadata(
: [
...excludedProviders.map((provider) => {
const metrics = metricsMap.get(
metricsKey(
resolveMetricsModelId(modelId, provider.modelName),
provider.providerId,
provider.region,
provider.modelName,
),
metricsKey(modelId, provider.providerId, provider.region),
);

return {
Expand Down Expand Up @@ -1857,10 +1851,9 @@ chat.openapi(completions, async (c) => {
if (selectedModel && selectedProviders.length > 0) {
// Fetch uptime/latency metrics from last 5 minutes for provider selection
const metricsCombinations = selectedProviders.map((p) => ({
modelId: resolveMetricsModelId(selectedModel.id, p.modelName),
modelId: selectedModel.id,
providerId: p.providerId,
region: p.region,
modelName: p.modelName,
}));
const metricsMap =
await getProviderMetricsForCombinations(metricsCombinations);
Expand Down Expand Up @@ -2068,10 +2061,9 @@ chat.openapi(completions, async (c) => {

if (eligibleMappings.length > 1) {
const metricsCombinations = eligibleMappings.map((provider) => ({
modelId: resolveMetricsModelId(modelInfo.id, provider.modelName),
modelId: modelInfo.id,
providerId: provider.providerId,
region: provider.region,
modelName: provider.modelName,
}));
const metricsMap =
await getProviderMetricsForCombinations(metricsCombinations);
Expand Down Expand Up @@ -2256,10 +2248,9 @@ chat.openapi(completions, async (c) => {

if (modelWithPricing) {
const metricsCombinations = candidatesForRouting.map((p) => ({
modelId: resolveMetricsModelId(modelWithPricing.id, p.modelName),
modelId: modelWithPricing.id,
providerId: p.providerId,
region: p.region,
modelName: p.modelName,
}));
const allMetricsMap =
await getProviderMetricsForCombinations(metricsCombinations);
Expand Down Expand Up @@ -2328,20 +2319,18 @@ chat.openapi(completions, async (c) => {
// Find the base model ID for metrics lookup
// Since custom providers are excluded above, modelInfo always has 'id'
const baseModelId = (modelInfo as ModelDefinition).id;
const metricsModelId = resolveMetricsModelId(baseModelId, usedModel);

// Fetch uptime metrics for the requested provider
const metricsMap = await getProviderMetricsForCombinations([
{
modelId: metricsModelId,
modelId: baseModelId,
providerId: usedProvider,
region: usedRegion,
modelName: usedModel,
},
]);

const metrics = metricsMap.get(
metricsKey(metricsModelId, usedProvider, usedRegion, usedModel),
metricsKey(baseModelId, usedProvider, usedRegion),
);

// If we have metrics and uptime is below 90%, route to an alternative
Expand Down Expand Up @@ -2412,10 +2401,9 @@ chat.openapi(completions, async (c) => {
if (modelWithPricing) {
// Fetch metrics for all available providers
const metricsCombinations = uptimeFallbackCandidates.map((p) => ({
modelId: resolveMetricsModelId(modelWithPricing.id, p.modelName),
modelId: modelWithPricing.id,
providerId: p.providerId,
region: p.region,
modelName: p.modelName,
}));
const allMetricsMap =
await getProviderMetricsForCombinations(metricsCombinations);
Expand All @@ -2435,12 +2423,7 @@ chat.openapi(completions, async (c) => {
const betterUptimeProviders = providerAgnosticCandidates.filter(
(p) => {
const providerMetrics = allMetricsMap.get(
metricsKey(
resolveMetricsModelId(modelWithPricing.id, p.modelName),
p.providerId,
p.region,
p.modelName,
),
metricsKey(modelWithPricing.id, p.providerId, p.region),
);
// If no metrics, assume the provider is healthy (100% uptime)
// If has metrics, only include if uptime is better than original
Expand Down Expand Up @@ -2631,13 +2614,9 @@ chat.openapi(completions, async (c) => {
...routingCandidates,
...contentFilterRoutingExcludedProviders,
].map((provider) => ({
modelId: resolveMetricsModelId(
modelWithPricing.id,
provider.modelName,
),
modelId: modelWithPricing.id,
providerId: provider.providerId,
region: provider.region,
modelName: provider.modelName,
}));
const metricsMap =
await getProviderMetricsForCombinations(metricsCombinations);
Expand Down Expand Up @@ -2808,10 +2787,9 @@ chat.openapi(completions, async (c) => {
...routingMetadataProviders,
...contentFilterRoutingExcludedProviders,
].map((provider) => ({
modelId: resolveMetricsModelId(baseModelId, provider.modelName),
modelId: baseModelId,
providerId: provider.providerId,
region: provider.region,
modelName: provider.modelName,
}));
metricsMap = await getProviderMetricsForCombinations(metricsCombinations);
}
Expand All @@ -2837,12 +2815,7 @@ chat.openapi(completions, async (c) => {
weightedScores?.metadata.providerScores ??
routingMetadataProviders.map((p) => {
const metrics = metricsMap.get(
metricsKey(
resolveMetricsModelId(baseModelId, p.modelName),
p.providerId,
p.region,
p.modelName,
),
metricsKey(baseModelId, p.providerId, p.region),
);
return {
providerId: p.providerId,
Expand Down
13 changes: 2 additions & 11 deletions apps/gateway/src/chat/tools/parse-model-input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,19 +94,10 @@ export function parseModelInput(modelInput: string): ParseModelInputResult {
});
}

// Use the provider-specific model name if available
// For models with multiple mappings for the same provider (routing models),
// keep the base model ID so routing can select the right variant later
const providerMappings = modelDef.providers.filter(
const providerMapping = modelDef.providers.find(
(p) => p.providerId === requestedProvider,
);
if (providerMappings.length > 1) {
requestedModel = modelDef.id as Model;
} else if (providerMappings.length === 1) {
requestedModel = providerMappings[0].modelName;
} else {
requestedModel = modelName as Model;
}
requestedModel = (providerMapping?.modelName ?? modelName) as Model;
Comment on lines +97 to +100

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Find model definition files (likely TS in models package)
fd -i 'xai.ts|models.ts|model.*\.ts|.*models.*\.ts' packages/models src apps 2>/dev/null || true

# Surface provider arrays where the same providerId appears more than once within one model object.
python - <<'PY'
import re, pathlib, json

roots = [pathlib.Path("packages/models"), pathlib.Path("apps"), pathlib.Path("src")]
files = []
for r in roots:
    if r.exists():
        files.extend([p for p in r.rglob("*.ts") if "node_modules" not in p.parts])

model_block_re = re.compile(r'\{[^{}]*?\bid\s*:\s*"([^"]+)"[^{}]*?\bproviders\s*:\s*\[', re.S)
provider_id_re = re.compile(r'providerId\s*:\s*"([^"]+)"')

found_any = False
for f in files:
    text = f.read_text(encoding="utf-8", errors="ignore")
    idx = 0
    while True:
        m = model_block_re.search(text, idx)
        if not m:
            break
        model_id = m.group(1)
        start = m.end() - 1  # at '['
        depth = 0
        end = start
        for i in range(start, len(text)):
            c = text[i]
            if c == '[':
                depth += 1
            elif c == ']':
                depth -= 1
                if depth == 0:
                    end = i
                    break
        providers_chunk = text[start:end+1]
        provider_ids = provider_id_re.findall(providers_chunk)
        dups = sorted({p for p in provider_ids if provider_ids.count(p) > 1})
        if dups:
            found_any = True
            print(f"{f}: model '{model_id}' has duplicate providerId entries: {', '.join(dups)}")
        idx = end + 1

if not found_any:
    print("No duplicate providerId entries detected within model.providers arrays.")
PY

Repository: theopenco/llmgateway

Length of output: 2052


🏁 Script executed:

# First, let's understand the repository structure and find model-related files
git ls-files | head -20

Repository: theopenco/llmgateway

Length of output: 599


🏁 Script executed:

# Find files that likely contain model definitions
find . -path ./node_modules -prune -o -type f -name "*.ts" -print | grep -i model | head -30

Repository: theopenco/llmgateway

Length of output: 1378


🏁 Script executed:

# Read the file under review to understand context
cat -n ./apps/gateway/src/chat/tools/parse-model-input.ts

Repository: theopenco/llmgateway

Length of output: 5381


🏁 Script executed:

# Look at the model definitions structure
head -100 ./packages/models/src/models.ts

Repository: theopenco/llmgateway

Length of output: 3381


🏁 Script executed:

# Continue reading to see model structure
sed -n '80,200p' ./packages/models/src/models.ts

Repository: theopenco/llmgateway

Length of output: 3844


🏁 Script executed:

# Look at a specific model definition file to see structure
head -150 ./packages/models/src/models/openai.ts

Repository: theopenco/llmgateway

Length of output: 3694


🏁 Script executed:

# Search all model definition files for duplicate providerId within the same model
python3 << 'EOF'
import re
import pathlib

# Find all model definition files in packages/models/src/models/
model_files = list(pathlib.Path("./packages/models/src/models").glob("*.ts"))

duplicate_found = False

for model_file in sorted(model_files):
    content = model_file.read_text()
    
    # Match model definitions: find id and its providers array
    model_pattern = r'\{\s*id:\s*"([^"]+)"[^}]*?providers:\s*\['
    
    for m in re.finditer(model_pattern, content, re.DOTALL):
        model_id = m.group(1)
        start = m.end() - 1  # Start at '['
        
        # Find matching closing bracket
        depth = 0
        end = start
        for i in range(start, len(content)):
            if content[i] == '[':
                depth += 1
            elif content[i] == ']':
                depth -= 1
                if depth == 0:
                    end = i
                    break
        
        providers_chunk = content[start:end+1]
        
        # Find all providerId values
        provider_ids = re.findall(r'providerId:\s*"([^"]+)"', providers_chunk)
        
        # Check for duplicates
        seen = {}
        for pid in provider_ids:
            if pid in seen:
                duplicate_found = True
                print(f"{model_file.name}: model '{model_id}' has duplicate providerId '{pid}'")
                seen[pid] += 1
            else:
                seen[pid] = 1

if not duplicate_found:
    print("✓ No duplicate providerId entries found within any model definition.")
EOF

Repository: theopenco/llmgateway

Length of output: 129


🏁 Script executed:

# Find and display the kimi-k2 model definition
grep -A 100 'id: "kimi-k2"' ./packages/models/src/models/moonshot.ts | head -120

Repository: theopenco/llmgateway

Length of output: 2488


Fix .find() logic to handle multiple provider mappings per model.

The code assumes only one providerId mapping exists per model, but kimi-k2 in moonshot.ts has two entries with providerId: "moonshot" (for versions kimi-k2-0711-preview and kimi-k2-0905-preview). When a user requests "moonshot/kimi-k2", .find() silently returns the first mapping, ignoring other versions. Either match both providerId and a version indicator, or select based on which version is most recent/active.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@apps/gateway/src/chat/tools/parse-model-input.ts` around lines 97 - 100, The
current use of modelDef.providers.find(...) only picks the first provider
mapping and misses other mappings (e.g., multiple moonshot entries); update the
logic that sets requestedModel so it considers all mappings in
modelDef.providers for the requestedProvider: use filter(...) to collect all
entries where p.providerId === requestedProvider, then pick the correct mapping
either by matching an explicit version indicator in the incoming request (if
available) or by choosing the most recent/active mapping (compare version
strings/dates) and assign requestedModel from that chosen mapping (fall back to
modelName if none match); update the code around providerMapping,
modelDef.providers, requestedProvider, requestedModel and modelName accordingly.

}
} else if (models.find((m) => m.id === modelInput)) {
requestedModel = modelInput as Model;
Expand Down
14 changes: 3 additions & 11 deletions apps/gateway/src/videos/videos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import {
getProviderHeaders,
getProviderSelectionPrice,
processImageUrl,
resolveMetricsModelId,
type RoutingMetadata,
type VideoPricingContext,
} from "@llmgateway/actions";
Expand Down Expand Up @@ -1453,10 +1452,9 @@ async function resolveVideoExecution(

if (configuredEligibleMappings.length > 1) {
const metricsCombinations = configuredEligibleMappings.map((provider) => ({
modelId: resolveMetricsModelId(modelInfo.id, provider.modelName),
modelId: modelInfo.id,
providerId: provider.providerId,
region: provider.region,
modelName: provider.modelName,
}));
const metricsMap =
await getProviderMetricsForCombinations(metricsCombinations);
Expand All @@ -1468,10 +1466,9 @@ async function resolveVideoExecution(
: undefined;
const requestedKey = requestedMapping
? metricsKey(
resolveMetricsModelId(modelInfo.id, requestedMapping.modelName),
modelInfo.id,
requestedMapping.providerId,
requestedMapping.region,
requestedMapping.modelName,
)
: undefined;

Expand All @@ -1491,12 +1488,7 @@ async function resolveVideoExecution(
}

const providerMetrics = metricsMap.get(
metricsKey(
resolveMetricsModelId(modelInfo.id, provider.modelName),
provider.providerId,
provider.region,
provider.modelName,
),
metricsKey(modelInfo.id, provider.providerId, provider.region),
);
return (
!providerMetrics ||
Expand Down
25 changes: 2 additions & 23 deletions packages/actions/src/get-cheapest-from-available-providers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,11 @@ import { Decimal } from "decimal.js";
import { type ProviderMetrics, metricsKey } from "@llmgateway/db";
import {
getProviderDefinition,
models,
type AvailableModelProvider,
type ModelWithPricing,
type ProviderModelMapping,
} from "@llmgateway/models";

/**
* Resolve the model id to use when looking up routing metrics for a candidate.
*
* For virtual models like `grok-4-1-fast`, the worker writes metrics to the
* concrete variant's mapping row (e.g. `grok-4-1-fast-non-reasoning`) because
* the request flows through the concrete model. The candidate's `modelName`
* matches that concrete model's id, so we use it. For non-virtual models the
* candidate's `modelName` is a provider-specific name with no matching catalog
* entry, and we fall back to the parent model id.
*/
export function resolveMetricsModelId(
parentModelId: string,
candidateModelName: string,
): string {
const concrete = models.find((m) => m.id === candidateModelName);
return concrete?.id ?? parentModelId;
}

interface ProviderScore<T extends AvailableModelProvider> {
provider: T;
score: Decimal;
Expand Down Expand Up @@ -399,10 +380,9 @@ export function getCheapestFromAvailableProviders<
const priority = providerDef?.priority ?? 1;
const metrics = metricsMap?.get(
metricsKey(
resolveMetricsModelId(modelWithPricing.id, provider.modelName),
modelWithPricing.id,
provider.providerId,
provider.region,
provider.modelName,
),
);

Expand Down Expand Up @@ -443,10 +423,9 @@ export function getCheapestFromAvailableProviders<
const price = getProviderSelectionPrice(providerInfo, videoPricing);

const mKey = metricsKey(
resolveMetricsModelId(modelWithPricing.id, provider.modelName),
modelWithPricing.id,
provider.providerId,
provider.region,
provider.modelName,
);
const metrics = metricsMap.get(mKey);

Expand Down
Loading
Loading