feat(codex): Sync and display credit balances - #823
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 📝 WalkthroughWalkthrough本次变更为 Codex 凭据点数增加观测、快照保存和前端展示。后端从配额数据、HTTP 响应头及 WebSocket 事件提取点数，并将其纳入被动观测和快照合并。classic 与 modern 前端解析点数摘要，并显示余额或无限额度状态。 Priority: ➖ Normal Merge Risk: 🔵 Low · up to Some later-arriving credit updates can be missed, leaving an older balance displayed. The impact is bounded, but the timestamp handling should be corrected or explicitly accepted before merging. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Credit balances remain behind existing credential access controls. No new privilege or unauthenticated access path was identified. Risk is low, with remaining uncertainty around observation ordering and recovery after process interruption. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 28.57% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 21 functions across 23 files. (2 skipped: 2 unsupported.)
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.
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (1)
internal/subscription/passive_quota_flush.go-151-168 (1)
151-168: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win在刷新边界按点数时间独立合并。
flushOnePassiveQuotaObservationLocked会在observation.ObservedAtMS <= row.ObservedAtMS时直接丢弃整份样本。mergePassiveQuotaSamples还会再次按快照总时间跳过样本，并把点数时间与总时间比较。这样，窗口观测时间较新但没有点数，或已有窗口使总时间较新时，后到的点数样本即使拥有更新的CreditSummary.ObservedAtMS也不会写入。最终snapshot_json.credits保持缺失或旧余额。请在刷新边界分别校验窗口时间和点数时间，并在点数合并中与已有
credits.observed_at_ms比较。点数单独合并时必须保留较新的快照总时间。Suggested fix
@@ - if row.ObservedAtMS != nil && observation.ObservedAtMS <= *row.ObservedAtMS { - // An observation at least as new -- typically a manual refresh -- - // was persisted while this sample sat pending. Writing it now would - // rewind observed_at_ms and overwrite newer quota values with older - // ones. The tie is included on purpose: a passive header captured - // just before an active refresh that completed within the same - // millisecond truncates to the same value, and the active result is - // the authoritative one. The CAS below only catches writes that land - // after this read. - manager.passiveQuota.ack(observation.CredentialID, observation.Version) - return nil - } merge, observedAtMS, mergeErr := mergePassiveQuotaSamples(row.SnapshotJSON, row.ObservedAtMS, observation) @@ - var observedAtMS int64 + var observedAtMS int64 + if storedAtMS != nil { + observedAtMS = *storedAtMS + } samples := [2]*PassiveQuotaSample{ @@ - if sample == nil || (storedAtMS != nil && sample.ObservedAtMS <= *storedAtMS) { + if sample == nil { continue } + windows := sample.Windows + if storedAtMS != nil && sample.ObservedAtMS <= *storedAtMS { + windows = nil + } credits := sample.Credits - if credits != nil && credits.ObservedAtMS != nil && storedAtMS != nil && *credits.ObservedAtMS <= *storedAtMS { - credits = nil + if len(windows) == 0 && credits == nil { + continue } - merged, err := mergePassiveQuotaSnapshot(result.Encoded, sample.Windows, credits) + merged, err := mergePassiveQuotaSnapshot(result.Encoded, windows, credits) @@ - if merged.Matched { + if merged.Matched && sample.ObservedAtMS > observedAtMS { observedAtMS = sample.ObservedAtMS } @@ - if patch.Balance != "" { - next.Balance = patch.Balance - next.ObservedAtMS = cloneInt64(patch.ObservedAtMS) - } - if patch.HasCredits != nil { - next.HasCredits = patch.HasCredits - } - if patch.Unlimited != nil { - next.Unlimited = patch.Unlimited - if *patch.Unlimited { + newer := previous == nil || previous.ObservedAtMS == nil || + patch.ObservedAtMS == nil || *patch.ObservedAtMS > *previous.ObservedAtMS + if newer { + if patch.Balance != "" { + next.Balance = patch.Balance next.ObservedAtMS = cloneInt64(patch.ObservedAtMS) } + if patch.HasCredits != nil { + next.HasCredits = patch.HasCredits + } + if patch.Unlimited != nil { + next.Unlimited = patch.Unlimited + if *patch.Unlimited { + next.ObservedAtMS = cloneInt64(patch.ObservedAtMS) + } + } + outcome.Matched = true } - outcome.Matched = true
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Organization UI
- Review profile: QUIET
- Plan: Advanced
- Run ID:
84bc6a09-9e69-4789-b920-e3b789ad874f
📒 Files selected for processing (25)
internal/control/classic_api_contract_test.gointernal/control/credential_credits_test.gointernal/control/credential_observations.gointernal/execution/cpa/adapter.gointernal/execution/cpa/adapter_test.gointernal/execution/cpa/codex_provider.gointernal/execution/cpa/credits_test.gointernal/execution/cpa/provider.gointernal/execution/cpa/websocket.gointernal/subscription/passive_credits_test.gointernal/subscription/passive_quota.gointernal/subscription/passive_quota_flush.gointernal/subscription/providers/codex/credits.gointernal/subscription/providers/codex/credits_test.gointernal/subscription/providers/codex/observation.gointernal/subscription/providers/observation/snapshot.goweb/src/frontends/classic/api/control/types.tsweb/src/frontends/classic/app/resources/credentials.tsweb/src/frontends/classic/features/groups/credentials/SubscriptionAccountCard.vueweb/src/frontends/classic/i18n/locales/en-US/group.tsweb/src/frontends/classic/i18n/locales/ja-JP/group.tsweb/src/frontends/classic/i18n/locales/zh-CN/group.tsweb/src/frontends/modern/api/credential-observation.tsweb/src/frontends/modern/features/groups/SubscriptionCredentialCard.vueweb/src/frontends/modern/i18n/locales/credential-cards.ts
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
|
Addressed the credit ordering issue from the review and the discussion in adbc0c7. Pending accumulation and persistence now compare credits against their own observation time, retain newer windows, and keep the overall snapshot time monotonic. Active refreshes without credit data retain only the credit timestamp, preventing earlier passive responses from restoring a cleared balance. Existing identity and CAS protections remain in place. Regression tests cover late credits before and after a window flush, missing and zero balances, active refreshes, timestamp ties, and legacy snapshots. For the docstring advisory: This repository does not enforce an 80% docstring threshold; the new exported credit parsers are documented. |
关联 Issue / Related Issue
Closes #822
变更内容 / Change Content
Read Codex credit balances from the existing account quota response and refresh them passively from HTTP response headers, for streaming and non-streaming requests, and WebSocket
codex.rate_limitsevents. Credit-only observations also update the stored balance without requiring a quota window.Persist zero balances while hiding zero and missing balances in the UI. Show positive balances or unlimited credits beside the plan badge in the modern frontend and beside reset credits in the classic frontend. Credit balances remain independent from quota windows and reset credits; values from different observation sources are never added together.
Preserve credit observation timestamps when later responses omit credit data, and retain the existing identity and concurrency checks against stale writes. Existing snapshots remain compatible through optional JSON fields; no database migration, SDK upgrade, or additional upstream request is required.
Validation:
make checkpassed. Regression coverage includes legacy snapshots, missing and zero balances, credit-only HTTP and WebSocket observations, empty quota arrays, and stale observation timestamps. Live upstream requests confirmed that HTTP headers and WebSocket quota events provide credit balances.自查清单 / Checklist
make check，或在说明中写明无法运行的原因和未验证范围。 / I ranmake check, or documented why it could not run and what remains unverified.