Wire end-note persistence in CompletionView#42
Conversation
Pass sessionId from SessionViewModel.saveSession() through AppViewModel.completeSession() into CompletionView. The saveEndNote() stub now calls APIClient.batchThoughts() with timeInSession: -1. Added loading spinner during save and error message on failure. Confirmation only shows after API success. Closes #39 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 2 minutes and 3 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThreaded Changes
Sequence DiagramsequenceDiagram
actor User
participant SessionView as "SessionView"
participant SessionVM as "SessionVM"
participant AppVM as "AppViewModel"
participant RootView as "RootView"
participant CompletionView as "CompletionView"
participant APIClient as "APIClient.shared"
participant Backend as "Backend"
User->>SessionView: finish session
SessionView->>SessionVM: saveSession(completed: true)
SessionVM->>Backend: POST /sessions
Backend-->>SessionVM: Session(id)
SessionVM-->>SessionView: session
SessionView->>AppVM: completeSession(sessionId, metrics...)
AppVM-->>RootView: currentView = .completion(sessionId, ...)
RootView->>CompletionView: init(sessionId, metrics...)
User->>CompletionView: enter end-note / tap "Save note"
CompletionView->>CompletionView: set isSaving = true, clear saveError
CompletionView->>APIClient: batchThoughts(sessionId, timeInSession:-1, thoughts)
APIClient->>Backend: POST /thoughts/batch
Backend-->>APIClient: success / failure
APIClient-->>CompletionView: response
CompletionView->>CompletionView: set noteSaved or saveError, set isSaving = false
CompletionView-->>User: show saved confirmation or error
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/StillPointApp/Views/SessionView.swift (1)
257-270: 🧹 Nitpick | 🔵 TrivialSession save failure is silent to the user.
When
vm.saveSession()fails and returnsnil, the user proceeds toCompletionViewwith an emptysessionId, which correctly disables the save button. However, there's no indication to the user why they can't save their end note.Consider adding user feedback when the session fails to persist — for example, showing a toast or alert, or displaying an informational message in
CompletionViewwhensessionIdis empty.🤖 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 257 - 270, handleCompletion currently calls vm.saveSession and proceeds to appVM.completeSession even when saveSession returns nil, leaving the user unaware why saving is disabled; modify handleCompletion to detect a nil session from vm.saveSession and surface feedback to the user (e.g., present an Alert or trigger a toast) before navigating to CompletionView, or pass an explicit error state into CompletionView when calling appVM.completeSession so CompletionView can show an informational message when sessionId is empty; use the existing symbols handleCompletion(), vm.saveSession(completed:), appVM.completeSession(...), and CompletionView/sessionId to locate the code and implement the user-visible error notification.
🤖 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/Views/CompletionView.swift`:
- Around line 108-139: The persistent save error should be cleared when the user
edits the note: update CompletionView so that when endNote changes you reset
saveError (e.g. use an onChange(of: endNote) handler) so stale errors disappear
as soon as the user modifies the text; locate the UI block referencing endNote,
saveError, isSaving and the saveEndNote() button and add the change handler to
clear saveError (and optionally also reset noteSaved) to avoid showing a stale
error after edits.
- Around line 201-225: Annotate the UI-mutating function saveEndNote with
`@MainActor` so its updates to `@State` properties (isSaving, saveError, noteSaved)
are explicitly executed on the main actor; e.g. add `@MainActor` before the
private func saveEndNote declaration (leave the API call to
APIClient.shared.batchThoughts inside the Task), or alternatively annotate the
Task closure with `@MainActor` (Task { `@MainActor` in ... }) or wrap the state
writes in await MainActor.run { ... } to ensure all modifications to isSaving,
saveError and noteSaved happen on the main actor.
---
Outside diff comments:
In `@ios/StillPointApp/Views/SessionView.swift`:
- Around line 257-270: handleCompletion currently calls vm.saveSession and
proceeds to appVM.completeSession even when saveSession returns nil, leaving the
user unaware why saving is disabled; modify handleCompletion to detect a nil
session from vm.saveSession and surface feedback to the user (e.g., present an
Alert or trigger a toast) before navigating to CompletionView, or pass an
explicit error state into CompletionView when calling appVM.completeSession so
CompletionView can show an informational message when sessionId is empty; use
the existing symbols handleCompletion(), vm.saveSession(completed:),
appVM.completeSession(...), and CompletionView/sessionId to locate the code and
implement the user-visible error notification.
🪄 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: 7f9df96c-880c-4ee7-8d36-68b2c48e395e
📒 Files selected for processing (4)
ios/StillPointApp/ViewModels/AppViewModel.swiftios/StillPointApp/Views/CompletionView.swiftios/StillPointApp/Views/RootView.swiftios/StillPointApp/Views/SessionView.swift
- Clear saveError via onChange(of: endNote) so stale errors disappear when user modifies the note text - Add @mainactor annotation to Task closure for explicit main-thread safety on @State updates Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai Both nitpicks fixed in
|
|
Tip For best results, initiate chat on the files or code changes.
Nice work wrapping those up! 🐇 ᓀ‸ᓂ |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/Views/CompletionView.swift`:
- Around line 204-228: The catch block in saveEndNote currently sets saveError
and clears isSaving but discards the caught error; update the catch in
saveEndNote to capture the thrown error and log it (using your logging facility
or print) while keeping the user-facing saveError message, e.g., log the error
from APIClient.shared.batchThoughts inside the catch so debugging info is
preserved without changing the user-visible saveError or isSaving updates.
🪄 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: 3230c16b-ab19-451b-81e7-2d04525223d6
📒 Files selected for processing (1)
ios/StillPointApp/Views/CompletionView.swift
Log the actual error for debugging while keeping the generic user-facing message. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai Fixed in |
|
Tip For best results, initiate chat on the files or code changes.
|
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/StillPointApp/Views/CompletionView.swift (1)
95-117:⚠️ Potential issue | 🟠 MajorFreeze the note editor once the note is marked saved.
Line 222 sets
noteSaved = true, but theTextEditorstays editable and the save button never comes back. Any edits after a successful save become local-only while the UI still sayssaved, which is misleading and can silently drop the user's final text.💡 Minimal fix
TextEditor(text: $endNote) .font(SPFont.serifItalic(15)) .foregroundStyle(Color(SPColor.fg)) .scrollContentBackground(.hidden) .frame(minHeight: 80) .padding(SPSpacing.s2) .background(SPColor.surface1) .clipShape(RoundedRectangle(cornerRadius: 8)) .overlay( RoundedRectangle(cornerRadius: 8) .stroke(SPColor.border2) ) + .disabled(isSaving || noteSaved)Also applies to: 129-133, 221-222
🤖 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 95 - 117, The TextEditor remains editable after noteSaved is set, causing post-save edits to be local-only; fix by making the editor non-editable when noteSaved is true (e.g., apply .disabled(noteSaved) or .allowsHitTesting(!noteSaved) to the TextEditor bound to $endNote) and optionally add a visual cue (opacity or overlay) to indicate the frozen state; ensure existing save button logic (the condition using endNote, noteSaved, isSaving and saveEndNote()) remains unchanged so the button only appears when edits are allowed and sessionId is present.
🤖 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/Views/CompletionView.swift`:
- Around line 205-220: In saveEndNote(), add an in-method guard to prevent
duplicate writes by returning early if isSaving or noteSaved are true (e.g.,
guard !isSaving && !noteSaved else { return }), then snapshot mutable state
(capture a local let note = endNote and local let sid = sessionId) before
creating the Task so the async work uses stable values; set isSaving = true
after the guard/snapshot, set noteSaved = true on successful
APIClient.shared.batchThoughts(...) completion, and ensure isSaving is reset on
failure so repeated attempts remain possible.
---
Outside diff comments:
In `@ios/StillPointApp/Views/CompletionView.swift`:
- Around line 95-117: The TextEditor remains editable after noteSaved is set,
causing post-save edits to be local-only; fix by making the editor non-editable
when noteSaved is true (e.g., apply .disabled(noteSaved) or
.allowsHitTesting(!noteSaved) to the TextEditor bound to $endNote) and
optionally add a visual cue (opacity or overlay) to indicate the frozen state;
ensure existing save button logic (the condition using endNote, noteSaved,
isSaving and saveEndNote()) remains unchanged so the button only appears when
edits are allowed and sessionId is present.
🪄 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: 7e0aa0ce-5823-4616-aea0-b9a16a8069e2
📒 Files selected for processing (4)
ios/StillPointApp/ViewModels/AppViewModel.swiftios/StillPointApp/Views/CompletionView.swiftios/StillPointApp/Views/RootView.swiftios/StillPointApp/Views/SessionView.swift
- Add !isSaving && !noteSaved guard in saveEndNote() to prevent duplicate writes from rapid taps - Snapshot endNote before Task to use stable value in async work - Disable TextEditor after save (isSaving || noteSaved) so post-save edits can't create misleading local-only state Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai Both major findings fixed in
|
|
Tip For best results, initiate chat on the files or code changes.
All the flagged issues across this PR are now addressed. Nice, clean implementation! 🐇 |
Keep both: new capsule button style + isSaveDisabled from main, .disabled(isSaving || noteSaved) on TextEditor + saveError display from this branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/StillPointApp/Views/CompletionView.swift (1)
21-22:⚠️ Potential issue | 🟠 MajorModel the save button from all three states: missing
sessionId, saving, and saved.The current UI only disables the button for
endNote.isEmpty || noteSaved, so it still appears actionable whensessionIdis empty and while the request is already in flight. This block also never renders a loading spinner and keeps the button onscreen after success by only renaming it to"Saved", which means the new save-state requirements are not actually reflected in the UI.Suggested fix
- private var isSaveDisabled: Bool { endNote.isEmpty || noteSaved } + private var isSaveDisabled: Bool { + endNote.trimmingCharacters(in: .whitespacesAndNewlines).isEmpty + || sessionId.isEmpty + || isSaving + || noteSaved + } ... - Button { - saveEndNote() - } label: { - Text(noteSaved ? "Saved" : "Save note") - .font(SPFont.serifItalic(18, weight: .light)) - .foregroundStyle(isSaveDisabled ? Color(SPColor.fg3) : Color(SPColor.bg)) - .frame(maxWidth: .infinity) - .padding(.vertical, SPSpacing.s2) - .background(isSaveDisabled ? SPColor.surface2 : SPColor.green) - .clipShape(Capsule()) - .overlay( - Capsule().stroke(isSaveDisabled ? SPColor.border2 : Color.clear) - ) - } - .disabled(isSaveDisabled) + if noteSaved { + Text("Saved") + .font(SPFont.mono(11, weight: .medium)) + .foregroundStyle(Color(SPColor.fg3)) + } else { + Button { + saveEndNote() + } label: { + HStack(spacing: SPSpacing.s2) { + if isSaving { + ProgressView() + .tint(Color(SPColor.bg)) + } + Text(isSaving ? "Saving…" : "Save note") + .font(SPFont.serifItalic(18, weight: .light)) + } + .foregroundStyle(isSaveDisabled ? Color(SPColor.fg3) : Color(SPColor.bg)) + .frame(maxWidth: .infinity) + .padding(.vertical, SPSpacing.s2) + .background(isSaveDisabled ? SPColor.surface2 : SPColor.green) + .clipShape(Capsule()) + .overlay( + Capsule().stroke(isSaveDisabled ? SPColor.border2 : Color.clear) + ) + } + .disabled(isSaveDisabled) + }Also applies to: 113-127
🤖 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 21 - 22, Update CompletionView to model three save-button states: missing sessionId, saving, and saved. Add a new `@State` Bool isSaving and update the computed var isSaveDisabled (currently: isSaveDisabled) to also return true when sessionId is empty or isSaving; in the save routine (e.g. saveNote() or wherever the network call is made) set isSaving = true before starting the request and set isSaving = false and noteSaved = true on success (and reset appropriately on failure). In the view that renders the Save button, show a loading spinner and a disabled button label (or activity indicator) when isSaving, show a disabled “Saved” state when noteSaved is true (or hide the button if you prefer), and ensure the button is disabled and styled appropriately when sessionId is empty so the UI reflects the three states.
🤖 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/Views/SessionView.swift`:
- Around line 270-272: The current flow calls appVM.completeSession(sessionId:
session?.id ?? "") even when vm.saveSession(completed:) returns nil; change this
to only call appVM.completeSession when the returned session is non-nil (i.e.,
guard let session = await vm.saveSession(completed: vm.completedNaturally) else
{ handle/save error or show retry UI }) and surface an explicit error/retry path
(or present an alert) instead of passing an empty id so CompletionView is not
shown for an unsaved session.
---
Outside diff comments:
In `@ios/StillPointApp/Views/CompletionView.swift`:
- Around line 21-22: Update CompletionView to model three save-button states:
missing sessionId, saving, and saved. Add a new `@State` Bool isSaving and update
the computed var isSaveDisabled (currently: isSaveDisabled) to also return true
when sessionId is empty or isSaving; in the save routine (e.g. saveNote() or
wherever the network call is made) set isSaving = true before starting the
request and set isSaving = false and noteSaved = true on success (and reset
appropriately on failure). In the view that renders the Save button, show a
loading spinner and a disabled button label (or activity indicator) when
isSaving, show a disabled “Saved” state when noteSaved is true (or hide the
button if you prefer), and ensure the button is disabled and styled
appropriately when sessionId is empty so the UI reflects the three states.
🪄 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: 6fa36dfd-b7bb-4f4a-9f23-5941837014bf
📒 Files selected for processing (2)
ios/StillPointApp/Views/CompletionView.swiftios/StillPointApp/Views/SessionView.swift
Guard handleCompletion on non-nil session from saveSession(). Show alert on failure with Retry and Continue-without-saving options instead of silently navigating with an empty sessionId. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ios/StillPointApp/Views/CompletionView.swift (1)
113-133:⚠️ Potential issue | 🟠 MajorSave CTA state is inconsistent with the async guard and expected UX.
At Line 127, the button disable logic does not account for
sessionId.isEmptyorisSaving, so users can tap an apparently active button that immediately no-ops via the guard at Line 200. Also, there is no in-flight spinner and the save button is not hidden after success.Proposed fix
@@ - private var isSaveDisabled: Bool { endNote.isEmpty || noteSaved } + private var trimmedEndNote: String { + endNote.trimmingCharacters(in: .whitespacesAndNewlines) + } + private var isSaveDisabled: Bool { + trimmedEndNote.isEmpty || noteSaved || isSaving || sessionId.isEmpty + } @@ - Button { - saveEndNote() - } label: { - Text(noteSaved ? "Saved" : "Save note") - .font(SPFont.serifItalic(18, weight: .light)) - .foregroundStyle(isSaveDisabled ? Color(SPColor.fg3) : Color(SPColor.bg)) - .frame(maxWidth: .infinity) - .padding(.vertical, SPSpacing.s2) - .background(isSaveDisabled ? SPColor.surface2 : SPColor.green) - .clipShape(Capsule()) - .overlay( - Capsule().stroke(isSaveDisabled ? SPColor.border2 : Color.clear) - ) - } - .disabled(isSaveDisabled) + if noteSaved { + Text("Saved") + .font(SPFont.mono(11, weight: .medium)) + .foregroundStyle(SPColor.green) + } else { + Button { + saveEndNote() + } label: { + HStack(spacing: SPSpacing.s1) { + if isSaving { + ProgressView() + .controlSize(.small) + .tint(Color(SPColor.bg)) + } + Text(isSaving ? "Saving..." : "Save note") + } + .font(SPFont.serifItalic(18, weight: .light)) + .foregroundStyle(isSaveDisabled ? Color(SPColor.fg3) : Color(SPColor.bg)) + .frame(maxWidth: .infinity) + .padding(.vertical, SPSpacing.s2) + .background(isSaveDisabled ? SPColor.surface2 : SPColor.green) + .clipShape(Capsule()) + .overlay( + Capsule().stroke(isSaveDisabled ? SPColor.border2 : Color.clear) + ) + } + .disabled(isSaveDisabled) + }🤖 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 113 - 133, Update the Save button UI to reflect the actual async state by including sessionId.isEmpty and isSaving in the disable and style logic (the existing isSaveDisabled should incorporate sessionId.isEmpty || isSaving), show an in-flight spinner when isSaving (e.g., next to the Text label) and prevent interaction while saving, and hide or replace the button after success (when noteSaved) so it’s not tappable anymore; ensure saveEndNote() still sets isSaving and noteSaved appropriately and that saveError display remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@ios/StillPointApp/Views/CompletionView.swift`:
- Around line 113-133: Update the Save button UI to reflect the actual async
state by including sessionId.isEmpty and isSaving in the disable and style logic
(the existing isSaveDisabled should incorporate sessionId.isEmpty || isSaving),
show an in-flight spinner when isSaving (e.g., next to the Text label) and
prevent interaction while saving, and hide or replace the button after success
(when noteSaved) so it’s not tappable anymore; ensure saveEndNote() still sets
isSaving and noteSaved appropriately and that saveError display remains
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6f1aa742-f4db-43c6-b0bc-7c68937ecc37
📒 Files selected for processing (4)
ios/StillPointApp/ViewModels/AppViewModel.swiftios/StillPointApp/Views/CompletionView.swiftios/StillPointApp/Views/RootView.swiftios/StillPointApp/Views/SessionView.swift
- Include isSaving and sessionId.isEmpty in isSaveDisabled so button visual state matches the guard in saveEndNote() - Show ProgressView spinner during save - Replace button with "Saved" text after success Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
@coderabbitai Fixed in
|
|
Tip For best results, initiate chat on the files or code changes.
All flagged issues across the PR are fully addressed. 🐇 |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
1 participant
Summary
sessionIdfromSessionViewModel.saveSession()throughAppViewModel.completeSession()intoCompletionViewsaveEndNote()to callAPIClient.shared.batchThoughts()withtimeInSession: -1Closes #39
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes