mirror of
https://github.com/NickoScope/nickol-knx-mcp.git
synced 2026-09-29 19:31:12 +02:00
feat(P2): role-aware feedback completeness — catch a value command with no value status
'Does the function have *a* status' missed a dimmer with on/off status but no brightness status (demo planted error #3), silently inflating coverage/Matter. detect_role_completeness flags a brightness/position command (5.001) whose device has no matching value status (missing_value_status), with a device-identity match so it doesn't borrow a sibling's status. Demo recall 4/5 -> 5/5. Raised by the external expert review. Test + demo ground-truth updated. Full suite green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
c53f388dcb
commit
5036a19b4b
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
|
||||
|
||||
@@ -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()
|
||||
Reference in New Issue
Block a user