feat(firestore-genai-chatbot): add responseMimeType param#674
feat(firestore-genai-chatbot): add responseMimeType param#674
Conversation
c7a4d05 to
5be1818
Compare
5be1818 to
ab1150d
Compare
954a161 to
256cd7d
Compare
6f3ec9a to
e08f43d
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new responseMimeType parameter to allow callers to specify the MIME type of generated responses.
- Extended type definitions and Zod override schema with
responseMimeType - Propagated the new field through Vertex, Google, and Genkit client calls and config loading
- Exposed the parameter in the extension manifest, README, tests, and bumped the
@google/generative-aidependency
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| functions/src/types.ts | Added responseMimeType to GenerateMessageOptions |
| functions/src/overrides.ts | Included responseMimeType in Zod overridesSchema |
| functions/src/generative-client/vertex_ai.ts | Passed responseMimeType into Vertex AI client options |
| functions/src/generative-client/google_ai.ts | Passed responseMimeType into Google AI client options |
| functions/src/generative-client/genkit.ts | Passed responseMimeType into Genkit client configuration |
| functions/src/config.ts | Added responseMimeType to Config interface and loaded from env |
| functions/package.json | Bumped @google/generative-ai from ^0.1.1 to ^0.11.0 |
| tests/vertex_ai/index.test.ts | Increased wait time in the no-op integration test |
| tests/unit/overrides.test.ts | Updated unit test to include responseMimeType override |
| extension.yaml | Exposed RESPONSE_MIME_TYPE parameter in extension parameters |
| README.md | Documented the new Response MIME Type parameter |
| CHANGELOG.md | Added version 0.0.15 entry for responseMimeType feature |
Files not reviewed (1)
- firestore-genai-chatbot/functions/package-lock.json: Language not supported
Comments suppressed due to low confidence (1)
firestore-genai-chatbot/functions/src/generative-client/vertex_ai.ts:90
- There are currently no unit tests asserting that
responseMimeTypeis correctly included in the generated API request. Adding a test to verify this propagation would help prevent regressions.
responseMimeType: options.responseMimeType,
| /** | ||
| * Allows specification of the format of the response. | ||
| */ | ||
| responseMimeType?: string; |
There was a problem hiding this comment.
[nitpick] The JSDoc comment for responseMimeType is quite generic. Consider specifying allowed or common MIME types (e.g., text/plain, application/json) or linking to a standard reference to clarify expected values.
|
|
||
| const expectNoOp = async () => { | ||
| await new Promise(resolve => setTimeout(resolve, 100)); | ||
| await new Promise(resolve => setTimeout(resolve, 1000)); |
There was a problem hiding this comment.
[nitpick] Using a real timeout to wait for asynchronous behavior can slow down the test suite. Consider using Jest's fake timers or mocking the relevant API call instead of increasing the delay.
| await new Promise(resolve => setTimeout(resolve, 1000)); | |
| jest.advanceTimersByTime(1000); |
Resolves #646