Skip to content

Commit e49ef92

Browse files
nehraaabhinav-dvp
andauthored
fix(reporting): render WhatsApp and SMS links as plain text (#4846)
* fix(reporting): render WhatsApp and SMS links as plain text WhatsApp and SMS recipients were shown Slack's link markup verbatim — `[<https://grafana...|E1>]` instead of a readable citation. Two Slack-flavoured helpers emitted `<url|label>` through format_slack_link: - _render_claim_lines (report.py) — evidence citations [E1] - build_investigation_trace (infrastructure.py) — S3 console links Telegram already dodges this with its own _render_claim_lines_telegram calling format_html_link. WhatsApp was the only channel with no plain-text path, and ReportMessages.sms_text returns whatsapp_text, so SMS inherited the bug. The conversion happens at the WhatsApp boundary rather than per-helper. That covers citations, trace links, and any Slack link a future change adds upstream, in one place — the same approach _render_plain_report already takes for terminal output. `label (url)` keeps the URL rather than dropping it, because WhatsApp and SMS auto-linkify bare URLs. The slack_links_to_plain_text helper is the inverse of format_slack_link and is now shared with the terminal plain-text renderer, which previously held its own byte-identical copy. The parametrized cases in test_terminal_renderer.py follow the rename and become coverage for the shared helper. Not touched: integrations/telegram/markdown.py:16 holds a third copy of the regex, but it drives an HTML conversion rather than plain text, so folding it in would change Telegram behavior. Left for a separate change. Fixes #4695 * refactor(reporting): move SLACK_LINK_RE to config/constants/reporting Greptile flagged P2: shared static constants belong under config/constants/, not in a feature module. The regex is now in config/constants/reporting.py and re-exported via config/constants/__init__.py. Both consumers — formatters/base.py and renderers/terminal.py — import it from the canonical location. Fixes the review comment on #4846. --------- Co-authored-by: nehraa <abhinavnehra2203@gmail.com>
1 parent 49adcdd commit e49ef92

7 files changed

Lines changed: 117 additions & 20 deletions

File tree

config/constants/__init__.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,7 @@
252252
REDIS_SSL_ENV,
253253
REDIS_USERNAME_ENV,
254254
)
255+
from config.constants.reporting import SLACK_LINK_RE
255256
from config.constants.runtime_metadata import (
256257
GITHUB_REPO_ENV,
257258
GITHUB_REPOSITORY_ENV,
@@ -548,6 +549,7 @@
548549
"SIGNOZ_URL_ENV",
549550
"SLACK_APP_TOKEN_ENV",
550551
"SLACK_BOT_TOKEN_ENV",
552+
"SLACK_LINK_RE",
551553
"SMTP_DEFAULT_TO_ENV",
552554
"SMTP_FROM_ADDRESS_ENV",
553555
"SMTP_HOST_ENV",

config/constants/reporting.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
"""Constants shared across the investigation reporting package."""
2+
3+
from __future__ import annotations
4+
5+
import re
6+
from typing import Final
7+
8+
# Matches Slack-style links: <url|label> or <url>. Shared between the WhatsApp
9+
# formatter and the terminal plain-text renderer so both stay in step.
10+
SLACK_LINK_RE: Final[re.Pattern[str]] = re.compile(r"<(https?://[^|>]+)(?:\|([^>]+))?>")
11+
12+
__all__ = ["SLACK_LINK_RE"]

tests/delivery/test_terminal_renderer.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,10 @@
55
import pytest
66

77
from platform.terminal.theme import DEFAULT_THEME_NAME, set_active_theme
8+
from tools.investigation.reporting.formatters.base import slack_links_to_plain_text
89
from tools.investigation.reporting.renderers.terminal import (
910
_rich_line_with_links,
1011
_strip_mrkdwn,
11-
_strip_slack_links,
1212
render_report,
1313
)
1414

@@ -125,7 +125,7 @@ def test_strip_mrkdwn(raw: str, expected: str) -> None:
125125
"A (https://a.test) and B (https://b.test)",
126126
),
127127
# No Slack-style link -> passthrough (including bare URLs, which
128-
# _strip_slack_links is not responsible for).
128+
# slack_links_to_plain_text is not responsible for).
129129
("plain text", "plain text"),
130130
(
131131
"https://example.com without angle brackets",
@@ -135,8 +135,8 @@ def test_strip_mrkdwn(raw: str, expected: str) -> None:
135135
("", ""),
136136
],
137137
)
138-
def test_strip_slack_links(raw: str, expected: str) -> None:
139-
assert _strip_slack_links(raw) == expected
138+
def test_slack_links_to_plain_text(raw: str, expected: str) -> None:
139+
assert slack_links_to_plain_text(raw) == expected
140140

141141

142142
@pytest.mark.parametrize(
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
"""WhatsApp and SMS are plain-text channels: no Slack link markup may reach them."""
2+
3+
from __future__ import annotations
4+
5+
from tools.investigation.reporting.context import build_report_context
6+
from tools.investigation.reporting.formatters.messages import build_report_messages
7+
from tools.investigation.reporting.formatters.report import format_whatsapp_message
8+
9+
10+
def _make_state() -> dict:
11+
return {
12+
"alert_name": "Checkout latency spike",
13+
"severity": "critical",
14+
"root_cause": "Checkout service was throttled by the upstream API cluster.",
15+
"validated_claims": [
16+
{
17+
"claim": "Grafana logs show repeated 500 responses.",
18+
"evidence_sources": ["grafana_logs"],
19+
}
20+
],
21+
"available_sources": {
22+
"grafana": {
23+
"grafana_endpoint": "https://myorg.grafana.net",
24+
"service_name": "checkout-api",
25+
},
26+
},
27+
"evidence": {
28+
"grafana_logs": [{"message": "service unavailable"}],
29+
"grafana_logs_query": '{service="checkout-api"}',
30+
},
31+
}
32+
33+
34+
def test_whatsapp_message_renders_evidence_links_as_plain_text() -> None:
35+
body = format_whatsapp_message(build_report_context(_make_state()))
36+
37+
assert "<https://" not in body
38+
assert "E1 (https://myorg.grafana.net" in body
39+
40+
41+
def test_whatsapp_message_renders_s3_trace_links_as_plain_text() -> None:
42+
state = _make_state()
43+
state["raw_alert"] = {
44+
"annotations": {
45+
"landing_bucket": "tracer-landing",
46+
"s3_key": "runs/checkout/input.json",
47+
},
48+
}
49+
50+
body = format_whatsapp_message(build_report_context(state))
51+
52+
assert "<https://" not in body
53+
assert "S3 object (https://" in body
54+
55+
56+
def test_sms_body_carries_no_slack_link_markup() -> None:
57+
messages = build_report_messages(build_report_context(_make_state()))
58+
59+
assert messages.sms_text == messages.whatsapp_text
60+
assert "<https://" not in messages.sms_text
61+
62+
63+
def test_slack_and_telegram_keep_their_own_link_syntax() -> None:
64+
"""The plain-text conversion must not bleed into the other channels."""
65+
messages = build_report_messages(build_report_context(_make_state()))
66+
67+
assert "<https://myorg.grafana.net" in messages.slack_text
68+
assert '<a href="https://myorg.grafana.net' in messages.telegram_html

tools/investigation/reporting/formatters/base.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
"""Base formatting utilities for report generation."""
22

3+
from __future__ import annotations
4+
35
import html
6+
import re
7+
8+
from config.constants import SLACK_LINK_RE
9+
10+
__all__ = ["format_html_link", "format_slack_link", "shorten_text", "slack_links_to_plain_text"]
411

512

613
def shorten_text(text: str, max_chars: int = 120, suffix: str = "...") -> str:
@@ -32,6 +39,22 @@ def format_slack_link(label: str, url: str | None) -> str:
3239
return f"<{url}|{safe_label}>"
3340

3441

42+
def slack_links_to_plain_text(text: str) -> str:
43+
"""Convert Slack ``<url|label>`` links to plain ``label (url)``.
44+
45+
The inverse of :func:`format_slack_link`, for channels that render no markup
46+
(WhatsApp, SMS, terminal plain-text mode). The URL is kept because
47+
plain-text clients auto-linkify bare URLs.
48+
"""
49+
50+
def _repl(match: re.Match[str]) -> str:
51+
url = str(match.group(1))
52+
label = match.group(2)
53+
return f"{label} ({url})" if label else url
54+
55+
return SLACK_LINK_RE.sub(_repl, text)
56+
57+
3558
def format_html_link(label: str, url: str | None) -> str:
3659
"""Return a Telegram HTML <a> tag, or escaped plain label without a URL."""
3760
if not url:

tools/investigation/reporting/formatters/report.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from tools.investigation.reporting.formatters.base import (
88
format_html_link,
99
format_slack_link,
10+
slack_links_to_plain_text,
1011
)
1112
from tools.investigation.reporting.formatters.evidence import (
1213
format_cited_evidence_section,
@@ -716,7 +717,9 @@ def format_whatsapp_message(ctx: ReportContext) -> str:
716717
if meta_bits:
717718
parts.append(" | ".join(meta_bits))
718719

719-
return "\n\n".join(p for p in parts if p)
720+
# Shared helpers emit Slack <url|label> links; WhatsApp and SMS render no
721+
# markup, so convert at this boundary to plain `label (url)` form.
722+
return slack_links_to_plain_text("\n\n".join(p for p in parts if p))
720723

721724

722725
# ---------------------------------------------------------------------------

tools/investigation/reporting/renderers/terminal.py

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,24 @@
88
from rich.console import Console
99
from rich.text import Text
1010

11+
from config.constants import SLACK_LINK_RE
1112
from platform.observability import get_output_format
1213
from platform.terminal.theme import BRAND, DIM, HIGHLIGHT, WARNING
14+
from tools.investigation.reporting.formatters.base import slack_links_to_plain_text
1315

1416
# ─────────────────────────────────────────────────────────────────────────────
1517
# Helpers
1618
# ─────────────────────────────────────────────────────────────────────────────
1719

1820
_URL_RE = re.compile(r"https?://\S+")
19-
# Matches Slack-style links: <url|label> or <url>
20-
_SLACK_LINK_RE = re.compile(r"<(https?://[^|>]+)(?:\|([^>]+))?>")
2121

2222

2323
def _rich_line_with_links(text: str) -> Text:
2424
"""Convert a plain/Slack-mrkdwn string into a Rich Text with blue hyperlinks."""
2525
result = Text()
2626
cursor = 0
2727

28-
for m in _SLACK_LINK_RE.finditer(text):
28+
for m in SLACK_LINK_RE.finditer(text):
2929
# Text before the match
3030
if m.start() > cursor:
3131
result.append(text[cursor : m.start()])
@@ -49,17 +49,6 @@ def _rich_line_with_links(text: str) -> Text:
4949
return result
5050

5151

52-
def _strip_slack_links(text: str) -> str:
53-
"""Convert Slack <url|label> to plain 'label (url)' for plain text mode."""
54-
55-
def _repl(m: re.Match[str]) -> str:
56-
url = str(m.group(1))
57-
label = m.group(2)
58-
return f"{label} ({url})" if label else url
59-
60-
return _SLACK_LINK_RE.sub(_repl, text)
61-
62-
6352
def _strip_mrkdwn(text: str) -> str:
6453
"""Remove Slack mrkdwn bold markers (*text*) for plain output."""
6554
return re.sub(r"\*([^*\n]+)\*", r"\1", text)
@@ -251,5 +240,5 @@ def _render_rich_report(slack_message: str) -> str:
251240

252241

253242
def _render_plain_report(slack_message: str) -> str:
254-
clean = _strip_slack_links(_strip_mrkdwn(slack_message))
243+
clean = slack_links_to_plain_text(_strip_mrkdwn(slack_message))
255244
return f"\n{clean}\n"

0 commit comments

Comments
 (0)