Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR adds durable billing settlements and cache invalidation, updates task relay and polling behavior, expands secure verification to account deletion, adds signed Midjourney image URLs, centralizes session verification, and updates channel, pricing, provider, and frontend configuration flows. ChangesPlatform behavior
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
@CodeRabbit review --committed |
|
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 34
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
web/default/src/features/models/components/drawers/model-mutate-drawer.tsx (1)
99-117: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winAdd descriptive messages to the new pricing
.refine()calls.These 7 fields previously had no schema-level validation; now that
isOptionalPricingNumbercan fail, none of the.refine()calls supply a message, so a failure would surface Zod's generic default text inFormMessageinstead of a clear, translatable string like the other refine calls in this codebase use (e.g.channelFormSchema'sstatus_code_mapping/model_mapping).♻️ Suggested fix
- price: z.string().optional().refine(isOptionalPricingNumber), - ratio: z.string().optional().refine(isOptionalPricingNumber), - cacheRatio: z.string().optional().refine(isOptionalPricingNumber), - completionRatio: z.string().optional().refine(isOptionalPricingNumber), - imageRatio: z.string().optional().refine(isOptionalPricingNumber), - audioRatio: z.string().optional().refine(isOptionalPricingNumber), - audioCompletionRatio: z.string().optional().refine(isOptionalPricingNumber), + price: z.string().optional().refine(isOptionalPricingNumber, 'Must be a non-negative number'), + ratio: z.string().optional().refine(isOptionalPricingNumber, 'Must be a non-negative number'), + cacheRatio: z.string().optional().refine(isOptionalPricingNumber, 'Must be a non-negative number'), + completionRatio: z.string().optional().refine(isOptionalPricingNumber, 'Must be a non-negative number'), + imageRatio: z.string().optional().refine(isOptionalPricingNumber, 'Must be a non-negative number'), + audioRatio: z.string().optional().refine(isOptionalPricingNumber, 'Must be a non-negative number'), + audioCompletionRatio: z.string().optional().refine(isOptionalPricingNumber, 'Must be a non-negative number'),🤖 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 `@web/default/src/features/models/components/drawers/model-mutate-drawer.tsx` around lines 99 - 117, Add clear, translatable validation messages to each pricing field’s isOptionalPricingNumber refine call in extendedModelFormSchema: price, ratio, cacheRatio, completionRatio, imageRatio, audioRatio, and audioCompletionRatio. Use the existing message conventions from nearby schema refinements so FormMessage displays a descriptive error instead of Zod’s generic text.relay/mjproxy_handler.go (1)
314-314: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRe-derive
status_code_mappingwhen switching to the origin Midjourney channel.In
RelayMidjourneyTaskImageSeedandRelayMidjourneySubmit,channel_id/base_urlare replaced withoriginTask.ChannelId, butwriteMidjourneyStatusCodestill readsc.GetString("status_code_mapping"). If that context value came from the initially selected channel, status codes are remapped with the wrong channel’s mapping. Update the context with the origin channel’sstatus_code_mappingalongsidechannel_id/base_urlbefore callingwriteMidjourneyStatusCode.Also applies to lines 314, 324, 488-489, and 659.
🤖 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 `@relay/mjproxy_handler.go` at line 314, When RelayMidjourneyTaskImageSeed and RelayMidjourneySubmit switch context to originTask.ChannelId and its base URL, also set status_code_mapping from the origin channel before any writeMidjourneyStatusCode call. Update each referenced channel-switching block, including the assignments near lines 314, 324, 488-489, and 659, so the mapping always matches the active origin channel.service/task_polling.go (1)
509-546: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
shouldRefund/shouldSettleare now dead after the CAS block.Billing is gated solely by
transitionWon && settlement != nil, so the resets on lines 526-527 and 530-531 no longer affect anything and read as if they still guard billing.♻️ Proposed cleanup
if err != nil { logger.LogError(ctx, fmt.Sprintf("UpdateWithStatus failed for task %s: %s", task.TaskID, err.Error())) - shouldRefund = false - shouldSettle = false } else if !won { logger.LogWarn(ctx, fmt.Sprintf("Task %s already transitioned by another process, skip billing", task.TaskID)) - shouldRefund = false - shouldSettle = false } else { transitionWon = 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 `@service/task_polling.go` around lines 509 - 546, Remove the assignments that reset shouldRefund and shouldSettle inside the UpdateWithStatus error and !won branches of the settlement transition block. Billing is already gated by transitionWon and settlement in applyTaskBillingSettlement, so retain the existing logging and transition handling without these dead resets.relay/channel/xunfei/relay-xunfei.go (1)
182-204: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winNo-event completion silently returns an empty 200.
If the event channel closes without delivering any event (client/context cancellation, or a producer exit path that emits nothing),
xunfeiResponsestays zero-valued and the handler returns an empty successful completion with zero usage, which then gets billed as a success. Track whether any response was received and return an error (orc.Request.Context().Err()) when none was.🤖 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 `@relay/channel/xunfei/relay-xunfei.go` around lines 182 - 204, Track whether the event loop in the Xunfei response handler received any response event, and after the loop return an appropriate error, preferably c.Request.Context().Err() when available, if none was received. Prevent the existing zero-value xunfeiResponse finalization and successful empty completion path from running when the channel closes without a response; preserve normal aggregation for received responses.
🤖 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 `@controller/relay.go`:
- Around line 611-621: Define a named SettlementPreparer interface alongside the
billing session type with the PrepareSettlement signature, then update the relay
settlement logic to assert relayInfo.Billing against SettlementPreparer instead
of an inline anonymous interface. Ensure the billing session implementation is
checked against this named interface at compile time so signature changes fail
the build.
In `@main.go`:
- Around line 74-80: Add a shared shutdown signal and completion wait mechanism
for the goroutines started by StartBillingSettlementTaskRunner and
StartCacheInvalidationTaskRunner; have both runners stop accepting work, finish
in-flight database operations, and signal completion when shutdown is requested.
Update main’s graceful shutdown sequence to signal the runners and wait for both
to exit before calling CloseDB, while preserving the existing RedisEnabled
conditional.
- Around line 192-197: Update the SESSION_COOKIE_SECURE handling around
common.InitEnv and the os.LookupEnv check so blank values are treated as unset,
invalid non-empty boolean values are rejected, and only valid explicit
true/false values bypass server-address auto-detection. Preserve explicit
overrides while retaining the existing HTTPS warning for unset or blank values.
In `@model/billing_settlement_regression_test.go`:
- Around line 178-187: Update failCacheInvalidationOutboxInserts to explicitly
restrict the SQLite-specific trigger setup to SQLite, using common.UsingSQLite
or t.Skip when another database is configured; preserve the existing Redis state
setup and trigger cleanup for SQLite runs.
In `@model/billing_settlement.go`:
- Around line 552-567: The BillingSettlement retry path in
ApplyBillingSettlementOnce reconstructs input without the pre-consume selection
fields, so successful retries may not insert BillingPreConsumeSelection. Persist
PreConsumeRequestID, PreConsumeModelName, PreConsumeRequestedQuota, and
PreConsumeEffectiveQuota on the settlement record and restore them when building
BillingSettlementInput, or reliably re-derive the same values from
record.OperationKey before applying the settlement.
- Around line 501-510: Update invalidateBillingSettlementCaches to gate token
cache invalidation on the applied settlement’s record.AppliedTokenDelta rather
than input.TokenDelta, and pass that applied record/value from the replay
settlement path. Preserve the existing token identity and non-empty tokenKey
checks so only an actually mutated token cache is invalidated.
In `@model/cache_invalidation_task_test.go`:
- Around line 66-99: Refactor
TestApplyBillingSettlementOnceResolvesTokenKeyForCacheInvalidation to use
useRecoverableCacheMutationRedis with a no-op hook instead of duplicating the
common.RDB and common.RedisEnabled swap/restore setup. Preserve the test’s Redis
client, cleanup behavior, and cache invalidation assertions while centralizing
global Redis state management in the helper.
In `@model/cache_invalidation_task.go`:
- Around line 72-82: Preserve durable retry backoff during cache invalidation
re-staging: in model/cache_invalidation_task.go lines 72-82, update the upsert
conflict assignments to refresh only version_key, updated_at, and revision,
leaving attempts and next_attempt unchanged (or advancing next_attempt with the
existing backoff). In model/user_cache.go line 138, retain persistence for the
initial enqueue but skip re-persisting during in-memory retry attempts so the
pending task schedule is not reset.
- Around line 132-149: Update the cache invalidation retry flow around
executeCacheInvalidationTask to distinguish permanent failures, including
unknown kinds and invalid persisted user EntityKey values, using the
errCacheInvalidationPermanent classification. Delete permanently failing rows
(or move them to a dead state) instead of rescheduling them, while preserving
the existing retry and backoff behavior for transient errors.
In `@model/log.go`:
- Around line 697-735: The transaction in the operationKey path currently leaves
BillingLogReceipt rows indefinitely; add a timed retention cleanup for
billing_log_receipts using the existing createdAt/CreatedAt fields and database
access patterns, while preserving claim-token duplicate protection. Place the
cleanup in the surrounding billing-log persistence flow and ensure it runs
periodically rather than on every insert.
In `@model/subscription.go`:
- Around line 1374-1378: The alreadyConsumed replay branch must invalidate using
the persisted token identifier rather than an empty or stale caller-supplied
key. Refresh resolvedTokenKey from the persisted record’s TokenId before calling
invalidateTokenQuotaCache, while preserving the existing
dispatchStagedCacheInvalidation path for non-replays.
- Around line 1614-1618: The no-op CleanupSubscriptionPreConsumeRecords leaves
subscription_pre_consume_records unbounded. Implement retention using
olderThanSeconds to delete only sufficiently old records with terminal
BillingSettlement values applied or manual, preserve pending and other
nonterminal rows, and return the deleted count and any error. Document that
olderThanSeconds remains part of the API for compatibility, and add monitoring
for table size if existing monitoring hooks are available.
- Line 1503: Update the insufficient-quota return in the subscription
consumption flow to wrap the ErrSubscriptionQuotaInsufficient sentinel while
preserving the existing required amount in the error message. Ensure this path
matches the sentinel-wrapping behavior already used by
postConsumeUserSubscriptionDeltaTx so errors.Is classification is consistent.
- Around line 1295-1314: In the transaction callback, update the
preConsumeUserSubscriptionTx multi-return assignment to discard alreadyConsumed
directly with _, and remove the separate alreadyConsumed declaration and _ =
alreadyConsumed statement.
In `@model/task_cas_test.go`:
- Around line 349-354: The poll-fairness test setup around insertTask must not
provide timestamps that insertTask overwrites. Remove the misleading CreatedAt
and UpdatedAt assignments from these seeded Task values, unless the test
explicitly updates those fields after insertion to enforce timestamp ordering;
preserve the intended ordering assertion.
In `@model/task.go`:
- Line 501: Update the task status update paths around the
`Select("*").Updates(t)` calls, including the corresponding logic near lines
531-533, so they modify only the status-owned columns rather than writing every
field from a potentially partial `Task`; preserve unrelated fields such as
timestamps and properties.
- Around line 344-368: Update the candidate task query inside the DB.Transaction
callback to apply withRowLock(tx) before Find(&tasks), ensuring rows are locked
during selection and claim while preserving the existing SQLite no-op behavior
and subsequent updated_at update flow.
In `@model/token_cache.go`:
- Around line 105-108: Update the token query in the surrounding token-cache
method to select only the Key column before loading results, while preserving
the existing user_id filter, Unscoped behavior, transaction, and error handling.
In `@model/user_update_test.go`:
- Around line 439-440: Extract the repeated cache-outbox trigger setup and
cleanup into a failCacheOutboxInserts helper in model/user_update_test.go. Move
the CREATE TRIGGER statement and t.Cleanup DROP TRIGGER logic into that helper,
call t.Helper(), and replace each duplicated pair at the referenced test sites
with calls to failCacheOutboxInserts(t).
In `@model/user.go`:
- Around line 1250-1280: Update deleteWithCacheInvalidation so hardDelete also
removes the user’s token rows within the same transaction. Use the existing
token-related deletion or staging path, passing the hard-delete behavior through
as needed, while preserving cache invalidation for soft deletes and ensuring
token deletion occurs before the transaction commits.
In `@relay/channel/xunfei/relay-xunfei.go`:
- Around line 257-303: Add a write deadline on conn before WriteJSON, and set or
refresh a read deadline on conn at the start of each ReadMessage loop iteration.
Use the existing request timeout/deadline configuration to bound both operations
while preserving context cancellation and normal websocket completion behavior.
- Around line 138-170: Update the streaming flow around SetEventStreamHeaders
and streamErr so mid-stream errors after any data chunk has been rendered
terminate the SSE stream with a [DONE] event and return accumulated usage
without propagating a request failure. Track whether output has been written,
while preserving hard-error behavior when the stream fails before the first
rendered chunk.
In `@relay/relay_task.go`:
- Around line 342-352: Move the X-Max-Api-Other-Ratios and
X-New-Api-Other-Ratios header assignments out of the buffered response closure
and place them after adaptor.AdjustBillingOnSubmit finalizes finalQuota and
info.PriceData.OtherRatios, before responseSnapshot and TaskSubmitResult are
constructed. Preserve the existing empty-map fallback and JSON serialization so
both headers reflect the final adjusted ratios.
- Around line 69-102: Update ensureTaskPlaceholder so errors from
model.DB.First, task.Update, and task.Insert are wrapped with sanitized
client-facing messages rather than passing raw database errors to
TaskErrorWrapperLocal. Preserve the existing error codes and status while
ensuring each failure path logs the underlying error but returns only a safe
message through the response path.
In `@relay/task_response_buffer.go`:
- Around line 23-25: Update the header restoration loop in the response buffer
snapshot logic to clear the destination writer’s existing headers before copying
values from s.header, so headers removed from the buffer are also removed from
the original writer. Preserve the existing defensive slice-copy behavior when
restoring each header.
In `@router/api-router.go`:
- Line 88: Update the DELETE /self route’s secure verification configuration
around SecureVerificationRequired so every verification method requires the
account_delete scope and a fresh verification; do not allow 2FA or passkey
sessions created for another scope, such as access_token, to satisfy
controller.DeleteSelf.
In `@service/billing_session_test.go`:
- Around line 234-243: Update
TestBillingSessionTrustedWalletReplayFailsClosedInsteadOfSwitchingFunding to
explicitly configure a positive trust-quota value for the test, or temporarily
set the corresponding option and restore it with t.Cleanup. Use that pinned
value consistently when seeding quota and configuring the request context so
first.trusted remains true independently of ambient defaults.
- Around line 576-603: Update TestBillingSessionRefundUsesDurableOperation to
replace require.Eventually with immediate assertions after session.Refund(ctx),
since Refund now completes synchronously. Assert the expected user and token
quotas directly while preserving the existing settlement lookup and status
assertion.
In `@service/billing_session.go`:
- Around line 494-511: Replace the duplicated subscription error-mapping block
in the legacy funding path with a call to mapAtomicPreConsumeError, preserving
the existing rollback and return behavior. Apply the same routing to the
additional funding-error handling around the nearby lines so strings.Contains
checks and the sentinel-error TODO exist only in mapAtomicPreConsumeError.
- Around line 251-322: Refactor BillingSession.Refund to avoid holding s.mu
during ApplyBillingSettlementOnce and centralize unlock handling. Snapshot the
required session state under the lock in a small helper with deferred unlock,
then release the lock before the durable settlement call; preserve all existing
early-return checks, manual-review behavior, and successful state updates.
In `@service/task_billing_test.go`:
- Around line 429-467: The test currently starts the unbounded background runner
via StartBillingSettlementTaskRunner, allowing it to affect later tests. Replace
that call in TestBackgroundTaskSettlementRecoveryRestoresLogAndUsageExactlyOnce
with a direct single-pass settlement processor, such as
processPendingBillingSettlements or processPendingBillingSettlementEffects, or
use a test-only controlled runner that terminates after processing this pending
settlement.
In `@service/task_billing.go`:
- Around line 173-177: Update the refund BillingSettlementEffect construction to
assign the failure reason to Content instead of hardcoding an empty string.
Preserve the existing other["reason"] data if needed, and ensure
buildTaskFinalSettlementInput and ProcessBillingSettlementEffect propagate the
same reason into the user-facing refund log.
In `@web/default/src/features/channels/lib/channel-form.test.ts`:
- Around line 83-92: Update the test “rejects aliased numeric status-code source
keys” to use two distinct source keys that both pass validation but normalize to
the same numeric code, such as “429” and “ 429”. Keep the assertion that
channelFormSchema.safeParse returns success false so the test exercises the
seenSourceCodes duplicate-detection branch in isOptionalStatusCodeMapping.
In
`@web/default/src/features/profile/components/dialogs/delete-account-dialog.tsx`:
- Around line 107-118: Update the catch block surrounding withVerification and
performDelete to pass the caught error through the existing handleServerError
path, then display its handled, i18n-safe result via the standard toast.error
flow; remove the direct error.message fallback while preserving the generic
translated failure message for unhandled cases.
---
Outside diff comments:
In `@relay/channel/xunfei/relay-xunfei.go`:
- Around line 182-204: Track whether the event loop in the Xunfei response
handler received any response event, and after the loop return an appropriate
error, preferably c.Request.Context().Err() when available, if none was
received. Prevent the existing zero-value xunfeiResponse finalization and
successful empty completion path from running when the channel closes without a
response; preserve normal aggregation for received responses.
In `@relay/mjproxy_handler.go`:
- Line 314: When RelayMidjourneyTaskImageSeed and RelayMidjourneySubmit switch
context to originTask.ChannelId and its base URL, also set status_code_mapping
from the origin channel before any writeMidjourneyStatusCode call. Update each
referenced channel-switching block, including the assignments near lines 314,
324, 488-489, and 659, so the mapping always matches the active origin channel.
In `@service/task_polling.go`:
- Around line 509-546: Remove the assignments that reset shouldRefund and
shouldSettle inside the UpdateWithStatus error and !won branches of the
settlement transition block. Billing is already gated by transitionWon and
settlement in applyTaskBillingSettlement, so retain the existing logging and
transition handling without these dead resets.
In `@web/default/src/features/models/components/drawers/model-mutate-drawer.tsx`:
- Around line 99-117: Add clear, translatable validation messages to each
pricing field’s isOptionalPricingNumber refine call in extendedModelFormSchema:
price, ratio, cacheRatio, completionRatio, imageRatio, audioRatio, and
audioCompletionRatio. Use the existing message conventions from nearby schema
refinements so FormMessage displays a descriptive error instead of Zod’s generic
text.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 4f751ebe-ad64-4a67-b2c7-cee9b641de7e
📒 Files selected for processing (101)
common/init.gocommon/session_cookie_test.gocontroller/channel.gocontroller/channel_add_test.gocontroller/midjourney.gocontroller/oauth.gocontroller/oauth_test.gocontroller/relay.gocontroller/relay_retry_test.gocontroller/secure_verification.gocontroller/secure_verification_test.gocontroller/user.godocs/openapi/api.jsondto/task.gomain.gomiddleware/secure_verification_test.gomodel/billing_settlement.gomodel/billing_settlement_regression_test.gomodel/cache_invalidation_task.gomodel/cache_invalidation_task_test.gomodel/log.gomodel/log_test.gomodel/main.gomodel/option.gomodel/option_test.gomodel/subscription.gomodel/task.gomodel/task_cas_test.gomodel/token.gomodel/token_cache.gomodel/user.gomodel/user_cache.gomodel/user_update_test.gorelay/channel/api_request.gorelay/channel/api_request_test.gorelay/channel/aws/relay-aws.gorelay/channel/aws/relay_aws_test.gorelay/channel/baidu/relay-baidu.gorelay/channel/cohere/relay-cohere.gorelay/channel/gemini/relay-gemini.gorelay/channel/openai/adaptor.gorelay/channel/openai/adaptor_realtime_test.gorelay/channel/openai/relay-openai.gorelay/channel/vertex/relay-vertex.gorelay/channel/vertex/relay_vertex_test.gorelay/channel/xunfei/relay-xunfei.gorelay/channel/xunfei/relay_xunfei_test.gorelay/channel/zhipu/relay-zhipu.gorelay/common/relay_info.gorelay/mjproxy_handler.gorelay/mjproxy_handler_test.gorelay/relay_task.gorelay/relay_task_test.gorelay/task_response_buffer.gorouter/api-router.gorouter/api_router_test.goservice/billing.goservice/billing_session.goservice/billing_session_test.goservice/error.goservice/error_test.goservice/funding_source.goservice/midjourney_image_url.goservice/midjourney_image_url_test.goservice/pre_consume_quota.goservice/pre_consume_validation_test.goservice/quota.goservice/task_billing.goservice/task_billing_test.goservice/task_polling.goservice/violation_fee.gosetting/ratio_setting/cache_ratio.gosetting/ratio_setting/model_ratio.gosetting/ratio_setting/pricing_validation.gotools/jsonwrapcheck/allowlist.txtweb/default/src/components/json-editor.tsxweb/default/src/features/auth/lib/app-session-verifier.tsweb/default/src/features/auth/lib/session-verifier.tsweb/default/src/features/auth/secure-verification/types.tsweb/default/src/features/channels/components/drawers/channel-editor-state.tsweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/channels/components/drawers/sections/channel-models-section.test.tsxweb/default/src/features/channels/components/drawers/sections/channel-models-section.tsxweb/default/src/features/channels/lib/channel-form.test.tsweb/default/src/features/channels/lib/channel-form.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.test.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsxweb/default/src/features/models/components/drawers/model-pricing-config.tsweb/default/src/features/profile/api.tsweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsxweb/default/src/i18n/locales/en.jsonweb/default/src/i18n/locales/fr.jsonweb/default/src/i18n/locales/ja.jsonweb/default/src/i18n/locales/ru.jsonweb/default/src/i18n/locales/vi.jsonweb/default/src/i18n/locales/zh.jsonweb/default/src/routes/__root.tsxweb/default/src/routes/_authenticated/route.tsxweb/default/src/stores/auth-store.tsweb/default/src/stores/notification-store.test.tsweb/default/tests/channel-form.test.ts
💤 Files with no reviewable changes (5)
- web/default/src/stores/notification-store.test.ts
- service/pre_consume_quota.go
- web/default/src/features/channels/components/drawers/sections/channel-models-section.tsx
- tools/jsonwrapcheck/allowlist.txt
- web/default/tests/channel-form.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
router/api-router.gocontroller/channel_add_test.gosetting/ratio_setting/pricing_validation.gomodel/cache_invalidation_task.gorelay/channel/zhipu/relay-zhipu.gocontroller/channel.gorelay/channel/openai/relay-openai.goservice/midjourney_image_url.gocommon/session_cookie_test.gocontroller/midjourney.gorelay/channel/openai/adaptor_realtime_test.gorelay/channel/openai/adaptor.gomiddleware/secure_verification_test.gosetting/ratio_setting/cache_ratio.gorouter/api_router_test.gorelay/channel/baidu/relay-baidu.gocontroller/secure_verification_test.goservice/pre_consume_validation_test.gorelay/channel/gemini/relay-gemini.gorelay/channel/vertex/relay_vertex_test.gorelay/channel/api_request.gorelay/channel/vertex/relay-vertex.goservice/billing.gorelay/mjproxy_handler_test.goservice/midjourney_image_url_test.gorelay/channel/aws/relay_aws_test.gorelay/common/relay_info.gorelay/channel/xunfei/relay_xunfei_test.gomodel/option.gorelay/channel/cohere/relay-cohere.gorelay/task_response_buffer.gomodel/option_test.gomain.gocontroller/oauth.godto/task.goservice/error_test.gocontroller/relay_retry_test.gocontroller/oauth_test.gocontroller/secure_verification.gomodel/main.goservice/error.goservice/funding_source.gosetting/ratio_setting/model_ratio.gomodel/cache_invalidation_task_test.goservice/violation_fee.goservice/quota.gocommon/init.gorelay/channel/xunfei/relay-xunfei.gomodel/user_cache.gomodel/token.gorelay/channel/api_request_test.gomodel/token_cache.gorelay/relay_task_test.gocontroller/relay.gomodel/log_test.gorelay/channel/aws/relay-aws.gocontroller/user.gorelay/mjproxy_handler.goservice/task_billing.goservice/task_polling.gomodel/billing_settlement.gomodel/log.gorelay/relay_task.goservice/billing_session.gomodel/subscription.gomodel/task_cas_test.gomodel/task.gomodel/billing_settlement_regression_test.gomodel/user_update_test.gomodel/user.goservice/task_billing_test.goservice/billing_session_test.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/features/auth/secure-verification/types.tsweb/default/src/routes/_authenticated/route.tsxweb/default/src/features/auth/lib/app-session-verifier.tsweb/default/src/features/profile/api.tsweb/default/src/stores/auth-store.tsweb/default/src/features/auth/lib/session-verifier.tsweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/routes/__root.tsxweb/default/src/features/models/components/drawers/model-pricing-config.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.test.tsweb/default/src/features/channels/components/drawers/sections/channel-models-section.test.tsxweb/default/src/features/channels/lib/channel-form.test.tsweb/default/src/features/channels/components/drawers/channel-editor-state.tsweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsxweb/default/src/components/json-editor.tsxweb/default/src/features/channels/lib/channel-form.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsx
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/auth/secure-verification/types.tsweb/default/src/features/auth/lib/app-session-verifier.tsweb/default/src/features/profile/api.tsweb/default/src/features/auth/lib/session-verifier.tsweb/default/src/features/channels/components/drawers/channel-mutate-drawer.tsxweb/default/src/features/models/components/drawers/model-pricing-config.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.test.tsweb/default/src/features/channels/components/drawers/sections/channel-models-section.test.tsxweb/default/src/features/channels/lib/channel-form.test.tsweb/default/src/features/channels/components/drawers/channel-editor-state.tsweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsxweb/default/src/features/channels/lib/channel-form.tsweb/default/src/features/models/components/drawers/model-mutate-drawer.tsx
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/zhipu/relay-zhipu.gorelay/channel/openai/relay-openai.gorelay/channel/openai/adaptor_realtime_test.gorelay/channel/openai/adaptor.gorelay/channel/baidu/relay-baidu.gorelay/channel/gemini/relay-gemini.gorelay/channel/vertex/relay_vertex_test.gorelay/channel/api_request.gorelay/channel/vertex/relay-vertex.gorelay/channel/aws/relay_aws_test.gorelay/channel/xunfei/relay_xunfei_test.gorelay/channel/cohere/relay-cohere.gorelay/channel/xunfei/relay-xunfei.gorelay/channel/api_request_test.gorelay/channel/aws/relay-aws.go
web/default/src/routes/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
路由应使用 TanStack Router，并通过
createFileRoute定义；搜索参数应使用 Zod schema +validateSearch校验；认证与重定向应放在beforeLoad中；导航应优先使用useNavigate或Link，避免直接操作window.location。
Files:
web/default/src/routes/_authenticated/route.tsxweb/default/src/routes/__root.tsx
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/auth/lib/app-session-verifier.tsweb/default/src/features/auth/lib/session-verifier.tsweb/default/src/features/channels/lib/channel-form.test.tsweb/default/src/features/channels/lib/channel-form.ts
web/default/src/stores/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
使用 Zustand 时应通过
create定义 store，并为 state 与 actions 定义清晰类型；优先使用选择器订阅，避免整 store 订阅导致多余渲染；需持久化状态应在 store 内处理 localStorage 恢复；store 应按功能放在src/stores/。
Files:
web/default/src/stores/auth-store.ts
dto/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
For request structs parsed from client JSON and then re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types with
omitemptyso that absent fields stay omitted while explicit zero/false values are preserved.
Files:
dto/task.go
web/default/src/**/*.test.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
工具函数与纯逻辑应优先编写单元测试；测试文件应命名为
*.test.ts。
Files:
web/default/src/features/models/components/drawers/model-mutate-drawer.test.tsweb/default/src/features/channels/lib/channel-form.test.ts
🪛 ast-grep (0.45.0)
controller/secure_verification.go
[warning] 22-22: A credential is hard-coded as a string literal. Secrets stored in source code, such as passwords, API keys, and tokens, can be leaked through version control or binaries and used by internal or external malicious actors. Rotate the exposed secret and load it at runtime from a secure secret vault, a Hardware Security Module (HSM), or an environment variable if permitted by your company policy (e.g. password := os.Getenv("APP_PASSWORD")).
Context: secureVerificationMethodPassword = "password"
Note: [CWE-798] Use of Hard-coded Credentials.
(hardcoded-credentials-string-literal-go)
| // Recover durable balance settlements after process restarts or transient DB failures. | ||
| model.StartBillingSettlementTaskRunner() | ||
|
|
||
| if common.RedisEnabled { | ||
| // for compatibility with old versions | ||
| common.MemoryCacheEnabled = true | ||
| model.StartCacheInvalidationTaskRunner() |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Stop the background runners before closing the database.
Both runners launch unbounded goroutines with no cancellation or wait path. During graceful shutdown, main closes the database at Line [67-72] while these goroutines can still execute database operations. Add a shared shutdown signal and wait for both runners before CloseDB.
🤖 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 `@main.go` around lines 74 - 80, Add a shared shutdown signal and completion
wait mechanism for the goroutines started by StartBillingSettlementTaskRunner
and StartCacheInvalidationTaskRunner; have both runners stop accepting work,
finish in-flight database operations, and signal completion when shutdown is
requested. Update main’s graceful shutdown sequence to signal the runners and
wait for both to exit before calling CloseDB, while preserving the existing
RedisEnabled conditional.
| func failCacheInvalidationOutboxInserts(t *testing.T) { | ||
| t.Helper() | ||
| oldRedisEnabled := common.RedisEnabled | ||
| common.RedisEnabled = true | ||
| require.NoError(t, DB.Exec("CREATE TRIGGER cache_outbox_insert_failure BEFORE INSERT ON cache_invalidation_tasks BEGIN SELECT RAISE(FAIL, 'cache outbox unavailable'); END").Error) | ||
| t.Cleanup(func() { | ||
| _ = DB.Exec("DROP TRIGGER IF EXISTS cache_outbox_insert_failure") | ||
| common.RedisEnabled = oldRedisEnabled | ||
| }) | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
SQLite-only failure injection.
CREATE TRIGGER ... RAISE(FAIL, ...) is SQLite syntax, so these outbox-rollback regressions silently become non-runnable if the model test suite is ever pointed at MySQL/PostgreSQL. A guard (common.UsingSQLite) or t.Skip would make the constraint explicit.
🤖 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/billing_settlement_regression_test.go` around lines 178 - 187, Update
failCacheInvalidationOutboxInserts to explicitly restrict the SQLite-specific
trigger setup to SQLite, using common.UsingSQLite or t.Skip when another
database is configured; preserve the existing Redis state setup and trigger
cleanup for SQLite runs.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
web/default/src/features/profile/components/dialogs/delete-account-dialog.tsx (1)
199-201: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winConsume verification failures from the async callback.
executeVerificationrethrows after handling its error; this callback returns that rejected promise to React unhandled. Catch it here (without another toast, since the hook already reports it) to prevent an unhandled rejection.Proposed fix
onVerify={async (method, code) => { - await executeVerification(method, code) + try { + await executeVerification(method, code) + } catch { + // executeVerification already presents the failure. + } }}🤖 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 `@web/default/src/features/profile/components/dialogs/delete-account-dialog.tsx` around lines 199 - 201, Update the onVerify async callback to catch and consume rejections from executeVerification without displaying another toast, since the hook already handles error reporting. Keep successful verification behavior unchanged.service/billing_session.go (2)
80-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
settleLocked's durable branch still performs the DB round-trip unders.mu(same pattern already fixed forRefund).
Refundwas refactored intoprepareRefundIntent/finishRefundIntentspecifically to avoid holdings.muacrossmodel.ApplyBillingSettlementOnce.settleLocked's new durable branch calls the same DB operation (plusmodel.ProcessBillingSettlementEffect) but appears to run entirely under the caller's lock (no equivalent snapshot/release pattern here). Consider applying the same prepare-under-lock/apply-unlocked/finalize-under-lock structure used forRefund.🤖 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/billing_session.go` around lines 80 - 112, Refactor the durable path in settleLocked to avoid holding s.mu during ApplyBillingSettlementOnce and ProcessBillingSettlementEffect. Follow the existing prepareRefundIntent/finishRefundIntent pattern: capture the settlement intent under the lock, perform database operations unlocked, then reacquire the lock to apply results and mark settled state, preserving existing error handling and funding updates.
92-109: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHandle effect-status conflicts as non-retryable or preserve the applied funding delta.
When durable settle succeeds,
ProcessBillingSettlementEffectcan still returnbilling settlement effect is not readyif the transaction committed and a concurrent replay markseffect_status = applyingbefore the current call updates it. That current path returns an error without settings.fundingSettled/s.appliedFundingDelta, soNeedsRefund()reports refund-needed after funding has already been applied. Make effect-idempotency failures leavefundingSettled=true/appliedFundingDelta=appliedconsistent with the committed funding update, or gate refund eligibility against the durable settlementstatus=appliedrather than only local flags.🤖 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/billing_session.go` around lines 92 - 109, Update the durable settlement path around ApplyBillingSettlementOnce and ProcessBillingSettlementEffect so an effect-status conflict after a committed funding settlement does not leave the session eligible for refund. Preserve applied and mark fundingSettled before returning the conflict, or make NeedsRefund use the durable settlement status=applied as an equivalent guard; keep genuinely retryable settlement errors unchanged.relay/relay_task.go (1)
326-330: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUnclosed
resp.Bodyon non-200 upstream response.
io.ReadAll(resp.Body)drains the error body but doesn't close it, preventing the HTTP transport from reusing the underlying connection. Under sustained upstream error rates this leaks connections/file descriptors.🔧 Proposed fix
if resp != nil && resp.StatusCode != http.StatusOK { + defer resp.Body.Close() responseBody, _ := io.ReadAll(resp.Body) taskErr := service.TaskErrorWrapper(fmt.Errorf("%s", string(responseBody)), "fail_to_fetch_task", resp.StatusCode) return nil, mapUpstreamTaskError(c, taskErr) }🤖 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 `@relay/relay_task.go` around lines 326 - 330, Close resp.Body after reading it in the non-OK response branch of the task-fetch flow, ensuring cleanup occurs before returning through mapUpstreamTaskError while preserving the existing error mapping behavior.model/task.go (1)
556-582: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winError message conflates "not found" with "already terminal".
UpdateWithSettlementIntent'sWHEREclause excludesTaskStatusFailure/TaskStatusSuccess, soRowsAffected != 1can also happen when the task legitimately exists but already reached a terminal state (e.g., raced with a timeout sweep) — not only when the row is missing. Returning"persisted task not found: id=%d"in that case will mislead debugging of billing-settlement failures.🔧 Suggested fix: distinguish the two cases
- if result.RowsAffected != 1 { - return fmt.Errorf("persisted task not found: id=%d", t.ID) - } + if result.RowsAffected != 1 { + var exists Task + if err := tx.Select("id", "status").First(&exists, t.ID).Error; err == nil { + return fmt.Errorf("task already terminal (status=%s): id=%d", exists.Status, t.ID) + } + return fmt.Errorf("persisted task not found: id=%d", t.ID) + }🤖 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/task.go` around lines 556 - 582, Update UpdateWithSettlementIntent so a zero RowsAffected result is distinguished between a missing task and an existing task already in TaskStatusFailure or TaskStatusSuccess; query or otherwise verify the task’s existence/status before returning an error, preserving success for the intended update path and reporting a terminal-state-specific error instead of “persisted task not found” when applicable.
🤖 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/task.go`:
- Around line 456-494: Update statusUpdateValues and submitResultUpdateValues to
support explicitly including data and private_data when callers intend to clear
them, rather than inferring field presence from nil or zero-value checks. Add an
explicit field-presence mechanism through the relevant
UpdateWithStatus/UpdateWithSettlementIntent call paths, and ensure requested
zero values are retained in the update map while omitted fields remain
unchanged.
In `@relay/relay_task.go`:
- Around line 104-108: Export the canonical taskPersistenceError implementation
in relay/relay_task.go for shared use. In controller/relay.go, remove the
duplicate local helper and update its callers to use the exported relay
implementation, preserving the existing raw-error logging and safe
TaskErrorWrapperLocal behavior at both sites.
In `@service/system_task.go`:
- Around line 269-272: Replace the single DeleteOldBillingLogReceipts call in
the system-task handler with an explicit batched cleanup loop, renewing the task
lock via model.UpdateSystemTaskState(..., systemTaskLockUntil()) after each
successful batch as the sibling log-deletion loop does. Preserve
payload.TargetTimestamp, payload.BatchSize, cancellation/error handling, and
stop when fewer than the batch limit is deleted; call failSystemTask and return
on any deletion or lock-update error.
In `@web/default/src/lib/handle-server-error.ts`:
- Around line 49-58: Update the exported handleServerError function signature to
explicitly declare a void return type, preserving its existing parameters and
implementation.
---
Outside diff comments:
In `@model/task.go`:
- Around line 556-582: Update UpdateWithSettlementIntent so a zero RowsAffected
result is distinguished between a missing task and an existing task already in
TaskStatusFailure or TaskStatusSuccess; query or otherwise verify the task’s
existence/status before returning an error, preserving success for the intended
update path and reporting a terminal-state-specific error instead of “persisted
task not found” when applicable.
In `@relay/relay_task.go`:
- Around line 326-330: Close resp.Body after reading it in the non-OK response
branch of the task-fetch flow, ensuring cleanup occurs before returning through
mapUpstreamTaskError while preserving the existing error mapping behavior.
In `@service/billing_session.go`:
- Around line 80-112: Refactor the durable path in settleLocked to avoid holding
s.mu during ApplyBillingSettlementOnce and ProcessBillingSettlementEffect.
Follow the existing prepareRefundIntent/finishRefundIntent pattern: capture the
settlement intent under the lock, perform database operations unlocked, then
reacquire the lock to apply results and mark settled state, preserving existing
error handling and funding updates.
- Around line 92-109: Update the durable settlement path around
ApplyBillingSettlementOnce and ProcessBillingSettlementEffect so an
effect-status conflict after a committed funding settlement does not leave the
session eligible for refund. Preserve applied and mark fundingSettled before
returning the conflict, or make NeedsRefund use the durable settlement
status=applied as an equivalent guard; keep genuinely retryable settlement
errors unchanged.
In
`@web/default/src/features/profile/components/dialogs/delete-account-dialog.tsx`:
- Around line 199-201: Update the onVerify async callback to catch and consume
rejections from executeVerification without displaying another toast, since the
hook already handles error reporting. Keep successful verification 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: ASSERTIVE
Plan: Pro Plus
Run ID: 13feb385-c629-41b3-93da-65827e8708b7
📒 Files selected for processing (30)
controller/relay.gomain.gomain_test.gomiddleware/secure_verification.gomiddleware/secure_verification_test.gomodel/billing_settlement.gomodel/billing_settlement_regression_test.gomodel/cache_invalidation_task.gomodel/cache_invalidation_task_test.gomodel/log.gomodel/log_cleanup_test.gomodel/subscription.gomodel/task.gomodel/task_cas_test.gomodel/token_cache.gomodel/user.gomodel/user_cache.gomodel/user_update_test.gorelay/channel/xunfei/relay-xunfei.gorelay/relay_task.gorelay/relay_task_test.gorelay/task_response_buffer.goservice/billing_session.goservice/billing_session_test.goservice/system_task.goservice/task_billing.goservice/task_billing_test.goweb/default/src/features/channels/lib/channel-form.test.tsweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsxweb/default/src/lib/handle-server-error.ts
💤 Files with no reviewable changes (1)
- model/task_cas_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (6)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
relay/task_response_buffer.goservice/system_task.gomain_test.gomiddleware/secure_verification.gomodel/log_cleanup_test.gomodel/token_cache.gocontroller/relay.gomodel/cache_invalidation_task.gorelay/relay_task.gomodel/user.gorelay/relay_task_test.gomodel/log.gomiddleware/secure_verification_test.gomodel/user_cache.gomodel/cache_invalidation_task_test.gorelay/channel/xunfei/relay-xunfei.goservice/task_billing.gomodel/task.gomodel/subscription.goservice/billing_session_test.gomodel/billing_settlement_regression_test.gomodel/billing_settlement.goservice/task_billing_test.gomain.goservice/billing_session.gomodel/user_update_test.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/lib/handle-server-error.tsweb/default/src/features/channels/lib/channel-form.test.tsweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsx
web/default/src/features/**/lib/**/*.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
表单应使用 React Hook Form + Zod：在功能模块的
lib/下定义 schema，并用z.infer导出表单类型；useForm应配合@hookform/resolvers/zod进行校验。
Files:
web/default/src/features/channels/lib/channel-form.test.ts
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/channels/lib/channel-form.test.tsweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsx
web/default/src/**/*.test.ts
📄 CodeRabbit inference engine (web/default/AGENTS.md)
工具函数与纯逻辑应优先编写单元测试；测试文件应命名为
*.test.ts。
Files:
web/default/src/features/channels/lib/channel-form.test.ts
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/xunfei/relay-xunfei.go
🔇 Additional comments (34)
web/default/src/lib/handle-server-error.ts (1)
30-32: LGTM!main.go (1)
33-33: LGTM!Also applies to: 74-80, 192-194, 245-254, 274-291
main_test.go (1)
11-11: LGTM!Also applies to: 105-128
middleware/secure_verification.go (1)
101-114: LGTM!middleware/secure_verification_test.go (1)
101-141: LGTM!web/default/src/features/channels/lib/channel-form.test.ts (1)
88-88: LGTM!web/default/src/features/profile/components/dialogs/delete-account-dialog.tsx (1)
26-26: LGTM!Also applies to: 107-117
relay/relay_task_test.go (1)
4-4: LGTM!Also applies to: 13-14, 254-318
service/task_billing.go (1)
148-179: LGTM! This resolves the previously flagged issue where the refund logContentwas hardcoded empty instead of carrying the failurereason.service/billing_session_test.go (2)
234-271: LGTM! Resolves the prior nit about the test's implicit dependency on the ambient trust-quota default.
577-604: LGTM! Resolves the prior nit about usingrequire.Eventuallyfor a now-synchronousRefund.service/billing_session.go (4)
26-49: LGTM!
259-345: LGTM! This resolves the prior nit aboutRefundholdings.muacross the durable settlement round-trip via the newprepareRefundIntent/finishRefundIntentsplit.
557-572: LGTM!mapAtomicPreConsumeErrorconsolidates the previously duplicated subscription error-mapping logic, andreserveFundingnow routes through it too, resolving the prior nit.Also applies to: 584-584
502-521: 🗄️ Data Integrity & IntegrationNo change needed.
PreConsumeTokenAndUserSubscriptionalready rejects a blankrequestIdwith"requestId is empty"before reserving quota, so the subscription path already has the empty-RequestIdguard.model/subscription.go (1)
1283-1312: LGTM!Also applies to: 1317-1384, 1386-1508, 1618-1645
model/billing_settlement.go (1)
4-4: LGTM!Also applies to: 148-205, 229-231, 482-486, 548-557, 613-617, 648-664
model/billing_settlement_regression_test.go (1)
120-156: LGTM!Also applies to: 216-222, 538-570
model/cache_invalidation_task.go (1)
22-23: LGTM!Also applies to: 53-91, 112-163, 164-197, 199-221
model/log.go (1)
1046-1069: LGTM!Also applies to: 1088-1152
model/log_cleanup_test.go (1)
108-169: LGTM!model/token_cache.go (1)
98-121: LGTM!Also applies to: 140-195
model/user.go (1)
1263-1274: LGTM!model/cache_invalidation_task_test.go (1)
33-66: LGTM!Also applies to: 161-213
model/task.go (3)
339-373: LGTM!
510-519: LGTM!
521-554: LGTM!service/task_billing_test.go (1)
429-463: LGTM!model/user_cache.go (2)
95-131: LGTM!Also applies to: 322-358
176-178: 🩺 Stability & AvailabilityOutbox persistence is already handled by the helper.
persistCacheInvalidationTaskwrapsupsertCacheInvalidationTaskand logs failures internally, so this call does not discard the durability error check at this call site.> Likely an incorrect or invalid review comment.model/user_update_test.go (1)
434-448: LGTM!Also applies to: 450-516, 518-532, 569-628
relay/channel/xunfei/relay-xunfei.go (1)
132-179: LGTM!Also applies to: 238-325
controller/relay.go (1)
505-537: LGTM!Also applies to: 584-586, 606-650
relay/task_response_buffer.go (1)
19-36: LGTM!Also applies to: 78-78
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
relay/relay_task.go (1)
326-333: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNil-body guard is incomplete —
io.ReadAll(resp.Body)still panics.Line 327 acknowledges
resp.Bodymay be nil, but line 330 passes it toio.ReadAllunconditionally; a nilio.ReadCloserinterface will panic onRead. Either drop the nil check (if bodies are guaranteed) or skip the read too.🛡️ Proposed fix
if resp != nil && resp.StatusCode != http.StatusOK { - if resp.Body != nil { - defer resp.Body.Close() - } - responseBody, _ := io.ReadAll(resp.Body) + var responseBody []byte + if resp.Body != nil { + defer resp.Body.Close() + responseBody, _ = io.ReadAll(resp.Body) + } taskErr := service.TaskErrorWrapper(fmt.Errorf("%s", string(responseBody)), "fail_to_fetch_task", resp.StatusCode) return nil, mapUpstreamTaskError(c, taskErr) }🤖 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 `@relay/relay_task.go` around lines 326 - 333, Update the non-OK response handling in the task-fetch flow so `io.ReadAll` is only called when `resp.Body` is non-nil; otherwise use an empty response body while still returning the wrapped upstream error. Keep the existing deferred close behavior for present bodies.
🤖 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 `@service/billing_session.go`:
- Around line 91-93: Rename prepareSettleAttemptLocked to beginSettleAttempt and
update all call sites to match the established convention that methods with the
Locked suffix require the caller to hold s.mu. Preserve the existing settlement
behavior while removing the internal s.mu.Lock and defer s.mu.Unlock from the
renamed method.
In `@service/system_task_test.go`:
- Around line 29-38: Make the setup around the system task audit trigger
backend-compatible: either replace the SQLite-specific trigger/table observation
and subsequent system_task_update_audit count assertions with an
application-level counter or updated_at/locked_until transition check, or guard
the existing DDL and assertions with common.UsingSQLite and provide an
equivalent verification path for MySQL/PostgreSQL. Preserve the test’s intended
validation of running-status updates.
---
Outside diff comments:
In `@relay/relay_task.go`:
- Around line 326-333: Update the non-OK response handling in the task-fetch
flow so `io.ReadAll` is only called when `resp.Body` is non-nil; otherwise use
an empty response body while still returning the wrapped upstream error. Keep
the existing deferred close behavior for present bodies.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 82f98895-2610-415f-a32c-c5cf08e40ee4
📒 Files selected for processing (11)
controller/relay.gomodel/task.gomodel/task_cas_test.gorelay/relay_task.gorelay/relay_task_test.goservice/billing_session.goservice/billing_session_test.goservice/system_task.goservice/system_task_test.goweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsxweb/default/src/lib/handle-server-error.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
service/system_task_test.gorelay/relay_task.gorelay/relay_task_test.gomodel/task_cas_test.goservice/system_task.gocontroller/relay.goservice/billing_session.gomodel/task.goservice/billing_session_test.go
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/lib/handle-server-error.tsweb/default/src/features/profile/components/dialogs/delete-account-dialog.tsx
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/profile/components/dialogs/delete-account-dialog.tsx
🔇 Additional comments (12)
web/default/src/features/profile/components/dialogs/delete-account-dialog.tsx (1)
200-204: LGTM!relay/relay_task_test.go (1)
4-4: LGTM!Also applies to: 13-14, 236-325
model/task_cas_test.go (1)
5-5: LGTM!Also applies to: 43-43, 53-55, 73-73, 82-84, 189-215, 216-241, 243-260, 262-293, 294-311, 313-339, 341-352, 354-374, 376-390
service/billing_session.go (2)
41-41: LGTM!Also applies to: 306-306, 409-409
51-53: LGTM!Also applies to: 71-71
relay/relay_task.go (1)
69-107: LGTM!Also applies to: 313-316, 372-388
model/task.go (1)
70-93: LGTM!Also applies to: 487-490, 505-508, 576-614
controller/relay.go (1)
626-626: LGTM!service/billing_session_test.go (1)
605-651: LGTM!service/system_task.go (1)
269-282: LGTM!service/system_task_test.go (1)
12-27: LGTM!Also applies to: 45-64
web/default/src/lib/handle-server-error.ts (1)
31-31: LGTM!Also applies to: 49-58
| func (s *BillingSession) prepareSettleAttemptLocked(actualQuota int, effect *model.BillingSettlementEffect) (*billingSettleIntent, bool, error) { | ||
| s.mu.Lock() | ||
| defer s.mu.Unlock() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
...Locked suffix now means the opposite of the rest of this file.
needsRefundLocked, settleNonDurableLocked, and prepareDurableSettlementLocked all require the caller to hold s.mu, but prepareSettleAttemptLocked acquires it itself. Since sync.Mutex isn't reentrant, a future caller following the existing convention will deadlock. Rename to something like beginSettleAttempt.
🤖 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/billing_session.go` around lines 91 - 93, Rename
prepareSettleAttemptLocked to beginSettleAttempt and update all call sites to
match the established convention that methods with the Locked suffix require the
caller to hold s.mu. Preserve the existing settlement behavior while removing
the internal s.mu.Lock and defer s.mu.Unlock from the renamed method.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
service/billing_session.go (2)
271-280: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftUse distinct idempotency keys for settlement and refund.
Both paths pass
request:<requestId>:finalizetoApplyBillingSettlementOnce, but they carry opposite funding/token deltas. If a settlement attempt leaves a durable pending/manual record, a later refund will reuse that record and can be rejected as a payload conflict instead of reversing the pre-consumption. Use separate keys such as:settleand:refund, with regression coverage for settlement failure followed by refund.Also applies to: 327-340
🤖 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/billing_session.go` around lines 271 - 280, Update the OperationKey construction used by ApplyBillingSettlementOnce in both settlement and refund paths to use distinct suffixes, such as :settle and :refund, rather than the shared :finalize key. Preserve the existing request ID prefix and add regression coverage for a failed settlement followed by a refund.
538-556: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRequire a stable request ID before atomic subscription pre-consumption.
Unlike the wallet branch, this branch calls
PreConsumeTokenAndUserSubscriptionwiths.relayInfo.RequestIdwithout checking that it is non-empty. That can bypass durable idempotency or produce a lower-layer failure after selecting the atomic path. Mirror the wallet guard and return the mapped error (or explicitly use the intended non-atomic fallback) when the request ID is absent.🤖 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/billing_session.go` around lines 538 - 556, Add the same non-empty request ID guard used by the wallet branch before calling PreConsumeTokenAndUserSubscription in the SubscriptionFunding case. When s.relayInfo.RequestId is absent, return the mapped validation error or follow the established non-atomic fallback; only perform atomic pre-consumption with a stable request ID.
🤖 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/cache_invalidation_task_test.go`:
- Around line 64-66: Update the t.Cleanup callback configuring
cache_outbox_insert_failure to check the DROP TRIGGER error and report it with
t.Errorf or require.NoError, rather than discarding it. Keep the existing
trigger setup and cleanup timing unchanged.
In `@service/system_task_test.go`:
- Line 66: Update the renewal assertion in the test around renewals.Load to
verify the exact expected count of five updates—one pre-billing update plus four
receipt-loop updates—instead of accepting any value greater than or equal to
four.
- Around line 31-50: Replace the callback registration on shared model.DB in the
renewal-count test with an isolated test *gorm.DB instance, and run the
lock/renewal operation through that instance so the callback assertion remains
valid. Keep callback setup and cleanup scoped to the isolated database, or
instrument the renewal path directly without mutating the global callback
registry.
---
Outside diff comments:
In `@service/billing_session.go`:
- Around line 271-280: Update the OperationKey construction used by
ApplyBillingSettlementOnce in both settlement and refund paths to use distinct
suffixes, such as :settle and :refund, rather than the shared :finalize key.
Preserve the existing request ID prefix and add regression coverage for a failed
settlement followed by a refund.
- Around line 538-556: Add the same non-empty request ID guard used by the
wallet branch before calling PreConsumeTokenAndUserSubscription in the
SubscriptionFunding case. When s.relayInfo.RequestId is absent, return the
mapped validation error or follow the established non-atomic fallback; only
perform atomic pre-consumption with a stable request ID.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 10e4c0db-903c-4403-96a2-6259bbc1727f
📒 Files selected for processing (3)
model/cache_invalidation_task_test.goservice/billing_session.goservice/system_task_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
service/system_task_test.gomodel/cache_invalidation_task_test.goservice/billing_session.go
🔇 Additional comments (3)
service/billing_session.go (1)
84-116: LGTM!Also applies to: 237-255, 257-270, 282-293, 303-326, 341-346, 348-380, 493-537, 560-590, 628-650
model/cache_invalidation_task_test.go (1)
61-65: LGTM!service/system_task_test.go (1)
5-11: LGTM!
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
service/system_task.go (1)
271-283: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winInclude billing-receipt deletions in
DeletedCount.The new loop deletes receipt rows but discards
rowsAffected;LogCleanupResultat Line 286 still reports onlystate.Processed, which tracks ordinary log rows. A receipt-only cleanup can therefore succeed while reportingDeletedCount == 0. Accumulate receipt deletions in the result and assert that contract in the test.🤖 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/system_task.go` around lines 271 - 283, The billing cleanup loop must include receipt deletions in the reported DeletedCount. Accumulate each rowsAffected value from DeleteOldBillingLogReceiptsBatch into the cleanup result used by LogCleanupResult, alongside state.Processed, and update the relevant test to assert receipt-only cleanup reports the deleted count.
🤖 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.
Outside diff comments:
In `@service/system_task.go`:
- Around line 271-283: The billing cleanup loop must include receipt deletions
in the reported DeletedCount. Accumulate each rowsAffected value from
DeleteOldBillingLogReceiptsBatch into the cleanup result used by
LogCleanupResult, alongside state.Processed, and update the relevant test to
assert receipt-only cleanup reports the deleted count.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d3eb6e8d-65ce-4854-abc9-044cbc4ee2ca
📒 Files selected for processing (3)
model/cache_invalidation_task_test.goservice/system_task.goservice/system_task_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
service/system_task_test.goservice/system_task.gomodel/cache_invalidation_task_test.go
🔇 Additional comments (3)
model/cache_invalidation_task_test.go (1)
66-68: LGTM!service/system_task_test.go (1)
31-40: LGTM!Also applies to: 56-56
service/system_task.go (1)
33-34: LGTM!Also applies to: 223-223, 254-254, 361-361
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/billing_settlement_regression_test.go`:
- Around line 126-140: Move the t.Cleanup(hook.unblock) registration immediately
after creating and adding the blocking hook, before starting the settlement
goroutine or calling waitForCacheHook. Keep the existing cleanup action and
synchronization flow otherwise unchanged.
In `@model/token_cache.go`:
- Around line 344-350: The normal Redis-hit paths in
model/token_cache.go:344-350 and model/user_cache.go:423-429 must stop calling
cacheInvalidationTaskPending, which performs a database lookup. Replace that
check with the established in-memory or Redis invalidation fence, while
retaining cache_invalidation_tasks database checks only for recovery or retry
flows.
In `@service/task_billing_test.go`:
- Line 56: Add cleanup for the cache_invalidation_tasks table in TestMain’s
truncate teardown, alongside the existing test-table deletions. Ensure DELETE
FROM cache_invalidation_tasks runs after migrating model.CacheInvalidationTask
so invalidation outbox rows do not persist between tests.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 13f33c4c-306e-45bd-84e9-3c724707dcc1
📒 Files selected for processing (12)
model/billing_settlement_regression_test.gomodel/cache_invalidation_task.gomodel/cache_invalidation_task_test.gomodel/checkin.gomodel/redemption.gomodel/token_cache.gomodel/user.gomodel/user_cache.gomodel/user_update_test.goservice/system_task.goservice/system_task_test.goservice/task_billing_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
service/system_task_test.gomodel/redemption.gomodel/checkin.gomodel/user_cache.gomodel/token_cache.goservice/system_task.gomodel/cache_invalidation_task.gomodel/cache_invalidation_task_test.gomodel/user.gomodel/billing_settlement_regression_test.goservice/task_billing_test.gomodel/user_update_test.go
🔇 Additional comments (10)
model/redemption.go (1)
155-155: LGTM!Also applies to: 189-192, 203-203
model/checkin.go (1)
96-96: LGTM!Also applies to: 110-119
model/cache_invalidation_task.go (1)
93-116: LGTM!model/cache_invalidation_task_test.go (1)
66-68: LGTM!model/token_cache.go (1)
60-73: LGTM!model/user.go (1)
1648-1675: LGTM!Also applies to: 1687-1716
model/user_cache.go (1)
334-338: LGTM!Also applies to: 478-507
model/user_update_test.go (1)
252-345: LGTM!Also applies to: 460-460, 843-933
service/system_task.go (1)
33-33: LGTM!Also applies to: 223-284, 361-361
service/system_task_test.go (1)
31-56: LGTM!
| pending, err := cacheInvalidationTaskPending(cacheInvalidationKindToken, cacheKey) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| if pending { | ||
| return nil, fmt.Errorf("token cache invalidation pending") | ||
| } |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟡 Minor
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline model/cache_invalidation_task.go --items all
rg -n -C6 --type=go 'func cacheInvalidationTaskPending\(' model
rg -n -C4 --type=go 'cacheInvalidationTaskPending\(' model/token_cache.go model/user_cache.goRepository: MAX-API-Next/MAX-API
Length of output: 5612
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '90,110p' model/cache_invalidation_task.go
sed -n '330,352p' model/token_cache.go
sed -n '410,430p' model/user_cache.go
rg -n -C3 --type=go 'func .*cacheInvalidation.*\(|CacheInvalidationTask|cache_invalidation_tasks' model | head -n 200Repository: MAX-API-Next/MAX-API
Length of output: 14182
Avoid DB lookups on the Redis cache hit path.
cacheInvalidationTaskPending queries cache_invalidation_tasks even when Redis returned one, so successful token/user cache hits still hit the database. Use an in-memory/Redis fence for the normal read path, and keep the DB outbox only for recovery/retry cases.
📍 Affects 2 files
model/token_cache.go#L344-L350(this comment)model/user_cache.go#L423-L429
🤖 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/token_cache.go` around lines 344 - 350, The normal Redis-hit paths in
model/token_cache.go:344-350 and model/user_cache.go:423-429 must stop calling
cacheInvalidationTaskPending, which performs a database lookup. Replace that
check with the established in-memory or Redis invalidation fence, while
retaining cache_invalidation_tasks database checks only for recovery or retry
flows.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model/billing_settlement_regression_test.go (1)
128-146: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd the blocking cache-invalidation hook helper before using it.
model/billing_settlement_regression_test.gocallsnewBlockingCacheInvalidationHook()and useshook.unblock, but the helper is not defined in this file or imported from another package, so this test will not compile.🤖 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/billing_settlement_regression_test.go` around lines 128 - 146, Define the missing newBlockingCacheInvalidationHook helper in billing_settlement_regression_test.go before the test uses it, including the hook state and unblock behavior required by waitForCacheHook and hook.unblock. Keep the helper scoped to the test package and compatible with the existing cache-invalidation hook setup.
🤖 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.
Outside diff comments:
In `@model/billing_settlement_regression_test.go`:
- Around line 128-146: Define the missing newBlockingCacheInvalidationHook
helper in billing_settlement_regression_test.go before the test uses it,
including the hook state and unblock behavior required by waitForCacheHook and
hook.unblock. Keep the helper scoped to the test package and compatible with the
existing cache-invalidation hook setup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e31cc912-133d-462f-8a62-0d7e38231a23
📒 Files selected for processing (2)
model/billing_settlement_regression_test.goservice/task_billing_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
service/task_billing_test.gomodel/billing_settlement_regression_test.go
🔇 Additional comments (1)
service/task_billing_test.go (1)
129-129: LGTM!
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
model/log.go (1)
697-735: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve task-billing log receipts until the settlement effect is terminal.
billing_settlementretries remain pending without an upper bound until manually failed or permanently errored, whilebilling_log_receipts are deleted strictly bycreated_at. If a pending retry crosses that deletion window,ProcessBillingSettlementEffectrunsRecordTaskBillingLogOnceagainst an absent receipt and can insert the task billing log a second time. Tie receipt cleanup to a terminal settlement state, or ensure retention guarantees exceed the maximum possible pending-retry horizon.🤖 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/log.go` around lines 697 - 735, Update the billing log receipt cleanup and settlement flow around BillingLogReceipt, ProcessBillingSettlementEffect, and RecordTaskBillingLogOnce so receipts are retained while a billing_settlement remains pending. Only delete receipts after the associated settlement reaches a terminal state, or enforce retention longer than any possible pending-retry period, preventing a retry from inserting a duplicate task billing log.relay/channel/palm/relay-palm.go (1)
53-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftDuplicate cancellation-safe streaming shutdown pattern. Both handlers independently implement the same
done/producerDone/buffered-stopChancoordination (plus forced upstream body close to unblock a stuck read on client cancellation) to fix the same class of goroutine-lifecycle bug.
relay/channel/palm/relay-palm.go#L53-L119: extract the producer/consumer synchronization skeleton (channel setup, deferred stop-signal,done-awareselects, post-c.Streamclose/wait sequence) into a small reusable streaming helper.relay/channel/zhipu/relay-zhipu.go#L156-L249: adapt its data+meta channel variant to use the same shared helper/primitive.🤖 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 `@relay/channel/palm/relay-palm.go` around lines 53 - 119, Extract the duplicated cancellation-safe producer/consumer lifecycle from palmStreamHandler in relay/channel/palm/relay-palm.go:53-119 into a reusable streaming helper, preserving done-aware sends, buffered stop signaling, upstream response-body closure, and producer completion waiting; adapt the data-plus-meta flow in relay/channel/zhipu/relay-zhipu.go:156-249 to use the same helper or primitive, with no separate synchronization implementation.
🤖 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 `@middleware/rate-limit.go`:
- Around line 85-90: The Redis failure path in the rate-limiting flow
intentionally falls back from redisRateLimiter to applyInMemoryRateLimit,
weakening global enforcement across instances during outages. Preserve this
availability-first behavior, and add operational alerting or logging around
repeated Redis rate-limit failures so the degraded enforcement state is visible.
In `@relay/channel/aws/dto.go`:
- Around line 98-115: The max-tokens fallback logic is duplicated across
providers. Add a shared dto.ResolveMaxTokens(maxTokens, maxCompletionTokens
*uint) *uint helper preserving the existing preference rules, then replace the
inline resolution in relay/channel/aws/dto.go lines 98-115 and
relay/channel/xunfei/relay-xunfei.go lines 52-56 with calls to it.
In `@relay/channel/xunfei/relay-xunfei.go`:
- Line 51: Update the Xunfei request construction to assign request.TopK to
xunfeiRequest.Parameter.Chat.TopK instead of request.N, preserving the
provider’s top-k field semantics.
---
Outside diff comments:
In `@model/log.go`:
- Around line 697-735: Update the billing log receipt cleanup and settlement
flow around BillingLogReceipt, ProcessBillingSettlementEffect, and
RecordTaskBillingLogOnce so receipts are retained while a billing_settlement
remains pending. Only delete receipts after the associated settlement reaches a
terminal state, or enforce retention longer than any possible pending-retry
period, preventing a retry from inserting a duplicate task billing log.
In `@relay/channel/palm/relay-palm.go`:
- Around line 53-119: Extract the duplicated cancellation-safe producer/consumer
lifecycle from palmStreamHandler in relay/channel/palm/relay-palm.go:53-119 into
a reusable streaming helper, preserving done-aware sends, buffered stop
signaling, upstream response-body closure, and producer completion waiting;
adapt the data-plus-meta flow in relay/channel/zhipu/relay-zhipu.go:156-249 to
use the same helper or primitive, with no separate synchronization
implementation.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 15dff5eb-f4c0-48d9-8510-70efd6fe7cea
📒 Files selected for processing (25)
middleware/rate-limit.gomiddleware/rate_limit_test.gomodel/log.gomodel/log_test.gorelay/channel/aws/dto.gorelay/channel/aws/relay_aws_test.gorelay/channel/baidu/dto.gorelay/channel/baidu/relay-baidu.gorelay/channel/baidu/relay_baidu_test.gorelay/channel/cohere/relay-cohere.gorelay/channel/cohere/stream_cancel_test.gorelay/channel/ollama/dto.gorelay/channel/ollama/relay-ollama.gorelay/channel/ollama/stream_test.gorelay/channel/palm/relay-palm.gorelay/channel/palm/stream_cancel_test.gorelay/channel/task/ali/adaptor.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/xunfei/dto.gorelay/channel/xunfei/relay-xunfei.gorelay/channel/xunfei/relay_xunfei_test.gorelay/channel/zhipu/dto.gorelay/channel/zhipu/relay-zhipu.gorelay/channel/zhipu/stream_cancel_test.gotools/jsonwrapcheck/allowlist.txt
💤 Files with no reviewable changes (1)
- tools/jsonwrapcheck/allowlist.txt
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
relay/channel/ollama/dto.gorelay/channel/baidu/dto.gorelay/channel/zhipu/dto.gorelay/channel/palm/stream_cancel_test.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/xunfei/relay_xunfei_test.gorelay/channel/zhipu/stream_cancel_test.gorelay/channel/xunfei/dto.gorelay/channel/ollama/relay-ollama.gorelay/channel/ollama/stream_test.gorelay/channel/baidu/relay_baidu_test.gorelay/channel/aws/relay_aws_test.gorelay/channel/cohere/relay-cohere.gomiddleware/rate-limit.gorelay/channel/cohere/stream_cancel_test.gomodel/log_test.gorelay/channel/palm/relay-palm.gorelay/channel/task/ali/adaptor.gomiddleware/rate_limit_test.gorelay/channel/aws/dto.gorelay/channel/zhipu/relay-zhipu.gorelay/channel/xunfei/relay-xunfei.gomodel/log.gorelay/channel/baidu/relay-baidu.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/ollama/dto.gorelay/channel/baidu/dto.gorelay/channel/zhipu/dto.gorelay/channel/palm/stream_cancel_test.gorelay/channel/task/ali/adaptor_wan27_test.gorelay/channel/xunfei/relay_xunfei_test.gorelay/channel/zhipu/stream_cancel_test.gorelay/channel/xunfei/dto.gorelay/channel/ollama/relay-ollama.gorelay/channel/ollama/stream_test.gorelay/channel/baidu/relay_baidu_test.gorelay/channel/aws/relay_aws_test.gorelay/channel/cohere/relay-cohere.gorelay/channel/cohere/stream_cancel_test.gorelay/channel/palm/relay-palm.gorelay/channel/task/ali/adaptor.gorelay/channel/aws/dto.gorelay/channel/zhipu/relay-zhipu.gorelay/channel/xunfei/relay-xunfei.gorelay/channel/baidu/relay-baidu.go
🔇 Additional comments (24)
model/log.go (2)
17-20: LGTM!Also applies to: 656-673, 754-754, 998-1001
65-73: 🗄️ Data Integrity & IntegrationNo change needed.
BillingLogReceiptis included in both the main DB startup migration and the LOG DB migration.relay/channel/cohere/relay-cohere.go (2)
102-128: LGTM!Also applies to: 186-192
237-239: LGTM!Also applies to: 276-278
relay/channel/ollama/dto.go (1)
39-39: LGTM!Also applies to: 51-51
relay/channel/baidu/dto.go (1)
18-20: LGTM!relay/channel/zhipu/dto.go (1)
17-17: LGTM!relay/channel/palm/stream_cancel_test.go (1)
24-60: LGTM!relay/channel/xunfei/relay_xunfei_test.go (1)
80-91: LGTM!relay/channel/zhipu/stream_cancel_test.go (1)
27-63: LGTM!Also applies to: 65-72
relay/channel/xunfei/dto.go (1)
18-19: LGTM!relay/channel/ollama/relay-ollama.go (1)
25-25: LGTM!Also applies to: 152-152
relay/channel/ollama/stream_test.go (1)
97-132: LGTM!relay/channel/baidu/relay_baidu_test.go (1)
14-47: LGTM!relay/channel/cohere/stream_cancel_test.go (1)
25-63: LGTM!relay/channel/task/ali/adaptor_wan27_test.go (1)
9-9: LGTM!Also applies to: 96-116
relay/channel/aws/relay_aws_test.go (1)
12-12: LGTM!Also applies to: 89-103
model/log_test.go (1)
1194-1197: LGTM!relay/channel/task/ali/adaptor.go (1)
62-65: LGTM! Correctly switches these optional scalars to pointer types so explicitfalse/0values from metadata survive serialization to the upstream Ali API, per the pointer-field guideline for re-marshaled request structs.Also applies to: 467-468, 547-547, 604-604
Source: Coding guidelines
middleware/rate_limit_test.go (1)
286-325: LGTM!Also applies to: 327-384
relay/channel/aws/dto.go (1)
75-78: LGTM! Pointer fields correctly allow explicit-zero values to be preserved in the Nova inference config.relay/channel/zhipu/relay-zhipu.go (1)
97-103: LGTM! Same correct cancellation-safe shutdown pattern as relay-palm.go'spalmStreamHandler. Duplication across handlers is noted in the consolidated comment below.Also applies to: 162-172, 181-209, 242-248
relay/channel/baidu/relay-baidu.go (1)
30-32: LGTM! Good extraction —parseBaiduAccessTokenResponseadds an HTTP-status check and correctly usescommon.DecodeJson, per the JSON-wrapper guideline.Also applies to: 236-265
Source: Coding guidelines
middleware/rate-limit.go (1)
92-98: 🚀 Performance & ScalabilityNo change needed.
InMemoryRateLimiter.Initguards store initialization with a double-checked lock and only starts the cleanup goroutine once, so repeated calls on the middleware hot path remain cheap/idempotent.
| if common.RedisEnabled && redisRateLimiter(c, maxRequestNum, duration, key, policy) { | ||
| return | ||
| } | ||
|
|
||
| applyInMemoryRateLimit(c, maxRequestNum, duration, key, policy) | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Note: Redis-outage fallback trades cross-instance consistency for availability.
When Redis fails, rate limiting reverts to a per-process in-memory limiter, so under multi-instance deployments the effective limit is no longer globally enforced during the outage. This is a reasonable fail-safe design (confirmed by the added tests), just worth being aware of operationally (e.g., alert on repeated Redis rate-limit failures since it signals silently degraded global enforcement).
Also applies to: 100-138
🤖 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 `@middleware/rate-limit.go` around lines 85 - 90, The Redis failure path in the
rate-limiting flow intentionally falls back from redisRateLimiter to
applyInMemoryRateLimit, weakening global enforcement across instances during
outages. Preserve this availability-first behavior, and add operational alerting
or logging around repeated Redis rate-limit failures so the degraded enforcement
state is visible.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model/main.go (1)
946-1001: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoff
migrateSubscriptionPlanPriceAmounthard-fails on metadata query errors, unlike sibling migration functions.Lines 973 and 985 now return a wrapped error when the
information_schemametadata query fails. This is inconsistent withmigrateTokenModelLimitsToText(Line 819, Line 829) andmigrateQuotaColumnsToBigInt(Line 883, Line 903), which still log a warning viacommon.SysLogand proceed with theALTERregardless of the metadata query outcome. Mixing "fail fast" and "warn and continue" semantics across near-identical migration helpers makes the overall startup migration behavior harder to reason about and increases the risk that a transient database hiccup during this one migration blocks startup while equivalent hiccups in the other two do not.Align the error-handling strategy across all three metadata-query-based migrations, either by making all of them return errors on metadata query failure or by keeping all of them as best-effort warnings.
🤖 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/main.go` around lines 946 - 1001, Align the metadata-query error handling in migrateSubscriptionPlanPriceAmount with migrateTokenModelLimitsToText and migrateQuotaColumnsToBigInt, using the same best-effort warning behavior rather than returning errors immediately. On PostgreSQL and MySQL query failures, log a warning through common.SysLog and continue to the existing ALTER statement; preserve the current early returns when metadata confirms the column is already decimal.
🤖 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 `@controller/token.go`:
- Around line 318-320: Validate token.Status against the supported range 1–4
before assigning it in the UpdateToken flow around the token status update.
Reject unsupported nonzero values before cleanToken.Status is modified, while
preserving the existing behavior for status 0 and valid statuses.
In `@model/main.go`:
- Around line 301-305: Update migrateDB so failures from the legacy
migrateSubscriptionPlanPriceAmount step do not abort subsequent DB.AutoMigrate
calls for Channel, Token, User, BillingSettlement, BillingPreConsumeSelection,
and CacheInvalidationTask. Preserve the legacy failure as a warning or otherwise
continue migration, while ensuring the critical AutoMigrate operations still
execute; alternatively, make metadata-query failures inside
migrateSubscriptionPlanPriceAmount non-fatal and skip only the affected ALTER.
---
Outside diff comments:
In `@model/main.go`:
- Around line 946-1001: Align the metadata-query error handling in
migrateSubscriptionPlanPriceAmount with migrateTokenModelLimitsToText and
migrateQuotaColumnsToBigInt, using the same best-effort warning behavior rather
than returning errors immediately. On PostgreSQL and MySQL query failures, log a
warning through common.SysLog and continue to the existing ALTER statement;
preserve the current early returns when metadata confirms the column is already
decimal.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 53ee90d5-f419-4e23-b3c8-9ecf7d114792
📒 Files selected for processing (6)
controller/channel.gocontroller/channel_multi_key_test.gocontroller/token.gocontroller/token_test.gomodel/main.gomodel/main_migration_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
controller/token_test.gocontroller/token.gocontroller/channel_multi_key_test.gocontroller/channel.gomodel/main_migration_test.gomodel/main.go
🔇 Additional comments (12)
controller/token_test.go (1)
640-683: LGTM!controller/token.go (1)
284-296: LGTM!controller/channel_multi_key_test.go (3)
1-69: LGTM!
80-84: LGTM!
71-78: 🎯 Functional CorrectnessNo change needed. The module declares Go 1.25.1, so
for range 2is supported.model/main_migration_test.go (1)
9-12: LGTM!Also applies to: 69-100
model/main.go (6)
228-262: LGTM!
264-299: LGTM!
306-378: Migration ordering and new model registration look correct.
migrateTokenModelLimitsToText,migrateUserQuotaColumnsToBigInt, andmigrateTokenQuotaColumnsToBigInterror returns are now propagated (Line 307-315), andBillingLogReceipt,BillingSettlement,BillingPreConsumeSelection, andCacheInvalidationTaskare registered inAutoMigrate(Line 326, Line 343-345). This is consistent with the durable billing/cache feature described in the PR summary.
607-607: LGTM!
796-928: LGTM!
379-379: 🎯 Functional CorrectnessNo remaining
migrateDBFastreferences exist.
| func migrateDB() error { | ||
| // Migrate price_amount column from float/double to decimal for existing tables | ||
| migrateSubscriptionPlanPriceAmount() | ||
| if err := migrateSubscriptionPlanPriceAmount(); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Legacy migration failure now blocks all subsequent schema migrations.
migrateSubscriptionPlanPriceAmount is called first in migrateDB, and its error now aborts the entire function via the early return err at Line 304. Since DB.AutoMigrate for Channel, Token, User, and the newly added BillingSettlement, BillingPreConsumeSelection, and CacheInvalidationTask (Line 317-351) runs after this call, any failure in the legacy price_amount migration (for example, a transient metadata query error on Line 973 or Line 985) now prevents these newer, more critical tables from ever being created or updated. Previously this failure path only logged a warning and let migration continue.
Consider running the new/critical AutoMigrate step independently of this legacy column-type migration, or treat metadata query failures in migrateSubscriptionPlanPriceAmount as non-fatal (log and skip the ALTER) so a transient error on one legacy column doesn't block essential startup schema changes.
🤖 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/main.go` around lines 301 - 305, Update migrateDB so failures from the
legacy migrateSubscriptionPlanPriceAmount step do not abort subsequent
DB.AutoMigrate calls for Channel, Token, User, BillingSettlement,
BillingPreConsumeSelection, and CacheInvalidationTask. Preserve the legacy
failure as a warning or otherwise continue migration, while ensuring the
critical AutoMigrate operations still execute; alternatively, make
metadata-query failures inside migrateSubscriptionPlanPriceAmount non-fatal and
skip only the affected ALTER.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
relay/channel/openai/relay-openai.go (1)
578-593: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winBill only the current realtime usage increment.
PreWssConsumeQuotareserves quota from the values passed inusage. Passing the runningtotalUsagemakes eachresponse.done/finish call re-reserve the entire accumulated usage. Pass the current usage delta and add it tototalUsageonly after billing succeeds so quota records match the returned billing usage.🤖 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 `@relay/channel/openai/relay-openai.go` around lines 578 - 593, The preConsumeUsage function currently bills the accumulated totalUsage instead of only the current usage increment. Call service.PreWssConsumeQuota with usage, and update totalUsage with the delta only after billing succeeds; preserve the existing validation and return any billing error without mutating the running total.
🤖 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 `@setting/task_billing_setting/rate_card.go`:
- Around line 33-45: The configuration update flow must reject null values for
RWMap-backed settings or restore their initialized maps after updates. Preserve
the initialized RateCards in setting/task_billing_setting/rate_card.go:33-45 and
BillingMode and BillingExpr in setting/billing_setting/tiered_billing.go:22-28;
update the shared UpdateConfigFromMap boundary or these setting handlers so null
cannot leave any RWMap pointer nil.
---
Outside diff comments:
In `@relay/channel/openai/relay-openai.go`:
- Around line 578-593: The preConsumeUsage function currently bills the
accumulated totalUsage instead of only the current usage increment. Call
service.PreWssConsumeQuota with usage, and update totalUsage with the delta only
after billing succeeds; preserve the existing validation and return any billing
error without mutating the running total.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: d6cf95fd-ed98-4d67-929d-8ea3b270823f
📒 Files selected for processing (13)
controller/channel.gocontroller/channel_multi_key_test.gorelay/channel/openai/realtime_connection.gorelay/channel/openai/realtime_connection_test.gorelay/channel/openai/relay-openai.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/hailuo/adaptor_test.gorelay/channel/task/jimeng/adaptor.gorelay/channel/task/jimeng/adaptor_test.gosetting/billing_setting/tiered_billing.gosetting/billing_setting/tiered_billing_test.gosetting/task_billing_setting/rate_card.gosetting/task_billing_setting/rate_card_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
relay/channel/openai/realtime_connection_test.gorelay/channel/task/jimeng/adaptor_test.gosetting/billing_setting/tiered_billing_test.gorelay/channel/openai/realtime_connection.gosetting/task_billing_setting/rate_card.gorelay/channel/task/hailuo/adaptor_test.gorelay/channel/task/hailuo/adaptor.gocontroller/channel.gosetting/billing_setting/tiered_billing.gocontroller/channel_multi_key_test.gorelay/channel/task/jimeng/adaptor.gosetting/task_billing_setting/rate_card_test.gorelay/channel/openai/relay-openai.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/openai/realtime_connection_test.gorelay/channel/task/jimeng/adaptor_test.gorelay/channel/openai/realtime_connection.gorelay/channel/task/hailuo/adaptor_test.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/jimeng/adaptor.gorelay/channel/openai/relay-openai.go
🔇 Additional comments (14)
controller/channel.go (2)
617-705: LGTM!
1379-1861: LGTM!relay/channel/openai/realtime_connection_test.go (1)
1-49: LGTM!relay/channel/task/jimeng/adaptor_test.go (1)
1-21: LGTM!setting/billing_setting/tiered_billing_test.go (1)
1-45: LGTM!relay/channel/task/jimeng/adaptor.go (1)
434-459: LGTM!relay/channel/openai/relay-openai.go (1)
344-364: 🩺 Stability & AvailabilityNo change needed.
runRealtimePingLoopuseswebsocket.WriteControlfor pings, which is safe to call concurrently with thewebsocket.WriteMessagecalls fromhelper.WssString.relay/channel/openai/realtime_connection.go (1)
10-14: LGTM!Also applies to: 16-24, 26-41
setting/task_billing_setting/rate_card.go (1)
140-140: LGTM!Also applies to: 173-177
relay/channel/task/hailuo/adaptor_test.go (1)
13-34: LGTM!relay/channel/task/hailuo/adaptor.go (1)
215-225: LGTM!controller/channel_multi_key_test.go (1)
15-15: LGTM!Also applies to: 62-69
setting/task_billing_setting/rate_card_test.go (2)
5-11: LGTM!
82-105: 📐 Maintainability & Code QualityNo change needed.
The module declares Go 1.25.1, so
for rangeis supported by the declared toolchain.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
setting/config/config.go (1)
249-270: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider reusing
jsonUnmarshalerTypefor the existing inline interface check.Line 255 (unchanged) still uses an ad-hoc anonymous interface literal
interface{ UnmarshalJSON([]byte) error }to detect the same capability that the newjsonUnmarshalerTypeat line 25 now names. Consolidating both checks ontojsonUnmarshalerTyperemoves the duplicate interface declaration and keeps the "does this field implement JSON unmarshaling" concept expressed once.🤖 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 `@setting/config/config.go` around lines 249 - 270, Replace the inline UnmarshalJSON interface assertion in the reflect.Ptr handling with the existing jsonUnmarshalerType assertion, preserving the current unmarshaling and error behavior.relay/channel/openai/relay-openai.go (1)
451-523: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNarrow
stateMu's scope aroundpreConsumeUsagecalls.
stateMu.Lock()at line 451 stays held across thepreConsumeUsagecalls at lines 463 and 488.preConsumeUsagecallsservice.PreWssConsumeQuota, which is very likely to do I/O (DB/cache) given the billing context. While that call is in flight, the client reader goroutine cannot acquirestateMuat line 391, so it cannot process or forward the next client message. A slow or stalled quota call therefore stalls both directions of the realtime relay, not just the target side.Compute the
usage/localUsagesnapshot and reset understateMu, then release the lock before callingpreConsumeUsage.sumUsageis only touched by this goroutine and the post-workers.Wait()flush, so releasing the lock around the external call does not reintroduce a race on it.🔒 Suggested direction for the `ResponseDone`+usage branch (apply the same pattern to the branch at line 488)
if realtimeUsage != nil { usage.TotalTokens += realtimeUsage.TotalTokens usage.InputTokens += realtimeUsage.InputTokens usage.OutputTokens += realtimeUsage.OutputTokens usage.InputTokenDetails.AudioTokens += realtimeUsage.InputTokenDetails.AudioTokens usage.InputTokenDetails.CachedTokens += realtimeUsage.InputTokenDetails.CachedTokens usage.InputTokenDetails.TextTokens += realtimeUsage.InputTokenDetails.TextTokens usage.OutputTokenDetails.AudioTokens += realtimeUsage.OutputTokenDetails.AudioTokens usage.OutputTokenDetails.TextTokens += realtimeUsage.OutputTokenDetails.TextTokens - err := preConsumeUsage(c, info, usage, sumUsage) - if err != nil { - usage = &dto.RealtimeUsage{} - localUsage = &dto.RealtimeUsage{} - stateMu.Unlock() - errChan <- fmt.Errorf("error consume usage: %v", err) - return - } - // 本次计费完成，清除 - usage = &dto.RealtimeUsage{} - - localUsage = &dto.RealtimeUsage{} + usageSnapshot := *usage + usage = &dto.RealtimeUsage{} + localUsage = &dto.RealtimeUsage{} + stateMu.Unlock() + if err := preConsumeUsage(c, info, &usageSnapshot, sumUsage); err != nil { + errChan <- fmt.Errorf("error consume usage: %v", err) + return + } + stateMu.Lock()🤖 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 `@relay/channel/openai/relay-openai.go` around lines 451 - 523, In the realtime event handling loop around stateMu and preConsumeUsage, snapshot and reset usage or localUsage while holding stateMu, then unlock before invoking preConsumeUsage so external quota I/O does not block the relay. Apply this pattern to both ResponseDone usage branches, preserving existing error handling and ensuring the lock is reacquired only where subsequent shared-state updates require it.
🤖 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 `@relay/channel/task/hailuo/adaptor_test.go`:
- Around line 16-19: Update the httptest handler in the ParseTaskResult test to
only record r.URL.Path through a channel or shared result, then perform the
require.Equal assertion in the test goroutine after ParseTaskResult returns.
Preserve the existing HTTP 503 response behavior.
---
Outside diff comments:
In `@relay/channel/openai/relay-openai.go`:
- Around line 451-523: In the realtime event handling loop around stateMu and
preConsumeUsage, snapshot and reset usage or localUsage while holding stateMu,
then unlock before invoking preConsumeUsage so external quota I/O does not block
the relay. Apply this pattern to both ResponseDone usage branches, preserving
existing error handling and ensuring the lock is reacquired only where
subsequent shared-state updates require it.
In `@setting/config/config.go`:
- Around line 249-270: Replace the inline UnmarshalJSON interface assertion in
the reflect.Ptr handling with the existing jsonUnmarshalerType assertion,
preserving the current unmarshaling and error behavior.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: a8ccf8da-11ee-4e24-a4e9-dac96379aa73
📒 Files selected for processing (21)
controller/channel.gocontroller/channel_multi_key_test.gocontroller/token.gocontroller/token_test.gomodel/main.gomodel/main_migration_test.gomodel/option.gomodel/option_test.gorelay/channel/openai/realtime_connection.gorelay/channel/openai/realtime_connection_test.gorelay/channel/openai/relay-openai.gorelay/channel/task/hailuo/adaptor.gorelay/channel/task/hailuo/adaptor_test.gorelay/channel/task/jimeng/adaptor.gorelay/channel/task/jimeng/adaptor_test.gosetting/billing_setting/tiered_billing.gosetting/billing_setting/tiered_billing_test.gosetting/config/config.gosetting/config/config_test.gosetting/task_billing_setting/rate_card.gosetting/task_billing_setting/rate_card_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
setting/config/config_test.gosetting/config/config.gorelay/channel/task/hailuo/adaptor.gorelay/channel/openai/realtime_connection.gocontroller/channel_multi_key_test.gorelay/channel/task/hailuo/adaptor_test.gocontroller/token_test.gorelay/channel/openai/realtime_connection_test.gorelay/channel/task/jimeng/adaptor_test.gosetting/billing_setting/tiered_billing_test.gomodel/option.gosetting/billing_setting/tiered_billing.gocontroller/channel.gocontroller/token.gomodel/main_migration_test.gomodel/option_test.gosetting/task_billing_setting/rate_card.gorelay/channel/openai/relay-openai.gosetting/task_billing_setting/rate_card_test.gomodel/main.gorelay/channel/task/jimeng/adaptor.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/task/hailuo/adaptor.gorelay/channel/openai/realtime_connection.gorelay/channel/task/hailuo/adaptor_test.gorelay/channel/openai/realtime_connection_test.gorelay/channel/task/jimeng/adaptor_test.gorelay/channel/openai/relay-openai.gorelay/channel/task/jimeng/adaptor.go
🔇 Additional comments (21)
setting/config/config_test.go (1)
120-131: LGTM!setting/config/config.go (1)
21-26: 🗄️ Data Integrity & IntegrationNull-rejection for pointer-backed JSON config fields looks correct.
The pre-pass in
validateConfigPointerNullsruns before any field mutation and usesfield.Type().Implements(jsonUnmarshalerType), which is evaluated on the static type, so it correctly blocksnulleven when the pointer field is currently nil. This closes the gap from the earlier review wherenullcould zero a*types.RWMapfield viareflect.Zerowithout going through itsUnmarshalJSON.Verify that
types.RWMapimplementsUnmarshalJSONon a pointer receiver, and that this protection applies to every other*types.RWMap-typed config field beyondRateCards(for exampleBillingMode/BillingExprinsetting/billing_setting/tiered_billing.go, which is not in this review batch).#!/bin/bash set -euo pipefail fd -t f 'rw_map' types 2>/dev/null | xargs -r cat -n echo '--- UnmarshalJSON receiver check ---' rg -n -B2 -A15 'func \(.*RWMap.*\) UnmarshalJSON' types 2>/dev/null || true echo '--- other *types.RWMap config fields ---' rg -n 'types\.RWMap\[' setting --type=goAlso applies to: 181-183, 293-316, 327-339
relay/channel/task/hailuo/adaptor.go (1)
215-225: 🩺 Stability & AvailabilityConfirm the HTTP client used by
buildVideoURLhas a timeout.The success branch now calls
buildVideoURL, which issues a synchronous HTTP request throughservice.GetHttpClient()with no per-request deadline set at this call site. SinceParseTaskResultruns on every poll while status is success but the URL isn't yet available, an unresponsive upstreamfiles/retrieveendpoint could block a poll cycle. Confirmservice.GetHttpClient()has a bounded timeout.#!/bin/bash set -euo pipefail rg -n -B3 -A20 'func GetHttpClient' service --type=gorelay/channel/openai/realtime_connection.go (1)
1-42: LGTM!model/main_migration_test.go (1)
68-103: LGTM!model/option_test.go (1)
157-211: LGTM!setting/task_billing_setting/rate_card.go (1)
32-46: LGTM! The switch toRWMapplus reading throughReadAll()infindRateCardandGetRateCardsCopyresolves the prior concurrent-read concern forRateCards. Null-safety for this pointer field is now handled at thesetting/config/config.golayer (see that file's review comment for the remaining verification item ontypes.RWMap'sUnmarshalJSON).Also applies to: 139-178
relay/channel/openai/relay-openai.go (2)
583-593: 🗄️ Data Integrity & Integration | 🏗️ Heavy liftVerify whether
PreWssConsumeQuotaexpects the delta or the cumulative total.
preConsumeUsagenow buildsnextUsageas*totalUsageplus this call'susagedeltas, stores it back into*totalUsage, and passes that updated cumulative total toservice.PreWssConsumeQuota. Per the summary, this replaces passing only the current call's usage. IfPreWssConsumeQuotadeducts quota based on the value it receives, feeding it the running cumulative total on every invocation (instead of the marginal delta) will make each subsequent call consume progressively larger amounts, over-charging the session. ConfirmPreWssConsumeQuota's contract before merging.#!/bin/bash set -euo pipefail rg -n -B3 -A40 'func PreWssConsumeQuota' service --type=go echo '--- other callers, to compare delta vs cumulative usage patterns ---' rg -n -B3 -A5 'PreWssConsumeQuota\(' --type=go
344-424: LGTM! Worker lifecycle setup, per-goroutine ownership ofclientClosed/targetClosed, bufferederrChansized to the number of senders, and non-blocking ping-error reporting are all correctly structured to avoid goroutine leaks and double-close panics.Also applies to: 523-564
setting/task_billing_setting/rate_card_test.go (1)
5-11: LGTM!Also applies to: 81-105
model/main.go (1)
301-378: LGTM! Reordering the legacymigrateSubscriptionPlanPriceAmountcall to run after the coreAutoMigrateresolves the earlier concern that a legacy migration failure could block creation of the new billing/cache tables. This is backed by the new test in model/main_migration_test.go.relay/channel/task/jimeng/adaptor.go (1)
440-448: LGTM! The early return correctly prevents a failure response from being overwritten by the subsequent status switch.controller/channel_multi_key_test.go (2)
15-69: LGTM!Also applies to: 72-84
71-71: 📐 Maintainability & Code QualityNo change needed. The module sets Go 1.25.1, so these integer
rangeloops are compatible.controller/token_test.go (1)
685-733: LGTM!relay/channel/openai/realtime_connection_test.go (1)
14-49: LGTM!relay/channel/task/jimeng/adaptor_test.go (1)
10-21: LGTM!model/option.go (1)
294-296: LGTM!Also applies to: 310-310, 329-339
controller/channel.go (1)
1387-1400: LGTM!controller/token.go (1)
263-267: LGTM!Also applies to: 288-305, 323-325, 339-346
setting/billing_setting/tiered_billing.go (1)
22-55: 📐 Maintainability & Code QualityNo change needed.
RWMapis updated in place during configuration reload, so the preserved billing-mode and billing-expression string data remain available to the tiered billing system. The expression semantics are stored as-is in these fields and are applied by the expression compiler/runtime.
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
model/ability.go (1)
169-192: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick winFix the index-out-of-range panic when every ability at a priority tier is excluded.
The
len(abilities) == 0guard at line 170 runs before exclusion filtering. The loop at lines 175-185 thencontinues past every ability whoseChannelIdis inexcludedChannelIDs. If all abilities inabilitiesbelong to excluded channels,prioritiesends up empty whileabilitieswas non-empty, so the early guard never triggers.With
prioritiesempty,retry >= len(priorities)at line 189 is always true, soretrybecomes-1.targetPriority := priorities[retry]at line 192 then indexespriorities[-1], which panics with "index out of range [-1]".This is reachable in production: a group/model whose queried priority tier has exactly one ability triggers this on the very next retry after that one channel gets excluded by
retryParam.ExcludeChannelincontroller/relay.go.model/channel_cache.go'sGetRandomSatisfiedChannelExcluding(lines 148-159) already guards the equivalent case with a secondlen(channels) == 0check after exclusion filtering; this function is missing that guard.Because the panic happens before
maxAPIError/taskErris set in the callers, the deferred billing-refund logic incontroller/relay.godoes not run, so pre-consumed quota is not refunded when Gin's recovery middleware catches the panic.🐛 Proposed fix
sort.Slice(priorities, func(i, j int) bool { return priorities[i] > priorities[j] }) + if len(priorities) == 0 { + return 0, false + } if retry >= len(priorities) { retry = len(priorities) - 1 } targetPriority := priorities[retry]🤖 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/ability.go` around lines 169 - 192, Update selectChannelIdFromAbilities to handle an empty priorities slice after excluded channels are filtered out: return the same no-selection result used for an empty input before indexing priorities. Preserve the existing retry clamping and priority selection behavior when at least one eligible priority remains.model/subscription.go (1)
497-532: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared "active subscription still grants this group" query.
downgradeUserGroupForSubscriptionTx(lines 512-520) andExpireDueSubscriptions(lines 1199-1207) both build the same query: filter active subscriptions for the user with a non-emptyupgrade_group, matched againstTRIM(upgrade_group) = currentGroup, limited to 1 row, and checkRowsAffected. Only theid <> ?exclusion differs.Duplicated business logic like this can drift apart in future changes. Fix one site without updating the other, and the two downgrade paths silently disagree on when to retain the current group.
Extract a shared helper, for example:
func activeSubscriptionGrantsGroupTx(tx *gorm.DB, userId int, excludeSubId int, now int64, group string) (bool, error) { query := tx.Where("user_id = ? AND status = ? AND end_time > ? AND upgrade_group <> ''", userId, "active", now) if excludeSubId > 0 { query = query.Where("id <> ?", excludeSubId) } result := query.Where("TRIM(upgrade_group) = ?", group).Limit(1).Find(&UserSubscription{}) return result.RowsAffected > 0, result.Error }Call it from both
downgradeUserGroupForSubscriptionTxandExpireDueSubscriptions.Also applies to: 1196-1209
🤖 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/subscription.go` around lines 497 - 532, Extract the duplicated active-subscription group check into a shared helper such as activeSubscriptionGrantsGroupTx, preserving the optional excluded subscription ID behavior. Replace the inline queries in both downgradeUserGroupForSubscriptionTx and ExpireDueSubscriptions with this helper, and retain each caller’s existing handling of the boolean result and returned error.
🤖 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 `@relay/channel/openai/adaptor.go`:
- Around line 522-539: Update the mask branch in the image/form-building flow to
preserve underlying errors from helper.CreateFormFileWithContentType and
io.Copy. Replace the fixed errors in the mask failure paths with contextual
fmt.Errorf messages using %w, matching the diagnostic wrapping pattern used by
the preceding image loop while retaining immediate maskFile.Close handling.
---
Outside diff comments:
In `@model/ability.go`:
- Around line 169-192: Update selectChannelIdFromAbilities to handle an empty
priorities slice after excluded channels are filtered out: return the same
no-selection result used for an empty input before indexing priorities. Preserve
the existing retry clamping and priority selection behavior when at least one
eligible priority remains.
In `@model/subscription.go`:
- Around line 497-532: Extract the duplicated active-subscription group check
into a shared helper such as activeSubscriptionGrantsGroupTx, preserving the
optional excluded subscription ID behavior. Replace the inline queries in both
downgradeUserGroupForSubscriptionTx and ExpireDueSubscriptions with this helper,
and retain each caller’s existing handling of the boolean result and returned
error.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 72b2570e-ed21-4598-92da-8e1f18a253d2
📒 Files selected for processing (19)
controller/relay.gocontroller/topup_stripe.gocontroller/topup_stripe_test.gomodel/ability.gomodel/channel.gomodel/channel_cache.gomodel/channel_cache_concurrency_test.gomodel/channel_selection_test.gomodel/subscription.gomodel/subscription_reset_test.gomodel/topup.gorelay/channel/openai/adaptor.gorelay/channel/openai/image_edit_test.gorelay/channel/replicate/adaptor.gorelay/channel/task/hailuo/adaptor_test.gorelay/channel/task/sora/adaptor.gorelay/helper/multipart.gorelay/helper/multipart_test.goservice/channel_select.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
controller/topup_stripe_test.gomodel/channel_cache_concurrency_test.gorelay/helper/multipart.gomodel/subscription_reset_test.gomodel/ability.gorelay/helper/multipart_test.gorelay/channel/task/hailuo/adaptor_test.gomodel/channel.gomodel/channel_selection_test.goservice/channel_select.gorelay/channel/openai/image_edit_test.gomodel/topup.gorelay/channel/task/sora/adaptor.gomodel/channel_cache.gorelay/channel/replicate/adaptor.gocontroller/topup_stripe.gocontroller/relay.gorelay/channel/openai/adaptor.gomodel/subscription.go
relay/channel/**/*.go
📄 CodeRabbit inference engine (AGENTS.md)
When implementing a new channel, confirm whether the provider supports
StreamOptions; if it does, add the channel tostreamSupportedChannels.
Files:
relay/channel/task/hailuo/adaptor_test.gorelay/channel/openai/image_edit_test.gorelay/channel/task/sora/adaptor.gorelay/channel/replicate/adaptor.gorelay/channel/openai/adaptor.go
🔇 Additional comments (21)
controller/topup_stripe_test.go (1)
1-107: LGTM!model/channel_cache_concurrency_test.go (2)
13-98: LGTM!
71-71: 🩺 Stability & AvailabilityNo action needed. The module declares
go 1.25.1, sofor range 3is supported.model/channel.go (1)
200-219: LGTM!Also applies to: 265-271
model/channel_selection_test.go (1)
161-199: LGTM!model/topup.go (1)
111-127: LGTM!controller/topup_stripe.go (2)
176-223: LGTM!Also applies to: 264-269, 291-325
282-289: 🗄️ Data Integrity & IntegrationNo change needed.
relay/helper/multipart.go (1)
1-37: LGTM!model/subscription_reset_test.go (1)
8-11: LGTM!Also applies to: 204-318
relay/helper/multipart_test.go (1)
1-44: LGTM!relay/channel/task/hailuo/adaptor_test.go (1)
7-7: LGTM!Also applies to: 17-19, 37-42
relay/channel/openai/image_edit_test.go (1)
21-24: LGTM!Also applies to: 35-35, 75-75, 99-117
relay/channel/task/sora/adaptor.go (1)
19-19: LGTM!Also applies to: 205-205
relay/channel/replicate/adaptor.go (1)
20-20: LGTM!Also applies to: 449-449
model/subscription.go (1)
607-676: LGTM!controller/relay.go (1)
234-234: LGTM!Also applies to: 506-536, 585-587, 599-601, 610-647
relay/channel/openai/adaptor.go (1)
495-513: LGTM!Also applies to: 562-589
model/ability.go (1)
68-74: LGTM!Also applies to: 97-97, 193-216
model/channel_cache.go (1)
91-99: LGTM!Also applies to: 119-127, 148-159
service/channel_select.go (1)
15-21: LGTM!Also applies to: 50-61, 132-132, 173-173
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@controller/model_sync_test.go`:
- Around line 12-31: The test coverage around
TestFetchUpstreamMetadataFailsWhenVendorsAreUnavailable should also exercise
fetchUpstreamMetadata’s models-envelope failure and transport-error paths, using
valid vendor data for the models failure case and a 404 response for the
transport failure case; retain the existing vendor-envelope failure and
success-path coverage where applicable, and assert the returned errors identify
the upstream operation.
In `@model/metadata_name_key.go`:
- Around line 40-113: Update migrateMetadataNameKeys to handle duplicate active
name_key values before ensureMetadataNameKeyIndexes creates unique indexes. Add
a helper near backfillMetadataNameKeys that detects duplicate active Vendor and
Model keys and deterministically resolves them by assigning retired keys to all
but the earliest record, preserving the earliest record’s active key; invoke it
after backfilling and before index creation for both MySQL and transactional
paths.
In `@model/model_meta.go`:
- Around line 88-90: Update Model.Delete to execute
softDeleteMetadataWithNameKey inside DB.Transaction, matching the transaction
pattern used by Vendor.Delete. Pass the transaction handle to the soft-delete
helper and return any transaction or helper error so the name_key update and row
deletion remain atomic.
In `@model/option.go`:
- Around line 213-222: Enforce IsRegisteredOptionKey in both model.UpdateOption
and model.UpdateOptionsBulk before any database write or OptionMap commit.
Reject every unregistered key, including non-dotted names accepted by
validateOptionUpdate, while preserving the existing validation and atomic
bulk-update behavior for registered keys.
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: aa534d65-4d72-40a3-aa7b-1b2649ac5c81
📒 Files selected for processing (15)
controller/model.gocontroller/model_list_test.gocontroller/model_meta.gocontroller/model_sync.gocontroller/model_sync_test.gocontroller/option.gocontroller/option_test.gocontroller/vendor_meta.gomodel/main.gomodel/metadata_name_key.gomodel/metadata_name_key_test.gomodel/model_meta.gomodel/option.gomodel/option_test.gomodel/vendor_meta.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: In Go business code, all JSON marshal/unmarshal operations must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) and must not directly callencoding/jsonfor actual encode/decode work.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+; prefer GORM abstractions over raw SQL, avoid directAUTO_INCREMENT/SERIAL, usecommonGroupCol/commonKeyColandcommonTrueVal/commonFalseValfor DB-specific SQL, branch withcommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQL, avoid unsupported DB-specific functions/operators without fallback, and make migrations work across all three databases.
When implementing a new relay channel, confirm whether the provider supportsStreamOptions; if it does, add that channel tostreamSupportedChannels.
For request structs parsed from client JSON and re-marshaled to upstream providers, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved instead of dropped.
When working on tiered/dynamic billing expression code, readpkg/billingexpr/expr.mdfirst and follow its documented expression language, architecture, token normalization, quota conversion, and versioning patterns.
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrapper functions incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly importing or callingencoding/jsonfor actual marshal/unmarshal work.
All database code in Go must remain compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless necessary, use the shared DB helper variables for reserved words and boolean literals, branch with thecommon.UsingPostgreSQL/common.UsingSQLite/common.UsingMySQLflags when need...
Files:
controller/model_sync_test.gocontroller/option.gocontroller/option_test.gocontroller/model_list_test.gocontroller/model.gocontroller/model_meta.gocontroller/vendor_meta.gomodel/metadata_name_key.gomodel/metadata_name_key_test.gomodel/option_test.gomodel/option.gomodel/vendor_meta.gocontroller/model_sync.gomodel/model_meta.gomodel/main.go
🪛 ast-grep (0.45.0)
model/metadata_name_key.go
[warning] 129-134: Detected a SQL statement built with 'fmt.Sprintf' and passed directly to 'db.Exec'/'db.ExecContext'. Interpolating values into a query string lets an attacker inject arbitrary SQL. Use parameterized queries instead: pass the SQL with placeholders ('?' or '') as the query argument and supply the values as separate arguments, e.g. 'db.Exec("UPDATE t SET x = ? WHERE id = ?", x, id)'.
Context: db.Exec(fmt.Sprintf(
"CREATE UNIQUE INDEX %s ON %s (%s)",
quoteDBIdentifier(index),
quoteDBIdentifier(table),
quoteDBIdentifier(column),
))
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-exec-sprintf-go)
model/metadata_name_key_test.go
[error] 46-46: SQL query is built by concatenating a string literal with a variable and passed to a database/sql call (Query, Exec, QueryRow, Prepare, or their Context variants). String concatenation lets attacker-controlled input alter the query structure, enabling SQL injection. Use parameterized queries with placeholders ('?' or '') and pass the values as separate arguments instead of concatenating them into the query string.
Context: db.Exec("CREATE UNIQUE INDEX "+legacyVendorNameKeyIndex+" ON vendors (name, deleted_at)")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-query-string-concat-go)
[error] 47-47: SQL query is built by concatenating a string literal with a variable and passed to a database/sql call (Query, Exec, QueryRow, Prepare, or their Context variants). String concatenation lets attacker-controlled input alter the query structure, enabling SQL injection. Use parameterized queries with placeholders ('?' or '') and pass the values as separate arguments instead of concatenating them into the query string.
Context: db.Exec("CREATE UNIQUE INDEX "+legacyModelNameKeyIndex+" ON models (model_name, deleted_at)")
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection').
(sql-injection-query-string-concat-go)
🪛 OpenGrep (1.26.0)
model/metadata_name_key.go
[ERROR] 130-135: SQL query built via fmt.Sprintf or string concatenation passed to a database method. Use parameterized queries with placeholder arguments.
(coderabbit.sql-injection.go-query-format)
model/metadata_name_key_test.go
[ERROR] 47-47: SQL query built via fmt.Sprintf or string concatenation passed to a database method. Use parameterized queries with placeholder arguments.
(coderabbit.sql-injection.go-query-format)
[ERROR] 48-48: SQL query built via fmt.Sprintf or string concatenation passed to a database method. Use parameterized queries with placeholder arguments.
(coderabbit.sql-injection.go-query-format)
🔇 Additional comments (18)
model/option.go (2)
4-4: LGTM!Also applies to: 77-80, 325-328, 342-342
203-206: 🗄️ Data Integrity & IntegrationNo change needed.
updateOptionMapalready callsnormalizeOptionUpdateValuebeforevalidateOptionUpdate, soloadOptionsFromDatabasealready validates and normalizes restoredDataExportIntervalvalues.controller/model_sync_test.go (1)
33-39: LGTM!controller/option_test.go (1)
14-34: LGTM!controller/model_list_test.go (1)
247-266: LGTM!controller/model.go (1)
315-323: LGTM!model/option_test.go (1)
194-205: LGTM!Also applies to: 226-242
controller/model_sync.go (1)
58-67: LGTM!Also applies to: 69-74, 265-302, 347-349, 511-516, 528-530
controller/option.go (1)
131-137: 🎯 Functional CorrectnessNo change needed.
common.OptionMapis initialized with the full static option registry before database rows are loaded, and registered compound config keys are populated throughconfig.GlobalConfig.ExportAllConfigs().controller/model_meta.go (1)
148-166: LGTM!controller/vendor_meta.go (1)
4-4: LGTM!Also applies to: 120-129
model/metadata_name_key.go (2)
1-35: LGTM!Also applies to: 115-127
129-136: 🗄️ Data Integrity & IntegrationNo change needed for multi-database identifier quoting.
quoteDBIdentifieruses PostgreSQL-compatible double-quote escaping for PostgreSQL and backtick escaping for MySQL/SQLite, matching the appliedCREATE UNIQUE INDEXstatement dialects.model/metadata_name_key_test.go (2)
38-69: LGTM!
71-84: LGTM!model/vendor_meta.go (1)
4-12: LGTM!Also applies to: 21-36, 53-73
model/model_meta.go (1)
26-27: LGTM!Also applies to: 37-37, 52-52, 81-86, 92-95
model/main.go (1)
301-301: LGTM!The reordering of
migrateSubscriptionPlanPriceAmountto run afterDB.AutoMigrateresolves the previously flagged concern that a legacy migration failure could block creation of the newer billing tables.Also applies to: 313-347, 351-357, 376-380
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@web/default/src/features/system-settings/models/tiered-pricing-editor.runtime.test.tsx`:
- Around line 97-109: 添加显式返回类型：为 getInputValueByLabel 标注 string，为两个 renderPanel
辅助函数、renderEditor 和 selectRow 根据其实际返回值标注类型，并为新增的异步测试回调标注
Promise<void>；保持现有实现和行为不变。
🪄 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: ASSERTIVE
Plan: Pro Plus
Run ID: 71fa5e1d-4f54-4f18-b650-44c918e96b91
📒 Files selected for processing (2)
web/default/src/features/system-settings/models/model-pricing-sheet.tsxweb/default/src/features/system-settings/models/tiered-pricing-editor.runtime.test.tsx
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (2)
web/default/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (web/default/AGENTS.md)
web/default/src/**/*.{ts,tsx}: 前端页面文本与组件内文案必须支持 i18n：React 组件中应使用useTranslation()取得t，并通过t()渲染用户可见文本；子组件也应自行使用useTranslation()保持独立性。
禁止使用两层及以上嵌套三元表达式；复杂逻辑应改用if-else、提前返回或抽取函数。
控制函数圈复杂度，复杂逻辑应拆成更小的函数；变量与函数命名应有意义并遵循驼峰等常规命名约定。
TypeScript 代码应避免使用any，优先使用具体类型或unknown；参数与返回值应显式标注类型；仅类型用途的导入应使用import type。
修改 TypeScript 或 TSX 代码后必须执行类型检查，并修复所有类型错误，不得遗留。
对象非必要不要解构，尤其是组件 props；优先直接使用props.xxx以保持代码清晰。
组件应使用函数式组件与 Hooks，遵循单一职责；组件 props 必须有明确类型（接口或类型别名）。
单文件超过约 200 行时应考虑拆分子组件或抽取自定义 Hooks；类型定义可与组件同文件或放在同模块的types中。
在 React 中应合理使用useMemo、useCallback、React.memo，避免在渲染路径中创建新对象或数组；必要时进行代码分割与动态import。
React Query 的数据获取应使用useQuery、变更应使用useMutation；每个查询需配置唯一queryKey，并在成功后对相关查询执行invalidateQueries；服务端错误应统一交给handleServerError。
Axios 请求应使用项目统一的api实例；GET 请求默认去重，特殊请求可显式关闭；认证与通用错误应在拦截器中统一处理。
服务端错误应统一使用handleServerError，展示层应使用toast.error等统一方式；文案需走 i18n；路由级错误应由errorComponent承接；表单错误应通过form.setError等方式映射到字段。
样式应以 Tailwind 工具类为主，动态类名使用cn()合并；非动态场景避免内联样式；响应式采用移动优先与 Tailwind 断点，主题与暗色模式通过 CSS 变量与dark:处理。
应使用语义化 HTML、正确关联label与输入、保证键盘可操作与合理焦点顺序；必要时添加 ARIA 属性，装饰性图标应使用aria-hidden="true"。
认证与权限应在路由与接口层校验；前后端都应做数据校验（如 Zod）；敏感信息不得落前端存储；避免使用dangerouslySetInnerHTML；跨域与 Cookie 需配合withCredentials并按后端要求处理 CSRF。
组件测试应使用 React Testing Library，关注交互与行为，避免测试实现细节；关键流程可补充集成与 E2E 测试。
环境变量应通过.env读取，并使用VITE_前缀；代码中不得硬编码密钥。
Files:
web/default/src/features/system-settings/models/model-pricing-sheet.tsxweb/default/src/features/system-settings/models/tiered-pricing-editor.runtime.test.tsx
web/default/src/features/**
📄 CodeRabbit inference engine (web/default/AGENTS.md)
功能模块应放在
src/features/<feature>/，并按需包含components/、lib/、hooks/、api.ts、types.ts、constants.ts等；通用组件应放在src/components/，通用工具与类型应放在src/lib/。
Files:
web/default/src/features/system-settings/models/model-pricing-sheet.tsxweb/default/src/features/system-settings/models/tiered-pricing-editor.runtime.test.tsx
🔇 Additional comments (3)
web/default/src/features/system-settings/models/model-pricing-sheet.tsx (1)
446-461: LGTM!web/default/src/features/system-settings/models/tiered-pricing-editor.runtime.test.tsx (2)
30-30: LGTM!Also applies to: 52-52, 163-163, 248-262
294-353: 📐 Maintainability & Code QualityNo change needed.
The test disposes the
ReactRootwithunmount(root, container)in afinallyblock.
| function getInputValueByLabel(container: HTMLElement, labelText: string) { | ||
| const label = Array.from(container.querySelectorAll('label')).find( | ||
| (candidate) => candidate.textContent === labelText | ||
| ) | ||
| assert.ok(label, `missing ${labelText} label`) | ||
|
|
||
| const inputId = label.getAttribute('for') | ||
| assert.ok(inputId, `missing input id for ${labelText}`) | ||
|
|
||
| const input = dom.window.document.getElementById(inputId) as HTMLInputElement | ||
| assert.ok(input, `missing input for ${labelText}`) | ||
| return input.value | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add explicit return types to the new functions.
Add return types to getInputValueByLabel, both renderPanel helpers, renderEditor, selectRow, and the new async test callbacks.
As per coding guidelines, “参数与返回值应显式标注类型”.
Also applies to: 264-279, 294-342
🤖 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
`@web/default/src/features/system-settings/models/tiered-pricing-editor.runtime.test.tsx`
around lines 97 - 109, 添加显式返回类型：为 getInputValueByLabel 标注 string，为两个 renderPanel
辅助函数、renderEditor 和 selectRow 根据其实际返回值标注类型，并为新增的异步测试回调标注
Promise<void>；保持现有实现和行为不变。
Source: Coding guidelines
1 participant
Important
📝 变更描述 / Description
(简述：做了什么？为什么这样改能生效？请基于你对代码逻辑的理解来写，避免粘贴未经整理的内容)
🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
(请在此粘贴截图、关键日志或测试报告，以证明变更生效)