Fix usage meters and iOS composer send freeze#806
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
📝 WalkthroughWalkthroughChangesDesktop usage quotas
iOS composer focus scheduling
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
@copilot review but do not make fixes |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift (1)
311-355: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider skipping Task creation when nothing needs to change.
The coalescing shortcut at Line 324 only reuses
pendingTaskwhen a request with the sameisFocusedarrives while a task is already pending. Once that task completes (pendingTaskbecomesnil), any subsequent call with the sameisFocusedvalue falls through to spawn a brand-newTask+Task.yield()even though the responder already matches the desired state — this happens on everyupdateUIViewcall while the composer stays focused (i.e. on effectively every keystroke), sinceapplyFocusRequestis called unconditionally fromupdateUIView(Line 416, Line 590).This doesn't cause incorrect behavior (the task body's own guards make it a no-op), but it's avoidable churn on a hot path.
♻️ Proposed early-exit to avoid redundant Task creation
func apply(_ isFocused: Bool, to textView: UITextView) -> Task<Void, Never>? { // SwiftUI may update the representable more than once for the same state. // Keep the queued transition instead of canceling it without a replacement. if lastRequest == isFocused, pendingTask != nil { return pendingTask } let previousRequest = lastRequest lastRequest = isFocused + + // No transition in flight and the responder already matches the + // request: skip spawning a Task on every re-render (e.g. per keystroke) + // when nothing actually needs to change. + if pendingTask == nil, textView.isFirstResponder == isFocused { + return nil + } + pendingTask?.cancel() pendingTask = nil🤖 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 `@apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift` around lines 311 - 355, Update WorkComposerFocusScheduler.apply to return early when the requested focus state already matches the textView’s current responder state and no transition is pending, avoiding a new Task and Task.yield on repeated updateUIView calls. Preserve queued-task coalescing and the existing initial-unfocused behavior, and only schedule a task when the responder state still needs to change.
🤖 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 `@apps/desktop/src/main/services/usage/providerQuotaParsers.ts`:
- Around line 43-46: Update computeResetsInMs to validate the parsed timestamp
with an isFinite check before subtracting Date.now(); return 0 for malformed or
otherwise non-finite dates, while preserving the existing behavior for valid
future and past dates.
---
Nitpick comments:
In `@apps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swift`:
- Around line 311-355: Update WorkComposerFocusScheduler.apply to return early
when the requested focus state already matches the textView’s current responder
state and no transition is pending, avoiding a new Task and Task.yield on
repeated updateUIView calls. Preserve queued-task coalescing and the existing
initial-unfocused behavior, and only schedule a task when the responder state
still needs to change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 9a4e8017-030c-4a87-a7f7-c319a25482be
⛔ Files ignored due to path filters (4)
docs/ARCHITECTURE.mdis excluded by!docs/**docs/features/onboarding-and-settings/README.mdis excluded by!docs/**docs/features/onboarding-and-settings/usage-tracking.mdis excluded by!docs/**docs/features/sync-and-multi-device/ios-companion.mdis excluded by!docs/**
📒 Files selected for processing (6)
apps/desktop/src/main/services/usage/providerQuotaParsers.tsapps/desktop/src/main/services/usage/usageTrackingService.test.tsapps/desktop/src/main/services/usage/usageTrackingService.tsapps/desktop/src/renderer/components/usage/usage.test.tsxapps/ios/ADE/Views/Work/WorkComposerTypedTriggers.swiftapps/ios/ADETests/WorkComposerTriggerDetectorTests.swift
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Validation
Summary by CodeRabbit
New Features
Bug Fixes
Greptile Summary
This PR updates quota parsing and iOS composer focus handling. The main changes are:
providerQuotaParsers.ts.Confidence Score: 5/5
Safe to merge with low risk.
The updated code is focused, preserves the existing usage service contract, and includes tests for the quota parsing and composer focus paths changed in this PR.
No files require special attention.
What T-Rex did
Important Files Changed
Sequence Diagram
%%{init: {'theme': 'neutral'}}%% sequenceDiagram participant Provider as Claude/Codex quota API or CLI participant Service as usageTrackingService participant Parser as providerQuotaParsers participant Snapshot as Usage snapshot/cache participant Renderer as UsageQuotaPanel participant SwiftUI as SwiftUI composer update participant Scheduler as WorkComposerFocusScheduler participant UIKit as UITextView responder Provider->>Service: Return quota payload Service->>Parser: parseClaudeWindows / parseCodexRateLimitWindows Parser-->>Service: Normalized UsageWindow[] and extraUsage Service->>Snapshot: Store provider windows and pacing inputs Renderer->>Snapshot: getSnapshot / noteDemand Snapshot-->>Renderer: Windows with pacing meters SwiftUI->>Scheduler: apply latest focus binding Scheduler->>Scheduler: Coalesce pending request Scheduler-->>SwiftUI: Yield past updateUIView Scheduler->>UIKit: become/resign first responder only if latest request still applies%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%% sequenceDiagram participant Provider as Claude/Codex quota API or CLI participant Service as usageTrackingService participant Parser as providerQuotaParsers participant Snapshot as Usage snapshot/cache participant Renderer as UsageQuotaPanel participant SwiftUI as SwiftUI composer update participant Scheduler as WorkComposerFocusScheduler participant UIKit as UITextView responder Provider->>Service: Return quota payload Service->>Parser: parseClaudeWindows / parseCodexRateLimitWindows Parser-->>Service: Normalized UsageWindow[] and extraUsage Service->>Snapshot: Store provider windows and pacing inputs Renderer->>Snapshot: getSnapshot / noteDemand Snapshot-->>Renderer: Windows with pacing meters SwiftUI->>Scheduler: apply latest focus binding Scheduler->>Scheduler: Coalesce pending request Scheduler-->>SwiftUI: Yield past updateUIView Scheduler->>UIKit: become/resign first responder only if latest request still appliesReviews (2): Last reviewed commit: "fix: address usage and focus review edge..." | Re-trigger Greptile