Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 19 additions & 2 deletions core/wren/src/wren/memory/schema_indexer.py
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,26 @@ def _describe_column(col: dict, lines: list[str]) -> None:
lines.append("".join(parts))


def _relationship_models(rel: dict, name: str) -> list:
"""Return relationship endpoint models; raise on wrong-typed ``models``.

Missing/null ``models`` is empty. A present non-list is structural error
(same policy as :func:`_iter_section` for top-level sections).
"""
models = rel.get("models")
if models is None:
return []
if not isinstance(models, list):
raise ValueError(
f"relationship {name!r}: 'models' must be a list, "
f"got {type(models).__name__}"
)
return models


def _describe_relationship(rel: dict, lines: list[str]) -> None:
name = rel["name"]
models = rel.get("models") or []
models = _relationship_models(rel, name)
left = models[0] if len(models) > 0 else "?"
right = models[1] if len(models) > 1 else "?"
join_type = rel.get("joinType", "")
Expand Down Expand Up @@ -374,7 +391,7 @@ def _column_record(col: dict, model_name: str, mdl_h: str, now: datetime) -> dic

def _relationship_record(rel: dict, mdl_h: str, now: datetime) -> dict:
name = rel["name"]
models = rel.get("models") or []
models = _relationship_models(rel, name)
join_type = rel.get("joinType", "")
condition = rel.get("condition", "")

Expand Down
75 changes: 75 additions & 0 deletions core/wren/tests/unit/test_schema_indexer_rel_models_non_list.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
"""relationship models must be a list — wrong types raise ValueError (#2590)."""

from __future__ import annotations

import pytest

from wren.memory.schema_indexer import describe_schema, extract_schema_items


@pytest.mark.parametrize(
("bad", "got"), [({"a": 1}, "dict"), ("orders", "str"), (5, "int")]
)
def test_describe_schema_non_list_relationship_models_raises(bad, got):
with pytest.raises(
ValueError, match=rf"relationship 'r': 'models' must be a list, got {got}"
):
describe_schema({"relationships": [{"name": "r", "models": bad}]})


@pytest.mark.parametrize(
("bad", "got"), [({"a": 1}, "dict"), ("orders", "str"), (5, "int")]
)
def test_extract_schema_items_non_list_relationship_models_raises(bad, got):
with pytest.raises(
ValueError, match=rf"relationship 'r': 'models' must be a list, got {got}"
):
extract_schema_items({"relationships": [{"name": "r", "models": bad}]})


def test_string_models_do_not_emit_truncated_endpoints():
"""Regression: str models used to index first two characters as endpoints."""
m = {
"relationships": [
{
"name": "orders_customers",
"models": "orders",
"joinType": "MANY_TO_ONE",
"condition": "o.cid = c.id",
}
]
}
with pytest.raises(ValueError):
describe_schema(m)
with pytest.raises(ValueError):
extract_schema_items(m)


def test_missing_models_still_ok():
text = describe_schema({"relationships": [{"name": "r1"}]})
assert "r1" in text
items = extract_schema_items({"relationships": [{"name": "r1"}]})
assert any(i.get("item_name") == "r1" for i in items)
Comment thread
coderabbitai[bot] marked this conversation as resolved.


def test_explicit_none_models_still_ok():
"""`models:` with no value parses to None, the shape users hit by accident."""
text = describe_schema({"relationships": [{"name": "r1", "models": None}]})
assert "r1" in text
items = extract_schema_items({"relationships": [{"name": "r1", "models": None}]})
assert any(i.get("item_name") == "r1" for i in items)


def test_valid_list_models_unchanged():
text = describe_schema(
{
"relationships": [
{
"name": "r1",
"models": ["orders", "customers"],
"joinType": "MANY_TO_ONE",
}
]
}
)
assert "orders → customers" in text
Loading