feat: grok Usage Guidelines Violation Fee - #2753
Conversation
WalkthroughThis PR adds a Grok violation-fee subsystem: error normalization for CSAM/violation errors, fee calculation and charging logic, Grok configuration, and frontend support to display and manage violation-fee data and logs. Changes
Sequence DiagramsequenceDiagram
participant Client as Client/API
participant Relay as Relay Controller
participant VF as ViolationFee Service
participant Quota as Quota Management
participant Log as Consumption Logger
Client->>Relay: Send request / receive response
Relay->>Relay: PreConsumeQuota
Relay->>Relay: Process upstream call (may error)
Relay->>VF: NormalizeViolationFeeError(apiError)
VF-->>Relay: Normalized API error (maybe marked skip-retry)
alt Error indicates violation fee
Relay->>VF: ChargeViolationFeeIfNeeded(ctx, relayInfo, apiErr)
VF->>VF: shouldChargeViolationFee & calcViolationFeeQuota
VF->>Quota: PostConsumeQuota(feeQuota)
Quota-->>VF: Quota updated
VF->>Log: Record consumption with fee metadata
Log-->>VF: Logged
end
Relay->>Relay: ReturnPreConsumedQuota if FinalPreConsumedQuota != 0
Relay-->>Client: Return error response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@controller/relay.go`:
- Around line 222-223: Remove the redundant early normalization call to
service.NormalizeViolationFeeError in the relay handler's success/failure path
and rely on the single normalize call in the defer block; specifically delete
the invocation of service.NormalizeViolationFeeError that occurs after the
shouldRetry check so errors are only normalized once in the defer, leaving
IsViolationFeeCode and HasCSAMViolationMarker logic unchanged.
In `@service/violation_fee.go`:
- Around line 125-131: The sequential calls to PostConsumeQuota,
model.UpdateUserUsedQuotaAndRequestCount, and model.UpdateChannelUsedQuota can
leave quotas inconsistent if one succeeds and a later one fails; wrap these
operations in a single atomic transaction (or implement explicit
compensation/rollback) so that either all three succeed or all are reverted,
e.g., begin a DB transaction around PostConsumeQuota and the two model updates,
check errors on each call, rollback the transaction and log the error if any
step fails, and commit only after all succeed; ensure transaction handling is
added around PostConsumeQuota, UpdateUserUsedQuotaAndRequestCount, and
UpdateChannelUsedQuota and that errors propagate to calling code.
🧹 Nitpick comments (5)
web/src/helpers/log.js (1)
27-31: Inconsistent return type on parse failure.The function returns
{}for empty/null/undefined inputs but returnsnullon JSON parse failure. This inconsistency could cause downstream issues if callers expect an object type.Consider returning
{}on parse failure for consistency:Suggested fix
try { return JSON.parse(otherStr); } catch (e) { console.error(`Failed to parse record.other: "${otherStr}".`, e); - return null; + return {}; }web/src/pages/Setting/Model/SettingGrokModel.jsx (2)
62-68: Fragile error detection relying onundefinedin response array.The check
res.includes(undefined)assumes the API wrapper returnsundefinedon failure. This may not be reliable depending on how the API helper handles errors. If errors are thrown instead, the.catch()block handles them, but if the API returns a response with an error status,undefinedmay not be present.Additionally, the single-request case silently returns without user feedback when
undefinedis found.Consider checking for actual success/failure from the response data:
Suggested improvement
Promise.all(requestQueue) .then((res) => { - if (requestQueue.length === 1) { - if (res.includes(undefined)) return; - } else if (requestQueue.length > 1) { - if (res.includes(undefined)) - return showError(t('部分保存失败，请重试')); - } + const failures = res.filter((r) => !r || !r.data?.success); + if (failures.length > 0) { + showError(t('部分保存失败，请重试')); + return; + } showSuccess(t('保存成功')); props.refresh(); })
137-142: Consider adding a maximum value constraint.The
Form.InputNumberhasmin={0}but nomaxprop. A user could accidentally enter an extremely large violation fee amount. Consider adding a reasonable upper bound to prevent misconfiguration.service/violation_fee.go (2)
107-109: Remove or address commented-out playground check.The commented-out
IsPlaygroundcheck appears to be debug code or an incomplete feature. If playground users should not be charged violation fees, uncomment this block. Otherwise, remove the dead code to improve clarity.
137-147: Consider derivingviolation_fee_codefrom actual error code.Line 139 hardcodes
ErrorCodeViolationFeeGrokCSAMin theothermap, butshouldChargeViolationFeecan returntrueviaHasCSAMViolationMarkereven if the error wasn't normalized. For consistency, consider usingapiErr.GetErrorCode()or ensuring normalization always happens before this function is called.Suggested fix
other := map[string]any{ "violation_fee": true, - "violation_fee_code": string(types.ErrorCodeViolationFeeGrokCSAM), + "violation_fee_code": string(apiErr.GetErrorCode()), "fee_quota": feeQuota,
| newAPIError = service.NormalizeViolationFeeError(newAPIError) | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and examine the relay.go file to see the context at lines 222-223 and 171
fd -t f "relay.go" | head -5Repository: QuantumNous/new-api
Length of output: 82
🏁 Script executed:
# Get the content around line 222 and the defer block around line 171
cat -n controller/relay.go | sed -n '160,230p'Repository: QuantumNous/new-api
Length of output: 3017
🏁 Script executed:
# Search for the normalization function definitions
rg -n "func.*NormalizeViolationFeeError|func.*WrapAsViolationFeeGrokCSAM" -A 15Repository: QuantumNous/new-api
Length of output: 1916
🏁 Script executed:
# Check for idempotency documentation or markers
rg -n "idempotent|violation_fee_marker|ErrorCodeViolationFee" -B 2 -A 5Repository: QuantumNous/new-api
Length of output: 4255
🏁 Script executed:
# Find complete IsViolationFeeCode function
rg -n "func.*IsViolationFeeCode" -A 10Repository: QuantumNous/new-api
Length of output: 658
🏁 Script executed:
# Find HasCSAMViolationMarker function
rg -n "func.*HasCSAMViolationMarker" -A 10Repository: QuantumNous/new-api
Length of output: 628
🏁 Script executed:
# Check all violation fee related error codes
rg -n "ErrorCode.*[Vv]iolation" -B 1 -A 1Repository: QuantumNous/new-api
Length of output: 995
🏁 Script executed:
# Find ViolationFeeCodePrefix definition
rg -n "ViolationFeeCodePrefix" -B 2 -A 2Repository: QuantumNous/new-api
Length of output: 582
🏁 Script executed:
# Check ToOpenAIError implementation to understand behavior
rg -n "func.*ToOpenAIError" -A 15Repository: QuantumNous/new-api
Length of output: 1489
🏁 Script executed:
# Check the shouldRetry and processChannelError calls to understand flow
rg -n "func shouldRetry|func processChannelError" -A 5Repository: QuantumNous/new-api
Length of output: 1365
🏁 Script executed:
# Check if there are other paths to line 171 that bypass line 222
rg -n "break" controller/relay.go | grep -A 2 -B 2 "191\|203"Repository: QuantumNous/new-api
Length of output: 114
Line 222 unnecessarily normalizes errors that will be normalized again in the defer block.
In the relay handler path (line 216 success/failure that passes shouldRetry check), NormalizeViolationFeeError is called at line 222, then again at line 171 in the defer block. While the function is idempotent by design—IsViolationFeeCode uses a prefix check that handles already-wrapped errors, and HasCSAMViolationMarker persists in the message—the redundant call is unnecessary. Either normalize once in the defer (removing line 222) for all error paths, or skip the defer normalization if already normalized in the loop.
🤖 Prompt for AI Agents
In `@controller/relay.go` around lines 222 - 223, Remove the redundant early
normalization call to service.NormalizeViolationFeeError in the relay handler's
success/failure path and rely on the single normalize call in the defer block;
specifically delete the invocation of service.NormalizeViolationFeeError that
occurs after the shouldRetry check so errors are only normalized once in the
defer, leaving IsViolationFeeCode and HasCSAMViolationMarker logic unchanged.
| if err := PostConsumeQuota(relayInfo, feeQuota, 0, true); err != nil { | ||
| logger.LogError(ctx, fmt.Sprintf("failed to charge violation fee: %s", err.Error())) | ||
| return false | ||
| } | ||
|
|
||
| model.UpdateUserUsedQuotaAndRequestCount(relayInfo.UserId, feeQuota) | ||
| model.UpdateChannelUsedQuota(relayInfo.ChannelId, feeQuota) |
There was a problem hiding this comment.
Quota updates lack atomicity.
PostConsumeQuota, UpdateUserUsedQuotaAndRequestCount, and UpdateChannelUsedQuota are called sequentially without a transaction. If any call fails after another succeeds, quota data could become inconsistent.
Consider wrapping these operations in a transaction or implementing compensation logic if a later operation fails.
🤖 Prompt for AI Agents
In `@service/violation_fee.go` around lines 125 - 131, The sequential calls to
PostConsumeQuota, model.UpdateUserUsedQuotaAndRequestCount, and
model.UpdateChannelUsedQuota can leave quotas inconsistent if one succeeds and a
later one fails; wrap these operations in a single atomic transaction (or
implement explicit compensation/rollback) so that either all three succeed or
all are reverted, e.g., begin a DB transaction around PostConsumeQuota and the
two model updates, check errors on each call, rollback the transaction and log
the error if any step fails, and commit only after all succeed; ensure
transaction handling is added around PostConsumeQuota,
UpdateUserUsedQuotaAndRequestCount, and UpdateChannelUsedQuota and that errors
propagate to calling code.
# Conflicts: # web/src/components/settings/ModelSetting.jsx
* feat: grok Usage Guidelines Violation Fee ui setting * feat: grok Usage Guidelines Violation Fee consume log * fix: grok Usage Guidelines Violation Fee log detail
2 participants
Summary by CodeRabbit
New Features
Bug Fixes / Improvements
✏️ Tip: You can customize this high-level summary in your review settings.