feat: Gemini Provider Support#17
Conversation
- Bump GeminiLogReader cache version to invalidate improperly cached zero-event files from earlier iterations. - Update Pricing.swift canonicalModel to strip '-preview' and '-latest' suffixes so Gemini models map correctly to the pricing table.
- Refactored PanelHeader to align provider titles centrally over their respective columns, by moving the entire header below the notch. Increased expandedContentHeight to compensate. - Defaulted Gemini plan to 'ADVANCED' in UsageFetcher since the API does not provide one, ensuring consistent UI treatment.
- Extracted message ID as dedupKey in GeminiLogReader to prevent summing cumulative turn updates, fixing inflated cost calculation. - Updated UsageFetcher to default Gemini plan to 'pro' for parity with user subscription.
…nal turn event - The Gemini API reports 'input' tokens as the total context window size (including cached tokens). The log parser now computes `billableInput = max(0, input - cached)` to prevent double-charging cache reads at the full input rate. - The deduplication logic was capturing the first event of a turn (which often lacked output tokens during streaming). It now uses a dictionary to retain the last, most complete event for each turn. - Bumped the cache version to 4 to force a rescan of all logs.
📝 WalkthroughWalkthroughThis PR adds Gemini as a third provider to the CodexIsland macOS cost/usage tracker. It extends state, log parsing, API fetching, and persistence to handle Gemini; refactors two-provider UI logic into dynamic multi-provider layouts; and updates styling, build identity, and async safety patterns. ChangesGemini Provider Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
Sources/Usage/UsageStore.swift (1)
44-72:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDemo mode should populate Gemini too
In demo mode, Claude and Codex are injected but Gemini is not updated, so the third provider can show stale/empty data during recordings.
Suggested fix
self.codex = AppUsage( fiveHour: WindowUsage( usedPercent: 0.67, resetAt: now.addingTimeInterval(2 * 3600 + 23 * 60), error: nil ), weekly: WindowUsage( usedPercent: 0.76, resetAt: now.addingTimeInterval(4 * 86400 + 18 * 3600), error: nil ), plan: "pro" ) + self.gemini = AppUsage( + fiveHour: WindowUsage( + usedPercent: 0.58, + resetAt: now.addingTimeInterval(1 * 3600 + 55 * 60), + error: nil + ), + weekly: WindowUsage( + usedPercent: 0.62, + resetAt: now.addingTimeInterval(3 * 86400 + 20 * 3600), + error: nil + ), + plan: "pro" + ) self.lastUpdated = now 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/Usage/UsageStore.swift` around lines 44 - 72, The demo initializer currently sets self.claude and self.codex but leaves self.gemini untouched, causing stale/empty data; update the demo mode block to also assign self.gemini to an AppUsage instance (similar to claude/codex) using WindowUsage values and a plan string, and ensure self.lastUpdated is set as done now; locate the demo code where claude and codex are assigned (symbols: self.claude, self.codex, AppUsage, WindowUsage, self.lastUpdated) and add a matching self.gemini assignment with appropriate usedPercent, resetAt, error, and plan fields.Sources/Model/AlertEngine.swift (1)
82-87:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing
$geminisubscription in trigger list — Gemini updates won't driverecompute().The
triggersarray still only includesUsageStore.shared.$claudeandUsageStore.shared.$codex. Whenusage.geminipublishes alone (e.g., gemini-only refresh, test injection from the preview buttons that may set providers sequentially, or a partial-failure scenario where only Gemini's value changes), no subscription fires andrecompute()is skipped — sogeminiSeverity, the combinedseverity, and any new crossings for Gemini won't be picked up until Claude or Codex next ticks.🔧 Proposed fix
let triggers: [AnyPublisher<Void, Never>] = [ UsageStore.shared.$claude.map { _ in () }.eraseToAnyPublisher(), UsageStore.shared.$codex.map { _ in () }.eraseToAnyPublisher(), + UsageStore.shared.$gemini.map { _ in () }.eraseToAnyPublisher(), AlertThresholdStore.shared.objectWillChange.map { _ in () }.eraseToAnyPublisher(), ProviderVisibilityStore.shared.objectWillChange.map { _ in () }.eraseToAnyPublisher(), ]🤖 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/Model/AlertEngine.swift` around lines 82 - 87, The triggers array is missing a subscription for UsageStore.shared.$gemini so updates to gemini won't call recompute(); add a publisher entry analogous to the existing ones (map { _ in () }.eraseToAnyPublisher()) for UsageStore.shared.$gemini in the triggers array in AlertEngine (where UsageStore.shared.$claude and .$codex are listed) so geminiSeverity, the combined severity, and any Gemini crossings are recomputed promptly.Sources/Views/CostView.swift (1)
3-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale doc comment — still describes the two-provider switch.
The doc still references
(claudeOn, codexOn)and the three two-provider cases (both on / one on / both off). The actual implementation now branches on a dynamicvisiblearray of any subset of {Claude, Codex, Gemini}. Future readers will get the wrong mental model.🔧 Suggested doc update
/// Cost data row. Mirrors `UsageView`'s data-row shape so swipe transitions /// between them don't reflow the panel. Chrome (provider titles, footer /// chip + page dots + sync status) lives in `PanelHeader` / `PanelFooter`. /// -/// Branches on `(claudeOn, codexOn)` from `ProviderVisibilityStore`: -/// - both on: two `CostBlock`s with a hairline divider (default). -/// - one on: the live block on its native side (centered tiles, since -/// its half doubled), hairline, then a per-model dollar -/// breakdown filling the freed half. -/// - both off: a centered `BothHiddenPlaceholder`. +/// Branches on the ordered visible-provider list from +/// `ProviderVisibilityStore` (Claude/Codex/Gemini): +/// - none visible: centered `BothHiddenPlaceholder`. +/// - one visible: a centered `CostBlock`, hairline, then a per-model +/// dollar breakdown filling the freed columns. +/// - 2+ visible: one `CostBlock` per visible provider, separated +/// by hairlines.🤖 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/Views/CostView.swift` around lines 3 - 12, The doc comment on CostView is outdated: it still describes branching on (claudeOn, codexOn) and three two-provider cases, but the code now uses a dynamic visible array (e.g., visible: [Provider]) that can contain any subset of {Claude, Codex, Gemini}; update the comment above CostView to reflect the current behavior by describing that rendering branches based on the dynamic visible array from ProviderVisibilityStore (or the local visible variable), enumerate the possible general outcomes (multiple visible providers -> multiple CostBlocks with dividers, single visible -> centered single block plus breakdown, none -> BothHiddenPlaceholder or equivalent), and remove references to the old (claudeOn, codexOn) flags so the doc matches the implementation.Sources/Cost/CostStore.swift (1)
135-170:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDemo mode leaves
geminiat.empty— third column will render zero in demo recordings.
loadDemoData()hand-tunesclaudeandcodexbut never assignsself.gemini. With Gemini now part of the 3-column compact island layout and likely visible by default, demo screen recordings would show an empty Gemini panel alongside two populated providers — exactly the asymmetry demo mode exists to avoid.Want me to draft a
self.gemini = ProviderCost(...)block in the same style as Claude/Codex (with plausible April 2026 daily series for Gemini 3.1 Pro)?🤖 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/Cost/CostStore.swift` around lines 135 - 170, The demo data loader loadDemoData() never assigns self.gemini, leaving the Gemini panel empty; add a self.gemini = ProviderCost(...) block mirroring the style of self.claude and self.codex using CostWindow for today and month (dollars, tokens, billableTokens, series, label, error, unknownModels) and then set self.lastUpdated = Date(); reference ProviderCost, CostWindow and the existing self.claude/self.codex initializers to copy formatting and plausible April daily series for Gemini.Sources/Views/UsageView.swift (1)
4-12:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStale doc comment — mirrors the
CostViewissue.Same as
CostView: this still describes the two-provider(claudeOn, codexOn)switch with the "both on / one on / both off" trichotomy, but the body now iterates a dynamicvisibleProviders()list that includes Gemini. Worth updating in lockstep.🔧 Suggested doc update
/// Usage data row. The chrome (provider titles, footer chip + page dots + /// sync status) lives in `PanelHeader` / `PanelFooter` so it stays fixed /// while this row swipes between usage and cost screens. /// -/// Branches on `(claudeOn, codexOn)` from `ProviderVisibilityStore`: -/// - both on: two `ChartsBlock`s with a hairline divider (default). -/// - one on: the live block on its native side, hairline, then a -/// per-model token breakdown filling the freed half. -/// - both off: a centered `BothHiddenPlaceholder`. +/// Branches on the ordered visible-provider list from +/// `ProviderVisibilityStore` (Claude/Codex/Gemini): +/// - none visible: centered `BothHiddenPlaceholder`. +/// - one visible: that provider's `ChartsBlock`, hairline, then a +/// per-model token breakdown filling the freed half. +/// - 2+ visible: one `ChartsBlock` per visible provider, separated +/// by hairlines.🤖 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/Views/UsageView.swift` around lines 4 - 12, The top-of-file doc comment in UsageView.swift is stale: it describes a hardcoded (claudeOn, codexOn) three-way branching while the implementation now uses visibleProviders() and supports additional providers (e.g., Gemini). Update the comment to describe the current behavior: that the chrome lives in PanelHeader/PanelFooter, that the view iterates visibleProviders() from ProviderVisibilityStore to decide layout (multiple visible providers render multiple ChartsBlock instances separated by hairline dividers, a single visible provider shows the live block plus a per-model token breakdown, and when none are visible it shows BothHiddenPlaceholder or a more generic empty placeholder), and mention that this mirrors CostView’s doc style so keep wording consistent with CostView.
🧹 Nitpick comments (4)
Sources/Views/CostView.swift (1)
27-55: 💤 Low valueSuggest extracting the
AlertEngine.Provider → TokenEvent.Providermapping.The same 3-case switch is duplicated at L27-33 and L46-52. A single private helper would keep the two paths from drifting if a future provider is added.
♻️ Suggested helper
+ private func tokenEventProvider(for provider: AlertEngine.Provider) -> TokenEvent.Provider { + switch provider { + case .claude: return .claude + case .codex: return .codex + case .gemini: return .gemini + } + }Then both
CostBlock(...)call sites become:CostBlock(color: p.color, cost: p.cost, loading: p.loading, provider: tokenEventProvider(for: p.provider), centerWhenSingle: ...)🤖 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/Views/CostView.swift` around lines 27 - 55, Duplicate switch mapping from AlertEngine.Provider to TokenEvent.Provider appears twice (in the closures building costProvider). Add a single private helper, e.g. private func tokenEventProvider(for provider: AlertEngine.Provider) -> TokenEvent.Provider, that contains the switch (claude/codex/gemini) and replace both inline closures with calls to tokenEventProvider(for: p.provider) in the CostBlock invocations (both the single-view branch and the ForEach branch) so future provider changes are centralized.Sources/Views/IslandRootView.swift (3)
148-166: 💤 Low valueVestigial second
setState(.compact)in hover end.The immediate
model.setState(.compact)at Line 152 already commits the state transition, andIslandModel.setStateearly-returns whennew == state(Lines 56 inIslandModel.swift). The delayed block at Lines 160-165 therefore never does anything useful — by the time it runs,stateis already.compactand the secondwithAnimation(.closeMorph) { model.setState(.compact) }is a no-op. It's also redundant with thecase .compactguard at Line 150. Looks like leftover choreography from the prior delayed-transition flow.♻️ Suggested trim
case .ended: hovering = false guard model.state != .compact else { return } withAnimation(.closeMorph) { model.setState(.compact) } withAnimation(.easeOut(duration: 0.08)) { pillsVisible = false } withAnimation(.easeOut(duration: 0.10)) { contentVisible = false } - DispatchQueue.main.asyncAfter(deadline: .now() + 0.10) { - guard !hovering else { return } - withAnimation(.closeMorph) { - model.setState(.compact) - } - } }🤖 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/Views/IslandRootView.swift` around lines 148 - 166, Remove the redundant delayed state-setting in the hover .ended handler: in IslandRootView, drop the DispatchQueue.main.asyncAfter block that calls withAnimation(.closeMorph) { model.setState(.compact) } because model.setState(.compact) is already invoked immediately (and early-returns if state is unchanged); keep the immediate animations that set hovering = false, pillsVisible = false and contentVisible = false, and only remove the DispatchQueue.main.asyncAfter { guard !hovering else { return }; withAnimation(.closeMorph) { model.setState(.compact) } } block to eliminate the vestigial/no-op call.
412-467: 💤 Low valueFinish the cleanup of
LogoOverlayandPeekPillOverlay.Both views are now self-documented as deprecated:
LogoOverlay(Lines 412-452) — kept "for types/consistency until final cleanup".PeekPillOverlay(Lines 454-467) — body isEmptyView()while the struct and itsprovider/topPadding/pillsVisible/ store wires are still live.Carrying empty/dead views forward keeps the
geminicase (Line 449) in step with the real provider list and tempts future readers to wire them up again. Since they're no longer referenced from any rendering path in this file, dropping them removes the maintenance drag in one shot.Want me to open a follow-up issue to track removing both structs (and any remaining call sites in other files) as part of the "final cleanup" alluded to in the comment?
🤖 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/Views/IslandRootView.swift` around lines 412 - 467, The two deprecated view structs LogoOverlay and PeekPillOverlay are dead code and keep the obsolete provider cases (e.g., .gemini) in sync; remove both structs (LogoOverlay and PeekPillOverlay) and their associated observed-object properties (ProviderVisibilityStore, UsageStore, AlertEngine) from this file, and then locate and delete any remaining call sites that reference LogoOverlay or PeekPillOverlay in the codebase so no references remain; ensure you also remove providerLabel/switch logic only if it becomes unused after deletion and run tests/compile to confirm no missing symbols.
248-270: ⚡ Quick winDead helper with a misleading comment — remove or wire it up.
balancedDistribution(for:)(and itsProviderUnitsstruct) is defined but never called; the inline HStack at lines 83–99 uses its owncount / 2leadingCount logic. Worse, the comment on Line 257 claims "1 leading, 2 trailing. [Claude] | [Notch] | [Codex, Gemini]" for N=3, but the code computestrailingCount = count / 2 = 1andleadingCount = count - trailingCount = 2— i.e. the opposite split. If a future maintainer revives this helper based on the comment, they'll get an inverted layout.♻️ Suggested cleanup
- private struct ProviderUnits { - var leading: [VisibleLogoProvider] - var trailing: [VisibleLogoProvider] - } - - private func balancedDistribution(for visible: [VisibleLogoProvider]) -> ProviderUnits { - var units = ProviderUnits(leading: [], trailing: []) - let count = visible.count - - // For N=3: 1 leading, 2 trailing. [Claude] | [Notch] | [Codex, Gemini] - // This keeps Codex in the middle of the whole sequence. - let trailingCount = count / 2 - let leadingCount = count - trailingCount - - for (idx, p) in visible.enumerated() { - if idx < leadingCount { - units.leading.append(p) - } else { - units.trailing.append(p) - } - } - return units - } -If you do intend to keep it for a future left/right anchored layout, please fix the swap so the implementation matches the doc comment:
- let trailingCount = count / 2 - let leadingCount = count - trailingCount + let leadingCount = count / 2 + let trailingCount = count - leadingCount🤖 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/Views/IslandRootView.swift` around lines 248 - 270, The ProviderUnits struct and balancedDistribution(for:) are unused and have a comment that contradicts their implementation; either remove both (ProviderUnits and balancedDistribution) to eliminate dead code, or wire balancedDistribution into the HStack logic (replace the inline count/2 split in the HStack with a call to balancedDistribution(visible:) and use the returned .leading and .trailing arrays), and if you keep the helper fix its math so the comment matches behavior (for N=3 set trailingCount = count - (count / 2) and leadingCount = count / 2, or update the comment to match the current split).
🤖 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 `@build.sh`:
- Line 7: Revert the BUNDLE_ID change in build.sh: restore the original bundle
identifier (set the BUNDLE_ID variable back to "dev.codexisland.CodexIsland"
instead of "dev.codexisland.CodexIsland.gemini"), ensure any code or config that
referenced the temporary value is updated back, and include a clear commit
message stating this reversion; only change the bundle ID if there is an
explicit migration request and document that request.
In `@Sources/Cost/CostStore.swift`:
- Around line 369-374: The Gemini branch in CostStore.swift currently does
canonical.replacingOccurrences(of: "gemini-", with: "Gemini
").replacingOccurrences(of: "flash-lite", with: "Flash-Lite").capitalized which
leaves internal hyphens (e.g., "3.1-pro") and yields "Gemini 3.1-Pro"; update
this branch to normalize internal dashes before capitalizing — after the
"gemini-" and "flash-lite" replacements also replace remaining "-" characters
(or at least the dash between version and qualifier) with a space (or convert to
the same separator used in the Claude branch), then call .capitalized; refer to
the canonical variable and the replacingOccurrences calls when making the
change.
In `@Sources/Cost/GeminiLogReader.swift`:
- Around line 36-43: The dedup logic currently unconditionally overwrites
latestById[ev.dedupKey] with event (in the loop that processes walk/jsonlFiles),
which relies on enumeration order; change it to be timestamp-aware: when
ev.dedupKey is non-empty, look up the existing entry in latestById and only
replace it if the current event's timestamp is newer (compare event.timestamp or
the event's existing time field) than the stored event's timestamp (or insert if
none exists); keep the else branch appending to withoutId and ensure you
reference latestById, ev.dedupKey, event (and the event timestamp field) in the
update check so final turn state is chosen by time, not file order.
In `@Sources/Usage/UsageFetcher.swift`:
- Around line 399-403: The parsing of the "resetTime" string uses an
ISO8601DateFormatter with only .withInternetDateTime, which fails for
fractional-second timestamps; update the formatter setup where "resetTime" is
parsed (the block that assigns resetAt) to include the same formatOptions used
for the "resets_at" parsing (i.e., include .withFractionalSeconds in addition to
.withInternetDateTime) so fractional-second ISO‑8601 values are correctly parsed
by the ISO8601DateFormatter.
- Around line 373-375: The parsing block that uses JSONSerialization and
stringly-typed casts (obj["buckets"], d["remainingFraction"], d["resetTime"])
should be replaced with Decodable models and JSONDecoder to get compile-time
safety: create a GeminiQuotaResponse: Decodable { let buckets: [GeminiBucket] }
and GeminiBucket: Decodable { let remainingFraction: Double; let resetTime:
Date? /*or String depending on API*/ } (name types as
GeminiBucket/GeminiQuotaResponse), configure JSONDecoder.dateDecodingStrategy if
needed, decode data via JSONDecoder().decode(GeminiQuotaResponse.self, from:
data), then map the decoded buckets to the existing usage structures and remove
all `as?` casts and manual dictionary indexing; on decode failure, call
errorPair with the decoder error to preserve existing error handling.
- Around line 353-354: The URL is being force-unwrapped when constructing `url`
for the request (let url = URL(string: ...)!), which violates Swift guidelines;
replace it with a safe unwrap (guard let url = URL(string: "...") else { handle
failure (log/return/throw) }) before creating `req` so that `req =
URLRequest(url: url)` only runs with a valid URL; update the surrounding
function (the block creating `url` and `req`) to handle the fallback path
appropriately (log an error and return or throw) instead of force-unwrapping.
---
Outside diff comments:
In `@Sources/Cost/CostStore.swift`:
- Around line 135-170: The demo data loader loadDemoData() never assigns
self.gemini, leaving the Gemini panel empty; add a self.gemini =
ProviderCost(...) block mirroring the style of self.claude and self.codex using
CostWindow for today and month (dollars, tokens, billableTokens, series, label,
error, unknownModels) and then set self.lastUpdated = Date(); reference
ProviderCost, CostWindow and the existing self.claude/self.codex initializers to
copy formatting and plausible April daily series for Gemini.
In `@Sources/Model/AlertEngine.swift`:
- Around line 82-87: The triggers array is missing a subscription for
UsageStore.shared.$gemini so updates to gemini won't call recompute(); add a
publisher entry analogous to the existing ones (map { _ in ()
}.eraseToAnyPublisher()) for UsageStore.shared.$gemini in the triggers array in
AlertEngine (where UsageStore.shared.$claude and .$codex are listed) so
geminiSeverity, the combined severity, and any Gemini crossings are recomputed
promptly.
In `@Sources/Usage/UsageStore.swift`:
- Around line 44-72: The demo initializer currently sets self.claude and
self.codex but leaves self.gemini untouched, causing stale/empty data; update
the demo mode block to also assign self.gemini to an AppUsage instance (similar
to claude/codex) using WindowUsage values and a plan string, and ensure
self.lastUpdated is set as done now; locate the demo code where claude and codex
are assigned (symbols: self.claude, self.codex, AppUsage, WindowUsage,
self.lastUpdated) and add a matching self.gemini assignment with appropriate
usedPercent, resetAt, error, and plan fields.
In `@Sources/Views/CostView.swift`:
- Around line 3-12: The doc comment on CostView is outdated: it still describes
branching on (claudeOn, codexOn) and three two-provider cases, but the code now
uses a dynamic visible array (e.g., visible: [Provider]) that can contain any
subset of {Claude, Codex, Gemini}; update the comment above CostView to reflect
the current behavior by describing that rendering branches based on the dynamic
visible array from ProviderVisibilityStore (or the local visible variable),
enumerate the possible general outcomes (multiple visible providers -> multiple
CostBlocks with dividers, single visible -> centered single block plus
breakdown, none -> BothHiddenPlaceholder or equivalent), and remove references
to the old (claudeOn, codexOn) flags so the doc matches the implementation.
In `@Sources/Views/UsageView.swift`:
- Around line 4-12: The top-of-file doc comment in UsageView.swift is stale: it
describes a hardcoded (claudeOn, codexOn) three-way branching while the
implementation now uses visibleProviders() and supports additional providers
(e.g., Gemini). Update the comment to describe the current behavior: that the
chrome lives in PanelHeader/PanelFooter, that the view iterates
visibleProviders() from ProviderVisibilityStore to decide layout (multiple
visible providers render multiple ChartsBlock instances separated by hairline
dividers, a single visible provider shows the live block plus a per-model token
breakdown, and when none are visible it shows BothHiddenPlaceholder or a more
generic empty placeholder), and mention that this mirrors CostView’s doc style
so keep wording consistent with CostView.
---
Nitpick comments:
In `@Sources/Views/CostView.swift`:
- Around line 27-55: Duplicate switch mapping from AlertEngine.Provider to
TokenEvent.Provider appears twice (in the closures building costProvider). Add a
single private helper, e.g. private func tokenEventProvider(for provider:
AlertEngine.Provider) -> TokenEvent.Provider, that contains the switch
(claude/codex/gemini) and replace both inline closures with calls to
tokenEventProvider(for: p.provider) in the CostBlock invocations (both the
single-view branch and the ForEach branch) so future provider changes are
centralized.
In `@Sources/Views/IslandRootView.swift`:
- Around line 148-166: Remove the redundant delayed state-setting in the hover
.ended handler: in IslandRootView, drop the DispatchQueue.main.asyncAfter block
that calls withAnimation(.closeMorph) { model.setState(.compact) } because
model.setState(.compact) is already invoked immediately (and early-returns if
state is unchanged); keep the immediate animations that set hovering = false,
pillsVisible = false and contentVisible = false, and only remove the
DispatchQueue.main.asyncAfter { guard !hovering else { return };
withAnimation(.closeMorph) { model.setState(.compact) } } block to eliminate the
vestigial/no-op call.
- Around line 412-467: The two deprecated view structs LogoOverlay and
PeekPillOverlay are dead code and keep the obsolete provider cases (e.g.,
.gemini) in sync; remove both structs (LogoOverlay and PeekPillOverlay) and
their associated observed-object properties (ProviderVisibilityStore,
UsageStore, AlertEngine) from this file, and then locate and delete any
remaining call sites that reference LogoOverlay or PeekPillOverlay in the
codebase so no references remain; ensure you also remove providerLabel/switch
logic only if it becomes unused after deletion and run tests/compile to confirm
no missing symbols.
- Around line 248-270: The ProviderUnits struct and balancedDistribution(for:)
are unused and have a comment that contradicts their implementation; either
remove both (ProviderUnits and balancedDistribution) to eliminate dead code, or
wire balancedDistribution into the HStack logic (replace the inline count/2
split in the HStack with a call to balancedDistribution(visible:) and use the
returned .leading and .trailing arrays), and if you keep the helper fix its math
so the comment matches behavior (for N=3 set trailingCount = count - (count / 2)
and leadingCount = count / 2, or update the comment to match the current split).
🪄 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: 50c3d4f3-4dc3-485c-bb7b-db1ef6b53ff0
📒 Files selected for processing (28)
Sources/Cost/CostStore.swiftSources/Cost/GeminiLogReader.swiftSources/Cost/Pricing.swiftSources/Cost/TokenEvent.swiftSources/Model/AlertEngine.swiftSources/Model/ChartStyle.swiftSources/Model/CostStylePref.swiftSources/Model/IslandModel.swiftSources/Model/ProviderVisibilityStore.swiftSources/Model/TokenCountModeStore.swiftSources/Theme/Colors.swiftSources/Theme/NumericTransition.swiftSources/Usage/UsageFetcher.swiftSources/Usage/UsageStore.swiftSources/Views/CostBlock.swiftSources/Views/CostView.swiftSources/Views/IslandRootView.swiftSources/Views/IslandShape.swiftSources/Views/NotchPeekPill.swiftSources/Views/PanelHeader.swiftSources/Views/ProviderBreakdown.swiftSources/Views/SettingsView.swiftSources/Views/UsageView.swiftSources/Window/IslandWindowController.swiftbuild.shscripts/gemini_test.swiftscripts/log_test.swiftscripts/verify.sh
| // Google: "gemini-3.1-pro" → "Gemini 3.1 Pro" | ||
| if canonical.hasPrefix("gemini-") { | ||
| return canonical.replacingOccurrences(of: "gemini-", with: "Gemini ") | ||
| .replacingOccurrences(of: "flash-lite", with: "Flash-Lite") | ||
| .capitalized | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Swift String capitalized behavior with hyphens word boundaries
💡 Result:
Swift's String.capitalized property (inherited from NSString) capitalizes the first character of each "word" and lowercases the rest. A "word" is defined as any sequence of characters delimited by spaces, tabs, or line terminators [1][2]. Hyphens are not listed as delimiters [1]. However, empirical evidence shows that certain punctuation, including hyphens (-), commas (,), and plus signs (+), is treated as a word delimiter in practice: "ab,cd+ef-gh".capitalized produces "Ab,Cd+Ef-Gh" [2]. Official docs note that "some common word delimiting punctuation isn’t considered," explaining why it may not always work as expected for strings with punctuation like hyphens [1]. For user-facing text, use localizedCapitalized or capitalized(with:) instead [1]. This behavior is consistent across Swift versions up to the latest docs (2026) [1].
Citations:
- 1: https://developer.apple.com/documentation/foundation/nsstring/capitalized
- 2: https://thedeveloperblog.com/swift/lower-swift
🏁 Script executed:
fd -t f CostStore.swiftRepository: ericjypark/codex-island
Length of output: 96
🏁 Script executed:
cat -n Sources/Cost/CostStore.swift | sed -n '360,380p'Repository: ericjypark/codex-island
Length of output: 1223
🏁 Script executed:
cat -n Sources/Cost/CostStore.swift | sed -n '340,365p'Repository: ericjypark/codex-island
Length of output: 1638
Gemini display name inconsistency — .capitalized preserves hyphens.
The code comment claims "gemini-3.1-pro" → "Gemini 3.1 Pro", but String.capitalized treats hyphens as word delimiters while preserving them. After replacingOccurrences(of: "gemini-", with: "Gemini "), the string becomes "Gemini 3.1-pro". Calling .capitalized then produces "Gemini 3.1-Pro" (hyphen retained), not "Gemini 3.1 Pro" (space) as intended.
Compare this to the Claude branch (lines 347–356 above), which explicitly converts internal dashes to dots for clean version formatting. The Gemini branch needs equivalent normalization.
Suggested fix
// Google: "gemini-3.1-pro" → "Gemini 3.1 Pro"
if canonical.hasPrefix("gemini-") {
- return canonical.replacingOccurrences(of: "gemini-", with: "Gemini ")
- .replacingOccurrences(of: "flash-lite", with: "Flash-Lite")
- .capitalized
+ let trimmed = String(canonical.dropFirst("gemini-".count))
+ let normalized = trimmed
+ .replacingOccurrences(of: "flash-lite", with: "flash\u{2011}lite")
+ .replacingOccurrences(of: "-", with: " ")
+ .replacingOccurrences(of: "flash\u{2011}lite", with: "Flash-Lite")
+ return "Gemini " + normalized.capitalized
}🤖 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/Cost/CostStore.swift` around lines 369 - 374, The Gemini branch in
CostStore.swift currently does canonical.replacingOccurrences(of: "gemini-",
with: "Gemini ").replacingOccurrences(of: "flash-lite", with:
"Flash-Lite").capitalized which leaves internal hyphens (e.g., "3.1-pro") and
yields "Gemini 3.1-Pro"; update this branch to normalize internal dashes before
capitalizing — after the "gemini-" and "flash-lite" replacements also replace
remaining "-" characters (or at least the dash between version and qualifier)
with a space (or convert to the same separator used in the Claude branch), then
call .capitalized; refer to the canonical variable and the replacingOccurrences
calls when making the change.
| if !ev.dedupKey.isEmpty { | ||
| // Overwrite earlier events for the same ID to capture the | ||
| // final turn state (which has the complete output tokens). | ||
| latestById[ev.dedupKey] = event | ||
| } else { | ||
| withoutId.append(event) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -t f LogParseCache.swift
fd -t f LogParseCache.swift --exec rg -n -C3 'func walk|sorted|enumeratorAt|directoryEnumerator'Repository: ericjypark/codex-island
Length of output: 435
🏁 Script executed:
cat -n Sources/Cost/LogParseCache.swift | sed -n '120,200p'Repository: ericjypark/codex-island
Length of output: 2073
🏁 Script executed:
rg -n 'func jsonlFiles' Sources/Cost/Repository: ericjypark/codex-island
Length of output: 131
🏁 Script executed:
cat -n Sources/Cost/LogParseCache.swift | sed -n '17,80p'Repository: ericjypark/codex-island
Length of output: 3305
🏁 Script executed:
cat -n Sources/Cost/GeminiLogReader.swift | head -50Repository: ericjypark/codex-island
Length of output: 2548
🏁 Script executed:
rg -n 'dedupKey|CachedEvent' Sources/Cost/GeminiLogReader.swift | head -20Repository: ericjypark/codex-island
Length of output: 468
🏁 Script executed:
cat -n Sources/Cost/GeminiLogReader.swift | sed -n '96,115p'Repository: ericjypark/codex-island
Length of output: 399
🏁 Script executed:
cat -n Sources/Cost/GeminiLogReader.swift | sed -n '50,95p'Repository: ericjypark/codex-island
Length of output: 2288
Dedup should compare timestamps to avoid depending on file enumeration order.
The unconditional overwrite latestById[ev.dedupKey] = event assumes walk yields events in chronological order. However, jsonlFiles uses FileManager.enumerator without sorting — it returns files in filesystem traversal order (typically alphabetical within directories), not by modification time or date. If a single turn's streaming updates span multiple files, the last-enumerated file may not be the one with the most recent timestamp, causing dedup to keep an intermediate state instead of the final turn state.
Fix: compare timestamps before overwriting:
🔧 Timestamp-aware dedup
if !ev.dedupKey.isEmpty {
- // Overwrite earlier events for the same ID to capture the
- // final turn state (which has the complete output tokens).
- latestById[ev.dedupKey] = event
+ // Keep the event with the latest timestamp per turn id — captures
+ // the final streaming update regardless of file enumeration order.
+ if (latestById[ev.dedupKey]?.timestamp ?? .distantPast) <= event.timestamp {
+ latestById[ev.dedupKey] = event
+ }
} else {
withoutId.append(event)
}📝 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.
| if !ev.dedupKey.isEmpty { | |
| // Overwrite earlier events for the same ID to capture the | |
| // final turn state (which has the complete output tokens). | |
| latestById[ev.dedupKey] = event | |
| } else { | |
| withoutId.append(event) | |
| } | |
| } | |
| if !ev.dedupKey.isEmpty { | |
| // Keep the event with the latest timestamp per turn id — captures | |
| // the final streaming update regardless of file enumeration order. | |
| if (latestById[ev.dedupKey]?.timestamp ?? .distantPast) <= event.timestamp { | |
| latestById[ev.dedupKey] = event | |
| } | |
| } else { | |
| withoutId.append(event) | |
| } | |
| } |
🤖 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/Cost/GeminiLogReader.swift` around lines 36 - 43, The dedup logic
currently unconditionally overwrites latestById[ev.dedupKey] with event (in the
loop that processes walk/jsonlFiles), which relies on enumeration order; change
it to be timestamp-aware: when ev.dedupKey is non-empty, look up the existing
entry in latestById and only replace it if the current event's timestamp is
newer (compare event.timestamp or the event's existing time field) than the
stored event's timestamp (or insert if none exists); keep the else branch
appending to withoutId and ensure you reference latestById, ev.dedupKey, event
(and the event timestamp field) in the update check so final turn state is
chosen by time, not file order.
| let url = URL(string: "https://cloudcode-pa.googleapis.com/v1internal:retrieveUserQuota")! | ||
| var req = URLRequest(url: url) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check if the file exists and get its size
wc -l Sources/Usage/UsageFetcher.swiftRepository: ericjypark/codex-island
Length of output: 104
🏁 Script executed:
# Now let's examine the code around lines 353-354
sed -n '345,360p' Sources/Usage/UsageFetcher.swiftRepository: ericjypark/codex-island
Length of output: 815
🏁 Script executed:
# Let's get more context to understand the function containing these lines
sed -n '340,365p' Sources/Usage/UsageFetcher.swiftRepository: ericjypark/codex-island
Length of output: 1118
🏁 Script executed:
# Check if there are other URL constructions in the file to compare pattern
rg 'URL\(string:' Sources/Usage/UsageFetcher.swift -n -B 1 -A 1Repository: ericjypark/codex-island
Length of output: 916
Remove force-unwrap from URL construction at line 353
Line 353 uses force-unwrap on a URL(string:) call. Replace with guard-let to align with Swift guidelines requiring justification for force-unwraps:
Suggested fix
- let url = URL(string: "https://cloudcode-pa.googleapis.com/v1internal:retrieveUserQuota")!
- var req = URLRequest(url: url)
+ guard let url = URL(string: "https://cloudcode-pa.googleapis.com/v1internal:retrieveUserQuota") else {
+ return errorPair("internal url error")
+ }
+ var req = URLRequest(url: url)Per coding guidelines for **/*.swift: "Do not use force-unwraps in Swift code without justification."
🤖 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/Usage/UsageFetcher.swift` around lines 353 - 354, The URL is being
force-unwrapped when constructing `url` for the request (let url = URL(string:
...)!), which violates Swift guidelines; replace it with a safe unwrap (guard
let url = URL(string: "...") else { handle failure (log/return/throw) }) before
creating `req` so that `req = URLRequest(url: url)` only runs with a valid URL;
update the surrounding function (the block creating `url` and `req`) to handle
the fallback path appropriately (log an error and return or throw) instead of
force-unwrapping.
| guard let obj = try JSONSerialization.jsonObject(with: data) as? [String: Any], | ||
| let buckets = obj["buckets"] as? [[String: Any]] else { | ||
| return errorPair("parse error") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "UsageFetcher.swift" -type f 2>/dev/null | head -5Repository: ericjypark/codex-island
Length of output: 49
🏁 Script executed:
git ls-files | grep -i "usagefetcher"Repository: ericjypark/codex-island
Length of output: 100
🏁 Script executed:
wc -l Sources/Usage/UsageFetcher.swiftRepository: ericjypark/codex-island
Length of output: 104
🏁 Script executed:
sed -n '360,420p' Sources/Usage/UsageFetcher.swiftRepository: ericjypark/codex-island
Length of output: 2601
Replace stringly-typed Gemini quota parsing with Decodable structs to ensure type safety and prevent field drift
The current implementation decodes the Gemini API response using JSONSerialization with [String: Any] casting across lines 373–375 and 392–405. This string-based dictionary access is error-prone: obj["buckets"], d["remainingFraction"], and d["resetTime"] rely on correct spelling without compile-time verification. Using typed Decodable structs eliminates this risk.
Suggested direction
+private struct GeminiQuotaResponse: Decodable {
+ let buckets: [GeminiBucket]
+}
+
+private struct GeminiBucket: Decodable {
+ let modelId: String
+ let remainingFraction: Double?
+ let resetTime: String?
+}Then decode with JSONDecoder() and pass GeminiBucket through a typed parser, eliminating all as? casts for dictionary access.
🤖 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/Usage/UsageFetcher.swift` around lines 373 - 375, The parsing block
that uses JSONSerialization and stringly-typed casts (obj["buckets"],
d["remainingFraction"], d["resetTime"]) should be replaced with Decodable models
and JSONDecoder to get compile-time safety: create a GeminiQuotaResponse:
Decodable { let buckets: [GeminiBucket] } and GeminiBucket: Decodable { let
remainingFraction: Double; let resetTime: Date? /*or String depending on API*/ }
(name types as GeminiBucket/GeminiQuotaResponse), configure
JSONDecoder.dateDecodingStrategy if needed, decode data via
JSONDecoder().decode(GeminiQuotaResponse.self, from: data), then map the decoded
buckets to the existing usage structures and remove all `as?` casts and manual
dictionary indexing; on decode failure, call errorPair with the decoder error to
preserve existing error handling.
| if let s = d["resetTime"] as? String { | ||
| let f = ISO8601DateFormatter() | ||
| f.formatOptions = [.withInternetDateTime] | ||
| resetAt = f.date(from: s) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and read the file to verify the current code
find . -name "UsageFetcher.swift" -type fRepository: ericjypark/codex-island
Length of output: 102
🏁 Script executed:
# Let's search for the file and read the relevant section
rg "resetTime" -B 5 -A 5 --type swiftRepository: ericjypark/codex-island
Length of output: 1765
🏁 Script executed:
# Also search for any tests or API response examples mentioning resetTime
rg "resetTime" -B 3 -A 3Repository: ericjypark/codex-island
Length of output: 1133
🏁 Script executed:
# Let's search for API documentation, tests, or examples that show resetTime format
rg "ISO8601" -B 2 -A 2 --type swiftRepository: ericjypark/codex-island
Length of output: 3850
🏁 Script executed:
# Search for any test files or mock responses
fd -e json -e txt -e md | xargs rg -l "resetTime" 2>/dev/null || echo "No JSON/TXT/MD files with resetTime found"Repository: ericjypark/codex-island
Length of output: 109
🏁 Script executed:
# Look for documentation about the API response structure
rg "reset|Reset" --type md -B 2 -A 2 | head -50Repository: ericjypark/codex-island
Length of output: 5085
🏁 Script executed:
# Check if there are any comments or documentation about the API format
rg "formatOptions" -B 5 -A 5 --type swiftRepository: ericjypark/codex-island
Length of output: 5789
resetTime parsing should handle fractional-second ISO-8601 values
The code only uses .withInternetDateTime; if the API returns fractional seconds, resetAt becomes nil. The same file already handles this correctly for the resets_at field—apply the same pattern here.
Suggested fix
var resetAt: Date?
if let s = d["resetTime"] as? String {
- let f = ISO8601DateFormatter()
- f.formatOptions = [.withInternetDateTime]
- resetAt = f.date(from: s)
+ let withFractional = ISO8601DateFormatter()
+ withFractional.formatOptions = [.withInternetDateTime, .withFractionalSeconds]
+ let noFractional = ISO8601DateFormatter()
+ noFractional.formatOptions = [.withInternetDateTime]
+ resetAt = withFractional.date(from: s) ?? noFractional.date(from: s)
}📝 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.
| if let s = d["resetTime"] as? String { | |
| let f = ISO8601DateFormatter() | |
| f.formatOptions = [.withInternetDateTime] | |
| resetAt = f.date(from: s) | |
| } | |
| if let s = d["resetTime"] as? String { | |
| let withFractional = ISO8601DateFormatter() | |
| withFractional.formatOptions = [.withInternetDateTime, .withFractionalSeconds] | |
| let noFractional = ISO8601DateFormatter() | |
| noFractional.formatOptions = [.withInternetDateTime] | |
| resetAt = withFractional.date(from: s) ?? noFractional.date(from: s) | |
| } |
🤖 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/Usage/UsageFetcher.swift` around lines 399 - 403, The parsing of the
"resetTime" string uses an ISO8601DateFormatter with only .withInternetDateTime,
which fails for fractional-second timestamps; update the formatter setup where
"resetTime" is parsed (the block that assigns resetAt) to include the same
formatOptions used for the "resets_at" parsing (i.e., include
.withFractionalSeconds in addition to .withInternetDateTime) so
fractional-second ISO‑8601 values are correctly parsed by the
ISO8601DateFormatter.
|
|
Thanks for putting work into this, but I can’t merge this in the current state. The main issue is that this PR is too broad and the UX still looks unfinished. From the screenshots the single-provider / multi-provider layouts don’t look stable yet. I also don’t want a provider PR to rewrite or change existing behavior. Adding Gemini should be mostly additive and isolated behind the provider abstraction. Could you split this into smaller PRs?
Before I can review Gemini support, I’d need:
I’m happy to review a smaller Gemini provider PR, but this one changes too much at once and the UI isn’t ready yet. |
2 participants
Adds full support for Google Gemini as a provider, including:
cloudcode-pa.googleapis.comto track quota consumption, defaulting to the 'PRO' plan.UnevenRoundedRectangle) and strict concurrency warnings.Summary by CodeRabbit
New Features
Refactor