fix(predbat): don't read db_primary off a None ha_interface in update_time_loop

[springfall2008]

This is an automated draft PR generated from issue #5135 — a maintainer should review it before merging.

Fixes #5135

Summary

update_time_loop's HA-interface guard logged self.ha_interface.db_primary unconditionally, so in the one case the guard exists for — ha_interface being None, which short-circuits the first clause — the log line raised AttributeError before self.fatal_error = True on the next line ever ran. The flag that tells standalone mode to stop Predbat (hass.py:159) therefore stayed False.

db_primary is now only reported when there is an interface to read it from; with no interface the message matches run_time_loop's existing wording ("Error: HA interface not active"), and the flag is set either way.

Testing

• coverage/run_pre_commit — all hooks pass (ruff, black, cspell, interrogate, markdownlint) and the quick suite is green, exit 0.
• New test module apps/predbat/tests/test_time_loop_guard.py, registered as time_loop_guard in TEST_REGISTRY. It clears ha_interface and drives both update_time_loop and run_time_loop, asserting each raises the intended HA interface not active exception (not AttributeError) and leaves fatal_error set.
• Red/green: with the source change stashed (test kept), tools/triage_test.sh time_loop_guard fails on exactly the reported defect — ERROR: update_time_loop raised AttributeError ('NoneType' object has no attribute 'db_primary') followed by ERROR: update_time_loop did not set fatal_error, while run_time_loop already passed. With the fix restored it passes, all four assertions OK.

Notes

• run_time_loop keeps its own copy of the same guard and message rather than sharing a helper with update_time_loop. That duplication pre-dates this change and de-duplicating it would touch the scheduled-run path, so it is left alone; the new test covers both copies so a future divergence is caught.
• Blast radius checked with GitNexus impact({target: "update_time_loop", direction: "upstream"}): LOW risk, one direct caller (initialize, which only schedules it). No behaviour change outside the ha_interface is None branch.
• As the triage comment notes, this path is latent rather than routinely hit — initialize() raises at predbat.py:2002 if ha_interface is unset, before the loop is scheduled — so the fix is about the guard being able to do its job, not a reproduction from a live report.

[springfall2008]

Automated review failed to complete for this PR - see the triage bot's logs for details. Not retrying automatically; remove BOT_FAILED and re-add BOT_REVIEW to try again.

[springfall2008]

Coverage gap: this test only drives the ha_interface is None case, but the only guard branch reachable in production is the other one — interface present, websocket_active False and db_primary False (the websocket disconnect paths set websocket_active = False at ha.py:667/785, and db-primary defaults off). No existing test covers that branch either: test_active_flag sets websocket_active = True so the guard passes, and test_predheat's timer_tick drive (test_predheat.py:136) runs PredHeat's same-named update_time_loop, not PredBat's. A future edit to the condition itself (e.g. dropping the db_primary term, or swapping and/or so a db-primary install with the websocket down now trips the guard) would pass this suite. Worth one more loop pass with ha_interface kept, websocket_active = False, db_primary = False asserting the same raise + fatal_error.

[springfall2008]

This fix now leaves three hand-maintained variants of the same guard semantics: this two-branch log here, run_time_loop's shorter copy at predbat.py:2175 (different wording, no db_primary), and the None-branch message chosen to match it. The PR body documents why it didn't dedupe, and the new test pins both copies so divergence is caught — but any future change to the guard's contract (new attribute in the message, different flag, changed exception) still has to be made in two places. If another guard tweak lands, consider extracting one _check_ha_interface() helper returning a reason string that both loops log and raise from.

[springfall2008]

Minor: my_predbat is unused here — the loops are already passed as bound methods, so def _run_guard(loop) does the same job. (Same shape as _run_loop_expecting_failure in test_active_flag.py, so this is only worth tidying if that helper is touched too.)

[Copilot]

Copilot review overview

🟢 Approval recommended

The fix is narrowly scoped to the failing branch and is covered by a focused regression test that reproduces the original defect and verifies the intended behavior.

Review effort: Lite
Findings: None

What changed in this PR

Fixes a crash in PredBat.update_time_loop() when ha_interface is None, ensuring the HA-interface guard reliably sets fatal_error and raises the intended "HA interface not active" exception instead of failing early with AttributeError.

Changes:

• Guard log in update_time_loop now only reads/logs db_primary when ha_interface is present.
• Added a regression test covering both update_time_loop and run_time_loop behavior when ha_interface is None.
• Registered the new test in the unit test registry.

File	Description
apps/​predbat/​predbat.py	Prevents AttributeError in the HA-interface guard by conditionally logging db_primary only when ha_interface exists.
apps/​predbat/​tests/​test_time_loop_guard.py	Adds a regression test ensuring both loops raise the correct exception and set fatal_error when ha_interface is missing.
apps/​predbat/​unit_test.py	Registers the new time_loop_guard test in the suite.

---

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