Skip to content

revert(feat): improve auto routing #1969

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
steebchen merged 1 commit into from
Apr 5, 2026
Merged

revert(feat): improve auto routing #1969

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
127 changes: 26 additions & 101 deletions apps/gateway/src/api-individual.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,7 @@ import {
} from "@/chat-helpers.e2e.js";

import { db, tables, eq } from "@llmgateway/db";
import {
hasProviderEnvironmentToken,
models,
providers,
getProviderEnvVar,
} from "@llmgateway/models";
import { models, providers, getProviderEnvVar } from "@llmgateway/models";

import { app } from "./app.js";
import {
Expand Down Expand Up @@ -108,15 +103,6 @@ describe("e2e individual tests", () => {
}
}

function hasConfiguredProviderForModel(modelId: string) {
const model = models.find((modelDef) => modelDef.id === modelId);
return (
model?.providers.some((provider) =>
hasProviderEnvironmentToken(provider.providerId),
) ?? false
);
}

test(
"JSON output mode error for unsupported model",
getTestOptions({ completions: false }),
Expand Down Expand Up @@ -317,16 +303,19 @@ describe("e2e individual tests", () => {
"completions with llmgateway/auto in credits mode",
getTestOptions({ completions: false }),
async () => {
if (!hasConfiguredProviderForModel("claude-sonnet-4-6")) {
console.log(
"Skipping llmgateway/auto in credits mode test - no Claude Sonnet 4.6 provider is configured",
);
return;
// require all provider keys to be set
for (const provider of providers) {
const envVarName = getProviderEnvVar(provider.id);
const envVarValue = envVarName ? process.env[envVarName] : undefined;
if (!envVarValue) {
console.log(
`Skipping llmgateway/auto in credits mode test - no API key provided for ${provider.id}`,
);
return;
}
Comment on lines +306 to +315

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

The credits auto-routing test now requires all provider env vars to be set in order to run. This makes the test extremely likely to be skipped in most environments and reduces coverage of the auto-routing path. Consider only requiring the providers that can actually be selected by the auto-routing candidates under test (e.g., the providers backing allowedAutoModels), or at minimum the provider you expect the test to route to.

Suggested change
// require all provider keys to be set
for (const provider of providers) {
const envVarName = getProviderEnvVar(provider.id);
const envVarValue = envVarName ? process.env[envVarName] : undefined;
if (!envVarValue) {
console.log(
`Skipping llmgateway/auto in credits mode test - no API key provided for ${provider.id}`,
);
return;
}
// Require at least one provider key so the auto-routing path can run.
// Requiring every provider key makes this test skip in most environments.
const availableProviders = providers.filter((provider) => {
const envVarName = getProviderEnvVar(provider.id);
return envVarName ? Boolean(process.env[envVarName]) : false;
});
if (availableProviders.length === 0) {
console.log(
"Skipping llmgateway/auto in credits mode test - no provider API keys configured",
);
return;

Copilot uses AI. Check for mistakes.
Comment on lines +306 to +315

@coderabbitai coderabbitai Bot Apr 5, 2026

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

This gate will skip the auto-routing credits test in most environments.

Requiring keys for every provider (providers) makes this test effectively non-running unless the environment is fully populated. That removes meaningful coverage for the auto-routing path.

🔧 Proposed fix 
-			// require all provider keys to be set
-			for (const provider of providers) {
-				const envVarName = getProviderEnvVar(provider.id);
-				const envVarValue = envVarName ? process.env[envVarName] : undefined;
-				if (!envVarValue) {
-					console.log(
-						`Skipping llmgateway/auto in credits mode test - no API key provided for ${provider.id}`,
-					);
-					return;
-				}
-			}
+			// require at least one provider key eligible for auto-routing
+			const autoModelIds = new Set(["gpt-oss-120b", "gpt-5-nano", "gpt-4.1-nano"]);
+			const autoProviderIds = new Set(
+				models
+					.filter((m) => autoModelIds.has(m.id))
+					.flatMap((m) => m.providers.map((p) => p.providerId)),
+			);
+			const hasAnyAutoProviderKey = Array.from(autoProviderIds).some((providerId) => {
+				const envVarName = getProviderEnvVar(providerId);
+				return Boolean(envVarName && process.env[envVarName]);
+			});
+			if (!hasAnyAutoProviderKey) {
+				console.log(
+					"Skipping llmgateway/auto in credits mode test - no eligible auto-routing provider key configured",
+				);
+				return;
+			}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@apps/gateway/src/api-individual.e2e.ts` around lines 306 - 315, The test
currently requires every provider in providers to have an API key, causing the
auto-routing credits test to be skipped in most environments; change the gate to
require at least one provider with a valid API key instead of all. Locate the
provider check using providers, getProviderEnvVar(provider.id) and envVarValue,
replace the all-provider loop with a check like providers.some(...) to detect
any provider that has a configured env var and only skip (with the existing log)
when none are present; ensure subsequent test code uses the available
provider(s) accordingly.

}

const { userId, orgId, projectId } = await createTestData(
`credits-auto-${Date.now()}`,
);
const { userId, orgId, projectId } = await createTestData("credits-auto");

Copilot AI Apr 5, 2026

Copy link

Choose a reason for hiding this comment

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

createTestData does not clean up the DB between runs (only clearCache() runs in beforeEach), so using fixed ids like credits-auto can cause primary-key collisions when the suite is rerun against the same database. Reintroduce a per-run unique suffix (e.g., Date.now() / generateTestRequestId()) or add DB cleanup for this file to keep these e2e tests repeatable.

Copilot uses AI. Check for mistakes.

await db
.update(tables.organization)
Expand All @@ -338,10 +327,9 @@ describe("e2e individual tests", () => {
.set({ mode: "credits" })
.where(eq(tables.project.id, projectId));

const creditsSuffix = generateTestRequestId();
const creditsToken = `credits-token-auto-${creditsSuffix}`;
const creditsToken = "credits-token-auto";
await db.insert(tables.apiKey).values({
id: `token-credits-auto-${creditsSuffix}`,
id: "token-credits-auto",

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 5, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Use a unique API key id in credits auto e2e test

This insert reuses id: "token-credits-auto", but createTestData("credits-auto") already creates an API key with the same id (token-${testId}), so the second insert hits a primary-key/unique violation when the test reaches this path. As written, the test will fail in environments where its provider-key preconditions are satisfied.

Useful? React with 👍 / 👎.

token: creditsToken,
projectId: projectId,
description: "Test API Key for Credits",
Expand Down Expand Up @@ -374,27 +362,17 @@ describe("e2e individual tests", () => {
expect(json).toHaveProperty("choices.[0].message.content");

const log = await waitForLogByRequestId(requestId);
expect(log.requestedModel).toBe("llmgateway/auto");
expect(log.requestedModel).toBe("auto");
expect(log.usedProvider).toBeTruthy();
expect(log.usedModel).toBeTruthy();
expect(log.usedModel).toContain("claude-sonnet-4-6");
},
);

test(
"completions with bare 'auto' model and credits",
getTestOptions({ completions: false }),
async () => {
if (!hasConfiguredProviderForModel("claude-sonnet-4-6")) {
console.log(
"Skipping bare auto credits test - no Claude Sonnet 4.6 provider is configured",
);
return;
}

const { orgId, projectId, token } = await createTestData(
`bare-auto-${Date.now()}`,
);
const { orgId, projectId, token } = await createTestData("bare-auto");

await db
.update(tables.organization)
Expand Down Expand Up @@ -434,7 +412,6 @@ describe("e2e individual tests", () => {
expect(log.requestedModel).toBe("auto");
expect(log.usedProvider).toBeTruthy();
expect(log.usedModel).toBeTruthy();
expect(log.usedModel).toContain("claude-sonnet-4-6");
},
);

Expand Down Expand Up @@ -665,9 +642,11 @@ describe("e2e individual tests", () => {
"Auto-routing sets reasoning_effort appropriately",
getTestOptions({ completions: false }),
async () => {
if (!hasConfiguredProviderForModel("claude-sonnet-4-6")) {
const envVarName = getProviderEnvVar("openai");
const envVarValue = envVarName ? process.env[envVarName] : undefined;
if (!envVarValue) {
console.log(
"Skipping auto-routing reasoning_effort test - no Claude Sonnet 4.6 provider is configured",
"Skipping auto-routing reasoning_effort test - no OpenAI API key provided",
);
return;
}
Expand Down Expand Up @@ -719,11 +698,14 @@ describe("e2e individual tests", () => {
// Verify a reasoning model was selected
const usedModel = log.usedModelMapping;
expect(usedModel).toBeDefined();
expect(usedModel).toContain("claude-sonnet-4-6");

// Claude auto-routing should always default to low reasoning effort.
// Verify reasoningEffort is set and has the correct value based on model
expect(log.reasoningEffort).toBeDefined();
expect(log.reasoningEffort).toEqual("low");
if (usedModel?.startsWith("gpt-5")) {
expect(log.reasoningEffort).toEqual("minimal");
} else {
expect(log.reasoningEffort).toEqual("low");
}

// Verify the response has valid usage information (if available)
// Some auto-routed models may not return usage in non-streaming mode
Expand All @@ -742,63 +724,6 @@ describe("e2e individual tests", () => {
},
);

test(
"Auto-routing with no_reasoning chooses Claude Haiku 4.5",
getTestOptions({ completions: false }),
async () => {
if (!hasConfiguredProviderForModel("claude-haiku-4-5")) {
console.log(
"Skipping auto-routing no_reasoning test - no Claude Haiku 4.5 provider is configured",
);
return;
}

const { orgId, projectId, token } = await createTestData(
`auto-no-reasoning-${Date.now()}`,
);

await db
.update(tables.organization)
.set({ credits: "1000" })
.where(eq(tables.organization.id, orgId));

await db
.update(tables.project)
.set({ mode: "credits" })
.where(eq(tables.project.id, projectId));

const requestId = generateTestRequestId();
const res = await app.request("/v1/chat/completions", {
method: "POST",
headers: {
"Content-Type": "application/json",
"x-request-id": requestId,
"x-no-fallback": "true",
Authorization: `Bearer ${token}`,
},
body: JSON.stringify({
model: "auto",
no_reasoning: true,
messages: [
{
role: "user",
content: "Summarize this request without extended reasoning.",
},
],
}),
});

expect(res.status).toBe(200);
const json = await res.json();
validateResponse(json);

const log = await validateLogByRequestId(requestId);
expect(log.requestedModel).toBe("auto");
expect(log.usedModelMapping).toContain("claude-haiku-4-5");
expect(log.reasoningEffort).toBeNull();
},
);

test(
"Success when requesting multi-provider model without prefix",
getTestOptions({ completions: false }),
Expand Down
Loading
Loading