Skip to content

Conversation

CorieW
Copy link
Member

@CorieW CorieW commented Sep 10, 2025

Checklist (if applicable):

  • Tested (manually, unit tested, etc.)

@CorieW CorieW force-pushed the @invertase/migrate-googlegenai-plugin branch 8 times, most recently from 2f8295a to eb9aa5f Compare September 15, 2025 20:00
@cabljac cabljac requested a review from Copilot September 16, 2025 09:52
@@ -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
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// For conditional types in index.ts model
// For conditional types in index.ts createModelRef

maybe i think

Copy link

@Copilot Copilot AI left a 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 to createModelRef() 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);
Copy link
Preview

Copilot AI Sep 16, 2025

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.

Suggested change
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);
Copy link
Preview

Copilot AI Sep 16, 2025

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;
Copy link
Contributor

@cabljac cabljac Sep 16, 2025

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

Copy link
Contributor

@cabljac cabljac left a 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)

@cabljac cabljac self-requested a review September 16, 2025 09:58
Copy link
Contributor

@cabljac cabljac left a 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.

@CorieW CorieW force-pushed the @invertase/migrate-googlegenai-plugin branch from eb9aa5f to 3a8d64a Compare September 16, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants