Skip to content

feat(channel): optional adaptive balance with circuit breaker (default off) - #6301

Closed
xvyimu wants to merge 2 commits into
QuantumNous:mainfrom
xvyimu:pr/b5-adaptive-channel-balance
Closed

feat(channel): optional adaptive balance with circuit breaker (default off)#6301
xvyimu wants to merge 2 commits into
QuantumNous:mainfrom
xvyimu:pr/b5-adaptive-channel-balance

Conversation

@xvyimu

@xvyimu xvyimu commented Jul 18, 2026

Copy link
Copy Markdown

Summary

Fifth small PR from the withdrawn hardening effort: optional adaptive channel balance.

Behavior

  • Default OFF — without flags, selection stays on the existing legacy random/auto-group path.
  • ADAPTIVE_BALANCE_SHADOW_MODE=true: still routes via legacy, logs score comparison only.
  • ADAPTIVE_BALANCE_ENABLED=true (non-shadow): score-based top‑K weighted pick + local circuit breaker.
  • CHANNEL_CIRCUIT_BREAKER_ENABLED=true: open / half-open permits for failing upstreams.

Includes

  • EWMA metrics, scoring, circuit breaker
  • Retry integration in controller/relay.go (concurrency tracking + adaptive result recording)
  • Affinity cache key updates needed by the new selection path

Test plan

  • go test ./service ./controller ./model ./common
  • Manual: flags unset → identical channel selection to stock main
  • Manual: shadow mode → logs only; traffic path unchanged

Notes

Summary by CodeRabbit

  • New Features
    • Added optional adaptive channel balancing that uses latency, success/error signals, rate-limit and concurrency awareness, plus half-open probing and Top‑K weighted selection.
    • Added configurable circuit-breaker and retry/channel tuning (including shadow mode).
    • Improved channel-affinity caching with request scoping and bounded Redis/LRU behavior.
  • Security
    • Restricted realtime WebSocket access to approved origins (valid http/https, same-host allowed).
  • Bug Fixes
    • Hardened relay retry flow: safer permit lifecycle, more reliable channel usage tracking, and quota-aware retry routing.
    • Ensured retries avoid reusing already-selected channels during a request.

…t off)

Add score-based channel selection, local circuit breaker, and EWMA metrics.
Flags default false so behavior matches upstream until operators enable
ADAPTIVE_BALANCE_ENABLED / SHADOW / CHANNEL_CIRCUIT_BREAKER_ENABLED.
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

This PR adds configurable adaptive channel balancing with EWMA metrics, weighted scoring, circuit breakers, retry exclusions, relay integration, and tests. It also scopes and bounds channel-affinity cache keys and adds Redis LRU index management.

Changes

Adaptive channel balance

