fix: address review findings in show_sample_rows and PII macros

- Restructure empty elif branch in test.sql with explicit comments
- Column-level show_sample_rows now checks model-level PII tags too
- Add explicit parens for Jinja operator precedence in get_pii_columns
- Replace 'is iterable' with 'is string' guard in is_pii_table.sql
  (strings are iterable in Jinja, causing single tags to be split)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Author: joostboon

macros/edr/materializations/test/test.sql

@@ -86,12 +86,17 @@
       4. PII tag detection (model/test/column) — hide when disable_samples_on_pii_tags
          is true and a PII tag is detected at any level.
     #}
-    {% if disable_test_samples %} {% set sample_limit = 0 %}
+    {% if disable_test_samples %}
+        {% set sample_limit = 0 %}
     {% elif elementary.should_show_sample_rows(flattened_test) %}
+        {# Tag explicitly opts in — keep sample_limit as-is #}
     {% elif elementary.get_config_var("enable_samples_on_show_sample_rows_tags") %}
+        {# Feature is on but no show_sample_rows tag found — hide by default #}
+        {% set sample_limit = 0 %}
+    {% elif elementary.is_pii_table(flattened_test) %}
+        {% set sample_limit = 0 %}
+    {% elif elementary.is_pii_test(flattened_test) %}
         {% set sample_limit = 0 %}
-    {% elif elementary.is_pii_table(flattened_test) %} {% set sample_limit = 0 %}
-    {% elif elementary.is_pii_test(flattened_test) %} {% set sample_limit = 0 %}
     {% elif elementary.should_disable_sampling_for_pii(flattened_test) %}
         {% set sample_limit = 0 %}
     {% endif %}

macros/edr/system/system_utils/get_pii_columns_from_parent_model.sql

@@ -58,16 +58,10 @@
         {# Skip column from PII list only if show_sample_rows is set and pii is not #}
         {% set has_show_tag = (
             enable_show_tags
-            and elementary.lists_intersection(
-                all_column_tags_lower, show_tags
-            )
-            | length
-            > 0
+            and (elementary.lists_intersection(all_column_tags_lower, show_tags) | length > 0)
         ) %}
         {% set has_pii_tag = (
-            elementary.lists_intersection(all_column_tags_lower, pii_tags)
-            | length
-            > 0
+            elementary.lists_intersection(all_column_tags_lower, pii_tags) | length > 0
         ) %}
         {% if has_show_tag and not has_pii_tag %} {% continue %} {% endif %}
 

macros/edr/system/system_utils/is_pii_table.sql

@@ -5,20 +5,16 @@
     {% if not disable_samples_on_pii_tags %} {% do return(false) %} {% endif %}
 
     {% set raw_pii_tags = elementary.get_config_var("pii_tags") %}
-    {% set pii_tags = (
-        (raw_pii_tags if raw_pii_tags is iterable else [raw_pii_tags])
-        | map("lower")
-        | list
-    ) %}
+    {% if raw_pii_tags is string %} {% set pii_tags = [raw_pii_tags | lower] %}
+    {% else %} {% set pii_tags = (raw_pii_tags or []) | map("lower") | list %}
+    {% endif %}
 
     {% set raw_model_tags = elementary.insensitive_get_dict_value(
         flattened_test, "model_tags", []
     ) %}
-    {% set model_tags = (
-        (raw_model_tags if raw_model_tags is iterable else [raw_model_tags])
-        | map("lower")
-        | list
-    ) %}
+    {% if raw_model_tags is string %} {% set model_tags = [raw_model_tags | lower] %}
+    {% else %} {% set model_tags = (raw_model_tags or []) | map("lower") | list %}
+    {% endif %}
 
     {% set intersection = elementary.lists_intersection(model_tags, pii_tags) %}
     {% set is_pii = intersection | length > 0 %}

macros/edr/system/system_utils/should_show_sample_rows.sql

@@ -5,9 +5,9 @@
   Checks three levels in order: model → test → column (test's target column only).
   Returns true if any level has a matching show_sample_rows tag.
 
-  PII precedence: if disable_samples_on_pii_tags is also enabled and the same
-  model/column has a PII tag, PII wins and this returns false. This handles the
-  edge case where both features are active simultaneously.
+  PII precedence: if disable_samples_on_pii_tags is also enabled and the model
+  or column has a PII tag, PII wins and this returns false. A model-level PII
+  tag blocks show_sample_rows at every level (model, test, and column).
 
   All tag matching is case-insensitive (tags are normalized to lowercase).
 #}
@@ -88,10 +88,11 @@
                     {% if elementary.lists_intersection(
                         col_tags, show_tags
                     ) | length > 0 %}
-                        {# PII on the column takes precedence over show_sample_rows on the same column #}
-                        {% if check_pii and elementary.lists_intersection(
-                            col_tags, pii_tags
-                        ) | length > 0 %}
+                        {# PII on the column or model takes precedence over show_sample_rows #}
+                        {% if check_pii and (
+                            elementary.lists_intersection(col_tags, pii_tags) | length > 0
+                            or elementary.lists_intersection(model_tags, pii_tags) | length > 0
+                        ) %}
                             {% do return(false) %}
                         {% endif %}
                         {% do return(true) %}