Skip to content
Open
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
3 changes: 3 additions & 0 deletions services/platform/convex/lib/agent_chat/internal_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,9 @@ export async function runGenerationCore(
// later model sharing that resource is skipped instead of waiting on a
// doomed request. Keying by credential (not provider name) means a sibling
// model with its own `secretsEnv` key is still tried after another key dies.
// An out-of-funds (credit) failure is the one exception that does NOT doom
// every sibling: a zero-cost model on the same credential (`:free` / priced
// at 0) draws no credits, so `isModelScopeRetired` still attempts it (#1454).
let lastFallbackError: unknown;
const deadScopes = new Set<string>();

Expand Down
97 changes: 91 additions & 6 deletions services/platform/convex/providers/failure_scope.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
import { describe, expect, it } from 'vitest';

import {
creditScopeKey,
credentialScopeKey,
endpointScopeKey,
isFreeModel,
isModelScopeRetired,
modelScopeKeys,
retiredScopeKey,
} from './failure_scope';

const model = (
over: Partial<{ providerName: string; apiKey: string; baseUrl: string }> = {},
over: Partial<{
providerName: string;
apiKey: string;
baseUrl: string;
modelId: string;
inputCentsPerMillion: number;
outputCentsPerMillion: number;
}> = {},
) => ({
providerName: 'openrouter',
apiKey: 'key-A',
Expand Down Expand Up @@ -47,15 +56,18 @@ describe('endpointScopeKey', () => {
});

describe('retiredScopeKey', () => {
it('retires the CREDENTIAL for funds and auth failures', () => {
expect(retiredScopeKey('credit_exhausted', model())).toBe(
credentialScopeKey(model()),
);
it('retires the AUTH credential for an auth failure', () => {
expect(retiredScopeKey('auth_error', model())).toBe(
credentialScopeKey(model()),
);
});

it('retires the CREDIT credential for an out-of-funds failure', () => {
expect(retiredScopeKey('credit_exhausted', model())).toBe(
creditScopeKey(model()),
);
});

it('retires the ENDPOINT for an unreachable host', () => {
expect(retiredScopeKey('provider_unreachable', model())).toBe(
endpointScopeKey(model()),
Expand Down Expand Up @@ -93,10 +105,83 @@ describe('isModelScopeRetired', () => {
expect(isModelScopeRetired(model(), new Set())).toBe(false);
});

it('exposes both scopes a model belongs to', () => {
it('exposes both unconditional scopes a model belongs to', () => {
expect(modelScopeKeys(model())).toEqual([
credentialScopeKey(model()),
endpointScopeKey(model()),
]);
});

describe('credit retirement spares zero-cost siblings (#1454)', () => {
it('still skips a PAID model on a credit-dead credential', () => {
const dead = new Set([creditScopeKey(model())]);
expect(
isModelScopeRetired(
model({ modelId: 'openai/gpt-4o', inputCentsPerMillion: 250 }),
dead,
),
).toBe(true);
});

it('does NOT skip a `:free` sibling on a credit-dead credential', () => {
const dead = new Set([creditScopeKey(model())]);
expect(
isModelScopeRetired(
model({ modelId: 'meta-llama/llama-3.3-70b-instruct:free' }),
dead,
),
).toBe(false);
});

it('does NOT skip a zero-priced sibling on a credit-dead credential', () => {
const dead = new Set([creditScopeKey(model())]);
expect(
isModelScopeRetired(
model({ inputCentsPerMillion: 0, outputCentsPerMillion: 0 }),
dead,
),
).toBe(false);
});

it('STILL skips a free model when the credential died from AUTH', () => {
// A bad/expired key kills free models too — only credit exhaustion spares them.
const dead = new Set([credentialScopeKey(model())]);
expect(isModelScopeRetired(model({ modelId: 'x/y:free' }), dead)).toBe(
true,
);
});

it('STILL skips a free model when the endpoint is unreachable', () => {
const dead = new Set([endpointScopeKey(model())]);
expect(isModelScopeRetired(model({ modelId: 'x/y:free' }), dead)).toBe(
true,
);
});
});
});

describe('isFreeModel', () => {
it('treats the OpenRouter `:free` suffix as free', () => {
expect(isFreeModel(model({ modelId: 'deepseek/deepseek-r1:free' }))).toBe(
true,
);
});

it('treats explicit zero token pricing on both sides as free', () => {
expect(
isFreeModel(model({ inputCentsPerMillion: 0, outputCentsPerMillion: 0 })),
).toBe(true);
});

it('does NOT treat unconfigured pricing as free', () => {
expect(isFreeModel(model({ modelId: 'openai/gpt-4o' }))).toBe(false);
});

it('does NOT treat a paid model as free', () => {
expect(
isFreeModel(
model({ inputCentsPerMillion: 250, outputCentsPerMillion: 1000 }),
),
).toBe(false);
});
});
65 changes: 58 additions & 7 deletions services/platform/convex/providers/failure_scope.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,33 @@ interface ScopeModelData {
apiKey?: string;
/** Endpoint base URL; absent when the provider default is used (treated as ''). */
baseUrl?: string;
/** Model identifier, e.g. `meta-llama/llama-3.3-70b-instruct:free`. */
modelId?: string;
/** Per-million-token input price in cents; `0` marks a no-cost model. */
inputCentsPerMillion?: number;
/** Per-million-token output price in cents; `0` marks a no-cost model. */
outputCentsPerMillion?: number;
}

/**
* Whether a model draws on NO provider credits — so an out-of-funds failure on a
* sibling that shares its credential must NOT retire it. Two independent signals:
*
* - the OpenRouter `:free` id suffix (its free variants never bill the account);
* - explicit zero token pricing on BOTH sides (a deliberately free/local model).
*
* Unconfigured pricing (`undefined`) is intentionally NOT treated as free — only
* an explicit `0` counts, so a model whose cost simply wasn't filled in stays
* subject to credit retirement.
*/
export function isFreeModel(data: ScopeModelData): boolean {
if (
typeof data.modelId === 'string' &&
data.modelId.toLowerCase().includes(':free')
) {
Comment on lines +52 to +54

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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use suffix matching for :free detection.

Line 53 uses includes(':free'), but this behavior is defined as a :free suffix check. A model ID like vendor/model:freedom would be misclassified as free and incorrectly bypass credit-scope retirement.

Proposed fix
 export function isFreeModel(data: ScopeModelData): boolean {
   if (
     typeof data.modelId === 'string' &&
-    data.modelId.toLowerCase().includes(':free')
+    data.modelId.toLowerCase().endsWith(':free')
   ) {
     return true;
   }
   return data.inputCentsPerMillion === 0 && data.outputCentsPerMillion === 0;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
typeof data.modelId === 'string' &&
data.modelId.toLowerCase().includes(':free')
) {
export function isFreeModel(data: ScopeModelData): boolean {
if (
typeof data.modelId === 'string' &&
data.modelId.toLowerCase().endsWith(':free')
) {
return true;
}
return data.inputCentsPerMillion === 0 && data.outputCentsPerMillion === 0;
}
🤖 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 `@services/platform/convex/providers/failure_scope.ts` around lines 52 - 54,
The condition checking for free models uses includes(':free') which performs
substring matching rather than suffix matching. This causes false positives
where model IDs like vendor/model:freedom would be incorrectly classified as
free. Replace the includes(':free') method call with endsWith(':free') to ensure
only model IDs that actually end with the :free suffix are detected, preventing
misclassification of similarly named models and avoiding incorrect credit-scope
retirement bypass.

return true;
}
return data.inputCentsPerMillion === 0 && data.outputCentsPerMillion === 0;
}

/** FNV-1a 32-bit → 8-char hex. Stable, non-reversible bucket id for an API key. */
Expand All @@ -40,17 +67,34 @@ function fingerprint(value: string): string {
return (hash >>> 0).toString(16).padStart(8, '0');
}

/** Credential identity: provider + API key (funds / auth are key-scoped). */
/** Credential identity for AUTH failures: provider + API key. */
export function credentialScopeKey(data: ScopeModelData): string {
return `cred:${data.providerName}:${fingerprint(data.apiKey ?? '')}`;
}

/**
* Credential identity for CREDIT (out-of-funds) failures: provider + API key.
*
* Distinct namespace from {@link credentialScopeKey} so the dead-set records WHY
* the credential died. An out-of-funds failure must spare zero-cost siblings on
* the same key (they don't draw credits); an auth failure must not. Same inputs,
* different prefix — they never collide.
*/
export function creditScopeKey(data: ScopeModelData): string {
return `credit:${data.providerName}:${fingerprint(data.apiKey ?? '')}`;
}

/** Endpoint identity: provider + baseUrl (reachability is endpoint-scoped). */
export function endpointScopeKey(data: ScopeModelData): string {
return `host:${data.providerName}:${data.baseUrl ?? ''}`;
}

/** Every scope a model belongs to — for dead-set membership tests. */
/**
* The UNCONDITIONAL scopes a model belongs to — auth credential + endpoint.
* A dead entry in either retires the model regardless of its pricing. Credit
* retirement is conditional (free models are exempt) and handled separately in
* {@link isModelScopeRetired}.
*/
export function modelScopeKeys(data: ScopeModelData): readonly string[] {
return [credentialScopeKey(data), endpointScopeKey(data)];
}
Expand All @@ -65,16 +109,23 @@ export function retiredScopeKey(
data: ScopeModelData,
): string | null {
if (code === 'provider_unreachable') return endpointScopeKey(data);
if (code === 'credit_exhausted' || code === 'auth_error') {
return credentialScopeKey(data);
}
if (code === 'auth_error') return credentialScopeKey(data);
if (code === 'credit_exhausted') return creditScopeKey(data);
return null;
}

/** True if any of the model's scopes has already been retired this turn. */
/**
* True if any of the model's scopes has already been retired this turn.
*
* Auth-credential and endpoint deaths retire every model on the resource. A
* CREDIT (out-of-funds) death retires only the paid models on the credential —
* a zero-cost sibling ({@link isFreeModel}) is still attempted, because the
* account being out of funds doesn't stop a model that costs nothing.
*/
export function isModelScopeRetired(
data: ScopeModelData,
deadScopes: ReadonlySet<string>,
): boolean {
return modelScopeKeys(data).some((key) => deadScopes.has(key));
if (modelScopeKeys(data).some((key) => deadScopes.has(key))) return true;
return deadScopes.has(creditScopeKey(data)) && !isFreeModel(data);
}