Skip to content

fix(langchain): skip non-dict models in format_list_models_content #2562

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

Closed
Bartok9 wants to merge 2 commits into from
Closed

fix(langchain): skip non-dict models in format_list_models_content #2562

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6a311c8
fix(langchain): skip non-dict models in format_list_models_content
Bartok9 Jul 22, 2026
59d358d
style(langchain): ruff format list_models non-dict guard test
Bartok9 Jul 22, 2026
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
20 changes: 15 additions & 5 deletions sdk/wren-langchain/src/wren_langchain/_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,21 @@ def format_list_models_content(manifest: dict[str, Any]) -> str:

lines = ["| model | cols | description |", "|---|---|---|"]
for m in models:
name = m.get("name", "")
col_count = len(m.get("columns", []) or [])
desc = (
(m.get("properties") or {}).get("description") or m.get("description") or ""
)
# MDL loaders / LLM-shaped manifests may include None rows, bare
# strings, or mixed types. Non-dicts previously AttributeError'd
# on ``.get`` and aborted the whole list_models content path.
if not isinstance(m, dict):
continue
name = m.get("name", "") or ""
columns = m.get("columns", []) or []
if not isinstance(columns, list):
columns = []
col_count = len(columns)
props = m.get("properties") or {}
if not isinstance(props, dict):
props = {}
desc = props.get("description") or m.get("description") or ""
desc = str(desc)
# Trim long descriptions to keep table compact.
if len(desc) > 80:
desc = desc[:77] + "..."
Expand Down
25 changes: 25 additions & 0 deletions sdk/wren-langchain/tests/unit/test_format_list_models_guard.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import importlib.util
from pathlib import Path

_ROOT = Path(__file__).resolve().parents[2]
_PATH = _ROOT / "src" / "wren_langchain" / "_format.py"
_spec = importlib.util.spec_from_file_location("_fmt", _PATH)
_mod = importlib.util.module_from_spec(_spec)
_spec.loader.exec_module(_mod)


def test_skips_non_dict_and_bad_nested():
out = _mod.format_list_models_content(
{
"models": [
None,
"x",
{"name": "ok", "columns": None, "properties": "bad", "description": "d"},
{"name": "wide", "columns": [1, 2], "properties": {"description": "y" * 100}},
]
}
)
assert "| ok |" in out
assert "| wide |" in out
assert "None" not in out.split("\n")[2]
assert "..." in out
Comment on lines +11 to +34

@coderabbitai coderabbitai Bot Jul 22, 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.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Strengthen the regression assertions for the new guards.

The None assertion only inspects out.split("\n")[2], which is the ok row, so it does not prove malformed rows were skipped. The test also never exercises non-string description coercion. Assert | None | and | x | are absent, and add a numeric or list description case.

Suggested test adjustments 
                 {"name": "ok", "columns": None, "properties": "bad", "description": "d"},
+                {"name": "typed", "properties": {"description": 123}},
                 {"name": "wide", "columns": [1, 2], "properties": {"description": "y" * 100}},
@@
-    assert "None" not in out.split("\n")[2]
+    assert "| None |" not in out
+    assert "| x |" not in out
+    assert "| typed | 0 | 123 |" in out
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_skips_non_dict_and_bad_nested():
out = _mod.format_list_models_content(
{
"models": [
None,
"x",
{"name": "ok", "columns": None, "properties": "bad", "description": "d"},
{"name": "wide", "columns": [1, 2], "properties": {"description": "y" * 100}},
]
}
)
assert "| ok |" in out
assert "| wide |" in out
assert "None" not in out.split("\n")[2]
assert "..." in out
def test_skips_non_dict_and_bad_nested():
out = _mod.format_list_models_content(
{
"models": [
None,
"x",
{"name": "ok", "columns": None, "properties": "bad", "description": "d"},
{"name": "typed", "properties": {"description": 123}},
{"name": "wide", "columns": [1, 2], "properties": {"description": "y" * 100}},
]
}
)
assert "| ok |" in out
assert "| wide |" in out
assert "| None |" not in out
assert "| x |" not in out
assert "| typed | 0 | 123 |" in out
assert "..." in out
🤖 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 `@sdk/wren-langchain/tests/unit/test_format_list_models_guard.py` around lines
11 - 25, Strengthen test_format_list_models_content coverage in
test_skips_non_dict_and_bad_nested by asserting the output does not contain “|
None |” or “| x |” anywhere, rather than checking one row position, and add a
model with a numeric or list description to verify non-string descriptions are
safely coerced while preserving the existing valid-row and truncation
assertions.

Loading