observatory: forward has_label_formatter + layout to JS extension payload

The fx_viewer JS runtime gates canvas label rendering and the "L" badge
in the layer panel on `extData.has_label_formatter` (introduced in
a66f82a). The Observatory pipeline was dropping that flag in two
places, so per-layer-accuracy labels never rendered and the badge never
appeared, even though `Extension.build_payload()` set the flag correctly.

* **`GraphHub.add_analysis_layers`** selectively copied `name` / `legend`
  / `sync_keys` / `nodes` into the per-layer dict; `has_label_formatter`
  was never in the allowlist and silently became `undefined` on the JS
  side. Replaced the manual dict construction with `asdict(payload)` so
  every current and future `GraphExtensionPayload` field forwards
  automatically; `id` is then overwritten with the namespaced form
  because the dict key is authoritative on the JS side.

* **`GraphLayerContribution.to_payload`** rebuilt a
  `GraphExtensionPayload` when `id_override` / `name_override` was set
  but only forwarded `id` / `name` / `legend` / `nodes`. `sync_keys` and
  `has_label_formatter` reset to dataclass defaults, so any lens that
  renamed its layer lost both. Both fields are now forwarded explicitly.

* **`GraphHub.build_viewer_payload`** did not emit `payload.layout`. JS
  fell back to hardcoded constants in `graph_data_store.js`; today they
  match Python defaults, but the first change to `_NODE_LINE_HEIGHT` or
  `_MAX_LABEL_EXTENSIONS` would silently break label-slot reservation
  vs canvas drawing. Now sourced from
  `FXGraphExporter._layout_constants_payload()`.

Tests in `test_graph_hub.py`: 9 new cases lock the contract field-by-
field. Two of them iterate `dataclasses.fields(GraphExtensionPayload)`
and `GraphExtensionNodePayload` so any new field added without
forwarding through `GraphHub` fails CI immediately.

Verified: 11/11 graph_hub tests pass; 102/102 observatory tests pass
(3 unrelated pre-existing failures require Python ≥ 3.11 for
fast-sugiyama and are independent of this change).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: quic-boyuc

devtools/observatory/graph_hub.py

@@ -49,15 +49,14 @@ def add_analysis_layers(
             payload = contribution.to_payload()
             namespaced_id = f"{lens_name}/{layer_key}"
 
-            slot[namespaced_id] = {
-                "name": payload.name,
-                "legend": payload.legend,
-                "sync_keys": payload.sync_keys,
-                "nodes": {
-                    node_id: asdict(node_payload)
-                    for node_id, node_payload in payload.nodes.items()
-                },
-            }
+            # asdict() forwards every GraphExtensionPayload field (and recurses
+            # into per-node GraphExtensionNodePayload) so the JS runtime sees
+            # the full schema — including `has_label_formatter`, which gates
+            # canvas label rendering and the layer panel's "L" badge.  The
+            # original `id` is replaced by `namespaced_id` because the dict
+            # key is authoritative on the JS side.
+            slot[namespaced_id] = asdict(payload)
+            slot[namespaced_id]["id"] = namespaced_id
 
     def get_asset(self, graph_ref: str) -> Dict[str, Any]:
         return self._graph_assets.get(graph_ref, {})
@@ -70,10 +69,22 @@ def build_payload(self) -> Dict[str, Any]:
 
     @staticmethod
     def build_viewer_payload(graph_assets: Dict[str, Any], graph_layers: Dict[str, Any], graph_ref: str) -> Dict[str, Any]:
+        # Layout constants live alongside the JS hardcoded fallback in
+        # graph_data_store.js; sourcing them here from FXGraphExporter keeps
+        # Python (build-time bbox reservation) and JS (canvas rendering) on
+        # the same dimensions so labels never overflow their reserved slot.
+        from executorch.devtools.fx_viewer.exporter import FXGraphExporter
+
+        layout = FXGraphExporter._layout_constants_payload()
         asset = graph_assets.get(graph_ref, {})
         if not asset:
-            return {"base": {"legend": [], "nodes": [], "edges": []}, "extensions": {}}
+            return {
+                "base": {"legend": [], "nodes": [], "edges": []},
+                "extensions": {},
+                "layout": layout,
+            }
         return {
             "base": asset.get("base", {"legend": [], "nodes": [], "edges": []}),
             "extensions": graph_layers.get(graph_ref, {}),
+            "layout": layout,
         }

devtools/observatory/interfaces.py

@@ -436,6 +436,8 @@ def to_payload(self) -> "GraphExtensionPayload":
                 name=self.name_override or payload.name,
                 legend=payload.legend,
                 nodes=payload.nodes,
+                sync_keys=payload.sync_keys,
+                has_label_formatter=payload.has_label_formatter,
             )
 
         return payload

devtools/observatory/tests/test_graph_hub.py

@@ -4,9 +4,15 @@
 # This source code is licensed under the BSD-style license found in the
 # LICENSE file in the root directory of this source tree.
 
+from dataclasses import fields
+
 from executorch.devtools.observatory.graph_hub import GraphHub
-from executorch.devtools.observatory.interfaces import RecordAnalysis
+from executorch.devtools.observatory.interfaces import (
+    GraphLayerContribution,
+    RecordAnalysis,
+)
 from executorch.devtools.fx_viewer import (
+    GraphExtension,
     GraphExtensionNodePayload,
     GraphExtensionPayload,
 )
@@ -54,3 +60,189 @@ def test_build_viewer_payload() -> None:
     payload = GraphHub.build_viewer_payload(graph_assets, graph_layers, "r1")
     assert payload["base"]["nodes"][0]["id"] == "a"
     assert "x/y" in payload["extensions"]
+
+
+# ---------------------------------------------------------------------------
+# Python ↔ JS extension-payload schema contract
+#
+# JS GraphDataStore reads `name`, `legend`, `nodes`, `sync_keys`, and
+# `has_label_formatter` off `payload.extensions[extId]`. The JS LRU and the
+# layer panel's "L" badge depend on `has_label_formatter`, so a silent drop
+# of any of these fields breaks node-label rendering on canvas.
+# ---------------------------------------------------------------------------
+
+# Fields that GraphHub MUST forward verbatim from a GraphExtensionPayload into
+# the per-layer dict that lands in `payload.extensions[extId]` for the JS
+# runtime.  `id` is intentionally excluded — it lives in the dict key.
+_REQUIRED_EXTENSION_DICT_FIELDS = frozenset(
+    f.name for f in fields(GraphExtensionPayload) if f.name != "id"
+)
+
+
+def _make_label_extension(
+    ext_id: str = "m", *, with_label: bool = True
+) -> GraphExtension:
+    ext = GraphExtension(id=ext_id, name="Metric")
+    ext.add_node_data("n0", {"foo": 1.0})
+    if with_label:
+        ext.set_label_formatter(lambda d: [f"foo={d.get('foo', 0):.2f}"])
+    return ext
+
+
+def test_graph_hub_preserves_has_label_formatter() -> None:
+    """JS reads `has_label_formatter` to gate label rendering and the L badge."""
+    hub = GraphHub()
+    hub.register_asset("r0", {"legend": [], "nodes": [{"id": "n0"}], "edges": []}, {})
+    analysis = RecordAnalysis()
+    analysis.add_graph_layer("m", _make_label_extension())
+
+    hub.add_analysis_layers("r0", "lens", analysis)
+
+    slot = hub.build_payload()["graph_layers"]["r0"]["lens/m"]
+    assert slot.get("has_label_formatter") is True
+
+
+def test_graph_hub_no_label_formatter_emits_false() -> None:
+    """Layer without a label formatter must serialize an explicit `False`."""
+    hub = GraphHub()
+    hub.register_asset("r0", {"legend": [], "nodes": [{"id": "n0"}], "edges": []}, {})
+    analysis = RecordAnalysis()
+    analysis.add_graph_layer("m", _make_label_extension(with_label=False))
+
+    hub.add_analysis_layers("r0", "lens", analysis)
+
+    slot = hub.build_payload()["graph_layers"]["r0"]["lens/m"]
+    assert slot.get("has_label_formatter") is False
+
+
+def test_graph_hub_layer_dict_has_full_extension_contract() -> None:
+    """All non-id GraphExtensionPayload fields must round-trip through GraphHub.
+
+    Adding a new field to GraphExtensionPayload without forwarding it through
+    GraphHub silently breaks the JS contract.  This test pins the contract.
+    """
+    hub = GraphHub()
+    hub.register_asset("r0", {"legend": [], "nodes": [{"id": "n0"}], "edges": []}, {})
+    payload = GraphExtensionPayload(
+        id="m",
+        name="Metric",
+        legend=[{"label": "L", "color": "#000"}],
+        sync_keys=["debug_handle"],
+        has_label_formatter=True,
+        nodes={"n0": GraphExtensionNodePayload(label_append=["x=1"])},
+    )
+    analysis = RecordAnalysis()
+    analysis.add_graph_layer("m", payload)
+    hub.add_analysis_layers("r0", "lens", analysis)
+
+    slot = hub.build_payload()["graph_layers"]["r0"]["lens/m"]
+    missing = _REQUIRED_EXTENSION_DICT_FIELDS - set(slot)
+    assert not missing, f"GraphHub dropped fields: {sorted(missing)}"
+
+
+def test_graph_hub_node_payload_dict_has_full_node_contract() -> None:
+    """Per-node extension entries must carry every GraphExtensionNodePayload field."""
+    hub = GraphHub()
+    hub.register_asset("r0", {"legend": [], "nodes": [{"id": "n0"}], "edges": []}, {})
+    payload = GraphExtensionPayload(
+        id="m",
+        name="Metric",
+        nodes={
+            "n0": GraphExtensionNodePayload(
+                info={"k": 1},
+                tooltip=["t"],
+                label_append=["lbl"],
+                fill_color="#abc",
+            )
+        },
+    )
+    analysis = RecordAnalysis()
+    analysis.add_graph_layer("m", payload)
+    hub.add_analysis_layers("r0", "lens", analysis)
+
+    node_dict = hub.build_payload()["graph_layers"]["r0"]["lens/m"]["nodes"]["n0"]
+    expected_fields = {f.name for f in fields(GraphExtensionNodePayload)}
+    missing = expected_fields - set(node_dict)
+    assert not missing, f"node payload dropped fields: {sorted(missing)}"
+
+
+def test_to_payload_with_overrides_preserves_has_label_formatter() -> None:
+    """`id_override`/`name_override` must not strip the rest of the contract.
+
+    The override branch in interfaces.GraphLayerContribution.to_payload re-
+    constructs a GraphExtensionPayload; if it omits a field, JS loses it.
+    """
+    contribution = GraphLayerContribution(
+        extension=_make_label_extension(),
+        id_override="renamed_id",
+        name_override="Renamed",
+    )
+    payload = contribution.to_payload()
+
+    assert payload.id == "renamed_id"
+    assert payload.name == "Renamed"
+    assert payload.has_label_formatter is True
+
+
+def test_to_payload_with_overrides_preserves_sync_keys() -> None:
+    payload_in = GraphExtensionPayload(
+        id="m",
+        name="Metric",
+        sync_keys=["debug_handle", "from_node"],
+        has_label_formatter=True,
+        nodes={"n0": GraphExtensionNodePayload(label_append=["x"])},
+    )
+    contribution = GraphLayerContribution(
+        extension=payload_in,
+        id_override="renamed",
+    )
+    payload_out = contribution.to_payload()
+
+    assert payload_out.sync_keys == ["debug_handle", "from_node"]
+    assert payload_out.has_label_formatter is True
+
+
+def test_graph_hub_preserves_has_label_formatter_through_overrides() -> None:
+    """End-to-end: lens contributes with overrides → JS-bound dict still flagged."""
+    hub = GraphHub()
+    hub.register_asset("r0", {"legend": [], "nodes": [{"id": "n0"}], "edges": []}, {})
+    analysis = RecordAnalysis()
+    analysis.add_graph_layer(
+        "m",
+        _make_label_extension(),
+        id_override="metric_v2",
+        name_override="Metric V2",
+    )
+
+    hub.add_analysis_layers("r0", "lens", analysis)
+
+    slot = hub.build_payload()["graph_layers"]["r0"]["lens/m"]
+    assert slot.get("name") == "Metric V2"
+    assert slot.get("has_label_formatter") is True
+
+
+def test_build_viewer_payload_includes_layout_constants() -> None:
+    """JS reads `payload.layout` for line_height / max_label_extensions / etc.
+
+    Without this, JS silently uses the fallback constants in graph_data_store.js;
+    those happen to match today's Python defaults but will drift on first
+    Python-side change to `_NODE_LINE_HEIGHT` or `_MAX_LABEL_EXTENSIONS`.
+    """
+    from executorch.devtools.fx_viewer.exporter import FXGraphExporter
+
+    graph_assets = {
+        "r1": {"base": {"legend": [], "nodes": [{"id": "a"}], "edges": []}, "meta": {}}
+    }
+    payload = GraphHub.build_viewer_payload(graph_assets, {"r1": {}}, "r1")
+
+    assert "layout" in payload
+    expected = FXGraphExporter._layout_constants_payload()
+    for key in ("line_height", "y_padding", "max_label_extensions", "base_font_px"):
+        assert payload["layout"][key] == expected[key]
+
+
+def test_build_viewer_payload_layout_present_for_missing_graph_ref() -> None:
+    """Empty asset path must still emit layout — JS fallback path shouldn't kick in."""
+    payload = GraphHub.build_viewer_payload({}, {}, "missing")
+    assert "layout" in payload
+    assert "max_label_extensions" in payload["layout"]