Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a major restructuring of the open-sse services, modularizing the monolithic chatCore handler, image generation handlers, usage fetchers, and combo routing services into dedicated sub-modules. It also soft-deprecates @omniroute/opencode-provider in favor of @omniroute/opencode-plugin, cleans up fictional endpoints in the API reference, migrates public OAuth client IDs to secure resolvers, and fixes bugs related to reasoning models and unclosed reasoning tags. The code review identified several critical issues: a missing await on an asynchronous database query in the Xiaomi usage fetcher that causes incorrect token tracking; a missing check for job_id in the Haiper image generation handler that can lead to malformed polling URLs; overwriting of rate limit keys in the Kimi usage fetcher loop; and a potential RangeError in parseResetTime when handling invalid dates.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if (!connectionId) { | ||
| return { message: "Xiaomi MiMo: connection id unavailable for self-tracked quota." }; | ||
| } | ||
| try { |
There was a problem hiding this comment.
Since getMonthlyProviderTokensForConnection is imported from @/lib/usage/usageStats and likely performs asynchronous database queries, it should be awaited. Otherwise, used will be a Promise object, which toNumber (inside createQuotaFromUsage) will parse as NaN and default to 0, resulting in incorrect usage tracking.
const used = await getMonthlyProviderTokensForConnection("xiaomi-mimo", connectionId);| }); | ||
| const status = await statusRes.json(); | ||
| if (status.status === "completed" || status.status === "succeeded") { | ||
| const imgUrl = status.creation_url || status.output?.image_url; |
There was a problem hiding this comment.
If res.json() does not contain job_id (e.g., due to an unexpected upstream response format or API error), job_id will be undefined. This will cause the subsequent polling loop to request a malformed URL (${providerConfig.statusUrl}/undefined), leading to unnecessary network requests and a less clear error. We should explicitly check for job_id and return a proper 502 error if it is missing.
const { job_id } = await res.json();
if (!job_id) {
const errorText = "No job ID returned from Haiper";
saveCallLog({
method: "POST",
path: "/v1/images/generations",
status: 502,
model: `${provider}/${model}`,
provider,
duration: Date.now() - startTime,
error: errorText,
}).catch(() => {});
return { success: false, status: 502, error: errorText };
}| quotas["Ratelimit"] = { | ||
| used: limit - remaining, | ||
| total: limit, | ||
| remaining, | ||
| remainingPercentage: limit > 0 ? (remaining / limit) * 100 : 0, | ||
| resetAt: parseResetTime(resetTime), | ||
| unlimited: false, | ||
| }; |
There was a problem hiding this comment.
If limitsArray contains multiple rate limit objects (e.g., different windows or types), they will all overwrite quotas["Ratelimit"] in the loop, meaning only the last one will be preserved. We should use a unique key (such as appending the index or window duration) to ensure all rate limits are captured and displayed.
const quotaKey = limitsArray.length > 1 ? `Ratelimit #${i + 1}` : "Ratelimit";
quotas[quotaKey] = {
used: limit - remaining,
total: limit,
remaining,
remainingPercentage: limit > 0 ? (remaining / limit) * 100 : 0,
resetAt: parseResetTime(resetTime),
unlimited: false,
};| // Epoch-zero (1970-01-01) means no scheduled reset — treat as null | ||
| if (date.getTime() <= 0) return null; |
There was a problem hiding this comment.
In parseResetTime, calling date.getTime() <= 0 is false when date is an Invalid Date (since NaN <= 0 is false). This causes the function to proceed to date.toISOString(), which throws a RangeError: Invalid time value. Although this is caught by the try-catch block, it is cleaner and more performant to explicitly check Number.isNaN(time) to avoid throwing and catching exceptions for malformed date strings.
| // Epoch-zero (1970-01-01) means no scheduled reset — treat as null | |
| if (date.getTime() <= 0) return null; | |
| // Epoch-zero (1970-01-01) means no scheduled reset — treat as null | |
| const time = date.getTime(); | |
| if (Number.isNaN(time) || time <= 0) return null; |
…gosouzapw#3691) Integrated into release/v3.8.23
Integrated into release/v3.8.23
…iegosouzapw#3689) Integrated into release/v3.8.23
…ution (diegosouzapw#3692) Integrated into release/v3.8.23
Integrated into release/v3.8.23
Integrated into release/v3.8.23
…osouzapw#3685) (diegosouzapw#3702) Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…osouzapw#3696) (diegosouzapw#3703) Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
…souzapw#3660) Integrated into release/v3.8.23
…API (diegosouzapw#3712) Integrated into release/v3.8.23. Vertex dynamic model discovery — surfaces image models (imagen-*, gemini-*-image), embeddings and audio from the live Generative Language catalog, with cached→static fallback and the shared parseGeminiModelsList helper. Validated: parser test 5/5, typecheck:core clean.
Integrated into release/v3.8.23. Makes the diegosouzapw#3588 reasoning token buffer safe and configurable: only inflates max_tokens when the model has a known, non-default output cap and the buffered value fits inside it; otherwise preserves/clamps the client limit. Adds the reasoningTokenBufferEnabled kill switch (default ON). Validated: combo-routing-engine 81/81, combo-config 25/25, combo-quality-validator-reasoning 12/12, phase1f 10/10, typecheck:core clean.
…3408 LOC (-654) (diegosouzapw#3717) Phase 1g-1j of diegosouzapw#3501: client 4062→3408 LOC. Pure extraction (ProviderPlaygroundPanel, useCommandCodeAuth, useExternalLinkFlow+ExternalLinkModal, useAuthFileHandlers) + loadConnProxies ReferenceError fix + phase1f test path fix. Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…2553 LOC (-855) (diegosouzapw#3721) Phase 1k-1m of diegosouzapw#3501: client 3408→2553 LOC. Pure extraction (useModelImportHandlers+ImportProgressModal, useModelVisibilityHandlers, ProviderModelsSection). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
… release branch The fix itself reached main pre-tag via cherry-pick diegosouzapw#3591, but its changelog bullet (commit e33fdd4) only ever existed on release/v3.8.20 after the squash-merge. Restored under [3.8.20] per the 2026-06-12 release-branch leftover audit (_tasks/release-audit/release-leftovers-audit-2026-06-12.md).
…rofileArn (diegosouzapw#3722) Integrated into release/v3.8.23
…1376 LOC (-1177) (diegosouzapw#3725) Phase 1n-1s of diegosouzapw#3501: client 2553→1376 LOC. Pure extraction (ConnectionsListPanel, ConnectionsHeaderToolbar, ZedImportCard, BatchTestResultsModal, AdaptaTutorialModal, useApiKeySave + helpers). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…cation, and success-decay recovery (diegosouzapw#3629) Integrated into release/v3.8.23
… LOC (≤800 TARGET REACHED ✅) (diegosouzapw#3727) Phase 1t of diegosouzapw#3501: client 1376→781 LOC (≤800 reached). Original god-component 12,882→781 (−94%). Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
…opencode' CLI command (diegosouzapw#3726) Integrated into release/v3.8.23
…w#3724) Integrated into release/v3.8.23
…egosouzapw#3288) (diegosouzapw#3723) Integrated into release/v3.8.23
… every chatHelpers import diegosouzapw#3692 added a lazy 'await import(proxyEgress)' for egress-IP visibility inside safeLogEvents, which is a sync function — an ES syntax error. It went unnoticed because typecheck:core does not cover src/sse and no test in the merge gates loaded chatHelpers via tsx; any consumer that did (chat-context-relay and chat-route-coverage suites, integration harnesses) failed at module load with 'await can only be used inside an async function'. safeLogEvents is fire-and-forget logging with an outer try/catch, so making it async (and 'void'-ing the single chat.ts call site) preserves behavior exactly. Validation: tests/unit/chat-context-relay.test.ts + chat-route-coverage.test.ts went from failing-at-load to green (+14 tests destravados).
… + combo/proxy audit fixes (diegosouzapw#3699) Integrated into release/v3.8.23
) Split-out PR A from diegosouzapw#3584. Normalizes the Antigravity/agy Gemini 3.5 Flash tier IDs to clean public names (gemini-3.5-flash-low/medium/high), maps them to the live upstream IDs at the executor boundary, and removes Antigravity from the global model resolver so the executor owns wire normalization. Maintainer follow-up: kept gemini-3.5-flash-preview as a hidden backward-compat alias routing to the High tier (so saved combos/configs keep working). Live-validated the tier set via the agy CLI catalog. Integrated into release/v3.8.21. Thanks @dhaern!
…s port 443 (diegosouzapw#3606) (diegosouzapw#3608) Integrated into release/v3.8.21 (diegosouzapw#3606)
…500 (diegosouzapw#3589) (diegosouzapw#3609) Integrated into release/v3.8.21 (diegosouzapw#3589)
…de-plugin (diegosouzapw#3419) (diegosouzapw#3613) Integrated into release/v3.8.21 (diegosouzapw#3419)
…pw#3604) Split-out PR B from diegosouzapw#3584. Normalizes Antigravity/agy provider quotas: prefers retrieveUserQuota for live consumption, falls back to fetchAvailableModels and local usage_history, sanitizes cached Provider Limits so retired upstream IDs are not re-exposed, and schedules a deduplicated post-usage refresh. Maintainer follow-up: decoupled the post-usage refresh via a lightweight usageEvents bus (usageHistory no longer dynamic-imports providerLimits) so it does not pull the executors/translator graph into the typecheck-core surface — typecheck:core stays at 0. Integrated into release/v3.8.21. Thanks @dhaern!
…ode (diegosouzapw#3331) (diegosouzapw#3614) Integrated into release/v3.8.21 (diegosouzapw#3331)
…zapw#3604 (provider quotas) + diegosouzapw#3605 (reasoning wrappers) Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…apw#3594) Breaks the 12,882-line providers/[id]/page.tsx into 6 domain-focused files: - types.ts — all shared interfaces/types - utils.tsx — helper functions and utility components (ModelSourceBadge, CooldownTimer, ModelCompatPopover, etc.) - modelManagement.tsx — ModelRow, ModelVisibilityToolbar, PassthroughModelsSection, CustomModelsSection, CompatibleModelsSection - connectionManagement.tsx — ConnectionRow, EditConnectionModal, EditCompatibleNodeModal - authModals.tsx — AddApiKeyModal, SiliconFlowEndpointModal, Import/Apply (Codex/Claude/Gemini) auth modals - playground.tsx — ProviderPlaygroundPanel, renderKindPanel page.tsx reduced from 12,882 → 4,687 lines (63% reduction). TypeScript typecheck passes cleanly. ESLint: 0 errors, only 3 pre-existing warnings. Pre-parse and de-duplicate all imports per file to eliminate unused imports. Resolves part of diegosouzapw#3594.
- Replaced Buffer with browser-safe atob in utils.tsx - Fixed memory leaks/infinite intervals in cooldown timers - Typed chatCore handlers with 'any' instead of 'unknown' to fix strict-mode property access - Fixed single-model reasoning payload mutation in combo.ts
) - Splits the 4,600-line combo.ts monolithic file into 12 domain-specific files - Extracts Auto-Combo, Round-Robin, Quota tracking, Shadow routing, and Context Affinity logic into dedicated modules - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
- Splits the monolithic provider validation file into 7 domain-specific files - Extracts Enterprise, Web, Search, Standard, Specialty, and Utils into dedicated modules - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
…#3594) - Splits the monolithic image generation handler into 13 domain-specific files - Extracts provider-specific implementations (OpenAI, Gemini, Stability, Fal, BFL, etc.) - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
- Splits the monolithic usage fetcher into 24 domain-specific files - Extracts provider-specific usage implementations (Antigravity, Claude, Cursor, GitHub, etc.) - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
…zapw#3594) - Splits the monolithic provider constants file into 12 domain-specific files - Extracts provider dictionaries into definitions/ (apiKey, oauth, webCookie, local, search, etc.) - Uses folder-as-module pattern with index.ts facade for zero backward-compatibility breakages - Auto-prunes and manages all imports for strict typechecking
23c4801 to
0f4046f
Compare
|
Thanks @oyi77 for the modularization work on
What would let this land: re-cut it as a single, independent PR branched from the current |
Summary
This PR addresses the final High Priority massive file from Issue #3594:
src/shared/constants/providers.tsat 3,122 lines.Changes
Splits the monolithic configuration dictionary into 12 domain-focused files inside
src/shared/constants/providers/:types.ts,utils.ts,groups.tscore.tsgetProviderById, map initializers, and static dictionariesdefinitions/apiKey.tsAPIKEY_PROVIDERSdefinition blockdefinitions/oauth.ts,definitions/webCookie.ts,definitions/local.tsdefinitions/search.ts,definitions/audio.ts,definitions/noauth.tsdefinitions/compatible.tsindex.tsResults
providers/index.ts)Verification
npm run typecheck:core— passes cleanly.npx eslint src/shared/constants/providers/**/*.ts— 0 errors or warnings.Resolves part of #3594.