Add Claude support - #1
Merged
Merged
Conversation
Deduplicate Claude usage metrics so the popover always shows one All models card and one Claude Design card in a fixed order. Keep the loading UI visible while provider state is still being loaded, instead of briefly showing the Connect a Provider screen on relaunch. Add parser regression tests for repeated and reordered Claude usage rows.
Document the upcoming v0.4.0 Claude support release, including provider-aware dashboard updates, stable Claude usage cards, startup loading behavior, login item support, and helper scripts. Add the missing v0.3.0 changelog entry for the compact menu bar progress-only status item.
divyanshub024
marked this pull request as ready for review
May 7, 2026 13:46
|
Warning Rate limit exceeded
To continue reviewing without waiting, purchase usage credits in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR implements Claude as a multi-provider parallel to Cursor, refactoring the app architecture to support generalized provider protocols and data models. It adds comprehensive Claude web scraping via WKWebView with DOM polling and network interception, introduces macOS launch-at-login functionality, and unifies Cursor and Claude into a shared dashboard UI with provider-aware status, metrics, and controls throughout the popover and settings views. ChangesClaude Multi-Provider Support & Launch-at-Login Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
|
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Sources/AIMeter/UI/SettingsWindowController.swift (1)
26-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winInitial window content size is below
SettingsViewminHeightof 520.
window.setContentSize(NSSize(width: 520, height: 320))is shorter than the SwiftUI form'sminHeight: 520. The window will auto-grow on first show, but the explicit initial size is misleading. Bump it to520x520(or similar) to matchSettingsView.- window.setContentSize(NSSize(width: 520, height: 320)) + window.setContentSize(NSSize(width: 520, height: 520))🤖 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 `@Sources/AIMeter/UI/SettingsWindowController.swift` at line 26, Update the initial window content size to match the SettingsView minHeight by changing the call to window.setContentSize(NSSize(width: 520, height: 520)); modify the existing window.setContentSize(...) invocation in SettingsWindowController (the constructor/initializer or setup method that calls window.setContentSize) so the height is 520 to align with SettingsView's minHeight.Sources/AIMeter/Core/CursorUsageCoordinator.swift (1)
47-57:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock
connect()while a refresh is already in flight.
refresh()guards againstisConnecting, butconnect()does not guard againstisRefreshing. Since both paths suspend onawait, a scheduled refresh can still be running when the user clicks Connect, which lets two client operations overlap against the same provider session state.Suggested fix
func connect() async { - if isConnecting { + if isConnecting || isRefreshing { return }🤖 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 `@Sources/AIMeter/Core/CursorUsageCoordinator.swift` around lines 47 - 57, connect() can run concurrently with an in-flight refresh because it only checks isConnecting; update connect() to also guard against isRefreshing (e.g. guard !isConnecting && !isRefreshing { return } or return early if isRefreshing) before setting isConnecting, so client.connect() and fetchAndStoreSnapshot() cannot overlap with refresh(); reference the connect() method, the isConnecting and isRefreshing flags, and the client.connect()/fetchAndStoreSnapshot() calls when making the change.
🧹 Nitpick comments (6)
Sources/AIMeter/Core/LaunchAtLoginController.swift (1)
86-88: 💤 Low valueUnnecessary
breakin aswitchcase.Swift
switchcases don't fall through, so thebreakon line 87 is dead code and can be removed.🧹 Proposed fix
case .enabled, .requiresApproval: userDefaults.set(true, forKey: defaultAppliedKey) - break }🤖 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 `@Sources/AIMeter/Core/LaunchAtLoginController.swift` around lines 86 - 88, Remove the redundant `break` inside the switch case in LaunchAtLoginController where `userDefaults.set(true, forKey: defaultAppliedKey)` is called — Swift switch cases do not fall through, so delete the `break` statement (search for the `userDefaults.set(true, forKey: defaultAppliedKey)` line or the switch inside LaunchAtLoginController to locate the exact case).scripts/run_dev.sh (1)
84-84: 💤 Low valueApp name is hardcoded in
osascriptcall instead of using$SCHEME.If
SCHEMEis ever renamed or reused for a different target, the graceful quit won't find the process by its display name.🛠️ Proposed fix
- osascript -e 'tell application "AIMeter" to quit' >/dev/null 2>&1 || true + osascript -e "tell application \"$SCHEME\" to quit" >/dev/null 2>&1 || true🤖 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 `@scripts/run_dev.sh` at line 84, The osascript call uses a hardcoded app name "AIMeter" instead of the SCHEME variable; update the command that currently calls osascript -e 'tell application "AIMeter" to quit' to interpolate the SCHEME variable (e.g., use the quoted/escaped value of $SCHEME) so the script quits the app matching the current SCHEME; ensure proper quoting/escaping of $SCHEME in the osascript argument and preserve the >/dev/null 2>&1 || true behavior.Sources/AIMeter/UI/MenuBarController.swift (3)
91-104: 💤 Low valueResizing
popover.contentSizewhile shown may cause visible jumps.
updateStatusItemis invoked on everyDashboardStatechange. If a snapshot transitions a provider from disconnected to connected (or vice versa) while the popover is open,preferredPopoverSize(for:)will switch heights between 370/420/560 and the user will see the popover snap. Consider only updatingcontentSizewhen the popover is not shown, or guarding with a "shape" key derived from(presentationState, connectedProviderCount)so it only changes on actual category transitions.🤖 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 `@Sources/AIMeter/UI/MenuBarController.swift` around lines 91 - 104, updateStatusItem currently sets popover.contentSize on every DashboardState change causing visible jumps when the popover is open; change it to only assign popover.contentSize when the popover is not shown (check popover.isShown) or when a derived "shape" key changes (compute a key from presentationState and connectedProviderCount) — store the lastShape on the MenuBarController and update contentSize only if newShape != lastShape, otherwise skip modifying popover.contentSize; keep button image/title/toolTip updates as-is.
118-133: 💤 Low valueHardcoded provider names in empty-state tooltip.
"AIMeter: Connect Cursor or Claude"is hardcoded; if you add another provider this string will silently go stale. Deriving the list fromUsageProvider.allCases(usingdisplayName) keeps it in sync.🤖 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 `@Sources/AIMeter/UI/MenuBarController.swift` around lines 118 - 133, The tooltip(for state: DashboardState) currently returns a hardcoded empty-state string "AIMeter: Connect Cursor or Claude"; change it to build the provider list dynamically from UsageProvider.allCases using each provider's displayName so new providers stay in sync. Locate tooltip(for:) and replace the hardcoded branch that returns the static string when connectedProviderSnapshots is empty with logic that maps UsageProvider.allCases.map(\.displayName).joined(separator: " or ") (or similar formatting) to produce the message (e.g., "AIMeter: Connect <providers>"), ensuring you still use the existing connectedProviderSnapshots, DashboardState, and displayName symbols.
220-273: 💤 Low valueRename
StatusBarImageFactoryparameters to useProviderConnectionStateinstead of theCursorConnectionStatealias.Since
CursorConnectionStateis a typealias forProviderConnectionStateand the popover now handles multiple providers, using the generic type name inimage(...),drawIndicator(...), andindicatorColor(for:)improves clarity and prevents naming from suggesting cursor-specific behavior.🤖 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 `@Sources/AIMeter/UI/MenuBarController.swift` around lines 220 - 273, Change the type names used in StatusBarImageFactory to the more generic ProviderConnectionState: update the signature of image(progress:state:) to accept state: ProviderConnectionState, update drawIndicator(atX:state:) and indicatorColor(for:) to take ProviderConnectionState, and update all internal uses and switch cases to refer to ProviderConnectionState variants (instead of CursorConnectionState) so the factory reflects provider-wide connection states.Tests/AIMeterTests/CursorUsageCoordinatorTests.swift (1)
149-149: 💤 Low valuePrefer the provider-generic error type in Claude coordinator test.
CursorUsageErroris a typealias forProviderUsageError. In a test explicitly driving a Claude coordinator, useProviderUsageError.syncFailed("Claude layout changed")for naming consistency.🤖 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 `@Tests/AIMeterTests/CursorUsageCoordinatorTests.swift` at line 149, The test currently asserts a failure with the provider-specific alias CursorUsageError; change it to use the underlying provider-generic type ProviderUsageError so naming matches the Claude coordinator context—replace .failure(CursorUsageError.syncFailed("Claude layout changed")) with .failure(ProviderUsageError.syncFailed("Claude layout changed")) in the CursorUsageCoordinatorTests.swift test case.
🤖 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 `@scripts/build_verify_notarized_dmg.sh`:
- Around line 8-9: Remove the hardcoded Apple Team ID default in the script by
no longer setting DEVELOPMENT_TEAM to "W2B7PMH9SQ" (and similarly avoid
embedding a default for NOTARYTOOL_PROFILE if desired); instead require callers
to provide DEVELOPMENT_TEAM via environment and add a fail-fast check in the
script (e.g., at script start) that validates DEVELOPMENT_TEAM is set and exits
with a clear error message if not. Update the export lines that reference
DEVELOPMENT_TEAM and NOTARYTOOL_PROFILE so they do not assign the hardcoded
defaults and add a small validation block in build_verify_notarized_dmg.sh that
checks the DEVELOPMENT_TEAM (and optionally NOTARYTOOL_PROFILE) environment
variable(s) and aborts with a descriptive error to prevent accidental
notarization attempts under the wrong account.
In `@Sources/AIMeter/App/AppDelegate.swift`:
- Around line 11-16: applicationWillTerminate currently launches an async Task
which may not run before process exit; replace that with synchronous Main-thread
calls so both stop() methods run reliably: remove Task { `@MainActor` in ... } and
call AppEnvironment.shared.cursorUsageCoordinator.stop() and
AppEnvironment.shared.claudeUsageCoordinator.stop() inside a
DispatchQueue.main.sync (or equivalent MainActor-synchronous) block so the
`@MainActor-annotated` stop() methods execute synchronously before termination.
In `@Sources/AIMeter/Claude/ClaudeSessionManager.swift`:
- Around line 64-78: clearWebsiteData() runs its removeData call asynchronously
and can race with immediate reconnects in connect(to:) / fetchUsage(from:);
change clearWebsiteData to provide an async completion (or return asynchronously
via withCheckedContinuation) and await it from disconnect(), or alternatively
add a serializing flag (e.g. isCleaningWebsiteData) that clearWebsiteData
sets/clears and that connect(to:)/fetchUsage(from:) check/wait on before reusing
dataStore; specifically modify clearWebsiteData(), disconnect(), and the callers
connect(to:)/fetchUsage(from:) to either await the dataStore.removeData
completion or block new connects until cleanup finishes so the removeData
callback cannot delete a newly created session.
In `@Sources/AIMeter/Claude/ClaudeURLValidator.swift`:
- Around line 47-65: Update deprecated URL property accesses to their
percentEncoded: variants: replace uses of url.user, url.password, url.host, and
url.path with url.user(percentEncoded:), url.password(percentEncoded:),
url.host(percentEncoded:) and url.path(percentEncoded:) respectively in
ClaudeURLValidator.swift (notably inside isAllowedClaudeURL(_:),
sanitizedUsageURL, and isUsageSettingsURLString) and adapt the logic to safely
unwrap the percent-encoded strings (e.g., lowercasing host after unwrapping) to
preserve existing behavior while avoiding macOS 14 compiler deprecation
warnings.
- Around line 23-65: Replace deprecated direct URL property accesses with the
percentEncoded variants to silence macOS 14 warnings: use
url.path(percentEncoded: false) wherever url.path is read (including the earlier
method that returns usagePageURL), use url.user(percentEncoded: false) and
url.password(percentEncoded: false) in the credential checks, and use
url.host(percentEncoded: false) when normalizing the host in
isAllowedClaudeURL(_:) and isUsageSettingsURLString(_:); keep the same logic
(lowercased/trimming/ comparisons) but call the percentEncoded: false accessors
in isAllowedClaudeURLString(_:), isUsageSettingsURLString(_:), and
isAllowedClaudeURL(_:).
In `@Sources/AIMeter/Core/CursorUsageClient.swift`:
- Around line 30-32: The .cancelled case in ProviderUsageError currently returns
a hardcoded "Cursor" string; update the message in the ProviderUsageError
.cancelled case (in CursorUsageClient.swift) to avoid referencing Cursor
directly and instead use a generic provider placeholder or the actual provider
identifier (e.g., use "\(providerName)" or "the provider") so it reads like
"Connection window closed before provider finished loading." Ensure you
reference the provider name variable used in the surrounding context (or fall
back to a generic "provider") when composing the returned error description.
- Around line 31-32: The .cancelled error case in CursorUsageClient.swift
currently returns a Cursor-specific message ("Connection window closed before
Cursor finished loading.") which is provider-specific; update the .cancelled
error description to a provider-agnostic message (e.g., "Connection window
closed before the operation finished." or "Connection window closed before
completion.") inside the same error/description implementation so any provider
(Claude or others) shows a neutral message; keep the change localized to the
.cancelled return value and preserve the existing error type and surrounding
structure.
In `@Sources/AIMeter/Core/Models.swift`:
- Around line 253-255: The computed property connectedProviderSnapshots
currently filters providerSnapshots by requiring connectionState == .connected,
which incorrectly hides snapshots with .authExpired and .syncFailed; change the
predicate in connectedProviderSnapshots to only exclude .disconnected (e.g.,
filter { $0.connectionState != .disconnected }) so preserved snapshots with
error/reconnect states remain visible; update the ProviderUsageSnapshot usage to
reference connectionState when applying this new filter.
In `@Sources/AIMeter/UI/MenuBarController.swift`:
- Around line 135-144: primaryConnectionState(for:) currently only returns
.connected or .disconnected, dropping .authExpired and .syncFailed; update the
function to scan state.connectedProviderSnapshots and preserve the worst
non-connected state: if any snapshot.connectionState == .authExpired return
.authExpired; else if any == .syncFailed return .syncFailed; else if any
snapshot.connectionState == .connected && snapshot.progressPercent != nil return
.connected; otherwise return .disconnected. Use the existing
connectedProviderSnapshots and connectionState checks to implement this order so
the status bar can surface auth/sync failures.
---
Outside diff comments:
In `@Sources/AIMeter/Core/CursorUsageCoordinator.swift`:
- Around line 47-57: connect() can run concurrently with an in-flight refresh
because it only checks isConnecting; update connect() to also guard against
isRefreshing (e.g. guard !isConnecting && !isRefreshing { return } or return
early if isRefreshing) before setting isConnecting, so client.connect() and
fetchAndStoreSnapshot() cannot overlap with refresh(); reference the connect()
method, the isConnecting and isRefreshing flags, and the
client.connect()/fetchAndStoreSnapshot() calls when making the change.
In `@Sources/AIMeter/UI/SettingsWindowController.swift`:
- Line 26: Update the initial window content size to match the SettingsView
minHeight by changing the call to window.setContentSize(NSSize(width: 520,
height: 520)); modify the existing window.setContentSize(...) invocation in
SettingsWindowController (the constructor/initializer or setup method that calls
window.setContentSize) so the height is 520 to align with SettingsView's
minHeight.
---
Nitpick comments:
In `@scripts/run_dev.sh`:
- Line 84: The osascript call uses a hardcoded app name "AIMeter" instead of the
SCHEME variable; update the command that currently calls osascript -e 'tell
application "AIMeter" to quit' to interpolate the SCHEME variable (e.g., use the
quoted/escaped value of $SCHEME) so the script quits the app matching the
current SCHEME; ensure proper quoting/escaping of $SCHEME in the osascript
argument and preserve the >/dev/null 2>&1 || true behavior.
In `@Sources/AIMeter/Core/LaunchAtLoginController.swift`:
- Around line 86-88: Remove the redundant `break` inside the switch case in
LaunchAtLoginController where `userDefaults.set(true, forKey:
defaultAppliedKey)` is called — Swift switch cases do not fall through, so
delete the `break` statement (search for the `userDefaults.set(true, forKey:
defaultAppliedKey)` line or the switch inside LaunchAtLoginController to locate
the exact case).
In `@Sources/AIMeter/UI/MenuBarController.swift`:
- Around line 91-104: updateStatusItem currently sets popover.contentSize on
every DashboardState change causing visible jumps when the popover is open;
change it to only assign popover.contentSize when the popover is not shown
(check popover.isShown) or when a derived "shape" key changes (compute a key
from presentationState and connectedProviderCount) — store the lastShape on the
MenuBarController and update contentSize only if newShape != lastShape,
otherwise skip modifying popover.contentSize; keep button image/title/toolTip
updates as-is.
- Around line 118-133: The tooltip(for state: DashboardState) currently returns
a hardcoded empty-state string "AIMeter: Connect Cursor or Claude"; change it to
build the provider list dynamically from UsageProvider.allCases using each
provider's displayName so new providers stay in sync. Locate tooltip(for:) and
replace the hardcoded branch that returns the static string when
connectedProviderSnapshots is empty with logic that maps
UsageProvider.allCases.map(\.displayName).joined(separator: " or ") (or similar
formatting) to produce the message (e.g., "AIMeter: Connect <providers>"),
ensuring you still use the existing connectedProviderSnapshots, DashboardState,
and displayName symbols.
- Around line 220-273: Change the type names used in StatusBarImageFactory to
the more generic ProviderConnectionState: update the signature of
image(progress:state:) to accept state: ProviderConnectionState, update
drawIndicator(atX:state:) and indicatorColor(for:) to take
ProviderConnectionState, and update all internal uses and switch cases to refer
to ProviderConnectionState variants (instead of CursorConnectionState) so the
factory reflects provider-wide connection states.
In `@Tests/AIMeterTests/CursorUsageCoordinatorTests.swift`:
- Line 149: The test currently asserts a failure with the provider-specific
alias CursorUsageError; change it to use the underlying provider-generic type
ProviderUsageError so naming matches the Claude coordinator context—replace
.failure(CursorUsageError.syncFailed("Claude layout changed")) with
.failure(ProviderUsageError.syncFailed("Claude layout changed")) in the
CursorUsageCoordinatorTests.swift test case.
🪄 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
Run ID: 2a7edf32-0b49-4c19-ba79-d9eef9f868b5
📒 Files selected for processing (30)
AIMeter.xcodeproj/project.pbxprojCHANGELOG.mdSources/AIMeter/App/AIMeterApp.swiftSources/AIMeter/App/AppDelegate.swiftSources/AIMeter/App/AppEnvironment.swiftSources/AIMeter/Claude/ClaudeDashboardClient.swiftSources/AIMeter/Claude/ClaudeDashboardParser.swiftSources/AIMeter/Claude/ClaudeSessionManager.swiftSources/AIMeter/Claude/ClaudeURLValidator.swiftSources/AIMeter/Claude/ClaudeWebViewScraper.swiftSources/AIMeter/Core/CursorUsageClient.swiftSources/AIMeter/Core/CursorUsageCoordinator.swiftSources/AIMeter/Core/DashboardStore.swiftSources/AIMeter/Core/LaunchAtLoginController.swiftSources/AIMeter/Core/Models.swiftSources/AIMeter/Cursor/CursorDashboardClient.swiftSources/AIMeter/Storage/SettingsStore.swiftSources/AIMeter/UI/MenuBarController.swiftSources/AIMeter/UI/MenuPopoverView.swiftSources/AIMeter/UI/SettingsView.swiftSources/AIMeter/UI/SettingsWindowController.swiftTests/AIMeterTests/ClaudeDashboardClientTests.swiftTests/AIMeterTests/ClaudeDashboardParserTests.swiftTests/AIMeterTests/ClaudeURLValidatorTests.swiftTests/AIMeterTests/CursorUsageCoordinatorTests.swiftTests/AIMeterTests/DashboardStoreTests.swiftTests/AIMeterTests/LaunchAtLoginControllerTests.swiftTests/AIMeterTests/SettingsStoreTests.swiftscripts/build_verify_notarized_dmg.shscripts/run_dev.sh
Remove the scripts directory from Git tracking while keeping the files available locally. Ignore scripts/ so build, release, icon, and development helper scripts remain local-only going forward.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Tests/AIMeterTests/DashboardStoreTests.swift (1)
241-243: ⚡ Quick winGuard mock fetch queue to avoid trap-based test failures.
Line 242 uses
removeFirst()directly; if a test refreshes more than expected, this crashes with an index trap instead of producing a clear assertion failure.Proposed test-harness hardening
func fetchUsage() async throws -> CursorUsageSnapshot { - let result = fetchResults.removeFirst() + XCTAssertFalse(fetchResults.isEmpty, "DashboardMockCursorUsageClient.fetchResults exhausted before fetchUsage()") + let result = fetchResults.removeFirst() return try result.get() }🤖 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 `@Tests/AIMeterTests/DashboardStoreTests.swift` around lines 241 - 243, The test harness currently uses fetchResults.removeFirst() in fetchUsage(), which will crash with an index trap when the mock queue is empty; instead guard the queue: check fetchResults.isEmpty at the start of fetchUsage(), call a test failure (e.g., XCTFail with a clear message) and then throw a descriptive test error (or return a failed Result) so the test fails cleanly; update the fetchUsage() implementation and any test helper types to include a TestError (or reuse an existing test error type) and reference fetchResults and fetchUsage() when making this change.Sources/AIMeter/Claude/ClaudeSessionManager.swift (1)
48-55: 💤 Low valueUnnecessary
do-catchthat only rethrows.The
catch { throw error }block adds no handling. Remove it and let the error propagate naturally.♻️ Proposed simplification
- do { - let snapshot = try await scraper.start() - isConnected = true - windowController.close() - return snapshot - } catch { - throw error - } + let snapshot = try await scraper.start() + isConnected = true + return snapshot🤖 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 `@Sources/AIMeter/Claude/ClaudeSessionManager.swift` around lines 48 - 55, The do-catch around calling scraper.start() is redundant because the catch only rethrows; remove the entire do-catch and await scraper.start() directly, set isConnected = true and call windowController.close() then return the snapshot (i.e. replace the do { let snapshot = try await scraper.start(); isConnected = true; windowController.close(); return snapshot } catch { throw error } with a direct try await call and the same subsequent lines), keeping function signature and error propagation unchanged.
🤖 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 `@Sources/AIMeter/Claude/ClaudeSessionManager.swift`:
- Around line 43-55: The connection window isn't closed when scraper.start()
throws because windowController.close() is only called on the success path; move
the dismissal into the defer so it always runs: in ClaudeSessionManager.swift
ensure the defer block that currently nils activeScraper and
connectionWindowController also calls windowController.close(), so that
regardless of whether await scraper.start() succeeds or throws (non-cancellation
errors or timeouts), the "Connect Claude" sheet is dismissed and resources are
cleaned up; reference activeScraper, connectionWindowController,
windowController and the await scraper.start() call when making the change.
In `@Sources/AIMeter/UI/MenuBarController.swift`:
- Line 94: The popover is being resized on every state update causing a visible
jump; update the assignment of popover.contentSize in MenuBarController so it
only applies the value from preferredPopoverSize(for:) when the popover is not
currently shown (check the NSPopover isShown state) and skip resizing while
popover.isShown is true, keeping the current size until the popover is closed.
---
Nitpick comments:
In `@Sources/AIMeter/Claude/ClaudeSessionManager.swift`:
- Around line 48-55: The do-catch around calling scraper.start() is redundant
because the catch only rethrows; remove the entire do-catch and await
scraper.start() directly, set isConnected = true and call
windowController.close() then return the snapshot (i.e. replace the do { let
snapshot = try await scraper.start(); isConnected = true;
windowController.close(); return snapshot } catch { throw error } with a direct
try await call and the same subsequent lines), keeping function signature and
error propagation unchanged.
In `@Tests/AIMeterTests/DashboardStoreTests.swift`:
- Around line 241-243: The test harness currently uses
fetchResults.removeFirst() in fetchUsage(), which will crash with an index trap
when the mock queue is empty; instead guard the queue: check
fetchResults.isEmpty at the start of fetchUsage(), call a test failure (e.g.,
XCTFail with a clear message) and then throw a descriptive test error (or return
a failed Result) so the test fails cleanly; update the fetchUsage()
implementation and any test helper types to include a TestError (or reuse an
existing test error type) and reference fetchResults and fetchUsage() when
making this 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 996f5fa9-edec-4691-90b9-ca54ab5d59e7
📒 Files selected for processing (12)
.gitignoreSources/AIMeter/App/AppDelegate.swiftSources/AIMeter/Claude/ClaudeSessionManager.swiftSources/AIMeter/Claude/ClaudeURLValidator.swiftSources/AIMeter/Core/CursorUsageClient.swiftSources/AIMeter/Core/Models.swiftSources/AIMeter/UI/MenuBarController.swiftTests/AIMeterTests/DashboardStoreTests.swiftscripts/build_dmg.shscripts/generate_app_icon.swiftscripts/generate_dmg_background.swiftscripts/release_github.sh
💤 Files with no reviewable changes (4)
- scripts/release_github.sh
- scripts/generate_dmg_background.swift
- scripts/generate_app_icon.swift
- scripts/build_dmg.sh
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- Sources/AIMeter/Core/Models.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- Sources/AIMeter/App/AppDelegate.swift
- Sources/AIMeter/Claude/ClaudeURLValidator.swift
| private func updateStatusItem(_ state: DashboardState) { | ||
| guard let button = statusItem.button else { return } | ||
|
|
||
| popover.contentSize = preferredPopoverSize(for: state) |
There was a problem hiding this comment.
Popover resizes while open, causing a visible size jump.
preferredPopoverSize is applied on every state update, including while the popover is visible. Transitioning between 1 and 2 connected providers changes height by 190 pt in-place. Consider only applying the new size when the popover is not shown.
🔧 Suggested guard
- popover.contentSize = preferredPopoverSize(for: state)
+ if !popover.isShown {
+ popover.contentSize = preferredPopoverSize(for: state)
+ }📝 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
| popover.contentSize = preferredPopoverSize(for: state) | |
| if !popover.isShown { | |
| popover.contentSize = preferredPopoverSize(for: state) | |
| } |
🤖 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 `@Sources/AIMeter/UI/MenuBarController.swift` at line 94, The popover is being
resized on every state update causing a visible jump; update the assignment of
popover.contentSize in MenuBarController so it only applies the value from
preferredPopoverSize(for:) when the popover is not currently shown (check the
NSPopover isShown state) and skip resizing while popover.isShown is true,
keeping the current size until the popover is closed.
Move Claude connection window dismissal into the connect defer path so scraper errors, cancellations, and timeouts clean up the visible window and controller references.
4 tasks
4 tasks
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
1 participant
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
Details
Validation
Summary by CodeRabbit
New Features
Changed
Bug Fixes