Skip to content

fix(pydantic): skip non-dict models in list_models - #2522

Merged
goldmedal merged 1 commit into
Canner:mainfrom
Bartok9:fix/pydantic-list-models-non-dict
Jul 23, 2026
Merged

fix(pydantic): skip non-dict models in list_models#2522
goldmedal merged 1 commit into
Canner:mainfrom
Bartok9:fix/pydantic-list-models-non-dict

Conversation

@Bartok9

@Bartok9 Bartok9 commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • _run_list_models assumed every models[] element was a dict with name/columns/properties.
  • Skip non-dict rows and nameless models; coerce non-list columns and non-dict properties.

Motivation

Malformed MDL fragments should not AttributeError wren_list_models mid-comprehension. Apache-2.0: sdk/**.

Verification

Logic exercised with an in-process fixture reproducing non-dict/name-less rows (ok model kept with column_count 0). Full pydantic-ai env not on this host; unit test added for CI:

  • tests/unit/test_tools_runtime.py::test_wren_list_models_skips_non_dict_and_nameless

Test plan

  • Local behavioral proof + regression test
  • CI green

Summary by CodeRabbit

  • Bug Fixes

    • Improved model listing resilience when metadata is incomplete or malformed.
    • Invalid or unnamed models are now skipped instead of causing failures.
    • Missing or incorrectly formatted fields receive safe default values.
  • Tests

    • Added coverage for partially invalid model metadata and expected fallback behavior.

Guard wren_list_models against non-list models, non-dict entries, and
missing names so partial MDL manifests cannot crash the tool.
@coderabbitai

coderabbitai Bot commented Jul 17, 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

Run ID: 95c32934-e8a6-4e2a-a47f-c1c102581203

📥 Commits

Reviewing files that changed from the base of the PR and between 243eec8 and c6a44fc.

📒 Files selected for processing (2)
  • sdk/wren-pydantic/src/wren_pydantic/_tools.py
  • sdk/wren-pydantic/tests/unit/test_tools_runtime.py

Walkthrough

The model-listing runtime path now validates manifest structure, skips invalid or nameless entries, normalizes field types, and safely derives descriptions. A unit test covers malformed entries and default values.

Changes

Model manifest parsing

Layer / File(s) Summary
Defensive model listing
sdk/wren-pydantic/src/wren_pydantic/_tools.py, sdk/wren-pydantic/tests/unit/test_tools_runtime.py
_run_list_models validates and normalizes manifest entries, while the runtime test verifies invalid entries are skipped and missing values receive safe defaults.

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

Poem

A bunny found a crooked chart,
And checked each model part by part.
Bad names hopped away from sight,
Safe defaults made the list just right.
“Now parse with care!” it cheered tonight.

🚥 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 accurately reflects the main change: skipping non-dict models in list_models.
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.

@goldmedal goldmedal left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks @Bartok9

@goldmedal
goldmedal merged commit 507bda6 into Canner:main Jul 23, 2026
8 checks passed
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.

2 participants