Add native iOS app (SwiftUI) with full feature parity#21
Merged
Conversation
Scaffold the complete iPhone app matching all 8 web views: - Auth (login/signup), Home (day counter with breathing animation), Session (countdown timer, block grid, mind state toggle, thought capture), Completion (stats, thoughts, session note), History (aggregate stats, stacked bar chart), Thought Journal, Public Board (leaderboard), and Settings (public toggle, logout) Architecture: - StillPointShared Swift Package: SwiftData models (User, Session, Thought), APIClient (cookie-based auth matching web API), AudioEngine (AVAudioEngine synthesis matching Web Audio API sounds), SessionLogic (block building algorithm ported from BlockTimer.tsx), DTOs, Constants - StillPointApp iPhone target (iOS 17+): MVVM with @observable, TabView navigation, full dark theme design system matching web CSS custom properties - Xcode project generated via xcodegen (project.yml included) Closes #20 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Owner
Author
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds a complete iOS SwiftUI app and shared Swift package: Xcode project/workspace, app entry, eight screens, reusable components, MVVM view models, SwiftData models, API client, synthesized audio engine, session/block logic, design tokens, and asset catalog entries. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HomeView
participant AppVM
participant SessionView
participant SessionVM
participant SessionLogic
participant AudioEngine
participant APIClient
User->>HomeView: tap "Begin"
HomeView->>AppVM: beginSession()
AppVM->>SessionView: present session
SessionView->>SessionVM: start()
loop session running
SessionVM->>SessionLogic: buildBlocks / calculateClearPercent / statusLabel
SessionVM->>AudioEngine: playTick / playChime / playCompletion
SessionVM-->>SessionView: update UI (elapsed, blocks, status)
end
User->>SessionView: capture thought / toggle mind state
SessionView->>SessionVM: captureThought()/toggleMindState()
SessionVM->>APIClient: createSession + batchThoughts (save)
APIClient-->>SessionVM: SessionDTO / ThoughtDTOs
SessionVM->>AppVM: completeSession(...)
AppVM->>SessionView: navigate to CompletionView
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
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: 28
🤖 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/project.yml`:
- Around line 1-37: The file ending for the XcodeGen config (project.yml
containing name: StillPoint and targets: StillPoint with settings like
SWIFT_VERSION: "5.9") is missing a trailing newline; open the ios/project.yml
and add a single newline character at the end of the file so the file ends with
a blank line.
In `@ios/StillPoint.xcodeproj/project.pbxproj`:
- Line 394: Project-level SWIFT_VERSION is set to 5.0 while targets use
SWIFT_VERSION = 5.9; update the project-level SWIFT_VERSION entry to 5.9 (or
regenerate via xcodegen with consistent settings) so the project-wide setting
matches the target-level Swift version and avoids future inconsistencies when
adding targets; locate the SWIFT_VERSION key in the project build settings block
and change its value from 5.0 to 5.9.
In `@ios/StillPointApp/Components/BlockGridView.swift`:
- Around line 106-109: The pulseOpacity() function in BlockGridView currently
creates a Timer as a side effect (but never mutates pulsePhase), so remove the
timer scheduling from pulseOpacity() and instead drive the pulsing with SwiftUI
animation state: bind the border opacity to pulsePhase (or an explicit `@State`
Bool) and set that state in the view's .onAppear using
withAnimation(.easeInOut(duration: 0.8).repeatForever(autoreverses: true)) {
pulsePhase.toggle() } (or set to true if using implicit animation), ensuring
pulseOpacity() becomes a pure computed value (e.g., return pulsePhase ? 1 : 0.4)
and delete the empty Timer creation; update uses around isCurrent rendering so
the RoundedRectangle stroke reads its opacity from the animated state.
In `@ios/StillPointApp/Components/MindStateBarView.swift`:
- Around line 21-29: The divisions computing startFraction and endFraction can
divide by zero when totalSeconds == 0; update the ForEach/mapping logic in
MindStateBarView to guard against totalSeconds == 0 (from
mindStateLog/enumerated, entry/time and elapsed) by either
early-returning/skipping rendering when totalSeconds is zero or by computing a
safeDenominator = Double(max(1, totalSeconds)) and using that for divisions, and
ensure startFraction and endFraction are clamped within 0...1 so rendering code
that consumes those values remains stable.
- Around line 6-9: The parameter currentState on MindStateBarView is declared
but not used; either remove it from MindStateBarView's properties and
initializer, or use it to highlight the matching entry by comparing currentState
to each MindStateEntry (e.g., in the view body iterate mindStateLog and when
entry.state == currentState apply distinctive styling such as a different
background color, border, or opacity). Update the MindStateBarView struct, its
initializer, and any call sites accordingly to keep the API consistent with your
chosen approach.
In `@ios/StillPointApp/Components/ThoughtCaptureView.swift`:
- Around line 32-45: The handlers currently treat whitespace-only input as
valid; update both the onSubmit closure and the Button action to trim the input
(use text.trimmingCharacters(in: .whitespacesAndNewlines)) and validate the
trimmed string before calling onCapture or onDismiss so that inputs containing
only spaces/newlines are treated as empty; locate this logic around the onSubmit
block and the Button action that currently checks if !text.isEmpty and replace
those checks with the trimmed-string version.
In `@ios/StillPointApp/Navigation/MainTabView.swift`:
- Around line 40-46: The UITabBarAppearance is being set inside MainTabView's
onAppear, causing global UITabBar.appearance() to be reconfigured every time the
view appears; move this setup to the app launch (e.g., StillPointApp init or a
dedicated AppearanceConfigurator) so it runs once at startup, then remove the
onAppear block from MainTabView; locate the onAppear in MainTabView that calls
UITabBar.appearance() and copy the appearance creation/configuration there into
the StillPointApp initializer (or a static initializer) and ensure only a single
initialization path configures UITabBar.appearance().
In
`@ios/StillPointApp/Resources/Assets.xcassets/AppIcon.appiconset/Contents.json`:
- Around line 2-8: The AppIcon entry in Contents.json is missing a "filename"
property for the 1024x1024 image, so add a "filename" field referencing the
bundled PNG (e.g., "AppIcon-1024.png") inside the images array entry where
"idiom":"universal" and "size":"1024x1024" is defined, or if icon omission is
intentional add a TODO comment and include a placeholder PNG in the asset
catalog; ensure the referenced filename matches the actual file placed into the
app icon asset set.
In `@ios/StillPointApp/Theme/DesignTokens.swift`:
- Around line 77-89: The custom font references used by DesignTokens (serif,
serifItalic, mono functions referencing "Newsreader", "Newsreader-Italic", and
"JetBrainsMono") are not bundled or registered, causing silent fallbacks; add
the corresponding .ttf/.otf font files into the app bundle (e.g., Resources/ or
Fonts/), ensure they are included in the target, and register their filenames in
Info.plist under UIAppFonts (Fonts provided by application) so the
.custom("Newsreader", ...), .custom("Newsreader-Italic", ...), and
.custom("JetBrainsMono", ...) calls resolve at runtime.
In `@ios/StillPointApp/ViewModels/AppViewModel.swift`:
- Around line 105-113: The returnHome() function flips currentView to .home
before currentUser is refreshed, allowing Home-derived state (currentDay,
todayDuration, todayBlockCount) to remain stale; modify returnHome() so it
awaits the API refresh and updates currentUser before changing currentView:
perform the APIClient.shared.me() call inside an async Task and only set
currentView = .home after successfully updating currentUser (or handle the
failure path explicitly, e.g., retry or show error) so Home always reads
up-to-date session data from currentUser.
In `@ios/StillPointApp/ViewModels/AuthViewModel.swift`:
- Around line 24-29: Add a re-entry guard at the start of submit(): check if
isSubmitting is already true and immediately return nil if so, and ensure the
check-and-set of isSubmitting = true happens on the main actor to avoid a race
(either mark submit() `@MainActor` or wrap the guard and isSubmitting assignment
in await MainActor.run { ... }); keep the existing defer { isSubmitting = false
} and error reset behavior and reference the submit(), isSubmitting, and error
symbols when making the change.
In `@ios/StillPointApp/ViewModels/BoardViewModel.swift`:
- Around line 9-18: Add a visible error state to the BoardViewModel so failures
in load() surface to the UI: introduce a `@Published` var loadError: Error? (or
loadErrorMessage: String?) on the ViewModel, set loadError = nil before calling
APIClient.shared.getBoard(), and in the catch block assign the caught error (or
a user-friendly message) to loadError; keep isLoading handling and entries
assignment as-is so the view can observe loadError to show "Failed to load" and
a retry button that calls load() again.
In `@ios/StillPointApp/ViewModels/SessionViewModel.swift`:
- Line 75: The minute-chime misfire comes from initializing lastChimeMin with
ceil(...) while wholeMinutesLeft is computed via floor/truncation; make
lastChimeMin use the same rounding as wholeMinutesLeft (e.g.,
Int(floor(Double(self.totalSeconds) / 60.0)) or integer division) so both use
the same minute calculation and the first tick won't immediately trigger chimes;
update the initialization in the lastChimeMin assignment and the same pattern at
the other occurrences around the 235-240 block (referencing lastChimeMin,
wholeMinutesLeft, and totalSeconds).
- Around line 167-191: The current catch returns nil for any error, collapsing
partial success when APIClient.shared.createSession(...) succeeded but the
subsequent APIClient.shared.batchThoughts(...) failed; change the flow so
createSession's success is preserved: call createSession, then perform
batchThoughts in its own do/catch (or try? with mapped error handling) and on
batchThoughts failure log the error and return the created session (not nil) so
callers can see the session was created and handle the partial-write state;
reference the createSession and batchThoughts calls and the capturedThoughts ->
allThoughts mapping in SessionViewModel to locate where to add the separate
error handling and logging.
- Around line 153-165: The date string for sessionDate is produced with a
DateFormatter that can vary by device locale/calendar; update the DateFormatter
used before creating CreateSessionRequest (the dateFormatter variable that
formats sessionDate) to use a fixed locale and calendar (e.g., locale =
Locale(identifier: "en_US_POSIX") and calendar = Calendar(identifier:
.gregorian)) and set an explicit timezone (e.g., UTC) so the "yyyy-MM-dd" output
is stable across non-Gregorian devices.
In `@ios/StillPointApp/ViewModels/ThoughtJournalViewModel.swift`:
- Around line 23-27: The view model currently swallows errors from
APIClient.shared.getThoughts() (in ThoughtJournalViewModel) by only printing
them, so surface a user-facing error state: add a `@Published` var (e.g.,
loadingError: Error? or errorMessage: String?) to ThoughtJournalViewModel, set
it inside the catch block when getThoughts() throws, and clear it on successful
loads; also consider adding a `@Published` isLoading flag if useful. Update the
view to observe this new published property and present an alert or inline error
UI when loadingError/errorMessage is non-nil.
In `@ios/StillPointApp/Views/AuthView.swift`:
- Line 42: Replace the deprecated .autocapitalization(.none) modifier in
AuthView.swift with the iOS 15+ API .textInputAutocapitalization(.never); update
both occurrences (the modifiers applied to the TextField / SecureField in
AuthView) to use .textInputAutocapitalization(.never) and, if you need to
maintain backward compatibility for < iOS 15, wrap the new modifier in an
availability check and fall back to the existing .autocapitalization(.none)
inside the else branch.
In `@ios/StillPointApp/Views/CompletionView.swift`:
- Around line 181-188: The saveEndNote function currently sets noteSaved = true
immediately without persisting the end note; change saveEndNote to perform the
actual persistence (via the AppViewModel SwiftData save path or the API call
that creates a Note with timeInSession = -1 and the current session ID), await
the save result inside the Task, and only set noteSaved = true after confirming
the save succeeded; on failure set noteSaved = false and handle/report the
error. Reference saveEndNote, noteSaved and the timeInSession = -1 sentinel when
implementing the real save and result handling.
In `@ios/StillPointApp/Views/HistoryView.swift`:
- Around line 25-37: HistoryView currently only renders stats when vm.stats is
non-nil, so on first load/new account/error the UI collapses; update the view to
handle vm.isLoading, empty, and error states: show a ProgressView or loading
placeholder while vm.isLoading is true, show a clear “no data”/empty state (and
still render the today preview) when vm.stats is nil and not loading, and show
an error message when vm.error (or equivalent) is set; modify the branch around
vm.stats and the async load call that triggers fetch (the ViewModel load method
you call onAppear) so it sets vm.isLoading and vm.error appropriately and the UI
reads those flags to decide between Loading / Empty / Error / Stats (statCell)
states.
In `@ios/StillPointApp/Views/PublicBoardView.swift`:
- Around line 87-88: The username Text in PublicBoardView (Text(isCurrentUser ?
"\(username) (you)" : username)) can wrap and change row height; fix it by
forcing a single truncated line—add .lineLimit(1) and .truncationMode(.tail) to
that Text view so long usernames are clamped to one line and end with an
ellipsis, preserving the table's fixed column layout.
In `@ios/StillPointApp/Views/SessionView.swift`:
- Around line 196-205: The current flow fire-and-forgets
vm.saveSession(completed:) and immediately calls appVM.completeSession(...), so
navigation appears successful even when saveSession returns nil; change both
occurrences (the current completion branch and the "End Early" branch) to await
vm.saveSession(completed:) and check its return before calling
appVM.completeSession(...): if saveSession returns a non-nil/success value then
call appVM.completeSession with the same parameters, otherwise present an
error/keep the view (or persist locally first) so navigation only happens after
persistence succeeds.
In `@ios/StillPointApp/Views/SettingsView.swift`:
- Around line 61-83: The onChange handler for the Toggle can start multiple
concurrent API calls because isUpdating is set but never checked; modify the
handler in SettingsView so it first guards against concurrent updates (e.g., if
isUpdating { revert UI change by setting isPublic = !newValue and return })
before creating the Task, then set isUpdating = true/false around the await to
prevent overlapping calls; ensure updates to isPublic and isUpdating happen on
the main actor and keep calling APIClient.shared.updateSettings(isPublic:) as
before, reverting isPublic on failure.
In `@ios/StillPointApp/Views/ThoughtJournalView.swift`:
- Around line 36-46: The foreground color usage is inconsistent:
SPColor.amberText is passed directly to .foregroundStyle while other tokens
(e.g., SPColor.fg2) are wrapped in Color(...). In the ForEach HStack (Text
showing time using thought.timeInSession and Text(thought.text)), update the
.foregroundStyle call for SPColor.amberText to use Color(SPColor.amberText) so
it matches the Color(...) wrapper pattern used elsewhere (or alternatively, if
SPColor.* are already SwiftUI Color types, remove the Color(...) wrappers across
the file — choose one consistent approach and apply it to SPColor.amberText and
SPColor.fg2 usage).
In `@ios/StillPointShared/Sources/StillPointShared/AudioEngine.swift`:
- Around line 93-134: The closure in playSynthesized captures and mutates
currentPhase across threads causing a race; fix it by removing the shared
mutable Double and instead track samples rendered only on the audio thread using
an atomic integer or thread-local counter (e.g., replace currentPhase with an
Atomic<Int64> or stdatomic-backed sampleIndex) and update that atomically inside
the AVAudioSourceNode render block (referencing currentPhase/sampleIndex and
sourceNode), or restructure the logic so the render block computes phase from a
local per-call offset and a single atomic frame counter; ensure engine.stop is
called without reading/mutating that non-atomic phase variable.
- Around line 6-14: AudioEngine is marked `@unchecked` Sendable but
playSynthesized creates local AVAudioEngine instances concurrently and the
shared engine property is unused; fix by reusing the single engine stored in the
AudioEngine class (engine) and serialize all audio operations via a private
serial DispatchQueue (or make AudioEngine an actor) so playSynthesized and
playTick() always run on that queue, guard start/stop so multiple calls don't
spawn concurrent engines, and remove the unused local engine creation in
playSynthesized while keeping configureAudioSession initialization as-is.
In `@ios/StillPointShared/Sources/StillPointShared/Constants.swift`:
- Around line 14-16: The duration(forDay:) function lacks validation for the day
parameter; add a defensive check at the start of public static func
duration(forDay day: Int) to ensure day > 0 (e.g. use precondition(day > 0,
"duration(forDay:): day must be > 0, got \(day)") or guard with
preconditionFailure/assertion) so invalid inputs fail fast with a clear message;
keep the rest of the calculation (baseDuration + (day - 1) * increment)
unchanged.
In `@ios/StillPointShared/Sources/StillPointShared/Models/MindStateEntry.swift`:
- Around line 9-17: Replace the raw String state on MindStateEntry with a
String-backed enum (e.g., MindState: String, Codable) to enforce allowed values
("clear", "thinking"); update the property signature from state: String to
state: MindState, adjust the initializer MindStateEntry.init(time:state:) to
accept MindState (or accept String and map/throw to the enum), change isClear to
compare state == .clear, and ensure MindState conforms to
Codable/RawRepresentable so existing JSON encoding/decoding remains compatible.
In `@ios/StillPointShared/Sources/StillPointShared/SessionLogic.swift`:
- Around line 71-90: calculateClearPercent currently ignores the initial
interval and returns 0 for an empty mindStateLog; update calculateClearPercent
to return 100 when mindStateLog.isEmpty and to account for the period from time
0 to the first MindStateEntry. Implement this by treating a previousTime = 0 and
a previousIsClear initial value (true for default clear), iterate through
mindStateLog computing duration = entry.time - previousTime and adding it to
clearTime when previousIsClear, then set previousIsClear = entry.isClear and
previousTime = entry.time; after the loop add the final segment from
previousTime to totalElapsed if previousIsClear, and compute the percent as
before.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9611b7dd-aeb1-42ce-a30d-0108d7b316eb
📒 Files selected for processing (38)
ios/StillPoint.xcodeproj/project.pbxprojios/StillPoint.xcodeproj/project.xcworkspace/contents.xcworkspacedataios/StillPointApp/Components/BlockGridView.swiftios/StillPointApp/Components/MindStateBarView.swiftios/StillPointApp/Components/ThoughtCaptureView.swiftios/StillPointApp/Navigation/MainTabView.swiftios/StillPointApp/Resources/Assets.xcassets/AccentColor.colorset/Contents.jsonios/StillPointApp/Resources/Assets.xcassets/AppIcon.appiconset/Contents.jsonios/StillPointApp/Resources/Assets.xcassets/Contents.jsonios/StillPointApp/StillPointApp.swiftios/StillPointApp/Theme/DesignTokens.swiftios/StillPointApp/ViewModels/AppViewModel.swiftios/StillPointApp/ViewModels/AuthViewModel.swiftios/StillPointApp/ViewModels/BoardViewModel.swiftios/StillPointApp/ViewModels/HistoryViewModel.swiftios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointApp/ViewModels/ThoughtJournalViewModel.swiftios/StillPointApp/Views/AuthView.swiftios/StillPointApp/Views/CompletionView.swiftios/StillPointApp/Views/HistoryView.swiftios/StillPointApp/Views/HomeView.swiftios/StillPointApp/Views/PublicBoardView.swiftios/StillPointApp/Views/RootView.swiftios/StillPointApp/Views/SessionView.swiftios/StillPointApp/Views/SettingsView.swiftios/StillPointApp/Views/ThoughtJournalView.swiftios/StillPointShared/Package.swiftios/StillPointShared/Sources/StillPointShared/APIClient.swiftios/StillPointShared/Sources/StillPointShared/AudioEngine.swiftios/StillPointShared/Sources/StillPointShared/Constants.swiftios/StillPointShared/Sources/StillPointShared/DTOs/DTOs.swiftios/StillPointShared/Sources/StillPointShared/Models/MindStateEntry.swiftios/StillPointShared/Sources/StillPointShared/Models/Session.swiftios/StillPointShared/Sources/StillPointShared/Models/SyncStatus.swiftios/StillPointShared/Sources/StillPointShared/Models/Thought.swiftios/StillPointShared/Sources/StillPointShared/Models/User.swiftios/StillPointShared/Sources/StillPointShared/SessionLogic.swiftios/project.yml
Critical fixes: - calculateClearPercent: handle empty mindStateLog (return 100%) and account for initial clear interval before first state change - Session save: separate thought batch failure from session failure so a successful session is not reported as nil on thought error - Fonts: add system fallback (serif/monospaced) when custom fonts are not yet bundled; document registration steps Major fixes: - returnHome: await user refresh before navigating to prevent stale currentDay from allowing session restart - Auth submit: guard against re-entry on double-tap - Chime: use floor() for wholeMinutesLeft to match web behavior and prevent false trigger on first tick - DateFormatter: set POSIX locale + Gregorian calendar for yyyy-MM-dd - AuthView: replace deprecated .autocapitalization with .textInputAutocapitalization - CompletionView: document end-note save as TODO stub - HistoryView: add loading spinner, error message with retry, and empty state - SessionView: await persistence before navigating to completion - AudioEngine: serialize play calls through a serial DispatchQueue; use reference-type PhaseBox to avoid data race in render callback - BlockGridView: replace broken pulse timer with SwiftUI animation Minor/nitpick fixes: - MindStateBarView: remove unused currentState param, guard div-by-zero - ThoughtCaptureView: trim whitespace before validation - MainTabView: configure tab bar appearance only once - BoardViewModel/ThoughtJournalViewModel: expose errorMessage - PublicBoardView: lineLimit(1) on usernames - SettingsView: disable toggle during pending API call - Constants: precondition day >= 1 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
auerbachb
commented
Mar 27, 2026
auerbachb
commented
Mar 27, 2026
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (3)
ios/StillPointApp/Views/CompletionView.swift (1)
181-187:⚠️ Potential issue | 🟡 MinorEnd-note persistence is still stubbed.
The
saveEndNote()function setsnoteSaved = truewithout actual persistence. The TODO comment (lines 183-185) correctly identifies the missing implementation, but the UI currently misleads users by showing "saved" when the note is only held in local state and will be lost on navigation.Consider either:
- Removing the save button until persistence is implemented, or
- Clarifying in the UI that notes are "captured locally" rather than "saved"
,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/Views/CompletionView.swift` around lines 181 - 187, The saveEndNote() implementation currently sets noteSaved = true without persisting the end note, which misleads the user; either disable/hide the save button until persistence is implemented or implement real persistence by calling APIClient.shared.batchThoughts with timeInSession = -1 (using the sessionId once passed into CompletionView) and only set noteSaved = true after the API call succeeds; update saveEndNote(), CompletionView UI logic around the save button, and any labels that display saved state to reflect local-only capture if you choose the interim UI change.ios/StillPointApp/ViewModels/SessionViewModel.swift (1)
71-79:⚠️ Potential issue | 🟠 MajorMinute chimes may still fire immediately on first tick.
The initialization uses
ceil(line 76) while the tick usesfloor(line 244). For a 10-minute session:
- Init:
lastChimeMinutesLeft = ceil(600/60) = 10- First tick (~0.05s):
wholeMinutesLeft = floor(599.95/60) = 9- Condition
9 >= 1 && 9 < 10is true → plays 9 chimes immediatelyBoth calculations should use the same rounding strategy.
🔊 Proposed fix: use floor consistently
init(dayNumber: Int) { self.dayNumber = dayNumber self.totalSeconds = StillPoint.duration(forDay: dayNumber) self.soundPrefs = AudioEngine.loadPrefs() - // Initialize to ceil so the first tick doesn't immediately fire chimes - self.lastChimeMinutesLeft = Int(ceil(Double(self.totalSeconds) / 60.0)) + // Initialize with floor + 1 to match the tick's floor calculation + // This prevents the first tick from immediately triggering chimes + self.lastChimeMinutesLeft = Int(floor(Double(self.totalSeconds) / 60.0)) + 1 // Initial mind state log entry self.mindStateLog = [MindStateEntry(time: 0, state: "clear")] }Alternatively, initialize to
Int(floor(Double(self.totalSeconds) / 60.0))and change the condition on line 245 to<=instead of<.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/ViewModels/SessionViewModel.swift` around lines 71 - 79, The minute-chime logic is inconsistent because init sets lastChimeMinutesLeft using ceil while the tick computes wholeMinutesLeft using floor, causing chimes on the first tick; in the SessionViewModel initializer change the initialization of lastChimeMinutesLeft to use the same rounding as the tick (use Int(floor(Double(totalSeconds) / 60.0))) or, if you prefer keeping ceil, update the ticking condition in the tick handler (the comparison that checks wholeMinutesLeft vs lastChimeMinutesLeft before calling playMinuteChime) to match the ceil-based logic (e.g., adjust the relational operator) so both calculations use the same rounding strategy.ios/StillPointApp/Views/SessionView.swift (1)
267-279:⚠️ Potential issue | 🟠 MajorPersistence failure still allows navigation to completion screen.
The
saveSessionresult is discarded on line 270. If persistence fails (returnsnil), the user still navigates to the completion screen with potentially unsaved session data. This was flagged in a prior review.🛡️ Proposed fix: Guard on save result
private func handleCompletion() { Task { // Persist session before navigating to completion screen - _ = await vm.saveSession(completed: true) - appVM.completeSession( - clearPercent: vm.clearPercent, - thoughtCount: vm.thoughtCount, - thoughts: vm.capturedThoughts, - dayNumber: vm.dayNumber, - duration: vm.totalSeconds - ) + guard await vm.saveSession(completed: true) != nil else { + // Show error state or retry option + return + } + appVM.completeSession( + clearPercent: vm.clearPercent, + thoughtCount: vm.thoughtCount, + thoughts: vm.capturedThoughts, + dayNumber: vm.dayNumber, + duration: vm.totalSeconds + ) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/Views/SessionView.swift` around lines 267 - 279, The current handleCompletion() discards the result of vm.saveSession(completed: true) so navigation proceeds even if saving failed; update handleCompletion() to await vm.saveSession and guard that the returned value is non-nil/success before calling appVM.completeSession (use the save result or a boolean to decide), and on failure return early and surface an error (e.g., show an alert via vm or appVM) instead of navigating; target the handleCompletion, vm.saveSession(...) call, and appVM.completeSession(...) references when making the change.
🤖 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/StillPointApp/Components/BlockGridView.swift`:
- Around line 106-114: The pulsing never starts because isPulsing is never
changed; update BlockGridView so isPulsing is toggled in an onAppear to drive
the .animation(value:) change: initialize isPulsing to the opposite steady state
you want (e.g., false) and in the view containing the RoundedRectangle (or the
parent cell) add an onAppear that triggers a state flip (e.g., withAnimation or
DispatchQueue.main.async { isPulsing.toggle() }) so the .animation(value:
isPulsing) sees a change and the repeatForever pulse runs; apply the same fix to
the other identical block (lines 128-135).
In `@ios/StillPointApp/Theme/DesignTokens.swift`:
- Around line 83-108: The current implementation applies .weight() to
Font.custom(fontName, size:) which is ignored for static font files; update
SPFont to select weight-specific font files when available instead of relying on
.weight(): modify customOrFallback (and calls from serif, serifItalic, mono) to
accept/derive the exact font postscript name for the requested Font.Weight
(e.g., "Newsreader-Bold" vs "Newsreader-Regular") and check
UIFont(name:derivedName,size:) before returning .custom(derivedName,size:),
falling back to the system design if that specific weight file is missing; for
serifItalic, ensure you derive the italic variant names (e.g.,
"Newsreader-BoldItalic") when weight != .regular and only call .weight() on
system fonts.
In `@ios/StillPointApp/ViewModels/AppViewModel.swift`:
- Around line 15-26: The current Equatable implementation for AppView in static
func == treats any two .completion cases as equal and ignores their associated
values; change the .completion case to pattern-match and compare the associated
values (e.g., case (.completion(let a), .completion(let b)): return a == b) so
differences in completion metrics are respected (ensure the associated value
type conforms to Equatable), or if ignoring those values was intentional, add a
clear comment above static func == explaining that .completion associated values
are deliberately ignored for navigation-change detection.
In `@ios/StillPointApp/ViewModels/HistoryViewModel.swift`:
- Around line 28-42: The toggleDay(_: ) function currently swallows network
errors and only prints them; add a per-day error state (e.g., a new property
dayErrors: [Int: String]) and update toggleDay to set dayErrors[dayNumber] =
error.localizedDescription in the catch block, clear dayErrors[dayNumber] when a
successful getSession returns, and ensure when collapsing (expandedDay ==
dayNumber) you also clear any transient error for that day; this exposes
getSession failures (APIClient.shared.getSession) to the UI so the view can show
an inline error/toast for dayThoughts fetch failures.
In `@ios/StillPointApp/ViewModels/ThoughtJournalViewModel.swift`:
- Around line 4-8: Mark ThoughtJournalViewModel as `@MainActor` so all observed
state mutations (thoughts, isLoading, errorMessage) happen on the main actor; in
the load() async method guard against re-entrant calls by returning early if
isLoading is true, set isLoading = true at the start and ensure it's reset to
false in a defer (or finally) block, and check Task.isCancelled after any
suspension points to abort without mutating state; ensure any state updates are
performed on the MainActor (i.e., inside the actor-isolated context of the
class).
In `@ios/StillPointApp/Views/SessionView.swift`:
- Around line 93-97: The onChange observer for vm.isComplete and the "End Early"
button both trigger completion, causing duplicate save/complete calls; remove
the duplicate logic from the button Task (delete the
saveSession/appVM.completeSession calls in the "End Early" button) and let
vm.endEarly() set isComplete which will flow through the existing .onChange
handler to call handleCompletion(); add a small flag or change
handleCompletion(signature) (e.g., handleCompletion(completed: Bool) or a
vm.isEarlyEnd flag) so handleCompletion() can distinguish natural completion vs
early end and pass completed: false when ending early, and ensure vm.endEarly()
sets that flag before setting isComplete so the onChange branch uses the correct
value.
In `@ios/StillPointShared/Sources/StillPointShared/AudioEngine.swift`:
- Around line 112-153: playSynthesized currently creates a new AVAudioEngine
(engine) and AVAudioSourceNode (sourceNode) for every call which is inefficient;
refactor to reuse a single AVAudioEngine instance by promoting engine to a
long-lived property (e.g., a private let/var engine in the AudioEngine type),
initialize and start it once, and update playSynthesized to create/attach only a
reusable source node or schedule buffers onto the shared engine (ensuring proper
stopping/removal of the sourceNode when a tone ends); reference playSynthesized,
engine, sourceNode and PhaseBox when modifying initialization, start/stop logic,
and cleanup to avoid repeatedly creating/destroying AVAudioEngine.
---
Duplicate comments:
In `@ios/StillPointApp/ViewModels/SessionViewModel.swift`:
- Around line 71-79: The minute-chime logic is inconsistent because init sets
lastChimeMinutesLeft using ceil while the tick computes wholeMinutesLeft using
floor, causing chimes on the first tick; in the SessionViewModel initializer
change the initialization of lastChimeMinutesLeft to use the same rounding as
the tick (use Int(floor(Double(totalSeconds) / 60.0))) or, if you prefer keeping
ceil, update the ticking condition in the tick handler (the comparison that
checks wholeMinutesLeft vs lastChimeMinutesLeft before calling playMinuteChime)
to match the ceil-based logic (e.g., adjust the relational operator) so both
calculations use the same rounding strategy.
In `@ios/StillPointApp/Views/CompletionView.swift`:
- Around line 181-187: The saveEndNote() implementation currently sets noteSaved
= true without persisting the end note, which misleads the user; either
disable/hide the save button until persistence is implemented or implement real
persistence by calling APIClient.shared.batchThoughts with timeInSession = -1
(using the sessionId once passed into CompletionView) and only set noteSaved =
true after the API call succeeds; update saveEndNote(), CompletionView UI logic
around the save button, and any labels that display saved state to reflect
local-only capture if you choose the interim UI change.
In `@ios/StillPointApp/Views/SessionView.swift`:
- Around line 267-279: The current handleCompletion() discards the result of
vm.saveSession(completed: true) so navigation proceeds even if saving failed;
update handleCompletion() to await vm.saveSession and guard that the returned
value is non-nil/success before calling appVM.completeSession (use the save
result or a boolean to decide), and on failure return early and surface an error
(e.g., show an alert via vm or appVM) instead of navigating; target the
handleCompletion, vm.saveSession(...) call, and appVM.completeSession(...)
references when making the 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1e4c2ef6-c73e-4511-8dea-729a67e8ee59
📒 Files selected for processing (20)
ios/StillPointApp/Components/BlockGridView.swiftios/StillPointApp/Components/MindStateBarView.swiftios/StillPointApp/Components/ThoughtCaptureView.swiftios/StillPointApp/Navigation/MainTabView.swiftios/StillPointApp/Theme/DesignTokens.swiftios/StillPointApp/ViewModels/AppViewModel.swiftios/StillPointApp/ViewModels/AuthViewModel.swiftios/StillPointApp/ViewModels/BoardViewModel.swiftios/StillPointApp/ViewModels/HistoryViewModel.swiftios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointApp/ViewModels/ThoughtJournalViewModel.swiftios/StillPointApp/Views/AuthView.swiftios/StillPointApp/Views/CompletionView.swiftios/StillPointApp/Views/HistoryView.swiftios/StillPointApp/Views/PublicBoardView.swiftios/StillPointApp/Views/SessionView.swiftios/StillPointApp/Views/SettingsView.swiftios/StillPointShared/Sources/StillPointShared/AudioEngine.swiftios/StillPointShared/Sources/StillPointShared/Constants.swiftios/StillPointShared/Sources/StillPointShared/SessionLogic.swift
3 tasks
Critical: - SessionView: End Early no longer duplicates completion flow — single path through onChange(vm.isComplete) handles both natural completion and early end via completedNaturally flag Major: - BlockGridView: pulse animation now triggers via onAppear toggle instead of static initial value that never changed - ViewModels: add @mainactor to ThoughtJournalViewModel, BoardViewModel, HistoryViewModel for thread-safe state mutations; guard against re-entrant loads Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
auerbachb
commented
Mar 27, 2026
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
ios/StillPointApp/ViewModels/HistoryViewModel.swift (1)
29-40:⚠️ Potential issue | 🟡 MinorSurface per-day fetch failures instead of only logging them.
On Line 39, fetch errors are swallowed into logs, so an expanded row can appear empty with no user-visible cause.
Proposed fix
final class HistoryViewModel { var sessions: [SessionDTO] = [] var stats: StatsDTO? var isLoading = false var errorMessage: String? var expandedDay: Int? var dayThoughts: [Int: [ThoughtDTO]] = [:] + var dayErrors: [Int: String] = [:] func toggleDay(_ dayNumber: Int) async { if expandedDay == dayNumber { expandedDay = nil + dayErrors[dayNumber] = nil } else { expandedDay = dayNumber + dayErrors[dayNumber] = nil if dayThoughts[dayNumber] == nil { do { let detail = try await APIClient.shared.getSession(dayNumber: dayNumber) dayThoughts[dayNumber] = detail.thoughts + dayErrors[dayNumber] = nil } catch { + dayErrors[dayNumber] = error.localizedDescription print("Failed to load day \(dayNumber) thoughts: \(error)") } } } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ios/StillPointApp/ViewModels/HistoryViewModel.swift` around lines 29 - 40, In toggleDay(_:), failures from APIClient.shared.getSession are only printed; change this to surface per-day fetch errors by adding a per-day error state (e.g., a Published dictionary like dayErrors: [Int: Error?] or [Int: String]) and update it inside the catch block (set dayErrors[dayNumber] = error or error.localizedDescription), clear any previous error on successful fetch (remove or set nil), and ensure the UI binds to dayErrors so an expanded row can display the error message; reference toggleDay, dayThoughts, and APIClient.shared.getSession when making these changes.
🤖 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/StillPointApp/Components/BlockGridView.swift`:
- Around line 106-115: The pulse state `isPulsing` is shared across blocks so
when a new block becomes current its `.onAppear` doesn't toggle the state and
the `.animation(_, value:)` won't restart; update BlockGridView to watch the
current block identity (e.g. `currentBlock?.id`) and reset `isPulsing` when it
changes so the pulse restarts for the new `isCurrent` block — add an
`onChange(of: currentBlock?.id)` handler that sets `isPulsing = false` then
asynchronously sets it true inside a
`withAnimation(.easeInOut(duration:1.0).repeatForever(autoreverses:true))` (or
alternately move the repeated animation logic into the `if isCurrent` block's
`onAppear` so each `RoundedRectangle` restarts its own pulse); reference
`isPulsing`, `currentBlock?.id`, `isCurrent`, and the
`RoundedRectangle(cornerRadius: blockRadius)` pulse block to locate changes.
In `@ios/StillPointApp/ViewModels/BoardViewModel.swift`:
- Around line 11-22: Add a re-entrancy guard at the top of load(): if a load is
already in flight (isLoading == true) return immediately so you don't start a
second concurrent request; keep the existing isLoading = true / defer {
isLoading = false } behavior and ensure you check the same isLoading flag before
calling APIClient.shared.getBoard() to avoid interleaved updates to entries and
errorMessage in the load() method.
- Around line 4-9: BoardViewModel is missing an explicit deinit which triggers
the required_deinit SwiftLint rule; add a deinit to BoardViewModel (final class
BoardViewModel) that logs or cleans up state (e.g., set entries = [], cancel any
tasks if later added) to provide a clear teardown point and satisfy linting —
implement a simple deinit that at minimum logs deinitialization via your logger
or clears sensitive state so the lint rule is quiet and future resources can be
released.
In `@ios/StillPointApp/ViewModels/HistoryViewModel.swift`:
- Around line 14-17: The load() method can start while a previous load is still
running causing overlapping API calls and conflicting state updates; fix it by
short-circuiting re-entrancy: at the start of load() check the existing
isLoading flag (guard !isLoading else { return }) before mutating state, then
proceed to set isLoading = true, clear errorMessage, and defer resetting
isLoading = false; ensure these checks and writes reference the load() function
and the isLoading and errorMessage properties so concurrent invocations simply
return instead of producing overlapping work.
In `@ios/StillPointApp/ViewModels/SessionViewModel.swift`:
- Around line 5-6: Add an explicit deinit to SessionViewModel that
cancels/cleans up the AnyCancellable subscriptions (timer and controlHideTimer)
to satisfy the SwiftLint required_deinit rule and document teardown intent;
locate the SessionViewModel class and implement a deinit that calls cancel() on
timer and controlHideTimer (and performs any future cleanup) so resources are
explicitly torn down when the instance is deallocated.
In `@ios/StillPointApp/Views/SessionView.swift`:
- Around line 257-269: handleCompletion currently calls
vm.saveSession(completed:) but ignores a nil return and proceeds to
appVM.completeSession, causing navigation even when persistence fails; update
handleCompletion to check the result of vm.saveSession(completed:
vm.completedNaturally) and branch: if non-nil proceed to call
appVM.completeSession(...) as before, otherwise perform a graceful fallback
(e.g., persist locally for later sync or present a retry/error UI) so session
data won't be lost; reference vm.saveSession, vm.completedNaturally and
appVM.completeSession when implementing the guard/check and fallback logic.
- Around line 206-219: The Abandon button currently calls vm.abandon(), which
sets isComplete = true and triggers the .onChange(of: vm.isComplete) handler
that calls handleCompletion() (which saves the session) while the button also
calls appVM.returnHome(), causing unintended save + navigation races; fix by
distinguishing an "abandoned" path: modify SessionViewModel.abandon() to either
(a) set a new flag (e.g., isAbandoned) instead of isComplete, or (b) avoid
mutating isComplete at all, and then update the onChange handler in
SessionView.swift / handleCompletion() to early-return when isAbandoned is true
(or only run when isComplete && !isAbandoned); ensure the Abandon button still
calls vm.abandon() and then appVM.returnHome(), and remove any concurrent Task
that would trigger save for abandoned sessions.
---
Duplicate comments:
In `@ios/StillPointApp/ViewModels/HistoryViewModel.swift`:
- Around line 29-40: In toggleDay(_:), failures from APIClient.shared.getSession
are only printed; change this to surface per-day fetch errors by adding a
per-day error state (e.g., a Published dictionary like dayErrors: [Int: Error?]
or [Int: String]) and update it inside the catch block (set dayErrors[dayNumber]
= error or error.localizedDescription), clear any previous error on successful
fetch (remove or set nil), and ensure the UI binds to dayErrors so an expanded
row can display the error message; reference toggleDay, dayThoughts, and
APIClient.shared.getSession when making these changes.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 10ebaa92-da22-492a-bab8-29f1352d0313
📒 Files selected for processing (6)
ios/StillPointApp/Components/BlockGridView.swiftios/StillPointApp/ViewModels/BoardViewModel.swiftios/StillPointApp/ViewModels/HistoryViewModel.swiftios/StillPointApp/ViewModels/SessionViewModel.swiftios/StillPointApp/ViewModels/ThoughtJournalViewModel.swiftios/StillPointApp/Views/SessionView.swift
Owner
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
Major: - SessionView: abandon no longer triggers handleCompletion — added isAbandoned flag checked in onChange guard - HistoryViewModel/BoardViewModel: guard against re-entrant load() calls to prevent overlapping state writes - BlockGridView: replace state-based pulse with phaseAnimator for continuous animation that works across block transitions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This was referenced Mar 27, 2026
This was referenced Mar 28, 2026
This was referenced Apr 18, 2026
This was referenced Apr 23, 2026
This was referenced May 1, 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
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
StillPointSharedSwift Package with SwiftData models, API client, audio engine, and session logicproject.ymlincluded for reproducibility)Closes #20
Architecture
@Observable,TabViewnavigation, dark theme design system matching web CSS tokenssyncStatusfor future background syncKey Implementation Details
Date().timeIntervalSince(startDate)) to avoid drift — matches web behaviorBlockTimer.tsx(minute blocks + 10-sec blocks for sessions >2 min)AVAudioEngine— matching Web Audio API parametersglobals.csscustom propertiesTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit