fix(context): harden relationships load + validate like views

[Bartok9]

Summary

Relationships counterpart of 9bdae39 (#2604):

• load_relationships filters to list[dict] (drop non-mapping entries)
• validate_project re-reads raw relationships.yml and reports:
  • 'relationships' must be a list, got …
  • relationship entry must be a mapping, got …

Motivation

After #2604, views report hand-edited junk; relationships silently dropped it. Same class of bad data, opposite UX.

Note on siblings

This supersedes the split approach in #2607 (reporting half only / models-field only). Downstream consumer re-guards #2606/#2608 become unnecessary once context normalizes the contract — closing those as follow-through.

Verification

cd core/wren && python -m pytest tests/unit/test_context.py -q -k relationship

Summary by CodeRabbit

• Bug Fixes
  • Improved validation for relationship configuration files.
  • Invalid relationship entries are now reported clearly instead of causing unexpected behavior.
  • Relationship collections with unsupported formats now produce validation errors.
  • Valid relationship definitions continue to load while malformed entries are safely ignored.

[coderabbitai]

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: 904eea86-e425-4d98-b21a-8cb33a5ccdfc

📥 Commits

Reviewing files that changed from the base of the PR and between f70dd32 and a54c844.

📒 Files selected for processing (1)

• core/wren/tests/unit/test_context.py

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

• core/wren/tests/unit/test_context.py

---

Walkthrough

load_relationships() now accepts mapping-root YAML with list-valued relationships and filters non-mapping entries. validate_project() reports malformed relationship collections and entries. Unit tests cover each validation case.

Changes

Relationship YAML validation

Layer / File(s)	Summary
Validate and test relationship loading core/wren/src/wren/context.py, core/wren/tests/unit/test_context.py	load_relationships() filters invalid relationship entries. validate_project() reports non-list collections and non-mapping entries. Unit tests cover these cases.

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

Possibly related PRs

• Canner/WrenAI#2494: Both changes update relationship parsing and validation for malformed YAML data.
• Canner/WrenAI#2607: Both changes update validate_project() and relationship YAML validation.

Poem

A rabbit checks each YAML line,
Keeps valid mappings neat and fine.
Bad entries raise a clear alert,
While tests protect each repaired part.
🐇 Relationships now hop safely.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	The description includes a summary and test command but omits the required failure reproduction, actual error output, and explicit duplicate check.	Add the observed failure reproduction and actual error output, then state which open PRs were checked for duplicates.
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 (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the relationship loading and validation changes in context.py.
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

• X
• Mastodon
• Reddit
• LinkedIn

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

[goldmedal]

The loader half here is right, but #2604 landed as 9bdae39 after this branch was opened and set the convention for exactly this situation — and this PR implements only half of it.

_load_views_v1 drops malformed entries silently and validate_project reports them, so a hand-edit never vanishes without the user hearing about it. The merged comment states the rule explicitly:

load_views() silently drops those (matching the other loaders), but validate_project's job is to tell the user about hand-edited mistakes rather than let them vanish quietly, so re-check the raw entries here.

Measured on main + this branch, with the same shape of bad data in each file:

views.yml — reported (9bdae39):

[ERROR] views.yml > views[0]: view entry must be a mapping, got str
[ERROR] views.yml > views[1]: view entry must be a mapping, got int

relationships.yml — silently gone:

relationships:
  - not-a-mapping
  - 42
  - {name: ok, models: [...], join_type: MANY_TO_ONE, condition: ...}

load_relationships() -> [{'name': 'ok', ...}]       # the two junk entries dropped
validate_project()   -> no message mentions them at all

Same repo, same file type, same class of bad data, opposite user experience.

What to do

#2613 and #2607 are the two halves of one change and should be a single PR — the relationships counterpart of 9bdae39:

• load_relationships filters to list[dict] (fix(context): harden relationships load + validate like views #2613 already does this)
• validate_project reports both 'relationships' must be a list and relationship entry must be a mapping, got <type> — fix(context): treat non-list relationship models as validation error #2607 currently covers only the models field, not the entry itself

Note the ordering trap: with #2613's filter in place, validate_project never sees the junk entries, so the reporting must read the raw YAML the way 9bdae39 does — not the loader output.

That pair also makes #2606 (memory/seed_queries.py) and #2608 (mcp_server.py) unnecessary: once context.py normalises relationships to list[dict], re-guarding at each consumer is exactly the downstream duplication #2613's own comment argues against ("so every consumer trusts the contract without re-filtering at each callsite"). Please close those two rather than landing three copies of one guard — the contribution bar asks for a mechanical change repeated across files to come as one PR.

Rebase

This branch is 17 commits behind main and its CI ran against a base that predates 9bdae39. It merges cleanly, but a textually clean merge is not a semantically correct one — please rebase and re-run.

[Bartok9]

Thanks @goldmedal — agreed on the #2604 pattern.

Rebased onto current main and folded both halves into this PR:

1. load_relationships → list[dict] (silent drop, same as views loaders)
2. validate_project re-checks raw YAML for non-list relationships and non-mapping entries (so junk is reported, not only filtered)

Also closing #2607 as the incomplete reporting-only half, and #2606/#2608 as redundant consumer re-guards once this lands.

[coderabbitai]

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

core/wren/tests/unit/test_context.py (1)

1761-1766: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Assert the concrete type in the validation message.

The validation contract includes the offending type. This assertion only checks the prefix, so it would pass if got str were removed.

Proposed test assertion

-    assert any("'relationships' must be a list" in m for m in msgs)
+    assert any("'relationships' must be a list, got str" in m for m in msgs)

🤖 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/tests/unit/test_context.py` around lines 1761 - 1766, Strengthen
test_validate_project_reports_relationships_not_list by asserting the complete
validation message includes both "'relationships' must be a list" and the
offending type "got str", rather than checking only the message prefix.

🤖 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/tests/unit/test_context.py`:
- Line 10: Fix the import statement in test_context.py by moving
load_relationships and validate_project inside the parenthesized from
wren.context import list, preserving the existing imported symbols and valid
Python syntax.

---

Nitpick comments:
In `@core/wren/tests/unit/test_context.py`:
- Around line 1761-1766: Strengthen
test_validate_project_reports_relationships_not_list by asserting the complete
validation message includes both "'relationships' must be a list" and the
offending type "got str", rather than checking only the message prefix.

🪄 Autofix

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: 61fcd03b-3ca1-4dde-9141-5e8da4ea159f

📥 Commits

Reviewing files that changed from the base of the PR and between 9bdae39 and f70dd32.

📒 Files selected for processing (2)

• core/wren/src/wren/context.py
• core/wren/tests/unit/test_context.py