Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR updates configuration defaults, makes streaming timeout handling nullable, adds fallback billing for abnormal stream endings, converts log retry marker backfill to async execution, and adjusts retry-filter and quota aggregation behavior. ChangesConfiguration Default Constants
Nullable Streaming Timeout
Stream Fallback Quota Billing
Async Log Retry Marker Backfill and
Sequence Diagram(s)sequenceDiagram
participant Client
participant PostTextConsumeQuota
participant streamFallbackQuota
participant Database
Client->>PostTextConsumeQuota: stream ends with zero tokens
PostTextConsumeQuota->>streamFallbackQuota: check end reason and relay info
streamFallbackQuota-->>PostTextConsumeQuota: fallback quota and applied flag
PostTextConsumeQuota->>Database: update user/channel usage stats
PostTextConsumeQuota->>Database: insert consume log with fallback quota
sequenceDiagram
participant migrateLOGDB
participant scheduleLogRetryMarkerBackfill
participant isLogRetryMarkerBackfillCompleted
participant gopool
participant backfillLogRetryMarker
migrateLOGDB->>scheduleLogRetryMarkerBackfill: schedule after DB migration
scheduleLogRetryMarkerBackfill->>isLogRetryMarkerBackfillCompleted: check completion marker
isLogRetryMarkerBackfillCompleted-->>scheduleLogRetryMarkerBackfill: incomplete
scheduleLogRetryMarkerBackfill->>gopool: Go(backfillLogRetryMarker)
gopool->>backfillLogRetryMarker: run under logRetryMarkerBackfillMu
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 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 `@constant/env.go`:
- Around line 16-33: Promote the boolean environment defaults in the env
constants block to dedicated Default* constants so there is a single source of
truth for these values. Update the env-backed variables in constant/env.go to
use those new Default* symbols, then change common/init.go to reference the same
Default* constants instead of raw true/false literals. Keep the affected symbols
consistent across both files, including DifyDebug, ForceStreamOption,
CountToken, GetMediaToken, GetMediaTokenNotStream, UpdateTask,
GenerateDefaultToken, and ErrorLogEnabled.
In `@model/log.go`:
- Around line 103-114: The retry-marker readiness helper currently only logs
failures, so read paths can still run with an incomplete retry-marker state.
Update ensureLogRetryMarkerBackfillCompletedForRead to return an error instead
of swallowing failures from isLogRetryMarkerBackfillCompleted and
backfillLogRetryMarker, then thread that error through applyRetryLogFilter and
the retry-filtered read flow so the query is aborted before using logs.is_retry
= true when readiness cannot be confirmed.
In `@service/text_quota.go`:
- Around line 393-397: The fallback path in text quota handling is missing the
nil-upstream-usage case, so `streamFallbackQuota` in `service/text_quota.go`
should also trigger when `usage == nil` and the stream ended abnormally even if
`summary.Quota` was populated from estimated prompt tokens. Update the logic
around `streamFallbackQuota`, `summary.Quota`, and the abnormal-stream handling
in `RelayInfo`/`StreamStatus` so the no-upstream-usage path uses
`FinalPreConsumedQuota` instead of settling the estimated quota, while keeping
the existing logging and `shouldUpdateUsageStats` 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: 6c3b94d6-81de-47ba-b6ff-a733ffd090d4
📒 Files selected for processing (14)
common/init.goconstant/env.goconstant/env_test.gomodel/log.gomodel/log_test.gomodel/main.gorelay/channel/gemini/relay_gemini_usage_test.gorelay/channel/openai/empty_completion_test.gorelay/channel/openai/image_stream_test.gorelay/helper/stream_scanner.gorelay/helper/stream_scanner_test.goservice/quota.goservice/text_quota.goservice/text_quota_test.go
💤 Files with no reviewable changes (3)
- relay/channel/openai/image_stream_test.go
- relay/channel/gemini/relay_gemini_usage_test.go
- relay/channel/openai/empty_completion_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly callingencoding/json;encoding/jsontypes likejson.RawMessageandjson.Numbermay still be used as types.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ simultaneously: prefer GORM methods over raw SQL, avoid directAUTO_INCREMENT/SERIAL, use cross-DB quoting/boolean helpers when raw SQL is unavoidable, avoid DB-specific functions/operators without fallbacks, and ensure migrations work on all three databases.
For request structs parsed from client JSON and later re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved and omitted-only fields remainnil.
**/*.go: Usecommon.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson, andcommon.GetJsonTypefromcommon/json.gofor all JSON marshal/unmarshal operations; do not directly callencoding/jsonin business code.
Keep all database code fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless cross-DB-safe, and use the provided DB-specific helpers/flags for quoting, booleans, and branching.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, use pointer types withomitemptyfor optional scalar fields so explicit zero/false values are preserved.
Files:
constant/env_test.gocommon/init.gorelay/helper/stream_scanner.goservice/text_quota.goservice/text_quota_test.goservice/quota.gomodel/log.goconstant/env.gomodel/main.gorelay/helper/stream_scanner_test.gomodel/log_test.go
🔇 Additional comments (9)
relay/helper/stream_scanner.go (1)
43-50: LGTM!Also applies to: 69-71, 103-105, 247-249, 293-293
relay/helper/stream_scanner_test.go (1)
95-100: LGTM!Also applies to: 626-654
service/text_quota.go (1)
322-344: LGTM!service/quota.go (1)
217-232: LGTM!Also applies to: 346-361
service/text_quota_test.go (1)
10-10: LGTM!Also applies to: 72-178
constant/env_test.go (1)
5-31: LGTM!model/main.go (1)
15-15: LGTM!Also applies to: 32-37, 325-325, 407-434
model/log.go (1)
721-725: LGTM!Also applies to: 742-748
model/log_test.go (1)
36-38: LGTM!Also applies to: 51-54, 72-89, 139-220, 349-430, 569-582
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 (2)
model/log_test.go (1)
388-423: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winSeed the legacy retry row before
migrateLOGDB().This test is meant to prove migration schedules backfill without running it inline, but the retry log is inserted after Line 402. If
migrateLOGDB()accidentally ran the backfill synchronously, the test would still pass because there was nothing to update yet.Proposed test tightening
- logDB := newRetryBackfillTestDB(t) + logDB := newRetryBackfillTestDB(t, &Log{}) @@ - require.NoError(t, migrateLOGDB()) - require.Len(t, scheduled, 1) - log := Log{ UserId: 1, Type: LogTypeConsume, @@ require.NoError(t, LOG_DB.Create(&log).Error) require.NoError(t, LOG_DB.Model(&Log{}).Where("1 = 1").UpdateColumn("is_retry", false).Error) + + require.NoError(t, migrateLOGDB()) + require.Len(t, scheduled, 1) var reloaded Log🤖 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_test.go` around lines 388 - 423, The async backfill test is too weak because the retry row is created after migrateLOGDB(), so it would still pass even if the backfill ran synchronously. Move the legacy retry Log seeding and the reset of is_retry to before migrateLOGDB() in TestRetryMarkerBackfillAsync (the block using LOG_DB.Create, UpdateColumn, and migrateLOGDB), then keep the assertions that scheduled contains exactly one callback and that the row remains false until scheduled[0]() runs. This ensures the test verifies that migrateLOGDB only schedules the backfill and does not execute it inline.service/text_quota.go (1)
388-403: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winGate usage-stat updates on billable quota, not just token count.
shouldUpdateUsageStats := summary.TotalTokens != 0drops user/channel quota updates for zero-token but still-billable requests in this path, such as fixed-price web/file search or image-generation charges already reflected insummary.Quota.SettleBillingandRecordConsumeLogstill run, so persisted usage stats diverge from the actual billed quota.Suggested fix
- shouldUpdateUsageStats := summary.TotalTokens != 0 - if summary.TotalTokens == 0 { + shouldUpdateUsageStats := summary.TotalTokens != 0 || summary.Quota != 0 + if summary.TotalTokens == 0 && summary.Quota == 0 { extraContent = append(extraContent, "上游没有返回计费信息，无法扣费（可能是上游超时）") logger.LogError(ctx, fmt.Sprintf("total tokens is 0, cannot consume quota, userId %d, channelId %d, tokenId %d, model %s， pre-consumed quota %d", relayInfo.UserId, relayInfo.ChannelId, relayInfo.TokenId, summary.ModelName, relayInfo.FinalPreConsumedQuota)) }🤖 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/text_quota.go` around lines 388 - 403, The usage-stats update gate in the billing flow is tied only to summary.TotalTokens, which skips quota/stat updates for zero-token but still billable requests. Update the logic around shouldUpdateUsageStats in the settlement path so it also considers billed quota from summary.Quota (including cases like streamFallbackQuota, fixed-price search, or image charges), and ensure SettleBilling/RecordConsumeLog still align with the quota that was actually consumed.
🤖 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/log_test.go`:
- Around line 388-423: The async backfill test is too weak because the retry row
is created after migrateLOGDB(), so it would still pass even if the backfill ran
synchronously. Move the legacy retry Log seeding and the reset of is_retry to
before migrateLOGDB() in TestRetryMarkerBackfillAsync (the block using
LOG_DB.Create, UpdateColumn, and migrateLOGDB), then keep the assertions that
scheduled contains exactly one callback and that the row remains false until
scheduled[0]() runs. This ensures the test verifies that migrateLOGDB only
schedules the backfill and does not execute it inline.
In `@service/text_quota.go`:
- Around line 388-403: The usage-stats update gate in the billing flow is tied
only to summary.TotalTokens, which skips quota/stat updates for zero-token but
still billable requests. Update the logic around shouldUpdateUsageStats in the
settlement path so it also considers billed quota from summary.Quota (including
cases like streamFallbackQuota, fixed-price search, or image charges), and
ensure SettleBilling/RecordConsumeLog still align with the quota that was
actually consumed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a8a53db7-8ac6-40b7-9e5f-46fe77e238ca
📒 Files selected for processing (7)
common/init.goconstant/env.goconstant/env_test.gomodel/log.gomodel/log_test.goservice/text_quota.goservice/text_quota_test.go
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
- GitHub Check: Backend checks
🧰 Additional context used
📓 Path-based instructions (1)
**/*.go
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.go: All JSON marshal/unmarshal operations in Go business code must use the wrappers incommon/json.go(common.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson,common.GetJsonType) instead of directly callingencoding/json;encoding/jsontypes likejson.RawMessageandjson.Numbermay still be used as types.
All database code must be compatible with SQLite, MySQL 5.7.8+, and PostgreSQL 9.6+ simultaneously: prefer GORM methods over raw SQL, avoid directAUTO_INCREMENT/SERIAL, use cross-DB quoting/boolean helpers when raw SQL is unavoidable, avoid DB-specific functions/operators without fallbacks, and ensure migrations work on all three databases.
For request structs parsed from client JSON and later re-marshaled to upstream providers, especially in relay/convert paths, optional scalar fields must use pointer types withomitemptyso explicit zero/false values are preserved and omitted-only fields remainnil.
**/*.go: Usecommon.Marshal,common.Unmarshal,common.UnmarshalJsonStr,common.DecodeJson, andcommon.GetJsonTypefromcommon/json.gofor all JSON marshal/unmarshal operations; do not directly callencoding/jsonin business code.
Keep all database code fully compatible with SQLite, MySQL >= 5.7.8, and PostgreSQL >= 9.6; prefer GORM abstractions, avoid raw SQL unless cross-DB-safe, and use the provided DB-specific helpers/flags for quoting, booleans, and branching.
For request structs parsed from client JSON and re-marshaled to upstream providers, especially in relay/convert paths, use pointer types withomitemptyfor optional scalar fields so explicit zero/false values are preserved.
Files:
common/init.goconstant/env_test.goconstant/env.goservice/text_quota_test.goservice/text_quota.gomodel/log.gomodel/log_test.go
🔇 Additional comments (5)
constant/env.go (1)
14-21: LGTM!Also applies to: 24-39
common/init.go (1)
150-168: LGTM!constant/env_test.go (1)
8-22: LGTM!model/log.go (1)
98-126: LGTM!Also applies to: 544-547, 632-635, 716-723, 737-741, 758-764
model/log_test.go (1)
4-4: LGTM!Also applies to: 140-247, 426-457, 504-520
1 participant
Important
📝 变更描述 / Description
本次修复了几处后端边界问题：
InitEnv执行前仍为零值，导致流式超时、文件大小限制、任务轮询等配置行为不一致。SumUsedQuota能正确应用时间范围、日志类型和重试过滤，避免 RPM/TPM 与额度统计口径不一致。STREAMING_TIMEOUT=0时会创建非法 ticker 的问题，允许显式关闭流式超时。这些改动通过在写入日志时同步
is_retry、读取前检查回填完成状态、统一统计查询条件，以及基于StreamStatus判断异常流式结束来生效。🚀 变更类型 / Type of change
🔗 关联任务 / Related Issue
✅ 提交前检查项 / Checklist
Bug fix，我已提交或关联对应 Issue，且不会将设计取舍、预期不一致或理解偏差直接归类为 bug。📸 运行证明 / Proof of Work
本地验证结果：