Implemented high power features management module #13984
Conversation
This module manages high-power features based on battery status and thermal conditions. It includes functions to enable/disable high-power features and monitor battery changes.
WalkthroughA new TypeScript module is introduced that implements a battery-aware power management system. It defines interfaces and internal state flags to monitor device battery level, thermal conditions, and manual overrides, then applies computed settings to control rendering quality and animation features globally. Changes
Sequence DiagramsequenceDiagram
autonumber
participant Device as Device Events
participant Module as highPowerEffects
participant Settings as APP_SETTINGS
rect rgba(100, 200, 150, 0.2)
note right of Device: Initialization
Module->>Module: _applySettings('initialization')
Module->>Settings: Update liveRendering, renderQuality
end
Device->>Module: Battery level/charging change
Module->>Module: Update _batteryTriggeredLow flag
Module->>Module: _applySettings('battery')
Module->>Settings: Propagate settings
Device->>Module: Thermal state change
alt Manual Override Active
Module->>Module: Skip thermal adjustment
Note over Module: Manual override takes precedence
else No Override
Module->>Module: Update _thermalTriggeredLow
Module->>Module: _applySettings('thermal')
Module->>Settings: Propagate settings
end
Device->>Module: Manual disable request
Module->>Module: Set _manualOverrideLow = true
Module->>Module: Clear thermal/battery flags
Module->>Module: _applySettings('manual')
Module->>Settings: Update to low-power mode
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
packages/frontend/core/src/modules/effects/highPowerEffects.ts (2)
21-40: Improve type safety for APP_SETTINGS mutations.The repeated use of
(globalThis as any).APP_SETTINGSbypasses TypeScript's type checking. IfAPP_SETTINGShas a different shape than expected or lacks theliveRendering/renderQualityproperties, this could cause runtime errors without compile-time warnings.Consider defining a proper interface and type guard:
interface AppSettings { liveRendering?: boolean; renderQuality?: 'high' | 'low'; expensiveAnimations?: boolean; } const hasAppSettings = (): boolean => { return ( typeof (globalThis as any).APP_SETTINGS === 'object' && (globalThis as any).APP_SETTINGS !== null ); }; const _applySettings = (source: string = 'unknown'): void => { const isHighPower = !(_manualOverrideLow || _thermalTriggeredLow || _batteryTriggeredLow); _liveRendering = isHighPower; _expensiveAnimations = isHighPower; _renderQuality = isHighPower ? 'high' : 'low'; if (hasAppSettings()) { const settings = (globalThis as any).APP_SETTINGS as AppSettings; settings.liveRendering = _liveRendering; settings.renderQuality = _renderQuality; } console.log(`Settings updated by ${source}: Live rendering=${_liveRendering}, Expensive animations=${_expensiveAnimations}, Render quality=${_renderQuality}.`); };
128-129: Consider using proper TypeScript types instead of 'any' assertions.The type assertion
(navigator as any).getBattery()bypasses TypeScript's type checking. While this is common for experimental APIs, you could define a more precise type.+interface NavigatorWithBattery extends Navigator { + getBattery?: () => Promise<BatteryManager>; +} + export const initPowerManagement = (): void => { - if (typeof navigator !== 'undefined' && 'getBattery' in navigator) { - (navigator as any).getBattery().then((battery: BatteryManager) => { + const nav = navigator as NavigatorWithBattery; + if (typeof navigator !== 'undefined' && nav.getBattery) { + nav.getBattery().then((battery: BatteryManager) => { _batteryManager = battery;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/frontend/core/src/modules/effects/highPowerEffects.ts(1 hunks)
🔇 Additional comments (5)
packages/frontend/core/src/modules/effects/highPowerEffects.ts (5)
1-5: LGTM! BatteryManager interface is correctly defined.The interface accurately reflects the Battery Status API specification with appropriate read-only properties and EventTarget extension.
7-10: LGTM! Internal request flags are well-designed.The separation of manual, thermal, and battery triggers provides clear intent and allows proper precedence handling.
42-43: LGTM! Initialization at module load is appropriate.Calling
_applySettingswith a descriptive source ensures the initial state is properly configured.
46-56: LGTM! Manual disable logic is correctly implemented.The function properly sets manual override and clears environmental triggers, ensuring manual control takes precedence as specified in the PR objectives.
72-96: LGTM! Thermal state handling is correctly implemented.The function properly respects manual override, uses appropriate thermal thresholds, and avoids unnecessary updates by checking if the state changed.
| // Internal flags for managing high-power features *actual state* | ||
| // These will be derived from the above request flags | ||
| let _liveRendering: boolean = true; | ||
| let _expensiveAnimations: boolean = true; | ||
| let _renderQuality: 'high' | 'low' = 'high'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add getter functions for internal state flags.
The internal state flags (_liveRendering, _expensiveAnimations, _renderQuality) cannot be accessed by other modules that need to read current settings. While these values are written to globalThis.APP_SETTINGS, modules may need direct access before APP_SETTINGS is initialized or for consistency.
Consider adding getter functions:
+/**
+ * Returns the current live rendering state.
+ */
+export const isLiveRenderingEnabled = (): boolean => _liveRendering;
+
+/**
+ * Returns the current expensive animations state.
+ */
+export const areExpensiveAnimationsEnabled = (): boolean => _expensiveAnimations;
+
+/**
+ * Returns the current render quality setting.
+ */
+export const getRenderQuality = (): 'high' | 'low' => _renderQuality;🤖 Prompt for AI Agents
In packages/frontend/core/src/modules/effects/highPowerEffects.ts around lines
12 to 16, the internal flags _liveRendering, _expensiveAnimations and
_renderQuality are not readable by other modules; add and export simple getter
functions getLiveRendering(): boolean, getExpensiveAnimations(): boolean and
getRenderQuality(): 'high'|'low' that return the current values of those
internal variables (keep the variables unchanged and not exported), and update
the module exports/exports list to expose these getters so other modules can
read the current state safely even before globalThis.APP_SETTINGS is
initialized.
| /** | ||
| * Enables high-power features, reverting to default settings. | ||
| * This clears any manual override and allows environmental factors (thermal, battery) | ||
| * to potentially reduce load again if their conditions are met. | ||
| */ | ||
| export const enableHighPowerFeatures = (): void => { | ||
| _manualOverrideLow = false; | ||
| // Clear all environmental triggers when manually enabling high-power features | ||
| _thermalTriggeredLow = false; | ||
| _batteryTriggeredLow = false; | ||
| _applySettings('manual_enable'); | ||
| console.log('High-power features manually enabled.'); | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify environmental trigger re-evaluation after manual enable.
The function clears all environmental triggers (lines 66-67), but the JSDoc comment states it "allows environmental factors (thermal, battery) to potentially reduce load again if their conditions are met." After calling this function, environmental conditions won't be re-evaluated unless:
- Battery charging/level changes (triggers events)
reduceLoadForThermalStateis called again
If the battery is still in a low-power condition when enableHighPowerFeatures is called, the system will stay in high-power mode until the next battery event. Is this the intended behavior, or should environmental conditions be immediately re-evaluated?
Run this script to check if there's additional logic that re-evaluates conditions after manual enable:
🏁 Script executed:
#!/bin/bash
# Search for calls to enableHighPowerFeatures and any subsequent re-evaluation logic
rg -n -A 10 'enableHighPowerFeatures' --type tsLength of output: 1110
🏁 Script executed:
#!/bin/bash
# Find call sites of enableHighPowerFeatures (excluding the definition)
echo "=== Call sites of enableHighPowerFeatures ==="
rg -n 'enableHighPowerFeatures\s*\(' --type ts | grep -v 'export const enableHighPowerFeatures'
echo -e "\n=== What does _applySettings do? ==="
rg -n -B 2 -A 15 'const _applySettings' --type ts
echo -e "\n=== Environmental trigger re-evaluation logic ==="
rg -n '_thermalTriggeredLow|_batteryTriggeredLow' --type ts -A 2 -B 2Length of output: 6571
🏁 Script executed:
#!/bin/bash
# Check for BatteryManager event listeners and initialization
echo "=== BatteryManager event listeners ==="
rg -n -B 5 -A 10 'addEventListener.*battery|batteryManager.*addEventListener' --type ts
echo -e "\n=== Thermal event handling ==="
rg -n -B 5 -A 10 'addEventListener.*thermal|thermal.*addEventListener' --type ts
echo -e "\n=== initPowerManagement function ==="
rg -n -B 2 -A 25 'export const initPowerManagement' --type ts
echo -e "\n=== Exports from highPowerEffects.ts ==="
rg -n '^export' packages/frontend/core/src/modules/effects/highPowerEffects.tsLength of output: 4377
Clarify JSDoc comment to match actual environmental trigger behavior.
The JSDoc comment incorrectly suggests that environmental factors will be "re-evaluated" after enableHighPowerFeatures is called. In reality, the function clears the override flag and enters high-power mode, but environmental conditions are only re-evaluated when the next battery or thermal event occurs. If the battery is still in a low-power state, the system will remain in high-power mode until the next levelchange or chargingchange event fires.
Update the comment to clarify that the function enables high-power mode immediately and that environmental conditions will only be re-checked when their respective events occur, not automatically upon manual enable.
🤖 Prompt for AI Agents
In packages/frontend/core/src/modules/effects/highPowerEffects.ts around lines
58 to 70, update the JSDoc to accurately describe behavior: state that
enableHighPowerFeatures immediately enables high-power mode and clears
manual/environmental trigger flags, and explicitly note that environmental
conditions are NOT re-evaluated immediately but will only be checked when their
respective battery or thermal events (e.g., levelchange, chargingchange, or
thermal event handlers) fire in the future; replace the misleading sentence
about "reverting to default settings" and "potentially reduce load again if
their conditions are met" with this clarified behavior.
| // Low power if not charging, or if battery is low (e.g., <= 20%) | ||
| const shouldBeLow = !_batteryManager.charging || _batteryManager.level <= 0.2; |
There was a problem hiding this comment.
🧩 Analysis chain
Verify battery trigger logic - may be too aggressive.
The condition triggers low-power mode if the device is not charging OR battery ≤ 20%. This means:
- A laptop at 100% battery that's unplugged → low-power mode ✓
- A device charging but at 15% → low-power mode ✓
This seems overly aggressive for the first case. Users with devices at high battery levels but unplugged (common for laptops) would unexpectedly have features disabled.
Consider whether the logic should be !_batteryManager.charging && _batteryManager.level <= 0.2 (both conditions must be true) to only trigger low-power mode when battery is actually running low while unplugged.
Please verify the intended behavior matches the current implementation. You can also search for similar battery management patterns:
🌐 Web query:
Best practices for battery API thresholds in web applications
💡 Result:
Below are concise best-practice recommendations for choosing and using battery thresholds in web apps (how to detect/act, safe numeric thresholds, UX, privacy/compatibility). Sources: W3C Battery spec and MDN plus privacy analyses. (w3.org)
- Feature-detect and degrade gracefully
- Use navigator.getBattery() only when available; provide fallbacks (no-op, heuristic, server-driven limits). (w3.org)
- Prefer event-driven updates, avoid tight polling
- Subscribe to batterylevel/chargingchange events and throttle handlers; don’t poll frequently. This reduces CPU/network and battery churn. (w3.org)
- Use conservative numeric thresholds + hysteresis
- Typical breakpoints: “low” ~20% (begin power-saving), “critical” ~5–10% (stop nonessential work / save state).
- Apply hysteresis (e.g., only change state when crossing thresholds ±2–5%) to prevent oscillation when battery hovers near a cutoff. (These numeric ranges are conventional guidance — adapt per app). (w3.org)
- Respect charging state and rate
- If charging, relax limits (or use different thresholds). Consider charging rate/plugged-in info before forcing heavy reductions. (w3.org)
- Minimize precision & avoid fingerprinting
- Don’t rely on high-precision battery numbers; round/quantize reported values (e.g., 10% buckets) and avoid exposing exact timestamps — reduces fingerprinting risk and follows spec guidance. (w3.org)
- Restrict heavy or background work first
- When below thresholds: reduce polling frequency, suspend background sync, defer noncritical renders/animations, lower network sync rates, batch tasks. Save user data before critical cutoff. (developer.mozilla.org)
- Make behavior transparent and user-controllable
- Inform users (small unobtrusive notice) when you enter power-saving mode and provide a way to opt-out/override for essential flows. (w3.org)
- Test across devices/browsers and log metrics
- Different devices have different battery reporting and behaviors; test thresholds on representative hardware and measure user-visible outcomes before choosing defaults. (developer.mozilla.org)
- Privacy & permissions
- Treat battery info as potentially sensitive: don’t combine with other signals to fingerprint users; follow browser privacy recommendations (limit precision, request permission if required). Audit usages. (benthamsgaze.org)
If you want, I can:
- propose specific threshold values tuned to your app type (media, realtime sync, editor), or
- produce a small JS pattern (getBattery + event handlers + hysteresis + throttling). Which would you prefer?
Citations:
- 1: https://www.w3.org/TR/battery-status/?utm_source=openai
- 2: https://www.w3.org/TR/battery-status/?utm_source=openai
- 3: https://www.w3.org/TR/battery-status/?utm_source=openai
- 4: https://www.w3.org/TR/battery-status/?utm_source=openai
- 5: https://www.w3.org/TR/battery-status/?utm_source=openai
- 6: https://www.w3.org/TR/battery-status/?utm_source=openai
- 7: https://developer.mozilla.org/en-US/docs/Web/Progressive_web_apps/Guides/Best_practices?utm_source=openai
- 8: https://www.w3.org/TR/battery-status/?utm_source=openai
- 9: https://developer.mozilla.org/en-US/docs/Web/Progressive_web_apps/Guides/Best_practices?utm_source=openai
- 10: https://www.benthamsgaze.org/2017/03/24/battery-status-not-included-assessing-privacy-in-w3c-web-standards/?utm_source=openai
---
🏁 Script executed:
```shell
#!/bin/bash
# Examine the full function and surrounding context
cat packages/frontend/core/src/modules/effects/highPowerEffects.ts | head -150 | tail -60
Length of output: 2125
🏁 Script executed:
#!/bin/bash
# Search for tests related to battery trigger logic
fd -e test -e spec | xargs rg -l "shouldBeLow|_batteryTriggeredLow|_handleBatteryChange" 2>/dev/null | head -5Length of output: 62
🏁 Script executed:
#!/bin/bash
# Look for comments or documentation about the battery logic intent
rg -n "battery|charging|power.*low" packages/frontend/core/src/modules/effects/highPowerEffects.ts -i -B2 -A2Length of output: 3703
Verify battery trigger logic contradicts W3C best practices.
The condition !_batteryManager.charging || _batteryManager.level <= 0.2 triggers low-power mode if the device is not charging or battery ≤ 20%. W3C best practices recommend considering charging state as a modifier before forcing reductions, not as a standalone trigger.
Your implementation treats "discharging" alone as sufficient to enable power-saving, meaning a device at 100% battery but unplugged immediately enters low-power mode. Industry guidance typically uses combined conditions for "low" thresholds (~20% breakpoint) applied conditionally based on charging state.
The logic should likely be !_batteryManager.charging && _batteryManager.level <= 0.2 (both conditions required) or apply different thresholds based on charging state—only throttling when both unplugged AND battery is actually low.
🤖 Prompt for AI Agents
In packages/frontend/core/src/modules/effects/highPowerEffects.ts around lines
108-109, the current condition uses OR and will enable low-power mode whenever
the device is unplugged regardless of battery level; change the logic to require
both unplugged and low battery (or implement charging-state-specific thresholds)
so low-power mode only triggers when !_batteryManager.charging &&
_batteryManager.level <= 0.2 (or use separate thresholds for charging vs
discharging and apply accordingly); update the boolean expression and any
related comments/tests to reflect the corrected behavior.
| export const initPowerManagement = (): void => { | ||
| if (typeof navigator !== 'undefined' && 'getBattery' in navigator) { | ||
| (navigator as any).getBattery().then((battery: BatteryManager) => { | ||
| _batteryManager = battery; | ||
| _batteryManager.addEventListener('chargingchange', _handleBatteryChange); | ||
| _batteryManager.addEventListener('levelchange', _handleBatteryChange); | ||
|
|
||
| // Set initial state | ||
| _handleBatteryChange(); | ||
| console.log('Battery monitoring initialized.'); | ||
| }).catch((err: any) => { | ||
| console.error('Failed to initialize battery monitoring:', err); | ||
| }); | ||
| } else { | ||
| console.warn('Battery Status API not supported in this environment or browser. Automatic battery-based power reduction will not be active.'); | ||
| } | ||
| }; |
There was a problem hiding this comment.
Add cleanup mechanism to prevent memory leaks.
Event listeners are attached to _batteryManager (lines 131-132) but never removed. If the module is reloaded or the app navigates away, these listeners will leak memory. Additionally, if initPowerManagement is called multiple times, duplicate listeners will be attached.
Consider adding a cleanup function and guards:
+/**
+ * Cleans up power management resources.
+ * Should be called when the module is disposed or app unmounts.
+ */
+export const disposePowerManagement = (): void => {
+ if (_batteryManager) {
+ _batteryManager.removeEventListener('chargingchange', _handleBatteryChange);
+ _batteryManager.removeEventListener('levelchange', _handleBatteryChange);
+ _batteryManager = null;
+ console.log('Battery monitoring disposed.');
+ }
+};
+
+let _isInitialized = false;
+
export const initPowerManagement = (): void => {
+ if (_isInitialized) {
+ console.warn('Power management already initialized.');
+ return;
+ }
+
if (typeof navigator !== 'undefined' && 'getBattery' in navigator) {
(navigator as any).getBattery().then((battery: BatteryManager) => {
_batteryManager = battery;
_batteryManager.addEventListener('chargingchange', _handleBatteryChange);
_batteryManager.addEventListener('levelchange', _handleBatteryChange);
// Set initial state
_handleBatteryChange();
+ _isInitialized = true;
console.log('Battery monitoring initialized.');
}).catch((err: any) => {
console.error('Failed to initialize battery monitoring:', err);
});
} else {
console.warn('Battery Status API not supported in this environment or browser. Automatic battery-based power reduction will not be active.');
}
};🤖 Prompt for AI Agents
In packages/frontend/core/src/modules/effects/highPowerEffects.ts around lines
127 to 143, the battery event listeners are added but never removed which can
cause leaks and duplicate handlers on multiple initializations; add a guard so
initPowerManagement is no-op if already initialized (e.g. check _batteryManager
or an _initialized flag), register listeners only once, and expose or return a
cleanup function (or export a separate teardown function) that removes the
'chargingchange' and 'levelchange' listeners from _batteryManager and clears the
_batteryManager/_initialized state; also ensure the cleanup is called on error
paths and before re-attaching listeners when reinitializing.
1 participant
Battery & Power Management System
🔧 What's Included
Core Changes (
frontendmodule)Added battery monitoring via
navigator.getBattery()with listeners for:chargingchangelevelchangeAdded unified internal control flags:
manualOverrideLowthermalTriggeredLowbatteryTriggeredLowDerived rendering settings from combined state:
liveRenderingexpensiveAnimationsrenderQualityAdded
_applySettings()to propagate changes toglobalThis.APP_SETTINGSand dependent modulesAdded
initPowerManagement()for initialization at app startupUpdated thermal-state logic to coexist cleanly with battery-triggered conditions
🎯 Behavior
Automatically disables high-power effects when:
Automatically restores high-power features when conditions improve (unless manually overridden)
Manual overrides take precedence and prevent environmental triggers
🧪 Testing
Manual logs confirm correct transitions during testing.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.