Skip to content

fix(skills): wrap generate-mdl SKILL.md's relationships.yml example under the required key - #2676

Merged
goldmedal merged 1 commit into
Canner:mainfrom
AmirF194:fix/2672-generate-mdl-skill-relationships-example
Aug 26, 2026
Merged

fix(skills): wrap generate-mdl SKILL.md's relationships.yml example under the required key#2676
goldmedal merged 1 commit into
Canner:mainfrom
AmirF194:fix/2672-generate-mdl-skill-relationships-example

Conversation

@AmirF194

@AmirF194 AmirF194 commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Root cause

generate-mdl's SKILL.md, Step 3 — Write relationships, showed:

# relationships.yml
- name: orders_customers
  models:
    - orders
    - customers
  join_type: many_to_one
  condition: "orders.customer_id = customers.customer_id"

a bare top-level YAML list. load_relationships() (core/wren/src/wren/context.py)
only reads a top-level relationships: mapping key:

data = yaml.safe_load(rel_file.read_text(encoding="utf-8")) or {}
rels = data.get("relationships") if isinstance(data, dict) else None
if not isinstance(rels, list):
    return []

so an agent following the doc verbatim ends up with a relationships.yml
that loads as zero relationships. validate_project() does report a clear
relationships.yml must be a mapping with a 'relationships' key, got list
error for this exact shape (and by default wren build runs validation
first and aborts on it), so the failure today is loud rather than silent —
but the doc's own example has never actually matched what the loader
expects.

Fix

Wrap the example under the relationships: key, matching what
load_relationships/validate_project require:

# relationships.yml
relationships:
  - name: orders_customers
    models:
      - orders
      - customers
    join_type: many_to_one
    condition: "orders.customer_id = customers.customer_id"

Verification

  • New regression test (test_generate_mdl_skill_step3_example_round_trips
    in tests/unit/test_context.py) reads the skill's own served content via
    wren.skills_delivery.get_skill("generate-mdl"), extracts the Step 2/Step 3
    YAML blocks, and feeds them through load_relationships/validate_project.
    Fails on unmodified main (load_relationships returns 0 relationships
    instead of 1); passes with this fix.
  • Full tests/unit/ (excl. test_memory.py/test_mcp_server.py, matching
    this repo's test-unit CI job): 1146 passed, 2 skipped, no regressions.
  • ruff format --check src/ / ruff check src/ clean.
  • Not verified: an end-to-end wren context init && wren build run against a
    live database: this is a pure-doc/test change and doesn't touch runtime code
    paths beyond what the new test already exercises directly.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected the MDL generation workflow’s relationships YAML structure to ensure relationship definitions are recognized properly.
  • Tests

    • Added regression coverage to verify relationship loading and project validation for generated v2 projects.

…nder the required key

Step 3 of the generate-mdl skill showed a bare top-level YAML list for
relationships.yml. load_relationships() only reads a top-level
`relationships:` mapping key, so a bare list silently loads as zero
relationships; validate_project() (run by default before `wren build`)
does catch it with an explicit error, but only because a fail-loud guard
was added for hand-edited files after this example was written, not
because the example itself was ever correct.

Fixes Canner#2672
@github-actions github-actions Bot added documentation Improvements or additions to documentation python Pull requests that update Python code core labels Aug 17, 2026
@coderabbitai

coderabbitai Bot commented Aug 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 Plus

Run ID: 24389265-8162-4bb6-9bf2-a4b2fb8ca364

📥 Commits

Reviewing files that changed from the base of the PR and between 28920e7 and bc8a2c9.

📒 Files selected for processing (2)
  • core/wren/src/wren/skills_content/generate-mdl/SKILL.md
  • core/wren/tests/unit/test_context.py

Included review availability: Your plan includes up to 10 reviews per rolling hour; 9 remain after this review.


Walkthrough

The MDL generation skill now nests relationship definitions under relationships. A regression test extracts the documented YAML examples and verifies relationship loading and project validation.

Changes

MDL relationship validation

Layer / File(s) Summary
Correct relationship shape and regression validation
core/wren/src/wren/skills_content/generate-mdl/SKILL.md, core/wren/tests/unit/test_context.py
The relationship example now includes the top-level relationships key. The regression test loads the documented model and relationship YAML and verifies one relationship with no validation errors.

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

Merge Risk: ⚪ Minimal · up to bc8a2

The example now matches the required relationships.yml format, preventing users from following the documentation into a validation failure. No actionable merge-blocking risk remains after normal checks and review.

Possibly related issues

  • Canner/WrenAI#2672 — Addresses the same relationship YAML nesting issue and adds regression coverage.

Suggested reviewers: goldmedal

Poem

A rabbit found YAML tucked in the wrong place,
So nested relationships now keep their proper space.
Tests load the models, validation turns green,
One link is confirmed in the project machine.
Hop, hop—MDL flows clean and bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 75.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
Title check ✅ Passed The title clearly and concisely describes the primary documentation fix.
Description check ✅ Passed The description explains the root cause, fix, observed error, regression test, and verification results; only the duplicate-check section is missing.
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.

@AmirF194

Copy link
Copy Markdown
Contributor Author

Checking in, no pressure. Let me know if anything here needs rework or more context.

@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 @AmirF194, nice catch 👍

@goldmedal
goldmedal merged commit a8e8d04 into Canner:main Aug 26, 2026
11 checks passed
@AmirF194

Copy link
Copy Markdown
Contributor Author

Thanks for the quick merge!

@AmirF194
AmirF194 deleted the fix/2672-generate-mdl-skill-relationships-example branch August 26, 2026 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core documentation Improvements or additions to documentation python Pull requests that update Python code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants