Skip to content

Conversation

@jsonbailey
Copy link
Contributor

@jsonbailey jsonbailey commented Oct 3, 2025

Note

Refactors LangChain integration to an AIProvider-based provider with a new invoke flow and metrics, removes TrackedChat, and updates tests and configs.

  • Core (LangChainProvider)
    • Convert to AIProvider subclass with constructor-held BaseChatModel, invokeModel, and getChatModel.
    • Add static create(aiConfig) and createLangChainModel factory using initChatModel and mapProvider.
    • Replace createTokenUsage/tracking helpers with createAIMetrics returning { success, usage }.
    • Keep convertMessagesToLangChain and provider mapping (e.g., geminigoogle-genai).
  • Removal
    • Delete LangChainTrackedChat and its exports; index.ts now exports only LangChainProvider.
  • Tests
    • Update message imports to @langchain/core/messages; mock langchain/chat_models/universal.
    • Add tests for createAIMetrics and mapProvider; adjust expectations and formatting.
  • Config
    • Update Jest config: use transform, broaden testMatch, simplify coverage settings, and add moduleFileExtensions.
    • Adjust ESLint tsconfig includes/excludes.

Written by Cursor Bugbot for commit fe29f8f. This will update automatically on new commits. Configure here.

@jsonbailey jsonbailey changed the title Convert LangChain implementation to new AIProvider interface feat: Convert LangChain implementation to new AIProvider interface Oct 3, 2025
@jsonbailey jsonbailey marked this pull request as ready for review October 3, 2025 15:46
@jsonbailey jsonbailey requested a review from a team as a code owner October 3, 2025 15:46
{
"extends": "./tsconfig.json",
"include": ["src/**/*", "**/*.test.ts", "**/*.spec.ts"]
"include": ["/**/*.ts"],
Copy link

Choose a reason for hiding this comment

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

Bug: ESLint Include Pattern Uses Absolute Path

The include pattern ["/**/*.ts"] in tsconfig.eslint.json uses a leading slash, which makes it an absolute path from the filesystem root. This prevents ESLint from finding TypeScript files within the package, as it expects a relative path from the project directory.

Fix in Cursor Fix in Web

if (langChainResponse?.response_metadata?.tokenUsage) {
const { tokenUsage } = langChainResponse.response_metadata;
usage = {
total: tokenUsage.totalTokens || 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought: should this be promptTokens + completionTokens instead? I don't know if totalTokens would ever be different than prompt+completion, but if it ever was I could see users being confused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am trying to find the conversation where this came up before. I believe @kinyoklion mentioned it. We used total tokens because it is possible some models may use additional tokens not accounted for in just the prompt / completion tokens.

Copy link
Member

@kinyoklion kinyoklion Oct 7, 2025

Choose a reason for hiding this comment

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

Yeah, that is the thought. We originally didn't know that prompt + completion = total would be a guarantee. Which seems more likely if we eventually care about thinking tokens.

For example total may become prompt + thinking + completion in which case our total would be incorrect if it was just prompt + token.

Copy link
Member

Choose a reason for hiding this comment

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

Though some providers currently aren't charging for thinking so they are excluding it from their total calculation. But I could see it change either way.

@jsonbailey jsonbailey merged commit 2143219 into jb/sdk-1454/ai-provider-langchain Oct 7, 2025
5 of 6 checks passed
@jsonbailey jsonbailey deleted the jb/sdk-1454/ai-provider-langchain-new-interface branch October 7, 2025 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants