From 4e99347f1fa85eef31c7078cca2f357e2e58ad0a Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Thu, 16 Jul 2026 02:04:33 -0400 Subject: [PATCH 1/5] fix(memory): skip non-dict columns in schema_indexer --- core/wren/src/wren/memory/schema_indexer.py | 16 ++++++-- .../test_schema_indexer_malformed_columns.py | 38 +++++++++++++++++++ 2 files changed, 50 insertions(+), 4 deletions(-) create mode 100644 core/wren/tests/unit/test_schema_indexer_malformed_columns.py diff --git a/core/wren/src/wren/memory/schema_indexer.py b/core/wren/src/wren/memory/schema_indexer.py index 7e28cd44fa..b8f8ded21b 100644 --- a/core/wren/src/wren/memory/schema_indexer.py +++ b/core/wren/src/wren/memory/schema_indexer.py @@ -91,12 +91,16 @@ def _describe_model(model: dict, lines: list[str]) -> None: if cols: lines.append(" Columns:") for col in cols: + if not isinstance(col, dict): + continue _describe_column(col, lines) lines.append("") 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})"] @@ -230,7 +234,9 @@ def extract_schema_items(manifest: dict) -> list[dict]: for model in manifest.get("models", []): 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", []): @@ -264,7 +270,7 @@ 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,7 +303,9 @@ 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: - name = col["name"] + name = col.get("name") or "" + if not name: + raise ValueError("column record requires name") dtype = col.get("type", "") expr = col.get("expression") or None is_calc = col.get("isCalculated", False) 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..0a8b8c4c82 --- /dev/null +++ b/core/wren/tests/unit/test_schema_indexer_malformed_columns.py @@ -0,0 +1,38 @@ +"""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 + assert "None" not in text + + +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"] From 5bd34dd098496b359437bb3518eea47a66d6a99f Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Thu, 16 Jul 2026 04:13:38 -0400 Subject: [PATCH 2/5] style(memory): ruff format schema_indexer non-dict guard CI lint failed on ruff format --check for schema_indexer.py. --- core/wren/src/wren/memory/schema_indexer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/wren/src/wren/memory/schema_indexer.py b/core/wren/src/wren/memory/schema_indexer.py index b8f8ded21b..9d815d7779 100644 --- a/core/wren/src/wren/memory/schema_indexer.py +++ b/core/wren/src/wren/memory/schema_indexer.py @@ -270,7 +270,9 @@ def extract_schema_items(manifest: dict) -> list[dict]: def _model_record(model: dict, mdl_h: str, now: datetime) -> dict: name = model["name"] - cols = [c for c in (model.get("columns") or []) if isinstance(c, dict) and c.get("name")] + 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 "" From 589e0bb408aa59678db15acd577876b41aa3e315 Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Thu, 16 Jul 2026 04:15:19 -0400 Subject: [PATCH 3/5] refactor(memory): pre-filter valid columns in schema description Address CodeRabbit nitpick: filter columns before appending the 'Columns:' header so a list of only malformed entries doesn't emit a dangling header. Mirrors _model_record's filtering approach. --- core/wren/src/wren/memory/schema_indexer.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/core/wren/src/wren/memory/schema_indexer.py b/core/wren/src/wren/memory/schema_indexer.py index 9d815d7779..dfebe97b85 100644 --- a/core/wren/src/wren/memory/schema_indexer.py +++ b/core/wren/src/wren/memory/schema_indexer.py @@ -87,12 +87,10 @@ 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: - if not isinstance(col, dict): - continue _describe_column(col, lines) lines.append("") From 0d07541c225c4e7a8a3a61c5039390e51ade01e6 Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Thu, 16 Jul 2026 04:18:17 -0400 Subject: [PATCH 4/5] style: ruff format schema_indexer --- core/wren/src/wren/memory/schema_indexer.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/core/wren/src/wren/memory/schema_indexer.py b/core/wren/src/wren/memory/schema_indexer.py index dfebe97b85..0b1295058a 100644 --- a/core/wren/src/wren/memory/schema_indexer.py +++ b/core/wren/src/wren/memory/schema_indexer.py @@ -87,7 +87,9 @@ def _describe_model(model: dict, lines: list[str]) -> None: if data_scope: lines.append(f" Data scope: {data_scope}") - cols = [c for c in (model.get("columns") or []) if isinstance(c, dict) and c.get("name")] + 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: From 8730311d045315717489e28f92f17192e15bf8e6 Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Sun, 26 Jul 2026 22:01:36 -0400 Subject: [PATCH 5/5] fix(memory): widen null-slot guards, drop unreachable raise, tighten tests - extend isinstance/name guards to models/relationships/views in describe_schema and extract_schema_items (null slots no longer crash) - drop unreachable ValueError in _column_record; caller guarantees name - add tests: dangling Columns header, empty-string/null-column, null model/rel/view slots, scalar columns; tighten weak substring assertion --- core/wren/src/wren/memory/schema_indexer.py | 20 ++++--- .../test_schema_indexer_malformed_columns.py | 52 ++++++++++++++++++- 2 files changed, 65 insertions(+), 7 deletions(-) diff --git a/core/wren/src/wren/memory/schema_indexer.py b/core/wren/src/wren/memory/schema_indexer.py index 0b1295058a..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): @@ -233,6 +236,8 @@ 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", []) or []: if not isinstance(col, dict) or not col.get("name"): @@ -240,9 +245,13 @@ def extract_schema_items(manifest: dict) -> list[dict]: 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 [] @@ -305,9 +314,8 @@ 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: - name = col.get("name") or "" - if not name: - raise ValueError("column record requires name") + # Caller (extract_schema_items) already guarantees a truthy name. + name = col["name"] dtype = col.get("type", "") expr = col.get("expression") or None is_calc = col.get("isCalculated", False) diff --git a/core/wren/tests/unit/test_schema_indexer_malformed_columns.py b/core/wren/tests/unit/test_schema_indexer_malformed_columns.py index 0a8b8c4c82..0209af289f 100644 --- a/core/wren/tests/unit/test_schema_indexer_malformed_columns.py +++ b/core/wren/tests/unit/test_schema_indexer_malformed_columns.py @@ -17,7 +17,57 @@ def test_describe_schema_skips_non_dict_columns(): } ) assert "amount" in text - assert "None" not 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():