Skip to content

fix(langchain): keep tool envelopes JSON-serializable for date and decimal rows - #2816

Open
MohammadHijjawi97 wants to merge 1 commit into
Canner:mainfrom
MohammadHijjawi97:fix/langchain-json-safe-envelope-data
Open

MohammadHijjawi97 wants to merge 1 commit into
Canner:mainfrom
MohammadHijjawi97:fix/langchain-json-safe-envelope-data

Conversation

@MohammadHijjawi97

@MohammadHijjawi97 MohammadHijjawi97 commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Summary

make_success now passes data through the existing json_safe helper, so wren_query / wren_recall_queries envelopes stay JSON-serializable when rows contain DATE, TIMESTAMP or DECIMAL values. An agent now receives JSON tool output instead of a Python repr.

What failure does this repair?

wren_query stores table.to_pylist() in data.rows, which keeps datetime.date and Decimal objects. LangChain builds the ToolMessage content with json.dumps and falls back to str() when that raises (langchain_core.tools.base._stringify), so the model gets a Python repr. Reproduction: DuckDB project with orders(id INTEGER, order_date DATE, amount DECIMAL(10,2)), tool invoked as an agent would ({"type": "tool_call", ...}):

Before:

ToolMessage.content sent to the LLM:
{'ok': True, 'content': '[{"id": 1, "order_date": "2026-05-06", "amount": "19.99"}]', 'data': {'columns': ['id', 'order_date', 'amount'], 'rows': [{'id': 1, 'order_date': datetime.date(2026, 5, 6), 'amount': Decimal('19.99')}], 'row_count': 1, 'content_truncated': False}, 'warnings': []}
-> not JSON: JSONDecodeError Expecting property name enclosed in double quotes: line 1 column 2 (char 1)
json.dumps(envelope) -> TypeError: Object of type date is not JSON serializable

After:

{"ok": true, "content": "[{\"id\": 1, \"order_date\": \"2026-05-06\", \"amount\": \"19.99\"}]", "data": {"columns": ["id", "order_date", "amount"], "rows": [{"id": 1, "order_date": "2026-05-06", "amount": "19.99"}], "row_count": 1, "content_truncated": false}, "warnings": []}
-> parses as JSON

wren_recall_queries goes through the same path; its LanceDB rows include a created_at datetime. The conversion matches what content already shows (default=str) and what format_error already does for metadata (ISO strings, Decimal as string). Calling the tool directly now also returns these values as strings in data; toolkit.query() is unchanged and still returns Arrow.

How is it tested?

  • tests/unit/test_envelope.py::test_make_success_makes_query_rows_json_serializable
  • tests/integration/test_runtime_tools.py::test_wren_query_tool_call_returns_json_for_date_and_decimal_rows: real DuckDB project and a real tool call; asserts json.loads(message.content). Not marked slow, so it runs in the sdk-langchain CI job.

Both fail without the change (TypeError / JSONDecodeError) and pass with it. Full pytest in sdk/wren-langchain: 124 passed, 2 deselected (slow). ruff check . and ruff format --check . are clean.

Duplicate check

Checked open and closed PRs and issues for wren-langchain / json_safe / envelope serialization. No open PR touches sdk/wren-langchain.

Summary by CodeRabbit

  • Bug Fixes
    • Query results containing dates, timestamps, or decimals can now be returned in a JSON-serializable success response, with these values represented as strings.

…cimal rows

wren_query put `table.to_pylist()` straight into `data.rows`, so a DATE,
TIMESTAMP or DECIMAL column left `datetime.date` / `Decimal` objects in the
envelope. LangChain builds the ToolMessage content with `json.dumps` and
falls back to `str()` when that raises, so the agent received a Python repr
(single quotes, `datetime.date(...)`, `True`) instead of JSON. The same
applied to wren_recall_queries, whose LanceDB rows carry `created_at`.

make_success now runs `data` through the existing `json_safe` helper, as
format_error already does for error metadata.
@coderabbitai

coderabbitai Bot commented Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 2cd59ef0-fdef-4313-b484-71c21cda9cf2
📥 Commits

Reviewing files that changed from the base of the PR and between caa23ac and e523998.

📒 Files selected for processing: 3
  • sdk/wren-langchain/src/wren_langchain/_envelope.py
  • sdk/wren-langchain/tests/integration/test_runtime_tools.py
  • sdk/wren-langchain/tests/unit/test_envelope.py

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

Walkthrough

The success envelope now applies json_safe to its data. Unit and integration tests cover JSON serialization of date, datetime, and decimal values.

Changes

JSON-safe success envelopes

Layer / File(s) Summary
Convert and validate success envelope data
sdk/wren-langchain/src/wren_langchain/_envelope.py, sdk/wren-langchain/tests/unit/test_envelope.py, sdk/wren-langchain/tests/integration/test_runtime_tools.py
make_success applies json_safe to envelope data. Unit tests check JSON serialization of date, datetime, and decimal values. An integration test checks date and decimal values returned through wren_query.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: paulchen79

Merge Risk: ⚪ Minimal · up to e5239

No actionable merge-blocking risk is identified; this is ready for normal checks.

Pre-merge checks: Passed 5
Check name Status Explanation
Title check Passed The title clearly and concisely identifies the main change: keeping LangChain tool envelopes JSON-serializable for date and decimal rows.
Description check Passed The description includes all required sections. It explains the change, provides a reproduction and actual error output, lists targeted and full test results, and documents the duplicate check.
Docstring Coverage Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files.
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 a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit checks the dates in rows,
And sees each decimal string now shows.
JSON carries each value through,
The envelope keeps them safe and true.
The rabbit hops, review is done.

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

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant