Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions elementary/messages/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ class ActionsBlock(BaseBlock):
"LineBlock",
]

LineBlock.update_forward_refs()
LineBlock.model_rebuild()


class HeaderBlock(BaseBlock):
Expand Down Expand Up @@ -184,4 +184,4 @@ class ExpandableBlock(BaseBlock):
]

# Update forward references for recursive types
ExpandableBlock.update_forward_refs()
ExpandableBlock.model_rebuild()
146 changes: 146 additions & 0 deletions elementary/messages/formats/markdown.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import json
import re
from enum import Enum

from tabulate import tabulate

from elementary.messages.blocks import (
ActionsBlock,
CodeBlock,
DividerBlock,
ExpandableBlock,
FactListBlock,
HeaderBlock,
Icon,
IconBlock,
InlineBlock,
InlineCodeBlock,
LineBlock,
LinesBlock,
LinkBlock,
MentionBlock,
TableBlock,
TextBlock,
TextStyle,
WhitespaceBlock,
)
from elementary.messages.formats.html import ICON_TO_HTML
from elementary.messages.message_body import MessageBlock, MessageBody


class TableStyle(Enum):
TABULATE = "tabulate"
JSON = "json"


class MarkdownFormatter:
def __init__(self, table_style: TableStyle):
self._table_style = table_style

def format_icon(self, icon: Icon) -> str:
return ICON_TO_HTML[icon]

def format_text_block(self, block: TextBlock) -> str:
if block.style == TextStyle.BOLD:
return f"**{block.text}**"
elif block.style == TextStyle.ITALIC:
return f"_{block.text}_"
else:
return block.text

def format_inline_block(self, block: InlineBlock) -> str:
if isinstance(block, IconBlock):
return self.format_icon(block.icon)
elif isinstance(block, TextBlock):
return self.format_text_block(block)
elif isinstance(block, LinkBlock):
return f"[{block.text}]({block.url})"
elif isinstance(block, InlineCodeBlock):
return f"`{block.code}`"
elif isinstance(block, MentionBlock):
return block.user
elif isinstance(block, LineBlock):
return self.format_line_block(block)
elif isinstance(block, WhitespaceBlock):
return " "
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Replace HTML entity with Markdown-appropriate whitespace.

Using   is HTML syntax, not standard Markdown. Consider using regular spaces or Markdown-specific non-breaking space alternatives.

-            return " "
+            return " "

For non-breaking space in Markdown, you could also use multiple spaces or consider if whitespace blocks need special handling at all.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return " "
return " "
🤖 Prompt for AI Agents
In elementary/messages/formats/markdown.py at line 65, replace the HTML entity
" " with a Markdown-appropriate whitespace. Use regular spaces or multiple
spaces instead of the HTML non-breaking space, ensuring the output conforms to
Markdown syntax rather than HTML.

else:
raise ValueError(f"Unsupported inline block type: {type(block)}")

def format_line_block(self, block: LineBlock) -> str:
return block.sep.join(
[self.format_inline_block(inline) for inline in block.inlines]
)

def format_lines_block(self, block: LinesBlock) -> str:
formatted_parts = []
for index, line_block in enumerate(block.lines):
formatted_line = self.format_line_block(line_block)
formatted_parts.append(formatted_line)
is_bullet = re.match(r"^\s*[*-]", formatted_line)
is_last = index == len(block.lines) - 1
if not is_bullet and not is_last:
# in markdown, single line breaks are not rendered as new lines, except for bullet lists
# so we need to add a backslash to force a new line
formatted_parts.append("\\")
if not is_last:
formatted_parts.append("\n")
return "".join(formatted_parts)
Comment on lines +74 to +87
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify the line break handling logic.

The complex logic for handling line breaks and bullet lists appears correct for Markdown rendering, but this should be tested to ensure proper output formatting.


🏁 Script executed:

#!/bin/bash
# Description: Search for existing tests of the MarkdownFormatter to verify line break handling

# Look for markdown formatter tests
fd -t f -e py test | xargs rg -l "MarkdownFormatter|format_lines_block" -A 5 -B 5

Length of output: 83


🏁 Script executed:

#!/bin/bash
# Search for the MarkdownFormatter class definition and usage across all Python files
rg -n "class .*MarkdownFormatter" -g "*.py" || echo "No class definition found"
rg -n "MarkdownFormatter" -g "*.py" || echo "No MarkdownFormatter references found"
rg -n "format_lines_block" -g "*.py" || echo "No format_lines_block references found"

Length of output: 938


Add unit tests for MarkdownFormatter.format_lines_block

There are currently no tests covering elementary/messages/formats/markdown.py:74–87. We need to validate the backslash + newline logic for bullets vs. non-bullets to prevent regressions.

Please add tests that cover at least:

  • A single non-bullet line (no trailing backslash or extra newline)
  • Multiple consecutive non-bullet lines (each should end with \\\n except the last)
  • Bullet lines (* or -), ensuring they don’t get a backslash
  • Mixed blocks where bullets and non-bullets alternate
  • Edge cases (empty lines, whitespace-only lines)

File to test:

  • elementary/messages/formats/markdown.py, method format_lines_block
🤖 Prompt for AI Agents
In elementary/messages/formats/markdown.py around lines 74 to 87, add unit tests
for the format_lines_block method to cover its behavior thoroughly. Create tests
for a single non-bullet line ensuring no trailing backslash or extra newline,
multiple consecutive non-bullet lines verifying each ends with a backslash and
newline except the last, bullet lines starting with * or - confirming they do
not get a backslash, mixed blocks alternating bullets and non-bullets to check
correct formatting, and edge cases including empty and whitespace-only lines.
These tests should validate the backslash plus newline logic to prevent
regressions.


def format_fact_list_block(self, block: FactListBlock) -> str:
facts = [
f"{self.format_line_block(fact.title)}: {self.format_line_block(fact.value)}"
for fact in block.facts
]
return " | ".join(facts)

def format_table_block(self, block: TableBlock) -> str:
if self._table_style == TableStyle.TABULATE:
table = tabulate(block.rows, headers=block.headers, tablefmt="simple")
return f"```\n{table}\n```"
elif self._table_style == TableStyle.JSON:
dicts = [
{header: cell for header, cell in zip(block.headers, row)}
for row in block.rows
]
return f"```\n{json.dumps(dicts, indent=2)}\n```"
else:
raise ValueError(f"Invalid table style: {self._table_style}")

def format_expandable_block(self, block: ExpandableBlock) -> str:
return f"**{block.title}**\\\n{self.format_message_blocks(block.body)}"

def format_message_block(self, block: MessageBlock) -> str:
if isinstance(block, HeaderBlock):
return f"# {block.text}"
elif isinstance(block, CodeBlock):
return f"```\n{block.text}\n```"
elif isinstance(block, LinesBlock):
return self.format_lines_block(block)
elif isinstance(block, FactListBlock):
return self.format_fact_list_block(block)
elif isinstance(block, ExpandableBlock):
return self.format_expandable_block(block)
elif isinstance(block, TableBlock):
return self.format_table_block(block)
elif isinstance(block, DividerBlock):
return "---"
elif isinstance(block, ActionsBlock):
# Actions not supported for text
return ""
else:
raise ValueError(f"Unsupported message block type: {type(block)}")

def format_message_blocks(self, blocks: list[MessageBlock]) -> str:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Consider Python version compatibility for type annotations.

The list[MessageBlock] syntax requires Python 3.9+. Ensure this is consistent with the project's Python version requirements.


🏁 Script executed:

#!/bin/bash
# Description: Check Python version requirements and existing type annotation patterns

# Check for Python version requirements
fd -t f pyproject.toml setup.py setup.cfg requirements.txt | xargs cat 2>/dev/null | rg -i "python.*version|python_requires"

# Check existing type annotation patterns in the codebase
rg "List\[|list\[" --type py -A 1 -B 1 | head -20

Length of output: 1950


🏁 Script executed:

#!/bin/bash
set -euxo pipefail

echo "Searching for project metadata files..."
# Find potential Python version specifiers
fd -HI pyproject.toml setup.py Pipfile | sed -e 's/^/FOUND: /'

# Display contents if found
for f in $(fd -HI pyproject.toml setup.py Pipfile); do
  echo "=== $f ==="
  head -n50 "$f"
done

echo
echo "Checking GitHub Actions for python-version..."
rg -n "python-version" -t yaml .github/workflows || echo "No python-version keys found in workflows"

echo
echo "Checking for Travis CI configuration..."
if [ -f .travis.yml ]; then
  echo "=== .travis.yml ==="
  head -n30 .travis.yml
else
  echo "No .travis.yml"
fi

echo
echo "Checking for runtime.txt..."
if [ -f runtime.txt ]; then
  echo "=== runtime.txt ==="
  head -n10 runtime.txt
else
  echo "No runtime.txt"
fi

echo
echo "Looking for Dockerfiles with Python base images..."
rg -n "FROM python" -t docker || echo "No Dockerfiles using Python base image"

echo
echo "Searching for Poetry configuration..."
rg -R "tool.poetry" -t toml || echo "No Poetry metadata found"

Length of output: 324


Switch to typing.List for consistency and Python <3.9 compatibility

It looks like the rest of the codebase (and your tests) still use typing.List[...], whereas built-in generics (list[...]) only work on Python 3.9+. To avoid a breaking change and keep annotations uniform, update this signature to use List[MessageBlock].

• File elementary/messages/formats/markdown.py
– Add at top (if not already present):
python from typing import List
– Change
diff - def format_message_blocks(self, blocks: list[MessageBlock]) -> str: + def format_message_blocks(self, blocks: List[MessageBlock]) -> str:

🤖 Prompt for AI Agents
In elementary/messages/formats/markdown.py at line 135, the method signature
uses the built-in generic list[MessageBlock], which is incompatible with Python
versions earlier than 3.9 and inconsistent with the rest of the codebase. To fix
this, import List from typing at the top of the file if not already imported,
and change the method signature to use List[MessageBlock] instead of
list[MessageBlock].

if not blocks:
return ""
return "\n\n".join([self.format_message_block(block) for block in blocks])

def format(self, message: MessageBody) -> str:
return self.format_message_blocks(message.blocks)


def format_markdown(
message: MessageBody, table_style: TableStyle = TableStyle.TABULATE
) -> str:
formatter = MarkdownFormatter(table_style)
return formatter.format(message)
140 changes: 140 additions & 0 deletions elementary/messages/formats/text.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import json
from enum import Enum
from typing import List

from tabulate import tabulate

from elementary.messages.blocks import (
ActionsBlock,
CodeBlock,
DividerBlock,
ExpandableBlock,
FactListBlock,
HeaderBlock,
Icon,
IconBlock,
InlineBlock,
InlineCodeBlock,
LineBlock,
LinesBlock,
LinkBlock,
MentionBlock,
TableBlock,
TextBlock,
WhitespaceBlock,
)
from elementary.messages.formats.html import ICON_TO_HTML
from elementary.messages.message_body import MessageBlock, MessageBody


class IconStyle(Enum):
UNICODE = "unicode"
NAME = "name"
OMIT = "omit"


class TableStyle(Enum):
TABULATE = "tabulate"
JSON = "json"


class TextFormatter:
def __init__(self, icon_style: IconStyle, table_style: TableStyle):
self._icon_style = icon_style
self._table_style = table_style

def format_icon(self, icon: Icon) -> str:
if self._icon_style == IconStyle.OMIT:
return ""
elif self._icon_style == IconStyle.UNICODE:
return ICON_TO_HTML[icon]
elif self._icon_style == IconStyle.NAME:
return f":{icon.value}:"
else:
raise ValueError(f"Invalid icon style: {self._icon_style}")

def format_inline_block(self, block: InlineBlock) -> str:
if isinstance(block, IconBlock):
return self.format_icon(block.icon)
elif isinstance(block, TextBlock):
return block.text
elif isinstance(block, LinkBlock):
return f"{block.text} ({block.url})"
elif isinstance(block, InlineCodeBlock):
return block.code
elif isinstance(block, MentionBlock):
return block.user
elif isinstance(block, LineBlock):
return self.format_line_block(block)
elif isinstance(block, WhitespaceBlock):
return " "
else:
raise ValueError(f"Unsupported inline block type: {type(block)}")

def format_line_block(self, block: LineBlock) -> str:
return block.sep.join(
[self.format_inline_block(inline) for inline in block.inlines]
)

def format_lines_block(self, block: LinesBlock) -> str:
return "\n".join(
[self.format_line_block(line_block) for line_block in block.lines]
)

def format_fact_list_block(self, block: FactListBlock) -> str:
facts = [
f"{self.format_line_block(fact.title)}: {self.format_line_block(fact.value)}"
for fact in block.facts
]
return " | ".join(facts)

def format_table_block(self, block: TableBlock) -> str:
if self._table_style == TableStyle.TABULATE:
return tabulate(block.rows, headers=block.headers, tablefmt="simple")
elif self._table_style == TableStyle.JSON:
dicts = [
{header: cell for header, cell in zip(block.headers, row)}
for row in block.rows
]
return json.dumps(dicts, indent=2)
else:
raise ValueError(f"Invalid table style: {self._table_style}")

def format_expandable_block(self, block: ExpandableBlock) -> str:
return f"{block.title}\n{self.format_message_blocks(block.body)}"

def format_message_block(self, block: MessageBlock) -> str:
if isinstance(block, (HeaderBlock, CodeBlock)):
return block.text
elif isinstance(block, LinesBlock):
return self.format_lines_block(block)
elif isinstance(block, FactListBlock):
return self.format_fact_list_block(block)
elif isinstance(block, ExpandableBlock):
return self.format_expandable_block(block)
elif isinstance(block, TableBlock):
return self.format_table_block(block)
elif isinstance(block, ActionsBlock):
# Actions not supported for text
return ""
elif isinstance(block, DividerBlock):
return "--------------------------------"
else:
raise ValueError(f"Unsupported message block type: {type(block)}")

def format_message_blocks(self, blocks: List[MessageBlock]) -> str:
if not blocks:
return ""
return "\n".join([self.format_message_block(block) for block in blocks])
Comment on lines +125 to +128
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify block separation behavior for text format.

The method joins message blocks with single newlines, which differs from the markdown formatter that uses double newlines (\n\n). Ensure this single newline separation provides adequate visual distinction between blocks in plain text output.


🏁 Script executed:

#!/bin/bash
# Check if there are any tests that verify the block separation behavior
ast-grep --pattern 'format_message_blocks($$$)' | head -10
rg -A 5 -B 5 'format_message_blocks.*join'

Length of output: 325


Add unit tests to verify single-newline separation in TextFormatter

No existing tests cover format_message_blocks in elementary/messages/formats/text.py. To ensure that joining blocks with a single \n still gives adequate visual separation (vs. the Markdown formatter’s \n\n), please:

  • Manually verify the plain-text output for a multi-block message.
  • Add a unit test (e.g. under tests/…) for TextFormatter.format_message_blocks, asserting:
    formatter = TextFormatter()
    blocks = [MessageBlock(text="A"), MessageBlock(text="B")]
    assert formatter.format_message_blocks(blocks) == "A\nB"
  • Update or extend any existing tests for other formatters to include a similar case for plain text.
🤖 Prompt for AI Agents
In elementary/messages/formats/text.py around lines 125 to 128, the method
format_message_blocks lacks unit tests verifying that multiple message blocks
are joined with a single newline. Add a new unit test in the appropriate test
file under tests/ that creates a TextFormatter instance, formats a list of
MessageBlock objects with texts "A" and "B", and asserts the output equals
"A\nB". Also, review and extend existing formatter tests to include similar
cases ensuring consistent plain-text formatting.


def format(self, message: MessageBody) -> str:
return self.format_message_blocks(message.blocks)


def format_text(
message: MessageBody,
icon_style: IconStyle = IconStyle.UNICODE,
table_style: TableStyle = TableStyle.TABULATE,
) -> str:
formatter = TextFormatter(icon_style, table_style)
return formatter.format(message)
2 changes: 1 addition & 1 deletion elementary/messages/message_body.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,4 @@ class MessageBody(BaseModel):
id: Optional[str] = None


MessageBody.update_forward_refs()
MessageBody.model_rebuild()
6 changes: 3 additions & 3 deletions tests/unit/alerts/alert_messages/test_alert_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from elementary.monitor.data_monitoring.alerts.integrations.utils.report_link import (
ReportLinkData as ReportLink,
)
from tests.unit.messages.utils import assert_expected_json, get_expected_json_path
from tests.unit.messages.utils import assert_expected_json, get_expected_file_path
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Silent creation of empty JSON fixtures can hide real mismatches

get_expected_file_path will happily create the file and pre-populate it with {} when the fixture is missing.
This causes two problems:

  1. A missing / miss-typed fixture passes CI once OVERRIDE=true, and
  2. The first local run overwrites the file with {}, making the diff in the subsequent assertion noisy and confusing.

Fail fast instead:

-from tests.unit.messages.utils import assert_expected_json, get_expected_file_path
+from tests.unit.messages.utils import assert_expected_json
+
+def _must_exist(path: Path) -> Path:
+    if not path.exists():
+        raise FileNotFoundError(f"Expected fixture is missing: {path}")
+    return path

and use _must_exist where you currently call get_expected_file_path.
This keeps the convenience helper for writers, while tests remain strict.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In tests/unit/alerts/alert_messages/test_alert_utils.py at line 26, replace
calls to get_expected_file_path with its _must_exist variant to prevent silent
creation of empty JSON fixture files. This change ensures that missing or
mistyped fixture files cause immediate test failures instead of creating empty
files, improving test strictness and avoiding confusing diffs. Update all
relevant test code to use _must_exist to enforce this fail-fast behavior while
retaining helper convenience.


FIXTURES_DIR = Path(__file__).resolve().parent / "fixtures"

