mirror of
https://github.com/hwchase17/langchain.git
synced 2026-07-11 09:26:29 +00:00
fix(core): improve langsmith loader error messages (#35648)
`LangSmithLoader.lazy_load()` now raises a `ValueError` with a descriptive message when a configured `content_key` cannot be resolved against an example's inputs — whether a key along the path is missing or the path runs into a non-mapping value. Previously the missing-key case raised a bare `KeyError` and the non-mapping case raised a `TypeError`, depending on the payload. Callers that caught `KeyError` or `TypeError` around `lazy_load()` to detect a misconfigured `content_key` should now catch `ValueError`. --- Improve `LangSmithLoader` error handling by surfacing clearer exceptions for conflicting client configuration and invalid nested `content_key` paths. Valid loader behavior is unchanged; this only improves invalid-input diagnostics. When a `content_key` cannot be resolved against an example's inputs, `lazy_load()` now raises a `ValueError` whose message names the full path, the offending key, and where traversal stopped — instead of a bare `KeyError` (missing key) or an opaque `TypeError` (path running into a non-mapping value, e.g. a leaf string). --------- Co-authored-by: Divy Yadav <yadavdipu296@gmai.com> Co-authored-by: Mason Daugherty <github@mdrxy.com> Co-authored-by: Mason Daugherty <mason@langchain.dev>
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
import datetime
|
||||
import json
|
||||
import uuid
|
||||
from collections.abc import Callable, Iterator, Sequence
|
||||
from collections.abc import Callable, Iterator, Mapping, Sequence
|
||||
from typing import Any
|
||||
|
||||
from langsmith import Client as LangSmithClient
|
||||
@@ -94,7 +94,10 @@ class LangSmithLoader(BaseLoader):
|
||||
ValueError: If both `client` and `client_kwargs` are provided.
|
||||
""" # noqa: E501
|
||||
if client and client_kwargs:
|
||||
msg = "Only one of 'client' and 'client_kwargs' should be provided."
|
||||
msg = (
|
||||
"Received both `client` and `client_kwargs`. "
|
||||
"Pass `client_kwargs` only when `client` is not provided."
|
||||
)
|
||||
raise ValueError(msg)
|
||||
self._client = client or LangSmithClient(**client_kwargs)
|
||||
self.content_key = list(content_key.split(".")) if content_key else []
|
||||
@@ -124,9 +127,7 @@ class LangSmithLoader(BaseLoader):
|
||||
metadata=self.metadata,
|
||||
filter=self.filter,
|
||||
):
|
||||
content: Any = example.inputs
|
||||
for key in self.content_key:
|
||||
content = content[key]
|
||||
content = _get_content_from_inputs(example.inputs, self.content_key)
|
||||
content_str = self.format_content(content)
|
||||
metadata = pydantic_to_dict(example)
|
||||
# Stringify datetime and UUID types.
|
||||
@@ -135,6 +136,44 @@ class LangSmithLoader(BaseLoader):
|
||||
yield Document(content_str, metadata=metadata)
|
||||
|
||||
|
||||
def _get_content_from_inputs(inputs: Any, content_key: Sequence[str]) -> Any:
|
||||
"""Resolve nested example input content for `LangSmithLoader`.
|
||||
|
||||
Args:
|
||||
inputs: Example input payload returned by LangSmith.
|
||||
content_key: Ordered key path used to extract the document content.
|
||||
|
||||
Returns:
|
||||
The extracted content value.
|
||||
|
||||
Raises:
|
||||
ValueError: If a key in `content_key` is missing, or a value along the path
|
||||
(including `inputs` itself) is not a mapping.
|
||||
"""
|
||||
content = inputs
|
||||
full_path = ".".join(content_key)
|
||||
|
||||
for i, key in enumerate(content_key):
|
||||
current_path = ".".join(content_key[:i]) or "<root>"
|
||||
if not isinstance(content, Mapping):
|
||||
msg = (
|
||||
f"Could not resolve content_key {full_path!r}: expected a mapping at "
|
||||
f"{current_path!r}, but found {type(content).__name__}."
|
||||
)
|
||||
# A too-deep `content_key` is an invalid-argument error, not a runtime
|
||||
# type bug, so it is unified with the missing-key case as `ValueError`.
|
||||
raise ValueError(msg) # noqa: TRY004
|
||||
if key not in content:
|
||||
msg = (
|
||||
f"Could not resolve content_key {full_path!r}: missing key {key!r} "
|
||||
f"under {current_path!r}."
|
||||
)
|
||||
raise ValueError(msg)
|
||||
content = content[key]
|
||||
|
||||
return content
|
||||
|
||||
|
||||
def _stringify(x: str | dict[str, Any]) -> str:
|
||||
if isinstance(x, str):
|
||||
return x
|
||||
|
||||
@@ -1,11 +1,14 @@
|
||||
import datetime
|
||||
import json
|
||||
import uuid
|
||||
from typing import Any
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from langsmith.schemas import Example
|
||||
|
||||
from langchain_core.document_loaders import LangSmithLoader
|
||||
from langchain_core.document_loaders.langsmith import _get_content_from_inputs
|
||||
from langchain_core.documents import Document
|
||||
from langchain_core.tracers._compat import pydantic_to_dict
|
||||
|
||||
@@ -14,10 +17,20 @@ def test_init() -> None:
|
||||
LangSmithLoader(api_key="secret")
|
||||
|
||||
|
||||
def test_init_client_and_client_kwargs_conflict() -> None:
|
||||
"""Passing both `client` and `client_kwargs` should raise."""
|
||||
with pytest.raises(ValueError, match="Only one of 'client' and 'client_kwargs'"):
|
||||
LangSmithLoader(client=MagicMock(), api_key="secret")
|
||||
def test_init_with_client_and_client_kwargs_raises() -> None:
|
||||
client = MagicMock()
|
||||
|
||||
with pytest.raises(ValueError, match="Received both `client` and `client_kwargs`"):
|
||||
LangSmithLoader(client=client, api_key="secret")
|
||||
|
||||
|
||||
def test_init_with_client_only() -> None:
|
||||
"""A bare `client` (no `client_kwargs`) should be accepted."""
|
||||
client = MagicMock()
|
||||
|
||||
loader = LangSmithLoader(client=client)
|
||||
|
||||
assert loader._client is client
|
||||
|
||||
|
||||
EXAMPLES = [
|
||||
@@ -67,3 +80,79 @@ def test_lazy_load() -> None:
|
||||
)
|
||||
actual = list(loader.lazy_load())
|
||||
assert expected == actual
|
||||
|
||||
|
||||
@patch("langsmith.Client.list_examples", MagicMock(return_value=iter(EXAMPLES[:1])))
|
||||
def test_lazy_load_with_empty_content_key_returns_whole_inputs() -> None:
|
||||
"""An empty `content_key` (the default) yields the full inputs payload."""
|
||||
loader = LangSmithLoader(api_key="dummy", dataset_id="mock")
|
||||
|
||||
docs = list(loader.lazy_load())
|
||||
|
||||
assert len(docs) == 1
|
||||
assert docs[0].page_content == json.dumps({"first": {"second": "foo"}}, indent=2)
|
||||
|
||||
|
||||
@patch("langsmith.Client.list_examples", MagicMock(return_value=iter(EXAMPLES[:1])))
|
||||
def test_lazy_load_with_missing_content_key_raises() -> None:
|
||||
loader = LangSmithLoader(
|
||||
api_key="dummy",
|
||||
dataset_id="mock",
|
||||
content_key="first.third",
|
||||
)
|
||||
|
||||
with pytest.raises(
|
||||
ValueError,
|
||||
match=r"Could not resolve content_key 'first\.third': "
|
||||
r"missing key 'third' under 'first'",
|
||||
):
|
||||
list(loader.lazy_load())
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("inputs", "content_key", "expected"),
|
||||
[
|
||||
# Empty key path (the default) returns the whole payload.
|
||||
({"first": {"second": "foo"}}, [], {"first": {"second": "foo"}}),
|
||||
# Partial path resolves to an intermediate mapping.
|
||||
({"first": {"second": "foo"}}, ["first"], {"second": "foo"}),
|
||||
# Full path resolves to a leaf value.
|
||||
({"first": {"second": "foo"}}, ["first", "second"], "foo"),
|
||||
],
|
||||
)
|
||||
def test_get_content_from_inputs_resolves_path(
|
||||
inputs: Any, content_key: list[str], expected: Any
|
||||
) -> None:
|
||||
assert _get_content_from_inputs(inputs, content_key) == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("inputs", "content_key", "match"),
|
||||
[
|
||||
# Missing key at the root level reports the "<root>" context.
|
||||
(
|
||||
{"first": {"second": "foo"}},
|
||||
["missing"],
|
||||
r"missing key 'missing' under '<root>'",
|
||||
),
|
||||
# Missing key nested under an existing mapping.
|
||||
(
|
||||
{"first": {"second": "foo"}},
|
||||
["first", "third"],
|
||||
r"missing key 'third' under 'first'",
|
||||
),
|
||||
# Path traverses past a leaf value that is not a mapping.
|
||||
(
|
||||
{"first": {"second": "foo"}},
|
||||
["first", "second", "third"],
|
||||
r"expected a mapping at 'first\.second', but found str",
|
||||
),
|
||||
# The root inputs payload itself is not a mapping.
|
||||
(None, ["first"], r"expected a mapping at '<root>', but found NoneType"),
|
||||
],
|
||||
)
|
||||
def test_get_content_from_inputs_raises(
|
||||
inputs: Any, content_key: list[str], match: str
|
||||
) -> None:
|
||||
with pytest.raises(ValueError, match=match):
|
||||
_get_content_from_inputs(inputs, content_key)
|
||||
|
||||
Reference in New Issue
Block a user