Skip to content

feat: /v1/messages -> /v1/responses for claude code - #2866

Closed
seefs001 wants to merge 46 commits into
QuantumNous:mainfrom
seefs001:feature/messages2responses
Closed

feat: /v1/messages -> /v1/responses for claude code#2866
seefs001 wants to merge 46 commits into
QuantumNous:mainfrom
seefs001:feature/messages2responses

Conversation

@seefs001

@seefs001 seefs001 commented Feb 6, 2026

Copy link
Copy Markdown
Collaborator

Summary by CodeRabbit

  • Bug Fixes

    • Chat streaming and endpoint selection now behave consistently across relay modes and model formats; usage reporting for non-OpenAI formats is correctly propagated.
  • Behavior Changes

    • Claude requests may take an early pass-through path in some configurations; response-format translation and token-output handling adjusted.
    • Removed previous hard clamp on max-output-tokens.
  • New Features

    • Per-user search rate limits and paginated token search with a configurable max-user-tokens setting.
    • Session-based billing with automated settlement and refund handling.
  • UI / Localization

    • Subscription/top-up UI refactor, new token limit setting, sandboxed HTML preview, improved code-view escaping, and updated translations.
  • Deprecated

    • Two secure-verification status endpoints removed; some log-search endpoints deprecated.

seefs001 and others added 15 commits February 5, 2026 16:36
…th user creation and binding

- Improve error handling in DeleteCustomOAuthProvider to log and return errors when fetching binding counts.
- Refactor user creation and OAuth binding logic to use transactions for atomic operations, ensuring data integrity.
- Add unique constraints to UserOAuthBinding model to prevent duplicate bindings.
- Enhance GitHub OAuth provider error logging for non-200 responses.
- Update AccountManagement component to provide clearer error messages on API failures.
…ers for optional fields

- Change fields in UpdateCustomOAuthProviderRequest struct to use pointers for optional values, allowing for better handling of nil cases.
- Update UpdateCustomOAuthProvider function to check for nil before assigning optional fields, ensuring existing values are preserved when not provided.
Mitigate XSS vulnerabilities in the playground where AI-generated content
is rendered without sanitization, allowing potential script injection via
prompt injection attacks.

MarkdownRenderer.jsx:
- Replace dangerouslySetInnerHTML with a sandboxed iframe for HTML preview
- Use sandbox="allow-same-origin" to block script execution while allowing
  CSS rendering and iframe height auto-sizing
- Add SandboxedHtmlPreview component with automatic height adjustment

CodeViewer.jsx:
- Add escapeHtml() utility to encode HTML entities before rendering
- Rewrite highlightJson() to process tokens iteratively, escaping each
  token and structural text before wrapping in syntax highlighting spans
- Escape non-JSON and very-large content paths that previously bypassed
  sanitization
- Update linkRegex to correctly match URLs containing & entities

These changes only affect the playground (AI output rendering). Admin-
configured content (home page, about page, footer, notices) remains
unaffected as they use separate code paths and are within the trusted
admin boundary.
@coderabbitai

coderabbitai Bot commented Feb 6, 2026

Copy link
Copy Markdown
Contributor

Walkthrough

This PR adds a BillingSession abstraction (pre-consume/settle/refund), shifts pre-consume/refund flows to session-based billing, extends Claude/Gemini format-aware response streaming and conversion, adds adaptive-thinking/output_config support for Claude opus-4-6 variants, introduces per-user search rate limits and token-setting UI, and includes many ancillary fixes and i18n/UI updates.

Changes

