Skip to content

refactor: modularize 4.6K-line open-sse/services/combo.ts (Issue #3594) - #3640

Closed
oyi77 wants to merge 52 commits into
cheaper-inference:release/v3.8.24from
oyi77:refactor/modularize-combo
Closed

oyi77 wants to merge 52 commits into
cheaper-inference:release/v3.8.24from
oyi77:refactor/modularize-combo

Conversation

@oyi77

@oyi77 oyi77 commented Jun 11, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR addresses the 4th highest priority massive file from Issue #3594: open-sse/services/combo.ts at 4,600+ lines.

Changes

Splits the monolithic core combination service into 12 domain-focused files inside open-sse/services/combo/:

File Contents
types.ts All shared interfaces, runtime step types, target types
utils.ts Validation helpers, normalizers, basic parsing
constants.ts Shared weights, quotas, reset constants
state.ts In-memory atomic counters, connection caches, soft penalties
dag.ts DAG validation, nested target resolution, target flattening
sorting.ts Target sorting (Cost-optimized, Least-used, Context-size, Weighted)
quota.ts Reset-aware fetching, reset window alignment, and SLA scoring
context.ts Context affinity, prompt extraction, and intent classification
shadow.ts Shadow routing clones, execution, and draining
auto.ts buildAutoCandidates scoring logic, request-tag routing
roundRobin.ts The handleRoundRobinCombo execution path
chat.ts The main handleComboChat execution orchestration loop
index.ts Exposes the 15 exported functions back to the rest of the app

Results

  • combo.ts: 4,657 lines → 1 line (re-exports combo/index.ts)
  • All imports between files are resolved correctly.
  • Full backward compatibility: all 15 previously exported functions from combo.ts are re-exported.

Verification

  • npm run typecheck:core — passes cleanly.
  • npx eslint open-sse/services/combo/*.ts — 0 errors or warnings.
  • Strictly structural refactoring with identical runtime behavior.

Resolves part of #3594.

@gemini-code-assist gemini-code-assist Bot left a comment

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.

Code Review

This pull request introduces several bug fixes, refactorings, and feature additions across the repository. Key changes include soft-deprecating @omniroute/opencode-provider in favor of @omniroute/opencode-plugin, modularizing the handleChatCore handler and the combo routing engine, and resolving several undocumented or missing API endpoints (such as gamification, guardrails, and agent-bridge). Additionally, it hardens the Antigravity/agy provider limits, translates chat-pipeline responses back to the legacy text-completion shape for /v1/completions, and migrates public OAuth client IDs to secure public credential resolvers. Feedback on the changes highlights a potential issue in open-sse/services/combo/constants.ts where an invalid QUOTA_SOFT_DEPRIORITIZE_FACTOR environment variable could parse to NaN and break sorting; a fallback check using Number.isFinite is recommended.

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.

Comment thread open-sse/services/combo/constants.ts Outdated
Comment on lines +145 to +147
export const QUOTA_SOFT_DEPRIORITIZE_FACTOR = Number(
process.env.QUOTA_SOFT_DEPRIORITIZE_FACTOR ?? "0.7"
);

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.

medium

If process.env.QUOTA_SOFT_DEPRIORITIZE_FACTOR is set to an invalid numeric string, Number(...) will return NaN. This will result in NaN scores during auto-combo scoring and break the sorting logic. Use Number.isFinite to safely fall back to 0.7 if the parsed value is invalid.

const rawFactor = Number(process.env.QUOTA_SOFT_DEPRIORITIZE_FACTOR ?? "0.7");
export const QUOTA_SOFT_DEPRIORITIZE_FACTOR = Number.isFinite(rawFactor) ? rawFactor : 0.7;

@diegosouzapw
diegosouzapw changed the base branch from main to release/v3.8.23 June 12, 2026 01:19
zhiru and others added 24 commits June 12, 2026 02:40
…API (cheaper-inference#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 cheaper-inference#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.
…4062→3408 LOC (-654) (cheaper-inference#3717)

Phase 1g-1j of cheaper-inference#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>
…3408→2553 LOC (-855) (cheaper-inference#3721)

Phase 1k-1m of cheaper-inference#3501: client 3408→2553 LOC. Pure extraction (useModelImportHandlers+ImportProgressModal, useModelVisibilityHandlers, ProviderModelsSection).

Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
….8.20 release branch

The fix itself reached main pre-tag via cherry-pick cheaper-inference#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).
…2553→1376 LOC (-1177) (cheaper-inference#3725)

Phase 1n-1s of cheaper-inference#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 (cheaper-inference#3629)

Integrated into release/v3.8.23
…6→781 LOC (≤800 TARGET REACHED ✅) (cheaper-inference#3727)

Phase 1t of cheaper-inference#3501: client 1376→781 LOC (≤800 reached). Original god-component 12,882→781 (−94%).

Co-authored-by: oyi77 <14921983+oyi77@users.noreply.github.com>
… every chatHelpers import

cheaper-inference#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).
diegosouzapw and others added 24 commits June 13, 2026 02:18
…ce#3598)

Integrated into release/v3.8.21 — chatCore phase modularization. Adjusted: re-derive idempotencyKey for the save path after the check moved into the module (co-authored). Thanks @oyi77!
…on) + cheaper-inference#3588 (combo reasoning buffer)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…drop shadow/guardrails doc-fiction (cheaper-inference#3496) (cheaper-inference#3602)

Integrated into release/v3.8.21 — implements GET /api/guardrails + POST /api/guardrails/test, removes shadow/guardrails doc-fiction. TDD-validated (5/5) + check-docs-symbols/typecheck/eslint green.
Split-out PR C from cheaper-inference#3584. Isolates textual reasoning wrappers (<think>/<thinking>/<thought>/<internal_thought>, including malformed/open tags) into reasoning_content across both the non-streaming sanitizer and the Gemini streaming translator, with split-chunk buffering. Additive to the existing textual tool-call pipeline; does not touch the cheaper-inference#3569 native functionResponse path. Integrated into release/v3.8.21. Thanks @dhaern!
…nce#3603)

Split-out PR A from cheaper-inference#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!
…ference#3604)

Split-out PR B from cheaper-inference#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!
…aper-inference#3604 (provider quotas) + cheaper-inference#3605 (reasoning wrappers)

Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
…nference#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 cheaper-inference#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
…nce#3594)

- 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
@oyi77
oyi77 force-pushed the refactor/modularize-combo branch from 7e46500 to 165fb6e Compare June 12, 2026 19:18
@diegosouzapw
diegosouzapw changed the base branch from release/v3.8.23 to release/v3.8.24 June 13, 2026 02:52
@diegosouzapw

Copy link
Copy Markdown
Collaborator

Thanks @oyi77 for the modularization work on open-sse/services/combo.ts (Issue #3594). I can't merge this as it currently stands — and the reason is structural, not about the quality of the split:

  1. It's a stacked/cumulative chain. This branch carries ~49–55 commits all starting from chore(release): open v3.8.23 development cycle, and the per-PR additions grow monotonically (refactor: modularize providerRegistry.ts (Issue #3594) #3617 ≈ +22k → refactor: modularize 3.1K-line providers.ts constants (Issue #3594) #3646 ≈ +52k). Each refactor branch is built on top of the previous one, so merging any single PR drags in all the earlier refactors too. That makes each PR impossible to review in isolation and guarantees conflicts between them.

  2. The base is stale (v3.8.23 era). Since these were cut, release/v3.8.24 has merged dozens of PRs — including changes to the very files several of these PRs modularize (e.g. providerRegistry.ts grew via fix(providers): correct Ollama Cloud kimi-k2.7-code capabilities (#3761) #3768, validation.ts via fix(providers): repair qwen-web validation + stop mislabeling benign redirects as SSRF (#3288, #3758) #3767, plus combo/stream/usage edits). GitHub now reports this PR as CONFLICTING, and the conflicts are substantial.

What would let this land: re-cut it as a single, independent PR branched from the current release/v3.8.24 tip, modularizing only open-sse/services/combo.ts (not stacked on the other refactors), small enough to review on its own. One file per PR, rebased on current release. I'll gladly review and merge them in that form — the god-file modularization goal is worth it; it's the 14-deep stack on a 2-week-old base that can't go in. Leaving this open so nothing is lost.

@oyi77

oyi77 commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Closed in favor of clean re-cut — see #3793 (providerRegistry), #3798 (combo), #3797 (validation), #3796 (imageGeneration), #3795 (usage), #3794 (providers). Only modularization commits were cherry-picked onto current release/v3.8.24 — no stacked dependencies, no unrelated changes.

@oyi77 oyi77 closed this Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.