Skip to content

fix: read source-level meta.elementary config in tests#1002

Open
joostboon wants to merge 5 commits intomasterfrom
fix/source-level-meta-elementary-config
Open

fix: read source-level meta.elementary config in tests#1002
joostboon wants to merge 5 commits intomasterfrom
fix/source-level-meta-elementary-config

Conversation

@joostboon
Copy link
Copy Markdown
Contributor

@joostboon joostboon commented May 5, 2026

Problem

When meta.elementary (e.g. timestamp_column, days_back, seasonality) is configured at the source level in a dbt sources YAML, it is silently ignored by Elementary tests. Tests on tables under that source don't pick up the config, leading to unexpected behaviour (tests running without a timestamp column, or SQL errors when the config is required).

Root cause: dbt stores source-level meta in node.source_meta on each table node — it is not merged into node.meta. get_elementary_config_from_node only read node.meta, so source-level config was never found.

Fix

Read node.source_meta in get_elementary_config_from_node, inserted before node.meta so that table-level meta still wins (consistent with this macro's existing precedence convention — node.meta already won over node.config.meta before this change).

Also extracts the repeated meta→elementary merge pattern into a _merge_elementary_from_meta helper to reduce duplication.

Example — now works

sources:
  - name: my_source
    meta:
      elementary:
        timestamp_column: last_update   # previously ignored for tests
        days_back: 21
        seasonality: day_of_week
    tables:
      - name: my_table
        data_tests:
          - elementary.volume_anomalies  # now picks up source-level config

Table-level meta.elementary still overrides source-level config.

CI failures

test-cloud (fusion, bigquery) and test-cloud (latest_official, fabric) both failed at "Check DWH connection" before any tests ran — pre-existing infra issue, unrelated to this change.

Test plan

  • test_source_meta_config_is_picked_up — asserts source-level timestamp_column and where_expression are resolved when no model/test config is set
  • test_model_meta_overrides_source_meta_config — asserts table-level meta wins when both are set
  • Manual: confirm models (non-source nodes) are unaffected (source_meta absent on model nodes → new block is a no-op)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Unified merging of elementary configurations now includes source-level metadata; model-level settings continue to override source values.
    • Public configuration interface extended to accept an optional source-level config so adapters can receive and apply it.
  • Tests

    • Added integration tests validating that source-level config is picked up and that model-level config overrides source-level values.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a helper to merge elementary from meta dictionaries, updates get_elementary_config_from_node to merge node.config.elementary, node.config.meta.elementary, node.source_meta, and node.meta (via the helper), propagates source_meta_config through adapter-dispatched anomaly config macros, and adds tests for source→model override behavior.

Changes

Elementary config merge, adapter propagation, and tests

Layer / File(s) Summary
Helper / Data shape
macros/utils/graph/get_elementary_config_from_node.sql
Adds _merge_elementary_from_meta(res, meta_dict) to extract and merge elementary when present and a mapping.
Core merge logic
macros/utils/graph/get_elementary_config_from_node.sql
Refactors get_elementary_config_from_node(node) to initialize result and merge in order: node.config.elementary, node.config.meta.elementary (via helper), node.source_meta (via helper), then node.meta (via helper).
Adapter dispatch / Wiring
integration_tests/dbt_project/macros/get_anomaly_config.sql
Adds get_anomaly_config(model_config, config, source_meta_config=none) which dispatches to adapter implementations. Updates default__get_anomaly_config, and adds/updates spark__get_anomaly_config and clickhouse__get_anomaly_config to accept and propagate source_meta_config into the mock model; test macros use api.Relation.create("db","schema","mock_model").
Tests
integration_tests/tests/test_anomaly_test_configuration.py
Adds test_source_meta_config_is_picked_up and test_model_meta_overrides_source_meta_config to assert source-level meta.elementary is consumed and is overridable by model-level meta.
Microbatch tests & macros
integration_tests/tests/test_dbt_artifacts/test_microbatch_compiled_code.py, macros/edr/dbt_artifacts/microbatch/capture_microbatch_compiled_code.sql, macros/utils/graph/get_compiled_code.sql
Rewrites microbatch test helpers to write temp SQL/macro files, adjusts compiled-code assertions, reformats compiled_code extraction, and applies adapter formatting to compiled SQL.
Project metadata
integration_tests/dbt_project/dbt_project.yml
Aligns require_batched_execution_for_custom_microbatch_strategy: True entry under flags (YAML indentation/placement correction).

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐇 I nibble configs from leaf and root,
I stitch small meta crumbs into one suit.
Source hums softly, model sings more loud,
I blend their tunes and make the chorus proud.
🥕📜

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: read source-level meta.elementary config in tests' directly summarizes the main bug fix: enabling Elementary tests to read source-level meta.elementary configuration, which is the core change across the modified macros and test files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/source-level-meta-elementary-config

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 5, 2026

👋 @joostboon
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
Copy Markdown

@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

🧹 Nitpick comments (1)
integration_tests/dbt_project/macros/get_anomaly_config.sql (1)

9-73: ⚡ Quick win

Significant duplication across the three adapter macros.

default__get_anomaly_config, spark__get_anomaly_config, and clickhouse__get_anomaly_config are nearly identical — the only difference is the first argument to api.Relation.create(...). The shared body (mock-model construction, source_meta_config injection, context update) could be extracted into a private helper to avoid this triplication.

♻️ Sketch of a DRY refactor
+{% macro _build_anomaly_config(model_config, config, source_meta_config, relation) %}
+    {% set mock_model = {
+        "alias": "mock_model",
+        "config": {"elementary": model_config},
+    } %}
+    {% if source_meta_config is not none %}
+        {% do mock_model.update({"source_meta": source_meta_config}) %}
+    {% endif %}
+    {% do context.update(
+        {
+            "model": {"depends_on": {"nodes": ["id"]}},
+            "graph": {"nodes": {"id": mock_model}},
+        }
+    ) %}
+    {% do return(
+        elementary.get_anomalies_test_configuration(relation, **config)[0]
+    ) %}
+{% endmacro %}
+
 {% macro default__get_anomaly_config(model_config, config, source_meta_config=none) %}
-    {% set mock_model = {
-        "alias": "mock_model",
-        "config": {"elementary": model_config},
-    } %}
-    {% if source_meta_config is not none %}
-        {% do mock_model.update({"source_meta": source_meta_config}) %}
-    {% endif %}
-    {% do context.update(...) %}
-    {% do return(
-        elementary.get_anomalies_test_configuration(
-            api.Relation.create("db", "schema", "mock_model"), **config
-        )[0]
-    ) %}
+    {% do return(
+        elementary_tests._build_anomaly_config(
+            model_config, config, source_meta_config,
+            api.Relation.create("db", "schema", "mock_model")
+        )
+    ) %}
 {% endmacro %}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@integration_tests/dbt_project/macros/get_anomaly_config.sql` around lines 9 -
73, Duplication: the three macros default__get_anomaly_config,
spark__get_anomaly_config, and clickhouse__get_anomaly_config repeat the same
mock_model build, optional source_meta injection, context.update, and call to
elementary.get_anomalies_test_configuration — only the api.Relation.create(...)
argument differs; extract the shared body into a new helper macro (e.g.,
_get_anomaly_config_base(model_config, config, source_meta_config, relation))
that builds mock_model, applies source_meta_config, updates context, and returns
elementary.get_anomalies_test_configuration(relation, **config)[0], then rewrite
default__get_anomaly_config, spark__get_anomaly_config, and
clickhouse__get_anomaly_config to call that helper passing the appropriate
api.Relation.create(...) value for each adapter.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@integration_tests/dbt_project/macros/get_anomaly_config.sql`:
- Around line 1-9: The file failed sqlfmt formatting; run the pre-commit hook to
apply sqlfmt and commit the changes for the macros get_anomaly_config and
default__get_anomaly_config: run `pre-commit run --files
integration_tests/dbt_project/macros/get_anomaly_config.sql` (or run your repo's
pre-commit for all files), review the diffs produced for those macros, stage and
commit the resulting formatting changes so the pipeline no longer reports
formatting failures.

---

Nitpick comments:
In `@integration_tests/dbt_project/macros/get_anomaly_config.sql`:
- Around line 9-73: Duplication: the three macros default__get_anomaly_config,
spark__get_anomaly_config, and clickhouse__get_anomaly_config repeat the same
mock_model build, optional source_meta injection, context.update, and call to
elementary.get_anomalies_test_configuration — only the api.Relation.create(...)
argument differs; extract the shared body into a new helper macro (e.g.,
_get_anomaly_config_base(model_config, config, source_meta_config, relation))
that builds mock_model, applies source_meta_config, updates context, and returns
elementary.get_anomalies_test_configuration(relation, **config)[0], then rewrite
default__get_anomaly_config, spark__get_anomaly_config, and
clickhouse__get_anomaly_config to call that helper passing the appropriate
api.Relation.create(...) value for each adapter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1b308415-687e-492b-8554-c53d62f74ec6

📥 Commits

Reviewing files that changed from the base of the PR and between 9c28a7c and 3b02a79.

📒 Files selected for processing (3)
  • integration_tests/dbt_project/macros/get_anomaly_config.sql
  • integration_tests/tests/test_anomaly_test_configuration.py
  • macros/utils/graph/get_elementary_config_from_node.sql

Comment thread integration_tests/dbt_project/macros/get_anomaly_config.sql
joostboon and others added 5 commits May 6, 2026 22:46
dbt stores source-level meta separately in node.source_meta (not merged
into node.meta for table nodes). get_elementary_config_from_node only
read node.meta, so meta.elementary set at the source level (above
tables:) was silently ignored for test configuration like
timestamp_column, days_back, and seasonality.

Source_meta is already read and merged in upload_dbt_sources.sql for
artifact upload — this aligns test config resolution to match.
Table-level meta still takes precedence over source-level meta.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Extract repeated meta→elementary merge pattern into a
  _merge_elementary_from_meta helper, reducing duplication from three
  identical blocks to one.
- Extend get_anomaly_config test macro to accept source_meta_config so
  the source-meta code path can be exercised in integration tests.
- Add two integration tests: source-level meta.elementary is picked up
  when no model/test config overrides it, and model-level meta wins when
  both are set.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
In real dbt, node.source_meta is the full meta dict with the elementary
config nested under an "elementary" key (same structure as node.meta).
The mock was setting source_meta directly to the elementary config dict,
so _merge_elementary_from_meta couldn't find the "elementary" sub-key.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Rebase onto master picked up a new commit that added microbatch files
without running pre-commit. Apply black/isort/sqlfmt/prettier so
code-quality CI passes.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@joostboon joostboon force-pushed the fix/source-level-meta-elementary-config branch from eb4d044 to a8aa951 Compare May 7, 2026 16:44
Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
integration_tests/tests/test_dbt_artifacts/test_microbatch_compiled_code.py (1)

99-117: 💤 Low value

Hardcoded macro file path may conflict under parallel test execution.

macro_path is always macros/microbatch.sql regardless of macro_name. The two parametrize cases ("with_override" / "without_override") use different names but share the same file. Under sequential pytest this is fine, but if the suite is ever run with pytest-xdist (-n auto), both workers would race to create/delete the same path.

A simple guard is to include macro_name in the filename:

♻️ Proposed fix
-    macro_path = dbt_project.project_dir_path / "macros" / "microbatch.sql"
+    macro_path = dbt_project.project_dir_path / "macros" / f"microbatch_{macro_name}.sql"
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@integration_tests/tests/test_dbt_artifacts/test_microbatch_compiled_code.py`
around lines 99 - 117, The helper _with_microbatch_macro_file creates a
hardcoded macro file at macros/microbatch.sql which can cause race conditions
when tests run in parallel; change the macro_path construction to include the
macro_name (e.g., macros/microbatch_{macro_name}.sql) so each parametrized case
writes a unique file, keep the same write_text/yield/finally cleanup logic, and
ensure the exists check and unlink refer to the updated macro_path variable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@integration_tests/tests/test_dbt_artifacts/test_microbatch_compiled_code.py`:
- Around line 99-117: The helper _with_microbatch_macro_file creates a hardcoded
macro file at macros/microbatch.sql which can cause race conditions when tests
run in parallel; change the macro_path construction to include the macro_name
(e.g., macros/microbatch_{macro_name}.sql) so each parametrized case writes a
unique file, keep the same write_text/yield/finally cleanup logic, and ensure
the exists check and unlink refer to the updated macro_path variable.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: da347541-436d-4ced-9296-814bc0e5baf1

📥 Commits

Reviewing files that changed from the base of the PR and between eb4d044 and a8aa951.

📒 Files selected for processing (7)
  • integration_tests/dbt_project/dbt_project.yml
  • integration_tests/dbt_project/macros/get_anomaly_config.sql
  • integration_tests/tests/test_anomaly_test_configuration.py
  • integration_tests/tests/test_dbt_artifacts/test_microbatch_compiled_code.py
  • macros/edr/dbt_artifacts/microbatch/capture_microbatch_compiled_code.sql
  • macros/utils/graph/get_compiled_code.sql
  • macros/utils/graph/get_elementary_config_from_node.sql
✅ Files skipped from review due to trivial changes (2)
  • integration_tests/dbt_project/dbt_project.yml
  • macros/edr/dbt_artifacts/microbatch/capture_microbatch_compiled_code.sql
🚧 Files skipped from review as they are similar to previous changes (2)
  • integration_tests/tests/test_anomaly_test_configuration.py
  • integration_tests/dbt_project/macros/get_anomaly_config.sql

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.

2 participants