Repository navigation
Heat: keep Low Power Mode on until nominal, and show it in the menu - #80
aldikosh23 wants to merge 4 commits into
Conversation
paoValle
left a comment
There was a problem hiding this comment.
Source review only — I could not run it: this machine has Command Line Tools and no Xcode, so the package does not compile here (the SwiftUI macro plugin is missing) and I did not run swift test. Read from the diff:
- the rule change is the one the spec now describes (
serious, orfairwith nonominalsince aserious) andFloorRulesstays a pure function withhotas an input; HeatLoglives onFloorRuleDriver, andAppServices.start(for:)builds a new driver per session, so the history is session-scoped as the comment claims — it does not leak into the next session or survive a relaunch;Config.FloorInputscovers exactly the four settingsevaluate/lowPowerCauseread, so the Settings hook cannot miss one;- the release wording now says which state it waited for, and
lastCausedistinguishes the thermal release from the battery one; - the two new "Not run" rows in
docs/release-validation.mdare the honest place for the hardware part of this.
One question inline about the release condition. Also: the branch is CONFLICTING against main right now; the section 6 table rows it shares with #61 are the likely spot, and you already flagged that whichever lands second takes the small conflict.
| config: Config | ||
| ) -> LowPowerCause? { | ||
| if config.thermalRules, thermal == .serious { return .thermal } | ||
| if config.thermalRules, thermal == .serious || (hot && thermal == .fair) { return .thermal } |
There was a problem hiding this comment.
The hold releases only on nominal, and nothing else ends it: if the Mac stays at fair for the rest of the session (a sustained build is enough), Low Power Mode and the display sample hold stay on until the session ends. The cycling you are fixing is real and I would not go back to releasing at fair — the question is whether the hold should also have a bound, for example a release after the state has stayed below serious for N minutes, or after a second fair reading. If "session end is the bound" is the intended answer, one sentence in the spec next to the hold would say so for the next reader.
There was a problem hiding this comment.
Thanks for reading it this closely. Session end is the intended bound. A Mac at fair after serious is still carrying the heat, and a release on a timer, or on a second fair reading, brings the cycling back at a slower pace. While the hold lasts, the menu shows "Low Power Mode set by Insomnia" and the thermal state, and turning the thermal rules off in Settings releases it at once. I wrote that next to the hold in docs/spec.md and in docs/lid-close.md (fecfc61).
The branch is rebased on main now, so the conflict is gone, and the two hardware checks moved to "Real-Mac checks before a release" in docs/releasing.md, after #66.
Low Power Mode turned on for a serious thermal state went off at fair, the first step down. That brought back the load that made the Mac serious, so the mode went on and off again as the Mac heated up, with a notification each time. The thermal cause now holds from serious until nominal; FloorRuleDriver keeps that history for the session, in memory. The notification after the cooldown said "Back above the floor.", which is about the battery. It now says "Thermal state is back to nominal.", or "Thermal rules are off." when Settings turned them off, and the one at the start says the mode stays on until then. A battery floor or the thermal rules changed in Settings during a session applied only at the next battery or thermal event. They now run the floors at once, as the lid option already did.
The machine line adds "Thermal: serious" (or fair, critical), read again when the menu opens rather than through the monitor, whose copy moves only with the notification and is not kept current outside a session. The actions line says "Low Power Mode set by Insomnia" while the journal has lowPowerSetByUs, so the reason the Mac is slower is in the menu.
The Settings view asked for a floor run with an onChange per setting, a path no test could reach. SessionManager.config now calls floorInputsChanged when a battery floor, the thermal rules or the lid option changes (Config.floorInputs), and AppServices points it at reevaluateFloors(). The view's onChange calls, the lid option's included, are gone, so there is one path for every floor input. Tests: a floor input change asks for one run and any other change none, and during a session each change applies through that hook with no battery, thermal or lid event, in the power calls, the journal and the session end.
A Mac that stays at fair after serious, under a long build say, keeps Low Power Mode and the held display sample until the session ends. Say so next to the hold, with why a release on a timer is not used: it would bring the cycling back at a slower pace. The menu shows the mode and the state meanwhile, and turning the thermal rules off releases it.
cdb87e3 to
fecfc61
Compare
|
@greptileai please review this PR. |
|
Thanks for this. The Codex review of 1. The open review threadpaoValle's thread on 2. A test report from your MacThe PR says nothing ran on hardware, and the Settings and menu lines weren't checked in an installed app. CI covers the logic. This report covers the app as people run it.
Build and what to post
Post the results as a comment on this PR. Put long logs in a gist and link it. If you push another commit, the report needs to be redone at the new head. |
|
Thanks both. The sentence now in I cannot resolve the thread myself: the API answers |
Problem
fair, the first step down. That brings back the load that made the Mac serious, so the mode goes on and off again as the Mac heats up, with a notification each time.Fix
seriousreading until anominalone, or until the session ends if the Mac stays atfair(by design, see the spec next to the hold: a release on a timer would bring the cycling back, only slower).FloorRuleDriverkeeps that history for the session, in memory (hotinFloorRules.evaluate); after a relaunch a Mac atfaircounts as cooled down. The notifications say what the mode waits for, "Thermal state is serious. Low Power Mode stays on until it is back to nominal.", and why it went off, "Thermal state is back to nominal." (or "Thermal rules are off." when Settings turned them off). Settings has one line under the thermal toggle saying the same.SessionManager.configcalls a new hook,floorInputsChanged, when a battery floor, the thermal rules or the lid option changes (Config.floorInputs), andAppServicespoints it atreevaluateFloors(). Settings writes every change throughmanager.config, so the view'sonChangefor the lid option is gone too and every floor input takes the same path.PowerMonitor, whose copy moves only with the notification. The actions line says "Low Power Mode set by Insomnia" while the journal haslowPowerSetByUs.Nothing here touches the journal, the backstop or the scripts. One side effect: while Insomnia's Low Power Mode is on, the display sample is held (the "display sampling hold" in
.greptile/rules.md), so with the mode on longer after heat, a brightness change made in that time is not journaled at the next lid close. The spec says so next to the hold.Related
UserIsActiveevery ~40 s keeps idle sleep away. The Mac would run hotter in the bag than before. I will send that end as a separate pull request on top of Lid: a timer, heat or battery end with the lid closed puts the Mac to sleep #61.pmset -g customprintspowermode(Low Power Mode fails on macOS 26.7.1: parser expects lowpowermode, pmset reports powermode #69) and the hold applies there too.seriousrow of the section 6 table indocs/spec.md, so whichever lands second has a small conflict there.docs/releasing.md.Tests
FloorRulesTests: the hold at fair after serious, its release at nominal and with the thermal rules off, and the cause it names.FloorRuleDriverTests: the mode goes on once and stays on through fair and a second serious (onelowpowermode 1, one notification), the cooldown and rules-off notifications, and the takeover case with its new text.FloorRuleDriverTests, Settings: a floor input change asks for one floor run and any other change for none, and during a session each change (the thermal rules on and off, a Low Power Mode floor and an end floor raised above the charge) applies through that hook with no battery, thermal or lid event, in the power calls, the journal and the session end.UIStatusTestsandIntegrationWiringTests: the two menu lines, and the thermal state throughLiveStatusSource.swift testat fecfc61, rebased onmaindb9085e, on macOS 27.2 (MacBook Pro M5, Xcode 27.0), with tmux installed: 1,240 tests, 0 skipped. The only failures are 4 assertions in 3 tests that failed the same way on unmodifiedmainon this Mac before the rebase, because they compare macOS error text and the system language here is Russian:PackagingTests.testPinOfTheRunningAppRefusesABundleReSignedSinceLaunch,PackagingTests.testPinRefusesABundleWhoseSealedScriptWasEditedAfterSigningandUIStatusTests.testHotspotPasswordProblemCopy.swift build -c release -Xswiftc -warnings-as-errorspasses, and each commit builds with its tests on its own.Not verified
docs/releasing.md.

Confidence Score: 5/5 
The PR appears safe to merge; no new actionable issues were found.Summary
Keeps heat-triggered Low Power Mode on through
fairuntilnominal, applies floor settings without waiting for a power event, and adds thermal and Low Power Mode status to the menu.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD Settings["Settings changes manager.config"] --> Hook["floorInputsChanged"] Hook --> Queue["reevaluateFloors queues a floor run"] Events["Battery, thermal, or lid event"] --> Queue Queue --> History["Update this session's heat history"] History --> Rules["Evaluate battery, heat, and lid causes"] Rules --> Mode["Keep or change Low Power Mode"] Rules --> End["End session when an end rule matches"]Reviews (4) · Last reviewed commit: "Docs: the heat hold ends at nominal or a..." · Reviewed by Greptile