control: close eight defects found in continued review of #4638

[mgazza]

What

Follow-up to #4638: continued review of the merged code found eight defects in the control ownership ledger. This fixes them, with no change to the ledger API the Metrics dashboard (fb8120f) consumes — test_metrics_dashboard_control_conflicts passes unmodified, and is the acceptance test for that claim.

(An earlier revision of this PR proposed replacing the event window with a bare counter; withdrawn — the dashboard's event/sustained detail is exactly what a bare count can't provide, as its own docstring says.)

The fixes

False-accusation paths closed:

• The freshness gate failed open. When either side had no timing metadata the gate was skipped entirely, so a timestamp-less entity plus a vendor serving a cached pre-write read produced exactly the false accusation the gate exists to prevent. Without timing metadata, a divergence must now persist across two distinct cycles before it is reported (a new PENDING verdict) — a cached read resolves on the next poll; somebody genuinely holding the control does not. Costs one cycle of latency on timestamp-less entities only.
• sustained_controls() conflated inverters. It counted by control name, so on a three-inverter install one external change per inverter tripped the sustained threshold. It now counts per entity and reports the control name.

Silent-blindness paths closed:

• quick_inverter_data_update() observed outside any ledger cycle. It runs every 120s between plan runs and confirms through update_status(), so all of its observations were unconditionally classified stale — a third-party change between plan runs was silently reverted without ever being reported. Every entry point that can observe now opens its own cycle.
• A control at Predbat's target was never re-owned after an event or a clear. The no-write-needed early return skipped record_write(), so once ownership was dropped the control became permanently undetectable. A matching read now re-arms ownership when no record exists, recorded as confirmed_by: "read" rather than "write" — weaker evidence, accepted because the alternative is permanent blindness on any control Predbat has no reason to rewrite. (write_and_poll_option needed no change — it always writes at least once — and a test pins that.)

Robustness:

• Clock-ahead poisoning. A process booting with its clock ahead stamps events in the future; once the clock corrects, those events were never pruned and permanently occupied the 20-event publish cap, evicting all real history. prune() now clamps a future-dated timestamp to the current time — the detection was real, the timestamp was not — so the event ages out naturally instead of becoming immortal.
• newest_events(): one branch returned an unsorted slice violating its own oldest-first contract, and limit=0 returned the entire list ([-0:]). Both fixed and pinned.
• Calibration/read-only ledger clears now log, and the docstring records why the clear is deliberately global (the calibration branch writes rates, target and reserve to every inverter through the ordinary helpers before breaking out).
• Doc/test sweeps: a docstring still describing a deleted suppression path, and a source-order test that string-scraped for one spelling of fetch_inverter_data() — replaced with a behavioural pin.

Two review findings are deliberately not addressed here because they are design questions about the event window rather than defects — raised separately with our findings so they can be decided rather than patched around.

Testing

Full quick suite (what CI runs): 268 passed, 0 failed, including test_metrics_dashboard_control_conflicts unmodified. Each behavioural fix was verified by reverting it and watching its test fail.

🤖 Generated with Claude Code

[Copilot]

🟡 Changes recommended

It removes ControlLedger’s events/recent_events/newest_events/sustained_controls APIs but other code paths (notably PredBat._emit_snapshot_metrics() and metrics/dashboard tests) still call them, which will cause runtime/test failures.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates Predbat’s control-interference detection to publish predbat.control_conflicts as a monotonic counter (event_count) plus “last event” attributes, and adjusts the control-ledger logic to address several correctness issues around cycle handling, ownership re-arming, and timestamp-less freshness gating.

Changes:

• Replace the in-process rolling 24h event window/list machinery with a counter (event_count) + last_event details, and update predbat.control_conflicts publishing accordingly.
• Fix ledger correctness around observation cycles and ownership re-arming (including a new record_agreement() path for “no write needed” cases) and add a PENDING verdict for timestamp-less divergence that must repeat across cycles.
• Improve diagnostics by logging a detailed line on EXTERNAL events and documenting/logging global ledger clears.

File summaries

File	Description
apps/predbat/control_ledger.py	Removes windowed event list machinery; adds event_count, last_event, PENDING, and re-arming support via record_agreement().
apps/predbat/predbat.py	Publishes predbat.control_conflicts as a counter + last-event attributes instead of a 24h window.
apps/predbat/inverter.py	Logs detailed EXTERNAL events and re-arms ownership on “no write needed” early returns.
apps/predbat/execute.py	Makes ledger clears explicitly global with a logged reason; ensures quick_inverter_data_update() opens its own ledger cycle.
apps/predbat/tests/test_control_ledger.py	Updates unit tests to the new counter/last-event model and adds new regression tests for the fixed defect cases.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

prune() can leave the event list out of timestamp order after clamping, causing recent_events() to violate its documented oldest-first ordering contract.

Review details

Suppressed comments (1)

apps/predbat/control_ledger.py:565

• In prune(), future-dated events are clamped by mutating event["at"], but the list is not re-sorted afterwards. That can leave self.events out of timestamp order (e.g. when a far-future event is clamped to now but a within-jitter event remains slightly in the future), which means recent_events() can return a list that violates its “oldest first” contract.

        self.events = self._in_window(now, window_s, drop_future=False)
        # Keep the protected set in step, or it grows for the life of the process and can protect
        # events that have already aged out of the store.
        surviving = {id(event) for event in self.events}
        self.session_events = [event for event in self.session_events if id(event) in surviving]

• Files reviewed: 5/5 changed files
• Comments generated: 0 new
• Review effort level: Lite