Repository navigation
feat: add ModelScope provider with circuit breaker and per-model quota support - #1408
clousky2020 wants to merge 17 commits into
Conversation
- add new modelscope provider with moonshotai/Kimi-K2.5, ZhipuAI/GLM-5, and stepfun-ai/Step-3.5-Flash models - implement connection-level passthroughModels toggle for granular rate limit handling - add ModelStatusBadge component showing model cooldown/unavailable/error states - update combo bad request patterns with context overflow, model not found, and Chinese transient errors
- Antigravity SSE: limit buffer to 16KB and use single TextDecoder instance - ModelStatusBadge: implement shared polling via React Context (1 req/15s vs N reqs) - Add ModelStatusProvider to dashboard layout for global state Fixes: Gemini Code Assist Bot comments on cheaper-inference#1366
- track provider failures with threshold (5) and window (20min) - cooldown entire provider for 10min after reaching threshold - only count transient error codes (429, 408, 500, 502, 503, 504) - clear provider failure state on successful request - export cooldown introspection functions - add comprehensive unit tests for circuit breaker 🐛 fix(sse): prevent OOM from unbounded stream buffer growth - use singleton TextDecoder for correct streaming decode - limit sseBuffer to 16KB to prevent memory exhaustion - flush pending decoder bytes in transform stream flush 🐛 fix(layout): correct ModelStatusProvider import path - fix incorrect import path for ModelStatusContext
…learing - fix incorrect variable reference from modelInfo.provider to provider in success callback
- add patterns to detect empty or missing function names in tool calls - add patterns to detect content moderation errors (sensitive content)
- Fix memory leak in ModelStatusContext: properly cleanup polling when no models registered - Add re-entrancy guard in recordProviderFailure to prevent race conditions - Tighten rate limit regex pattern to avoid false positive fallback triggers Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- find last newline before truncating to avoid mid-payload splits - fallback to original behavior when no newline found in discard region 🐛 fix(dashboard): ensure explicit boolean for passthroughModels - write true/false instead of undefined to override registry default - allows unchecked state to properly override registry settings 🐛 fix(model-status): use exact matching before prefix fallback - prioritize exact model matching to avoid false positives like gpt-4 matching gpt-4-turbo - improve prefix matching to handle related model variants correctly
…sitives The previous pattern `/请检查/` matched generic "please check" messages that are not content moderation errors. Replace with provider-specific phrases that indicate actual policy-blocked content. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Clear provider failure state before tests that simulate provider failures to prevent circuit breaker from blocking requests and causing 503 responses. - chat-route-coverage.test.ts: reset openai state before emergency fallback tests - chat-pipeline.test.ts: reset openai/claude state before provider failure tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- detect content moderation responses in OpenAI/Anthropic formats to trigger combo fallback - add daily quota exhaustion detection with model lock until tomorrow - create specialized error types for content moderation with zero cooldown - add comprehensive unit tests for new detection and error functions
- update comment to explain it guards against synchronous re-entrant calls - note it is not a true mutex due to Node.js single-threaded nature ✨ feat(combo): add content moderation rejection detection - import isContentModerationResponse from accountFallback - check for moderation rejections in validateResponseQuality - handle providers that return HTTP 200 with rejection messages
- clear sseBuffer when no newline found in discard region - prevents returning truncated data to remainingCredits parser 🐛 fix(fallback): separate content moderation patterns and fix model locking - split CONTENT_MODERATION_PATTERNS from MALFORMED_REQUEST_PATTERNS - fix isContentModerationResponse to use correct pattern array - skip model-level lock when provider is in circuit-breaker cooldown - add DST edge case guard in getMsUntilTomorrow calculation 🐛 fix(dashboard): fix passthroughModels default and persistence - change default passthroughModels to false in AddApiKeyModal - properly read passthroughModels from connection data in EditConnectionModal - only write passthroughModels when explicitly enabled, omit to use registry default
- Restore 30-attempt globalAttempts cap in handleComboChat and handleRoundRobinCombo to prevent runaway background requests on aggressive retry configs - Re-add 401/403 to TRANSIENT_FOR_BREAKER so auth errors trigger fast-open circuit breaker to prevent background request leaks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…c numbers - add optional log parameter to recordProviderFailure for configurable logging - replace hardcoded 30 with MAX_GLOBAL_ATTEMPTS constant in combo service - use template literal for consistent attempt limit messages
- remove server-generated IDs (rs_, fc_, resp_, msg_) from input array to prevent 404 errors - strip previous_response_id since codex endpoint does not persist responses - add originator header matching codex_cli_rs for backend compatibility ✨ feat(provider): add fallback helpers for claude models and cloud thinking - add getClaudeCodeDefaultModels to derive opus/sonnet/haiku from registry - add cloudCodeThinking service to strip thinking config for unsupported models - mark both as deprecated for v4.0 removal 🔧 chore(release): bump version to 3.6.9 ♻️ refactor(dataPaths): convert module format from ESM to CJS - replace import/export with require/module.exports - add __importDefault helper for compatibility ✨ feat(cli-tools): deprecate qwen config save endpoint - return 410 Gone for qwen tool config save requests - direct users to manually configure ~/.qwen/settings.json
There was a problem hiding this comment.
Code Review
This pull request implements a provider-level circuit breaker to manage transient failures and introduces a "Model Status" UI system for real-time feedback on model availability and cooldowns. It adds support for the ModelScope provider, enhances SSE stream handling to prevent memory exhaustion, and introduces detection for content moderation and daily quota limits. Review feedback highlights a bug in parsing model IDs that contain slashes, potential data loss in the SSE buffer truncation logic, and the need to synchronize circuit breaker error codes with authentication failure handling. Additionally, it was noted that one source file was incorrectly replaced with transpiled code.
| }; | ||
|
|
||
| // Error codes that count toward provider-level failure threshold | ||
| const PROVIDER_FAILURE_ERROR_CODES = new Set([429, 408, 500, 502, 503, 504]); |
There was a problem hiding this comment.
The PROVIDER_FAILURE_ERROR_CODES set is missing 401 and 403. The changes in combo.ts (line 53) explicitly added these codes to TRANSIENT_FOR_BREAKER with the intent of quickly opening the circuit breaker for authentication errors. To fulfill this intent, they must be included here so that recordProviderFailure is triggered.
| const PROVIDER_FAILURE_ERROR_CODES = new Set([429, 408, 500, 502, 503, 504]); | |
| const PROVIDER_FAILURE_ERROR_CODES = new Set([401, 403, 429, 408, 500, 502, 503, 504]); |
| // Update all registered models with fresh data | ||
| const now = Date.now(); | ||
| registeredModels.forEach((key) => { | ||
| const [provider, model] = key.split("/"); |
There was a problem hiding this comment.
The logic key.split("/") is incorrect for model IDs that contain slashes (e.g., moonshotai/Kimi-K2.5). This will result in an array with more than two elements, causing the model variable to only contain a partial ID (e.g., moonshotai), which will then fail to match the entries returned by the API. Use indexOf and substring to correctly split the key into provider and the full model ID.
| const [provider, model] = key.split("/"); | |
| const slashIndex = key.indexOf("/"); | |
| const provider = key.substring(0, slashIndex); | |
| const model = key.substring(slashIndex + 1); |
| if (sseBuffer.length > MAX_BUFFER_SIZE) { | ||
| const lastNewline = sseBuffer.lastIndexOf( | ||
| "\n", | ||
| sseBuffer.length - MAX_BUFFER_SIZE | ||
| ); | ||
| if (lastNewline !== -1) { | ||
| sseBuffer = sseBuffer.slice(lastNewline + 1); | ||
| } else { | ||
| // No newline found in discard region — buffer contains an incomplete SSE line. | ||
| // Discard it entirely to avoid returning malformed data; the remainingCredits | ||
| // parser won't find valid data in a truncated line anyway. | ||
| sseBuffer = ""; | ||
| } | ||
| } |
There was a problem hiding this comment.
The current truncation logic for sseBuffer is risky. If a single SSE line (e.g., a large JSON payload) exceeds MAX_BUFFER_SIZE, the lastIndexOf search for a newline in the discard region will fail, causing the entire buffer to be cleared. This results in data loss for long lines. A safer approach is to find the first newline after the truncation point to ensure the buffer stays within limits while preserving the integrity of the most recent SSE lines.
if (sseBuffer.length > MAX_BUFFER_SIZE) {
const discardCount = sseBuffer.length - MAX_BUFFER_SIZE;
const nextNewline = sseBuffer.indexOf("\n", discardCount);
if (nextNewline !== -1) {
sseBuffer = sseBuffer.slice(nextNewline + 1);
} else {
// If no newline is found in the remaining buffer, it means we have a single
// extremely long line. We must clear it to prevent OOM.
sseBuffer = "";
}
}| // Increment failure count | ||
| const newCount = entry ? entry.failureCount + 1 : 1; | ||
|
|
||
| if (newCount >= PROVIDER_FAILURE_THRESHOLD) { | ||
| // Threshold reached, enter cooldown | ||
| const cooldownUntil = now + PROVIDER_COOLDOWN_MS; | ||
| providerFailureState.set(provider, { | ||
| failureCount: newCount, | ||
| lastFailureAt: now, | ||
| resetAfterMs: PROVIDER_FAILURE_WINDOW_MS, | ||
| cooldownUntil, | ||
| }); | ||
| log?.warn?.( | ||
| `[ProviderFailure] ${provider}: ${newCount} failures in ${PROVIDER_FAILURE_WINDOW_MS / 1000}s — entering ${PROVIDER_COOLDOWN_MS / 1000}s cooldown` | ||
| ); | ||
| } else { | ||
| // Just increment counter | ||
| providerFailureState.set(provider, { | ||
| failureCount: newCount, | ||
| lastFailureAt: now, | ||
| resetAfterMs: PROVIDER_FAILURE_WINDOW_MS, | ||
| cooldownUntil: null, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The current implementation of recordProviderFailure uses a sliding window that updates lastFailureAt on every failure. This means the failure count will only reset if there is a gap of at least 20 minutes between any two failures. A more standard circuit breaker pattern uses a fixed window starting from the first failure in a sequence. Additionally, updating lastFailureAt on every call makes the log message at line 580 misleading, as the failures could have occurred over a much longer period than PROVIDER_FAILURE_WINDOW_MS.
// Increment failure count
const lastFailureAt = entry ? entry.lastFailureAt : now;
const newCount = entry ? entry.failureCount + 1 : 1;
if (newCount >= PROVIDER_FAILURE_THRESHOLD) {
// Threshold reached, enter cooldown
const cooldownUntil = now + PROVIDER_COOLDOWN_MS;
providerFailureState.set(provider, {
failureCount: newCount,
lastFailureAt,
resetAfterMs: PROVIDER_FAILURE_WINDOW_MS,
cooldownUntil,
});
log?.warn?.(
`[ProviderFailure] ${provider}: ${newCount} failures in ${PROVIDER_FAILURE_WINDOW_MS / 1000}s — entering ${PROVIDER_COOLDOWN_MS / 1000}s cooldown`
);
} else {
// Just increment counter
providerFailureState.set(provider, {
failureCount: newCount,
lastFailureAt,
resetAfterMs: PROVIDER_FAILURE_WINDOW_MS,
cooldownUntil: null,
});
}| export function getMsUntilTomorrow(): number { | ||
| const now = new Date(); | ||
| const tomorrow = new Date(now); | ||
| tomorrow.setDate(tomorrow.getDate() + 1); | ||
| tomorrow.setHours(0, 0, 0, 0); | ||
| const ms = tomorrow.getTime() - now.getTime(); | ||
| // Guard against DST edge cases: if ms is negative (shouldn't happen) or | ||
| // unreasonably large (>25h due to spring-forward), cap at 24 hours. | ||
| return ms > 0 && ms <= 25 * 60 * 60 * 1000 ? ms : 24 * 60 * 60 * 1000; | ||
| } |
There was a problem hiding this comment.
Using local server time to calculate the daily quota reset is unreliable because AI providers typically reset quotas based on a specific timezone (e.g., UTC or Beijing Time for ModelScope). If the server is in a different timezone, the model might be locked out for significantly longer than necessary or unlocked too early. Using UTC is generally a safer default for cross-region API interactions.
export function getMsUntilTomorrow(): number {
const now = new Date();
const tomorrow = new Date(now);
tomorrow.setUTCHours(24, 0, 0, 0); // Next midnight UTC
const ms = tomorrow.getTime() - now.getTime();
// Guard against DST edge cases: if ms is negative (shouldn't happen) or
// unreasonably large (>25h due to spring-forward), cap at 24 hours.
return ms > 0 && ms <= 25 * 60 * 60 * 1000 ? ms : 24 * 60 * 60 * 1000;
}| "use strict"; | ||
| var __importDefault = | ||
| (this && this.__importDefault) || | ||
| function (mod) { | ||
| return mod && mod.__esModule ? mod : { default: mod }; | ||
| }; | ||
| Object.defineProperty(exports, "__esModule", { value: true }); | ||
| exports.APP_NAME = void 0; | ||
| exports.getLegacyDotDataDir = getLegacyDotDataDir; | ||
| exports.getDefaultDataDir = getDefaultDataDir; | ||
| exports.resolveDataDir = resolveDataDir; | ||
| exports.isSamePath = isSamePath; | ||
| const path_1 = __importDefault(require("path")); | ||
| const os_1 = __importDefault(require("os")); | ||
| exports.APP_NAME = "omniroute"; | ||
| function fallbackHomeDir() { | ||
| const envHome = process.env.HOME || process.env.USERPROFILE; | ||
| if (typeof envHome === "string" && envHome.trim().length > 0) { | ||
| return path.resolve(envHome); | ||
| return path_1.default.resolve(envHome); | ||
| } | ||
|
|
||
| return os.tmpdir(); | ||
| return os_1.default.tmpdir(); | ||
| } | ||
|
|
||
| function safeHomeDir() { | ||
| try { | ||
| return os.homedir(); | ||
| return os_1.default.homedir(); | ||
| } catch { | ||
| return fallbackHomeDir(); | ||
| } | ||
| } | ||
|
|
||
| function normalizeConfiguredPath(dir) { | ||
| if (typeof dir !== "string") return null; | ||
| const trimmed = dir.trim(); | ||
| if (!trimmed) return null; | ||
| return path.resolve(trimmed); | ||
| return path_1.default.resolve(trimmed); | ||
| } | ||
|
|
||
| export function getLegacyDotDataDir() { | ||
| return path.join(safeHomeDir(), `.${APP_NAME}`); | ||
| function getLegacyDotDataDir() { | ||
| return path_1.default.join(safeHomeDir(), `.${exports.APP_NAME}`); | ||
| } | ||
|
|
||
| export function getDefaultDataDir() { | ||
| function getDefaultDataDir() { | ||
| const homeDir = safeHomeDir(); | ||
|
|
||
| if (process.platform === "win32") { | ||
| const appData = process.env.APPDATA || path.join(homeDir, "AppData", "Roaming"); | ||
| return path.join(appData, APP_NAME); | ||
| const appData = process.env.APPDATA || path_1.default.join(homeDir, "AppData", "Roaming"); | ||
| return path_1.default.join(appData, exports.APP_NAME); | ||
| } | ||
|
|
||
| // Support XDG on Linux/macOS when explicitly configured. | ||
| const xdgConfigHome = normalizeConfiguredPath(process.env.XDG_CONFIG_HOME); | ||
| if (xdgConfigHome) { | ||
| return path.join(xdgConfigHome, APP_NAME); | ||
| return path_1.default.join(xdgConfigHome, exports.APP_NAME); | ||
| } | ||
|
|
||
| return getLegacyDotDataDir(); | ||
| } | ||
|
|
||
| export function resolveDataDir({ isCloud = false } = {}) { | ||
| function resolveDataDir({ isCloud = false } = {}) { | ||
| if (isCloud) return "/tmp"; | ||
|
|
||
| const configured = normalizeConfiguredPath(process.env.DATA_DIR); | ||
| if (configured) return configured; | ||
|
|
||
| return getDefaultDataDir(); | ||
| } | ||
|
|
||
| export function isSamePath(a, b) { | ||
| function isSamePath(a, b) { | ||
| if (!a || !b) return false; | ||
| const normalizedA = path.resolve(a); | ||
| const normalizedB = path.resolve(b); | ||
|
|
||
| const normalizedA = path_1.default.resolve(a); | ||
| const normalizedB = path_1.default.resolve(b); | ||
| if (process.platform === "win32") { | ||
| return normalizedA.toLowerCase() === normalizedB.toLowerCase(); | ||
| } | ||
|
|
||
| return normalizedA === normalizedB; | ||
| } |
There was a problem hiding this comment.
This file appears to have been replaced with transpiled/machine-generated code (containing __importDefault, exports.APP_NAME, etc.) instead of maintaining the original ESM source format. Committing build artifacts or transpiled code as source makes the codebase difficult to read and maintain. Please revert this file to its original hand-written source format.
…r-model quota support Includes: provider circuit breaker, content moderation detection, daily quota handling, Codex executor fixes, SSE buffer improvements
Modelscope models like moonshotai/Kimi-K2.5 contain slashes in their IDs. Previously only the bare part was checked against the registry, which could miss entries that include the provider prefix. Now checks both the full prefixed ID first, then falls back to the bare ID.
b4a505a to
fff5aef
Compare
Summary
This PR adds ModelScope provider support with comprehensive circuit breaker and fallback improvements.
Features
Breaking Changes Mitigation
This PR originally contained breaking changes that have been optimized to maintain backward compatibility:
getClaudeCodeDefaultModels()with@deprecatedmarkercloudCodeThinking.tsmodule with@deprecatedmarkersstripStoredItemReferences()with@deprecatedmarkerdataPaths.jsCommonJS formatTesting
Checklist
npm test)npm run lint)npm run build)