Skip to content

fix(dbt): tolerate non-dict column rows in import helpers - #2618

Open
Bartok9 wants to merge 2 commits into
Canner:mainfrom
Bartok9:fix/dbt-find-column-row-guards
Open

fix(dbt): tolerate non-dict column rows in import helpers#2618
Bartok9 wants to merge 2 commits into
Canner:mainfrom
Bartok9:fix/dbt-find-column-row-guards

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What failure does this repair?

_convert_dbt_artifacts raised AttributeError: 'str' object has no attribute 'get' when a dbt manifest.json model carried a non-list columns value or a list containing non-dict rows (e.g. a bare string). Repro: import a dbt project whose model has "columns": ["id"] — the helper called .get() on the string and crashed the import.

Summary

dbt import helpers crashed when model.columns was non-list or held non-dict rows. _find_column, _finalize_column_tests, and _seed_model_payload now skip malformed model and column entries.

Verification

pytest tests/unit/test_dbt_column_guards.py -q
# 3 passed

Duplicate check

Searched open/closed PRs and existing dbt guard coverage; #2514 and #2549 harden the seed/query path but not these import helpers (_find_column, _finalize_column_tests). No overlap with the row-shape guards added here.

Summary by CodeRabbit

  • Bug Fixes

    • Improved resilience when processing malformed or unexpected dbt model and column metadata.
    • Invalid entries are now safely skipped instead of interrupting artifact conversion.
    • Seeded column output now includes only valid, named columns.
    • Valid dbt test and primary-key metadata continues to be processed correctly.
  • Tests

    • Added coverage for malformed model and column data handling.

_find_column, finalize column tests, and seed payload builders assumed
columns were always dicts in a list. Skip bad rows so YAML drift cannot
crash dbt→MDL conversion.
@github-actions github-actions Bot added python Pull requests that update Python code core labels Jul 29, 2026
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 14cd5251-9f39-4a9b-802f-9156fde38ab6

📥 Commits

Reviewing files that changed from the base of the PR and between 40d6a56 and 0af74ff.

📒 Files selected for processing (1)
  • core/wren/src/wren/dbt.py

Walkthrough

dbt artifact conversion now validates model and column structures before processing them. New unit tests cover malformed entries, valid mixed inputs, dbt test metadata, primary-key derivation, and seed payload filtering.

Changes

dbt artifact conversion guards

Layer / File(s) Summary
Guard dbt conversion helpers
core/wren/src/wren/dbt.py
_find_column, _finalize_column_tests, and _seed_model_payload skip malformed model and column entries.
Test malformed dbt inputs
core/wren/tests/unit/test_dbt_column_guards.py
Tests cover invalid column filtering, dbt test metadata, primary-key derivation, and seed payload defaults.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • Canner/WrenAI#2514: Adds similar defensive filtering for malformed columns during seed query generation.
  • Canner/WrenAI#2549: Hardens _seed_model_payload and adds overlapping unit coverage.

Suggested reviewers: goldmedal

Poem

I’m a bunny guarding columns with care,
Skipping odd shapes that shouldn’t be there.
Tests hop through seeds and keys,
Metadata lands with ease—
Safe dbt paths bloom everywhere!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately summarizes the dbt import helper hardening against malformed column rows.
Description check ✅ Passed The description covers summary, observed failure with reproduction/error, verification, and duplicate check details.
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

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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.

Inline comments:
In `@core/wren/src/wren/dbt.py`:
- Around line 1294-1295: Normalize the model’s columns value before the
comprehension iterates it: in the columns-processing logic, retain the provided
value only when it is a list and use an empty list for all other values,
including truthy scalars such as integers. Preserve the existing dictionary and
non-null name filter.
- Around line 1152-1154: Update the column guard in the primary-key derivation
loop to also skip dictionaries whose "name" value is None, before any
column["name"] access. Preserve processing for dictionary columns with a
non-None name.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d4a1a0c-d3ba-4571-a61b-2e6fdb52910c

📥 Commits

Reviewing files that changed from the base of the PR and between 99f1dbd and 40d6a56.

📒 Files selected for processing (2)
  • core/wren/src/wren/dbt.py
  • core/wren/tests/unit/test_dbt_column_guards.py

Comment thread core/wren/src/wren/dbt.py
Comment thread core/wren/src/wren/dbt.py Outdated
@Bartok9

Bartok9 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @coderabbitai — both points addressed in 0af74ff: the primary-key loop now skips columns whose name is None before any access, and the seed payload comprehension normalizes a non-list columns value to an empty list. Existing guard tests still pass.

@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@Bartok9 Thanks for addressing both cases and confirming the guard tests still pass.

🐇 ✅


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant