Feat/lazy account mode#154
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds a persistent system setting "lazy_mode" and implements lazy-mode behavior across database migrations, auth store/scheduler logic (selection, refresh gating), admin settings API, and frontend UI and labels. ChangesLazy Mode Feature
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a “Lazy Mode” system setting that disables proactive refresh/probing behavior, and surfaces additional “usage updated” timestamps in the Accounts UI when Lazy Mode is enabled.
Changes:
- Introduces
lazy_modein settings across backend (store + admin API), DB migrations (Postgres/SQLite), and frontend types/UI. - Updates background refresh / probe behavior to no-op under Lazy Mode; adjusts account dispatch selection logic for Lazy Mode.
- Enhances Accounts table to show separate “record updated” vs “usage updated” times in Lazy Mode, with new i18n strings.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| main.go | Adds default LazyMode: false in startup settings initialization. |
| frontend/src/types.ts | Extends types for lazy_mode and codex_usage_updated_at. |
| frontend/src/pages/Settings.tsx | Adds Lazy Mode control and disables related controls when enabled. |
| frontend/src/pages/Accounts.tsx | Fetches settings to detect Lazy Mode and conditionally renders extra timestamps. |
| frontend/src/locales/en.json | Adds i18n strings for Lazy Mode and updated-at labels. |
| frontend/src/locales/zh.json | Adds i18n strings for Lazy Mode and updated-at labels. |
| database/sqlite.go | Adds lazy_mode column to system_settings and migration list. |
| database/postgres.go | Adds lazy_mode column, plumbs through SystemSettings read/write. |
| auth/store.go | Implements Lazy Mode flag, disables background jobs, adds lazy dispatch path. |
| admin/handler.go | Adds Lazy Mode to settings API; avoids async refresh on add/import when lazy. |
| admin/bootstrap.go | Adds default LazyMode: false to bootstrap settings. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <SettingField label={t('settings.autoCleanFullUsage')} description={t('settings.autoCleanFullUsageDesc')}> | ||
| <Select | ||
| value={settingsForm.auto_clean_full_usage ? 'true' : 'false'} | ||
| value={lazyModeActive ? 'false' : settingsForm.auto_clean_full_usage ? 'true' : 'false'} |
Comment on lines
696
to
+700
| type="number" | ||
| min={1} | ||
| max={1440} | ||
| value={settingsForm.background_refresh_interval_minutes} | ||
| value={lazyModeActive ? 0 : settingsForm.background_refresh_interval_minutes} | ||
| disabled={lazyModeActive} |
Comment on lines
+405
to
418
| const [ | ||
| accountsResponse, | ||
| apiKeysResponse, | ||
| opsOverview, | ||
| groupsResponse, | ||
| settings, | ||
| ] = | ||
| await Promise.all([ | ||
| api.getAccounts(), | ||
| api.getAPIKeys(), | ||
| api.getOpsOverview().catch((): OpsOverviewResponse | null => null), | ||
| api.listAccountGroups().catch(() => ({ groups: [] })), | ||
| api.getSettings().catch((): SystemSettings | null => null), | ||
| ]); |
| accounts: accountsResponse.accounts ?? [], | ||
| apiKeys: apiKeysResponse.keys ?? [], | ||
| opsOverview, | ||
| lazyMode: settings?.lazy_mode ?? false, |
Comment on lines
+2645
to
+2654
| if acc.NeedsRefresh() && hasRefreshCredential { | ||
| ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) | ||
| err := s.refreshAccount(ctx, acc) | ||
| cancel() | ||
| if err != nil { | ||
| log.Printf("[账号 %d] lazy mode 调度前刷新失败: %v", acc.DBID, err) | ||
| return false | ||
| } | ||
| } | ||
| return acc.IsAvailable() |
Comment on lines
4244
to
4250
| defer s.mu.RUnlock() | ||
| count := 0 | ||
| for _, acc := range s.accounts { | ||
| if acc.IsAvailable() { | ||
| if (s.GetLazyMode() && s.accountLazySelectable(acc)) || (!s.GetLazyMode() && acc.IsAvailable()) { | ||
| count++ | ||
| } | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
frontend/src/pages/Settings.tsx (1)
464-469:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPersist lazy-mode forced values when saving.
The UI forces
background_refresh_interval_minutesandauto_clean_full_usagein lazy mode, butapi.updateSettings(settingsForm)can still submit stale underlying values. That can save a config different from what users see.Suggested fix
const handleSaveSettings = async () => { setSavingSettings(true) try { const adminSecretChanged = settingsForm.admin_auth_source !== 'env' && settingsForm.admin_secret !== loadedAdminSecret - const updated = await api.updateSettings(settingsForm) + const payload = settingsForm.lazy_mode + ? { + ...settingsForm, + background_refresh_interval_minutes: 0, + auto_clean_full_usage: false, + } + : settingsForm + const updated = await api.updateSettings(payload) setSettingsForm(updated)Also applies to: 699-701, 904-907
🤖 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 `@frontend/src/pages/Settings.tsx` around lines 464 - 469, The save path can submit stale values for fields that the UI forces in lazy mode (background_refresh_interval_minutes and auto_clean_full_usage), so before calling api.updateSettings from handlers like handleSaveSettings (and the similar save handlers around the other save flows), ensure you overwrite those keys on the payload with the enforced UI values when lazy mode is active; i.e., compute the effective payload from settingsForm, then if lazyMode/isLazyModeEnabled (or the UI condition that forces values) set payload.background_refresh_interval_minutes = <forced value> and payload.auto_clean_full_usage = <forced value> and then call api.updateSettings(payload), then setSettingsForm(updated).
🧹 Nitpick comments (1)
frontend/src/pages/Accounts.tsx (1)
404-418: ⚡ Quick winAvoid fetching settings on every silent accounts reload.
loadAccountsnow callsapi.getSettings()on each reload, including polling-drivenreloadSilently(). Consider cachinglazy_mode(or sourcing it from accounts payload) to avoid repeated settings calls on hot refresh paths.Also applies to: 515-520, 721-726
🤖 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 `@frontend/src/pages/Accounts.tsx` around lines 404 - 418, loadAccounts currently calls api.getSettings() on every invocation (including polling-driven reloadSilently), causing unnecessary requests; modify loadAccounts (and the other callers like reloadSilently) to avoid fetching settings on silent reloads by caching the needed setting (lazy_mode) in component state (e.g., lazyMode) or sourcing it from the accounts payload, and only call api.getSettings() when the cache is empty or when a full/manual refresh is requested; update places that call api.getSettings() (the loadAccounts implementation and the other load paths referenced) to read from the cached lazyMode state instead of always calling api.getSettings().
🤖 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 `@frontend/src/pages/Settings.tsx`:
- Around line 464-469: The save path can submit stale values for fields that the
UI forces in lazy mode (background_refresh_interval_minutes and
auto_clean_full_usage), so before calling api.updateSettings from handlers like
handleSaveSettings (and the similar save handlers around the other save flows),
ensure you overwrite those keys on the payload with the enforced UI values when
lazy mode is active; i.e., compute the effective payload from settingsForm, then
if lazyMode/isLazyModeEnabled (or the UI condition that forces values) set
payload.background_refresh_interval_minutes = <forced value> and
payload.auto_clean_full_usage = <forced value> and then call
api.updateSettings(payload), then setSettingsForm(updated).
---
Nitpick comments:
In `@frontend/src/pages/Accounts.tsx`:
- Around line 404-418: loadAccounts currently calls api.getSettings() on every
invocation (including polling-driven reloadSilently), causing unnecessary
requests; modify loadAccounts (and the other callers like reloadSilently) to
avoid fetching settings on silent reloads by caching the needed setting
(lazy_mode) in component state (e.g., lazyMode) or sourcing it from the accounts
payload, and only call api.getSettings() when the cache is empty or when a
full/manual refresh is requested; update places that call api.getSettings() (the
loadAccounts implementation and the other load paths referenced) to read from
the cached lazyMode state instead of always calling api.getSettings().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0832fc8c-a34e-4e57-8125-e9a732a4468d
📒 Files selected for processing (11)
admin/bootstrap.goadmin/handler.goauth/store.godatabase/postgres.godatabase/sqlite.gofrontend/src/locales/en.jsonfrontend/src/locales/zh.jsonfrontend/src/pages/Accounts.tsxfrontend/src/pages/Settings.tsxfrontend/src/types.tsmain.go
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@auth/store.go`:
- Around line 2637-2642: The helper currently schedules a background refresh via
s.triggerLazyRefreshAsync(acc) when s.lazyNeedsDispatchRefresh(acc) is true but
then returns false, causing callers (e.g., NextExcludingWithFilter* and dispatch
code) to drop an otherwise still-valid access token; change the helper so that
after calling s.triggerLazyRefreshAsync(acc) it does NOT return false
immediately but instead returns acc.IsAvailable() (i.e., keep using the token
while refresh runs), making sure you only treat the account as unavailable when
it actually reports unavailable/expired and not just when
lazyNeedsDispatchRefresh flips ~5 minutes before expiry.
- Around line 2726-2729: The loop calling s.lazyNeedsDispatchRefresh(acc) then
s.triggerLazyRefreshAsync(acc) is redundant and causes many candidates to be
dispatched during a full account scan; remove this trigger so only the candidate
selected by acquireLazyCandidate() (which already goes through
ensureLazyDispatchReady()) is refreshed. Locate the code that calls
s.lazyNeedsDispatchRefresh(acc) and s.triggerLazyRefreshAsync(acc) inside the
full scan and delete that conditional block (or guard it with a check that the
account equals the one returned by acquireLazyCandidate()), leaving lazy
dispatch responsibility solely to acquireLazyCandidate() /
ensureLazyDispatchReady().
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1b3dc038-425c-450d-bd80-86357778b27b
📒 Files selected for processing (4)
auth/store.gofrontend/src/hooks/useDataLoader.tsfrontend/src/pages/Accounts.tsxfrontend/src/pages/Settings.tsx
Comment on lines
+2637
to
+2642
| if s.lazyNeedsDispatchRefresh(acc) { | ||
| s.triggerLazyRefreshAsync(acc) | ||
| return false | ||
| } | ||
| return acc.IsAvailable() | ||
| } |
There was a problem hiding this comment.
Keep using a still-valid access token while refresh runs.
NeedsRefresh() flips 5 minutes before expiry, but this helper now returns false immediately after scheduling the background refresh. That makes lazy mode drop otherwise usable accounts for that whole window, and direct NextExcludingWithFilter* callers can get nil even though dispatch could still succeed.
Suggested fix
func (s *Store) ensureLazyDispatchReady(acc *Account) bool {
if acc == nil {
return false
}
if s.lazyNeedsDispatchRefresh(acc) {
s.triggerLazyRefreshAsync(acc)
- return false
+ if acc.GetAccessToken() == "" {
+ return false
+ }
}
return acc.IsAvailable()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if s.lazyNeedsDispatchRefresh(acc) { | |
| s.triggerLazyRefreshAsync(acc) | |
| return false | |
| } | |
| return acc.IsAvailable() | |
| } | |
| if s.lazyNeedsDispatchRefresh(acc) { | |
| s.triggerLazyRefreshAsync(acc) | |
| if acc.GetAccessToken() == "" { | |
| return false | |
| } | |
| } | |
| return acc.IsAvailable() | |
| } |
🤖 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 `@auth/store.go` around lines 2637 - 2642, The helper currently schedules a
background refresh via s.triggerLazyRefreshAsync(acc) when
s.lazyNeedsDispatchRefresh(acc) is true but then returns false, causing callers
(e.g., NextExcludingWithFilter* and dispatch code) to drop an otherwise
still-valid access token; change the helper so that after calling
s.triggerLazyRefreshAsync(acc) it does NOT return false immediately but instead
returns acc.IsAvailable() (i.e., keep using the token while refresh runs),
making sure you only treat the account as unavailable when it actually reports
unavailable/expired and not just when lazyNeedsDispatchRefresh flips ~5 minutes
before expiry.
Comment on lines
+2726
to
+2729
| if s.lazyNeedsDispatchRefresh(acc) { | ||
| s.triggerLazyRefreshAsync(acc) | ||
| continue | ||
| } |
There was a problem hiding this comment.
Only refresh the chosen lazy-mode candidate.
This runs inside the full account scan, so a single request can kick off refreshes for every near-expiry candidate in the pool. That defeats the passive/lazy goal and creates unnecessary upstream bursts. acquireLazyCandidate() already sends the selected account through ensureLazyDispatchReady(), so this trigger is redundant there.
Suggested fix
- if s.lazyNeedsDispatchRefresh(acc) {
- s.triggerLazyRefreshAsync(acc)
- continue
- }
-
load := atomic.LoadInt64(&acc.ActiveRequests)
tier, _, dispatchScore, limit := acc.schedulerSnapshot(maxConcurrency)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if s.lazyNeedsDispatchRefresh(acc) { | |
| s.triggerLazyRefreshAsync(acc) | |
| continue | |
| } | |
| load := atomic.LoadInt64(&acc.ActiveRequests) | |
| tier, _, dispatchScore, limit := acc.schedulerSnapshot(maxConcurrency) |
🤖 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 `@auth/store.go` around lines 2726 - 2729, The loop calling
s.lazyNeedsDispatchRefresh(acc) then s.triggerLazyRefreshAsync(acc) is redundant
and causes many candidates to be dispatched during a full account scan; remove
this trigger so only the candidate selected by acquireLazyCandidate() (which
already goes through ensureLazyDispatchReady()) is refreshed. Locate the code
that calls s.lazyNeedsDispatchRefresh(acc) and s.triggerLazyRefreshAsync(acc)
inside the full scan and delete that conditional block (or guard it with a check
that the account equals the one returned by acquireLazyCandidate()), leaving
lazy dispatch responsibility solely to acquireLazyCandidate() /
ensureLazyDispatchReady().
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
新增惰性模式，只被动获取账号信息，或许可以降低封禁概率？
Summary by CodeRabbit