fix: make tracker.stop() idempotent

[davidberenstein1957]

Description

Calling tracker.stop() twice wrote a second complete emissions row (two CSV rows, two API POSTs, two handler.exit() calls) for a single run, because nothing recorded that the tracker had already stopped. _initialize_runtime_state() now initializes _is_stopped, final_emissions, and final_emissions_data; stop() returns early once _is_stopped is set, returning the cached final_emissions; the lock release moved after that guard so a repeat stop() no longer retries os.remove on an already-removed lock file; and the misplaced else: logger.warning(...) (which was bound to the _scheduler_monitor_power check, not a stop-state check) is replaced by the real terminal-state guard.

Related Issue

Fixes #1307

Motivation and Context

__exit__ calls stop() unconditionally, so a with block combined with an explicit stop() call (the usual way to get the return value) double-counted the whole run's emissions.

How Has This Been Tested?

tests/test_emissions_tracker.py::TestCarbonTracker::test_offline_tracker_stop_is_idempotent asserts one CSV row and an identical return value after a double stop. It fails on master (2 rows) and passes with this change. Full file: 32 passed.

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 (ee7ff84).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1336      +/-   ##
==========================================
+ Coverage   91.70%   91.78%   +0.08%     
==========================================
  Files          49       49              
  Lines        5157     5162       +5     
==========================================
+ Hits         4729     4738       +9     
+ 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.