Skip to content

fix: reverse used quota on async task refunds (#4211)#6091

Open
cat0825 wants to merge 2 commits into
QuantumNous:mainfrom
cat0825:codex/fix-4211-task-refund-used-quota
Open

fix: reverse used quota on async task refunds (#4211)#6091
cat0825 wants to merge 2 commits into
QuantumNous:mainfrom
cat0825:codex/fix-4211-task-refund-used-quota

Conversation

@cat0825

@cat0825 cat0825 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

⚠️ 提交说明 / PR Notice

Important

  • 请提供人工撰写的简洁摘要,避免直接粘贴未经整理的 AI 输出。

📝 变更描述 / Description

异步任务失败退款和负差额结算现在会同步回退用户/渠道 used_quota。此前任务预扣会增加 used_quota,但退款只返还余额,导致“剩余额度 + 已用额度”虚增。新增 DecreaseUserUsedQuota / DecreaseChannelUsedQuota,并对负向调整做 0 下限保护。

This PR was AI-assisted and manually reviewed before submission.

🚀 变更类型 / Type of change

  • 🐛 Bug 修复 (Bug fix) - 请关联对应 Issue,避免将设计取舍、理解偏差或预期不一致直接归类为 bug
  • ✨ 新功能 (New feature) - 重大特性建议先通过 Issue 沟通
  • ⚡ 性能优化 / 重构 (Refactor)
  • 📝 文档更新 (Documentation)

🔗 关联任务 / Related Issue

✅ 提交前检查项 / Checklist

  • 人工确认: 我已亲自整理并撰写此描述,没有直接粘贴未经处理的 AI 输出。
  • 非重复提交: 我已搜索现有的 IssuesPRs,确认不是重复提交。
  • Bug fix 说明: 若此 PR 标记为 Bug fix,我已提交或关联对应 Issue,且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。
  • 变更理解: 我已理解这些更改的工作原理及可能影响。
  • 范围聚焦: 本 PR 未包含任何与当前任务无关的代码改动。
  • 本地验证: 已在本地运行并通过测试或手动验证,维护者可以据此复核结果。
  • 安全合规: 代码中无敏感凭据,且符合项目代码规范。

📸 运行证明 / Proof of Work

docker run --rm -v "$PWD":/work -w /work -e GOMODCACHE=/gomod -e GOCACHE=/gocache golang:1.25.1 go test ./service -run 'TestRefundTaskQuota|TestRecalculate_NegativeDelta'
ok  	github.com/QuantumNous/new-api/service	0.032s

Summary by CodeRabbit

  • Bug Fixes
    • Corrected quota refund and billing recalculation so user and channel used quota totals decrease by the refunded/rolled-back amount.
    • Ensured used quota never drops below zero, including during refund/negative-delta adjustments and quota updates.
  • Tests
    • Added assertions and helpers to verify that refund and recalculation flows update user and channel usage totals as expected.

@coderabbitai

coderabbitai Bot commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9c83000e-5ac7-42ac-a740-b4f2efe7aa49

📥 Commits

Reviewing files that changed from the base of the PR and between 9c92d5c and fb4de27.

📒 Files selected for processing (1)
  • model/channel.go

Walkthrough

Refund and negative recalculation flows now decrease user and channel used_quota values with zero-floor clamping. Billing tests verify restored quota and usage totals.

Changes

Quota refund accounting

Layer / File(s) Summary
Quota model updates
model/channel.go, model/user.go
Adds validated used-quota decrement helpers, supports batched deltas, and clamps refunds at zero.
Billing refund wiring
service/task_billing.go
Refund and negative recalculation paths decrement user and channel usage before recording refund results.
Billing quota tests
service/task_billing_test.go
Adds usage read-back helpers and verifies refund and recalculation usage totals.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant TaskBilling
  participant QuotaModels
  participant Database
  TaskBilling->>QuotaModels: decrease user and channel used_quota
  QuotaModels->>Database: apply clamped decrement
  Database-->>QuotaModels: updated usage totals
Loading

Possibly related PRs

Poem

I’m a bunny hopping through the bill,
Refunds now make used quotas still.
User and channel counters meet zero,
No extra carrots for the ledger hero.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: reversing used quota during async task refunds for issue #4211.
Linked Issues check ✅ Passed The changes update refund and negative-delta paths to reduce user and channel used_quota as required by #4211, with tests covering the behavior.
Out of Scope Changes check ✅ Passed The summarized diff stays focused on quota refund accounting and tests; no unrelated changes remain in scope.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

❤️ Share

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

Keep QuantumNous#4211 scoped to used-quota reversal only. Channel group column
widening belongs in QuantumNous#6017.
@cat0825

cat0825 commented Jul 10, 2026

Copy link
Copy Markdown
Contributor Author

Addressed CodeRabbit out-of-scope warning:

Commit: fb4de27

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@model/channel.go`:
- Line 40: Revert the `channels.Group` GORM width change in the `Channel` model
so this PR preserves the existing `varchar(1024)` schema; defer narrowing it to
a separate ticket or dedicated migration.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9c83000e-5ac7-42ac-a740-b4f2efe7aa49

📥 Commits

Reviewing files that changed from the base of the PR and between 9c92d5c and fb4de27.

📒 Files selected for processing (1)
  • model/channel.go

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

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 `@model/channel.go`:
- Line 40: Revert the `channels.Group` GORM width change in the `Channel` model
so this PR preserves the existing `varchar(1024)` schema; defer narrowing it to
a separate ticket or dedicated migration.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

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

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 9c83000e-5ac7-42ac-a740-b4f2efe7aa49

📥 Commits

Reviewing files that changed from the base of the PR and between 9c92d5c and fb4de27.

📒 Files selected for processing (1)
  • model/channel.go
🛑 Comments failed to post (1)
model/channel.go (1)

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Inspect the new/updated channel used-quota helpers and the Group field history
ast-grep outline model/channel.go --items all
sed -n '855,895p' model/channel.go
# Verify current base definition of the Group column tag
rg -nP 'Group\s+string\s+`json:"group"' model/channel.go

Repository: QuantumNous/new-api

Length of output: 7437


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the current schema/model references for channel group length and any migrations around it.
rg -n "varchar\\(1024\\)|varchar\\(64\\)|Group.*default:'default'|group length|channel group" .

Repository: QuantumNous/new-api

Length of output: 2867


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Show the exact diff for model/channel.go and any mentions of the group-size change in the repo.
git diff -- model/channel.go
printf '\n---\n'
rg -n "codex/fix-6017-channel-group-length|`#6017`|group length|varchar\\(1024\\)" .

Repository: QuantumNous/new-api

Length of output: 386


🌐 Web query:

GORM AutoMigrate shrink varchar column existing data truncate alter column size behavior

💡 Result:

When using GORM's AutoMigrate, the library is designed to automatically detect and apply changes to database columns, including size, if they differ from your struct definition [1][2]. Specifically, if you shrink the size of a varchar column in your struct (e.g., changing size:255 to size:100), GORM will issue an ALTER TABLE statement (typically using MODIFY COLUMN or CHANGE COLUMN depending on the database driver) to update the column definition [3]. Regarding the behavior of existing data, it is critical to understand the following: 1. No Automatic Truncation: GORM itself does not perform data truncation or validation logic to trim existing records that exceed the new, smaller size. It merely issues the DDL command to the database [4][3]. 2. Database-Level Enforcement: The actual behavior depends entirely on the underlying database engine (e.g., MySQL, PostgreSQL, SQLite). When you issue an ALTER TABLE command to shrink a varchar column: - Some databases (like strict MySQL configurations) may reject the operation if existing data exceeds the new size, resulting in an error. - Others may allow it but behave unpredictably, potentially causing truncation or data corruption at the database storage level. 3. Reliability Limitations: While GORM attempts to be "smart" about migrations, the logic for detecting when a column needs to be altered can sometimes be inconsistent [3]. Users have reported that simply updating the struct tag does not always trigger the necessary ALTER statement [5][6][7]. In some cases, explicitly defining other attributes or using a custom migration might be required if GORM fails to detect the change [8]. Recommendation: Because AutoMigrate is a convenience tool and not a replacement for robust database migration management, you should not rely on it to safely shrink columns containing production data. Before applying such a change, you should manually verify your existing data size, perform a backup, and consider using a dedicated migration tool (like golang-migrate or GORM's own Migrator interface with manual Execute calls) to perform the change safely, potentially involving a staging column or data cleanup to avoid errors during the schema update.

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check whether group values are validated/bounded elsewhere and whether any migration exists for this model field.
rg -n "group.{0,40}(len|length|varchar|64|1024)|Group.*64|Group.*1024|Validate.*Group|NormalizeChannelGroupFilter|ApplyChannelGroupFilter" model common dto internal .

Repository: QuantumNous/new-api

Length of output: 16153


Keep the channels.Group width change out of this PR model/channel.go:40
This narrows Group from varchar(1024) to varchar(64), which can truncate existing rows during migration. Move it to the separate group-length ticket or add a dedicated migration/backfill.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/channel.go` at line 40, Revert the `channels.Group` GORM width change
in the `Channel` model so this PR preserves the existing `varchar(1024)` schema;
defer narrowing it to a separate ticket or dedicated migration.

Source: Path instructions

@cat0825

cat0825 commented Jul 11, 2026

Copy link
Copy Markdown
Contributor Author

@Calcium-Ion @t0ng7u 麻烦有空时 review 一下这个 PR。

范围:仅 #4211 — 异步任务退款时同步回退 user/channel used quota。
此前误带的 channels.group 扩长已从本 PR 移除(单独在 #6111 / #6017)。谢谢。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug 报告:异步任务退款导致用户总额度异常增加

1 participant