From 69213fbbed29f12ac83b01abe694ae30df996f8e Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 24 Apr 2025 13:28:05 -0400 Subject: [PATCH 1/5] fix(vertexai): pass `GenerativeModel`'s `BaseParams` to `ChatSession` --- .../src/models/generative-model.test.ts | 29 +++++++++++++++++++ .../vertexai/src/models/generative-model.ts | 7 +++++ 2 files changed, 36 insertions(+) diff --git a/packages/vertexai/src/models/generative-model.test.ts b/packages/vertexai/src/models/generative-model.test.ts index 987f9b115e2..849aa0c0880 100644 --- a/packages/vertexai/src/models/generative-model.test.ts +++ b/packages/vertexai/src/models/generative-model.test.ts @@ -165,6 +165,35 @@ describe('GenerativeModel', () => { ); restore(); }); + it('passes base model params through to ChatSession when there are no startChatParams', async () => { + const genModel = new GenerativeModel(fakeVertexAI, { + model: 'my-model', + generationConfig: { + topK: 1 + } + }); + const chatSession = genModel.startChat(); + expect(chatSession.params?.generationConfig).to.deep.equal({ + topK: 1 + }); + restore(); + }); + it('overrides base model params with startChatParams', () => { + const genModel = new GenerativeModel(fakeVertexAI, { + model: 'my-model', + generationConfig: { + topK: 1 + } + }); + const chatSession = genModel.startChat({ + generationConfig: { + topK: 2 + } + }); + expect(chatSession.params?.generationConfig).to.deep.equal({ + topK: 2 + }); + }); it('passes params through to chat.sendMessage', async () => { const genModel = new GenerativeModel(fakeVertexAI, { model: 'my-model', diff --git a/packages/vertexai/src/models/generative-model.ts b/packages/vertexai/src/models/generative-model.ts index 983118bf6ff..1af1ee700d5 100644 --- a/packages/vertexai/src/models/generative-model.ts +++ b/packages/vertexai/src/models/generative-model.ts @@ -132,6 +132,13 @@ export class GenerativeModel extends VertexAIModel { tools: this.tools, toolConfig: this.toolConfig, systemInstruction: this.systemInstruction, + generationConfig: this.generationConfig, + safetySettings: this.safetySettings, + /** + * Overrides params inherited from GenerativeModel with those explicitly set in the + * StartChatParams. For example, if startChatParams.generationConfig is set, it'll override + * this.generationConfig. + */ ...startChatParams }, this.requestOptions From 8801ee3f2465ac40251db66b4c4c54881d8561b0 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 24 Apr 2025 13:32:39 -0400 Subject: [PATCH 2/5] Add changeset --- .changeset/fast-mangos-chew.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/fast-mangos-chew.md diff --git a/.changeset/fast-mangos-chew.md b/.changeset/fast-mangos-chew.md new file mode 100644 index 00000000000..c5d2e4b4d1f --- /dev/null +++ b/.changeset/fast-mangos-chew.md @@ -0,0 +1,5 @@ +--- +'@firebase/vertexai': patch +--- + +Pass `GenerativeModel`'s `BaseParams` to created chat sessions. This fixes an issue where `GenerationConfig` would not be inherited from `ChatSession`. From 6a4a3248963a521a1e6d8a8d8fe9288bd75de799 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 1 May 2025 15:23:12 -0400 Subject: [PATCH 3/5] move test case --- .../src/models/generative-model.test.ts | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/packages/vertexai/src/models/generative-model.test.ts b/packages/vertexai/src/models/generative-model.test.ts index 849aa0c0880..28a56d9deb0 100644 --- a/packages/vertexai/src/models/generative-model.test.ts +++ b/packages/vertexai/src/models/generative-model.test.ts @@ -165,19 +165,6 @@ describe('GenerativeModel', () => { ); restore(); }); - it('passes base model params through to ChatSession when there are no startChatParams', async () => { - const genModel = new GenerativeModel(fakeVertexAI, { - model: 'my-model', - generationConfig: { - topK: 1 - } - }); - const chatSession = genModel.startChat(); - expect(chatSession.params?.generationConfig).to.deep.equal({ - topK: 1 - }); - restore(); - }); it('overrides base model params with startChatParams', () => { const genModel = new GenerativeModel(fakeVertexAI, { model: 'my-model', @@ -201,7 +188,10 @@ describe('GenerativeModel', () => { { functionDeclarations: [{ name: 'myfunc', description: 'mydesc' }] } ], toolConfig: { functionCallingConfig: { mode: FunctionCallingMode.NONE } }, - systemInstruction: { role: 'system', parts: [{ text: 'be friendly' }] } + systemInstruction: { role: 'system', parts: [{ text: 'be friendly' }] }, + generationConfig: { + topK: 1 + } }); expect(genModel.tools?.length).to.equal(1); expect(genModel.toolConfig?.functionCallingConfig?.mode).to.equal( @@ -225,7 +215,8 @@ describe('GenerativeModel', () => { return ( value.includes('myfunc') && value.includes(FunctionCallingMode.NONE) && - value.includes('be friendly') + value.includes('be friendly') && + value.includes('topK') ); }), {} From 11718f5d55c164cffee8fc432a5e81e150e91f51 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Thu, 1 May 2025 15:26:26 -0400 Subject: [PATCH 4/5] add test case for override issue --- .../vertexai/src/models/generative-model.test.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/packages/vertexai/src/models/generative-model.test.ts b/packages/vertexai/src/models/generative-model.test.ts index 28a56d9deb0..b3fcd593e03 100644 --- a/packages/vertexai/src/models/generative-model.test.ts +++ b/packages/vertexai/src/models/generative-model.test.ts @@ -256,7 +256,10 @@ describe('GenerativeModel', () => { { functionDeclarations: [{ name: 'myfunc', description: 'mydesc' }] } ], toolConfig: { functionCallingConfig: { mode: FunctionCallingMode.NONE } }, - systemInstruction: { role: 'system', parts: [{ text: 'be friendly' }] } + systemInstruction: { role: 'system', parts: [{ text: 'be friendly' }] }, + generationConfig: { + responseMimeType: 'image/jpeg' + } }); expect(genModel.tools?.length).to.equal(1); expect(genModel.toolConfig?.functionCallingConfig?.mode).to.equal( @@ -282,7 +285,10 @@ describe('GenerativeModel', () => { toolConfig: { functionCallingConfig: { mode: FunctionCallingMode.AUTO } }, - systemInstruction: { role: 'system', parts: [{ text: 'be formal' }] } + systemInstruction: { role: 'system', parts: [{ text: 'be formal' }] }, + generationConfig: { + responseMimeType: 'image/png' + } }) .sendMessage('hello'); expect(makeRequestStub).to.be.calledWith( @@ -294,7 +300,9 @@ describe('GenerativeModel', () => { return ( value.includes('otherfunc') && value.includes(FunctionCallingMode.AUTO) && - value.includes('be formal') + value.includes('be formal') && + value.includes('image/png') && + !value.includes('image/jpeg') ); }), {} From 102fb8c850958dc9fe4a8a0a544d131e764828b3 Mon Sep 17 00:00:00 2001 From: Daniel La Rocque Date: Fri, 2 May 2025 16:14:52 -0400 Subject: [PATCH 5/5] remove test --- .../vertexai/src/models/generative-model.test.ts | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/packages/vertexai/src/models/generative-model.test.ts b/packages/vertexai/src/models/generative-model.test.ts index b3fcd593e03..51ea8aafead 100644 --- a/packages/vertexai/src/models/generative-model.test.ts +++ b/packages/vertexai/src/models/generative-model.test.ts @@ -165,22 +165,6 @@ describe('GenerativeModel', () => { ); restore(); }); - it('overrides base model params with startChatParams', () => { - const genModel = new GenerativeModel(fakeVertexAI, { - model: 'my-model', - generationConfig: { - topK: 1 - } - }); - const chatSession = genModel.startChat({ - generationConfig: { - topK: 2 - } - }); - expect(chatSession.params?.generationConfig).to.deep.equal({ - topK: 2 - }); - }); it('passes params through to chat.sendMessage', async () => { const genModel = new GenerativeModel(fakeVertexAI, { model: 'my-model',