feat: derive model/test owners from dbt group owner when no direct owner is set

[arbiv]

Summary

Projects that moved ownership from model-level +owner to dbt +group config lost ownership visibility in the Elementary schema: dbt_models.owner and, downstream, dbt_tests.model_owners → elementary_test_results.owners became []. This breaks alert routing / node ownership, which read the owners field.

Root cause: owner resolution only ever read the node's direct owner:

raw_owner = meta.owner or config.owner   # never consulted the group's owner

Groups were captured (in dbt_groups + group_name columns) but their owner was never folded into owner.

This adds a group-owner fallback, following the email-first precedence already used by dbt_groups and keeping direct owner authoritative:

formatted_owner = split(meta.owner or config.owner)
if not formatted_owner and group_name:
    group_owner = get_group_owner(group_name)   # owner.email or owner.name from graph.groups
    if group_owner: formatted_owner.append(group_owner)

Because test model_owners derive from the tested model's flattened owner, this automatically flows into elementary_test_results.owners — no change needed in flatten_test.

Changes

• New macro elementary.get_group_owner(group_name) — resolves a group's owner from graph.groups, preferring owner.email, falling back to owner.name.
• flatten_model: fall back to the group owner only when no direct model/config owner exists (direct owner still wins, mirroring dbt's config precedence).
• Integration tests in test_groups.py:
  • test_model_owner_derived_from_group — grouped model, no direct owner → owner = [group email].
  • test_test_owner_derived_from_group — test on that model → model_owners = [group email].
  • test_direct_owner_takes_precedence_over_group — direct owner wins, group ignored.

Ran locally against Postgres: the 3 new tests plus existing test_groups.py / test_test_owners.py (13 total) pass.

Link to Devin session: https://app.devin.ai/sessions/7cc0b65582e44ceb9b95e5ea9559f08d
Requested by: @arbiv

---

Summary by CodeRabbit

• New Features
  • Added a new macro to look up group ownership from the dbt graph.
• Bug Fixes
  • Improved owner handling in dbt artifacts so models and tests can inherit ownership from their group when no direct owner is set.
  • Direct ownership now correctly takes precedence over group ownership when both are provided.
• Tests
  • Added coverage for owner parsing (including JSON/edge cases) and group-based ownership inheritance scenarios.

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[github-actions]

👋 @arbiv
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]

Warning

Review limit reached

You’ve reached a temporary PR review limit under our Fair Usage Limits Policy.

Your recent review volume is higher than typical usage, so adaptive limits are currently applied.

Next review available in: 42 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0390a56c-789f-4763-adbb-1b2ce7c319b6

📥 Commits

Reviewing files that changed from the base of the PR and between e0165c7 and 3c1488e.

📒 Files selected for processing (1)

• macros/utils/graph/get_group_owner.sql

📝 Walkthrough

Walkthrough

Adds a get_group_owner macro for dbt group metadata, wires it into model artifact flattening as an owner fallback, and adds integration tests covering group-based owner inheritance and direct-owner precedence.

Changes

Group owner inheritance

Layer / File(s)	Summary
get_group_owner macro macros/utils/graph/get_group_owner.sql	New macro scans graph.groups.values() for a matching group node and returns the owner's email, falling back to name, or none if not found.
Owner fallback wiring in upload_dbt_models.sql macros/edr/dbt_artifacts/upload_dbt_models.sql	flatten_model derives group_name and calls elementary.get_group_owner(group_name) when formatted_owner is empty; metadata dict reuses the derived group_name.
Integration tests for owner inheritance integration_tests/tests/test_dbt_artifacts/test_groups.py	Adds _parse_owners plus three tests verifying group-derived owners for models and tests, and direct-owner precedence over group owner.

Estimated code review effort: 3 (Moderate) | ~25 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: falling back to dbt group ownership when no direct owner is set.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/derive-owner-from-dbt-group

---

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

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🚩 Behavioral change to owner enforcement: grouped models now pass enforce_owners checks

The enforce_project_configurations macro at macros/commands/enforce_project_configurations.sql:85 checks flattened_node.owner | length == 0 to enforce that models have owners. Since flatten_model now populates the owner field from the group owner when no direct owner is set, models that previously failed this check (no direct owner) will now silently pass if they belong to a group with an owner. This may be the desired behavior (groups as an ownership mechanism), but it changes the semantics of enforce_owners from 'model must have a directly-assigned owner' to 'model must have an owner from any source'. Users relying on enforce_owners to ensure explicit per-model ownership will no longer get warnings for grouped models.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Good catch — this is a real, intended consequence. enforce_project_configurations (line 85) checks flattened_node.owner | length == 0, so grouped models with a group owner will now pass enforce_owners even without a direct +owner.

This is consistent with the goal of the change: a dbt +group owner is treated as a legitimate ownership source, so enforce_owners now means "model has an owner from any source (direct or group)" rather than "model has a directly-assigned owner". For projects that consolidated ownership into groups (the motivating use case here), that's the desired behavior.

Flagging to the repo owners: if you'd prefer enforce_owners to keep requiring a direct per-model owner, we can gate the group fallback behind a config var (e.g. derive_owner_from_group, default true) and have the enforcement check the direct owner only. Happy to add that if preferred.

[coderabbitai]

🧹 Nitpick comments (1)

integration_tests/tests/test_dbt_artifacts/test_groups.py (1)

565-729: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Consider extracting shared test setup.

The three new tests duplicate the same group-config/schema-yaml/write/run/cleanup boilerplate. Extracting a small helper (e.g., parameterized by model config, direct owner, and expected owner list/table) would reduce duplication and make future group-owner test cases cheaper to add.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@integration_tests/tests/test_dbt_artifacts/test_groups.py` around lines 565 -
729, The three new tests in test_model_owner_derived_from_group,
test_test_owner_derived_from_group, and
test_direct_owner_takes_precedence_over_group repeat the same group config,
schema YAML, model file write, dbt run, and cleanup steps. Extract the shared
setup into a small helper in the same test module, parameterized by model name,
group name, model SQL, schema config, expected table name, and expected owner
assertion, and have the three tests call that helper with their specific owner
expectations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@integration_tests/tests/test_dbt_artifacts/test_groups.py`:
- Around line 565-729: The three new tests in
test_model_owner_derived_from_group, test_test_owner_derived_from_group, and
test_direct_owner_takes_precedence_over_group repeat the same group config,
schema YAML, model file write, dbt run, and cleanup steps. Extract the shared
setup into a small helper in the same test module, parameterized by model name,
group name, model SQL, schema config, expected table name, and expected owner
assertion, and have the three tests call that helper with their specific owner
expectations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 3dd97b5a-ff92-473b-930c-ffc514a5ec4f

📥 Commits

Reviewing files that changed from the base of the PR and between 6aec58b and 0ca6bbb.

📒 Files selected for processing (3)

• integration_tests/tests/test_dbt_artifacts/test_groups.py
• macros/edr/dbt_artifacts/upload_dbt_models.sql
• macros/utils/graph/get_group_owner.sql

[elazarlachkar]

Wondering about the approach - as we sync both groups and models, perhaps we should do the fallback when using the data (in Elementary CLI)?
It will allow us to keep the "real" data as is, and we could change the fallbacks easily in the future.

[devin-ai-integration]

@elazarlachkar re: doing the fallback at consumption time (Elementary CLI/Cloud) instead of in the package —

Good question, worth deciding explicitly. The reason I put it in the package: the original request is that ownership shows up in the Elementary schema in the customer's warehouse (elementary_test_results.owners), not just in Cloud/alerts. That column is written by this package at artifact-upload time, so a consumption-time fallback wouldn't backfill the warehouse column the customer queries directly.

Your point does hold for the Cloud path (we already sync both dbt_groups and dbt_models, so a join/fallback there keeps raw data untouched and makes the policy easy to change later). The two aren't mutually exclusive — tradeoff is "materialize derived owner into the warehouse schema" (this PR) vs. "keep raw, derive on read". Happy to move it to consumption-time if we decide the warehouse owners column should stay strictly the direct owner. Flagging to @arbiv since the warehouse-schema requirement drove this approach.