Layer / File(s) Summary
Configuration and candidate discovery
.env.example, common/..., constant/..., model/..., service/channel_select.go
Adaptive settings, satisfied-channel discovery, explicit exclusions, and request-scoped used-channel tracking are added.
Metrics, scoring, and circuit state
service/channel_metrics.go, service/channel_score.go, service/channel_circuit.go
EWMA metrics, concurrency counters, weighted scoring, and generation-scoped circuit permits are implemented.
Adaptive routing and relay integration
service/channel_adaptive.go, controller/relay.go
Adaptive and shadow routing manage permits, record results, track concurrency, validate websocket origins, and classify upstream quota retries.
Validation
service/channel_adaptive_test.go, controller/*_test.go, model/*_test.go
Tests cover scoring, retries, circuit transitions, origin validation, exclusions, metrics snapshots, and half-open permit handling.

Scoped affinity cache

Layer / File(s) Summary
Scoped keys and Redis lifecycle
service/channel_affinity.go, service/channel_affinity_template_test.go, service/channel_affinity_usage_cache_test.go
Affinity keys use hashed credential-scoped components, while Redis writes maintain a bounded LRU index and cache clearing removes index entries.

Estimated code review effort: 5 (Critical) | ~120 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant Relay
  participant AdaptiveSelector
  participant MetricsAndCircuit
  participant Upstream
  Client->>Relay: submit relay request
  Relay->>AdaptiveSelector: select eligible channel
  AdaptiveSelector->>MetricsAndCircuit: read metrics and circuit permits
  MetricsAndCircuit-->>AdaptiveSelector: scored eligible channels
  AdaptiveSelector-->>Relay: selected channel
  Relay->>Upstream: execute relay attempt
  Upstream-->>Relay: response or error
  Relay->>MetricsAndCircuit: record status, latency, and circuit result
Loading

Possibly related PRs

Suggested reviewers: calcium-ion

Poem

I’m a rabbit routing channels bright,
With EWMA stars to guide the night.
Circuits open, probes peek through,
Retries hop where paths are new.
Scoped keys keep caches neat—
Adaptive hops make balance sweet!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: optional adaptive channel balancing with a circuit breaker, disabled by default.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai 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.

Actionable comments posted: 14

🤖 Prompt for all review comments with 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.

Inline comments:
In `@common/init.go`:
- Around line 116-119: Validate the values read for MaxRetryChannels,
ChannelCooldownSeconds, EwmaAlpha, and MaxChannelConcurrency before assigning
them to constant: reject invalid retry or concurrency values, require a positive
cooldown, and require EwmaAlpha to be finite and within (0,1]. When validation
fails, publish the existing safe defaults instead of the invalid environment
values, including NaN handling for EWMA_ALPHA.

In `@controller/relay.go`:
- Around line 215-230: Update the deferred closure around the relay helper
switch to detect an active panic, release the corresponding adaptive circuit
permit via the existing circuit-breaker mechanism, and then re-panic so
CustomRecovery still handles it. Preserve the existing DecChannelConcurrency
cleanup and normal RecordAdaptiveResult behavior for non-panicking requests.
- Around line 393-400: Reorder the checks in the retry-decision logic around
isUpstreamChannelQuotaError so types.IsSkipRetryError(openaiErr) is evaluated
before types.IsChannelError(openaiErr), ensuring explicit skip-retry errors
return false even when they also satisfy the generic channel check. Preserve the
existing quota-error override ahead of both checks.

In `@model/channel_cache.go`:
- Around line 185-196: Update the channel selection flow around the exclusion
filtering to determine the retry target priority from the original channels
before removing excluded IDs. Then apply exclusions only within the selected
priority tier, preserving tier values and matching the database path so
excluding a higher-priority channel does not cause lower tiers to be skipped.
- Around line 114-126: Update GetSatisfiedChannels so the memory-cache-disabled
path loads every eligible channel matching group, modelName, and requestPath
through a GORM query instead of calling GetChannel. Preserve the cached path’s
enabled-channel filtering and highest-priority ordering, and return all matching
candidates for adaptive balancing.

In `@service/channel_adaptive_test.go`:
- Around line 28-39: The test init function changes package-wide runtime
configuration and globalSnapshot state without restoration, causing unrelated
tests to depend on execution order. Remove this setup from init, add an explicit
fixture for applicable tests that initializes the required constants and metrics
map, and use t.Cleanup to restore the original configuration and globalSnapshot
state after each test.
- Around line 89-105: The tests currently only invoke ScoreCandidates without
verifying filtering, and the retry-exclusion case must exercise the production
selection path. In service/channel_adaptive_test.go lines 89-105, assert that
circuit-open channel 20 is absent from the channels returned by ScoreCandidates;
in lines 108-138, pass channel 30 through the real used-channel retry exclusion
flow and assert it cannot be selected. Use the production adaptive-selection
behavior rather than duplicating its filtering logic or asserting only
implementation details.
- Around line 141-167: Replace the probabilistic loop in
TestTopKWeightedRandomFairness with a deterministic random source injected into
SelectTopKWeighted, using explicit inputs that exercise the top-three weighting
boundaries. Assert the exact selected channel for each boundary value instead of
counting 1,000 trials or applying the 5% ratio threshold; update
SelectTopKWeighted only as needed to accept and use the injectable source.

In `@service/channel_adaptive.go`:
- Around line 287-309: Update the success/failure metric classification in the
relevant channel result handling so non-429 4xx responses are treated as
successes, matching the existing circuit branch that calls
RecordCircuitSuccessWithPermit. Keep 429, 5xx, and other unsuccessful responses
on the ObserveFailure path, while preserving the existing success handling and
circuit-permit logic.
- Around line 178-208: Update getCandidateChannels to return both the candidate
channels and the concrete group resolved from “auto”, preserving the original
group for non-auto requests. Modify AdaptiveSelectChannel to consume and use the
returned group consistently for scoring, affinity queries, context updates,
metrics, and its final result instead of continuing with “auto”.
- Around line 31-37: Update the guard in AdaptiveSelectChannel so shadow-only
mode can continue through adaptive comparison even when AdaptiveBalanceEnabled
is false. Preserve the legacy routing return only when neither full adaptive
balancing nor shadow mode is enabled, and keep the existing recursion-avoidance
behavior for legacy calls.

In `@service/channel_affinity.go`:
- Around line 212-218: Update the channel-affinity clear flow before the Redis
Del call so channelAffinityRedisLRUIndex is deleted only when both Keys
enumeration and DeleteMany complete successfully. Propagate or return
immediately on either failure, and preserve the existing timeout and error
reporting for the index deletion.
- Around line 795-836: Update setChannelAffinityWithLimit to receive the logical
key suffix rather than the already fully namespaced cache key, and use
HybridCache’s established key construction consistently for Redis operations.
Ensure the write updates both Redis and the local memory tier, preferably
through an atomic bounded-write operation on HybridCache, while preserving TTL
and maxEntries behavior; adjust the caller around the existing cacheKey
construction accordingly.

In `@service/channel_circuit.go`:
- Around line 206-217: Update the half-open timeout reset in the circuit permit
flow around HalfOpenInFlight and HalfOpenSince to also invalidate the
outstanding permit by advancing or otherwise changing cb.Generation before
issuing a new probe. Ensure expired requests can no longer close the circuit or
release the replacement probe, while preserving normal generation validation for
active permits.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 47e5da5f-e7a6-4dfe-9343-c8198514f5e3

📥 Commits

Reviewing files that changed from the base of the PR and between 5a6c53d and b605a9c.

📒 Files selected for processing (19)
  • .env.example
  • common/env.go
  • common/init.go
  • constant/env.go
  • controller/relay.go
  • controller/relay_origin_test.go
  • controller/relay_retry_test.go
  • model/ability.go
  • model/channel_cache.go
  • model/channel_selection_exclusion_test.go
  • service/channel_adaptive.go
  • service/channel_adaptive_test.go
  • service/channel_affinity.go
  • service/channel_affinity_template_test.go
  • service/channel_affinity_usage_cache_test.go
  • service/channel_circuit.go
  • service/channel_metrics.go
  • service/channel_score.go
  • service/channel_select.go

Comment thread common/init.go
Comment thread controller/relay.go
Comment thread controller/relay.go Outdated
Comment on lines 393 to 400
if isUpstreamChannelQuotaError(openaiErr) {
return true
}
if types.IsChannelError(openaiErr) {
return true
}
if types.IsSkipRetryError(openaiErr) {
return false

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.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Honor explicit skip-retry errors before generic channel errors.

An error satisfying both IsChannelError and IsSkipRetryError currently returns true at Line 396, ignoring the explicit skip marker. Keep the quota override if intentional, but check IsSkipRetryError before the generic channel-error branch.

Proposed fix
 	if isUpstreamChannelQuotaError(openaiErr) {
 		return true
 	}
-	if types.IsChannelError(openaiErr) {
-		return true
-	}
 	if types.IsSkipRetryError(openaiErr) {
 		return false
 	}
+	if types.IsChannelError(openaiErr) {
+		return true
+	}
📝 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
if isUpstreamChannelQuotaError(openaiErr) {
return true
}
if types.IsChannelError(openaiErr) {
return true
}
if types.IsSkipRetryError(openaiErr) {
return false
if isUpstreamChannelQuotaError(openaiErr) {
return true
}
if types.IsSkipRetryError(openaiErr) {
return false
}
if types.IsChannelError(openaiErr) {
return true
}
🤖 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 `@controller/relay.go` around lines 393 - 400, Reorder the checks in the
retry-decision logic around isUpstreamChannelQuotaError so
types.IsSkipRetryError(openaiErr) is evaluated before
types.IsChannelError(openaiErr), ensuring explicit skip-retry errors return
false even when they also satisfy the generic channel check. Preserve the
existing quota-error override ahead of both checks.

Comment thread model/channel_cache.go Outdated
Comment thread model/channel_cache.go Outdated
Comment on lines +178 to +208
// getCandidateChannels 获取 group+model 全部候选(非单渠道路由)
func getCandidateChannels(group, modelName string, param *RetryParam) ([]*model.Channel, error) {
// auto 分组:优先用上下文已解析的 auto group,否则 legacy 解析一次
if group == "auto" || param.TokenGroup == "auto" {
if g := common.GetContextKeyString(param.Ctx, constant.ContextKeyAutoGroup); g != "" {
group = g
} else {
// 用 legacy 解析 auto → 具体 group,再拉全量候选
ch, selectGroup, err := cacheGetRandomSatisfiedChannelLegacy(param)
if err != nil {
return nil, err
}
if ch == nil {
return nil, nil
}
if selectGroup != "" {
group = selectGroup
}
// 继续用解析后的 group 拉全量;若失败至少返回当前渠道
list, listErr := model.GetSatisfiedChannels(group, modelName, param.RequestPath)
if listErr != nil {
return []*model.Channel{ch}, nil
}
if len(list) == 0 {
return []*model.Channel{ch}, nil
}
return list, nil
}
}

return model.GetSatisfiedChannels(group, modelName, param.RequestPath)

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.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Propagate the resolved auto group back to adaptive selection.

This helper resolves "auto" to a concrete group only in its local variable. The caller continues scoring, querying affinity, storing context, and returning "auto", so metrics and affinity are bucketed under the wrong group.

Return both the candidate list and resolved group, then use that group throughout AdaptiveSelectChannel.

🤖 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 `@service/channel_adaptive.go` around lines 178 - 208, Update
getCandidateChannels to return both the candidate channels and the concrete
group resolved from “auto”, preserving the original group for non-auto requests.
Modify AdaptiveSelectChannel to consume and use the returned group consistently
for scoring, affinity queries, context updates, metrics, and its final result
instead of continuing with “auto”.

Comment thread service/channel_adaptive.go
Comment on lines +212 to +218
if common.RedisEnabled && common.RDB != nil {
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
if err := common.RDB.Del(ctx, channelAffinityRedisLRUIndex).Err(); err != nil {
common.SysError(fmt.Sprintf("channel affinity LRU index clear failed: err=%v", err))
}
}

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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Only delete the LRU index after all affinity keys are deleted.

Lines 202-210 continue after Keys or DeleteMany fails, but this block still deletes the index. Surviving Redis entries then become untracked and the clear operation leaves stale routing data behind.

Gate Del(channelAffinityRedisLRUIndex) on successful key enumeration and deletion.

🤖 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 `@service/channel_affinity.go` around lines 212 - 218, Update the
channel-affinity clear flow before the Redis Del call so
channelAffinityRedisLRUIndex is deleted only when both Keys enumeration and
DeleteMany complete successfully. Propagate or return immediately on either
failure, and preserve the existing timeout and error reporting for the index
deletion.

Comment on lines +795 to +836
if err := setChannelAffinityWithLimit(cache, cacheKey, channelID, time.Duration(ttlSeconds)*time.Second, setting.MaxEntries); err != nil {
common.SysError(fmt.Sprintf("channel affinity cache set failed: key=%s, err=%v", cacheKey, err))
}
}

const channelAffinityRedisSetScript = `
redis.call('SET', KEYS[1], ARGV[1], 'PX', ARGV[2])
redis.call('ZADD', KEYS[2], ARGV[3], KEYS[1])
local max_entries = tonumber(ARGV[4])
if max_entries and max_entries > 0 then
local count = redis.call('ZCARD', KEYS[2])
local overflow = count - max_entries
if overflow > 0 then
local victims = redis.call('ZRANGE', KEYS[2], 0, overflow - 1)
for _, key in ipairs(victims) do
redis.call('DEL', key)
end
redis.call('ZREMRANGEBYRANK', KEYS[2], 0, overflow - 1)
end
end
return 1
`

func setChannelAffinityWithLimit(cache *cachex.HybridCache[int], key string, channelID int, ttl time.Duration, maxEntries int) error {
if !common.RedisEnabled || common.RDB == nil {
return cache.SetWithTTL(key, channelID, ttl)
}
if maxEntries <= 0 {
maxEntries = 100_000
}
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
defer cancel()
_, err := common.RDB.Eval(
ctx,
channelAffinityRedisSetScript,
[]string{cache.FullKey(key), channelAffinityRedisLRUIndex},
strconv.Itoa(channelID),
strconv.FormatInt(ttl.Milliseconds(), 10),
strconv.FormatInt(time.Now().UnixNano(), 10),
strconv.Itoa(maxEntries),
).Result()
return err

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.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Preserve the HybridCache key and memory-tier contracts.

cacheKey is already fully namespaced when stored at Lines 653-655, so Line 830 applies the namespace again. The direct Redis Eval also bypasses the local memory cache, allowing reads to miss the new Redis key or return a stale channel.

Pass the logical suffix into this helper and update the memory tier through HybridCache, preferably by adding an atomic bounded-write operation to that abstraction.

🤖 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 `@service/channel_affinity.go` around lines 795 - 836, Update
setChannelAffinityWithLimit to receive the logical key suffix rather than the
already fully namespaced cache key, and use HybridCache’s established key
construction consistently for Redis operations. Ensure the write updates both
Redis and the local memory tier, preferably through an atomic bounded-write
operation on HybridCache, while preserving TTL and maxEntries behavior; adjust
the caller around the existing cacheKey construction accordingly.

Comment thread service/channel_circuit.go
CodeRabbit #6301 P0:
- allow shadow-only path to score and emit [shadow] LogInfo (5% sample)
- treat non-429 4xx as healthy for metrics (align with circuit)
- release half-open permit on relay helper panic; bump generation on probe timeout
- load full DB candidate set when memory cache is off
- compute retry priority tiers before exclusions
- honor IsSkipRetryError before generic channel-error retry
- validate EWMA/cooldown/concurrency env knobs

@coderabbitai coderabbitai 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.

Actionable comments posted: 2

🤖 Prompt for all review comments with 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.

Inline comments:
In `@model/channel_cache.go`:
- Around line 382-397: Update both ability queries in the surrounding
channel-cache lookup to use the existing commonTrueVal database boolean constant
instead of the hardcoded true value in the enabled predicate, preserving the
current filtering and fallback behavior.
- Around line 419-430: Update the channel filtering logic around
GetChannelsByIds to preserve the priority order from ids: index the returned
channels by their Id, then iterate through ids and append only matching enabled,
non-nil channels. Keep the existing error propagation and filtering behavior
unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b90fa25f-d477-4ec7-95bd-16885d7afb0d

📥 Commits

Reviewing files that changed from the base of the PR and between b605a9c and 063b481.

📒 Files selected for processing (5)
  • common/init.go
  • controller/relay.go
  • model/channel_cache.go
  • service/channel_adaptive.go
  • service/channel_circuit.go
🚧 Files skipped from review as they are similar to previous changes (4)
  • common/init.go
  • controller/relay.go
  • service/channel_circuit.go
  • service/channel_adaptive.go

Comment thread model/channel_cache.go
Comment on lines +382 to +397
err := DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, modelName, true).
Order("priority DESC").Find(&abilities).Error
if err != nil {
return nil, err
}
if len(abilities) == 0 {
normalizedModel := ratio_setting.FormatMatchingModelName(modelName)
if normalizedModel != modelName {
err = DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, normalizedModel, true).
Order("priority DESC").Find(&abilities).Error
if err != nil {
return nil, err
}
modelName = normalizedModel
}
}

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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use commonTrueVal for boolean database comparisons.

Hardcoded boolean true can cause compatibility issues across different database drivers (especially SQLite). As per coding guidelines, use commonTrueVal/commonFalseVal for boolean values in database queries.

🛠️ Proposed fix
-	err := DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, modelName, true).
+	err := DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, modelName, commonTrueVal).
 		Order("priority DESC").Find(&abilities).Error
 	if err != nil {
 		return nil, err
 	}
 	if len(abilities) == 0 {
 		normalizedModel := ratio_setting.FormatMatchingModelName(modelName)
 		if normalizedModel != modelName {
-			err = DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, normalizedModel, true).
+			err = DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, normalizedModel, commonTrueVal).
 				Order("priority DESC").Find(&abilities).Error
 			if err != nil {
 				return nil, err
 			}
 			modelName = normalizedModel
 		}
 	}
📝 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
err := DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, modelName, true).
Order("priority DESC").Find(&abilities).Error
if err != nil {
return nil, err
}
if len(abilities) == 0 {
normalizedModel := ratio_setting.FormatMatchingModelName(modelName)
if normalizedModel != modelName {
err = DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, normalizedModel, true).
Order("priority DESC").Find(&abilities).Error
if err != nil {
return nil, err
}
modelName = normalizedModel
}
}
err := DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, modelName, commonTrueVal).
Order("priority DESC").Find(&abilities).Error
if err != nil {
return nil, err
}
if len(abilities) == 0 {
normalizedModel := ratio_setting.FormatMatchingModelName(modelName)
if normalizedModel != modelName {
err = DB.Where(commonGroupCol+" = ? and model = ? and enabled = ?", group, normalizedModel, commonTrueVal).
Order("priority DESC").Find(&abilities).Error
if err != nil {
return nil, err
}
modelName = normalizedModel
}
}
🤖 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 `@model/channel_cache.go` around lines 382 - 397, Update both ability queries
in the surrounding channel-cache lookup to use the existing commonTrueVal
database boolean constant instead of the hardcoded true value in the enabled
predicate, preserving the current filtering and fallback behavior.

Source: Coding guidelines

Comment thread model/channel_cache.go
Comment on lines +419 to +430
channels, err := GetChannelsByIds(ids)
if err != nil {
return nil, err
}
out := make([]*Channel, 0, len(channels))
for _, ch := range channels {
if ch == nil || ch.Status != common.ChannelStatusEnabled {
continue
}
out = append(out, ch)
}
return out, nil

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Verify the primary key field name in the Channel struct.
ast-grep outline model/channel.go --match Channel

Repository: QuantumNous/new-api

Length of output: 5036


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== model/channel.go: GetChannelsByIds and nearby definitions =="
sed -n '1030,1060p' model/channel.go | cat -n

echo
echo "== model/channel_cache.go: caller around lines 390-440 =="
sed -n '390,440p' model/channel_cache.go | cat -n

echo
echo "== search for call sites of GetChannelsByIds =="
rg -n "GetChannelsByIds\(" -S .

Repository: QuantumNous/new-api

Length of output: 2930


Preserve the priority order here. ids is already built in priority DESC order, but GetChannelsByIds returns rows via IN (?) without preserving that order. Iterating over channels can reshuffle the ranking used for adaptive balancing; map by Id and walk ids instead.

🤖 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 `@model/channel_cache.go` around lines 419 - 430, Update the channel filtering
logic around GetChannelsByIds to preserve the priority order from ids: index the
returned channels by their Id, then iterate through ids and append only matching
enabled, non-nil channels. Keep the existing error propagation and filtering
behavior unchanged.

@xvyimu

xvyimu commented Jul 19, 2026

Copy link
Copy Markdown
Author

Addressed CodeRabbit P0 items for adaptive balance:

  1. Shadow observability: shadow-only path now scores + emits [shadow] ... at Info (5% sample), not Debug-only.
  2. 4xx metrics: non-429 client 4xx treated as healthy (ObserveSuccess) — aligned with circuit classification.
  3. Panic permit: relay helper panic releases adaptive half-open permit then re-panics.
  4. Half-open generation: probe timeout bumps Generation so late responses cannot close a newer probe.
  5. Candidates: when memory cache is off, load full DB candidate set (not one random channel).
  6. Retry priority: compute priority tiers before exclusions.
  7. Skip-retry order: IsSkipRetryError checked before generic channel-error retry.
  8. Env validation: EWMA/cooldown/concurrency knobs rejected when invalid.

Pushed: 063b4815 on pr/b5-adaptive-channel-balance.
Still draft until maintainers review.

@xvyimu

xvyimu commented Jul 23, 2026

Copy link
Copy Markdown
Author

Withdrawing this contribution. No further action needed from maintainers. Closing and deleting the head branch.

@xvyimu xvyimu closed this Jul 23, 2026
@xvyimu
xvyimu deleted the pr/b5-adaptive-channel-balance branch July 23, 2026 05:44
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.

2 participants