Add Block Kit layout to frontend-triage notifications

agents/frontend-triage/hackbot_agents/frontend_triage/notify.py

@@ -53,22 +53,29 @@ def channel_for(product: str | None, component: str | None) -> str | None:
     return SLACK_CHANNELS.get(f"{product.strip()} :: {component.strip()}")
 
 
-def build_message(result: FrontendTriageResult, *, run_id: str) -> str:
-    """Render the notification for an auto-applied run."""
+def _bug_link(result: FrontendTriageResult) -> str:
+    return _link(BUG_URL.format(bug_id=result.bug_id), f"Bug {result.bug_id}")
+
+
+def _summary(result: FrontendTriageResult) -> str:
+    return result.summary.strip() if result.summary else ""
+
+
+def _is_urgent(result: FrontendTriageResult) -> bool:
     assessment = result.severity_assessment
     suggested = (assessment.suggested or "") if assessment else ""
-    # The severity the run *judged*, which is not always the one it wrote to the bug:
-    # `rules/severity-assessment.md` holds back the field change when the severity
-    # confidence is not high. An S1 it only suspects is still worth flagging.
-    urgent = suggested.strip().upper() == URGENT_SEVERITY
-
-    headline = _link(BUG_URL.format(bug_id=result.bug_id), f"Bug {result.bug_id}")
-    if result.summary and result.summary.strip():
-        headline += f" — {result.summary.strip()}"
+    return suggested.strip().upper() == URGENT_SEVERITY
+
+
+def build_message(result: FrontendTriageResult, *, run_id: str) -> str:
+    headline = _bug_link(result)
+    summary = _summary(result)
+    if summary:
+        headline += f" — {summary}"
     # The level is spelled out next to the emoji, so it still reads as an S1 for anyone
     # whose client does not render one.
     headline = f"*{headline}*"
-    if urgent:
+    if _is_urgent(result):
         headline = f":red_circle: {headline} ({URGENT_SEVERITY})"
 
     return "\n".join(
@@ -79,6 +86,68 @@ def build_message(result: FrontendTriageResult, *, run_id: str) -> str:
     )
 
 
+def _severity_field(result: FrontendTriageResult) -> str | None:
+    assessment = result.severity_assessment
+    suggested = (assessment.suggested or "").strip() if assessment else ""
+    if not suggested:
+        return None
+    value = suggested.upper()
+    # The assessment's own confidence, not the run's: `rules/severity-assessment.md`
+    # holds the field change back below `high`, so anything less is a suggestion the
+    # bug did not receive, and reading it as applied would be wrong.
+    confidence = (assessment.confidence or "").strip() if assessment else ""
+    if confidence and confidence.lower() != "high":
+        value += f" (suggested, {confidence.lower()} confidence)"
+    return f"*Severity*\n{value}"
+
+
+def _component_field(result: FrontendTriageResult) -> str | None:
+    """Where the bug lives, for the channels that own more than one component."""
+    if not result.product or not result.component:
+        return None
+    return f"*Component*\n{result.product.strip()} :: {result.component.strip()}"
+
+
+def build_blocks(result: FrontendTriageResult, *, run_id: str) -> list[dict]:
+    headline = f"*{_bug_link(result)}*"
+    summary = _summary(result)
+    if summary:
+        headline += f"\n{summary}"
+    if _is_urgent(result):
+        headline = f":red_circle: *{URGENT_SEVERITY}* {headline}"
+
+    blocks: list[dict] = [
+        {"type": "section", "text": {"type": "mrkdwn", "text": headline}}
+    ]
+
+    fields = [
+        field
+        for field in (_severity_field(result), _component_field(result))
+        if field is not None
+    ]
+    if fields:
+        blocks.append(
+            {
+                "type": "section",
+                "fields": [{"type": "mrkdwn", "text": field} for field in fields],
+            }
+        )
+
+    blocks.append(
+        {
+            "type": "context",
+            "elements": [
+                {
+                    "type": "mrkdwn",
+                    "text": "Triaged by frontend-triage · "
+                    + _link(RUN_URL.format(run_id=run_id), "run details"),
+                }
+            ],
+        }
+    )
+    return blocks
+
+
 def record_notification(
     recorder: ActionsRecorder, result: FrontendTriageResult, *, run_id: str
 ) -> dict | None:
