Feature: make the number of columns that are formatted as a table in Teams alerts a cli flag so that users have more control over the formatting in Teams #1958
Conversation
FrankTub
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
WalkthroughAdds a configurable maximum_columns_in_alert_samples option (config, CLI, builder), centralizes AlertMessageBuilder instantiation, updates Adaptive Card formatting and Teams webhook handling (rate limiting and stricter status checks), adds a .gitignore rule, and updates many Adaptive Card test fixtures. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as CLI
participant Config as Config
participant DMA as DataMonitoringAlerts
participant AMB as AlertMessageBuilder
participant MI as MessagingIntegration
CLI->>Config: create Config(maximum_columns_in_alert_samples)
CLI->>DMA: start DataMonitoringAlerts(config, max_cols)
DMA->>AMB: instantiate AlertMessageBuilder(config=MessageBuilderConfig{max_cols})
loop for each alert
DMA->>AMB: build(alert)
AMB-->>DMA: message (Adaptive Card / blocks)
DMA->>MI: send(message)
end
sequenceDiagram
autonumber
participant Sender as Alerts Sender
participant TeamsMI as TeamsWebhookMessagingIntegration
participant Teams as Teams Webhook
rect rgba(235,245,255,0.6)
note right of TeamsMI: Rate-limited (1 call / 1s)
end
Sender->>TeamsMI: send_message(payload)
TeamsMI->>Teams: POST webhook
Teams-->>TeamsMI: Response (status, body)
alt HTTPStatus.ACCEPTED (202) or OK (200) with empty body
TeamsMI-->>Sender: Success
else 200 with non-empty body or other status
TeamsMI-->>Sender: Raise MessagingIntegrationError(status, text)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (3)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
👋 @FrankTub |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
elementary/config/config.py (1)
74-74: LGTM! Consider adding validation for the parameter.The implementation correctly follows the existing pattern for config parameters. The parameter is optional with a sensible default value of 4, maintaining backward compatibility.
Consider adding validation to ensure the parameter is positive:
self.maximum_columns_in_alert_samples = self._first_not_none( maximum_columns_in_alert_samples, config.get("maximum_columns_in_alert_samples"), 4, ) + +if self.maximum_columns_in_alert_samples <= 0: + raise InvalidArgumentsError("maximum_columns_in_alert_samples must be positive")Also applies to: 107-112
elementary/messages/messaging_integrations/teams_webhook.py (1)
27-27: Consider a more descriptive constant name.While
ONE_SECONDis clear, considerONE_SECOND_IN_SECONDSorRATE_LIMIT_PERIOD_SECONDSto be more explicit about the unit and purpose.-ONE_SECOND = 1 +RATE_LIMIT_PERIOD_SECONDS = 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
elementary/config/config.py(2 hunks)elementary/messages/formats/adaptive_cards.py(3 hunks)elementary/messages/messaging_integrations/teams_webhook.py(4 hunks)elementary/monitor/alerts/alert_messages/builder.py(2 hunks)elementary/monitor/cli.py(3 hunks)elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (1)
elementary/monitor/alerts/alert_messages/builder.py (1)
AlertMessageBuilder(62-718)
elementary/messages/formats/adaptive_cards.py (1)
elementary/messages/blocks.py (1)
TableBlock(153-165)
elementary/messages/messaging_integrations/teams_webhook.py (5)
elementary/messages/messaging_integrations/base_messaging_integration.py (2)
send_message(35-40)MessageSendResult(19-22)elementary/messages/message_body.py (1)
MessageBody(36-39)elementary/messages/messaging_integrations/empty_message_context.py (1)
EmptyMessageContext(4-5)elementary/messages/formats/adaptive_cards.py (1)
format_adaptive_card(253-261)elementary/messages/messaging_integrations/exceptions.py (1)
MessagingIntegrationError(1-2)
🔇 Additional comments (8)
elementary/monitor/alerts/alert_messages/builder.py (1)
59-59: LGTM! The parameter addition is correct.The new parameter follows the existing pattern and provides the intended configurability for table formatting.
elementary/monitor/cli.py (1)
288-294: LGTM! The CLI option implementation is well-structured.The new option follows the existing patterns with proper typing, default value, and clear help text. The parameter is correctly passed through to the Config constructor.
Also applies to: 326-326, 358-358
elementary/messages/formats/adaptive_cards.py (3)
134-134: LGTM! Return type modernization is appropriate.The change from
Dict[str, Any]todictis a good modernization that maintains compatibility.
147-149: Excellent improvement to cell handling and formatting.The changes improve the user experience:
- Empty string for
Nonecells is better than displaying "None" text- Text wrapping prevents layout issues with long content
260-260: Good addition for Teams layout optimization.The
"msteams": {"width": "Full"}property ensures full width usage in Teams, which aligns with the PR objectives to improve Teams alert formatting.elementary/messages/messaging_integrations/teams_webhook.py (3)
2-2: Good practice: Using standard library constants and adding rate limiting.The addition of
HTTPStatusconstants and rate limiting decorators improves code clarity and prevents API rate limit issues.Also applies to: 6-6
48-51: Good improvement: Using HTTPStatus enum for clarity.Using
HTTPStatus.ACCEPTEDinstead of hardcoded 202 improves code readability and maintainability.
64-65: Good addition: Rate limiting prevents API throttling.The rate limiting decorators ensure the Teams webhook isn't called more than once per second, preventing potential rate limit violations.
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py
Outdated
Show resolved
Hide resolved
michael-myaskovsky
left a comment
michael-myaskovsky
left a comment
There was a problem hiding this comment.
Hi @FrankTub! Thank you for your contribution, I left some comments under coderabbit's comments, if you could take a look and explain/fix I would be glad to approve your contribution.
Thanks again!
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py
Outdated
Show resolved
Hide resolved
FrankTub
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
elementary/messages/messaging_integrations/teams_webhook.py (1)
42-46: Add request timeouts to avoid indefinite hangs.Without a timeout, transient network issues can block the run.
response = requests.post( webhook_url, json=payload, headers={"Content-Type": "application/json"}, + timeout=(3.05, 10), # (connect, read) timeouts in seconds )
♻️ Duplicate comments (1)
elementary/messages/messaging_integrations/teams_webhook.py (1)
92-93: Resolved: sensitive URL removed; network errors are now handled.This addresses prior concerns about leaking the webhook URL and uncaught RequestException. LGTM.
🧹 Nitpick comments (4)
elementary/messages/messaging_integrations/teams_webhook.py (4)
48-51: Clarify the 202 handling log message (nit).Log explicitly that body validation is skipped for 202 to aid debugging.
- logger.debug( - "Got %s response from Teams webhook, assuming success", HTTPStatus.ACCEPTED - ) + logger.debug( + "Got %s (Accepted) from Teams webhook; skipping body validation and assuming success", + HTTPStatus.ACCEPTED, + )
64-66: Global rate limit affects all instances; confirm intent and consider configurability.Decorators apply per function object, so this enforces a process-wide 1 req/sec across all TeamsWebhookMessagingIntegration instances. If multiple webhooks or parallel runs are expected, consider:
- Per-webhook rate limiting (instance-level throttle keyed by URL), or
- Making calls/period configurable via CLI/config to balance throughput vs. Teams’ limits.
Is the global 1 req/sec intended? If not, I can propose a per-instance throttle implementation.
27-27: Nit: Name conveys purpose better.ONE_SECOND is generic; RATE_LIMIT_PERIOD_SECONDS is clearer at the usage site.
-ONE_SECOND = 1 +RATE_LIMIT_PERIOD_SECONDS = 1Also update:
-@limits(calls=1, period=ONE_SECOND) +@limits(calls=1, period=RATE_LIMIT_PERIOD_SECONDS)
85-89: Prefer timezone-aware UTC timestamps.Naive datetimes can cause downstream ambiguity. Use UTC-aware timestamps.
-from datetime import datetime +from datetime import datetime, timezone ... - timestamp=datetime.utcnow(), + timestamp=datetime.now(timezone.utc),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
elementary/messages/messaging_integrations/teams_webhook.py(4 hunks)elementary/monitor/dbt_project/.gitignore(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- elementary/monitor/dbt_project/.gitignore
🧰 Additional context used
🧬 Code Graph Analysis (1)
elementary/messages/messaging_integrations/teams_webhook.py (4)
elementary/messages/messaging_integrations/base_messaging_integration.py (2)
send_message(35-40)MessageSendResult(19-22)elementary/messages/messaging_integrations/empty_message_context.py (1)
EmptyMessageContext(4-5)elementary/messages/formats/adaptive_cards.py (1)
format_adaptive_card(253-261)elementary/messages/messaging_integrations/exceptions.py (1)
MessagingIntegrationError(1-2)
🔇 Additional comments (2)
elementary/messages/messaging_integrations/teams_webhook.py (2)
2-2: Good use of HTTPStatus for readability and correctness.This improves clarity over magic numbers.
6-6: ratelimit dependency is declared and up to dateThe project’s
pyproject.tomllistsratelimit = "*"under[tool.poetry.dependencies], so the import inteams_webhook.pyis covered and will not cause a runtimeImportError. No further action is needed.
| response = send_adaptive_card(self.url, card) | ||
| # The status code is not reliable for the older version and 202 for the newer version | ||
| # The response returns 1 in the body for the old version for success and otherwise some text | ||
| # The new version is not reliable to do a check and in the response there is nothing that can be used | ||
| # to determine if the message was sent successfully. | ||
| if ( | ||
| response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) | ||
| or len(response.text) > 1 | ||
| ): | ||
| raise MessagingIntegrationError( | ||
| f"Could not post message to Teams via webhook. Status code: {response.status_code}, Error: {response.text}" | ||
| ) |
There was a problem hiding this comment.
Tighten response validation: only check body for 200 OK and strip whitespace.
Per the comments, the body content is meaningful only for the older 200 variant ("1" indicates success) and is not reliable for the newer 202 variant. The current condition still treats any non-empty body with 202 as failure. Also, consider stripping whitespace.
try:
- response = send_adaptive_card(self.url, card)
+ response = send_adaptive_card(self.url, card)
# The status code is not reliable for the older version and 202 for the newer version
# The response returns 1 in the body for the old version for success and otherwise some text
# The new version is not reliable to do a check and in the response there is nothing that can be used
# to determine if the message was sent successfully.
- if (
- response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED)
- or len(response.text) > 1
- ):
+ text = response.text.strip()
+ if (
+ response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED)
+ or (response.status_code == HTTPStatus.OK and text != "1")
+ ):
raise MessagingIntegrationError(
f"Could not post message to Teams webhook. Status code: {response.status_code}, Error: {response.text}"
)📝 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.
| response = send_adaptive_card(self.url, card) | |
| # The status code is not reliable for the older version and 202 for the newer version | |
| # The response returns 1 in the body for the old version for success and otherwise some text | |
| # The new version is not reliable to do a check and in the response there is nothing that can be used | |
| # to determine if the message was sent successfully. | |
| if ( | |
| response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) | |
| or len(response.text) > 1 | |
| ): | |
| raise MessagingIntegrationError( | |
| f"Could not post message to Teams via webhook. Status code: {response.status_code}, Error: {response.text}" | |
| ) | |
| response = send_adaptive_card(self.url, card) | |
| # The status code is not reliable for the older version and 202 for the newer version | |
| # The response returns 1 in the body for the old version for success and otherwise some text | |
| # The new version is not reliable to do a check and in the response there is nothing that can be used | |
| # to determine if the message was sent successfully. | |
| text = response.text.strip() | |
| if ( | |
| response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) | |
| or (response.status_code == HTTPStatus.OK and text != "1") | |
| ): | |
| raise MessagingIntegrationError( | |
| f"Could not post message to Teams webhook. Status code: {response.status_code}, Error: {response.text}" | |
| ) |
🤖 Prompt for AI Agents
In elementary/messages/messaging_integrations/teams_webhook.py around lines 73
to 84, the response validation treats any non-empty body as failure even for 202
responses; update the logic to only inspect the response body when status_code
== HTTPStatus.OK (200) and use response.text.strip() when checking for the old
"1" success marker, allow HTTPStatus.ACCEPTED (202) to succeed regardless of
body, and raise MessagingIntegrationError for any other status codes or when a
200 response does not have a stripped body equal to "1".
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (1)
19-22: Correct imports for AlertMessageBuilder and MessageBuilderConfig (resolves earlier type mismatch).This aligns with the builder’s constructor signature and fixes the previously noted issue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (2)
elementary/monitor/cli.py (1)
monitor(296-403)elementary/monitor/alerts/alert_messages/builder.py (2)
AlertMessageBuilder(62-718)MessageBuilderConfig(57-59)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py
Outdated
Show resolved
Hide resolved
FrankTub
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
|
@michael-myaskovsky I think I addressed all your comments. I will highlight one piece of code which I just don't get how this ever worked in this setup. |
|
Great! Please update from main and then I'll run the tests |
michael-myaskovsky
had a problem deploying
to
elementary_test_env
GitHub Actions
Failure
FrankTub
force-pushed
the
feature/variable-number-of-columns-allowed
branch
from
cf07a33 to
3e2f91f
Compare
FrankTub
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
elementary/messages/messaging_integrations/teams_webhook.py (2)
90-93: Broaden exception message; satisfy Ruff TRY003/RUF010.
RequestException isn’t always a “network” error. Keep details via exception chaining and avoid explicit str().- raise MessagingIntegrationError( - f"Network error while posting message to Teams webhook: {str(e)}" - ) from e + raise MessagingIntegrationError( + "Request error while posting message to Teams webhook." + ) from e
73-84: Fix false negatives: 202 with any body is treated as failure.
Per your own comment, only the 200 variant uses body=="1" as success. Strip whitespace and check body only for 200; allow 202 regardless of body.- if ( - response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) - or len(response.text) > 1 - ): + text = response.text.strip() + if ( + response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) + or (response.status_code == HTTPStatus.OK and text != "1") + ): raise MessagingIntegrationError( f"Could not post message to Teams via webhook. Status code: {response.status_code}, Error: {response.text}" )Optional: add a short client timeout to requests.post to avoid hangs (e.g., timeout=(3.05, 10)).
🧹 Nitpick comments (3)
elementary/messages/messaging_integrations/teams_webhook.py (3)
48-51: Tweak log to show actual status code; keep parameterized logging.
Log the numeric code instead of the enum constant. Parameterized logging avoids needless string formatting.- logger.debug( - "Got %s response from Teams webhook, assuming success", HTTPStatus.ACCEPTED - ) + logger.debug( + "Got %d response from Teams webhook, assuming success", response.status_code + )
64-66: Rate limit scope — confirm 1 call/sec per-process is intended here.
If you need a central throttle for all Teams sends, consider moving the decorator to a shared sender or configuring limits at a higher layer.
66-70: Silence ARG002 by marking destination unused.
Base signature likely requires it; rename to underscore.def send_message( self, - destination: None, + _destination: None, body: MessageBody, ) -> MessageSendResult[EmptyMessageContext]:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
elementary/config/config.py(2 hunks)elementary/messages/formats/adaptive_cards.py(3 hunks)elementary/messages/messaging_integrations/teams_webhook.py(4 hunks)elementary/monitor/alerts/alert_messages/builder.py(2 hunks)elementary/monitor/cli.py(3 hunks)elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py(3 hunks)elementary/monitor/dbt_project/.gitignore(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (6)
- elementary/monitor/alerts/alert_messages/builder.py
- elementary/monitor/dbt_project/.gitignore
- elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py
- elementary/monitor/cli.py
- elementary/config/config.py
- elementary/messages/formats/adaptive_cards.py
🧰 Additional context used
🪛 Ruff (0.12.2)
elementary/messages/messaging_integrations/teams_webhook.py
68-68: Unused method argument: destination
(ARG002)
82-84: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (2)
elementary/messages/messaging_integrations/teams_webhook.py (2)
2-2: Importing HTTPStatus improves clarity — LGTM.
27-27: ONE_SECOND constant — clear and self-documenting.
| from typing import Any, Optional | ||
|
|
||
| import requests | ||
| from ratelimit import limits, sleep_and_retry |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Rate limiting import added — verify dependency is declared/pinned.
Ensure ratelimit is listed in your dependency files (and pinned) to avoid runtime ImportError.
🏁 Script executed:
#!/bin/bash
# Verify ratelimit is declared in common dependency files
fd -a '(^requirements.*\.txt$|^pyproject\.toml$|^setup\.(cfg|py)$)' -HI \
| xargs -I{} sh -c 'echo "--- {} ---"; rg -n "ratelimit" "{}" || true'Length of output: 207
Pin ratelimit dependency to a specific version
Replace ratelimit = "*" in pyproject.toml (line 22) with a fixed version specifier (e.g. ratelimit = "==1.0.0") to ensure reproducible builds.
🤖 Prompt for AI Agents
In elementary/messages/messaging_integrations/teams_webhook.py around line 6,
the project currently depends on a non-pinned ratelimit version; update
pyproject.toml (line 22) to replace ratelimit = "*" with a specific version pin
such as ratelimit = "==1.0.0" (or the tested working version), then run your
dependency lock/update command (poetry lock or pip-compile) and install to
refresh the lockfile and verify imports and tests still pass.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
elementary/messages/messaging_integrations/teams_webhook.py (1)
42-47: Add HTTP timeouts to external call.
Prevent indefinite hangs on webhook POSTs.- response = requests.post( + # (connect, read) seconds + REQUESTS_TIMEOUT = (5, 15) + response = requests.post( webhook_url, json=payload, headers={"Content-Type": "application/json"}, + timeout=REQUESTS_TIMEOUT, )If you prefer, define REQUESTS_TIMEOUT once near ONE_SECOND and reuse.
♻️ Duplicate comments (2)
elementary/messages/messaging_integrations/teams_webhook.py (2)
6-6: Pin ratelimit dependency to avoid runtime drift.
Make sure ratelimit is pinned in your deps (pyproject/requirements).#!/bin/bash # Verify ratelimit is declared and pinned fd -a '(^requirements.*\.txt$|^pyproject\.toml$|^setup\.(cfg|py)$)' -HI \ | while read f; do echo "--- $f ---"; rg -n "ratelimit" "$f" || true; done
73-82: Tighten 200-OK body check: strip whitespace and require '1'.
Old Teams returns "1" (possibly with whitespace). Current code treats " 1 " as failure. Only inspect body for 200; accept 202 regardless of body.- if response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) or ( - response.status_code == HTTPStatus.OK and len(response.text) > 1 - ): + text = response.text.strip() + if ( + response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) + or (response.status_code == HTTPStatus.OK and text != "1") + ): raise MessagingIntegrationError( f"Could not post message to Teams via webhook. Status code: {response.status_code}, Error: {response.text}" )
🧹 Nitpick comments (4)
elementary/messages/messaging_integrations/teams_webhook.py (4)
48-51: Nit: log numeric code instead of enum repr.
Slightly clearer logs.- logger.debug( - f"Got {HTTPStatus.ACCEPTED} response from Teams webhook, assuming success" - ) + logger.debug( + "Got %s (%d) response from Teams webhook, assuming success", + HTTPStatus.ACCEPTED.phrase, + HTTPStatus.ACCEPTED.value, + )
66-70: Silence unuseddestinationarg to satisfy linters.
Keep signature for compatibility, but mark as used.def send_message( self, destination: None, body: MessageBody, ) -> MessageSendResult[EmptyMessageContext]: + _ = destination # unused (ARG002) card = format_adaptive_card(body)
83-85: Optional: shorten exception message (TRY003).
Keep essential details; the response body may be very long.- raise MessagingIntegrationError( - f"Could not post message to Teams via webhook. Status code: {response.status_code}, Error: {response.text}" - ) + raise MessagingIntegrationError( + f"Teams webhook post failed. status={response.status_code}" + )If detailed text is needed, log it at debug level instead of embedding in the exception string.
91-94: Use explicit conversion flag in f-string (RUF010).
Avoid calling str() inside f-strings.- raise MessagingIntegrationError( - f"An error occurred while posting message to Teams webhook: {str(e)}" - ) from e + raise MessagingIntegrationError( + f"An error occurred while posting message to Teams webhook: {e!s}" + ) from e
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
elementary/messages/formats/adaptive_cards.py(2 hunks)elementary/messages/messaging_integrations/teams_webhook.py(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- elementary/messages/formats/adaptive_cards.py
🧰 Additional context used
🧬 Code graph analysis (1)
elementary/messages/messaging_integrations/teams_webhook.py (6)
elementary/messages/messaging_integrations/base_messaging_integration.py (2)
send_message(35-40)MessageSendResult(19-22)elementary/messages/messaging_integrations/file_system.py (1)
send_message(46-77)elementary/messages/messaging_integrations/slack_webhook.py (1)
send_message(61-70)elementary/messages/messaging_integrations/empty_message_context.py (1)
EmptyMessageContext(4-5)elementary/messages/formats/adaptive_cards.py (1)
format_adaptive_card(253-261)elementary/messages/messaging_integrations/exceptions.py (1)
MessagingIntegrationError(1-2)
🪛 Ruff (0.12.2)
elementary/messages/messaging_integrations/teams_webhook.py
68-68: Unused method argument: destination
(ARG002)
83-85: Avoid specifying long messages outside the exception class
(TRY003)
93-93: Use explicit conversion flag
Replace with conversion flag
(RUF010)
🔇 Additional comments (3)
elementary/messages/messaging_integrations/teams_webhook.py (3)
2-2: LGTM: explicit HTTPStatus import.
Clearer than magic numbers.
27-27: LGTM: rate-limit period constant.
Readable and self-explanatory.
64-65: LGTM: decorator order for ratelimit.
@sleep_and_retry wrapping @limits is the correct pattern.
FrankTub
had a problem deploying
to
elementary_test_env
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (21)
tests/unit/messages/formats/adaptive_cards/fixtures/all_icons.json (1)
16-19: Confirm Teams width casing and cross-fixture consistency.
Microsoft samples often show msteams.width as "full" (lowercase). You’re using "Full". Ensure generator and all fixtures use the same casing and add an assertion that the emitted card includes msteams.width with the expected value.tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-error_link-True_tags-True_owners-False_path-False_suppression-True_materialization-view_full_refresh-True_env-True.json (1)
156-159: Double-check casing and scope for msteams.width.Teams examples vary between "Full" and "full". Ensure the runtime generator and all fixtures use the same casing and that this field is only emitted for Teams (not generic Adaptive Cards), to avoid flaky snapshot diffs.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-False_message-True_tags-True_owners-False_path-True_suppression-False_env-False.json (1)
113-116: Avoid fixture churn by centralizing common metadataIf many snapshots now include the same msteams block, consider generating snapshots via a helper/factory that injects this metadata, or adjust snapshot normalization to ignore field order.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-table.json (1)
205-208: Stabilize fixtures against key-order brittleness (optional)If tests diff raw JSON, minor reordering could cause flakes. Consider parsing and comparing dicts or normalizing key order when asserting.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_anomaly-True_env-True.json (1)
174-177: Teams width casing and client supportGood addition; root-level msteams is the right place. Please verify casing: Microsoft docs/examples typically show "full" (lowercase). If classic/new Teams accept only lowercase, standardize to "full" to avoid brittle tests across clients.
If needed:
- "msteams": { - "width": "Full" - } + "msteams": { + "width": "full" + }Also confirm all target tenants support AdaptiveCard "version": "1.5"; if not, consider falling back to 1.4 in fixtures for compatibility.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-False_message-True_tags-True_owners-False_path-False_error-True_suppression-True_env-True.json (1)
172-175: Reduce fixture churn with an assertion-based testOptional: add a focused unit test that asserts the presence/value of
msteams.widthon produced cards, so future changes don’t require mass fixture edits.tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-fail_link-False_tags-False_owners-False_path-False_suppression-False_materialization-table_full_refresh-False_env-False.json (1)
105-108: Gate Teams-specific extension in builder (if not already done)Ensure the code only injects the "msteams" object for Teams targets to avoid leaking channel-specific fields to other renderers.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-False_test-warnings-True_test-errors-True_link-False_env-False_subscribers-True.json (1)
151-154: Broaden tests to cover the new column-count flag behavior.This fixture exercises full-width rendering but not the variable max-columns logic. Add/extend fixtures with table blocks to assert:
- default column cap
- overridden cap via CLI (-mc)
- wrapping of long cell text
Happy to draft table-based fixtures and corresponding assertions.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-False_message-False_tags-False_owners-False_path-False_suppression-False_env-False.json (1)
29-32: Avoid brittle string-based snapshot comparisons.If snapshots compare raw JSON strings, ordering/whitespace changes can cause false diffs. Prefer structure-based assertions (parse JSON and compare dicts) in tests.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-True_test-errors-True_link-True_env-True.json (1)
240-243: Optional: Add schema validation to guard fixturesConsider a lightweight test that validates the produced Adaptive Card against the 1.5 schema and ensures msteams is only present for Teams paths. This prevents drift across many duplicated fixtures.
I can provide a small jsonschema-based validator and a parametric test to assert presence/absence of msteams per channel.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_anomaly-False_env-False.json (1)
29-32: Consistent addition of msteams.width is good.Matches the generator change and should render full width in Teams. Consider a focused assertion test on presence/value of msteams.width to reduce fixture churn in future.
Example (new test):
def test_adaptive_card_has_full_width_for_teams(make_card): card = make_card(...) # build minimal card via the generator assert card.get("msteams", {}).get("width") == "Full"tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json (1)
301-304: Optional: reduce fixture churn by centralizing the Teams extensionTo avoid touching many fixtures when Teams-wide settings change, consider asserting core card content separately and normalizing the root with a helper (e.g., a test factory that injects
msteams.widthbefore snapshot comparison).tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-error_link-False_message-True_tags-True_owners-False_path-False_suppression-True_env-False.json (1)
109-112: Optional: gate Teams-specific metadata behind channel-specific buildersIf the same AdaptiveCard objects are ever serialized for other destinations (email, web previews, Slack), consider adding
msteamsonly in the Teams path to avoid channel-specific leakage into generic fixtures.tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-error_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json (1)
186-189: Optional: include $schema for clearer validation (non-blocking).Consider adding the Adaptive Card schema URI at the root to aid tooling/validation:
Example (top of the card):
{ "$schema": "http://adaptivecards.io/schemas/adaptive-card.json", "type": "AdaptiveCard", "version": "1.5", "msteams": { "width": "Full" }, ... }tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-False_test-warnings-True_test-errors-True_link-True_env-True_subscribers-True.json (1)
212-215: Consistent host-specific metadata — looks goodAdding msteams.width keeps Teams rendering deterministic without affecting other hosts.
Consider asserting presence of msteams.width explicitly in tests (vs. full-file snapshot) to reduce fixture churn from non-semantic field ordering.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-False_test-errors-True_link-False_env-False.json (1)
179-182: Stabilize snapshots against key-order churn.Since you added a new root key, consider asserting via a schema/normalized comparison (e.g., sort keys or schema-validate) rather than strict text snapshots to reduce brittle diffs across fixtures.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-False_message-False_tags-True_owners-True_path-True_error-True_suppression-False_env-False.json (1)
126-129: Consider adding schema validation for fixtures.Add a JSON Schema check for AdaptiveCard 1.5 in unit tests to catch structural issues early (including new msteams hints) and reduce reliance on large golden files.
tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_yellow.json (1)
97-100: LGTM: Consistent msteams width hint.Matches other fixtures and expected builder output.
If fixture churn becomes frequent, consider generating expected cards via a small helper using the same builder (then snapshot) to avoid repeating root metadata in static JSON.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_4.json (1)
276-279: LGTM: Full-width hint for 4-column table.Looks correct and consistent.
Do you want a minimal unit test asserting that the new CLI flag (e.g., -mc 3) caps rendered columns for Teams cards while keeping width Full? I can draft it.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_5.json (1)
333-336: Teams width metadata added — confirm cell wrapping expectation.If format_table_block now sets TextBlock.wrap: true, update this fixture to reflect it; otherwise, tests may drift from runtime output.
- "type": "TextBlock", - "text": "Column 0" + "type": "TextBlock", + "text": "Column 0", + "wrap": true(Apply similarly to other cell TextBlocks if applicable.)
tests/unit/messages/formats/adaptive_cards/fixtures/divider_blocks.json (1)
43-46: Consistent Teams metadata; keep test churn low.Change is fine. Consider asserting the presence/value of msteams via schema/partial match to avoid broad fixture churn in future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (107)
tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-False_test-warnings-True_test-errors-True_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-False_test-errors-False_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-False_test-errors-True_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-False_test-warnings-False_test-errors-True_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-False_test-warnings-True_test-errors-False_link-True_env-False_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-False_test-warnings-True_test-errors-True_link-True_env-True_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-False_test-errors-True_link-True_env-True_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-True_test-errors-False_link-True_env-True_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-column.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-description.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-owners.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-result_message.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-subscribers.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-table.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-tags.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-test_parameters.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-test_query.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-test_results_sample.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-None_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-None_link-True_description-True_tags-False_owners-False_table-True_error-True_sample-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-error_link-False_description-True_tags-False_owners-True_table-False_error-True_sample-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-error_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_anomaly-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-False_description-True_tags-False_owners-True_table-False_error-True_sample-False_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-error_link-False_description-True_tags-True_owners-False_table-True_error-False_sample-True_anomaly-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-error_link-True_description-True_tags-False_owners-False_table-False_error-True_sample-False_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-fail_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_anomaly-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-warn_link-False_description-True_tags-False_owners-True_table-False_error-True_sample-False_anomaly-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-warn_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_anomaly-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-False_test-warnings-True_test-errors-True_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-False_test-errors-False_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-False_test-errors-True_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-False_test-errors-True_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-True_test-errors-False_link-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-False_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-True_test-errors-False_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-None_link-False_tags-False_owners-True_path-False_suppression-True_materialization-table_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-None_link-True_tags-False_owners-True_path-True_suppression-True_materialization-table_full_refresh-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-error_link-False_tags-True_owners-True_path-True_suppression-False_materialization-incremental_full_refresh-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-error_link-True_tags-True_owners-False_path-False_suppression-True_materialization-view_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-fail_link-False_tags-False_owners-False_path-False_suppression-False_materialization-table_full_refresh-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-fail_link-True_tags-False_owners-False_path-True_suppression-False_materialization-incremental_full_refresh-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-fail_link-True_tags-True_owners-True_path-True_suppression-True_materialization-table_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-warn_link-False_tags-True_owners-False_path-True_suppression-True_materialization-incremental_full_refresh-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-warn_link-False_tags-True_owners-True_path-True_suppression-True_materialization-view_full_refresh-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-warn_link-True_tags-True_owners-True_path-False_suppression-True_materialization-view_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-None_link-False_message-False_tags-False_owners-True_path-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-None_link-True_message-False_tags-True_owners-False_path-False_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-error_link-False_message-True_tags-True_owners-False_path-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-error_link-True_message-True_tags-False_owners-True_path-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-False_message-False_tags-False_owners-False_path-False_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-True_message-False_tags-True_owners-True_path-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-True_message-True_tags-True_owners-True_path-True_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-False_message-True_tags-False_owners-True_path-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-False_message-True_tags-True_owners-False_path-True_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-True_message-False_tags-True_owners-False_path-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-False_message-False_tags-True_owners-True_path-True_error-True_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-False_message-True_tags-True_owners-True_path-False_error-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-False_tags-False_owners-False_path-False_error-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-False_tags-True_owners-True_path-True_error-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-True_tags-False_owners-False_path-True_error-True_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-False_message-False_tags-False_owners-False_path-False_error-False_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-False_message-True_tags-True_owners-False_path-False_error-True_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-True_message-False_tags-True_owners-False_path-True_error-False_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-True_message-True_tags-False_owners-True_path-False_error-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-True_message-True_tags-True_owners-True_path-True_error-True_suppression-True_env-True.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_green.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_no_color.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_red.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_yellow.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_icons.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/bullet_list.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/code_block_200.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/code_block_50.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/code_block_500.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/colored_header.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/divider_blocks.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/expandable_block.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/fact_list.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/nested_expandable.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/simple_header.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_1.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_2.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_3.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_4.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_5.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_1.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_2.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_3.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_4.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_5.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_1.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_2.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_3.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_4.json(1 hunks)
⛔ Files not processed due to max files limit (7)
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_5.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_1000.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_200.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_50.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_500.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_styles.json
- tests/unit/messages/formats/adaptive_cards/fixtures/whitespace_block.json
✅ Files skipped from review due to trivial changes (13)
- tests/unit/messages/formats/adaptive_cards/fixtures/code_block_200.json
- tests/unit/messages/formats/adaptive_cards/fixtures/fact_list.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-False_test-errors-True_link-True_env-True.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-False_test-errors-False_link-False_env-False.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_3.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-description.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-False_test-errors-True_link-True_env-True_subscribers-False.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-False_test-errors-True_link-False_env-False.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True_subscribers-False.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-None_link-False_tags-False_owners-True_path-False_suppression-True_materialization-table_full_refresh-True_env-True.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_4.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-error_link-False_description-True_tags-True_owners-False_table-True_error-False_sample-True_anomaly-False_env-True.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-False_tags-True_owners-True_path-True_error-False_suppression-True_env-False.json
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM: Teams full-width hint added correctly.
The top-level msteams.width="Full" is valid for Teams and the JSON remains well-formed after the version comma.
Optional: verify all AdaptiveCard fixtures include the new msteams hint when version=1.5.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Lists JSON fixtures under tests/unit that declare version 1.5 but lack the Teams width hint.
missing=()
while IFS= read -r -d '' f; do
if rg -q '"version":\s*"1\.5"' "$f"; then
if ! rg -q '"msteams":\s*\{\s*"width":\s*"Full"\s*\}' "$f"; then
missing+=("$f")
fi
fi
done < <(fd -t f -e json tests/unit | tr '\n' '\0')
printf "%s\n" "${missing[@]:-}" | sortLength of output: 534
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Lists JSON fixtures under tests/unit that declare version 1.5 but lack the Teams width hint.
missing=()
while IFS= read -r -d '' f; do
if grep -q '"version":\s*"1\.5"' "$f"; then
if ! grep -q '"msteams":\s*{[^}]*"width":\s*"Full"' "$f"; then
missing+=("$f")
fi
fi
done < <(find tests/unit -type f -name '*.json' -print0)
printf "%s\n" "${missing[@]:-}" | sortLength of output: 192
#!/bin/bash
set -euo pipefail
# Find all JSON fixtures under tests/unit and list those that have version "1.5" but lack the msteams width hint
find tests/unit -type f -name '*.json' -exec sh -c '
for f; do
if grep -Eq '"version":[[:space:]]*"1\.5"' "$f"; then
if ! grep -Eq '"msteams":[[:space:]]*{[^}]*"width":[[:space:]]*"Full"' "$f"; then
echo "$f"
fi
fi
done
' sh {} +🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Find JSON fixtures under tests/unit with version "1.5" missing msteams.width="Full"
find tests/unit -type f -name '*.json' -exec sh -c '
f="$1"
if grep -Eq "\"version\":[[:space:]]*\"1\.5\"" "$f" && \
! grep -Eq "\"msteams\":[[:space:]]*{[^}]*\"width\":[[:space:]]*\"Full\"" "$f"; then
echo "$f"
fi
' _ {} \;Length of output: 16609
Ensure all version “1.5” AdaptiveCard fixtures include the Teams width hint
Search across tests/unit JSON fixtures found numerous files declaring "version": "1.5" but missing the top-level
"msteams": { "width": "Full" }block. Add this hint to every AdaptiveCard fixture with version 1.5.
🤖 Prompt for AI Agents
In
tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-False_test-errors-True_link-False_env-False_subscribers-True.json
around lines 151-154, the AdaptiveCard fixture declares "version": "1.5" but is
missing the required top-level "msteams": { "width": "Full" } hint; add that
exact block as a sibling to "version" (i.e., immediately adjacent at the top
level of the JSON object) so every version "1.5" fixture includes the Teams
width hint and ensure JSON remains valid (commas, ordering).
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Teams full-width metadata looks correct.
Adding msteams: { "width": "Full" } aligns with Microsoft’s guidance for rendering Adaptive Cards at full width in Teams. Looks good. (github.com, learn.microsoft.com)
Run to ensure all Adaptive Card fixtures were updated consistently and no lowercase "full" slipped in:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Report any Adaptive Card fixtures missing the Teams width metadata
while IFS= read -r -d '' f; do
if ! rg -q -nP '"msteams"\s*:\s*\{\s*"width"\s*:\s*"Full"' "$f"; then
echo "Missing msteams width Full: $f"
fi
done < <(fd -a -e json adaptive_card tests -0)
# 2) Flag any inconsistent casing
rg -nP '"width"\s*:\s*"full"' tests -g '**/adaptive_card/**/*.json' || trueLength of output: 168
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# 1) Ensure all Adaptive Card fixtures include msteams width "Full"
find tests -type f -path '*/adaptive_card/*.json' -name '*.json' -print0 | \
while IFS= read -r -d '' f; do
if ! rg -q -nP '"msteams"\s*:\s*\{\s*"width"\s*:\s*"Full"' "$f"; then
echo "Missing msteams width Full: $f"
fi
done
# 2) Flag any fixtures using lowercase "full" for casing consistency
rg -nP '"width"\s*:\s*"full"' -g 'tests/unit/alerts/alert_messages/fixtures/adaptive_card/**/*.json' || trueLength of output: 52787
Add missing Teams width metadata to all Adaptive Card fixtures
Script output indicates numerous JSON fixtures under tests/unit/alerts/alert_messages/fixtures/adaptive_card are missing the
"msteams": { "width": "Full" }property. Please update each fixture to include this metadata for consistent full-width rendering in Teams.
🤖 Prompt for AI Agents
In
tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-False.json
around lines 186-189, the fixture is missing the Teams width metadata; add the
property "msteams": { "width": "Full" } at the top-level of the Adaptive Card
JSON (alongside "version" and other root keys) so the file includes the exact
metadata object for full-width rendering in Teams.
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
msteams width addition looks correct; verify generator parity.
The new root msteams.width = "Full" matches the PR intent for Teams full-width rendering.
Run to confirm all AdaptiveCard fixtures and the generator now agree on this field:
🏁 Script executed:
#!/bin/bash
rg -nP '"version":\s*"1\.5",\s*\n\s*"msteams":\s*\{\s*"width":\s*"Full"' tests/unit | wc -l
rg -n '"msteams":\s*\{\s*"width":\s*"Full"' elementary/messages/formats/adaptive_cards.pyLength of output: 229
Regenerate AdaptiveCard fixtures to include msteams.width = "Full"
No tests/unit JSON fixtures contain the new "msteams": { "width": "Full" } alongside version "1.5" (0 matches found), yet the generator emits it at elementary/messages/formats/adaptive_cards.py:260; update all fixtures accordingly.
🤖 Prompt for AI Agents
In
tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-True_test-errors-False_link-True_env-True.json
around lines 240 to 243, the fixture is missing the new "msteams": { "width":
"Full" } entry for Adaptive Card version "1.5"; regenerate or update all unit
JSON fixtures emitted by the generator
(elementary/messages/formats/adaptive_cards.py:260) so that whenever "version":
"1.5" is present the fixture includes "msteams": { "width": "Full" }, then
commit the updated fixtures to keep tests in sync.
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
LGTM: Teams host config added correctly.
Adding msteams: { "width": "Full" } at the root aligns with the code changes and Teams’ host-specific properties; the trailing comma after "version": "1.5" is correct for JSON syntax with the new field.
Run to ensure all Adaptive Card fixtures were updated consistently:
🏁 Script executed:
#!/bin/bash
# Expect at least one match in each fixtures directory
rg -nP '"msteams"\s*:\s*\{\s*"width"\s*:\s*"Full"\s*\}' tests/unit/alerts/alert_messages/fixtures/adaptive_card
rg -nP '"version"\s*:\s*"1\.5"' tests/unit/alerts/alert_messages/fixtures/adaptive_cardLength of output: 15783
Add msteams host config to every Adaptive Card fixture
Every JSON under tests/unit/alerts/alert_messages/fixtures/adaptive_card must include
"msteams": {
"width": "Full"
}immediately after "version": "1.5". Run
rg -nP '"msteams"\s*:\s*\{\s*"width"\s*:\s*"Full"\s*\}' tests/unit/alerts/alert_messages/fixtures/adaptive_cardto confirm coverage.
🤖 Prompt for AI Agents
In
tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-error_link-False_tags-True_owners-True_path-True_suppression-False_materialization-incremental_full_refresh-True_env-False.json
around lines 151-154, the file is missing the required msteams host config;
insert the following JSON object immediately after the line containing
"version": "1.5": an "msteams" object with a single property "width" set to
"Full" (ensure proper comma placement and JSON formatting), then run the
suggested ripgrep command to verify all adaptive_card fixtures include this
exact block.
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify channel compatibility and fixture consistency.
Confirm: (1) Teams supports the chosen Adaptive Card version in your tenant, (2) all Adaptive Card fixtures were updated consistently, and (3) generators always emit this field only for Teams cards (or that other channels safely ignore it).
Run:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Validate all Adaptive Card fixtures are well-formed JSON
fd -e json 'adaptive_card' tests | xargs -n1 -I{} sh -c 'jq . >/dev/null "{}" || echo "Invalid JSON: {}"'
# 2) Check all fixtures include the Teams width metadata
fd -e json 'adaptive_card' tests | xargs rg -nP '"msteams"\s*:\s*\{\s*"width"\s*:\s*"Full"'
# 3) Ensure card version is consistently 1.5 in fixtures
fd -e json 'adaptive_card' tests | xargs rg -nP '"version"\s*:\s*"1\.5"'
# 4) Verify generator emits msteams width (and locate call sites)
rg -nC2 -g '!**/tests/**' -P '\bmsteams\b|\bwidth"\s*:\s*"Full"' || trueLength of output: 19933
Ensure Teams metadata consistency across fixtures and generator
- Add
"msteams": {"width": "Full"}to every Adaptive Card fixture alongside"version": "1.5"(e.g. all files under tests/unit/**/adaptive_card/fixtures). - In elementary/messages/formats/adaptive_cards.py (around lines 258–261), emit the
msteamsproperty only for Teams cards (or verify non-Teams channels safely ignore it). - Confirm your Teams tenant supports Adaptive Card version 1.5.
🤖 Prompt for AI Agents
In
tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-False_message-False_tags-False_owners-False_path-False_suppression-False_env-False.json
around lines 29–32, the fixture has "version": "1.5" but you must ensure every
Adaptive Card fixture also includes "msteams": {"width": "Full"} for Teams
consistency; update all fixtures under tests/unit/**/adaptive_card/fixtures
accordingly. In elementary/messages/formats/adaptive_cards.py around lines
258–261, change the generator so it only emits the msteams property when
producing Teams cards (or guard it so non-Teams channels safely ignore it) to
avoid leaking Teams-specific metadata into other channel formats. Also verify
your Teams tenant supports Adaptive Card 1.5 before finalizing.
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Teams width metadata added — LGTM
msteams.width: "Full" and version "1.5" look correct for Teams full-width rendering.
Validate all updated fixtures are well-formed JSON and consistently include the msteams block:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Validate JSON
fd -e json 'tests/unit' | xargs -I{} python -m json.tool {} >/dev/null
# 2) List AdaptiveCard fixtures missing the msteams.width block (should print nothing)
rg -nP '"type"\s*:\s*"AdaptiveCard"' tests/unit \
| awk -F: '{print $1}' | sort -u \
| xargs -I{} bash -c 'rg -q "\"msteams\"\\s*:\\s*\\{\\s*\"width\"\\s*:\\s*\"Full\"" "{}" || echo "Missing msteams.width: {}"'Length of output: 429
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Validate all JSON fixtures under tests/unit
fd . tests/unit -e json | xargs -I{} python -m json.tool {} >/dev/null
# 2) List AdaptiveCard fixtures missing the msteams.width block
rg -nP '"type"\s*:\s*"AdaptiveCard"' tests/unit --glob '*.json' \
| awk -F: '{print $1}' | sort -u \
| xargs -I{} bash -c 'rg -q "\"msteams\"\\s*:\\s*\\{\\s*\"width\"\\s*:\\s*\"Full\"" "{}" || echo "Missing msteams.width: {}"'Length of output: 19279
Include MS Teams width metadata in all AdaptiveCard fixtures
Multiple AdaptiveCard JSON fixtures under tests/unit are missing the required
"msteams": {
"width": "Full"
}block. Add this metadata to every AdaptiveCard fixture to ensure consistent Teams full-width rendering.
🤖 Prompt for AI Agents
In
tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-True_tags-False_owners-False_path-True_error-True_suppression-True_env-True.json
around lines 181-184, the AdaptiveCard fixture is missing the required MS Teams
width metadata; add the "msteams": { "width": "Full" } block to the root of the
AdaptiveCard JSON (alongside "version") so every fixture includes the Teams
full-width hint; ensure formatting/commas remain valid after insertion.
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Teams width metadata added—looks correct; confirm AC 1.5 support baseline.
The msteams.width = "Full" hint is appropriate. Please confirm all target Tenants/clients used in CI support AdaptiveCard version 1.5 to avoid rollout regressions.
🌐 Web query:
Do Microsoft Teams clients (desktop, web, mobile) fully support Adaptive Cards 1.5 and the root-level "msteams": {"width": "Full"} hint as of September 2025?
💡 Result:
Short answer: No — as of September 2, 2025 Teams does not uniformly “fully” support Adaptive Cards 1.5 across all clients.
-
Desktop and Teams web: most 1.5 features (including the root-level "msteams": {"width":"Full"} full‑width behavior) are supported in current stable clients, and Microsoft has published guidance and blog posts about full‑width and other 1.5 features. [1][2]
-
Mobile (iOS/Android): partial support. Teams mobile has known limitations and regressions with Adaptive Cards 1.5 — certain 1.5 elements (for example Chart.Gauge and other advanced/visual controls) can fail to render or cause the card to not display on mobile, and developers are advised to provide fallbacks and test on mobile. Recent Microsoft Q&A reports (Aug 2025) confirm rendering failures on mobile for v1.5 cards. [3][1]
If you need full cross‑client compatibility today, either:
- target Adaptive Cards 1.4 (safer), or
- implement element-level fallbacks/fallbackText and test on desktop, web, and mobile. [3][1]
Sources:
[1] Microsoft dev blog: Adaptive Cards / full‑width feature announcement.
[2] Microsoft Q&A / Teams thread about full‑width behavior and client fixes.
[3] Microsoft Q&A (Aug 2025) — Adaptive Card v1.5 rendering issues on Teams mobile (Chart.Gauge example).
Ensure cross-client support for AC v1.5 full-width hint
- Desktop/web: root-level
"msteams": {"width":"Full"}is supported in current stable clients. - Mobile (iOS/Android): partial support; certain v1.5 elements can fail to render, causing cards to not display.
- Mandate element-level fallbacks/fallbackText or revert to Adaptive Cards 1.4 for guaranteed mobile compatibility.
🤖 Prompt for AI Agents
tests/unit/messages/formats/adaptive_cards/fixtures/colored_header.json lines
24-27: the root-level "msteams": {"width":"Full"} targeting Adaptive Cards v1.5
can break rendering on mobile clients; either revert the card version to "1.4"
for guaranteed mobile compatibility or add element-level fallbacks so mobile
clients can render gracefully—update the JSON to use "version":"1.4" if you
choose the revert path, or add fallbackText or "fallback":"drop"/fallback
elements on any v1.5-specific pieces (and a root-level fallbackText) so clients
that don’t support width:"Full" still show a usable card.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_1.json
Show resolved
Hide resolved
FrankTub
had a problem deploying
to
elementary_test_env
GitHub Actions
Failure
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_1.json (1)
24-26: Resolved: missing wrap:true now presentAddresses the earlier review about wrapping in TableCell TextBlocks.
Also applies to: 39-41, 54-56, 69-71, 84-86, 99-101
🧹 Nitpick comments (7)
tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json (1)
138-140: Good: wrapping cell text avoids truncation in Teams tables.This should improve readability for longer headers/values.
If you want clearer headers, consider bolding the header row:
{ "type": "TextBlock", "text": "column1", + "weight": "bolder", "wrap": true } ... { "type": "TextBlock", "text": "column2", + "weight": "bolder", "wrap": true }Also applies to: 148-150, 163-165, 173-175
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_3.json (3)
65-67: Row cells: wrap additions are consistent and necessaryApplying wrap: true across body cells matches the formatter update and prevents clipped content in Teams. Consider optionally bounding growth with maxLines to avoid excessively tall cards for very long values.
If you want to cap height centrally in the builder (not fixtures), you could add:
- {"type": "TextBlock", "text": cell_text, "wrap": True} + {"type": "TextBlock", "text": cell_text, "wrap": True, "maxLines": 5}Happy to send a follow-up PR if you want this guarded behind a CLI flag/env to keep current defaults.
Also applies to: 75-77, 85-87, 100-102, 110-112, 120-122, 135-137, 145-147, 155-157, 170-172, 180-182, 190-192, 205-207, 215-217, 225-227
21-56: Fixture still models 3 columns—add boundary tests for the new CLI flagSince this PR introduces a CLI flag to control “table-formatted columns,” add fixtures/tests that cover: below limit (N-1), at limit (N), and above limit (N+1) to validate fallback formatting for overflow columns.
I can generate parametric tests plus fixtures like table_block_200_10.json and a mixed table+bullet rendering snapshot. Want me to draft them?
60-91: Add a null/None-cell case to exercise empty-string formattingThe formatter now converts None to "". Please include one cell with null in a dedicated fixture (or row) to lock this behavior in snapshot tests.
I can add a table_block_null_cell.json fixture and the corresponding test assertion—let me know.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_1.json (1)
111-114: Top-level msteams width field addedLooks correct. Consider adding a small unit snapshot asserting presence of msteams.width to prevent regressions.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_4.json (1)
300-303: msteams.width addition acknowledgedIf you have mixed render targets, consider gating this field in formatter behind a “teams” target flag to keep fixtures renderer-agnostic.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_3.json (1)
57-231: Optional: cap lines to prevent excessively tall cards.Consider adding
"maxLines": 3(or another sensible cap) alongside"wrap": truefor dense samples. This keeps cards readable when cells contain long strings.Example for each TextBlock:
- "wrap": true + "wrap": true, + "maxLines": 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-error_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json(5 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-False.json(5 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_env-False.json(5 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json(5 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_1.json(7 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_2.json(13 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_3.json(19 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_4.json(25 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_5.json(31 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_1.json(7 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_2.json(13 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_3.json(19 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_4.json(25 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_5.json(31 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_1.json(7 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_2.json(13 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_3.json(19 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_4.json(25 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_5.json(31 hunks)
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_3.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_4.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-error_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-False.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_2.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_1.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_5.json
- tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_env-False.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_4.json
🔇 Additional comments (16)
tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json (1)
190-193: Verified: All Adaptive Card fixtures include msteams.width="Full"
Script confirms the width setting in every fixture; approving changes.tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_5.json (2)
36-38: Header cells now wrap — good.Using wrap: true on header TextBlocks prevents truncation and matches the new Teams formatting intent.
Also applies to: 46-48, 56-58, 66-68, 76-78
91-93: Body cells wrap enabled across all rows — good.This aligns with Adaptive Cards TextBlock semantics and avoids clipped sample values in Teams.
Also applies to: 101-103, 111-113, 121-123, 131-133, 146-148, 156-158, 166-168, 176-178, 186-188, 201-203, 211-213, 221-223, 231-233, 241-243, 256-258, 266-268, 276-278, 286-288, 296-298, 311-313, 321-323, 331-333, 341-343, 351-353
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_3.json (2)
30-32: Header cells: enabling wrap is correct for Teams renderingSetting wrap: true on header TextBlocks avoids truncation in Teams and aligns with the new formatter behavior. Looks good.
Also applies to: 40-42, 50-52
237-240: Approve: msteams width “Full” is supported for Adaptive Cards v1.5
Docs confirm Teams supports Adaptive Cards version 1.5 and the msteams host property"width": "Full"for full-width cards; no changes needed.tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_2.json (2)
27-39: Text wrapping for table headers and cells — LGTMAdding
"wrap": trueacross header and body cells should prevent truncation in Teams and improves readability for long values. No issues spotted.Also applies to: 52-64, 77-89, 102-114, 127-139, 152-164
174-177: All Adaptive Card fixtures valid and consistent
All fixtures pass JSON validation, includemsteams.width: "Full", and table-based TextBlocks havewrap: true.tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_5.json (2)
36-78: Wrap enabled across all TableCell TextBlocks looks goodConsistent with formatter changes; this should prevent truncation in Teams.
Also applies to: 91-133, 146-188, 201-243, 256-298, 311-353
363-366: Teams width casing and Adaptive Cards v1.5 support verified
Official docs specify msteams.width as "Full" (uppercase) and Teams supports Adaptive Cards schema v1.5; lowercase "full" is also accepted.tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_1.json (1)
24-26: Good: added wrap:true to header and cellsMatches the formatter behavior; avoids clipped text in narrow columns.
Also applies to: 39-41, 54-56, 69-71, 84-86, 99-101
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_1.json (1)
111-114: Confirm Teams-specific hint compatibilitymsteams.width at root looks fine; just ensure downstream renderers that may reuse these fixtures ignore unknown fields.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_4.json (1)
33-65: Consistent wrapping across a 4-column tableNice consistency; header and body cells now wrap uniformly.
Also applies to: 78-110, 123-155, 168-200, 213-245, 258-290
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_2.json (2)
27-39: Wrap:true applied correctly in 2-column layoutGood alignment with the global wrapping policy.
Also applies to: 52-64, 77-89, 102-114, 127-139, 152-164
174-177: Root msteams field addedLGTM. Same note on verifying "Full" casing as in other fixtures.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_3.json (2)
29-32: Good addition: enable wrapping in table cells.Adding
"wrap": trueis aligned with Teams card rendering quirks and prevents truncation. Spot-checked across header and body cells in this fixture—looks consistent.
237-240: Confirm Teams support for msteams.width and schema tolerance
All fixtures under tests/unit/messages/formats/adaptive_cards and the builder at elementary/messages/formats/adaptive_cards.py:260 already include"msteams": {"width": "Full"}; ensure your target Teams clients render Adaptive Card v1.5 with this setting and that any schema validation permits root‐level additionalProperties.
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add msteams width Full — align with Teams full-width rendering.
Root-level msteams: { "width": "Full" } is correct for Teams. Ensure all Adaptive Card fixtures that represent Teams messages include this, and non-Teams renderers ignore it as expected.
Run to verify consistency across fixtures:
🏁 Script executed:
#!/bin/bash
# Verify all Adaptive Card fixtures include msteams width Full
fd -t f -e json tests/unit/messages/formats/adaptive_cards/fixtures \
| while read -r f; do
if ! rg -nP '"msteams"\s*:\s*\{\s*"width"\s*:\s*"Full"\s*\}' "$f" > /dev/null; then
echo "Missing msteams width Full in: $f"
fi
doneLength of output: 591
🏁 Script executed:
#!/bin/bash
# Correctly verify all Adaptive Card fixtures include msteams width Full
fd . tests/unit/messages/formats/adaptive_cards/fixtures -t f -e json \
| while read -r f; do
if ! grep -P '"msteams"\s*:\s*\{\s*"width"\s*:\s*"Full"\s*\}' "$f" > /dev/null; then
echo "Missing msteams width Full in: $f"
fi
doneLength of output: 13070
Add msteams.width: "Full" to every Adaptive Card fixture
The verification script shows that nearly all JSON fixtures under tests/unit/messages/formats/adaptive_cards/fixtures are missing the root‐level
"msteams": { "width": "Full" }Please update each fixture accordingly so Teams renders full width while non–Teams renderers continue to ignore it.
🤖 Prompt for AI Agents
In tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_5.json
around lines 363–366, the root object is missing the Teams-specific property;
add a top-level "msteams": { "width": "Full" } entry (e.g., alongside "version")
so the JSON remains valid and Teams will render full width; apply the same
change to every other fixture under
tests/unit/messages/formats/adaptive_cards/fixtures.
6187968 to
738c2e7
Compare
FrankTub
had a problem deploying
to
elementary_test_env
GitHub Actions
Failure
…mber of columns that are allowed in a result sample that will be displayed as a table. If it exceeds this number of columns than it will be parsed as a JSON
… are better readable
…it in Teams for scenarios where a lot of alerts need to be sent
…e message that is sent to MS Teams
FrankTub
force-pushed
the
feature/variable-number-of-columns-allowed
branch
from
738c2e7 to
e068485
Compare
FrankTub
temporarily deployed
to
elementary_test_env
GitHub Actions
Inactive
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
elementary/messages/formats/adaptive_cards.py (1)
254-255: Version comparison is incorrect (string vs numeric) — fix to avoid rejecting valid versionsLexicographic string compare will misorder versions (e.g., "1.10" < "1.6"). Compare numerically.
Apply this diff:
- if version < "1.2" or version > "1.6": - raise ValueError(f"Version {version} is not supported") + try: + major, minor = map(int, version.split(".", 1)) + except ValueError: + raise ValueError(f"Version {version} is not supported") + if (major, minor) < (1, 2) or (major, minor) > (1, 6): + raise ValueError(f"Version {version} is not supported")elementary/messages/messaging_integrations/teams_webhook.py (1)
42-46: Add a requests timeout to avoid hanging on network issuesExternal calls should set a timeout.
- response = requests.post( + response = requests.post( webhook_url, json=payload, headers={"Content-Type": "application/json"}, - ) + timeout=10, # seconds; consider making this configurable + )
♻️ Duplicate comments (10)
tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-False.json (1)
190-193: Teams full-width metadata added correctlyTop-level
"msteams": { "width": "Full" }is placed and cased correctly; JSON syntax (comma after version) is valid. This resolves the earlier feedback about missing Teams width metadata.tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-True_tags-False_owners-False_path-True_error-True_suppression-True_env-True.json (1)
181-184: Resolved: msteams width addedThis addresses the earlier ask to include Teams width metadata. All good now.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-error_link-False_tags-True_owners-True_path-True_suppression-False_materialization-incremental_full_refresh-True_env-False.json (1)
151-154: Earlier ask is now addressed.
msteams.width = Fullis present and correctly positioned after version.tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-False_message-False_tags-False_owners-False_path-False_suppression-False_env-False.json (1)
29-32: Repeat: Gate Teams-specific metadata and confirm 1.5 support.Echoing prior guidance—ensure the generator emits
"msteams": {"width":"Full"}only for Teams cards and verify your tenant supports Adaptive Card 1.5.tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-False_test-errors-True_link-False_env-False_subscribers-True.json (1)
151-154: Repeat: Ensure fixture/generator consistency for Teams-only field.Same note as earlier—emit
msteams.widthonly for Teams channel and keep fixtures consistent.tests/unit/messages/formats/adaptive_cards/fixtures/colored_header.json (1)
24-27: Dup: confirm AC 1.5 + msteams width Full across Teams clients (esp. mobile).Echoing the earlier note here—if mobile regressions appear, either add fallbacks or pin to "1.4" for those flows.
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_1.json (1)
24-25: Cell wrapping enabled in table cellsSetting
"wrap": trueon TextBlocks inside TableCell fixes truncation and aligns with prior review guidance.Also applies to: 39-41, 54-56, 69-71, 84-86, 99-101
tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_5.json (1)
363-366: Earlier request satisfied: msteams width addedThis resolves the prior comment about missing
"msteams": {"width": "Full"}.elementary/messages/messaging_integrations/teams_webhook.py (2)
6-7: Dependency pin for ratelimitEnsure ratelimit is declared and pinned to a known-good version.
Run to verify:
#!/bin/bash # Check common dependency files for a pinned ratelimit entry fd -a '(^requirements.*\.txt$|^pyproject\.toml$|^setup\.(cfg|py)$)' -HI \ | xargs -I{} sh -c 'echo "--- {} ---"; rg -nP "ratelimit\s*[=><!]" "{}" || true'
80-85: Tighten old-webhook success check (strip and compare to "1")Length-based check can misclassify whitespace. Strip and require explicit "1".
- if response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) or ( - response.status_code == HTTPStatus.OK and len(response.text) > 1 - ): + text = response.text.strip() + if response.status_code not in (HTTPStatus.OK, HTTPStatus.ACCEPTED) or ( + response.status_code == HTTPStatus.OK and text != "1" + ): raise MessagingIntegrationError( f"Could not post message to Teams via webhook. Status code: {response.status_code}, Error: {response.text}" )
🧹 Nitpick comments (18)
tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-error_link-False_message-True_tags-True_owners-False_path-False_suppression-True_env-False.json (1)
109-112: All AdaptiveCard fixtures include the requiredmsteams.width; centralization remains optional for maintainability.tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-False_message-True_tags-True_owners-False_path-True_suppression-False_env-False.json (1)
113-116: Centralize channel-specific metadataIf not already, set the card version and
msteams.widthin one place in the Teams alert builder to avoid future fixture churn and ensure this metadata is injected only for Teams.tests/unit/messages/formats/adaptive_cards/fixtures/all_icons.json (1)
16-19: Ensure msteams.width uses uppercase "Full" and unify via constant Confirmed per Teams docs thatmsteams.widthmust be"Full"(uppercase) (learn.microsoft.com) and all adaptive_card fixtures include it. Recommend centralizing this value in a single constant within the formatter to prevent future drift.tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-None_link-True_tags-False_owners-True_path-True_suppression-True_materialization-table_full_refresh-False_env-True.json (1)
156-159: Consistent with full-width rollout.Change is correct; keep fixtures consistent across alert types. Consider adding a test asserting the builder always injects this for Teams.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True_subscribers-False.json (1)
273-276: Looks good; group cards will render full-width.Keep an eye on card height—full width plus many TextBlocks can trigger scroll; consider batching if needed.
tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-True_message-True_tags-False_owners-True_path-False_error-True_suppression-False_env-True.json (1)
177-180: Change is correct; verify long monospace blocks wrap well in full-width.If wrapping is poor on some clients, consider setting
wrap: trueand maxLines where applicable.tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_2.json (1)
174-177: Add Teams full-width metadata to all AdaptiveCard fixtures
Several AdaptiveCard JSON fixtures are missing the root-level"msteams": { "width": "Full" }entry; please update eachtests/unit/.../*.jsoncontaining"type": "AdaptiveCard"to include it for consistency.elementary/messages/formats/adaptive_cards.py (1)
260-260: Always adding msteams metadata — consider gating by targetIf this formatter is reused outside Teams, consider a flag to include/exclude
"msteams"to avoid unexpected fields in strict renderers.Apply this minimal change:
-def format_adaptive_card(message: MessageBody, version: str = "1.5") -> Dict[str, Any]: +def format_adaptive_card(message: MessageBody, version: str = "1.5", include_teams_meta: bool = True) -> Dict[str, Any]: @@ - return { + card = { "type": "AdaptiveCard", "body": format_adaptive_card_body(message), "version": version, - "msteams": {"width": "Full"}, } + if include_teams_meta: + card["msteams"] = {"width": "Full"} + return cardEnsure all call sites either accept the default or explicitly pass
include_teams_meta=Truefor Teams.elementary/monitor/alerts/alert_messages/builder.py (3)
345-349: Harden table-sample rendering guardGuard against non-dict rows and prefer len(dict) over len(keys). Functionally the same, slightly safer.
- if ( - isinstance(result_sample, list) - and len(result_sample[0].keys()) - <= self.config.maximum_columns_in_alert_samples - ): + if ( + isinstance(result_sample, list) + and result_sample + and isinstance(result_sample[0], dict) + and len(result_sample[0]) + <= self.config.maximum_columns_in_alert_samples + ):
100-110: Avoid mutable default argumentsA few parameters default to [] which is shared across calls. Use None and normalize.
- def _get_run_alert_subtitle_block(..., links: list[ReportLinkData] = [],) -> LinesBlock: + def _get_run_alert_subtitle_block(..., links: Optional[list[ReportLinkData]] = None,) -> LinesBlock: @@ - if links: + if links: ...- def _get_details_blocks(..., fields: List[str] = [],) -> List[MessageBlock]: + def _get_details_blocks(..., fields: Optional[List[str]] = None,) -> List[MessageBlock]: @@ - blocks: List[MessageBlock] = [] + fields = fields or [] + blocks: List[MessageBlock] = []- def _get_result_blocks(..., fields: List[str] = [],) -> List[MessageBlock]: + def _get_result_blocks(..., fields: Optional[List[str]] = None,) -> List[MessageBlock]: @@ - result_blocks: List[MessageBlock] = [] + fields = fields or [] + result_blocks: List[MessageBlock] = []Also applies to: 214-216, 303-305
57-61: Validate config bounds (optional)Consider constraining maximum_columns_in_alert_samples to non-negative ints (0 can mean “never table”). With Pydantic: conint(ge=0).
-from pydantic import BaseModel +from pydantic import BaseModel, conint @@ - maximum_columns_in_alert_samples: int = 4 + maximum_columns_in_alert_samples: conint(ge=0) = 4elementary/config/config.py (1)
74-75: Config propagation looks good; precedence is clearConstructor arg > config file > default (4) is correct. Consider documenting the new key in sample config/README.
Also applies to: 107-112
elementary/messages/messaging_integrations/teams_webhook.py (3)
66-70: Suppress unused parameter warningdestination is unused by design; rename to underscore to satisfy linters.
- def send_message( + def send_message( self, - destination: None, + destination: None, # unused body: MessageBody, ) -> MessageSendResult[EmptyMessageContext]:If allowed, alternatively:
- destination: None, + _: None,
26-28: Remove unused TypeAliasChannel is unused.
-Channel: TypeAlias = Optional[str] -ONE_SECOND = 1 +ONE_SECOND = 1
91-94: Minor f-string tweak and message trimmingUse explicit string conversion flag and consider truncating very long exception strings.
- raise MessagingIntegrationError( - f"An error occurred while posting message to Teams webhook: {str(e)}" - ) from e + raise MessagingIntegrationError( + f"An error occurred while posting message to Teams webhook: {e!s}" + ) from eelementary/monitor/cli.py (1)
342-359: Wiring to Config looks good; consider adding to telemetry props.Value is threaded correctly into Config. For observability, include this option in get_cli_properties().
Outside this hunk, add to get_cli_properties():
def get_cli_properties() -> dict: @@ suppression_interval = params.get("suppression_interval") override_dbt_project_config = params.get("override_dbt_project_config") + maximum_columns_in_alert_samples = params.get("maximum_columns_in_alert_samples") @@ "override_dbt_project_config": override_dbt_project_config, + "maximum_columns_in_alert_samples": maximum_columns_in_alert_samples, }elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (2)
93-97: Pre-configured builder per instance — good for consistency and perf.Nice move to construct once. Minor: prefer private attribute to signal immutability.
Apply:
- self.alert_message_builder = AlertMessageBuilder( + self._alert_message_builder = AlertMessageBuilder( MessageBuilderConfig( maximum_columns_in_alert_samples=self.config.maximum_columns_in_alert_samples ) )And update call site below (Line 320).
320-322: Reuse the prebuilt builder; consider richer error details.Reuse is correct. Optionally improve error logging to include the exception for easier troubleshooting with Teams webhooks.
Apply (paired with the rename above):
- alert_message_body = self.alert_message_builder.build( + alert_message_body = self._alert_message_builder.build( alert=alert, )Outside this hunk (in the except block):
- except MessagingIntegrationError: - logger.error(f"Could not send the alert - {type(alert)}.") + except MessagingIntegrationError as exc: + logger.error(f"Could not send the alert - {type(alert)}. Reason: {exc}")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (107)
elementary/config/config.py(2 hunks)elementary/messages/formats/adaptive_cards.py(2 hunks)elementary/messages/messaging_integrations/teams_webhook.py(4 hunks)elementary/monitor/alerts/alert_messages/builder.py(2 hunks)elementary/monitor/cli.py(3 hunks)elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py(3 hunks)elementary/monitor/dbt_project/.gitignore(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-False_test-warnings-True_test-errors-True_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-False_test-errors-False_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-False_test-errors-True_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-False_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-False_test-warnings-False_test-errors-True_link-False_env-False_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-False_test-warnings-True_test-errors-False_link-True_env-False_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-False_test-warnings-True_test-errors-True_link-True_env-True_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-False_test-errors-True_link-True_env-True_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-True_test-errors-False_link-True_env-True_subscribers-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/alerts_group_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True_subscribers-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-column.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-description.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-owners.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-result_message.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-subscribers.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-table.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-tags.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-test_parameters.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-test_query.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_removed_field-test_results_sample.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-None_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-None_link-True_description-True_tags-False_owners-False_table-True_error-True_sample-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-error_link-False_description-True_tags-False_owners-True_table-False_error-True_sample-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-error_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json(5 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-False.json(5 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_env-False.json(5 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/dbt_test_alert_status-warn_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_env-True.json(5 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_anomaly-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-False_description-True_tags-False_owners-True_table-False_error-True_sample-False_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-None_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-error_link-False_description-True_tags-True_owners-False_table-True_error-False_sample-True_anomaly-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-error_link-True_description-True_tags-False_owners-False_table-False_error-True_sample-False_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-fail_link-False_description-False_tags-False_owners-False_table-False_error-False_sample-False_anomaly-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-fail_link-True_description-True_tags-True_owners-True_table-True_error-True_sample-True_anomaly-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-warn_link-False_description-True_tags-False_owners-True_table-False_error-True_sample-False_anomaly-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/elementary_test_alert_status-warn_link-True_description-False_tags-True_owners-False_table-True_error-False_sample-True_anomaly-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-False_test-warnings-True_test-errors-True_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-False_test-errors-False_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-False_test-errors-True_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-False_test-errors-True_link-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-True_test-errors-False_link-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-False_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-False_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-True_test-errors-False_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-True_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-None_link-False_tags-False_owners-True_path-False_suppression-True_materialization-table_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-None_link-True_tags-False_owners-True_path-True_suppression-True_materialization-table_full_refresh-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-error_link-False_tags-True_owners-True_path-True_suppression-False_materialization-incremental_full_refresh-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-error_link-True_tags-True_owners-False_path-False_suppression-True_materialization-view_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-fail_link-False_tags-False_owners-False_path-False_suppression-False_materialization-table_full_refresh-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-fail_link-True_tags-False_owners-False_path-True_suppression-False_materialization-incremental_full_refresh-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-fail_link-True_tags-True_owners-True_path-True_suppression-True_materialization-table_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-warn_link-False_tags-True_owners-False_path-True_suppression-True_materialization-incremental_full_refresh-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-warn_link-False_tags-True_owners-True_path-True_suppression-True_materialization-view_full_refresh-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/model_alert_status-warn_link-True_tags-True_owners-True_path-False_suppression-True_materialization-view_full_refresh-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-None_link-False_message-False_tags-False_owners-True_path-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-None_link-True_message-False_tags-True_owners-False_path-False_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-error_link-False_message-True_tags-True_owners-False_path-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-error_link-True_message-True_tags-False_owners-True_path-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-False_message-False_tags-False_owners-False_path-False_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-True_message-False_tags-True_owners-True_path-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-fail_link-True_message-True_tags-True_owners-True_path-True_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-False_message-True_tags-False_owners-True_path-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-False_message-True_tags-True_owners-False_path-True_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/snapshot_alert_status-warn_link-True_message-False_tags-True_owners-False_path-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-False_message-False_tags-True_owners-True_path-True_error-True_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-False_message-True_tags-True_owners-True_path-False_error-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-False_tags-False_owners-False_path-False_error-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-False_tags-True_owners-True_path-True_error-False_suppression-True_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-error_link-True_message-True_tags-False_owners-False_path-True_error-True_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-False_message-False_tags-False_owners-False_path-False_error-False_suppression-False_env-False.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-False_message-True_tags-True_owners-False_path-False_error-True_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-True_message-False_tags-True_owners-False_path-True_error-False_suppression-True_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-True_message-True_tags-False_owners-True_path-False_error-True_suppression-False_env-True.json(1 hunks)tests/unit/alerts/alert_messages/fixtures/adaptive_card/source_freshness_alert_status-runtime error_link-True_message-True_tags-True_owners-True_path-True_error-True_suppression-True_env-True.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_green.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_no_color.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_red.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_blocks_yellow.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/all_icons.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/bullet_list.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/code_block_200.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/code_block_50.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/code_block_500.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/colored_header.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/divider_blocks.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/expandable_block.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/fact_list.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/nested_expandable.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/simple_header.json(1 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_1.json(7 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_2.json(13 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_3.json(19 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_4.json(25 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_200_5.json(31 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_1.json(7 hunks)tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_2.json(13 hunks)
⛔ Files not processed due to max files limit (14)
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_3.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_4.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_30_5.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_1.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_2.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_3.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_4.json
- tests/unit/messages/formats/adaptive_cards/fixtures/table_block_8_5.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_1000.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_200.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_50.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_length_500.json
- tests/unit/messages/formats/adaptive_cards/fixtures/text_styles.json
- tests/unit/messages/formats/adaptive_cards/fixtures/whitespace_block.json
🧰 Additional context used
🧬 Code graph analysis (3)
elementary/config/config.py (1)
tests/unit/config/test_config.py (1)
config(38-41)
elementary/messages/messaging_integrations/teams_webhook.py (5)
elementary/messages/messaging_integrations/base_messaging_integration.py (2)
send_message(35-40)MessageSendResult(19-22)elementary/messages/message_body.py (1)
MessageBody(36-39)elementary/messages/messaging_integrations/empty_message_context.py (1)
EmptyMessageContext(4-5)elementary/messages/formats/adaptive_cards.py (1)
format_adaptive_card(253-261)elementary/messages/messaging_integrations/exceptions.py (1)
MessagingIntegrationError(1-2)
elementary/monitor/data_monitoring/alerts/data_monitoring_alerts.py (1)
elementary/monitor/alerts/alert_messages/builder.py (3)
AlertMessageBuilder(62-718)MessageBuilderConfig(57-59)build(677-718)
🪛 Ruff (0.12.2)
elementary/messages/messaging_integrations/teams_webhook.py
68-68: Unused method argument: destination
(ARG002)
83-85: Avoid specifying long messages outside the exception class
(TRY003)
93-93: Use explicit conversion flag
Replace with conversion flag
(RUF010)
| @click.option( | ||
| "--maximum-columns-in-alert-samples", | ||
| "-mc", | ||
| type=int, | ||
| default=4, | ||
| help="Maximum number of columns to display as a table in alert samples. Above this, the output is shown as raw JSON.", | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix precedence bug: CLI default overrides config; validate range.
With default=4 here, Click always passes 4, shadowing any value from config.yml despite the intended precedence (CLI explicit > config > default 4). Also, negatives should be rejected.
Apply:
@click.option(
"--maximum-columns-in-alert-samples",
"-mc",
- type=int,
- default=4,
- help="Maximum number of columns to display as a table in alert samples. Above this, the output is shown as raw JSON.",
+ type=click.IntRange(min=0),
+ default=None,
+ show_default=4,
+ help="Maximum number of columns to display as a table in alert samples. Above this, the output is shown as raw JSON. Set to 0 to force JSON for all samples.",
)📝 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.
| @click.option( | |
| "--maximum-columns-in-alert-samples", | |
| "-mc", | |
| type=int, | |
| default=4, | |
| help="Maximum number of columns to display as a table in alert samples. Above this, the output is shown as raw JSON.", | |
| ) | |
| @click.option( | |
| "--maximum-columns-in-alert-samples", | |
| "-mc", | |
| type=click.IntRange(min=0), | |
| default=None, | |
| show_default=4, | |
| help="Maximum number of columns to display as a table in alert samples. Above this, the output is shown as raw JSON. Set to 0 to force JSON for all samples.", | |
| ) |
🤖 Prompt for AI Agents
In elementary/monitor/cli.py around lines 288 to 294, the Click option currently
sets default=4 which causes the CLI to always pass 4 and override values from
config.yml; change the option to default=None so absence of a CLI flag allows
the config value to take precedence, and validate the value to reject negatives
(e.g., use click.IntRange(min=0) or a callback that raises a BadParameter for
value < 0); also ensure the downstream code interprets None as "use config or
fallback default 4" rather than treating None as 4.
| "version": "1.5", | ||
| "msteams": { | ||
| "width": "Full" | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
msteams width metadata — LGTM
The added "msteams": { "width": "Full" } looks correct for full-width Teams rendering; JSON structure remains consistent.
Quick sweep to validate all fixtures:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Validate JSON
fd -e json tests/unit | xargs -I{} python -m json.tool {} >/dev/null
# 2) Ensure every AdaptiveCard fixture includes msteams.width
rg -nP '"type"\s*:\s*"AdaptiveCard"' tests/unit --glob '*.json' \
| awk -F: '{print $1}' | sort -u \
| xargs -I{} bash -c 'rg -q "\"msteams\"\\s*:\\s*\\{\\s*\"width\"\\s*:\\s*\"Full\"" "{}" || echo "Missing msteams.width: {}"'Length of output: 345
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# 1) Validate JSON syntax for all fixtures
find tests/unit -type f -name '*.json' -print0 \
| xargs -0 -n1 python -m json.tool >/dev/null
# 2) Verify that every AdaptiveCard fixture includes msteams.width = "Full"
grep -RPl '"type"\s*:\s*"AdaptiveCard"' tests/unit --include '*.json' \
| xargs -r -n1 grep -L '"msteams"\s*:\s*{\s*"width"\s*:\s*"Full"' \
| sed 's/^/Missing msteams.width: /'Length of output: 19181
Add msteams.width metadata to all AdaptiveCard fixtures
A sweep of tests/unit/**/*.json reveals dozens of AdaptiveCard fixtures missing the
"msteams": { "width": "Full" }block. Please update all AdaptiveCard fixtures under tests/unit to include this metadata for consistent Teams full-width rendering.
🤖 Prompt for AI Agents
In
tests/unit/alerts/alert_messages/fixtures/adaptive_card/group_by_table_model-errors-False_test-failures-True_test-warnings-True_test-errors-True_link-True_env-True.json
around lines 240 to 243, the AdaptiveCard JSON is missing the required
msteams.width metadata; add a "msteams" object with "width": "Full" to the
top-level Adaptive Card object (e.g., alongside "version") so the JSON includes
"msteams": { "width": "Full" } to ensure Teams full-width rendering and keep
formatting/commas valid.
haritamar
had a problem deploying
to
elementary_test_env
GitHub Actions
Error
haritamar
temporarily deployed
to
elementary_test_env
GitHub Actions
Inactive
4 participants
I've played around quite a lot with this teams integrations and sadly it is indeed not possible to detect for the new workflows alert webhooks to determine if an alert was sent successfully.
By default the formatting of the tables will behave as it used to do with one exception. The full width is used for the message, independent of the number of columns that the Test Results Sample will contain. So in the old situation:
In the new situation it will always look like:
When running something like it will simply behave like it used to:
The new situation can be triggered with for example:
Example output:
Summary by CodeRabbit