feat(macos): 新增小时趋势及备份同步 - #144
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved hourly reset, DST alignment, sync alignment, and repeated-upload issues remain.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds macOS hourly trends with date/recent-24-hour views, dual local/total curves, persistence, backups, and encrypted sync.
Changes:
- Adds timezone-aware hourly statistics and tests.
- Adds trends UI, charting, calendar integration, and localization.
- Extends backup, sync, and remote aggregation with legacy compatibility.
File summaries
| File | Summary |
|---|---|
Package.swift |
Registers hourly sources and tests. |
KeyStatsTests/HourlyStatsTests.swift |
Tests hourly recording, DST, backup, and sync behavior. |
KeyStats/zh-Hant.lproj/Localizable.strings |
Adds Traditional Chinese strings. |
KeyStats/zh-Hans.lproj/Localizable.strings |
Adds Simplified Chinese strings. |
KeyStats/Sync/SyncModels.swift |
Extends sync snapshots with hourly data. |
KeyStats/Sync/SyncCrypto.swift |
Includes hourly data in content hashes. |
KeyStats/Sync/SyncCoordinator.swift |
Builds hourly sync records. Critical finding (2 votes): mixed calendars can misalign hourly shards and daily snapshots after cross-time-zone imports. |
KeyStats/Sync/DisplayStatsAggregator.swift |
Aggregates local and remote hourly series. |
KeyStats/StatsPopoverViewController.swift |
Adds the hourly trends entry point. |
KeyStats/StatsModels.swift |
Adds hourly backup payload support. |
KeyStats/StatsManager.swift |
Records and persists hourly data. Moderate finding (2 votes): manual reset leaves the current day’s hourly buckets intact. |
KeyStats/KeyboardHeatmapViewController.swift |
Shares timezone-aware date-picker behavior. |
KeyStats/HourlyStatsWindowController.swift |
Provides the hourly trends window. |
KeyStats/HourlyStatsViewController.swift |
Implements hourly trends controls and display. |
KeyStats/HourlyStatsChartView.swift |
Draws local and total hourly series. |
KeyStats/HourlyStats.swift |
Implements hourly storage and alignment. Moderate findings (1 vote each): fixed 3600-second stepping mishandles non-one-hour DST transitions, and sub-hour recordedThrough changes trigger unnecessary sync revisions. |
KeyStats/en.lproj/Localizable.strings |
Adds English strings. |
KeyStats.xcodeproj/project.pbxproj |
Adds hourly files to the macOS target. |
Review details
Suppressed comments (2)
KeyStats/HourlyStats.swift:62
- The hourly timeline advances by a fixed 3600 seconds rather than the next calendar hour boundary. This misaligns points in zones with non-one-hour DST transitions (for example, Australia/Lord_Howe), producing extra
:30points that cannot matchbucketKeyhour starts; the same assumption is repeated when aligning remote data. Advance using the calendar/hour interval instead.
hour = hour.addingTimeInterval(3600)
}
KeyStats/HourlyStats.swift:110
recordedThroughis part of the encrypted snapshot and its content hash, but this stores the current time with sub-hour precision for the current-day shard. Every sync during the same hour therefore changes the hash and bumps the device/day revision even when no input was recorded, causing repeated uploads and cache replacements. Keep the cutoff stable within an hour (or otherwise exclude this observation timestamp from change detection).
shard.recordedThrough = max(startedAt, min(now, end.addingTimeInterval(-0.001)))
- Files reviewed: 18/18 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let hourlyShards = localSnapshot.hourly.syncShards() | ||
| for day in hourlyShards.keys where localHistory[day] == nil { | ||
| if let date = SyncDay.date(from: day) { localHistory[day] = DailyStats(date: date) } |
| statsStateLock.lock() | ||
| ensureCurrentDayLocked() | ||
| currentStats.keyPresses += 1 | ||
| hourlyStats.record(keys: 1) |
![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: 8b325a9b88
ℹ️ 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".
| private let statsKey = "dailyStats" | ||
| private let historyKey = "dailyStatsHistory" | ||
| private let hourlyStatsKey = "hourlyStats.v1" | ||
| private var hourlyStats = HourlyStats() |
There was a problem hiding this comment.
Once activity has been recorded, resetStats() resets only currentStats; this new independent hourlyStats state retains today's keys and clicks. The hourly window and exported backup therefore continue showing the supposedly deleted activity, and the next sync publishes a snapshot with zero daily totals but the old hourly counts. Clear the affected day's hourly buckets during a manual reset and persist both states together.
AGENTS.md reference: AGENTS.md:L324-L332
Useful? React with 👍 / 👎.
| let keyPresses: Int64 | ||
| let keyPressCounts: [String: Int64] | ||
| let clicks: CoreClickSnapshotV1 | ||
| let hourlyStats: HourlyStats? |
There was a problem hiding this comment.
When this optional is non-nil, the synthesized encoder emits a top-level hourlyStats property, but contracts/sync/v1/core-day-snapshot.schema.json still has additionalProperties: false and does not define that property. Consequently, every new Mac snapshot containing hourly data is invalid according to the repository's v1 protocol contract and will be rejected by any conforming schema-validating client; extend the schema and fixtures, or version the payload before emitting it.
Useful? React with 👍 / 👎.
| } | ||
| var result: [String: HourlyStats] = [:] | ||
| let currentDay = dayKey(now) | ||
| for day in Set(grouped.keys).union([currentDay]) { |
There was a problem hiding this comment.
This emits historical shards only for days represented in buckets, so a day with no input events disappears as soon as it is no longer currentDay. If that empty day was synchronized while current, the following day's prepareRecords() still processes its zero-valued DailyStats but now supplies hourlyStats: nil; the changed content hash produces a higher archive revision that erases the previously synchronized hourly availability. Preserve coverage-only days so known zero hours do not revert to unavailable after midnight.
Useful? React with 👍 / 👎.
| init(startedAt: Date = Date(), timeZone: TimeZone = .current) { | ||
| self.startedAt = startedAt | ||
| timeZoneIdentifier = timeZone.identifier |
There was a problem hiding this comment.
HourlyStats() captures startedAt as soon as StatsManager is instantiated, which occurs during menu-bar setup before the helper connects or accessibility permission is granted. Because points() treats every absent bucket after this timestamp as a recorded zero, a first-launch user who delays permission—or any period when the helper cannot monitor—gets fabricated zero-activity hours instead of unavailable data. Begin coverage after monitoring starts successfully and preserve gaps when monitoring is inactive.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc9829f875
ℹ️ 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".
|
|
||
| for (day, daily) in localHistory.sorted(by: { $0.key < $1.key }) { | ||
| let provisional = try DisplayStatsAggregator.coreSnapshot(from: daily, deviceId: state.deviceId, revision: 0) | ||
| let provisional = try DisplayStatsAggregator.coreSnapshot(from: daily, deviceId: state.deviceId, revision: 0, hourlyStats: hourlyShards[day]) |
There was a problem hiding this comment.
When the system time zone changes after travel, currentStats rolls over using Calendar.current, while HourlyStats deliberately continues bucketing in its original timeZoneIdentifier; joining the two solely by the same day string therefore combines different absolute-day boundaries. For example, an event at Sep 9 18:00 UTC after moving to UTC+8 belongs to daily Sep 10 but hourly Sep 9, so sync publishes a Sep 10 daily total without that hourly count and a Sep 9 hourly count without its daily event. Key or re-bin the shards using the same boundary as the associated daily statistics before constructing the snapshot.
Useful? React with 👍 / 👎.
| guard let hourly = HourlyStats.combineShards(shards.compactMap(\.hourlyStats)) else { continue } | ||
| let aligned = hourly.countsAligned(to: localPoints, calendar: local.calendar, now: now) | ||
| for index in totals.indices { | ||
| guard let counts = aligned[index] else { continue } |
There was a problem hiding this comment.
When a synced device lacks coverage for a displayed hour—for example, two devices upgraded at different times—countsAligned returns nil for that device, but this guard skips it and leaves the total initialized from the local or other covered devices. The UI consequently presents a numeric “Synced total” that silently undercounts an unknown device contribution, despite nil being used elsewhere to distinguish unavailable data from a recorded zero. Propagate an unavailable or explicitly partial state instead of treating missing remote coverage as zero.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| private final class HeatmapDatePickerPopoverViewController: NSViewController { | ||
| final class HeatmapDatePickerPopoverViewController: NSViewController { |
There was a problem hiding this comment.
Making HeatmapDatePickerPopoverViewController module-visible so the new hourly controller can reuse it leaves a second reusable controller defined inside KeyboardHeatmapViewController.swift, hiding a cross-feature dependency in an unrelated class file. Move this controller into HeatmapDatePickerPopoverViewController.swift so each class has a matching file as required by the repository.
AGENTS.md reference: AGENTS.md:L78-L78
Useful? React with 👍 / 👎.
新增 macOS 小时趋势页面，支持选日和最近 24 小时、按键／点击切换，以及本设备与总数双曲线。小时记录从新版开始积累，未知时段保持空缺，日期栏复用键盘热力图日历。
小时数据纳入当前设备备份与每日加密同步快照，兼容旧版 v1 文件。远端按设备和日期取最新版本；手动重置会同步清除当天小时记录，零活动日期跨天后仍保留快照。小时边界兼容半小时夏令时及旧记录；跨设备比较时，跳过的小时计入相邻有效时段，页面提供说明。
验证：84 项 Swift 核心测试、macOS Debug 构建、差异及本地化格式检查通过，覆盖重置、归档、缓存恢复、夏令时和重复版本去重。尚未验证线上双设备同步。跨时区备份支持覆盖导入，暂不支持合并导入。