From 1e130bbe19151d0481ab9de69101618a625d2e02 Mon Sep 17 00:00:00 2001 From: Nikolay Miroshnichenko Date: Wed, 1 Jul 2026 19:37:26 +0200 Subject: [PATCH] feat: repair-suggestion engine + relative-dim + cover-invert (v0.5.0) From validator to repairer: propose concrete fixes, not just flag problems. - B1 repair-suggestion engine (repair.py) + suggest_repairs MCP tool: infer a DPT for a missing-DPT GA, correct a suspect sub-DPT, synthesise a status GA in a free slot, or add an absolute-brightness GA. Suggestions only; accepted new GAs feed generate_ets_group_addresses; never writes to ETS or the bus. - A2 relative-only-dimming detector (analyze.py): 3.007 relative dimmer with no 5.001 absolute-brightness GA in its zone -> HA cannot set a level. - A3 cover invert/travel-time surfacing (generate_ha.py): verify_cover_invert review note lists actuator-dependent flags absent from the .knxproj and warns when position lacks a state address. - A2/A3/B1 regression tests; CHANGELOG + version bump to 0.5.0. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 23 +++++++ nickol_knx_mcp/__init__.py | 2 +- nickol_knx_mcp/analyze.py | 23 ++++++- nickol_knx_mcp/generate_ha.py | 12 +++- nickol_knx_mcp/repair.py | 122 ++++++++++++++++++++++++++++++++++ nickol_knx_mcp/server.py | 14 ++++ pyproject.toml | 2 +- tests/test_pipeline.py | 37 +++++++++++ 8 files changed, 230 insertions(+), 5 deletions(-) create mode 100644 nickol_knx_mcp/repair.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ec0a67..5af75a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.5.0] — 2026-07-01 + +**From validator to repairer.** Previous releases *flagged* problems in a `.knxproj`; this one +starts *proposing the fix*. The headline is a repair-suggestion engine that turns each finding +into a concrete, reviewable change, alongside two new detectors for gaps that silently break the +Home Assistant layer: relative-only dimmers and actuator-dependent cover behaviour. + +### Added +- **B1 — repair-suggestion engine** (`repair.py`, new module; new MCP tool `suggest_repairs`). + For each finding it proposes a concrete fix rather than only naming the problem: infer a DPT for + a group address that has none, correct a suspect sub-DPT, synthesise a status/feedback GA in a + free address slot, or add an absolute-brightness GA for a relative-only dimmer. Suggestions only + — a human reviews them, and accepted new GAs feed `generate_ets_group_addresses`; the server + never writes to ETS or the bus. On a real 3646-GA project it produced **145 proposals**: 32 + set-DPT, 1 change-DPT, and 112 synthesised status GAs. +- **A2 — relative-only-dimming detector** (`analyze.py`, `relative_only_dimming` finding). A + `3.007` relative dimmer with no `5.001` absolute-brightness GA in its zone is flagged, because + Home Assistant cannot set a brightness level from relative dimming alone. +- **A3 — cover invert / travel-time surfacing** (`generate_ha.py`). The cover review reason is now + `verify_cover_invert` and carries a note listing the actuator-dependent flags that are *not* in + the `.knxproj` (`invert_position` / `invert_updown` / `invert_angle`, `travelling_time_up` / + `travelling_time_down`), plus a warning when a cover's position lacks a state address. + ## [0.4.0] — 2026-07-01 **Two new lint dimensions: does the DPT sub-type match what the name promises, and is the diff --git a/nickol_knx_mcp/__init__.py b/nickol_knx_mcp/__init__.py index 98c485f..b8cfa3c 100644 --- a/nickol_knx_mcp/__init__.py +++ b/nickol_knx_mcp/__init__.py @@ -1,2 +1,2 @@ """nickol-knx-mcp: design-time KNX/ETS project assistant MCP server.""" -__version__ = "0.4.0" +__version__ = "0.5.0" diff --git a/nickol_knx_mcp/analyze.py b/nickol_knx_mcp/analyze.py index 6615f05..b51be8e 100644 --- a/nickol_knx_mcp/analyze.py +++ b/nickol_knx_mcp/analyze.py @@ -15,7 +15,7 @@ from collections import defaultdict from typing import Any, Optional from .project import LoadedProject, GARecord, STATUS_KEYWORDS -from .pairing import find_status, function_status_pairs +from .pairing import find_status, function_status_pairs, base_tokens from .intent import INTENT_FUNCTIONAL, INTENT_RESERVE, INTENT_SCRATCH SEVERITY_ERROR = "error" @@ -361,6 +361,27 @@ def detect_dpt_issues(project: LoadedProject) -> list[dict[str, Any]]: name=ga.name, found=ga.dpt, expected=f"{em}.{es:03d}", )) + # 5. relative-only dimming (A2) — a 3.007 relative dimmer with no 5.001 + # absolute-brightness GA in the same zone: Home Assistant cannot set a level. + abs5 = [g for g in project.gas.values() if g.dpt_main == 5] + for addr, ga in project.gas.items(): + if ga.intent != INTENT_FUNCTIONAL or ga.dpt_main != 3: + continue + toks = base_tokens(ga.name) + if not toks: + continue + need = min(2, len(toks)) + has_abs = any(g.main == ga.main and len(toks & base_tokens(g.name)) >= need + for g in abs5) + if not has_abs: + findings.append(_finding( + SEVERITY_WARN, "relative_only_dimming", addr, + f"'{ga.name}' has relative dimming (3.007) but no absolute-brightness " + "(5.001) GA in its zone — Home Assistant cannot set a brightness level; " + "add an absolute-brightness group address.", + name=ga.name, + )) + return findings diff --git a/nickol_knx_mcp/generate_ha.py b/nickol_knx_mcp/generate_ha.py index e3f833a..08f15f6 100644 --- a/nickol_knx_mcp/generate_ha.py +++ b/nickol_knx_mcp/generate_ha.py @@ -242,8 +242,16 @@ def generate_ha_yaml(project: LoadedProject) -> dict[str, Any]: consumed.add(sib.address) covers.append(entity) consumed.add(ga.address) - review.append({"reason": "verify_cover_mapping", "address": ga.address, - "name": ga.name}) + # A3: surface the actuator-dependent flags that are NOT in the .knxproj — + # the three invert flags (position / up-down / angle) and travel times — + # so the installer sets them; and whether position lacks its state address. + note = ("set actuator-dependent flags not in the .knxproj: " + "invert_position / invert_updown / invert_angle, and " + "travelling_time_up / travelling_time_down") + if "position_address" in entity and "position_state_address" not in entity: + note += " — position has NO position_state_address (HA will guess position)" + review.append({"reason": "verify_cover_invert", "address": ga.address, + "name": ga.name, "note": note}) # ---- 2. LIGHTS (brightness 5.001 command, lighting category) ---- for ga in project.gas.values(): diff --git a/nickol_knx_mcp/repair.py b/nickol_knx_mcp/repair.py new file mode 100644 index 0000000..f44fecf --- /dev/null +++ b/nickol_knx_mcp/repair.py @@ -0,0 +1,122 @@ +"""Repair-suggestion engine (B1) — propose fixes, don't just flag problems. + +Every converter refuses on an imperfect `.knxproj`. This module does the opposite: +for each finding it proposes a concrete, reviewable fix — infer a DPT from the name, +synthesise a status group address in a free slot, add an absolute-brightness GA — so +an imperfect project can be *repaired* toward import-ready. Suggestions only: a human +reviews them, and accepted new GAs feed ``generate_ets_group_addresses``. The server +never writes to a bus or to ETS. +""" + +from __future__ import annotations + +from typing import Any, Optional + +from .project import LoadedProject +from .analyze import detect_missing_status, detect_dpt_issues, _expected_subdpt +from .intent import INTENT_FUNCTIONAL + + +# command DPT main -> the status/feedback DPT to synthesise for it +_STATUS_DPT = {1: "1.011", 3: "5.001", 5: "5.001", 9: "9.001", + 13: "13.013", 14: "14.056", 20: "20.102"} + + +def _infer_dpt(ga: Any) -> str: + """Best-effort DPT for a GA that has none, from its name/category/kind.""" + exp = _expected_subdpt(ga.name) + if exp: + return f"{exp[0]}.{exp[1]:03d}" + low = (ga.name or "").lower() + if ga.kind == "status" or any(k in low for k in ("статус", "status", "rück", "rueck")): + return "1.011" + if any(k in low for k in ("вверх/вниз", "up/down", "auf/ab", "up-down")): + return "1.008" + if "стоп" in low or "stop" in low: + return "1.010" + if "позиц" in low or "position" in low or "stellung" in low: + return "5.001" + if any(k in low for k in ("диммир", "dimming", "яркост", "brightness")): + return "5.001" + if "сцен" in low or "scene" in low or "szene" in low: + return "18.001" + if ga.category == "shutter": + return "1.008" + # switch-like boolean is the safest default + return "1.001" + + +def _next_free(used: set[str], main: int, prefer_middle: Optional[int] = None) -> Optional[str]: + """Suggest a free 3-level address in ``main`` (prefer a given middle first).""" + order = ([prefer_middle] if prefer_middle is not None else []) + list(range(8)) + seen = set() + for mid in order: + if mid in seen: + continue + seen.add(mid) + for sub in range(1, 256): + a = f"{main}/{mid}/{sub}" + if a not in used: + used.add(a) + return a + return None + + +def suggest_repairs(project: LoadedProject) -> dict[str, Any]: + """Propose concrete fixes for the project's findings. Suggestions only.""" + used = set(project.gas.keys()) + proposals: list[dict[str, Any]] = [] + + for f in detect_dpt_issues(project): + code = f["code"] + addr = f["address"] + if code == "missing_dpt" and addr in project.gas: + ga = project.gas[addr] + proposals.append({ + "code": code, "action": "set_dpt", "address": addr, "name": ga.name, + "dpt": _infer_dpt(ga), + "rationale": "inferred from the name/category so HA can decode it", + }) + elif code == "subdpt_suspect": + proposals.append({ + "code": code, "action": "change_dpt", "address": addr, + "name": f.get("name"), "dpt": f.get("expected"), + "rationale": "expected sub-type for this named function", + }) + elif code == "relative_only_dimming" and addr in project.gas: + ga = project.gas[addr] + new = _next_free(used, ga.main if ga.main is not None else 1) + proposals.append({ + "code": code, "action": "add_ga", "address": new, "for": addr, + "name": f"{ga.name} - Значение яркости", "dpt": "5.001", + "rationale": "absolute-brightness GA so Home Assistant can set a level", + }) + + for f in detect_missing_status(project): + if f["code"] != "missing_status_address": + continue + addr = f["address"] + ga = project.gas.get(addr) + if ga is None: + continue + sdpt = _STATUS_DPT.get(ga.dpt_main, "1.011") + new = _next_free(used, ga.main if ga.main is not None else 1, prefer_middle=4) + proposals.append({ + "code": "missing_status", "action": "add_ga", "address": new, "for": addr, + "name": f"{ga.name} (статус)", "dpt": sdpt, + "rationale": "status/feedback GA so Home Assistant reads real state", + }) + + by_action: dict[str, int] = {} + for p in proposals: + by_action[p["action"]] = by_action.get(p["action"], 0) + 1 + + return { + "count": len(proposals), + "by_action": by_action, + "proposals": proposals, + "note": "Suggestions only — review before applying. `set_dpt`/`change_dpt` edit an " + "existing GA in ETS; `add_ga` addresses are suggested FREE slots (adjust to " + "your convention), then feed the accepted new GAs to " + "`generate_ets_group_addresses`. This server never writes to ETS or the bus.", + } diff --git a/nickol_knx_mcp/server.py b/nickol_knx_mcp/server.py index b08aa14..c0a939a 100644 --- a/nickol_knx_mcp/server.py +++ b/nickol_knx_mcp/server.py @@ -23,6 +23,7 @@ from .generate_ets import generate_ets_csv, generate_ets_xml from .report import build_report from .handover import build_handover from .device_library import decompose_device as _decompose_device, list_recipes +from .repair import suggest_repairs as _suggest_repairs mcp = FastMCP("nickol-knx") @@ -164,6 +165,19 @@ def check_dpt() -> list[dict[str, Any]]: return detect_dpt_issues(_project()) +@mcp.tool() +def suggest_repairs() -> dict[str, Any]: + """Propose concrete fixes for the project's findings — repair, don't just flag. + + For each issue it suggests a reviewable fix: infer a DPT for a GA that has none, + correct a suspect sub-DPT, synthesise a status/feedback GA in a free address slot, + or add an absolute-brightness GA for a relative-only dimmer. Suggestions only — + a human reviews them; accepted new GAs feed generate_ets_group_addresses. The + server never writes to ETS or the bus. + """ + return _suggest_repairs(_project()) + + @mcp.tool() def check_secure() -> dict[str, Any]: """Summarise KNX Data Secure posture + the keyring handover checklist. diff --git a/pyproject.toml b/pyproject.toml index 13183cc..f0883a3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "nickol-knx-mcp" -version = "0.4.0" +version = "0.5.0" description = "Design-time KNX/ETS6 project assistant as an MCP server (parse .knxproj, validate, generate HA YAML + ETS CSV/XML). No live bus access." readme = "README.md" requires-python = ">=3.10" diff --git a/tests/test_pipeline.py b/tests/test_pipeline.py index 756c102..872f4d6 100644 --- a/tests/test_pipeline.py +++ b/tests/test_pipeline.py @@ -518,3 +518,40 @@ assert _sp["keyring_required"] is True assert len(_sp["mixed_middle_groups"]) >= 1, "mixed secure/plaintext middle not flagged" assert any("keyring" in s.lower() for s in _sp["checklist"]), "no keyring step" print("OK: secure posture — counts, mixed-middle flag, keyring checklist") + +# --------------------------------------------------------------------------- # +# Regression (v0.5.0): A2 relative-only dimming · A3 cover invert · B1 repairs. +# --------------------------------------------------------------------------- # +print("\n=== REGRESSION: A2 relative-dim + A3 cover invert + B1 repair engine ===") +from nickol_knx_mcp.repair import suggest_repairs +# A2 +_a2raw = {"group_addresses": { + "1/0/0": ga("1/0/0", "Кухня свет - Относительное диммирование", 3, 7), # no abs -> flag + "2/0/0": ga("2/0/0", "Зал свет - Относительное диммирование", 3, 7), + "2/0/1": ga("2/0/1", "Зал свет - Значение яркости", 5, 1), # abs present -> ok +}} +_a2 = {(f["code"], f["address"]) for f in detect_dpt_issues(build_loaded_from_raw(_a2raw, "mem"))} +assert ("relative_only_dimming", "1/0/0") in _a2, "relative-only dimmer not flagged" +assert ("relative_only_dimming", "2/0/0") not in _a2, "false positive: zone has abs brightness" +print("OK: A2 — relative-only dimmer flagged; dimmer with absolute brightness clean") +# A3 +_a3raw = {"group_addresses": { + "2/0/0": ga("2/0/0", "Спальня штора - Вверх/вниз", 1, 8), + "2/1/0": ga("2/1/0", "Спальня штора - Позиция", 5, 1), +}} +_a3 = generate_ha_yaml(build_loaded_from_raw(_a3raw, "mem"))["review"] +assert any(r["reason"] == "verify_cover_invert" and "invert_position" in r.get("note", "") for r in _a3), \ + "cover invert/travel review note missing" +print("OK: A3 — cover surfaces invert/travel flags for review") +# B1 +_b1raw = {"group_addresses": { + "3/0/0": ga("3/0/0", "Спальня свет - Вкл/выкл", None, None), # missing DPT -> set_dpt + "4/0/0": ga("4/0/0", "Холл выключатель", 1, 1), # no status -> add_ga +}} +_r = suggest_repairs(build_loaded_from_raw(_b1raw, "mem")) +_acts = {p["action"] for p in _r["proposals"]} +assert "set_dpt" in _acts, "no set_dpt proposal for missing_dpt" +assert any(p["action"] == "add_ga" and "статус" in p["name"] for p in _r["proposals"]), "no status add_ga" +_sd = [p for p in _r["proposals"] if p["action"] == "set_dpt"][0] +assert _sd["dpt"] == "1.001", _sd +print("OK: B1 — repair engine proposes set_dpt + synthesised status GA")