-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
openai-experimental[patch]: Adds experimental raw response field to OpenAI chat models #6087
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,24 @@ import { AIMessageChunk } from "@langchain/core/messages"; | |
import { AzureChatOpenAI } from "../../azure/chat_models.js"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey team, I've flagged the recent change in the PR for review as it explicitly accesses and sets environment variables using |
||
import { ChatOpenAICallOptions } from "../../chat_models.js"; | ||
|
||
beforeAll(() => { | ||
if (!process.env.AZURE_OPENAI_API_KEY) { | ||
process.env.AZURE_OPENAI_API_KEY = process.env.TEST_AZURE_OPENAI_API_KEY; | ||
} | ||
if (!process.env.AZURE_OPENAI_API_DEPLOYMENT_NAME) { | ||
process.env.AZURE_OPENAI_API_DEPLOYMENT_NAME = | ||
process.env.TEST_AZURE_OPENAI_API_DEPLOYMENT_NAME; | ||
} | ||
if (!process.env.AZURE_OPENAI_BASE_PATH) { | ||
process.env.AZURE_OPENAI_BASE_PATH = | ||
process.env.TEST_AZURE_OPENAI_BASE_PATH; | ||
} | ||
if (!process.env.AZURE_OPENAI_API_VERSION) { | ||
process.env.AZURE_OPENAI_API_VERSION = | ||
process.env.TEST_AZURE_OPENAI_API_VERSION; | ||
} | ||
}); | ||
|
||
class AzureChatOpenAIStandardIntegrationTests extends ChatModelIntegrationTests< | ||
ChatOpenAICallOptions, | ||
AIMessageChunk | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,25 @@ | ||
/* eslint-disable no-process-env */ | ||
import { test, expect } from "@jest/globals"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey there! I noticed that the recent PR includes changes to access and set environment variables using |
||
import { AzureOpenAIEmbeddings as OpenAIEmbeddings } from "../../azure/embeddings.js"; | ||
|
||
beforeAll(() => { | ||
if (!process.env.AZURE_OPENAI_API_KEY) { | ||
process.env.AZURE_OPENAI_API_KEY = process.env.TEST_AZURE_OPENAI_API_KEY; | ||
} | ||
if (!process.env.AZURE_OPENAI_API_DEPLOYMENT_NAME) { | ||
process.env.AZURE_OPENAI_API_DEPLOYMENT_NAME = | ||
process.env.TEST_AZURE_OPENAI_API_DEPLOYMENT_NAME; | ||
} | ||
if (!process.env.AZURE_OPENAI_BASE_PATH) { | ||
process.env.AZURE_OPENAI_BASE_PATH = | ||
process.env.TEST_AZURE_OPENAI_BASE_PATH; | ||
} | ||
if (!process.env.AZURE_OPENAI_API_VERSION) { | ||
process.env.AZURE_OPENAI_API_VERSION = | ||
process.env.TEST_AZURE_OPENAI_API_VERSION; | ||
} | ||
}); | ||
|
||
test("Test AzureOpenAIEmbeddings.embedQuery", async () => { | ||
const embeddings = new OpenAIEmbeddings(); | ||
const res = await embeddings.embedQuery("Hello world"); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,24 @@ import { AzureOpenAI } from "../../azure/llms.js"; | |
// Save the original value of the 'LANGCHAIN_CALLBACKS_BACKGROUND' environment variable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey team, just a heads up that I've flagged the recent changes in the |
||
const originalBackground = process.env.LANGCHAIN_CALLBACKS_BACKGROUND; | ||
|
||
beforeAll(() => { | ||
if (!process.env.AZURE_OPENAI_API_KEY) { | ||
process.env.AZURE_OPENAI_API_KEY = process.env.TEST_AZURE_OPENAI_API_KEY; | ||
} | ||
if (!process.env.AZURE_OPENAI_API_DEPLOYMENT_NAME) { | ||
process.env.AZURE_OPENAI_API_DEPLOYMENT_NAME = | ||
process.env.TEST_AZURE_OPENAI_API_DEPLOYMENT_NAME; | ||
} | ||
if (!process.env.AZURE_OPENAI_BASE_PATH) { | ||
process.env.AZURE_OPENAI_BASE_PATH = | ||
process.env.TEST_AZURE_OPENAI_BASE_PATH; | ||
} | ||
if (!process.env.AZURE_OPENAI_API_VERSION) { | ||
process.env.AZURE_OPENAI_API_VERSION = | ||
process.env.TEST_AZURE_OPENAI_API_VERSION; | ||
} | ||
}); | ||
|
||
test("Test Azure OpenAI invoke", async () => { | ||
const model = new AzureOpenAI({ | ||
maxTokens: 5, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey team, I've flagged the added
beforeAll
block in the PR for review as it explicitly sets environment variables usingprocess.env
. This change should be reviewed to ensure it aligns with our environment variable management practices.