Production-ready audit: portability, engine safety, CI & dashboard fixes (#14–#31) - #12
Merged
Merged
Conversation
…rgeting, recipe errors - #15: replace removed `hass.helpers.template.now()` with `homeassistant.util.dt.now()` in all six service handlers; services no longer crash on current HA cores. - #16: services and computed sensors resolve a `room` slug to that room's entity-id prefix (default room = un-prefixed, unchanged). Adds optional `room` field to the service schemas; None-guards the check_transition_conditions setpoint reads. - #27 (recipe half): apply_recipe / save_recipe now raise HomeAssistantError instead of silently doing nothing when the recipe store is unavailable. - #31: replace the five dead SERVICE_* constants with the real service names (consumed by the SERVICES dict); manifest iot_class local_polling -> calculated. Adds tests/ha_stubs.py (dependency-free fake-HA harness) and tests/test_services.py (7 tests driving the real handlers). Full suite: 74 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…29) .gitignore was copied from an HA config-directory template and ignored `custom_components/` and `www/` — both SHIPPED deliverables. New platform modules and dashboards under them were silently invisible to git and would never land in a commit or release zip. - Rewrite .gitignore for a source repo: un-ignore custom_components/ and www/, keep cache/venv/OS/editor rules (add .ruff_cache, .mypy_cache, dist/build). - Track five www/ files that already ship via publish_addon.sh (which copies all of www/ into the add-on) but had never been committed: crop_steering_rules.html, home.html, office.html, floorplan/assets/*.js. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
async_step_load_env, async_step_reconfigure_env and async_step_load_yaml opened/stat'd files directly inside async flow steps, blocking the HA event loop and tripping HA's "blocking call to open inside the event loop" guard. Wrap os.path.exists + load_env_config + yaml read in hass.async_add_executor_job. No behavior change; env_parser stays synchronous (easier to unit-test). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gitignore) `.gitignore` ignored `custom_components/`, and black honours .gitignore even for explicitly-passed dirs, so `black --check custom_components/` was a no-op in CI — the shipped integration code was never actually format-checked. Un-ignoring it (#29) exposed 11 files that needed formatting. Pure `black` run, no logic change (AST-equivalent); confirmed by the full test suite still passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
) Controller (addons/f2_control): - #14 default room takes pump/mainline/valves from the integration's engine_config descriptor; the hardcoded F2 fallback is gone. An unmapped room holds safe (_blocked returns "no hardware mapped") instead of actuating F2's entities. - #20 shots are delivered in <=2s slices (_wait_shot) so the kill switch or a zone manual override stops water mid-shot; partial volume is counted proportionally. - #21 notify_service defaults empty (no fallback to the dev's phone); dosing/fill/ flush holds are now the configurable `hold_entities` option (empty default), not four hardcoded F2 ids; vitals branding uses `instance_name`. - #26 rooms are re-discovered every `rediscover_seconds` (default 300s): UI-added rooms and a late default-room descriptor join without a restart, fail-safe OFF. - #27 a setpoint entity missing from BOTH per-zone and global for >=3 loops raises a rate-limited alert + a vitals line, instead of silently running an engine default. - #28 tzdata added to the image; startup logs the effective TZ/offset and alerts if it disagrees with Home Assistant's configured zone. Add-on packaging: - #17 tracked placeholder addons/f2_control/www/public/index.html so `docker build` succeeds from a clean checkout (publish_addon.sh still overlays the full UI). - config.yaml: notify_service empty, new hold_entities/instance_name/rediscover_seconds options; version 0.10.5 -> 0.11.0. Integration: - #22 health.py resolves the kill switch + heartbeat per room (default room from the descriptor's enable_flag, named rooms from their own switch) and monitors ALL rooms instead of only the default — no more false "kill switch missing" on custom setups. Tests: new addons/f2_control/tests (11, real Controller) + tests/test_health.py (4); ha_stubs gains an issue_registry double. Full suite: 89 + 46 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…n real tests (#18 #19) - #18 ci-validate.yml gains: an engine vendored-copy sync check (the tested source package must be byte-identical to the copy baked into the add-on image), a repo- hygiene gate (no shipped path git-ignored, no tracked caches), an add-on controller pytest step, and a new `addon-build` job that `docker build`s the add-on from a clean checkout (catches Dockerfile/dependency breakage that used to ship unnoticed). - #19 install.yml's "Full Installation Test" no longer echoes success — it installs test deps and actually runs the integration + add-on + engine suites, so its green check and the uploaded report reflect real results. - run_ci.sh mirrors the new checks; requirements-test.txt adds `requests` (needed by the add-on controller suite). Local CI: ruff/black/yamllint + engine-sync + hygiene + 78 integration + 11 add-on + 46 engine tests all pass. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ator (#23 #24) - #23 build_lovelace.py no longer hardcodes [1,2,3]: the live-computed verdict and exception centre loop the zones actually present, so a 6- or 24-zone install shows every zone. The whole builder is now a pure `build_dashboard()` (unit-testable). - #24 the generator carries NO facility ids: the F2 RAW-probe map and RELATED list are now opt-in env inputs (empty by default), and the phantom input_select.growth_phase / nutrient_phase "selectors disagree" check is gone. Also removed that phantom block from the committed crop_steering_lovelace.yaml so current importers stop seeing the false alarm. - Per-room dashboards: CROP_STEERING_PREFIX=<slug>_ generates a dashboard scoped to a named room's prefixed entities (writes crop_steering_lovelace_<slug>.yaml). - The F2-specific static dashboards/ and packages/ are marked as facility examples (README in each) pointing to the portable generator + the integration's fused per-zone entities. - Fix a test-harness state leak (controller /data/state.json resolving to a real path on dev boxes); each build now uses an isolated temp state file. New: tests/test_build_lovelace.py (6 tests — zone scaling, no baggage, no phantom helpers, per-room prefixing). Full suite: 95 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ple marker (#30) - installation_guide.md: new "Step 10 — Dashboards" (portable generator + per-room, card-mod prereq for the F2 static files, links to DASHBOARDS.md and dashboards/README); notify_service noted as optional (persistent notifications still fire); hold_entities documented; DASHBOARDS.md added to "Learn more". - README.md: a Dashboards section (generator usage) + DASHBOARDS.md in the Docs list. - crop_steering.env: re-headed as a FACILITY EXAMPLE (do not copy verbatim) pointing to templates/ and crop_steering.env.example, so a fresh install doesn't inherit F2's sensor.substrate_* ids. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JakeTheRabbit
changed the title
install.yml's suite runner installed only `pytest requests`, so tests/test_build_lovelace.py (imports pyyaml) failed collection with ModuleNotFoundError. ci-validate passed only because yamllint pulls pyyaml transitively — make it explicit there too, and add pyyaml to requirements-test.txt. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…udit fixes Adversarial re-review of the #14–#31 fixes surfaced five issues introduced or left incomplete by them; all fixed and tested: 1. False-alarm regression (from the #27 fix): `min_floor_drown_ceiling` is an engine-only knob the integration deliberately never creates, so the new missing-setpoint detector alerted permanently on EVERY healthy install. `_zone_num` gains `optional=True` for engine-only knobs; a new test drives the real `_params()` path with every integration-created setpoint present and asserts zero false alarms. 2. Hollow fix (from the #22 fix): the engine_config descriptor always publishes the documented default kill switch for the default room, so a CUSTOM add-on `enable_flag` still produced a false "kill switch missing" repair. The engine now publishes its actual `enable_flag` on each room's heartbeat sensor and health.py prefers it (heartbeat → descriptor → per-room switch/default). 3. Wrong-room hazard (in the #16 fix): an unknown `room` slug silently fell back to the DEFAULT room — the exact wrong-room class the parameter exists to prevent. `_resolve_prefix` now raises HomeAssistantError; nothing is actuated on a typo. 4. Missed #16 criterion: `services.yaml` now documents the `room` field on all five room-aware services. 5. Release hygiene: integration 2.11.0 → 2.12.0 with a dual-view CHANGELOG entry; add-on CHANGELOG gains the 0.11.0 entry with upgrade-ordering notes (update the integration BEFORE rebuilding the add-on; F2 must set `hold_entities` or its dosing/fill holds go dark — v0.8.0 feed-sensor precedent). Stale docstrings in sensor.py/room.py ("the add-on ignores the default descriptor") corrected; DOCS.md quick-start reflects optional notify + hold_entities; the schema-less `hardware`/`zones` option reads are marked test-harness-only. Suite grows 95 → 98 (+ heartbeat enable_flag, engine-only-knob no-false-alarm, descriptor-fallback tests). Full local CI green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
addons/f2_control now carries a tests/ directory (added for the controller test harness); the publish script's whole-dir copy would have shipped it to the JakeTheRabbit/f2-control add-on repository on the next release. Strip tests/ and .pytest_cache alongside the existing bytecode cleanup. Assembly logic verified by simulating the copy into a scratch dir (tests absent, web root correct, recovered dashboards included). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
JakeTheRabbit
marked this pull request as ready for review
July 2, 2026 21:27
This was referenced Jul 2, 2026
Closed
Closed
This was referenced Jul 2, 2026
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ffec33440
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+486
to
+488
| for room in self._discover_rooms(): | ||
| if room.slug in known: | ||
| continue |
There was a problem hiding this comment.
When an operator edits an existing room's hardware or zone mapping while the add-on is running, the integration republishes the room's engine_config, but this branch skips every slug that is already known. The controller therefore keeps using the old room.hw/room.zones until an add-on restart, which can continue actuating a valve or pump that the UI mapping has just moved away from.
Useful? React with 👍 / 👎.
| "type": "history-graph", | ||
| "title": "VWC — all zones (24h)", | ||
| "hours_to_show": 24, | ||
| "entities": [sc("sensor", f"zone_{z}_vwc") for z in zones], |
There was a problem hiding this comment.
For prefixed rooms (CROP_STEERING_PREFIX=veg_), this builds sensor.crop_steering_veg_zone_1_vwc, but the integration's zone sensors are created from keys like vwc_zone_1/ec_zone_1 (CropSteeringSensor object ids), e.g. sensor.crop_steering_veg_vwc_zone_1. The generated named-room dashboard history graphs, gauges, and matching Jinja blocks will read unknown even though the real fused sensors exist.
Useful? React with 👍 / 👎.
| raw = {int(k): v for k, v in (raw or {}).items()} | ||
| P = prefix | ||
| csp = f"crop_steering_{P}" | ||
| cs = sorted(e for e in all_ids if f".{csp}" in e) |
There was a problem hiding this comment.
With no prefix, csp is crop_steering_, so this substring filter also matches every named-room entity such as sensor.crop_steering_veg_*. In a multi-room install the generated default dashboard then includes controls and sensors from other rooms despite the function contract that an empty prefix selects the default room only.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves the 18 issues from the senior audit (#14–#31). Makes the system Home-Assistant-agnostic (no F2 entity baggage on the default path), safer at the engine, and actually enforced by CI. 141 tests pass (84 integration + 11 add-on + 46 engine); ruff/black/yamllint/engine-sync/hygiene green; the add-on image builds and runs.
Highlights
Portability (the core mandate)
engine_configdescriptor — the hardcoded F2 fallback is gone; an unmapped room holds safe instead of actuating F2's entities.notify_servicedefault, configurablehold_entities(was 4 hardcoded F2 ids), generic branding.CROP_STEERING_PREFIX), and free of facility ids; phantomgrowth_phase/nutrient_phase"selectors disagree" false alarm removed.Engine safety
Correctness & compat
hass.helpers.template.now()(was crashing every event-firing service on current HA).iot_class→calculated.Repo / CI
.gitignorestopped hidingcustom_components/+www/— which had also hidden them from black in CI, so shipped code was never format-checked (one-time black pass inf746d20); recovered 5 dashboard files that ship but were never committed.crop_steering.envis marked a facility example.Verification
docker build+ container run confirm Add-on Docker build fails from this repo: Dockerfile COPYs www/public that only the publish script assembles #17 (COPY www/public) and Add-on photoperiod runs on an undeclared container timezone (no timezone flag, mixed naive/aware datetimes) #28 (tzdata →ZoneInforesolves).tests/,addons/f2_control/tests/).bash tests/run_ci.sh) is green.Notes
pytest-homeassistant-custom-componentinstance (fragile on Windows); a full config-flow flow test is the natural next harness addition.🤖 Generated with Claude Code