Skip to content

refactor(metrics): Isolate visible sampling sessions - #113

Merged
bigtomcat6 merged 2 commits into
mainfrom
feat/energy-impact-sampling-lifecycle
Aug 9, 2026
Merged

refactor(metrics): Isolate visible sampling sessions#113
bigtomcat6 merged 2 commits into
mainfrom
feat/energy-impact-sampling-lifecycle

Conversation

@bigtomcat6

@bigtomcat6 bigtomcat6 commented Aug 9, 2026

Copy link
Copy Markdown
Owner

Summary

Refactor energy impact data flow so sampling is isolated to currently visible sessions:

  • Split sampling-session filtering into EnergyImpactSampler and move related orchestration out of the provider and model.
  • Update EnergyImpactProvider and EnergyImpactModel to consume the isolated sampler output.
  • Expand unit test coverage across app and core energy-impact paths to preserve behavior during the refactor.

How to test

  • Run the affected energy-impact focused test suites before/after the refactor.
  • Manually verify the Energy Impact screen with a representative sampling workload and confirm visible-session totals align with expectations.

Release impact

  • Type: Fixed
  • Release note:
    • Restrict energy-impact sampling calculations to visible sessions so metrics are no longer affected by invisible/filtered sessions.

Checklist

  • I have seen this code, I have run this code, and I take responsibility for this code.
  • PR title follows type(scope): Summary.
  • Tests or a clear manual verification path are included.
  • User-facing changes include release-note wording or Release impact: None.

@bigtomcat6 bigtomcat6 added the bugfix PR Label label Aug 9, 2026
@codecov

codecov Bot commented Aug 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.41488% with 67 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...tyCore/Metrics/Providers/EnergyImpactSampler.swift 91.25% 48 Missing ⚠️
...yCore/Metrics/Providers/EnergyImpactProvider.swift 67.85% 18 Missing ⚠️
...rces/MacActivityApp/Models/EnergyImpactModel.swift 98.93% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@bigtomcat6

Copy link
Copy Markdown
Owner Author

@codex review

@bigtomcat6 bigtomcat6 self-assigned this Aug 9, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Already looking forward to the next diff.

Reviewed commit: c40087a1c4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@bigtomcat6 bigtomcat6 left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed

@bigtomcat6
bigtomcat6 marked this pull request as ready for review August 9, 2026 11:53
@bigtomcat6
bigtomcat6 merged commit 17e319e into main Aug 9, 2026
13 of 15 checks passed
@bigtomcat6
bigtomcat6 deleted the feat/energy-impact-sampling-lifecycle branch August 14, 2026 12:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix PR Label

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant