diff --git a/core/wren/src/wren/memory/schema_indexer.py b/core/wren/src/wren/memory/schema_indexer.py index 7e28cd44fa..2e6ae62335 100644 --- a/core/wren/src/wren/memory/schema_indexer.py +++ b/core/wren/src/wren/memory/schema_indexer.py @@ -51,13 +51,16 @@ def describe_schema(manifest: dict) -> str: lines.append("") for model in manifest.get("models", []): - _describe_model(model, lines) + if isinstance(model, dict) and model.get("name"): + _describe_model(model, lines) for rel in manifest.get("relationships", []): - _describe_relationship(rel, lines) + if isinstance(rel, dict): + _describe_relationship(rel, lines) for view in manifest.get("views", []): - _describe_view(view, lines) + if isinstance(view, dict) and view.get("name"): + _describe_view(view, lines) cubes = manifest.get("cubes", []) or [] if isinstance(cubes, list): @@ -87,7 +90,9 @@ def _describe_model(model: dict, lines: list[str]) -> None: if data_scope: lines.append(f" Data scope: {data_scope}") - cols = model.get("columns", []) + cols = [ + c for c in (model.get("columns") or []) if isinstance(c, dict) and c.get("name") + ] if cols: lines.append(" Columns:") for col in cols: @@ -96,7 +101,9 @@ def _describe_model(model: dict, lines: list[str]) -> None: def _describe_column(col: dict, lines: list[str]) -> None: - name = col["name"] + name = col.get("name") + if not name: + return dtype = col.get("type", "?") parts = [f" - {name} ({dtype})"] @@ -229,14 +236,22 @@ def extract_schema_items(manifest: dict) -> list[dict]: items: list[dict] = [] for model in manifest.get("models", []): + if not isinstance(model, dict) or not model.get("name"): + continue items.append(_model_record(model, mdl_h, now)) - for col in model.get("columns", []): + for col in model.get("columns", []) or []: + if not isinstance(col, dict) or not col.get("name"): + continue items.append(_column_record(col, model["name"], mdl_h, now)) for rel in manifest.get("relationships", []): + if not isinstance(rel, dict): + continue items.append(_relationship_record(rel, mdl_h, now)) for view in manifest.get("views", []): + if not isinstance(view, dict): + continue items.append(_view_record(view, mdl_h, now)) cubes = manifest.get("cubes", []) or [] @@ -264,7 +279,9 @@ def extract_schema_items(manifest: dict) -> list[dict]: def _model_record(model: dict, mdl_h: str, now: datetime) -> dict: name = model["name"] - cols = model.get("columns", []) + cols = [ + c for c in (model.get("columns") or []) if isinstance(c, dict) and c.get("name") + ] col_summaries = ", ".join(f"{c['name']} ({c.get('type', '?')})" for c in cols[:20]) pk = model.get("primaryKey") or "" @@ -297,6 +314,7 @@ def _model_record(model: dict, mdl_h: str, now: datetime) -> dict: def _column_record(col: dict, model_name: str, mdl_h: str, now: datetime) -> dict: + # Caller (extract_schema_items) already guarantees a truthy name. name = col["name"] dtype = col.get("type", "") expr = col.get("expression") or None diff --git a/core/wren/tests/unit/test_schema_indexer_malformed_columns.py b/core/wren/tests/unit/test_schema_indexer_malformed_columns.py new file mode 100644 index 0000000000..0209af289f --- /dev/null +++ b/core/wren/tests/unit/test_schema_indexer_malformed_columns.py @@ -0,0 +1,88 @@ +"""schema_indexer must skip non-dict / nameless columns.""" + +from __future__ import annotations + +from wren.memory.schema_indexer import describe_schema, extract_schema_items + + +def test_describe_schema_skips_non_dict_columns(): + text = describe_schema( + { + "models": [ + { + "name": "orders", + "columns": [None, {"name": "amount", "type": "int"}, {"type": "x"}], + } + ] + } + ) + assert "amount" in text + # Exactly one column entry rendered (the malformed ones are dropped). + assert text.count(" - ") == 1 + + +def test_describe_schema_omits_dangling_columns_header(): + # When every column is malformed, no dangling " Columns:" header (589e0bb). + text = describe_schema( + {"models": [{"name": "orders", "columns": [None, {"type": "x"}]}]} + ) + assert "Columns:" not in text + assert "### Model: orders" in text + + +def test_describe_schema_skips_empty_string_name_and_null_columns(): + text = describe_schema( + { + "models": [ + {"name": "a", "columns": [{"name": "", "type": "int"}]}, + {"name": "b", "columns": None}, + ] + } + ) + assert "### Model: a" in text + assert "### Model: b" in text + assert " - " not in text + + +def test_describe_schema_skips_null_model_relationship_view_slots(): + # Null slots in hand-edited manifests must not crash. + text = describe_schema({"models": [None], "relationships": [None], "views": [None]}) + assert isinstance(text, str) + + +def test_extract_schema_items_skips_null_model_slots(): + items = extract_schema_items({"models": [None, {"columns": []}]}) + assert items == [] + + +def test_extract_schema_items_skips_scalar_and_empty_name_columns(): + items = extract_schema_items( + { + "models": [ + { + "name": "orders", + "columns": ["amount", {"name": "", "type": "int"}], + } + ] + } + ) + col_items = [i for i in items if i["item_type"] == "column"] + assert col_items == [] + + +def test_extract_schema_items_skips_non_dict_columns(): + items = extract_schema_items( + { + "models": [ + { + "name": "orders", + "columns": [None, {"name": "amount", "type": "int"}, {"type": "x"}], + } + ] + } + ) + col_items = [i for i in items if i["item_type"] == "column"] + assert len(col_items) == 1 + assert col_items[0]["item_name"] == "amount" + model_items = [i for i in items if i["item_type"] == "model"] + assert "amount" in model_items[0]["text"]