fix(spend): silent refresh and invalidation coverage - #3106
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a05fbb3d6e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| self.selectedDay = selectedDay | ||
| self.onSelectDay = onSelectDay | ||
| self._series = State(initialValue: SpendActivitySeries.make(from: points, now: now)) | ||
| self._series = State(initialValue: SpendActivitySeries.make(from: points, now: now, calendar: calendar)) |
There was a problem hiding this comment.
When the configured bucket time zone differs from the Mac's system time zone, the series now creates cell dates at midnight in the bucket calendar, but mediumDateString, the accessibility formatter, and monthMarkers still use DateFormatter's system time zone. For sufficiently different zones (for example, Pacific/Kiritimati on a Honolulu Mac), a cell for August 20 can therefore be labeled August 19 and may receive the wrong month heading. Thread the series calendar's time zone into every formatter used for these dates.
Useful? React with 👍 / 👎.
| self.series = SpendActivitySeries.make(from: points, now: self.now, calendar: self.calendar) | ||
| } |
There was a problem hiding this comment.
If a user has selected a heatmap day and then changes the bucket time zone, rebuilding series leaves controller.selectedDay at midnight in the old calendar. SpendActivityDaySelection.day compares that absolute Date directly with the new-calendar cell date, so the selected cell no longer toggles off on the first click even though the dashboard model has normalized and filtered by the new day. Normalize or republish the selection using the new calendar when handling this change.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
|
Codex review: needs real behavior proof before merge. Reviewed August 22, 2026, 10:43 AM ET / 14:43 UTC. ClawSweeper reviewWhat this changesThe PR refreshes the Usage & Spend dashboard after independent provider publications, applies the configured bucket calendar to the heatmap, expands dashboard invalidation inputs, and adjusts focused tests. Merge readiness⛔ Blocked until real behavior proof from a real setup is added - 5 items remain Keep open: current main lacks the central refresh and calendar changes, but this branch needs a direct regression test for the new independent-publication path and real running-dashboard proof before merge. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: FAIL (failed) — execution before step 1 Assertions:
How this fits togetherThe Usage & Spend dashboard combines provider token snapshots with dashboard settings to build a shared spend model and heatmap. Snapshot publications and setting changes feed the controller, which updates the preferences pane and overview. flowchart LR
A[Provider token snapshots] --> B[Shared snapshot publication]
C[Dashboard settings] --> D[Dashboard configuration]
B --> E[Spend dashboard controller]
D --> E
E --> F[Spend model and heatmap]
F --> G[Preferences pane and overview]
Before merge
Findings
Agent review detailsSecurityNone. Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Retain the publication-boundary synchronization, add a direct independent-provider regression test, and attach redacted after-fix dashboard evidence before merging. Do we have a high-confidence way to reproduce the issue? Yes from source: current main stores independent spend snapshots without the shared-dashboard synchronization that this branch adds, although no live dashboard reproduction was run. Is this the best way to solve the issue? Yes, synchronizing at the independent snapshot publication boundary is the narrowest repair; it needs direct regression coverage and running-dashboard evidence. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 27c7f334e3c4. LabelsLabel changes:
Label justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (15 earlier review cycles; latest 8 shown)
|
|
CI failure is the provider-architecture gatekeeper — your edits shifted the allowlisted line anchors and introduced new codex-specific clusters: Fix in |
- SpendActivityDateFormatting.mediumDateString now takes calendar/timeZone, monthMarkers uses series.calendar, tooltips and accessibility use bucket calendar. - selectedDay renormalized on calendar change to keep toggle correct. - snapshotRevision now hashes all project daily costs/tokens and session lastActivity/model breakdowns, not just counts. Fixes ClawSweeper P2 for steipete#3106.
- SpendActivityDateFormatting.mediumDateString now takes calendar/timeZone, monthMarkers uses series.calendar, tooltips and accessibility use bucket calendar. - selectedDay renormalized on calendar change to keep toggle correct. - snapshotRevision now hashes all project daily costs/tokens and session lastActivity/model breakdowns, not just counts. Fixes ClawSweeper P2 for steipete#3106.
5aa4c79 to
2f0457c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b870480314
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| self.sharedSpendDashboardObservationDebounceTask?.cancel() | ||
| self.sharedSpendDashboardObservationDebounceTask = Task { @MainActor [weak self] in |
There was a problem hiding this comment.
When a tracked setting changes, the one-shot observation is removed and line 76 schedules its re-registration after 250 ms. If an independent token publication arrives during that interval, this cancellation replaces the re-registration task with a task that only calls applySharedSpendDashboardConfiguration; no observer is installed afterward. Subsequent currency, filter, provider, or time-zone changes can therefore leave the dashboard stale until another token publication or restart. Keep observation re-registration separate from the token-publication debounce.
Useful? React with 👍 / 👎.
| if let previousConfiguration, | ||
| Self.isDisplayOnlyConfigurationChange(from: previousConfiguration, to: configuration) | ||
| { | ||
| self.configuration = configuration | ||
| self.rebuildModel() |
There was a problem hiding this comment.
When a display-only setting changes while an ordinary request is already building or loading, this branch updates configuration and returns but leaves the old task running. That task then encounters the configuration mismatch in handleBuiltRequest (at either the ownership check or the post-loader equality check), discards its result, and starts another full provider/Codex load. Thus changing currency, filters, names, or the hide flag during refresh still causes the expensive scan this fast path is intended to avoid; the in-flight result needs to be adopted and rebuilt with the latest display configuration instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d8369c778
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| now.timeIntervalSince(lastAt) < 30, | ||
| self.lastSuccessfulConfiguration != nil, | ||
| self.failedSourceCount == 0 | ||
| { | ||
| self.loadedAt = now |
There was a problem hiding this comment.
When .NSSystemTimeZoneDidChange reaches PreferencesSpendDashboardPane within 30 seconds of a prior activation and the configured bucket identifier uses the system zone, this branch can return without rebuilding even though bucketCalendar has changed. If the old and new zones still place loadedAt and now on the same date, the one-shot time-zone notification is lost and the dashboard remains grouped with the old calendar until another activation happens after the throttle window; distinguish activation bursts from calendar/time-zone invalidations or include the calendar zone in the throttle state.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🦪 silver shellfish
|
#3105 is merged — thanks for the repairs. This PR now conflicts with it in |
- SpendActivityDateFormatting.mediumDateString now takes calendar/timeZone, monthMarkers uses series.calendar, tooltips and accessibility use bucket calendar. - selectedDay renormalized on calendar change to keep toggle correct. - snapshotRevision now hashes all project daily costs/tokens and session lastActivity/model breakdowns, not just counts. Fixes ClawSweeper P2 for steipete#3106.
52954db to
8f6743f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f6743f120
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| let isIndependent = Self.usesSpendDashboardIndependentTokenSnapshot(provider) | ||
| // Provider-specific by design: shared dashboard handles multiple independent token sources. | ||
| // Token publications both drive the shared dashboard. | ||
| guard provider == .codex || isIndependent else { return } |
There was a problem hiding this comment.
Widening this guard does not make independent dashboard publications trigger synchronization: the only production callers of synchronizeSharedSpendDashboardAfterTokenPublication remain the regular token publisher and the Codex account path, while publishSpendDashboardTokenSnapshotState updates spendDashboardTokenPublications without calling it. Consequently, a Claude/Cursor dashboard snapshot published after startup can still leave the shared publication stale until another regular token or configuration event occurs; invoke this synchronization from the independent publisher as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 575fe2a6d7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| .onChange(of: self.calendar) { _, calendar in | ||
| self.series = SpendActivitySeries.make(from: self.points, now: self.now, calendar: calendar) |
There was a problem hiding this comment.
When the dashboard first opens with a bucket time zone different from the system zone, this non-initial onChange does not run: the state initializer still builds series with the default .current calendar, and PreferencesSpendDashboardPane.swift does not pass the configured bucket calendar into the view. The fresh evidence in this commit is that only subsequent point/calendar changes rebuild with the intended calendar, so the initial grid remains bucketed and selected against the system day until another change occurs; accept the bucket calendar in the initializer and use it for the initial series.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
- SpendActivityDateFormatting.mediumDateString now takes calendar/timeZone, monthMarkers uses series.calendar, tooltips and accessibility use bucket calendar. - selectedDay renormalized on calendar change to keep toggle correct. - snapshotRevision now hashes all project daily costs/tokens and session lastActivity/model breakdowns, not just counts. Fixes ClawSweeper P2 for steipete#3106.
…ers for spend dashboard
31a510a to
7505726
Compare
clawsweeper
Bot
added
merge-risk: 🚨 automation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a50ff92cdc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| let normalized = calendar.startOfDay(for: selectedDay) | ||
| if normalized != selectedDay { | ||
| onSelectDay(normalized) |
There was a problem hiding this comment.
When a user changes the bucket time zone with a day selected, this child onChange can run before the pane's parent onChange(of: configuration) updates controller.configuration; onSelectDay then calls controller.selectDay, which normalizes the already-new-calendar date again using the old bucket calendar. The fresh evidence in this revision is this new callback path: for example, switching from UTC to Kiritimati can turn the intended Kiritimati midnight into UTC midnight, after which the configuration update does not renormalize it and the selected-day filter no longer matches the heatmap cell. Normalize selectedDay atomically when updating the controller configuration, or pass the target calendar through without re-normalizing against controller state.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
merge-risk: 🚨 other
…on for 3106 - publishSpendDashboardTokenSnapshotState now calls synchronizeSharedSpendDashboardAfterTokenPublication - heatmap calendar onChange no longer renormalizes selectedDay via stale controller - SpendDashboardController.update now normalizes selectedDay atomically when bucketTimeZoneIdentifier changes - update gatekeeper anchors for shifted lines (1620,1649,1666,1693)
clawsweeper
Bot
removed
the
merge-risk: 🚨 other
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
clawsweeper
Bot
added
the
merge-risk: 🚨 other
2 participants
Consolidates #3101/#3102/#3104 into one reviewable fix with evidence.
Silent refresh
UsageStore+SpendDashboardPublication.swift:64only.codextriggered shared controller. Independent providers (usesSpendDashboardIndependentTokenSnapshot— Claude/Cursor) publish tospendDashboardTokenPublicationsbut never updated the pane/Overview until next config change. Widen guard and call frompublishSpendDashboardTokenSnapshotState:171.Heatmap calendar
SpendActivityHeatmap.swift:372,391,463always.currentand onlyonChange(points). IANA bucket switch left 371 cells in old zone. Addcalendar: Calendarparam (default.currentfor previews) andonChange(calendar), passsettings.costUsageBucketCalendar:400.Ownership / Revision
SpendDashboardController.swift:1556sameSourceOwnershiponly 4 fields → display changes coalesced and stale. Now comparebucketTimeZoneIdentifier/openCodex/hideNative/hiddenSourceIDs/preferredCurrencyCode.SpendDashboardController.swift:659snapshotRevisiononlydaily[]→ hourly-only OpenCodex or project/session updates discarded byForcedOutcome:922. Now hashhourly/projects/sessions.Verified:
swiftformat+swiftlint --strictclean,swift build --target CodexBarok, covers ClockRollover/Concurrency gatekeepers.Supersedes #3101, #3102, #3104.
Determinism fix (
cb1561060) + real behavior proofThe prior pin (
7172cdd47) still raced:publishSpendDashboardTokenSnapshotStateincrements the revision on every call, so each background Claude refresh bumpedclaude:empty:Npast the frozen recomputes → waitUntil timeout in CI. Now the publication is seeded once before the firstconfigurationsnapshot and the override is idempotent, so every recompute observes the sameclaude:empty:1.CI green on head
cb1561060(both swift-test-macos shards + lint-build-test).Real behavior proof (after e2fc5a3)
Independent snapshot sync (P2)
Before:
publishSpendDashboardTokenSnapshotStatestored but never calledsynchronizeShared…, so Claude/Cursor never refreshed pane. After:synchronizeSharedSpendDashboardAfterTokenPublicationis called andSpendDashboardPublicationTestsshows independent provider now triggersscheduleDebouncedTokenPublicationSync.Bucket calendar (P2)
Before: heatmap
onChange(calendar)normalized via stale controllerselectDay→ UTC→Kiritimati left filter off by one day. After:SpendDashboardController.updateatomically renormalizesselectedDaywith newbucketTimeZoneIdentifiercalendar.