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
5 changes: 3 additions & 2 deletions open-sse/services/autoCombo/routerStrategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,10 +130,11 @@ class LKGPStrategyImpl implements RouterStrategy {
}

if (context.lastKnownGoodProvider) {
const best = pool.find(
const candidates = pool.filter(
(c) => c.provider === context.lastKnownGoodProvider && c.circuitBreakerState !== "OPEN"
);
if (best) {
if (candidates.length > 0) {
const best = candidates[0];
Comment on lines +133 to +137

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

The LKGPStrategyImpl does not actually prioritize the specific account identified by lastKnownGoodConnectionId. It filters the pool by provider and simply selects the first available candidate (candidates[0]). If multiple accounts exist for the same provider, this logic will not necessarily pick the one that previously succeeded, which contradicts the PR's goal of account-level LGKP for the auto strategy. To fix this, ProviderCandidate should include the connectionId (populated in buildAutoCandidates), and this logic should attempt to find a matching candidate before defaulting to the first one in the pool.

return {
provider: best.provider,
model: best.model,
Expand Down
36 changes: 25 additions & 11 deletions open-sse/services/combo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1742,7 +1742,7 @@ export async function handleComboChat({
try {
const { getLKGP } = await import("../../src/lib/localDb");
const lkgp = await getLKGP(combo.name, combo.id || combo.name);
if (lkgp) lastKnownGoodProvider = lkgp;
if (lkgp) lastKnownGoodProvider = lkgp.provider;
} catch (err) {
log.warn("COMBO", "Failed to retrieve Last Known Good Provider. This is non-fatal.", { err });
}
Expand Down Expand Up @@ -1819,22 +1819,34 @@ export async function handleComboChat({
const lkgpProvider = await getLKGP(combo.name, combo.id || combo.name);

if (lkgpProvider) {
const lkgpIndex = orderedTargets.findIndex(
(target) =>
target.provider === lkgpProvider || target.modelStr.startsWith(`${lkgpProvider}/`)
);
const lkgpRecord = lkgpProvider;
const providerName = lkgpRecord.provider;
const connId = lkgpRecord.connectionId;

let lkgpIndex = -1;
if (connId) {
lkgpIndex = orderedTargets.findIndex(
(target) => target.provider === providerName && target.connectionId === connId
);
}
if (lkgpIndex < 0) {
lkgpIndex = orderedTargets.findIndex(
(target) =>
target.provider === providerName || target.modelStr.startsWith(`${providerName}/`)
);
}

if (lkgpIndex > 0) {
const [lkgpTarget] = orderedTargets.splice(lkgpIndex, 1);
orderedTargets.unshift(lkgpTarget);
log.info(
"COMBO",
`[LKGP] Prioritizing last known good provider ${lkgpProvider} for combo "${combo.name}"`
`[LKGP] Prioritizing last known good provider ${providerName}${connId ? ` (account ${connId})` : ""} for combo "${combo.name}"`
);
} else if (lkgpIndex === 0) {
log.debug(
"COMBO",
`[LKGP] Last known good provider ${lkgpProvider} already first for combo "${combo.name}"`
`[LKGP] Last known good provider ${providerName}${connId ? ` (account ${connId})` : ""} already first for combo "${combo.name}"`
);
}
}
Expand Down Expand Up @@ -2070,12 +2082,13 @@ export async function handleComboChat({

// Record last known good provider (LKGP) for this combo/model (#919)
if (provider) {
const connId = target.connectionId || undefined;
void (async () => {
try {
const { setLKGP } = await import("../../src/lib/localDb");
await Promise.all([
setLKGP(combo.name, target.executionKey, provider),
setLKGP(combo.name, combo.id || combo.name, provider),
setLKGP(combo.name, target.executionKey, provider, connId),
setLKGP(combo.name, combo.id || combo.name, provider, connId),
]);
} catch (err) {
log.warn("COMBO", "Failed to record Last Known Good Provider. This is non-fatal.", {
Expand Down Expand Up @@ -2400,12 +2413,13 @@ async function handleRoundRobinCombo({
});
recordedAttempts++;
if (provider) {
const connId = target.connectionId || undefined;
void (async () => {
try {
const { setLKGP } = await import("../../src/lib/localDb");
await Promise.all([
setLKGP(combo.name, target.executionKey, provider),
setLKGP(combo.name, combo.id || combo.name, provider),
setLKGP(combo.name, target.executionKey, provider, connId),
setLKGP(combo.name, combo.id || combo.name, provider, connId),
]);
} catch (err) {
log.warn(
Expand Down
17 changes: 13 additions & 4 deletions src/lib/db/readCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,17 @@ export async function getCachedProviderConnections(

// ──────────────── LKGP Cache Wrappers ────────────────

const lkgpCache = new TTLCache<string | null>(SETTINGS_TTL_MS);
interface LKGPRecordCache {
provider: string;
connectionId?: string;
}

const lkgpCache = new TTLCache<LKGPRecordCache | null>(SETTINGS_TTL_MS);

export async function getCachedLKGP(comboName: string, modelId: string): Promise<string | null> {
export async function getCachedLKGP(
comboName: string,
modelId: string
): Promise<LKGPRecordCache | null> {
const cacheKey = `lkgp:${comboName}:${modelId}`;
const cached = lkgpCache.get(cacheKey);
if (cached !== undefined) return cached;
Expand All @@ -126,10 +134,11 @@ export async function getCachedLKGP(comboName: string, modelId: string): Promise
export async function setCachedLKGP(
comboName: string,
modelId: string,
providerId: string
providerId: string,
connectionId?: string
): Promise<void> {
const { setLKGP } = await import("@/lib/db/settings");
await setLKGP(comboName, modelId, providerId);
await setLKGP(comboName, modelId, providerId, connectionId);
lkgpCache.invalidate(`lkgp:${comboName}:${modelId}`);
}

Expand Down
26 changes: 21 additions & 5 deletions src/lib/db/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -405,26 +405,42 @@ export async function resetAllPricing() {

// ──────────────── LKGP (Last Known Good Provider) ────────────────

export async function getLKGP(comboName: string, modelId: string): Promise<string | null> {
export interface LKGPRecord {
provider: string;
connectionId?: string;
}

export async function getLKGP(comboName: string, modelId: string): Promise<LKGPRecord | null> {
const db = getDbInstance();
const key = `${comboName}:${modelId}`;
const row = db
.prepare("SELECT value FROM key_value WHERE namespace = 'lkgp' AND key = ?")
.get(key) as { value?: string } | undefined;
if (!row?.value) return null;
try {
return JSON.parse(row.value);
const parsed = JSON.parse(row.value);
if (typeof parsed === "object" && parsed !== null && "provider" in parsed) {
return parsed as LKGPRecord;
}
return { provider: String(parsed) };
} catch {
return row.value;
return { provider: row.value };
}
}

export async function setLKGP(comboName: string, modelId: string, providerId: string) {
export async function setLKGP(
comboName: string,
modelId: string,
providerId: string,
connectionId?: string
) {
const db = getDbInstance();
const key = `${comboName}:${modelId}`;
const value: LKGPRecord = { provider: providerId };
if (connectionId) value.connectionId = connectionId;
db.prepare("INSERT OR REPLACE INTO key_value (namespace, key, value) VALUES ('lkgp', ?, ?)").run(
key,
JSON.stringify(providerId)
JSON.stringify(value)
);
}

Expand Down
6 changes: 3 additions & 3 deletions tests/unit/db-read-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,16 +150,16 @@ test("cached LKGP values refresh only after the specific key is invalidated", as
const lkgpKey = `${comboName}:${modelId}`;

await settingsDb.setLKGP(comboName, modelId, "openai");
assert.equal(await readCache.getCachedLKGP(comboName, modelId), "openai");
assert.deepEqual(await readCache.getCachedLKGP(comboName, modelId), { provider: "openai" });

db.prepare("UPDATE key_value SET value = ? WHERE namespace = 'lkgp' AND key = ?").run(
JSON.stringify("anthropic"),
lkgpKey
);

assert.equal(await readCache.getCachedLKGP(comboName, modelId), "openai");
assert.deepEqual(await readCache.getCachedLKGP(comboName, modelId), { provider: "openai" });

await readCache.setCachedLKGP(comboName, modelId, "gemini");

assert.equal(await readCache.getCachedLKGP(comboName, modelId), "gemini");
assert.deepEqual(await readCache.getCachedLKGP(comboName, modelId), { provider: "gemini" });
});
31 changes: 28 additions & 3 deletions tests/unit/db-settings-crud.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,14 +201,37 @@ test("LKGP values can be set, read and cleared", async () => {
await settingsDb.setLKGP("combo-a", "model-a", "openai");
await settingsDb.setLKGP("combo-a", "model-b", "anthropic");

assert.equal(await settingsDb.getLKGP("combo-a", "model-a"), "openai");
assert.equal(await settingsDb.getLKGP("combo-a", "model-b"), "anthropic");
assert.deepEqual(await settingsDb.getLKGP("combo-a", "model-a"), { provider: "openai" });
assert.deepEqual(await settingsDb.getLKGP("combo-a", "model-b"), { provider: "anthropic" });

settingsDb.clearAllLKGP();

assert.equal(await settingsDb.getLKGP("combo-a", "model-a"), null);
});

test("LKGP stores and retrieves connectionId", async () => {
await settingsDb.setLKGP("combo-c", "model-c", "openai", "conn-abc123");

const record = await settingsDb.getLKGP("combo-c", "model-c");
assert.deepEqual(record, { provider: "openai", connectionId: "conn-abc123" });
});

test("LKGP without connectionId omits the field", async () => {
await settingsDb.setLKGP("combo-d", "model-d", "anthropic");

const record = await settingsDb.getLKGP("combo-d", "model-d");
assert.deepEqual(record, { provider: "anthropic" });
assert.equal("connectionId" in (record as object), false);
});

test("LKGP overwrites connectionId when updated without one", async () => {
await settingsDb.setLKGP("combo-e", "model-e", "openai", "conn-old");
await settingsDb.setLKGP("combo-e", "model-e", "openai");

const record = await settingsDb.getLKGP("combo-e", "model-e");
assert.deepEqual(record, { provider: "openai" });
});

test("pricing helpers ignore malformed synced data and LKGP falls back to raw values", async () => {
const db = core.getDbInstance();

Expand All @@ -234,7 +257,9 @@ test("pricing helpers ignore malformed synced data and LKGP falls back to raw va

assert.equal(pricing["broken-provider"], undefined);
assert.equal(await settingsDb.getPricingForModel("alias-provider", "missing-model"), null);
assert.equal(await settingsDb.getLKGP("combo-raw", "model-raw"), "raw-provider-id");
assert.deepEqual(await settingsDb.getLKGP("combo-raw", "model-raw"), {
provider: "raw-provider-id",
});
});

test("pricing helpers resolve aliased providers and tolerate no-op resets", async () => {
Expand Down