Skip to content

fix(langchain): coerce non-str nl/sql in format_recall_content #2588

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): coerce non-str nl/sql in format_recall_content #2588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a56f776
fix(langchain): coerce non-str nl/sql in format_recall_content
Bartok9 Jul 26, 2026
a8c248d
chore(langchain): ruff-format README python code blocks
Bartok9 Jul 26, 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
14 changes: 7 additions & 7 deletions sdk/wren-langchain/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,9 @@ pip install wren-langchain
- `wren_store_query` — persist a confirmed NL→SQL pair for future recall
- **Direct Python API**:
```python
toolkit.query("SELECT ...") # → pyarrow.Table
toolkit.dry_plan("SELECT ...") # → str (target-dialect SQL)
toolkit.dry_run("SELECT ...") # → None (validation only)
toolkit.query("SELECT ...") # → pyarrow.Table
toolkit.dry_plan("SELECT ...") # → str (target-dialect SQL)
toolkit.dry_run("SELECT ...") # → None (validation only)
toolkit.memory.fetch("revenue trends")
toolkit.memory.recall("top customers")
toolkit.memory.store(nl="...", sql="...", tags=["..."])
Expand All @@ -99,13 +99,13 @@ pip install wren-langchain

```python
WrenToolkit.from_project(
path, # required — path to your prepared Wren project
profile="prod", # optional — picks a named profile (default: active)
path, # required — path to your prepared Wren project
profile="prod", # optional — picks a named profile (default: active)
)

toolkit.get_tools(
include_memory_write=True, # set False to keep memory read-only
raise_on_error=False, # set True to surface exceptions to LangChain retry
include_memory_write=True, # set False to keep memory read-only
raise_on_error=False, # set True to surface exceptions to LangChain retry
)
```

Expand Down
5 changes: 5 additions & 0 deletions sdk/wren-langchain/tests/unit/test_format_malformed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,3 +121,8 @@ def test_format_list_models_normalizes_non_dict_properties() -> None:
}
)
assert "| customers | 1 | fallback desc |" in out


def test_format_recall_coerces_non_str_nl_sql() -> None:
out = format_recall_content([{"nl": 123, "sql": None}])
assert '1. "123"' in out
Comment on lines +126 to +128

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Assert the SQL output too.

Line [128] verifies only the NL value, so the test would pass if the SQL fence were omitted or rendered incorrectly. Assert the complete output, including the empty SQL fence required for sql=None; use a non-string SQL value too if coercion is part of this test’s contract.

🤖 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 126 -
128, Update test_format_recall_coerces_non_str_nl_sql to assert the complete
formatted output, including the SQL section and its empty code fence when sql is
None. Use a non-string SQL input as well if the test is intended to verify SQL
coercion, and replace the partial substring assertion with an exact
expected-output assertion.

Loading