diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cfe1c9..9cb3552 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Role-aware feedback completeness** (`detect_role_completeness` in `analyze.py`, surfaced through + `check_missing_status`). "Does the function have *a* status?" was not enough — a dimmer with an on/off + status but no brightness status silently passed and inflated coverage/Matter scores. The new check + flags a brightness/position **command** (DPT 5.001) whose device has no matching value **status** + (`missing_value_status`), using a device-identity match so it does not borrow a sibling's status. + Closes the demo's own documented limitation — recall on the demo house is now 5/5. Raised by an + external expert review. `tests/test_role_completeness.py`. + - **Project Policy Profile** (`policy.py`, new MCP tool `check_policy`). Validate a project against *its own* agreed rules (main-group taxonomy, naming regex, command/status exemptions) instead of one universal "professional standard" — a direct answer to integrator feedback that diff --git a/examples/demo-home/README.md b/examples/demo-home/README.md index 31360e9..654a96b 100644 --- a/examples/demo-home/README.md +++ b/examples/demo-home/README.md @@ -30,14 +30,15 @@ flagged in `generated/project_report.md`: |---|---|---| | 1 | A group address with **no DPT** (`Living room CO2`, 4/2/1) | ✅ `check_dpt` 🔴 | | 2 | Two GAs with the **same name, different DPT** (`Kitchen temperature`) | ✅ `check_dpt` | -| 3 | A dimmer with an on/off status but **no brightness status** (`Kitchen worktop LED`) | ⚠️ not flagged* | +| 3 | A dimmer with an on/off status but **no brightness status** (`Kitchen worktop LED`) | ✅ `check_missing_status` (`missing_value_status`)* | | 4 | A switch with **no status** at all (`Guest WC ceiling`) | ✅ `check_missing_status` | | 5 | A GA with an **empty name** (`2/5/2`) | ✅ `check_naming` 🔴 | -\* **Honest limitation surfaced by this very demo:** the missing-status check currently asks -"does this control have *a* status?", not "does it have *each expected* status type?". The Kitchen -dimmer has an on/off status, so it isn't flagged for its missing brightness status. Tracked for a -future release. +\* **Now fixed (role-aware feedback completeness).** This demo originally surfaced an honest +limitation — the missing-status check asked "does this control have *a* status?", so the Kitchen +dimmer's on/off status masked its missing brightness status. `detect_role_completeness` now flags a +brightness/position **command** (5.001) that has no matching value **status** (`missing_value_status`), +without borrowing a sibling device's status. Recall on this demo is now 5/5. Inventory: 239 GAs · 47 Functions · 0 errors that block parsing. The HA generator produced **19 switches, 13 lights (incl. RGBW / RGB / CCT colour), 6 covers, 6 climate zones, 13 binary diff --git a/nickol_knx_mcp/analyze.py b/nickol_knx_mcp/analyze.py index 5bfae44..125367e 100644 --- a/nickol_knx_mcp/analyze.py +++ b/nickol_knx_mcp/analyze.py @@ -296,6 +296,61 @@ def detect_missing_status(project: LoadedProject) -> list[dict[str, Any]]: f"identical name) and {n_self_report} self-reporting (actuator R+T object " "on the command GA) — no separate status GA needed.", )) + findings.extend(detect_role_completeness(project)) + return findings + + +# --------------------------------------------------------------------------- # +# Role-aware feedback completeness (external-review gap): "does the function have +# *a* status" is not enough — a dimmer with an on/off status but NO brightness +# status silently passes and inflates coverage. Flag a value/level COMMAND +# (brightness/position, 5.001) that has no matching value STATUS. +# --------------------------------------------------------------------------- # +# Function words stripped to get the DEVICE identity (so "Kitchen worktop LED +# brightness" doesn't borrow "Kitchen island pendants brightness status" just +# because both share "kitchen"+"brightness"). +_VALUE_WORDS = ( + "brightness", "value", "level", "position", "dim", "dimming", "scaling", + "яркост", "значени", "уровень", "позици", "диммир", "положени", "стеллунг", +) + + +def _device_ident(name: str) -> set[str]: + return {t for t in base_tokens(name) if not any(w in t for w in _VALUE_WORDS)} + + +def detect_role_completeness(project: LoadedProject) -> list[dict[str, Any]]: + """Flag a brightness/position COMMAND (5.001) with no matching value STATUS. + + Complements detect_missing_status (which only asks for *a* status): a dimmer + can have an on/off status yet no brightness-level feedback, which Home Assistant + needs to show the real level after a manual/external change. + """ + findings: list[dict[str, Any]] = [] + val_status = [g for g in project.gas.values() + if g.intent == INTENT_FUNCTIONAL and g.dpt_main == 5 and g.kind == "status"] + for addr, ga in project.gas.items(): + if ga.intent != INTENT_FUNCTIONAL or ga.kind != "command": + continue + if ga.dpt_main != 5 or ga.dpt_sub != 1: + continue + if ga.category not in ("lighting", "shutter"): + continue + ident = _device_ident(ga.name) + if not ident: + continue + need = min(2, len(ident)) + has_status = any(s.main == ga.main and len(ident & _device_ident(s.name)) >= need + for s in val_status) + if not has_status: + what = "brightness" if ga.category == "lighting" else "position" + findings.append(_finding( + SEVERITY_WARN, "missing_value_status", addr, + f"'{ga.name}' is a {what} command (DPT 5.001) but its device has no " + f"{what} status GA — Home Assistant cannot read the actual {what} " + "after a manual or external change (an on/off status is not enough).", + name=ga.name, category=ga.category, + )) return findings diff --git a/tests/test_role_completeness.py b/tests/test_role_completeness.py new file mode 100644 index 0000000..f05148a --- /dev/null +++ b/tests/test_role_completeness.py @@ -0,0 +1,55 @@ +"""Role-aware feedback completeness (external-review gap #3): a brightness/position +COMMAND (5.001) with no matching value STATUS must be flagged, even when the device +already has an on/off status. And it must NOT borrow a sibling device's status. +""" +from nickol_knx_mcp.project import build_loaded_from_raw +from nickol_knx_mcp.analyze import detect_role_completeness, detect_missing_status + + +def _ga(addr, name, dmain, dsub): + return {"name": name, "identifier": f"GA-{addr}", "raw_address": 0, "address": addr, + "project_uid": None, "dpt": {"main": dmain, "sub": dsub}, "data_secure": False, + "communication_object_ids": [], "description": "", "comment": ""} + + +def _proj(gas): + raw = {"info": {"group_address_style": "ThreeLevel", "schema_version": "21"}, + "group_addresses": gas, "communication_objects": {}, "devices": {}, + "functions": {}, "topology": {}, "group_ranges": {}} + return build_loaded_from_raw(raw, "t.knxproj") + + +def main(): + gas = { + # incomplete dimmer: on/off + on/off status + brightness cmd, but NO brightness status + "1/0/11": _ga("1/0/11", "Kitchen worktop switch", 1, 1), + "1/2/11": _ga("1/2/11", "Kitchen worktop brightness", 5, 1), + "1/4/11": _ga("1/4/11", "Kitchen worktop status", 1, 11), + # complete dimmer: brightness cmd + brightness status + "1/2/12": _ga("1/2/12", "Living pendant brightness", 5, 1), + "1/5/12": _ga("1/5/12", "Living pendant brightness status", 5, 1), + # a shutter position command without position status + "2/2/1": _ga("2/2/1", "Bedroom blind position", 5, 1), + } + p = _proj(gas) + rc = {f["address"] for f in detect_role_completeness(p)} + + assert "1/2/11" in rc, "incomplete dimmer brightness must be flagged" + assert "1/2/12" not in rc, "complete dimmer must NOT be flagged" + assert "2/2/1" in rc, "shutter position command without status must be flagged" + + # must not borrow the sibling's status: worktop is flagged even though 'Living + # pendant brightness status' exists (different device identity) + f = next(f for f in detect_role_completeness(p) if f["address"] == "1/2/11") + assert "brightness" in f["message"] and f["code"] == "missing_value_status" + + # and it surfaces through detect_missing_status (the aggregate the tools use) + assert any(x["code"] == "missing_value_status" and x["address"] == "1/2/11" + for x in detect_missing_status(p)) + + print("test_role_completeness: OK — brightness/position command without a value " + "status is flagged (no sibling-status borrowing); complete dimmer stays clean.") + + +if __name__ == "__main__": + main()