Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(openai): Make only AzureOpenAI respect Azure env vars, remove class defaults, update withStructuredOutput defaults #7535

Merged
merged 22 commits into from
Jan 25, 2025

Conversation

jacoblee93
Copy link
Collaborator

@jacoblee93 jacoblee93 commented Jan 16, 2025

summary of changes:

  • moves azure-specific properties from ChatOpenAI to AzureChatOpenAI
  • same for embeddings and llm
  • removes default values (defaults to undefined; let openai populate default)
  • update default withStructuredOutput method to be jsonSchema for models that support it

Copy link

vercel bot commented Jan 16, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
langchainjs-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 25, 2025 1:43am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
langchainjs-api-refs ⬜️ Ignored (Inspect) Jan 25, 2025 1:43am

@dosubot dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. auto:improvement Medium size change to existing code to handle new use-cases labels Jan 16, 2025
@jacoblee93
Copy link
Collaborator Author

CC @sinedied

Copy link
Contributor

@sinedied sinedied left a comment

Choose a reason for hiding this comment

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

Can I ask why only moving the env vars and not all azure related properties, since it's a breaking change anyways? (Unless it's that's part of a multi-PR change, then ignore that). Just wondering what the goal is with this change, as it keeps the Azure compatibility with the base OpenAI class when using config property but breaks it when using env vars?

I might be wrong on this one, but should also be lc_secrets() updated in the Azure implementation since the env vars are moved?

@sarangan12 any thoughts?

libs/langchain-openai/src/chat_models.ts Show resolved Hide resolved
@jacoblee93
Copy link
Collaborator Author

Can I ask why only moving the env vars and not all azure related properties, since it's a breaking change anyways? (Unless it's that's part of a multi-PR change, then ignore that). Just wondering what the goal is with this change, as it keeps the Azure compatibility with the base OpenAI class when using config property but breaks it when using env vars?

Yeah I think this is probably a good idea. CC @efriis

@jacoblee93
Copy link
Collaborator Author

jacoblee93 commented Jan 22, 2025

The minor bump and removal of defaults will also enable #7564

@jacoblee93 jacoblee93 changed the title feat(openai): Make only AzureOpenAI respect Azure env vars feat(openai): Make only AzureOpenAI respect Azure env vars, remove defaults Jan 22, 2025
@dosubot dosubot bot added size:XXL This PR changes 1000+ lines, ignoring generated files. and removed size:M This PR changes 30-99 lines, ignoring generated files. labels Jan 22, 2025
@jacoblee93
Copy link
Collaborator Author

@sinedied @efriis @ccurme have to verify tests but WDYT

Will make a different branch that I'll merge this into as well as the switch to json_schema for withStructuredOutput and treat that all as breaking changes for 0.4.0

libs/langchain-openai/src/chat_models.ts Show resolved Hide resolved
libs/langchain-openai/src/chat_models.ts Show resolved Hide resolved
libs/langchain-openai/src/chat_models.ts Outdated Show resolved Hide resolved
libs/langchain-openai/src/chat_models.ts Outdated Show resolved Hide resolved
libs/langchain-openai/src/embeddings.ts Outdated Show resolved Hide resolved
libs/langchain-openai/src/legacy.ts Outdated Show resolved Hide resolved
libs/langchain-openai/src/llms.ts Outdated Show resolved Hide resolved
@jacoblee93
Copy link
Collaborator Author

jacoblee93 commented Jan 24, 2025

Going to try to ship 0.4.0 today - @sinedied any other thoughts here?

@jacoblee93 jacoblee93 changed the title feat(openai): Make only AzureOpenAI respect Azure env vars, remove defaults feat(openai): Make only AzureOpenAI respect Azure env vars, remove class defaults, update withStructuredOutput defaults Jan 24, 2025
@dosubot dosubot bot added size:XL This PR changes 500-999 lines, ignoring generated files. and removed size:XXL This PR changes 1000+ lines, ignoring generated files. labels Jan 25, 2025
Copy link
Member

@efriis efriis left a comment

Choose a reason for hiding this comment

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

looks good - some small nits if helpful but you understand better than me here

Comment on lines +496 to +499
this.azureOpenAIApiVersion =
fields?.azureOpenAIApiVersion ??
fields?.openAIApiVersion ??
getEnvironmentVariable("AZURE_OPENAI_API_VERSION");
Copy link
Member

Choose a reason for hiding this comment

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

this feels funky - should we just update this.openAIApiVersion?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

WDYM?

this.azureOpenAIApiDeploymentName =
fields?.azureOpenAIApiDeploymentName ??
fields?.deploymentName ??
getEnvironmentVariable("AZURE_OPENAI_API_DEPLOYMENT_NAME");
Copy link
Member