Cohort / File(s) Summary
Billing / Pre-consume Refactor
service/billing_session.go, service/billing.go, service/funding_source.go, service/pre_consume_quota.go
Adds BillingSession, FundingSource implementations, NewBillingSession factory, SettleBilling API; replaces legacy PreConsume/ReturnPreConsumedQuota with session-based pre-consume/settle/refund flows; removes old pre_consume_quota file.
Relay Info & Billing Interface
relay/common/billing.go, relay/common/relay_info.go, relay/compatible_handler.go, controller/relay.go
Introduces BillingSettler interface and RelayInfo.Billing field; updates relay finalization and error refund paths to use BillingSettler.Settle/Refund.
Claude model & request handling
relay/channel/claude/relay-claude.go, relay/claude_handler.go, relay/channel/aws/..., relay/channel/vertex/..., relay/channel/claude/constants.go, relay/channel/aws/dto.go, relay/channel/vertex/dto.go
Adds opus-4-6 model entries and mapping, adds OutputConfig to DTOs, supports adaptive-thinking/effect-suffix parsing (effort levels), and adds early-return path converting Claude requests to Responses API when configured.
OpenAI Responses → Chat conversions & streaming
relay/channel/openai/chat_via_responses.go, relay/channel/openai/adaptor.go, service/openaicompat/chat_to_responses.go
Unifies streaming via sendChatChunk, marshals format-specific payloads for Claude/Gemini/OpenAI, adjusts GetRequestURL behavior for Claude/Gemini in responses modes, and extracts convertChatResponseFormatToResponsesText helper.
Relay/Convert streaming tweaks
service/convert.go, relay/common/relay_info.go
Introduces stopOpenBlocks/stopOpenBlocksAndAdvance helpers, tool-call index tracking, adds ToolCallBaseIndex/ToolCallMaxIndexOffset to ClaudeConvertInfo to manage content_block lifecycle.
Rate limiting & auth
middleware/rate-limit.go, common/constants.go, middleware/auth.go, router/api-router.go
Adds per-user SearchRateLimit factory and Redis-backed user limiter, new TokenAuthReadOnly middleware, increases MaxRecentItems and search rate-limit constants, wires SearchRateLimit and TokenAuthReadOnly into routes.
Models / DB changes
model/token.go, model/log.go, model/subscription.go, model/custom_oauth_provider.go, model/user_oauth_binding.go, model/user.go
SearchUserTokens gains pagination and sanitization; log queries now query by token_id and return errors; SumUsedQuota returns error; HasActiveUserSubscription added; IsSlugTaken now fail-closed; transactional CreateUserOAuthBinding/InsertWithTx/FinalizeOAuthUserCreation additions and DB tag changes for unique constraints.
Controllers: OAuth, tokens, logs, secure verification, relay
controller/oauth.go, controller/token.go, controller/log.go, controller/custom_oauth.go, controller/secure_verification.go, controller/relay.go
Adds transactional OAuth user creation, token creation limit checks and token search pagination, deprecates secure verification endpoints, strengthens log handlers with token-based queries and error checks, changes relay error refund logic to use Billing.
Quota & notify
service/quota.go
Replaces post-consume delta flow with SettleBilling calls; adds subscription-specific quota notification helper.
Settings / operation / ratio
setting/config/config.go, setting/operation_setting/token_setting.go, setting/ratio_setting/*, setting/reasoning/suffix.go
Accept numeric decimal strings for int/uint config, adds TokenSetting.MaxUserTokens (default 1000) and UI wiring, adds model/ratio/cache entries for claude-opus-4-6 variants and adds "-max" effort suffix.
Frontend: token UI, topup/subscriptions, i18n, channel settings
web/src/.../OperationSetting.jsx, web/src/components/topup/*, web/src/components/settings/..., web/src/components/table/..., web/src/i18n/locales/*, web/src/components/.../EditChannelModal.jsx
Adds token_setting.max_user_tokens input, substantial RechargeCard/SubscriptionPlansCard refactor (tabs, props), Search/Token pagination hooks, safer CodeViewer/Markdown sandboxing, new i18n disclaimer and subscription messages, and channel claude_beta_query UI field.
Misc utils & oauth providers
common/utils.go, oauth/github.go, others
Changes float formatting in Interface2String, adds HTTP status/body check in GitHub GetUserInfo, and several small imports/formatting tweaks across files.

Sequence Diagram

sequenceDiagram
    participant Client
    participant ClaudeHandler as Claude Handler
    participant ResponsesAdapter as Responses Adapter
    participant BillingSession as BillingSession
    participant Upstream as Upstream API
    participant StreamWriter as HTTP Stream

    Client->>ClaudeHandler: POST Claude request
    ClaudeHandler->>ResponsesAdapter: Convert to Responses (early-return path)
    ClaudeHandler->>BillingSession: NewBillingSession / PreConsume
    ResponsesAdapter->>Upstream: Forward request (stream)
    Upstream-->>ResponsesAdapter: Streamed chunks
    ResponsesAdapter->>StreamWriter: sendChatChunk (format-aware marshal)
    ResponsesAdapter-->>ClaudeHandler: final usage & finish
    ClaudeHandler->>BillingSession: Settle(actualQuota)
    BillingSession-->>ClaudeHandler: Settlement result
    ClaudeHandler-->>Client: Final response
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

enhancement

Suggested reviewers

  • Calcium-Ion

"I hopped through codepaths, swift and spry,
Claude and Gemini now choose where to fly,
Chunks march in order, streaming in tune,
Quotas settled neat beneath the moon,
🐇✨"

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 37.68% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: migrating Claude message handling from /v1/messages to /v1/responses endpoint.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.40.5)
web/src/components/table/channels/modals/EditChannelModal.jsx

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 and usage tips.

@seefs001 seefs001 linked an issue Feb 6, 2026 that may be closed by this pull request

@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: 1

🤖 Fix all issues with AI agents
In `@service/openaicompat/chat_to_responses.go`:
- Around line 326-329: The clamp on maxOutputTokens (the block that sets
maxOutputTokens = 16 when 0 < maxOutputTokens < 16) is based on an unverified
claim about the Responses API; either remove this silent clamp or replace the
comment with a clear justification and source (e.g., "empirically
observed/responses API rejects values <16 in X test; keep for compatibility")
and add a unit/integration test or runtime validation that asserts behavior
against the Responses API; locate the clamp by searching for the maxOutputTokens
variable in chat_to_responses.go (the conditional that enforces the 16 minimum)
and either delete the conditional or update the comment and tests to document
and verify the reason for keeping it.
🧹 Nitpick comments (1)
service/openaicompat/chat_to_responses.go (1)

124-142: Optional: Extract duplicated assistant tool-call logic into a helper.

The block that iterates msg.ParseToolCalls() and appends function_call items is repeated identically in three branches (nil content, string content, and parsed-parts content). A small helper like appendToolCallItems(inputItems, msg) would reduce ~50 lines of duplication and make future changes less error-prone.

Not urgent — pre-existing and outside the scope of this PR.

Also applies to: 151-169, 212-230

Comment thread service/openaicompat/chat_to_responses.go Outdated
Calcium-Ion and others added 11 commits February 6, 2026 16:16
🔒 fix(security): sanitize AI-generated HTML to prevent XSS in playground
…idation

- Add configurable per-user token creation limit (max_user_tokens)
- Sanitize search input patterns to prevent expensive queries
- Add per-user search rate limiting (by user ID)
- Add pagination to search endpoint with strict page size cap
- Skip empty search fields instead of matching nothing
- Hide internal errors from API responses
- Fix Interface2String float64 formatting causing config parse failures
- Add float-string fallback in config system for int/uint fields
fix: harden token search with pagination, rate limiting and input validation
- Change ESCAPE character from '\' to '!' for compatibility with MySQL/PostgreSQL/SQLite
- Adjust sanitization logic to escape '!' and '_' correctly, improving input validation for search queries
fix: /v1/chat/completions -> /v1/responses json_schema
Calcium-Ion and others added 19 commits February 6, 2026 23:14
将散落在多个文件中的预扣费/结算/退款逻辑抽象为统一的 BillingSession 生命周期管理:

- 新增 BillingSettler 接口 (relay/common/billing.go) 避免循环引用
- 新增 FundingSource 接口 + WalletFunding / SubscriptionFunding 实现 (service/funding_source.go)
- 新增 BillingSession 封装预扣/结算/退款原子操作 (service/billing_session.go)
- 新增 SettleBilling 统一结算辅助函数,替换各 handler 中的 quotaDelta 模式
- 重写 PreConsumeBilling 为 BillingSession 工厂入口
- controller/relay.go 退款守卫改用 BillingSession.Refund()

修复的 Bug:
- 令牌额度泄漏:PreConsumeTokenQuota 成功但 DecreaseUserQuota 失败时未回滚
- 订阅退款遗漏:FinalPreConsumedQuota=0 但 SubscriptionPreConsumed>0 时跳过退款
- 订阅多扣费:subConsume 强制为 1 但 FinalPreConsumedQuota 不同步
- 退款路径不统一:钱包/订阅退款逻辑现统一由 FundingSource.Refund 分派
- Settle 部分失败保护:新增 fundingSettled 标记,资金来源提交后
  令牌调整失败不再导致 Refund 误退已结算的资金
- 订阅多扣费修复:trySubscription 传 subConsume 而非 preConsumedQuota
  给 preConsume,保证三者(amount/preConsume/FinalPreConsumedQuota)一致
- 令牌回滚错误记录:preConsume 中 funding 失败时令牌回滚错误不再丢弃
- 移除钱包路径死代码:用户额度不足的 strings.Contains 匹配不可能命中
- WalletFunding.Refund 不重试:IncreaseUserQuota 非幂等,重试会多退
…e recharge card tabs

- Defaulting to subscriptions when available and avoiding initial flash when no plans exist.
- Adjust the wide-screen layout to place wallet and invite sections side by side, simplify the subscription header and controls, and add padding to prevent card borders from clipping.
- Update related i18n strings by adding the new tab label and removing the obsolete subscription blurb.
…-when-no-plans

✨ refactor(wallet): Top-up layout to embed subscription plans into the recharge card tabs
refactor: 抽象统一计费会话 BillingSession
Add a lightweight active-subscription check to skip subscription pre-consume when none exist, reducing unnecessary transactions and locks. In the subscription UI, disable subscription-first options when no active plan is available, show the effective fallback to wallet with a clear notice, and distinguish “invalidated” from “expired” states. Update i18n strings across supported locales to reflect the new messages and status labels.
Aligns the error variable types in the subscription-first path so that quota fallback checks use the correct NewAPIError.
This prevents build failures and preserves the intended wallet fallback when subscription pre-consume returns an insufficient quota error.
Routes quota alerts through a subscription-specific check when billing from subscriptions, preventing wallet-based thresholds from triggering false warnings.
Updates the notification settings description and localization keys to clarify that both wallet and subscription balances are monitored.
🔔 feat: Add subscription-aware quota notifications and update UI copy
…-fallback

✨ chore: Improve subscription billing fallback and UI states
Modified the formatUserLogs function to include a startIdx parameter, allowing for more flexible log ID assignment. Updated calls to this function in GetLogByTokenId and GetUserLogs to pass the appropriate starting index.
feat: add Codex channel disclaimer (i18n, OpenAI terms)
feat: Force beta=true parameter for Anthropic channel
feat(oauth): implement custom OAuth provider
fix: Claude stream block index/type transitions
fix: add paragraph breaks between reasoning summary chunks
# Conflicts:
#	service/openaicompat/chat_to_responses.go
@seefs001 seefs001 closed this Feb 7, 2026
@seefs001 seefs001 removed a link to an issue Feb 7, 2026

@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: 11

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
web/src/components/playground/CodeViewer.jsx (1)

198-206: ⚠️ Potential issue | 🟡 Minor

Hardcoded Chinese in truncation message — not i18n'd.

Line 204 has the literal string '// ... 内容被截断以提升性能 ...' embedded directly in the truncation logic, while the rest of the component uses t(...) for translations. This will display Chinese regardless of the user's locale.

🌐 Proposed fix: use translation function

Note: t is not available outside the component, so either move this logic into the component where t is accessible, or pass the truncation suffix as a parameter. The simplest approach is to compute displayContent inside the component:

  const displayContent = useMemo(() => {
    if (!contentMetrics.isLarge || isExpanded) {
      return formattedContent;
    }
    return (
      formattedContent.substring(0, PERFORMANCE_CONFIG.PREVIEW_LENGTH) +
-     '\n\n// ... 内容被截断以提升性能 ...'
+     `\n\n// ... ${t('内容被截断以提升性能')} ...`
    );
-  }, [formattedContent, contentMetrics.isLarge, isExpanded]);
+  }, [formattedContent, contentMetrics.isLarge, isExpanded, t]);
web/src/components/topup/index.jsx (1)

460-462: ⚠️ Potential issue | 🟡 Minor

Remove debug console.log statements.

These debug logs for Creem product data should not be shipped to production.

🧹 Proposed fix
          // 设置 Creem 产品
          try {
-           console.log(' data is ?', data);
-           console.log(' creem products is ?', data.creem_products);
            const products = JSON.parse(data.creem_products || '[]');
            setCreemProducts(products);
          } catch (e) {
service/convert.go (1)

438-476: ⚠️ Potential issue | 🟡 Minor

Nil-Index fallback is intentional and safe—all streaming upstreams explicitly set Index.

The code correctly handles toolCall.Index being nil by falling back to the loop iteration index. However, this fallback is defensive: the struct comment in dto/openai_response.go explicitly states "Index is not nil only in chat completion chunk object," and inspection of streaming implementations confirms all upstreams (Gemini, Ollama, Claude) explicitly call SetIndex() before populating tool calls. Non-streaming pathways like responses_to_chat that omit Index don't flow through this code.

The duplicate content_block_start scenario you noted is theoretically possible if a chunk reused loop position i=0, but the state management prevents this: LastMessagesType tracking ensures that when chunks transition contexts (tools → text/thinking or back), stopOpenBlocksAndAdvance() resets ToolCallBaseIndex and ToolCallMaxIndexOffset, preventing index collisions across chunk boundaries.

Consider adding an inline comment documenting that the nil-Index fallback assumes each chunk's tool calls are at distinct loop positions, or validate that adaptor implementations consistently set Index before appending to Delta.ToolCalls.

web/src/hooks/tokens/useTokensData.jsx (1)

344-350: ⚠️ Potential issue | 🟠 Major

useEffect on pageSize causes double-fetch and resets search mode.

When handlePageSizeChange is called in search mode, it calls setPageSize(size) (line 239) followed by searchTokens(1, size) (line 241). However, the state update to pageSize also triggers this useEffect, which unconditionally calls loadTokens(1) — overwriting the search results and resetting searchMode to false.

This means changing page size while in search mode will briefly show search results, then immediately replace them with the full token list.

Proposed fix — guard the useEffect with searchMode

Option 1: Remove pageSize from the dependency array entirely (since handlePageSizeChange already handles the fetch):

   useEffect(() => {
-    loadTokens(1)
+    loadTokens(1, pageSize)
       .then()
       .catch((reason) => {
         showError(reason);
       });
-  }, [pageSize]);
+  }, []); // eslint-disable-line react-hooks/exhaustive-deps

Option 2: Track searchMode in a ref and guard:

   useEffect(() => {
+    if (searchMode) return;
     loadTokens(1)
       .then()
       .catch((reason) => {
         showError(reason);
       });
-  }, [pageSize]);
+  }, [pageSize, searchMode]);

Option 1 is preferred since handlePageSizeChange already manages fetching on size changes.

🤖 Fix all issues with AI agents
In `@controller/token.go`:
- Around line 163-176: Replace the hardcoded Chinese response in the token limit
check with the i18n error helper: when CountUserTokens >=
operation_setting.GetMaxUserTokens(), call common.ApiErrorI18n(c,
"token.max_limit_reached", map[string]interface{}{"max": maxTokens}) (or the
project’s existing param convention) instead of c.JSON(fmt.Sprintf(...)); add a
new i18n key "token.max_limit_reached" with a localized message that accepts the
max param, and remove the now-unused fmt import; locate this change around the
AddToken handler where CountUserTokens and GetMaxUserTokens are used.

In `@middleware/auth.go`:
- Around line 203-211: The handler currently returns internal error text from
model.GetUserCache to the client (err.Error()); change this to return a generic
message (e.g., "internal server error" or "failed to retrieve user data") in the
c.JSON response while preserving the existing c.Abort() and return, and log the
original err server-side instead (use your application's logger or
c.Error/c.Error(err) before responding) so model.GetUserCache errors are not
leaked to clients.

In `@model/log.go`:
- Around line 302-306: The count query currently uses
tx.Model(&Log{}).Limit(logSearchCountLimit).Count(&total) which incorrectly caps
the total; remove the Limit(logSearchCountLimit) from the Count() call so
Count(&total) runs on the full base query (e.g.,
tx.Model(&Log{}).Count(&total)), then apply pagination
(Limit(logSearchCountLimit) and Offset(...)) only on the subsequent fetch query
that loads the actual Log records (the query that does Find/Scan for logs).

In `@model/user_oauth_binding.go`:
- Around line 96-101: The tx.Count call ignores errors which can silently skip
the duplicate check; update the transactional check in the binding flow to
capture and handle the error returned by
tx.Model(&UserOAuthBinding{}).Where(...).Count(&count) (e.g., err :=
tx.Model(...).Where(...).Count(&count).Error) and return that error (or a
wrapped contextual error) when non-nil instead of proceeding; also apply the
same error-checking pattern to the non-transactional IsProviderUserIdTaken
function to ensure queries always propagate failures rather than treating
count==0 as success.

In `@relay/channel/claude/relay-claude.go`:
- Around line 145-153: The adaptive-thinking path triggered by
reasoning.TrimEffortSuffix for claude-opus-4-6-* sets claudeRequest.Thinking and
claudeRequest.OutputConfig but later ReasoningEffort and Reasoning handling
unconditionally overwrite them; add a guard before the ReasoningEffort and
Reasoning blocks so they only set Thinking/OutputConfig when
claudeRequest.Thinking is nil or not adaptive (e.g., check
claudeRequest.Thinking == nil || claudeRequest.Thinking.Type != "adaptive"), and
ensure those blocks do not clear claudeRequest.OutputConfig/TopP/Temperature
when skipping the overwrite.

In `@service/billing_session.go`:
- Around line 39-77: After acquiring s.mu in BillingSession.Settle, add a
defensive check for s.refunded (similar to the existing s.settled check) and
bail out immediately if it's true to avoid concurrent operations against the
same funding/token state; ensure you do this check while holding s.mu so it
races safely with Refund which must set s.refunded under the same mutex. In
short: inside Settle (function name), after s.mu.Lock() and before any
funding/token adjustments, if s.refunded { return nil } (and optionally set
s.settled = true if you want to mark it settled) so that funding.Settle,
fundingSettled, and token quota adjustments are never performed concurrently
with the async Refund goroutine.

In `@service/billing.go`:
- Around line 34-78: The fallback path in SettleBilling is causing subscriptions
to receive wallet-style notifications because PostConsumeQuota unconditionally
calls checkAndSendQuotaNotify when sendEmail=true; update PostConsumeQuota (or
the fallback call) to dispatch like the session path: when
relayInfo.BillingSource == BillingSourceSubscription call
checkAndSendSubscriptionQuotaNotify(relayInfo) else call
checkAndSendQuotaNotify(relayInfo, quota, preConsumedQuota); ensure the
sendEmail branch in PostConsumeQuota uses relayInfo.BillingSource to choose the
correct notification function so subscription notifications match the session
path behavior.

In `@web/src/components/common/markdown/MarkdownRenderer.jsx`:
- Around line 100-119: The iframe load handler registration race can be fixed by
removing the addEventListener logic in the useEffect and instead passing the
same handler to the iframe's onLoad JSX prop; define handleLoad as a stable
function (e.g., via useCallback) that contains the existing try/catch and
setIframeHeight logic, keep the height computation and clamping, and remove the
useEffect addEventListener/removeEventListener code that references iframeRef
and 'load' to avoid missing the event when srcDoc updates; ensure handleLoad's
dependencies (like code if needed) are correct so it sees the latest srcDoc when
invoked.

In `@web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx`:
- Around line 723-730: The Typography.Paragraph rendering branch currently
configures ellipsis with rows: 3 but omits showTooltip, so truncated text can't
be hovered to view full content; update the ellipsis prop on this
Typography.Paragraph instance to include showTooltip: true (i.e., { rows: 3,
showTooltip: true }) so it matches the other render paths and lets users view
the full `content` on hover.

In `@web/src/components/topup/RechargeCard.jsx`:
- Around line 248-261: The onChange handler currently calls await
getAmount(value) on every keystroke and onBlur forces the value to 1; change
this to debounce or defer network calls and use the minTopUp constant for blur
validation: remove the immediate await getAmount from the onChange in
RechargeCard.jsx (or wrap it with a debounce wrapper like debounceGetAmount) and
instead call getAmount when the debounced function triggers or onBlur; in the
onBlur handler replace the hardcoded 1 with minTopUp and ensure you call
setTopUpCount(minTopUp) and getAmount(minTopUp) when parsed value is falsy or <
minTopUp; keep existing setSelectedPreset(null) behavior when appropriate.

In `@web/src/components/topup/SubscriptionPlansCard.jsx`:
- Around line 331-353: The Select currently uses displayBillingPreference for
its value and calls onChangeBillingPreference with the raw new value; when
disableSubscriptionPreference is true this can cause a user action to overwrite
the real billingPreference with the displayed override. Update the Select's
onChange handler (or the onChangeBillingPreference implementation) to detect
when disableSubscriptionPreference is true and the chosen value equals
displayBillingPreference but differs from the actual billingPreference, and in
that case suppress or ignore the change (or map it back to the real
billingPreference) so the backend-stored billingPreference isn't accidentally
overwritten; reference Select, displayBillingPreference,
onChangeBillingPreference, disableSubscriptionPreference, and billingPreference
when locating the logic to modify.
🧹 Nitpick comments (24)
model/custom_oauth_provider.go (1)

100-111: Good fail-closed semantics; consider logging the DB error.

The fail-closed approach is sound — treating DB errors as "slug taken" prevents accidental conflicts. However, the error is silently discarded, which can make transient DB issues hard to diagnose.

💡 Optional: log the error for observability
+	"github.com/QuantumNous/new-api/common/logger"
 ...
 	res := query.Count(&count)
 	if res.Error != nil {
-		// Fail-closed: treat DB errors as slug being taken to prevent conflicts
+		// Fail-closed: treat DB errors as slug being taken to prevent conflicts
+		logger.SysError("IsSlugTaken DB error: " + res.Error.Error())
 		return true
 	}

Adjust the import/logger call to match the project's logging convention.

oauth/github.go (1)

126-135: Good addition — consider bounding the read with io.LimitReader.

io.ReadAll(res.Body) will buffer the entire response into memory before you truncate. If a misbehaving proxy or upstream returns a very large error body, this becomes an unbounded allocation. Wrapping with io.LimitReader is a cheap safeguard:

Suggested hardening
-		body, _ := io.ReadAll(res.Body)
+		body, _ := io.ReadAll(io.LimitReader(res.Body, 1024))
controller/custom_oauth.go (1)

169-184: Partial-update pattern is inconsistent across field types.

Enabled, WellKnown, and AuthStyle correctly use pointer types to distinguish "not provided" from zero-value. However, the remaining string fields (e.g., Scopes, EmailField, DisplayNameField) still rely on != "" checks (lines 248–262), making it impossible for callers to clear those fields to an empty string via the update API.

If clearing those fields is a valid use case, consider making them pointers (or using a sentinel) as well — but this can be deferred since it's a pre-existing pattern.

web/src/components/table/channels/modals/EditChannelModal.jsx (1)

3327-3343: Verify the switch renders with the persisted initial value on edit.

The Form.Switch for claude_beta_query doesn't set an initValue prop (unlike some other switches such as the is_enterprise_account switch at line 1892 which uses initValue={inputs.is_enterprise_account}). While the form's initValues are set from originInputs (which defaults to false), and loadChannel subsequently calls setInputs(data) + formApiRef.current.setValues(data), the value should propagate correctly via the form API.

That said, the auto_ban switch (line 3034) also uses initValue={autoBan} for a similar pattern. Consider adding initValue={inputs.claude_beta_query} for consistency and to ensure the toggle reflects the persisted value when editing an existing channel, especially if there's any timing edge case with form value propagation.

Proposed fix for consistency
 <Form.Switch
   field='claude_beta_query'
   label={t('Claude 强制 beta=true')}
   checkedText={t('开')}
   uncheckedText={t('关')}
   onChange={(value) =>
     handleChannelOtherSettingsChange(
       'claude_beta_query',
       value,
     )
   }
+  initValue={inputs.claude_beta_query}
   extraText={t(
     '开启后,该渠道请求 Claude 时将强制追加 ?beta=true(无需客户端手动传参)',
   )}
 />
web/src/components/playground/CodeViewer.jsx (2)

132-145: Module-scoped linkRegex with g flag used in String.replace — safe, but fragile.

String.prototype.replace resets lastIndex after each call, so this works correctly today. However, if anyone later reuses linkRegex with .exec() or .test() elsewhere, the shared mutable lastIndex will cause subtle bugs. Consider either moving it inside linkifyHtml or dropping the g flag and using replaceAll (or creating the regex per call).


139-142: Potential XSS if linkRegex is ever relaxed — href is not independently escaped.

Currently safe because the regex excludes ", <, >, and ' from matched URLs, preventing attribute breakout. However, the URL is injected raw into href="${url}" without attribute-level escaping. If the regex character class is ever broadened, this becomes an injection vector. A defensive escapeHtml(url) on the href value would make this robust against future regex changes.

🛡️ Defensive escaping for href attribute
      return part.replace(
        linkRegex,
-       (url) => `<a href="${url}" target="_blank" rel="noreferrer">${url}</a>`,
+       (url) => `<a href="${escapeHtml(url)}" target="_blank" rel="noreferrer">${url}</a>`,
      );
web/src/components/topup/RechargeCard.jsx (2)

399-408: localStorage.getItem + JSON.parse called inside .map() on every render — hoist outside the loop.

getCurrencyConfig() (line 400) and the manual localStorage.getItem('status') + JSON.parse (lines 401-408) are called per preset card on every render. This is redundant since the values are identical for every iteration. Hoist them above the .map() call (or useMemo-ize the currency config) to avoid repeated I/O and parsing.

♻️ Proposed fix: hoist currency config outside .map()
+                  {(() => {
+                    const { symbol, rate, type } = getCurrencyConfig();
+                    const statusStr = localStorage.getItem('status');
+                    let usdRate = 7;
+                    try {
+                      if (statusStr) {
+                        const s = JSON.parse(statusStr);
+                        usdRate = s?.usd_exchange_rate || 7;
+                      }
+                    } catch (e) { }
+
+                    return presetAmounts.map((preset, index) => {
-                  {presetAmounts.map((preset, index) => {
                      const discount =
                        preset.discount || topupInfo?.discount?.[preset.value] || 1.0;
                      const originalPrice = preset.value * priceRatio;
                      const discountedPrice = originalPrice * discount;
                      const hasDiscount = discount < 1.0;
                      const actualPay = discountedPrice;
                      const save = originalPrice - discountedPrice;

-                      // 根据当前货币类型换算显示金额和数量
-                      const { symbol, rate, type } = getCurrencyConfig();
-                      const statusStr = localStorage.getItem('status');
-                      let usdRate = 7; // 默认CNY汇率
-                      try {
-                        if (statusStr) {
-                          const s = JSON.parse(statusStr);
-                          usdRate = s?.usd_exchange_rate || 7;
-                        }
-                      } catch (e) { }
-
                      let displayValue = preset.value;
                      // ... rest of the map body unchanged ...
                      return (
                        <Card ... />
                      );
-                    })}
+                    });
+                  })()}

456-463: Discount tag text logic uses t('折').includes('off') — fragile i18n check.

The rendering decides between percentage-off and multiplier format by checking whether the translated string for '折' contains 'off'. This breaks if the translation is anything other than exactly containing "off" (e.g., "descuento", "rabatt"). Consider using the current locale or a dedicated i18n key/flag instead of inspecting translated content.

web/src/components/topup/index.jsx (1)

547-551: Three independent API calls fire in parallel — consider Promise.all for error visibility.

getTopupInfo, getSubscriptionPlans, and getSubscriptionSelf are all fire-and-forget (.then() with no rejection handler). If any of them reject with an unhandled error, it'll surface as an unhandled promise rejection in the console. Wrapping in Promise.all (or Promise.allSettled) lets you handle failures uniformly.

♻️ Suggested improvement
  useEffect(() => {
-   getTopupInfo().then();
-   getSubscriptionPlans().then();
-   getSubscriptionSelf().then();
+   Promise.allSettled([
+     getTopupInfo(),
+     getSubscriptionPlans(),
+     getSubscriptionSelf(),
+   ]);
  }, []);
setting/config/config.go (2)

215-221: Silent float-to-int truncation may mask config errors.

Values like "2.5" will silently become 2. If the intent is only to handle whole-number floats (e.g., "2.000000"), consider validating that the fractional part is zero before converting, so genuine misconfigurations aren't silently swallowed.

💡 Optional: reject non-integer floats
 				floatValue, fErr := strconv.ParseFloat(strValue, 64)
 				if fErr != nil {
 					continue
 				}
+				if floatValue != float64(int64(floatValue)) {
+					continue
+				}
 				intValue = int64(floatValue)

226-232: Same truncation concern for unsigned int path.

Same consideration as the int path — a value like "3.7" silently becomes 3.

relay/claude_handler.go (1)

54-63: Adaptive thinking for opus-4-6 looks correct, minor note on OutputConfig construction.

The logic follows the existing -thinking adapter pattern well. The json.RawMessage(fmt.Sprintf(...)) on line 60 is safe because effortLevel is constrained to values from EffortSuffixes ("max", "high", "medium", "low", "minimal"). However, for robustness, consider validating or using json.Marshal instead of Sprintf:

💡 Optional: safer JSON construction
-		request.OutputConfig = json.RawMessage(fmt.Sprintf(`{"effort":"%s"}`, effortLevel))
+		outputCfg, _ := json.Marshal(map[string]string{"effort": effortLevel})
+		request.OutputConfig = json.RawMessage(outputCfg)
middleware/rate-limit.go (1)

155-196: userRedisRateLimiter is nearly identical to redisRateLimiter — extract a shared helper.

The only difference between userRedisRateLimiter (lines 155-196) and redisRateLimiter (lines 21-65) is how the key is constructed. The entire sliding-window logic is duplicated. Consider refactoring redisRateLimiter to accept a pre-built key, then have the IP-based version construct the key and delegate.

♻️ Suggested refactor
-func redisRateLimiter(c *gin.Context, maxRequestNum int, duration int64, mark string) {
-	ctx := context.Background()
-	rdb := common.RDB
-	key := "rateLimit:" + mark + c.ClientIP()
+func redisRateLimiterByKey(c *gin.Context, maxRequestNum int, duration int64, key string) {
+	ctx := context.Background()
+	rdb := common.RDB
 	listLength, err := rdb.LLen(ctx, key).Result()
 	// ... rest unchanged ...
 }

+func redisRateLimiter(c *gin.Context, maxRequestNum int, duration int64, mark string) {
+	key := "rateLimit:" + mark + c.ClientIP()
+	redisRateLimiterByKey(c, maxRequestNum, duration, key)
+}
+
+func userRedisRateLimiter(c *gin.Context, maxRequestNum int, duration int64, key string) {
+	redisRateLimiterByKey(c, maxRequestNum, duration, key)
+}
model/user.go (1)

432-489: Duplicated logic between Insert and InsertWithTx+FinalizeOAuthUserCreation.

The password hashing, quota init, aff code generation (lines 436-450) duplicate Insert (lines 376-393), and FinalizeOAuthUserCreation (lines 462-489) duplicates Insert's post-creation block (lines 400-428). If either path is updated (e.g., new default settings, different quota logic), the other can easily drift out of sync.

Consider extracting a shared prepareNewUser helper and a finalizeUserCreation helper that both Insert and InsertWithTx/FinalizeOAuthUserCreation call.

controller/oauth.go (1)

296-306: Built-in provider: updates all provider ID columns, not just the relevant one.

SetProviderUserID sets only the one field for the active provider, but the Updates map writes all six provider ID columns. For a freshly created user this is functionally harmless (they're all empty), but it does an unnecessary wider UPDATE. A narrower approach would be to build the map dynamically or update only the single changed column.

Suggested approach
-			provider.SetProviderUserID(user, oauthUser.ProviderUserID)
-			if err := tx.Model(user).Updates(map[string]interface{}{
-				"github_id":    user.GitHubId,
-				"discord_id":   user.DiscordId,
-				"oidc_id":      user.OidcId,
-				"linux_do_id":  user.LinuxDOId,
-				"wechat_id":    user.WeChatId,
-				"telegram_id":  user.TelegramId,
-			}).Error; err != nil {
+			provider.SetProviderUserID(user, oauthUser.ProviderUserID)
+			providerColumn := provider.GetProviderColumn() // e.g. "github_id"
+			if err := tx.Model(user).Update(providerColumn, oauthUser.ProviderUserID).Error; err != nil {

This requires adding a GetProviderColumn() method to the Provider interface (or similar), so treat this as a future improvement.

setting/operation_setting/token_setting.go (1)

25-28: No backend validation for zero or negative MaxUserTokens.

The frontend enforces min={1}, but there's no server-side guard. A direct API call with max_user_tokens: 0 would effectively prevent all users from creating tokens. Consider adding a floor in the getter or during config update.

Suggested guard
 func GetMaxUserTokens() int {
-	return GetTokenSetting().MaxUserTokens
+	v := GetTokenSetting().MaxUserTokens
+	if v <= 0 {
+		return 1000 // safe default
+	}
+	return v
 }
model/user_oauth_binding.go (1)

84-105: Duplicated validation logic with CreateUserOAuthBinding.

The validation block (lines 86-94) is identical to CreateUserOAuthBinding (lines 65-73). Consider extracting a shared validateBinding helper to reduce duplication.

service/funding_source.go (2)

86-102: PreConsume silently ignores its amount parameter — interface contract is misleading.

SubscriptionFunding.PreConsume ignores the amount argument and uses the internally stored s.amount. While the _ int naming signals this, it violates the interface's documented contract ("PreConsume 从该资金来源预扣 amount 额度"). Callers relying on the interface may pass a meaningful value that is silently discarded.

This is partially mitigated by billing_session.go line 303 which carefully passes int(subConsume) to align. Consider at minimum adding a doc comment on this method explaining the deviation.

Also, on line 97, the error from GetSubscriptionPlanInfoByUserSubscriptionId is silently swallowed. If plan info is optional metadata, this is acceptable, but logging the error would aid debugging.


122-139: refundWithRetry uses fixed linear backoff without jitter.

The retry delays are fixed at 200ms and 400ms. For database contention scenarios, adding jitter would reduce thundering-herd effects. This is a minor concern given the small retry count (3), but worth noting for future scalability.

controller/log.go (1)

72-81: Function name GetLogByKey no longer matches its behavior.

The function now retrieves logs by token_id from context rather than by a key parameter. While keeping the function name for route compatibility is understandable, consider adding a comment or alias to reduce confusion for maintainers.

service/quota.go (1)

559-606: Consider extracting shared notification content formatting to reduce duplication.

The notification content formatting logic (Bark/Gotify/Email branching with template strings) in checkAndSendSubscriptionQuotaNotify is nearly identical to checkAndSendQuotaNotify (lines 512–558). If the template or notification types evolve, both functions must be updated in lockstep.

A small helper like buildQuotaNotifyContent(notifyType, prompt, remainingFormatted, topUpLink) could DRY this up.

router/api-router.go (1)

283-286: Appending middleware to logRoute after existing routes — verify intent.

logRoute.Use(middleware.CORS(), middleware.CriticalRateLimit()) is called on line 283 after routes were already registered on lines 270–277. In Gin, middleware added via .Use() only applies to routes registered afterward. This means the CORS and CriticalRateLimit middleware only affect /token (line 285), not the earlier admin/user log routes.

If this is intentional (only the token-auth log endpoint needs CORS + rate limiting), it works correctly but the pattern is subtle. Consider adding a brief comment to clarify, e.g., // CORS + rate limit only for token-auth routes below.

relay/channel/claude/relay-claude.go (1)

145-153: Prefer json.Marshal over fmt.Sprintf for building JSON.

Line 151 interpolates effortLevel directly into a JSON string via fmt.Sprintf. While the current values come from a hardcoded suffix list (EffortSuffixes), using json.Marshal is more robust against future changes to the suffix list that might introduce characters requiring JSON escaping.

Proposed fix
-	claudeRequest.OutputConfig = json.RawMessage(fmt.Sprintf(`{"effort":"%s"}`, effortLevel))
+	outputCfg, _ := json.Marshal(map[string]string{"effort": effortLevel})
+	claudeRequest.OutputConfig = json.RawMessage(outputCfg)
model/token.go (1)

115-117: Pre-existing: strings.Trim removes individual characters, not the prefix "sk-".

strings.Trim(token, "sk-") trims all occurrences of s, k, or - from both ends of the string, not the prefix "sk-". For example, input "sk-mykey-ss" would become "mykey" (stripping trailing ss too). This is pre-existing code (not introduced in this PR), but it could cause unexpected key lookups.

The likely intent was strings.TrimPrefix(token, "sk-").

Comment thread controller/token.go
Comment on lines +163 to +176
// 检查用户令牌数量是否已达上限
maxTokens := operation_setting.GetMaxUserTokens()
count, err := model.CountUserTokens(c.GetInt("id"))
if err != nil {
common.ApiError(c, err)
return
}
if int(count) >= maxTokens {
c.JSON(http.StatusOK, gin.H{
"success": false,
"message": fmt.Sprintf("已达到最大令牌数量限制 (%d)", maxTokens),
})
return
}

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.

⚠️ Potential issue | 🟡 Minor

Hardcoded Chinese string — inconsistent with surrounding i18n usage.

All other user-facing error messages in AddToken use common.ApiErrorI18n (lines 148, 154, 159), but this new limit message uses fmt.Sprintf with a raw Chinese string and c.JSON directly. This should use common.ApiErrorI18n with a new i18n key for consistency and localization support.

Proposed fix
-	if int(count) >= maxTokens {
-		c.JSON(http.StatusOK, gin.H{
-			"success": false,
-			"message": fmt.Sprintf("已达到最大令牌数量限制 (%d)", maxTokens),
-		})
-		return
-	}
+	if int(count) >= maxTokens {
+		common.ApiErrorI18n(c, i18n.MsgTokenCountExceedMax, map[string]any{"Max": maxTokens})
+		return
+	}

This also removes the need for the fmt import added on line 4.

🤖 Prompt for AI Agents
In `@controller/token.go` around lines 163 - 176, Replace the hardcoded Chinese
response in the token limit check with the i18n error helper: when
CountUserTokens >= operation_setting.GetMaxUserTokens(), call
common.ApiErrorI18n(c, "token.max_limit_reached", map[string]interface{}{"max":
maxTokens}) (or the project’s existing param convention) instead of
c.JSON(fmt.Sprintf(...)); add a new i18n key "token.max_limit_reached" with a
localized message that accepts the max param, and remove the now-unused fmt
import; locate this change around the AddToken handler where CountUserTokens and
GetMaxUserTokens are used.

Comment thread middleware/auth.go
Comment on lines +203 to +211
userCache, err := model.GetUserCache(token.UserId)
if err != nil {
c.JSON(http.StatusInternalServerError, gin.H{
"success": false,
"message": err.Error(),
})
c.Abort()
return
}

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.

⚠️ Potential issue | 🟡 Minor

Internal error details exposed to the client.

Line 207 returns err.Error() from model.GetUserCache directly in the response body. This could leak internal details (e.g., database errors, connection info) to the caller. Consider returning a generic message instead.

Proposed fix
 		userCache, err := model.GetUserCache(token.UserId)
 		if err != nil {
+			common.SysError("failed to get user cache: " + err.Error())
 			c.JSON(http.StatusInternalServerError, gin.H{
 				"success": false,
-				"message": err.Error(),
+				"message": "内部服务错误",
 			})
 			c.Abort()
 			return
 		}
🤖 Prompt for AI Agents
In `@middleware/auth.go` around lines 203 - 211, The handler currently returns
internal error text from model.GetUserCache to the client (err.Error()); change
this to return a generic message (e.g., "internal server error" or "failed to
retrieve user data") in the c.JSON response while preserving the existing
c.Abort() and return, and log the original err server-side instead (use your
application's logger or c.Error/c.Error(err) before responding) so
model.GetUserCache errors are not leaked to clients.

Comment thread model/log.go
Comment on lines +302 to 306
err = tx.Model(&Log{}).Limit(logSearchCountLimit).Count(&total).Error
if err != nil {
return nil, 0, err
common.SysError("failed to count user logs: " + err.Error())
return nil, 0, errors.New("查询日志失败")
}

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🌐 Web query:

GORM Limit affect Count query documentation

💡 Result:

In GORM, Count(&total) builds a SELECT count(*) ... using whatever clauses are currently on the query (e.g., Where, Joins, Group, etc.). If you previously added pagination clauses, they can still be present unless you clear them.

How Limit / Offset interact with Count

  • Limit / Offset are part of the query state and can carry over in a chained query or a reused db handle. GORM documents that you can cancel them with:
    • Limit(-1) to cancel limit
    • Offset(-1) to cancel offset
      [1]
  • In GORM v1 docs, Count is documented as needing to be the last call in the chain because it overwrites the SELECT columns. [2]

Recommended pattern for pagination (get total + page data)

Do the total first (no pagination), then fetch the page with Limit/Offset:

var total int64
base := db.Model(&User{}).Where(...)

base.Count(&total)
base.Limit(pageSize).Offset(offset).Find(&users)

This matches how Count is intended to be used (a dedicated count query). [3]

If you already applied pagination and now want the full count

Clear pagination before counting:

base.Limit(-1).Offset(-1).Count(&total)

(Reset behavior for Limit(-1) / Offset(-1) is documented.) [1]

Sources:
[1] GORM v2 Query docs — Limit & Offset (including cancel with -1)
[2] GORM v1 Query docs — Count note about being last in chain
[3] GORM Advanced Query docs — Count usage examples


Remove Limit(logSearchCountLimit) from the Count() query.

In GORM, Limit is part of the query state and does affect Count(). Applying Limit(logSearchCountLimit) before Count() will cap the count at 10,000, which breaks pagination logic—you need the total count of all matching records to calculate page counts correctly.

The proper pattern is to call Count() on the base query without pagination, then apply Limit/Offset only when fetching the actual data:

Proposed fix
-	err = tx.Model(&Log{}).Limit(logSearchCountLimit).Count(&total).Error
+	err = tx.Model(&Log{}).Count(&total).Error
 	if err != nil {
 		common.SysError("failed to count user logs: " + err.Error())
 		return nil, 0, errors.New("查询日志失败")
 	}
📝 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 = tx.Model(&Log{}).Limit(logSearchCountLimit).Count(&total).Error
if err != nil {
return nil, 0, err
common.SysError("failed to count user logs: " + err.Error())
return nil, 0, errors.New("查询日志失败")
}
err = tx.Model(&Log{}).Count(&total).Error
if err != nil {
common.SysError("failed to count user logs: " + err.Error())
return nil, 0, errors.New("查询日志失败")
}
🤖 Prompt for AI Agents
In `@model/log.go` around lines 302 - 306, The count query currently uses
tx.Model(&Log{}).Limit(logSearchCountLimit).Count(&total) which incorrectly caps
the total; remove the Limit(logSearchCountLimit) from the Count() call so
Count(&total) runs on the full base query (e.g.,
tx.Model(&Log{}).Count(&total)), then apply pagination
(Limit(logSearchCountLimit) and Offset(...)) only on the subsequent fetch query
that loads the actual Log records (the query that does Find/Scan for logs).

Comment on lines +96 to +101
// Check if this provider user ID is already taken (use tx to check within the same transaction)
var count int64
tx.Model(&UserOAuthBinding{}).Where("provider_id = ? AND provider_user_id = ?", binding.ProviderId, binding.ProviderUserId).Count(&count)
if count > 0 {
return errors.New("this OAuth account is already bound to another user")
}

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.

⚠️ Potential issue | 🟡 Minor

Unchecked error from tx.Count query.

Line 98 ignores the error returned by tx.Model(...).Count(&count). If the query fails (e.g., connection issue within the transaction), count defaults to 0, silently skipping the duplicate check and allowing a potentially duplicate binding to be created (relying only on the DB constraint).

The non-transactional IsProviderUserIdTaken (line 59) has the same pattern, but in a transactional context this is more consequential since the whole point is atomic consistency.

Proposed fix
-	var count int64
-	tx.Model(&UserOAuthBinding{}).Where("provider_id = ? AND provider_user_id = ?", binding.ProviderId, binding.ProviderUserId).Count(&count)
-	if count > 0 {
+	var count int64
+	if err := tx.Model(&UserOAuthBinding{}).Where("provider_id = ? AND provider_user_id = ?", binding.ProviderId, binding.ProviderUserId).Count(&count).Error; err != nil {
+		return fmt.Errorf("failed to check existing binding: %w", err)
+	}
+	if count > 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
// Check if this provider user ID is already taken (use tx to check within the same transaction)
var count int64
tx.Model(&UserOAuthBinding{}).Where("provider_id = ? AND provider_user_id = ?", binding.ProviderId, binding.ProviderUserId).Count(&count)
if count > 0 {
return errors.New("this OAuth account is already bound to another user")
}
// Check if this provider user ID is already taken (use tx to check within the same transaction)
var count int64
if err := tx.Model(&UserOAuthBinding{}).Where("provider_id = ? AND provider_user_id = ?", binding.ProviderId, binding.ProviderUserId).Count(&count).Error; err != nil {
return fmt.Errorf("failed to check existing binding: %w", err)
}
if count > 0 {
return errors.New("this OAuth account is already bound to another user")
}
🤖 Prompt for AI Agents
In `@model/user_oauth_binding.go` around lines 96 - 101, The tx.Count call ignores
errors which can silently skip the duplicate check; update the transactional
check in the binding flow to capture and handle the error returned by
tx.Model(&UserOAuthBinding{}).Where(...).Count(&count) (e.g., err :=
tx.Model(...).Where(...).Count(&count).Error) and return that error (or a
wrapped contextual error) when non-nil instead of proceeding; also apply the
same error-checking pattern to the non-transactional IsProviderUserIdTaken
function to ensure queries always propagate failures rather than treating
count==0 as success.

Comment on lines +145 to +153
if baseModel, effortLevel, ok := reasoning.TrimEffortSuffix(textRequest.Model); ok && effortLevel != "" &&
strings.HasPrefix(textRequest.Model, "claude-opus-4-6") {
claudeRequest.Model = baseModel
claudeRequest.Thinking = &dto.Thinking{
Type: "adaptive",
}
claudeRequest.OutputConfig = json.RawMessage(fmt.Sprintf(`{"effort":"%s"}`, effortLevel))
claudeRequest.TopP = 0
claudeRequest.Temperature = common.GetPointer[float64](1.0)

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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the full context of relay-claude.go around lines 145-210
fd relay-claude.go --type f | head -1 | xargs wc -l

Repository: QuantumNous/new-api

Length of output: 103


🏁 Script executed:

# Get the complete code section from lines 130-220 to understand the full context
fd relay-claude.go --type f | head -1 | xargs cat -n | sed -n '130,220p'

Repository: QuantumNous/new-api

Length of output: 3795


🏁 Script executed:

# Search for any documentation or comments about adaptive thinking and ReasoningEffort interaction
rg -n -B2 -A2 'adaptive.*thinking|ReasoningEffort.*adaptive' --type=go relay/channel/claude/

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

# Check if there are test files that document this behavior
fd relay-claude -t f | grep -E '(test|spec)'

Repository: QuantumNous/new-api

Length of output: 45


🏁 Script executed:

# Search for how ReasoningEffort and Thinking fields are used together
rg -n 'ReasoningEffort|Reasoning' --type=go relay/channel/claude/relay-claude.go | head -20

Repository: QuantumNous/new-api

Length of output: 550


🏁 Script executed:

# Search for any documentation or tests that clarify expected behavior
fd -t f relay-claude --type f

Repository: QuantumNous/new-api

Length of output: 99


🏁 Script executed:

# Check for test files or documentation about this interaction
rg -n -i 'adaptive.*reasoning|reasoning.*adaptive|effort.*reasoning' --type=go relay/

Repository: QuantumNous/new-api

Length of output: 1554


🏁 Script executed:

# Look for any comments explaining the precedence or mutual exclusivity
rg -B5 'ReasoningEffort|Reasoning' --type=go relay/channel/claude/relay-claude.go | head -40

Repository: QuantumNous/new-api

Length of output: 1049


🏁 Script executed:

# Check how claude_handler.go handles the effort suffix parsing and if it prevents ReasoningEffort from being set
sed -n '40,80p' relay/claude_handler.go

Repository: QuantumNous/new-api

Length of output: 1717


🏁 Script executed:

# Look for where ReasoningEffort is set in the request to see if it's cleared when effort suffix is used
rg -B10 -A10 'TrimEffortSuffix' relay/claude_handler.go

Repository: QuantumNous/new-api

Length of output: 933


Add guard to prevent ReasoningEffort/Reasoning from overwriting adaptive thinking.

When a claude-opus-4-6-* model with effort suffix triggers the adaptive thinking path (lines 145–153), the subsequent ReasoningEffort (lines 176–193) and Reasoning (lines 197–209) blocks will unconditionally overwrite claudeRequest.Thinking and discard the OutputConfig effort configuration.

Unlike claude_handler.go which guards with if request.Thinking == nil, this code lacks protection. Add a check such as if claudeRequest.Thinking == nil || claudeRequest.Thinking.Type != "adaptive" before lines 176 and 197 to prevent the override, or document the intended precedence.

🤖 Prompt for AI Agents
In `@relay/channel/claude/relay-claude.go` around lines 145 - 153, The
adaptive-thinking path triggered by reasoning.TrimEffortSuffix for
claude-opus-4-6-* sets claudeRequest.Thinking and claudeRequest.OutputConfig but
later ReasoningEffort and Reasoning handling unconditionally overwrite them; add
a guard before the ReasoningEffort and Reasoning blocks so they only set
Thinking/OutputConfig when claudeRequest.Thinking is nil or not adaptive (e.g.,
check claudeRequest.Thinking == nil || claudeRequest.Thinking.Type !=
"adaptive"), and ensure those blocks do not clear
claudeRequest.OutputConfig/TopP/Temperature when skipping the overwrite.

Comment thread service/billing.go
Comment on lines +34 to 78
func SettleBilling(ctx *gin.Context, relayInfo *relaycommon.RelayInfo, actualQuota int) error {
if relayInfo.Billing != nil {
preConsumed := relayInfo.Billing.GetPreConsumedQuota()
delta := actualQuota - preConsumed

relayInfo.BillingSource = BillingSourceSubscription
relayInfo.SubscriptionId = res.UserSubscriptionId
relayInfo.SubscriptionPreConsumed = res.PreConsumed
relayInfo.SubscriptionPostDelta = 0
relayInfo.SubscriptionAmountTotal = res.AmountTotal
relayInfo.SubscriptionAmountUsedAfterPreConsume = res.AmountUsedAfter
if planInfo, err := model.GetSubscriptionPlanInfoByUserSubscriptionId(res.UserSubscriptionId); err == nil && planInfo != nil {
relayInfo.SubscriptionPlanId = planInfo.PlanId
relayInfo.SubscriptionPlanTitle = planInfo.PlanTitle
if delta > 0 {
logger.LogInfo(ctx, fmt.Sprintf("预扣费后补扣费:%s(实际消耗:%s,预扣费:%s)",
logger.FormatQuota(delta),
logger.FormatQuota(actualQuota),
logger.FormatQuota(preConsumed),
))
} else if delta < 0 {
logger.LogInfo(ctx, fmt.Sprintf("预扣费后返还扣费:%s(实际消耗:%s,预扣费:%s)",
logger.FormatQuota(-delta),
logger.FormatQuota(actualQuota),
logger.FormatQuota(preConsumed),
))
} else {
logger.LogInfo(ctx, fmt.Sprintf("预扣费与实际消耗一致,无需调整:%s(按次计费)",
logger.FormatQuota(actualQuota),
))
}
relayInfo.FinalPreConsumedQuota = preConsumedQuota

logger.LogInfo(c, fmt.Sprintf("用户 %d 使用订阅计费预扣:订阅=%d,token_quota=%d", relayInfo.UserId, res.PreConsumed, preConsumedQuota))
return nil
}

tryWallet := func() *types.NewAPIError {
relayInfo.BillingSource = BillingSourceWallet
relayInfo.SubscriptionId = 0
relayInfo.SubscriptionPreConsumed = 0
return PreConsumeQuota(c, preConsumedQuota, relayInfo)
}

switch pref {
case "subscription_only":
return trySubscription()
case "wallet_only":
return tryWallet()
case "wallet_first":
if err := tryWallet(); err != nil {
// only fallback for insufficient wallet quota
if err.GetErrorCode() == types.ErrorCodeInsufficientUserQuota {
return trySubscription()
}
if err := relayInfo.Billing.Settle(actualQuota); err != nil {
return err
}
return nil
case "subscription_first":
fallthrough
default:
if err := trySubscription(); err != nil {
// fallback only when subscription not available/insufficient
if err.GetErrorCode() == types.ErrorCodeInsufficientUserQuota {
return tryWallet()

// 发送额度通知(订阅计费使用订阅剩余额度)
if actualQuota != 0 {
if relayInfo.BillingSource == BillingSourceSubscription {
checkAndSendSubscriptionQuotaNotify(relayInfo)
} else {
checkAndSendQuotaNotify(relayInfo, actualQuota-preConsumed, preConsumed)
}
return err
}
return nil
}

// 回退:无 BillingSession 时使用旧路径
quotaDelta := actualQuota - relayInfo.FinalPreConsumedQuota
if quotaDelta != 0 {
return PostConsumeQuota(relayInfo, quotaDelta, relayInfo.FinalPreConsumedQuota, true)
}
return 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.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for PostConsumeQuota function definition
rg 'func PostConsumeQuota' -A 30

Repository: QuantumNous/new-api

Length of output: 1580


🏁 Script executed:

#!/bin/bash
# Search for BillingSession.Settle method
rg 'func.*Settle\(' -A 20 | head -100

Repository: QuantumNous/new-api

Length of output: 3324


🏁 Script executed:

#!/bin/bash
# Search for checkAndSendQuotaNotify and checkAndSendSubscriptionQuotaNotify
rg 'func (checkAndSendQuotaNotify|checkAndSendSubscriptionQuotaNotify)' -A 15

Repository: QuantumNous/new-api

Length of output: 1679


🏁 Script executed:

#!/bin/bash
# Get complete PostConsumeQuota function
rg 'func PostConsumeQuota' -A 60 service/quota.go

Repository: QuantumNous/new-api

Length of output: 1771


🏁 Script executed:

#!/bin/bash
# Search for sendEmail parameter usage in PostConsumeQuota or related functions
rg 'sendEmail' -B 5 -A 10 service/quota.go

Repository: QuantumNous/new-api

Length of output: 896


Notification behavior differs between session and fallback paths for subscriptions — fix required.

The session path (lines 54–66) correctly dispatches notifications: for subscriptions it calls checkAndSendSubscriptionQuotaNotify(relayInfo), while for wallet quotas it calls checkAndSendQuotaNotify(relayInfo, actualQuota-preConsumed, preConsumed).

However, the fallback path (lines 72–77) calls PostConsumeQuota(relayInfo, quotaDelta, relayInfo.FinalPreConsumedQuota, true). Within PostConsumeQuota, when sendEmail=true, it unconditionally calls checkAndSendQuotaNotify(relayInfo, quota, preConsumedQuota) regardless of relayInfo.BillingSource. This means subscriptions in the fallback path use the wallet notification function instead of the subscription-specific one, producing inconsistent user notifications.

Update PostConsumeQuota to check relayInfo.BillingSource and dispatch to the correct notification function, or handle subscription notifications separately in the fallback code path to match the session path behavior.

🤖 Prompt for AI Agents
In `@service/billing.go` around lines 34 - 78, The fallback path in SettleBilling
is causing subscriptions to receive wallet-style notifications because
PostConsumeQuota unconditionally calls checkAndSendQuotaNotify when
sendEmail=true; update PostConsumeQuota (or the fallback call) to dispatch like
the session path: when relayInfo.BillingSource == BillingSourceSubscription call
checkAndSendSubscriptionQuotaNotify(relayInfo) else call
checkAndSendQuotaNotify(relayInfo, quota, preConsumedQuota); ensure the
sendEmail branch in PostConsumeQuota uses relayInfo.BillingSource to choose the
correct notification function so subscription notifications match the session
path behavior.

Comment on lines +100 to +119
useEffect(() => {
const iframe = iframeRef.current;
if (!iframe) return;

const handleLoad = () => {
try {
const doc = iframe.contentDocument || iframe.contentWindow?.document;
if (doc) {
const height =
doc.documentElement.scrollHeight || doc.body.scrollHeight;
setIframeHeight(Math.min(Math.max(height + 16, 60), 600));
}
} catch {
// sandbox restrictions may prevent access, that's fine
}
};

iframe.addEventListener('load', handleLoad);
return () => iframe.removeEventListener('load', handleLoad);
}, [code]);

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.

⚠️ Potential issue | 🟡 Minor

Potential missed load event when code changes.

When code updates, React writes the new srcDoc synchronously, which may trigger the iframe's load event before this useEffect fires (effects run after paint). If that happens, the new listener misses the event, and iframeHeight stays stale at 150 px.

Using the onLoad JSX prop eliminates the race and simplifies the component.

Proposed fix
 function SandboxedHtmlPreview({ code }) {
   const iframeRef = useRef(null);
   const [iframeHeight, setIframeHeight] = useState(150);

-  useEffect(() => {
-    const iframe = iframeRef.current;
-    if (!iframe) return;
-
-    const handleLoad = () => {
-      try {
-        const doc = iframe.contentDocument || iframe.contentWindow?.document;
-        if (doc) {
-          const height =
-            doc.documentElement.scrollHeight || doc.body.scrollHeight;
-          setIframeHeight(Math.min(Math.max(height + 16, 60), 600));
-        }
-      } catch {
-        // sandbox restrictions may prevent access, that's fine
-      }
-    };
-
-    iframe.addEventListener('load', handleLoad);
-    return () => iframe.removeEventListener('load', handleLoad);
-  }, [code]);
+  const handleLoad = () => {
+    try {
+      const iframe = iframeRef.current;
+      const doc = iframe?.contentDocument || iframe?.contentWindow?.document;
+      if (doc) {
+        const height =
+          doc.documentElement.scrollHeight || doc.body.scrollHeight;
+        setIframeHeight(Math.min(Math.max(height + 16, 60), 600));
+      }
+    } catch {
+      // sandbox restrictions may prevent access, that's fine
+    }
+  };

   return (
     <iframe
       ref={iframeRef}
       sandbox='allow-same-origin'
       srcDoc={code}
       title='HTML Preview'
+      onLoad={handleLoad}
       style={{
📝 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
useEffect(() => {
const iframe = iframeRef.current;
if (!iframe) return;
const handleLoad = () => {
try {
const doc = iframe.contentDocument || iframe.contentWindow?.document;
if (doc) {
const height =
doc.documentElement.scrollHeight || doc.body.scrollHeight;
setIframeHeight(Math.min(Math.max(height + 16, 60), 600));
}
} catch {
// sandbox restrictions may prevent access, that's fine
}
};
iframe.addEventListener('load', handleLoad);
return () => iframe.removeEventListener('load', handleLoad);
}, [code]);
const handleLoad = () => {
try {
const iframe = iframeRef.current;
const doc = iframe?.contentDocument || iframe?.contentWindow?.document;
if (doc) {
const height =
doc.documentElement.scrollHeight || doc.body.scrollHeight;
setIframeHeight(Math.min(Math.max(height + 16, 60), 600));
}
} catch {
// sandbox restrictions may prevent access, that's fine
}
};
return (
<iframe
ref={iframeRef}
sandbox='allow-same-origin'
srcDoc={code}
title='HTML Preview'
onLoad={handleLoad}
style={{
width: '100%',
height: `${iframeHeight}px`,
border: 'none',
borderRadius: '4px',
}}
/>
);
🤖 Prompt for AI Agents
In `@web/src/components/common/markdown/MarkdownRenderer.jsx` around lines 100 -
119, The iframe load handler registration race can be fixed by removing the
addEventListener logic in the useEffect and instead passing the same handler to
the iframe's onLoad JSX prop; define handleLoad as a stable function (e.g., via
useCallback) that contains the existing try/catch and setIframeHeight logic,
keep the height computation and clamping, and remove the useEffect
addEventListener/removeEventListener code that references iframeRef and 'load'
to avoid missing the event when srcDoc updates; ensure handleLoad's dependencies
(like code if needed) are correct so it sees the latest srcDoc when invoked.

Comment on lines +723 to +730
<Typography.Paragraph
ellipsis={{
rows: 3,
}}
style={{ maxWidth: 240, whiteSpace: 'pre-line' }}
>
{content}
</Typography.Paragraph>

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.

⚠️ Potential issue | 🟡 Minor

Missing showTooltip on the ellipsis — truncated content won't be viewable.

The other render paths in this same column (lines 636–647 and 666–678) include showTooltip in their ellipsis config so users can hover to see the full text. This path omits it, so when content exceeds 3 rows it will just be cut off with no way to read the rest.

Proposed fix
             <Typography.Paragraph
                 ellipsis={{
                   rows: 3,
+                  showTooltip: {
+                    type: 'popover',
+                    opts: { style: { width: 240 } },
+                  },
                 }}
                 style={{ maxWidth: 240, whiteSpace: 'pre-line' }}
             >
📝 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
<Typography.Paragraph
ellipsis={{
rows: 3,
}}
style={{ maxWidth: 240, whiteSpace: 'pre-line' }}
>
{content}
</Typography.Paragraph>
<Typography.Paragraph
ellipsis={{
rows: 3,
showTooltip: {
type: 'popover',
opts: { style: { width: 240 } },
},
}}
style={{ maxWidth: 240, whiteSpace: 'pre-line' }}
>
{content}
</Typography.Paragraph>
🤖 Prompt for AI Agents
In `@web/src/components/table/usage-logs/UsageLogsColumnDefs.jsx` around lines 723
- 730, The Typography.Paragraph rendering branch currently configures ellipsis
with rows: 3 but omits showTooltip, so truncated text can't be hovered to view
full content; update the ellipsis prop on this Typography.Paragraph instance to
include showTooltip: true (i.e., { rows: 3, showTooltip: true }) so it matches
the other render paths and lets users view the full `content` on hover.

Comment on lines +248 to +261
onChange={async (value) => {
if (value && value >= 1) {
setTopUpCount(value);
setSelectedPreset(null);
await getAmount(value);
}
extraText={
<Skeleton
loading={showAmountSkeleton}
active
placeholder={
<Skeleton.Title
style={{
width: 120,
height: 20,
borderRadius: 6,
}}
/>
}
>
<Text type='secondary' className='text-red-600'>
{t('实付金额:')}
<span style={{ color: 'red' }}>
{renderAmount()}
</span>
</Text>
</Skeleton>
}}
onBlur={(e) => {
const value = parseInt(e.target.value);
if (!value || value < 1) {
setTopUpCount(1);
getAmount(1);
}
style={{ width: '100%' }}
/>
</Col>
<Col xs={24} sm={24} md={24} lg={14} xl={14}>
<Form.Slot label={t('选择支付方式')}>
{payMethods && payMethods.length > 0 ? (
<Space wrap>
{payMethods.map((payMethod) => {
const minTopupVal =
Number(payMethod.min_topup) || 0;
const isStripe = payMethod.type === 'stripe';
const disabled =
(!enableOnlineTopUp && !isStripe) ||
(!enableStripeTopUp && isStripe) ||
minTopupVal > Number(topUpCount || 0);

const buttonEl = (
<Button
key={payMethod.type}
theme='outline'
type='tertiary'
onClick={() => preTopUp(payMethod.type)}
disabled={disabled}
loading={
paymentLoading && payWay === payMethod.type
}
icon={
payMethod.type === 'alipay' ? (
<SiAlipay size={18} color='#1677FF' />
) : payMethod.type === 'wxpay' ? (
<SiWechat size={18} color='#07C160' />
) : payMethod.type === 'stripe' ? (
<SiStripe size={18} color='#635BFF' />
) : (
<CreditCard
size={18}
color={
payMethod.color ||
'var(--semi-color-text-2)'
}
/>
)
}
className='!rounded-lg !px-4 !py-2'
>
{payMethod.name}
</Button>
);

return disabled &&
minTopupVal > Number(topUpCount || 0) ? (
<Tooltip
content={
t('此支付方式最低充值金额为') +
' ' +
minTopupVal
}
key={payMethod.type}
>
{buttonEl}
</Tooltip>
) : (
<React.Fragment key={payMethod.type}>
{buttonEl}
</React.Fragment>
);
})}
</Space>
) : (
<div className='text-gray-500 text-sm p-3 bg-gray-50 rounded-lg border border-dashed border-gray-300'>
{t('暂无可用的支付方式,请联系管理员配置')}
</div>
)}
</Form.Slot>
</Col>
</Row>
)}

{(enableOnlineTopUp || enableStripeTopUp) && (
<Form.Slot
label={
<div className='flex items-center gap-2'>
<span>{t('选择充值额度')}</span>
{(() => {
const { symbol, rate, type } = getCurrencyConfig();
if (type === 'USD') return null;

return (
<span
}}

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.

⚠️ Potential issue | 🟡 Minor

onChange fires an API call (getAmount) on every keystroke with no debounce; onBlur resets to 1 instead of minTopUp.

Two issues:

  1. No debounce on getAmount: Each character typed in the input triggers await getAmount(value), which is a network request. Consider debouncing or deferring the call to onBlur.

  2. onBlur resets to 1 (line 258), but the minimum allowed value is minTopUp. If minTopUp > 1, the user ends up with an invalid value after blur.

🐛 Proposed fix for onBlur
                      onBlur={(e) => {
                        const value = parseInt(e.target.value);
-                       if (!value || value < 1) {
-                         setTopUpCount(1);
-                         getAmount(1);
+                       if (!value || value < minTopUp) {
+                         setTopUpCount(minTopUp);
+                         getAmount(minTopUp);
                        }
                      }}
🤖 Prompt for AI Agents
In `@web/src/components/topup/RechargeCard.jsx` around lines 248 - 261, The
onChange handler currently calls await getAmount(value) on every keystroke and
onBlur forces the value to 1; change this to debounce or defer network calls and
use the minTopUp constant for blur validation: remove the immediate await
getAmount from the onChange in RechargeCard.jsx (or wrap it with a debounce
wrapper like debounceGetAmount) and instead call getAmount when the debounced
function triggers or onBlur; in the onBlur handler replace the hardcoded 1 with
minTopUp and ensure you call setTopUpCount(minTopUp) and getAmount(minTopUp)
when parsed value is falsy or < minTopUp; keep existing setSelectedPreset(null)
behavior when appropriate.

Comment on lines +331 to +353
<Select
value={displayBillingPreference}
onChange={onChangeBillingPreference}
size='small'
optionList={[
{
value: 'subscription_first',
label: disableSubscriptionPreference
? `${t('优先订阅')} (${t('无生效')})`
: t('优先订阅'),
disabled: disableSubscriptionPreference,
},
{ value: 'wallet_first', label: t('优先钱包') },
{
value: 'subscription_only',
label: disableSubscriptionPreference
? `${t('仅用订阅')} (${t('无生效')})`
: t('仅用订阅'),
disabled: disableSubscriptionPreference,
},
{ value: 'wallet_only', label: t('仅用钱包') },
]}
/>

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.

⚠️ Potential issue | 🟡 Minor

Select value is the overridden displayBillingPreference, but onChange fires onChangeBillingPreference with the raw new value — verify this is intentional.

When disableSubscriptionPreference is true, the Select shows wallet_first via displayBillingPreference, but the saved preference on the backend may still be subscription_first. If the user selects wallet_first explicitly, onChangeBillingPreference will overwrite the saved preference, silently losing the user's original subscription-based preference. Consider whether onChange should be suppressed (or filtered) when the new value equals the overridden display value but differs from the actual billingPreference.

🤖 Prompt for AI Agents
In `@web/src/components/topup/SubscriptionPlansCard.jsx` around lines 331 - 353,
The Select currently uses displayBillingPreference for its value and calls
onChangeBillingPreference with the raw new value; when
disableSubscriptionPreference is true this can cause a user action to overwrite
the real billingPreference with the displayed override. Update the Select's
onChange handler (or the onChangeBillingPreference implementation) to detect
when disableSubscriptionPreference is true and the chosen value equals
displayBillingPreference but differs from the actual billingPreference, and in
that case suppress or ignore the change (or map it back to the real
billingPreference) so the backend-stored billingPreference isn't accidentally
overwritten; reference Select, displayBillingPreference,
onChangeBillingPreference, disableSubscriptionPreference, and billingPreference
when locating the logic to modify.

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.

6 participants