Allow removing devices the integration no longer provides

[notexpected]

Summary

Implements per-device removal (#174 — where the workaround is still "delete the
whole integration entry and re-add it"): the integration now defines
async_remove_config_entry_device, so every device page gets Home Assistant's
Delete action, and devices the coordinator no longer provides can be removed
individually — an inverter dropped from the station or the configuration, a
battery module no longer reported, a dissolved parallel group, or a
legacy-format duplicate left behind by an older version.

Design

A device is deletable only when none of its identifiers is currently
provided by the coordinator. The provided set covers all four identifier shapes
the integration registers:

• {serial} — inverters, GridBOSS, parallel groups (the keys of
  coordinator.data["devices"])
• {serial}_battery_bank — kept while the parent inverter is provided,
  even on cycles where bank sensors are temporarily absent (link-down reads,
  shared-battery secondary inverters — Secondary Inverter Battery Bank show multiple sensors as Unknown / Unavailable - Primary Inverter Battery Bank No problems #169)
• per-module battery keys from device_data["batteries"]
• station_{plant_id} — while station data is present (a station device left
  over after moving to a LOCAL connection becomes removable)

Refusals are deliberately conservative, applying the #217 smart-port-cleanup
lesson that placeholder/degraded cycles must never be read as authoritative
absence:

• Live devices are refused — their entities would recreate them on the next
  cycle under fresh registry entries, losing customizations and breaking
  registry-pinned automations.
• A failed last update or an empty device table refuses everything — an
  outage can't make the fleet look stale.
• Battery-shaped identifiers ({parent}-…) whose parent is provided but
  carries an empty batteries dict are refused — the LOCAL-mode first
  refresh reports inverters with "batteries": {} before the first real poll,
  and a live battery module must not be deletable during that window. Once the
  parent has authoritative battery data, a key absent from it (round-robin
  eviction, module removed) is removable.

Changes

• device_removal.py (new module): the async_remove_config_entry_device
  hook plus the _provided_device_identifiers helper.
• __init__.py: re-exports the hook (Home Assistant resolves it by name on
  the component module); no other changes.
• tests/test_device_removal.py (new file): 13 tests covering every identifier shape, the
  placeholder-cycle guard, the failed-update/empty-table/never-loaded refusals,
  and legacy serial-based parallel-group ids.
• docs/TROUBLESHOOTING.md: FAQ entry describing the flow and the
  refuse-if-still-provided rule.
• CHANGELOG.md: Unreleased entry.

Testing

• pytest tests/ passes (full suite).
• ruff check / ruff format --check (CI-pinned 0.15.5) clean.
• mypy --config-file tests/mypy.ini (strict) clean.
• Validated live on a Hybrid-connection system (FlexBOSS21 + GridBOSS,
  3.5.0 + this patch): the registry carried a ghost FlexBOSS21
  (44700E0241, left behind by a hardware swap and parked as disabled
  because there was no delete path — the exact Remove disabled device #174 situation) plus its
  battery bank. Removing the live inverter via
  config/device_registry/remove_config_entry was refused
  ("Failed to remove device entry, rejected by integration"); removing the
  ghost inverter and its battery bank succeeded, and afterwards the entity
  registry held zero entities referencing the ghost serial while all live
  devices and entities were untouched.

Disclosure

This PR was written by Claude Fable 5 (Anthropic's AI assistant, model
claude-fable-5, working in Claude Code) and reviewed by me before
submission. The live validation above was performed on my own
FlexBOSS21 + GridBOSS system.

[btli]

Code review

Not ready to merge. Adversarial review (gpt-5.6-sol + gemini-3.1-pro, independently) surfaced six defects; I verified each against the source. Three of them are the severe class this PR explicitly set out to prevent — permitting deletion of a device that is still live.

The root problem is that "provided" is computed from a single coordinator cycle, but battery presence in any one cycle is a subset, not the truth.

1. A partial battery payload authorizes deleting a live module. The placeholder guard only fires when the parent's batteries dict is entirely empty. A non-empty-but-incomplete dict passes straight through.

https://github.com/notexpected/eg4_web_monitor/blob/abecc2176375d1965acb04810f1c431870cdaa05/custom_components/eg4_web_monitor/device_removal.py#L86-L98

After a restart the round-robin and carry-forward caches are cold. On an 8-module bank whose first poll exposes modules 1–4, a live …-BAT005 is absent while parent["batteries"] is truthy — so the hook returns True, and rotation recreates the device later under a fresh registry entry. test_issue_252_battery_identity.py already demonstrates reported_count=8 yielding a single key on an early cycle. CLOUD and HYBRID have the same hole, since a cold carry-forward cache can't repair a subset-omitting cloud payload.

2. Battery identifiers without a hyphen bypass the guard completely. The guard keys on "-" in identifier, but the registered identifier is the raw battery_key:

https://github.com/notexpected/eg4_web_monitor/blob/abecc2176375d1965acb04810f1c431870cdaa05/custom_components/eg4_web_monitor/coordinator_mixins.py#L2819-L2823

utils.py shows BAT001-style keys are preserved verbatim, and the round-robin path also produces @pos-suffixed and pos:-style keys. None contain -, so for those the placeholder guard never runs at all. (gemini-3.1-pro found the mirror-image case: a parent serial containing - makes split("-", 1)[0] resolve to a non-existent parent, devices.get(...) returns None, and the guard is skipped the same way.)

3. A degraded parallel-group discovery permits deleting a live group. If the device list succeeds but getParallelGroupDetails fails, pylxpweb swallows the error and returns the members as standalone devices, so the coordinator publishes a healthy non-empty table with parallel_group_a simply missing. last_update_success is True and the table is non-empty, so the still-live group is deletable and reappears on reload.

The remaining three are the inverse — the feature can't do what it was built for:

4. The last physically removed module can never be deleted. Once the coordinator evicts the final module, the steady-state payload is {"INV001": {"batteries": {}}}, which the guard permanently reads as a LOCAL startup placeholder. There's no readiness or phase signal that ever unblocks it.

5. A leftover battery bank is never removable. {serial}_battery_bank is added unconditionally for every device-table key:

https://github.com/notexpected/eg4_web_monitor/blob/abecc2176375d1965acb04810f1c431870cdaa05/custom_components/eg4_web_monitor/device_removal.py#L44-L47

But actual bank registration requires bank data and battery_bank_count > 0. An orphan bank on a shared-battery secondary (#169) stays "provided" for as long as its inverter exists.

6. A plant's former sole inverter can never be deleted. rows=[] is a legitimate successful response; the coordinator completes with a zero device count. The blanket empty-table refusal then blocks exactly the cleanup a user needs after removing their only inverter.

Suggested direction

1–3 need presence to be judged over more than one cycle. The coordinator already tracks battery_last_seen for the #258 carry-forward — deleting a battery is probably only safe once that says the module has been gone for the eviction window, rather than merely absent right now. 4 and 6 want an explicit "authoritative and complete" signal to distinguish a real empty set from a placeholder, instead of inferring it from emptiness. 5 wants the bank identifier added only when a bank is actually registered.

Worth noting for whatever lands: the tests currently encode the unsafe assumptions rather than catching them — test_device_removal.py:103 pins "missing while siblings exist means stale", and :179 classifies every empty table as degraded.

[notexpected]

Thank you — the review was right that the single-cycle foundation was unsound, so 4a8105c replaces it with your suggested direction rather than patching the heuristics: an observation ledger. The coordinator stamps every provided identifier with a monotonic last-seen mark on each successful refresh (never on the 3-strike cached fallback), and deletion requires the identifier to have been absent for its class's full window within the current unbroken run of successful refreshes — 15 minutes for device-class identifiers (serials, parallel groups, station), and BATTERY_CARRY_FORWARD_MAX_AGE (6 h, the #258 eviction window you pointed to) for battery modules and banks. The observation clock restarts on the first success after any failed cycle, so blind time behind an outage never counts as absence. A never-seen identifier is held to the conservative battery window.

Per item:

1. Partial battery payloads — a module seen 90 s ago is refused regardless of what this cycle's dict contains, and a cold session (restart) refuses all battery-class deletions for the first 6 hours outright, so cold round-robin/carry-forward caches can no longer expose live modules. Same mechanism covers CLOUD/HYBRID subset payloads.

2. Identifier shapes — all identifier parsing is gone. Keys are tracked verbatim in every shape the coordinator registers (serial-based, BAT001-style, @pos-suffixed), and a hyphenated parent serial can no longer misroute anything.

3. Degraded parallel-group discovery — the vanished group row was seen minutes ago, so it stays refused for the full 15-minute window; a transient getParallelGroupDetails failure self-heals long before that.

4. Last removed module — an empty batteries dict is steady state now, not a permanent placeholder; the evicted module ages out of the 6-hour window and becomes deletable.

5. Orphan bank — the bank identifier is recorded only under the Secondary Inverter Battery Bank show multiple sensors as Unknown / Unavailable - Primary Inverter Battery Bank No problems #169 registration gate (bank sensors present AND battery_bank_count > 0), so a shared-battery secondary's leftover bank ages out while its parent lives on.

6. Sole inverter — the blanket empty-table refusal is gone. A legitimately empty table lets the former sole inverter age out; outages are covered by last_update_success plus the windows.

Additionally, while any device is degraded — an "error" row (LOCAL link-down, cloud per-device failure) or a mode-independent is_transport_link_down() verdict (HYBRID serves cloud fallback with no row marker) — battery-class deletions are refused outright: a degraded parent cannot attest its modules' absence. The tests you flagged as encoding the unsafe assumptions are rewritten around the ledger, with each of the six scenarios pinned explicitly, and the docs/changelog state the practical rule (after a restart: 15 minutes for device deletions, 6 hours for battery deletions — about 12 hours after physically removing a module, since the carry-forward keeps it visible for its first 6 hours of absence).

Adversarial follow-up — mirroring your process, an independent second review was run against the fix itself before pushing. Its HIGH finding is folded in: the first draft measured window coverage from the session's first success, so time spent blind behind failed cycles counted as absence — a >6 h outage could both wall-clock-evict the #258 carry-forward and satisfy the window at the moment of recovery, re-admitting a narrow live-module deletion. The clock is now observed_since (start of the current contiguous success run, restarting after any failed cycle), with the post-outage recovery attack pinned as a test. It also drove the mode-independent link-down guard above, a wiring test that the coordinator stamps on success and skips the cached fallback, and the honest 12-hour doc timeline. Two residuals are acknowledged rather than fixed, as inherent to the window design: a getParallelGroupDetails degradation persisting past 15 minutes of otherwise-successful CLOUD/HYBRID cycles would let the group be deleted (it reappears with fresh registry ids on recovery — bounded loss, and LOCAL is immune since the degraded row stays error-marked and provided); and a live module that reports nothing for 6+ observed hours (deep sleep) is deletable — consistent with #258's own semantics, whose carry-forward declares such a module gone and drops its entities anyway, and recoverable since the device re-registers on its next appearance.

Full suite (2282), CI-pinned ruff, and strict mypy are clean. Same disclosure as the PR body: this response and the fix commits were written by Claude Fable 5 (model claude-fable-5, working in Claude Code) and reviewed by me before posting.

[btli]

Code review — fix round

The observation ledger is the right shape, and the care shows: deliberately not stamping it on the 3-strike cached-fallback path ("served cache is old evidence, not a fresh sighting") is exactly correct reasoning.

But adversarial review (gpt-5.6-sol at xhigh) found the round delays the unsafe deletions rather than eliminating them — a live device is still deletable in several reachable degraded states. I verified the sharpest one against the source myself. Three confirmed HIGH, ordered by how easily they trigger.

1. A failed cloud device-list at cold start permits deleting a live inverter after six hours.

Station._load_devices() catches a device-list failure and continues with empty device collections; that station still counts as successfully loaded, and later refreshes deliberately don't reload the hierarchy. So the coordinator publishes and records a successful {"devices": {}} snapshot. A registry inverter the new session never saw is classified battery-class unknown at device_removal.py:203, and once both time checks pass it's deletable.

Sequence: HA restarts → plant-details succeeds → the first device-list request fails → the live inverter's registry device remains → the hierarchy stays empty until another reload → six hours later Delete returns True.

This is specifically what replacing the blanket empty-table refusal with timed authorization costs, absent a discovery-completeness signal.

2. A battery-endpoint failure permits deleting live modules after six hours.

Refresh subtasks run with return_exceptions=True, and _fetch_battery_http() swallows API, connection, device and unexpected errors without propagating. Local battery-read failures are caught the same way while runtime stays healthy. So the coordinator publishes a perfectly normal inverter row with an empty module dict — and the guard recognises degradation only via an "error" row or a down whole-device link, neither of which a battery-only failure sets.

After a restart the ledger and carry-forward caches are both cold, so a live module absent from every partial battery response ages out and becomes deletable. Reachable in all three modes: CLOUD with getBatteryInfo failing while runtime succeeds, LOCAL with the battery register block failing, HYBRID with a healthy link exposing only some modules while cloud supplementation fails.

3. The first two failed updates are treated as healthy.

eg4_web_monitor/custom_components/eg4_web_monitor/coordinator.py

Lines 556 to 566 in 4a8105c

	raise
	except UpdateFailed as err:
	self._consecutive_update_failures += 1
	if self._consecutive_update_failures < 3 and self.data is not None:
	_LOGGER.warning(
	"Update failure %d/3, serving cached data: %s",
	self._consecutive_update_failures,
	err,
	)
	return cast(dict[str, Any], self.data)
	raise

Under three consecutive failures _async_update_data returns cached data instead of raising, so HA leaves last_update_success=True — and the hook checks only that flag, not _consecutive_update_failures:

eg4_web_monitor/custom_components/eg4_web_monitor/device_removal.py

Lines 167 to 172 in 4a8105c

	"""
	coordinator = getattr(config_entry, "runtime_data", None)
	if coordinator is None or not coordinator.last_update_success:
	# No healthy data to judge staleness against — refuse rather than
	# let an outage make every device look removable.
	return False

So during a real outage the hook judges against a stale cached table it believes is healthy. Not stamping the ledger on that path — right in itself — means absence keeps aging through the outage. At the 10-minute cloud interval two suppressed failures already exceed the 15-minute device window. The comment calling these failures immaterial doesn't hold for device-class identifiers.

Two more, non-blocking:

4. any_degraded is global across the whole device table, so one permanently offline inverter blocks every battery cleanup indefinitely — even a module whose own healthy parent can attest its absence. That partially reopens the "can't delete the last module" gap this round set out to fix.

5. Because the ledger starts empty on every coordinator construction, every never-seen identifier — inverter, GridBOSS, station, legacy parallel group — takes the conservative battery-class six-hour window. The CHANGELOG advertises 15 minutes after restart, which isn't what device-class entries get.

(Also minor: ledger entries are never pruned, so it grows for the session.)

Direction

1 and 2 are the same gap: the guard trusts "the coordinator returned data" as "discovery was complete", and both cloud discovery and battery fetch can fail silently while the cycle still reports success. An explicit completeness signal — set only when the device list and the battery fetch actually succeeded — would close both, and is what 4 and 5 in my earlier review were also asking for. 3 wants the hook to consult _consecutive_update_failures rather than last_update_success alone.

Worth saying plainly: the asymmetry still governs. Refusing a legitimate deletion is an annoyance; permitting one destroys entity customizations and breaks registry-pinned automations irreversibly. Where the two conflict, err toward refusing.

[notexpected]

Thank you — the fix-round review was right that the ledger delayed the unsafe deletions rather than eliminating them. f72ddc8 replaces "a cycle returned data" with an explicit per-cycle discovery-completeness verdict, and judges absence only over COMPLETE observed time. The guard now trusts a snapshot only when the device list and every battery-capable parent's fetch actually succeeded.

Per item:

1. Failed cloud device-list at cold start. assess_discovery_completeness computes device_list_ok: LOCAL/DONGLE/MODBUS enumerate no devices remotely, so their config-defined set is always complete; for CLOUD/HYBRID a non-empty table is self-evidently loaded, and an EMPTY table is confirmed with a fresh get_devices call before it is trusted. A call that succeeds (even returning zero rows) proves a genuinely empty plant — so the sole inverter of an emptied plant still ages out (your finding 6) — while a raising call marks the list unconfirmed and the device clock never advances, so the still-registered inverter is never authorized no matter how long the outage runs. Rows returned while our station is empty force a hierarchy reload.

2. Battery-endpoint failure. battery_ok is false when any device carries an "error" row or an is_transport_link_down verdict, or when a battery-capable inverter's pylxpweb _battery_cache_time is still None. That stamp advances only on a successful battery fetch (both the transport and the HTTP legs keep cached data untouched on failure), so a cold-restart battery-endpoint outage never masquerades as an empty bank — the battery clock stays unset for the whole outage and no module is authorized. Reachable in all three modes (cloud getBatteryInfo, LOCAL register block, HYBRID link), since all route through inverter.refresh()/_fetch_battery.

3. The first two suppressed failures. The hook now refuses whenever _consecutive_update_failures != 0 — the table is cached-through-an-outage, not a fresh sighting — and the coordinator clears both observation clocks on the cached-fallback path, so absence can no longer age across the <=2-strike blip that keeps last_update_success True.

Each identifier class now has its own observation clock (the start of the current contiguous run of cycles that observed that class completely), so a device-list failure blocks inverter deletion without a battery outage blocking it, and vice versa. A clock is unset on a failed cycle, an incomplete cycle, or a cached fallback, and a complete recovery cycle restarts it at now.

The two non-blocking items and the nit:

4 (global battery clock) -- now closed. Moved to per-parent granularity (e151744). Each battery-capable parent keeps its own observation clock, judged on that parent's own signals -- an "error" row, a down local transport link, or an inverter whose _battery_cache_time is still None resets only that parent -- so a module ages out on ITS parent's clock while a degraded sibling no longer freezes the whole entry, closing the "can't delete the last module" gap you flagged. The ledger now records each battery identifier's parent serial, so a module absent from the current payload is still routed to the right clock. Two cases keep the conservative global clock because there is no parent to attribute: a module whose recorded parent has since left the device table (its per-parent clock is pruned with it), and one never once observed this session (parent, and class, unknowable). A failed device list still drops every per-parent clock.

5 (never-seen window). Left at the conservative 6 h regardless of type — an identifier the running session has never once observed has an unknowable class — but the CHANGELOG and TROUBLESHOOTING now say exactly that (a ghost already gone before the restart waits the 6 h window, not 15 min), so the doc no longer over-promises.

Ledger growth. Pruned on each stamp of entries older than 4 * BATTERY_ABSENCE_WINDOW — far past any deletion decision, so pruning only reclaims memory from identifier churn and never turns a still-relevant sighting into a conservative "never seen" one.

One residual remains, now traced rather than merely asserted consistent with #258: a HYBRID system whose local battery read succeeds (stamping the parent attestable) while its cloud supplement for a 5th..Nth module fails can age that module out. I walked the boundary in pylxpweb source: #258's carry-forward re-presents a module seen even once, with a frozen last_seen, until its own whole-feed staleness gate (_supplemental_feed_stale_after) evicts it — so removal's absence window only starts after #258 has already concluded the module is gone. Removal is therefore strictly downstream of, and consistent with, #258 for any module observed at least once this session. Only the COLD-restart sub-case is left: a module never seen since boot has nothing to carry forward, and pylxpweb stamps the supplemental timestamp even on a failed supplement (_fetch_battery_http swallows every error, then _fetch_supplemental_battery_http stamps unconditionally), so there is no library success signal to gate on. At the integration layer that sub-case is indistinguishable from a genuine post-removal restart, and it self-heals the instant the module reappears. If you'd rather gate on it hard, the clean fix is a supplement-success signal in pylxpweb (a timestamp that advances only on success) that this guard could then require — happy to send that as a follow-up.

The completeness bookkeeping is wrapped so a fault in it clears the clocks (deletion refused — fail-safe) rather than failing the data update every sensor depends on. The scenarios are each pinned as tests (cold-start device-list failure refuses a live inverter; a 7-hour battery outage refuses a never-seen live module; serving cached data refuses; the sole inverter of a confirmed-empty plant still deletes; the last module and a stale station still age out; and for finding 4, a healthy parent's module ages out while a sibling is degraded and the global clock never advances, while a departed parent's module falls back to the global clock), alongside assess_discovery_completeness's own verdicts and the clock-restart logic. Full suite 2299, ruff and strict mypy clean.

Same disclosure as before: this response and the fix commit were written by Claude Fable 5 (model claude-fable-5, working in Claude Code) and reviewed by me on Claude Opus 4.8 (model claude-opus-4-8) before posting.