-
Notifications
You must be signed in to change notification settings - Fork 416
refactor(plugins/google-genai): migrate to v2 API #3542
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
base: main
Are you sure you want to change the base?
Conversation
2f8295a
to
eb9aa5f
Compare
@@ -126,13 +126,13 @@ const KNOWN_MODELS = { | |||
} as const; | |||
export type KnownModels = keyof typeof KNOWN_MODELS; // For autocomplete | |||
|
|||
// For conditional types in index.ts model() | |||
// For conditional types in index.ts model |
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.
// For conditional types in index.ts model | |
// For conditional types in index.ts createModelRef |
maybe i think
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.
Pull Request Overview
This PR migrates the Google Genkit plugin from the v1 to v2 API, refactoring how models, embedders, and background models are defined and managed. The migration simplifies the plugin architecture by removing the explicit Genkit instance parameter and transitioning to a more functional approach.
- Rename
model()
functions tocreateModelRef()
for consistency - Migrate from v1 plugin architecture to v2 with updated initialization and resolution patterns
- Update function signatures to remove Genkit instance parameters and return action objects directly
Reviewed Changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
js/plugins/google-genai/tests/vertexai/veo_test.ts | Update test mocks and function calls for v2 API migration |
js/plugins/google-genai/tests/vertexai/lyria_test.ts | Replace model() calls with createModelRef() and update defineModel calls |
js/plugins/google-genai/tests/vertexai/index_test.ts | Update test assertions and function calls for v2 API changes |
js/plugins/google-genai/tests/vertexai/imagen_test.ts | Replace model function references and update defineModel signature |
js/plugins/google-genai/tests/vertexai/gemini_test.ts | Update createModelRef calls and remove Genkit parameter from defineModel |
js/plugins/google-genai/tests/vertexai/embedder_test.ts | Update defineEmbedder calls to remove Genkit parameter |
js/plugins/google-genai/tests/googleai/veo_test.ts | Replace model() with createModelRef() and update test setup |
js/plugins/google-genai/tests/googleai/index_test.ts | Update test structure for v2 plugin API and function renaming |
js/plugins/google-genai/tests/googleai/imagen_test.ts | Replace model calls with createModelRef and update defineModel signature |
js/plugins/google-genai/tests/googleai/gemini_test.ts | Update function calls and remove Genkit parameters from model definitions |
js/plugins/google-genai/tests/googleai/embedder_test.ts | Update embedder definition calls to remove Genkit parameter |
js/plugins/google-genai/src/vertexai/veo.ts | Implement v2 API with createModelRef and backgroundModel functions |
js/plugins/google-genai/src/vertexai/lyria.ts | Migrate to v2 API removing Genkit parameter and using model function |
js/plugins/google-genai/src/vertexai/index.ts | Complete plugin migration to v2 architecture with new init/resolve pattern |
js/plugins/google-genai/src/vertexai/imagen.ts | Update to v2 API with createModelRef and model function |
js/plugins/google-genai/src/vertexai/gemini.ts | Migrate gemini models to v2 API and remove Genkit parameters |
js/plugins/google-genai/src/vertexai/embedder.ts | Update embedder implementation for v2 API with new function signatures |
js/plugins/google-genai/src/googleai/veo.ts | Implement v2 changes with createModelRef and backgroundModel |
js/plugins/google-genai/src/googleai/index.ts | Complete googleAI plugin migration to v2 architecture |
js/plugins/google-genai/src/googleai/imagen.ts | Update imagen models to v2 API pattern |
js/plugins/google-genai/src/googleai/gemini.ts | Migrate gemini implementation to v2 API structure |
js/plugins/google-genai/src/googleai/embedder.ts | Update embedder for v2 API with new function signatures |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
fetchStub = sinon.stub(global, 'fetch'); | ||
authMock = sinon.createStubInstance(GoogleAuth); | ||
backgroundModelStub = sinon.stub(backgroundModel as any); |
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.
Avoid using as any
type assertion. Consider creating a proper stub interface or using more specific typing for the backgroundModel stub to maintain type safety.
backgroundModelStub = sinon.stub(backgroundModel as any); | |
backgroundModelStub = sinon.stub<typeof backgroundModel>(backgroundModel); |
Copilot uses AI. Check for mistakes.
fetchStub = sinon.stub(global, 'fetch'); | ||
envStub = sinon.stub(process, 'env').value({}); | ||
backgroundModelStub = sinon.stub(backgroundModel as any); |
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.
Avoid using as any
type assertion. Consider creating a proper stub interface or using more specific typing for the backgroundModel stub to maintain type safety.
Copilot uses AI. Check for mistakes.
options?: GoogleAIPluginOptions | ||
): GenkitPluginV2 { | ||
// Cache for list function - per plugin instance | ||
let listCache: any[] | null = null; |
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.
Can we avoid introducing an explicit any
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.
lgtm, make sure to test in dev UI to ensure we're not changing the display names of anything (same goes for all model plugin migrations)
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.
generally lgtm - one comment about an any
we've introduced, would rather not.
eb9aa5f
to
3a8d64a
Compare
Checklist (if applicable):