Ele 5191 fix for the logic updating test result rows

[haritamar]

Summary by CodeRabbit

• New Features
  • Test result data now includes row index and test type information for each result, enabling more detailed tracking of test outcomes across different test types.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[linear]

ELE-5191 Fix for the logic updating test_result_rows

[github-actions]

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

[coderabbitai]

📝 Walkthrough

Walkthrough

The changes introduce row_index and test_type fields to test result tracking. The pop_test_result_rows macro now extracts and appends these fields to each result row, while the test_result_rows table model adds corresponding columns and removes its unique_key constraint.

Changes

Cohort / File(s)	Summary
Macro Enhancement macros/edr/tests/on_run_end/handle_tests_results.sql	Added row_index (set to loop.index) and test_type fields to result_row objects in pop_test_result_rows macro output structure
Model Schema Update models/edr/run_results/test_result_rows.sql	Removed unique_key config; added two new columns (row_index as int, test_type as string) to table schema

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

🐰 Row by row, the tests now track,
With index marks and types so clear,
Each result finds its perfect place,
No more just one unique face—
Data flows with newfound grace!

Pre-merge checks

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly references the issue (Ele 5191) and clearly describes the main change: a fix for the logic updating test result rows, which aligns with the PR's core modifications to the test result row handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

models/edr/run_results/test_result_rows.sql (1)

23-24: Address NULL values for new columns in incremental table.

The row_index and test_type columns are being added to an incremental model with on_schema_change = 'append_new_columns', causing existing rows to have NULL values for these fields.

Verification confirms:

• The insert logic in pop_test_result_rows (handle_tests_results.sql) always populates both columns for new rows
• However, historical records will retain NULL values

Ensure existing data handling is appropriate:

1. Determine if backfill is needed for existing records
2. Verify any internal queries or tools consuming this table handle NULL values correctly

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between a45c384 and 96f29dc.

📒 Files selected for processing (3)

• dbt-data-reliability
• macros/edr/tests/on_run_end/handle_tests_results.sql
• models/edr/run_results/test_result_rows.sql

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-15T06:06:05.601Z

Learnt from: arbiv
Repo: elementary-data/dbt-data-reliability PR: 861
File: macros/edr/tests/test_utils/clean_elementary_test_tables.sql:45-50
Timestamp: 2025-09-15T06:06:05.601Z
Learning: In the Elementary dbt-data-reliability codebase, the team prefers simpler, direct approaches over creating helper macros for centralized quoting logic, even when it might provide better maintainability across multiple use cases. They value code simplicity and readability over abstraction.

Applied to files:

• dbt-data-reliability

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)

• GitHub Check: test (latest_official, bigquery) / test
• GitHub Check: test (latest_official, databricks_catalog) / test
• GitHub Check: test (latest_official, athena) / test
• GitHub Check: test (latest_official, clickhouse) / test
• GitHub Check: test (fusion, bigquery) / test
• GitHub Check: test (latest_official, trino) / test
• GitHub Check: test (fusion, databricks_catalog) / test
• GitHub Check: test (fusion, snowflake) / test
• GitHub Check: test (latest_official, redshift) / test
• GitHub Check: test (fusion, redshift) / test
• GitHub Check: test (latest_pre, postgres) / test
• GitHub Check: test (latest_official, postgres) / test
• GitHub Check: test (latest_official, snowflake) / test
• GitHub Check: test (latest_official, dremio) / test

🔇 Additional comments (3)

macros/edr/tests/on_run_end/handle_tests_results.sql (2)

205-207: Schema alignment looks good.

The new fields row_index and test_type added to the result rows dictionary correctly match the schema defined in test_result_rows.sql (int and string types respectively). The use of loop.index provides a 1-based sequential index for each result row, which is appropriate for tracking row order.

---

197-213: No action required. Verification confirms that test_type is always present in test_result dictionaries. The get_dbt_test_result_row macro, which is the sole source of test result creation, explicitly includes 'test_type': elementary.get_test_type(flattened_test). The get_test_type macro always returns a value—either an elementary test type or the fallback value "dbt_test"—ensuring the field is never missing or NULL.

models/edr/run_results/test_result_rows.sql (1)

3-14: The removal of unique_key constraint requires proper deduplication logic in the insert strategy.

The unique_key = 'elementary_test_results_id' has been removed, but the insert_rows() macro performs direct SQL INSERTs without deduplication. On subsequent dbt runs, the same (elementary_test_results_id, row_index) pairs will be re-inserted, creating duplicates.

The incremental_strategy configuration (merge for some adapters) cannot prevent this since rows are inserted directly via insert_rows() rather than through dbt's incremental merge logic. Without either:

• Restoring unique_key + merge_update_columns, or
• Adding deduplication logic in insert_rows() to skip existing rows

Incremental runs risk accumulating duplicate rows. Verify that this multi-row-per-test pattern is intentional and that duplicate prevention is handled elsewhere (e.g., in the test execution layer or row generation logic).

[haritamar]

this is not for the unique key, just it's really annoying to understand if a certain row is a sample or not as it needs a join, so wanted to have it at least going forward