Skip to content

Conversation

@GuyEshdat
Copy link
Contributor

@GuyEshdat GuyEshdat commented Nov 9, 2025

Summary by CodeRabbit

  • Improvements
    • Updated anomaly alert wording to clarify freshness: instead of a simple "hours ago" timestamp, descriptions now explain that the alert reflects "X hours without updates (only full buckets are considered)" and retain the last-update time and usual update window for clearer temporal context.

@linear
Copy link

linear bot commented Nov 9, 2025

@coderabbitai
Copy link

coderabbitai bot commented Nov 9, 2025

Walkthrough

Updated the freshness_description macro in the anomaly detection alerts to change the user-facing duration wording from "hours ago." to "which is X hours without updates (only full buckets are considered)"; numeric calculation and surrounding logic are unchanged.

Changes

Cohort / File(s) Summary
Phrase update in macro description
macros/edr/alerts/anomaly_detection_description.sql
Replaced the final literal in freshness_description from "hours ago." to "which is X hours without updates (only full buckets are considered)". No changes to numeric calculations, concatenation logic, or error handling.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

  • Single-line text change in one macro — no logic modifications.
  • Review focus: confirm only the literal text changed and tests/documentation (if any) that reference the exact phrase.

Poem

🐰 I nibbled words and hopped about,
A phrase made clear, without a doubt,
Buckets counted, numbers stay—
The description dances, bright and gay. 🥕✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: improving the description text in the anomaly detection macro to be more accurate about how the test calculates and reports freshness duration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch core-9-clear-anomaly-description

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 9, 2025

👋 @GuyEshdat
Thank you for raising your pull request.
Please make sure to add tests and document all user-facing changes.
You can do this by editing the docs files in the elementary repository.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ac34be5 and 44f3504.

📒 Files selected for processing (1)
  • macros/edr/alerts/anomaly_detection_description.sql (1 hunks)
⏰ 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). (14)
  • GitHub Check: test (fusion, snowflake) / test
  • GitHub Check: test (latest_official, bigquery) / test
  • GitHub Check: test (latest_official, databricks_catalog) / test
  • GitHub Check: test (latest_official, trino) / test
  • GitHub Check: test (fusion, redshift) / test
  • GitHub Check: test (latest_official, clickhouse) / test
  • GitHub Check: test (latest_official, postgres) / test
  • GitHub Check: test (fusion, bigquery) / test
  • GitHub Check: test (fusion, databricks_catalog) / test
  • GitHub Check: test (latest_official, athena) / test
  • GitHub Check: test (latest_pre, postgres) / test
  • GitHub Check: test (latest_official, dremio) / test
  • GitHub Check: test (latest_official, redshift) / test
  • GitHub Check: test (latest_official, snowflake) / test


{% macro freshness_description() %}
'Last update was at ' || anomalous_value || ', ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('metric_value/3600') ~ ', 2))') }} || ' hours ago. Usually the table is updated within ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('training_avg/3600') ~ ', 2))') }} || ' hours.'
'Last update was at ' || anomalous_value || ', ' || 'which is' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('metric_value/3600') ~ ', 2))') }} || ' hours without updates (only full buckets are considered). Usually the table is updated within ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('training_avg/3600') ~ ', 2))') }} || ' hours.'
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add space between "which is" and the numeric value.

The string concatenation is missing a space between 'which is' and the calculated metric value. This will produce user-facing output like "which is2.5 hours" instead of "which is 2.5 hours".

Apply this diff to fix the spacing:

-    'Last update was at ' || anomalous_value || ', ' || 'which is' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('metric_value/3600') ~ ', 2))') }} || ' hours without updates (only full buckets are considered). Usually the table is updated within ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('training_avg/3600') ~ ', 2))') }} || ' hours.'
+    'Last update was at ' || anomalous_value || ', ' || 'which is ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('metric_value/3600') ~ ', 2))') }} || ' hours without updates (only full buckets are considered). Usually the table is updated within ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('training_avg/3600') ~ ', 2))') }} || ' hours.'
📝 Committable suggestion

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

Suggested change
'Last update was at ' || anomalous_value || ', ' || 'which is' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('metric_value/3600') ~ ', 2))') }} || ' hours without updates (only full buckets are considered). Usually the table is updated within ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('training_avg/3600') ~ ', 2))') }} || ' hours.'
'Last update was at ' || anomalous_value || ', ' || 'which is ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('metric_value/3600') ~ ', 2))') }} || ' hours without updates (only full buckets are considered). Usually the table is updated within ' || {{ elementary.edr_cast_as_string('abs(round(' ~ elementary.edr_cast_as_numeric('training_avg/3600') ~ ', 2))') }} || ' hours.'
🤖 Prompt for AI Agents
In macros/edr/alerts/anomaly_detection_description.sql around line 13, the
string concat lacks a space after "which is", causing output like "which is2.5
hours"; fix by adding a trailing space to that literal (e.g., change 'which is'
to 'which is ') or insert an explicit ' ' || between the literal and the numeric
expression so the rendered sentence reads "which is 2.5 hours".

@GuyEshdat GuyEshdat merged commit db1ea97 into master Nov 9, 2025
31 checks passed
@GuyEshdat GuyEshdat deleted the core-9-clear-anomaly-description branch November 9, 2025 16:32
@coderabbitai coderabbitai bot mentioned this pull request Dec 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants