fix(power): popup edits update active power profile; disable redundant model picker#644
Merged
Merged
Conversation
…t model picker When power-based switching is on, the menu bar Engine/Model pickers now write the current power source's profile (battery vs. plugged-in) instead of calling selectEngine/selectModel directly, which the power switcher would immediately revert (the popup appeared to reset the user's choice). The Settings > Models 'Selected Model' picker is disabled while switching is on, since the Power section's per-source pickers are the source of truth. Also includes in-progress WritingPaneView edits and FAQ.md.
Comment on lines
+36
to
43
| Text("Min: \(suggestionSettings.customWordCountLowWords)") | ||
| } | ||
| Stepper( | ||
| value: customHighBinding, | ||
| in: SuggestionWordRange.minimumWord...SuggestionWordRange.maximumWord | ||
| ) { | ||
| HStack(spacing: 4) { | ||
| Text("Max:") | ||
| TextField("", value: customHighBinding, format: .number) | ||
| .textFieldStyle(.plain) | ||
| .frame(width: 32) | ||
| .multilineTextAlignment(.trailing) | ||
| } | ||
| Text("Max: \(suggestionSettings.customWordCountHighWords)") | ||
| } |
Contributor
There was a problem hiding this comment.
Direct numeric input removed from custom word count steppers
Replacing the labeled TextField with a display-only Text means users can no longer type a value directly; they must click the stepper arrow once per word, which is impractical when jumping from, say, 5 to 50. The old TextField(…, format: .number) approach provided instant entry at no extra complexity cost. Worth confirming this simplification is intentional before shipping.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
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
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
Two fixes to power-based model switching. (1) In the menu bar popup, changing Engine/Model while switching is on now writes the current power source's profile (battery vs. plugged-in) instead of calling
selectEngine/selectModeldirectly, which the power switcher reverted on its next evaluation, making the popup appear to reset the user's choice. (2) The Settings > Models "Selected Model" picker is disabled while switching is on, since the Power section's per-source pickers are the source of truth.Validation
Linked issues
(none)
Risk / rollout notes
PowerSourceMonitorthroughAppDelegate->CotabbyApp->MenuBarViewso the popup knows the current power source.WritingPaneViewedits and addsFAQ.md.Greptile Summary
Fixes two related bugs in power-source-driven model switching: menu-bar engine/model edits now write through the active power-source profile instead of bypassing it, and the Settings › Models "Selected Model" picker is disabled while power switching is on. The PR also ships a FAQ document and simplifies the custom word-count stepper labels in
WritingPaneView.selectedEngineBindingandselectedModelBindingnow route edits throughapplyProfileForCurrentPowerSource, which writes the battery or plugged-in profile so the power-switcher observer picks up the change rather than reverting it..disabled(isPowerBasedModelSwitchingEnabled)guard and context-sensitive description, consistent with how the Power section's per-source pickers own the active model.Confidence Score: 3/5
The menu-bar fix is correct and well-reasoned, but a parallel gap in the Settings pane means the Engine picker there can still be silently overridden.
The
selectedEngineBindinginEngineAndModelPaneViewstill callsselectEnginedirectly, so a user who changes the engine from the Settings pane while power-based switching is on will see their choice quietly reverted the next time any profile trigger fires — the exact class of bug the PR fixes in the menu bar, left unaddressed in the Settings view.Cotabby/UI/Settings/Panes/EngineAndModelPaneView.swift — the Engine picker's binding needs the same treatment (disabled with explanation, or routed through the profile) as the Model picker directly below it.
Important Files Changed
powerSourceMonitoras a stored property on AppDelegate, mirroring the pattern used for other services; straightforward plumbing change.powerSourceMonitorfrom AppDelegate into MenuBarView; a one-line addition to an existing argument list, no issues.applyProfileForCurrentPowerSourcehelper is correct.selectEnginedirectly and is not disabled, which leads to silent reverts when power-based switching is on.Sequence Diagram
sequenceDiagram participant User participant MenuBarView participant PowerSourceMonitor participant SuggestionSettingsModel participant CotabbyAppEnvironment Note over MenuBarView: isPowerBasedModelSwitchingEnabled = true User->>MenuBarView: Change Engine or Model picker MenuBarView->>PowerSourceMonitor: isPluggedIn? alt Plugged in MenuBarView->>SuggestionSettingsModel: setPluggedInProfile(profile) else On battery MenuBarView->>SuggestionSettingsModel: setBatteryProfile(profile) end SuggestionSettingsModel-->>CotabbyAppEnvironment: "@Published batteryEngine/pluggedInEngine fires" CotabbyAppEnvironment->>SuggestionSettingsModel: selectEngine(profile.engine) CotabbyAppEnvironment->>RuntimeBootstrapModel: selectModel(profile.filename) Note over MenuBarView: isPowerBasedModelSwitchingEnabled = false User->>MenuBarView: Change Engine or Model picker MenuBarView->>SuggestionSettingsModel: selectEngine(engine) / selectModel(filename) directlyComments Outside Diff (1)
Cotabby/UI/Settings/Panes/EngineAndModelPaneView.swift, line 344-349 (link)selectedEngineBindinginEngineAndModelPaneViewstill callssuggestionSettings.selectEnginedirectly. This is the same pattern the PR fixes in the menu bar: when power-based switching is enabled,selectedEnginewill be silently overwritten byapplyPowerProfilethe next time any profile trigger fires (power source change, model-list refresh, etc.). Meanwhile the Model picker right below it is correctly disabled with an explanation — leaving the Engine picker editable creates a confusing inconsistency where the change appears to stick in the UI but is quietly reverted later.Reviews (1): Last reviewed commit: "fix(power): popup edits update active po..." | Re-trigger Greptile