Fix typing lag in Settings text editors by committing on focus loss#275
Open
tejasnafde wants to merge 1 commit into
Open
Fix typing lag in Settings text editors by committing on focus loss#275tejasnafde wants to merge 1 commit into
tejasnafde wants to merge 1 commit into
Conversation
The Custom Vocabulary, System Prompt, and Context Prompt editors wrote their value into the shared AppState on every keystroke. Because AppState is a single ObservableObject observed by the whole settings window and the menu bar, each keystroke fired objectWillChange and rebuilt all of those views. On a macOS 13 target there is no per-property observation to scope the invalidation, so typing in these fields was noticeably laggy (measured around 85ms of main-thread work per keystroke, with the menu bar rebuilding on every character). These fields now commit to AppState when the editor loses focus, matching what the API base URL and key fields already do, and also on disappear so nothing typed is lost if the window closes while focused. The prompt test runners commit first so they still test the latest text. Fixes zachlatta#274 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughSettings text editors no longer update ChangesSettings editor commits
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Sources/SettingsView.swift (1)
1570-1596: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting a shared prompt-commit helper.
commitCustomSystemPrompt()andcommitCustomContextPrompt()are structurally identical — same trim, default-comparison, clear-with-timestamp, and update-with-timestamp logic, differing only in the input variable, appState property, and default constant. A generic helper would eliminate the duplication and make future changes (e.g., timestamp format adjustments) a single-point edit.♻️ Optional refactor: shared commit helper
+ private func commitPrompt( + input: String, + currentValue: String, + defaultPrompt: String, + setCustom: (String) -> Void, + setLastModified: (String) -> Void + ) { + let trimmed = input.trimmingCharacters(in: .whitespacesAndNewlines) + let defaultTrimmed = defaultPrompt.trimmingCharacters(in: .whitespacesAndNewlines) + if trimmed == defaultTrimmed || trimmed.isEmpty { + if !currentValue.isEmpty { + setCustom("") + setLastModified("") + } + } else if currentValue != trimmed { + setCustom(trimmed) + setLastModified(iso8601DayFormatter.string(from: Date())) + } + } + private func commitCustomSystemPrompt() { - let trimmed = customSystemPromptInput.trimmingCharacters(in: .whitespacesAndNewlines) - let defaultTrimmed = PostProcessingService.defaultSystemPrompt.trimmingCharacters(in: .whitespacesAndNewlines) - if trimmed == defaultTrimmed || trimmed.isEmpty { - if !appState.customSystemPrompt.isEmpty { - appState.customSystemPrompt = "" - appState.customSystemPromptLastModified = "" - } - } else if appState.customSystemPrompt != trimmed { - appState.customSystemPrompt = trimmed - appState.customSystemPromptLastModified = iso8601DayFormatter.string(from: Date()) - } + commitPrompt( + input: customSystemPromptInput, + currentValue: appState.customSystemPrompt, + defaultPrompt: PostProcessingService.defaultSystemPrompt, + setCustom: { appState.customSystemPrompt = $0 }, + setLastModified: { appState.customSystemPromptLastModified = $0 } + ) } private func commitCustomContextPrompt() { - let trimmed = customContextPromptInput.trimmingCharacters(in: .whitespacesAndNewlines) - let defaultTrimmed = AppContextService.defaultContextPrompt.trimmingCharacters(in: .whitespacesAndNewlines) - if trimmed == defaultTrimmed || trimmed.isEmpty { - if !appState.customContextPrompt.isEmpty { - appState.customContextPrompt = "" - appState.customContextPromptLastModified = "" - } - } else if appState.customContextPrompt != trimmed { - appState.customContextPrompt = trimmed - appState.customContextPromptLastModified = iso8601DayFormatter.string(from: Date()) - } + commitPrompt( + input: customContextPromptInput, + currentValue: appState.customContextPrompt, + defaultPrompt: AppContextService.defaultContextPrompt, + setCustom: { appState.customContextPrompt = $0 }, + setLastModified: { appState.customContextPromptLastModified = $0 } + ) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/SettingsView.swift` around lines 1570 - 1596, Extract the duplicated logic from commitCustomSystemPrompt() and commitCustomContextPrompt() into a shared helper that accepts the input text, current stored prompt, default prompt, and update callback or equivalent state mutation hooks. Update both commit methods to delegate to this helper while preserving trimming, default/empty clearing, and ISO8601 timestamp behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Sources/SettingsView.swift`:
- Around line 1570-1596: Extract the duplicated logic from
commitCustomSystemPrompt() and commitCustomContextPrompt() into a shared helper
that accepts the input text, current stored prompt, default prompt, and update
callback or equivalent state mutation hooks. Update both commit methods to
delegate to this helper while preserving trimming, default/empty clearing, and
ISO8601 timestamp behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 20d87d2f-b969-487f-ad82-e583063e7976
📒 Files selected for processing (1)
Sources/SettingsView.swift
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.
What
Typing in the Custom Vocabulary, System Prompt, and Context Prompt fields in Settings is laggy. Each of these editors wrote its value into the shared
AppStateon every keystroke, and sinceAppStateis a singleObservableObjectobserved by the whole settings window and the menu bar, every character firedobjectWillChangeand rebuilt all of those views. On the macOS 13 target there is no per-property observation to scope that, so the whole thing re-rendered on each keystroke.I measured it with a small timer around the affected views on a MacBook Air M1: each keystroke rebuilt 3 view bodies and took 80 to 170 ms of main-thread work before going idle, usually around 85 ms. One of the rebuilt views was the menu bar extra, which has nothing to do with the text box.
Fix
These three editors now commit into
AppStatewhen the field loses focus instead of on every keystroke, which is the same pattern the API base URL and key fields already use. The live text stays in local@Statewhile you type, so there is no per-character churn on the shared object.To avoid losing anything typed, they also commit on
onDisappear(in case the window is closed while a field is still focused), and the prompt test runners commit first so they still test the latest text.Testing
AppStateper keystroke.make testpasses.Fixes #274
Summary by CodeRabbit