Skip to content

fix(context): harden description checks for bad MDL rows #2616

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Bartok9 wants to merge 1 commit into
base: main
Choose a base branch
from
+50 −4
Open

fix(context): harden description checks for bad MDL rows #2616

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
26 changes: 22 additions & 4 deletions core/wren/src/wren/context.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1554,28 +1554,46 @@ def _apply_v4_to_v5(project_path: Path) -> None:


def _prop_description(item: dict) -> str | None:
return (item.get("properties") or {}).get("description")
props = item.get("properties") or {}
if not isinstance(props, dict):
return None
return props.get("description")


def _check_descriptions(manifest: dict, *, strict: bool = False) -> list[str]:
warnings: list[str] = []

for model in manifest.get("models", []):
models = manifest.get("models", []) or []
if not isinstance(models, list):
models = []
for model in models:
if not isinstance(model, dict):
continue
name = model.get("name", "<unknown>")
if not _prop_description(model):
warnings.append(
f"Model '{name}' has no description — "
"add properties.description to improve memory search and agent comprehension"
)
if strict:
for col in model.get("columns", []):
cols = model.get("columns", []) or []
if not isinstance(cols, list):
cols = []
for col in cols:
if not isinstance(col, dict):
continue
col_name = col.get("name", "<unknown>")
if not _prop_description(col):
warnings.append(
f"Column '{col_name}' in model '{name}' has no description"
)

for view in manifest.get("views", []):
views = manifest.get("views", []) or []
if not isinstance(views, list):
views = []
for view in views:
if not isinstance(view, dict):
continue
Comment on lines +1591 to +1596

@coderabbitai coderabbitai Bot Jul 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Malformed view rows can still crash the public validator.

The helper-level guards are correct, but validate_manifest processes raw views before reaching _check_descriptions. Fix the public dry-plan path and add regression coverage through that entry point.

  • core/wren/src/wren/context.py#L1591-L1596: normalize/filter malformed views before dry-planning.
  • core/wren/tests/unit/test_check_descriptions_guards.py#L4-L23: exercise validate_manifest, including malformed views and non-list collections.
📍 Affects 2 files
  • core/wren/src/wren/context.py#L1591-L1596 (this comment)
  • core/wren/tests/unit/test_check_descriptions_guards.py#L4-L23
🤖 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/context.py` around lines 1591 - 1596, Normalize and filter
raw view entries in validate_manifest before the dry-planning logic, ensuring
non-list collections and non-dictionary rows are safely ignored without
crashing. Update core/wren/tests/unit/test_check_descriptions_guards.py to cover
validate_manifest through its public entry point with malformed views and
non-list view collections.

view_name = view.get("name", "<unknown>")
if not _prop_description(view):
warnings.append(
Expand Down
28 changes: 28 additions & 0 deletions core/wren/tests/unit/test_check_descriptions_guards.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
from wren.context import _check_descriptions


def test_check_descriptions_skips_non_dict_models_and_columns():
warnings = _check_descriptions(
{
"models": [
"bad",
{
"name": "orders",
"properties": "oops",
"columns": "x",
},
{
"name": "ok",
"properties": {"description": "d"},
"columns": [None, {"name": "id"}], # missing col desc when strict
},
],
"views": ["nope", {"name": "v1"}],
},
strict=True,
)
# orders model missing desc; ok column id missing desc; v1 view missing desc
joined = "\n".join(warnings)
assert "Model 'orders'" in joined
assert "Column 'id' in model 'ok'" in joined
assert "View 'v1'" in joined
Loading