Skip to content

fix: populate results_sample when dbt sample_data is dict-shaped#2271

Open
orenniapete wants to merge 1 commit into
elementary-data:masterfrom
orenniapete:fix/2269-missing-example-rows
Open

fix: populate results_sample when dbt sample_data is dict-shaped#2271
orenniapete wants to merge 1 commit into
elementary-data:masterfrom
orenniapete:fix/2269-missing-example-rows

Conversation

@orenniapete

@orenniapete orenniapete commented Jun 10, 2026

Copy link
Copy Markdown

Fixes #2269

Problem

The EDR report shows failure row counts but no example rows for dbt_test failures, even though rows exist in dbt_test__audit and test_result_rows.

Root cause

TestsAPI._get_test_result_from_test_result_db_row hard-cast sample_data to list. Some dbt-core / elementary-dbt-package version combinations (confirmed with dbt-core 1.11.5 + elementary 0.22.0 vs 0.24.0) return sample rows wrapped in a dict such as {"rows": [...]} instead of as a bare list. The cast silently produced None, leaving results_sample empty in the serialized report payload, so the report UI had nothing to render.

Fix

Added TestsAPI._normalize_results_sample() which handles both payload shapes:

  • list → passed through unchanged (no behaviour change for existing users)
  • dict with a known wrapper key (rows, sample_rows, results_sample, data) → the rows list is extracted

disable_samples is fully respected in both cases.

Files changed

File Change
elementary/monitor/api/tests/tests.py Add _normalize_results_sample(); replace hard cast(list) with it
tests/mocks/fetchers/tests_fetcher_mock.py Add two mock dbt rows — one with list sample_data, one with dict sample_data
tests/unit/monitor/api/tests/test_tests_api.py 13 new tests covering the normalizer, result builder, and full serialization path

All 15 tests in the file pass.

Summary by CodeRabbit

  • Bug Fixes

    • Enhanced test result sample data handling to support multiple input formats, fixing an issue where certain sample data was not properly displayed in test results.
  • Tests

    • Added comprehensive test coverage for sample data normalization, including tests for multiple data format scenarios and edge cases.

Some dbt-core / elementary-dbt-package version combinations return
sample rows wrapped in a dict (e.g. {"rows": [...]}) instead of as a
bare list. The previous code used a hard cast to list which silently
left results_sample empty, so the EDR report showed failure counts but
no example rows.

Fix: add TestsAPI._normalize_results_sample() that accepts both shapes
and extract the rows list from common dict wrapper keys (rows,
sample_rows, results_sample, data). The fix is backward-compatible –
list payloads pass through unchanged and disable_samples is still
respected.

Tests:
- Unit tests for _normalize_results_sample with list, dict (all keys),
  None, and unknown dict
- Unit tests for _get_test_result_from_test_result_db_row covering list
  sample, dict sample, None sample, and disable_samples suppression
- Integration test through get_test_results() verifying the full
  serialization path populates results_sample from both payload shapes

Fixes elementary-data#2269
@orenniapete orenniapete requested a deployment to elementary_test_env June 10, 2026 16:21 — with GitHub Actions Waiting
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3b8ef7b8-48c7-42f1-b11d-66cd87cf3bd7

📥 Commits

Reviewing files that changed from the base of the PR and between 9cc84dd and 0537d7b.

📒 Files selected for processing (3)
  • elementary/monitor/api/tests/tests.py
  • tests/mocks/fetchers/tests_fetcher_mock.py
  • tests/unit/monitor/api/tests/test_tests_api.py

📝 Walkthrough

Walkthrough

This PR fixes issue #2269 by adding a _normalize_results_sample helper that handles sample_data payloads arriving in either list or dict-wrapped formats (with keys like rows, sample_rows, results_sample, or data). The fix is integrated into test result processing and covered by comprehensive unit and integration tests using mock data in both formats.

Changes

Sample Data Normalization

Layer / File(s) Summary
Sample data normalization method and integration
elementary/monitor/api/tests/tests.py
Adds _normalize_results_sample helper that accepts sample_data as a list or unwraps it from dict wrappers via recognized keys, returning None for unrecognized shapes. Integrates the helper into _get_test_result_from_test_result_db_row to replace direct casting, and removes the unused cast import from typing.
Unit and integration tests with mock data
tests/mocks/fetchers/tests_fetcher_mock.py, tests/unit/monitor/api/tests/test_tests_api.py
Adds mock test rows with list-shaped and dict-wrapped sample_data formats; introduces _make_dbt_row helper for test construction; validates _normalize_results_sample behavior across list, dict, None, and unknown-dict cases; tests results_sample population and suppression in _get_test_result_from_test_result_db_row; includes end-to-end integration test verifying get_test_results output includes sample rows from dict-shaped payloads.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested reviewers

  • ofek1weiss

Poem

🐰 A rabbit hops through dict and list,
Unwrapping samples never missed,
Rows and data, wrapped so tight,
Now normalized, all feeling right,
Sample data shines in test reports bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 52.94% 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 clearly and specifically describes the main change: fixing population of results_sample when dbt sample_data is in dict shape rather than list form.
Linked Issues check ✅ Passed The PR directly addresses issue #2269 by adding normalization logic to handle both list and dict-shaped sample_data, restoring sample rows display in EDR reports as required.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing the sample_data handling issue: the normalizer function, mock test rows, and comprehensive unit tests all align with issue #2269 objectives.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.


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.

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.

EDR report no longer shows sample rows for failures and warnings

1 participant