@@ -106,4 +175,9 @@ def record_notification(
         return None
 
     logger.info("Bug %s: reporting triage to %s", result.bug_id, channel)
-    return record_message(recorder, channel, build_message(result, run_id=run_id))
+    return record_message(
+        recorder,
+        channel,
+        build_message(result, run_id=run_id),
+        blocks=build_blocks(result, run_id=run_id),
+    )

agents/frontend-triage/tests/test_notify.py

@@ -6,12 +6,16 @@
 ask for it.
 """
 
+import re
+
+import pytest
 from hackbot_agents.frontend_triage.agent import (
     FrontendTriageResult,
     SeverityAssessment,
 )
 from hackbot_agents.frontend_triage.config import TRIAGE_SCOPE
 from hackbot_agents.frontend_triage.notify import (
+    build_blocks,
     build_message,
     channel_for,
     record_notification,
@@ -153,6 +157,154 @@ def test_an_auto_applied_run_records_one_slack_action():
     assert [a["type"] for a in recorder.actions] == ["slack.post_message"]
     assert action["params"]["channel"] == "#hnt-dev-triage"
     assert action["params"]["text"] == build_message(_result(), run_id=RUN_ID)
+    # The layout travels with the text, which stays the fallback.
+    assert action["params"]["blocks"] == build_blocks(_result(), run_id=RUN_ID)
+
+
+# --- the Block Kit layout ---
+
+
+def _blocks(**overrides) -> list[dict]:
+    return build_blocks(_result(**overrides), run_id=RUN_ID)
+
+
+def _block_of(kind: str, **overrides) -> dict | None:
+    return next((b for b in _blocks(**overrides) if b["type"] == kind), None)
+
+
+def test_the_layout_reads_bug_then_facts_then_run():
+    assert [b["type"] for b in _blocks()] == ["section", "section", "context"]
+
+
+def test_the_headline_links_the_bug_and_puts_the_summary_under_it():
+    assert _blocks()[0] == {
+        "type": "section",
+        "text": {"type": "mrkdwn", "text": f"*{BUG_LINK}*\n{SUMMARY}"},
+    }
+
+
+def test_a_bug_with_no_summary_is_just_the_link():
+    for summary in (None, "", "   "):
+        assert _blocks(summary=summary)[0]["text"]["text"] == f"*{BUG_LINK}*"
+
+
+def test_an_s1_leads_the_headline():
+    # Marked and named, so it still reads as an S1 where the emoji does not render.
+    headline = _blocks(
+        severity_assessment=SeverityAssessment(suggested="S1", confidence="high")
+    )[0]["text"]["text"]
+    assert headline == f":red_circle: *S1* *{BUG_LINK}*\n{SUMMARY}"
+
+
+def test_the_fields_grid_carries_the_severity_and_the_component():
+    assert _blocks()[1]["fields"] == [
+        {"type": "mrkdwn", "text": "*Severity*\nS3"},
+        {"type": "mrkdwn", "text": "*Component*\nFirefox :: New Tab Page"},
+    ]
+
+
+def test_a_severity_the_bug_did_not_receive_says_so():
+    # Below `high` the assessment is held back from the bug, so reporting it plainly
+    # would read as a field that was written when it was not.
+    fields = _blocks(
+        severity_assessment=SeverityAssessment(suggested="S2", confidence="medium")
+    )[1]["fields"]
+    assert fields[0]["text"] == "*Severity*\nS2 (suggested, medium confidence)"
+
+
+def test_a_field_with_nothing_to_say_is_dropped():
+    fields = _blocks(severity_assessment=None)[1]["fields"]
+    assert [f["text"] for f in fields] == ["*Component*\nFirefox :: New Tab Page"]
+    # And with neither, the grid itself goes rather than rendering empty.
+    assert [b["type"] for b in _blocks(severity_assessment=None, product=None)] == [
+        "section",
+        "context",
+    ]
+
+
+def test_the_run_sits_in_the_context_line():
+    element = _block_of("context")["elements"][0]
+    assert element["text"] == (
+        "Triaged by frontend-triage · "
+        f"<https://hackbot.moz.tools/runs/{RUN_ID}|run details>"
+    )
+
+
+def test_the_notification_asks_for_nothing_yet():
+    # The layout is the whole change: no interactive element is posted until the
+    # buttons land, so nothing here can be clicked.
+    assert _block_of("actions") is None
+
+
+# --- the two renderings say the same things ---
+
+_LINK = re.compile(r"<([^|>]+)\|([^>]+)>")
+# Punctuation that only ever joins words: dropped so that "(S1)" and "*S1*" are the
+# same fact said twice, rather than two tokens that happen to look alike.
+_PUNCTUATION = "*()·,—"
+
+
+def _visible_text(blocks: list[dict]) -> str:
+    """Every string a reader sees in `blocks`, whatever block it sits in."""
+    parts: list[str] = []
+    for block in blocks:
+        if isinstance(block.get("text"), dict):
+            parts.append(block["text"]["text"])
+        for field in block.get("fields", []):
+            parts.append(field["text"])
+        for element in block.get("elements", []):
+            if isinstance(element.get("text"), dict):
+                parts.append(element["text"]["text"])
+            elif isinstance(element.get("text"), str):
+                parts.append(element["text"])
+    return "\n".join(parts)
+
+
+def _facts(text: str) -> set[str]:
+    """What `text` tells a reader, as words, with link markup reduced to its label."""
+    labelled = _LINK.sub(lambda match: match.group(2), text)
+    return {word.strip(_PUNCTUATION) for word in labelled.split()} - {""}
+
+
+def _urls(text: str) -> set[str]:
+    return {match.group(1) for match in _LINK.finditer(text)}
+
+
+@pytest.mark.parametrize(
+    "overrides",
+    [
+        {},
+        {"summary": None},
+        {"severity_assessment": None},
+        {"severity_assessment": SeverityAssessment(suggested="S1", confidence="high")},
+        {
+            "severity_assessment": SeverityAssessment(
+                suggested="S1", confidence="medium"
+            )
+        },
+        {"product": None, "component": None},
+    ],
+    ids=[
+        "ordinary",
+        "no-summary",
+        "no-severity",
+        "urgent",
+        "urgent-suggested",
+        "no-component",
+    ],
+)
+def test_the_blocks_say_everything_the_fallback_text_says(overrides):
+    # The blocks are what almost everyone reads, and the text is what Slack falls back
+    # to. They may lay the same facts out differently, but nothing may be in the
+    # fallback and missing from the blocks: that would be a fact only the people
+    # reading a push notification ever see.
+    result = _result(**overrides)
+    text = build_message(result, run_id=RUN_ID)
+    blocks = _visible_text(build_blocks(result, run_id=RUN_ID))
+
+    assert _facts(text) <= _facts(blocks)
+    # Labels may be worded differently; the things they link to may not.
+    assert _urls(text) <= _urls(blocks)
 
 
 def test_a_run_that_was_not_auto_applied_reports_nothing():