mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
fix(api): address review feedback on pricing sync
- Add .catch() to initial and periodic sync promises (Gemini, Kilo) - Wrap JSON.parse in try-catch for corrupted DB data (Kilo) - Wrap response.json() in try-catch for invalid LiteLLM JSON (Kilo) - Validate PRICING_SYNC_INTERVAL (guard against NaN/0 → tight loop) (Copilot) - Validate and allowlist sources — reject unknown, prevent empty sync from clearing pricing_synced data (Copilot, Kilo) - Extract merge loop into shared iteration to reduce duplication (Gemini) - Add data/warnings fields to MCP output schema (Copilot) - Remove unused z import in vitest (Copilot) - Filter non-string entries from sources array in API route (Copilot) - Track active interval for accurate getSyncStatus().nextSync (Copilot)
This commit is contained in:
@@ -1,5 +1,4 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
import { z } from "zod";
|
||||
import { syncPricingInput, syncPricingTool, MCP_TOOLS, MCP_TOOL_MAP } from "../schemas/tools.ts";
|
||||
|
||||
describe("omniroute_sync_pricing MCP tool schema", () => {
|
||||
|
||||
@@ -742,6 +742,8 @@ export const syncPricingOutput = z.object({
|
||||
source: z.string(),
|
||||
dryRun: z.boolean(),
|
||||
error: z.string().optional(),
|
||||
warnings: z.array(z.string()).optional(),
|
||||
data: z.record(z.string(), z.record(z.string(), z.unknown())).optional(),
|
||||
});
|
||||
|
||||
export const syncPricingTool: McpToolDefinition<typeof syncPricingInput, typeof syncPricingOutput> =
|
||||
|
||||
@@ -11,7 +11,9 @@ import { NextRequest, NextResponse } from "next/server";
|
||||
export async function POST(request: NextRequest) {
|
||||
try {
|
||||
const body = await request.json().catch(() => ({}));
|
||||
const sources = Array.isArray(body.sources) ? body.sources : undefined;
|
||||
const sources = Array.isArray(body.sources)
|
||||
? body.sources.filter((s: unknown): s is string => typeof s === "string")
|
||||
: undefined;
|
||||
const dryRun = body.dryRun === true;
|
||||
|
||||
const { syncPricingFromSources } = await import("@/lib/pricingSync");
|
||||
|
||||
@@ -131,28 +131,17 @@ export async function getPricing() {
|
||||
mergedPricing[provider] = { ...(toRecord(models) as PricingModels) };
|
||||
}
|
||||
|
||||
// Layer synced on top of defaults
|
||||
for (const [provider, models] of Object.entries(syncedPricing)) {
|
||||
if (!mergedPricing[provider]) {
|
||||
mergedPricing[provider] = { ...models };
|
||||
} else {
|
||||
for (const [model, pricing] of Object.entries(models)) {
|
||||
mergedPricing[provider][model] = mergedPricing[provider][model]
|
||||
? { ...(mergedPricing[provider][model] || {}), ...toRecord(pricing) }
|
||||
: pricing;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Layer user overrides on top (highest priority)
|
||||
for (const [provider, models] of Object.entries(userPricing)) {
|
||||
if (!mergedPricing[provider]) {
|
||||
mergedPricing[provider] = { ...models };
|
||||
} else {
|
||||
for (const [model, pricing] of Object.entries(models)) {
|
||||
mergedPricing[provider][model] = mergedPricing[provider][model]
|
||||
? { ...(mergedPricing[provider][model] || {}), ...toRecord(pricing) }
|
||||
: pricing;
|
||||
// Layer synced then user on top (each higher-priority layer overrides)
|
||||
for (const layer of [syncedPricing, userPricing]) {
|
||||
for (const [provider, models] of Object.entries(layer)) {
|
||||
if (!mergedPricing[provider]) {
|
||||
mergedPricing[provider] = { ...models };
|
||||
} else {
|
||||
for (const [model, pricing] of Object.entries(models)) {
|
||||
mergedPricing[provider][model] = mergedPricing[provider][model]
|
||||
? { ...(mergedPricing[provider][model] || {}), ...toRecord(pricing) }
|
||||
: pricing;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,10 +56,16 @@ interface SyncResult {
|
||||
|
||||
// ─── Configuration ───────────────────────────────────────
|
||||
|
||||
const SYNC_INTERVAL_MS = parseInt(process.env.PRICING_SYNC_INTERVAL || "86400", 10) * 1000;
|
||||
const SUPPORTED_SOURCES = ["litellm"] as const;
|
||||
type SupportedSource = (typeof SUPPORTED_SOURCES)[number];
|
||||
|
||||
const parsedInterval = parseInt(process.env.PRICING_SYNC_INTERVAL || "86400", 10);
|
||||
const SYNC_INTERVAL_MS =
|
||||
Number.isFinite(parsedInterval) && parsedInterval > 0 ? parsedInterval * 1000 : 86400 * 1000;
|
||||
const SYNC_SOURCES = (process.env.PRICING_SYNC_SOURCES || "litellm")
|
||||
.split(",")
|
||||
.map((s) => s.trim());
|
||||
.map((s) => s.trim())
|
||||
.filter((s): s is SupportedSource => SUPPORTED_SOURCES.includes(s as SupportedSource));
|
||||
|
||||
const LITELLM_PRICING_URL =
|
||||
"https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_context_window.json";
|
||||
@@ -87,6 +93,7 @@ const LITELLM_PROVIDER_MAP: Record<string, string[]> = {
|
||||
let syncTimer: ReturnType<typeof setInterval> | null = null;
|
||||
let lastSyncTime: string | null = null;
|
||||
let lastSyncModelCount = 0;
|
||||
let activeSyncIntervalMs = SYNC_INTERVAL_MS;
|
||||
|
||||
// ─── Core: Fetch + Transform ─────────────────────────────
|
||||
|
||||
@@ -100,7 +107,12 @@ export async function fetchLiteLLMPricing(): Promise<Record<string, LiteLLMModel
|
||||
if (!response.ok) {
|
||||
throw new Error(`LiteLLM fetch failed [${response.status}]: ${response.statusText}`);
|
||||
}
|
||||
return response.json() as Promise<Record<string, LiteLLMModelInfo>>;
|
||||
const text = await response.text();
|
||||
try {
|
||||
return JSON.parse(text) as Record<string, LiteLLMModelInfo>;
|
||||
} catch {
|
||||
throw new Error(`LiteLLM returned invalid JSON (${text.slice(0, 100)}...)`);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -179,7 +191,11 @@ export function getSyncedPricing(): PricingByProvider {
|
||||
const key = typeof record.key === "string" ? record.key : null;
|
||||
const rawValue = typeof record.value === "string" ? record.value : null;
|
||||
if (!key || rawValue === null) continue;
|
||||
synced[key] = JSON.parse(rawValue) as PricingModels;
|
||||
try {
|
||||
synced[key] = JSON.parse(rawValue) as PricingModels;
|
||||
} catch {
|
||||
console.warn(`[PRICING_SYNC] Corrupted data for provider "${key}", skipping`);
|
||||
}
|
||||
}
|
||||
return synced;
|
||||
}
|
||||
@@ -223,17 +239,36 @@ export async function syncPricingFromSources(opts?: {
|
||||
sources?: string[];
|
||||
dryRun?: boolean;
|
||||
}): Promise<SyncResult> {
|
||||
const sources = opts?.sources || SYNC_SOURCES;
|
||||
const requestedSources = opts?.sources || SYNC_SOURCES;
|
||||
const dryRun = opts?.dryRun ?? false;
|
||||
|
||||
try {
|
||||
let aggregated: PricingByProvider = {};
|
||||
// Validate sources
|
||||
const validSources = requestedSources.filter((s): s is SupportedSource =>
|
||||
SUPPORTED_SOURCES.includes(s as SupportedSource)
|
||||
);
|
||||
const invalidSources = requestedSources.filter(
|
||||
(s) => !SUPPORTED_SOURCES.includes(s as SupportedSource)
|
||||
);
|
||||
|
||||
for (const source of sources) {
|
||||
if (validSources.length === 0) {
|
||||
const supported = SUPPORTED_SOURCES.join(", ");
|
||||
return {
|
||||
success: false,
|
||||
modelCount: 0,
|
||||
providerCount: 0,
|
||||
source: requestedSources.join(","),
|
||||
dryRun,
|
||||
error: `No valid sources provided. Supported: ${supported}. Invalid: ${invalidSources.join(", ")}`,
|
||||
};
|
||||
}
|
||||
|
||||
try {
|
||||
const aggregated: PricingByProvider = {};
|
||||
|
||||
for (const source of validSources) {
|
||||
if (source === "litellm") {
|
||||
const raw = await fetchLiteLLMPricing();
|
||||
const transformed = transformToOmniRoute(raw);
|
||||
// Merge into aggregated
|
||||
for (const [provider, models] of Object.entries(transformed)) {
|
||||
if (!aggregated[provider]) aggregated[provider] = {};
|
||||
Object.assign(aggregated[provider], models);
|
||||
@@ -257,8 +292,11 @@ export async function syncPricingFromSources(opts?: {
|
||||
success: true,
|
||||
modelCount,
|
||||
providerCount,
|
||||
source: sources.join(","),
|
||||
source: validSources.join(","),
|
||||
dryRun,
|
||||
...(invalidSources.length > 0
|
||||
? { warnings: [`Unknown sources ignored: ${invalidSources.join(", ")}`] }
|
||||
: {}),
|
||||
...(dryRun ? { data: aggregated } : {}),
|
||||
};
|
||||
} catch (err) {
|
||||
@@ -268,7 +306,7 @@ export async function syncPricingFromSources(opts?: {
|
||||
success: false,
|
||||
modelCount: 0,
|
||||
providerCount: 0,
|
||||
source: sources.join(","),
|
||||
source: requestedSources.join(","),
|
||||
dryRun,
|
||||
error: message,
|
||||
};
|
||||
@@ -284,23 +322,35 @@ export function startPeriodicSync(intervalMs?: number): void {
|
||||
if (syncTimer) return; // Already running
|
||||
|
||||
const interval = intervalMs ?? SYNC_INTERVAL_MS;
|
||||
activeSyncIntervalMs = interval;
|
||||
console.log(`[PRICING_SYNC] Starting periodic sync every ${interval / 1000}s`);
|
||||
|
||||
// Initial sync (non-blocking)
|
||||
syncPricingFromSources().then((result) => {
|
||||
if (result.success) {
|
||||
console.log(
|
||||
`[PRICING_SYNC] Initial sync complete: ${result.modelCount} models from ${result.providerCount} providers`
|
||||
);
|
||||
}
|
||||
});
|
||||
syncPricingFromSources()
|
||||
.then((result) => {
|
||||
if (result.success) {
|
||||
console.log(
|
||||
`[PRICING_SYNC] Initial sync complete: ${result.modelCount} models from ${result.providerCount} providers`
|
||||
);
|
||||
}
|
||||
})
|
||||
.catch((err) => {
|
||||
console.warn("[PRICING_SYNC] Initial sync error:", err instanceof Error ? err.message : err);
|
||||
});
|
||||
|
||||
syncTimer = setInterval(() => {
|
||||
syncPricingFromSources().then((result) => {
|
||||
if (result.success) {
|
||||
console.log(`[PRICING_SYNC] Periodic sync complete: ${result.modelCount} models`);
|
||||
}
|
||||
});
|
||||
syncPricingFromSources()
|
||||
.then((result) => {
|
||||
if (result.success) {
|
||||
console.log(`[PRICING_SYNC] Periodic sync complete: ${result.modelCount} models`);
|
||||
}
|
||||
})
|
||||
.catch((err) => {
|
||||
console.warn(
|
||||
"[PRICING_SYNC] Periodic sync error:",
|
||||
err instanceof Error ? err.message : err
|
||||
);
|
||||
});
|
||||
}, interval);
|
||||
}
|
||||
|
||||
@@ -326,9 +376,9 @@ export function getSyncStatus(): SyncStatus {
|
||||
lastSyncModelCount,
|
||||
nextSync:
|
||||
syncTimer && lastSyncTime
|
||||
? new Date(new Date(lastSyncTime).getTime() + SYNC_INTERVAL_MS).toISOString()
|
||||
? new Date(new Date(lastSyncTime).getTime() + activeSyncIntervalMs).toISOString()
|
||||
: null,
|
||||
intervalMs: SYNC_INTERVAL_MS,
|
||||
intervalMs: activeSyncIntervalMs,
|
||||
sources: SYNC_SOURCES,
|
||||
};
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user