feat(sdk): add wren-langchain package for langchain/langgraph integration#2247
Merged
Conversation
github-actions
Bot
added
documentation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a new Python SDK package ChangesWren LangChain SDK Package Introduction
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
2 times, most recently
from
5f71add to
cb246b3
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-please.yml (1)
15-88:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: duplicated
publish-wren-langchainjob and missingwren-langchain--*outputs on therelease-pleasejob.Two related blockers in the same change:
- Duplicate job key (Lines 68 and 79). The
publish-wren-langchainmapping appears twice; YAML disallows duplicate keys at the same level.actionlintandyamllintboth flag this — the workflow will fail to parse (or silently keep only one entry, depending on the runner's loader).- Missing outputs declaration (Lines 15-24). The
release-pleasejob'soutputs:block declares onlywren-core-py--*,wren--*, andwren-core-wasm--*. The newif:andwith:references toneeds.release-please.outputs['wren-langchain--release_created' | 'wren-langchain--version' | 'wren-langchain--tag_name']will resolve to empty strings, so the publish job will never trigger even after the duplicate is removed.actionlintflags lines 70, 73, 74 for the same reason.Apply both fixes together:
🔧 Proposed fix
release-please: runs-on: ubuntu-latest outputs: wren-core-py--release_created: ${{ steps.release.outputs['core/wren-core-py--release_created'] }} wren-core-py--tag_name: ${{ steps.release.outputs['core/wren-core-py--tag_name'] }} wren-core-py--version: ${{ steps.release.outputs['core/wren-core-py--version'] }} wren--release_created: ${{ steps.release.outputs['core/wren--release_created'] }} wren--tag_name: ${{ steps.release.outputs['core/wren--tag_name'] }} wren--version: ${{ steps.release.outputs['core/wren--version'] }} wren-core-wasm--release_created: ${{ steps.release.outputs['core/wren-core-wasm--release_created'] }} wren-core-wasm--tag_name: ${{ steps.release.outputs['core/wren-core-wasm--tag_name'] }} wren-core-wasm--version: ${{ steps.release.outputs['core/wren-core-wasm--version'] }} + wren-langchain--release_created: ${{ steps.release.outputs['sdk/wren-langchain--release_created'] }} + wren-langchain--tag_name: ${{ steps.release.outputs['sdk/wren-langchain--tag_name'] }} + wren-langchain--version: ${{ steps.release.outputs['sdk/wren-langchain--version'] }} steps: @@ publish-wren-langchain: needs: release-please if: needs.release-please.outputs['wren-langchain--release_created'] == 'true' uses: ./.github/workflows/publish-wren-langchain.yml with: version: ${{ needs.release-please.outputs['wren-langchain--version'] }} tag_name: ${{ needs.release-please.outputs['wren-langchain--tag_name'] }} permissions: contents: read id-token: write - - publish-wren-langchain: - needs: release-please - if: needs.release-please.outputs['wren-langchain--release_created'] == 'true' - uses: ./.github/workflows/publish-wren-langchain.yml - with: - version: ${{ needs.release-please.outputs['wren-langchain--version'] }} - tag_name: ${{ needs.release-please.outputs['wren-langchain--tag_name'] }} - permissions: - contents: read - id-token: write🤖 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 @.github/workflows/release-please.yml around lines 15 - 88, Remove the duplicated job key "publish-wren-langchain" (keep a single job block named publish-wren-langchain) and add the missing release-please outputs for the langchain package by declaring wren-langchain--release_created, wren-langchain--tag_name, and wren-langchain--version in the release-please job's outputs mapping, each wired to the corresponding steps.release.outputs['core/wren-langchain--release_created'], ['core/wren-langchain--tag_name'], and ['core/wren-langchain--version'] so the publish-wren-langchain job's if: and with: references resolve correctly.
🧹 Nitpick comments (3)
sdk/wren-langchain/src/wren_langchain/_tools.py (1)
85-100: ⚡ Quick win
toolkit._mdl_sourcebreaches encapsulation — add a public accessor instead.
wren_list_modelsreaches intoWrenToolkit's private_mdl_sourceattribute from an external module. Any internal rename of that attribute will silently break this tool at runtime with anAttributeError. Sinceload_manifest()is a first-class operation used by a public-facing LLM tool, it deserves a small public method onWrenToolkit.♻️ Suggested fix
In
_toolkit.py, expose a thin public method:def load_manifest(self) -> dict: return self._mdl_source.load_manifest()Then in
_tools.py:- manifest = toolkit._mdl_source.load_manifest() + manifest = toolkit.load_manifest()🤖 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/_tools.py` around lines 85 - 100, The tool function wren_list_models is accessing the private attribute WrenToolkit._mdl_source directly which breaks encapsulation; add a public thin accessor on WrenToolkit (e.g., def load_manifest(self) -> dict: return self._mdl_source.load_manifest()) and then update wren_list_models to call toolkit.load_manifest() instead of toolkit._mdl_source.load_manifest(), preserving the existing try/except and return behavior in wren_list_models.sdk/wren-langchain/tests/unit/test_memory_api.py (1)
17-52: ⚡ Quick winFragile test:
from_projectis constructed outside the mock patch in several tests.
test_memory_fetch_calls_get_context_with_manifest,test_memory_recall_calls_recall_queries, andtest_memory_store_calls_store_queryall callWrenToolkit.from_project(project)before entering thepatch()context. These tests silently rely on_MemoryAPIdeferringprovider.open()until the first operation. If the lazy-init contract ever changes (e.g., a future refactor eagerly opens the store during toolkit construction), the mock will be applied too late, calls will reach the realMemoryStore, and the assertion results will be unreliable or the tests will produce environment-dependent failures rather than clear failures.The analogous tests in
test_tools_memory.pyconsistently wrapWrenToolkit.from_project(...)inside thepatch()block — that's the safer pattern. Consider aligningtest_memory_api.pywith the same approach.♻️ Suggested fix (example for test_memory_fetch_calls_get_context_with_manifest)
def test_memory_fetch_calls_get_context_with_manifest(tmp_project, fake_active_profile): project = _enable_memory(tmp_project) fake_store = MagicMock(name="MemoryStore") fake_store.get_context.return_value = {"strategy": "search", "results": []} - toolkit = WrenToolkit.from_project(project) - with patch( "wren_langchain._providers.memory.MemoryStore", return_value=fake_store ): + toolkit = WrenToolkit.from_project(project) result = toolkit.memory.fetch("revenue trends", limit=3) assert result == {"strategy": "search", "results": []}🤖 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_memory_api.py` around lines 17 - 52, The tests construct WrenToolkit.from_project(project) before applying the MemoryStore patch which makes them fragile; wrap the WrenToolkit.from_project(...) call inside the with patch("wren_langchain._providers.memory.MemoryStore", return_value=fake_store) block in test_memory_fetch_calls_get_context_with_manifest, test_memory_recall_calls_recall_queries (and similar tests like test_memory_store_calls_store_query) so the mock is effective during toolkit construction and any eager provider.open() calls; in short, move the WrenToolkit.from_project(project) invocation into the patch context that returns fake_store so toolkit.memory.fetch/recall use the mocked MemoryStore.sdk/wren-langchain/src/wren_langchain/_providers/memory.py (1)
28-29: 💤 Low value
open()is not idempotent — document that callers are responsible for caching.
LocalLanceDBMemoryProvider.open()constructs a newMemoryStoreon every call. The caching contract lives in_MemoryAPI, which is fine, but nothing enforces or documents this. If a future refactor bypasses_MemoryAPIand callsopen()directly in a loop, LanceDB will be re-opened per invocation. A docstring note on the method (or renaming tocreate()) would make the ownership clear.📝 Suggested docstring addition
def open(self) -> MemoryStore: + """Create a new MemoryStore instance. Callers are responsible for caching the result.""" return MemoryStore(path=self._memory_path)🤖 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/_providers/memory.py` around lines 28 - 29, LocalLanceDBMemoryProvider.open() constructs a new MemoryStore on every call and is not idempotent; add a clear docstring to LocalLanceDBMemoryProvider.open() stating that callers are responsible for caching the returned MemoryStore (or alternatively rename the method to create() to indicate it always makes a new instance), and reference the intended caching contract in _MemoryAPI so future callers know not to call open() repeatedly in hot paths (mention MemoryStore and _MemoryAPI in the docstring).
🤖 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.
Inline comments:
In @.github/workflows/publish-wren-langchain.yml:
- Around line 20-24: The workflow currently treats any value other than
"testpypi" as production, which risks accidental prod publishes; add explicit
validation of the pypi_target input (the pypi_target workflow input) and tighten
the publish routing condition so it only treats the value "pypi" as production
and "testpypi" as test. Concretely: add an early job/step that checks the
pypi_target value and fails with a clear message if it is not exactly "pypi" or
"testpypi", and change the publish decision/if-expression that currently checks
for != "testpypi" to explicit equality checks (e.g., pypi_target == "pypi" for
prod, pypi_target == "testpypi" for test) so typos like "test-pypi" cannot route
to production.
In `@sdk/wren-langchain/src/wren_langchain/_envelope.py`:
- Around line 56-67: redact_secrets currently only inspects top-level keys and
will leak nested secrets; update the redact_secrets function to recursively
traverse dicts and lists (handling dict values, list items, and preserving
non-collection types) and replace any value whose key (or nested key path)
contains a case-insensitive match against _SECRET_PATTERNS with "***"; ensure
you do not mutate the input (create new dict/list copies), reference
redact_secrets and _SECRET_PATTERNS when implementing the recursive helper, and
keep the original return type dict[str, Any].
In `@sdk/wren-langchain/src/wren_langchain/_format.py`:
- Around line 53-73: The current formatter (using variables strategy, result and
CONTENT_CAP_BYTES) truncates the "full" schema by characters and imposes no cap
on the search/results branch; update both branches to enforce a byte-aware cap
by measuring len(...encode("utf-8")) and truncating bytes, not characters.
Specifically: in the "full" branch (where text = result.get("schema")), perform
truncation based on bytes so the returned text.encode("utf-8") length never
exceeds CONTENT_CAP_BYTES and append a clear truncation marker; in the list
branch (items, lines, summary), ensure each summary is truncated by bytes when
shortening and after building the joined lines check the total byte length and
trim the tail (e.g., remove or truncate last entries/summary by bytes) so the
final "\n".join(lines).encode("utf-8") <= CONTENT_CAP_BYTES while preserving the
truncation marker.
In `@sdk/wren-langchain/src/wren_langchain/_providers/connection.py`:
- Around line 41-44: The check in _resolve_profile currently treats empty string
as falsy and falls through; change the guard to "if explicit is not None" in
_resolve_profile so callers passing an explicit empty string are handled as
intended, and add an explicit check that raises a clear error (e.g., ValueError)
when explicit == "" before calling _lookup_named_profile(explicit) to surface
misconfigured callers early.
In `@sdk/wren-langchain/src/wren_langchain/_providers/mdl_source.py`:
- Around line 23-29: The load_manifest method currently allows JSONDecodeError
from json.loads to propagate; update load_manifest to catch json.JSONDecodeError
(and optionally ValueError) when reading/parsing self._mdl_path.read_text(), and
re-raise a WrenToolkitInitError that includes the path (self._mdl_path) and the
underlying parse error message so callers get a consistent init-error contract;
reference the load_manifest method, self._mdl_path, and WrenToolkitInitError
when making this change.
In `@sdk/wren-langchain/src/wren_langchain/_toolkit.py`:
- Around line 214-215: The auto-detection currently treats any existing path
(including files/symlinks) as a memory root; change the check from
memory_dir.exists() to memory_dir.is_dir() so you only construct
LocalLanceDBMemoryProvider for actual directories (leave the rest of the logic
unchanged and reference the memory_dir variable and LocalLanceDBMemoryProvider
constructor).
In `@sdk/wren-langchain/tests/unit/test_prompt.py`:
- Around line 107-109: The failing assertion allows "useful" to slip through
because it uses OR; change the third assertion to require both substrings be
absent (replace the `or` with `and`) so it reads like the first two negatives
(e.g., assert "useful" not in prompt.lower() and "useful query" not in
prompt.lower()), updating the assertion in the test snippet that currently
contains `"useful" not in prompt.lower() or "useful query" not in
prompt.lower()`.
In `@sdk/wren-langchain/tests/unit/test_toolkit_init.py`:
- Around line 94-98: The test resets global loader state at the start using
_reset_env_loaded_for_tests() but never restores it, risking leakage to other
tests; after creating toolkit via WrenToolkit.from_project and asserting
toolkit._connection.connection_info()["host"] == "resolved-from-project-env",
call _reset_env_loaded_for_tests() (or equivalent teardown) to restore the
global env-loaded state so subsequent tests run cleanly.
---
Outside diff comments:
In @.github/workflows/release-please.yml:
- Around line 15-88: Remove the duplicated job key "publish-wren-langchain"
(keep a single job block named publish-wren-langchain) and add the missing
release-please outputs for the langchain package by declaring
wren-langchain--release_created, wren-langchain--tag_name, and
wren-langchain--version in the release-please job's outputs mapping, each wired
to the corresponding
steps.release.outputs['core/wren-langchain--release_created'],
['core/wren-langchain--tag_name'], and ['core/wren-langchain--version'] so the
publish-wren-langchain job's if: and with: references resolve correctly.
---
Nitpick comments:
In `@sdk/wren-langchain/src/wren_langchain/_providers/memory.py`:
- Around line 28-29: LocalLanceDBMemoryProvider.open() constructs a new
MemoryStore on every call and is not idempotent; add a clear docstring to
LocalLanceDBMemoryProvider.open() stating that callers are responsible for
caching the returned MemoryStore (or alternatively rename the method to create()
to indicate it always makes a new instance), and reference the intended caching
contract in _MemoryAPI so future callers know not to call open() repeatedly in
hot paths (mention MemoryStore and _MemoryAPI in the docstring).
In `@sdk/wren-langchain/src/wren_langchain/_tools.py`:
- Around line 85-100: The tool function wren_list_models is accessing the
private attribute WrenToolkit._mdl_source directly which breaks encapsulation;
add a public thin accessor on WrenToolkit (e.g., def load_manifest(self) ->
dict: return self._mdl_source.load_manifest()) and then update wren_list_models
to call toolkit.load_manifest() instead of toolkit._mdl_source.load_manifest(),
preserving the existing try/except and return behavior in wren_list_models.
In `@sdk/wren-langchain/tests/unit/test_memory_api.py`:
- Around line 17-52: The tests construct WrenToolkit.from_project(project)
before applying the MemoryStore patch which makes them fragile; wrap the
WrenToolkit.from_project(...) call inside the with
patch("wren_langchain._providers.memory.MemoryStore", return_value=fake_store)
block in test_memory_fetch_calls_get_context_with_manifest,
test_memory_recall_calls_recall_queries (and similar tests like
test_memory_store_calls_store_query) so the mock is effective during toolkit
construction and any eager provider.open() calls; in short, move the
WrenToolkit.from_project(project) invocation into the patch context that returns
fake_store so toolkit.memory.fetch/recall use the mocked MemoryStore.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d6d389a4-2a40-44bb-a986-faaf317f9653
📒 Files selected for processing (46)
.github/workflows/publish-wren-langchain.yml.github/workflows/release-please.yml.release-please-manifest.jsonLICENSEREADME.mdrelease-please-config.jsonsdk/wren-langchain/.gitignoresdk/wren-langchain/LICENSEsdk/wren-langchain/README.mdsdk/wren-langchain/examples/langchain_demo.pysdk/wren-langchain/examples/langgraph_demo.pysdk/wren-langchain/pyproject.tomlsdk/wren-langchain/src/wren_langchain/__init__.pysdk/wren-langchain/src/wren_langchain/_envelope.pysdk/wren-langchain/src/wren_langchain/_format.pysdk/wren-langchain/src/wren_langchain/_memory_api.pysdk/wren-langchain/src/wren_langchain/_prompt.pysdk/wren-langchain/src/wren_langchain/_providers/__init__.pysdk/wren-langchain/src/wren_langchain/_providers/connection.pysdk/wren-langchain/src/wren_langchain/_providers/mdl_source.pysdk/wren-langchain/src/wren_langchain/_providers/memory.pysdk/wren-langchain/src/wren_langchain/_toolkit.pysdk/wren-langchain/src/wren_langchain/_tools.pysdk/wren-langchain/src/wren_langchain/_tools_memory.pysdk/wren-langchain/src/wren_langchain/exceptions.pysdk/wren-langchain/tests/__init__.pysdk/wren-langchain/tests/conformance/__init__.pysdk/wren-langchain/tests/conformance/test_langchain_contract.pysdk/wren-langchain/tests/conftest.pysdk/wren-langchain/tests/integration/__init__.pysdk/wren-langchain/tests/integration/conftest.pysdk/wren-langchain/tests/integration/test_langgraph_toolnode.pysdk/wren-langchain/tests/integration/test_memory_tools.pysdk/wren-langchain/tests/integration/test_runtime_tools.pysdk/wren-langchain/tests/unit/__init__.pysdk/wren-langchain/tests/unit/test_envelope.pysdk/wren-langchain/tests/unit/test_exceptions.pysdk/wren-langchain/tests/unit/test_memory_api.pysdk/wren-langchain/tests/unit/test_prompt.pysdk/wren-langchain/tests/unit/test_providers_connection.pysdk/wren-langchain/tests/unit/test_providers_mdl.pysdk/wren-langchain/tests/unit/test_providers_memory.pysdk/wren-langchain/tests/unit/test_toolkit_init.pysdk/wren-langchain/tests/unit/test_toolkit_runtime.pysdk/wren-langchain/tests/unit/test_tools_memory.pysdk/wren-langchain/tests/unit/test_tools_runtime.py
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
from
cb246b3 to
57222b7
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release-please.yml (1)
15-24:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMissing
wren-langchain--*output declarations in therelease-pleasejob.The three outputs consumed by
publish-wren-langchain—wren-langchain--release_created,wren-langchain--version, andwren-langchain--tag_name— are never declared in therelease-pleasejob'soutputsblock. Without them, the downstream job'sifcondition always evaluates to empty (i.e., never'true'), so the publish job will silently never run after a release. This is confirmed by theactionlinterrors on lines 70, 73, and 74.🐛 Proposed fix — add the missing outputs
wren-core-wasm--release_created: ${{ steps.release.outputs['core/wren-core-wasm--release_created'] }} wren-core-wasm--tag_name: ${{ steps.release.outputs['core/wren-core-wasm--tag_name'] }} wren-core-wasm--version: ${{ steps.release.outputs['core/wren-core-wasm--version'] }} + wren-langchain--release_created: ${{ steps.release.outputs['sdk/wren-langchain--release_created'] }} + wren-langchain--tag_name: ${{ steps.release.outputs['sdk/wren-langchain--tag_name'] }} + wren-langchain--version: ${{ steps.release.outputs['sdk/wren-langchain--version'] }}Note: The key prefix (
sdk/wren-langchainvswren-langchain) should match the path declared inrelease-please-config.json/.release-please-manifest.jsonfor this package. Verify the exact path key against those config files.🤖 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 @.github/workflows/release-please.yml around lines 15 - 24, Add the three missing output declarations to the release-please job's outputs block so the downstream publish-wren-langchain job can read them: declare wren-langchain--release_created, wren-langchain--version, and wren-langchain--tag_name mapping to the appropriate release step outputs (e.g., steps.release.outputs['<path-key>/wren-langchain--release_created'] etc.); ensure the key prefix matches the package path used in release-please-config.json / .release-please-manifest.json (verify whether the path is sdk/wren-langchain or wren-langchain) and use those exact keys when adding the new entries to the outputs block.
🧹 Nitpick comments (2)
sdk/wren-langchain/examples/langgraph_demo.py (1)
145-145: 💤 Low valueAvoid accessing the private
_memoryattribute in example code.
toolkit._memory.enableddirectly couples the demo to an internal implementation detail. If_memoryis renamed or the memory subsystem is restructured, this line breaks silently (or with anAttributeError). A public property onWrenToolkit(e.g.,toolkit.memory_enabled) would be more appropriate for user-facing example code.🤖 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/examples/langgraph_demo.py` at line 145, The example is accessing the private attribute toolkit._memory; change the example to use a public API (e.g., toolkit.memory_enabled) and implement that public property on WrenToolkit to return the current memory enabled state (delegating to the internal _memory.enabled) so examples no longer rely on internals—update the print in langgraph_demo.py to use toolkit.memory_enabled and add a memory_enabled property (or equivalent getter) to the WrenToolkit class that returns the boolean state.sdk/wren-langchain/tests/unit/test_tools_memory.py (1)
152-153: 💤 Low value
call_args.kwargs["tags"]raisesKeyError(notAssertionError) ifstore_queryis called positionally.If the implementation passes
tagsas a positional argument,.kwargsis{}and the dict lookup fails with aKeyErrorrather than a clean assertion failure, obscuring the real problem.🛡️ Proposed more-robust assertion
- fake_store.store_query.assert_called_once() - assert fake_store.store_query.call_args.kwargs["tags"] == "ranking,demo" + fake_store.store_query.assert_called_once() + _args, _kwargs = fake_store.store_query.call_args + tags_value = _kwargs.get("tags") or (_args[2] if len(_args) > 2 else None) + assert tags_value == "ranking,demo"Alternatively, if the calling convention is known to always use kwargs:
- assert fake_store.store_query.call_args.kwargs["tags"] == "ranking,demo" + assert fake_store.store_query.call_args.kwargs.get("tags") == "ranking,demo"🤖 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_tools_memory.py` around lines 152 - 153, The test's assertion on fake_store.store_query assumes tags was passed as a kwarg and uses call_args.kwargs["tags"], which raises KeyError if tags was passed positionally; update the assertion in test_tools_memory (around fake_store.store_query) to robustly extract tags from fake_store.store_query.call_args by first grabbing call = fake_store.store_query.call_args, asserting call is not None, then checking tags = call.kwargs.get("tags") or (call.args[index] if present) and finally assert tags == "ranking,demo" so the test fails with a clear assertion rather than a KeyError.
🤖 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.
Inline comments:
In @.github/workflows/release-please.yml:
- Around line 79-88: Remove the duplicated GitHub Actions job definition for
publish-wren-langchain by deleting the second job block (the later
publish-wren-langchain mapping) and keep only the first occurrence; ensure the
retained job contains the needs: release-please, if condition using
needs.release-please.outputs['wren-langchain--release_created'], the uses:
./.github/workflows/publish-wren-langchain.yml and the with/permissions entries
so there is a single publish-wren-langchain job named publish-wren-langchain in
the workflow.
In @.github/workflows/sdk-langchain-ci.yml:
- Around line 9-12: The workflow's permissions block unnecessarily grants
pull-requests: write; update the permissions in the permissions section to
follow least-privilege by removing or downgrading the "pull-requests: write"
entry (leave contents: read and any other required minimal scopes) so the
GITHUB_TOKEN only has the permissions needed for lint/test/build; target the
permissions block that currently contains "pull-requests: write" and remove or
set it to none.
In `@sdk/wren-langchain/examples/langgraph_demo.py`:
- Around line 3-5: Update the docstring to reference the correct
LangChain/LangGraph factory function: replace the incorrect reference to
langchain.agents.create_agent with langchain.agents.create_react_agent (or
langgraph.prebuilt.create_react_agent for newer LangGraph usages) so readers
aren’t misled; search for the symbol "create_agent" in the docstring and change
it to "create_react_agent" (and optionally mention the alternative
"langgraph.prebuilt.create_react_agent") to match the actual API.
In `@sdk/wren-langchain/src/wren_langchain/_prompt.py`:
- Around line 134-152: The system prompt currently advertises the
wren_store_query/write behavior using _build_tools_section(toolkit) and memory
workflow blocks without checking whether the toolkit was created with memory
write enabled; update build_system_prompt to query the actual toolset/config
from the toolkit (e.g., use toolkit.get_tools(include_memory_write=...) or a
toolkit property that reflects include_memory_write) and pass that flag into
_build_tools_section (or conditionally omit wren_store_query) and select the
appropriate memory workflow/error_recovery/avoidance sections based on the
toolkit's actual include_memory_write setting so the prompt matches the real
tools provided.
---
Outside diff comments:
In @.github/workflows/release-please.yml:
- Around line 15-24: Add the three missing output declarations to the
release-please job's outputs block so the downstream publish-wren-langchain job
can read them: declare wren-langchain--release_created, wren-langchain--version,
and wren-langchain--tag_name mapping to the appropriate release step outputs
(e.g., steps.release.outputs['<path-key>/wren-langchain--release_created']
etc.); ensure the key prefix matches the package path used in
release-please-config.json / .release-please-manifest.json (verify whether the
path is sdk/wren-langchain or wren-langchain) and use those exact keys when
adding the new entries to the outputs block.
---
Nitpick comments:
In `@sdk/wren-langchain/examples/langgraph_demo.py`:
- Line 145: The example is accessing the private attribute toolkit._memory;
change the example to use a public API (e.g., toolkit.memory_enabled) and
implement that public property on WrenToolkit to return the current memory
enabled state (delegating to the internal _memory.enabled) so examples no longer
rely on internals—update the print in langgraph_demo.py to use
toolkit.memory_enabled and add a memory_enabled property (or equivalent getter)
to the WrenToolkit class that returns the boolean state.
In `@sdk/wren-langchain/tests/unit/test_tools_memory.py`:
- Around line 152-153: The test's assertion on fake_store.store_query assumes
tags was passed as a kwarg and uses call_args.kwargs["tags"], which raises
KeyError if tags was passed positionally; update the assertion in
test_tools_memory (around fake_store.store_query) to robustly extract tags from
fake_store.store_query.call_args by first grabbing call =
fake_store.store_query.call_args, asserting call is not None, then checking tags
= call.kwargs.get("tags") or (call.args[index] if present) and finally assert
tags == "ranking,demo" so the test fails with a clear assertion rather than a
KeyError.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: de78b3c6-a3a0-425c-8ab4-f608826f6959
📒 Files selected for processing (47)
.github/workflows/publish-wren-langchain.yml.github/workflows/release-please.yml.github/workflows/sdk-langchain-ci.yml.release-please-manifest.jsonLICENSEREADME.mdrelease-please-config.jsonsdk/wren-langchain/.gitignoresdk/wren-langchain/LICENSEsdk/wren-langchain/README.mdsdk/wren-langchain/examples/langchain_demo.pysdk/wren-langchain/examples/langgraph_demo.pysdk/wren-langchain/pyproject.tomlsdk/wren-langchain/src/wren_langchain/__init__.pysdk/wren-langchain/src/wren_langchain/_envelope.pysdk/wren-langchain/src/wren_langchain/_format.pysdk/wren-langchain/src/wren_langchain/_memory_api.pysdk/wren-langchain/src/wren_langchain/_prompt.pysdk/wren-langchain/src/wren_langchain/_providers/__init__.pysdk/wren-langchain/src/wren_langchain/_providers/connection.pysdk/wren-langchain/src/wren_langchain/_providers/mdl_source.pysdk/wren-langchain/src/wren_langchain/_providers/memory.pysdk/wren-langchain/src/wren_langchain/_toolkit.pysdk/wren-langchain/src/wren_langchain/_tools.pysdk/wren-langchain/src/wren_langchain/_tools_memory.pysdk/wren-langchain/src/wren_langchain/exceptions.pysdk/wren-langchain/tests/__init__.pysdk/wren-langchain/tests/conformance/__init__.pysdk/wren-langchain/tests/conformance/test_langchain_contract.pysdk/wren-langchain/tests/conftest.pysdk/wren-langchain/tests/integration/__init__.pysdk/wren-langchain/tests/integration/conftest.pysdk/wren-langchain/tests/integration/test_langgraph_toolnode.pysdk/wren-langchain/tests/integration/test_memory_tools.pysdk/wren-langchain/tests/integration/test_runtime_tools.pysdk/wren-langchain/tests/unit/__init__.pysdk/wren-langchain/tests/unit/test_envelope.pysdk/wren-langchain/tests/unit/test_exceptions.pysdk/wren-langchain/tests/unit/test_memory_api.pysdk/wren-langchain/tests/unit/test_prompt.pysdk/wren-langchain/tests/unit/test_providers_connection.pysdk/wren-langchain/tests/unit/test_providers_mdl.pysdk/wren-langchain/tests/unit/test_providers_memory.pysdk/wren-langchain/tests/unit/test_toolkit_init.pysdk/wren-langchain/tests/unit/test_toolkit_runtime.pysdk/wren-langchain/tests/unit/test_tools_memory.pysdk/wren-langchain/tests/unit/test_tools_runtime.py
✅ Files skipped from review due to trivial changes (18)
- sdk/wren-langchain/LICENSE
- sdk/wren-langchain/tests/unit/test_providers_memory.py
- release-please-config.json
- sdk/wren-langchain/src/wren_langchain/init.py
- sdk/wren-langchain/tests/unit/test_toolkit_runtime.py
- .release-please-manifest.json
- sdk/wren-langchain/src/wren_langchain/_providers/mdl_source.py
- sdk/wren-langchain/src/wren_langchain/exceptions.py
- sdk/wren-langchain/tests/conftest.py
- sdk/wren-langchain/src/wren_langchain/_providers/memory.py
- sdk/wren-langchain/tests/integration/conftest.py
- sdk/wren-langchain/src/wren_langchain/_tools.py
- sdk/wren-langchain/.gitignore
- sdk/wren-langchain/tests/unit/test_providers_connection.py
- sdk/wren-langchain/tests/conformance/test_langchain_contract.py
- sdk/wren-langchain/src/wren_langchain/_providers/connection.py
- sdk/wren-langchain/tests/unit/test_prompt.py
- sdk/wren-langchain/tests/unit/test_providers_mdl.py
🚧 Files skipped from review as they are similar to previous changes (11)
- LICENSE
- sdk/wren-langchain/tests/unit/test_exceptions.py
- sdk/wren-langchain/tests/unit/test_envelope.py
- sdk/wren-langchain/tests/integration/test_runtime_tools.py
- sdk/wren-langchain/examples/langchain_demo.py
- sdk/wren-langchain/tests/unit/test_tools_runtime.py
- sdk/wren-langchain/pyproject.toml
- .github/workflows/publish-wren-langchain.yml
- sdk/wren-langchain/tests/integration/test_memory_tools.py
- sdk/wren-langchain/tests/unit/test_toolkit_init.py
- sdk/wren-langchain/src/wren_langchain/_toolkit.py
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
2 times, most recently
from
15e44bb to
471f7b6
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/release-please.yml (1)
15-24:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAdd missing
wren-langchain--*output declarations to therelease-pleasejob.The
publish-wren-langchainjob (lines 68–77) references three outputs that are never declared in therelease-pleasejob'soutputsblock:
needs.release-please.outputs['wren-langchain--release_created'](line 70)needs.release-please.outputs['wren-langchain--version'](line 73)needs.release-please.outputs['wren-langchain--tag_name'](line 74)Without these declarations, all three expressions resolve to an empty string. The
if:condition on line 70 will never evaluate to'true', so the publish job will silently never run, and theversion/tag_nameinputs will always be empty.🐛 Proposed fix — add the missing output declarations
wren-core-wasm--release_created: ${{ steps.release.outputs['core/wren-core-wasm--release_created'] }} wren-core-wasm--tag_name: ${{ steps.release.outputs['core/wren-core-wasm--tag_name'] }} wren-core-wasm--version: ${{ steps.release.outputs['core/wren-core-wasm--version'] }} + wren-langchain--release_created: ${{ steps.release.outputs['sdk/wren-langchain--release_created'] }} + wren-langchain--tag_name: ${{ steps.release.outputs['sdk/wren-langchain--tag_name'] }} + wren-langchain--version: ${{ steps.release.outputs['sdk/wren-langchain--version'] }}🤖 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 @.github/workflows/release-please.yml around lines 15 - 24, Add three output declarations to the release-please job outputs: declare wren-langchain--release_created, wren-langchain--version, and wren-langchain--tag_name and map each to the corresponding step output from the release step (use the same pattern as existing entries, e.g. map wren-langchain--release_created to ${{ steps.release.outputs['core/wren-langchain--release_created'] }}, wren-langchain--version to ${{ steps.release.outputs['core/wren-langchain--version'] }}, and wren-langchain--tag_name to ${{ steps.release.outputs['core/wren-langchain--tag_name'] }}), so the publish-wren-langchain job can read needs.release-please.outputs['wren-langchain--*'] correctly.sdk/wren-langchain/tests/conformance/test_langchain_contract.py (1)
108-123:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winConformance test doesn’t verify actual JSON serializability.
The contract says
tool.invoke(...)returns a JSON-serializable envelope, but this test only checks keys/types. Add ajson.dumps(result)assertion so non-serializable payloads fail fast.Suggested addition
+import json @@ def test_each_tool_invoke_returns_envelope_dict(all_tools, tool_name, invoke_args): @@ result = tool.invoke(invoke_args) assert isinstance(result, dict) + json.dumps(result) assert "ok" in result🤖 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/conformance/test_langchain_contract.py` around lines 108 - 123, The test test_each_tool_invoke_returns_envelope_dict currently checks structure but not JSON serializability; after obtaining result from tool.invoke(invoke_args) (and after existing key/assert checks), call json.dumps(result) (import json at top if missing) and assert it does not raise to ensure the returned envelope from tool.invoke is JSON-serializable; reference test_each_tool_invoke_returns_envelope_dict, tool.invoke, and all_tools.get_tools to locate where to add this assertion.
🤖 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.
Inline comments:
In `@sdk/wren-langchain/src/wren_langchain/_tools.py`:
- Around line 37-57: wren_query lets callers pass arbitrarily large limit and
then calls table.to_pylist() which can OOM or produce huge responses; add a
strict upper bound (e.g. MAX_QUERY_ROWS constant) and validate the incoming
limit in wren_query before calling toolkit.query or materializing rows: if limit
is <=0 or > MAX_QUERY_ROWS, return make_error (or raise) with a clear message;
when materializing, only convert up to the validated limit (use
table.head(limit) or slice the result) so table.to_pylist() never produces more
than the allowed rows; update references in the function to toolkit.query(sql,
limit=limit), table.num_rows, table.to_pylist(), make_error and make_success
accordingly.
In `@sdk/wren-langchain/tests/unit/test_toolkit_runtime.py`:
- Around line 42-56: The test currently injects the same cached_connector into
every mocked WrenEngine via make_engine, producing a false positive when
asserting connector reuse; change make_engine so each new engine starts with a
distinct initial connector (e.g., give engines[1] a different MagicMock
_connector) before toolkit.query runs, then run the two queries under the
patched WrenEngine and assert that engines[1]._connector was replaced with
cached_connector after the second call; refer to the make_engine helper, the
WrenEngine patch, WrenToolkit.from_project, toolkit.query and the engines list
to implement this change.
---
Outside diff comments:
In @.github/workflows/release-please.yml:
- Around line 15-24: Add three output declarations to the release-please job
outputs: declare wren-langchain--release_created, wren-langchain--version, and
wren-langchain--tag_name and map each to the corresponding step output from the
release step (use the same pattern as existing entries, e.g. map
wren-langchain--release_created to ${{
steps.release.outputs['core/wren-langchain--release_created'] }},
wren-langchain--version to ${{
steps.release.outputs['core/wren-langchain--version'] }}, and
wren-langchain--tag_name to ${{
steps.release.outputs['core/wren-langchain--tag_name'] }}), so the
publish-wren-langchain job can read
needs.release-please.outputs['wren-langchain--*'] correctly.
In `@sdk/wren-langchain/tests/conformance/test_langchain_contract.py`:
- Around line 108-123: The test test_each_tool_invoke_returns_envelope_dict
currently checks structure but not JSON serializability; after obtaining result
from tool.invoke(invoke_args) (and after existing key/assert checks), call
json.dumps(result) (import json at top if missing) and assert it does not raise
to ensure the returned envelope from tool.invoke is JSON-serializable; reference
test_each_tool_invoke_returns_envelope_dict, tool.invoke, and
all_tools.get_tools to locate where to add this assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 446c9034-f45a-44bb-a2e8-53f52b59bf89
📒 Files selected for processing (47)
.github/workflows/publish-wren-langchain.yml.github/workflows/release-please.yml.github/workflows/sdk-langchain-ci.yml.release-please-manifest.jsonLICENSEREADME.mdrelease-please-config.jsonsdk/wren-langchain/.gitignoresdk/wren-langchain/LICENSEsdk/wren-langchain/README.mdsdk/wren-langchain/examples/langchain_demo.pysdk/wren-langchain/examples/langgraph_demo.pysdk/wren-langchain/pyproject.tomlsdk/wren-langchain/src/wren_langchain/__init__.pysdk/wren-langchain/src/wren_langchain/_envelope.pysdk/wren-langchain/src/wren_langchain/_format.pysdk/wren-langchain/src/wren_langchain/_memory_api.pysdk/wren-langchain/src/wren_langchain/_prompt.pysdk/wren-langchain/src/wren_langchain/_providers/__init__.pysdk/wren-langchain/src/wren_langchain/_providers/connection.pysdk/wren-langchain/src/wren_langchain/_providers/mdl_source.pysdk/wren-langchain/src/wren_langchain/_providers/memory.pysdk/wren-langchain/src/wren_langchain/_toolkit.pysdk/wren-langchain/src/wren_langchain/_tools.pysdk/wren-langchain/src/wren_langchain/_tools_memory.pysdk/wren-langchain/src/wren_langchain/exceptions.pysdk/wren-langchain/tests/__init__.pysdk/wren-langchain/tests/conformance/__init__.pysdk/wren-langchain/tests/conformance/test_langchain_contract.pysdk/wren-langchain/tests/conftest.pysdk/wren-langchain/tests/integration/__init__.pysdk/wren-langchain/tests/integration/conftest.pysdk/wren-langchain/tests/integration/test_langgraph_toolnode.pysdk/wren-langchain/tests/integration/test_memory_tools.pysdk/wren-langchain/tests/integration/test_runtime_tools.pysdk/wren-langchain/tests/unit/__init__.pysdk/wren-langchain/tests/unit/test_envelope.pysdk/wren-langchain/tests/unit/test_exceptions.pysdk/wren-langchain/tests/unit/test_memory_api.pysdk/wren-langchain/tests/unit/test_prompt.pysdk/wren-langchain/tests/unit/test_providers_connection.pysdk/wren-langchain/tests/unit/test_providers_mdl.pysdk/wren-langchain/tests/unit/test_providers_memory.pysdk/wren-langchain/tests/unit/test_toolkit_init.pysdk/wren-langchain/tests/unit/test_toolkit_runtime.pysdk/wren-langchain/tests/unit/test_tools_memory.pysdk/wren-langchain/tests/unit/test_tools_runtime.py
✅ Files skipped from review due to trivial changes (18)
- sdk/wren-langchain/LICENSE
- .release-please-manifest.json
- release-please-config.json
- sdk/wren-langchain/tests/unit/test_exceptions.py
- sdk/wren-langchain/.gitignore
- sdk/wren-langchain/src/wren_langchain/init.py
- sdk/wren-langchain/tests/integration/conftest.py
- sdk/wren-langchain/tests/conftest.py
- sdk/wren-langchain/src/wren_langchain/exceptions.py
- sdk/wren-langchain/tests/unit/test_providers_connection.py
- sdk/wren-langchain/src/wren_langchain/_tools_memory.py
- sdk/wren-langchain/pyproject.toml
- .github/workflows/sdk-langchain-ci.yml
- sdk/wren-langchain/tests/unit/test_toolkit_init.py
- sdk/wren-langchain/tests/unit/test_envelope.py
- sdk/wren-langchain/src/wren_langchain/_memory_api.py
- sdk/wren-langchain/tests/integration/test_memory_tools.py
- sdk/wren-langchain/tests/unit/test_tools_memory.py
🚧 Files skipped from review as they are similar to previous changes (15)
- sdk/wren-langchain/tests/unit/test_providers_mdl.py
- sdk/wren-langchain/tests/unit/test_providers_memory.py
- sdk/wren-langchain/src/wren_langchain/_providers/mdl_source.py
- sdk/wren-langchain/tests/integration/test_runtime_tools.py
- sdk/wren-langchain/tests/integration/test_langgraph_toolnode.py
- sdk/wren-langchain/src/wren_langchain/_providers/memory.py
- sdk/wren-langchain/src/wren_langchain/_providers/connection.py
- sdk/wren-langchain/tests/unit/test_memory_api.py
- LICENSE
- sdk/wren-langchain/examples/langgraph_demo.py
- .github/workflows/publish-wren-langchain.yml
- sdk/wren-langchain/src/wren_langchain/_prompt.py
- sdk/wren-langchain/src/wren_langchain/_toolkit.py
- sdk/wren-langchain/src/wren_langchain/_format.py
- sdk/wren-langchain/tests/unit/test_prompt.py
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
from
471f7b6 to
cd91108
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
sdk/wren-langchain/tests/integration/test_runtime_tools.py (1)
39-42: ⚡ Quick winAvoid order-dependent model assertions in integration test.
wren_list_modelsoutput order is not guaranteed as a contract; asserting index0can make this test flaky across manifest/build variations.Proposed test hardening
- assert envelope["data"]["models"][0]["name"] == "customers" + assert any(m.get("name") == "customers" for m in envelope["data"]["models"])🤖 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/integration/test_runtime_tools.py` around lines 39 - 42, The test is asserting a specific index which makes it order-dependent; change the assertion on envelope["data"]["models"] to check that some model has name "customers" (e.g. use any(m["name"] == "customers" for m in envelope["data"]["models"]) or assert "customers" in {m["name"] for m in envelope["data"]["models"]}) instead of envelope["data"]["models"][0]["name"] == "customers"; keep the existing envelope["ok"] and content checks but replace the index-based assertion with an order-independent membership check.sdk/wren-langchain/tests/integration/test_memory_tools.py (1)
50-68: 💤 Low valueConsider getting all tools in one call.
Lines 52–53 call
toolkit.get_tools()twice. While not a correctness issue (both tool objects target the same LanceDB on-disk path), a single call is cheaper and cleaner ifget_tools()allocates connections each time.♻️ Suggested cleanup
- store = next(t for t in toolkit.get_tools() if t.name == "wren_store_query") - recall = next(t for t in toolkit.get_tools() if t.name == "wren_recall_queries") + tools = {t.name: t for t in toolkit.get_tools()} + store = tools["wren_store_query"] + recall = tools["wren_recall_queries"]🤖 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/integration/test_memory_tools.py` around lines 50 - 68, The test calls toolkit.get_tools() twice; change it to a single call and reuse the result to avoid duplicate allocation. In test_store_then_recall_round_trip, call tools = toolkit.get_tools() once, then find store = next(t for t in tools if t.name == "wren_store_query") and recall = next(t for t in tools if t.name == "wren_recall_queries") so the rest of the test (store_env, recall_env, assertions) remains 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.
Inline comments:
In @.github/workflows/release-please.yml:
- Around line 69-74: The publish-wren-langchain job is referencing undefined
outputs (wren-langchain--release_created, wren-langchain--version,
wren-langchain--tag_name) from the release-please job; either add those outputs
to the release-please job (jobs.release-please.outputs) by mapping them to the
appropriate step outputs (e.g., steps.<step_id>.outputs.release_created,
.version, .tag_name) or update the publish job to use the actual output keys
that already exist; locate the release-please job and the step that
creates/releases artifacts (and the publish-wren-langchain job) and ensure the
output key names match exactly between jobs (use the step id that produces
version/tag_name and expose them under the wren-langchain--* names in
jobs.release-please.outputs if you prefer to keep the current references).
In `@sdk/wren-langchain/tests/unit/test_tools_memory.py`:
- Around line 152-153: The test currently inspects
fake_store.store_query.call_args.kwargs["tags"], which will raise KeyError if
tags was passed positionally; update the assertion to verify the full call
contract using fake_store.store_query.assert_called_once_with(...) or, if you
only care about tags, read it safely from the call args (e.g., inspect call_args
and use .kwargs.get("tags") or examine positional args) so the test fails with a
clear assertion rather than a KeyError; reference the fake_store.store_query
mock when making this change.
---
Nitpick comments:
In `@sdk/wren-langchain/tests/integration/test_memory_tools.py`:
- Around line 50-68: The test calls toolkit.get_tools() twice; change it to a
single call and reuse the result to avoid duplicate allocation. In
test_store_then_recall_round_trip, call tools = toolkit.get_tools() once, then
find store = next(t for t in tools if t.name == "wren_store_query") and recall =
next(t for t in tools if t.name == "wren_recall_queries") so the rest of the
test (store_env, recall_env, assertions) remains unchanged.
In `@sdk/wren-langchain/tests/integration/test_runtime_tools.py`:
- Around line 39-42: The test is asserting a specific index which makes it
order-dependent; change the assertion on envelope["data"]["models"] to check
that some model has name "customers" (e.g. use any(m["name"] == "customers" for
m in envelope["data"]["models"]) or assert "customers" in {m["name"] for m in
envelope["data"]["models"]}) instead of envelope["data"]["models"][0]["name"] ==
"customers"; keep the existing envelope["ok"] and content checks but replace the
index-based assertion with an order-independent membership check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 50f6de95-c5f1-4874-a9cc-f781029f3eec
📒 Files selected for processing (47)
.github/workflows/publish-wren-langchain.yml.github/workflows/release-please.yml.github/workflows/sdk-langchain-ci.yml.release-please-manifest.jsonLICENSEREADME.mdrelease-please-config.jsonsdk/wren-langchain/.gitignoresdk/wren-langchain/LICENSEsdk/wren-langchain/README.mdsdk/wren-langchain/examples/langchain_demo.pysdk/wren-langchain/examples/langgraph_demo.pysdk/wren-langchain/pyproject.tomlsdk/wren-langchain/src/wren_langchain/__init__.pysdk/wren-langchain/src/wren_langchain/_envelope.pysdk/wren-langchain/src/wren_langchain/_format.pysdk/wren-langchain/src/wren_langchain/_memory_api.pysdk/wren-langchain/src/wren_langchain/_prompt.pysdk/wren-langchain/src/wren_langchain/_providers/__init__.pysdk/wren-langchain/src/wren_langchain/_providers/connection.pysdk/wren-langchain/src/wren_langchain/_providers/mdl_source.pysdk/wren-langchain/src/wren_langchain/_providers/memory.pysdk/wren-langchain/src/wren_langchain/_toolkit.pysdk/wren-langchain/src/wren_langchain/_tools.pysdk/wren-langchain/src/wren_langchain/_tools_memory.pysdk/wren-langchain/src/wren_langchain/exceptions.pysdk/wren-langchain/tests/__init__.pysdk/wren-langchain/tests/conformance/__init__.pysdk/wren-langchain/tests/conformance/test_langchain_contract.pysdk/wren-langchain/tests/conftest.pysdk/wren-langchain/tests/integration/__init__.pysdk/wren-langchain/tests/integration/conftest.pysdk/wren-langchain/tests/integration/test_langgraph_toolnode.pysdk/wren-langchain/tests/integration/test_memory_tools.pysdk/wren-langchain/tests/integration/test_runtime_tools.pysdk/wren-langchain/tests/unit/__init__.pysdk/wren-langchain/tests/unit/test_envelope.pysdk/wren-langchain/tests/unit/test_exceptions.pysdk/wren-langchain/tests/unit/test_memory_api.pysdk/wren-langchain/tests/unit/test_prompt.pysdk/wren-langchain/tests/unit/test_providers_connection.pysdk/wren-langchain/tests/unit/test_providers_mdl.pysdk/wren-langchain/tests/unit/test_providers_memory.pysdk/wren-langchain/tests/unit/test_toolkit_init.pysdk/wren-langchain/tests/unit/test_toolkit_runtime.pysdk/wren-langchain/tests/unit/test_tools_memory.pysdk/wren-langchain/tests/unit/test_tools_runtime.py
✅ Files skipped from review due to trivial changes (18)
- LICENSE
- sdk/wren-langchain/LICENSE
- sdk/wren-langchain/tests/unit/test_exceptions.py
- sdk/wren-langchain/src/wren_langchain/init.py
- sdk/wren-langchain/.gitignore
- release-please-config.json
- sdk/wren-langchain/tests/conftest.py
- sdk/wren-langchain/tests/integration/conftest.py
- sdk/wren-langchain/tests/unit/test_providers_connection.py
- sdk/wren-langchain/examples/langchain_demo.py
- sdk/wren-langchain/src/wren_langchain/_providers/connection.py
- sdk/wren-langchain/pyproject.toml
- sdk/wren-langchain/tests/unit/test_memory_api.py
- sdk/wren-langchain/src/wren_langchain/_providers/memory.py
- .release-please-manifest.json
- sdk/wren-langchain/src/wren_langchain/_memory_api.py
- .github/workflows/publish-wren-langchain.yml
- .github/workflows/sdk-langchain-ci.yml
🚧 Files skipped from review as they are similar to previous changes (11)
- README.md
- sdk/wren-langchain/tests/unit/test_providers_memory.py
- sdk/wren-langchain/tests/unit/test_providers_mdl.py
- sdk/wren-langchain/src/wren_langchain/exceptions.py
- sdk/wren-langchain/tests/unit/test_envelope.py
- sdk/wren-langchain/tests/integration/test_langgraph_toolnode.py
- sdk/wren-langchain/tests/unit/test_toolkit_init.py
- sdk/wren-langchain/src/wren_langchain/_prompt.py
- sdk/wren-langchain/src/wren_langchain/_providers/mdl_source.py
- sdk/wren-langchain/src/wren_langchain/_format.py
- sdk/wren-langchain/src/wren_langchain/_toolkit.py
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
from
cd91108 to
fba79b7
Compare
Contributor
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (2)
sdk/wren-langchain/tests/unit/test_providers_connection.py (1)
74-84: ⚡ Quick winKeep
test_unknown_profile_name_raisesfully isolated from ambient env.This test only patches
list_profiles. IfProfileConnectionProviderlater callsget_active_profile()before explicit-profile validation, this can become flaky. Patchget_active_profilehere too for deterministic behavior.Suggested change
def test_unknown_profile_name_raises(monkeypatch, tmp_path): monkeypatch.setattr( "wren_langchain._providers.connection.list_profiles", lambda: {"prod": {"datasource": "postgres", "host": "x"}}, ) + monkeypatch.setattr( + "wren_langchain._providers.connection.get_active_profile", + lambda: ("prod", {"datasource": "postgres", "host": "x"}), + ) with pytest.raises(WrenToolkitInitError, match="profile.*not found"): ProfileConnectionProvider( project_path=tmp_path, explicit_profile="nonexistent", )🤖 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_providers_connection.py` around lines 74 - 84, The test test_unknown_profile_name_raises patches list_profiles but leaves get_active_profile unpatched, which can make the test flaky if ProfileConnectionProvider calls get_active_profile(); patch get_active_profile as well in the test to return a deterministic value (e.g., None or a known profile) so the constructor's explicit_profile validation is the only path exercised; update the test to monkeypatch "wren_langchain._providers.connection.get_active_profile" alongside "list_profiles" when instantiating ProfileConnectionProvider to ensure isolation..github/workflows/publish-wren-langchain.yml (1)
107-111: ⚡ Quick winPin to immutable commit SHA instead of mutable
release/v1branch.
@release/v1is a mutable branch that can receive force-push updates. The latest stable version is v1.14.0, commitcef2210(released 2026-04-07). Pin to this commit for better supply-chain security:uses: pypa/gh-action-pypi-publish@cef2210.🤖 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 @.github/workflows/publish-wren-langchain.yml around lines 107 - 111, The workflow currently references the mutable tag "pypa/gh-action-pypi-publish@release/v1"; change the action reference to the immutable commit SHA suggested (pypa/gh-action-pypi-publish@cef2210) so the publish step uses a fixed, auditable commit. Locate the "uses: pypa/gh-action-pypi-publish@release/v1" line in the publish-wren-langchain.yml workflow and replace the ref with "@cef2210" (keeping the surrounding inputs: repository-url, packages-dir, and attestations 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.
Inline comments:
In @.github/workflows/publish-wren-langchain.yml:
- Around line 74-79: The file uses open(path).read() and open(path,
"w").write(text) without specifying encoding, which risks corrupting non-ASCII
content on some runners; update these calls to explicitly use encoding="utf-8"
(and prefer using with statements) when reading and writing the file in this
section that performs re.subn on the file contents so the read/write operations
preserve UTF-8 characters.
- Around line 26-28: The workflow currently grants id-token: write at the
top-level permissions, giving all jobs OIDC access; move the id-token: write
permission out of the global permissions block and add a job-level permissions
entry for the publish job only (leave contents: read at workflow level),
updating the publish job (named "publish") to include permissions: id-token:
write (and contents: read if needed) so only publish has OIDC token while
validate-inputs and build keep the reduced global permission set.
- Around line 107-111: The publish step using
pypa/gh-action-pypi-publish@release/v1 inside this reusable workflow (invoked
via workflow_call) relies on OIDC Trusted Publishing which fails in
workflow_call contexts; either move the publish job out of this reusable
workflow into the top-level calling workflow (e.g., in release-please.yml) and
have it need: build so it runs with proper OIDC claims, or keep the job here but
switch to using a PyPI API token stored in a secret and passed to
pypa/gh-action-pypi-publish via the token input; additionally replace the
floating `@release/v1` ref with a pinned tag or commit SHA for reproducible,
secure pinning of the pypa/gh-action-pypi-publish action.
In `@sdk/wren-langchain/src/wren_langchain/_format.py`:
- Around line 28-41: The loop that computes keep can yield zero when every
single row exceeds CONTENT_CAP_BYTES, causing an empty JSON array and misleading
footer; update the logic after the loop to handle keep == 0 by producing a safe
truncated-single-row representation and an explicit warning instead of an empty
list: when keep == 0, serialize rows[0] (using json.dumps(..., default=str)),
truncate its UTF-8 byte representation to CONTENT_CAP_BYTES - 64 (preserving
valid UTF-8), set body to that truncated string (optionally wrapped or marked as
truncated), and set footer/warning to indicate a single-row truncation; ensure
variables referenced are keep, rows, CONTENT_CAP_BYTES, body, footer, and
warning so the change is localized to the existing return path.
In `@sdk/wren-langchain/src/wren_langchain/_memory_api.py`:
- Line 74: The code builds tag_str with ",".join(tags) which corrupts tags that
contain commas; update _memory_api.store() (and the tag_str construction) to
either validate and reject tags containing commas (raise a clear ValueError) or
switch to a safe delimiter/encoding (e.g., join with "\x1f" or
percent-encode/escape commas) and ensure the same decoding is used by
MemoryStore.store_query and the wren_store_query tool; pick one approach and
apply it consistently: add validation in _memory_api.store() (or replace the
join), update any consumer code that splits tag_str (MemoryStore.store_query and
wren_store_query) to decode using the chosen scheme, and add unit tests for tags
with commas to prevent regressions.
In `@sdk/wren-langchain/tests/integration/test_memory_tools.py`:
- Line 22: The fixture function project_with_memory declares an unused
tmp_path_factory parameter; remove tmp_path_factory from the function signature
so the fixture only accepts duckdb_project, and ensure no other references to
tmp_path_factory remain in project_with_memory to avoid misleading readers and
unused fixture injection.
---
Nitpick comments:
In @.github/workflows/publish-wren-langchain.yml:
- Around line 107-111: The workflow currently references the mutable tag
"pypa/gh-action-pypi-publish@release/v1"; change the action reference to the
immutable commit SHA suggested (pypa/gh-action-pypi-publish@cef2210) so the
publish step uses a fixed, auditable commit. Locate the "uses:
pypa/gh-action-pypi-publish@release/v1" line in the publish-wren-langchain.yml
workflow and replace the ref with "@cef2210" (keeping the surrounding inputs:
repository-url, packages-dir, and attestations unchanged).
In `@sdk/wren-langchain/tests/unit/test_providers_connection.py`:
- Around line 74-84: The test test_unknown_profile_name_raises patches
list_profiles but leaves get_active_profile unpatched, which can make the test
flaky if ProfileConnectionProvider calls get_active_profile(); patch
get_active_profile as well in the test to return a deterministic value (e.g.,
None or a known profile) so the constructor's explicit_profile validation is the
only path exercised; update the test to monkeypatch
"wren_langchain._providers.connection.get_active_profile" alongside
"list_profiles" when instantiating ProfileConnectionProvider to ensure
isolation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 62e7802b-841f-486f-b224-68e71a6e42a2
📒 Files selected for processing (47)
.github/workflows/publish-wren-langchain.yml.github/workflows/release-please.yml.github/workflows/sdk-langchain-ci.yml.release-please-manifest.jsonLICENSEREADME.mdrelease-please-config.jsonsdk/wren-langchain/.gitignoresdk/wren-langchain/LICENSEsdk/wren-langchain/README.mdsdk/wren-langchain/examples/langchain_demo.pysdk/wren-langchain/examples/langgraph_demo.pysdk/wren-langchain/pyproject.tomlsdk/wren-langchain/src/wren_langchain/__init__.pysdk/wren-langchain/src/wren_langchain/_envelope.pysdk/wren-langchain/src/wren_langchain/_format.pysdk/wren-langchain/src/wren_langchain/_memory_api.pysdk/wren-langchain/src/wren_langchain/_prompt.pysdk/wren-langchain/src/wren_langchain/_providers/__init__.pysdk/wren-langchain/src/wren_langchain/_providers/connection.pysdk/wren-langchain/src/wren_langchain/_providers/mdl_source.pysdk/wren-langchain/src/wren_langchain/_providers/memory.pysdk/wren-langchain/src/wren_langchain/_toolkit.pysdk/wren-langchain/src/wren_langchain/_tools.pysdk/wren-langchain/src/wren_langchain/_tools_memory.pysdk/wren-langchain/src/wren_langchain/exceptions.pysdk/wren-langchain/tests/__init__.pysdk/wren-langchain/tests/conformance/__init__.pysdk/wren-langchain/tests/conformance/test_langchain_contract.pysdk/wren-langchain/tests/conftest.pysdk/wren-langchain/tests/integration/__init__.pysdk/wren-langchain/tests/integration/conftest.pysdk/wren-langchain/tests/integration/test_langgraph_toolnode.pysdk/wren-langchain/tests/integration/test_memory_tools.pysdk/wren-langchain/tests/integration/test_runtime_tools.pysdk/wren-langchain/tests/unit/__init__.pysdk/wren-langchain/tests/unit/test_envelope.pysdk/wren-langchain/tests/unit/test_exceptions.pysdk/wren-langchain/tests/unit/test_memory_api.pysdk/wren-langchain/tests/unit/test_prompt.pysdk/wren-langchain/tests/unit/test_providers_connection.pysdk/wren-langchain/tests/unit/test_providers_mdl.pysdk/wren-langchain/tests/unit/test_providers_memory.pysdk/wren-langchain/tests/unit/test_toolkit_init.pysdk/wren-langchain/tests/unit/test_toolkit_runtime.pysdk/wren-langchain/tests/unit/test_tools_memory.pysdk/wren-langchain/tests/unit/test_tools_runtime.py
✅ Files skipped from review due to trivial changes (11)
- sdk/wren-langchain/LICENSE
- .release-please-manifest.json
- sdk/wren-langchain/.gitignore
- sdk/wren-langchain/src/wren_langchain/init.py
- sdk/wren-langchain/tests/conftest.py
- sdk/wren-langchain/src/wren_langchain/_providers/mdl_source.py
- sdk/wren-langchain/src/wren_langchain/_providers/memory.py
- .github/workflows/sdk-langchain-ci.yml
- sdk/wren-langchain/tests/conformance/test_langchain_contract.py
- sdk/wren-langchain/pyproject.toml
- sdk/wren-langchain/tests/unit/test_prompt.py
🚧 Files skipped from review as they are similar to previous changes (19)
- sdk/wren-langchain/tests/unit/test_providers_mdl.py
- LICENSE
- sdk/wren-langchain/tests/unit/test_exceptions.py
- release-please-config.json
- sdk/wren-langchain/tests/unit/test_providers_memory.py
- sdk/wren-langchain/tests/unit/test_envelope.py
- sdk/wren-langchain/src/wren_langchain/_providers/connection.py
- README.md
- sdk/wren-langchain/tests/unit/test_toolkit_runtime.py
- sdk/wren-langchain/src/wren_langchain/exceptions.py
- sdk/wren-langchain/src/wren_langchain/_tools.py
- sdk/wren-langchain/tests/unit/test_toolkit_init.py
- sdk/wren-langchain/tests/unit/test_memory_api.py
- sdk/wren-langchain/examples/langgraph_demo.py
- sdk/wren-langchain/tests/unit/test_tools_memory.py
- .github/workflows/release-please.yml
- sdk/wren-langchain/tests/integration/conftest.py
- sdk/wren-langchain/src/wren_langchain/_prompt.py
- sdk/wren-langchain/examples/langchain_demo.py
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
from
fba79b7 to
f5cedd6
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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.
Inline comments:
In `@sdk/wren-langchain/src/wren_langchain/_tools_memory.py`:
- Around line 95-119: The code in _build_store_query/wren_store_query must
normalize tags before the try/except to avoid format_store_content raising
outside error handling; move or add a tags_normalized = tags or [] (or
equivalent) at the start of wren_store_query and use that normalized variable
when calling toolkit.memory.store(...) and format_store_content(...), and when
building the data payload for make_success so any exception from
format_store_content is caught by the existing try/except and respects
raise_on_error (keep make_error/make_success usage as-is).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: c6b0a522-8ba6-4c8c-95f7-58cb186615fb
📒 Files selected for processing (47)
.github/workflows/publish-wren-langchain.yml.github/workflows/release-please.yml.github/workflows/sdk-langchain-ci.yml.release-please-manifest.jsonLICENSEREADME.mdrelease-please-config.jsonsdk/wren-langchain/.gitignoresdk/wren-langchain/LICENSEsdk/wren-langchain/README.mdsdk/wren-langchain/examples/langchain_demo.pysdk/wren-langchain/examples/langgraph_demo.pysdk/wren-langchain/pyproject.tomlsdk/wren-langchain/src/wren_langchain/__init__.pysdk/wren-langchain/src/wren_langchain/_envelope.pysdk/wren-langchain/src/wren_langchain/_format.pysdk/wren-langchain/src/wren_langchain/_memory_api.pysdk/wren-langchain/src/wren_langchain/_prompt.pysdk/wren-langchain/src/wren_langchain/_providers/__init__.pysdk/wren-langchain/src/wren_langchain/_providers/connection.pysdk/wren-langchain/src/wren_langchain/_providers/mdl_source.pysdk/wren-langchain/src/wren_langchain/_providers/memory.pysdk/wren-langchain/src/wren_langchain/_toolkit.pysdk/wren-langchain/src/wren_langchain/_tools.pysdk/wren-langchain/src/wren_langchain/_tools_memory.pysdk/wren-langchain/src/wren_langchain/exceptions.pysdk/wren-langchain/tests/__init__.pysdk/wren-langchain/tests/conformance/__init__.pysdk/wren-langchain/tests/conformance/test_langchain_contract.pysdk/wren-langchain/tests/conftest.pysdk/wren-langchain/tests/integration/__init__.pysdk/wren-langchain/tests/integration/conftest.pysdk/wren-langchain/tests/integration/test_langgraph_toolnode.pysdk/wren-langchain/tests/integration/test_memory_tools.pysdk/wren-langchain/tests/integration/test_runtime_tools.pysdk/wren-langchain/tests/unit/__init__.pysdk/wren-langchain/tests/unit/test_envelope.pysdk/wren-langchain/tests/unit/test_exceptions.pysdk/wren-langchain/tests/unit/test_memory_api.pysdk/wren-langchain/tests/unit/test_prompt.pysdk/wren-langchain/tests/unit/test_providers_connection.pysdk/wren-langchain/tests/unit/test_providers_mdl.pysdk/wren-langchain/tests/unit/test_providers_memory.pysdk/wren-langchain/tests/unit/test_toolkit_init.pysdk/wren-langchain/tests/unit/test_toolkit_runtime.pysdk/wren-langchain/tests/unit/test_tools_memory.pysdk/wren-langchain/tests/unit/test_tools_runtime.py
✅ Files skipped from review due to trivial changes (18)
- .release-please-manifest.json
- sdk/wren-langchain/.gitignore
- sdk/wren-langchain/tests/unit/test_exceptions.py
- LICENSE
- sdk/wren-langchain/src/wren_langchain/init.py
- sdk/wren-langchain/tests/unit/test_providers_memory.py
- sdk/wren-langchain/src/wren_langchain/_providers/memory.py
- sdk/wren-langchain/tests/integration/conftest.py
- .github/workflows/sdk-langchain-ci.yml
- .github/workflows/release-please.yml
- sdk/wren-langchain/src/wren_langchain/_memory_api.py
- sdk/wren-langchain/tests/integration/test_runtime_tools.py
- sdk/wren-langchain/src/wren_langchain/_providers/mdl_source.py
- sdk/wren-langchain/LICENSE
- sdk/wren-langchain/tests/unit/test_toolkit_runtime.py
- sdk/wren-langchain/src/wren_langchain/_toolkit.py
- sdk/wren-langchain/tests/unit/test_memory_api.py
- sdk/wren-langchain/src/wren_langchain/_envelope.py
🚧 Files skipped from review as they are similar to previous changes (15)
- README.md
- release-please-config.json
- sdk/wren-langchain/src/wren_langchain/exceptions.py
- sdk/wren-langchain/tests/unit/test_providers_connection.py
- sdk/wren-langchain/tests/conftest.py
- sdk/wren-langchain/tests/unit/test_tools_runtime.py
- sdk/wren-langchain/tests/unit/test_toolkit_init.py
- .github/workflows/publish-wren-langchain.yml
- sdk/wren-langchain/tests/integration/test_memory_tools.py
- sdk/wren-langchain/tests/unit/test_tools_memory.py
- sdk/wren-langchain/examples/langchain_demo.py
- sdk/wren-langchain/tests/unit/test_prompt.py
- sdk/wren-langchain/src/wren_langchain/_prompt.py
- sdk/wren-langchain/src/wren_langchain/_format.py
- sdk/wren-langchain/pyproject.toml
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
from
f5cedd6 to
3cfd125
Compare
…tion
Introduces wren-langchain at sdk/wren-langchain/, a new Python package
that exposes WrenToolkit — a facade over an existing CLI-prepared Wren
project producing LangChain/LangGraph-compatible tools and a system
prompt.
Public API:
toolkit = WrenToolkit.from_project(path, *, profile=None)
toolkit.query / dry_plan / dry_run # direct sync API, raises
toolkit.memory.fetch / recall / store # raises MemoryNotEnabledError
toolkit.get_tools(include_memory_write=True, raise_on_error=False)
toolkit.system_prompt() # 3-section markdown
Six LLM-facing tools (3 always-on runtime + 3 memory-gated, auto-detected
from .wren/memory/): wren_query, wren_dry_plan, wren_list_models,
wren_fetch_context, wren_recall_queries, wren_store_query.
Key behaviours:
- Manifest read-through: target/mdl.json re-read per tool call so external
\`wren context build\` is auto-picked-up; no toolkit.reload() needed.
- Connector cached at toolkit level to avoid DB reconnects.
- Project-local .env auto-loaded by from_project so \${VAR} secrets resolve
regardless of caller CWD.
- Envelope error contract for LLM tools (ok/content/error) with secret
redaction and 4 KB metadata cap; direct API raises Pythonically.
- Sync only; LangChain auto-bridges to thread pool for async LangGraph.
- One toolkit per agent — no multi-toolkit naming in v0.1.
Packaging:
- Datasource pass-through extras: pip install \"wren-langchain[mysql,memory]\"
installs the matching wren-engine[mysql,memory] in one shot. Available
extras: postgres, mysql, bigquery, snowflake, clickhouse, trino, mssql,
databricks, redshift, spark, athena, oracle, memory, all.
- Apache 2.0 license file included in package, root LICENSE path map
updated for sdk/**.
CI / release automation:
- .github/workflows/sdk-langchain-ci.yml runs ruff (check + format),
pytest (Python 3.11 / 3.12), and \`python -m build\` (asserts LICENSE
is bundled in the wheel) on any PR touching sdk/wren-langchain/**.
- Registered with release-please (release-please-config.json,
.release-please-manifest.json) so CHANGELOG and version bumps are
generated from conventional commits.
- .github/workflows/publish-wren-langchain.yml mirrors publish-wren.yml
(PyPI Trusted Publisher / OIDC, no API token).
- .github/workflows/release-please.yml wired to dispatch the publish
workflow when a wren-langchain release PR is merged.
Examples (examples/):
- langchain_demo.py uses langchain.agents.create_agent (high-level).
- langgraph_demo.py builds the ReAct loop from LangGraph primitives with
optional streaming.
Tests (93 default + 2 slow, ruff clean, format clean):
- tests/unit (60): mocked Core deps
- tests/integration (8, 2 marked slow): real DuckDB and LanceDB
- tests/conformance (25): every tool satisfies LangChain BaseTool spec
- tests/integration/test_langgraph_toolnode: simulated tool_call flows
through a compiled ToolNode graph end-to-end without a real LLM
Dependencies: wren-engine >= 0.5.0, langchain >= 1.0, langgraph >= 1.0.
PaulChen79
force-pushed
the
feat/langchain-sdk
branch
from
3cfd125 to
3222ce8
Compare
goldmedal
approved these changes
May 8, 2026
Collaborator
goldmedal
left a comment
goldmedal
left a comment
There was a problem hiding this comment.
Thanks @PaulChen79, Overall looks great. I didn't review the details, but the CI flow and the project structures make sense to me. 🚀
This was referenced May 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Introduces
wren-langchainatsdk/wren-langchain/— a new Python package that exposesWrenToolkit, a facade over an existing CLI-prepared Wren project producing LangChain/LangGraph-compatible tools and a system prompt.What's in this PR
sdk/wren-langchain/— full package:WrenToolkit, 6 LLM-facing tools (3 always-on runtime + 3 memory-gated), direct Python API for sync use,system_prompt()adapted from the Wren CLI'swren-usageskill.examples/—langchain_demo.py(high-levelcreate_agent) andlanggraph_demo.py(LangGraph primitives + optional streaming)..github/workflows/publish-wren-langchain.ymlmirrorspublish-wren.yml(Trusted Publisher / OIDC, no API token).release-please; CHANGELOG + version bumps are auto-generated from conventional commits.LICENSEpath map updated forsdk/**; rootREADME.mdpackage layout reflects shipped LangChain SDK and remaining "coming soon" siblings.Key behaviours (locked design decisions)
from_project(): eager prereqs validation (wren_project.yml+target/mdl.jsonmust exist; profile resolves), lazy resources, 3-layer profile fallback (kwarg →wren_project.ymlprofile:→ globally active), project-local.envauto-loading.target/mdl.jsonre-read per tool call so externalwren context buildis auto-picked-up.<project>/.wren/memory/; LLM tools self-filter when disabled.ok/content/error) with secret redaction and 4 KB metadata cap; direct API raises Pythonically.wren-usageCLI skill (recall → fetch context → write SQL → dry_plan if complex → execute → store) with explicit "skip ONLY when" lists in place of soft hedges that GPT-4o ignored.Packaging
Datasource pass-through extras:
postgres,mysql,bigquery,snowflake,clickhouse,trino,mssql,databricks,redshift,spark,athena,oracle, plusmemoryandall.Tests
tests/unit(60): mocked Core depstests/integration(8, 2 markedslow): real DuckDB + LanceDBtests/conformance(25): every tool satisfies LangChainBaseToolspectests/integration/test_langgraph_toolnode: simulated tool_call routed through a compiledToolNodegraph end-to-end without a real LLMTest plan
pytestgreen on default suite (93 tests)pytest -m slowgreen on real-LanceDB suite (2 tests)ruff check .cleanpython -m buildproduces sdist + wheel; LICENSE bundled in dist-infopython -c "from wren_langchain import WrenToolkit"workslangchain_demo.py,langgraph_demo.py) lint-clean and import-safewren-langchainon TestPyPI + PyPI (one-time, by Canner org owner)Dependencies
wren-engine >= 0.5.0langchain >= 1.0langgraph >= 1.0Summary by CodeRabbit
New Features
Documentation
Tests
Chores