fix(iOS): port minute-block chime behavior from web#177
Merged
Conversation
Port the web minute-boundary chime behavior so iOS chimes follow completed minute blocks, skip final-minute blocks, and never replay after mute toggles or resume. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📝 WalkthroughWalkthroughPort of minute-block chime behavior from web to iOS: adds SessionLogic minute-block APIs, updates SessionViewModel to track completed minute blocks (seeded correctly on resume vs fresh start) and call new chime update API per tick, adds macOS-safe AudioEngine stubs, and introduces unit tests and SPM test target. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "UI / Timer UI"
participant VM as "SessionViewModel"
participant Logic as "SessionLogic"
participant Audio as "AudioEngine"
UI->>VM: start / resume / tick events
VM->>Logic: completedMinuteBlockIndex(elapsed,totalSeconds) (seed on resume)
VM->>Logic: nextMinuteChimeUpdate(elapsed,totalSeconds,lastCompletedBlockIndex) each tick
alt completed block index advanced
VM->>Audio: playChime(count) [if soundPrefs.chime]
VM->>VM: update lastCompletedMinuteBlockIndex
else no advancement
VM-->>UI: update UI (no chime)
end
VM->>Audio: playTick() per-second (if enabled)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. 📋 Issue PlannerBuilt with CodeRabbit's Coding Plans for faster development and fewer bugs. View plan used: ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (1)
ios/StillPointShared/Sources/StillPointShared/SessionLogic.swift (1)
49-50: Keep the remaining-minute math integer-only.
blockEndandtotalSecondsare alreadyInt, so converting toDoublehere reintroduces floating-point boundary math in the middle of a port that is explicitly trying to avoid it. Integer division preserves the current behavior and keeps this calculation exact.Proposed change
guard blockIndex >= 0 else { return nil } let blockEnd = (blockIndex + 1) * 60 - let chimeCount = Int(floor(Double(totalSeconds - blockEnd) / 60.0)) + let chimeCount = (totalSeconds - blockEnd) / 60 return chimeCount >= 1 ? chimeCount : nil🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointShared/Sources/StillPointShared/SessionLogic.swift` around lines 49 - 50, The calculation for chimeCount introduces floating-point math by casting to Double and using floor; replace this with integer-only arithmetic to preserve exactness: compute blockEnd as (blockIndex + 1) * 60 and set chimeCount using integer division on (totalSeconds - blockEnd) (e.g., (totalSeconds - blockEnd) / 60), taking care to handle negative results if necessary; update the code referencing blockEnd, blockIndex, totalSeconds, and chimeCount accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@ios/StillPointShared/Package.swift`:
- Around line 15-18: The test target is untestable on macOS because the package
declares iOS/watchOS platforms while AudioEngine.swift imports AVFoundation
directly; either gate the AVFoundation usage with conditional compilation in
AudioEngine.swift (e.g., wrap AVFoundation import and AV-specific types with `#if`
canImport(AVFoundation) / `#endif`) or extract the AVFoundation-dependent code
into a separate iOS-only target in Package.swift and have the main
StillPointShared target contain only platform-neutral code; update the
.testTarget to depend on the platform-neutral StillPointShared core target (or
add tests for the core target) so swift test runs on CI without requiring
iOS-only APIs.
---
Nitpick comments:
In `@ios/StillPointShared/Sources/StillPointShared/SessionLogic.swift`:
- Around line 49-50: The calculation for chimeCount introduces floating-point
math by casting to Double and using floor; replace this with integer-only
arithmetic to preserve exactness: compute blockEnd as (blockIndex + 1) * 60 and
set chimeCount using integer division on (totalSeconds - blockEnd) (e.g.,
(totalSeconds - blockEnd) / 60), taking care to handle negative results if
necessary; update the code referencing blockEnd, blockIndex, totalSeconds, and
chimeCount accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6ddaa812-1430-4839-bcf1-0e9cb2d26250
📒 Files selected for processing (4)
ios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointShared/Package.swiftios/StillPointShared/Sources/StillPointShared/SessionLogic.swiftios/StillPointShared/Tests/StillPointSharedTests/SessionLogicMinuteChimeTests.swift
Use integer division for minute-block chime counts to match web semantics without float boundaries. Gate AVFoundation behind non-macOS so swift test can compile on macOS hosts; declare macOS v14 in the package manifest. Co-authored-by: Bretton Auerbach <auerbachb@users.noreply.github.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
ios/StillPointShared/Sources/StillPointShared/AudioEngine.swift (1)
15-37: Consider de-duplicating sound prefs logic across compile branches.
SoundPrefs,prefsKey,loadPrefs(), andsavePrefs(_:)are duplicated in both branches. This is easy to drift over time; extracting shared prefs code outside the#if/#elseblock would reduce maintenance risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointShared/Sources/StillPointShared/AudioEngine.swift` around lines 15 - 37, The SoundPrefs struct and its persistence helpers are duplicated across compile branches; remove the duplicates by extracting the shared declarations (SoundPrefs, prefsKey, loadPrefs(), savePrefs(_:) ) out of the `#if/`#else blocks and into a single common section so both branches use the same types and functions; keep any branch-specific behavior inside the conditional blocks but reference the unified SoundPrefs and its methods from there to avoid drift.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@ios/StillPointShared/Sources/StillPointShared/AudioEngine.swift`:
- Around line 15-37: The SoundPrefs struct and its persistence helpers are
duplicated across compile branches; remove the duplicates by extracting the
shared declarations (SoundPrefs, prefsKey, loadPrefs(), savePrefs(_:) ) out of
the `#if/`#else blocks and into a single common section so both branches use the
same types and functions; keep any branch-specific behavior inside the
conditional blocks but reference the unified SoundPrefs and its methods from
there to avoid drift.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1d335b96-24a4-446f-96bd-2d5ff750b6c7
📒 Files selected for processing (3)
ios/StillPointShared/Package.swiftios/StillPointShared/Sources/StillPointShared/AudioEngine.swiftios/StillPointShared/Sources/StillPointShared/SessionLogic.swift
🚧 Files skipped from review as they are similar to previous changes (2)
- ios/StillPointShared/Package.swift
- ios/StillPointShared/Sources/StillPointShared/SessionLogic.swift
This was referenced Apr 22, 2026
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
2 participants
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.
Closes #78
Summary
StillPointSharedregression tests covering 4:20 and 5:00 sequences, <1 minute/final-minute exclusions, <=120s sessions, mute independence, resume seeding, and fresh-start resetManual test matrix
Validation run
xcodebuild -scheme StillPointShared -destination 'generic/platform=iOS' buildNotes
swift teston this machine compiles the package for macOS host and fails inAudioEnginedue to iOS-only AVFoundation APIs; iOS scheme build passes.Made with Cursor
Summary by CodeRabbit
New Features
Tests