Choose a reason for hiding this comment

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

this is not in py but we should add it in py

libs/langchain-openai/src/chat_models.ts Show resolved Hide resolved
libs/langchain-openai/src/embeddings.ts Show resolved Hide resolved
libs/langchain-openai/src/llms.ts Outdated Show resolved Hide resolved
@dosubot dosubot bot added the lgtm PRs that are ready to be merged as-is label Jan 25, 2025
@jacoblee93 jacoblee93 merged commit 90b06fa into main Jan 25, 2025
43 checks passed
@jacoblee93 jacoblee93 deleted the jacob/openai-rc branch January 25, 2025 01:44
FilipZmijewski added a commit to FilipZmijewski/langchainjs that referenced this pull request Jan 30, 2025
* Rename auth method in docs

* fix(core): Fix trim messages mutation bug (langchain-ai#7547)

* release(core): 0.3.31 (langchain-ai#7548)

* fix(community): Updated Embeddings URL (langchain-ai#7545)

* fix(community): make sure guardrailConfig can be added even with anthropic models (langchain-ai#7542)

* docs: Fix PGVectorStore import in install dependencies (TypeScript) example (langchain-ai#7533)

* fix(community): Airtable url (langchain-ai#7532)

* docs: Fix typo in OpenAIModerationChain example (langchain-ai#7528)

* docs: Resolves langchain-ai#7483, resolves langchain-ai#7274 (langchain-ai#7505)

Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* docs: Rename auth method in IBM docs (langchain-ai#7524)

* docs: correct misspelling (langchain-ai#7522)

Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* release(community): 0.3.25 (langchain-ai#7549)

* feat(azure-cosmosdb): add session context for a user mongodb (langchain-ai#7436)

Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* release(azure-cosmosdb): 0.2.7 (langchain-ai#7550)

* fix(ci): Fix build (langchain-ai#7551)

* feat(anthropic): Add Anthropic PDF support (document type) in invoke (langchain-ai#7496)

Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* release(anthropic): 0.3.12 (langchain-ai#7552)

* chore(core,langchain,community): Relax langsmith deps (langchain-ai#7556)

* release(community): 0.3.26 (langchain-ai#7557)

* release(core): 0.3.32 (langchain-ai#7558)

* Release 0.3.12 (langchain-ai#7559)

* fix(core): Prevent cache misses from triggering model start callback runs twice (langchain-ai#7565)

* fix(core): Ensure that cached flag in run extras is only set for cache hits (langchain-ai#7566)

* release(core): 0.3.33 (langchain-ai#7567)

* feat(community): Adds graph_document to export list (langchain-ai#7555)

Co-authored-by: quantropi-minh <101128605+quantropi-minh@users.noreply.github.com>
Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* fix(langchain): Fix ZeroShotAgent createPrompt with correct formatted tool names (langchain-ai#7510)

* docs: Add document for AzureCosmosDBMongoChatMessageHistory (langchain-ai#7519)

Co-authored-by: root <root@CPC-yangq-FRSGK>

* fix(langchain): Allow pulling hub prompts with associated models (langchain-ai#7569)

* fix(community,aws): Update handleLLMNewToken to include chunk metadata (langchain-ai#7568)

Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* feat(community): Provide fallback relationshipType in case it is not present in graph_transformer (langchain-ai#7521)

Co-authored-by: quantropi-minh <101128605+quantropi-minh@users.noreply.github.com>
Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* docs: Add redirect (langchain-ai#7570)

* fix(langchain,core): Add shim for hub mustache templates with nested input variables (langchain-ai#7581)

* fix(chat-models): honor disableStreaming even for `generateUncached` (langchain-ai#7575)

* release(core): 0.3.34 (langchain-ai#7584)

* feat(langchain): Add hub entrypoint with automatic dynamic entrypoint of models (langchain-ai#7583)

* chore(ollama): Export `OllamaEmbeddingsParams` interface (langchain-ai#7574)

* docs: Clarify tool creation process in structured outputs documentation (langchain-ai#7578)

Co-authored-by: Sahar Shemesh <sahar.shemesh@zoominfo.com>
Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* fix(community): Set awaitHandlers to true in upstash ratelimit (langchain-ai#7571)

Co-authored-by: Jacob Lee <jacoblee93@gmail.com>

* fix(core): Fix trim messages mutation (langchain-ai#7585)

* feat(openai): Make only AzureOpenAI respect Azure env vars, remove class defaults, update withStructuredOutput defaults (langchain-ai#7535)

* fix(community): Make postgresConnectionOptions optional in PostgresRecordManager (langchain-ai#7580)

Co-authored-by: jacoblee93 <jacoblee93@gmail.com>

* release(community): 0.3.27 (langchain-ai#7586)

* release(ollama): 0.1.5 (langchain-ai#7587)

* Release 0.3.13 (langchain-ai#7588)

* release(openai): 0.4.0 (langchain-ai#7589)

* release(core): 0.3.35 (langchain-ai#7590)

* fix(ci): Update lock (langchain-ai#7591)

* feat(core): Allow passing returnDirect in tool wrapper params (langchain-ai#7594)

* release(core): 0.3.36 (langchain-ai#7595)

* fix(openai): Revert Azure default withStructuredOutput changes (langchain-ai#7596)

* release(openai): 0.4.1 (langchain-ai#7597)

* feat(openai): Refactor to allow easier subclassing (langchain-ai#7598)

* release(openai): 0.4.2 (langchain-ai#7599)

* feat(deepseek): Adds Deepseek integration (langchain-ai#7604)

* release(deepseek): 0.0.1 (langchain-ai#7608)

* feat: update Novita AI doc (langchain-ai#7602)

* Add deployment chat to chat class

* feat(langchain): Add DeepSeek to initChatModel (langchain-ai#7609)

* Release 0.3.14 (langchain-ai#7611)

* fix: Add test for pdf uploads anthropic (langchain-ai#7613)

* feat: Update google genai to support file uploads (langchain-ai#7612)

* chore(google-genai): Drop .only in test (langchain-ai#7614)

* release(google-genai): 0.1.7 (langchain-ai#7615)

* Upadate Watsonx sdk

* fix(core): Fix stream events bug when errors are thrown too quickly during iteration (langchain-ai#7617)

* release(core): 0.3.37 (langchain-ai#7619)

* fix(langchain): Fix Groq import for hub (langchain-ai#7620)

* docs: update README/intro

* Release 0.3.15

* feat(community): improve support for Tavily search tool args (langchain-ai#7561)

* feat(community): Add boolean metadata type support in Supabase structured query translator (langchain-ai#7601)

* feat(google-genai): Add support for fileUri in media type in Google GenAI (langchain-ai#7621)

Co-authored-by: Jacob Lee <jacoblee93@gmail.com>

* release(google-genai): 0.1.8 (langchain-ai#7628)

* release(community): 0.3.28 (langchain-ai#7629)

* Rework interfaces in llms as well

* Bump watsonx-ai sdk version

* Remove unused code

* Add fake auth

* Fix broken changes

---------

Co-authored-by: Jacob Lee <jacoblee93@gmail.com>
Co-authored-by: Jacky Chen <jackychen4@gmail.com>
Co-authored-by: Mohamed Belhadj <medbelh@gmail.com>
Co-authored-by: Brian Ploetz <bploetz@gmail.com>
Co-authored-by: Eduard-Constantin Ibinceanu <ibinceanu.eduard@yahoo.com>
Co-authored-by: Jonathan V <jonathanvelkeneers@hotmail.com>
Co-authored-by: ucev <zhangshuaiyf@icloud.com>
Co-authored-by: crisjy <cjy1994116@163.com>
Co-authored-by: Adham Badr <adhambadr2@gmail.com>
Co-authored-by: Minh Ha <hlminh2000@gmail.com>
Co-authored-by: quantropi-minh <101128605+quantropi-minh@users.noreply.github.com>
Co-authored-by: Chi Thu Le <thu2004@yahoo.se>
Co-authored-by: fatmelon <708842811@qq.com>
Co-authored-by: root <root@CPC-yangq-FRSGK>
Co-authored-by: Mohamad Mohebifar <mohebifar@users.noreply.github.com>
Co-authored-by: David Duong <david@duong.cz>
Co-authored-by: Brace Sproul <braceasproul@gmail.com>
Co-authored-by: Matus Gura <contact@matusgura.com>
Co-authored-by: Sahar Shemesh <48128579+saharis9988@users.noreply.github.com>
Co-authored-by: Sahar Shemesh <sahar.shemesh@zoominfo.com>
Co-authored-by: Cahid Arda Öz <cahidardaooz@hotmail.com>
Co-authored-by: Jason <ggbbddjm@gmail.com>
Co-authored-by: vbarda <vadym@langchain.dev>
Co-authored-by: Vadym Barda <vadim.barda@gmail.com>
Co-authored-by: Hugo Borsoni <44852104+hugoleborso@users.noreply.github.com>
Co-authored-by: Arman Ghazaryan <arm.ghazaryan01@gmail.com>
Co-authored-by: Andy <andy+github@savage.hk>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
auto:improvement Medium size change to existing code to handle new use-cases lgtm PRs that are ready to be merged as-is size:XL This PR changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants