From e96ab5e9e42a06c6cffc6ff82970449155be3599 Mon Sep 17 00:00:00 2001 From: Oleksii Dolhov Date: Mon, 5 Oct 2026 15:22:18 +0300 Subject: [PATCH] fix(compat): D-003 exempts metric-bound widgets from the fields the binding fills (#3186) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A metric/status/progress widget bound to a declared metric gets its value (and a status widget its color) from the recorded points on every read (ent#479), and the agent server's validate_widget already exempts them. D-003 still demanded value/color, so an agent following the documented binding contract was reported HARD-incompatible. D-003 now applies the agent server's predicate (a truthy `metric`) and skips exactly the filled fields (_BOUND_FILLED); `label` stays required and unbound widgets keep today's rule. Decided: a binding to an undeclared name is exempt too — the backend renders "not declared" and drops any written value, so requiring one demands a number nobody sees. D-009 checks the metrics block's shape only; no check flags undeclared bindings yet. Co-Authored-By: Claude Opus 5.5 --- .../agent_server/routers/dashboard.py | 4 +- docs/agent-validation-spec.md | 1 + .../services/compatibility/static_checks.py | 19 ++++++++- tests/unit/test_compatibility_checks.py | 40 +++++++++++++++++++ 4 files changed, 62 insertions(+), 2 deletions(-) diff --git a/docker/base-image/agent_server/routers/dashboard.py b/docker/base-image/agent_server/routers/dashboard.py index 526c32ef9..e13f1c143 100644 --- a/docker/base-image/agent_server/routers/dashboard.py +++ b/docker/base-image/agent_server/routers/dashboard.py @@ -45,7 +45,9 @@ def validate_widget(widget: Dict[str, Any], index: int) -> Optional[str]: # An agent on an older base image still has the strict rule, so the guide # tells authors to keep a placeholder `value:` until their image carries # this change — the backend overwrites it when the binding resolves, so - # the placeholder is never what an operator sees. + # the placeholder is never what an operator sees. Compatibility check D-003 + # (`services/compatibility/static_checks.py::_BOUND_FILLED`) applies the same + # exemption — keep the two in step (#3186). bound = bool(widget.get('metric')) # Type-specific validation diff --git a/docs/agent-validation-spec.md b/docs/agent-validation-spec.md index 5c92330be..2d8a7654d 100644 --- a/docs/agent-validation-spec.md +++ b/docs/agent-validation-spec.md @@ -460,6 +460,7 @@ Common mistakes that break rendering: - `metric` widget: must have `label` and `value` - `status` widget: must have `label`, `value`, and `color` - `progress` widget: must have `label` and `value` +- A widget bound to a declared metric (`metric: `) gets its `value` — and a `status` widget its `color` — from the recorded points, so a bound `metric`, `status` or `progress` widget needs only its `label` (#3186). The exemption holds for any non-empty binding: a binding to an undeclared name renders "not declared" and drops a written `value` anyway. **D-004** — Progress values in range Severity: SOFT | Type: STATIC diff --git a/src/backend/services/compatibility/static_checks.py b/src/backend/services/compatibility/static_checks.py index b604213a2..c5cad11bc 100644 --- a/src/backend/services/compatibility/static_checks.py +++ b/src/backend/services/compatibility/static_checks.py @@ -1164,7 +1164,23 @@ def f(widgets): return _with_dashboard(snap, f) +# #3186: fields a widget BOUND to a metric (`metric: `) does not need — +# the backend fills them from the recorded points on every read (ent#479). The +# agent server's `validate_widget` exempts exactly these, keyed on the same +# truthy `metric` predicate; the two rules must agree, or an agent the +# dashboard renders is reported HARD-incompatible. +_BOUND_FILLED = {"metric": {"value"}, "status": {"value", "color"}, "progress": {"value"}} + + def c_d003(snap): + """HARD: each widget carries the fields it renders from. + + A widget bound to a metric is exempt from the fields the binding fills + (`_BOUND_FILLED`) — whether or not the name is declared: an undeclared + binding renders "not declared" and the backend DROPS any written `value`, + so demanding one would demand a number nobody sees. `label` stays required: + it is the widget's own copy, not a measurement. + """ req = { "text": ["content"], "markdown": ["content"], "list": ["items"], "link": ["url"], "metric": ["label", "value"], @@ -1176,8 +1192,9 @@ def f(widgets): t = w.get("type") if not isinstance(t, str): # a non-string type is D-002's finding; `req.get({..})` raised at HEAD continue + filled = _BOUND_FILLED.get(t, set()) if w.get("metric") else set() for field in req.get(t, []): - if field not in w: + if field not in w and field not in filled: bad.append({"type": t, "missing": field}) if bad: pairs = sorted({(b["type"], b["missing"]) for b in bad}) diff --git a/tests/unit/test_compatibility_checks.py b/tests/unit/test_compatibility_checks.py index cf89b722f..973e53805 100644 --- a/tests/unit/test_compatibility_checks.py +++ b/tests/unit/test_compatibility_checks.py @@ -414,6 +414,46 @@ def test_d003_non_string_type_is_not_a_hard_check_error(self): assert status == "pass" assert "check_error" not in (detail or {}) + # -- #3186: a widget bound to a metric gets its value (and a status widget its + # colour) from the recorded points, so D-003 must not demand them -------- + + @pytest.mark.parametrize("widget", [ + " - type: metric\n label: Revenue\n metric: revenue\n", + " - type: status\n label: Health\n metric: health\n", + " - type: progress\n label: Quota\n metric: quota\n", + ]) + def test_d003_a_bound_widget_needs_no_value_or_color(self, widget): + snap = self._dash(good_snapshot(), "widgets:\n" + widget) + assert _run_one("D-003", snap)[0] == "pass" + + @pytest.mark.parametrize("widget,missing", [ + (" - type: metric\n label: Revenue\n", "'metric' needs value"), + (" - type: status\n label: Health\n value: ok\n", "'status' needs color"), + (" - type: status\n label: Health\n color: green\n", "'status' needs value"), + (" - type: progress\n label: Quota\n", "'progress' needs value"), + ]) + def test_d003_an_unbound_widget_still_needs_them(self, widget, missing): + status, msg, _detail = _run_one("D-003", self._dash(good_snapshot(), "widgets:\n" + widget)) + assert status == "fail" and missing in msg + + @pytest.mark.parametrize("binding", ["metric: ''", "metric: null", "metric: 0"]) + def test_d003_an_empty_binding_is_not_a_binding(self, binding): + snap = self._dash(good_snapshot(), f"widgets:\n - type: metric\n label: R\n {binding}\n") + assert _run_one("D-003", snap)[0] == "fail" + + def test_d003_a_bound_widget_still_needs_its_label(self): + snap = self._dash(good_snapshot(), "widgets:\n - type: metric\n metric: revenue\n") + status, msg, _detail = _run_one("D-003", snap) + assert status == "fail" and "'metric' needs label" in msg + + def test_d003_a_binding_to_an_undeclared_name_is_not_a_required_field_failure(self): + """Decided in #3186: the renderer drops `value` for an undeclared + binding and shows "not declared" instead, so demanding a number here + would demand one nobody sees. The undeclared name is the finding, and + it is not D-003's.""" + snap = self._dash(good_snapshot(), "widgets:\n - type: metric\n label: R\n metric: nope\n") + assert _run_one("D-003", snap)[0] == "pass" + def test_d004_detail_is_json_safe_for_date_labels(self): # the hardened loader hands back a `datetime.date` for `label: 2026-01-01`; # at HEAD that reached `detail` raw and `json.dumps(checks)` in upsert_result raised