Skip to content

fix(core): improve langsmith loader error messages#35648

Open
Divy yadav (dvy246) wants to merge 2 commits intolangchain-ai:masterfrom
dvy246:fix-core-langsmith-loader-errors
Open

fix(core): improve langsmith loader error messages#35648
Divy yadav (dvy246) wants to merge 2 commits intolangchain-ai:masterfrom
dvy246:fix-core-langsmith-loader-errors

Conversation

@dvy246
Copy link

@dvy246 Divy yadav (dvy246) commented Mar 8, 2026

Summary

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.

Testing

  • make format
  • make lint
  • make test TEST_FILE=tests/unit_tests/document_loaders/test_langsmith.py

@github-actions github-actions bot added core `langchain-core` package issues & PRs external fix For PRs that implement a fix and removed fix For PRs that implement a fix labels Mar 8, 2026
@codspeed-hq
Copy link

codspeed-hq bot commented Mar 8, 2026

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 13 untouched benchmarks
⏩ 23 skipped benchmarks1


Comparing dvy246:fix-core-langsmith-loader-errors (96ad25f) with master (fcca6e2)2

Open in CodSpeed

Footnotes

  1. 23 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on master (7cef35b) during the generation of this report, so fcca6e2 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@org-membership-reviewer org-membership-reviewer bot added the size: S 50-199 LOC label Mar 9, 2026
Copy link
Contributor

@gambletan Ethan T. (gambletan) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvement to error reporting in the LangSmith loader. A few specific observations:

  1. _get_content_from_inputs is well-structured: The progressive path tracking via traversed_keys gives clear error messages like "missing key 'third' under 'first'". This is much more debuggable than a bare KeyError from content[key].

  2. Type check before key access: The isinstance(content, Mapping) guard (line ~155) catches the case where traversal hits a non-dict value mid-path (e.g., content_key="a.b" but inputs["a"] is a string). The error message clearly identifies the type mismatch. Good.

  3. ValueError message on __init__: The new message "Received both client and client_kwargs..." is clear and actionable. Previously the bare raise ValueError gave no guidance at all.

  4. Test for the init validation: test_init_with_client_and_client_kwargs_raises uses pytest.raises(ValueError, match=...) which validates both the exception type and the message. Clean.

  5. Minor: In _get_content_from_inputs, the full_path variable is computed as ".".join(content_key) which could be an empty string if content_key is empty. However, when content_key is empty, the for key in content_key loop never executes and content (which is inputs) is returned directly, so this is fine — full_path is never used in the empty case.

  6. Suggestion: The function currently raises KeyError for missing/invalid keys. Since this is a user-facing configuration error (wrong content_key), you might consider raising ValueError instead, which is more conventional for "bad argument" scenarios. KeyError is typically for dict-style access patterns. That said, the original code raised KeyError implicitly, so keeping it as KeyError maintains backward compatibility for anyone catching it.

Overall a solid usability improvement. LGTM.

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

Labels

core `langchain-core` package issues & PRs external fix For PRs that implement a fix size: S 50-199 LOC

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants