fix(langchain): coerce None in format_store_content

[Bartok9]

Summary

• format_store_content called .strip() on sql/nl without guarding None.
• Coerce non-str nl/sql; treat non-list tags as zero tags.

Motivation

Store-tool confirmation paths can pass placeholders. formatting must not TypeError after a successful store.

License

sdk/** Apache-2.0

Verification

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

Test plan

• Local unit tests
• CI green

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of stored content with unexpected input types.
  • Prevented formatting failures when SQL and natural-language text are missing or not strings.
  • Hardened tag reporting by only counting tags when they are provided as a list.
• Tests
  • Added unit tests covering normal formatting, missing values, non-string title/SQL/natural-language inputs, and non-list tags.

[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: 33d51053-efb4-4ad6-817d-74077a140320

📥 Commits

Reviewing files that changed from the base of the PR and between e9e522b and e60f254.

📒 Files selected for processing (2)

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

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

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

---

Walkthrough

format_store_content now safely normalizes nl and sql values, counts tags only when tags is a list, and adds unit tests for valid and invalid input types.

Changes

Store content formatting

Layer / File(s)	Summary
Input normalization and validation sdk/wren-langchain/src/wren_langchain/_format.py, sdk/wren-langchain/tests/unit/test_format_store_content.py	None text inputs become empty strings, other non-string values are stringified, non-list tags produce zero counts, and tests cover standard, null, non-string, and non-list inputs.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related PRs

• Canner/WrenAI#2561: Extends hardening of format_store_content with stricter text coercion and tag-count validation.

Poem

A rabbit checks each string with care,
Empty thoughts become fresh air.
Lists count tags in tidy rows,
Tests hop where bad input goes.
Squeak—safe formatting grows!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is concise and accurately reflects the main fix in format_store_content, though it only mentions None handling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 (1)

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

26-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover non-None non-string text inputs.

The tests exercise the None branches, but not the str(nl)/str(sql) coercion paths described by the PR objective. Add a case such as integers to prevent regressions.

Suggested test

+def test_format_store_content_non_string_text() -> None:
+    out = _fmt.format_store_content(123, 456, [])
+    assert 'Stored: "123"' in out
+    assert "456" 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_store_content.py` around lines 26 -
34, Add a unit test for format_store_content covering non-None, non-string nl
and sql inputs such as integers, and assert their string-coerced values appear
in the formatted output. Keep the existing None and non-list tag tests
unchanged.

🤖 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_store_content.py`:
- Around line 26-34: Add a unit test for format_store_content covering non-None,
non-string nl and sql inputs such as integers, and assert their string-coerced
values appear in the formatted output. Keep the existing None and non-list tag
tests unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b078faef-eb48-40cb-a599-638b3e1dd3d7

📥 Commits

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

📒 Files selected for processing (2)

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

[goldmedal]

Reviewed — the fix is correct and I'd be happy to see it merged. One test-quality suggestion and a small nit, neither blocking.

Correctness ✅
Reproduced on main: format_store_content(None, ...) raises AttributeError: 'NoneType' object has no attribute 'strip'. The motivation holds — a content formatter shouldn't TypeError after a store has already succeeded — so defensive coercion at this layer is a reasonable choice. The coercion logic is sound (None → "", other non-str → str(x), non-list tags → 0), and the three test cases cover it well.

🟡 Suggestion — test uses importlib file-loading instead of a normal import

test_format_store_content.py loads the module via importlib.util.spec_from_file_location with a hardcoded parents[2]/"src"/"wren_langchain"/"_format.py" path. Every existing test in tests/unit/ imports submodules directly, e.g. test_envelope.py:

from wren_langchain._envelope import (...)

I confirmed from wren_langchain._format import format_store_content works in this package (it's how I reproduced the bug). The importlib approach isn't needed and is more fragile — hardcoded relative path, bypasses the package, and would break if _format.py ever adds a relative sibling import. Suggest replacing the loader block with:

from wren_langchain._format import format_store_content

and using format_store_content(...) directly in the three tests.

🟢 Nit
In if tags is None or not isinstance(tags, list):, the tags is None or is redundant — not isinstance(None, list) is already True, so if not isinstance(tags, list): suffices.

[Bartok9]

Thanks for the thorough review, @goldmedal — much appreciated. Applied both:

• Import: dropped the importlib file-loader in favor of from wren_langchain._format import format_store_content, matching test_envelope.py and the rest of tests/unit/. Cleaner and not path-fragile.
• Nit: simplified to tag_count = len(tags) if isinstance(tags, list) else 0 — the tags is None or was indeed redundant.

Also added a small case (from CodeRabbit) covering the non-None non-string coercion path (str(123)/str(456)) so the str(x) branch is exercised, not just the None branch.

Pushed as e60f254.

[goldmedal]

Thanks @Bartok9 👍