Skip to content

Fix list-type metric filters under models key rendered at parse time#12634

Open
theyostalservice wants to merge 1 commit intomainfrom
patricky/fix-v2-list-filter-rendering
Open

Fix list-type metric filters under models key rendered at parse time#12634
theyostalservice wants to merge 1 commit intomainfrom
patricky/fix-v2-list-filter-rendering

Conversation

@theyostalservice
Copy link
Contributor

Summary

When v2 metrics defined under the models: key use list-type filters, the YAML schema renderer incorrectly attempts to render them at parse time, causing 'Dimension' is undefined errors.

The bug

SchemaYamlRenderer.should_render_keypath() uses deep_map_render from dbt_common, which builds a keypath tuple as it traverses nested structures. For dict keys, it appends strings (e.g. "filter"). For list elements, it appends integer indices (e.g. 0, 1).

The models key handler checked keypath[-1] == "filter", which works for string filters where the keypath looks like ('metrics', 0, 'filter'). But for list filters like:

filter:
  - "{{ Dimension('entity__is_fraud') }} = false"
  - "{{ Dimension('entity__is_employee') }} = false"

...the keypath for each element is ('metrics', 0, 'filter', 0) — and keypath[-1] is 0 (int), not "filter". So the check fails, the renderer tries to evaluate the Jinja, and Dimension is not in the parse-time context.

The fix

One-line change in core/dbt/parser/schema_renderer.py:

# Before (only catches string filters):
if keypath[-1] == "filter" and "metrics" in keypath:

# After (catches both string and list filter elements):
if "filter" in keypath and "metrics" in keypath:

This matches the approach already used by the metrics key handler (line 107), which checks keypath[-1] == "filter" or keypath[-2] == "filter" to handle both forms.

New tests

General coverage tests (these all passed before the fix — they fill gaps in test coverage for list-type filters and nested filter locations that previously had no unit tests):

  • test__metrics_list_filter — list-type top-level filters for standalone metrics key
  • test__metrics_nested_filtertype_params.numerator.filter as both string and list for standalone metrics key
  • test__models_v2_metric_filter_string — string filter under models key (regression test)
  • test__models_v2_nested_metric_filtersnumerator.filter, input_metrics[].filter, base_metric.filter as string and list under models key

Bug-specific regression test (this test fails without the fix with 'Dimension' is undefined):

  • test__models_v2_metric_filter_list — list-type filter on v2 metrics under the models key

Test plan

  • All 10 unit tests pass: python -m pytest tests/unit/parser/test_schema_renderer.py -v
  • test__models_v2_metric_filter_list fails without the fix (verified by reverting the one-line change)
  • test__models_v2_nested_metric_filters also fails without the fix for list-form nested filters
  • Pre-commit hooks pass (black, flake8, mypy, isort)

🤖 Generated with Claude Code

When v2 metrics defined under the `models:` key use list-type filters
(e.g. `filter: ["{{ Dimension(...) }}", "{{ Dimension(...) }}"]`), the
`deep_map_render` function appends integer indices to the keypath for
list elements. The previous check `keypath[-1] == "filter"` only matched
string filters (where the last element is literally "filter"), but missed
list elements where `keypath[-1]` is an integer like 0 or 1.

Changed to `"filter" in keypath` which correctly matches both forms.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@theyostalservice theyostalservice requested a review from a team as a code owner March 10, 2026 21:06
@cla-bot cla-bot bot added the cla:yes label Mar 10, 2026
@github-actions github-actions bot added the community This PR is from a community member label Mar 10, 2026
@codecov
Copy link

codecov bot commented Mar 11, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.36%. Comparing base (6bb1ea1) to head (5095551).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12634      +/-   ##
==========================================
- Coverage   91.40%   91.36%   -0.05%     
==========================================
  Files         203      203              
  Lines       25677    25677              
==========================================
- Hits        23470    23459      -11     
- Misses       2207     2218      +11     
Flag Coverage Δ
integration 88.14% <100.00%> (-0.12%) ⬇️
unit 65.36% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
Unit Tests 65.36% <100.00%> (+<0.01%) ⬆️
Integration Tests 88.14% <100.00%> (-0.12%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla:yes community This PR is from a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant