fix(langchain): skip non-dict rows in format helpers

[Bartok9]

Summary

• format_fetch_context_content, format_recall_content, and format_list_models_content assumed every list element was a dict.
• Skip non-dict/null rows; tolerate non-list results/models and non-dict properties so tool envelopes never AttributeError on partial memory/search payloads.

Motivation

Partial or malicious memory/search payloads can inject non-dict rows. Removing those edges silently is more useful than crashing the LangChain tool content formatter mid-response.

License

Touches sdk/** only (Apache-2.0).

Verification

cd sdk/wren-langchain && python3 -m pytest tests/unit/test_format_malformed.py -v
# 3 passed

Test plan

• Local unit tests
• CI green

Summary by CodeRabbit

• Bug Fixes
  • Improved robustness of search-context, recall, and model-list formatting for malformed or wrong-shaped inputs.
  • Invalid entries are skipped while preserving numbering/counters for valid items.
  • Added safer normalization for non-string summaries/descriptions and non-list columns/properties.
  • Ensures correct fallback messages when no valid content can be produced.
• Tests
  • Expanded unit coverage for malformed inputs across fetch-context, recall, and model listing formatters, including normalization and “all invalid” fallback cases.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 50de9c97-3e36-454a-8144-7cf1baa32621

📥 Commits

Reviewing files that changed from the base of the PR and between 45bd89f and 5139f56.

📒 Files selected for processing (1)

• sdk/wren-langchain/tests/unit/test_format_malformed.py

🚧 Files skipped from review as they are similar to previous changes (1)

• sdk/wren-langchain/tests/unit/test_format_malformed.py

---

Walkthrough

Formatting helpers now defensively process malformed fetch-context, recall, and model-list inputs. Invalid entries are skipped, valid output is numbered correctly, values are normalized, fallback sentinels are preserved, and unit tests cover these cases.

Changes

Formatting robustness

Layer / File(s)	Summary
Defensive formatter handling sdk/wren-langchain/src/wren_langchain/_format.py	Fetch, recall, and model-list formatters validate inputs, skip non-dictionary entries, normalize fields, number valid rows, and return fallback sentinels when no valid output is produced.
Malformed input coverage sdk/wren-langchain/tests/unit/test_format_malformed.py	Tests verify malformed fetch, recall, and model entries are omitted while valid data remains formatted.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• Canner/WrenAI#2562: Related defensive handling and tests for model-list formatting.
• Canner/WrenAI#2563: Related defensive handling and tests for recall formatting.

Poem

I’m a rabbit guarding each row,
Skipping odd shapes as they show.
Valid bits line up in a neat little trail,
With safe fallback signs when good data fails.
Hop, hop—malformed inputs set sail!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is relevant to the main formatter hardening work, though it only mentions skipping non-dict rows and omits the broader validation and fallback changes.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (2)

sdk/wren-langchain/src/wren_langchain/_format.py (2)

78-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consolidate validation checks.

For brevity and consistency with format_list_models_content, consider consolidating the type and emptiness checks.

♻️ Proposed refactor

-    if not isinstance(items, list):
-        return "_No relevant context items found._"
-    if not items:
-        return "_No relevant context items found._"
+    if not isinstance(items, list) or not items:
+        return "_No relevant context items found._"

🤖 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/src/wren_langchain/_format.py` around lines 78 - 81,
Consolidate the separate type and emptiness checks in the relevant formatting
function into one validation condition, matching the pattern used by
format_list_models_content, while preserving the existing "_No relevant context
items found._" return behavior for non-list and empty inputs.

---

119-122: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consolidate validation checks.

For brevity and consistency with format_list_models_content, consider consolidating the type and emptiness checks. Checking isinstance before emptiness is also slightly safer if rows happens to be an unexpected non-list object where boolean evaluation is unsupported (e.g., a numpy array or pandas DataFrame).

♻️ Proposed refactor

-    if not rows:
-        return "_No similar past queries found._"
-    if not isinstance(rows, list):
-        return "_No similar past queries found._"
+    if not isinstance(rows, list) or not rows:
+        return "_No similar past queries found._"

🤖 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/src/wren_langchain/_format.py` around lines 119 - 122,
Consolidate the validation in the relevant formatting function by checking that
rows is a list and non-empty in a single condition, evaluating isinstance(rows,
list) before its emptiness. Preserve the existing "_No similar past queries
found._" fallback and match the validation style used by
format_list_models_content.

🤖 Prompt for all review comments with 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.

Nitpick comments:
In `@sdk/wren-langchain/src/wren_langchain/_format.py`:
- Around line 78-81: Consolidate the separate type and emptiness checks in the
relevant formatting function into one validation condition, matching the pattern
used by format_list_models_content, while preserving the existing "_No relevant
context items found._" return behavior for non-list and empty inputs.
- Around line 119-122: Consolidate the validation in the relevant formatting
function by checking that rows is a list and non-empty in a single condition,
evaluating isinstance(rows, list) before its emptiness. Preserve the existing
"_No similar past queries found._" fallback and match the validation style used
by format_list_models_content.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: c3d8dcd1-17d8-404c-bc58-b1e4e4dba5b4

📥 Commits

Reviewing files that changed from the base of the PR and between 243eec8 and 3430ff6.

📒 Files selected for processing (2)

• sdk/wren-langchain/src/wren_langchain/_format.py
• sdk/wren-langchain/tests/unit/test_format_malformed.py

[goldmedal]

Review

Small, well-targeted defensive fix — scope is sdk/wren-langchain/ only (_format.py + one new test), no API / MDL / connector surface touched. Implementation is correct and introduces no bugs. LGTM to merge; the notes below are mostly test-coverage and consistency polish, none blocking.

Correctness ✅

The three formatters' isinstance filtering, the renumbering via n (replacing enumerate's i), and the "all-invalid → fallback sentinel" branches are all correct. format_list_models_content is the most thorough (also handles non-list columns → col_count=0, non-dict properties, non-str desc). No defects found.

Main note (non-blocking): new branches are under-tested

The 3 new tests only cover "a non-dict element interleaved with a valid dict is skipped." The edges this PR exists to handle aren't exercised:

1. Container itself not a list (results / rows / models not a list) → should hit the new isinstance(items, list) guards and return the fallback. Currently uncovered.
2. Entire list invalid → the if not lines / if not chunks / if not any_model fallback branches. Currently uncovered.
3. Field normalization — non-str summary/desc taking the str(...) path, non-list columns → 0. Untested.
4. Renumbering not locked in — the recall / fetch_context tests don't assert the leading 1., so the n-vs-i renumbering (the subtlest part of the change) isn't pinned. Only list_models uses an exact string, and that one has no numbering.

A few cheap cases would close this (non-list container → fallback, all-invalid → fallback, and asserting '1. "List orders"' in the recall test).

Minor consistency (agreeing with CodeRabbit's nitpick)

The three functions use three different validation shapes. format_recall_content checks if not rows before if not isinstance(rows, list) — the reverse of the other two. Functionally equivalent for JSON types, but checking emptiness first is slightly less safe for a truthiness-ambiguous object. Suggest unifying all three on the format_list_models_content form:

if not isinstance(x, list) or not x:
    return "<sentinel>"

Minor style: test import mechanism

The new test loads _format.py via importlib.util.spec_from_file_location + parents[2] path-walking + a synthetic module name, whereas every other unit test in this directory does from wren_langchain._X import .... CI installs -e ".[dev]" (deps present, and __init__ only pulls WrenToolkit/exceptions), so a direct import works, matches convention, and won't break if the file moves:

from wren_langchain._format import (
    format_fetch_context_content,
    format_recall_content,
    format_list_models_content,
)

Non-blocking

• Scope: this hardens list elements only — the outer envelope is still assumed to be a dict (result.get(...) / manifest.get(...) would still AttributeError on a non-dict). Consistent with the stated motivation, but worth saying "protects rows, not the envelope" in the description.
• Docstring coverage warning from the pre-merge check is due to the 3 test functions lacking docstrings; repo CI (ruff + pytest) doesn't enforce it, so it won't block merge.
• Confirm ruff format --check passes locally before merge — it's the one hard CI gate here.

[Bartok9]

Thanks for the thorough review @goldmedal — really appreciate the care. Pushed 45bd89f addressing all the non-blocking notes:

• Under-tested branches: added coverage for (1) non-list containers → fallback, (2) entirely-invalid lists → the if not lines/if not chunks/if not any_model fallbacks, (3) field normalization (non-str summary/desc via str(...), non-list columns → 0), and (4) locked in the renumbering by asserting the leading 1. in both the recall and fetch_context tests.
• Consistency: unified format_recall_content and format_fetch_context_content on the format_list_models_content shape — if not isinstance(x, list) or not x: — so the isinstance check comes first everywhere.
• Test import: switched to from wren_langchain._format import ... to match convention (deps are present via -e .[dev]).

ruff format --check / ruff check pass locally and all 11 tests are green. Left the envelope-level (.get(...) on a non-dict) out of scope per the stated motivation — happy to note 'protects rows, not the envelope' in the description if you'd like.

[coderabbitai]

🧹 Nitpick comments (1)

sdk/wren-langchain/tests/unit/test_format_malformed.py (1)

103-107: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover non-dict properties normalization.

This test covers malformed columns, but not the formatter’s defensive handling of non-dict properties. Add a case such as "properties": "oops" and assert the model still renders with the expected description/fallback.

🤖 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_malformed.py` around lines 103 -
107, Extend test_format_list_models_normalizes_non_list_columns to include a
model with non-dict properties such as "properties": "oops", then assert
format_list_models_content renders that model with the expected description and
fallback values without raising an error.

🤖 Prompt for all review comments with 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.

Nitpick comments:
In `@sdk/wren-langchain/tests/unit/test_format_malformed.py`:
- Around line 103-107: Extend
test_format_list_models_normalizes_non_list_columns to include a model with
non-dict properties such as "properties": "oops", then assert
format_list_models_content renders that model with the expected description and
fallback values without raising an error.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 130fc6f3-29e3-423d-b54d-a21d9c4da3da

📥 Commits

Reviewing files that changed from the base of the PR and between 3430ff6 and 45bd89f.

📒 Files selected for processing (2)

• sdk/wren-langchain/src/wren_langchain/_format.py
• sdk/wren-langchain/tests/unit/test_format_malformed.py

🚧 Files skipped from review as they are similar to previous changes (1)

• sdk/wren-langchain/src/wren_langchain/_format.py

[Bartok9]

Pushed 5139f56 covering CodeRabbit's last nitpick — added test_format_list_models_normalizes_non_dict_properties ("properties": "oops" → falls back to top-level description, model still renders). All format-malformed tests green + ruff clean. Thanks again @goldmedal!

[goldmedal]

Thanks @Bartok9, nice catch 👍