fix(memory): skip non-dict columns in seed query generation by Bartok9 · Pull Request #2514 · Canner/WrenAI

Bartok9 · 2026-07-16T06:04:02Z

Summary

_model_seeds skips non-dict or nameless column entries instead of TypeErroring on col["name"].
Apache-2.0 path: core/wren/**.

Motivation

Partial/hand-edited MDL column arrays can include nulls or unshaped rows. Seed generation is pure and runs during indexing; a single bad column listed should not kill the model.

Verification

cd core/wren && .venv/bin/python -m pytest tests/unit/test_seed_queries.py -v — 39 passed

Real behavior proof

test_skips_non_dict_columns PASSED (null/string/missing-name columns ignored; SUM(amount) still generated).

Summary by CodeRabbit

Bug Fixes
- Improved resilience when processing incomplete or malformed model column metadata.
- Seed query generation now safely ignores invalid column entries while continuing to generate listings and aggregations for valid columns.
Tests
- Added unit coverage to ensure seed queries are produced correctly even when a model’s columns list contains non-dictionary or unnamed entries.

Closes nothing specific — defensive MDL resilience.

coderabbitai · 2026-07-16T06:04:27Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3f50500d-c9e6-4af8-87d3-7bb3d76d19df

📥 Commits

Reviewing files that changed from the base of the PR and between a3b21f2 and 3eeeb3b.

📒 Files selected for processing (1)

core/wren/src/wren/memory/seed_queries.py

🚧 Files skipped from review as they are similar to previous changes (1)

core/wren/src/wren/memory/seed_queries.py

Walkthrough

Seed query generation filters malformed or unnamed manifest columns, uses safe name extraction for aggregation and accepted-values queries, and includes a unit test covering mixed invalid and valid metadata.

Changes

Seed query robustness

Layer / File(s)	Summary
Defensive column parsing and validation `core/wren/src/wren/memory/seed_queries.py`, `core/wren/tests/unit/test_seed_queries.py`	Column entries are filtered to valid named dictionaries, extracted names drive aggregation and accepted-values SQL generation, and tests verify valid queries remain available with malformed metadata.

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

Possibly related PRs

Canner/WrenAI#2279: Also modifies seed-query generation and manifest column handling.
Canner/WrenAI#2358: Also modifies aggregation seed selection and identifier-like column handling.

Suggested reviewers: goldmedal

Poem

I hop past columns malformed or bare,
Keeping named values safely there.
amount still sums without delay,
While valid seeds bloom along the way. 🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% 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 concisely describes the main change: skipping malformed column entries during seed query generation.
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

🧹 Nitpick comments (1)

core/wren/src/wren/memory/seed_queries.py (1)
77-88: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider pre-filtering valid columns to avoid duplicated checks.

The exact same defensive validation logic is duplicated in the second loop below (lines 124-128). You can DRY this up by pre-filtering the columns list once before the loops.
♻️ Proposed refactor
    valid_columns = [
        c for c in columns 
        if isinstance(c, dict) and c.get("name")
    ]
Then, you can simply iterate over valid_columns in both loops and safely remove the isinstance and col_name checks from the loop bodies.
🤖 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 `@core/wren/src/wren/memory/seed_queries.py` around lines 77 - 88, Pre-filter
the columns once into a valid_columns collection containing only dictionary
entries with a non-empty name, then iterate over it in both loops. Update the
current loop and the second loop to use valid_columns and remove their
duplicated isinstance and col_name checks, preserving the existing normalization
and column-processing behavior.

🤖 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 `@core/wren/src/wren/memory/seed_queries.py`:
- Around line 77-88: Pre-filter the columns once into a valid_columns collection
containing only dictionary entries with a non-empty name, then iterate over it
in both loops. Update the current loop and the second loop to use valid_columns
and remove their duplicated isinstance and col_name checks, preserving the
existing normalization and column-processing behavior.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 63556fb2-76ca-412b-9a69-1c1e948cd6e0

📥 Commits

Reviewing files that changed from the base of the PR and between 82ad7a1 and 14de044.

📒 Files selected for processing (2)

core/wren/src/wren/memory/seed_queries.py
core/wren/tests/unit/test_seed_queries.py

CI lint failed on ruff format --check for seed_queries.py.

Address CodeRabbit nitpick: DRY up the duplicated isinstance/name validation by pre-filtering columns into a single valid-columns list used by both the aggregation and accepted-values loops.

Bartok9 · 2026-07-16T08:16:03Z

Thanks @coderabbitai — pre-filtered columns into a single valid_columns list used by both loops, removing the duplicated isinstance/name checks. 39 unit tests pass.

coderabbitai · 2026-07-16T08:16:17Z

Tip

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

@Bartok9 Thanks for the update — pre-filtering into a single valid_columns list is a cleaner approach and removes the duplication nicely. Glad all 39 tests pass!

ヽ(•‿•)ノ

goldmedal

Thanks @Bartok9 👍

fix(memory): skip non-dict columns in seed query generation

14de044

Closes nothing specific — defensive MDL resilience.

github-actions Bot added python Pull requests that update Python code core labels Jul 16, 2026

coderabbitai Bot reviewed Jul 16, 2026

View reviewed changes

style(memory): ruff format seed_queries non-dict guard

37f139b

CI lint failed on ruff format --check for seed_queries.py.

coderabbitai Bot mentioned this pull request Jul 16, 2026

fix(memory): skip non-dict columns in schema_indexer #2515

Closed

refactor(memory): pre-filter valid columns once in seed query generation

a3b21f2

Address CodeRabbit nitpick: DRY up the duplicated isinstance/name validation by pre-filtering columns into a single valid-columns list used by both the aggregation and accepted-values loops.

style: ruff format seed_queries

3eeeb3b

This was referenced Jul 19, 2026

fix(memory): skip non-dict models/columns in seed query generation #2542

Closed

fix(dbt): skip unusable models/columns in seed payloads #2549

Closed

goldmedal approved these changes Jul 22, 2026

View reviewed changes

goldmedal merged commit a3de389 into Canner:main Jul 22, 2026
10 checks passed

github-actions Bot mentioned this pull request Jul 22, 2026

chore(main): release wren 0.13.2 #2554

Merged

This was referenced Jul 25, 2026

fix(genbi): treat blank provider tokens as missing #2584

Closed

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

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(memory): skip non-dict columns in seed query generation - #2514

fix(memory): skip non-dict columns in seed query generation#2514
goldmedal merged 4 commits into
Canner:mainfrom
Bartok9:fix/seed-queries-skip-non-dict-columns

Bartok9 commented Jul 16, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Bartok9 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Uh oh!

goldmedal left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

Bartok9 commented Jul 16, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Motivation

Verification

Real behavior proof

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Jul 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Bartok9 commented Jul 16, 2026

Uh oh!

coderabbitai Bot commented Jul 16, 2026

Uh oh!

goldmedal left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Bartok9 commented Jul 16, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Jul 16, 2026 •

edited

Loading