docs(code): add explanatory comments to show_sample_rows and PII sampling macros

Author: joostboon

macros/edr/materializations/test/test.sql

@@ -75,8 +75,19 @@
         {% set disable_test_samples = flattened_test["meta"]["disable_test_samples"] %}
     {% endif %}
 
+    {#
+      Sampling control precedence (highest to lowest):
+      1. disable_test_samples meta flag — explicit per-test kill switch, always wins.
+      2. show_sample_rows tag (model/test/column) — opt-in when
+         enable_samples_on_show_sample_rows_tags is true. If the tag is present,
+         skip all further checks and keep the sample_limit.
+      3. enable_samples_on_show_sample_rows_tags — hide-by-default mode: if the
+         feature is on but no show_sample_rows tag was found, disable samples.
+      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 %}
-    {% elif elementary.should_show_sample_rows(flattened_test) %}  {# show_sample_rows tag overrides default-hide #}
+    {% elif elementary.should_show_sample_rows(flattened_test) %}
     {% elif elementary.get_config_var("enable_samples_on_show_sample_rows_tags") %}
         {% set sample_limit = 0 %}
     {% elif elementary.is_pii_table(flattened_test) %} {% set sample_limit = 0 %}

macros/edr/system/system_utils/get_pii_columns_from_parent_model.sql

@@ -38,6 +38,12 @@
     {% set column_nodes = parent_model.get("columns") %}
     {% if not column_nodes %} {% do return(pii_columns) %} {% endif %}
 
+    {#
+      A column tagged show_sample_rows (without pii) should still appear in samples
+      even when disable_samples_on_pii_tags is active — it is intentionally opted in.
+      We only skip it from the PII columns list if it does NOT also carry a PII tag,
+      since PII always takes precedence over show_sample_rows.
+    #}
     {% set enable_show_tags = elementary.get_config_var(
         "enable_samples_on_show_sample_rows_tags"
     ) %}
@@ -49,7 +55,7 @@
     {% for column_node in column_nodes.values() %}
         {% set all_column_tags_lower = elementary.get_column_tags(column_node) %}
 
-        {# PII takes precedence: only skip if show_sample_rows is set and pii is not #}
+        {# 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(

macros/edr/system/system_utils/is_pii_test.sql

@@ -1,3 +1,8 @@
+{#
+  Complements is_pii_table (model-level) and should_disable_sampling_for_pii
+  (column-level) by adding test-level PII tag support. A test tagged with a PII
+  tag will have its samples disabled, consistent with the other two levels.
+#}
 {% macro is_pii_test(flattened_test) %}
     {% if not elementary.get_config_var("disable_samples_on_pii_tags") %}
         {% do return(false) %}

macros/edr/system/system_utils/should_show_sample_rows.sql

@@ -1,3 +1,16 @@
+{#
+  Inverse of PII protection: when enable_samples_on_show_sample_rows_tags is true,
+  samples are hidden by default and only shown when the show_sample_rows tag is present.
+
+  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.
+
+  All tag matching is case-insensitive (tags are normalized to lowercase).
+#}
 {% macro should_show_sample_rows(flattened_test) %}
     {% if not elementary.get_config_var("enable_samples_on_show_sample_rows_tags") %}
         {% do return(false) %}
@@ -8,7 +21,10 @@
     {% else %} {% set show_tags = (raw_show_tags or []) | map("lower") | list %}
     {% endif %}
 
-    {# Resolve PII tags once — only relevant when PII hiding is also enabled #}
+    {#
+      Resolve PII tags once upfront. We use `is string` (not `is iterable`) because
+      strings are iterable in Jinja — iterating a string gives individual characters.
+    #}
     {% set check_pii = elementary.get_config_var("disable_samples_on_pii_tags") %}
     {% if check_pii %}
         {% set raw_pii_tags = elementary.get_config_var("pii_tags") %}
@@ -18,14 +34,15 @@
     {% else %} {% set pii_tags = [] %}
     {% endif %}
 
-    {# Model-level check #}
+    {# Model-level: show_sample_rows on the model applies to all its tests #}
     {% set raw_model_tags = elementary.insensitive_get_dict_value(
         flattened_test, "model_tags", []
     ) %}
     {% 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 %}
     {% if elementary.lists_intersection(model_tags, show_tags) | length > 0 %}
+        {# PII on the model takes precedence over show_sample_rows on the same model #}
         {% if check_pii and elementary.lists_intersection(
             model_tags, pii_tags
         ) | length > 0 %}
@@ -34,14 +51,15 @@
         {% do return(true) %}
     {% endif %}
 
-    {# Test-level check #}
+    {# Test-level: show_sample_rows on the test definition itself #}
     {% set raw_test_tags = elementary.insensitive_get_dict_value(
         flattened_test, "tags", []
     ) %}
     {% if raw_test_tags is string %} {% set test_tags = [raw_test_tags | lower] %}
     {% else %} {% set test_tags = (raw_test_tags or []) | map("lower") | list %}
     {% endif %}
     {% if elementary.lists_intersection(test_tags, show_tags) | length > 0 %}
+        {# If the model itself is PII-tagged, respect that even for test-level overrides #}
         {% if check_pii and elementary.lists_intersection(
             model_tags, pii_tags
         ) | length > 0 %}
@@ -50,7 +68,10 @@
         {% do return(true) %}
     {% endif %}
 
-    {# Column-level check: only the test's target column #}
+    {#
+      Column-level: only checks the specific column the test targets (test_column_name),
+      not all columns on the model. This avoids showing samples for unrelated columns.
+    #}
     {% set test_column_name = elementary.insensitive_get_dict_value(
         flattened_test, "test_column_name"
     ) %}
@@ -67,6 +88,7 @@
                     {% 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 %}