fix: refuse restart after stop()

[davidberenstein1957]

Description

Stop idempotency itself (a repeat stop() writing only one CSV row) is covered by the already-merged #1408. This PR adds a _stopped guard, set by stop(), and checked by both start() and start_task(), so calling either after stop() logs the same clear "already stopped, cannot be restarted" error instead of running into whatever the now-torn-down state produces. On master, start_task() after stop() hits self._scheduler_monitor_power.start() on None, since _scheduler is None post-stop, and raises AttributeError; start() after stop() was not previously guarded either. _stopped is initialized in _initialize_runtime_state() and only ever set in stop().

Also adds a regression test, test_stop_releases_the_lock_only_once, covering that a repeat stop() does not release the lock a second time (the lock by then may belong to a different tracker instance).

Related Issue

None. Stop idempotency itself (one CSV row on a double stop()) is covered by the already-merged #1408.

Motivation and Context

start() and start_task() are two separate entry points that a caller can invoke after stop(). Before this PR, neither refused to run after a stop, so a caller could hit stale or torn-down state; in the case of start_task(), that meant an unrelated AttributeError from calling .start() on the now-None _scheduler_monitor_power, instead of a clear error saying the tracker cannot be restarted.

How Has This Been Tested?

tests/test_emissions_tracker.py::TestRestartAfterStop::test_start_after_stop_is_refused calls start() after stop() and asserts the "cannot be restarted" error is logged.

tests/test_emissions_tracker.py::TestRestartAfterStop::test_start_task_after_stop_is_refused calls start_task() after stop() and asserts the same error is logged, with no active task started, instead of an AttributeError. It fails on master (AttributeError: 'NoneType' object has no attribute 'start') and passes with this change.

tests/test_emissions_tracker.py::TestCarbonTracker::test_stop_releases_the_lock_only_once asserts Lock.release() is called exactly once across two stop() calls.

Full file: 38 passed (tests/test_emissions_tracker.py + tests/test_offline_emissions_tracker.py).

Screenshots (if appropriate):

N/A

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

AI Usage Disclosure

• 🟥 AI-vibecoded
• 🟠 AI-generated
• ⭐ AI-assisted
• ♻️ No AI used

Checklist:

• My code follows the code style of this project.
• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have read the docs/how-to/contributing.md document.
• I have added tests to cover my changes.
• All new and existing tests passed.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.78%. Comparing base (e5e46ab) to head (044c0d6).
⚠️ Report is 6 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1336      +/-   ##
==========================================
+ Coverage   91.70%   91.78%   +0.08%     
==========================================
  Files          49       49              
  Lines        5157     5160       +3     
==========================================
+ Hits         4729     4736       +7     
+ Misses        428      424       -4     

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

[benoit-cty]

🤖 This review comment was written and posted by Claude Opus 5.5 (AI assistant), at the request of @benoit-cty. Findings were checked by reading the code and running tests locally (merged with current master where relevant), but please double-check before acting on them.

Verdict: 🔧 Request changes (rebase down, or close)

Mostly superseded by #1408 (8171d13, now on master). stop() already sets _start_time = None and returns the cached final_emissions, and issue #1307 is closed. The only new behaviour left here is "refuse to restart after stop".

Must fix:

1. Conflicts with master, and the remaining logic won't work after a rebase.

   • The _stopped_at guard duplicates fix: make tracker.stop() idempotent #1408, and conflicts with it in stop() (emissions_tracker.py ~L911-925).
   • The restart refusal sits inside if self._start_time is not None: in start(). After fix: make tracker.stop() idempotent #1408 that value is None once stopped, so the refusal would never fire.
   • If you keep this PR, rebase it down to the restart refusal only, and use a dedicated flag (e.g. self._stopped = True set in stop()).

2. Behaviour choice to confirm with maintainers. On master today, start → stop → start → stop "works", noisily:

   • The second start() logs a suppressed AttributeError: 'NoneType' object has no attribute 'start', because the scheduler is None.
   • The second stop() writes a second CSV row whose energy includes the first run's.

   With this PR, the second run writes nothing and only logs an error. Refusing is defensible, but the error message should say so clearly and suggest creating a new tracker. The alternative is to support restarting properly: re-create the scheduler and reset the counters.

Nit:

• test_offline_tracker_stop_is_idempotent overlaps with fix: make tracker.stop() idempotent #1408's tests. Drop it when rebasing.

[davidberenstein1957]

Made the changes in ee7ff84: merged master, reduced to the restart refusal only.

• Dropped the _stopped_at guard (fix: make tracker.stop() idempotent #1408 covers it) and test_offline_tracker_stop_is_idempotent.
• start() after stop() now checks a dedicated _stopped flag and logs a clear error suggesting a new tracker.
• Kept the lock-released-once test, since fix: make tracker.stop() idempotent #1408 added no tests.

Refusing vs. properly supporting restart is still your call.