refactor(ai): update WebhookOptions, Gemini, and Vertex types - #1952
Conversation
### Description - Remove regionalWebhook field and array location typing from WebhookOptions in AI provider. - Remove labels field from Vertex GenerateContentRequest. - Explicitly add tools and toolConfig fields to Vertex GenerateContentRequest. - Remove generateContentStream reference from JSDoc comments in Gemini v1beta. ### Scenarios Tested - Ran unit tests in spec/v2/providers/ai.spec.ts - Ran npm test and npm run format
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request simplifies the AI provider webhooks by removing the generic Regional option and regionalWebhook configurations from WebhookOptions, beforeGenerateContent, and afterGenerateContent. It also cleans up related tests, unused imports, and updates type definitions for Gemini and Vertex AI. Feedback was provided regarding a redundant definition of tools and toolConfig in GenerateContentRequest within the Vertex AI types, as these are already inherited from BaseModelParams.
# Conflicts: # src/v2/providers/ai/index.ts # src/v2/providers/ai/types/gemini/v1beta/index.ts
…erateContentRequest
ajperel
left a comment
ajperel
left a comment
There was a problem hiding this comment.
Nit: It's weird to have "relnote:" repeated with different messages at the top and bottom of your PR description. Maybe best to have it in only one place though I'm not too fussed on whether it's the top or the bottom. I would have done the bottom since it looks git-footer like.
|
|
||
| it("should allow global webhooks to specify a single location", () => { | ||
| it("should allow webhooks to specify a single region", () => { | ||
| ai.beforeGenerateContent({ region: "us-central1" }, () => {}); |
There was a problem hiding this comment.
Do you think we'd benefit from the test verifying that region is correctly populated in the the Endpoint object after this.
| * the stream is done. | ||
| * Individual response from {@link GenerativeModel.generateContent}. |
There was a problem hiding this comment.
The {@link} here would cause the same issue as it did earlier. The api-documenter is unable to resolve it and that leaves broken sentences in the docgen files.
We might want to put it in just bacticks like *Individual response from generateContent
There was a problem hiding this comment.
| * the stream is done. | |
| * Individual response from {@link GenerativeModel.generateContent}. | |
| * Individual response from `generateContent`. |
marb2000
left a comment
marb2000
left a comment
There was a problem hiding this comment.
LGTM. In sync with we we talked internally
6 participants
relnote: Refactor WebhookOptions, Gemini, and Vertex AI provider types in v2/ai
Description
Scenarios Tested
relnote: remove unused fields from FAL functions