Expand Down Expand Up @@ -320,7 +320,7 @@ def assert_expected_json_adaptive_card(
):
adaptive_card_filename = f"{filename}.json"
adaptive_card_json = format_adaptive_card(message_body)
expected_adaptive_card_json_path = get_expected_json_path(
expected_adaptive_card_json_path = get_expected_file_path(
FIXTURES_DIR / "adaptive_card", adaptive_card_filename
)
assert_expected_json(adaptive_card_json, expected_adaptive_card_json_path)
Expand All @@ -345,7 +345,7 @@ def assert_expected_json_block_kit(
os.environ["TEST_SLACK_CHANNEL"],
formatted_block_kit_message,
)
expected_block_kit_json_path = get_expected_json_path(
expected_block_kit_json_path = get_expected_file_path(
FIXTURES_DIR / "block_kit", block_kit_filename
)
assert_expected_json(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
from elementary.messages.formats.adaptive_cards import format_adaptive_card
from elementary.messages.message_body import MessageBody
from tests.unit.messages.formats.base_test_format import BaseTestFormat
from tests.unit.messages.utils import assert_expected_json, get_expected_json_path
from tests.unit.messages.utils import assert_expected_json, get_expected_file_path
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid shadowing by aliasing the imported helper

Importing get_expected_file_path and then defining a method with the same name may confuse readers and static-analysis tools. Alias the helper on import to keep the two clearly distinct.

-from tests.unit.messages.utils import assert_expected_json, get_expected_file_path
+from tests.unit.messages.utils import (
+    assert_expected_json,
+    get_expected_file_path as _get_expected_file_path,
+)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
from tests.unit.messages.utils import assert_expected_json, get_expected_file_path
from tests.unit.messages.utils import (
assert_expected_json,
get_expected_file_path as _get_expected_file_path,
)
🤖 Prompt for AI Agents
In tests/unit/messages/formats/adaptive_cards/test_adaptive_cards.py at line 20,
the imported helper function get_expected_file_path is shadowed by a local
method with the same name. To fix this, alias the imported
get_expected_file_path during import to a different name, such as
get_expected_file_path_helper, to clearly distinguish it from the local method
and avoid confusion.


FIXTURES_DIR = Path(__file__).resolve().parent / "fixtures"

Expand All @@ -44,7 +44,7 @@ def format(self, message_body: MessageBody) -> dict:
return format_adaptive_card(message_body)

def get_expected_file_path(self, name: str) -> str:
return get_expected_json_path(FIXTURES_DIR, f"{name}.json")
return get_expected_file_path(FIXTURES_DIR, f"{name}.json")

def assert_expected_value(self, result: dict, expected_file_path: Path) -> None:
assert_expected_json(result, expected_file_path)
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/messages/formats/block_kit/test_block_kit.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
SlackWebMessagingIntegration,
)
from tests.unit.messages.formats.base_test_format import BaseTestFormat
from tests.unit.messages.utils import assert_expected_json, get_expected_json_path
from tests.unit.messages.utils import assert_expected_json, get_expected_file_path

Comment on lines +13 to 14
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Same hidden-fixture pitfall as above

get_expected_file_path silently fabricates empty JSON files. Consider re-using the _must_exist helper (see previous comment) or switching to a stricter helper for tests.

🤖 Prompt for AI Agents
In tests/unit/messages/formats/block_kit/test_block_kit.py around lines 13 to
14, the use of get_expected_file_path can lead to silent creation of empty JSON
files, which is problematic. Replace get_expected_file_path with the _must_exist
helper or another stricter helper function that ensures the expected JSON files
actually exist, preventing silent fabrication and improving test reliability.

FIXTURES_DIR = Path(__file__).parent / "fixtures"

Expand All @@ -20,7 +20,7 @@ def format(self, message_body: MessageBody) -> FormattedBlockKitMessage:
return format_block_kit(message_body, resolve_mention=lambda x: "resolved_" + x)

def get_expected_file_path(self, name: str) -> Path:
return get_expected_json_path(FIXTURES_DIR, f"{name}.json")
return get_expected_file_path(FIXTURES_DIR, f"{name}.json")

def assert_expected_value(
self, result: FormattedBlockKitMessage, expected_file_path: Path
Expand Down
Empty file.
16 changes: 16 additions & 0 deletions tests/unit/messages/formats/markdown/fixtures/all_blocks_green.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Main Header

Normal text **Bold text** _Italic text_

- First bullet point
- Second bullet point

✅ Check item

Status: Passed | Tags: test, example

**Show Details**\
🔎 **Details Section**\
Here's some content with a [link](https://example.com)

user1 `select 1`
Loading