[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 93.75%. Comparing base (e159a68) to head (adab363).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1336      +/-   ##
==========================================
+ Coverage   93.66%   93.75%   +0.08%     
==========================================
  Files          50       50              
  Lines        5591     5600       +9     
==========================================
+ Hits         5237     5250      +13     
+ Misses        354      350       -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.

[benoit-cty]

Rebased on master and cleaned up. Head is now adab363.

Rebase

• ee7ff84 merged master into this branch. Because of that, a plain rebase replayed the old _stopped_at commit (334f76d), which conflicted with fix: make tracker.stop() idempotent #1408. I rebuilt the branch as a linear history on master instead:
  • 4fd5ea3: net content of ee7ff84
  • 49ae9d6: 044c0d6, cherry-picked
• 334f76d is dropped, since ee7ff84 had already reverted it. The net diff is the same as before and David is still the author of both commits.

Review

• No correctness bugs found.
• The earlier review requests are all handled in the code:
  • the guard is a dedicated _stopped flag;
  • the duplicate idempotent-stop test is removed;
  • the error message says to create a new tracker.
• In-repo callers (@track_emissions, the CLI, the examples) all create a new tracker for each run, so none of them are affected.

Commits added

• 529b6c6 refactor(tracker): the error message was copied in both start() and start_task(). It is now one module constant, _RESTART_AFTER_STOP_ERROR.
• adab363 docs: docs/tutorials/python-api.md now says that a stopped tracker cannot be restarted and that you should create a new one.

Validation

• All changed lines in codecarbon/ are covered (test_emissions_tracker.py + test_offline_emissions_tracker.py: 45 passed).
• pre-commit passes, and uv run task docs builds with the link check passing.
• Full package suite: 831 passed, 4 failed. The failures are in test_rapl_mmio_scanning / test_rapl_permissions. They also fail on master in my environment, most likely because the tests run as root. They are not related to this PR.

Left for the maintainer

• Should a tracker refuse to restart after stop() (this PR), or support restarting properly by re-creating the scheduler and resetting the counters? I left the approach unchanged.

---

Generated by Claude Code