iOS/iPadOS device vitals: skip no-op writes to reduce writer load

[nulmete]

Related issue: Relates to #49984

Every DeviceInformation refetch unconditionally rewrote both new vitals tables even when nothing had changed since the last refetch — raised as a nit during review of #50046 (#50046 (comment)). This mirrors what SetOrUpdateHostDisksSpace/UpdateHostOperatingSystem already do for other per-refetch host data: skip the write when the stored data already matches. Battery level tolerates a 5-point delta so it doesn't defeat the optimization on nearly every refetch.

Checklist for submitter

If some of the following don't apply, delete the relevant line.

• Changes file added for user-visible changes in changes/, orbit/changes/ or ee/fleetd-chrome/changes.
  See Changes files for more information.
  It will be included in the feature branch.

• Input data is properly validated, SELECT * is avoided, SQL injection is prevented (using placeholders for values in statements), JS inline code is prevented especially for url redirects, and untrusted data interpolated into shell scripts/commands is validated against shell metacharacters.

• Timeouts are implemented and retries are limited to avoid infinite loops

• If paths of existing endpoints are modified without backwards compatibility, checked the frontend/CLI for any necessary changes

Testing

• Added/updated automated tests

• Where appropriate, automated tests simulate multiple hosts and test for host isolation (updates to one hosts's records do not affect another)

• QA'd all new/changed functionality manually

For unreleased bug fixes in a release candidate, one of:

• Confirmed that the fix is not expected to adversely impact load test results
• Alerted the release DRI if additional load testing is needed

Database migrations

• Checked schema for all modified table for columns that will auto-update timestamps during migration.
• Confirmed that updating the timestamps is acceptable, and will not cause unwanted side effects.
• Ensured the correct collation is explicitly set for character columns (COLLATE utf8mb4_unicode_ci).

New Fleet configuration settings

• Setting(s) is/are explicitly excluded from GitOps

If you didn't check the box above, follow this checklist for GitOps-enabled settings:

• Verified that the setting is exported via fleetctl generate-gitops
• Verified the setting is documented in a separate PR to the GitOps documentation
• Verified that the setting is cleared on the server if it is not supplied in a YAML file (or that it is documented as being optional)
• Verified that any relevant UI is disabled when GitOps mode is enabled

fleetd/orbit/Fleet Desktop

• Verified compatibility with the latest released version of Fleet (see Must rule)
• If the change applies to only one platform, confirmed that runtime.GOOS is used as needed to isolate changes
• Verified that fleetd runs on macOS, Linux and Windows
• Verified auto-update works from the released version of component to the new version (see tools/tuf/test)

Summary by CodeRabbit

• Bug Fixes
  • Improved syncing of Apple device vitals so unchanged data is no longer rewritten unnecessarily.
  • Made battery and timestamp comparisons more tolerant, reducing false change detection.
  • Ensured subscription updates handle replacements more reliably by removing outdated entries and preserving unchanged ones.

[nulmete]

@coderabbitai full review

[coderabbitai]

✅ Action performed

Full review finished.

[coderabbitai]

Walkthrough

Apple MDM device vitals now use no-op detection before persistence. Comparisons tolerate small battery changes, equivalent timestamps, and reordered JSON keys. Changed or missing rows retain update-then-insert behavior. Subscription replacement loads existing records, removes stale slots, and skips unchanged records. Tests cover helper comparisons and end-to-end persistence behavior.

Possibly related PRs

• fleetdm/fleet#50046: Extends the same vitals persistence logic with no-op detection and subscription update skipping.
• fleetdm/fleet#50047: Introduces the vitals parsing and persistence path enhanced by this change.
• fleetdm/fleet#50082: Modifies Apple MDM device vitals persistence and related host API loading.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description explains the change and related issue, but the required checklist remains largely unchecked, including automated testing and QA items.	Complete the applicable checklist items, especially automated tests, host-isolation testing, manual QA, load-impact assessment, and database checks, or remove non-applicable sections.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the main change: skipping no-op writes for iOS and iPadOS device vitals.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 49984-vitals-dedup-writes

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@server/datastore/mysql/apple_mdm_device_vitals.go`:
- Around line 164-176: Update batteryLevelPtrEqual so an exact
five-percentage-point change, including 0.40 versus 0.45, is treated as changed
despite floating-point rounding; use a small boundary epsilon or quantized
battery-point comparison while preserving nil handling and the under-five-point
equality behavior. Add a regression test covering 0.40 and 0.45.

🪄 Autofix

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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a5047e10-b63a-4c81-9a4f-cd514be7dd91

📥 Commits

Reviewing files that changed from the base of the PR and between fe0f92c and 2b5ed55.

📒 Files selected for processing (2)

• server/datastore/mysql/apple_mdm_device_vitals.go
• server/datastore/mysql/apple_mdm_device_vitals_test.go

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Handle floating-point rounding at the battery threshold.

Line 176 compares a binary floating-point difference directly with 0.05. For example, 0.45 - 0.40 can evaluate slightly below 0.05. An exact five-point change can then skip persistence.

Compare with a small boundary epsilon, or compare quantized battery points. Add a regression case for 0.40 and 0.45.

Proposed fix

 func batteryLevelPtrEqual(a, b *float64) bool {
 	const batteryLevelTolerance = 0.05
+	const batteryLevelBoundaryEpsilon = 1e-9
 	if a == nil || b == nil {
 		return a == b
 	}
 	diff := *a - *b
 	if diff < 0 {
 		diff = -diff
 	}
-	return diff < batteryLevelTolerance
+	return diff < batteryLevelTolerance-batteryLevelBoundaryEpsilon
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// batteryLevelPtrEqual treats a battery level change under 5 percentage
	// points as unchanged, since it otherwise drifts on nearly every refetch and
	// would defeat the point of skipping no-op writes.
	func batteryLevelPtrEqual(a, b *float64) bool {
	const batteryLevelTolerance = 0.05
	if a == nil || b == nil {
	return a == b
	}
	diff := *a - *b
	if diff < 0 {
	diff = -diff
	}
	return diff < batteryLevelTolerance
	// batteryLevelPtrEqual treats a battery level change under 5 percentage
	// points as unchanged, since it otherwise drifts on nearly every refetch and
	// would defeat the point of skipping no-op writes.
	func batteryLevelPtrEqual(a, b *float64) bool {
	const batteryLevelTolerance = 0.05
	const batteryLevelBoundaryEpsilon = 1e-9
	if a == nil || b == nil {
	return a == b
	}
	diff := *a - *b
	if diff < 0 {
	diff = -diff
	}
	return diff < batteryLevelTolerance-batteryLevelBoundaryEpsilon
	}

🤖 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 `@server/datastore/mysql/apple_mdm_device_vitals.go` around lines 164 - 176,
Update batteryLevelPtrEqual so an exact five-percentage-point change, including
0.40 versus 0.45, is treated as changed despite floating-point rounding; use a
small boundary epsilon or quantized battery-point comparison while preserving
nil handling and the under-five-point equality behavior. Add a regression test
covering 0.40 and 0.45.

[nulmete]

Good catch — 0.45 - 0.40 evaluates to 0.04999999999999999 in float64, so an exact 5-point change could round the wrong way and get skipped. Fixed by subtracting a small epsilon from the tolerance boundary, plus a regression test for 0.40 vs 0.45: 5d137dd.

[codecov]

Codecov Report

❌ Patch coverage is 81.94444% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.27%. Comparing base (fe0f92c) to head (5d137dd).

Files with missing lines	Patch %	Lines
server/datastore/mysql/apple_mdm_device_vitals.go	81.94%	8 Missing and 5 partials ⚠️

Additional details and impacted files

@@                     Coverage Diff                     @@
##           49984-ios-ipados-vitals   #50581      +/-   ##
===========================================================
- Coverage                    68.28%   68.27%   -0.01%     
===========================================================
  Files                         3953     3953              
  Lines                       252500   252492       -8     
  Branches                     13495    13495              
===========================================================
- Hits                        172410   172392      -18     
- Misses                       64681    64687       +6     
- Partials                     15409    15413       +4     

Flag	Coverage Δ
backend	69.54% <81.94%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.